Repository navigation
R3 gate #85: SuiteClaim wrapper migration — Enumerated/Quantified coproduct - #2743
Conversation
# Conflicts: # src/v3/compiler/src/bootstrap_generated.rs # src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs
|
Disposition for the prior REQUEST_CHANGES review at head
Non-concerns from the prior review (clean coproduct dissolution, mechanical fixture wrap, fail-closed runner on unknown variants) are unchanged at current head. — sent from sleek-ibex-221 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
de6148da· Trigger:schedule - Thinking:
769s wall
BLOCKING (1)
Root Cause
src/v3/std/verification.dagSuiteClaim wrapper migration flipped the suite carrier before generalizing obligation materialization overEnumeratedandQuantifiedentries → add a SuiteClaim-level obligation projection that reads each variant's singlerequiresauthority.
ROADMAP — Verified
- T-Tests-As-Data-Completeness gate #85:
docs/r3-program-plan.md§1.8 row #85 exists and tracksforall_exists_quantifier_substrate_landedunder the cited lane.
| type TestSuite { | ||
| name: String | ||
| claims: List<TestClaim> | ||
| claims: List<SuiteClaim> |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Per PR #2743 reviews (briansrls inline at verification.dag:404, codex api-review): the SuiteClaim wrapper migration flipped the suite carrier but left materialize_test_obligations consuming List<TestClaim>, so QuantifiedTestClaim.requires had no suite-level consumer (INVARIANTS P2 — facts must flow forward). Add obligation_for_quantified_claim + obligation_for_suite_claim variant dispatch, and flip materialize_test_obligations to consume List<SuiteClaim>. requires remains the sole authority on both variants. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Addressed at commit The SuiteClaim-level obligation projection now lives at
— sent from sleek-ibex-221 |
|
Review metadata
1. Story of the diffThis PR finishes the SuiteClaim wrapper migration for tests-as-data: The substrate side also generalizes obligation projection: enumerated claims still use 2. Invariant categories1. LAYER MODEL (substrate vs implementation)Compliant — this does touch substrate: 2. INVARIANTS.md + modeling-discipline.mdFinding — non-blocking, single-authority metadata. The new quantified obligation projection uses the modeled quantified claim name as the claim identity: chatgpt-review-2bfd2a62-e8ac-4c… The low-cost fix is to parse or project the 3. CODING.mdCompliant — the Rust change keeps the old pipeline readable by extracting the new suite-entry dispatch into chatgpt-review-0e895021-a534-41… 4. TESTING.mdCompliant, with one caveat tied to the finding above — the diff updates the existing suite fixtures and runner string fixtures to the new The caveat: I did not see a new test in this diff that exercises a 5. LOCKED DESIGN DECISIONSCompliant — no locked design target appears to be contradicted. The change is aligned with the thesis/testing direction that tests live as 6. TRACKED vs UNTRACKED DEBTCompliant — the only obvious temporary behavior is quantified runner evaluation returning 2.5. Top-down PM intent reviewCompliant, subject to the non-blocking identity comment above. At the PM level, this PR preserves the intended direction: tests remain I do not see a concrete diff-cited semantic dilution of the zero-floor / tests-as-data plan. The added hand-Rust dispatch in 3. VerdictAPPROVE_WITH_COMMENTS The substrate migration is coherent: one ordered |
openai-pro APPROVE_WITH_COMMENTS non-blocking: runner Quantified branch reported claim_name from the declaration label while obligation-walk uses QuantifiedTestClaim.name. Read the modeled `name` field off the declaration so both surfaces agree on a single identity (INVARIANTS P2), with fail-closed fallback to the declaration label when the structural read fails. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Addressed openai-pro APPROVE_WITH_COMMENTS non-blocking finding at commit
— sent from sleek-ibex-221 |
CI regen_bootstrap --verify drift on bootstrap_generated_without_parse_surface.rs. Apparent regen-stale at 5f6af16 — the obligation-projection .dag edits shifted PortId allocations downstream of the new `match` arms but the verify-snapshot wasn't fully captured. Fresh regen now consistent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
codex REQUEST_CHANGES: the `Quantified(...)` runner branch accepted any declaration as a deferred quantified claim without verifying it actually has `QuantifiedTestClaim` shape (INVARIANTS P3 / fail-closed). The enumerated path already validates via `TestClaimValue::from_declaration`; mirror that discipline on the quantified path. Add `validate_quantified_claim_shape` checking `name` + `generator` + `quantifier` + `predicate` + `requires` are structurally present on the referenced declaration before returning `NotYetImplemented`. Malformed declarations now fail closed with a structured `ClaimResult::Fail`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Addressed codex REQUEST_CHANGES at commit
On success the validator returns the modeled — sent from sleek-ibex-221 |
# Conflicts: # src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
5f6af161· Trigger:schedule - Thinking:
622s wall
BLOCKING (1)
Root Cause
src/v3/compiler/src/test_runner.rsQuantified evaluation was deferred after adding the SuiteClaim runner boundary → validate the deferred carrier's generator, quantifier, predicate, and payload arity against the substrate shape before returning NotYetImplemented.
ROADMAP — Verified
- T-Tests-As-Data-Completeness gate #85: docs/r3-program-plan.md §1.8 row #85 still tracks forall_exists_quantifier_substrate_landed and names the wrapper migration/generated-runner consumer path.
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
c3c2edd1· Trigger:schedule - Thinking:
578s wall
BLOCKING (1)
Root Cause
src/v3/std/verification.dagSuiteClaim wrapper migration updated the tests/dag copy but missed the live tests/fixtures/r1_pb_census_gates.dag consumer included by integration tests → wrap that fixture's six entries as Enumerated(...) in the same migration.
ROADMAP — Verified
- T-Tests-As-Data-Completeness gate #85: docs/r3-program-plan.md §1.8 row #85 tracks forall_exists_quantifier_substrate_landed and the SuiteClaim consumer/wrapper prerequisite.
| type TestSuite { | ||
| name: String | ||
| claims: List<TestClaim> | ||
| claims: List<SuiteClaim> |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
codex BLOCKING follow-up: the suite-entry validator accepted any payload-non-empty `Enumerated(...)` / `Quantified(...)` while the substrate carriers are single-argument coproduct arms. Reject payloads with arity != 1 with a structured Fail (INVARIANTS P3). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Verified against current HEAD ( The codex review at
All four reviewer-named shape facets (generator, quantifier, predicate, payload arity) are now validated before — sent from sleek-ibex-221 |
briansrls inline review (BLOCKING) at verification.dag:404: an integration fixture with bare TestClaim entries leaks past the new SuiteClaim boundary. Cited fixture path (r1_pb_census_gates.dag) does not exist; the real unmigrated site is the inline Rust string fixture at m1_5_verification_test.rs:340. Wrap each entry as `Enumerated(...)` to match the post-#85 carrier shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Addressed inline BLOCKING at The cited fixture path Sweep verification: — sent from sleek-ibex-221 |
|
Verified against current HEAD ( The cited path ``` The actually-existing PB-census-gates fixture The one inline Rust fixture I had missed ( — sent from sleek-ibex-221 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
de39c846· Trigger:schedule - Thinking:
605s wall
Non-blocking — Strengths
src/v3/std/verification.dagSuiteClaim preserves suite ordering while projecting both Enumerated and Quantified entries through one TestObligation surface.src/v3/compiler/src/test_runner.rsThe runner fails malformed SuiteClaim entries closed before claim execution and uses the modeled QuantifiedTestClaim.name for deferred reporting.
ROADMAP — Verified
- forall_exists_quantifier_substrate_landed: docs/r3-program-plan.md §1.8 row #85 tracks the SuiteClaim wrapper migration and deferred quantified-claim consumer work.
✅ No blocking concerns; targeted cargo verification could not complete because restricted network/offline cache resolution blocked dependency lookup.
|
Review metadata
1. Story of the diffThis PR finishes the SuiteClaim wrapper migration that was previously staged: 2. Invariant categories
Compliant — this does touch substrate:
Compliant — P2/facts-flow-forward is handled by
Compliant — the Rust change keeps the runner logic decomposed into a small helper (
Finding — missing direct Quantified regression for the new runner branch. The diff adds a new
N/A — I do not see this diff altering a locked design decision. It appears to implement the already-staged wrapper migration by making
Compliant — the remaining deferred piece is explicitly scoped: the runner returns 2.5. Top-down PM intent reviewCompliant, with the testing caveat above. The PM-level intent is preserved: tests remain structural data, the suite surface becomes a single ordered wrapper carrier, and quantified claims are introduced without creating a parallel dependency surface. That aligns with the thesis/testing direction that the proof/test surface is structurally derived from The one mismatch is not semantic dilution in the implementation; it is verification coverage: the new quantified branch should be directly exercised before the gate is considered closed. 3. VerdictREQUEST_CHANGES The substrate and implementation shape look right: single ordered |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
8b2473a5· Trigger:schedule - Thinking:
516s wall
Non-blocking — Strengths
src/v3/compiler/src/test_runner.rsSuiteClaim runner dispatch fails closed on malformed wrapper shape and uses QuantifiedTestClaim.name as the single reporting authority.
ROADMAP — Verified
- forall_exists_quantifier_substrate_landed: docs/r3-program-plan.md §1.8 row #85 tracks the SuiteClaim wrapper migration plus runner/generated consumer prerequisite.
openai-pro REQUEST_CHANGES: the gate-#85 headline behavior — Quantified suite entries flowing through run_suite_entry — was not directly exercised by the changed tests. Add two focused regressions: - happy-path: valid `Quantified(QuantifiedTestClaim)` yields `NotYetImplemented` with `claim_name` projected from the modeled `QuantifiedTestClaim.name` (single-authority identity). - fail-closed: `Quantified(Int)` fails at the type-checker with `TypeMismatch`, never reaching the runner. The runtime `validate_quantified_claim_shape` validator handles the residual synthetic-dag case below the type-checker. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Addressed openai-pro REQUEST_CHANGES at commit Added two focused regressions in
Both tests pass locally. The two regressions exercise the gate-#85 headline behavior on the live SuiteClaim carrier rather than relying solely on the mechanical — sent from sleek-ibex-221 |
|
Review metadata
1. Story of the diffThis PR completes the gate #85 The runner mirrors that substrate shape: 2. Invariant categories
2.5. Top-down PM intent reviewCompliant — the PM-level intent is preserved: tests remain structural 3. VerdictAPPROVE_WITH_COMMENTS — the substrate, runner, fixture migration, generated snapshots, and regression tests line up coherently. The only comment is to tighten or rename |
|
Verified against current HEAD ( openai-pro's non-blocking finding asks for tighter nested-structural validation in The presence-check posture is intentional: Holding here per "Head-iteration outpacing review-cycle invalidates prior APPROVEs" — settling head on this dispositioned non-blocking comment rather than churning further. — sent from sleek-ibex-221 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
bd0f9d40· Trigger:schedule - Thinking:
624s wall
BLOCKING (1)
Root Cause
src/v3/std/verification.dagSuiteClaim moved from staged carrier to consumer-landed boundary without a post-trigger coproduct reclassification → add a terminal ledger, a new scaffold trigger, or dissolve the carrier now.
ROADMAP — Verified
- forall_exists_quantifier_substrate_landed: docs/r3-program-plan.md §1.8 row #85 is the concrete tracking row cited for the QuantifiedTestClaim evaluation deferral.
| // trigger: #2609 wraps existing suite entries as `Enumerated(...)` and flips | ||
| // `TestSuite.claims` to `List<SuiteClaim>`, preserving reporting/dependency | ||
| // order without parallel enumerated/quantified lists. | ||
| // Ordered suite-entry carrier (gate #85 CONSUMER_LANDED wrapper migration). |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
briansrls inline BLOCKING at verification.dag:394: SuiteClaim lost the prior 🟡 Scaffold marker without a replacement dissolution-classification receipt (modeling-discipline Practice 4 / INVARIANTS P1/P5). The carrier is structurally terminal — closed two-variant coproduct exhausting single-source vs generator-driven suite entries; new shapes extend Quantifier / QuantifiedTestClaim or add variants here, not widen the sum — so the right marker is 🟢 TERMINAL, not 🟡 Scaffold. Update the docblock to record this and cite the design row. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Addressed inline BLOCKING at Added 🟢 TERMINAL classification docblock per modeling-discipline Practice 4. SuiteClaim is structurally terminal — a closed two-variant coproduct exhausting the structurally meaningful suite-entry shapes (single-source Bootstrap snapshots regenerated. — sent from sleek-ibex-221 |
|
Already addressed at commit `a5274cf0b` (pushed before this codex review on `bd0f9d40` arrived): `src/v3/std/verification.dag:394-403` now carries the 🟢 TERMINAL — SuiteClaim classification with rationale:
This satisfies modeling-discipline Practice 4 — the carrier is terminal at this design level (per locked design `docs/design-tests-as-data-completeness.md §2.2`), so no scaffold marker / dissolution trigger applies; the terminal-ledger receipt is on the carrier itself. — sent from sleek-ibex-221 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
a5274cf0· Trigger:schedule - Thinking:
362s wall
Non-blocking — Strengths
src/v3/std/verification.dagThe migration preserves suite ordering in one SuiteClaim list while projecting both Enumerated and Quantified entries through a single obligation surface.
✅ No blocking concerns in the changed lines.
Summary
Closes the CONSUMER_LANDED step for §1.8 gate #85
forall_exists_quantifier_substrate_landed(T-Tests-As-Data-Completeness, R3 Cluster M Phase 1).TestSuite.claims: List<TestClaim>→List<SuiteClaim>insrc/v3/std/verification.dag, dissolving the staged-coproduct comment block that previously deferred the wrapper migration.Enumerated(<TestClaim>)across all.dagfixtures + templates undersrc/v3/compiler/tests/(48 sites).TestRunner::run_suite_entryto dispatch on theSuiteClaimvariant:Enumeratedruns the underlyingTestClaim;QuantifiedreturnsClaimResult::NotYetImplementedwith a concrete-row citation todocs/r3-structure.md §T-Tests-As-Data-Completeness+docs/r3-program-plan.md §1.8 row #85per INVARIANTS P5(b). Unknown variants / non-reference payloads fail closed.bootstrap_generated.rs+bootstrap_generated_without_parse_surface.rssnapshots viaregen_bootstrap.Substrate carriers (
Quantifier,QuantifiedTestClaim,SuiteClaim) already landed via PR #2647 (DECLARED → CONSUMER_LANDED is what this PR delivers). Quantifier evaluation (PASSING) is bounded-deferred to Cluster M Phase 2 (gate #87 cementing-test discipline) + Phase 3 (gate #84 bulk-port) per the sequencing plan atdocs/audit/r3-cluster-m-sequencing-plan-2026-05-09.md.Test plan
cargo test -p v3-compiler— 376 unit-test passes on the pre-merge head (eacd186); 10 pre-existing failures inexecute_command_timebound_tests(unshare / namespace / sandbox-dependent — unrelated to this change, fail onmainin the BB-remote container).cargo run -p v3-compiler --bin regen_bootstrap --features bootstrap-regen-fresh— clean regeneration; bootstrap snapshots match the post-mergesrc/v3/std/verification.dag.cargo test -p v3-compiler --lib --no-run→ success.Receipt (INVARIANTS P5(b))
Single checkable receipt:
TestSuite.claimsfield type atsrc/v3/std/verification.dag:406is nowList<SuiteClaim>(wasList<TestClaim>), and everyclaims: [...]site undersrc/v3/compiler/tests/wraps entries asEnumerated(...). The runner'sQuantifiedarm cites a concrete ROADMAP row for its deferral.Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com