diff --git a/.agents/skills/proof/SKILL.md b/.agents/skills/proof/SKILL.md index 866094fd..636ba640 100644 --- a/.agents/skills/proof/SKILL.md +++ b/.agents/skills/proof/SKILL.md @@ -212,6 +212,20 @@ Pick the read by what you need, in this order: commits' subjects. Read those commits before you rely on what the record says the code does. An anchor that still holds, and code on disk that is not committed yet, add nothing. + + **SUSPECT means the code wins.** A live Decision or Constraint whose code + moved on after it was recorded prints `SUSPECT — the code this Decision rests on changed after it was recorded: . Read the code first; where they disagree, the code is the current behaviour.` before its body + in `get`, on its find card, above the head compaction in `brief`, in + `state`, and as `(suspect: …)` on listings. The reasons: a code Citation + it cites or names went stale, a code file it names was deleted, or a + quarter of the lines a code file it names had then is rewritten or + removed since. One that rests on a dead record says so instead. A + listing with `suspect_limit` in `cap_reasons` checked only part of its + records; `proof get` checks one. Do not follow a suspect record past the code: read the + files and commits it names, build on what the code does now, and say in + your answer which recorded rule the code no longer follows. + `proof records --suspect` lists them. + 5. **Before building on a Decision: `proof find --refs all`.** It overlays the Proof records other branches change, without a checkout, tags each card `main`, `only on `, or `differs on : vs ` @@ -227,7 +241,8 @@ Pick the read by what you need, in this order: `superseded`, `retracted`, any standing a card prints) lists records by where they stand, links included. `--rests-on invalidated` lists the live records that derive from, cite, or give as evidence a record in that - standing. + standing. `--suspect` lists the live Decisions and Constraints that read + as SUSPECT. Browse digests (`list`, `records`, `relations`, `blocking-decisions`) excerpt each body at 600 chars / 12 lines (`[…truncated]`). **`proof get` digests diff --git a/.agents/skills/proof/reference.md b/.agents/skills/proof/reference.md index e568f7ba..4b857bd1 100644 --- a/.agents/skills/proof/reference.md +++ b/.agents/skills/proof/reference.md @@ -241,8 +241,11 @@ order: 3. If `cap_reasons` is not empty, narrow the query or fail closed. It can hold several sorted, duplicate-free values from `primary_records`, `displayed_edges`, `bytes`, `find_limit` (`proof find` matched more - records than it shows), and `state_limit` (`proof state` cut at least one - list short; each cut list says how many it left out). + records than it shows), `state_limit` (`proof state` cut at least one + list short; each cut list says how many it left out), and + `suspect_limit` (the suspect check stopped at a cap or its git budget, + so a record it did not flag may still be suspect; `proof get ` + checks one record on its own). 4. A hard cap alone leaves `page.has_more: false` and `next_cursor: null`. Paging and hard caps can occur together, but caps never create a cursor. @@ -254,7 +257,7 @@ digest or `summary` as a data feed. ```bash flatbread proof get [--resolve exact|head] [consistency flags] flatbread proof list [--status active,paused,...] [--limit n] [--cursor c] [consistency flags] -flatbread proof records [--kinds k1,k2] [--state s1,s2] [--status s1,s2] [--standing s1,s2] [--rests-on s1,s2] [--kind k1,k2] [--since iso] [--until iso] [--retracted exclude|include|only] [--limit n] [--cursor c] [consistency flags] +flatbread proof records [--kinds k1,k2] [--state s1,s2] [--status s1,s2] [--standing s1,s2] [--rests-on s1,s2] [--suspect] [--kind k1,k2] [--since iso] [--until iso] [--retracted exclude|include|only] [--limit n] [--cursor c] [consistency flags] flatbread proof relations --relations r1,r2 [--limit n] [--cursor c] [consistency flags] flatbread proof blocking-decisions [consistency flags] flatbread proof brief [consistency flags] @@ -298,6 +301,9 @@ flatbread proof serve [--socket .flatbread/proof/serve.sock] give as evidence a record in one of those standings, in any Effort. The record that overturned it does not count. This is the rule behind the count in `proof state`'s Dead evidence. +- `--suspect` keeps the live Decisions and Constraints that are suspect + (see [Suspect Decisions and Constraints](#suspect-decisions-and-constraints)). + The envelope's `suspect` maps each id on the page to its warning. - `--relations` values: `derives_from`, `supersedes`, `superseded_by`, `invalidates`, `invalidated_by`, `rejected_by`, `mitigated_by`, `resolved_by`, `evidence`, `cites` (one hop, explicit only). Missing targets @@ -539,6 +545,74 @@ the last 500 commits. Path-limited walks have no cap on length, as for any `git log -- `: `--follow` on the target file, trailer reads, and the deletion lookup for a path HEAD lacks. +### Suspect Decisions and Constraints + +A live Decision or Constraint is **suspect** when the code it rests on +moved or changed after it was recorded, or when it rests on dead evidence. +Every read that shows one prints, before its body (under its heading in a +digest, under its title on a find card): + +```text +SUSPECT — the code this Decision rests on changed after it was recorded: src/keys/keymap.ts: #registerBinding renamed to legacyAction in bc1c5b5 "Split key bindings by scope"; src/keys/keymap.ts: 20 of the 57 lines it had when this was recorded are rewritten or removed (git diff 1722a2a HEAD); since then: bc1c5b5 "Split key bindings by scope", 07b910e "…", 54dc6ec "…", +1 more. Read the code first; where they disagree, the code is the current behaviour. +SUSPECT — this Decision rests on records that are no longer live: fnd-… (invalidated). Check what replaced them before relying on it. +``` + +**The code wins.** The record says what was true when it was written; a +suspect one may describe code that has since been replaced. Read the files +and commits the warning names before following it, build on what the code +does now, and say which recorded rule the code no longer follows. Only a +person, or a new Decision that supersedes it, changes the record. + +It is suspect when any of these holds: + +1. **A code Citation tied to it is stale** at HEAD: its path is gone, or + its symbol is gone or renamed, by the checks in + [Code anchors at HEAD](#code-anchors-at-head) (a `changed since` anchor + alone does not count). Tied means it cites the Citation, the Citation + cites it, derives from it, or is evidence for it, the Citation is + within 2 `derives_from`, `evidence`, or `cites` links of it, or its + title or body names the Citation's symbol as code **and** it, or a + record within those 2 links, names the Citation's file. A symbol is + named as a whole word when the name can only be an identifier + (`registerBinding`, `DEFAULT_KEYMAP`, `parse2`), otherwise only as + `name(`, `.name`, `#name`, or `` `name` ``, so a symbol that is also an + English word is not found in prose, and the dotted parts of a file name + (`package.json`, `vite.config.ts`) are not `.name`. The same name in + another file is another symbol. +2. **A code file its title or body names was deleted or largely + rewritten.** A named file is the file at HEAD with that path, or the one + file whose path ends with it (`keys/keymap.ts`, `transform.ts`); docs + (`.md`, `.mdx`, `.txt`, `.rst`, `.adoc`) are left out. Deleted: HEAD and + the working tree lack it and history has the commit that deleted it; a + bare name (`legacy.ts`) counts when history deleted exactly one file of + that name. A file that only moved is followed to its new path. + Rewritten: at least max(10, min(100, a quarter of the lines the file had + then)) of the lines it had at the commit that added the record's file + are rewritten or removed at HEAD, by one `git diff --numstat` between + that commit and HEAD, so work merged from a branch counts whenever it + was committed. Lines only added (implementing the record, appending to + the file) never count, nor does a file that did not exist yet, nor a + record whose file is not committed. The warning gives the lines, the + diff to read, and up to 3 subjects of the newest commits since. +3. **It rests on dead evidence:** the `--rests-on` rule, as `proof state`'s + Dead evidence counts it, less a record superseded by a live revision of + itself (a `superseded_by` chain through records of the same kind and + title that ends at a live one). + +Where: `get` (and its summary, which leads with the warning), each digest +record (`records`, `relations`, `blocking-decisions`), find cards and the +find summary, `brief` (a block above the head compaction listing the +Effort's suspect records, and how many of them the head derives from), +`state` (a `Suspect records` section after the counts, only when there are +any), and the index line of every digest as `(suspect: code changed)` or +`(suspect: dead evidence)`. A check reads at most 24 anchors, asks +history about at most 6 gone paths, diffs at most 16 record commits +against HEAD, and has its own budget of 60 local git commands; reads that +walk history stop after 5 seconds. A check that hits any of these says +so: `cap_reasons` holds `suspect_limit`, `complete` is false, and the +summary says a record not flagged may still be suspect. `proof get ` +checks one record on its own. + ### `proof find --refs`: what other branches propose `--refs feat/a,origin/feat/b` or `--refs all` overlays records from other @@ -632,6 +706,9 @@ The Markdown has these sections, in order: Findings, and Constraints also by their own `kind` (`blocker`, `question`, `hard`, …), how many Decisions were ever reopened, and how many records are live or retracted. Exact numbers, never cut. +- **Suspect records:** shown only when the Effort has any. Each live + Decision or Constraint that is suspect, with why (see + [Suspect Decisions and Constraints](#suspect-decisions-and-constraints)). - **Open Issues and blockers:** each open Issue, blockers first, with the live Decisions that derive from it (`gates`), other live records built on it, and what it comes from. diff --git a/benchmarks/proof-build/scripts/inject.mjs b/benchmarks/proof-build/scripts/inject.mjs index a83a87dc..53b7aaaa 100644 --- a/benchmarks/proof-build/scripts/inject.mjs +++ b/benchmarks/proof-build/scripts/inject.mjs @@ -1,7 +1,10 @@ // The text that the `brief` and `dump` arms paste into the task: the // output of `proof brief` for every Effort, or every live record in full. // Both are made once per bed commit and cached, since every session on -// that bed gets the same text. +// that bed gets the same text. The dump reads record files directly, so +// it asks the CLI which records are suspect (`proof records --suspect`) +// and prints the same warning before each one's body that `proof get` +// prints. import { existsSync, readdirSync, readFileSync } from 'node:fs'; import { mkdir, readFile, rm, symlink, writeFile } from 'node:fs/promises'; import { dirname, join } from 'node:path'; @@ -60,9 +63,10 @@ const fieldValue = (value) => /** * Every live record's full text, grouped by Effort, compactions included. - * Dead means retracted, superseded, invalidated, or rejected. + * Dead means retracted, superseded, invalidated, or rejected. `suspect` + * maps a record id to the warning the CLI prints before its body. */ -export function dumpText(proofRoot) { +export function dumpText(proofRoot, suspect = {}) { const records = readRecords(proofRoot); const efforts = records.filter((r) => r.kind === 'effort').sort(byDate); const parts = []; @@ -94,7 +98,7 @@ export function dumpText(proofRoot) { f.created_at ?? '' ).slice(0, 10)} · ${f.created_by ?? ''}${ fields.length ? `\n${fields.join('\n')}` : '' - }\n\n${r.body.trim()}\n` + }\n\n${suspect[r.id] ? `> ${suspect[r.id]}\n\n` : ''}${r.body.trim()}\n` ); } } @@ -102,12 +106,12 @@ export function dumpText(proofRoot) { } /** - * `proof brief ` for every Effort, run by the real CLI in a - * throwaway clone of `bed` under `scratch` (so the bed's own folder gains - * no read cache). The clone's path is replaced with `.` in the output. + * Runs `use(clone, efforts)` in a throwaway clone of `bed` under `scratch` + * (so the bed's own folder gains no read cache), with the clone's Efforts + * oldest first. */ -export async function briefText(bed, scratch) { - const clone = join(scratch, `brief-clone-${process.pid}`); +async function inClone(bed, scratch, name, use) { + const clone = join(scratch, `${name}-clone-${process.pid}`); await rm(clone, { recursive: true, force: true }); await mkdir(dirname(clone), { recursive: true }); git(dirname(clone), 'clone', '--quiet', '--no-hardlinks', bed, clone); @@ -116,18 +120,78 @@ export async function briefText(bed, scratch) { const efforts = readRecords(join(clone, '.flatbread-proof')) .filter((r) => r.kind === 'effort') .sort(byDate); + return await use(clone, efforts); + } finally { + await rm(clone, { recursive: true, force: true }); + } +} + +/** One `proof` CLI read in `cwd`, as its JSON envelope. */ +async function proofRead(cwd, args) { + const run = await runCommand( + process.execPath, + [FLATBREAD_BIN, 'proof', ...args], + { cwd, env: cleanEnv(), timeout: 120e3 } + ); + if (run.code !== 0) + throw new Error( + `proof ${args.join(' ')} failed: ${run.stderr || run.stdout}` + ); + return JSON.parse(run.stdout); +} + +/** + * The CLI's warning for each suspect record in `bed`, by id, from + * `proof records --suspect` for every Effort, every page. When a + * listing's check stopped at a cap (`suspect_limit`), each live Decision + * and Constraint of that Effort is checked on its own with `proof get`, as + * the arms that read through `get` see it. + */ +export async function suspectBanners(bed, scratch) { + return inClone(bed, scratch, 'suspect', async (clone, efforts) => { + const banners = {}; + const records = readRecords(join(clone, '.flatbread-proof')); + for (const effort of efforts) { + let cursor; + let partial = false; + do { + const envelope = await proofRead(clone, [ + 'records', + effort.id, + '--suspect', + '--limit', + '25', + ...(cursor ? ['--cursor', cursor] : []), + ]); + Object.assign(banners, envelope.suspect ?? {}); + if (envelope.cap_reasons?.includes('suspect_limit')) partial = true; + cursor = envelope.page.next_cursor; + } while (cursor); + if (!partial) continue; + for (const r of records.filter( + (r) => + r.frontmatter.effort === effort.id && + ['decision', 'constraint'].includes(r.kind) && + !DEAD.has(r.standing) + )) { + const envelope = await proofRead(clone, ['get', r.id]); + if (envelope.suspect?.[r.id]) banners[r.id] = envelope.suspect[r.id]; + } + } + return banners; + }); +} + +/** + * `proof brief ` for every Effort, run by the real CLI in a + * throwaway clone of `bed` under `scratch`. The clone's path is replaced + * with `.` in the output. + */ +export async function briefText(bed, scratch) { + return inClone(bed, scratch, 'brief', async (clone, efforts) => { const parts = []; for (const effort of efforts) { - const run = await runCommand( - process.execPath, - [FLATBREAD_BIN, 'proof', 'brief', effort.id], - { cwd: clone, env: cleanEnv(), timeout: 120e3 } - ); - if (run.code !== 0) - throw new Error( - `proof brief ${effort.id} failed: ${run.stderr || run.stdout}` - ); - const envelope = JSON.parse(run.stdout); + const envelope = await proofRead(clone, ['brief', effort.id]); // The digest opens with the read's query metadata (hashes, cursor, // caps), which is plumbing, not memory; the Markdown after it stays. const digest = (await readFile(envelope.artifact_path, 'utf8')).replace( @@ -142,9 +206,7 @@ export async function briefText(bed, scratch) { ); } return { text: parts.join('\n'), efforts: efforts.length }; - } finally { - await rm(clone, { recursive: true, force: true }); - } + }); } /** @@ -159,7 +221,7 @@ export async function injectText(kind, bed, cache) { await mkdir(dir, { recursive: true }); const made = kind === 'dump' - ? dumpText(join(bed, '.flatbread-proof')) + ? dumpText(join(bed, '.flatbread-proof'), await suspectBanners(bed, dir)) : await briefText(bed, dir); const { text, ...about } = made; await writeFile(file, text); diff --git a/packages/flatbread/src/cli/proof-suspect.test.ts b/packages/flatbread/src/cli/proof-suspect.test.ts new file mode 100644 index 00000000..45850286 --- /dev/null +++ b/packages/flatbread/src/cli/proof-suspect.test.ts @@ -0,0 +1,606 @@ +import test from 'ava'; +import { execFile } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; +import { + lstat, + mkdir, + mkdtemp, + readFile, + rm, + symlink, + unlink, + writeFile, +} from 'node:fs/promises'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { + handleEffortBrief, + handleEffortFind, + handleEffortGet, + handleEffortRecords, + handleEffortState, + handleEffortWrite, +} from './proof.js'; +import { churnThreshold, namesSymbol } from '../proof/suspect.js'; + +// A live Decision or Constraint reads as suspect when the code it rests on +// moved or changed after it was recorded, or when it rests on dead +// evidence. Each test builds its own repository in a temp folder, with a +// commit clock that moves a minute per commit, so "after the record" never +// hangs on two commits in the same second. + +const repositoryNodeModules = fileURLToPath( + new URL('../../../../node_modules/', import.meta.url) +); +const GIT_ENV = { + GIT_AUTHOR_NAME: 'Fixture', + GIT_AUTHOR_EMAIL: 'fixture@example.com', + GIT_COMMITTER_NAME: 'Fixture', + GIT_COMMITTER_EMAIL: 'fixture@example.com', + GIT_CONFIG_NOSYSTEM: '1', + GIT_CONFIG_GLOBAL: '/dev/null', +}; + +type Context = { teardown(callback: () => void | Promise): void }; + +interface Project { + readonly cwd: string; + commit(message: string): Promise; + write(input: Record): Promise; + file(path: string, text: string): Promise; +} + +function git( + cwd: string, + env: Record, + ...args: string[] +): Promise { + return new Promise((resolve, reject) => + execFile( + 'git', + ['-c', 'commit.gpgsign=false', ...args], + { cwd, env: { ...process.env, ...GIT_ENV, ...env } }, + (error, stdout, stderr) => + error + ? reject(new Error(`${error.message}\n${stderr}`)) + : resolve(stdout) + ) + ); +} + +async function createProject(t: Context): Promise { + const cwd = await mkdtemp(join(tmpdir(), 'flatbread-proof-suspect-')); + const nodeModules = join(cwd, 'node_modules'); + await symlink(repositoryNodeModules, nodeModules, 'junction'); + t.teardown(async () => { + if ((await lstat(nodeModules).catch(() => null))?.isSymbolicLink()) + await unlink(nodeModules); + await rm(cwd, { recursive: true, force: true }); + }); + await writeFile( + join(cwd, 'flatbread.config.js'), + `import { source } from '@flatbread/source-filesystem'; +import { transformer } from '@flatbread/transformer-markdown'; +import { proofContent } from '@flatbread/proof'; +export default { + source: source(), + transformer: transformer(), + content: proofContent('.flatbread-proof'), +};` + ); + await writeFile( + join(cwd, '.gitignore'), + 'node_modules\n**/.flatbread-proof/.journal/\n.flatbread/\n' + ); + await git(cwd, {}, 'init', '-q', '-b', 'main'); + // Records carry today's created_at; commits start an hour later and move a + // minute each, so every commit comes after every record before it. + let clock = Math.floor(Date.now() / 1000) + 3600; + return { + cwd, + async commit(message) { + const date = `@${(clock += 60)} +0000`; + const env = { GIT_AUTHOR_DATE: date, GIT_COMMITTER_DATE: date }; + await git(cwd, env, 'add', '-A'); + await git(cwd, env, 'commit', '-q', '-m', message); + return (await git(cwd, {}, 'rev-parse', 'HEAD')).trim(); + }, + async write(input) { + return (await handleEffortWrite(JSON.stringify(input), { cwd })) + .artifacts[0].id; + }, + async file(path, text) { + await mkdir(join(cwd, path, '..'), { recursive: true }); + await writeFile(join(cwd, path), text); + }, + }; +} + +const short = (sha: string) => sha.slice(0, 7); +const lines = (n: number, word: string) => + Array.from({ length: n }, (_, i) => `export const ${word}${i} = ${i};`).join( + '\n' + ) + '\n'; + +async function digest(envelope: { artifact_path: string }): Promise { + return readFile(envelope.artifact_path, 'utf8'); +} + +const KEYMAP = `export const DEFAULT_KEYMAP = { canvas: { 'Mod+Z': 'undo' } }; +export function registerBinding(scope: string, chord: string, action: string) { + (DEFAULT_KEYMAP as Record>)[scope][chord] = action; +} +${lines(10, 'keyPad')}`; + +test('a symbol counts as named only where the text writes it as code', (t) => { + t.true(namesSymbol('call registerBinding at run time', 'registerBinding')); + t.true(namesSymbol('add it to DEFAULT_KEYMAP first', 'DEFAULT_KEYMAP')); + t.false(namesSymbol('records nested dispatches as a batch', 'batch')); + t.true(namesSymbol('wrap them in `batch` so', 'batch')); + t.true(namesSymbol('call batch() around them', 'batch')); + t.false(namesSymbol('the registerBindings list', 'registerBinding')); + // The dotted parts of a file name are not `.word`. + t.false(namesSymbol('Scripts are declared in package.json', 'json')); + t.false(namesSymbol('Build settings live in vite.config.ts', 'config')); + t.true(namesSymbol('it returns res.json() as is', 'json')); + t.true(namesSymbol('see store.ts#batch', 'batch')); +}); + +test('the churn threshold: ten lines, a quarter of the file, at most 100', (t) => { + t.is(churnThreshold(8), 10); + t.is(churnThreshold(60), 15); + t.is(churnThreshold(1000), 100); +}); + +test.serial( + 'a Decision whose cited code Citation went stale is suspect, as is one that derives from it; the code wins', + async (t) => { + t.timeout(180_000); + const project = await createProject(t); + const { cwd, write, commit, file } = project; + const effort = await write({ + type: 'CreateEffort', + title: 'Keys', + body: '', + }); + await file('src/keys/keymap.ts', KEYMAP); + const added = await commit('Add shortcuts'); + const citation = await write({ + type: 'WriteCitation', + effort, + title: 'Registering a shortcut', + body: `src/keys/keymap.ts#registerBinding@${added}`, + role: 'code', + }); + const decision = await write({ + type: 'WriteDecision', + effort, + title: 'Shortcuts go in one table', + body: 'Every shortcut is one row in the table.', + cites: [citation], + }); + const after = await write({ + type: 'WriteDecision', + effort, + title: 'Plugins read the table', + body: 'Plugins see what the editor runs.', + derives_from: [decision], + }); + await commit('Record the shortcut rules'); + await file( + 'src/keys/keymap.ts', + KEYMAP.replace('registerBinding', 'legacyAction') + ); + const renamed = await commit('Rename the binding helper'); + + const got = await handleEffortGet(decision, { cwd }); + const banner = `SUSPECT — the code this Decision rests on changed after it was recorded: src/keys/keymap.ts: #registerBinding renamed to legacyAction in ${short( + renamed + )} "Rename the binding helper". Read the code first; where they disagree, the code is the current behaviour.`; + t.is(got.suspect?.[decision], banner); + t.true(got.summary.startsWith(`${banner} | 1 record;`), got.summary); + const text = await digest(got); + // Before the body: under the heading, ahead of the frontmatter. + t.true(text.includes(`### ${decision}\n> ${banner}\n\n\`\`\`yaml`), text); + t.true(text.includes(`(#${decision}) (suspect: code changed)`), text); + // Two links out, through derives_from and cites. + const derived = await handleEffortGet(after, { cwd }); + t.regex( + derived.suspect?.[after] ?? '', + /#registerBinding renamed to legacyAction/ + ); + } +); + +test.serial( + 'a Decision whose text names a stale symbol is suspect without a link; a prose word is not a symbol', + async (t) => { + t.timeout(180_000); + const { cwd, write, commit, file } = await createProject(t); + const effort = await write({ + type: 'CreateEffort', + title: 'Keys', + body: '', + }); + await file('src/keys/keymap.ts', KEYMAP); + await file('src/batch.ts', 'export function batch() {}\n'); + const added = await commit('Add shortcuts'); + await write({ + type: 'WriteCitation', + effort, + title: 'Registering a shortcut', + body: `src/keys/keymap.ts#registerBinding@${added}`, + role: 'code', + }); + await write({ + type: 'WriteCitation', + effort, + title: 'Batches', + body: `src/batch.ts#batch@${added}`, + role: 'code', + }); + const named = await write({ + type: 'WriteDecision', + effort, + title: 'Plugins add shortcuts at run time', + body: 'A plugin calls registerBinding in keys/keymap.ts with a scope and a chord.', + }); + // Through its chain: the Decision it derives from names the file. + const chained = await write({ + type: 'WriteDecision', + effort, + title: 'Plugins see their own shortcuts', + body: 'What a plugin adds with registerBinding shows up at once.', + derives_from: [named], + }); + // The same name in another file is another symbol. + const elsewhere = await write({ + type: 'WriteDecision', + effort, + title: 'The menu registers its own items', + body: 'src/menu.ts has a registerBinding of its own for menu items.', + }); + const prose = await write({ + type: 'WriteDecision', + effort, + title: 'Undo takes a whole batch', + body: 'One batch of edits is one undo step.', + }); + await commit('Record the rules'); + await file( + 'src/keys/keymap.ts', + KEYMAP.replace('registerBinding', 'legacyAction') + ); + await file('src/batch.ts', 'export function group() {}\n'); + await commit('Rename helpers'); + + const got = await handleEffortGet(named, { cwd }); + t.regex( + got.suspect?.[named] ?? '', + /src\/keys\/keymap\.ts: #registerBinding renamed to legacyAction/ + ); + t.regex( + (await handleEffortGet(chained, { cwd })).suspect?.[chained] ?? '', + /#registerBinding renamed to legacyAction/ + ); + t.is((await handleEffortGet(elsewhere, { cwd })).suspect, undefined); + const plain = await handleEffortGet(prose, { cwd }); + t.is(plain.suspect, undefined); + } +); + +test.serial( + 'a Decision whose named file is gone, or was largely rewritten since it was recorded, is suspect; a small edit is not', + async (t) => { + t.timeout(180_000); + const { cwd, write, commit, file } = await createProject(t); + const effort = await write({ + type: 'CreateEffort', + title: 'Code', + body: '', + }); + await file('src/keys/keymap.ts', KEYMAP); + await file('src/legacy/loader.ts', lines(20, 'loader')); + await file('src/panel/fields.ts', lines(40, 'field')); + await commit('Add the code'); + const rewritten = await write({ + type: 'WriteDecision', + effort, + title: 'Shortcuts are registered in keys/keymap.ts', + body: 'Add a row to the table.', + }); + const gone = await write({ + type: 'WriteConstraint', + effort, + title: 'Old saves load through src/legacy/loader.ts', + body: 'Never drop the loader.', + kind: 'hard', + }); + const holds = await write({ + type: 'WriteDecision', + effort, + title: 'Fields parse in panel/fields.ts', + body: 'One parser for every field.', + }); + const recorded = await commit('Record the rules'); + await file( + 'src/keys/keymap.ts', + `${lines(8, 'scopeA')}${KEYMAP.split('\n').slice(0, 4).join('\n')}\n` + ); + const first = await commit('Split bindings by scope'); + await file('src/keys/keymap.ts', `${lines(8, 'scopeB')}`); + const second = await commit('Move the table out'); + await rm(join(cwd, 'src/legacy/loader.ts')); + const dropped = await commit('Drop the legacy loader'); + await file( + 'src/panel/fields.ts', + lines(40, 'field').replace('field3 = 3', 'field3 = 33') + ); + await commit('Fix a typo'); + + const got = await handleEffortGet(rewritten, { cwd }); + t.regex( + got.suspect?.[rewritten] ?? '', + new RegExp( + `^SUSPECT — the code this Decision rests on changed after it was recorded: src/keys/keymap\\.ts: 14 of the 14 lines it had when this was recorded are rewritten or removed \\(git diff ${short( + recorded + )} HEAD\\); since then: ${short(second)} "Move the table out", ${short( + first + )} "Split bindings by scope"\\. Read the code first` + ) + ); + const loader = await handleEffortGet(gone, { cwd }); + t.is( + loader.suspect?.[gone], + `SUSPECT — the code this Constraint rests on changed after it was recorded: src/legacy/loader.ts is gone at HEAD: deleted in ${short( + dropped + )} "Drop the legacy loader". Read the code first; where they disagree, the code is the current behaviour.` + ); + // Two changed lines of 40: the code still holds, and nothing is said. + const held = await handleEffortGet(holds, { cwd }); + t.is(held.suspect, undefined); + t.false((await digest(held)).includes('SUSPECT')); + + // Listings: a marker on the index line, and --suspect keeps only them. + const listed = await handleEffortRecords(effort, { + cwd, + kinds: ['decision', 'constraint'], + }); + const text = await digest(listed); + t.true(text.includes(`(#${rewritten}) (suspect: code changed)`), text); + t.true(text.includes(`(#${holds})\n`), text); + t.regex(listed.summary, /; 2 suspect \(/); + const only = await handleEffortRecords(effort, { cwd, suspect: true }); + t.deepEqual( + Object.keys(only.suspect ?? {}).sort(), + [gone, rewritten].sort() + ); + t.is(only.page.returned, 2); + + // find cards carry the warning under the title. + const found = await handleEffortFind('shortcuts registered', { + cwd, + effort, + }); + t.regex( + found.cards, + new RegExp( + `1\\. ${rewritten} \\| decision \\| proposed \\| [^\\n]+\\n "Shortcuts are registered in keys/keymap\\.ts"\\n SUSPECT — the code this Decision rests on changed` + ) + ); + t.regex(found.summary, new RegExp(`SUSPECT ${rewritten}`)); + + // brief lists them above the body; state lists them after the counts. + const brief = await digest(await handleEffortBrief(effort, { cwd })); + t.regex( + brief, + /# Proof read\n> \*\*SUSPECT\*\* — 2 live records in this Effort may be out of date/ + ); + t.true(brief.includes(`> - ${rewritten} "Shortcuts are registered`)); + const state = await handleEffortState(effort, { cwd }); + t.regex( + state.markdown, + /### Counts[\s\S]*### Suspect records\n\*\*SUSPECT\*\*: 2 live records may be out of date/ + ); + t.true( + state.markdown.includes(`- ${gone} (live): src/legacy/loader.ts is gone`) + ); + } +); + +test.serial( + 'a Decision that rests on dead evidence is suspect, and says what to check', + async (t) => { + t.timeout(180_000); + const { cwd, write, commit } = await createProject(t); + const effort = await write({ + type: 'CreateEffort', + title: 'Perf', + body: '', + }); + const finding = await write({ + type: 'WriteFinding', + effort, + title: 'Hit testing takes 2 ms', + body: 'Measured on 5,000 nodes.', + kind: 'measurement', + }); + const decision = await write({ + type: 'WriteDecision', + effort, + title: 'Keep hit testing linear', + body: 'Linear is fast enough.', + derives_from: [finding], + }); + const wrong = await write({ + type: 'WriteFinding', + effort, + title: 'The 2 ms figure was a warm cache', + body: 'Cold, it is 40 ms.', + kind: 'measurement', + }); + await write({ type: 'Invalidate', findingId: wrong, targetId: finding }); + await commit('Record the measurements'); + + const got = await handleEffortGet(decision, { cwd }); + t.is( + got.suspect?.[decision], + `SUSPECT — this Decision rests on records that are no longer live: ${finding} (invalidated). Check what replaced them before relying on it.` + ); + t.true( + (await digest(got)).includes(`(#${decision}) (suspect: dead evidence)`) + ); + } +); + +test.serial( + 'implementing, appending to, moving, or documenting a named file is not suspect; branch work merged after the record and a deleted bare name are', + async (t) => { + t.timeout(180_000); + const { cwd, write, commit, file } = await createProject(t); + const effort = await write({ + type: 'CreateEffort', + title: 'Code', + body: '', + }); + await file('src/append.ts', lines(30, 'stub')); + await file('src/utils.ts', lines(30, 'util')); + await file('src/keys.ts', lines(40, 'key')); + await file('src/legacy.ts', lines(20, 'old')); + await file('NOTES.md', `${lines(40, 'note')}`); + await commit('Add the code'); + // Branch work committed before the record lands, merged after it: its + // commit time is earlier than the record's commit. + const base = Number( + (await git(cwd, {}, 'log', '-1', '--format=%ct')).trim() + ); + const early = `@${base + 30} +0000`; + const env = { GIT_AUTHOR_DATE: early, GIT_COMMITTER_DATE: early }; + await git(cwd, {}, 'checkout', '-q', '-b', 'feat'); + await file('src/keys.ts', lines(40, 'scope')); + await git(cwd, env, 'add', '-A'); + await git(cwd, env, 'commit', '-q', '-m', 'Rewrite keys on a branch'); + await git(cwd, {}, 'checkout', '-q', 'main'); + const decide = (title: string, body: string) => + write({ type: 'WriteDecision', effort, title, body }); + const implement = await decide( + 'Config is parsed in src/config.ts', + 'One parser for the config.' + ); + const append = await decide( + 'Validation lives in src/append.ts', + 'Add checks at the end.' + ); + const move = await decide( + 'Helpers live in src/utils.ts', + 'Small pure helpers only.' + ); + const doc = await decide( + 'Working notes live in NOTES.md', + 'Rewrite them freely.' + ); + const merged = await decide( + 'Shortcuts are listed in src/keys.ts', + 'One row per shortcut.' + ); + const bare = await decide( + 'Old files load through legacy.ts', + 'Keep the loader.' + ); + await commit('Record the rules'); + await file('src/config.ts', lines(40, 'config')); + await file('src/append.ts', `${lines(30, 'stub')}${lines(12, 'check')}`); + await mkdir(join(cwd, 'src/lib'), { recursive: true }); + await git(cwd, {}, 'mv', 'src/utils.ts', 'src/lib/utils.ts'); + await file('NOTES.md', lines(40, 'draft')); + await rm(join(cwd, 'src/legacy.ts')); + await commit('Implement, append, move, and drop'); + const late = `@${base + 3600} +0000`; + await git( + cwd, + { GIT_AUTHOR_DATE: late, GIT_COMMITTER_DATE: late }, + 'merge', + '-q', + '--no-ff', + 'feat', + '-m', + 'Merge the branch' + ); + + for (const id of [implement, append, move, doc]) + t.is((await handleEffortGet(id, { cwd })).suspect, undefined, id); + t.regex( + (await handleEffortGet(merged, { cwd })).suspect?.[merged] ?? '', + /src\/keys\.ts: 40 of the 40 lines it had when this was recorded are rewritten or removed/ + ); + t.regex( + (await handleEffortGet(bare, { cwd })).suspect?.[bare] ?? '', + /src\/legacy\.ts is gone at HEAD: deleted in [0-9a-f]{7} "Implement, append, move, and drop"/ + ); + } +); + +test.serial( + 'dead evidence a live revision of itself replaced is not suspect; a check that stops at a cap says so', + async (t) => { + t.timeout(180_000); + const { cwd, write, commit, file } = await createProject(t); + const effort = await write({ + type: 'CreateEffort', + title: 'Rules', + body: '', + }); + const first = await write({ + type: 'WriteConstraint', + effort, + title: 'The mutation list stays small', + body: 'Thirteen mutations.', + kind: 'hard', + }); + const decision = await write({ + type: 'WriteDecision', + effort, + title: 'Blobs are a citation field', + body: 'Not a new mutation.', + derives_from: [first], + }); + await write({ + type: 'WriteConstraint', + effort, + title: 'The mutation list stays small', + body: 'Sixteen mutations.', + kind: 'hard', + supersedes: [first], + }); + t.is((await handleEffortGet(decision, { cwd })).suspect, undefined); + + // Seven Decisions name seven deleted files: one check asks history + // about six, so a listing is marked incomplete. + const ids: string[] = []; + for (let i = 0; i < 7; i++) { + await file(`src/gone${i}/mod.ts`, lines(12, `m${i}`)); + ids.push( + await write({ + type: 'WriteDecision', + effort, + title: `Module ${i} lives in src/gone${i}/mod.ts`, + body: 'Keep it.', + }) + ); + } + await commit('Add modules and rules'); + for (let i = 0; i < 7; i++) + await rm(join(cwd, `src/gone${i}`), { recursive: true }); + await commit('Drop the modules'); + const listed = await handleEffortRecords(effort, { cwd, suspect: true }); + t.true(listed.page.returned >= 6); + t.regex(listed.summary, /incomplete: suspect_limit/); + t.regex(listed.summary, /the suspect check stopped at its limit/); + t.true((await digest(listed)).includes('cap_reasons: ["suspect_limit"]')); + for (const id of ids) + t.regex( + (await handleEffortGet(id, { cwd })).suspect?.[id] ?? '', + /is gone at HEAD/ + ); + } +); diff --git a/packages/flatbread/src/cli/proof.test.ts b/packages/flatbread/src/cli/proof.test.ts index acf13845..800946ae 100644 --- a/packages/flatbread/src/cli/proof.test.ts +++ b/packages/flatbread/src/cli/proof.test.ts @@ -1853,12 +1853,21 @@ export default { source: source(), transformer: transformer(), content: proofCon ); t.true((await digest(secondId)).includes('- Stale: no.')); - // Other records print what they printed before. + // Other records print what they printed before, but for the warning + // that the Decision now rests on a replaced record. const decisionAfter = await digest(decision); t.false(decisionAfter.includes('## Compaction')); t.false(decisionAfter.includes('## Changes since compaction')); + t.true( + decisionAfter.includes( + `> SUSPECT — this Decision rests on records that are no longer live: ${measured} (superseded).` + ) + ); t.is( - decisionAfter.replace(/^served_generation: .*$/m, ''), + decisionAfter + .replace(/^served_generation: .*$/m, '') + .replace(' (suspect: dead evidence)', '') + .replace(/^> SUSPECT — .*\n\n/m, ''), decisionBefore.replace(/^served_generation: .*$/m, '') ); } @@ -1959,7 +1968,11 @@ export default { source: source(), transformer: transformer(), content: proofCon 'invalidated,rejected' ); t.is(spawned.code, 0, spawned.stderr); - t.regex(JSON.parse(spawned.stdout).summary, /^1 record;/); + // A lone suspect record's warning leads the summary. + t.regex( + JSON.parse(spawned.stdout).summary, + /^SUSPECT — this Decision rests on records that are no longer live: .* \| 1 record;/ + ); const refused = await runCli( cwd, 'proof', diff --git a/packages/flatbread/src/cli/proof.ts b/packages/flatbread/src/cli/proof.ts index 6c3ee90f..fccc83fc 100644 --- a/packages/flatbread/src/cli/proof.ts +++ b/packages/flatbread/src/cli/proof.ts @@ -51,6 +51,7 @@ export interface EffortCliOptions { retracted?: RetractedMode; standing?: string[]; restsOn?: string[]; + suspect?: boolean; effort?: string; all?: boolean; asOf?: string; @@ -94,6 +95,10 @@ export function mapEffortCliOptions( : undefined, standing: split(options.standing), restsOn: split(options['rests-on']), + suspect: + options.suspect === true || options.suspect === 'true' + ? true + : undefined, effort: typeof options.effort === 'string' ? options.effort : undefined, all: options.all === true || options.all === 'true' ? true : undefined, // The option parser turns `--as-of 2026` into a number; keep it as @@ -249,6 +254,7 @@ export async function handleEffortRecords( retracted: options.retracted, standing: options.standing, restsOn: options.restsOn, + suspect: options.suspect, page: { limit: options.limit, cursor: options.cursor }, }); } @@ -672,6 +678,10 @@ export function registerProofCommands(prog: any): void { '--rests-on ', 'Live records that derive from, cite, or give as evidence a record in these dead standings: superseded, invalidated, rejected, retracted' ) + .option( + '--suspect', + 'Live Decisions and Constraints whose code changed after they were recorded, or that rest on dead evidence' + ) ); recordsOptions( prog.command('proof records ', 'Read records in an Effort') diff --git a/packages/flatbread/src/cli/serve.test.ts b/packages/flatbread/src/cli/serve.test.ts index a26951ce..685acaf4 100644 --- a/packages/flatbread/src/cli/serve.test.ts +++ b/packages/flatbread/src/cli/serve.test.ts @@ -415,7 +415,10 @@ test.serial( cwd ); t.is(resting.code, 0, resting.stderr); - t.regex(JSON.parse(resting.stdout).summary, /^1 record; proposed 1;/); + t.regex( + JSON.parse(resting.stdout).summary, + /^SUSPECT — this Decision rests on records that are no longer live: .* \| 1 record; proposed 1;/ + ); const refused = await runRead( ['records', effortId, '--state', 'invalidated'], cwd diff --git a/packages/flatbread/src/proof/find.ts b/packages/flatbread/src/proof/find.ts index 17c202cd..f0be87bf 100644 --- a/packages/flatbread/src/proof/find.ts +++ b/packages/flatbread/src/proof/find.ts @@ -522,6 +522,12 @@ export interface FindHit { * was renamed. The renderer prints them as given and never cuts them. */ readonly codeNotes?: readonly string[]; + /** + * Set by the caller: the warning for a live Decision or Constraint whose + * code changed after it was recorded, or that rests on dead evidence. + * Printed under the title and never cut. + */ + readonly suspect?: string; /** With `governed`: the ties of every chain member, strongest first. */ readonly via?: readonly FindVia[]; /** With `--refs`: where the shown version lives, as `only on feat/a`. */ @@ -1271,9 +1277,9 @@ function attachedLine(attached: NonNullable): string { const VIAS_SHOWN = 3; -/** The `via:`, `branch:`, and code note lines, which are never cut. */ +/** The suspect, `via:`, `branch:`, and code note lines, which are never cut. */ function tieLines(hit: FindHit): string[] { - const lines: string[] = []; + const lines: string[] = hit.suspect ? [` ${hit.suspect}`] : []; const vias = (hit.via ?? []).filter((via) => via.text); if (vias.length) lines.push( diff --git a/packages/flatbread/src/proof/history.ts b/packages/flatbread/src/proof/history.ts index b0644acf..558ecdff 100644 --- a/packages/flatbread/src/proof/history.ts +++ b/packages/flatbread/src/proof/history.ts @@ -66,6 +66,25 @@ export interface ShortCommit { readonly subject: string; } +/** A commit with its committer time. */ +export interface CommitAt { + readonly sha: string; + /** Committer time, in ms. */ + readonly time: number; +} + +/** Lines one commit added to and deleted from one file. */ +export interface LineChange { + readonly added: number; + readonly deleted: number; +} + +/** A commit that touched files. */ +export interface FileCommit extends CommitAt { + readonly subject: string; + readonly files: readonly string[]; +} + /** What HEAD says about one code anchor. */ export interface AnchorStatus { readonly anchor: CodeAnchor; @@ -89,6 +108,15 @@ export interface AnchorStatus { } export const HISTORY_GIT_CALLS = 60; +/** Commits `recentCommits` reads, at most. */ +const RECENT_MAX = 200; +/** + * A history read that may walk many commits is stopped after this long, + * and then says nothing; `timedOut` records that it was. + */ +export const SLOW_READ_MS = 5000; +/** The same for a read whose result is only shown, never decides. */ +const SHOWN_READ_MS = 2000; const FOLLOW_RENAMES = 50; const FOLDER_ADDS = 100; const SINCE_MAX = 200; @@ -109,7 +137,9 @@ interface GitOutput { function runGit( cwd: string, args: readonly string[], - input?: string + input?: string, + timeout?: number, + onTimeout?: () => void ): Promise { return new Promise((resolveGit) => { const child = execFile( @@ -120,9 +150,11 @@ function runGit( env: { ...process.env, ...NO_FETCH_ENV }, maxBuffer: 64 * 1024 * 1024, encoding: 'utf8', + ...(timeout ? { timeout, killSignal: 'SIGKILL' as const } : {}), }, (error, stdout) => { if (!error) return resolveGit({ code: 0, out: stdout }); + if ((error as { killed?: unknown }).killed) onTimeout?.(); const code = (error as { code?: unknown }).code; resolveGit( typeof code === 'number' ? { code, out: stdout ?? '' } : undefined @@ -429,8 +461,12 @@ export class CodeHistory { private readonly texts = new Map>(); private headSha?: Promise; private windowBase?: Promise; + private fileList?: Promise; + private recordAdds?: Promise>; /** Set once a read was skipped because the call budget ran out. */ capped = false; + /** Set once a read was stopped after SLOW_READ_MS. */ + timedOut = false; constructor( readonly top: string, @@ -459,7 +495,8 @@ export class CodeHistory { private run( args: readonly string[], - input?: string + input?: string, + timeout?: number ): Promise { const key = JSON.stringify([args, input ?? null]); let result = this.memo.get(key); @@ -469,7 +506,9 @@ export class CodeHistory { return Promise.resolve(undefined); } this.calls++; - result = runGit(this.top, args, input); + result = runGit(this.top, args, input, timeout, () => { + this.timedOut = true; + }); this.memo.set(key, result); } return result; @@ -477,9 +516,10 @@ export class CodeHistory { private async ok( args: readonly string[], - input?: string + input?: string, + timeout?: number ): Promise { - const result = await this.run(args, input); + const result = await this.run(args, input, timeout); return result?.code === 0 ? result.out : undefined; } @@ -1240,6 +1280,163 @@ export class CodeHistory { return found; } + /** Every file in HEAD's tree, in one `ls-tree` call. */ + headFiles(): Promise { + return (this.fileList ??= this.head().then(async (head) => + head + ? (await this.ok(['ls-tree', '-r', '-z', '--name-only', head])) + ?.split('\0') + .filter(Boolean) + : undefined + )); + } + + /** + * The commit that first added each record file under the Proof folder, + * keyed by file name less `.md` (a record's id), in one `git log` call. + */ + recordCommits(): Promise> { + return (this.recordAdds ??= this.head().then(async (head) => { + const found = new Map(); + if (!head) return found; + const out = await this.ok( + [ + 'log', + '--diff-filter=A', + '--no-renames', + '--format=%x1e%H%x1f%ct', + '--name-only', + head, + '--', + this.proofRoot || '.', + ], + undefined, + SLOW_READ_MS + ); + // Newest first, so the last add of a file is its first. + for (const { sha, subject, files } of entries(out ?? '')) { + const time = Number(subject) * 1000; + if (!Number.isFinite(time)) continue; + for (const [file] of files) { + const name = file.slice(file.lastIndexOf('/') + 1); + if (name.endsWith('.md')) found.set(name.slice(0, -3), { sha, time }); + } + } + return found; + })); + } + + /** + * Lines added and deleted between `base` and HEAD's tree, by HEAD's path, + * for `paths`, in one `git diff --numstat` call (no history walk). A + * rename between them whose old and new paths are both in `paths` is + * followed, so a move counts only the lines it changed. Binary files are + * left out; undefined when git could not say. + */ + async diffStat( + base: string, + paths: readonly string[] + ): Promise | undefined> { + const head = await this.head(); + const list = [...new Set(paths)].sort(); + const found = new Map(); + if (!head || !list.length) return found; + const out = await this.ok( + ['diff', '--numstat', '-z', '-M', base, head, '--', ...list], + undefined, + SLOW_READ_MS + ); + if (out === undefined) return undefined; + const fields = out.split('\0'); + for (let i = 0; i < fields.length; i++) { + const [added, deleted, path] = fields[i].split('\t'); + if (added === undefined || deleted === undefined) continue; + let now = path; + // A rename: the old and new paths follow as their own fields. + if (path === '') { + now = fields[i + 2]; + i += 2; + } + const change = { added: Number(added), deleted: Number(deleted) }; + if ( + now && + Number.isFinite(change.added) && + Number.isFinite(change.deleted) + ) + found.set(now, change); + } + return found; + } + + /** + * The newest commits in HEAD's history after `since` (ms) that touched + * any of `paths`, at most RECENT_MAX, with the files of `paths` each + * touched; one `git log` call stopped after SHOWN_READ_MS, since what it + * finds is only shown. + */ + async recentCommits( + paths: readonly string[], + since: number + ): Promise { + const head = await this.head(); + const list = [...new Set(paths)].sort(); + if (!head || !list.length || !Number.isFinite(since)) return []; + const out = await this.ok( + [ + 'log', + '--no-renames', + '--name-only', + `--max-count=${RECENT_MAX}`, + `--since=${new Date(since).toISOString()}`, + '--format=%x1e%H%x1f%ct%x1f%s', + head, + '--', + ...list, + ], + undefined, + SHOWN_READ_MS + ); + if (out === undefined) return undefined; + return out + .split('\x1e') + .filter((entry) => entry.trim()) + .flatMap((entry) => { + const [first, ...rest] = entry.split('\n'); + const [sha, ct, subject = ''] = first.split('\x1f'); + const time = Number(ct) * 1000; + if (!sha || !Number.isFinite(time)) return []; + return [ + { sha: sha.trim(), time, subject, files: rest.filter(Boolean) }, + ]; + }); + } + + /** How many lines each text file has at HEAD, in one `git grep` call. */ + async lineCounts(paths: readonly string[]): Promise> { + const counts = new Map(); + const head = await this.head(); + const list = [...new Set(paths)].sort(); + if (!head || !list.length) return counts; + const result = await this.run([ + 'grep', + '-I', + '-c', + '-e', + '', + head, + '--', + ...list, + ]); + for (const line of result?.out.split('\n') ?? []) { + if (!line.startsWith(`${head}:`)) continue; + const rest = line.slice(head.length + 1); + const cut = rest.lastIndexOf(':'); + const n = Number(rest.slice(cut + 1)); + if (cut > 0 && Number.isFinite(n)) counts.set(rest.slice(0, cut), n); + } + return counts; + } + /** * Where each path HEAD and the working tree lack went, for paths history * knows; at most `max`. @@ -1265,6 +1462,41 @@ export class CodeHistory { }); return fates; } + + /** + * For bare file names such as `legacy.ts`, the paths with that name that + * a commit in HEAD's history deleted, in one `git log` call stopped after + * SLOW_READ_MS. Keys are the names. + */ + async deletedPaths(names: readonly string[]): Promise> { + const found = new Map(); + const list = [...new Set(names)].filter((name) => + /^[\w@+-][\w.@+-]*$/.test(name) + ); + const head = await this.head(); + if (!head || !list.length) return found; + const out = await this.ok( + [ + 'log', + '--diff-filter=D', + '--no-renames', + '--name-only', + '--format=', + head, + '--', + ...list.map((name) => `:(glob)**/${name}`), + ], + undefined, + SLOW_READ_MS + ); + for (const path of new Set((out ?? '').split('\n').filter(Boolean))) + for (const name of list) + if (path === name || path.endsWith(`/${name}`)) { + if (!found.has(name)) found.set(name, []); + (found.get(name) as string[]).push(path); + } + return found; + } } const ALIASES_MAX = 12; diff --git a/packages/flatbread/src/proof/read.ts b/packages/flatbread/src/proof/read.ts index 70197a92..1ab6c08c 100644 --- a/packages/flatbread/src/proof/read.ts +++ b/packages/flatbread/src/proof/read.ts @@ -83,6 +83,15 @@ import { restsOnDead, type EffortState, } from './state.js'; +import { + findSuspects, + SuspectMap, + suspectable, + suspectBanner, + suspectMarker, + suspectPartial, + type Suspect, +} from './suspect.js'; import { createHash } from 'node:crypto'; import { readdir, realpath, stat } from 'node:fs/promises'; import { join, relative, resolve } from 'node:path'; @@ -635,6 +644,14 @@ async function render( * worked out for the code Citations on the page. */ codeNotes?: string[]; + /** + * Suspect records, by id. Left out, they are worked out for the live + * Decisions and Constraints on the page, when `projection` is given. + */ + suspect?: ReadonlyMap; + projection?: EngineProjection; + /** Lines printed under the digest's title, before the index. */ + notices?: string[]; } = {} ): Promise { const served = await servedGeneration(options.rootDir, options.consistency); @@ -660,7 +677,30 @@ async function render( : 0; const selected = records.slice(offset, offset + limit); const hasMore = offset + selected.length < records.length; - const { codeNotes: given, ...digestExtra } = extra; + const { + codeNotes: given, + suspect: suspectGiven, + projection, + ...digestExtra + } = extra; + const primary = selected.filter((record) => !record.foreign_reference); + const suspect = + suspectGiven ?? + (projection + ? await suspectsOn(projection, options, primary) + : new SuspectMap()); + // A check that stopped short may have missed a suspect record. + const suspectCut = suspectPartial(suspect); + const flagged = primary.filter((record) => suspect.has(record.id)); + const flags = Object.fromEntries( + flagged.map((record) => { + const one = suspect.get(record.id) as Suspect; + return [ + record.id, + { banner: suspectBanner(one), marker: suspectMarker(one) }, + ]; + }) + ); let codeNotes = given; if (!codeNotes) { const citations = selected.filter(isCodeCitation); @@ -709,6 +749,15 @@ async function render( : null, hints, ...digestExtra, + ...(suspectCut + ? { + capReasons: [ + ...(digestExtra.capReasons ?? []), + 'suspect_limit' as const, + ], + } + : {}), + ...(flagged.length ? { flags } : {}), sections: [ ...(digestExtra.sections ?? []), ...(codeNotes.length @@ -721,19 +770,74 @@ async function render( : []), ], }); - if (!codeNotes.length) return envelope; - const summary = `${envelope.summary}; ${codeNotes.length} code ${ - codeNotes.length === 1 ? 'note' : 'notes' - } (## Code anchors at HEAD): ${codeNotes[0]}`; + if (!codeNotes.length && !flagged.length && !suspectCut) return envelope; + // A lone suspect record's warning leads; on a list, a count and the first. + const lead = + flagged.length && primary.length === 1 + ? `${flags[flagged[0].id].banner} | ` + : ''; + const summary = `${lead}${envelope.summary}${ + flagged.length && !lead + ? `; ${flagged.length} suspect (${flagged + .slice(0, 3) + .map((record) => record.id) + .join(', ')}${ + flagged.length > 3 ? `, +${flagged.length - 3} more` : '' + }): each rests on code that changed after it was recorded, or on dead evidence; each one's warning is under its heading` + : '' + }${ + suspectCut + ? '; the suspect check stopped at its limit, so a record not flagged here may still be suspect (`proof get ` checks one record)' + : '' + }${ + codeNotes.length + ? `; ${codeNotes.length} code ${ + codeNotes.length === 1 ? 'note' : 'notes' + } (## Code anchors at HEAD): ${codeNotes[0]}` + : '' + }`; return { ...envelope, summary: [...summary].length <= 640 ? summary : `${[...summary].slice(0, 639).join('')}…`, + ...(flagged.length + ? { + suspect: Object.fromEntries( + flagged.map((record) => [record.id, flags[record.id].banner]) + ), + } + : {}), }; } +/** + * The suspect records among `records`: the live Decisions and + * Constraints whose code changed after they were recorded, or that rest + * on dead evidence. Reads the `find` index for links and git history at + * HEAD for code, with its own call budget. + */ +async function suspectsOn( + projection: EngineProjection, + options: EffortProjectionOptions, + records: readonly ReadRecord[], + history?: CodeHistory +): Promise { + if (!records.some(suspectable)) return new SuspectMap(); + const index = await projection.findIndex(); + let check = history; + if (!check) { + const repo = await proofRepository(options.rootDir); + check = repo ? new CodeHistory(repo.top, repo.root) : undefined; + } + return findSuspects( + records.map((record) => index.records.get(record.id) ?? record), + index, + check + ); +} + export async function getRecord( id: string, options: EffortProjectionOptions & { resolve?: 'exact' | 'head' } @@ -860,6 +964,7 @@ export async function getRecord( fullBody: true, ...(sections ? { sections } : {}), codeNotes, + projection, } ); } @@ -964,6 +1069,12 @@ export async function effortRecords( standing?: string[]; /** Keeps live records that rest on a record in one of these dead standings. */ restsOn?: string[]; + /** + * Keeps live Decisions and Constraints that are suspect: the code they + * rest on changed after they were recorded, or they rest on dead + * evidence. + */ + suspect?: boolean; page?: { cursor?: string; limit?: number }; } ): Promise { @@ -1042,6 +1153,14 @@ export async function effortRecords( ...records.filter((record) => restsOnDead(record, get, restsOn).length) ); } + let suspectOnly: SuspectMap | undefined; + if (options.suspect) { + suspectOnly = await suspectsOn(projection, options, records); + const kept = records.filter((record) => + (suspectOnly as SuspectMap).has(record.id) + ); + records.splice(0, records.length, ...kept); + } return render( options, { @@ -1053,6 +1172,7 @@ export async function effortRecords( ...(retracted === 'exclude' ? {} : { retracted }), ...(standing ? { standing } : {}), ...(restsOn ? { rests_on: restsOn } : {}), + ...(options.suspect ? { suspect: true } : {}), page: options.page, }, records, @@ -1062,7 +1182,8 @@ export async function effortRecords( ...kinds .slice(0, 8) .map((kind) => `effortRecords("${effortId}", { kinds: ["${kind}"] })`), - ] + ], + suspectOnly ? { suspect: suspectOnly } : { projection } ); } @@ -1195,7 +1316,8 @@ export async function relations( }, records, edges, - [`getRecord("${fromId}")`] + [`getRecord("${fromId}")`], + { projection } ); } @@ -1241,7 +1363,7 @@ export async function blockingDecisions( `effortRecords("${effortId}", { kinds: ["issue"], where: { kind: ["blocker"], status: ["open"] } })` ), ], - { relatedRecords: issues } + { relatedRecords: issues, projection } ); } @@ -1499,7 +1621,9 @@ export async function effortBrief( const changedIds = [...stale, ...changes] .filter((change) => change.to !== null) .map((change) => `getRecord("${change.id}")`); - return render( + const suspect = await suspectsOn(projection, options, records); + const notices = suspectNotices(effortId, [...suspect.values()], byId, head); + const envelope = await render( options, { type: 'effortBrief', effort_id: effortId }, head ? [head] : [], @@ -1516,8 +1640,77 @@ export async function effortBrief( fullBody: true, sections, compaction, + suspect, + notices, } ); + if (!suspect.size) return envelope; + const summary = `${envelope.summary}; SUSPECT ${suspect.size}: ${[ + ...suspect.keys(), + ] + .slice(0, 3) + .join(', ')}${ + suspect.size > 3 ? `, +${suspect.size - 3} more` : '' + } (each rests on code that changed after it was recorded, or on dead evidence; listed above the body)`; + return { + ...envelope, + summary: + [...summary].length <= 640 + ? summary + : `${[...summary].slice(0, 639).join('')}…`, + suspect: Object.fromEntries( + [...suspect].map(([id, one]) => [id, suspectBanner(one)]) + ), + }; +} + +/** Suspect records `brief` lists, at most. */ +const BRIEF_SUSPECT_LIMIT = 10; + +/** + * The lines `brief` prints above the head compaction for an Effort's + * suspect records: one line each with why, and how many of them the head + * derives from, since its summary repeats what they say. + */ +function suspectNotices( + effortId: string, + suspects: readonly Suspect[], + byId: ReadonlyMap, + head: ReadRecord | undefined +): string[] { + if (!suspects.length) return []; + const derived = new Set(head?.relations.derives_from ?? []); + const under = suspects.filter((one) => derived.has(one.id)).length; + const lines = [ + `> **SUSPECT** — ${suspects.length} live ${ + suspects.length === 1 ? 'record' : 'records' + } in this Effort may be out of date: the code ${ + suspects.length === 1 ? 'it rests' : 'they rest' + } on changed after ${ + suspects.length === 1 ? 'it was' : 'they were' + } recorded, or ${ + suspects.length === 1 ? 'it rests' : 'they rest' + } on dead evidence.${ + under + ? ` The head compaction derives from ${under} of them, so what it says about them may be out of date too.` + : '' + } Read the code first; where they disagree, the code is the current behaviour.`, + ...suspects + .slice(0, BRIEF_SUSPECT_LIMIT) + .map( + (one) => + `> - ${one.id} "${String( + byId.get(one.id)?.frontmatter.title ?? '' + )}": ${one.reasons.map((reason) => reason.text).join('; ')}` + ), + ]; + if (suspects.length > BRIEF_SUSPECT_LIMIT) + lines.push( + `> - … ${ + suspects.length - BRIEF_SUSPECT_LIMIT + } more: \`proof records ${effortId} --suspect\`` + ); + return lines; } /** A `find` envelope: the usual read fields plus the hit cards inline. */ @@ -2032,12 +2225,26 @@ export async function effortFind( : () => false, }) : new Map(); + // Live Decisions and Constraints whose code moved on, or that rest on + // dead evidence. Records only on a ref are not checked. + const suspect = await findSuspects( + found.hits + .map((hit) => hit.record) + .filter( + (record) => + !overlay || !(overlay as Overlay).tag(record.id).startsWith('only on') + ), + index, + history ? new CodeHistory(history.top, history.proofRoot) : undefined + ); const withNotes = (hits: readonly FindHit[]): FindHit[] => - hits.map((hit) => - staleLines.get(hit.record.id)?.length + hits.map((hit) => { + const one = suspect.get(hit.record.id); + const noted = staleLines.get(hit.record.id)?.length ? { ...hit, codeNotes: staleLines.get(hit.record.id) } - : hit - ); + : hit; + return one ? { ...noted, suspect: suspectBanner(one) } : noted; + }); const result: FindResult = codePath || overlay ? { @@ -2083,6 +2290,7 @@ export async function effortFind( { sections: [{ heading: 'Find hits', lines: cards.split('\n') }], codeNotes: [], + suspect, // A ranked search that shows fewer records than matched is not complete. capReasons: result.matched > result.hits.length ? ['find_limit'] : [], } @@ -2092,8 +2300,16 @@ export async function effortFind( } searched); matched: ${result.found.join(' ') || 'none'}; missing: ${ result.missing.join(' ') || 'none' }${conflicts ? `; refs ${sides.length}; conflicts ${conflicts.total}` : ''}${ - note ? `; ${note}` : '' - }`; + suspect.size + ? `; SUSPECT ${[...suspect.keys()].join( + ', ' + )}: each rests on code that changed after it was recorded, or on dead evidence (see the card)` + : '' + }${ + suspect.partial + ? '; the suspect check stopped at its limit, so a card without a warning may still be suspect (`proof get ` checks one record)' + : '' + }${note ? `; ${note}` : ''}`; return { ...envelope, summary: @@ -2291,7 +2507,23 @@ export async function effortState( 'PROOF_INVALID_ARGUMENT', `Effort ${effortId} does not exist` ); - const { markdown, ...state } = buildEffortState(index, effortId); + const suspect = await suspectsOn( + projection, + options, + [...index.records.values()].filter( + (record) => record.frontmatter.effort === effortId + ) + ); + const { markdown, ...state } = buildEffortState( + index, + effortId, + new Map( + [...suspect].map(([id, one]) => [ + id, + one.reasons.map((reason) => reason.text), + ]) + ) + ); const envelope = await render( options, { type: 'effortState', effort_id: effortId }, @@ -2301,6 +2533,8 @@ export async function effortState( { sections: [{ heading: 'Effort state', lines: markdown.split('\n') }], capReasons: state.capped ? ['state_limit'] : [], + // Only for the cap: state lists its suspect records in its markdown. + suspect: suspect.partial ? suspect : new SuspectMap(), } ); return { diff --git a/packages/flatbread/src/proof/state.ts b/packages/flatbread/src/proof/state.ts index 153871fe..7c0abb86 100644 --- a/packages/flatbread/src/proof/state.ts +++ b/packages/flatbread/src/proof/state.ts @@ -41,6 +41,8 @@ export interface StateLimits { readonly compactionTargets: number; /** Records the head compaction no longer matches, listed by id. */ readonly compactionChanges: number; + /** Suspect Decisions and Constraints, listed with why. */ + readonly suspect: number; } export const STATE_LIMITS: StateLimits = { @@ -57,6 +59,7 @@ export const STATE_LIMITS: StateLimits = { retractions: 10, compactionTargets: 10, compactionChanges: 10, + suspect: 10, }; /** * Characters the Markdown aims for: under 4,000 tokens at 4 characters a @@ -251,14 +254,20 @@ function changeRef(change: CompactionChange): string { */ export function buildEffortState( index: FindIndex, - effortId: string + effortId: string, + /** + * Suspect records and why, from git history at HEAD: live Decisions and + * Constraints whose code changed after they were recorded, or that rest + * on dead evidence. Listed after the counts when there are any. + */ + suspect: ReadonlyMap = new Map() ): EffortState { let limits = STATE_LIMITS; - let state = stateWithin(index, effortId, limits); + let state = stateWithin(index, effortId, limits, suspect); for (const step of SHRINK_STEPS) { if (state.markdown.length <= STATE_CHARS) break; limits = { ...limits, ...step }; - state = stateWithin(index, effortId, limits); + state = stateWithin(index, effortId, limits, suspect); } return state; } @@ -266,7 +275,8 @@ export function buildEffortState( function stateWithin( index: FindIndex, effortId: string, - limits: StateLimits + limits: StateLimits, + suspect: ReadonlyMap ): EffortState { const effort = index.records.get(effortId); const all = [...index.records.values()] @@ -484,6 +494,35 @@ function stateWithin( shown: countLines.length, }); + // Suspect records: live Decisions and Constraints whose code moved on + // after they were recorded, or that rest on dead evidence. Only when + // there are any. + const suspects = records([...suspect.keys()]); + if (suspects.length) + section( + 'Suspect records', + [ + `**SUSPECT**: ${count( + suspects.length, + 'live record' + )} may be out of date: the code ${ + suspects.length === 1 ? 'it rests' : 'they rest' + } on changed after ${ + suspects.length === 1 ? 'it was' : 'they were' + } recorded, or ${ + suspects.length === 1 ? 'it rests' : 'they rest' + } on dead evidence. Read the code first; where they disagree, the code is the current behaviour.`, + ], + suspects.map((record) => ({ + ids: [record.id], + lines: [ + `- ${ref(record.id)}: ${(suspect.get(record.id) ?? []).join('; ')}`, + ], + })), + limits.suspect, + `suspect records; \`proof records ${effortId} --suspect\`` + ); + // Open Issues, blockers first, each with what it gates and comes from. const openIssues = all .filter( diff --git a/packages/flatbread/src/proof/suspect.ts b/packages/flatbread/src/proof/suspect.ts new file mode 100644 index 00000000..07ef1897 --- /dev/null +++ b/packages/flatbread/src/proof/suspect.ts @@ -0,0 +1,588 @@ +import { isLiveStanding, type ReadRecord } from '@flatbread/proof'; +import { + anchorText, + codeAnchors, + isCodeCitation, + type CodeAnchor, +} from './anchors.js'; +import { codeReferences, standingOf, type FindIndex } from './find.js'; +import { + CodeHistory, + cutSubject, + fateText, + gitWord, + symbolText, + symbolWord, + type AnchorStatus, + type CommitAt, + type LineChange, +} from './history.js'; +import { restsOnDead } from './state.js'; + +/** + * Whether a live Decision or Constraint should be read as suspect: the code + * it rests on moved or changed after it was recorded, or it rests on a + * record that is no longer live. It is suspect when any of these holds: + * + * 1. A code Citation tied to it is stale at HEAD (its path is gone, or its + * symbol is gone or renamed), by the history checks `--path` and the + * code notes use. Tied means it cites the Citation, the Citation cites, + * derives from, or is evidence for it, the Citation is within + * CHAIN_HOPS `derives_from`, `evidence`, or `cites` links of it, or its + * text names the Citation's symbol as code and it, or a record within + * those links, names the Citation's file. + * 2. A code file its title or body names was deleted (history says so), or + * has at least `churnThreshold` of the lines it held at the commit that + * added the record rewritten or removed at HEAD (a tree diff between + * the two, so work merged from a branch counts too). Lines only added, + * a file that did not exist yet, a file that only moved, and docs + * (`.md`, `.txt`, …) never count. A bare name (`legacy.ts`) that HEAD + * lacks counts when history deleted exactly one file of that name. + * 3. It rests on dead evidence: it derives from, cites, or gives as + * evidence a superseded, invalidated, rejected, or retracted record, + * other than one superseded by a live revision of itself (same kind and + * title). + * + * Every git read goes through one `CodeHistory`, so it is cached and + * capped like the code notes; a check the budget cannot pay for says + * nothing, and the result is then marked `partial`. + */ + +/** Kinds a suspect check applies to. */ +export const SUSPECT_KINDS: readonly string[] = ['decision', 'constraint']; +/** Links followed from a record to the code Citations its evidence holds. */ +const CHAIN_HOPS = 2; +const CHAIN_RELATIONS = ['derives_from', 'evidence', 'cites'] as const; +/** Inbound links from a code Citation that tie it to the record. */ +const INBOUND_LABELS = ['cited_by', 'derived_by', 'evidence_for']; +/** Anchors one check reads at HEAD, at most; linked ones first. */ +const ANCHORS_CHECKED = 24; +/** Anchors read per Citation, as code notes read them. */ +const ANCHORS_PER_CITATION = 2; +/** Files one record's text names that a check looks at, at most. */ +const PATHS_PER_RECORD = 6; +/** Named paths HEAD lacks that one check asks history about, at most. */ +const GONE_CHECKED = 6; +/** Record commits one check diffs against HEAD, at most. */ +const DIFF_BASES = 16; +/** Revisions followed from a superseded record to a live one, at most. */ +const REVISION_HOPS = 8; +/** Changed lines below which a named file never makes a record suspect. */ +export const CHURN_MIN_LINES = 10; +/** Share of a file's lines at the record that must have changed. */ +export const CHURN_SHARE = 0.25; +/** Changed lines that always count, however long the file. */ +export const CHURN_CAP_LINES = 100; +const SUBJECTS_SHOWN = 3; +const DEAD_SHOWN = 3; +/** Named files that are prose, not code, which rule 2 leaves out. */ +const DOC_FILE = /\.(?:md|mdx|markdown|txt|rst|adoc)$/i; +/** + * A file name in prose, such as `package.json` or `vite.config.ts`, so its + * dotted parts are not read as `.json` or `.config`. A name followed by + * `(` is a call, not a file. + */ +const FILE_NAME = + /(? { + partial = false; +} + +/** Whether a check behind `suspects` stopped short. */ +export function suspectPartial( + suspects: ReadonlyMap | undefined +): boolean { + return suspects instanceof SuspectMap && suspects.partial; +} + +function text(value: unknown): string { + return typeof value === 'string' ? value : ''; +} + +/** A live Decision or Constraint. */ +export function suspectable(record: ReadRecord): boolean { + return ( + SUSPECT_KINDS.includes(record.kind) && + record.frontmatter.retracted !== true && + isLiveStanding(record.kind, standingOf(record)) + ); +} + +/** + * Whether `body` names `word` as code: as a whole word when it could only + * be an identifier (camelCase, snake_case, a digit, or ALL_CAPS), and + * otherwise only as `word(`, `.word`, `#word`, or in backticks, so a symbol + * that is also an English word is not found in prose. The dotted parts of + * a file name (`package.json`, `vite.config.ts`) are not `.word`. + */ +export function namesSymbol(body: string, word: string): boolean { + if (word.length < 3) return false; + const codeLike = + /[a-z][A-Z]/.test(word) || + /[A-Za-z0-9]_[A-Za-z0-9]/.test(word) || + /[A-Za-z]\d/.test(word) || + /^[A-Z][A-Z0-9_]{3,}$/.test(word); + const escaped = word.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + if (codeLike) return new RegExp(gitWord(word)).test(body); + return new RegExp( + `(?:(?(); + const add = (one: ReadRecord | undefined) => { + if (one && isCodeCitation(one) && one.frontmatter.retracted !== true) + found.set(one.id, one); + }; + const inbound = index.inbound.get(record.id); + for (const label of INBOUND_LABELS) + for (const id of inbound?.get(label) ?? []) add(index.records.get(id)); + for (const one of linked) add(one); + return [...found.values()]; +} + +/** Files a text names, as written. */ +function namedFiles(value: string): string[] { + return codeReferences(value) + .filter( + (ref) => + /\.[\w-]+$/.test(ref) && + !ref.endsWith('()') && + !ref.startsWith('#') && + !ref.includes('..') && + !/^[a-z]+:/i.test(ref) + ) + .map((ref) => ref.replace(/^\.\//, '')); +} + +/** Whether one of `refs` stands for `path`: it, or the end of it. */ +function namesFile(refs: readonly string[], path: string): boolean { + return refs.some((ref) => path === ref || path.endsWith(`/${ref}`)); +} + +/** + * The files at HEAD a named path may stand for: itself, or else every file + * whose path ends with it (`keys/keymap.ts` for `src/keys/keymap.ts`). + */ +function filesFor( + named: string, + files: readonly string[], + set: ReadonlySet +): string[] { + if (set.has(named)) return [named]; + return files.filter((file) => file.endsWith(`/${named}`)); +} + +/** + * What a stale anchor's history says, as `path: …`; undefined for an + * anchor whose path and symbol hold at HEAD. + */ +function staleClause(status: AnchorStatus): string | undefined { + const { anchor } = status; + const parts: string[] = []; + let file = anchor.path; + if (status.path === 'unknown') parts.push('not at HEAD or in its history'); + else if (status.path) { + parts.push(`file ${fateText(status.path)}`); + file = status.path.now ?? file; + } + if (status.symbol) parts.push(symbolText(status.symbol, file)); + return parts.length ? `${anchor.path}: ${parts.join('; ')}` : undefined; +} + +/** + * Whether a superseded record was replaced by a live revision of itself: a + * chain of `superseded_by` links through records of its kind and title + * that ends at a live one. + */ +function revised( + record: ReadRecord, + get: (id: string) => ReadRecord | undefined +): boolean { + const title = text(record.frontmatter.title); + let current = record; + for (let hop = 0; hop < REVISION_HOPS; hop++) { + if (standingOf(current) !== 'superseded') return false; + const next = (current.relations.superseded_by ?? []) + .map(get) + .find( + (one): one is ReadRecord => + one !== undefined && + one.kind === record.kind && + text(one.frontmatter.title) === title + ); + if (!next) return false; + if ( + next.frontmatter.retracted !== true && + isLiveStanding(next.kind, standingOf(next)) + ) + return true; + current = next; + } + return false; +} + +/** + * The suspect records among `candidates`: live Decisions and Constraints + * whose code moved or changed after they were recorded, or that rest on + * dead evidence, with why. `index` gives every record and its inbound + * links; without `history`, only dead evidence is checked. + */ +export async function findSuspects( + candidates: readonly ReadRecord[], + index: FindIndex, + history: CodeHistory | undefined +): Promise { + const found = new SuspectMap(); + const targets = [ + ...new Map( + candidates.filter(suspectable).map((record) => [record.id, record]) + ).values(), + ]; + const reasons = new Map( + targets.map((record) => [record.id, []]) + ); + const push = (id: string, reason: SuspectReason) => { + const list = reasons.get(id) as SuspectReason[]; + if (!list.some((one) => one.text === reason.text)) list.push(reason); + }; + if (targets.length && history && (await history.head())) { + // 1. Stale code Citations: linked ones, then ones the text names in a + // file it, or its chain, names too. + const tied = new Map(); + const bySymbol = new Map(); + for (const citation of index.records.values()) { + if (!isCodeCitation(citation) || citation.frontmatter.retracted === true) + continue; + for (const anchor of anchorsOf(citation)) + if (anchor.symbol) { + const word = symbolWord(anchor.symbol); + if (!bySymbol.has(word)) bySymbol.set(word, []); + (bySymbol.get(word) as CodeAnchor[]).push(anchor); + } + } + const linkedAnchors: CodeAnchor[] = []; + const namedAnchors: CodeAnchor[] = []; + for (const record of targets) { + const links = chain(record, index); + const linked = linkedCitations(record, index, links).flatMap(anchorsOf); + const body = words(record); + const refs = [record, ...links].flatMap((one) => namedFiles(words(one))); + const named: CodeAnchor[] = []; + if (refs.length) + for (const [word, anchors] of bySymbol) + if (namesSymbol(body, word)) + for (const anchor of anchors) + if (namesFile(refs, anchor.path)) named.push(anchor); + tied.set(record.id, [...linked, ...named]); + linkedAnchors.push(...linked); + namedAnchors.push(...named); + } + const anchors = [ + ...new Map( + [...linkedAnchors, ...namedAnchors].map((anchor) => [ + anchorText(anchor), + anchor, + ]) + ).values(), + ]; + if (anchors.length > ANCHORS_CHECKED) found.partial = true; + const statuses = await history.anchorStatuses(anchors, ANCHORS_CHECKED); + for (const record of targets) + for (const anchor of tied.get(record.id) ?? []) { + const status = statuses.get(anchorText(anchor)); + const clause = status && staleClause(status); + if (clause) push(record.id, { code: true, text: clause }); + } + + // 2. Code files the text names: deleted, or rewritten since. + const files = (await history.headFiles()) ?? []; + const fileSet = new Set(files); + const proof = history.proofRoot ? `${history.proofRoot}/` : undefined; + const inProof = (file: string) => !!proof && file.startsWith(proof); + const resolved = new Map(); + const gone = new Map>(); + const bare = new Map>(); + const addTo = (map: Map>, key: string, id: string) => { + if (!map.has(key)) map.set(key, new Set()); + (map.get(key) as Set).add(id); + }; + for (const record of targets) { + const list: string[] = []; + for (const ref of namedFiles(words(record)) + .filter((ref) => !DOC_FILE.test(ref)) + .slice(0, PATHS_PER_RECORD)) { + const matches = filesFor(ref, files, fileSet); + if (matches.length === 1) { + if (!inProof(matches[0]) && !list.includes(matches[0])) + list.push(matches[0]); + } else if (!matches.length) + addTo(ref.includes('/') ? gone : bare, ref, record.id); + } + resolved.set(record.id, list); + } + // A bare name HEAD lacks counts when history deleted one such file. + if (bare.size > GONE_CHECKED) found.partial = true; + const deleted = await history.deletedPaths( + [...bare.keys()].slice(0, GONE_CHECKED) + ); + for (const [name, paths] of deleted) { + const left = [...new Set(paths)].filter( + (path) => !fileSet.has(path) && !inProof(path) + ); + if (left.length === 1) + for (const id of bare.get(name) ?? []) addTo(gone, left[0], id); + } + if (gone.size > GONE_CHECKED) found.partial = true; + const fates = await history.pathFates([...gone.keys()], GONE_CHECKED); + /** Named files that only moved, by record and HEAD path: the old path. */ + const moved = new Map(); + for (const [path, fate] of fates) + for (const id of gone.get(path) ?? []) + if (fate.now && !fate.deleted) { + if (inProof(fate.now)) continue; + moved.set(`${id}\0${fate.now}`, path); + const list = resolved.get(id) ?? []; + if (!list.includes(fate.now)) list.push(fate.now); + resolved.set(id, list); + } else + push(id, { + code: true, + text: `${path} is gone at HEAD: ${fateText(fate)}`, + }); + // Lines each file had when the record was committed that HEAD has + // rewritten or removed: one tree diff per record commit, so merged + // branch work counts and lines only added never do. + const added = await history.recordCommits(); + const byBase = new Map(); + for (const record of targets) { + const commit = added.get(record.id); + if (!commit || !resolved.get(record.id)?.length) continue; + if (!byBase.has(commit.sha)) byBase.set(commit.sha, []); + (byBase.get(commit.sha) as ReadRecord[]).push(record); + } + if (byBase.size > DIFF_BASES) found.partial = true; + const allFiles = new Set(); + const flagged: { + id: string; + file: string; + from?: string; + base: CommitAt; + removed: number; + then: number; + }[] = []; + const diffs = new Map | undefined>(); + for (const [sha, records] of [...byBase].slice(0, DIFF_BASES)) { + const paths = new Set(); + for (const record of records) + for (const file of resolved.get(record.id) ?? []) { + paths.add(file); + allFiles.add(file); + const from = moved.get(`${record.id}\0${file}`); + if (from) paths.add(from); + } + const diff = await history.diffStat(sha, [...paths]); + if (!diff) found.partial = true; + diffs.set(sha, diff); + } + const lines = allFiles.size + ? await history.lineCounts([...allFiles]) + : new Map(); + for (const [sha, records] of [...byBase].slice(0, DIFF_BASES)) + for (const record of records) + for (const file of resolved.get(record.id) ?? []) { + const change = diffs.get(sha)?.get(file); + const size = lines.get(file); + if (!change || size === undefined) continue; + // Lines the file had at the record; none means it is new since. + const then = size - change.added + change.deleted; + if (then <= 0 || change.deleted < churnThreshold(then)) continue; + flagged.push({ + id: record.id, + file, + from: moved.get(`${record.id}\0${file}`), + base: added.get(record.id) as CommitAt, + removed: change.deleted, + then, + }); + } + // What decides a flag is read; the subjects below are only shown. + if (history.capped || history.timedOut) found.partial = true; + const commits = flagged.length + ? await history.recentCommits( + [ + ...new Set( + flagged.flatMap((one) => [one.file, one.from ?? []].flat()) + ), + ], + Math.min(...flagged.map((one) => one.base.time)) + ) + : []; + for (const one of flagged) { + const after = (commits ?? []).filter( + (commit) => + commit.sha !== one.base.sha && + commit.time > one.base.time && + (commit.files.includes(one.file) || + (!!one.from && commit.files.includes(one.from))) + ); + const listed = after + .slice(0, SUBJECTS_SHOWN) + .map( + (commit) => + `${commit.sha.slice(0, 7)} "${cutSubject(commit.subject, 50)}"` + ); + const more = after.length - listed.length; + push(one.id, { + code: true, + text: `${one.file}${one.from ? ` (moved from ${one.from})` : ''}: ${ + one.removed + } of the ${ + one.then + } lines it had when this was recorded are rewritten or removed (git diff ${one.base.sha.slice( + 0, + 7 + )} HEAD)${ + listed.length + ? `; since then: ${listed.join(', ')}${ + more > 0 ? `, +${more} more` : '' + }` + : '' + }`, + }); + } + } + + // 3. Dead evidence, less records a live revision of themselves replaced. + const get = (id: string) => index.records.get(id); + for (const record of targets) { + const dead = restsOnDead(record, get).filter((id) => { + const one = get(id); + return !one || !revised(one, get); + }); + if (!dead.length) continue; + const shown = dead + .slice(0, DEAD_SHOWN) + .map((id) => `${id} (${standingOf(get(id) as ReadRecord)})`); + push(record.id, { + code: false, + text: `it rests on ${shown.join(', ')}${ + dead.length > DEAD_SHOWN ? `, +${dead.length - DEAD_SHOWN} more` : '' + }`, + }); + } + + for (const record of targets) { + const list = reasons.get(record.id) ?? []; + if (list.length) + found.set(record.id, { + id: record.id, + kind: record.kind, + reasons: [ + ...list.filter((reason) => reason.code), + ...list.filter((reason) => !reason.code), + ], + }); + } + return found; +} + +function kindWord(kind: string): string { + return kind === 'constraint' ? 'Constraint' : 'Decision'; +} + +/** + * The warning a read prints before a suspect record's body: what changed, + * and that the code wins. + */ +export function suspectBanner(suspect: Suspect): string { + const code = suspect.reasons.filter((reason) => reason.code); + const dead = suspect.reasons.filter((reason) => !reason.code); + const kind = kindWord(suspect.kind); + if (!code.length) + return `SUSPECT — this ${kind} rests on records that are no longer live: ${dead + .map((reason) => reason.text.replace(/^it rests on /, '')) + .join('; ')}. Check what replaced them before relying on it.`; + return `SUSPECT — the code this ${kind} rests on changed after it was recorded: ${code + .map((reason) => reason.text) + .join('; ')}${ + dead.length ? `; also, ${dead.map((reason) => reason.text).join('; ')}` : '' + }. Read the code first; where they disagree, the code is the current behaviour.`; +} + +/** The short marker a listing prints after a suspect record's id. */ +export function suspectMarker(suspect: Suspect): string { + const code = suspect.reasons.some((reason) => reason.code); + const dead = suspect.reasons.some((reason) => !reason.code); + return `suspect: ${[code ? 'code changed' : '', dead ? 'dead evidence' : ''] + .filter(Boolean) + .join(', ')}`; +} diff --git a/packages/proof/skills/proof/SKILL.md b/packages/proof/skills/proof/SKILL.md index 866094fd..636ba640 100644 --- a/packages/proof/skills/proof/SKILL.md +++ b/packages/proof/skills/proof/SKILL.md @@ -212,6 +212,20 @@ Pick the read by what you need, in this order: commits' subjects. Read those commits before you rely on what the record says the code does. An anchor that still holds, and code on disk that is not committed yet, add nothing. + + **SUSPECT means the code wins.** A live Decision or Constraint whose code + moved on after it was recorded prints `SUSPECT — the code this Decision rests on changed after it was recorded: . Read the code first; where they disagree, the code is the current behaviour.` before its body + in `get`, on its find card, above the head compaction in `brief`, in + `state`, and as `(suspect: …)` on listings. The reasons: a code Citation + it cites or names went stale, a code file it names was deleted, or a + quarter of the lines a code file it names had then is rewritten or + removed since. One that rests on a dead record says so instead. A + listing with `suspect_limit` in `cap_reasons` checked only part of its + records; `proof get` checks one. Do not follow a suspect record past the code: read the + files and commits it names, build on what the code does now, and say in + your answer which recorded rule the code no longer follows. + `proof records --suspect` lists them. + 5. **Before building on a Decision: `proof find --refs all`.** It overlays the Proof records other branches change, without a checkout, tags each card `main`, `only on `, or `differs on : vs ` @@ -227,7 +241,8 @@ Pick the read by what you need, in this order: `superseded`, `retracted`, any standing a card prints) lists records by where they stand, links included. `--rests-on invalidated` lists the live records that derive from, cite, or give as evidence a record in that - standing. + standing. `--suspect` lists the live Decisions and Constraints that read + as SUSPECT. Browse digests (`list`, `records`, `relations`, `blocking-decisions`) excerpt each body at 600 chars / 12 lines (`[…truncated]`). **`proof get` digests diff --git a/packages/proof/skills/proof/reference.md b/packages/proof/skills/proof/reference.md index e568f7ba..4b857bd1 100644 --- a/packages/proof/skills/proof/reference.md +++ b/packages/proof/skills/proof/reference.md @@ -241,8 +241,11 @@ order: 3. If `cap_reasons` is not empty, narrow the query or fail closed. It can hold several sorted, duplicate-free values from `primary_records`, `displayed_edges`, `bytes`, `find_limit` (`proof find` matched more - records than it shows), and `state_limit` (`proof state` cut at least one - list short; each cut list says how many it left out). + records than it shows), `state_limit` (`proof state` cut at least one + list short; each cut list says how many it left out), and + `suspect_limit` (the suspect check stopped at a cap or its git budget, + so a record it did not flag may still be suspect; `proof get ` + checks one record on its own). 4. A hard cap alone leaves `page.has_more: false` and `next_cursor: null`. Paging and hard caps can occur together, but caps never create a cursor. @@ -254,7 +257,7 @@ digest or `summary` as a data feed. ```bash flatbread proof get [--resolve exact|head] [consistency flags] flatbread proof list [--status active,paused,...] [--limit n] [--cursor c] [consistency flags] -flatbread proof records [--kinds k1,k2] [--state s1,s2] [--status s1,s2] [--standing s1,s2] [--rests-on s1,s2] [--kind k1,k2] [--since iso] [--until iso] [--retracted exclude|include|only] [--limit n] [--cursor c] [consistency flags] +flatbread proof records [--kinds k1,k2] [--state s1,s2] [--status s1,s2] [--standing s1,s2] [--rests-on s1,s2] [--suspect] [--kind k1,k2] [--since iso] [--until iso] [--retracted exclude|include|only] [--limit n] [--cursor c] [consistency flags] flatbread proof relations --relations r1,r2 [--limit n] [--cursor c] [consistency flags] flatbread proof blocking-decisions [consistency flags] flatbread proof brief [consistency flags] @@ -298,6 +301,9 @@ flatbread proof serve [--socket .flatbread/proof/serve.sock] give as evidence a record in one of those standings, in any Effort. The record that overturned it does not count. This is the rule behind the count in `proof state`'s Dead evidence. +- `--suspect` keeps the live Decisions and Constraints that are suspect + (see [Suspect Decisions and Constraints](#suspect-decisions-and-constraints)). + The envelope's `suspect` maps each id on the page to its warning. - `--relations` values: `derives_from`, `supersedes`, `superseded_by`, `invalidates`, `invalidated_by`, `rejected_by`, `mitigated_by`, `resolved_by`, `evidence`, `cites` (one hop, explicit only). Missing targets @@ -539,6 +545,74 @@ the last 500 commits. Path-limited walks have no cap on length, as for any `git log -- `: `--follow` on the target file, trailer reads, and the deletion lookup for a path HEAD lacks. +### Suspect Decisions and Constraints + +A live Decision or Constraint is **suspect** when the code it rests on +moved or changed after it was recorded, or when it rests on dead evidence. +Every read that shows one prints, before its body (under its heading in a +digest, under its title on a find card): + +```text +SUSPECT — the code this Decision rests on changed after it was recorded: src/keys/keymap.ts: #registerBinding renamed to legacyAction in bc1c5b5 "Split key bindings by scope"; src/keys/keymap.ts: 20 of the 57 lines it had when this was recorded are rewritten or removed (git diff 1722a2a HEAD); since then: bc1c5b5 "Split key bindings by scope", 07b910e "…", 54dc6ec "…", +1 more. Read the code first; where they disagree, the code is the current behaviour. +SUSPECT — this Decision rests on records that are no longer live: fnd-… (invalidated). Check what replaced them before relying on it. +``` + +**The code wins.** The record says what was true when it was written; a +suspect one may describe code that has since been replaced. Read the files +and commits the warning names before following it, build on what the code +does now, and say which recorded rule the code no longer follows. Only a +person, or a new Decision that supersedes it, changes the record. + +It is suspect when any of these holds: + +1. **A code Citation tied to it is stale** at HEAD: its path is gone, or + its symbol is gone or renamed, by the checks in + [Code anchors at HEAD](#code-anchors-at-head) (a `changed since` anchor + alone does not count). Tied means it cites the Citation, the Citation + cites it, derives from it, or is evidence for it, the Citation is + within 2 `derives_from`, `evidence`, or `cites` links of it, or its + title or body names the Citation's symbol as code **and** it, or a + record within those 2 links, names the Citation's file. A symbol is + named as a whole word when the name can only be an identifier + (`registerBinding`, `DEFAULT_KEYMAP`, `parse2`), otherwise only as + `name(`, `.name`, `#name`, or `` `name` ``, so a symbol that is also an + English word is not found in prose, and the dotted parts of a file name + (`package.json`, `vite.config.ts`) are not `.name`. The same name in + another file is another symbol. +2. **A code file its title or body names was deleted or largely + rewritten.** A named file is the file at HEAD with that path, or the one + file whose path ends with it (`keys/keymap.ts`, `transform.ts`); docs + (`.md`, `.mdx`, `.txt`, `.rst`, `.adoc`) are left out. Deleted: HEAD and + the working tree lack it and history has the commit that deleted it; a + bare name (`legacy.ts`) counts when history deleted exactly one file of + that name. A file that only moved is followed to its new path. + Rewritten: at least max(10, min(100, a quarter of the lines the file had + then)) of the lines it had at the commit that added the record's file + are rewritten or removed at HEAD, by one `git diff --numstat` between + that commit and HEAD, so work merged from a branch counts whenever it + was committed. Lines only added (implementing the record, appending to + the file) never count, nor does a file that did not exist yet, nor a + record whose file is not committed. The warning gives the lines, the + diff to read, and up to 3 subjects of the newest commits since. +3. **It rests on dead evidence:** the `--rests-on` rule, as `proof state`'s + Dead evidence counts it, less a record superseded by a live revision of + itself (a `superseded_by` chain through records of the same kind and + title that ends at a live one). + +Where: `get` (and its summary, which leads with the warning), each digest +record (`records`, `relations`, `blocking-decisions`), find cards and the +find summary, `brief` (a block above the head compaction listing the +Effort's suspect records, and how many of them the head derives from), +`state` (a `Suspect records` section after the counts, only when there are +any), and the index line of every digest as `(suspect: code changed)` or +`(suspect: dead evidence)`. A check reads at most 24 anchors, asks +history about at most 6 gone paths, diffs at most 16 record commits +against HEAD, and has its own budget of 60 local git commands; reads that +walk history stop after 5 seconds. A check that hits any of these says +so: `cap_reasons` holds `suspect_limit`, `complete` is false, and the +summary says a record not flagged may still be suspect. `proof get ` +checks one record on its own. + ### `proof find --refs`: what other branches propose `--refs feat/a,origin/feat/b` or `--refs all` overlays records from other @@ -632,6 +706,9 @@ The Markdown has these sections, in order: Findings, and Constraints also by their own `kind` (`blocker`, `question`, `hard`, …), how many Decisions were ever reopened, and how many records are live or retracted. Exact numbers, never cut. +- **Suspect records:** shown only when the Effort has any. Each live + Decision or Constraint that is suspect, with why (see + [Suspect Decisions and Constraints](#suspect-decisions-and-constraints)). - **Open Issues and blockers:** each open Issue, blockers first, with the live Decisions that derive from it (`gates`), other live records built on it, and what it comes from. diff --git a/packages/proof/src/digest.ts b/packages/proof/src/digest.ts index 1269c59b..6617de9d 100644 --- a/packages/proof/src/digest.ts +++ b/packages/proof/src/digest.ts @@ -35,14 +35,17 @@ export interface ReadEdge { * Why a read is not complete. `find_limit` means `proof find` matched more * records than it shows; it pages no further, so narrow the query instead. * `state_limit` means `proof state` cut at least one list short; each cut - * list says how many entries it left out. + * list says how many entries it left out. `suspect_limit` means the check + * for suspect records stopped at a cap or its git budget, so a record it + * did not flag may still be suspect; `proof get` checks one record. */ export type ReadCapReason = | 'primary_records' | 'displayed_edges' | 'bytes' | 'find_limit' - | 'state_limit'; + | 'state_limit' + | 'suspect_limit'; /** Machine-readable state of an Effort's head compaction (`brief` only). */ export interface CompactionStatus { @@ -73,6 +76,17 @@ export interface ReadEnvelope { page: { returned: number; has_more: boolean; next_cursor: string | null }; hints: string[]; compaction?: CompactionStatus; + /** + * Records on the page a read flagged, by id, with the warning printed + * before each one's body. + */ + suspect?: Record; +} + +/** A warning printed before a record's body, and its short index marker. */ +export interface RecordFlag { + readonly banner: string; + readonly marker: string; } export interface DigestInput { @@ -98,6 +112,10 @@ export interface DigestInput { compaction?: CompactionStatus; /** Caps the caller applied before the digest, such as `find_limit`. */ capReasons?: readonly ReadCapReason[]; + /** Warnings for records, by id, printed under the heading before the body. */ + flags?: Readonly>; + /** Lines printed under the title, before the index. */ + notices?: readonly string[]; } const CAP_RECORDS = 25; @@ -246,15 +264,15 @@ function retractedBanner(record: ReadRecord): string { ].join(' '); } -function indexLine(record: ReadRecord): string { +function indexLine(record: ReadRecord, flag?: RecordFlag): string { return `- [\`${record.id}\`](#${record.id})${ isRetracted(record) ? ' (retracted)' : '' - }`; + }${flag ? ` (${flag.marker})` : ''}`; } function renderRecord( record: ReadRecord, - options: { bodyMode?: RecordBodyMode } = {} + options: { bodyMode?: RecordBodyMode; flag?: RecordFlag } = {} ): string { if (record.foreign_reference) { const state = record.frontmatter.retracted @@ -282,6 +300,7 @@ function renderRecord( return [ `### ${record.id}`, ...(isRetracted(record) ? [retractedBanner(record), ''] : []), + ...(options.flag ? [`> ${options.flag.banner}`, ''] : []), '```yaml', ...Object.keys(frontmatter) .sort() @@ -374,16 +393,23 @@ export async function renderDigest(input: DigestInput): Promise { const renderPrimary = ( record: ReadRecord, bodyMode: RecordBodyMode = primaryBodyMode - ) => renderRecord(record, { bodyMode }); + ) => renderRecord(record, { bodyMode, flag: input.flags?.[record.id] }); // Related / browse neighbors stay excerpted even when primary bodies are full. const renderRelated = (record: ReadRecord) => - renderRecord(record, { bodyMode: 'excerpt' }); + renderRecord(record, { + bodyMode: 'excerpt', + flag: input.flags?.[record.id], + }); + const index = (record: ReadRecord) => + indexLine(record, input.flags?.[record.id]); + const notices = input.notices?.length ? [...input.notices, ''] : []; let markdown = [ yamlHeader(input, completeness, hasMore), ...(input.anomaly ? [`> anomaly: ${input.anomaly}`, ''] : []), '# Proof read', + ...notices, '## Index', - ...visible.filter((record) => !record.foreign_reference).map(indexLine), + ...visible.filter((record) => !record.foreign_reference).map(index), '## Records', ...visible.map((record) => renderPrimary(record)), ...(input.relatedRecords?.length @@ -417,8 +443,9 @@ export async function renderDigest(input: DigestInput): Promise { yamlHeader(input, completeness, hasMore), ...(anomaly ? [`> anomaly: ${anomaly}`, ''] : []), '# Proof read', + ...notices, '## Index', - ...visible.filter((record) => !record.foreign_reference).map(indexLine), + ...visible.filter((record) => !record.foreign_reference).map(index), ...checkpoints, '## Records', ];