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..1ed4d92 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -33,12 +33,21 @@ 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, + 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"; @@ -51,7 +60,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 +131,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 +141,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 +281,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 +333,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 +497,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 +696,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 +899,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 +931,7 @@ const commands: Record Promise> = { pins: review.pins, mapYaml, anchors: doc.anchors, + kind, }); diags.push(...m.diagnostics); map = m.map; @@ -764,7 +954,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 +1022,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 +1030,7 @@ const commands: Record Promise> = { at: now(), title: doc.document.title, pins: review.pins, + kind, hasMap: !!map, theme: theme?.name ?? "default", }); @@ -810,15 +1052,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 +1240,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 +1317,25 @@ 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 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: scopeTruncated(allFiles, head, scope), + ...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 +1612,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..30b933a --- /dev/null +++ b/src/coverage.ts @@ -0,0 +1,290 @@ +/** + * 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, + isGraphLanguage, + 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; + /** 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 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 */ + 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 */ + unresolved: number; + /** the repo-wide file cap excluded at least one in-scope, graph-language file */ + 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)), + }; +} + +/** + * 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("."); + 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)); + + // 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)) { + 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, + scope: glob, + files: { + total: files.length, + inGraph: graph.files.length, + outsideGraph: outsideGraphCount, + capped, + }, + 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), + reason: isGraphLanguage(f) ? ("capped" as const) : ("outsideGraph" as const), + })), + 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: files.reduce((n, f) => n + (input.graph.unresolvedByFile?.[f] ?? 0), 0), + truncated: capped > 0, + }; + 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 outside = + c.files.outsideGraph > 0 + ? ` ${c.files.outsideGraph} of them are outside the languages the code graph reads.` + : ""; + 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/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..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; } @@ -157,7 +159,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"; } @@ -172,6 +174,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 @@ -181,9 +188,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)), - ); + 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[] }[] = []; @@ -234,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 => { @@ -256,6 +262,7 @@ export async function buildGraph(cwd: string, commit: string): Promise 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 +541,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..53ef9f9 100644 --- a/src/ui/app.css +++ b/src/ui/app.css @@ -1756,3 +1756,187 @@ 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-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; + 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..211865e --- /dev/null +++ b/src/ui/views/coverage.ts @@ -0,0 +1,326 @@ +/** + * 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 ? " 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.", + ), + 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.files.capped && !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" && 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", + ), + ) + : 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/coverage.test.ts b/test/coverage.test.ts new file mode 100644 index 0000000..2316cb1 --- /dev/null +++ b/test/coverage.test.ts @@ -0,0 +1,74 @@ +import { describe, it, expect } from "vitest"; +import { computeCoverage, scopeTruncated } from "../src/coverage.ts"; +import type { CodeGraph } from "../src/graph.ts"; + +function graph(overrides: Partial = {}): 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/e2e.test.ts b/test/e2e.test.ts index 9076973..1d14eab 100644 --- a/test/e2e.test.ts +++ b/test/e2e.test.ts @@ -599,6 +599,139 @@ 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("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"); 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 () => {