From fff4579f688f425ba1f1c4fecbc6c226fb3d2ce3 Mon Sep 17 00:00:00 2001 From: Shashank Shekhar Singh Date: Fri, 31 Jul 2026 22:33:51 +0530 Subject: [PATCH] Rank object spans ahead of array spans in JSON extraction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit extract_json ranked balanced spans by length alone, so a prose bracket longer than the answer still hijacked it: a citation list like [101, 205, 309, ...] outranked the real {"supported": false} verdict, resurrecting the substituted-answer bug the module records as fixed. Downstream, the verifier false-rejected correct claims ("reviewer reply unparseable — failing closed"), claim extraction dropped every claim, and three such planner replies stopped a GovernedLoop with planning_failed. Prose brackets are square while every shipped caller — the verifier's verdict, claim extraction, the planner's Subgraph — expects a top-level object, so balanced {...} spans now rank longest-first ahead of [...] spans longest-first. The whole-reply and fenced candidates still come first, so fenced and unfenced top-level arrays keep working, and junk still returns None so fail-closed is unchanged. Fixes #21 Co-Authored-By: Claude Fable 5 --- README.md | 2 +- grapharc/runtime/parsing.py | 19 +++++++++++++------ tests/test_parsing.py | 20 ++++++++++++++++++++ 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index dbf58a8..60ab4a2 100644 --- a/README.md +++ b/README.md @@ -493,7 +493,7 @@ Re-derived on 2026-07-28 by running each item, not by reading the commit log. - **`run_command` is not confined.** Argv-only and never a shell, but the child is an ordinary process with your privileges. - *Closed:* a `max_seconds` past the platform's `time_t` — `float("inf")`, or a plausible "effectively unlimited" like `1e10` — used to **disable the deadline guard for the rest of the process**. `setitimer` raised *after* the SIGALRM handler was installed and the process-wide slot taken, leaking both, so every later run silently fell back to the mechanism that cannot unwind a blocking syscall: a 0.3s ceiling then took a 5s sleep to notice. Arming is undone on failure now, and the armed delay is clamped to what both mechanisms accept. - *Closed:* every `async def` node **double-charged** its token re-reports. The re-report ledger was keyed by thread ident, but `on_llm_end` is sync — under `ainvoke` LangChain dispatches it to a worker thread while the body stays on the event loop — so the automatic charge found no ledger and the node's named re-report was charged again. Any node using the shipped `charge_usage`, `AgentNode._charge_tokens` or `planner.proposal._charge` reported double its real spend and hit `max_tokens` at half its declared allowance. The ledger is a `contextvars` scope now, which also fixes an inner scope discarding the enclosing node's. -- *Closed:* a bracket anywhere in a model's **prose** hijacked JSON extraction, because only the first `{`/`[` was ever tried. `Based on the context [lines 3-5]: {…}` was rejected as unparseable, and — worse — `Analysis (note [1]): {"supported": false}` returned a perfectly valid `[1]`, substituting a fabricated value for the verifier's actual answer. Every opener is tried now, longest parse wins; junk still returns `None`, so fail-closed is unchanged. +- *Closed:* a bracket anywhere in a model's **prose** hijacked JSON extraction, because only the first `{`/`[` was ever tried. `Based on the context [lines 3-5]: {…}` was rejected as unparseable, and — worse — `Analysis (note [1]): {"supported": false}` returned a perfectly valid `[1]`, substituting a fabricated value for the verifier's actual answer. Every opener is tried now, and length alone turned out not to be a safe rank — a citation list like `[101, 205, 309, …]` *longer* than the verdict still won — so object spans are tried before array spans, each longest-first; junk still returns `None`, so fail-closed is unchanged. - **`interrupt()` suspends but cannot be resumed.** LangGraph's native interrupt stops the graph and shows on `get_state`, and there is no supported resume path — resuming means passing a `Command` as *input*, which is closed by design. Use the session layer's approval gate for human-in-the-loop. - **Still unwrapped from LangGraph:** `retry_policy`, `cache_policy`, `durability`, subgraphs. `.inner` reaches them, but execution entry points there fail closed, so `.inner` is an inspection escape hatch and not a way to run the graph. - **Cost is recorded when a backend reports one, estimated when it does not.** Both gateways publish the provider's `cost_usd` through the same `llm_output` envelope, the runtime's usage callback writes it onto the node's `end` event, and an agent's `model` events carry the per-call breakdown. A backend that reports no price still falls back to a `RateCard` estimate, and the two figures stay apart — `recorded_cost_usd` is never a guess. Still missing: no tenant on a trace event, so per-tenant attribution is not offered. diff --git a/grapharc/runtime/parsing.py b/grapharc/runtime/parsing.py index 216cb02..c1d8ca8 100644 --- a/grapharc/runtime/parsing.py +++ b/grapharc/runtime/parsing.py @@ -50,7 +50,8 @@ def _span_from(text: str, start: int) -> str | None: def _balanced_spans(text: str) -> list[str]: - """Every balanced {...} or [...] region, longest first. + """Every balanced {...} or [...] region: objects longest-first, then arrays + longest-first. Every opener is tried, not just the first one in the text. Taking only the first meant any bracket in the model's *prose* hijacked the span and the real @@ -59,17 +60,23 @@ def _balanced_spans(text: str) -> list[str]: `Analysis (note [1]): {"supported": false}` yielded a perfectly valid `[1]`, substituting a fabricated value for the model's actual answer. - Longest first is what makes the ranking safe. It prefers a complete structure - over both a prose fragment that happens to parse and a nested piece of the - answer itself, so `{"claims": [{...}]}` returns the whole object rather than - the inner list. + Length alone was not sufficient to make the ranking safe. It does prefer a + complete structure over a nested piece of the answer itself — `{"claims": + [{...}]}` returns the whole object rather than the inner list — but a prose + fragment *longer* than the answer still won: a citation list like + `[101, 205, 309, ...]` outranked the real `{"supported": false}` verdict. + Prose brackets are square (`[1]`, `[lines 3-5]`, citation lists) while the + model's answer is a top-level object for every shipped caller, so object + spans rank ahead of array spans, each group longest-first. An array-only + reply still parses whole or fenced before any span is tried, so top-level + arrays remain reachable. """ spans = [ span for i, ch in enumerate(text) if ch in "{[" and (span := _span_from(text, i)) is not None ] - return sorted(spans, key=len, reverse=True) + return sorted(spans, key=lambda span: (span[0] != "{", -len(span))) def extract_json(content: Any) -> Any | None: diff --git a/tests/test_parsing.py b/tests/test_parsing.py index 7d120b3..e0c1a9c 100644 --- a/tests/test_parsing.py +++ b/tests/test_parsing.py @@ -67,6 +67,26 @@ def test_a_prose_bracket_is_never_substituted_for_the_answer(): assert extract_json(reply) == {"supported": False, "reason": "negated"} +@pytest.mark.parametrize("supported", [False, True]) +def test_a_long_prose_bracket_does_not_outrank_the_answer(supported): + """Length-only ranking let a citation list *longer* than the verdict win, so + the model's actual answer was never returned. Objects rank ahead of arrays: + prose brackets are square, and every shipped caller expects an object.""" + verdict = "false" if supported is False else "true" + reply = ( + "Verified against source lines [101, 205, 309, 412, 518, 622, 733, 848]: " + f'{{"supported": {verdict}}}' + ) + assert extract_json(reply) == {"supported": supported} + + +def test_an_array_answer_survives_a_brace_in_the_prose(): + """Ranking objects first must not strand array answers: `figure{2}` does not + parse, so the balanced-span fallback still reaches the unfenced array.""" + reply = "Matching ids, see figure{2}: [101, 205, 309]" + assert extract_json(reply) == [101, 205, 309] + + def test_the_outermost_structure_wins_over_a_nested_piece_of_it(): reply = 'Verdict: {"claims": [{"text": "a"}], "n": 1}' assert extract_json(reply) == {"claims": [{"text": "a"}], "n": 1}