fix(release): require user-observable surface for minor bumps#953
fix(release): require user-observable surface for minor bumps#953carlos-alm merged 3 commits intomainfrom
Conversation
The release skill previously treated any `feat:` commit outside a fixed list of "internal scopes" as a minor bump. This misclassified extractor accuracy improvements (e.g. js-extractor callback resolution) and native engine perf refactors as minor, even though neither adds any new CLI command, flag, MCP tool, output field, or config option. Rewrite Step 1b so the conventional commit prefix is a hint, not the verdict: - Define the "real test" for a minor bump — the commit must add a new CLI command, CLI flag, MCP tool, output format/field, config option, or public programmatic API export - List what is NOT a minor bump even when prefixed `feat:` — extractor accuracy (→ Bug Fixes), perf refactors (→ Performance), internal plumbing (→ patch) - Add a decision procedure: read the PR body, ask whether new user- facing surface exists, re-categorize if not - Require stating the specific new surface before committing to a minor bump — if you cannot name it, the bump is patch
|
Claude finished @carlos-alm's task —— View job Todo List
Root Cause AnalysisProblem Being Solved: The Why This Approach: The fix correctly addresses the root cause by redefining what constitutes a minor bump. Instead of using scope exclusion lists, it requires identifying actual observable user functionality. This is the right approach - the problem was conceptual (wrong classification logic), not implementation. Risk Assessment: Low risk. This is a process improvement to documentation that will prevent incorrect version bumping. No code changes, just better decision criteria. Backlog Compliance
Technical AssessmentExcellent problem diagnosis and solution. The rewrite moves from brittle scope-based exclusions to semantic analysis of actual user impact. Key improvements: What Makes This Better (.claude/skills/release/SKILL.md:46-101)
Addresses the Root ProblemThe problematic commits mentioned in the PR description would now be correctly classified:
This fixes the exact issue where scope-based exclusions failed to catch feature-prefixed internal improvements. Code Quality
Final Assessment |
Greptile SummaryReplaces Step 1b's brittle Confidence Score: 5/5Safe to merge — logic is sound, the prior P1 concern about missing criterion #6 is resolved, and the one remaining finding is a minor formatting inconsistency. All remaining findings are P2 style suggestions. The decision procedure is internally consistent across the 6-point real-test list, the priority table, the decision-procedure question, and the "before committing" narrative. No correctness or data-integrity issues. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Scan commits since last tag] --> B{BREAKING CHANGE\nor ! suffix?}
B -- Yes --> MAJOR[major bump]
B -- No --> C{feat: or feat scope commit?}
C -- No --> PATCH[patch bump]
C -- Yes --> D[Read PR body or commit body]
D --> E{Passes real test?\nNew CLI cmd / flag /\nMCP tool / output field /\nconfig / public API export}
E -- Yes --> F[Name the specific surface]
F --> MINOR[minor bump]
E -- No --> G{What kind of change?}
G -- Accuracy fix --> BF[Bug Fixes in CHANGELOG → patch]
G -- Perf refactor --> PERF[Performance in CHANGELOG → patch]
G -- Internal plumbing --> CHORES[Chores in CHANGELOG → patch]
Reviews (2): Last reviewed commit: "fix(release): include public API export ..." | Re-trigger Greptile |
| | Condition | Bump | | ||
| |-----------|------| | ||
| | Any commit has a `BREAKING CHANGE:` / `BREAKING-CHANGE:` footer, or uses `!` suffix (`feat!:`, `fix!:`, etc.) | **major** | | ||
| | Any `feat:` commit passes the "real test" above (adds new CLI command/flag/MCP tool/output field/config) | **minor** | |
There was a problem hiding this comment.
Missing criterion in table and inline summaries
The "real test" in lines 50–57 lists 6 criteria, but the summary references in the decision procedure (line 69), table (line 78), and "before committing" narrative (line 96) all reduce it to 5, dropping item 6: a new public programmatic API export from index.ts. An agent relying on any of the three shorthands would silently miss this case.
| | Any `feat:` commit passes the "real test" above (adds new CLI command/flag/MCP tool/output field/config) | **minor** | | |
| | Any `feat:` commit passes the "real test" above (adds new CLI command/flag/MCP tool/output field/config/public API export) | **minor** | |
The same omission appears on lines 69 and 96 — suggest adding "or public API export" to both for consistency.
There was a problem hiding this comment.
Fixed in 1aeaff1 — added "public API export" to all three shorthand locations (decision procedure line 69, rules table line 78, and patch-bump narrative line 96) so the shorthand matches the 6-item real-test list above.
…ries (#953) The real-test list (lines 50-57) includes 6 criteria, but the shorthand summaries in the decision procedure, rules table, and patch-bump narrative reduced it to 5 — dropping the 'public API export' criterion. Add it to all three locations so agents using any shorthand see the full list.
Summary
The
/releaseskill was treating anyfeat:commit outside a fixed list of "internal scopes" as a minor bump. During the v3.9.4 release prep it flaggedfeat(js-extractor)(#947) andfeat(native)(#937) as minor, proposing 3.10.0 — but neither adds any new CLI command, flag, MCP tool, output field, or config option. The first is an extractor accuracy improvement; the second is a perf refactor.Rewrite Step 1b so the conventional commit prefix is a hint, not the verdict:
feat:— extractor accuracy (→ Bug Fixes), perf refactors (→ Performance), internal plumbing (→ patch)Test plan
feat:commits as patch