Skip to content

Python: Enforce the Purview cache capacity for oversized values - #9607

Draft
emme1t (emme1t) wants to merge 1 commit into
microsoft:mainfrom
emme1t:fix/purview-oversized-cache-entry
Draft

emme1t (emme1t) wants to merge 1 commit into
microsoft:mainfrom
emme1t:fix/purview-oversized-cache-entry

Conversation

@emme1t

@emme1t emme1t (emme1t) commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Motivation & Context

A Purview cache with max_size_bytes=20 accepts 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

  • What are the major changes? Check the estimated value size before eviction. When it exceeds the total capacity, remove any previous value for that key and finish the cache write.
  • What is the impact of these changes? Other cached entries remain available, the superseded value is invalidated, and a value exactly matching the capacity still uses normal eviction.
  • What do you want reviewers to focus on? The early capacity check and regression coverage for new entries, replacements, and the inclusive capacity boundary.

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_type to builtins.type in the base core package. Comparing that unchanged core package with itself locally on Python 3.13 reproduces the same AliasResolutionError; 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

  • No material AI assistance was used.
  • This is an AI-assisted contribution.

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

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR links to an agreed issue with no competing open PR, or the Related Issue section documents a trivial-change or repository-automation exception.
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Copilot AI balanced review requested due to automatic review settings October 22, 2026 15:51
@emme1t
emme1t (emme1t) deployed to github-app-auth October 22, 2026 15:51 — with GitHub Actions Active
@emme1t
emme1t (emme1t) deployed to github-app-auth October 22, 2026 15:51 — with GitHub Actions Active
@agent-framework-automation agent-framework-automation Bot added the python Usage: [Issues, PRs], Target: Python label Oct 22, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

This branch was successfully deployed

1 active deployment
github-app-auth — f2c3f4fa Deployed Oct 22, 2026 by emme1t via add_label #25968
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants