Skip to content

fix(agent): clamp provider context images#3232

Open
roboomp wants to merge 2 commits into
mainfrom
farm/f7e593a1/clamp-umans-context-images
Open

fix(agent): clamp provider context images#3232
roboomp wants to merge 2 commits into
mainfrom
farm/f7e593a1/clamp-umans-context-images

Conversation

@roboomp

@roboomp roboomp commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

Repro

A focused regression test that imports clampProviderContextImages and builds a 31-image umans context failed before the fix because @oh-my-pi/pi-coding-agent/session/provider-image-budget did not exist: bun test packages/coding-agent/test/session/provider-image-budget.test.ts exited 1 with Cannot find module '@oh-my-pi/pi-coding-agent/session/provider-image-budget'.

Cause

packages/snapcompact/src/snapcompact.ts shipped providerImageBudget("umans") === 10, but packages/coding-agent/src/sdk.ts only installed transformProviderContext when secret obfuscation or snapcompact inline imaging was enabled. No coding-agent request path clamped existing image blocks, so live contexts with more than 10 images could still be sent unchanged to umans.

Fix

  • Added packages/coding-agent/src/session/provider-image-budget.ts with clampProviderContextImages(), which preserves text and the newest images while dropping oldest image blocks above the active provider cap.
  • Updated packages/coding-agent/src/sdk.ts to always run the provider-context transform and apply the clamp after obfuscation and snapcompact inline transforms.
  • Added packages/coding-agent/test/session/provider-image-budget.test.ts coverage for the 31-image umans regression and the below-budget no-op case.
  • Added the coding-agent changelog entry for Regression: #3227 not actually fixed — clamp logic from PR #3228 never merged; 31-image 400 still fires on umans #3230.

Verification

bun test packages/coding-agent/test/session/provider-image-budget.test.ts passed with 2 tests; bun run --cwd packages/coding-agent check passed. Fixes #3230

Dropped oldest outgoing image blocks above the active provider budget so umans requests honor the shipped 10-image cap even when snapcompact is disabled. Added regression coverage for preserving text and newest images.\n\nFixes #3230

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eb88ecd7bb

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +54 to +55
const content = clampContent(message.content, state);
return content ? { ...message, content } : message;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve a placeholder for fully dropped tool-result images

When an older successful tool result contains only images, dropping all image parts here returns content: []. I checked the Anthropic/umans conversion path: buildToolResultBlock() only fills empty content for isError results, so once the provider budget is exceeded an image-only successful tool result is serialized as an empty tool_result rather than a textual omission marker. Add a placeholder text block when the clamp removes the last tool-result block so the request stays meaningful and protocol-safe.

Useful? React with 👍 / 👎.

Added a textual omission marker when provider image clamping removes every block from a successful tool result, keeping the serialized tool_result meaningful and protocol-safe.\n\nFixes #3230
@roboomp

roboomp commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator Author

Added a [image omitted: provider image limit] text block when clamping removes every block from a successful tool result.
Added regression coverage for image-only tool results so they stay non-empty after provider image clamping.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

vouched Passed the vouch gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression: #3227 not actually fixed — clamp logic from PR #3228 never merged; 31-image 400 still fires on umans

1 participant