feat(images): paste a screenshot, ask about it - #90
Conversation
…p dropping blocks
First slice of docs/IMAGES.md. No UI yet — this is the wire seam, and half of it is a
bug that predates images and is worth fixing on its own.
THE SILENT DROP. toOpenAIMessages' user-message loop handled tool_result and text and
fell through on everything else. A block type it had never seen vanished between the
composer and the wire with no error, no warning and no log line — and the model then
answered confidently about content it was never sent. A user would reasonably conclude
it hallucinates. That fall-through is now a throw naming the type.
Checked before making it throw: the only block types this codebase produces are text,
tool_use and tool_result, so nothing in production reaches the new error path.
IMAGES. Anthropic carries bytes in `source` (base64 + media_type, or url); OpenAI takes
one `url` that is either a real URL or a data: URI. toOpenAIImagePart maps between them
and refuses a `file` source outright — Files API references are Anthropic-only, and
there is nothing to translate them to, so sending SOMETHING would mean sending a request
whose subject is missing.
Two ordering decisions:
- Images lead the turn. The model reads them best before the text that asks about
them, and it keeps markLastOpenAICacheable's breakpoint (which lands on the LAST
block) on a text block rather than on an image.
- A text-only turn still emits a plain string, not a one-element block array.
Widening every turn would change the bytes of every cached prefix for no gain.
Wrote the tests against the wrong signature first — toOpenAIMessages is
(system, messages, opts), not (messages) — and got 'system' !== 'user' rather than a
passing test, which is the failure mode I would rather have.
Bypass-verified, each by reverting the fix:
- the silent drop restored (the bug this slice exists for)
- image blocks no longer recognised
- text ordered before the image
- malformed base64 no longer refused; a Files-API source sent as an empty url
- text-only turns widened to block arrays
32 tests in translate (was 24), all suites green.
…ed screenshot
I2 — supportsVisionForModel, beside supportsToolsForModel, reading the `vision` flag
that has been sitting in the catalog unread since the multi-provider work.
Deliberately STRICTER than the tools gate. That one defaults unknown models to
tools:true, because most modern chat models have tools and refusing the agent is the
bigger loss. Vision inverts the trade: attaching to a blind model costs the user a
composed message and returns a provider error — or worse, a confident answer about the
text alone — while a disabled attach button that names a model which CAN see costs one
click. The catalog marks every vision model we know and the family heuristic covers the
big three, so the strict default bites only on genuinely unrecognised ids.
I3 — imageCost.js. Pure geometry and cost; no canvas, no fs, no vscode. The webview
does the pixel work, this decides what the pixel work should aim for and tells the
context meter what the result costs.
THE NUMBERS ARE NOT ESTIMATES. The formula I had in mind (w*h/750) is stale and wrong.
Claude sees 28x28 patches, so cost is ceil(w/28)*ceil(h/28), and each tier caps both
the long edge and the token count. The doc-parity test reproduces every worked example
in the vision documentation on both tiers — 1092^2 -> 1521, 1000^2 -> 1296, 1920x1080
-> 2691 high-res and 1456x819/1560 standard, 3840x2160 -> 2576x1449/4784.
Three decisions worth naming:
- Binary search on the scale, not a step-down loop. Stepping lands a few pixels short
and misreports the size — which matters here because it is also what the UI shows.
- clientScale returns exactly 1 when nothing should happen, so the caller can skip
re-encoding and forward the ORIGINAL bytes. Re-encoding an untouched image only
stacks artifacts, worst on the screenshots of text that are most of what gets pasted.
- Unknown model -> standard tier, and unknown image size -> charged the tier cap.
Both fail toward OVER-counting. Under-counting is the direction that lets a
conversation full of images look empty to the compaction cut.
Bypass-verified, each by reverting the fix:
- the stale w*h/750 formula
- upscaling allowed (the `sips -Z` trap that grew a capture 40KB -> 89KB)
- step-down instead of binary search
- unknown model treated as high-res; unknown-size image costed at zero
- cap 0 collapsing an image to nothing
- aspect ratio skewed, in both fitToTier and clientTarget
9 tests in imageCost, all suites green.
The I3 comment (and its test) carried the same overstatement a reviewer caught on #89: that a bad token estimate makes findCompactionCut evict real conversation history. It does not. findCompactionCut cuts on message count and goal boundaries and never reads a token number; compactAgentMemory uses estimateMsgTokens only for its before/after report. Checked by opening both, which is what I should have done before writing it. Today the consequence is a UI meter that reads wildly high the moment an image is attached — telling someone to start a new chat when they are nowhere near full. It becomes a correctness bug the day anything automatic keys off that number. Both files now say that instead. Fixing the comment rather than only the doc: a false causal claim in the source outlives a false claim in a design note, because the next person to touch imageCost.js reads the comment and not the PR thread.
… the truth Local, session-attached. Nothing is uploaded. A screenshot of someone's proprietary code never leaves their machine — which is also the only shape that works for BYOK, where the editor talks to the provider directly and a detour through our infrastructure would add a failure mode and contradict the promise that we are not in the middle. WHY A SIBLING media/ DIRECTORY RATHER THAN INLINE BASE64. Claude Code inlines image bytes in its own JSONL and that works fine there — I measured it: 24 images, all base64, in a 34MB transcript. It does not work here, for a reason specific to this codebase. sessionStore.scanProject does readFileSync + JSON.parse on EVERY session file in a project whenever index.json is missing, malformed, or on an older schema — first run, and after any schema bump. Inlined bytes would make drawing a list of session titles parse every screenshot in every session. Refs keep that scan cheap, keep the transcript greppable, and dedupe the re-paste that follows a failed send. Content-addressed by sha256, so the same screenshot twice is one file. Written tmp+rename, because a crash mid-write must never leave a truncated file under a hash claiming to describe its full contents. Capped at 5MB of bytes — the Claude API allows 10MB of base64, Bedrock and Vertex 5MB, and base64 inflates by 4/3, so 5MB of bytes is what is safe everywhere. REFS ARE NOT PATHS. A ref is read straight out of a session file, which is data on disk a user can edit. isRef pins the shape to 64 hex characters plus a known extension, and read() returns null rather than touching anything else — `../../etc/passwd` is not a ref. THE METER. estimateMsgTokens now counts an image by its real visual cost and excludes its JSON entirely. chars/4 is sound for text and wrong for an image in EITHER shape: inline base64 books about a third of its byte count (a 1MB screenshot read as ~350,000 tokens), while a stored ref is 64 hex characters and reads as ~18 — for the same ~4,800. A test pins that the storage shape does not move the number, which is the point of the design. Bypass-verified, each by reverting the fix: - path traversal accepted as a ref - not content-addressed, so duplicate pastes duplicate files - oversize and unsupported media types accepted - a missing file silently becoming empty text instead of throwing - tmp file left behind by a non-atomic write - the meter counting images by JSON length again; and costing them nothing at all 11 tests in imageStore, 36 suites green.
Paste, drop, a thumbnail chip you can take back off, and the refusal that has to happen
before anything is lost.
THE CAP IS 2000px ON THE LONG EDGE, AND IT IS MEASURED, NOT CHOSEN. The plan argued 1568
from first principles. Claude Code's own transcripts are on this machine, so I read them
instead: 24 images, and every single one it re-encodes is exactly 2000 on the long edge.
That is the threshold the vision docs name for staying clear of the stricter per-image
dimension limit above 20 images per request — the largest size that is never unsafe. It
also sits above both model tiers' caps, so the server does the final downscale and we
never throw away fidelity it would have kept. Changed 1568 -> 2000.
The same transcripts confirmed the rule I had derived rather than observed: images UNDER
the cap are passed through untouched IN THEIR ORIGINAL FORMAT (their PNGs stay PNG, their
JPEGs stay JPEG), and only oversize ones are resized and re-encoded to WebP. Re-encoding
something that did not need resizing only stacks artifacts, and that is worst on
screenshots of code, which is most of what gets pasted.
Verified in a real browser against real images, not just in unit tests:
4K screenshot 3840x2160 png 764KB -> 2000x1125 webp 115KB resized
retina window 3024x1964 png 654KB -> 2000x1299 webp 137KB resized
small PNG 1160x480 png 121KB -> 1160x480 png 121KB PASS-THROUGH
under-cap JPEG 1600x900 jpg 208KB -> 1600x900 jpg 208KB PASS-THROUGH
wide panorama 2400x600 png 178KB -> 2000x500 webp 51KB resized
TIFF -> refused, with what to do instead
Aspect ratios come out exact (16:9 stays 16:9, 4:1 stays 4:1).
THE REFUSAL RUNS AT ATTACH TIME, NOT SEND TIME. The composer clears on send, so refusing
host-side would throw away what someone had written. canSeeImages travels with the model
config instead — from BOTH config paths, gateway and BYOK, which a test pins, because one
of them silently allowing is exactly the kind of half-gate that looks fine in review.
Other decisions worth naming:
- An image with no words is a valid message. "Look at this" is implied by attaching it.
- The chip carries its own thumbnail. An attachment you cannot see is one you cannot
check, and a screenshot is the case where the wrong one looks exactly like the right
one in a filename.
- Drop is bound to the whole panel, not the composer: someone dragging a screenshot
aims at the conversation, which is the far bigger target.
- A paste with no image falls through untouched, and the bail comes BEFORE
preventDefault — reversing those two silently breaks ordinary text pasting.
- withImages() copies. agentMessages persists across runs and is what recordTurn writes
to the session log; materializing in place would put megabytes of base64 in both.
9 tests in imageAttach, 37 suites green.
…refused them Two bugs found by asking "how would you test this", which is the question I should have asked myself before opening the PR. AGENT MODE DROPPED THEM. handleSend stored the bytes, then early-returned into agentFlow(text) — which never received the blocks. Agent is the DEFAULT mode, so the common path wrote a screenshot to disk and then silently sent a text-only request. That is precisely the failure this whole feature exists to prevent, reintroduced two slices after I1 removed it from the translator. agentFlow now takes the blocks and leads the goal message with them, and keeps a plain string when there are none so cached agent prefixes do not churn. NO WORKSPACE REFUSED THEM. storeImages went through sessionsManager(), which returns null with no folder open — so a pasted screenshot got "Images need a session to attach to". v1.1.0 deliberately made the agent answer with no folder open; refusing an image in that state re-introduces the limitation that release removed. Images need a place on DISK, not a session. imageRoot() prefers the project's session dir and falls back to a shared bucket, so the feature works wherever the agent does. Bypass-verified: agent mode dropping images again; the goal message ignoring them; the no-workspace fallback removed. 11 tests in imageAttach, 37 suites green.
There was a problem hiding this comment.
Pull request overview
Adds local screenshot paste/drop support for LevelCode AI, including image normalization, storage, vision gating, provider translation, and token accounting.
Changes:
- Adds paste/drop attachment UI with resizing and validation.
- Stores images content-addressably under session media.
- Adds image-aware provider integration, capability checks, accounting, and tests.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Final review findings |
|---|---|
extensions/levelcode-ai/test/translate.test.js |
No final comments. |
extensions/levelcode-ai/test/imageStore.test.js |
No final comments. |
extensions/levelcode-ai/test/imageCost.test.js |
No final comments. |
extensions/levelcode-ai/test/imageAttach.test.js |
No final comments. |
extensions/levelcode-ai/sessions.js |
Moderate (2 votes): Add media lifecycle/GC or accurately document retention and storage scope. |
extensions/levelcode-ai/providers/translate.js |
Moderate (2 votes): Unsupported assistant content blocks can still be silently dropped. |
extensions/levelcode-ai/providers/catalog.js |
Moderate (2 votes): Vision gating ignores provider-level capabilities for custom endpoints. |
extensions/levelcode-ai/media/chat.html |
Moderate (2 votes each): Image removal is overridden by the generic chip handler; reset does not clear images; asynchronous attachment normalization can race with sending. |
extensions/levelcode-ai/imageStore.js |
No final comments. |
extensions/levelcode-ai/imageCost.js |
No final comments. |
extensions/levelcode-ai/extension.js |
Critical (2 votes): OpenAI-compatible chat receives unsupported Anthropic image blocks. Moderate (4 votes): Agent mode omits image blocks from requests and persisted turns. Moderate (3 votes): Materialized image messages lose generated agent history. Moderate (2 votes each): Normal-chat media refs are not persisted; vision capability is not revalidated after model changes. |
extensions/levelcode-ai/agentMemory.js |
Moderate (3 votes): High-resolution image costs are undercounted when compacting memory without the active model ID. |
Suppressed comments (13)
extensions/levelcode-ai/agentMemory.js:60
- Once an agent goal carries
[image, text]blocks,findCompactionCutstill cannot recognize it as a goal boundary becauseisGoalBoundaryonly accepts string content (lines 14–15). Image-bearing turns would then prevent compaction from finding a safe cut, so long agent sessions stop compacting after this feature is wired through. Treat non-tool_resultuser block arrays as boundaries, consistently withsessionResume.isTurnStart.
if (!Array.isArray(m.content)) { chars += JSON.stringify(m).length; continue; }
chars += 24; // role + envelope, roughly what the object costs around its blocks
for (const b of m.content) {
if (b && b.type === 'image') { imageTokens += imageBlockTokens(b, modelId); }
extensions/levelcode-ai/extension.js:1831
- The vision flag only gates attachment time; it can change after an image is attached or after an earlier image turn is in
conversation.withImages(conversation)materializes every stored image on every request, so switching to a non-vision model still sends those images (and can make the next text-only turn fail). Revalidate the current provider/model at this wire boundary, or otherwise prevent image history from being sent to a blind model.
post({ type: 'assistantStart' });
extensions/levelcode-ai/extension.js:1749
- A host-side store failure is caught and the image is skipped, but the webview has already rendered every submitted image as a sent user message and cleared the tray. For an image-only oversize/invalid payload this returns from
handleSendwith no request or error, while the transcript claims it was sent; with text, the image is shown even though only text was transmitted. Do not silently continue here—acknowledge accepted images back to the webview or reject the send before the optimistic render.
}
/**
* Store what the webview normalized, and return the blocks that will ride the conversation.
extensions/levelcode-ai/extension.js:1647
withImagesdeliberately returns a new array when it materializes a stored ref, butrunAgentmutatesctx.messagesby appending assistant, tool-result, and continuation messages. That leaves those messages on the temporary copy;agentMessagesis then persisted and reused without the completed turn. Any agent run containing an image consequently loses its generated history. Materialize at the provider boundary or merge the mutations back into the canonical ref-backed transcript.
}
const diagBaseline = verifyCfg.enabled ? snapshotDiagnostics() : new Map();
extensions/levelcode-ai/extension.js:1832
withImagesproduces Anthropic{type: 'image', source: ...}blocks here, butproviders.streamChatdispatches tostreamOpenAI, which passes its messages directly tobuildChatBody;toOpenAIMessagesis only used by the agent adapter. OpenAI-compatible chat therefore sends an unsupported image shape instead ofimage_url, so pasted images fail for normal BYOK/OpenRouter chat.
if (auto.names.length) { post({ type: 'autoContext', names: auto.names }); }
pendingContext = null;
post({ type: 'clearContext' });
post({ type: 'assistantStart' });
extensions/levelcode-ai/imageCost.js:60
- A valid extreme aspect ratio such as 1×10,000 rounds the narrow axis to 0 at the chosen scale.
fitToTierthen reports a zero-width result, and the equivalent rounding inclientTarget/normalizeImagecan create a zero-sized canvas and reject the attachment. Clamp every resized dimension to at least one pixel in both the pure target calculations and the webview normalizer.
let hi = Math.min(1, t.edge / Math.max(w, h));
const at = (s) => [Math.round(w * s), Math.round(h * s)];
extensions/levelcode-ai/imageCost.js:40
- The implementation documents high-res as Claude 4.7+, but only
claude-opus-4-(7|8|9)is recognized here; a Claude Sonnet or Haiku 4.7+ id falls back to the standard tier.imageBlockTokensthen under-counts the visual cost (for example, 1560 instead of the high-res 4784 cap), making the context meter inaccurate. Match all supported Claude families at 4.7+ and add a regression case.
if (/claude-(opus|sonnet|fable|mythos)-5/.test(id)) { return 'high'; }
if (/claude-opus-4-(7|8|9)/.test(id)) { return 'high'; }
extensions/levelcode-ai/imageStore.js:102
- Keeping refs in the persisted transcript means
sessions.resumewill plan from these blocks beforematerializeruns, butsessionResume.estimateTokenschargesJSON.stringify(content).length / 4. A stored 4K screenshot is therefore counted as roughly the ref length instead of thousands of visual tokens, so a resume can load far beyond its configured budget and only fail at the provider request. Feed the same model-aware image-cost estimator into resume planning, not only the compact-memory meter.
function materialize(root, slug, block) {
if (!block || block.type !== 'image') { return block; }
if (block.source) { return block; } // already materialized (or an inline block from elsewhere)
const data = read(root, slug, block.ref);
if (!data) { throw new Error('imageStore: attached image is missing from disk: ' + String(block.ref)); }
return { type: 'image', source: { type: 'base64', media_type: mediaTypeOf(block.ref), data } };
extensions/levelcode-ai/media/chat.html:1585
- This element is exposed as a focusable
role="button", but it only handlesonclick; Enter or Space on a focused image remove control does nothing. Add keyboard activation, consistent with the existing keyboard handling for file chips, so keyboard users can remove an attachment.
chipsEl.querySelectorAll('.imgx').forEach(function(x){
x.onclick = function(e){ e.stopPropagation(); removeImage(x.getAttribute('data-img')); };
extensions/levelcode-ai/media/chat.html:3230
- The
dragoverhandler accepts every file drag, but this handler returns beforepreventDefault()when the drop contains no images. Dropping a non-image file onto the panel can therefore trigger the browser's default open/navigate behavior and replace the chat. Cancel file drops before filtering, then ignore non-image files.
if (!ev.dataTransfer || !ev.dataTransfer.files || !ev.dataTransfer.files.length) { return; }
const imgs = Array.from(ev.dataTransfer.files).filter(function(f){ return /^image\//.test(f.type); });
if (!imgs.length) { return; }
extensions/levelcode-ai/media/chat.html:3106
- The vision check only runs in
attachImageFiles. If a user queues an image under a vision model and then switches models,doSendstill emitsimages: imgswithout revalidatingcanSeeImages, so the strict gate is bypassed and the blind provider receives the composed request. Reject or clear pending images when the selected model changes, or check again before building this payload.
const imgs = pendingImages.map(function(i){
return { media_type: i.media_type, base64: i.base64, w: i.w, h: i.h, bytes: i.bytes };
});
extensions/levelcode-ai/media/chat.html:3195
attachImageFilesis async and both paste/drop handlers invoke it without awaiting, so two rapid attachment events can enter this loop whilependingImages.lengthis still below the cap. Each call then passes the check before itsnormalizeImageawait resolves, allowing more thanIMG_MAX_PER_TURNimages to be added. Serialize attachment jobs or reserve the remaining slots before the first await.
for (const f of list) {
if (pendingImages.length >= IMG_MAX_PER_TURN) {
note('Up to ' + IMG_MAX_PER_TURN + ' images per message — the rest were not attached.');
break;
}
try {
const im = await normalizeImage(f);
extensions/levelcode-ai/providers/catalog.js:133
- Dynamic model discovery preserves
visionon each choice, but after selectionsupportsVisionForModelrecomputes only from static/heuristicmodelCaps. A dynamically discovered OpenRouter model that advertises image input under an unrecognized id is consequently treated as blind and cannot use the new attachment flow. Retain the dynamic capability metadata per provider/model or pass it into the gate.
function supportsVisionForModel(providerId, modelId) {
const p = getProvider(providerId);
if (!p) { return false; }
return modelCaps(modelId).vision === true;
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…rds 400'd
Three bugs from a real run, all mine.
1. THE THUMBNAIL WAS BROKEN BECAUSE NOTHING COULD RENDER IT. The webview CSP is
`default-src 'none'` with no img-src, so every image — including a data: URI built from
the attachment's own bytes — was blocked. I added images without touching the policy.
Now `img-src data:`, and deliberately NOT https:, so the panel still cannot fetch a
remote image.
2. AN IMAGE WITH NO WORDS RETURNED A 400. Anthropic rejects an empty text block outright
("text content blocks must be non-empty"), and sending an image with nothing typed
produced exactly that. Both paths now include the text block only when there IS text,
and fall back to the images alone. This is the case I deliberately made valid in I5 —
"look at this" is implied by attaching — so it was the one shape guaranteed to be hit.
3. DRAGGING FROM FINDER DID NOTHING. VS Code's workbench intercepts OS file drops before a
webview iframe sees them, so dataTransfer.files is empty — while the PATH is still
there as a uri-list. The drop handler now falls back to reading the uri-list and handing
the paths to the host, which reads them off disk and sends the bytes back for the same
normalizer a paste uses. Size is checked host-side BEFORE base64 crosses the message
bus, so a 200MB file is refused rather than serialized first.
Also I6: a visible Attach-image button next to +, using showOpenDialog. The picker is the
route that works regardless of what the webview is allowed to receive; the drop fallback is
best-effort on top of it.
Two guards needed updating rather than adding — the empty-text ternary changed the shape
they matched. That is the guards doing their job; I updated them and added one that pins
the 400 itself.
14 tests in imageAttach, 37 suites green.
Reported: dragging a PNG from Finder opens it in an editor tab instead of attaching it.
THE DROP NEVER REACHES US. VS Code's workbench handles an OS file drop before a webview
iframe sees any event — it opens the file. The panel's drop handler never fires, and the
uri-list fallback I added last round cannot fire either, because there is no event to fall
back from. This is platform behaviour for a webview, not something the extension can
intercept; patching it would mean patching the workbench, which is a different change in
a different repo.
So take the flow as it actually is. The drop lands as a TAB, which means the image the
user wanted is already open and named. Two ways to attach it from there:
- The image editor's own title bar gets an attach button, gated on the file extension.
That is the shortest possible path from what happened to what they wanted: drop,
then one click, in the place they are already looking.
- The picker lists images that are open in tabs, active tab first, before offering
Browse. Deduped across groups, file:// only, images only.
The picker still falls through to showOpenDialog, and paste is still the primary route.
Bypass-verified: open tabs no longer offered; the active tab not sorted first; the title
bar button moved out of navigation into the overflow.
15 tests in imageAttach, 37 suites green.
Drag-and-drop from Finder could not be fixed from the extension, so this is the first core patch the image work needed. WHY IT CANNOT LIVE IN THE EXTENSION. A webview iframe is never offered an OS file drop. The workbench takes it first and opens the file in a tab — which is what the report showed: orcs.png sitting in its own editor instead of attached to the chat. The panel's drop handler never fires, and the text/uri-list fallback I added last round has no event to fall back FROM. I was wrong to think that fallback could rescue it. THE PATCH. In DropOverlay.handleDrop, immediately before the URI-transfer branch hands off to ResourcesDropHandler, tryLevelCodeChatImageDrop forwards the paths to the extension via a new levelcode.ai.attachImagePaths command and consumes the drop. The paths travel by command rather than a new IPC channel so the diff stays a routing decision and nothing more — on a fork, the smaller the diff the cheaper every rebase. Deliberately narrow. Every condition is a reason not to change behaviour someone relies on: only when the chat is the ACTIVE editor of the group dropped on; only when no split is requested; only when EVERY dropped file is an image; only when the paths resolve (getPathForFile is native-only); and on a throw it falls through to the normal handler, because a dropped image doing nothing is worse than one that opens. TWO PROCESS MISTAKES WORTH RECORDING. The type-check "passed" the first time because tsc had CRASHED — a Node OOM, stack trace, zero error lines. I nearly reported that as clean. Re-ran with a bigger heap and then proved the check was real by planting a deliberate type error and watching it get caught. Then I regenerated patches/levelcode-core.patch wholesale, which swept 78 unrelated lines of de-brand link-strips into files.contribution.ts. CORE-PATCHES.md NOTE 3 documents exactly this trap — including that it had happened once before, at ~95 lines — because the checkout is already de-branded by the time you are looking at it. Backed it out and followed the documented path: append only the new file's entry, then verify the prior entries are byte-identical, the appended entry strips no MS doc links, and `git apply --check --reverse` accepts it. All three checked. 37 suites green.
THE × DID NOTHING, and the cause is worth naming because it is invisible in review. An image chip's × carries BOTH `x` and `imgx`. renderChips registers two handlers, and both ASSIGN .onclick rather than adding a listener — so the generic `.x` block, which runs second, silently overwrote the image one. Clicking × posted removeContext with a null id and the image stayed attached. Fixed by selector, `.x:not(.imgx)`, not by reordering. Ordering is what broke it, and a reorder fix would come apart the next time those two blocks move relative to each other. THE CAP IS NOW ONE NUMBER. levelcode.ai.chat.maxImagesPerMessage, default 5 (was a hardcoded 8 in the webview and a SECOND hardcoded 8 in the host's path reader — two numbers that could drift, and one route quietly allowing more than the other). Default 5 rather than the API's 20 because the cost is real: each image is ~1,800-3,000 input tokens, so a maxed-out message is a five-figure prompt before a word is typed. Anyone who wants 20 can set 20. Clamped in code, not just in the schema: minimum/maximum only guide the settings editor, and a hand-edited settings.json reaches the extension unchecked — a 0 there would otherwise disable attaching entirely, and a huge value would sail past the API's own ceiling. The cap is also visible before it bites: the attach button dims at the limit and its tooltip counts (3/5), and the refusal says how many were dropped and how to make room. Bypass-verified, each by reverting the fix: - the generic handler stealing the image × again (the reported bug) - removeImage not re-rendering, so the chip stays on screen - the drop path keeping its own hardcoded cap - a cap of 0 disabling attaching entirely - the BYOK config path never publishing the cap - the cap becoming per-batch instead of cumulative One older guard needed updating — it pinned the exact refusal wording, which I changed. That is the guard doing its job. 17 tests in imageAttach, 37 suites green.
The × inherited .chip .x — 14px box, 9px glyph — which is small anywhere and too small on an image chip. Now 22px with a 15px glyph, and a higher resting opacity: an affordance you have to hunt for reads as one that is not there. Scoped to .chip.imgchip .imgx rather than raising .chip .x for everything. Two reasons the image chip can carry more: it is twice the height of a text chip, so 22px fits without changing the row; and removing the wrong attachment is cheap to undo on a file pin and annoying on a screenshot you have to go and take again. Also fixed while in here: the × is role="button" tabindex="0" and had only a click handler, so a keyboard user could focus it and then not activate it — reachable-but-dead, which is worse than not reachable. Enter and Space now work, and it has a visible focus ring. Rendered both chip kinds side by side to check the two sizes read as deliberate rather than inconsistent. 37 suites green.
…s in the panel
THE CORE PATCH WAS NOT FIRING. An extension-created webview panel does not keep the
viewType the extension registers: the API layer rewrites it
(WebviewViewTypeTransformer('mainThreadWebview-') in mainThreadWebviewPanels.ts), so the
editor input reads 'mainThreadWebview-levelcode.ai.chat'. My check compared against the
bare id, was silently always false, and a plain drag still opened the file.
That also explains the report that "drag and drop + shift works". Shift is a DIFFERENT
path — isDragIntoEditorEvent makes onDragEnter return early, the overlay never appears,
and handleDrop (where the patch lives) never runs. What worked was the webview's own
uri-list fallback. I claimed the patch worked without ever seeing it fire.
Now matches both forms. Re-appended to patches/levelcode-core.patch per NOTE 3 rather than
regenerated wholesale, and `git apply --check --reverse` accepts it.
THE UX PASS.
Errors moved INTO the composer. They were VS Code notifications: appearing seconds later
in the far corner, a long way from the paste, outliving the moment they described.
Attachment problems are small, immediate and local, so the message is too — under the
chips, aria-live, self-clearing after six seconds.
A chip appears the INSTANT you paste. Decoding and re-encoding a 4K screenshot takes long
enough to read as "nothing happened"; the honest fix is to show the attachment
immediately, not to make the work faster. A shimmering placeholder is replaced in place
when the bytes are ready — and if it was removed mid-decode it stays removed, rather than
reappearing when its bytes arrive.
The chip says what the image will COST — ~3.0k on the chip, the exact figure and size in
the tooltip. This product meters credits per turn and an image is a few thousand input
tokens; someone deciding whether to attach three should see that before sending, not
after being billed. Compact because three full-width chips wrapped the tray onto a second
row, which made the composer jump as you pasted.
Click any thumbnail — tray or transcript — for a full-size view. A 28px thumb cannot tell
you WHICH screenshot you attached, which is the one thing worth checking before sending.
Escape closes it and drops the src, so the bytes do not stay live in the DOM.
An existing guard caught a bare toLocaleString() in the new cost figure — the house rule is
a fixed CREDIT_LOCALE so the editor and the dashboard never disagree about a number. Good
catch by a test I did not write.
Bypass-verified: errors thrown back to a toast; no placeholder; an image reappearing after
being removed mid-decode; a failed image leaving its placeholder stuck; the cost shown full
width; the zoom keeping bytes live on close; transcript images no longer opening.
21 tests in imageAttach, 37 suites green.
16ae48c to
eda94c7
Compare
…ot how many things
Reported against the Claude Code reference: a finished run collapsed to "3 steps", which
tells nobody anything, where the reference says "Ran 2 commands".
THE CAUSE was narrower than it looked. groupAggregate already builds a real sentence for
file work and commands — "Read and edited extension.js, ran a command". What it could not
describe were the SETUP steps: project rules, project memory, recall, skills, MCP. Those
arrive as pre-baked emoji chips, chipStep dropped every one of them into `note`, `note`
contributes no phrase, and a run made only of setup fell through to the bare count.
So they are classified now — rules / memory / recall / skill / mcp / mcpcall / preview —
and each contributes a phrase. The screenshot's run goes from
3 steps
to
Loaded project rules and memory, connected MCP tools
Two distinctions worth having:
- An MCP tool CALL (🔌 github · search_code) is different work from setup chatter about
servers (🔌 mcp · github (26) · 2/26 allow-listed). The first says "Called github", the
second "connected MCP tools". Collapsing them would let a run that actually used a
tool read as if it had only connected one.
- Setup is named LAST, so what changed still leads the sentence. A run that edited a file
and loaded rules reads "Edited a.ts, ran a command, loaded project rules" — a test pins
that ordering.
And the last-resort fallback names the first step and counts the rest ("Something unusual
happened and 1 more") rather than emitting a bare number. Sentence-cased like every other
path, which the first version was not.
The expanded steps now sit in one hairline container instead of loose rows under a heading
— closer to the reference, and it replaces a vertical cue rather than adding one: the group
already had a rail and a chevron, and a third was what made the expanded state look busy.
Bypass-verified: rules unrecognised; setup contributing no phrase; an MCP call lumped in
with setup; the fallback not sentence-cased; no setup kind detected at all.
groupAggregate and chipStep are now exposed on the test harness's api — they were extracted
into the sandbox but not reachable, so neither had ever been called directly by a test.
20 tests in groupReducer (was 16), 37 suites green.
…oup read as text Two complaints, one behavioural and one visual. "WHY ON EACH TURN THERE IS AN MCP LOADING AND TOOLS". Because the run's context is rebuilt every run — which is correct, a run's servers are whatever is configured and reachable now — and the chips were posted unconditionally alongside it. Rebuilding it every turn and ANNOUNCING it every turn are different things, and three identical rows before every single answer is noise the reference transcript does not have. The three lines are collected instead of posted (the MCP one is handed back from setupMcp rather than posted there, so nothing is decided before the whole picture is known), hashed, and shown only when that signature changes. A signature and not a flag, because the announcement has to come back the moment a server drops out, a rules file appears, or memory arrives for the first time — a boolean would go quiet forever after the first run. An EMPTY picture clears the memo rather than pinning it, or the first real context would be suppressed. Failures still post immediately: a server that broke is news every time. A new conversation hears it again — the suppression is about repetition WITHIN a conversation, not across them, so resetConversationState clears the memo. "THIS TIMELINE LINE AROUND GREEN CHECKMARK SEEMS NO LONGER CONSISTENT". Fair, and it was my doing. The rail threads consecutive tool nodes into one connected line, which is right for loose nodes. A group is already one unit — so when I put its body in a bordered container last commit, the rail became a second grouping cue doing the same job. A circled glyph, a vertical line AND a box around three rows of grey text. My own commit message claimed the container "replaces a vertical cue rather than adding one". It did not; I left the rail in. Now a group has no rail and no circled node. It sits on the same left edge as the paragraph above it, the summary is muted like every other piece of secondary chrome, and the outcome is a small glyph inside the line rather than a badge beside it. One grouping device, not three. Failure keeps its colour — quiet is for history, not for problems. The rail stays for ungrouped nodes, where it still does its threading job. Bypass-verified: announced every turn again; a boolean memo instead of a signature; MCP posting outside the decision; an empty picture pinning the memo shut; a new chat never hearing the context again. 5 tests in the new contextAnnounce suite, 38 suites green.
"Ran 2 commands ⌄", not "⌄ Ran 2 commands" — matching the reference. Flipped in BOTH headers, not just the group. A trailing chevron on the group summary and a leading one on a command card would be two disclosure controls disagreeing inside the same transcript, which is a worse inconsistency than the one being fixed. It hugs the label rather than right-aligning to the card edge. A disclosure control belongs beside the thing it opens; a chevron alone at the far right of a wide row reads as unrelated chrome. Change counts (+42 -8) stay trailing after it — they are status, not a control. Moved in the MARKUP rather than with CSS `order`, so tab order and visual order stay the same thing. The guard pins that too: reordering with `order` would satisfy a looks-right-on-screen check while leaving a keyboard user tabbing through a line backwards. Bypass-verified by flipping it back. 21 tests in groupReducer, 38 suites green.
Reported against the reference: the rows sat almost on the border and the block read as a slab rather than a list. TWO CAUSES, and the fill was the bigger one. The container had a hairline border AND a tinted background — two containers drawn on top of each other. A tint also fights whatever ground the theme paints, which is why it looked heavier here than in the reference. The border alone is the container now; rows keep the page's own ground, and it holds on a light theme as well as a dark one (checked in both). Then the inset. Rows had 2px of horizontal padding, so text started a couple of pixels off the border — which reads as overflowing a container even when it is not. Now 14px, matching the reference's proportions, with the summary line still aligned to the container edge so the expanded state reads as one block rather than two stacked things. Also added overflow:hidden, without which the first and last rows square off the corners the radius had just rounded — visible only once the fill was gone. A bypass that did not bypass: my first attempt at "put the fill back" replaced the first `background: transparent` in the file, which is `body`, not the group container — so the test passed and I nearly recorded a guard as verified when nothing had been changed. Re-ran it against the right rule, where it fails correctly. Worth writing down: a bypass targeting a common declaration needs to be anchored on something unique to the rule under test. Bypass-verified: the tinted fill restored; rows crammed back to 2px; overflow:visible squaring the corners. 35 tests in webviewCss, 38 suites green.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
extensions/levelcode-ai/media/chat.html:3299
- Sending while an image is still "preparing" will post an image object with missing
base64/media_type(because placeholders haveloading: true). The host then refuses it as an empty image, so the user's intended attachment is silently lost on send.
Block send until all pending images are ready (or filter out placeholders and warn).
const imgs = pendingImages.map(function(i){
return { media_type: i.media_type, base64: i.base64, w: i.w, h: i.h, bytes: i.bytes };
});
add('user', (imgs.length ? imgs.map(function(i, k){
return '<img class="msgimg" src="' + escAttr(pendingImages[k].url) + '" alt="attached image">';
}).join('') : '') + render(t));
Eleven comments. Two (the agent branch dropping images, and the generic .x handler stealing
the image ×) were already fixed in commits later than the reviewed diff. The other nine were
all real.
THE METER WAS UNDERCOUNTING. estimateMsgTokens takes a modelId to pick the resolution tier —
4,784 visual tokens on high-res against 1,568 on standard — and BOTH compaction call sites
omitted it. So every screenshot was costed at a third of what a Claude 4.7+ model is charged,
by the very module whose contract says it must never under-count. meterModel() resolves it
the same way currentContextLimit does, gateway and BYOK alike.
THE SEND PATH HAD TWO HOLES. Normalization is async and a placeholder chip carries no bytes,
so sending mid-decode posted an attachment with undefined media_type and base64 — refused
host-side, and the image vanished from a message the user watched themselves attach. doSend
now waits on in-flight work and refuses a surviving placeholder outright. Separately, the
model was only checked at ATTACH time: switch models between attaching and sending and the
images went to a model that cannot read them. Re-checked at send, refusing without
discarding anything typed or attached.
NEW CHAT INHERITED THE TRAY. The reset handler cleared the log and the context chips and
never touched pendingImages, so the next conversation opened holding the last one's
screenshots and stored them under the new session.
THE NOT-ATTACHED COUNT COULD GO NEGATIVE: it subtracted the whole tray from the batch size.
Four attached, cap five, drop two, and it claimed "-3 not attached". Counted from the
position in the batch now.
THE VISION GATE IGNORED THE PROVIDER. The registry enumerates vision providers — anthropic,
openai, openrouter declare it; ollama and `custom` deliberately do not — and the gate read
only the model id, so `custom` (an arbitrary user-supplied endpoint) was handed images
whenever the model NAME looked right. Both halves must agree now, as supportsToolsForModel
already required.
I1 FIXED ONE LOOP AND LEFT THE OTHER. The assistant branch still dropped unknown blocks
silently — the same bug, one branch over. It throws now; `thinking` is an explicit,
documented drop rather than a fall-through, because an OpenAI-shaped request has nowhere to
put it.
"DELETED WITH THEM" WAS NOT TRUE. Sessions are append-only and trash() only writes a
lifecycle event, so nothing ever removed a stored image — and refsIn, which I wrote for
exactly this, had no caller. There is a real sweep now, run on session seal, with an AGE
FLOOR: a normal chat writes media whose refs are never persisted anywhere, so
unreferenced-means-delete would break the open conversation. The comment says what the code
does now.
AND THE FEATURE GAP: several images are introduced by name ("Image 1:", "Image 2:") per the
vision guidance, so a question — and every follow-up turn — can refer to them. Only when
there is more than one; labelling a lone screenshot is noise.
TWO OF MY OWN GUARDS PASSED ON BROKEN CODE while verifying this, both from matching text
rather than behaviour: a commented-out resetImages() still satisfied /resetImages\(\)/, and
`inflightImages.size` appears twice in doSend so disabling the gate left the name present.
Fixed by stripping comments before matching, and by asserting the gate's POSITION relative
to the send plus the presence of the await. That is the fourth instance of this family this
session.
Bypass-verified, each by reverting: the undercount; the negative count; the wait deleted and
the await removed; no model re-check; New Chat inheriting; custom taking images; the
assistant loop silent again; the sweep removed; the labels dropped.
30 tests in imageAttach (was 21), 38 suites green.
Implements docs/IMAGES.md — local, session-attached, no upload. Slices I1–I5 of seven.
The cap is measured, not chosen
The plan argued 1568px from first principles. Claude Code's transcripts are on disk, so I read them instead of guessing: 24 images, every re-encoded one exactly 2000px on the long edge. That's the threshold the vision docs name for staying clear of the stricter per-image limit above 20 images per request — the largest size that is never unsafe, and above both tier caps, so the server does the final downscale and we never discard fidelity it would have kept.
The same transcripts confirmed a rule I'd derived rather than observed: under the cap, pass through untouched in the original format; only oversize images are resized and re-encoded to WebP. Re-encoding what didn't need resizing only stacks artifacts, worst on screenshots of code.
Verified in a real browser against real images:
Aspect ratios come out exact.
Slices
I1 — the wire seam, and a bug that predates images.
toOpenAIMessagesfell through on any block it didn't recognise, so an unknown type vanished between composer and wire with no error and no log line — the model would answer confidently about content it was never sent. Now a throw naming the type. Images map toimage_url; a Files-APIsourceis refused rather than sent as something.I2 — the vision gate, deliberately stricter than the tools gate: unknown model means no vision, because attaching to a blind model costs a composed message while a disabled button costs a click.
I3 —
imageCost.js. The formula I had in mind (w*h/750) was stale. It's⌈w/28⌉ × ⌈h/28⌉over 28px patches with per-tier caps. Doc-parity test reproduces every worked example in the vision docs on both tiers.I4 — local store + the meter. Content-addressed under the session's
media/, tmp+rename, 5MB cap, andisRefpins the shape so../../etc/passwdis not a ref. The meter now counts images by real visual cost — a test pins that the storage shape doesn't move the number.I5 — paste, drop, chips, refusal. The refusal runs at attach time, not send time, because the composer clears on send.
canSeeImagesships from both config paths — a test pins that, since one silently allowing is the kind of half-gate that passes review.Why a
media/dir rather than Claude Code's inline base64Claude Code inlines bytes in its JSONL and that's fine there. It isn't here, for a reason specific to this codebase:
sessionStore.scanProjectdoesreadFileSync+JSON.parseon every session file wheneverindex.jsonis missing, malformed, or on an older schema — first run, and after any schema bump. Inlined bytes would make drawing a list of session titles parse every screenshot in every session.Not in this PR
I6 — the Add Files picker doesn't yet treat an image as an image. I7 — the remaining refusals (too many images, oversize at the host boundary) are partly in place via the store's caps but not surfaced as first-class messages.
37 suites green; 29 bypasses verified across the five slices.