mirror of
https://github.com/vectorize-io/hindsight.git
synced 2026-09-14 19:31:49 +08:00
fix(llm): recover malformed JSON via json_repair as a last-resort parse fallback (#2871)
Recover structurally-malformed LLM JSON (trailing commas, unterminated strings, single quotes, invalid \escape) via json_repair as a terminal fallback in parse_llm_json, after fence-strip and control-char scrub both fail. Empty repair result keeps raising JSONDecodeError so retry ladders / #1833 fail-loud still fire. LiteLLM prefers a clean re-roll first (repair only after retries exhausted). Scoped to structural malformation only — the degenerate-but-valid-JSON class (#2544/#2547) is deliberately out of scope. Regenerated the docs skill to clear pre-existing #2865 drift.
This commit is contained in:
@@ -12,6 +12,8 @@ import uuid
|
||||
from contextlib import AsyncExitStack
|
||||
from typing import TYPE_CHECKING, Any
|
||||
|
||||
from json_repair import repair_json
|
||||
|
||||
# Vertex AI imports (conditional - for LLMProvider to pass credentials to GeminiLLM)
|
||||
try:
|
||||
from google.oauth2 import service_account
|
||||
@@ -182,6 +184,14 @@ def parse_llm_json(raw: str) -> Any:
|
||||
1. Markdown code fences (```json ... ```) — strip them before parsing.
|
||||
2. Embedded control characters (\\x00-\\x1f, \\x7f) — replace with space
|
||||
and retry if the initial parse fails.
|
||||
3. Structural malformation (trailing commas, unterminated strings, single
|
||||
quotes, invalid ``\\escape`` sequences) — repaired as a last resort via
|
||||
``json_repair`` (#2547/#2544).
|
||||
|
||||
The repair pass is purely *structural*: it fixes JSON that ``json.loads``
|
||||
cannot parse at all. It deliberately does NOT touch content semantics —
|
||||
degenerate-but-valid JSON (repetition loops or leaked scaffolding inside
|
||||
string values) parses fine here and is out of scope for this helper.
|
||||
|
||||
Args:
|
||||
raw: Raw text returned by the LLM.
|
||||
@@ -190,7 +200,8 @@ def parse_llm_json(raw: str) -> Any:
|
||||
Parsed Python object (dict, list, etc.).
|
||||
|
||||
Raises:
|
||||
json.JSONDecodeError: If the text cannot be parsed even after cleanup.
|
||||
json.JSONDecodeError: If the text cannot be parsed even after cleanup
|
||||
and structural repair (e.g. repair yields an empty result).
|
||||
"""
|
||||
text = raw.strip()
|
||||
|
||||
@@ -207,7 +218,19 @@ def parse_llm_json(raw: str) -> Any:
|
||||
# Some models (e.g. Gemini) embed raw control characters inside JSON
|
||||
# string values. Replacing them with a space usually produces valid JSON.
|
||||
cleaned = re.sub(r"[\x00-\x1f\x7f]", " ", text)
|
||||
|
||||
try:
|
||||
return json.loads(cleaned)
|
||||
except json.JSONDecodeError:
|
||||
# Last resort: structural repair of malformed JSON. ``repair_json`` never
|
||||
# raises — unrecoverable input yields an empty result ("" / {} / []). Keep
|
||||
# failing loudly in that case rather than let an empty object masquerade
|
||||
# as a successful parse: callers (retry ladders, the #1833 fail-loud path)
|
||||
# rely on JSONDecodeError to retry or surface the failure.
|
||||
repaired = repair_json(cleaned, return_objects=True)
|
||||
if not repaired:
|
||||
raise
|
||||
return repaired
|
||||
|
||||
|
||||
_PROVIDERS_WITHOUT_API_KEY = frozenset(
|
||||
|
||||
@@ -30,6 +30,7 @@ from hindsight_api.engine.llm_interface import (
|
||||
OutputTooLongError,
|
||||
)
|
||||
from hindsight_api.engine.llm_trace import LLMResponseUsage, stash_response_usage
|
||||
from hindsight_api.engine.llm_wrapper import parse_llm_json
|
||||
from hindsight_api.engine.providers.llm_debug import dump_request_on_4xx
|
||||
from hindsight_api.engine.response_models import LLMToolCall, LLMToolCallResult, TokenUsage
|
||||
from hindsight_api.engine.structured_output import strict_json_schema
|
||||
@@ -285,7 +286,17 @@ class LiteLLMLLM(LLMInterface):
|
||||
try:
|
||||
json_data = json.loads(clean_content)
|
||||
except json.JSONDecodeError:
|
||||
json_data = json.loads(content)
|
||||
try:
|
||||
json_data = json.loads(content)
|
||||
except json.JSONDecodeError:
|
||||
if attempt < max_retries:
|
||||
# Prefer a clean re-roll first — a fresh generation
|
||||
# usually beats repairing a malformed one.
|
||||
raise
|
||||
# Retry budget spent: structural repair as a last
|
||||
# resort (#2547/#2544). Raises again if unrecoverable,
|
||||
# which the outer handler surfaces loudly.
|
||||
json_data = parse_llm_json(content)
|
||||
|
||||
if skip_validation:
|
||||
result = json_data
|
||||
|
||||
@@ -75,6 +75,7 @@ dependencies = [
|
||||
"claude-agent-sdk>=0.2.82",
|
||||
"boto3>=1.42.74",
|
||||
"croniter>=2.0.0", # Cron parsing for scheduled mental model refresh
|
||||
"json-repair>=0.30.0", # Structural repair of malformed LLM JSON (last-resort parse fallback)
|
||||
]
|
||||
|
||||
[project.optional-dependencies]
|
||||
|
||||
@@ -0,0 +1,115 @@
|
||||
"""
|
||||
Malformed-JSON recovery on the LiteLLM provider parse path (#2547/#2544).
|
||||
|
||||
When a structured-output response is malformed JSON (``json.loads`` cannot parse
|
||||
it at all — trailing commas, unterminated strings, invalid ``\\escape``), the
|
||||
provider prefers a clean re-roll first and only falls back to a structural
|
||||
``json_repair`` pass once the retry budget is exhausted. Unrecoverable output
|
||||
still fails loudly through the existing retry ladder.
|
||||
"""
|
||||
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
from pydantic import BaseModel
|
||||
|
||||
from hindsight_api.engine.providers.litellm_llm import LiteLLMLLM
|
||||
|
||||
|
||||
class _Facts(BaseModel):
|
||||
a: int
|
||||
|
||||
|
||||
def _make_provider() -> LiteLLMLLM:
|
||||
return LiteLLMLLM(
|
||||
provider="litellm",
|
||||
api_key="unused",
|
||||
base_url="http://localhost:0/v1",
|
||||
model="litellm_proxy/test-model",
|
||||
)
|
||||
|
||||
|
||||
def _make_response(content: str) -> MagicMock:
|
||||
response = MagicMock()
|
||||
response.usage = None
|
||||
response.model = "test-model"
|
||||
choice = MagicMock()
|
||||
choice.message.content = content
|
||||
choice.finish_reason = "stop"
|
||||
response.choices = [choice]
|
||||
return response
|
||||
|
||||
|
||||
async def test_malformed_json_repaired_after_retries_exhausted(monkeypatch):
|
||||
"""Persistently malformed JSON: retry ladder runs, then repair rescues it."""
|
||||
provider = _make_provider()
|
||||
calls = 0
|
||||
|
||||
async def _fake(**kwargs):
|
||||
nonlocal calls
|
||||
calls += 1
|
||||
return _make_response('{"a": 1,}') # trailing comma — never valid
|
||||
|
||||
monkeypatch.setattr(provider, "_acompletion", _fake)
|
||||
|
||||
result = await provider.call(
|
||||
messages=[{"role": "user", "content": "hi"}],
|
||||
response_format=_Facts,
|
||||
skip_validation=True,
|
||||
max_retries=1,
|
||||
initial_backoff=0.01,
|
||||
max_backoff=0.01,
|
||||
)
|
||||
|
||||
# A clean re-roll is preferred first: attempt 0 raises, attempt 1 repairs.
|
||||
assert calls == 2
|
||||
assert result == {"a": 1}
|
||||
|
||||
|
||||
async def test_clean_reroll_preferred_over_repair(monkeypatch):
|
||||
"""A malformed first response is retried; a valid re-roll wins (no repair)."""
|
||||
provider = _make_provider()
|
||||
responses = [_make_response('{"a": 1,}'), _make_response('{"a": 2}')]
|
||||
|
||||
async def _fake(**kwargs):
|
||||
return responses.pop(0)
|
||||
|
||||
monkeypatch.setattr(provider, "_acompletion", _fake)
|
||||
|
||||
result = await provider.call(
|
||||
messages=[{"role": "user", "content": "hi"}],
|
||||
response_format=_Facts,
|
||||
skip_validation=True,
|
||||
max_retries=1,
|
||||
initial_backoff=0.01,
|
||||
max_backoff=0.01,
|
||||
)
|
||||
|
||||
assert result == {"a": 2} # the clean re-roll, not a repair of the first
|
||||
|
||||
|
||||
async def test_unrecoverable_json_raises_after_retries(monkeypatch):
|
||||
"""Garbage that repair cannot rescue still fails loudly through the ladder."""
|
||||
import json
|
||||
|
||||
provider = _make_provider()
|
||||
calls = 0
|
||||
|
||||
async def _fake(**kwargs):
|
||||
nonlocal calls
|
||||
calls += 1
|
||||
return _make_response("not json at all !!!")
|
||||
|
||||
monkeypatch.setattr(provider, "_acompletion", _fake)
|
||||
|
||||
with pytest.raises(json.JSONDecodeError):
|
||||
await provider.call(
|
||||
messages=[{"role": "user", "content": "hi"}],
|
||||
response_format=_Facts,
|
||||
skip_validation=True,
|
||||
max_retries=1,
|
||||
initial_backoff=0.01,
|
||||
max_backoff=0.01,
|
||||
)
|
||||
|
||||
assert calls == 2
|
||||
@@ -230,6 +230,56 @@ def test_sanitize_llm_output(input_text, expected):
|
||||
assert sanitize_llm_output(input_text) == expected
|
||||
|
||||
|
||||
# --- parse_llm_json: structural repair of malformed LLM JSON (#2547/#2544) ---
|
||||
|
||||
|
||||
def test_parse_llm_json_valid_passthrough():
|
||||
from hindsight_api.engine.llm_wrapper import parse_llm_json
|
||||
|
||||
assert parse_llm_json('{"a": 1, "b": "two"}') == {"a": 1, "b": "two"}
|
||||
|
||||
|
||||
def test_parse_llm_json_strips_fences():
|
||||
from hindsight_api.engine.llm_wrapper import parse_llm_json
|
||||
|
||||
assert parse_llm_json('```json\n{"a": 1}\n```') == {"a": 1}
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"malformed,expected",
|
||||
[
|
||||
('{"a": 1,}', {"a": 1}), # trailing comma
|
||||
('{"a": "unterminated', {"a": "unterminated"}), # unterminated string
|
||||
("{'a': 'single quotes'}", {"a": "single quotes"}), # single quotes
|
||||
(r'{"path": "C:\Users"}', {"path": "C:\\Users"}), # invalid \escape (#2504)
|
||||
('```json\n{"a": 1,}\n```', {"a": 1}), # fenced + trailing comma
|
||||
],
|
||||
)
|
||||
def test_parse_llm_json_repairs_structural_malformation(malformed, expected):
|
||||
from hindsight_api.engine.llm_wrapper import parse_llm_json
|
||||
|
||||
assert parse_llm_json(malformed) == expected
|
||||
|
||||
|
||||
def test_parse_llm_json_control_char_scrub_still_works():
|
||||
from hindsight_api.engine.llm_wrapper import parse_llm_json
|
||||
|
||||
# Raw control char embedded in a string value — scrubbed to a space, no repair.
|
||||
assert parse_llm_json('{"a": "line\x01break"}') == {"a": "line break"}
|
||||
|
||||
|
||||
@pytest.mark.parametrize("garbage", ["", " ", "not json at all !!!", "```json\n```"])
|
||||
def test_parse_llm_json_unrecoverable_raises(garbage):
|
||||
"""Repair that yields an empty result must not masquerade as success — the
|
||||
contract is to raise so retry ladders / the #1833 fail-loud path can act."""
|
||||
import json
|
||||
|
||||
from hindsight_api.engine.llm_wrapper import parse_llm_json
|
||||
|
||||
with pytest.raises(json.JSONDecodeError):
|
||||
parse_llm_json(garbage)
|
||||
|
||||
|
||||
def test_llm_provider_constructor_ignores_global_config(monkeypatch):
|
||||
"""The constructor uses only its arguments — it never reads global config.
|
||||
|
||||
|
||||
@@ -1691,6 +1691,7 @@ dependencies = [
|
||||
{ name = "google-genai" },
|
||||
{ name = "greenlet" },
|
||||
{ name = "httpx" },
|
||||
{ name = "json-repair" },
|
||||
{ name = "langchain-core" },
|
||||
{ name = "langchain-text-splitters" },
|
||||
{ name = "langsmith" },
|
||||
@@ -1823,6 +1824,7 @@ requires-dist = [
|
||||
{ name = "httpx", specifier = ">=0.27.0" },
|
||||
{ name = "huggingface-hub", marker = "extra == 'local-llm'", specifier = ">=0.20.0" },
|
||||
{ name = "huggingface-hub", marker = "extra == 'local-onnx'", specifier = ">=0.20.0" },
|
||||
{ name = "json-repair", specifier = ">=0.30.0" },
|
||||
{ name = "langchain-core", specifier = ">=1.2.22" },
|
||||
{ name = "langchain-text-splitters", specifier = ">=0.3.0" },
|
||||
{ name = "langsmith", specifier = ">=0.8.18" },
|
||||
@@ -2310,6 +2312,15 @@ wheels = [
|
||||
{ url = "https://files.pythonhosted.org/packages/1e/e8/685f47e0d754320684db4425a0967f7d3fa70126bffd76110b7009a0090f/joblib-1.5.2-py3-none-any.whl", hash = "sha256:4e1f0bdbb987e6d843c70cf43714cb276623def372df3c22fe5266b2670bc241", size = 308396, upload-time = "2025-08-27T12:15:45.188Z" },
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "json-repair"
|
||||
version = "0.61.7"
|
||||
source = { registry = "https://pypi.org/simple" }
|
||||
sdist = { url = "https://files.pythonhosted.org/packages/f0/a3/6001c2448ee54a80f35a2501b848f4bbd87987bd41ead8cc17367a2bfd56/json_repair-0.61.7.tar.gz", hash = "sha256:a3754543f050093efcda6c9ab00b20a236b5d082c8c622bc65b88fa74ff8d51f", size = 51573, upload-time = "2026-07-21T13:01:15.085Z" }
|
||||
wheels = [
|
||||
{ url = "https://files.pythonhosted.org/packages/76/da/7f9e2b0a1120b107a204bbab6d0ef7ff2ae37790bddc5ee21c9c1f961f3b/json_repair-0.61.7-py3-none-any.whl", hash = "sha256:45c99b8cffef404e846b60d3dc21fc6f0fd5a4595cebad169dfab083ffb8246a", size = 50146, upload-time = "2026-07-21T13:01:13.734Z" },
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "jsonpatch"
|
||||
version = "1.33"
|
||||
|
||||
Reference in New Issue
Block a user