feat(skill): replace forge-review with review-fix skill

[LeTuR]

Intent

thurview could explain code and help a human review it, but it had no way to review a change and fix it on its own. The operator asked for one simple review-and-autofix skill built on thurview's code graph, to replace forge-review (#26), and for the graph to show what a diff alone can't.

What Changed

• skills/review-fix/ replaces skills/forge-review/. It pins a branch, a range or a PR/MR, asks the graph who depends on each changed symbol, and records findings with file:line, severity, a one-line why and graph evidence. It fixes what it is sure of, keeps only fixes that pass the repo's own tests and lint, commits them once and reports the rest. --post sends the unfixed findings through thurview forge submit. It never pushes or posts unless asked. 165 lines and one reference, down from 276 and two.

• thurview graph interfaces|impact|callers|tests-for|architecture take --base/--head, so they run on two commits with no published review. They pin the same way scaffold does (head defaults to HEAD, base to the trunk fork point); --review together with either is refused.

• impact.reach rows gain at (the call-site line) and tested (whether any test reaches that caller). Every reach row is already code the diff left alone, so a row reads as a finding:

  reach[1]{symbol,file,line,depth,via,at,tested}:
    checkout,src/cart.js,3,1,"src/price.js:discount",5,false
  

• README, CONTRIBUTING, .no-mistakes.yaml, the setup skill hint and the one sentence in skills/thurview/SKILL.md now point at review-fix.

Risk Assessment

• impact.reach has two new columns. Anything reading that table by position sees them; nothing in this repo does.
• thurview forge status|prior|submit|reply stay. Removing them would break the CLI beyond this change; the skill uses only status and submit.
• security-surfaces.md is dropped on purpose. The skill keeps a short definition of a security finding instead.
• The skill needs the new flags, so an install from main needs a released CLI. It tells the agent to run thurview update on unknown flag.

Testing

Each of these failed before its change and passes after:

• e2e: answers on two commits without a review. Failed on unknown flag --base, then on the data.yaml hint.
• graph.test: gives each caller the change left alone its call site and whether a test reaches it. Failed on missing at/tested.
• skill-frontmatter: ships one review skill. Failed on skills/review-fix/SKILL.md missing.

The skill itself ran on a seeded bug. In a throwaway repo, a branch changed discount(total, percent) to take a rate in [0, 1] and updated its own test, but not checkout in src/cart.js, which is outside the diff and untested. A fresh agent ran the skill with an empty request, without being told where the bug was:

flowchart LR
  A["graph impact --head HEAD"] -->|"reach: checkout src/cart.js:5, tested false"| B["finding, high: checkout passes 15, discount now takes 0.15"]
  C["graph interfaces"] -->|"discount (total, percent) to (total, rate)"| B
  B --> D["new test/cart.test.js fails with RangeError"]
  D --> E["fix: coupon.percent / 100"]
  E --> F["npm test 3/3, one fix commit"]

Loading

npm test went 1/1 before, 2/3 with the new test, 3/3 after the fix commit. The agent's notes on the skill (watch the new test fail first, the repo's commit convention wins, a place for problems already present at base) are folded into the second commit.

Not verified: --post against a live forge, since the demo repo has no remote (forge submit itself is covered by test/forge.test.ts), and the new reach fields on languages other than JavaScript and TypeScript.

Evidence: Manual seeded-bug reproduction transcript

Manual end-to-end reproduction of the seeded-bug scenario from the user intent
("discount(total, percent) -> discount(total, rate)" caller left untested),
run against a throwaway TypeScript repo (/tmp/rate-demo-ts, discarded after
this run) using the `thurview graph` CLI at commit d5fa83c
(feat/review-fix-skill), via `npx tsx src/main.ts`.

=== graph impact --head HEAD ===
base: d7290d9c
head: 9d64d2be
changed[1]: src/price.ts:discount (function, modified)
reach[1]{symbol,file,line,depth,via,at,tested}:
  checkout,src/cart.ts,2,1,"src/price.ts:discount",3,false
tests[1]: test/price.test.ts covers src/price.ts:discount
untested: []

=== graph interfaces --head HEAD ===
verdict: 1 changed.
interfaces[1]: src/price.ts:discount changed
  was: export function discount(total: number, percent: number): number
  now: export function discount(total: number, rate: number): number

=== graph callers discount --head HEAD ===
callers[2]{symbol,file,line,at}:
  checkout,src/cart.ts,2,3
  <module>,test/price.test.ts,1,2

Result: the graph correctly flags `checkout` (src/cart.ts) as a caller of the
changed `discount` signature that lies outside the diff and is not reached by
any test (tested: false), while the discount's own test in
test/price.test.ts is marked tested: true elsewhere (see test/graph.test.ts
"gives each caller the change left alone its call site and whether a test
reaches it"). This is exactly the class of bug (untested caller broken by a
signature change) the operator's brief describes as the target use case, and
demonstrates impact.reach's new `at`/`tested` fields ("useful insights")
working end-to-end, not just in unit tests.

=== --review together with --base/--head is refused ===
$ thurview graph impact --review someid --base main
error: "pass --review or --base/--head, not both"
code: VALIDATION_ERROR
exit=2

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

✅ **Test** - passed

✅ No issues found.

• pnpm test && bats scripts/ci/*.bats
• npx vitest run test/skill-frontmatter.test.ts test/graph.test.ts (16/16 passed, including the new tests)
• npx vitest run test/e2e.test.ts full file (34/34 passed)
• Manual repro with throwaway TS repo and thurview graph impact/interfaces/callers via npx tsx src/main.ts
• git diff d617e3c..d5fa83c -- skills/thurview/SKILL.md
• grep -rn forge-review across md/yaml/ts files

✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

https://claude.ai/code/session_01XEkc6BULdsADTNu1mABPVe

— LeTuR's agent