fix: strip markdown code fences from all LLM providers, not just local (#646)
LLM providers like MiniMax wrap JSON responses in markdown code fences (```json ... ```), causing JSON parse failures and 5-11 retries per extraction. The existing fence stripping logic was gated to only "lmstudio" and "ollama" providers (and for Ollama, unreachable due to the _call_ollama_native redirect). Changes: - Extract _strip_code_fences() helper function - Apply fence stripping to all providers in call() (not just local) - Add fence stripping safety net to _call_ollama_native() - Add 10 tests covering bare JSON, fenced JSON, malformed fences, and real-world MiniMax response format Fixes vectorize-io/hindsight#645 Co-authored-by: feniix <feniix@desktop>
This commit is contained in:
Sebastian B Otaegui
GitHub
parent
caa53ee370
commit
2f2db2a6e2
2 changed files with 145 additions and 33 deletions
|
|
@ -40,6 +40,25 @@ logger = logging.getLogger(__name__)
|
|||
DEFAULT_LLM_SEED = 4242
|
||||
|
||||
|
||||
def _strip_code_fences(content: str) -> str:
|
||||
"""Strip markdown code fences from LLM response if present.
|
||||
|
||||
Many LLM providers (MiniMax, some Ollama models, Claude via proxies)
|
||||
wrap JSON responses in ```json ... ``` fences even when json_object
|
||||
response format is requested. This strips the fences while preserving
|
||||
the JSON content inside. Returns the original content unchanged if
|
||||
no fences are detected.
|
||||
"""
|
||||
if "```" not in content:
|
||||
return content
|
||||
try:
|
||||
if "```json" in content:
|
||||
return content.split("```json")[1].split("```")[0].strip()
|
||||
return content.split("```")[1].split("```")[0].strip()
|
||||
except (IndexError, ValueError):
|
||||
return content
|
||||
|
||||
|
||||
class OpenAICompatibleLLM(LLMInterface):
|
||||
"""
|
||||
LLM provider for OpenAI-compatible APIs.
|
||||
|
|
@ -322,20 +341,14 @@ class OpenAICompatibleLLM(LLMInterface):
|
|||
if len(content) < original_len:
|
||||
logger.debug(f"Stripped {original_len - len(content)} chars of reasoning tokens")
|
||||
|
||||
# For local models, they may wrap JSON in markdown code blocks
|
||||
if self.provider in ("lmstudio", "ollama"):
|
||||
clean_content = content
|
||||
if "```json" in content:
|
||||
clean_content = content.split("```json")[1].split("```")[0].strip()
|
||||
elif "```" in content:
|
||||
clean_content = content.split("```")[1].split("```")[0].strip()
|
||||
try:
|
||||
json_data = json.loads(clean_content)
|
||||
except json.JSONDecodeError:
|
||||
# Fallback to parsing raw content
|
||||
json_data = json.loads(content)
|
||||
else:
|
||||
# Log raw LLM response for debugging JSON parse issues
|
||||
# Strip markdown code fences if present — any provider may
|
||||
# produce these (confirmed with MiniMax, some Ollama models,
|
||||
# Claude via proxies). No-op when content is already bare JSON.
|
||||
clean_content = _strip_code_fences(content)
|
||||
try:
|
||||
json_data = json.loads(clean_content)
|
||||
except json.JSONDecodeError:
|
||||
# Fallback to parsing raw content in case stripping was wrong
|
||||
try:
|
||||
json_data = json.loads(content)
|
||||
except json.JSONDecodeError as json_err:
|
||||
|
|
@ -730,26 +743,33 @@ class OpenAICompatibleLLM(LLMInterface):
|
|||
result = response.json()
|
||||
content = result.get("message", {}).get("content", "")
|
||||
|
||||
# Parse JSON response
|
||||
# Strip markdown code fences if present (safety net —
|
||||
# Ollama with schema enforcement usually returns bare JSON,
|
||||
# but some models may still wrap in fences)
|
||||
clean_content = _strip_code_fences(content)
|
||||
try:
|
||||
json_data = json.loads(content)
|
||||
except json.JSONDecodeError as json_err:
|
||||
content_preview = content[:500] if content else "<empty>"
|
||||
if content and len(content) > 700:
|
||||
content_preview = f"{content[:500]}...TRUNCATED...{content[-200:]}"
|
||||
logger.warning(
|
||||
f"Ollama JSON parse error (attempt {attempt + 1}/{max_retries + 1}): {json_err}\n"
|
||||
f" Model: ollama/{self.model}\n"
|
||||
f" Content length: {len(content) if content else 0} chars\n"
|
||||
f" Content preview: {content_preview!r}"
|
||||
)
|
||||
if attempt < max_retries:
|
||||
backoff = min(initial_backoff * (2**attempt), max_backoff)
|
||||
await asyncio.sleep(backoff)
|
||||
last_exception = json_err
|
||||
continue
|
||||
else:
|
||||
raise
|
||||
json_data = json.loads(clean_content)
|
||||
except json.JSONDecodeError:
|
||||
# Fallback to raw content
|
||||
try:
|
||||
json_data = json.loads(content)
|
||||
except json.JSONDecodeError as json_err:
|
||||
content_preview = content[:500] if content else "<empty>"
|
||||
if content and len(content) > 700:
|
||||
content_preview = f"{content[:500]}...TRUNCATED...{content[-200:]}"
|
||||
logger.warning(
|
||||
f"Ollama JSON parse error (attempt {attempt + 1}/{max_retries + 1}): {json_err}\n"
|
||||
f" Model: ollama/{self.model}\n"
|
||||
f" Content length: {len(content) if content else 0} chars\n"
|
||||
f" Content preview: {content_preview!r}"
|
||||
)
|
||||
if attempt < max_retries:
|
||||
backoff = min(initial_backoff * (2**attempt), max_backoff)
|
||||
await asyncio.sleep(backoff)
|
||||
last_exception = json_err
|
||||
continue
|
||||
else:
|
||||
raise
|
||||
|
||||
# Extract token usage from Ollama response
|
||||
duration = time.time() - start_time
|
||||
|
|
|
|||
92
hindsight-api-slim/tests/test_strip_code_fences.py
Normal file
92
hindsight-api-slim/tests/test_strip_code_fences.py
Normal file
|
|
@ -0,0 +1,92 @@
|
|||
"""Tests for _strip_code_fences helper in OpenAI-compatible LLM provider."""
|
||||
|
||||
import pytest
|
||||
|
||||
from hindsight_api.engine.providers.openai_compatible_llm import _strip_code_fences
|
||||
|
||||
|
||||
class TestStripCodeFences:
|
||||
"""Test markdown code fence stripping from LLM responses."""
|
||||
|
||||
def test_bare_json_unchanged(self):
|
||||
"""Bare JSON passes through unchanged."""
|
||||
content = '{"facts": [{"what": "test"}]}'
|
||||
assert _strip_code_fences(content) == content
|
||||
|
||||
def test_json_fence_stripped(self):
|
||||
"""```json ... ``` fences are stripped."""
|
||||
content = '```json\n{"facts": [{"what": "test"}]}\n```'
|
||||
assert _strip_code_fences(content) == '{"facts": [{"what": "test"}]}'
|
||||
|
||||
def test_plain_fence_stripped(self):
|
||||
"""``` ... ``` fences without language tag are stripped."""
|
||||
content = '```\n{"facts": [{"what": "test"}]}\n```'
|
||||
assert _strip_code_fences(content) == '{"facts": [{"what": "test"}]}'
|
||||
|
||||
def test_fence_with_trailing_whitespace(self):
|
||||
"""Fences with extra whitespace are handled."""
|
||||
content = '```json\n{"facts": []}\n```\n'
|
||||
result = _strip_code_fences(content)
|
||||
assert result == '{"facts": []}'
|
||||
|
||||
def test_fence_with_leading_whitespace(self):
|
||||
"""Content with leading whitespace before fence."""
|
||||
content = ' ```json\n{"facts": []}\n```'
|
||||
# The function checks for ``` in content, not startswith
|
||||
result = _strip_code_fences(content)
|
||||
assert '{"facts": []}' in result
|
||||
|
||||
def test_no_fences_no_change(self):
|
||||
"""Content without any backticks passes through."""
|
||||
content = "Just some text without fences"
|
||||
assert _strip_code_fences(content) == content
|
||||
|
||||
def test_empty_string(self):
|
||||
"""Empty string passes through."""
|
||||
assert _strip_code_fences("") == ""
|
||||
|
||||
def test_multiline_json(self):
|
||||
"""Multi-line JSON inside fences is preserved."""
|
||||
content = '```json\n{\n "facts": [\n {"what": "line1"},\n {"what": "line2"}\n ]\n}\n```'
|
||||
result = _strip_code_fences(content)
|
||||
assert '"line1"' in result
|
||||
assert '"line2"' in result
|
||||
assert "```" not in result
|
||||
|
||||
def test_malformed_fence_returns_original(self):
|
||||
"""Malformed fences (missing closing) return something parseable."""
|
||||
content = '```json\n{"facts": []}'
|
||||
result = _strip_code_fences(content)
|
||||
# Should attempt to strip and return best effort
|
||||
assert isinstance(result, str)
|
||||
|
||||
def test_minimax_style_response(self):
|
||||
"""Real-world MiniMax response format."""
|
||||
content = (
|
||||
"```json\n"
|
||||
"{\n"
|
||||
' "facts": [\n'
|
||||
" {\n"
|
||||
' "what": "Sebastian switched the Hindsight extraction LLM",\n'
|
||||
' "when": "2026-03-21",\n'
|
||||
' "where": "N/A",\n'
|
||||
' "who": "Sebastian",\n'
|
||||
' "why": "MiniMax wraps JSON in code fences",\n'
|
||||
' "fact_kind": "event",\n'
|
||||
' "fact_type": "world",\n'
|
||||
' "entities": [{"text": "Sebastian"}, {"text": "Hindsight"}],\n'
|
||||
' "labels": {"source_type": "stated", "domain": ["infrastructure"]}\n'
|
||||
" }\n"
|
||||
" ]\n"
|
||||
"}\n"
|
||||
"```"
|
||||
)
|
||||
result = _strip_code_fences(content)
|
||||
assert not result.startswith("```")
|
||||
assert not result.endswith("```")
|
||||
# Should be valid JSON
|
||||
import json
|
||||
|
||||
parsed = json.loads(result)
|
||||
assert len(parsed["facts"]) == 1
|
||||
assert parsed["facts"][0]["who"] == "Sebastian"
|
||||
Loading…
Reference in a new issue