fix(cache): isolate Windows probes and stabilize nightly checks - #1782
Conversation
|
@codex review |
Performance BenchmarksCompared
|
There was a problem hiding this comment.
Pull request overview
This PR addresses Nightly CI failures by hardening cache identity probing on Windows to avoid creating locked probe files within scanned content, and by ensuring pickle scan results preserve suspicious findings even when shared call-graph analysis becomes inconclusive due to source instability.
Changes:
- Update Windows cache “change clock” probe selection to restrict probe locations to system temp/cache directories (and fail closed otherwise).
- Add/adjust regression tests covering Windows probe behavior and call-graph source-stability inconclusive reporting while preserving suspicious findings.
- Document the Windows probe fix in the root changelog.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
modelaudit/cache/scan_results_cache.py |
Adjusts _get_change_clock_probe() candidate directories on Windows to avoid scanned ancestors and include cache dir fallback. |
tests/cache/test_cache_correctness.py |
Splits POSIX-vs-Windows probe expectations and adds a Windows regression ensuring scanned ancestors are not probed. |
packages/modelaudit-picklescan/tests/test_api.py |
Extends framework-global unresolved tests to cover source-stability failure mode and allow INCONCLUSIVE while keeping suspicious findings. |
CHANGELOG.md |
Adds an Unreleased bug-fix entry describing the Windows probe behavior change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40cb9a420e
ℹ️ 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".
| ("module", "name", "source_changes"), | ||
| [ | ||
| ("joblib.numpy_pickle", "NumpyArrayWrapper"), | ||
| ("numpy._core.multiarray", "_reconstruct"), | ||
| ("torch._utils", "_rebuild_tensor_v2"), | ||
| ("joblib.numpy_pickle", "NumpyArrayWrapper", False), | ||
| ("joblib.numpy_pickle", "NumpyArrayWrapper", True), |
There was a problem hiding this comment.
Because this fix(cache): ... commit also touches the standalone package tree, release-please will treat it as a patch-worthy modelaudit-picklescan change and initiate a separate package version bump and publish, even though the standalone package's shipped code is unchanged. Move these test-only edits into a non-releasing test: commit so the cache fix releases only the root package.
AGENTS.md reference: packages/modelaudit-picklescan/AGENTS.md:L69-L75
Useful? React with 👍 / 👎.
|
@codex review |
| for workflow, job_name in ( | ||
| (_load_release_workflow(), "build-picklescan-package"), | ||
| (python_workflow, "picklescan-package"), | ||
| ): | ||
| for step in _job_steps(workflow, job_name): | ||
| run = step.get("run", "") | ||
| if "ruff check" in run or "ruff format" in run: | ||
| assert expected_requirement in run | ||
|
|
| with self._change_clock_probe_lock: | ||
| for probe, _directory in self._change_clock_probes.values(): | ||
| probe.close() | ||
| self._change_clock_probes.clear() |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e09366cd6
ℹ️ 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".
| - name: Lint standalone package with Ruff | ||
| run: | | ||
| uv run --with ruff ruff check src tests | ||
| uv run --with 'ruff==0.15.10' ruff check src tests |
There was a problem hiding this comment.
uv help run describes --with as layering the requested package into a separate ephemeral environment. On a fresh package checkout, however, the required standalone commands remain uv run --with ruff ..., so they can resolve a newer Ruff instead of 0.15.10—the exact condition these workflow changes avoid—while CI passes with the pin. Update the package guide and docs/agents/picklescan-package-split.md:127-134, or make those commands invoke the root-locked tool, so canonical local validation matches CI.
AGENTS.md reference: packages/modelaudit-picklescan/AGENTS.md:L39-L45
Useful? React with 👍 / 👎.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1782 +/- ##
==========================================
+ Coverage 76.28% 76.29% +0.01%
==========================================
Files 217 217
Lines 115280 115403 +123
Branches 27124 27151 +27
==========================================
+ Hits 87938 88049 +111
- Misses 19970 19979 +9
- Partials 7372 7375 +3
🚀 New features to boost your workflow:
|
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
tests/cache/test_cache_correctness.py:269
- This branch checks for
system_temp_ancestor, but the parametrization forunsafe_locationin this test only suppliessystem_temp_parentandsystem_temp_descendant. Thesystem_temp_ancestorpath is unreachable and makes the test harder to reason about.
if unsafe_location == "system_temp_parent":
system_temp = scanned_directory
elif unsafe_location == "system_temp_ancestor":
system_temp = tmp_path
else:
| if report.status == ScanStatus.INCONCLUSIVE: | ||
| _assert_call_graph_source_stability_error(report) | ||
| else: | ||
| assert report.status == ScanStatus.COMPLETE | ||
| if source_changes: | ||
| assert report.status == ScanStatus.INCONCLUSIVE |
| if unsafe_location.startswith("cache_"): | ||
| if unsafe_location == "cache_parent": | ||
| cache_directory = scanned_directory | ||
| elif unsafe_location == "cache_ancestor": | ||
| cache_directory = tmp_path | ||
| else: | ||
| cache_directory = scanned_directory / ".scan-cache" |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| try: | ||
| identity = cache.capture_file_identity(str(file_path)) | ||
| cache.release_ancestor_identity(identity[-1]) | ||
| except BaseException as exc: |
| clear_started.set() | ||
| try: | ||
| cache.clear_cache() | ||
| except BaseException as exc: |
Summary
Validation
uv lock --checkandcargo fmt --check: passed.uv.lock, standaloneuv.lock, andCargo.lockremain unchanged. Known box-local root resolver drift also reproduces on main-identical inputs; the exact-head GitHub lock check is authoritative.Release impact
This repository is squash-only. The existing stabilization touches standalone regression coverage and its canonical agent documentation, so release-please may identify a standalone
modelaudit-picklescanpatch despite no standalone implementation or version change. Splitting commits does not change squash component attribution. No release policy, dependency constraints, package version, lockfile, CODEOWNER requirement, or branch protection is modified.