Plan Eng Review

/plan-eng-review

The required review per Lintel's Review Readiness Dashboard. Scope: architecture, code quality, test coverage, performance. Outputs a structured plan-file review report + persists via first-party bin/li-review-log so the ship phase can read it.

The architecture-and-tests gate before build — the one review Lintel requires. On top of the architecture/quality/coverage/performance pass it also runs:

• cli_support frontmatter check on every skill/agent the plan adds
• Voice-tier check on every customer-facing skill the plan adds
• the active pack's compliance gates as a checklist inside Step 0 (resolve_pack_field compliance.hooks; none by default)

When to use

• Before any non-trivial implementation begins
• Before ship — Dashboard verdict gate depends on this
• After any major plan revision (re-run, supersedes prior report)

When NOT to use

• Trivial fix (typo, comment) — no plan involved
• Pure docs PR — /review is the lighter pre-landing alternative

Inputs

• Optional path to a plan/design doc. Auto-discovers from ~/.lintel/projects/<slug>/*-design-*.md if not provided.
• Optional --scope diff — review the current branch's diff instead of a plan doc (degrades to /review semantics).

For mapped work, validate the explicitly selected work.json with bin/li-work-artifacts.py and use the shared work-map contract. Read design from plan, requirements from spec and leaf IDs/checkboxes from tasks. Package membership in the design or linked handoff references those original IDs; do not generate a parallel task list or require a Lintel approval heading inside Spec Kit's technical plan. Ungrouped tasks use singleton packages. Honor the same selection as PLAN/BUILD, never the newest directory.

Workflow

Step 0: Scope Challenge (BLOCKING — no review-section work until this resolves)

1. What existing code partially solves each sub-problem? Map reusable patterns.

2. Minimum set of changes? Ruthless about scope creep.

3. Complexity check: plan touches 8+ files OR introduces 2+ new classes/services?

   • If yes: AskUserQuestion proposing minimal version, ask reduce-or-proceed.
   • STOP until resolved.

4. TODOS.md cross-reference: any deferred items now blocking? Any items the plan SHOULD subsume?

5. Completeness check: is plan doing the complete version or a shortcut? With AI-assisted coding, completeness cost is 10-100x cheaper than human-team — recommend the lake, not the puddle.

6. Distribution check: new artifact type (binary, package, container)? CI/CD pipeline included or deferred?

7. Granularity hard check (2–5 min per verifiable leaf): For every leaf in plan.md, estimate implementation time from its specification and linked context:

   • Target: 2–5 minutes per leaf, described precisely enough to implement without prior conversation context.
   • Any leaf estimated >5 min → AskUserQuestion with two options:
     • A) Decompose now — split the task into 2-N smaller tasks ≤5 min each. Preferred.
     • B) Accept with concern — keep the task; log the concern in plan.md "Reviewer Concerns" section.
   • Tasks <2 min are fine (combinable if useful, but no hard rule).
   • Evaluate every leaf; grouping into a work package does not waive this check.

8. Work-package review: apply the shared package contract. Verify one outcome, common write owner/edit boundary, connected dependencies and acceptance evidence for every unchanged leaf ID. Split across different owners, security boundaries, irreversible decisions or independent rollback boundaries. No arbitrary package size limit. Check aggregate complexity: a multi-file package cannot be classified mechanical solely because its leaves are small. Packages execute sequentially, their leaves in dependency order; one package spec review precedes one quality review, with findings mapped back to leaves. A package is incomplete while any leaf lacks verification. Older ungrouped plans use singleton packages. The plan table does not create new job state or claim automatic enforcement.

Sections 1-4 (after scope agreed)

Use numbered findings and label decision options by issue NUMBER + option LETTER. Present related findings together and request only unresolved decisions; retain each finding's affected package/leaf IDs so a shared review cannot hide gaps.

1. Architecture — system design, dependency graph, data flow, scaling, security boundaries, ASCII diagrams worth embedding in code comments.
2. Code quality — DRY (aggressively flag), error handling, technical debt, over/under-engineering.
3. Tests — coverage diagram per Step 3 below; 100% coverage is the goal. Test plan artifact written.
4. Performance — N+1 queries, memory, caching opportunities, slow paths.

Test Step 3 — coverage diagram (mandatory)

Trace every codepath the plan introduces. Map user flows + interaction edge cases. ★★★/★★/★ rating per existing test. Mark [→E2E] vs [→EVAL] vs unit. Output ASCII diagram.

REGRESSION RULE (mandatory, no AskUserQuestion): If the diff modifies existing behavior + test suite doesn't cover the changed path → regression test added as CRITICAL task.

Optional: Outside voice

After all 4 sections: offer codex (or Claude subagent if codex unavailable) for independent challenge. Cross-model tension surfaced per topic via AskUserQuestion. User decides per tension point — outside voice is INFORMATIONAL, not auto-applied.

Report format — written to the plan/design doc

## REVIEW REPORT

| Review | Trigger | Why | Runs | Status | Findings |
|--------|---------|-----|------|--------|----------|
| Eng Review | /plan-eng-review | Architecture & tests (required) | N | CLEAR (PLAN) | M issues, K critical gaps |

**UNRESOLVED:** count
**VERDICT:** ENG CLEARED — ready to implement | NOT CLEARED — <reason>

Persist via first-party bin/li-review-log:

review_source="${LINTEL_SOURCE_ROOT:-${LINTEL_REPO_ROOT:-$(git rev-parse --show-toplevel 2>/dev/null)}}"
"$review_source/bin/li-review-log" '{"skill":"plan-eng-review","timestamp":"...","status":"...","unresolved":N,"critical_gaps":N,"issues_found":N,"mode":"FULL_REVIEW","commit":"..."}'
"$review_source/bin/li-review-read"

Run both helpers from the selected source bundle. LINTEL_REPO_ROOT selects the working repository for audit data, commit matching and legacy import; it does not select helper code. If unset, the helpers use the current working repository.

Required outputs

• NOT in scope section — explicit deferrals with one-line rationale.
• What already exists — reuse map.
• TODOS.md updates — trace proposed deferrals to their findings and resolve any missing scope decision.
• Failure modes — per new codepath: realistic failure + test? + err handling? + silent vs visible. Critical gaps flagged.
• Worktree parallelization — dependency table + lanes + execution order + conflict flags.
• Implementation Tasks — unchanged leaf IDs, each derived from a finding (no padding), grouped by bounded work package in the plan. Keep the existing flat JSONL artifact via jq -nc for /autoplan aggregation; packaging adds no job schema.
• Completion Summary — section-by-section issue counts + Lake Score (X/Y recommendations chose complete).

Compliance integration

• The active pack's compliance gates run at Step 0 (resolve_pack_field compliance.hooks; none by default). The advisory baseline still applies — no customer data in plan prose, no secrets, no production mutations without auth.
• Per Lintel v1: also verify every new skill/agent introduced declares cli_support in frontmatter (per C1) and voice tier (per A6).

Exit Plan Mode Gate (BLOCKING)

Before ExitPlanMode:

1. Read the plan file. Confirm the LAST ## heading is ## REVIEW REPORT. Dual-accept (grace until 2026-09-12): the legacy ## GSTACK REVIEW REPORT heading still passes, so design docs approved before the rename remain valid. New writes use ## REVIEW REPORT.
2. Report contains: Runs/Status/Findings table + VERDICT line.
3. bin/li-review-log called + bin/li-review-read consumed at least once.

Failing this gate + calling ExitPlanMode = contract violation. User sees a plan with a missing/stale report + rejects it.

Failure modes

• No design doc: offer /office-hours as prerequisite. If user skips, proceed with standard review against the diff.
• Operator skips a per-issue AskUserQuestion: mark as unresolved decision, list in "Unresolved decisions that may bite later" at end.
• jq missing: skip JSONL write, warn user — markdown task list still primary deliverable.

Examples

Healthy clear:

> /plan-eng-review
[Step 0 + 4 sections, 6 issues resolved]
✓ ENG CLEARED — ready to implement
Tasks: 17 in markdown + JSONL
Dashboard updated

Scope reduction:

> /plan-eng-review
[Step 0 complexity check fired — 140-file scope]
AskUserQuestion: reduce-or-proceed?
Operator: proceed (Path C accepted)
[continue 4 sections]

See also

• /plan-ceo-review — strategy review (runs before this)
• /plan-design-review — UI/UX review (parallel if there's a UI surface)
• /review — diff-scoped lighter variant (when plan-eng-review is overkill)
• /ship — reads this skill's review-log output as ship-gate signal
• /autoplan — chains office-hours → ceo-review → eng-review → design-review