feat(emit-rust): emit_rust_fixtures_rustc_green gate test

[briansrls]

Summary

• Adds emit_rust_fixtures_rustc_green to m1_3_emit_rust_test.rs — a single #[ignore]d gate test that sweeps all 9 program fixtures and 5 reflected-module fixtures through the batched rustc roundtrip harness
• Confirms the full Rust emit baseline is green before the test is declared passing
• All 14 non-ignored rustc_roundtrip_* tests already passed; this test encodes that invariant as a named gate

Test plan

• cargo test -p v3-compiler --test integration emit_rust_fixtures_rustc_green -- --ignored → ok
• cargo clippy --all-targets -- -D warnings → clean

🤖 Generated with Claude Code

[briansrls]

Review metadata

• Provider / model: claude / claude-opus-4-7
• Commit: fba61b8e · Trigger: schedule
• Thinking: 29s wall

---

Verdict: APPROVE — diff adds a single #[ignore]d aggregator test that exercises the existing run_program/run_reflected helpers across the 9 program + 5 reflected fixtures. Test-only, no substrate impact, fail-closed (collects failures and asserts), helpers and case names line up with the existing individual rustc_roundtrip_* tests.

Optional observation: the aggregator duplicates assertions already covered by the per-fixture rustc_roundtrip_* tests. That's fine as a one-shot gate, but if the per-fixture tests stay around, drift between the two lists is a future maintenance hazard — worth considering whether the per-fixture tests could be generated from (or replaced by) this single matrix once the design settles.

[briansrls]

Review finding:

The new gate test’s run instructions are wrong. This file is compiled into the shared integration test binary via src/v3/compiler/tests/integration.rs, not as a standalone Cargo test target named m1_3_emit_rust_test.

So the doc comment currently tells readers to run a command that won’t work:

cargo test -p v3-compiler --test m1_3_emit_rust_test ...

The same issue appears in the existing nearby ignored roundtrip comment too, but this PR is adding a new named gate receipt, so it’s a good place to fix the invocation. It should point at the real integration target, e.g.:

cargo test -p v3-compiler --test integration emit_rust_fixtures_rustc_green -- --ignored --nocapture

Otherwise the gate is harder to reproduce exactly when someone tries to run the documented receipt.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fba61b8ebb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Drive gate coverage from fixture lists

This gate hard-codes PROGRAM_CASES/REFLECTED_CASES instead of deriving coverage from PROGRAM_FIXTURES and REFLECTED_FIXTURES, so adding a new fixture can leave emit_rust_fixtures_rustc_green green while silently skipping that new case. In that scenario the test no longer enforces the stated “full fixture matrix” invariant and can give a false sense of baseline health until someone manually updates these arrays.

Useful? React with 👍 / 👎.

[briansrls]

Review metadata

• Provider / model: codex / gpt-5.4
• Commit: fba61b8e · Trigger: schedule
• Thinking: 354s wall

---

Findings

• src/v3/compiler/tests/boundary/m1_3_emit_rust_test.rs:857 / :870 The new gate introduces PROGRAM_CASES and REFLECTED_CASES as a second hand-maintained definition of the fixture matrix, even though this file already has PROGRAM_FIXTURES and REFLECTED_FIXTURES as the harness authorities. That violates docs/modeling-discipline.md practice 5 / INVARIANTS.md P2 single-authority metadata, and it cuts against TESTING.md’s cost-of-change guidance: adding or renaming a fixture can leave this “full matrix” gate stale while still passing.

Verdict: APPROVE_WITH_COMMENTS. The added test looks behaviorally fine, and I don’t see a concrete fail-closed or modeling regression in the Rust emission path itself. I did not wait for a full cargo test -p v3-compiler --test integration --no-run workspace build to finish; this review is based on the diff and file context.

[briansrls]

Review metadata

• Provider / model: claude / claude-opus-4-7
• Commit: 10f64d2d · Trigger: schedule
• Thinking: 37s wall

---

Verdict: APPROVE — Test-only, #[ignore]-gated additive change. Adds expected_stdout to fixture tables and a single "green matrix" gate test that reuses run_program/run_reflected helpers. Values match the hard-coded asserts in the existing individual rustc_roundtrip_* tests. Doc-comment cargo test invocations are corrected to use --test integration. No substrate touched, no invariant implicated.

Exploratory observations (optional):

• expected_stdout on fixtures now parallels the hard-coded literals inside the individual rustc_roundtrip_* tests (e.g. assert_eq!(stdout, "6", ...) at line 745 vs. expected_stdout: "6" in PROGRAM_FIXTURES). Not duplication-of-authority in the invariant sense, but a future tidy would be to have the individual tests assert against fixture.expected_stdout so there's one place to edit when a fixture's expected output changes.
• The "+" sentinel for node_count is a small stringly-typed shortcut in a struct otherwise promising exact equality. If more "fuzzy" expectations appear, consider enum Expected { Exact(&'static str), PositiveInt } — cheap today, but worth naming.

[briansrls]

Review metadata

• Provider / model: codex / gpt-5.4
• Commit: 7db3e498 · Trigger: schedule
• Thinking: 236s wall

---

Findings

• NON-BLOCKING — src/v3/compiler/tests/integration/common/determinism_fixtures.rs:15 and src/v3/compiler/tests/boundary/m1_3_emit_rust_test.rs:201 add shared expected_stdout fields for the new aggregate gate, but the existing per-fixture rustc_roundtrip_* tests in m1_3_emit_rust_test.rs and m1_3_emit_rust_test.rs still hardcode the same expectations. That creates two independent authorities for the same expected behavior, which is the kind of drift risk P2 / “single-authority metadata” is trying to avoid. I’d prefer the individual checks to derive from the shared table, or the gate to reuse the existing assertions indirectly, so a fixture output change only has one place to update.

Verdict
APPROVE_WITH_COMMENTS

The diff is otherwise clean and appropriately scoped for a boundary test addition; I didn’t see a concrete fail-closed, modeling, or coding-discipline regression in the changed lines. I also ran cargo test -p v3-compiler --test integration emit_rust_fixtures_rustc_green -- --ignored --nocapture, and the new gate passed locally.

[briansrls]

Review metadata

• Provider / model: claude / claude-opus-4-7
• Commit: bd8c490d · Trigger: schedule
• Thinking: 21s wall

---

Verdict: APPROVE — diff is test-only, narrowly scoped. Expected stdout is lifted onto ProgramFixture (single source of truth) and a new #[ignore]d gate test exercises the full fixture matrix end-to-end. No substrate or modeling concerns; ReflectedExpected is a file-local implementation enum, and PositiveInt is a deliberate loose check for node_count whose exact value isn't pinned. Nothing in this diff violates INVARIANTS/MODELING/CODING/TESTING.

Exploratory observations (optional):

• The nine individual rustc_roundtrip_* program tests are now pure wrappers around run_program(name) + program_expected(name). They could collapse into a single parameterized test (or just be subsumed by emit_rust_fixtures_rustc_green), since the gate test now duplicates their coverage. Keeping them gives per-fixture test names in output, which is the likely intent — worth naming that tradeoff if someone later asks "why both?"
• Minor: the doc-comment block added above ReflectedFixture (lines 194-202 in the new file) sits between two comment paragraphs that used to flow together. The "Batching all fixtures into one compilation…" paragraph and the new "Expected output shape…" paragraph now read as if they document the same item. A blank /// separator or moving the new enum's doc above the enum only (not adjacent to the struct's prose) would read cleaner.

[briansrls]

Review metadata

• Provider / model: codex / gpt-5.4
• Commit: 3e44003b · Trigger: schedule
• Thinking: 192s wall

---

Findings

• [NON-BLOCKING] src/v3/compiler/tests/boundary/m1_3_emit_rust_test.rs:209 adds expected_stdout as a second authority for reflected-fixture expectations, while the individual reflected tests in the same file still pin "2", "1", etc. inline. That violates P2 / single-authority metadata from INVARIANTS.md and docs/modeling-discipline.md: reflected expected outputs now have to stay in sync in two places.
• [NON-BLOCKING] src/v3/compiler/tests/boundary/m1_3_emit_rust_test.rs:934 says the new gate is ignored “for the same reason as the individual roundtrip tests,” but the individual fixture tests above are intentionally unignored. That comment no longer describes the live state, which conflicts with the “documentation describes live state” guidance in INVARIANTS.md.

Verdict
APPROVE_WITH_COMMENTS — the diff is small and the program-fixture expectations are cleaner after centralization into PROGRAM_FIXTURES. I don’t see a blocking invariant or testing failure here, but I would clean up the reflected expectation duplication and the inaccurate new comment.

[briansrls]

Review metadata

• Provider / model: claude / claude-opus-4-7
• Commit: 328920b0 · Trigger: schedule
• Thinking: 23s wall

---

Verdict: APPROVE — Test-only diff. Adds expected_stdout as a single source of truth on PROGRAM_FIXTURES/REFLECTED_FIXTURES, refactors the existing per-fixture tests to read from it, and adds an #[ignore]d batched gate (emit_rust_fixtures_rustc_green) that sweeps the full matrix. The ReflectedExpected::{Exact, PositiveInt} coproduct cleanly replaces the prior ad-hoc assertion branching, and the per-fixture tests + gate coexistence is explicitly justified in the doc comment (per-fixture names for triage, gate for go/no-go). No substrate changes, no invariant implications, expected values match the pre-diff inline assertions.

Minor exploratory observations (not asks):

• The let ReflectedExpected::Exact(expected) = ... else { panic!(...) } pattern at lines 887, 898, 909, 920 encodes a static expectation dynamically — if a future edit flips one fixture to PositiveInt, the panic fires at runtime rather than at compile time. Could be tightened by having the per-fixture tests dispatch through a small helper like the gate does, or by just asserting on stdout directly with the expected literal. Not worth changing for this PR.
• The doc comment fix on line 943 (--test m1_3_emit_rust_test → --test integration) is a nice drive-by; worth confirming there aren't other stale --test m1_3_emit_rust_test invocations elsewhere, but that's outside this diff.

[briansrls]

Review metadata

• Provider / model: claude / claude-opus-4-7
• Commit: 320b0954 · Trigger: schedule
• Thinking: 25s wall

---

APPROVE — Test-only change that consolidates per-fixture expected stdout onto the ProgramFixture/ReflectedFixture descriptors and adds a single #[ignore]d gate test that sweeps the full matrix. This actually improves single-authority discipline: before, expected values were duplicated between the fixture list and each hard-coded assert_eq!; now they live once on the fixture. Doc-comment cargo test invocations updated to the --test integration harness path consistently. No invariant/modeling/CODING/TESTING violations observed in the diff.

Exploratory observations (optional):

• The per-fixture rustc_roundtrip_* tests and the new emit_rust_fixtures_rustc_green gate now assert the same thing twice. The PR body/comment explicitly justifies this ("per-fixture names are useful for triage"), so it's tracked, but once the harness reports per-fixture failures cleanly inside the gate, the individual tests become a candidate for dissolution.
• The let ReflectedExpected::Exact(expected) = reflected_expected(name) else { panic!(...) }; pattern in the four exact-match reflected tests is mildly awkward — the variant is statically known per test. A small helper like reflected_expected_exact(name) would dissolve the runtime panic branch, but it's a test-local nit, not a finding.

[briansrls]

Review metadata

• Provider / model: codex / gpt-5.4
• Commit: 320b0954 · Trigger: schedule
• Thinking: 292s wall

---

APPROVE

No concerns. The diff is narrowly scoped to test infrastructure, moves program expected stdout to a single authoritative fixture table, and keeps the new full-matrix gate aligned with the existing per-fixture tests rather than introducing a parallel behavior path. I also ran cargo test -p v3-compiler --test integration emit_rust_fixtures_rustc_green -- --ignored --nocapture, and the new gate passed.