Code Audit Deep

Code Audit (Deep)

Line-level audit. Companion to architectural-hotspots. Hotspots ranks files by structural shape (fan-in, fan-out, LOC, cycles); this skill reads files and emits specific, actionable findings with line numbers and fix sketches.

The core failure mode this skill exists to prevent: producing vague "consider refactoring X" advice instead of commit.rs:426 — clock sampled per element in hot loop; hoist or seed-and-increment. The first is what a graph tool already said; the second is what the user actually wanted.

Language scope

This skill is language-agnostic. The smells below are concepts that recur across stacks; the parenthetical examples are illustrative for a few common languages but never exhaustive. When you read a file, map each smell to the equivalent construct in the target language:

• "fallible call return value discarded" covers let _ = f() (Rust), bare f() ignoring its error return (Go), try: f(); except: pass (Python), unawaited f() returning a Promise (TS / JS), _, _ = f() patterns (Lua / Go), f(); // ignore everywhere.
• "owned-when-borrow-suffices" covers Vec<String> vs Vec<&str> (Rust), []string copies vs slice aliases (Go), deep-copying lists (Python), .slice() cloning vs index access (JS), std::string vs std::string_view (C++).
• "lock-when-atomic-suffices" covers Arc<Mutex<u64>> (Rust), sync.Mutex for a counter (Go), threading.Lock around an int (Python), Object.synchronized for a primitive (Java).

Never report a finding using a language construct the project does not use. Substitute the project's equivalent before writing the fix sketch.

When to run

Use whenever the user wants findings inside code rather than rankings across files. Triggers include:

• "audit commit.rs" / "review plan.rs"
• "find perf issues in this module"
• "what could go wrong in apply_changes?"
• "deep review of the parser"
• "where is this slow?"
• After running architectural-hotspots and the user wants the next level of detail on the flagged files.

Do not use when the user wants:

• Architectural overview / rankings → that's architectural-hotspots.
• Style / formatting issues → clippy / eslint / ruff / staticcheck etc.
• Security review of specific CVE classes → a dedicated security skill should run. Overlap is fine; this skill won't hunt for known CVEs.
• A single trivial bug fix in a known file — just fix it.

Method

The strength of this skill comes from depth, not breadth. Better to audit three files thoroughly than ten superficially.

0. Find the project's record of already-rejected findings before reading any code: DECISIONS.md, ADRs, a "rejection ledger" or "verified negatives" section, *findings*.md, dismissed-issue labels. Ask the user if nothing turns up and the repo looks mature. Re-raising something the team already considered and rejected on the record costs more credibility than the finding was ever worth, and it is the fastest way to get an audit ignored wholesale. When you drop a candidate for this reason, name the entry — that tells the user their record is being read, not that you found nothing.

1. Pick targets.

   • If the user named files, those are the targets.
   • If not, and a recent hotspots report exists, take the top 3-5 files by combined signal (god + hub, god + tangle, or any file inside a cycle).
   • If neither, ask the user which files — not "should we audit?" but "which files do you want audited?" so the answer drives work.

2. Read every function body end to end. Headers and type signatures are not enough. A finding requires a line number, and a line number requires having read the line. Do not skip a function because its name sounds boring or routine — apply_changes, commit_one, cleanup, recover, flush often hold the subtle ordering bugs. Skim-reading is the dominant failure mode of this skill — defend against it actively by asking, for each function, "if this had a bug, where would it be?".

   Reading until the finding appears is a separate failure from reading too little, and it feels like diligence. You read the body, you found the line that confirms the bug, and you stopped — one statement before the guard that made it safe. Reading Motion::Last => adj.upper() and reporting an unbounded scroll is correct up to the very next statement, adj.set_value(to.clamp(lower, ceiling)), which disproves it. The fix is a habit, not more effort: after a candidate appears, keep reading to the end of the enclosing block and ask "what here would make this not a bug?" — then answer it before writing the finding.

3. First pass: glyph sweep. Before going deep on novel findings, do a fast scan at the language-glyph level — every smell below lists concrete syntactic glyphs in several languages. These glyphs are where the obvious bugs hide; missing them to chase one interesting novel finding is a regression of attention. After the glyph sweep produces its candidate list, then go deep on higher-order concerns (durability ordering, coupling, axis- specific structural issues).

4. Audit along the eight axes (next section). Most files only hit three or four. Don't reach for findings in an axis that genuinely doesn't apply.

5. Emit findings in the fixed format (later section). Categorized. file:line. One-line symptom. One-line fix sketch. No paragraphs of prose around each finding — the user is reading for signal.

6. Prove what you can; label the rest. For each finding, name the command that would falsify it, and run it. If no such command exists, mark the finding traced rather than proved. Both are publishable — a traced finding backed by an honest label is useful, a traced finding presented as measured is a liability.

   The trap this closes is specific and common: running a real check near the claim and letting it stand in for the claim. Reading Motion::Last => adj.upper(), measuring how the toolkit clamps adjustments, and reporting the finding verified — while the very next statement was adj.set_value(to.clamp(lower, ceiling)) — is a measurement of the wrong thing, and the label "verified" is what makes it damaging. The question is never "did I run something?" but "would this command have come out differently if the finding were false?"

   Before hand-rolling a harness, check whether the language already has the tool: mutation testers, fuzzers, sanitizers, and coverage tools mechanize a whole class of proof, and a replica you wrote this afternoon is a slower substitute you also have to trust. Verify what the tool actually does before citing it.

7. Pick the top 3-5 highest-leverage fixes out of the full list and name them at the end. This is the deliverable the user will actually act on first.

The eight axes

Each axis answers a different question. They are deliberately orthogonal — a finding belongs in exactly one category.

1. Perf

What is making this slow that shouldn't be?

Smells:

• Syscalls / IO inside a per-element loop. File-system probes (exists / stat / metadata / readdir), opens, reads, network round-trips, DB queries — fine once, expensive per element. Glyphs: Rust std::fs::{metadata,exists,read} / Path::exists; Go os.Stat / os.Open; Python os.path.exists / open(); JS fs.existsSync / fs.statSync; C stat() / open() / access().
• Clock / RNG / time calls per element. Wall-clock or monotonic-clock samples, RNG draws, UUID generations done per iteration when one sample-and-increment would do. Glyphs: Rust SystemTime::now() / Instant::now() / rand::*; Go time.Now() / rand.Int*; Python time.time() / datetime.now() / random.* / uuid.uuid4(); JS Date.now() / performance.now() / Math.random(); C clock_gettime / gettimeofday / rand().
• Allocation in hot loop. Building a fresh container, formatting a fresh string, copying a non-trivial value each iteration when a reused buffer or borrowed view would do. Glyphs: Rust String::new() / Vec::new() / .to_owned() / .clone() / format!(...); Go []T{} literal / make(...) / fmt.Sprintf / string([]byte); Python list/dict comprehension / f"..." / copy.copy / copy.deepcopy; JS array literal […] / template literal in inner loop / Object.assign({}, …) / spread; C++ std::string/std::vector constructor in loop body.
• Full re-scans where incremental would do. Re-running a regex / parse / hash over the entire post-image to detect changes that could be tracked at write-time. Re-walking a tree that was just walked.
• Redundant parse / compile. Same input parsed twice. Same pattern compiled in two code paths. Same query / plan built per call instead of cached. Glyphs: regex Pattern::compile / re.compile / new RegExp inside a function called per request; SQL prepare per request; tree-sitter Query::new / parser set_language per call.
• Forced materialization. Collecting a stream into a fully materialised container whose only consumer is one-pass iteration. Glyphs: Rust .collect::<Vec<_>>() followed by .iter(); Go full slice from iterator then range; Python list(...) then for x in ...; JS Array.from(...) then .forEach.

How to look: read the body of every loop. Ask "is this O(work-per-item) or am I doing O(work-per-file * items)?".

2. Correctness — error handling

Where do errors silently disappear?

Smells:

• Fallible call whose return value is discarded. Often intentional (best-effort cleanup) but often a hidden bug. If intentional, a comment explains why. If no comment, suspect. Glyphs: Rust let _ = f() / f().ok(); / f().unwrap_or(...) with no log; Go bare f() ignoring err return / _ = f(); Python try: f(); except: pass / try: f(); except Exception: pass; JS unawaited Promise (f(); where f returns Promise) / .catch(() => {}); C++ (void)f(); / discarded int return; C bare unlink(p); ignoring -1.
• Error converted to absent-value type, original error dropped. The error type carried useful information; the caller has lost it. Glyphs: Rust r.ok() / r.err() then dropping the other side; Go if err != nil { return nil } / return val, nil swallowing err; Python try: ...; except: return None; JS try { ... } catch { return undefined }.
• First-error-wins aggregation where the caller needs to see all failures. Common in parallel / batch contexts — collecting results and returning only the first Err hides every other failure. Glyphs: Rust .collect::<Result<Vec<_>, _>>() / results.into_iter().find(|r| r.is_err()) / Rayon .try_reduce; Go errgroup.Wait() returning first non-nil; Python next(e for e in errs if e); JS Promise.all short-circuit vs Promise.allSettled.
• Cleanup / rollback / recover function whose own failures are not surfaced. A rollback that itself calls a fallible operation and returns () is hiding partial-state bugs. Glyphs: any rollback / cleanup / recover / compensate / unwind function whose signature returns () / void / None but whose body invokes fallible IO (rename, unlink, flush).
• Early-return inside a loop where one bad element should not abort the batch (or vice versa — should fail fast but does not). Glyphs: Rust ? inside for; Go if err != nil { return err } inside range; Python bare raise inside for; JS throw inside forEach.
• match / switch arms that absorb specific error variants without comment — silently classify an error as success. Glyphs: Rust Err(_) => return Ok(...); Go case errors.Is(err, X): return nil; Python except SpecificError: pass; JS catch (e) { if (e.code === 'X') return; }.
• fsync / flush failure ignored — only observable when the OS later loses data. Glyphs: Rust let _ = file.sync_all() / let _ = file.flush(); Go _ = f.Sync() / unchecked Close(); Python os.fsync in a try/except: pass; C bare fsync(fd); ignoring -1.
• Byte-by-byte text manipulation losing UTF-8 / encoding info. Iterating raw bytes and pushing them into a text container as if they were codepoints corrupts every multibyte character. Glyphs: Rust out.push(b as char); Go string(b) where b is one byte of a multibyte rune; Python operating on bytes and decoding wrong / mixing str and bytes; JS String.fromCharCode in a UTF-8 byte loop; C wchar_t cast from unsigned char.
• Identifier / index conflation. Function expects positional index (e.g. 0-based child index), caller passes opaque identifier (e.g. pointer-derived node id) cast to the same numeric type. The types align so the compiler is silent. Glyphs: any as u32 / int(...) / uintptr cast that joins two semantically different numbers; APIs named …_for_index vs …_for_id consumed interchangeably; tree-sitter field_name_for_child vs field_name_for_named_child mismatch is the canonical example.

3. Correctness — durability / ordering

If the process dies mid-operation, what state remains?

Smells, applicable any time the code mutates shared / persisted / external state:

• Effect ordered before its precondition is durable. Deleting backups before fsyncing the new content's parent directory. Removing the old row before the new row is committed. Releasing the in-memory lock before the on-disk update is observable.
• Commit before fsync. Reporting success before the write is durable.
• Cache invalidation before write. Readers can observe the gap.
• Lock released before the protected state is fully published.
• Compensating-action ordering wrong. Rollback does steps in same order as forward path, leaving an inconsistent intermediate.
• Parent-directory fsync skipped. On POSIX, a fsynced file in an un-fsynced directory can vanish on crash. Easy to forget.

How to look: trace the order of side-effects through each function that touches shared state. Ask "if I crash here, can a reader see the new state without the old state, or vice versa, in a way the contract forbids?".

4. Concurrency / pool placement

Are concurrency primitives installed in the scope the caller expects?

Smells:

• Worker pool / thread pool installed around the wrong scope. The user-facing flag (--threads N, --concurrency M) is honored in one phase but ignored in another because the pool wasn't scoped wide enough.
• Async runtime spawned per call instead of shared.
• Connection pool / channel pool created at wrong unit of work (per-request when it should be per-process, per-process when it should be per-tenant).
• Per-thread state read from cross-thread context (or vice versa).
• spawn / Promise.all / errgroup with no bound on parallelism — accidentally unlimited fan-out under load.
• Single-threaded fast-path inside otherwise-parallel pipeline — a serial bottleneck masked by aggregate timing.

How to look: find every place the project defines a pool, an executor, a runtime, a spawn site. For each one, identify the scope it covers and the scope the caller assumed. Mismatches are the bug.

5. Memory shape

What is held in memory that doesn't need to be, or held twice?

Smells:

• Pre- and post- of the same data held together. Struct holds full new text and the rendered diff of old→new. The diff already encodes the new text relative to old — keeping both doubles per-item footprint.
• Owned-when-borrow-suffices. A copy is taken where a view or reference into the original would be safe given lifetimes / ownership. Glyphs: Rust Vec<String> vs Vec<&str> / String field where &str suffices / .to_owned() on a value that outlives the borrow; Go full-slice copy append([]T{}, src...) vs slice alias; Python list(other) vs reference / copy.copy; JS [...arr] / Array.from(arr) for read-only iteration; C++ std::string vs std::string_view.
• Lock-when-atomic-suffices. A mutex protects a value that fits in a machine word and could be an atomic. Glyphs: Rust Arc<Mutex<u64>> / Arc<Mutex<bool>>; Go sync.Mutex around an int64 counter; Java synchronized for a long / boolean; Python threading.Lock around an int.
• Variant-size disparity. One large variant of a discriminated union inflates every instance — boxing the large variant fixes. Glyphs: Rust enum { Small(u8), Huge([u8; 4096]) }; C++ union / std::variant with size-imbalanced alternatives; Go interface holding inconsistently-sized concrete types.
• Long-lived cache with no eviction policy.
• Whole-input buffer where a stream would work — tool that could pipeline reads holds the entire input in memory. Glyphs: Rust fs::read_to_string / Vec::from_iter on a stream; Go io.ReadAll; Python f.read() then process; JS await response.text() on a 1GB response.
• Optional / nullable field that is always populated in practice — the absent case is dead, the wrapper costs bytes and forces every reader to handle a case that cannot occur. Glyphs: Rust Option<T> field whose constructor always sets Some(...); Go *T always non-nil; Python Optional[T] annotation but never None; TS T | undefined that's never undefined in practice.

6. Function-level complexity

Where is one function doing too much, or too dangerously?

Visible only by reading function bodies — graph tools miss these because they live below the file boundary.

Smells:

• Recursion with no depth bound — especially on data derived from user input (parser, walker, AST visitor, JSON decoder). Stack-overflow vector. The iterative-stack version is usually one rewrite away.
• Function > ~80 lines doing more than one thing.
• Dispatch / switch / match with > ~10 arms — often a table or registry pattern is clearer and easier to extend.
• Closure / inner function capturing many outer mutable variables — usually a struct trying to be born.
• Function with > 5 parameters — parameter object or builder.
• Two functions whose bodies differ only by a constant or a branch — collapse to one parameterised version.

7. Coupling — semantic, not graph

What couples that the import graph cannot see?

architectural-hotspots sees file-to-file imports. This axis catches the rest:

• Two modules look orthogonal but both call the same set of helpers from a third — they share an implicit protocol that wants to be made explicit.
• A "library" module that branches on a value it gets from exactly one caller — the abstraction has one user. Inline it or own the branch in the caller.
• Dual orchestrators with overlapping responsibilities — one orchestrator can usually absorb the other, or both should delegate to a thinner core. Hotspots flags both as "tangles" without naming the relationship.
• Type defined in module A, used only by module B — wrong home.
• "Generic" helper used only by one site — not generic, just premature.
• Two functions doing the same operation in slightly different ways across modules — same shape, divergent details.

8. Enforcement — is the invariant actually held?

The project states a rule. Does anything make it true?

Every other axis asks what the code does. This one asks whether the guard on the code can fire. It is the axis that catches a check that passes for the wrong reason, and it is invisible to the other seven because nothing is wrong with the code under the guard — the guard is wrong about its own reach.

Classify each invariant the project claims into three states. Only one of them is safe:

• Derived — the check computes the invariant from the thing under test, so it moves when the code moves. Safe.
• Asserted — the check names the value or symbol literally. It holds today and goes stale silently the moment the code is renamed, inlined, or restated somewhere the pattern does not reach. Finding.
• Absent — the rule exists only in prose: a comment, a README, a docstring, a convention. Nothing enforces it. Finding.

Smells:

• Assertion that cannot fail. The check's inputs and its expected value both derive from the single artifact under test, so it is true by construction. Tell: a docstring promising a comparison ("compares X against the Y baseline") beside an assertion that reads only X. A sibling step in the same block that does carry a floor is strong evidence the missing one was an oversight.
• One-sided check on a two-sided property. Asserting the bad state disappears without asserting the good state survives. A fix that suppresses everything passes. Anything with a mirror, an inverse, or an on/off needs both exercised.
• Gate whose selection rule cannot reach the violation. A source-scanning check has several independent filters — which files it reads, at what granularity, which keywords it matches, what name shapes it accepts. A violation outside any one filter is invisible while the gate stays green. Audit the rule by re-running it with one filter widened at a time.
• Literal where the gate matches a symbol. A gate keyed on const NAME cannot see the same value written as a bare number, and a gate matching named constants cannot see a magic literal — including the same value restated in a second language.
• Invariant stated only in prose. "Callers must…", "never call this before…", "keep these two lists in sync" next to shared mutable state or a required order. See code-smells' Invariant by Convention entry — that skill names the design smell, this axis reports the specific unenforced site.
• Documentation citing something that no longer exists. A doc naming a test, gate, or symbol that was deleted. Especially where a gate exists to catch exactly this and its filters miss the file, the verb, or the qualified name shape.

How to look: list every rule the project asserts about itself — gate scripts, CI steps, test names, "must"/"never" comments, README guarantees. For each, find the code that makes it true. Read the gate's implementation, never its name — the name describes the intent, and the gap between intent and selection rule is the finding.

Note this axis judges the project's own checks, which includes any you or a predecessor added. A check you wrote and never watched fail belongs here.

Output format

Use this exact template. Counts in the headings let the user scan volume at a glance. Omit any section with zero findings — do not pad.

**Perf (N)**
- `file:line` — symptom in ≤8 words. Fix: <one short phrase>.

**Correctness (N)**
- `file:line` — symptom. Fix: <one short phrase>.

**Durability/ordering (N)**
- `file:line` — symptom. Fix: <one short phrase>.

**Concurrency (N)**
- `file:line` — symptom. Fix: <one short phrase>.

**Memory (N)**
- `file:line` — symptom. Fix: <one short phrase>.

**Complexity (N)**
- `file:line` — symptom. Fix: <one short phrase>.

**Coupling (N)**
- `file_a:line + file_b:line` — symptom. Fix: <one short phrase>.

**Enforcement (N)**
- `file:line` — invariant, and which of derived/asserted/absent it is.
  Fix: <one short phrase>.

**Top targets**
3-5 highest-leverage fixes, named with file + symptom.

Mark any finding you could not falsify by running something with a trailing [traced]. Say once, at the end, how many findings were proved and how many traced. A reader who knows which half is which can act on both; a reader who cannot tell has to re-check all of them.

Examples

Good:

Perf (2)

• commit.rs:426 — clock sampled per nonce in hot path. Fix: sample once at construction, increment a counter.
• commit.rs:328 — FS-probe syscall per backup entry. Fix: single directory listing, then filter in memory.

Durability/ordering (1)

• commit.rs:301 — backup deleted before parent-dir fsync. Fix: fsync parents first, then unlink backups.

Concurrency (1)

• main.rs:343 — --threads flag scoped only around planner; apply phase runs on global pool. Fix: install scoped pool around the whole pipeline.

Bad (vague, no line, no fix):

Consider refactoring commit.rs for performance. Some operations may be inefficient.

Bad (over-prosaic, paragraph form):

Looking at commit.rs, around the nonce generator, I noticed that it samples the wall clock, which involves a syscall. This could be slow if called many times, although it depends on the use case…

The format constraint matters because the user reads dozens of findings; signal density wins.

Calibration

Finding counts depend far more on the codebase than on the line count, so treat any number here as a prompt to re-examine, never as a quota.

Do not scale a per-file range up across files. A rough anchor for one unfamiliar ~500-line file with no local discipline is a handful to a dozen findings. Four files is not four times that. On a mature codebase — dense justifying comments, its own gates, a written record of rejected findings — single digits across several files is the honest result, and the padding pressure is the real risk. Auditing 2400 lines and reporting eight findings with a stated coverage claim is a better deliverable than thirty with twenty-two of them reaching.

Two outcomes worth stating plainly rather than hiding:

• A file yielding one finding, or none. Say so. "I read all of ops.rs and found one thing" is information. Quietly padding to three destroys the signal in the other two.
• A low total because the project already rejected these. Name the record you read (step 0). That is coverage, not absence.

More than ~25 findings on one file almost always means the bar dropped. Cut to the ones that survive the table below.

Every finding must survive the "would the user act on this?" test:

When in doubt, drop it.

Anti-patterns

• Skim-reading. Defaulting to function signatures and headers, emitting findings about "what the function probably does". Always read bodies before claiming a finding. The most bug-prone functions are the ones whose names sound mundane — read them first, not last.
• Reporting in the wrong language. Writing a Rust-flavoured fix sketch for a Python project. Match the project's actual stack.
• Findings the codebase already addresses. A safety annotation, a documented "ignore error here because X", a comment explaining why a clone is required — not findings. Read the comments.
• …but a comment explaining a mechanism is a hypothesis, not a reason to drop. "Safe because the toolkit never reuses the widget", "these paths would agree only by accident", "this cannot overflow because the caller validates" — each is a factual claim about behaviour, and the axes apply to it exactly as they apply to code. Dropping a real finding because a comment asserted it was fine is the same error as reporting a false one, and it is harder to catch because it leaves no trace in the output. Check the explanation, then drop it or keep it. (Measured example: a comment justified a re-cut on the grounds that widget reuse was coincidental; a probe under the project's own headless display showed 196 of 197 rebinds returned the item to the same widget, so the stated reason was false and the finding was real.)
• Hand-waving "consider X" verbs. Replace with concrete imperatives: hoist, inline, collapse, extract, box, bound the recursion, aggregate all errors, swap order, scope the pool.
• Trying to be exhaustive. A focused list of 10 real findings beats a sprawling list of 25 mostly-noise. The user picks 3-5 to act on either way.
• Confusing axes. Durability/ordering bugs are not perf bugs. Concurrency-scope mistakes are not correctness in the error-handling sense. Pick the axis that points to the right fix shape.