Skip to content

fix(cache): preserve Windows source fingerprint cache hits - #1793

Open
mldangelo-oai wants to merge 1 commit into
mainfrom
fix/windows-cache-source-fingerprints
Open

fix(cache): preserve Windows source fingerprint cache hits#1793
mldangelo-oai wants to merge 1 commit into
mainfrom
fix/windows-cache-source-fingerprints

Conversation

@mldangelo-oai

Copy link
Copy Markdown
Contributor

Summary

  • reconcile descriptor and path stat identities using the same Windows-safe contract already used by standalone picklescan
  • preserve reusable source and read fingerprints when Windows reports different permission bits or creation times for the same file
  • retain inode/device/file-type/size/mtime replacement checks on Windows and change-time protection on POSIX
  • additionally verify that read-fingerprint paths do not change between their initial and final path stats

Regression proof

  • both new Windows cross-view fingerprint cases fail on current main with ValueError: ... changed while being read
  • after the fix, the simulated Windows source/read fingerprints succeed while replaced files and POSIX ctime changes still fail closed
  • exact-source parallel cache suite: 205 passed, 7 platform-specific skips
  • exact-source parallel Joblib scanner regressions: 91 passed
  • repository-wide Ruff lint and formatting checks pass
  • changed test passes mypy; changed cache module passes after excluding two pre-existing union-attr errors already present on main
  • full fast suite reached 6,123 passing tests before an unrelated existing path-sensitive Jinja YAML routing failure; that test explicitly disables caching, so it cannot be caused by this cache change
  • full local mypy reproduces 14 existing errors across six baseline files; current main's GitHub Type Check is green

CI investigation: the latest scheduled Nightly run fails on Windows and exposes cache reuse instability: https://github.com/promptfoo/modelaudit/actions/runs/30515363374

Copilot AI review requested due to automatic review settings July 30, 2026 22:22
@mldangelo-oai

Copy link
Copy Markdown
Contributor Author

@codex review

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.

Pull request overview

This PR fixes cache fingerprint stability on Windows by reconciling file “identity” comparisons between descriptor stats (os.fstat) and path stats (Path.stat) using a Windows-safe contract already proven in the standalone modelaudit-picklescan implementation. It reduces false cache invalidations caused by Windows-only differences in reported permission bits and st_ctime, while still failing closed on real replacements or metadata changes that indicate risk.

Changes:

  • Introduces a cross-view stat identity helper that ignores Windows-only stat discrepancies (but keeps POSIX ctime protections).
  • Updates source/read fingerprint validation to use the cross-view identity and additionally verifies that path-view stats are stable across the read-fingerprint operation.
  • Adds regression tests simulating Windows cross-view stat differences and verifies rejection behavior for file replacement and POSIX ctime changes; updates CHANGELOG.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
tests/cache/test_cache_correctness.py Adds regression tests covering Windows cross-view stat differences and ensuring replacements / POSIX ctime changes still invalidate fingerprints.
modelaudit/cache/scan_results_cache.py Implements _cross_view_file_stat_identity() and applies it to source/read fingerprint correctness checks to preserve Windows cache hits safely.
CHANGELOG.md Documents the Windows source-fingerprint cache-hit preservation bug fix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: 06fcfcec39

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@github-actions

Copy link
Copy Markdown
Contributor

Workflow run and artifacts

Performance Benchmarks

Compared 13 shared benchmarks with a regression threshold of 15%.
Status: 0 regressions, 0 improved, 13 stable, 0 new, 0 missing.
Aggregate shared-benchmark median: 4.248s -> 4.219s (-0.7%).

Workload Benchmark Target Size Files Baseline Current Change Status
direct-malicious-upload tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_direct_malicious_upload malicious_reduce 52 B 1 187.9us 197.2us +5.0% stable
padded-multi-stream-upload tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_padded_multi_stream_upload multi_stream_padded 4.1 KiB 1 302.6us 296.4us -2.0% stable
rejected-basic-auth-candidates tests/benchmarks/test_scan_benchmarks.py::test_rejected_basic_auth_candidates_scan_linearly - 371.1 KiB 1 2.429s 2.388s -1.7% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_hex] nested_hex 130 B 1 263.4us 259.1us -1.6% stable
clean-training-checkpoint tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_clean_training_checkpoint safe_large 278.2 KiB 1 113.14ms 114.90ms +1.6% stable
duplicate-heavy-registry tests/benchmarks/test_scan_benchmarks.py::test_scan_duplicate_registry_snapshot registry-snapshot 915.2 KiB 13 575.34ms 580.77ms +0.9% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_base64] nested_base64 98 B 1 257.7us 255.4us -0.9% stable
suspicious-pickle-intake tests/benchmarks/test_scan_benchmarks.py::test_scan_suspicious_pickle_intake suspicious-intake 183.8 KiB 4 147.01ms 148.23ms +0.8% stable
chunked-upload-stream tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_chunked_upload_stream chunked_stream 278.2 KiB 1 116.51ms 117.27ms +0.7% stable
single-checkpoint-preflight tests/benchmarks/test_scan_benchmarks.py::test_scan_single_checkpoint_before_load single_checkpoint.pkl 183.0 KiB 1 105.86ms 106.53ms +0.6% stable
warm-cache-rescan tests/benchmarks/test_scan_benchmarks.py::test_scan_warm_cached_repository_rescan release-candidate 547.3 KiB 32 140.53ms 139.75ms -0.6% stable
mixed-model-repository tests/benchmarks/test_scan_benchmarks.py::test_scan_release_candidate_repository release-candidate 547.3 KiB 32 619.59ms 621.41ms +0.3% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_raw] nested_raw 78 B 1 238.5us 239.2us +0.3% stable

@chatgpt-codex-connector

Copy link
Copy Markdown

Security review completed. No security issues were found in this pull request.

Reviewed commit: 06fcfcec39

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants