hotspots v1.35.1–v1.35.4: an audit, twelve fixes, four days

Four patch releases in four days, all tracing back to the same internal weakness audit: suppression comments silently doing nothing outside TypeScript, three languages' CFG builders undercounting real branches, policy and config validation gaps, and a git-log-walk gating bug that showed up twice.

Stephen Collins ·

I shipped four patch releases of hotspots between August 26th and 30th — v1.35.1 through v1.35.4, twelve merged PRs total. None of them are individually a big feature. What’s worth writing up is where they came from: line them up and most trace back to an internal weakness audit rather than an incoming bug report, and one bug — a git-log walk not respecting a skip flag — showed up twice, in two different signals, months apart.

v1.35.1 — the language gap nobody had checked for

#136 — a swc lexer panic that took the whole scan down with it. parser.parse() called into swc’s lexer without a panic boundary, so a malformed numeric JSX entity (�, a surrogate half outside valid Unicode) inside one file could crash the entire analysis run instead of being skipped like every other bad-file case already was. The fix wraps the call in std::panic::catch_unwind and degrades to the existing warning: skipping ... pattern. I didn’t just trust the wrapper — I rebuilt the release binary and ran it against the actual swc-project/swc repository, which contains the real fixture that triggers this panic in its own test suite. It hit the panic, logged the warning, and exited 0 with a valid report instead of crashing.

#137 — suppression comments were a no-op outside TypeScript and JavaScript. This is the more interesting one. // hotspots-ignore comments work by calling extract_suppression, but that function was only ever invoked from the TS/JS-specific discovery path — every other language’s parser (Go, Python, Java, C#, Rust, C) set suppression_reason: None unconditionally, each with the identical comment // Will be extracted separately. That follow-up was never written. A suppression comment on a Python or Go function parsed fine and simply did nothing, with zero indication to the person who wrote it that CI policy gating was still watching that function. The fix moves suppression extraction to the one call site every language funnels through, and adds a Language::suppression_comment_prefix() method since the marker check was hardcoded to // and Python doesn’t use that syntax. The regression tests that would have caught this originally called discover::discover_functions directly and only ever exercised the TS path — the new tests go through the real end-to-end analyze_file_with_config pipeline instead.

v1.35.2 — five small correctness gaps, same day

Five PRs landed within about an hour of each other on the 27th, each closing a specific numbered issue from the same audit pass:

  • #147 — a missing parent snapshot silently skips all policy evaluation for that run, with no warning printed anywhere. hotspots analyze --policy would exit 0/”passed” on a fresh CI runner or an evicted snapshot cache, regardless of how risky the actual change was. Now it warns loudly, specifically when a parent SHA exists but its snapshot file doesn’t — not on a genuine root commit, where skipping is correct.
  • #148hotspots prune --unreachable only treated refs/heads/* as reachable, so a detached-HEAD CI checkout (no local branches at all) saw every snapshot as unreachable, and a tag pointing at a deleted local branch lost its history on the next prune. Tags and remotes are now reachable refs too.
  • #149 — a pure rename of an already-Critical function produced a false-positive “introduced as Critical” policy violation in CI, because evaluate_critical_introduction never consulted rename_hint to check whether a “new” entry was actually a rename of something that was already Critical before the move.
  • #150 — a config with all four scoring weights (cc/nd/fo/ns) set to zero passed validation cleanly — each value is individually in range — while silently collapsing every function’s risk score to zero. hotspots config validate now checks joint degeneracy after resolving defaults, so an omitted weights block or a partial zero-out still works as expected.
  • #151hotspots config show labeled exclude patterns “custom” any time any config file was found, not when that file actually set exclude. Diagnostic-only, but misleading when debugging why a file was or wasn’t scanned.

None of these are dramatic on their own. What they have in common is that each one is the kind of gap you only find by asking “what happens if this specific field is wrong/missing/renamed,” not something a normal test run surfaces on its own.

v1.35.3 — three languages’ CFG builders were undercounting

This release is where the audit found its biggest gaps, and it’s also where I want to slow down, because the pattern across all three language fixes is the same: functions that had real branching were being represented in the control-flow graph as straight-line code, and the cyclomatic-complexity formula (E - N + 2) doesn’t complain when you feed it a straight line — it just quietly returns a smaller, plausible-looking number.

#152 (Java) — ternary expressions, &&/|| short-circuit evaluation, and lambda bodies were all treated as single no-branch nodes, with three TODOs left in the code admitting it. The fix adds a recursive expression visitor that models a ternary like an if/else, short-circuit operators as a condition branching to either the right operand or straight to the join (the skipped path), and lambda bodies through the existing block visitor so list.forEach(x -> { if (...) {...} }) contributes real nodes instead of being skipped. One golden fixture’s CC moved from 3 to 4 — the fix working as intended, not a regression, since the lambda in that fixture has an if inside it that’s now finally counted.

#154 (Python)match statements collapsed into a single stub node, and case bodies were never visited at all — meaning return statements inside a case clause were silently dropped from the graph entirely, not just undercounted. The rewrite mirrors the if/elif-chain pattern this file already uses elsewhere: each case becomes its own condition node chained off the previous one, with a bare case _: treated like an unconditional else.

#155 (Go) — this is the big one. An audit issue noted Go had the thinnest CFG test coverage of any supported language (9 tests versus C’s 40) and asked for a targeted review of goroutines, select, defer, labeled break/continue/goto, and type switches. The audit found the coverage gap was hiding a structural bug, not just missing edge cases: tree-sitter-go wraps every multi-statement block — a function body, an if branch, a switch case — in a statement_list node, and the CFG builder’s catch-all arm treated that wrapper as one opaque, undispatched statement instead of walking into its children. Any block with more than one top-level statement collapsed into a single generic node; any control-flow statement that wasn’t alone in its block was silently skipped. This was invisible in the original 9 tests because every one of them happened to use single-statement bodies. On top of that: type-switch case clauses were dropped entirely (the builder only recognized expression_case/default_case, not type_case), a spurious fallthrough edge inflated CC on switches that already had a default, labeled break/continue ignored the label and always targeted the innermost loop, and goto/labeled statements weren’t handled at all. Twenty new regression tests and four updated golden fixtures later, every function with real branching in the Go test corpus reports a meaningfully higher — and now correct — CC.

#153 — the fix that closed the loop on a bug I’d hit myself, twice. prune.rs’s test helper spins up a disposable git repo in a tempdir to test reachability logic, sandboxed with .current_dir() alone — which isn’t enough, because GIT_DIR (set by git itself for hook subprocesses) overrides current_dir and silently redirects the “isolated” test repo’s git commands at the real repository instead. The PR author found this the hard way: this repo’s own .git/config picked up user.name=Test / user.email=test@example.com after a routine commit, traced back to cargo test running as a pre-commit hook child process. The fix explicitly strips every git-location env var (GIT_DIR, GIT_WORK_TREE, GIT_INDEX_FILE, and others) before the test helper’s git calls. I hit the exact same failure mode independently two days later while preparing v1.35.4, from a worktree branched before this fix had landed on main — same three prune::tests failures, same root cause, already fixed a commit away. Cherry-picked it in rather than re-deriving the same patch.

v1.35.4 — the bug that showed up twice

#158burst_score computation calls its own full-history git log --name-only walk, unbounded and unscoped to whatever path you passed to analyze. --skip-touch-metrics is documented to skip exactly this kind of walk — “no git log calls for churn/recency… skip directed coupling (also a git log walk)” — but the call to with_burst_score() sat outside the flag’s gate entirely, in both snapshot-building code paths. On a repo with unremarkable history this is invisible; on one with a large .git — I found it running hotspots analyze against a single file in TypeScript’s compiler source, 1.2GB of history — it’s the difference between a sub-second command and one that hangs past 30 seconds for no reason connected to the file being analyzed at all.

The reason this one stings a little: it’s the same bug class as v1.34.1’s PR #131, which gated directed_coupling’s git log walk behind the same flag for the same reason. burst_score didn’t exist as a separate signal when that fix landed — it was folded into a shared history-signals walk afterward (v1.33.1) — and the new call site never got the same gate. Fixed the same way: moved with_burst_score() inside the if !skip_touch_metrics block in both build_snapshot_via_db and build_enriched_snapshot, and updated the flag’s help text to actually mention burst_score this time.

Shipping four of these

Each release went out through the same patch-level cargo release workflow — version bump, tag, macOS/Linux/Windows binaries, crates.io, npm, three-platform PyPI wheels, the GitHub Release, and the Homebrew tap, all from workflow_dispatch rather than a manually-bumped Cargo.toml. v1.35.4’s release was the one I watched end to end: eight minutes to bump and tag, then the platform builds and package-registry publishes ran in parallel and finished within about five minutes of each other, GitHub Release publish trailing slightly behind the rest before the run reported success.

If you write suppression comments outside TypeScript, or you’ve been staring at a slow hotspots analyze run on a repo with a lot of history, both are fixed now.

hotspots upgrade

Was this useful? Let me know →

Want to see analysis like this for your own codebase? Try hotspots — free & open source →