[#221] feat: quality model document (3 layers + 3 pillars + classification rules + taxonomy)#268
[#221] feat: quality model document (3 layers + 3 pillars + classification rules + taxonomy)#268rucka wants to merge 6 commits into
Conversation
Review InformationPR Number: #268 Review SummaryOverall Assessment
Key Changes SummaryAdds Business Value ValidationConfirmed. This is the FIRST story of epic #208 and unblocks assess-cost, assess-security, review-template, and classify — all listed as dependent stories. AC1–AC4 are covered by §1–§7 of the document; verified by direct reading of the diff against the ACs. Code Review ChecklistFunctionality Review
Code Quality Assessment
Technical Standards Compliance
Security ReviewSecurity Checklist
Security ConcernsNone identified. Pure content/test addition. Testing ReviewTest Coverage Assessment
Test Quality Review
Testing FeedbackPerformance ReviewN/A for all sub-sections — pure documentation/test content, no runtime code path, no database, no caching. Documentation ReviewDocumentation Checklist
Documentation Quality
Detailed Review CommentsPositive FeedbackWhat's Done Well:
Issues to AddressCritical Issues
|
| Risk | Impact | Probability | Mitigation |
|---|---|---|---|
| Model too verbose for skills to consume reliably | Low | Low | Reading-budget structure already applied; ~128 lines, tables over prose |
| 2/10 taxonomy pointers forward-reference unwritten guidelines | Low | High (confirmed present) | Disclosed, non-dead-link, tracked against dependent backlog items |
| Resolution-cascade tests are textual only | Medium | High (confirmed present) | Acceptable given zero runtime code shipped; real verification deferred to dependent stories — must not be silently assumed closed later |
Business Risks
| Risk | Impact | Probability | Mitigation |
|---|---|---|---|
| Dependent stories build against 2 forward-referenced guidelines not yet written | Low | Medium | Both disclosed in-doc; epic #208 already tracks the dependent stories |
.pair/llms.txt staleness reduces AI-agent discoverability of the new doc |
Low | Medium (confirmed static index unchanged) | Low impact short-term since the website's dynamic llms.txt route already reflects the new page; static repo-root index gap should be closed before/at next release batch |
Tech Debt Assessment
No NEW tech debt introduced by the code/test in this diff beyond the 3 items above, all of which are pre-existing gaps in the story's own scope (already surfaced by the story's own task-level review notes) rather than debt newly created by this PR's implementation choices:
- Repo-root
.pair/llms.txtnot updated (Major) — recommend a follow-up task or an explicit ADL if batch-regeneration is the accepted pattern. - 2/10 taxonomy pointers forward-reference unwritten guidelines (Minor) — self-resolving once
coupling-balance.mdand the cost-assessment guideline land in their own stories; no action needed here beyond tracking. - Resolution-cascade coverage is textual, not behavioral (Minor) — expected to be closed by dependent skill stories' own test suites; flag in this PR is to prevent it being silently assumed already covered.
None of the three block merge; none require immediate code changes to this PR's content per the review's remit (report-only, no fixes applied here).
Adoption Compliance
| Area | Status | Notes |
|---|---|---|
| Tech stack / new dependencies | CONFORMANT | Zero new dependencies (git diff --stat -- '*package.json' 'pnpm-lock.yaml' empty) |
| Architectural decisions (ADR) | CONFORMANT | No new architectural decision introduced by this diff; .pair/adoption/tech/adr/ untouched; Q1/Q2/Q2b decisions predate this PR (made during story refinement) |
| Non-architectural decisions (ADL) | CONFORMANT | .pair/adoption/decision-log/ untouched; none needed for a pure-content addition |
| Security | CONFORMANT | No secrets, no new attack surface, no dependency changes |
| Coding standards | CONFORMANT | markdownlint green via quality-gate; test file follows existing Vitest conventions in packages/knowledge-hub/src |
| Infrastructure | CONFORMANT | No infra changes |
| Bounded context / architecture files | CONFORMANT | .pair/adoption/tech/architecture.md, boundedcontext/ untouched — not applicable to a docs-only change |
Decision
TECH-DEBT — Approve with tracking. No critical issues, no missing ADR, no security holes, all quality gates (quality-gate, smoke-tests, website e2e) passing. The 3 findings (1 Major: .pair/llms.txt staleness; 2 Minor: forward-referenced taxonomy pointers, textual-only resolution-cascade coverage) are all pre-existing, self-disclosed gaps confirmed still present on the rebased branch tip — recommend tracking them (e.g., a follow-up task against epic #208, or an ADL if .pair/llms.txt batch-regeneration is confirmed as accepted practice) rather than blocking this PR.
Per this review's constraints: no fixes have been applied to the branch, and this PR has not been merged.
4506bda to
76f6b94
Compare
76f6b94 to
fa9daf3
Compare
fa9daf3 to
a7cad81
Compare
a7cad81 to
db520c1
Compare
3-layer principle, 3 pillars (Cost/Security/Delivery), classification dimensions with tier=max, per-tier review/gate requirements, chromatic tag projection, eligibility-as-filter, tech/risk-matrix.md delta schema + nested taxonomy pointers. One doc, KB defaults complete without delta.
New /docs/reference/quality-model mirrors the KB doc; guidelines catalog gets a Quality Model row + Related link.
- point risk-matrix delta section to actual KB source files (quality-model.md §6, risk-matrix-example.md) instead of a catalog anchor that only links back here - add R1.3 tag to shift-left statement per AC1, matching sibling dimension tags - assert all §7 taxonomy pointer links resolve to files on disk (regression guard)
db520c1 to
d3e2aaa
Compare
PR Information
PR Title: [#229] feat: pair-capability-grill interview engine
Story/Epic: #229 (pair-capability-grill — interview engine); Parent Epic #205 (Structured & parametrized brainstorming)
Type: Feature
Priority: P0 (Must-Have)
Assignee: N/A (agent-authored)
Reviewers: N/A
Labels: feature, documentation, skills
Summary
What Changed
Adds a new capability skill,
pair-capability-grill(.claude/skills/pair-capability-grill/SKILL.md), a reusable write-free interview engine with two modes:interview— freeform exploration of a topic, one question at a time, always with a recommendation.sync— systematic AI↔human alignment across six story aspects (Goal, AC, Edge Cases, Dependencies, Design, Risks), ending only on explicit shared-understanding confirmation.The skill explores the KB/codebase before asking anything answerable that way, never writes to adoption files or PM tool issues (the only permitted write is an optional
.pair/working/handoff), and is invocable standalone or composed by/pair-process-brainstorm(phase 1) and a future/pair-process-refine-storyphase 0.Docs and distribution were updated to catalog the new skill and reconcile capability/total skill counts (20→22 capability, 32/33→34 total) across the docs site and
.pair/knowledge/skills-guide.md, plus the packagedknowledge-hubdataset copy.A second commit (
0b55179/e12cb71on this branch) fixed a mapping bug found during self-review: the Sync Coverage Checklist's Goal row referenced aUser Valuesection that only exists in the template's Initial Breakdown, not the Refined User Story Template thatsyncmode targets — removed. The same commit also reconciled the capability count, which had been bumped 20→21 in the first commit against a stale baseline rather than the actual directory count (22).Why This Change
Interviews currently happen ad hoc and inconsistently across brainstorm and refine-story. This story extracts a single, reusable, write-free interview engine so both flows (and standalone use) share the same one-at-a-time / always-recommend / explore-first discipline, and so the
syncmode can gate the Ready transition (R3.11) on an explicit, systematically-covered shared understanding rather than an implicit one.Story Context
User Story: As a pair user in discovery or refinement, I want
pair-capability-grill— a reusable, write-free interview engine (one question at a time, always with a recommendation, exploring KB/codebase before asking) with asyncmode for AI↔human story alignment — so that interviews are consistent across brainstorm and refine, and shared understanding gates the Ready transition (R3.11).Acceptance Criteria:
$mode: synccovers all six story aspects systematically and ends only at explicit shared understanding, returning the alignment synthesis to the caller..pair/working/handoff on request.Changes Made
Implementation Details
.claude/skills/pair-capability-grill/SKILL.md— frontmatter, Arguments ($mode/$topic/$story/$context/$aspects), Modes, 7-step Algorithm (Step 0 mode/target detection + HALT, Step 1 explore-first, Step 2 queue building, Step 3 one-question-at-a-time with mandatory recommendation, Step 4 explicit-confirmation-only exit, Step 5 synthesis assembly, Step 6 return-never-write + optional handoff), Sync Coverage Checklist, Output Format, Composition Interface, Example Transcripts, Edge Cases, HALT Conditions, Graceful Degradation, Notes..pair/working/handoff file convention (naming, content, degradation path if the directory can't be written).skills-catalog.mdx,writing-skills.mdx,skill-management.mdx) and.pair/knowledge/skills-guide.md(project + distributed copies) with a new "Discovery" category and reconciled skill counts; added the distributed copypackages/knowledge-hub/dataset/.skills/capability/grill/SKILL.md.Files Changed
.pair/knowledge/skills-guide.md,packages/knowledge-hub/dataset/.pair/knowledge/skills-guide.md,apps/website/content/docs/contributing/writing-skills.mdx,apps/website/content/docs/reference/skill-management.mdx,apps/website/content/docs/reference/skills-catalog.mdx.claude/skills/pair-capability-grill/SKILL.md,packages/knowledge-hub/dataset/.skills/capability/grill/SKILL.mdDatabase Changes
N/A — no database involved in this repo/change.
API Changes
N/A — no API surface changed; this is a prose skill definition (no runtime code per story sizing).
Testing
Test Coverage
@pair/websitee2e suite re-run (see Test Results) since this branch touches docs-site pages/counts.Test Results
Testing Strategy
syncmode's HALT when no target story is resolvable is specified (Step 0.4 / HALT Conditions) but not exercised.Quality Assurance
Code Quality Checklist
pair-capability-*skills)Review Areas
Deployment Information
Environment Impact
.claude/skills/packages/knowledge-hub/dataset/on next knowledge-hub releaseDeployment Notes
package.jsonchanges in this diff)Rollback Plan
Revert the commits / delete
.claude/skills/pair-capability-grill/and the distributed dataset copy, and revert the doc count changes. No data or runtime state to roll back.Breaking Changes
API Breaking Changes
N/A — no API in this repo affected.
Integration Breaking Changes
N/A — grill is a net-new, opt-in composition point; nothing currently calls it (refine-story's phase 0 composition is documented as a future extension, not yet wired).
Migration Guide
N/A.
Documentation
Documentation Updates
skills-catalog.mdx, count bumps inwriting-skills.mdx/skill-management.mdx.pair/knowledge/skills-guide.mdupdated with new "Discovery Skills (1)" subsection and directory tree entryKnowledge Sharing
User Valuesection reference) and count-reconciliation miss are both disclosed in the issue's Task Breakdown notes and fixed in the follow-up commit on this branchPerformance Impact
Performance Metrics
Benchmarking Results
N/A — no runtime code, nothing to benchmark.
Monitoring
Security Considerations
Security Review
Security Testing
Accessibility
Accessibility Compliance
N/A — no UI shipped in this change (docs-site count/text edits only, no new components).
Accessibility Testing
N/A.
Risk Assessment
Technical Risks
/pair-process-refine-storyphase 0 landsBusiness Risks
docs:stalenessquality-gate check already catches this (ran PASS)Reviewer Guide
Review Focus Areas
pair-capability-*skills, correctness of the Sync Coverage Checklist's section-name mapping againstuser-story-template.md.Testing the Changes
Key Test Scenarios
interviewmode with no$topic— should ask for the topic first, then proceed one question at a time with recommendations.syncmode with$storypointing at a partially-refined story — should skip already-covered aspects and only queue genuinely open ones.syncmode with no$storyand no composer$context— should HALT per Step 0.4.Dependencies & Related Work
Blocking Dependencies
Related PRs
Follow-up Work
Stakeholder Communication
Stakeholder Notification
Communication Plan