lee/graphify
1
0
Fork 0
mirror of https://github.com/safishamsi/graphify.git synced 2026-08-28 17:26:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(claude-cli): eliminate hollow-response loop from system-prompt conflict

Browse Source 
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.
This commit is contained in:
christophepub
2026-05-28 17:44:35 +02:00
parent 7c12499783
commit 379d35e088
2 changed files with 225 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
graphify/llm.py
+79 -15
View File
@@ -194,16 +194,63 @@ def _parse_llm_json(raw: str) -> dict:
file=sys.stderr,
)
return {"nodes": [], "edges": [], "hyperedges": []}
if raw.startswith("```"):
raw = raw.split("```", 2)[1]
if raw.startswith("json"):
raw = raw[4:]
raw = raw.rsplit("```", 1)[0]
# Strategy 1: strip whitespace, then handle markdown fences anywhere in the
# text (not only at offset 0 — the original code only stripped fences when
# `raw.startswith("```")`, missing the common case where Claude prepends a
# preamble like "Here's the extracted entities:\n\n```json\n{...}\n```").
stripped = raw.strip()
fence_start = stripped.find("```")
if fence_start != -1:
after_fence = stripped[fence_start + 3 :]
# Optional language tag (json, JSON, javascript, etc.) up to newline.
nl = after_fence.find("\n")
if nl != -1 and after_fence[:nl].strip().lower() in {"json", "javascript", "js", ""}:
after_fence = after_fence[nl + 1 :]
fence_end = after_fence.rfind("```")
if fence_end != -1:
stripped = after_fence[:fence_end].strip()
else:
stripped = after_fence.strip()
try:
return json.loads(raw.strip())
except json.JSONDecodeError as exc:
print(f"[graphify] LLM returned invalid JSON, skipping chunk: {exc}", file=sys.stderr)
return {"nodes": [], "edges": [], "hyperedges": []}
return json.loads(stripped)
except json.JSONDecodeError:
pass
# Strategy 2: extract the first balanced JSON object found anywhere in
# the text. Handles the case where Claude wraps the JSON in prose without
# any markdown fence ("The extracted graph is { ... }. Hope this helps!").
start = stripped.find("{")
if start != -1:
depth = 0
in_string = False
escape = False
for i in range(start, len(stripped)):
ch = stripped[i]
if escape:
escape = False
continue
if ch == "\\":
escape = True
continue
if ch == '"':
in_string = not in_string
continue
if in_string:
continue
if ch == "{":
depth += 1
elif ch == "}":
depth -= 1
if depth == 0:
try:
return json.loads(stripped[start : i + 1])
except json.JSONDecodeError:
break
print(
f"[graphify] LLM returned invalid JSON, skipping chunk "
f"(first 200 chars: {raw[:200]!r})",
file=sys.stderr,
)
return {"nodes": [], "edges": [], "hyperedges": []}
def _response_is_hollow(raw_content: str | None, parsed: dict) -> bool:
@@ -452,13 +499,30 @@ def _call_claude_cli(user_message: str, max_tokens: int = 8192, *, deep_mode: bo
"https://claude.ai/code and run `claude` once to authenticate."
)
# Use --system-prompt (replaces) instead of --append-system-prompt (adds
# to Claude Code's default coding-agent prompt). The default prompt
# pushes the model towards markdown + prose explanations, which conflict
# with the "raw JSON only" extraction instruction and cause ~30-50% of
# responses to come back wrapped in ```json fences or prefixed with a
# preamble — both of which fail the strict json.loads in _parse_llm_json.
# Replacing the default prompt eliminates the conflict at the source.
# Side benefit: cache-creation tokens per call drop ~19% in practice.
cli_args = [
"claude", "-p",
"--output-format", "json",
"--no-session-persistence",
"--system-prompt", _extraction_system(deep=deep_mode),
]
# claude-cli defaults to Opus, which is overkill for the structured-JSON
# extraction graphify performs. GRAPHIFY_CLAUDE_CLI_MODEL=haiku (or
# sonnet, or a full model ID like claude-haiku-4-5-20251001) lets users
# opt into a cheaper / faster model. Default behaviour unchanged when
# the env var is unset.
cli_model = os.environ.get("GRAPHIFY_CLAUDE_CLI_MODEL", "").strip()
if cli_model:
cli_args.extend(["--model", cli_model])
proc = subprocess.run(
[
"claude", "-p",
"--output-format", "json",
"--no-session-persistence",
"--append-system-prompt", _extraction_system(deep=deep_mode),
],
cli_args,
input=user_message,
capture_output=True,
text=True,
tests/test_llm_parser.py
+146
View File
@@ -0,0 +1,146 @@
"""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