Skip to content

Add dead-code gate (vulture) - #9

Merged
jonnyspicer merged 1 commit into
mainfrom
chore/dead-code-gate
Aug 11, 2026
Merged

jonnyspicer merged 1 commit into
mainfrom
chore/dead-code-gate

Conversation

@jonnyspicer

Copy link
Copy Markdown
Contributor

Adds a vulture dead-code gate: tools/check-dead-code.sh, a whitelist, and a CI step.

Why a wrapper rather than calling vulture directly

The obvious setup excludes tests/, which makes anything used only by
tests look dead — the single largest source of false positives. This scans
tests so their usage counts, then drops findings whose location is a test
file, which is the behaviour you actually want.

Combined with excluding build artefacts (a stale source copy that doubles every
finding) and ignoring framework-dispatched decorators, that took the estate-wide
count from 217 to 102 without a single judgement call.

The whitelist has two sections, deliberately

Contracts — names genuinely referenced by something vulture cannot see:
stdlib http.server handler methods, ssl.SSLContext attributes, generated
wire models. These are permanent.

UNREVIEWED — 5 entries in this repo that appear genuinely dead and
need a decision: delete, or finish wiring up whatever was left unfinished.

This section is a backlog, not an exemption. Nothing here has been deleted,
because "this config field is never read" usually means someone left work
half-done, and that is a call for whoever wrote it. Listing them keeps the gate
green today, so it starts catching new dead code from this PR onward
rather than waiting on a cleanup that may take weeks.

Verified

Gate passes on this branch, and fails (exit 1) when a dead function is added —
tested, since a gate that cannot fail is worthless.

🤖 Generated with Claude Code

Adds tools/check-dead-code.sh, a vulture wrapper shared across the org, plus
a whitelist and a CI step.

The wrapper exists because the obvious vulture setup — excluding tests — makes
anything used only by tests look dead, which is the largest single source of
false positives. This scans tests so their usage counts, then drops findings
whose location is a test file. Across the estate that took the raw finding
count from 217 to 102 without a single judgement call; build artefacts and
framework decorators account for the rest.

The whitelist separates two things that are usually conflated: names vulture
cannot see being used (framework contracts, wire formats), and names that
appear genuinely dead but need a decision. The second section is a backlog,
listed so the gate is green today and starts catching NEW dead code
immediately.
@jonnyspicer
jonnyspicer merged commit a745bb8 into main Aug 11, 2026
2 checks passed
@jonnyspicer
jonnyspicer deleted the chore/dead-code-gate branch August 11, 2026 01:59
Purple10101 added a commit that referenced this pull request Aug 11, 2026
main went red on the merge of #7 without either branch being wrong. The gate
landed in #9 against the models generated from the pre-revision spec; #7 adopted
the 2026-08-10 revision and regenerated them. Two green branches, one red merge.

Seventeen findings, all in retina_telemetry/wire/models.py, and six distinct
names. None is dead code:

  model_config, root   pydantic reads both itself, so nothing we write names
                       them. `root` is RootModel's field.
  stopping, unknown    spec vocabulary we deliberately never send — a final
                       heartbeat during shutdown means a network call on the way
                       out (Q17), and we distinguish "have not looked" from
                       "looked and could not tell" by omitting the field.
  public, private      carried as strings, so the enum members are never named.

The generated-models section is rewritten rather than extended, because the old
one had rotted in a way worth recording. Its entries carried line numbers that
now point at unrelated code, and its `Error` entry no longer means what it did:
datamodel-codegen collided on the name, so `Error` is now the item type of
HeartbeatRequest.errors and the actual error response schema regenerated as
`Error1`. Line numbers are gone from this section for that reason — they churn
on every regeneration and going stale silently is worse than not having them.

Deliberately not excluding the generated file wholesale. A new name appearing in
the wire contract is worth a glance, and a failing gate is how that glance gets
scheduled; the cost is one whitelist edit per spec revision.

Note for whoever hits this next: tools/check-dead-code.sh reports "no dead code
found" when vulture is not installed, because the missing-module error goes to
/dev/null and the empty result reads as success. It passed locally for that
reason while CI failed. The script is shared from offworldlabs/ops so the fix
belongs upstream, not here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant