From e1403171f1c88ca82b5807317658be3335679e43 Mon Sep 17 00:00:00 2001 From: Rene Arredondo <120709323+Rene0422@users.noreply.github.com> Date: Mon, 1 Jun 2026 19:08:34 -0700 Subject: [PATCH] fix(chat): sanitize NaN/Inf scores before serializing chat completions (#15245) (#15266) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes #15245 — `POST /api/v1/chat/completions` with `stream=true` intermittently returns 500: ``` data:{"code": 500, "message": "failed to encode response: json: unsupported value: NaN (status code: 500)", "data": {...}} ``` …even though "the same question" works on retry. ## Root cause The streaming path serialized the answer with bare `json.dumps(...)` (`api/apps/restful_apis/chat_api.py:1221`). `json.dumps` defaults to `allow_nan=True` and emits the literal token `NaN` for NaN / Infinity float values. That is valid Python-flavored JSON but **invalid per RFC 8259**, so downstream consumers reject it. The reporter's gateway is Go-based and the error wording (`failed to encode response: json: unsupported value: NaN`) is straight from Go's `encoding/json`. How NaN gets into the payload: retrieval scoring in `rag/nlp/search.py` runs `np.mean(...)` over aggregations that can be empty, and similarity denominators can be zero. Reference chunk fields like `similarity`, `vector_similarity`, `term_similarity` can therefore be NaN depending on which chunks a given query retrieves — which is exactly why the failure is intermittent for the same question. The non-streaming branch (`get_json_result(data=answer)`, `chat_api.py:1243`) has the same vulnerability — Quart's `jsonify` also defaults to `allow_nan=True` and the same retrieval pipeline feeds both branches. `agent/tools/exesql.py:88-102` already has the same NaN/Inf guard for SQL results. This PR brings the chat completions path up to parity. ## Fix Add a small `_sanitize_json_floats(obj)` helper near the top of `api/apps/restful_apis/chat_api.py`. It walks `dict` / `list` / `tuple` and replaces any `float` that is `NaN` or `±Infinity` with `None`. Apply it at the two serialization boundaries: - **Streaming branch** (`stream()`): sanitize the SSE payload before `json.dumps`. - **Non-streaming branch**: sanitize the `answer` dict before `get_json_result(data=...)`. The terminal `data:True` frame and the `code:500` error frame carry no scores and are left untouched. Added `import math` to the existing alphabetical import block. No change to retrieval logic — replacing NaN with `null` at the serialization boundary is conservative: clients still parse the JSON, a missing-score chunk is a strictly better failure mode than a 500 that kills the whole reply. ### Type of change - [x] Bug Fix (non-breaking change which fixes an issue) --- api/apps/restful_apis/chat_api.py | 38 +++- .../test_chat_completions_sanitize_floats.py | 182 ++++++++++++++++++ 2 files changed, 218 insertions(+), 2 deletions(-) create mode 100644 test/unit_test/api/apps/restful_apis/test_chat_completions_sanitize_floats.py diff --git a/api/apps/restful_apis/chat_api.py b/api/apps/restful_apis/chat_api.py index acb8f8f81a..d0ea5f6280 100644 --- a/api/apps/restful_apis/chat_api.py +++ b/api/apps/restful_apis/chat_api.py @@ -16,6 +16,7 @@ import json import logging +import math import os import re import tempfile @@ -50,6 +51,38 @@ from common.misc_utils import get_uuid, thread_pool_exec from rag.prompts.generator import chunks_format from rag.prompts.template import load_prompt +def _sanitize_json_floats(obj): + """Replace NaN/Infinity floats with None so the result is RFC 8259 JSON. + + `json.dumps` emits the literal tokens `NaN`/`Infinity` by default + (allow_nan=True). Those tokens are valid Python JSON output but invalid + per the JSON spec, and downstream proxies / Go consumers reject the + response with `failed to encode response: json: unsupported value: NaN` + (fixes #15245). Retrieval scores (similarity, vector_similarity, + term_similarity) can become NaN when an aggregation runs over an empty + set or when a similarity denominator is zero, so the chat completions + stream is the realistic trigger. + + `isinstance(obj, float)` alone catches Python float and numpy.float64 + (a float subclass) but misses numpy.float32 / numpy.float16 and any + other duck-typed numeric. Probe via math.isnan/isinf in a try/except + so any object math can evaluate gets sanitized — without changing + upstream callers like chunks_format or rag/nlp/search.py. + """ + try: + if math.isnan(obj) or math.isinf(obj): + return None + except TypeError: + pass + if isinstance(obj, dict): + return {k: _sanitize_json_floats(v) for k, v in obj.items()} + if isinstance(obj, list): + return [_sanitize_json_floats(v) for v in obj] + if isinstance(obj, tuple): + return tuple(_sanitize_json_floats(v) for v in obj) + return obj + + _DEFAULT_PROMPT_CONFIG = { "system": ( 'You are an intelligent assistant. Please summarize the content of the dataset to answer the question. ' @@ -1225,7 +1258,8 @@ async def session_completion(chat_id_in_arg=""): try: async for ans in async_chat(dia, msg, True, **req): ans = _format_answer(ans) - yield "data:" + json.dumps({"code": 0, "message": "", "data": ans}, ensure_ascii=False) + "\n\n" + payload = _sanitize_json_floats({"code": 0, "message": "", "data": ans}) + yield "data:" + json.dumps(payload, ensure_ascii=False) + "\n\n" if conv is not None: await thread_pool_exec(ConversationService.update_by_id, conv.id, conv.to_dict()) except Exception as ex: @@ -1247,6 +1281,6 @@ async def session_completion(chat_id_in_arg=""): if conv is not None: await thread_pool_exec(ConversationService.update_by_id, conv.id, conv.to_dict()) break - return get_json_result(data=answer) + return get_json_result(data=_sanitize_json_floats(answer)) except Exception as ex: return server_error_response(ex) diff --git a/test/unit_test/api/apps/restful_apis/test_chat_completions_sanitize_floats.py b/test/unit_test/api/apps/restful_apis/test_chat_completions_sanitize_floats.py new file mode 100644 index 0000000000..a04cfe88b6 --- /dev/null +++ b/test/unit_test/api/apps/restful_apis/test_chat_completions_sanitize_floats.py @@ -0,0 +1,182 @@ +# +# Copyright 2026 The InfiniFlow Authors. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +"""Regression tests for `_sanitize_json_floats` in +`api/apps/restful_apis/chat_api.py` (fixes #15245). + +The function strips NaN / ±Infinity from chat completion payloads +before serialization so the response is RFC 8259 JSON. Without it, +Go-style downstream consumers reject the stream with +`failed to encode response: json: unsupported value: NaN`. + +The function has no module-level dependencies beyond stdlib `math`, +so the test extracts the function definition from `chat_api.py` via +the `ast` module and executes it in an isolated namespace. That +avoids stubbing the dozens of heavy imports `chat_api.py` pulls in +at module load (quart, peewee models, services, etc.) and keeps the +test fast and focused. +""" + +from __future__ import annotations + +import ast +import math +from pathlib import Path + +import pytest + + +def _load_sanitize_function(): + """Extract `_sanitize_json_floats` from chat_api.py without importing it. + + Walks the source AST, finds the function definition, compiles only + that node into a tiny module, and exec's it in a namespace that + only exposes `math`. Returns the function object ready to call. + """ + repo_root = Path(__file__).resolve().parents[5] + source = (repo_root / "api" / "apps" / "restful_apis" / "chat_api.py").read_text() + tree = ast.parse(source) + fn_node = None + for node in tree.body: + if isinstance(node, ast.FunctionDef) and node.name == "_sanitize_json_floats": + fn_node = node + break + assert fn_node is not None, "_sanitize_json_floats not found in chat_api.py" + extracted = ast.Module(body=[fn_node], type_ignores=[]) + ns = {"math": math} + exec(compile(extracted, str(repo_root / "api/apps/restful_apis/chat_api.py"), "exec"), ns) + return ns["_sanitize_json_floats"] + + +_sanitize = _load_sanitize_function() + + +@pytest.mark.p1 +class TestSanitizeJsonFloats: + """Regression for #15245: NaN/Inf in retrieval scores must not leak + into the JSON response.""" + + @pytest.mark.p1 + def test_passthrough_for_healthy_values(self): + """Plain numbers, strings, bools, None, lists, and dicts survive + untouched.""" + assert _sanitize(0) == 0 + assert _sanitize(1.5) == 1.5 + assert _sanitize(-3.25) == -3.25 + assert _sanitize("hello") == "hello" + assert _sanitize(None) is None + assert _sanitize(True) is True + assert _sanitize(False) is False + assert _sanitize([1, 2, 3]) == [1, 2, 3] + assert _sanitize({"a": 1, "b": "x"}) == {"a": 1, "b": "x"} + + @pytest.mark.p1 + def test_nan_becomes_none(self): + """NaN at the top level is replaced with None — the only RFC + 8259-compatible representation.""" + assert _sanitize(float("nan")) is None + + @pytest.mark.p1 + @pytest.mark.parametrize("value", [float("inf"), float("-inf")]) + def test_positive_and_negative_infinity_become_none(self, value): + assert _sanitize(value) is None + + @pytest.mark.p1 + def test_nan_inside_dict_replaced(self): + """NaN appearing as a dict value is replaced; sibling keys are + preserved verbatim.""" + out = _sanitize({"similarity": float("nan"), "doc_id": "abc", "score": 0.42}) + assert out == {"similarity": None, "doc_id": "abc", "score": 0.42} + + @pytest.mark.p1 + def test_inf_inside_list_replaced(self): + out = _sanitize([1, float("inf"), 2, float("-inf"), 3]) + assert out == [1, None, 2, None, 3] + + @pytest.mark.p1 + def test_deeply_nested_chunks_format(self): + """Mimics the actual `reference.chunks` shape — list of dicts of + scores, with NaN possible per-chunk.""" + payload = { + "answer": "...", + "reference": { + "chunks": [ + { + "id": "c1", + "similarity": float("nan"), + "vector_similarity": 0.81, + "term_similarity": 0.62, + }, + { + "id": "c2", + "similarity": 0.74, + "vector_similarity": float("inf"), + "term_similarity": 0.55, + }, + ] + }, + } + out = _sanitize(payload) + assert out["answer"] == "..." + chunks = out["reference"]["chunks"] + assert chunks[0] == { + "id": "c1", + "similarity": None, + "vector_similarity": 0.81, + "term_similarity": 0.62, + } + assert chunks[1] == { + "id": "c2", + "similarity": 0.74, + "vector_similarity": None, + "term_similarity": 0.55, + } + + @pytest.mark.p1 + def test_tuples_are_walked_and_returned_as_tuples(self): + """Tuples are not idiomatic in JSON payloads but the function + walks them defensively. The container type is preserved.""" + out = _sanitize((1, float("nan"), 3)) + assert out == (1, None, 3) + assert isinstance(out, tuple) + + @pytest.mark.p1 + def test_numpy_float32_nan_caught_via_math_isnan(self): + """numpy float scalar types (float32 / float16) do not subclass + Python's `float` so a naive isinstance check would miss them. + The probe-via-math.isnan approach used by the function should + catch them anyway. Skip cleanly if numpy is unavailable so this + test doesn't gate CI on the optional dep.""" + np = pytest.importorskip("numpy") + + # float64 is a subclass of float so the easy isinstance path + # would have caught this; included for documentation. + assert _sanitize(np.float64("nan")) is None + + # float32 / float16 are NOT subclasses of Python float; the + # math.isnan probe is what saves us. + assert _sanitize(np.float32("nan")) is None + assert _sanitize(np.float16("nan")) is None + + # Healthy numpy scalars still pass through. + assert _sanitize(np.float32(1.5)) == pytest.approx(1.5) + + @pytest.mark.p1 + def test_strings_are_not_treated_as_numeric(self): + """`math.isnan` would TypeError on a string; the function must + swallow that and treat the string as a regular non-numeric leaf.""" + assert _sanitize("NaN") == "NaN" + assert _sanitize("Infinity") == "Infinity" + assert _sanitize({"label": "NaN"}) == {"label": "NaN"}