Skip to content

Commit 0ff6ea7

Browse files
committed
feat(speculation): fold predictor into Scorer
Score takes the path-set snapshot. Evidence is a scorer wrapping a base; heuristic and composite ignore paths. Delete the sibling Predictor factory.
1 parent ff043d9 commit 0ff6ea7

20 files changed

Lines changed: 92 additions & 293 deletions

File tree

submitqueue/extension/speculation/generator/bestfirst/bestfirst.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ func (g *bestFirst) score(ctx context.Context, ids []string, batchByID map[strin
121121
probabilityByID[id] = defaultProbability
122122
continue
123123
}
124-
probability, err := g.scorer.Score(ctx, batch)
124+
probability, err := g.scorer.Score(ctx, batch, entity.SpeculationPathSet{})
125125
if err != nil {
126126
// A scorer that failed because the caller went away has not found
127127
// an unpriceable dependency — it has found a dead ctx, which ends

submitqueue/extension/speculation/generator/bestfirst/bestfirst_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ type stubScorer struct {
3838
scores map[string]float64
3939
}
4040

41-
func (s stubScorer) Score(_ context.Context, b entity.Batch) (float64, error) {
41+
func (s stubScorer) Score(_ context.Context, b entity.Batch, _ entity.SpeculationPathSet) (float64, error) {
4242
if v, ok := s.scores[b.ID]; ok {
4343
return v, nil
4444
}
@@ -136,7 +136,7 @@ func newCountingScorer(scores map[string]float64) *countingScorer {
136136
return &countingScorer{scores: scores, calls: map[string]int{}}
137137
}
138138

139-
func (c *countingScorer) Score(_ context.Context, b entity.Batch) (float64, error) {
139+
func (c *countingScorer) Score(_ context.Context, b entity.Batch, _ entity.SpeculationPathSet) (float64, error) {
140140
c.calls[b.ID]++
141141
c.total++
142142
if v, ok := c.scores[b.ID]; ok {
@@ -148,14 +148,14 @@ func (c *countingScorer) Score(_ context.Context, b entity.Batch) (float64, erro
148148
// errScorer always fails, to exercise error propagation from scoring.
149149
type errScorer struct{}
150150

151-
func (errScorer) Score(context.Context, entity.Batch) (float64, error) {
151+
func (errScorer) Score(context.Context, entity.Batch, entity.SpeculationPathSet) (float64, error) {
152152
return 0, assert.AnError
153153
}
154154

155155
// constScorer scores every batch identically, regardless of ID.
156156
type constScorer struct{ v float64 }
157157

158-
func (c constScorer) Score(context.Context, entity.Batch) (float64, error) { return c.v, nil }
158+
func (c constScorer) Score(context.Context, entity.Batch, entity.SpeculationPathSet) (float64, error) { return c.v, nil }
159159

160160
// wideHead builds one Speculating head over n unresolved dependencies, each at a
161161
// distinct score so no two combinations tie.
@@ -839,7 +839,7 @@ func TestBestFirst_HonorsCancelledContext(t *testing.T) {
839839
// own call was cancelled would.
840840
type cancellingScorer struct{ cancel context.CancelFunc }
841841

842-
func (s cancellingScorer) Score(context.Context, entity.Batch) (float64, error) {
842+
func (s cancellingScorer) Score(context.Context, entity.Batch, entity.SpeculationPathSet) (float64, error) {
843843
s.cancel()
844844
return 0, context.Canceled
845845
}

submitqueue/extension/speculation/predictor/BUILD.bazel

Lines changed: 0 additions & 9 deletions
This file was deleted.

submitqueue/extension/speculation/predictor/README.md

Lines changed: 0 additions & 17 deletions
This file was deleted.

submitqueue/extension/speculation/predictor/mock/BUILD.bazel

Lines changed: 0 additions & 13 deletions
This file was deleted.

submitqueue/extension/speculation/predictor/mock/predictor_mock.go

Lines changed: 0 additions & 97 deletions
This file was deleted.

submitqueue/extension/speculation/predictor/predictor.go

Lines changed: 0 additions & 56 deletions
This file was deleted.
Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,23 @@
11
# scorer
22

3-
A `Scorer` returns the probability that a batch ultimately succeeds — reaches its terminal `Succeeded` state with its changes landed, not merely a passing build — as a number between 0.0 and 1.0. It is handed the batch identity and resolves the batch's changes itself through an injected `changeset.Resolver`, so callers pass an `entity.Batch` and nothing more.
3+
A `Scorer` returns how likely a batch is to reach `Succeeded` with its changes landed, as a number between 0.0 and 1.0. `Score(ctx, batch, paths)` is handed the batch identity and that batch's own `SpeculationPathSet` — zero-valued when nothing has speculated on it yet. Callers pass a snapshot they already hold; a scorer must not load the path-set store.
44

55
Callers may score every batch a queue is waiting on, so implementations should be cheap. A speculation run scores each batch at most once, but it does not carry results across runs; anything expensive belongs behind the implementation's own cache.
66

7-
The default speculation pipeline does not rank on the scorer directly. The queue's `Predictor` is built over its `Scorer` and revises the scorer's price with path-set evidence before `bestfirst` ranks paths. The scorer still prices only the change; it does not see path sets.
7+
The default `bestfirst` generator ranks on this number. The default scorer is **evidence** wrapping a **base**: heuristic or composite prices the change and ignores `paths`; evidence revises that price from the path set and batch state.
88

99
Like the other extensions, a `Scorer` is selected **per queue** by the wiring layer through the `Config` (queue name) and `Factory` interface.
1010

11+
See [doc/rfc/submitqueue/outcome-predictor.md](../../../../doc/rfc/submitqueue/outcome-predictor.md) for the GLM, factor contract, evidence rules, and configuration shape.
12+
1113
## Implementations
1214

13-
**`heuristic`** scores a batch by extracting one number from its changes and matching that against ordered buckets, each mapping a `[Min, Max]` range to a probability. The extraction is a caller-supplied `ValueFunc` over the resolved `entity.BatchChanges`, so the same bucketing works for files touched, lines changed, or any other metric.
15+
**`evidence`** revises a nested base scorer with YAML-configured factors for `pathPassed`, `pathFailed`, `merging`, and `cancelling`. A factor of `1` leaves the base price alone; every factor defaults to `1` until someone sets one. Only paths that assume every dependency succeeds count as path evidence.
16+
17+
**`heuristic`** scores a batch by extracting one number from its changes and matching that against ordered buckets, each mapping a `[Min, Max]` range to a probability. The extraction is a caller-supplied `ValueFunc` over the resolved `entity.BatchChanges`, so the same bucketing works for files touched, lines changed, or any other metric. It ignores `paths`.
1418

15-
**`composite`** runs several named scorers and reduces their scores to one. The reduce function receives the scores keyed by scorer name, so it can weigh sources differently rather than treating them as interchangeable; `Min`, `Max`, and `Avg` are provided.
19+
**`composite`** runs several named scorers and reduces their scores to one. The reduce function receives the scores keyed by scorer name, so it can weigh sources differently rather than treating them as interchangeable; `Min`, `Max`, and `Avg` are provided. It ignores `paths` except to forward them to children.
1620

1721
## Adding a backend
1822

19-
Create a package under `scorer/<backend>/` whose `New(...)` returns a `scorer.Scorer`, injecting whatever it needs at construction — a `changeset.Resolver` to reach the batch's changes, a metrics scope, any client. Do not add a `Config` or `Factory` implementation here; per-queue routing and the factory adapter live in the wiring layer.
23+
Create a package under `scorer/<backend>/` whose `New(...)` returns a `scorer.Scorer`, injecting whatever it needs at construction — a nested `Scorer` for evidence, a `changeset.Resolver` to reach the batch's changes, a metrics scope, any client. Do not add a `Config` or `Factory` implementation here; per-queue routing and the factory adapter live in the wiring layer.

submitqueue/extension/speculation/scorer/composite/scorer.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,13 +94,13 @@ func New(cfg scorer.Config, scorers map[string]scorer.Scorer, reduce ReduceFunc,
9494

9595
// Score evaluates all child scorers on the batch and combines their results using the
9696
// reduce function. If any child scorer returns an error, that error is returned immediately.
97-
func (c *compositeScorer) Score(ctx context.Context, batch entity.Batch) (ret float64, retErr error) {
97+
func (c *compositeScorer) Score(ctx context.Context, batch entity.Batch, paths entity.SpeculationPathSet) (ret float64, retErr error) {
9898
op := metrics.Begin(c.scope, "score", metrics.FastLatencyBuckets)
9999
defer func() { op.Complete(retErr) }()
100100

101101
scores := make(map[string]float64, len(c.scorers))
102102
for name, s := range c.scorers {
103-
score, err := s.Score(ctx, batch)
103+
score, err := s.Score(ctx, batch, paths)
104104
if err != nil {
105105
return 0, err
106106
}

submitqueue/extension/speculation/scorer/composite/scorer_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,14 @@ type fixedScorer struct {
3434
score float64
3535
}
3636

37-
func (f *fixedScorer) Score(_ context.Context, _ entity.Batch) (float64, error) {
37+
func (f *fixedScorer) Score(_ context.Context, _ entity.Batch, _ entity.SpeculationPathSet) (float64, error) {
3838
return f.score, nil
3939
}
4040

4141
// errorScorer always returns an error.
4242
type errorScorer struct{}
4343

44-
func (e *errorScorer) Score(_ context.Context, _ entity.Batch) (float64, error) {
44+
func (e *errorScorer) Score(_ context.Context, _ entity.Batch, _ entity.SpeculationPathSet) (float64, error) {
4545
return 0, fmt.Errorf("scorer failed")
4646
}
4747

@@ -102,7 +102,7 @@ func TestScorer_Score(t *testing.T) {
102102
for _, tt := range tests {
103103
t.Run(tt.name, func(t *testing.T) {
104104
s := New(testCfg, tt.scorers, tt.reduce, tally.NoopScope)
105-
got, err := s.Score(context.Background(), entity.Batch{})
105+
got, err := s.Score(context.Background(), entity.Batch{}, entity.SpeculationPathSet{})
106106
require.NoError(t, err)
107107
assert.InDelta(t, tt.want, got, 1e-9)
108108
})
@@ -114,7 +114,7 @@ func TestScorer_Score_ChildError(t *testing.T) {
114114
"error": &errorScorer{},
115115
"files": &fixedScorer{0.9},
116116
}, Min, tally.NoopScope)
117-
_, err := s.Score(context.Background(), entity.Batch{})
117+
_, err := s.Score(context.Background(), entity.Batch{}, entity.SpeculationPathSet{})
118118
require.Error(t, err)
119119
}
120120

@@ -143,7 +143,7 @@ func TestReduceFunc_ReceivesNames(t *testing.T) {
143143
"files": &fixedScorer{0.9},
144144
"deps": &fixedScorer{0.95},
145145
}, custom, tally.NoopScope)
146-
got, err := s.Score(context.Background(), entity.Batch{})
146+
got, err := s.Score(context.Background(), entity.Batch{}, entity.SpeculationPathSet{})
147147
require.NoError(t, err)
148148
assert.Equal(t, 0.9, got)
149149
assert.ElementsMatch(t, []string{"files", "deps"}, receivedNames)

0 commit comments

Comments
 (0)