feat(collector): Collector V3 design + manifest v2 per-op runtime pins (AIC-1217, AIC-1500)#1370
feat(collector): Collector V3 design + manifest v2 per-op runtime pins (AIC-1217, AIC-1500)#1370Arsene12358 wants to merge 9 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (6)
WalkthroughThe collector now uses schema v2 manifests and an op-family catalog to resolve workload-specific runtimes, enforce digest and version pinning, validate registry operations, and document the Collector V3 data and provenance model. ChangesCollector V3 runtime management
Estimated code review effort: 4 (Complex) | ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@collector/framework_manifest.py`:
- Around line 241-249: Update validate_resolution’s registry-module loop so
_registry_entries(framework_key) is executed within the existing
exception-handling scope. Catch the missing/renamed module error alongside the
current KeyError, LookupError, and ValueError cases, append a
framework-qualified validation error string, and continue validating remaining
registry modules instead of propagating the exception.
- Around line 183-199: Update the resolution logic around _registry_entries()
and validate_resolution() so every requested operation must match a registry
entry; if any requested op is unknown, fail closed with a per-operation
validation error instead of resolving the container. Catch ImportError and
ModuleNotFoundError from registry loading in validate_resolution() and convert
them into the same per-op error result, while preserving successful resolution
for valid operations.
In `@collector/README.md`:
- Around line 172-174: The documentation must consistently describe the digest
rule as applying to every image reference containing “/”, with bare internal
names without “/” exempt. In collector/README.md lines 172-174, replace the
“public registry path” wording accordingly; in
docs/perf_database/collector-v3-op-centric-design.md lines 151-156, remove the
registry-support qualification and document the same bare-name exemption.
In `@docs/perf_database/collector-v3-op-centric-design.md`:
- Line 41: Add the text language identifier to the three fenced code blocks in
the documentation examples, including the identity chain, filesystem layout, and
source-order blocks referenced by the review. Ensure each opening fence uses
text so the markdownlint MD040 violations are resolved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 207e2e43-bfb2-43b7-b37a-ce66eb961f77
📒 Files selected for processing (8)
collector/README.mdcollector/framework_manifest.pycollector/framework_manifest.yamlcollector/op_catalog.pydocs/perf_database/collector-v3-op-centric-design.mdtests/unit/collector/test_framework_manifest.pytests/unit/collector/test_manifest_resolution.pytests/unit/collector/test_op_catalog.py
📜 Review details
⏰ Context from checks skipped due to timeout. (6)
- GitHub Check: Collect snapshot (old)
- GitHub Check: Collect snapshot (new)
- GitHub Check: Build and Test (unit)
- GitHub Check: Build and Test (e2e)
- GitHub Check: Rust/Python engine-step parity
- GitHub Check: Cargo Deny
🧰 Additional context used
📓 Path-based instructions (12)
{collector/**,tests/unit/collector/**}
📄 CodeRabbit inference engine (.claude/rules/collector/failure_handling.md)
{collector/**,tests/unit/collector/**}: In collector failure handling, observe failures rather than predict them: do not use a declarative expected-failure layer or automatic skips; failing groups must be fixed.
Record every worker failure automatically in the relevant error and collection-summary JSON files with classification, case parameters, exception details, and model/dtype grouping.
Reset a worker after CUDA-fatal errors only after recording the failed task.
Treat missing data points as tolerable downstream because the SDK can interpolate, extrapolate, reuse sibling-version rows, or use HYBRID empirical estimates.
For a hanging case or one that kills the node, add a dated denylist.yaml entry with a reason.
For an entirely unvalidated operation/backend pair, mark the registry OpEntry as unverified=true.
For cases not validated on a specific SM, mark the registry OpEntry with unverified_sms=(sm,).
Represent physically impossible hardware capabilities with a positive floor in capabilities.yaml; do not use it for framework-version kernel gaps.
Treat out-of-memory failures as unclassified until reproduced on a clean GPU; only a genuine OOM may be excluded by the sanctioned generation-time memory filter.
Fix proven collector-code bugs in code rather than using skips, and re-check dispatch and skip rules.
Do not encode framework-version gaps in YAML; allow them to fail and be re-tested after version changes.
Treat isolated, explained, unclustered failures as acceptable; investigate around 10% unexpected failures or sooner when failures cluster.
Treat roughly one-third failing cases or an entire failing family as a systemic collector problem; stop collection and fix the collector.
Never hide failures with broad skips, retries, generic OOM labels, reduced coverage, synthetic rows, or weakened benchmarks.
Compare failure records with the previous run for the same backend and version; prioritize only new failure groups.
Do not mechanically translate failure records in...
Files:
collector/framework_manifest.yamlcollector/op_catalog.pytests/unit/collector/test_op_catalog.pytests/unit/collector/test_manifest_resolution.pycollector/README.mdtests/unit/collector/test_framework_manifest.pycollector/framework_manifest.py
collector/**
📄 CodeRabbit inference engine (.claude/rules/generator/cross_module_impact.md)
Review the collector when generated configuration formats change or when generator parameter names used in benchmark configurations change.
collector/**: Keep collector-layer rules in their permitted locations: base operation YAML for sweep parameters and cited quant-mode SM limits; model case YAML for structural shapes and policies; capabilities YAML for positive hardware dtype/op minimum-SM floors; backend registries for version routes and unverified markers; collectors for dispatch, classified runtime errors, and memory-feasibility generation filters; and the denylist only for hangs or node-killing cases.
Do not silently skip queued cases in collector code. A queued case must be executed or produce a classified exception; only generation-time capability floors, registry unverified markers, and the hang denylist may prevent execution.
Use framework-selected kernel/backend dispatch for measurements. Prefer the framework builder or selector; manually pin a backend only with a pinned-version file-and-line source citation; record the actually invoked kernel inkernel_source; and re-verify pins andFIXME(kernel-limit)notes on framework version bumps.
Do not invent backend fallbacks or swap backends in exception handlers. If the framework-selected path cannot be constructed or invoked, raise a classified error. Replicate fallback chains only when the framework itself uses them, and record the actual path.
The only sanctioned in-collector filter is memory feasibility: perform it only during generation insideget_*_test_cases(), use size-versus-capacity arithmetic based on live device memory or an approved capacity fallback, never use framework-version or model-name predicates, and log dropped counts in the required format.
Keep framework kernel limits asFIXME(kernel-limit)comments at the owning collector invocation site, including the claimed limit, origin, and unverified status. Do not encode unverified limits in YAML; affected cases should fail at runtime...
Files:
collector/framework_manifest.yamlcollector/op_catalog.pycollector/README.mdcollector/framework_manifest.py
⚙️ CodeRabbit configuration file
collector/**: - Enforce the collector rules from.claude/rules/collector/layer_permissions.md,failure_handling.md, andcase_authoring.md.
- Flag any silent case skip in collector code (a queued case may only execute or raise); the sole sanctioned filter is generation-time memory feasibility with counted drops.
- Flag invented fallbacks on both ends: generation must raise on unresolvable declarations (never substitute defaults or another model's geometry); collectors must never swap in a different backend/kernel than the framework's own dispatch selects — manual pins require framework source citations.
- Flag any reintroduction of selector/exception machinery (case_ids/contains/indices/ranges/limit/rules, sm_exceptions-style shape or version predicates) in YAML or code.
- Capability floors (
cases/capabilities.yaml) may hold hardware facts only: no shapes, no framework versions, no per-backend nesting.cases/denylist.yamlis for hang/node-killers only, dated.- Collector changes must stay within
collector/andtests/unit/collector/; flag producer+consumer contract changes (perf row schema, PerfFile names) unless the PR explicitly declares them.- Check collector changes for backend/runtime version accuracy, GPU resource assumptions, reproducible command construction, and clear failure evidence.
- Flag changes that make support-matrix or perf-data results harder to trace back to the command, model, system, quantization, or runtime version that produced them.
Files:
collector/framework_manifest.yamlcollector/op_catalog.pycollector/README.mdcollector/framework_manifest.py
**/*
📄 CodeRabbit inference engine (.claude/rules/repo-guide.md)
**/*: Treat this file as the only always-injected rule file; do not add new always-on rules here without human approval. New rule files must includepaths:frontmatter.
When reviewing changes in a governed area, read that area's rule files first, even if path-based auto-loading does not occur during a read-only review. Rule violations are review findings even when the code works.
A task must remain within its module: collector tasks may change onlycollector/and its tests, generator tasks onlysrc/aiconfigurator/generator/and its tests, and SDK tasks must not modify either. Cross-module contract changes require explicit human approval.
Files:
collector/framework_manifest.yamlcollector/op_catalog.pytests/unit/collector/test_op_catalog.pytests/unit/collector/test_manifest_resolution.pydocs/perf_database/collector-v3-op-centric-design.mdcollector/README.mdtests/unit/collector/test_framework_manifest.pycollector/framework_manifest.py
⚙️ CodeRabbit configuration file
**/*: - Prefer applicable inline comments. When the correct fix is clear, small, and limited to the commented diff hunk, include it as a GitHub Suggested Change so the author can apply it with one click.
- Do not use a suggested change when the fix requires broader design choices, multiple files, generated artifacts, unavailable context, or validation that cannot be inferred from the diff.
- If a comment is not directly applicable, state the smallest concrete next step and why a one-click suggestion is not safe.
Files:
collector/framework_manifest.yamlcollector/op_catalog.pytests/unit/collector/test_op_catalog.pytests/unit/collector/test_manifest_resolution.pydocs/perf_database/collector-v3-op-centric-design.mdcollector/README.mdtests/unit/collector/test_framework_manifest.pycollector/framework_manifest.py
collector/**/*
📄 CodeRabbit inference engine (.claude/rules/repo-guide.md)
Changes under
collector/must follow.claude/rules/collector/layer_permissions.mdandfailure_handling.md; case YAML work must also followcase_authoring.md. Do not apply generator rules tocollector/case_generator.py.
Files:
collector/framework_manifest.yamlcollector/op_catalog.pycollector/README.mdcollector/framework_manifest.py
collector/**/*.{yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Before modifying case YAML files under
collector/**, read.claude/rules/collector/case_authoring.md.
Files:
collector/framework_manifest.yaml
collector/**/*.py
📄 CodeRabbit inference engine (.claude/rules/collector/case_authoring.md)
collector/**/*.py: If a generator lacks support for a required model-correlated field, extend the generator mechanism and obtain human review; do not approximate the behavior with post-generation filtering.
Unresolvable declared inputs must raise; generators must not substitute defaults, another model's geometry, or a close-enough quantization mode. Planned operations that expand to zero cases must have logged drop reasons, otherwise the population bug must be fixed.
Files:
collector/op_catalog.pycollector/framework_manifest.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (AGENTS.md)
Run
ruff check .andruff format --check .to validate Python linting and formatting.
Files:
collector/op_catalog.pytests/unit/collector/test_op_catalog.pytests/unit/collector/test_manifest_resolution.pytests/unit/collector/test_framework_manifest.pycollector/framework_manifest.py
tests/**/*.{py,yaml,txt,sh}
📄 CodeRabbit inference engine (.claude/rules/generator/testing.md)
Use integration tests for the full input-to-artifacts pipeline, comparing output against golden snapshots without external dependencies.
Files:
tests/unit/collector/test_op_catalog.pytests/unit/collector/test_manifest_resolution.pytests/unit/collector/test_framework_manifest.py
tests/unit/collector/**
📄 CodeRabbit inference engine (.claude/rules/collector/layer_permissions.md)
tests/unit/collector/**: Mark every new collector test withpytest.mark.unit; otherwise a test that only works with-m unitis not collected by CI.
Collector tests must preserve the collector data contract, including perf row schema, canonical perf filenames, and parquet finalization format; contract changes require explicit human approval.
Files:
tests/unit/collector/test_op_catalog.pytests/unit/collector/test_manifest_resolution.pytests/unit/collector/test_framework_manifest.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Run unit tests with
pytest -m unit; usepytest -m "unit or build"for the build-test subset when required data is available.
Files:
tests/unit/collector/test_op_catalog.pytests/unit/collector/test_manifest_resolution.pytests/unit/collector/test_framework_manifest.py
tests/**
⚙️ CodeRabbit configuration file
tests/**: - Check that tests cover the changed behavior rather than only the happy path.
- Watch for fixtures or golden outputs that mask backend drift, support-matrix ordering changes, or CLI output regressions.
Files:
tests/unit/collector/test_op_catalog.pytests/unit/collector/test_manifest_resolution.pytests/unit/collector/test_framework_manifest.py
docs/**
⚙️ CodeRabbit configuration file
docs/**: - Check that docs match changed CLI, SDK, generator, backend, and support-matrix behavior.
- Flag docs that describe unsupported runtimes, stale command names, or behavior not covered by tests or support-matrix evidence.
Files:
docs/perf_database/collector-v3-op-centric-design.md
🧠 Learnings (1)
📚 Learning: 2026-02-28T11:44:28.109Z
Learnt from: tianhaox
Repo: ai-dynamo/aiconfigurator PR: 466
File: collector/trtllm/collect_moe_v3.py:4-4
Timestamp: 2026-02-28T11:44:28.109Z
Learning: In collector-related Python files (e.g., collector/trtllm/collect_moe_v3.py), document and enforce that version incompatibilities are surfaced as non-fatal runtime errors during collection. Do not add aggressive preventive version-gating; instead, allow generating test cases that may not be supported across all versions within the __compat__ range and rely on runtime error handling to skip or flag unsupported cases. This should be verifiable by ensuring collection proceeds, errors are reported, and unsupported cases do not halt the overall process.
Applied to files:
collector/op_catalog.pytests/unit/collector/test_op_catalog.pytests/unit/collector/test_manifest_resolution.pytests/unit/collector/test_framework_manifest.pycollector/framework_manifest.py
🪛 LanguageTool
docs/perf_database/collector-v3-op-centric-design.md
[style] ~261-~261: Consider an alternative for the overused word “exactly”.
Context: ...ward* (from a newer version) — which is exactly what today's blank marker dirs do impli...
(EXACTLY_PRECISELY)
[style] ~481-~481: ‘by accident’ might be wordy. Consider a shorter alternative.
Context: ... before the second quarterly cycle, not by accident.
(EN_WORDINESS_PREMIUM_BY_ACCIDENT)
🪛 markdownlint-cli2 (0.23.0)
docs/perf_database/collector-v3-op-centric-design.md
[warning] 41-41: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 74-74: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 228-228: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (8)
collector/README.md (1)
142-144: LGTM!collector/op_catalog.py (1)
1-47: LGTM!collector/framework_manifest.yaml (2)
3-17: LGTM!Also applies to: 20-53
18-19: 🗄️ Data Integrity & Integration
cu130digest needs a registry-side lookup before treating it as wrong. Different tags can point at the same manifest-list digest, so this line alone doesn’t prove a bad pin.tests/unit/collector/test_op_catalog.py (1)
1-56: LGTM!collector/framework_manifest.py (1)
8-153: LGTM!Also applies to: 225-240, 253-300
tests/unit/collector/test_framework_manifest.py (1)
29-37: LGTM!Also applies to: 58-68, 70-87, 89-114, 116-141, 173-177, 202-254
tests/unit/collector/test_manifest_resolution.py (1)
1-140: LGTM!
…alidator, align digest-rule docs Addresses PR #1370 review-bot findings: mixed valid+unknown requested ops now raise KeyError listing the unknown ops; validate_resolution reports a registry import failure as an error string instead of crashing; README and design doc describe the digest rule as it is implemented (syntactic '/' check, bare internal names exempt); bare doc fences tagged for MD040. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: yimingl <yimingl@yimingls-MacBook-Pro.local>
|
History rewritten in place (no content change — |
…alidator, align digest-rule docs Addresses PR #1370 review-bot findings: mixed valid+unknown requested ops now raise KeyError listing the unknown ops; validate_resolution reports a registry import failure as an error string instead of crashing; README and design doc describe the digest rule as it is implemented (syntactic '/' check, bare internal names exempt); bare doc fences tagged for MD040. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Yiming Liu <yimingl@nvidia.com>
Arsene12358
force-pushed
the
yimingl/aic-1500-collector-v3-manifest-v2
branch
from
ae8127e to
d75271e
Compare
The reviewed contract for op-centric collector management: op identity (registry op -> PerfFile table -> catalog family), family-first physical data layout, manifest v2 per-op runtime pins, provenance sidecars, three-channel data reuse, changed-operation manifest, CI enforcement, evidence policy, four-PR delivery plan, and quarterly operation. Linear: AIC-1217 (design), project 'Feature: Collector V3'. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Yiming Liu <yimingl@nvidia.com>
…ep, digest-pinned images (AIC-1500) Signed-off-by: Yiming Liu <yimingl@nvidia.com>
…log absent (AIC-1500) Signed-off-by: Yiming Liu <yimingl@nvidia.com>
…tes (AIC-1500) Signed-off-by: Yiming Liu <yimingl@nvidia.com>
…ime (AIC-1500) Signed-off-by: Yiming Liu <yimingl@nvidia.com>
…v2 docs (AIC-1500) Signed-off-by: Yiming Liu <yimingl@nvidia.com>
…g (AIC-1500 final review) Signed-off-by: Yiming Liu <yimingl@nvidia.com>
…alidator, align digest-rule docs Addresses PR #1370 review-bot findings: mixed valid+unknown requested ops now raise KeyError listing the unknown ops; validate_resolution reports a registry import failure as an error string instead of crashing; README and design doc describe the digest rule as it is implemented (syntactic '/' check, bare internal names exempt); bare doc fences tagged for MD040. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Yiming Liu <yimingl@nvidia.com>
Arsene12358
force-pushed
the
yimingl/aic-1500-collector-v3-manifest-v2
branch
from
d75271e to
37dee97
Compare
|
Rebased onto main now that #1345 is merged. This is the integration moment the design set up: with |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/unit/collector/test_framework_manifest.py (1)
252-260: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert that the family-specific image override is selected.
The test permits the overridden version to be paired with the default image, weakening runtime reproducibility. As per path instructions, tests should ensure results remain traceable to the runtime that produced them.
Proposed change
assert (runtime.family, runtime.version) == ("gemm", "0.5.15") + assert runtime.image() == f"lmsysorg/sglang:v0.5.15{digest}"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/collector/test_framework_manifest.py` around lines 252 - 260, Strengthen the require_collector_runtime test by asserting that the returned runtime uses the family-specific image override, not only the expected family and version. Inspect the runtime image/container reference and compare it with the sglang 0.5.15 override defined by the temporary manifest and catalog inputs, preserving traceability to the producing runtime.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/perf_database/collector-v3-op-centric-design.md`:
- Around line 179-193: Update the moe_perf status contract to define complete
only when CI validates collected coverage against the manifest’s required
systems and cases, not merely when provenance fields are present. Specify the
required coverage validation and ensure incomplete or missing systems/cases
produce status: partial.
---
Nitpick comments:
In `@tests/unit/collector/test_framework_manifest.py`:
- Around line 252-260: Strengthen the require_collector_runtime test by
asserting that the returned runtime uses the family-specific image override, not
only the expected family and version. Inspect the runtime image/container
reference and compare it with the sglang 0.5.15 override defined by the
temporary manifest and catalog inputs, preserving traceability to the producing
runtime.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f24d446e-e22e-45e2-8911-755f73d2940d
📒 Files selected for processing (8)
collector/README.mdcollector/framework_manifest.pycollector/framework_manifest.yamlcollector/op_catalog.pydocs/perf_database/collector-v3-op-centric-design.mdtests/unit/collector/test_framework_manifest.pytests/unit/collector/test_manifest_resolution.pytests/unit/collector/test_op_catalog.py
🚧 Files skipped from review as they are similar to previous changes (5)
- collector/framework_manifest.yaml
- collector/op_catalog.py
- tests/unit/collector/test_op_catalog.py
- tests/unit/collector/test_manifest_resolution.py
- collector/framework_manifest.py
📜 Review details
⏰ Context from checks skipped due to timeout. (6)
- GitHub Check: Collect snapshot (new)
- GitHub Check: Collect snapshot (old)
- GitHub Check: Rust/Python engine-step parity
- GitHub Check: Cargo Deny
- GitHub Check: Build and Test (unit)
- GitHub Check: Build and Test (e2e)
🧰 Additional context used
📓 Path-based instructions (10)
{collector/**,tests/unit/collector/**}
📄 CodeRabbit inference engine (.claude/rules/collector/failure_handling.md)
{collector/**,tests/unit/collector/**}: In collector failure handling, observe failures rather than predict them: do not use a declarative expected-failure layer or automatic skips; failing groups must be fixed.
Record every worker failure automatically in the relevant error and collection-summary JSON files with classification, case parameters, exception details, and model/dtype grouping.
Reset a worker after CUDA-fatal errors only after recording the failed task.
Treat missing data points as tolerable downstream because the SDK can interpolate, extrapolate, reuse sibling-version rows, or use HYBRID empirical estimates.
For a hanging case or one that kills the node, add a dated denylist.yaml entry with a reason.
For an entirely unvalidated operation/backend pair, mark the registry OpEntry as unverified=true.
For cases not validated on a specific SM, mark the registry OpEntry with unverified_sms=(sm,).
Represent physically impossible hardware capabilities with a positive floor in capabilities.yaml; do not use it for framework-version kernel gaps.
Treat out-of-memory failures as unclassified until reproduced on a clean GPU; only a genuine OOM may be excluded by the sanctioned generation-time memory filter.
Fix proven collector-code bugs in code rather than using skips, and re-check dispatch and skip rules.
Do not encode framework-version gaps in YAML; allow them to fail and be re-tested after version changes.
Treat isolated, explained, unclustered failures as acceptable; investigate around 10% unexpected failures or sooner when failures cluster.
Treat roughly one-third failing cases or an entire failing family as a systemic collector problem; stop collection and fix the collector.
Never hide failures with broad skips, retries, generic OOM labels, reduced coverage, synthetic rows, or weakened benchmarks.
Compare failure records with the previous run for the same backend and version; prioritize only new failure groups.
Do not mechanically translate failure records in...
Files:
collector/README.mdtests/unit/collector/test_framework_manifest.py
collector/**
📄 CodeRabbit inference engine (.claude/rules/generator/cross_module_impact.md)
Review the collector when generated configuration formats change or when generator parameter names used in benchmark configurations change.
collector/**: Keep collector-layer rules in their permitted locations: base operation YAML for sweep parameters and cited quant-mode SM limits; model case YAML for structural shapes and policies; capabilities YAML for positive hardware dtype/op minimum-SM floors; backend registries for version routes and unverified markers; collectors for dispatch, classified runtime errors, and memory-feasibility generation filters; and the denylist only for hangs or node-killing cases.
Do not silently skip queued cases in collector code. A queued case must be executed or produce a classified exception; only generation-time capability floors, registry unverified markers, and the hang denylist may prevent execution.
Use framework-selected kernel/backend dispatch for measurements. Prefer the framework builder or selector; manually pin a backend only with a pinned-version file-and-line source citation; record the actually invoked kernel inkernel_source; and re-verify pins andFIXME(kernel-limit)notes on framework version bumps.
Do not invent backend fallbacks or swap backends in exception handlers. If the framework-selected path cannot be constructed or invoked, raise a classified error. Replicate fallback chains only when the framework itself uses them, and record the actual path.
The only sanctioned in-collector filter is memory feasibility: perform it only during generation insideget_*_test_cases(), use size-versus-capacity arithmetic based on live device memory or an approved capacity fallback, never use framework-version or model-name predicates, and log dropped counts in the required format.
Keep framework kernel limits asFIXME(kernel-limit)comments at the owning collector invocation site, including the claimed limit, origin, and unverified status. Do not encode unverified limits in YAML; affected cases should fail at runtime...
Files:
collector/README.md
⚙️ CodeRabbit configuration file
collector/**: - Enforce the collector rules from.claude/rules/collector/layer_permissions.md,failure_handling.md, andcase_authoring.md.
- Flag any silent case skip in collector code (a queued case may only execute or raise); the sole sanctioned filter is generation-time memory feasibility with counted drops.
- Flag invented fallbacks on both ends: generation must raise on unresolvable declarations (never substitute defaults or another model's geometry); collectors must never swap in a different backend/kernel than the framework's own dispatch selects — manual pins require framework source citations.
- Flag any reintroduction of selector/exception machinery (case_ids/contains/indices/ranges/limit/rules, sm_exceptions-style shape or version predicates) in YAML or code.
- Capability floors (
cases/capabilities.yaml) may hold hardware facts only: no shapes, no framework versions, no per-backend nesting.cases/denylist.yamlis for hang/node-killers only, dated.- Collector changes must stay within
collector/andtests/unit/collector/; flag producer+consumer contract changes (perf row schema, PerfFile names) unless the PR explicitly declares them.- Check collector changes for backend/runtime version accuracy, GPU resource assumptions, reproducible command construction, and clear failure evidence.
- Flag changes that make support-matrix or perf-data results harder to trace back to the command, model, system, quantization, or runtime version that produced them.
Files:
collector/README.md
**/*
📄 CodeRabbit inference engine (.claude/rules/repo-guide.md)
**/*: Treat this file as the only always-injected rule file; do not add new always-on rules here without human approval. New rule files must includepaths:frontmatter.
When reviewing changes in a governed area, read that area's rule files first, even if path-based auto-loading does not occur during a read-only review. Rule violations are review findings even when the code works.
A task must remain within its module: collector tasks may change onlycollector/and its tests, generator tasks onlysrc/aiconfigurator/generator/and its tests, and SDK tasks must not modify either. Cross-module contract changes require explicit human approval.
Files:
collector/README.mdtests/unit/collector/test_framework_manifest.pydocs/perf_database/collector-v3-op-centric-design.md
⚙️ CodeRabbit configuration file
**/*: - Prefer applicable inline comments. When the correct fix is clear, small, and limited to the commented diff hunk, include it as a GitHub Suggested Change so the author can apply it with one click.
- Do not use a suggested change when the fix requires broader design choices, multiple files, generated artifacts, unavailable context, or validation that cannot be inferred from the diff.
- If a comment is not directly applicable, state the smallest concrete next step and why a one-click suggestion is not safe.
Files:
collector/README.mdtests/unit/collector/test_framework_manifest.pydocs/perf_database/collector-v3-op-centric-design.md
collector/**/*
📄 CodeRabbit inference engine (.claude/rules/repo-guide.md)
Changes under
collector/must follow.claude/rules/collector/layer_permissions.mdandfailure_handling.md; case YAML work must also followcase_authoring.md. Do not apply generator rules tocollector/case_generator.py.
Files:
collector/README.md
tests/**/*.{py,yaml,txt,sh}
📄 CodeRabbit inference engine (.claude/rules/generator/testing.md)
Use integration tests for the full input-to-artifacts pipeline, comparing output against golden snapshots without external dependencies.
Files:
tests/unit/collector/test_framework_manifest.py
tests/unit/collector/**
📄 CodeRabbit inference engine (.claude/rules/collector/layer_permissions.md)
tests/unit/collector/**: Mark every new collector test withpytest.mark.unit; otherwise a test that only works with-m unitis not collected by CI.
Collector tests must preserve the collector data contract, including perf row schema, canonical perf filenames, and parquet finalization format; contract changes require explicit human approval.
Files:
tests/unit/collector/test_framework_manifest.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (AGENTS.md)
Run
ruff check .andruff format --check .to validate Python linting and formatting.
Files:
tests/unit/collector/test_framework_manifest.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Run unit tests with
pytest -m unit; usepytest -m "unit or build"for the build-test subset when required data is available.
Files:
tests/unit/collector/test_framework_manifest.py
tests/**
⚙️ CodeRabbit configuration file
tests/**: - Check that tests cover the changed behavior rather than only the happy path.
- Watch for fixtures or golden outputs that mask backend drift, support-matrix ordering changes, or CLI output regressions.
Files:
tests/unit/collector/test_framework_manifest.py
docs/**
⚙️ CodeRabbit configuration file
docs/**: - Check that docs match changed CLI, SDK, generator, backend, and support-matrix behavior.
- Flag docs that describe unsupported runtimes, stale command names, or behavior not covered by tests or support-matrix evidence.
Files:
docs/perf_database/collector-v3-op-centric-design.md
🧠 Learnings (1)
📚 Learning: 2026-02-28T11:44:28.109Z
Learnt from: tianhaox
Repo: ai-dynamo/aiconfigurator PR: 466
File: collector/trtllm/collect_moe_v3.py:4-4
Timestamp: 2026-02-28T11:44:28.109Z
Learning: In collector-related Python files (e.g., collector/trtllm/collect_moe_v3.py), document and enforce that version incompatibilities are surfaced as non-fatal runtime errors during collection. Do not add aggressive preventive version-gating; instead, allow generating test cases that may not be supported across all versions within the __compat__ range and rely on runtime error handling to skip or flag unsupported cases. This should be verifiable by ensuring collection proceeds, errors are reported, and unsupported cases do not halt the overall process.
Applied to files:
tests/unit/collector/test_framework_manifest.py
🪛 LanguageTool
docs/perf_database/collector-v3-op-centric-design.md
[style] ~262-~262: Consider an alternative for the overused word “exactly”.
Context: ...ward* (from a newer version) — which is exactly what today's blank marker dirs do impli...
(EXACTLY_PRECISELY)
[style] ~484-~484: ‘by accident’ might be wordy. Consider a shorter alternative.
Context: ... before the second quarterly cycle, not by accident.
(EN_WORDINESS_PREMIUM_BY_ACCIDENT)
🔇 Additional comments (3)
collector/README.md (1)
142-144: LGTM!Also applies to: 156-189
tests/unit/collector/test_framework_manifest.py (1)
29-37: LGTM!Also applies to: 58-68, 70-87, 89-114, 116-141, 173-182, 210-250
docs/perf_database/collector-v3-op-centric-design.md (1)
250-258: 🗄️ Data Integrity & Integration
reuse.yamldoesn’t need extra evidence fields
The kernel-identity and spot-benchmark requirements are already defined in §9 viacollector/evidence_policy.yamlandtools/perf_database/evidence_check.py;reuse.yamlonly declares the reuse target, and CI rejects missing or invalid reuse chains.> Likely an incorrect or invalid review comment.
Review finding on #1370 (CodeRabbit): §5 showed the sidecar schema but never said how 'status: complete' is decided, leaving it readable as self-asserted. Document the actual contract: status is derived at finalize from the run's observed failure records (complete iff zero unresolved failed cases and no module-level failure for the table's producing ops), case_plan_hash attests the attempted case set, and per-dir status is execution-completeness only — cross-system/case sufficiency is the §8 manifest + §9 evidence gate's layer, and CI never re-expands a case plan (generation consults live device memory). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Yiming Liu <yimingl@nvidia.com>
1 participant
PR 1 of 4 of the Collector V3: op-centric collector management project (Linear: AIC-1217 design + AIC-1500 manifest v2). It carries the reviewed design contract and the first implementation slice: per-op runtime pins. The remaining PRs (family-tree data reorg → provenance/
reuse.yaml/changed_ops+ CI audit → loader reuse rules + evidence policy) build on this one; the full delivery plan is §10 of the design doc.What's in here
docs/perf_database/collector-v3-op-centric-design.md— the Collector V3 design contract (op identity chain, family-first physical layout, manifest v2, provenance, three-channel reuse, changed-op manifest, evidence policy, quarterly operation). Mirrored in Linear on the project; this copy is canonical.collector/framework_manifest.yaml→schema_version: 2— per-frameworkdefaultruntime + explicit per-family override slots; wideep flattened to peer frameworks (wideep_sglang,wideep_trtllm) withbase_framework/data_backend; all public images pinned by multi-arch index digest (name:tag@sha256:...; bare internal image names exempt). Pins are unchanged: sglang 0.5.14, trtllm 1.3.0rc10, vllm 0.24.0, wideep-sglang 0.5.10.collector/framework_manifest.py— v2 validation (incl. the digest rule),resolve_op_runtime(framework, op)implementingop → PerfFile table → catalog family → families[family] or default, per-oprequire_collector_runtime(public signature preserved —collector/collect.pyis deliberately untouched; mixed-pin requests fail with the exact op→version split), andvalidate_resolution()returning one error per unresolvable registry op.collector/op_catalog.py— loader for the table→family map fromcollector/op_backend_catalog.yaml. The catalog does not exist on main yet (it arrives with feat(tools): op-backend facts — a truthful record of which backend each op runs on per platform/version/precision #1345), so the loader returnsNonewhen absent: all-default resolution works today, while declaring afamilies:override without the catalog is a hard error.test_real_manifest_resolves_every_registry_opassertsvalidate_resolution() == []in the standard unit suite — every op in all five registries must resolve to exactly one pinned runtime, fail-closed.Coordination notes
op_backend_catalog.yamllands, the CI gate here starts requiring every registry op's table to have a catalog family (includingwideep_moe_perf, used by both wideep registries). Three currently-missing tables are already flagged on that PR (comment); whichever PR rebases second will see the gate fire if the catalog misses a registry table — that is intended fail-closed behavior.frameworks.<fw>.version(v1 schema), it must learn the v2 pathframeworks.<fw>.default.versionbefore its next run against a checkout containing this change.Known limitation (deliberate)
require_collector_runtimedetects mixed pins by version string only; two family overrides sharing a version but pinning different images would not be split. Unreachable with today's manifest (no overrides yet) — noted here so PR 4's reviewer sees it with fresh eyes.Test plan
uv run pytest tests/unit/collector -m unit→ 428 passed, 5 skipped (includes the real-manifest CI gate, all five v1require_collector_runtimeregression cases, fail-closed gates for override-without-catalog / table-without-family / unknown-op, malformed-catalog hardening, and wideep validator branches). All five image digests were verified against the live registries as manifest-list/index digests, twice (implementation + independent review pass).🤖 Generated with Claude Code
Summary by CodeRabbit
(framework, op)→ table → catalog family mappings.validate_resolutionerror reporting.