[#313] fix: router/catalog + correctness fixes (task T1)#325
Conversation
- next: catalog 33->35 (add checkpoint+grill to tables and cascades), explicit corpus-derivation note, corrected counts - next: distinguishing predicates on overlapping cascade rows, tie-break co-located with the table - numbering resets fixed: review Step 6.2, refine-story Step 1 - verify-quality: Step 5.C write-exception surfaced in overview + Arguments - grill: never-answer-for-the-human HITL rule promoted to a Core Rule - forward refs marked planned: /brainstorm (#230), /publish-pr (#255) - dataset + .claude mirror byte-consistent Refs: #313 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code Review — PR #325Review InformationPR Number: #325 — Review SummaryOverall Assessment
Rationale: AC1 (routability + no ambiguous co-firing + next stated counts match reality) and AC8 (mirror byte-consistency + gates green) are both fully met and independently verified. Only two non-blocking Minor items and one by-design Nit remain — none touches the T1 acceptance bar. Key Changes SummaryT1 correctness pass over
Business Value ValidationConfirmed. The router previously could not reach 2/35 installed skills and overlapping cascade rows had the resolution rule buried in Notes. Both are the AC1 correctness bar and are now resolved. This unblocks the rest of #313. Verification Performed (independent)
Detailed Review CommentsPositive Feedback
Minor Issues 💡 (non-blocking)
Nit 💡 (by-design — reported for transparency only)
Testing ReviewAdequate for a docs/skill-corpus correctness change. No executable source touched; no unit-test additions expected. The existing structural gates (byte-consistency, staleness, links, mdlint) are the right coverage and all pass. Note (informational, not a finding): no gate currently guards the Adoption Compliance
Risk Assessment
VerdictApproved with Comments. All T1 acceptance criteria (AC1 + AC8) are met and independently verified; every stated gate is green. Two Minor items (one stale count in an adjacent file; one pre-existing state-coverage gap) and one by-design Nit are surfaced for transparency and do not block merge. The stale-count Minor is worth folding into this PR since it directly serves the PR's own "correct every stated count" objective and keeps the corpus internally consistent. Independent review — reconstructed from issue #313 (AC + T1), the PR diff/description, and the code at |
…ch-all - way-of-working.md + getting-started.md (dataset+mirror): stale "33 skills" / "30 (11+19)" → 35 (9 process + 25 capability + 1 navigator) - next: add catch-all cascade row (In Progress + no checkpoint → /implement); renumber Step 3 (6-11) and Step 4 (12-16); tie-break notes every In-Progress story is routed, fallback never reached for active work - dataset + .claude mirror byte-consistent - changelogs/decision-log left untouched (historical records) Refs: #313 Addresses PR #325 review findings 1-2 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review remediation — findings 1 & 2Commit Finding 1 (Minor) — residual stale counts → FIXED
Finding 2 (Minor) — In-Progress-without-checkpoint gap → FIXED with a catch-all row (not a note)Chose the catch-all row over the defer-to-fallback note. Rationale: a story that is New Step 3 cascade (renumbered):
Rows 7 and 8 are mutually exclusive on Finding 3 (Nit, non-actionable) — acknowledged, no changeThe verify-quality write-exception appearing in both the overview and the Arguments section is by design: AC6 requires the buried rule surfaced in both the skill's top section and Arguments. Not merging — awaiting your call. 🤖 Generated with Claude Code |
Re-review — PR #325 remediation verified — ApproveReviewer: Independent reviewer (adversarial, blind to author working context) Both actionable findings from the initial review are genuinely resolved (not merely acknowledged). The by-design Nit was correctly left as-is. All gates re-run green from a detached worktree at Finding 1 (stale counts) — RESOLVED ✓
Finding 2 (In-Progress-without-checkpoint) — RESOLVED ✓Author chose a catch-all row over a note — verified sound:
Finding 3 (Nit) — ACKNOWLEDGEDThe verify-quality dual-placement of the Step 5.C write-exception was Gates (re-run at
|
Code Review — PR #325 (canonical
|
| Risk | Impact | Probability | Mitigation |
|---|---|---|---|
next count drifts again |
Low | Low | In-file derivation note (9+25+1=35); T7/#324 to add a count gate |
| Stale "33/30 skills" in README + website index misleads a reader | Low | Low | Two NEW Minor findings above; users invoke /next (which covers all 35), so functional impact nil |
| Merge order vs #319 (touches review/checkpoint, different regions) | Low | Medium | Per PR: whichever merges second rebases |
Verdict
Approved with Comments. Both ACs T1 owns (AC1 + AC8) are fully met and independently re-verified at 177ed4c; all gates green; both prior actionable findings genuinely resolved. Two new Minor stale counts (README.md:42, website getting-started/index.mdx:18) and two by-design Nits are surfaced for transparency and do not block merge. The two Minor counts are worth folding into this PR since they directly serve the PR's own "correct every stated count" objective and the remediation's whole-repo-sweep intent; otherwise defer them explicitly so T3/T7 does not silently inherit them. Phase 6 (merge) not run — human gate.
Independent /pair-process-review run — reconstructed from issue #313 (AC + T1 task), the PR diff/description/existing comments, and the code at 177ed4c. Author working context under .pair/working/ was not consulted. Not merging — human gate; self-approve via gh pr review intentionally not attempted (author == gh account).
- README.md:26 "33 idempotent workflows" → 35 - README.md:42 "33 Agent Skills — ..." → 35 - repo-root README not covered by docs:staleness (scans content/docs only), so these had no gate Refs: #313 Addresses PR #325 /pair-process-review finding (residual stale counts) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review remediation — /pair-process-review finding (residual stale counts, round 2)Commit Finding — repo-root
|
Canonical review found the gate could not catch the very phrasings it was meant to guard: - skill regex now matches "N+ skills" (trailing +) and the agent/idempotent adjectives, not just pair/composable — "30+ skills" and "N agent skills" previously slipped through silently - guide regex tightened: requires a how-to qualifier (adjective OR how-to/process word), so arbitrary "N guides" prose no longer false-positives in CI (was over-broadened in prior remediation) - Check 5 dead-link scan extended to JSX `href="/docs/..."` card attributes (Fumadocs <Card>), not just markdown `](/docs/...)` - README.md documented as intentionally out of docs-gate scope (literal counts owned by PR #325) tests: +6 (N+/agent/composable skill mismatches, bare "N guides" non-false-positive, dead + valid JSX href). All test-first (red→green). Refs: PR #315 canonical review Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…es/, dedupe, extend staleness gate (#315) * docs: website docs structure assessment + restructuring proposal Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [#312] docs: batch 1 hygiene — hub, section indexes, links, counts, gate extension - /docs rewritten as real hub (4 journeys + top tasks); new concepts/index + reference/index (F1/F2/F11) - 6 stale skill counts fixed to 35 (verified vs dataset/.skills: 25 capability + 9 process + 1 meta) - docs-staleness-check.js: skill-count check now scans ALL content/docs/**/*.mdx; regex extended with 'composable' variant (getting-started phrasing) — subset counts ('9 process skills') excluded - gate also gains dead-internal-link check (resolves ](/docs/...) against content tree) — the optional check from the story, implemented since clean against the filesystem source - test-first proof: extended gate FAILed with 6 count mismatches + 8 dead section links, PASSes after fixes - 8 broken section links fixed (concepts/reference via new indexes; 4 /docs/guides links retargeted to stable pages ahead of T3 dissolve) - quickstart variants retitled 'Quickstart: Solo/Team/Organization' (F6); developer-journey unified on 'Process Lifecycle' (meta.json + 4 prose labels) (F10) - llms-txt orphan: inbound links from concepts/index + reference/kb-structure (F12) - e2e title/sidebar assertions updated (docs + landing); CP5 QA doc: +2 index URLs, label sync Refs: #312 T1 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [#312] docs: batch 2 — merge FAQ+troubleshooting into support/troubleshooting Union of support/faq.mdx (361L) + guides/troubleshooting.mdx (236L) into support/troubleshooting.mdx; conflicting permission fix reconciled (nvm recommended, npm-prefix alternative). Redirects /docs/support/faq + /docs/guides/troubleshooting -> /docs/support/troubleshooting (new redirects() block). Inbound links updated (grep zero in content/). E2e: redirect test added, sidebar/smoke lists updated. CP5 inventory synced. * [#312] docs: batch 3 — dissolve guides/ (moves, merges, redirects, nav) 4 moves: adopter-checklist->getting-started/checklist, cli-workflows-> reference/cli/workflows, install-from-url->customization/install-from-url, update-link->reference/cli/update-link. 2 merges: customize-kb-> customization/team (Quick Recipe section), packaging->customization/ organization (layout-modes detail kept, org flags reconciled with parser, pair->pair-cli normalized). guides/ removed; 7 redirects added (full table now 9 rows incl /docs/guides->/docs). Root meta: drop guides, promote tutorials (13->12). Inbound links updated — grep /docs/guides zero in content/ (incl. one link-only edit in frozen reference/cli/commands.mdx). Hub: duplicate h1 fixed. E2e: moved-pages + dissolved-guides redirect tests, smoke/prev-next lists updated — 31/31 green. CP5 inventory synced. * [#312] fix: review remediation — canonical CLI URL, count fixes, e2e smoke, gate negative test Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore: relocate IA assessment doc to decision-log apps/website/docs-structure-assessment.md was an analysis/decision artifact sitting inside the published site's app directory (not published content itself, but out of place there). Moved to .pair/adoption/decision-log/ where decision records belong. Updated the two references in the #312 issue body accordingly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: correct migration docs URL — was pointing at removed guides/ path DOCS_MIGRATIONS_BASE built URLs under /docs/guides/migrations, a page that never existed there (migrations live at /docs/migrations, top-level). Found during #312's docs restructuring review — pre-existing, unrelated to that PR's scope, so fixed here. Updated the 4 hardcoded test-expected URLs across migration-url.test.ts, version-check.test.ts, version-check-formatter.test.ts. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * [#312] fix: stale how-to guide counts + gate coverage - ai-assisted-sdlc said "11 process guides", developer-journey/index "all 11 guides" — actual KB count is 9; normalize phrasing to "how-to guides" - extend docs-staleness-check: "N how-to/process guides" claims now verified against dataset .pair/knowledge/how-to (test-first, negative + positive fixture cases added) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [#312] docs: IA polish — reading order, orphan link, migrations task - concepts/index: replace wall-of-text with recommended reading order (4 core concepts) + deeper-mechanics list + forward link to lifecycle - customization/index: surface install-from-url (moved here by T3 but invisible from the section index) - docs hub: add Migrations row to top tasks (only section unreachable from the hub) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [#312] style: prettier formatting from pre-push hook Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [#312] fix: re-review findings — gate regex adjectives + loud missing-dir - GUIDE_COUNT_RE now covers adjective phrasings (sequential, step-by-step) with optional modifier — mirrors Check 1's shape; caught 2 more live stale counts on the real tree (knowledge-base.mdx, quickstart.mdx: "11 step-by-step ... guides" vs actual 9) — fixed - guidelines-catalog description normalized "(9 sequential steps)" → "9 sequential how-to guides" so every guide-count claim is gate-covered - missing how-to dataset dir now pushes an error instead of silently skipping the check (consistent with Check 1) - tests: +2 (adjective-phrasing negative, missing-dir failure) Refs: PR #315 re-review findings 1-2 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [#312] fix: harden staleness gate regexes + JSX href dead-link scan Canonical review found the gate could not catch the very phrasings it was meant to guard: - skill regex now matches "N+ skills" (trailing +) and the agent/idempotent adjectives, not just pair/composable — "30+ skills" and "N agent skills" previously slipped through silently - guide regex tightened: requires a how-to qualifier (adjective OR how-to/process word), so arbitrary "N guides" prose no longer false-positives in CI (was over-broadened in prior remediation) - Check 5 dead-link scan extended to JSX `href="/docs/..."` card attributes (Fumadocs <Card>), not just markdown `](/docs/...)` - README.md documented as intentionally out of docs-gate scope (literal counts owned by PR #325) tests: +6 (N+/agent/composable skill mismatches, bare "N guides" non-false-positive, dead + valid JSX href). All test-first (red→green). Refs: PR #315 canonical review Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [#312] refactor: docs-staleness logic into tested website module, drop script Owner rule: tested functionality lives in production code and is tested there — do NOT unit-test scripts. The gate violated this (logic in scripts/docs-staleness-check.js, test spawned it black-box). - port ALL logic verbatim (incl. #315 hardening: tightened guide regex, widened skill regex with +/adjectives, JSX href dead-link scan, README out-of-scope note) into apps/website/lib/docs-staleness-check.ts — ESM module, functions exported individually, import.meta main-guard CLI wrapper, DOCS_STALENESS_ROOT retained, root resolved up 3 - rewrite the test as WHITE-BOX vitest unit tests importing the exported functions directly; ZERO spawnSync/exec — real-tree parity asserted in-process via runAllChecks(). red->green coverage preserved as per-function negatives (wrong N+/composable/agent skills, bare "N guides" non-false-positive, dead/valid JSX href, missing how-to dir) - wire: tsx (catalog 4.23.1) + website script docs:staleness=tsx; root docs:staleness delegates to @pair/website, unchanged in quality-gate + ci.yml (now delegates) - delete scripts/docs-staleness-check.js (single source of truth); update decision-log path refs Refs: PR #315 architectural rule (no test-on-script) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [#312] docs: ADL — gate/tooling code in tested modules, no unit-test on scripts Record the architectural rule the docs-staleness refactor implements as a non-architectural ADL (adoption/decision-log/2026-07-13-gate-tooling- code-in-tested-modules.md): gate logic lives in tested owning-package modules; scripts/CLIs are thin entrypoints; root gates delegate; scripts are never unit-tested (smoke tests cover CLI end-to-end). - complements (does not amend) the test-file-colocation ADL — placement vs module-vs-script boundary are orthogonal; cross-linked - surfaced from way-of-working Quality Gates ("Gate & tooling code" bullet) - no dataset mirror (adoption decision-log is not auto-mirrored; siblings husky/colocation are adoption-only) Refs: PR #315 architectural rule (no test-on-script) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…talog # Conflicts: # .claude/skills/pair-capability-checkpoint/SKILL.md # packages/knowledge-hub/dataset/.skills/capability/checkpoint/SKILL.md
1 participant
Summary
What Changed
Task T1 of story #313 — router/catalog correctness fixes across the
nextrouter and four skills (dataset.skills/**+.claude/skills/**mirror, byte-consistent in this commit):nextcatalog 33 → 35 — addedcheckpointandgrill(previously unreachable via the router) to the catalog tables and the suggestion cascades; corrected every stated count (33→35, 23→25 capability); added an explicit note that the catalog is derived from the installed corpus so drift is detectable./brainstorm(planned — pair-process-brainstorm — 3 phases, parametrized ($root, orientation) #230) and/publish-pr(planned — pair-capability-publish-pr — gate + PR + tag propagation + git-workflow/conditional sections #255) are no longer presented as live skills.Why This Change
The router silently could not reach 2 of the 35 installed skills; overlapping cascade rows could co-fire with the resolution rule hidden in Notes; numbering bugs rendered non-sequentially; and load-bearing behaviors (a skill's only write, an HITL invariant) were buried. Fixing correctness first (T1) unblocks the rest of #313.
Story Context
User Story: #313 — skill corpus effectiveness (router/catalog + correctness fixes)
Acceptance Criteria: AC1 (every installed skill routable, no ambiguous co-firing rows, counts match reality) + AC8 (dataset + mirror byte-consistent, gates green).
Changes Made
Implementation Details
nextcatalog completed to 35 with corpus-derivation note; counts corrected; cascade rows renumbered/brainstormand/publish-prmarked planned with issue linksFiles Changed
next,capability/{verify-quality,grill,checkpoint},process/{review,refine-story}SKILL.md — each in dataset (packages/knowledge-hub/dataset/.skills/**) +.claude/skills/**mirror (12 files total)Testing
Test Results
Testing Strategy
nextcatalog tables + cascade rows and confirmed every one of the 35 corpus skills is reachable.transfer-datasetbyte-consistency test passes.Documentation
Dependencies & Related Work
Refs #313
🤖 Generated with Claude Code