fix: shared LLM output guardrail layer (closes #17)#30
Merged
Conversation
Reasoning models (MiniMax/GLM/DeepSeek/Qwen) and JSON-mode endpoints leak <think> blocks, markdown code fences, and truncated/malformed JSON into the content field. Commit 985f789 patched this inline in one method, but three other code paths parsed LLM JSON independently with duplicated, inconsistent repair logic — and the two simulation generators stripped nothing, so the same class of output could still 500 them. Centralize all of it in a single, pure, testable layer: - New app/utils/llm_sanitizer.py: strip_reasoning, strip_code_fences, extract_json_block, close_truncated_json (string/stack-aware), repair_json (progressive repair), and parse_json — the one validation step that sanitizes, parses, repairs, optionally validates required keys, and raises a single predictable ValueError instead of leaking JSONDecodeError. - llm_client.py: chat() -> sanitize_content, chat_json() -> parse_json with an optional required_keys schema check. Covers every chat_json caller (report_agent, zep_tools, ontology_generator) for free. - oasis_profile_generator.py / simulation_config_generator.py: these bypass LLMClient and call OpenAI directly; they now sanitize content before parsing and delegate their duplicated _fix_truncated_json/_try_fix_*_json bodies to the shared module (oasis keeps its bio/persona salvage fallback). - tests/test_llm_sanitizer.py: 24 tests covering reasoning/fence stripping, truncated/nested JSON repair, required-keys validation, and an LLMClient integration test proving reasoning-block + fence + truncated-brace output now parses instead of 500ing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
_classify_batch's signature is (scores, topics, success); the test's fake_classify stub still returned a 2-tuple, so analyze_all() failed to unpack `success` with "not enough values to unpack (expected 3, got 2)". Update the stub to match the current contract. Pre-existing failure, unrelated to the guardrail change in this PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jun 20, 2026
pyproject declares flask-limiter but uv.lock never included it, so CI's `uv sync --frozen` silently skipped it and every test failed at conftest import with `ModuleNotFoundError: No module named 'flask_limiter'`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #17. Generalizes the one-off
<think>/code-fence fix from commit 985f789 into a single, reusable output guardrail / validation layer that every agent response flows through.Reasoning models (MiniMax/GLM/DeepSeek/Qwen) and JSON-mode endpoints leak
<think>blocks, markdown fences, and truncated/malformed JSON into the content field. Previously onlyLLMClienthandled some of this inline, while three other paths parsed LLM JSON independently with duplicated repair logic — and the two simulation generators stripped nothing, so the same output could still cause 500s.Changes
app/utils/llm_sanitizer.py— pure, testable helpers:strip_reasoning(handles well-formed, truncated, and orphan-closer tag variants)strip_code_fences(extracts JSON from```jsonfences, incl. unclosed)extract_json_block(first balanced{…}/[…]out of surrounding prose)close_truncated_json(string/stack-aware closing of token-limit truncation)repair_json(progressive repair passes)parse_json— the single validation step: sanitize → parse → repair → optionalrequired_keysschema check; raises one predictableValueErrorinstead of leakingJSONDecodeErrorllm_client.py—chat()→sanitize_content;chat_json()→parse_json(now repairs instead of raising; added optionalrequired_keys). Coversreport_agent,zep_tools,ontology_generatorfor free.oasis_profile_generator.py/simulation_config_generator.py— these bypassLLMClient; now sanitize content before parsing and delegate their duplicated_fix_truncated_json/_try_fix_*_jsonbodies to the shared module (oasis keeps its bio/persona salvage fallback).Acceptance criteria
Testing
tests/test_llm_sanitizer.py: 24 tests (reasoning/fence stripping, truncated/nested JSON repair, required-keys validation, and anLLMClient.chat_jsonintegration test proving reasoning-block + fence + truncated-brace output parses instead of 500ing).test_sentiment_analyzer_cache) is pre-existing and unrelated (a_classify_batchtuple-arity bug insentiment_analyzer.py); it fails identically on a clean tree.🤖 Generated with Claude Code