Uh oh!
There was an error while loading. Please reload this page.
fix: improve JSON extraction and TOC fallback handling - #333
Conversation
KairosMarco
commented
Jun 23, 2026
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 returnjson.loads(json_content, strict=False)
...
decoder=json.JSONDecoder(strict=False)2. (minor) 3. (minor) Also a tiny nit: |
KairosMarco
commented
Jul 6, 2026
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
KylinMountain
commented
Jul 6, 2026
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).
fromjson_repairimportrepair_jsondefextract_json(content):
ifnotcontent:
return {}
returnrepair_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? |
KairosMarco
commented
Jul 7, 2026
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. |
KairosMarco
commented
Jul 7, 2026
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. |
BukeLy
commented
Aug 31, 2026
Thanks for the contribution. Closing this PR because later JSON/Markdown/parser and error-recovery changes now cover or replace this legacy implementation. Keeping this branch open would duplicate current code paths, so it is no longer suitable to merge as-is. |
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