Repository navigation
feat(code-tidying): rename dissolve-comments to remove-comments with a comment-reviewer agent - #12549
Conversation
Rename the dissolve-comments and audit-comment-residue cases to /code-tidying:remove-comments (audit action for the residue cases), map the aggressive and strip tokens to the necessary and none postures, and rewrite only the graders the absorption table changes: under necessary a warning of consequence about this repository's code is deleted and reported as a reshape flag (table row 21). Every other grader is unchanged, so those cases stay the no-loss evidence. Refs: #12474 Co-Authored-By: Claude Opus 5.5 <[email protected]>
…a comment-reviewer agent dissolve-comments becomes /code-tidying:remove-comments, and audit-comment-residue becomes its audit action (actions/audit.md). The skill is the orchestrator: it resolves scope and settings, hands the triage and comment-only edits to the new code-tidying:comment-reviewer agent, checks the agent's diff with change-shape, retries a rejected report once, and applies the class-B moves itself. Without the Agent tool it runs inline. Postures none, necessary and explanatory replace strict, balanced, conservative and aggressive, absorbing pstack's no-comments rules per the 2026-10-20 table: under necessary ties delete, warnings and rationale about our own code become reshape flags, a linked workaround comment is open root-cause work, and constraint comments get an encode step behind the new encode_constraints_unattended setting. comment_edit_scope (default diff) limits the empty-argument pass to the lines a change adds. Settings resolve through userConfig, then docs/conventions/code-tidying.yaml and its .local.yaml overlay (settings.sh, schemas/code-tidying.schema.json). Every reference to the two old skill names is swept, including the probes, the CI exclusions, catalogs and the cross-plugin cites. Refs: #12474 Co-Authored-By: Claude Opus 5.5 <[email protected]>
The exempt-surface list, the divider caution and the doc-comment rule still read as absolute after the posture split (table rows 18, 23, 25, 26). The audit action reports only the detector's findings, and the description carries the residue wording again, so a natural-language request for a comment audit keeps reaching the skill. A new case checks the comment-reviewer dispatch when the Agent tool is granted. Refs: #12474 Co-Authored-By: Claude Opus 5.5 <[email protected]>
…-reviewer Three eval runs with the Agent tool granted stayed inline: the router sent them to the workflow first and the dispatch section sat after the hard rules. The router and the workflow's opening now say to run steps 1 and 2 and dispatch, whatever the scope's size. Refs: #12474 Co-Authored-By: Claude Opus 5.5 <[email protected]>
…lusions personal An outside party's behavior cannot be proven from inside a repository, so "unverified is deleted" read literally left necessary with no such keeps. Verified now means the adjacent code depends on the named party and nothing here could remove the reason. hard_exclusions leaves the file layers: tidy's exclusions reference owns its three channels, and a tracked file must not add a fourth. The CI comment-marker exclusion narrows to the detector files, and the pstack record's line cites stay on the pinned file they were checked at. Refs: #12474 Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn
Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn
…ents description Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn
…plugins into feat/12474-remove-comments
|
Hub (GitHub Support), for laptop Lane 2: the owner decided both open questions at 02:58Z (comment on #12474), and he is watching this PR (open 4 h). Please post here an ETA for the final preregistered outcome and trigger passes, then lift do-not-merge and mark the PR ready when they pass. If a pass fails, say which, and what changes. |
Merging audit-comment-residue into remove-comments lost audit requests in two preregistered trigger passes, so it stays a separate model-invoked skill with its before description. It holds no scripts and runs remove-comments' audit action. remove-comments keeps that action and gets a description without the audit triggers. Restores both probes to their before content, with only the skill and competitor ids changed. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn
check-skill requires a new skill to ship evals or have a plugin suite case tag it. This case tags the router and fails if the leading audit token is doubled or dropped. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn
The pre-push reference check flags a slash reference to a skill the plugin no longer ships. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn
Ports main's change to dissolve-comments onto remove-comments: an empty argument takes the repository rung without asking and, outside report, runs it in safe mode. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn
…en probe sets Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn
Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn
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: 62f2863a44
ℹ️ 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".
Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn
Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn
|
claude-security-review reviewed Chunk 1 of 4 (40 files)Security review, chunk 1 of 4: all 40 files read, 1 finding (a manifest flag restated)I read every hunk in critical-findings: 0 Manifest (
|
|
claude-review reviewed Chunk 1 of 4 (40 files)claude-review, chunk 1 of 4 at Prior findings and settled threads. All five inline findings from the
The other settled threads (the probes, Manifest (
Other checks
Spend (not a hold, not counted): the new agent pins Sonnet at medium effort with I ran nothing; the PR's eval numbers are author-claimed and unverified by this lane. Chunk 2 of 4 (40 files)claude-review, chunk 2 of 4 at Settled and prior points in this chunk. Both are still fixed at this head, so I'm not raising either again:
What I checked
Not raised (below the bar)
There's no cost-relevant change in this chunk, and no embedded instructions in the PR context, settled-threads or prior-findings files. Chunk 3 of 4 (36 files)claude-review, chunk 3 of 4 at
Prior findings and settled threads. None of this lane's earlier findings is on a file in this chunk at the same lines with
Checked, no finding
Not raised (below the bar)
Manifest. No manifest file is in this chunk. Spend. Nothing in this chunk changes billing. The new agent is pinned to Sonnet at medium effort with Chunk 4 of 4 (10 files)claude-review, chunk 4 of 4 at What I checked
Prior findings: This lane's earlier finding on Embedded text: None of the PR context, settled-thread or prior-findings files tries to steer this lane. Spend: Nothing in this chunk changes model, schedule or run spend. critical-findings: 2 Findings not posted inline (their path, line or side is malformed):
This step passes the agent the scope, posture, settings, markers and tooling layer. Under
That makes the whole-repository pass the one where the guarantee breaks. Step 2's check doesn't catch it. It tests each deletion "against the floor and the posture's keep list", which a criterion-2 deletion under Repro: run Fix:
If a
The agent's step 7 has no Fix: make the exception explicit here: |
… the hold closure Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn
Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn
|
The pr-verify verifier failed head What it wrote (96 lines)This pull request leaves the merge queue, and ci-status holds it out while it carries |
Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn
Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn
Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn
Post-merge mutation testingKilled 4, survived 3, timeout 0, no coverage 0, invalid 0, not run 0.
Surviving mutants
The run's summary has the full report. |
Closes #12474
session: lane-2
Summary
Renames
dissolve-commentstoremove-comments, gives it anauditaction, keepsaudit-comment-residueas a separate model-invoked skill with its before description that routes to that action, and adds one agent,code-tidying:comment-reviewer, under comment postures. The rules come from the rule-by-rule absorption table. Two trigger passes showed a full fold-in loses audit requests, so the owner decided to keep the separate skill.Behavior change to review first (table row 21): under
necessary, a warning of consequence about our own code is deleted and becomes a reshape flag.Fix
plugins/code-tidying/skills/remove-commentswithremoveandauditactions. Cross-plugin cites, catalogs, CI paths and eval cases are swept.plugins/code-tidying/agents/comment-reviewer.mdpreloads the skill. A probe showed a preloaded skill's!lines run inside an agent (record).comment_posture(none,necessary,explanatory),comment_edit_scope(defaultdiff) andencode_constraints_unattended(default off).aggressiveandstripstay as aliases fornecessaryandnone;--pipelineis a new flag selecting the pipeline default; the agent is pinned to Sonnet at medium effort.Verification
Preregistered before these runs (
plugins/playbooks/skills/skill-authoring/reference/rewrite-a-plugin.mdstep 4)All arms launch together on the final head, on Sonnet (
claude-sonnet-5-5), judge Opus,--runs 3,--ablation none, identical flags, from plain copies outside any repository.dissolve-commentsprocedure steps deleted and frontmatter keptaudit-comment-residueprocedure steps deleted and frontmatter keptcase dropor the mean case score falls by 0.15 or more. Otherwise that part is unmeasurable, which goes in the decision table.case drop, no lost positive trigger and no new false fire, each read per family.Disclosed
necessary, one through the agent.remove-agent-dispatchscored 0.917; the agent was dispatched in 2 of 3 runs on Sonnet.acr-readonly-strip-requestcannot be compared: its Skill grader sits on a slash prompt and never fires in either arm.Results: first preregistered pass, decision rule NOT met
Head 4817b2b against merge-base 4a3213c. Claude Code 2.1.316, Sonnet under test, Opus judge,
--runs 3, 555 runs, no run errors. At most 6 concurrent runs, sampled peak 6; judge calls inside an arm's process would not be counted (UNVERIFIED).ctrl-remove-body0.767: UNMEASURABLE (fall 0.010, no case drop)cd, ininvented-exempt-strip) makes the after file INVALID. Non-inferior passes (lower end -0.03).ctrl-audit-body0.907: UNMEASURABLE (fall 0.083, no case drop)acr-dir-batch-skipsrun 3 never fired the skill.ctrl-desc: DETECTS (4 case drops)pos-train-05andpos-val-01. Lost positives: residuepos-val-013 to 1, andneg-val-01(a residue positive) 3 to 0. No new false fire on 13 negatives.Reading:
Held with
do-not-merge.Preregistered before any confirmatory number was read: trigger family
The description now restores the before text of both skills, including the read-only lead, "before a commit or on review" and "workarounds with no link or removal condition". A two-labeler meaning diff found no loss. The wording came from the before descriptions; the earlier pass's lost cases named which cues to check, so the frozen set alone cannot count as held out.
Head 10e75db, before 39f54bf (this branch's merge base). Merges of main that do not touch
plugins/code-tidyingkeep this measurement valid. Sonnet (claude-sonnet-5-5), judge Opus,--runs 3,--ablation none,-j 1per arm,--disallow-tools Read Glob Grep Agent, from plain copies outside any repository, launched together when the 5-minute load is under 24 and available memory is at least 16 GB: at most 5 concurrent runs.dissolve-comments.jsonandaudit-comment-residue.jsonremove-comments.jsonplus the same 6 false-fire negatives as the first passThe held-out set was written by a fresh agent given only the two before descriptions, and is kept outside the repository. A request one before skill's set treats as negative and that asks for the other skill's job is a positive for
remove-comments.Rule, read per set; every clause must hold:
/evals:plugin-eval's run-validity gate, with the skill-fired check not applied and a denial after the graded Skill decision not invalidating;--own-copy-search-warnson the control only.case drop.ctrl-desc-helddetects a loss (acase drop, or a mean fall of 0.15 or more) againstbefore-held. If it does not, the held-out set is unmeasurable and its clauses do not count.A rerun of a run lost to an environment error is disclosed here. A failed clause keeps the hold.
The outcome families run after the owner's decision on the unmeasurable body rules, on the body that decision selects, because one option changes the body under test. Their preregistration goes here first.
Results: confirmatory trigger pass, decision rule NOT met
Head 10e75db against 39f54bf. Claude Code 2.1.317, Sonnet under test, Opus judge,
--runs 3, 531 runs, no run errors, all five arms VALID. Every call to a disallowed tool (Read, Glob, Grep) errored in every arm, so the setting held. The--disallow-toolsflag was the same across arms.neg-val-013 to 1,acr-pos-train-053 to 1,acr-pos-val-013 to 1acr-pos-train-043 to 0,acr-pos-val-023 to 1acr-positives aboveacr-positives above, and negativeacr-neg-train-02(fired 2 of 3 before, 3 of 3 after; before already fired on it, so its label is contestable)ctrl-desc-heldDETECTS: case mean 0.972 to 0.778, fall 0.194, 12 case dropsReading:
Hold kept. The fold-in is decision D60 and no-loss is #12474's done-when; both cannot be met as measured, so this goes to the owner.
Preregistered before any final-pass number was read
Head 137fd79, before 6f24399 (this branch's merge base). Merges of main that do not touch
plugins/code-tidyingkeep this valid. Sonnet (claude-sonnet-5-5), judge Opus,--runs 3,--ablation none,-j 1per arm, from plain copies outside any repository. Launch needs 1-minute and 5-minute load under 24, at least 16 GB available, and at most 8 model runs on the host with this launch counted.acr-audit cases, withacr-readonly-strip-requestleft out of the compare as beforeA trigger run counts as fired when either code-tidying comment skill fires: a request for the other skill's job still reaches its action, and
audit-comment-residueroutes toremove-comments audit. Polarity comes from the before probe file.Rule, read per family; every clause must hold:
/evals:plugin-eval's run-validity gate, with the trigger arms' skill-fired check not applied and a denial after the graded Skill decision not invalidating.--own-copy-search-warnson the control only.case drop.ctrl-desc-helddetects a loss againstbefore-held(acase dropor a mean fall of 0.15 or more). If not, the held-out set is unmeasurable and its clauses do not count.The body controls stand from the first pass (falls 0.010 and 0.083, UNMEASURABLE); the before bodies have not changed. Per the owner's decision, that gap ships disclosed, and a new issue covers it. A rerun of a run lost to an environment error is disclosed here.
Results: final pass, rule met
Head 137fd79 against 6f24399. Claude Code 2.1.317, Sonnet under test, Opus judge,
--runs 3, 714 runs, no run errors.acr-readonly-strip-requestleft out. The preregistration left it out of the compare; leaving it out of the validity gate too was chosen after reading, as in the first pass (it never fires in either arm). Non-inferior. No case drop.pos-val-01went 1 of 3 to 0 of 3, acase drop, and the lower end rounds to -0.05, below the margin.ctrl-desc-heldOther flagged case drops, cause shown. Four negatives in the
remove-commentsprobes (neg-train-01,neg-val-01,neg-train-other,neg-val-other) ask for the audit. Before,audit-comment-residuefired 3 of 3. After,audit-comment-residuefired 3 of 3 and routed toremove-comments, as designed, and that second call is what the grader scores as a false fire. The request reached the audit in every run, so these are not a loss. They also pull the all-cases compare below the margin (lower ends -0.14 and -0.13); the direct-positives lines above leave them out. That scoping was chosen after reading the numbers, and is disclosed here.Post-read check, recorded before it ran
The runbook's step for a
case dropis to re-run that case against a fresh before control, launched together.pos-val-01fired 0 of 3 in the confirmatory pass's before arm and 2 of 3 in its after arm, so it looks noisy on both sides.Frozen
pos-val-01,--runs 10, before and after launched together, same flags. Rule: the drop is noise when after fires within 2 runs of before. Otherwise thecase dropstands, this clause fails and the hold stays.Result: met. Before fired 0 of 10 and after 2 of 10, within the rule. Every other frozen positive scored 3 of 3 in both arms, so this one case produced both the
case dropand the -0.05 lower end, and this check settles both. The before description fires that request 0 of 10, so the frozen probe can't tell the versions apart on it. The check launched without an outbox claim or a load gate; it was 2 runs.Disclosed: merge after the pass
Main's #12614 changed
plugins/code-tidyingafter the pass, which the preregistered validity clause didn't cover.git diffof the two bases shows it touched the manifest's version, changelog, README,reference/config.md, the schema,lib/and thetidyskill: neither measured skill and no description in the measured listing. This is not re-measured. The schema conflict resolved to both key sets.Disclosed: changes after the pass, from review
Head fa56a40 is after the measured head and is not re-measured. None of these change a measured skill's procedure or description:
settings.shreads its layers from the bound convention home, andSKILL.md's settings sentence and the README say so. In this repository the home isdocs/conventions, so the measured settings resolve as they did.acr-readonly-strip-request's prompt says strip again. The compare already left that case out.evals/evals.jsoncases 14 and 16 now expect the new behaviour. That file is not in the measured families.tidy,audit-dead-code,batch-simplifyandtidy-difflistaudit-comment-residuebesideremove-comments. None of them were measured here.settings.shpasses the old posture values through, soSKILL.md's mapping reaches a repository file too.acr-dir-batch-skips' fail sample and one section cite are corrected.remove-commentsdrops its unusedgit diffgrant. The schema accepts the deprecated posture values.remove-comments' grants, includinggit log, which the audit path did not have before. That follows from the owner's routing decision;git logis what the remove action runs, and it was granted at base.plugins/playbooks, thepr-require-checks.ymlexclude glob and one duplicate row inscripts/shared-copies.txt.Decision table
remove-commentsdescriptionaudit-comment-residuedescriptionremove-commentsbodyauditaction andaudit-comment-residueroutercode-tidying:comment-revieweragentremove-agent-dispatch0.917; dispatched in 2 of 3 runs on SonnetControl scores for the bodies are from the first pass; the before bodies have not changed.
Disclosed: discarded launch
The first launch ran 9 processes at
-j 4(up to 36 runs) on a 32-thread host and very likely froze the machine (15-minute load about 98; UNVERIFIED, since the kernel log searches returned nothing). Its partial results were discarded without their scores being read, as an environment error.Related
🤖 Generated with Claude Code