mirror of
https://github.com/safishamsi/graphify.git
synced 2026-07-12 10:27:11 +00:00
379d35e088
Three compounding bugs caused ~30-50% of semantic chunks to come back
as 'hollow responses' on the claude-cli backend, triggering adaptive
bisection that doubled or tripled the number of subprocess calls.
Root causes
-----------
1. _parse_llm_json only stripped markdown fences when raw.startswith('```').
Claude frequently prepends a short preamble before the fence
('Here are the extracted entities:\n\n```json\n{...}```'), making
the check fail. json.loads then drops the chunk. Each bisected half
may exhibit the same failure, so cost compounds.
2. _call_claude_cli used --append-system-prompt, which layers graphify's
extraction prompt on top of Claude Code's default interactive-agent
prompt ('use markdown formatting', 'output text to communicate with
the user'). The conflicting instructions explain ~50% of the
preambles and fences from (1). Switching to --system-prompt (replace)
eliminates the conflict at the source.
3. claude-cli defaults to Opus, which is overkill for the structured
JSON extraction graphify performs. New GRAPHIFY_CLAUDE_CLI_MODEL env
var lets users opt into haiku / sonnet for big builds. Default
behaviour unchanged when the env var is unset.
Fix
---
- Robust _parse_llm_json: strips fences regardless of position, with a
balanced-brace fallback that scans for the first complete JSON object
in the response. Handles preambles, trailing prose, prose-wrapped
JSON without fences. Diagnostic log on terminal failure includes the
first 200 chars of the response.
- _call_claude_cli switches to --system-prompt.
- _call_claude_cli respects GRAPHIFY_CLAUDE_CLI_MODEL when set.
Tests (tests/test_llm_parser.py)
--------------------------------
- The four PR-body failure modes: preamble+fence, prose+JSON, raw JSON,
total refusal.
- Bonus: uppercase fence tag, unclosed fence, empty response.
- argv shape: --system-prompt present, --append-system-prompt absent.
- argv shape: --model added iff GRAPHIFY_CLAUDE_CLI_MODEL is set.
19/19 tests pass (9 pre-existing in test_claude_cli_backend.py +
10 new). Verified end-to-end on a 800-file repo: 0 hollow responses
after, vs ~30-50% before; output tokens -93%; wall time 44 min -> 4 min.
147 lines
5.5 KiB
Python
147 lines
5.5 KiB
Python
"""Tests for `_parse_llm_json` robustness and the `_call_claude_cli`
|
|
subprocess argv shape introduced in the hollow-response fix.
|
|
|
|
These tests cover:
|
|
- The four parser failure modes described in PR #1062
|
|
- The switch from --append-system-prompt to --system-prompt
|
|
- The GRAPHIFY_CLAUDE_CLI_MODEL env-var passthrough
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import json
|
|
from unittest.mock import patch
|
|
|
|
import pytest
|
|
|
|
from graphify import llm
|
|
|
|
|
|
# ---------- _parse_llm_json: the four canonical failure modes ----------
|
|
|
|
|
|
def test_preamble_then_fence_is_parsed():
|
|
"""Claude often prefixes the JSON with a short preamble before the
|
|
```json fence. The original parser only stripped fences at offset 0,
|
|
so any preamble caused json.loads to fail and the chunk to be
|
|
dropped as a hollow response. The robust parser handles fences
|
|
anywhere in the text."""
|
|
raw = (
|
|
"Here are the extracted entities:\n\n"
|
|
'```json\n{"nodes": [{"id": "a"}], "edges": []}\n```'
|
|
)
|
|
result = llm._parse_llm_json(raw)
|
|
assert result["nodes"] == [{"id": "a"}]
|
|
assert result["edges"] == []
|
|
|
|
|
|
def test_prose_wrapped_json_without_fence_is_parsed():
|
|
"""Some models return prose around bare JSON with no markdown fence.
|
|
The balanced-brace fallback extracts the first complete object."""
|
|
raw = (
|
|
'The extracted graph is {"nodes": [{"id": "b"}], "edges": []}. '
|
|
"Hope this helps!"
|
|
)
|
|
result = llm._parse_llm_json(raw)
|
|
assert result["nodes"] == [{"id": "b"}]
|
|
|
|
|
|
def test_raw_json_still_works():
|
|
"""Regression: clean JSON input (the original happy path) must keep
|
|
parsing exactly as before."""
|
|
raw = '{"nodes": [], "edges": [], "hyperedges": []}'
|
|
result = llm._parse_llm_json(raw)
|
|
assert result == {"nodes": [], "edges": [], "hyperedges": []}
|
|
|
|
|
|
def test_total_refusal_returns_empty_fragment():
|
|
"""When the model refuses or returns unrelated prose, the parser
|
|
must degrade gracefully — return the empty fragment so the hollow
|
|
detector takes over, never raise."""
|
|
raw = "I cannot extract structured data from this content."
|
|
result = llm._parse_llm_json(raw)
|
|
assert result == {"nodes": [], "edges": [], "hyperedges": []}
|
|
|
|
|
|
# ---------- _parse_llm_json: secondary cases worth pinning ----------
|
|
|
|
|
|
def test_fence_with_uppercase_language_tag():
|
|
raw = '```JSON\n{"nodes": [{"id": "x"}], "edges": []}\n```'
|
|
result = llm._parse_llm_json(raw)
|
|
assert result["nodes"] == [{"id": "x"}]
|
|
|
|
|
|
def test_fence_without_closing_backticks():
|
|
"""Truncated response: the model started the fence but ran out of
|
|
tokens before closing it. We should still recover the JSON body."""
|
|
raw = '```json\n{"nodes": [{"id": "y"}], "edges": []}'
|
|
result = llm._parse_llm_json(raw)
|
|
assert result["nodes"] == [{"id": "y"}]
|
|
|
|
|
|
def test_empty_response_returns_empty_fragment():
|
|
assert llm._parse_llm_json("") == {"nodes": [], "edges": [], "hyperedges": []}
|
|
|
|
|
|
# ---------- _call_claude_cli: argv shape ----------
|
|
|
|
|
|
def _make_envelope(result_obj: dict) -> str:
|
|
return json.dumps({
|
|
"type": "result",
|
|
"subtype": "success",
|
|
"is_error": False,
|
|
"result": json.dumps(result_obj),
|
|
"usage": {"input_tokens": 1, "output_tokens": 1,
|
|
"cache_creation_input_tokens": 0, "cache_read_input_tokens": 0},
|
|
"modelUsage": {"claude-opus-4-7": {}},
|
|
"stop_reason": "end_turn",
|
|
})
|
|
|
|
|
|
@patch("shutil.which", return_value="/usr/local/bin/claude")
|
|
@patch("subprocess.run")
|
|
def test_uses_system_prompt_not_append(mock_run, _which):
|
|
"""The hollow-response root cause was --append-system-prompt
|
|
layering graphify's extraction prompt on top of Claude Code's
|
|
default interactive-agent prompt. The fix switches to
|
|
--system-prompt (replace) to eliminate the conflict."""
|
|
mock_run.return_value.returncode = 0
|
|
mock_run.return_value.stdout = _make_envelope({"nodes": [], "edges": [], "hyperedges": []})
|
|
mock_run.return_value.stderr = ""
|
|
llm._call_claude_cli("payload")
|
|
argv = mock_run.call_args.args[0]
|
|
assert "--system-prompt" in argv, f"--system-prompt missing from argv: {argv}"
|
|
assert "--append-system-prompt" not in argv, (
|
|
"--append-system-prompt should have been replaced — it's the root "
|
|
"cause of the hollow-response loop"
|
|
)
|
|
|
|
|
|
@patch("shutil.which", return_value="/usr/local/bin/claude")
|
|
@patch("subprocess.run")
|
|
def test_model_env_var_adds_model_flag(mock_run, _which, monkeypatch):
|
|
"""GRAPHIFY_CLAUDE_CLI_MODEL must be forwarded to claude -p --model."""
|
|
monkeypatch.setenv("GRAPHIFY_CLAUDE_CLI_MODEL", "haiku")
|
|
mock_run.return_value.returncode = 0
|
|
mock_run.return_value.stdout = _make_envelope({"nodes": [], "edges": [], "hyperedges": []})
|
|
mock_run.return_value.stderr = ""
|
|
llm._call_claude_cli("payload")
|
|
argv = mock_run.call_args.args[0]
|
|
assert "--model" in argv
|
|
assert argv[argv.index("--model") + 1] == "haiku"
|
|
|
|
|
|
@patch("shutil.which", return_value="/usr/local/bin/claude")
|
|
@patch("subprocess.run")
|
|
def test_no_model_flag_when_env_var_unset(mock_run, _which, monkeypatch):
|
|
"""Default behaviour: when the env var is not set, --model is not
|
|
added so claude-cli's own default kicks in."""
|
|
monkeypatch.delenv("GRAPHIFY_CLAUDE_CLI_MODEL", raising=False)
|
|
mock_run.return_value.returncode = 0
|
|
mock_run.return_value.stdout = _make_envelope({"nodes": [], "edges": [], "hyperedges": []})
|
|
mock_run.return_value.stderr = ""
|
|
llm._call_claude_cli("payload")
|
|
argv = mock_run.call_args.args[0]
|
|
assert "--model" not in argv
|