Merge pull request #786 from danmackinlay/fix/full-path-unresolved-signal

fix(cli): stop --full-path from degrading silently (#785)

CHANGELOG.md

@@ -2,6 +2,25 @@
 
 ## [Unreleased]
 
+### Changed
+
+- `--full-path` no longer degrades silently when a result cannot be resolved on
+  disk (#785). A fallback there means the file moved or was deleted since the
+  last index, so `search`, `query`, `get` and `multi-get` now print a notice to
+  stderr naming how many results fell back and suggesting `qmd update`; stdout
+  stays machine-readable.
+- `search`/`query` now decide per result whether to show the docid under
+  `--full-path`, matching `multi-get` and `get`: a result that resolved shows
+  its on-disk path and no docid, one that did not keeps its `qmd://` URI *and*
+  its docid, so it is still addressable. Previously the docid was dropped for
+  every row whenever the flag was set, leaving unresolved rows with neither a
+  usable path nor an identifier.
+- `search --format csv` always emits the `docid` column, empty for rows that
+  resolved to an on-disk path. Under `--full-path` the header previously
+  dropped the column entirely — which also disagreed with the empty-result
+  header, always printed with `docid`. Column positions are now stable across
+  runs and formats.
+
 ### Fixed
 
 - `qmd --version` no longer reports an unrelated repository's commit (#787).

README.md

@@ -790,6 +790,9 @@ and `deep-search` (→ `query`).
 --no-rerank        # Skip LLM reranking (RRF scores only; faster on CPU)
 -C, --candidate-limit <n>  # Max candidates to rerank (default: 40)
 --full-path        # Emit on-disk filesystem paths instead of qmd:// URIs
+                   # (a result whose file has moved or been deleted since
+                   #  indexing keeps its qmd:// URI + docid, and a notice is
+                   #  printed to stderr — run `qmd update` to refresh)
 
 # Output formats (for search and multi-get)
 --format <kind>    # cli (default) | json | csv | md | xml | files

src/cli/qmd.ts

@@ -970,6 +970,26 @@ function renderFullPath(absolutePath: string, cwd: string = process.cwd()): stri
   return real;
 }
 
+/**
+ * Report rows that `--full-path` could not turn into an on-disk path.
+ *
+ * The flag's whole job is producing openable paths, so falling back to a
+ * `qmd://` URI is worth saying out loud: it means the file moved or was
+ * deleted since the last index, not that the path was normalized away. The
+ * notice goes to stderr so stdout stays machine-readable.
+ */
+function warnUnresolvedFullPaths(unresolved: number, total: number): void {
+  if (unresolved <= 0) return;
+  const subject = total === 1
+    ? "the file"
+    : `${unresolved} of ${total} results`;
+  console.error(
+    `${c.yellow}warning:${c.reset} --full-path could not resolve ${subject} on disk ` +
+    `(moved or deleted since indexing); showing qmd:// + docid instead. ` +
+    `Run 'qmd update' to refresh the index.`
+  );
+}
+
 function getDocument(filename: string, fromLine?: number, maxLines?: number, lineNumbers?: boolean, fullPath: boolean = false): void {
   // Parse :line suffix from filename. Two forms:
   //   "file.md:100"     -> start at line 100
@@ -1028,7 +1048,8 @@ function getDocument(filename: string, fromLine?: number, maxLines?: number, lin
   const canonicalPath = `qmd://${doc.displayPath}`;
 
   // --full-path: show the on-disk path instead of the qmd:// URL + docid, when
-  // the file actually exists. Fall back to the canonical header otherwise.
+  // the file actually exists. Fall back to the canonical header otherwise, and
+  // say so on stderr — a fallback here means the index is stale.
   let header: string;
   if (fullPath) {
     const fsPath = resolveVirtualPath(db, canonicalPath);
@@ -1036,6 +1057,7 @@ function getDocument(filename: string, fromLine?: number, maxLines?: number, lin
       header = renderFullPath(fsPath);
     } else {
       header = docid ? `${canonicalPath}  #${docid}` : canonicalPath;
+      warnUnresolvedFullPaths(1, 1);
     }
   } else {
     header = docid ? `${canonicalPath}  #${docid}` : canonicalPath;
@@ -1264,6 +1286,7 @@ function multiGet(pattern: string, maxLines?: number, maxBytes: number = DEFAULT
   // resolved). Per result: pick the identifier and whether to show the docid.
   const identOf = (r: typeof results[number]): string => (fullPath && r.fsPath) ? r.fsPath : r.displayPath;
   const docidOf = (r: typeof results[number]): string | undefined => (fullPath && r.fsPath) ? undefined : r.docid;
+  const unresolvedCount = fullPath ? results.filter(r => !r.fsPath).length : 0;
 
   // Output based on format
   if (format === "json") {
@@ -1354,6 +1377,8 @@ function multiGet(pattern: string, maxLines?: number, maxBytes: number = DEFAULT
       console.log(r.body);
     }
   }
+
+  warnUnresolvedFullPaths(unresolvedCount, results.length);
 }
 
 // List files in virtual file tree
@@ -2169,20 +2194,37 @@ function outputResults(results: OutputRow[], query: string, opts: OutputOptions)
     );
   };
 
-  // Helper to pick the visible path for a result. With --full-path we swap
+  // Resolve every row's visible identifier up front. With --full-path we swap
   // the qmd:// URI for the file's on-disk path via renderFullPath() (./-
-  // prefixed relative when under $PWD, absolute realpath otherwise). Falls
-  // back to qmd:// if the file is no longer resolvable on disk.
+  // prefixed relative when under $PWD, absolute realpath otherwise). A row
+  // whose file is gone from disk falls back to qmd:// and *keeps its docid*,
+  // so it stays addressable — the same per-row rule multiGet() uses. Resolving
+  // eagerly also means unresolved rows are counted before anything is printed.
   const linkDbForPaths = opts.fullPath ? getDb() : null;
-  const displayPathFor = (row: OutputRow): string => {
+  const resolutions = new Map<OutputRow, { ident: string; resolved: boolean }>();
+  for (const row of filtered) {
     // Always rebuild from displayPath so the active index name is included
     // as ?index=… for non-default indexes. row.file may not carry it.
     const qmdUri = toQmdPath(row.displayPath);
-    if (!opts.fullPath || !linkDbForPaths) return qmdUri;
-    const absolute = resolveVirtualPath(linkDbForPaths, qmdUri);
-    if (!absolute || !existsSync(absolute)) return qmdUri;
-    return renderFullPath(absolute);
-  };
+    let resolution = { ident: qmdUri, resolved: false };
+    if (opts.fullPath && linkDbForPaths) {
+      const absolute = resolveVirtualPath(linkDbForPaths, qmdUri);
+      if (absolute && existsSync(absolute)) {
+        resolution = { ident: renderFullPath(absolute), resolved: true };
+      }
+    }
+    resolutions.set(row, resolution);
+  }
+  const unresolvedCount = opts.fullPath
+    ? filtered.filter(row => !resolutions.get(row)?.resolved).length
+    : 0;
+
+  const displayPathFor = (row: OutputRow): string =>
+    resolutions.get(row)?.ident ?? toQmdPath(row.displayPath);
+  // Show the docid whenever it is still the row's identifier: always without
+  // --full-path, and with it only for rows that have no on-disk path to show.
+  const showDocid = (row: OutputRow): boolean =>
+    !opts.fullPath || !resolutions.get(row)?.resolved;
 
   if (opts.format === "json") {
     // JSON output for LLM consumption
@@ -2195,9 +2237,11 @@ function outputResults(results: OutputRow[], query: string, opts: OutputOptions)
         if (body) body = addLineNumbers(body);
         if (snippet) snippet = addLineNumbers(snippet);
       }
-      // With --full-path, omit docid (the on-disk path is the identifier).
+      // With --full-path, omit docid (the on-disk path is the identifier) —
+      // unless the path could not be resolved, in which case it is all the
+      // caller has.
       return {
-        ...(docid && !opts.fullPath && { docid: `#${docid}` }),
+        ...(docid && showDocid(row) && { docid: `#${docid}` }),
         score: Math.round(row.score * 100) / 100,
         file: displayPathFor(row),
         line: snippetInfo.line,
@@ -2249,7 +2293,7 @@ function outputResults(results: OutputRow[], query: string, opts: OutputOptions)
       const snippetBody = snippet.split("\n").slice(1).join("\n").toLowerCase();
       const hasMatch = query.toLowerCase().split(/\s+/).some(t => t.length > 0 && snippetBody.includes(t));
       const lineInfo = hasMatch ? `:${line}` : "";
-      const docidStr = (docid && !opts.fullPath) ? ` ${c.dim}#${docid}${c.reset}` : "";
+      const docidStr = (docid && showDocid(row)) ? ` ${c.dim}#${docid}${c.reset}` : "";
 
       if (process.stdout.isTTY && absolutePath && parsed?.path) {
         const linkLine = hasMatch ? line : 1;
@@ -2318,8 +2362,9 @@ function outputResults(results: OutputRow[], query: string, opts: OutputOptions)
         content = addLineNumbers(content);
       }
       const fileLine = `**file:** \`${visiblePath}\`\n`;
-      // With --full-path the on-disk path is the identifier; drop the docid line.
-      const docidLine = (docid && !opts.fullPath) ? `**docid:** \`#${docid}\`\n` : "";
+      // With --full-path the on-disk path is the identifier; drop the docid
+      // line unless this row had no path to show.
+      const docidLine = (docid && showDocid(row)) ? `**docid:** \`#${docid}\`\n` : "";
       const contextLine = row.context ? `**context:** ${row.context}\n` : "";
       console.log(`---\n# ${heading}\n${fileLine}${docidLine}${contextLine}\n${content}\n`);
     }
@@ -2332,15 +2377,15 @@ function outputResults(results: OutputRow[], query: string, opts: OutputOptions)
       if (opts.lineNumbers) {
         content = addLineNumbers(content);
       }
-      const docidAttr = opts.fullPath ? "" : ` docid="#${docid}"`;
+      const docidAttr = showDocid(row) ? ` docid="#${docid}"` : "";
       console.log(`<file${docidAttr} name="${displayPathFor(row)}"${titleAttr}${contextAttr}>\n${content}\n</file>\n`);
     }
   } else {
-    // CSV format
-    const csvHeader = opts.fullPath
-      ? "score,file,title,context,line,snippet"
-      : "docid,score,file,title,context,line,snippet";
-    console.log(csvHeader);
+    // CSV format. The docid column is always present — under --full-path it is
+    // empty for rows whose on-disk path was found and carries the docid for
+    // rows that fell back to a qmd:// URI, so the columns stay positional.
+    // (multi-get's CSV already emits the docid column unconditionally.)
+    console.log("docid,score,file,title,context,line,snippet");
     for (const row of filtered) {
       const { line, snippet } = extractSnippet(row.body, query, 500, row.chunkPos, row.chunkLen, opts.intent);
       let content = opts.full ? row.body : snippet;
@@ -2351,13 +2396,12 @@ function outputResults(results: OutputRow[], query: string, opts: OutputOptions)
       const snippetText = content || "";
       const path = escapeCSV(displayPathFor(row));
       const tail = `${path},${escapeCSV(row.title || "")},${escapeCSV(row.context || "")},${line},${escapeCSV(snippetText)}`;
-      if (opts.fullPath) {
-        console.log(`${row.score.toFixed(4)},${tail}`);
-      } else {
-        console.log(`#${docid},${row.score.toFixed(4)},${tail}`);
-      }
+      const docidField = (docid && showDocid(row)) ? `#${docid}` : "";
+      console.log(`${docidField},${row.score.toFixed(4)},${tail}`);
     }
   }
+
+  warnUnresolvedFullPaths(unresolvedCount, filtered.length);
 }
 
 // Resolve -c collection filter: supports single string, array, or undefined.
@@ -3361,6 +3405,7 @@ function showHelp(): void {
   console.log("  --no-line-numbers          - Disable line numbers for get/multi-get");
   console.log("  --full-path                - Show on-disk paths instead of qmd:// + docid (get/multi-get/search/query)");
   console.log("                                Paths are ./-prefixed when under $PWD, absolute otherwise");
+  console.log("                                Results whose file is gone keep qmd:// + docid and warn on stderr");
   console.log("  --explain                  - Include retrieval score traces (query, CLI/--format json)");
   console.log("  --format <kind>            - Output format: cli (default) | json | csv | md | xml | files");
   console.log("  -c, --collection <name>    - Filter by one or more collections");

test/full-path-fallback.test.ts

@@ -0,0 +1,210 @@
+/**
+ * `--full-path` fallback tests.
+ *
+ * `--full-path` swaps the `qmd://` URI + docid for the file's on-disk path.
+ * When a result can't be resolved on disk — the file moved or was deleted
+ * since the last index — it falls back to the URI. That fallback must:
+ *   1. keep the docid, so the row is still addressable (search/query used to
+ *      drop it, unlike get/multi-get), and
+ *   2. say so on stderr, so the stale index is visible rather than silent.
+ *
+ * stdout must stay machine-clean in every format.
+ */
+
+import { describe, test, expect, beforeAll, afterAll } from "vitest";
+import { mkdir, mkdtemp, rename, rm, writeFile } from "fs/promises";
+import { realpathSync } from "fs";
+import { tmpdir } from "os";
+import { join, dirname } from "path";
+import { spawn } from "child_process";
+import { fileURLToPath } from "url";
+
+const thisDir = dirname(fileURLToPath(import.meta.url));
+const projectRoot = join(thisDir, "..");
+const qmdScript = join(projectRoot, "src", "cli", "qmd.ts");
+const isBunRuntime = typeof (globalThis as { Bun?: unknown }).Bun !== "undefined";
+const tsxCli = join(projectRoot, "node_modules", "tsx", "dist", "cli.mjs");
+
+async function runQmd(
+  args: string[],
+  opts: { cwd: string; dbPath: string; configDir: string }
+): Promise<{ stdout: string; stderr: string; exitCode: number }> {
+  const runner = isBunRuntime
+    ? { command: process.execPath, args: [qmdScript, ...args] }
+    : { command: process.execPath, args: [tsxCli, qmdScript, ...args] };
+
+  const proc = spawn(runner.command, runner.args, {
+    cwd: opts.cwd,
+    env: {
+      ...process.env,
+      INDEX_PATH: opts.dbPath,
+      QMD_CONFIG_DIR: opts.configDir,
+      PWD: opts.cwd,
+      QMD_DOCTOR_DEVICE_PROBE: "0",
+    },
+    stdio: ["ignore", "pipe", "pipe"],
+  });
+
+  let stdout = "";
+  let stderr = "";
+  proc.stdout?.on("data", (c: Buffer) => { stdout += c.toString(); });
+  proc.stderr?.on("data", (c: Buffer) => { stderr += c.toString(); });
+  const exitCode = await new Promise<number>((res, rej) => {
+    proc.once("error", rej);
+    proc.on("close", (code) => res(code ?? 1));
+  });
+  return { stdout, stderr, exitCode };
+}
+
+// The runtime prints unrelated Node deprecation notices on some versions;
+// assert on our own warning rather than on stderr being empty.
+const hasFullPathWarning = (stderr: string) =>
+  /--full-path could not resolve/.test(stderr);
+
+let testDir: string;
+let collectionDir: string;
+let dbPath: string;
+let configDir: string;
+
+beforeAll(async () => {
+  testDir = await mkdtemp(join(tmpdir(), "qmd-full-path-"));
+  const envDir = join(testDir, "env");
+  collectionDir = join(envDir, "corpus");
+  dbPath = join(envDir, "test.sqlite");
+  configDir = join(envDir, "config");
+
+  await mkdir(collectionDir, { recursive: true });
+  await mkdir(configDir, { recursive: true });
+  await writeFile(join(configDir, "index.yml"), "collections: {}\n");
+  await writeFile(join(collectionDir, "alpha.md"), "# Alpha\n\nsearchterm-stale alpha\n");
+  await writeFile(join(collectionDir, "beta.md"), "# Beta\n\nsearchterm-stale beta\n");
+  collectionDir = realpathSync(collectionDir);
+
+  const add = await runQmd(
+    ["collection", "add", collectionDir, "--name", "stale"],
+    { cwd: collectionDir, dbPath, configDir }
+  );
+  expect(add.exitCode, `collection add failed: ${add.stderr}`).toBe(0);
+
+  // beta.md moves out of the collection: its row is now stale.
+  await rename(join(collectionDir, "beta.md"), join(testDir, "beta-moved.md"));
+});
+
+afterAll(async () => {
+  await rm(testDir, { recursive: true, force: true });
+});
+
+describe("--full-path fallback for unresolvable results", () => {
+  test("search --json keeps the docid on the row it could not resolve", async () => {
+    const { stdout, stderr, exitCode } = await runQmd(
+      ["search", "searchterm-stale", "--full-path", "--json"],
+      { cwd: collectionDir, dbPath, configDir }
+    );
+    expect(exitCode).toBe(0);
+
+    const results = JSON.parse(stdout) as Array<{ file: string; docid?: string }>;
+    expect(results.length).toBe(2);
+
+    const resolved = results.find((r) => !r.file.startsWith("qmd://"));
+    const unresolved = results.find((r) => r.file.startsWith("qmd://"));
+    expect(resolved, "alpha.md should resolve on disk").toBeDefined();
+    expect(unresolved, "beta.md should fall back to its qmd:// URI").toBeDefined();
+
+    // Resolved row: the path is the identifier, so no docid.
+    expect(resolved!.file).toContain("alpha.md");
+    expect(resolved!.docid).toBeUndefined();
+    // Unresolved row: the docid is all that is left to address it by.
+    expect(unresolved!.docid).toMatch(/^#[a-f0-9]{6}$/);
+
+    expect(hasFullPathWarning(stderr)).toBe(true);
+    expect(stderr).toContain("qmd update");
+  });
+
+  test("search --format csv always emits the docid column", async () => {
+    const { stdout, exitCode } = await runQmd(
+      ["search", "searchterm-stale", "--full-path", "--format", "csv"],
+      { cwd: collectionDir, dbPath, configDir }
+    );
+    expect(exitCode).toBe(0);
+
+    const lines = stdout.trim().split("\n");
+    expect(lines[0]).toBe("docid,score,file,title,context,line,snippet");
+
+    // Resolved rows leave the column empty; the unresolved row fills it. The
+    // column count is the same either way, so positional parsing still works.
+    const resolvedRow = lines.find((l) => l.includes("alpha.md"));
+    const unresolvedRow = lines.find((l) => l.includes("qmd://stale/beta.md"));
+    expect(resolvedRow).toBeDefined();
+    expect(unresolvedRow).toBeDefined();
+    expect(resolvedRow!.startsWith(",")).toBe(true);
+    expect(unresolvedRow).toMatch(/^#[a-f0-9]{6},/);
+  });
+
+  test("search default CLI format keeps the docid next to the fallback URI", async () => {
+    const { stdout, stderr, exitCode } = await runQmd(
+      ["search", "searchterm-stale", "--full-path"],
+      { cwd: collectionDir, dbPath, configDir }
+    );
+    expect(exitCode).toBe(0);
+    // eslint-disable-next-line no-control-regex
+    const plain = stdout.replace(/\x1b\[[0-9;]*m/g, "").replace(/\x1b\]8;;[^\x07]*\x07/g, "");
+
+    const betaLine = plain.split("\n").find((l) => l.includes("qmd://stale/beta.md"));
+    expect(betaLine, "beta should fall back to its qmd:// URI").toBeDefined();
+    expect(betaLine).toMatch(/#[a-f0-9]{6}\s*$/);
+
+    const alphaLine = plain.split("\n").find((l) => l.includes("alpha.md") && !l.startsWith("Title"));
+    expect(alphaLine).toBeDefined();
+    expect(alphaLine).not.toMatch(/#[a-f0-9]{6}/);
+
+    expect(hasFullPathWarning(stderr)).toBe(true);
+  });
+
+  test("get warns and falls back to qmd:// + docid", async () => {
+    const { stdout, stderr, exitCode } = await runQmd(
+      ["get", "beta.md", "--full-path"],
+      { cwd: collectionDir, dbPath, configDir }
+    );
+    expect(exitCode).toBe(0);
+    expect(stdout.split("\n")[0]).toMatch(/^qmd:\/\/stale\/beta\.md {2}#[a-f0-9]{6}$/);
+    expect(hasFullPathWarning(stderr)).toBe(true);
+  });
+
+  test("multi-get warns when a requested file is gone from disk", async () => {
+    const { stdout, stderr, exitCode } = await runQmd(
+      ["multi-get", "alpha.md,beta.md", "--full-path", "--format", "files"],
+      { cwd: collectionDir, dbPath, configDir }
+    );
+    expect(exitCode).toBe(0);
+    expect(stdout).toMatch(/#[a-f0-9]{6} qmd:\/\/stale\/beta\.md/);
+    expect(hasFullPathWarning(stderr)).toBe(true);
+  });
+
+  test("no warning when every result resolves", async () => {
+    const { stdout, stderr, exitCode } = await runQmd(
+      ["search", "alpha", "--full-path", "--json"],
+      { cwd: collectionDir, dbPath, configDir }
+    );
+    expect(exitCode).toBe(0);
+    const results = JSON.parse(stdout) as Array<{ file: string; docid?: string }>;
+    expect(results.length).toBe(1);
+    expect(results[0]!.file).not.toMatch(/^qmd:\/\//);
+    expect(results[0]!.docid).toBeUndefined();
+    expect(hasFullPathWarning(stderr)).toBe(false);
+  });
+
+  test("without --full-path nothing changes and nothing is warned", async () => {
+    const { stdout, stderr, exitCode } = await runQmd(
+      ["search", "searchterm-stale", "--json"],
+      { cwd: collectionDir, dbPath, configDir }
+    );
+    expect(exitCode).toBe(0);
+    const results = JSON.parse(stdout) as Array<{ file: string; docid?: string }>;
+    expect(results.length).toBe(2);
+    for (const r of results) {
+      expect(r.file).toMatch(/^qmd:\/\/stale\//);
+      expect(r.docid).toMatch(/^#[a-f0-9]{6}$/);
+    }
+    expect(hasFullPathWarning(stderr)).toBe(false);
+  });
+});