Repository navigation
fix(openai): compress tool history on Responses requests - #1958
Conversation
📝 WalkthroughWalkthroughThe OpenAI shim gates conversion and compression by transport, builds Responses input from compressed messages, detects images across serialized request formats, and preserves image ownership through message processing. Tests cover compression, routing, retries, image handling, Codex serialization, and abort propagation. ChangesOpenAI shim transport behavior
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/api/openaiShim.ts (1)
3859-3888: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winSkip compression for Anthropic/Gemini routes
compressedMessagesis still built beforeeffectiveTransportis known, so/messagesand/models/gemini-*requests pay forcompressToolHistory(...)even though those paths useparams.messagesdirectly. Move compression behind the transport check so it only runs for routes that consume it.🤖 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/services/api/openaiShim.ts` around lines 3859 - 3888, Move the `compressToolHistory` call out of the initial `compressedMessages` assignment and perform it only after `effectiveTransport` is determined for transports that consume compressed messages. Ensure `/messages` and `/models/gemini-*` routes use `rawMessages` or their existing direct message handling without invoking compression, while preserving current compression behavior for applicable routes.
🤖 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.
Outside diff comments:
In `@src/services/api/openaiShim.ts`:
- Around line 3859-3888: Move the `compressToolHistory` call out of the initial
`compressedMessages` assignment and perform it only after `effectiveTransport`
is determined for transports that consume compressed messages. Ensure
`/messages` and `/models/gemini-*` routes use `rawMessages` or their existing
direct message handling without invoking compression, while preserving current
compression behavior for applicable routes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 346b64b9-d7db-4b8f-99ed-fd05459121f0
📒 Files selected for processing (2)
src/services/api/openaiShim.compression.test.tssrc/services/api/openaiShim.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
Files:
src/services/api/openaiShim.compression.test.tssrc/services/api/openaiShim.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/services/api/openaiShim.compression.test.tssrc/services/api/openaiShim.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/openaiShim.compression.test.tssrc/services/api/openaiShim.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/api/openaiShim.compression.test.ts
🔇 Additional comments (11)
src/services/api/openaiShim.ts (7)
903-963: LGTM!
1486-1492: LGTM!
3894-3924: LGTM!
4060-4131: Responses input caching looks correct.
getResponsesInput()memoizes the converted array;buildResponsesBodyonly reassigns its owninputproperty when empty rather than mutating the shared cached array, so repeated calls across retries/fallback stay consistent.
4592-4663: LGTM!Caching keyed by the exact serialized string correctly invalidates when the body changes (e.g. after
omitResponsesToolstriggersrefreshSerializedBody).
4907-4928: Correct fix: fallback classification now uses its own serialized body.Previously (per summary)
bodyContainsImages()classified the/responsesCopilot-fallback failure using the outer chat-completionsserializedBody; now it correctly usesresponsesSerializedBody, the body actually sent to the fallback endpoint.
5149-5149: LGTM!src/services/api/openaiShim.compression.test.ts (4)
5-5: LGTM!Also applies to: 17-22, 134-163, 208-237, 253-335
501-642: LGTM!Good coverage of structured compression, tool output truncation, image content typing (
input_image/input_text/output_text), and the no-tool-result passthrough case.
644-700: LGTM!
701-847: LGTM!Good regression coverage for single-pass conversion during error classification, local self-healing image routing, JSON-normalized (
toJSON) image detection, and pre-abort signal propagation.
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Detailed Findings
1. [Minor] messages key dropped for empty chat array (openaiShim.ts:3924)
The request body is built as:
const body = { model, ...(openaiMessages ? { messages: openaiMessages } : {}), stream, store }openaiMessages is undefined for non-chat transports (correct) or [] if convertMessages legitimately returns an empty array. An empty array is falsy, so messages would be omitted from a chat_completions request — changing the wire contract (some providers distinguish "missing messages" from "empty messages") versus the prior unconditional messages: openaiMessages.
Reachability: In the current Claude Code flow a request always carries ≥1 message, so convertMessages returns a non-empty array; the empty case is latent. Still a real footgun for any future path (e.g., empty-history probes / external SDK callers) and should be hardened.
Suggested fix: gate on presence, not truthiness:
...(openaiMessages !== undefined ? { messages: openaiMessages } : {})2. [Minor] Gemini fileData false-positive for vision_not_supported (openaiShim.ts:952-961)
return Array.isArray(parts) && parts.some(part =>
part && typeof part === 'object' && ('inlineData' in part || 'fileData' in part))Any Gemini part with fileData (audio, pdf, etc.) is treated as an image. A 400/404 on a request carrying a non-image file would be misclassified as vision_not_supported (see openaiErrorClassification.ts:399-429), giving misleading "model may not support vision" guidance.
Suggested fix: only treat inlineData (or inspect inlineData.mimeType for image/*) as image content; do not blanket-treat fileData.
3. [Minor] Responses/Chat tool_result image detection is dead code (pre-existing gap)
contentBlocksContainImages recurses into tool_result content looking for image blocks, but in practice tool-result images are flattened to text/[Image](url) before serialization (codexShim.ts for Responses; convertToolResultContent for Chat). So the tool_result branch never fires for a serialized request body, and a Responses request containing only tool-result images reports hasImages=false. This is a pre-existing limitation exposed by the new detection code, not a regression from this PR, and the PR's stated claim ("…including nested tool_result image blocks") only truly holds for the raw Anthropic messages shape.
4. [Minor] Misleading test name — does not exercise image classification (VALIDATED)
image error classification follows the serialized body for local self-healing asserts a /chat/completions → /v1/chat/completions self-heal for a 404. I executed this test: it passes. Inspecting the captured request body shows the assistant image is reduced to the text "Image attached." — the image is stripped before serialization because localhost:8000 is detected as a native-Ollama chat endpoint (ensureTextPartForImageContent fallback). Consequently bodyContainsImages() returns false, classification is endpoint_not_found, and the local retry fires.
Implication: the test validates only the local /v1 retry, not image error classification on the chat path. Genuine image-classification coverage lives in image error classification follows JSON-normalized Anthropic content (which uses a remote OpenAI-compatible endpoint where the image stays on the wire and correctly asserts openai_category=vision_not_supported). The localized test's name overclaims its coverage; consider renaming or adding a real image-bearing local assertion. (Note: this refutes an earlier "the test would fail / mask a false-negative" hypothesis — validation confirmed it passes and the image is simply absent from the wire body.)
5. [Minor] Silent JSON.parse catch in bodyContainsImages (openaiShim.ts:4604-4613)
try { const payload = JSON.parse(bodyToInspect); /* detect images */ }
catch { /* Request serialization already succeeded before this error path. */ }If the serialized body were ever non-JSON, this would silently report hasImages=false, potentially routing a vision 404 as endpoint_not_found. Currently unreachable because both call sites pass guaranteed-valid JSON. Low severity; a debug log on the catch would aid future diagnosis.
|
Thanks for the detailed review. Commit Unused compression on Anthropic and Gemini routesFixed in Gemini
|
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found issues that need to be addressed before this is ready.
Findings
- [P2] Preserve image-bearing tool results before Responses history compression
src/services/api/openaiShim.ts:3902
Responses requests now pass the history throughcompressToolHistorybeforeconvertAnthropicMessagesToResponsesInput. For an old eligible tool result whose content is an image (or a mix of image and text),compressToolHistoryderives its stub length from text blocks only. An image-only result is therefore rewritten to e.g.[Read args={"path":"x"} → 0 chars omitted]; the Responses converter previously emitted[Image](https://...)for that same result. This drops the earlier screenshot/image reference from a long Responses conversation and tells the model that an empty result was omitted. Preserve non-text/image content while compressing the text portion (or exempt such results), and add a Responses regression case for an old image-bearing tool result.
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found issues that need to be addressed before this is ready.
Findings
- [P1] Count individual tool results when assigning compression tiers
src/services/api/compressToolHistory.ts:249
indexToolResultMessagesadds one entry for a whole user message whenever it contains anytool_result, and the early return at line 320 therefore treats a batch of arbitrarily many parallel results as one recent result. Parallel calls are emitted together in a single user message (and this module explicitly assumesparallel_tool_calls=true), so five batches of six 5,000-characterReadresults at a 100k window return unchanged: all 30 outputs are sent, with no stubs or truncation. On smaller-window routes even two such batches bypass compression entirely and can exceed the request/context budget. Assign tiers pertool_resultblock (while preserving the enclosing-message structure) and add a regression that exercises multiple results in the same user message.
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found issues that need to be addressed before this is ready.
Findings
- [P2] Preserve local endpoint fallback for native Ollama image requests
src/services/api/openaiShim.ts:932
Native Ollama serializes image inputs inmessages[].images, which this new detector now classifies ashasImages=true. Consequently, an HTTP 404 is turned intovision_not_supportedbefore the localendpoint_not_foundrecovery at lines 5024–5029 can run. For example, a local endpoint/proxy configured without its required/v1route receives one failed image request and reports that the model lacks vision, whereas the same text request advances to the next local URL and succeeds. Preserve the local endpoint retry for a route-level 404 (or distinguish an explicit vision rejection) before emitting the vision-specific diagnosis.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/api/openaiShim.ts (1)
4088-4093: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRecompress structured history for the Responses fallback.
A Chat request is compressed using
'\n\n', but the GitHub fallback converts those same blocks to Responses using'\n'. For multipart results this over-truncates content and makes the omitted-character marker inaccurate.Cache a separate fallback input compressed from
rawMessageswithtextBlockSeparator: '\n', and cover the fallback with multipart structured text.Also applies to: 4943-4948
🤖 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/services/api/openaiShim.ts` around lines 4088 - 4093, The getResponsesInput fallback currently reuses Chat-compressed messages with the wrong text-block separator. Cache a separate Responses fallback input by recompressing rawMessages with textBlockSeparator set to '\n', use it for the responses_compat conversion, and add coverage for multipart structured text to verify truncation and omitted-character markers.
🤖 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/services/api/codexShim.test.ts`:
- Around line 756-779: Add focused coverage for the changed performCodexRequest
compression path, rather than only testing
convertAnthropicMessagesToResponsesInput. Construct an oversized structured
tool-result input, invoke performCodexRequest with the relevant Codex
provider/model configuration, and assert the serialized request applies the
single-newline compression budget.
In `@src/services/api/openaiShim.ts`:
- Around line 5080-5108: The tool_stream self-healing logic in the request retry
flow is unrelated to the compression/image-classification change and alters
retry budgets and credential leasing. Remove this block from the current change,
or explicitly document affected providers and limitations in the pull request
description and add focused validation for the recovery behavior.
---
Outside diff comments:
In `@src/services/api/openaiShim.ts`:
- Around line 4088-4093: The getResponsesInput fallback currently reuses
Chat-compressed messages with the wrong text-block separator. Cache a separate
Responses fallback input by recompressing rawMessages with textBlockSeparator
set to '\n', use it for the responses_compat conversion, and add coverage for
multipart structured text to verify truncation and omitted-character markers.
🪄 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
Run ID: 7fbde34c-3ea8-4c85-a68d-e7798481d649
📒 Files selected for processing (6)
src/services/api/codexShim.test.tssrc/services/api/codexShim.tssrc/services/api/compressToolHistory.test.tssrc/services/api/compressToolHistory.tssrc/services/api/openaiShim.compression.test.tssrc/services/api/openaiShim.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: smoke-and-tests (22)
- GitHub Check: smoke-and-tests (24.11.x)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Runbun run typecheckandbun run typecheck:type-testsfor TypeScript changes.
Run provider tests and provider recommendation tests when changing provider behavior:bun run test:providerandbun run test:provider-recommendation.
Files:
src/services/api/codexShim.test.tssrc/services/api/compressToolHistory.test.tssrc/services/api/codexShim.tssrc/services/api/compressToolHistory.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.compression.test.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Keep changes focused on one problem or feature and avoid mixing unrelated cleanup into the same change.
Preserve existing repository patterns unless intentionally refactoring them.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary changes, and adherence to project architecture before submitting them.
Follow the existing code style in touched files.
Prefer small, readable changes over broad rewrites.
Do not reformat unrelated files.
Keep comments useful and concise.
Provider changes must explicitly identify affected providers, limitations, and follow-up work in the pull request description.
Do not assign or use provider tags; provider tags are controlled by maintainers.
Run the relevant validation checks locally before submitting changes; pull requests must pass CI checks.
Runbun run security:pr-scanbefore submitting a pull request.
Dependency changes require a concrete project benefit, such as fixing a bug, addressing a security issue, or supporting an approved feature.
Do not change the project's language, core runtime, or dependency stack without prior maintainer agreement.
Files:
src/services/api/codexShim.test.tssrc/services/api/compressToolHistory.test.tssrc/services/api/codexShim.tssrc/services/api/compressToolHistory.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.compression.test.ts
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/services/api/codexShim.test.tssrc/services/api/compressToolHistory.test.tssrc/services/api/codexShim.tssrc/services/api/compressToolHistory.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.compression.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when a code change affects behavior.
Files:
src/services/api/codexShim.test.tssrc/services/api/compressToolHistory.test.tssrc/services/api/codexShim.tssrc/services/api/compressToolHistory.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.compression.test.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/codexShim.test.tssrc/services/api/compressToolHistory.test.tssrc/services/api/codexShim.tssrc/services/api/compressToolHistory.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.compression.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/api/codexShim.test.tssrc/services/api/compressToolHistory.test.tssrc/services/api/openaiShim.compression.test.ts
🔇 Additional comments (4)
src/services/api/compressToolHistory.ts (1)
90-130: LGTM!Also applies to: 169-229, 237-266, 319-320, 339-414
src/services/api/compressToolHistory.test.ts (1)
274-274: LGTM!Also applies to: 504-536
src/services/api/codexShim.ts (1)
604-615: LGTM!src/services/api/openaiShim.compression.test.ts (1)
533-602: LGTM!Also applies to: 604-647, 706-707, 1069-1110
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/api/openaiShim.ts (1)
4088-4093: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRecompress structured history for the Responses fallback.
A Chat request is compressed using
'\n\n', but the GitHub fallback converts those same blocks to Responses using'\n'. For multipart results this over-truncates content and makes the omitted-character marker inaccurate.Cache a separate fallback input compressed from
rawMessageswithtextBlockSeparator: '\n', and cover the fallback with multipart structured text.Also applies to: 4943-4948
🤖 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/services/api/openaiShim.ts` around lines 4088 - 4093, The getResponsesInput fallback currently reuses Chat-compressed messages with the wrong text-block separator. Cache a separate Responses fallback input by recompressing rawMessages with textBlockSeparator set to '\n', use it for the responses_compat conversion, and add coverage for multipart structured text to verify truncation and omitted-character markers.
🤖 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/services/api/codexShim.test.ts`:
- Around line 756-779: Add focused coverage for the changed performCodexRequest
compression path, rather than only testing
convertAnthropicMessagesToResponsesInput. Construct an oversized structured
tool-result input, invoke performCodexRequest with the relevant Codex
provider/model configuration, and assert the serialized request applies the
single-newline compression budget.
In `@src/services/api/openaiShim.ts`:
- Around line 5080-5108: The tool_stream self-healing logic in the request retry
flow is unrelated to the compression/image-classification change and alters
retry budgets and credential leasing. Remove this block from the current change,
or explicitly document affected providers and limitations in the pull request
description and add focused validation for the recovery behavior.
---
Outside diff comments:
In `@src/services/api/openaiShim.ts`:
- Around line 4088-4093: The getResponsesInput fallback currently reuses
Chat-compressed messages with the wrong text-block separator. Cache a separate
Responses fallback input by recompressing rawMessages with textBlockSeparator
set to '\n', use it for the responses_compat conversion, and add coverage for
multipart structured text to verify truncation and omitted-character markers.
🪄 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
Run ID: 7fbde34c-3ea8-4c85-a68d-e7798481d649
📒 Files selected for processing (6)
src/services/api/codexShim.test.tssrc/services/api/codexShim.tssrc/services/api/compressToolHistory.test.tssrc/services/api/compressToolHistory.tssrc/services/api/openaiShim.compression.test.tssrc/services/api/openaiShim.ts
📜 Review details
🔇 Additional comments (4)
src/services/api/compressToolHistory.ts (1)
90-130: LGTM!Also applies to: 169-229, 237-266, 319-320, 339-414
src/services/api/compressToolHistory.test.ts (1)
274-274: LGTM!Also applies to: 504-536
src/services/api/codexShim.ts (1)
604-615: LGTM!src/services/api/openaiShim.compression.test.ts (1)
533-602: LGTM!Also applies to: 604-647, 706-707, 1069-1110
🛑 Comments failed to post (2)
src/services/api/codexShim.test.ts (1)
756-779: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Exercise the changed Codex compression path.
This only verifies the converter’s newline behavior and still passes if Line 613 is removed. Add a
performCodexRequesttest with oversized structured tool-result text and assert the serialized request uses the single-newline compression budget.As per coding guidelines, “test the exact provider/model path changed when possible.” As per path instructions, “Block when risky runtime changes lack focused regression coverage.”
🤖 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/services/api/codexShim.test.ts` around lines 756 - 779, Add focused coverage for the changed performCodexRequest compression path, rather than only testing convertAnthropicMessagesToResponsesInput. Construct an oversized structured tool-result input, invoke performCodexRequest with the relevant Codex provider/model configuration, and assert the serialized request applies the single-newline compression budget.Sources: Coding guidelines, Path instructions
src/services/api/openaiShim.ts (1)
5080-5108: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Split or explicitly document the unrelated
tool_streamrecovery.This changes provider retry budgets and credential-lease selection but is outside the compression/image-classification objectives. Move it to a focused PR, or update the PR description with affected providers and add its focused validation.
As per coding guidelines, “Keep changes focused on one problem or feature” and “Provider changes must explicitly identify affected providers, limitations, and follow-up work in the pull request description.”
🤖 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/services/api/openaiShim.ts` around lines 5080 - 5108, The tool_stream self-healing logic in the request retry flow is unrelated to the compression/image-classification change and alters retry budgets and credential leasing. Remove this block from the current change, or explicitly document affected providers and limitations in the pull request description and add focused validation for the recovery behavior.Sources: Coding guidelines, Path instructions
fcc5d79 to
3c57b4c
Compare
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/services/api/codexShim.test.ts`:
- Around line 756-779: The current test only validates converter behavior and
does not cover the changed Codex request path. Add a focused performCodexRequest
test using structured tool-result output larger than the mid-tier limit, capture
the serialized request body, and assert the exact provider/model-specific
truncation boundary, ensuring the test fails if the textBlockSeparator
configuration is removed.
In `@src/services/api/compressToolHistory.ts`:
- Around line 405-410: Update the TOOL_RESULT_CLEARED_MESSAGE handling around
shouldCompressBlock in the tool-result compression flow so cleared results still
undergo embedded-image sanitization and claim ownership of following permission
images. Preserve the existing truncateBlock/buildStub behavior for other
compressible results, and add coverage for cleared content with an inline image
plus a following permission image.
🪄 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
Run ID: 4cf7fb0e-7253-468f-9a0c-5e79918fdc39
📒 Files selected for processing (6)
src/services/api/codexShim.test.tssrc/services/api/codexShim.tssrc/services/api/compressToolHistory.test.tssrc/services/api/compressToolHistory.tssrc/services/api/openaiShim.compression.test.tssrc/services/api/openaiShim.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: smoke-and-tests (22)
- GitHub Check: smoke-and-tests (24.11.x)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Runbun run typecheckandbun run typecheck:type-testsfor TypeScript changes.
Run provider tests and provider recommendation tests when changing provider behavior:bun run test:providerandbun run test:provider-recommendation.
Files:
src/services/api/codexShim.test.tssrc/services/api/codexShim.tssrc/services/api/compressToolHistory.test.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.compression.test.tssrc/services/api/compressToolHistory.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Keep changes focused on one problem or feature and avoid mixing unrelated cleanup into the same change.
Preserve existing repository patterns unless intentionally refactoring them.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary changes, and adherence to project architecture before submitting them.
Follow the existing code style in touched files.
Prefer small, readable changes over broad rewrites.
Do not reformat unrelated files.
Keep comments useful and concise.
Provider changes must explicitly identify affected providers, limitations, and follow-up work in the pull request description.
Do not assign or use provider tags; provider tags are controlled by maintainers.
Run the relevant validation checks locally before submitting changes; pull requests must pass CI checks.
Runbun run security:pr-scanbefore submitting a pull request.
Dependency changes require a concrete project benefit, such as fixing a bug, addressing a security issue, or supporting an approved feature.
Do not change the project's language, core runtime, or dependency stack without prior maintainer agreement.
Files:
src/services/api/codexShim.test.tssrc/services/api/codexShim.tssrc/services/api/compressToolHistory.test.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.compression.test.tssrc/services/api/compressToolHistory.ts
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/services/api/codexShim.test.tssrc/services/api/codexShim.tssrc/services/api/compressToolHistory.test.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.compression.test.tssrc/services/api/compressToolHistory.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when a code change affects behavior.
Files:
src/services/api/codexShim.test.tssrc/services/api/codexShim.tssrc/services/api/compressToolHistory.test.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.compression.test.tssrc/services/api/compressToolHistory.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/codexShim.test.tssrc/services/api/codexShim.tssrc/services/api/compressToolHistory.test.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.compression.test.tssrc/services/api/compressToolHistory.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/api/codexShim.test.tssrc/services/api/compressToolHistory.test.tssrc/services/api/openaiShim.compression.test.ts
🔇 Additional comments (5)
src/services/api/compressToolHistory.ts (1)
38-320: LGTM!Also applies to: 342-404
src/services/api/compressToolHistory.test.ts (1)
246-282: LGTM!Also applies to: 432-442, 504-537
src/services/api/codexShim.ts (1)
611-613: LGTM!src/services/api/openaiShim.compression.test.ts (1)
5-5: LGTM!Also applies to: 17-22, 117-147, 165-194, 239-268, 284-366, 532-602, 604-647, 649-719, 721-820, 822-890, 892-947, 949-1001, 1002-1039, 1040-1126, 1127-1215, 1217-1253, 1255-1285
src/services/api/openaiShim.ts (1)
903-969: LGTM!Also applies to: 1492-1510, 3883-3950, 4085-4109, 4600-4624, 4637-4669, 4713-4713, 4958-4979, 5048-5056, 5236-5238
582ebd8 to
5c729c5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/services/api/compressToolHistory.ts`:
- Around line 403-406: The cleared-result branch in the block-processing flow
incorrectly sets omitFollowingInlineImages for every cleared tool result. Update
the logic around isAlreadyCleared and sanitizeClearedBlock to apply the same
toolUseResult ownership check used by the normal compression path, so
independent following user images remain intact. Add focused regression coverage
for a cleared result followed by an independent image.
🪄 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
Run ID: ceed5bff-2510-4510-9bea-cafaf861096a
📒 Files selected for processing (4)
src/services/api/codexShim.test.tssrc/services/api/compressToolHistory.test.tssrc/services/api/compressToolHistory.tssrc/services/api/openaiShim.compression.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: smoke-and-tests (24.11.x)
- GitHub Check: smoke-and-tests (22)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Runbun run typecheckandbun run typecheck:type-testsfor TypeScript changes.
Run provider tests and provider recommendation tests when changing provider behavior:bun run test:providerandbun run test:provider-recommendation.
Files:
src/services/api/codexShim.test.tssrc/services/api/compressToolHistory.test.tssrc/services/api/compressToolHistory.tssrc/services/api/openaiShim.compression.test.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Keep changes focused on one problem or feature and avoid mixing unrelated cleanup into the same change.
Preserve existing repository patterns unless intentionally refactoring them.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary changes, and adherence to project architecture before submitting them.
Follow the existing code style in touched files.
Prefer small, readable changes over broad rewrites.
Do not reformat unrelated files.
Keep comments useful and concise.
Provider changes must explicitly identify affected providers, limitations, and follow-up work in the pull request description.
Do not assign or use provider tags; provider tags are controlled by maintainers.
Run the relevant validation checks locally before submitting changes; pull requests must pass CI checks.
Runbun run security:pr-scanbefore submitting a pull request.
Dependency changes require a concrete project benefit, such as fixing a bug, addressing a security issue, or supporting an approved feature.
Do not change the project's language, core runtime, or dependency stack without prior maintainer agreement.
Files:
src/services/api/codexShim.test.tssrc/services/api/compressToolHistory.test.tssrc/services/api/compressToolHistory.tssrc/services/api/openaiShim.compression.test.ts
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/services/api/codexShim.test.tssrc/services/api/compressToolHistory.test.tssrc/services/api/compressToolHistory.tssrc/services/api/openaiShim.compression.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when a code change affects behavior.
Files:
src/services/api/codexShim.test.tssrc/services/api/compressToolHistory.test.tssrc/services/api/compressToolHistory.tssrc/services/api/openaiShim.compression.test.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/codexShim.test.tssrc/services/api/compressToolHistory.test.tssrc/services/api/compressToolHistory.tssrc/services/api/openaiShim.compression.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/api/codexShim.test.tssrc/services/api/compressToolHistory.test.tssrc/services/api/openaiShim.compression.test.ts
🔇 Additional comments (3)
src/services/api/compressToolHistory.test.ts (1)
80-80: LGTM!Also applies to: 504-565
src/services/api/codexShim.test.ts (1)
1-1: LGTM!Also applies to: 14-17, 31-31, 784-834
src/services/api/openaiShim.compression.test.ts (1)
687-688: LGTM!
2b6e7ea to
cee99e8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/utils/messages.ts`:
- Around line 2188-2193: Update the universal smoosh partitioning around
toSmoosh, toolResults, and images so all non-smooshed blocks remain in their
original relative order, including interleaved tool_result and image entries.
Use one filtered collection for the retained blocks and adjust downstream
handling to preserve that sequence.
🪄 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
Run ID: a5df5168-7859-4744-b222-14dd662e5d1b
📒 Files selected for processing (13)
src/services/api/codexShim.test.tssrc/services/api/codexShim.tssrc/services/api/compressToolHistory.test.tssrc/services/api/compressToolHistory.tssrc/services/api/openaiShim.compression.test.tssrc/services/api/openaiShim.tssrc/services/tools/toolExecution.tssrc/types/message.tssrc/utils/messages.tssrc/utils/messages/apiTransform.test.tssrc/utils/messages/factories.tssrc/utils/messages/normalize.test.tssrc/utils/messages/normalize.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: smoke-and-tests (22)
- GitHub Check: typecheck
- GitHub Check: smoke-and-tests (24.11.x)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Provider changes must follow the documented integration patterns and avoid inconsistent behavior across provider paths.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Review AI-generated code for correctness, style consistency, unnecessary changes, and adherence to project architecture before submission.
Run multiple rounds of self-review on AI-generated code; compilation alone is insufficient to establish correctness.
Files:
src/services/api/codexShim.tssrc/types/message.tssrc/utils/messages/factories.tssrc/utils/messages/apiTransform.test.tssrc/services/api/codexShim.test.tssrc/utils/messages/normalize.tssrc/utils/messages/normalize.test.tssrc/services/tools/toolExecution.tssrc/utils/messages.tssrc/services/api/compressToolHistory.tssrc/services/api/compressToolHistory.test.tssrc/services/api/openaiShim.compression.test.tssrc/services/api/openaiShim.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Keep pull requests focused on one issue or one clearly scoped improvement; avoid unrelated cleanup, fixes, features, or refactors in the same change.
Preserve existing repository patterns unless intentionally refactoring them, and stay within the project's existing language, runtime, dependency, and architectural direction.
Add or update tests when a change affects behavior.
Update documentation when setup, commands, or user-facing behavior changes.
Follow the existing code style in touched files.
Prefer small, readable changes over broad rewrites.
Do not reformat unrelated files merely because they are nearby.
Keep comments useful and concise.
Run the narrowest meaningful validation command for the touched area before opening a pull request, and ensure relevant CI checks pass before merge.
Provider-change pull requests must identify affected providers, state the tested provider/model path, and document limitations or follow-up work.
Do not assign or use provider tags; provider tags are controlled and applied by maintainers.
Security reports must follow the instructions inSECURITY.md.
PR descriptions must explain what changed and why, user or developer impact, exact checks run, and include relevant issue links; UI, terminal presentation, or VS Code extension changes require screenshots.
PR authors must address CodeRabbit findings before maintainer review proceeds.
Files:
src/services/api/codexShim.tssrc/types/message.tssrc/utils/messages/factories.tssrc/utils/messages/apiTransform.test.tssrc/services/api/codexShim.test.tssrc/utils/messages/normalize.tssrc/utils/messages/normalize.test.tssrc/services/tools/toolExecution.tssrc/utils/messages.tssrc/services/api/compressToolHistory.tssrc/services/api/compressToolHistory.test.tssrc/services/api/openaiShim.compression.test.tssrc/services/api/openaiShim.ts
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/services/api/codexShim.tssrc/types/message.tssrc/utils/messages/factories.tssrc/utils/messages/apiTransform.test.tssrc/services/api/codexShim.test.tssrc/utils/messages/normalize.tssrc/utils/messages/normalize.test.tssrc/services/tools/toolExecution.tssrc/utils/messages.tssrc/services/api/compressToolHistory.tssrc/services/api/compressToolHistory.test.tssrc/services/api/openaiShim.compression.test.tssrc/services/api/openaiShim.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/codexShim.tssrc/services/api/codexShim.test.tssrc/services/api/compressToolHistory.tssrc/services/api/compressToolHistory.test.tssrc/services/api/openaiShim.compression.test.tssrc/services/api/openaiShim.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/utils/messages/apiTransform.test.tssrc/services/api/codexShim.test.tssrc/utils/messages/normalize.test.tssrc/services/api/compressToolHistory.test.tssrc/services/api/openaiShim.compression.test.ts
🔇 Additional comments (18)
src/utils/messages/apiTransform.test.ts (1)
364-376: LGTM!src/types/message.ts (1)
119-120: LGTM!src/services/tools/toolExecution.ts (1)
1302-1302: LGTM!Also applies to: 1641-1641
src/utils/messages/factories.ts (1)
205-205: LGTM!Also applies to: 222-222, 264-264
src/utils/messages/normalize.test.ts (1)
42-61: LGTM!src/utils/messages/normalize.ts (1)
127-135: LGTM!src/services/api/compressToolHistory.ts (2)
385-426: LGTM!
106-235: LGTM!src/services/api/compressToolHistory.test.ts (1)
251-668: LGTM!src/services/api/codexShim.ts (1)
597-615: LGTM!Also applies to: 685-685
src/services/api/codexShim.test.ts (1)
784-835: LGTM!src/services/api/openaiShim.ts (5)
1272-1337: LGTM!
1861-1878: LGTM!Also applies to: 4304-4349, 4380-4380
4516-4536: LGTM!
5040-5113: LGTM!Also applies to: 5507-5515
5389-5451: LGTM!src/services/api/openaiShim.compression.test.ts (2)
630-732: LGTM!Also applies to: 930-964, 966-981, 1021-1106, 1236-1266
1198-1202: 📐 Maintainability & Code Quality
CLAUDE_CODE_USE_OPENAIis already reset by the shared test cleanup. It’s part oforiginalEnvand gets restored inafterEach, so this assignment won’t leak into later tests.> Likely an incorrect or invalid review comment.
…ures Post-rebase reconciliation with Twigpine#1958's compressToolHistory rework (the guard's extractText call no longer even typechecked against the new signature) plus the CodeRabbit findings: - Tier-aware idempotency replaces the blanket already-compressed skip, which permanently blocked mid→old upgrades: stubs stay terminal, and a truncated block left alone while mid-tier still upgrades to a stub when later exchanges age it into the old tier. The upgraded stub's omitted-chars count is recovered from the truncation marker (visible length + marker's omitted count = exact pre-truncation length), so it reports the tool's real output size — asserted equal to what a fresh single-pass stub would have produced. Regression tests cover the aging upgrade, recovered length, and same-input no-op. - computeSignature hashes the FULL serialized input (sha-256, fixed-size stored signature) instead of comparing a 2KB prefix, which treated distinct calls sharing a long prefix (e.g. Write calls differing only in trailing content) as identical — a false-positive block on legitimate work. Regression test included. - DEFAULT_COMPACT_TAIL_TURNS shared constant replaces the `3` duplicated across autoCompact, pruneByRelevance's default, and the /config UI. - Doom-loop block path: added a tengu_doom_loop_blocked analytics event (false-positive rates become observable for threshold tuning) and the nudge now tells the model a deliberate repeat is fine once something observable has changed. The blocked yield's message shape mirrors the sibling pre-execution error paths, preserving tool_use/tool_result pairing. Co-Authored-By: OpenClaude <openclaude@gitlawb.com>
…om loop detection, configurable compaction (#1869) * feat(perf): tier1 token optimization — universal tool compression, doom loop detection, configurable compaction - Extend compressToolHistory to Anthropic-native transports (firstParty/ bedrock/vertex/native-GitHub), gated to runs where prompt caching is inactive: rewriting old messages as they cross tier boundaries diverges the request prefix every call, so cached native sessions keep relying on the cache-aware microCompact instead. Shim-routed traffic (OpenAI-compatible env providers, per-agent providerOverride, Codex) still compresses at its own layer, where the local fast-path opt-out applies. compressToolHistory is now idempotent (skips its own stub/truncation markers) so layered call sites can never re-mangle output. - Add doom loop detection: blocks after 3 consecutive identical tool calls (same name + input signature). State is keyed per agent (main thread and each subagent separately) so concurrent subagents neither trip nor reset each other's counters. Resets at the start of each agent's query turn. - Add configurable compactTailTurns in GlobalConfig, wired into autoCompact's relevance pruning (default: 3, clamped to positive) and exposed in the /config UI next to the other compaction settings. Co-Authored-By: OpenClaude <openclaude@gitlawb.com> * fix(perf): address review — tier-aware idempotency, full-input signatures Post-rebase reconciliation with #1958's compressToolHistory rework (the guard's extractText call no longer even typechecked against the new signature) plus the CodeRabbit findings: - Tier-aware idempotency replaces the blanket already-compressed skip, which permanently blocked mid→old upgrades: stubs stay terminal, and a truncated block left alone while mid-tier still upgrades to a stub when later exchanges age it into the old tier. The upgraded stub's omitted-chars count is recovered from the truncation marker (visible length + marker's omitted count = exact pre-truncation length), so it reports the tool's real output size — asserted equal to what a fresh single-pass stub would have produced. Regression tests cover the aging upgrade, recovered length, and same-input no-op. - computeSignature hashes the FULL serialized input (sha-256, fixed-size stored signature) instead of comparing a 2KB prefix, which treated distinct calls sharing a long prefix (e.g. Write calls differing only in trailing content) as identical — a false-positive block on legitimate work. Regression test included. - DEFAULT_COMPACT_TAIL_TURNS shared constant replaces the `3` duplicated across autoCompact, pruneByRelevance's default, and the /config UI. - Doom-loop block path: added a tengu_doom_loop_blocked analytics event (false-positive rates become observable for threshold tuning) and the nudge now tells the model a deliberate repeat is fine once something observable has changed. The blocked yield's message shape mirrors the sibling pre-execution error paths, preserving tool_use/tool_result pairing. Co-Authored-By: OpenClaude <openclaude@gitlawb.com> * fix(perf): normalize compactTailTurns everywhere + native-routing coverage Second CodeRabbit round: - normalizeCompactTailTurns (relevancePruning.ts, next to the shared default) is now the single rule for the hand-editable config: finite values >= 1 floor to integers, everything else falls back to the default. The /config UI displays AND persists through it, so the picker shows exactly what autoCompact preserves (a hand-edited 2.5 no longer displays as 2.5 while running as 2; 0/negatives no longer display as selected while running as 3). Also fixes a real edge in the previous inline clamp: 0.5 passed the `> 0` check and floored to a ZERO-message tail, pruning everything. Unit tests cover the boundary matrix. - shouldCompressNativeToolHistory extracted from queryModel and exported: the request-mutating routing decision is now parameterized-tested across all four native transports (first-party, Bedrock, Vertex, GitHub-native-Anthropic) x caching on/off, the providerOverride exclusion, and non-native providers — queryModel itself needs a live client, so the predicate is the honest testable seam. Co-Authored-By: OpenClaude <openclaude@gitlawb.com> * fix(perf): guard custom first-party endpoints + strict config coercion Third CodeRabbit round: - shouldCompressNativeToolHistory now requires an Anthropic first-party base URL before accepting the firstParty provider as native, mirroring the exact guard getPromptCachingEnabled uses. Without it, a custom ANTHROPIC_BASE_URL (proxy / compatible endpoint) reported firstParty with caching disabled and had every request's messages compressed — an assumption we cannot make about arbitrary endpoints. Test added for the custom-base-URL exclusion. - normalizeCompactTailTurns only coerces numbers (persisted config) and strings (the /config picker channel); other hand-edited shapes no longer smuggle a tiny tail through Number() coercion (true → 1, [2] → 2) and fall back to the default instead. Boundary tests added. Co-Authored-By: OpenClaude <openclaude@gitlawb.com> --------- Co-authored-by: OpenClaude <openclaude@gitlawb.com>
3 participants
Summary
Impact
Testing
bun run typecheckbun run typecheck:type-testsbun run buildbun run smokegit diff --checkbun run security:pr-scanbun run checkremains nonzero on existing repository-wide baseline failures; comparison found no branch-only failures.Provider paths exercised include generic Responses,
responses_compat, GitHub Chat-to-Responses fallback, local fast-path requests, native Ollama Chat, Anthropic Messages overrides, and Gemini overrides.Notes
main; adjacent PRs feat(perf): tier1 token optimization — universal tool compression, doom loop detection, configurable compaction #1869, fix(api): enforce API_TIMEOUT_MS for OpenAI-compatible headers #1940, and feat(gemini-vertex): provider routing, selection and cross-provider tool-id safety (2/3) #1614 remained open.Summary by CodeRabbit
function_call/function_call_outputalignment across tiering, truncation/stubbing, and parallel tool batches.