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.
3. Dead code and deletions — landed
Section titled “3. Dead code and deletions — landed”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.
4. Shrinks — landed
Section titled “4. Shrinks — landed”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.
REGIONis never produced: no code constructsBlockKind.REGION, asks for it ingranularities, or emits a region route —_routesemits document and page scopes only.ScopeKind.REGIONandRegionEvidenceare live: observers may emit region evidence, and nothing routes on it. LikewiseExecutionActuals.gpu_regionsandRegionIdhave no producer. The request side is answered rather than silent, see ADR-0016.- The 501 /
NotImplementedErrormapping (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.
6. Assessed and rejected
Section titled “6. Assessed and rejected”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 declarations → ADR-0006. - Sharing PDF rasterization and text extraction across capability packages → ADR-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.DEFAULTSandmedia_type._SIGNATURESare 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.
singledispatchon two-branchisinstanceunions (indx_source.load,QueryEmbedder.embed): registration ceremony longer than theif.