Define catalogue contract and deterministic validation - #17
Conversation
|
Warning Review limit reached
Next review available in: 55 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR introduces catalogue schema version 2, formalizes catalogue and contribution contracts, expands validation across package and public surfaces, adds pull-request enforcement, and synchronizes README, package, catalogue, and website metadata. ChangesCatalogue contract and synchronized surfaces
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant ValidateWorkflow
participant validate_catalog.py
participant CatalogueSurfaces
PullRequest->>ValidateWorkflow: open or update pull request
ValidateWorkflow->>validate_catalog.py: run with base commit
validate_catalog.py->>CatalogueSurfaces: validate catalogue, packages, README, site, and links
validate_catalog.py-->>ValidateWorkflow: return accumulated validation result
ValidateWorkflow-->>PullRequest: publish check status
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 1
🧹 Nitpick comments (3)
scripts/validate_catalog.py (2)
508-508: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
zip()withoutstrict=.Safe today only because the preceding
actual_ids != expected_idscheck (line 504-506) already guaranteescardsandpetsare equal length before this loop runs. Addingstrict=Truewould make that invariant explicit and guard against future refactors that remove the early return.🤖 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 `@scripts/validate_catalog.py` at line 508, Add strict=True to the zip call in the validation loop over cards and pets, making the required equal-length invariant explicit while preserving the existing iteration behavior.Source: Linters/SAST tools
379-482: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded example pet ID couples the validator to one specific catalogue entry.
Line 467-468 checks for the literal string
"PET_ID=aetherwing # choose an ID from the table above". This ties the validator (and README.md) to "aetherwing" specifically rather than deriving the example from the current catalogue (e.g.typed_pets[0]["id"]). Not a live bug — a rename/retirement of that pet would just surface a clear validation error — but it's a hidden coupling not called out in the "Change requirements"/"Change matrix" tables.🤖 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 `@scripts/validate_catalog.py` around lines 379 - 482, Update validate_root_readme to derive the manual-install example ID from the canonical pets collection instead of hardcoding “aetherwing”. Build the expected guidance using the first catalogue entry’s id, while preserving the existing validation error and guidance format.CONTRIBUTING.md (1)
44-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClarify maintainer-authorization requirement before, not after, the site-file edit list.
Step 3 lists
site/assets/...,site/install/<pet-id>/index.html, andsite/index.htmlas files to update in this same PR, and only afterward (lines 55-59) states website content is maintainer-initiated work requiring explicit authorization before it's edited. A contributor following the numbered list top-to-bottom could start editingsite/before reaching that caveat, contradicting the "Approval gates" indocs/CATALOG_CONTRACT.md("Maintainer initiation is required before website content work").✏️ Suggested reordering
3. Update every public catalogue surface in the same pull request: + Website content remains maintainer-initiated manual work: get explicit + maintainer authorization before editing `site/` files below. + ```text pets/<pet-id>/ site/assets/<pet-id>-preview.gif site/install/<pet-id>/index.html catalog.json README.md site/index.html ``` The root README update is mandatory for both new pets and existing-pet upgrades. - Website content remains maintainer-initiated manual work: a maintainer must explicitly - authorize the website portion before it is edited or approved. The Pages workflow + The Pages workflow deploys approved `site/` files after merge; it does not create or autonomously update catalogue content.🤖 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 `@CONTRIBUTING.md` around lines 44 - 60, Reorder the maintainer-authorization requirement in the Step 3 guidance so it appears before the list of site/ files, clearly requiring explicit authorization before contributors edit or approve any website content. Keep the existing catalogue file list, README requirement, and Pages workflow clarification unchanged.
🤖 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 `@README.md`:
- Line 23: Change the pet-links heading in the README from level-three to
level-two by replacing the leading ### with ##, preserving all linked entries
and their order.
---
Nitpick comments:
In `@CONTRIBUTING.md`:
- Around line 44-60: Reorder the maintainer-authorization requirement in the
Step 3 guidance so it appears before the list of site/ files, clearly requiring
explicit authorization before contributors edit or approve any website content.
Keep the existing catalogue file list, README requirement, and Pages workflow
clarification unchanged.
In `@scripts/validate_catalog.py`:
- Line 508: Add strict=True to the zip call in the validation loop over cards
and pets, making the required equal-length invariant explicit while preserving
the existing iteration behavior.
- Around line 379-482: Update validate_root_readme to derive the manual-install
example ID from the canonical pets collection instead of hardcoding
“aetherwing”. Build the expected guidance using the first catalogue entry’s id,
while preserving the existing validation error and guidance format.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 010978af-dbcb-44d7-bba6-4fda4fa5d7a9
📒 Files selected for processing (23)
.github/ISSUE_TEMPLATE/add-your-pet.yml.github/pull_request_template.md.github/workflows/validate.ymlCONTRIBUTING.mdREADME.mdcatalog.jsondocs/CATALOG_CONTRACT.mdpets/aetherbite/README.mdpets/aethercore/README.mdpets/aethermite/README.mdpets/aetherwing/README.mdpets/bella/README.mdpets/calian/README.mdpets/scarlet/README.mdscripts/validate_catalog.pysite/index.htmlsite/install/aetherbite/index.htmlsite/install/aethercore/index.htmlsite/install/aethermite/index.htmlsite/install/aetherwing/index.htmlsite/install/bella/index.htmlsite/install/calian/index.htmlsite/install/scarlet/index.html
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/validate_catalog.py (1)
551-554: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStrictly verify that the contribution card follows all published pet cards.
Currently,
last_card_positionsearches for any</article>before the contribution card, not necessarily the last pet card in the document. If the contribution card is incorrectly placed between two pet cards, this check will falsely pass.Compare its position against the last pet card to guarantee it appears at the very end of the list.
🐛 Proposed fix
- contribution_position = text.find('class="contribution-card"') - last_card_position = text.rfind('</article>', 0, contribution_position) - if contribution_position < 0 or last_card_position < 0: + contribution_position = text.find('class="contribution-card"') + last_pet_card = text.rfind('class="pet-card') + if contribution_position < 0 or contribution_position < last_pet_card: add_error(errors, "contribution card must follow all published pet cards")🤖 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 `@scripts/validate_catalog.py` around lines 551 - 554, Update the contribution-card validation around contribution_position to locate the last published pet card specifically, rather than any preceding </article>. Compare contribution_position against that final pet-card position and add the existing error when the contribution card is missing or appears before the last published pet card, ensuring it must follow all published pet cards.
🤖 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 `@scripts/validate_catalog.py`:
- Line 273: Update the deep-link regex in the catalog validation logic to
exclude single quotes, closing brackets, and closing parentheses alongside the
existing whitespace, double-quote, and HTML-boundary exclusions. Preserve
matching of valid codex://pets/install query URLs while preventing trailing
Markdown or HTML delimiters from being captured.
---
Outside diff comments:
In `@scripts/validate_catalog.py`:
- Around line 551-554: Update the contribution-card validation around
contribution_position to locate the last published pet card specifically, rather
than any preceding </article>. Compare contribution_position against that final
pet-card position and add the existing error when the contribution card is
missing or appears before the last published pet card, ensuring it must follow
all published pet cards.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 496fe339-f9b3-4b61-9b6e-135b03531a08
📒 Files selected for processing (2)
README.mdscripts/validate_catalog.py
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
1 participant
Summary
Scope
catalog.jsonto schema v2 with immutable catalogue numbers, publication state, and canonical presentation metadata for all 7 published pets.catalog-contractpull-request workflow and catalogue contract documentation.Reason
Catalogue truth was repeated manually across package metadata, README surfaces, site cards, and installers without a complete deterministic contract. The change keeps the static architecture and manual website workflow while preventing silent drift.
Files affected
catalog.jsonscripts/validate_catalog.py.github/workflows/validate.ymldocs/CATALOG_CONTRACT.mdsite/index.htmland all 7 installer pagesCONTRIBUTING.mdChange type
Pet details
Package checklist
pets/<pet-id>/pet.jsonmatches the lowercase directory IDcatalog.jsoncontains the assigned number and canonical presentation fields1536 × 2288, transparent, and uses the8 × 11layoutcodex://parameters match the canonical package metadata and raw spritesheet URLValidation performed
python3 scripts/validate_catalog.pypassesgit diff --checkpassesResults:
python3 scripts/validate_catalog.py --base-ref origin/main: passedcodex://activationImpact
Runtime impact
No pet runtime package changes. No
pet.json, spritesheet, preview, QA image, checksum, or installer-parameter changes. Home Assistant entity semantics and deterministic control lanes are unaffected.UI impact
No redesign, CSS, layout, or image changes. Public copy and alt text are normalized, and stable
data-pet-idhooks are added. Generated dashboards are unaffected. UI truth-consistency risk is reduced by deterministic cross-surface validation.Migration impact
catalog.jsonchanges from schema version 1 to 2. External consumers hard-coded to v1 must support v2. No installed-pet migration, cache clear, application restart, or system restart is required.Rollback safety
Revert the PR commits. Runtime packages and binary assets are untouched, so rollback restores schema v1 and the prior public surfaces without data cleanup.
Rights
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Chores