docs(entity): entity docs describe data, not controller choreography#350
Merged
Conversation
## 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.
JamyDev
approved these changes
Jul 14, 2026
behinddwalls
temporarily deployed
to
stack-rebase
GitHub Actions
Inactive
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
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
2 participants
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?
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