Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/rfc/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Design documents and technical proposals, grouped by scope. Shared/cross-cutting
- [Extension Contract](submitqueue/extension-contract.md) - When extensions take orchestrator identity (request/batch) and resolve granular content themselves vs. take controller-resolved data; revises the BuildRunner base/head contract
- [Gateway Status and List APIs](submitqueue/status-list-api.md) - Gateway-owned request context, materialized current status, sqid or change-URI status lookup, and queue admission listing
- [Speculation](submitqueue/speculation.md) - Why SubmitQueue speculates, the path/tree model, and the two pluggable seams: speculation-tree enumeration and path selection
- [Outcome Predictor](submitqueue/outcome-predictor.md) - How likely a batch is to succeed: a predictor built with a Scorer that multiplies its price by what the pipeline has observed (a build passed, the batch is merging), factors written by hand first and fitted later
- [Best-First Speculation Path Generation](submitqueue/speculation-generator-best-first.md) - The default Generator: per-head lazy streams of flip subsets merged best-first across heads, log-probability ranking, and the strict snapshot contract
- [Modular Queue Wiring](submitqueue/modular-queue-wiring.md) - Declare-don't-assemble engine (`pipeline.Construct`) that unifies topic registry, controller registration, DLQ pairing, and lifecycle ordering into one typed call; services self-declare via Deps struct + Stages slice, hosts own per-queue profiles and transport

Expand Down
103 changes: 103 additions & 0 deletions doc/rfc/submitqueue/outcome-predictor.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
# Outcome Predictor

How likely a batch is to succeed, from a Scorer's price plus what the pipeline has since observed about the batch.

## Problem

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 queue holds that evidence in memory during the run that needs it, and throws it away.

The prices are also fixed numbers someone typed. The heuristic scorer (`submitqueue/extension/speculation/scorer/heuristic`) maps lines changed onto a bucket table, an unconfigured queue prices everything at 0.5, and the bestfirst generator (`submitqueue/extension/speculation/generator/bestfirst`) uses 0.95 for anything it could not price. Nothing improves them over time.

This document covers the first problem only. Pricing a change from its content is a separate question, and it stays in whatever Scorer a queue configures for it.

## What it does

The predictor is its own contract, separate from the Scorer. It is handed a batch and what that batch's builds have done, and it returns how likely the batch is to reach Succeeded. It is built with a Scorer: it asks the Scorer for the batch's price, then multiplies the **odds** of that price by one factor per piece of evidence.

The generator depends on the predictor, not on the Scorer.

A batch the base prices at 0.6 has odds of 1.5 to 1. Its build passes, worth ten times the odds: 15 to 1, or 0.94. It reaches merging, worth another twelve: 180 to 1, or 0.995. The generator already resolves a batch to certainty once it is terminal, so the predictor never has to handle that case.

Multiplying odds keeps the answer a probability without clamping, and it makes each factor mean the same thing wherever it applies — the same evidence is worth the same amount whether the base said 0.5 or 0.95. Adding to the probability directly has neither property.

The factors are what gets configured, and later learned. Written as logs and summed, this is ordinary logistic regression, so fitting it needs nothing special.

**All factors at one, the predictor returns the base price unchanged.** Turning it on is a no-op until someone sets a factor.

## Where the line falls

**The scorer prices the change. The predictor prices the situation.**

Lines, files, which directories, who wrote it — that describes the change, and it belongs in the scorer. Builds, batch state, dependencies, time waiting — that describes where the batch sits right now, and it belongs in the predictor.

That is also why they are two contracts rather than one. The evidence is an input the scorer has no use for: put it on `Score` and every implementation that prices content — all three that exist — takes a parameter it discards. A parameter every implementation throws away belongs to a different contract.

Keeping them apart means either side can be replaced without redoing the other, and it leaves the scorer per-queue configurable, which it already is.

## What it looks at

| Evidence | Direction |
| --- | --- |
| A build passed for this batch | Strongly up — the biggest factor, with the caveat below |
| Builds failed for this batch, counted | Down |
| How old the passing build is | Down as it ages — trunk moves |
| A build is running | Slightly up: better than never tried |
| The batch is merging | Strongly up — it cleared speculation and is pushing, but can still lose a race |
| The batch is cancelling | Strongly down, though not to zero — cancelling is best effort |
| How many dependencies it has | Down — more assumptions that can break |
| How long it has been in the queue | Down — usually it has been invalidated before |
| The queue's recent landing rate | Moves everything, so a bad week does not need re-fitting |

Two of these are decisions rather than just numbers.

**A passing build only counts under the assumptions it was built with.** A dependency can hold a green build for a path assuming *its* dependency succeeds, while the candidate being priced assumes that one fails. That build says nothing about the second case. So the evidence is narrower than "a build passed": it is "a build passed on the all-succeed path", which is right whenever the candidate assumes the same — the common case — and must not count otherwise.

**Merging and cancelling belong here and nowhere else.** The generator may only tell terminal from non-terminal; the allocator reads path status only. Pinning a merging dependency to certain inside the generator was tried and reverted, correctly — a merge can fail, so nothing is settled. How much a state is worth is a price, and that price is the predictor's.

## Getting the evidence

Batch state comes free: the predictor is handed the batch, which carries it. The build evidence does not. The speculate controller reads each in-flight head's path set once per run and hands it to the Speculator; the Generator never sees it.

So the Generator takes the path sets alongside the batches, and hands each batch its own set when it asks for a prediction. The Scorer contract does not change at all.

The alternative — the predictor reads the path-set store itself — needs no contract change but re-reads what the run already holds, once per dependency, and can see a newer version than the rest of the run is working from. The run reads once so that two decisions in it can never disagree about the world, and that is worth more than the plumbing costs.

## Fitting the factors

Written by hand first. A factor is "how much does this multiply the odds", which is a number an engineer can propose and a reviewer can argue with, so the first version needs no data at all.

Learning them later needs three things:

- **A record written when the price is set** — the batch, the base price, each piece of evidence, the result, and the predictor version. Writing it when the outcome arrives instead would record values the serving path never has.
- **Outcomes joined to it.** Succeeded is a yes, failed is a no, and a batch the author cancelled is neither, so it is left out — counting it as a failure would teach the predictor that abandoned work is bad code. Batch outcomes are not logged today; they can be derived from request logs through the request-batch store.
- **An awareness that the data is biased.** Only paths that got funded produce outcomes, so the records show what the previous ranking already believed. Fitted factors will lean toward agreeing with it.

The fitted result is a small versioned file of named factors loaded at wiring time, with the evidence list hashed so a file that does not match the code fails at startup rather than mispricing quietly.

## Judging it

Whether the probabilities are honest is measurable, but it is not the point. The point is builds per landed change, share of builds spent on paths later thrown away, and how long the queue takes. Measuring those before production means replaying recorded runs against a candidate predictor. No such harness exists, and that is the main thing between fitted factors and trusting them.

## Rollout

1. Start recording prices and outcomes. Nothing else works without it.
2. Ship the predictor with all factors at one — identical output — then set factors for builds and batch state by hand. This is the largest win and it needs no data.
3. Fit the factors. Run it alongside the current ranking first, then one queue, then everywhere.

## Not in scope

Worth doing, separately: predicting how long and how large a build will be, so the allocator can rank by value per CI-minute rather than by probability alone; per-directory history in the base; and pricing a dependency against the specific assumptions a path makes about its own dependencies, which would remove the caveat on build evidence.

## Rejected

**One contract, with the evidence on `Score`.** Tried: every scorer that prices content took a parameter it discarded, and the composite forwarded one it never read. The evidence is not part of pricing a change.

**One estimate over content and evidence together.** They change at different rates and need different amounts of data, and it would force every queue onto the same content scorer.

**Adding to the probability instead of multiplying odds.** Leaves the range, needs clamping, and the same increment means different things at different prices.

**More dimensions on the bucket table.** A second dimension squares it, a third makes it unwritable, and every cell is still a guess.

**Putting state in the generator.** Tried and reverted: a merge can fail, so nothing is settled, and how much a state is worth is a price.

**A scoring stage.** Prices only mean anything inside the run that produced them; storing them would make them stale by construction.
3 changes: 3 additions & 0 deletions service/submitqueue/orchestrator/server/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ go_library(
"//submitqueue/extension/conflict/pathoverlap:go_default_library",
"//submitqueue/extension/speculation/allocator/sticky:go_default_library",
"//submitqueue/extension/speculation/generator/bestfirst:go_default_library",
"//submitqueue/extension/speculation/predictor:go_default_library",
"//submitqueue/extension/speculation/predictor/regression:go_default_library",
"//submitqueue/extension/speculation/scorer:go_default_library",
"//submitqueue/extension/speculation/scorer/composite:go_default_library",
"//submitqueue/extension/speculation/scorer/fake:go_default_library",
Expand Down Expand Up @@ -118,6 +120,7 @@ go_test(
"//submitqueue/extension/buildrunner:go_default_library",
"//submitqueue/extension/changeprovider:go_default_library",
"//submitqueue/extension/conflict:go_default_library",
"//submitqueue/extension/speculation/predictor:go_default_library",
"//submitqueue/extension/speculation/scorer:go_default_library",
"//submitqueue/extension/speculation/speculator:go_default_library",
"//submitqueue/extension/storage:go_default_library",
Expand Down
67 changes: 66 additions & 1 deletion service/submitqueue/orchestrator/server/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,22 @@ const (
// Ways a composite scorer combines its components.
const combineAvg = "avg"

// Predictor types selectable from configuration.
const predictorTypeRegression = "regression"

// Evidence a regression predictor prices, as named in configuration. The set is
// closed: a factor under any other name would be applied to nothing and never
// noticed.
const (
factorPathPassed = "pathPassed"
factorPathFailed = "pathFailed"
factorMerging = "merging"
factorCancelling = "cancelling"
)

// neutralFactor leaves the scorer's price untouched: odds multiplied by one.
const neutralFactor = 1.0

// defaultBuildBudget is how many builds a queue may have occupying CI at once
// when it states no budget of its own. Four is enough for speculation to be
// visible — a queue that can only build one path never speculates — while
Expand Down Expand Up @@ -110,6 +126,7 @@ type namedQueueProfileConfig struct {
Analyzer *analyzerConfig `yaml:"analyzer"`
Scorer *scorerConfig `yaml:"scorer"`
Speculator *speculatorConfig `yaml:"speculator"`
Predictor *predictorConfig `yaml:"predictor"`
}

// queueProfileConfig is the full set of extensions a queue resolves to.
Expand All @@ -119,6 +136,7 @@ type queueProfileConfig struct {
Analyzer analyzerConfig `yaml:"analyzer"`
Scorer scorerConfig `yaml:"scorer"`
Speculator speculatorConfig `yaml:"speculator"`
Predictor predictorConfig `yaml:"predictor"`
}

// changeProviderConfig selects how change metadata is fetched. The github and
Expand Down Expand Up @@ -239,6 +257,17 @@ type speculatorConfig struct {
BuildBudget int `yaml:"buildBudget"`
}

// predictorConfig tunes how a queue turns its scorer's price into the
// probability the generator ranks on. The scorer being revised is the queue's
// own, so it is not named again here.
type predictorConfig struct {
Type string `yaml:"type"`
// Factors multiply the odds of the scorer's price, one per piece of
// evidence, keyed by evidence name. An omitted factor is neutral, so an
// omitted block ranks on the scorer's price alone.
Factors map[string]float64 `yaml:"factors"`
}

// loadProfilesConfig reads and validates the profiles configuration at path.
func loadProfilesConfig(path string) (profilesConfig, error) {
data, err := os.ReadFile(path)
Expand Down Expand Up @@ -300,6 +329,11 @@ func (c *profilesConfig) normalizeAndValidate() error {
return err
}
}
if q.Predictor != nil {
if err := q.Predictor.normalizeAndValidate(where); err != nil {
return err
}
}
}
return c.validateGitRepoPaths()
}
Expand Down Expand Up @@ -358,6 +392,9 @@ func (c profilesConfig) resolve(q namedQueueProfileConfig) queueProfileConfig {
if q.Speculator != nil {
profile.Speculator = *q.Speculator
}
if q.Predictor != nil {
profile.Predictor = *q.Predictor
}
return profile
}

Expand All @@ -374,7 +411,10 @@ func (p *queueProfileConfig) normalizeAndValidate(where string) error {
if err := p.Scorer.normalizeAndValidate(where); err != nil {
return err
}
return p.Speculator.normalizeAndValidate(where)
if err := p.Speculator.normalizeAndValidate(where); err != nil {
return err
}
return p.Predictor.normalizeAndValidate(where)
}

func (c *changeProviderConfig) normalizeAndValidate(where string) error {
Expand Down Expand Up @@ -556,6 +596,31 @@ func (s *scorerConfig) normalizeAndValidate(where string) error {
return nil
}

// normalizeAndValidate applies defaults and rejects a predictor that could not
// be built. An empty block is a regression predictor with every factor neutral,
// which prices a batch at exactly its scorer's price.
func (p *predictorConfig) normalizeAndValidate(where string) error {
if p.Type == "" {
p.Type = predictorTypeRegression
}
if p.Type != predictorTypeRegression {
return fmt.Errorf("%s: unknown predictor type %q", where, p.Type)
}
for name, factor := range p.Factors {
switch name {
case factorPathPassed, factorPathFailed, factorMerging, factorCancelling:
default:
return fmt.Errorf("%s: unknown predictor factor %q", where, name)
}
// Zero would pin every batch carrying the evidence to a probability of
// zero, and a negative multiplier on odds means nothing at all.
if factor <= 0 {
return fmt.Errorf("%s: predictor factor %q is %v, must be positive", where, name, factor)
}
}
return nil
}

func (s *speculatorConfig) normalizeAndValidate(where string) error {
// A negative budget is rejected rather than clamped: sticky would compute no
// free slots from it, so the queue would batch and then never build anything,
Expand Down
46 changes: 46 additions & 0 deletions service/submitqueue/orchestrator/server/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -666,3 +666,49 @@ func TestLoadProfilesConfig_RejectsBadScorers(t *testing.T) {
})
}
}

func TestLoadProfilesConfig_RejectsBadPredictors(t *testing.T) {
tests := []struct {
name string
contents string
}{
{name: "unknown predictor type", contents: "defaults:\n predictor: {type: vibes}\n"},
{name: "unknown factor", contents: "defaults:\n predictor:\n factors: {pathPased: 2}\n"},
{name: "zero factor", contents: "defaults:\n predictor:\n factors: {merging: 0}\n"},
{name: "negative factor", contents: "defaults:\n predictor:\n factors: {pathFailed: -1}\n"},
{name: "bad factor on a queue override", contents: "defaults: {}\nqueues:\n - name: q\n predictor:\n factors: {merging: 0}\n"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, err := loadProfilesConfig(writeProfiles(t, tt.contents))
require.Error(t, err)
})
}
}

// An omitted predictor block leaves the queue ranking on its scorer's price
// alone, which is what every queue does until someone states a factor.
func TestLoadProfilesConfig_DefaultsThePredictorToNeutral(t *testing.T) {
cfg, err := loadProfilesConfig(writeProfiles(t, "defaults: {}\nqueues:\n - name: q\n"))
require.NoError(t, err)

assert.Equal(t, predictorTypeRegression, cfg.Defaults.Predictor.Type)

factors := factorsFrom(cfg.resolve(cfg.Queues[0]).Predictor)
assert.Equal(t, neutralFactor, factors.PathPassed)
assert.Equal(t, neutralFactor, factors.PathFailed)
assert.Equal(t, neutralFactor, factors.Merging)
assert.Equal(t, neutralFactor, factors.Cancelling)
}

func TestLoadProfilesConfig_ReadsPredictorFactors(t *testing.T) {
cfg, err := loadProfilesConfig(writeProfiles(t,
"defaults:\n predictor:\n factors: {pathPassed: 10, pathFailed: 0.3, merging: 12, cancelling: 0.1}\n"))
require.NoError(t, err)

factors := factorsFrom(cfg.Defaults.Predictor)
assert.Equal(t, 10.0, factors.PathPassed)
assert.Equal(t, 0.3, factors.PathFailed)
assert.Equal(t, 12.0, factors.Merging)
assert.Equal(t, 0.1, factors.Cancelling)
}
Loading