feat(speculate): naive happy-path speculation in speculate controller#148
Merged
Conversation
behinddwalls
approved these changes
Apr 30, 2026
| // startSpeculation kicks off CI for this batch on top of the speculative head | ||
| // (batch.Dependencies assumed to all pass), then transitions to Speculating. | ||
| func (c *Controller) startSpeculation(ctx context.Context, batch entity.Batch) error { | ||
| c.logger.Infow("starting speculation", |
Collaborator
There was a problem hiding this comment.
we probably want to use the context aware logger for tracing/ maybe we deal it with separately to track request journey in the logs end to end
Contributor
Author
There was a problem hiding this comment.
not today, I just want to have something working first
sbalabanov
force-pushed
the
sergeyb/speculate
branch
4 times, most recently
from
70eb275 to
dae5582
Compare
Replaces the unconditional fan-out with a state-machine driver that advances a batch one step per invocation along a single happy-path speculation chain (batch.Dependencies + [batch.ID]): - Created/Scored → publish to build, Speculating - Speculating + deps OK → publish to merge, Merging - Speculating + dep failed → fail batch, log per request, publish to conclude - Speculating, waiting → no-op (re-evaluated when deps advance) - Merging/terminal → no-op - Unrecognized state → error (nack so the event is retried) When a dependency reaches Failed/Cancelled the batch cannot land on top of it; rather than livelock in Speculating forever the controller now emits a RequestStatusError log per contained request, transitions the batch to Failed, and publishes to conclude so the request store and log get reconciled. Assumes every in-flight build will pass and ignores score; failure replanning, score-driven prioritization, and the full Java-style SpeculationTree (2^N orderings + scoring) are explicit TODOs. Publishes before CAS-updating state so a publish failure leaves state unchanged for retry. Also renames BatchStateFinalizing → BatchStateMerging across the entity, batch controller, and tests.
sbalabanov
force-pushed
the
sergeyb/speculate
branch
from
dae5582 to
43fb559
Compare
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
3 participants
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
Replaces the speculate controller's unconditional fan-out (always publishing to both
buildandmerge) with a state-machine driver that advances a batch one step per invocation along a single happy-path speculation chain (batch.Dependencies + [batch.ID]).Per call, based on
batch.State:Created/Scoredbuild, transition toSpeculatingSpeculating+ all depsSucceededmerge, transition toFinalizingSpeculating, deps still in flightSpeculating, depFailed/CancelledFinalizing/ terminalInspired by the legacy Java
SpeculationTree/StrategyImpl(underdev-platform/cicd/submitqueue). Deliberately omitted from this PR — these are TODOs for follow-ups:NodeScorer/PrioritizerImpl).ExhaustiveEnumerator).Implementation notes
UpdateState. A publish failure leaves the state unchanged for the retry to re-attempt cleanly.buildcontroller does not yet receive it as part of the message (TODO: threadSpeculationPathInfothrough tobuild/buildsignal).merge → speculateandbuildsignal → speculatehops; downstream batches getting re-evaluated when an upstream batch concludes is a separate gap (not addressed here).Test plan
go test ./orchestrator/controller/speculate/...— passesgo test ./orchestrator/controller/...— passes (no regressions in adjacent controllers)go vet ./...— clean🤖 Generated with Claude Code