feat(review): always-on Fowler smell baseline in the Standards axis#394
Merged
Conversation
… axis Folds Refactoring ch.3 "Bad Smells in Code" into the in-progress `review` skill. The skill stays two-axis: smells layer into the Standards axis as a fixed, always-on baseline alongside a repo's documented standards, rather than a third axis. Because the baseline is needed on every run it lives inline in SKILL.md. Each smell is one line — its name carries the definition (the names are strong leading words) plus a diff-specific cue. Curated to ~12 high-signal smells; vague ones (Comments, Loops) and tooling-caught ones (Long Parameter List) are deliberately dropped. Two rules bind the baseline: a documented repo standard always overrides it, and every smell is reported as a judgement call, never a hard violation. The Standards sub-agent prompt now carries the baseline and these rules. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 7a4c756 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Each smell now reads what-it-is → how-to-fix, telegraphic style — the fix is the actionable half a reviewer needs, kept to a clause. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Merged
oskarhane
added a commit
to oskarhane/hone-ai
that referenced
this pull request
Jun 30, 2026
Inline a 12-smell 'Bad Smells in Code' baseline (Refactoring ch.3) into both review surfaces — hone-auditor (standalone /hone:review) and hone-reviewer (in-loop /hone:run). Two binding rules: documented repo conventions override the baseline; every smell is a judgement call, never a hard violation. Mirrors mattpocock/skills#394.
paulund
added a commit
to paulund/ai
that referenced
this pull request
Jun 30, 2026
Adopt the always-on smell baseline pattern from mattpocock/skills#394 into our review skill. The 12 Fowler smells (Refactoring, ch. 3) now sit inside the Readability axis as a fixed checklist, layered on top of whatever the repo documents. Binding rules: - The repo overrides (documented standards win). - Always a judgement call ("possible X"), never a hard violation. - Capped at Medium severity — never Critical/High. Smell findings get a carve-out from the single-finding bar: they cite file:line + smell name + quoted code, no data-flow trace required.
pengelbrecht
added a commit
to pengelbrecht/ticks
that referenced
this pull request
Jun 30, 2026
Add references/code-smells.md — a curated subset of Martin Fowler's "Bad Smells in Code" as the new Maintainability review axis, adapted from mattpocock/skills#394. Kept as a selectable axis (not an always-on baseline) to match the skill's "axes the diff earns" principle and progressive-disclosure habit; binding rules make every smell a judgment call (never a blocker) deferring to documented repo standards. Wire it into the per-tick Code-quality stage and the review axis menu in agent-runner.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
notque
added a commit
to notque/vexjoy-agent
that referenced
this pull request
Jun 30, 2026
… reviewer (#818) Adopts the curated 'Bad Smells in Code' baseline from mattpocock/skills#394 with three vexjoy modifications established by blind A/B test (B' won 6/6 trap-focused setups, 4 fewer trap false positives, ~50% fewer invented findings vs the verbatim baseline): 1. Named language-idiom counter-examples (Go type switch, TS discriminated union, TS .map().filter().reduce() chains, TS string literal unions, Python single-site if/elif, Python ABCs with one impl+caller, Python dict[str, Any]) — each pattern explicitly suppressed by name. 2. Severity cap: smells default to LOW, may rise to MEDIUM if compounded, never HIGH/CRITICAL from a smell alone. 3. Lower-signal OO smells (Feature Envy, Message Chains, Middle Man, Refused Bequest) demoted with Go-specific caveats; Message Chains scoped to property navigation only (not method chains on collections). The brief lives in references/architecture-smell-baseline.md and is passed verbatim to the Architecture reviewer in Phase 2. Two-line edit in SKILL.md adds the integration point and references entry; everything else is in the reference file.
titouanmathis
added a commit
to ikko-dev/gitlab-review
that referenced
this pull request
Jul 1, 2026
#90) * feat(skill): add a Fowler code-smell baseline to the code-review skill Layer a fixed set of ~12 Fowler design smells (Refactoring, ch. 3) into the code-review skill as a secondary, non-blocking dimension alongside correctness. Two binding rules: a documented repo standard overrides the baseline, and every smell is a labelled judgment call ("possible Feature Envy"), never a hard violation. Smells map to INFO by default, WARN only when concrete and costly, and never CRITICAL. Inspired by mattpocock/skills#394, adapted to our correctness-first, single-skill design: correctness still comes first and smells never crowd out a real bug; the reviewer flags only smells the diff introduces or worsens and suppresses anything a linter or repo convention covers. Adds a recording-only eval (design-smells fixture) asserting a smell is surfaced and never escalated to CRITICAL. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013ndQzhvfHbaHQ2gJAkT5kr * docs: changelog entry for the Fowler code-smell baseline (#90) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013ndQzhvfHbaHQ2gJAkT5kr * test(evals): repair stale eval config and make the smell fixture bug-free makeConfig in review.eval.ts was missing Config fields added in 0.7.0 (modelPool, maxDiffChars, decomposeHintLines, verbose, refreshGitSkills), so every accuracy eval crashed at setup with "reading 'length'" before any LLM call. These evals run only via `npm run test:evals` (not CI), so the staleness went unnoticed. Add the missing fields. Rewrite the design-smells fixture to be airtight on correctness (typed primitive params and an inline non-optional Money type), so the only thing to report is the smell. The prior fixture's untyped nested field access legitimately drew CRITICAL null-deref findings, which is correct behaviour but defeats the eval's intent. Verified: DesignSmellSurfaced and NoCriticalSmell both score 1.00 on sonnet; the clean-code false-positive guard also stays 1.00 with the smell baseline enabled. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013ndQzhvfHbaHQ2gJAkT5kr * test(evals): graduate the code-smell baseline eval to enforcing Flip judgeThreshold from null to 1 after three consecutive green sonnet runs on the airtight fixture (DesignSmellSurfacedJudge and NoCriticalSmellJudge both 1.00 each time). The triple-identical slug functions make the smell unmistakable, so recall is robust; the fixture is bug-free, so a CRITICAL is a clear violation. Evals run only via `npm run test:evals`, never in the default CI test job, so an enforcing threshold here cannot flake PR CI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013ndQzhvfHbaHQ2gJAkT5kr --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jul 1, 2026
notque
added a commit
to notque/vexjoy-agent
that referenced
this pull request
Jul 2, 2026
… reviewer (#818) Adopts the curated 'Bad Smells in Code' baseline from mattpocock/skills#394 with three vexjoy modifications established by blind A/B test (B' won 6/6 trap-focused setups, 4 fewer trap false positives, ~50% fewer invented findings vs the verbatim baseline): 1. Named language-idiom counter-examples (Go type switch, TS discriminated union, TS .map().filter().reduce() chains, TS string literal unions, Python single-site if/elif, Python ABCs with one impl+caller, Python dict[str, Any]) — each pattern explicitly suppressed by name. 2. Severity cap: smells default to LOW, may rise to MEDIUM if compounded, never HIGH/CRITICAL from a smell alone. 3. Lower-signal OO smells (Feature Envy, Message Chains, Middle Man, Refused Bequest) demoted with Go-specific caveats; Message Chains scoped to property navigation only (not method chains on collections). The brief lives in references/architecture-smell-baseline.md and is passed verbatim to the Architecture reviewer in Phase 2. Two-line edit in SKILL.md adds the integration point and references entry; everything else is in the reference file.
This was referenced Jul 3, 2026
This was referenced Jul 4, 2026
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
1 participant
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.
What
Folds Martin Fowler's Refactoring ch.3 "Bad Smells in Code" into the in-progress
reviewskill, giving its Standards axis more to catch.The skill stays two-axis. Smells layer into the Standards axis as a fixed, always-on baseline alongside whatever the repo documents — not a new third axis (that separation is load-bearing for the skill).
How
SKILL.md— the baseline is needed on every run, so it isn't progressively disclosed.Not in this PR (deliberate)
Does not graduate
reviewout ofin-progress/. Smells improve coverage, not reliability — graduation is gated on exercising the baseline against real diffs.🤖 Generated with Claude Code