From c131204acd45ace3748b26479e74333b959f6836 Mon Sep 17 00:00:00 2001 From: letur Date: Wed, 9 Sep 2026 16:38:42 +0200 Subject: [PATCH 1/4] feat: explain a codebase, not only a change thurview's unit was a CHANGE: anchors pin to commits, and the diff, the commits and the interface delta all assume a range. Explaining a codebase was a workaround - scaffold with base == head - which leaves every one of those surfaces rendering a claim about a change that does not exist. Add an explainer as a second document kind beside a review, sharing the whole understanding machine (anchors, peeks, map, threads, revisions, publish/wait) and dropping what describes a range. `thurview explain [] [--commit ]` pins one commit and an optional path scope; Files and Commits are absent rather than empty, and a Coverage tab takes their place. Coverage is derived at publish rather than claimed in prose: every file in scope at the pinned commit is explained (an anchor points into it), placed (a map node owns it) or not examined, and the counts sit above the document in the slot a review gives the interface delta. An explainer that anchors nothing is rejected; one with no map is warned, because "everything unanchored was not examined" is true but rarely meant. Everything the Coverage tab states is a count or a list of named things the reader can re-derive with `thurview graph` at the same commit - cluster sizes and hubs, cross-cluster reference counts, names defined in more than one cluster, files the graph cannot read. No severities, no scores, no issues list: it surfaces structure and leaves the conclusion to the reader. Also seed Louvain in architecture(), which was unseeded and so clustered the same commit differently between runs. Claude-Session: https://claude.ai/code/session_01WW5sEutdtcbSUtgLQKBKpn --- README.md | 49 ++- skills/thurview/SKILL.md | 42 ++- skills/thurview/references/code-explainer.md | 166 ++++++++++ src/cli.ts | 301 +++++++++++++++++- src/coverage.ts | 249 +++++++++++++++ src/document/compile.ts | 50 ++- src/graph.ts | 20 +- src/server/server.ts | 6 +- src/store.ts | 20 +- src/ui/api.ts | 3 + src/ui/app.css | 174 +++++++++++ src/ui/app.ts | 64 ++-- src/ui/state.ts | 32 +- src/ui/threads.ts | 22 +- src/ui/views/coverage.ts | 306 +++++++++++++++++++ src/ui/views/map.ts | 73 ++++- src/ui/views/review.ts | 82 ++++- test/e2e.test.ts | 111 +++++++ 18 files changed, 1671 insertions(+), 99 deletions(-) create mode 100644 skills/thurview/references/code-explainer.md create mode 100644 src/coverage.ts create mode 100644 src/ui/views/coverage.ts diff --git a/README.md b/README.md index 1370235..402668e 100644 --- a/README.md +++ b/README.md @@ -9,6 +9,12 @@ and serves it in your browser: the walkthrough, live code peeks, the diff, commits, and a software map. You ask questions, leave anchored comments, and approve or request changes. The agent answers and republishes. +It also explains a codebase. A **code explainer** is the same document over a +different unit: one pinned commit instead of a range, so a reader can see the +architecture well enough to spot design problems themselves. It has no diff and +nothing to approve, it surfaces structure rather than grading it, and it states +what it did not examine. + It does not review the code for you. It helps you understand it fast enough to review it yourself. @@ -133,6 +139,12 @@ and open it. Click an identifier to see where it is defined at that commit; Ctrl-click jumps there. - **Commits**: the commits between base and head. +- **Coverage** (explainers): every file in scope at the pinned commit, in one + of three states - anchored in the document, placed on the map only, or not + examined - with the parts of the system they belong to, the references that + cross between those parts, and the names defined in more than one of them. + Derived at publish, so what the explainer skipped is a stated fact rather + than something the reader has to infer. - **Map**: systems, containers, components and code, with what the change added, removed or touched, linked to files and code. - **Threads**: _Ask now_ sends a question to the agent immediately and the @@ -153,18 +165,19 @@ bar. ## CLI -| Command | Purpose | -| --------------------------------------------------------------------- | ------------------------------------------------------------ | -| `thurview scaffold [--pr N \| --base R --head R]` | Create a review pinned to exact commits (`--update` re-pins) | -| `thurview info [--all]` | Reviews bound to this worktree | -| `thurview publish --review ID [--view T] [--open]` | Validate the document and map, seal a revision | -| `thurview open --review ID [--view T]` | Start the server if needed and open the browser | -| `thurview wait --review ID [--timeout S]` | Block until the reader needs the agent | -| `thurview threads list\|get\|reply\|resolve` | Read and answer threads | -| `thurview graph interfaces\|impact\|callers\|tests-for\|architecture` | Ask the code graph at the pinned commits | -| `thurview serve` / `thurview stop` | Run the server in the foreground / stop the background one | -| `thurview setup hooks\|skill\|status` | Session hooks, agent skill, install state | -| `thurview update` | Self-update from npm | +| Command | Purpose | +| --------------------------------------------------------------------- | ---------------------------------------------------------------- | +| `thurview scaffold [--pr N \| --base R --head R]` | Create a review pinned to exact commits (`--update` re-pins) | +| `thurview explain [] [--commit R]` | Create a code explainer of a codebase or subsystem at one commit | +| `thurview info [--all]` | Reviews bound to this worktree | +| `thurview publish --review ID [--view T] [--open]` | Validate the document and map, seal a revision | +| `thurview open --review ID [--view T]` | Start the server if needed and open the browser | +| `thurview wait --review ID [--timeout S]` | Block until the reader needs the agent | +| `thurview threads list\|get\|reply\|resolve` | Read and answer threads | +| `thurview graph interfaces\|impact\|callers\|tests-for\|architecture` | Ask the code graph at the pinned commits | +| `thurview serve` / `thurview stop` | Run the server in the foreground / stop the background one | +| `thurview setup hooks\|skill\|status` | Session hooks, agent skill, install state | +| `thurview update` | Self-update from npm | thurview is an [AXI](https://axi.md): built for agents that drive it through a shell. Output is [TOON](https://toonformat.dev) on stdout, errors are @@ -186,16 +199,22 @@ The agent writes three files in `~/.thurview/reviews//`: - `data.yaml`: typed inputs: `actors`, `anchors` (file, from, to, graph), `stores`, `interfaces` (a capability line per derived entry, plus the interfaces the graph cannot see). -- `map.yaml`: the software map at head, optionally at base. +- `map.yaml`: the software map at head, optionally at base. In an explainer it + carries the breadth the prose has no room for, and a node's `files` globs are + what let a file count as placed rather than not examined. - `theme.yaml`: the look, derived from the reviewed project's own design system (tokens, fonts, shape, code palette). Empty means the default skin. +An explainer writes the same files, minus `interfaces`: there is no change to +derive a delta from, and `graph: base` on an anchor is an error because there +is one commit. + `thurview publish` rejects an anchor whose file or lines do not exist at the pinned commit, a call stack frame that claims an added or removed call the diff does not show, a storage operation on an unknown field, a map edge to an unknown node, an interface annotation for a symbol the change did not -move, and a declared interface whose anchor holds no added or deleted line. -The full format is in +move, a declared interface whose anchor holds no added or deleted line, and an +explainer that anchors nothing at all. The full format is in [skills/thurview/references](skills/thurview/references). Optional guidance for the agent: `~/.thurview/THURVIEW.md` for you, diff --git a/skills/thurview/SKILL.md b/skills/thurview/SKILL.md index e7af18f..98f6732 100644 --- a/skills/thurview/SKILL.md +++ b/skills/thurview/SKILL.md @@ -1,12 +1,24 @@ --- name: thurview -description: Author and publish a thurview review - a guided, evidence-anchored explanation of a branch, pull request or commit range that the reader opens in the browser, annotates, asks questions about, and approves or sends back. Use when the user asks to review a branch or PR, to explain or walk through a change, "review my branch against main", to explain how a codebase or subsystem works, or invokes /thurview. Not for a pass/fail bug hunt. +description: Author and publish a thurview document - a guided, evidence-anchored explanation the reader opens in the browser, annotates, asks questions about, and approves or sends back. Two kinds: a review of a branch, pull request or commit range, and a code explainer of a whole codebase or one subsystem at a pinned commit. Use when the user asks to review a branch or PR, to explain or walk through a change, "review my branch against main", to explain how a codebase or subsystem works or where its design problems might be, or invokes /thurview. Not for a pass/fail bug hunt. user-invocable: true -argument-hint: "[ | --base --head | ]" +argument-hint: "[ | --base --head | explain []]" --- # thurview +There are two kinds of document, and the first decision is which one the +request asks for. + +- A **review** explains a CHANGE: a branch, a pull request, a commit range. It + has a diff, commits and an interface delta, and the reader approves it or + sends it back. Everything below describes it. +- A **code explainer** explains a CODEBASE, or one subsystem, at a single + pinned commit, so the reader can see the architecture well enough to spot + design problems themselves. It has no diff and nothing to approve, and it + states what it did not examine. Read + [Code explainer](references/code-explainer.md) and follow that instead. + The agent studies the change and writes a short document in which every claim about code is anchored to an exact file and line range at a pinned commit. thurview validates those anchors, seals a revision, and serves it in the @@ -41,9 +53,11 @@ directory; `thurview --help` shows flags and examples. $ARGUMENTS -Empty: the current branch against its up-to-date trunk. A PR number or URL: -that pull request. `--base`/`--head`: that range. Anything else: an -architecture review of that topic in the current repository. +Empty: a review of the current branch against its up-to-date trunk. A PR +number or URL: that pull request. `--base`/`--head`: that range. `explain`, or +a request to explain the codebase, a subsystem or its architecture rather than +a change: a code explainer, per +[Code explainer](references/code-explainer.md). ## Before authoring @@ -60,7 +74,8 @@ Read [Components](references/components.md) before you edit `data.yaml` or add a fenced component. Read [Lifecycle](references/lifecycle.md) for statuses, storage and thread rules. Read [Software map](references/software-map.md) before you author `map.yaml`. Read [Theme](references/theme.md) before you -write `theme.yaml`. +write `theme.yaml`. Read [Code explainer](references/code-explainer.md) when +the request is a codebase rather than a change. ## Workflow @@ -281,12 +296,17 @@ Then publish again (step 7), tell the user what changed since the previous revision in a line or two, and wait again (step 9). A republish requires zero open submitted comment threads; questions do not block. -## Architecture reviews +## Explaining a codebase rather than a change + +Do not pin the same commit as base and head to fake it. That leaves a review +whose Files, Commits and interface-delta surfaces all describe a change that +does not exist, which is a claim, not a gap. -Pin the same commit as base and head: `thurview scaffold --base HEAD --head -HEAD`. Choose sections that describe the system (data flows, state, storage, -module boundaries) and skip diff-specific ones. Scope to one subsystem. All -other steps are the same; the Files tab shows any file at head on request. +Run `thurview explain []` instead and follow +[Code explainer](references/code-explainer.md). It is the same loop - pin, +author, publish, wait, answer - over a document kind whose unit is a codebase: +no diff, no commits, no interface delta, and a Coverage tab stating what the +document reached and what it did not. ## Completion criteria diff --git a/skills/thurview/references/code-explainer.md b/skills/thurview/references/code-explainer.md new file mode 100644 index 0000000..9ecc3a5 --- /dev/null +++ b/skills/thurview/references/code-explainer.md @@ -0,0 +1,166 @@ +# Code explainer + +A **review** explains a change. An **explainer** explains a codebase, or one +subsystem of it, at a single pinned commit, so the reader can see the +architecture well enough to spot design problems themselves. + +Same engine, different unit. It shares anchors, peeks, the map, threads, +revisions and the publish → wait → answer loop. It has no diff, no commits and +no interface delta, because those are claims about a change and there is no +change. Where a review shows the interface delta, an explainer shows +**coverage**: what it examined at that commit, and what it did not. + +| Tab | Review | Explainer | +| ------------- | ---------------------------------- | --------------------------------------------- | +| Review | the walkthrough, with the delta | **Explainer**: the document, with coverage | +| Files | split diff of the changed files | absent | +| Commits | base..head | absent | +| Map | parts, marked added/changed/removed | parts at the pinned commit | +| Coverage | absent | **what the document reached, and what it did not** | +| Threads | ask, comment, decide | same, and the decision reads *Done reading* / *Send it back* | + +## When to write one + +Write an explainer when the request is about the code as it stands: "explain +this codebase", "how does the server work", "walk me through `src/graph`", +"I want to see the architecture". Write a review when the request is about a +change: a branch, a pull request, a range. + +If the request is a change, do not reach for an explainer because the change is +large. A large change is still a change. + +## The document kind, in one command + +```sh +thurview explain # the whole repository at HEAD +thurview explain src/server # one subsystem +thurview explain --commit v1.2.0 # a released commit rather than HEAD +``` + +The positional argument is a path or a glob; a bare path means that directory +and everything under it. It is the **scope**, and everything else obeys it: +`graph architecture` reports the clusters inside it, and coverage accounts for +every file inside it. A scope that matches no file at that commit is refused. + +Record `explainer.id`, `explainer.dir`, `explainer.commit`, `explainer.scope` +and `scale.filesInScope` from the output. Everywhere else the id is passed as +`--review `; that flag names a document, whichever kind it is. + +## Keeping it short without lying about it + +A review is bounded by its diff. A codebase is not, and this is the hard part: +evidence-anchored prose over a whole repository either runs unreadably long or +quietly leaves most of the system out. Prose that leaves things out silently is +misleading about architecture, which is the one thing an explainer must not be. + +So work in three layers, and let each carry what it is good at. + +1. **System — the map carries breadth.** Author `map.yaml` first, seeded from + `thurview graph architecture --review `: communities become nodes, their + files become the node's `files` globs, and the edges between communities + become edges. Every part of the scope should appear here, including the parts + the prose will not reach. See [Software map](software-map.md) for the shape. +2. **Subsystem — the prose carries depth.** Pick the parts that carry the most + structure and the most traffic, and explain those. Three to six sections. + Everything else stays on the map. +3. **File and symbol — anchors carry the proof.** Every claim gets an anchor. + The reader opens code where they want it and nowhere else. + +**Select by structure, not by taste, and say what you selected on.** The graph +gives you the basis: cluster size in files and symbols, the hub symbols of each +cluster (most referenced), and the reference counts on the edges between +clusters. Say in the document which parts you took and why they were the ones - +"the two clusters with the most traffic between them" is a reason a reader can +check. "The interesting bits" is not. + +## Coverage is derived, not claimed + +`thurview publish` accounts for every file in scope at the pinned commit and +puts one of three states on it: + +- **explained** - an anchor in the document points into the file. +- **placed** - a map node's `files` globs match it, and no anchor does. The + reader is told where it sits, not what it does. +- **not examined** - neither. + +The counts go above the document and onto the Coverage tab, and `publish` +prints them with the files it did not examine. You cannot forget to state +coverage, and you cannot overstate it: to move a file out of *not examined* you +have to actually anchor it or actually place it on the map. + +Two consequences worth planning for: + +- **An explainer without a map counts everything the prose does not anchor as + not examined.** `publish` warns when there is no map. That is a true + statement, and usually not the one you want to make: author the map. +- **A broad glob is visible.** The Coverage tab lists each map node with the + globs it owns and how many files they match, so `**/*` on one node inflates + nothing quietly. + +Coverage also states what the code graph could not read: files in languages it +does not parse (`thurview graph` covers TypeScript, JavaScript, Python, Go, +Rust and Java), and whether its file list was capped. Those files are absent +from the structure, not empty. If a large part of the scope is outside the +graph, say so in the document rather than letting the map imply the system is +smaller than it is. + +## Surface structure; do not grade it + +thurview's thesis holds here: *it does not review the code for you; it helps +you understand it fast enough to review it yourself.* An explainer exists so +the reader can **detect** design problems. That is only consistent with the +thesis if you surface structure and leave the conclusion to them. + +The test: **every fact in an explainer is a count, or a list of named things, +at the pinned commit, that the reader could re-derive with `thurview graph`.** + +Observation - write these: + +- "`src/server` is referenced from four other parts; it references one." +- "`Store` is defined in `src/db.ts` and `src/cache.ts`." +- "The API layer reaches the database layer in 14 places and the model layer in + 2; the model layer reaches the API layer in 6." +- "Nothing in the scope references `legacy/` at this commit." +- "No test file reaches this cluster." (a count of zero, stated as one) + +Judgement - never write these: + +- "This violates separation of concerns." +- "The god object here should be split." +- severities, scores, "issues found", "critical", "smell", a ranked list of + problems, or a recommendation section. + +The difference is not tone. "A module with 14 inbound dependencies" is +something the reader acts on; "an over-coupled module" is a verdict they cannot +check. When you are unsure, write the count and stop. If a structure genuinely +worries you, the honest move is a question in the document - "the two stores +both define `Session`; whether that is one concept or two is not visible from +the code" - not a finding. + +## Workflow + +1. `thurview explain []`. Note the id, the commit and the scope. +2. `thurview graph architecture --review `. This is the structure at the + pinned commit; do not re-derive it by reading directories. + `thurview graph callers ` and `tests-for ` answer the follow-ups. + `graph interfaces` and `graph impact` compare two commits and are refused. +3. Author `map.yaml` from the architecture output, covering the whole scope. + Dispatch a sub-agent for it if you have one, exactly as a review does. +4. Author `review.md` and `data.yaml` per [Document authoring](document-authoring.md), + minus the interface-delta section: an explainer has none, and declaring + `interfaces` in `data.yaml` is an error. So is `graph: base` on an anchor - + there is one commit. +5. `theme.yaml` as usual - see [Theme](theme.md). +6. `thurview publish --review `. Read `coverage` and `notExamined`. If the + split is not the one you meant, anchor or place more and publish again. +7. `thurview open --review `, then `thurview wait --review `. The loop, + the statuses and the thread rules are identical to a review; see + [Lifecycle](lifecycle.md). + +## Hand over + +In a few lines: the url, the scope and the commit, the coverage line in its own +words (including how many files were not examined), which parts you chose to +explain and on what basis, and that you are waiting for their questions. Do not +list the design problems you think you saw. The document is built so the reader +finds them. diff --git a/src/cli.ts b/src/cli.ts index 8c36965..89c658d 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -33,12 +33,15 @@ import { readJson, serverStateFile, deleteReview, + kindOf, type ReviewState, type Binding, + type DocumentKind, type Thread, type ThreadTarget, } from "./store.js"; -import { compileDocument, compileMap, type Diagnostic } from "./document/compile.js"; +import { compileDocument, compileMap, globToRegExp, type Diagnostic } from "./document/compile.js"; +import { computeCoverage, scopeGlob, scopeGraph, type Coverage } from "./coverage.js"; import type { CodeGraph } from "./graph.js"; import type { InterfaceDelta } from "./interfaces.js"; import { parseTheme, compileTheme, type CompiledTheme } from "./theme.js"; @@ -51,7 +54,7 @@ import { VERSION } from "./version.js"; const execFileP = promisify(execFile); const HERE = dirname(fileURLToPath(import.meta.url)); const DESCRIPTION = - "Guided, evidence-anchored reviews of agent-written code, read and answered in the browser"; + "Guided, evidence-anchored reviews of a change and explainers of a codebase, read and answered in the browser"; type Out = Record; function note(msg: string): void { @@ -122,6 +125,7 @@ async function reviewRow(r: ReviewState, fields: Set): Promise { const t = await readThreads(r.id); const row: Out = { id: short(r.id), + kind: kindOf(r), title: r.title, status: r.status, rev: r.revision, @@ -131,7 +135,10 @@ async function reviewRow(r: ReviewState, fields: Set): Promise { if (fields.has("all") || fields.has("binding")) row["binding"] = r.binding.kind === "pr" ? `PR #${r.binding.name}` : r.binding.name; if (fields.has("all") || fields.has("pins")) - row["pins"] = `${r.pins.base.slice(0, 12)}..${r.pins.head.slice(0, 12)}`; + row["pins"] = + kindOf(r) === "explainer" + ? r.pins.head.slice(0, 12) + : `${r.pins.base.slice(0, 12)}..${r.pins.head.slice(0, 12)}`; if (fields.has("all") || fields.has("worktree")) row["worktree"] = r.worktree; if (fields.has("all") || fields.has("inSync")) row["inSync"] = await g @@ -268,6 +275,34 @@ const TEMPLATE_THEME = `# Look of this review, derived from the reviewed project # code: { keyword: "#7c3aed", string: "#15803d", function: "#b45309", variable: "#0369a1", comment: "#9ca3af" } `; +const TEMPLATE_EXPLAIN_MD = (title: string) => `# ${title} + +**Summary** + +- The agent is still writing this explainer. The Coverage tab already states + what it has and has not examined at the pinned commit; this page offers the + new revision when the walkthrough lands. +`; +const TEMPLATE_EXPLAIN_DATA = `# Typed inputs for the explainer: actors, anchors and stores. An explainer has +# one pinned commit, so every anchor reads that commit and \`graph: base\` is an +# error. It has no interface delta: there is no change to take one from. +# +# anchors: +# dispatch: +# title: where a request picks its handler +# peek: { file: src/server/router.ts, from: 41, to: 58 } +actors: {} +anchors: {} +stores: {} +`; +const TEMPLATE_EXPLAIN_MAP = `# The structure of the code at the pinned commit: systems, containers, +# components, code. The map carries breadth so the prose can carry depth, and a +# node's \`files\` globs are what tell the Coverage tab a file was at least placed. +# Seed it from \`thurview graph architecture\`. +nodes: [] +edges: [] +`; + // ---- commands ---- const SPECS: Record< @@ -292,8 +327,26 @@ const SPECS: Record< "thurview scaffold --update --review ", ], }, + explain: { + description: + "Create a code explainer pinned to one commit: a whole codebase, or one subsystem of it", + args: "[]", + flags: { + commit: { kind: "string", help: "commit to pin (default: HEAD)" }, + title: { kind: "string", help: "initial title" }, + new: { kind: "boolean", help: "create another explainer even if one matches the scope" }, + update: { kind: "boolean", help: "re-pin an existing explainer to a new commit" }, + review: { kind: "string", help: "explainer to update (id prefix)" }, + }, + examples: [ + "thurview explain", + "thurview explain src/server", + "thurview explain --commit v1.2.0", + "thurview explain --update --review ", + ], + }, info: { - description: "Reviews bound to this worktree (or every review with --all)", + description: "Reviews and explainers bound to this worktree (or all of them with --all)", flags: { all: { kind: "boolean", help: "every review, not only this worktree" }, fields: { @@ -438,6 +491,7 @@ async function homeView(): Promise { help: [ "Run `thurview scaffold` to create a review of the current branch", "Run `thurview scaffold --pr ` for a pull request", + "Run `thurview explain []` to explain the codebase at HEAD instead", ], }; const reviews = []; @@ -636,6 +690,129 @@ const commands: Record Promise> = { }; }, + async explain(args) { + const p = parseFlags("explain", args, spec("explain").flags, 1); + const cwd = process.cwd(); + const worktree = await worktreeOf(cwd); + if (!worktree) + throw new AxiError("not inside a git repository", "VALIDATION_ERROR", [ + "Run `thurview explain` inside the source worktree", + ]); + const existing = + bool(p, "update") || str(p, "review") ? await resolveReview(str(p, "review")) : null; + if (existing && kindOf(existing) !== "explainer") + throw new AxiError( + `${short(existing.id)} is a review, not an explainer`, + "VALIDATION_ERROR", + [ + "Run `thurview scaffold --update --review ` to re-pin a review", + "Run `thurview explain` with no --review to start an explainer", + ], + ); + const scope = scopeGlob(p.positional[0] ?? existing?.binding.name); + let commit: string; + try { + commit = await g.revParse(worktree, str(p, "commit") ?? "HEAD"); + } catch (e) { + throw new AxiError((e as Error).message, "VALIDATION_ERROR", [ + "Pass a resolvable ref: `thurview explain --commit `", + ]); + } + // A scope that matches nothing is a typo, and an explainer of nothing would + // still publish and still state honest-looking coverage of zero files. + const files = await g.listFiles(worktree, commit); + const inScope = scope === "**" ? files : files.filter((f) => globToRegExp(scope).test(f)); + if (!inScope.length) + throw new AxiError( + `no file matches "${scope}" at ${commit.slice(0, 12)}`, + "VALIDATION_ERROR", + [ + "Pass a path that exists at that commit: `thurview explain src/server`", + "Run `thurview explain` with no scope for the whole repository", + ], + ); + const binding: Binding = { kind: "codebase", name: scope }; + const defaultTitle = + scope === "**" ? worktree.split("/").pop() || "Codebase" : scope.replace(/\/\*\*$/, ""); + let review: ReviewState; + let reused = false; + if (existing) { + review = existing; + review.pins = { base: commit, head: commit }; + review.binding = binding; + if (str(p, "title")) review.title = str(p, "title")!; + await writeReview(review); + } else { + const match = bool(p, "new") + ? [] + : (await reviewsFor(worktree)).filter( + (r) => + kindOf(r) === "explainer" && + r.binding.name === scope && + r.status !== "accepted" && + r.status !== "closed", + ); + if (match.length) { + review = match[0]!; + reused = true; + review.pins = { base: commit, head: commit }; + await writeReview(review); + } else { + const id = newId(); + review = { + schema: SCHEMA, + id, + kind: "explainer", + title: str(p, "title") || defaultTitle, + worktree, + repoRoot: worktree, + binding, + pins: { base: commit, head: commit }, + status: "draft", + revision: 0, + dismissed: false, + createdAt: now(), + updatedAt: now(), + }; + await mkdir(reviewDir(id), { recursive: true }); + await writeText(join(reviewDir(id), "review.md"), TEMPLATE_EXPLAIN_MD(review.title)); + await writeText(join(reviewDir(id), "data.yaml"), TEMPLATE_EXPLAIN_DATA); + await writeText(join(reviewDir(id), "map.yaml"), TEMPLATE_EXPLAIN_MAP); + await writeText(join(reviewDir(id), "theme.yaml"), TEMPLATE_THEME); + await writeReview(review); + } + } + const dir = reviewDir(review.id); + return { + explainer: { + id: short(review.id), + uuid: review.id, + kind: "explainer", + title: review.title, + status: review.status, + rev: review.revision, + scope, + commit, + worktree, + reused, + dir, + }, + files: { + document: join(dir, "review.md"), + data: join(dir, "data.yaml"), + map: join(dir, "map.yaml"), + theme: join(dir, "theme.yaml"), + }, + scale: { filesInScope: inScope.length }, + guidance: await guidanceFiles(worktree), + help: [ + `Run \`thurview graph architecture --review ${short(review.id)}\` for the clusters, their hubs and the links between them`, + `Author ${join(dir, "map.yaml")} first: it carries the breadth the prose cannot`, + `Edit ${join(dir, "review.md")} and data.yaml, then run \`thurview publish --review ${short(review.id)}\``, + ], + }; + }, + async info(args) { const p = parseFlags("info", args, spec("info").flags); const fields = new Set((str(p, "fields") ?? "").split(",").filter(Boolean)); @@ -716,21 +893,27 @@ const commands: Record Promise> = { } } const themeName = theme ? await registerTheme(theme.shiki) : undefined; + const kind = kindOf(review); + // An explainer has one pinned commit, so there is no delta to derive: the + // panel above its document states coverage instead. let interfaces: InterfaceDelta | null = null; - try { - interfaces = await deltaFor(review); - } catch (e) { - diags.push({ - level: "warning", - file: "review.md", - message: `the interface delta is unavailable: ${(e as Error).message}`, - }); + if (kind === "review") { + try { + interfaces = await deltaFor(review); + } catch (e) { + diags.push({ + level: "warning", + file: "review.md", + message: `the interface delta is unavailable: ${(e as Error).message}`, + }); + } } const doc = await compileDocument({ cwd: review.worktree, pins: review.pins, reviewMd, dataYaml: dataYaml ?? "", + kind, ...(themeName ? { themeName } : {}), interfaces, }); @@ -742,6 +925,7 @@ const commands: Record Promise> = { pins: review.pins, mapYaml, anchors: doc.anchors, + kind, }); diags.push(...m.diagnostics); map = m.map; @@ -764,7 +948,57 @@ const commands: Record Promise> = { ], }; } + // Coverage is derived, not claimed: every file in scope at the pinned commit + // is accounted for, so the reader is told what the prose never reached. + let coverage: Coverage | null = null; + if (kind === "explainer") { + try { + const graph = await import("./graph.js"); + const g0 = await graph.graphAt(review.worktree, review.pins.head, dir); + coverage = computeCoverage({ + commit: review.pins.head, + scope: review.binding.name, + allFiles: await g.listFiles(review.worktree, review.pins.head), + graph: g0, + anchored: Object.values(doc.document.anchors) + .map((a) => a.peek?.file) + .filter((f): f is string => !!f), + owners: (map?.head.nodes ?? []) + .filter((n) => n.files?.length) + .map((n) => ({ node: n.id, globs: n.files! })), + }); + } catch (e) { + diags.push({ + level: "error", + file: "review.md", + message: `coverage is unavailable, so the explainer cannot state what it skipped: ${(e as Error).message}`, + }); + process.exitCode = 1; + return { + error: "publish failed: coverage could not be derived", + code: "PUBLISH_FAILED", + diagnostics: diags.map((d) => ({ + level: d.level, + file: d.file, + line: d.line ?? "", + message: d.message, + })), + help: [`Run \`thurview publish --review ${short(review.id)}\` again`], + }; + } + } const warnings: string[] = []; + if (kind === "explainer" && !map) + warnings.push( + "this explainer has no map, so every file it does not anchor counts as not examined; author map.yaml to place the rest", + ); + if (review.binding.kind === "codebase") { + const tip = await g.revParse(review.worktree, "HEAD").catch(() => null); + if (tip && tip !== review.pins.head) + warnings.push( + `HEAD has moved past the pinned commit; run \`thurview explain --update --review ${short(review.id)}\` to re-pin`, + ); + } if (review.binding.kind === "branch") { const tip = await g.revParse(review.worktree, review.binding.name).catch(() => null); if (tip && tip !== review.pins.head) @@ -782,6 +1016,7 @@ const commands: Record Promise> = { await writeJson(join(rdir, "document.json"), doc.document); await writeJson(join(rdir, "map.json"), map); await writeJson(join(rdir, "changes.json"), changes); + await writeJson(join(rdir, "coverage.json"), coverage); if (themeYaml !== null) await cp(join(dir, "theme.yaml"), join(rdir, "theme.yaml")); await writeJson(join(rdir, "theme.json"), theme); await writeJson(join(rdir, "meta.json"), { @@ -789,6 +1024,7 @@ const commands: Record Promise> = { at: now(), title: doc.document.title, pins: review.pins, + kind, hasMap: !!map, theme: theme?.name ?? "default", }); @@ -810,15 +1046,26 @@ const commands: Record Promise> = { const out: Out = { published: { id: short(review.id), + kind, rev: n, title: review.title, status: review.status, map: !!map, - interfaces: doc.document.interfaces?.verdict ?? "(unavailable)", + ...(kind === "explainer" + ? { coverage: coverage ? coverage.verdict : "(unavailable)" } + : { interfaces: doc.document.interfaces?.verdict ?? "(unavailable)" }), theme: theme?.name ?? "default", url: url ?? "(server not running)", }, }; + if (coverage) + out["notExamined"] = { + files: coverage.states.uncovered, + first: coverage.uncovered.slice(0, 8), + byPart: coverage.clusters + .filter((c) => c.uncovered.length) + .map((c) => ({ part: c.label, files: c.uncovered.length })), + }; if (rows.length) out["diagnostics"] = rows; if (warnings.length) out["warnings"] = warnings; out["help"] = [ @@ -987,6 +1234,15 @@ const commands: Record Promise> = { throw new AxiError("--graph must be head or base", "VALIDATION_ERROR", help); const graph = await import("./graph.js"); const review = await resolveReview(str(p, "review")); + if (kindOf(review) === "explainer" && (sub === "interfaces" || sub === "impact")) + throw new AxiError( + `graph ${sub} compares two commits; an explainer is pinned to one`, + "VALIDATION_ERROR", + [ + `Run \`thurview graph architecture --review ${short(review.id)}\` for the structure at that commit`, + `Run \`thurview graph callers --review ${short(review.id)}\` to follow one symbol`, + ], + ); const dir = reviewDir(review.id); const at = (commit: string) => graph.graphAt(review.worktree, commit, dir); if (sub === "callers" || sub === "tests-for") { @@ -1055,6 +1311,24 @@ const commands: Record Promise> = { ], }; } + // An explainer is scoped to a path, so its structure is that path's, not the + // repository's: the same bound the Coverage tab accounts for. + if (kindOf(review) === "explainer") { + const scope = review.binding.name; + const g0 = scopeGraph(head, scope); + const { diff: _diff, truncated, ...rest } = graph.architecture(g0, g0); + return { + commit: short(head.commit), + scope, + languages: pins.languages, + truncated: truncated.head, + ...rest, + help: [ + "Seed map.yaml nodes from communities, their `files` from a community's files, and edges from edges", + "A file in no community is outside the languages the graph reads; `thurview publish` counts those", + ], + }; + } return { ...pins, ...graph.architecture(base, head), @@ -1331,6 +1605,7 @@ function topLevelHelp(): string { examples: [ "thurview", "thurview scaffold", + "thurview explain src/server", "thurview publish --view files --open", "thurview wait", 'thurview threads reply --body ""', diff --git a/src/coverage.ts b/src/coverage.ts new file mode 100644 index 0000000..411b76d --- /dev/null +++ b/src/coverage.ts @@ -0,0 +1,249 @@ +/** + * What an explainer examined, and what it did not. + * + * A review is bounded by its diff, so its own scope is the reader's proof that + * nothing was skipped. A codebase has no such bound: prose over a whole repo + * either runs unreadably long or quietly leaves most of the system out, and an + * explanation that leaves things out silently is misleading about architecture. + * + * So coverage is DERIVED here rather than claimed in prose. Every file at the + * pinned commit inside the scope is accounted for, in one of three states, and + * the reader is shown the ones the document never touched. Everything on this + * record is a count or a list of named things at that commit: the reader can + * re-derive any of it with `thurview graph architecture`. Nothing here grades + * the code - no scores, no thresholds, no severity. Where a number invites a + * conclusion, drawing it is the reader's job. + */ +import { globToRegExp } from "./document/compile.js"; +import { architecture, isNestedNonMethod, type CodeGraph, type Sym } from "./graph.js"; + +/** How a file at the pinned commit is accounted for. */ +export type FileState = + /** an anchor in the document points into it: prose covers it */ + | "explained" + /** only a map node owns it: the reader is told where it sits, not what it does */ + | "placed" + /** neither: named here and nowhere else */ + | "uncovered"; + +export interface ClusterCoverage { + id: string; + label: string; + files: number; + symbols: number; + /** the most referenced symbols in the cluster, the names a reader knows it by */ + hubs: string[]; + explained: string[]; + placed: string[]; + uncovered: string[]; +} + +export interface Coverage { + /** one line stating the bound, shown above the document and printed by publish */ + verdict: string; + commit: string; + /** the path glob the explainer is scoped to; `**` is the whole repository */ + scope: string; + files: { + total: number; + /** files in a language the code graph parses */ + inGraph: number; + /** everything else: config, docs, other languages */ + outsideGraph: number; + }; + states: { explained: number; placed: number; uncovered: number }; + clusters: ClusterCoverage[]; + /** every file in scope the document never examined, clustered or not */ + uncovered: string[]; + /** in-scope files the code graph cannot read, so they are in no cluster above */ + unclustered: { file: string; state: FileState }[]; + /** references between clusters at the pinned commit */ + links: { from: string; to: string; references: number; bothWays: boolean }[]; + /** names defined in more than one cluster, most-spread first */ + sharedNames: { name: string; clusters: string[]; files: string[] }[]; + sharedNamesTotal: number; + /** files the graph cannot read, grouped by extension */ + outsideGraph: { extension: string; files: number }[]; + /** map nodes that own files, so an over-broad glob is visible rather than silent */ + owners: { node: string; globs: string[]; files: number }[]; + /** references the graph could not place, and whether its file list was capped */ + unresolved: number; + truncated: boolean; +} + +const MODULE = ""; +const SHARED_NAMES_SHOWN = 20; +/** enough for a reader to act on; the counts beside them are never capped */ +const FILES_LISTED = 300; + +/** + * Normalise what a reader types as a scope into a glob. A bare path is the + * directory and everything under it, which is what `thurview explain src/server` + * means to the person who typed it. + */ +export function scopeGlob(scope: string | undefined): string { + const s = (scope ?? "") + .trim() + .replace(/^\.\/+/, "") + .replace(/\/+$/, ""); + if (!s || s === "." || s === "**") return "**"; + return /[*?]/.test(s) ? s : `${s}/**`; +} + +/** The subset of a graph whose files match `glob`, with the edges that survive it. */ +export function scopeGraph(g: CodeGraph, glob: string): CodeGraph { + if (glob === "**") return g; + const re = globToRegExp(glob); + const files = g.files.filter((f) => re.test(f)); + const symbols = g.symbols.filter((s) => re.test(s.file)); + const kept = new Set(symbols.map((s) => s.id)); + return { + ...g, + files, + symbols, + edges: g.edges.filter((e) => kept.has(e.from) && kept.has(e.to)), + }; +} + +function extensionOf(path: string): string { + const name = path.split("/").pop() ?? path; + const dot = name.lastIndexOf("."); + return dot > 0 ? name.slice(dot + 1) : "(none)"; +} + +export interface CoverageInput { + commit: string; + scope: string; + /** every path at the pinned commit */ + allFiles: string[]; + /** the code graph at that commit, before scoping */ + graph: CodeGraph; + /** files an anchor's peek points into */ + anchored: Iterable; + /** map nodes and the globs they own */ + owners: { node: string; globs: string[] }[]; +} + +/** Account for every file in scope at the pinned commit. */ +export function computeCoverage(input: CoverageInput): Coverage { + const glob = scopeGlob(input.scope); + const inScope = glob === "**" ? () => true : (f: string) => globToRegExp(glob).test(f); + const files = input.allFiles.filter(inScope); + const graph = scopeGraph(input.graph, glob); + const inGraph = new Set(graph.files); + + const explained = new Set([...input.anchored].filter(inScope)); + const owned = new Map(); + const owners = input.owners.map(({ node, globs }) => { + const res = globs.map(globToRegExp); + const hits = files.filter((f) => res.some((re) => re.test(f))); + for (const f of hits) owned.set(f, [...(owned.get(f) ?? []), node]); + return { node, globs, files: hits.length }; + }); + const stateOf = (f: string): FileState => + explained.has(f) ? "explained" : owned.has(f) ? "placed" : "uncovered"; + + const arch = architecture(graph, graph); + // Two clusters can share a main directory, and a duplicate label makes the + // whole page ambiguous: keep them apart by their id. + const seen = new Map(); + for (const c of arch.communities) seen.set(c.label, (seen.get(c.label) ?? 0) + 1); + const labelOf = (c: { id: string; label: string }) => + (seen.get(c.label) ?? 0) > 1 ? `${c.label} (${c.id})` : c.label; + const clusters: ClusterCoverage[] = arch.communities.map((c) => { + const buckets: Record = { explained: [], placed: [], uncovered: [] }; + for (const f of c.files) buckets[stateOf(f)].push(f); + return { + id: c.id, + label: labelOf(c), + files: c.files.length, + symbols: c.symbols, + hubs: c.hubs, + explained: buckets.explained, + placed: buckets.placed, + uncovered: buckets.uncovered, + }; + }); + + const both = new Set(arch.edges.map((e) => `${e.from} -> ${e.to}`)); + const links = arch.edges.map((e) => ({ + from: e.from, + to: e.to, + references: e.references, + bothWays: both.has(`${e.to} -> ${e.from}`), + })); + + const clusterOf = new Map(); + for (const c of arch.communities) for (const f of c.files) clusterOf.set(f, c.id); + // Only definitions a consumer outside the file could name. A definition nested + // inside another - an `onclick` handler, a local helper - is scoped to its file, + // so it cannot be the same concept living in two parts, only the same word. + const byName = new Map(); + for (const s of graph.symbols) { + if (s.name === MODULE || isNestedNonMethod(s)) continue; + byName.set(s.name, [...(byName.get(s.name) ?? []), s]); + } + const spread: Coverage["sharedNames"] = []; + for (const [name, syms] of byName) { + const cs = [...new Set(syms.map((s) => clusterOf.get(s.file)).filter(Boolean))] as string[]; + if (cs.length < 2) continue; + spread.push({ name, clusters: cs.sort(), files: [...new Set(syms.map((s) => s.file))].sort() }); + } + spread.sort((a, b) => b.clusters.length - a.clusters.length || a.name.localeCompare(b.name)); + + const byExt = new Map(); + for (const f of files) + if (!inGraph.has(f)) { + const e = extensionOf(f); + byExt.set(e, (byExt.get(e) ?? 0) + 1); + } + + const states = { explained: 0, placed: 0, uncovered: 0 }; + for (const f of files) states[stateOf(f)]++; + + const record: Coverage = { + verdict: "", + commit: input.commit, + scope: glob, + files: { + total: files.length, + inGraph: graph.files.length, + outsideGraph: files.length - graph.files.length, + }, + states, + clusters, + uncovered: files.filter((f) => stateOf(f) === "uncovered").slice(0, FILES_LISTED), + unclustered: files + .filter((f) => !inGraph.has(f)) + .slice(0, FILES_LISTED) + .map((f) => ({ file: f, state: stateOf(f) })), + links, + sharedNames: spread.slice(0, SHARED_NAMES_SHOWN), + sharedNamesTotal: spread.length, + outsideGraph: [...byExt] + .map(([extension, n]) => ({ extension, files: n })) + .sort((a, b) => b.files - a.files || a.extension.localeCompare(b.extension)), + owners: owners.sort((a, b) => b.files - a.files || a.node.localeCompare(b.node)), + unresolved: input.graph.unresolved, + truncated: input.graph.truncated, + }; + record.verdict = coverageVerdict(record); + return record; +} + +/** One line stating coverage, for the panel above the document and for the CLI. */ +function coverageVerdict(c: Coverage): string { + const { explained, placed, uncovered } = c.states; + const where = c.scope === "**" ? "the repository" : c.scope; + const parts = [ + `${c.files.total} file${c.files.total === 1 ? "" : "s"} at ${c.commit.slice(0, 12)} in ${where}`, + `${explained} anchored in the document`, + `${placed} placed on the map only`, + `${uncovered} not examined`, + ]; + const tail = + c.files.outsideGraph > 0 + ? ` ${c.files.outsideGraph} of them are outside the languages the code graph reads.` + : ""; + return `${parts.join(", ")}.${tail}`; +} diff --git a/src/document/compile.ts b/src/document/compile.ts index b2f6709..3e42200 100644 --- a/src/document/compile.ts +++ b/src/document/compile.ts @@ -11,6 +11,7 @@ import { import { highlightLines, languageFor } from "../highlight.js"; import { parseDocument, type RawBlock } from "./parse.js"; import { withVerdict, type InterfaceDelta } from "../interfaces.js"; +import type { DocumentKind } from "../store.js"; import { DataSchema, SequenceSchema, @@ -115,6 +116,8 @@ export interface CompileInput { pins: { base: string; head: string }; reviewMd: string; dataYaml: string; + /** review (a change) or explainer (a codebase at one commit); default review */ + kind?: DocumentKind; /** registered highlighter theme name (default skin when omitted) */ themeName?: string; /** the derived interface delta; null when the code graph could not be built */ @@ -158,6 +161,13 @@ export async function compileDocument(input: CompileInput): Promise<{ : { actors: {}, anchors: {}, stores: {}, interfaces: {} }; if (!dataParsed.ok) for (const m of dataParsed.errors) err("data.yaml", m); + const kind: DocumentKind = input.kind ?? "review"; + if (kind === "explainer" && Object.keys(data.interfaces).length) + err( + "data.yaml", + "an explainer has no interface delta: it explains a codebase at one commit, not a change", + ); + const parsed = parseDocument(input.reviewMd); if (!parsed.title) err("review.md", "the document needs an H1 title"); @@ -177,7 +187,12 @@ export async function compileDocument(input: CompileInput): Promise<{ ...(a.detail ? { detail: a.detail } : {}), ...(a.map ? { map: a.map } : {}), }; - if (a.peek) { + if (a.peek && kind === "explainer" && a.peek.graph === "base") { + err( + "data.yaml", + `anchor ${id}: an explainer has one pinned commit, so \`graph: base\` has no meaning`, + ); + } else if (a.peek) { const text = await fileAt(a.peek.graph, a.peek.file); if (text === null) { err( @@ -241,19 +256,31 @@ export async function compileDocument(input: CompileInput): Promise<{ if (block) blocks.push(block); } - const interfaces = compileInterfaces({ - delta: input.interfaces ?? null, - declared: data.interfaces, - anchors, - changes, - used, - err, - warn, - }); + const interfaces = + kind === "explainer" + ? null + : compileInterfaces({ + delta: input.interfaces ?? null, + declared: data.interfaces, + anchors, + changes, + used, + err, + warn, + }); for (const id of Object.keys(anchors)) if (!used.has(id)) warn("data.yaml", `anchor "${id}" is defined but never used`); + // A review may publish a stub so the reader can read the diff while the + // walkthrough is written. An explainer has no diff to read in the meantime, + // so an unanchored one is only prose about code the reader cannot check. + if (kind === "explainer" && !used.size) + err( + "review.md", + "an explainer needs at least one anchored claim: link prose to code with [text](anchor:)", + ); + const errors = diags.filter((d) => d.level === "error"); if (errors.length) return { document: null, diagnostics: diags, anchors }; return { @@ -607,6 +634,7 @@ export async function compileMap(input: { pins: { base: string; head: string }; mapYaml: string; anchors: Record; + kind?: DocumentKind; }): Promise<{ map: CompiledMap | null; diagnostics: Diagnostic[] }> { const diags: Diagnostic[] = []; const err = (message: string) => diags.push({ level: "error", file: "map.yaml", message }); @@ -624,6 +652,8 @@ export async function compileMap(input: { return { map: null, diagnostics: diags }; } const m: MapFile = r.value; + if (input.kind === "explainer" && m.base) + err("an explainer has one pinned commit, so there is no base structure to compare against"); const validateGraph = async ( g: { nodes: MapNode[]; edges: MapEdge[] }, graph: "head" | "base", diff --git a/src/graph.ts b/src/graph.ts index 9d828ef..91fcf69 100644 --- a/src/graph.ts +++ b/src/graph.ts @@ -157,7 +157,7 @@ const SKIP = /(^|\/)(node_modules|dist|build|vendor|target|\.git)\//; /** A definition nested inside another (its qualified name has a dot) that isn't a class * method has no meaning outside the file that scopes it, so it can't be a cross-file * resolution target picked by the repo-wide-unique fallback. */ -function isNestedNonMethod(s: Sym): boolean { +export function isNestedNonMethod(s: Sym): boolean { const qualified = s.id.slice(s.file.length + 1).replace(/#\d+$/, ""); return qualified.includes(".") && s.kind !== "method"; } @@ -489,6 +489,17 @@ export interface Architecture { truncated: { base: boolean; head: boolean }; } +/** A fixed-seed PRNG, so a pinned commit always clusters the same way. */ +function seeded(seed: number): () => number { + let a = seed >>> 0; + return () => { + a = (a + 0x6d2b79f5) >>> 0; + let t = Math.imul(a ^ (a >>> 15), 1 | a); + t = (t + Math.imul(t ^ (t >>> 7), 61 | t)) ^ t; + return ((t ^ (t >>> 14)) >>> 0) / 4294967296; + }; +} + function fileEdges(g: CodeGraph): Map { const byId = new Map(g.symbols.map((s) => [s.id, s.file])); const out = new Map(); @@ -523,7 +534,12 @@ export function architecture(base: CodeGraph, head: CodeGraph): Architecture { if (g.hasEdge(a, b)) g.updateEdgeAttribute(a, b, "weight", (x: number) => x + w); else g.addEdge(a, b, { weight: w }); } - const membership: Record = g.order ? louvain(g, { getEdgeWeight: "weight" }) : {}; + // Louvain is randomised, and a partition that moves between runs would move + // every count derived from it at the same pinned commit. Seed it so the same + // commit always yields the same structure. + const membership: Record = g.order + ? louvain(g, { getEdgeWeight: "weight", rng: seeded(0x7c1f9e3d) }) + : {}; const groups = new Map(); for (const [file, c] of Object.entries(membership)) { const list = groups.get(c) ?? []; diff --git a/src/server/server.ts b/src/server/server.ts index 7747153..e8bc13c 100644 --- a/src/server/server.ts +++ b/src/server/server.ts @@ -81,10 +81,11 @@ async function findReview(idOrPrefix: string) { async function revisionData(id: string, n: number) { const dir = revisionDir(id, n); - const [document, map, changes, meta, theme] = await Promise.all([ + const [document, map, changes, coverage, meta, theme] = await Promise.all([ readJson(join(dir, "document.json")), readJson(join(dir, "map.json")), readJson(join(dir, "changes.json")), + readJson(join(dir, "coverage.json")), readJson(join(dir, "meta.json")), readJson(join(dir, "theme.json")), ]); @@ -92,6 +93,7 @@ async function revisionData(id: string, n: number) { document, map, changes: changes ?? [], + coverage, meta, theme: theme ? { name: theme.name, source: theme.source, css: theme.css } : null, }; @@ -197,7 +199,7 @@ export async function startServer( const n = Number(url.searchParams.get("revision") ?? review.revision); const data = review.revision ? await revisionData(id, n) - : { document: null, map: null, changes: [], meta: null }; + : { document: null, map: null, changes: [], coverage: null, meta: null }; const threads = await readThreads(id); return { review, diff --git a/src/store.ts b/src/store.ts index 1d6df29..10cce4a 100644 --- a/src/store.ts +++ b/src/store.ts @@ -9,9 +9,18 @@ export const SCHEMA = 1; export type ReviewStatus = "draft" | "awaiting-review" | "awaiting-agent-updates" | "accepted" | "closed"; +/** + * What a document explains. A review explains a CHANGE, so its pins are a range + * and the diff, the commits and the interface delta all mean something. An + * explainer explains a CODEBASE at one commit: same anchors, same peeks, same + * threads, no range — so the surfaces that describe a range are absent rather + * than rendered empty. Records written before this field read as reviews. + */ +export type DocumentKind = "review" | "explainer"; + export interface Binding { - kind: "branch" | "pr" | "range"; - /** branch name, pr number, or "base..head" */ + kind: "branch" | "pr" | "range" | "codebase"; + /** branch name, pr number, "base..head", or the path scope of an explainer */ name: string; url?: string; } @@ -19,6 +28,8 @@ export interface Binding { export interface ReviewState { schema: number; id: string; + /** absent on records written before explainers existed, which are reviews */ + kind?: DocumentKind; title: string; worktree: string; repoRoot: string; @@ -125,6 +136,11 @@ export async function writeJson(path: string, value: unknown): Promise { await writeAtomic(path, JSON.stringify(value, null, 2) + "\n"); } +/** The kind of a stored document, defaulting old records to a review. */ +export function kindOf(r: Pick): DocumentKind { + return r.kind === "explainer" ? "explainer" : "review"; +} + export async function readReview(id: string): Promise { return readJson(join(reviewDir(id), "review.json")); } diff --git a/src/ui/api.ts b/src/ui/api.ts index 2d2f18b..cbfdc5f 100644 --- a/src/ui/api.ts +++ b/src/ui/api.ts @@ -1,5 +1,6 @@ import type { ReviewState, Thread, Decision, ThreadTarget } from "../store.js"; import type { CompiledDocument, CompiledMap } from "../document/compile.js"; +import type { Coverage } from "../coverage.js"; import type { FileDiff } from "../diff.js"; import type { ChangedFile, Commit } from "../git.js"; import type { SymbolDef } from "../symbols.js"; @@ -10,6 +11,8 @@ export interface Payload { document: CompiledDocument | null; map: CompiledMap | null; changes: ChangedFile[]; + /** explainers only: what the document examined at the pinned commit, and what it did not */ + coverage: Coverage | null; meta: { revision: number; at: string; title: string; hasMap: boolean; theme?: string } | null; theme: { name: string; source?: string; css: string } | null; threads: Thread[]; diff --git a/src/ui/app.css b/src/ui/app.css index bc41cbd..5fabf5d 100644 --- a/src/ui/app.css +++ b/src/ui/app.css @@ -1756,3 +1756,177 @@ kbd { overflow: visible; } } + +/* ---- Coverage (explainers) ---- + Deliberately neutral: covered/uncovered is a fact about the DOCUMENT, not a + grade on the code, so nothing here borrows the ok/warn/del palette that the + review views use to mark a change. */ +.cov-intro, +.cov-section { + max-width: 1000px; + margin: 0 auto 26px; + padding: 0 24px; +} +.cov-intro { + padding-top: 24px; +} +.cov-intro h2, +.cov-section h3 { + font-family: var(--font-mono); + font-size: 15px; + margin: 0 0 6px; +} +.cov-section h3 { + font-size: 11px; + font-weight: 700; + text-transform: uppercase; + letter-spacing: 0.08em; + color: var(--muted); + margin-bottom: 10px; +} +.cov-section h4 { + font-family: var(--font-mono); + font-size: 12px; + margin: 18px 0 6px; +} +.cov-intro p, +.cov-section p { + margin: 0 0 12px; + color: var(--fg2); + font-size: 13px; + line-height: 1.55; + max-width: 72ch; +} +.cov-bar { + display: flex; + height: 10px; + border: 1px solid var(--line); + border-radius: var(--radius); + overflow: hidden; + background: var(--bg2); +} +.cov-seg.explained { + background: var(--accent); +} +.cov-seg.placed { + background: var(--accent-bg); +} +.cov-seg.uncovered { + background: var(--bg3); +} +.cov-key { + display: flex; + flex-wrap: wrap; + gap: 14px; + margin: 8px 0 16px; + font-size: 12px; + color: var(--fg2); +} +.cov-key-item { + display: inline-flex; + align-items: center; + gap: 6px; +} +.cov-dot { + width: 9px; + height: 9px; + border: 1px solid var(--line); + border-radius: 2px; +} +.cov-dot.explained { + background: var(--accent); +} +.cov-dot.placed { + background: var(--accent-bg); +} +.cov-dot.uncovered { + background: var(--bg3); +} +.cov-facts { + display: flex; + flex-wrap: wrap; + gap: 10px; +} +.cov-fact { + border: 1px solid var(--line); + border-radius: var(--radius); + padding: 7px 10px; + min-width: 0; + font-size: 12px; +} +.cov-fact b { + display: block; + font-family: var(--font-mono); + font-size: 13px; +} +.cov-cluster { + border: 1px solid var(--line); + border-radius: var(--radius); + padding: 10px 12px; + margin-bottom: 10px; + background: var(--bg2); +} +.cov-cluster-head { + display: flex; + flex-wrap: wrap; + align-items: center; + gap: 8px; + font-size: 12px; +} +.cov-cluster-head .spacer { + flex: 1; +} +.cov-label { + font-weight: 600; +} +.cov-hubs { + font-size: 12px; + margin-top: 6px; +} +.cov-hubs code { + margin-right: 6px; +} +.cov-files { + display: flex; + flex-wrap: wrap; + gap: 6px; + margin-top: 8px; +} +.cov-file { + font-size: 11px; + color: var(--muted); + border: 1px dashed var(--line); + border-radius: var(--radius); + padding: 1px 5px; +} +.cov-b-explained { + color: var(--accent); + border-color: var(--accent); +} +.cov-links, +.cov-shared { + display: flex; + flex-direction: column; + border: 1px solid var(--line); + border-radius: var(--radius); + overflow: hidden; +} +.cov-link { + display: flex; + flex-wrap: wrap; + align-items: baseline; + gap: 8px; + padding: 6px 10px; + font-size: 12px; + border-top: 1px solid var(--line2); + min-width: 0; +} +.cov-link:first-child { + border-top: 0; +} +.cov-link:hover { + background: var(--bg3); +} +.cov-link > .muted { + overflow-wrap: anywhere; +} diff --git a/src/ui/app.ts b/src/ui/app.ts index 57d0e8a..e0caaa2 100644 --- a/src/ui/app.ts +++ b/src/ui/app.ts @@ -1,19 +1,32 @@ import { api } from "./api.js"; import { h, append, clear, dialog, timeAgo } from "./dom.js"; -import { state, on, emit, readHash, navigate, isTerminal, NARROW, type View } from "./state.js"; +import { + state, + on, + emit, + readHash, + navigate, + isTerminal, + kind, + view, + VIEWS, + NARROW, + type View, +} from "./state.js"; import { renderPeek } from "./code.js"; import { renderThreadsPanel, submitDialog, reload } from "./threads.js"; import { renderReview } from "./views/review.js"; import { renderFiles } from "./views/files.js"; import { renderCommits } from "./views/commits.js"; import { renderMap } from "./views/map.js"; +import { renderCoverage } from "./views/coverage.js"; const app = document.getElementById("app")!; async function home(): Promise { clear(app); const reviews = await api.reviews(); - const el = h("div", { class: "home" }, h("h2", null, "Reviews")); + const el = h("div", { class: "home" }, h("h2", null, "Reviews and explainers")); const active = reviews.filter((r) => !r.dismissed); const dismissed = reviews.filter((r) => r.dismissed); const item = (r: (typeof reviews)[number]) => @@ -23,16 +36,16 @@ async function home(): Promise { h("span", { class: "t" }, r.title), h("span", { class: `badge ${statusClass(r.status)}` }, r.status), r.openThreads ? h("span", { class: "badge accent" }, `${r.openThreads} open`) : null, - h( - "span", - { class: "muted mono", style: { fontSize: "12px" } }, - r.binding.kind === "pr" ? `PR #${r.binding.name}` : r.binding.name, - ), + h("span", { class: "muted mono", style: { fontSize: "12px" } }, bindingLabel(r)), h("span", { class: "muted", style: { fontSize: "12px" } }, timeAgo(r.updatedAt)), ); if (!active.length) el.appendChild( - h("div", { class: "empty-state" }, "No reviews. Ask your agent to scaffold and publish one."), + h( + "div", + { class: "empty-state" }, + "Nothing published yet. Ask your agent for a review of a change, or an explainer of the codebase.", + ), ); active.forEach((r) => el.appendChild(item(r))); if (dismissed.length) { @@ -42,6 +55,12 @@ async function home(): Promise { app.appendChild(el); } +function bindingLabel(r: { binding: { kind: string; name: string } }): string { + if (r.binding.kind === "pr") return `PR #${r.binding.name}`; + if (r.binding.kind === "codebase") return r.binding.name === "**" ? "codebase" : r.binding.name; + return r.binding.name; +} + function statusClass(s: string): string { return s === "accepted" ? "ok" @@ -93,12 +112,16 @@ function renderTopbar(): void { const r = d.review; const pending = d.threads.filter((t) => !t.submitted).length; const open = d.threads.filter((t) => t.status === "open").length; - const tabs: [View, string][] = [ - ["review", "Review"], - ["commits", "Commits"], - ["files", `Files${d.changes.length ? ` (${d.changes.length})` : ""}`], - ["map", "Map"], - ]; + const explainer = kind() === "explainer"; + const labels: Record = { + review: explainer ? "Explainer" : "Review", + commits: "Commits", + files: `Files${d.changes.length ? ` (${d.changes.length})` : ""}`, + map: "Map", + coverage: "Coverage", + }; + const current = view(); + const tabs: [View, string][] = VIEWS[kind()].map((v) => [v, labels[v]]); const revSel = h("select", { class: "small", style: { font: "inherit", fontSize: "12px" }, @@ -131,7 +154,7 @@ function renderTopbar(): void { style: { fontSize: "12px" }, title: `${r.pins.base} → ${r.pins.head}`, }, - r.binding.kind === "pr" ? `PR #${r.binding.name}` : r.binding.name, + bindingLabel(r), ), ]); const actions = h("div", { class: "bar-row bar-actions" }, [ @@ -139,7 +162,7 @@ function renderTopbar(): void { "div", { class: "tabs" }, tabs.map(([v, label]) => - h("button", { class: v === state.view ? "active" : "", onclick: () => navigate(v) }, label), + h("button", { class: v === current ? "active" : "", onclick: () => navigate(v) }, label), ), ), h("span", { class: "spacer" }), @@ -227,7 +250,9 @@ function moreMenu(e: MouseEvent): void { h( "div", { class: "item muted" }, - `base ${r.pins.base.slice(0, 12)} · head ${r.pins.head.slice(0, 12)}`, + kind() === "explainer" + ? `commit ${r.pins.head.slice(0, 12)}` + : `base ${r.pins.base.slice(0, 12)} · head ${r.pins.head.slice(0, 12)}`, ), h( "div", @@ -241,7 +266,7 @@ function moreMenu(e: MouseEvent): void { function renderCenter(): void { const scroll = center.scrollTop; clear(center); - switch (state.view) { + switch (view()) { case "review": renderReview(center); break; @@ -254,6 +279,9 @@ function renderCenter(): void { case "map": renderMap(center); break; + case "coverage": + renderCoverage(center); + break; } center.scrollTop = scroll; } diff --git a/src/ui/state.ts b/src/ui/state.ts index e090cd5..eb0fc80 100644 --- a/src/ui/state.ts +++ b/src/ui/state.ts @@ -1,7 +1,29 @@ import type { Payload } from "./api.js"; -import type { Thread, ThreadTarget } from "../store.js"; +import type { DocumentKind, Thread, ThreadTarget } from "../store.js"; -export type View = "review" | "commits" | "files" | "map"; +export type View = "review" | "commits" | "files" | "map" | "coverage"; + +/** + * The tabs a document kind has. A review is a CHANGE, so the diff, the commits + * and the interface delta all mean something. An explainer is a CODEBASE at one + * commit: those three would render an empty claim about a change that does not + * exist, so they are absent, and Coverage - what the document reached and what + * it did not - takes their place. + */ +export const VIEWS: Record = { + review: ["review", "commits", "files", "map"], + explainer: ["review", "map", "coverage"], +}; + +export function kind(): DocumentKind { + return state.data?.review.kind === "explainer" ? "explainer" : "review"; +} + +/** The current view, or the document's first tab when this kind has no such tab. */ +export function view(): View { + const allowed = VIEWS[kind()]; + return allowed.includes(state.view) ? state.view : allowed[0]!; +} export interface SideState { kind: "none" | "peek" | "threads"; @@ -74,8 +96,10 @@ export function navigate( export function readHash(): void { const m = /^#\/(\w+)(?:\?(.*))?$/.exec(location.hash); - const view = (m?.[1] ?? "review") as View; - state.view = ["review", "commits", "files", "map"].includes(view) ? view : "review"; + const v = (m?.[1] ?? "review") as View; + state.view = (["review", "commits", "files", "map", "coverage"] as View[]).includes(v) + ? v + : "review"; state.params = new URLSearchParams(m?.[2] ?? ""); } diff --git a/src/ui/threads.ts b/src/ui/threads.ts index b7d0d80..e29359c 100644 --- a/src/ui/threads.ts +++ b/src/ui/threads.ts @@ -1,6 +1,6 @@ import { api } from "./api.js"; import { h, closePopover, dialog, timeAgo } from "./dom.js"; -import { state, emit, describeTarget, navigate, readOnly } from "./state.js"; +import { state, emit, describeTarget, navigate, readOnly, kind } from "./state.js"; import type { Thread, ThreadTarget } from "../store.js"; export async function reload(): Promise { @@ -289,12 +289,19 @@ function threadCard(t: Thread): HTMLElement { export function submitDialog(): void { const pending = (state.data?.threads ?? []).filter((t) => !t.submitted).length; + // Same state machine, different words. A reader decides whether a CHANGE should + // land; a reader of an explainer decides whether the DOCUMENT is finished, and + // "Approve" would read as a verdict on code the explainer never judged. + const explainer = kind() === "explainer"; + const words = explainer + ? { title: "Send this back or finish", accept: "Done reading", back: "Send it back" } + : { title: "Submit review", accept: "Approve", back: "Request changes" }; const ta = h("textarea", { rows: 4, placeholder: "Summary for the agent (optional)" }); const d = dialog( h( "div", null, - h("h3", null, "Submit review"), + h("h3", null, words.title), h( "p", { class: "muted" }, @@ -309,11 +316,16 @@ export function submitDialog(): void { h("button", { class: "ghost", onclick: () => d.close() }, "Cancel"), h( "button", - { title: "End the review without approving it", onclick: () => decide("close") }, + { + title: explainer + ? "End it without reading it through" + : "End the review without approving it", + onclick: () => decide("close"), + }, "Close", ), - h("button", { onclick: () => decide("request-changes") }, "Request changes"), - h("button", { class: "ok", onclick: () => decide("approve") }, "Approve"), + h("button", { onclick: () => decide("request-changes") }, words.back), + h("button", { class: "ok", onclick: () => decide("approve") }, words.accept), ), ), ); diff --git a/src/ui/views/coverage.ts b/src/ui/views/coverage.ts new file mode 100644 index 0000000..fb4fe8e --- /dev/null +++ b/src/ui/views/coverage.ts @@ -0,0 +1,306 @@ +/** + * What the explainer examined, and what it did not. + * + * A review is bounded by its diff; a codebase is not. So this tab exists to + * make the bound of an explainer a stated fact rather than something the reader + * has to infer from what the prose happens to mention. Everything here is + * derived at publish from the code graph at the pinned commit: counts and lists + * of named things, nothing graded. Where a number invites a conclusion - a part + * everything reaches, a name defined in four places, a link that runs both ways + * - drawing it is the reader's job, and this page deliberately stops short. + */ +import { h, append } from "../dom.js"; +import { state, navigate } from "../state.js"; +import { openAnchorPeek } from "../code.js"; +import type { Coverage, ClusterCoverage } from "../../coverage.js"; + +const SHOWN = 8; + +export function renderCoverage(root: HTMLElement): void { + const cov = state.data?.coverage; + if (!cov) { + append(root, [ + h( + "div", + { class: "empty-state" }, + h("p", null, "No coverage recorded for this revision."), + h("p", null, "Coverage is derived when an explainer is published."), + ), + ]); + return; + } + append(root, [intro(cov), clusters(cov), links(cov), sharedNames(cov), unread(cov)]); +} + +function bar(cov: Coverage): HTMLElement { + const total = Math.max(cov.files.total, 1); + const seg = (n: number, cls: string, label: string) => + n + ? h("span", { + class: `cov-seg ${cls}`, + style: { width: `${(n / total) * 100}%` }, + title: `${n} ${label}`, + }) + : null; + return h( + "div", + { class: "cov-bar" }, + seg(cov.states.explained, "explained", "anchored in the document"), + seg(cov.states.placed, "placed", "placed on the map only"), + seg(cov.states.uncovered, "uncovered", "not examined"), + ); +} + +function intro(cov: Coverage): HTMLElement { + const key: [number, string, string][] = [ + [cov.states.explained, "explained", "anchored in the document"], + [cov.states.placed, "placed", "placed on the map only"], + [cov.states.uncovered, "uncovered", "not examined"], + ]; + return h( + "div", + { class: "cov-intro" }, + h("h2", null, "What this explainer covers"), + h( + "p", + null, + "A codebase does not fit in a short document, so this one selects. Here is the", + " selection, counted rather than claimed: every file in scope at the pinned commit,", + " and which of the three states it is in.", + ), + bar(cov), + h( + "div", + { class: "cov-key" }, + key.map(([n, cls, label]) => + h("span", { class: "cov-key-item" }, h("i", { class: `cov-dot ${cls}` }), `${n} ${label}`), + ), + ), + h( + "div", + { class: "cov-facts" }, + fact("scope", cov.scope === "**" ? "the whole repository" : cov.scope), + fact("commit", cov.commit.slice(0, 12)), + fact("files in scope", String(cov.files.total)), + fact("read by the code graph", `${cov.files.inGraph} of ${cov.files.total}`), + cov.unresolved ? fact("references the graph could not place", String(cov.unresolved)) : null, + cov.truncated + ? fact("file list capped", "the graph is partial; treat every count as a floor") + : null, + ), + ); +} + +function fact(label: string, value: string): HTMLElement { + return h("div", { class: "cov-fact" }, h("b", null, value), h("span", { class: "muted" }, label)); +} + +function fileList(files: string[], cls: string): HTMLElement | null { + if (!files.length) return null; + const wrap = h("div", { class: `cov-files ${cls}` }); + const chip = (f: string) => + h("code", { class: "cov-file", title: f }, f.split("/").slice(-2).join("/")); + files.slice(0, SHOWN).forEach((f) => wrap.appendChild(chip(f))); + const rest = files.slice(SHOWN); + if (rest.length) { + const more = h( + "button", + { + class: "small ghost", + onclick: () => more.replaceWith(...rest.map(chip)), + }, + `show ${rest.length} more`, + ); + wrap.appendChild(more); + } + return wrap; +} + +function clusterRow(c: ClusterCoverage): HTMLElement { + const anchors = state.data?.document?.anchors ?? {}; + const firstAnchor = c.explained.length + ? Object.values(anchors).find((a) => a.peek && c.explained.includes(a.peek.file)) + : undefined; + return h( + "div", + { class: "cov-cluster" }, + h( + "div", + { class: "cov-cluster-head" }, + h("code", { class: "cov-label" }, c.label), + h("span", { class: "muted" }, `${c.files} files · ${c.symbols} symbols`), + h("span", { class: "spacer" }), + c.explained.length + ? h("span", { class: "badge cov-b-explained" }, `${c.explained.length} anchored`) + : null, + c.placed.length + ? h("span", { class: "badge cov-b-placed" }, `${c.placed.length} placed`) + : null, + c.uncovered.length + ? h("span", { class: "badge cov-b-uncovered" }, `${c.uncovered.length} not examined`) + : null, + firstAnchor + ? h("button", { class: "small", onclick: () => openAnchorPeek(firstAnchor) }, "Peek code ▸") + : null, + ), + c.hubs.length + ? h( + "div", + { class: "muted cov-hubs" }, + "most referenced here: ", + c.hubs.map((n) => h("code", null, n)), + ) + : null, + fileList(c.uncovered, "uncovered"), + ); +} + +function clusters(cov: Coverage): HTMLElement { + return h( + "div", + { class: "cov-section" }, + h("h3", null, "By part of the system"), + h( + "p", + { class: "muted" }, + "Files clustered by how they reference each other at the pinned commit, largest first.", + " The files listed under a part are the ones this explainer never examined.", + ), + cov.clusters.map(clusterRow), + ); +} + +function links(cov: Coverage): HTMLElement | null { + if (!cov.links.length) return null; + return h( + "div", + { class: "cov-section" }, + h("h3", null, "What reaches what"), + h( + "p", + { class: "muted" }, + "References that cross from one part to another, with how many there are.", + " A pair marked both ways references each other in both directions.", + ), + h( + "div", + { class: "cov-links" }, + cov.links + .slice(0, 24) + .map((l) => + h( + "div", + { class: "cov-link" }, + h("code", null, label(cov, l.from)), + h("span", { class: "muted" }, "→"), + h("code", null, label(cov, l.to)), + h("span", { class: "muted mono" }, `${l.references} refs`), + l.bothWays ? h("span", { class: "badge" }, "both ways") : null, + ), + ), + ), + ); +} + +function label(cov: Coverage, id: string): string { + return cov.clusters.find((c) => c.id === id)?.label ?? id; +} + +function sharedNames(cov: Coverage): HTMLElement | null { + if (!cov.sharedNames.length) return null; + return h( + "div", + { class: "cov-section" }, + h("h3", null, "Names defined in more than one part"), + h( + "p", + { class: "muted" }, + `${cov.sharedNamesTotal} name${cov.sharedNamesTotal === 1 ? "" : "s"} at this commit are`, + " defined in two or more parts. What that means here - the same idea in two places,", + " two different ideas sharing a word, or a name too common to mean anything - is", + " what the code says and this page does not.", + ), + h( + "div", + { class: "cov-shared" }, + cov.sharedNames.map((n) => + h( + "div", + { class: "cov-link" }, + h("code", null, n.name), + h("span", { class: "muted mono" }, `${n.clusters.length} parts`), + h("span", { class: "muted" }, n.files.join(" · ")), + ), + ), + ), + ); +} + +function unread(cov: Coverage): HTMLElement | null { + if (!cov.outsideGraph.length && !cov.owners.length) return null; + return h( + "div", + { class: "cov-section" }, + h("h3", null, "Outside the code graph"), + cov.outsideGraph.length + ? h( + "div", + null, + h( + "p", + { class: "muted" }, + `${cov.files.outsideGraph} files in scope are not in a language the graph reads,`, + " so they are in no part above. They are absent from the structure, not empty.", + ), + h( + "div", + { class: "cov-links" }, + cov.outsideGraph + .slice(0, 16) + .map((e) => + h( + "div", + { class: "cov-link" }, + h("code", null, `.${e.extension}`), + h("span", { class: "muted mono" }, `${e.files} files`), + ), + ), + ), + fileList( + cov.unclustered.filter((u) => u.state === "uncovered").map((u) => u.file), + "uncovered", + ), + ) + : null, + cov.owners.length + ? h( + "div", + null, + h("h4", null, "What each map node owns"), + h( + "p", + { class: "muted" }, + "A file counts as placed because a map node's globs match it. The globs are here", + " so a broad one is visible rather than silently inflating the count.", + ), + h( + "div", + { class: "cov-links" }, + cov.owners.map((o) => + h( + "div", + { + class: "cov-link", + onclick: () => navigate("map", { node: o.node }), + style: { cursor: "pointer" }, + }, + h("code", null, o.node), + h("span", { class: "muted" }, o.globs.join(" ")), + h("span", { class: "muted mono" }, `${o.files} files`), + ), + ), + ), + ) + : null, + ); +} diff --git a/src/ui/views/map.ts b/src/ui/views/map.ts index 25070cf..e0f5300 100644 --- a/src/ui/views/map.ts +++ b/src/ui/views/map.ts @@ -1,5 +1,5 @@ import { h, append, popover } from "../dom.js"; -import { state, navigate, threadsFor } from "../state.js"; +import { state, navigate, threadsFor, kind } from "../state.js"; import { openAnchorPeek } from "../code.js"; import { commentPopover, threadPinRow } from "../threads.js"; import { @@ -22,19 +22,33 @@ export function renderMap(root: HTMLElement): void { const map = state.data?.map; // A map with no nodes is the same to the reader as no map at all, and the // reason for both is worth saying: an absent map is a claim, not a gap. + const explainer = kind() === "explainer"; if (!map || !map.head.nodes.length) { append(root, [ - h( - "div", - { class: "empty-state" }, - h("p", null, "No map in this review."), - h( - "p", - null, - "The map places a change inside the system and shows what sits next to it. ", - "A change that lands in one place, where the Files tab already answers that, ships without one.", - ), - ), + explainer + ? h( + "div", + { class: "empty-state" }, + h("p", null, "No map in this explainer."), + h( + "p", + null, + "The map is where an explainer carries breadth: it places the parts the prose ", + "had no room for. Without one, every file the document does not anchor counts ", + "as not examined on the Coverage tab.", + ), + ) + : h( + "div", + { class: "empty-state" }, + h("p", null, "No map in this review."), + h( + "p", + null, + "The map places a change inside the system and shows what sits next to it. ", + "A change that lands in one place, where the Files tab already answers that, ships without one.", + ), + ), ]); return; } @@ -62,6 +76,28 @@ export function renderMap(root: HTMLElement): void { // so it says what the map answers that the diff cannot, and hands them one // place to start rather than a board of equal boxes. const intro = () => { + if (explainer) + return h( + "div", + { class: "map-intro" }, + h("h2", null, "The parts of the system"), + h( + "p", + null, + "The document carries depth on the parts it selected; this carries breadth over the", + " rest — what the parts are, what they own, and what they connect to at the pinned", + " commit. Open a part to see its files, its code and its neighbours.", + ), + h( + "div", + { class: "map-route" }, + h( + "button", + { class: "small", onclick: () => navigate("coverage") }, + "What this explainer left out ▸", + ), + ), + ); const counts: [number, string, string][] = [ [map.diff.added.length, "ok", "added"], [map.diff.changed.length, "warn", "changed"], @@ -125,7 +161,7 @@ export function renderMap(root: HTMLElement): void { return h( "div", { - class: `map-node ${n.status} ${touched(all, n) ? "" : "quiet"} ${n.id === selected ? "selected" : ""}`, + class: `map-node ${n.status} ${explainer || touched(all, n) ? "" : "quiet"} ${n.id === selected ? "selected" : ""}`, onclick: () => navigate("map", { node: n.id }), ondblclick: () => inside && openLevel(n.id), }, @@ -171,6 +207,9 @@ export function renderMap(root: HTMLElement): void { h("div", { class: "map-grid" }, items.map(card)), ) : null; + // An explainer has no change, so there is no reading order to impose: the + // parts are drawn as the author grouped them, and none of them is "context". + if (explainer) return group(null, kids) ?? h("div", null); if (!hot.length) return h( "div", @@ -216,7 +255,9 @@ export function renderMap(root: HTMLElement): void { h( "div", { class: "muted map-note" }, - "A link touching a changed part is where the two sides can fall out of step.", + explainer + ? "A link is a dependency the parts have on each other, drawn as the author declared it." + : "A link touching a changed part is where the two sides can fall out of step.", ), rankEdges(folded, all).map(({ edge, touchesChange }) => h( @@ -253,7 +294,9 @@ export function renderMap(root: HTMLElement): void { h( "div", { class: "muted" }, - "Pick a part to see what changed in it, the code behind it, and what it connects to.", + explainer + ? "Pick a part to see what it owns, the code behind it, and what it connects to." + : "Pick a part to see what changed in it, the code behind it, and what it connects to.", ), ); return; diff --git a/src/ui/views/review.ts b/src/ui/views/review.ts index 9598322..9beba64 100644 --- a/src/ui/views/review.ts +++ b/src/ui/views/review.ts @@ -1,10 +1,11 @@ import { h, popover } from "../dom.js"; -import { state, threadsFor, navigate } from "../state.js"; +import { state, threadsFor, navigate, kind } from "../state.js"; import { codeTable, openAnchorPeek } from "../code.js"; import { commentPopover, threadPinRow } from "../threads.js"; import { sequenceDiagram, callstackDiff, databaseLens } from "../diagrams.js"; import type { Block } from "../../document/compile.js"; import type { InterfaceDelta, InterfaceEntry } from "../../interfaces.js"; +import type { Coverage } from "../../coverage.js"; export function renderReview(root: HTMLElement): void { const doc = state.data?.document; @@ -20,7 +21,11 @@ export function renderReview(root: HTMLElement): void { ); return; } - root.appendChild(interfaceDelta(doc.interfaces)); + root.appendChild( + kind() === "explainer" + ? coveragePanel(state.data?.coverage ?? null) + : interfaceDelta(doc.interfaces), + ); const layout = h("div", { class: "doc-layout" }); const toc = h( "nav", @@ -97,6 +102,7 @@ export function renderReview(root: HTMLElement): void { /** Ids the parser cannot produce, so the panel can hold threads like a document block. */ const DELTA_BLOCK = "interface-delta"; +const COVERAGE_BLOCK = "coverage"; const DELTA_SHOWN = 12; const CHANGE_CLASS: Record = { @@ -186,6 +192,78 @@ function interfaceDelta(delta: InterfaceDelta | null): HTMLElement { return wrap; } +/** + * What an explainer examined and what it did not, in the slot a review gives the + * interface delta - because it is the same kind of thing: a fact derived at + * publish from the pinned commit, above prose the agent wrote, so the reader + * knows the bound of the document before reading a word of it. + */ +function coveragePanel(cov: Coverage | null): HTMLElement { + const wrap = h("div", { class: "block ifd", "data-block": COVERAGE_BLOCK }); + const threads = threadsFor((t) => t.type === "document" && t.blockId === COVERAGE_BLOCK); + if (threads.length) wrap.classList.add("has-threads"); + const actions = h( + "div", + { class: "block-actions" }, + h( + "button", + { + class: threads.length ? "count" : "", + title: "Comment on coverage", + onclick: (e: MouseEvent) => { + if (state.viewingRevision !== null) return; + popover(commentPopover({ type: "document", blockId: COVERAGE_BLOCK }), { + x: e.pageX + 10, + y: e.pageY, + }); + }, + }, + threads.length ? String(threads.length) : "+", + ), + ); + const body = h("div", { class: "ifd-body" }); + const toggle = h("span", { class: "heading-toggle" }, "hide"); + toggle.addEventListener("click", () => { + const hidden = body.hidden; + body.hidden = !hidden; + toggle.textContent = hidden ? "hide" : "show"; + }); + wrap.appendChild( + h( + "div", + { class: "ifd-head" }, + h("span", { class: "t" }, "Coverage"), + cov ? h("span", { class: "badge" }, `${cov.states.uncovered} not examined`) : null, + h("span", { class: "spacer" }), + toggle, + actions, + ), + ); + body.appendChild( + h( + "p", + { class: "ifd-verdict" }, + cov ? cov.verdict : "Unavailable: coverage could not be derived for this revision.", + ), + ); + body.appendChild( + h( + "p", + { class: "ifd-verdict" }, + "A codebase does not fit in one document, so this one selects. ", + h( + "button", + { class: "small", onclick: () => navigate("coverage") }, + "See what it left out ▸", + ), + ), + ); + wrap.appendChild(body); + if (threads.length) + wrap.appendChild(h("div", { class: "thread-pins" }, threads.map(threadPinRow))); + return wrap; +} + function interfaceRow(e: InterfaceEntry): HTMLElement { return h( "div", diff --git a/test/e2e.test.ts b/test/e2e.test.ts index 9076973..bcf05ed 100644 --- a/test/e2e.test.ts +++ b/test/e2e.test.ts @@ -599,6 +599,117 @@ check expect((await f.arrayBuffer()).byteLength).toBeGreaterThan(1000); }); + // ---- the explainer document kind ---- + // Its own block, and it never touches reviewId: the review path above must + // keep passing exactly as it did before explainers existed. + + let explainerId = ""; + let explainerDir = ""; + + it("pins an explainer to one commit and the scope the reader asked for", async () => { + const out = await cli(["explain", "src"]); + const e = out["explainer"]; + explainerId = e["id"]; + explainerDir = e["dir"]; + expect(e["kind"]).toBe("explainer"); + expect(e["scope"]).toBe("src/**"); + expect(e["title"]).toBe("src"); + expect(e["commit"]).toMatch(/^[0-9a-f]{40}$/); + expect(out["scale"]["filesInScope"]).toBe(2); + const info = await cli(["info", "--fields", "pins"]); + const row = info["reviews"].find((r: Out) => r["id"] === explainerId); + expect(row["kind"]).toBe("explainer"); + // one commit, not a range + expect(row["pins"]).not.toContain(".."); + }, 20_000); + + it("refuses a scope that matches no file at the pinned commit", async () => { + const out = await cli(["explain", "does/not/exist"], { expectCode: 2 }); + expect(out["code"]).toBe("VALIDATION_ERROR"); + expect(String(out["error"])).toContain("no file matches"); + }); + + it("refuses the graph queries that compare two commits", async () => { + for (const sub of ["impact", "interfaces"]) { + const out = await cli(["graph", sub, "--review", explainerId], { expectCode: 2 }); + expect(String(out["error"])).toContain("compares two commits"); + } + const arch = await cli(["graph", "architecture", "--review", explainerId]); + expect(arch["scope"]).toBe("src/**"); + expect(arch["base"]).toBeUndefined(); + for (const c of arch["communities"]) + for (const f of c["files"]) expect(f.startsWith("src/")).toBe(true); + }, 60_000); + + it("rejects an explainer that claims a change it cannot have", async () => { + await writeFile( + join(explainerDir, "data.yaml"), + `anchors:\n old: { title: old, peek: { file: src/auth.ts, from: 1, to: 2, graph: base } }\ninterfaces:\n x: { name: --flag, change: added, capability: does a thing, anchor: old }\n`, + ); + await writeFile(join(explainerDir, "review.md"), `# Explainer\n\nSee [old](anchor:old).\n`); + const out = await cli(["publish", "--review", explainerId], { expectCode: 1 }); + const messages = out["diagnostics"].map((d: Out) => String(d["message"])); + expect(messages.some((m: string) => m.includes("no interface delta"))).toBe(true); + expect(messages.some((m: string) => m.includes("`graph: base` has no meaning"))).toBe(true); + }, 60_000); + + it("rejects an explainer with no anchored claim", async () => { + await writeFile(join(explainerDir, "data.yaml"), `anchors: {}\n`); + await writeFile(join(explainerDir, "review.md"), `# Explainer\n\nTrust me.\n`); + const out = await cli(["publish", "--review", explainerId], { expectCode: 1 }); + expect( + out["diagnostics"].some((d: Out) => + String(d["message"]).includes("needs at least one anchored claim"), + ), + ).toBe(true); + }, 60_000); + + it("publishes an explainer and states what it did not examine", async () => { + await writeFile( + join(explainerDir, "data.yaml"), + `anchors:\n login: { title: login(), peek: { file: src/auth.ts, from: 3, to: 6 } }\n`, + ); + await writeFile( + join(explainerDir, "review.md"), + `# How auth works\n\nA caller reaches [login()](anchor:login).\n`, + ); + const out = await cli(["publish", "--review", explainerId]); + expect(out["published"]["kind"]).toBe("explainer"); + // the interface delta is a claim about a change, so an explainer has none + expect(out["published"]["interfaces"]).toBeUndefined(); + expect(String(out["published"]["coverage"])).toContain("2 files at"); + // src/audit.ts is neither anchored nor owned by a map node, and the document says so + expect(out["notExamined"]["files"]).toBe(1); + expect(out["notExamined"]["first"]).toContain("src/audit.ts"); + // no map, so nothing carries the breadth the prose left out, and it says so + expect(String(out["warnings"])).toContain("no map"); + }, 60_000); + + it("counts a file a map node owns as placed, not as examined", async () => { + await writeFile( + join(explainerDir, "map.yaml"), + `nodes:\n - id: audit\n kind: component\n label: Audit log\n files: ["src/audit.ts"]\nedges: []\n`, + ); + const out = await cli(["publish", "--review", explainerId]); + expect(out["notExamined"]["files"]).toBe(0); + expect(String(out["published"]["coverage"])).toContain("1 placed on the map only"); + }, 60_000); + + it("serves an explainer with coverage and without a change", async () => { + const d = await api(`/api/reviews/${explainerId}`); + expect(d["review"]["kind"]).toBe("explainer"); + expect(d["review"]["binding"]["kind"]).toBe("codebase"); + expect(d["document"]["interfaces"]).toBe(null); + expect(d["changes"]).toEqual([]); + const cov = d["coverage"]; + expect(cov["scope"]).toBe("src/**"); + expect(cov["states"]).toEqual({ explained: 1, placed: 1, uncovered: 0 }); + expect(cov["uncovered"]).toEqual([]); + expect(cov["verdict"]).toContain("not examined"); + // every count is re-derivable from the graph at the same commit + expect(cov["clusters"].flatMap((c: Out) => c["explained"])).toContain("src/auth.ts"); + }, 20_000); + it("answers --help per command without loading live state", async () => { const h = await cli(["threads", "--help"]); expect(h["command"]).toContain("thurview threads"); From cbdcdc6ceef77e74b95cbccfc7a8ac9d63cea69a Mon Sep 17 00:00:00 2001 From: letur Date: Wed, 9 Sep 2026 16:49:19 +0200 Subject: [PATCH 2/4] chore: no-mistakes review - Fix coverage cap misclassification and add missing coverage badge CSS --- src/coverage.ts | 42 +++++++++++++++++++++++++++++----------- src/graph.ts | 7 ++++++- src/ui/app.css | 10 ++++++++++ src/ui/views/coverage.ts | 24 +++++++++++++++++++++-- 4 files changed, 69 insertions(+), 14 deletions(-) diff --git a/src/coverage.ts b/src/coverage.ts index 411b76d..729ac26 100644 --- a/src/coverage.ts +++ b/src/coverage.ts @@ -15,7 +15,7 @@ * conclusion, drawing it is the reader's job. */ import { globToRegExp } from "./document/compile.js"; -import { architecture, isNestedNonMethod, type CodeGraph, type Sym } from "./graph.js"; +import { architecture, isGraphLanguage, isNestedNonMethod, type CodeGraph, type Sym } from "./graph.js"; /** How a file at the pinned commit is accounted for. */ export type FileState = @@ -50,13 +50,15 @@ export interface Coverage { inGraph: number; /** everything else: config, docs, other languages */ outsideGraph: number; + /** in-scope files in a graph language, skipped only because the repo-wide file cap was hit before scoping */ + capped: number; }; states: { explained: number; placed: number; uncovered: number }; clusters: ClusterCoverage[]; /** every file in scope the document never examined, clustered or not */ uncovered: string[]; - /** in-scope files the code graph cannot read, so they are in no cluster above */ - unclustered: { file: string; state: FileState }[]; + /** in-scope files with no cluster above: either the graph cannot read them, or the file cap skipped them */ + unclustered: { file: string; state: FileState; reason: "outsideGraph" | "capped" }[]; /** references between clusters at the pinned commit */ links: { from: string; to: string; references: number; bothWays: boolean }[]; /** names defined in more than one cluster, most-spread first */ @@ -66,8 +68,9 @@ export interface Coverage { outsideGraph: { extension: string; files: number }[]; /** map nodes that own files, so an over-broad glob is visible rather than silent */ owners: { node: string; globs: string[]; files: number }[]; - /** references the graph could not place, and whether its file list was capped */ + /** references the graph could not place */ unresolved: number; + /** the repo-wide file cap excluded at least one in-scope, graph-language file */ truncated: boolean; } @@ -191,16 +194,24 @@ export function computeCoverage(input: CoverageInput): Coverage { } spread.sort((a, b) => b.clusters.length - a.clusters.length || a.name.localeCompare(b.name)); + // A file missing from the scoped graph is either genuinely outside a graph + // language, or a graph-language file the repo-wide MAX_FILES cap dropped + // before scoping ever saw it (graph.ts:capFiles runs over the whole repo). + // Only the first is "outside the languages the graph reads"; the second is + // just unexamined, and saying otherwise would misstate what the cap did. const byExt = new Map(); + let capped = 0; for (const f of files) if (!inGraph.has(f)) { - const e = extensionOf(f); - byExt.set(e, (byExt.get(e) ?? 0) + 1); + if (isGraphLanguage(f)) capped++; + else byExt.set(extensionOf(f), (byExt.get(extensionOf(f)) ?? 0) + 1); } const states = { explained: 0, placed: 0, uncovered: 0 }; for (const f of files) states[stateOf(f)]++; + const outsideGraphCount = files.length - graph.files.length - capped; + const record: Coverage = { verdict: "", commit: input.commit, @@ -208,7 +219,8 @@ export function computeCoverage(input: CoverageInput): Coverage { files: { total: files.length, inGraph: graph.files.length, - outsideGraph: files.length - graph.files.length, + outsideGraph: outsideGraphCount, + capped, }, states, clusters, @@ -216,7 +228,11 @@ export function computeCoverage(input: CoverageInput): Coverage { unclustered: files .filter((f) => !inGraph.has(f)) .slice(0, FILES_LISTED) - .map((f) => ({ file: f, state: stateOf(f) })), + .map((f) => ({ + file: f, + state: stateOf(f), + reason: isGraphLanguage(f) ? ("capped" as const) : ("outsideGraph" as const), + })), links, sharedNames: spread.slice(0, SHARED_NAMES_SHOWN), sharedNamesTotal: spread.length, @@ -225,7 +241,7 @@ export function computeCoverage(input: CoverageInput): Coverage { .sort((a, b) => b.files - a.files || a.extension.localeCompare(b.extension)), owners: owners.sort((a, b) => b.files - a.files || a.node.localeCompare(b.node)), unresolved: input.graph.unresolved, - truncated: input.graph.truncated, + truncated: capped > 0, }; record.verdict = coverageVerdict(record); return record; @@ -241,9 +257,13 @@ function coverageVerdict(c: Coverage): string { `${placed} placed on the map only`, `${uncovered} not examined`, ]; - const tail = + const outside = c.files.outsideGraph > 0 ? ` ${c.files.outsideGraph} of them are outside the languages the code graph reads.` : ""; - return `${parts.join(", ")}.${tail}`; + const capped = + c.files.capped > 0 + ? ` ${c.files.capped} of them were skipped by the repo-wide file cap; treat every count as a floor.` + : ""; + return `${parts.join(", ")}.${outside}${capped}`; } diff --git a/src/graph.ts b/src/graph.ts index 91fcf69..7c4cb8d 100644 --- a/src/graph.ts +++ b/src/graph.ts @@ -172,6 +172,11 @@ export function isTestFile(path: string): boolean { const MAX_FILES = 4000; +/** Whether the graph can parse `path` at all - independent of the repo-wide MAX_FILES cap. */ +export function isGraphLanguage(path: string): boolean { + return Boolean(GRAMMARS[languageFor(path)]) && !SKIP.test(path); +} + /** Cap a file list at MAX_FILES, reporting whether it had to be truncated. */ export function capFiles(files: string[]): { files: string[]; truncated: boolean } { return files.length > MAX_FILES @@ -182,7 +187,7 @@ export function capFiles(files: string[]): { files: string[]; truncated: boolean /** Parse every supported file at `commit` and resolve references to definitions by name. */ export async function buildGraph(cwd: string, commit: string): Promise { const { files, truncated } = capFiles( - (await listFiles(cwd, commit)).filter((p) => GRAMMARS[languageFor(p)] && !SKIP.test(p)), + (await listFiles(cwd, commit)).filter(isGraphLanguage), ); const symbols: Sym[] = []; const byName = new Map(); diff --git a/src/ui/app.css b/src/ui/app.css index 5fabf5d..53ef9f9 100644 --- a/src/ui/app.css +++ b/src/ui/app.css @@ -1903,6 +1903,16 @@ kbd { color: var(--accent); border-color: var(--accent); } +.cov-b-placed { + color: var(--accent); + border-color: var(--accent); + background: var(--accent-bg); +} +.cov-b-uncovered { + color: var(--muted); + border-color: var(--line); + background: var(--bg3); +} .cov-links, .cov-shared { display: flex; diff --git a/src/ui/views/coverage.ts b/src/ui/views/coverage.ts index fb4fe8e..bd34dd6 100644 --- a/src/ui/views/coverage.ts +++ b/src/ui/views/coverage.ts @@ -237,7 +237,7 @@ function sharedNames(cov: Coverage): HTMLElement | null { } function unread(cov: Coverage): HTMLElement | null { - if (!cov.outsideGraph.length && !cov.owners.length) return null; + if (!cov.outsideGraph.length && !cov.files.capped && !cov.owners.length) return null; return h( "div", { class: "cov-section" }, @@ -267,7 +267,27 @@ function unread(cov: Coverage): HTMLElement | null { ), ), fileList( - cov.unclustered.filter((u) => u.state === "uncovered").map((u) => u.file), + cov.unclustered + .filter((u) => u.state === "uncovered" && u.reason === "outsideGraph") + .map((u) => u.file), + "uncovered", + ), + ) + : null, + cov.files.capped + ? h( + "div", + null, + h( + "p", + { class: "muted" }, + `${cov.files.capped} files in scope are in a language the graph reads, but the`, + " repo-wide file cap was hit before this scope was read, so they were never parsed.", + ), + fileList( + cov.unclustered + .filter((u) => u.state === "uncovered" && u.reason === "capped") + .map((u) => u.file), "uncovered", ), ) From 56893b7599fadc56cddcdc72a36035ad8f3604f5 Mon Sep 17 00:00:00 2001 From: letur Date: Wed, 9 Sep 2026 18:19:40 +0200 Subject: [PATCH 3/4] test: cover the two reasons a file is absent from the structure The review round's fix separated a file the code graph cannot read from one the repo-wide file cap dropped before scoping saw it. Only the first is "outside the languages the graph reads"; reporting the second that way would state something false about the code, which is the one thing an explainer must not do. Lock the classification through the CLI and the HTTP payload, and fix a singular/plural slip in the shared-names copy. Claude-Session: https://claude.ai/code/session_01WW5sEutdtcbSUtgLQKBKpn --- src/coverage.ts | 8 +++++++- src/graph.ts | 4 +--- src/ui/views/coverage.ts | 2 +- test/e2e.test.ts | 22 ++++++++++++++++++++++ 4 files changed, 31 insertions(+), 5 deletions(-) diff --git a/src/coverage.ts b/src/coverage.ts index 729ac26..57a53c6 100644 --- a/src/coverage.ts +++ b/src/coverage.ts @@ -15,7 +15,13 @@ * conclusion, drawing it is the reader's job. */ import { globToRegExp } from "./document/compile.js"; -import { architecture, isGraphLanguage, isNestedNonMethod, type CodeGraph, type Sym } from "./graph.js"; +import { + architecture, + isGraphLanguage, + isNestedNonMethod, + type CodeGraph, + type Sym, +} from "./graph.js"; /** How a file at the pinned commit is accounted for. */ export type FileState = diff --git a/src/graph.ts b/src/graph.ts index 7c4cb8d..6321510 100644 --- a/src/graph.ts +++ b/src/graph.ts @@ -186,9 +186,7 @@ export function capFiles(files: string[]): { files: string[]; truncated: boolean /** Parse every supported file at `commit` and resolve references to definitions by name. */ export async function buildGraph(cwd: string, commit: string): Promise { - const { files, truncated } = capFiles( - (await listFiles(cwd, commit)).filter(isGraphLanguage), - ); + const { files, truncated } = capFiles((await listFiles(cwd, commit)).filter(isGraphLanguage)); const symbols: Sym[] = []; const byName = new Map(); const pending: { file: string; defs: Sym[]; refs: Tag[] }[] = []; diff --git a/src/ui/views/coverage.ts b/src/ui/views/coverage.ts index bd34dd6..211865e 100644 --- a/src/ui/views/coverage.ts +++ b/src/ui/views/coverage.ts @@ -215,7 +215,7 @@ function sharedNames(cov: Coverage): HTMLElement | null { h( "p", { class: "muted" }, - `${cov.sharedNamesTotal} name${cov.sharedNamesTotal === 1 ? "" : "s"} at this commit are`, + `${cov.sharedNamesTotal} name${cov.sharedNamesTotal === 1 ? " at this commit is" : "s at this commit are"}`, " defined in two or more parts. What that means here - the same idea in two places,", " two different ideas sharing a word, or a name too common to mean anything - is", " what the code says and this page does not.", diff --git a/test/e2e.test.ts b/test/e2e.test.ts index bcf05ed..1d14eab 100644 --- a/test/e2e.test.ts +++ b/test/e2e.test.ts @@ -710,6 +710,28 @@ check expect(cov["clusters"].flatMap((c: Out) => c["explained"])).toContain("src/auth.ts"); }, 20_000); + it("tells a file it cannot read apart from one the file cap dropped", async () => { + // notes.md is in no graph language, so it is absent from the structure. Saying + // that about a file the repo-wide cap merely skipped would be a false claim, + // and the two are counted separately. + // the surface branch is where notes.md exists + await cli(["explain", "**", "--update", "--commit", "surface", "--review", explainerId]); + const out = await cli(["publish", "--review", explainerId]); + expect(String(out["published"]["coverage"])).toContain( + "outside the languages the code graph reads", + ); + const d = await api(`/api/reviews/${explainerId}`); + const cov = d["coverage"]; + expect(cov["scope"]).toBe("**"); + expect(cov["files"]["outsideGraph"]).toBe(1); + expect(cov["files"]["capped"]).toBe(0); + expect(cov["truncated"]).toBe(false); + expect(cov["unclustered"]).toEqual([ + { file: "notes.md", state: "uncovered", reason: "outsideGraph" }, + ]); + expect(cov["outsideGraph"]).toEqual([{ extension: "md", files: 1 }]); + }, 60_000); + it("answers --help per command without loading live state", async () => { const h = await cli(["threads", "--help"]); expect(h["command"]).toContain("thurview threads"); From c9aec9053bb7718ae0f619315e3f094eee0d06a4 Mon Sep 17 00:00:00 2001 From: letur Date: Wed, 9 Sep 2026 18:29:27 +0200 Subject: [PATCH 4/4] chore: no-mistakes review - Scope unresolved/truncated to explainer scope; test capped branch --- src/cli.ts | 13 ++++++-- src/coverage.ts | 17 +++++++++- src/graph.ts | 6 +++- test/coverage.test.ts | 74 +++++++++++++++++++++++++++++++++++++++++++ test/graph.test.ts | 1 + 5 files changed, 106 insertions(+), 5 deletions(-) create mode 100644 test/coverage.test.ts diff --git a/src/cli.ts b/src/cli.ts index 89c658d..1ed4d92 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -41,7 +41,13 @@ import { type ThreadTarget, } from "./store.js"; import { compileDocument, compileMap, globToRegExp, type Diagnostic } from "./document/compile.js"; -import { computeCoverage, scopeGlob, scopeGraph, type Coverage } from "./coverage.js"; +import { + computeCoverage, + scopeGlob, + scopeGraph, + scopeTruncated, + type Coverage, +} from "./coverage.js"; import type { CodeGraph } from "./graph.js"; import type { InterfaceDelta } from "./interfaces.js"; import { parseTheme, compileTheme, type CompiledTheme } from "./theme.js"; @@ -1316,12 +1322,13 @@ const commands: Record Promise> = { if (kindOf(review) === "explainer") { const scope = review.binding.name; const g0 = scopeGraph(head, scope); - const { diff: _diff, truncated, ...rest } = graph.architecture(g0, g0); + const allFiles = await g.listFiles(review.worktree, review.pins.head); + const { diff: _diff, truncated: _truncated, ...rest } = graph.architecture(g0, g0); return { commit: short(head.commit), scope, languages: pins.languages, - truncated: truncated.head, + truncated: scopeTruncated(allFiles, head, scope), ...rest, help: [ "Seed map.yaml nodes from communities, their `files` from a community's files, and edges from edges", diff --git a/src/coverage.ts b/src/coverage.ts index 57a53c6..30b933a 100644 --- a/src/coverage.ts +++ b/src/coverage.ts @@ -114,6 +114,21 @@ export function scopeGraph(g: CodeGraph, glob: string): CodeGraph { }; } +/** + * Whether the repo-wide MAX_FILES cap (graph.ts:capFiles, run over the whole + * repository before scoping) dropped at least one in-scope, graph-language + * file. `graph.truncated` alone can't answer this for a scope: it is a + * whole-repo flag from before `scopeGraph` ever filtered anything, so a + * fully-parsed small scope inside a truncated repo would read as truncated + * too. `allFiles` is the unscoped, uncapped file list at the same commit. + */ +export function scopeTruncated(allFiles: string[], graph: CodeGraph, scope: string): boolean { + const glob = scopeGlob(scope); + const inScope = glob === "**" ? () => true : (f: string) => globToRegExp(glob).test(f); + const inGraph = new Set(scopeGraph(graph, glob).files); + return allFiles.some((f) => inScope(f) && !inGraph.has(f) && isGraphLanguage(f)); +} + function extensionOf(path: string): string { const name = path.split("/").pop() ?? path; const dot = name.lastIndexOf("."); @@ -246,7 +261,7 @@ export function computeCoverage(input: CoverageInput): Coverage { .map(([extension, n]) => ({ extension, files: n })) .sort((a, b) => b.files - a.files || a.extension.localeCompare(b.extension)), owners: owners.sort((a, b) => b.files - a.files || a.node.localeCompare(b.node)), - unresolved: input.graph.unresolved, + unresolved: files.reduce((n, f) => n + (input.graph.unresolvedByFile?.[f] ?? 0), 0), truncated: capped > 0, }; record.verdict = coverageVerdict(record); diff --git a/src/graph.ts b/src/graph.ts index 6321510..5300ab4 100644 --- a/src/graph.ts +++ b/src/graph.ts @@ -39,6 +39,8 @@ export interface CodeGraph { edges: Edge[]; /** references that matched no definition, or several in other files */ unresolved: number; + /** unresolved reference count, by the file the reference appears in - lets a scoped view sum only its own files */ + unresolvedByFile: Record; /** the file list was capped at MAX_FILES; the graph is incomplete */ truncated: boolean; } @@ -237,6 +239,7 @@ export async function buildGraph(cwd: string, commit: string): Promise(); let unresolved = 0; + const unresolvedByFile: Record = {}; for (const { file, defs, refs } of pending) { // innermost enclosing definition: the last one that starts at or before the line const enclosing = (line: number): Sym => { @@ -259,6 +262,7 @@ export async function buildGraph(cwd: string, commit: string): Promise = {}): CodeGraph { + return { + commit: "deadbeef", + files: [], + symbols: [], + edges: [], + unresolved: 0, + unresolvedByFile: {}, + truncated: false, + ...overrides, + }; +} + +describe("computeCoverage", () => { + it("tells a graph-language file the repo-wide cap dropped apart from one the graph cannot read", () => { + const cov = computeCoverage({ + commit: "deadbeef", + scope: "**", + allFiles: ["src/seen.ts", "src/dropped.ts", "notes.md"], + graph: graph({ files: ["src/seen.ts"] }), + anchored: [], + owners: [], + }); + expect(cov.files.capped).toBe(1); + expect(cov.truncated).toBe(true); + expect(cov.unclustered).toContainEqual( + expect.objectContaining({ file: "src/dropped.ts", reason: "capped" }), + ); + expect(cov.unclustered).toContainEqual( + expect.objectContaining({ file: "notes.md", reason: "outsideGraph" }), + ); + expect(cov.files.outsideGraph).toBe(1); + }); + + it("sums unresolved references only from files inside the requested scope", () => { + const cov = computeCoverage({ + commit: "deadbeef", + scope: "src/in", + allFiles: ["src/in/a.ts", "src/out/b.ts"], + graph: graph({ + files: ["src/in/a.ts", "src/out/b.ts"], + unresolved: 5, + unresolvedByFile: { "src/in/a.ts": 2, "src/out/b.ts": 3 }, + }), + anchored: [], + owners: [], + }); + expect(cov.unresolved).toBe(2); + }); +}); + +describe("scopeTruncated", () => { + it("is false when the whole-repo graph is truncated but nothing inside the scope was dropped", () => { + const truncated = scopeTruncated( + ["src/in/a.ts", "src/out/dropped.ts"], + graph({ files: ["src/in/a.ts"], truncated: true }), + "src/in", + ); + expect(truncated).toBe(false); + }); + + it("is true when a graph-language file inside the scope is missing from the graph", () => { + const truncated = scopeTruncated( + ["src/in/a.ts", "src/in/dropped.ts"], + graph({ files: ["src/in/a.ts"] }), + "src/in", + ); + expect(truncated).toBe(true); + }); +}); diff --git a/test/graph.test.ts b/test/graph.test.ts index 723c9b2..0d9b58e 100644 --- a/test/graph.test.ts +++ b/test/graph.test.ts @@ -37,6 +37,7 @@ describe("buildGraph", () => { const g = await buildGraph(dir, head); expect(g.unresolved).toBe(1); expect(g.edges).toEqual([]); + expect(g.unresolvedByFile).toEqual({ "src/a.ts": 1 }); }); it("does not resolve a call to a nested non-method definition in another file", async () => {