Skip to content

[aw][test audit] vscode_parser: _BoundedFileSummaryCache has no unit tests #807

@microsasa

Description

@microsasa

Root Cause

_BoundedFileSummaryCache is a custom OrderedDict subclass that enforces LRU-eviction semantics for the per-file partial-summary cache (_PER_FILE_SUMMARY_CACHE). Its __setitem__ override implements a deliberate delete-before-reinsert pattern to move updated keys to the MRU position — a behaviour that plain OrderedDict.__setitem__ does not provide (it leaves existing keys at their current position).

There are zero direct unit tests for this class. All coverage is incidental — exercised only when get_vscode_summary happens to update or evict an entry during an integration-level test. If the class is ever simplified or refactored, the regression would be silent.


Untested branches

1. __setitem__ when the key already exists (LRU-refresh path)

def __setitem__(self, key: Path, value: _CachedFileSummary) -> None:
    if key in self:
        super().__delitem__(key)      # ← delete first to reset position
    super().__setitem__(key, value)  # ← insert at end (MRU)
    while len(self) > self._max_size:
        self.popitem(last=False)

If del is removed or the if guard is dropped, the key stays at its original (potentially LRU) position and gets evicted prematurely. No test catches this.

What to assert: after updating an existing key, it must be the last entry in the dict (MRU), not at its old position.

2. Eviction when capacity is exceeded

When a new key is inserted into a full cache, the oldest entry (LFU front) must be evicted.

What to assert: len(cache) == max_size after inserting max_size + 1 distinct keys; the first key inserted is no longer present.

3. Size-invariant after key replacement

Replacing an existing key must not grow the cache beyond max_size. Because __setitem__ deletes before inserting, size stays constant — but this is subtle and easy to break.

What to assert: inserting a key that already exists does not increment len(cache).

4. The _get_cached_vscode_requests caller-provided file_id path

The internal function accepts an optional pre-computed file_id to avoid a redundant stat() call:

resolved_id = (
    safe_file_identity(log_path) if file_id == _FILE_ID_UNSET else file_id
)

When file_id=None is passed explicitly (file was unstat-able at call time), resolved_id=None. A cached entry with file_id=None is a hit; any other value invalidates it. This None(mtime_ns, size) transition (file becomes readable between calls) is exercised through get_vscode_summary integration tests but has no dedicated unit test.

What to assert:

  • Calling with file_id=None populates the cache with file_id=None.
  • A subsequent call with file_id=(mtime_ns, size) is a cache miss and re-parses.

Regression scenarios

Scenario Symptom without tests
__setitem__ loses the del cache[key] guard Updated files stay at LRU position and are evicted before stale entries; get_vscode_summary re-parses unchanged files
while eviction becomes if No behavioural change for single inserts, but bulk-insert usage (future refactor) could silently overflow the cache
File-id Nonetuple transition File that briefly fails stat() never re-parses even after it becomes readable again; stale (or empty) parse results are served indefinitely

Suggested tests

class TestBoundedFileSummaryCache:
    def test_insert_below_capacity(self): ...
    def test_eviction_when_full(self): ...
    def test_update_existing_key_moves_to_mru(self): ...
    def test_update_existing_key_does_not_grow_cache(self): ...

class TestGetCachedVscodeRequestsExplicitFileId:
    def test_none_file_id_populates_cache_with_none(self): ...
    def test_none_to_real_file_id_invalidates_cache(self): ...
    def test_real_file_id_avoids_extra_stat_call(self): ...

Generated by Test Suite Analysis · ● 15.1M ·

Metadata

Metadata

Assignees

No one assigned

    Labels

    awCreated by agentic workflowaw-dispatchedIssue has been dispatched to implementertest-auditTest coverage gaps

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions