Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .claude/rules/grade-layer.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ Sequential, not parallel (mirrors prune DEC-028). `asyncio.gather` deferred to v

The cached prompt block is the rubric criterion list (constant per run); the dynamic block is the per-pair `<ARTIFACT>...</ARTIFACT>` envelope. Anthropic prompt-cache TTL defaults to `"1h"` for the grader (vs. drafter's `"5m"`) — 60 sequential calls fit easily with margin for stalls.

**Tolerant JSON extraction (issue #144).** `parse_grade_response` routes the response through `signalforge._common.json_payload.extract_json_payload` (after `_strip_code_fence`) so a judge that narrates a prose preamble before the `{` still parses. The judge model (`claude-sonnet-4-6`) does NOT support an assistant-turn prefill (API 400), so the parser is the only JSON-only guardrail. Same decode rule as the drafter — decode at the first structural char (`{` or `[`) only, return unchanged on failure — see `llm-drafter.md` § "Tolerant JSON extraction"; a no-JSON response still routes to `GradeOutputError(violation_type="json_parse")` and the conservative degrade.

## Reproducibility hash fields on every GradeEvent (DEC-010, DEC-019)

Every `GradeEvent` carries five 16-hex blake2b-8 fingerprints:
Expand Down
11 changes: 11 additions & 0 deletions .claude/rules/llm-drafter.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,17 @@ The `tests/llm/test_prompt_cache_stability.py` snapshot pins the cached-block by

`signalforge.llm.client` emits `WARNING: cache marker no-op` only when **both** `cache_creation_input_tokens == 0` and `cache_read_input_tokens == 0` despite the cached block carrying a marker and passing pre-send size check. `cache_creation == 0` alone is the **normal healthy cache-hit case**. Any future "cache health" signal must apply the same dual-zero pattern.

## Tolerant JSON extraction — prose-preamble guardrail (issue #144)

`claude-sonnet-4-6` reproducibly narrates a reasoning preamble ("I need to analyze the business rules carefully...") **before** the JSON object on the business-rules drafting path, so `parse_draft_response`'s strict `CandidateSchema.model_validate_json` failed at line 1. **Assistant-turn prefill (the usual "JSON only" guardrail) is NOT available on this model** — the API rejects it with HTTP 400 `"This model does not support assistant message prefill. The conversation must end with a user message."` So the **parser is the only place a JSON-only guarantee can live**; the prompt is advisory.

`parse_draft_response` routes `raw_text` through `signalforge._common.json_payload.extract_json_payload` before `model_validate_json`. The helper strips a leading prose preamble (and trailing content) around a cleanly-decodable JSON value. Two load-bearing rules:

1. **Decode at the FIRST `{`/`[` only — never scan deeper.** A truncated outer object (whose first brace fails to decode) would otherwise match a complete *inner* fragment, silently turning a "not valid JSON" failure into a wrong-shape parse. On first-candidate failure the helper returns the input unchanged so the strict parser raises the normal `LLMOutputJSONError` with the correct excerpt/position — the truncated/garbage paths are unchanged.
2. **Error envelopes keep the ORIGINAL `raw_text`** (preamble included) so incident reports show exactly what the model emitted; only the `model_validate_json` call sees the extracted payload. The response-audit `response_text_hash` is likewise unchanged (hashes the full API response).

The grade parser shares the same helper (`grade-layer.md`). Prompt-level "JSON only" hardening (the issue's option 3) was deliberately NOT added: it would rotate the cached system-prompt golden for no load-bearing gain once the parser is the guarantee.

## Whole-draft fail-loud anchor contract (DEC-003, DEC-022)

`signalforge.draft.parser._validate_anchor_contract` collects **every** violation — never short-circuits. Returns a tuple; non-empty raises `LLMOutputAnchorContractError(violations=...)` with the full list. Three independent checks per column:
Expand Down
56 changes: 56 additions & 0 deletions src/signalforge/_common/json_payload.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
"""Tolerant JSON-payload extraction shared by the LLM-response parsers.

Issue #144: ``claude-sonnet-4-6`` reproducibly emitted a reasoning preamble
("I need to analyze the business rules carefully...") *before* the JSON
object on the business-rules drafting path, so the strict
``json.loads`` / ``model_validate_json`` parse failed at line 1. The
obvious guardrail — an assistant-turn prefill forcing JSON-only output —
is **not available**: the API rejects it with
``"This model does not support assistant message prefill. The
conversation must end with a user message."`` (HTTP 400). The parser is
therefore the only place a JSON-only guarantee can live, and prompts are
advisory at best.

:func:`extract_json_payload` locates the first complete JSON value
embedded in a response and returns just that substring, so the caller's
existing strict parser runs on clean JSON. Both the drafter
(:func:`signalforge.draft.parser.parse_draft_response`) and the grader
(:func:`signalforge.grade.parser.parse_grade_response`) route through it.
"""

from __future__ import annotations

import json

__all__ = ("extract_json_payload",)


def extract_json_payload(text: str) -> str:
"""Return the first complete JSON value embedded in ``text``.

Tolerates a leading prose preamble (and any trailing content) around a
single JSON object/array — the issue #144 failure mode, where the model
narrates plain sentences before the ``{``. The text is
whitespace-stripped, then decoding is attempted **at the first** ``{``
or ``[`` via :meth:`json.JSONDecoder.raw_decode`; on success the
substring spanning that value is returned (trailing content discarded).

Decoding is attempted at the first structural character ONLY — never at
later braces. Scanning deeper would let a *truncated* outer object
(whose first ``{`` fails to decode) match a complete **inner** fragment,
silently turning a "not valid JSON" failure into a wrong-shape parse.
So if the first candidate fails to decode, the whitespace-stripped input
is returned unchanged and the caller's strict parser raises its normal
JSON error with the correct excerpt/position. This helper never raises
and never mutates the JSON bytes — it only trims a leading preamble and
trailing content around a cleanly-decodable value.
"""
stripped = text.strip()
first = next((i for i, ch in enumerate(stripped) if ch in "{["), -1)
if first == -1:
return stripped
try:
_value, end = json.JSONDecoder().raw_decode(stripped, first)
except json.JSONDecodeError:
return stripped
return stripped[first:end]
15 changes: 13 additions & 2 deletions src/signalforge/draft/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@

from pydantic import ValidationError

from signalforge._common.json_payload import extract_json_payload
from signalforge.draft.errors import (
LLMOutputAnchorContractError,
LLMOutputJSONError,
Expand Down Expand Up @@ -207,8 +208,18 @@ def parse_draft_response(
sniff message text to render an incident report.
"""
# Stage 1 — JSON parse + Pydantic validation.
#
# Extract the embedded JSON object first (issue #144): some models
# (notably claude-sonnet-4-6 on the business-rules path) narrate a
# prose preamble before the `{`, and the model does not support an
# assistant-turn prefill to force JSON-only output. `extract_json_payload`
# strips the preamble; on a response with no JSON it returns the text
# unchanged so the error path below still fires with the right excerpt.
# The error envelopes keep the ORIGINAL `raw_text` so incident reports
# show exactly what the model emitted, preamble included.
payload = extract_json_payload(raw_text)
try:
candidate = CandidateSchema.model_validate_json(raw_text)
candidate = CandidateSchema.model_validate_json(payload)
except ValidationError as exc:
if _is_json_invalid_error(exc):
# Recover (line, column) positional context by re-parsing
Expand All @@ -218,7 +229,7 @@ def parse_draft_response(
# stdlib parser), fall through to the validation-error path
# so we never lose the failure signal.
try:
json.loads(raw_text)
json.loads(payload)
except json.JSONDecodeError as decode_exc:
raise LLMOutputJSONError(
"LLM response was not valid JSON.",
Expand Down
19 changes: 15 additions & 4 deletions src/signalforge/grade/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,14 @@

1. Strip surrounding whitespace and a single optional Markdown code
fence (the model occasionally wraps its JSON in `````json ...
`````; we strip the common cases but do **not** attempt to
extract JSON from arbitrary prose).
2. ``json.loads`` the stripped text. A :class:`json.JSONDecodeError`
`````), then extract the embedded JSON value via
:func:`signalforge._common.json_payload.extract_json_payload` — the
judge can narrate a prose preamble before the ``{`` and the model
does not support an assistant-turn prefill to force JSON-only output
(issue #144). Extraction decodes at the first ``{``/``[`` only and
returns the text unchanged when no JSON value is present, so a
genuinely prose-only / truncated response still fails loud at step 2.
2. ``json.loads`` the extracted text. A :class:`json.JSONDecodeError`
raises :class:`GradeOutputError` with
``violation_type="json_parse"``; a top-level non-object payload
(list / scalar / etc.) likewise raises ``violation_type="json_parse"``
Expand Down Expand Up @@ -47,6 +52,7 @@
import json
import math

from signalforge._common.json_payload import extract_json_payload
from signalforge.grade.errors import GradeOutputError
from signalforge.grade.models import GradingResult
from signalforge.grade.rubric import Criterion
Expand Down Expand Up @@ -99,7 +105,12 @@ def parse_grade_response(
with a ``violation_type`` from the locked taxonomy on every
malformed shape.
"""
cleaned = _strip_code_fence(response_text)
# Strip a Markdown code fence, then extract the embedded JSON object —
# the judge (claude-sonnet-4-6) can narrate a prose preamble before the
# `{`, and the model does not support an assistant-turn prefill to force
# JSON-only output (issue #144). `extract_json_payload` returns the text
# unchanged when no JSON value is present so the error path still fires.
cleaned = extract_json_payload(_strip_code_fence(response_text))
Comment thread
wjduenow marked this conversation as resolved.
try:
payload = json.loads(cleaned)
except json.JSONDecodeError as exc:
Expand Down
80 changes: 80 additions & 0 deletions tests/_common/test_json_payload.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
"""Tests for :func:`signalforge._common.json_payload.extract_json_payload`.

The helper is the load-bearing JSON-only guardrail for the LLM-response
parsers (issue #144): claude-sonnet-4-6 can narrate a prose preamble
before the JSON object and does NOT support an assistant-turn prefill to
force JSON-only output, so the parser must tolerate the preamble.
"""

from __future__ import annotations

import json

import pytest

from signalforge._common.json_payload import extract_json_payload

pytestmark = pytest.mark.unit


def test_plain_json_object_passthrough() -> None:
text = '{"a": 1, "b": [2, 3]}'
assert extract_json_payload(text) == text


def test_strips_leading_prose_preamble() -> None:
"""The issue #144 failure mode: prose before the object."""
preamble = "I need to analyze the business rules carefully. The first rule is a tautology. "
payload = '{"columns": [], "tests": []}'
extracted = extract_json_payload(preamble + payload)
assert extracted == payload
assert json.loads(extracted) == {"columns": [], "tests": []}


def test_strips_trailing_content_after_object() -> None:
payload = '{"score": 0.8, "passed": true}'
assert extract_json_payload(payload + "\n\nHope that helps!") == payload


def test_strips_markdown_code_fence_via_first_brace() -> None:
text = '```json\n{"ok": true}\n```'
assert json.loads(extract_json_payload(text)) == {"ok": True}


def test_decodes_at_first_brace_when_it_is_valid_json() -> None:
"""When the first structural char begins a valid object, it is returned
even if more prose/JSON follows."""
text = 'Result: {"verdict": "yes"} and some trailing note {"ignored": 1}'
assert json.loads(extract_json_payload(text)) == {"verdict": "yes"}


def test_truncated_outer_object_not_rescued_by_inner_fragment() -> None:
"""A truncated outer object must NOT be silently replaced by a complete
inner fragment — decoding is attempted at the first brace only, so this
returns unchanged and the caller's strict parser fails loud."""
truncated = '{"columns": [{"name": "order_id"}], "tests": [' # no closing braces
assert extract_json_payload(truncated) == truncated
with pytest.raises(json.JSONDecodeError):
json.loads(extract_json_payload(truncated))


def test_extracts_leading_array() -> None:
text = "Sure! [1, 2, 3]"
assert extract_json_payload(text) == "[1, 2, 3]"


def test_no_json_returns_stripped_input_unchanged() -> None:
"""No decodable JSON → return the stripped text so the caller's strict
parser raises its normal error with the right excerpt."""
text = " this is not json at all "
assert extract_json_payload(text) == "this is not json at all"


def test_empty_input_returns_empty() -> None:
assert extract_json_payload(" ") == ""


def test_does_not_mutate_json_bytes() -> None:
"""Whitespace inside the object is preserved verbatim (no re-encoding)."""
payload = '{"a": 1,\n "b": 2}'
assert extract_json_payload("preamble " + payload) == payload
22 changes: 22 additions & 0 deletions tests/draft/test_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,28 @@ def test_parse_draft_response_happy_path() -> None:
assert {c.name for c in result.columns} == _FCT_ORDERS_COLUMNS


def test_parse_draft_response_tolerates_prose_preamble() -> None:
"""Issue #144: claude-sonnet-4-6 narrates before the `{` on the
business-rules path and the model rejects an assistant prefill, so the
parser must strip the preamble and still parse the embedded JSON."""
raw = (
"I need to analyze the business rules carefully. The first rule is a "
"tautology, so I'll only propose tests that add signal.\n\n"
) + _read("llm_response_valid.json")
result = parse_draft_response(raw, _FCT_ORDERS_COLUMNS, llm_result_meta=_meta())
assert isinstance(result, CandidateSchema)
assert result.name == "fct_orders"
assert {c.name for c in result.columns} == _FCT_ORDERS_COLUMNS


def test_parse_draft_response_pure_prose_still_raises_json_error() -> None:
"""A response with no JSON object at all still fails loud (the preamble
tolerance must not mask a genuinely empty/garbage response)."""
raw = "I cannot help with that request."
with pytest.raises(LLMOutputJSONError):
parse_draft_response(raw, _FCT_ORDERS_COLUMNS, llm_result_meta=_meta())


def test_parse_draft_response_truncated_raises_json_error() -> None:
raw = _read("llm_response_truncated.json")
with pytest.raises(LLMOutputJSONError) as excinfo:
Expand Down
9 changes: 9 additions & 0 deletions tests/grade/test_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,15 @@ def test_parse_grade_response_strips_unfenced_code_fence() -> None:
assert result.criterion_id == _CRITERION.id


def test_parse_grade_response_tolerates_prose_preamble() -> None:
"""Issue #144: the judge can narrate before the `{` and the model
rejects an assistant prefill, so the parser strips the preamble."""
raw = "Let me think about this. The description is clear, so:\n\n" + json.dumps(_payload())
result = parse_grade_response(raw, artifact_id=_ARTIFACT_ID, criterion=_CRITERION)
assert result.criterion_id == _CRITERION.id
assert result.score == 0.8


def test_parse_grade_response_strips_surrounding_whitespace() -> None:
raw = " \n\t" + json.dumps(_payload()) + "\n "
result = parse_grade_response(raw, artifact_id=_ARTIFACT_ID, criterion=_CRITERION)
Expand Down
Loading