fix(quality-list,todo-check): add validation-parity + ordering guards - #33
Conversation
Extend implementation-guards with two refinements of its existing sibling-parity and entry-validation concerns: - Mirrored-entry validation parity: a new public entry that mirrors an existing sibling (layout-dispatch trait impl, auto/explicit-policy pair, parallel dense/block-sparse path) must replicate the reference sibling's full input-validation set, flagged even when the sibling's guard is not itself in the diff. - Validation precedes panic-prone operations: flag input validation that ends up sequenced after an operation that panics on the same unvalidated input, whether the validator was added late or a panic-prone operation was placed ahead of an existing validator. Tighten the N/A clause to exclude both new concerns and sync the todo-check preflight one-liner.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughUpdated marketplace metadata version and expanded implementation-guard guidance. The guidance now covers mirrored-entry validation parity, validation-before-panic ordering, and matching preflight reference text. ChangesValidation guard guidance
Marketplace metadata
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Summary
When a diff adds a public entry that mirrors an existing sibling — a layout-dispatch trait impl, an auto-policy / explicit-policy pair, a parallel dense / block-sparse path — the gates checked structural and signature symmetry but not validation symmetry. Two real misses motivated this: a mirrored path that dropped a sibling's operand-rank check (silently returning an outer product), and an entry whose validator was sequenced after a panic-prone planner call (panicking instead of returning a semantic error). This extends the
implementation-guardsquality-list item to cover both, refining concerns it already owns rather than adding a new item.Closes #30
Changes
skills/quality-list/items/implementation-guards.md: add a "Mirrored-entry validation parity" paragraph (a new entry mirroring a sibling must replicate the sibling's full input-validation set, flagged even when the sibling's guard is not in the diff) and a "Validation precedes panic-prone operations" paragraph (input validation must not end up sequenced after a panic-prone operation on the same unvalidated input); add two matching Concern conditions; tighten the N/A clause to exclude both.skills/todo-check/SKILL.md: sync theimplementation-guardspreflight one-liner with the two new dimensions..claude-plugin/marketplace.json: version bump.Notes
public-api-surfacewas rejected as the home: its name scopes the public surface (signatures), while this is internal validation behavior. The checks stay in the mechanical lane — a new entry's guards diff against the reference sibling, and validation-vs-panic ordering, are both decidable from literal code without intent or execution. Issue #30 also floats a matching diff-time/code-reviewangle; that is out of scope here because/code-reviewis a built-in command, not authored in this repository.Summary by CodeRabbit
Documentation
Chores