fix(stats): invalidate bank stats cache on unit/document deletes and observation clears#2337
Conversation
…observation clears delete_bank invalidates the 60s-TTL BankStatsCache after mutating counts, but delete_memory_unit, delete_document, clear_observations, and update_document (on tag-change observation deletion) did not, so get_bank_stats served pre-mutation counts for up to a minute. Follow-up to vectorize-io#2315 which hardened the cache primitive but left the mutation call sites untouched. Invalidation is best-effort (guarded), matching the other post-commit side-effects in these methods. Adds tests/test_bank_stats_cache_invalidation.py covering the deletion paths with a pinned long TTL so the regression is deterministic.
|
Quick CI note: The formatter diff is tiny:
I reproduced the formatter part locally. After applying that ruff output, these pass: cd hindsight-api-slim
uv run ruff check hindsight_api/engine/memory_engine.py tests/test_bank_stats_cache_invalidation.py
uv run ruff format --check hindsight_api/engine/memory_engine.py tests/test_bank_stats_cache_invalidation.pySo committing the ruff-format output should clear this particular CI failure. I tried the focused pytest too, but my local run fails during |
The verify-generated-files CI job was red because `ruff format` reformats two lines that were committed unformatted: - wrap the long `logger.warning(...)` call in memory_engine.py - collapse the `test_delete_document_invalidates_stats_cache` signature No logic change; this is purely the `uv run ruff format` output. Thanks to @koriyoshi2041 for the precise diagnosis.
|
Thanks @koriyoshi2041 — spot-on diagnosis. Pushed the |
|
Thanks for the precise diagnosis and the local repro, @koriyoshi2041 — you're right that it was purely |
2 participants
Summary
get_bank_statsis served from a 60s-TTLBankStatsCache.delete_bankinvalidates it after mutating counts (with a comment to that effect), but the other operations that change the same counts did not — so a client polling stats right after a deletion saw pre-mutation node/link/document/observation counts for up to a minute.Follow-up to #2315, which hardened the cache primitive (
bank_stats_cache.py) but did not touch thememory_engine.pymutation call sites. This wires the samedelete_bank-style invalidation into every path that changes the countsget_bank_statsreports:delete_memory_unit— on an actual delete (cascades to links/entities)delete_document— on an actual delete (cascades to memory units/links)clear_observations— bulk observation delete + consolidation-timestamp resetupdate_document— when a tag change deletes observations (invalidated_obs > 0)High-frequency inserts intentionally keep tolerating TTL staleness (only
delete_bankinvalidated before); this matches that intent by invalidating on the explicit user-initiated delete/clear paths only. Invalidation is best-effort (guardedtry/except+ warning, like the other post-commit side-effects in these methods), so a cache-eviction failure can never fail an already-committed operation.delete_bankis intentionally left untouched.Tests
tests/test_bank_stats_cache_invalidation.pypins a long cache TTL, warmsget_bank_stats, mutates, and asserts the next read reflects the change immediately — without the fix the long-TTL cache would still serve the stale value. Coversdelete_memory_unit,delete_document, andclear_observations.If this has already been addressed via a separate patch, please feel free to close.