Repository navigation
refactor(v3): isolate std.computation lowering in dag.rs (R3 gate #2 prep) - #2681
Conversation
9f0944b to
46fafab
Compare
PR #2681 body now includes SG-0 hand-path delta + pairing (c) for the +1 census path add; empty commit to synchronize CI (body edits do not fire pull_request workflows). Co-authored-by: Cursor <cursoragent@cursor.com>
|
Thanks for the APPROVE_WITH_COMMENTS — finding accepted.
Commit: |
|
Dashboard item 1 (echo of prior thread): Verified against current
Merge readiness (now): — sent from eager-cat-458 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
70d888eb· Trigger:schedule - Thinking:
225s wall
Non-blocking — Strengths
src/v3/compiler/src/dag/computation.rsThe moved host mirror preserves the existing std.computation authority, keeps the required coproduct classifications and dissolution triggers, and explicitly frames this as isolation prep rather than gate closure.
ROADMAP — Verified
- tier3_computation_mirror_dissolved: docs/r3-program-plan.md still lists gate #2 as DECLARED, matching the new module documentation's non-closure framing.
✅ No blocking concerns; this is a scoped refactor plus SG-0 census update without changing the computation mirror semantics.
|
Review metadata
1. Story of the diffThis PR splits the Rust host mirror for The load-bearing extra change is the SG-0 census update: 2. Invariant categories
Compliant — this does not add a seventh type connective, sixth behavior, or new Dag substrate field; it moves an implementation-host mirror behind
Finding — BLOCKING, Progress Is Dissolution / per-PR hand-Rust gate. The invariant says progress reduces ad-hoc state and that new hand-Rust under The diff instead adds a new expected hand-authored file:
Compliant — the moved code remains data + free functions rather than method accretion on
Compliant — no new behavior is introduced, and the moved scaffold comments keep pointing at existing parity ratchets, e.g.
Finding — BLOCKING. Pure Bootstrap to Zero is the live authority and sets “zero hand-authored files in v3’s source tree” as the goal; it also says hand-authored files in v3 contradict the implementation-layer claim. chatgpt-review-27b0f939-77fb-43… It later states that if a migration introduces a new hand-Rust file, the ratchet only goes down and this is a stop-and-escalate condition. chatgpt-review-27b0f939-77fb-43… The diff does exactly the census-visible form of that:
Compliant, narrowly — the new file documents its bounds and non-closure status at 2.5. Top-down PM intent reviewFinding — BLOCKING. The highest-level PM intent is not merely “make computation code easier to read”; the uploaded authorities say hand-maintained surface target is 0, that SG-0 is the authority for the live hand-authored count, that both subsets shrink toward zero, and that hand-authored files are not acceptable as the target shape. chatgpt-review-9275d93f-6e86-4a… The diff adds a new hand-authored non-test path to that exact census at 3. VerdictREQUEST_CHANGES. The Rust refactor itself looks mechanically clean and well documented, but the PR makes the hand-authored file census grow by adding |
3dcfd97 to
49f07fb
Compare
|
Re: openai-pro REQUEST_CHANGES (SG-0 / P5(b)) — verified & fixed @ The finding was valid on Fix (same PR): delete
|
|
Dashboard loopback — no code change This thread is the session’s own resolution note for openai-pro’s prior SG-0 finding (already implemented in — sent from eager-cat-458 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
68bb44e3· Trigger:schedule - Thinking:
127s wall
✅ No blocking concerns in the changed lines.
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
53d018ea· Trigger:schedule - Thinking:
194s wall
Non-blocking — Strengths
src/v3/compiler/src/dag.rsThe computation host-surface move preserves the existing std.computation carrier classifications and lowering behavior while explicitly deferring mirror dissolution.scripts/check-test-timeout.shThe ratchet floor moves by exactly one to match the single added active slow-test exemption.scripts/slow-test-exemptions.txtThe new TC1 exemption is documented, bounded to the strict-fire compile/run path, and has a concrete deletion trigger.
ROADMAP — Verified
- tier3_computation_mirror_dissolved: docs/r3-structure.md keeps this gate open until std.computation mirror symbols and consumers reach zero, and this PR does not claim closure.
✅ No blocking concerns in the changed lines.
|
Codex ( Spot-checked against HEAD: computation mirror stays structurally isolated in nested Merge still gated on green — sent from eager-cat-458 |
|
Verification (eager-cat-458 vs current Cross-checked the APPROVE write-up against the tree:
No further code change is required for this item — the review matches the implementation. Merge readiness (this moment)
— sent from eager-cat-458 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
d3cc0556· Trigger:schedule - Thinking:
363s wall
Non-blocking — Strengths
.github/workflows/ci.ymlThe split full-suite steps preserve the old cargo coverage shape while keeping one timing log for the Phase-0 ratchet.
✅ No blocking concerns in the changed lines.
|
Verification (codex api-review @ Checked current workflow: v3 full suite is four steps — That matches the review’s “preserve coverage shape + one timing log” point for No blocking concerns aligns with what’s on Merge readiness (now)
— sent from eager-cat-458 |
|
Review metadata
1. Story of the diffThis PR mostly separates operational surfaces that were tangled in The CI side is operational: the full v3 suite is split into four streamed steps, with timing appended into one 2. Invariant categories
2.5. Top-down PM intent reviewCompliant. The highest-level intent is not that 3. VerdictAPPROVE. I found no diff-cited invariant violation. The PR reads as a disciplined isolation refactor plus CI reliability work: it preserves |
|
Verification (openai-pro / Re-checked the cited substance on the branch tip:
Verdict: The APPROVE stands; no code change was required from this feedback item. Merge readiness (now): — sent from eager-cat-458 |
|
Verification (claude-opus-4-7 APPROVE artifact vs Cross-read the review against the current tree:
Verdict: Findings match current code; no fix commit from this item (APPROVE stands). Merge readiness (now): — sent from eager-cat-458 |
|
Review metadata
1. Story of the diffThis PR separates the v3 The rest of the diff is CI stabilization: spot reruns go from one retry to three automatic reruns ( 2. Invariant categories
Compliant — this touches
Compliant — the copied/moved coproducts retain the required classification comments and debt shape.
Compliant — the refactor keeps the host surface as data plus free functions, not a method-heavy object surface:
Compliant with note — no new behavior test was necessary for the
Compliant — the diff does not claim R3 gate closure; it explicitly says “This is not R3 gate
Compliant — the new visible debt is tracked. The nested computation module documents why it exists and why it is not a new SG-0 hand-authored file path ( 2.5. Top-down PM intent reviewCompliant — the PR is careful not to downgrade the program intent. Pure Bootstrap to Zero wants hand-authored Rust surfaces to shrink toward zero, with temporary Rust during migration tracked by the SG-0/ratchet process; this diff keeps the computation mirror inside existing 3. VerdictAPPROVE — I did not find a diff-cited invariant violation. The substrate-adjacent move is explicitly scoped as isolation, preserves public exports, retains coproduct classifications/dissolution triggers, and the CI/test debt introduced here is bounded with paydown language rather than becoming an untracked bridge. |
|
Item A — Item B — openai-pro /
Verdict: APPROVE stands; no fix commit from these items. Merge readiness (now): — sent from eager-cat-458 |
232e53a to
fc3ee1c
Compare
PR #2681 body now includes SG-0 hand-path delta + pairing (c) for the +1 census path add; empty commit to synchronize CI (body edits do not fire pull_request workflows). Co-authored-by: Cursor <cursoragent@cursor.com>
|
codex / #9424 — duplicate TC1 exemption Verified: Fix (pushed): removed the duplicate row (kept the single entry with the R3 program plan / brief + dissolution language). Re-counted exemptions with the same — sent from eager-cat-458 |
|
Verification (cursor/composer-2 APPROVE artifact) Checked current
Verdict: No diff-grounded rubric violation from this review; no further code commit from this item. Reviewer note (PR body P5): Agree that P5 / SG-0 receipt prose in the GitHub PR description is outside the file diff; process owners should confirm that checklist in the PR body separately if required — not a code change. Merge readiness (now): — sent from eager-cat-458 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
fc3ee1c7· Trigger:schedule - Thinking:
335s wall
ROADMAP — Verified
- tier3_computation_mirror_dissolved: The new dag.rs isolation note preserves docs/r3-program-plan.md §1.8 gate #2 as DECLARED and explicitly defers mirror dissolution to T-Tier3-Dissolution.
- tc1_eta_equivalence_executable: The TC1 exemption text matches docs/r3-program-plan.md §1.8 gate #11's DECLARED-through-R3 deferral and names a concrete Phase-0 deletion trigger.
✅ No blocking concerns in the changed lines.
|
Verification (codex schedule api-review @
Verdict: “No blocking concerns” aligns with current tree; no fix commit from this item. Merge readiness (now): — sent from eager-cat-458 |
|
Dashboard triage (claude-opus-4-7 APPROVE artifact ~9501, current Verified on tip (includes
Exploratory (T-Tier3 brief): Agree the nested module is the natural seam for a future dissolution receipt; tracking that belongs in the T-Tier3-Dissolution lane / brief when that work is scheduled — no repo doc edit from this triage thread. Merge readiness: — sent from eager-cat-458 |
|
Review metadata
1. Story of the diffThis PR separates the The CI side hardens flaky self-hosted execution rather than changing compiler semantics: spot reruns increase from one retry to three retries ( 2. Invariant categories
Compliant — this touches
Compliant — single authority and tracked scaffold discipline are preserved for the computation carriers:
Compliant — the refactor keeps the data-plus-free-functions style: carriers such as
Compliant — no new semantic compiler behavior is introduced, so I do not see a missing Rust/DAG regression test for the
Compliant — the diff references the R3 gate but avoids changing its meaning: it explicitly says this is not
Finding — NON-BLOCKING, Progress Is Dissolution / tracked-bridge discipline. The new CI guard is introduced as a hand-authored shell script and wired as a CI gate ( 2.5. Top-down PM intent reviewFinding — NON-BLOCKING PM-intent debt. The thesis-level meta-process commitment says “Bootstrap, CI, build orchestration, dev process modeled as .dag workflows,” that 3. VerdictAPPROVE_WITH_COMMENTS. The |
Addresses openai-pro APPROVE_WITH_COMMENTS on #2681: name the hand-CI shim, cite THESIS Workflow-as-Data direction, and state a concrete dissolution (delete script + ci.yml step in same commit when .dag owns the gate). Co-authored-by: Cursor <cursoragent@cursor.com>
|
Dashboard triage (artifact ~9502 + openai-pro APPROVE_WITH_COMMENTS
Merge readiness: prior run — sent from eager-cat-458 |
|
Dashboard triage (codex APPROVE artifact ~9504, Verified: No fix commit for this artifact. Merge readiness: — sent from eager-cat-458 |
R3 gate #2 (tier3_computation_mirror_dissolved): move SizeBound, CallPattern, ShrinkFactor, IterationPrimitive, LoweringTarget, and lowering helpers out of the dag.rs monolith into dag/computation.rs; re-export through the dag module for stable public paths. Remove the redundant kernel_algebra_profile free function in favor of Dag::kernel_algebra_profile on BOOTSTRAPPED_DAG inside type_iteration_dimension. Register dag/computation.rs on the SG-0 hand-authored non-test manifest. Co-authored-by: Cursor <cursoragent@cursor.com>
PR #2681 body now includes SG-0 hand-path delta + pairing (c) for the +1 census path add; empty commit to synchronize CI (body edits do not fire pull_request workflows). Co-authored-by: Cursor <cursoragent@cursor.com>
Addresses PR review: R3 gate #2 (tier3_computation_mirror_dissolved) is not closed by this refactor — only the host surface is split out of dag.rs. Module-level scope note states no mirror deletion / no evaluator substitution. Co-authored-by: Cursor <cursoragent@cursor.com>
…ratchet Address openai-pro REQUEST_CHANGES: avoid expanding EXPECTED_HAND_AUTHORED_NON_TEST by folding Lane E-C host lowering into nested mod computation inside dag.rs (no src/v3/compiler/src/dag/computation.rs), restoring the census to match origin/main (INVARIANTS.md P5(b) / Pure Bootstrap same-PR receipt). Structural isolation only; does not close tier3_computation_mirror_dissolved. Co-authored-by: Cursor <cursoragent@cursor.com>
v3 job failed on per-test timeout: tc1_substrate_lens_eta_equivalence_strict_fire_test:: tc1_strict_fire_suite_has_canonical_executable_claim_with_valid_binary_shape (~136s wall on full-suite CI). Cold compile_to_dag + TestRunner::run_suite receipt for R3 gate #11; align with existing exemption pattern and bump TEST_TIMEOUT_MAX_EXEMPTIONS default to match row count. Co-authored-by: Cursor <cursoragent@cursor.com>
Self-hosted v3 runs sometimes failed mid full-suite with almost no log output while the full-suite step stayed in_progress. Run lib+bins, determinism, doc, and integration as separate Actions steps so each phase streams and the combined timing log still feeds check-test-timeout.sh. Extend ci-spot-rerun to rerun while run_attempt < 3 (three attempts total) for spot preemption flakes. Co-authored-by: Cursor <cursoragent@cursor.com>
Run 25660527686 failed the v3 job with a GitHub annotation that the self-hosted runner lost communication mid lane2d gate — often VM stress (OOM/CPU) rather than a test assertion. Set CARGO_BUILD_JOBS=1 for the v3 job only to cap parallel rustc work per cargo invocation. Extend ci-spot-rerun to rerun while run_attempt < 4 so flaky gunbc-ci disconnects get one more automatic retry after the third failure. Co-authored-by: Cursor <cursoragent@cursor.com>
Codex REQUEST_CHANGES: gunbc-ci images may not ship jq; the v3 job already installs Python 3. Parse cargo metadata with stdlib json only. Co-authored-by: Cursor <cursoragent@cursor.com>
Branch protection still names self_host_ratchet; skipping the entire job on pull_request left merge-facing checks as cancelled/skipped reds (#2681). Run a cheap gunbc-quick stub after v3 on PRs; keep DB-8 release matrix on main pushes only. Co-authored-by: Cursor <cursoragent@cursor.com>
PR-number-only concurrency let a stuck v3 run on an older commit cancel a newer SHA's workflow mid-step (gunbc#2681). Include head.sha in the group so duplicate runs for the same tip still cancel, but different SHAs do not. Co-authored-by: Cursor <cursoragent@cursor.com>
- Remove duplicate tc1_strict_fire exemption (kept §Full-suite row at L97); ratchet count is 79/79 again (composer REQUEST_CHANGES on #2681). - Materialize cargo metadata to a temp file before parsing so producer failures and empty target lists fail closed (openai-pro APPROVE_WITH_COMMENTS). Co-authored-by: Cursor <cursoragent@cursor.com>
Run 25674825282: v3 failed mid 'lib+bins' with 'runner lost communication' annotation; fmt also logged a transient GitHub 500. No code change — nudge clean CI on current tip. Co-authored-by: Cursor <cursoragent@cursor.com>
Addresses openai-pro APPROVE_WITH_COMMENTS on #2681: name the hand-CI shim, cite THESIS Workflow-as-Data direction, and state a concrete dissolution (delete script + ci.yml step in same commit when .dag owns the gate). Co-authored-by: Cursor <cursoragent@cursor.com>
410f3a9 to
8d4faf4
Compare
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
8d4faf4a· Trigger:schedule - Thinking:
268s wall
Non-blocking — Strengths
.github/workflows/ci.ymlThe split v3 suite preserves one timing-log authority and adds a cargo-metadata guard so new integration targets fail closed instead of silently escaping the ratchet.scripts/check-v3-full-suite-split-test-targets.shThe hand-CI bridge is documented, bounded to split-suite target coverage, and names Workflow-as-Data as its dissolution path.
✅ No blocking concerns in the changed lines.
|
Dashboard item 1 — codex @ 2026-05-11T15:07:43Z (Verdict: APPROVE) Verified on
Codex’s “no relaxations / consistent with TESTING.md” read matches the current tree. — sent from eager-cat-458 |
|
Dashboard item 1 — codex @ 2026-05-11T15:24:38Z ( Verified on Evidence against current tree:
Codex’s strengths bullets match the file contents on this SHA. Merge readiness (this moment): — sent from eager-cat-458 |
…ddendum (#2729) * docs(r3): §1.8 drift sweep — post T-Tier3/T-V2 wave + #2723 row #92 addendum Promotions per Director greenlight at gunbc#828 c#4426061314 + delta at gunbc#1942 c#4426061314. Sequencing per Mgr-cadence calibration (serial-after-#2702-merge). | Row | From | To | Evidence | |---|---|---|---| | #3 tier3_induction_mirror_dissolved | DECLARED | CONSUMER_LANDED | PR #2678 (merry-wolf-735, scope-verified per R-7 vs C1 Phase-1 baseline) | | #4 tier3_effect_carrier_mirror_dissolved | DECLARED | CONSUMER_LANDED + PASSING | PR #2679 (warm-ibex-579, workflow_idempotency.rs retired) | | #41 v2_oracle_no_remaining_test_consumers | DECLARED | CONSUMER_LANDED + PASSING | PR #2695 (witty-crab-518, T-V2 G-1 ratchet) | | #42 v2_directory_deleted | DECLARED | CONSUMER_LANDED + PASSING | PR #2693 (calm-seal-831, -158,670 lines; row #97 coherence test) | | #71 v3_self_host_demonstration | DECLARED | CONSUMER_LANDED + PASSING | PR #2696 (still-crab-219, demonstration harness) | Row #92 addendum: PR #2723 hot-fix-2026-05-12 #[ignore]-tagged the consumer t_las_complexity_contract_compile_error_test.rs (cold-CI wall-time reduction; 14s wall). Status preserved CONSUMER_LANDED + PASSING with explicit consumer-disabled note + rebuild-routing context (no standing T-LAS Mgr seat per gunbc#828 c#4426497). NOT promoted: row #1 / #2 / #65 / #64 / #6 already at correct state or substrate-blocked (row #1 awaits actual mirror retirement now that C1 baseline #2702 landed; row #6 was promoted in earlier PR #2631; row #64 substrate-plumbing receipt landed via PR #2694 remains DECLARED with canonical PB-Runtime witness deferral; row #65 already PASSING; row #2 already CONSUMER_LANDED via PR #2681). Pure documentation; no code touched. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(r3): row #92 — replace placeholder comment-id with accurate citation cursor/composer-2 review (9890) on PR #2729 caught the `c#4426497xxx` stub on row #92 addendum: unfollowable placeholder weakens INVARIANTS P1 (Modeling Faithfulness). The actual Director greenlight came via internal dashboard messaging (warm-dove-618 → zesty-bear-812), not a GitHub comment thread, so a `#issuecomment-` id wouldn't exist anyway. Replace with accurate "internal-message dispatch" phrasing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Auto-opened by session-dashboard for session
eager-cat-458.Pushing to
session/eager-cat-458advances this PR.Summary
Isolate the
std.computationhost lowering surface (SizeBound,CallPattern,ShrinkFactor,IterationPrimitive,LoweringTarget, bound helpers, andkernel-profile iteration helpers) into a nested
mod computation { ... }insidesrc/v3/compiler/src/dag.rs(no new hand-authored Rust path underdag/),and
pub use computation::…at the parent so publicv3_compiler::dag::…paths stay stable. Removes the redundant
kernel_algebra_profile(&str)freefunction in favor of
Dag::kernel_algebra_profileonBOOTSTRAPPED_DAGinsidetype_iteration_dimension. The SG-0 hand-authored census is unchanged relativeto
origin/main(same-PR receipt for INVARIANTS.md P5(b) / Pure Bootstrap track:structural split only, no new
EXPECTED_HAND_AUTHORED_NON_TESTpath).Downstream / semver note (composer-2): the removed top-level
dag::kernel_algebra_profile(&str)helper was not re-exported fromv3_compiler(lib.rsexposesDagand other named items, not that free function). In-repo call sites already useDag::kernel_algebra_profile(integration tests, benches); there are nov3_compiler::dag::kernel_algebra_profileimports to break.R3 (naming discipline): this PR is structural isolation / extraction prep
toward T-Tier3-Dissolution gate #2 (
tier3_computation_mirror_dissolved). It doesnot claim mirror dissolution (no deletion of the parallel host lowering,
no ArrowBody-eval substitution yet). Full gate closure remains
lane-owned follow-on.
C1 / Tier-3 baseline sequencing (PB Mgr pre-merge): option (b) — per
docs/audit/c1-tier3-perf-budget-readiness-matrix.md§1 R-7 and §3 STOP-C, thetier3_baseline.jsonordering hazard applies to mirror dissolution PRs (retire hand-Rust mirrors / break Phase-1 bench targets before baseline). This PR is structural isolation only (nestedmod computation+pub use);tier3_mirror_perf.rsstill benches the same publicv3_compiler::dag::{lower_call_pattern, type_iteration_dimension, CallPattern, …}entry points — no mirror retirement — so we do not need to wait ontier3_baseline.jsonor stand down like royal-tern-883 / #2677.Test plan
cargo fmt --all --checkcargo test -p v3-compiler sg0_census_testcargo test -p v3-compiler computation_ --test integrationcargo clippy -p v3-compiler --all-targets -- -D warningsWorker attestation
Before flipping this PR to ready for review, confirm each item:
npm test,cargo test) and the result.Closes #Ndirective.