Skip to content

feat(docs-hygiene): rename-references auto-applies the Certain bucket and reports it - #12805

Closed
kyle-sexton wants to merge 5 commits into
mainfrom
fix/12786-replacement
Closed

kyle-sexton wants to merge 5 commits into
mainfrom
fix/12786-replacement

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

Closes #12592

Summary

Replaces #12786, the work-resolve-issue lane's change to /docs-hygiene:rename-references: apply
mode 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-threads on one review thread and labeled the PR needs-session. This branch
merges #12786's head onto current main and settles that thread.

Fix

  • Merge: feat(docs-hygiene): rename-references auto-applies the Certain bucket and reports it #12786's head merged onto main at a884f42 with no conflict.
  • Review thread (claude-review, advisory, evals/evals.json eval 49): valid. verify is on the
    English-verb blocklist (context/triage.md:92), and SKILL.md:189 forces a blocklisted token
    into Ambiguous regardless of position, so a model following SKILL.md could bucket every match
    Ambiguous and fail the eval for a reason unrelated to the Certain bucket. Eval 49 now renames
    /deploy-docs to /publish-docs, which is not on the blocklist. The older SKILL.md versus
    patterns.md Phase 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 changed
    files.
  • plugins/skill-quality/scripts/check-evals-quality.sh on the rename-references evals: PASS; its
    4 warnings are on existing cases 27, 34 and 39.
  • Not run: the evals against a model.

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_01Vcupom3BKF6QuC5U3sRJPj

melodic-automation-lanes Bot and others added 4 commits November 3, 2026 08:02
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
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-03T09:36:10.433485Z 7aef80a PR opened
ℹ️ 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" or "@codex security review".

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 chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +54 to +58
- 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 2f60e5ab1bd5d919c39979bc166c10238979ec10: read 3 of 3 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #12805 (head 2f60e5a)

Result: no security issues found.

I read every hunk of all 3 files in the full diff:

  • .changes/docs-hygiene/work-resolve-issue-issue-12592-41586966486-1-affe032d.md: a new changelog fragment.
  • plugins/docs-hygiene/skills/rename-references/context/apply.md: a heading changes from "Per-bucket confirmation flow" to "Per-bucket flow", and the Phase 5 accepted-match sentence is reworded.
  • plugins/docs-hygiene/skills/rename-references/evals/evals.json: adds eval 50.

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:

  • A gate still runs. The proceed/abort AskUserQuestion gate still runs before any edit (apply.md#L48). The Certain count, examples and the one-by-one opt-out are shown in the report before that gate (apply.md#L55).
  • Nothing sensitive is involved. The edits are local, reversible and idempotent working-tree text substitutions. They touch no secret, publish nothing and grant no authority.

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.

  • The PR context file contains no embedded directives aimed at this lane.
  • The one prior entry is this lane's earlier clean summary at head 7aef80a. It reported no findings, so nothing is restated. The round cap (0 of 3) does not apply.

critical-findings: 0

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed 2f60e5ab1bd5d919c39979bc166c10238979ec10: read 3 of 3 changed file(s); 2 finding(s), 2 as inline comments.

Code review: PR #12805, rename-references applies the Certain bucket without asking

I read every hunk of all 3 changed files. I also read the head copy of context/apply.md, evals 49 and 50 in evals/evals.json, the top of plugins/docs-hygiene/CHANGELOG.md, and the blocklist lines in context/triage.md.

This PR now mostly repeats what main already shipped. The last review covered 5 files at 7aef80a8. After the latest merge of main, only 3 files differ. Main has already released this behaviour as docs-hygiene 2.5.0: see CHANGELOG.md#L3-L7, which cites #12593 and #12592. Main also has the Certain-bucket text in apply.md (L54-L55) and eval 49. What this PR still adds:

  • two wording edits in apply.md;
  • a second changelog fragment (finding 1);
  • eval 50, which repeats eval 49 (finding 2).

The PR description no longer matches the diff. It says "Eval 49 now renames /deploy-docs to /publish-docs". On head, eval 49 is main's version and still renames /verify to /verify-changes (evals.json#L603). The /deploy-docs case became a new eval 50. So the review-thread fix the description reports did not land on eval 49.

  • I don't count this as a finding, since those lines are main's and untouched here.
  • triage.md L83 and L115 limit the blocklist to Form 2 bare tokens and keep slash and path forms Certain, so the risk to eval 49 may be smaller than the description suggests.

Closing issue #12592, "Done when" criteria (judged on head):

  • apply.md's Certain bucket no longer calls AskUserQuestion; it auto-applies and reports. Met at apply.md L54-L55 (from main). The Phase 5 change at L78 agrees with it. Eval 49 and eval 50 both test it.
  • triage.md:31 matches it. Met on main; this PR does not change triage.md.
  • The proceed/abort gate, the Chain-context bucket and the per-match Ambiguous bucket are unchanged. Met: L48 and L57-L67 are untouched.

The apply.md edits read correctly: the heading "Per-bucket flow" and the Phase 5 "every Certain match not moved to one-by-one review" both agree with L54-L55.

No plugin.json or marketplace.json changed, so the manifest checks don't apply.

Not verified by me: the description's check-affected.sh and check-evals-quality.sh results are the author's claims. They were run before the latest merge of main. The description says the evals were not run against a model.

The prior entry (my earlier clean review at 7aef80a8) has no line anchor and had no findings, so nothing is repeated from it. There were no settled threads and no embedded instructions.

@melodic-automation-lanes

melodic-automation-lanes Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Unattended merge: no risk reason holds the merge at head 7aef80a83a8739fdd8ef89c21c9c81958cddd01d. The earlier refusal no longer holds.

Lane-Activity: pr-unblock/resolve-conflicts

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-review: 2 inline finding(s) on 2f60e5ab1bd5d919c39979bc166c10238979ec10; the summary is in this lane's pull request comment.

claude-lane: claude-review

Comment on lines +1 to +2
---
bump: minor

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 remaining apply.md edits are the heading and the Phase 5 sentence, which add no behaviour;
  • add a second, near-identical ### Changed entry 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

Comment on lines +614 to +624
{
"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"
]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-docs prompt into eval 49 and delete this entry, if you still think the /verify blocklist risk is real. triage.md L83 and L115 suggest it isn't for slash and path forms.

claude-lane: claude-review

@kyle-sexton

Copy link
Copy Markdown
Member Author

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.

@kyle-sexton kyle-sexton closed this Nov 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(docs-hygiene): rename-references apply auto-applies the certain bucket and reports it

1 participant