feat(v3): parser staging — runtime_mirrors carriers, parse_generated, regen_parse (not SG-2b)

[briansrls]

What this is (convergence framing)

This PR converges as parser staging, not as full SG-2b closure. The merged claim matches the diff: Surface carrier shapes are substrate-authoritative in src/v3/compiler/runtime_mirrors.dag; src/v3/compiler/src/parse_generated.rs is producer-owned generated output from regen_parse; parse semantics still live in a checked-in Rust fragment until a later lane makes parse logic structurally .dag-owned.

Temporary semantic authority

src/v3/compiler/parse_parser_body.txt is the temporary semantic authority for the recursive-descent parse algorithm. regen_parse splices that fragment into parse_generated.rs after emitted Surface* definitions. It is explicit scaffolding with a named paydown (see the file header), not final parser .dag authority.

Named dissolution trigger

Remove parse_parser_body.txt when parse logic is structurally .dag-owned (src/v3/SELF_HOSTING.md §6 — Phase 4a direct port or Phase 4b grammar-as-data) and the regen path emits the full module without any external Rust body fragment, leaving parse_generated.rs as projection only (not a spliced Rust-body host). Same trigger is stated in the header of parse_parser_body.txt.

Ratchets (unchanged intent)

SG-0 / generated-file census and freshness ratchets stay as implemented so this staged boundary stays mechanically honest. Roadmap deferrals for the string-mirror paydown in regen_parse remain documented in docs/history/roadmap-active-deferrals.md.

---

Opened from session-dashboard for session quiet-owl-783; body updated for merge convergence.

[briansrls]

This is useful SG-2b groundwork, but it is not SG-2b hard cutover yet.

The SG-2b brief was: retire src/v3/compiler/src/parse.rs as handwritten authority. This diff only adds src/v3/compiler/parse.dag with the surface carriers. It does not yet:

• generate a parser module
• switch callers to generated/parser-owned authority
• delete or reduce src/v3/compiler/src/parse.rs
• update hand_maintained_src / the SG-0 census

So from my side this should either be re-scoped as SG-2b prep or completed through the actual parser cutover + file retirement in the same PR.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: edb6413bf9

ℹ️ 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]

Align SurfaceLiteral variants with existing AST names

parse.dag is introduced as the authority for the parse-layer carriers, but this enum renames literals to IntLit/BoolLit/StringLit instead of the existing Int/Bool/String shape used by the current parser and lowerer. When SG-2b starts generating parse_generated.rs from this file, the generated SurfaceLiteral API will diverge from downstream matches and constructor sites, causing compile-time breakage during the cutover rather than a mechanical replacement.

Useful? React with 👍 / 👎.

[briansrls]

ChatGPT review in progress... (view conversation)

Check back in ~30 minutes for the full review.

[briansrls]

ChatGPT review in progress... (view conversation)

Check back in ~30 minutes for the full review.

[briansrls]

Latest diff read: this is no longer prep-only, but it still does not converge to SG-2b parse hard cutover as originally briefed.

What landed is parser staging:

• parse.rs drops from hand_maintained_src
• parse_generated.rs becomes producer-owned output
• parse.dag now owns the surface carrier schema
• but the parser algorithm still lives in parse_parser_body.txt, and parse.dag explicitly says regen_parse splices that checked-in recursive-descent Rust body into the generated module

That means the merged-state authority is no longer handwritten .rs under src/v3, but it is still not .dag parser authority. For this repo's SG framing, that matters.

Direction:

1. If you want to merge this as-is, rename/reframe it as parser staging rather than SG-2b hard cutover.
2. If you want to keep the SG-2b label, the remaining work is to eliminate parse_parser_body.txt as semantic authority and make the parser logic itself structurally owned by .dag.
3. Either way, add one explicit note naming parse_parser_body.txt as temporary staging with a dissolution trigger, so this does not read like final parser authority by omission.

So my convergence read is: useful and honest as staging; not converged as SG-2b hard cutover.

[briansrls]

codex · gpt-5.4 · 4914b7cc

✅ Review (blocking: 0, non-blocking: 1+/1-)

Non-blocking — Strengths

• src/v3/compiler/parse.dag This keeps SG-2b at the right layer: parse.dag owns only the surface carriers, while the recursive-descent body stays implementation detail and the existing parse→lower fact set survives intact.

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

• src/v3/compiler/src/bin/regen_parse.rs emit_surface_types still mirrors the authority through a hardcoded root-type roster and std-name matches (SurfaceModule, Int, List, etc.), so an SG-2 follow-up should derive the reachable declarations and primitive mapping from the compiled Dag instead of parallel Rust strings.

✅ I did not find a blocking thesis/invariant violation in this SG-2b cutover.

[briansrls]

Wrap-up direction from re-review:

I do not think this is converged as SG-2b yet.

What is good in the current diff:

• parse.rs leaves hand_maintained_src
• parse_generated.rs becomes producer-owned output
• parse.dag now owns the parser surface schema
• SG-0 / freshness ratchets move with that

What still blocks convergence:

• the parser algorithm still lives in src/v3/compiler/parse_parser_body.txt
• parse.dag explicitly says regen_parse splices that checked-in recursive-descent Rust body into the generated module
• so this is parser staging, not .dag parser authority

To get this PR to convergence, pick one of these two endings and make it explicit:

1. Re-scope to staging

• rename/rebody the PR as parser staging
• add one explicit note that parse_parser_body.txt is temporary semantic authority
• name the dissolution trigger that removes it
• do not count this as SG-2b hard cutover

2. Finish SG-2b for real

• eliminate parse_parser_body.txt as semantic authority
• make the parser logic itself structurally owned by .dag
• keep parse_generated.rs as projection only, not a spliced Rust-body host

Given the size of the current branch, I recommend option 1 for this PR: land it honestly as staging, then dispatch the remaining parser-authority move as the next lane.

So the shortest path to convergence here is: rename/reframe as parser staging, name the scaffold and dissolution trigger, and stop claiming full SG-2b closure.

[briansrls]

Meta-review in progress... (view conversation)

Loop-health check: is this review cycle making forward progress, or shifting debt? Posts in ~5-15 minutes.

[briansrls]

⚠️ Reviewer account gunbc-secondary appears rate-limited / downgraded to Instant. The harvested response for sha 18dfc88 was discarded because no Pro thinking marker was visible (response complete in <56min — Pro mode would have shown a "Thought for …" marker). Account is in cooldown; next attempt will retry after the cooldown window.

Conversation

[briansrls]

⚠️ Reviewer account gunbc-secondary appears rate-limited / downgraded to Instant. The harvested response for sha c0161eb was discarded because no Pro thinking marker was visible (response complete in <8min — Pro mode would have shown a "Thought for …" marker). Account is in cooldown; next attempt will retry after the cooldown window.

Conversation

[briansrls]

codex · gpt-5.4 · c0161ebd

✅ Review (blocking: 0, non-blocking: 1+/1-)

Non-blocking — Strengths

• src/v3/compiler/parse_parser_body.txt The staged parser body is documented as temporary semantic authority with a concrete dissolution trigger, so this lands as tracked scaffold rather than hidden dual authority.

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

• docs/history/roadmap-active-deferrals.md The new SG-2 staging summary says runtime_mirrors.dag is omitted from the bootstrap COMPILER_FILES bundle, but build.rs/bootstrap.rs now keep it in the normal bootstrap and omit it only in the dedicated compile_runtime_mirrors_authority_dag path; fix the prose so the authority story stays factual.

ROADMAP — Verified

• regen_parse string-mirror paydown: The follow-up now explicitly names the hardcoded root roster/std-name mapping, bounds the debt, and gives a named dissolution trigger.

ROADMAP — Incomplete

• SG-2b hard cutover: The PR correctly leaves SG-2b open because parse_parser_body.txt still holds semantic parse logic.

✅ The parser-staging cutover is honestly scoped, the handwritten parser authority is retired cleanly, and I did not find a blocking thesis/invariant violation in the diff target.

[briansrls]

codex · gpt-5.4 · 0ed8784f

✅ Review (blocking: 0, non-blocking: 3+/0-)

Non-blocking — Strengths

• src/v3/compiler/src/lib.rs compile_runtime_mirrors_authority_dag stays fail-closed by routing the special bootstrap path through tokenize, parse, strict lower finalization, and infer instead of introducing a second semantic shortcut.
• src/v3/compiler/parse_parser_body.txt This is tracked scaffold rather than hidden debt: the file names its temporary authority role, bounds it to parser logic, and gives a concrete SG-2b dissolution trigger.
• src/v3/compiler/tests/integration/sg2_parse_authority_test.rs The new SG-2 ratchet covers both halves of the staging contract by checking that runtime_mirrors.dag compiles on the fixture-skipping path and that regen_parse reproduces the checked-in generated module.

ROADMAP — Verified

• regen_parse authority debt tracking: The remaining regen_parse Rust-string mirroring from prior review is now explicitly documented, bounded, and tied to the next runtime_mirrors.dag change or a dedicated SG-2 follow-up.

✅ I did not find a new blocking thesis or invariant violation in this parser-staging cutover.

[briansrls]

⚠️ ChatGPT review abandoned — conversation unavailable after 336 minutes

The conversation page did not render any content. The bot will start a fresh conversation on the next push.

View conversation

[briansrls]

⚠️ ChatGPT review abandoned — conversation unavailable after 287 minutes

The conversation page did not render any content. The bot will start a fresh conversation on the next push.

View conversation

[briansrls]

⚠️ ChatGPT review abandoned — conversation unavailable after 57 minutes

The conversation page did not render any content. The bot will start a fresh conversation on the next push.

View conversation

[briansrls]

codex · gpt-5.4 · 8100b9a8

✅ Review (blocking: 0, non-blocking: 2+/0-)

Non-blocking — Strengths

• src/v3/compiler/src/lib.rs compile_runtime_mirrors_authority_dag keeps parser staging on the normal fail-closed tokenize to parse to lower to infer path instead of introducing a parallel unchecked compile boundary.
• src/v3/compiler/tests/integration/sg2_parse_authority_test.rs The SG-2 ratchet now checks both the filtered authority compile path and the checked-in parse_generated.rs snapshot using an in-memory render.

ROADMAP — Verified

• parser staging framing: docs/design-pure-bootstrap.md, docs/history/roadmap-active-deferrals.md, and src/v3/SELF_HOSTING.md now consistently describe this lane as parser staging rather than SG-2b closure.
• regen_parse mirror paydown: docs/history/roadmap-active-deferrals.md documents the hardcoded root and type-mapping mirror in regen_parse_emit.rs, bounds it to staging, and gives a concrete dissolution trigger.

ROADMAP — Incomplete

• SG-2b hard cutover: src/v3/compiler/parse_parser_body.txt still owns parser semantics, so the PR correctly lands as parser staging rather than .dag-owned parse logic.

✅ The current +lines read as an honestly scoped parser-staging cutover, and I do not see a new blocking thesis, invariant, or modeling-discipline violation.

[briansrls]

codex · gpt-5.4 · 8100b9a8

✅ Review (blocking: 0, non-blocking: 2+/0-)

Non-blocking — Strengths

• src/v3/compiler/src/lib.rs compile_runtime_mirrors_authority_dag keeps parser staging on the normal fail-closed tokenize to parse to lower to infer path instead of introducing a parallel unchecked compile boundary.
• src/v3/compiler/tests/integration/sg2_parse_authority_test.rs The SG-2 ratchet now checks both the filtered authority compile path and the checked-in parse_generated.rs snapshot using an in-memory render.

ROADMAP — Verified

• parser staging framing: docs/design-pure-bootstrap.md, docs/history/roadmap-active-deferrals.md, and src/v3/SELF_HOSTING.md now consistently describe this lane as parser staging rather than SG-2b closure.
• regen_parse mirror paydown: docs/history/roadmap-active-deferrals.md documents the hardcoded root and type-mapping mirror in regen_parse_emit.rs, bounds it to staging, and gives a concrete dissolution trigger.

ROADMAP — Incomplete

• SG-2b hard cutover: src/v3/compiler/parse_parser_body.txt still owns parser semantics, so the PR correctly lands as parser staging rather than .dag-owned parse logic.

✅ The current +lines read as an honestly scoped parser-staging cutover, and I do not see a new blocking thesis, invariant, or modeling-discipline violation.

[briansrls]

Findings

• .claude/scheduled_tasks.lock committed. File is session-local lock state (sessionId/pid/acquiredAt) and clearly not PR content. Drop it from the commit and add to .gitignore. (NON-BLOCKING — doesn't affect substrate, but it's build/workflow noise that will churn on every local run.)

• Dissolution ledger thinned on SurfaceItem move to runtime_mirrors.dag (src/v3/compiler/runtime_mirrors.dag:158). The retired parse.rs carried rich SurfaceItem receipts distinguishing FnExternalBody case 1 (parse lag), case 2a (pipeline host stages → ExternalRealization), and case 2c (compile orchestrator — body-span-as-ordering-authority, DB-16). The new .dag receipt collapses to "🟡 SCAFFOLD — top-level item carrier mirror; same dissolution trigger as SurfaceType." Per modeling-discipline.md §4, yellow classification needs a specific named trigger; "same as SurfaceType" works for carrier-shape dissolution (parse-rule authority), but the case 1/2a/2c receipts on FnExternalBody had different dissolution paths (DB-16, not SG-2b). Either (a) extend the SurfaceItem / FnExternalBody comment in the .dag to preserve the case distinction, or (b) make a follow-up doc the single authority for the DB-16 trajectory and cite it from the .dag. NON-BLOCKING (substrate dissolution still has a trigger), but modeling-context loss is worth naming.

• compile_runtime_mirrors_authority_dag duplicates the pipeline stitch of compile_to_dag (src/v3/compiler/src/lib.rs:857). It's a near-copy differing only in the bootstrap choice (Dag::new_without_runtime_mirrors_compiler_fixture_bootstrap()). Implementation-layer, not substrate, so non-blocking per the calibration — but a parameterized entry point (compile_to_dag(source, file, BootstrapChoice::{Full,ExcludeRuntimeMirrors})) would retire both the forked function and the forked LazyLock without adding authority. (NON-BLOCKING; worth addressing when the next similar carve-out shows up, to avoid bootstrap_without_<X> / BOOTSTRAPPED_DAG_WITHOUT_<X> proliferation.)

Good things worth calling out:

• The prior blocking finding about runtime_mirrors.dag coproducts lacking 🟢/🟡/🔴 receipts is addressed — VariantPayload, SurfaceType, SurfaceLiteral, SurfacePattern, SurfaceExpr, SurfaceItem all now have inline classification + trigger. The SurfaceLiteral naming contract is a nice paranoid receipt.
• parse_parser_body.txt header has an explicit dissolution trigger tied to SG-2b.
• The snapshot test parse_generated_module_matches_checked_in_snapshot does an in-process render compare via include_str! + render_parse_generated_rs, not a write-to-tracked-file path — earlier relay hermeticity concern is resolved.
• Framing is honestly scoped as "parser staging, not SG-2b" across docs (SELF_HOSTING.md, design-pure-bootstrap.md, roadmap-active-deferrals.md) and the PR preserves SG-2b as a named follow-on. regen_parse emitter's hardcoded root-type roster / std-name → Rust string matches are called out with bounded dissolution trigger.

Verdict

APPROVE_WITH_COMMENTS. Scoped, honest parser-staging cutover with dissolution receipts addressed. Two clean-ups before merge (drop the .claude/scheduled_tasks.lock commit; optionally extend the SurfaceItem .dag comment so the DB-16 case 1/2a/2c receipts don't get lost when parse.rs retires), plus one forward note about the compile_* / bootstrap_* pair as future consolidation.

[briansrls]

Findings

Nothing blocking in this diff.

On the prior blocking concern (Codex 039db5a5… / human rollup @ 11:27:44Z — missing dissolution receipts on new .dag coproducts): addressed. Every new coproduct in src/v3/compiler/runtime_mirrors.dag now carries an inline 🟢/🟡 annotation per docs/modeling-discipline.md §4:

• VariantPayload (lines 39–41) — 🟢 TERMINAL + ledger
• SurfaceType (lines 45–47) — 🟡 SCAFFOLD + named trigger (parse_parser_body.txt header)
• SurfacePattern (line 84) — 🟡 SCAFFOLD (same trigger)
• SurfaceLiteral (lines 98–103) — 🟢 TERMINAL + ledger
• SurfaceExpr (line 108) — 🟡 SCAFFOLD (same trigger)
• SurfaceItem (lines 158–167) — 🟡 SCAFFOLD + FnExternalBody/DB-16 ledger

On the prior non-hermetic snapshot concern: tests/integration/sg2_parse_authority_test.rs:26–38 calls render_parse_generated_rs(...) with include_str!-ed inputs and compares to include_str!(CHECKED_IN_GENERATED) — purely in-process, no tracked-file rewrite. The original concern described regen_parse being shelled out during the test; the landed test does not do that.

On scaffold boundaries (INVARIANTS.md / modeling-discipline): src/v3/compiler/parse_parser_body.txt:1–9 names the scaffold kind, declares the dissolution trigger (delete when parse logic is .dag-owned via SG-2b), and the framing is consistently reinforced in docs/design-pure-bootstrap.md, docs/history/roadmap-active-deferrals.md, and src/v3/SELF_HOSTING.md. This satisfies the three-part bridge test (documented, bounded, named dissolution trigger).

On fail-closed / facts flow forward: src/v3/compiler/src/lib.rs:857–873 — compile_runtime_mirrors_authority_dag reuses the normal tokenize → parse → lower → finalize_strict_user_lower_range → infer pipeline and returns CompileError::Semantic(dag) when diagnostics are non-empty. No parallel unchecked boundary. The refactor in lower.rs:145–173 that factored out finalize_strict_user_lower_range(...) is a straight extraction — semantics preserved.

Exploratory observations

• Parser body as .txt + census movement (noted, not blocking). dsl/gunbc/compiler.dag:96 drops parse.rs and adds regen_parse_emit.rs; tests/integration/sg0_census_test.rs:105,130 mirrors that swap. The parse algorithm (~1350 lines) moves from .rs census to .txt staging fragment. This is honestly framed as parser staging and has a named dissolution trigger, so it clears the scaffold-boundaries bar. But structurally this is the most load-bearing scaffold in tree right now — if SG-2b slips repeatedly, .txt-as-semantic-authority could normalize. Worth watching that parse_parser_body.txt doesn't accrete ad-hoc edits in follow-on PRs (any real parser change should force the SG-2b lane instead of growing the fragment).

• regen_parse_emit.rs hardcoded root roster + type mapping (src/v3/compiler/src/regen_parse_emit.rs:113–131, rust_type_for_field/map_substrate_scalar_to_rust): already called out in docs/history/roadmap-active-deferrals.md with a concrete dissolution trigger ("next non-trivial runtime_mirrors.dag surface-type change, or dedicated SG-2 follow-up"). That's tracked debt; not worth flagging again.

Verdict

APPROVE_WITH_COMMENTS — the prior blocking coproduct-ledger gap is resolved, the .txt staging fragment is honestly scoped with a dissolution trigger, the authority-compile path is fail-closed, and the snapshot test is in-process/hermetic. The remaining substrate work (SG-2b — actual .dag-owned parse rules) is correctly left as a named follow-on lane.

[briansrls]

Looking at pr-589.diff directly.

What I verified against the reference docs:

• Coproduct dissolution ledgers (modeling-discipline §4). Every new substrate coproduct in runtime_mirrors.dag carries its classification: VariantPayload 🟢 terminal (line 39), SurfaceType 🟡 scaffold with named trigger (line 47), SurfacePattern 🟡 with same trigger (line 85), SurfaceLiteral 🟢 terminal with naming contract (line 98), SurfaceExpr 🟡 (line 109), SurfaceItem 🟡 with DB-16 cross-reference (line 158). This addresses the prior relay blocker.
• Scaffold-boundaries. parse_parser_body.txt header names it temporary, bounds it to parser semantics only, and names the dissolution trigger ("delete this file when parse logic is structurally .dag-owned"). Tracked bridge, not untracked.
• Fail-closed (modeling §1). compile_runtime_mirrors_authority_dag (src/v3/compiler/src/lib.rs:862-875) threads the standard tokenize → parse → lower → finalize_strict → infer path and returns Semantic(dag) on any diagnostic — no parallel unchecked path for the regen binary.
• Facts flow forward. finalize_strict_user_lower_range (src/v3/compiler/src/lower.rs:150-173) is factored out of lower with the same strictness guarantees; identifier resolution + unparsed-scaffold rejection are preserved for the new entry point.
• Single authority. bootstrap_without_runtime_mirrors_fixture (src/v3/compiler/src/bootstrap.rs:92-95) is crate-private and only used by the dedicated regen path; production bootstrap() path unchanged.
• Hermetic test (TESTING §1). sg2_parse_authority_test.rs uses include_str! + in-process render_parse_generated_rs; no in-place rewrite of tracked files during the test run (the prior relay note about shell-out-that-mutates-tree does not match this implementation — render_parse_generated_rs in regen_parse_emit.rs:33-46 only shells to rustfmt via stdin/stdout).

Non-blocking observations:

• regen_parse_emit.rs:109-130 (emit_surface_types) uses a hardcoded root-type roster and rust_type_for_field / map_substrate_scalar_to_rust encode Int/Bool/String/List → Rust as string matches. This is parallel representation of runtime_mirrors.dag facts. Already documented as tracked follow-up in docs/history/roadmap-active-deferrals.md with a concrete dissolution trigger ("next non-trivial runtime_mirrors.dag surface-type change, or a dedicated SG-2 follow-up PR"). Accept as tracked debt.
• The parse.rs → parse_generated.rs rename with the parser algorithm moved into parse_parser_body.txt does drop one entry from the SG-0 census while the parse algorithm remains hand-maintained (renamed scaffold host). The PR frames this honestly as "parser staging, not SG-2b closure" in docs/design-pure-bootstrap.md:140-155 and roadmap-active-deferrals.md:86. The census-shape question (.txt vs .rs for hand-maintained algorithm) is worth watching but the documentation is candid about it.

Verdict: APPROVE_WITH_COMMENTS. The prior blocking concern (missing dissolution ledgers on new runtime_mirrors.dag coproducts) is resolved in this diff. Parser staging is honestly scoped, every scaffold has a named trigger, and the new compile path preserves the existing fail-closed pipeline. The hardcoded-roster emitter debt is tracked, bounded, and has a dissolution trigger — acceptable as a staged bridge.

[briansrls]

Findings

Nothing in this diff that I'd call a concrete violation of the cited documents. The substrate coproducts added to runtime_mirrors.dag (VariantPayload, SurfaceType, SurfacePattern, SurfaceLiteral, SurfaceExpr, SurfaceItem) each carry the modeling-discipline §4 ledger annotations (🟢/🟡 with named dissolution triggers) — e.g. runtime_mirrors.dag:39-41 and 44-46. Earlier relay commentary that flagged missing receipts predates these lines.

The sg2 authority test is hermetic now: sg2_parse_authority_test.rs:26-37 calls render_parse_generated_rs over include_str! inputs and compares in-memory. No tracked-file mutation, contra the earlier relay note about shelling to regen_parse.

Exploratory observations

• parse_parser_body.txt is the whole parser, renamed. src/v3/compiler/parse_parser_body.txt:1-1350 is ~1.3k lines of recursive-descent Rust spliced verbatim into parse_generated.rs. It satisfies the tracked-bridge rubric on paper (documented header, file-scoped, named SG-2b trigger), and sg0_census_test.rs:115 + sg6_hand_authored_census_test.rs:162 are each re-framed honestly. But it's worth saying out loud: this scaffold's weight is the same as the parse.rs it replaced — the win is purely "Surface carriers are now .dag-owned," not "parser algorithm is moving." The SG-2b lane needs to actually land for this to be forward progress rather than relabeled debt.
• regen_parse_emit.rs:138-236 hardcodes the root roster, primitive mapping ("Int" → i64, "List" → Vec, etc.), and Box-edge table. Already tracked in roadmap-active-deferrals.md with a named trigger. Fine as staging; flagging so the follow-up doesn't get lost.
• Test budget 15s → 45s in lane2_stage_2d_symbolic_cost_test.rs:592 attributed to runtime_mirrors.dag now sitting in the production bootstrap bundle. Each future substrate-sized .dag in COMPILER_FILES will compound this; worth keeping an eye on whether bootstrap cost deserves structural work rather than more ratchet bumps.

Verdict

APPROVE_WITH_COMMENTS — parser staging is honestly framed, new .dag coproducts carry the required ledger annotations, the authority test is hermetic, and remaining debts (parse_parser_body.txt, emit_surface_types string-mapping) each have named dissolution triggers. The observations above are for the next lane, not blockers for this one.

[briansrls]

Review metadata

• Provider / model: openai-pro / gpt-5-4-pro
• Commit: 337158c3 · Trigger: manual
• Conversation: View conversation

---

Findings

• src/v3/compiler/tests/integration/sg2_parse_authority_test.rs:25-38 — parse_generated_module_matches_checked_in_snapshot shells out to cargo run ... --bin regen_parse, rewrites src/parse_generated.rs in place, then reads that shared file back. That misses TESTING.md’s hermetic bar for new tests: it requires a writable checkout and introduces shared mutable state during the test run. A temp output path, stdout compare, or library-returned string would keep the same ratchet without mutating tracked source.

Verdict

APPROVE_WITH_COMMENTS — The parser-staging cutover itself reads cleanly: the new compile_runtime_mirrors_authority_dag path stays on the normal fail-closed tokenize → parse → lower → infer flow, and the bootstrap carve-out is tightly scoped. My only concrete concern is the new snapshot test mutating the checked-in tree while it runs.

Exploratory observations

• src/v3/compiler/parse_parser_body.txt:1-8, dsl/gunbc/compiler.dag:96, src/v3/compiler/tests/integration/sg0_census_test.rs:127 — worth double-checking whether the hand-authored-source censuses are intentionally .rs-only. This PR makes parse_parser_body.txt the explicit hand-authored semantic parser authority, but drops the old parse.rs census entries at the same time.

[briansrls]

Review metadata

• Provider / model: openai-pro / gpt-5-4-pro
• Commit: c35dc9fd · Trigger: manual
• Conversation: View conversation

---

Findings

• NON-BLOCKING src/v3/compiler/tests/integration/sg2_parse_authority_test.rs:25-38, src/v3/compiler/src/bin/regen_parse.rs:63-64 — TESTING.md / Hermetic: the new snapshot test shells out to cargo run --bin regen_parse, and that binary rewrites src/parse_generated.rs in place before the test reads it back. That gives the test shared mutable workspace state instead of fully declared inputs, so it can leave the tree dirty and is harder to reason about under concurrent test/process execution than a temp-file or string-return snapshot path.

Verdict: APPROVE_WITH_COMMENTS

The parser-staging cutover otherwise reads clean to me: the dedicated bootstrap carve-out is localized, and compile_runtime_mirrors_authority_dag stays on the normal tokenize → parse → lower → infer fail-closed path. My only concrete concern is the new non-hermetic snapshot test.