Dedupe path-mention images by content hash

Author: TheGreatAxios

src/tui/image-attachments.test.ts

@@ -5,6 +5,7 @@ import { tmpdir } from "node:os";
 import { join } from "node:path";
 import {
   extractPastedImagePaths,
+  findDuplicateAttachment,
   findImagePathMentions,
   imageMimeTypeForPath,
   capImageForIngestion,
@@ -160,10 +161,10 @@ describe("image attachment helpers", () => {
     expect(result.contentType).toBe("image/png");
   });
 
-  // The dedupe fix (see shell.ts attachClipboardImage) rests entirely on this:
-  // two ingests of identical source bytes must hash identically even though
-  // capImageForIngestion re-encodes oversized images through `sips`, whose
-  // JPEG output is not byte-stable across runs. Sizing the fixture above
+  // Identity is SHA-256 of the source bytes, not the (lossy, non-deterministic)
+  // capped output -- two ingests of identical source bytes must hash identically
+  // even though capImageForIngestion re-encodes oversized images through `sips`,
+  // whose JPEG output is not byte-stable across runs. Sizing the fixture above
   // DOWNSCALE_THRESHOLD_BYTES (300 KB) exercises that re-encode path -- a
   // small fixture would pass even if the hash were taken after capping.
   test("hashes identical source bytes the same regardless of filename, even through the sips recompression path", async () => {
@@ -187,3 +188,29 @@ describe("image attachment helpers", () => {
     }
   });
 });
+
+describe("findDuplicateAttachment", () => {
+  const first = {
+    id: "a",
+    name: "kept.png",
+    contentType: "image/png",
+    data: new Uint8Array([1]),
+    contentHash: "hash-a",
+  };
+  const other = {
+    id: "b",
+    name: "other.png",
+    contentType: "image/png",
+    data: new Uint8Array([2]),
+    contentHash: "hash-b",
+  };
+
+  test("returns the first existing attachment with the same content hash", () => {
+    const candidate = { ...first, id: "later", name: "copy.png", path: "/tmp/copy.png" };
+    expect(findDuplicateAttachment([first, other], candidate)).toBe(first);
+  });
+
+  test("returns undefined when no existing attachment shares the hash", () => {
+    expect(findDuplicateAttachment([first], other)).toBeUndefined();
+  });
+});

src/tui/image-attachments.ts

@@ -27,10 +27,18 @@ const IMAGE_MIME_BY_EXT: Readonly<Record<string, string>> = {
 export type PendingImageAttachment = MessageAttachment & {
   id: string;
   path?: string;
-  /** SHA-256 of the source image file's bytes, used to dedupe repeat pastes. */
+  /** SHA-256 of the source image file's bytes, used to identify identical images. */
   contentHash: string;
 };
 
+/** First existing attachment whose `contentHash` matches the candidate, if any. */
+export function findDuplicateAttachment(
+  existing: readonly PendingImageAttachment[],
+  candidate: PendingImageAttachment,
+): PendingImageAttachment | undefined {
+  return existing.find((attachment) => attachment.contentHash === candidate.contentHash);
+}
+
 export type AttachImageResult =
   { ok: true; attachment: PendingImageAttachment } | { ok: false; reason: string };
 
@@ -118,7 +126,7 @@ export async function imageAttachmentFromPath(path: string): Promise<AttachImage
   };
 }
 
-/** SHA-256 of the source image file's bytes, used to identify identical pastes regardless of filename or timing. */
+/** SHA-256 of the source image file's bytes, used to identify identical images regardless of filename or timing. */
 async function hashImageBytes(bytes: Buffer): Promise<string> {
   // Buffer's type parameter is the looser ArrayBufferLike (it may back onto a
   // pooled allocation), but readFile never actually hands back a

src/tui/prompt-attachments.test.ts

@@ -42,6 +42,24 @@ describe("ingestPathMentions", () => {
     expect(result.text).toBe("./shot.png");
     expect(result.attachments).toEqual([]);
   });
+
+  test("two mentions of the same bytes keep one attachment and rewrite both tokens to its name", async () => {
+    const load = async (path: string): Promise<AttachImageResult> => ({
+      ok: true,
+      attachment: {
+        id: path,
+        name: path.endsWith("alias.png") ? "alias.png" : "shot.png",
+        contentType: "image/png",
+        data: new Uint8Array([1, 2, 3]),
+        path,
+        contentHash: "same-bytes",
+      },
+    });
+    const result = await ingestPathMentions("see ./shot.png and ./alias.png", "/repo", load);
+    expect(result.attachments).toHaveLength(1);
+    expect(result.attachments[0]?.name).toBe("shot.png");
+    expect(result.text).toBe("see [Attached image: shot.png] and [Attached image: shot.png]");
+  });
 });
 
 describe("ingestOperatorPrompt", () => {
@@ -66,6 +84,19 @@ describe("ingestOperatorPrompt", () => {
     expect(result.text).toBe("just words");
     expect(result.attachments).toEqual([]);
   });
+
+  test("a path mention matching a pending clipboard hash keeps the pending attachment", async () => {
+    const pending = attachment("clipboard.png");
+    const pendingHash = pending.contentHash;
+    const load = async (path: string): Promise<AttachImageResult> => ({
+      ok: true,
+      attachment: { ...attachment("shot.png"), path, contentHash: pendingHash },
+    });
+    const result = await ingestOperatorPrompt("see ./shot.png", "/repo", load, [pending]);
+    expect(result.attachments).toHaveLength(1);
+    expect(result.attachments[0]).toEqual(pending);
+    expect(result.text).toBe("see [Attached image: clipboard.png]");
+  });
 });
 
 describe("spliceMentionCompletion", () => {

src/tui/prompt-attachments.ts

@@ -5,6 +5,7 @@
  */
 
 import {
+  findDuplicateAttachment,
   findImagePathMentions,
   type AttachImageResult,
   type PendingImageAttachment,
@@ -20,13 +21,15 @@ export interface PathMentionIngestion {
 
 /**
  * Replace image paths written inline in the prompt with attachment markers and
- * return the loaded attachments. `load` is injected so this stays testable
- * without touching the filesystem.
+ * return only attachments whose content hash is not already in `pending` or
+ * earlier in this batch. Duplicate tokens still rewrite to the kept name.
+ * `load` is injected so this stays testable without touching the filesystem.
  */
 export async function ingestPathMentions(
   text: string,
   cwd: string,
   load: (path: string) => Promise<AttachImageResult>,
+  pending: readonly PendingImageAttachment[] = [],
 ): Promise<PathMentionIngestion> {
   const mentions = findImagePathMentions(text, cwd);
   if (mentions.length === 0) return { text, attachments: [] };
@@ -37,8 +40,12 @@ export async function ingestPathMentions(
   for (const [index, mention] of mentions.entries()) {
     const result = loaded[index];
     if (result === undefined || !result.ok) continue;
-    attachments.push(result.attachment);
-    out = out.replace(mention.raw, `[Attached image: ${result.attachment.name}]`);
+    const kept =
+      findDuplicateAttachment(pending, result.attachment) ??
+      findDuplicateAttachment(attachments, result.attachment) ??
+      result.attachment;
+    if (kept === result.attachment) attachments.push(result.attachment);
+    out = out.replace(mention.raw, `[Attached image: ${kept.name}]`);
   }
   return { text: out, attachments };
 }
@@ -53,7 +60,7 @@ export async function ingestOperatorPrompt(
   load: (path: string) => Promise<AttachImageResult>,
   pending: readonly PendingImageAttachment[] = [],
 ): Promise<PathMentionIngestion> {
-  const ingested = await ingestPathMentions(text, cwd, load);
+  const ingested = await ingestPathMentions(text, cwd, load, pending);
   const resolved = await resolveAtMentions(ingested.text, cwd);
   return { text: resolved, attachments: [...pending, ...ingested.attachments] };
 }

src/tui/shell.ts

@@ -36,6 +36,7 @@ import { sliceTailToWidth, sliceToWidth, stringWidth } from "./view/height.js";
 import { listPathSuggestions } from "./components/at-mention/list.js";
 import { parseAtState, type AtState } from "./components/at-mention/parse.js";
 import {
+  findDuplicateAttachment,
   formatAttachmentSummary,
   readClipboardImage,
   type ClipboardImageResult,
@@ -990,9 +991,7 @@ export async function attachClipboardImage(shell: AppShell): Promise<boolean> {
     });
     return false;
   }
-  const duplicate = shell.pendingAttachments.find(
-    (attachment) => attachment.contentHash === result.attachment.contentHash,
-  );
+  const duplicate = findDuplicateAttachment(shell.pendingAttachments, result.attachment);
   if (duplicate !== undefined) {
     setStatusFlash(shell, `${duplicate.name} is already attached`, {
       ttlMs: RUNTIME_FLASH_MS,