TypeScript Code Doctor

A critical-reviewer skill. Its job is to make TypeScript simpler, more consistent, and more correct — by deleting what isn't needed, flattening what's tangled, fixing real bugs, and converging the codebase on one good way of doing each thing.

Reviewer mindset (read this first)

Approach the code as too complex until proven otherwise. The default question is not "is this OK?" but "why isn't this simpler?" Be specific and unsparing. Two hard limits keep the criticism honest:

1. Behavior is sacred. Never change what the code does. If it isn't tested, write a characterization test that pins current behavior before refactoring.
2. Simpler, not cleverer. Aim for code a tired developer reads at a glance — not a showcase of conditional types. A clever line that needs a comment has failed, and a type that needs a comment has failed twice.

Bias toward: deleting code, flattening structure, the platform over hand-rolled machinery, one canonical pattern applied everywhere, and small behavior-preserving steps. The burden of proof is on complexity, not its removal.

The TypeScript-specific bias: prefer making the compiler responsible. A narrowed type, an exhaustive switch, a readonly field, or one strictness flag removes a class of bug permanently — where a fix removes one instance.

How the skill works: two pronged

• Deterministic scripts find what can be found mechanically (a real TS/TSX tokenizer, low false-positive). Run them first and triage their output before reviewing by hand.
• Judgment guides in references/ cover what needs a reading brain — whether an abstraction earns its keep, whether duplication is real, whether a cast is the honest answer at a boundary. Load the relevant guide when doing that review.

Running the scripts

Let SKILL=/path/to/this/skill — the directory holding this SKILL.md. The commands below run from the project being reviewed, so they need that prefix.

This skill is self-contained: everything it needs is in its own directory, and it depends on no other skill. The detectors ship their own TypeScript scanner and never invoke node, so they run against a checkout with no node_modules, no install step and no build — which is what makes them usable on a repo you have just cloned or on a project whose build is currently broken. The only requirement is a Python 3.11+ interpreter to launch them with; nothing needs installing.

run_external_tools.py additionally drives tsc, ESLint, Biome, Prettier, madge, knip, npm/pnpm/yarn audit (whichever the lockfile names) and coverage when the project already has them; it reports the ones that are missing rather than failing, and no detector depends on it.

What the scanner cannot do: it reads syntax, not types. Questions that need the type checker — is this promise handled, is this condition always true, is this cast unnecessary, is x really an Error — are tsc's and typescript-eslint's, and run_external_tools.py is how you get them. A file that will not tokenize is named on stderr rather than silently reported clean.

Workflow

Cleaning up a whole poorly-written repo from cold? The steps below assume the code compiles, has some tests, and is roughly formatted. When it doesn't, follow references/messy-repo-runbook.md first — get tsc --noEmit passing, build a test safety net before touching anything (references/safety-net-and-testing.md), normalize formatting in one behavior-free commit, then return here to triage.

1. Read tsconfig first. python "$SKILL/scripts/find_tsconfig_issues.py" . Strictness decides how much of everything else the compiler was ever going to catch. A repo with strict: false has a different set of real problems than its detector output suggests.
2. Run the analyzer. python "$SKILL/scripts/analyze_all.py" <path> (add --format json for tooling). Triage deterministic findings first — don't spend judgment on what a tool already caught.
3. Run the project's own tools. python "$SKILL/scripts/run_external_tools.py" . tsc and typescript-eslint see types and these detectors do not; their findings outrank overlapping ones here.
4. Find the hot files. Effort follows change frequency, not line count:

   git log --since="1 year ago" --name-only --pretty=format: \
     | grep -E '\.(ts|tsx)$' | sort | uniq -c | sort -rn | head -30

   High-churn × high-complexity = top priority. Don't refactor cold code.
5. Review the hot files with the judgment guides open (see the reference index).
6. Produce a findings artifact. One smell → one entry → one small PR: python "$SKILL/scripts/analyze_all.py" . --format json | python "$SKILL/scripts/format_findings.py". This is the deliverable — see Output & ticketing; never create tickets in a tracker without asking.
7. Ratchet. When a whole class of problem is cleared, turn on the check that keeps it gone: a tsconfig flag, an ESLint rule, or one of these scripts in CI.

Output & ticketing

The deliverable is always an artifact, never a side effect. Produce one of: a findings list (markdown table), detailed cards, a JSON array, or the full report from analyze_all.py — saved as a file in the workspace or returned inline. scripts/format_findings.py renders any detector's JSON into these shapes.

This skill does not create tickets in any system on its own. When the user wants findings filed as real tickets, ask which ticket software or MCP to use (e.g. Jira, Linear, GitHub Issues, Asana, or a connected MCP) and create them through that tool — never assume or fabricate a tracker.

Deterministic scripts

analyze_all.py parses each file once and asks every detector about that one tree, across as many processes as there are cores — the parser here is a hand-written scanner, so re-parsing per detector was most of the runtime on a large repository. --jobs N sets the worker count; --jobs 1 runs everything in one process, which is what to use when a crash needs a clean traceback or a large tree will not fit in memory at N workers.

tsproject.load_project returns the same Project for a repeated root, which is how the whole-tree detectors avoid parsing the tree once each. Treat it as read-only — a detector that rewrote it would change what every later detector sees.

python "$SKILL/scripts/analyze_all.py" /path           # Run everything, unified report
python "$SKILL/scripts/analyze_all.py" . --format json > report.json
python "$SKILL/scripts/analyze_all.py" . --jobs 1      # one process, for a clean traceback

# The type system (start here — this is what makes it TypeScript)
python "$SKILL/scripts/find_tsconfig_issues.py" .      # strict, noUncheckedIndexedAccess, suppressions
python "$SKILL/scripts/find_type_gaps.py" .            # any, as, as unknown as, !, @ts-ignore, Function/{}, all-optional types

# Correctness bugs (these find real bugs, not style)
python "$SKILL/scripts/find_async_issues.py" .         # floating promises, forEach(async), await-in-loop, swallowed rejections
python "$SKILL/scripts/find_mutation_hazards.py" .     # mutated arguments/imports, mutation during iteration, mutable exported constants
python "$SKILL/scripts/find_exception_issues.py" .     # empty catch, swallowed errors, thrown non-Errors, return-in-finally
python "$SKILL/scripts/find_resource_leaks.py" .       # timers/listeners/subscriptions without cleanup, effects without teardown
python "$SKILL/scripts/find_security_issues.py" .      # eval, innerHTML, shell interpolation, weak hashes, hardcoded secrets

# Structure, design and encapsulation
python "$SKILL/scripts/analyze_complexity.py" .        # cyclomatic/cognitive complexity, nesting, size, arity
python "$SKILL/scripts/find_encapsulation_issues.py" . # public mutable fields, exported let, leaked internals, message chains
python "$SKILL/scripts/find_design_smells.py" .        # type switches, flag parameters, data clumps, temporary fields, refused bequest
python "$SKILL/scripts/find_coupling_issues.py" .      # feature envy, low cohesion (LCOM), middle man
python "$SKILL/scripts/find_overengineering.py" .      # single-impl interfaces, all-static classes, pass-through modules (YAGNI)
python "$SKILL/scripts/find_code_smells.py" .          # ==, var, nested ternaries, switch without default, god class

# Architecture & repo structure (cross-file)
python "$SKILL/scripts/find_module_issues.py" .        # import cycles, barrel files, god modules, deep relative paths
python "$SKILL/scripts/find_dependency_issues.py" .    # package.json vs imports, stylesheets, HTML, tool configs: missing, unused, misplaced, unpinned
python "$SKILL/scripts/find_dead_code.py" .            # unused imports/exports, unreachable code, dead private members
python "$SKILL/scripts/find_duplicates.py" .           # duplicated blocks, identical type shapes, repeated literals (not CSS class lists)

# Safety net (build this BEFORE refactoring — see references/safety-net-and-testing.md)
python "$SKILL/scripts/find_untested_modules.py" .     # source modules no test reaches; "no tests in repo" alarm
python "$SKILL/scripts/find_test_smells.py" .          # assertion-less tests, .only/.skip, unawaited rejects, over-mocking

# Simplification & hygiene
python "$SKILL/scripts/find_loop_simplifications.py" . # index loops, for..in over arrays, push-loops, filter().length
python "$SKILL/scripts/find_outdated_idioms.py" .      # require/module.exports, namespace, <T>casts, arguments, x && x.y, || defaults
python "$SKILL/scripts/find_naming_issues.py" .        # casing, I-prefix, shadowed browser globals, boolean naming
python "$SKILL/scripts/find_ai_scaffolding.py" .       # stubs, ignored options parameters, duplicate definitions, merge markers
python "$SKILL/scripts/find_debug_leftovers.py" .      # debugger, console noise, blanket eslint-disable
python "$SKILL/scripts/find_comment_smells.py" .       # commented-out code, TODO inventory, JSDoc that restates types

# Format findings as a portable artifact (does NOT create tickets)
python "$SKILL/scripts/format_findings.py" report.json                       # markdown list
<any detector> --format json | python "$SKILL/scripts/format_findings.py" --format cards
<any detector> --format json | python "$SKILL/scripts/format_findings.py" --format json --min-severity high

All detectors share one interface: --format text|json, --ignore type1,type2, and 🔴/🟡/🟢 severities. JSON output is a flat list of findings; analyze_all.py aggregates them and can drop whole categories with --skip cat1,cat2 (--skip-duplicates is shorthand for the slowest one). They are deliberately conservative (false negatives over false positives) so the output stays trustworthy, and several apply a lighter standard inside test files — a cast that installs a mock is not a claim about the product's types. A test file is one under tests/, __tests__/, spec/, e2e/ or cypress/ below the project root (the outermost package.json or tsconfig inside the checkout, bounded by the first .git), or one named *.test.* / *.spec.* — where the checkout itself lives does not change the classification. The scaffolding suites share counts as test code too: anything under __mocks__/, __fixtures__/, testkit/, test-utils/, testutils/ or test-helpers/ (below the root, by the same rule), and the runner's setup file — test-setup.*, setupTests.*, vitest.setup.*, jest.setup.*.

Declaration files (.d.ts, .d.mts, .d.cts) carry no implementation, so the code detectors skip them. So does any file whose first five lines say @generated or DO NOT EDIT: a tool owns it and a finding there is a bug report against the generator, not the checkout. The skipped generated files are counted — on stderr by the standalone detectors, and in meta.generated_files_skipped (and the report header) by analyze_all.py — so their silence is never mistaken for a clean bill.

Findings and candidates. Most records assert a defect and carry the fix. A record that instead carries "kind": "candidate" is an unverified lead — it names what was observed, not what is wrong, because the detector reads syntax one file at a time and the evidence that settles it is elsewhere. A candidate carries also_caused_by, the benign readings a healthy codebase produces, and no suggestion: there is nothing to fix until the lead is confirmed.

Thirteen heuristics are always reported this way: type_assertion, missing_return_type, all_optional_type, definite_assignment_assertion, await_in_loop, single_implementation_interface, barrel_file, data_clump, primitive_obsession, public_mutable_field, switch_without_default (whether a missing default matters depends on the discriminant's type), boolean_without_predicate_name (a noun, or an adjective?) and falsy_default_with_or (the left operand's type decides). Three more are candidates in their ambiguous shape only: a bare alert() under browser_dialog; a swallowed_rejection that a comment explains, that waits on a promise someone else holds, or that sits in a test; and a mutates_argument_property that edits a syntax-tree parameter in place. Each is ordinary, correct TypeScript often enough that only a reader with the surrounding context can tell. Confirm one before acting on it — an as at a boundary where the value really was checked is the honest spelling, not a defect.

format_findings.py files candidates as Investigate, not Refactor; analyze_all.py counts them in summary.total_candidates, lists them under their own heading rather than among the high-severity issues, and never lets one buy a RECOMMENDATIONS bullet; and code-overview excludes them from a code-health score, so a candidate never costs a grade.

Use the project's own tools when they exist

The detectors read syntax. The compiler reads types, and that is where the expensive bugs are. run_external_tools.py resolves each tool from the project's own node_modules/.bin (then PATH), runs every available one in non-mutating check mode, and merges the output into this skill's findings shape:

python "$SKILL/scripts/run_external_tools.py" .                 # run all available (check only)
python "$SKILL/scripts/run_external_tools.py" . --format json   # {tools_run, missing_tools, findings}
python "$SKILL/scripts/run_external_tools.py" . --tools tsc,eslint
python "$SKILL/scripts/run_external_tools.py" . --fix           # also run prettier/eslint --fix (MUTATES)
python "$SKILL/scripts/run_external_tools.py" . --run-coverage  # run the test suite first (SLOW, EXECUTES CODE)

Four of these answer questions the detectors structurally cannot:

• tsc — the types themselves. find_type_gaps.py finds where checking was switched off; only the compiler finds what it would have caught. Run tsc --noEmit before believing any refactor preserved behavior.
• typescript-eslint (type-aware rules) — no-floating-promises, no-misused-promises, no-unnecessary-condition, no-unsafe-*. The async detector here catches the syntactic cases; these catch the rest, including the one it deliberately skips (an async callback handed to something that discards its promise, such as a React onClick).
• npm/pnpm/yarn audit — known advisories. find_dependency_issues.py can say a range is unpinned; only an advisory database can say the version you have is vulnerable.
• coverage — what actually executed. find_untested_modules.py answers "does a test reach this through its imports"; coverage answers "does this code ever run".

When a tool is missing, ask before installing. The script never installs anything and never runs npx in download mode; it lists each absent tool with an npm install --save-dev hint under missing_tools — except audit, whose hint names the package manager the lockfile calls for, since that is what is missing. When that list is non-empty and the tools would help, ask the user whether to install them (e.g. via the AskUserQuestion tool) and only install on confirmation.

Reviewing a change request (diff lens)

For an AI-written feature or PR, review what changed, not the legacy around it. analyze_diff.py runs the file-level detectors against only the changed files, and by default only the added/modified lines:

python "$SKILL/scripts/analyze_diff.py"                 # working tree vs. merge-base with the default branch
python "$SKILL/scripts/analyze_diff.py" origin/main     # vs. an explicit base ref
python "$SKILL/scripts/analyze_diff.py" --format json | python "$SKILL/scripts/format_findings.py"

Whole-tree detectors (tsconfig, import cycles, dependency hygiene, dead code, duplication, over-engineering, untested modules) need the full project — run those with analyze_all.py separately. See references/ai-generated-code.md for the review stance.

Reference index (load on demand)

Keep SKILL.md lean; pull in depth only when a review needs it.

Over-engineering anti-patterns (quick reference)

Code smells (quick reference → fix)

When NOT to simplify

• Untested code — write a characterization test first, then refactor.
• Hot paths — measure before trading clarity for speed.
• Code being replaced or retired soon.
• Complexity genuinely forced by an external API, a framework's contract, or a real present requirement. A cast at a boundary you do not control, with a runtime validation beside it, is the right answer — not a smell.
• Cold code that never changes and blocks nothing — leave it; fix what churns.

Relationship to ESLint and tsc

These scripts complement the ecosystem's tools; they do not replace them. Some detectors overlap rule sets: eqeqeq, no-var, prefer-const, @typescript-eslint/no-explicit-any, no-non-null-assertion, ban-ts-comment, no-floating-promises, prefer-optional-chain, prefer-nullish-coalescing, prefer-includes, no-namespace, no-extraneous-class, no-debugger, no-console. If the project already runs those rules, disable the matching detector via --ignore (or drop its category with analyze_all.py --skip) to avoid double-reporting — or better, run the real tools with run_external_tools.py and lean on the detectors only for what the tools do not cover.

The unique value here is the design and architecture detectors (encapsulation leaks, cohesion, feature envy, data clumps, temporary fields, over-engineering, import cycles, duplication, type-shape duplication), the repo-level checks (tsconfig strictness audit, dependency reconciliation, untested-module and test-smell detection that scaffold a safety net), the AI-code tells (scaffolding, ignored options, duplicate definitions), the fact that it runs with no install and no node_modules — plus the judgment guides, which no linter provides.