Repository navigation
Python: Enforce the Purview cache capacity for oversized values - #9607
emme1t (emme1t) wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The focused guard matches the intended behavior, regression tests cover the affected paths, and no blocking issues remain.
No open issues 🎉
What changed in this PR
This PR enforces Purview’s existing cache capacity limit for individual oversized values, complementing the separate replacement-accounting work in #8988 / #8990.
Changes:
- Reject oversized values before eviction, preserving other cached entries.
- Invalidate the previous value when an oversized replacement is rejected.
- Add regression tests for new entries, replacements, and values exactly matching capacity.
| File | Description |
|---|---|
python/packages/purview/tests/purview/test_cache.py |
Adds three regression tests covering oversized writes and the capacity boundary. |
python/packages/purview/agent_framework_purview/_cache.py |
Adds an early capacity check that removes a superseded value and skips insertion. |
Review effort: Balanced
Bicheng (Kenneth) (BichengWang)
left a comment
There was a problem hiding this comment.
Reviewed f2c3f4fa25f738cd775e7cfafb546d0d252740ed. No blocking correctness findings in this diff.
Offline on macOS / CPython 3.13.16, all 203 Purview unit tests passed. The two oversized-write regressions fail against base 07a83a7c369b7d51247630a60e48bebf85429943, while the exact-capacity test passes there. Ruff 0.17.1 lint and formatting checks passed for both changed files.
I also exercised an oversized replacement followed by reusing the same key and forcing eviction. The stale expiry record did not evict the reinserted value, and the byte counter matched the remaining entries. Rejecting a new oversized key preserved both live entries and the expiry heap; a subsequent exact-capacity write succeeded. Using remove() here correctly invalidates the superseded value and adjusts its size once.
Live-service integration and the complete repository suite were not run.
Motivation & Context
A Purview cache with
max_size_bytes=20accepts a JSON-estimated 22-byte value after evicting its existing entries. The oversized value then remains cached above the configured capacity. This also occurs when replacing an existing key.Description & Review Guide
The two oversized-entry regressions fail before the fix. All 203 Purview unit tests pass on Windows/Python 3.13.13, with 89% package coverage and 100% coverage of the cache module. Package Ruff, changed-file formatting, strict source Pyright, and all five test type checkers report no errors; Pyrefly additionally reports three warning-level diagnostics in unchanged middleware tests. The source distribution and wheel build successfully. Live-service integration tests and the complete repository suite were outside this validation scope.
CI note: The latest public API compatibility job fails while Griffe 2.4.0 resolves
agent_framework._workflows._edge.EdgeGroup.builtin_typetobuiltins.typein the base core package. Comparing that unchanged core package with itself locally on Python 3.13 reproduces the sameAliasResolutionError; the core directory matches the PR base exactly.Related Issue
N/A - trivial change. This four-line guard enforces the existing documented maximum cache size. The desired capacity bound follows directly from
max_size_bytes.Related cache accounting work: #8988 / #8990. Those address duplicate size subtraction when an existing entry grows within the total capacity. This change covers a single value exceeding the entire cache capacity.
AI Assistance
AI assistance details: Astra辅助 — reproduction, implementation, regression tests, local validation, and this description. The coding agent executed the reported checks. This draft is prepared for the human contributor's review.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.