brainstorming
brainstorming
You MUST use this before any creative work - creating features, building components, adding functionality, modifying behavior, or planning anything new, in software or out of it (a talk, a business, a renovation).
code-simplifierCleanup pass that edits the working tree — over a session's uncommitted changes, or a named path or whole codebase. Reads `git diff` (or the named target) and simplifies, consolidates, and aligns code with the existing codebase — modernize syntax, cut unnecessary complexity and slop, consolidate duplicated logic, catch efficiency issues. Not a bug hunt: def…
Independent GitHub snapshot · Automated classification · Original documentation governs
git diff (or the named target) and simplifies, consolidates, and aligns code with the existing codebase — modernize syntax, cut unnecessary complexity and slop, consolidate duplicated logic, catch efficiency issues. Not a bug hunt: defects are reported, not fixed. Use after a substantive working session, or when asked to clean up, simplify, reduce slop, consolidate, modernize, tighten up, de-slop, or scan a codebase. For @cyanheads/mcp-ts-core projects, includes specific transformations for tool/resource/prompt definitions, the ctx pattern, error factories, and framework idioms.
metadata:
author: cyanheads
version: "1.7"
audience: external
type: workflowCleanup pass over a session's changes or a named target. Reviews the code in scope, understands how it fits the existing codebase, and makes targeted improvements — modernizing syntax, removing unnecessary complexity, consolidating duplicated logic, catching efficiency issues. Prioritizes codebase cohesion over local perfection.
Every change must earn its keep. A simplification that doesn't meaningfully improve clarity, correctness, or cohesion is noise. Don't refactor for refactoring's sake. Don't create new files, abstractions, or utilities unless they solve a demonstrated problem. If the existing code works and is readable, leave it alone. The goal is a cohesive codebase, not a pristine one.
Two scopes; the caller's wording picks one, and the diff is the default.
git status to see the shape of the working tree, then git diff HEAD for all uncommitted changes (staged and unstaged). Untracked files never appear in the diff — list them with git ls-files --others --exclude-standard (git status collapses a new directory to one line) and read them directly. If the diff is empty and there are no untracked files, review the last commit (git diff HEAD~1 HEAD); if that is also empty, say the tree is clean and stop. Don't go hunting through the codebase for files to improve.Don't review changes in isolation. Before any modifications:
CLAUDE.md / AGENTS.md names; absent one, take it from package.json scripts — devcheck if present, else check, else the separate typecheck / lint scripts — or a Makefile check target. Python projects gate on uv run ruff check, uv run ruff format --check, and the configured type checker. Add the test suite when the gate doesn't run it — read the script rather than assume (a devcheck often stops at lint and typecheck); without tests, nothing shows behavior survived the pass. In a Bun project that tests with Vitest, run bun run test — bare bun test bypasses the script and runs Bun's own runner. If the gate is already red, say so in the summary and don't attribute the failure to your changes.Evaluate the changes across these dimensions. Not every dimension applies to every diff — skip what's irrelevant.
console.log, print, debugger) that aren't the program's real output or the project's logger. An export nothing imports is dead in an application or a package-internal module; on a published package's public surface it is API — leave it and note it in the summary.any, as casts that paper over a mismatch, non-null !, @ts-ignore, and Python's # type: ignore / cast(). Each is a claim the compiler couldn't check: replace with a narrowed type, a type guard, or a parse at the boundary. Keep the ones documenting a genuine type-system or third-party-types limitation — confirm the limitation is gone before removing one — and prefer @ts-expect-error with a one-line reason over @ts-ignore.catch {}, catch { return null }, and try blocks that log and continue. A fallback that hides a failure is worse than the crash it prevents: rethrow or let it propagate. When wrapping, preserve the chain (new Error(msg, { cause }), raise X from err).?? '', || [], ?? 0, .get(key, {}) standing in for a value that must exist. The default turns a missing config key or a broken upstream into quietly wrong output further down. When the type already rules out absence, the default is dead — drop it; when the value is optional in the type but required in fact, replace the default with an error that names what's missing, where the value is read. Keep defaults only where absence is a legitimate, expected state.Promise.all / Promise.allSettled.Promise.all / asyncio.gather over a caller-sized or otherwise unbounded array fires everything at once. Cap it with the project's existing concurrency helper or a batched loop. A fixed handful of independent calls needs no limit.bun run devcheck plus the test suite (bun run test) is the project gate in Phase 2 step 4 and Phase 4 step 5.src/utils/ and src/errors/ in the project and node_modules/@cyanheads/mcp-ts-core/ for framework exports: pagination helpers, schema builders, retry primitives, and the ATTR_* OTel attribute constants are framework-provided. Raw OTel attribute keys should be ATTR_* imports from @cyanheads/mcp-ts-core/utils.McpError, validationError, notFound, httpErrorFromResponse) over raw throw new Error(). Tool handlers should throw — the framework catches, classifies, and instruments.InvalidParams for input the schema rejects (the framework raises it before the handler runs) and for an upstream 400 mapped by httpErrorFromResponse. ValidationError for a business rule the schema cannot express. NotFound for missing entities. Don't conflate them.ctx.log, ctx.state, ctx.enrich — don't reach for global loggers or request-scoped storage directly. The ctx pattern carries tenant scope and OTel context..describe(). Zod 4 requires z.record(z.string(), z.string()) not z.record(z.string()). Use .optional() rather than .nullish() unless null is semantically distinct from absent.readOnlyHint, idempotentHint, openWorldHint should reflect reality. A read-only tool with readOnlyHint: false gives clients the wrong picture.exactOptionalPropertyTypes boundaries — If a downstream type insists on the field being present-or-not-present (not present-as-undefined), use a mapped widening type at the boundary. The pattern is documented in the framework.format() ↔ structuredContent parity — Different MCP clients forward different surfaces. Tests should assert both surfaces carry equivalent data.initFooService() / getFooService()), the tool definition → service split, and a provider interface the framework selects by config are prescribed convention; the deletion test doesn't apply to them, and a single-implementation service accessor is the framework's seam, not a hypothetical one.git diff, never by setting work aside.When done, report:
file:line.When nothing earned a change, say the code was already clean.
The tables below cover TypeScript and Python. For other languages, apply analogous principles: prefer modern idioms, reduce nesting, eliminate dead code, follow project conventions. Check the project's language floor (tsconfig target/lib, pyproject requires-python) before applying a version-gated row.
| Before | After | Why |
|---|---|---|
const x: Foo = { ... } as Foo | const x = { ... } satisfies Foo | Type-checked without assertion |
let resource = acquire(); try { ... } finally { release(resource) } | using resource = acquire() | Explicit resource management (TS 5.2+) — only when the resource implements Symbol.dispose (await using for Symbol.asyncDispose); otherwise the try/finally stays |
if (x !== null && x !== undefined) | if (x != null) | Idiomatic null/undefined check |
arr.filter(x => x !== null) as T[] | arr.filter(x => x != null) | TS 5.5+ infers the type predicate — no cast; on older TS use an explicit (x): x is T predicate |
import { foo } from './index.js' (a module importing through its own barrel) | import { foo } from './foo.js' | Inside a module, import siblings directly — routing through the module's own barrel invites import cycles. Across modules, the public barrel is the right entry point; leave those imports alone |
import { readFile } from 'fs/promises' | import { readFile } from 'node:fs/promises' | node: protocol — unambiguous, lint-enforced in Biome |
async function f() { const a = await x(); const b = await y(); } | const [a, b] = await Promise.all([x(), y()]) | Parallel when independent |
value || fallback | value ?? fallback | || also swallows 0, '', and false — use ?? unless every falsy value really should take the fallback |
obj.x !== undefined ? obj.x : fallback | obj.x ?? fallback | Nullish coalescing — equivalent only when null should take the fallback too |
if (a) { if (b) { if (c) { ... } } } | Guard clauses with early returns | Reduce nesting |
try { risky() } catch (e: any) { ... } | try { risky() } catch (e) { ... } | Under strict the catch binding is already unknown; narrow with a type guard before use |
catch (err) { throw new Error('load failed') } | throw new Error('load failed', { cause: err }) | Preserve the cause chain |
[...arr].sort(cmp) / arr.slice().sort(cmp) | arr.toSorted(cmp) | Non-mutating array methods (ES2023) — also toReversed, toSpliced, with |
arr[arr.length - 1] | arr.at(-1) | ES2022 — typed T | undefined: equivalent under noUncheckedIndexedAccess, a new undefined to handle otherwise |
arr.reduce((acc, x) => { (acc[key(x)] ??= []).push(x); return acc }, {}) | Object.groupBy(arr, key) | ES2024 — returns a null-prototype object whose values are typed T[] | undefined; Map.groupBy for non-string keys |
let resolve!: (v: T) => void; const p = new Promise<T>((r) => { resolve = r }) | const { promise, resolve, reject } = Promise.withResolvers<T>() | ES2024 — the deferred without the captured-variable dance |
new Set([...a].filter((x) => b.has(x))) | a.intersection(b) | ES2025 Set methods — also union, difference, symmetricDifference, isSubsetOf; the receiver must be a Set |
const c = new AbortController(); setTimeout(() => c.abort(), ms) | AbortSignal.timeout(ms) | Built-in timeout signal; combine with a caller's signal via AbortSignal.any([...]) |
JSON.parse(JSON.stringify(x)) | structuredClone(x) | Deep clone that keeps Date, Map, Set, and cycles. Not a drop-in: it throws on functions and strips class prototypes, and Dates stay Dates instead of becoming strings |
enum Status { A, B, C } | const Status = { A: 'A', B: 'B', C: 'C' } as const | enum, namespace, and constructor parameter properties are non-erasable syntax rejected by TS 5.8 erasableSyntaxOnly and Node type-stripping — but switching numeric values to strings changes serialized output; keep values stable if they're persisted |
function f(a: string, b: string, c: string, d?: string) | function f(opts: FnOptions) | Internal functions whose same-typed positional params can be swapped and still type-check. An exported signature is API — leave it |
throw new Error('Bad input') (in a tool handler) | throw validationError('Bad input', { field: 'x' }) | Use framework error factories so the framework can classify and instrument |
const ATTR_KEY = 'mcp.tool.name' | import { ATTR_MCP_TOOL_NAME } from '@cyanheads/mcp-ts-core/utils' | Use framework attribute constants |
| Before | After | Why |
|---|---|---|
Optional[str] | str | None | Modern union syntax (3.10+) |
List[str], Dict[str, int] | list[str], dict[str, int] | Built-in generics (3.9+) |
T = TypeVar("T") + def f(x: T) -> T | def f[T](x: T) -> T | PEP 695 generics (3.12+) — also class C[T]: |
TypeAlias = Union[A, B, C] | type ABC = A | B | C | type statement (3.12+) |
if isinstance(x, Foo): a = x.a; b = x.b | match x: case Foo(a=a, b=b): ... | Structural pattern matching (3.10+) where it destructures — not as a replacement for a flat equality if/elif chain |
class Config: def __init__(self, a, b, c): self.a = a ... | @dataclass(slots=True) class Config: a: str; b: int; c: float | Less boilerplate, built-in eq/repr; frozen=True when instances shouldn't mutate |
results = []; for item in items: results.append(transform(item)) | results = [transform(item) for item in items] | Idiomatic comprehension |
[items[i:i + n] for i in range(0, len(items), n)] | itertools.batched(items, n) | 3.12+ — works on any iterable, not just sequences; yields tuples, not lists |
f = open('x'); try: ... finally: f.close() | with open('x') as f: ... | Context manager for resources |
os.path.join(d, n), os.path.exists(p), open(p).read() | Path(d) / n, p.exists(), p.read_text() | pathlib over os.path string juggling |
datetime.utcnow() / datetime.utcfromtimestamp(t) | datetime.now(UTC) / datetime.fromtimestamp(t, UTC) | Deprecated in 3.12 — the old calls return naive datetimes that compare wrong against aware ones |
zip(a, b) where the inputs must match in length | zip(a, b, strict=True) | 3.10+ — a mismatch raises instead of silently truncating; plain zip stays where truncation is intended |
m = pattern.match(s) then if m: use(m) | if (m := pattern.match(s)): use(m) | Walrus operator where it removes a throwaway assignment |
"Hello " + name + "!" | f"Hello {name}!" | f-string over concatenation |
except Exception: pass / except Exception as e: log(e) | except SpecificError: with real handling, or no try at all | Catch only what you can handle; everything else propagates (see Swallowed errors) |
from module import * | from module import specific_name | Explicit imports only |
Sequential await for independent I/O | async with asyncio.TaskGroup() as tg: tg.create_task(a()); tg.create_task(b()) | Structured concurrency (3.11+) — cancels siblings on failure and raises an ExceptionGroup; asyncio.gather(..., return_exceptions=True) stays correct when every result is wanted regardless of failures |
Leave code alone when:
data to result isn't worth the churn.return await inside try / finally. Collapsing it to return is not equivalent — the promise settles outside the block, so catch never fires and finally runs early. Only strip await from a return in plain function-body position.logger.info("loaded %s in %sms", name, ms) defers formatting until the record is emitted — don't turn it into an f-string.Promise.all.dist/ are regenerated, not edited — skip them even when they appear in the diff..describe() prose. They are the contract an LLM client reads — tightening them for brevity degrades the surface. Treat them as API text, not as comments.