Add deep-review extension + UX polish

[ferologics]

What this PR adds

This PR introduces a new deep-review extension for Pi that runs a full deep code-review flow in-session.

It is designed to mirror your Prompt Editor workflow:

1. Build a PR context pack first
2. Send a direct OpenAI Responses API request with that pack + your review query
3. Stream progress live
4. Post a final markdown review with usage/cost + handoff artifacts

Why

The goal is deterministic, high-context review quality with request-level control/parity, without relying on the normal Pi agent turn loop.

How it works

• /deep-review <query> [options] (query is required)
• Runs nested pi -p with explicitly scoped skill execution:
  • --no-extensions --no-skills --skill <pr-context-packer SKILL.md>
• Reads the generated context pack path
• Calls POST https://api.openai.com/v1/responses directly (SSE stream)
  • input = 2 user messages (context pack, query)
  • includes reasoning effort/summary settings
  • tools: [], stream: true, store: false
• Updates a compact live status widget during run
• Posts final markdown response + timing/token/cost summary
• Writes handoff artifacts and copies answer to clipboard

Commands

• /deep-review
• /deep-review-stop (stops in-flight run cleanly)

Output artifacts

Saved to a temp directory per run:

• answer.txt
• thinking.txt
• report.md
• metadata.json

Validation

• just check passes (tsc --noEmit, tests)
• Non-blocking existing warnings remain:
  • Biome schema version drift info
  • regex-literal lint suggestion

[ferologics]

Deep review (gpt-5.2)

• Query: review the code
• Context pack: /tmp/context-packer/pr-pi-extensions-20260209-002855/pr-context.txt
• Context pack time: 10.8s
• Responses time: 6m 5s
• Total time: 6m 16s
• Response ID: resp_080128b69e50bea10169891c3cf46481948a2dd6a744e223a5

Usage

• Input tokens: 24,561
• Output tokens: 22,285
• Cached tokens: 0
• Total tokens: 46,846
• Cost: $0.354972 (estimated)

Final response

High-level

The new deep-review extension is a solid, pragmatic implementation of a 2-phase flow:

• Phase 1: nested pi -p with an explicitly-scoped skill to produce a context pack
• Phase 2: direct Responses API call with SSE streaming, plus a compact live UI widget
• Good UX touches: /deep-review-stop, duration + token + cost summary, temp “handoff” artifacts, clipboard copy.

Overall structure and error-handling are thoughtful, and the code is readable despite being large.

---

Issues / risks worth addressing

1) --query can be silently overridden by positional args (surprising behavior)

In parseOptions():

if (positional.length > 0) {
    options.query = positional.join(" ").trim();
}

So /deep-review --query "foo" bar ends up using "bar" as the query. Even worse, /deep-review foo --query "bar" uses "foo".

Fix

only use positional query if --query wasn’t provided, or error if both were provided.

Suggested change:

if (positional.length > 0) {
    if (options.query) {
        return { ok: false, message: "Query provided both positionally and via --query; choose one." };
    }
    options.query = positional.join(" ").trim();
}

2) Windows portability is effectively broken (even though clipboard support is included)

Two main problems:

• splitArgs() treats \ as an escape character. A typical Windows path like C:\repo will be mangled unless the user doubles backslashes or uses forward slashes.
• extractContextPackPath() only recognizes POSIX paths (startsWith("/") and regex like /\/...\.txt/).

If Windows isn’t a target: document “POSIX only” and remove/ignore Windows clipboard bits.
If Windows is a target: you’ll need to:

• make splitArgs platform-aware (don’t treat \ as escape on win32, or only treat it as escape inside quotes),
• accept C:\...\.txt candidates and/or use path.win32.isAbsolute() when process.platform === "win32",
• ideally verify candidates exist with fs.access() and pick the first real file.

3) Default skill path is very machine-specific

~/dev/pi-skills/pr-context-packer/SKILL.md will fail for most users. Since this is an extension repo, it’s likely to be used elsewhere.

Options:

• Require DEEP_REVIEW_CONTEXT_PACKER_SKILL (no default), with a clear error message.
• Or search a few common locations (Pi skills dir, repo-relative, etc.).
• Also consider tilde expansion for the env override (~/... currently won’t resolve via access()).

4) /deep-review-stop “cancelResponseStream” won’t cancel once streaming starts

cancelStream() checks body.locked and no-ops if locked (which it will be once getReader() is acquired in parseSseStream()).

You do abort the fetch via AbortController, which is the real cancellation mechanism—so this isn’t fatal, but the extra cancellation hook is misleading.

If you want a real “cancel stream”:

• create the reader in streamResponses() and cancel the reader (not body.cancel()), or
• drop cancelResponseStream entirely and rely on AbortController only.

5) Context-pack failure reporting can hide stderr

On non-zero exit you do:

const failureDetails = truncate(cleanedPackOutput || cleanedPackStderr, 12000)

If stdout has anything, stderr is ignored—even if stderr contains the useful error. Prefer including both (each truncated) or prioritizing stderr when exitCode != 0.

---

Non-blocking improvements (nice to have)

• UI ticker: setInterval(..., 100ms) runs even when ctx.hasUI === false (renderLiveWidget returns early, but the interval still burns cycles). Only start the ticker when ctx.hasUI is true. Also 200–250ms usually feels fine and reduces churn.
• SSE parsing efficiency: using buffer.match(/\r?\n\r?\n/) repeatedly is okay, but indexOf("\n\n") / indexOf("\r\n\r\n") can be simpler and faster.
• Pack path robustness: after extracting a candidate path, await access(candidate) before accepting it. This prevents “wrong .txt path” selection if output includes multiple .txt paths.
• Debug mode size: request_meta includes the full contextText inside payload, which can make responses-events.jsonl enormous. Consider writing the payload to its own file and keeping the events file small (or omit the context text in debug meta and reference the pack path).
• Token/key sourcing: resolveBearerToken() only checks ctx.modelRegistry.getApiKey() if getModel("openai", modelId) exists. If the user picks a model id not in pi-ai metadata but still wants to use stored OpenAI creds, they’ll be forced to set env vars. If there’s a provider-level “openai key” retrieval in Pi, consider using it.
• Tests: this file has several testable pure helpers (splitArgs, parseOptions, extractContextPackPath, normalizeSectionLikeBoldMarkdown, SSE parsing). A small test suite would lock behavior down quickly.

---

What’s already strong

• Clean separation of phases and clear user-facing reporting.
• Good abort hygiene for the subprocess (SIGTERM then SIGKILL timer).
• Stream event handling and usage/cost calculation are practical.
• Artifact writing is useful and the metadata JSON is a nice touch.

If you address the query-parsing precedence + (either) document POSIX-only or fix Windows path handling, this will be in very good shape.

Handoff

• Output directory: /var/folders/mx/z9593lg90cdg6_br3f36gs4h0000gn/T/deep-review-output-98Rq3c
• Answer file: /var/folders/mx/z9593lg90cdg6_br3f36gs4h0000gn/T/deep-review-output-98Rq3c/answer.txt
• Thinking file: /var/folders/mx/z9593lg90cdg6_br3f36gs4h0000gn/T/deep-review-output-98Rq3c/thinking.txt
• Report file: /var/folders/mx/z9593lg90cdg6_br3f36gs4h0000gn/T/deep-review-output-98Rq3c/report.md
• Metadata file: /var/folders/mx/z9593lg90cdg6_br3f36gs4h0000gn/T/deep-review-output-98Rq3c/metadata.json
• Clipboard: copied answer via pbcopy