Skip to content

feat(speculation): generator contract and bestfirst impl - #446

Open
behinddwalls wants to merge 1 commit into
preetam/speculation-speculatorfrom
preetam/speculation-generator
Open

feat(speculation): generator contract and bestfirst impl#446
behinddwalls wants to merge 1 commit into
preetam/speculation-speculatorfrom
preetam/speculation-generator

Conversation

@behinddwalls

@behinddwalls behinddwalls commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

Why?

The standard Speculator (#445) has to rank candidate paths and then decide how many to fund. This branch is the ranking half.

It is deliberately not controller-facing — the controller knows only the Speculator contract — so there is no Config or Factory; a Generator is chosen when the standard Speculator is constructed. It is a pull-based stream rather than a slice because a head's path count is exponential in its unfinished dependencies while the budget funds a handful.

What?

Adds generator (the contract) and generator/bestfirst (the first implementation).

Open returns a PathIterator; Next yields one candidate at a time in descending score. All scoring happens in Open, so Next never calls the scorer and never fails.

A path scores as the product of its dependencies' landing chances. A dependency that already finished is a fact, not a guess — pinned into the path and dropped from the search, halving the work each time one resolves.

Lazy at two levels. A cross-head heap holds each head's current offer — a real, already-built path, never an estimate — so the top of the heap is the best path in the queue. Each head in turn builds its next path only once its current one is taken. Pulling k candidates builds about k paths, not 2ⁿ.

The best path is not "bet everything lands." Each guess takes the likelier outcome, so a dependency that will probably fail is excluded. Other paths flip guesses, each flip costing a known factor. The walk gives every path exactly one parent, so nothing is built twice and no visited set is needed, and children never outrank parents — which is what makes the heap hand paths out in true score order.

Two smaller things: the depth-bound check now runs before any scoring (a skipped head costs no scorer calls), and both heaps share one generic rankedHeap over container/heap instead of repeating the interface boilerplate.

Test Plan

bazel test //submitqueue/extension/speculation/... (23 tests), make lint, make check-gazelle

The strongest check is a brute-force cross-check: over randomized chance vectors for 1–6 dependencies (150 trials, fixed seed), the lazy walk must emit exactly the 2ⁿ paths with the same scores, in the same order, as eager enumeration.

Laziness is asserted directly through a counter of paths built: a 4096-path head builds at most 3 at Open and 9 after three pulls; consuming one head's candidate leaves a sibling's stream untouched; Next makes no scorer calls; a head over the depth bound costs zero.

The rest pins ordering and semantics — exact emitted sequences across heads, the best path excluding a likely-to-fail dependency, every combination appearing exactly once, deterministic tie-breaking, and terminal filtering across all three terminal statuses (and deliberately not for pending, building, or cancelling).

Notes for the reviewer

headStream.generated and iterator.streams are read only by tests — the first counts paths built, the second is the streams the iterator owns while the merge runs off the heap. Both are commented as such and easy to drop.

Stack

  1. feat(entity): speculation path and run entities #444
  2. feat(speculation): speculator extension contract #445
  3. @ feat(speculation): generator contract and bestfirst impl #446
  4. feat(speculation): allocator contract and sticky impl #450
  5. feat(speculation): standard composed speculator #451

@behinddwalls
behinddwalls force-pushed the preetam/speculation-generator branch 2 times, most recently from 437af45 to c8ec6b1 Compare July 27, 2026 21:21
Add submitqueue/extension/speculation/generator, the candidate-stream composition point (Generator/PathIterator) the standard Speculator pulls from, plus the bestfirst implementation and mocks.

bestfirst ranks each path by the product of its dependencies' landing chances (scored through an injected scorer) and generates lazily at two levels. A cross-head heap holds one real, non-terminal candidate per head — never an optimistic upper bound — so popping it yields the globally best-ranked path; each head is itself a lazy stream that materializes its next candidate only once the current one is consumed. Pulling k candidates materializes O(k) paths however large the space behind them, so a head with twelve unresolved dependencies costs no more at the front of the queue than one with two.

Within a head, the best path takes the preferred (likelier) bet on every unresolved dependency, and every other path flips some subset of those choices, each flip multiplying the score by that choice's penalty (alternative over preferred). Subsets are walked by a canonical expansion under which every subset has exactly one parent, so each combination is generated exactly once with no visited set, and a child never outranks its parent — which is what lets the heap pop in true descending order.

Also corrects the generator README: the leading path is the preferred-bet path, not the all-included one. The two differ whenever a dependency is likelier to fail than to land, so the old wording was wrong for any below-even dependency.

The two heaps (across heads, and within a head) order candidates identically, so they share one generic rankedHeap over the standard library's container/heap rather than repeating the heap.Interface boilerplate twice.

Test coverage is 22 cases, including the exact emitted sequence across heads, the preferred bet on a below-even dependency, mixed pinned dependencies dropping out of the search, terminal filtering across every terminal status (and deliberately not filtering the non-terminal ones), the materialization counts behind the laziness claim, deterministic tie-breaking, and a randomized cross-check of the whole walk against brute-force enumeration. The generator README gains a worked walkthrough tracing the frontier pop by pop.

A head over the depth bound is now recognised from its dependencies' states alone, before anything is scored. Previously every unfinished dependency was scored and only then was the head found to be over the bound. Scoring can be expensive and a head we skip should not pay for it. One visible consequence: a scorer error on a head that was going to be skipped no longer fails Open.
@behinddwalls
behinddwalls force-pushed the preetam/speculation-generator branch from c8ec6b1 to 3fb7e2b Compare July 27, 2026 23:14
@behinddwalls
behinddwalls marked this pull request as ready for review July 27, 2026 23:17
@behinddwalls
behinddwalls requested review from a team and sbalabanov as code owners July 27, 2026 23:17
@behinddwalls
behinddwalls force-pushed the preetam/speculation-generator branch 2 times, most recently from 5e0bdbd to 3fb7e2b Compare July 28, 2026 19:13

**Laziness at two levels.** Across heads, a heap holds each head's current offer — a real, already-built path, never an estimate — so the top of the heap is the best path in the queue. Within a head the same shape repeats: it keeps a small heap, hands out its best, and builds only the one or two paths that come after it. Pulling *k* candidates builds about *k* paths, whatever the size of the set behind them.

`bestfirst` discards nothing: pull long enough and every path within the depth bound comes out. It does no conflict relaxation (`dropped` bets) — that is the `Speculator`'s call, not the generator's.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why need a depth bound? Should it be simply guarded by build budget?

// only what is pulled.
type PathIterator interface {
// Next yields the next candidate in the generator's order. ok is false once
// the coherent, depth-capped space is exhausted. Candidates never repeat and

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"depth-capped space is exhausted"
contradicts to the contract above saying that generator yields candidates however it likes

type PathIterator interface {
// Next yields the next candidate in the generator's order. ok is false once
// the coherent, depth-capped space is exhausted. Candidates never repeat and
// never contradict a resolved fact.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably document, may be example, what "resolved fact" is (like failed spec path)


// defaultLandingChance is what we assume for a dependency we cannot score
// because it is not among the live batches: a coin flip.
const defaultLandingChance = 0.5

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd do default to at least 95%, or what is practically observed conservative success rate is

// chanceOf remembers each dependency's chance of landing, so a dependency
// that several heads wait on is only scored once.
chances := make(map[string]float64)
chanceOf := func(depID string) (float64, error) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be hard to read vs standalone unexported functions

slot int
// alternativeBet is the bet used when this guess is flipped.
alternativeBet entity.DependencyBetType
// preferred is the chance of the likelier outcome, so always >= 0.5.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but less than 1? should say explicitly

case entity.BatchStateSucceeded:
// Already landed, so including it is a fact, not a guess.
return entity.BetIncluded, true
case entity.BatchStateFailed, entity.BatchStateCancelled:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it is still "cancelling", we probably should proactively exclude as well?

// final. An unfinished dependency has no forced bet and has to be guessed at.
func settledBet(state entity.BatchState) (entity.DependencyBetType, bool) {
switch state {
case entity.BatchStateSucceeded:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if can probably be even "landed" - for bypass large diffs

// piles — ones that already finished, whose bet is a known fact, and unfinished
// ones, which become choices to guess about — and then builds the head's best
// path so the stream has something to offer. It returns ok=false when the head
// has more unfinished dependencies than the depth bound allows.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so it combines multiple iterators into the one final one?
isn't it simpler to build a dependency graph instead and traverse it with a single iterator?

return s
}
}
t.Fatalf("no stream for head %q", headID)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer "require" on nil/nonnil return value

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants