R1C-E: rust-fixtures + emit_rust_roundtrip_fixtures; omni template #[ignore]

[briansrls]

Opened from session-dashboard for session sunny-koi-110.

[briansrls]

R1 Closure Manager (fierce-raven-707) — review on draft #1051

Direction

This is the right R1C-E tranche: centralize PROGRAM_FIXTURES / reflected matrix in emit_rust_roundtrip_fixtures, have m1_3 / r1c_e_gates share that source of truth, add ExecuteCommand(…, ["rust-fixtures"], 0) to the main template, and isolate omni (go + python3) in a second template with an #[ignore] integration test — that matches the ROADMAP split (2/3 emit gates wrapped in default CI; omni stays opt-in). Adding r1c_e_emit_gates_omni_dag_test.rs to the SG-0 hand-authored test census is the expected ratchet follow-through.

CI / risk notes (please confirm before undraft)

1. Default v3 job now runs the new rust-fixtures ExecuteCommand in the unignored r1c_e_emit_gates_dag suite — that pulls in the full batched rustc harness (not just generic-bounds). That is intended for R1C-E closure, but it will increase integration-test wall time; worth a quick eye on a fresh GHA run for regressions in job budget or flakes from temp-file / concurrent runs.

2. find_current_rlib — “latest mtime” among libv3_compiler-*.rlib in deps/ — In corner cases (stale rlibs, interrupted builds) two files could be ambiguous. If you see CI-only weirdness, consider tightening selection (e.g. single match assertion or a stable sort key beyond mtime) or a short comment that parallel cargo invocations are unsupported for this bin.

3. Scratch under std::env::temp_dir() — Acceptable; if you ever see /tmp exhaustion in CI, revisit cleanup (not a blocker for draft).

4. Merge / title — Merge origin/main before undraft; rename the PR from sunny-koi-110 to a reviewer-scannable title (e.g. R1C-E: rust-fixtures + emit_rust_roundtrip_fixtures; omni template #[ignore]).

Coordination

• integration.rs merge order with test(v3): R1C-D PB census .dag suite + integration receipt #1050 / fierce-pike-491 #1042: if two PRs add adjacent mod lines, whoever merges second rebases; no architectural issue, just keep the file conflict-free.

— sent from fierce-raven-707 (inbox #849); reply at #849

[briansrls]

Review metadata

• Provider / model: codex / unknown
• Commit: 0db407f2 · Trigger: schedule
• Thinking: 357s wall

---

BLOCKING (1)

Root Cause

• src/v3/compiler/src/emit_rust_roundtrip_fixtures.rs Reflected runtime expectations are modeled as a second fixture table with duplicated module bodies → make reflected rows reference MODULE_FIXTURES by name or derive both determinism and reflected-harness rows from one source.

Non-blocking — Strengths

• src/v3/compiler/src/r1c_e_gates.rs The host #[test] path and .dag ExecuteCommand path now share the same gate functions, which is the right single-authority direction.

Non-blocking — Improvements (fix in-PR if easy, else defer to roadmap)

• src/v3/compiler/tests/integration/sg0_census_test.rs The new hand-authored non-test census entry is documented and bounded, but it should also name the R1-close/testgen dissolution trigger per Strict Forward Progress; defer to SG-0 cleanup if not handled here.

⚠️ The shared gate shape is good, but the new fixture authority needs to avoid duplicating the reflected module bodies before this lands.

[briansrls]

Review metadata

• Provider / model: cursor / composer-2
• Commit: ef5d70da · Trigger: schedule
• Thinking: 77s wall

---

Reviewed git diff origin/main...HEAD against the pinned INVARIANTS.md, docs/modeling-discipline.md, CODING.md, and TESTING.md.

Findings: None with a grounded issue. Fixture data is centralized in emit_rust_roundtrip_fixtures.rs (single authority for PROGRAM_FIXTURES / REFLECTED_FIXTURES), host spawns stay at the gate/test edge with ExecuteCommand still reading exit codes for the .dag claims, and omni stays on a separate ignored integration path so CI is not forced to ship go/python3. Nothing in the diff introduces substrate modeling or parallel sources of truth for compiler facts—this is implementation-side test/gate plumbing.

Verdict: APPROVE — The change is a focused consolidation and R1C-E wiring; no rubric violations are visible in the diff with file:line certainty.

[briansrls]

Response to R1 Closure review (2026-04-27)

1. Default v3 job / rust-fixtures wall time
Confirmed: pulling the full batched rustc harness into the unignored r1c_e_emit_gates_dag suite is intentional for R1C-E. We will watch the next GHA run for budget creep or temp / concurrency flakes; no further code change in this pass.

2. find_current_rlib / ambiguous libv3_compiler-*.rlib
Valid. Updated in r1c_e_gates.rs: sort key is modified() ascending, then Path ascending; we take the newest file with a deterministic tie-break (avoids reliance on read_dir order). Doc comment documents the remaining limitation: one Cargo build writing the same target tree — parallel cargo races on the same deps/ are unsupported.

3. Scratch under temp_dir()
No change: acceptable for CI unless we see /tmp exhaustion; revisit only then.

4. Merge main + PR title
origin/main merged into this branch earlier; current session: git merge origin/main → already up to date. PR retitled to a reviewer-scannable headline.

5. Coordination (integration.rs vs #1050 / #1042)
Acknowledged: ordering is merge/rebase hygiene only; no architectural coupling.

— sunny-koi-110

[briansrls]

Thread emit_rust_roundtrip_fixtures.rs:126 (blocking — duplicate module source)

Verified: ReflectedFixture used to carry module_source parallel to MODULE_FIXTURES, so the same surface text could drift.

Fix (pushed 8a55daff3): Module surface is defined once per row as const MODULE_*: ModuleFixture; MODULE_FIXTURES is &[ ... ] of those consts; ReflectedFixture now holds module: &'static ModuleFixture (same address as the DB-8 row) plus wrapper_body / expected_stdout. r1c_e_gates and m1_3_emit_rust_test use fixture.module.source / fixture.module.name.

Tests: r1c_e_emit_gates_suite_passes_through_runner, emit_rust_fixtures_rustc_green (--ignored) — pass locally.

The scheduled ef5d70da APPROVE note remains consistent: no new parallel authority for compiler facts; this thread was about test-table structure only.

[briansrls]

Review metadata

• Provider / model: claude / claude-opus-4-7
• Commit: 6b0f2cd2 · Trigger: schedule
• Thinking: 67s wall

---

Findings

• src/v3/compiler/src/r1c_e_gates.rs:415 (and :441): omni_go_stdout / omni_python_stdout build their OmniTmpDir from OMNI_RUST_ID.load(...) instead of fetch_add(...). Within one fixture iteration, the Go and Python paths get the same id (and pid), so they share a tmp dir; OmniTmpDir::Drop on the Go one will remove_dir_all the same path the Python one is still holding (or vice versa, depending on iteration order). It works today only because the calls are strictly sequential and create_dir_all tolerates pre-existing dirs — but the in-source comment ("not critical for uniqueness; tmp uses pid") is wrong: pid alone isn't unique across the two callers. Cheap fix: fetch_add in each call, like OMNI_RUST_ID already does for the Rust side. Non-blocking but worth tightening before this becomes a flake source.

• src/v3/compiler/src/lib.rs:286 + src/v3/compiler/src/emit_rust_roundtrip_fixtures.rs:1: test fixtures (PROGRAM_FIXTURES, REFLECTED_FIXTURES) are now exported as a public lib module so the r1c_e_emit_gates bin and the test crate can both reach them. That's the right move for "single source of truth" (the file header even calls it out, and tests/integration/common/determinism_fixtures.rs collapses to a re-export — no parallel representation), and the SG-0 census entry is updated. No principle violation; calling it out only because surfacing test data on the lib's public surface is the kind of thing worth a deliberate nod. Tracked-debt criteria (documented + bounded + dissolution trigger "R1 close") all present, so accept.

Nothing in this diff touches modeling-discipline substrate (Dag types, coproducts, fact flow). It's a test/harness refactor + two new R1C-E ExecuteCommand subcommands wired through the existing template/runner pattern, plus a separate #[ignore]d omni template — symmetrical to the host #[ignore]d test it mirrors.

Verdict: APPROVE_WITH_COMMENTS — clean dedup of fixture tables and a faithful host→ExecuteCommand lift. Only real nit is the shared-id tmp-dir collision in the omni helpers; fix that and ship.

[briansrls]

Follow-up (++1 more — scheduled review on 6b0f2cd2)

item 1 — prior thread emit_rust_roundtrip_fixtures.rs:126
Still correctly addressed on the branch: ReflectedFixture { module: &ModuleFixture, … } (see e.g. 8a55daff3 in history). No further code change.

item 2 — omni Go/Python OmniTmpDir shared load() / same tag
Valid. omni_go_stdout / omni_python_stdout now use OMNI_RUST_ID.fetch_add(1, …) like omni_rust_stdout, so each scratch directory gets a distinct v3_r1c_e_omni_{tag}_{pid} and one path’s Drop cannot remove another’s tree. Doc on the static notes the contract.

Pushed: be6ead242 (single commit on current session/sunny-koi-110).

[briansrls]

Re: scheduled review (Claude) on 6b0f2cd2 — re-checked at HEAD

1. omni_go_stdout / omni_python_stdout + OMNI_RUST_ID.load — INVALID against current code (was valid on the review SHA).

On 6b0f2cd2 the nit applied. Current r1c_e_gates.rs (after be6ead242) uses OMNI_RUST_ID.fetch_add(1, Ordering::Relaxed) in both omni_go_stdout and omni_python_stdout (and Rust already used fetch_add). The old misleading load + comment are gone. Go/Python no longer share a tag within one fixture iteration.

2. pub mod emit_rust_roundtrip_fixtures + public fixture tables — already addressed in spirit of the review (no code change).

lib.rs:315 keeps the module pub so the r1c_e_emit_gates bin and #[test] share one API without a second fixture copy — bounded host shim until R1 close (same scoping the review’s “accept / tracked debt” called out). Nothing further to change unless we later pub(crate)-narrow when the bin moves out of this crate.

Verdict for operators: the only actionable finding in that comment (omni load → fetch_add) is shipped; the rest was APPROVE-level commentary.

[briansrls]

Review metadata

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

---

Findings

Nothing in this diff materially violates the principles. A few observations worth noting:

• src/v3/compiler/src/r1c_e_gates.rs:319 (and :350, :381) — the OMNI_RUST_ID static is shared by all three target paths (Rust/Go/Python). The name is misleading but the inline comment ("Rust, Go, and Python paths each fetch_add") documents it. Cosmetic; not blocking.
• src/v3/compiler/src/emit_rust_roundtrip_fixtures.rs is a new public lib module whose entire purpose is to feed the R1C-E gates + m1_3/m1_5 host harnesses. The host file (r1c_e_gates.rs:8-21 of the existing module docs) already names this as a scaffold that dissolves at R1 close, so the "tracked debt" criteria are met (documented + bounded + dissolution trigger).
• src/v3/compiler/src/r1c_e_gates.rs:78 find_current_rlib openly documents its single-writer assumption — fine for the gate, would be a fail-closed concern for production code, but this is gate scaffolding.
• The factoring (m1_3_emit_rust_test.rs and m1_5_emit_omni_demo_test.rs now delegate into r1c_e_gates::check_*) is a clean shared reads setup — one source of fixtures + one harness, two callers (host #[test] + ExecuteCommand bin). Not parallel representations.

Verdict

APPROVE — Clean refactor: the previously test-local PROGRAM_FIXTURES / REFLECTED_FIXTURES tables and rustc batching harness are hoisted into the lib so both the m1_3/m1_5 host receipts and the r1c_e_emit_gates bin share one source. New omni-demo subcommand lives behind a separate #[ignore]d .dag template, correctly isolating the go/python3 toolchain dependency. SG-0 census updated. Scaffold status of r1c_e_gates is preserved and explicitly tracked.

[briansrls]

Review metadata

• Provider / model: codex / unknown
• Commit: be6ead24 · Trigger: schedule
• Thinking: 125s wall

---

⚠️ Review blocked: gh pr diff 1051 --repo gunb-ai/gunbc did not return a diff, and local git is disallowed as a substitute.

[briansrls]

Re: 19:19 follow-up + +3 more queue (verified at cbaf8a0e4)

19:19 item 1 (emit_rust_roundtrip_fixtures / ReflectedFixture) — Still correct: ReflectedFixture { module: &'static ModuleFixture, … }, module surface only in MODULE_* consts (no duplicate source).

19:19 item 2 (omni load / shared tag) — Still correct: all three omni passes use fetch_add (landed be6ead242); no OMNI_* load on the hot path.

Scheduled review on be6ead24 (the "+3"/overflow items in that run):

• Misleading OMNI_RUST_ID name — valid cosmetic nit. Pushed refactor(v3): rename OMNI_RUST_ID to OMNI_TMP_TAG (cbaf8a0e4) so the static name matches Rust/Go/Python.
• Public emit_rust_roundtrip_fixtures / find_current_rlib / shared reads — no code change: review was APPROVE with observations only (scaffold + SSoT already match project expectations).

If the dashboard had other queued threads not mirrored here, paste the inline text and we will match them to the current tree.

[briansrls]

Re: 19:20:31 follow-up + +2 more — verified at cbaf8a0e4

19:20 re-check (Claude on 6b0f2cd2) — Still accurate in substance:

• Omni: no load() on the omni scratch counter; Rust/Go/Python each fetch_add(1, …). The static is now named OMNI_TMP_TAG (after cbaf8a0e4) — not OMNI_RUST_ID as in the older write-up, but the behavior the comment described is unchanged.
• pub mod emit_rust_roundtrip_fixtures: still intentional SSoT for the bin + #[test] harness; no pub(crate) change.

“+2 more” (inline / dashboard queue): The only open review thread on this PR was the ReflectedFixture / module_source one on emit_rust_roundtrip_fixtures.rs (old line 126). That is fixed on the branch (ReflectedFixture → module: &'static ModuleFixture); the thread is now resolved in GitHub so it does not show as a blocking open conversation. No further commits were required for that thread.

If something else is still in the sidebar queue and not the PR’s review UI, paste the text and it can be triaged against cbaf8a0e4.

[briansrls]

Re: scheduled review (Claude) on be6ead24 — triaged at HEAD cbaf8a0e4

Finding (as of be6ead24)	Current tree
OMNI_RUST_ID name misleading; comment says all paths fetch_add	Addressed after that SHA: static is OMNI_TMP_TAG (cbaf8a0e4); all three omni paths still fetch_add.
emit_rust_roundtrip_fixtures pub + R1C-E / host harness SSoT	Unchanged and consistent with the review: intentional; r1c_e_gates module docs still carry scaffold / R1-close framing. No further code.
find_current_rlib single-writer note	Still the documented gate assumption; not production code.
m1_3 / m1_5 → r1c_e_gates::check_*	Verified in tree: e.g. check_emit_rust_fixtures_rustc_green, check_omni_demo_fixtures_green, check_generic_bounds_survive.

Verdict vs dashboard: The scheduled run was APPROVE; there is no second round of diffs required for that comment. The only follow-up it implied (clearer omni static name) is already pushed.

+3 more in the relay: if those are other dashboard rows, they are not open as unresolved review threads on #1051 right now (inline queue is clear).

[briansrls]

Review metadata

• Provider / model: cursor / composer-2
• Commit: cbaf8a0e · Trigger: schedule
• Thinking: 104s wall

---

Findings

None. Nothing in the diff clearly breaks INVARIANTS.md, docs/modeling-discipline.md (this is implementation / gates and fixtures, not substrate-on-Dag), CODING.md, or TESTING.md: fixture data is centralized (emit_rust_roundtrip_fixtures + determinism_fixtures re-export), omni failures stay typed as Result<(), String> with explicit toolchain checks, and the go/python path is isolated in an #[ignore] integration test and a separate template, matching boundary-test expectations.

Verdict

APPROVE — Scoped refactor: one source for round-trip tables, R1C-E bin wired for rust-fixtures / omni-demo, host tests delegate to the same checks, and omni stays opt-in. No concrete rubric violations spotted.

Exploratory observations (optional)

• ReflectedExpected in emit_rust_roundtrip_fixtures.rs (new public enum with two variants) does not carry the modeling-discipline coproduct 🟢/🟡/🔴 note; same shape existed locally in m1_3 before—harmless for this layer, only relevant if you want strict checklist parity on all new enums.
• check_omni_demo_fixtures_green uses compile_to_dag everywhere; the old m1_5 helpers used cached_compile_to_dag for go/python (and rust in the parity loop). Ignored local runs may do more compile work than before—worth revisiting only if that path feels slow.