diff --git a/README.md b/README.md index 8f5d50b..abc0b60 100644 --- a/README.md +++ b/README.md @@ -783,7 +783,8 @@ session-start line or `work` says something unexpected, these files are what it | `revision/claims.md`, `revision/merges.txt`, `revision/sweep/` | `revise`'s sweep at `prepare` step 2: the rules and documented claims it judges, `scripts/window_merges.py`'s printout with `--doc`, and one `diff-probe` block per merged pull request (`pr-NNNN.md`) | `revise`, for the report's Practice section, through `scripts/block_tally.py` | Replaced at each revision | | `revision/drift.txt` | `revise` `prepare` step 2, through `scripts/drift_checks.py`: the scripted claims' `claim` and `break` lines | `prepare` steps 2 and 6, for "Practice" and `ticket/` and `doc/` proposals | Rewritten at each revision | | `revision/state.json` | `revise` `prepare`, through `scripts/revise_state.py`: each stage done or skipped, with why | `prepare` step 10, which writes the report only once every stage is recorded | `revise_state.py start`, at the next `prepare` | -| `revision/notes/` | The driver, before `revise` `prepare`: meeting or chat notes, pasted | `revise`, read as asks at `prepare` step 5 | Not cleared by the tools | +| `revision/notes/` | The driver, before `revise` `prepare`: meeting or chat notes, pasted | `revise`, read as asks at `prepare` step 5, and by `scripts/quote_check.py` for the report's pasted quotes | Not cleared by the tools | +| `revision/spot-check.md` | `revise` `prepare` step 2: one line per guidance item `scripts/spot_check.py` sampled, right or wrong | `scripts/spot_check.py tally`, for the report's Head | Rewritten at each revision | | `revision/tickets/` | `revise` `apply`: each accepted `ticket/` proposal as a prompt (`-.md`) | `ticket-draft`, given a prompt's path, one at a time | Replaced at each `apply` | | `revision/feedback/` | `revise`: a `skill-feedback` draft per procedure finding, from `prepare` step 2 and `correct` step 2 | `skill-feedback`, given a draft's path | Not cleared by the tools | diff --git a/agents/diff-probe.md b/agents/diff-probe.md index 419b008..66aa80f 100644 --- a/agents/diff-probe.md +++ b/agents/diff-probe.md @@ -69,7 +69,9 @@ One line per claim id you were given, in the order given: Settle each verdict before you write its line: weigh the evidence, then write the one word it supports. A line's reason never argues another verdict than its word; when the evidence leaves -you between two, write the one it leans to and say in the reason what made it close. +you between two, write the one it leans to and say in the reason what made it close. A line +whose place or reason names a second verdict, such as `judged as follows` after `breaks`, fails +the block, and you are run again. ## The answer diff --git a/agents/source-reader.md b/agents/source-reader.md index 40f4a2c..506f936 100644 --- a/agents/source-reader.md +++ b/agents/source-reader.md @@ -134,8 +134,10 @@ Epic candidates: ``` **`page `**: `confluence_get_page`. Returns `## page — `, a line `Version: <n> · -Updated: <date>`, and one paragraph: what the page decides, and which code or process it -constrains. +Updated: <date>`, one paragraph: what the page decides, and which code or process it +constrains, then one or two lines `Excerpt: "<the page's words>"`, each one run of the page's +text, verbatim, at most 40 words, that bears out the paragraph, so a reader can find it on the +page. A paragraph no excerpt bears out goes to `Questions:` instead. **A file in place of a key or an id.** `ticket` and `page` take a file path the caller names instead: build the block from the file's text, with `Source: file <path>` as its first line, and diff --git a/docs/contracts.md b/docs/contracts.md index 64ecde9..ba0717b 100644 --- a/docs/contracts.md +++ b/docs/contracts.md @@ -466,6 +466,71 @@ Writers: the driver, in harvest.md's "Clusters". Readers: `scripts/guidance_clus Pinned by: tests/test_guidance_clusters.py::test_prints_each_cluster_with_its_people_prs_and_quotes +## The report's quotes + +Version: 1 + +Fields: every line of a revision report, `ratchet-YYYY-MM.md`, that opens `said:` once its indent +is stripped, a drafted entry's under `entry:` included: `said: "<words>" — <login>, <url>`, a +comment's, or `said: "<words>" — <name>, pasted <file>`, a note in `.review/revision/notes/`. +`scripts/quote_check.py <report> --comments <comment-list.json ...> --notes <dir> [--held +<ledger>]` finds each: 1 to 40 words, in the own words of the comment with that `url` in the +comment lists, whose `author` is `<login>`, as "The guidance items" finds an item's quote, or in +the text of a note inside `--notes`, every line kept, never starting or ending inside a word. A +line holding `said: "` anywhere that is not such a line of its own, a quote nested in a list item, +a table or a blockquote, or a `said:` line in another case, is a fault. A URL's pull request is +its repository and number, whatever page of it the URL names; a quote from a pull request no +comment in the lists shares is a fault unless the ledger given with `--held` carries both its +words and its URL under its `## Candidates below the criteria` heading, however that section lays +them out: a held candidate's quote from before the log kept captures, which is unchecked. It prints `quotes: <n> · from +comments: <n> · from notes: <n> · unchecked: <n>`, then `unchecked · line <n> · <url>: a held +candidate's, no comment list given holds its pull request` per unchecked quote, and exits 0, or one `refused · line <n> · <why>` per fault, and `refused · <path> · <why>` per +input it cannot read, and exits 1. An ellipsis or a changed word is a fault: a quote is one run of +the comment, verbatim. + +Writers: `revise` `prepare` step 10, the report as `outline.md` gives it. Readers: +`scripts/quote_check.py`, run by `prepare` step 10 before the report is offered for commit. + +Pinned by: tests/test_quote_check.py::test_a_quote_absent_from_its_comment_fails_the_report + +## The spot check + +Version: 1 + +Fields: `scripts/spot_check.py sample --seed <text> [--n 10] <slice ...>` prints, for `--n` items +picked at random from the guidance slices given (every item when there are fewer, the same ones for +the same seed and slices), `spot · <dir>/<slice>#<n> · pr-NNNN · <url> · <kind> · <generality> · +"<quote>" · <principle>`, `<dir>` the slice's directory name, `<slice>` its name without `.json`, +`<n>` the item's place in it from 1. `.review/revision/spot-check.md` holds one line per sampled +item, `<id> · right` or `<id> · wrong · <what>`. `spot_check.py tally` with the same seed and slices +and `--checked <that file>` prints `spot check: <n> items · <n> wrong · error rate <percent>%`, then +`wrong · <id> · <what>` per wrong item, or `spot check: 0 items` with none. Both exit 1 with one +`refused · <where> · <why>` line per fault: a slice that cannot be read, `--n` below 1, and for +`tally` a line of neither form, an id not sampled, or a sampled id with no line or two. + +Writers: `revise` `prepare` step 2, `.review/revision/spot-check.md`. Readers: +`scripts/spot_check.py tally`, whose `spot check:` line is the report's Head line. + +Pinned by: tests/test_spot_check.py::test_the_tally_counts_wrong_items_into_an_error_rate + +## The window's cost + +Version: 1 + +Fields: `scripts/window_cost.py --threads <dir>` reads every `pr-NNNN/` dump under --threads, a +harvest's or a load's, a pull request with no record included: its `pr.json` and `body.md`. It +prints `cost: open-to-merge median <d>d, longest <d>d · <n> merged without a build · <n> pull +requests`, durations in days to one decimal from `created_at` to `merged_at`, a pull request +merged without a build one whose body has no Already checked block, as `scripts/record.py` reads +it; or `cost: no merged pull request`. It exits 1 with one `refused · <path> · <why>` line per +fault: a `pr.json` missing or with no readable `created_at` or `merged_at`, a merge before its +opening, a dump with no `body.md`, or a `--threads` that is not a directory. + +Writers: `scripts/window_cost.py`. Readers: `revise` `prepare` step 1, whose line is the report's +Head `cost:` line. + +Pinned by: tests/test_window_cost.py::test_the_median_and_longest_open_to_merge_and_the_unbuilt_count + ## The judgments Version: 1 @@ -740,10 +805,10 @@ Pinned by: tests/test_profile.py::test_a_github_tracker_maps_at_most_one_label_t ## The record -Version: 14 +Version: 15 Fields: `<log>/pr-NNNN.yaml`, `<log>` the records directory `revise`'s "The standard's repository" -names, first line `contract: 14`. This section is the one +names, first line `contract: 15`. This section is the one place the fields are defined; the records directory's own README says how the revision reads them. @@ -794,6 +859,9 @@ reads them. reviewed heads. - `findings:`, every cited finding: `key` (the full finding key `review-contract.md` defines), `severity`, `at` (the file and line, or the file alone at file level), `thread` (the review thread the verdict was read from), + `reviewer` (the login of the account that posted the finding's comment, whose token ran the + review; the attribution line names it too when the poster is the person, but the record reads the + comment's author, not that line), `verdict` (agree, disagree, discuss, unanswered, unparsed or contested), with `judged: true` and `basis:` when it was judged from a reply's own words, and `done: true` when that reply said the change was made, or `basis:` alone when the judge could not tell, `reason` on a @@ -807,11 +875,11 @@ reads them. first line; its replies are not read, since the severity decides how they read. `scripts/ratchet_counts.py` counts it as a firing in its `unlabeled` column, not with the unanswered. -- `questions:`, every cited question: `q`, `key`, `at`, `thread`, and `a`, the answer, or +- `questions:`, every cited question: `q`, `key`, `at`, `thread`, `reviewer` as on a finding, and `a`, the answer, or `unanswered: true`. - `threads:`, a pointer to every other review thread: a person's, a bot's, or a finding posted with no key. Each carries `thread`; `by`, its opener, `bot` for a bot or `review` for an uncited - finding, which also carries its `severity`, `unparsed` with `verdict: unparsed` and `line:` as + finding, which also carries `reviewer` as on a finding and its `severity`, `unparsed` with `verdict: unparsed` and `line:` as on a finding; `authorship` of its opening comment, as "Authorship" below defines it; `at`; `opened`, the day; `resolved` and `outdated`, GitHub's flags; `replies`, the count from people; `verdict`, an uncited finding's read as a finding's, otherwise the pull @@ -849,6 +917,8 @@ Version 13: `authorship` reads the signatures under "Authorship" as well as the and the standard's phrases, so a reply reworded from the attribution line, and an agent's own signature, read `ai-drafted` where earlier versions read `human`; a thread the author opened that way reads `verdict: none`, where it read `unanswered` or `unparsed`. +Version 15 added `reviewer` on every finding, cited question and uncited finding, so a finding is +attributable to the person who ran the review; a record of an earlier version has none. A harvested record reaches the log through `scripts/archive_records.py`, which refuses to replace a record whose findings, counted per rule, the harvested one falls short of, unless told @@ -1109,7 +1179,7 @@ Pinned by: tests/test_revise_state.py::test_the_report_waits_until_every_stage_i ## The analysis blocks -Version: 1 +Version: 2 Fields: `.review/harvest/analysis/pr-NNNN.md`, one `thread-analyst` report, its blank lines ignored. Either the one line `record mismatch: <ids>`, or the heading `## pr-NNNN — threads read` @@ -1129,8 +1199,9 @@ which is not the record's verdict; `procedure · pr-NNNN · <the Procedure: text pr-NNNN · <the Not read: text>` for each not `none`; and `mismatch · pr-NNNN · <ids>`. It exits 2 with a `CANNOT RUN` line, printing nothing, on a named block missing, a pull request named twice or not as `pr-NNNN`, an empty block, a heading missing or naming another pull request, a `record -mismatch:` line with more after it, a line of no form above, or a `Procedure:`, `Not read:` or -`Read:` line missing or repeated. +mismatch:` line with more after it, a line of no form above, an ask line with a second `· +outcome:` or `· diff:`, which is two verdicts in one line (version 2), or a `Procedure:`, `Not +read:` or `Read:` line missing or repeated. Writers: `thread-analyst`, at the path harvest.md's "Analysts" gives it. Readers: `scripts/block_tally.py`, run by harvest.md's "Analysts"; `revise` `prepare` step 2 and `onboard`'s @@ -1140,7 +1211,7 @@ Pinned by: tests/test_block_tally.py::test_the_analysis_counts_asks_by_who_sourc ## The sweep blocks -Version: 2 +Version: 3 Fields: `.review/revision/sweep/pr-NNNN.md`, one `diff-probe` block, its blank lines ignored: the heading `## pr-NNNN · <sha, 7 characters>` naming the file's pull request; one line per claim, @@ -1158,8 +1229,14 @@ rules first, `claim · <id> · judged <n> · follows <n> · breaks <n> · not to exits 2 with a `CANNOT RUN` line, printing nothing, on a named block, the claims file or a named merges file missing, a `### C-n` heading not in its form, an `every change` claim's document with no `doc` line in the merges file, a pull request named twice or not as `pr-NNNN`, a heading missing or naming another pull request, a -line of no form above, a claim judged twice, a block with no line for a `C-n` claim it owes, or a `Not -read:` line missing or repeated. +line of no form above, a line holding two verdicts, a claim judged twice, a block with no line for a `C-n` claim it owes, or a `Not +read:` line missing or repeated. A line holds two verdicts when its place or its reason has a `·` +field that is `follows`, `breaks`, `not touched` or `not judged` alone, or opens with one as a +label, `breaks:` or `breaks —`, other than its own verdict, or reads `judged as` one of them, the word ending at a space, punctuation other than `/` +or `-`, a `.` not followed by a letter, or the line's end; a field that uses the word in a +sentence, `follows the old habit`, or a path, `breaks/x.py`, is not one; the quoted line, which is +code, is not read for it (version 3). Such a block is rerun, never +counted. A claim whose heading ends ` · script: <check> [<arg> ...]` is judged by `scripts/drift_checks.py`, diff --git a/docs/design.md b/docs/design.md index 190b2fd..e5e66fe 100644 --- a/docs/design.md +++ b/docs/design.md @@ -67,6 +67,8 @@ Libera's standard. **The month is read as it merges, and the revision reads the log.** Harvesting a month in one `prepare` started about sixty agents in one session, past Claude Code's limit on agents in a session, and a failure late in the run cost everything read before it. So the driver runs `revise capture` weekly: it harvests the whole window again, one `pr-reader`, so every record takes the replies posted since the last capture, and sends only the pull requests new or changed since then to the analysts and the extractor, keeping each pull request's harvest beside its record in the standard's log; `prepare` restores the window from there, capturing this repository only when its git shows a merge the log lacks, and each other covered repository always, one `pr-reader` that reads nothing unchanged, since no git here shows what merged there today. The log, committed, not a local directory, holds it, so a capture survives a new machine, a new driver and the clone `prepare` makes; the days each capture searched are written down, so `prepare` can tell a quiet week from one nobody captured, and checks git's merges against the captures by number, since GitHub's search is the only list a capture has. Nothing runs at merge itself: a GitHub Action, or a scheduled agent, would need a model's credentials and a person's answers at the archive's refusal and the commit, and a weekly capture spreads the same work. (`skills/revise/capture.md`; `docs/contracts.md`, "The capture".) +**The report's evidence is checked, not trusted.** A revision once carried an analyst's paraphrase where a person's words belonged, counted a probe line that gave two verdicts as a break, recorded 42 findings with no one named as having run the review, and, in its write-up, eight figures nobody re-derived. Each is a place where a model's output reached the group unchecked. So every quote in the report is a `said:` line a script finds in the comment it cites; a line holding a second verdict fails its block, which is rerun rather than counted; each finding records the login that posted it; every figure is copied from a script's printout; and ten extracted items, picked at random, are checked against their comments by the driver, who did not extract them, with the error rate in the head, so the group can see how far to trust the extraction. A schema in JSON for the probes was weighed against tightening their line forms: a fixed line form a script parses already is a schema, and refusing a second verdict closes the gap with no new format. (`skills/revise/prepare.md` steps 2 and 10; `docs/contracts.md`, "The report's quotes", "The spot check" and "The sweep blocks".) + **The revision reads what merged, not only what was said.** A rule's firings come only from findings, so a merged change that breaks a rule nobody flagged, or a documented practice that most pull requests skip, never reached the report. The sweep reads the merged pull requests' diffs, commits and descriptions against the rules whose `scope:` they touch and the claims the repository's documents make, from git alone, so it runs with no server: a script judges the claims a script can, such as a template's sections and a TODO's ticket, on every pull request opened after the document, and an agent the rest on at most five. It is a sibling of `thread-analyst`, `diff-probe`, rather than more work for it, because it runs on pull requests that had no threads and leaves the analyst's output unchanged; and it is not the full reviewer, which costs too much per pull request. Its results are reported and counted by no Dial until two revisions have shown how often they agree with the group, the same report-first start the probe had. What the sweep and the threads find still in the code becomes a `ticket/` proposal, and a document the merged work has left behind a `doc/` proposal, marked like a rule; an accepted ticket goes to `ticket-draft` one at a time, so Jira is still written only through its write plan. The report opens with an overview of every proposal by priority and impact in place of a fixed cap: a cap hid proposals the group might have wanted, while an ordered list lets the meeting stop when its time is up and leaves the rest blank, deferred. (`skills/revise/prepare.md`, steps 2, 6 and 10; `agents/diff-probe.md`; `skills/revise/apply.md`, step 3.) **A verdict is what a reply says, read from its word first.** A reply under a finding says Agree, Disagree and a reason, or Discuss. When its first line opens with the word, `scripts/record.py` reads it, for free and the same every time. When it does not, `pr-reader` judges what the reply says, and the record marks the verdict judged with a one-line basis: people answer in their own words, and a marker they must remember to type turned clear agreements into unanswered threads that sent the author back to `pr-fix`. The judge sees only replies with no word after the last word in the thread, and the script refuses a judgment on anything else, so a judgment never overrides a later word, the author's judged reply decides as their word would, and a person who disagrees with a judgment settles it by replying with the word. A reply the judge cannot read stays open and is walked in `pr-fix`; a finding nobody answers is recorded as unanswered and counts for nothing, and a resolved thread is not a verdict. The reason under a Disagree is the one part of a record that cannot be reconstructed later. (`pr-review`, "Whose reply counts"; `docs/contracts.md`, "The judgments".) diff --git a/sandbox/fixture/evidence/comment-list.json b/sandbox/fixture/evidence/comment-list.json new file mode 100644 index 0000000..1a908f3 --- /dev/null +++ b/sandbox/fixture/evidence/comment-list.json @@ -0,0 +1,83 @@ +{ + "pr": 7, + "comments": [ + { + "pr": 7, + "id": 101, + "url": "https://github.com/example/widgets/pull/7#discussion_r101", + "author": "alice", + "authorship": "human", + "kind": "thread", + "thread": "PRRT_a", + "path": "widgets/rows.py", + "line": 12, + "created": "2026-10-02T10:00:00Z", + "body": "Please raise on an empty table here instead of returning None.\n\nIn general, a reader should fail loudly on input it cannot use: a silent None turns into a wrong number three steps later." + }, + { + "pr": 7, + "id": 102, + "url": "https://github.com/example/widgets/pull/7#discussion_r102", + "author": "bob", + "authorship": "human", + "kind": "reply", + "thread": "PRRT_a", + "path": "widgets/rows.py", + "line": 12, + "created": "2026-10-03T10:00:00Z", + "body": "Agree, changed to raise ValueError." + }, + { + "pr": 7, + "id": 103, + "url": "https://github.com/example/widgets/pull/7#discussion_r103", + "author": "alice", + "authorship": "human", + "kind": "thread", + "thread": "PRRT_103", + "path": "widgets/rows.py", + "line": 12, + "created": "2026-10-04T10:00:00Z", + "body": "Keep the timeout for this one fetch at 30 s; the upstream archive is slow on Mondays." + }, + { + "pr": 7, + "id": 104, + "url": "https://github.com/example/widgets/pull/7#discussion_r104", + "author": "carol", + "authorship": "human", + "kind": "thread", + "thread": "PRRT_104", + "path": "widgets/rows.py", + "line": 12, + "created": "2026-10-05T10:00:00Z", + "body": "Name the column `flux_w_m2`, not `flux`. Units go in the column name so nobody has to guess them." + }, + { + "pr": 7, + "id": 105, + "url": "https://github.com/example/widgets/pull/7#pullrequestreview-105", + "author": "dana", + "authorship": "human", + "kind": "review", + "thread": null, + "path": null, + "line": null, + "created": "2026-10-06T10:00:00Z", + "body": "Looks good overall. For future PRs: put the ticket key on the first line of the description, the release notes are built from it." + }, + { + "pr": 7, + "id": 106, + "url": "https://github.com/example/widgets/pull/7#issuecomment-106", + "author": "bob", + "authorship": "human", + "kind": "comment", + "thread": null, + "path": null, + "line": null, + "created": "2026-10-07T10:00:00Z", + "body": "Thanks all, merging once CI is green." + } + ] +} \ No newline at end of file diff --git a/sandbox/fixture/evidence/page-units.md b/sandbox/fixture/evidence/page-units.md new file mode 100644 index 0000000..93c0f34 --- /dev/null +++ b/sandbox/fixture/evidence/page-units.md @@ -0,0 +1,15 @@ +# Units in the widgets tables + +Last updated by the data team. + +## Decision + +Every column that holds a physical quantity carries its unit in its name, in SI, as `<quantity>_<unit>`, for example `flux_w_m2` or `wavelength_nm`. A reader never converts units silently: a column whose name lacks a unit is refused at load. + +## Why + +Two releases shipped irradiance in the wrong unit because the loader guessed. Naming the unit makes the guess unnecessary and a review can see it. + +## Scope + +The loaders in `widgets/` and every table they write. Plots and notebooks are out of scope. diff --git a/sandbox/fixture/evidence/ratchet-2026-10.md b/sandbox/fixture/evidence/ratchet-2026-10.md new file mode 100644 index 0000000..9f83fc3 --- /dev/null +++ b/sandbox/fixture/evidence/ratchet-2026-10.md @@ -0,0 +1,25 @@ +window: 2026-10-01..2026-10-31 + +# Standards revision · widgets · 2026-10 + +## Head +standard: this repository's standards/ +records: 1 harvested · 0 missing · 0 baseline +guidance: 6 comments · 5 items · 4 general, 1 pr-specific · general by kind: rule 2, data-convention 1, process 1 + +## Proposals +### Rules +- [ ] R-004 · A reader raises on input it cannot use · single statement · pr-0007 + said: "In general, a reader should fail loudly on any input it cannot use" — alice, https://github.com/example/widgets/pull/7#discussion_r101 + Recommended: accept — a person's general statement, changed as asked + Mark: +### Data conventions +- [ ] F-001 · A column's name carries its units · source: carol · pr-0007 + said: "Units go in the column name so nobody has to guess them." — carol, https://github.com/example/widgets/pull/7#discussion_r104 + Recommended: accept + Mark: +### Decisions +- [ ] D-001 · A pull request description opens with its ticket key · process · pr-0007 + said: "put the ticket key on the first line of the description, the release notes are built from it." — dana, https://github.com/example/widgets/pull/7#pullrequestreview-105 + Recommended: accept + Mark: diff --git a/sandbox/fixture/evidence/slice-1.json b/sandbox/fixture/evidence/slice-1.json new file mode 100644 index 0000000..94d50d7 --- /dev/null +++ b/sandbox/fixture/evidence/slice-1.json @@ -0,0 +1,83 @@ +{ + "prs": [ + 7 + ], + "comments_read": 6, + "items": [ + { + "pr": 7, + "comment_id": 101, + "url": "https://github.com/example/widgets/pull/7#discussion_r101", + "author": "alice", + "authorship": "human", + "quote": "In general, a reader should fail loudly on input it cannot use", + "generality": "general", + "signal": "states a principle", + "kind": "rule", + "principle": "A reader raises on input it cannot use rather than returning None.", + "rationale": "a silent None turns into a wrong number three steps later.", + "covered_by": null, + "outcome": "changed as asked" + }, + { + "pr": 7, + "comment_id": 101, + "url": "https://github.com/example/widgets/pull/7#discussion_r101", + "author": "alice", + "authorship": "human", + "quote": "Please raise on an empty table here instead of returning None.", + "generality": "pr-specific", + "signal": "none", + "kind": "rule", + "principle": "Raise on an empty table in this reader.", + "rationale": null, + "covered_by": null, + "outcome": "changed as asked" + }, + { + "pr": 7, + "comment_id": 103, + "url": "https://github.com/example/widgets/pull/7#discussion_r103", + "author": "alice", + "authorship": "human", + "quote": "Keep the timeout for this one fetch at 30 s", + "generality": "general", + "signal": "states a principle", + "kind": "rule", + "principle": "Every network call in the project times out at 30 seconds.", + "rationale": "the upstream archive is slow on Mondays.", + "covered_by": null, + "outcome": "unanswered" + }, + { + "pr": 7, + "comment_id": 104, + "url": "https://github.com/example/widgets/pull/7#discussion_r104", + "author": "carol", + "authorship": "human", + "quote": "Units go in the column name so nobody has to guess them.", + "generality": "general", + "signal": "states a principle", + "kind": "data-convention", + "principle": "A column's name carries its units.", + "rationale": "so nobody has to guess them.", + "covered_by": null, + "outcome": "changed as asked" + }, + { + "pr": 7, + "comment_id": 105, + "url": "https://github.com/example/widgets/pull/7#pullrequestreview-105", + "author": "dana", + "authorship": "human", + "quote": "For future PRs: put the ticket key on the first line of the description", + "generality": "general", + "signal": "names a habit", + "kind": "process", + "principle": "A pull request description opens with its ticket key.", + "rationale": "the release notes are built from it.", + "covered_by": null, + "outcome": "unanswered" + } + ] +} \ No newline at end of file diff --git a/sandbox/scenarios.yaml b/sandbox/scenarios.yaml index 69341bd..e4ed64e 100644 --- a/sandbox/scenarios.yaml +++ b/sandbox/scenarios.yaml @@ -153,6 +153,68 @@ scenarios: files: [".review/harvest/threads/pr-0007/pr.json", ".review/harvest/guidance/slice-1.json"] says: ['(?i)floor|Write the counts only'] + - id: revise-prepare-evidence + repo: widgets + setup: + - mkdir -p standards/log/widgets .review/harvest/threads/pr-0007 .review/harvest/guidance + - cp "$SBX_FIXTURES/standard-review-rules.md" standards/review-rules.md + - cp "$SBX_FIXTURES/evidence/comment-list.json" .review/harvest/threads/pr-0007/ + - cp "$SBX_FIXTURES/evidence/slice-1.json" .review/harvest/guidance/ + - cp "$SBX_FIXTURES/evidence/ratchet-2026-10.md" standards/log/widgets/ + origin: "#88" + why: >- + Runs revise prepare's two evidence checks on a window already loaded and a report already + drafted: the spot check of the extraction, where one of five items calls a one-fetch timeout + a project-wide rule, and the quote check, where one `said:` line adds a word the comment does + not say. The run should mark that item wrong, put the tally's `spot check:` line in the Head, + and copy the quote again from the comment until the check passes. + turns: + - >- + /lasp-dev:revise prepare. The window 2026-10-01..2026-10-31 is already loaded into + .review/harvest/ and its report drafted at standards/log/widgets/ratchet-2026-10.md. Run + only step 2's spot check of the extraction and step 10's quote check on that report, then + stop before the commit question. + must_not_run: ["mcp__*", "Agent", "Task", "Bash(git push*)", "Bash(git commit*)", "Bash(claude *)"] + look_at: >- + spot_check.py sample prints all five items; the run reads each against its comment and + writes .review/revision/spot-check.md with guidance/slice-1#3 wrong (the comment is about one + fetch, the item says every network call); tally prints `spot check: 5 items · 1 wrong · error + rate 20%` or near it, which goes into the Head. quote_check.py refuses line 13; the run copies + alice's words again ("on input", not "on any input") and reruns it to `quotes: 3`. + verify: + - "grep -qE '^guidance/slice-1#3 · wrong' .review/revision/spot-check.md" + - "grep -qE '^spot check: 5 items · [0-9]+ wrong · error rate [0-9]+%' standards/log/widgets/ratchet-2026-10.md" + - "! grep -q 'on any input' standards/log/widgets/ratchet-2026-10.md" + - "grep -q 'Units go in the column name so nobody has to guess them' standards/log/widgets/ratchet-2026-10.md" + expect: + ends: success + files: [".review/revision/spot-check.md"] + + - id: source-reader-page-excerpt + repo: widgets + setup: + - mkdir -p .review/pages + - cp "$SBX_FIXTURES/evidence/page-units.md" .review/pages/units.md + origin: "#88" + why: >- + Runs source-reader's `page` task on a page given as a file, so its block gives the version + and update lines the file cannot supply, one paragraph on what the page decides and what it + constrains, and one or two `Excerpt:` lines, each the page's own words, that bear the + paragraph out. + turns: + - >- + Run the lasp-dev:source-reader agent with the task `page .review/pages/units.md`, and show + me the block it returns, verbatim. + must_not_run: ["mcp__*", "Edit", "Bash(git push*)", "Bash(git commit*)", "Bash(claude *)"] + look_at: >- + The block opens `## page`, has `Source: file .review/pages/units.md`, `Version:` reading not + in the file, a paragraph naming the unit-in-the-name decision and the loaders it constrains, + and `Excerpt:` lines quoting the page word for word, such as "carries its unit in its name". + expect: + ends: success + agents: ["source-reader"] + says: ['(?m)^\W*Excerpt: "', '(?i)not in the file'] + - id: revise-prepare-uncaptured repo: widgets setup: diff --git a/scripts/block_tally.py b/scripts/block_tally.py index 15a43df..94c1125 100644 --- a/scripts/block_tally.py +++ b/scripts/block_tally.py @@ -14,7 +14,11 @@ or the sweep's claims or merges file is missing, a pull request is named twice or not as `pr-NNNN`, or a block does not hold its form: a heading missing or naming another pull request than its file, a line of no form the block takes, a `Procedure:`, `Not read:` or `Read:` line missing or -repeated, a claim judged twice in one sweep block, or a sweep block missing a `C-n` claim it owes. +repeated, a line holding two verdicts, a claim judged twice in one sweep block, or a sweep block +missing a `C-n` claim it owes. A line holds two verdicts when an ask line carries a second `· +outcome:` or `· diff:`, or a sweep line's place or reason has a `·` field that is a verdict +word other than its own alone, or reads `judged as` one; the quoted line, which is code, is not +read for it. Stdlib only. """ @@ -51,6 +55,17 @@ r"|(?P<untouched>not touched)|(?P<unjudged>not judged · no description))$" ) TAILS = ("Procedure:", "Not read:", "Read:") +SWEEP_VERDICTS = ("follows", "breaks", "not touched", "not judged") + + +def second_verdict(word: str, text: str) -> bool: + """Whether a sweep line's own words, its place or reason, give a verdict other than its word: + a `·` field that is one alone or opens with one as a label, `breaks:` or `breaks —`, or `judged as` one, the word ending at a space, punctuation + other than `/` or `-`, a `.` not followed by a letter, or the end. A field that opens with one + in a sentence, `follows the old habit`, is prose, and `breaks/x.py` is a path.""" + others = "|".join(v for v in SWEEP_VERDICTS if v != word) + end = r"(?![\w/-]|\.\w)" + return bool(re.search(rf"(?:^|· )(?:{others})(?:\.?(?= ·|$)|:| —| -)|\bjudged as (?:{others}){end}", text, re.I)) class CannotRun(Exception): @@ -120,6 +135,8 @@ def analysis(folder: Path, prs: list[str]) -> list[str]: a = ASK.match(line) if not a: raise CannotRun(f"{pr}: not an ask line: {line}") + if line.count(" · outcome: ") > 1 or line.count(" · diff: ") > 1: + raise CannotRun(f"{pr}: two verdicts in one ask line: {line}") if a["asked"] == "none": none += 1 continue @@ -185,10 +202,12 @@ def sweep(folder: Path, prs: list[str], claims: Path | None, merges: Path | None v = VERDICT.match(line) if not v: raise CannotRun(f"{pr}: not a verdict line: {line}") + word = "follows" if v["follows"] else "breaks" if v["place"] else "not touched" if v["untouched"] else "not judged" + if any(f and second_verdict(word, f) for f in (v["follows"], v["place"], v["why"])): + raise CannotRun(f"{pr}: two verdicts in one line: {line}") if v["id"] in seen: raise CannotRun(f"{pr}: {v['id']} judged twice") seen.add(v["id"]) - word = "follows" if v["follows"] else "breaks" if v["place"] else "not touched" if v["untouched"] else "not judged" verdicts.setdefault(v["id"], Counter())[word] += 1 if word == "breaks": breaks.append((v["id"], f"break · {v['id']} · {pr} · {v['place']}")) diff --git a/scripts/quote_check.py b/scripts/quote_check.py new file mode 100644 index 0000000..3f4cc05 --- /dev/null +++ b/scripts/quote_check.py @@ -0,0 +1,153 @@ +#!/usr/bin/env python3 +"""Check every quote a revision report gives against the comment or note it cites. + +Reads the report, `ratchet-YYYY-MM.md`, and every line in it that opens `said:` once its indent is +stripped, a drafted entry's included, or holds `said: "` anywhere (`docs/contracts.md`, "The +report's quotes"). Each must be a line of its own reading `said: "<words>" — <login>, <url>`, a +comment's, or `said: "<words>" — <name>, pasted <file>`, a note the driver pasted, so a quote +nested in a list item, a table or a blockquote is refused, and its words must be 1 to 40 and +found as `scripts/guidance_check.py` finds an item's quote: for a comment, in its own words, one +run of its `body` between the lines that open `>`, never starting or ending inside a word, a run +of whitespace read as one space and nothing else forgiven; for a note, in the file's text read the +same way, every line kept. A comment is the one with that `url` in the comment lists given with +--comments, `comment-list.json` files, and `<login>` must be its `author`; a note is `<file>` under +--notes, never outside it. A URL's pull request is its repository and number, whatever page of +it the URL names. A quote from a pull request no comment list given holds is refused, unless the ledger given +with --held carries both its words and its URL under its `## Candidates below the criteria` +heading, however that section lays them out: a held candidate's quote from before the log kept +captures, which is unchecked. A line opening `Said:` in another case is refused for its form. + +Prints `quotes: <n> · from comments: <n> · from notes: <n> · unchecked: <n>`, then `unchecked · +line <n> · <url>: a held candidate's, no comment list given holds its pull request` per +unchecked quote, and exits 0. +Otherwise prints one `refused · line <n> · <why>` per fault, every fault found, and exits 1; an +input that cannot be read is one such line, `refused · <path> · <why>`. Stdlib only. +""" + +from __future__ import annotations + +import argparse +import json +import re +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) +from guidance_check import MAX_WORDS, flat, holds # noqa: E402 + +SAID = re.compile( + r'said: "(?P<quote>.+)" — (?:(?P<login>[^\s,]+), (?P<url>https?://\S+)|(?P<name>[^,]+), pasted (?P<file>.+))' +) +# A quote's opening anywhere in a line, so one nested in a list item, a table or a blockquote is +# refused for its form rather than passed over. +OPENING = re.compile(r'(?<![\w`])said: "', re.I) +# A pull request by its repository and number, whatever page of it a URL names. +PULL = re.compile(r"https?://[^/]+/([^/]+/[^/]+)/pull/(\d+)\b") +HELD = "## Candidates below the criteria" + + +def pull(url: str) -> tuple[str, int] | None: + m = PULL.match(url) + return (m[1].lower(), int(m[2])) if m else None + + +def held_text(ledger: Path | None, faults: list[str]) -> str: + """The ledger's held candidates, from its `## Candidates below the criteria` heading to its + end, its blockquote markers dropped and flattened, or empty with no ledger or no heading.""" + if ledger is None: + return "" + try: + lines = ledger.read_text().splitlines() + except OSError as e: + faults.append(f"refused · {ledger} · cannot be read: {e}") + return "" + start = next((i for i, line in enumerate(lines) if line.rstrip() == HELD), len(lines)) + return flat("\n".join(re.sub(r"^\s*(?:>\s?)+", "", line) for line in lines[start + 1 :])) + + +def found_in(text: str, quote: str) -> bool: + """Whether quote is in text, every line kept, without cutting a word.""" + words = flat(quote) + left = r"(?<!\w)" if re.match(r"\w", words) else "" + right = r"(?!\w)" if re.search(r"\w$", words) else "" + return bool(re.search(left + re.escape(words) + right, flat(text))) + + +def comments(paths: list[Path], faults: list[str]) -> dict[str, dict]: + """Every listed comment by its url.""" + by_url: dict[str, dict] = {} + for path in paths: + try: + listed = {c["url"]: c for c in json.loads(path.read_text())["comments"]} + except (OSError, json.JSONDecodeError, KeyError, TypeError) as e: + faults.append(f"refused · {path} · not a comment list: {e}") + continue + by_url.update(listed) + return by_url + + +def check(report: Path, lists: list[Path], notes: Path | None, ledger: Path | None = None) -> tuple[list[str], list[str]]: + faults: list[str] = [] + by_url = comments(lists, faults) + held = held_text(ledger, faults) + try: + lines = report.read_text().splitlines() + except OSError as e: + return [], [*faults, f"refused · {report} · cannot be read: {e}"] + listed = {pull(url) for url in by_url} + quoted = pasted = 0 + unchecked: list[str] = [] + for n, raw in enumerate(lines, 1): + line = raw.strip() + if not (line.lower().startswith("said:") or OPENING.search(line)): + continue + m = SAID.fullmatch(line) + if not m: + faults.append(f'refused · line {n} · not a line of its own `said: "<words>" — <login>, <url>` or `— <name>, pasted <file>`') + continue + quote = m["quote"] + if not 1 <= len(quote.split()) <= MAX_WORDS: + faults.append(f"refused · line {n} · {len(quote.split())} words, not 1 to {MAX_WORDS}") + continue + if m["url"]: + c = by_url.get(m["url"]) + if c is None and pull(m["url"]) not in listed and m["url"] in held and found_in(held, quote): + unchecked.append(f"unchecked · line {n} · {m['url']}: a held candidate's, no comment list given holds its pull request") + elif c is None and pull(m["url"]) not in listed: + faults.append(f"refused · line {n} · {m['url']}: no comment list given holds its pull request, and the ledger's held candidates do not carry this line") + elif c is None: + faults.append(f"refused · line {n} · {m['url']} is no comment in the lists given") + elif c["author"] != m["login"]: + faults.append(f"refused · line {n} · {m['url']} is {c['author']}'s, not {m['login']}'s") + elif not holds(c["body"], quote): + faults.append(f"refused · line {n} · the quote is not in {m['url']}'s own words") + else: + quoted += 1 + continue + path = (notes / m["file"]).resolve() if notes else None + if path is None or not path.is_file() or notes.resolve() not in path.parents: + faults.append(f"refused · line {n} · pasted {m['file']} is no file under --notes") + elif not found_in(path.read_text(), quote): + faults.append(f"refused · line {n} · the quote is not in {m['file']}") + else: + pasted += 1 + if faults: + return [], faults + head = f"quotes: {quoted + pasted} · from comments: {quoted} · from notes: {pasted} · unchecked: {len(unchecked)}" + return [head, *unchecked], [] + + +def main(argv: list[str] | None = None) -> int: + ap = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + ap.add_argument("report", type=Path) + ap.add_argument("--comments", type=Path, nargs="*", default=[], help="comment-list.json files") + ap.add_argument("--notes", type=Path, help="the directory of pasted notes") + ap.add_argument("--held", type=Path, help="the ledger, whose held candidates' said: lines may cite no list") + a = ap.parse_args(argv) + out, faults = check(a.report, a.comments, a.notes, a.held) + print("\n".join(faults or out)) + return 1 if faults else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/record.py b/scripts/record.py index c9474a0..dab20ad 100644 --- a/scripts/record.py +++ b/scripts/record.py @@ -42,7 +42,8 @@ the first of those, its standards sha `none`, and it has no findings. A finding is a thread whose first comment opens on one of the four severity lines and carries the review's attribution line; one with a `Standard` block and a citation goes under `findings:`, and one without, or with a legacy `other` key, under `threads:` with `by: review`, -its severity and its verdict read as a finding's. Every other thread, a person's or a bot's, is +its severity and its verdict read as a finding's; each carries `reviewer:`, the login of the +account that posted it, whose token ran the review. Every other thread, a person's or a bot's, is listed under `threads:` as a pointer, with the author's verdict from the first of their replies in it that starts with one of the three words, or the judgment of one with none, a Discuss settled by their next Agree or Disagree, `unanswered` when the author has not replied, on a thread @@ -84,7 +85,7 @@ JSON, exits 1 with the reason; nothing is guessed. Stdlib only, so the plugin needs no environment. -A record's first line is `contract: 14` and the --state printout's `contract: 6`: the version +A record's first line is `contract: 15` and the --state printout's `contract: 6`: the version of each interface as `docs/contracts.md` defines it. With --state it writes nothing and prints where the pull request stands: the head, then @@ -144,7 +145,7 @@ SEVERITIES = {"must fix before merge": "must-fix", "should fix": "should-fix", "suggestion": "suggestion", "question": "question"} # The versions of the --state printout and of the record, as docs/contracts.md defines them. CONTRACT = 6 -RECORD_CONTRACT = 14 +RECORD_CONTRACT = 15 # What a judgment may say a reply means, as docs/contracts.md "The judgments" defines it. JUDGED = (*VERDICTS, "answered", "unclear") # The Already checked lines a built pull request's body must carry; pr-create's body.md writes them. @@ -376,6 +377,7 @@ def parse_finding(thread: dict) -> dict | None: return { "thread": thread["id"], "severity": SEVERITIES[m.group(1).lower()] if m else "unparsed", + "reviewer": first["author"], "title": m.group(2).strip() if m else first_line(first["body"]), "label": None if m else first_line(first["body"]), "key": key, @@ -429,6 +431,7 @@ def pointer(thread: dict, pr_author: str, finding: dict | None = None, judgment: return { "thread": thread["id"], "by": by, + "reviewer": finding["reviewer"] if finding else None, "severity": finding["severity"] if finding else None, "at": f"{first['path']}:{line}" if line is not None else first["path"], "created": when(first["created_at"]), @@ -1075,6 +1078,7 @@ def build( f" severity: {f['severity']}", f" at: {q(f['at'])}", f" thread: {f['thread']}", + f" reviewer: {f['reviewer']}", f" verdict: {f['verdict'] if f['state'] == 'answered' else f['state']}", ] out += judged_lines(f.get("judged"), f.get("unclear", "")) @@ -1099,6 +1103,7 @@ def build( f" key: {q(f['key'])}", f" at: {q(f['at'])}", f" thread: {f['thread']}", + f" reviewer: {f['reviewer']}", f" a: {q(f['answer'])}" if f["state"] == "answered" else " unanswered: true", ] @@ -1107,6 +1112,7 @@ def build( out += [ f" - thread: {t['thread']}", f" by: {t['by']}", + *([f" reviewer: {t['reviewer']}"] if t["reviewer"] else []), f" authorship: {t['authorship']}", *([f" severity: {t['severity']}"] if t["severity"] else []), f" at: {q(t['at'])}", diff --git a/scripts/spot_check.py b/scripts/spot_check.py new file mode 100644 index 0000000..fe5b1d1 --- /dev/null +++ b/scripts/spot_check.py @@ -0,0 +1,111 @@ +#!/usr/bin/env python3 +"""Sample a revision's guidance items for a spot check, and count what the check found. + +`sample --seed <text> [--n 10] <slice> [...]` reads the guidance slices `scripts/guidance_check.py` +passed (`docs/contracts.md`, "The guidance items") and picks `--n` of their items at random, every +item when there are fewer, the same ones for the same seed and slices, and prints one line each, +in the slices' order: +`spot · <dir>/<slice>#<n> · pr-NNNN · <url> · <kind> · <generality> · "<quote>" · <principle>`, +`<dir>` the slice's directory name, `<slice>` its name without `.json` and `<n>` the item's place +in it from 1. + +`tally --seed <text> [--n 10] --checked <file> <slice> [...]` samples again and reads the check, one +line per sampled item, `<dir>/<slice>#<n> · right` or `<dir>/<slice>#<n> · wrong · <what>`, blank +lines ignored. It prints `spot check: <n> items · <wrong> wrong · error rate <percent>%`, the +percent rounded to a whole number, then `wrong · <id> · <what>` per wrong item, and exits 0. + +Exits 1 with one `refused · <where> · <why>` line per fault, printing nothing else: a slice that +cannot be read or holds no `items` list, an `--n` below 1, and for `tally` a line of neither form, +an item not sampled, or a sampled item with no line or with two. With no items at all, `sample` +prints nothing and `tally` prints `spot check: 0 items`. Stdlib only. +""" + +from __future__ import annotations + +import argparse +import json +import random +import re +import sys +from pathlib import Path + +LINE = re.compile(r"(?P<id>\S+#\d+) · (?:(?P<right>right)|wrong · (?P<what>\S.*))") + + +def items(slices: list[Path], faults: list[str]) -> list[tuple[str, dict]]: + out = [] + for path in slices: + try: + listed = json.loads(path.read_text())["items"] + if not isinstance(listed, list): + raise TypeError("items is not a list") + except (OSError, json.JSONDecodeError, KeyError, TypeError) as e: + faults.append(f"refused · {path} · not a guidance slice: {e}") + continue + out += [(f"{path.parent.name}/{path.stem}#{i}", item) for i, item in enumerate(listed, 1)] + return out + + +def sample(slices: list[Path], seed: str, n: int, faults: list[str]) -> list[tuple[str, dict]]: + if n < 1: + faults.append(f"refused · --n · {n} is below 1") + return [] + every = items(slices, faults) + picked = set(random.Random(seed).sample(range(len(every)), min(n, len(every)))) + return [every[i] for i in sorted(picked)] + + +def tally(picked: list[tuple[str, dict]], checked: Path, faults: list[str]) -> list[str]: + try: + lines = [line.strip() for line in checked.read_text().splitlines() if line.strip()] + except OSError as e: + faults.append(f"refused · {checked} · cannot be read: {e}") + return [] + owed = {i for i, _ in picked} + seen: dict[str, str | None] = {} + for line in lines: + m = LINE.fullmatch(line) + if not m: + faults.append(f"refused · {checked} · not `<id> · right` or `<id> · wrong · <what>`: {line}") + elif m["id"] not in owed: + faults.append(f"refused · {checked} · {m['id']} was not sampled") + elif m["id"] in seen: + faults.append(f"refused · {checked} · {m['id']} checked twice") + else: + seen[m["id"]] = m["what"] + faults += [f"refused · {checked} · {i} not checked" for i, _ in picked if i not in seen] + if not picked: + return ["spot check: 0 items"] + wrong = [(i, what) for i, what in seen.items() if what is not None] + return [ + f"spot check: {len(picked)} items · {len(wrong)} wrong · error rate {round(100 * len(wrong) / len(picked))}%", + *(f"wrong · {i} · {what}" for i, what in wrong), + ] + + +def main(argv: list[str] | None = None) -> int: + ap = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + ap.add_argument("command", choices=("sample", "tally")) + ap.add_argument("slices", type=Path, nargs="+") + ap.add_argument("--seed", required=True) + ap.add_argument("--n", type=int, default=10) + ap.add_argument("--checked", type=Path) + a = ap.parse_args(argv) + if a.command == "tally" and a.checked is None: + ap.error("tally needs --checked <file>") + faults: list[str] = [] + picked = sample(a.slices, a.seed, a.n, faults) + if a.command == "sample": + out = [ + f'spot · {i} · pr-{item["pr"]:04d} · {item["url"]} · {item["kind"]} · {item["generality"]} · "{item["quote"]}" · {item["principle"]}' + for i, item in picked + ] + else: + out = tally(picked, a.checked, faults) if not faults else [] + if faults or out: + print("\n".join(faults or out)) + return 1 if faults else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/window_cost.py b/scripts/window_cost.py new file mode 100644 index 0000000..a7a0ec4 --- /dev/null +++ b/scripts/window_cost.py @@ -0,0 +1,76 @@ +#!/usr/bin/env python3 +"""Print what a revision window's merged pull requests cost, for the report's Head. + +`window_cost.py --threads <dir>` reads every `pr-NNNN/` dump under --threads, a harvest's or a +load's (`docs/contracts.md`, "The thread dump"), whether or not the pull request has a record: its +`pr.json` and `body.md`. It prints one line, +`cost: open-to-merge median <d>d, longest <d>d · <n> merged without a build · <n> pull requests`, +each duration in days to one decimal, from `created_at` to `merged_at`; a pull request merged +without a build is one whose body has no Already checked block, as `scripts/record.py` reads it. +With no pull request it prints `cost: no merged pull request`. + +Exits 1 with one `refused · <path> · <why>` line per fault, printing nothing else: a `pr.json` that +cannot be read or lacks `created_at` or `merged_at` as an ISO 8601 UTC time, a merge before its +opening, a dump with no `pr.json` or no `body.md`, or a --threads that is not a directory, which +a load always makes. Stdlib only. +""" + +from __future__ import annotations + +import argparse +import json +import statistics +import sys +from datetime import datetime +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) +from record import already_checked # noqa: E402 + + +def when(value) -> datetime: + if not isinstance(value, str): + raise ValueError(f"{value!r} is not a time") + return datetime.strptime(value, "%Y-%m-%dT%H:%M:%SZ") + + +def main(argv: list[str] | None = None) -> int: + ap = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + ap.add_argument("--threads", type=Path, required=True) + a = ap.parse_args(argv) + if not a.threads.is_dir(): + print(f"refused · {a.threads} · not a directory") + return 1 + faults: list[str] = [] + days: list[float] = [] + unbuilt = 0 + for dump in sorted(d for d in a.threads.glob("pr-*") if d.is_dir()): + path = dump / "pr.json" + try: + pr = json.loads(path.read_text()) + opened, merged = when(pr.get("created_at")), when(pr.get("merged_at")) + except (OSError, json.JSONDecodeError, ValueError, AttributeError) as e: + faults.append(f"refused · {path} · no created_at and merged_at to read: {e}") + continue + if merged < opened: + faults.append(f"refused · {path} · merged before it was opened") + continue + body = path.parent / "body.md" + if not body.is_file(): + faults.append(f"refused · {body} · no body.md in the dump") + continue + days.append((merged - opened).total_seconds() / 86400) + unbuilt += already_checked(body.read_text()) is None + if faults: + print("\n".join(faults)) + return 1 + if not days: + print("cost: no merged pull request") + return 0 + print(f"cost: open-to-merge median {statistics.median(days):.1f}d, longest {max(days):.1f}d" + f" · {unbuilt} merged without a build · {len(days)} pull requests") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/skills/onboard/SKILL.md b/skills/onboard/SKILL.md index 9bab1d0..b0a3834 100644 --- a/skills/onboard/SKILL.md +++ b/skills/onboard/SKILL.md @@ -15,7 +15,7 @@ description: > argument-hint: "[rules [Confluence page or Jira epic to add]]" metadata: author: lasp-dev - version: 0.6.6 + version: 0.6.7 --- # Onboard diff --git a/skills/onboard/first-rules.md b/skills/onboard/first-rules.md index 198d579..737cc78 100644 --- a/skills/onboard/first-rules.md +++ b/skills/onboard/first-rules.md @@ -128,9 +128,10 @@ the share of AI-drafted threads in its own right, separate from which rules it d R-001, since a repository with no ledger has no rule ids to collide with, or, for a standard elsewhere, from its next free id. 6. Extract terms where two names mean one thing, or a name carries a unit, frame or epoch. -7. Reduce each listed page to a link, today's date and one paragraph saying what it decides and - which parts of the code it constrains, from one `source-reader` with the task `page <id>` per - page, all in one message. Copy nothing. A page the block lists as `Withheld:` gets no paragraph +7. Reduce each listed page to a link, its version, today's date and one paragraph saying what it + decides and which parts of the code it constrains, with the block's `Excerpt:` lines under it, + from one `source-reader` with the task `page <id>` per page, all in one message. Copy nothing + else. A page the block lists as `Withheld:` gets no paragraph until the person clears it; its question goes to them. 8. Run harvest.md's "The bar" on each candidate that meets admission, with the decisions files `<standard>/decisions.md` and, when this run wrote one, `.review/harvest/decisions.md`. Each diff --git a/skills/revise/SKILL.md b/skills/revise/SKILL.md index 44c8eb4..1b77af0 100644 --- a/skills/revise/SKILL.md +++ b/skills/revise/SKILL.md @@ -13,7 +13,7 @@ argument-hint: "[prepare [measures] [Confluence page or Jira epic to add] | capt disable-model-invocation: true metadata: author: lasp-dev - version: 2.24.0 + version: 2.25.0 --- # Revise the standard @@ -138,6 +138,8 @@ In this repository's `.review/`: `revision/merges.txt`, the window's merged pull requests, from `scripts/window_merges.py`; `revision/drift.txt`, the scripted claims' lines, from `scripts/drift_checks.py`; and `revision/sweep/pr-NNNN.md`, each `diff-probe` block, written by the probe. +- `revision/spot-check.md`, the guidance items `scripts/spot_check.py` sampled, ten or every item + when there are fewer, each right or wrong, from `prepare` step 2; its tally is the report's `spot check:` line. - `harvest/context-refresh.md`, from `context-refresh.md`. - `checks/`, the drafted checks for graduation candidates. - `revision/feedback/`, a `skill-feedback` draft per procedure finding, from `prepare` step 2 and diff --git a/skills/revise/context-refresh.md b/skills/revise/context-refresh.md index 9294c09..26d5399 100644 --- a/skills/revise/context-refresh.md +++ b/skills/revise/context-refresh.md @@ -14,7 +14,10 @@ entry, from the `Source:` and `Read:` lines `context/README.md` defines: `source-reader` with the task `page <id>`, one per stale entry, launched as `<plugin root>/skills/revise/harvest.md`'s "Analysts" launches agents, and draft a candidate paragraph from its block, with the new dates and version, into - `.review/harvest/context-refresh.md` beside the entry's current paragraph. + `.review/harvest/context-refresh.md` beside the entry's current paragraph. The candidate cites + the page's ID and version and carries the block's `Excerpt:` lines, so the person marking it + can check the summary against the page; a block with no excerpt gives no candidate, and its + question goes to the person. A page the block lists as `Withheld:` gets no candidate paragraph: its question goes to the person, and the entry stays as it is until they clear it. - A Jira epic: `jira_get_issue` and its `updated` field, the same way. @@ -33,7 +36,9 @@ with the previous refresh's record of it. Record every child's version in `.review/harvest/context-refresh.md` so the next refresh has a baseline; on the first, every child is listed as unseen. These are candidates for new entries, within the cap and the retirement `context/README.md` states; at the cap, a new entry's proposal names the retirement -it displaces. +it displaces. A new entry's paragraph, from such a page or the page given with `prepare`, comes +from one `source-reader` `page <id>` block, as a refresh's does, and cites the page's ID and +version and carries the block's `Excerpt:` lines. Each candidate joins the report's proposals in their form, `- [ ] context/<name> · refresh · <source> · <old date> → <new date>`, marked in the session diff --git a/skills/revise/outline.md b/skills/revise/outline.md index b2973a8..510e15a 100644 --- a/skills/revise/outline.md +++ b/skills/revise/outline.md @@ -18,10 +18,13 @@ standard: <where it is stored> scope, one line per repository covered: <repo> · <the rules that reach it> · <comments read, threads analyzed and diffs swept | comments read, archived and counted | comments read and archived, no rule reaches it> skipped: <stage> · <reason>, one line per stage `revise_state.py check` printed as skipped records: <n> harvested · <n> missing (<the Missing-record alarm Dial, when it fires>) · <n> baseline -cost: open-to-merge median <d>, longest <d> · <n> merged without a build · corrections: <n>, on <ids> -agree rate by severity · share unanswered · share of agreed findings changed as asked +cost: open-to-merge median <d>d, longest <d>d · <n> merged without a build · <n> pull requests, window_cost.py's line, one per repository +corrections: <n>, on <ids> +agree rate by severity · share unanswered · share of agreed findings changed as asked, each `<n> of <n> (<percent>%)` counted: <verdicts, firings, contested, missing, from record.py and ratchet_counts.py>; read by the analyst: <categories, outcomes, procedure ids> guidance: <n> comments · <n> items · <n> general, <n> pr-specific · general by kind: <kind> <n>, … | none read +spot check: <n> items · <n> wrong · error rate <n>% | spot check: 0 items | none: no slice passed +quotes: <n> · from comments: <n> · from notes: <n> · unchecked: <n>, quote_check.py's first line ## Overview step 10: every proposal below, the held ones included, one line each | Priority | Proposal | Applies to | Impact | Rests on | Apply changes | @@ -87,9 +90,9 @@ guidance: <n> comments · <n> items · <n> general, <n> pr-specific · general b ### Rules steps 1, 3 the window's and all records' count tables and the expiry table, every row the script prints, each ledger rule included at 0 when it has no firing; contested rules and what the -disagreements shared; rules held provisional for breadth and what would release them; each guidance tally `covered` statement under the rule, decision, entry or instruction file it names, with its quote; each agent guidance entry seen again, with its new `seen` month +disagreements shared; rules held provisional for breadth and what would release them; each guidance tally `covered` statement under the rule, decision, entry or instruction file it names, with its `said:` line; each agent guidance entry seen again, with its new `seen` month -### Held candidates step 6: each with its quotes and the criterion it fails +### Held candidates step 6: each with its `said:` lines and the criterion it fails ### What review asked for step 2 ### Practice step 2's sweep, report only: per claim, judged on, follows, breaks, untouched; drift; missed by review; rules not swept ### About the procedure step 2: each skill-feedback draft's path and ask; each settled finding and what settled it @@ -99,6 +102,8 @@ disagreements shared; rules held provisional for breadth and what would release ### Not read step 2: unanalysed pull requests, Not read: ids, unread guidance slices ``` -A held proposal is never recommended a plain `accept`. The meeting takes the proposals in the +Every quote in the report, a drafted entry's included, is a `said:` line of the form above, with +the comment's URL or the pasted note's file, so `scripts/quote_check.py` can find it; a person's +words given any other way are paraphrase. A held proposal is never recommended a plain `accept`. The meeting takes the proposals in the overview's order and stops when its time is up; a proposal it leaves blank is deferred, and `apply` returns a blank admission to the held candidates with its evidence. diff --git a/skills/revise/prepare.md b/skills/revise/prepare.md index 26743c7..82da15f 100644 --- a/skills/revise/prepare.md +++ b/skills/revise/prepare.md @@ -105,7 +105,7 @@ record `sources`. before its pointer merged (`revise`'s "The standard's repository"), and the records the script names as predating `merged:`, the corrections and notes as counts, never evidence, and the cost: median and longest open-to-merge time of the window's pull requests, and the count merged without a build, whose body has no Already checked - block. + block, the `cost:` line `python3 "<plugin root>/scripts/window_cost.py" --threads <out>/threads` prints for each repository, as it prints it. 2. **Read the threads.** Run harvest.md's "Analysts" on the records step 1 loaded that have no block in `.review/harvest/analysis/`, none when the capture analyzed them all, and its `block_tally.py analysis` on every record loaded; the report cites thread ids. Then run @@ -116,6 +116,20 @@ record `sources`. under "Not read" each repository's unread slices. What either did not read goes under "Not read". + **Spot-check the extraction.** Run `python3 "<plugin root>/scripts/spot_check.py" sample + --seed <the window, as the report's `window:` line gives it> <every slice the guidance checks + passed, every repository's>`, which picks ten items at random. For each line, read the comment + at its URL in its pull request's `comment-list.json`, under `.review/harvest/threads/` or + `.review/harvest/captured-<repo>/threads/`, and judge the item against it: the quote is that + comment's own words, and its generality, kind and principle say what the comment says, no + more. Write one line per item to `.review/revision/spot-check.md`, `<id> · right` or `<id> · + wrong · <what is wrong, in a few words>`, the id as the sample printed it, then run the script + with `tally` in place of `sample`, the same seed and slices, and `--checked + .review/revision/spot-check.md`. Its `spot check:` line goes into "Head" as it prints it, and + each `wrong` line under "About the procedure"; a refusal names the line to fix. You did not + extract these items, so you judge them as a reader would; with no slice passed, record none + and say so in "Head". + From the blocks, leaving out lines with `asked: none`, the counts from harvest.md's `block_tally.py analysis` printout, never counted by hand: - **What review asked for**: a table of category by outcome, with counts and a source column @@ -321,7 +335,14 @@ record `sources`. positive on `main`. 10. **Write the report.** First run `python3 "<plugin root>/scripts/revise_state.py" check`: an `unrecorded` line is a stage nobody accounted for, so run or skip it with its reason first, and - the report is not written until the check passes. Its `skipped` lines go under "Head". Then write the overview: every proposal, the held ones and the + the report is not written until the check passes. Its `skipped` lines go under "Head". Every + count and time the report gives of what happened in the window, in "Head", a table, an + overview cell or a proposal's evidence, is one a script printed in this run, copied as printed; + none is counted or summed by hand, and the steps above name the printout each comes from. A + rate or a share, such as an agree rate or the share unanswered, is worked from those printed + counts and written with them, `<n> of <n> (<percent>%)`, so a reader can check it against the + printout. The one count no script prints is the overview's files a proposal would change, which + counts the files its drafted change names, not the window. Then write the overview: every proposal, the held ones and the context refreshes among them, one line each, grouped by priority, high, medium and low, and within a group by impact: the number of merged pull requests in its evidence, then the number of files it would change, most first, both written in its Impact cell. A @@ -340,7 +361,19 @@ record `sources`. and where `apply` would change it: the standard's repository, this repository's diff, Jira through `ticket-draft` for a `ticket/`, or, for a `question/`, the standard's repository once answered. Write it as `outline.md` gives it, in the words `writing` sets out, every checkbox blank; no - diff and no archive entry. Ask, so `apply` can check the branch out: **Commit and push the + diff and no archive entry. Then check its quotes: run `python3 "<plugin + root>/scripts/quote_check.py" <the report> --comments <each comment-list.json file that exists + under .review/harvest/threads/pr-*/, .review/harvest/captured-<repo>/threads/pr-*/ and each + covered repository's <log>/capture/pr-*/, the last holding earlier windows' comments for the + held candidates> --notes .review/revision/notes --held <standard files>/review-rules.md`, + leaving out `--held` when the ledger does not exist. Each `refused` line names a `said:` line whose words + are not where it says, a quote not on a line of its own, or a quote from a pull request no list + holds whose words and URL the ledger's held candidates do not carry: copy the words again from + the comment or note it cites, or, for a held candidate's, from the ledger as it carries them, + or write it as its own `said:` line, and run it again until it passes; the report is not + offered for commit before. A quote no comment, note or held candidate holds is not the + person's, and comes out. An `unchecked` line is a held candidate's quote from a pull request no + capture holds: it stays, and the check's first line goes into "Head" as printed. Ask, so `apply` can check the branch out: **Commit and push the branch** (Recommended: the group marks it on GitHub or in any checkout), **Commit only**, naming the branch and any clone's path, **Stop**, nothing committed and any clone's path named, or **Let's discuss**. Either commit answer commits the report in `<log>` with the records and diff --git a/tests/fixtures/pr-0042/expected.yaml b/tests/fixtures/pr-0042/expected.yaml index 3c06560..bc2a560 100644 --- a/tests/fixtures/pr-0042/expected.yaml +++ b/tests/fixtures/pr-0042/expected.yaml @@ -1,4 +1,4 @@ -contract: 14 +contract: 15 pr: 42 ticket: none stack: none @@ -30,12 +30,14 @@ findings: severity: should-fix at: "widgets/loader.py:576" thread: PRRT_synthetic001 + reviewer: alice verdict: agree by: alice - key: "R-012/pyproject.toml::version" severity: should-fix at: "pyproject.toml:3" thread: PRRT_synthetic002 + reviewer: alice verdict: disagree reason: "Nothing callers rely on changes here: `ROW_LIMIT` is informational and the warning fires only on files that were already malformed, so a patch release is right. 2.4.1 is reserved for the fix queued ahead of this one." by: alice @@ -43,29 +45,34 @@ findings: severity: suggestion at: "widgets/loader.py:592" thread: PRRT_synthetic003 + reviewer: alice verdict: agree by: alice - key: "ticket/AC-2/tests/test_loader.py::test_report_short_rows_classifies_rows" severity: suggestion at: "tests/test_loader.py:328" thread: PRRT_synthetic004 + reviewer: alice verdict: discuss - key: "ticket/AC-5/widgets/loader.py::load_rows" severity: suggestion at: "widgets/loader.py:237" thread: PRRT_synthetic005 + reviewer: alice verdict: agree by: alice - key: "R-004/widgets/loader.py::load_rows" severity: suggestion at: "widgets/loader.py:203" thread: PRRT_synthetic006 + reviewer: alice verdict: agree by: alice questions: [] threads: - thread: PRRT_synthetic007 by: review + reviewer: alice authorship: ai-drafted severity: question at: "widgets/loader.py:568" diff --git a/tests/test_block_tally.py b/tests/test_block_tally.py index 4e3ab66..0fdfde3 100644 --- a/tests/test_block_tally.py +++ b/tests/test_block_tally.py @@ -239,6 +239,41 @@ def test_a_pull_request_named_twice_or_not_as_pr_nnnn_is_refused(self): def test_a_line_of_no_verdict_form_is_refused(self): self.refused("sweep", "pr-0007.md", SWEEP_7.replace("C-1 · not touched", "C-1 · breaks, judged as follows on the evidence"), "not a verdict line: C-1") + def test_a_sweep_line_with_a_second_verdict_is_refused_never_counted(self): + two = SWEEP_7.replace('the pull request changed the public loader', 'judged as follows on a second read') + self.refused("sweep", "pr-0007.md", two, "pr-0007: two verdicts in one line: C-2 · breaks") + self.refused("sweep", "pr-0007.md", SWEEP_7.replace("follows · pkg/loader.py:12", "follows · pkg/loader.py:12 · breaks"), "two verdicts in one line: R-001") + self.refused("sweep", "pr-0007.md", SWEEP_7.replace("the pull request changed the public loader", "on reflection, judged as follows."), "two verdicts in one line: C-2") + + def test_a_verdict_word_inside_a_reason_or_the_quoted_line_is_one_verdict(self): + self.write("pr-0007.md", SWEEP_7.replace("the pull request changed the public loader", "the change breaks callers that follows the old API").replace('"no CHANGELOG.md change"', '"# breaks · follows"')) + code, out, _ = self.run_it("sweep", "pr-0007") + self.assertEqual(code, 0) + self.assertIn("break · C-2 · pr-0007 · none", out) + + def test_a_reason_opening_with_its_own_verdict_or_a_path_named_like_one_is_one_verdict(self): + own = SWEEP_7.replace("the pull request changed the public loader", "Breaks the changelog rule: the loader changed") + own = own.replace("follows · pkg/loader.py:12", "follows · breaks/loader.py:12") + self.write("pr-0007.md", own) + code, out, _ = self.run_it("sweep", "pr-0007") + self.assertEqual(code, 0) + self.assertIn("claim · R-001 · judged 1 · follows 1", "\n".join(out)) + + def test_a_reason_that_uses_another_verdict_word_in_a_sentence_is_one_verdict(self): + for reason in ("follows the old silent-None habit the rule retired", "Not touched by any test, so it went unseen", + "not judged before; this one raises"): + self.write("pr-0007.md", SWEEP_7.replace("the pull request changed the public loader", reason)) + code, out, _ = self.run_it("sweep", "pr-0007") + self.assertEqual(code, 0, reason) + + def test_an_ask_line_with_a_second_outcome_is_refused(self): + two = ANALYSIS_7.replace("asked: test the failure path", "asked: test the failure path · outcome: changed as asked · diff: addresses it") + self.refused("analysis", "pr-0007.md", two, "pr-0007: two verdicts in one ask line: thread PRRT_a3") + + def test_a_second_verdict_written_as_a_label_is_refused(self): + for label in ("breaks: pkg/loader.py:3 lacks the guard", "breaks — the guard is gone"): + self.refused("sweep", "pr-0007.md", SWEEP_7.replace("follows · pkg/loader.py:12", f"follows · pkg/loader.py:12 · {label}"), "two verdicts in one line: R-001") + def test_a_claim_judged_twice_is_refused(self): self.refused("sweep", "pr-0007.md", SWEEP_7.replace("C-1 · not touched", "R-001 · not touched"), "R-001 judged twice") diff --git a/tests/test_quote_check.py b/tests/test_quote_check.py new file mode 100644 index 0000000..4cea548 --- /dev/null +++ b/tests/test_quote_check.py @@ -0,0 +1,156 @@ +"""Tests for scripts/quote_check.py.""" + +import contextlib +import importlib.util +import io +import json +import shutil +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +spec = importlib.util.spec_from_file_location("quote_check", ROOT / "scripts" / "quote_check.py") +quote_check = importlib.util.module_from_spec(spec) +spec.loader.exec_module(quote_check) + +URL = "https://github.com/o/r/pull/7#discussion_r12" +REPLY = { + "pr": 7, "id": 12, "url": URL, "author": "bob", "authorship": "human", "kind": "reply", "thread": "PRRT_a", + "path": "x.py", "line": 5, "created": "2026-10-01T11:00:00Z", + "body": "> Always validate units at the boundary.\n\nIn general, import dependencies directly: a degraded\nstate is broken.", +} +REPORT = f"""# Standards revision · r · 2026-10 + +### Rules +- [ ] R-004 · Import dependencies at module top · Recurring ask · pr-0007 + said: "import dependencies directly: a degraded state is broken." — bob, {URL} + said: "units go in the column name" — Dana Lee, pasted meeting.md + entry: + ### A-001 · Import directly + said: "In general, import dependencies directly" — bob, {URL} +""" + + +class QuoteCheck(unittest.TestCase): + def setUp(self): + self.dir = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, self.dir) + self.list = self.dir / "comment-list.json" + self.list.write_text(json.dumps({"pr": 7, "comments": [REPLY]})) + (self.dir / "notes").mkdir() + (self.dir / "notes" / "meeting.md").write_text("- Dana: units go in\nthe column name, always.\n") + + def run_it(self, report: str, *extra: str, lists: bool = True) -> tuple[int, list[str]]: + path = self.dir / "ratchet-2026-10.md" + path.write_text(report) + out = io.StringIO() + given = ["--comments", str(self.list)] if lists else [] + with contextlib.redirect_stdout(out): + code = quote_check.main([str(path), *given, "--notes", str(self.dir / "notes"), *extra]) + return code, out.getvalue().splitlines() + + def test_every_quote_found_where_it_cites_passes_with_its_counts(self): + self.assertEqual(self.run_it(REPORT), (0, ["quotes: 3 · from comments: 2 · from notes: 1 · unchecked: 0"])) + + def test_a_quote_absent_from_its_comment_fails_the_report(self): + code, out = self.run_it(REPORT.replace("a degraded state is broken.", "a degraded state is always broken.")) + self.assertEqual((code, out), (1, [f"refused · line 5 · the quote is not in {URL}'s own words"])) + + def test_words_the_comment_only_quotes_from_someone_else_fail(self): + code, out = self.run_it(REPORT.replace("In general, import dependencies directly", "Always validate units at the boundary.")) + self.assertEqual((code, out), (1, [f"refused · line 9 · the quote is not in {URL}'s own words"])) + + def test_a_quote_cut_inside_a_word_fails(self): + code, out = self.run_it(REPORT.replace('"In general, import dependencies directly"', '"In general, import dependencies direct"')) + self.assertEqual(code, 1) + self.assertIn("refused · line 9 · the quote is not in", out[0]) + + def test_every_fault_is_named_not_only_the_first(self): + bad = REPORT.replace(URL, "https://github.com/o/r/pull/7#discussion_r99", 1) + bad = bad.replace("— bob, " + URL, "— carol, " + URL) + bad = bad.replace("units go in the column name", "units go in the row name") + code, out = self.run_it(bad) + self.assertEqual(code, 1) + self.assertEqual(out, [ + "refused · line 5 · https://github.com/o/r/pull/7#discussion_r99 is no comment in the lists given", + "refused · line 6 · the quote is not in meeting.md", + f"refused · line 9 · {URL} is bob's, not carol's", + ]) + + def test_a_said_line_of_no_form_or_past_forty_words_fails(self): + code, out = self.run_it(REPORT + " said: import dependencies directly — bob\n") + self.assertEqual((code, out[-1].split(" · ")[:2]), (1, ["refused", "line 10"])) + long = " ".join(["word"] * 41) + code, out = self.run_it(REPORT + f' said: "{long}" — bob, {URL}\n') + self.assertEqual((code, out), (1, ["refused · line 10 · 41 words, not 1 to 40"])) + + def test_a_quote_nested_in_a_list_item_table_or_blockquote_is_refused_not_skipped(self): + for nested in (" - said:", "> said:", "| R-004 | said:"): + code, out = self.run_it(REPORT.replace(" said: \"units", nested + ' "units')) + self.assertEqual((code, out), (1, ["refused · line 6 · not a line of its own `said: \"<words>\" — <login>, <url>` or `— <name>, pasted <file>`"]), nested) + + def test_a_held_candidates_quote_from_a_pull_request_no_list_holds_is_unchecked(self): + old = "https://github.com/o/r/pull/3#discussion_r5" + said = f'said: "an older ask" — bob, {old}' + ledger = self.dir / "review-rules.md" + ledger.write_text(f"# Rules\n\n## Candidates below the criteria\n\n### Import directly\n{said}\n") + code, out = self.run_it(REPORT + f" {said}\n", "--held", str(ledger)) + self.assertEqual((code, out), (0, ["quotes: 3 · from comments: 2 · from notes: 1 · unchecked: 1", + f"unchecked · line 10 · {old}: a held candidate's, no comment list given holds its pull request"])) + + def test_a_held_quote_counts_however_the_ledger_lays_it_out(self): + old = "https://github.com/o/r/pull/3#discussion_r5" + ledger = self.dir / "review-rules.md" + ledger.write_text(f"## Candidates below the criteria\n- **Import directly**\n > an older\n > ask\n ({old}, bob)\n") + code, out = self.run_it(REPORT + f' said: "an older ask" — bob, {old}\n', "--held", str(ledger)) + self.assertEqual((code, out[0]), (0, "quotes: 3 · from comments: 2 · from notes: 1 · unchecked: 1")) + + def test_a_quote_from_a_pull_request_no_list_holds_is_refused_unless_held(self): + made_up = "https://github.com/o/r/pull/77#discussion_r1" + ledger = self.dir / "review-rules.md" + ledger.write_text(f'## Candidates below the criteria\n- **Other**\n said: "something else" — bob, {made_up}\n') + code, out = self.run_it(REPORT + f' said: "anything I like" — bob, {made_up}\n', "--held", str(ledger)) + self.assertEqual((code, out[-1]), (1, f"refused · line 10 · {made_up}: no comment list given holds its pull request, and the ledger's held candidates do not carry this line")) + + def test_with_no_comment_lists_every_comment_quote_is_refused(self): + code, out = self.run_it(REPORT, lists=False) + self.assertEqual(code, 1) + self.assertEqual([line.split(" · ")[1] for line in out], ["line 5", "line 9"]) + + def test_another_page_of_a_listed_pull_request_is_refused_not_unchecked(self): + files = "https://github.com/o/r/pull/7/files#r12" + code, out = self.run_it(REPORT.replace(URL, files, 1)) + self.assertEqual((code, out), (1, [f"refused · line 5 · {files} is no comment in the lists given"])) + + def test_a_pasted_file_outside_notes_or_a_said_in_another_case_is_refused(self): + (self.dir / "outside.md").write_text("units go in the column name") + code, out = self.run_it(REPORT.replace("pasted meeting.md", "pasted ../outside.md")) + self.assertEqual((code, out), (1, ["refused · line 6 · pasted ../outside.md is no file under --notes"])) + code, out = self.run_it(REPORT.replace(' said: "units', ' Said: "units')) + self.assertEqual((code, out[0].split(" · ")[:2]), (1, ["refused", "line 6"])) + + def test_a_note_named_with_a_space_can_be_cited(self): + (self.dir / "notes" / "meeting.md").rename(self.dir / "notes" / "meeting notes.md") + self.assertEqual(self.run_it(REPORT.replace("pasted meeting.md", "pasted meeting notes.md"))[0], 0) + + def test_an_unreadable_report_is_a_fault(self): + out = io.StringIO() + with contextlib.redirect_stdout(out): + code = quote_check.main([str(self.dir / "missing.md"), "--comments", str(self.list)]) + self.assertEqual(code, 1) + self.assertIn("missing.md · cannot be read", out.getvalue()) + + def test_a_pasted_note_that_is_not_under_notes_fails(self): + code, out = self.run_it(REPORT.replace("pasted meeting.md", "pasted chat.md")) + self.assertEqual((code, out), (1, ["refused · line 6 · pasted chat.md is no file under --notes"])) + + def test_an_unreadable_comment_list_is_a_fault(self): + self.list.write_text("not json") + code, out = self.run_it(REPORT) + self.assertEqual(code, 1) + self.assertTrue(out[0].startswith(f"refused · {self.list} · not a comment list")) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_record.py b/tests/test_record.py index 7a87e6f..96968a2 100644 --- a/tests/test_record.py +++ b/tests/test_record.py @@ -138,7 +138,7 @@ def test_reproduces_the_committed_record(self): def test_date_is_the_first_review_in_local_time_and_merged_the_utc_merge_day(self): lines, _ = record.build(PR, [REVIEW, {**REVIEW, "id": 2, "submitted_at": "2026-09-27T00:10:00Z"}], [], BODY, "o/r", "abc1234", issue_comments=[]) - self.assertEqual(lines[0], "contract: 14") + self.assertEqual(lines[0], "contract: 15") self.assertIn("date: 2026-09-25\nmerged: 2026-09-27\n", "\n".join(lines) + "\n") # 03:00 UTC is the evening before in Denver; the harvest's merged:>= qualifier reads UTC. lines, _ = record.build({**PR, "merged_at": "2026-10-01T03:00:00Z"}, [REVIEW], [], BODY, "o/r", "abc1234", issue_comments=[]) @@ -200,7 +200,7 @@ def test_six_findings_and_an_uncited_question_with_their_threads(self): for t in threads: self.assertIn(f"thread: {t['id']}", text) self.assertEqual(text.count(" - q: "), 0) - self.assertIn(" - thread: PRRT_synthetic007\n by: review\n authorship: ai-drafted\n severity: question\n", text) + self.assertIn(" - thread: PRRT_synthetic007\n by: review\n reviewer: alice\n authorship: ai-drafted\n severity: question\n", text) self.assertEqual(report, []) @@ -502,9 +502,22 @@ def test_a_reasonless_disagree_and_nothing_after_is_unanswered_with_the_reason(s def test_a_finding_nobody_answered_is_recorded_unanswered(self): text, report = self.run_build(thread("Should fix")) - self.assertIn(' - key: "R-001/x.py::f"\n severity: should-fix\n at: "x.py:3"\n thread: PRRT_1\n verdict: unanswered\nquestions:', text) + self.assertIn(' - key: "R-001/x.py::f"\n severity: should-fix\n at: "x.py:3"\n thread: PRRT_1\n reviewer: alice\n verdict: unanswered\nquestions:', text) self.assertEqual(report, ["unanswered: PRRT_1 R-001/x.py::f (no reply from a person)"]) + def test_each_finding_names_the_login_that_posted_it_whoever_answered(self): + cited, uncited, asked = thread("Should fix", ("bob", "Agree.")), thread("Should fix", ("bob", "Agree.")), thread("Question") + for th, n in ((cited, 1), (uncited, 2), (asked, 3)): + th["id"] = f"PRRT_{n}" + th["comments"][0]["author"] = "carol" + th.update(is_resolved=False, is_outdated=False) + uncited["comments"][0]["body"] = uncited["comments"][0]["body"].replace("key: `R-001/x.py::f`", "key: `other/x.py::f`") + lines, _ = record.build(PR, [REVIEW], [cited, uncited, asked], BODY, "o/r", "abc1234", issue_comments=[]) + text = "\n".join(lines) + self.assertIn(" thread: PRRT_1\n reviewer: carol\n verdict: agree\n by: bob\n", text) + self.assertIn(" - thread: PRRT_2\n by: review\n reviewer: carol\n", text) + self.assertIn(" thread: PRRT_3\n reviewer: carol\n unanswered: true", text) + def test_a_question_nobody_answered_is_kept(self): text, _ = self.run_build(thread("Question")) self.assertIn(" - q: ", text) @@ -827,11 +840,11 @@ def test_a_finding_whose_label_is_not_one_of_the_four_is_recorded_unparsed(self) lines, report = record.build(PR, [REVIEW], [self.bad_label(("bob", "Agree.")), cited], BODY, "o/r", "abc1234", issue_comments=[]) text = "\n".join(lines) self.assertIn( - ' - key: "R-001/x.py::f"\n severity: unparsed\n at: "x.py:3"\n thread: PRRT_1\n verdict: unparsed\n' + ' - key: "R-001/x.py::f"\n severity: unparsed\n at: "x.py:3"\n thread: PRRT_1\n reviewer: alice\n verdict: unparsed\n' ' line: "**Suggestion — a finding**"\n', text, ) - self.assertIn(" thread: PRRT_2\n verdict: agree\n", text) + self.assertIn(" thread: PRRT_2\n reviewer: alice\n verdict: agree\n", text) self.assertIn("adjudicated_by: bob", text) self.assertEqual(report, ["unparsed label: PRRT_1 R-001/x.py::f (first line: **Suggestion — a finding**)"]) # The rule's firing is still counted, its verdict apart from the unanswered. @@ -843,7 +856,7 @@ def test_an_uncited_finding_whose_label_is_not_one_of_the_four_is_an_unparsed_re th["comments"][0]["body"] = th["comments"][0]["body"].replace("key: `R-001/x.py::f`", "key: other/x.py::f") text, report = self.run_build(th) self.assertIn("findings: []", text) - self.assertIn(" by: review\n authorship: ai-drafted\n severity: unparsed\n", text) + self.assertIn(" by: review\n reviewer: alice\n authorship: ai-drafted\n severity: unparsed\n", text) self.assertIn(' replies: 1\n verdict: unparsed\n line: "**Suggestion — a finding**"\n', text) self.assertEqual(report, ["unparsed label: PRRT_1 (first line: **Suggestion — a finding**)"]) @@ -889,7 +902,7 @@ def test_an_uncited_finding_agreed_by_a_reviewer_is_a_review_thread(self): self.assertIn("findings: []", text) self.assertEqual(report, []) self.assertIn( - " - thread: PRRT_1\n by: review\n authorship: ai-drafted\n severity: should-fix\n at: \"x.py:3\"\n", text + " - thread: PRRT_1\n by: review\n reviewer: alice\n authorship: ai-drafted\n severity: should-fix\n at: \"x.py:3\"\n", text ) self.assertIn(" outdated: true\n replies: 1\n verdict: agree", text) row = record.state({**PR, "head": {"ref": "b", "sha": "c" * 40}}, [REVIEW], [th], BODY, "o/r")[-1].split(" | ") @@ -908,7 +921,7 @@ def test_a_legacy_other_key_is_uncited(self): th |= {"is_resolved": True, "is_outdated": False} text, _ = self.run_build(th) self.assertIn("findings: []", text) - self.assertIn(" by: review\n authorship: ai-drafted\n severity: suggestion\n", text) + self.assertIn(" by: review\n reviewer: alice\n authorship: ai-drafted\n severity: suggestion\n", text) self.assertNotIn("other/", text) diff --git a/tests/test_sandbox.py b/tests/test_sandbox.py index 754f3bb..bb4c386 100644 --- a/tests/test_sandbox.py +++ b/tests/test_sandbox.py @@ -366,7 +366,7 @@ def test_a_line_separator_inside_a_string_is_not_a_line_break(self): def test_every_generic_scenario_is_well_formed_and_runs_in_the_fixture(self): known = run.scenarios() - self.assertEqual(len(known), 32) + self.assertEqual(len(known), 34) well_formed(self, known, {"widgets"}) def test_every_fixture_a_scenario_copies_is_in_the_pack(self): diff --git a/tests/test_spot_check.py b/tests/test_spot_check.py new file mode 100644 index 0000000..acce171 --- /dev/null +++ b/tests/test_spot_check.py @@ -0,0 +1,101 @@ +"""Tests for scripts/spot_check.py.""" + +import contextlib +import importlib.util +import io +import json +import shutil +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +spec = importlib.util.spec_from_file_location("spot_check", ROOT / "scripts" / "spot_check.py") +spot_check = importlib.util.module_from_spec(spec) +spec.loader.exec_module(spot_check) + + +def item(i: int) -> dict: + return {"pr": 7, "url": f"https://github.com/o/r/pull/7#discussion_r{i}", "kind": "rule", "generality": "general", + "quote": f"quote {i}", "principle": f"principle {i}"} + + +class SpotCheck(unittest.TestCase): + def setUp(self): + self.dir = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, self.dir) + (self.dir / "guidance").mkdir() + self.slices = [] + for s, n in ((1, 8), (2, 6)): + path = self.dir / "guidance" / f"slice-{s}.json" + path.write_text(json.dumps({"prs": [7], "comments_read": 1, "items": [item(100 * s + i) for i in range(n)]})) + self.slices.append(str(path)) + + def run_it(self, *args: str) -> tuple[int, list[str]]: + out = io.StringIO() + with contextlib.redirect_stdout(out): + code = spot_check.main([*args, *self.slices]) + return code, out.getvalue().splitlines() + + def sampled(self, seed: str = "2026-10-01..2026-10-31") -> list[str]: + code, out = self.run_it("sample", "--seed", seed) + self.assertEqual(code, 0) + return [line.split(" · ")[1] for line in out] + + def test_ten_items_the_same_for_the_same_seed_each_with_what_to_check(self): + code, out = self.run_it("sample", "--seed", "w") + self.assertEqual(code, 0) + self.assertEqual(len(out), 10) + self.assertEqual(self.sampled("w"), [line.split(" · ")[1] for line in out]) + self.assertNotEqual(self.sampled("w"), self.sampled("v")) + self.assertRegex(out[0], r'^spot · guidance/slice-\d#\d+ · pr-0007 · https://\S+ · rule · general · "quote \d+" · principle \d+$') + + def test_fewer_items_than_asked_are_all_sampled(self): + code, out = self.run_it("sample", "--seed", "w", "--n", "20") + self.assertEqual((code, len(out)), (0, 14)) + + def test_the_tally_counts_wrong_items_into_an_error_rate(self): + ids = self.sampled("w") + checked = self.dir / "spot-check.md" + checked.write_text("\n".join([f"{ids[0]} · wrong · the principle says more than the quote", "", *(f"{i} · right" for i in ids[1:])])) + code, out = self.run_it("tally", "--seed", "w", "--checked", str(checked)) + self.assertEqual((code, out), (0, ["spot check: 10 items · 1 wrong · error rate 10%", + f"wrong · {ids[0]} · the principle says more than the quote"])) + + def test_an_item_unchecked_checked_twice_or_not_sampled_is_refused(self): + ids = self.sampled("w") + every = {f"guidance/slice-{s}#{i}" for s, n in ((1, 8), (2, 6)) for i in range(1, n + 1)} + stray = sorted(every - set(ids))[0] + checked = self.dir / "spot-check.md" + checked.write_text("\n".join([*(f"{i} · right" for i in ids[1:]), f"{ids[1]} · right", f"{stray} · right", "looks fine"])) + code, out = self.run_it("tally", "--seed", "w", "--checked", str(checked)) + self.assertEqual(code, 1) + self.assertEqual(out, [ + f"refused · {checked} · {ids[1]} checked twice", + f"refused · {checked} · {stray} was not sampled", + f"refused · {checked} · not `<id> · right` or `<id> · wrong · <what>`: looks fine", + f"refused · {checked} · {ids[0]} not checked", + ]) + + def test_n_below_one_is_refused_and_tally_needs_its_check(self): + self.assertEqual(self.run_it("sample", "--seed", "w", "--n", "0"), (1, ["refused · --n · 0 is below 1"])) + with self.assertRaises(SystemExit), contextlib.redirect_stderr(io.StringIO()): + self.run_it("tally", "--seed", "w") + + def test_an_unreadable_slice_is_refused(self): + Path(self.slices[1]).write_text("{}") + code, out = self.run_it("sample", "--seed", "w") + self.assertEqual(code, 1) + self.assertTrue(out[0].startswith(f"refused · {self.slices[1]} · not a guidance slice")) + + def test_no_items_reads_zero(self): + for s in self.slices: + Path(s).write_text(json.dumps({"prs": [7], "comments_read": 0, "items": []})) + checked = self.dir / "spot-check.md" + checked.write_text("") + self.assertEqual(self.run_it("sample", "--seed", "w"), (0, [])) + self.assertEqual(self.run_it("tally", "--seed", "w", "--checked", str(checked)), (0, ["spot check: 0 items"])) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_window_cost.py b/tests/test_window_cost.py new file mode 100644 index 0000000..e5bc3c9 --- /dev/null +++ b/tests/test_window_cost.py @@ -0,0 +1,71 @@ +"""Tests for scripts/window_cost.py.""" + +import contextlib +import importlib.util +import io +import json +import shutil +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +spec = importlib.util.spec_from_file_location("window_cost", ROOT / "scripts" / "window_cost.py") +window_cost = importlib.util.module_from_spec(spec) +spec.loader.exec_module(window_cost) + + +class WindowCost(unittest.TestCase): + def setUp(self): + self.dir = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, self.dir) + for n, opened, merged, built in ((7, "2026-10-01T00:00:00Z", "2026-10-02T12:00:00Z", False), + (8, "2026-10-01T00:00:00Z", "2026-10-01T12:00:00Z", True), + (9, "2026-10-01T00:00:00Z", "2026-10-05T00:00:00Z", False)): + self.add(n, opened, merged, built) + + def add(self, n, opened, merged, built): + """A dump as a harvest leaves it, record or none: an unreviewed pull request has no record.""" + dump = self.dir / "threads" / f"pr-{n:04d}" + dump.mkdir(parents=True, exist_ok=True) + (dump / "pr.json").write_text(json.dumps({"created_at": opened, "merged_at": merged})) + if built is not None: + block = "<details><summary>Already checked</summary>\n\nBuild: passed\n</details>\n" if built else "" + (dump / "body.md").write_text(f"Ticket: none\n\n{block}") + + def run_it(self) -> tuple[int, list[str]]: + out = io.StringIO() + with contextlib.redirect_stdout(out): + code = window_cost.main(["--threads", str(self.dir / "threads")]) + return code, out.getvalue().splitlines() + + def test_the_median_and_longest_open_to_merge_and_the_unbuilt_count(self): + self.assertEqual(self.run_it(), (0, ["cost: open-to-merge median 1.5d, longest 4.0d · 2 merged without a build · 3 pull requests"])) + + def test_a_dump_with_no_body_or_no_merge_time_is_refused(self): + self.add(10, "2026-10-01T00:00:00Z", "2026-10-02T00:00:00Z", None) + self.add(11, "2026-10-01T00:00:00Z", None, False) + self.add(12, "2026-10-03T00:00:00Z", "2026-10-02T00:00:00Z", False) + code, out = self.run_it() + self.assertEqual(code, 1) + self.assertEqual([line.split(" · ")[-1].split(":")[0] for line in out], + ["no body.md in the dump", "no created_at and merged_at to read", "merged before it was opened"]) + + def test_a_threads_directory_missing_or_a_dump_with_no_pr_json_is_refused(self): + out = io.StringIO() + with contextlib.redirect_stdout(out): + code = window_cost.main(["--threads", str(self.dir / "captured-gadgets" / "threads")]) + self.assertEqual((code, out.getvalue().split(" · ")[-1].strip()), (1, "not a directory")) + (self.dir / "threads" / "pr-0013").mkdir() + code, lines = self.run_it() + self.assertEqual((code, len(lines)), (1, 1)) + self.assertIn("pr-0013/pr.json", lines[0]) + + def test_no_pull_request_says_so(self): + shutil.rmtree(self.dir / "threads") + (self.dir / "threads").mkdir() + self.assertEqual(self.run_it(), (0, ["cost: no merged pull request"])) + + +if __name__ == "__main__": + unittest.main()