From e96b5c52e05e9fd6f5b9cb4aedc96b171f4a4872 Mon Sep 17 00:00:00 2001 From: t Date: Mon, 7 Sep 2026 19:19:06 +0900 Subject: [PATCH] fix(responses): make whole-string citation stripping agree with the streaming filter After #3868 the streaming filter keeps a malformed START verbatim when a later START opens a real span, but stripCitationMarkers still paired the first START with that later span's END and deleted everything between. bridge.ts re-strips the accumulated text for output_text.done and output_item.done, so the terminal text disagreed with the concatenated deltas. Walk START-delimited segments in the whole-string path too, and share the 4096 span bound with the whole-string path (an over-bound span that is terminated late is malformed text in both), and assert delta-vs-whole equality across several chunkings. Found by the lane A fresh-base composition audit on dev d00615d56. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> --- src/responses/citation-markers.ts | 53 +++++++++++++----------- tests/responses/citation-markers.test.ts | 33 +++++++++++++++ 2 files changed, 62 insertions(+), 24 deletions(-) diff --git a/src/responses/citation-markers.ts b/src/responses/citation-markers.ts index 3e3943cad5..9c56c7a197 100644 --- a/src/responses/citation-markers.ts +++ b/src/responses/citation-markers.ts @@ -42,23 +42,24 @@ export function hasCitationMarker(text: string): boolean { */ 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; + // Walk START-delimited segments exactly like the streaming filter below: a START whose + // own segment (up to the next START) contains an END within the span bound is a span and + // is removed; a START that is superseded by another START before any END, or whose span + // exceeds MAX_CITATION_SPAN_LENGTH, is malformed text and stays verbatim. Pairing an + // earlier malformed START with a later span's END would delete real answer text and, + // worse, disagree with what the streaming deltas already emitted (#3843). The bound is + // shared with the streaming filter for the same reason: a span it has already released + // as over-bound must not be swallowed here when the END finally arrives. + let start = text.indexOf(CITATION_MARKER_START); + let out = text.slice(0, start); + while (start !== -1) { + const nextStart = text.indexOf(CITATION_MARKER_START, start + 1); + const segment = text.slice(start, nextStart === -1 ? text.length : nextStart); + const end = segment.indexOf(CITATION_MARKER_END, 1); + out += end === -1 || end + 1 > MAX_CITATION_SPAN_LENGTH ? segment : segment.slice(end + 1); + start = nextStart; } + return out; } export interface CitationMarkerFilter { @@ -69,13 +70,16 @@ export interface CitationMarkerFilter { } /** - * Upper bound on the text withheld for one unterminated START. + * Upper bound on the length of a citation span (START through END inclusive), and therefore + * on the text the streaming filter withholds for one unterminated START. * * A real span is `cite` plus a few turn-scoped ids, so it is far under this. Without a * bound, a backend that emits a START and never terminates it makes `held` grow for the - * whole response, and every later delta re-scans that accumulated prefix. + * whole response, and every later delta re-scans that accumulated prefix. The whole-string + * strip applies the same bound so both paths classify a span identically regardless of how + * the text was chunked. */ -const MAX_STREAMING_MARKER_SPAN_LENGTH = 4_096; +const MAX_CITATION_SPAN_LENGTH = 4_096; /** * Streaming filter. @@ -85,7 +89,7 @@ const MAX_STREAMING_MARKER_SPAN_LENGTH = 4_096; * 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). * - * A span that grows past `MAX_STREAMING_MARKER_SPAN_LENGTH` is malformed ordinary text, so + * A span that grows past `MAX_CITATION_SPAN_LENGTH` is malformed ordinary text, so * it is released verbatim instead of withheld; a later START can still open a valid span. */ export function createCitationMarkerFilter(): CitationMarkerFilter { @@ -104,15 +108,16 @@ export function createCitationMarkerFilter(): CitationMarkerFilter { const nextStart = combined.indexOf(CITATION_MARKER_START, start + 1); const segment = combined.slice(start, nextStart === -1 ? combined.length : nextStart); const end = segment.indexOf(CITATION_MARKER_END, 1); - if (end !== -1) { + if (end !== -1 && end + 1 <= MAX_CITATION_SPAN_LENGTH) { // A complete span: drop it, keep whatever trails it inside this segment. out += segment.slice(end + 1); - } else if (nextStart === -1 && segment.length <= MAX_STREAMING_MARKER_SPAN_LENGTH) { + } else if (end === -1 && nextStart === -1 && segment.length <= MAX_CITATION_SPAN_LENGTH) { // Only a bounded trailing span can still be completed by a later delta. held = segment; } else { - // Superseded by a later START, or over the bound: ordinary text, emitted verbatim - // so neither the retained text nor the per-delta rescan grows without limit. + // Superseded by a later START, or over the bound (with or without a late END): + // ordinary text, emitted verbatim so neither the retained text nor the per-delta + // rescan grows without limit. out += segment; } start = nextStart; diff --git a/tests/responses/citation-markers.test.ts b/tests/responses/citation-markers.test.ts index dcc7f9abc6..b0d92d0fad 100644 --- a/tests/responses/citation-markers.test.ts +++ b/tests/responses/citation-markers.test.ts @@ -52,6 +52,15 @@ describe("citation marker stripping (#3150)", () => { expect(stripCitationMarkers(`a${P}b`)).toBe(`a${P}b`); expect(stripCitationMarkers(`a${E}b`)).toBe(`a${E}b`); }); + + test("a malformed START before a later valid span is kept, not paired with that span's END", () => { + // Whole-string stripping must agree with the streaming filter: the malformed prefix + // survives and only the real span is removed (bridge re-strips the accumulated text + // for output_text.done, so any disagreement would make done != concatenated deltas). + const malformed = `${S}${"y".repeat(5_000)}`; + expect(stripCitationMarkers(`a${malformed}${S}cite${P}turn1view0${E} tail`)).toBe(`a${malformed} tail`); + expect(stripCitationMarkers(`a${S}cite${S}cite${P}turn1view0${E}b`)).toBe(`a${S}citeb`); + }); }); describe("streaming citation marker filter (#3150)", () => { @@ -117,4 +126,28 @@ describe("streaming citation marker filter (#3150)", () => { .toBe(`a${malformed} tail`); expect(filter.flush()).toBe(""); }); + + test("concatenated streaming output equals whole-string stripping for every chunking", () => { + // The bridge emits deltas through the filter and then re-strips the accumulated text for + // output_text.done / output_item.done, so the two contracts must produce identical text. + const malformed = `${S}${"y".repeat(5_000)}`; + const inputs = [ + `a${span}${malformed}${S}cite${P}turn1view0${E} tail`, + `kept ${S}cite${"x".repeat(5_000)}`, + `a${S}cite${S}cite${P}turn1view0${E}b`, + `a${span}b${S}cite${P}turn2view0${E}c`, + // An over-bound span that is eventually terminated: the streaming filter has already + // released it verbatim, so whole-string stripping must keep it too. + `late ${S}${"z".repeat(4_096)}${E} end`, + // Exactly at the bound (4096 chars START..END inclusive) is still a span. + `edge ${S}${"z".repeat(4_094)}${E} end`, + ]; + for (const input of inputs) { + for (const size of [1, 7, 4_097, input.length]) { + const chunks: string[] = []; + for (let i = 0; i < input.length; i += size) chunks.push(input.slice(i, i + size)); + expect(drain(chunks)).toBe(stripCitationMarkers(input)); + } + } + }); });