refactor(dnn,psnr): clear the dnn + psnr clang-tidy buckets to zero (ADR-1142 wave 2)

[lusoris]

Summary

Wave-2 of the epic #1237 ADR-1142 whole-tree clang-tidy ratchet, covering the core/src/dnn/ and core/src/feature/*psnr* buckets. Nine sources go from 60 measured warnings to 0: model_loader.c 29 → 0 (four oversized functions split into named helpers, the per-encoder preset ladder replaced by a lookup table, two ADR-0461-cited getenv NOLINTs), psnr_tools.cpp 24 → 0 (designated initialisers), integer_psnr.c 3 → 0, dnn_api.c / onnx_scan.c / float_psnr.c 2 → 0 each, dnn_attach_api.c / op_allowlist.c / psnr.c 1 → 0 each. Every finding is fixed by refactoring; the only new suppressions reuse an established in-tree citation (ADR-0278 cross-TU registry linkage on the two feature-extractor symbols, ADR-0461 caller-contract on the two getenv sites, ADR-1138 file-scoped modernize-use-nullptr brackets on the seven touched C TUs). Numeric paths are proven inert: --feature psnr --precision=max on the src01_hrc00 / src01_hrc01 pair is byte-identical before and after once the fps and version metadata lines are excluded, and the Netflix golden gate stays at 271 passed / 12 skipped / 0 failed. The pass also surfaced a real defect: core/test/dnn/test_cli.sh's DNN-availability probe never reached the check it was probing for, so the test failed instead of skipping on every -Denable_dnn=disabled build — fixed here and recorded in docs/state.md as T-DNN-TESTCLI-PROBE-NEVER-SKIPS-2026-09-05. scripts/ci/tidy-baseline-cpu.json is deliberately NOT updated here — see the "Baseline" note below. Deferred: core/src/feature/third_party/xiph/psnr_hvs.c (2 modernize-use-nullptr) is vendored Xiph code outside the brief's core/src/feature/*psnr*.c scope and is left for the vendored-code wave; the psnr test files (test_psnr_hvs_coverage.c 32, test_integer_psnr_coverage.c 31, …) belong to the test-tree bucket of the same epic.

C translation units keep NULL (ADR-1138)

The first revision of this branch rewrote 90 NULLs to the C23 nullptr keyword across the seven touched .c files. That is exactly the rewrite ADR-1138 forbids: Build — Windows MSVC + CUDA (build only) is a required status check, it compiles these translation units with cl.exe, and MSVC's documented /std:clatest C23 feature set does not include nullptr — PR #1192 had to revert the identical rewrite after all three Windows legs failed with "use of undeclared identifier nullptr". Commit 35ec49cfc restores NULL everywhere and gives each of the seven files the file-scoped /* NOLINTBEGIN(modernize-use-nullptr) … ADR-1138. */ … /* NOLINTEND */ bracket the ADR prescribes, worded as in core/src/dnn/ort_backend.c and core/src/feature/integer_adm.c. psnr_tools.cpp is C++ and keeps nullptr; ADR-0915's ratchet still covers it in full.

The revert re-opened one finding the keyword rewrite had masked — vmaf_dnn_session_open() at 114 lines against the 60-line readability-function-size budget. It is split into resolve_load_path() (the ADR-0174 int8-sibling lookup plus its fp32 fallback) and setup_luma_fast_path() (the NCHW [1,1,H,W] shape check and scratch allocation), which also retires the function's goto fail ladder. Behaviour is unchanged: same validation order, same ownership on every error path, same -ENAMETOOLONG / -ENOMEM / -ENOTSUP contracts, and all seven dnn meson tests (including test_dnn_session_api and test_model_loader) stay green.

Baseline — must be tightened from the CI artifact

The 60-warning drop needs scripts/ci/tidy-baseline-cpu.json tightened in this PR (ADR-1142 exit 3). That baseline cannot be produced on this workstation: .github/workflows/lint-and-format.yml measures with CC=gcc-14 CXX=g++-14 meson setup build core (build dir build/ at the repo root), and this machine has no gcc-14 (gcc 16.2.1 / gcc-15 only). A local --write run therefore produces two classes of false delta on top of the real ones: every generated model TU moves to a different build-dir path, and several C++ TUs the diff never touches drift upward purely from the newer libstdc++ headers. Committing that would encode paths CI never emits and would silently raise the ceiling on untouched files, so the local write is discarded and master's baseline is unchanged. ADR-1142 also forbids hand-editing a baseline.

The Tidy Ratchet job is expected to exit 3 on the first non-draft run and upload the correct measurement as the tidy-ratchet-cpu.json artifact — that file is the baseline to commit, exactly as the workflow step "Upload measurement (commit it as the new baseline when the ratchet asks)" describes, and exactly the flow PR #1192 used. All nine touched files measure 0 locally with clang-tidy 22.1.8 (the version the baseline records), so the CI measurement should show 0 for each of them and no increase anywhere else.

Type

• refactor — lint-debt cleanup of the dnn + psnr buckets, plus one test-harness bug fix.

Checklist

• Commits follow Conventional Commits.
• make format-check green; pre-commit run --files green on every touched file (clang-format, markdownlint-cli2, shellcheck, semgrep, copyright, state.md-id and ADR guards).
• Unit tests: meson test -C bt → Ok 129 / Fail 0 on a CPU-only -Denable_cuda=false -Denable_sycl=false -Db_lto=false build, and make VENV=… test-netflix-golden (271 passed / 12 skipped / 0 failed). core/test/dnn/test_cli.sh now correctly reports meson-skip (77) on a DNN-disabled build instead of failing.
• Docs — no docs needed: internal refactor plus a test-harness fix; no CLI flag, public C API, meson_options.txt entry, feature extractor, backend or output-schema change. The maintainer-facing notes land in the two AGENTS.md files and docs/rebase-notes.md.
• SIMD/GPU, twins, new C sources, breaking change, ADR — n/a: no SIMD or GPU path touched (CPU scalar and shared code only), no twin drift (the CUDA/SYCL/HIP psnr twins are untouched), no new C source files, no API or behaviour change, and no new ADR (this is ratchet work under the existing ADR-1142 / ADR-0141 / ADR-0278 / ADR-1138).

Bug-status hygiene (ADR-0165)

• docs/state.md — one new Recently closed row, T-DNN-TESTCLI-PROBE-NEVER-SKIPS-2026-09-05, covering the test_cli.sh probe defect, its root cause in configure_tiny_model()'s position in core/tools/vmaf.cpp, the fix and the rc=77 verification, citing PR #1323.

Netflix golden-data gate (ADR-0024)

• I did not modify any assertAlmostEqual(...) score in the Netflix golden Python tests.

Deep-dive deliverables (ADR-0108)

• Research digest — no digest needed: mechanical lint-debt cleanup against an already-decided policy (ADR-1142); every finding class already has a settled in-tree remedy.
• Decision matrix — no alternatives: each clang-tidy finding has exactly one sanctioned remedy in this tree (refactor, or the ADR-0278 / ADR-0461 / ADR-1138 citation that already covers the identical construct elsewhere), and the task brief explicitly scopes this work as "no ADR".
• AGENTS.md invariant note — core/src/dnn/AGENTS.md (model_loader lint shape, the ADR-1138 NULL bracket, and the test_cli.sh probe contract) and core/src/feature/AGENTS.md (psnr bucket lint shape + ADR-1138 bracket).
• Reproducer / smoke-test command — below.
• CHANGELOG fragment — changelog.d/changed/tidy-wave2-dnn-psnr.md
• Rebase note — docs/rebase-notes.md entry "core/src/feature/psnr_tools.cpp, psnr.c, integer_psnr.c, float_psnr.c — ADR-1142 tidy ratchet"

Reproducer

# 1. CPU build, the flags CI's tidy lane uses (-Db_lto=false: ADR-1172's
#    -flto=4 is rejected by clang and breaks clang-tidy on every TU).
meson setup bt core -Denable_cuda=false -Denable_sycl=false -Db_lto=false
nice -n 10 ninja -C bt -j 4          # expect: 0 warnings, 0 errors

# 2. Every touched source measures 0 clang-tidy warnings (clang-tidy 22.1.8).
for f in core/src/dnn/model_loader.c core/src/dnn/dnn_api.c \
         core/src/dnn/dnn_attach_api.c core/src/dnn/onnx_scan.c \
         core/src/dnn/op_allowlist.c core/src/feature/psnr_tools.cpp \
         core/src/feature/integer_psnr.c core/src/feature/float_psnr.c \
         core/src/feature/psnr.c; do
  echo "=== $f"; clang-tidy -p bt --quiet "$f" 2>/dev/null | grep -E 'warning:|error:'
done
# expect: nine "=== <file>" lines and nothing else.

# 3. No C translation unit uses the C23 keyword (ADR-1138 / Windows MSVC lane).
grep -rnE '(^|[^-A-Za-z_])nullptr([^A-Za-z_]|$)' core/src/dnn/*.c \
     core/src/feature/*psnr*.c | grep -v 'modernize-use-nullptr'
# expect: no output.

# 4. psnr is numerically inert — scores byte-identical before/after.
./bt/tools/vmaf \
  --reference python/test/resource/yuv/src01_hrc00_576x324.yuv \
  --distorted python/test/resource/yuv/src01_hrc01_576x324.yuv \
  --width 576 --height 324 --pixel_format 420 --bitdepth 8 \
  --feature psnr --precision=max --json --output /tmp/psnr.json
grep -vE '"(fps|version)"' /tmp/psnr.json | sha256sum

# 5. The test_cli.sh probe now skips instead of failing on a DNN-disabled build.
VMAF_BIN=$PWD/bt/tools/vmaf bash core/test/dnn/test_cli.sh; echo "rc=$?"
# expect: "libvmaf built without DNN support; skipping DNN CLI smoke", rc=77

# 6. Full suites.
meson test -C bt                             # expect: Ok 129, Fail 0
make VENV="$PWD/.venv" test-netflix-golden   # expect: 271 passed, 12 skipped

🤖 Generated with Claude Code