feat(speculation): predict a batch's outcome from its scorer price and its builds - #626
Draft
behinddwalls wants to merge 1 commit into
Draft
feat(speculation): predict a batch's outcome from its scorer price and its builds#626behinddwalls wants to merge 1 commit into
behinddwalls wants to merge 1 commit into
Conversation
…d its builds ## Summary ### Why? Nothing the pipeline learns about a batch changes its price. A batch whose build has passed, whose dependencies have landed, and which is being merged is priced exactly as it was before anything was known about it — on the size of its diff. The speculate controller holds that evidence in memory during the run that needs it, and throws it away, so `bestfirst` ranks paths on a number that ignores the queue's own results. The evidence does not belong on `Score`. Putting it there was tried first and reverted: all three scorer implementations took a parameter they discarded, which is the tell that it belongs to a different contract. A scorer prices a change; how far a batch has got is not part of that question. ### What? A second contract, `predictor.OutcomePredictor`, is handed a batch and its path set and returns how likely the batch is to reach `Succeeded`. It is built with a `Scorer` and revises that scorer's price. The generator now depends on the predictor; `scorer.Scorer` is untouched. `predictor/regression` is the implementation. It converts the scorer's price to odds, multiplies by one configured factor per piece of evidence, and converts back — so the result stays a probability with no clamping, and a factor means the same thing whatever the price was. Written as logs and summed the same arithmetic is a logistic regression, which is what lets hand-written factors later be replaced by fitted ones without changing the form. Evidence priced today: a passed build, failed builds (compounding), and the merging and cancelling states. A passed build only counts on the all-succeed path — one built without a dependency's changes says nothing about a candidate that assumes the dependency lands. Two edges worth knowing. The price is bounded away from 0 and 1 before conversion, because those have no finite odds and a certain scorer could otherwise never be revised. A price that is not a probability is an error rather than a clamp: that is a broken scorer, and `bestfirst` already substitutes its own default when a price cannot be had. `Generate` gains the path sets, threaded from the Speculator, which already receives them. The alternative — reading the path-set store from inside the predictor — would re-read what the run already holds and could see a newer version than the rest of the run, breaking the single-read invariant the snapshot depends on. Configuration is a `predictor:` block whose factors default to 1, so an absent block ranks on the scorer's price exactly as before. The queue's existing `scorer:` is the base; it is not named again. Design and rationale: `doc/rfc/submitqueue/outcome-predictor.md`. ## Test Plan ✅ `bazel test //submitqueue/... //service/... //platform/...` — 80 tests pass ✅ New: the neutral set returns the scorer's price unchanged; each factor applies only on its evidence; failures compound; a passed path assuming a failure does not count; prices of 0 and 1 stay in range and still move; a non-probability price, a NaN price and a scorer error each surface as errors; `New` rejects a nil base and a non-positive factor ✅ New: each dependency is priced against its own path set, and one that never speculated gets the zero value ✅ New: config defaults to neutral, rejects unknown and non-positive factors ✅ `make fmt`, `make gazelle`, `make mocks` — idempotent Not run: `make e2e-test`. The end-to-end check worth doing before the factors are turned up is a queue configured with a large `pathPassed`, confirming a batch with a passed path ranks ahead of a same-size batch without one.
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?
Nothing the pipeline learns about a batch changes its price. A batch whose build has passed, whose dependencies have landed, and which is being merged is priced exactly as it was before anything was known about it — on the size of its diff. The speculate controller holds that evidence in memory during the run that needs it, and throws it away, so
bestfirstranks paths on a number that ignores the queue's own results.The evidence does not belong on
Score. Putting it there was tried first and reverted: all three scorer implementations took a parameter they discarded, which is the tell that it belongs to a different contract. A scorer prices a change; how far a batch has got is not part of that question.What?
A second contract,
predictor.OutcomePredictor, is handed a batch and its path set and returns how likely the batch is to reachSucceeded. It is built with aScorerand revises that scorer's price. The generator now depends on the predictor;scorer.Scoreris untouched.predictor/regressionis the implementation. It converts the scorer's price to odds, multiplies by one configured factor per piece of evidence, and converts back — so the result stays a probability with no clamping, and a factor means the same thing whatever the price was. Written as logs and summed the same arithmetic is a logistic regression, which is what lets hand-written factors later be replaced by fitted ones without changing the form.Evidence priced today: a passed build, failed builds (compounding), and the merging and cancelling states. A passed build only counts on the all-succeed path — one built without a dependency's changes says nothing about a candidate that assumes the dependency lands.
Two edges worth knowing. The price is bounded away from 0 and 1 before conversion, because those have no finite odds and a certain scorer could otherwise never be revised. A price that is not a probability is an error rather than a clamp: that is a broken scorer, and
bestfirstalready substitutes its own default when a price cannot be had.Generategains the path sets, threaded from the Speculator, which already receives them. The alternative — reading the path-set store from inside the predictor — would re-read what the run already holds and could see a newer version than the rest of the run, breaking the single-read invariant the snapshot depends on.Configuration is a
predictor:block whose factors default to 1, so an absent block ranks on the scorer's price exactly as before. The queue's existingscorer:is the base; it is not named again.Design and rationale:
doc/rfc/submitqueue/outcome-predictor.md.Test Plan
✅
bazel test //submitqueue/... //service/... //platform/...— 80 tests pass✅ New: the neutral set returns the scorer's price unchanged; each factor applies only on its evidence; failures compound; a passed path assuming a failure does not count; prices of 0 and 1 stay in range and still move; a non-probability price, a NaN price and a scorer error each surface as errors;
Newrejects a nil base and a non-positive factor✅ New: each dependency is priced against its own path set, and one that never speculated gets the zero value
✅ New: config defaults to neutral, rejects unknown and non-positive factors
✅
make fmt,make gazelle,make mocks— idempotentNot run:
make e2e-test. The end-to-end check worth doing before the factors are turned up is a queue configured with a largepathPassed, confirming a batch with a passed path ranks ahead of a same-size batch without one.Stack