Skip to content

Add shared code smell vocabulary doc - #88

Merged
bborbe merged 3 commits into
masterfrom
feature/fowler-smells
Aug 8, 2026
Merged

Add shared code smell vocabulary doc#88
bborbe merged 3 commits into
masterfrom
feature/fowler-smells

Conversation

@bborbe

@bborbe bborbe commented Aug 8, 2026

Copy link
Copy Markdown
Owner

What

Adds docs/code-smell-vocabulary.md — Fowler's 12 structural code smells as one-line definitions in a single language-agnostic doc — and wires the three architecture agents to load it.

Before this, exactly 1 of the 12 terms ("shotgun surgery") existed anywhere in the repo, at docs/architecture-dimensions-guide.md:142.

Why one shared doc instead of per-language copies

The terms are language-independent by construction, but this repo's architecture docs are per-language and already structurally parallel — go-architecture-patterns.md and python-architecture-patterns.md both run Constructor Injection → composition root → Factory → Antipatterns. Copying 12 definitions into each would mean 36 copies drifting apart, and Node (no architecture doc at all) would still get none.

Language docs now carry only the language-specific fix — Go named types vs Python NewType vs TS branded types — while the definitions live once.

Mechanical vs judgment tier

The doc splits the 12 rather than presenting them as one flat checklist:

  • Mechanical (long method, large class, long parameter list, duplicated code, dead code) — already owned by funlen / dupl / vulture / ts-prune
  • Judgment (the other 7) — require knowing what the code means

architecture-dimensions-guide.md:185 already rejects mechanical findings as non-architectural, so feeding all 12 to an agent would reimport exactly the noise that line exists to keep out.

Scope contract preserved

architecture-dimensions-guide.md §7 names divergent change and speculative generality as evolvability probes — the latter was already described there unnamed ("abstraction is premature") — and states explicitly that the other nine are structural and out of scope for the behavioral pass, per its own scope table at lines 16–19.

Files

File Change
docs/code-smell-vocabulary.md New
docs/go-architecture-patterns.md Links shared doc + Go-specific fix table
docs/python-architecture-patterns.md Same, Python fixes
docs/node-service-guide.md Same, TS fixes — Node had no structural-smell vocabulary before
docs/architecture-dimensions-guide.md §7 names 2 evolvability terms; scope note
agents/{go-architecture,python-architecture,node-quality}-assistant.md Name the shared doc in their source-of-truth block
README.md, llms.txt Doc-table entries (repo CLAUDE.md § Adding a new guide)

Verification

make precommit passes — link validation, plugin JSON, coverage 30/30, 103 bench tests.

Known limitation

This change is not measurable by bench/: bench/run.py:42 sets HASHED_SUBDIRS = ("rules", "commands"), so a docs/ or agents/ change produces an identical config_hash and would be served from cache. Filed separately against the benchmark runner — not fixed here to avoid conflicting with in-flight bench work.

No new agent added: repo CLAUDE.md states reference-only docs don't need one.

@ben-s-pull-request-reviewer ben-s-pull-request-reviewer Bot 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.

I have completed the review. Here is my consolidated report:


Must Fix (Critical)

None.

Should Fix (Important)

None.

Nice to Have (Optional)

None.


Notes:

All concerns from the Plan were verified as correct:

  • docs/code-smell-vocabulary.md — Fowler's 12 definitions are accurate one-liners; mechanical tier (Long method, Large class, Long parameter list, Duplicated code, Dead code) is correctly separated from judgment tier (the remaining 7); the layer table is internally consistent.

  • agents/go-architecture-assistant.md:17, python-architecture-assistant.md:17, node-quality-assistant.md:20 — each correctly names docs/code-smell-vocabulary.md in a "Shared vocabulary" block; each correctly scopes the mechanical tier to its own linters (funlen/dupl/linters, mccabe/pylint/vulture, ESLint/ts-prune`) and states not to re-report it.

  • docs/architecture-dimensions-guide.md:148 — explicitly states the other nine smells are structural and belong to the sibling assistants per the scope table (lines 16–19), and §7 uses only divergent change, shotgun surgery, and speculative generality as evolvability probes.

  • docs/go-architecture-patterns.md, python-architecture-patterns.md, node-service-guide.md — each adds a "Structural Smells" table linking the shared vocabulary and carrying only language-specific fixes; no definitions are duplicated.

  • CHANGELOG.md:13 — bullet starts with docs: which is in the conventional-prefix allowlist.

  • llms.txt:93 — new entry present: docs/code-smell-vocabulary.md with correct description.

Selector-mode traceability:

  • Candidates: rules matched by glob filter on changed files (agents/**/*.md, docs/**/*.md, CHANGELOG.md, README.md, llms.txt)
  • Applicable: 0 — no judgment rule's applies_when condition is triggered by doc-only changes; no Go/Python/Node source files, no config files, no license/readme issues
  • Skipped: all judgment candidates → applies_when requires source code patterns not present in this diff; changelog/conventional-prefix-required → bullet is correctly prefixed; agent-cmd/single-source-of-truth → shared doc IS the single source, each agent names it
{
  "verdict": "approve",
  "summary": "PR adds a well-structured shared code-smell vocabulary doc with accurate Fowler definitions, correctly splits mechanical from judgment tiers, links it from all three architecture assistants' source-of-truth blocks, adds language-specific fix tables to the three language guides, and updates the evolvability section of architecture-dimensions-guide.md to name the two missing probes. No violations found in any checked rule.",
  "comments": [],
  "concerns_addressed": [
    "correctness: Fowler's 12 definitions verified accurate in code-smell-vocabulary.md",
    "correctness: shared doc correctly referenced in go-architecture-assistant.md source-of-truth block",
    "correctness: shared doc correctly referenced in node-quality-assistant.md source-of-truth block",
    "correctness: shared doc correctly referenced in python-architecture-assistant.md source-of-truth block",
    "correctness: architecture-dimensions-guide §7 correctly names divergent change and speculative generality as evolvability probes; scope table (lines 16-19) excludes the 9 structural smells",
    "correctness: go-architecture-patterns.md fix table correctly supplements shared definitions with Go-specific fixes",
    "correctness: python-architecture-patterns.md fix table correctly supplements shared definitions with Python-specific fixes",
    "correctness: node-service-guide.md fix table correctly supplements shared definitions with TS-specific fixes",
    "tests: llms.txt updated correctly with new doc entry"
  ]
}

@ben-s-pull-request-reviewer
ben-s-pull-request-reviewer Bot dismissed their stale review August 8, 2026 21:59

superseded by new automated review

@ben-s-pull-request-reviewer ben-s-pull-request-reviewer Bot 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.

{
  "verdict": "approve",
  "summary": "PR adds a new shared code-smell vocabulary doc (Fowler's 12 structural smells, language-agnostic) plus cross-references from the three architecture assistants and their companion guides. All three changelog rules pass, the README is user-facing, and agent frontmatter is unchanged. Mechanical funnel clean (0 findings). selector clean — no adjudication needed.",
  "comments": [],
  "concerns_addressed": []
}

@bborbe
bborbe merged commit 4139256 into master Aug 8, 2026
1 check passed
@bborbe
bborbe deleted the feature/fowler-smells branch August 8, 2026 21:59
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.

1 participant