mirror of
https://github.com/discourse/discourse.git
synced 2026-08-08 17:53:55 +08:00
Reported 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/`.
107 lines
2.7 KiB
Ruby
Vendored
107 lines
2.7 KiB
Ruby
Vendored
# frozen_string_literal: true
|
|
|
|
module DiscourseAi
|
|
module Completions
|
|
class StructuredOutput
|
|
def initialize(json_schema_properties)
|
|
@property_names = json_schema_properties.keys.map(&:to_sym)
|
|
@property_cursors =
|
|
json_schema_properties.reduce({}) do |m, (k, prop)|
|
|
m[k.to_sym] = 0 if prop[:type] == "string"
|
|
m
|
|
end
|
|
|
|
@tracked = {}
|
|
|
|
@raw_response = +""
|
|
@raw_cursor = 0
|
|
|
|
@partial_json_tracker = JsonStreamingTracker.new(self)
|
|
|
|
@type_map = {}
|
|
json_schema_properties.each { |name, prop| @type_map[name.to_sym] = prop[:type].to_sym }
|
|
|
|
@done = false
|
|
end
|
|
|
|
def to_s
|
|
# we may want to also normalize the JSON here for the broken case
|
|
@raw_response.to_s
|
|
end
|
|
|
|
# require for any implicity string conversions
|
|
def to_str
|
|
to_s
|
|
end
|
|
|
|
attr_reader :last_chunk_buffer
|
|
|
|
def <<(raw)
|
|
raise "Cannot append to a completed StructuredOutput" if @done
|
|
@raw_response << raw
|
|
@partial_json_tracker << raw
|
|
end
|
|
|
|
def finish
|
|
@done = true
|
|
end
|
|
|
|
def finished?
|
|
@done
|
|
end
|
|
|
|
def broken?
|
|
@partial_json_tracker.broken?
|
|
end
|
|
|
|
def read_buffered_property(prop_name)
|
|
if @partial_json_tracker.broken?
|
|
if @done
|
|
return nil if @type_map[prop_name.to_sym].nil?
|
|
log_broken_stream
|
|
return(
|
|
DiscourseAi::Utils::BestEffortJsonParser.extract_key(
|
|
@raw_response,
|
|
@type_map[prop_name.to_sym],
|
|
prop_name,
|
|
)
|
|
)
|
|
else
|
|
return nil
|
|
end
|
|
end
|
|
|
|
# Maybe we haven't read that part of the JSON yet.
|
|
return nil if @tracked[prop_name].nil?
|
|
|
|
# This means this property is a string and we want to return unread chunks.
|
|
if @property_cursors[prop_name].present?
|
|
unread = @tracked[prop_name][@property_cursors[prop_name]..]
|
|
@property_cursors[prop_name] = @tracked[prop_name].length
|
|
unread
|
|
else
|
|
# Ints and bools, and arrays are always returned as is.
|
|
@tracked[prop_name]
|
|
end
|
|
end
|
|
|
|
def notify_progress(key, value)
|
|
key_sym = key.to_sym
|
|
return if !@property_names.include?(key_sym)
|
|
|
|
@tracked[key_sym] = value
|
|
end
|
|
|
|
private
|
|
|
|
def log_broken_stream
|
|
return if @broken_logged
|
|
@broken_logged = true
|
|
Rails.logger.warn(
|
|
"Discourse AI: structured output response was not valid JSON, " \
|
|
"falling back to best-effort parsing (#{@raw_response.bytesize} bytes)",
|
|
)
|
|
end
|
|
end
|
|
end
|
|
end
|