Phase 2: the accountable frame (#713)#738
Merged
Merged
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @macanderson, your pull request is larger than the review limit of 150000 diff characters
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Owner
Author
|
Merge-viability check: Phase 2 and Phase 3 integrate cleanly. These two branches were built concurrently, so I trial-merged them rather than assume they compose. Result:
Ownership held in both directions: neither PR touched a single file on the other's do-not-touch list. Merge order does not matter for correctness. Whichever lands second needs the one-line |
macanderson
marked this pull request as ready for review
July 26, 2026 22:35
macanderson
added a commit
that referenced
this pull request
Jul 26, 2026
…ete baseline entry (#739) **`main` is red, and in two ways.** The second one is the real problem. ## 1. `main` does not compile `cargo check -p stella-cli` fails with nine `E0308`s: ``` error[E0308]: mismatched types --> stella-cli/src/main.rs:1140:17 = note: `MemoryCmd` and `memory_cmd::MemoryCmd` have similar names, but are actually distinct types ``` **Cause: two PRs that each merged cleanly and are each individually correct.** - **#733** *moved* `MemoryCmd` into `memory_cmd.rs`, to get `main.rs` under its size ratchet. - **#735** was cut from a `main` that predated that move, so it carried its own copy of the enum — and added two new variants, `Compact` and `Index`, to *that* copy. Git merged both without a textual conflict. The result declares the enum twice: `Command::Memory` holds a `memory_cmd::MemoryCmd`, while the dispatch arms match on the stale `crate::MemoryCmd`. This is the failure mode a textual merge is worst at — neither side touched the same lines, so nothing conflicted, and the incompatibility surfaces as a type error two files away from either change. **Fix, in the direction #733 intended:** move #735's `Compact` and `Index` variants onto `memory_cmd::MemoryCmd` with their doc comments intact, delete the stale duplicate from `main.rs`, and qualify the dispatch arms. ## 2. An obsolete baseline entry This is what CI reported *first*, and it masked the compile error — `check-file-size` runs before clippy and exits first. #733 deleted `main.rs`'s baseline entry when the file fell to 1337 lines; #735 re-added it at 1816. The file is now 1421 — simultaneously under the limit and exempted from it, which the ratchet rejects by design, since an exemption must not outlive the problem it covered. ## Verification - `make gate` green: **exit 0, 3670 tests**, `check-file-size: OK — 456 Rust files, none over 1500 lines except 27 grandfathered (none grew).` - `stella memory --help` lists all **nine** subcommands, so #735's `compact` and `index` survive the move — the variants were relocated, not dropped. Unblocks #736 and #738, which cannot get a meaningful CI signal against a `main` that does not build.
ADR 0006 predates the receipts plane. Read literally, "a new aggregate stella builds" sanctions a second immutable record of one turn's context, and two such records can disagree where one cannot. The receipts manifest already records ordered, content-addressed, cache-zoned, cost-attributed blocks and reconstructs byte-exactly; what it lacked was recall decomposition, per-item provenance, and a frame-level identity. Those are additions to one artifact. Also corrects the phase. The Consequences section assigns this to Phase 3 under the eleven-phase roadmap; the four-phase plan makes the accountable frame Phase 2 and the typed loop Phase 3. The original text is left intact with the correction stated above it rather than rewritten, so the record of what was decided when survives. The amendment lands before the code it governs, so the code cannot contradict a document that was never updated. Refs #713
…re move) event.rs sits at its file-size ceiling (2,636 lines, grandfathered in the ratchet baseline), and Phase 2 must add fields to the manifest and the frame reference. Growing a file at its ceiling means raising the ceiling, which the ratchet exists to make hard; splitting it first is the alternative the guard is designed to encourage. The seam is the one already there: BlockKind, CacheZone, ContextFrameRef, BlockOrigin, ManifestEntry, ProviderShare, ContextProviderUsage and ContextUsage are the persisted receipt of one model call. They share a forward-compatibility discipline the rest of the event vocabulary does not need, and they are the only types in the file that a store reads back rather than a renderer consuming live. Verbatim move, no wire change: the types are re-exported from `event` as well as the crate root, so `stella_protocol::event::BlockKind` and `stella_protocol::BlockKind` both still resolve. Tests are untouched and stay where they are — they exercise these types through AgentEvent, which is where they belong. Refs #713
…nifest Deliverable 4 of Phase 2 (#713). ADR 0006 as amended says the compiled frame IS the step manifest, so this adds two fields to the manifest rather than a second aggregate: `compiled_frame_id` and `frame_hash`, computed over a canonical body the receipt already has in hand. The hash reuses the scheme ADR 0004 ratified — `record_hash`: strip nulls, RFC 8785 JCS, sha256. Not a new canonicalizer. A codebase with two hashing schemes has two answers to "are these the same bytes", and the existing one is already golden-pinned. What the preimage excludes is the substance of the change. `provider`, `model`, `call_seq`, `effective_budget_tokens`, `calibration_factor`, `estimated_input_tokens`, and each entry's `resident_since_step` are all absent — by construction, as a struct that does not have them, so adding one back is a visible edit rather than a filter someone forgets to update. They are excluded because they are accounting ABOUT the call, not what entered the prompt: a provider fallback mid-session, or a calibration factor that learns across a turn, would otherwise move the hash of an identical prompt and make the determinism gate unmeetable in exactly the situations it exists for. Gated on `context.lifecycle.enabled`, which was inert until now — the setting existed and nothing read it. It is threaded CLI → EngineConfig → ReceiptLedger, and while off the manifest is byte-identical to what it was: `compiled_frame` is `skip_serializing_if = "Option::is_none"`, so the key does not appear on the wire at all. That is what "default off" has to mean for a persisted wire type with existing readers. Store: v15 → v16 claims the slot reserved for this phase — two nullable columns on `step_receipt`, not a `compiled_frame` table, for the same reason the event has fields rather than a sibling event. A reader must treat a null frame hash as "the lifecycle was off", never as a damaged receipt; the migration test pins that a pre-v16 row survives readable. `stella inspect` grows a FRAME column, shown only when at least one call in the execution has one. Two calls displaying the same id saw byte-identical context — that comparison is the column's whole job. Tests: a pinned golden JCS preimage and digest (the shape context_event.rs set), determinism across independent ledgers, invariance under each volatile field and under residency, and sensitivity to the prompt and the step — a hash that never moves is trivially deterministic and proves nothing. The file-size baseline moves: event.rs drops 237 lines from the preceding pure move, driver.rs grows 7 for a config field, store tests grow 2 for the version assertion. Both increases are the irreducible kind the ratchet's --update path exists for. Refs #713
… for it Deliverables 1 and the block half of 2 for Phase 2 (#713). Four BlockKinds have been defined since the receipts plane landed and none of them was ever produced. Every User-role message became one UserGoal block, which meant a receipt could say a turn had recall but not what was in it — and, worse, attributed the engine's own compaction notice and stuck-loop warning to the person. Now: - The overflow summary and the stuck-loop steer are classified by the markers the driver already writes, so `Summary` and `Steered` are emitted for the driver's text and `UserGoal` means what it says. - The assembled recall block splits into one `RecalledFrame` block per recalled item, each carrying the `nod_…` record it came from and the human label it was cited under. That is the join #713 calls "the one that makes every later question answerable" — a frame reference now resolves to a record, which is the whole of per-item provenance at the block layer. - Attachments get blocks. They were counted by the estimator and decomposed nowhere, so a multimodal turn's manifest read materially under the same event's estimated_input_tokens. Two things were load-bearing and are worth stating plainly. **The split had to be byte-recoverable.** Reconstruction rebuilds a message by appending its blocks' preimages in manifest order, so a split that lost or duplicated a separator would produce a transcript that looks plausible and is not what the model saw — the exact failure this plane exists to prevent. The segments are contiguous slices that partition the message, pinned by a property test over adversarial separators (CRLF, blank lines, a frame body whose own line starts with "- "). `append_block` appends instead of assigning; for the single-block kinds that is identical, since the message was just created empty. **`empty_message_for` mapped `summary` to an assistant message.** The comment said "the summary is spliced as assistant-authored". It is not — the summarizer splices `CompletionMessage::user`. That was a latent bug, invisible because nothing had ever emitted a summary block; emitting one would have made it a wrong reconstruction. Corrected here, affecting no stored row. Attachment payloads never reach the journal. At rest an attachment is a path and metadata, which is the preimage and is reconstructable. A hydrated one carries inline base64; its identity still hashes the real value but the local preimage is withheld, so the bytes cannot reach stream-json stdout. Such a block reports as unresolved, which is the honest answer. Block ids moved for the reclassified kinds — a block's id hashes its kind tag. That is accepted, not worked around: the ids that moved were wrong. Cache zones did not move (spec §5.1); splitting only adds blocks inside the volatile region, and a test pins the head and tail. The gate's most important signal is the end-to-end test: a turn with a summary, a decomposed recall block, an attachment, and a steer, driven through the real emitter into a real store, reconstructs byte-exact and verified. RECALL_MARKER moved to stella-core (re-exported by the CLI) because the engine cannot recognize a recall block without it, and two spellings of one marker is a decomposition that silently stops firing. Refs #713
…surface Deliverables 2 and 3 of Phase 2 (#713). **The digest was minted and then dropped.** `frame_from_node` has always declared `content_digest` — the sha256 of exactly the bytes that become the frame's content — and the CGP→pipeline projection threw it away, which is why every `ContextFrameRef` on every recall event carried `content_digest: null`. `RecalledFrame` grows the field and the projection carries it. Carried, never recomputed: this projection trims the content, so a locally derived hash would disagree with the provider's, and a digest that does not match the record it claims to identify is worse than none. A frame whose provider declared none keeps `None` — per context-reuse §1 such a frame is not verifiable and must be re-queried, so the absence is information. `block_id` stays null on the pipeline path, and that is a fact rather than a remaining gap. Those frames feed the planner's structured split-context prompt; they are never rendered into a message, so no context block exists for them to name, and fabricating an id would point at a block no manifest ever cited. The join to a block runs through the record — `BlockOrigin::memory_id`, populated by the recall decomposition in the previous commit. **Four of five recall paths reported nothing.** `ContextRecall` fired from the pipeline only. The one-shot run, the interactive REPL, `/goal`, and the Command Deck all recalled and emitted nothing, so the surfaces users actually touch were invisible to `stella inspect` — the event existed and described the least used path. The cause was structural: `recall_block` rendered a String and dropped the `Recall` behind it, so those callers had nothing left to emit. It now returns `RecalledBlock { text, recall }`, and the convenience form is `#[cfg(test)]` — a caller that only wants the string is a caller that reports nothing, and leaving that shape reachable would let the next recall site reintroduce the defect silently. The projection into the event lives once, on `Recall::telemetry_event`, and the pipeline now calls it too. Copying it into five sites would have made the event's shape a convention rather than a definition, and the first divergence would be a provider mix only some surfaces counted. Recall necessarily runs before a turn's event channel exists — its frames are part of the messages the turn is built from — so four paths carry the event forward and emit it as the turn's first event, ahead of the stage boundaries. That ordering is deliberate: the context was assembled before the turn began. The one-shot path already has a live stream and emits in place. Refs #713
… evict Deliverable 5 of Phase 2 (#713). Neither concept existed: `rg 'SelectionReason|non_evictable'` returned nothing, and `pack_to_budget` was strictly rank-ordered with no notion of "the caller asked for this one". `SelectionReason` names the mechanism that put a candidate in front of the budget — anchored, ranked, or lexical fallback. The vocabulary is deliberately small and names mechanisms, which are stable, rather than rationales, which drift into prose. **Anchored is required.** An anchor is a file the goal named verbatim: the one signal in the whole ranking that came from the user rather than from a heuristic, and no similarity score is a better judge of relevance than that. ADR 0006 says required items cannot be evicted by ranking, so: - Required items take their token budget in a first pass, before any ranked candidate competes for it. - `max_frames` bounds the *ranked* admissions only. Counting required items against the frame budget would let it evict one for FrameCount, which is precisely the eviction the ADR forbids. The result stays bounded — anchors are capped where they are extracted, and the token budget still applies. - The one permitted drop is a required item that exceeds the entire budget, so no ordering could have admitted it. That is `RequiredOverBudget`, its own reason rather than folded into TokenBudget: collapsing them would hide the only case where the caller's own instruction was overruled behind a reason that reads as ordinary budget pressure. The kept set is emitted in the original rank order. A packer that returned required-first would reorder the rendered block whenever an anchor appeared, which is a cache-prefix change (spec §5.1) bought for nothing. **The second packer is fixed too, and it was the one that mattered.** `recall_via_host` re-packs across providers with its own `max_frames`/ `max_tokens` and had no drop report of any kind — so a required item that survived `pack_to_budget` was silently dropped here the moment a higher-scoring frame from another provider filled the count first. The guarantee held in one packer and was undone in the other, invisibly. It now runs the same required-first pass and returns a `dropped` report; its `break` on a full count became a `continue` with a report, because the old loop stopped walking and everything after it vanished uncounted as well as unkept. A dropped anchor is surfaced to the user with the setting that would fix it. The reason rides the frame's **provenance**, which is the only channel that survives the CGP seam — `RecallResult` → `ContextQueryResult` keeps the frames and a collapsed truncated/dropped_estimate and nothing else, so a reason recorded anywhere else is a reason a CGP consumer never sees. It is also how the host reads required-ness back. A provider that declares no reason is not required: an external provider must not be able to claim exemption from the host's budget by omitting a field. Refs #713
The rebase onto main brings in #732, which rewrote the same function this phase's deliverable 1 rewrites. Both survive; neither was reverted to make the merge easy. CONTENT IS A Cow, NOT AN OWNED String OR A BARE BORROW #732 made `BlockDraft.content` a borrowed `&'a str` for a specific reason: a block's bytes ride `BlockRegistered`, which fires once per block per turn, so owning them meant re-cloning the system prefix — the largest gap block there is — on every step for the rest of the turn. Deliverable 1 splits recall into per-frame blocks, whose text is a borrowed slice of the same message and needs no allocation, but it also decomposes attachments, whose preimage is a freshly serialized JSON string that nothing owns. `Option<Cow<'a, str>>` is the resolution that concedes nothing: the prefix and every recall segment stay borrowed, and only the one site that genuinely mints bytes allocates. Reverting to `Option<String>` would have merged cleanly and silently undone a merged performance fix. ATTACHMENTS GO THROUGH THE MEMO, NOT AROUND IT Deliverable 1 built attachment blocks with a `BlockDraft::new` constructor. #732 removed that constructor — construction is inline, through a `push` closure that consults `BlockDigestCache`. Attachments now route through `push` like every other kind, so the memo covers them too; a hydrated attachment passes `None` for the local preimage, which keeps its inline base64 out of the event stream while still hashing the real value for identity. The two rewrites turned out to be complementary rather than competing: splitting a message into more blocks makes the position-keyed memo finer, never absent, so a decomposed recall block is still hashed once per turn. RECEIPTS.RS IS SPLIT The merged file reached 1532 lines, over the 1500 ratchet, and a baseline entry is not allowed for a file that crosses it. The inline `mod tests` moves to a sibling `receipts/tests.rs` — a pure move, no assertion touched — taking the module to 949. The signal that matters is green: both byte-exact reconstruction tests pass with recall now decomposed — `reconstruct::tests::reconstructs_a_tool_round_trip_byte_exact_and_verified_from_the_fold` and `agent::persistence::stream_tests::end_to_end_receipt_reconstructs_the_step_byte_exact_from_the_persisted_store`. `make gate` green: exit 0, 3698 tests, check-file-size OK.
macanderson
force-pushed
the
adaptive-context-phase2
branch
from
July 26, 2026 23:18
e5c311e to
bb48660
Compare
macanderson
added a commit
that referenced
this pull request
Jul 26, 2026
…ader (#745) **`main` does not compile.** ``` error[E0432]: unresolved import `tuning::session_lifecycle_enabled` --> stella-cli/src/memory.rs:78:9 ``` ## Cause Phase 2 (#738) and Phase 3 (#736) were built in parallel, and each independently needed to read `context.lifecycle.enabled`. Neither could see the other, so both wrote the reader — with a **byte-identical body** and the same fail-closed reasoning, differing only in name and visibility: | | | |---|---| | Phase 3 | `pub(super) fn lifecycle_enabled(..)` | | Phase 2 | `pub fn session_lifecycle_enabled(..)` | Phase 3 merged first. Phase 2 merged second **from a rebase that predated it**, so `tuning.rs` kept Phase 3's function while `memory.rs` kept Phase 2's re-export of a name that no longer exists. Neither PR was wrong, and neither conflicted textually with the `main` that existed when it merged. ## Fix The resolution both branches were converging on anyway: one reader, `pub` because Phase 2's engine-config builder calls it from outside the module, documented with both phases' reasoning, and one call site updated. ## The pattern, since this is the third repair this week - #739 — #733 moved `MemoryCmd`; #735 was cut from an older main and re-added it. Zero textual conflict, nine type errors. - This one — same shape, one level down. **Parallel branches that merge cleanly are not thereby compatible.** A branch that lands second is verified against the main it was *rebased onto*, not the main it *lands on*, and nothing in the tooling notices the difference. Re-verifying the second branch against the actual post-merge tree is the step that catches this, and it is far cheaper than a red main. ## Gate `make gate` green: **exit 0, 3801 tests**, `check-file-size: OK — 475 Rust files, none over 1500 lines except 25 grandfathered (none grew).`
Closed
6 tasks
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
1 participant
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.
Six deliverables, six commits,
make gategreen on each. Ships behindcontext.lifecycle.enabled(default off), which was inert before this —the setting existed and nothing read it.
Base is
worktree-adaptive-context-prelude, notmain.What each commit does
1.
docs(adr)— amend ADR 0006. Landed first, before the code it governs.The ADR predates the receipts plane; read literally, "a new aggregate stella
builds" sanctions a second immutable record of one turn's context, and two such
records can disagree where one cannot. The amendment records that the compiled
frame is the step manifest extended, names the volatile fields the hash
excludes, pins the hash to ADR 0004's scheme, and corrects the phase (the
Consequences section says Phase 3 under the eleven-phase roadmap; the
four-phase plan makes it Phase 2). Original text left intact with the
correction above it, so the record of what was decided when survives.
2.
refactor(protocol)— pure move, no behavior change.event.rswas atits file-size ceiling and Phase 2 must add fields to the manifest. The receipts
vocabulary (
BlockKind,CacheZone,ContextFrameRef,BlockOrigin,ManifestEntry,ProviderShare,ContextProviderUsage,ContextUsage) movesto
stella-protocol/src/receipt.rs, re-exported from botheventand thecrate root so every existing path resolves.
3. Deliverable 4 — the compiled frame.
compiled_frame_id+frame_hashon
StepManifest, computed over a canonical body via the existingrecord_hash(strip nulls → RFC 8785 JCS → sha256). Excluded as volatile, bybeing absent from the struct rather than filtered:
provider,model,call_seq,effective_budget_tokens,calibration_factor,estimated_input_tokens, per-blockresident_since_step. Store migrationv15→v16 (the reserved slot) adds two nullable columns to
step_receipt— not atable, for the same reason the event has fields rather than a sibling event.
stella inspectgrows a FRAME column.4. Deliverable 1 (+ the block half of 2) — decompose recall. Four
BlockKinds were defined and never produced. Now: the overflow summary andstuck-loop steer classify as
Summary/Steeredinstead of being attributed tothe user; the recall block splits into one
RecalledFrameblock per item, eachcarrying its
nod_…and citation label; attachments get blocks.5. Deliverables 2 + 3 — provenance and telemetry.
content_digestwasminted at the store boundary and dropped at the CGP→pipeline projection, which
is why every
ContextFrameRefcarriedcontent_digest: null. Threaded end toend.
ContextRecallfired from the pipeline only; the one-shot run,interactive REPL,
/goal, and Command Deck all recalled and reported nothing.All five now emit, through one projection (
Recall::telemetry_event).6. Deliverable 5 — selection reasons and required items.
SelectionReason(anchored / ranked / lexical_fallback); anchored is required and ranking cannot
evict it. Both packers fixed — including the undocumented second one in
recall_via_host, which had no drop report at all and would have silentlyundone any precedence that only landed in
pack_to_budget.Gate criteria
is_verified()+ byte equality.RequiredOverBudget).COSINE_CALLS-style work counters are unchanged and green.pack_to_budgetis now two linear passes instead of one; the expensive work (cosines, body reads, frame construction) is untouched. Decomposition adds asplit_inclusivewalk over one message per step.Deliberately left out — decisions a reviewer should confirm
Scope-leakage tests at every dimension. One dimension (domain scope) is
covered by a pre-existing test. The record-level scope/sharing dimensions
(ADR 0002) live in
stella-core/src/context_record/scope.rs, which isPhase 3's territory under the ownership split for this phase — and the typed
records that have those dimensions are what Phase 3 builds. I did not reach
into it. This is the one gate criterion I did not meet.
block_idstaysNoneonContextFrameRef. Not an omission: thoseframes feed the planner's structured split-context prompt, are never
rendered into a message, and so no context block exists for them to name.
Fabricating one would point at a block no manifest ever cited. The join runs
through
BlockOrigin::memory_id, which decomposition now populates. If youwant a literal
block_idthere, it requires rendering the recall blockbefore the event is emitted — say so and I will restructure.
Attachment payloads are not journaled. An at-rest attachment is metadata
and a path, which is reconstructable. A hydrated one carries inline base64;
its identity still hashes the real value but the local preimage is withheld,
so it reports as
unresolved. The alternative — addingAttachmentto gapkinds unconditionally — would put image bytes on
--output-format stream-jsonstdout.Reasons still collapse at the CGP seam.
From<RecallResult> for ContextQueryResultreduces the drop report totruncated+dropped_estimate;ContextQueryResultis a CGP wire type this repo doesnot own. Mitigated by writing the reason onto each frame's provenance,
which does survive — that is also how the host reads required-ness back. The
drop reasons still do not cross.
Things that moved, that a reviewer should look at
tag). Accepted deliberately: the ids that moved were wrong. Nothing rewrites
history; stored ids remain valid for the receipts that cite them.
empty_message_formappedsummaryto an assistantmessage, with a comment claiming the summary is spliced as assistant-authored.
It is not — the summarizer splices
CompletionMessage::user. Unobservableuntil now because nothing ever emitted a
summaryblock; emitting one wouldhave produced a wrong reconstruction. No stored row is affected.
append_blockappends instead of assigns. Required for multi-blockmessages; identical for the single-block kinds, which start from an empty
message. The recall split is a byte-exact partition, pinned by a property
test over adversarial separators (CRLF, blank lines, a frame body whose own
line starts with
-).max_framesnow bounds ranked admissions only. A query for 5 frames thatanchors 2 files can yield 7. Counting required items against the frame budget
would let it evict one for
FrameCount, which ADR 0006 forbids. Bounded bythe anchor cap and the token budget.
recall_blockis now#[cfg(test)]. A caller that only wants the stringis a caller that emits no telemetry; leaving it reachable from production
would let the next recall site reintroduce the defect silently.
event.rs−237 (the pure move),pipeline.rs−28, and small irreducible growth indriver.rs(+7, one configfield),
agent.rs(+30),command_deck.rs(+12), and two test files. All viamake file-size-update, which is the visible path the ratchet documents.Note PR #732 (
perf/receipts-hash-memoization) also touchesstella-core/src/receipts.rs. This PR is additive there rather than a rewrite,but a rebase is coming.
Refs #713 — five of six deliverables complete; the sixth (scope-leakage tests
at every dimension) is listed above as a gap.