Skip to content

fix(core): cap images per request and compact on image-count errors - #63581

Open
neriousy wants to merge 2 commits into
devfrom
image-count-cap
Open

neriousy wants to merge 2 commits into
devfrom
image-count-cap

Conversation

@neriousy

@neriousy neriousy commented Nov 12, 2026 •

Copy link
Copy Markdown
Member

Issue for this PR

Closes #47721

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

Rebase of #59303 by @rekram1-node onto current dev (it had conflicts in model-request.ts and its test). The two original commits are kept as-is apart from the conflict resolution: GENERATION_KEYS is now imported instead of defined locally, and the test asserts UserMessage after the Message split.

Sessions that read lots of screenshots break once history passes the provider's image cap: every later request replays all images and gets a 400 such as request contains 52 images, exceeding the maximum of 50 allowed per request, even for text-only prompts. Manual compaction fails the same way, because it first sends the unchanged request with the images.

  • boundImages now caps count as well as bytes: past 30 images it drops the oldest down to 20, moving in steps of 10 so the replaced prefix (and prompt cache) stays stable between cuts.
  • The placeholder for a dropped image names the file, so the model can read it again.
  • Image-count rejections (Console/Zen, OpenCode Go, Azure OpenAI, Anthropic/Bedrock) are classified as context overflow, so the runner compacts and compaction falls back to text.

Stuck sessions recover on their next prompt since the cap applies when the request is built.

Out of scope: per-model image limits in the catalog, and counting PDFs toward Anthropic's combined images-and-documents limit.

How did you verify your code works?

  • packages/core: bun test test/session-model-request.test.ts (12 pass), bun test test/session-compaction*.test.ts test/session-runner*.test.ts (356 pass)
  • packages/ai: bun test test/provider-error.test.ts (47 pass)
  • bun typecheck in core and ai, bun run check from the root: clean

Screenshots / recordings

N/A

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

Added by the opencode bot (AI-generated).

Related: #48095

@opencodejr opencodejr Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good. I ran the #47721 reproduction (simulated model, 56 tiny PNGs read one at a time, mock rejecting requests with more than 50 images) on both checkouts. On the base, requests grew to 51 images and were rejected. With this PR, no request carried more than 30 images: the count dropped to 20 each time it passed 30, and nothing was rejected. The changed tests pass, and tsc --noEmit is clean in core and ai. I didn't run the compaction-after-image-error path end to end; the classifier unit tests cover it.

Public API unchanged. The only visible changes are internal: when old images are dropped from a request, and the placeholder text that replaces them.

One non-blocking note inline: the placeholder names only the file's basename.

Related: #48095

// A per-model limit would replace this constant.
const IMAGE_COUNT_TRIGGER = 30
const IMAGE_COUNT_TARGET = 20
const imageRemoved = (name: string | undefined) =>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For images from the read tool, name is only the basename (path.basename(input) in tool/read-filesystem.ts). So the placeholder says read "shot-0.png" again when the file is at shots/shot-0.png. That path can fail, or open a different file with the same name. The model can usually recover the path from the original tool call, so this isn't blocking. Still, the result's uri (a file:// URL for local reads) or the call's input path would make the hint reliable. The new test uses /tmp/shot-${index}.png as the name, which hides this, because real read results never contain a path.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

core: request accumulates 52 images, exceeds provider max of 50

2 participants