Repository navigation
Conversation
github-actions
Bot
added
needs:compliance
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
![opencodejr[bot]](https://avatars.githubusercontent.com/in/5497873?s=60&v=4){kind=small}
There was a problem hiding this comment.
Looks correct. I ran the #61391 reproduction (two remote images followed by two 13 MiB inline ones) on the base and with this change: the base replaced three images, this PR replaces only c.png. A case with remote images placed between three inline ones also drops only the two oldest inline images. session-model-request.test.ts passes (11/11), including the prompt-cache prefix-stability test, and typecheck:tsc in packages/core is clean.
The cut loop and the replacement pass count the same set of images (those with a non-zero payload) in the same order, so the replaced prefix still follows document order for inline images. The inner while (sizes[next] === 0) can't run past the end, because kept > IMAGE_BYTES_TARGET means a non-zero image is still left.
Public API: unchanged.
No blocking concerns.
There was a problem hiding this comment.
The new commits only merge dev. The fix in model-request.ts and its test are unchanged. I re-ran the #61391 reproduction and session-model-request.test.ts on the new head: only the inline c.png is replaced, the tests pass and typecheck:tsc in packages/core is clean.
Public API: unchanged.
No new concerns.
Issue for this PR
Closes #61391
Type of change
What does this PR do?
boundImagespicks how many images to replace by walkingsizesin order, then applies thatcutby document position.mediaBytesreturns0forurlandrefmedia because they carry no local payload, so those images still take slots in the removal queue. The budget loop subtracts0fromkeptbut advancescut, andreplaceMediathen replaces the remote image too. With two remote images followed by two 13 MiB inline images (26 MiB total), three images were replaced instead of one.The loop now skips zero-byte entries when choosing what to drop, and the replacement pass only counts images with a non-zero payload. Remote and provider-referenced images stay in the request, so removals always free payload. The source-order prefix that keeps the prompt cache stable is unchanged for images that carry payload.
How did you verify your code works?
Added a regression test in
packages/core/test/session-model-request.test.tswith two remote images followed by two 13 MiB inline images. Ondevit fails withExpected: 1, Received: 3; with the fix it passes.cd packages/core && bun run script/test.ts test/session-model-request.test.ts-> 11 pass, 0 failbun run checkfrom the repo root -> 35 successful tasks, 0 errorsbun run typecheck:tscfrompackages/core-> cleanVisual
Not a user-facing visual change; no new UI.
Screenshots / recordings
N/A (request shaping only).
Checklist