Skip to content

feat(collector): committed provenance, authored reuse, changed-op manifest, fail-closed audit (AIC-1502)#1372

Open
Arsene12358 wants to merge 18 commits into
yimingl/aic-1501-family-layout-reorgfrom
yimingl/aic-1502-provenance-reuse
Open

feat(collector): committed provenance, authored reuse, changed-op manifest, fail-closed audit (AIC-1502)#1372
Arsene12358 wants to merge 18 commits into
yimingl/aic-1501-family-layout-reorgfrom
yimingl/aic-1502-provenance-reuse

Conversation

@Arsene12358

Copy link
Copy Markdown
Contributor

PR 3 of 4 of the Collector V3: op-centric collector management project (Linear: AIC-1502), implementing design §5, §6.3, and §8. Stacked on #1371 (base yimingl/aic-1501-family-layout-reorg); retargets when the stack merges. This PR declares an approved cross-module contract change per .claude/rules/collector/layer_permissions.md: collector finalize + aic-core loader + tools + one new workflow, coordinated.

What this does

Every table in the perf tree now carries committed provenance, reuse is authored instead of implied, and change tracking + enforcement are wired into CI:

  • collection_meta.yaml (design §5): collector finalize writes it flat beside staged parquet — producing runtime (framework/version/image/digest from the manifest), per-table collector_ref (repo SHA), collector_hash (content hash over an authored closure: the op's module + a fixed shared core + its case files, collector/hash_closures.yaml, 34/34 registry modules covered by a two-direction CI-riding gate), case_plan_hash (attested from the run's checkpoint case ids), rows, status: complete|partial (derived from checkpoint failures). The external promotion step carries the sidecar with the parquet; the CI audit makes forgetting it impossible.
  • reuse.yaml (design §6.3): replaces SHARED_LAYER_REUSE.txt. The tree conversion (via the reviewed tools/perf_database/migrate_markers.py, plan/execute/verify, idempotent): 255 markers → per-table declarations with derived effective donors (newest sibling under the legacy ordering — behavior-preserving; every reason field discloses the mechanical derivation), 27 comm-family markers dropped without declarations (design §6.5 rule 5 — comm never participates in sibling reuse), 7 INCOMPLETE.txtstatus: partial sidecars, and 527 pre-V3 dirs backfilled with provenance: legacy sidecars (design §5 legacy tier, amended this PR) so the sidecar-coverage gate is total over the tree. Zero legacy markers remain. Discovery parity held through every data commit: 10 systems / 89 (backend, version) pairs, byte-identical.
  • tools/perf_database/changed_ops.py (design §8): diffs two revisions per (framework, family) on three GPU-free signals — manifest pin, closure hash (revision-pure: even SHARED_CORE is ast-read from each revision), case-input files — emitting the locked manifest schema consumed by the evidence resolver (PR 4), the CI gate (AIC-1214), and the support-matrix healer. Fail-closed: unknown ops, VersionRoute shapes (unsupported → raise), and pre-V3 baselines (dedicated exit code 3, mapped to a neutral skip in CI) never pass silently.
  • tools/perf_database/audit_collector_data.py + .github/workflows/collector-audit.yml: six fail-closed rules on every PR touching data/collector/tools — sidecar coverage, reuse validity (donors must exist with real data, same backend), comm exclusion, family placement, manifest resolution, no legacy markers. All six pass on the real tree as committed. Evidence gating against the changed-ops manifest remains AIC-1214 scope (the workflow uploads it as an artifact).
  • Loader + 7 consumers read the structured files yaml-first with legacy .txt fallback for one transition window (once-per-tree deprecation warnings), including a 4-way predicate-parity test pinning the one deliberate divergence (hot-path leniency in operations/base.py).

⚠️ Maintainer sign-off requested: runtime gate extension

Populating provenance required capturing the resolved CollectorRuntime in collect_vllm/collect_trtllm — which means require_collector_runtime's exact-version fail-closed gate now applies to vLLM and TRT-LLM runs (previously sglang-only), including the vLLM XPU path which was fully ungated before. A container whose installed framework version differs from the manifest pin now hard-fails with instructions to use the pinned image. This is design §4's uniform enforcement and is zero-behavior-change against the committed manifest, but it is a real new failure path for out-of-pin containers: please confirm explicitly in review.

Migration notes for reviewers

  • 12 reuse.yaml files (17 entries), all under l40s, declare tables that also exist as parquet in the declaring dir (identification predicate: self-overlap between declared table and local data). Their original marker text asserted channel-3 (cross-framework) intent; the conversion is by marker type, per the migration algorithm. Inert in this PR (declarations are not yet consumed for ordering) and safe under §6.1 (primary always wins), but PR 4 must re-derive or drop these before its ordering flip treats declarations as reviewed evidence — they deviate from §6.3's "a declared-reuse dir contains no parquet of its own".
  • The 527-dir status: complete backfill makes explicit what the absence of INCOMPLETE.txt already meant implicitly; every backfilled sidecar is honestly tagged provenance: legacy (no fabricated hashes). Strict mode (PR 4) treats legacy as warn-not-fail for one release.
  • Known gaps, documented in-code: XPU registries are invisible to provenance/change tracking (AIC-1509 filed); VersionRoute registry entries unsupported by the manifest tool (raises).

Test plan

uv run pytest tests/unit -m unit -n 2 green (collector suite 469, tools 221, sdk incl. 22 discovery + 8 parity tests; the finalize-glue writer has dedicated coverage incl. the legacy-tier merge guard) · real-tree audit: all six rules OK, exit 0 · changed_ops --base HEAD --head HEAD: 30 pairs unchanged; --base <pre-V3>: exit 3 with the documented message · discovery parity 10/89 unchanged across all four data commits · every fix wave re-reviewed.

🤖 Generated with Claude Code

@Arsene12358 Arsene12358 requested review from a team as code owners July 15, 2026 22:39
@copy-pr-bot

copy-pr-bot Bot commented Jul 15, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e5228ed4-7994-4652-8003-55d30193bfc7

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • ✅ Review completed - (🔄 Check again to review again)

Comment @coderabbitai help to get the list of available commands.

@Arsene12358

Copy link
Copy Markdown
Contributor Author

Merge protocol (stacked PR): please do not merge this PR until its base PR has merged to main and GitHub has auto-retargeted this one to main (the repo's delete-branch-on-merge triggers that automatically). Merging it as-is would fold this PR into its base PR's branch instead of main. After each upstream merge I'll rebase this branch onto main (squash-merge cascade) — the diff you review is unaffected. Merge order: #1370#1371#1372.

@Arsene12358 Arsene12358 force-pushed the yimingl/aic-1502-provenance-reuse branch from 0d3375e to 2b73e49 Compare July 16, 2026 03:35
@Arsene12358

Copy link
Copy Markdown
Contributor Author

CI fix pushed (branch rebased onto the updated #1371 base, new head 2b73e491): in addition to the base-branch fixes, this branch adds src/aiconfigurator_core/systems/data/**/*.yaml to the core wheel's maturin includes — the reuse.yaml/collection_meta.yaml sidecars this PR introduces were not being packaged, which would have made installed-wheel behavior silently diverge from repo behavior. The wheel-layer check now probes a sidecar and a reuse declaration explicitly.

@Arsene12358 Arsene12358 changed the title feat(collector,sdk,tools,ci): committed provenance, authored reuse, changed-op manifest, fail-closed audit (AIC-1502) feat(collector): committed provenance, authored reuse, changed-op manifest, fail-closed audit (AIC-1502) Jul 16, 2026
@github-actions github-actions Bot added the feat label Jul 16, 2026
@Arsene12358 Arsene12358 force-pushed the yimingl/aic-1502-provenance-reuse branch from 2b73e49 to cf0dc28 Compare July 16, 2026 06:17
@Arsene12358

Copy link
Copy Markdown
Contributor Author

CI fixes pushed (head cf0dc28e, rebased onto the updated #1371 branch — cascade per the stack protocol):

  • Validate PR title: retitled to single-scope feat(collector) (the title-check action cannot parse comma-separated scopes; see refactor(sdk): family-first perf-data layout with dual-read transition (AIC-1501) #1371 comment for details).
  • copyright-checks: the check covers .yaml and flagged exactly the 762 sidecars this PR introduces (534 collection_meta.yaml + 228 reuse.yaml). All 762 now carry the repo-standard SPDX header (YAML comments are invisible to yaml.safe_load, so loader/audit parsing is byte-for-byte unchanged — real-tree audit still 6/6 OK), and both writers emit the header going forward so quarterly collections and backfill re-runs stay compliant: collector/provenance.py::write_collection_meta and the tools/perf_database/migrate_markers.py renderers. Renderer tests pin the header with startswith assertions (the yaml round-trips are comment-blind).
  • Build and Test (unit): inherits the git-less-container test fix from refactor(sdk): family-first perf-data layout with dual-read transition (AIC-1501) #1371.

@Arsene12358 Arsene12358 force-pushed the yimingl/aic-1502-provenance-reuse branch from cf0dc28 to b0eb31f Compare July 16, 2026 06:31
@Arsene12358

Copy link
Copy Markdown
Contributor Author

Round 2 (head b0eb31f7, cascade rebase onto #1371): the remaining unit failure was test_changed_ops.py erroring on the git-less test container (FileNotFoundError: 'git' — the tool diffs real git revisions, so its tests legitimately need a git binary; test_migrate_markers.py's execute-path tests were next in line). Fixed at the root on #1371: the test Docker stage now installs git. No changes to this PR's own commits beyond the rebase.

@Arsene12358

Copy link
Copy Markdown
Contributor Author

Round 3 (head 18e70d29): with git in the test image, 2211 tests passed and one remained — test_real_repo_base_equals_head_is_all_unchanged, the real-repo integration smoke test, fails in the container because /workspace is a source COPY without .git (git rev-parse HEAD → exit 128). That one test is now gated on the checkout actually being a git worktree; the 27 synthetic-repo changed_ops tests all run in CI. Verified locally both ways (28 passed in a real checkout; the probe returns skip in a git-less tree).

… closures (AIC-1502)

Signed-off-by: Yiming Liu <yimingl@nvidia.com>
…fallback (AIC-1502)

Teach the loader and all marker consumers reuse.yaml / collection_meta.yaml
(design §5/§6.3), yaml-first with SHARED_LAYER_REUSE.txt / INCOMPLETE.txt
kept as a one-transition-window fallback. New shared
perf_database._version_dir_state() consolidates declared-reuse and partial
detection with a one-time-per-data_dir deprecation warning when a legacy
marker is honored; every discovery site (get_supported_databases,
is_shared_layer_marker_only_version, get_database, database-ref discovery,
resolve_op_data_path) is rewired through it or a deliberately duplicated
local equivalent where importing aic-core isn't appropriate
(operations/base.py, and the standalone tools grid.py, create_charts.py,
audit_kernel_source.py). reuse.yaml entries are validated (raise ValueError
on malformed) and treated as declared-marker-equivalent only; ordering
semantics are PR 4 scope.

Signed-off-by: Yiming Liu <yimingl@nvidia.com>
…, orphan cleanup (AIC-1502)

Signed-off-by: Yiming Liu <yimingl@nvidia.com>
Adds migrate_markers.py (plan/execute/verify, modeled on
migrate_family_layout.py): converts SHARED_LAYER_REUSE.txt into a
per-table reuse.yaml (from_version = newest sibling version holding
that table, matching today's newest-first effective donor) and
INCOMPLETE.txt into a synthesized provenance:legacy collection_meta.yaml
sidecar (every own table marked status: partial), in place, no
cross-family replication. Fails closed (lists offenders) when a
SHARED marker has no donor table anywhere, or an INCOMPLETE marker has
no data of its own. --verify round-trips every generated file through
the real loader parser (aiconfigurator_core.sdk.perf_database).

Signed-off-by: Yiming Liu <yimingl@nvidia.com>
…eta.yaml (AIC-1502)

Runs tools/perf_database/migrate_markers.py --execute over the real
perf-data tree: 255 SHARED_LAYER_REUSE.txt -> reuse.yaml (per-table
donor entries, from_version = newest sibling version holding that
table) and 7 INCOMPLETE.txt -> collection_meta.yaml (provenance:
legacy, every own table status: partial). Zero legacy .txt markers
remain; --verify passes; discovery parity confirmed
(get_supported_databases: unchanged 10 systems / 89 versions, zero
legacy-marker deprecation warnings on the migrated tree).

Signed-off-by: Yiming Liu <yimingl@nvidia.com>
…le 5 (AIC-1502)

migrate_markers.py converted SHARED_LAYER_REUSE.txt markers into reuse.yaml
declarations uniformly, but design §6.5 rule 5 excludes the comm family from
sibling-version reuse entirely (NCCL/oneCCL curves are topology-bound). A
comm reuse.yaml is a standing contradiction of that rule that PR 4's loader
and CI audit must never consume.

A SHARED_LAYER_REUSE.txt marker inside a <system>/comm/<backend>/<version>
dir is now deleted with no reuse.yaml emitted, logging the exclusion instead
of running donor lookup. Non-comm families are unaffected.

Signed-off-by: Yiming Liu <yimingl@nvidia.com>
migrate_markers.py emitted these before the comm-family exclusion (design
§6.5 rule 5) was enforced in the tool. Sibling-version reuse is invalid for
comm entirely (NCCL/oneCCL curves are topology-bound), so these
declarations were contradictions PR 4's loader/CI audit must never see.

Discovery parity unaffected: get_supported_databases() still reports
10 systems / 89 (backend, version) pairs -- every affected (backend,
version) remains declared via its non-comm families.

Signed-off-by: Yiming Liu <yimingl@nvidia.com>
Diffs two git revisions per (framework, family) and emits design §8's
locked output schema (changed: [{framework, family, reasons, tables,
systems, action: recollect}], unchanged: [{framework, family, tables,
systems}]) — the single input for the evidence resolver (PR 4), the CI
gate (AIC-1214), and the out-of-repo support-matrix healer.

Three GPU-free signals per pair: pin_version (manifest v2 resolution),
collector_code (revision-aware collector_hash mirroring
collector.provenance's construction), case_plan (content hash of the
family's case-input files). Everything is read via `git show`/`git
ls-tree`; registries are parsed with `ast`, never imported/exec'd.
Registry paths are derived from each revision's own manifest content
(collector_dir / convention), so the tool is revision-safe and
independently testable without replicating all five real registries.

Reuses collector.provenance's SHARED_CORE/MODEL_CASES_GROUP constants;
imitates (does not import) parquet_diff.py's git plumbing, per the
"keep changed_ops standalone" constraint.

Signed-off-by: Yiming Liu <yimingl@nvidia.com>
…-V3 baseline exit, xpu gap docs (AIC-1502)

Signed-off-by: Yiming Liu <yimingl@nvidia.com>
…-1502)

migrate_markers.py's existing rules only converted the two legacy .txt
markers (SHARED_LAYER_REUSE.txt, INCOMPLETE.txt); a version dir that never
had EITHER marker -- i.e. pre-V3 data that was always served as complete --
got no sidecar at all. That left 527 of 534 parquet-holding dirs on the
real tree failing R1 sidecar coverage in the new collector-audit CI gate.

Adds a third migration rule: any parquet-holding dir with no
collection_meta.yaml at all gets a synthesized provenance: legacy sidecar
with every held table marked status: complete (not partial -- these dirs
were never incomplete). Skips dirs that already carry a sidecar (including
the 7 INCOMPLETE-derived ones, which keep status: partial) and dirs with no
parquet of their own (declared-reuse-only dirs -- nothing to describe).
plan/execute/verify all support the new rule; verify's coverage check now
mirrors audit_collector_data.py's R1 rule directly, superseding the old
"legacy implies >=1 partial table" invariant that the backfill's
complete-only sidecars intentionally break.

-- tools/perf_database/migrate_markers.py
-- tests/unit/tools/test_migrate_markers.py

Signed-off-by: Yiming Liu <yimingl@nvidia.com>
Generated by migrate_markers.py's new backfill rule (previous commit):
every pre-V3 family version dir that held >=1 parquet table but had never
carried an INCOMPLETE.txt marker (and so was skipped by the earlier
marker-to-yaml migration) gets a provenance: legacy collection_meta.yaml
with every held table marked status: complete.

Closes the gap the collector-audit CI gate's R1 rule (next commit) would
otherwise fail on: 527 of 534 parquet-holding dirs had no sidecar at all.
The 7 dirs already migrated from INCOMPLETE.txt (status: partial) are
untouched. Discovery parity unaffected: get_supported_databases() still
reports 10 systems / 89 (backend, version) pairs.

Signed-off-by: Yiming Liu <yimingl@nvidia.com>
The primary enforcement surface of Collector V3 design §8: six rules over
the committed data tree, each named after the design section it enforces
(R1 sidecar coverage, R2 reuse validity, R3 comm exclusion, R4 family
placement, R5 identity/manifest v2 resolution, R6 no legacy markers). Exits
non-zero on ANY failure and prints every failure found, never stopping at
the first. Wired into CI via collector-audit.yml on every PR touching
data/, collector/, or the manifest.

This was blocked pending the previous two commits: R1 requires a
collection_meta.yaml on every parquet-holding dir, but 527 of 534 such
dirs on the real tree had no sidecar at all (pre-V3 data that predates
even the INCOMPLETE.txt marker convention). The backfill migration and its
527-file execution close that gap; with the tree now fully covered, all
six rules pass real-tree-as-is with exit 0.

-- tools/perf_database/audit_collector_data.py
-- tests/unit/tools/test_audit_collector_data.py
-- .github/workflows/collector-audit.yml

Signed-off-by: Yiming Liu <yimingl@nvidia.com>
Arsene12358 and others added 6 commits July 16, 2026 16:16
… provenance tier (AIC-1502)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Yiming Liu <yimingl@nvidia.com>
…age, legacy-tier merge guard, no-LFS workflow (AIC-1502)

Signed-off-by: Yiming Liu <yimingl@nvidia.com>
…t in layer checks (AIC-1502)

Signed-off-by: Yiming Liu <yimingl@nvidia.com>
The copyright CI check covers .yaml and flagged all 762 committed
collection_meta.yaml/reuse.yaml sidecars. Prepend the repo-standard header
(YAML comments are invisible to yaml.safe_load, so loader/audit parsing is
unchanged — real-tree audit still 6/6 OK), and make both writers emit it so
future collections and re-runs stay compliant:

- collector/provenance.py write_collection_meta (quarterly collections)
- tools/perf_database/migrate_markers.py renderers (one-shot backfill tool)

Header year is the year of emission, matching the check's last-commit-year
rule.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Yiming Liu <yimingl@nvidia.com>
Review follow-up: the renderer round-trips parse via yaml.safe_load, which
is comment-blind — a reverted header prepend would only surface at the next
backfill emission. Pin it with startswith assertions on both renderer paths.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Yiming Liu <yimingl@nvidia.com>
The CI test image COPYs sources into /workspace without .git, so
'git rev-parse HEAD' exits 128 there. Gate the one real-repo integration
test on the checkout actually being a git worktree; the 27 synthetic-repo
tests are unaffected (they git-init their own repos).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Yiming Liu <yimingl@nvidia.com>
@Arsene12358 Arsene12358 force-pushed the yimingl/aic-1501-family-layout-reorg branch from 168d7f6 to 163813b Compare July 16, 2026 08:28
@Arsene12358 Arsene12358 requested a review from a team as a code owner July 16, 2026 08:28
@Arsene12358 Arsene12358 requested review from AliceChenyy and Harrilee and removed request for a team July 16, 2026 08:28
@Arsene12358 Arsene12358 force-pushed the yimingl/aic-1502-provenance-reuse branch from 18e70d2 to d641c8e Compare July 16, 2026 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant