Skip to content

Refactor: split caches.py (954 lines) into a caches/ subpackage by concern #789

Description

@pmrv

Location

  • src/fleche/caches.py — 954 lines, five loosely coupled concerns in one file.

Rough per-concern boundaries as they stand today:

Lines Concern
23–286 Rejected, BaseCache abstract interface, _combine_shrink helper
288–458 Concrete Cache (plus its GC, redigest, and query decoding)
461–572 Single-cache wrappers: CacheWrapper, ReadOnlyMixin, ReadOnlyCache, FilteringMixin, FilteredCache, RefreshingCache
575–868 Multi-cache aggregation: _MultiCache (with _first_hit / _collect / _foreach), CacheStack, CachePool
871–954 Size-limit policy: SizeLimitedMixin, SizeLimitedCache

Problem

caches.py is the largest module in the codebase (wc -l: 954 vs. next-largest remote.py at 930, config.py at 770). It also has the highest inbound-import surface: wrapper.py, config.py, query.py, state.py, remote.py, executor.py, and every test file that touches the cache layer all import from it.

Concrete symptoms of the "too many concerns in one file" pressure:

  1. Import cycle worked around at method level. BaseCache.from_config (caches.py:34-49) does a lazy from . import config as _config because config.py imports from caches.py. The docstring calls this out explicitly. That cycle exists structurally because caches.py conflates the abstract interface (BaseCache) with concrete constructors that need config; splitting them into two files (base.py + simple.py inside a caches/ package) breaks the cycle at the module boundary, not with a per-call lazy import.
  2. _combine_shrink (base layer) lives next to Cache (concrete) even though only _MultiCache uses it. Line 275-286 is a free helper about combining shrink results across sibling caches; it's read by _MultiCache._shrink 300 lines away.
  3. Reader navigation. Finding "what does CacheStack do when a load misses?" requires paging through Cache, CacheWrapper, _MultiCache._first_hit, then CacheStack.load. Concern-separated files map to a mental model that already exists in the code.
  4. Test file naming already assumes the split. tests/unit/caches/test_cache.py, test_cache_stack.py, test_size_limited.py, test_operation_context_cache.py etc. treat these as distinct units — the tests are organised by concern; the source is not.

Suggested refactor

Turn caches.py into a package. Keep the public re-exports 100% intact so no importer needs to change:

src/fleche/caches/
    __init__.py     # public re-exports; keeps every current import path working
    base.py         # BaseCache, Rejected, _combine_shrink
    simple.py       # Cache (the concrete single-storage cache)
    wrappers.py     # CacheWrapper, ReadOnlyMixin, ReadOnlyCache,
                    #   FilteringMixin, FilteredCache, RefreshingCache
    multi.py        # _MultiCache, CacheStack, CachePool
    size.py         # SizeLimitedMixin, SizeLimitedCache

caches/__init__.py:

from .base import BaseCache, Rejected
from .simple import Cache
from .wrappers import (
    CacheWrapper, ReadOnlyMixin, ReadOnlyCache,
    FilteringMixin, FilteredCache, RefreshingCache,
)
from .multi import CacheStack, CachePool
from .size import SizeLimitedMixin, SizeLimitedCache
# backwards-compat re-exports (line 30-31 today):
from ..storage.destructuring import DigestedIterable, DigestedDict

Because every current import path (from fleche.caches import Cache, from fleche.caches import BaseCache, …) still resolves via __init__.py, no other module in the repo needs to change — the whole diff is git mv caches.py caches/__init__.py + shifting classes into siblings.

Pros

  • Aligns files with mental model. base / simple / wrappers / multi / size are how the module's classes are actually grouped in review comments, tests, and docs.
  • Kills the caches ↔ config import cycle at the module boundary. BaseCache.from_config can move to caches/__init__.py (import-safe now that config doesn't need to import all of caches) and drop the in-function lazy import.
  • Removes reader-scroll cost. Contributors landing a fix to SizeLimitedMixin don't page past 850 lines of unrelated code.
  • Sets the stage for related refactors already open. Refactor: reshape SizeLimitedMixin as a CacheWrapper so it composes with CacheStack / ReadOnlyCache #710 (make SizeLimitedMixin composable as a CacheWrapper) becomes a pure inside-size.py-and-wrappers.py change. Refactor: unify Cache and _MultiCache fan-out via a shared _sources() hook #761 (unify Cache and _MultiCache fan-out) lives naturally in base.py (the shared hook) + a slim override in simple.py and multi.py.
  • Enables per-file circular-import discipline. Once split, importers can pin exactly what they need (e.g. state.py only actually needs BaseCache — importing the package top-level pulls all subclasses transitively).
  • No behaviour change. Pure move. Full test suite is the safety net.

Cons / risks

  • Big diff, small semantic change. git blame on each moved class points to the split commit; if you rely on git blame for context, use --follow. Mitigated by keeping the split as one commit and by conventional-commit refactor: split caches.py into subpackage messaging (per agents/DEVELOPING.md).
  • __init__.py becomes the public-surface source of truth. If a downstream user imports a private name (e.g. _combine_shrink, _MultiCache), the split will break them. Currently _MultiCache is at least referenced by remote.py; verify with grep -R '_MultiCache\|_combine_shrink' src/ tests/ before landing and re-export what's still used internally. Nothing named _* is documented as public API.
  • Sphinx docs may need updated cross-references. Any :class: refs like :class:fleche.caches.Cache keep working (Python re-exports); `:mod:` refs like `:mod:`fleche.caches still resolve to the package. Fully-qualified caches.py-in-source references (if any) would need re-pointing — a grep will show them.
  • __init__.py gets ~30 lines of re-exports. Trivial but has to stay in sync when a new class is added.
  • Doesn't itself reduce code size. LOC total is the same after the split. The win is in navigation and coupling clarity, not lines removed.
  • No new external dependencies. Pure stdlib package layout.

Complexity / maintainability

Medium in touched-line-count (git mv + minor import fix-ups); low in semantic risk (behaviour-preserving). Best done as a single atomic commit with all classes moved and the __init__.py populated in one go — a partial move leaves the codebase in a weird state where both the module and the package would try to claim fleche.caches.

Worth doing before the wave of open cache-layer refactors (#761 fan-out unification, #710 SizeLimitedMixin reshape, #708 cache config registry) so those PRs land in the layered layout instead of extending the flat file further.

Out of scope


Filed from a codebase-wide refactoring scan; not yet implemented.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions