Skip to content

fix(qa): tolerate non-dict JSON from QA LLM instead of crashing#408

Open
Mubashirrrr wants to merge 1 commit into
dograh-hq:mainfrom
Mubashirrrr:fix/qa-analysis-non-dict-llm-response
Open

fix(qa): tolerate non-dict JSON from QA LLM instead of crashing#408
Mubashirrrr wants to merge 1 commit into
dograh-hq:mainfrom
Mubashirrrr:fix/qa-analysis-non-dict-llm-response

Conversation

@Mubashirrrr

Copy link
Copy Markdown

Bug

parse_llm_json (api/services/gen_ai/json_parser.py) is explicitly designed to return a list when the model emits a top-level JSON array — it has a dedicated unit test for "[1, 2, 3]", and _extract_json_array is one of its parse strategies.

The QA analyzers in api/services/workflow/qa/analysis.py (both _run_qa_analysis per-node and the _run_whole_call_qa_analysis fallback) call .get() directly on the result:

try:
    parsed = parse_llm_json(raw_response)
    node_result["tags"] = parsed.get("tags", [])      # AttributeError if parsed is a list
    node_result["summary"] = parsed.get("summary", "")
    ...
except (json.JSONDecodeError, ValueError):
    ...

When parsed is a list, parsed.get(...) raises AttributeError, which is not caught by the surrounding except (json.JSONDecodeError, ValueError). So a single stray array response from the QA model crashes the entire QA analysis run instead of degrading to empty results.

Notably, the live variable-extraction path already guards this exact case (api/services/workflow/pipecat_engine.py: if not isinstance(extracted_data, dict): ... skip), so this is an inconsistency — the same defensive check is present in one consumer of LLM-parsed JSON but missing in the QA consumers.

Reproduce

from api.services.gen_ai.json_parser import parse_llm_json
import json

raw_response = '["tag1", "tag2"]'   # model returned a top-level array
node_result = {}
try:
    parsed = parse_llm_json(raw_response)
    node_result["tags"] = parsed.get("tags", [])     # boom
except (json.JSONDecodeError, ValueError):
    node_result["tags"] = []
# AttributeError: 'list' object has no attribute 'get'

Fix

Mirror the existing engine guard in both QA call sites: coerce a non-dict parse result to {} so the .get() lookups produce empty defaults instead of crashing.

parsed = parse_llm_json(raw_response)
if not isinstance(parsed, dict):
    parsed = {}

Regression test

api/tests/test_qa_analysis_non_dict_response.py drives _run_whole_call_qa_analysis with an array LLM response (mocking the LLM seams) and asserts empty results (tags == [], summary == "", score is None) rather than an exception. The core parse-block behavior was verified directly: it raises AttributeError on the old code and degrades to empty defaults on the new code, while the normal dict path still maps correctly.

parse_llm_json is explicitly designed to return a list when the model emits a
top-level JSON array (it has a dedicated test for that). The QA analyzers then
call parsed.get("tags", ...) directly on the result. When parsed is a list,
that raises AttributeError, which is NOT caught by the surrounding
except (json.JSONDecodeError, ValueError) — so a single stray array response
from the QA model crashed the entire QA analysis run instead of degrading to
empty results.

The live variable-extraction path already guards this exact case with an
isinstance(..., dict) check; mirror it in both QA analysis call sites
(_run_qa_analysis per-node and _run_whole_call_qa_analysis fallback) so a
non-dict parse result coerces to {} and the run produces empty defaults.

Adds a regression test that drives the whole-call analyzer with an array
response and asserts empty results rather than a crash.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions github-actions Bot added the fix Bug fix (changelog: Bug Fixes) label Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Bug fix (changelog: Bug Fixes)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant