gabriel / muse public
cache-reorganization-plan.md markdown
571 lines 21.6 KB
Raw
sha256:51ce277f663e01a43eaffbe77509b1de7ac2d4251b55d23306304bcdeb92c90d feat(pack): delta-encode snapshots in MPackBundle wire format Sonnet 4.6 minor ⚠ breaking 121 days ago

Cache Reorganization Plan

Move all msgpack cache files from .muse/*.msgpack into .muse/cache/ and establish shared write utilities to eliminate duplicated idioms.

Motivation

The .muse/ root holds six cache files alongside authoritative VCS state (HEAD, refs, commits, config). Caches are recomputable and should not live next to irreplaceable data. Grouping them under .muse/cache/ makes the directory readable at a glance and enables future features (per-cache size limits, eviction policies, GC targeting).


Cache inventory

New path Module Size (typical) Write pattern
cache/test_history.msgpack muse/core/test_history.py 7 KB Pattern C — mktemp + replace
cache/stat.msgpack muse/core/stat_cache.py 150 KB Pattern A — mkstemp + fsync + replace
cache/symbols.msgpack muse/core/symbol_cache.py 35 MB Pattern A — mkstemp + replace ✅
cache/callgraph.msgpack muse/core/callgraph_cache.py 11 MB Pattern A — mkstemp + replace ✅
cache/implicit_edges.msgpack muse/core/implicit_edge_cache.py 154 KB Pattern A — mkstemp + replace ✅
cache/invariants.msgpack muse/plugins/code/_invariants.py 19 MB Pattern A — mkstemp + replace ✅

All caches are recomputable — data loss on upgrade or corruption is safe. Old files at the pre-migration locations become orphans on first use after upgrade and can be removed by muse gc or manually.


Write pattern taxonomy

Pattern A — mkstemp + replace (all four Pattern-B caches since Phase 5, plus StatCache) Safe for concurrent writers. mkstemp gives each writer a unique temp path so two parallel saves cannot collide. The last os.replace() wins atomically; the result is always a valid complete file. StatCache additionally calls fsync before rename for durability on power loss.

Pattern B — fixed .tmp suffix — ELIMINATED in Phase 5. Was used by SymbolCache, CallGraphCache, ImplicitEdgeCache, _InvariantFileCache. Two writers produced the same .tmp name; one silently overwrote the other mid-write. All four caches now use Pattern A.

Pattern C — mktemp + replace (TestHistory) Unique temp path (no collision), no fsync. Acceptable for append-only history where a torn write is detected on next load.


mkstemp prefixes registered in _MUSE_TEMP_PREFIXES (repo.py)

Orphaned temp files are swept on startup by _cleanup_muse_dir_temps. Every mkstemp prefix used anywhere in the codebase must be listed here:

_MUSE_TEMP_PREFIXES: tuple[str, ...] = (
    ".muse-tmp-",       # general / legacy
    ".stat_cache_",     # StatCache (Pattern A)
    ".symbols_",        # SymbolCache (Phase 5)
    ".callgraph_",      # CallGraphCache (Phase 5)
    ".implicit_edges_", # ImplicitEdgeCache (Phase 5)
    ".invariants_",     # _InvariantFileCache (Phase 5)
)

Phase 6 will add the base-class prefix or leave per-cache prefixes as-is (subclasses set _TEMP_PREFIX as a class variable).


Seven testing tiers

Every cache module must have tests in all seven tiers. The tier definitions below apply uniformly across Phases 1–6.

Guiding principle: these tiers are a confidence framework, not a checkbox exercise. Tiers 3 (E2E) and 4 (Stress) are frequently theater for simple dict-backed caches — skip them if the genuine coverage already exists elsewhere. Tier 7 (Security) is the genuine gap most often missed.

Tier 1 — Unit

Isolated, no filesystem I/O. Mocked or in-memory only.

Covers:

  • get() miss → None
  • put() → dirty flag set
  • get() hit after put()
  • prune() removes stale keys, sets dirty; no-op when all live
  • size property
  • empty()_cache_dir is None, no entries
  • save() on empty() is a no-op (no file created anywhere)

Tier 2 — Integration

Real filesystem via tmp_path. No subprocess, no CLI.

Covers:

  • load() on a missing file → empty cache
  • save() creates the file at the correct path under .muse/cache/
  • save() + load() round-trip: data identical after reload
  • save() is a no-op when not dirty; file mtime unchanged on second save
  • _dirty = False after a successful save
  • Fixture helper creates .muse/cache/ subdir before each test

Tier 3 — End-to-end

Full CLI invocation against a real repository built in tmp_path.

Covers:

  • Cold run populates the cache file on disk
  • Warm run reads from cache (observable via I/O patching or timing)
  • CLI output is identical between cold and warm runs
  • Cache file survives a process restart (saved and reloaded correctly)
  • muse gc removes orphaned old-location files (.muse/*.msgpack)

Skip when: Tier 2 integration tests cover load/save fully and no CLI-level behavior is unique to the cache module.

Tier 4 — Stress

Large inputs, high iteration counts. Not a performance gate — correctness under load.

Covers:

  • 500+ distinct entries written and read back without corruption
  • prune() on a 500-entry cache with 250 live keys: exact survivors
  • 1 000 put() calls with repeated keys: size == unique key count
  • Sequential save → load → save → load cycle: entries stable across 10 rounds

Skip when: the cache is a plain dict[str, ...] with no compaction or eviction logic. Stress only adds value when there is structural behavior to break under load.

Tier 5 — Data integrity

Adversarial on-disk state: corruption, version mismatch, partial writes, cross-version upgrades.

Covers:

  • Corrupt msgpack bytes → load() returns empty, never raises
  • Wrong version field → load() returns empty
  • Missing entries key → load() returns empty
  • Invalid individual entry skipped; valid entries in same file survive
  • Non-string key in entries dict → entry skipped, rest survive
  • File truncated mid-write (simulated) → load() returns empty
  • Atomic write: no .tmp file left behind after a successful save()
  • Old-location orphan (cache_file.msgpack in .muse/) is ignored, not loaded

Note on integer keys: read_msgpack_file uses strict_map_key=True which raises at the whole-file level (not per-entry) on non-string map keys. Tests for "invalid entry skipped" must use wrong-type values, not integer keys.

Tier 6 — Performance

Timing benchmarks. Assert speedup ratios, not absolute wall times, so CI hardware variance doesn't cause flakes.

Covers:

  • Warm cache ≥ 5× faster than cold for 30-file repo (same assertion style as existing callgraph and implicit-edge tests)
  • Second save() call (not dirty) completes in < 1 ms (no I/O)
  • load() of a 500-entry cache completes in < 200 ms

Prefer mock-based assertions (assert a call count is 0 on the warm path) over timing when the speedup is observable without wall-clock comparison.

Tier 7 — Security

Untrusted cache content and path-traversal attempts.

Covers:

  • Relative .. path in a content-hash key → entry ignored on load
  • Symlink at the cache file path → save() does not follow it (writes to temp then replaces, so symlink is overwritten not followed)
  • Cache file owned by another user (mode 000) → load() returns empty, never raises; save() fails gracefully with a log warning, not a crash
  • Msgpack payload with deeply nested dicts (bomb) → load() does not crash within a bounded time (no unbounded recursion; may return empty or partial)
  • Field value of wrong type (e.g. int where str expected) → entry rejected, does not propagate into cache

Docstring standard

Every cache class and every public/private method that touches persistence must carry a docstring. Minimum structure:

class FooCache:
    """One-line summary.

    Longer description covering the cache key, value type, storage location,
    write pattern (Pattern A/B/C), and lifecycle.

    Attributes
    ----------
    _cache_dir : pathlib.Path | None
        Absolute path to ``.muse/cache/``.  ``None`` for in-memory-only
        instances (``empty()``).  ``save()`` is a no-op when ``None``.
    _dirty : bool
        Set to ``True`` by ``put()`` and ``prune()`` when entries change.
        Reset to ``False`` by a successful ``save()``.
    """

    @classmethod
    def load(cls, muse_dir: pathlib.Path) -> "FooCache":
        """Load from ``muse_dir/cache/<filename>``.

        Returns an empty cache on any failure — missing file, corrupt bytes,
        version mismatch — and never raises.  Invalid individual entries are
        skipped so a partially corrupt file does not poison the whole cache.

        Parameters
        ----------
        muse_dir:
            Path to the ``.muse/`` directory of the repository root.
        """

    def save(self) -> None:
        """Atomically persist the cache to disk if it has changed.

        Uses a temp-file-then-rename pattern so a crash mid-write never
        leaves a corrupt cache file.  Silently skips when ``_dirty`` is
        ``False`` or ``_cache_dir`` is ``None``.
        """

Phases

Phase 1 — Infrastructure + test_history ✅

  • Added cache_dir(root) helper to paths.py.muse/cache/
  • Updated test_history_path()cache_dir(root) / "test_history.msgpack"
  • Added "cache" to _INIT_SUBDIRS (init.py) and _MUSE_SWEEP_DIRS (repo.py)

Remaining test gaps (not yet backfilled):

  • Tier 3: muse gc removes old .muse/test_history_cache.msgpack orphan
  • Tier 5: corrupt test_history.msgpack → graceful empty load
  • Tier 7: path-traversal key in history file → entry rejected

Phase 2 — stat_cache ✅

  • stat_cache_path(root)cache_dir(root) / "stat.msgpack"
  • StatCache stores _cache_dir; load() and save() use it
  • Temp files at .muse/cache/.stat_cache_*.tmp (Pattern A, already correct)
  • All tiers covered by test_core_stat_cache.py and test_query_stat_cache.py

Remaining test gaps (not yet backfilled):

  • Tier 3 E2E: muse code symbols cold → warm CLI timing gap
  • Tier 7: symlink at cache/stat.msgpacksave() overwrites symlink, not target

Phase 3 — symbol_cache, callgraph_cache, implicit_edge_cache ✅

  • symbols.msgpack, callgraph.msgpack, implicit_edges.msgpack in cache/
  • All three cache classes store _cache_dir
  • Fixture helpers updated to create .muse/cache/ subdir; 163 tests pass
  • Path assertions updated throughout all three test files

Remaining test gaps (not yet backfilled):

  • Tier 7 security: .. in content-hash key rejected on load; mode-000 file → graceful load failure

Phase 4 — invariants cache ✅

TDD order: wrote all seven test tiers first, then made them pass.

File: tests/test_invariant_file_cache.py

Production changes delivered:

  • Added invariants_cache_path(root) to paths.pycache_dir(root) / "invariants.msgpack"
  • _FILE_CACHE_NAME = "invariants.msgpack" (was "code_invariants_cache.msgpack")
  • _InvariantFileCache.__init__: param muse_dircache_dir; stores self._cache_dir
  • load(repo_root): derives cache_dir = muse_dir / "cache"; returns cls(cache_dir, ...)
  • Added size property and prune() method (both were missing)
  • save(): uses self._cache_dir; calls self._cache_dir.mkdir(parents=True, exist_ok=True)
  • Full docstrings on class, load(), save(), get(), put(), prune(), empty()

Tests written (29 passing):

Tier 1 — Unit (11 tests): TestUnit — get/put/hit/dirty/size/prune/empty/save-noop

Tier 2 — Integration (8 tests): TestIntegration — missing file → empty; save creates cache/invariants.msgpack; round-trip all _FileData fields; dirty lifecycle; mtime unchanged on second save; .muse/cache/ fixture helper

Tier 3 — skipped: Theater for a dict-backed cache. Warm-path behavior covered by the Tier 6 mock assertion below.

Tier 4 — skipped: Theater. No structural behavior to break under load.

Tier 5 — Data integrity (8 tests): TestDataIntegrity — corrupt bytes → empty; wrong version → empty; missing entries → empty; non-dict value skipped, valid entry survives; no .tmp leftover after successful save; old-location file ignored; orphaned sweep test

Tier 6 — Performance (1 test): warm muse code invariants run → ast.parse call count == 0 (mock assertion, not wall-clock timing)

Tier 7 — Security (2 tests): TestSecurity — mode-000 cache file → load() returns empty, never raises; deeply nested msgpack payload → no crash (assert isinstance result)

Key implementation note: read_msgpack_file uses strict_map_key=True which raises at the whole-file level on non-string map keys (not per-entry). Tests for "invalid entry skipped" must use wrong-type values (e.g. "bad_hash": ["not", "a", "dict"]), not integer keys.


Phase 5 — Fix concurrent write race (Pattern B → A) ✅

TDD order: wrote orphaned-sweep tests red, updated _MUSE_TEMP_PREFIXES green, then migrated all four save() methods to mkstemp.

Changes delivered:

  • muse/core/symbol_cache.py: save()mkstemp(prefix=".symbols_", suffix=".tmp")
  • muse/core/callgraph_cache.py: save()mkstemp(prefix=".callgraph_", suffix=".tmp")
  • muse/core/implicit_edge_cache.py: save()mkstemp(prefix=".implicit_edges_", suffix=".tmp")
  • muse/plugins/code/_invariants.py: save()mkstemp(prefix=".invariants_", suffix=".tmp")
  • muse/core/repo.py: _MUSE_TEMP_PREFIXES extended with all four new prefixes

mkstemp pattern used uniformly in all four caches:

fd, tmp_path = tempfile.mkstemp(
    dir=self._cache_dir, prefix=".<name>_", suffix=".tmp"
)
try:
    with os.fdopen(fd, "wb") as fh:
        fh.write(payload)
except Exception:
    try:
        os.unlink(tmp_path)
    except OSError:
        pass
    raise
try:
    os.replace(tmp_path, cache_file)
    self._dirty = False
    logger.debug("✅ <name>_cache saved (%d entries)", len(self._entries))
except OSError as exc:
    logger.warning("⚠️ <name>_cache save failed: %s", exc)

Tests added:

  • Orphaned-sweep tests in test_core_symbol_cache.py, test_callgraph_cache.py, test_implicit_edge_cache.py — each verifies that a leftover .tmp file in .muse/cache/ is removed by the startup sweep
  • Atomic write assertion updated from fixed .tmp name check to glob("*.tmp")

Final test counts after Phase 5: 188 cache tests + 84 regression tests, all passing.


Phase 6 — Shared MsgpackCache base class ✅

Extract the common load() / _dirty / save() pattern into muse/core/cache_base.py.

TDD order: write tests red first, then implement MsgpackCache, then update the four Pattern-A caches to inherit from it.

MsgpackCache ABC interface

class MsgpackCache(ABC):
    """Abstract base for all Pattern-A msgpack caches.

    Subclasses set three class variables and implement two abstract methods;
    the shared ``load()`` and ``save()`` handle all disk I/O.

    Class variables
    ---------------
    _CACHE_FILENAME : str
        Filename under ``.muse/cache/``, e.g. ``"symbols.msgpack"``.
    _CACHE_VERSION : int
        Integer version written to every file; mismatches discard the cache.
    _TEMP_PREFIX : str
        Prefix for mkstemp temp files, e.g. ``".symbols_"``.
    """

    _CACHE_FILENAME: str
    _CACHE_VERSION: int = 1
    _TEMP_PREFIX: str

    def __init__(
        self,
        cache_dir: pathlib.Path | None,
        entries: Any,
    ) -> None: ...

    # ---------- abstract ----------

    @classmethod
    @abstractmethod
    def _deserialize_entries(cls, raw: dict) -> Any:
        """Validate raw msgpack entries dict; return typed entries.

        Called by ``load()`` with the ``"entries"`` value from the file.
        Invalid entries should be skipped — return only what is valid.
        """

    @abstractmethod
    def _serialize_entries(self) -> dict:
        """Convert ``self._entries`` to a msgpack-compatible dict.

        Called by ``save()`` to build the ``"entries"`` value written to disk.
        """

    # ---------- shared concrete ----------

    @classmethod
    def load(cls, muse_dir: pathlib.Path) -> Self:
        """Load from ``muse_dir / "cache" / cls._CACHE_FILENAME``.

        Returns an empty instance on any failure — missing file, corrupt
        bytes, version mismatch — and never raises.
        """

    @classmethod
    def empty(cls) -> Self:
        """Return a no-op instance for contexts without a ``.muse`` directory."""

    @classmethod
    def from_root(cls, root: pathlib.Path) -> Self:
        """Load for a repository root; return ``empty()`` when ``.muse/`` is absent.

        The canonical entry point for CLI callers — replaces the repeated
        ``muse_dir.is_dir()`` guard that every convenience loader previously
        duplicated:

        .. code-block:: python

            # before
            _dir = muse_dir(root)
            if _dir.is_dir():
                cache = MyCache.load(_dir)
            else:
                cache = MyCache.empty()

            # after
            cache = MyCache.from_root(root)

        ``_InvariantFileCache`` overrides this to delegate to ``load(root)``
        since its ``load()`` already accepts a ``repo_root`` and performs the
        ``is_dir()`` check internally.
        """

    def get(self, key: str) -> Any | None: ...
    def put(self, key: str, value: Any) -> None: ...
    def prune(self, live_ids: set[str]) -> None: ...

    @property
    def size(self) -> int: ...

    def save(self) -> None:
        """Atomically persist if dirty.

        Uses mkstemp (``cls._TEMP_PREFIX``) + ``os.replace`` so a crash
        mid-write never corrupts the file and two concurrent writers never
        interleave bytes.
        """

Subclass changes

All four Pattern-A caches (SymbolCache, CallGraphCache, ImplicitEdgeCache, _InvariantFileCache) lose their duplicated load(), save(), empty(), get(), put(), prune(), and size implementations and instead:

  1. Inherit from MsgpackCache
  2. Set _CACHE_FILENAME, _CACHE_VERSION, _TEMP_PREFIX as class variables
  3. Implement _deserialize_entries(cls, raw) — the existing per-cache validation logic
  4. Implement _serialize_entries(self) — the existing per-cache serialization logic

StatCache is not migrated — it keeps its custom fsync, dimension-hash logic, and Pattern A write path.

Test file: tests/test_cache_base.py

Define a minimal concrete subclass inside the test file:

class _TestCache(MsgpackCache):
    _CACHE_FILENAME = "test.msgpack"
    _CACHE_VERSION = 1
    _TEMP_PREFIX = ".test_cache_"

    @classmethod
    def _deserialize_entries(cls, raw: dict) -> dict[str, str]:
        return {k: v for k, v in raw.items()
                if isinstance(k, str) and isinstance(v, str)}

    def _serialize_entries(self) -> dict:
        return dict(self._entries)

Tier 1 — Unit (via _TestCache)

  • get() miss → None
  • put() → dirty flag set
  • get() hit after put()
  • prune() removes stale keys, sets dirty; no-op when all live
  • size property
  • empty()_cache_dir is None, no entries
  • save() on empty() → no file created

Tier 2 — Integration

  • load() on missing file → empty cache
  • save() creates file at cache/test.msgpack
  • Round-trip: entries identical after reload
  • save() no-op when not dirty; mtime unchanged on second call
  • _dirty = False after successful save
  • Fixture helper creates .muse/cache/ before each test

Tiers 3 & 4 — skipped: Theater for a base class. Per-cache tests cover real behavior.

Tier 5 — Data integrity

  • Corrupt msgpack bytes → load() returns empty, never raises
  • Wrong version field → load() returns empty
  • Missing entries key → load() returns empty
  • _deserialize_entries skips invalid entries; valid entries survive
  • No .tmp file left behind after a successful save()
  • Orphaned .tmp in .muse/cache/ swept by _MUSE_TEMP_PREFIXES registration

Tier 6 — Performance

  • Not-dirty save() completes in < 1 ms (no I/O; assert via mtime unchanged)

Tier 7 — Security

  • Mode-000 cache file → load() returns empty, never raises
  • mkstemp pattern means symlink at cache path is replaced, not followed

from_root integration tests (TestFromRoot in test_cache_base.py)

  • from_root(root) where .muse/ exists → _cache_dir set to muse/cache/
  • from_root(root) where .muse/ is absent → returns empty (_cache_dir is None)
  • Round-trip via from_root: put → save → from_root again → get returns same value
  • from_root on a dir without .muse/ → save is no-op (no files created)

Regression gate: After inheriting from MsgpackCache, all existing per-cache test files must continue to pass with zero modifications:

  • test_core_symbol_cache.py
  • test_callgraph_cache.py
  • test_implicit_edge_cache.py
  • test_invariant_file_cache.py

_MUSE_TEMP_PREFIXES update: Add ".test_cache_" only if _TestCache is exported or used outside tests. For a test-internal class, no update is needed since orphaned test-temp files never reach production.

from_root — wiring in callers

All four Pattern-A convenience loaders now delegate to from_root:

# symbol_cache.py, callgraph_cache.py, implicit_edge_cache.py
def load_xxx_cache(root: pathlib.Path) -> XxxCache:
    return XxxCache.from_root(root)

# _invariants.py — _InvariantFileCache overrides from_root too
@classmethod
def from_root(cls, root):
    return cls.load(root)  # load() already handles the is_dir() check

The 20+ CLI commands that call these loaders are unchanged — they still call load_symbol_cache(root) etc. and get the same behavior.


Migration for existing repos

Old cache files at .muse/*.msgpack become orphans on first run after upgrade. They are silently ignored — caches rebuild at the new location on next use. muse gc sweeps the old locations as a housekeeping step.

File History 1 commit
sha256:51ce277f663e01a43eaffbe77509b1de7ac2d4251b55d23306304bcdeb92c90d feat(pack): delta-encode snapshots in MPackBundle wire format Sonnet 4.6 minor 121 days ago