fix(cursor): estimate context after restart#376
Conversation
📝 WalkthroughWalkthroughCursor requests now estimate visible input tokens, store the estimate in protobuf event state, and use it for estimated usage when checkpoint or carried context totals are unavailable. Request encoding and model-prefix coverage are updated, with tests covering estimation and finalized or partial usage paths. ChangesCursor usage estimation
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant CursorRunRequest
participant liveTransport
participant protobufEvents
participant UsageConsumer
CursorRunRequest->>liveTransport: run request
liveTransport->>protobufEvents: initialize state with estimatedInputTokens
protobufEvents->>protobufEvents: finalize usage without checkpoint or carry
protobufEvents->>UsageConsumer: estimated input, output, and total tokens
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7869c0020
ℹ️ 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".
| export function estimateCursorInputTokens(request: CursorRunRequest): number { | ||
| return estimateTokens(JSON.stringify({ | ||
| system: request.system, | ||
| messages: request.rawMessages ?? request.messages, |
There was a problem hiding this comment.
When a Cursor request includes an image part and there is no checkpoint/carry-forward usage available (for example after a proxy restart), this fallback estimates from rawMessages, which still contains the full imageUrl/data URL. The Cursor adapter actually converts image parts to a short unsupported-image placeholder before sending them, so a large screenshot can be reported as hundreds of thousands of input tokens even though those bytes were not in Cursor's prompt; that can inflate usage logs and trigger compaction far too early. Use the already-normalized request.messages or the same text conversion used for encoding when computing the estimate.
Useful? React with 👍 / 👎.
|
Maintainer triage review (2026-07-24). Not mergeable as-is — the estimate ignores the wire-level pruning that actually ships: High — estimation counts what we no longer send. src/adapters/cursor/protobuf-request.ts:325-330 computes over the full pre-truncation rawMessages + all tools, but current dev prunes external-model input to 192 blobs / 512 KiB and drops older history (protobuf-request.ts:53-59,223-290). Long sessions will over-report context and can trigger premature compaction. The estimator must share the same pruned/filtered payload view as the request builder. Ratio mismatch. The estimator applies a generic ~4 chars/token for Grok (src/lib/token-estimate.ts:24-31) while the same file documents that code/JSON needs a lower ratio — under the cap it can under-estimate instead. Tests bypass the real function. tests/cursor-protobuf-events.test.ts:416-422 and cursor-interaction-query.test.ts:174 inject 1,200 directly instead of calling estimateCursorInputTokens() — so pruning, tool filtering, and restart/compaction boundaries are unverified. Good bones though: the checkpoint -> carry-forward -> request-local priority and input+output=total accounting are sound, and not persisting the estimate avoids stale carry. CI is green and a test-merge against current dev typechecks. Please rebuild so the estimator consumes the same pruned payload the request sends, with regression tests at those boundaries. Happy to re-review. |
Use a request-local input estimate when Cursor emits no checkpoint and the process-local context cache is empty.\n\nFixes lidge-jun#373\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
b7869c0 to
01466a7
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/adapters/cursor/live-transport.ts`:
- Line 5: Update the run flow around
cursorContextUsageTracker.controlsForConversation and estimateCursorInputTokens
so the estimate is computed only when no carryForwardTokens value is available.
Create the encoded request payload once and reuse it for both token estimation
and the later open/encodeCursorRunRequest path, avoiding duplicate
rootPromptMessages work while preserving carry-forward precedence.
In `@src/adapters/cursor/protobuf-request.ts`:
- Around line 509-538: Refactor modelVisibleRequestPayload usage so each turn
computes the payload only once: allow estimateCursorInputTokens and
encodeCursorRunRequest to accept a shared payload, and update the live transport
run/open flow to create it once and pass it through. In the same flow, skip
estimateCursorInputTokens when contextCarryForwardTokens is already available,
since the estimate is not used in that case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0ed25d97-73d6-47c4-b586-20463c309113
📒 Files selected for processing (8)
src/adapters/cursor/live-transport.tssrc/adapters/cursor/protobuf-events.tssrc/adapters/cursor/protobuf-request.tssrc/lib/token-estimate.tstests/cursor-blob.test.tstests/cursor-interaction-query.test.tstests/cursor-protobuf-events.test.tstests/token-estimate.test.ts
| import { namespacedToolName, type OcxProviderConfig, type OcxUsage } from "../../types"; | ||
| import { CONNECT_FLAG_END_STREAM, decodeAvailableConnectFrames, encodeConnectFrame } from "./framing"; | ||
| import { activePromptText, encodeCursorRunRequest } from "./protobuf-request"; | ||
| import { activePromptText, encodeCursorRunRequest, estimateCursorInputTokens } from "./protobuf-request"; |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Estimate is computed unconditionally, even when carry-forward already makes it unreachable — and duplicates the encode-time payload work.
estimateCursorInputTokens(request) (Line 517) is called on every run() regardless of whether cursorContextUsageTracker.controlsForConversation(...) (Line 518-521) already resolves a carryForwardTokens value. Per reportableContextTokens/finalizeTurnEvents in protobuf-events.ts, carry-forward always takes precedence over the estimate, so for the common case (an ongoing conversation with an already-known context total) this call's rootPromptMessages work is pure waste. Worse, this.open(...) later calls encodeCursorRunRequest(request) (Line 799), which independently re-derives the same payload — see companion comment on protobuf-request.ts Lines 509-538 for the shared root cause and proposed fix.
♻️ Proposed fix: gate the estimate on carry-forward absence, and reuse a single payload
+ const contextUsage = cursorContextUsageTracker.controlsForConversation(request.conversationId, {
+ clearPrior: request.contextUsageReset === true,
+ storeCheckpoints: request.contextUsageStoreCheckpoints !== false,
+ });
state = createCursorProtobufEventState({
clientToolNames: clientToolDefs.map(tool => tool.toolName || tool.name),
parallelToolCalls: request.parallelToolCalls,
toolSchemas,
cursorToolNameMap,
- estimatedInputTokens: estimateCursorInputTokens(request),
- contextUsage: cursorContextUsageTracker.controlsForConversation(request.conversationId, {
- clearPrior: request.contextUsageReset === true,
- storeCheckpoints: request.contextUsageStoreCheckpoints !== false,
- }),
+ estimatedInputTokens: contextUsage.carryForwardTokens === undefined
+ ? estimateCursorInputTokens(request)
+ : undefined,
+ contextUsage,
});Also applies to: 512-522
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/adapters/cursor/live-transport.ts` at line 5, Update the run flow around
cursorContextUsageTracker.controlsForConversation and estimateCursorInputTokens
so the estimate is computed only when no carryForwardTokens value is available.
Create the encoded request payload once and reuse it for both token estimation
and the later open/encodeCursorRunRequest path, avoiding duplicate
rootPromptMessages work while preserving carry-forward precedence.
| function modelVisibleRequestPayload(request: CursorRunRequest) { | ||
| const rawText = activePromptText(request); | ||
| const lastRole = request.messages.at(-1)?.role; | ||
| const text = lastRole === "user" || lastRole === "developer" | ||
| ? appendCursorShellAliasHint(request.tools, appendCursorGenericToolUseHint(request.tools, rawText)) | ||
| : rawText; | ||
| return { | ||
| text, | ||
| roots: rootPromptMessages(request), | ||
| mcpToolDefs: buildCursorToolDefinitions( | ||
| cursorToolsForActivePrompt(request.tools, rawText, request.toolChoice), | ||
| request.toolChoice, | ||
| ), | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Conservative request-local fallback when Cursor emits neither a checkpoint nor a previously | ||
| * observed context total. This is deliberately not persisted: a later authoritative checkpoint | ||
| * remains the only source carried across turns. | ||
| */ | ||
| export function estimateCursorInputTokens(request: CursorRunRequest): number { | ||
| const payload = modelVisibleRequestPayload(request); | ||
| const lastRawIsToolResult = request.rawMessages?.at(-1)?.role === "toolResult"; | ||
| const action = !lastRawIsToolResult && payload.text.trim().length > 0 ? payload.text : ""; | ||
| return estimateTokens(JSON.stringify({ roots: payload.roots.serialized, action, tools: payload.mcpToolDefs }), request.modelId); | ||
| } | ||
|
|
||
| export function encodeCursorRunRequest(request: CursorRunRequest): Uint8Array { | ||
| const payload = modelVisibleRequestPayload(request); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
modelVisibleRequestPayload(request) is now computed twice per turn — once for the estimate, once for the real encode.
estimateCursorInputTokens() (Line 530-535) calls modelVisibleRequestPayload(request), which internally re-runs rootPromptMessages(request): iterating every raw message, JSON.stringify-ing it, UTF‑8 encoding it, and SHA‑256‑hashing it via storedRootBlob/storeCursorBlob for every historical message before truncation is applied. encodeCursorRunRequest() (Line 537-538) then calls modelVisibleRequestPayload(request) again for the same request, redoing all of that work a second time.
Before this PR, rootPromptMessages was invoked exactly once per turn (only inside encodeCursorRunRequest). Now it's invoked twice on every single Cursor turn, and — per the caller in live-transport.ts (Line 512-522) — the estimate is computed even when contextCarryForwardTokens already exists and will make the estimate unreachable in finalizeTurnEvents/partialUsageFromEventState (checkpoint/carry always take precedence). For long conversations approaching the 192-blob/512 KiB budget, that's real, unconditional CPU/crypto work being duplicated (and often wasted) on the request-handling hot path.
♻️ Proposed fix: share one payload computation, and skip the estimate when carry-forward is already known
-function modelVisibleRequestPayload(request: CursorRunRequest) {
+export function modelVisibleRequestPayload(request: CursorRunRequest) {
const rawText = activePromptText(request);
...
}
-export function estimateCursorInputTokens(request: CursorRunRequest): number {
- const payload = modelVisibleRequestPayload(request);
+export function estimateCursorInputTokens(
+ request: CursorRunRequest,
+ payload: ReturnType<typeof modelVisibleRequestPayload> = modelVisibleRequestPayload(request),
+): number {
const lastRawIsToolResult = request.rawMessages?.at(-1)?.role === "toolResult";
const action = !lastRawIsToolResult && payload.text.trim().length > 0 ? payload.text : "";
return estimateTokens(JSON.stringify({ roots: payload.roots.serialized, action, tools: payload.mcpToolDefs }), request.modelId);
}
-export function encodeCursorRunRequest(request: CursorRunRequest): Uint8Array {
- const payload = modelVisibleRequestPayload(request);
+export function encodeCursorRunRequest(
+ request: CursorRunRequest,
+ payload: ReturnType<typeof modelVisibleRequestPayload> = modelVisibleRequestPayload(request),
+): Uint8Array {Then in live-transport.ts, compute payload once in run(), pass it to estimateCursorInputTokens(request, payload), and thread it through open() into encodeCursorRunRequest(request, payload) (see companion comment on that file).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/adapters/cursor/protobuf-request.ts` around lines 509 - 538, Refactor
modelVisibleRequestPayload usage so each turn computes the payload only once:
allow estimateCursorInputTokens and encodeCursorRunRequest to accept a shared
payload, and update the live transport run/open flow to create it once and pass
it through. In the same flow, skip estimateCursorInputTokens when
contextCarryForwardTokens is already available, since the estimate is not used
in that case.
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
The missing post-restart context estimate is a real issue, but the current estimate still diverges from what Cursor actually receives. It must consume the already-pruned and normalized wire payload, avoid duplicate request construction, and be computed only when checkpoint/carry-forward data is absent. Please add regressions through estimateCursorInputTokens for pruning, tool filtering, image placeholders, and restart boundaries, then re-request review.
Summary
Fixes #373
Validation
bun run typecheckbun test tests/cursor-protobuf-events.test.ts tests/cursor-tool-continuation.test.ts tests/cursor-interaction-query.test.ts tests/cursor-request-builder.test.tsSummary by CodeRabbit
Bug Fixes
Tests