fix(done-check,todo-check,quality-list): derive item set from index - #45
Conversation
Runners hardcoded parallel slug lists that drifted from the quality-list index, leaving docstring-drift defined but never audited. Make the Items index the single source of truth for item enumeration and lane; both runners now derive their active set from it. Remove the redundant H1 lane-tag surface from item files so lane lives only in the index. Closes #44
📝 WalkthroughWalkthroughBumped the marketplace metadata version and updated quality-list docs to treat the Items index as the source of truth for lanes and item coverage. Done-check and todo-check now derive their item sets from that index, and todo-check adds docstring-drift coverage. ChangesMarketplace metadata bump
Quality-list audit docs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
skills/quality-list/items/escape-hatch-necessity.md (1)
3-7: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClarify the parenthetical examples to be more language-agnostic.
The parenthetical
(trait dispatch, generics, enum)in line 11 uses Rust-centric terminology. While language-specific addenda exist, the base item's examples should be more universally recognizable or explicitly marked as illustrative.Consider rephrasing to broaden the examples or add a clarifying phrase:
- Diff adds an escape hatch to convert between types a safe construct (trait dispatch, generics, enum) could keep concrete at the use site + Diff adds an escape hatch to convert between types a safe construct (trait dispatch / interface method, generics / templates, enum / discriminated union / std::variant) could keep concrete at the use siteOr more minimally:
- Diff adds an escape hatch to convert between types a safe construct (trait dispatch, generics, enum) could keep concrete at the use site + Diff adds an escape hatch to convert between types a safe construct (e.g., trait dispatch, generics, enum — or their language equivalents) could keep concrete at the use site🤖 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 `@skills/quality-list/items/escape-hatch-necessity.md` around lines 3 - 7, The parenthetical examples in the escape-hatch necessity guidance are too Rust-centric, so update the wording in the quality-list item to use more language-agnostic examples or explicitly mark them as illustrative. Keep the intent in the paragraph about safe alternatives by rephrasing the concrete-type examples around the existing “trait / interface dispatch, generics, an enum” language so it reads universally across languages while preserving the same guidance.
🤖 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.
Outside diff comments:
In `@skills/quality-list/items/escape-hatch-necessity.md`:
- Around line 3-7: The parenthetical examples in the escape-hatch necessity
guidance are too Rust-centric, so update the wording in the quality-list item to
use more language-agnostic examples or explicitly mark them as illustrative.
Keep the intent in the paragraph about safe alternatives by rephrasing the
concrete-type examples around the existing “trait / interface dispatch,
generics, an enum” language so it reads universally across languages while
preserving the same guidance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4ab2715e-2b75-4cd1-aad9-65fc8530c7b2
📒 Files selected for processing (22)
.claude-plugin/marketplace.jsonskills/done-check/SKILL.mdskills/quality-list/SKILL.mdskills/quality-list/items/architectural-boundary.mdskills/quality-list/items/behavior-coverage.mdskills/quality-list/items/completion-hygiene.mdskills/quality-list/items/discovery-surfacing.mdskills/quality-list/items/docstring-drift.mdskills/quality-list/items/escape-hatch-necessity.mdskills/quality-list/items/impact-verification.mdskills/quality-list/items/implementation-guards.mdskills/quality-list/items/invariant-derivation.mdskills/quality-list/items/paired-artifact-drift.mdskills/quality-list/items/pattern-audit.mdskills/quality-list/items/ported-code-attribution.mdskills/quality-list/items/public-api-surface.mdskills/quality-list/items/public-doc-durability.mdskills/quality-list/items/purpose-verification.mdskills/quality-list/items/scope-discipline.mdskills/quality-list/items/signature-change-regression.mdskills/quality-list/items/test-execution.mdskills/todo-check/SKILL.md
Summary
docstring-driftwas defined inquality-listbut audited by neither runner, becausedone-checkandtodo-checkhardcoded parallel item lists that drifted from the index. The duplication was an incidental side effect of an earlier change (b36ea8e) that moved item bodies out of main context: it correctly dropped the bodies but hand-copied the enumeration into each runner, when the short index already held it. This makes thequality-listItems index the single source of truth for item enumeration and lane, and both runners now derive their active set from it. Item bodies are still not loaded into main context — only the index is.Closes #44
Changes
quality-list/SKILL.md: declare the Items index authoritative for which items exist and their lane (not only item-body text); item files no longer carry an H1 lane tag.quality-list/items/*.md: remove the[mechanical]/[contextual]suffix from every item's H1 heading, so the lane lives only in the index.done-check: the Step 2 mechanical subagent and the Step 3 contextual main-context audit both select their item set from the index by lane; the output table is trimmed to illustrative rows emitted one-per-index-item.todo-check: the index is the authoritative spine walked in preflight; the quick reference is reframed as an accelerator over it, with a safe fallback that reads an item file when an index item lacks a quick-reference entry, plus a newdocstring-driftentry.Impact
Runner behaviour — audit every
quality-listitem — is unchanged, except thatdocstring-driftis now covered and the active set can no longer drift from the index. No other skill enumerates the item list (consumers reference items by single slug), so nothing else is affected.Test plan
These are prompt / markdown skills, so there is no executable suite. Verified by deriving the lanes from the index: the
mechanicallane resolves to 9 items (matchingdone-check's prior hardcoded list) and thecontextuallane to 9 (the prior 8 plus the previously-missingdocstring-drift); the dual-laneported-code-attributionlands in both lanes via itsmechanical (+ contextual half)index tag.mdformatruns clean. Adone-checkpass over this diff dogfooded the new derivation — the mechanical subagent selected its items from the index, anddocstring-driftwas audited as a contextual item.Notes
The compressed-lens text in
todo-check's quick reference is still hand-maintained and can drift from the item bodies — a content axis distinct from the enumeration drift fixed here. If that becomes a problem, each item's one-line preflight framing would move into its item file sotodo-checkderives the lens content too.Summary by CodeRabbit