feat(catalog): register STORAGE as a capability platform#547
Conversation
The storage suite (suites/storage.yaml, added in NVIDIA#439) was never registered in PLATFORM_CONFIGS, so pushed catalogs omitted STORAGE from the platform axis and storage tests carried no platform - the storage capability never appeared in the catalog UI even though the tests did. - Register suites/storage.yaml in PLATFORM_CONFIGS and map the "storage" label in LABEL_TO_PLATFORM (19 tests now carry STORAGE). - Add STORAGE to isvreporter platform constants/aliases: storage suite runs declare "tests.platform: storage", which normalize_platform() silently coerced to the KUBERNETES default when reporting test runs. - Extend the validate-suites pre-commit hook to require every suites/*.yaml to be registered in PLATFORM_CONFIGS, so the next new capability suite cannot ship unregistered. Hook-only by design: make test stays green with untracked scratch suites present. The backend (V17 catalog axis metadata) and frontend (axis read from the uploaded document) already support arbitrary capabilities; this was the missing client-side piece. Requires a new release + catalog push to take effect - catalog push skips versions that already exist. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com>
|
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 with no reviewable changes (1)
📝 WalkthroughWalkthroughThe change adds ChangesSTORAGE platform support
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant SuiteFiles
participant ValidateSuiteWiring
participant CatalogPlatformRegistry
participant PlatformNormalizer
SuiteFiles->>ValidateSuiteWiring: provide suite YAML files
ValidateSuiteWiring->>CatalogPlatformRegistry: read PLATFORM_CONFIGS
CatalogPlatformRegistry-->>ValidateSuiteWiring: return registered suite paths
ValidateSuiteWiring->>PlatformNormalizer: normalize registered platforms
PlatformNormalizer-->>ValidateSuiteWiring: return canonical platforms
ValidateSuiteWiring-->>SuiteFiles: report registration or consistency errors
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🔐 TruffleHog Secret Scan✅ No secrets or credentials found! Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉 🕐 Last updated: 2026-07-16 15:13:22 UTC | Commit: 5d5d8b6 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@isvctl/src/isvctl/config/schema.py`:
- Around line 361-364: Wrap the description value in the platform Field
declaration into adjacent string literals so no source line exceeds 120
characters, while preserving the exact metadata text and formatting the existing
Field definition consistently.
In `@scripts/validate_suite_wiring.py`:
- Line 214: Update the --check argument help text in main() to mention platform
registration failures alongside missing test_id and labels, reflecting the
errors returned by wiring_errors() and platform_registration_errors().
🪄 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: 5592b6e1-cb31-4f9a-ba6d-e8db01cb925f
📒 Files selected for processing (8)
.pre-commit-config.yamlisvctl/src/isvctl/config/schema.pyisvreporter/src/isvreporter/platform.pyisvreporter/tests/test_platform.pyisvtest/src/isvtest/catalog.pyisvtest/tests/test_catalog.pyscripts/tests/test_validate_suite_wiring.pyscripts/validate_suite_wiring.py
Cleanup pass on the STORAGE registration: - Move PLATFORM_CONFIGS to a new dependency-free isvtest.catalog_platforms module (re-exported by isvtest.catalog) so the validate-suites hook no longer pays ~140ms of pytest/jinja2/test-discovery imports to read one static dict. - Derive LABEL_TO_PLATFORM and the script's SUITE_REQUIRED_LABELS from PLATFORM_CONFIGS instead of hand-maintaining three lockstep copies; values verified identical to the previous literals. - Add registry_consistency_errors() to the validate-suites hook: every catalog platform must round-trip through isvreporter's normalize_platform, closing the cross-registry drift that let storage test runs get reported as KUBERNETES - the guard now catches the very bug class that motivated the parent commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com>
Wrap the platform Field description to stay within the 120-column limit and update --check help to document platform registration failures. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com>
Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com>
…ction Syncs the branch with upstream/main (4d61b18), which added NVIDIA#547 "register STORAGE as a capability platform" on top of v0.9.0. NVIDIA#547 and this branch are two divergent designs of the same test-taxonomy area. Per decision, this branch's design wins: - Storage stays a MODULE (module: storage), not a capability platform. - Taxonomy is derived from suite YAML (build_axis_taxonomy) rather than the hardcoded PLATFORM_CONFIGS registry NVIDIA#547 introduced. Kept the branch versions of scripts/validate_suite_wiring.py, isvtest/catalog.py, and their tests; dropped the orphaned isvtest/catalog_platforms.py registry and its platform_registration/registry_consistency validators. Kept upstream's compatible, non-taxonomy additions: - isvreporter.platform STORAGE constant + its config test (coexists with the branch's get_axes_from_config). - .pre-commit-config.yaml validate-suites hook tweak. Verified: ruff, validate_suite_wiring, test_plan_coverage label_sync, make test (all packages), catalog build, and make demo-test all pass. Catalog axes: platforms=[bare_metal, kubernetes, slurm, vm], modules=[..., storage]. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com>
Problem
The 0.9.0 catalog push surfaced the new storage tests in the catalog UI, but the storage capability itself never appeared (https://stg.ai-cloud-labs.nvidia.com/catalog).
Root cause: the storage suite (
suites/storage.yaml, introduced in #439) was never registered inPLATFORM_CONFIGSinisvtest/catalog.py, and thestoragelabel has noLABEL_TO_PLATFORMmapping. Pushed catalogs therefore omit STORAGE from the platform axis and storage tests carry an emptyplatformslist — the UI derives capabilities from exactly those two fields. The backend (V17 catalog axis metadata) and frontend (axis read from the uploaded document, unknown names title-cased) already support new capabilities; this was the missing client-side piece. Unlike security (#370) and observability (#431), which registered their platform in the same PR that created the suite, the storage suite grew across many PRs and the registration step was never hit.Changes
isvtest/catalog.py: register"STORAGE": ["suites/storage.yaml"]inPLATFORM_CONFIGSand"storage": "STORAGE"inLABEL_TO_PLATFORM. 19 tests (Hss*/Directory* + shared checks) now carry the STORAGE platform, and the document axis includes it.isvreporter/platform.py: add the STORAGE constant andstoragealias. The storage suite declarestests.platform: storage, whichnormalize_platform()silently coerced to the KUBERNETES default when reporting test runs.scripts/validate_suite_wiring.py(+validate-suitespre-commit hook): everysuites/*.yamlmust now be registered inPLATFORM_CONFIGS, so the next capability suite cannot ship unregistered. The hook also triggers onisvtest/catalog.pyand the script itself. Enforced hook-only by design:make teststays green when untracked scratch suites are present in the working tree.Verification
make test(all three packages + scripts),make lint,uvx pre-commit run -aall pass.isvctl catalog list --json: axis now[..., SLURM, STORAGE, VM], 19 entries withSTORAGEplatform.suites/makes thevalidate-suiteshook fail with a pointed message whilemake teststill passes.Rollout note
isvctl catalog pushskips upload when theisvTestVersionalready exists, so re-pushing 0.9.0 will not pick this up — this needs to land in a 0.9.1 release followed by a catalog push (or the stored 0.9.0 catalog row must be deleted server-side before a re-push).🤖 Generated with Claude Code
Summary by CodeRabbit
STORAGEas a supported platform, included across platform normalization, configuration/schema docs, and the test catalog platform axis.storage(case-insensitive) toSTORAGE.STORAGEbehavior and for suite registration/registry round-trip consistency checks.