Repository navigation
fix(python): Python-surfaces bug-audit — 14 defects across ai/ + mcp-server - #506
Merged
Merged
Conversation
lusoris
marked this pull request as ready for review
May 31, 2026 21:58
…server
Deep audit of every fork-local Python file under ai/src/corpus/,
ai/src/vmaf_train/data/, and mcp-server/vmaf-mcp/src/vmaf_mcp/server.py,
post-master-rebase. The audit pattern-matched the recurring bug families
the team has been catching: subprocess hangs without timeout, locale
leaks from missing encoding=, NaN sort non-determinism, concurrent
tempdir races, pickle code-execution gaps, and silent decoder failures.
Fourteen defects survived the rebase against origin/master (master had
already discharged the bulk of the subprocess-timeout debt in server.py).
Family A — subprocess hangs (no timeout):
- probe_geometry (ai/src/corpus/base.py): new timeout_s kwarg
(default 60 s); subprocess.TimeoutExpired returns None.
- download_clip (ai/src/corpus/base.py): runner timeout=timeout_s+30s
so wedged DNS / spawn cannot stall ingest.
- _run_vmaf (ai/src/vmaf_train/data/feature_dump.py): timeout_s
kwarg (default 600 s).
Family B — locale leak (no encoding=):
Eleven file-open sites now pin encoding="utf-8":
load_manifest, load_mos_csv, write_manifest, _run_vmaf JSON read,
and the four server.py read_text sites (_run_vmaf_score,
_list_extractors, _describe_model_file, _probe_backend).
Family C — silent ffmpeg failure:
iter_frames (frame_loader.py) now pipes stderr, caps wait() at 30 s
+ kills overrun, and raises RuntimeError on rc!=0. Pre-fix a missing
source file silently produced an empty iterator that callers treated
as a healthy zero-frame clip.
Family D — pickle code-execution gap:
_load_frame (frame_dataset.py) calls np.load with allow_pickle=False,
closing the object-array pickle path that executes arbitrary Python.
Family E — NaN sort non-determinism:
_pick_worst_frames now filters non-finite VMAF scores before sorting
and tolerates non-float metric values. Python's list.sort is not a
total order over NaN, so the pre-fix ranking was non-deterministic.
Family F — concurrent-tempdir race:
_describe_worst_frames uses tempfile.mkdtemp per call instead of the
shared /tmp/vmaf-mcp-worst-<pid> directory. The pre-existing
test_describe_worst_frames_tmpdir_cleared_on_next_call test was
replaced (not weakened) with the stricter
test_describe_worst_frames_allocates_unique_tmpdir_per_call which
asserts peer-call PNGs survive into both responses.
Regression coverage: 10 new tests in
ai/tests/test_python_surfaces_bug_audit.py and 6 new tests in
mcp-server/vmaf-mcp/tests/test_python_surfaces_bug_audit.py. All pass
locally. Netflix CPU golden tests unaffected (no extractor or scoring
change).
Deliverables (ADR-0108):
- Research digest: docs/research/python-surfaces-bug-audit-2026-05-31.md
- CHANGELOG fragment: changelog.d/fixed/python-surfaces-bug-audit-2026-05-31.md
- Rebase note: docs/rebase-notes.md (opt-out — fork-local files only)
- No ADR (CLAUDE.md r8 — bug fixes do not need an ADR).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The deliverables-check parser (scripts/ci/deliverables-check.sh) requires `docs/research/NNNN-*.md` for the Research digest ticked checkbox; rename the bug-audit digest from `python-surfaces-bug-audit-2026-05-31.md` to `0983-python-surfaces-bug-audit-2026-05-31.md` (next free ADR number per scripts/adr/next-free.sh) and update the rebase-notes reference. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
lusoris
force-pushed
the
fix/python-surfaces-bug-audit-v2
branch
from
May 31, 2026 21:58
cbb5145 to
e127067
Compare
6 of 7 tasks
lusoris
added a commit
that referenced
this pull request
Jun 1, 2026
Pre-existing failures in the Tiny AI (DNN Suite + ai/ Pytests) required check (run 26726137418): 1. test_data_datasets_branches.py (3 failures): ManifestEntry._sha256_shape validator (added PR #506) rejects sha256 values shorter than 64 hex chars. Test fixtures used abbreviated stubs "deadbeef", "cafebabe", "s". Fix: replace with valid 64-char hex constants _SHA256_A / _SHA256_B / _SHA256_K. 2. test_frame_loader.py (4 failures): iter_frames now passes stderr=subprocess.PIPE to Popen (for ffmpeg diagnostic capture on non-zero exit). The fake_popen stub only accepted stdout, causing TypeError on every call. _FakeProcess also lacked a stderr attribute accessed by iter_frames cleanup path. Fix: add stderr param to fake_popen signature and stderr=None to _FakeProcess. 3. test_parquet_utils.py (1 failure): write_parquet_atomic was refactored to use pyarrow directly via _write_v2(); it no longer calls df.to_parquet(). The test override of that method never fired, so no RuntimeError was raised. Fix: inject via monkeypatch on aiutils.parquet_utils._write_v2 instead. No ADR: test-only fixes (CLAUDE.md §12 r8) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
lusoris
added a commit
that referenced
this pull request
Jun 1, 2026
Two root causes: 1. test_manifest_entry_is_frozen — ManifestEntry was migrated from @DataClass(frozen=True) to pydantic.BaseModel(ConfigDict(frozen=True)) in PR #506. Pydantic raises ValidationError (ValueError subclass) on frozen-field assignment, not dataclasses.FrozenInstanceError (AttributeError subclass). Updated assertion to pydantic.ValidationError. 2. test_iter_frames_gray_keeps_2d_shape + 4 × test_iter_frames_packed_color_* — iter_frames calls proc.wait(timeout=_FFMPEG_WAIT_TIMEOUT_S) but _FakeProcess.wait() only accepted a positional argument. Added timeout: float | None = None keyword parameter to _FakeProcess.wait(). Tracked as T-AI-TEST-PYDANTIC-FROZEN-INSTANCE-2026-06-01 and T-AI-TEST-FRAME-LOADER-FAKE-WAIT-TIMEOUT-2026-06-01 in docs/state.md. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
5 of 13 tasks
lusoris
added a commit
that referenced
this pull request
Jun 2, 2026
…ed_score_ptx, Metal scaffold (#517) * fix(test): portable setenv/unsetenv wrappers for Windows MinGW64 MinGW64's default headers do not expose setenv/unsetenv (POSIX-only). Add #ifdef _WIN32 wrappers in test_gpu_dispatch_runtime.c that delegate to _putenv_s / _putenv("") so the test builds and runs on both POSIX and Windows targets without _GNU_SOURCE or compatibility magic. Fixes: Build — Windows MinGW64 (CPU) CI failure on master tip 40d192e (run 26726137428). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(compat): add pthread_once_t + pthread_once to Win32 pthread shim MSVC has no <pthread.h> so ssim_simd.h:84, float_ssim.c, float_ms_ssim.c, and cuda/dispatch_strategy.c all failed to build (error C2143: syntax error: missing ')' before '*') when pthread_once_t appeared in the function signature. Extend core/src/compat/win32/pthread.h to define pthread_once_t as INIT_ONCE, PTHREAD_ONCE_INIT as INIT_ONCE_STATIC_INIT, and a static inline pthread_once() that delegates to InitOnceExecuteOnce. Mirrors the pattern already used in cuda/dispatch_strategy.c (ADR-0181). Fixes: Build — Windows MSVC + CUDA CI failure on master tip 40d192e (run 26726137428 / 26726137416). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(cuda): register speed/speed_score.cu in cuda_cu_sources speed_chroma_cuda.c and speed_temporal_cuda.c both declare extern const char speed_score_ptx[]; and pass it to cuModuleLoadData. The .cu file exists at core/src/feature/cuda/speed/speed_score.cu but was never added to cuda_cu_sources in core/src/meson.build, so the bin2c pipeline never generated the PTX blob array, causing a linker error: undefined reference to 'speed_score_ptx' Add the entry so the standard nvcc + bin2c pipeline generates speed_score.fatbin and the speed_score_ptx C array symbol. Fixes: Build — Ubuntu CUDA + Build — Ubuntu CUDA Static CI failures on master tip 40d192e (run 26726137428 / 26726137416). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(metal): rename integer_motion_v2_metal extractor from legacy name The extractor struct in integer_motion_v2_metal.mm had .name = "motion_v2_metal" which is the legacy T8-1 scaffold name. The coverage-audit test test_metal_kernel_coverage_audit looks up "<basename>_metal" for each basename in the canonical list — "integer_motion_v2_metal" — and got NULL back because the stored name did not match. Rename the .name field to "integer_motion_v2_metal" and update: - core/src/metal/dispatch_strategy.c (g_metal_features[] entry) - core/test/test_metal_smoke.c (extractor lookup + dispatch check) - core/test/test_metal_kernel_registration.c (kRegisteredMetalExtractors + kTemporal) - core/test/test_metal_motion_v2_parity.c (vmaf_use_feature call) No behaviour change: the TEMPORAL flag, provided_features array, and all kernel dispatch paths are unchanged; only the public name string is corrected to match the canonical integer_motion_v2_metal convention used by every other integer_* Metal extractor in the tree. Fixes: Build — macOS Metal (T8-1 scaffold) CI failure on master tip 40d192e (run 26726137428 / 26726137416). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(sycl): add close_fex_sycl forward decls in integer_adm + integer_vif The close_fex_sycl calls in init_fex_sycl error paths (introduced by the SY-2a leak fix) appear before the function definition at the bottom of the file. Builds fail under strict C++ modes with "use of undeclared identifier 'close_fex_sycl'". Add a forward decl right before init_fex_sycl. Master Ubuntu SYCL + CUDA build failed with this error; bundled here into the master-unblock PR so a single PR restores green CI on all 5 affected platforms. This is the build-fix portion of PR #516. The full leak-fix (calling close_chroma_sycl / close_temporal_sycl from speed extractor error paths) remains in #516 to land separately. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(iqa): replace ATOMIC_VAR_INIT(NULL) with NULL — MSVC C2099 ATOMIC_VAR_INIT is absent from MSVC's <stdatomic.h> (undefined identifier, treated as extern returning int), making the file-scope static initialiser of g_ssim_dispatch_installer non-constant and causing C2099 on the Windows MSVC + CUDA build matrix. C11 §7.17.2.1 p3 explicitly allows plain NULL as an initial value for any atomic type; the macro was deprecated in C17 for exactly this reason. Replace ATOMIC_VAR_INIT(NULL) with NULL — semantically identical on all three compilers we target (GCC, Clang, MSVC). Fixes CI job "Build — Windows MSVC + CUDA (build only)". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(test): portable temp-file handling in test_svm_api — MSVC + MinGW64 Two separate Windows failures in test_svm_api.c: 1. MSVC + SYCL build (C compilation): `fatal error: 'unistd.h' file not found` at line 47. Guard the include with `#ifndef _WIN32 / #endif`. 2. MinGW64 runtime: `test_save_load_roundtrip` failed at assertion "mkstemp ok" — `mkstemp("/tmp/vmaf-test-svm-XXXXXX", ...)` returned -1 because MSYS2/MinGW64 in the GitHub Actions runner does not expose a usable /tmp from the MINGW64 shell. Fix: add `make_svm_temp_path()` portable helper (mirrors the `make_temp_output_path()` pattern in `test_public_api_score.c` per `core/test/AGENTS.md §6`). On `_WIN32`: query `GetTempPathA()` + embed PID for uniqueness + pre-create the file. On POSIX: mkstemp on the existing /tmp template. Replace `unlink()` with `remove()` so no unistd.h dependency is needed for the cleanup path. Fixes CI jobs "Build — Windows MSVC + oneAPI SYCL (build only)" and "Build — Windows MinGW64 (CPU)". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(test): portable mkdtemp in test_mkdirp — MinGW64 /tmp missing test_mkdirp_single_level failed on Windows MinGW64 at assertion "mkdtemp must succeed" because MSYS2/MinGW64 in the GitHub Actions runner does not expose a usable /tmp from the MINGW64 shell — hardcoded /tmp/vmaf_mkdirp_*_XXXXXX templates return NULL from mkdtemp. Fix: add `portable_mkdtemp()` + MKDTEMP/RMDIR macros (mirrors the approach documented in `core/test/AGENTS.md §6`). On `_WIN32`: query `GetTempPathA()`, build a PID-unique path, and `CreateDirectoryA()`. On POSIX: delegate to `mkdtemp(3)` unchanged. Replace `(void)rmdir(p)` with the `RMDIR()` macro so Windows builds use `_rmdir()` from `<direct.h>`. Guard `<unistd.h>` with `#ifndef _WIN32`. All three test functions (single_level, idempotent_eexist, normalize_double_slash) updated consistently. Fixes CI job "Build — Windows MinGW64 (CPU)". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(changelog): add Layer-2 platform-breakage fix entries Document three new fixes (MSVC C2099, MSVC+SYCL unistd.h, MinGW64 mkstemp/mkdtemp runtime) in the changelog fragment for PR #517. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(test): test_mkdirp.c — POSIX path needs /tmp/...XXXXXX template The previous portable_mkdtemp() helper only existed inside the _WIN32 branch and the POSIX MKDTEMP macro expanded to bare mkdtemp(tmpl) — which was called with the uninitialised char tmpl[260] buffer. mkdtemp(3) requires a NUL-terminated template ending in 6 'X' chars; without it, it returns NULL with EINVAL and every single test_mkdirp_* case failed on Linux (test_mkdirp_single_level: "mkdtemp must succeed"). Unify portable_mkdtemp across platforms: on POSIX it now initialises the buffer with "/tmp/vmaf_mkdirp_XXXXXX" via snprintf before delegating to mkdtemp(3). The Win32 branch is unchanged. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(test): test_mkdirp.c — define S_ISDIR shim on Windows Windows MSVC + Clang/SYCL ship <sys/stat.h> with _S_IFMT / _S_IFDIR but no POSIX S_ISDIR(mode) macro, causing: error: call to undeclared function 'S_ISDIR' on lines 105 and 153 of test_mkdirp.c (Layer-3 follow-up to the portable_mkdtemp helper from Layer-2). Define the standard expansion under #ifdef _WIN32 / #ifndef S_ISDIR. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(mkdirp): normalize POSIX '/' to '\\' on Windows in path_normalize mkdirp.c uses PATH_SEPARATOR == '\\' on Windows when walking the path backwards to strip leaves. But path_normalize() only collapses '/' sequences and never converts them, so a mixed-separator path like C:\Users\runneradmin\AppData\Local\Temp\vmaf_mkdirp_1234/a/b/c walked backwards stops at the LAST '\\' (i.e. \\vmaf_mkdirp_1234), leaving '/a/b/c' as a single un-split leaf. _mkdir() then tries to create 'a/b/c' inside the parent in one shot and fails with ENOENT. Concrete symptom on master tip + Layer-3 commits of #517: MinGW64 runner reports `test_mkdirp_normalize_double_slash` FAIL with "mkdirp with redundant '/' must succeed". Under _WIN32, convert each '/' to '\\' as we copy + collapse runs of either separator. POSIX path remains unchanged. The Windows kernel treats both separators interchangeably at the API surface, so this only affects mkdirp's internal walker. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(backends/sycl): document close_fex_sycl forward-decl pattern (SY-2a) The Layer-1 commit added forward declarations of close_fex_sycl in integer_adm_sycl.cpp + integer_vif_sycl.cpp so init_fex_sycl's USM-failure cleanup compiles under strict C++ modes. ADR-0167 doc-substance gate requires a corresponding edit under docs/backends/sycl/ when SYCL feature kernels are touched. Add a "## Design notes" bullet explaining the pattern + why each TU needs the local forward declaration. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(test): correct metal coverage audit basename + fix go-ci meson setup path Two pre-existing master CI failures preventing merges via Required Checks Aggregator: 1. test_metal_kernel_coverage_audit (all macOS jobs): g_metal_kernel_basenames[] listed "integer_motion_v2" expecting extractor "integer_motion_v2_metal", but the registered name is "motion_v2_metal" (ADR-0421 T8-1c short alias). Fix: change entry to "motion_v2"; add clarifying comment that entries are registered extractor name prefixes, not .mm file stems. 2. Go CI: meson setup core/build-cpu passed only one positional arg, so meson interpreted it as the source dir (no meson.build there) and exited 1. Fix: meson setup core core/build-cpu matching the build.yml pattern. Research: docs/research/preexisting-macos-python-tinyai-failures-2026-06-01.md Changelog: changelog.d/fixed/preexisting-macos-python-tinyai.md state.md: T-PREEXISTING-MACOS-TINYAI-CI-FAILURES-2026-06-01 added to Recently closed No ADR: bug fixes (CLAUDE.md §12 r8) no rebase impact: test-only and CI workflow changes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ai/test): repair 9 Tiny AI pytest failures on master tip Pre-existing failures in the Tiny AI (DNN Suite + ai/ Pytests) required check (run 26726137418): 1. test_data_datasets_branches.py (3 failures): ManifestEntry._sha256_shape validator (added PR #506) rejects sha256 values shorter than 64 hex chars. Test fixtures used abbreviated stubs "deadbeef", "cafebabe", "s". Fix: replace with valid 64-char hex constants _SHA256_A / _SHA256_B / _SHA256_K. 2. test_frame_loader.py (4 failures): iter_frames now passes stderr=subprocess.PIPE to Popen (for ffmpeg diagnostic capture on non-zero exit). The fake_popen stub only accepted stdout, causing TypeError on every call. _FakeProcess also lacked a stderr attribute accessed by iter_frames cleanup path. Fix: add stderr param to fake_popen signature and stderr=None to _FakeProcess. 3. test_parquet_utils.py (1 failure): write_parquet_atomic was refactored to use pyarrow directly via _write_v2(); it no longer calls df.to_parquet(). The test override of that method never fired, so no RuntimeError was raised. Fix: inject via monkeypatch on aiutils.parquet_utils._write_v2 instead. No ADR: test-only fixes (CLAUDE.md §12 r8) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * revert: undo Layer-1 Metal extractor rename — kept basenames fix instead Layer-1 commit 2b61f9b renamed the integer_motion_v2_metal extractor's .name field from "motion_v2_metal" to "integer_motion_v2_metal" to satisfy the kernel coverage audit. The later bundled commit 902d8c2 (from #518) applied the opposite resolution — change the audit's basename list from "integer_motion_v2" to "motion_v2" — because "motion_v2_metal" is the canonical short alias chosen in ADR-0421 / T8-1c. Both fixes individually closed the audit; together they re-broke it. Revert the .name rename so the registered name matches the short-alias convention and the bundled audit fix. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(ci/sanitizers): exclude test_y4m_alloc_failure from ASan/TSan/MSan runs test_y4m_alloc_failure uses setrlimit(RLIMIT_AS) to cap the process address space at 256 MiB, forcing malloc failure in y4m_input_open_impl. ASan, TSan, and MSan runtimes each require hundreds of MiB of shadow-memory mmap during startup; the sanitizer's own mmap fails before any test logic runs, producing "Failed to mmap" / "internal allocator is out of memory" SIGABRT on every sanitizer build. The bug the test guards (dst_buf-NULL regression) is exercised by every unsanitized fast-suite run, so no correctness gap results from the exclusion. Added to EXCLUDE regex in: - sanitizers.yml (ASan+UBSan PR gate + TSan master gate) - tests-and-quality-gates.yml (address/undefined/thread matrix) Tracked as T-Y4M-ALLOC-RLIMIT-AS-SANITIZER-INCOMPATIBLE in docs/state.md. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ci/lint): exclude core/src/compat/win32/ from clang-tidy changed-files scan The Win32 pthread shim (core/src/compat/win32/pthread.h) contains an intentional #error "win32/pthread.h shim included on a non-Windows target" guard. When the file appears in the PR diff, clang-tidy on the Linux CI runner hits this guard as a clang-diagnostic-error and exits with code 1, blocking the required Clang-Tidy check. Add core/src/compat/win32/ to all four grep -v exclusion blocks in the "Run clang-tidy on changed files" step (PR/push/zero-sha push/dispatch variants), matching the existing pattern for arm64, cuda, sycl, hip, mcp, and fuzz paths that are likewise excluded because they require non-Linux toolchains or non-CPU build configurations. Tracked as T-CLANG-TIDY-WIN32-PTHREAD-SHIM-2026-06-01 in docs/state.md. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ai/test): repair 5 remaining Tiny AI pytest failures on master tip Two root causes: 1. test_manifest_entry_is_frozen — ManifestEntry was migrated from @DataClass(frozen=True) to pydantic.BaseModel(ConfigDict(frozen=True)) in PR #506. Pydantic raises ValidationError (ValueError subclass) on frozen-field assignment, not dataclasses.FrozenInstanceError (AttributeError subclass). Updated assertion to pydantic.ValidationError. 2. test_iter_frames_gray_keeps_2d_shape + 4 × test_iter_frames_packed_color_* — iter_frames calls proc.wait(timeout=_FFMPEG_WAIT_TIMEOUT_S) but _FakeProcess.wait() only accepted a positional argument. Added timeout: float | None = None keyword parameter to _FakeProcess.wait(). Tracked as T-AI-TEST-PYDANTIC-FROZEN-INSTANCE-2026-06-01 and T-AI-TEST-FRAME-LOADER-FAKE-WAIT-TIMEOUT-2026-06-01 in docs/state.md. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(mcp): remove dead _run_benchmark() + fix test_bug3 for new raise semantics server.py contained two definitions of async def _run_benchmark: - Line 824: no parameters, returned dict on failure (pre-ADR-0608) - Line 1409: takes progress_token, raises RuntimeError on failure Python uses the last definition; the first was dead code. Its presence was the source of confusion that led to test_bug3_run_benchmark_surfaces_ silent_pipefail being written for the old return-dict contract. Remove the dead first definition (80 lines). Update the test to: - Use monkeypatch.setattr(asyncio, "create_subprocess_exec", ...) like the other tests in test_path_and_bench_env.py, to avoid patching via patch.object(srv.asyncio, ...) which resolves to the wrong binding - Assert pytest.raises(RuntimeError, match="benchmark failed.*no output") matching the ADR-0608 E-1 raise semantics of the current implementation Tracked as T-MCP-RUN-BENCHMARK-DEAD-CODE-2026-06-01 in docs/state.md. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(state/changelog): document Layer-5 residual CI failure fixes Add 5 new entries to docs/state.md under Recently Closed: - T-Y4M-ALLOC-RLIMIT-AS-SANITIZER-INCOMPATIBLE-2026-06-01 - T-CLANG-TIDY-WIN32-PTHREAD-SHIM-2026-06-01 - T-AI-TEST-PYDANTIC-FROZEN-INSTANCE-2026-06-01 - T-AI-TEST-FRAME-LOADER-FAKE-WAIT-TIMEOUT-2026-06-01 - T-MCP-RUN-BENCHMARK-DEAD-CODE-2026-06-01 Append 5 fix entries to changelog.d/fixed/preexisting-macos-python-tinyai.md. Update docs/rebase-notes.md Layer-5 entry with newly touched files. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(mcp/tools): note Layer-5 dead-code removal of legacy _run_benchmark ADR-0167 Doc-Substance Gate flags the Layer-5 MCP server.py edit as a surface change without matching docs/mcp/ entry. Add an "Error contract" callout under run_benchmark explaining that the legacy partial-dict fallback was removed in PR #517 Layer-5 and that MCP clients should branch on isError=True (ADR-0608). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(state): rephrase "this PR" → "PR #517" in clang-tidy row ADR-0165 state.md Touch Gate rejects placeholder strings like "this PR" in newly-inserted rows because they never get rewritten to numeric refs after merge. Replace the only such phrase in #517's Layer-5 row T-CLANG-TIDY-WIN32-PTHREAD-SHIM-2026-06-01 with the explicit PR number. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat!: complete ANSNR sunset — drop 20 stale Netflix golden assertions PR #38 (ADR-0709) dropped float_ansnr from the C library on 2026-05-28 because Netflix never adopted it as a VMAF component (Research-0733: "pre-VMAF 2001 era; legacy only"). The sunset was correct — ANSNR has no production use today, isn't part of the VMAF v0.6.1 model, and was only kept by Netflix upstream for libvmaf C-surface completeness. But the corresponding Netflix golden Python test assertions in python/test/feature_extractor_test.py (20 assertAlmostEqual calls expecting VMAF_feature_anpsnr_score / VMAF_integer_feature_anpsnr_score values) were left in place. macOS CI now KeyErrors on every test_run_vmaf_(integer_)fextractor* variant because the C library no longer produces the key the test reads. Per ADR-0709 / PR #38 sunset (user-authorized 2026-06-01: "we dont use it for anything and decided to"), complete the Python-side sunset: - Strip 20 anpsnr assertions from python/test/feature_extractor_test.py (11 single-line + 9 multi-line VMAF_feature_anpsnr_score / VMAF_integer_feature_anpsnr_score variants) - Drop 2 anpsnr fixture rows from compat/python-vmaf/core/result.py - Drop 1 anpsnr legacy comment in compat/python-vmaf/core/feature_extractor.py - Drop the "float_anpsnr → float_ansnr" mapping in ai/data/feature_extractor.py This is the canonical case where CLAUDE.md Global Rule #1 ("never modify Netflix golden-data assertions") yields to ADR-0709's explicit sunset: the assertions test a feature the fork no longer ships. They are not score-correctness goldens — they are completeness checks against a removed code path. Net behavioural change: macOS feature_extractor_test.py::test_run_vmaf_* variants stop KeyError-ing; all other golden assertions (VMAF_score, vif_score, adm_score, motion_score, integer_*, ssim, etc.) untouched. Closes T-LEGACY-RUNNER-ANSNR-BROKEN. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat!: extend ANSNR sunset round 2 - strip ansnr from VMAF_feature lists * test: ANSNR sunset round 3 --------- Co-authored-by: Lusoris <lusoris@pm.me> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
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
Deep audit of every fork-local Python file under
ai/src/corpus/,ai/src/vmaf_train/data/, andmcp-server/vmaf-mcp/src/vmaf_mcp/server.py,post-master-rebase. Fourteen defects across six families (subprocess hangs,
locale leaks, silent ffmpeg failures, pickle code-exec, NaN sort
non-determinism, concurrent-tempdir race) are fixed in one bundle.
Type
fix— bug fixBug families discharged
timeout=)probe_geometry,download_clip,_run_vmafencoding=)load_manifest,load_mos_csv,write_manifest,_run_vmafJSON read,_run_vmaf_score,_list_extractors,_describe_model_file,_probe_backenditer_frames(no stderr, no rc check, unboundedwait())_load_frame—np.loadwithoutallow_pickle=False_pick_worst_frames_describe_worst_framesshared/tmp/vmaf-mcp-worst-<pid>Full per-defect breakdown in
docs/research/0983-python-surfaces-bug-audit-2026-05-31.md.Tests
ai/tests/test_python_surfaces_bug_audit.py.mcp-server/vmaf-mcp/tests/test_python_surfaces_bug_audit.py.test_describe_worst_frames_tmpdir_cleared_on_next_callin
mcp-server/vmaf-mcp/tests/test_server.pywas replaced (notweakened) by
test_describe_worst_frames_allocates_unique_tmpdir_per_call,which asserts the stricter post-fix contract — each call gets its own
mkdtemproot AND peer-call PNGs survive.suite need a built vmaf binary (CI-only).
Constraints honoured
--no-verify.Bug-status hygiene (ADR-0165)
were not tracked as numbered entries in
docs/state.md. Noopen/closed/not-affecting row needs to move.
Netflix golden-data gate (ADR-0024)
assertAlmostEqual(...)score in the Netflix golden Python testswas modified.
Cross-backend numerical results
Deep-dive deliverables (ADR-0108)
docs/research/0983-python-surfaces-bug-audit-2026-05-31.md.## Decision matrix.AGENTS.mdinvariant note — no rebase-sensitive invariants:every touched file is fork-local (no upstream-mirror behaviour).
changelog.d/fixed/python-surfaces-bug-audit-2026-05-31.md.docs/rebase-notes.mdunderPython-surfaces bug-audit bundle (2026-05-31, ...); flagged as
no rebase impactbecause every touched file is fork-local.Reproducer
# From repo root, with the dev venv installed. .venv/bin/python -m pytest \ ai/tests/test_python_surfaces_bug_audit.py \ mcp-server/vmaf-mcp/tests/test_python_surfaces_bug_audit.py \ mcp-server/vmaf-mcp/tests/test_server.py \ -qKnown follow-ups
ai/scripts/corner (out of scope today; the user pinned the surfaceslist explicitly).