mirror of
https://github.com/discourse/discourse.git
synced 2026-08-08 17:53:55 +08:00
87775447c0Reported at https://meta.discourse.org/t/translation-silently-truncated-when-json-stream-parsing-breaks-no-error-raised/407251 ## The bug Some providers stream structured output whose string values were unescaped by an outer JSON parse, so real newlines appear inside string values. When that happened, `JsonStreamingTracker` had two failure modes: - It marked the stream broken and `StructuredOutput` fell back to `BestEffortJsonParser`, whose extraction regex (`[^"]+`) cut the value at the first escaped quote and left `\n` sequences as literal text. A 2,000-char translation could come back as ~50 chars, cut right before the first quoted word — exactly what the report shows. - Its escape-and-resume hack (`String#dump` + buffer-growth offset) miscomputed the resume index whenever the chunk contained non-ASCII, quotes, or backslashes, silently duplicating or corrupting content **without ever marking the stream broken**. Testing a realistic corrupted payload across chunk sizes 1–60: 31 produced the truncated fallback, 27 produced silent corruption, 1 raised, and only 1 came out correct. Either way the result was persisted as a successful translation with nothing in the logs. ## The fix Replace the hand-rolled parsing with two gems and keep only glue: - **json_completer** (pure Ruby): `JsonStreamingTracker` now feeds the cumulative buffer — with control characters re-escaped — to an incremental, truncation-tolerant parser and notifies consumers of changed keys. The corrupted payloads above stream correctly at every chunk size, so the broken-stream path is only reached for responses that aren't JSON at all. - **smarter_json**: `BestEffortJsonParser` becomes a three-attempt chain (strict-with-completion → control-chars re-escaped → lenient) covering the quirk shapes the old regexes handled: single quotes, unquoted keys, markdown fences, prose-wrapped JSON. This deletes the vendored 668-line SAX parser, the resume hack, and all manual regex extraction (net −694 lines), and adds a log warning whenever a response falls back to best-effort parsing. ## Behavior changes - Scalars now stream progressively: mid-stream `read_buffered_property(:number)` returns the digits buffered so far instead of `nil`. Consumers act on final values, so this only affects mid-stream reads. - Arrays of objects stream partial objects mid-stream instead of returning `nil` until finish. - A trailing comma in an array reads as a `nil` placeholder slot until the next element arrives. - Partial tool calls surface a few more progressive updates (the openai endpoint spec count moved 128 → 134); values still only ever grow. ## Tests - Regression specs for the report: unescaped control characters with escaped quotes/emoji streamed across chunk boundaries, fenced + unescaped responses, truncated JSON, numeric casting. - 916 examples green across `completions/`, `translation/`, `modules/ai_helper/`, and `utils/`.
58 lines
1.3 KiB
Ruby
Vendored
58 lines
1.3 KiB
Ruby
Vendored
# frozen_string_literal: true
|
|
|
|
module DiscourseAi
|
|
module Completions
|
|
class JsonStreamingTracker
|
|
attr_reader :stream_consumer
|
|
|
|
def initialize(stream_consumer)
|
|
@stream_consumer = stream_consumer
|
|
@escaped_buffer = +""
|
|
@completer = JsonCompleter.new
|
|
@broken = false
|
|
@last_notified = {}
|
|
end
|
|
|
|
def broken?
|
|
@broken
|
|
end
|
|
|
|
def <<(raw_json)
|
|
return if @broken
|
|
|
|
if !raw_json.is_a?(String)
|
|
@broken = true
|
|
return
|
|
end
|
|
|
|
@escaped_buffer << DiscourseAi::Utils::BestEffortJsonParser.escape_control_characters(
|
|
raw_json,
|
|
)
|
|
|
|
parsed =
|
|
begin
|
|
@completer.parse(@escaped_buffer)
|
|
rescue JsonCompleter::ParseError
|
|
@broken = true
|
|
return
|
|
end
|
|
|
|
notify_changes(parsed) if parsed.is_a?(Hash)
|
|
end
|
|
|
|
private
|
|
|
|
def notify_changes(parsed)
|
|
parsed.each do |key, value|
|
|
next if value.nil?
|
|
next if @last_notified[key] == value
|
|
|
|
# the completer mutates parsed containers in place between calls, so
|
|
# compare against a snapshot
|
|
@last_notified[key] = value.deep_dup
|
|
stream_consumer.notify_progress(key, value)
|
|
end
|
|
end
|
|
end
|
|
end
|
|
end
|
