Skip to content

Refactoring opportunities

A repo-wide audit of the workspace for duplication, per-call I/O, dead code and over-engineering, with what was assessed and rejected.

A repo-wide audit of packages/ for duplication, per-call I/O, dead code and over-engineering, taken 2026-09-01. The headline: the codebase is already functional and table-driven where it matters — pure decision functions in policy.py, economics as a kind-keyed table with a deployment override, entry points as the one registry. What it found was repetition across packages and I/O done per call.

All four actionable sections have landed. Measured across the two pull requests that carried them, source files only, that is 99 net lines removed against 200 lines of new test, and no architectural change. The estimate here was 450 to 500 net lines and it was high: section 1 turned out to need behaviour rather than deletion, and every fix it named arrived with the tests that pin it.

The audit names files and symbols rather than line numbers, because a line number on a page nobody re-audits rots faster than the observation does. Confirm the current shape before editing.

Every change below must respect: plan_id and snapshot-ID byte stability (or a POLICY_VERSION bump, see ADR-0005), the dependency graph pinned by tests/unit/test_workspace_boundaries.py, and the generated client (openapi.json + tests/bdd/generated) for anything on the wire.

1. Correctness-adjacent duplication — landed

Section titled “1. Correctness-adjacent duplication — landed”

Everything this section held has landed and is gone from here: the copied is_local rule, the two ValidationError mappings, the alphabetical device trap, the double base64 decode, and the classifier billed for facets another classifier already took.

The last of those was the one the audit could not price. A request names classifiers and never facets, so “no facet outstanding” was unknowable until a classifier declared what it answers. DocumentClassifier now reads an optional facets attribute the way it reads device and cost_usd; absent means unknown and the classifier is always asked, and a classifier whose declared facets are all taken for a unit is skipped whole. The three shipped classifiers declare the facets of the taxonomy they loaded.

2. Per-call I/O and hidden state — landed

Section titled “2. Per-call I/O and hidden state — landed”

Everything this section held has landed too. The economics file is parsed behind an lru_cache keyed on its path, mtime and size, so a repriced capability is still believed without a restart. indx/facade.py holds no module global; _default() is an lru_cache(maxsize=1) and a test resets it with cache_clear(). The seven rebuilds of {c.id: c for c in snapshot.capabilities} are one cached_property by_id on CapabilitySnapshot, outside the content hash and outside the JSON schema. And encode expands the plan once and hands the mapping to unrouted and groups, which is what routes_by_page’s docstring always claimed.

Still open from the first of those: the fully pure version — resolve economics once in Router.plan and pass a mapping into decide() — remains the better end state, and still touches indx-benchmark’s import.

Everything this section held has landed and is gone from here. Three items were a different size on contact with the code, and each is recorded rather than quietly adjusted.

The byte-identical provider.py files were twenty, not six. The classifier, extractor, enricher and language distributions carry the same empty descriptors() and the same refusing create() as the loaders, observers and chunkers, so the optional Plugin base in indx-interfaces deleted all twenty rather than the six the audit counted.

The facade injection kwargs went, and the Executor and Embedder protocols with them. Their two protocol pages stayed. What those pages document is two stages of the system, twenty-two links across both languages point at them, and each now names the port a caller calls and says why no protocol is declared for it.

Of the dead re-exports only EXTRA was dead. The rest of both __all__ blocks is what the two packages’ own tests import by that path.

VectorEncoder lost its @runtime_checkable. PageReader kept it: the three extraction packages assert with it that create() returned a reader, which is the only check there is on a create() that returns Any.

CorrelationFilter is gone and JsonFormatter reads the contextvar itself, because a handler formats in the context that emitted. The ceiling is in the comment the audit asked for: the shortcut is false under QueueHandler, which formats on another thread, and LOG_CONFIG installs none.

Every item here landed too, and three were larger or stricter than described.

“Numeric but not bool” was written eleven ways rather than five, in two polarities, across the executor, the VLM reader, the hosted embedder and the three LLM reply parsers. indx_interfaces.number is the one rule now. indx-llm keeps its own copy: it declares no dependency at all, which is what lets the three lanes share it, and the boundary test pins that.

The extras-missing probe was pinned whole before it moved, because the string it builds goes on the wire as unavailable_reason. indx-language-lingua, -classifier-zeroshot and -ner-onnx were left alone: they answer with a detector or a bool rather than with a reason, so there is no format to share.

One refusal is stricter rather than preserved. economics.py is now a pydantic model with extra="forbid" and strict=True, which refuses a fractional latency_ms_per_page where the hand-rolled read truncated it silently. The api-contract page says so in both languages, and the parametrized test pins it.

The registry’s tuples became Declared, Collected and Inventory, all frozen, and the registry holds one object where it held seven attributes assigned in sequence. The lock-guarded memo stayed: cached_property offers no guarantee that concurrent first readers share one build, and test_concurrent_first_readers_share_one_build_and_one_attribution_table is what says so. The boolean sort keys got the comment they were owed, naming ADR-0027 as the reason they are not a defect.

The rest went as the audit described: content_digest for the four things that mint an ID, resolvable_schemes so the advertised set and the 415’s list agree by construction, the bound _assemble arguments, math.hypot, starlette’s Headers, the hoisted version lookup, the declared CLI defaults with one _fail, _actuals taking the embedding events instead of a field-by-field copy that dropped gpu_regions, and the one-line first-page map.

5. Known, deferred — each moves the wire contract

Section titled “5. Known, deferred — each moves the wire contract”

Changing any of these costs an openapi.json + generated-client regeneration (just api::gen-client, just api::contract-check). Recorded so nobody hunts for the missing producer; act only when the API is moving anyway.

  • REGION is never produced: no code constructs BlockKind.REGION, asks for it in granularities, or emits a region route — _routes emits document and page scopes only. ScopeKind.REGION and RegionEvidence are live: observers may emit region evidence, and nothing routes on it. Likewise ExecutionActuals.gpu_regions and RegionId have no producer. The request side is answered rather than silent, see ADR-0016.
  • The 501 / NotImplementedError mapping (server and CLI) has no in-repo producer; the one plausible source is swallowed into a failed rung by the dispatch loop. A documented promise with no producer — or dead weight, depending on how much you trust third-party planners.

Recorded so the question is not re-litigated. Four of these became decision records, because they are architectural rather than code-shaped:

  • Metaclasses / __init_subclass__ registries, and config-file-driven capability declarationsADR-0006.
  • Sharing PDF rasterization and text extraction across capability packagesADR-0007.

The rest stay here, because they are about how this code is written rather than about what the system is:

  • More tables in the policy. _ESCALATION, _DEVICE_PREFERENCE, economics.DEFAULTS and media_type._SIGNATURES are already tables. Table-driving _devices (two rules), _admit (three checks with three comparison shapes) or _ladder (two outcomes) writes more code to express less; revisit at the third rule, not before.
  • Dispatch tables for the three POST endpoints and the CLI subcommands. ~15 lines saved, paid for with grep-opacity, fussier FastAPI schema generation, and a client regen to prove nothing moved. Add when a fourth operation lands.
  • singledispatch on two-branch isinstance unions (indx_source.load, QueryEmbedder.embed): registration ceremony longer than the if.