Repository navigation
fix(codeql): CodeQL Python note-level cleanup (unused imports, lambdas, empty-except) - #870
Merged
Merged
Conversation
…ated Python note alerts (batch 1)
Fixes 32 py/unused-import, 10 py/unnecessary-lambda, 4 py/unused-local-variable,
1 py/repeated-import, and 1 py/import-and-import-from CodeQL alerts:
- Remove unused imports: asyncio/math/Path (test_mcp_hardening_wave1.py), tempfile
(test_result_store.py), MagicMock (test_cross_validation.py), patch (test_result.py),
patch+Result (test_quality_runner.py), np+FeatureExtractor (test_feature_assembler.py),
AsyncMock+patch module-level (test_coverage_round3.py), os+shutil (test_server.py),
subprocess (test_coverage_round2.py, test_probe_backend_pr850.py), patch
(test_mcp_p0_adr0608.py), svmutil from libsvm (quality_runner.py), repeated asyncio
(test_probe_backend_pr850.py).
- Fix py/import-and-import-from in quality_runner.py: consolidate `import vmaf` +
`from vmaf import` into a single from-import; add model_path to the import list
and replace vmaf.model_path() calls with the direct name.
- Mark plt re-export in compat/python-vmaf/__init__.py with # noqa: F401 so CodeQL
sees it is intentional; add type: ignore for the None fallback assignment.
- Fix py/unnecessary-lambda: replace `lambda p: Path(p)` with Path directly in
test_http_transport.py, test_coverage_round{2,3,4}.py, test_http_transport_round5.py.
- Fix py/unused-local-variable: remove unused fake_payload (test_mcp_p0_adr0608.py),
rename unused registry to _registry (test_coverage_round3.py), remove unused
_fake_event_wait function (test_http_transport_round5.py).
Skipped (genuine false positives / intentional patterns):
- run_testing.py imports with # noqa: F401 + side-effect comment: intentional registration.
- misc.py sleep re-export with # noqa: F401: downstream callers import sleep from here.
- test_iserror_invariant.py aiohttp/prometheus_client: optional-dep probes with noqa.
- test_smoke_e2e.py pytest_asyncio: needed for asyncio mode, already has noqa.
- train_test_model.py `== None`: numpy element-wise comparison; is None is incorrect there.
- test_tools_misc.py Child class: class definition IS the test side-effect; CodeQL FP.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s to all silent except-pass blocks All 6 open CodeQL py/empty-except alerts (alerts #183–188, #338) are addressed: - compat/python-vmaf/config.py: OSError on temp-file removal during download cleanup - mcp-server/…/http_transport.py: CancelledError + KeyboardInterrupt on event-loop shutdown - mcp-server/…/server.py (_describe_model): JSONDecodeError/OSError on malformed model JSON - mcp-server/…/server.py (progress-token extraction): LookupError/AttributeError when called outside request context - mcp-server/…/server.py (JSON-RPC parse error): bare Exception on unparseable stdin line - mcp-server/…/server.py (ladder/tune-per-shot): JSONDecodeError fallback to raw stdout - mcp-server/…/tests/test_coverage_round2.py: replace try/except/pass with contextlib.suppress per SIM105; add contextlib import, remove unused subprocess import Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…import ordering Remove two unused `# noqa: PLW0603` directives from server.py (PLW0603 was not in the ruff enabled-ruleset, making them RUF100 violations) and fix ruff I001 import-ordering in test_coverage_round3.py (formatter blank-line normalisation after local patch/AsyncMock imports were added to resolve the conflict with the batch-1 cherry-pick). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced May 31, 2026
lusoris
added a commit
that referenced
this pull request
Sep 30, 2026
…t code The Release Script Contract failed on #1639: check-issue-reference-provenance pins five historical blocks that cite lusoris/vmaf#857 and #870, four of them in integer_cambi_cuda.c (the dispatch helpers and the host download step the device-resident rewrite removed) and one in docs/metrics/cambi.md. The four code contracts go with the code they described. The CAMBI page keeps its history as an "Implementation note (before ADR-1379)" block that still names lusoris/vmaf#870, so that contract stays. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
lusoris
added a commit
that referenced
this pull request
Sep 30, 2026
…t code The Release Script Contract failed on #1639: check-issue-reference-provenance pins five historical blocks that cite lusoris/vmaf#857 and #870, four of them in integer_cambi_cuda.c (the dispatch helpers and the host download step the device-resident rewrite removed) and one in docs/metrics/cambi.md. The four code contracts go with the code they described. The CAMBI page keeps its history as an "Implementation note (before ADR-1379)" block that still names lusoris/vmaf#870, so that contract stays.
lusoris
added a commit
that referenced
this pull request
Oct 1, 2026
…t code The Release Script Contract failed on #1639: check-issue-reference-provenance pins five historical blocks that cite lusoris/vmaf#857 and #870, four of them in integer_cambi_cuda.c (the dispatch helpers and the host download step the device-resident rewrite removed) and one in docs/metrics/cambi.md. The four code contracts go with the code they described. The CAMBI page keeps its history as an "Implementation note (before ADR-1379)" block that still names lusoris/vmaf#870, so that contract stays.
lusoris
added a commit
that referenced
this pull request
Oct 1, 2026
* perf(cuda): run cambi and SpEED entirely on the device cambi_cuda, speed_chroma_cuda and speed_temporal_cuda no longer hand work back to the host inside a frame. Each frame reads back one small result block (88 bytes for cambi, 40 for SpEED) and waits once, in collect(); the twins read the planes the CUDA engine already uploaded and upload nothing of their own. cambi_cuda used to download the distorted picture, preprocess it on the host and read the image and mask back at every scale for host c-values and top-K pooling. It now runs the ADR-1357 design on CUDA: twelve kernels for preprocessing, the spatial mask, decimation, the mode filter, the column-histogram c-values and an exact 128-bit top-K sum. Scores equal the CPU's to the last bit whenever cambi.c's own double sum is exact. The twin also gains cambi.c's guard against windows above 65 x 65. The SpEED twins run the ADR-1358 chain, 25x25 eigenvalues and QR included, in nine kernels shared through speed_cuda_pipeline.c. Every rounding the CPU performs is spelled with a round-to-nearest intrinsic and the fatbin builds with --fmad=false, so the scores equal a CPU build that rounds log2f correctly and does not fuse multiply-adds. cambi.c exports the host helpers both device twins need, and speed_internal_gpu_configure() sets up the SYCL and CUDA SpEED pipelines; the SYCL twins drop their private copies. CPU output is unchanged. There is no NVIDIA device on this host. The kernels were built for sm_80 to sm_120 and checked frame by frame through a host emulation of the CUDA driver; the two state rows stay open with the commands to verify and time the port on ryzen-4090-arc. Three rows are opened for what the checks found: the lanczos4 prescale drift of both device twins, the CPU speed_temporal buffer overflow with speed_prescale above 1, and the dev image's -march=native FMA drift of the CPU SpEED scores. ADR-1379, ADR-1380, Research-1379. * fix(ci): retire the issue-provenance anchors of the removed CAMBI host code The Release Script Contract failed on #1639: check-issue-reference-provenance pins five historical blocks that cite lusoris/vmaf#857 and #870, four of them in integer_cambi_cuda.c (the dispatch helpers and the host download step the device-resident rewrite removed) and one in docs/metrics/cambi.md. The four code contracts go with the code they described. The CAMBI page keeps its history as an "Implementation note (before ADR-1379)" block that still names lusoris/vmaf#870, so that contract stays. * fix(speed): correct 48 log2 hard cases in CUDA and SYCL twins The fp32-pair log2 evaluation misrounds 48 positive finite floats whose exact log2 falls closer than 2^-45 to a rounding boundary. Both device twins now look the input up in a shared table (speed_log2_hard_cases.h) and return the correctly rounded result. The common path costs one fraction-field compare. The CUDA cambi twin also drops a score < 0.0 clamp that the CPU does not perform. Contract tests verify the table entries against quad-precision log2 and detect removal of the correction call in both twins. * fix(ci): suppress modernize-use-using in speed_gpu_common.h and update hardware evidence * fix(tidy): name the includers and cite ADR-1138 in the speed_gpu_common.h NOLINT bracket The bracket suppresses modernize-use-using for the header's typedef structs, which clang-tidy parses as C++ under core/tools/vmaf.cpp (through feature_dimensions.h and speed_internal.h) and under the SYCL SpEED translation units; C cannot spell the `using` alias it proposes. The CPU lane of the Tidy Ratchet had counted six findings there (0 -> 6). The comment now names the C and C++ includers and cites ADR-1138, which prescribes this file-scoped shape for C code parsed as C++. A CPU-lane ratchet run scoped to vmaf.cpp measures 0 findings in the header. The source ADR citation registry follows the new citation. * test(cuda): cover the speed_temporal_cuda solve launch at 1920x1080 test_cuda_speed_temporal_parity_1080p builds the temporal parity test at 1920x1080, where the luma plane has 312 SpEED blocks. The host-split twin on master launched its solve kernel with ((blocks + 7) / 8) * 32 threads per block, 1248 > 1024, so every frame at 1080p and above failed with CUDA_ERROR_INVALID_VALUE (speed_temporal_cuda.c:425 at 10f27ef). The 768x432 and 960x540 fixtures stay under 256 blocks and never reached it. On the RTX 4090 the test fails against 10f27ef with that error and passes with the ADR-1380 twin. * docs: record the RTX 4090 verification of the CUDA CAMBI and SpEED twins Measured on ryzen-4090-arc (RTX 4090, icx release build without -march=native; before = origin/master 10f27ef built the same way): - every per-frame cambi, speed_chroma_u/v/uv and speed_temporal equals --backend cpu at --precision max on the Netflix 576x324 pair (48/48) and on BBB 3840x2160 (50/50); - compute-sanitizer memcheck, racecheck and synccheck report no error or hazard on the five CAMBI and SpEED parity tests; - one readback and one stream synchronisation per frame (CUPTI count); - 4K ms/frame before -> after: cambi_cuda 64.71 -> 6.01, speed_chroma_cuda 24.90 -> 6.89, speed_temporal_cuda fails -> 5.88; - speed_log2() correctly rounded on every positive finite float, on the RTX 4090 and on the Arc A380. T-CUDA-CAMBI-HOST-RESIDUAL-2026-09-29, T-CUDA-SPEED-HOST-RESIDUAL-2026-09-29 and T-CUDA-SPEED-TEMPORAL-SOLVE-LAUNCH-1080P-2026-09-30 are closed. The lanczos4 row now carries device numbers (up to 2.1e-2 on a smooth 1080p gradient), and the Arc A380 SpEED row records that its check is blocked by the xe kernel driver, which returns wrong values from SYCL scratch memory, rather than a vmafx bug. ADR-1379, ADR-1380, Research-1379, the CAMBI and SpEED pages, the CUDA backend page, the changelog fragments, the CUDA AGENTS.md and the rebase notes replace the emulation-only statements with these measurements. * fix(ci): regenerate the ADR citation map after rebasing onto #1643 * fix(speed): take the device prescale rule from speed_prescale_resamples() #1643 replaced speed_internal.c's SI_ALMOST_EQUAL with the shared speed_prescale_resamples() helper. The device geometry spelled the same rule out by hand with the removed macro; it now calls the helper, so the CPU and the device twins decide the resample from one function.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolves ~53 CodeQL Python note-level alerts across two batches:
py/unused-import,py/unnecessary-lambda,py/unused-local-variable,py/repeated-import,py/import-and-import-fromincompat/python-vmaf/__init__.py,compat/python-vmaf/core/quality_runner.py,compat/python-vmaf/tests/test_cross_validation.py,compat/python-vmaf/tests/test_feature_assembler.py, and MCP server test files.py/empty-except— adds explanatory comments to all silentexcept: passblocks incompat/python-vmaf/config.py,mcp-server/vmaf-mcp/src/vmaf_mcp/http_transport.py,mcp-server/vmaf-mcp/src/vmaf_mcp/server.py,mcp-server/vmaf-mcp/tests/test_coverage_round2.py.# noqa: PLW0603directives (RUF100) and fixes I001 import ordering introduced during conflict resolution.ADR-0108 deliverables
cd mcp-server/vmaf-mcp && python -m pytest tests/test_coverage_round2.py tests/test_coverage_round3.py -q→ 119 passed;python -m ruff check compat/ mcp-server/→ cleanState.md impact
No bugs opened or closed; no user-visible behaviour changed.
Test plan
ruff checkon all changed files: cleanpytest mcp-server/vmaf-mcp/tests/test_coverage_round2.py test_coverage_round3.py— 119 passedgit grep '<<<<<<'— clean)🤖 Generated with Claude Code