AI assistant cost optimization: Keep prompt history byte-stable so the cache prefix survives

[jurgenwerk]

Every message re-sends the whole conversation. The provider caches it — cached tokens cost ~1/10th — but only the byte-identical prefix: change one byte and everything after it re-bills at full price. Replaying real sessions showed 40–60% of cost going to bytes we changed for no reason. This PR stops all four sources of pointless byte churn:

#	What rewrote already-sent bytes	When it fired	Fix in this PR
1	Attaching a newer version of a card/file deleted the older message's copy of its content; readRealmFile results were emptied the same way	Every re-attach / re-read of the same card or file	Each message keeps its own attachment snapshot permanently; a header tells the model a later attachment supersedes earlier ones
2	An applied patch's SEARCH/REPLACE block was replaced with [Omitting previously suggested code change], then rewritten again to […and applied…] when the patch result arrived. Side effect: weaker models imitated the placeholder instead of writing real patches — sessions stalled or claimed phantom edits	Every applied code patch (twice)	Patch blocks stay in history verbatim; elideCodeBlocks deleted
3	getTools rebuilt the array every turn: latest definition won, then alphabetical sort — a re-read of an edited skill rewrote and moved entries, and tools render ahead of all history, so each change re-billed the entire conversation	Every skill re-read, re-uploaded definition, or name collision	Monotonic union: first definition of a name wins for the life of the room, new names append in first-seen order, nothing moves or is removed (except user disabling a skill). Replay test asserts each turn's serialized array is a byte-prefix extension of the previous
4	patchCardInstance's schema was generated from the open card's type — regenerated on every card open, card switch, and even edits to the card's own code. No interactive session ever called it (skills route edits to patch-fields / code patches)	Every card open/switch, and any schema-affecting card edit	No longer injected on interactive messages; the executor still honors calls from old rooms, and the programmatic SendAiAssistantMessage API keeps it

Measured on the same task (wedding-planner app build), same model (Sonnet 4.6):

Metric	main	this PR
Session cost	$2.49	$0.92
Cached share of input tokens	59%	87%
Cache resets caused by byte churn (rows 1–4)	3	0
The turn right after opening a card	$0.45, 0 tokens cached	$0.08, 100% of prior request cached

Remaining, by design: the first read of a tool-declaring skill appends its definitions (one reset per session — CS-12576 moves those to room creation) and the provider's ~5-minute idle cache expiry.

🤖 Generated with Claude Code

[github-actions]

Preview deployments

• Host staging preview

• Host production preview

Host Test Results

    1 files      1 suites   2h 29m 29s ⏱️
4 368 tests 4 354 ✅ 14 💤 0 ❌
4 387 runs  4 373 ✅ 14 💤 0 ❌

Results for commit d0eaaae.

Realm Server Test Results

    1 files  ± 0      1 suites  ±0   15m 12s ⏱️ -42s
2 206 tests +14  2 206 ✅ +14  0 💤 ±0  0 ❌ ±0 
2 289 runs  +14  2 289 ✅ +14  0 💤 ±0  0 ❌ ±0 

Results for commit d0eaaae. ± Comparison against earlier commit 41a4814.

[Copilot]

Pull request overview

Improves AI prompt-cache stability to reduce repeated token costs.

Changes:

• Preserves historical attachments and code patches byte-for-byte.
• Makes tool definitions first-seen and removes interactive card-specific patch tools.
• Adds Anthropic routing preference and provider telemetry.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
packages/runtime-common/ai/prompt.ts	Stabilizes prompt history and tool ordering.
packages/matrix/tests/tools.spec.ts	Verifies interactive patch-tool removal.
packages/host/app/services/matrix-service.ts	Stops injecting card-specific patch tools.
packages/base/matrix-event.gts	Extends usage telemetry types.
packages/ai-bot/tests/prompt-construction-test.ts	Tests stable attachments, patches, and tools.
packages/ai-bot/main.ts	Prefers Anthropic routing for Anthropic models.
packages/ai-bot/lib/responder.ts	Records provider and generation identifiers.

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[jurgenwerk]

[Claude Code 🤖] Fixed in d0eaaae, and the fix goes the direction you suggested: admission is now ungated (every definition a state event lists was declared by a then-enabled skill when the host wrote it — the host rebuilds the list from enabled skills on every write), and removal is driven by what the disabled skills declare: their contents are downloaded and parsed exactly like enabled ones, minus any name an enabled skill still declares. Editing an enabled skill to drop a tool therefore cannot shrink the union — pinned by the new replay test a definition survives its enabled skill being edited to no longer declare it, which also asserts byte-identical rebuilds. One side finding: nameless definition entries (pre-rename rooms carry them) were only excluded by accident under the old gate; they are now skipped explicitly.

[habdelra]

[Claude Code 🤖] This resolves it — verified against d0eaaaed rather than against the description.

Admission in getTools is genuinely ungated now: the state-event branch skips only on !name, disabledToolNames.has(name), or toolMap.has(name), so nothing is compared against current skill contents. Removal comes from getDisabledSkillToolNames, which downloads and parses disabledSkillCards the same way getEnabledSkills handles enabled ones (markdown via parseMarkdownSkill, cards via the attributes.tools ?? attributes.commands fallback — both reach functionName, so the two skill shapes behave identically), then subtracts every name an enabled skill still declares. Editing an enabled skill to drop a tool therefore cannot reach the removable set. a definition survives its enabled skill being edited to no longer declare it pins it, and its second getTools call asserting byte-identical JSON is the right assertion — it pins the property the cache actually bills by, not just the name list.

I also checked the premise the ungating rests on, since it's the load-bearing claim: that every definition a state event lists was declared by a then-enabled skill. packages/host/app/tools/update-room-skills.ts rebuilds toolDefinitions from validEnabledSkills on each write, seeded from the previous list — it can carry a definition forward when a skill fails to rehydrate, but never invents one, so the union stays sound. And I looked for the case that would defeat the disabled-skills removal entirely: a skill leaving the room outright, which would strand its tools with no disabledSkillCards entry to gate them. update-room-skills.ts only moves skills between the enabled and disabled maps, and file-def-manager.ts reads that state without writing it, so there is no such path.

One thing the fix leaves unpinned, which I've raised as a separate comment rather than reopening this: with the source-ranking merge gone, getTools is pure first-seen-in-history-wins, but a skill both enabled and read yields one definition; the uploaded one wins still asserts the old precedence rule and passes only because its fixture happens to put the state event first.

---

Generated by Claude Code

[jurgenwerk]

[Claude Code 🤖] Fixed in d0eaaae by making the note claim only what is always true. The policy stays what it was before this PR (opening a card is the grant — the old tool-presence check encoded exactly that, since the tool was only generated for attached ∩ open cards): the wording is now "the user has no card open, and editing requires the card to be open" with no claim about attachments, so it can no longer contradict an Attached Cards section in the same prompt. Test title and comment aligned with the open-is-the-grant policy.

[habdelra]

[Claude Code 🤖] Resolves the half this thread named, but the replacement introduces a different inaccuracy, so I've opened a new comment on buildContextMessage rather than closing here.

The contradiction is gone: the wording no longer mentions attachments, so it can't disagree with an Attached Cards section rendered from the same message.

What it now claims instead is that opening the card is what unblocks editing — and after this PR that isn't true either. MatrixService#sendMessage sends tools: [] unconditionally, so opening a card injects nothing; the only card-patch tool an interactive room can have comes from a skill (patch-fields via boxel-environment) or from a legacy patchCardInstance already sitting in an old room's history. So the note tells the user to do something that changes nothing, and — the more consequential direction — openCardIds.length > 0 suppresses it in rooms where the model genuinely has no way to edit, leaving it to propose patches it cannot apply.

On the policy question in my original comment: "opening is the grant" was the right reading of the pre-PR code, but it stopped being the grant in this PR — the host-side generator that made it true is what got deleted. The signal that survives the change is the tools array, and CARD_PATCH_COMMAND_NAMES is already in the module for exactly this. Reading it in buildContextMessage costs nothing cache-wise, since that message is pushed after addHistoryCacheBreakpoint and already re-serializes every turn for Current date and time. Detail, including how this subsumes the codex read-only-realm finding, is in the new comment.

---

Generated by Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d0eaaaedcf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Bound retained attachment snapshots

When an open card or source file is automatically reattached on successive turns, this unconditional content download makes every historical snapshot remain in the prompt; buildAttachmentsMessagePart now similarly retains every prior image, PDF, audio, and video payload. Provider caching may reduce the billed price, but cached input still occupies the model's context window, so prompt size grows by another full copy on every turn and sufficiently long attachment-heavy sessions will exceed request/context limits. Preserve byte stability without retaining an unbounded number of full attachment bodies, for example by compacting old snapshots at a stable boundary.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Preserve the writable-card check in edit guidance

When the user opens an attached card from a read-only realm, openCardIds is nonempty, so this condition suppresses the unable-to-edit warning even though the removed host logic previously withheld patchCardInstance after this.realm.canWrite(c.id) returned false. Opening a card therefore now gets treated as edit authorization regardless of realm permissions, leading the model to propose or attempt edits that cannot be applied; carry an explicit writable-card signal into this decision rather than inferring access from openCardIds alone.

Useful? React with 👍 / 👎.

[habdelra]

[Claude Code 🤖] Reviewed as a re-review of d0eaaaed, with the lens the PR itself sets: for each of the four churn sources, does the new rendering actually depend only on bytes that are already fixed at the time the message is written — and what does the fix cost when it isn't free. That means tracing every input each rendering path reads (later history, current skill contents, room state, the tools array) and pricing the "carry it forward instead" trade rather than taking it as uniformly cheap.

Bottom line: the tools-array and code-block halves hold up under tracing, and both previously-raised threads are genuinely resolved by d0eaaaed. One blocking issue: the attachment half buys byte-stability for text snapshots but silently converts the media path into unbounded per-turn work — every image/PDF/audio/video in the room is re-fetched, re-encoded, and re-sent on every turn, through a code path that (unlike the text path) has no cache. That needs a decision before merge; the rest are non-blocking.

On the two open threads.

• getTools monotonicity (the edited-enabled-skill shrinkage thread): resolved. Admission is now ungated, removal is driven by what the disabled skills declare, and a definition survives its enabled skill being edited to no longer declare it pins both survival and a byte-identical rebuild. I checked the claim the design rests on — that a state event's toolDefinitions were all declared by a then-enabled skill — against packages/host/app/tools/update-room-skills.ts: the list is rebuilt from validEnabledSkills on each write and seeded from the previous list, so it can carry a definition forward but never invents one. I also checked whether a skill can leave a room entirely (which would strand its tools in the union with no disabledSkillCards entry to remove them): update-room-skills.ts only ever moves skills between the enabled and disabled maps, so there is no removal path. The invariant holds.
• The "unable to edit any cards" note: the self-contradiction is gone, but the replacement claim is inaccurate in the other direction — see the thread and the inline comment on buildContextMessage.

Recommendations, in priority order.

1. Blocking. Decide what happens to historical media. Re-adding an isCurrentMessage-style gate is not an option (position-relative gates churn by construction); the trailing volatile message that already carries the context block is a home that is both bounded and prefix-safe. Detail and the alternative (memoize downloadFileAsBase64DataUrl, cap retained bytes) in the comment on the media loop in buildAttachmentsMessagePart. There is no multi-turn media test either way.
2. Gate the editability note on the presence of a card-patch tool rather than openCardIds — CARD_PATCH_COMMAND_NAMES is already in the module, and reading the tools array there is free because that message sits after the cache breakpoint. See the comment on buildContextMessage; this also subsumes the codex read-only-realm finding.
3. a skill both enabled and read yields one definition; the uploaded one wins asserts a precedence rule the code no longer has — it passes on fixture ordering. Retitle to first-seen-wins and add the reversed-order case, or restore the precedence deliberately. Comment on the test.
4. The usage payload's whole-object deepEqual in responding-test.ts is blind to the new provider / generationId fields because the fixture chunk omits id. Two-line fix; comment on responder.ts.
5. Delete the now-vacuous open card that is not attached Playwright test. Comment on tools.spec.ts.
6. Three of the new comments describe the revision they replace rather than the code as it stands (buildContextMessage, MatrixService#sendMessage, tools.spec.ts). Per the repo's evergreen-comments skill, state the rule and its reason. Folded into the relevant comments.

Adjacent, out of scope. toResultMessages calls buildAttachmentsMessagePart with no inputModalities and then discards mediaParts, keeping only .text — so media on a tool result is downloaded and base64-encoded for nothing, the model-capability gate is skipped for it, and mediaSourceUrls then omits those files from the text listing too, meaning the model sees neither the file nor a mention of it. Pre-existing (the old call passed isCurrentMessage: true literally), not widened by this PR, and not something to fix here — but it's in the same function as recommendation 1, so whoever implements that will be standing next to it.

One question rather than a finding: Percy reports 12 visual changes on a branch that touches no CSS or templates. Most likely baseline drift from main, but worth a glance in case removing the per-card patch tool changed what the assistant renders.

---

Generated by Claude Code

[habdelra]

[Claude Code 🤖] Regression — blocking (needs a decision, not necessarily this exact fix): dropping the isCurrentMessage guard around this loop makes every image / PDF / audio / video ever attached to the room get re-downloaded, re-base64-encoded, and re-uploaded to the router on every turn. The text-snapshot half of the byte-stability change is cheap; this half is not, and it is unbounded.

The mechanism. buildPromptForModel calls buildAttachmentsMessagePart once per non-bot m.room.message in history (and toResultMessages calls it once per tool result). Before this PR the media block was wrapped in if (isCurrentMessage) { … }, so at most one message's media was materialized per turn. That wrapper is gone, so the loop now runs for the whole history.

Why it costs more than the text path. downloadFile in packages/runtime-common/ai/matrix-utils.ts is memoized — fileCache keyed by canonical media URL, plus inFlightFetches de-duping concurrent fetches — so re-reading a historical text snapshot is nearly free after the first turn. downloadFileAsBase64DataUrl, four lines below it in the same file, has no cache at all: it goes straight to fetchMatrixMediaWithFallback, arrayBuffer(), and a full base64 encode, every call. So an N-image session pays N media fetches + N encodes per turn, holds all N encoded copies live in the messages array at once, and ships all N in the request body.

Prompt caching does not pay this back. A cache hit discounts billing; it does not shrink the context window, and it does not stop the bytes going over the wire — the provider still receives the full prompt each turn and matches the prefix server-side. So an attachment-heavy session grows its request body and its context consumption linearly with turns until it hits a request-size or context limit. This is the same concern the codex thread on getAttachedFiles raised; the media path is the sharp end of it.

Why the obvious fix does not work. Re-adding isCurrentMessage would reintroduce exactly the churn this PR removes: "am I the newest message" flips as history grows, so the older message's rendering changes and the prefix breaks. Any position-relative gate (last message, last K messages) has the same defect.

A shape that gets both properties. The prompt already has a home for volatile per-turn content: buildPromptForModel runs addHistoryCacheBreakpoint(messages, messages.length - 1) and then pushes the trailing { role: 'user', content: contextContent } message, which is deliberately re-serialized every turn because it carries Current date and time. Media could ride there: history messages render their attachments as stable metadata-only text forever (byte-stable, and cheap because they never re-download), and the current turn's media parts attach to that trailing message after the breakpoint. Bounded per turn, and the cached prefix is untouched. normalizeHistoryContentShape runs before the push, so a parts-array trailing message needs no extra handling.

If you'd rather keep media in history, the minimum is (a) memoize downloadFileAsBase64DataUrl the way downloadFile is memoized, so the per-turn cost is encode-free after the first fetch, and (b) cap total retained media bytes with a documented, position-independent rule. Either way there's a test gap: every media test in prompt-construction-test.ts (image attachments produce native image_url content parts, the audio/video ones) uses a single-message history, so nothing pins what a multi-turn history with media now emits.

---

Generated by Claude Code

[habdelra]

[Claude Code 🤖] Regression — non-blocking, but the wording is new in this PR so the fix belongs here: openCardIds is no longer a proxy for edit access, so this condition both suppresses the note when the model genuinely cannot edit and states a remedy that no longer produces a tool.

What changed underneath it. MatrixService#sendMessage now sends tools: [] unconditionally, so opening a card injects nothing. The only card-patch tools an interactive room can have are the ones a skill supplies — patch-fields via boxel-environment, or a legacy patchCardInstance still sitting in an old room's history. So:

• Card open, no skill supplying a patch tool → note suppressed, but the model has no way to edit. It will propose edits it cannot apply.
• Note fires → it says "editing requires the card to be open. Ask them to open the card they want changed." Opening the card changes nothing; the user would need a skill enabled.

The codex thread below flags the read-only-realm case of the same defect: the old host-side filter was attachedCards.filter(c => openCardIds.includes(c.id)).filter(c => this.realm.canWrite(c.id)), so a card open in a read-only realm produced no tool and therefore fired the note. That half is real, but it's a symptom — openCardIds is simply the wrong signal now, whatever it's intersected with.

The fix, and why it costs nothing. The accurate signal is "does the request carry a card-patch tool", which the module already names: CARD_PATCH_COMMAND_NAMES = new Set(['patchCardInstance', 'patchFields']). buildContextMessage had tools and this PR renamed it to _tools to make it unused. Take it back and gate on the tools array:

let hasCardPatchTool = tools.some((t) =>
  CARD_PATCH_COMMAND_NAMES.has(t.function.name),
);
if (attachedFiles.length == 0 && !hasCardPatchTool && hasSomeAttachedCards(history, aiBotUserId)) {

This does not reintroduce the churn the PR is removing. buildContextMessage's output is pushed as the trailing message after addHistoryCacheBreakpoint(messages, messages.length - 1), and it already carries Current date and time, so it is volatile by design — reading the tools array there changes no cached byte. (The tools array itself is now monotonic, so it wouldn't even churn if it were cached.) The wording should then name the actual grant, e.g. that no card-editing tool is available and the user needs the relevant skill enabled.

Separately, on the comment block above (also packages/host/app/services/matrix-service.ts around let tools: Tool[] = [], and the new comment in packages/matrix/tests/tools.spec.ts): per the repo's evergreen-comments skill these should describe the code as it is, not the revision it replaced. "The old wording keyed off the per-card patchCardInstance tool… The policy is unchanged from before this change" only parses for someone who has the previous revision open. The durable version is the rule plus its reason — interactive messages carry no per-card patch tool because a card-type-derived schema churns the tools array, which renders ahead of all history.

---

Generated by Claude Code

[habdelra]

[Claude Code 🤖] Regression in what this test pins — non-blocking: the title still claims a precedence rule ("the uploaded one wins") that the new getTools does not implement. It passes only because of where the fixture puts the events.

What the code now guarantees. getTools is a single chronological pass with add() short-circuiting on toolMap.has(name). There is no source ranking left — the old "merge discovered first, so an uploaded definition overwrites it" ordering was replaced by first-seen-in-history-wins. In this fixture skillsRoomFixture puts the skills state event at index 0 and readResultEvent('read-1', 2000, …) is pushed after it, so the upload wins by position.

What that leaves unpinned. Reverse the order — the bot reads a skill file via readRealmFile, and the user then attaches that same skill so the host uploads a definition for the same functionName — and the discovered definition wins. Same tool name, a different schema in the request, and no test notices. That's a plausible sequence (the skills-index pull model has the model read skills it was never handed), and it's the case a reader of this test would assume is covered.

Ask: retitle to what the code guarantees (e.g. the first definition seen in history wins, whatever its source) and add the reversed-order case asserting the discovered definition is the one emitted. If upload-beats-discovery is actually the intended rule rather than an accident of ordering, then the code needs a precedence pass and this test title is right — but then the first-wins comment on getTools needs to say so too. Worth deciding explicitly; right now the comment and the test disagree about which rule is in force.

---

Generated by Claude Code

[habdelra]

[Claude Code 🤖] Test gap — non-blocking: these two fields have no coverage, and the test that looks like it covers them is structurally blind to them.

packages/ai-bot/tests/responding-test.ts asserts the usage payload with a whole-object assert.deepEqual(JSON.parse(last.content.data as string).usage, { promptTokens, completionTokens, cachedTokens, costUsd }). That assertion would catch an unexpected extra key — except the fixture chunk it feeds onChunk is { choices: [], usage: { … } } with no id and no provider, so generationId and provider both come out undefined and are stripped by the typeof … === 'string' spread. A real OpenRouter chunk always carries id, so the assertion has quietly stopped describing the production shape of usage.

Two lines fix it: add id: 'gen-abc123' and provider: 'Anthropic' to that fixture chunk and the corresponding generationId / provider entries to the expected object. That also pins the typeof guards, which are the only thing keeping a non-string id out of the room event.

---

Generated by Claude Code

[habdelra]

[Claude Code 🤖] Follow-up / cleanup — non-blocking: this sibling test is now vacuous and should go.

MatrixService#sendMessage sets tools: [] unconditionally, so boxelMessageData.context.tools is [] for every interactive message regardless of what is open or attached. expect(boxelMessageData.context.tools).toMatchObject([]) therefore asserts a constant — while paying for a browser session, a subscribed user, a fresh realm, a posted card, and a detach interaction to do it. The retitled test above already pins the real contract (no per-card patch tool even in the most favourable case: card open, writable, attached, context shared), which strictly subsumes this one.

Ask: delete this test. If the "open but not attached" path is still worth guarding, the thing worth guarding is now context.openCardIds / attachedCards — assert that, not tools.

---

Generated by Claude Code

[habdelra]

[Claude Code 🤖] Nit — non-blocking: tools is now a const-in-all-but-name empty array whose only use is the tools, shorthand ~20 lines down inside the context literal. Inline it as tools: [] there and drop the local; a named let that can never be anything but [] reads like a hook someone is expected to extend.

Worth keeping the reason at the tools: [] site rather than here, since that's where a future reader is tempted to start appending — and per evergreen-comments, state the rule rather than the change: interactive messages carry no per-card patch tool, because a schema derived from the open card's type changes on every card open, switch, and schema edit, and tool definitions render ahead of all message history. The "still honored by the executor / still injected by SendAiAssistantMessage" half is genuinely useful and durable — that stays.

---

Generated by Claude Code