Repository navigation
fix(responses): strip ChatGPT citation control markers before the client #3195
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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)); | ||
|
Comment on lines
+86
to
+92
There was a problem hiding this comment.
When an open span receives another Useful? React with 👍 / 👎. |
||
| }, | ||
| flush(): string { | ||
| const rest = held; | ||
| held = ""; | ||
| return rest; | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 "); | ||
| }); | ||
| }); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the reported
github-copilot/gpt-5.6-solroute,src/providers/registry.ts:2872-2879selects theopenai-responsesadapter, whosepassthrough: truecausessrc/server/responses/core.ts:3611to return through the native passthrough branch. That branch appliescreateGithubCopilotResponsesBlockRewriteatsrc/server/responses/core.ts:4615-4617and never callsbridgeToResponsesSSE, so this new filter is bypassed and the citation markers still reach the Codex client in the exact scenario this commit claims to fix. Add equivalent stateful filtering to the Copilot passthrough block rewrite, including delta, done, and completed snapshot text fields.Useful? React with 👍 / 👎.