ums: don't record an identifier that appears nowhere else - #728
Merged
Conversation
From ai-config#727's own review round. That PR's `memories/github.md` bullet named `mcp__github__list_commits`, which appeared exactly once in the corpus with no registry entry. The reviewer's point generalizes: a lone mention reads identically whether it is correct or hallucinated, so a later session has nothing to check it against, and the guidance is only actionable if the name is right. Worth stating because the obvious defense doesn't survive: the tool name *was* verified, by actually calling it in the session being written up. That evidence is real but dies with the session, so it can't anchor anything for a later reader. Registering the identifier can. Adds it as an anti-pattern, with the concrete routing (a cross-model tool goes in `tool-mappings.yml`; anything else cites where it was confirmed). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JCwW1H9AFVc8J1eRcobG32
This comment has been minimized.
This comment has been minimized.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
The insertion landed between the `memories/repo/<repo>.md` bullet's opening line and its eight-line continuation, so Markdown re-parented that continuation onto the new bullet. The repo-lore guidance is the remediation for the retired `memories/repo/` pattern and has nothing to do with identifier anchoring. This is precisely the pitfall the *next* bullet in the same list describes, added after review caught the same mistake on ai-config#335. Its stated remedy -- read the lines around the insertion point, or place the bullet after the complete enclosing item -- is what should have run here. Reading only a fixed window below the target's first line is what missed it: the continuation sat past the end of that window. The diff against main is now purely additive (11 insertions, 0 deletions), which is the mechanical check that no existing bullet was disturbed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JCwW1H9AFVc8J1eRcobG32
Contributor
|
Claude finished @d-morrison's task in 3m 21s —— View job Verdict: Ready for merge
Confirming reviewThe prior verdict was Changes Requested (round 1 review, job run 30170991571) for one structural bug: the new bullet was inserted between the opening line and the multi-paragraph continuation of the Commit No new findings:
The prior finding is resolved. No new issues. Ready for merge. |
Contributor
2026-07-25 20:40 UTC |
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
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.
Post-merge UMS from #727's own review round. One learning, one bullet.
The learning
#727's
memories/github.mdbullet namedmcp__github__list_commits, which appeared exactly once in the corpus with no registry entry. The reviewer flagged it, and the point generalizes past that one tool: a lone mention reads identically whether it is correct or hallucinated, so a later session has nothing to check it against — and the guidance is only actionable if the name is right.What makes this worth writing down is that the obvious defense doesn't survive contact. The tool name was verified — I called it with
sha: gh-pagesduring the session and got back real deploy history. But that evidence lived only in a session that has since ended, so it anchored nothing for anyone reading the bullet later. Registering the identifier does.Added to
skills/ums/SKILL.md's anti-patterns, with the routing: a cross-model tool goes intool-mappings.yml(then regenerate); anything else cites where it was confirmed.Stopping the recursion here
This is UMS-of-a-UMS-PR, which could obviously go on forever. It stops at this one: the learning came from a genuine review finding on real content, and the next merge in the chain would only teach "the process worked," which isn't worth a file.
A note on my own compliance
Two things this session got wrong against rules already in this skill, both worth stating plainly since the skill is what I'm editing:
git add -A— the anti-pattern list already forbids it, and I used it for both of ums: capture learnings from serocalculator#392 #727's commits. Benign there (the tree held only my own edits plus intentionally-regenerated files), but the rule exists so that it stays benign by construction, not by luck. This PR stages the single file by path.ascii-punctuation-in-sourcebans them in tracked source. I introduced five in ums: capture learnings from serocalculator#392 #727 and one more here, catching each only via a post-hoc scan. The scan works; writing them in the first place is the habit to fix.Checks
validate-skills.pyandcheck-links.pypass. Added lines scanned for banned punctuation (0) and multi-sentence lines (0).🤖 Generated with Claude Code
https://claude.ai/code/session_01JCwW1H9AFVc8J1eRcobG32
Generated by Claude Code