fix(cursor): prefer observed checkpoint maxTokens as the overflow ceiling - #4816
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (12)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughCursor now records positive checkpoint ChangesCursor adapter context-window observations
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant CursorRequest
participant LiveCursorTransport
participant createCursorProtobufEventState
participant recordObservedCursorContextWindow
participant cursorRequestSizeContext
participant inferCursorContextWindow
CursorRequest->>LiveCursorTransport: send request with identity scope
LiveCursorTransport->>createCursorProtobufEventState: pass wire model and identity scope
createCursorProtobufEventState->>recordObservedCursorContextWindow: record checkpoint maxTokens
CursorRequest->>cursorRequestSizeContext: prepare next request
cursorRequestSizeContext->>inferCursorContextWindow: resolve scoped context window
inferCursorContextWindow-->>cursorRequestSizeContext: return observed or heuristic window
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The observed context-window behavior remains isolated by supported identity routing and does not require separate Max Mode state. No merge-blocking risk is established. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 15.38% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 8 files. (4 skipped: 4 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
![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: 8763dee2d2
ℹ️ 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".
| ): void { | ||
| if (!modelId.trim()) return; | ||
| if (typeof maxTokens !== "number" || !Number.isFinite(maxTokens) || maxTokens <= 0) return; | ||
| observedCursorContextWindows.set(normalizeObservedWindowKey(modelId), Math.floor(maxTokens)); |
There was a problem hiding this comment.
observedCursorContextWindows is process-global and keyed only by model ID, even though this ceiling is account-specific and variants such as Max Mode retain the same wire model ID. In a shared proxy, a 200k checkpoint from credential A can make credential B with a 32k ceiling classify a 20k bare RESOURCE_EXHAUSTED as a 429 because 20k < 0.5 * 200k, rather than as an overflow; a Max Mode observation can similarly poison ordinary turns. This changes error mapping and the associated overflow recovery for unrelated requests, so key the observation by authenticated identity and effective request mode, or keep it in account-scoped transport state.
AGENTS.md reference: src/AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
리뷰 · 우선순위 73 / 80이 PR은 Cursor가 체크포인트로 알려 주는 진짜 컨텍스트 한도( 현재 동작 갈래는 테스트로 꽤 잘 잡혀 있습니다. 라인 / 심볼 수준의 메모입니다.
메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
Requesting changes on exact head 47e8fe61ff4fd4546d341315a8ebcf9992ac7b5e.
The observed checkpoint ceiling cannot be stored process-wide by model id alone. Cursor ceilings are credential/account and request-mode dependent; one account or Max-mode observation can therefore change overflow-vs-429 classification and recovery for another account sharing the same wire model id. Key observations by authenticated identity plus effective request mode, or keep them in account-scoped transport state, with bounded retention and regressions proving cross-account and ordinary-vs-Max isolation.
This PR also targets the non-integration branch cursor/l1-text-toolcall-quarantine, while current policy accepts only dev, and its parent #4815 has unresolved P1 tool-execution blockers. Land/fix the parent first, rebuild this change on current dev, resolve the account-scope issue, and run exact-head hosted CI before re-requesting review.
47e8fe6 to
fddfc57
Compare
…ling Plan-gated Cursor accounts advertise a smaller window than the id heuristic. Record a positive conversationCheckpointUpdate.tokenDetails.maxTokens and feed it into the existing 0.5-window overflow vs 429 prior. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
fddfc57 to
5be47f3
Compare
2 participants
Summary
dev, so this no longer sits on a stack: it now targetsdevdirectly and its diff is only the observed-window change.conversationCheckpointUpdate.tokenDetails.maxTokensand prefer it ininferCursorContextWindow, socursorRequestSizeContextfeeds the real plan-gated ceiling into the existing 0.5-window overflow vs 429 prior._cursorIdentityScopenormalization thatcursorOverflowRemintScopeKeyandcursorConversationIdFromClientThreadalready use rather than a second notion of "account". Keyed on the model id alone, two plans sharing one proxy overwrite each other: a free-plan 32k observation reclassifies a paid account's genuine overflow as a 429, and the reverse hides a real overflow.CURSOR_OVERFLOW_REMINT_MAX_ENTRIES.inferCursorContextWindowtakes an options object so a caller that omits the scope is a type error rather than a silent global lookup, and_cursorIdentityScopeis declared onCursorRunRequestinstead of being carried through a cast.maxTokens(the first senpi checkpoint is 0) keeps the id heuristic. A 20-token request against an observed 32k ceiling stays on the 429 class within its own scope.Verification
tests/providers/cursor/cursor-discovery.test.tsandcursor-errors.test.ts: two scopes observing different ceilings for one model id do not see each other's value; an unscoped request does not read a scoped observation; eviction keeps the map bounded; zero and negative ceilings are ignored; the 20-token-against-32k case stays on the 429 class inside its scope.devlog/_plan/260917_l5_cursor_stabilization/020_u2_observed_window_scope.md, now ondev.Checklist
Summary by CodeRabbit
New Features
Documentation
Tests