diff --git a/src/bridge.ts b/src/bridge.ts index c1666cb84be..c50cac8c04e 100644 --- a/src/bridge.ts +++ b/src/bridge.ts @@ -28,6 +28,11 @@ import { awaitThoughtSignatureDurability, } from "./responses/thought-signature-replay"; import { resolveStallTimeoutSec } from "./stall-timeout"; +import { + createCitationMarkerFilter, + stripCitationMarkers, + type CitationMarkerFilter, +} from "./responses/citation-markers"; import { normalizeDeclaredToolName } from "./types"; import { usageDisplayTotalTokens } from "./usage/totals"; import { appendSafeWebSearchSource, safeWebSearchSources } from "./web-search/sources"; @@ -435,7 +440,14 @@ export function bridgeToResponsesSSE( const stallSec = resolveStallTimeoutSec(options?.stallTimeoutSec); const maxStallTicks = Math.ceil((stallSec * 1000) / heartbeatMs); - let currentMsg: { itemId: string; outputIndex: number; text: string; textBytes: number; phase?: OcxMessagePhase } | null = null; + let currentMsg: { + itemId: string; + outputIndex: number; + text: string; + textBytes: number; + citationFilter: CitationMarkerFilter; + phase?: OcxMessagePhase; + } | null = null; let currentReasoning: { itemId: string; outputIndex: number; text: string; textBytes: number } | null = null; let currentRawReasoning: { itemId: string; outputIndex: number; text: string; textBytes: number } | null = null; // Anthropic extended-thinking round-trip state: the signature signs the CURRENT thinking @@ -565,6 +577,17 @@ export function bridgeToResponsesSSE( const closeCurrentMessage = (inferredPhase?: OcxMessagePhase) => { if (!currentMsg) return; + // Release anything the citation filter was holding for this message, then strip the + // accumulated text: closeCurrentMessage re-sends it in output_text.done and + // output_item.done, so filtering only the deltas would leave the markers in both. + const trailing = currentMsg.citationFilter.flush(); + if (trailing) { + emit("response.output_text.delta", { + item_id: currentMsg.itemId, output_index: currentMsg.outputIndex, + content_index: 0, delta: trailing, + }); + } + const messageText = stripCitationMarkers(currentMsg.text); // Chat Completions has no message-phase field. Keep its live item provisional, then // classify it only when the next adapter event proves whether this text led into more // work or completed the turn. Explicit adapter phases always outrank this inference. @@ -574,15 +597,15 @@ export function bridgeToResponsesSSE( // Finalize the text part (Responses protocol). Without these .done events Codex never // commits the content part and renders the message as truncated / cut off. emit("response.output_text.done", { - item_id: currentMsg.itemId, output_index: currentMsg.outputIndex, content_index: 0, text: currentMsg.text, + item_id: currentMsg.itemId, output_index: currentMsg.outputIndex, content_index: 0, text: messageText, }); emit("response.content_part.done", { item_id: currentMsg.itemId, output_index: currentMsg.outputIndex, content_index: 0, - part: { type: "output_text", text: currentMsg.text, annotations }, + part: { type: "output_text", text: messageText, annotations }, }); const item = { type: "message", id: currentMsg.itemId, status: "completed", role: "assistant", - content: [{ type: "output_text", text: currentMsg.text, annotations }], + content: [{ type: "output_text", text: messageText, annotations }], ...(phase ? { phase } : {}), }; emit("response.output_item.done", { output_index: currentMsg.outputIndex, item }); @@ -936,7 +959,11 @@ export function bridgeToResponsesSSE( item_id: itemId, output_index: outputIndex, content_index: 0, part: { type: "output_text", text: "", annotations: [] }, }); - currentMsg = { itemId, outputIndex, text: "", textBytes: 0, ...(event.phase ? { phase: event.phase } : {}) }; + currentMsg = { + itemId, outputIndex, text: "", textBytes: 0, + citationFilter: createCitationMarkerFilter(), + ...(event.phase ? { phase: event.phase } : {}), + }; } ({ value: currentMsg.text, bytes: currentMsg.textBytes } = appendString( currentMsg.text, @@ -944,10 +971,16 @@ export function bridgeToResponsesSSE( event.text, "retained_collectors", )); - emit("response.output_text.delta", { - item_id: currentMsg.itemId, output_index: currentMsg.outputIndex, - content_index: 0, delta: event.text, - }); + // A citation span can straddle a delta boundary, so the filter withholds an + // unterminated tail and releases it at close (#3150). The accumulator above + // keeps the raw text; it is stripped once in closeCurrentMessage. + const visible = currentMsg.citationFilter.push(event.text); + if (visible) { + emit("response.output_text.delta", { + item_id: currentMsg.itemId, output_index: currentMsg.outputIndex, + content_index: 0, delta: visible, + }); + } break; } case "thinking_delta": { @@ -1621,6 +1654,10 @@ function buildResponseJSONWithBudget( const flushText = (inferredPhase?: OcxMessagePhase) => { if (!currentText) return; const phase = currentTextPhase ?? inferredPhase; + // ChatGPT-backend citation markers arrive as literal private-use characters that the + // Codex TUI prints verbatim (#3150). Strip them here rather than at the accumulator so + // the retained byte accounting above still describes what the upstream actually sent. + const text = stripCitationMarkers(currentText); const sourceBytes = pendingWebSources.reduce((sum, source) => sum + bytesOf(JSON.stringify(source)), 0); const annotations = pendingWebSources.map(s => ({ type: "url_citation", url: s.url, ...(s.title ? { title: s.title } : {}), start_index: 0, end_index: 0, @@ -1628,7 +1665,7 @@ function buildResponseJSONWithBudget( pendingWebSources = []; const item = { type: "message", id: `msg_${uuid()}`, role: "assistant", status: "completed", - content: [{ type: "output_text", text: currentText, annotations }], + content: [{ type: "output_text", text, annotations }], ...(phase ? { phase } : {}), } as OutputItem; pushOutput(item, currentTextBytes); diff --git a/src/responses/citation-markers.ts b/src/responses/citation-markers.ts new file mode 100644 index 00000000000..5fe58142cf0 --- /dev/null +++ b/src/responses/citation-markers.ts @@ -0,0 +1,101 @@ +/** + * ChatGPT-backend citation markers. + * + * The ChatGPT backend delimits inline citations with Unicode private-use characters: + * + * \uE200 cite \uE202 turn1view0 \uE202 turn1view1 \uE201 + * + * The desktop client renders that as source chips. The Codex TUI does not: it prints the + * codepoints literally, so the user sees "citeturn1view0turn1view1" in the answer and in + * the saved transcript (#3150). + * + * OpenCodex neither produces nor understands this grammar — it arrives as ordinary + * assistant text from a ChatGPT-derived backend (GitHub Copilot in the report). The proxy + * is the last place that can remove it before a client that cannot render it. + * + * Strip, do not translate. The `turnNviewN` ids are turn-scoped and opaque, and the + * response carries no mapping from them to a URL, so there is nothing to convert them + * into. Structured `url_citation` annotations are a separate path and are untouched. + */ + +/** Opens a citation span. */ +export const CITATION_MARKER_START = "\uE200"; +/** Separates the `cite` keyword and each source reference inside a span. */ +export const CITATION_MARKER_SEPARATOR = "\uE202"; +/** Closes a citation span. */ +export const CITATION_MARKER_END = "\uE201"; + +/** True when the text contains any of the three delimiters. Cheap pre-check. */ +export function hasCitationMarker(text: string): boolean { + return text.includes(CITATION_MARKER_START) + || text.includes(CITATION_MARKER_SEPARATOR) + || text.includes(CITATION_MARKER_END); +} + +/** + * Remove every complete `START … END` span from a whole string. + * + * A START with no END is left alone rather than truncating the remainder: an unterminated + * marker is malformed input, and dropping everything after it would delete real answer + * text. A stray SEPARATOR or END outside a span is also left alone for the same reason — + * this function only removes what it can prove is a citation span. + */ +export function stripCitationMarkers(text: string): string { + if (!text.includes(CITATION_MARKER_START)) return text; + let out = ""; + let index = 0; + for (;;) { + const start = text.indexOf(CITATION_MARKER_START, index); + if (start === -1) { + out += text.slice(index); + return out; + } + const end = text.indexOf(CITATION_MARKER_END, start + 1); + if (end === -1) { + // Unterminated: keep the rest verbatim. + out += text.slice(index); + return out; + } + out += text.slice(index, start); + index = end + 1; + } +} + +export interface CitationMarkerFilter { + /** Feed one streaming delta; returns the portion safe to emit now. */ + push(delta: string): string; + /** Release anything still held when the message closes. */ + flush(): string; +} + +/** + * Streaming filter. + * + * A marker can straddle a delta boundary — `\uE200cite` in one chunk and the rest in the + * next — so a stateless per-delta strip would emit the tail of a span it never recognized. + * This holds back the text from an unterminated START and releases it once the END arrives + * (removed) or the stream ends (verbatim, so nothing the model actually said is lost). + */ +export function createCitationMarkerFilter(): CitationMarkerFilter { + // Text from an open START that has not been terminated yet. + let held = ""; + return { + push(delta: string): string { + const combined = held + delta; + held = ""; + const start = combined.lastIndexOf(CITATION_MARKER_START); + if (start === -1) return stripCitationMarkers(combined); + const endAfterStart = combined.indexOf(CITATION_MARKER_END, start + 1); + if (endAfterStart !== -1) return stripCitationMarkers(combined); + // The trailing span is still open: emit everything before it, hold the rest. + held = combined.slice(start); + return stripCitationMarkers(combined.slice(0, start)); + }, + flush(): string { + const rest = held; + held = ""; + return rest; + }, + }; +} + diff --git a/tests/bridge.test.ts b/tests/bridge.test.ts index f031ce8f8bd..2d433f94675 100644 --- a/tests/bridge.test.ts +++ b/tests/bridge.test.ts @@ -1253,6 +1253,59 @@ describe("Responses bridge web_search_call native item", () => { }); }); +describe("citation markers never reach the client (#3150)", () => { + const S = "\uE200"; + const P = "\uE202"; + const E = "\uE201"; + + test("a span split across text deltas is absent from every emitted event", async () => { + // End-to-end through the real bridge, not just the filter. closeCurrentMessage re-sends + // the accumulated text in output_text.done, content_part.done and output_item.done, so + // filtering only the deltas would still leak the markers into the saved transcript. + const events = await collectSse(bridgeToResponsesSSE(replay([ + { type: "text_delta", text: `The setting is supported. ${S}cite${P}` }, + { type: "text_delta", text: `turn1view0${P}turn1view1${E}` }, + { type: "text_delta", text: " Next sentence." }, + { type: "done" }, + ]), "routed/model")); + + const serialized = JSON.stringify(events); + expect(serialized).not.toContain(S); + expect(serialized).not.toContain(P); + expect(serialized).not.toContain(E); + expect(serialized).not.toContain("turn1view0"); + + const streamed = events + .filter(e => e.event === "response.output_text.delta") + .map(e => e.data.delta as string) + .join(""); + expect(streamed).toBe("The setting is supported. Next sentence."); + + const done = events.find(e => e.event === "response.output_text.done"); + expect(done?.data.text).toBe("The setting is supported. Next sentence."); + }); + + test("a stream ending inside a span still delivers the held text", async () => { + // Withhold, not drop: an unterminated marker is malformed input, and swallowing it + // would delete words the model actually produced. + const events = await collectSse(bridgeToResponsesSSE(replay([ + { type: "text_delta", text: `partial ${S}cite${P}turn1` }, + { type: "done" }, + ]), "routed/model")); + const done = events.find(e => e.event === "response.output_text.done"); + expect(done?.data.text).toContain("partial "); + }); + + test("ordinary text is untouched", async () => { + const events = await collectSse(bridgeToResponsesSSE(replay([ + { type: "text_delta", text: "plain answer" }, + { type: "done" }, + ]), "routed/model")); + const done = events.find(e => e.event === "response.output_text.done"); + expect(done?.data.text).toBe("plain answer"); + }); +}); + describe("Responses bridge stopReason threading (issue #246)", () => { test("done with stopReason max_tokens emits response.incomplete", async () => { const frames = await collectSse(bridgeToResponsesSSE(replay([ diff --git a/tests/citation-markers.test.ts b/tests/citation-markers.test.ts new file mode 100644 index 00000000000..726585457c4 --- /dev/null +++ b/tests/citation-markers.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, test } from "bun:test"; +import { + CITATION_MARKER_END, + CITATION_MARKER_SEPARATOR, + CITATION_MARKER_START, + createCitationMarkerFilter, + hasCitationMarker, + stripCitationMarkers, +} from "../src/responses/citation-markers"; + +/** + * #3150: the ChatGPT backend delimits inline citations with private-use characters + * (U+E200 open, U+E202 separate, U+E201 close). The desktop client renders them as source + * chips; the Codex TUI prints them literally, so the user saw + * "citeturn1view0turn1view1" in the answer and in the saved transcript. + * + * OpenCodex neither emits nor understands the grammar - it is upstream text passing + * through - so the proxy strips it before a client that cannot render it. + */ + +const S = CITATION_MARKER_START; +const P = CITATION_MARKER_SEPARATOR; +const E = CITATION_MARKER_END; +const span = `${S}cite${P}turn1view0${P}turn1view1${E}`; + +describe("citation marker stripping (#3150)", () => { + test("a complete span is removed and the surrounding text survives", () => { + expect(stripCitationMarkers(`The setting is supported. ${span} Next.`)) + .toBe("The setting is supported. Next."); + }); + + test("several spans in one message are all removed", () => { + expect(stripCitationMarkers(`a${span}b${S}cite${P}turn2view0${E}c`)).toBe("abc"); + }); + + test("text with no markers is returned unchanged", () => { + // The common case must not be rewritten at all. + const plain = "ordinary answer text with no private-use characters"; + expect(stripCitationMarkers(plain)).toBe(plain); + expect(hasCitationMarker(plain)).toBe(false); + }); + + test("an unterminated span keeps its text instead of truncating the answer", () => { + // Malformed input must not delete everything after the opening marker: that would + // silently drop real answer text. + // The opening marker is kept too: without a terminator there is no proof this is a + // citation span at all, so the input is returned verbatim rather than partly rewritten. + expect(stripCitationMarkers(`tail ${S}cite${P}turn1`)).toBe(`tail ${S}cite${P}turn1`); + }); + + test("a stray separator or terminator alone is left alone", () => { + expect(stripCitationMarkers(`a${P}b`)).toBe(`a${P}b`); + expect(stripCitationMarkers(`a${E}b`)).toBe(`a${E}b`); + }); +}); + +describe("streaming citation marker filter (#3150)", () => { + const drain = (chunks: readonly string[]): string => { + const filter = createCitationMarkerFilter(); + let out = ""; + for (const chunk of chunks) out += filter.push(chunk); + return out + filter.flush(); + }; + + test("a span split across deltas is removed, not leaked", () => { + // The case a stateless per-delta strip gets wrong: the opening marker arrives in one + // chunk and the terminator in the next, so the tail would be emitted unrecognized. + expect(drain([`The setting is supported. ${S}cite${P}`, `turn1view0${P}turn1view1${E}`, " Next."])) + .toBe("The setting is supported. Next."); + }); + + test("a span split one character at a time is still removed", () => { + expect(drain([...`ok ${span} done`])).toBe("ok done"); + }); + + test("a stream ending mid-span releases the held text rather than swallowing it", () => { + // Withhold, not drop: if the stream dies inside a marker the bytes still reach the user. + expect(drain([`abc ${S}cite${P}turn1`])).toBe(`abc ${S}cite${P}turn1`); + }); + + test("marker-free deltas pass through byte-identical", () => { + expect(drain(["hello ", "world", "!"])).toBe("hello world!"); + }); + + test("text before an open span is emitted immediately, not held to the end", () => { + // Streaming must stay streaming: only the unterminated span is withheld. + const filter = createCitationMarkerFilter(); + expect(filter.push(`visible now ${S}cite`)).toBe("visible now "); + }); +});