diff --git a/docs/development/agent-experience-audit.md b/docs/development/agent-experience-audit.md index 0e7274be2..78a057b8a 100644 --- a/docs/development/agent-experience-audit.md +++ b/docs/development/agent-experience-audit.md @@ -2616,3 +2616,149 @@ measured intent there is silence, the kernel's reading is leakage. semantics change to a load-bearing invariant and needs Sam, not a patch. - An agent's self-audit is a detection channel. The seat found in one turn what the operator's noise measurement had misattributed for a day. + +## 51. "Has anyone reviewed this head?" has two disjoint answers, and the one an agent reaches for first cannot name a commit (2026-08-29, pod-architect, caught by a peer) + +**The surface:** a pull request's review state, read from an agent. GitHub +has two collections an agent reaches for when asking it, and `gh` projects both: + +| what you call | what it returns | carries `commit_id`? | +|---|---|---| +| `gh pr view N --json comments` · `/issues/:n/comments` | issue comments | **no** | +| `gh pr view N --json reviews` · `/pulls/:n/reviews` | review events | yes | + +They are disjoint sets, not one set and a subset. `gh pr review --comment` +files a review event that never appears in the comments collection. Verified +on `#1332`: three reviews and three issue comments, zero overlap. + +**A third collection exists, and it does not change the rule.** +`/pulls/:n/comments` returns *inline* review comments, and those do carry +`commit_id`. They are not a fourth answer to hunt for: every inline comment +carries a `pull_request_review_id`, and that id is always a review event +`/pulls/:n/reviews` already returns — verified on `#1312` (4 inline comments, +all under review `5046947281`), `#1302` and `#1260`. So reading the reviews +surface still surfaces them. Two cautions, because this entry is itself about +getting a surface count wrong: the frame is "two collections that answer the +gate question, one of which cannot name a tree", not "a PR has two comment +collections" — it has three. And they are not rare here. The peer who caught +this measured zero across five of my PRs and reasonably read that as "no seat +files them"; a repo-wide sweep of `/repos/:o/:r/pulls/comments` returns them on +`#1312`, `#1302`, `#1297`, `#1274`, `#1260`, `#1176`, `#1094`, `#1022` and +further back. The five-PR sample was all docs rows; inline comments live on +code review. + +**The false model it taught.** The comments surface is the obvious one: its +name is the English word for the thing, and `--json comments` is the projection +an agent reaches for. Worse, `gh`'s human projection actively hides the split. +`gh pr view N` with no flags prints *neither* collection — verified, it stops at +the body. `gh pr view N --comments` prints **both, interleaved**, distinguishable +only by a `status:` line (`none` for an issue comment, `commented` for a review +event) and carrying no sha on either. So the flag that reads as "show me the +comments" is the one place the two are unified, and `--json comments` — the form +an agent would pick to filter them — silently returns half. On `#1338`: +`--comments` shows 2, `--json comments` returns 1. So an agent asking "has a reviewer +looked at the tree that would press?" reads it, sees nothing, and concludes +nobody has. On 2026-08-29 that produced a published claim on TASK-087: "three +hours later the PR still carries no comment after mine, so the re-gate has not +happened … a press today would be pressing a tree nobody re-read." Two review +events were sitting at that exact head, stamped 07:43:09Z and 07:54:09Z. The +warning was an argument against pressing a PR that was ready. + +**The sharper half, which is not "the comments API is incomplete."** An issue +comment carries **no `commit_id` at all** — confirmed against the raw payload's +key set, and against `gh`'s own projection, neither of which has any sha field. +So that surface cannot answer the question even in the cases where it *does* +show a gate. It can tell you somebody said something; it can never tell you +which tree they said it about. A negative from it is not a result, and a +positive from it is not one either. + +**Measured on eight open PRs the same morning, the blind spot cuts three ways:** + +- `#1268` — a review at the *current* head. A comments-only read omits a live gate. +- `#1323` — a review at `2d180528` while the head is `976e2a6f`. A comments-only + read cannot see it; a reviews read shows it is **stale**, which is the fact + that matters. +- `#1330` — **zero review events**, and yet correctly gated: the approval lives + in an issue comment whose body says "Re-gated at `a1607e89`". The gate is real + and current, and the only thing binding it to a tree is that the reviewer + *typed the sha into the prose*. Nothing queryable records it. + +That third case is the one worth keeping. It is not a gap in the record — it is +a convention doing the record's job, and it fails silently the first time +someone omits the sha, or edits the head afterwards. + +**And the check that convention forces you to build is width-sensitive.** Once +`#1330`'s gate exists only as a hand-typed sha in prose, the only way to ask +"is this PR gated at its current head?" is to search comment bodies for a +prefix of the head sha. That check has a free parameter nobody thinks about, +and getting it wrong fails in the direction that looks like an answer. Most +gate comments here write **8**-character shas (`a1607e89`, `976e2a6f`, +`2d180528`) — that is what `%h` produces — but it is a majority habit and not a +rule: 9-character gates exist too, on `#1322` and on a `#1325` comment. Run the +query at a **9**-character prefix and the issue-comment arm returns **zero on +all 12 open PRs** measured on 2026-08-29 — not one hit anywhere in the +population. At 8 the same query finds a gate at head on **9 of those 12**. A +width that is one character too long does not degrade; it silently converts the +arm into an arm that never ran, and its output is indistinguishable from "no +seat gates PRs in comments." + +**Retune nothing — delete the parameter.** The first version of this paragraph +prescribed cutting the prefix to 7, git's minimum abbreviation, "so the check +catches both conventions and any future one shorter than 8". That is +self-refuting, and a reviewer said so: `grep 'a1607e8'` does not match `a1607e`. +Seven catches conventions of length ≥7 and is defeated by anything shorter, in +exactly the way 9 was defeated by 8 — it moves the threshold and tells the next +reader the check is safe. Instead, invert the comparison: extract hex tokens +from the comment body and ask whether **the head starts with the token**. That +has no width in it at all. Verified against the same population, it finds +`a1607e89` on `#1330` and `35e4a1af` on `#1327`, and on `#1325` it catches both +`1d777174` and `1d7771748` without being told either width exists — and it +drops `#1330` from three hits to one, because the substring form was +triple-counting one sha across three comments while the token form counts +distinct shas. + +A minimum token length survives, and it is a different kind of knob: it exists +only to stop short hex strings colliding with unrelated prose and manufacturing +a gate. Getting it wrong over-reports, which someone notices; getting a *prefix +width* wrong under-reports to zero, which reads as an answer. When a parameter +cannot be removed, move it to the side that fails loudly. + +**The load-bearing half is the control, not the width.** An absence is a claim +about an instrument. Give the query a positive control: if an arm returns zero +across an entire population, prove it can return non-zero at all before +publishing the zero. That catches this whole class regardless of which width — +or which comparison — anyone picks. + +**What to do.** + +- **Read `/pulls/:n/reviews` for anything about gates.** It is the only surface + that answers "which tree" — and the question is almost always about a tree, + because an approval is a statement about a sha and not about a PR. +- **Do not treat an empty comments read as evidence of an ungated head.** State + which surface you queried, the same way you would name any other instrument + before reporting an absence. +- **When you gate a PR in an issue comment, name the sha in the body** — you are + supplying by hand the field that surface does not have. And when you *move* a + head after someone gated it, say so where they gated, because their approval + is now a claim about a tree that no longer exists. +- **Do not build a check on `gh pr view --comments`.** It is the friendliest + read and the worst instrument: it merges the two collections, labels the + difference only as `status: none` vs `status: commented`, and prints no sha + for either. It is the right thing to *read*, and never the right thing to + parse. +- **Before publishing an absence, run the positive control.** An arm that + returns zero across an entire population has to be shown capable of returning + non-zero first. This is the rule; everything below about widths is one + instance of it. +- **Do not match a sha prefix in prose at a fixed width.** Extract the hex + token and test whether the head starts with it — the comparison then has no + width to get wrong. A 9-character prefix against this repo's mostly-8 + convention returns zero on every PR and reads as "nobody gates in comments" + rather than as a broken query, and cutting to 7 only relocates that failure. +- Scope: PRs only. An issue has one comment surface and none of this applies. + +**Method note.** The correction came from the peer whose review I had just +declared missing; verifying it myself rather than accepting it is what turned +"the comments API is incomplete" into the `commit_id` asymmetry, and sweeping +the other seven PRs is what found `#1330`, where both APIs are silent and the +gate is real anyway.