Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ jobs:
- name: Check every probe record
run: npm run check:probes

# Fails when a mined verdict record is malformed or states its own
# disposition. What it counts, and what it withheld, prints as a note.
- name: Check every verdict record
run: npm run check:verdicts

# Fails when the resident fragment and the skill it comes from disagree.
# One rule has two delivery forms, and two forms of one rule drift.
- name: Check the resident fragment against its skill
Expand Down
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,9 @@ node_modules

# Bench samples are generated. The protocol reproduces them.
bench/out/

# Review scenarios are rebuilt from the pinned commits in bench/verdicts/, so
# they are derived rather than evidence. A promoted study retains the copy the
# arms actually answered.
bench/review-prompts/
.claude/worktrees/
61 changes: 59 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,21 @@ This repository ships writing skills. Its own documents are written under one of
those skills, and continuous integration checks them with its own tool. Hold a
change here to the standard the repository sells.

## Run these eight before you claim a change is done
## Run these nine before you claim a change is done

```bash
npm test # unit and conformance tests
npm run lint:docs # our own writing rules, applied to our own documents
npm run check:ground # every grounding matrix still matches its skill
npm run check:docs # every document's front matter fits the schema
npm run check:probes # every probe record carries what a reader derives from
npm run check:verdicts # every mined review thread states no disposition
npm run check:resident # the resident fragment still matches its skill
npm run check:studies # every promoted study still matches its own digests
npm run check:editorial # the editorial audit record, and what it counts
```

`npm run check` runs all eight.
`npm run check` runs all nine.

## What counts as a defect here

Expand Down Expand Up @@ -519,6 +520,17 @@ number published in `bench/README.md`.
killed and refused by name at a deadline, because a hung re-run takes
the whole gate with it. ADR-0023 carries the reasoning and the flip
condition.
- **A retained command spells every path with one separator, `/`.**
`commandPath` in `bench/study.mjs` states them and `commandProblems`
refuses the other spelling by name, so the writer and the reader sit in
one file. `path.relative` spells a path `bench\samples\...` on Windows,
and that spelling travels: the same command resolved inside the study
there and read as one filename everywhere else, so a study promoted on
Windows was refused on Linux and the message named containment rather
than the separator. The separator is checked BEFORE containment, and the
containment message is withheld, because it is the artifact. This is the
rule `src/manifest.js` and `src/tree.js` already state for a manifest
key, and `SCORER` already states for the program name.
- A study that is already refused for any reason is never re-run. The
narrower gate read the command's own problems alone, and containment
here is a string predicate over `path.resolve`, which resolves no
Expand Down Expand Up @@ -664,6 +676,51 @@ number published in `bench/README.md`.
model as names and descriptions, and a body loads when the model invokes
the skill. A body-planted nonce measured invocation with section 4.1's
instrument, and its failure attributed to nothing.
- A verdict record states no disposition. `bench/verdicts.mjs` derives one
from the thread the record retains, and `check:verdicts` prints what it
derived. A record that grades itself is refused. That is the probe
record's rule over a second corpus, and `bench/verdicts/README.md`
carries the protocol. ADR-0032.
- **Two readings come off one thread, and each withholds on its own
cause.** The verdict withholds as `no-reply`, `no-verdict-block`,
`ambiguous-block` or `unrecognised-word`. The anchor withholds as
`left-side`, `no-path`, `no-line` or `inverted-range`. One cause for
both would tell a reader the wrong thing about whichever half was fine,
which is what `trace_withheld` fixed one file over. A withheld reading
contributes no disposition and fails nothing, and the census counts it.
- **No message printed by the verdict reader carries a byte of a mined
body.** A body is third-party text about our code, some of it written by
an automated reviewer, and `bench/verdicts/README.md` declares the
directory untrusted data the way `bench/samples/README.md` does. An
unrecognised verdict word is named as a cause and never quoted, and
every message goes through `redact` at the point of emission.
- **A round is a reviewed commit, and a run names its pull requests.** The
arm reads `git diff <base>...<review commit>`, which is the diff the
reviewer saw, so the mined anchors point at lines the arm can name. The
merged diff carries the fixes, so the ground truth is not in it.
Eligibility is the corpus rule and selection is `--pr`, because each
scenario costs two arms of live calls and only the operator buys those.
- **The review counts are bounds, not identifications.** A match is a path
and a line within `MATCH_WINDOW`, so two confirmed findings close
together in one file are not separable. `confirmed` is a ceiling and
`missed` is a floor, and a document that states either as a count claims
a precision the rule does not have. Widening the window is not the fix.
- **A study retains the ground truth it scored against.** The scorer's
`--review` names the promoted copy under `<study>/verdicts/`, because
`commandProblems` refuses a path outside the study and because a re-run
against the live corpus would reproduce a figure from bytes the study
does not hold. The prompts are retained for that reason, and the
argument transfers whole.
- **A withheld output-token count withholds the rate.** `extract.mjs`
writes `absent` where the harness reported no usage, never a zero, and
`reviewMetrics` prints an empty cell rather than dividing. `--review`
requires the sidecar FIELD and admits `absent` as its VALUE, which is
ADR-0024's split: a field a check reads is required, and a protocol
choice about the value decides a reading.
- Three of the five mined review rounds refuse promotion's content scan,
because the diffs under study are this repository's own and carry that
scan's own test fixtures. The scan is right and the corpus is smaller.
Do not narrow the scan to widen the corpus.

### A write into a file the user owns

Expand Down
20 changes: 20 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,26 @@ and [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

### Added

- A verdict corpus, mined from this repository's own review threads, and the
scorer support that reads it. This repository disposes of every review finding
with a fenced `review-verdict` block, so it already holds a labelled corpus of
findings and their dispositions. `bench/mine-verdicts.mjs` retains the thread
and states no disposition, `bench/verdicts.mjs` derives one, and
`npm run check:verdicts` prints what it derived and joins the gate as a ninth
check. Two readings come off one thread and each withholds on its own cause, so
a thread with no reply and one anchored on the left of the diff say different
things rather than both reading as broken. `bench/score.mjs --review` adds five
columns for the review-verbosity study: what an arm claimed, how much of the
round's confirmed ground truth it reached, what it dropped, the output tokens
it spent, and confirmed findings per thousand of them. Both counts are bounds
rather than identifications, because a match is a path and a line within ten
lines. `bench/review-arms.mjs` rebuilds the diff each reviewer read, from the
commit the corpus pins, and prints the two arm invocations rather than running
them. A promoted study retains the corpus it scored against, so the re-run in
`check:studies` reads bytes the study holds. No arm has run and no figure
exists. ADR-0032 records the decision, its two deviations, and what a wider
window would cost.

- A grounding matrix disposes of one file, and `ground --check` reads every file
a skill ships to a writer. `SKILL.md` keeps `grounding/<tier>/<skill>.md`, and
a Markdown file under `references/` answers to a matrix that mirrors its path,
Expand Down
3 changes: 2 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ npm ci
npm test
```

Eight checks gate every pull request. `npm run check` runs all eight, and you
Nine checks gate every pull request. `npm run check` runs all nine, and you
can run any one of them on its own.

```
Expand All @@ -43,6 +43,7 @@ npm run lint:docs # our own writing rules, applied to our own documents
npm run check:ground # every grounding matrix still matches its skill
npm run check:docs # every document's front matter fits the schema
npm run check:probes # every probe record carries what a reader derives from
npm run check:verdicts # every mined review thread states no disposition
npm run check:resident # the resident fragment still matches its skill
npm run check:studies # every promoted study still matches its own digests
npm run check:editorial # the editorial audit record, and what it counts
Expand Down
64 changes: 64 additions & 0 deletions bench/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,48 @@ Add a scenario by dropping a `.txt` file in `prompts/`. Keep one position per
file, and keep the reader's own words in it, because the echo measure below
compares the reply against the prompt.

### The review arms

Issue #109 runs two arms over pull request diffs rather than over the four
positions above. The question is whether a fixed per-finding shape buys more
real findings per thousand output tokens, and what it drops to get them.

Every other scenario here is scored on shape alone. This one has a right
answer, and the answer is already in this repository. AGENTS.md disposes of
every review finding with a fenced verdict block, so a person has already said
which findings described a real defect. `bench/verdicts/` mines those blocks
and `bench/verdicts/README.md` carries the protocol. ADR-0032 records the
design.

```
node bench/review-arms.mjs --pr 112 --pr 118 --write
bench/run.sh review-baseline-112-118 --prompts bench/review-prompts/112-118 --reps 5
bench/run.sh review-compact-112-118 --prompts bench/review-prompts/112-118 --reps 5 \
--system bench/review-contract.md
```

`review-arms.mjs` prints those two lines, so read them from the plan rather than
typing them. The selection is in every name. Two selections sharing one
directory gave one arm covering both, and that arm covered its plan, so nothing
downstream saw a problem. ADR-0032 carries the trace.

A scenario is one review round, and a round is one commit a reviewer read.
`review-arms.mjs` rebuilds that diff and writes the scenario file. It spends
nothing, it prints the byte size of each prompt before you buy it, and it
prints the commands rather than running them.

A run names its pull requests. Mining says which ones are eligible, and `--pr`
says which ones this run buys. The two are separate because each scenario costs
two arms of live calls.

Read the arms with `--review`, which needs the corpus:

```
node bench/score.mjs --compare --review bench/verdicts \
--prompt bench/review-prompts/112-118/pr-118-r1.txt \
bench/out/review-{baseline,compact}-112-118/pr-118-r1-*.txt
```

## Half two, the field

The bench cannot reach the two places the length problem is worst: long agentic
Expand Down Expand Up @@ -164,6 +206,28 @@ sample, a median row, and a range row.
| `echo` | Share of the reply's prose word pairs that appear in the prompt's prose. | See the warning below. Not a restatement measure. |
| `noise` | Harness lines stripped from an older sample. | Non-zero means that arm may not be comparable to one scored at zero. |

`--review DIR` prints five more columns and needs the mined corpus. Without the
flag the table is exactly the one above, because a column of empty cells reads
as a measurement of nothing rather than as a mode nobody asked for.

| Metric | What it counts | Read it as |
|---|---|---|
| `anchors` | Distinct `path:line` places the reply names. | How much the arm claimed, whatever shape it claimed it in. |
| `confirmed` | Confirmed findings of that round the anchors reached. | A ceiling on agreement. See below. |
| `missed` | The rest of that round's confirmed findings. | A floor on what the arm dropped. It sums with `confirmed`. |
| `outTokens` | Output tokens the sidecar recorded. | Empty means the harness reported none, and the rate is withheld. |
| `perKtok` | `confirmed` per thousand output tokens. | Issue #109's primary metric. |

**Both counts are bounds, and neither identifies a finding.** A match is a file
path and a line within ten lines of the mined anchor, so two confirmed findings
close together in one file are not separable. Pull request #118 is that case:
three of its confirmed findings anchor within seven lines of each other, so one
stated line reaches all three. ADR-0032 states the rule and this failure mode.

The counterweight is the difference between the two arms' `missed` rows. It is
not a cell, because a cell would have to pick which baseline sample to subtract
and would then hide that choice inside one number.

Everything but `words` reads prose with fenced code removed, because a heading or
a bullet quoted inside a fence is the reader's material, not the writer's shape.
That includes `echo`, which briefly did not: on `adjacent-bug` a reply quoting the
Expand Down
19 changes: 17 additions & 2 deletions bench/extract.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,17 @@ if (typeof run.result !== 'string' || !run.result.trim()) {
// call alongside the answering one on some prompts and not others, so a strict
// "exactly one" check refuses good runs at random. The model that wrote the
// answer is the one that emitted the output tokens.
//
// Two readings come off the same entry, and they are kept apart. The RANK is
// how the answering build is chosen, and an absent count ranks as zero so the
// comparison below still has numbers to compare. The REPORTED count is what
// the sidecar records, and it is null when neither spelling is there, because a
// zero written for an absent field is the wrong number rather than a missing
// one. Issue #109 divides by this figure, and a silent zero would divide by it.
const count = (u) => (typeof u?.outputTokens === 'number' ? u.outputTokens
: (typeof u?.output_tokens === 'number' ? u.output_tokens : null));
const usage = Object.entries(run.modelUsage ?? {})
.map(([id, u]) => [id, u.outputTokens ?? u.output_tokens ?? 0])
.map(([id, u]) => [id, count(u) ?? 0, count(u)])
.sort((a, b) => b[1] - a[1]);

if (!usage.length) {
Expand All @@ -56,4 +65,10 @@ if (usage.length > 1 && usage[0][1] === usage[1][1]) {
}

await fs.writeFile(outPath, run.result);
process.stdout.write(usage[0][0]);
// Two whitespace-separated fields: the build, and the output tokens it emitted
// or the word `absent`. `run.sh` splits them into two sidecar entries. The
// spelling `outputTokens` is the one this runner has read since it was written,
// and no measurement here confirms it under a review invocation, so `absent` is
// a state the protocol has to carry rather than an accident. ADR-0032 names it
// as a gap for exactly that reason.
process.stdout.write(`${usage[0][0]} ${usage[0][2] === null ? 'absent' : usage[0][2]}`);
Loading