WIP: Memory/draft verification - #145
Draft
Hareet wants to merge 46 commits into
Draft
Conversation
validate-schema proves a draft is well-shaped, not that the shape is true. `/issues/<pr-number>` redirects to `/pull/`, so a draft keyed to its own merge PR validates perfectly — which is how ~60 of the first 107 drafts reached review mis-keyed, and how five drafts naming symbols that do not exist in cht-core passed CI green. Adds `npm run verify-drafts`, the part of that gap a machine can close hermetically: identity coherence (id/issueNumber/issueUrl agreement, the issueNumber-equals-own-source_pr signature), filename-token vs frontmatter agreement, cross-domain duplicate detection against the already-landed corpus, and a near-miss lint against a committed cht-core vocabulary snapshot that catches `con_create_people`, `docs_by_type` and `task.status`. Leakage and metadata-distribution lints are warnings. `--online` adds the one check needing the network — asserting issueNumber really names an issue — and reports "unverified" rather than a pass when gh is throttled. Wired into the existing CI job with --changed-only so per-file findings are scoped to the PR's own drafts and pre-existing corpus debt does not fail unrelated PRs; --online stays off in CI until a run-scoped GITHUB_TOKEN is proven able to read medic/cht-core. actions/checkout gets fetch-depth: 0, without which the base is unreachable and the diff silently empty. Every fixture in test/fixtures/drafts transcribes a defect a reviewer actually found in PRs #120/#121/#123/#130/#131/#132, so those classes cannot return unnoticed. docs/memory-draft-verification.md states plainly what this does NOT catch — absent symbols that are not near-misses, misattribution, inverted semantics, mechanism and backport claims — all of which need the source tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Layer 1 (verify-drafts) is hermetic, so by construction it only sees defects
visible in the draft itself. The defects that actually survived two review
rounds need the source tree: a fabricated symbol resembling nothing real
(getOidc, isDue), a real symbol credited to the wrong file
(updateServiceWorker), an inverted mechanism claim ("preserved" a file the PR
deleted), and a backport attributed to the wrong release line.
Two stages, deliberately split. An LLM reads a draft and extracts checkable
CLAIMS — the one step that needs a model, since recognising an assertion in
prose is not mechanisable. Every claim is then settled by a git probe. The model
never decides truth; `git grep` does. An LLM verdict can flip between runs on
identical bytes, which is why this split, and not a "review the draft" prompt,
is what makes the result trustworthy.
claim-probes.ts holds the deterministic half, with three load-bearing rules:
- Word-bounded search only. Substring `getOidc` returns 8 hits in cht-core, all
longer identifiers (getOidcUsername, getOidcBaseUrl); an unanchored grep would
have CERTIFIED the exact hallucination the reviewer caught. -F -w returns 0.
- Absence is only provable at a commit, so probes are scoped to the draft's
anchor. Where the anchor will not resolve — cht-core does not stamp every PR
number into its subject, the SSO cluster among them — tree-scoped claims fall
back to origin/master and are marked provenance: 'fallback', while claims
needing the commit itself stay unverifiable. Unverifiable is never a pass.
- Reverts are not evidence. PR #10599 resolves to a commit reverting the change
its draft describes; that anchor is refused outright.
Verified against the real checkout: it reproduces every round-2 finding,
including the two Layer 1 cannot reach — updateServiceWorker comes back "0 hits,
but 16 elsewhere: attributed to the wrong file" pointing at config-watcher.js,
and add-branding-doc.js comes back "was deleted, not modified". It also settles
the chtRolesSettings claim the first pass could not, naming the real parameter
chtPermissionsSettings.
Not wired into CI, on purpose: it needs a large checkout and an LLM, and a
required check that can flake gets de-required. Reports land in
outputs/verification/ (already gitignored) stamped with a content hash, so a
later promotion gate can refuse a report that does not match the draft bytes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nd anchor-resolution limits Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…llback Dogfooding ground-claims on the configuration branch surfaced three false positives, all from treating a fallback probe as if it were an anchor probe. Absence only refutes a claim at the draft's OWN commit. Under fallback we search a tree that predates the change, so every symbol a post-cutoff PR introduces looks fabricated. Draft 11057 (PR postdates the clone) had `weight` and `header_tab` reported as defects when their absence from master is exactly what you would expect. Absence under fallback is now unverifiable. Misattribution gets the same treatment for the same reason: under fallback we cannot distinguish a wrong file attribution from a symbol the PR itself added to that file, and a common word like `weight` matches somewhere in any large tree (165 hits). It is now reported as unverifiable with the real location as a hint, so a human still sees it. At the anchor it remains a hard defect — that is the updateServiceWorker case, and it still fires. Finally, symbolHits retries a dotted symbol with optional chaining before declaring it absent: source writes `res?.resources` while prose and claim extraction normalise to `res.resources`, which made a real member access read as fabricated. Net effect on the configuration branch: 3 residual findings, all artifacts, drop to 0 — while every genuine defect the sweep found still reports. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… references Regenerating the vocabulary against a current cht-core checkout dropped doc_by_type from the couch-view family — which would have silently disabled the check that catches `docs_by_type`, the exact defect this family exists for. The view has not gone anywhere: ddocs/medic-db/medic-client/views/doc_by_type/ is still there. What changed is that its callers moved to constants, so the name no longer appears in any .js/.ts content. Mining from references makes the vocabulary only as complete as the current call sites, and shrinks it silently whenever a refactor introduces an indirection. couch-view now mines ddocs/**/views/<name>/ — the repository's own declaration of what views exist. The family grows 26 -> 32 (also more complete than the original 29, since it now includes views declared but not referenced anywhere). Snapshot refreshed to 1f3e56f226 (2026-07-27) from the current clone at medic-cht-agent/cht-core; the previous snapshot was ~4 months stale. The committed spec assertions (real terms in, fabricated terms out) are what caught this, and they still pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…aleness Measured against a same-day cht-core clone, six PR numbers across the corpus still resolve to nothing — fetching does not fix that class. Two of the three configuration drafts that would not anchor had real provenance problems: one describes a feature absent from master, one credits a mechanism to the wrong PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ning
Grounding the messaging branch produced 11 findings, 6 of which were defects in
these probes rather than in the drafts.
BACKPORTS. `branch --contains <anchor>` can never see a backport, because a
cherry-pick is a different commit. Two drafts stating true backports were
reported as defects: #10073 -> f4e54b43f on 4.21.x, #10230 -> b567b0a8a on
4.22.x, both reachable only by searching for a commit that references the same
PR number. checkReleaseBranch now falls back to that search, and the branch
matcher tolerates prose forms ("4.21", "4.x") against refs ("origin/4.21.x").
OPTIONAL CHAINING. The previous retry replaced every dot, so it could only match
`doc?.fields?.patient_id`. Real source puts the `?` on an arbitrary subset —
`doc.fields?.patient_id` — and three claims in one draft were called fabricated
because of it. The retry now builds an -E pattern making `?` optional at every
position instead of guessing one spelling.
Both classes share a root cause worth naming: a probe that answers "not found"
is only trustworthy if it has searched every spelling the source could plausibly
use. Net on messaging: 11 findings -> 5, all 5 remaining confirmed as extraction
artifacts rather than content defects.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…hird branch
Each of these reported a correct draft as defective.
TRANSLATION KEYS vs TASK FIELDS. The task-field candidate pattern matched
`task.list` inside `task.list.complete`, a real translation key with 11
occurrences, and reported it as a fabricated field. The pattern now rejects a
match followed by `.<lowercase>`, so a longer dotted key is skipped while a
sentence-ending period still leaves a real field detectable.
BACKPORT PR NUMBERS. checkReleaseBranch searched only the draft's own PR number,
but a backport is carried by a different PR and the draft names it right in the
sentence ("backported to 4.13.x (PR #9555)"). Two true backport claims were
reported as defects. It now also tries every PR number quoted in the claim.
COLLAPSED CLUSTERS. A draft that merged a duplicate cluster carries
`source_prs[]` and its Related Files legitimately span all of them, but
file-touched checked only `source_sha` — so the sibling PRs' files read as
untouched. Sibling anchors are now resolved and their diffs unioned.
Pattern across all seven fixes so far: a probe answering "not found" is only
trustworthy if it has searched every place and spelling the source could
plausibly use, and every unresolvable anchor must degrade to unverifiable rather
than to a defect. The vocabulary snapshot is regenerated at 1f3e56f226.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
FOURTH false-positive class, proven live on the promote-configuration review (#130): a PR merged into a FEATURE branch that later squash-merged onto master leaves no trace a clone can see. The child's merge commit lives only on the deleted branch and no squash subject carries its number, so both local strategies failed and nine TRUE claims in the 11057 draft degraded to unverifiable — and the same blind spot produced a wrong "not in cht-core" provenance diagnosis on 11021's removal. resolveAnchor now takes the repo slug and, when the clone alone cannot name the commit, asks the GitHub API which LOCAL commit carries the PR: - merged PR whose merge commit is in the clone but whose subject has no "(#N)" stamp (the SSO cluster) -> anchored directly; - merged into a feature branch -> one hop: the PR whose HEAD is that branch (pulls?head=owner:branch) supplies the squash the clone can see (11057 -> 10224-ui-extensions -> #11050 / 180c29ecf). Resolution only — adjudication never leaves git, a sha the clone lacks is never anchored to, and every failure (gh missing, curl down, rate limit, unmerged PR) degrades to the existing unverifiable semantics rather than a defect or a crash. gh is tried first for the authenticated rate limit; anonymous curl is the fallback. Anchors resolved this way carry a note surfaced in claims.json and REPORT.md; --no-api-resolve keeps a run fully offline. Grounding against an epic squash reuses the sibling-union over-approximation already accepted for source_prs: file-touched sees the union of every child's changes and statuses reflect the landed state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ng branch
Each reported a TRUE claim as a defect, or silently skipped a checkable one:
HAND-AUTHORED ANCHORS. The pre-pipeline drafts carry only source_prs[] —
no source_pr, no source_sha — so anchorFor never resolved them and every
commit-scoped claim degraded to unverifiable (24 across five messaging
drafts). The first source_prs entry is now the canonical PR candidate.
LINE-WRAPPED MEMBER CHAINS. request.post spelled as 'request\n .post({'
is invisible to line-oriented git grep, so a correct symbol-in-file claim
read as misattribution (10073, africas-talking.js:80-81). A dotted symbol
that misses the scoped grep is re-checked against the file blob with
whitespace tolerated around the dots and word-ish boundaries at both ends.
PATH ABSENCE UNDER FALLBACK. checkPathExists returned ungrounded at the
fallback ref, but layouts move (api/ -> api/src/, protractor -> wdio):
three true 2018-era path claims in the 4278 draft were reported as
defects. Absence now degrades by provenance exactly like checkSymbol.
Pattern unchanged from the first seven fixes: a probe answering 'not
found' is only trustworthy if it has searched every place and spelling
the source could plausibly use, and anchor problems must degrade to
unverifiable rather than manufacture defects.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eading, not probing Round 3 on #120/#130 was almost entirely defects no existing layer could see. Each class below is now checked mechanically, with the LLM confined to identifying candidates that code then verifies. SELF-CONTRADICTION (new Layer 3, check-coherence.ts). A grounding pass corrects the sections that assert mechanism against code and leaves the interpretive ones asserting what it just disproved, so both halves ground independently while the document tells two stories. 10198 says template safety comes from ng-if attributes 'not from scaffolded keys' in Solution and that the controller 'scaffolds the minimum keys the template requires' in Design Choices. The model may only IDENTIFY pairs and must quote both sides verbatim; verifyContradictions drops any pair whose quotes do not occur in the file, so a hallucinated finding cannot land. DRIFT (claim-probes.ts). resource-icons.service.ts was real when its PR shipped and is gone today: checking only the anchor certifies it, checking only master refutes it, and both are wrong — the claim is true, the tense is not. A claim that grounds at its anchor is now re-checked against master and flagged when the draft names a dead entity with no temporal qualifier. Exits 3, not 1: nothing is disproven, so 'ungrounded' keeps meaning 'this is wrong'. CROSS-REFERENCE ACCURACY (verify-drafts.ts, --online). #10754 sat in Related Issues glossed as 'Scheduled task duplicate processing'; it is 'Cookies not being sent with secure: true'. Every #N there is now classified and its gloss compared to the real title, with only total disjointness reported so paraphrase stays free. PRs cited as issues warn unless labelled 'PR #N'. SNIPPET FIDELITY (claim-probes.ts). The 4278 fence was a composite of two real helpers, so every symbol in it probed clean while the block existed nowhere. Fences are matched against every file the draft names, whitespace- and comment-insensitively, with elision markers as segment boundaries. STALE TIMESTAMPS (verify-drafts.ts). lastUpdated must not predate the file's last commit; 13 rewritten drafts kept their old stamp. Also: the extraction prompt no longer turns a REMOVAL claim ('removed the parseResponseBody helper') into an existence probe, which cost a false flag last run; gh-classify gains describeNumber for titles; Draft carries its absolute path so git pathspecs work when scanning a sibling worktree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…anches CURL FALLBACK for the issues endpoint (gh-classify). The online checks were unusable on a host without `gh`: every lookup threw and reported 'unverified', which is honest but checks nothing. fetchIssueRecord now retries anonymously via curl, distinguishing a real 404 from a transient failure by status code, so the same three-way answer reaches callers. This is what let the cross-reference audit run at all — and it immediately found #10754 on messaging and confirmed all nine configuration refs. RELATIONSHIP GLOSSES are exempt from the title comparison. 'Blocker for #10908' describes linkage, not the referenced issue's subject, so comparing it to that issue's title flagged a correct reference (11057's #10901). Any gloss opening with a relationship word, or containing another issue number, is now skipped. DRIFT IS PER ENTITY, not per sentence. A draft normally qualifies a dead path once — in a note or an annotated Related Files entry — then names it plainly elsewhere. Keying off the claim's own quote demanded the caveat in every sentence and flagged 10278 four times after it had been fixed properly. One honest mention now settles the entity. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…hecks
SUMMARY IS A SECTION. Two of three residual contradictions had one side in
the summary field, and the coherence prompt listed summary among the
sections a grounding pass 'corrects', so the model under-weighted it. It
is now checked against every other section first, with both real misses as
worked examples. The summary is the most-read line in a memory; it should
be the most scrutinised, not the least.
(added)/(modified) NO LONGER COUNT AS TIME-SCOPING. Those annotate what a
PR did to a file. Whether the path still exists today is a different
question, so treating them as scoping let a draft mark a path '(added)' in
Related Files and recommend it in the present tense elsewhere, unflagged.
'(deleted)' stays: it does say the path is gone.
GLOSS CHECKING GOT ONE TIER MORE HONEST. A wrong cross-reference sharing a
single incidental word with the real title passed - #10446 ('Dont send
empty messages') described as 'failed/invalid scheduled messages were not
being cleared', joined only by 'messages'. Tightening the blocking rule to
require two shared words would flag correct short glosses, so instead the
blocking rule still fires only on total disjointness and a new
related-ref-gloss-weak WARNING covers the gap: a gloss of four or more
content words sharing exactly one. Separately, a relationship prefix now
exempts only itself - 'parent improvement - <claim>' still has <claim>
checked, where before one leading word excused the whole line.
Also documents, without closing, what these checks cannot see: untagged
and comment-only fence content, snippets misattributed among a draft's own
files, prose aliases for a dead path, and that 'last edited' counts
stamp-only and revert commits. The coherence header no longer implies its
quote gate proves a contradiction is real - it proves the quotes are.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s it to
Drift's mirror image, and the second false positive of this shape. A
time-scoping note added in response to review names the REPLACEMENT for a
dead path - '...replaced by custom-resource.service.ts in #11050' - and the
replacement by definition does not exist at an anchor that predates it, so
the prober refuted a true sentence. The 9467 'current master reads
err?.status' aside failed the same way last round and I reworded around it;
rewording is the wrong fix twice.
A tree-scoped claim that fails at its anchor is now re-checked at the
current ref when the quote scopes it forward ('replaced by', 'renamed to',
'on master', 'now reads', ...). Grounding there is reported with fallback
provenance and an evidence note saying the anchor lacked it, so the weaker
scope stays visible. A forward-scoped claim false in BOTH trees is still
ungrounded, and a claim with no forward wording is never rescued - absence
at the anchor remains a defect by default.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two precision fixes from a re-verification pass.
SILENT EMPTY SELECTION. --changed-only resolves its diff in the --dir
target, but display paths are relative to the repo running the tool. Point
it at another worktree and git returns paths in that repo's language while
the drafts are named in this one, so nothing matches, zero drafts are
selected, and the run reports clean. The existing guard only fired on an
empty diff, which is the shallow-checkout case, not this one. A non-empty
diff matching no draft now throws and says why. For a verification tool,
reporting 'clean' because it checked nothing is the worst failure mode
available, and it is the one a reviewer would never see.
'moved to' IS NOT FORWARD-SCOPING. Anchor-era prose says 'the guard moved
to the top of the function', meaning the PR moved it - not that the current
tree differs from the anchor. Leaving it in FORWARD_SCOPED could rescue a
genuinely wrong anchor-era claim by re-checking it against master. The
unambiguous markers ('replaced by', 'renamed to', 'since renamed', 'on
master', 'now reads') carry the real cases.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…udget Anonymous GitHub allows 60 requests an hour and describeNumber had no cache, so a corpus that cites the same issue from several drafts spent its budget re-fetching a handful of numbers and reported everything after that as 'unverified' - which reads like a clean run unless you check the count. That is why the tasks-and-targets branch went unaudited: 23 of 25 online checks were rate-limited on an earlier attempt. Two changes: describeNumber takes a shared DescribeCache, and checkIssueIsNotPr now goes through it instead of classifyNumber. A draft almost always cites its own issue in Related Issues as well, so the two checks were fetching the same record twice. For the 25 tasks drafts that turns 58 requests into 33 distinct ones - a scan that did not fit now does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The extraction half of ground-claims is an LLM and the settling half is
git, so verdicts were reproducible while COVERAGE was not. Measured over 13
drafts whose bytes did not change between two runs:
claims extracted run 1: 165 run 2: 180
present in both: 78 present in only one: 189
union: 267 -> one pass saw 61-67% of it, 29% overlap
A single run is a sample, so 'N grounded, 0 ungrounded' from one run
overstates what was checked - including in review comments already written.
Prose has no canonical decomposition into claims, so the semantic tier
('the fix throws on write errors') will always need a model. But the tier
the reviewer keeps catching - a named file, a named symbol, a path that has
since moved - is code-shaped and can just be enumerated. enumerate-claims
regexes the draft for repo paths and backticked identifiers, splits Related
Files (file-touched) from everything else (path-exists), dedupes, and hands
the lot to the same probes. Same bytes in, same claims out. It is unioned
with the model's output, so coverage is now exhaustive over code-shaped
claims and still sampled over semantic ones - which is what the tool can
honestly promise. It also runs when the model fails, so an outage degrades
coverage instead of erasing it.
Deliberately not extracted: bare lowercase words in backticks such as
'pending' or 'due', which are usually state strings and indistinguishable
from prose emphasis. A false 'this is fabricated' costs more than a missed check.
Paired with a fallback for dotted tokens prose writes whole and code never
spells: sms.clear_failing_schedules reached via config.get('sms')?.…,
smsparser.parse declared as exports.parse, RulesEngineService.fetchTargets
defined bare. All four were hand-adjudicated as false positives during
review; the final segment now resolves them to grounded with an evidence
note, so exhaustive extraction does not mean an exhaustive noise floor.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tive noise
The first exhaustive run over all three branches produced 44 ungrounded
findings and every single one was an artifact - 41 of them mine. Triaging
them by hand would have relocated the work instead of removing it, and a
reviewer running the tool would have had to repeat that triage.
Six filters, each from a real false positive in that run:
- A FILENAME IS NOT A SYMBOL. Backticked smsparser.js, sender.component.ts,
tasks.component.ts and friends were probed as identifiers. Real repo paths
are already caught by the path regex.
- ABSENCE CONTEXT. A line saying the thing is gone, renamed or was never
adopted is asserting its ABSENCE, so probing existence inverts the claim.
This was the largest class: "Removed the parseResponseBody helper", "the
original can_hide_target_count_past_goal permission was superseded",
"isTelemetryOrFeedback -> isReplicableDoc". Note the arrow alternative has
to sit outside the \b group - neither character is a word character, so \b
could never match beside it and my first version of this was dead code.
- DISCLAIMED RELATED FILES. "api/controllers/sms-gateway.js (the endpoint
under test; not modified)" is not a file-touched claim. Downgraded to
path-exists rather than dropped, since the path is still asserted to exist.
- REMOTE ENDPOINTS. api/v2/broadcasts.json is RapidPro's, not ours, but it
starts with api/ and ends in .json.
- OUR OWN FRONTMATTER KEYS. A draft discussing source_sha or domainFit is
not claiming a cht-core symbol.
- ELLIPSES. `for...of` is prose.
Also tightened the LLM extractor, which invented a directory prefix for a
draft that named only a bare filename ("rendered from analytics.component.html"
became webapp/src/ts/components/analytics/... when the real tree has
modules/analytics/...). It is now told to use the path exactly as spelled or
emit nothing.
Worth recording what the 44 findings actually proved: the corpus is clean.
An exhaustive pass examined 10-15% more claims per draft than sampling did -
127 vs 117 on configuration, 262 vs 229 on messaging, 452 vs 395 on tasks -
and surfaced no new real defect on any branch.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Self-inflicted, and worth recording as such. To stop the extractor
inventing a directory for a bare filename, I told it to emit the bare
string instead - and tasks-and-targets promptly went from 33 ungrounded to
50, because a claim of "integration.spec.js" cannot match a diff that
stores shared-libs/rules-engine/test/integration.spec.js. Seventeen new
findings, all of them real files, none of them defects.
Suppressing those claims would have been the wrong fix: prose names files
by basename constantly ("Updated unit tests across the rules engine
(integration.spec.js, pouchdb-provider.spec.js, provider-wireup.spec.js)")
and those are checkable, just not by exact path. file-touched and
path-exists now resolve a name containing no slash against the diff and the
tree respectively, and report which full path it matched.
The rescue is deliberately narrow: it fires ONLY when the draft gave no
directory at all. A claim naming webapp/src/ts/components/analytics/... when
the tree has modules/analytics/... stays ungrounded, because a wrong
directory is a real defect and must not be laundered by its basename. An
explicit status is still enforced against whatever the basename resolves to.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… squash 27 of the 34 remaining tasks-and-targets findings were one structural cause. A PR merged into a feature branch has an unreachable merge commit, so it is anchored at the epic's squash - but that squash is a DIFFERENT changeset. It carries every sibling's work, and where the epic renamed things before landing it does not contain the child's files under the names the draft correctly records. 10390 lists 21 target-interval.* files that PR #10390 really created and that the #10423 squash landed as target.*, so an accurate Related Files list read as 21 fabricated paths. file-touched now asks the API for the child PR's own file list when the anchor was reached by the epic hop, and says so in the evidence. Verified live: 10390's target-interval.ts and api/src/controllers/target-interval.js ground, while an invented path still refutes. Results are cached per PR, so an epic's children cost one request each, and any transport failure falls back to the squash diff rather than failing the run. This is the last of the epic-squash blind spots. The first was anchor resolution (adfb256), the second drift against master, and this is the third: three different probes each assuming the anchor commit IS the PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…regression 2797d95 taught file-touched to ask the API for an epic child's own file list, and in doing so returned early - skipping the sibling union that collapsed clusters depend on. 9232 went from 2 ungrounded to 8. It is both things at once: a cluster of three PRs (9232/9282/9317) whose Related Files legitimately span all three, AND an epic child anchored at the squash of a different PR. Neither source alone can settle it. file-touched now absorbs all three sources before deciding: each epic child's own PR file list, the anchor commit's diff, and every sibling commit's diff. The evidence line names which sources were consulted. Two more gaps closed while here: - SUFFIX MATCHING. A sub-package names files relative to its own root ("test/qualifier.spec.ts" inside shared-libs/cht-datasource), which is a proper suffix of the real path but has a slash, so the basename rescue skipped it. Now any proper suffix resolves. A WRONG directory is still a defect: components/analytics/x.html is not a suffix of modules/analytics/x.html, and the test pinning that stays green. - path-exists ON AN EPIC CHILD. Files a child created are absent from the squash tree when the epic renamed them before landing, so ls-tree refuted paths the PR demonstrably added. It now falls back to the PR's own file list and says the file is absent from the squash but added by the PR. Verified live: 9232's rules-engine.service.ts and targets.less ground, 10390's test/qualifier.spec.ts and api/tests/mocha/controllers/ target-interval.spec.js ground, and an invented path still refutes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The last residual on configuration and messaging, and the third member of a family already fixed twice. Drift is "true at the anchor, gone from master". Forward-scoping is "false at the anchor, true on master". This is "false at the anchor, true at the anchor's parent" - a Problem or Root Cause section describing the state the PR CHANGED. 10604's Root Cause says the languages service "queried medic-client/ doc_by_type with key ['translations', true]", which is precisely what the fix deleted, so probing the post-fix anchor refuted a correct sentence. 10073's names the parseResponseBody helper its PR removed. Both are now grounded at anchor^, with the evidence saying why; a symbol absent from both trees is still ungrounded, and the fabrication control still fails. Scope is assigned by section, not guessed: sectionOfQuote locates a claim's verbatim quote and reads the enclosing heading, so only Problem and Root Cause get the parent-tree retry. Solution, Code Patterns and Testing keep being judged at the anchor, where they belong. Also applied the disclaimer filters to EVERY claim rather than only enumerated ones. The model read 4278's "was not untested" sentence and produced a file-touched claim from it; the enumerator would have skipped that line. quoteDisclaims now runs over the merged set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ster union Three fixes, the first found by asking why configuration's claim total fell from 113 to 91 between runs. 'INSTEAD OF' AND 'RATHER THAN' ARE NOT ABSENCE. I had put them in the absence-context list, where they suppressed every claim on their line. Measured on the configuration batch they hit 13 code-bearing lines - more than every genuine absence pattern combined - because Design Choices is built out of "chose X rather than Y" and "reused X instead of building Y", and both name real symbols worth checking. That was pure lost coverage masquerading as precision, and it explains most of the drop. A sentence that really removes something says so with a verb the rest of the list already catches. A SENTENCE WITH NO CHANGE VERB DESCRIBES WHAT A FILE WAS. 4278 says the endpoint "was not untested: api/tests/unit/controllers/sms-gateway.js covered it at the unit level" - an existence claim about the file under test, which the model extracted as file-touched. When such a quote carries no change verb, no explicit status, and the path exists at the anchor, it is now read as the existence claim it is. A COLLAPSED CLUSTER MUST ASK ABOUT EVERY PR. 9553 names four PRs in source_prs and its Related Files span all of them, but only one sibling resolved to a local commit, so provider-wireup.spec.js - genuinely touched by #9569 and #9570 - read as untouched. checkFileTouched now takes the whole cluster and fetches each PR's file list. Verified live: 4278's endpoint and 9553's spec both ground, and an invented path in the same cluster still refutes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…dead lookup The first fully-trusted run (0 extraction errors on all three branches) left 13 ungrounded, and nearly all of them were filters that only ever ran on half the pipeline. MODEL CLAIMS BYPASSED EVERY FILTER. The enumerator drops bare filenames, strips call suffixes, skips this corpus's own frontmatter keys and refuses ellipsis prose - but it applies those rules while extracting, so nothing touched the LLM's claims. That is why emitter.nools.js, rules-engine.service.ts and provider-wireup.js were probed as symbols, and why Number() reached git with its parentheses attached. normaliseClaim now runs over the merged set. RELATED ISSUES GLOSSES DESCRIBE OTHER TICKETS. 9486 cites #9432, "Merge ensureTaskFreshness and ensureTargetFreshness into single event", and the model produced two symbol claims from that title. They were never claims about 9486's tree - and the irony is the gloss is only there because an earlier fix made it quote the real issue title. Symbol claims from Related Issues are dropped. THE BASENAME LOOKUP WAS DEAD. pathExistsAt fell back to a `*/name` ls-tree pathspec, which silently matches nothing: git ls-tree -r --name-only 88f9e463a -- '*/analytics.component.html' # empty git ls-tree -r --name-only 88f9e463a | grep -c '/analytics.component.html$' # 1 So every bare filename in a path-exists claim had been failing since the rescue was added. It now lists the tree once per ref, caches it, and filters in JS. 9277 and 10390's target-aggregates.service.ts ground as a result. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ting "change" as a verb The last two tooling findings, both narrow. BARE FILENAMES IN symbol-in-file. Prose attributes a symbol to a file by its short name - "handleIntervalTurnover in provider-wireup.js" - and no git pathspec resolves that, so the probe searched a path git could not find, came back empty, and reported the symbol as misattributed. symbolHits now expands a slash-free pathspec to the real path first. A genuinely wrong file still fails: asking for handleIntervalTurnover in target-state.js is still ungrounded. "CHANGE" IS A NOUN HERE. The existence rescue for file-touched claims is gated on the sentence carrying no change verb, and I had put bare chang(e|es|ed) in that list. In this corpus "change" is almost always a noun - 4278's sentence is "the api/src/ layout postdates this change" - so the one rescue that sentence needed was blocked by the word "change" inside it. Every remaining entry in the list is unambiguously verbal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A PATH ABSENT FROM THE DRAFT WAS INVENTED BY THE EXTRACTOR. 10390 writes "bespoke code in target-aggregates.service.ts" and "surfaced via analytics.getTargetDocs". The model supplied webapp/src/ts/modules/analytics/target-aggregates.service.ts and webapp/src/ts/services/analytics.service.ts - neither string occurs anywhere in the file - and both were then reported as defects. Probing a path the draft never wrote proves nothing about the draft. normaliseClaim now checks each claim's path against the raw text. If the draft writes only the basename, the claim falls back to it and the basename resolvers settle it; a symbol-in-file whose file was invented degrades to a plain symbol check rather than a misattribution verdict; and a path with no support at all is dropped. The obvious substring test was wrong and a test caught it: `index.js` contains the letters of `x.js`, so an invented path could be "rescued" onto an unrelated file. Matching now requires a real boundary. A CHANGE VERB ONLY COUNTS NEAR THE FILE. 4278's Root Cause names the endpoint and then, 200 characters later, says the missing tests made it "risky to modify or extend" - a statement about risk, not about what the PR did, but enough to block the existence rescue when the whole quote is scanned. The search is now windowed around the file mention. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…he budget Anonymous GitHub allows 60 requests an hour, and one sweep of three branches spends most of it on anchor resolution and epic-child file lists. The second sweep in an hour therefore came back with 9 unverifiable on configuration and 31 on messaging - not regressions, just the budget gone, with unresolvable anchors correctly degrading to unverifiable. But re-running after a fix is what this workflow does constantly, and a gate that reports differently depending on how recently it last ran is not a gate. githubApi now reads and writes a cache under outputs/verification/, keyed by API path. The data is immutable - a merged PR's file list, a closed PR's base branch - so keeping it between runs is safe. A null is deliberately NOT cached: a null is usually the budget running out, and persisting it would bake a transient failure into every later run as though the PR did not exist. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
NOT EVERY "file" IS A PATH. The model produced symbol-in-file with file = "analytics.getTargetDocs" - the dotted symbol itself. git searched for that as a pathspec, found nothing, and reported the symbol as misattributed to a path that never existed. A value with no slash and no file extension is not a path: symbol-in-file degrades to a plain symbol check, other kinds are dropped. TIME-SCOPING IS A PROPERTY OF THE DRAFT, NOT OF ONE SENTENCE. driftFor tested only the quoted line, so 9232 kept being flagged for a dead sidebar-filter component it annotates in Related Files and for permissions it retires in Design Choices - the extractor had simply quoted a different mention. A reader warned once is warned. driftFor now consults the whole draft via entityIsTimeScoped, which had been written and documented for exactly this and never wired in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…onale check-coherence filed a pair on 10371 whose `why` read "These do not conflict." The pair was counted, reported, and sent to a human to adjudicate two statements the model had already cleared. That noise is indistinguishable from a real non-recurring finding, which is the expensive kind: the documented standard is that recurrence proves a contradiction real while non-recurrence proves nothing, so every one-off has to be triaged by hand against cht-core. A withdrawn pair costs that triage and returns nothing. verifyContradictions now discards a pair whose rationale negates it, on the same footing as a fabricated quote or a sentence paired with itself. The predicate is deliberately narrow — "These conflict: ..." still counts as a finding — and is covered both ways in the spec. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
10390's Root Cause says target docs reached callers "via analytics.getTargetDocs, so there was no centralized, reusable, typed access path". Extraction pinned that symbol to shared-libs/cht-datasource/src/local/target-interval.ts, and the probe reported ungrounded. No tree can satisfy that pairing. The sentence describes the state before the fix; the file was added by the fix. Checking the parent — which the pre-fix path already does — cannot help either, because the file does not exist there. The verdict was reporting a real sentence as a fabrication. The file attribution is the artefact, not the claim: prose naming a symbol rarely names its home, so the model borrows a path from elsewhere in the draft. When a pre-fix symbol-in-file claim names a path the anchor ADDED, drop the binding and judge the symbol repo-wide at the parent instead. A modified file keeps its binding — there the pairing is answerable, so a miss stays a defect. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
11142 says "reducers/global.ts only gains a `search?: string` field on the
`TasksFilters` interface". That is true — the interface is declared at
webapp/src/ts/reducers/global.ts:292 — but the claim came back ungrounded
as "attributed to the wrong file".
basenameMatches exists because a `*/name` pathspec matches nothing in
ls-tree, so it lists the tree and filters on `p === file ||
p.endsWith('/' + file)`. That filter already handles a path tail. The
early `if (file.includes('/')) return []` above it meant tails never
reached it, so prose that shortens a path instead of dropping it entirely
got the worse verdict of the two.
Dropping the guard is safe: the suffix test is anchored on a leading
slash, so it matches whole path segments only — api/src/foo.js still
cannot resolve onto webapp/src/foo.js.
symbolHits now resolves whenever the pathspec is not LITERALLY in the
tree. It cannot use pathExistsAt for that test: pathExistsAt falls back to
basenameMatches itself, so with tails resolving it would report every
shortened path as present and skip the resolution it needs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
9718's Root Cause reads "The interval turnover mechanism in provider-wireup.js snapshotted the last calculation at interval boundaries instead of recalculating". Extraction offered "interval turnover" as the symbol, git grep -F -w found it nowhere in the file the sentence names, and the verdict came back as a misattributed symbol. It is prose. No identifier in any language this corpus covers contains whitespace, so normaliseClaim now drops a symbol containing any. Dotted member chains, which is what the filter has to be careful of, have none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
10230 says "Added a dedicated api/src/services/nepal-doit-sms.js service that encapsulates the Nepal DoIT gateway's API integration". Extraction turned the filename into a symbol and asked whether api/src/services/nepal-doit-sms.js contains `nepal-doit-sms`. It does not, and the verdict came back as a misattributed symbol on a branch that is already up for review. normaliseClaim now drops a symbol-in-file claim whose symbol is the stem of the very file it is checked against. Whatever such a sentence asserts, it is the file-touched claim the draft already makes in Related Files. Deliberately narrower than "drop hyphenated symbols": Angular selectors are kebab-case and drafts name them legitimately, so `overdue-filter` survives when the file is not overdue-filter.*. Only self-reference goes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sugat009
reviewed
Aug 3, 2026
sugat009
left a comment
sugat009
left a comment
Member
There was a problem hiding this comment.
Reviewing as a comment rather than a change request, since this is still a WIP draft and you have not put it up for approval. Flagging now because two of these affect whether the tool can do its job at all.
Headline: test/scripts/claim-probes.spec.ts contains raw NUL bytes, so git classifies it as binary. git diff --stat shows Bin 0 -> 41954 bytes and the API returns +0/-0 with no patch, which means the 77 tests in your largest spec cannot be reviewed by anyone. Worth fixing first, because it is also the spec that would demonstrate the two behaviours below.
I want to be fair about what this tool is up against. The methodology is right, and the drafts it targets do carry exactly the defects it enumerates. But the two heuristics below currently fail in the permissive direction, and the false-positive class they exist to suppress is real: your own b78ed35 on #120 retracted a fabrication count that turned out to be nested JSON keys. So the fix is not to make the checks stricter, it is to add a third verdict between grounded and absent.
CI is red at this head (Lint PR title on the WIP title, SonarCloud with 118 new issues), which I read as expected in-progress state rather than a defect.
For calibration, the four #123 items I filed today that a working version of this tool should catch: a draft saying a PR added files it deleted; a comparator claimed as a shared-libs/task-utils export when master has it in webapp/src/ts/reducers/tasks.ts; a UI label ("Previous month") that exists in neither tree; and a mechanism paragraph describing detection logic as missing when it pre-dated the PR. The first three are symbol and path claims squarely in scope.
| * a false positive during review; resolving the last segment turns the whole | ||
| * class into a grounded verdict that says how it was reached. | ||
| */ | ||
| function lastSegmentHit(ctx: ProbeCtx, ref: string, symbol: string, pathspec?: string): string | null { |
Member
There was a problem hiding this comment.
issue (important): lastSegmentHit grounds a dotted symbol whenever its final segment appears anywhere in the tree, with only a tail.length < 3 filter. Concrete failure: task.status (a real fabrication I caught by hand on the 10802 messaging draft, where the field is task.state) has tail status, which is everywhere in cht-core, so this returns a hit and checkSymbol reports grounded. The tool would wave through the exact class of defect it exists to catch.
The motivation in the docstring is sound and the false positives it names are real (smsparser.parse declared as exports.parse). The problem is the verdict it produces: it converts "cannot resolve this" into "grounded" rather than "needs a human". The evidence string is admirably honest about the weakness, but nothing downstream acts on that honesty.
Suggest a third outcome, e.g. tail-only, counted as unverified rather than grounded. Optionally require ancestry: after locating the tail, demand the preceding segment appear in the same file within a small line window, which keeps the exports.parse rescue and kills the task.status pass.
| if (hits.length > 0) { | ||
| return verdict(claim, 'grounded', `${cmd} → ${hits.length} hit(s), e.g. ${hits[0]}`, undefined, prov); | ||
| } | ||
| const tail = lastSegmentHit(ctx, ref, claim.symbol); |
Member
There was a problem hiding this comment.
issue: this is the repo-wide call site, where the rescue is least constrained. The checkSymbolInFile caller passes a pathspec, so its version is much tighter. If the third-outcome change above is too big for now, scoping this call the same way would remove most of the exposure.
| // PR did to it — 4278's "was not untested: api/tests/unit/... covered it" | ||
| // names the endpoint under test. If such a path exists at the anchor, the | ||
| // draft is right and the claim kind was simply mis-inferred. | ||
| if (status === undefined && !changeVerbNearFile(claim.quote, claim.file) && !claim.status |
Member
There was a problem hiding this comment.
issue: the existence rescue in checkFileTouched reportedly neutralizes the Related Files check for the bare-path format the corpus actually uses, so a file that exists but was never touched by the PR passes. That is the updateServiceWorker defect class, which checkSymbolInFile's docstring explicitly says a bare existence check waves through, so the pipeline guards it in one place and not the other. Worth carrying the claim's provenance (the enumerator already knows it came from ## Related Files) and refusing the rescue for those.
| @@ -0,0 +1,1045 @@ | |||
| /** | |||
Member
There was a problem hiding this comment.
issue: nothing in the pipeline records or checks a PR's base branch. That is how the corpus acquired 10767-feat10663-add-ui-extension-to-service-worker.md, already on main, which documents an appendUiExtensions hook from a PR merged to the feature branch 10224-ui-extensions and then abandoned; it never reached master, and appendUiExtensions is zero-hit there. gh-classify.ts already reads pr.base?.ref, so recording it on the anchor and flagging non-default bases looks cheap. Note 9232 and 10436 in the tasks-and-targets branch also merged to feature branches, so the pattern is common and only sometimes benign.
| @@ -12,6 +12,10 @@ jobs: | |||
| steps: | |||
Member
There was a problem hiding this comment.
issue: the gate is wired only under on: pull_request, so it never runs on push to main. Combined with --changed-only, that means a draft can reach main unverified if it lands any other way. Also: an empty --base silently degrades --changed-only into a full-corpus scan rather than erroring, and vocab-near-miss is the only heuristic that hard-fails CI with no suppression path, so a truthful draft using an unusual-but-real symbol fails the build with no override.
…e model `checkFileTouched` has always compared a claimed file status against the PR's real file list — "X was modified, not added" is one of its verdicts. It just never had a status to compare. The LLM extractor supplied one only when it felt like it, and `enumerate-claims`, the half that is supposed to make coverage reproducible, emitted every `file-touched` claim bare. So the check existed and did not run. That is the gap #10436 fell through for three review rounds. Its Testing section said a mocha harness "was added ... (webapp/tests/mocha/.mocharc.js, tsconfig.mocha.json, tsconfig.spec.json)" when the PR's own diff is deletions only — config.spec.ts +0/-103, tsconfig.mocha.json +0/-9, .mocharc.js +0/-2. Every symbol in the sentence is real, every path resolves, and the reviewer caught it by hand against `gh pr view --json files`. Inference is narrow on purpose, because a false "this is fabricated" costs more here than a missed check. The verb must precede the path, within ~90 characters and the same clause, and the path must not be a locating preposition's object. The last rule carries most of the weight: "added a `dbQuery` wrapper in pouchdb-provider.js" creates a symbol in a file the PR modified. A screen keyed naively on verb-near-path returns 64 hits over the tasks-and-targets batch, 63 of them that shape. The clause bound handles this corpus writing a paragraph per line, where an opening verb would otherwise govern every path after it. Measured on those 25 drafts: 6 statuses inferred, all confirmed by the PR file lists. Against 10436's pre-fix text it reports `.mocharc.js` claimed as added and actually modified — the reviewer's finding, from a deterministic probe. Also documents three blind spots this layer does not settle: symbol attribution (a real symbol introduced by a different PR — #10324 credited itself with `telemetryKey`, which #10371 added), counterfactual claims, and backticked placeholder templates no literal grep can match. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The backport probe searched the release branches for a commit referencing the draft's own PR number, or a PR number quoted in the sentence. cht-core stamps neither on a pick: it keeps the original subject, which names the ISSUE, and appends a bare "(backport)". So draft 9608's "cherry-picked as a backport to the 4.14.x release branch (PR #9610)" was reported ungrounded. The backport is real and is the tip of origin/4.14.x: 69ae8c0ab fix(#9604): fix integer validation in sms rules (backport) touching the same three files as the master commit. The probe searched (#9608) and (#9610); the pick carries (#9604). Mine the anchor's own subject for the reference it was stamped with — the one token a cherry-pick must preserve — and list every reference searched in the refusal message, so a miss is visible rather than silent. Verified the other two backport claims in the same corpus, which the probe had passed for the accidental reason that their picks do repeat the source PR: 8746 reaches 4.4.x (2b5cd23dd) and 4.5.x (f67e9d0c8), 9434 reaches 4.11.x (806b5906f, "(#9436)"). Also records two things found while grounding forms-and-reports: XLSForm column headers (instance::cht:duration, instance::cht:unique_tel) live only inside zipped .xlsx XML and can never be grepped, so the probe calls them fabricated when they are real; and the backport reasoning above. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tion Draft 9641 cites the `--skip-validate` upload that triggers its bug. The extractor takes that as a symbol, and the probe ran git grep -n -F -w --skip-validate <sha> which git answers with "error: unknown option `skip-validate'" and a usage dump. The throw was not confined to that claim — it propagated and killed the entire ground-claims run, so 41 drafts produced no report at all because one draft mentioned a CLI flag. Pass the needle after `-e` in both the fixed-string and regex searches. The test doubles now assert `-e` is present rather than just tolerating it, so the guard cannot be dropped without a red test, and there are two regression tests built from the real shape: one that finds the flag, one that reports absence instead of throwing. Found by ground-claims pass 3 on cht-agent#122, where prose written during this same review round was what first produced a dash-leading symbol. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The status inference was tuned on tasks-and-targets, where it produced 6 statuses and 0 false positives. forms-and-reports surfaced a prose shape it mis-reads: Regenerated 53 config form fixtures … plus the e2e and cht-form test fixtures (e.g. tests/e2e/default/contacts/forms/ngo-create.xlsx, tests/integration/cht-form/default/forms/dates.xml) "create" inside the fixture name ngo-create.xlsx matches ADD_VERB, sits within VERB_REACH of the next path, and inferred that dates.xml was ADDED. It is `M`, so checkFileTouched contradicted a sentence that is true — the expensive direction, since a false "this is fabricated" is what makes an operator edit correct prose into incorrect prose. Note "Regenerated" is deliberately absent from ADD_VERB, because regenerating a file that exists modifies it. The fixture's own name was the only thing in that sentence that looked like a create. Mask path-shaped tokens — unspaced, carrying a `/` or a file extension — before the verb scan, preserving length so VERB_REACH still measures real distance. Verbs contain neither slashes nor dots, so nothing real is hidden. Masking happens after the clause cut so CLAUSE_BREAK still sees punctuation. Two tests: the 8336 sentence must infer nothing, and a genuine "Added …/ngo-create.xlsx" must still infer 'added', so the masking cannot be widened into silence. The #10436 harness test still passes, which is the check that matters — this narrows false positives without giving up the defect the inference exists to catch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`source_prs[]` lists every PR a collapsed or hand-authored draft covers, but
`anchorFor` takes only the first entry as canonical. `file-touched` already
consulted the siblings; the tree-scoped probes -- `symbol`, `symbol-in-file`,
`path-exists` -- settled at the canonical anchor alone, so a symbol the fifth PR
introduced was judged at the first PR's commit, found absent, and reported as
fabricated.
Verified on contacts `9835`, which lists five source_prs and resolves to #10022,
the earliest. It returned 12 ungrounded claims and every one was false:
git -C $CORE grep -l assertSameParentLineage origin/master | wc -l # 5
git -C $CORE grep -c 'minifyDoc\|assertSameParentLineage\|getUpdatedContact' \
origin/master -- shared-libs/cht-datasource/src/local/libs/lineage.ts # 3
git -C $CORE grep -c assertPermissions origin/master -- api/src/controllers/person.js # 4
Every symbol is real, on master, in exactly the file the draft names; they
arrived in #10081/#10083/#10222/#10246. Tree-scoped claims now retry at each
sibling anchor before being called ungrounded, and the evidence string names the
sibling that settled them. A single-PR draft has no siblings, so no verdict on
one can change -- covered by a test.
Also guards paths under `agent-memory/`. A domain-note banner citing
`agent-memory/schema.json` was adjudicated against cht-core, which has no such
tree (`git ls-tree origin/master --name-only` -- no `agent-memory`), so a file
that plainly exists came back ungrounded on four drafts. Now `unverifiable`:
not our tree to settle.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`outputs/` is gitignored but was not eslint-ignored, so an ad-hoc `.ts` left
there by one investigation fails `eslint .` for everyone -- the file sits
outside tsconfig, so @typescript-eslint/parser rejects it before any rule runs:
outputs/audit/kinds.ts
0:0 error Parsing error: "parserOptions.project" has been provided ...
Two such files were failing the gate on this branch. Nothing under `outputs/`
is source; verification reports land there.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two false-positive classes, both found by running the gate on the contacts
branch after it was already clean by hand.
**A sentence that scopes itself to before the change was judged after it.**
`FORWARD_SCOPED` rescues "on master" claims by re-checking the current tree;
there was no mirror for the past. Problem/Root Cause claims are re-checked at
the parent, but only when extraction tagged them `pre-fix`, and it often does
not. `9394`'s Root Cause reads "before this PR contacts.effects.ts called
`getCurrentTargetDoc()` and forwarded exactly one doc" and came back ungrounded:
git -C $CORE grep -c getCurrentTargetDoc bbe5dedd5 -- webapp/src/ts/ # 0
git -C $CORE grep -n getCurrentTargetDoc bbe5dedd5^ -- webapp/src/ts/
# effects/contacts.effects.ts:182: .getCurrentTargetDoc(this.selectedContact)
# services/target-aggregates.service.ts:362: getCurrentTargetDoc(contact?) {
Real at the parent, deleted by the PR: the sentence is true and was reported as
fabricated. An explicit "before this PR" / "used to" / "previously" in the quote
now re-checks the parent, independently of whether extraction volunteered a
scope. Narrower than the section heuristic on purpose -- the marker has to be in
the sentence, not merely in a Problem section. A claim with no temporal marker
is not rescued; that is a test.
**A pair the model withdrew by downgrading it was still filed.** The existing
screen wants a negation next to the noun ("These do not conflict"). On contacts
`10804` the rationale read "a minor framing difference rather than a factual
conflict" -- no negation, so the pair was filed and a human sent to adjudicate
something the model had already cleared. This is the same defect the
"These do not conflict" screen was added for, in its other grammatical form.
"rather than a <conflict|contradiction|inconsistency>" now withdraws too.
`npm test` 1214 passing, `tsc --noEmit` and `eslint .` clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The online tier works without `gh` -- `gh-classify` falls back to anonymous `curl` -- which is why nobody noticed it is badly degraded that way. Anonymous gets 60 requests/hour and a single 37-draft domain exhausts it, so the run reports `unverified` counts that read exactly like content defects and exits 3. Measured on contacts, same bytes, same command: anonymous 37 drafts checked, 0 blocking, 1 warnings, 3 unverified rc=3 GH_TOKEN=... 37 drafts checked, 0 blocking, 1 warnings, 0 unverified rc=0 Three anonymous runs each left drafts unverified and needed an hour's wait between them. No code change is needed, only the token, so this is a runbook fix rather than a tooling one -- but the failure mode is invisible unless you already know to distrust an `unverified`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Third grammatical form of the same defect, third round in a row it has cost an
adjudication. The model cleared a pair on contacts `9177` with:
"this is a difference of issue-vs-PR attribution, not necessarily exclusive"
and it is right — #9065 is the parent issue, #9090 the PR that implemented it,
so "get-person existed (from #9065)" and "PR #9090 added the person get-by-uuid
datasource" are both true. But the rationale never says "conflict", so neither
SELF_NEGATING (negation beside the noun) nor DOWNGRADED ("rather than a
conflict") matched, and the pair was filed.
The pattern across all three variants is that the model withdraws by describing
the RELATIONSHIP, not by naming the noun, so the screens have to key on the
withdrawal phrase. "not (necessarily|strictly|mutually) exclusive" now withdraws
too.
Worth stating plainly since this is the third widening: each variant was found by
running the checker again on bytes it had already passed, not by reasoning about
the regex. I would expect a fourth.
`npm test` 1217 passing, `tsc --noEmit` and `eslint .` clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every symbol probe so far answers "is this identifier real". None answers "did
the PR you credited actually add it", and that is the question the corpus keeps
getting wrong: #10071 credited place-create to #10099 when #10065 and #10089 had
landed it the week before. Every symbol in the sentence existed, every section
agreed, and three consecutive coherence passes plus nine ground passes said
nothing, because nothing was asking.
`introduced-by` asks it, and is settled per-file rather than tree-wide. Both
simpler versions were implemented first and both produce false accusations on
this very history, which is why the comment records them:
"symbol exists at #N's parent" refutes #10065 introducing `createPlace` — a
true claim — because the old `places` controller had an unrelated function of
the same name elsewhere in the tree.
"symbol appears on an added line of #N's diff" accepts #10099 introducing it —
a false claim — because its one such line is an import edit in index.ts.
Locality separates them. For each file where #N adds a line naming the symbol,
ask whether that file already contained it at #N's parent. All of them did → #N
edited around something already there. Some file gained it → #N introduced it
there. Collisions in other modules cannot interfere because only the touched
files are read. Verified against the real history: #10099/createPlace ungrounded,
#10065 and #10089 grounded.
The enumerator only emits the claim when exactly one PR number governs a sentence
carrying a create verb. Two numbers ("via #10065 and #10089") is the shape a
CORRECT draft uses for work spanning PRs, so guessing between them would
manufacture defects; that case stays unverifiable, as does a PR whose diff never
names the symbol.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`vocab-near-miss` compares against a snapshot of TODAY's cht-core vocabulary, so a draft that describes history correctly gets flagged. Contacts `9835` quotes the report controller's permission list as it stood at #10222 -- `['can_view_reports', 'can_update_records']` -- and `can_update_records` is one edit from the live `can_update_reports`, so the check called a true, precisely scoped sentence a fabrication. Blocking, which makes the tempting fix "delete the history". The name was real: git -C $CORE log --all --oneline -S can_update_records -- api/src shared-libs config # b6470973b feat(#10041): remote update report implementation (#10200) <- added # a89955a9f feat(#9835): refactor create/update cht-datasource apis (#10522) <- renamed git -C $CORE grep -c can_update_records 8c92517ec -- api/src/controllers/report.js # 1 git -C $CORE grep -c can_update_records origin/master # 0 `verify-drafts` must stay hermetic to gate CI, so it cannot ask git about history. The snapshot answers instead: a family may now declare `historical` terms, which suppress a near-miss but are **never** offered as a suggestion -- kept out of `terms` precisely so the checker can never advise a reader to use a name that no longer exists. `historicalNote` records the commit that removed each one, so the list stays auditable rather than becoming a junk drawer for whatever tripped the gate last. Checked before the distance loop, so no input can surface a dead name as a suggestion; that is a test, alongside one proving a genuine typo still resolves to the live term and one proving a family without `historical` is unchanged. `npm test` 1227 passing, `tsc --noEmit` and `eslint .` clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A drift note naturally reaches for the possessive — "master's `src/input.ts` is types-only", "not master's shape" — and FORWARD_SCOPED only knew "on master" and "current master". Without the marker the claim is judged at the anchor, where a master-era symbol correctly does not exist, so a correct drift disclosure reads as a fabrication. Three such claims on the contacts branch: `PersonInput` (10043), `assertHasValidParentType` and `minifyDoc` (10057). All three are real on master and absent at their anchors, which is precisely what the notes say. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I'm not sure we'll merge this, but I wanted to share the scripts that were created in the hopes of making the reviews of the memory PRs easier. They added some of the grep and git commands Sugat used to identify inaccuracies.
https://github.com/medic/cht-agent/blob/93d47baca49c7147cc6085236154ce653094cb37/docs/memory-draft-verification.md#layer-2-ground-claims