From b1e6fe86834b87d9bca83c1899444c8e85bce4b3 Mon Sep 17 00:00:00 2001 From: Andy Stark Date: Fri, 18 Sep 2026 11:41:49 +0100 Subject: [PATCH 1/2] DOC-7085 Replace assess-comments ledger with repo-memory precedent lookup The hand-maintained coverage ledger (.claude/state/assess-comments.coverage.md) was a single shared file every branch's run wrote to, so it repeatedly collided on merges (documented twice in its own worked-examples log). It also duplicated by hand what /reflect + /finalize squash messages already carry. Now that the Redis repo memory GitHub Action comments on PRs with links to semantically related history, assess-comments follows those links to the real squash commit message (repo memory's own bodySummary is too shallow to use directly) and cites it as precedent during adjudication instead. The durable, cross-PR bugbot calibration axes that aren't tied to any single PR (async dispatch order, npm hoisting, list-vs-item endpoint shape) move into a static reference section in the command itself so they aren't lost. Co-Authored-By: Claude Sonnet 5 --- .claude/commands/docs/assess-comments.md | 475 ++++++++++++---------- .claude/state/assess-comments.coverage.md | 381 ----------------- 2 files changed, 260 insertions(+), 596 deletions(-) delete mode 100644 .claude/state/assess-comments.coverage.md diff --git a/.claude/commands/docs/assess-comments.md b/.claude/commands/docs/assess-comments.md index 3c3d30b84a..3c5626389c 100644 --- a/.claude/commands/docs/assess-comments.md +++ b/.claude/commands/docs/assess-comments.md @@ -4,43 +4,32 @@ argument-hint: "[commenter-login] (optional — defaults to all commenters)" --- You're going to read **all** the comments on the pull request for the current -branch — from automation (Cursor Bugbot, the security scanner, the history bot, -the AI PR summary, CI) *and* from humans — and produce a single **reconciliation -report**: what each comment raises, where the tools **agree**, where they -**contradict each other**, and where they're chasing each other in circles. This -is the synthesis layer the individual tools don't have. - -**This command assesses and reports. It does not edit PR content, commit, push, -or reply** — the one and only file it may write is its own coverage ledger -(step 11). Fixing is a separate step. Note that `/docs:bugbot` only triages and -fixes **Cursor Bugbot's own** comments — so it's the right hand-off *only* for -bugbot-sourced clusters. Safe fixes that came from humans, Codex, the security -or history bots, etc. won't be touched by `/docs:bugbot`; apply those directly -(or ask me to) once you've seen the report. - -It's also meant to be run **repeatedly** as the PR evolves: the bots tend to post -fresh comments after each round of fixes, so each run is a checkpoint — verify -the few highest-impact things deeply, leave a "story so far", and re-run to pick -up the rest. Don't try to settle the whole PR in one pass. - -The command **tracks its own test coverage** in -`.claude/state/assess-comments.coverage.md`. Read it at the start of every run -(step 1) and update it at the end (step 11) — it's how the command knows, -honestly, which of its checks have actually fired on real data and which are -still unproven (ping-pong loop detection, notably). It's a *coverage* record, -not a correctness proof. - -If `$ARGUMENTS` is given, treat it as a commenter login and assess **only** that -author's comments (match case-insensitively; `cursor` matches `cursor[bot]`). -With no arguments, assess **every** commenter — bots and humans alike, because -human intent ("this is a CTF example", "deliberate UK spelling in a quote") is -exactly what lets you adjudicate the tool findings. - -1. **Load your coverage ledger, then find the branch and its PR.** First read - `.claude/state/assess-comments.coverage.md` so you know which of your own - checks are proven vs unproven this run (it also tells you to watch hardest for - the untested ones — ping-pong loops especially). Then run - `git branch --show-current`, and: +branch — from automation (Cursor Bugbot, the security scanner, Redis repo +memory, the AI PR summary, CI) *and* from humans — and produce a single +**reconciliation report**: what each comment raises, where the tools **agree**, +where they **contradict each other**, and where they're chasing each other in +circles. This is the synthesis layer the individual tools don't have. + +**This command assesses and reports. It does not edit PR content, commit, +push, or reply, and it writes nothing to disk.** Fixing is a separate step. +Note that `/docs:bugbot` only triages and fixes **Cursor Bugbot's own** +comments — so it's the right hand-off *only* for bugbot-sourced clusters. Safe +fixes that came from humans, Codex, the security bot, etc. won't be touched by +`/docs:bugbot`; apply those directly (or ask me to) once you've seen the +report. + +It's also meant to be run **repeatedly** as the PR evolves: the bots tend to +post fresh comments after each round of fixes, so each run is a checkpoint — +verify the few highest-impact things deeply, leave a "story so far", and +re-run to pick up the rest. Don't try to settle the whole PR in one pass. + +If `$ARGUMENTS` is given, treat it as a commenter login and assess **only** +that author's comments (match case-insensitively; `cursor` matches +`cursor[bot]`). With no arguments, assess **every** commenter — bots and +humans alike, because human intent ("this is a CTF example", "deliberate UK +spelling in a quote") is exactly what lets you adjudicate the tool findings. + +1. **Find the branch and its PR.** Run `git branch --show-current`, and: ``` gh pr view --json number,url,title,headRefName,body @@ -56,7 +45,8 @@ exactly what lets you adjudicate the tool findings. ``` gh api repos/{owner}/{repo}/pulls//comments --paginate ``` - - **Top-level PR / issue comments** (summaries, bot posts, human replies): + - **Top-level PR / issue comments** (summaries, bot posts, human replies, + Redis repo memory): ``` gh api repos/{owner}/{repo}/issues//comments --paginate ``` @@ -66,18 +56,19 @@ exactly what lets you adjudicate the tool findings. ``` - **Thread resolution state** — *don't skip this.* The REST `comments` endpoint's `position`/outdated field is **not** a reliable "addressed" - signal (a resolved thread routinely still reports `position` non-null). The - authoritative signal is the GraphQL `reviewThreads`: + signal (a resolved thread routinely still reports `position` non-null). + The authoritative signal is the GraphQL `reviewThreads`: ``` gh api graphql -F query='query { repository(owner:"", name:"") { pullRequest(number:) { reviewThreads(first:100) { nodes { isResolved isOutdated comments(first:100){ nodes { databaseId author{login} path body } } } } } } }' ``` - `first:100` covers any normal PR, but unlike the `--paginate`d REST calls it - does **not** page — so if a PR genuinely has >100 review threads (or a thread - with >100 comments), check `pageInfo.hasNextPage` and follow the cursor, or - you'll silently miss resolution state and skew the open/resolved split. + `first:100` covers any normal PR, but unlike the `--paginate`d REST calls + it does **not** page — so if a PR genuinely has >100 review threads (or a + thread with >100 comments), check `pageInfo.hasNextPage` and follow the + cursor, or you'll silently miss resolution state and skew the + open/resolved split. Use `databaseId` to map each thread back to the inline comments from the REST pull, so you know which are **resolved** vs **open**. @@ -85,218 +76,272 @@ exactly what lets you adjudicate the tool findings. For each comment capture: author `login`, `created_at`, the body, and for inline comments the `path`, line, and its thread's `isResolved` / `isOutdated`. If `$ARGUMENTS` was given, it narrows which comments become - **findings you cluster and adjudicate** (steps 4–7) to the matching author — - but **don't discard the others**. Always retain every review verdict and the - full set of open findings as *context*, because step 8 (approval-over-open- - finding) needs to see approvals and bot findings together; filtering them out - would silently disable that safety check. In filtered mode, run step 8 - against the full comment set, not the filtered one. - - If no comment **or review verdict** matches `$ARGUMENTS`, stop and tell me — a - reviewer who left only an approve/request-changes verdict still counts as a - match. + **findings you cluster and adjudicate** (steps 4–7) to the matching author + — but **don't discard the others**. Always retain every review verdict and + the full set of open findings as *context*, because step 8 + (approval-over-open-finding) needs to see approvals and bot findings + together; filtering them out would silently disable that safety check. In + filtered mode, run step 8 against the full comment set, not the filtered + one. + + If no comment **or review verdict** matches `$ARGUMENTS`, stop and tell me + — a reviewer who left only an approve/request-changes verdict still counts + as a match. 3. **Tag each comment _and review verdict_ by its source role.** You're reconciling *roles*, not usernames. Reviews are first-class here: a **request-changes** (or any review whose body raises specific, actionable points) is a *finding* from its author's role, not just an approve/reject signal for step 8 — tag and carry it forward like any comment. (A bare - approval with no actionable body stays a step-8 signal only.) Classify each - author as one of: - - - **bugbot** — author `login` is `cursor` / `cursor[bot]`. **Tag by author, - not body text:** a *human* reply that quotes or mentions "Bugbot" is still a - human comment — don't tag it bugbot (that would mis-route it to - `/docs:bugbot`). Only treat a body mention of "Bugbot" as a signal when the - author is some other bot relaying bugbot's output. + approval with no actionable body stays a step-8 signal only.) Classify + each author as one of: + + - **bugbot** — author `login` is `cursor` / `cursor[bot]`. **Tag by + author, not body text:** a *human* reply that quotes or mentions + "Bugbot" is still a human comment — don't tag it bugbot (that would + mis-route it to `/docs:bugbot`). Only treat a body mention of "Bugbot" as + a signal when the author is some other bot relaying bugbot's output. - **security** — the security scanner - - **history** — the commit-history / semantic-similarity bot + - **repo-memory** — the Redis repo memory bot (`github-actions[bot]` + comment containing the `` marker and a "🧠 + Redis Memory" heading). See step 3a — this is a distinct role from + plain CI, because it's a live pointer into project history, not a + status check. - **summary** — the AI PR summary (usually the PR body or a bot comment) - - **ci** — `github-actions` / build / staging-link bots + - **ci** — `github-actions` / build / staging-link bots (everything else + posted by automation, including the Jira-link and staging-link comments) - **human** — everyone else - Treat **summary**, **ci**, and staging-build links as *context*, not - findings — they tell you what the author intended and whether the build is - healthy. Don't raise them as issues. + Treat **summary** and plain **ci** (staging links, build status) as + *context*, not findings — they tell you what the author intended and + whether the build is healthy. Don't raise them as issues. + +3a. **Mine repo memory for prior art — but only via the real squash message, + never its comment body.** Redis repo memory's comment lists related PRs + and commits ranked by similarity, each with a `sourceUrl` and a + `bodySummary`. **The `bodySummary` is not the evidence** — it's often a + one-line title echo or empty. It's a pointer, nothing more. + + For each related item whose title or file path plausibly overlaps a + cluster you're about to adjudicate (steps 4–7), follow the link and pull + the **real** message: + + - For a PR: `gh pr view --json state,mergedAt,mergeCommit` — if + merged, the squash commit at `mergeCommit.oid` carries the full + `/reflect` + `/finalize` distillation (what was learned, what was + rejected, what future-you must not break). Fetch it with + `git log -1 --format=%B ` (or `gh api + repos/{owner}/{repo}/commits/{oid} --jq .commit.message` if the commit + isn't in your local history). + - For a bare commit link: the same `git log -1 --format=%B ` — it + may be a smaller, less distilled message, so weight it accordingly. + + Don't do this for every related item indiscriminately — it costs a + network round-trip per item. Do it for the ones that could plausibly + bear on a finding you're actually adjudicating this round (same + file/subsystem, or the same bot raising a similar-shaped claim). + + Use what you find as **precedent, cited by PR number**, in step 7's + adjudication — e.g. "PR #3604's squash message documents that bugbot's + concurrency heuristic over-fires on async fire-on-call ordering; today's + finding has the same shape, so treat it as low-confidence on that + precedent" (see the calibration patterns in step 7 for the specific, + durable axes worth watching for). A precedent is supporting evidence for + your own verification, not a substitute for it — still open the current + file and check. 4. **Cluster findings by what they touch, not by who said them.** Group the - actual findings — from bugbot, security, history, humans, **and actionable + actual findings — from bugbot, security, humans, **and actionable review-verdict bodies (step 3)** — by file + line, or by shared theme when - they're not line-specific. One cluster may hold comments from several tools - pointing at the same code — that overlap is the whole point. + they're not line-specific. One cluster may hold comments from several + tools pointing at the same code — that overlap is the whole point. -5. **Split clusters into OPEN and RESOLVED — and treat them differently.** Using - the resolution state from step 2, mark each cluster open or resolved. An - active PR is almost always a mix; don't let resolved threads drown the open - ones. +5. **Split clusters into OPEN and RESOLVED — and treat them differently.** + Using the resolution state from step 2, mark each cluster open or + resolved. An active PR is almost always a mix; don't let resolved threads + drown the open ones. - **Open clusters** are your real work — they go through prioritise → adjudicate (the steps below). - **Resolved clusters** are *not* re-litigated as fresh findings. But they aren't worthless either — do two cheap, high-value things with them: - - **Fix-quality spot-check.** For a sample of resolved findings (favour the - higher-impact ones), check that the committed change *actually addressed* - the point, not just silenced the comment — e.g. if a reviewer asked to - remove a term or fix a command, confirm it's genuinely gone/correct in - the current file. Flag any "fixed in name only." - - **"Resolved ≠ fixed" flag.** Call out any thread that is **resolved but - has no corresponding code change** (resolved, `isOutdated` false, and no - intervening commit touched the line). These are often legitimate - deferrals ("leaving as-is pending eng decision before GA") — but that's a - future obligation hiding inside a green checkmark, so surface it. - **Mandatory deep-verify:** any such thread that the bot rated **High (or - Critical) severity** must be opened and verified against the current code - *this round*, regardless of the depth cap in step 6 — a serious bug must - never hide behind a green checkmark just because the run hit its budget. - If it's genuinely fixed, say so; if it's resolved-but-still-present, - that's a headline. - - Resolved threads also feed **bot calibration**: the ratio of a bot's - findings that humans accepted-and-fixed vs dismissed tells you how much to - trust that bot's next *solo* finding. Note it when it's informative. + - **Fix-quality spot-check.** For a sample of resolved findings (favour + the higher-impact ones), check that the committed change *actually + addressed* the point, not just silenced the comment — e.g. if a + reviewer asked to remove a term or fix a command, confirm it's + genuinely gone/correct in the current file. Flag any "fixed in name + only." + - **"Resolved ≠ fixed" flag.** Call out any thread that is **resolved + but has no corresponding code change** (resolved, `isOutdated` false, + and no intervening commit touched the line). These are often + legitimate deferrals ("leaving as-is pending eng decision before GA") + — but that's a future obligation hiding inside a green checkmark, so + surface it. **Mandatory deep-verify:** any such thread that the bot + rated **High (or Critical) severity** must be opened and verified + against the current code *this round*, regardless of the depth cap in + step 6 — a serious bug must never hide behind a green checkmark just + because the run hit its budget. If it's genuinely fixed, say so; if + it's resolved-but-still-present, that's a headline. + - Resolved threads also feed **bot calibration** (step 7): the ratio of a + bot's findings that humans accepted-and-fixed vs dismissed tells you how + much to trust that bot's next *solo* finding this round. Note it when + it's informative — you don't need to persist it anywhere; if it matters + again, repo memory will surface this PR next time something similar + comes up, and the squash message will carry the same ratio in context. 6. **Prioritise, and cap how deep you go.** This command is meant to be run **repeatedly** over a PR's life — the bots re-comment after each round of fixes, so a single run is never the whole story. Don't try to verify every cluster to the bottom. Rank the **open** clusters by impact — broken - `relref`/`image`/`embed-md` paths, wrong commands or code samples, incorrect - technical claims, and anything that breaks a feature the PR description calls - out, rank highest; pure style nits lowest — and pick **at most the top 3–5** - to verify in depth this round. The rest you list shallowly (see step 9) as - "not assessed this round" so the next run can pick them up. - - Three things always make the deep-verify set regardless of the cap, because - they're the failures a reconciler exists to catch: **contradictions**, - **ping-pong loops**, and **resolved-but-not-outdated High/Critical findings** - (step 5). Everything else competes for the remaining slots. - - One assessment runs *outside* the cap entirely: **subsystem churn** (step 7). - Churn is a pattern across findings — including ones you deferred this round - and ones from prior rounds — so judge it against the **full** open+recent set, - never just the capped deep set. If a deferred cluster turns out to be part of - a churn pattern, pull it into deep-verify; the cap must not hide churn. + `relref`/`image`/`embed-md` paths, wrong commands or code samples, + incorrect technical claims, and anything that breaks a feature the PR + description calls out, rank highest; pure style nits lowest — and pick **at + most the top 3–5** to verify in depth this round. The rest you list + shallowly (see step 9) as "not assessed this round" so the next run can + pick them up. + + Three things always make the deep-verify set regardless of the cap, + because they're the failures a reconciler exists to catch: + **contradictions**, **ping-pong loops**, and **resolved-but-not-outdated + High/Critical findings** (step 5). Everything else competes for the + remaining slots. + + One assessment runs *outside* the cap entirely: **subsystem churn** (step + 7). Churn is a pattern across findings — including ones you deferred this + round and ones from prior rounds — so judge it against the **full** + open+recent set, never just the capped deep set. If a deferred cluster + turns out to be part of a churn pattern, pull it into deep-verify; the cap + must not hide churn. 7. **Adjudicate the chosen clusters — and verify, don't trust.** For each - cluster in your deep set, open the file it points at (and the underlying data - it depends on) and judge for yourself; these tools have false positives, and - a single tool can be *correct about the code but wrong about impact*, or - *plausible but refuted once you read the data*. Label each: - - - **Agreement** — multiple sources flag the same thing → high confidence it's - real. - - **Contradiction** — sources disagree, *or* a finding conflicts with stated - human intent or the PR summary (e.g. security flags a sample that a human - said is a deliberate CTF example; bugbot wants a refactor the history bot - says was reverted before). Call these out loudest — they're where blind - auto-fixing does damage. - - **Ping-pong** — a genuine loop is narrower than it looks. Using `created_at` - order and the commits in between, flag it **only** when *the same concern is - reopened* (a finding you thought fixed comes back), or *a fix for tool A's - comment re-triggered tool B in a cycle* (A→fix→B→fix→A…). It is **not** - ping-pong when a fix round simply prompts a re-scan that surfaces **new, - independent findings** — even if they land in the region you just edited. - That's normal iteration; treat those as fresh clusters, not a loop. (Real - example of the non-loop case in the coverage ledger's near-miss log.) When - it *is* a true loop, flag it rather than extending it. - - **Subsystem churn** — distinct from ping-pong, and the *earlier* warning - sign. Look across fix rounds: if several findings (even new, independent - ones) keep landing on the **same area of code you keep patching**, the - symptom isn't any single finding — it's that the subsystem is - under-specified and each patch just exposes the next adjacent gap. - Incrementally fixing the latest one tends to trigger another round. When you - see this, **recommend a single consolidated redesign of that subsystem - rather than another patch** — that's the move that actually ends the churn. - (Worked example in the ledger: PR #3536's review-handling findings across - rounds 1 and 4.) - - **Solo** — a single source, no corroboration → judge it on its merits and - say how confident you are. A solo finding you've *verified against the code - and data* is high-confidence even with one source. - - **Stale / already-addressed** — the code no longer matches what the comment - describes; a later commit already handled it. - - US-vs-UK spelling counts as a real issue in this docs repo even though it's - small; pure style nits usually don't. + cluster in your deep set, open the file it points at (and the underlying + data it depends on) and judge for yourself; these tools have false + positives, and a single tool can be *correct about the code but wrong + about impact*, or *plausible but refuted once you read the data*. Label + each: + + - **Agreement** — multiple sources flag the same thing → high confidence + it's real. + - **Contradiction** — sources disagree, *or* a finding conflicts with + stated human intent or the PR summary (e.g. security flags a sample + that a human said is a deliberate CTF example). Call these out + loudest — they're where blind auto-fixing does damage. + - **Ping-pong** — a genuine loop is narrower than it looks. Using + `created_at` order and the commits in between, flag it **only** when + *the same concern is reopened* (a finding you thought fixed comes back), + or *a fix for tool A's comment re-triggered tool B in a cycle* + (A→fix→B→fix→A…). It is **not** ping-pong when a fix round simply + prompts a re-scan that surfaces **new, independent findings** — even if + they land in the region you just edited. That's normal iteration; treat + those as fresh clusters, not a loop. When it *is* a true loop, flag it + rather than extending it. If repo memory surfaces an earlier PR on the + same file, check whether its squash message already describes a loop or + a settled fixed-point — that's the fastest way to tell which one you're + in. + - **Subsystem churn** — distinct from ping-pong, and the *earlier* + warning sign. Look across fix rounds: if several findings (even new, + independent ones) keep landing on the **same area of code you keep + patching**, the symptom isn't any single finding — it's that the + subsystem is under-specified and each patch just exposes the next + adjacent gap. Incrementally fixing the latest one tends to trigger + another round. When you see this, **recommend a single consolidated + redesign of that subsystem rather than another patch** — that's the + move that actually ends the churn. If a repo-memory-linked PR's squash + message already names this subsystem's churn history, cite it — it + tells you how many rounds it took last time and what finally ended it. + - **Solo** — a single source, no corroboration → judge it on its merits + and say how confident you are. A solo finding you've *verified against + the code and data* is high-confidence even with one source. + - **Stale / already-addressed** — the code no longer matches what the + comment describes; a later commit already handled it. + + US-vs-UK spelling counts as a real issue in this docs repo even though + it's small; pure style nits usually don't. + + **Known bugbot calibration patterns** (durable across many PRs — not tied + to any one of them, so kept here rather than relying on repo memory to + resurface them): bugbot is strong on **pure language/API semantics** it + can evaluate from the source alone (e.g. `Double.MIN_VALUE` is the + smallest *positive* double, not `-Infinity`), and on **cross-file + doc-consistency / procedure-completeness** (two files in a PR that must + agree by construction). It's unreliable on **runtime mechanics invisible + in the diff**: async dispatch order (a `CompletableFuture`/`allOf` "race" + is often already ordered by call-site on one connection), npm/package + transitive-dependency hoisting (a metapackage's declared deps land + hoisted-flat, so "missing dependency" claims from import analysis alone + are often wrong), list-vs-single-item REST endpoint shape (a field present + on `GET /pulls/{n}` may be absent from the `GET /pulls` list shape bugbot + generalized from), and test-marker stripping the build performs + independently of `REMOVE` regions. It can also cite a **stale PR + description** as ground truth rather than checking whether the + description or the code is the side that's out of date. When a finding + falls in one of these blind spots, verify against the actual + runtime/build behavior before trusting it. 8. **Cross-check review verdicts against open findings.** Separately from the clusters, line up the PR's **review verdicts** (approve / request-changes) - against the findings still open. Flag the dangerous combination explicitly: - **a human approval (especially a low-confidence one — "skimmed it", "LGTM", - "didn't review in detail") sitting on top of an unresolved bot finding.** - Neither the reviewer nor the bot is wrong alone, but together they can let a - real issue merge — exactly the gap a reconciler should catch. Surface it even - when the PR is marked WIP / don't-merge. + against the findings still open. Flag the dangerous combination + explicitly: **a human approval (especially a low-confidence one — + "skimmed it", "LGTM", "didn't review in detail") sitting on top of an + unresolved bot finding.** Neither the reviewer nor the bot is wrong alone, + but together they can let a real issue merge — exactly the gap a + reconciler should catch. Surface it even when the PR is marked WIP / + don't-merge. 9. **Produce the reconciliation report.** One row per **open cluster you deep-verified this round** (step 7) — those are the only clusters with a - real verdict. Open clusters you deferred under the step-6 cap do **not** get a - table row or a verdict (you haven't verified them); they're listed, plainly - marked unverified, in the "story so far" below. The table never promises more - coverage than you actually did. + real verdict. Open clusters you deferred under the step-6 cap do **not** + get a table row or a verdict (you haven't verified them); they're listed, + plainly marked unverified, in the "story so far" below. The table never + promises more coverage than you actually did. | Cluster | File / line | Sources | Verdict | Recommended action | |---|---|---|---|---| - - **Verdict**: agreement / contradiction / ping-pong / churn / solo / stale. - - **Recommended action**: one of — *safe to fix* (uncontested and low-risk; - hand **bugbot-sourced** clusters to `/docs:bugbot`, but apply safe fixes - from any other source — human, Codex, security, history — directly, since - `/docs:bugbot` won't touch those); *needs your call* (contradiction, - approval-over-finding, or history-bot warning); *dismiss* (false positive — - say why); *already handled* (stale). + - **Verdict**: agreement / contradiction / ping-pong / churn / solo / + stale. Where a repo-memory-linked squash message informed the verdict, + cite the PR number inline (e.g. "solo, but matches #3604's precedent"). + - **Recommended action**: one of — *safe to fix* (uncontested and + low-risk; hand **bugbot-sourced** clusters to `/docs:bugbot`, but apply + safe fixes from any other source — human, Codex, security — directly, + since `/docs:bugbot` won't touch those); *needs your call* (contradiction, + approval-over-finding, or a serious repo-memory-sourced precedent); + *dismiss* (false positive — say why); *already handled* (stale). Then, in this order: - - the **headline** — contradictions, ping-pong loops, **subsystem churn (with - its consolidate-don't-patch recommendation)**, and any approval-over-open- - finding from step 8 (these need a human) first; + - the **headline** — contradictions, ping-pong loops, **subsystem churn + (with its consolidate-don't-patch recommendation)**, and any + approval-over-open-finding from step 8 (these need a human) first; - the **safe-to-fix** list; - - what you're **dismissing** and why, so I can sanity-check your reasoning; - - a short **resolved-threads** note (from step 5) — only what's worth saying: - any "fixed in name only" or "resolved ≠ fixed / deferred" flags, and a bot- - calibration line if it's informative. Don't list cleanly-resolved threads - one by one; a count is enough; - - a **"Story so far"** line: which open clusters you deep-verified this round, - how many you deferred as "not assessed this round" (list them by one-line - title), how many threads were already resolved, and a reminder that - re-running after the next round of fixes will pick up the deferred ones plus - any new comments. This is a checkpoint, not a verdict on the whole PR. - -10. **Offer a second opinion via Codex, only if it's available.** Check whether - the Codex CLI is installed — run `command -v codex`. If it is, end by - suggesting the user get an independent review of the same diff the bots saw: + - what you're **dismissing** and why, so I can sanity-check your + reasoning; + - a short **resolved-threads** note (from step 5) — only what's worth + saying: any "fixed in name only" or "resolved ≠ fixed / deferred" + flags, and a bot-calibration line if it's informative. Don't list + cleanly-resolved threads one by one; a count is enough; + - a **"Story so far"** line: which open clusters you deep-verified this + round, how many you deferred as "not assessed this round" (list them by + one-line title), how many threads were already resolved, which + repo-memory-linked PRs you actually opened and used as precedent (by + number, so I can follow the trail), and a reminder that re-running after + the next round of fixes will pick up the deferred ones plus any new + comments. This is a checkpoint, not a verdict on the whole PR. + +10. **Offer a second opinion via Codex, only if it's available.** Check + whether the Codex CLI is installed — run `command -v codex`. If it is, end + by suggesting the user get an independent review of the same diff the bots + saw: ``` /codex:review --base main --scope branch ``` - Frame it as optional and note it adds runtime, and that it's worth it mainly - for the contested or high-impact clusters — the ones where a second model - disagreeing (or agreeing) changes your confidence. Don't run it yourself: - `/codex:review` is user-invoked only, and this command is report-only. If - `command -v codex` finds nothing, say nothing about Codex. - -11. **Update your coverage ledger — automatically, but evidence-gated.** Open - `.claude/state/assess-comments.coverage.md` and update it to reflect what - *actually fired on real data this run*: - - - For each capability you genuinely exercised, bump its **encounters**, set - **last verified** to today, and add this PR (with a comment id or commit - SHA) to **Evidence**. Raise confidence per the scale — but **never raise a - confidence without a citable artifact.** No evidence, no bump. - - A capability counts as **🟢 corroborated** only on ≥2 *distinct* PRs, or - once with an independent `/codex:review` concurrence on the same finding. - - If you hit a **rare pattern for the first time** — a real ping-pong loop, a - resolved-but-still-broken thread — record its concrete signature in the - **Worked examples library** (tools involved, comment ids, commits). This is - how detection sharpens over time. - - Refresh ⏳ **decaying** entries you re-confirmed this run. - - If the run revealed that an *instruction* in this command should change - (a heuristic nearly missed something), **don't edit the command yourself** - — note the suggested refinement in your output and let me decide. - - The ledger update is the one file you *may* write. After writing it, **tell - me plainly that the command learned something and updated its own coverage - ledger**, name what changed in one line, and remind me I can include that - change in the PR or leave it out — my call. - -12. **Stop there.** Don't edit anything except the coverage ledger (step 11); - don't commit, push, or reply to the PR. Your output is the assessment — - I'll decide what to act on. + Frame it as optional and note it adds runtime, and that it's worth it + mainly for the contested or high-impact clusters — the ones where a + second model disagreeing (or agreeing) changes your confidence. Don't run + it yourself: `/codex:review` is user-invoked only, and this command is + report-only. If `command -v codex` finds nothing, say nothing about Codex. + +11. **Stop there.** Don't edit, write, or delete anything; don't commit, + push, or reply to the PR. Your output is the assessment — I'll decide + what to act on. diff --git a/.claude/state/assess-comments.coverage.md b/.claude/state/assess-comments.coverage.md deleted file mode 100644 index 6054089bb9..0000000000 --- a/.claude/state/assess-comments.coverage.md +++ /dev/null @@ -1,381 +0,0 @@ -# assess-comments — coverage ledger - -This file is the self-tracked **test coverage** of the `/docs:assess-comments` -command. It records, per capability, how confident we are that the logic has -actually *fired correctly on real data* — not how accurate its judgement is. - -**Read this honestly:** this is a *coverage* tracker, not a correctness proof. -It can only record capabilities it has *seen fire* (positive cases). It cannot -see false negatives — e.g. a ping-pong loop that was present but missed — so a -🟢 means "exercised on real PRs", never "guaranteed correct". - -The command reads this at the start of every run and updates it at the end -(step 1 and step 11). Updates are **automatic but evidence-gated**: no -confidence may rise without a citable artifact (PR number + comment id / commit -SHA). When the command updates this file it tells the user, who can choose -whether to commit the change. - -## Confidence scale -- ❓ **untested** — logic is written but has never fired on real data. -- 🟡 **seen once** — fired correctly on a single real PR. -- 🟢 **corroborated** — fired correctly on ≥2 *distinct* PRs, **or** once with an - independent `/codex:review` concurrence on the same finding. -- ⏳ **decaying** — was 🟡/🟢 but not re-confirmed in a long time (treat the API - shape, bot identities, and repo conventions as possibly drifted; re-verify). - -## Capabilities - -| Capability | Confidence | Real encounters | Last verified | Evidence | -|---|---|---|---|---| - -| Branch/PR identification + arg handling | 🟢 corroborated | 13 | 2026-07-23 | #3415, #3507, #3374, #3536, #3542, #3543, #3573, #2531, #3585, #3604, #3612, #3627, #3687 | -| Multi-source collection (inline + top-level + reviews) | 🟢 corroborated | 14 | 2026-07-23 | #3415, #3507, #3510, #3374, #3536, #3542, #3543, #3573, #2531, #3585, #3604, #3612, #3627, #3687 | -| GraphQL thread-resolution pull (`isResolved`/`isOutdated`) | 🟢 corroborated | 10 | 2026-07-10 | #3510 (12/12 resolved), #3374 (15/15), #3536 (2/2 open), #3542 (3 resolved/1 open), #3543 (1/1 open), #3573 (2/2 open), #2531 (r1: 11 open/2 res-outdated/1 open; r2: 11 of 14 outdated after fixes + APPROVED), #3585 (r1 2 open; r2 3 open + 2 resolved — one outdated, one resolved-not-outdated); #3604 (r1 2/2 open; r2 4/4 open; r3 5 threads = 2 resolved [one outdated, one resolved-not-outdated] / 3 open, hasNextPage:false); #3612 (r1 4/4 open, hasNextPage:false). **#3687: GraphQL unavailable** (no `gh` CLI in this environment — `gh api graphql` requires the `gh` CLI, which is absent; curl returned 401 because no token was found). Fell back to context-based inference (CHANGES_REQUESTED + no post-review commits → all 9 threads open). Known failure mode: when `gh` is absent, resolution state is inferred, not confirmed. | -| Source-role tagging (bugbot/security/history/summary/ci/human) | 🟢 corroborated | 12 | 2026-07-23 | #3415, #3507, #3374, #3536, #3542, #3543, #3573, #2531, #3585, #3604, #3612, #3627, #3687 (yoavredis=human CHANGES_REQUESTED; github-actions=ci Jira+staging; no bots) | -| Open/resolved split | 🟢 corroborated | 11 | 2026-07-23 | #3510, #3374, #3536 (0 resolved/2 open), #3542 (3 resolved/1 open), #3543, #3573 (r1/r2 mixed), #2531 (11 open / 3 resolved), #3585 (r1 2 open/0 res; r2 3 open/2 resolved), #3604 (r1 0 res/2 open; r2 0 res/4 open; r3 2 res/3 open), #3612 (0 res/4 open), #3627 (0 res/1 open), #3687 (9 open/0 resolved — inferred from context, GraphQL unavailable) | - -| Fix-quality spot-check (genuinely fixed vs silenced) | 🟢 corroborated | 8 | 2026-07-10 | #3510 (term removals landed), #3374 (`num_docs`, dropIndex landed), #3542 (xargs+guard, narrowed exclude, SHA pin landed), #3573 (relpath + `--add` dedup landed), #2531 (r1 found decimal thread 2619563419 reverted; r2 doc fixes landed + engineer ZdravkoDonev **APPROVED**), #3585 r2 (both r1 findings genuinely fixed — byId multimap + isError wrapping present in code, not silenced); #3604 r3 (reactive race 3544441638 resolved+outdated by 6735cf9cc — sequential `.then()` chain present + harness PASS; smismember 3545172677 resolved-not-outdated, genuinely fixed in client sources not the anchored sets.md line); #3612 (r1's 3 doc-consistency fixes + r2 commit-gap all resolved; r2 3558314964 resolved+**outdated** by 7044aefc1 — consolidation genuinely present in Step 5, not silenced; **structural de-dup confirmed**: bugbot re-scan of the collapsed manifest §5 (`03ec1e1f5`) raised nothing on it — the copy-drift class is genuinely closed, not merely silenced) | -| "Resolved ≠ fixed" flag — **legitimate deferral** variant | 🟡 seen once | 1 | 2026-06-23 | #3510 (TS.BGET:122 left pending eng) | -| "Resolved ≠ fixed" flag — **still-broken / reverted** variant | 🟡 seen once | 1 | 2026-06-30 | #2531 (resolved+outdated thread 2619563419 said decimal default=`string`; a later rewrite reverted current code to `precise`, so the resolved fix is no longer in the code — engineer re-raised it as 3496835587). Regression flavour; see worked examples | -| Cross-tool **agreement** | 🟡 seen once | 1 | 2026-06-23 | #3374 (Claude + bugbot independently on `num_docs`) | -| **Contradiction** detection | 🟢 corroborated | 5 | 2026-07-28 | #3415 (approval vs open bugbot finding); #2531 (RDI engineer's repo ground truth contradicts the page's Debezium-docs claims on ≥4 points — version, decimal default, temporal pass-through, MariaDB connector — **and** engineer-vs-existing-doc on temporal normalization); #3612 (**new flavour: internal doc-vs-doc within one PR's own files** — bugbot 3558223727: `park-manifest.md` "On pickup, then" drops labels *before* `/reflect`+`/finalize` while `pickup` Step 5 drops them *after*; `/park` copies the manifest into every PR body so the guard could lift early); #3627 (**new flavour: tool-finding vs verified-harness-reality + author design-intent** — bugbot 3587084676 "auth2 needs missing ACL user" claims the AUTH examples "break on the same environment the harness uses elsewhere"; verified `build/example-test-harness/run.sh` has **zero** `cmds_cnxmgmt` entries so the harness never runs them, and they are illustrative-by-design [matching the pre-existing shipped redis_py/node auth examples using the same `temp_pass`/`test-user` creds]. bugbot flagged only auth2 though auth1 has the identical property — a tell it's about call-shape not a runtime contract); #3714 (**new flavour: tool-finding vs harness-reality where the finding proposes the WRONG fix direction** — bugbot 3665551935 "Ruby incorrectly excluded from hexpire/hpexpire" argues redis-rb "supports HEXPIRE/HTTL/HPEXPIRE/HPTTL" so re-add it; harness proved released redis-rb 5.4.1 has NO native methods [`hexpire` → method_missing → no `FIELDS` → `ERR wrong number of arguments`], so exclusion is correct. bugbot also read the **stale PR body** [said only hexpireat omits Ruby] as ground truth. Correct reconciler move: fix the *description*, not the code). *(#3507 was an off-branch manual demo — not counted.)* | -| **Ping-pong loop** detection | ❓ untested | 0 | 2026-07-28 | still no true tool A↔B loop across #3536 (4 rounds), #3542 (r2 "empty-scope"), #3573 (r1 fixed point; r2 independent), #2531, #3585, or #3604 (r2: SMISMEMBER finding fixed by 9bf49b2f4 with no re-flag; node-dep FP independent; r3: new `Double.MIN_VALUE` finding in `SortedSetExample.java`, a file *untouched* by the 6735cf9cc push that triggered the re-scan — new-independent-in-different-file, textbook normal iteration). #3542/#3573/#3585 were churn not loops; #3585 r2 was the closest yet (the r1 fix *caused* r2's findings) but still not a loop — new independent findings + no fix undone, no A↔B cycle. #2531's nearest was the decimal regression (resolved → reverted by a rewrite → re-raised), a regression not a cycle. #3627 (r1) had a single bugbot finding on the PR's first review — no prior fix to undo, so no loop possible. #3714 (r1) same shape — single bugbot finding on first review, no prior fix to undo → no loop possible | -| **Subsystem churn** detection (repeated findings on one patched area) | 🟢 corroborated | 5 PRs | 2026-07-10 | 5 distinct PRs. #3536 — 3 instances (review-handling / churn-feature / cap↔report contract). #3542 — 2 instances on the extraction *fail-loud-on-empty* contract. #3573 — 2 instances on the `--add` virtual-merge mechanism. #3585 — get_page/search **resolution** subsystem: r1 fix (dup-id → byId multimap + matchByUrlSuffix + ambiguous-url early-return) directly caused 2 of r2's 3 findings (suffix path-boundary High, url-blocks-id-fallback) plus adjacent version-default gap. Cleanest "fix-caused-its-own-next-round" churn yet. #3612 — pickup **closing sequence** (Step 4→5), **3 rounds**: r1 label-drop ordering (3558223727) + r2 commit-before-reflect High (3558314964) + r3 push-before-merge High (3558614316). The r2 "consolidation" (explicit ordered commit→reflect→finalize→drop-labels) was **scoped too narrowly** — it threaded *commit* but not the full local→remote→squash dependency, so r3 landed on that very commit. 2nd PR to show the #3536-r6 "consolidation boundary too narrow" meta-pattern. Worked examples below | - -| Approval-over-open-finding cross-check | 🟢 corroborated | 9 | 2026-07-23 | #3415 (dwdougherty), #3374 (low-confidence over open HIGH), #3536 (high-confidence over 2 open Mediums: benign), #3542 (paoloredis "yep go ahead" 7 min after open Medium #3498159511; unacknowledged), #3573 (dwdougherty "Sure, why not?" APPROVED 13:41 over open findings; 2 bot findings landed 13:49 after), #2531 (run1 correct **negative** — no approval; run2 **positive** — ZdravkoDonev APPROVED 13:13 then bugbot finding 3499796857 landed 15:08, and he approved over 2-3 of his own still-open findings incl. the temporal one), #3585 (correct **negative** both rounds — open bugbot findings, only bot COMMENTED verdicts, no human approval); #3604 (correct **negative** all 3 rounds — r3 3 open bugbot findings, only cursor COMMENTED verdicts, no human approval); #3612 (correct **negative** — 4 open bugbot findings, only cursor COMMENTED verdict, no human approval); #3627 (correct **negative** — 1 open bugbot finding, only cursor COMMENTED verdict, no human approval); #3687 (correct **negative** — CHANGES_REQUESTED only, no human approval over open findings) | - -| Depth cap / prioritisation under load | 🟢 corroborated | 2 | 2026-07-02 | #3374 (19 candidate findings → 4 deep-verified); #2531 (r1: 14 threads → 5 deep-verified, 6 deferred). *(#3542/#3573 were under cap — not load tests)* | -| Mandatory deep-verify of resolved+not-outdated HIGH | 🟢 corroborated | 2 | 2026-07-02 | #3542 #3467309496 (High "Grep failure skips link check", resolved + isOutdated:false) — deep-verified: xargs+guard genuinely present, legitimately fixed. #3585 #3513924736 (High "Duplicate page IDs break get_page", resolved + isOutdated:false) — deep-verified against current code: byId multimap + url-first resolution present, genuinely fixed. 2nd distinct PR → corroborated | -| Bot calibration (fixed-vs-dismissed ratio) | 🟢 corroborated | 10 | 2026-07-28 | #3714 (**0 valid / 1 FP-on-facts**: bugbot 3665551935 argued redis-rb "supports HEXPIRE/HTTL/HPEXPIRE/HPTTL" so Ruby should be re-added to the hexpire/hpexpire filters — refuted by the harness [released redis-rb 5.4.1 has no native methods; method_missing omits `FIELDS` → `ERR wrong number of arguments`]. **Same calibration axis** as #3604/#3612/#3627: bugbot mis-models runtime/environment it can't see from the diff [here: released-gem API surface] and trusted the **stale PR description** as ground truth); prior: #3374 (bugbot mostly accepted); #3536 (5/5 valid); #3542 (3/3 valid — 2 fixed, 1 open); #3543 (1/1 valid; Jit 0); #3573 (4/4 valid; Jit 0); #2531 (r1 bugbot 0 findings; r2 bugbot 1/1 valid — caught the ledger duplicate-rows defect 3499796857; Jit 0); #3585 (bugbot **5/5 valid** across 2 rounds); #3604 (across 3 rounds **3 valid / 2 FP**: valid = reactive `Mono.when` race 3544441638 + SMISMEMBER client-tab parity 3545172677 [fixed by 9bf49b2f4] + `Double.MIN_VALUE`≠−∞ range bug 3550287458 [r3, a language-semantics catch]; FP = async `allOf` 3544581411 + node harness `@redis/time-series` "missing dep" 3545172685. **Calibration axis**: bugbot is strong on API/language semantics [`Double.MIN_VALUE` is smallest-positive] but both FPs mis-model runtime dependency/ordering it can't see from the diff [async fire-on-call; npm transitive hoisting]. Also found 2 of 3 identical `Double.MIN_VALUE` instances — correctly skipped the jedis one, pre-existing on main / outside the PR diff); #3612 (**8 valid / 1 FP across the review cycle** [r7 added 3559171169 rebase-force-lease + 3559171157 scan-mode-stop-rule, both pickup] — r1 valid = internal doc-vs-doc label-order contradiction 3558223727 + scan-mode wrong-step-ref 3558223733 + park update-path-only-shows-`gh pr create` 3558223738; r2 valid = commit-before-reflect High 3558314964; r3 valid = push-before-merge High 3558614316; r5 valid = manifest-omits-rebase 3558867168 [manifest copy-drift]; r6 valid = rebase-before-commit 3558994646 [pickup git-op ordering]; FP = r1 High "`merged` field doesn't exist" 3558223722, refuted empirically — single-PR REST endpoint returns top-level `merged`. **Calibration axis holds**: all 8 valid catches are cross-file doc-consistency / procedure-completeness / git-correctness [bugbot's strength], and it effectively ran the churn detection one seam per round across 7 rounds; the 1 FP is again an **unseen API mechanic** [list-endpoint vs single-PR-endpoint response shape] — same root as #3604's async/npm FPs); #3627 (**0 valid / 1 FP-on-impact**: bugbot 3587084676 correct on the literal [`AUTH test-user` errors on a bare Redis] but wrong on impact [claimed the harness runs these; it does not — no `cmds_cnxmgmt` in run.sh]. **Same calibration axis**: bugbot mis-models the runtime/environment it can't see from the diff — here that the auth set is illustrative-only and unwired from the harness, exactly the async/npm/endpoint-shape blind spot from #3604/#3612) | -| Codex second-opinion availability gate | 🟢 corroborated | 11 | 2026-07-28 | #3415, #3374 (CLI on PATH; #3374 had a real Codex review), #3542, #3543, #3573, #2531, #3585, #3604, #3612, #3627, #3714 (codex on PATH at /Users/andrew.stark/bin/codex) | -| Ledger self-integrity after `main` merge (no duplicate rows) | 🟡 seen twice | 2 | 2026-07-02 | #2531 r2 — bugbot 3499796857 caught the shared ledger gaining duplicate rows when `main` (carrying a #3573-era ledger) merged in and git kept both blocks. **2026-07-02**: merging `main` again produced a real conflict as #3542/#3573 edited the same rows — union-merged per capability. Recurring shared-file hazard; see worked examples + step-11 refinement | - -## Worked examples library - -Concrete real-world signatures, so detection sharpens over time. Add to this -whenever a rare pattern is seen for the first time. - -### Ping-pong loops -*(no true loop observed yet — when the first real one appears, record the tool -sequence, the comment ids, and the commits between them here)* - -**Near-miss (NOT a loop) — #3536, 2026-06-23.** Round-1 bugbot findings (ids -3460160413, 3460160429 on commit `17517ed`) were fixed in commit `da7c04a`; -bugbot re-reviewed and raised 3 *new* findings (ids 3460820452/470/474). One new -finding (GraphQL pagination, `assess-comments.md:72-75`) sat in the same step I'd -just edited — superficially loop-shaped. But the new findings were independent, -pre-existing issues exposed by the file changing, **not** the same spot toggled -back and forth or a fix being undone. Lesson for the detector: "fix → re-scan → -new findings in an edited region" is normal iteration, **not** ping-pong. A true -loop needs the *same concern* reopened, or tool A's fix re-triggering tool B in a -cycle. - -*Confirmed convergence (round 3):* the fixes for those 3 new findings were -pushed, and bugbot's next re-scan came back **clean — no comments**. A real loop -would have spawned another round; this settled. So the "not a loop" judgement is -borne out by what happened next: assess → fix → re-scan reached a fixed point. - -**Near-miss (NOT a loop) — #3542, 2026-07-01.** Round-1 bugbot High #3467309496 -(ARG_MAX / `|| true` silent-green) was fixed (commit `21f079e1d`: xargs + -zero-URL `exit 1` guard). Round-2 re-scan raised Medium #3498159511 ("empty scope -exits successfully") — the *sibling* zero-files `exit 0` path the new guard didn't -cover. Superficially loop-shaped (same file, same "silent green" theme), but it's -a new adjacent gap, not the same concern reopened → correctly judged churn, not -ping-pong. - -### Ledger duplication / conflict after a `main` merge (shared-file hazard) -- **#2531 — 2026-06-30, and again 2026-07-02.** The coverage ledger is a single shared - file edited by *every* assess-comments run across all branches, so it repeatedly - collides on merges. (1) 2026-06-30: a #3573 run edited it on `main` while my #2531 run - edited the branch from the same #3543 ancestor; merging `main` (commit `3007dfb3`) - kept **both** blocks, duplicating nearly every capability with conflicting confidence. - Bugbot caught it (finding `3499796857`, Low). (2) 2026-07-02: merging `main` again - produced a real git **conflict** because #3542/#3573 runs had since edited the same - rows — resolved by union-merging evidence per capability. **Suggested step-11 - refinement:** reconcile *by capability name* (one row each; merge evidence) and - re-dedupe/re-merge after any `main` merge — concurrent runs on different branches - keep colliding in this shared file. - -### Resolved-but-still-broken -*(none of the classic "resolved but bug never fixed" variant confirmed yet)* - -**Reverted-after-resolve (regression) — #2531, 2026-06-30.** Thread `2619563419` -(ZdravkoDonev-redis, Dec 2025) stated RDI's decimal default is -`debezium.source.decimal.handling.mode=string`; it was **resolved + marked -outdated**. A June 2026 full rewrite of the page (commit `43e02c1d0`) then changed -the documented default to `precise` and added a `TODO` questioning it — i.e. it -**reverted the agreed, resolved fix**. The engineer had to re-raise the identical -point as a fresh finding (`3496835587`). Signature: a *resolved+outdated* thread -whose agreed resolution is no longer reflected in current code because a later -large edit overwrote it. Lesson for the detector: when spot-checking resolved -threads, don't assume "resolved+outdated" means the fix still holds — a big rewrite -can silently undo it. This is a **regression**, distinct from both ping-pong (no -A↔B cycle) and a never-fixed thread (it *was* fixed, then un-fixed). - -### Cross-source contradiction (authoritative human vs author's own prior work / other docs) -- **#2531 RDI engineer vs Debezium-docs basis, 2026-06-30.** The page's mappings - were built from the Debezium 3.0 reference because RDI internals were non-public. - RDI engineer ZdravkoDonev then supplied repo ground truth contradicting it on - ≥4 points: shipped Debezium is `3.5.0.Final-rdi.3` not 3.0.8 (`3496835571`); - decimal default is `string` not `precise` (`3496835587`); RDI *normalizes* - temporal logical types rather than passing Debezium values through (`3496835595`); - RDI maps both `mysql` and `mariadb` to `MySqlConnector`, no separate MariaDB - connector (`3496835604`). Additionally an **doc-vs-doc** contradiction: the - engineer's temporal-normalization examples disagree with the published - `formatting-date-and-time-values.md` (Debezium `Date` → ms per engineer vs → days - per the doc). Lesson: a finding can be authoritative-human-correct yet contradict - *the author's own verified claims and another shipped doc* — the reconciler's job - is to surface that the Debezium-docs basis was the weaker source all along. - -**Internal doc-vs-doc within one PR's own files — #3612, 2026-07-10.** A new contradiction -flavour: not human-vs-doc or tool-vs-intent, but two files *in the same PR* that must stay -consistent by construction. Bugbot Medium `3558223727` caught `_shared/park-manifest.md`'s -"On pickup, then" listing *"drop the labels, run `/reflect` then `/finalize`"* (guard removed -**before** the durable commit) while `pickup/SKILL.md` Step 5 removes labels **after** reflect -+ finalize. The bite: `/park` copies the manifest section verbatim into every parked PR body, -so an agent following the PR-body copy could lift the `do not merge yet` guard before -reconciliation finishes. Signature: a shared/duplicated spec fragment whose ordering disagrees -with the authoritative procedure that consumes it. Extra irony — these very skills exist to stop -`/park` and `/pickup` drifting, and the seam bugbot found is a drift between them. Lesson for the -detector: when one PR ships a spec split across N files, diff the *duplicated* fragments against -their canonical procedure; contradictions hide in the copies. - -**Deep-verify came back CLEAN — #3542, 2026-07-01.** First real firing of the -"mandatory deep-verify of resolved+not-outdated High" rule: thread #3467309496 was -`isResolved:true, isOutdated:false` (a green checkmark on an unchanged-looking -line). Opened the current file anyway — the xargs fix + `count==0 → exit 1` guard -are genuinely present (committed `21f079e1d`). So this was resolved-*and*-fixed, -not the still-broken variant. Recorded here as the negative case that proves the -rule fires and distinguishes fixed from still-broken. - -### Subsystem churn (not a loop, but the precursor) -- **#3536 review-handling, 2026-06-23.** Bugbot finding 429 (round 1) flagged the - `$ARGUMENTS` filter dropping cross-check data; it was patched. Two rounds later - bugbot raised 862 ("filter abort ignores reviews") and 874 ("review bodies skip - clustering") — *new, independent* findings (so not ping-pong), but all three - cluster on the **same under-specified subsystem**: how review verdicts flow - through collect → filter → tag → cluster → abort. Root cause: reviews were - second-class vs comments. Signature: each incremental patch exposed the next - adjacent gap. Response that ended it: one **consolidated** fix promoting reviews - to first-class across steps 2–4, rather than patching 862/874 individually. -- **#3536 churn-on-the-churn-feature (round 5), 2026-06-23.** The *fix* for the - case above added "subsystem churn" to step 7 — but only there. Round 5 bugbot - raised 442 ("churn detection capped incorrectly" — gated behind step 6's depth - cap) and 449 ("report verdict omits churn" — missing from step 9's verdict list - and headline). Same signature as any churn: a feature added in one place, - adjacent integration gaps exposed next round. Cured by threading churn through - steps 6 and 9 in one pass. Lesson: adding a capability *is* a subsystem edit — - wire it through every dependent step at once, or it becomes its own churn. -- **#3536 churn persisted — consolidation scoped too narrowly (round 6), - 2026-06-23.** The round-5 "consolidated" fix threaded the *churn feature* - through steps 6 & 9 but left the deeper contract under them unfixed, so round 6 - raised finding 3461052859 ("open table needs full verdicts": step 9 promised a - verdict per open cluster while step 6 only verifies 3–5). Same area a third - time. Real root: the **cap ↔ report-completeness contract**, not the churn - feature. Fixed by making the table cover only deep-verified clusters and - marking deferred ones unverified. Meta-lesson: when round N+1 finds another gap - in an area you *just* "consolidated", your consolidation boundary was wrong — - widen it to the true subsystem, don't re-patch the edge. -- **#3542 extraction "fail-loud-on-empty" contract, 2026-07-01.** 2nd distinct PR - showing the pattern (promotes churn detection to 🟢). Round-1 bugbot High - #3467309496 flagged `grep "${files[@]}"` + `|| true` as a silent-green path - (extraction failure → empty output → lychee passes checking nothing). Fixed - (commit `21f079e1d`) with xargs streaming + a `count==0 → exit 1` guard — but - the guard only covered the *zero-URLs-from-found-files* path. Round-2 bugbot - Medium #3498159511 then flagged the sibling *zero-files-found* branch, which - still does `: > "$OUTPUT_FILE"; exit 0` (silent green on a broken/mis-scoped - build). Same under-specified contract ("degenerate extraction must fail loudly"), - each patch exposing the adjacent path. Signature identical to #3536. Recommended - consolidation: enforce one invariant — any empty `external-urls.txt` in CI fails - — in a single place, rather than flipping just the zero-files branch and inviting - a round-3 finding on a third degenerate path. - -- **#3573 `--add` virtual-merge, 2026-06-30 (2nd PR → corroborates the pattern).** - Round 1 bugbot #3499137529 flagged `--add` duplicating a page that's also on - disk; fixed by an existence-check in `build_rows`. Round-2 re-scan then raised - #3499226329: `--add` into a `--collapse`d folder is silently dropped (virtual - entries merge only when `walk` recurses, which collapsed folders skip). *New, - independent finding* (not ping-pong) but the **same under-specified subsystem** - — how `--add` virtual entries reconcile with the on-disk walk and collapse - state. Signature again: each point-patch exposed the next adjacent gap. - Consolidation that would end it: resolve all `--add` entries against the final - tree in one place — validate the parent is a real, non-collapsed, walked dir - and **warn instead of silently dropping**, and dedupe vs disk there too — - rather than threading a `virtual` dict through the recursion. Borderline - (only 2 instances, niche feature), but the cross-round, same-mechanism shape - is the early churn signal. - -- **#3585 get_page/search resolution, 2026-07-02 (fix-caused-its-own-next-round — the purest signature).** - Round 1 bugbot raised a High (duplicate page ids break get_page, `3513924736`) and a - Medium (missing page returns success, `3513924746`). Both were fixed in `ea1b97254`: - `byId` became a multimap, a new `matchByUrlSuffix` returned all suffix matches, an - ambiguous `url` returned an error early, and a `toolResult` wrapper set `isError`. - Round-2 re-scan then raised THREE new findings, **two of them defects created by that very - fix**: `matchByUrlSuffix` uses `endsWith` with no path boundary so `url:"get"` matches 19 - pages incl. `arget`/`config-get` (High `3514124777`, confirmed empirically); the ambiguous-url - early-return skips the `id` fallback (Medium `3514124798`); plus the adjacent pre-existing - version-default gap (Medium `3514124788`). Not ping-pong (no fix undone, no A↔B cycle) — - it's churn: the get_page/search *resolution* logic is under-specified, and each patch exposes - the next adjacent gap. Distinct from #3536/#3542/#3573 in that here the *fix itself* introduced - the next round's findings, making the "consolidate, don't patch" call unambiguous. Recommended - consolidation: one resolver with explicit handle precedence (exact url → unique id → - boundary-anchored suffix url, collecting candidates across handles), path-segment-anchored - suffix matching, and version semantics decided once. - -### Cross-tool agreement -- **#3374 `num_docs`** — Claude (Critical #2, top-level review) and bugbot - ("Wrong FT.INFO document count field", inline, 06-17) independently flagged - the same `info.numDocs` → `num_docs` bug. Verified fixed in current code. - -### Approval-over-open-finding — benign variants - -- **#3312 dwdougherty, 2026-05-12.** Review state `COMMENTED` (not `APPROVED`) - with body *"Apart from the issues already identified by Bugbot, LGTM."* sat - over 3 open bugbot findings. **Explicit carveout** — the human deliberately - excluded the bot's findings from the LGTM. Over the following months, 1 - finding (3228632453 raw-md → relref) was substantively fixed by the author and - 2 (3228632434 streaming.md empty page; 3228632444 manage-workspace.md empty - frontmatter) became *stale via the broader restructure* (streaming.md - deleted; manage-workspace.md fully populated with frontmatter). Pattern: the - approval-over-open-finding signal *did* fire, but the carveout language made - it benign and the open findings were later defanged by unrelated work. Distinct - from the dangerous variant where an "LGTM" silently rides over a real bug. - -### Bot false positives (calibration signatures) -- **#3604 async fire-on-call mis-modeled, 2026-07-08 (first adjudicated bugbot FP).** - Bugbot raised two Medium concurrency findings on new Lettuce doc examples, same - heuristic ("concurrent Redis ops on a shared key"): (a) `3544441638` reactive - `StringExample` — `Mono.when(setAndGet, setnx, setxx, mset, incrby).block()` - subscribes cold publishers ~concurrently → **REAL** interleaving race on `bike:1` - (corroborated: the sibling reactive `SortedSetExample` blocks per-step, and the - async `StringExample` uses per-step `join()` — the `Mono.when` file is the outlier); - (b) `3544581411` async `HashExample` — `CompletableFuture.allOf(delResult, hIncrBy, - incrByGetMget)` claimed to race `del bike:1:stats` against the stats increments → - **FALSE POSITIVE**: `delResult = asyncCommands.del(...)` fires *on call* (line 26), - and a single Lettuce connection executes commands in write order, so the delete - completes before `incrByGetMget`'s increments are dispatched (line ~120); `allOf` - only awaits. Corroborated by upstream `redis/lettuce` shipping the identical async - structure under CI. Lesson for the detector: bugbot's concurrency heuristic is sound - for reactive **cold-publisher** subscription (`Mono.when`) but over-fires on **async - fire-on-call** commands, which are already ordered by call site on one connection. - When a "race" finding lands on Lettuce **async** code, check dispatch order before - trusting it. -- **#3604 node "missing dependency" mis-modeled npm hoisting, 2026-07-08 (2nd bugbot FP on same PR).** - Bugbot Medium `3545172685` claimed `run_node` installs only `redis` while the time-series - node example `import`s `@redis/time-series`, so `./run.sh time_series_tutorial node` "fails - at module load." **FALSE POSITIVE**: the `redis` npm metapackage (v6.1.0) declares - `@redis/time-series` (and `@redis/bloom`/`json`/`search`/`client`) as **direct dependencies**; - after `npm i redis` they sit hoisted-flat at `node_modules/@redis/time-series`, so the bare - import resolves. Refuted empirically: `node_modules/@redis/` contains `time-series`, and the - existing `results/time_series_tutorial_node.log` shows the module **loaded** and failed later - on a `deepEqual` assertion (out-of-scope `.rules` staleness), not at import. Lesson: bugbot's - "missing dependency / fails at module load" findings don't account for metapackage transitive - hoisting — check `node_modules` (or the metapackage's `dependencies`) before trusting them. - Same root as the async-`allOf` FP: bugbot mis-models a **dependency/ordering mechanic** it - can't see from the diff alone. - -- **#3612 pickup closing-sequence churn, 2026-07-10 (5th churn PR; docs/spec target).** Round 1 - flagged the label-drop *ordering* between `park-manifest.md` and `pickup` Step 5 (3558223727); - the fix reordered Step 5. Round 2 (on the fix commit `d5b6c86fd`) then raised a High, - `3558314964`: Step 4 reconciles + rebases but never *commits*, and Step 5 runs `/reflect` on - "the reconciliation commit" + `/finalize` — both of which see committed history only, so an - uncommitted reconcile is silently dropped from the squash. New, independent finding (not - ping-pong — no fix undone, no A↔B cycle) but the **same under-specified subsystem**: the - pickup closing sequence (reconcile → commit → reflect → finalize → drop-labels). The r1 edit - sat right in that area, so it's a "fix-caused-its-own-next-round" flavour like #3585. Consolidated (r2 fix `7044aefc1`): - an explicit ordered 4-step closing sequence in `pickup` Step 5 with the commit as step 1, and - the manifest summary deferring to it as authoritative. **But r3 (2026-07-10) proved that - consolidation was scoped too narrowly** — bugbot High `3558614316` "Missing push before merge": - Step 5 commits locally but never pushes, and the squash merges the *remote* head, so the - reconciliation commit **and** Step 4's rebased/merged-`main` history stay local and can be - omitted from the merge. Landed on the r2 commit itself. This is the **2nd PR** exhibiting the - #3536-round-6 meta-pattern ("when round N+1 finds another gap in an area you *just* - consolidated, your boundary was wrong — widen it"): the true subsystem is the - **local→remote→squash** dependency, not "is it committed." Recommended r4 fix is not another - step but an **invariant**: before the `gh pr merge` hand-off the remote PR head must contain - everything that must land (commit + push + Step 4 history + re-push after any `/reflect` - amend). Calibration note: bugbot effectively performed the churn detection one seam per round - — strong on procedure-completeness, which is why the fix must widen the boundary, not chase - the next seam. Meta-flag: if even the invariant rewrite draws an r4, the closing sequence - likely needs to leave prose for an actual checklist/script the skill references. - **It did (r5/bugbot-r4, 2026-07-10): `3558867168` "Manifest omits rebase step"** — the - *third* finding on the manifest-summary-vs-`pickup` drift (after r1 label-order and Codex's - push-order P2). Root cause finally clear: the manifest kept a hand-maintained **copy** of - pickup's closing sequence, and a copy drifts from its source every round — each fix matched - one more element, the next round found the next omission (rebase, this time). Ended not by - patching in "rebase" but by **eliminating the duplication**: manifest §5 now defers entirely - to `/pickup` Steps 4–5 (the single authoritative sequence) and restates *no* steps, so there - is nothing left to drift. Lesson (generalises the #3536-r6 "widen the boundary" rule): when a - *summary/duplicate* of a procedure keeps drawing drift findings, the boundary to widen is - "stop duplicating" — collapse to one source and point at it, don't keep re-syncing the copy. - The 5-round arc (3 of them manifest-copy drift) is the textbook case for it. - **Validated + a 2nd thread exposed (r6/bugbot re-scan of `03ec1e1f5`, 2026-07-10):** the - re-scan of the collapsed manifest raised **nothing** on it — the copy-drift class is - structurally closed (fix-quality confirmed). But a *distinct* churn thread surfaced underneath - on the authoritative source: `3558994646` "Rebase before reconciliation commit" - (pickup/SKILL.md) — Step 4 applies edits then `git rebase`, which fails on a dirty tree, while - Step 5 commits only afterwards. This is thread B: the **git-operation ordering** in the - closing sequence (commit r2 → push r3 → rebase-vs-commit r6) — three genuine ordering gaps in - pickup itself, separate from the manifest-copy thread. Lesson refinement: de-duplication kills - *copy* drift but not an under-specified *procedure*; six prose rounds is the signal to stop - describing a precise git sequence in prose and make it one explicit ordered block/checklist - (commit → rebase → link-check → /reflect → push → /finalize → drop-labels). Also: **longest - clean churn-not-loop case on record (6 rounds, still no ping-pong)** — new independent finding - each round, no fix undone, no A↔B cycle. - **7th round (bugbot re-scan of `91d0c2083`, 2026-07-10): churn is now 4-deep on the git - operations specifically.** After the r6 consolidation, bugbot raised `3559171169` "Rebase push - omits force lease" (a rebased parked branch was already pushed at `/park`, so a plain `git - push` is non-fast-forward — needs `--force-with-lease`) plus `3559171157` "Scan mode inherits - stop rule" (Mode A borrows Step 2's trigger check, which includes *stop-if-not-met*, so a scan - would abort on the first not-ready PR instead of recording `fired?=no` and continuing). The - git-op count is now commit (r2) → push (r3) → rebase-order (r6) → force-lease (r8): every time - the sequence spells out one more git command, the next git edge case surfaces, and **two of - these were my own prior fixes exposing the next** (rebase step → force-lease; Mode-A→Step-2 - reference → stop-rule import). Terminal diagnosis: the skill **over-specifies git plumbing** - that is context-dependent and standard operator competence. Recommended endgame is NOT another - consolidation but **de-prescription**: reduce Step 5 to the *invariant* (remote head must - contain everything before hand-off) + an ordered checklist of *intent* (reconcile → commit → - sync main → /reflect → push → /finalize → drop labels), delegating the exact commands - (force-with-lease after rebase, dirty-tree ordering) to the operator — an invariant can't be - "incomplete" the way a command list can. Generalises the churn rule one step further: when - *enumerated mechanics* keep drawing correct-but-incremental findings, stop enumerating and - assert the invariant instead. -- **#3604 `@Test`/`[Fact]` "leak" ignores the build's marker stripping, 2026-07-09 (3rd FP flavour).** - Bugbot Low `3552548353` flagged a Lettuce override for leaving `@Test` on `run()` *outside* - any `REMOVE_START`/`REMOVE_END` region, concluding it would "expose JUnit markup in the client - tab." The *observation* is correct (the `@Test` is unwrapped) but the *impact is wrong*: - `build/components/example.py:185` strips any line matching the language's test marker - (`@Test` for java/java-async/java-reactive, `[Fact]` for c#) during extraction, independent - of REMOVE markers. Verified empirically — the rendered file has **zero** `@Test`. Lesson: - when bugbot claims a test annotation (`@Test`/`[Fact]`/`[SkippableFact]`) "leaks" into a - rendered client tab, check `build/components/example.py`'s `TEST_MARKER` stripping and the - actual rendered `examples/...` output before trusting it — the extractor already removes them. - (User flagged this one as a likely FP before I confirmed it — trust that instinct.) - -- **#3612 "`merged` field doesn't exist" — list-vs-single-PR endpoint confusion, 2026-07-10 (4th bugbot FP flavour; docs/spec target, not code).** - Bugbot High `3558223722` claimed the park/pickup snapshot command `gh api repos///pulls/ --jq '{merged, ...}'` - selects a nonexistent top-level `merged` field (should use `merged_at`), so merge-state diffs "silently fail." - **FALSE POSITIVE**: the **single-PR** REST endpoint (`GET /pulls/{n}`) — which the skills document — returns a - top-level `merged` boolean. Refuted empirically twice: `gh api repos/redis/docs/pulls/3612 --jq '{merged}'` → - `{"merged":false}`, and the DOC-6829 park run got `"merged":false` for spring-data-redis#3390 via the same path. - Root cause: the **list** endpoint (`GET /pulls`) omits `merged`, so the field is context-dependent; bugbot generalised - the list-shape to the single-PR call it can't distinguish from the diff. Same family as #3604's async/npm FPs — an - **API/runtime mechanic bugbot can't see from the diff**. Lesson: when bugbot says "this field/API won't work", run the - documented call before trusting it, especially where list-vs-item endpoint response shapes differ. - -- **#3714 "Ruby incorrectly excluded" — finding refuted by harness + reads stale PR body, 2026-07-28 (5th FP flavour; the "wrong fix direction" case).** - Bugbot Medium `3665551935` (hashes.md L164-165, L179-180): the `hexpire`/`hpexpire` `lang_filter`s exclude Ruby, but "per the PR description only `hexpireat` needs to omit Ruby" and "Ruby supports `HEXPIRE`/`HTTL`/`HPEXPIRE`/`HPTTL`, so it belongs in the first two filters" → re-add Ruby + add Ruby steps to `dt_hash.rb`. **Refuted:** the harness run this session proved released **redis-rb 5.4.1 has no native methods** for any of these — `r.hexpire(...)` falls through `Redis::Commands#method_missing` and emits `HEXPIRE key 60 f1 f2` *without* the mandatory `FIELDS ` token → `ERR wrong number of arguments`. Ruby was deliberately dropped from **all three** blocks in commit `a4528e2f4`. Two blind spots stacked: (1) bugbot trusted the **API surface** (the repo's own `data/command-api-mapping/HEXPIRE.json` even lists `redis_rb` — the aspirational-mapping trap), and (2) it treated the **stale PR body** (written pre-harness, said only hexpireat omits Ruby) as ground truth. Same root axis as #3604/#3612/#3627 — a runtime/environment fact invisible in the diff. **Distinctive reconciler lesson:** the finding *correctly detected* a description↔code inconsistency but proposed the **wrong resolution direction** (change the code). The right move is to **fix the stale PR description**, not the harness-verified code. When a finding cites "the PR description says…", check whether the description or the code is the stale side before acting. - -### Bot true positives (calibration — what bugbot is good at) -- **#3604 `Double.MIN_VALUE` ≠ −∞, 2026-07-09 (language-semantics strength).** Bugbot Low - `3550287458` flagged `Range.create(Double.MIN_VALUE, 10)` in the Lettuce `SortedSetExample` - async+reactive overrides: `Double.MIN_VALUE` is Java's smallest *positive* value (~4.9e-324), - not −∞, so the range is ~0→10 not −∞→10 (the CLI uses `-inf`). Correct, and correctly scoped - as "produces right output on this all-positive dataset but teaches a copy-paste-wrong pattern." - This is the **complement** to the two dependency/ordering FPs on the same PR: bugbot is reliable - on **pure language/API semantics** (values it can evaluate from the source alone) and unreliable - on **runtime mechanics** it can't see (dispatch order, transitive deps). Reconciler value-add: - the *identical* bug also sits in `tmp/datatypes/sorted-sets/SortedSetsExample.java` (jedis, from - DOC-6057 `2569d4bc2`) — bugbot correctly did **not** flag it because it's pre-existing on `main`, - outside the PR diff. So "bugbot found N instances" ≠ "N instances exist"; grep the whole tree for - the pattern when a semantics finding lands. From 68da99bada46b559554deb9e6d9d19fa41b92c45 Mon Sep 17 00:00:00 2001 From: Andy Stark Date: Fri, 18 Sep 2026 11:48:07 +0100 Subject: [PATCH 2/2] DOC-7085 Decode repo-memory's hidden data blob before fetching squash messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live-tested step 3a against PR #4047's real repo-memory comment. The rendered bullets only echo the title; the actual score + a ~500-char bodySummary preview live in the hidden redis-repo-memory-data base64 blob, already present in the comment fetched in step 2 at zero extra cost. For commit-typed items the preview is a genuine slice of the /reflect+/finalize message (verified on 4465b6ad6's own squash commit), but truncated well short of the full 1500+ char message and can cut off mid-finding — so use the decoded preview + score to decide what's worth a full fetch, rather than fetching every related item indiscriminately. Co-Authored-By: Claude Sonnet 5 --- .claude/commands/docs/assess-comments.md | 48 +++++++++++++++--------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/.claude/commands/docs/assess-comments.md b/.claude/commands/docs/assess-comments.md index 3c5626389c..c398a83d8d 100644 --- a/.claude/commands/docs/assess-comments.md +++ b/.claude/commands/docs/assess-comments.md @@ -116,30 +116,42 @@ spelling in a quote") is exactly what lets you adjudicate the tool findings. *context*, not findings — they tell you what the author intended and whether the build is healthy. Don't raise them as issues. -3a. **Mine repo memory for prior art — but only via the real squash message, - never its comment body.** Redis repo memory's comment lists related PRs - and commits ranked by similarity, each with a `sourceUrl` and a - `bodySummary`. **The `bodySummary` is not the evidence** — it's often a - one-line title echo or empty. It's a pointer, nothing more. - - For each related item whose title or file path plausibly overlaps a - cluster you're about to adjudicate (steps 4–7), follow the link and pull - the **real** message: +3a. **Mine repo memory for prior art, cheaply first.** Redis repo memory's + comment renders each related item as a one-line title echo — that + rendered line is not useful. But the same comment carries a hidden + `` block at the bottom: decode + it (`base64 -d` the payload, it's a JSON array) to get, per item, a + `score` (lower = closer match), a `sourceUrl`, and a `bodySummary` + truncated to roughly 500 characters. You already fetched this comment in + step 2 — decoding costs nothing. + + - Use `score` to rank the related items; don't treat the comment's own + display order as relevance. + - For a `push_summary`/commit-typed item, the `bodySummary` preview is a + genuine slice of the real `/reflect`+`/finalize` message (verified: it + correctly led with the actual "verified X against the source rather + than the PR description" finding, not a title echo). It's often + enough on its own to judge relevance, and sometimes enough to cite. + - It is **truncated**, though — a real distillation easily runs to + 1,500+ characters and the cut can land mid-thought, before a second or + third finding. Don't treat the preview as complete. + + For the items whose score and preview show real overlap with a cluster + you're about to adjudicate (steps 4–7) — same file/subsystem, or the + same bot raising a similar-shaped claim — pull the **full** message + before citing it as precedent: - For a PR: `gh pr view --json state,mergedAt,mergeCommit` — if merged, the squash commit at `mergeCommit.oid` carries the full - `/reflect` + `/finalize` distillation (what was learned, what was - rejected, what future-you must not break). Fetch it with - `git log -1 --format=%B ` (or `gh api - repos/{owner}/{repo}/commits/{oid} --jq .commit.message` if the commit - isn't in your local history). + distillation. Fetch it with `git log -1 --format=%B ` (or + `gh api repos/{owner}/{repo}/commits/{oid} --jq .commit.message` if + the commit isn't in your local history). - For a bare commit link: the same `git log -1 --format=%B ` — it may be a smaller, less distilled message, so weight it accordingly. - Don't do this for every related item indiscriminately — it costs a - network round-trip per item. Do it for the ones that could plausibly - bear on a finding you're actually adjudicating this round (same - file/subsystem, or the same bot raising a similar-shaped claim). + Don't do the full fetch for every related item indiscriminately — it + costs a network round-trip per item, on top of decoding the blob. Reach + for it only once the cheap preview says it's worth it. Use what you find as **precedent, cited by PR number**, in step 7's adjudication — e.g. "PR #3604's squash message documents that bugbot's