Add decompose-pr skill#649
Draft
kmontemayor2-sc wants to merge 4 commits into
Draft
Conversation
Teach future Claude how to break a large feature branch into a series of self-contained PRs that each base directly on `main` — never on each other — so reviewers can evaluate each one independently and the queue keeps moving. Skill body covers: identifying the atomic unit before splitting; classifying each change category (protos, configs, refactor, new utility, behavior, tests, docs, unrelated cleanup) with a per-category rule; ordering without stacking via the orphan-code / feature-flag / deliberate-duplication escape-hatch hierarchy; concrete git workflow including the `git restore --source -p` escape for interleaved same-file changes; self-contained PR descriptions that forbid forward references like "Depends on #N" or "Final integration PR"; and presenting the user an explicit review order with one-line reasoning per PR. Includes a Common Mistakes table and Red Flags list seeded from baseline subagent pressure tests, plus a worked example that walks through a hypothetical 940-LOC mixed-concerns branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
User clarified that the no-stacking rule was being interpreted too strictly. Git base must be `main`, but a dependent PR's branch may (and should) contain cherry-picked copies of predecessor PRs' commits so the PR builds, type-checks, and is reviewable standalone. The output of decomposition is a dependency DAG of PRs, not a linear stack. When PR_A merges, dependent branches rebase onto the updated main and Git drops the duplicated commits automatically — each dependent PR's diff "cleans up" as predecessors land. Both PRs are independent merge candidates in GitHub. Edits: - Intro and core principle: clarify git-base vs diff-content distinction. - Step 3 (Order via a dependency tree, not a stack): full rewrite. Replaces "land A, rebase B, then review B" with the simultaneous-open / cherry-pick / rebase-after-merge model. Adds ASCII DAG diagram. Removes the "diamond is a smell" claim; diamonds are fine under this model. Names the real smells: deep chains, high in-degree, many roots. - Step 6: output format updated to render a dependency tree plus a suggested review order that respects it. - Worked example: adds a "Cherry-picks" column; renders the DAG explicitly; shows the integration PR cherry-picking all predecessors. Notes the transient diff bloat and how rebase resolves it. - Quick Reference: rewords the "Can two PRs be based on each other" row to specify git-base; updates the PR-B-needs-PR-A's-code row; adds a row describing the DAG output. - Common Mistakes: updates the "stack them" entry to explain the cherry-pick alternative; adds entries for "PR_B can't be opened until PR_A merges" and "a diamond is a smell." - Red Flags: tightens the "Never" entry to specify GitHub-stacking. Verified by a fresh REFACTOR subagent pressure pass on a synthetic diamond-dependency scenario (BatchSpec → sampler + loader → trainer); subagent produced a clean DAG, cherry-picked predecessors, kept `--base main`, and correctly did not flag the diamond as a smell. `uv run mdformat --check .claude/skills/decompose-pr/SKILL.md` passes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Five changes per user notes annotated in the skill: 1. Plan-only is now the default. The skill stops at Step 6 with the dependency tree + per-PR breakdown — no branches created, no cherry-picks, no PRs opened — so the user can workshop the breakdown before any git work happens. New Step 7 "Execute" runs the git commands, gated on the `--execute` flag or explicit user confirmation. `gh pr create` is per-PR confirmed. 2. Removed Section 1 "When to use, when not to" entirely and tightened the `description:` line to "Invoke only when the user explicitly asks." The skill must not auto-trigger on diff size or perceived complexity. 3. LOC is now framed as a heuristic, not a rule. A complex 600-LOC single algorithm is fine atomic. Tests don't count toward the LOC budget (they still ship with their code, but a 100-prod + 250-test PR is a 100-LOC PR). 4. Reversed the configs-go-separate rule. New configs bundle with the consumer that reads them — full stop — across all config locations (deployment/configs, task configs, examples). The only config-only PR is one where the config change is the only change in the branch. Removed two Common Mistakes entries that reinforced the old rule; added a new entry calling out the old rule as obsolete. Removed the "Bundle resource/task configs with the feature code" Red Flag. 5. Updated the worked example: PR_E (separate configs PR) is gone; its content bundles into PR_F. LOC counts are production-only with test deltas noted separately. The DAG is now 5 PRs (A, B, C, D, F) with the diamond preserved at F. Also renumbered sections after removing the original Section 1. `uv run mdformat --check .claude/skills/decompose-pr/SKILL.md` passes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…, skip-blocks-descendants Three execution-correctness fixes from the Codex review (issues #2, #3, #4 — all High): 1. Clean-worktree preflight in Step 7. Before any `git checkout` in --execute mode, require `git status --short` to be empty. Tracked local edits can otherwise follow a checkout into the new decomposition branch and get staged or committed by mistake. Skill now stops and asks the user to stash / commit / discard before continuing. 2. Cherry-pick semantics: each PR's OWN commits, exactly once. The previous phrasing ("cherry-pick predecessor commits") was ambiguous and could be read as "cherry-pick the range `main..decomp/C`". That range includes C's own ancestors (e.g. shared root B), so picking it after already picking B directly causes B's commits to replay and conflict. The skill now says explicitly: cherry-pick each ancestor PR's OWN commits — never full branch ranges — in topological order. Shared ancestors (top of a diamond) get cherry-picked exactly once total per dependent branch. Updated: - Step 3 basic-build example to use `<PR_A's OWN commits>` and add a paragraph spelling out the rule. - Step 3 diamond paragraph: "each ancestor PR's own commits exactly once, in topological order." - Worked example F construction: per-line comments noting that C and D each contain B's commits but we don't re-include them. - Step 7 bash block to mirror the same convention. 3. Skipping a PR may block its descendants. Per user clarification: skipping a parent PR defers the entire subtree rooted at that PR. Descendants can't be merged either — their cherry-picked copy of the predecessor's code is unreviewed at HEAD until the predecessor lands. Only independent siblings remain unblocked. Skill now explicitly tells the user "Skipping #N defers #M and #K (its dependents) too." Also added three Common Mistakes entries covering each of the above so the rules survive deadline-pressure rationalization. mdformat clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adding skill so we can easily breakup large changes / features into smaller prs more easily.