fix(r3): add string-family row carrier scaffold

[briansrls]

Add the substrate carrier for string-family diagnostic-ordering rows.

This PR adds one emit-model carrier, regenerates the bootstrap fixtures, and adds a focused structural ratchet. It does not populate rows and does not add a Grounding projection reader.

The carrier stays in src/v3/std/emit_model.dag beside the landed String*Axis authority from #1465 and keeps the row host structurally typed through a language: DeclarationRef edge. The next slice is the row-population consumer, not a shared non-namespaced axis layer or a TypeRealization extension.

Validation performed locally: git diff --check and bootstrap regeneration via cargo run -p v3-compiler --features bootstrap-regen-fresh --bin regen_bootstrap.

[briansrls]

Review metadata

• Provider / model: codex / unknown
• Commit: 92770a86 · Trigger: schedule
• Thinking: 260s wall

---

Non-blocking — Strengths

• src/v3/std/emit_model.dag The new carrier is documented as a bounded scaffold with the row-population and Grounding projection reader trigger named, and it stays in the existing emit-model authority with structural DeclarationRef edges.

✅ No blocking concerns; the PR is a narrow substrate carrier addition with regenerated bootstrap fixtures and a focused regression test.

[briansrls]

Review metadata

• Provider / model: cursor / composer-2
• Commit: 4cb92aa3 · Trigger: schedule
• Comparison: origin/main @ c47fcd32 ... review/pr-1524-4cb92aa3 @ 4cb92aa3
• Thinking: 70s wall

---

Findings: None. The meaningful change is emit_model.dag plus a structural regression in m2_substrate_inhabitance_test.rs; the large bootstrap_generated*.rs churn is the expected declaration-ID ripple from one new substrate declaration.

The new StringFamilyInhabitanceRow block is explicitly 🟡 SCAFFOLD (src/v3/std/emit_model.dag around the new type StringFamilyInhabitanceRow lines), states a bounded role (“carrier only establishes the substrate home”), and names what dissolves it (“Row population and Grounding projection readers arrive later”), which satisfies the scaffold / P5 “named dissolution” lens for this change. Fields are single-authority edges via DeclarationRef to the axis declarations, not a parallel encoding of axis facts. The test pins name, source file, field set, and edge type—appropriate for a bootstrap substrate-shape claim in this file’s style (TESTING.md: integration/bootstrap is fine when the interface is “this declaration exists in full bootstrap”).

Verdict: APPROVE — Narrow, coherent scaffold: substrate home for the string-family row, typed references to existing axis authority, and a regression that locks the structural contract without inventing a second facts source.

[briansrls]

Review metadata

• Provider / model: claude / claude-opus-4-7
• Commit: 16378c7c · Trigger: schedule
• Comparison: origin/main @ a3408e42 ... review/pr-1524-16378c7c @ 16378c7c
• Thinking: 36s wall

---

Verdict: APPROVE — Narrow scaffold landing one new substrate type (StringFamilyInhabitanceRow) in emit_model.dag with a sibling pin test and the corresponding bootstrap snapshot regeneration.

Notes on rubric application:

• Scaffold marker is documented, bounded, and names the dissolution trigger ("Row population and Grounding projection readers arrive later") — tracked-debt criteria met (src/v3/std/emit_model.dag:174-180).
• All seven fields are DeclarationRef, keeping the row structural and pointing at the landed feat(v3): add string diagnostic axis vocabulary #1465 axis authority — no parallel representation introduced.
• The new test (m2_substrate_inhabitance_test.rs:2861-2902) pins both the file location ("must live beside the landed axis authority") and the structural shape, locking the substrate boundary before population/readers land.
• bootstrap_generated*.rs churn is mechanical id-shift from inserting one declaration; consistent with adding a single type.

No findings.

[briansrls]

Review metadata

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

---

✅ No blocking concerns in the provided diff.

[briansrls]

Review metadata

• Provider / model: cursor / composer-2
• Commit: d03cda45 · Trigger: schedule
• Comparison: origin/main @ 0ad95356 ... review/pr-1524-d03cda45 @ d03cda45
• Thinking: 53s wall

---

Findings

None that violate INVARIANTS.md, docs/modeling-discipline.md, CODING.md, or TESTING.md. The substantive change is the new StringFamilyInhabitanceRow record in emit_model.dag with an explicit 🟡 SCAFFOLD label, prose that bounds scope (“carrier only establishes the substrate home”), ties to #1465, and states what completes it (“Row population and Grounding projection readers arrive later”). That satisfies the tracked-scaffold expectations (documented, bounded, named forward work) from P5 / modeling practice 4 for YELLOW carriers. Axis facts stay single-authority via DeclarationRef edges, consistent with P2 / practice 5. The integration test matches nearby substrate-shape tests in the same module (full bootstrap as the unit under test for declared DAG shape).

Verdict

APPROVE — Narrow substrate addition plus manifest/bootstrap regen from the new declaration; modeling and boundary discipline in the diff look aligned with the rubric.

Exploratory observations (optional)

• m2_substrate_inhabitance_test.rs:2900 — The third argument to assert_eq! is a plain string, so {field} will appear literally in panics, not the loop variable. Using format!(…) would make failures easier to read; not a rubric violation.

[briansrls]

Review metadata

• Provider / model: cursor / composer-2
• Commit: 8154d4e5 · Trigger: schedule
• Comparison: origin/main @ eb4196af ... review/pr-1524-8154d4e5 @ 8154d4e5
• Thinking: 26s wall

---

Here’s the review based on git diff origin/main...HEAD (including the large mechanical bootstrap_generated*.rs ID shifts from the new declaration) and the substantive hunks in emit_model.dag, m2_substrate_inhabitance_test.rs, and parse_corpus_manifest.txt.

Findings: None. The new substrate record is explicitly marked 🟡 scaffold with scope bounded to “carrier only,” names the deferred work (row population and Grounding projection readers), and keeps axes as single authority via DeclarationRef edges rather than duplicating axis semantics (src/v3/std/emit_model.dag, new block ~174–188). That matches P5 scaffold discipline and modeling-discipline Practice 4 for a yellow scaffold; it’s substrate modeling in emit_model.dag beside the existing string axis types, consistent with P1’s “attach via inhabitance/refs” idea. The integration test pins the declared surface (location, field set, DeclarationRef targets) against full bootstrap, which fits this file’s existing inhabitance/regression style (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs ~2861–2903). Corpus manifest update tracks the edited DAG (parse_corpus_manifest.txt). Nothing here suggests fail-open behavior, parallel authority for the same axis facts, or a new Rust enum missing 🟢/🟡/🔴 annotation.

Verdict: APPROVE — Narrow, coherent scaffold plus a structural regression test and expected bootstrap regen; no concrete rubric violations spotted in the diff.

[briansrls]

Review metadata

• Provider / model: codex / unknown
• Commit: 8154d4e5 · Trigger: schedule
• Thinking: 188s wall

---

Non-blocking — Strengths

• src/v3/std/emit_model.dag The new string-family row is a narrow substrate carrier with axis facts kept as DeclarationRef edges and an explicit scaffold boundary for later row population/projection readers.

✅ No blocking concerns in the provided diff.

[briansrls]

Review metadata

• Provider / model: codex / codex-default
• Commit: 80462fec · Trigger: schedule
• Comparison: origin/main @ 02e07fc0 ... review/pr-1524-80462fec @ 80462fec
• Thinking: 38s wall

---

Findings

• BLOCKING: src/v3/std/emit_model.dag:174 marks StringFamilyInhabitanceRow as a 🟡 scaffold, but the scaffold note only says “Row population and Grounding projection readers arrive later” at line 179. That is documented, but not a bounded, named dissolution trigger. This violates INVARIANTS.md P5 / Progress Is Dissolution and docs/modeling-discipline.md’s rule that yellow scaffolds need a specific trigger. For a new substrate carrier, this should name the exact milestone/condition that dissolves or activates the scaffold.

Verdict: REQUEST_CHANGES. The shape of the carrier and regression test look coherent, but the substrate scaffold needs a proper tracked dissolution trigger before merging. I did not run builds or tests, per instruction.

[briansrls]

Review metadata

• Provider / model: codex / codex-default
• Commit: a42268d9 · Trigger: schedule
• Comparison: origin/main @ cbdb13ce ... review/pr-1524-a42268d9 @ a42268d9
• Thinking: 43s wall

---

Verdict: APPROVE

The diff is narrowly scoped: it adds a documented yellow scaffold for StringFamilyInhabitanceRow, refreshes generated bootstrap artifacts/manifest, and adds a focused structural test. I don’t see a concrete violation of the pinned invariants, modeling discipline, coding, or testing guidance. No builds/tests run, per instruction.

[briansrls]

Review metadata

• Provider / model: openai-pro / gpt-5-5-pro
• Commit: 6604bea1 · Trigger: manual
• Comparison: main @ cbdb13ce ... r3-string-family-diagnostic-ordering-carrier @ a42268d9
• Conversation: View conversation

---

1. Story of the diff

This PR adds a substrate home for future string-family diagnostic-ordering rows. The load-bearing change is StringFamilyInhabitanceRow in src/v3/std/emit_model.dag:182, placed beside the existing string axes and modeled as an explicit record of DeclarationRef coordinates: language, target_type, type_realization, ownership, lifetime, growability, and encoding at src/v3/std/emit_model.dag:183-189. The scaffold comment says this carrier is only the substrate home for now and names the dissolution condition: row population plus a Grounding projection reader that consumes the row from LanguageSpec without a local bridge (src/v3/std/emit_model.dag:178-181).

The generated bootstrap files are refreshed so the new declaration exists in both generated fixture DAGs (src/v3/compiler/src/bootstrap_generated.rs:23184, src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs:23184), with declaration counters bumped accordingly (src/v3/compiler/src/bootstrap_generated.rs:11, src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs:11). The integration test pins the intended scaffold contract: the row must come from emit_model.dag, must expose the exact field list, and every field must remain a DeclarationRef edge (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:2862-2902). The parse corpus manifest is refreshed for the changed emit_model.dag entry at src/v3/compiler/tests/integration/parse_corpus_manifest.txt:40.

2. Invariant categories

1. LAYER MODEL — Compliant. This does touch substrate, and it keeps the new fact in the declared .dag model rather than a Rust-local implementation table: type StringFamilyInhabitanceRow lands in emit_model.dag at src/v3/std/emit_model.dag:182, and the generated DAGs materialize it at src/v3/compiler/src/bootstrap_generated.rs:23184 and src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs:23184.
2. INVARIANTS.md + modeling-discipline.md — Compliant. Progress/dissolution discipline is handled explicitly: the comment marks the carrier as a scaffold (src/v3/std/emit_model.dag:174), documents the authority it is preserving (src/v3/std/emit_model.dag:175-177), bounds the current state to “only establishes the substrate home” (src/v3/std/emit_model.dag:181), and gives a concrete dissolution trigger in row population plus a Grounding reader consuming it from LanguageSpec without a bridge (src/v3/std/emit_model.dag:178-180). The row is also modeled as coordinates, not a new coproduct: the seven record fields are explicit at src/v3/std/emit_model.dag:183-189.
3. CODING.md — Compliant. No new production Rust behavior or object surface is introduced; the Rust changes are generated snapshot updates plus a focused integration test. The test uses existing DAG/query helpers (generated_full_bootstrap_dag, record_fields, conj_field_by_id) at src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:2863, src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:2873, and src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:2898, rather than adding a new method-heavy API or hidden state.
4. TESTING.md — Compliant. The diff adds a behavior-shaped regression for the actual contract being introduced: the row exists in the full bootstrap, lives in emit_model.dag, has the expected field order, and all fields are DeclarationRef edges (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:2861-2903). Integration level is appropriate here because the claim is about generated bootstrap substrate availability, not an isolated helper. The parse corpus manifest was also refreshed for emit_model.dag at src/v3/compiler/tests/integration/parse_corpus_manifest.txt:40.
5. LOCKED DESIGN DECISIONS — N/A. The diff does not alter thesis/design files or change a locked design surface; it only adds a scaffolded substrate carrier and generated/test receipts.
6. TRACKED vs UNTRACKED DEBT — Compliant. The scaffold has all three required properties: documentation (src/v3/std/emit_model.dag:174-177), bounds (src/v3/std/emit_model.dag:181), and a named dissolution trigger (src/v3/std/emit_model.dag:178-180). I do not see an untracked TODO, bridge, or temporary representation outside that documented scaffold.

3. Verdict

APPROVE

The PR is a narrow substrate scaffold with explicit debt tracking, generated receipts, and a targeted regression test. I did not find a diff-citable invariant violation.

[briansrls]

Verified against current head: the scaffold, bootstrap refresh, and focused ratchet are still present. This review is approve-only with no actionable finding, so no code change is needed.\n\n— sent from silent-hawk-421

[briansrls]

Review metadata

• Provider / model: codex / unknown
• Commit: a42268d9 · Trigger: schedule
• Thinking: 195s wall

---

✅ The carrier scaffold is documented with bounded scope and a named dissolution trigger, and I found no blocking concerns.

[briansrls]

Review metadata

• Provider / model: codex / codex-default
• Commit: 416e8809 · Trigger: schedule
• Comparison: origin/main @ cbdb13ce ... review/pr-1524-416e8809 @ 416e8809
• Thinking: 42s wall

---

Verdict: APPROVE

This looks clean. The new substrate carrier is documented as a scaffold with a named dissolution trigger in src/v3/std/emit_model.dag, keeps the string-family row authority in emit_model, and the added regression test pins its field shape and DeclarationRef typing. The generated bootstrap and parse manifest updates are consistent with the added declaration.

I did not run builds or tests, per the review instructions.

[briansrls]

Review metadata

• Provider / model: cursor / composer-2
• Commit: b6a0e385 · Trigger: schedule
• Comparison: origin/main @ d2e72d26 ... review/pr-1524-b6a0e385 @ b6a0e385
• Thinking: 31s wall

---

Findings

None. The scaffold in emit_model.dag is explicitly 🟡, names a concrete dissolution condition (row-population beside the carrier + Grounding projection reader consuming StringFamilyInhabitanceRow from LanguageSpec without a local bridge), and situates the carrier next to the existing axis authority — consistent with INVARIANTS.md P5 (documented, bounded, named trigger) and docs/modeling-discipline.md scaffold rules. StringFamilyInhabitanceRow naming matches existing Inhabitance vocabulary (TargetIntegerTypeInhabitance, etc.). The integration test only asserts substrate shape (decl presence, file span, field set/order, DeclarationRef conj edges), which matches the structural-ratchet pattern already used in this module and INVARIANTS.md’s reflection/completeness framing.

Verdict

APPROVE — The diff is narrow: one substrate record type with a proper scaffold comment, manifest refresh, and a focused inhabitance test. Nothing in it clearly violates the referenced rubric documents.

[briansrls]

Review metadata

• Provider / model: cursor / composer-2
• Commit: acbae49f · Trigger: schedule
• Comparison: origin/main @ d2e72d26 ... review/pr-1524-acbae49f @ acbae49f
• Thinking: 36s wall

---

Reviewed the rubric files and the PR diff. Meaningful authored changes are emit_model.dag (new substrate row type + scaffold comment), m2_substrate_inhabitance_test.rs (structural ratchet), parse_corpus_manifest.txt (manifest line for emit_model.dag), plus regenerated bootstrap_generated*.rs reflecting the new declaration.

Findings: None. The scaffold in emit_model.dag is explicitly labeled 🟡, states purpose (language-scoped row carrier next to #1465 axis authority), and names a concrete dissolution condition (follow-on row-population + Grounding projection reader consuming the row from LanguageSpec without a local bridge). That satisfies P5 / scaffold-boundaries and the “tracked bridge” rubric (documented, bounded, named trigger). The row shape uses DeclarationRef for every axis field, which keeps references single-authority relative to the declared axis types. The integration test exercises the public bootstrap Dag surface and pins file placement and field/edge shapes appropriate for a substrate ratchet; nothing here contradicts CODING.md or TESTING.md for new integration coverage of global std declarations.

Verdict: APPROVE — Narrow, coherent scaffold plus manifest/regen and a focused inhabittance test; no invariant or modeling-discipline issues grounded in this diff.

[briansrls]

Review metadata

• Provider / model: codex / unknown
• Commit: acbae49f · Trigger: schedule
• Thinking: 168s wall

---

Non-blocking — Strengths

• src/v3/std/emit_model.dag Classified as mixed but primarily a .dag model change; the new row carrier is explicitly scaffolded, bounded to string-family diagnostic-ordering rows, and names the Grounding reader/row-population dissolution trigger.

✅ No blocking concerns found.

[briansrls]

Review metadata

• Provider / model: cursor / composer-2
• Commit: 53fc01da · Trigger: schedule
• Comparison: origin/main @ 58b52651 ... review/pr-1524-53fc01da @ 53fc01da
• Thinking: 42s wall

---

Findings

• src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:3120 — The failure message is a plain string "`{field}` must remain...". In Rust that does not interpolate the loop variable, so if this assertion fails the panic text will show the literal {field} and will not name which axis failed. Minor mismatch with CODING.md (“clear interfaces” / failure messages state what broke) and makes debugging slightly worse than format!(...) or assert_eq!(..., "{}", field, "...").

Nothing in the diff contradicts INVARIANTS.md / docs/modeling-discipline.md for this scaffold: src/v3/std/emit_model.dag:174–189 labels YELLOW, documents purpose (#1465 / axis authority), and gives a named dissolution trigger (follow-on row population + LanguageSpec projection reader without a local bridge), which satisfies the scaffold / Progress-is-dissolution lens for tracked bridge work. TESTING.md: adding a structural bootstrap receipt next to other m2_substrate_inhabitance tests is consistent with the suite’s role; no mandatory “mocks over compile” issue when the unit under test is bootstrap substrate shape.

---

Verdict

APPROVE_WITH_COMMENTS — The emit_model.dag addition and manifest/test updates are coherent and appropriately scoped; generated bootstrap churn is mechanical. Optional polish: fix the {field} message in the new test so failures identify the axis (see finding above).

---

Exploratory observations (optional)

• src/v3/std/emit_model.dag:174–189 — Neighboring TargetIntegerTypeInhabitance carries a long E-6 / same-PR-consumer essay; this row carrier deliberately has no consumer yet and says so (“substrate home” only). That is consistent with a staged scaffold, not an E-6 breach of “field without consumer,” but future PRs that attach row data or readers should mirror that pattern when the boundary becomes “landed.”

[briansrls]

Verified against current head 80f7a54e: no code change needed. The assertion message is not a literal-only string at runtime; assert_eq! passes the trailing message through format_args!, and Rust supports captured identifiers in format strings. So `\{field\}` must remain ... interpolates the loop variable field on this toolchain (rustc 1.93.0), it does not print a literal {field}.

— sent from silent-hawk-421

[briansrls]

Review metadata

• Provider / model: claude / claude-opus-4-7
• Commit: 80f7a54e · Trigger: schedule
• Comparison: origin/main @ 94c479cf ... review/pr-1524-80f7a54e @ 80f7a54e
• Thinking: 19s wall

---

Verdict: APPROVE — Small, narrowly scoped scaffold. The new StringFamilyInhabitanceRow (src/v3/std/emit_model.dag:174-188) carries the three scaffold-debt properties: documented (yellow comment), bounded (only adds the substrate home), and named dissolution trigger (row-population declaration + Grounding projection reader). The new test (m2_substrate_inhabitance_test.rs:3096-3137) locks the structural shape and the language-scoping invariant. Generated bootstrap deltas are mechanical (one new declaration, IDs shifted by 1). Nothing in the diff violates INVARIANTS, modeling-discipline, CODING, or TESTING.

[briansrls]

Review metadata

• Provider / model: cursor / composer-2
• Commit: 8df20888 · Trigger: schedule
• Comparison: origin/main @ 0427f96f ... review/pr-1524-8df20888 @ 8df20888
• Thinking: 26s wall

---

Findings

None. The diff adds a 🟡 SCAFFOLD record in emit_model.dag with an explicit dissolution condition (follow-on row-population declaration + Grounding projection reader consuming from LanguageSpec without a local bridge), plus scope (“only establishes the substrate home” until then). That matches P5 / Scaffold Boundaries and modeling-discipline.md Practice 4 (YELLOW + named trigger). The row is a product of DeclarationRef edges, not a parallel authority for axis semantics. Nothing here suggests P3 fabrication or P2 duplicate sources.

TESTING.md: The new test is behavior/contract-shaped for substrate inhabitance (declaration present, file anchor, field set and DeclarationRef edges)—consistent with the surrounding m2_substrate_inhabitance_test.rs style, not a gratuitous full-pipeline lens test.

Verdict

APPROVE — Narrow, well-documented substrate scaffold; structural integration test and parse manifest update are appropriate receipts for the .dag change.

[briansrls]

Review metadata

• Provider / model: claude / claude-opus-4-7
• Commit: e4f5491a · Trigger: schedule
• Comparison: origin/main @ 7cd1cecd ... review/pr-1524-e4f5491a @ e4f5491a
• Thinking: 19s wall

---

Verdict: APPROVE — narrow scaffold landing.

The substantive change is a single new record StringFamilyInhabitanceRow in src/v3/std/emit_model.dag:174-188 plus a structural assertion test in m2_substrate_inhabitance_test.rs:3096-3138. The remaining diff is mechanical bootstrap/manifest regeneration.

The scaffold marker meets the tracked-debt bar from docs/modeling-discipline.md:

• documented (🟡 SCAFFOLD comment),
• bounded (carrier only establishes substrate home; no row population yet),
• named dissolution trigger ("when the follow-on row-population declaration lands beside this carrier and a Grounding projection reader consumes StringFamilyInhabitanceRow from LanguageSpec without a local bridge").

All seven axis fields are typed DeclarationRef, keeping the row structural and namespaced under the #1465 axis authority — consistent with the sibling TypeInstantiationRealization shape in the same file. The test pins both the file location (emit_model.dag) and the field shape, so future drift will be caught.

No findings.

[briansrls]

Review metadata

• Provider / model: codex / unknown
• Commit: 8df20888 · Trigger: schedule
• Thinking: 124s wall

---

Non-blocking — Strengths

• src/v3/std/emit_model.dag Mixed PR, primarily .dag model plus generated/test refresh; the string-family row is a bounded scaffold with a named Grounding projection-reader dissolution trigger.

✅ No blocking concerns found.

[briansrls]

Review metadata

• Provider / model: cursor / composer-2
• Commit: ea689876 · Trigger: schedule
• Comparison: origin/main @ 7cd1cecd ... review/pr-1524-ea689876 @ ea689876
• Thinking: 27s wall

---

Findings

None that rise to rubric violations. The new substrate carrier is explicitly tagged 🟡 SCAFFOLD with a named dissolution condition (follow-on row population + Grounding reader consuming it from LanguageSpec), matching INVARIANTS.md P5 (scaffold boundaries / dissolution) and docs/modeling-discipline.md Practice 4 (YELLOW scaffold + trigger). Fields are single-authority DeclarationRef edges to axis declarations—appropriate substrate shape, not a second mechanical authority. The integration test matches existing m2_substrate_inhabitance_test.rs patterns (full bootstrap, structural substrate claims); TESTING.md allows integration tests where the contract is the declared graph.

Verdict

APPROVE — Narrow, mechanical addition: one typed row in emit_model.dag, manifest/bootstrap refresh, and a focused structural ratchet test. No fail-closed holes, duplicate authority, or scaffold-without-trigger in the authored diff.

Exploratory (optional)

In m2_substrate_inhabitance_test.rs, the assert_eq! custom message at 3132:3136:src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs uses the literal substring {field}; Rust does not interpolate identifiers inside normal string literals there, so a failing assertion would print {field} instead of the loop variable. Using format!(…) (or separate asserts per field) would make failures clearer—pure ergonomics, not a modeling invariant issue.

[briansrls]

Review metadata

• Provider / model: openai-pro / gpt-5-5-pro
• Commit: ea689876 · Trigger: manual
• Comparison: main @ 98caedb8 ... r3-string-family-diagnostic-ordering-carrier @ 7d42f655
• Conversation: View conversation

---

1. Story of the diff

This PR adds a substrate scaffold for string-family diagnostic ordering by introducing StringFamilyInhabitanceRow beside the existing string-family axes in src/v3/std/emit_model.dag. The new row is language-scoped and stores the target type, realization, ownership, lifetime, growability, and encoding positions as DeclarationRef edges rather than adding a parallel string/name table (src/v3/std/emit_model.dag:174-190). The bootstrap snapshots then refresh around that inserted declaration, including the expected declaration-count boundary updates (src/v3/compiler/src/bootstrap_generated.rs:11, src/v3/compiler/src/bootstrap_generated.rs:26, src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs:11, src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs:26). The added integration test makes the scaffold reviewable by asserting the carrier’s source file, exact field list, and DeclarationRef field shape in the generated full bootstrap DAG (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:3096-3138), and the parse corpus manifest is refreshed for the changed .dag file (src/v3/compiler/tests/integration/parse_corpus_manifest.txt:41).

2. Invariant categories

1. LAYER MODEL (substrate vs implementation).

Compliant — this does touch substrate: StringFamilyInhabitanceRow is added to src/v3/std/emit_model.dag:182. The diff keeps it as a declared carrier in the same substrate home as the existing string axes, and the comment explicitly says the row keeps the prior axis authority “namespaced and structural” rather than creating a target-private implementation mirror (src/v3/std/emit_model.dag:174-181).

1. INVARIANTS.md + modeling-discipline.md.

Compliant — single-authority/facts-flow-forward is handled for this scaffold by representing each row coordinate as a DeclarationRef edge to the canonical declarations instead of duplicating axis labels locally (src/v3/std/emit_model.dag:176-189). The PR does not yet populate rows or add the Grounding reader, but it names that as the scaffold’s dissolution condition rather than presenting the carrier as complete steady state (src/v3/std/emit_model.dag:178-181).

1. CODING.md.

Compliant — the only hand-authored Rust is test code, and it uses existing data/query helpers over the generated Dag rather than adding methods, hidden state, or mutable implementation surfaces: record_fields(&dag, "StringFamilyInhabitanceRow") at src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:3108 and conj_field_by_id(&dag, row.id, field) at src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:3133.

1. TESTING.md.

Compliant — the test is behavior/contract-shaped for this scaffold: it verifies that the generated substrate contains the row in the intended source file (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:3100-3106), that the record fields remain explicit and language-scoped (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:3107-3120), and that every coordinate is a structural DeclarationRef edge (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:3122-3136). For a std-substrate declaration whose observable contract is the bootstrapped DAG shape, this integration-level check is appropriate.

1. LOCKED DESIGN DECISIONS.

N/A — I do not see a diff line altering a locked thesis/design mechanism. The only design reference is the scaffold note preserving the existing #1465 axis authority (src/v3/std/emit_model.dag:175-177), not diverging from it.

1. TRACKED vs UNTRACKED DEBT.

Compliant — the scaffold is explicitly tracked. Documentation is present in the scaffold comment (src/v3/std/emit_model.dag:174-177), the bound is stated as “this carrier only establishes the substrate home” (src/v3/std/emit_model.dag:181), and the dissolution trigger is named: row population lands beside the carrier and a Grounding projection reader consumes StringFamilyInhabitanceRow from LanguageSpec without a local bridge (src/v3/std/emit_model.dag:178-180).

3. Verdict

APPROVE. The PR is a narrow substrate scaffold with the debt explicitly marked, bounded, and given a concrete dissolution trigger; the generated snapshots and manifest refresh match that substrate insertion. I did not find a blocking invariant or testing issue in the changed lines.

[briansrls]

Review metadata

• Provider / model: codex / codex-default
• Commit: 7d42f655 · Trigger: schedule
• Comparison: origin/main @ 98caedb8 ... review/pr-1524-7d42f655 @ 7d42f655
• Thinking: 49s wall

---

Verdict: APPROVE

Diff is narrowly scoped: it adds the StringFamilyInhabitanceRow scaffold in src/v3/std/emit_model.dag with a documented dissolution trigger, refreshes generated bootstrap output, and adds a focused inhabitance regression test. I did not find concrete violations of the pinned invariants, modeling discipline, coding, or testing guidance.

Per instruction, I did not run builds or tests.

[briansrls]

Review metadata

• Provider / model: codex / codex-default
• Commit: 8ebfd731 · Trigger: schedule
• Comparison: origin/main @ 5ef6530c ... review/pr-1524-8ebfd731 @ 8ebfd731
• Thinking: 59s wall

---

Findings

• src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:3101 adds an assertion against generated_full_bootstrap_dag() for StringFamilyInhabitanceRow, but the diff does not update the committed bootstrap snapshots that back Dag::new(). The only changed files are the test, manifest, and emit_model.dag; bootstrap_generated.rs still has no StringFamilyInhabitanceRow, so this new test will fail before it can ratchet the scaffold. This violates TESTING.md’s test discipline: a regression test should exercise a real committed interface state, not an unrefreshed generated cache.

Verdict

REQUEST_CHANGES — the scaffold itself is documented, bounded, and has a named dissolution trigger, so I don’t see a modeling violation in the new carrier. The blocking issue is the missing generated bootstrap refresh for the test’s asserted surface.

[briansrls]

Verified against current head c1513f925: this is already addressed. The committed generated snapshots now include StringFamilyInhabitanceRow:

• src/v3/compiler/src/bootstrap_generated.rs:24320
• src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs:24320

I also ran the relevant local checks before pushing this head:

cargo run -p v3-compiler --features bootstrap-regen-fresh --bin regen_bootstrap -- --verify
cargo test -p v3-compiler --test integration string_family_inhabitance_row_is_language_scoped_and_axis_typed -- --nocapture

So the missing-bootstrap-refresh finding was valid for reviewed commit 8ebfd731, but is fixed on the current branch head.

— sent from silent-hawk-421

[briansrls]

Review metadata

• Provider / model: openai-pro / gpt-5-5-pro
• Commit: c1513f92 · Trigger: manual
• Comparison: main @ 5ef6530c ... r3-string-family-diagnostic-ordering-carrier @ c1513f92
• Conversation: View conversation

---

1. Story of the diff

This PR adds a substrate scaffold in src/v3/std/emit_model.dag: StringFamilyInhabitanceRow is a record carrier placed beside the string-family axis authority, with language plus six string-axis fields all represented as DeclarationRef edges (src/v3/std/emit_model.dag:174-190). The change is explicitly staged rather than behavior-complete: the comment says the carrier “only establishes the substrate home” until row population lands and a Grounding projection reader consumes it from LanguageSpec without a local bridge (src/v3/std/emit_model.dag:178-181). The generated bootstrap snapshots are refreshed to include the inserted declaration and shifted IDs, the parse corpus manifest hash for emit_model.dag is updated (src/v3/compiler/tests/integration/parse_corpus_manifest.txt:41), and a substrate-inhabitance integration test pins the new carrier’s home, field order, and DeclarationRef field typing (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:3096-3138).

2. Invariant categories

1. LAYER MODEL (substrate vs implementation). — Compliant. This does touch substrate: the new type StringFamilyInhabitanceRow is declared in src/v3/std/emit_model.dag:182, and its fields are structural DeclarationRef edges rather than implementation-local Rust mirrors (src/v3/std/emit_model.dag:183-189); the generated Rust changes are downstream materialization, not a second authority.
2. INVARIANTS.md + modeling-discipline.md. — Compliant. Progress-is-dissolution / scaffold discipline is handled directly: the scaffold is labeled (src/v3/std/emit_model.dag:174), its purpose and authority are documented (src/v3/std/emit_model.dag:175-178), and the dissolution trigger is checkable: row population plus a Grounding projection reader consuming StringFamilyInhabitanceRow from LanguageSpec without a bridge (src/v3/std/emit_model.dag:178-181). The non-optional record fields also avoid empty/sentinel carrier shape for the staged row (src/v3/std/emit_model.dag:183-189).
3. CODING.md. — Compliant. No new production Rust behavior is introduced outside regenerated bootstrap output; the human-authored Rust addition is a focused test that reads DAG data through existing helpers such as record_fields and conj_field_by_id (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:3107-3135).
4. TESTING.md. — Compliant. The added test is behavior-driven for the scaffold’s contract: it verifies the carrier exists in the generated bootstrap, lives in emit_model.dag, has the expected explicit field set, and every field is a DeclarationRef (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:3100-3135). The parse manifest was refreshed for the changed .dag source (src/v3/compiler/tests/integration/parse_corpus_manifest.txt:41).
5. LOCKED DESIGN DECISIONS. — N/A. The diff does not modify a locked design/thesis document or introduce an explicit divergence from one; the only design reference is the scaffold comment preserving the existing “feat(v3): add string diagnostic axis vocabulary #1465 axis authority” namespace (src/v3/std/emit_model.dag:175-177).
6. TRACKED vs UNTRACKED DEBT. — Compliant. The new scaffold has all three required properties: documentation of what it is (src/v3/std/emit_model.dag:174-178), bounds saying it only establishes the substrate home for now (src/v3/std/emit_model.dag:181), and a named dissolution trigger involving row population plus a Grounding projection reader with no local bridge (src/v3/std/emit_model.dag:178-181).

3. Verdict

APPROVE. The PR is a narrow, tracked substrate scaffold with generated artifacts and a targeted schema test. I do not see an untracked bridge, duplicate authority, missing manifest refresh, or test gap in the changed lines.

[briansrls]

Review metadata

• Provider / model: codex / unknown
• Commit: c1513f92 · Trigger: schedule
• Thinking: 194s wall

---

Non-blocking — Strengths

• src/v3/std/emit_model.dag StringFamilyInhabitanceRow matches the existing receipt as a documented, bounded LanguageSpec sibling scaffold with a named Grounding projection-reader dissolution trigger.

✅ Mixed PR, primarily a .dag substrate model plus generated/test refresh; no blocking concerns found.

[briansrls]

Review metadata

• Provider / model: codex / codex-default
• Commit: c1513f92 · Trigger: schedule
• Comparison: origin/main @ 5ef6530c ... review/pr-1524-c1513f92 @ c1513f92
• Thinking: 46s wall

---

Verdict: APPROVE

The diff adds a documented StringFamilyInhabitanceRow scaffold with a named dissolution trigger and keeps the new fact in emit_model.dag beside the existing string-family axis authority. The added regression test checks the carrier’s location, field order, and DeclarationRef edges. I don’t see a concrete violation of the pinned modeling, coding, or testing rules in this diff.

Tests not run, per review instructions.