diff --git a/src/features/selection-rewrite/app-server-transport.ts b/src/features/selection-rewrite/app-server-transport.ts index 5f744a2e..bbecc84b 100644 --- a/src/features/selection-rewrite/app-server-transport.ts +++ b/src/features/selection-rewrite/app-server-transport.ts @@ -3,9 +3,13 @@ import { runEphemeralStructuredTurnForLastAgentText, type StructuredTurnOutputSchema, } from "../../app-server/services/ephemeral-structured-turn"; -import { SelectionRewriteOutputError, selectionRewriteOutputParseResultFromText } from "./output"; import { SELECTION_REWRITE_DEVELOPER_INSTRUCTIONS, SELECTION_REWRITE_SERVICE_NAME } from "./prompt"; -import type { SelectionRewriteTransport, SelectionRewriteTransportRequest } from "./transport"; +import { + type SelectionRewriteOutput, + SelectionRewriteOutputError, + type SelectionRewriteTransport, + type SelectionRewriteTransportRequest, +} from "./transport"; const SELECTION_REWRITE_TIMEOUT_MS = 120_000; @@ -60,7 +64,19 @@ async function runAppServerSelectionRewrite(options: AppServerSelectionRewriteTr }, options.runner, ); - const { output, rawText } = selectionRewriteOutputParseResultFromText(lastAgentText); - if (!output) throw new SelectionRewriteOutputError("Codex did not return a valid selection rewrite response.", rawText); + const output = selectionRewriteOutputFromText(lastAgentText); + if (!output) throw new SelectionRewriteOutputError("Codex did not return a valid selection rewrite response.", lastAgentText); return output; } + +function selectionRewriteOutputFromText(text: string | null): SelectionRewriteOutput | null { + if (!text) return null; + try { + const parsed = JSON.parse(text.trim()) as unknown; + if (!parsed || typeof parsed !== "object") return null; + const replacementText = (parsed as { replacementText?: unknown }).replacementText; + return typeof replacementText === "string" ? { replacementText } : null; + } catch { + return null; + } +} diff --git a/src/features/selection-rewrite/output.ts b/src/features/selection-rewrite/output.ts deleted file mode 100644 index b7cf971b..00000000 --- a/src/features/selection-rewrite/output.ts +++ /dev/null @@ -1,35 +0,0 @@ -export interface SelectionRewriteOutput { - replacementText: string; -} - -export interface SelectionRewriteOutputParseResult { - output: SelectionRewriteOutput | null; - rawText: string | null; -} - -export class SelectionRewriteOutputError extends Error { - constructor( - message: string, - readonly rawText: string | null, - ) { - super(message); - this.name = "SelectionRewriteOutputError"; - } -} - -function parseSelectionRewriteOutput(text: string): SelectionRewriteOutput | null { - try { - const parsed = JSON.parse(text.trim()) as unknown; - if (!parsed || typeof parsed !== "object") return null; - const replacementText = (parsed as { replacementText?: unknown }).replacementText; - if (typeof replacementText !== "string") return null; - return { replacementText }; - } catch { - return null; - } -} - -export function selectionRewriteOutputParseResultFromText(text: string | null): SelectionRewriteOutputParseResult { - if (!text) return { output: null, rawText: null }; - return { output: parseSelectionRewriteOutput(text), rawText: text }; -} diff --git a/src/features/selection-rewrite/session.ts b/src/features/selection-rewrite/session.ts index 7e80c293..70ee6ef7 100644 --- a/src/features/selection-rewrite/session.ts +++ b/src/features/selection-rewrite/session.ts @@ -1,7 +1,6 @@ import type { SelectionRewriteInstructionHistoryDirection, SelectionRewriteRuntimeSettings, SelectionRewriteState } from "./model"; -import { SelectionRewriteOutputError } from "./output"; import { buildSelectionRewritePrompt } from "./prompt"; -import type { SelectionRewriteActivity, SelectionRewriteTransport } from "./transport"; +import { type SelectionRewriteActivity, SelectionRewriteOutputError, type SelectionRewriteTransport } from "./transport"; const MAX_SELECTION_REWRITE_INSTRUCTION_HISTORY = 20; diff --git a/src/features/selection-rewrite/transport.ts b/src/features/selection-rewrite/transport.ts index dc653f85..96351549 100644 --- a/src/features/selection-rewrite/transport.ts +++ b/src/features/selection-rewrite/transport.ts @@ -1,8 +1,21 @@ import type { SelectionRewriteRuntimeSettings } from "./model"; -import type { SelectionRewriteOutput } from "./output"; export type SelectionRewriteActivity = "reasoning" | "writing"; +export interface SelectionRewriteOutput { + replacementText: string; +} + +export class SelectionRewriteOutputError extends Error { + constructor( + message: string, + readonly rawText: string | null, + ) { + super(message); + this.name = "SelectionRewriteOutputError"; + } +} + export interface SelectionRewriteTransportRequest { prompt: string; runtimeSettings: SelectionRewriteRuntimeSettings; diff --git a/tests/features/selection-rewrite/selection-rewrite.test.ts b/tests/features/selection-rewrite/selection-rewrite.test.ts index 397c699f..76c95f88 100644 --- a/tests/features/selection-rewrite/selection-rewrite.test.ts +++ b/tests/features/selection-rewrite/selection-rewrite.test.ts @@ -7,11 +7,10 @@ import type { EphemeralStructuredTurnRunner } from "../../../src/app-server/serv import { createAppServerSelectionRewriteTransport } from "../../../src/features/selection-rewrite/app-server-transport"; import { buildSelectionDiffLines } from "../../../src/features/selection-rewrite/diff"; import { canApplySelectionRewrite, type SelectionRewriteState } from "../../../src/features/selection-rewrite/model"; -import { SelectionRewriteOutputError, selectionRewriteOutputParseResultFromText } from "../../../src/features/selection-rewrite/output"; import { SelectionRewritePopover } from "../../../src/features/selection-rewrite/popover.dom"; import { positionSelectionRewritePopover } from "../../../src/features/selection-rewrite/position.dom"; import { buildSelectionRewritePrompt } from "../../../src/features/selection-rewrite/prompt"; -import type { SelectionRewriteTransportRequest } from "../../../src/features/selection-rewrite/transport"; +import { SelectionRewriteOutputError, type SelectionRewriteTransportRequest } from "../../../src/features/selection-rewrite/transport"; import { deferred } from "../../support/async"; import { installObsidianDomShims } from "../../support/dom"; @@ -31,25 +30,6 @@ afterEach(() => { vi.restoreAllMocks(); }); -describe("selection selection rewrite output", () => { - it("parses valid selection rewrite JSON", () => { - expect(selectionRewriteOutputParseResultFromText('{"replacementText":"rewritten"}').output).toEqual({ replacementText: "rewritten" }); - }); - - it("rejects invalid selection rewrite JSON", () => { - expect(selectionRewriteOutputParseResultFromText("replacementText: rewritten").output).toBeNull(); - expect(selectionRewriteOutputParseResultFromText('{"replacementText":42}').output).toBeNull(); - expect(selectionRewriteOutputParseResultFromText('{"text":"rewritten"}').output).toBeNull(); - }); - - it("keeps raw model text when selection rewrite output parsing fails", () => { - expect(selectionRewriteOutputParseResultFromText("replacementText: final")).toEqual({ - output: null, - rawText: "replacementText: final", - }); - }); -}); - describe("selection rewrite prompt", () => { it("always includes note context with the selected text", () => { const prompt = buildSelectionRewritePrompt(rewriteState()); @@ -243,10 +223,14 @@ describe("selection rewrite app-server transport", () => { ); }); - it("reports invalid structured output with the raw assistant text", async () => { - const runner = vi.fn(async () => turn([agentMessage("answer", "invalid raw output")])); + it.each([ + "invalid raw output", + '{"replacementText":42}', + '{"text":"rewritten"}', + ])("reports invalid structured output with the raw assistant text: %s", async (rawText) => { + const runner = vi.fn(async () => turn([agentMessage("answer", rawText)])); - await expect(runSelectionRewrite(runOptions(runner))).rejects.toMatchObject({ rawText: "invalid raw output" }); + await expect(runSelectionRewrite(runOptions(runner))).rejects.toMatchObject({ rawText }); }); });