Skip to content

feat(speculate): write the speculation tree in shadow mode#353

Open
behinddwalls wants to merge 2 commits into
mainfrom
preetam/int/speculate-tree
Open

feat(speculate): write the speculation tree in shadow mode#353
behinddwalls wants to merge 2 commits into
mainfrom
preetam/int/speculate-tree

Conversation

@behinddwalls

@behinddwalls behinddwalls commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

Why?

This is the first of four slices reproducing the tree-driven speculate/build/buildsignal rework as an incremental, always-shippable stack instead of one large commit. Each slice keeps the orchestrator working end to end so the e2e suite stays green throughout, and reviewers can follow the pipeline shift (direct publish -> tree -> prioritize -> build) one deliberate step at a time.

This slice only adds the speculation tree as bookkeeping: nothing downstream reads it yet, so the batch's forward step (when it builds, when it merges) is untouched — with one deliberate exception called out below: a liveness fix to the dependent wake-up that review of this slice surfaced.

What?

speculate's Controller now takes an enumerator, path scorer, selector, and dependency-limit factory, and every Created/Scored/Speculating pass loads or creates the batch's entity.SpeculationTree, applies the scorer's and selector's outputs, and persists it only if something changed (the apply steps report whether they mutated anything, gating a version+1 conditional Update). The controller stays pure mechanics: which paths exist, how they score, and which are promoted or cancelled are the seams' decisions alone — the controller validates and records them. Tree creation is gated by the queue's dependency limit; the controller wraps the enumerator's structure-only paths into persisted entries itself — stamping Candidate, minting each path's immutable ID as {batchID}/path/{i}, and skipping structural duplicates as a contract violation — and a concurrent create race re-reads the winner's tree instead of erroring. Seam outputs are consumed by path ID: scores merged by ID (unknown IDs skipped, out-of-[0,1] values clamped, both logged), selector decisions resolved through the ID-keyed PathIndex with duplicates logged and skipped.

The pre-existing forward step is otherwise unchanged: Created/Scored batches CAS to Speculating and publish straight to build, and Speculating batches run the original tryFinalize (merge once every dependency has landed, cascade-fail on a failed dependency). Cancelling is the unmodified pre-existing flow.

Liveness fix in the forward step: a batch waiting on dependencies (in tryFinalize, or now in the dependency gate) was only ever woken when a dependency was cancelled — a dependency reaching Succeeded or Failed never re-published its dependents, because mergesignal routes every terminal transition back through speculate under the batch's own ID and the terminal branch only re-published conclude. The terminal branch now fans out to dependents for every terminal state, and failOnDependency wakes its own dependents right after the terminal CAS so failures cascade downstream. The fan-out publishes a wake for every listed dependent; a dangling reverse-index entry (left by an abandoned batch creation) surfaces as a not-found speculate message that dead-letters and is skipped by the DLQ reconciler — eliminating that class at the source, by creating the batch before the reverse-index update, is tracked in #354.

Cancel decisions on tree paths are recorded as status only (Cancelling on an in-flight build) — nothing in this controller calls a build runner, which is why it takes no buildrunner.Factory.

main.go wires the new seams with parity defaults: chain enumerates a single path per batch, the probability path scorer, the all-selector promotes every candidate, and a static dependency limit that is effectively ungated.

Test Plan

bazel test //submitqueue/... //service/... — 56/56 targets pass.

New unit coverage for the wake-up fix: dependent fan-out on every terminal state (with publish-order assertions), missing BatchDependent row surfaced as an invariant error, and dependent-publish failure after the terminal CAS nacking for redelivery convergence.

make gazelle && make fmt — no diffs beyond the intended BUILD file updates.

Issue

Part of the speculation rework. Interim fan-out caveats tracked in #354.

Issues

Stack

  1. @ feat(speculate): write the speculation tree in shadow mode #353
  2. feat(speculation): route builds through the tree via prioritize #365
  3. feat(speculation): reconcile dead paths and finalize from the tree #371

@behinddwalls behinddwalls force-pushed the preetam/int/speculate-tree branch from e278da8 to e494190 Compare July 14, 2026 08:46
@behinddwalls behinddwalls marked this pull request as ready for review July 14, 2026 14:13
@behinddwalls behinddwalls requested review from a team and sbalabanov as code owners July 14, 2026 14:13
behinddwalls added a commit that referenced this pull request Jul 14, 2026
…350)

## Summary
### Why?

Entity doc comments had started narrating pipeline choreography — which
controller writes a field, which seams read it, which stage never
touches it (e.g. "written only by the orchestrator's speculate
controller … the seams never write it"). That is control-flow knowledge
that belongs with the code that owns the behavior; restated on the
entity it goes stale as the pipeline evolves and distracts from what the
entity actually guarantees.

### What?

Adds entity guideline 7 to CLAUDE.md: docs describe what a type or field
*is* and its invariants (immutability, uniqueness scope, units, valid
range) — never which controller/stage/seam reads or writes it; ownership
and write-path rules live in controller/store/extension docs. Lifecycle
enums may define states in terms of pipeline stages where that is the
state's meaning, but must not name the components performing
transitions. Also trims the one instance in the merged entity files this
stack touches (`SpeculationPathBuild.Version`'s "version arithmetic is
owned by the controller" clause — the convention is already documented
in CLAUDE.md and the storage README).

## Test Plan
Doc-only change.

## Issues


## Stack
1. @ #350
1. #351
1. #353
## Summary

### Why?

Selection is per batch and blind to other batches, so it cannot ration a shared build budget: if every batch promoted generously, their combined demand could swamp CI. The speculation design closes that gap with a prioritization step that sees every candidate path across a queue's in-flight batches and admits only what fits the queue's concurrent-build budget. That reconcile is queue-scoped, not batch-scoped, so it gets its own pipeline stage rather than piggybacking on the per-batch speculate flow.

### What?

New `prioritize` topic (payload: `entity.QueueID`, partitioned by queue name) and controller. `entity.QueueID` is introduced here with ToBytes/QueueIDFromBytes, mirroring the BatchID payload pattern for queue-scoped stages. Each invocation is a full budget round for one queue: load every Speculating batch's speculation tree (skipping batches not yet speculated), flatten the queue-wide candidates (Selected / Prioritized / Building paths), hand them to the queue's `prioritizer.Prioritizer`, and apply the sparse decisions as captured intent — Promote flips Selected→Prioritized; Cancel on a Building path flips it to Cancelling (this controller never talks to the build system: the build stage owns runner interaction and enacts the persisted intent, retried on every build message until the build terminates); Cancel on a not-yet-building Prioritized path drops it straight to Cancelled; illegal decisions are logged and skipped so a policy bug cannot corrupt tree state. Path lookup for decision application lives on the entity (`SpeculationTree.PathIndex`), keyed by the path ID decisions carry; the controller maps each ID back to its tree via the candidates it loaded this round (never by parsing the ID), and treats a duplicate decision for the same path as a policy bug — logged and skipped like any other illegal decision. Each affected tree is persisted under its own optimistic lock (version arithmetic in the controller); any conflict nacks the round, which is safe to replay because decisions are recomputed from freshly read state — a redelivered round carries no memory of what a previous attempt picked and needs none, since already-promoted paths are counted as slot holders rather than re-promoted. After applying, the controller republishes to build for every batch whose tree carries work the build stage still has to enact — a Prioritized path with no build, or a Cancelling intent — healing dropped build messages idempotently.

Wiring: topic registered with an automatic DLQ pair. The DLQ reconciler re-arms the queue rather than terminalizing an entity: it logs the failure and republishes a fresh prioritize round under a distinct, deterministic message ID, so a queue whose only pending work is waiting on prioritization is not stranded when a round dead-letters. The requeue cannot poison-loop — a round carries only the queue name and recomputes from live state — and a persistently failing round cycles retry-ladder→DLQ→requeue at full-ladder cadence, visible in metrics, converging once the fault heals. The sticky prioritizer (never preempts) backed by a static admit-all parity limit is wired as the default per-queue profile. Tree-entity docs are cleaned to describe the data itself (states, invariants, uniqueness) rather than narrating which stage reads or writes what. Nothing publishes to the topic yet — the speculate rework turns it on.

## Test Plan

✅ `make gazelle && make fmt && make test` and `bazel build //service/submitqueue/orchestrator/...`. Controller unit tests cover: empty queue ack, missing-tree skip, promote transition + versioned update + build republish, illegal-decision skip, cancel-on-building capturing intent (Cancelling persisted, build republished, no runner dependency at all), cancel-on-prioritized dropping straight to Cancelled, version-mismatch and prioritizer errors nacking, and republish for pre-existing prioritized paths with zero new decisions. DLQ reconciler tests cover the requeued round's ID/partition/payload, publish-failure nack, and malformed/empty payload rejection. Entity tests cover `SpeculationTree.PathIndex` (order-sensitive base matching, empty tree).
## Summary

### Why?

This is the first of four slices reproducing the tree-driven speculate/build/buildsignal rework as an incremental, always-shippable stack instead of one large commit. Each slice keeps the orchestrator working end to end so the e2e suite stays green throughout, and reviewers can follow the pipeline shift (direct publish -> tree -> prioritize -> build) one deliberate step at a time.

This slice only adds the speculation tree as bookkeeping: nothing downstream reads it yet, so the batch's forward step (when it builds, when it merges) is untouched — with one deliberate exception called out below: a liveness fix to the dependent wake-up that review of this slice surfaced.

### What?

speculate's Controller now takes an enumerator, path scorer, selector, and dependency-limit factory, and every Created/Scored/Speculating pass loads or creates the batch's entity.SpeculationTree, applies the scorer's and selector's outputs, and persists it only if something changed (the apply steps report whether they mutated anything, gating a version+1 conditional Update). The controller stays pure mechanics: which paths exist, how they score, and which are promoted or cancelled are the seams' decisions alone — the controller validates and records them. Tree creation is gated by the queue's dependency limit; the controller wraps the enumerator's structure-only paths into persisted entries itself — stamping Candidate, minting each path's immutable ID as `{batchID}/path/{i}`, and skipping structural duplicates as a contract violation — and a concurrent create race re-reads the winner's tree instead of erroring. Seam outputs are consumed by path ID: scores merged by ID (unknown IDs skipped, out-of-[0,1] values clamped, both logged), selector decisions resolved through the ID-keyed PathIndex with duplicates logged and skipped.

The pre-existing forward step is otherwise unchanged: Created/Scored batches CAS to Speculating and publish straight to build, and Speculating batches run the original tryFinalize (merge once every dependency has landed, cascade-fail on a failed dependency). Cancelling is the unmodified pre-existing flow.

Liveness fix in the forward step: a batch waiting on dependencies (in tryFinalize, or now in the dependency gate) was only ever woken when a dependency was cancelled — a dependency reaching Succeeded or Failed never re-published its dependents, because mergesignal routes every terminal transition back through speculate under the batch's own ID and the terminal branch only re-published conclude. The terminal branch now fans out to dependents for every terminal state, and failOnDependency wakes its own dependents right after the terminal CAS so failures cascade downstream. The fan-out publishes a wake for every listed dependent; a dangling reverse-index entry (left by an abandoned batch creation) surfaces as a not-found speculate message that dead-letters and is skipped by the DLQ reconciler — eliminating that class at the source, by creating the batch before the reverse-index update, is tracked in #354.

Cancel decisions on tree paths are recorded as status only (Cancelling on an in-flight build) — nothing in this controller calls a build runner, which is why it takes no buildrunner.Factory.

main.go wires the new seams with parity defaults: chain enumerates a single path per batch, the probability path scorer, the all-selector promotes every candidate, and a static dependency limit that is effectively ungated.

## Test Plan

`bazel test //submitqueue/... //service/...` — 56/56 targets pass.

New unit coverage for the wake-up fix: dependent fan-out on every terminal state (with publish-order assertions), missing BatchDependent row surfaced as an invariant error, and dependent-publish failure after the terminal CAS nacking for redelivery convergence.

`make gazelle && make fmt` — no diffs beyond the intended BUILD file updates.

## Issue

Part of the speculation rework. Interim fan-out caveats tracked in #354.
@behinddwalls behinddwalls force-pushed the preetam/int/prioritize-stage branch from aeb8110 to 61ff6cc Compare July 14, 2026 17:28
@behinddwalls behinddwalls force-pushed the preetam/int/speculate-tree branch from e494190 to e30249e Compare July 14, 2026 17:28
behinddwalls added a commit that referenced this pull request Jul 14, 2026
## Summary
### Why?

Selection is per batch and blind to other batches, so it cannot ration a
shared build budget: if every batch promoted generously, their combined
demand could swamp CI. The speculation design closes that gap with a
prioritization step that sees every candidate path across a queue's
in-flight batches and admits only what fits the queue's concurrent-build
budget. That reconcile is queue-scoped, not batch-scoped, so it gets its
own pipeline stage rather than piggybacking on the per-batch speculate
flow.

### What?

New `prioritize` topic (payload: `entity.QueueID`, partitioned by queue
name) and controller. `entity.QueueID` is introduced here with
ToBytes/QueueIDFromBytes, mirroring the BatchID payload pattern for
queue-scoped stages. Each invocation is a full budget round for one
queue: load every Speculating batch's speculation tree (skipping batches
not yet speculated), flatten the queue-wide candidates (Selected /
Prioritized / Building paths), hand them to the queue's
`prioritizer.Prioritizer`, and apply the sparse decisions as captured
intent — Promote flips Selected→Prioritized; Cancel on a Building path
flips it to Cancelling (this controller never talks to the build system:
the build stage owns runner interaction and enacts the persisted intent,
retried on every build message until the build terminates); Cancel on a
not-yet-building Prioritized path drops it straight to Cancelled;
illegal decisions are logged and skipped so a policy bug cannot corrupt
tree state. Path lookup for decision application lives on the entity
(`SpeculationTree.PathIndex`), keyed by the path ID decisions carry; the
controller maps each ID back to its tree via the candidates it loaded
this round (never by parsing the ID), and treats a duplicate decision
for the same path as a policy bug — logged and skipped like any other
illegal decision. Each affected tree is persisted under its own
optimistic lock (version arithmetic in the controller); any conflict
nacks the round, which is safe to replay because decisions are
recomputed from freshly read state — a redelivered round carries no
memory of what a previous attempt picked and needs none, since
already-promoted paths are counted as slot holders rather than
re-promoted. After applying, the controller republishes to build for
every batch whose tree carries work the build stage still has to enact —
a Prioritized path with no build, or a Cancelling intent — healing
dropped build messages idempotently.

Wiring: topic registered with an automatic DLQ pair. The DLQ reconciler
re-arms the queue rather than terminalizing an entity: it logs the
failure and republishes a fresh prioritize round under a distinct,
deterministic message ID, so a queue whose only pending work is waiting
on prioritization is not stranded when a round dead-letters. The requeue
cannot poison-loop — a round carries only the queue name and recomputes
from live state — and a persistently failing round cycles
retry-ladder→DLQ→requeue at full-ladder cadence, visible in metrics,
converging once the fault heals. The sticky prioritizer (never preempts)
backed by a static admit-all parity limit is wired as the default
per-queue profile. Tree-entity docs are cleaned to describe the data
itself (states, invariants, uniqueness) rather than narrating which
stage reads or writes what. Nothing publishes to the topic yet — the
speculate rework turns it on.

## Test Plan
✅ `make gazelle && make fmt && make test` and `bazel build
//service/submitqueue/orchestrator/...`. Controller unit tests cover:
empty queue ack, missing-tree skip, promote transition + versioned
update + build republish, illegal-decision skip, cancel-on-building
capturing intent (Cancelling persisted, build republished, no runner
dependency at all), cancel-on-prioritized dropping straight to
Cancelled, version-mismatch and prioritizer errors nacking, and
republish for pre-existing prioritized paths with zero new decisions.
DLQ reconciler tests cover the requeued round's ID/partition/payload,
publish-failure nack, and malformed/empty payload rejection. Entity
tests cover `SpeculationTree.PathIndex` (order-sensitive base matching,
empty tree).

## Issues


## Stack
1. #350
1. @ #351
1. #353
@behinddwalls behinddwalls force-pushed the preetam/int/speculate-tree branch from e30249e to 61c005d Compare July 14, 2026 17:30
@behinddwalls behinddwalls changed the base branch from preetam/int/prioritize-stage to main July 14, 2026 17:30
@behinddwalls behinddwalls force-pushed the preetam/int/speculate-tree branch from 61c005d to e30249e Compare July 14, 2026 20:10
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.

1 participant