Repository navigation
docs: doc-sweep bundle batch-1 (#327 doxygen round 2 + #330 Go test coverage + #336 state.md row sweep + #383 README badges + #337 ADR-0865 ANSNR) - #529
Merged
Conversation
This was referenced Jun 1, 2026
lusoris
force-pushed
the
docs/doc-sweep-bundle-327-330-336-383-337
branch
from
June 3, 2026 11:24
1e7925d to
a9a5554
Compare
lusoris
marked this pull request as ready for review
June 3, 2026 11:24
lusoris
force-pushed
the
docs/doc-sweep-bundle-327-330-336-383-337
branch
from
June 3, 2026 11:31
a9a5554 to
5b423a0
Compare
… points Round-2 follow-on to PR #302 (which closed five targeted gap-findings in libvmaf.h / picture.h / dnn.h). This pass covers the public surfaces that PR #302 left untouched, focusing on the headers the ffmpeg patch stack, the upcoming Go/Rust bindings, and the embedded MCP server consume. Entry points documented: - feature.h (file was 100 % undocumented): - VmafFeatureDictionary (struct doc + ownership-transfer rules) - vmaf_feature_dictionary_set - vmaf_feature_dictionary_free - model.h: - VmafModelFlags (enum + per-flag semantics) - VmafModelConfig (struct + per-field doc) - vmaf_model_load - vmaf_model_load_from_path - vmaf_model_feature_overload (incl. opts_dict ownership transfer) - vmaf_model_destroy (incl. do-not-destroy-after-collection-handoff) - VmafModelCollection (struct doc) - VmafModelCollectionScoreType (enum doc) - VmafModelCollectionScore (struct + per-field doc) - vmaf_model_collection_load - vmaf_model_collection_load_from_path - vmaf_model_collection_feature_overload - vmaf_model_collection_destroy - dnn.h: - vmaf_dnn_session_close (pair-with-open contract) Each block documents the negative-errno return convention, NULL-safety, ownership-transfer semantics, and the destroy-pairing required to avoid double-free of collection-owned sub-models. No semantic / no ABI change. Cleanup pass (CLAUDE.md §12 r12 — touched-file lint-clean rule): the three Netflix-copyright include guards (__VMAF_FEATURE_H__ / __VMAF_MODEL_H__ / __VMAF_DNN_H__) trip clang-tidy's bugprone-reserved-identifier check. Renaming them would diverge from Netflix/vmaf master and break port-only upstream sync (CLAUDE.md §10), so each #ifndef / #define gets an inline NOLINT citing the upstream-mirror invariant — the exact pattern ADR-0278 endorses for load-bearing upstream-parity identifiers. Build + lint: - meson setup build-cpu-doc core -Denable_cuda=false -Denable_sycl=false - ninja -C build-cpu-doc (35 targets touched by header change; clean) - clang-tidy -p build-cpu-doc on all 3 touched headers: 0 fork-local warnings (the 3 reserved-identifier warnings present on master are now NOLINT-cited; remaining warnings are in system headers and suppressed). - pre-commit run --files <all 5 touched files>: green. ADR-0108 deliverables: - Research digest: no digest needed — trivial doc-only addition over Netflix-stable signatures already covered by the existing reference manual. - Decision matrix: no alternatives needed — only-one-way fix; the ownership-transfer text matches the implementation in core/src/model.c and core/src/dict.c verbatim. - AGENTS.md invariant note: no rebase-sensitive invariants — the doc text sits above unchanged upstream signatures; future merges from Netflix produce tractable 3-way merges. The NOLINT cites name the invariant they preserve (upstream-mirror include guards). - Reproducer / smoke test: see PR body. - Changelog fragment: changelog.d/added/libvmaf-public-header-doc-comments-round2.md. - Rebase notes: docs/rebase-notes.md updated with a dedicated section. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add unit tests for the lowest-coverage Go cmd/ subpackages identified by the master-tip workflow audit (Section F). No behavior change. Coverage deltas (against origin/master tip bbcaa8d): cmd/vmafx-controller 18.6% -> 32.4% (+13.8 pp) cmd/vmafx-controller/nodes 80.7% -> 82.5% (+1.8 pp) cmd/vmafx-server 27.5% -> 47.9% (+20.4 pp) cmd/vmafx-mcp 3.5% -> 24.6% (+21.1 pp) New test files: - cmd/vmafx-controller/main_extra_test.go 405 method-not-allowed on /healthz, /readyz, /v1/score; 400 invalid-JSON body; 500 scorer-error mapping via a stub vmaf binary; runHTTP graceful shutdown bounded by GracefulShutdownTimeout; envOr default+override; version(). - cmd/vmafx-controller/nodes/registry_edge_test.go Get(unknown), distinct-IDs-for-same-name contract pin, Heartbeat updates JobsRunning + advances LastHeartbeat (reaper eviction predicate), concurrent Register/Heartbeat under -race, defensive-copy assertion on All(). - cmd/vmafx-server/main_extra_test.go Same shape as the controller HTTP server tests; pins the PR #300 bounded-timeout shutdown invariant. - cmd/vmafx-mcp/impl_test.go All arg helpers (strArg, intArg, floatArg, boolArg, hasArg); every pure helper (classifySourceResolution, modelResolutionClass, resolutionMismatchWarning, inferBackendFromPayload, inferBackendFromSym, stripModelExt, toFFmpegPixfmt, pickWorstFrames, floatFromAny, roundF, truncate); representative handler error paths (handleProbeBackend missing/unknown backend, handleDescribeModel missing name, handleVmafScore invalid path / zero dimensions, handleCompareModels empty list). Pins the errorResult().IsError == true invariant from project memory (MCP isError must be True so clients branch correctly). Drive-by fix: .gitignore anchored the Go binary-ignore rules with a leading slash so they only match the repo-root binaries, not any path component sharing the name. Without this, untracked files like cmd/vmafx-server/main_extra_test.go were silently ignored by `git add`. Verification: go test -race -cover ./cmd/vmafx-controller/... \ ./cmd/vmafx-server/... \ ./cmd/vmafx-mcp/... go vet ./... All green; no behavior change. The pre-existing cmd/vmafx-operator/internal/controller failure (kubebuilder envtest needs etcd binaries on PATH) is unrelated to this change and reproduces on a clean origin/master checkout. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two Open rows in docs/state.md cited PRs that were CLOSED-not-merged and flagged as follow-up by the PR #291 closing agent. Verified against master tip bbcaa8d and master state of the underlying issues: 1. T-CUDA-FILTER1D-RES-DISPATCH-CONFLICT-2026-05-29 — migrated from Open to Recently closed (superseded). The conflict markers only existed on the unmerged scaffold branch tip 35a1fb6. PR #91 (merged 2026-05-29T09:37:48Z) landed ADR-0753 resolution-aware dispatch via the adm_cm_device() consumer without extending dispatch into filter1d_8(), so master never carried the build-failing scaffold variant. PR #214 (the planned conflict-marker cleanup) was CLOSED-not-merged 2026-05-30 when its base scaffold branch was abandoned. core/src/feature/cuda/ integer_vif_cuda.c::filter1d_8() on master uses the clean unconditional cuLaunchKernel paths. 2. T-CPP23-READ-JSON-MODEL-PENDING-2026-05-29 — kept Open but the dead PR #215 citation removed. The C++23 Wave 8 conversion of core/src/read_json_model.c is still pending on master (still a .c source per core/src/meson.build:1578); PR #215 was CLOSED-not-merged 2026-05-30. Owner field rewritten to "Owner-driven; pending fresh PR per ADR-0846 Wave 8". Scope-coordinated with DRAFT PR #291 (docs/state-md-drift-sync — already migrates the 3 Vulkan rows + T-LEGACY-RUNNER-ANSNR-BROKEN + T-LEGACY-RUNNER-STUB-MISSING). Row 203 (T-LEGACY-RUNNER-STUB-MISSING-2026-05-29) cites closed PRs #213 and #181 as OPEN but is left untouched here since PR #291 already rewrites it. Net Open count -1; total T-row count unchanged (153). No code changes — documentation cleanup only. Deliverables (ADR-0108): - no digest needed: state.md hygiene - no alternatives: only-one-way reconciliation - no rebase-sensitive invariants - Reproducer: `gh pr view 214 -R VMAFx/vmafx --json state,mergedAt` returns `{"state":"CLOSED","mergedAt":null}`; `grep -nE '^(<{7}|={7}|>{7})( |$)' core/src/feature/cuda/integer_vif_cuda.c` on master returns empty; `ls core/src/read_json_model.*` returns only `.c` and `.h`. - Changelog: changelog.d/changed/state-md-closed-pr-row-sweep.md - no rebase impact: docs only Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Audit + backfill the fork's repo-metadata surface so cargo / pip / GitHub all advertise the canonical VMAFx/vmafx URLs: - README.md: add Rust CI + Go CI workflow badges (both workflows ship on master but were not surfaced). All five pre-existing workflow badges already point at VMAFx/vmafx and reference real, active, master-green workflows; verified via the Actions API. License, Conventional Commits, OpenSSF Scorecard, ko-fi badges already present. - Cargo.toml: add [workspace.package] with repository / homepage / documentation / license / authors. Both workspace members (bindings/rust/vmafx-sys, core/src/feature/rust/tad) switched to workspace-inherited metadata so URL drift is impossible across the Rust workspace. cargo metadata confirms both crates now expose the VMAFx/vmafx URLs. - pyproject.toml (root, ai/, tools/vmaf-tune, tools/vmaf-roi-score, tools/ensemble-training-kit, dev-llm/, mcp-server/vmaf-mcp/): add [project.urls] with Homepage / Repository / Documentation / Issues / Changelog. All seven fork-authored projects now ship the same URL block; the package indexes (PyPI / internal) get a consistent repository link. - deploy/helm/vmafx/Chart.yaml: already correct (home + sources already point at VMAFx/vmafx). No change needed. - changelog.d/fixed/ + docs/rebase-notes.md: deliverables. Coordination with PR #331 (rebrand sweep): #331 only touches the line-1 copyright header of two of the seven pyproject files; this PR adds a new [project.urls] block below — no merge conflict. Deep-dive deliverables (ADR-0108): - Research digest: no digest needed: trivial repo-metadata sweep. - Decision matrix: no alternatives: only-one-way fix (URLs must match the rebrand target). - AGENTS.md invariant: no rebase-sensitive invariants — fork-only metadata files, none mirror upstream Netflix. - Reproducer: `cargo metadata --no-deps --format-version 1 | jq` + `python3 -c "import tomllib; tomllib.loads(open('pyproject.toml','rb').read().decode())"`. - CHANGELOG fragment: changelog.d/fixed/readme-badges-metadata-audit.md. - Rebase note: added to docs/rebase-notes.md (impact: none, fork-only).
PR #38 (merged 2026-05-28) removed `float_ansnr` from the C backend but cited `Parent ADR-0709` in its body — ADR-0709 is the Phase 4b distributed-platform umbrella and contains zero ANSNR content. PR #295 + PR #324 inherited the bad cite. No dedicated ANSNR-sunset ADR existed in tree. This change: - Authors `docs/adr/0865-ansnr-sunset-pre-vmaf-metric-drop.md` as the missing parent ADR, back-dated to 2026-05-28 (PR #38 merge date) so the dependency chain (ADR-0865 -> PR #38 -> ADR-0749 Python sunset) is consistent. - Documents the historical mis-cite in the new ADR's `## Notes` section so future readers landing on PR #38 can recover the trail. PR bodies on the remote are immutable merge-history and cannot be rewritten. - Adds the index fragment + `_order.txt` row; regenerates `docs/adr/README.md` via `scripts/docs/concat-adr-index.sh`. - Adds `docs/state.md` row (Updated note + Recently-closed entry). - Adds `docs/rebase-notes.md` entry documenting the rebase invariant (upstream still ships `ansnr` extractors; fork must keep deleting). - Adds `changelog.d/changed/ansnr-sunset-adr-authoring.md` fragment. In-tree audit confirmed zero `ADR-0709` references mis-cite ANSNR — all remaining tree-side `ADR-0709` cites correctly point at Phase 4b distributed-platform content. No tree-side citation fix-up required. Docs-only PR. No code changes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
lusoris
force-pushed
the
docs/doc-sweep-bundle-327-330-336-383-337
branch
from
June 3, 2026 11:34
5b423a0 to
b20f814
Compare
There was a problem hiding this comment.
Pull request overview
This PR bundles five docs/metadata/test-only draft PRs into a single doc-sweep against master, primarily improving public API documentation (Doxygen), project metadata (badges/URLs), ADR tracking, and Go unit-test coverage for the fork-added services.
Changes:
- Added/expanded Doxygen documentation for public libvmaf C headers (
feature.h,model.h,dnn.h). - Expanded Go test coverage for
cmd/vmafx-{controller,server,mcp}and tightened repo hygiene (.gitignore, docs state annotations). - Updated docs/metadata: README workflow badges, Rust/Python project URLs, and added ADR-0865 + ADR index updates.
Reviewed changes
Copilot reviewed 30 out of 31 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/vmaf-tune/pyproject.toml | Adds [project.urls] metadata for the vmaf-tune tool |
| tools/vmaf-roi-score/pyproject.toml | Adds [project.urls] metadata for the ROI scoring tool |
| tools/ensemble-training-kit/pyproject.toml | Adds [project.urls] metadata for the ensemble training kit |
| README.md | Adds Rust CI and Go CI workflow badges |
| pyproject.toml | Adds top-level [project.urls] metadata |
| mcp-server/vmaf-mcp/pyproject.toml | Adds [project.urls] metadata for the Python MCP server |
| docs/state.md | Updates the state log with coverage/row-sweep/ADR notes and row changes |
| docs/rebase-notes.md | Adds/updates rebase notes entries for bundled changes |
| docs/adr/README.md | Regenerated ADR index table; adds ADR-0865 |
| docs/adr/0865-ansnr-sunset-pre-vmaf-metric-drop.md | New ADR documenting ANSNR sunset decision |
| docs/adr/_index_fragments/0865-ansnr-sunset-pre-vmaf-metric-drop.md | New ADR index fragment for ADR-0865 |
| docs/adr/_index_fragments/_order.txt | Adds ADR-0865 fragment to ordering |
| dev-llm/pyproject.toml | Adds [project.urls] metadata for dev-llm tooling |
| core/src/feature/rust/tad/Cargo.toml | Switches crate metadata to inherit from [workspace.package] |
| core/include/libvmaf/model.h | Adds Doxygen docs for model flags/config and model/collection APIs |
| core/include/libvmaf/feature.h | Adds Doxygen docs for feature dictionary APIs |
| core/include/libvmaf/dnn.h | Expands doc comment for vmaf_dnn_session_close |
| cmd/vmafx-server/main_extra_test.go | Adds HTTP error-path + lifecycle tests for server |
| cmd/vmafx-mcp/impl_test.go | Adds unit tests for MCP helpers and handler error paths |
| cmd/vmafx-controller/nodes/registry_edge_test.go | Adds edge-case + concurrency tests for node registry |
| cmd/vmafx-controller/main_extra_test.go | Adds HTTP error-path + lifecycle tests for controller |
| changelog.d/fixed/readme-badges-metadata-audit.md | Changelog fragment for README/badges/metadata sweep |
| changelog.d/changed/state-md-closed-pr-row-sweep.md | Changelog fragment for docs/state.md row sweep |
| changelog.d/changed/bundle-doc-sweep-batch1.md | Changelog fragment for the overall bundle |
| changelog.d/changed/ansnr-sunset-adr-authoring.md | Changelog fragment for ADR-0865 authoring |
| changelog.d/added/libvmaf-public-header-doc-comments-round2.md | Changelog fragment for Doxygen round-2 |
| changelog.d/added/go-controller-mcp-coverage.md | Changelog fragment for Go coverage expansion |
| Cargo.toml | Adds [workspace.package] shared metadata |
| bindings/rust/vmafx-sys/Cargo.toml | Switches crate metadata to inherit from workspace |
| ai/pyproject.toml | Adds [project.urls] metadata for AI project |
| .gitignore | Anchors Go binary ignore patterns to repo root and expands list |
Comments suppressed due to low confidence (3)
docs/rebase-notes.md:1182
- This line looks like an accidental truncation (it currently starts with "/binary symbol renames"). It reads like it should be the continuation of the preceding sentence ("No source/binary symbol renames...") and the leading slash breaks the Markdown rendering/meaning.
docs/adr/README.md:476 - This section of the ADR index contains duplicated/corrupted rows (ADR-0457/0458/0459 are repeated, and ADR-0460 appears twice, once with an empty tags/date column). This breaks the Markdown table and makes the ADR index ambiguous.
| [ADR-0464](0464-cambi-cuda-smem-tile.md) | CAMBI CUDA spatial-mask shared-memory tile | Accepted | cuda, gpu, cambi, performance, kernel, fork-local |
| [ADR-0466](0466-mkdocs-strict-pre-push-hook.md) | mkdocs strict-mode pre-push hook | Accepted | docs, ci, git, hooks, mkdocs |
| [ADR-0467](0467-ssimulacra2-avx512-neon-ulp-audit.md) | SSIMULACRA2 AVX-512 + NEON IIR blur / `picture_to_linear_rgb` ULP audit — clean close | simd, ssimulacra2, audit | Accepted |
| [ADR-0468](0468-hip-float-adm-real-kernel.md) | HIP float_adm real kernel (ninth HIP consumer) | Accepted | hip, build, feature-extractor |
| [ADR-0469](0469-float-psnr-hip-enable-chroma.md) | `float_psnr` HIP twin — wire `enable_chroma` option | Accepted | hip, psnr, option-parity |
docs/adr/README.md:1043
- The ADR table row for ADR-0772 and ADR-0809 appears to have been concatenated into a single Markdown table line (there is a "||" mid-row). This will render incorrectly and breaks ADR indexing/search.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ts := httptest.NewServer(mux) | ||
| defer ts.Close() | ||
|
|
||
| reqBody := `{"reference":"/tmp/ref.yuv","distorted":"/tmp/dis.yuv"}` |
| ts := httptest.NewServer(mux) | ||
| defer ts.Close() | ||
|
|
||
| reqBody := `{"reference":"/tmp/ref.yuv","distorted":"/tmp/dis.yuv"}` |
Comment on lines
+317
to
+324
| // Accept a small float epsilon for half-up edge case. | ||
| diff := got - tt.want | ||
| if diff < -1e-9 || diff > 1e-9 { | ||
| // Half-up against IEEE-754 representation is fragile; only fail on bigger drift. | ||
| if (got-tt.want) > 0.5 || (tt.want-got) > 0.5 { | ||
| t.Errorf("roundF(%v, %d): got %v, want ~%v", tt.in, tt.dec, got, tt.want) | ||
| } | ||
| } |
Comment on lines
+444
to
+449
| func TestHandleVmafScoreMissingDimensions(t *testing.T) { | ||
| t.Parallel() | ||
| // Use a guaranteed-unreadable path to exercise the ref-validation error | ||
| // before any dimension check; this confirms that the handler surfaces | ||
| // ValidatePath failures rather than panicking. | ||
| _, err := handleVmafScore(context.Background(), map[string]any{ |
Comment on lines
+37
to
+43
| * Ownership transfer rules: | ||
| * - On success of @ref vmaf_use_feature / @ref vmaf_model_feature_overload, | ||
| * ownership of the dictionary passes to the VmafContext / VmafModel and | ||
| * the caller MUST NOT free it. | ||
| * - On failure of those calls (non-zero return), the caller still owns the | ||
| * dictionary and is responsible for releasing it with | ||
| * @ref vmaf_feature_dictionary_free. |
Comment on lines
+89
to
+93
| * @field name Optional display name for the loaded model. When NULL the | ||
| * loader uses the default `"vmaf"` for built-in models and the | ||
| * model file's `model_name` field for path-loaded models. | ||
| * When non-NULL the string is copied; the caller retains | ||
| * ownership and may free it after the load call returns. |
Comment on lines
+81
to
+90
| * Safe to call on a NULL @p dict or with `*dict == NULL`; both are no-ops. | ||
| * On success @p *dict is set to NULL so the handle cannot be reused. | ||
| * | ||
| * Only call this on dictionaries the caller still owns - once ownership has | ||
| * passed to a VmafContext via @ref vmaf_use_feature, or to a VmafModel via | ||
| * @ref vmaf_model_feature_overload, calling this is a double-free. See the | ||
| * `VmafFeatureDictionary` ownership-transfer rules above. | ||
| * | ||
| * @param dict In/out: address of the dictionary handle to free. NULL is a | ||
| * no-op; on a non-NULL @p dict, @p *dict is reset to NULL. |
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
Bundle of 5 open DRAFT PRs (#327, #330, #336, #383, #337) into a single DRAFT against master
tip
40d192ef1. All source PRs are docs-only, metadata, or test-only with no behavior changes.Conflicts resolved conservatively: HEAD wins for include-guard renames (master already adopted
the non-reserved form); additive doc sections unioned.
Source PRs in application order:
cmd/vmafx-{controller,server,mcp}docs/state.mdclosed-PR row sweep (reconcile 2 rows citing CLOSED PRs fix(cuda): resolve conflict markers in filter1d resolution dispatch (ADR-0753) #214, refactor(core): cpp23 read_json_model.c → .cpp (ADR-0846) #215)docs/adr/README.mdType
docs— documentation onlytest— test-onlybuild/ci— tooling / infraChecklist
make format && make lintis green locally.meson test -C build./cross-backend-diffand the worst ULP is ≤ 2..c/.cpp/.cu/.h/.hpp, it has the appropriate license header (seeCONTRIBUTING.md).!orBREAKING CHANGE:and the migration path is documented below.docs/adr/_index_fragments/<NNNN-slug>.mdand the slug is appended todocs/adr/_index_fragments/_order.txt— do not editdocs/adr/README.mddirectly (regenerated byscripts/docs/concat-adr-index.sh; see ADR-0221).Bug-status hygiene (ADR-0165)
docs/state.mdupdated in this PR with a rowin the appropriate section (Open / Recently closed / Confirmed
not-affected / Deferred).
Netflix golden-data gate (ADR-0024)
assertAlmostEqual(...)score in the Netflix golden Python tests.Cross-backend numerical results
No code changes — docs, tests, and metadata only. No backend or feature extractor code was modified.
Deep-dive deliverables (ADR-0108)
AGENTS.mdinvariant note — no rebase-sensitive invariants: all touched files are docs, Go tests, Rust/Python metadata, or changelog fragments; no rebase-sensitive C or SIMD paths modified.changelog.d/changed/bundle-doc-sweep-batch1.mddocs/rebase-notes.mdupdated (sections from all 5 source PRs preserved via union merge).Reproducer
Known follow-ups
Source PRs #327, #330, #336, #383, #337 will be closed as superseded upon merge.
No SIMD/GPU twins were touched. No C production code was modified.
Breaking changes / migration
None.