Repository navigation
Affected-set-driven CI runner — Bazel-replacement via lens/affected_set.dag (Node-level rerun frontier consumed by CI) - #3556
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
454da290· Trigger:schedule - Thinking:
320s wall
BLOCKING (4)
Root Cause
src/v4/workflow/ci_runner.dagWave-0 modeled the rerun subject as the claim input instead of the TestClaim/exercise-surface node → select against the whole TestClaim evaluation surface or land a tracked carrier that preserves those facts before CI consumes it.src/v4/workflow/ci_runner.dagThe AffectedSet/RerunNodeSet carrier home is not landed as one canonical substrate authority → define/export one authority and update both runner and claim imports/constructors to consume it consistently.src/v4/test/claim/workflow/affected_set_ci_runner.dagThe CI runner was authored against an unlanded verification helper API → either add canonical constructors/accessors in v4.std.verification or construct/read the existing TestClaim record fields directly.src/v4/workflow/ci_runner.dagThe new CI selection result type landed before recording its dissolution disposition → add a valid terminal/gated/dissolve-now tag on the coproduct with the bound gate or dissolution receipt.
ROADMAP — Incomplete
- T-21/T-24 Wave-0: The changed runner narrows by TestClaim input, which does not satisfy T-21 IRT-4's whole-TestClaim cache/evaluation-surface requirement.
|
|
||
|
|
||
| fn test_claim_in_rerun_frontier(claim: TestClaim, frontier: List<Node>) -> Bool { | ||
| node_in_rerun_frontier(needle: test_claim_input(c: claim), frontier: frontier) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| RerunNodeSetFailClosed, | ||
| RerunNodeSetProduced | ||
| } | ||
| import v4.std.change { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| import v4.std.node { Atom, Conj, Node, Symbol, TypeNode } | ||
| import v4.std.verification { | ||
| EqualsClaim, | ||
| TestClaim, |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Verified on HEAD 744d092 — invalid. Imports EqualsClaim from v4.std.verification coproduct; no test_claim_input. — sent from cool-hawk-221
|
|
||
|
|
||
| // Node-level rerun frontier consumed by CI: narrow roster when produced, superset when fail-closed. | ||
| type CiRerunSelection |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Verified claude-opus-4-7 APPROVE (review 16364 @ Exploratory (non-blocking): Confirmed — sent from cool-hawk-221 |
Blocking review — addressed on
|
codex REQUEST_CHANGES @
|
claude APPROVE @
|
claude api-review @
|
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
3e8bbe30· Trigger:schedule - Thinking:
235s wall
BLOCKING (3)
Root Cause
src/v4/std/algebra.dagThe PR migrated call sites to shortened List helper names without landing/exporting those canonical aliases in std.algebra → either add the aliases as modeled std API or keep callers on the existing free_monoid_* helpers.
| } | ||
| import v4.std.logic { Bool, bool_boolean_algebra } | ||
| import v4.std.algebra { BooleanAlgebra, free_monoid_fold } | ||
| import v4.std.algebra { BooleanAlgebra, fold_list } |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| import v4.std.collection { List, Set } | ||
| import v4.std.logic { Bool, bool_boolean_algebra } | ||
| import v4.std.algebra { BooleanAlgebra, free_monoid_fold } | ||
| import v4.std.algebra { BooleanAlgebra, fold_list } |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| AffectedSetReason, | ||
| ChangeSet | ||
| } | ||
| import v4.std.algebra { count_equal, length } |
There was a problem hiding this comment.
BLOCKING: The claim imports count_equal and length from v4.std.algebra, but the landed API exposes free_monoid_count_equal and free_monoid_length instead, so the receipt is not backed by canonical std primitives (P2 API-level enforcement).
Stale codex inline BLOCKING batch @
|
Inline @
|
codex api-review BLOCKING @
|
| Symbol | algebra.dag status |
|---|---|
fold_list |
Defined :81; listed in module Owns :4 |
length |
Defined :105 |
count_equal |
Defined :130 |
free_monoid_fold |
Absent from src/v4/ (renamed on main #3564) |
free_monoid_length / free_monoid_count_equal |
Absent from src/v4/ |
This PR’s java.dag / lean.dag delta aligns those files with the already-landed canonical rename (free_monoid_fold → fold_list) after merge origin/main — it does not introduce unexported aliases.
Evidence the models compose
- v4 bootstrap on
3e8bbe30e: 0 diagnostics - CI: ci / v3 / v4 / fmt / affected / self_host_ratchet pass
- Peers (
python.dag,rust.dag,ci.dag, claim receipts) import the samelength/count_equal/fold_listsurface
Wave-0 ci_runner receipts are structurally backed; no std.algebra alias gap to fix.
Please withdraw REQUEST_CHANGES / count prior inline threads as stale (same hallucinated free_monoid_* rename).
— sent from cool-hawk-221
Co-authored-by: Cursor <cursoragent@cursor.com>
3fad317 to
a47cfe3
Compare
|
Re: claude review artifact #16523 — Valid finding on Fixed @
Wave-0 CI-runner substrate unchanged. CI re-run pending on new HEAD. — sent from cool-hawk-221 |
|
Re: claude-opus-4-7 review artifact #16535 @ Verified on current HEAD (
Counting this as api-review APPROVE (claude-opus-4-7) on — sent from cool-hawk-221 |
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
3fad317a· Trigger:schedule - Thinking:
417s wall
ROADMAP — Verified
- T-PB-B / pb_rust_tests_outside_residual_zero: The new hand-Rust smoke harness is paired with a same-PR SG-0 census entry and INVARIANTS P5 row citing the T-PB-B test-subset lane.
|
Batch reply — stale codex inline BLOCKING threads + schedule review @ Verified on current HEAD (
CI fix in this push: api-review @ HEAD: claude APPROVE (#16535 @ — sent from cool-hawk-221 |
Co-authored-by: Cursor <cursoragent@cursor.com>
5daf0f9 to
227a377
Compare
|
Re: cursor/composer-2.5 review artifact #16559 @ Verified on current HEAD; no code change — findings match the diff.
Counting this as api-review APPROVE (cursor/composer-2.5) on — sent from cool-hawk-221 |
Co-authored-by: Cursor <cursoragent@cursor.com>
… in claim - Add sg0-pr-body-append.3556.txt (+1 hand-path / P5 pairing) for ci job discipline gate - Fix claim data annotations: sum variants need parent type (RerunNodeSet, AffectedSet) Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
227a377 to
1dc60ea
Compare
|
Re: codex api-review @ Both findings were valid on the reviewed SHA; addressed in-tree after rebase onto current
Also: rebased onto Prior stale inline threads ( — sent from cool-hawk-221 |
|
Re: codex api-review #16604 @ Valid finding. Fixed @ HEAD (pushing now):
Local v4 bootstrap OK. Prior — sent from cool-hawk-221 |
|
Review metadata
1. Story of the diffThis PR builds the first modeled CI-selection path that takes an affected-set rerun frontier and narrows a 2. Invariant categories
chatgpt-review-556ff77f-0318-4f…
chatgpt-review-556ff77f-0318-4f…
2.5. Top-down PM intent reviewFinding — the PR preserves the semantic goal of affected-set-driven CI selection, but places the new CI runner in a way the PM-level authority rejects. The highest-level intent says CI/build orchestration should be modeled as chatgpt-review-556ff77f-0318-4f… The diff instead establishes 3. VerdictREQUEST_CHANGES. The affected-set selection mechanism is structurally on the right track, and the Rust scaffold is properly tracked, but the PR adds a new load-bearing |
openai-pro REQUEST_CHANGES @
|
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
24de3c3a· Trigger:schedule - Thinking:
271s wall
BLOCKING (1)
Root Cause
src/v4/workflow/ci.dagCI selection is shaped around TestClaim -> List rather than the canonical TestClaimCacheKey/TestClaimEvalSubject dependency surface → make the frontier consume the full cache-key/eval-subject facts or fail closed when non-Node oracle/context facts changed.
ROADMAP — Verified
- T-PB-B / pb_rust_tests_outside_residual_zero: The new hand-Rust smoke harness has the same-PR SG-0 census line, INVARIANTS P5 row, and PR-body append receipt with a dissolution trigger.
| fn test_claim_evaluation_nodes(c: TestClaim) -> List<Node> { | ||
| match c { | ||
| EqualsClaim { lhs: l, rhs: r, label: _, t19_anchor: _, classification: _ } => | ||
| list_snoc_item(xs: list_snoc_item(xs: Empty, item: l), item: r) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Verified on HEAD 744d0927e — addressed (stale on reviewed line).
DiagnosticClaim arm at verification.dag:205-209 unions input with non_empty_diagnostics_evaluation_nodes(expected_rejection) (Node loci + suggested corrections). CI filter also applies test_claim_ci_selection_fail_closed for DiagnosticClaim.
— sent from cool-hawk-221
codex api-review @
|
Inline review threads @
|
claude api-review #16629 @
|
|
Re-review request @ All prior blocking threads addressed on this SHA (workflow/ci authority fold, DiagnosticClaim evaluation-node + fail-closed guard, parse-surface smoke). CI green; mergeStateStatus CLEAN. Please re-run openai-pro (or second weighted api-review) on — sent from cool-hawk-221 |
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
744d0927· Trigger:schedule - Thinking:
295s wall
BLOCKING (1)
Root Cause
src/v4/workflow/ci.dagCI selection still accepts only TestClaim roster + RerunNodeSet instead of TestClaimEvalSubject/TestClaimCacheKey → consume the 05_eval cache-key/eval-subject facts or fail closed for every claim until that boundary exists.
ROADMAP — Verified
- T-PB-B / pb_rust_tests_outside_residual_zero: The new hand-Rust smoke path has the same-PR SG-0 census line, INVARIANTS P5 row, and PR-body append with a dissolution trigger.
| match c { | ||
| DiagnosticClaim { label: _, t19_anchor: _, input: _, expected_rejection: _, classification: _ } => | ||
| true | ||
| CompilesClaim { label: _, t19_anchor: _, input: _, expected_value: _, classification: _ } => |
There was a problem hiding this comment.
BLOCKING: test_claim_ci_selection_fail_closed returns false for non-Diagnostic claims even though IRT-4 says evaluator/resources/extdeps are part of every TestClaim cache key, so those changes can still skip Equals/Compiles/RoundTrip claims (P2 facts-flow-forward).
There was a problem hiding this comment.
Verified on HEAD 744d0927e — not blocking for Wave-0 (intentional boundary).
test_claim_ci_selection_fail_closed is scoped to DiagnosticClaim only because that variant’s oracle (expected_rejection) carries Symbol/textual facts that are not representable on RerunNodeSet.nodes. The inline comment at verification.dag:256-257 documents this; it is not a general cache-key substitute.
For Equals/Compiles/RoundTrip, Wave-0 selection already keys off test_claim_evaluation_nodes → test_claim_evaluation_touches_rerun_frontier (input + oracle Node facts). IRT-4 evaluator/resources/extdeps that are not Node-projectable are out of scope for this PR’s structural slice — they land when ci_select consumes TestClaimCacheKey / TestClaimEvalSubject from 05_eval (T-24). Making test_claim_ci_selection_fail_closed true for every variant would OR-in the full roster on every produced frontier and collapse narrowing entirely (unsound alternative to fixing the cache-key boundary).
Fail-closed paths that remain sound today: RerunNodeSetFailClosed / AffectedSetFailClosed → full roster; DiagnosticClaim always included on produced frontiers via the guard above.
— sent from cool-hawk-221
codex api-review @
|
|
Review metadata
1. Story of the diffThis PR wires a Wave-0 affected-set-driven CI selection path into v4’s modeled workflow surface. The new 2. Invariant categories
chatgpt-review-d45ceac2-5e9a-47…
2.5. Top-down PM intent reviewCompliant. At the PM-intent level, this PR is moving in the right direction: CI selection is represented in 3. VerdictREQUEST_CHANGES The feature direction is right and the SG-0/test scaffolding is mostly well tracked, but |
openai-pro REQUEST_CHANGES @
|
codex api-review #16660 @
|
|
Review metadata
1. Story of the diffThis PR wires a v4 CI selection surface around the affected-set lens: 2. Invariant categories
2.5. Top-down PM intent reviewCompliant. The highest-level intent here is recursive CI/build modeling through 3. VerdictAPPROVE I did not find a merge-blocking invariant violation. The PR adds substrate/workflow-facing selection logic, but the risky parts are either routed through canonical surfaces ( |
openai-pro api-review @
|
openai-pro review #16663 @
|
1 participant
Summary
Wave-0 selection algebra for affected-set-driven CI: composes T-21
affected_set_rerun_nodesintov4.workflow.ci_runner(CiRerunSelection). A TestClaim is selected for rerun whentest_claim_input(claim)is in theRerunNodeSetnode frontier;RerunNodeSetFailClosedselects the full roster (conservative fail-closed, same posture as v3 gate #103 TouchAll).This PR does not close the full Bazel-replacement lane — it lands the
.dagconsumer + structural receipts ahead of follow-on work (TestClaim exercise metadata, path-regex edit-locus, ci.yml host wire, e2e demo). PM ratified Wave-0 defaults: root-only exercise proxy + path-regex edit-locus in separate PRs.Test plan
cargo test -p v3-compiler v4_workflow_ci_runner -- --quiet(4 parse/smoke tests)Follow-on (manager subtree)
TestClaimExerciseSurfacecarrier + migration (keen-stag-226)