========================== Team Review Implementation ========================== Run a team-based implementation review with platform-neutral multi-persona orchestration, producing a findings register with spec references, severity ratings, and recommendations. .. note:: Generated artifacts use the platform orchestration adapter. Claude can use agent teams where available; Codex and OpenCode ask before subagents and otherwise use the sequential persona fallback. Description ----------- The team-review-impl skill spawns 3 implementation-review-specialist judges who independently review implementation code against a spec document. Each persona operates in a separate context window, ensuring independent analysis. Judges then debate their findings across rounds before producing a consolidated findings register with spec references. **Purpose:** Verify that implementation code correctly and completely delivers what a spec document describes. The three judges cover complementary concerns: - **Fidelity** — is everything in the spec built? - **Correctness** — is what's built actually correct? - **Integration** — does what's built fit well in the codebase? **Relationship to team-review-tests:** - **team-review-tests** reviews *test quality* — are the tests trustworthy? - **team-review-impl** reviews *implementation quality* — does the code match the spec? Both follow the same platform-native persona orchestration workflow pattern with different specialist judges. **Relationship to pre-mortem:** - **Pre-mortem** uses platform-native persona orchestration for *failure analysis* of plans - **Team-review-impl** uses platform-native persona orchestration for *spec compliance analysis* of code Triggers -------- The skill activates when you mention: - Team-based implementation review - Multi-judge impl review - Review code against spec Explicit invocation: .. code-block:: text /wf:team-review-impl Default Judges -------------- The skill uses 3 complementary implementation-review-specialist judges: .. list-table:: :header-rows: 1 :widths: 5 25 70 * - # - Name - Perspective * - 1 - Fidelity Analyst - Spec-to-code mapping: every requirement implemented, no gaps, no scope creep beyond spec * - 2 - Correctness Analyst - Logic and robustness: implementation correctness, edge cases handled, bugs, race conditions * - 3 - Integration Analyst - Codebase fit and quality: project conventions followed, breaking changes, test coverage, API contracts These judges can be overridden via frontmatter, but the defaults are designed to cover complementary implementation review domains and overriding is not recommended. Input Format ------------ The skill uses a **spec-driven input model**: a spec file is required, implementation files are derived automatically or provided explicitly. **Spec file (required):** A markdown spec document describing what should have been implemented. Located via: 1. Explicit path with ``using`` (e.g., ``using spec.md``) 2. the platform default input file in the repository root 3. If not found, the skill asks the user for the spec path **Implementation files (derived):** Paths to implementation files discovered via cascade: 1. ``impl_scope`` frontmatter field (explicit paths) 2. ``@file`` references extracted from the spec body 3. Git diff on the current branch vs base branch 4. If nothing found, the skill asks the user for paths Frontmatter Fields ------------------ All fields are optional. Defaults are applied when not specified. .. list-table:: :header-rows: 1 :widths: 20 10 15 55 * - Field - Type - Default - Description * - ``judges`` - list - *3 impl specialists* - Override default analysts (not recommended) * - ``rounds`` - int - ``2`` - Number of inter-judge debate rounds * - ``verdict_threshold`` - int - ``3`` (unanimous) - Judges that must agree for PASS * - ``output_dir`` - string - ``"resources/agent-docs/reviews/impl"`` - Where transcripts are written * - ``context_budget_kb`` - int - ``100`` - Max total KB for embedded ``@file`` context * - ``impl_scope`` - list - *none* - Paths to implementation files or directories * - ``base_branch`` - string - *auto-detect* - Branch to diff against for file discovery * - ``review_round`` - int - *auto-detect* - Override the detected review round number **Key differences from team-review-tests defaults:** - **Spec file required** (vs team-review-tests' paths-only model) - **Implementation files are derived** — the spec drives what gets reviewed - **Findings include Spec Ref** — each finding links to a spec section - **Separate output directory** — ``resources/agent-docs/reviews/impl/`` Example Frontmatter ------------------- **Minimal (spec file only, impl files auto-detected):** .. code-block:: text /wf:team-review-impl using spec.md **With explicit impl scope:** .. code-block:: yaml --- impl_scope: - src/auth/ - src/middleware/auth.py --- # Implement OAuth2 Authentication The spec calls for OAuth2 PKCE flow with refresh token rotation. **With git diff discovery:** .. code-block:: yaml --- base_branch: main --- # Add User Profile Endpoints Implementation should add REST endpoints for user profile CRUD. ``@file`` Auto-Detection ------------------------ Same mechanism as the council, pre-mortem, and team-review-tests skills. The skill scans the spec body for ``@path/to/file`` references and embeds their contents into judge prompts. See the :doc:`council` documentation for full details on detection rules and embedding logic. Implementation File Auto-Discovery ----------------------------------- Unique to this skill: when ``impl_scope`` is not set and no ``@file`` references are found, the skill falls back to git diff to discover changed files on the current branch. This lets you review an implementation simply by pointing to a spec — the skill figures out what code changed. Context budget priority: 1. **Spec document** — always included in full (review target) 2. **Implementation files** — included if budget allows 3. **``@file`` references** — included if budget allows Reviewer Identity and Review Rounds ----------------------------------- Each report records who produced it and which iteration of the review it is, so a directory of reports stays readable after several rounds and across several models. .. warning:: Two different things are called "rounds". The ``rounds`` frontmatter field is the number of **debate rounds** — how many times the judges exchange assessments inside a single review. The **review round** is the Nth time this spec has been reviewed at all. The transcript labels them ``**Debate Rounds:**`` and ``**Review Round:**`` respectively. Reviewer identity ~~~~~~~~~~~~~~~~~ The reviewing model self-reports its own identifier. The reasoning effort level is resolved from, in order: 1. A reasoning-effort environment variable exposed by the host runtime, if one is set (for example ``CLAUDE_EFFORT`` on Claude Code) 2. The host runtime's own configuration, if readable 3. Otherwise ``unknown`` Either value falling back to ``unknown`` never blocks the review. Only the session model is tagged — judges inherit it, so they are not queried individually. Review round detection ~~~~~~~~~~~~~~~~~~~~~~ The round counts iterations over **the same spec**, regardless of which agent or model ran each one. A round 2 review run by Codex legitimately follows a round 1 review run by Claude — the series belongs to the spec, not the reviewer. Detection reads existing ``output_dir/team-review-impl-*.md`` reports, keeps those whose ``**Spec:**`` header resolves to the same spec file, and takes the highest ``**Review Round:**`` value plus one. With no prior matching reports the review is round 1. Reports written before round tagging existed count as round 1, so the next tagged report becomes round 2. Set ``review_round`` in frontmatter to override detection entirely. Prior rounds are summarised in a ``Prior Review Rounds`` table at the top of the new report, showing each earlier round's date, model, effort and verdict. Output naming ~~~~~~~~~~~~~ Reports are named round-first so iterations of one spec sort together: .. code-block:: text team-review-impl-r01-2026-07-30-143000-opus-5-high.md team-review-impl-r02-2026-07-31-091500-opus-5-high.md team-review-impl-r03-2026-08-01-160000-sonnet-5-medium.md The round is zero-padded to two digits. The model and effort slugs are lowercased with non-alphanumeric runs collapsed to single hyphens. Workflow -------- 1. **Parse spec and optional impl paths** - Locate the spec file using the search order - Split YAML frontmatter from markdown body - Apply defaults (rounds=2, threshold=unanimous) - Resolve the reviewer model, effort level and review round 2. **Identify implementation files** - Cascade: ``impl_scope`` frontmatter → ``@file`` refs → git diff → ask user 3. **Read spec requirements** - Extract section headings, numbered requirements, acceptance criteria - Build requirements checklist for spec coverage analysis 4. **Embed context** - Include spec document, implementation files, and ``@file`` refs within budget - Condense large files via ``smart_read`` 5. **Configure judges** - Use 3 default implementation-review specialists (unless overridden) 6. **Create agent team** - ``Start a session-scoped persona execution run`` - Lead coordinates all communication and writes output files 7. **Spawn judges and seed context** - Each persona receives spec, implementation code, role instructions, and the spec-compliance framing - Each persona produces initial assessment with findings 8. **Execute debate rounds** - For each round, each judge receives all other judges' latest responses - Judges look for finding interactions, missed issues, insufficient suggestions 9. **Consolidate verdict and build findings register** - Apply conservative verdict logic - Build findings register table with Spec Ref column - Produce spec coverage analysis and recommendations by severity 10. **Clean up agent team** - ``Cleanup`` runs unconditionally, even on error 11. **Write transcript** - Write to ``output_dir/team-review-impl-rNN-YYYY-MM-DD-HHMMSS--.md`` Verdict Logic ------------- Team-review-impl uses **conservative** verdict logic (same as pre-mortem and team-review-tests): - **FAIL** if ANY judge finds unimplemented requirements or critical issues - **WARN** if issues exist but implementation broadly satisfies the spec - **PASS** only if ALL judges agree implementation correctly fulfils the spec This means a single judge finding an unimplemented requirement produces an overall FAIL verdict. Findings Register ----------------- The transcript includes a consolidated findings register table with a **Spec Ref** column linking each finding to the relevant spec section: .. code-block:: markdown | # | Finding | Analyst | Severity | File:Line | Spec Ref | Suggestion | Status | |---|---------|---------|----------|-----------|----------|------------|--------| | 1 | Missing farewell function | Fidelity | HIGH | N/A | Req 2: Farewell | Add farewell() | UNADDRESSED | | 2 | No empty name validation | Correctness | HIGH | src/greet.py:12 | Req 1: Greeting | Add ValueError guard | ADDRESSED | **Status values:** - **ADDRESSED** — the suggestion provides a clear fix - **ACCEPTED** — finding acknowledged, risk is acceptable (LOW severity) - **UNADDRESSED** — no adequate fix identified (triggers FAIL if HIGH) Spec Coverage Analysis ---------------------- The transcript includes a section mapping each spec requirement to its implementation status: - **IMPLEMENTED** — requirement fully present in the code - **PARTIALLY IMPLEMENTED** — requirement exists but incomplete - **NOT IMPLEMENTED** — requirement missing from the code This gives a clear summary of spec-to-code fidelity. Recommendations --------------- The transcript organises recommendations by urgency: - **Immediate Actions** — from HIGH-severity findings; unimplemented requirements or critical bugs that must be fixed - **Suggested Improvements** — from MEDIUM-severity findings; should be planned - **Noted** — LOW-severity findings; documented for awareness Transcript Format ----------------- .. code-block:: markdown # Implementation Review Report — Round 2 **Topic:** Implement OAuth2 Authentication **Spec:** docs/specs/oauth2.md **Implementation Files:** src/auth/oauth.py, src/middleware/auth.py **Date:** 2026-02-26 14:30:00 **Review Round:** 2 **Reviewer:** opus-5 (effort: high) **Debate Rounds:** 2 **Verdict Threshold:** 3 of 3 (unanimous) ## Prior Review Rounds | Round | Date | Reviewer | Effort | Verdict | Report | |-------|------|----------|--------|---------|--------| | 1 | 2026-02-25 09:10:00 | sonnet-5 | medium | FAIL | team-review-impl-r01-2026-02-25-091000-sonnet-5-medium.md | ## Analysts | # | Name | Perspective | Final Verdict | |---|------|-------------|---------------| | 1 | Fidelity Analyst | Spec-to-code mapping, gaps, scope creep | FAIL | | 2 | Correctness Analyst | Logic, edge cases, bugs | WARN | | 3 | Integration Analyst | Conventions, breaking changes, tests | PASS | ## Spec Requirements Summary - Req 1: OAuth2 PKCE flow - Req 2: Refresh token rotation - ... ## Overall Verdict: FAIL [summary referencing spec gaps or correctness issues] ## Findings Register | # | Finding | Analyst | Severity | File:Line | Spec Ref | Suggestion | Status | [consolidated table] ## Spec Coverage Analysis [requirement-to-implementation mapping] ## Round 1 ... ## Recommendations ### Immediate Actions ### Suggested Improvements ### Noted ## Verdict Summary | Analysts PASS | Analysts WARN | Analysts FAIL | Threshold | Overall | |---------------|---------------|---------------|-----------|---------| | 1 | 1 | 1 | 3 of 3 | FAIL | Example Usage ------------- **Review implementation against a spec:** .. code-block:: text > /wf:team-review-impl using docs/specs/oauth2.md **Review with explicit impl scope in frontmatter:** .. code-block:: text > # Create spec file with impl_scope frontmatter > /wf:team-review-impl using spec-with-scope.md **Review from default spec location:** .. code-block:: text > # Create the platform default input file with spec and impl_scope > /wf:team-review-impl **Review using git diff for file discovery:** .. code-block:: text > # On a feature branch with changes > /wf:team-review-impl using feature-spec.md Error Handling -------------- - **No spec file provided:** Asks the user for the spec file path - **Spec file not found:** Reports the file was not found at the given path - **No implementation files found:** Asks the user for paths to implementation files - **Platform-native persona orchestration unavailable:** Use the generated sequential fallback and record the downgrade - **Analyst fails to respond:** Error recorded in transcript, remaining analysts continue - **``@file`` not found:** Warning in transcript, file skipped - **``smart_read`` fails:** File skipped, warning logged - **Git diff fails:** Skipped, other discovery methods continue - **Frontmatter parse error:** Reports the YAML error and line number - **Team cleanup fails:** Warning logged, does not block output - **Model or effort not resolvable:** Tagged as ``unknown``; review continues - **Prior reports unlistable or unreadable:** Treated as round 1, noted under Prior Review Rounds - **Detected review round exceeds 99:** Asks the user for an explicit ``review_round`` Limits ------ - **Judges:** 3 default implementation-review specialists. Override is possible but not recommended. - **Rounds:** 2 recommended. Implementation review benefits from cross-pollination but rarely needs more. - **Token cost:** 3 judges x (1 initial + 2 rounds) = 9 agent turns plus coordination overhead. Same cost as team-review-tests. Reserve for important implementations where spec compliance matters. - **Verdict threshold:** Unanimous by default. Deliberately conservative. - **Context budget:** Default 100 KB. Set to 0 to disable file embedding. - **Non-deterministic:** Real output varies between runs. - **One team per session:** Do not run team-review-impl while another team is active. When to Use team-review-impl vs Other Skills --------------------------------------------- .. list-table:: :header-rows: 1 :widths: 20 40 40 * - Aspect - ``team-review-impl`` - ``team-review-tests`` * - **Reviews** - Implementation code vs spec - Test code quality * - **Input** - Spec file (required) + impl files - Test file paths * - **Judges** - Fidelity, Correctness, Integration - Correctness, Coverage, Isolation * - **Use when** - Verifying spec compliance - Auditing test trustworthiness * - **Requires** - Platform-native persona orchestration + spec document - Platform-native persona orchestration .. list-table:: :header-rows: 1 :widths: 20 30 25 25 * - Skill - Best For - Technology - Cost * - **team-review-impl** - Spec-to-code compliance review - Platform-native persona orchestration (Claude only) - ~9 agent turns * - **team-review-tests** - Test quality and false-confidence detection - Platform-native persona orchestration (Claude only) - ~9 agent turns * - **pre-mortem** - Failure analysis of risky plans - Platform-native persona orchestration (Claude only) - ~12 agent turns * - **council** - General validation from multiple angles - Platform-native persona orchestration (Claude only) - ~6 agent turns **Suggested chain for high-stakes features:** 1. **team-review-impl** — verify the code matches the spec 2. **team-review-tests** — verify the tests are trustworthy 3. **Address findings** — fix issues from both reviews 4. **Pre-mortem** — check for deployment risks