Răsfoiți Sursa

perf: skip redundant fs work in tui-state, image-hook and board injection

- tui-state: memoize the last confirmed snapshot per project, validated
  by file identity (ino, mtime, size); identical message.updated events
  return before the inter-process lock, and only confirmed persistences
  seed the memo so failed writes are retried, not swallowed
- image-hook: memoize resolved data-URL attachments (keyed by target
  dir, effective filename and a sha256 of the URL, never the raw
  payload) so historical images are not re-decoded and re-hashed on
  every transform; hits revalidate with lstat and refuse symlinks
- board-injection: compute promptShapeKey lazily, only when a consumed
  terminal job needs reconciliation or a new terminal delivery must be
  registered
dhaern 1 săptămână în urmă
părinte
comite
dee0eeba2a

+ 102 - 1
src/hooks/image-hook.test.ts

@@ -1,4 +1,4 @@
-import { afterAll, describe, expect, it } from 'bun:test';
+import { afterAll, describe, expect, it, spyOn } from 'bun:test';
 import { spawnSync } from 'node:child_process';
 import { createHash } from 'node:crypto';
 import {
@@ -73,6 +73,20 @@ function imagePartCount(message: MessageWithParts): number {
   return message.parts.filter((part) => part.type === 'image').length;
 }
 
+const AUTO = {
+  imageRouting: 'auto' as const,
+  disabledAgents: new Set<string>(),
+  log: () => {},
+};
+
+function processAuto(messages: MessageWithParts[], workDir: string): boolean {
+  return processImageAttachments({ messages, workDir, ...AUTO });
+}
+
+function nudgeText(message: MessageWithParts): string {
+  return message.parts.find((part) => part.type === 'text')?.text ?? '';
+}
+
 afterAll(() => {
   rmSync(TEST_DIR, { recursive: true, force: true });
 });
@@ -713,6 +727,93 @@ describe('processImageAttachments image routing', () => {
     expect(imagePartCount(saved)).toBe(0);
   });
 
+  it('historical attachments reuse the memoized path without re-decoding', async () => {
+    const { workDir } = makeTestDir('memo-reuse');
+    const first = makeUserMsg([IMG]);
+    processAuto([first], workDir);
+    const marker = nudgeText(first).match(
+      /(\/[^\s,]+image-[0-9a-f]{8}\.png)/,
+    )?.[1];
+    expect(marker).toBeDefined();
+    const fromSpy = spyOn((await import('node:buffer')).Buffer, 'from');
+    try {
+      const second = makeUserMsg([IMG]);
+      processAuto([second], workDir);
+      expect(nudgeText(second)).toContain(marker as string);
+      expect(fromSpy).not.toHaveBeenCalled();
+    } finally {
+      fromSpy.mockRestore();
+    }
+  });
+
+  it('memoized attachment falls back to re-saving after deletion', () => {
+    const { workDir, saveDir } = makeTestDir('memo-evict');
+    processAuto([makeUserMsg([IMG])], workDir);
+    for (const entry of readdirSync(saveDir, { withFileTypes: true })) {
+      if (entry.isDirectory()) {
+        rmSync(path.join(saveDir, entry.name), {
+          recursive: true,
+          force: true,
+        });
+      }
+    }
+    const second = makeUserMsg([IMG]);
+    processAuto([second], workDir);
+    expect(imagePartCount(second)).toBe(0);
+    expect(nudgeText(second)).toContain('image-');
+  });
+
+  it('same payload with different filenames resolves to different memo entries', () => {
+    const { workDir } = makeTestDir('memo-filename');
+    const before = makeUserMsg([{ type: 'image', url: IMG.url }]);
+    const after = makeUserMsg([
+      { type: 'image', url: IMG.url, filename: 'after.png' },
+    ]);
+    processAuto([before], workDir);
+    processAuto([after], workDir);
+    expect(nudgeText(before)).toContain('image-');
+    expect(nudgeText(after)).toContain('after-');
+  });
+
+  it('suffixed resolution is not memoized: canonical path returns when obstacle is gone', () => {
+    const { workDir } = makeTestDir('memo-suffix');
+    const sessionDir = path.join(workDir, '.opencode', 'images', 's1');
+    mkdirSync(sessionDir, { recursive: true });
+    const canonical = path.join(sessionDir, IMG_CONTENT_NAME);
+    const external = path.join(TEST_DIR, 'memo-suffix-external.bin');
+    writeFileSync(external, 'external');
+    symlinkSync(external, canonical);
+    const first = makeUserMsg([IMG]);
+    processAuto([first], workDir);
+    expect(nudgeText(first)).toContain(`image-${IMG_HASH}-1.png`);
+    rmSync(canonical);
+    const second = makeUserMsg([IMG]);
+    processAuto([second], workDir);
+    expect(nudgeText(second)).toContain(IMG_CONTENT_NAME);
+    expect(nudgeText(second)).not.toContain(`image-${IMG_HASH}-1.png`);
+  });
+
+  it('memo hit does not reuse a path replaced by a symlink', () => {
+    const { workDir } = makeTestDir('memo-symlink');
+    const first = makeUserMsg([IMG]);
+    processAuto([first], workDir);
+    const savedPath = nudgeText(first).match(
+      /(\/[^\s,]+image-[0-9a-f]{8}\.png)/,
+    )?.[1];
+    expect(savedPath).toBeDefined();
+    const external = path.join(TEST_DIR, 'memo-symlink-external.bin');
+    writeFileSync(external, 'external');
+    rmSync(savedPath as string);
+    symlinkSync(external, savedPath as string);
+    const second = makeUserMsg([IMG]);
+    processAuto([second], workDir);
+    const text = nudgeText(second);
+    expect(text).not.toContain(savedPath as string);
+    expect(text).toContain('image-');
+    expect(lstatSync(savedPath as string).isSymbolicLink()).toBe(true);
+    expect(readFileSync(external, 'utf8')).toBe('external');
+  });
+
   it('ignores non-user messages and non-image parts', () => {
     const userText = makeUserMsg([{ type: 'text', text: 'hello' }]);
     const assistant = {

+ 80 - 9
src/hooks/image-hook.ts

@@ -222,6 +222,60 @@ function isImagePart(p: ImagePart): boolean {
   return false;
 }
 
+// Memo of already-materialized data-URL attachments. Key is
+// targetDir + effective filename + sha256(url) — never the raw base64.
+// Suffixed collision names (`-N`) are not stored: the obstacle may be
+// gone next transform. Hits revalidate with lstat (regular file only).
+const resolvedAttachmentByKey = new Map<string, string>();
+const RESOLVED_ATTACHMENT_MAX = 256;
+
+function attachmentMemoKey(
+  targetDir: string,
+  dataUrl: string,
+  effectiveName: string,
+): string {
+  return `${targetDir}\n${effectiveName}\n${createHash('sha256').update(dataUrl).digest('hex')}`;
+}
+
+function isSuffixedResolution(filePath: string): boolean {
+  return /-[0-9a-f]{8}-\d+\.[^.]+$/.test(basename(filePath));
+}
+
+function rememberResolvedAttachment(
+  targetDir: string,
+  dataUrl: string,
+  effectiveName: string,
+  filePath: string,
+): void {
+  if (isSuffixedResolution(filePath)) return;
+  const key = attachmentMemoKey(targetDir, dataUrl, effectiveName);
+  if (
+    !resolvedAttachmentByKey.has(key) &&
+    resolvedAttachmentByKey.size >= RESOLVED_ATTACHMENT_MAX
+  ) {
+    const oldest = resolvedAttachmentByKey.keys().next().value;
+    if (oldest !== undefined) resolvedAttachmentByKey.delete(oldest);
+  }
+  resolvedAttachmentByKey.set(key, filePath);
+}
+
+function recalledResolvedAttachment(
+  targetDir: string,
+  dataUrl: string,
+  effectiveName: string,
+): string | null {
+  const key = attachmentMemoKey(targetDir, dataUrl, effectiveName);
+  const saved = resolvedAttachmentByKey.get(key);
+  if (!saved) return null;
+  try {
+    if (lstatSync(saved).isFile()) return saved;
+  } catch {
+    // gone
+  }
+  resolvedAttachmentByKey.delete(key);
+  return null;
+}
+
 function decodeDataUrl(url: string): { mime: string; data: Buffer } | null {
   const match = url.match(/^data:([^;]+);base64,(.+)$/);
   if (!match) return null;
@@ -240,6 +294,11 @@ function extFromMime(mime: string): string {
   return map[mime] ?? '.png';
 }
 
+function extFromMimeFromUrl(url: string): string {
+  const match = url.match(/^data:([^;,]+)/);
+  return match ? extFromMime(match[1]) : '.png';
+}
+
 function sanitizeFilename(name: string): string {
   return name.replace(/[^a-zA-Z0-9._-]/g, '_');
 }
@@ -495,26 +554,38 @@ export function processImageAttachments(args: {
       const filename =
         (p.filename as string | undefined) ?? (p.name as string | undefined);
       if (url) {
+        const sanitizedFilename = filename
+          ? sanitizeFilename(filename)
+          : undefined;
+        const baseName = sanitizedFilename
+          ? sanitizedFilename.replace(/\.[^.]+$/, '') || 'image'
+          : 'image';
+        const ext = sanitizedFilename
+          ? extname(sanitizedFilename) || extFromMimeFromUrl(url)
+          : extFromMimeFromUrl(url);
+        const effectiveName = `${baseName}-${ext}`;
+        const recalled = recalledResolvedAttachment(
+          targetDir,
+          url,
+          effectiveName,
+        );
+        if (recalled) {
+          savedPaths.push(recalled);
+          savedImageParts.add(p);
+          continue;
+        }
         const decoded = decodeDataUrl(url);
         if (decoded) {
           const hash = createHash('sha1')
             .update(decoded.data)
             .digest('hex')
             .slice(0, 8);
-          const sanitizedFilename = filename
-            ? sanitizeFilename(filename)
-            : undefined;
-          const baseName = sanitizedFilename
-            ? sanitizedFilename.replace(/\.[^.]+$/, '') || 'image'
-            : 'image';
-          const ext = sanitizedFilename
-            ? extname(sanitizedFilename) || extFromMime(decoded.mime)
-            : extFromMime(decoded.mime);
           const name = `${baseName}-${hash}${ext}`;
           const filePath = writeUniqueFile(targetDir, name, decoded.data, log);
           if (filePath) {
             savedPaths.push(filePath);
             savedImageParts.add(p);
+            rememberResolvedAttachment(targetDir, url, effectiveName, filePath);
           }
         }
       }

+ 15 - 8
src/hooks/task-session-manager/board-injection.ts

@@ -1183,8 +1183,13 @@ function injectLatestBoard(state: InjectionState, messages: unknown[]): void {
   if (!sessionID || !state.shouldManageSession(sessionID)) return;
   if (!anchor) return;
 
-  const shapeKey = promptShapeKey(realMessages(messages, state.metadataKey));
-  reconcileConsumedTerminalJobs(state, sessionID, shapeKey);
+  // Hash the real history only when a prior terminal delivery needs
+  // reconciliation or a new one is about to be registered.
+  let shapeKey: string | undefined;
+  if (state.terminalJobsInjectedByParent.has(sessionID)) {
+    shapeKey = promptShapeKey(realMessages(messages, state.metadataKey));
+    reconcileConsumedTerminalJobs(state, sessionID, shapeKey);
+  }
 
   const boardMeta =
     state.backgroundJobBoard.formatForPromptWithMetadata(sessionID);
@@ -1196,12 +1201,14 @@ function injectLatestBoard(state: InjectionState, messages: unknown[]): void {
   );
   if (!textPart || isInternalInitiatorPart(textPart)) return;
 
-  rememberInjectedTerminalJobs(
-    state,
-    sessionID,
-    boardMeta.terminalUnreconciledTaskIDs,
-    shapeKey,
-  );
+  if (boardMeta.terminalUnreconciledTaskIDs.length > 0) {
+    rememberInjectedTerminalJobs(
+      state,
+      sessionID,
+      boardMeta.terminalUnreconciledTaskIDs,
+      shapeKey ?? promptShapeKey(realMessages(messages, state.metadataKey)),
+    );
+  }
 
   // Placement rules — correctness first, then prompt-cache safety.
   //

+ 110 - 9
src/tui-state.test.ts

@@ -30,6 +30,13 @@ afterEach(() => {
   fs.rmSync(tempDir, { recursive: true, force: true });
 });
 
+const LUNA = { agentName: 'explorer', model: 'openai/gpt-5.6-luna' } as const;
+const GPT = { agentName: 'explorer', model: 'openai/gpt-5.6' } as const;
+
+function recordLuna(): void {
+  recordTuiAgentModel(LUNA, tempDir);
+}
+
 describe('tui-state persistence', () => {
   test('persists enabled agent models', () => {
     recordTuiAgentModels(
@@ -320,24 +327,118 @@ describe('tui-state persistence', () => {
   });
 
   test('skips the disk write when the recorded value is unchanged', () => {
-    recordTuiAgentModel(
-      { agentName: 'explorer', model: 'openai/gpt-5.6-luna' },
-      tempDir,
-    );
-
+    recordLuna();
     const filePath = getTuiStatePath(tempDir);
     const oldMtime = new Date('2000-01-01T00:00:00Z');
     fs.utimesSync(filePath, oldMtime, oldMtime);
     const baselineMtime = fs.statSync(filePath).mtimeMs;
+    recordLuna();
+    expect(fs.statSync(filePath).mtimeMs).toBe(baselineMtime);
+  });
+
+  test('repeated no-op updates do not touch the lock or the filesystem', async () => {
+    recordLuna();
+    const fsModule = await import('node:fs');
+    let openCalls = 0;
+    let readCalls = 0;
+    const lockCreateSpy = spyOn(fsModule, 'openSync').mockImplementation(
+      (...args: Parameters<typeof fs.openSync>) => {
+        openCalls += 1;
+        return fs.openSync(...args);
+      },
+    );
+    const readSpy = spyOn(fsModule, 'readFileSync').mockImplementation(
+      (...args: Parameters<typeof fs.readFileSync>) => {
+        readCalls += 1;
+        return fs.readFileSync(...args);
+      },
+    );
+    try {
+      recordLuna();
+      recordLuna();
+      expect(openCalls).toBe(0);
+      expect(readCalls).toBe(0);
+    } finally {
+      lockCreateSpy.mockRestore();
+      readSpy.mockRestore();
+    }
+  });
+
+  test('a real change after a no-op still reaches the disk', () => {
+    recordLuna();
+    recordLuna();
+    recordTuiAgentModel(GPT, tempDir);
+    expect(readTuiSnapshot(tempDir).agentModels.explorer).toBe(GPT.model);
+  });
 
-    // Same agent, same model: nothing changed, so the file must not be
-    // rewritten (a write would bump the mtime from 2000 back to "now").
+  test('external snapshot changes are not masked by the memo', () => {
+    recordLuna();
+    const external = readTuiSnapshot(tempDir);
+    external.agentModels.builder = GPT.model;
+    fs.writeFileSync(getTuiStatePath(tempDir), `${JSON.stringify(external)}\n`);
     recordTuiAgentModel(
-      { agentName: 'explorer', model: 'openai/gpt-5.6-luna' },
+      { agentName: 'build', model: 'anthropic/claude-x' },
       tempDir,
     );
+    const snapshot = readTuiSnapshot(tempDir);
+    expect(snapshot.agentModels.builder).toBe(GPT.model);
+    expect(snapshot.agentModels.build).toBe('anthropic/claude-x');
+  });
 
-    expect(fs.statSync(filePath).mtimeMs).toBe(baselineMtime);
+  test('a failed persistence is retried, not swallowed by the memo', async () => {
+    recordLuna();
+    const fsModule = await import('node:fs');
+    const renameSpy = spyOn(fsModule, 'renameSync').mockImplementation(() => {
+      throw new Error('disk full');
+    });
+    try {
+      recordTuiAgentModel(GPT, tempDir);
+    } finally {
+      renameSpy.mockRestore();
+    }
+    recordTuiAgentModel(GPT, tempDir);
+    expect(readTuiSnapshot(tempDir).agentModels.explorer).toBe(GPT.model);
+  });
+
+  test('external atomic replacement invalidates the memo even with identical mtime and size', () => {
+    recordTuiAgentModel({ agentName: 'explorer', model: 'model-x' }, tempDir);
+    const filePath = getTuiStatePath(tempDir);
+    const external = readTuiSnapshot(tempDir);
+    external.agentModels.explorer = 'model-y';
+    const tmpPath = `${filePath}.external.tmp`;
+    fs.writeFileSync(tmpPath, `${JSON.stringify(external)}\n`);
+    const stat = fs.statSync(filePath);
+    fs.renameSync(tmpPath, filePath);
+    fs.utimesSync(filePath, stat.atime, stat.mtime);
+    recordTuiAgentModel({ agentName: 'explorer', model: 'model-x' }, tempDir);
+    expect(readTuiSnapshot(tempDir).agentModels.explorer).toBe('model-x');
+  });
+
+  test('a transient read failure does not seed the memo with an empty snapshot', async () => {
+    recordTuiAgentActivity(
+      { sessionID: 's1', agentName: 'oracle', active: true },
+      tempDir,
+    );
+    const filePath = getTuiStatePath(tempDir);
+    const fsModule = await import('node:fs');
+    const originalRead = fsModule.readFileSync;
+    const readSpy = spyOn(fsModule, 'readFileSync').mockImplementation(
+      (path: fs.PathOrFileDescriptor, ...args: unknown[]) => {
+        if (String(path) === filePath) {
+          const err = new Error('transient') as NodeJS.ErrnoException;
+          err.code = 'EACCES';
+          throw err;
+        }
+        return originalRead(path as fs.PathOrFileDescriptor, ...(args as []));
+      },
+    );
+    try {
+      recordTuiAgentActivity({ sessionID: 's1', active: false }, tempDir);
+    } finally {
+      readSpy.mockRestore();
+    }
+    recordTuiAgentActivity({ sessionID: 's1', active: false }, tempDir);
+    expect(readTuiSnapshot(tempDir).activeSessions).toEqual({});
   });
 
   test('keeps the final file intact when the atomic rename fails', async () => {

+ 105 - 12
src/tui-state.ts

@@ -80,6 +80,19 @@ export function readTuiSnapshot(projectDir: string): TuiSnapshot {
   }
 }
 
+// Locked-path reader: ENOENT is a first write; any other error must not
+// seed the memo (a fallback empty snapshot would swallow later retries).
+function readTuiSnapshotStrict(statePath: string): TuiSnapshot | null {
+  try {
+    return parseSnapshot(fs.readFileSync(statePath, 'utf8'));
+  } catch (error) {
+    if ((error as NodeJS.ErrnoException).code === 'ENOENT') {
+      return emptySnapshot();
+    }
+    return null;
+  }
+}
+
 export async function readTuiSnapshotAsync(
   projectDir: string,
 ): Promise<TuiSnapshot> {
@@ -92,7 +105,7 @@ export async function readTuiSnapshotAsync(
   }
 }
 
-function writeTuiSnapshot(snapshot: TuiSnapshot, projectDir: string): void {
+function writeTuiSnapshot(snapshot: TuiSnapshot, projectDir: string): boolean {
   try {
     const filePath = getTuiStatePath(projectDir);
     fs.mkdirSync(path.dirname(filePath), { recursive: true });
@@ -109,8 +122,10 @@ function writeTuiSnapshot(snapshot: TuiSnapshot, projectDir: string): void {
         // best-effort
       }
     }
+    return true;
   } catch {
     // TUI state is best-effort only.
+    return false;
   }
 }
 
@@ -194,11 +209,91 @@ function releaseStateLock(lock: TuiStateLock): void {
   }
 }
 
+// Last confirmed on-disk snapshot per project, keyed by identity
+// (ino,mtime,size). No-ops return before the lock; failed writes do not
+// seed the memo. An identity mismatch (external rename) invalidates it.
+const lastKnownSnapshots = new Map<
+  string,
+  { snapshot: TuiSnapshot; ino: number; mtimeMs: number; size: number }
+>();
+const LAST_KNOWN_SNAPSHOTS_MAX = 32;
+
+function statSnapshotFile(statePath: string): {
+  ino: number;
+  mtimeMs: number;
+  size: number;
+} | null {
+  try {
+    const stat = fs.statSync(statePath);
+    return { ino: stat.ino, mtimeMs: stat.mtimeMs, size: stat.size };
+  } catch {
+    return null;
+  }
+}
+
+function cloneSnapshot(snapshot: TuiSnapshot): TuiSnapshot {
+  return {
+    version: snapshot.version,
+    updatedAt: snapshot.updatedAt,
+    agentModels: { ...snapshot.agentModels },
+    agentVariants: { ...snapshot.agentVariants },
+    activeSessions: { ...snapshot.activeSessions },
+  };
+}
+
+function snapshotSectionsEqual(a: TuiSnapshot, b: TuiSnapshot): boolean {
+  return (
+    JSON.stringify(a.agentModels) === JSON.stringify(b.agentModels) &&
+    JSON.stringify(a.agentVariants) === JSON.stringify(b.agentVariants) &&
+    JSON.stringify(a.activeSessions) === JSON.stringify(b.activeSessions)
+  );
+}
+
+function rememberSnapshot(statePath: string, snapshot: TuiSnapshot): void {
+  const stat = statSnapshotFile(statePath);
+  if (!stat) {
+    lastKnownSnapshots.delete(statePath);
+    return;
+  }
+  if (
+    !lastKnownSnapshots.has(statePath) &&
+    lastKnownSnapshots.size >= LAST_KNOWN_SNAPSHOTS_MAX
+  ) {
+    const oldest = lastKnownSnapshots.keys().next().value;
+    if (oldest !== undefined) lastKnownSnapshots.delete(oldest);
+  }
+  lastKnownSnapshots.set(statePath, { snapshot, ...stat });
+}
+
+function memoFor(statePath: string): TuiSnapshot | undefined {
+  const entry = lastKnownSnapshots.get(statePath);
+  if (!entry) return undefined;
+  const stat = statSnapshotFile(statePath);
+  if (
+    !stat ||
+    stat.ino !== entry.ino ||
+    stat.mtimeMs !== entry.mtimeMs ||
+    stat.size !== entry.size
+  ) {
+    lastKnownSnapshots.delete(statePath);
+    return undefined;
+  }
+  return entry.snapshot;
+}
+
 function updateSnapshot(
   projectDir: string,
   mutator: (snapshot: TuiSnapshot) => void,
 ): void {
   const statePath = getTuiStatePath(projectDir);
+
+  const memo = memoFor(statePath);
+  if (memo) {
+    const candidate = cloneSnapshot(memo);
+    mutator(candidate);
+    if (snapshotSectionsEqual(candidate, memo)) return; // no-op update
+  }
+
   try {
     fs.mkdirSync(path.dirname(statePath), { recursive: true });
   } catch {
@@ -208,20 +303,18 @@ function updateSnapshot(
   if (!lock) return;
 
   try {
-    const snapshot = readTuiSnapshot(projectDir);
-    const beforeModels = JSON.stringify(snapshot.agentModels);
-    const beforeVariants = JSON.stringify(snapshot.agentVariants);
-    const beforeActiveSessions = JSON.stringify(snapshot.activeSessions);
+    const snapshot = readTuiSnapshotStrict(statePath);
+    if (!snapshot) return;
+    const before = cloneSnapshot(snapshot);
     mutator(snapshot);
-    if (
-      JSON.stringify(snapshot.agentModels) === beforeModels &&
-      JSON.stringify(snapshot.agentVariants) === beforeVariants &&
-      JSON.stringify(snapshot.activeSessions) === beforeActiveSessions
-    ) {
-      return; // state unchanged — skip the disk write
+    if (snapshotSectionsEqual(snapshot, before)) {
+      rememberSnapshot(statePath, snapshot);
+      return;
     }
     snapshot.updatedAt = Date.now();
-    writeTuiSnapshot(snapshot, projectDir);
+    if (writeTuiSnapshot(snapshot, projectDir)) {
+      rememberSnapshot(statePath, snapshot);
+    }
   } finally {
     releaseStateLock(lock);
   }