chore(suite): narrow constructs.lua skip with per-failure tracking

[davydog187]

Summary

Narrow the single constructs.lua mega-skip (left by A42 / #278) down to the
specific lines that still fail against the current tree, each with a standalone
reason.

The os stdlib (#289) and debug.getinfo name resolution (#290) shipped on main,
so the earlier debug.getinfo, os.time, and GLOB1-concat skips are gone —
those lines pass now. What remains are the short-circuit harness (a timeout, not
a VM defect) and the two checkload error-message assertions.

Skip ranges (was: single mega-skip, "pending separate triage")

Lines	Category	Issue	Cause
284..299	:performance	none	level=4 combination explosion: createcases(4) plus the load()/exec loop over all 204105 combinations exceeds the 60s ExUnit default timeout. The VM result is correct — verified green by test/lua/vm/short_circuit_test.exs --include slow.
303..304	:stdlib	none	checkload expects the load() error to contain expected; the suite-runner load() returns parse error: ... for syntax errors.
306..311	:stdlib	none	checkload expects the load() error to contain too long; the compiler has no control-structure-too-long check.

Suite delta

• lua53 suite: 13/13 files passing (unchanged).
• constructs.lua: single mega-skip → three narrow, disjoint ranges.

Out of scope

• The 60s timeout on the level-4 short-circuit harness (it is a correctness-verified
  performance ceiling, not a VM bug).
• Making the suite-runner load() error templates match PUC-Lua's expected /
  too long substrings, and adding a control-structure-too-long compiler check.

Test plan

• mix test
• mix test --only lua53 — 13 passed
• mix format / mix compile --warnings-as-errors clean

Related: #254 / #278 (A42 paren-adjust).

[davydog187]

Automated review (round 1)

Verdict: request changes — this PR is stale and conflicts with main. GitHub reports the branch as CONFLICTING / DIRTY. Two features this PR is built around have already merged to main, so the diff re-introduces skips and pins behavior that no longer matches reality.

Blockers

1. debug.getinfo name resolution already landed (#290) — pinning tests are now wrong.
cd3b48e feat(vm): populate debug.getinfo name/namewhat from the call site (#290) is on main. On the current tree, debug.getinfo(1, "n") for a global F returns ["F", "global"], not [nil, nil].

• test/lua/vm/debug_getinfo_name_test.exs asserts assert [nil, nil] = eval!(code) — this test fails against current main. The @tag :skip 'desired' test is now the actual behavior and should be the live assertion (it already exists on main via feat(vm): populate debug.getinfo name/namewhat from the call site #290's test).
• The 225..231 skip (issue debug.getinfo(level, 'n') returns nil for name and namewhat #279) for 'debug.getinfo returns nil' is stale; line 226 passes on main.

2. os stdlib already landed (#289) — skip ranges 237 / 248 are obsolete.
2f63f48 feat(stdlib): implement the os standard library (#289) is on main (lib/lua/vm/stdlib/os.ex). os.time exists, so the 237 and 248 skips (issue #280) are no longer needed.

3. Direct conflict on test/lua53_skips.exs.
main already narrowed constructs.lua to a single 232..313 entry (issue #280) whose reason notes the debug.getinfo block now passes and the short-circuit harness is green to level 4 via test/lua/vm/short_circuit_test.exs. This PR rewrites the same key back to five ranges off the old 225..313 base. These cannot both exist; the merge conflicts.

4. A43 plan-id collision.
main already has .agents/plans/A43-os-library.md (id: A43, issue 280, pr 289, status review). This PR adds .agents/plans/A43-os-stdlib.md (id: A43, issue 280) — a second file claiming id A43 for the same issue, describing work that already shipped.

Recommendation

Rebase onto current origin/main and re-triage constructs.lua from scratch. After the rebase, the only plausibly-still-valid skips are the load()/checkload error-message range (302..311) and possibly the level=4 short-circuit timeout (284..299) — but both need re-verification against the post-#289/#290 tree, since main's current single entry already claims the harness is green to level 4. Drop debug_getinfo_name_test.exs (superseded by #290's tests) and A43-os-stdlib.md (superseded by A43-os-library.md).

Generated by automated review tooling.

[davydog187]

Automated review — round 2

Reviewed the current remote state of chore/constructs-triage-narrow (HEAD 3bb680a). The branch was re-triaged after the os stdlib (#289) and debug.getinfo name resolution (#290) landed, so the diff against main now only narrows the single constructs.lua 232..313 skip into two disjoint ranges: 284..299 (#281) and 302..311 (no issue).

Validation run locally (detached HEAD off the branch):

• mix format --check-formatted, mix compile --warnings-as-errors — clean.
• mix test --only lua53 — 17 passed, 12 skipped. constructs.lua green.
• mix test test/lua/vm/short_circuit_test.exs --include slow — 15 passed, including the level-4 generative harness asserting all 204,105 cases match value + branch side effect (~420s).
• Verified 302..311: the suite-runner load() returns title-cased "...Expected '=' or 'in'...", so string.find(err, "expected") (lowercase) genuinely fails; and the compiler has no control-structure-too long limit. Reason is accurate.

Findings

1. (major) 284..299 is tagged category: :executor / issue: 281, but #281 is CLOSED as COMPLETED and the executor is verified correct.
The slow level-4 test I ran proves the short-circuit executor produces correct results for all 204,105 compositions — i.e. the bug #281 tracked is fixed. The reason text even says so: "The harness logic itself is verified green up to level 4." The only genuine reason this range stays skipped is the 60s ExUnit timeout (the level-4 run needs timeout: :infinity / ~420s). Pointing a skip at a closed bug issue with an :executor category is misleading: a future reader following #281 finds a completed fix and would reasonably conclude the skip is stale/removable, or that there's a live VM defect (there isn't). Suggest: drop issue: 281, recategorize as a performance/timeout skip (the reason should lead with "exceeds the 60s test timeout"), and either open a perf-tracking issue or set issue: nil.

2. (minor) PR description is stale vs. the round-2 diff.
The body still describes five ranges (#279/#280 entries, A43-os-stdlib.md, the debug_getinfo_name_test.exs additions) that no longer exist in the diff — the commit body says these were dropped because #289/#290 shipped. Please update the PR description to match the actual two-range diff so reviewers/changelog readers aren't misled.

3. (nit) 302..311 could be 303..304 + 306..311 to exclude the comment/blank at 302 and the separator at 300–301, but a single tight range covering both the checkload(..., "expected") calls and the too long block is reasonable; not worth churning.

No plan-id references in the skip reasons, scope chore(suite) is correct, no Co-Authored-By. The narrowing itself is correct and the suite stays green.

[davydog187]

Automated review — round 3

Re-reviewed the current remote tip of chore/constructs-triage-narrow (net diff is test/lua53_skips.exs only, +19/-4).

Verdict: LGTM — ship it.

Verified against the actual tree:

• Suite green. Swapped the PR's skip file in and ran mix test test/lua53_suite_test.exs --only lua53 → 17 passed, 12 skipped; constructs.lua prints testing syntax … OK.
• No stale ranges. mix lua.suite --audit reports 0 stale entries, 0 promotion candidates. All three ranges are still required.
• 284..299 (:performance). Honest categorization — the VM result is correct; the only failure is the level-4 combination explosion exceeding the 60s ExUnit timeout. Range is minimal (createcases(4) at 284 through the load/exec loop at 299); the 300..302 comment/blank gap is correctly left active.
• 303..304 (:stdlib, 'expected'). Confirmed independently: with 284..299 and 306..311 skipped, this range still fails (checkload assertion). The runner's load() returns parse error: …, and the parser emits Expected '=' or 'in' after for variable — capital E, so the case-sensitive string.find(msg, 'expected') will not match. Reason is accurate.
• 306..311 (:stdlib, 'too long'). Confirmed the compiler has no control-structure-too-long check, so load(s) succeeds, returns no error message, and checkload(s, 'too long') fails. Reason is accurate.
• Conventions. Scope chore(suite) is correct; no plan-id in reason strings (Plan: A26 lives only in commit bodies, which is allowed); none of the three PR commits carry a Co-Authored-By trailer.
• Doc comment correctly extends the category enum with :performance. Note: category is documentational only — load_skip_map! validates just the :all-vs-range conflict — so the new value is safe.

No blocking or actionable findings.