Phase 8: Real-world accuracy test against tj/commander.js

TESTING.md

@@ -0,0 +1,36 @@
+# Real-world accuracy test
+
+This documents a real-world test of seal against a fork of [tj/commander.js](https://github.com/tj/commander.js) (a real, actively-maintained, well-documented JavaScript library — chosen over the originally-suggested FastAPI/Pydantic for its much smaller size, to keep the test's embedding-API cost and runtime tractable on a free-tier key).
+
+## Methodology
+
+1. Forked `tj/commander.js` to `dakshcodez/commander.js` and cloned it locally.
+2. Ran `seal index` for real (live Gemini API, `gemini-3.7-flash`/`gemini-embedding-2` for indexing, `gemini-3.5-flash-lite` for the staleness/repair calls during this test session specifically to work around `3.7-flash`'s demand-driven `503`s — the shipped default remains `3.7-flash`). Indexed the full repo: **232 code chunks, 365 doc sections** (before the changelog-exclusion fix described below), producing 194 links (178 heuristic, 16 embedding).
+3. Made three deliberate, targeted code changes and staged them, then ran `seal check` for real against the live pipeline.
+4. Inspected the actual generated corrections and verdicts against expectations.
+
+## Deliberate test cases
+
+| # | Change | Expected | Actual result |
+|---|---|---|---|
+| 1 | Renamed `Command.prototype.name()` → `.setName()` in `lib/command.js` and `typings/index.d.ts` (breaking signature change, docs untouched) | Flag `Readme.md`'s `.name` section as stale | **Correctly flagged and auto-fixed** — `Readme.md#Commander.js > Automated help > .name` |
+| 2 | Added an explanatory comment inside `.help()`'s body, no logic change | Not flagged | **Correctly not flagged** — filtered out before ever reaching the LLM (AST-based comment stripping) |
+| 3 | Rewrote the internal, undocumented `_getCommandAndAncestors()` helper's loop style (`for` → `while`), same behavior, zero doc links | Not flagged | **Correctly produced zero suspects** — no doc section is linked to it at all |
+| 4 (unplanned) | Same rename from #1 also touched a second, more ambiguous section (`Bits and pieces > Legacy options as properties`) referencing `.name()` in a way that was only partly affected | Ambiguous | The generation pass proposed a correction; the **validation pass caught that it was actually wrong** (the typings still exposed a valid `.name()` getter alongside the new `.setName()` setter) and downgraded it to "needs review" instead of auto-applying — the two-pass generate→validate design working exactly as intended |
+
+## Bugs found and fixed as a direct result of this test
+
+Live testing against a real repository surfaced three real bugs that no amount of mocked testing had caught:
+
+1. **Indexing crashed outright** on a real repo: two locally-scoped test helper functions named `makeProgram` in different `describe()` blocks of the same test file produced identical chunk IDs, and Vectra's `insertItem` throws on a duplicate ID. Fixed two ways: test files are now excluded from code parsing entirely (they were never a useful doc-linking target anyway), and the embedding index now upserts rather than inserts, so any remaining ID collision degrades gracefully instead of crashing.
+2. **No retry/backoff on rate limits.** The free tier's per-minute quotas (100 embed requests/min, 15 generate requests/min for `gemini-3.5-flash-lite`) were hit repeatedly during real indexing and staleness-checking, and every `429`/`503` was previously fatal. Added retry-with-exponential-backoff (up to 4 attempts) to `GeminiClient`.
+3. **Changelog files were incorrectly treated as living documentation.** `CHANGELOG.md` entries describing historical releases (e.g. a 2020 entry for `.parseOption()`) got "corrected" to reflect current code state — rewriting history, not fixing stale docs. Fixed by excluding changelog-style files (`CHANGELOG.md`, `CHANGES.md`, `HISTORY.md`, `RELEASES.md`, case-insensitive) from doc parsing entirely, the same way test files are excluded from code parsing.
+
+Also observed, not fixed: the correction pass occasionally touched unrelated formatting in the same file (removed a blank line after a few `### Added` headings in `CHANGELOG.md` before that file was excluded) despite being instructed to preserve untouched content — a minor prompt-adherence gap worth revisiting.
+
+## Known limitations of this test
+
+- The changelog-exclusion fix (bug #3 above) is verified at the unit level (the file-matching pattern was tested directly against 7 real-world filename cases, all correct) but **not re-validated end-to-end against the live API** — the free tier's *daily* embedding quota (1000 requests/day) was exhausted while re-indexing to confirm it, and daily quotas don't reset within a session. A full end-to-end re-run is a natural follow-up once quota is available.
+- This test exercised the `@seal/cli` path (`seal index` + `seal check`) only, not the GitHub Action — the Action's `dist/index.js` isn't bundled for standalone execution yet (see the publish-phase task).
+- One real repository, one language (JavaScript), a handful of deliberate cases — not a statistically rigorous accuracy benchmark, but real signal from real code and real docs rather than synthetic fixtures.
+- The new `isTestFile`/`isChangelogFile` exclusions are unconditional — there's no way to opt a specific file back in short of renaming it. `.sealignore` exists as a user-facing override mechanism, but it's currently only wired into `check.ts`'s changed-file filtering, not into the doc/code indexing walk itself (a pre-existing gap, not introduced by this fix). A team that genuinely wants a file named `HISTORY.md` treated as living documentation has no escape hatch today.

packages/core/src/changes/filter.ts

@@ -1,13 +1,10 @@
 import type { Node } from 'web-tree-sitter';
 import type { GrammarId } from '../parsing/index.js';
 import { createParser, resolveGrammar } from '../parsing/index.js';
+import { isTestFile } from '../shared/is-test-file.js';
 import type { ChunkDiff } from './types.js';
 
-const TEST_FILE_PATTERN = /(^|\/)(__tests__|tests?)\/|\.(test|spec)\.[^/]+$/;
-
-export function isTestFile(filePath: string): boolean {
-  return TEST_FILE_PATTERN.test(filePath);
-}
+export { isTestFile };
 
 function collectCommentRanges(node: Node, ranges: [number, number][]): void {
   if (node.type === 'comment') {

packages/core/src/docs/file-walker.ts

@@ -1,11 +1,18 @@
 import { extname } from 'node:path';
 import { walkFiles } from '../shared/walk-files.js';
 
+const CHANGELOG_FILE_PATTERN = /(^|\/)(changelog|changes|history|releases)\.md$/i;
+
 export interface DocFile {
   absolutePath: string;
   relativePath: string;
 }
 
+export function isChangelogFile(filePath: string): boolean {
+  return CHANGELOG_FILE_PATTERN.test(filePath);
+}
+
 export async function walkDocFiles(rootDir: string): Promise<DocFile[]> {
-  return walkFiles(rootDir, (name) => extname(name) === '.md');
+  const files = await walkFiles(rootDir, (name) => extname(name) === '.md');
+  return files.filter((file) => !isChangelogFile(file.relativePath));
 }

packages/core/src/docs/index.ts

@@ -3,5 +3,5 @@ export { applySectionCorrection } from './apply-correction.js';
 export { extractCodeReferences } from './code-references.js';
 export { parseMarkdownSections } from './markdown-parser.js';
 export { parseDocs } from './parser.js';
-export { walkDocFiles } from './file-walker.js';
+export { isChangelogFile, walkDocFiles } from './file-walker.js';
 export type { DocFile } from './file-walker.js';

packages/core/src/index.ts

@@ -10,6 +10,7 @@ export type { DocFile, DocSection } from './docs/index.js';
 export {
   applySectionCorrection,
   extractCodeReferences,
+  isChangelogFile,
   parseDocs,
   parseMarkdownSections,
   walkDocFiles,

packages/core/src/linkgraph/embedding.ts

@@ -36,9 +36,19 @@ export async function buildEmbeddingLinks(
   }
   await index.createIndex({ version: 1 });
 
+  const seenChunkIds = new Set<string>();
   for (const chunk of chunks) {
     const vector = await llm.embed(chunkEmbeddingText(chunk));
-    await index.insertItem({ id: chunk.id, vector });
+    // upsert, not insert: two chunks can legitimately share an id (e.g.
+    // same-named locally-scoped helpers in different function bodies) -
+    // that's an inherent limit of a name-based id scheme, not a fatal error.
+    // Still surfaced as a warning, since it means one of the two silently
+    // becomes unlinkable rather than crashing loudly like insertItem did.
+    if (seenChunkIds.has(chunk.id)) {
+      console.warn(`seal: duplicate chunk id "${chunk.id}" - only the last one is linkable in the embedding index.`);
+    }
+    seenChunkIds.add(chunk.id);
+    await index.upsertItem({ id: chunk.id, vector });
   }
 
   const links: Link[] = [];

packages/core/src/llm/gemini-client.ts

@@ -1,4 +1,4 @@
-import { GoogleGenAI } from '@google/genai';
+import { ApiError, GoogleGenAI } from '@google/genai';
 import type { CompleteOptions, LLMClient } from './types.js';
 
 export interface GeminiClientOptions {
@@ -10,6 +10,38 @@ export interface GeminiClientOptions {
 const DEFAULT_GENERATION_MODEL = 'gemini-3.7-flash';
 const DEFAULT_EMBEDDING_MODEL = 'gemini-embedding-2';
 
+const MAX_RETRIES = 4;
+const INITIAL_BACKOFF_MS = 1000;
+const RETRYABLE_ERROR_CODES = new Set(['ECONNRESET', 'ETIMEDOUT', 'ECONNREFUSED', 'ENOTFOUND', 'EAI_AGAIN']);
+
+function isRetryable(error: unknown): boolean {
+  if (error instanceof ApiError) {
+    return error.status === 429 || error.status === 503;
+  }
+  // The SDK only wraps HTTP-level failures into ApiError - a raw fetch()
+  // failure (DNS, connection reset, timeout) surfaces as a plain network
+  // error instead, and is just as worth retrying in a CI environment.
+  if (error instanceof Error) {
+    if (error.name === 'AbortError') return true;
+    if (error instanceof TypeError && error.cause) return true;
+    const code = (error as NodeJS.ErrnoException).code;
+    if (code && RETRYABLE_ERROR_CODES.has(code)) return true;
+  }
+  return false;
+}
+
+async function withRetry<T>(fn: () => Promise<T>): Promise<T> {
+  for (let attempt = 0; ; attempt += 1) {
+    try {
+      return await fn();
+    } catch (error) {
+      if (!isRetryable(error) || attempt >= MAX_RETRIES) throw error;
+      const delayMs = INITIAL_BACKOFF_MS * 2 ** attempt;
+      await new Promise((resolve) => setTimeout(resolve, delayMs));
+    }
+  }
+}
+
 export class GeminiClient implements LLMClient {
   private readonly client: GoogleGenAI;
   private readonly generationModel: string;
@@ -29,15 +61,17 @@ export class GeminiClient implements LLMClient {
   }
 
   async complete(prompt: string, options: CompleteOptions = {}): Promise<string> {
-    const response = await this.client.models.generateContent({
-      model: this.generationModel,
-      contents: prompt,
-      config: {
-        systemInstruction: options.systemInstruction,
-        temperature: options.temperature,
-        maxOutputTokens: options.maxOutputTokens,
-      },
-    });
+    const response = await withRetry(() =>
+      this.client.models.generateContent({
+        model: this.generationModel,
+        contents: prompt,
+        config: {
+          systemInstruction: options.systemInstruction,
+          temperature: options.temperature,
+          maxOutputTokens: options.maxOutputTokens,
+        },
+      }),
+    );
 
     const text = response.text;
     if (text === undefined) {
@@ -47,10 +81,12 @@ export class GeminiClient implements LLMClient {
   }
 
   async embed(text: string): Promise<number[]> {
-    const response = await this.client.models.embedContent({
-      model: this.embeddingModel,
-      contents: text,
-    });
+    const response = await withRetry(() =>
+      this.client.models.embedContent({
+        model: this.embeddingModel,
+        contents: text,
+      }),
+    );
 
     const values = response.embeddings?.[0]?.values;
     if (!values) {

packages/core/src/parsing/file-walker.ts

@@ -1,4 +1,5 @@
 import { extname } from 'node:path';
+import { isTestFile } from '../shared/is-test-file.js';
 import { walkFiles } from '../shared/walk-files.js';
 import type { GrammarId } from './types.js';
 
@@ -24,8 +25,10 @@ export function resolveGrammar(filePath: string): GrammarId | undefined {
 
 export async function walkSourceFiles(rootDir: string): Promise<SourceFile[]> {
   const files = await walkFiles(rootDir, (name) => extname(name) in EXTENSION_TO_GRAMMAR);
-  return files.map((file) => ({
-    ...file,
-    grammar: EXTENSION_TO_GRAMMAR[extname(file.relativePath)],
-  }));
+  return files
+    .filter((file) => !isTestFile(file.relativePath))
+    .map((file) => ({
+      ...file,
+      grammar: EXTENSION_TO_GRAMMAR[extname(file.relativePath)],
+    }));
 }

packages/core/src/shared/is-test-file.ts

@@ -0,0 +1,5 @@
+const TEST_FILE_PATTERN = /(^|\/)(__tests__|tests?)\/|\.(test|spec)\.[^/]+$/;
+
+export function isTestFile(filePath: string): boolean {
+  return TEST_FILE_PATTERN.test(filePath);
+}