feat(review): add Fowler design-smell baseline to code-reviewer (0.12.0) by kyle-sexton · Pull Request #297 · melodic-software/claude-code-plugins

kyle-sexton · 2026-07-17T20:13:28Z

Summary

Adds a named design-smell baseline to the code-reviewer agent — the review plugin's designated owner of design judgment automated tooling misses. Twelve curated smells from Fowler, Refactoring 2nd ed., ch. 3 (Mysterious Name, Duplicated Code, Feature Envy, Data Clumps, Primitive Obsession, Repeated Switches, Shotgun Surgery, Divergent Change, Speculative Generality, Message Chains, Middle Man, Refused Bequest), each as a terse what-it-is → how-to-fix line in our own voice.

Semantics, per the evaluation that greenlit the port (adapted from mattpocock/skills upstream PR #464):

Advisory only. Smells default to SUGGESTION at medium/low confidence; escalation happens only through a documented project rule — the rule carries the severity, the smell label stays advisory. This rides the agent's existing judgement-call contract.
Repo overrides. A documented project standard that endorses a flagged pattern suppresses the smell; suppression policy lives in the existing REVIEW.md / project-rules seam, NOT userConfig (rich prose policy; a per-smell knob matrix fails the Rule of Three). The plugin keeps zero userConfig.
Single home. Inlined in the agent definition — its only reader. fanout (every tier) and quality-gate (fallback + self-with-agent paths) inherit by dispatching the agent; the external pr-review-toolkit orchestrator path and self-mode general fallback don't reach it — documented limitations, not gaps.
The prior duplicated-structural-boilerplate bullet folds into Duplicated Code so the concept appears once.
Provenance: Fowler cited as primary source; no upstream phrasing copied. No live upstream — regeneration trigger is a Fowler edition revision to ch. 3 or a taxonomy change in the agent.

Review plugin bumped 0.9.0 → 0.10.0. No evals required (agent-scoped enrichment; this repo's evals are skill-scoped). No security-review re-trigger (no hooks, MCP, config, or egress).

Verification

claude plugin validate plugins/review ✔
markdownlint-cli2 on all touched markdown: 0 errors

🤖 Generated with Claude Code

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f53b2a87e2

ℹ️ 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".

claude · 2026-07-17T20:24:33Z

Claude finished @kyle-sexton's task in 2m 3s —— View job

PR Review: feat(review): add Fowler design-smell baseline to code-reviewer (0.10.0)

Tasks

Load review tools
Read changed files
Analyze PR changes for correctness, security, and alignment
Post review findings

Summary

The baseline itself is well-formed — 12 smells accurately sourced from Fowler ch. 3, each with a terse what→fix line in the plugin's voice, cleanly integrated with the existing advisory/judgement-call framing. No security surface, no egress, no config addition; version bump is appropriate.

One IMPORTANT gap and one SUGGESTION follow.

IMPORTANT — Fanout confidence mismatch (confirms Codex P2)

Root cause: The agent says smell findings default to medium or low confidence (line 58), but nothing in the agent's Output format section instructs it to actually emit a confidence label. The fanout parse contract for code-reviewer was not updated and still reads "Native confidence: —" — absent confidence maps to unscored at Stage 2, and Stage 4 ranks unscored above low:

Stage 4 rank: high > medium > unscored > low

So in practice, a smell finding from this baseline enters the fanout queue as unscored, not medium/low as intended. The advisory-light semantics expressed in the agent definition are invisible to the normalization pipeline, and a low-confidence smell can sort above explicitly low-confidence findings from other surfaces.

Fix: Add guidance to the Output format section directing the agent to emit a confidence label for smell findings (e.g., Confidence: medium or Confidence: low inline with each smell finding). Stage 0 already extracts native_confidence from free-text parse — no pipeline change needed if the agent emits the label. Optionally, update the per-surface parse contract table in findings-normalization.md to show that code-reviewer now emits confidence for smell-category findings.

Fix this →

SUGGESTION — "(see Output format)" cross-reference is a dead end

Line 58 ends with (see Output format) to explain how escalation works, but the Output format section (lines 60–64) says nothing about confidence or escalation. A reader following that reference finds no actionable detail. If the Output format section gains confidence-emission guidance (per the fix above), this cross-reference will resolve correctly; otherwise drop it or repoint to context/severity.md.

What looks good

All 12 smells accurately represent Fowler ch. 3 with correct attribution and no upstream phrasing copied
Advisory-only escalation is well-integrated with the existing judgement-call labeling (line 64 already says "design-smell and convention findings are judgement calls")
Override semantics (project standard endorses → suppress) ride the existing REVIEW.md/project-rules seam cleanly
Folding the old "Duplicated structural boilerplate" bullet into Duplicated Code removes the duplication of the concept
No userConfig surface added — correct per project design rules
CHANGELOG entry is complete and accurate about limitations (undocumented paths, no evals required)
Version bump 0.9.0 → 0.10.0 is appropriate for a minor additive feature
No security-review re-trigger warranted (no hooks, MCP, egress, or config change)

The smell baseline claimed medium/low confidence but the agent never emitted a label, so fanout normalization mapped smell findings to unscored — which Stage 4 ranks above low. The agent now outputs an explicit Confidence line per smell finding, and the fanout parse contract and Stage 2 crosswalk recognize it. Addresses Codex P2 on PR #297 (confirmed by claude-review). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

kyle-sexton · 2026-07-17T20:56:29Z

IMPORTANT finding (fanout confidence mismatch) fixed in 8aa214b — agent emits Confidence: medium|low per smell finding; parse contract + Stage 2 crosswalk updated. The SUGGESTION (dead (see Output format) cross-reference) resolves with the same commit: the section now carries the confidence/escalation guidance the reference points at.

The smell baseline claimed medium/low confidence but the agent never emitted a label, so fanout normalization mapped smell findings to unscored — which Stage 4 ranks above low. The agent now outputs an explicit Confidence line per smell finding, and the fanout parse contract and Stage 2 crosswalk recognize it. Addresses Codex P2 on PR #297 (confirmed by claude-review). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

kyle-sexton · 2026-07-17T21:27:50Z

Rebased over #291 (standards grounding), which took review 0.10.0 — this PR is now 0.11.0; CHANGELOG entry restacked above the 0.10.0 standards entry. No content overlap beyond manifest/CHANGELOG.

claude · 2026-07-17T21:40:42Z

Claude finished @kyle-sexton's task in 1m 42s —— View job

PR Review: feat(review): add Fowler design-smell baseline to code-reviewer (0.11.0)

Tasks

Load review tools
Read changed files (post-fix, post-rebase state)
Verify previous findings are resolved
Check for new issues introduced by rebase/version bump
Post final review

Summary

Both previous findings are fully resolved, and the rebase over #291 (0.10.0 standards entry) is clean — no content conflicts, version numbering is consistent throughout (plugin.json at 0.11.0, CHANGELOG ## [0.11.0] stacked above ## [0.10.0]). The PR is in a shippable state.

Previous findings — resolved

IMPORTANT (fanout confidence mismatch): Confirmed fixed in 82aa367. The Output format section (line 64) now instructs the agent to emit an explicit Confidence: medium or Confidence: low line per design-smell finding, with the why (downstream normalization ranking) inline — an agent with no explanation for the requirement tends to drift. The findings-normalization.md parse contract (line 11) and Stage 2 description (line 41) are updated to match — the pipeline now has a coherent two-case confidence path for code-reviewer.

SUGGESTION (dead cross-reference): The (see Output format) cross-reference at line 58 now lands on the confidence and escalation guidance it was pointing at. Resolved as a side-effect of the same commit.

What looks good in the final state

All 12 smells accurately represent Fowler ch. 3 with correct attribution; each is terse what-it-is → how-to-fix
Advisory escalation semantics are correctly threaded: smell label stays advisory, rule carries the severity, project standard endorses → suppress
Confidence emission is end-to-end: agent emits label → Stage 0 extracts native_confidence from free text → Stage 2 passes straight through — no pipeline change needed
Stage 2's two-case description (security-reviewer always; code-reviewer on design-smell findings) is accurate; the per-finding label pattern means Stage 0 extracts per-finding, not per-section, which handles the conditional correctly
The old Duplicated structural boilerplate bullet absorbs cleanly into Duplicated Code — no duplication of the concept remains
CHANGELOG 0.11.0 entry is comprehensive: lists smells, confidence plumbing, escalation path, override mechanism, documented limitations, no-config surface, and regeneration trigger
plugin.json and CHANGELOG version are consistent at 0.11.0
No userConfig surface added, no security re-trigger warranted

Names twelve smells from Fowler, Refactoring 2nd ed. ch. 3, as an advisory vocabulary in the agent's Code quality section. Findings default to SUGGESTION at medium/low confidence; only a documented project rule escalates, and project standards override the baseline. The duplicated-structural-boilerplate bullet folds into Duplicated Code. Descriptions written in our own voice; no config surface — smell suppression rides the existing REVIEW.md / project-rules seam. Closes #283 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The smell baseline claimed medium/low confidence but the agent never emitted a label, so fanout normalization mapped smell findings to unscored — which Stage 4 ranks above low. The agent now outputs an explicit Confidence line per smell finding, and the fanout parse contract and Stage 2 crosswalk recognize it. Addresses Codex P2 on PR #297 (confirmed by claude-review). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

claude · 2026-07-17T22:20:54Z

Claude finished @kyle-sexton's task in 1m 53s —— View job

PR Review: feat(review): add Fowler design-smell baseline to code-reviewer (0.12.0)

Tasks

Load review tools
Read changed files (current HEAD 0093a77)
Verify previous findings remain resolved
Check for new issues introduced by latest rebase/version bump
Post final review

Summary

Reviewed the current HEAD 0093a77 — the post-rebase, post-fix state at 0.12.0. All previous findings remain resolved, the rebase over the 0.11.0 topic-docs entry is clean, and no new issues are introduced. PR is in a shippable state.

Previous findings — still resolved

IMPORTANT (fanout confidence mismatch): Confirmed resolved at current HEAD. The Output format section (line 64) instructs the agent to emit Confidence: medium or Confidence: low per design-smell finding, with the downstream-ranking rationale inline — an agent left without a why tends to drop the constraint over time. The parse contract table (line 11) and Stage 2 description (line 41) are updated consistently — the pipeline has a coherent two-case confidence path for code-reviewer.

SUGGESTION (dead cross-reference): The (see Output format) reference at line 58 continues to resolve to the confidence and escalation guidance added in the Output format section. Still resolved.

Version bump check

The rebase over #291 (which landed the 0.11.0 topic-docs entry) bumped this PR from 0.11.0 → 0.12.0. Both tracking files are consistent:

plugin.json → "version": "0.12.0" ✓
CHANGELOG.md → ## [0.12.0] stacked above ## [0.11.0] ✓

The CHANGELOG 0.12.0 entry correctly merges both commits (feature + confidence fix) into one release description, which is the right shape for a squash-merge repo.

What looks good in the final state

All 12 smells accurately represent Fowler ch. 3; each is terse what-it-is → how-to-fix in the plugin's own voice; no upstream phrasing copied
Advisory escalation semantics correctly threaded: smell label stays advisory, rule carries severity, project standard endorses → suppress
Confidence emission is end-to-end: agent emits label → Stage 0 extracts native_confidence from free text → Stage 2 passes straight through — no pipeline change needed
Stage 2's two-case description (security-reviewer always; code-reviewer on design-smell findings) is accurate — the per-finding label pattern means Stage 0 extracts per-finding, handling the conditional correctly without explicit category detection
Stage 4 rank (high > medium > unscored > low) is unchanged and now correctly served: smell findings tagged medium/low rank below high and above unscored as intended
Old Duplicated structural boilerplate bullet absorbs cleanly into Duplicated Code — no duplication of the concept remains
README table cell for code-reviewer updated accurately
CHANGELOG 0.12.0 entry is comprehensive: smells listed, confidence plumbing, escalation path, override mechanism, documented limitations, no-config surface, regeneration trigger
No userConfig surface added, no security re-trigger warranted (no hooks, MCP, egress, or config change)

…19.0) (#311) ## Summary Adds `/planning:questionnaire` — a user-invoked-only skill that turns a decision the user cannot answer, because another *person* holds the knowledge, into a Markdown discovery questionnaire handed off async. Adapted from Matt Pocock's `to-questionnaire` (mattpocock/skills, in-progress) per the evaluation that greenlit the port, with the grill→interview vocabulary rename throughout. The defining move: **interview the send, not the subject**. The skill asks the user only what they can always answer — who the questionnaire goes to (role, expertise, relationship) and what they need back — then writes questions aimed at the gap between what the recipient knows and what the user needs. It never quizzes the user on the subject the recipient holds. Decisions from the evaluation, all carried through: - **Home + name:** planning plugin, artifact-noun `questionnaire` (the `prd` precedent); keeps the future interview→questionnaire routing intra-plugin. `disable-model-invocation: true` (a side-effecting hand-off the user times, matching upstream). - **Composition:** third routing bucket beside `/planning:interview`'s facts-vs-decisions split — a person-arbitered deferral. Ships standalone; the interview-side one-line pull-out lands separately as an additive edit. - **PII:** output relocated from upstream's cwd write to the topic-docs **memory slice** (default `.work/`, self-ignoring), so recipient names never enter git history. No egress — the skill authors a local file; delivery is out-of-band. - **Tracker seam:** optional "awaiting answer" lifecycle item goes through the work-item-tracker seam (title/topic only, never the recipient's name), skipped gracefully when no tracker is bound. Never a hardcoded `gh` call. - **Config:** none — output path rides the existing topic-docs binding; no speculative knobs (Rule of Three). - **Evals (warranted):** four cases — send-only happy path, the never-quiz-the-subject guardrail, self-answerable routing back to `/planning:interview`, tracker-absent graceful degrade. Also fixes the planning README's stale "depth-first Q&A" phrasing in the interview row (missed residue from the frontier-rounds propagation). Planning plugin bumped 0.18.0 → 0.19.0. Security review clears trivially: no hooks, scripts, MCP, network, secrets, or userConfig. ## Related Closes #284. Same upstream gap-analysis batch as #278/#282 (#294), #283 (#297), #285 (#298). ## Verification - `jq empty` on `evals.json` and `plugin.json` - `claude plugin validate plugins/planning` ✔ - `markdownlint-cli2` on all new/touched markdown: 0 errors - `node scripts/generate-catalog.mjs`: catalog in sync (plugin description unchanged) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

## Summary Adds a `block-dangerous-git` guard to the guardrails plugin (PreToolUse on Bash, blocking), per the evaluation that greenlit the port: capability adapted from mattpocock/skills `git-guardrails-claude-code`; implementation rejected wholesale and rewritten on the house argv-grammar parser (upstream's substring matching blocks ALL `git push` and fires `checkout .` on `checkout .github/…`). **Default block-list — irreversible operations only:** | Blocked | Not blocked | |---|---| | `push --force` / `-f` | `--force-with-lease` (safe force), plain `push` | | `reset --hard` | soft/mixed reset, `--keep` | | `clean -f` / `-fd` / `-fdx` / `--force` | `clean -n` (dry run) | | `checkout .` / `restore .` (worktree-wide, incl. `:/`, exclude-only, magic-`top`) | path-scoped forms, `restore --staged .` (index-only) | | forced `checkout -f` / `switch --discard-changes` | plain branch switch/create | `branch -D` stays out of the defaults: reflog recovers deleted refs, and repo-hygiene's `/clean` issues it inline in a sanctioned, already-confirmed flow (verified collision). **Seams (native userConfig):** the guard follows the marketplace's kill-switch doctrine (per-hook toggles and tuning scalars are native `userConfig`, not `HOOK_*` env vars). Kill switch = the `block_dangerous_git_enabled` option (default `true`); per-repo/per-user allow-list = the `block_dangerous_git_allow` option (comma list, any subset of `push-force,reset-hard,clean-force,checkout-dot,restore-dot,checkout-force`), read by the hook from the `CLAUDE_PLUGIN_OPTION_*` process mirror. Configure with `/plugin configure guardrails` or `--config KEY=VALUE`. userConfig is resolved at plugin-enable time, so an inline env prefix on the command line cannot alter the guard; the residual trust boundary is settings write access. **Shared parser refactor:** the argv tokenizer, git-executable resolver, and subcommand walk that `block-no-verify` carried privately move into `lib/hook-utils.sh` (`hook::bash_parse_segments`, `hook::git_resolve_index`, `hook::git_resolve_subcommand`) so both git guards share one parser. `block-no-verify` behavior is unchanged with one documented alignment: `core.hooksPath` now blocks exactly on `git commit`/`git push` (its documented scope) instead of firing mid-walk on any git subcommand. Per the hook-utils sync contract, all ten carrying plugins bump versions so consumers receive the lib change. Guardrails → 0.6.0 (main's userConfig/prerequisite waves took it to 0.5.1 while this PR was in review; the new guard is the 0.6.0 minor). Hook plugins skip evals — the `.test.sh` contract tests are the coverage. ## Related Closes #285. Same upstream gap-analysis batch as #278/#282 (#294) and #283 (#297). ## Verification - `block-dangerous-git.test.sh`: 190/190 pass (each blocked form, safe near-misses, argv-faithful positives/negatives incl. quoting/wrappers/`env -S`/heredocs/process-subs/aliases, allow-list on/off/partial, kill switch, fail-closed length cap, telemetry envelope) - `block-no-verify.test.sh`: 75/75 pass on the shared-parser refactor; `lib/hook-utils.test.sh`: 65/65 - `scripts/sync-hook-utils.sh --check` / `--check-bump`: all 10 plugin copies match and are bumped - `shellcheck --rcfile .shellcheckrc -x` on all changed shell: clean - `claude plugin validate` on every bumped plugin: pass - `node scripts/generate-catalog.mjs`: catalog already in sync - `markdownlint-cli2` on changed markdown: 0 errors 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

chatgpt-codex-connector Bot reviewed Jul 17, 2026

View reviewed changes

Comment thread plugins/review/agents/code-reviewer.md

kyle-sexton mentioned this pull request Jul 17, 2026

feat(guardrails): add block-dangerous-git guard (0.6.0) #298

Merged

kyle-sexton force-pushed the feat/review-fowler-baseline branch from 8aa214b to 82aa367 Compare July 17, 2026 21:27

kyle-sexton changed the title ~~feat(review): add Fowler design-smell baseline to code-reviewer (0.10.0)~~ feat(review): add Fowler design-smell baseline to code-reviewer (0.11.0) Jul 17, 2026

kyle-sexton and others added 2 commits July 17, 2026 17:53

kyle-sexton changed the title ~~feat(review): add Fowler design-smell baseline to code-reviewer (0.11.0)~~ feat(review): add Fowler design-smell baseline to code-reviewer (0.12.0) Jul 17, 2026

kyle-sexton force-pushed the feat/review-fowler-baseline branch from 82aa367 to 0093a77 Compare July 17, 2026 21:53

kyle-sexton merged commit 449ef3f into main Jul 17, 2026
20 of 25 checks passed

kyle-sexton deleted the feat/review-fowler-baseline branch July 17, 2026 22:24

kyle-sexton mentioned this pull request Jul 17, 2026

feat(planning): add questionnaire skill for person-held decisions (0.19.0) #311

Merged

Navigation Menu

Uh oh!

feat(review): add Fowler design-smell baseline to code-reviewer (0.12.0) - #297

feat(review): add Fowler design-smell baseline to code-reviewer (0.12.0)#297
kyle-sexton merged 2 commits into
mainfrom
feat/review-fowler-baseline

kyle-sexton commented Jul 17, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

claude Bot commented Jul 17, 2026 •

edited

Loading

Uh oh!

kyle-sexton commented Jul 17, 2026

Uh oh!

kyle-sexton commented Jul 17, 2026

Uh oh!

claude Bot commented Jul 17, 2026 •

edited

Loading

Uh oh!

claude Bot commented Jul 17, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

kyle-sexton commented Jul 17, 2026

Summary

Related

Verification

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

claude Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: feat(review): add Fowler design-smell baseline to code-reviewer (0.10.0)

Summary

IMPORTANT — Fanout confidence mismatch (confirms Codex P2)

SUGGESTION — "(see Output format)" cross-reference is a dead end

What looks good

Uh oh!

kyle-sexton commented Jul 17, 2026

Uh oh!

kyle-sexton commented Jul 17, 2026

Uh oh!

claude Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: feat(review): add Fowler design-smell baseline to code-reviewer (0.11.0)

Summary

Previous findings — resolved

What looks good in the final state

Uh oh!

claude Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: feat(review): add Fowler design-smell baseline to code-reviewer (0.12.0)

Summary

Previous findings — still resolved

Version bump check

What looks good in the final state

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

claude Bot commented Jul 17, 2026 •

edited

Loading

claude Bot commented Jul 17, 2026 •

edited

Loading

claude Bot commented Jul 17, 2026 •

edited

Loading