[US-263] feat: coupling-balance guideline + assess-coupling capability#381
[US-263] feat: coupling-balance guideline + assess-coupling capability#381rucka wants to merge 4 commits into
Conversation
Architecture-quality slice (R7.5): the three-dimensional coupling model plus a capability skill that applies it at review and as a full audit. - Guideline architecture/design-patterns/coupling-balance.md — the single home of the model (D37): integration strength (intrusive>functional> model>contract) x socio-technical distance x volatility (essential vs accidental, from subdomain classification, never commit history), the balance rule, severity, DDD-pattern mapping, fractal application, test implications, coupling.dev bibliographic reference. Original wording. - pair-capability-assess-coupling: $scope diff|full. diff = output-only review verdict (D22, never blocks); full = codebase audit flagging only unbalanced+volatile integrations, writes .pair/working/reports/ architecture/, hands tech-debt tuples to /write-issue (#224). Reads real integration points; never a single-dimension decouple; no-DDD degrades (estimate volatility), never HALT. - assess-architecture reading list gains the guideline (informs pattern scoring); design-patterns README indexes it. - Distribution: .claude mirror, next + skills-guide catalog (30 capability / 40 total), website docs, conformance test; count bumps in dependent gates/tests. Out of scope (per directive): review step 2.3 wiring + Architecture row in the review template are #228's lane; skill authored ready-to-compose. Refs: #263
Code Review — PR #381Review InformationPR Number: #381 Review SummaryOverall Assessment
Key Changes SummaryAdds the original three-dimensional coupling model (integration strength × socio-technical distance × volatility + balance rule) as a KB guideline ( Business Value ValidationThe guideline is genuinely original content that fills the R7.5 "architecture quality" slot, correctly attributes the coupling.dev framing as bibliography, and maps the model onto the DDD patterns already in the KB. The skill faithfully encodes the model's operational rules (three-dimensional always, never single-dimension decouple, volatility-from-subdomain, no-DDD graceful estimate, D22 verdict shape). Value is real — but the review-time delivery that the story frames its ACs around is deferred (see Major #1), and no execution/dogfood evidence exists (Major #2). Code Review ChecklistFunctionality Review
Code Quality Assessment
Technical Standards Compliance
Security ReviewVerdict: green — 0 findings (0 introduced). Diff is Markdown KB/skill/docs + TypeScript count assertions only. No runtime code, no dependencies, no secrets, no auth/data-handling surface. No manual concerns. Testing Review
Documentation Review
Detailed Review CommentsPositive Feedback
Major Issues 🔍
Minor Issues 💡
Questions ❓
Risk Assessment
Adoption ComplianceLevel: full (manual inline check). Follows the two-mode assess-* convention, D14 report-write exception, D37 single-home rule (with the Minor #3 caveat), D22 verdict shape, output-only-for-adoption contract. No new dependency, no new architectural decision → no ADR required (refinement D36-D39 already cover the decisions). Compliant. Follow-up ActionsAuthor Action Items
Human Merge Gate
REVIEW COMPLETE:
|
Code Review — PR #381Review InformationPR Number: #381 Review SummaryOverall Assessment
Key Changes SummaryAdds the original three-dimensional coupling model (integration strength × socio-technical distance × volatility + balance rule) as a KB guideline ( Business Value ValidationThe guideline is genuinely original content that fills the R7.5 "architecture quality" slot, attributes the coupling.dev framing as bibliography (not lifted), and maps the model onto the DDD patterns already in the KB (ACL as strength-reducer, aggregates as low-distance-where-strength-must-stay-high). The skill faithfully encodes the model's operational rules. Value is real — but the review-time delivery the ACs are phrased around is deferred (Major #1) and no execution/dogfood evidence exists (Major #2). Code Review ChecklistFunctionality Review
Code Quality Assessment
Technical Standards Compliance
Security ReviewVerdict: green — 0 findings (0 introduced). Diff is Markdown KB/skill/docs + TypeScript count assertions only. No runtime code, no dependencies, no secrets, no auth/data-handling surface. No manual concerns. Testing Review
Documentation Review
Detailed Review CommentsPositive Feedback
Major Issues 🔍
Minor Issues 💡
Questions ❓
Risk Assessment
Adoption ComplianceLevel: full (manual inline check). Follows the two-mode assess-* convention, D14 report-write exception, D22 verdict shape, output-only-for-adoption contract, D37 single-home rule (with the Minor #1 caveat). No new dependency, no new architectural decision → no ADR required (refinement D36–D39 already cover the decisions). Compliant. Follow-up ActionsAuthor Action Items
Human Merge Gate
REVIEW COMPLETE:
|
Code Review — PR #381Review InformationPR Number: #381 Review SummaryOverall Assessment
Key Changes SummaryAdds the Business Value ValidationFills the R7.5 "architecture quality" slot. Guideline + skill are well-constructed and internally rigorous on the three-dimensional discipline. The headline value ("caught before merge" via review step 2.3) is not active in this PR — it is deferred to #228 (still OPEN); the skill is authored ready-to-compose. Independent verification performed
Acceptance Criteria
Detailed Review CommentsPositive Feedback
Major Issues 🔍
Minor Issues 💡
Questions ❓
Adoption / ADRNo new technical decision in this PR (refinement already recorded D36–D39; no ADR/ADL required). assess-architecture version bump 0.5.0→0.6.0 is appropriate. No files under #228's or #352's lanes were touched — isolation confirmed. Risk Assessment
DecisionRequest Changes — reconcile the tolerable-tier retention contradiction (Major-1, fixable in this PR, no #228 dependency) and address the DoD validation gap (Major-2, run the standalone full-audit dogfood). All quality gates, conformance, staleness, and link checks pass; byte-identity and catalog consistency verified. The guideline + skill are otherwise strong and the #228 review-wiring deferral is legitimate. |
…bump catalog date Review PR #381 round 1: - F1: retain unbalanced+low-volatility as tolerable tech-debt (never dropped, never blocking) across guideline "few critical" para + SKILL Steps 4.3/8.1 + ASCII + frontmatter (both mirrors); only balanced / <3-dim scores are non-findings. - F3: trim assess-coupling frontmatter description to sibling length (~665-678). - F4: bump skills-catalog "Last updated" to 2026-07-24. - F2 (dogfood) done as gitignored working artifact — no tracked change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-coupling Step 4.1/4.2 (dataset + root mirror) restated the balance rule + severity conditions that AC5/D37 require to live ONLY in the guideline. Replace with links to the guideline's #the-balance-rule / #severity sections; the skill now applies, never restates. Byte-parity of the name-transform preserved (edited lines identical in both trees). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remediation — round 1 (commit
|
…er (docs-staleness Check 2c) Rebalances the CRITICAL website<->dataset coupling surfaced by this PR's assess-coupling dogfood: the skills-catalog Command + Description columns are now derived from each dataset SKILL.md's frontmatter and CI-enforced by a new docs-staleness check (checkCatalogContent), so a skill description edit that isn't mirrored to the catalog fails the gate (single source = dataset). Reduces coupling strength; counts + row presence were already enforced (Checks 1 & 2). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
PR Information
PR Title: [US-263] feat: coupling-balance guideline + assess-coupling capability
Story/Epic: #263 (epic #209 — Code Quality & Testing foundations)
Type: Feature
Priority: Medium
Labels: user story
Summary
What Changed
Adds an original three-dimensional coupling model (integration strength × socio-technical distance × volatility + balance rule) as a KB guideline, plus the
pair-capability-assess-couplingskill that applies it in two modes:$scope: diff— verdict on the integrations touched by a PR (strength/distance/volatility → balance verdict, 1-line + findings with severity).$scope: full— codebase audit, flags only unbalanced and volatile integrations, feeds tech-debt items via the assess-* skills are output-only; record-decision is the sole adoption writer (no scan-mode) #224/pair-capability-write-issuepipeline, and writes a report to.pair/working/reports/architecture/.pair-capability-assess-architecture's reading list now includes the new guideline (version bump 0.5.0 → 0.6.0). Catalog counts, indexes, and website docs are updated everywhere the skill/guideline counts are asserted (39→40 total skills, 29→30 capability skills).Why This Change
Fills the R7.5 "architecture quality" slot in the quality model: pair had no first-class way to reason about coupling quality (only ad hoc mentions). The model is deliberately original content (not a lift from a third-party framework) with its own DDD mapping, severity criteria, and fractal application, and follows the same two-mode (
diff/full) shape already established byassess-cost(#226) andassess-security.Story Context
User Story: As a team practicing DDD, I want a coupling-balance model and an
assess-couplingcapability, so integration quality (not just cost/security) is assessed at review and can be audited full-codebase for tech debt.Acceptance Criteria:
$scope: diffverdict flow (strength/distance/volatility → balance + severity)$scope: fullaudit flow (unbalanced+volatile only, tech-debt handoff, report)Changes Made
Implementation Details
knowledge/guidelines/architecture/design-patterns/coupling-balance.md— the three-dimensional model, severity criteria, DDD pattern mapping, fractal application, test implications (contract + boundary tests), coupling.dev bibliography. Dataset + root mirror byte-identical.pair-capability-assess-couplingSKILL.md —$scope: diff|full, always three-dimensional, volatility-from-subdomain (never commit history), no-DDD graceful estimate (never HALT), severity model, never-single-dimension-decouple rule, D22 verdict + "not assessed" degradation, full-mode report format (inline in the skill, no separate template — same pattern as the assess-security audit), tech-debt handoff tuples for/pair-capability-write-issue(assess-* skills are output-only; record-decision is the sole adoption writer (no scan-mode) #224).next/SKILL.md,skills-guide.md,getting-started.md,way-of-working.md(dataset + mirror), 10 website docs (skills-catalog row + every "N skills" phrasing), count assertions inassess-cost.test.tsanddocs-staleness-check.test.ts.assess-coupling.test.ts— dataset+mirror invariants for the skill/guideline pair and catalog registration.Files Changed
.claude/skills/pair-capability-assess-coupling/SKILL.md,packages/knowledge-hub/dataset/.skills/capability/assess-coupling/SKILL.md,.pair/knowledge/guidelines/architecture/design-patterns/coupling-balance.md,packages/knowledge-hub/dataset/.pair/knowledge/guidelines/architecture/design-patterns/coupling-balance.md,packages/knowledge-hub/src/conformance/assess-coupling.test.ts.claude/skills/pair-capability-assess-architecture/SKILL.md,.claude/skills/pair-next/SKILL.md(+ dataset mirrors),.pair/knowledge/getting-started.md,.pair/knowledge/guidelines/architecture/design-patterns/README.md,.pair/knowledge/skills-guide.md,.pair/knowledge/way-of-working.md(+ dataset mirrors), 10apps/website/content/docs/**/*.mdxfiles,apps/website/lib/docs-staleness-check.test.ts,packages/knowledge-hub/src/conformance/assess-cost.test.tsTesting
Test Coverage
assess-coupling.test.ts(dataset+mirror skill/guideline invariants + catalog registration); existingassess-cost.test.tsanddocs-staleness-check.test.tsupdated for the new count.$scope: full, representative): the skill's algorithm was executed by hand against the pair repo (see the Dogfood section below). Bounded to the 5 most significant cross-module couplings — a representative run that proves the method works, not an exhaustive repo audit.Dogfood —
assess-coupling $scope: fullapplied to pair (representative)Method per coupling-balance.md: each coupling scored on all three dimensions (integration strength × socio-technical distance × volatility), the balance rule combined strength+distance, volatility set the stakes, severity per the guideline's table. Integration points read from real code (imports/exports/data access), never the dependency-graph shape. Volatility sourced from
.pair/adoption/product/subdomain/(core→high, supporting→medium, generic→low);(est.)marks inferred values.skills-catalog.mdx+ all "N skills" phrasings ↔dataset/.skills/)validateKBStructure)transfer-dataset,skills-guide-mirror,check-broken-links↔@pair/content-ops)fileSystemService,applyTransformCommands,rewriteSkillReferences, path ops — no internals).claude/.pairmirror (byte name-transform duplication)@pair/brandcomponents, tailwind preset,tokens.css)Triage: 5 couplings assessed → 1 critical, 4 balanced (1 with a tolerable contingency note). Matches the model's "few critical beat many minor" heuristic.
Critical finding (actionable tech-debt): website ↔ knowledge-hub dataset — the skills catalog + all "N skills" counts are hand-duplicated across ~10 website mdx files (exactly the multi-file sync cost this story's T6 paid). Rebalancing = reduce strength: generate the catalog rows + counts from the dataset at build time (single source of truth), making
docs-staleness-check.tsthe contract test that guarantees the derivation stays honest. NOT a "decouple" — splitting would raise distance without removing the duplication.The method executed end-to-end against real code and resolved volatility from the subdomain adoption, proving the algorithm works. Diff-mode-on-a-real-PR verification (compose
/pair-capability-assess-coupling $scope: diffinto/pair-process-reviewstep 2.3) is deferred to #228 (review-side wiring, now merged/in-flight); this PR authored the skill ready-to-compose.Test Results
Mirror parity:
coupling-balance.mdand design-patternsREADME.mdare byte-identical root↔dataset; the assess-coupling SKILL mirror is the expected deterministic name-transform of the dataset source. This parity is not asserted byassess-coupling.test.ts— that conformance test only checks keyword/invariant presence in each copy, not byte/transform equality. Guideline & skill mirror parity is verified centrally by the #352 mirror-equality guard (thetransfer-datasetname-transform + byte-parity check inpackages/knowledge-hub/src/tools, run in CI), corroborated here by a manual root↔dataset diff on the lines changed in this PR.Quality Assurance
Review Areas
$scope: diff|fullshape fromassess-cost/assess-security.Deployment Information
No deployable package is touched — this PR only changes
packages/knowledge-hub(KB dataset/skills),.pair//.claude/mirrors, andapps/websitedocs content. No environment/config/migration impact.Documentation
Documentation Updates
coupling-balance.md) + skill (assess-coupling) added to all catalogs, indexes, and website docs where skill/guideline counts and lists are asserted.Notes for the Reviewer (not visible from the checkpoint)
/pair-process-review— that's Extended code-review template (security + cost, reading budget) #228's lane (extended code-review template).assess-couplingis authored ready-to-compose: same$scope: diff|fullinterface, same D22 verdict shape, and a "Composition Interface" note at the end of the SKILL body describing exactly how step 2.3 + an "Architecture" row would invoke it, mirroring howassess-cost(pair-capability-assess-cost — multi-provider cost class at review #226) was authored ready-to-compose ahead of Extended code-review template (security + cost, reading budget) #228 wiring it in. No code in this PR touches the review template orpackages/knowledge-hub/src/tools(tech-debt: extend mirror-equality guard to per-skill SKILL.md pairs (all 36 skills) #352's lane) — isolation was checked file-by-file (see below).assess-coupling $scope: fullwas applied by hand (the skill is LLM-executor instructions, not runnable code — ADL 2026-07-13-gate-tooling-code-in-tested-modules.md, so there's no CLI to invoke) to the 5 most significant cross-module couplings in pair; results are in Testing → Dogfood above. It surfaced one critical finding (website↔dataset catalog duplication — actionable tech-debt: derive the catalog from the dataset) and four balanced couplings, proving the method executes end-to-end. Diff-mode-on-a-real-PR (compose into/pair-process-reviewstep 2.3) is deferred to Extended code-review template (security + cost, reading budget) #228./pair-process-review(Extended code-review template (security + cost, reading budget) #228's lane), none underpackages/knowledge-hub/src/tools(tech-debt: extend mirror-equality guard to per-skill SKILL.md pairs (all 36 skills) #352's lane) — onlypackages/knowledge-hub/src/conformance/*.test.tsfiles were touched there, which is in-scope for this story.gh issue view 263— only theuser storylabel is present), so no tags were copied to this PR; classification will happen fresh in review (G6).Rebalance in-place del finding CRITICAL del dogfood (commit
02efd4d)Il dogfood
assess-coupling $scope: fulldi questa PR ha rilevato un coupling CRITICAL website↔knowledge-hub dataset (skills-catalog + conteggi duplicati a mano su ~10 mdx). Rimediato qui, in-place (chiusura dogfood→fix):checkCatalogContent(Check 2c) inapps/website/lib/docs-staleness-check.ts: il contenuto delle righe del catalogo (Command + Description) è ora derivato dal frontmatter di ogniSKILL.mddel dataset e CI-enforced → una modifica alla description di una skill non rispecchiata nel catalogo fa fallire il gate. (I conteggi "N skills" e la presenza-riga erano già enforced dai Check 1 e 2.)skills-catalog.mdxriconciliato al dataset (descrizioni single-sourced; drop delle code cross-ref hand-written).docs-staleness-check.test.ts, 43 totali).docs:stalenessPASS — 40 skills in sync.Rebalance = riduzione della strength: il catalogo diventa una proiezione del dataset con contratto CI, non una copia parallela a rischio drift. Verifica diff-mode-su-PR-reale della skill resta deferita a #228.