界面已本地化;技能介绍与说明文档按所选语言由 AI 自动翻译,代码、命令、路径和专业术语保留原文,以原作者文档为准。
探索技能
设计与创意 · SKILL.md

improve-codebase-architecture

improve-codebase-architecture

【设计与创意】Surveys a real repository for structural problems and turns the findings into a staged plan: measures churn, size, import cycles, coupling and co-change, finds shallow modules (wrappers that hide nothing) and tangled clusters, ranks a few candidates on evidence, designs the new boundary and proposes the change as small reversible steps guarded by a C…

独立 GitHub 快照 · 自动分类 · 以原作者文档为准

Tterminalskills / skillsApache-2.0

技能说明

GitHub

name: improve-codebase-architecture description: >- Surveys a real repository for structural problems and turns the findings into a staged plan: measures churn, size, import cycles, coupling and co-change, finds shallow modules (wrappers that hide nothing) and tangled clusters, ranks a few candidates on evidence, designs the new boundary and proposes the change as small reversible steps guarded by a CI rule. Use when someone says "improve the architecture", "where should we refactor", "this codebase is a mess", "find the worst modules", "untangle these dependencies", "too many tiny files", "make this easier to test", or asks for a refactoring roadmap. license: Apache-2.0 compatibility: "Any git repository. Commands are given for JavaScript/TypeScript on Node.js 22.12+ (madge 8, dependency-cruiser 18, knip 6) and Python 3.10+ (grimp, radon 6, import-linter 2); the method carries over to other languages with their own dependency tools." metadata: author: terminal-skills version: "2.0.0" category: development tags: ["architecture", "refactoring", "dependency-analysis", "technical-debt", "modularity"]

Improve Codebase Architecture

Overview

Architecture work fails in two ways: opinions without evidence, and plans too large to ship. This skill replaces both with a procedure: measure the repository, name a handful of candidates with the numbers that support them, let the user choose, design the new boundary, and deliver it as steps that each keep the build green. Two kinds of problem repay the effort most:

  • Shallow module: its interface is nearly as large as what it does. Pass-through wrappers, one-function files and layers that only forward mean a reader opens five files to follow one idea and a caller gains nothing.
  • Tangled cluster: modules that cannot be understood, tested or changed apart. Import cycles, files in different folders that always change together, and modules that import half the codebase.

The output is an evidence table, three to five candidate cards, and one proposal with a staged plan. Code changes only after the user approves a step.

Instructions

1. Set the scope

Ask what hurts (slow changes, repeat bugs in one area, tests that need heavy mocking, onboarding), which paths are off limits (generated, vendored, about to be deleted), how much change the team can take per week, and where proposals live (a docs folder or the issue tracker). Then read the README, any architecture notes, the CI config and the test command, and run the tests. A red baseline makes every later step unverifiable, so report it and stop if they fail.

2. Measure

Keep measurement output outside the repository: export ARCH=$(mktemp -d).

Hotspots, any language. Commits in the last year multiplied by current length; columns are score, commits, lines, file. Run from the repository root and replace src with the source root.

git log --no-merges --since=12.months --name-only --pretty=format: -- src | grep . | sort | uniq -c | sort -rn > "$ARCH/churn.txt"
while read -r n f; do [ -f "$f" ] && echo "$((n * $(wc -l < "$f"))) $n $(wc -l < "$f") $f"; done < "$ARCH/churn.txt" | sort -rn | head -15

Co-change, any language. Save as $ARCH/cochange.py; it lists file pairs from different folders that keep landing in the same commit.

"""git log --no-merges --since=12.months --name-only --pretty=format:'@%h' -- src | python3 "$ARCH/cochange.py" 5"""
import collections, itertools, sys

min_shared = int(sys.argv[1]) if len(sys.argv) > 1 else 5
commits, files = [], None
for line in sys.stdin:
    line = line.strip()
    if line.startswith("@"):
        files = set()
        commits.append(files)
    elif line and files is not None:
        files.add(line)
changes, together = collections.Counter(), collections.Counter()
for files in commits:
    if len(files) > 30:  # sweeping commits (renames, formatting) say nothing about coupling
        continue
    changes.update(files)
    together.update(itertools.combinations(sorted(files), 2))
for (a, b), n in together.most_common():
    if n >= min_shared and a.rsplit("/", 1)[0] != b.rsplit("/", 1)[0]:
        print(f"{n:4} commits  {n / min(changes[a], changes[b]):4.0%} of the rarer file  {a}  <->  {b}")

Dependencies, JavaScript and TypeScript.

npx madge --circular --extensions ts,tsx src                       # import cycles
npx madge --json --extensions ts,tsx --exclude '\.test\.tsx?#x27; src > "$ARCH/deps.json"
npm install --prefix "$ARCH" dependency-cruiser typescript@6       # outside the repository, with a TypeScript it can load
"$ARCH/node_modules/.bin/depcruise" src --no-config --do-not-follow node_modules --metrics --output-type metrics   # Ca, Ce, instability per folder and module
npx knip --files --exports                                         # files and exports nothing uses
grep -rcE "(vi|jest)\.mock\(" src --include='*.test.ts' | sort -t: -k2 -rn | head   # mocks per test file

Save this as $ARCH/shape.cjs and run node "$ARCH/shape.cjs" src for per-file size, exports, importers (fanIn) and imports (fanOut):

const fs = require('fs'), path = require('path')
const root = process.argv[2] || 'src', deps = require(path.join(process.env.ARCH, 'deps.json'))
const fanIn = {}
for (const targets of Object.values(deps)) for (const t of targets) fanIn[t] = (fanIn[t] || 0) + 1
const rows = Object.keys(deps).map((file) => {
  const text = fs.readFileSync(path.join(root, file), 'utf8')
  const loc = text.split('\n').filter((l) => l.trim() && !l.trim().startsWith('//')).length
  return { file, loc, exports: (text.match(/^export\s/gm) || []).length, fanIn: fanIn[file] || 0, fanOut: deps[file].length }
})
const show = (title, list) => { console.log('\n' + title); console.table(list.slice(0, 10)) }
show('Most outgoing imports', [...rows].sort((a, b) => b.fanOut - a.fanOut))
show('One importer and under 40 lines', rows.filter((r) => r.fanIn === 1 && r.loc < 40).sort((a, b) => a.loc - b.loc))
show('Fewer than 8 lines per export', rows.filter((r) => r.exports >= 5 && r.loc / r.exports < 8))

Dependencies, Python. pip install grimp radon import-linter in the project's virtualenv, then radon cc src -s -n C for functions of cyclomatic complexity 11 and above, and for coupling (replace clinic with the top-level package):

import grimp
graph = grimp.build_graph("clinic")
for m in sorted(graph.modules, key=lambda m: -len(graph.find_modules_directly_imported_by(m)))[:15]:
    out, into = graph.find_modules_directly_imported_by(m), graph.find_modules_that_directly_import(m)
    print(f"{m:40} fan-out {len(out):2}  fan-in {len(into):2}")

3. Turn numbers into suspects, then read them

Signal (starting thresholds)SourceWhat it suggests
Import cyclemadgethe files in the loop are one module split in the wrong place
Pair in different folders sharing 5+ commits and 50%+ of the rarer file's changescochange.pyone decision lives in two places
Fan-out of 15 or more, or the top 5%shape script, grimpa module that knows too much
Top 10 by commits times lineshotspot loopwhere leaving things alone costs most
One importer and under 40 linesshape scriptshallow: could it merge into its only caller?
Fewer than 8 lines per exportshape scriptinterface almost as big as the body
Function bodies that are one call with the same argumentsreadinga layer that only forwards
The same three calls in the same order at many call sitesgrepcallers are doing the module's job
A unit test that replaces four or more modulesmock countthe boundary is in the wrong place

Numbers nominate; reading decides. Open every suspect. These are not problems: index files that only re-export, type-only files, generated code, files a framework requires (routes, migrations), and thin adapters that sit deliberately at the edge of one external system.

4. Write candidate cards and rank them

One card per candidate, at most five:

### C1. Pricing rules are split between orders and invoices
Kind: tangled cluster
Evidence: cycle orders/pricing.ts > invoices/totals.ts; 31 shared commits (57%); both in the hotspot top 5
Cost today: every discount change edits both files; four of the last nine billing bugs were totals that disagreed
Target: one pricing module with a single entry point that orders and invoices both call
Blast radius: 2 files move, 11 importers change, no schema change
Risk: medium, it is money. Behaviour is pinned by tests before anything moves
Effort: 5 pull requests

Score = independent signals (1 to 3) × relevance to the stated pain (1 to 3) ÷ effort (1 up to two pull requests, 2 up to five, 3 beyond). Show the table and let the user pick one or two. Do not design before they pick.

5. Design the new boundary

Write the target as declarations: the entry points, their types, and the list of what moves inside. It is good enough when callers need to know less than before, dependencies point one way, and the module can be tested through its entry point with at most one fake for real I/O. For a shallow chain decide the merge direction: fold the forwarding layer into its caller when the caller owns the decision, into its callee when the callee does.

6. Stage the change

Each step is one pull request that passes the tests and can be reverted alone.

  1. Pin behaviour. Add tests through the future entry point that record what the code does today, including odd results. Report suspected bugs; do not fix them in the same step.
  2. Add the entry point as a facade over the existing code. No caller changes.
  3. Move callers in batches, one folder at a time, while the old paths still work.
  4. Move the implementation behind the facade and delete the old paths. Re-run the cycle check here.
  5. Lock it in with a dependency rule in CI, and re-measure.

For step 5, add dependency-cruiser to the project (npm install --save-dev dependency-cruiser) and a .dependency-cruiser.cjs; npx depcruise src --config .dependency-cruiser.cjs exits with the number of errors, so CI fails on any violation:

module.exports = {
  forbidden: [
    { name: 'no-circular', severity: 'error', from: {}, to: { circular: true } },
    { name: 'pricing-through-its-entry-point', severity: 'error',
      from: { pathNot: '^src/pricing/' },
      to: { path: '^src/pricing/', pathNot: '^src/pricing/index\\.ts#x27; } },
  ],
  options: { doNotFollow: { path: 'node_modules' }, tsConfig: { fileName: 'tsconfig.json' } },
}

Existing violations elsewhere need not block the rule: --baseline writes them to .dependency-cruiser-known-violations.json and --ignore-known then fails only on new ones. In Python the same job is an import-linter contract (layers, forbidden or independence) checked by lint-imports.

7. Write the proposal

Save it where the scope step said (for example docs/architecture/2026-10-pricing-module.md), with these sections: problem and evidence table; target boundary as declarations; the steps as a table of pull request, change and proof; what is deliberately not being done; the numbers to re-measure afterwards. Ask before opening issues or pull requests.

Examples

Example 1: a tangled cluster in a TypeScript backend

Northbay Couriers runs an Express service of 412 files and 38,400 lines. The stated pain: "every pricing change breaks invoices". Tests pass. Measurements:

61596 87 708 src/orders/order.service.ts            hotspots: score commits lines file
35224 68 518 src/orders/pricing.ts
22302 54 413 src/invoices/totals.ts

✖ Found 3 circular dependencies!
1) orders/pricing.ts > invoices/totals.ts
2) orders/order.service.ts > shipments/label.ts > orders/order.repo.ts
3) auth/session.ts > users/user.service.ts

  31 commits   57% of the rarer file  src/invoices/totals.ts  <->  src/orders/pricing.ts
  12 commits   29% of the rarer file  src/orders/order.service.ts  <->  src/shipments/label.ts

Seven files had one importer and under 40 lines; reading them showed five index files and two real wrappers.

#CandidateKindSignalsPainEffortScore
C1Pricing rules split between orders/pricing.ts and invoices/totals.tstangled3: cycle, co-change, two hotspots324.5
C2order.service.ts also prints labels and queries the database (fan-out 23)tangled3: cycle, top hotspot, fan-out232.0
C3shipments/carrier.facade.ts forwards six calls unchangedshallow2: one importer, 3 lines per export112.0
C4auth/session.ts and users/user.service.ts import each othertangled1: cycle111.0

The user picks C1. Target boundary, in src/pricing/index.ts, the only file other folders may import:

export type Quote = { lines: QuoteLine[]; subtotal: Money; discount: Money; tax: Money; total: Money }
export function quote(order: OrderDraft, at: Date): Quote
PRChangeProof
1Tests run 40 recorded orders through today's order price and invoice total and compare with a saved snapshotgreen on main; 2 orders where the two disagree are reported, not fixed
2quote() added, delegating to the existing functionsno caller touched, tests green
3The 7 importers in orders/ call quote()tests green
4The 4 importers in invoices/ call quote(); rule code moves into src/pricing/; invoices/totals.ts deletedmadge --circular reports 2 cycles, not 3
5The dependency rule from step 6 runs in CI with the two older cycles baselineddepcruise --ignore-known exits 0

To re-measure after three months: commits that touch both orders and invoices (31 before), and billing bugs caused by disagreeing totals.

Example 2: a forwarding layer in a Python service

A dental clinic's booking API is layered app.api > app.services > app.managers > app.repositories. The pain: "adding a field means editing four files". The grimp script shows all 14 modules in app.managers with fan-in 1 and fan-out 1, and reading them shows 31 of 36 functions consist of one call to a repository method with the same arguments. Nine service test files patch a manager.

Card: shallow layer, 3 signals, pain 3, effort 2, score 4.5. Design: remove app.managers; the five functions with real logic move into the service that calls them, since the services own those decisions. Plan: three pull requests by domain (appointments, patients, billing), each moving functions, updating imports and deleting the emptied manager files, with the existing API tests as the safety net; a fourth adds the guard:

[importlinter]
root_package = app

[importlinter:contract:layers]
name = api above services above repositories
type = layers
layers =
    app.api
    app.services
    app.repositories

lint-imports then prints Contracts: 1 kept, 0 broken. Result: 14 files and one hop removed, and a new field touches three files.

Guidelines

  • Thresholds are starting points. In a repository of 60 files a fan-out of 15 is everything; in one of 6,000 it is ordinary. Compare modules with their neighbours, not with the table.
  • madge counts import type edges, so type-only loops appear as cycles. Add {"detectiveOptions":{"ts":{"skipTypeImports":true}}} to .madgerc to see runtime cycles only; dependency-cruiser ignores type-only imports by default.
  • dependency-cruiser reads TypeScript only through a typescript package installed beside it, versions 2 to 6 as of 18.5. Run as npx -p dependency-cruiser depcruise, or in a project on TypeScript 7, it finds none, reports 0 modules, 0 dependencies cruised and exits 0, so a guard set up that way never fails. Check the cruised count, and where the project's TypeScript is too new use the $ARCH install from step 2. A bare npx depcruise with no local install fetches an unrelated placeholder package.
  • knip loads the project's own config files, so run it after the dependencies are installed; otherwise it fails to read them and reports far too much as unused. Treat its list as leads, never as a delete list.
  • History-based signals need history: after a squash import or in a young repository, rely on the dependency graph and on reading.
  • Co-change inside one folder is normal and is filtered out. A pair across folders is only a finding when the commits were about one concern; check a few commit messages.
  • Small files are not the problem; files that add a hop without hiding a decision are. Merging two modules that change for different reasons creates a tangle.
  • Never combine a structural move with a behaviour change in one pull request. If pinning tests reveal a bug, record it and keep the behaviour until the move is finished.
  • Stop when the tests are red, when the user cannot name a pain, or when the top candidate scores under 2: a report saying "no change is worth its cost right now" is a valid result.
  • Not for renaming, formatting or style clean-ups, and not for choosing a framework or splitting into services. Those are different decisions with different evidence.
我的收藏