[WRONG BRANCH] fix(responses): bound combo recall memory - #472
Conversation
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. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 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 |
|
✅ Deterministic PR hygiene checks passed. |
github-actions
Bot
changed the title
⏳ DRAFT
What to do
Its title has been prefixed with |
![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: 5619d6a777
ℹ️ 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 (!lane || !comboId || !responseModel.trim()) return; | ||
| const modelBytes = boundedModelBytes(responseModel); | ||
| if (modelBytes === undefined) return; |
There was a problem hiding this comment.
When a lane already has a cached entry and its next completed combo response reports a model over 1 KiB, this early return leaves the previous response registered as the lane's latest result. If the client later submits that previous bare model for compaction, recallComboForLane can incorrectly route it back through the old combo even though a newer response completed on the lane. After validating the writer generation and owner, invalidate the lane when the new model cannot be retained, and add a valid-to-oversized transition test.
Useful? React with 👍 / 👎.
1 participant
Motivation
response.modelstrings from being stored unbounded in the process-global combo recall cache and causing aggregate memory exhaustion.Description
response.modelvalues above 1 KiB before remembering them inrememberComboForLane(bounded early on the JS string).responseModelBytesand enforce a 64 KiB aggregate model-string budget that evicts oldest entries when exceeded, in addition to the existing 256-entry cap.sweepExpiredComboRecalland register it inSTATE_STORE_REGISTRATIONSso dormant expired lanes are reclaimed by the global state-store sweeper.clearComboRecallForTestsreset hook.Testing
bun test tests/oauth/state-store-sweeper.test.ts— focused regression tests passed.bun run typecheck— typecheck passed.bun run privacy:scan— privacy scan passed.bun run test:changed— unavailable in this checkout (nodevcomparison ref), and a fullbun run testwas started but exposed unrelated parallel-suite failures in other subsystems and was not used as the focused verification; the targeted tests for this change remained green.Codex Task