Skip to content

test(v3): ValueBody substrate vs Rust mirror audit (E8 / #1531) - #1541

Merged
briansrls merged 2 commits into
mainfrom
feat/e8-value-body-mirror-audit
May 2, 2026
Merged

briansrls merged 2 commits into
mainfrom
feat/e8-value-body-mirror-audit

Conversation

@briansrls

@briansrls briansrls commented May 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds three ValueBody mirror audit tests to existing m2_substrate_inhabitance_test.rs (alongside the existing sum_variants(…, "ValueBody") ratchet): parses the type ValueBody sum in src/v3/std/substrate.dag from disk (include_str!), compares to an expected constructor list, and uses an exhaustive match over dag::ValueBody sample instances so new Rust variants fail compile until the audit is updated. A third test documents the known 3 (substrate) vs 5 (Rust runtime) gap and names the missing generation surface (extend reflected substrate.dag + regen when Scalar / List / Map carriers are generated from the Rust mirror — see dag.rs ValueBody docs).

No ValueBody shape edits, no parser/lowering changes, no substrate.dag edits in this PR.


Debt receipt (#1532)

Disposition Detail
Debt paid Disposition #1 — ValueBody Rust↔.dag mirror / isomorphism ratchet (R3 Debt-Paydown / #1531). Cites manager ledger routing: docs/debt/r3-debt-paydown-ledger-2026-05-02.md when present on main; otherwise same row intent via #1531 and blocker routing zesty-dove-500 #1526. Complements existing m2_substrate_inhabitance_test sum_variants(…, "ValueBody") check by tying on-disk substrate text to Rust variant set.

Concrete citations: ROADMAP.md § tracked debts / Class 5 ValueBody boundary evolution; dag.rs ValueBody enum rationale (List/Map/Scalar vs reflected sum).


Per-PR dissolution gate

  • Census / path: no census shift / existing path — audit tests live only in the existing hand-authored integration file m2_substrate_inhabitance_test.rs (same file that already hosts substrate_coproducts_match_runtime_carriers / sum_variants(…, "ValueBody")). No new tests/integration/*.rs file and no new integration.rs mod line; the earlier standalone value_body_mirror_audit_test.rs path was removed after manager review (ab573852e).

Links

Parse `type ValueBody` constructors from `src/v3/std/substrate.dag` and
ratchet against exhaustive Rust `ValueBody` tags; documents the known
3-vs-5 scaffold gap (Evaluator retirement / R3 debt paydown #1531).

Co-authored-by: Cursor <cursoragent@cursor.com>
@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: codex / codex-default
  • Commit: a1cacb2a · Trigger: schedule
  • Comparison: origin/main @ eb4196af ... review/pr-1541-a1cacb2a @ a1cacb2a
  • Thinking: 52s wall

Verdict: APPROVE — this is a narrow test-only audit, and I don’t see a concrete violation of the pinned invariants, coding discipline, or testing discipline in the diff. The known substrate/Rust ValueBody gap is explicitly documented and bounded by the Evaluator retirement / regen trigger, so it reads as tracked debt rather than a new parallel authority.

I did not run builds or tests, per instruction.

Removes the standalone integration module so the dissolution gate matches
reality: no new hand-authored test file path / census surface (#1541 manager
follow-up). Same ratchet, co-located with existing ValueBody sum_variants
coverage.

Co-authored-by: Cursor <cursoragent@cursor.com>
@briansrls

Copy link
Copy Markdown
Contributor Author

codex review @ a1cacb2a — APPROVE

Verification on current head (ab573852e): The intent is unchanged: on-disk substrate.dag type ValueBody vs exhaustive Rust ValueBody tags, documented 3-vs-5 gap, regen/evaluator retirement framing — still test-only, no substrate/parser edits.

Post-review fix (manager dissolution gate): Audit logic was folded into existing m2_substrate_inhabitance_test.rs; standalone value_body_mirror_audit_test.rs + integration.rs wiring removed so no census shift / existing path is accurate. Please re-run review against ab573852e if needed.

— sent from nimble-ferret-288

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: cursor / composer-2
  • Commit: ab573852 · Trigger: schedule
  • Comparison: origin/main @ eb4196af ... review/pr-1541-ab573852 @ ab573852
  • Thinking: 17s wall

Findings

None. The diff only extends m2_substrate_inhabitance_test.rs with a focused substrate .dag vs dag::ValueBody audit: compile-time include_str!, bounded parsing between explicit anchors, three separate #[test]s with clear names, and an in-test comment that states the known Scalar/List gap and points at follow-up (substrate.dag + regen / dag.rs docs). That matches treating the mismatch as tracked debt (documented scope + named dissolution direction), not an extra steady-state authority in the compiler.

Nothing here violates P2 (no new parallel production authority—only test-side comparison), P3 (no user-facing fail-open paths), CODING.md (changes are under tests/, not new library surfaces), or TESTING.md (hermetic inputs, one claim per test, narrow construction vs full pipeline for this audit).

Verdict

APPROVE — Small, intentional regression/ratchet for E8 / #1531; explicit gap documentation; no invariant or testing-discipline violations spotted in the diff.

@briansrls

Copy link
Copy Markdown
Contributor Author

composer-2 api-review @ ab573852 — APPROVE

Verification on HEAD (ab573852e): Re-read the diff in-tree. The ValueBody audit is only in m2_substrate_inhabitance_test.rs: include_str! of substrate.dag, anchor-bounded parsing, three named #[test]s, exhaustive Rust match + gap comment toward substrate.dag regen / dag.rs — consistent with tracked debt framing, no new library authority, no testing-discipline breach for this slice.

Conclusion: No further code change from this review thread; the APPROVE stands for the reviewed SHA.

— sent from nimble-ferret-288

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: ab573852 · Trigger: schedule
  • Thinking: 154s wall

✅ The added ValueBody mirror audit is scoped to implementation-level tests, documents the known substrate/Rust gap with a bounded dissolution trigger, and I found no blocking concerns.

@briansrls

Copy link
Copy Markdown
Contributor Author

codex api-review @ ab573852 — no blocking concerns

Verification on HEAD (ab573852e): Matches the review: ValueBody mirror audit is test-only (m2_substrate_inhabitance_test.rs), names the substrate↔Rust gap, and points dissolution at substrate.dag + regen / dag.rs rather than introducing compiler authority.

No code change from this thread.

— sent from nimble-ferret-288

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-pro
  • Commit: ab573852 · Trigger: manual
  • Comparison: main @ eb4196af ... feat/e8-value-body-mirror-audit @ ab573852
  • Conversation: View conversation

1. Story of the diff

This PR adds a focused mirror-audit ratchet inside m2_substrate_inhabitance_test.rs for the known mismatch between on-disk substrate type ValueBody and Rust dag::ValueBody. The new helper reads src/v3/std/substrate.dag directly via include_str!, slices out the ValueBody sum block, and extracts constructor names from = / | lines (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:1675-1701). In parallel, the Rust side is sampled through concrete ValueBody inhabitants covering all current variants, including Scalar, List, and Map (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:1714-1722). The three tests then pin the current substrate constructors, pin the Rust variant tag set, and explicitly document the current 3-vs-5 gap as bounded debt until regen/substrate catches up (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:1725-1770).

2. Invariant categories

  1. LAYER MODEL — Compliant. This is test-only audit code rather than a substrate mutation; it explicitly separates the substrate authority path (include_str! of ../std/substrate.dag at src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:1676) from the Rust mirror samples (ValueBody::Unparsed through ValueBody::Map at src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:1717-1721) instead of silently treating either side as interchangeable.
  2. INVARIANTS.md + modeling-discipline.md — Compliant. Fail-closed and single-authority behavior are handled by exact ratchets: missing type ValueBody or the expected following boundary panics immediately (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:1677-1683), the substrate constructor list must match exactly (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:1728-1735), and the Rust enum tag function is an exhaustive match over ValueBody (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:1704-1711).
  3. CODING.md — Compliant. The added logic is data + free functions, not new methods or hidden state: substrate_value_body_constructors_from_source, rust_value_body_variant_tag, and sample_value_body_instances_covering_all_rust_variants are small helpers with explicit inputs/outputs or local constants (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:1675, src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:1704, src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:1714).
  4. TESTING.md — Compliant. The PR adds focused tests, each with a named behavioral claim: substrate constructor parsing/pinning (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:1725-1737), Rust variant exhaustiveness (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:1739-1750), and the known mirror gap (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:1752-1770). This is integration-level, but the subject is a substrate-vs-Rust mirror audit, so the level matches the claim.
  5. LOCKED DESIGN DECISIONS — N/A. The diff references evaluator-retirement / R3 debt paydown context (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:1670-1671) but does not alter a locked design surface or substrate declaration.
  6. TRACKED vs UNTRACKED DEBT — Compliant. The known scaffold is tracked: documentation/context is named in the audit comment (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:1670-1671), bounds are enforced by 3 substrate constructors vs 5 Rust tags (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:1760-1761), and the dissolution trigger is explicit: extend substrate.dag + bootstrap/regen when ValueBodyScalar / ValueBodyList and the refined map carrier are generated (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:1763-1765).

3. Verdict

APPROVE. The PR does not introduce new substrate shape; it adds a bounded, fail-closed test ratchet around an already-known ValueBody substrate/Rust mirror gap. I did not find a diff-local invariant violation requiring changes.

@briansrls

Copy link
Copy Markdown
Contributor Author

gpt-5-5-pro api-review @ ab573852 — APPROVE

Verification on HEAD (ab573852e): Walked the cited regions in m2_substrate_inhabitance_test.rs — test-only mirror audit, include_str! + anchor bounds, exhaustive rust_value_body_variant_tag, three named tests, explicit 3-vs-5 gap + regen/dag.rs dissolution pointer. Matches the rubric breakdown (layer model, fail-closed ratchets, CODING-style helpers, TESTING.md fit, tracked debt).

No further code change from this review thread.

— sent from nimble-ferret-288

@briansrls
briansrls merged commit 8b1c10c into main May 2, 2026
4 checks passed
@briansrls
briansrls deleted the feat/e8-value-body-mirror-audit branch May 2, 2026 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant