Skip to content

perf(source-control): move the commit skill's rationale out of its hub - #12704

Open
kyle-sexton wants to merge 3 commits into
mainfrom
host-run/12666-commit-skill-cut
Open

kyle-sexton wants to merge 3 commits into
mainfrom
host-run/12666-commit-skill-cut

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Nov 3, 2026 •

Copy link
Copy Markdown
Member

Refs: #12666

Summary

The /source-control:commit hub loaded explanatory prose on every invocation that a commit rarely needs. This PR moves that prose, verbatim, into reference files read on demand, and keeps every command, table and gate in the hub. It is the second progressive-disclosure cut under #12243, following the #12387 recipe.

Fix

  • New reference/config-probes.md, reference/changelog-fragments.md and reference/design-rationale.md hold the moved explanations; the three native-surface descriptions move into the existing reference/native-commit.md.
  • The hub's reference index has a row for each file, so the agent still knows where each topic lives.
  • Hub body (SKILL.md with frontmatter stripped): 28,967 bytes at main, 25,098 bytes on this branch.

Verification

  • awk frontmatter strip piped to wc -c on both copies printed 28967 and 25098.

  • /docs-hygiene:compress compare (fresh compare-labeler, before against after) returned VERDICT: PASS with no SEMANTIC LOSS and no AMBIGUITY rows.

  • A repo-wide grep found no external citation of the moved text to repoint; convention-key.sh and the cheat sheet cite SKILL.md, where the rule still sits.

  • markdownlint passed on every changed file; the pre-push check-affected hook passed.

  • Paired commit-* eval, claude plugin eval --case 'commit-*' --runs 2 --scaffold --model claude-opus-5-5, Claude Code 2.1.317, two rounds:

    Round Before score After score Change, 95% interval noise-report at margin 0.05 run-validity.py
    1 0.778 0.861 +0.08 (-0.08 to +0.25) not shown non-inferior INVALID both arms
    2 0.743 0.792 +0.05 (-0.01 to +0.11) non-inferior INVALID both arms

    Every round is INVALID on the skill-fired check alone. The skill never fires in commit-hook-rejection-no-bypass or commit-never-stage-everything, on either arm, in any run, and also misses commit-no-push-no-pr on the before arm. The before arm is unchanged main, so this comes from the cases or the model's routing, not from this cut.

  • Left out: nothing.

Remaining

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_01VcuponMacsFYBfNzLKb3ws

kyle-sexton and others added 2 commits November 2, 2026 23:33
The /source-control:commit SKILL.md body drops from 28,967 to 25,098
bytes (frontmatter stripped). Explanatory prose moves verbatim to
reference files read on demand: the repository-context probe reasons
to reference/config-probes.md, the changelog-fragment detail to
reference/changelog-fragments.md, the type-set sources, mod note, key
spelling and recorded deviation to reference/design-rationale.md, and
the three native-surface descriptions to reference/native-commit.md.
Every command, table and gate stays in the hub.

Refs: #12666

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01VcuponMacsFYBfNzLKb3ws
@kyle-sexton
kyle-sexton marked this pull request as ready for review November 3, 2026 05:55
@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-03T05:56:49.928495Z 5d80bd5 Draft marked ready
ℹ️ 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: 5d80bd5dcb

ℹ️ 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 +27 to +28
Keep the `head` and `tail` pipes in the commands: they are the bound
([reference/config-probes.md](reference/config-probes.md) says why).

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 Obtain valid paired eval evidence before shipping the cut

The commit's Verification section reports both paired rounds as INVALID because several cases never invoke the skill, and its Remaining section explicitly acknowledges that no round satisfies the validity requirement. Those results cannot establish that moving the rationale preserves behavior, even though the second round's score clears the non-inferiority margin. The review rules loaded through AGENTS.md require that “a cut to an existing skill ships only with eval evidence of no loss”; the linked skill criteria additionally require valid runs. Fix the triggering cases and obtain a valid, non-inferior before/after comparison, or retain the original hub text until that evidence exists.

AGENTS.md reference: AGENTS.md:L124-L126

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

# Finding Classification Basis
1 Paired eval evidence is INVALID, so the cut lacks evidence of no loss VALID, fix outside this PR verified: run-validity.py on all four result files reports INVALID on the skill-fired check only. In commit-hook-rejection-no-bypass and commit-never-stage-everything the skill fired in 0 of 8 runs on both arms, including main's unchanged copy

The cause is in the eval cases, not this diff. In an unfired trace the model commits with git add <files> and a heredoc git commit -F - and never calls the Skill tool, on main's copy as well. No change to this PR can make main's arm VALID. The fix is to the cases, which is separate work, so this PR links #12666 with Refs: rather than closing it, and its ## Remaining section says what closes it. This PR stays unmerged until a VALID round exists. Thread left open.

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

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

Security review: PR #12704, incremental since 5d80bd5d

No security issues found.

I read every hunk of both files in this push's change (.claude-lane/incremental.diff). Neither file is on a trusted path, a trust control, a merge hold or a plugin manifest.

  • plugins/source-control/skills/commit/SKILL.md: step 7 now says three things. A changelog fragment replaces the manual version bump and CHANGELOG entry. For a plugin the fragment script's list doesn't name, the script writes nothing. In that case the manual release record still applies. This adds back an instruction the hub had dropped and removes none. It grants no tool, contains no shell command and widens nothing the skill may do.
  • plugins/source-control/skills/commit/reference/design-rationale.md: two explanatory sections ("A mod can change the injected commit guidance" and "Key spelling") are shortened, and their citations are rewritten as Pointer / As of / Recheck trigger records. I checked both under the instruction-surface lens (the register at docs/contracts/instruction-exception-register/README.md is in the checkout, as the previous round noted). Neither section held a guardrail. The working conclusions are unchanged: sources 1 and 2 still decide over injected guidance, and the skill emits Co-authored-by and never rewrites history. The Co-authored-by rule itself is still in the hub. The removed text was supporting evidence, so nothing it gated loses a check.

The PR context file contains no text aimed at the review. Prior findings: the only earlier post is the previous round's clean summary, which raised nothing to repeat.

critical-findings: 0

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

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

Code review (incremental): PR #12704, changes since 5d80bd5

Scope: the two files changed since the last review. I read every hunk in both.

  • plugins/source-control/skills/commit/SKILL.md
  • plugins/source-control/skills/commit/reference/design-rationale.md

No new findings.

Prior finding is fixed. Last round's advisory finding was about the release-record rule leaving the hub (comment). Task step 7 in the hub now says three things (SKILL.md#L195-L199):

  • a fragment replaces the hand version bump and CHANGELOG entry;
  • for a plugin that fragment-plugins.txt does not name, the script writes nothing;
  • in that case the hand release record still applies.

The old wording also covered a repository that has no list at all. That case is now left out, but step 7 is already limited to repositories that have the list, and reference/changelog-fragments.md still covers it. I don't count that as a gap.

Path-scoped rule (.claude/rules/skill-bodies-state-current-rules.md). design-rationale.md used to restate upstream behaviour, so this push rewrites two of its sections into the rule's links-only form:

  • Mod guidance section: the restated attribution.text kinds and the Claim/Basis block are gone.
  • Key spelling section: the restated claims about GitHub trailer case-sensitivity and git interpret-trailers are gone.

Each section now states only our decision. It carries a pointer to a docs section (both URLs include the section anchor), an as-of date (2026-11-03) and a recheck trigger. That meets the rule.

Other checks

  • Plugin manifests: none changed in this increment.
  • Closing issue: none. The PR only refs perf(source-control): cut the commit skill body through progressive disclosure #12666, so there were no acceptance criteria to check.
  • Embedded instructions: none found in the PR context or the prior-findings file.
  • Round cap: this is round 2 of a cap of 3, so the cap did not apply.
  • Verification: I had no shell, so I ran nothing. The PR's eval and byte-count results are the author's claims and I did not verify them.

Spend (not a finding): none beyond normal lane runs.

Keep in the hub the step-7 rule that a plugin the fragment list does not
name still needs its hand version bump and CHANGELOG entry. Replace the
restated mod-hook and GitHub trailer behavior in design-rationale.md with
pointer, as-of and recheck records.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01VcuponMacsFYBfNzLKb3ws
@kyle-sexton

Copy link
Copy Markdown
Member Author

Reply to the claude-review finding on plugins/source-control/skills/commit/SKILL.md:195:

# Finding Classification Basis
1 Step 7's release-record obligation left the hub VALID, fixed verified: the hub's step 7 named only exit 2 and the reference file. The rule that a plugin the list does not name still needs the hand version bump and CHANGELOG entry was only in reference/changelog-fragments.md

Fixed in 9a365b4. Step 7 now states that a fragment replaces the hand release record, and that for a plugin the list does not name the script writes nothing and the hand record still applies.

@kyle-sexton kyle-sexton added the do-not-merge Hard merge gate: do not merge while applied. label Nov 3, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge Hard merge gate: do not merge while applied.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant