fix(validate-doc-claims): stop reporting session ids as fabricated SHAs - #1608
Merged
Conversation
…s cited as a commit
The doc-claims validator matched every 7-40 character hex word in a doc
body and reported the ones that did not resolve as fabricated commit
SHAs. Its only guard required one digit and one a-f letter, which a
random hex identifier satisfies almost always, so session ids, content
hashes and blob hashes were all reported as defects — and a check that
reliably flags a legitimate citation format teaches its reader to skim
past it, which is where a real fabricated SHA gets through.
Resolution is proof of a commit, so reachability classification is
untouched. Only the "does not resolve" branch is gated: a hex word is
reported when its same-line context presents it as a commit — a word
naming a commit or a commit operation among the last three, a sentence
attributing something to it ("landed in", "resolved by"), or the
owner/repo@<sha> pin form. Command flags drop out first so
`git show --format=%H <sha>` reads like `git show <sha>`.
The attribution rule replaces a list of verbs. A list needs a new entry
for every phrasing someone writes; the preposition already carries the
attribution, so the verb before it needs no list of its own. Words that
name some other git object, or any hash, are deliberately absent from
the commit vocabulary — "the blob's sha is <hex>" is the very thing the
old flag mistook for a commit.
The manual-fallback sentence in grounding-validation.md now describes
the same rule, so the two paths agree on a platform without Python.
Fixes #1591
Claude-Session: https://claude.ai/code/session_01HSahUJjXzZpJt5ztL7CUyt
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a5e5b5169
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…xicon Second false-positive class on the doc-claims validator, and the first (placeholder content read as drafting scaffold) was never written down — so working this one meant reconstructing the earlier fix from git log. A third is already open on the same script's path check. The doc carries both halves: a detector that matches a syntactic shape must check whether the context makes the token that thing, and the context test must be stated as a condition rather than as the list of phrasings review has found so far. The repo already says the second part for skill prose; this episode is the same failure in Python, which is why the doctrine's stated scope is narrower than the principle needs. Adds a CONCEPTS.md entry for case accretion. Two existing entries lean on "case list" without it being defined anywhere, and the accretion loop is already live vocabulary in the repo's agent instructions. Claude-Session: https://claude.ai/code/session_01HSahUJjXzZpJt5ztL7CUyt
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1fc6db12fa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…tion Review caught that dropping the verb requirement traded the original defect for its mirror image. A preposition carries attribution but not attribution to a commit, so "the content digest is recorded at <hex>" and "the session identifier was issued with <hex>" both read as commit citations — the same false-positive class the fix exists to remove, arriving from the other side. Nothing structural separates a verb that says a change landed from one that says an identifier was assigned; only knowing the verbs does. So the verb requirement is back, and this time the rule that decides membership is stated above the list: a verb belongs when it says a change landed in this repository, and does not when it says an identifier was assigned or a value stored. That is what the first version was missing — not the list, the rule. The captured learning said the opposite, so it is corrected here rather than left to teach a lesson this round disproved. Claude-Session: https://claude.ai/code/session_01HSahUJjXzZpJt5ztL7CUyt
The comment beside the branch already said the pin form is owner/repo@<sha>, but the code accepted any "@", so "user@abcdef12" and "release@b3d4f5a6c7" read as commit citations — hex identifiers after an at-sign are ordinary in account names and image tags. Same defect as the preposition round: a rule stated correctly in a comment and code beside it accepting more than the rule allows. Reading the rule was not enough; nobody compared it against the branch it governs. The learning doc gains that as its own point, since it is what two of these rounds actually had in common. Claude-Session: https://claude.ai/code/session_01HSahUJjXzZpJt5ztL7CUyt
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57e5ea6d24
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…accusing it
Three review rounds each found a real boundary case of the same
instrument: a fixed-width lexical window over word lists, asked to
decide whether English prose attributes a hex token to a commit. It
cannot. The window is arbitrary and the vocabulary is open-ended, so
each fix exposed the next case while looking like progress.
The judgment already lived downstream. This script's own docstring says
its output is adjudication input, and an agent reads every line before
anything acts on it — but the output was worded as a verdict ("does not
resolve to a commit ... Replace with the PR number") for tokens that
were never commit citations. That wording is what made every missed
phrasing a correctness bug.
So the outcome splits by confidence rather than gating on it. Commit
context present stays a FLAG. Absent becomes a NOTE that says the script
cannot tell a session id or content hash from a commit, and notes leave
the exit code alone. Nothing is hidden and nothing false is claimed.
That demotes the cue vocabulary from a gate to a ranking heuristic: a
phrasing it misses costs one tier instead of dropping a fabricated SHA.
Only then was tightening safe, so the generic `git` cue — which
precedes every object kind equally — is gone, and the review round's
three findings became notes rather than defects. The pin boundary is
fixed too: a backticked or parenthesized owner/repo@sha is a pin.
A token noted on one line and cited on another now reports the citing
occurrence, which the earlier dedup would have swallowed.
Adjudication prose in both skills learns the two tiers. The captured
learning is rewritten around them and renamed, since it argued for
tuning the gate this replaces.
Claude-Session: https://claude.ai/code/session_01HSahUJjXzZpJt5ztL7CUyt
tmchow
added a commit
to Amanjyot-P/compound-engineering-plugin
that referenced
this pull request
Sep 1, 2026
…k saw The flag text now says the search is repo-scoped, but the Step 1 table is what the adjudicating agent actually reads to decide what to do, and its row still named one conclusion — "typo, or drafted from memory" — with "fix the citation or remove the claim". That is the global reading this PR exists to stop asserting, left sitting in the deciding position. The row now states what the observation supports and routes the agent to check the other stores and the citing sentence before treating the citation as wrong. Issue EveryInc#1545 measured this: across 767 backticked .md citations, 17 did not resolve and none was a mistake — six were correct paths on another machine, three pointed outside the search roots, and eight said in the same sentence that the target was gone or external. The manual-fallback sentence below the table carries the same global framing and is deliberately untouched here: it was rewritten on main in EveryInc#1608, so editing it on this branch would hand this PR a conflict. Claude-Session: https://claude.ai/code/session_01HSahUJjXzZpJt5ztL7CUyt
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
1 participant
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.
Summary
A learning doc that quotes a session id, a content hash, or a blob hash no longer collects flags saying those are fabricated commit SHAs. The validator matched every 7-40 character hex word and reported the ones that did not resolve, and its only guard — one digit plus one
a-fletter — is satisfied by almost any hex identifier, so a legitimate citation format reliably produced a defect-shaped flag. That is worse than noise: a check whose output is routinely wrong teaches its reader to skim, and a real fabricated SHA then passes with the rest.Resolution is proof of a commit, so reachability classification is untouched. Only the "does not resolve" branch is gated, on whether the hex word's same-line context presents it as a commit.
Fixes #1591
Two tiers, not a gate
An unresolvable hex word is reported at the confidence the script can actually defend:
FLAGFixed in commit <sha>,landed in <sha>,resolved by <sha>,git revert --no-edit <sha>,acme/widgets@<sha>NOTEThis is the third direction issue #1591 suggested, and it is the one that holds. The earlier attempts tuned a gate: a verb list that needed a new entry each round, then no verb list at all (which read
recorded at <hex>as a commit), then a narrower one. Each round found a genuine boundary case of a fixed-width lexical window over word lists — an instrument that cannot decide whether English prose attributes a token to a commit.The judgment already lived downstream. The script's docstring says its output is adjudication input, and an agent reads every line before anything acts on it. But the output was worded as a verdict for tokens that were never commit citations, which is what made every missed phrasing a correctness bug.
With tiers, a phrasing the vocabulary misses costs one tier instead of dropping a fabricated SHA — so the vocabulary could finally be tightened rather than extended. The generic
gitcue is gone (it precedes every object kind equally, soThe Git blob <hex>was a false accusation), the pin boundary now recognizes a backticked or parenthesizedowner/repo@<sha>, and a token noted on one line but cited on another reports the citing occurrence.Design decision: where the rule has to be honest
Two earlier rounds were the same defect wearing different clothes — a rule stated correctly in a comment, and code beside it accepting more than the rule allowed. The comment said the pin form is
owner/repo@<sha>; the code accepted any@. The comment said the phrase must attribute a change landing; the code accepted any preposition. Both rules were already written down and already right; nobody had compared them against the branch they governed. A stated rule the implementation quietly widens is worse than no rule, because it reads as settled.Validation
bun run release:validateandbun run plugin:validate --strictboth green.tests/doc-claims-validator.test.ts, each run against both byte-identical copies of the script — covering both tiers, the upgrade path, and every counterexample review surfaced.The manual-fallback sentence in
grounding-validation.mdnow states the same rule, so a platform without Python does not apply a different one.Unapplied review findings
gitcue could flag a non-commit object. Closed by removing it: with tiers,git show <sha>lands in the note tier rather than needing a cue that over-matches.Security Disclosure
No security-relevant changes. The script stays read-only, network-free, and pure standard library; the change only narrows which flags it emits.
Agent Disclosure
Claude Code · claude-opus-5[1m]Findings also came from an independent cross-model adversarial review run on Codex, which caught three of the six defects fixed during review.