fix(llm): slice every text document type, not just five of them (#2900) - #2902
fix(llm): slice every text document type, not just five of them (#2900)#2902abhay-codes07 wants to merge 1 commit into
Conversation
…hify-Labs#2900) _read_files caps each unit at _FILE_CHAR_CAP (20,000 characters) before joining it into the user message. expand_oversized_files exists so an oversized document is split into contiguous FileSlices covering the whole file -- but it only splits suffixes in _SPLITTABLE_TEXT_SUFFIXES, and that list drifted from detect.DOC_EXTENSIONS as the latter grew. reach the semantic pass : .html .md .mdx .pdf .qmd .rst .skill .txt .yaml .yml sliced when oversized : .markdown .md .mdx .rst .txt never sliced : .html .pdf .qmd .skill .yaml .yml Measured on a 38,411-character fixture against the 20,000 cap: .md units=3 chars reaching the LLM=38411 LOST=0 .qmd units=1 chars reaching the LLM=20000 LOST=18411 .html units=1 chars reaching the LLM=20000 LOST=18411 .yaml units=1 chars reaching the LLM=20000 LOST=18411 .skill units=1 chars reaching the LLM=20000 LOST=18411 Everything past 20k was invisible to the semantic pass, so nothing in it could become a node -- with no warning, no _partial_files marker (that machinery tracks truncated RESPONSES, not truncated inputs), and nothing in the report. The graph looks complete. graphify's own .skill files are in the gap. These five are plain text and _file_to_text reads them with a straight read_text, so a character range over the raw file is exactly what the model is shown -- they slice identically to .md, which has worked this way since Graphify-Labs#1369. .pdf is deliberately NOT added. Its bytes are not what the model sees: _file_to_text routes it through extract_pdf_text while read_slice_text does path.read_text(), so slicing it needs the converter threaded through the slicing path first. Left alone rather than half-done -- a paper over 20k characters is still truncated today. The contract test is the durable half: it asserts every non-binary DOC_EXTENSION is splittable, so adding a document type without deciding how it gets sliced fails loudly instead of quietly losing the tail.
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Graphify review — findings
Adds .qmd, .skill, .html, .yaml, and .yml to _SPLITTABLE_TEXT_SUFFIXES so oversized documents of those types get sliced by expand_oversized_files instead of being silently truncated at _FILE_CHAR_CAP (#2900). Adds tests/test_oversized_document_slicing.py, whose contract test fails when a new DOC_EXTENSIONS suffix isn't classified as splittable or binary, preventing the two lists from drifting apart again.
Worth a look
- .html now sliced as raw text but is not what the model is shown —
graphify/file_slice.py:38· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review Execution auto-disposal is off for this run; enable it (with sandbox isolation) to have Graphify try to confirm or refute this automatically.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 281 functions depend on the 30 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract_files_direct()— 17 callers, 20 callees - new:
extract_corpus_parallel()— 26 callers, 10 callees - new:
dispatch_command()— 2 callers, 117 callees - new:
_extract_with_adaptive_retry()— 18 callers, 10 callees - new:
_read_files()— 8 callers, 6 callees - new:
expand_oversized_files()— 16 callers, 3 callees - new:
_pack_chunks_by_tokens()— 10 callers, 3 callees - new:
_bind_node_evidence()— 4 callers, 3 callees - …and 1 more — each is listed as a finding
Verification — 281 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 135 function(s) in the blast radius were not formally verified this run
· 9 more finding(s) on lines outside this diff (see the check run).
Fixes #2900.
The bug
_read_filescaps every unit at_FILE_CHAR_CAP(20,000 characters) before joining it into the user message.expand_oversized_filesexists precisely so an oversized document is split into contiguousFileSlices covering the whole file — but it only splits suffixes listed in_SPLITTABLE_TEXT_SUFFIXES, and that list drifted fromdetect.DOC_EXTENSIONSas the latter grew.One 38,411-character document, same content, five extensions:
Why it stays invisible
Nothing reports it. There is no warning, and no
_partial_filesmarker — that machinery tracks truncated responses, not truncated inputs. Since the semantic pass never sees the tail, nothing in it can become a node, so the loss is undetectable from inside the graph too: a document whose second half was cut looks exactly like one whose second half had nothing in it..mdhas worked correctly since #1369. The other document types were added toDOC_EXTENSIONSlater and the splittable list was never updated to match. graphify's own.skillfiles are in the gap, and they are well over 20k characters.The change
Add the plain-text document types to
_SPLITTABLE_TEXT_SUFFIXES. All five are read by_file_to_textwith a straightread_text, so a character range over the raw file is exactly what the model is shown — they slice identically to.md, through the sameslice_boundariesthat already prefers heading, paragraph and line breaks.The contract test is the durable half. The root cause is two lists that must agree and silently didn't, so the fix asserts every non-binary
DOC_EXTENSIONis splittable. Adding a document type without deciding how it gets sliced now fails loudly instead of quietly losing tails..pdfis deliberately left outIts bytes are not what the model sees:
_file_to_textroutes it throughextract_pdf_text, whileread_slice_textdoespath.read_text(). Adding.pdfto the set without threading the converter through the slicing path would slice raw PDF bytes — worse than the current behaviour.So I have not fixed it, and I want to be plain that a paper over 20,000 characters is still truncated today. Papers are among the longest things anyone points graphify at, so this deserves its own change; happy to take it if you want the converter threaded through.
There is also a small mirror inconsistency I left alone:
.markdownis in_SPLITTABLE_TEXT_SUFFIXESbut not inDOC_EXTENSIONS, so it is never classified as a document in the first place. Fixing that is a detection change, not a slicing one.Tests
tests/test_oversized_document_slicing.py(19 tests): the contract above, then per-extension checks that an oversized document is sliced and that the slices tile it exactly ("".join(read_slice_text(u)) == original— gap-free, no overlap, nothing dropped), an end-to-end check that a marker past the 20k boundary actually reaches the prompt, and guards that small files, code files and PDFs still pass through unsliced.Reverting
file_slice.pyand keeping the tests fails 11 of 19.Validation
Windows 11, Python 3.12, branched off
b14b52e(0.9.47).15 failed, 4738 passed->15 failed, 4757 passed. Identical failure set — no regressions; the +19 are the new tests.