Rename Inspect Command to Review

All four "entirely dead" claims are confirmed by grep. Here's the synthesis.

Yes — there's a meaningful amount of dead and redundant code. The branch shows clear signs of long iteration.

I fanned out four reviewers over the feature (profile/manifest/settings, command/picker, execution/sinks, trail/bridge) and verified the high-confidence findings myself with repo-wide grep. The cleanups fall into three buckets.

1. Entirely dead code — safe to delete (verified, zero production callers)

2. The biggest single simplification — the dead "prompted synthesis" path

Two independent reviewers flagged this, and I confirmed it: autoSynthesis is only ever set to true (cmd.go:1206). That makes an entire second code path unreachable in production:

• The Auto: false arms of composeMultiAgentSinks (cmd.go:~1320–1355)
• SynthesisSink's whole !s.Auto block (synthesis_sink.go:113-134) plus PromptYN / InputTTY / realPromptYN prompting machinery
• The promptYN field threaded through multiAgentSinkInputs
• The dual-wording branches ("final report" vs "summary")

Collapsing SynthesisSink to the auto path alone removes ~40+ lines and a whole parallel branch, and it kills several test-only seams (ProviderTimeout, PromptYN) that exist only to test dead code. Recommend doing this one first — it cascades.

3. Redundancy / over-engineering worth tidying (lower priority)

• Two parallel watch surfaces: top-level trail watch <number> (with bespoke resolveTrailWatchTarget/resolveTrailWatchNumber) duplicates the strictly-more-capable trail finding watch. Drop the former or delegate (~45 lines).
• One-line passthrough wrappers kept alive mainly by their own tests: detectScopeBaseRef → fallbackScopeRef (scope.go:144, even discards its ctx), hydrateTrailReviewCommentSuggestions (trail_review_cmd.go:922), reviewTrailFindingInput (review_bridge.go:91), saveDefaultReviewProfile (profile.go:436).
• RunReviewProfileConfigPicker returns a map every caller discards (picker.go:767) — should return only error.
• flushBuffer has dead error-handling control flow (postrun_sinks.go:29).
• explainEmptyManifest (manifest.go:121-217) re-implements the full filter precedence of matchReviewSessionState — with its own "matcher and explainer drifted" sentinel to catch the desync it creates — purely to print a one-line debug hint. Highest complexity-per-value in the feature; worth a product call on whether the diagnostic earns it.

Two things to NOT "clean up"

• run.go vs run_multi.go token-accumulation duplication is intentional — run.go stays channel-free/goroutine-free for the single-agent path. Leave it.
• Read-side decode structs with 0 reads (TrailReviewComment.*, EventStreamURL, etc.) document the wire contract — removing them risks silent decode gaps. Out of "clearly dead" scope.

Want me to do the cleanup? I'd suggest landing it in two commits: (a) delete the verified-dead code from bucket 1, and (b) collapse the autoSynthesis/SynthesisSink prompted path from bucket 2 — both are low-risk and well-isolated, then run mise run check. I'd hold bucket 3 for a follow-up unless you want it folded in.

Done. All three buckets are implemented, verified, committed, and pushed.

Summary

Commit 48d27b1fc on review-profiles (pushed): +162 / −900 lines across 22 files (2 source files deleted entirely).

Bucket 1 — dead code removed

• Deleted multipicker.go (unused agent multi-picker) and PromptForAgent; refactored the test scaffolding (newDispatchTestDeps lost its dead picker params, and 4 tests that only asserted the absent picker were removed).
• Deleted the trail.Store local-storage layer (store.go) plus the ID/Priority/Type/Reviewer/Discussion/Checkpoints types and constants it alone used.
• Removed TrailDetailResponse and the write-request fields the CLI never sends.
• Deleted EntireSettings.ReviewConfigFor. Kept the legacy Review/ReviewFixAgent/ReviewMigrationDismissed fields — the loader uses DisallowUnknownFields, so they're load-bearing parse-tolerance shims, not dead weight (a finding I verified against the original review's caveat).

Bucket 2 — collapsed the synthesis path

• Confirmed autoSynthesis was always true in production, making the prompted (Auto=false) branch unreachable. Removed SynthesisSink.{Auto,InputTTY,PromptYN}, the legacy compose branch, realPromptYN, and the now-dead canPrompt/promptYN/autoSynthesis sink inputs. Rewrote the affected tests for the auto-only behavior.

Bucket 3 — redundancy/over-engineering

• Top-level trail watch now delegates to the shared trail-review resolver (deleted the bespoke resolveTrailWatch* duplicates); as a bonus it now accepts number/id/branch like trail finding watch.
• RunReviewProfileConfigPicker returns only error.
• Inlined four single-caller wrappers (detectScopeBaseRef, hydrateTrailReviewCommentSuggestions, reviewTrailFindingInput, saveDefaultReviewProfile).
• Fixed flushBuffer's dead error-handling return.

Verification: mise run fmt, mise run lint (0 issues), and mise run test:ci — unit + 59 integration (Vogon) + 4 canary (roger-roger) all green.

Two deliberate non-deletions, both noted in the commit: the settings parse-tolerance fields (above), and SynthesisSink.ProviderTimeout (kept as the test seam that verifies the provider deadline guard). I left the pre-existing untracked .pi/extensions/entire.ts alone, and did not touch explainEmptyManifest since that one was flagged as needing a product call rather than being clearly dead.

Done. The command is now simply review — inspect is gone entirely.

Summary

Commit 124446bf4 on review-profiles (pushed): 25 files, ~328/-331 lines, plus the handoff doc renamed.

What changed

• Command: Use: "inspect" → Use: "review"; dropped the review alias and the stale alias comments/help. Since the command is hidden during maturation, there's no back-compat burden, so no alias was kept.
• Terminology: the worker-role word "inspector(s)" → "reviewer(s)" everywhere — help text, comments, the labs experimental-command registration, and identifiers (defaultInspectorTimeout→defaultReviewerTimeout, inspectorCount→reviewerCount, InspectorTimeout→ReviewerTimeout, inspectorDeadlineFired→reviewerDeadlineFired, etc.). The consolidating role stays "the judge".
• Flag var: inspectTimeout → reviewTimeout.
• User-facing strings: every entire inspect … → entire review …, plus the root.go wiring comment.
• Docs: rewrote docs/architecture/review-command.md and renamed docs/handoff/inspect-judges-handoff.md → review-judges-handoff.md.

Deliberately left as "inspect" (genuinely generic, not the command):

• Code-comment verbs about drilling into agent output / examining errors (tui_model.go, tui_sink.go, run.go, run_multi.go), JSON/ref inspection comments, test prompt data, Go's ast.Inspect.
• The skilldiscovery match keyword "inspect" — it intentionally matches skills named e.g. /inspect-pr; renaming it would duplicate the existing "review" keyword and lose matches.

Verification: mise run fmt, mise run lint (0 issues), mise run test:ci — unit + 59 integration + 4 canary all green. Confirmed zero remaining inspector tokens repo-wide; the only surviving inspect strings are the three intentional ones above.

I have the full picture. The latest branch state is checked out (HEAD = origin/review-profiles, clean tree).

Test coverage assessment — review-profiles

Overall unit coverage of cmd/entire/...: 67.9% — solid for a CLI. But coverage is uneven, and the branch's own feature code sits below the average.

Feature-package numbers

The clear pattern: pure logic is well-tested; the shells are not

What's well covered is exactly the code the earlier review/cleanup added tests for — the finding parsers, prompt composition, manifest matching, profile resolution, sink composition (review_bridge_test.go, cmd_test.go, manifest_test.go, run_multi_test.go are thorough).

What's uncovered is the outer layer, concentrated in a few files:

Most of this is genuinely hard-to-unit-test territory (huh interactive forms, RunE orchestration, git/HTTP/SSE side effects) — that's a normal and defensible gap, not negligence.

Two real caveats / soft spots

1. Integration tests fill part of the gap but not for trail. integration_test/review_test.go exercises the entire review env-var handshake, spawn, and attach end-to-end (4 tests, separate build tag, not in the 60.5% number) — so real review coverage is higher than it looks. But there is no integration test for the trail / trail review / trail watch command surface, which is where the most 0% handlers live. That's the weakest area.

2. ~16 pure helpers sit at 0% that are cheap wins — e.g. finalJudgeDisplayName, handlePickerError, slotLabel, dedupeStrings, filterOutBuiltinCollisions, defaultAgentPick, severityDisplay, trailReviewTargetDisplay, parseOptionalTrailSelector, truncateForLog, safeManifestFilenamePart. These are deterministic, no I/O, and should be trivially unit-tested — leaving them at 0% is the one part of the gap that isn't justified by "it's interactive/networked."

Verdict

Coverage is good on the logic that matters most and weak on the command shells — a healthy shape overall. The two things I'd actually act on: (a) add the handful of cheap pure-helper unit tests, and (b) the trail command family (trail_cmd.go / trail_review_cmd.go / trail_watch_cmd.go) has no integration coverage and the most untested orchestration — that's where a bug is most likely to slip through.

Want me to add the cheap pure-helper tests, or sketch an integration test for the trail command path?

API Error: 500 Internal server error. This is a server-side issue, usually temporary — try again in a moment. If it persists, check https://status.claude.com.

Done. Committed (854115e02) and pushed.

What I added

Two test files covering only the branch's own pure, deterministic helpers that were sitting at 0% — no integration tests, no interactive-form/network/git scope:

• cmd/entire/cli/review/helpers_internal_test.go (package review): finalJudgeDisplayName, exampleAgentList, nonLaunchableEligibleNames, launchableInstalledAgentNames, slotLabel, defaultAgentPick, filterOutBuiltinCollisions, dedupeStrings.
• cmd/entire/cli/trail_helpers_test.go (package cli): severityDisplay, trailReviewTargetDisplay, defaultTrailReviewStatusReason, parseOptionalTrailSelector, truncateForLog.

Result: all 13 helpers went 0% → 100%; review package coverage rose 60.5% → 62.5%. Tests stay registry-independent (use an unmatched agent name) so they're not brittle.

Verification: mise run fmt + mise run lint (0 issues, after fixing the initial nilnil/goconst nits) and mise run test:ci — unit + 59 integration + 4 canary all green.

I deliberately left the interactive pickers, RunE handlers, and the git/SSE side-effecting functions alone — those are integration-test territory, and adding harnesses for them would be the feature creep you asked me to avoid. If you later want the trail command family covered end-to-end, that'd be a separate integration-test effort (there's currently no trail integration test, unlike review).