#144: tolerate LLM prose preamble before JSON in draft/grade parsers - #147
Merged
Conversation
claude-sonnet-4-6 reproducibly narrates a reasoning preamble before the
JSON object on the business-rules drafting path, so the strict parse failed
at line 1. The obvious guardrail — an assistant-turn JSON prefill — is
unavailable: the API rejects it with HTTP 400 "This model does not support
assistant message prefill." The parser is therefore the only place a
JSON-only guarantee can live (prompts are advisory).
Add signalforge._common.json_payload.extract_json_payload: decode at the
FIRST { or [ via json.raw_decode, returning that span and discarding a
leading preamble + trailing content. Decode the first candidate ONLY — a
truncated outer object must not be silently rescued by a complete inner
fragment, so on first-candidate failure the text is returned unchanged and
the strict parser raises the normal JSON error with the right excerpt.
- draft/parser.py: extract before model_validate_json; error envelopes keep
the original raw_text (preamble included) so incident reports are honest.
- grade/parser.py: extract after _strip_code_fence (shares the helper).
- tests: helper unit matrix (preamble, fence, trailing, array, truncated-not-
rescued, no-json passthrough) + draft/grade parser preamble regressions +
a pure-prose-still-raises check.
- llm-drafter.md / grade-layer.md: document the contract and why prefill
(option 1) and prompt hardening (option 3) were not used.
Full suite: 2416 passed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (4)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
Adds a tolerant JSON-payload extraction step to the draft + grade LLM response parsers so they can handle models that emit a prose preamble (and/or trailing text) around an otherwise-valid JSON payload, addressing issue #144.
Changes:
- Introduces
signalforge._common.json_payload.extract_json_payloadto extract the first decodable JSON object/array starting at the first{/[in the response. - Applies the helper in
draft/parser.py(beforemodel_validate_json) and ingrade/parser.py(after_strip_code_fence, beforejson.loads). - Adds targeted unit tests for the helper and new regression tests for both parsers.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/grade/test_parser.py | Adds regression coverage for grade-parser prose preambles. |
| tests/draft/test_parser.py | Adds regression coverage for draft-parser prose preambles and ensures pure-prose still fails loud. |
| tests/_common/test_json_payload.py | Adds unit tests for the new JSON extraction helper. |
| src/signalforge/grade/parser.py | Routes judge responses through the shared tolerant JSON extractor. |
| src/signalforge/draft/parser.py | Routes drafter responses through the shared tolerant JSON extractor before strict Pydantic parsing. |
| src/signalforge/_common/json_payload.py | New shared helper implementing first-brace-only JSON extraction. |
| .claude/rules/llm-drafter.md | Documents the new guardrail and its invariants for the drafter. |
| .claude/rules/grade-layer.md | Documents the new guardrail and its invariants for the grader. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/signalforge/grade/parser.py (1)
103-108: ⚡ Quick winUpdate the module-level parser contract text to match the new prose-tolerant behavior.
The implementation now extracts embedded JSON from prose-wrapped responses, but the top docstring still states no prose extraction. Please align that docstring to avoid contract drift.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/signalforge/grade/parser.py` around lines 103 - 108, Update the module-level docstring to reflect that the parser now tolerates prose-wrapped responses and will extract an embedded JSON payload (via extract_json_payload after _strip_code_fence) rather than requiring strictly JSON-only responses; mention that extract_json_payload will return the original text when no JSON is found and that callers should still handle the error path, and update any wording that currently claims "no prose extraction" to state the new prose-tolerant extraction behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/rules/grade-layer.md:
- Line 57: Update the wording that currently says “first-brace-only” to
explicitly state it accepts the first `{` or `[` so it matches the helper
contract used by parse_grade_response (which routes through
signalforge._common.json_payload.extract_json_payload after _strip_code_fence)
and mirrors the llm-drafter.md tolerant JSON extraction rule; ensure the doc
mentions that a response with no JSON still produces
GradeOutputError(violation_type="json_parse") and follows the conservative
degrade path.
---
Nitpick comments:
In `@src/signalforge/grade/parser.py`:
- Around line 103-108: Update the module-level docstring to reflect that the
parser now tolerates prose-wrapped responses and will extract an embedded JSON
payload (via extract_json_payload after _strip_code_fence) rather than requiring
strictly JSON-only responses; mention that extract_json_payload will return the
original text when no JSON is found and that callers should still handle the
error path, and update any wording that currently claims "no prose extraction"
to state the new prose-tolerant extraction behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3e75b8d2-b1a7-4439-a794-28a16e8c9033
📒 Files selected for processing (8)
.claude/rules/grade-layer.md.claude/rules/llm-drafter.mdsrc/signalforge/_common/json_payload.pysrc/signalforge/draft/parser.pysrc/signalforge/grade/parser.pytests/_common/test_json_payload.pytests/draft/test_parser.pytests/grade/test_parser.py
- grade/parser.py: fix stale module docstring that still claimed the parser
does not extract JSON from prose (it now does, via extract_json_payload).
- tests/_common/test_json_payload.py: mark `unit` (not `draft`) to match
CONTRIBUTING.md's unit/integration/error convention + sibling _common tests.
- grade-layer.md: clarify the decode rule is "first structural char ({ or [)",
not object-only.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
Author
PR Review SummaryAll three inline comments were real and are fixed in 6fb05b9. Fixed (3 items)
False PositivesNone. Validation after fixes: |
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.
Fixes #144.
Problem
tests/cli/test_e2e_business_rules.py::test_e2e_business_rules_drafts_prunes_custom_sqlfailed reproducibly:claude-sonnet-4-6emits a reasoning preamble ("I need to analyze the business rules carefully...") before the JSON object on the business-rules drafting path, soparse_draft_response's strictmodel_validate_jsonraisedLLMOutputJSONErrorat line 1 (generate→ exit 2).Why not the issue's option 1 (assistant prefill)
The issue ranked assistant-turn prefill as most robust. It is not available on this model — the API rejects it with HTTP 400:
(Confirmed live: wiring
assistant_prefill="{"traded the parse error for a hard 400 / exit 3.) So the parser is the only place a JSON-only guarantee can live; prompts are advisory. This PR implements option 2 (tolerant parser).Fix
signalforge._common.json_payload.extract_json_payloadstrips a leading prose preamble (and trailing content) around a cleanly-decodable JSON value, applied in bothdraft/parser.pyandgrade/parser.pybefore strict parsing.Load-bearing rules:
{/[only. Scanning deeper would let a truncated outer object (first brace fails to decode) match a complete inner fragment, silently turning "not valid JSON" into a wrong-shape parse. On first-candidate failure the input is returned unchanged so the strict parser raises the normalLLMOutputJSONError/GradeOutputErrorwith the correct excerpt — truncated/garbage paths are byte-unchanged.raw_text(preamble included); onlymodel_validate_jsonsees the extracted payload, so audits/incident reports stay honest.response_text_hashis unchanged.Option 3 (prompt-level "JSON only") was deliberately skipped — it would rotate the cached system-prompt golden for no load-bearing gain once the parser is the guarantee.
Tests / validation
tests/_common/test_json_payload.py(preamble, code fence, trailing content, leading array, truncated-not-rescued, no-JSON passthrough).ruff✅ ·ruff format✅ ·pyright0/0/0 ✅ · full suite 2416 passed.-m e2e, real BigQuery + Anthropic): the exact Business-rules custom_sql drafting reproducibly fails: LLM emits prose preamble before JSON (no assistant prefill) #144 repro test is now green (1 passed in 418s).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Documentation