feat(tools): use --format llm for the body, keep each tool's envelope

runtime/llm.ts

@@ -0,0 +1,182 @@
+/**
+ * ix `--format llm` fast-path, gated on the installed CLI's version.
+ *
+ * Tools here fetch `--format json`, parse it, and hand-render markdown. The
+ * rendering carries real value — a header, an empty-graph message, an error
+ * envelope — but the *body* is usually a table or list that `--format llm`
+ * already emits, 2-4x smaller than the JSON it was rebuilt from.
+ *
+ * So this is the middle path: keep each tool's envelope, swap the body. A tool
+ * that gets llm text back emits its own header and then the records verbatim;
+ * a tool that gets `null` runs its existing JSON path untouched.
+ *
+ * ## Why the floor is per command
+ *
+ * `--format llm` did not arrive all at once, and the two tiers that matter are
+ * three minor versions apart:
+ *
+ *   Tier 1-4  map subsystems impact smells overview stats inventory rank
+ *             depends trace callers callees imports imported-by text history
+ *             locate diff                                        -> v0.7.0
+ *   Tier 5    explain read status doctor savings                 -> v0.9.2
+ *
+ * ## Why a wrong floor fails silently
+ *
+ * `ix` does not validate `--format`. Every renderer is
+ * `if json … else if llm … else text`, so an unrecognised value falls through
+ * to **human-readable text and exits 0**. An old CLI answers `--format llm`
+ * with a rendered table, not an error — there is nothing to catch. Asking
+ * `explain` for llm on 0.9.1 returns prose, successfully.
+ *
+ * The same property is what makes this safe to ship: there is no version of
+ * `ix` on which asking for `llm` breaks. The floors buy output quality, not
+ * crash-avoidance.
+ *
+ * ## Pro commands are excluded outright
+ *
+ * `briefing`, `decisions` and the rest of `@ix/pro` declare only `text|json`.
+ * There is no llm renderer at any version, so no gate can help — they are
+ * absent from the table below and must stay absent.
+ */
+
+import { $ } from "bun";
+import { redactSecrets } from "./secrets.ts";
+
+type SemVer = [number, number, number];
+
+/** command -> release whose renderer it needs. */
+export const LLM_MIN_VERSION: Record<string, SemVer> = {
+  // Tier 1
+  map: [0, 7, 0],
+  subsystems: [0, 7, 0],
+  impact: [0, 7, 0],
+  smells: [0, 7, 0],
+  overview: [0, 7, 0],
+  stats: [0, 7, 0],
+  // Tier 2
+  inventory: [0, 7, 0],
+  rank: [0, 7, 0],
+  depends: [0, 7, 0],
+  trace: [0, 7, 0],
+  callers: [0, 7, 0],
+  callees: [0, 7, 0],
+  imports: [0, 7, 0],
+  "imported-by": [0, 7, 0],
+  // Tier 3
+  text: [0, 7, 0],
+  history: [0, 7, 0],
+  // Tier 4
+  locate: [0, 7, 0],
+  diff: [0, 7, 0],
+  // Tier 5 — the reason this is a table and not one constant.
+  explain: [0, 9, 2],
+  read: [0, 9, 2],
+};
+
+/**
+ * Flag combinations that stay on text even on a current CLI, documented as
+ * deliberate exceptions in docs/llm-format.md: `diff --content` emits verbatim
+ * hunks, which have no record form.
+ */
+const TEXT_ONLY_FLAGS: Record<string, readonly string[]> = {
+  diff: ["--content"],
+};
+
+export function parseSemver(value: string): SemVer | null {
+  const match = (value ?? "").match(/(\d+)\.(\d+)\.(\d+)/);
+  if (!match) return null;
+  return [Number(match[1]), Number(match[2]), Number(match[3])];
+}
+
+export function gte(a: SemVer, b: SemVer): boolean {
+  for (let i = 0; i < 3; i++) {
+    if (a[i]! > b[i]!) return true;
+    if (a[i]! < b[i]!) return false;
+  }
+  return true;
+}
+
+export function llmDisabled(): boolean {
+  const flag = (process.env["IX_DISABLE_LLM_FORMAT"] ?? "").toLowerCase();
+  return flag === "1" || flag === "true" || flag === "yes";
+}
+
+// Process-lifetime memo: the version is probed at most once per plugin process.
+let versionPromise: Promise<SemVer | null> | null = null;
+
+/** For tests. */
+export function resetLlmVersionCache(): void {
+  versionPromise = null;
+}
+
+async function detectVersion(cwd: string): Promise<SemVer | null> {
+  if (!versionPromise) {
+    versionPromise = (async () => {
+      try {
+        const out = await $`ix --version`.cwd(cwd).quiet().text();
+        return parseSemver(out.trim());
+      } catch {
+        // No CLI, or it failed. Fail closed: the JSON path still works, and a
+        // tool that cannot run `ix --version` cannot run anything else either.
+        return null;
+      }
+    })();
+  }
+  return versionPromise;
+}
+
+export function commandAllowsLlm(args: readonly string[]): boolean {
+  const command = args[0];
+  if (!command) return false;
+  if (!(command in LLM_MIN_VERSION)) return false;
+  const blocked = TEXT_ONLY_FLAGS[command];
+  if (blocked && blocked.some((flag) => args.includes(flag))) return false;
+  return true;
+}
+
+/**
+ * `ix` reports some failures as a record on stdout *with exit 0* —
+ * `error code=<slug> message="…"` is part of the llm format by design. Checking
+ * only the exit status would forward that line to the model as a result, so it
+ * is detected here and deferred to the JSON path, where the tool's own error
+ * envelope applies. No success record begins with `error code=`.
+ */
+export function isLlmErrorLine(text: string): boolean {
+  return /^error code=/.test(text.trimStart());
+}
+
+/**
+ * Run `ix <args> --format llm` and return its text, or null to signal
+ * "use the JSON path".
+ *
+ * Every failure mode returns null: unsupported command, CLI too old, no CLI,
+ * a non-zero exit, empty output, or an `error code=` record.
+ */
+export async function tryLlm(
+  args: readonly string[],
+  cwd: string,
+): Promise<string | null> {
+  if (llmDisabled()) return null;
+  if (!commandAllowsLlm(args)) return null;
+
+  const floor = LLM_MIN_VERSION[args[0]!]!;
+  const version = await detectVersion(cwd);
+  if (version === null || !gte(version, floor)) return null;
+
+  let out: string;
+  try {
+    out = await $`ix ${[...args, "--format", "llm"]}`.cwd(cwd).quiet().text();
+  } catch {
+    return null;
+  }
+
+  // Scrubbed before it reaches the model. The JSON path does not do this today
+  // — only the runtime client scrubs — so this is not parity with it, just the
+  // cheaper side of the choice: redactSecrets is idempotent and order-free, so
+  // one pass over flat key=value lines costs nothing and cannot make the output
+  // wrong. Bringing the JSON path up to match is a separate change.
+  const text = redactSecrets(out).trim();
+  if (!text) return null;
+  if (isLlmErrorLine(text)) return null;
+  return text;
+}

tests/llm.test.ts

@@ -0,0 +1,181 @@
+/**
+ * The `--format llm` gate.
+ *
+ * Run with: bun test
+ *
+ * `ix` does not validate `--format`. Every renderer is
+ * `if json … else if llm … else text`, so an unrecognised value falls through
+ * to human-readable text and exits 0. That is what makes this safe to ship —
+ * no version of `ix` breaks on `--format llm` — and equally what makes a wrong
+ * floor dangerous: an old CLI answers with prose, successfully, and nothing
+ * raises. Most of what follows pins that boundary.
+ */
+
+import { describe, test, expect, beforeEach } from "bun:test";
+
+import {
+  LLM_MIN_VERSION,
+  commandAllowsLlm,
+  gte,
+  isLlmErrorLine,
+  llmDisabled,
+  parseSemver,
+  resetLlmVersionCache,
+} from "../runtime/llm.ts";
+
+beforeEach(() => {
+  resetLlmVersionCache();
+  delete process.env["IX_DISABLE_LLM_FORMAT"];
+});
+
+describe("parseSemver", () => {
+  test("parses a plain version", () => {
+    expect(parseSemver("0.9.2")).toEqual([0, 9, 2]);
+  });
+
+  test("parses a decorated version", () => {
+    expect(parseSemver("ix 0.9.2 (linux-amd64)")).toEqual([0, 9, 2]);
+  });
+
+  test("returns null for junk", () => {
+    expect(parseSemver("unknown")).toBeNull();
+    expect(parseSemver("")).toBeNull();
+  });
+});
+
+describe("gte", () => {
+  test("compares across each position", () => {
+    expect(gte([0, 9, 2], [0, 9, 2])).toBe(true);
+    expect(gte([0, 9, 3], [0, 9, 2])).toBe(true);
+    expect(gte([0, 10, 0], [0, 9, 9])).toBe(true);
+    expect(gte([1, 0, 0], [0, 99, 99])).toBe(true);
+    expect(gte([0, 9, 1], [0, 9, 2])).toBe(false);
+    expect(gte([0, 6, 0], [0, 7, 0])).toBe(false);
+  });
+});
+
+describe("the version table", () => {
+  test("Tier 1-4 commands sit at 0.7.0", () => {
+    for (const command of [
+      "map", "subsystems", "impact", "smells", "overview", "stats",
+      "inventory", "rank", "depends", "trace", "callers", "callees",
+      "imports", "imported-by", "text", "history", "locate", "diff",
+    ]) {
+      expect(LLM_MIN_VERSION[command]).toEqual([0, 7, 0]);
+    }
+  });
+
+  test("Tier 5 commands sit at 0.9.2", () => {
+    // The reason this is a table and not one constant. Before 0.9.2 these two
+    // accepted `--format llm` and rendered text, so a single 0.7.0 floor would
+    // have forwarded prose to the model as though it were records.
+    expect(LLM_MIN_VERSION["explain"]).toEqual([0, 9, 2]);
+    expect(LLM_MIN_VERSION["read"]).toEqual([0, 9, 2]);
+  });
+
+  test("Pro commands are absent at every version", () => {
+    // @ix/pro declares only text|json — there is no llm renderer to gate on,
+    // so no floor can make these safe and none should try.
+    for (const command of ["briefing", "decisions", "goals", "plan", "truth", "bugs"]) {
+      expect(LLM_MIN_VERSION[command]).toBeUndefined();
+      expect(commandAllowsLlm([command])).toBe(false);
+    }
+  });
+});
+
+describe("commandAllowsLlm", () => {
+  test("accepts a known command", () => {
+    expect(commandAllowsLlm(["stats"])).toBe(true);
+    expect(commandAllowsLlm(["rank", "--by", "dependents"])).toBe(true);
+  });
+
+  test("refuses an unknown command", () => {
+    expect(commandAllowsLlm(["nonesuch"])).toBe(false);
+  });
+
+  test("refuses an empty argv", () => {
+    expect(commandAllowsLlm([])).toBe(false);
+  });
+
+  test("keeps `diff --content` on text", () => {
+    // docs/llm-format.md keeps this on text deliberately: verbatim hunks have
+    // no record form.
+    expect(commandAllowsLlm(["diff", "1", "5"])).toBe(true);
+    expect(commandAllowsLlm(["diff", "1", "5", "--content"])).toBe(false);
+  });
+});
+
+describe("isLlmErrorLine", () => {
+  test("detects the error record ix writes to stdout with exit 0", () => {
+    // Checking only the exit status would forward this to the model as though
+    // it were a result. Detecting it defers to the JSON path, whose error
+    // envelope is what each tool already documents.
+    expect(isLlmErrorLine('error code=unknown_target message="No entity named X"')).toBe(true);
+    expect(isLlmErrorLine('  error code=ambiguous_target message="…"')).toBe(true);
+  });
+
+  test("does not fire on real records", () => {
+    expect(isLlmErrorLine("stats nodes=98979 edges=354283")).toBe(false);
+    expect(isLlmErrorLine('region id=cli label="Cli / Client" level=2')).toBe(false);
+    // A record that merely mentions an error is not an error line.
+    expect(isLlmErrorLine('smell kind=has_smell.error_swallow file=a.ts')).toBe(false);
+  });
+});
+
+describe("kill switch", () => {
+  test("IX_DISABLE_LLM_FORMAT forces the JSON path", () => {
+    for (const value of ["1", "true", "TRUE", "yes"]) {
+      process.env["IX_DISABLE_LLM_FORMAT"] = value;
+      expect(llmDisabled()).toBe(true);
+    }
+    process.env["IX_DISABLE_LLM_FORMAT"] = "0";
+    expect(llmDisabled()).toBe(false);
+    delete process.env["IX_DISABLE_LLM_FORMAT"];
+    expect(llmDisabled()).toBe(false);
+  });
+});
+
+describe("tool wiring", () => {
+  // The envelope is the point of the middle path: each tool keeps its own
+  // header and error handling and swaps only the body. A fast-path that
+  // returned bare records would strip the header the model orients on.
+  const CASES: [string, string][] = [
+    ["ix-stats.ts", "## ix-stats"],
+    ["ix-subsystems.ts", "## ix-subsystems"],
+    ["ix-smells.ts", "## ix-smells"],
+    ["ix-trace.ts", "## ix-trace:"],
+    ["ix-locate.ts", "## ix-locate:"],
+    ["ix-rank.ts", "## ix-rank:"],
+    ["ix-inventory.ts", "## ix-inventory:"],
+    ["ix-explain.ts", "## ix-explain:"],
+  ];
+
+  for (const [file, header] of CASES) {
+    test(`${file} keeps its header on the fast path`, async () => {
+      const source = await Bun.file(`${import.meta.dir}/../tools/${file}`).text();
+      const index = source.indexOf("tryLlm(");
+      expect(index).toBeGreaterThan(-1);
+      // The header has to appear in the fast-path return, which is the few
+      // lines after the tryLlm call. Window is generous because some of those
+      // call sites carry a paragraph of comment before the return.
+      expect(source.slice(index, index + 900)).toContain(header);
+    });
+  }
+
+  test("ix-neighbors labels each section on the fast path", async () => {
+    const source = await Bun.file(`${import.meta.dir}/../tools/ix-neighbors.ts`).text();
+    const index = source.indexOf("tryLlm(");
+    expect(source.slice(index, index + 400)).toContain("capitalize(direction)");
+  });
+
+  test("no tool sends a Pro command down the fast path", async () => {
+    const { readdirSync } = await import("node:fs");
+    const dir = `${import.meta.dir}/../tools`;
+    for (const file of readdirSync(dir).filter((f) => f.endsWith(".ts"))) {
+      const source = await Bun.file(`${dir}/${file}`).text();
+      for (const match of source.matchAll(/tryLlm\(\s*\[\s*"([a-z-]+)"/g)) {
+        expect(LLM_MIN_VERSION[match[1]!]).toBeDefined();
+      }
+    }
+  });
+});

tools/ix-explain.ts

@@ -7,6 +7,7 @@
  */
 
 import { $ } from "bun";
+import { tryLlm } from "../runtime/llm.ts";
 
 export const name = "ix-explain";
 export const description =
@@ -29,6 +30,12 @@ type Context = { directory: string; worktree?: string };
 export async function execute(params: Params, context: Context): Promise<string> {
   const dir = context.worktree ?? context.directory;
 
+  // Tier 5: gated to ix >= 0.9.2, not 0.7.0. Before that release `explain`
+  // accepted `--format llm` and rendered *text* — no error, exit 0 — so an
+  // ungated call here would hand the model prose dressed as records.
+  const fast = await tryLlm(["explain", params.symbol], dir);
+  if (fast) return `## ix-explain: ${params.symbol}\n\n${fast}`;
+
   let output: string;
   try {
     output = await $`ix explain ${params.symbol} --format json`.cwd(dir).text();