feat(speculate): write the speculation tree in shadow mode#353
Open
behinddwalls wants to merge 2 commits into
Open
feat(speculate): write the speculation tree in shadow mode#353behinddwalls wants to merge 2 commits into
behinddwalls wants to merge 2 commits into
Conversation
This was referenced Jul 14, 2026
e278da8 to
e494190
Compare
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.
aeb8110 to
61ff6cc
Compare
e494190 to
e30249e
Compare
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
e30249e to
61c005d
Compare
61c005d to
e30249e
Compare
This was referenced Jul 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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