Repository navigation
refactor: simplify acquisition, caching, and progress - #1873
Conversation
…/codex/modelaudit-03-raw-evidence
…o/codex/modelaudit-04-picklescan
|
Codex Review: Didn't find any major issues. Can't wait for the next one! 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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review Please review the current head after integration with main |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It refactors multiple security-sensitive remote acquisition and cache boundaries while live remote and cross-platform behavior remain locally unverified.
Review effort: Balanced
Findings: None
Resolved since last review (1)
|
Codex Review: Didn't find any major issues. Swish! 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". |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ 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". |
Acquisition adapters, cache bookkeeping, progress reporting, and SBOM generation repeated shared decisions and conversions. Consolidate those implementations while preserving authentication and redirect checks, path containment, download budgets, cache identity and race handling, callback order, and exported results. SBOM conversion uses the existing result model, size formatting is shared, and cache operations avoid redundant intermediate state.
This is PR 6 of the six-PR simplification stack, following merged PR 5. It includes the original meaningful
force_update()callback assertion alongside the consolidated callback-order coverage.Validation:
Current-main integration (
2a5185a): preserves the upstream SafeTensors FDICT fix and deterministic initial/retried cache interruption fixture. Focused cache/routing checks and five neighboring compression cases pass on this branch; real benign/malicious CLI scans preserve scanner selection, JSON findings, SHA-256 metadata, and exit codes 0/1. Ruff checks pass. Changed-file mypy passes; full local mypy retains the previously documented optional TensorFlow baseline error. Hosted CI validates the published head.The integrated committed prefix passed 1,964 affected tests with three Windows-only and two opt-in integration skips, including fresh-process imports and public reporting paths. Ruff passed; mypy checked 498 files with only the known optional TensorFlow diagnostic.
On the prior committed PR6 prefix based on PR5, four completed local cohorts passed 2,023 tests covering cache/progress, SARIF/SBOM and acquisition, Hugging Face, cloud storage, Hub and streaming. Nine environment-dependent cases were skipped and two live/slow cases deselected. Import checks confirmed the actual checkout and expected native extension throughout.
Twenty-eight actual CLI executions produced fourteen matching JSON/SARIF, SBOM and operational-error pairs, covering exits 0, 1 and 2. Comparisons preserve findings, errors, order, evidence and identifiers, with only run metadata, checkout locations and generated temporary path components normalized.
Ruff lint/format passed across both packages and tests. Full mypy checked 496 files and reports only the existing unreachable optional TensorFlow import in
test_weight_distribution_scanner.py:2122. Earlier focused prototype checks include local DVC, redirect containment, license integration and cache isolation; an interrupted aggregate runner is not counted as a successful process.Measured reduction: 790 maintained lines — production −690; tests −100. Generated code is unchanged. Live remote acquisition and cross-platform execution remain unverified locally.
Latest integration: merged main
5fd039fafter #1872 landed. The original 27-path incremental patch and blob/mode changes are preserved exactly. The branch gains only two upstream dependency-test guards and one validated cache-fixture isolation line. Squash-ancestry conflicts and an incorrect automatic fixture merge were resolved against an independently reconstructed full tree. All 594 focused dependency/cache/progress/acquisition tests, two real benign/malicious CLI scans, full Ruff lint/format checks, and focused mypy passed. Full local mypy retains the known optional-TensorFlow baseline error. Hosted CI will validatec943c5b.PRs 1: tooling, 2: tests, 3: raw evidence, 4: picklescan, and 5: scanning/results have landed. This final 6: acquisition/cache/progress PR now targets
main.