Extended code-review template (security + cost, reading budget)

[rucka]

Story Statement

As a reviewer
I want the code-review template extended — input validation, output, authn, authz, introduced vulnerabilities, cost, architecture — with the verdict-first reading budget
So that every review covers security, cost and architecture consistently and the verdict reads in ~30 seconds (R6.6, D22)

Where: the review artifact already in the process — the native code review submitted by pair-process-review (verdict = the review action, body = tags + 1 line + collapsed details)

Epic Context

Parent Epic: Unified classification & assessments #208
Status: Refined
Priority: P0 (Must-Have)

Status Workflow

• Refined: Story is detailed, estimated, and ready for development
• In Progress: Story is actively being developed
• Done: Story delivered and accepted

Acceptance Criteria

Functional Requirements

Given-When-Then Format:

1. Given the code-review template in KB
   When it is extended
   Then it contains sections: input validation, output handling, authentication, authorization, introduced vulnerabilities, cost, architecture (coupling) — each verdict-first (1 line) with details in <details>

2. Given pair-process-review completes on a PR
   When it submits the review (existing native review action: Approve/Request-Changes/Comment)
   Then the review body opens with classification tags + 1-line verdict, followed by collapsed sections — no separate comment artifact

3. Given a reader opening the review
   When they scan the top
   Then verdict, tier and cost class are understandable in ~30 seconds (D22 budget)

Business Rules

• Verdict lives in the code review already in the process — no new artifact (decision Q5)
• Sections feed from assess-cost (pair-capability-assess-cost — multi-provider cost class at review #226), assess-security (pair-capability-assess-security + deterministic secret-scanning layer #227) and assess-coupling (architecture section) outputs
• Template = source of truth: skill output must follow it

Edge Cases and Error Handling

• Assessment unavailable (skill missing/failed): section shows "not assessed" explicitly — never silently omitted

Definition of Done Checklist

Development Completion

• All acceptance criteria implemented and verified
• Template + skill output aligned
• Code review completed and approved
• Documentation updated — docs site (review process page)

Quality Assurance

• Reading-budget check on a real PR review

Story Sizing and Sprint Readiness

Refined Story Points

Final Story Points: 2 (S)
Confidence Level: High
Sizing Justification: template extension + output alignment; assessments arrive from #226/#227

Sprint Capacity Validation

Sprint Fit Assessment: yes, single sprint
Total Effort Assessment: fits — Yes

Dependencies and Coordination

Story Dependencies

Prerequisite Stories: #226 assess-cost, #227 assess-security (sections feed from them; template can land with "not assessed" placeholders); architecture section feeds from assess-coupling (#263) — lands with "not assessed" placeholder until available
Dependent Stories: PR state flow story (slice ③)

Validation and Testing Strategy

Acceptance Testing Approach

Testing Methods: review dry-run on a real pair PR; template lint; 30s read test

Notes and Additional Context

Refinement Session Insights: Q5 decided — verdict in the native review already in the process
Documentation Links: R6.6 · Spec G7 · D22

Technical Analysis

Implementation Approach

Technical Strategy: extend knowledge/guidelines/collaboration/templates/code-review-template.md; align pair-process-review output format (Phase 5 verdict-first body)
Key Components: template file; review SKILL.md output section
Integration Points: assess-cost/assess-security/assess-coupling outputs; classification tags on PR

Technical Risks and Mitigation

Risk	Impact	Probability	Mitigation Strategy
Template drift vs skill output	Low	Medium	QA step compares generated review against template sections

Task Breakdown

Checklist

• T1 — Extend code-review-template.md (sections + verdict-first structure)
• T2 — Align pair-process-review output (native review body: tags + 1 line + <details>)
• T3 — Docs site update + reading-budget QA on real PR

Dependency Graph

T1 → T2 → T3

AC Coverage

Task	AC1	AC2	AC3
T1	✓
T2		✓	✓
T3		✓	✓

Addendum — spec integrations (2026-07-06)

• Security verdict line in the review report: 1 line + <details> (D22); finding severity mapping P0/P1 ⇒ critical/major review issue; graceful degradation when the security capability is not installed (line reports "not assessed").
• Cost verdict line: same 1-line + <details> pattern; red cost class surfaces the blocking human sign-off requirement in the verdict area.
• Architecture verdict line: same 1-line + <details> pattern, fed by assess-coupling $scope: diff (balance verdict on the integrations the diff touches; findings with severity in the collapsed details); capability absent ⇒ "not assessed" explicitly (same degradation pattern as cost/security).

Addendum — refinement-vs-review drift + code-based reassessment (2026-07-11)

• Classification drift note: when the review-time tier/cost class (assess-cost/assess-security/assess-coupling run against the diff) differs from the refinement-time classification already recorded on the story, the review verdict area surfaces an explicit "Classification changed" line (e.g. "risk: yellow → red — [1-line reason]") rather than silently overwriting it. Per the quality model's resolution rule (Quality model document (3 layers + 3 pillars + default classification rules + taxonomy) #221, §3.2, D17): the review value is a floor — it may raise the tier, never lower it — so a drift note only ever fires upward; a review that reduces a dimension's severity records the reduction as a finding/rationale in the collapsed details, not as a silent tag downgrade.
• Tags land on the PR itself, not only the story issue — reaffirms the existing Integration Points bullet ("classification tags on PR") as an explicit AC-level requirement: whichever tags are active per the story's Tag Projection declaration (risk:* always, if declared; cost:* and any other exposed parameter only if the project opted into it) are also applied to the PR at review time, so the tag is visible wherever the reader is looking (issue or PR) — never assume both families coexist.
• Review assesses the code, not the story: assess-cost/assess-security/assess-coupling MUST run against the actual diff at review time — never copy the story's refinement-time tags forward unexamined. The review-time run is a genuine re-assessment (per D17's "built twice" rule), and its output is what populates the verdict-first sections, not a restatement of the story's own classification.

Cross-ref (do NOT forget)

Story-side counterpart of this template extension is #242 (add a ## Classification slot to user-story-template.md). Keep the two template extensions — review report here, story body in #242 — tracked as a pair so the classification matrix has a first-class template home on both sides.

Classification

risk:red · cost:green — core review/quality-model artifact (code-review template + review-output alignment); multi-file shared-code change, no security-sensitive code surface (refinement).

Classification matrix — per dimension

Dimension	Tier	Source	Note
Service/domain criticality	yellow	KB default (no Criticality Table)	Medium default — file present, table absent (quality-model §3.1)
Change/diff risk	yellow	story scope	template extension + pair-process-review output alignment — touches shared template + review skill
Business impact	red	subdomain class	core — the review/quality-classification process is central to Pair
Security relevance	green	path heuristic	edits a markdown template + skill output format; adds content about security review but touches no authn/authz/secrets code
Coupling balance	not assessed	no DDD subdomain/bounded-context artifacts (refinement)	excluded from the max (quality-model §3.1, D21)

Tier = max(assessed) = risk:red (Business impact). Cost = highest detected signal = green (no cost signals). Model: KB defaults + Tag Projection (Active: risk). Confidence: high.