populate rag_chunks#1
Merged
Merged
Conversation
Anxhela21
pushed a commit
that referenced
this pull request
Apr 30, 2026
Apply CodeRabbit's actionable comments and the per-comment nits:
1. PoC results section in spike doc previously listed paths under
poc-results/ that are deleted before merge per howto-run-a-spike.md
step 10, leaving broken links in the merged document. Replace the
file list with a self-contained summary of what the PoC proved
plus the heading-degradation finding, and a note pointing future
readers at the PR diff if the raw artifacts are ever needed.
2. Drop the reference to docs/local-stack-testing.md (a local-only
file, never committed to the repo).
3. Replace fragile line-numbered references (document_processor.py:75,
:87, byok_guide.md ~106-118) with stable symbol anchors:
_BaseDB.__init__, _LlamaStackDB.__init__, "Knowledge Sources"
subsection, "Step 1" subsection. Line numbers rot; section names
and symbol names rot less.
4. Spec doc now instructs the implementation ticket to extract the
("markdown", "html", "pdf") predicate to a single
MARKDOWN_COMPATIBLE_DOC_TYPES: Final[tuple[str, ...]] constant in
document_processor.py and reference it from both call sites,
instead of duplicating the tuple. JIRA #1 scope updated to match.
5. Add R7: PDFReader.load_data emits a logger.warning when its docling
output is empty / under a small threshold (a likely indicator of a
scanned PDF given R5's no-OCR scope). Threshold is a module-level
Final[int] constant. JIRA #1 scope and JIRA lightspeed-core#2 test patterns
updated to require coverage via caplog. Surfacing the silent-
degradation case in custom_processor.py logs costs nothing and
makes the OCR-needed signal visible.
Plus the two reviewer nits worth carrying into JIRA #1:
- Use docling's TableFormerMode.ACCURATE enum, not the string literal
"accurate"; both work via Pydantic coercion but the enum is
type-checked.
- Mirror HTMLReader's choice on whether to call super().__init__();
llama-index's BaseReader does not require it but symmetry between
the two readers is preferred.
The spec doc changelog records this revision and its trigger (the
PR lightspeed-core#1598 CodeRabbit review).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Type of change
🤯
Related Tickets & Documents
Checklist before requesting a review
Testing