Repository navigation
feat(v3): wire PB census TestPredicate shapes (R1C-A Sub-B) - #939
Conversation
R1 Closure (fierce-raven-707) — review on draft #939What looks right (Sub-B / shape vocabulary)
Must-fix before undraft / merge
Nice-to-have (non-blocking)
R1 Closure dispositionTreat #939 as the Sub-B ship candidate pending #938 integration + green — R1 Closure Manager |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
9c21a7a5· Trigger:schedule - Thinking:
249s wall
BLOCKING (1)
Root Cause
src/v3/std/verification.dagPB census sub-vocabularies were added as name-bearing records instead of closed structural facts → replace the name:String records with closed .dag shapes and have the runner match those constructors, not strings.
Non-blocking — Improvements (fix in-PR if easy, else defer to roadmap)
src/v3/std/verification.dagLine 179 says "six PB census gates" but this PR adds and tests five predicate shapes; reconcile the count so R1C-D does not chase a phantom gate or miss an intended one.
| type Path = String | ||
| type SnapshotRef = String | ||
|
|
||
| type CensusSubsetPredicate { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Fixed on current head 126119bef: removed the free-string CensusSubsetPredicate / ConsolidationRatchet records from std.verification, added typed marker declarations (lens_producer_files_subset_predicate, compiler_std_positive_set_ratchet), and changed CensusSubsetCount / RatchetZero to carry DeclarationRef edges. The runner now validates both the referenced declaration name and its marker type before returning the R1C-D NYI. Also refreshed bootstrap snapshots and the parse corpus manifest.
Local verification:
cargo fmt --all --checkcargo run -p v3-compiler --features bootstrap-regen-fresh --bin regen_bootstrap -- --verifycargo test -p v3-compiler --test integration test_runner_dispatches_pb_census_predicate_shapes -- --nocapturecargo test -p v3-compiler --test integration test_runner_dispatches_mock_backed_invariant_claim -- --nocapturecargo test -p v3-compiler --test integration test_runner_fails_closed_on_requires_edges -- --nocapturecargo test -p v3-compiler --test integration bootstrap_loads_verification_authority_types -- --nocapturecargo test -p v3-compiler --test integration handwritten_parse_snapshot_matches_manifest -- --nocapturecargo test -p v3-compiler --test integration -- --nocapture
|
Addressed review + CI on latest head
Local verification after the fix:
|
|
Review metadata
Findings:
Verdict: REQUEST_CHANGES. The dispatch wiring is otherwise narrow and fail-closed, but the new substrate shapes introduce open string authority for finite categories. |
|
Review metadata
Findings None. The diff wires five Verdict APPROVE — Narrow, coherent R1C-A wiring; no rubric violation in the reviewed diff that warrants a change request. Exploratory (optional)
|
R1 Closure — v3 failure triage (run
|
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
8b4fa4e4· Trigger:schedule - Thinking:
304s wall
BLOCKING (1)
Root Cause
src/v3/std/verification.dagPB predicate vocabulary was scoped as fresh local String aliases → import/reuse std.types carriers and model closed census-list constants as typed vocabulary before R1C-D fixtures consume them.
| // R1C-A / T-TestGen PB-census predicate shape vocabulary. These are narrow | ||
| // bootstrap-local carriers for R1C-D's census-as-TestClaim fixtures; the Rust | ||
| // runner still owns evaluation against the live census authorities. | ||
| type Symbol = String |
There was a problem hiding this comment.
BLOCKING: Symbol/Path/SnapshotRef reintroduce raw String carriers even though std.types already provides NonEmptyStr/FilePath, leaving PB fixture constants typo-representable and violating M9/API-level enforcement.
…sion/zesty-hawk-239 # Conflicts: # src/v3/compiler/src/bootstrap_generated.rs # src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs # src/v3/compiler/tests/integration/parse_corpus_manifest.txt
|
Pushed #939 support fix at
Local verification: |
|
Review metadata
Read the context rubric and the non-generated part of the diff: Findings: None. Nothing in the reviewed hunk concretely breaks the cited docs:
The fixture using Verdict: APPROVE — The diff is narrowly scoped: extends |
|
Follow-up pushed at The first artifact patch reused the
|
|
Review metadata
Verdict: APPROVE — the diff is narrowly scoped to PB census predicate schema wiring, runner dispatch to explicit I did not run the test suite; this was a code-review pass over |
|
Second follow-up pushed at The previous run got through the full integration suite and manifest check; remaining failure was
|
|
Review metadata
Findings: None. The diff wires five Verdict: APPROVE — Scoped R1C-A wiring plus regen churn; behavior is explicit, fail-closed at the shape boundary, and the bridge names its dissolution lane (R1C-D) in both comments and |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
f9608274· Trigger:schedule - Thinking:
54s wall
|
Review metadata
Findings: None blocking. Minor observations (non-blocking):
Verdict: APPROVE — narrowly-scoped schema/shape wiring for the five PB census predicates with deferred evaluation, explicit NYI receipts, and correct ratchet refreshes (bootstrap mirrors, parse manifest, predicate-shape ratchet test). Suggest a follow-up to lift |
Summary
src/v3/std/verification.dag.test_runnerdispatch arms forCensusBoundCheck,CensusSubsetCount,FixedPointConverges,RatchetZero, andGeneratedFromDag.TestClaims compile and route to those explicit dispatch arms.Authority / Scope
docs/briefs/r1c-a-t-testgen-schema-extensions-worker.md, Sub-deliverable B.docs/briefs/r1c-d-t-pb-census-as-dag-worker.md) authors the six census-as-.dagfixtures after this lands.Non-goals
Generated Files
src/v3/compiler/src/bootstrap_generated.rsandsrc/v3/compiler/src/bootstrap_generated_without_parse_surface.rswere regenerated from the std verification schema change.src/v3/compiler/tests/integration/parse_corpus_manifest.txtwas refreshed viarefresh_handwritten_parse_snapshot_manifest.Verification
cargo fmt --all --checkcargo run -p v3-compiler --features bootstrap-regen-fresh --bin regen_bootstrap -- --verifycargo test -p v3-compiler --test integration test_runner_dispatches_pb_census_predicate_shapes -- --nocapturecargo test -p v3-compiler --test integration bootstrap_loads_verification_authority_types -- --nocapturecargo test -p v3-compiler --test integration handwritten_parse_snapshot_matches_manifest -- --nocapturecargo test -p v3-compiler --test integration -- --nocapture