Review Inspect Feature Cleanup

You are an expert code reviewer. Follow these steps:
Task: Review this change for correctness, regressions, API design, missing tests, maintainability, and user-facing behavior changes. Report only actionable findings with concrete evidence.
Scope: review the commits unique to this branch vs origin/HEAD, plus any uncommitted changes in the working tree. Ignore code outside this scope.
Checkpoint context from commits in scope:
- 75dabf75ec5d: prompt: Merge latest main
- 1a7f1bf3f1ec: prompt: Add a timeout. 10min default.
- 5cbbb55f9459: prompt: Add a timeout. 10min default.
- cb4f9fd21345: prompt: Add a timeout. 10min default.
- 94f89f9567c7: prompt: Add a timeout. 10min default.
- eaf38faaf53d: prompt: Add a timeout. 10min default.
- d4bac437f615: prompt: Add a timeout. 10min default.
- d1fe710ae227: prompt: Add a timeout. 10min default.
- 30b78df00de8: prompt: Add a timeout. 10min default.
- 0b9319108cb5: prompt: Add a timeout. 10min default.
- ff9f72c12650: prompt: Add a timeout. 10min default.
- 9babda34a47b: prompt: Add a timeout. 10min default.
- 74c4f7c8f5a9: prompt: Add a timeout. 10min default.
- c8c400c372be: prompt: Add a timeout. 10min default.
- c104c98fc155: prompt: /Users/dip/Library/Application\ Support/CleanShot/media/media_V7uqzyeA1O/CleanShot\ 2026-06-17\ at\ 12.12.56@2x.png there should be no "change model" option here for review inspectors.
- cc2e5e5a3dcc: prompt: /Users/dip/Library/Application\ Support/CleanShot/media/media_V7uqzyeA1O/CleanShot\ 2026-06-17\ at\ 12.12.56@2x.png there should be no "change model" option here for review inspectors.
- 28dcc15e4c3b: prompt: Yes, should be in the settings or local settings, let the user decide.
- 262d2ea09704: prompt: Yes, should be in the settings or local settings, let the user decide.
- a84cf443023b: prompt: Yes, should be in the settings or local settings, let the user decide.
- 4ddfc23385b7: prompt: Merge latest main
- ... 35 more checkpoints omitted
Use entire explain <id> for full checkpoint context, or entire explain <id> --raw-transcript for raw transcripts.
In-progress session context (uncommitted): 019eca64 Pi (touched: 5 files) prompt: Not the review findings. No commits, brief list.
I've reviewed the in-scope changes thoroughly: read the highest-risk logic (per-inspector timeout, single-writer RunMulti state, model-component matching, profile merge/persistence, codex error envelopes, synthesis sink), confirmed go build ./... is clean, and ran the review/settings/codex test packages (all green).
Review: review-profiles branch
Overview — Reworks entire review/inspect into a profile-based system: named profiles with N parallel inspectors + a single consolidating judge, stored in project/local settings; adds a per-inspector timeout, a local/trail output destination, live model enumeration, and codex error-envelope surfacing. Deletes the old migration path and most of fix.go. ~4900 LOC, exceptionally well-documented.
Findings
1. Untracked working-tree files risk being committed (actionable) Four untracked files are in scope and not gitignored:
.pi/extensions/entire.ts— header says "Auto-generated byentire enable --agent pi… Do not edit manually". This is a local install artifact, not a source deliverable.prompt-default-tasks.md,prompt-inspector.md,prompt-synthesis-judge.md— dev scratch notes dropped at repo root (they just describe where prompt text lives in the Go source).
A git add -A would commit all four. Recommend deleting them or adding ignore rules before committing. None are referenced by the Go code.
2. Parent-cancel vs deadline race in RunMulti (minor / negligible)
run_multi.go:252-255: st.timedOut overrides status to Failed after classifyStatus already returned Cancelled for a parent-cancelled ctx. If a user Ctrl+C races with an inspector's deadline firing and the deadline wins (waitErr == DeadlineExceeded), that agent reports "timed out" instead of "cancelled". Single-agent Run isn't affected (parent cancel surfaces as Canceled). Cosmetic, extremely rare — flagging for completeness, not blocking.
Verified sound
- Timeout flag —
--timeout 0is correctly translated to the-1disable sentinel (cmd.go:214-217); help text "(0 disables)" matches behavior. The 3-state convention (positive/zero-default/negative-disable) ininspectorTimeoutis consistent. - Single-writer
perAgentState— terminal fields routed throughfanInas a tagged marker; dispatch loop is the sole writer, post-loop reads are safe by construction. Sound. OnResult/aggregateOutput—SynthesisSinkis a direct sink;RunFinished(and thusOnResult) runs synchronously insideRunMultibefore it returns, so the read atcmd.go:1216is not racing the deferredtuiSink.Wait(). Correct.modelComponentsMatch— boundary-aware matching with the numeric-suffix requirement correctly distinguishes version (sonnet→claude-sonnet-4-5) from variant (gpt-4o→gpt-4o-mini); the suffix-exclusion tradeoff is documented.mergeReviewProfiles— pure, non-nil, no input mutation; per-profile merge semantics match the documented intent.- Codex
--model+ error envelope —failureMsgonly emitted when!seenTurnComplete, so stray messages on successful runs are ignored. Correct.
No correctness regressions found in the core logic. The only thing I'd act on before committing is the four stray untracked files.