fix: improve JSON extraction and TOC fallback handling#333
Conversation
|
Hi maintainers, I wanted to check whether this scope is aligned with the project direction. The PR is focused on JSON response resilience and TOC fallback handling, with unittest coverage. If this is too broad, I am happy to split it into a smaller parser-only PR first, then a separate TOC fallback PR. |
|
@KairosMarco Nice hardening PR — the defensive 1. (please fix) extract_json('{"thinking": "line1\nline2", "answer": "yes"}')
# before -> {'thinking': 'line1 line2', 'answer': 'yes'}
# after -> {}Many prompts here ask for a multi-line return json.loads(json_content, strict=False)
...
decoder = json.JSONDecoder(strict=False)2. (minor) 3. (minor) Also a tiny nit: |
|
Thanks for the careful review. I pushed a follow-up commit addressing the requested items:
Validation run locally: All passed. |
…esilience # Conflicts: # pageindex/page_index.py
|
Thanks for the quick turnaround — I verified the follow-up and all four items are fixed correctly (unescaped newlines parse, One same-class regression slipped through, and it points at a bigger simplification. Bug: the trailing-comma cleanup in extract_json('{"note": "see items,] and more"}') # -> {'note': 'see items] and more'} (comma eaten)
extract_json('{"formula": "f(x,} )"}') # -> {'formula': 'f(x} )'}(Regression vs the old code, which only ran the comma cleanup in the fallback after the first parse failed, so valid JSON never hit it.) Suggestion: rather than keep growing a hand-rolled repairer, consider
That last one is a bonus: it recovers truncated responses structurally, which could reduce reliance on the "continue generation" retry loops (they cost extra LLM calls).
from json_repair import repair_json
def extract_json(content):
if not content:
return {}
return repair_json(content, return_objects=True)and Caveat: If you'd rather keep the manual approach, the minimal fix is to make the trailing-comma removal string-aware too (fold it into the same scan as the literals). WDYT? |
|
Thanks again for catching this. I pushed a minimal follow-up fix in I kept the current manual path for this PR to avoid adding a new runtime dependency in the same hardening change, but I agree Changes in this follow-up:
Local validation: All passed. The full unittest run emitted only a LiteLLM remote model-cost-map timeout warning and fell back to the local backup. |
|
I also did one more same-class pass over this PR and pushed a small follow-up in Additional cleanup/hardening:
Validation re-run: All passed. |
Summary
This PR improves PageIndex robustness when LLM calls return JSON in common non-ideal formats or omit optional fields during TOC/page-index extraction.
It keeps the existing indexing flow unchanged, but adds safer parsing and fallback behavior for provider responses that include:
physical_indexvalues.Why
While running PageIndex over a FinanceBench PDF subset, I saw indexing failures from model response shape issues such as:
The failures were not specific to one document format; they came from LLM JSON formatting variance.
Changes
extract_json()tolerate fenced JSON, embedded JSON, arrays, trailing text, and Python-style literal tokens.list[dict]before list operations.Processing failedafter fallback attempts.unittestcoverage for the JSON parser and TOC fallback helpers.Validation
Local result:
I also validated equivalent fixes in a local benchmark workspace:
The benchmark artifacts are available here:
https://github.com/KairosMarco/pageindex-benchlab