[#365] refactor: relocate skill-conventions into guidelines (technical-standards/ai-development)

[rucka]

Pull Request Information

PR Title: [#365] refactor: relocate skill-conventions into guidelines (technical-standards/ai-development)
Story/Epic: #365 (standalone tech-debt, closest precedent #223 under #209; extends #313)
Type: Refactor (pure relocation, no content change)
Priority: P1 (Should-Have)

Summary

What Changed

Moved the cohesive skill-conventions/ folder (10 files — 9 convention files + README) from .pair/knowledge/skill-conventions/ to .pair/knowledge/guidelines/technical-standards/ai-development/skill-conventions/ — dataset canonical form, root mirror (.pair/knowledge/, .claude/skills/) regenerated via pair update. Rewrote every reference: 39 dataset SKILL.md/support files, 2 non-skill refs (writing-skills.mdx prose, to-issues-triage.test.ts path constant + template-resolution.test.ts), and registered the new location in three catalogs.

Why This Change

These are single-source skill-corpus conventions — they belong under guidelines/, catalogued and discoverable, not sitting as a loose top-level folder that reads as "guidelines dumped at the root." Same class of move as #223 (design rules → code-design guidelines).

Story Context

User Story: As a maintainer of the Pair KB, I want the skill-conventions/ files relocated into guidelines/technical-standards/ai-development/skill-conventions/ with every reference rewritten and the section registered in the guidelines catalogs, so these conventions live where guidelines belong.
Acceptance Criteria: All 4 ACs in #365 — grep-clean old path, check:links green + mirror byte-consistent + conformance green, registered in 3 catalogs, byte-identical content (renames only).

Changes Made

Implementation Details

• T1: git mv the 10-file skill-conventions/ folder (9 convention files + README, dataset) as a cohesive unit — sibling links untouched (relative, moved together).
• T2: Rewrote every reference in dataset canonical form — 39 SKILL.md/support files, writing-skills.mdx prose, to-issues-triage.test.ts + template-resolution.test.ts path constants; fixed two outward links the move broke (way-of-working-pm-resolution.md +3 depth, context-map-maintenance.md relative refs).
• T3: Registered in ai-development/README.md, guidelines/README.md, and the docs-site guidelines-catalog.mdx.
• T4: Regenerated the root mirror via node apps/pair-cli/dist/cli.js update --offline --source packages/knowledge-hub/dataset — verified re-running it now produces zero diff (byte-consistent).

Files Changed

• Renamed: packages/knowledge-hub/dataset/.pair/knowledge/skill-conventions/** (10 files — 9 convention files + README) → .../guidelines/technical-standards/ai-development/skill-conventions/** (dataset + regenerated root mirror .pair/knowledge/**).
• Modified: 39 dataset SKILL.md/support files + 39 root-mirror copies (.claude/skills/**) — path pointer rewrite only; ai-development/README.md, guidelines/README.md, guidelines-catalog.mdx (registration); writing-skills.mdx, templates.mdx (prose refs); to-issues-triage.test.ts, template-resolution.test.ts (path constants); context-map-maintenance.md (outward relative link fixed by the move); .pair/llms.txt (index regenerated by pair update).

Testing

Test Results

knowledge-hub: 18 test files, 368 tests passed + check:links green (dataset AND root)
pair-cli:      72 test files, 846 tests passed
lint:          @pair/knowledge-hub, @pair/pair-cli, @pair/website — clean (turbo cache hit, all green)
pair update --offline: re-run after commit → zero diff (mirror byte-consistent)

Testing Strategy

• Happy Path: grep for old path .pair/knowledge/skill-conventions/ across dataset + root + src + docs → zero hits.
• Edge Cases: the two non-link refs the refinement flagged (writing-skills.mdx prose, to-issues-triage.test.ts hardcoded constant) — both verified rewritten; next/SKILL.md's depth outlier (../../ vs the other 38 at ../../../) is handled by the link-rewriter, not hand-counted.
• Regression: full knowledge-hub + pair-cli suites green; the two conformance tests updated in this diff (to-issues-triage.test.ts, template-resolution.test.ts) pass against the new path. refine-story-draft-to-ready.test.ts was not changed (it never referenced skill-conventions) — it only ran green as part of the regression suite.

Quality Assurance

Code Quality Checklist

• Pure relocation — no content edits inside the moved convention files (renames only)
• No debugging code or console logs left behind
• Follows dataset-canonical-form rewrite convention (D-fix: make pair update self-consistent for the dogfooded root install #346 lineage) — root mirror regenerated, never hand-edited

Documentation

Documentation Updates

• Registered new location in ai-development/README.md, guidelines/README.md, docs-site guidelines-catalog.mdx
• Updated writing-skills.mdx and templates.mdx prose references

Risk Assessment

Classification carried over from refinement: risk:red (business impact — core KB/skill layer) · cost:green (pure relocation, no cost signals). Blast radius is wide (~118 files touched) but every change is a mechanical path substitution — no schema/contract/behavior change.

Risk	Impact	Probability	Mitigation
Missed reference breaks a skill pointer	High	Low	Grep-clean verified zero hits on old path; check:links green on dataset + root; full conformance suite green
Root mirror drifts from dataset transform	Medium	Low	Re-ran pair update --offline after commit — zero diff, confirms byte-consistency

Reviewer Guide

Testing the Changes

git checkout feat/365-relocate-skill-conventions
pnpm --filter @pair/knowledge-hub test   # vitest + check:links, dataset + root
node apps/pair-cli/dist/cli.js update --offline --source packages/knowledge-hub/dataset   # should produce zero diff
grep -rn '.pair/knowledge/skill-conventions/' . --include='*.md' --include='*.mdx' --include='*.ts'   # zero hits expected

Key Test Scenarios

1. Confirm the moved folder's 9 convention files + README (10 total) are byte-identical to main except for path/link updates (no prose/content edits).
2. Confirm writing-skills.mdx:141 and to-issues-triage.test.ts (DATASET_CONVENTIONS constant + registration assertion) point at the new path.
3. Confirm the three catalog registrations (ai-development/README.md, guidelines/README.md, guidelines-catalog.mdx) list the section.

Dependencies & Related Work

Follow-up Work

• Follow-up story filed: tech-debt: relocate skills-guide.md into guidelines (follow-up to #365) #369 — relocate skills-guide.md (same sibling smell, explicitly out of scope here per tech-debt: relocate skill-conventions/ into guidelines (technical-standards/ai-development) #365's Edge Cases note).

Pre-Submission Checklist

• All acceptance criteria implemented and tested
• All tests passing (unit + check:links)
• Documentation updated (3 catalogs + 2 prose refs)
• No breaking changes — pure relocation
• PR size appropriate — wide but mechanical (single-purpose path rewrite across the skill corpus)

[rucka]

Code Review — PR #370

Review Information

PR Number: #370
Author: rucka (Gianluca Carucci)
Reviewer: Independent reviewer (Claude Code, /pair-process-review)
Review Date: 2026-07-24
Story/Epic: #365 (standalone tech-debt; extends #313; precedent #223)
Review Type: Refactor (pure KB relocation, no content change)

Review Summary

Overall Assessment

• Approved with Comments — one trivial PR-description inaccuracy noted; all 4 ACs met, all gates green. Safe to merge.

Key Changes Summary

Cohesive relocation of the 9-file skill-conventions/ folder from the .pair/knowledge/ root into guidelines/technical-standards/ai-development/skill-conventions/ (dataset canonical form + regenerated root mirror), every reference rewritten (39 dataset SKILL.md + 39 mirror + non-skill refs), and the section registered in 3 catalogs. Blast radius wide (~114 files) but mechanical.

Business Value Validation

Confirmed: single-source skill-corpus conventions now live under guidelines/, catalogued and discoverable, rather than as a loose top-level folder. Same move-class as #223.

Acceptance Criteria Verification (independently re-run)

AC	Requirement	Result
AC1	Grep-clean of old root path (dataset + root, incl. prose + test constant)	PASS — 0 occurrences of knowledge/skill-conventions anywhere (md/mdx/ts/txt/json), both trees
AC2	check:links green both roots + mirror byte-consistent + conformance green	PASS — check:links green on dataset AND root; pair update --offline re-run → zero diff (byte-consistent); conformance 43/43; full knowledge-hub suite 368/368
AC3	Registered in 3 catalogs	PASS — ai-development/README.md, guidelines/README.md, guidelines-catalog.mdx all list the section
AC4	Byte-identical convention content (renames only)	PASS — 9/10 files byte-identical; way-of-working-pm-resolution.md has the required outward-link re-link only (no prose change), which AC4's own "relocation + re-linking" scope permits

Additional independent checks: old folder removed in both trees; new folder = 10 files (README + 9) in both trees; next/SKILL.md depth outlier (../../ dataset vs ../../../ mirror) correctly reconciled by the link-rewriter, not hand-counted; 44/44 relative links inside moved files resolve in both trees; follow-up story #369 filed for skills-guide.md.

Code Review Checklist

• Requirements met — all 4 AC independently verified
• Behavior preserved (refactor) — pure relocation, no logic touched
• Technical standards — dataset-canonical-form + regenerated mirror (D-fix: make pair update self-consistent for the dogfooded root install #346 lineage) respected; no hand-edited mirror
• Dependencies — none added
• Tests — conformance constants updated; suites green

Security Review

Not applicable — pure file relocation + link rewrite. No authn/authz/secrets/PII/untrusted input. Verdict: green (0 findings, 0 introduced).

Testing Review

knowledge-hub: 18 files / 368 tests PASS; check:links green (dataset AND root)
conformance (to-issues-triage, template-resolution): 43/43 PASS against new path
pair update --offline: zero diff (mirror byte-consistent)

Detailed Review Comments

Positive Feedback

• Byte-consistency verified by re-running pair update → zero diff: the fix: make pair update self-consistent for the dogfooded root install #346 drift class is genuinely closed here.
• Thorough beyond the story's stated blast radius: the refinement enumerated 2 non-skill refs; the author correctly also caught and fixed context-map-maintenance.md and templates.mdx — grep-clean confirms nothing dangles.
• Depth outlier (next/SKILL.md) handled by the rewriter, exactly as the story required.

Minor Issues 💡

• PR [#365] refactor: relocate skill-conventions into guidelines (technical-standards/ai-development) #370 description (Files Changed / Testing) — Lists refine-story-draft-to-ready.test.ts as modified and claims it "passes against the new path," but that file was not changed and does not reference the skill-conventions path (only to-issues-triage.test.ts and template-resolution.test.ts do). Harmless, but the PR body over-claims. Recommend correcting the description.

Questions ❓ / By-design note

• skill-conventions/way-of-working-pm-resolution.md — This convention file is not literally "byte-identical" (AC4 headline wording): 2 outward links change depth (../../../ → ../../../../../../) in both trees. This is required — without it check:links (AC2) would be red — and is explicitly in scope per AC4's "pure relocation + re-linking" preamble. No prose change. Not a defect; noted for transparency at the merge gate.

Risk Assessment

Classification risk:red (business impact — core KB/skill layer) confirmed, not lowered. cost:green. Wide but mechanical; every change is a path substitution. Byte-consistency + dual-root check:links + conformance are the gates, all green.

Definition of Done

All DoD items satisfied: relocation cohesive, mirror regenerated, references rewritten, 3 catalogs registered, grep-clean, check:links green both roots, mirror byte-consistent, conformance green, no content change, follow-up #369 filed.

---

Decision: APPROVED WITH COMMENTS. All 4 acceptance criteria independently verified against the code; all quality gates green; byte-consistency confirmed. The single Minor item (PR-body inaccuracy) is non-blocking. Merge is the human gate — not performed by this review.

[rucka]

Remediation — round 1 (commit c47e210)

Reply to the code-review (Approved with Comments).

Per finding:

• [Minor] PR description over-claims refine-story-draft-to-ready.test.ts as modified/tested against the new path (PR body: Files Changed / Edge Cases) → Corrected the PR description: Edge Cases now explicitly states this file was not changed and never referenced skill-conventions — only to-issues-triage.test.ts and template-resolution.test.ts do. No code change required; description-only fix. — PR #370 description
• [Questions/By-design] way-of-working-pm-resolution.md isn't literally byte-identical — AC4 headline wording vs the 2 outward-link depth changes → Confirmed by-design, no code change. AC4's lead clause explicitly permits relocation + re-linking; the two outward links (way-of-working.md, canonical-states.md) needed +3 ../ depth because the folder moved 3 levels deeper — not fixing them would break check:links (AC2). Sibling (intra-folder) links were correctly left untouched. — packages/knowledge-hub/dataset/.../skill-conventions/way-of-working-pm-resolution.md (+ root mirror), landed in c47e210
• [Questions/By-design] Story AC2 "2 non-skill references" undercounts the actual sweep (4 found: writing-skills.mdx + templates.mdx prose, to-issues-triage.test.ts + template-resolution.test.ts constants) → No code change — the extra two were correctly found and fixed; this is a strength (more complete sweep than the story's undercount), not a defect. The story's edge-cases wording is an inherited undercount, not something introduced by this PR. — writing-skills.mdx, templates.mdx, to-issues-triage.test.ts, template-resolution.test.ts, landed in c47e210
• [Questions/By-design] checkpoint/SKILL.md:149 retains "Shared-reference naming: skill-conventions/<topic>.md" prose → No change required. This documents the file-naming convention (folder name unchanged, files still <topic>.md), independent of the folder's location — remains accurate as-is. — packages/knowledge-hub/dataset/.skills/capability/checkpoint/SKILL.md (+ .claude/skills mirror)

Not changed (escalated): none — all findings are either fixed (PR description) or confirmed by-design/non-actionable with no code impact.

Quality gates: PASS — check:links green (dataset + root), mirror byte-consistent (pair update --offline → zero diff), knowledge-hub 368/368 + pair-cli 846/846, conformance 43/43, lint clean.

Independent re-review converged with zero actionable findings. Ready to merge (human gate).