Repository navigation
feat(docs-hygiene): rename-references auto-applies the Certain bucket and reports it - #12805
kyle-sexton wants to merge 5 commits into
Conversation
Lane-Activity: work-resolve-issue/resolve-issue
A lane run on the previous head could not get its lane token (lane-token-denied 503 github-rate-limited). This commit changes no file.
The lane branch merged cleanly onto main at a884f42. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vcupom3BKF6QuC5U3sRJPj
… eval Eval 49 renamed /verify, which is on the English-verb blocklist; a model following SKILL.md's force-ambiguous rule would bucket every match as Ambiguous and fail the eval for a reason unrelated to the Certain-bucket behavior it tests. It now renames /deploy-docs to /publish-docs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vcupom3BKF6QuC5U3sRJPj
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7aef80a83a
ℹ️ 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".
| - Mark every match accepted for Phase 5 without asking (these forms have empirically near-zero false-positive rate) | ||
| - Report the count and 1-2 example matches with the bucket counts, before the proceed/abort gate, | ||
| and say how to review the bucket one by one instead: answer "review Certain one by one" in the | ||
| reply to the gate or to any later Phase 4 prompt. The Certain matches are then presented per | ||
| match with 3 lines of context and the options "rename this", "skip this", "skip remaining |
There was a problem hiding this comment.
Run the skill's output evals before shipping the new flow
This changes the executable confirmation contract, but the commit's verification record explicitly says the evals were not run against a model. docs/review/skills-and-agents.md:6-8 requires compliance with the skill standard, whose skill-criteria.md:292-294 requires output remeasurement for a body change. The structural eval-quality check does not establish that the model preserves the proceed/abort gate or honors the new one-by-one override. Run the affected skill's output evals and record the results before shipping this behavior change.
AGENTS.md reference: AGENTS.md:L124-L126
Useful? React with 👍 / 👎.
|
claude-security-review reviewed Security review: PR #12805 (head 2f60e5a)Result: no security issues found. I read every hunk of all 3 files in the full diff:
None of these files matches a trusted-path glob. The diff touches no trust control, merge hold, token scope, kill switch, workflow or plugin manifest. Instruction-surface check. The change makes a standing skill instruction less strict: apply mode no longer asks a separate question before it applies the Certain bucket. I checked this by what happens when the rule is gone:
So the removed question did not guard an action in any protected class (irreversible action, secret handling, data integrity, external publication, legal compliance, agent authority). The Chain-context and Ambiguous per-match prompts are unchanged. Other inputs.
critical-findings: 0 |
|
claude-review reviewed Code review: PR #12805, rename-references applies the Certain bucket without askingI read every hunk of all 3 changed files. I also read the head copy of This PR now mostly repeats what main already shipped. The last review covered 5 files at
The PR description no longer matches the diff. It says "Eval 49 now renames
Closing issue #12592, "Done when" criteria (judged on head):
The No Not verified by me: the description's The prior entry (my earlier clean review at |
|
Unattended merge: no risk reason holds the merge at head |
Lane-Activity: pr-unblock/resolve-conflicts
| --- | ||
| bump: minor |
There was a problem hiding this comment.
Blocking: this fragment repeats a change that has already been released. plugins/docs-hygiene/CHANGELOG.md L3-L7 on head already has ## [2.5.0] - 2026-11-03 with this exact change: the Certain bucket no longer asks, the report gives the count, examples and one-by-one route, and the gate, Chain-context and Ambiguous buckets are unchanged (#12593, #12592). Main shipped it after this branch was cut.
If this fragment is merged, the next release will:
- bump docs-hygiene to 2.6.0 (
bump: minor), though nothing new reaches users. The only remainingapply.mdedits are the heading and the Phase 5 sentence, which add no behaviour; - add a second, near-identical
### Changedentry under 2.6.0.
Fix: delete the fragment. If you want the Phase 5 wording recorded, use a bump: patch fragment that describes just that wording change.
claude-lane: claude-review
| { | ||
| "id": 50, | ||
| "name": "certain-bucket-auto-applies-and-reports", | ||
| "prompt": "Rename the skill /deploy-docs to /publish-docs. The sweep finds only slash-token and path references.", | ||
| "expected_output": "Every match is a Form 1 slash-token or Form 3 path, so all of them land in the Certain bucket. The skill reports the bucket counts with the Certain count and one or two example matches, says the user can answer \"review Certain one-by-one\" to review them per match instead, and gates only the proceed/abort decision. It asks no separate \"auto-apply N matches?\" question for the Certain bucket; on proceed, Phase 5 edits every Certain match.", | ||
| "files": [], | ||
| "expectations": [ | ||
| "No `AskUserQuestion` asks whether to auto-apply the Certain bucket", | ||
| "The report gives the Certain count, one or two example matches, and how to review the bucket one by one instead", | ||
| "The proceed/abort gate still runs before any edit" | ||
| ] |
There was a problem hiding this comment.
Advisory: eval 50 repeats eval 49 with weaker checks. Eval 49 at L600-L613 (from main) already tests the same scenario: every hit is Certain, no Certain-specific AskUserQuestion, the report shows the count, 1-2 examples and the "review Certain one-by-one" route, and the edits happen after the proceed gate. It also checks the "skip remaining Certain" option and that the example lines appear only when the count is above 0.
Eval 50's three expectations are all covered by eval 49. If eval 50 fails, it can only fail for a reason eval 49 also catches, so it adds run cost and nothing new.
This eval was meant to replace eval 49's /verify token (see the PR description). The merge with main instead kept eval 49 as it was and added this as a new case.
Fix: either
- drop eval 50, or
- move its
/deploy-docs→/publish-docsprompt into eval 49 and delete this entry, if you still think the/verifyblocklist risk is real.triage.mdL83 and L115 suggest it isn't for slash and path forms.
claude-lane: claude-review
|
Superseded by #12604 (merged 2026-11-03T16:12:38Z), which made the same change to both rename-references/context/apply.md and context/triage.md, as #12592 asked (build the two together). Main's apply.md:54-55 already auto-applies the Certain bucket with no per-bucket AskUserQuestion and reports the count, examples and the one-by-one route. The Q3 eval is moot. |
Closes #12592
Summary
Replaces #12786, the work-resolve-issue lane's change to
/docs-hygiene:rename-references: applymode no longer asks a separate question before applying the Certain bucket; it accepts those
matches and reports their count, one or two examples and how to review them one by one. The
proceed/abort gate, the Chain-context prompt and the per-match Ambiguous prompts are unchanged.
The lane could not finish it: pr-unblock's address-feedback stopped with
feedback-untaken-threadson one review thread and labeled the PRneeds-session. This branchmerges #12786's head onto current main and settles that thread.
Fix
evals/evals.jsoneval 49): valid.verifyis on theEnglish-verb blocklist (
context/triage.md:92), andSKILL.md:189forces a blocklisted tokeninto Ambiguous regardless of position, so a model following
SKILL.mdcould bucket every matchAmbiguous and fail the eval for a reason unrelated to the Certain bucket. Eval 49 now renames
/deploy-docsto/publish-docs, which is not on the blocklist. The olderSKILL.mdversuspatterns.mdPhase 0 wording conflict the reviewer named is outside this change.Verification
scripts/check-affected.sh --base origin/main --no-fetch: 13 passed, 2 skipped over 5 changedfiles.
plugins/skill-quality/scripts/check-evals-quality.shon the rename-references evals: PASS; its4 warnings are on existing cases 27, 34 and 39.
Related
🤖 Generated with Claude Code
https://claude.ai/code/session_01Vcupom3BKF6QuC5U3sRJPj