1// SPDX-FileCopyrightText: Amolith <amolith@secluded.site>
  2// SPDX-FileCopyrightText: Armin Ronacher <armin.ronacher@active-4.com>
  3//
  4// SPDX-License-Identifier: Apache-2.0
  5
  6import type { Tool } from "@mariozechner/pi-ai";
  7import { Type } from "@sinclair/typebox";
  8
  9export interface ExtractedQuestion {
 10	question: string;
 11	context?: string;
 12}
 13
 14export interface ExtractionResult {
 15	questions: ExtractedQuestion[];
 16}
 17
 18export const SYSTEM_PROMPT = `You extract questions from assistant messages so a user can answer them without reading the full message. Call the extract_questions tool with your results.
 19
 20Each extracted question must be a standalone object with a "question" string and an optional "context" string. The user will read ONLY the extracted questions, not the original message, so both the question and context must be fully self-contained.
 21
 22Rules:
 23- Each question must make sense on its own without referring back to the message (no "Issue 1", "the above", "as mentioned")
 24- Context should include all relevant details, options, and reasoning needed to answer — it replaces reading the original message
 25- Merge duplicate or overlapping questions into one (e.g. an analysis section and a plan section about the same topic become one question)
 26- Capture both explicit questions ("Which approach?") and implicit ones (proposals that need approval, recommendations that need confirmation)
 27- Keep questions in logical order
 28- If no questions are found, call the tool with an empty questions array
 29
 30<examples>
 31<example>
 32<description>Simple explicit question</description>
 33<input>I can set up the database with either PostgreSQL or MySQL. The existing infrastructure uses PostgreSQL for two other services. Which database should I use?</input>
 34<output>
 35{"question": "Which database should be used for the new service?", "context": "Options are PostgreSQL or MySQL. The existing infrastructure already uses PostgreSQL for two other services."}
 36</output>
 37</example>
 38
 39<example>
 40<description>Implicit proposal needing approval</description>
 41<input>I looked at the failing test. The flakiness comes from a race condition in the cleanup goroutine. I think the simplest fix is to add a sync.WaitGroup so the test waits for cleanup to finish before asserting. Want me to go ahead with that?</input>
 42<output>
 43{"question": "Should the flaky test be fixed by adding a sync.WaitGroup to wait for the cleanup goroutine before asserting?", "context": "The test failure is caused by a race condition in the cleanup goroutine. The WaitGroup approach ensures cleanup completes before assertions run."}
 44</output>
 45</example>
 46
 47<example>
 48<description>Long message with analysis and plan sections that overlap — questions must be merged, deduplicated, and fully standalone</description>
 49<input>
 50## Issue 1: Non-deterministic map iteration for overrides → rawArgs
 51
 52The problem is in RunE at line ~75: for k, vals := range overrides gives random order each run.
 53
 54Three good approaches:
 551. Sort the keys before iterating. Simplest, most common Go pattern.
 562. Use []restic.Flag or []struct{key, vals} instead of a map. More invasive but preserves construction order.
 573. Skip the map→rawArgs→parsePassthrough round-trip entirely. Instead of serializing overrides to rawArgs and then having runCommand parse them back, just pass the overrides map directly to runCommand and merge there. This eliminates the round-trip and the ordering problem disappears.
 58
 59Recommend approach 3 as best: it kills the root cause.
 60
 61## Issue 2: Double-pointers (**screens.Snapshot etc.)
 62
 63The problem is buildCommandScreens needs to both return screens AND communicate which specific screen instances were created back to runInteractive. Double-pointers are gnarly.
 64
 65Simpler approach: return a struct holding typed screen pointers. The ResolveFunc closure captures the struct.
 66
 67## Issue 3: Picker height overflow
 68
 69The session already subtracts chrome height via adjustedSizeMsg() before forwarding WindowSizeMsg to screens. The legacy pickerModel does its own subtraction because it runs standalone. Assessment: this finding is a false positive.
 70
 71## Issue 2.1: huh form boilerplate
 72
 73The buildForm/Init/Update boilerplate is repetitive across Overwrite, Target, Preset, Snapshot. Code is simple and clear in each screen. Recommendation: skip — borderline DRY.
 74
 75## Issue 2.2: drain test helpers
 76
 77The drain* test helpers are copy-pasted with only the type changed. Perfect case for Go generics: func drain[S ui.Screen](s S, cmd tea.Cmd) (S, tea.Cmd)
 78
 79## Issue 3.1: Snapshot theme handler clears user input
 80
 81When BackgroundColorMsg arrives during phaseSnapshotManual, buildManualForm() resets s.entered = "". Fix: preserve s.entered before rebuilding. Also needs rebuildSelectForm() call during phaseSnapshotSelecting.
 82
 83## Issue 4: WindowSizeMsg missed during phaseFileLoading
 84
 85If the screen is loading when the initial WindowSizeMsg arrives, it only goes to the spinner and the size is lost. Fix: store the last WindowSizeMsg on the FilePicker struct, apply when picker is built.
 86
 87## Issue 5.1: Same as Issue 1
 88
 89Same root cause — map iteration in RunE. Fix for Issue 1 handles this.
 90
 91## Issue 6.1: Duplicated constant
 92
 93Both internal/config and internal/restic define the same commandSuffix constant.
 94
 95## Issue 6.2: Inconsistent empty-string semantics
 96
 97rc.Environ treats "" as present (checks map key existence), but os.Getenv checks != "" so an empty env var counts as absent.
 98
 99## Plan
100
1011. Remove legacy restore prompts (promptRestore, promptSnapshotID, promptFileSelection, printRestoreSummary, standalone pickerModel)
1022. Eliminate map→rawArgs round-trip (Issues 1 and 5.1)
1033. Replace double-pointers with return struct (Issue 2)
1044. Extract generic drain test helper (Issue 2.2)
1055. Fix snapshot theme handler (Issue 3.1)
1066. Store WindowSizeMsg during loading (Issue 4)
1077. Share commandSuffix constant (Issue 6.1)
1088. Fix inconsistent empty-string check (Issue 6.2)
109
110NOT planning to do Issue 2.1 (huh form boilerplate extraction).
111</input>
112<output>
113{"question": "How should the non-deterministic map iteration in RunE be fixed, where 'for k, vals := range overrides' produces random argument order each run?", "context": "Three approaches: (1) Sort map keys before iterating — simplest, small code change. (2) Replace map[string][]string with a slice of key-value pairs to preserve construction order. (3) Eliminate the map→rawArgs→parsePassthrough round-trip entirely by passing overrides directly to runCommand — this also fixes the identical issue in rawArgs parsing. Recommended: approach 3."}
114{"question": "Should the double-pointer output parameters (**screens.Snapshot, etc.) in buildCommandScreens be replaced with a returned struct?", "context": "Currently uses double-pointers so runInteractive can read results from specific screen instances. A commandScreens struct holding typed screen pointers would be returned instead, and the ResolveFunc closure captures it. Idiomatic Go — return a struct instead of output parameters."}
115{"question": "The picker height overflow appears to be a false positive because the session already subtracts chrome height via adjustedSizeMsg() before forwarding WindowSizeMsg to screens. Safe to skip?", "context": "session.go subtracts 5 chrome lines (breadcrumb + title + help bar) before forwarding. The legacy pickerModel does its own subtraction because it runs standalone with tea.NewProgram. The screen adapter doesn't need additional subtraction."}
116{"question": "Should the repeated huh form boilerplate (buildForm/Init/Update) across Overwrite, Target, Preset, and Snapshot screens be extracted into a shared helper?", "context": "Each screen repeats same form setup pattern with slight behavior variations. Code is currently simple and self-contained in each screen. Recommended: skip — borderline DRY."}
117{"question": "Should the copy-pasted type-specific drain test helpers be replaced with a single generic drain[S ui.Screen] function?", "context": "Multiple test files have identical drain helpers differing only in type parameter (e.g. drainSnapshot, drainFilePicker). A single generic function would replace all of them."}
118{"question": "Should the snapshot theme handler be fixed to preserve user input when the terminal theme changes?", "context": "When BackgroundColorMsg arrives during phaseSnapshotManual, buildManualForm() resets s.entered to empty, wiping user input. Fix: preserve s.entered before rebuild. Also needs rebuildSelectForm() call during phaseSnapshotSelecting."}
119{"question": "Should FilePicker store the last WindowSizeMsg to prevent size loss during the loading phase?", "context": "If the screen is loading when the initial WindowSizeMsg arrives, the message only goes to the spinner and size is lost. When buildPicker runs later, picker doesn't know terminal size. Fix: add lastSize field, apply when picker built."}
120{"question": "Should the duplicated commandSuffix constant be extracted to a shared package?", "context": "Both internal/config and internal/restic define the same constant."}
121{"question": "Should os.Getenv() != '' checks be replaced with os.LookupEnv so empty environment variables count as present?", "context": "rc.Environ treats empty string as present (checks map key existence), but os.Getenv checks != '' so an empty env var counts as absent. Using os.LookupEnv would make behavior consistent — explicitly setting RESTIC_REPOSITORY='' would count as configured."}
122{"question": "Should the legacy interactive restore prompts (promptRestore, promptSnapshotID, promptFileSelection, printRestoreSummary, standalone pickerModel) be removed from root.go?", "context": "The TUI session now handles the full restore flow through screens, making these standalone prompt functions unused."}
123</output>
124</example>
125</examples>`;
126
127/**
128 * Tool definition for structured question extraction. Models produce
129 * structured tool-call arguments far more reliably than free-form JSON
130 * in a text response.
131 */
132export const QUESTION_EXTRACTION_TOOL: Tool = {
133	name: "extract_questions",
134	description:
135		"Extract questions from the assistant's message. Each question is a self-contained object the user will read instead of the original message.",
136	parameters: Type.Object({
137		questions: Type.Array(
138			Type.Object({
139				question: Type.String({
140					description: "A complete, standalone question. Must make sense without reading the original message.",
141				}),
142				context: Type.Optional(
143					Type.String({
144						description:
145							"Self-contained context with all details, options, and reasoning needed to answer the question.",
146					}),
147				),
148			}),
149			{
150				description:
151					"Array of question objects. Merge overlapping questions. Each item MUST be an object with 'question' and optional 'context' strings, NOT a plain string.",
152			},
153		),
154	}),
155};