-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(cursor): refund spare envelope bytes to clipped invocation arguments #4543
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
bf8e04b
4ca2979
fd04b5d
3d41c8c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -325,7 +325,12 @@ function rootPromptMessages( | |
| const replacement = rootBlobCandidate( | ||
| { role: payload.role, content: [{ type: "text", text: marked }] }, | ||
| role, | ||
| { ...opts, messageIndex: previous.entry.messageIndex ?? opts.messageIndex }, | ||
| // `text` must mirror the payload actually stored, not the unmarked text it was built from. | ||
| // It did not, and every consumer that rebuilds a root from `text` therefore dropped the run | ||
| // note: truncating a collapsed root silently deleted the "produced N times" line, and so did | ||
| // the invocation-argument restoration below. The note is the repetition breaker's per-entry | ||
| // half, so losing it re-primes the self-reinforcing loop the breaker exists to end. | ||
| { ...opts, text: marked, messageIndex: previous.entry.messageIndex ?? opts.messageIndex }, | ||
| ); | ||
| entries[entries.indexOf(previous.entry)] = replacement; | ||
| replayRuns.set(role, { text: normalized, entry: replacement, length: runLength }); | ||
|
|
@@ -694,6 +699,20 @@ function rootPromptMessages( | |
| historyMessageStart = firstKept?.messageIndex ?? (messages.length); | ||
| } | ||
|
|
||
| // Refund envelope bytes the assembled set left unused to invocation arguments the per-call cap | ||
| // clipped. Gated on `echoToolResultInRoot`, not `externalModel`: native `composer-2.5` echoes its | ||
| // results into roots without being an external wire model, so the narrower gate would leave the one | ||
| // native model that has clipped invocation lines capped for no reason (#4516). | ||
| if (echoToolResultInRoot && replayedCalls) { | ||
| selected = restoreClippedInvocationArguments( | ||
| selected, | ||
| messages, | ||
| replayedCalls, | ||
| knownCallsOffset, | ||
| carriedRoots.byteLength, | ||
| ); | ||
| } | ||
|
|
||
| return { | ||
| ids: selected.map(entry => storeCursorBlob(entry.data, requestScope)), | ||
| byteLength: selected.reduce((sum, entry) => sum + entry.byteLength, 0), | ||
|
|
@@ -967,6 +986,83 @@ function toolInvocationLine(call: Extract<OcxAssistantContentPart, { type: "tool | |
| return `invoked: ${namespacedToolName(call.namespace, call.name)} with ${toolCallArgumentsText(call.arguments)}`; | ||
| } | ||
|
|
||
| /** | ||
| * Second pass over the assembled root set: spend envelope bytes nothing else claimed on invocation | ||
| * arguments the per-call cap clipped. | ||
| * | ||
| * `CURSOR_INVOCATION_ARGUMENTS_BYTE_LIMIT` is charged while the envelope is still being built, so it | ||
| * costs a call 2 KiB whether or not anything else wants those bytes. In a small replay nearly the | ||
| * whole 192-root / 512 KiB envelope goes unused and the cap still bites: a 4,693-byte successful | ||
| * `write_file` lost its tail inside a 6,011-byte replay, and because the result text does not repeat | ||
| * the argument, the model could no longer see what it had just written (#4516). | ||
| * | ||
| * The cap stays, and admission is still decided on its 2 KiB prefix — it is what keeps a 600 KiB | ||
| * argument from evicting the output it describes. This pass only refunds leftover aggregate bytes, | ||
| * after every pruning and truncation decision is already final: | ||
| * | ||
| * - newest `toolResult` first, because the argument the model is most likely to still need is the | ||
| * one belonging to the call it just made; | ||
| * - only out of `spare`, so restoring can never push the envelope past its own limit; | ||
| * - never for an `outputElided` root, whose own output is already gone — widening the invocation | ||
| * there would spend the last free bytes describing an answer that is not present; | ||
| * - never by dropping, shrinking or reordering another root, so nothing pruning chose to keep is | ||
| * evicted to pay for a wider invocation line. | ||
| */ | ||
| function restoreClippedInvocationArguments( | ||
| selected: RootBlobCandidate[], | ||
| messages: readonly OcxMessage[], | ||
| replayedCalls: Map<string, Extract<OcxAssistantContentPart, { type: "toolCall" }>>, | ||
| knownCallsOffset: number, | ||
| carriedBytes: number, | ||
| ): RootBlobCandidate[] { | ||
| let spare = CURSOR_EXTERNAL_ROOT_BYTE_LIMIT | ||
| - carriedBytes | ||
| - selected.reduce((sum, entry) => sum + entry.byteLength, 0); | ||
| if (spare <= 0) return selected; | ||
| const restored = [...selected]; | ||
| for (let i = restored.length - 1; i >= 0 && spare > 0; i--) { | ||
| const entry = restored[i]; | ||
| if (!entry || entry.role !== "toolResult" || entry.outputElided === true) continue; | ||
| if (entry.text === undefined || entry.messageIndex === undefined) continue; | ||
| const message = messages[entry.messageIndex]; | ||
| if (message?.role !== "toolResult") continue; | ||
| // Same full-history bound the envelope builder used: `messageIndex` is local to this call's | ||
| // `rawMessages`, and `knownCallsOffset` re-bases it when only a suffix is replayed. | ||
| const call = callBefore( | ||
| replayedCalls, | ||
| decodeCursorCallId(message.toolCallId), | ||
| knownCallsOffset + entry.messageIndex, | ||
| ); | ||
| if (!call) continue; | ||
| const full = serializeToolCallArguments(call.arguments); | ||
| if (full === undefined) continue; | ||
| const clipped = toolCallArgumentsText(call.arguments); | ||
| if (clipped === full) continue; | ||
| const name = namespacedToolName(call.namespace, call.name); | ||
| // Anchored on the preceding newline. `toolResultToText` always emits the invocation after the | ||
| // `[tool_result]`, `call_id:` and `name:` lines, so the real line is never first — and | ||
| // `name:` renders the RESULT's tool name, which nothing sanitizes, so an unanchored search could | ||
| // be satisfied by a crafted tool name and rewrite that header instead of the invocation. | ||
| const clippedLine = `\ninvoked: ${name} with ${clipped}`; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Match the invocation line structurally before restoring arguments.
A result name containing 🤖 Prompt for AI Agents |
||
| // Absent when truncation already cut through the invocation line itself; there is nothing to | ||
| // widen in that root, and re-rendering the envelope would undo the output truncation too. | ||
| if (!entry.text.includes(clippedLine)) continue; | ||
| // Callback replacement: serialized arguments routinely contain `$&`, `$'` and `$1`, and the | ||
| // string form of `replace` expands those into the surrounding match instead of inserting them. | ||
| const widened = entry.text.replace(clippedLine, () => `\ninvoked: ${name} with ${full}`); | ||
| const candidate = rootBlobCandidate( | ||
| toolResultRootPayload(widened), | ||
| "toolResult", | ||
| { messageIndex: entry.messageIndex, text: widened }, | ||
| ); | ||
| const cost = candidate.byteLength - entry.byteLength; | ||
| if (cost <= 0 || cost > spare) continue; | ||
| restored[i] = candidate; | ||
| spare -= cost; | ||
| } | ||
| return restored; | ||
| } | ||
|
|
||
| /** | ||
| * History position of each indexed call, keyed by the map `toolCallsByCallId` returned. | ||
| * | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This changes behavior under
src/adapters/, but the commit updates onlystructure/providers/cursor.md;structure/INDEX.mdalso maps this area to the runtime, byte-accounting, Responses, transport-inventory, inbound-compatibility, chat-compatibility, and adapter-registry contracts. Update each mapped document in this change so their descriptions remain synchronized with the new replay-budget behavior.AGENTS.md reference: structure/AGENTS.md:L49-L50
Useful? React with 👍 / 👎.