Every RAG pipeline starts the same way: you have stuff, and the model needs Documents.
The interesting question is how far that idea stretches. So...
For further actions, you may consider blocking this person and/or reporting abuse
The ExcelLoader stopping at the first blank row is the default I would surface most prominently at ingestion time. A hand-edited sheet can contain several tables separated by blank rows, so a successful load can silently omit valid rows rather than fail parsing.
I would emit a diagnostic when non-empty cells occur after the stopping row and retain sheet and row coordinates with each batch. Formula text also needs an explicit representation label: an answer should not describe a formula string as its computed value. Those small provenance details would preserve the common Document interface while making source-specific omissions visible downstream.
You read the code right, and it is worth spelling out exactly how silent it is:
readRow()returnsnullwhen every cell in a row isBLANKor_NONE, andload()responds with a plainbreak— so a sheet with several tables separated by blank rows loads only the first one, successfully, with no signal. Metadata today is justtype=exceltoo; batches are capped bydocumentMaxRows(default 200) but carry no sheet or row coordinates.Your two suggestions would both be real improvements: a diagnostic when non-empty cells appear after the stopping row would turn silent data loss into a warning, and retaining sheet/row coordinates would make the omission visible downstream. This is exactly the kind of thing that belongs in the repo — worth an issue on GitHub if you are up for it.
The blank-row break and missing coordinates make a compact reproduction possible: two tables separated by one fully blank row, with a nonempty row immediately after it. The expected diagnostic can name the stopping row without changing the existing loader behavior.
I cannot take on an individual repository review, but that fixture plus expected loaded-row count would give maintainers a concrete starting point. If a continue-past-blanks option is added later, preserve the original sheet and row coordinates across batches so citations do not shift when the batch size changes.
Your fixture sketch is exactly the right shape — two tables separated by one blank row, non-empty row after it, and the assertion names the stopping row plus the expected loaded-row count. I checked the current source once more to pin down what the test would assert:
readRow()returns null when every cell is_NONE/BLANK, and the loop does a hardbreakon the first null — so the second table never entersreadAll, and since the sheet'sreadAllis empty for that sheet... actually with a leading table it will contain only table one's rows. The quiet part is the absence of any signal: no log, no partial marker, and the batch output has no sheet/row coordinates either (documentMaxRowssplits by position only), so nothing downstream can even tell where the cutoff happened. That makes your "expected diagnostic names the stopping row" assertion the most valuable line of the fixture.On the coordinates-when-continue-past-blanks point: agreed, and it's worth spelling out why. Batching is
subList(start, end)over the flattenedreadAll— indices are recomputed per batch size, so if coordinates are derived from batch-relative position, changingdocumentMaxRowschanges them. The durable fix is to capture sheet index + physical row index at read time (they're available in thefor (Row row : sheet)iteration) and carry them in metadata, so citations key on source-of-truth positions rather than batch layout. I'd add one more line to your fixture: assert coordinates stay stable whendocumentMaxRowschanges from 200 to 5 with the same file.We've now collected enough substance on this (your original report, the reproduction fixture, and the coordinate design) that I'm drafting it as a GitHub issue for the maintainers rather than letting it live only in comments — your fixture would be the anchor. If you'd like to be named in the report for the repro, say so and I'll credit the comment; otherwise I'll link it as-is.
Capturing physical coordinates at read time is the durable boundary. I would extend that fixture with an empty row that is absent from the worksheet XML and another that exists but contains only blank cells, since iteration can expose those differently.
Both should preserve the original coordinates of later nonempty rows when documentMaxRows changes. The report can distinguish an intentionally skipped blank from a premature end-of-sheet decision. That would make the issue useful across spreadsheet producers, rather than only for the particular file that first revealed the cutoff.
Nice tour of the loaders, the jsoup-based HTML one is where I've been bitten most. One cheap guard that helped: check the extracted text length before chunking, and if it comes back short or empty (typical for JS-rendered pages), route that URL to a headless render step instead of indexing nav and footer boilerplate. It keeps a surprising amount of junk out of the vector store.
Agreed — and the current code makes that guard even more important than it looks.
HtmlSimpleLoaderis deliberately minimal: it runsJsoup.parse(...)and returnssoup.body().text()as a single document, keeping onlytitle/meta description/langin metadata. There is no length check at all, so a JS-only page indexes its nav and cookie-banner text just as happily as real content.The escalate-on-thin-text pattern you describe wraps this cleanly: keep
HtmlSimpleLoaderon the fast path, check the extracted text length, and only route near-empty extractions to a headless render. Same idea applies when the page comes from web search — in that flow the Tavily provider insolon-ai-rag-searchsalso exposes Extract, which can serve as the escalation step without running your own browser fleet.That matches what we see in practice — once you know HtmlSimpleLoader is intentionally a single soup.body().text() with no floor, a thin-text / low-word-count gate before any heavier pass stops the nav-and-banner false positives cold. Nice that the escalate decision can sit outside the loader itself.