review

review patchsets using your default editor
Log | Files | Refs

readme.md (31038B)


      1 # review
      2 
      3 Reads a change the way several narrow readers would, and reports what they noticed.
      4 
      5 It is **advisory** unless asked to gate. By default it never fails a build and never
      6 blocks a commit — the deterministic checks a repository already runs are what should do
      7 that; `--exit-code` lets a hook make it one of them. It looks at what those checks
      8 cannot: whether a name says what the thing is, whether a fact is already stated
      9 somewhere else, whether a test can fail, whether a comment claims something nobody
     10 verified — and, before any of that is asked of a model, at everything about the change
     11 that a comparison can settle.
     12 
     13 ```
     14 review                          the staged change
     15 review HEAD^..HEAD              the last commit
     16 review --json                   for an agent rather than a person
     17 review --show                   what each job would be sent, and what the static checks found
     18 review --fresh                  the change again, asking rather than replaying the cache
     19 review --no-verify              skip the second reading that verifies the findings
     20 review --message-file "$1"      the staged change with the message a commit-msg hook is given
     21 review --exit-code              exit 1 when a must-fix finding stands, for a hook
     22 review --baseline last.json     which findings resolved, persist, or are new since that report
     23 review rules [job | rule]       what a finding was judged against
     24 review rules -dismissed         how often each rule is dismissed in this tree, and why
     25 review bench [-n 200] [-author x] [-rule id]   fire rate of every static check over recent commits
     26 review hook install             the commit-msg hook that makes the review a gate
     27 review agent                    what an agent's instructions should say about this tool
     28 ```
     29 
     30 ## Building
     31 
     32 The tool is written in Odin, on the `jm` collection (`~/Source/Personal/jm`), and
     33 reads each language through a sidecar built on that language's own parser. `just build`
     34 compiles the four binaries into `build/`; `just install` puts them beside each other on
     35 the path: `review`, `review-go` (Go's parser, from `sidecar/gofront`), `review-vet` (go
     36 vet's multichecker, from `sidecar/govet`) and `odin-review-extract` (Odin's parser, from
     37 `sidecar/odin`). `just test` type-checks and runs every package's tests and the Go
     38 sidecar's. The repository's own `ols.json` names the collection for the editor and for
     39 the `odin-check` analyser.
     40 
     41 ## The jobs
     42 
     43 Each is a separate call, run at once, given only the part of the change it needs. A job
     44 that reads less is cheaper and harder to distract into reporting something another job
     45 owns.
     46 
     47 | job | reads | asks |
     48 |---|---|---|
     49 | `duplication` | new declarations + existing ones that resemble them | does the repository already state this? |
     50 | `tests` | whole test functions the change adds, with the functions they call and the line each skips on | could this test fail? |
     51 | `namer` | new and renamed declarations, with their doc comments | does the name say what the thing is? |
     52 | `claims` | comment lines the change adds, in their blocks, with the code beneath each | does anything support this assertion? |
     53 | `hygiene` | the commit message, the file statistics, recent subjects | does the message match the commit? |
     54 
     55 The candidate list the `duplication` job judges is built by parsing the repository, not
     56 by searching it. A pattern over lines misses an indented constant inside a block, which
     57 is exactly where a duplicated fact tends to live. The list is ranked: names sharing more
     58 whole words first, then the same kind, then the same file — `cache` no longer pulls in
     59 every `Cached` field — and cut at twelve. The `tests` job is shown the bodies of the
     60 functions its tests call, found by name in the same index, so that whether a test passes
     61 on a stub is judged against the function rather than guessed. The `claims` job is shown
     62 two lines of code beneath each comment block, so that `stale` can be judged against what
     63 the change does; a comment whose every word is in the line below it is never a claim, and
     64 is kept out of the packet and measured by a static check instead.
     65 
     66 ## The static checks
     67 
     68 Twenty-seven checks run before anything is asked of a provider, and whether or not one can
     69 be asked. Seven measure the commit message; one measures the diff's shape; one measures
     70 the change against the repository's history; two hold the review to itself; sixteen
     71 measure the code the change adds. Then the repository's own compilers and analysers run,
     72 each with its strictest settings and asked for JSON, over the units the change touched.
     73 Static analysis is cheaper than a reading, so wherever a rule turned out to be a
     74 comparison rather than a judgement it was moved here — three naming rules and the
     75 plainest case of a test that cannot fail came out of the criteria that way.
     76 
     77 | analyser | runs | reports |
     78 |---|---|---|
     79 | `go-build` | `go build -json` on the packages the change touched | a compile error, must-fix wherever it lands |
     80 | `go-vet/<analyzer>` | `go vet -json`, with `-vettool=review-vet` when that is installed | vet's default set and `nilness` as must-fix; `shadow` and `unusedwrite` as consider; the `modernize` suite as note |
     81 | `staticcheck/<code>` | `staticcheck -f json` on the same packages | SA as must-fix, S and U as consider, style as note |
     82 | `odin-check` | `odin check <pkg> -vet -strict-style -json-errors -no-entry-point` per package touched | a type error must-fix wherever it lands; `/vet` (unused, shadowing) consider; `/style` note |
     83 | `tsc/<code>` | `tsc --noEmit --pretty false -p tsconfig.json` per project touched, under the project's own strictness | a type error, must-fix wherever it lands; working tree only, since it needs `node_modules` |
     84 | `ruff/<code>` | `ruff check --output-format json` on the changed files | pyflakes codes consider, style codes note |
     85 | `mypy/<code>` | `mypy --output json --ignore-missing-imports` on the changed files | a type error must-fix; working tree only |
     86 | `cargo/<code>` | `cargo clippy --message-format json`, or `cargo check` without clippy, per crate touched, into the repository's own target directory | an error must-fix wherever it lands, a warning consider |
     87 | `semgrep/<rule>` | `semgrep scan --json` under the repository's `.semgrep.yml`, or `--config p/default` without one, on the changed files it can parse | the rule's own severity: ERROR must-fix, WARNING consider, INFO note |
     88 
     89 Each analyser runs only when its binary is on the path and the tree has what it needs, and
     90 says on stderr why it did not. The rule for what belongs to the change is the same for
     91 all of them: a **fault** — the compiler cannot build the unit — is the change's wherever
     92 the error lands, because the tree does not compile until it is answered; an **opinion**
     93 is the change's only on a line the change added. A run that cannot finish inside five
     94 minutes is abandoned rather than holding the review. A range is analysed on its
     95 materialised tree; the two analysers that need the working tree's surroundings say so
     96 and skip.
     97 
     98 `review-vet` is the multichecker in `sidecar/govet`: vet's own analysers plus the ones
     99 from `golang.org/x/tools/go/analysis/passes` that vet leaves out — `nilness` (nil
    100 dereferences and impossible comparisons, from SSA), `atomicalign`, `deepequalerrors`,
    101 `httpmux`, `reflectvaluecompare`, `scannererr`, `sortslice`, `sqlrowserr`, `shadow`,
    102 `unusedwrite`, and the `modernize` suite. `just install` builds it and puts it on the
    103 path beside `review`.
    104 
    105 Without it, vet runs its default set. `fieldalignment` is left out on purpose: it is
    106 noise on any struct not on a hot path.
    107 
    108 | check | reads | asks |
    109 |---|---|---|
    110 | `message-low-entropy` | the Shannon entropy of the message, in bits per byte | is this one phrase repeated rather than a description? |
    111 | `message-boilerplate` | the share of its length the message keeps after zlib | is this one block of text pasted whole? |
    112 | `message-common-words` | each word's rarity in the repository's own subject history | is this made only of the words this repository says most, naming nothing in the change? |
    113 | `message-frustration` | the message's words against a short exclamation list | is this the author's reaction — oops, whoops, damn — rather than a description? |
    114 | `message-not-imperative` | the subject's first word after its package prefix | does the subject open as a command — not past tense, not a gerund, not the author? |
    115 | `message-no-body` | the diff's size in changed lines | does a change over 50 lines say anything below the subject at all? |
    116 | `message-long-body` | the body's word count | is the body over 150 words, listing what the diff already shows? |
    117 | `message-names-unknown` | each identifier-shaped word of the message — camel or snake case, a path, a call, anything in backticks — against the diff and every file in the tree | does the message name code that is nowhere in the repository? |
    118 | `formatting-mixed-in` | git's count of changed lines with and without whitespace | is half the diff reformatting, with at least ten lines of logic hidden in it? |
    119 | `history-coupled-file` | each changed file's co-changes over the last 1,000 commits, as a Jaccard | does history tie this file to a partner the change does not touch — the test beside the code, the header beside the source, the golden file beside the renderer? |
    120 | `suppression-added` | dismissal comments the change adds | is the change dismissing what the readers would have found, before the readers ran? |
    121 | `test-deleted` | test functions the change removes, in Go, TypeScript, JavaScript, Python and Rust | is the change deleting the tests that would have failed? |
    122 | `no-stutter` | each exported name's first word against its package | does `ico.IcoEntry` say ico twice? `time.Time` is the idiom and is spared |
    123 | `no-shadow` | each new name against Go's predeclared identifiers and standard library packages, the JavaScript runtime's globals, or Python's builtins | does the name take a word the language already uses? |
    124 | `abbreviation` | each word of a new name against a short list | is this `cfg`, `mgr`, `hdlr`, `svc`, `btn`, `cnt`: a word the reader expands rather than reads? |
    125 | `test-no-assertion` | each added or altered test's body | is there any call in it that could fail it — or does it pass whatever the code does, and fail only by crashing? |
    126 | `assertion-always-true` | each assertion in an added or altered test | does it assert a literal true, two literals, or a value against itself? |
    127 | `duplicate-body` | each new function's tokens against every function in the repository and the change | is this body already written — token for token (must-fix), or in shape with every name changed (consider)? |
    128 | `function-too-long` | each new function's line count | is it over 150 lines? 95% of measured functions fit in 109 |
    129 | `nesting-too-deep` | each new function's block depth | does it nest more than 5 deep? 99% of measured functions stay within 6 |
    130 | `comment-restates-code` | each added comment's words against the line below it | does the comment say only what the code says? |
    131 | `todo-without-reference` | each added TODO, FIXME, XXX or HACK | does it name an issue, a ticket, a link or a person? |
    132 | `commented-out-code` | runs of added comment lines | is this code kept as a comment — two statement-shaped lines, or one beyond doubt? |
    133 | `debug-leftover` | added lines against the debugging shapes of each language | is this a `debugger`, a `breakpoint()`, a `dbg!`, a `spew.Dump`, a `console.log`, a `DEBUG` print? |
    134 | `error-swallowed` | added lines against the dropping shapes of each language | is this `_ = err`, an empty `catch`, an `except` that passes? |
    135 | `new-symbol-unreferenced` | each new declaration's name as a whole word over every text file in the tree | does anything refer to it but its own line? A doc comment does not count |
    136 | `code-without-tests` | the added lines by file kind, and the tree's test files | does a change of 50 or more code lines touch no test, in a repository that keeps them? |
    137 
    138 The first two are calibrated against the 21,000 commit messages on this machine, and
    139 fire below every one of them: under 3.2 bits per byte, where ordinary messages measure
    140 3.7–4.9; under 20% of length, where none kept less than 26%. Entropy says nothing about
    141 a message under 40 bytes, and zlib nothing about one under 400 — a short subject is
    142 low-entropy whatever it says, and framing dominates the ratio below that.
    143 
    144 Dismiss an analyser's finding where it is wrong, by its full id:
    145 
    146 ```go
    147 //review:ignore staticcheck/S1002 the comparison states the contract
    148 ```
    149 
    150 The common-words check needs a history to measure against, so it says nothing in a
    151 repository younger than a hundred commits. Against 23,956 measured messages it fires on
    152 sixteen, every one of them a message such as `fix`, `fix ci`, `Fix fix.` — and on
    153 nothing else. A message is spared when any word is rarer than a fifth of the history,
    154 when it names anything the diff adds, removes or touches (`Update requests.ts`), when it
    155 carries a number (`Bump to 2.0.26`), or when it is a merge or squash subject, which is
    156 git's prose rather than the commit's.
    157 
    158 The formatting check is git's own count: the change's lines with `--numstat`, and again
    159 with `-w`. Sampled over 1,853 local commits, a whitespace share of one half with at
    160 least twenty whitespace-only lines and ten of logic fires on 0.7%, and the ones it fires
    161 on are the mixed kind.
    162 
    163 The coupling check is the one that is not about the message: it is counted from
    164 `git log` alone, so it works for every language and sees pairs the compiler cannot —
    165 a frontend component and the test beside it, a C source and its header, a renderer
    166 and its golden files. It is counted over the last thousand commits before the change,
    167 never over the change itself, and commits listing over a hundred files are left out:
    168 a sweep touching everything once says nothing about any pair. It asks about at most
    169 three pairs, and only those that still exist at the end of the change. Measured fire
    170 rate on sampled history: 3.6% of commits at J ≥ 0.7 with at least 5 shared commits —
    171 12% at J ≥ 0.5, which is why the threshold sits where it does.
    172 
    173 The two about gaming the review itself: `suppression-added` reads every dismissal
    174 comment the change adds — a suppression that the readers have not seen yet, nameable
    175 before they run — and reports it as must-fix; the report carries no file and so cannot
    176 be dismissed itself. The prose that documents the mechanism does not count, and neither
    177 does a comment in a file no reader reads. `test-deleted` reads the test functions the
    178 change removes and reports each file that lost one, must-fix; a test that was renamed
    179 is spared, judged name-word by name-word, and a deleted test that survived as a new
    180 test elsewhere in the change is spared the same way.
    181 
    182 The sixteen about the code read what the language frontends read — declarations with
    183 their bodies, tests, comments — and the diff's added lines where a shape is enough.
    184 Their thresholds were measured rather than guessed. Function length and depth were read
    185 off 8,255 functions in the repositories on this machine: the 95th percentile of length
    186 is 109 lines and the 99th is 257, so the cut is 150; the 99th percentile of depth is 6,
    187 so the cut is 5. Two bodies are compared only past forty tokens, and compared in shape —
    188 every identifier and literal replaced, the grammar's keywords kept — only past eighty:
    189 two small wrappers share a shape because wrappers do, and the tool's own `decode` pair
    190 is what taught it that. The reference count skips comment lines, because a Go doc
    191 comment opens with the name it documents and a comment is not a caller; it reads every
    192 text file in the tree, so a use from a template counts. Debugging shapes are the
    193 statements that exist to be removed — `debugger`, `breakpoint()`, `dbg!`, `spew.Dump`
    194 — and not ordinary printing, because a command's output and a debug print share a
    195 function; only `console.log` is reported, and as a note. The code checks report as
    196 consider or note: each is a comparison, and comparisons have exceptions the reader
    197 knows and the tool does not. `duplicate-body`, token for token, and `assertion-always-true` are the must-fixes: a body
    198 copied whole and an assertion that cannot fail have no exceptions. The tautology check
    199 exists because of the assertion check: once a test must assert something, `assert True`
    200 and `expect(true).toBe(true)` are what a reading that wants to pass reaches for.
    201 
    202 Known misses of the message checks, measured against constructed bad messages: `wip`
    203 and `temp` (under every floor — statistically indistinguishable from `coffee!`, a real
    204 subject in the corpus); "fixed the bug by fixing the bug in the file" (43 bytes, 3.53
    205 bits — lexically repetitive but character-diverse); fluent generic prose ("This commit
    206 modifies the codebase…" at 4.2 bits and 0.73), which is the `hygiene` job's.
    207 
    208 ## Gating
    209 
    210 A staged change has no commit message, and nothing measures one: the previous commit's
    211 would be measured against work it never described, and a must-fix about a message the
    212 author did not write is exactly what an agent will try to fix. A commit-msg hook is
    213 given the message before the commit exists, and hands it over:
    214 
    215 ```sh
    216 #!/bin/sh
    217 # .git/hooks/commit-msg
    218 exec review --message-file "$1" --exit-code
    219 ```
    220 
    221 `--exit-code` is the one place the tool refuses: exit 1 when a must-fix finding stands
    222 after dismissals, 0 otherwise. Without it the tool is advisory whatever it finds, and
    223 the JSON `status` is what a stricter reader consults. An agent's harness can gate the
    224 same way — a hook before `git commit` that runs `review --exit-code`, or one after that
    225 runs `review HEAD^..HEAD --json` and feeds the findings back. The message file is read
    226 the way git reads it: lines opening with `#` are the template's, not the author's.
    227 
    228 `review hook install` writes that hook, naming the binary by its absolute path, and
    229 prints the stanza a Claude Code harness takes, which reviews the staged change before
    230 any `git commit` the agent runs. A hook already there is not replaced unasked. Where
    231 `core.hooksPath` is set, git reads hooks from one directory for every repository and
    232 ignores `.git/hooks`; the install then prints the hook and says where to put it rather
    233 than writing to a place git will not read or to every repository at once.
    234 
    235 The loop an agent runs is review, fix, review again, and `--baseline last.json` is what
    236 tells it the fixes took: each finding of the new run is named against the previous
    237 report's, and the report says which ids `resolved`, which `persisting`, and which are
    238 `new`. Every finding also carries a `snippet`, the line it points at as it stands, so an
    239 agent acts on most findings without opening the file.
    240 
    241 The rules travel with the binary. `review rules` prints every deterministic check and
    242 every job's criteria; `review rules tests` one job's; `review rules no-stutter` the one
    243 rule a finding cited, with the lines that continue it; `review agent` the paragraph an
    244 agent's instructions should hold — how to run the tool, how to read its report, how to
    245 dismiss a finding, and when it may stop. An agent handed a finding can read what it was
    246 judged against without leaving the terminal.
    247 
    248 ## Measuring the checks
    249 
    250 The deterministic checks cost nothing to run, so their precision is measured rather
    251 than assumed. `review bench -n 500` runs them over the last five hundred commits and
    252 prints the fire rate per rule; `-rule message-frustration` lists the commits one rule
    253 fired on with their subjects, which is how a threshold is argued about; `-author` narrows
    254 the commits to one author's, which is how the checks are held against the population
    255 they exist for — agent-authored commits, not the human history they were first
    256 calibrated on. A rule firing on a tenth of ordinary commits is not measuring what it
    257 claims to.
    258 
    259 Measured on the first run of the bench, over 150 commits of the icns corpus: no message
    260 check fired at all; `code-without-tests` fired on 9%, every one a command, a COM shell
    261 extension or a wasm shim that has no test and was never going to; `new-symbol-unreferenced`
    262 on 5%, of which two commits were interface methods an encoder calls by reflection and
    263 functions cgo exports — both exempt since — and the rest exported library API and spec
    264 constant tables, which the check cannot tell from a guess and reports as consider;
    265 `duplicate-body` on 2%, one of them a `Decode` copied whole into a sibling package;
    266 `message-names-unknown` once, on `gRPC`, which is a brand and exempt since.
    267 
    268 `review rules -dismissed` is the other half of that: it counts the `review:ignore`
    269 dismissals in the working tree per rule, with where each is and the reason given. A rule
    270 dismissed everywhere is a rule to rewrite, and the argument against it is already
    271 written in the source.
    272 
    273 ## Criteria
    274 
    275 `criteria/*.md` holds the rules, one file per job, each rule with an id. **Every finding
    276 must cite one**, and a finding citing anything else is dropped before you see it. That
    277 is deliberate: it makes the criteria the thing you tune, rather than the prompt, and it
    278 stops a job inventing a standard on the spot. A rule that turns out to be a comparison
    279 leaves the criteria for a static check under the same id, and a job's criteria say which
    280 of its former rules are measured before it reads.
    281 
    282 ## Dismissing a finding
    283 
    284 Where a finding is wrong, say so in the source it concerns:
    285 
    286 ```go
    287 //review:ignore already-named the zero value, taken by omission
    288 ```
    289 
    290 Not a configuration file. The reason belongs beside the code it justifies, where a
    291 reader meets it, and it survives a clone. `//review:ignore all <why>` dismisses
    292 everything at that spot.
    293 
    294 A dismissal reaches a few lines either side of itself, so one can answer a finding you
    295 did not mean it to. `--verbose` names each finding it dismissed and the reason given.
    296 The one dismissal that cannot be written is one that answers the suppression check
    297 itself: a finding with no file behind it is not dismissible by anyone.
    298 
    299 ## A second reading verifies
    300 
    301 After every job has answered, each job with findings is asked once more, with its
    302 findings listed and the same evidence it read the first time, and answers for each: it
    303 holds, or it does not, with a reason. A note is not put back: nothing gates on a note, so
    304 a second reading of one buys nothing, and it stands marked unverified. A finding that falls is reported as a retraction
    305 rather than deleted — the first reading's word and the second's are both on the record,
    306 with the reason. The pass is advisory like the rest: a job that cannot be asked again
    307 leaves its findings standing, marked unverified, and reports why.
    308 
    309 A finding the verdict list omits stands rather than falls, marked verified: a strict
    310 pass would let one dropped number retract everything the reading found. `--no-verify`
    311 skips the pass — useful when the answer itself is being tested, since then the review is
    312 one ask per job, deterministic against a frozen provider.
    313 
    314 Retractions are verified like everything else: each retracted finding carries an id, so
    315 an agent can answer it — restore the test, rewrite the comment — and read the next run
    316 to confirm the finding stayed retracted.
    317 
    318 ## Packets that do not fit
    319 
    320 A job's subject over 16 KB is asked in parts, cut file by file into runs that each fit,
    321 and each part is asked, cached and verified on its own, against its own evidence. The
    322 tests packet of one seven-file change measured 22 KB and was killed at the ask timeout on
    323 a slow local gateway; in parts, each ask is one that gateway finishes, and a part whose
    324 files did not change replays from the cache while the others are asked. The hygiene job
    325 reads the message and cannot be cut.
    326 
    327 ## The answer cache
    328 
    329 Every answer is keyed by the provider and the exact prompts that produced it, and
    330 recorded under `os.UserCacheDir()/review/answers.json` (usually
    331 `~/.cache/review/answers.json`). A second run of the same change asks nothing: it
    332 replays the recorded answers, reports `replayed` in the usage instead of token counts,
    333 and returns the same findings — ids and verdicts included. That is what makes a loop
    334 deterministic: the finding and the retraction come back the same way until the change,
    335 the criteria or the model changes.
    336 
    337 `--fresh` asks rather than replays, and records what it learned, so the cache is
    338 replaced rather than grown stale. The cache holds the 4,000 newest answers; a file it
    339 cannot read is replaced on the next save, and a prompt the cache cannot answer is asked
    340 the ordinary way. Only an answer the tool can read is recorded: a model that spent its
    341 whole output budget and said nothing was once replayed on every run, and the job failed
    342 the same way each time without asking again.
    343 
    344 ## The JSON contract
    345 
    346 `--json` writes one object, so an agent can read the whole measurement without parsing
    347 prose:
    348 
    349 ```json
    350 {
    351   "version": 1,
    352   "provider": "pi/maple/glm-5-3-flash",
    353   "status": "complete",
    354   "findings": [],
    355   "retracted": [],
    356   "failed": [], "skipped": [], "uncovered": [], "dismissed": [],
    357   "truncated": false,
    358   "usage": {"in": 3327, "out": 15550, "cached": 0, "replayed": 5, "usd": 0}
    359 }
    360 ```
    361 
    362 `status` is what keeps an empty findings list from being read as a pass: `empty` (no
    363 change), `complete` (every job answered, every file read), or `incomplete` — when a job
    364 failed, a file no reader could read, or the diff was cut to fit. A finding carries its
    365 `id`, `verified` (only the verify pass can say true; static checks are their own word),
    366 `severity`, and the `rule` id from the criteria it cites. A retraction carries the
    367 finding it ends and the verdict's `reason`; a dismissal carries the finding and the
    368 reason written in the source, named like the rest. `usage` is always present, and counts
    369 `replayed` answers rather than tokens when the answers were cached; `usd` is 0 unless
    370 the provider reports a cost.
    371 
    372 The id is a hash of what a finding is about, not where it sits or how it was worded:
    373 job, rule, file and symbol. The line is left out because lines move under edits that do
    374 not touch the finding; a model's message is left out because a fresh reading words the
    375 same finding differently, and an id that changed with the wording would name nothing.
    376 A finding with no symbol is told from its neighbours by its line; a deterministic
    377 check's message is part of what it is about — the coupled partner, the measured number
    378 — and stable, so it stays in. Two findings that still hash the same are told apart by a
    379 counter: `a1b2c3d4e5f6`, `a1b2c3d4e5f6-2`.
    380 
    381 ## Who answers
    382 
    383 Nothing here cares which model answers. A provider takes a prompt and returns text.
    384 
    385 | `--provider` | how | credentials |
    386 |---|---|---|
    387 | `chain` (default) | probes the configured order — the console API, then `claude` on the path, then `pi` against the local gateway — and the first that answers serves the whole reading | whichever entry it lands on holds |
    388 | `claude` | the coding assistant on the path, `-p --output-format json` | whatever it already holds |
    389 | `api` | the console API directly | `ant auth login` or `ANTHROPIC_API_KEY` |
    390 | `pi` | `pi -p --mode json`, which speaks to several providers of its own | its own; name the upstream in `REVIEW_PI_PROVIDER` |
    391 | `command` | anything at all: `REVIEW_COMMAND` is the command line, `REVIEW_COMMAND_FIELD` the JSON field its answer arrives in | its own |
    392 
    393 `--model` names the model in whatever form that provider uses. `REVIEW_PROVIDER` and
    394 `REVIEW_MODEL` set the defaults.
    395 
    396 The chain is for the hours when a limit is spent: one cheap ask per skipped entry,
    397 the reason on stderr, and the reading happens on whatever still answers. `REVIEW_SERIAL`
    398 asks a provider's jobs one at a time, for providers that cannot take concurrent reads.
    399 
    400 The default model is the middle one, not the smallest. Against the eval set the
    401 smallest reads the shortlist of duplicates and reports the first one it finds rather
    402 than all of them — roughly two thirds of the wanted findings, and never both of a pair
    403 in one reading. That is the one thing this tool is for, so it is not a saving.
    404 
    405 Only the `api` provider can enforce the answer's shape, through a strict tool schema.
    406 The rest are asked for JSON in the prompt and the object is taken out of whatever they
    407 wrap it in, so every provider answers the same way whether or not it can be held to it.
    408 It also pins sampling, which the others cannot: a loop between two models converges
    409 faster when one of them answers the same way twice.
    410 
    411 Measured, one job over a seven-file commit through `claude`: **$0.19 at the default
    412 model**, $0.06 at the smallest. A whole change, all five jobs, is a few times that. The
    413 same packet through `api` costs a fraction of a cent — the difference is the
    414 assistant's own harness, which these jobs do not use. Convenience has a price and this
    415 is it.
    416 
    417 Prompt caching does work through `claude`, but what it caches is that harness: about
    418 22,000 tokens, the same count on every job however different their criteria. The
    419 criteria themselves are appended after it and are not cached. The `input_tokens` that
    420 provider reports is 9 whatever it was sent, so `total_cost_usd` is the only figure from
    421 it worth reading.
    422 
    423 ## The eval set
    424 
    425 There is none yet. The Go implementation this tool replaced scored readings against
    426 cases with a known right answer; that harness went with it. Until one exists here, the
    427 measure of a reading is the side-by-side run the port was checked with: both tools on
    428 the same change, the reports diffed field by field, and the answer cache shared so a
    429 replayed answer is compared rather than re-asked.
    430 
    431 ## Languages
    432 
    433 Go is read through the `review-go` sidecar, built from `sidecar/gofront` on Go's own
    434 parser, and Odin through `odin-review-extract`, built from `sidecar/odin` on Odin's;
    435 both print one JSON shape the tool reads. TypeScript and JavaScript — `.ts`,
    436 `.tsx`, `.js`, `.jsx`, `.mjs`, `.cjs` — are read through ast-grep when it is on the
    437 path, by pattern; Python and Rust through the same ast-grep, by node kind — a function
    438 is whatever the grammar calls one, and its name is read out of the match.
    439 Every other language gets its comments read by shape, the message and history checks,
    440 and the line-shaped code checks; the jobs that need declarations or test bodies are
    441 skipped for its files, with a line on stderr saying so, and the files are named in the
    442 report's `uncovered` list. The repository is read by the
    443 tool itself: git is asked for the diff, the history and one archive of the tree, and
    444 everything after that is a loop over memory, the same on every system.
    445 
    446 ## What it is not
    447 
    448 It does not format, and it writes no analyser of its own where a language already has
    449 one: the compilers, vet, staticcheck, clippy, ruff, mypy and semgrep are run as they
    450 are, with their strictest settings, and their word is kept to the change. The code
    451 checks here measure what those tools leave alone — a test that asserts nothing, a body
    452 written twice, a name nothing refers to — and hand the rest to the readings.