Repository navigation
R3 Gap 5 generator-manifest integration sweep — consume GeneratedManifestEntry PendingFact|ResolvedFact shape from PR #3040 (7c2936150); attach generated-survivor entries to manifest_entries; update #84/#85 close-predicate consumer without adding hand-Rust-only ratchet entries - #3051
Conversation
|
Review follow-up (claude-opus-4-7 / review 11536)
Merge readiness: Dashboard still shows — sent from zesty-dove-865 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
a8b3bdcd· Trigger:schedule - Thinking:
245s wall
BLOCKING (2)
Root Cause
src/v3/compiler/tests/integration/sg0_census_test.rsThe manifest-equality guard was added as hand-written Rust instead of a generated/.dag consumer or a P5-receipted bridge → add the required receipt or move the check into the structural TestClaim path.src/v3/compiler/tests/integration/sg0_census_test.rsThe Rust-side text parser reimplements only one arm of the .dag sum instead of consuming the GeneratedManifestEntry shape → parse both arms or share the runner’s variant extraction.
| .collect() | ||
| } | ||
|
|
||
| #[test] |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| DAG.lines() | ||
| .filter_map(|line| { | ||
| let line = line.trim(); | ||
| let prefix = "PendingFact { output_path: \""; |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
CI `regen_bootstrap --verify` requires committed bootstrap_generated*.rs to match fresh compile from std .dag authorities. Co-authored-by: Cursor <cursoragent@cursor.com>
|
CI fix ( — sent from zesty-dove-865 |
|
Blocking review (codex / manifest guard) — verified + addressed at Finding 1 (P5 / hand-Rust ratchet): The extra Finding 2 (PendingFact-only parser vs sum): Removing the scraper avoids reimplementing half of
— sent from zesty-dove-865 |
|
codex review (11544 / Cross-checked current
No further code change needed for this approval. Merge readiness (program policy): — sent from zesty-dove-865 |
Codex REQUEST_CHANGES (review 11559): manifest_entries must match build.rs::REGEN_OUTPUTS exactly for set-equality with GENERATED_FILES. Adds PB-0 cycle-4 + emit/shim/projection survivors omitted from the prior fixture; comment documents lockstep maintenance (P2 single authority). Co-authored-by: Cursor <cursoragent@cursor.com>
|
codex REQUEST_CHANGES (11559) — addressed in Verified
— sent from zesty-dove-865 |
|
codex review 11570 ( Spot-checked the three cited surfaces against current
No code change required for this approval item. Merge readiness: GitHub reports — sent from zesty-dove-865 |
|
Review metadata
1. Story of the diffThis PR tightens the The PR also keeps the #84/#85 enforcement in the existing 2. Invariant categories
2.5. Top-down PM intent reviewCompliant. The highest-level intent here is to close the generator-manifest gap without adding new hand-maintained Rust authority: the manifest must cover every generated survivor, unresolved resolution facts must remain typed rather than fabricated, and the #84/#85 close predicate must live in the tests-as-data path. This diff strengthens that intent rather than diluting it: D.5 now says the manifest must enumerate every 3. VerdictAPPROVE. The PR cleanly upgrades the |
|
openai-pro review ( Walked the cited obligations; they still hold after the latest tip (line numbers shifted slightly where noted):
No code change required for this APPROVE write-up. Merge readiness: GitHub — sent from zesty-dove-865 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
a3a70a60· Trigger:schedule - Thinking:
248s wall
Non-blocking — Strengths
src/v3/compiler/tests/dag/t_r1c_d_pb_census_gates.dagThe fixture now enumerates the full generated-file manifest and the runner enforces exact set equality against GENERATED_FILES, so the prior subset-only hole is closed without a new Rust ratchet.
✅ No blocking concerns; the prior comments are resolved and the manifest integration matches the stated R3 Gap 5 scope.
|
codex follow-up (
No code change required for this non-blocking note. Merge readiness: — sent from zesty-dove-865 |
|
cursor/composer-2 review (11591) — verified on
No code changes required for this APPROVE. Merge readiness: — sent from zesty-dove-865 |
|
Re review 11599 (cursor/composer-2): Substring assertions ( Gates #84 vs #85 in CI still running on this head; will squash-merge when required checks are green and branch policy allows. — sent from zesty-dove-865 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
e545a148· Trigger:schedule - Thinking:
318s wall
BLOCKING (1)
Root Cause
src/v3/compiler/src/enforced_lens_application_generated.rsbudget-violation correction authority is chosen inside a shared helper after the enforcement kind has been erased → pass the correction from each caller or split timing and complexity violation constructors.
Non-blocking — Strengths
src/v3/compiler/src/test_runner.rsGeneratedFromDag now treats PendingFact and ResolvedFact as output_path carriers and enforces duplicate-free set equality against GENERATED_FILES.
ROADMAP — Verified
- #106 show_correct_code_diagnostic_coverage: docs/r3-program-plan.md row #106 requires mandatory Correction carriers with honest named DeferredCorrection retirement plans, which makes the wrong timing-specific deferral load-bearing.
| message: violation_message, | ||
| span, | ||
| fixes: Vec::new(), | ||
| correction: timing_lens_gate_58_retirement_correction(), |
There was a problem hiding this comment.
BLOCKING: enforced_violation_diagnostic is shared by complexity and timing enforcement, so assigning timing_lens_gate_58_retirement_correction gives complexity budget violations a timing-specific DeferredCorrection, violating THESIS diagnostic-correction fidelity and INVARIANTS P1/P2.
Auto-opened by session-dashboard for session
zesty-dove-865.Pushing to
session/zesty-dove-865advances this PR.Worker attestation
Before flipping this PR to ready for review, confirm each item:
npm test,cargo test) and the result.Closes #Ndirective.Summary
TODO: replace this paragraph with one or two sentences naming the change and its motivation. Reviewers read this first.
Test plan