# Reviewer Termination Isolation and Diagnostics Design

## Summary

Isolate parallel reviewers so a single `terminated` subprocess no longer cancels the whole reviewer batch, recover retryable terminations with the same specification, capture structured execution diagnostics, validate the subprocess resolved model against the request, and produce attributable failure reports. This closes issue #4, where an upstream `terminated` during the independent reviewer stage left only the bare word `terminated` in the final report and discarded the other reviewers' completed work.

The design was informed by a RegiPort PR #9 replay with `deepseek/deepseek-v4-pro` (thinking `max`, 1M context). Task Summary completed successfully; the reviewer stage then received an upstream `terminated` and the whole review went `incomplete` with `A required review stage failed. - terminated`. No reviewer name, Pi retry count, resolved model规格, completed count, or recoverable failure info was retained.

## Goals

- Keep all five reviewers concurrent; one terminal failure must not cancel the batch.
- Preserve successful reviewer results across a sibling's failure.
- Recover once from a retryable `terminated` using the exact same model, thinking, prompt, tools, snapshot manifest, expected changed-file list, JSON contract, and coverage contract.
- Capture structured diagnostics for every agent execution: stage, reviewer label, requested and resolved provider/model, context window, max tokens, thinking, attempt, Pi retry attempts, stop reason, error message, and partial text.
- Validate the subprocess resolved `provider/id` against the request and fail before validation on mismatch, instead of running to completion on the wrong model.
- Produce attributable failure reports naming the failed stage, reviewer, model, thinking, attempt, Pi retry attempts, stop reason, and reviewers completed.
- Keep partial output diagnostic-only; it must never enter findings, validation, or aggregation.

## Non-goals

- Lowering review strength to buy stability (no fewer reviewers, no model or thinking downgrade, no relaxed coverage or confidence thresholds).
- Retrying indefinitely or applying backoff strategies.
- Treating partial output as a usable reviewer result.
- Publishing incomplete reviews.
- Recovering non-retryable failures (aborted, model mismatch, coverage mismatch).
- Cross-attempt accumulation of Pi-internal retry counts; each external attempt reports its own subprocess `piRetryAttempts`.

## Execution Outcome Model

`AgentExecutor` now returns an `AgentOutcome` discriminated union instead of a raw string:

```ts
type AgentOutcome =
  | {
      ok: true;
      text: string;
      stopReason: string;
      partialText: string;
      resolvedModel?: ResolvedModel;
      piRetryAttempts: number;
    }
  | {
      ok: false;
      stopReason: string;
      errorMessage: string;
      partialText: string;
      resolvedModel?: ResolvedModel;
      piRetryAttempts: number;
    };

interface ResolvedModel {
  provider: string;
  id: string;
  contextWindow: number;
  maxTokens: number;
}
```

`partialText` on the success variant equals the final assistant text and is retained only for symmetry and diagnostics; consumers that need partial output read it from the `ok: false` variant. `piRetryAttempts` reflects Pi-internal retries for that single subprocess invocation, not external recovery attempts.

## JSONL Capture

`runPiAgent` streams the subprocess JSONL and aggregates state across all events rather than throwing on the first terminal `message_end`:

- `model_select` events populate `resolvedModel` (`provider`, `id`, `contextWindow`, `maxTokens`). Events missing any required field or with non-positive sizes are ignored, so model validation degrades to skipped when a Pi build does not emit the event.
- `message_end` events update `stopReason`, `errorMessage`, `retryAttempts`, and the assistant `partialText` (first text part retained; `finalText` always tracks the last).
- The first `partialText` snapshot is preserved; later text updates `finalText` only, so a failure after multiple assistant segments still reports the earliest captured text.
- `ok: false` is returned for terminal `stopReason` (`error`, `aborted`), non-zero exit, abort, missing final text, or oversized JSONL records. The orchestrator decides recovery; `runPiAgent` no longer throws on terminations.

## Model Validation

`validateResolvedModel(selection, outcome, options)` runs after every execution inside `runExecutor`. If `outcome.resolvedModel` is present and its `provider/id` differs from `selection.model`, it raises an `AgentExecutionError` with the real `stage` and `label` from `options`, the requested and resolved model, context window, max tokens, and a `model mismatch` message. This fails the review before validation rather than letting a wrong-model run continue. A model registered only via parent-session extensions but unavailable under `--no-extensions` surfaces here when the subprocess resolves a fallback.

## Reviewer Isolation and Recovery

The reviewer stage wraps each reviewer in `runSingleReviewer` and awaits them with `Promise.allSettled`:

- Each fulfilled reviewer increments a shared `completed` counter and emits `reviewer-complete`.
- A rejected reviewer that is an `AgentExecutionError` with a retryable termination (`stopReason === "error"` or `"terminated"`, excluding `aborted`, `model mismatch`, and `coverage mismatch`) and `attempt < REVIEWER_RECOVERY_MAX_ATTEMPTS` (2) emits `reviewer-recovery-retry` and re-runs `runSingleReviewer` with `attempt + 1`, reusing the identical reviewer task, coverage task, repair contract, and expected files.
- Recovery still passes through JSON repair, parse, and coverage retry/校验. No quality gate is bypassed.
- After `allSettled`, any remaining rejection is collected into `ReviewerFailure[]` with its `ExecutionDiagnostics`. If `failures.length > 0`, the review returns `incomplete` with a structured message and never reaches validation or aggregation.

`reviewers.length` (not the shared counter) determines `ReviewerBatchResult.completed` for the failure report, so the reported `completed/total` is deterministic regardless of progress-event ordering.

## Attributable Failure Report

`formatReviewerFailures` renders one block per failed reviewer:

```text
Review incomplete during independent review
- reviewer: bug-review #2
- model: deepseek/deepseek-v4-pro
- thinking: max
- attempt: 2
- stop reason: error
- Pi retry attempts: 3
- error: terminated
- reviewers completed: 4/5
- failed reviewer output excluded from review results
```

Coverage mismatches surface as `AgentExecutionError` with `stopReason: "coverage"` and the existing `coverage mismatch after retry` message, preserving the reviewer label and missing/unexpected path diagnostics.

## Quality Boundaries Preserved

- Five reviewers stay concurrent; model, thinking, reviewer count, and responsibilities are unchanged.
- Coverage contract and 80-point confidence threshold are unchanged.
- Partial output never enters findings, validation, or aggregation.
- Any reviewer that finally fails still yields `incomplete` and is not published.

## Regression Coverage

- Transient `terminated` recovery success (5/5 complete).
- Recovery exhaustion stays `incomplete` with structured report and `4/5` completed.
- Sibling reviewer results preserved when one fails.
- Structured failure report fields (model, thinking, attempt, stop reason, reviewers completed).
- Resolved-model capture from `model_select`.
- Resolved-model mismatch fails before validation with the reviewer label and `model mismatch`.
- Partial text recorded on terminated and on no-text failure.
- Progress `reviewer-recovery-retry` event rendered.