Skip to content

feat(speculation): rank bestfirst on evidence Scorer - #684

Open
behinddwalls wants to merge 8 commits into
preetam/outcome-predictorfrom
preetam/outcome-predictor-wiring
Open

feat(speculation): rank bestfirst on evidence Scorer#684
behinddwalls wants to merge 8 commits into
preetam/outcome-predictorfrom
preetam/outcome-predictor-wiring

Conversation

@behinddwalls

@behinddwalls behinddwalls commented Sep 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Summary

Why?

bestfirst already ranks on one probability per dependency. Wiring a sibling Predictor would reintroduce the factory the RFC dropped. YAML also had to show evidence as the outer scorer, not factors hanging off heuristic. A factors map or nested base under heuristic/composite used to load and then be ignored, so a misplaced pathPassed looked configured while ranking stayed at 1.

What?

Pass the speculate path-set snapshot into Generate and into Score. Default scorer.type is evidence wrapping a nested base. Named factors overlay; a present base replaces wholesale. Top-level type: heuristic is rejected. Content scorers reject factors and a nested base the same way. Drop the predictor: profile key. bestfirst depends only on scorer.Scorer.

Test Plan

  • ✅ ./tool/bazel test //service/submitqueue/orchestrator/server:go_default_test //submitqueue/extension/speculation/generator/bestfirst:go_default_test //submitqueue/extension/speculation/speculator/standard:go_default_test
  • ✅ go test ./service/submitqueue/orchestrator/server/ -run TestLoadProfilesConfig_RejectsBadScorers

Stack

This was referenced Sep 7, 2026
Open
Open
@behinddwalls
behinddwalls force-pushed the preetam/outcome-predictor-wiring branch from caeb62b to 3c5ee44 Compare September 8, 2026 00:13
@behinddwalls
behinddwalls force-pushed the preetam/outcome-predictor-wiring branch 2 times, most recently from 61d1369 to 33212d5 Compare September 8, 2026 00:51
@behinddwalls
behinddwalls force-pushed the preetam/outcome-predictor-wiring branch from 33212d5 to a97225b Compare September 8, 2026 00:57
@behinddwalls behinddwalls mentioned this pull request Sep 8, 2026
Open
@behinddwalls
behinddwalls marked this pull request as ready for review September 8, 2026 01:09
@behinddwalls
behinddwalls requested review from a team and sbalabanov as code owners September 8, 2026 01:09
@behinddwalls
feat(speculation): wire predictor into speculation pipeline
56f89a1 
## Summary

### Why?

The predictor implementation is additive until the speculation pipeline supplies each dependency's run-local path evidence and uses the revised probability for ranking.

### What?

Thread path sets through the Generator contract and standard Speculator, replace `bestfirst`'s scorer dependency with the predictor, and compose the evidence predictor from per-queue YAML configuration in orchestrator profiles. Neutral default factors preserve scorer-only ranking when no factors are configured.

## Test Plan

- `bazel test //submitqueue/extension/speculation/generator/... //submitqueue/extension/speculation/speculator/... //service/submitqueue/orchestrator/server:go_default_test`
- `make check-gazelle`



# Conflicts:
#	submitqueue/extension/speculation/generator/bestfirst/bestfirst.go
#	submitqueue/extension/speculation/generator/bestfirst/bestfirst_test.go
#	submitqueue/extension/speculation/speculator/standard/standard_test.go

# Please enter the commit message for your changes. Lines starting
# with '#' will be kept; you may remove them yourself if you want to.
# An empty message aborts the commit.
#
# interactive rebase in progress; onto 0ff6ea7
# Last command done (1 command done):
#    pick 15b2fa2 # feat(speculation): wire predictor into speculation pipeline
# Next commands to do (4 remaining commands):
#    pick 60d6cc4 # docs(speculation): align predictor configuration terms
#    pick 7fa8183 # fix(speculation): enforce predictor wiring contract
# You are currently rebasing branch 'preetam/outcome-predictor-wiring' on '0ff6ea72'.
#
# Changes to be committed:
#	modified:   service/submitqueue/orchestrator/server/BUILD.bazel
#	modified:   service/submitqueue/orchestrator/server/config.go
#	modified:   service/submitqueue/orchestrator/server/config_test.go
#	modified:   service/submitqueue/orchestrator/server/profiles.go
#	modified:   service/submitqueue/orchestrator/server/profiles_test.go
#	modified:   submitqueue/extension/speculation/generator/bestfirst/BUILD.bazel
#	modified:   submitqueue/extension/speculation/generator/bestfirst/bestfirst.go
#	modified:   submitqueue/extension/speculation/generator/bestfirst/bestfirst_test.go
#	modified:   submitqueue/extension/speculation/generator/generator.go
#	modified:   submitqueue/extension/speculation/generator/mock/generator_mock.go
#	modified:   submitqueue/extension/speculation/speculator/standard/standard.go
#	modified:   submitqueue/extension/speculation/speculator/standard/standard_test.go
#
@behinddwalls
docs(speculation): align predictor configuration terms
80e088c 
## Summary

### Why?

The orchestrator configuration comments still described factors as odds multipliers and referenced fitting work removed from the RFC.

### What?

Describe factors directly as revisions to the scorer price and retain the RFC's neutral, positive-factor contract without changing configuration behavior.

## Test Plan

- ✅ `./tool/bazel test //submitqueue/extension/speculation/predictor/... //submitqueue/extension/speculation/generator/... //submitqueue/extension/speculation/speculator/... //service/submitqueue/orchestrator/server:go_default_test`
- ✅ `make check-gazelle`
@behinddwalls
fix(speculation): enforce predictor wiring contract
03569eb 
## Summary

### Why?

Configuration accepted infinite evidence factors even though the predictor rejects them, and the speculator composition test did not prove that path-set evidence reaches the Generator.

### What?

Reject non-finite factors during profile loading, cover infinite YAML values, and require the exact path-set snapshot in the Generator wiring expectation.

## Test Plan

- ✅ `make fmt`
- ✅ `./tool/bazel test //submitqueue/extension/speculation/predictor/... //submitqueue/extension/speculation/generator/... //submitqueue/extension/speculation/speculator/... //service/submitqueue/orchestrator/server:go_default_test`
@behinddwalls
fix(speculation): overlay queue predictor factors
9d1c087 
A queue predictor block now revises named factors instead of replacing the whole map, so defaults like pathFailed stay in force. Best-first tests rank pathFailed, cancelling, and merging through the evidence predictor rather than a stub.

# Conflicts:
#	submitqueue/extension/speculation/generator/bestfirst/bestfirst_test.go

# Please enter the commit message for your changes. Lines starting
# with '#' will be kept; you may remove them yourself if you want to.
# An empty message aborts the commit.
#
# interactive rebase in progress; onto 0ff6ea7
# Last commands done (4 commands done):
#    pick 7fa8183 # fix(speculation): enforce predictor wiring contract
#    pick f636c33 # fix(speculation): overlay queue predictor factors
# Next command to do (1 remaining command):
#    pick a97225b # docs(speculation): document predictor wiring
# You are currently rebasing branch 'preetam/outcome-predictor-wiring' on '0ff6ea72'.
#
# Changes to be committed:
#	modified:   service/submitqueue/orchestrator/server/config.go
#	modified:   service/submitqueue/orchestrator/server/config_test.go
#	modified:   submitqueue/extension/speculation/generator/bestfirst/bestfirst_test.go
#
@behinddwalls
docs(speculation): document predictor wiring
dc9298f 
Update generator and standard-speculator guides for the shared path-set snapshot and predictor-backed best-first ranking.
@behinddwalls
feat(speculation): rank bestfirst on evidence Scorer
73addbe 
Compose evidence around a nested base from YAML type evidence. Drop the sibling predictor block. Overlay named factors; replace base wholesale.
@behinddwalls
behinddwalls force-pushed the preetam/outcome-predictor-wiring branch from a97225b to 73addbe Compare September 11, 2026 16:47
@behinddwalls behinddwalls changed the title feat(speculation): wire predictor into speculation pipeline feat(speculation): rank bestfirst on evidence Scorer Sep 11, 2026
@behinddwalls
fix(speculation): reject ranking-layer heuristic fields
84ca524 
Buckets, components, and combine on the evidence scorer used to be ignored, so a migrated heuristic block ranked every batch at the default flat price. Fail those configs, replace base wholesale in tests, and construct evidence at profile build.
@behinddwalls
fix(speculation): reject ranking fields on content scorers
7068a1b 
## Summary

### Why?

A factors map or nested base under heuristic/composite used to load and then be ignored, so a misplaced pathPassed looked configured while ranking stayed at 1.

### What?

Fail those configs in normalizeContent, including composite components and a queue overlay base.

## Test Plan

✅ `go test ./service/submitqueue/orchestrator/server/ -run TestLoadProfilesConfig_RejectsBadScorers`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sbalabanov sbalabanov Awaiting requested review from sbalabanov sbalabanov is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@behinddwalls