Repository navigation
fix(ci): let the clang-tidy header filter match absolute paths so headers are ratcheted - #1504
Merged
Merged
Conversation
lusoris
force-pushed
the
fix/tidy-header-filter
branch
2 times, most recently
from
September 20, 2026 07:46
d6213c9 to
14220ca
Compare
…ders are ratcheted .clang-tidy's HeaderFilterRegex was anchored at ^core/, but clang-tidy matches the regex against the absolute path it read from compile_commands.json, which never starts with core/. Every in-repo header was therefore dropped as non-user code: one translation unit reports 'Suppressed 540 warnings (480 in non-user code)', and the CPU baseline holds 462 warnings with none in a header while the tree has 60 headers carrying 313, adm_tools.h alone 142. The ADR-1142 whole-tree ratchet had never counted a header. The regex gains a (^|/) prefix, which matches the same files whether the path is relative or absolute and nothing outside the repository. The CPU baseline is committed from CI's own tidy-ratchet-cpu artifact in a follow-up commit on this branch, since that is the clang-tidy the gate runs; GPU-lane baselines are re-recorded locally where a toolchain exists. ADR-1265.
…h headers visible Re-recording the GPU lanes for ADR-1265 showed they could not be measured at all (ledger L-41), for three separate reasons. The Makefile passes each lane flag as --extra-arg=<flag>. The ratchet's own --extra-arg option strips that wrapper and forwarded the values to clang-tidy bare, so clang-tidy saw '--cuda-host-only', '-x', 'hip' as its own unknown options and every one of ~350 translation units was reported 'measurement unusable'. run_one now re-wraps them. The sycl lane's --clang-tidy scripts/ci/clang-tidy-sycl.sh is repository-relative, but run_one switches cwd to the build directory first: [Errno 2]. The path is resolved before the switch. An icx build records -fp-model=precise on every strict-FP unit; stock clang spells it -ffp-model=precise and treats the icx form as a hard 'unknown argument' error, which failed 104 sycl-lane units. The wrapper translates the flag in a copy of the compilation database. Baselines: cpu is CI's own tidy-ratchet-cpu artifact committed verbatim (775 warnings / 132 files, 313 of them in 60 headers, clang-tidy 22.1.8, gcc-15). cuda 1449/173, hip 1250/147 and sycl 815/125 are local whole-database measurements with 0 unusable units each. scripts/ci/tests/test_tidy_ratchet_extra_args.py pins both forwarding fixes; it fails on master's script with the bare flag visible in argv.
The test added with the forwarding fix loaded the hyphenated ratchet script without guarding spec or spec.loader against None and without a return annotation, which mypy reports as four findings, and it used os.getcwd() where ruff wants Path.cwd() (PTH109). Same shape as the test_gen_gpu_compile_commands.py fix on the ADM train.
lusoris
force-pushed
the
fix/tidy-header-filter
branch
from
September 20, 2026 08:33
14220ca to
8c9da62
Compare
lusoris
added a commit
that referenced
this pull request
Sep 20, 2026
The rebase left a conflict marker in scripts/ci/tidy-ratchet.py: the branch carried the pre-#1504 file and master now has both forwarding fixes. master's version is the one that stays.
lusoris
added a commit
that referenced
this pull request
Sep 20, 2026
…eep one copy of its tests The stack's own test_tidy_ratchet.py asserts that run_one() passes an absolute path to clang-tidy's -p, and it failed: run_one resolves the binary before switching cwd into the translation unit's directory but left the build dir relative. For a meson build that directory IS the build dir, so a relative -p resolves to <build>/<build> and clang-tidy finds no compilation database. Same class as the wrapper-path fix beside it; build_dir is resolved once, up front. #1504 landed an equivalent set of assertions as a standalone scripts/ci/tests/test_tidy_ratchet_extra_args.py while this branch was adding them to the in-tree test_tidy_ratchet.py. The in-tree file is the right home -- it already holds the rest of the ratchet's tests and covers strictly more -- so the standalone copy goes and its one unique assertion, that an already-wrapped --extra-arg is not wrapped twice, moves across. 16 tests pass.
lusoris
added a commit
that referenced
this pull request
Sep 20, 2026
The rebase left a conflict marker in scripts/ci/tidy-ratchet.py: the branch carried the pre-#1504 file and master now has both forwarding fixes. master's version is the one that stays.
lusoris
added a commit
that referenced
this pull request
Sep 20, 2026
…eep one copy of its tests The stack's own test_tidy_ratchet.py asserts that run_one() passes an absolute path to clang-tidy's -p, and it failed: run_one resolves the binary before switching cwd into the translation unit's directory but left the build dir relative. For a meson build that directory IS the build dir, so a relative -p resolves to <build>/<build> and clang-tidy finds no compilation database. Same class as the wrapper-path fix beside it; build_dir is resolved once, up front. #1504 landed an equivalent set of assertions as a standalone scripts/ci/tests/test_tidy_ratchet_extra_args.py while this branch was adding them to the in-tree test_tidy_ratchet.py. The in-tree file is the right home -- it already holds the rest of the ratchet's tests and covers strictly more -- so the standalone copy goes and its one unique assertion, that an already-wrapped --extra-arg is not wrapped twice, moves across. 16 tests pass.
lusoris
added a commit
that referenced
this pull request
Sep 20, 2026
The rebase left a conflict marker in scripts/ci/tidy-ratchet.py: the branch carried the pre-#1504 file and master now has both forwarding fixes. master's version is the one that stays.
lusoris
added a commit
that referenced
this pull request
Sep 20, 2026
…eep one copy of its tests The stack's own test_tidy_ratchet.py asserts that run_one() passes an absolute path to clang-tidy's -p, and it failed: run_one resolves the binary before switching cwd into the translation unit's directory but left the build dir relative. For a meson build that directory IS the build dir, so a relative -p resolves to <build>/<build> and clang-tidy finds no compilation database. Same class as the wrapper-path fix beside it; build_dir is resolved once, up front. #1504 landed an equivalent set of assertions as a standalone scripts/ci/tests/test_tidy_ratchet_extra_args.py while this branch was adding them to the in-tree test_tidy_ratchet.py. The in-tree file is the right home -- it already holds the rest of the ratchet's tests and covers strictly more -- so the standalone copy goes and its one unique assertion, that an already-wrapped --extra-arg is not wrapped twice, moves across. 16 tests pass.
lusoris
added a commit
that referenced
this pull request
Sep 22, 2026
ADR-0880 is Accepted and its changelog fragment shipped, yet testdata/check_borders.py, testdata/compare_a380.py and testdata/scores_sycl_b580_576_mq.json were still in the tree. `git log --all --diff-filter=D` over the three paths returns nothing on any branch, so the deletion was never made anywhere while the rendered CHANGELOG.md has claimed it since. Every claim the ADR makes about them was re-checked before acting: outside that one changelog fragment nothing in the tree references any of the three, and the _mq snapshot does carry 12 metrics per frame against the canonical 34 in scores_sycl_b580_576.json. No new changelog fragment — the existing removed/ one becomes true. docs/state.md records what a bug-ledger verification sweep established. Four of five rows were stale rather than open: the Go duplicate- implementation families (1f09437, in the train, still open on origin/master), the resurrected i686 lane (ef1c160, #1497) and the clang-tidy header filter (8845ac2, #1504, ADR-1265, all four baselines re-recorded with headers) are already fixed on this branch. T-SYCL-A380-SNAPSHOTS-MOTION-ZERO-2026-09-21 is opened with what the snapshots actually contain rather than what was filed: integer_motion and integer_motion2 are 0.0 on every one of the 48 frames of all five scores_sycl_a380_*.json, and at 576 and 640 all 48 frames carry one identical vmaf value. Every other backend snapshot in testdata/ has 45 non-zero motion frames. They record a broken run, not a stale-but-valid one. Nothing is regenerated here: icpx and the A380 are both available on this host, but the 720/1080/4k fixtures are gitignored and absent, and re-deriving them under today's ffmpeg would force the CPU snapshots at those resolutions to be re-recorded in the same pass.
3 of 6 tasks
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
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
.clang-tidy's header filter could never match a header, so the ADR-1142 whole-tree ratchet has never counted one.clang-tidy tests that regex against the path as it saw it — from
compile_commands.json, absolute:/home/…/vmafx/core/src/picture.h. An expression anchored at^core/cannot match that, so every in-repo header has been filtered as "non-user code" since the file was written. One translation unit (core/src/picture.c):Tree-wide, the CPU baseline holds 462 warnings in 72 files and not one is in a header, while the tree has 60 headers carrying 313 findings —
core/src/feature/adm_tools.halone has 142. LedgerL-45, reported by the CAMBI agent and confirmed by measurement.The regex gains a
(^|/)prefix — same file set, matched relative or absolute, nothing outside the repository (no system header contains/core/src/). ADR-1265 records the four alternatives (an.*filter, moving the filter into the script, keeping headers excluded as policy, fixing the regex without re-recording) and why each loses.How the baselines are re-recorded
CPU lane — from CI, not from my machine. The
Tidy Ratchetjob measures withclang-tidy-22onubuntu-26.04and uploadstidy-ratchet-cpu.json"to commit as the new baseline when the ratchet asks". A locally recorded file would carry a different clang-tidy and gcc and would drift immediately. So this PR has two commits: the fix (this one, whoseTidy Ratchetrun fails by design and uploads the measurement), then that artifact committed verbatim asscripts/ci/tidy-baseline-cpu.json.GPU lanes — locally, advisory. No CI job measures
cuda,hiporsycl; their baselines are re-recorded withmake tidy-ratchet-write LANE=<lane>on this machine (clang-tidy 22.1.8,/opt/cuda,/opt/rocm, oneAPI). Only headers next to each lane's translation units are attributed to that lane, per the ratchet's own ownership rule.The measured CPU delta, for the record (local clang-tidy 22.1.8; CI's numbers will differ slightly and are the ones that land):
No source file changes in this PR. Nothing is cleaned; this is the first recording of a class the gate had never seen, which is why the "never raise the baseline" rule does not apply — there is no prior count to raise.
Type
fix— bug fix (in the lint configuration)Checklist
assertAlmostEqual(...)score in the Netflix golden Python tests.docs/state.md: no state delta: a lint-configuration defect tracked as ledgerL-45/L-41; no bug in the fork's tracked set opens or closes.Deep-dive deliverables (ADR-0108)
Suppressed … in non-user codeline and a two-line regex check.## Alternatives considered, four options.AGENTS.mdinvariant note —scripts/ci/AGENTS.md, plusdocs/rebase-notes.md.changelog.d/fixed/clang-tidy-header-filter.md.docs/rebase-notes.md.Reproducer
Rebase hazard
If a sync or a tidy-config refresh restores the
^-only anchor,Tidy Ratchetwill report every header asN -> 0and ask to tighten. That is the filter breaking again, not a cleanup;scripts/ci/AGENTS.mdanddocs/rebase-notes.mdsay so.