Repository navigation
Conversation
There was a problem hiding this comment.
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) => |
There was a problem hiding this comment.
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.
Issue for this PR
Closes #47721
Type of change
What does this PR do?
Rebase of #59303 by @rekram1-node onto current
dev(it had conflicts inmodel-request.tsand its test). The two original commits are kept as-is apart from the conflict resolution:GENERATION_KEYSis now imported instead of defined locally, and the test assertsUserMessageafter the Message split.Sessions that
readlots of screenshots break once history passes the provider's image cap: every later request replays all images and gets a 400 such asrequest 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.boundImagesnow 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.readit again.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 typecheckin core and ai,bun run checkfrom the root: cleanScreenshots / recordings
N/A
Checklist
Added by the opencode bot (AI-generated).
Related: #48095