feat(speculation): reconcile dead paths and finalize from the tree#371
Open
behinddwalls wants to merge 1 commit into
Open
feat(speculation): reconcile dead paths and finalize from the tree#371behinddwalls wants to merge 1 commit into
behinddwalls wants to merge 1 commit into
Conversation
## Summary ### Why? Two speculation.go behaviors were still driven off the pre-tree dependency-only model from earlier commits in this stack: dependency outcomes were folded into the batch via a bespoke tryFinalize/failOnDependency pair that only looked at batch dependency state and never at the tree, and per-path build outcomes were never reconciled back into the tree at all. A build finishing, or a dependency dying mid-flight, left stale Candidate/Selected/Building statuses that scoring and selection had to route around indirectly. This split made the tree an incomplete picture of a batch's speculation state and kept the merge decision in logic with no visibility into per-path build results. ### What? speculateBatch now reconciles the tree against reality on every pass, before scoring. reconcile() folds each path's build outcome (via the path->build mapping and build store) into its status through reconcileStatus(), then walks the tree a second time to dead any path whose dependency chain can no longer land. Paths already at a terminal status (Passed/Failed/Cancelled) are settled and skipped without touching storage, and statuses are folded into the tree's own path slice in place (mirroring applyScores/applySelection — no per-pass copy), so a pass costs at most two point reads per unresolved path and stays O(paths) in reads and CPU while allocating nothing. The scaling rules for paths and trees — the enumerator implementation decides which and how many paths a batch gets, bounded frontier, never the 2^N power set of dependency subsets, O(paths) per pass — are captured in a new "speculation paths and trees" section in CLAUDE.md so future changes account for them. reconcileStatus() is pure status arithmetic with two rules its doc now spells out: a terminal path status is never downgraded, and Cancelling records a decision about the path (deselected, or assumptions dead), not a prediction about the build — so a Cancelling path holds until its build reaches any terminal state and then settles to Cancelled, including a build that raced to Succeeded before the intent was enacted (its result is moot for a path that will never merge; the build stage never cancels an already-terminal build). pathDead() treats a Failed base dependency, or a Succeeded non-base dependency, as fatal to a path; a Cancelled base dependency is deliberately tolerated (Phase-A leniency — a cancelled batch never lands and so cannot conflict, and with exactly one chain path per batch today, deading it would fail batches that currently survive). Tightening that case once multi-path enumeration lands is tracked in #369 and linked from the code. A dead path that is still Building is captured as a cancel intent (Cancelling status) with no runner call. Actually cancelling the in-flight build is the build stage's job (build.go's enactCancel, added earlier in this stack), reached one hop later through the prioritize round speculate already publishes on every pass — prioritize's republishBuilds re-triggers the build stage for any tree with a Cancelling path. A dead path with no build yet drops straight to Cancelled. finalize()/mergeableNow()/viable() replace tryFinalize()/failOnDependency(). finalize settles the batch's forward step in exactly one of three outcomes per pass, which its doc now enumerates: merge now (some path is mergeableNow — publish to merge, CAS to Merging), wait (no path mergeable but at least one still viable — no-op until the next event), or fail (no viable path remains — CAS to Failed, fan out to dependents, publish to conclude). mergeableNow checks the path's own status (must be Passed) in addition to its dependencies; viable distinguishes "still might merge" from "definitely won't" (Failed, Cancelled, and Cancelling paths are not viable). finalize runs unconditionally on every pass — it no longer needs the batch's original-state gate the previous commit introduced as a stopgap, since mergeableNow correctly requires a path to have actually passed before merging. The dependent fan-out on the fail outcome is publish-only — one speculate wake-up per dependent, each processed asynchronously on its own delivery — now stated explicitly at the call site. Process's terminal self-heal now re-fans dependents for Failed batches too, not just Cancelled ones: a batch failed by finalize (no viable path left) never passes through mergesignal, so speculate's redelivery self-heal is the only stage that can re-notify its dependents if the fan-out publish was ever lost. The buildRunners dependency and the tree-driven cancelBatch/cancelBuilds/cancelTree replacement are deliberately left out of this commit — reconcile() here never talks to a build runner by design. That lands in the next commit in the stack, which is also the one place a direct runner cancel remains justified (batch-level cancel, not path-level). ## Test Plan ✅ `go build ./...` and `go vet ./...` — clean (pre-existing vet warnings in mergeconflictsignal/mergesignal test files are unrelated to this change). ✅ `bazel test //submitqueue/... //service/...` — 56/56 targets pass; new reconcile/dead-path/finalize/pure-function tests pass alongside the preserved legacy cancel tests, and the merge-gate tests pin that terminal paths cost no path->build reads. ✅ `make gazelle && make fmt` — no changes. ✅ `make e2e-test` — stovepipe and submitqueue e2e suites both pass. ## Issue Follow-up tracked in #369 (tighten pathDead's Cancelled-base-dependency leniency once multi-path enumeration lands).
This was referenced Jul 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why?
Two speculation.go behaviors were still driven off the pre-tree dependency-only model from earlier commits in this stack: dependency outcomes were folded into the batch via a bespoke tryFinalize/failOnDependency pair that only looked at batch dependency state and never at the tree, and per-path build outcomes were never reconciled back into the tree at all. A build finishing, or a dependency dying mid-flight, left stale Candidate/Selected/Building statuses that scoring and selection had to route around indirectly. This split made the tree an incomplete picture of a batch's speculation state and kept the merge decision in logic with no visibility into per-path build results.
What?
speculateBatch now reconciles the tree against reality on every pass, before scoring. reconcile() folds each path's build outcome (via the path->build mapping and build store) into its status through reconcileStatus(), then walks the tree a second time to dead any path whose dependency chain can no longer land. Paths already at a terminal status (Passed/Failed/Cancelled) are settled and skipped without touching storage, and statuses are folded into the tree's own path slice in place (mirroring applyScores/applySelection — no per-pass copy), so a pass costs at most two point reads per unresolved path and stays O(paths) in reads and CPU while allocating nothing. The scaling rules for paths and trees — the enumerator implementation decides which and how many paths a batch gets, bounded frontier, never the 2^N power set of dependency subsets, O(paths) per pass — are captured in a new "speculation paths and trees" section in CLAUDE.md so future changes account for them.
reconcileStatus() is pure status arithmetic with two rules its doc now spells out: a terminal path status is never downgraded, and Cancelling records a decision about the path (deselected, or assumptions dead), not a prediction about the build — so a Cancelling path holds until its build reaches any terminal state and then settles to Cancelled, including a build that raced to Succeeded before the intent was enacted (its result is moot for a path that will never merge; the build stage never cancels an already-terminal build).
pathDead() treats a Failed base dependency, or a Succeeded non-base dependency, as fatal to a path; a Cancelled base dependency is deliberately tolerated (Phase-A leniency — a cancelled batch never lands and so cannot conflict, and with exactly one chain path per batch today, deading it would fail batches that currently survive). Tightening that case once multi-path enumeration lands is tracked in #369 and linked from the code.
A dead path that is still Building is captured as a cancel intent (Cancelling status) with no runner call. Actually cancelling the in-flight build is the build stage's job (build.go's enactCancel, added earlier in this stack), reached one hop later through the prioritize round speculate already publishes on every pass — prioritize's republishBuilds re-triggers the build stage for any tree with a Cancelling path. A dead path with no build yet drops straight to Cancelled.
finalize()/mergeableNow()/viable() replace tryFinalize()/failOnDependency(). finalize settles the batch's forward step in exactly one of three outcomes per pass, which its doc now enumerates: merge now (some path is mergeableNow — publish to merge, CAS to Merging), wait (no path mergeable but at least one still viable — no-op until the next event), or fail (no viable path remains — CAS to Failed, fan out to dependents, publish to conclude). mergeableNow checks the path's own status (must be Passed) in addition to its dependencies; viable distinguishes "still might merge" from "definitely won't" (Failed, Cancelled, and Cancelling paths are not viable). finalize runs unconditionally on every pass — it no longer needs the batch's original-state gate the previous commit introduced as a stopgap, since mergeableNow correctly requires a path to have actually passed before merging. The dependent fan-out on the fail outcome is publish-only — one speculate wake-up per dependent, each processed asynchronously on its own delivery — now stated explicitly at the call site.
Process's terminal self-heal now re-fans dependents for Failed batches too, not just Cancelled ones: a batch failed by finalize (no viable path left) never passes through mergesignal, so speculate's redelivery self-heal is the only stage that can re-notify its dependents if the fan-out publish was ever lost.
The buildRunners dependency and the tree-driven cancelBatch/cancelBuilds/cancelTree replacement are deliberately left out of this commit — reconcile() here never talks to a build runner by design. That lands in the next commit in the stack, which is also the one place a direct runner cancel remains justified (batch-level cancel, not path-level).
Test Plan
✅
go build ./...andgo vet ./...— clean (pre-existing vet warnings in mergeconflictsignal/mergesignal test files are unrelated to this change).✅
bazel test //submitqueue/... //service/...— 56/56 targets pass; new reconcile/dead-path/finalize/pure-function tests pass alongside the preserved legacy cancel tests, and the merge-gate tests pin that terminal paths cost no path->build reads.✅
make gazelle && make fmt— no changes.✅
make e2e-test— stovepipe and submitqueue e2e suites both pass.Issue
Follow-up tracked in #369 (tighten pathDead's Cancelled-base-dependency leniency once multi-path enumeration lands).
Stack