fix(images): nine review findings from #90, and the multi-image gap

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.

Author: ndemianc

extensions/levelcode-ai/extension.js

@@ -148,6 +148,19 @@ function contextLimitFor(providerId, model) {
 }
 
 /** The active model's context window (tokens) — drives the chat context-usage meter. */
+/**
+ * The model the meter should cost against — the same resolution currentContextLimit uses.
+ *
+ * estimateMsgTokens needs it to pick an image's resolution tier: the high-res tier is 4,784 visual
+ * tokens against the standard tier's 1,568, so calling it without a model silently costs every
+ * screenshot at a THIRD of what a Claude 4.7+ model is actually charged.
+ */
+function meterModel() {
+	const cfg = aiConfig();
+	if (providerMode() === 'gateway' && cloudSignedIn) { return capsModel(gatewayModel()); }
+	return activeModel(cfg, currentProviderId());
+}
+
 function currentContextLimit() {
 	const cfg = aiConfig();
 	if (providerMode() === 'gateway' && cloudSignedIn) {
@@ -1109,7 +1122,17 @@ function sealLiveSession(why) {
 		if (!m) { return; }
 		const sealedId = m.liveId();
 		m.seal('done');
-		if (sealedId) { enrichMemoryAsync(sealedId); }   // outcome + fact promotion, off the critical path
+		if (sealedId) { enrichMemoryAsync(sealedId); }
+		// Sealing is the natural moment to take out the rubbish: rare, already off the hot path, and
+		// the point at which a conversation's refs have just been written. Nothing else deletes media
+		// — sessions are append-only and trash() only marks a lifecycle — so without this the folder
+		// grows for the life of the project.
+		setTimeout(() => {
+			try {
+				const swept = m.sweepMedia();
+				if (swept.removed) { dbg('media.swept', { removed: swept.removed, kb: Math.round(swept.bytes / 1024) }); }
+			} catch (e) { dbg('media.sweep.error', { msg: String((e && e.message) || e) }); }
+		}, 0);   // outcome + fact promotion, off the critical path
 		dbg('sessions.sealed', { why, id: sealedId });
 	} catch (e) {
 		dbg('sessions.seal.error', { why, msg: String((e && e.message) || e) });
@@ -1544,7 +1567,7 @@ async function compactAgentMemory() {
 	if (abort) { return { ok: false, reason: 'running' }; }
 	const msgs = agentMessages;
 	const KEEP_RECENT = 8;
-	const beforeMsgTokens = estimateMsgTokens(msgs);
+	const beforeMsgTokens = estimateMsgTokens(msgs, meterModel());
 	if (msgs.length <= KEEP_RECENT + 2) { return { ok: false, reason: 'small' }; }
 	const cut = findCompactionCut(msgs, KEEP_RECENT);
 	if (cut < 0) { return { ok: false, reason: 'noboundary' }; }
@@ -1591,7 +1614,7 @@ async function compactAgentMemory() {
 	// transcript for them (it guards on indexOf), so keeping them would offer a half-working rollback.
 	for (let i = checkpoints.length - 1; i >= 0; i--) { if (msgs.indexOf(checkpoints[i].goalMsg) < 0) { checkpoints.splice(i, 1); } }
 
-	const afterMsgTokens = estimateMsgTokens(msgs);
+	const afterMsgTokens = estimateMsgTokens(msgs, meterModel());
 	dbg('compact.done', { cut, beforeMsgTokens, afterMsgTokens, msgs: msgs.length });
 	return { ok: true, beforeMsgTokens, afterMsgTokens };
 }
@@ -1620,7 +1643,7 @@ async function agentFlow(text, imageBlocks) {
 	// Agent mode is the DEFAULT, so this is the path most pasted screenshots take. Blocks only when
 	// there IS an image — a text-only goal stays a plain string so cached prefixes keep their bytes.
 	const goalMsg = (imageBlocks && imageBlocks.length)
-		? { role: 'user', content: text ? [...imageBlocks, { type: 'text', text }] : imageBlocks }
+		? { role: 'user', content: text ? [...labelImages(imageBlocks), { type: 'text', text }] : labelImages(imageBlocks) }
 		: { role: 'user', content: text };
 	currentCheckpoint = { turnId: ++checkpointSeq, label: (text || '').slice(0, 60), ts: Date.now(), goalMsg: goalMsg, files: new Map() };
 	checkpoints.push(currentCheckpoint);
@@ -1870,6 +1893,26 @@ function storeImages(images) {
 	return out;
 }
 
+/**
+ * Introduce each image with a short label when there is more than one.
+ *
+ * Straight from the vision guidance: with several images, precede each with "Image 1:", "Image 2:"
+ * so the conversation can refer to them by name — in the question being asked, and in every
+ * follow-up turn afterwards. Without it, "the second screenshot" has nothing to bind to.
+ *
+ * Only when there are several. A single image needs no name, and labelling it would put a pointless
+ * text block ahead of every screenshot anyone pastes.
+ */
+function labelImages(blocks) {
+	if (!Array.isArray(blocks) || blocks.length < 2) { return blocks; }
+	const out = [];
+	blocks.forEach((b, i) => {
+		out.push({ type: 'text', text: 'Image ' + (i + 1) + ':' });
+		out.push(b);
+	});
+	return out;
+}
+
 /**
  * A copy of `msgs` with every stored image turned into a real wire block.
  *
@@ -1924,7 +1967,7 @@ async function handleSend(text, images) {
 	// An empty text block is a 400 from Anthropic ("text content blocks must be non-empty"), and an
 	// image sent with no words produces exactly that. Include the text block only when there is text.
 	conversation.push(imageBlocks.length
-		? { role: 'user', content: userContent ? [...imageBlocks, { type: 'text', text: userContent }] : imageBlocks }
+		? { role: 'user', content: userContent ? [...labelImages(imageBlocks), { type: 'text', text: userContent }] : labelImages(imageBlocks) }
 		: { role: 'user', content: userContent });
 	post({ type: 'userMessage', text });
 	if (auto.names.length) { post({ type: 'autoContext', names: auto.names }); }

extensions/levelcode-ai/imageStore.js

@@ -113,4 +113,39 @@ function refsIn(msgs) {
 	return out;
 }
 
-module.exports = { MEDIA_EXT, MAX_BYTES, mediaDir, refPath, isRef, put, read, mediaTypeOf, materialize, refsIn };
+/**
+ * Delete media nothing refers to any more.
+ *
+ * Needed because nothing else deletes it. Sessions are append-only and `trash()` only writes a
+ * lifecycle event — the transcript stays on disk — so "the images go away with the session" was
+ * never true. And a normal (non-agent) chat writes media without ever calling recordTurn, so its
+ * refs are not in any session file at all.
+ *
+ * That second case is why there is an AGE FLOOR rather than a plain unreferenced-means-delete rule:
+ * a file written moments ago may belong to a live conversation whose refs have not been persisted
+ * and may never be. Deleting those would break the open chat. A week is long past the point where a
+ * conversation is still live, and it bounds the growth, which is the actual complaint.
+ *
+ * @param keep a Set of refs still referenced (from refsIn over the project's sessions)
+ */
+function sweep(root, slug, keep, maxAgeMs) {
+	const dir = mediaDir(root, slug);
+	const cutoff = Date.now() - (maxAgeMs > 0 ? maxAgeMs : 7 * 24 * 60 * 60 * 1000);
+	let removed = 0, bytes = 0;
+	let names;
+	try { names = fs.readdirSync(dir); } catch { return { removed: 0, bytes: 0 }; }
+	for (const name of names) {
+		if (!isRef(name)) { continue; }              // never touch anything we did not write
+		if (keep && keep.has(name)) { continue; }
+		const full = path.join(dir, name);
+		try {
+			const st = fs.statSync(full);
+			if (st.mtimeMs > cutoff) { continue; }    // young enough to belong to a live conversation
+			fs.unlinkSync(full);
+			removed++; bytes += st.size;
+		} catch { /* raced with another window, or already gone — either way, nothing to do */ }
+	}
+	return { removed, bytes };
+}
+
+module.exports = { MEDIA_EXT, MAX_BYTES, mediaDir, refPath, isRef, put, read, mediaTypeOf, materialize, refsIn, sweep };

extensions/levelcode-ai/media/chat.html

@@ -3279,11 +3279,34 @@
   function caretAtFirstLine(el){ return el.value.lastIndexOf('\n', el.selectionStart - 1) === -1; }
   function caretAtLastLine(el){ return el.value.indexOf('\n', el.selectionEnd) === -1; }
   function histSet(text){ input.value = text; auto(); const n = text.length; try { input.setSelectionRange(n, n); } catch (e) {} }
-  function doSend(){
+  async function doSend(){
     if (streaming) { flushAll = true; ensurePump(); vscode.postMessage({ type: 'stop' }); return; }
     const t = input.value.trim();
     // An image with no words is a real message — "look at this" is implied by attaching it.
     if (!t && !pendingImages.length) return;
+
+    // Wait for anything still decoding. A placeholder chip carries no bytes, so sending now would
+    // post an attachment with undefined media_type and base64 — refused host-side, and the image
+    // would vanish from a message the user watched themselves attach.
+    if (inflightImages.size) {
+      note('Still preparing ' + inflightImages.size + ' image' + (inflightImages.size === 1 ? '' : 's') + '…');
+      for (let i = 0; i < 200 && inflightImages.size; i++) { await new Promise(function(r){ setTimeout(r, 50); }); }
+      const el = document.getElementById('imgnote'); if (el) { el.hidden = true; }
+      if (inflightImages.size) { note('An image is taking too long to prepare — remove it or try again.'); return; }
+    }
+    // Nothing may be sent while a placeholder survives: a decode that failed silently would
+    // otherwise ride along as an empty attachment.
+    if (pendingImages.some(function(i){ return i.loading; })) {
+      note('An image did not finish preparing — remove it and try again.'); return;
+    }
+
+    // RE-CHECK the model. The attach-time gate is not enough: a model can be switched between
+    // attaching and sending, and this path would otherwise hand images to a model that cannot read
+    // them. Refuse without discarding anything the user typed or attached.
+    if (pendingImages.length && !canSeeImages) {
+      note((canSeeImagesModel || 'The selected model') + ' cannot read images. Switch back to a vision model, or remove the attachments.');
+      return;
+    }
     if (t) { cmdHistory.push(t); if (cmdHistory.length > 200) { cmdHistory.shift(); } }   // record for ↑/↓ recall
     histIdx = -1; histDraft = '';
     // Slash commands — handled locally (deterministic, no LLM call).
@@ -3325,6 +3348,11 @@
   const IMG_OK = { 'image/png': 1, 'image/jpeg': 1, 'image/gif': 1, 'image/webp': 1 };
   let pendingImages = [];   // { id, url, w, h, media_type, base64, bytes }
   let imgSeq = 0;
+  // Normalization is async, and a placeholder chip carries no bytes. Sending mid-decode would post
+  // an attachment with undefined media_type/base64 — the host would refuse it and the image would
+  // be silently lost from a message the user watched themselves attach. Track the work so doSend
+  // can wait for it.
+  const inflightImages = new Set();
 
   /**
    * A user-facing "that did not work" line, shown WHERE IT HAPPENED — directly under the composer,
@@ -3402,11 +3430,15 @@
         + 'Claude, GPT-4o and Gemini all read them — and paste again.');
       return true;
     }
-    for (const f of list) {
+    for (let fi = 0; fi < list.length; fi++) {
+      const f = list[fi];
       if (pendingImages.length >= IMG_MAX_PER_TURN) {
-        const dropped = list.length - pendingImages.length;
+        // From the position in THIS batch, not from the tray total. Subtracting the tray from the
+        // batch goes negative the moment the tray already holds more than the batch does — four
+        // attached, cap five, drop two, and the message claimed "-3 were not attached".
+        const dropped = list.length - fi;
         note('Up to ' + IMG_MAX_PER_TURN + ' images per message. '
-          + (dropped > 0 ? dropped + ' were not attached — ' : '') + 'remove one to add another.');
+          + (dropped > 0 ? dropped + ' not attached — ' : '') + 'remove one to add another.');
         break;
       }
       // A placeholder goes in FIRST. Decoding and re-encoding a 4K screenshot takes long enough to
@@ -3415,6 +3447,7 @@
       const id = 'img' + (++imgSeq);
       pendingImages.push({ id: id, loading: true, name: f.name || 'image' });
       renderChips();
+      inflightImages.add(id);
       try {
         const im = await normalizeImage(f);
         im.id = id;
@@ -3427,6 +3460,8 @@
         pendingImages = pendingImages.filter(function(x){ return x.id !== id; });
         renderChips();
         note(String((e && e.message) || e));
+      } finally {
+        inflightImages.delete(id);
       }
     }
     return true;
@@ -3463,7 +3498,10 @@
       : (n ? ('Attach an image (' + n + '/' + IMG_MAX_PER_TURN + ') — or paste with ⌘V')
            : 'Attach an image — or paste a screenshot with ⌘V');
   }
-  function clearImages(){ pendingImages = []; }
+  function clearImages(){ pendingImages = []; inflightImages.clear(); }
+
+  /** Everything a fresh conversation must forget about attachments. */
+  function resetImages(){ clearImages(); renderChips(); const el = document.getElementById('imgnote'); if (el) { el.hidden = true; } }
 
   // Paste is the primary way in — the whole interaction is screenshot, then Cmd+V.
   input.addEventListener('paste', function(ev){
@@ -4136,6 +4174,7 @@
     else if (m.type === 'context'){ selLabel = m.label; renderChips(); }
     else if (m.type === 'clearContext'){ selLabel = null; renderChips(); }
     else if (m.type === 'reset'){
+      resetImages();   // a new conversation must not inherit the previous one's attachments
       log.innerHTML = '<div id="empty"><div class="big">New chat</div>Ask about your code, or describe what to build.</div>';
       // Wiping the log detaches any open group; drop the dangling refs so the next run starts clean —
       // otherwise groupAppend() appends into a disconnected .groupbody (nothing shows), and a stale

extensions/levelcode-ai/providers/catalog.js

@@ -130,6 +130,21 @@ function supportsToolsForModel(providerId, modelId) {
 function supportsVisionForModel(providerId, modelId) {
 	const p = getProvider(providerId);
 	if (!p) { return false; }
+	// The PROVIDER gate first, exactly as supportsToolsForModel does. Without it this checked only
+	// the model id, so `custom` — an arbitrary OpenAI-compatible endpoint with no declared vision
+	// capability — returned true for any model whose NAME looked like a vision model, and the
+	// attachment gate would hand images to an endpoint nobody said could read them.
+	//
+	// The registry ENUMERATES vision providers — anthropic, openai, openrouter and the gateway
+	// declare `vision: true`; ollama and `custom` deliberately do not. Reading only the model id
+	// ignored that: `custom` is an arbitrary user-supplied endpoint, and any model NAMED like a
+	// vision model would have been handed images nobody said it could read.
+	//
+	// So both halves must agree, exactly as supportsToolsForModel requires both. A custom endpoint
+	// that does serve a vision model needs `vision: true` on its registry entry to opt in; there is
+	// deliberately no per-user override, because the honest place to declare a provider's
+	// capabilities is the provider registry.
+	if (!(p.caps && p.caps.vision === true)) { return false; }
 	return modelCaps(modelId).vision === true;
 }
 

extensions/levelcode-ai/providers/translate.js

@@ -106,6 +106,17 @@ function toOpenAIMessages(system, messages, opts) {
 						function: { name: b.name, arguments: JSON.stringify(b.input == null ? {} : b.input) }
 					});
 				}
+				// Reasoning an assistant produced is not something an OpenAI-shaped request carries,
+				// and re-sending it is not required to continue a conversation — dropping it is a
+				// deliberate translation, not a loss, so it is named rather than left to the
+				// fall-through below.
+				else if (b.type === 'thinking' || b.type === 'redacted_thinking') { continue; }
+				else {
+					// Same rule as the user loop. A block type we do not understand is a bug in us,
+					// and a request that silently loses part of an assistant turn desynchronises the
+					// conversation the model is asked to continue.
+					throw new Error('translate: unsupported content block in an assistant message: ' + String(b.type));
+				}
 			}
 			const msg = { role: 'assistant', content: text ? text : null };
 			if (toolCalls.length) { msg.tool_calls = toolCalls; }

extensions/levelcode-ai/sessions.js

@@ -19,6 +19,7 @@ const store = require('./sessionStore');
 const events = require('./sessionEvents');
 const planner = require('./sessionResume');
 const memory = require('./sessionMemory');
+const imageStore = require('./imageStore');
 
 /**
  * @param {{ root: string, slug: string, projectPath: string,
@@ -326,15 +327,35 @@ function createSessions(opts) {
 	}
 	/** Where this project's memory lives (for opening it — the transparency promise). */
 	function memoryPaths() { return { dir: memory.memoryDir(root, slug), journal: memory.journalFile(root, slug), memoryMd: memory.memoryMdFile(root, slug) }; }
-	/** Where attached images live: beside this project's sessions, so they are deleted with them. */
+	/**
+	 * Where attached images live: `media/` beside this project's sessions.
+	 *
+	 * PROJECT-scoped, not session-scoped, and nothing removes a file when a session goes away —
+	 * sessions are append-only and `trash()` only writes a lifecycle event. `sweepMedia()` below is
+	 * what actually bounds this.
+	 */
 	function mediaRoot() { return { root, slug }; }
 
+	/**
+	 * Delete images no session in this project refers to any more, and that are old enough not to
+	 * belong to a live conversation. Returns { removed, bytes }; never throws.
+	 */
+	function sweepMedia(maxAgeMs) {
+		try {
+			const keep = new Set();
+			for (const entry of list()) {
+				for (const ref of imageStore.refsIn(transcript(entry.id) || [])) { keep.add(ref); }
+			}
+			return imageStore.sweep(root, slug, keep, maxAgeMs);
+		} catch (e) { return { removed: 0, bytes: 0 }; }
+	}
+
 	/** The session index for this project (what the panel/view list). Empty (never throws) if unreadable. */
 	function list() { try { return store.loadIndex(root, slug).entries; } catch (e) { return []; } }
 
 	function liveId() { return live ? live.id : null; }
 
-	return { ensure, recordTurn, seal, resume, fork, archive, trash, restore, setPinned, rename, autoArchiveStale, digest, consolidate, transcript, refineSummary, recall, recallFacts, memoryItems, forget, recordFacts, factsList, factAction, supersedeFact, memoryPaths, mediaRoot, list, liveId };
+	return { ensure, recordTurn, seal, resume, fork, archive, trash, restore, setPinned, rename, autoArchiveStale, digest, consolidate, transcript, refineSummary, recall, recallFacts, memoryItems, forget, recordFacts, factsList, factAction, supersedeFact, memoryPaths, mediaRoot, sweepMedia, list, liveId };
 }
 
 module.exports = { createSessions };