diff --git a/packages/ai-bot/lib/responder.ts b/packages/ai-bot/lib/responder.ts index 9144dee0636..82b4b882dbe 100644 --- a/packages/ai-bot/lib/responder.ts +++ b/packages/ai-bot/lib/responder.ts @@ -309,6 +309,12 @@ export class Responder { let cachedTokens = (chunk.usage as any).prompt_tokens_details ?.cached_tokens; let costUsd = (chunk.usage as any).cost; + // OpenRouter also stamps each chunk with the provider that served the + // request and the generation id. Recording them alongside the counts + // makes a cache miss attributable: a cachedTokens collapse with a + // provider change is a routing miss, not a prompt-shape bug. + let provider = (chunk as any).provider; + let generationId = chunk.id; // Hand the counts to the publisher so they ride on the final room // event. When the final edit has already gone out (the usage chunk // trails the finish chunk), finalize() sends one more edit to carry @@ -318,6 +324,8 @@ export class Responder { completionTokens: chunk.usage.completion_tokens, ...(typeof cachedTokens === 'number' ? { cachedTokens } : {}), ...(typeof costUsd === 'number' ? { costUsd } : {}), + ...(typeof provider === 'string' ? { provider } : {}), + ...(typeof generationId === 'string' ? { generationId } : {}), }; log.info( `Request used ${chunk.usage.prompt_tokens} prompt tokens (${ diff --git a/packages/ai-bot/main.ts b/packages/ai-bot/main.ts index 6f5def90ee6..ad6e7898f91 100644 --- a/packages/ai-bot/main.ts +++ b/packages/ai-bot/main.ts @@ -142,6 +142,17 @@ class Assistant { // the cast. (request as Record).usage = { include: true }; + // Prompt caches live per provider, and the router is otherwise free to + // spread a room's requests across providers — which turns a warm cache + // prefix into a full-price miss mid-conversation. Bias Anthropic-model + // requests to Anthropic itself, keeping fallbacks for availability. + if (this.getModel(prompt).startsWith('anthropic/')) { + (request as Record).provider = { + order: ['anthropic'], + allow_fallbacks: true, + }; + } + if (prompt.reasoningEffort !== undefined) { request.reasoning_effort = prompt.reasoningEffort; } diff --git a/packages/ai-bot/tests/prompt-construction-test.ts b/packages/ai-bot/tests/prompt-construction-test.ts index 12a81443fb2..6e5562968e3 100644 --- a/packages/ai-bot/tests/prompt-construction-test.ts +++ b/packages/ai-bot/tests/prompt-construction-test.ts @@ -40,6 +40,9 @@ import { ensureTrailingSlash, skillCardRef, rri, + SEARCH_MARKER, + SEPARATOR_MARKER, + REPLACE_MARKER, } from '@cardstack/runtime-common'; import { absolutizeSkillLinks, @@ -860,7 +863,7 @@ Current date and time: 2025-06-11T11:43:00.533Z assert.equal(attachedCards.length, 0); }); - test('downloads and includes most recent version of attached files', async () => { + test('each message keeps its own attached-file snapshot content', async () => { const history: DiscreteMatrixEvent[] = [ { type: 'm.room.message', @@ -1042,7 +1045,24 @@ Current date and time: 2025-06-11T11:43:00.533Z }, ]; - // Set up mock responses for file downloads + // Set up mock responses for file downloads — every message's snapshot + // is downloaded now, so each version needs a response. + mockResponses.set('http://test.com/spaghetti-recipe-a.gts', { + ok: true, + text: 'spaghetti content version a', + }); + mockResponses.set('http://test.com/best-friends-a.txt', { + ok: true, + text: 'best friends version a', + }); + mockResponses.set('http://test.com/spaghetti-recipe-b.gts', { + ok: true, + text: 'spaghetti content version b', + }); + mockResponses.set('http://test.com/best-friends-b.txt', { + ok: true, + text: 'best friends version b', + }); mockResponses.set('http://test.com/spaghetti-recipe-c.gts', { ok: true, text: 'this is the content of the spaghetti-recipe.gts file', @@ -1071,33 +1091,35 @@ Current date and time: 2025-06-11T11:43:00.533Z assert.ok( messageText(userMessages[0]).includes( ` -Attached Files (files with newer versions don't show their content): -[spaghetti-recipe.gts](http://test-realm-server/my-realm/spaghetti-recipe.gts) -[best-friends.txt](http://test-realm-server/my-realm/best-friends.txt) +[spaghetti-recipe.gts](http://test-realm-server/my-realm/spaghetti-recipe.gts): + 1: spaghetti content version a +[best-friends.txt](http://test-realm-server/my-realm/best-friends.txt): + 1: best friends version a `.trim(), ), + 'first message keeps its own snapshot content', ); assert.ok( messageText(userMessages[1]).includes( ` -Attached Files (files with newer versions don't show their content): -[spaghetti-recipe.gts](http://test-realm-server/my-realm/spaghetti-recipe.gts) -[best-friends.txt](http://test-realm-server/my-realm/best-friends.txt) -[file-that-does-not-exist.txt](http://test.com/my-realm/file-that-does-not-exist.txt) -[example.pdf](http://test.com/my-realm/example.pdf): [application/pdf] +[spaghetti-recipe.gts](http://test-realm-server/my-realm/spaghetti-recipe.gts): + 1: spaghetti content version b +[best-friends.txt](http://test-realm-server/my-realm/best-friends.txt): + 1: best friends version b `.trim(), ), + 'second message keeps its own snapshot content', ); assert.ok( messageText(userMessages[2]).includes( ` -Attached Files (files with newer versions don't show their content): [spaghetti-recipe.gts](http://test-realm-server/my-realm/spaghetti-recipe.gts): 1: this is the content of the spaghetti-recipe.gts file [best-friends.txt](http://test-realm-server/my-realm/best-friends.txt): 1: this is the content of the best-friends.txt file `.trim(), ), + 'latest message includes its content with line numbers', ); assert.ok( @@ -1408,9 +1430,9 @@ Attached Files (files with newer versions don't show their content): 'http://localhost:4201/experiments/Author/1', ), ); - assert.false( + assert.true( messageText(userMessages[0]).includes('"firstName": "Terry"'), - 'should not include the contents of the first version of the card in the first user message', + 'each message keeps its own snapshot of the card content', ); assert.true( messageText(userMessages[1]).includes( @@ -1718,7 +1740,11 @@ Attached Files (files with newer versions don't show their content): }); }); - test('Adds the "unable to edit cards" only if there are attached cards and no tools', async () => { + test('Adds the "unable to edit cards" note only when no card is open or attached to the message', async () => { + // Interactive messages no longer inject per-card patch tools, so the + // access statement keys off what the user actually shared: an open or + // message-attached card is the grant; without one (and with no attached + // files) the model is told to ask instead of guessing at ids. const eventList: DiscreteMatrixEvent[] = [ { type: 'm.room.message', @@ -1729,7 +1755,7 @@ Attached Files (files with newer versions don't show their content): body: 'set the name to dave', data: { context: { - openCardIds: [rri('http://localhost:4201/drafts/Author/1')], + openCardIds: [], tools: [], submode: 'code', functions: [], @@ -1792,22 +1818,20 @@ Attached Files (files with newer versions don't show their content): ); let nonEditableCardsMessage = - 'You are unable to edit any cards, the user has not given you access, they need to open the card and let it be auto-attached.'; + 'You are unable to edit any cards: the user has no card open and none attached to this message. Ask them to open the card they want changed.'; let userContextMessage = messages?.[messages.length - 1]; assert.ok( messageText(userContextMessage).includes(nonEditableCardsMessage), - 'The context leading the user turn should include the "unable to edit cards" message when there are attached cards and no tools, and no attached files, but was ' + + 'The context leading the user turn should include the "unable to edit cards" message when there are attached cards but none open, and no attached files, but was ' + userContextMessage?.content, ); - // Now add a tool + // Now open a card: the grant is the user opening it, not a tool let cardMessageContent = eventList[0].content as CardMessageContent; cardMessageContent.data.context ||= {}; - cardMessageContent.data.context.tools = [ - getPatchTool(rri('http://localhost:4201/drafts/Author/1'), { - attributes: { firstName: { type: 'string' } }, - }), + cardMessageContent.data.context.openCardIds = [ + rri('http://localhost:4201/drafts/Author/1'), ]; const { messages: messages2 } = await getPromptParts( @@ -1820,7 +1844,7 @@ Attached Files (files with newer versions don't show their content): !messageText(messages2?.[messages2.length - 2]).includes( nonEditableCardsMessage, ), - 'System context message should not include the "unable to edit cards" message when there are attached cards and a tool', + 'System context message should not include the "unable to edit cards" message when a card is open', ); // Now remove cards, tools, and add an attached file @@ -1850,7 +1874,11 @@ Attached Files (files with newer versions don't show their content): ); }); - test('Gets only the latest functions', async () => { + test('the first-seen definition of a tool wins for the life of the room', async () => { + // The tools array renders ahead of message history, so a definition that + // mutated mid-session would re-bill the whole conversation. The union is + // first-wins: the earlier patch tool keeps its schema even though a later + // message carries a same-named tool generated from a different card. const history: DiscreteMatrixEvent[] = [ { type: 'm.room.message', @@ -1932,47 +1960,16 @@ Attached Files (files with newer versions don't show their content): fakeMatrixClient, ); assert.equal(functions.length, 1); - if (functions.length > 0) { - assert.deepEqual(functions[0], { - type: 'function', - function: { - name: 'patchCardInstance', - description: - 'Propose a patch to an existing card instance to change its contents. Any attributes specified will be fully replaced, return the minimum required to make the change. If a relationship field value is removed, set the self property of the specific item to null. When editing a relationship array, display the full array in the patch code. Ensure the description explains what change you are making.', - parameters: { - type: 'object', - properties: { - description: { - type: 'string', - }, - attributes: { - type: 'object', - properties: { - cardId: { - type: 'string', - const: 'http://localhost:4201/experiments/Meeting/2', - }, - patch: { - type: 'object', - properties: { - attributes: { - type: 'object', - properties: { - location: { - type: 'string', - }, - }, - }, - }, - }, - }, - }, - }, - required: ['attributes', 'description'], - }, + // Byte-for-byte the tool the FIRST message carried — regenerated with + // the same inputs, so the assertion stays type-safe. + assert.deepEqual( + functions[0], + getPatchTool(rri('http://localhost:4201/experiments/Friend/1'), { + attributes: { + firstName: { type: 'string' }, }, - }); - } + }), + ); }); test('should include instructions in system prompt for skill cards', async () => { @@ -3532,9 +3529,11 @@ Current date and time: 2025-06-11T11:43:00.533Z ); }); - test('Elides code blocks in prompt and includes results', async () => { - // sending older codeblocks back to the model just confuses it and wastes tokens - // so we need to remove them from the prompt + test('Keeps past code blocks verbatim in the prompt', async () => { + // Eliding old code blocks rewrote already-sent history (the placeholder + // text even changed once the patch result arrived), which broke the + // cache prefix — and models imitated the "[Omitting …]" placeholder in + // place of a real patch, stalling the session. The blocks stay verbatim. const eventList: DiscreteMatrixEvent[] = JSON.parse( readFileSync( path.join( @@ -3558,12 +3557,23 @@ Current date and time: 2025-06-11T11:43:00.533Z messageText(messages![2]), 'Updating the file...\n' + 'http://test.com/spaghetti-recipe.gts\n' + - '[Omitting previously suggested and applied code change]\n' + + `${SEARCH_MARKER}\n` + + 'this is the riveting content of the spaghetti-recipe.gts file\n' + + `${SEPARATOR_MARKER}\n` + + 'this is the engaging content of the spaghetti-recipe.gts file\n' + + `${REPLACE_MARKER}\n` + '\n' + 'I will also create a file for rigatoni:\n' + '\n' + 'http://test.com/rigatoni-recipe.gts\n' + - '[Omitting previously suggested code change that failed to apply]\n', + `${SEARCH_MARKER}\n` + + `${SEPARATOR_MARKER}\n` + + 'this is the holy content of the rigatoni-recipe.gts file\n' + + `${REPLACE_MARKER}\n`, + ); + assert.false( + messageText(messages![2]).includes('[Omitting'), + 'no elision placeholder appears in the prompt', ); }); @@ -4145,7 +4155,7 @@ Current date and time: 2025-06-11T11:43:00.533Z assert.true( messageText(toolCallMessage!).includes( ` -Attached Files (files with newer versions don't show their content): +Attached Files (each shows its content as of this message; a later attachment of the same file supersedes it): [postcard.gts](http://test-realm-server/user/test-realm/postcard.gts): 1: export default Postcard extends CardDef {} `.trim(), @@ -4183,7 +4193,7 @@ Attached Files (files with newer versions don't show their content): assert.true( messageText(toolCallMessage!).includes( ` -Attached Cards (cards with newer versions don't show their content): +Attached Cards (each shows its content as of this message; a later attachment of the same card supersedes it): [ { "url": "mxc://mock-server/nashville", @@ -5941,7 +5951,7 @@ new 'the surviving user message keeps its body', ); }); - test('only the most recent message attachments include file content in the prompt', async () => { + test('every message keeps its attached file content in the prompt', async () => { // Policy: files attached to older messages should show metadata only, // even if they are NOT re-attached in later messages. // Only the most recent user message's attachments should include content. @@ -6041,14 +6051,15 @@ new let userMessages = prompt.filter((m) => m.role === 'user'); - // Older message's unique file (config.json) should show metadata only, not content + // The older message keeps its own snapshot's content — re-rendering it + // later would change already-sent history bytes and break prompt caching. assert.ok( messageText(userMessages[0]).includes('[config.json]'), 'First message mentions config.json', ); - assert.notOk( + assert.ok( messageText(userMessages[0]).includes('"key": "value"'), - 'First message should NOT include config.json content (not the current message)', + 'First message keeps its config.json snapshot content', ); // Most recent message's file (utils.ts) should include content @@ -7997,15 +8008,20 @@ module('markdown skill tools', (hooks) => { assert.strictEqual(tools[0].function.description, 'Plans a trip'); }); - test('the latest read of a skill replaces its earlier definitions wholesale', async () => { + test('a re-read of a skill cannot mutate or remove earlier definitions', async () => { + // The tools array renders ahead of message history, so a later read of + // an edited skill file must not rewrite bytes the cache has already + // matched: the first-seen definition wins, and a tool the newer read no + // longer declares stays in the union. A room picks up the edited skill + // only when it is re-attached in a new room. const tools = await getTools( [ readResultEvent('read-1', 1000, [ - discoveredDef('plan-trip_ab12', { description: 'stale' }), + discoveredDef('plan-trip_ab12', { description: 'original' }), discoveredDef('book-hotel_cd34'), ]), readResultEvent('read-2', 2000, [ - discoveredDef('plan-trip_ab12', { description: 'fresh' }), + discoveredDef('plan-trip_ab12', { description: 'edited' }), ]), ], [], @@ -8014,10 +8030,54 @@ module('markdown skill tools', (hooks) => { ); assert.strictEqual( tools.length, - 1, - 'a tool the skill no longer declares disappears with the newer read', + 2, + 'a tool the newer read no longer declares stays in the union', + ); + assert.strictEqual( + tools[0].function.description, + 'original', + 'the first-seen definition wins over the re-read', ); - assert.strictEqual(tools[0].function.description, 'fresh'); + assert.strictEqual(tools[1].function.name, 'book-hotel_cd34'); + }); + + test('the tools array only ever grows by appending across turns', async () => { + // The exact property the prompt cache bills by: each turn's serialized + // tools array must be a prefix-preserving extension of the previous + // turn's. Replay a growing history and assert every prior entry keeps + // its position and bytes as new discoveries arrive. + const history = [ + readResultEvent('read-1', 1000, [ + discoveredDef('plan-trip_ab12', { description: 'original' }), + discoveredDef('book-hotel_cd34'), + ]), + readResultEvent('read-2', 2000, [ + discoveredDef('rent-car_ef56'), + discoveredDef('plan-trip_ab12', { description: 'edited' }), + ]), + readResultEvent('read-3', 3000, [discoveredDef('find-flight_gh78')]), + ]; + let previous: string[] = []; + for (let turn = 1; turn <= history.length; turn++) { + const tools = await getTools( + history.slice(0, turn), + [], + '@aibot:localhost', + fakeMatrixClient, + ); + const serialized = tools.map((t) => JSON.stringify(t)); + assert.deepEqual( + serialized.slice(0, previous.length), + previous, + `turn ${turn}: every previously emitted entry keeps its bytes and position`, + ); + assert.true( + serialized.length >= previous.length, + `turn ${turn}: the array never shrinks`, + ); + previous = serialized; + } + assert.strictEqual(previous.length, 4, 'all four tools were emitted'); }); test('discovered tools on a non-bot result event are ignored', async () => { diff --git a/packages/base/matrix-event.gts b/packages/base/matrix-event.gts index a3b73e5d96d..98c35e6f6dd 100644 --- a/packages/base/matrix-event.gts +++ b/packages/base/matrix-event.gts @@ -283,6 +283,13 @@ export interface TokenUsage { // What the provider charged for the whole request, in USD. Absent when // the provider reports no inline cost. costUsd?: number; + // Which upstream provider served the request (the router's routing + // target). Prompt caches live per provider, so a surprising cache miss is + // attributable when this changes between turns. Absent when unreported. + provider?: string; + // The router-side generation id for the request, for post-hoc lookup of + // routing and cache detail. Absent when unreported. + generationId?: string; } export interface SkillsConfigEvent extends RoomStateEvent { diff --git a/packages/host/app/services/matrix-service.ts b/packages/host/app/services/matrix-service.ts index 18351fd687e..ca9a48cf4fb 100644 --- a/packages/host/app/services/matrix-service.ts +++ b/packages/host/app/services/matrix-service.ts @@ -104,7 +104,6 @@ import { skillsIndexId } from '../lib/utils'; import { importResource } from '../resources/import'; import { getRoom } from '../resources/room'; -import { addPatchTools } from '../tools/utils'; import type CardService from './card-service'; import type LoaderService from './loader-service'; @@ -1982,22 +1981,17 @@ export default class MatrixService extends Service { clientGeneratedId = uuidv4(), context?: BoxelContext, ): Promise { + // Interactive messages no longer inject per-card patchCardInstance + // tools. Their definitions were generated from each open card's type, + // so every card open, switch, or schema edit changed the tools array — + // and tool definitions render ahead of all message history, so each + // change re-billed the whole conversation at full input price. Skills + // route card edits through patch-fields and SEARCH/REPLACE patches + // instead; the executor still honors patchCardInstance calls (old rooms + // carry them in history), and the programmatic + // SendAiAssistantMessage tool still injects it for callers that + // require a forced patch call. let tools: Tool[] = []; - // Open cards are attached automatically - // If they are not attached, the user is not allowing us to - // modify them - let openCardIds = context?.openCardIds ?? []; - let patchableCards = attachedCards - .filter((c) => openCardIds.includes(c.id)) - .filter((c) => this.realm.canWrite(c.id)); - // Generate tool calls for patching currently open cards permitted for modification - tools = tools.concat( - await addPatchTools( - this.toolService.toolContext, - patchableCards, - this.cardAPI, - ), - ); await this.updateSkillsAndToolsIfNeeded(roomId); let contentData = await this.withContextAndAttachments( diff --git a/packages/matrix/tests/tools.spec.ts b/packages/matrix/tests/tools.spec.ts index deee00b02be..05844db8588 100644 --- a/packages/matrix/tests/tools.spec.ts +++ b/packages/matrix/tests/tools.spec.ts @@ -28,7 +28,7 @@ function uniqueRealmName(prefix: string) { } test.describe('Commands', () => { - test(`it includes the patch tool in message event when top-most card is writable and context is shared`, async ({ + test(`it does not inject a per-card patch tool even when the top-most card is writable and context is shared`, async ({ page, }) => { const { username, password } = await createSubscribedUserAndLogin( @@ -75,15 +75,18 @@ test.describe('Commands', () => { ); }).toPass(); let boxelMessageData = JSON.parse(message!.content.data); - expect(boxelMessageData.context.tools.length).toBeGreaterThan(0); - let patchCardTool = boxelMessageData.context.tools.find( + // The card is open and shared in context — but interactive messages no + // longer inject the per-card patchCardInstance tool: its definition was + // generated from the open card's type, so every card open, switch, or + // schema edit changed the tools array, and tool definitions render + // ahead of all message history, re-billing the whole conversation on + // every change. Card edits go through patch-fields and SEARCH/REPLACE + // patches instead. + expect(boxelMessageData.context.openCardIds).toContain(cardId); + let patchCardTool = (boxelMessageData.context.tools ?? []).find( (t: any) => t.function?.name === 'patchCardInstance', ); - expect(patchCardTool).toBeDefined(); - expect( - patchCardTool?.function?.parameters?.properties?.attributes?.properties - ?.cardId?.const, - ).toEqual(cardId); + expect(patchCardTool).toBeUndefined(); }); test(`it does not include patch tool in message event for an open card that is not attached`, async ({ diff --git a/packages/runtime-common/ai/prompt.ts b/packages/runtime-common/ai/prompt.ts index 9faff4b7a2c..aa60f4f365b 100644 --- a/packages/runtime-common/ai/prompt.ts +++ b/packages/runtime-common/ai/prompt.ts @@ -31,7 +31,6 @@ import type { CardMessageContent, CardMessageEvent, CodePatchResultEvent, - DiscoveredToolDefinition, ToolResultEvent, MatrixEvent as DiscreteMatrixEvent, EncodedToolRequest, @@ -80,7 +79,6 @@ import { isMarkdownFile } from '../paths.ts'; import { SKILL_INSTRUCTIONS_MESSAGE, SYSTEM_MESSAGE } from './constants.ts'; import { MAX_CORRECTNESS_FIX_ATTEMPTS } from './correctness-constants.ts'; import { humanReadable } from '../code-ref.ts'; -import { findSearchReplaceBlock } from '../search-replace-markers.ts'; const CARD_PATCH_COMMAND_NAMES = new Set(['patchCardInstance', 'patchFields']); const CHECK_CORRECTNESS_TOOL_NAME = 'checkCorrectness'; @@ -95,14 +93,14 @@ function getLog() { * When building prompts for the model, file attachments are handled * according to the following rules: * - * 1. **Content inclusion**: Only the most recent user message's attached - * files have their content downloaded and included in the prompt. - * Older messages show file metadata (name, type) only. This keeps - * the prompt focused on fresh data and avoids redundant downloads. - * - * 2. **Supersession**: Within the current message, if a file (by - * sourceUrl) is re-attached in a later event, the earlier version - * is shown as metadata only (the later version wins). + * 1. **Byte-stable rendering**: A message's attachments are rendered from + * that message's own snapshot alone — content included at every age, + * never re-rendered because a newer version was attached later. Prompt + * caching is an exact prefix match, so rewriting an already-sent + * message re-bills the whole rest of the prompt at full input price on + * every later turn; carrying the superseded content forward costs only + * cached-read tokens. The model is told that a later attachment of the + * same card/file supersedes earlier ones. * * 3. **MIME type handling** (see `modality.ts` for classification): * - Text-based types (text/*, application/vnd.card+json, @@ -723,43 +721,35 @@ export function hasSomeAttachedCards( return false; } +// A message's rendering must depend only on the message itself, never on +// later history. Prompt caching is an exact prefix match: re-rendering an +// already-sent message (e.g. dropping an attachment's content because a +// newer version was attached later) changes bytes mid-history and re-bills +// everything after them at the full input price on every subsequent turn. +// Carrying the superseded content forward instead costs only cached-read +// tokens — a fraction of that. export async function getAttachedCards( client: MatrixClient, matrixEvent: MatrixEventWithBoxelContext, - history: DiscreteMatrixEvent[], ) { let attachedCards = matrixEvent.content?.data?.attachedCards ?? []; let results = await Promise.all( attachedCards.map(async (attachedCard: SerializedFileDef) => { - // If the file is attached later in the history, we should not include the content here - let shouldIncludeContent = !history - .slice(history.indexOf(matrixEvent) + 1) - .some((event) => { - // event is not always MatrixEventWithBoxelContext but casting lets us safely check attachedCards - return ( - event as MatrixEventWithBoxelContext - ).content?.data?.attachedCards?.some( - (cardAttachment: SerializedFileDef) => - cardAttachment.sourceUrl === attachedCard.sourceUrl, - ); - }); let result: SerializedFileDef = { url: attachedCard.url, sourceUrl: attachedCard.sourceUrl ?? '', name: attachedCard.name, contentType: attachedCard.contentType, }; - if (shouldIncludeContent) { - if (attachedCard.content) { - result.content = JSON.parse(attachedCard.content); - } else { - try { - result.content = await downloadFile(client, attachedCard); - } catch (error) { - getLog().error(`Failed to fetch file ${attachedCard.url}:`, error); - result.error = `Error loading attached card: ${(error as Error).message}`; - result.content = undefined; - } + if (attachedCard.content) { + result.content = JSON.parse(attachedCard.content); + } else { + try { + result.content = await downloadFile(client, attachedCard); + } catch (error) { + getLog().error(`Failed to fetch file ${attachedCard.url}:`, error); + result.error = `Error loading attached card: ${(error as Error).message}`; + result.content = undefined; } } return result; @@ -772,30 +762,17 @@ export async function getAttachedCards( return results; } +// Byte-stable for the same reason as getAttachedCards above: each message +// keeps its own snapshot's content forever, so history bytes never change +// after they are first sent. export async function getAttachedFiles( client: MatrixClient, matrixEvent: MatrixEventWithBoxelContext, - history: DiscreteMatrixEvent[], - isCurrentMessage: boolean = false, ): Promise { let attachedFiles = matrixEvent.content?.data?.attachedFiles ?? []; return Promise.all( attachedFiles.map(async (file: SerializedFileDef) => { - let isSuperseded = history - .slice(history.indexOf(matrixEvent) + 1) - .some((event) => - ( - event as MatrixEventWithBoxelContext - ).content?.data?.attachedFiles?.some( - (f: SerializedFileDef) => f.sourceUrl === file.sourceUrl, - ), - ); - - if ( - isCurrentMessage && - !isSuperseded && - isTextBasedContentType(file.contentType) - ) { + if (isTextBasedContentType(file.contentType)) { return downloadTextContent(client, file); } return toFileDefMetadata(file); @@ -804,7 +781,6 @@ export async function getAttachedFiles( } export async function loadCurrentlySerializedFileDefs( - client: MatrixClient, history: DiscreteMatrixEvent[], aiBotUserId: string, ): Promise { @@ -828,14 +804,9 @@ export async function loadCurrentlySerializedFileDefs( return []; } - // Reuse getAttachedFiles with isCurrentMessage=true — this is always the - // most recent user event, so supersession can't apply (no later events). - return getAttachedFiles( - client, - lastMessageEventByUser as MatrixEventWithBoxelContext, - history, - true, - ); + // The only consumer checks presence, so metadata is enough — downloading + // content here would be a per-turn fetch whose bytes are never sent. + return attachedFiles.map(toFileDefMetadata); } export function attachedFilesToMessage( @@ -887,126 +858,129 @@ export function attachedFilesToMessage( .join('\n'); } +// The tools array renders ahead of all message history in the request, so +// any byte change to it re-bills the entire conversation from the first +// changed byte. Tools are therefore accumulated first-definition-wins, in +// first-seen order, over one chronological pass of the room's history: once +// a function name has entered the array, no re-read of an edited skill +// file, re-uploaded definition, or same-named message-context tool can +// mutate its bytes or its position, and new names only ever append. The +// trade is deliberate: a tool whose definition changed mid-session keeps +// its original schema until a new room, matching how skill instructions +// are snapshotted at attach. +// +// The one sanctioned removal is the user disabling a skill: its tools drop +// out (state-event definitions through the enabled-names gate, discovered +// definitions through the disabled-skills list), costing one honest cache +// reset per user toggle. Re-enabling restores the exact prior bytes and +// order, because first-wins re-admits each definition from its original +// event. export async function getTools( eventList: DiscreteMatrixEvent[], enabledSkills: EnabledSkill[], aiBotUserId: string, client: MatrixClient, ): Promise { - // Build map directly from messages + // Names the currently-enabled skills declare; the admission gate for + // state-event definitions (the host never delists a definition when a + // skill is disabled — this gate is what removes it). let enabledToolNames = new Set(); - let toolMap = new Map(); - - // Get the list of all names from enabled skills for (let skill of enabledSkills) { let skillTools = skill.attributes?.tools ?? skill.attributes?.commands; - if (skillTools) { - for (let tool of skillTools) { - enabledToolNames.add(tool.functionName); - } + for (let tool of skillTools ?? []) { + enabledToolNames.add(tool.functionName); } } - - // Tools discovered by reading a skill file (readRealmFile): each read's - // result event embeds the definitions in `data.discoveredTools`, so this - // source is event-sourced like every other prompt input. A skill read many - // times contributes only its latest read's definitions (the file may have - // changed between reads); a skill toggled off in room state contributes - // none. Merged into the map first, so an enabled skill's uploaded - // definition or a message-context tool with the same functionName wins on - // conflict. let disabledSkillIds = new Set(await getDisabledSkillIds(eventList)); - let discoveredBySkill = new Map(); - for (let event of eventList) { - if (!isToolResultEvent(event)) { - continue; - } - // Only the bot publishes readRealmFile results; a discoveredTools block - // on anyone else's result event is not a legitimate discovery. - if (event.sender !== aiBotUserId) { - continue; + + // Map insertion order is the emitted array order. + let toolMap = new Map(); + let add = (tool: Tool | undefined) => { + let name = tool?.function?.name; + if (!name) { + return; } - let content = event.content; - if (!isToolResultWithOutputContent(content)) { - continue; + // Correctness commands should only be emitted by helper flows, not + // directly via LLM tool calls. + if (name === CHECK_CORRECTNESS_TOOL_NAME) { + return; } - // Only an applied read actually fetched the skill; discoveries claimed - // by a failed or invalid result are not evidence of anything. - if (content['m.relates_to']?.key !== 'applied') { - continue; + if (!toolMap.has(name)) { + toolMap.set(name, tool!); } - let discovered = content.data?.discoveredTools; - if (!discovered?.length) { + }; + + for (let event of eventList) { + // Definitions the host uploaded and listed in a room-skills state + // event. Every state event is scanned — not just the latest — so a + // definition's bytes and position come from its first listing even + // when the host re-uploads a changed version later. Only names not + // already admitted are downloaded. A failed download throws rather + // than silently dropping the definition: a dropped entry would change + // the array's bytes between rebuilds, which is exactly what this + // function exists to prevent. + if (event.type === APP_BOXEL_ROOM_SKILLS_EVENT_TYPE) { + let toolDefinitionFileDefs = + getToolDefinitions( + (event as SkillsConfigEvent).content, + ) ?? []; + for (let toolDefinitionFileDef of toolDefinitionFileDefs) { + if ( + !enabledToolNames.has(toolDefinitionFileDef.name) || + toolMap.has(toolDefinitionFileDef.name) + ) { + continue; + } + let definitionContent = await downloadFile( + client, + toolDefinitionFileDef, + ); + add(JSON.parse(definitionContent).tool); + } continue; } - let bySkill = new Map(); - for (let def of discovered) { + + // Tools discovered by the bot reading a skill file (readRealmFile): + // each applied read's result event embeds the definitions in + // `data.discoveredTools`. Only the bot publishes these; a + // discoveredTools block on anyone else's result event is not a + // legitimate discovery, and a failed or invalid read is not evidence + // of anything. + if (isToolResultEvent(event) && event.sender === aiBotUserId) { + let content = event.content; if ( - !def?.sourceSkillUrl || - def.definition?.type !== 'function' || - !def.definition.function?.name + !isToolResultWithOutputContent(content) || + content['m.relates_to']?.key !== 'applied' ) { continue; } - let defs = bySkill.get(def.sourceSkillUrl) ?? []; - defs.push(def); - bySkill.set(def.sourceSkillUrl, defs); - } - // eventList is chronological, so a later read of the same skill - // replaces the earlier one wholesale. - for (let [skillUrl, defs] of bySkill) { - discoveredBySkill.set(skillUrl, defs); - } - } - for (let [skillUrl, defs] of discoveredBySkill) { - if (disabledSkillIds.has(skillUrl)) { + for (let def of content.data?.discoveredTools ?? []) { + if ( + !def?.sourceSkillUrl || + disabledSkillIds.has(def.sourceSkillUrl) || + def.definition?.type !== 'function' || + !def.definition.function?.name + ) { + continue; + } + add(def.definition as Tool); + } continue; } - for (let def of defs) { - toolMap.set(def.definition.function.name, def.definition); - } - } - let skillsConfigEvent = findLast( - eventList, - (event) => event.type === APP_BOXEL_ROOM_SKILLS_EVENT_TYPE, - ) as SkillsConfigEvent; - - let toolDefinitionFileDefs = - getToolDefinitions(skillsConfigEvent?.content) ?? []; - for (let toolDefinitionFileDef of toolDefinitionFileDefs) { - if (enabledToolNames.has(toolDefinitionFileDef.name)) { - let commandDefinitionContent = await downloadFile( - client, - toolDefinitionFileDef, - ); - let commandDefinitionObject = JSON.parse(commandDefinitionContent); - toolMap.set(toolDefinitionFileDef.name, commandDefinitionObject.tool); - } - } - - // Add in tools from the user's messages - for (let event of eventList) { - if (event.type !== 'm.room.message' || event.sender == aiBotUserId) { - continue; - } - if (event.content.msgtype === APP_BOXEL_MESSAGE_MSGTYPE) { - let eventTools = event.content.data?.context?.tools; - if (eventTools?.length) { - for (let tool of eventTools) { - toolMap.set(tool.function.name, tool); - } + // Tools carried in a user message's context. + if ( + event.type === 'm.room.message' && + event.sender !== aiBotUserId && + event.content.msgtype === APP_BOXEL_MESSAGE_MSGTYPE + ) { + for (let tool of event.content.data?.context?.tools ?? []) { + add(tool); } } } - // Correctness commands should only be emitted by helper flows, not - // directly via LLM tool calls. - toolMap.delete(CHECK_CORRECTNESS_TOOL_NAME); - - return Array.from(toolMap.values()).sort((a, b) => - a.function.name.localeCompare(b.function.name), - ); + return Array.from(toolMap.values()); } export function getLastUserMessage( @@ -1194,8 +1168,6 @@ async function toResultMessages( let attachmentResult = await buildAttachmentsMessagePart( client, toolResult, - history, - true, ); content = [content, attachmentResult.text].filter(Boolean).join('\n\n'); let toolMessage: OpenAIPromptMessage = { @@ -1560,7 +1532,10 @@ function toCheckCorrectnessResultContent( export async function buildPromptForModel( history: DiscreteMatrixEvent[], aiBotUserId: string, - tools: Tool[] = [], + // Kept positionally for the many call sites; the context message no + // longer derives anything from the tools array (the request carries the + // tools separately). + _tools: Tool[] = [], skillCards: EnabledSkill[] = [], disabledSkillIds: string[] = [], client: MatrixClient, @@ -1574,13 +1549,6 @@ export async function buildPromptForModel( throw new Error("Username must be a full id, e.g. '@aibot:localhost'"); } let historicalMessages: OpenAIPromptMessage[] = []; - let lastUserMessageEvent = findLast( - history, - (event) => - event.sender !== aiBotUserId && - event.type === 'm.room.message' && - !isToolOrCodePatchResult(event), - ); for (let event of history) { if (event.type !== 'm.room.message') { continue; @@ -1597,11 +1565,13 @@ export async function buildPromptForModel( let body = event.content.body; if (event.sender === aiBotUserId) { - let codePatchResults = getCodePatchResults( - event as CardMessageEvent, - history, - ); - let content = elideCodeBlocks(body, codePatchResults); + // Past search/replace blocks ride in history verbatim. Eliding them + // rewrote already-sent messages — the placeholder text even changed + // once the patch result arrived — which broke the cache prefix, and + // models imitated the "[Omitting …]" placeholder in place of a real + // patch, stalling the session. Carrying the blocks forward costs + // cached-read tokens instead. + let content = body; let toolCalls = toToolCalls(event as CardMessageEvent); if (content || toolCalls.length) { let historicalMessage: OpenAIPromptMessage = { @@ -1624,12 +1594,9 @@ export async function buildPromptForModel( ).forEach((message) => historicalMessages.push(message)); } if (event.sender !== aiBotUserId) { - let isCurrentMessage = event === lastUserMessageEvent; let attachmentResult = await buildAttachmentsMessagePart( client, event as CardMessageEvent, - history, - isCurrentMessage, inputModalities, ); if (attachmentResult.mediaParts.length > 0) { @@ -1691,10 +1658,8 @@ export async function buildPromptForModel( ]; messages = messages.concat(historicalMessages); let contextContent = await buildContextMessage( - client, history, aiBotUserId, - tools, disabledSkillIds, ); // The context carries the current time, so its bytes change on every @@ -2202,102 +2167,97 @@ function hasAppliedChanges( ); } +// Renders a message's attachments from that message's own snapshot alone. +// Nothing here may depend on later history or on whether the message is the +// current one: the rendering becomes part of the cached prompt prefix, and +// any retroactive change re-bills everything after it (see getAttachedCards). export const buildAttachmentsMessagePart = async ( client: MatrixClient, matrixEvent: MatrixEventWithBoxelContext, - history: DiscreteMatrixEvent[], - isCurrentMessage: boolean = false, inputModalities?: string[], ): Promise<{ text: string; mediaParts: ContentPart[] }> => { - let attachedCards = await getAttachedCards(client, matrixEvent, history); + let attachedCards = await getAttachedCards(client, matrixEvent); let text = ''; if (attachedCards.length > 0) { - text += `Attached Cards (cards with newer versions don't show their content):\n${JSON.stringify(attachedCards, null, 2)}\n`; + text += `Attached Cards (each shows its content as of this message; a later attachment of the same card supersedes it):\n${JSON.stringify(attachedCards, null, 2)}\n`; } - let attachedFiles = await getAttachedFiles( - client, - matrixEvent, - history, - isCurrentMessage, - ); + let attachedFiles = await getAttachedFiles(client, matrixEvent); let mediaParts: ContentPart[] = []; let mediaSourceUrls = new Set(); let unsupportedFiles: { name: string; contentType: string }[] = []; - if (isCurrentMessage) { - for (let f of attachedFiles) { - if (!f.url) { - continue; - } - // Check model capability before downloading - let modality = requiredModality(f.contentType); - if (!modality) { - continue; // not a multimodal type — handled as text metadata below - } - if (inputModalities && !inputModalities.includes(modality)) { - unsupportedFiles.push({ - name: f.name ?? 'unknown', - contentType: f.contentType ?? 'unknown', + for (let f of attachedFiles) { + if (!f.url) { + continue; + } + // Check model capability before downloading + let modality = requiredModality(f.contentType); + if (!modality) { + continue; // not a multimodal type — handled as text metadata below + } + if (inputModalities && !inputModalities.includes(modality)) { + unsupportedFiles.push({ + name: f.name ?? 'unknown', + contentType: f.contentType ?? 'unknown', + }); + continue; + } + try { + if (isImageContentType(f.contentType)) { + let dataUrl = await downloadFileAsBase64DataUrl( + client, + f.url, + f.contentType!, + ); + mediaParts.push({ + type: 'image_url', + image_url: { url: dataUrl }, }); - continue; - } - try { - if (isImageContentType(f.contentType)) { - let dataUrl = await downloadFileAsBase64DataUrl( - client, - f.url, - f.contentType!, - ); - mediaParts.push({ - type: 'image_url', - image_url: { url: dataUrl }, - }); - } else if (isPdfContentType(f.contentType)) { - let dataUrl = await downloadFileAsBase64DataUrl( - client, - f.url, - f.contentType!, - ); - mediaParts.push({ - type: 'file', - file: { - filename: f.name ?? 'document.pdf', - file_data: dataUrl, - }, - }); - } else if (isAudioContentType(f.contentType)) { - let format = audioFormatFromMime(f.contentType!); - if (!format) { - getLog().error(`Unsupported audio format: ${f.contentType}`); - continue; - } - let dataUrl = await downloadFileAsBase64DataUrl( - client, - f.url, - f.contentType!, - ); - // Strip data URL prefix — OpenRouter expects raw base64 for audio - let base64 = dataUrl.replace(/^data:[^;]+;base64,/, ''); - mediaParts.push({ - type: 'input_audio', - input_audio: { data: base64, format }, - }); - } else if (isVideoContentType(f.contentType)) { - let dataUrl = await downloadFileAsBase64DataUrl( - client, - f.url, - f.contentType!, - ); - mediaParts.push({ - type: 'video_url', - video_url: { url: dataUrl }, - }); - } - if (f.sourceUrl) { - mediaSourceUrls.add(f.sourceUrl); + } else if (isPdfContentType(f.contentType)) { + let dataUrl = await downloadFileAsBase64DataUrl( + client, + f.url, + f.contentType!, + ); + mediaParts.push({ + type: 'file', + file: { + filename: f.name ?? 'document.pdf', + file_data: dataUrl, + }, + }); + } else if (isAudioContentType(f.contentType)) { + let format = audioFormatFromMime(f.contentType!); + if (!format) { + getLog().error(`Unsupported audio format: ${f.contentType}`); + continue; } - } catch (e) { - getLog().error(`Failed to download media file ${f.url}:`, e); + let dataUrl = await downloadFileAsBase64DataUrl( + client, + f.url, + f.contentType!, + ); + // Strip data URL prefix — OpenRouter expects raw base64 for audio + let base64 = dataUrl.replace(/^data:[^;]+;base64,/, ''); + mediaParts.push({ + type: 'input_audio', + input_audio: { data: base64, format }, + }); + } else if (isVideoContentType(f.contentType)) { + let dataUrl = await downloadFileAsBase64DataUrl( + client, + f.url, + f.contentType!, + ); + mediaParts.push({ + type: 'video_url', + video_url: { url: dataUrl }, + }); + } + if (f.sourceUrl) { + mediaSourceUrls.add(f.sourceUrl); } + } catch (e) { + getLog().error(`Failed to download media file ${f.url}:`, e); } } if (unsupportedFiles.length > 0) { @@ -2307,7 +2267,7 @@ export const buildAttachmentsMessagePart = async ( text += `Note: The following files were not sent to the model because it does not support their input type: ${fileList}\n`; } if (attachedFiles.length > 0) { - text += `Attached Files (files with newer versions don't show their content):\n${attachedFilesToMessage( + text += `Attached Files (each shows its content as of this message; a later attachment of the same file supersedes it):\n${attachedFilesToMessage( attachedFiles, { omitSourceUrls: mediaSourceUrls, @@ -2318,16 +2278,13 @@ export const buildAttachmentsMessagePart = async ( }; export const buildContextMessage = async ( - client: MatrixClient, history: DiscreteMatrixEvent[], aiBotUserId: string, - tools: Tool[], disabledSkillIds: string[], ): Promise => { let result = ''; let attachedFiles = await loadCurrentlySerializedFileDefs( - client, history, aiBotUserId, ); @@ -2432,17 +2389,19 @@ export const buildContextMessage = async ( } result += `\nCurrent date and time: ${new Date().toISOString()}\n`; - let cardPatchTool = tools.find( - (tool) => tool.function.name === 'patchCardInstance', - ); - + // The old wording keyed off the per-card patchCardInstance tool, which + // interactive messages no longer inject (its card-specific schema churned + // the tools array). The access statement is now derived from what the + // user actually shared: with no attached files and no open cards, there + // is nothing the model may write to, and it should ask rather than guess + // at ids. if ( attachedFiles.length == 0 && - !cardPatchTool && + (context?.openCardIds?.length ?? 0) === 0 && hasSomeAttachedCards(history, aiBotUserId) ) { result += - 'You are unable to edit any cards, the user has not given you access, they need to open the card and let it be auto-attached.'; + 'You are unable to edit any cards: the user has no card open and none attached to this message. Ask them to open the card they want changed.'; } return result; @@ -2662,48 +2621,6 @@ export function isCodePatchResultEvent( ); } -function elideCodeBlocks( - content: string, - codePatchResults: CodePatchResultEvent[], -) { - const DEFAULT_PLACEHOLDER: string = - '[Omitting previously suggested code change]'; - const PLACEHOLDERS = { - applied: '[Omitting previously suggested and applied code change]', - failed: '[Omitting previously suggested code change that failed to apply]', - }; - - function getPlaceholder(codeBlockIndex: number) { - let codePatchResult = codePatchResults.find((codePatchResult) => { - return codePatchResult.content.codeBlockIndex === codeBlockIndex; - }); - if (codePatchResult) { - return ( - PLACEHOLDERS[codePatchResult.content['m.relates_to'].key] ?? - DEFAULT_PLACEHOLDER - ); - } - return DEFAULT_PLACEHOLDER; - } - - let codeBlockIndex = 0; - - for (;;) { - const block = findSearchReplaceBlock(content); - if (!block) { - return content; - } - - // replace the content between the markers with a placeholder - content = - content.substring(0, block.start) + - getPlaceholder(codeBlockIndex) + - content.substring(block.end); - - codeBlockIndex++; - } -} - export function mxcUrlToHttp(mxc: string, baseUrl: string): string { if (mxc.indexOf('mxc://') !== 0) { throw new Error('Invalid MXC URL ' + mxc);