Skip to content

fix(file-pullreq): state Changes entries by what is now true - #184

Merged
ultimatile merged 2 commits into
mainfrom
fix/pr-body-changes-semantics
Aug 2, 2026
Merged

fix(file-pullreq): state Changes entries by what is now true#184
ultimatile merged 2 commits into
mainfrom
fix/pr-body-changes-semantics

Conversation

@ultimatile

Copy link
Copy Markdown
Owner

Summary

file-pullreq's ## Changes slot took a compressed plan checklist keyed to file paths, so an entry recorded which files an edit touched rather than what the change made true. An entry of that shape is a function of the diff's surface twice over: it repeats the changed-file list GitHub already renders beside the body, and any later edit to the diff falsifies it, so it has to be rewritten to go on saying as little. Entries now state the post-state, and the section that reports what was run is named for the act rather than for a plan.

Changes

A PR body's ## Changes is written as entries stating what is now true, with a read-back before the body is posted that catches any entry describing the editing instead.

The section reporting what was run is ## Verification, and it admits checks that are not a test suite.

Each research plan section reaches a body section by its own route: the Impact list folds into ## Impact, the Checklist of changes is rewritten into ## Changes entries, and the Test plan is reported against in ## Verification rather than folded — it names checks to run, while the section records the ones that ran.

Impact

Renaming the emitted ## Test plan section to ## Verification is the only change another skill could observe. rg -l "file-pullreq" --glob 'skills/**/*.md' --glob 'README.md' returns README.md, skills/file-issue/SKILL.md, skills/gh-body-conventions/SKILL.md, skills/review-pipeline/SKILL.md, and the skill's own file; none of the four others names the renamed section. review-pipeline invokes this skill and later appends ## Plan-vs-actual delta, which this change does not touch; gh-body-check, which this skill invokes, takes an artifact kind rather than a section vocabulary. The Test plan occurrences that remain in the repository belong to research's own plan body, which this change deliberately keeps and now cites by name.

Verification

Nothing here executes, and skills/gh-body-check/test-body-math-scan.sh — the repository's only test script — exercises that skill's math scanner, which this change does not touch.

  • pre-commit run --all-files reports mdformat Passed; it is the only hook .pre-commit-config.yaml declares.
  • In a git worktree checked out at this branch's commit, git status --porcelain prints nothing, so nothing untracked or modified stands behind the checks below.
  • There, jq -r '.metadata.version' .claude-plugin/marketplace.json prints 2026.8.2, and jq -e 'type == "object"' over that file exits 0.
  • diff between jq -S '.plugins[0].skills' on that worktree's manifest and on the same file at this PR's base commit prints nothing, so no skill was added or removed and the README table is unaffected.
  • pre-commit run --all-files reports Passed in that worktree with PRE_COMMIT_HOME unset, so no cached hook environment stands in for a clean one.
  • rg -n "Test plan" --hidden --glob '!.git' . returns three lines: two in skills/research/SKILL.md, which owns the plan section of that name, and one in skills/file-pullreq/SKILL.md, the line that now cites it.

Those runs were local; this repository declares no CI workflow that would witness them.

Notes

The ## Changes slot names no count and no unit, and file-pullreq's § Changes entries constrains only what an entry says, not how many entries there are. Fixing granularity would mean settling when two edits count as one change, which is a judgement about the work rather than about the text, so the rule shapes the entry and leaves the count to whoever drafts it.

Several skills each write out in full what a revised draft must re-run before it advances. Ten such sentences across five skills have no owning definition file; #183 enumerates them and is untouched here.

The `## Changes` slot took a plan checklist keyed to file paths, so an
entry tracked which files were edited rather than what the change made
true. Entries now state the post-state, and a read-back before the body
is posted catches any that describe the editing instead.

The section reporting what was run is named for the act rather than for
a plan, and admits checks that are not a test suite.

The plan-to-body mapping now gives each `research` plan section its own
treatment: the `Impact list` folds in, the `Checklist of changes` is
rewritten under the entry rule, and the `Test plan` is reported against
rather than folded, since it names checks to run while the section
records the ones that ran.

Copilot AI 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.

Pull request overview

Updates the file-pullreq skill’s PR-body guidance so the ## Changes section records post-state outcomes (not a diff-surface checklist) and renames the reporting section from ## Test plan to ## Verification to better match what the section contains.

Changes:

  • Reworks the PR body skeleton so ## Changes is expressed as “what is now true” entries, with an explicit read-back rule.
  • Renames the emitted section from ## Test plan to ## Verification and clarifies that non-test-suite checks may be reported there.
  • Bumps plugin version in .claude-plugin/marketplace.json from 2026.8.1 to 2026.8.2.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
skills/file-pullreq/SKILL.md Updates PR-body section semantics (## Changes entries) and renames ## Test plan## Verification in the emitted skeleton and related guidance.
.claude-plugin/marketplace.json Bumps marketplace metadata version to reflect the file-pullreq behavior change.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread skills/file-pullreq/SKILL.md Outdated
The plan-to-body mapping named a `research` plan's sections in backticks
and the body's own sections bare, in one sentence. Body sections now
carry the heading marker, as the rest of the file already writes them.

Copilot AI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

skills/file-pullreq/SKILL.md:49

  • This placeholder references “§ Changes entries below”, but the subsection is titled “Changes entries” (without a § label). Referring to it by name avoids confusion with external “§ ...” citations.
<entries — see § Changes entries below>

skills/file-pullreq/SKILL.md:33

  • Using the section sign (§) to refer to the local “Changes entries” subsection is ambiguous here, since the subsection isn’t labeled with a § and the same symbol is also used for external references (e.g., gh-body-conventions § Evidence claims). Consider referencing the local subsection by name instead.

This issue also appears on line 49 of the same file.

- **Whether the work went through `research`.** If yes: fold the plan's `Impact list` into `## Impact` when that section is kept; rewrite its `Checklist of changes` into `## Changes` entries per § Changes entries below; and report `## Verification` against its `Test plan` rather than folding that in. If no, derive `## Changes` and `## Impact` from the local diff and commits, and let `## Verification` report the checks this run made. Either way, do not transcribe the plan's process bookkeeping — see the reader-facing note below the skeleton.

@ultimatile
ultimatile merged commit 842a7c4 into main Aug 2, 2026
1 check passed
@ultimatile
ultimatile deleted the fix/pr-body-changes-semantics branch August 2, 2026 16:23
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.

2 participants