PageIndex/pageindex/backend/local.py

377 lines
17 KiB
Python
Raw Normal View History

# pageindex/backend/local.py
import hashlib
import os
import re
import sqlite3
import uuid
import shutil
from pathlib import Path
from ..parser.protocol import DocumentParser, ParsedDocument
from ..parser.pdf import PdfParser
from ..parser.markdown import MarkdownParser
from ..storage.protocol import StorageEngine
from ..index.pipeline import build_index
fix: address xhigh code-review findings on 2d46d68..8f536cb Verified 12 findings from an xhigh-effort review of the prior review-fix batch; all confirmed real. Most trace back to one root cause: the build_index() text-stripping fix (8f536cb) correctly stopped Markdown from leaking full text by default, but broke every path that assumed text could be re-read later. Correctness: - LocalBackend._fill_node_text (get_document(include_text=True)) only handled PDF's start_index/end_index convention; Markdown nodes use line_num and got silently empty text. Now handles both. - get_page_content's Markdown fallback (triggered when a StorageEngine legitimately returns None from get_pages()) read from the now-text-stripped structure. It now re-derives from the source file, mirroring the PDF fallback, so it no longer depends on structure text at all. - add_document's PDF-only text-stripping branch (with the stale "markdown needs text in structure for fallback retrieval" comment) is now dead/wrong since build_index() already applies if_add_node_text uniformly — removed. - _validate_llm_provider's keyless-provider allowlist was missing several local LiteLLM providers (xinference, llamafile, triton, oobabooga, openai_like, docker_model_runner, custom, custom_openai, petals) that need no API key just like ollama/lm_studio; expanded. - The three agent-tool closures (get_document, get_document_structure, get_page_content) had three different not-found patterns; two bypassed the backend's DocumentNotFoundError entirely. Extracted LocalBackend. _require_document as the single existence check every method/tool now uses. - examples/agentic_vectorless_rag_demo.py's hand-rolled Agent() didn't apply the litellm/ prefix normalization the SDK does internally, so its own documented "any LiteLLM provider" claim broke for non-openai models. - cloud delete_collection's cache eviction removed the "folders unavailable" None sentinel too, forcing a wasted re-fetch; now only pops on a real id. Cleanup / altitude: - build_index() skips the remove_structure_text walk entirely when text was never added (content_based + if_add_node_summary=False + if_add_node_text= False) instead of a guaranteed no-op tree walk. - page_index()'s locals()-capture-as-kwargs (fragile by construction) replaced with an explicit dict of the named parameters. - run_pageindex.py's _cli_bool and the page_index_md.py legacy shim's _coerce_bool were duplicate, diverging implementations; both now bind directly to the canonical pageindex.index.page_index_md._coerce_bool. - retrieve.py's _get_md_page_content delegated its own traversal instead of calling the canonical get_md_page_content; now a one-line delegation. - FileTypeError's docstring now calls out the except-ordering gotcha from also subclassing ValueError. 17 new regression tests (tests/test_review_fixes_2.py) plus 2 updated in tests/test_legacy_shims.py for the simplified md_to_tree shim. Full suite: 210 passed, 2 skipped. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
2026-07-08 21:56:41 +08:00
from ..index.utils import parse_pages, get_pdf_page_content, remove_fields
from ..backend.protocol import AgentTools
from ..errors import (FileTypeError, DocumentNotFoundError, CollectionNotFoundError,
IndexingError, PageIndexError)
# Matched with .fullmatch() (not .match()): a $-anchored .match() would accept a
# trailing newline ("papers\n") because $ matches just before a final \n.
_COLLECTION_NAME_RE = re.compile(r'[a-zA-Z0-9_-]{1,128}')
class LocalBackend:
def __init__(self, storage: StorageEngine, files_dir: str, model: str = None,
retrieve_model: str = None, index_config=None):
self._storage = storage
self._files_dir = Path(files_dir)
self._model = model
self._retrieve_model = retrieve_model or model
self._index_config = index_config
self._parsers: list[DocumentParser] = [PdfParser(), MarkdownParser()]
def register_parser(self, parser: DocumentParser) -> None:
self._parsers.insert(0, parser) # user parsers checked first
def get_retrieve_model(self) -> str | None:
return self._retrieve_model
def _resolve_parser(self, file_path: str) -> DocumentParser:
ext = os.path.splitext(file_path)[1].lower()
for parser in self._parsers:
if ext in parser.supported_extensions():
return parser
raise FileTypeError(f"No parser for extension: {ext}")
# Collection management
def _validate_collection_name(self, name: str) -> None:
if not _COLLECTION_NAME_RE.fullmatch(name):
raise PageIndexError(f"Invalid collection name: {name!r}. Must be 1-128 chars of [a-zA-Z0-9_-].")
def create_collection(self, name: str) -> None:
self._validate_collection_name(name)
self._storage.create_collection(name)
def get_or_create_collection(self, name: str) -> None:
self._validate_collection_name(name)
self._storage.get_or_create_collection(name)
def list_collections(self) -> list[str]:
return self._storage.list_collections()
def delete_collection(self, name: str) -> None:
# Validate before touching the filesystem — an unvalidated name like
# "../.." would make the rmtree below escape files_dir entirely.
self._validate_collection_name(name)
self._storage.delete_collection(name)
col_dir = self._files_dir / name
if col_dir.exists():
shutil.rmtree(col_dir)
@staticmethod
def _file_hash(file_path: str) -> str:
"""Compute SHA-256 hash of a file."""
h = hashlib.sha256()
with open(file_path, "rb") as f:
for chunk in iter(lambda: f.read(65536), b""):
h.update(chunk)
return h.hexdigest()
# Document management
def add_document(self, collection: str, file_path: str) -> str:
file_path = os.path.realpath(file_path)
if not os.path.isfile(file_path):
# Missing path is a file-not-found error, not an unsupported-type one.
raise FileNotFoundError(f"No such file: {file_path}")
# Fail fast before the expensive parse + LLM indexing if the collection
# doesn't exist — otherwise the FK constraint only trips at save time,
# after the LLM work (and its cost) is already spent.
if collection not in self._storage.list_collections():
raise CollectionNotFoundError(
f"Collection '{collection}' does not exist; "
f"create it first (e.g. client.collection('{collection}'))."
)
parser = self._resolve_parser(file_path)
# Dedup is content-only — same file is reused regardless of IndexConfig
# changes. If you've changed IndexConfig and need a fresh tree, delete
# the existing doc first or use a new collection.
file_hash = self._file_hash(file_path)
existing_id = self._storage.find_document_by_hash(collection, file_hash)
if existing_id:
return existing_id
doc_id = str(uuid.uuid4())
# Copy file to managed directory
ext = os.path.splitext(file_path)[1]
col_dir = self._files_dir / collection
col_dir.mkdir(parents=True, exist_ok=True)
managed_path = col_dir / f"{doc_id}{ext}"
shutil.copy2(file_path, managed_path)
try:
# Store images alongside the document: files/{collection}/{doc_id}/images/
images_dir = str(col_dir / doc_id / "images")
parsed = parser.parse(file_path, model=self._model, images_dir=images_dir)
result = build_index(parsed, model=self._model, opt=self._index_config)
fix: address xhigh code-review findings on 2d46d68..8f536cb Verified 12 findings from an xhigh-effort review of the prior review-fix batch; all confirmed real. Most trace back to one root cause: the build_index() text-stripping fix (8f536cb) correctly stopped Markdown from leaking full text by default, but broke every path that assumed text could be re-read later. Correctness: - LocalBackend._fill_node_text (get_document(include_text=True)) only handled PDF's start_index/end_index convention; Markdown nodes use line_num and got silently empty text. Now handles both. - get_page_content's Markdown fallback (triggered when a StorageEngine legitimately returns None from get_pages()) read from the now-text-stripped structure. It now re-derives from the source file, mirroring the PDF fallback, so it no longer depends on structure text at all. - add_document's PDF-only text-stripping branch (with the stale "markdown needs text in structure for fallback retrieval" comment) is now dead/wrong since build_index() already applies if_add_node_text uniformly — removed. - _validate_llm_provider's keyless-provider allowlist was missing several local LiteLLM providers (xinference, llamafile, triton, oobabooga, openai_like, docker_model_runner, custom, custom_openai, petals) that need no API key just like ollama/lm_studio; expanded. - The three agent-tool closures (get_document, get_document_structure, get_page_content) had three different not-found patterns; two bypassed the backend's DocumentNotFoundError entirely. Extracted LocalBackend. _require_document as the single existence check every method/tool now uses. - examples/agentic_vectorless_rag_demo.py's hand-rolled Agent() didn't apply the litellm/ prefix normalization the SDK does internally, so its own documented "any LiteLLM provider" claim broke for non-openai models. - cloud delete_collection's cache eviction removed the "folders unavailable" None sentinel too, forcing a wasted re-fetch; now only pops on a real id. Cleanup / altitude: - build_index() skips the remove_structure_text walk entirely when text was never added (content_based + if_add_node_summary=False + if_add_node_text= False) instead of a guaranteed no-op tree walk. - page_index()'s locals()-capture-as-kwargs (fragile by construction) replaced with an explicit dict of the named parameters. - run_pageindex.py's _cli_bool and the page_index_md.py legacy shim's _coerce_bool were duplicate, diverging implementations; both now bind directly to the canonical pageindex.index.page_index_md._coerce_bool. - retrieve.py's _get_md_page_content delegated its own traversal instead of calling the canonical get_md_page_content; now a one-line delegation. - FileTypeError's docstring now calls out the except-ordering gotcha from also subclassing ValueError. 17 new regression tests (tests/test_review_fixes_2.py) plus 2 updated in tests/test_legacy_shims.py for the simplified md_to_tree shim. Full suite: 210 passed, 2 skipped. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
2026-07-08 21:56:41 +08:00
# Cache page text for fast retrieval (avoids re-reading files) and to
# reconstruct node text on demand (get_document(include_text=True),
# get_page_content fallback) independent of whether IndexConfig kept
# text in the stored structure. build_index() already applies
# if_add_node_text to result["structure"] for every strategy, so no
# extra stripping is needed here.
pages = [{"page": n.index, "content": n.content,
**({"images": n.images} if n.images else {})}
for n in parsed.nodes if n.content]
self._storage.save_document(collection, doc_id, {
"doc_name": parsed.doc_name,
"doc_description": result.get("doc_description", ""),
"file_path": str(managed_path),
"file_hash": file_hash,
fix: address xhigh code-review findings on 2d46d68..8f536cb Verified 12 findings from an xhigh-effort review of the prior review-fix batch; all confirmed real. Most trace back to one root cause: the build_index() text-stripping fix (8f536cb) correctly stopped Markdown from leaking full text by default, but broke every path that assumed text could be re-read later. Correctness: - LocalBackend._fill_node_text (get_document(include_text=True)) only handled PDF's start_index/end_index convention; Markdown nodes use line_num and got silently empty text. Now handles both. - get_page_content's Markdown fallback (triggered when a StorageEngine legitimately returns None from get_pages()) read from the now-text-stripped structure. It now re-derives from the source file, mirroring the PDF fallback, so it no longer depends on structure text at all. - add_document's PDF-only text-stripping branch (with the stale "markdown needs text in structure for fallback retrieval" comment) is now dead/wrong since build_index() already applies if_add_node_text uniformly — removed. - _validate_llm_provider's keyless-provider allowlist was missing several local LiteLLM providers (xinference, llamafile, triton, oobabooga, openai_like, docker_model_runner, custom, custom_openai, petals) that need no API key just like ollama/lm_studio; expanded. - The three agent-tool closures (get_document, get_document_structure, get_page_content) had three different not-found patterns; two bypassed the backend's DocumentNotFoundError entirely. Extracted LocalBackend. _require_document as the single existence check every method/tool now uses. - examples/agentic_vectorless_rag_demo.py's hand-rolled Agent() didn't apply the litellm/ prefix normalization the SDK does internally, so its own documented "any LiteLLM provider" claim broke for non-openai models. - cloud delete_collection's cache eviction removed the "folders unavailable" None sentinel too, forcing a wasted re-fetch; now only pops on a real id. Cleanup / altitude: - build_index() skips the remove_structure_text walk entirely when text was never added (content_based + if_add_node_summary=False + if_add_node_text= False) instead of a guaranteed no-op tree walk. - page_index()'s locals()-capture-as-kwargs (fragile by construction) replaced with an explicit dict of the named parameters. - run_pageindex.py's _cli_bool and the page_index_md.py legacy shim's _coerce_bool were duplicate, diverging implementations; both now bind directly to the canonical pageindex.index.page_index_md._coerce_bool. - retrieve.py's _get_md_page_content delegated its own traversal instead of calling the canonical get_md_page_content; now a one-line delegation. - FileTypeError's docstring now calls out the except-ordering gotcha from also subclassing ValueError. 17 new regression tests (tests/test_review_fixes_2.py) plus 2 updated in tests/test_legacy_shims.py for the simplified md_to_tree shim. Full suite: 210 passed, 2 skipped. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
2026-07-08 21:56:41 +08:00
"doc_type": ext.lstrip("."),
"structure": result["structure"],
"pages": pages,
})
except sqlite3.IntegrityError:
# Lost a concurrent add of the same content (UNIQUE collection+hash).
# Discard our managed files and return the winner's doc_id.
managed_path.unlink(missing_ok=True)
doc_dir = col_dir / doc_id
if doc_dir.exists():
shutil.rmtree(doc_dir)
existing_id = self._storage.find_document_by_hash(collection, file_hash)
if existing_id:
return existing_id
raise
except Exception as e:
managed_path.unlink(missing_ok=True)
doc_dir = col_dir / doc_id
if doc_dir.exists():
shutil.rmtree(doc_dir)
raise IndexingError(f"Failed to index {file_path}: {e}") from e
return doc_id
fix: address xhigh code-review findings on 2d46d68..8f536cb Verified 12 findings from an xhigh-effort review of the prior review-fix batch; all confirmed real. Most trace back to one root cause: the build_index() text-stripping fix (8f536cb) correctly stopped Markdown from leaking full text by default, but broke every path that assumed text could be re-read later. Correctness: - LocalBackend._fill_node_text (get_document(include_text=True)) only handled PDF's start_index/end_index convention; Markdown nodes use line_num and got silently empty text. Now handles both. - get_page_content's Markdown fallback (triggered when a StorageEngine legitimately returns None from get_pages()) read from the now-text-stripped structure. It now re-derives from the source file, mirroring the PDF fallback, so it no longer depends on structure text at all. - add_document's PDF-only text-stripping branch (with the stale "markdown needs text in structure for fallback retrieval" comment) is now dead/wrong since build_index() already applies if_add_node_text uniformly — removed. - _validate_llm_provider's keyless-provider allowlist was missing several local LiteLLM providers (xinference, llamafile, triton, oobabooga, openai_like, docker_model_runner, custom, custom_openai, petals) that need no API key just like ollama/lm_studio; expanded. - The three agent-tool closures (get_document, get_document_structure, get_page_content) had three different not-found patterns; two bypassed the backend's DocumentNotFoundError entirely. Extracted LocalBackend. _require_document as the single existence check every method/tool now uses. - examples/agentic_vectorless_rag_demo.py's hand-rolled Agent() didn't apply the litellm/ prefix normalization the SDK does internally, so its own documented "any LiteLLM provider" claim broke for non-openai models. - cloud delete_collection's cache eviction removed the "folders unavailable" None sentinel too, forcing a wasted re-fetch; now only pops on a real id. Cleanup / altitude: - build_index() skips the remove_structure_text walk entirely when text was never added (content_based + if_add_node_summary=False + if_add_node_text= False) instead of a guaranteed no-op tree walk. - page_index()'s locals()-capture-as-kwargs (fragile by construction) replaced with an explicit dict of the named parameters. - run_pageindex.py's _cli_bool and the page_index_md.py legacy shim's _coerce_bool were duplicate, diverging implementations; both now bind directly to the canonical pageindex.index.page_index_md._coerce_bool. - retrieve.py's _get_md_page_content delegated its own traversal instead of calling the canonical get_md_page_content; now a one-line delegation. - FileTypeError's docstring now calls out the except-ordering gotcha from also subclassing ValueError. 17 new regression tests (tests/test_review_fixes_2.py) plus 2 updated in tests/test_legacy_shims.py for the simplified md_to_tree shim. Full suite: 210 passed, 2 skipped. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
2026-07-08 21:56:41 +08:00
def _require_document(self, collection: str, doc_id: str) -> dict:
"""Return the document's storage row, or raise DocumentNotFoundError.
Single source of truth for "does this doc exist" every public method
and agent tool below goes through this, so a missing doc always
surfaces the same way instead of each caller re-implementing its own
(and potentially inconsistent) existence check.
"""
doc = self._storage.get_document(collection, doc_id)
if not doc:
raise DocumentNotFoundError(f"Document {doc_id} not found")
return doc
def get_document(self, collection: str, doc_id: str, include_text: bool = False) -> dict:
"""Get document metadata with structure.
Args:
include_text: If True, populate each structure node's 'text' field
from cached page content. WARNING: may be very large do NOT
use in agent/LLM contexts as it can exhaust the context window.
"""
fix: address xhigh code-review findings on 2d46d68..8f536cb Verified 12 findings from an xhigh-effort review of the prior review-fix batch; all confirmed real. Most trace back to one root cause: the build_index() text-stripping fix (8f536cb) correctly stopped Markdown from leaking full text by default, but broke every path that assumed text could be re-read later. Correctness: - LocalBackend._fill_node_text (get_document(include_text=True)) only handled PDF's start_index/end_index convention; Markdown nodes use line_num and got silently empty text. Now handles both. - get_page_content's Markdown fallback (triggered when a StorageEngine legitimately returns None from get_pages()) read from the now-text-stripped structure. It now re-derives from the source file, mirroring the PDF fallback, so it no longer depends on structure text at all. - add_document's PDF-only text-stripping branch (with the stale "markdown needs text in structure for fallback retrieval" comment) is now dead/wrong since build_index() already applies if_add_node_text uniformly — removed. - _validate_llm_provider's keyless-provider allowlist was missing several local LiteLLM providers (xinference, llamafile, triton, oobabooga, openai_like, docker_model_runner, custom, custom_openai, petals) that need no API key just like ollama/lm_studio; expanded. - The three agent-tool closures (get_document, get_document_structure, get_page_content) had three different not-found patterns; two bypassed the backend's DocumentNotFoundError entirely. Extracted LocalBackend. _require_document as the single existence check every method/tool now uses. - examples/agentic_vectorless_rag_demo.py's hand-rolled Agent() didn't apply the litellm/ prefix normalization the SDK does internally, so its own documented "any LiteLLM provider" claim broke for non-openai models. - cloud delete_collection's cache eviction removed the "folders unavailable" None sentinel too, forcing a wasted re-fetch; now only pops on a real id. Cleanup / altitude: - build_index() skips the remove_structure_text walk entirely when text was never added (content_based + if_add_node_summary=False + if_add_node_text= False) instead of a guaranteed no-op tree walk. - page_index()'s locals()-capture-as-kwargs (fragile by construction) replaced with an explicit dict of the named parameters. - run_pageindex.py's _cli_bool and the page_index_md.py legacy shim's _coerce_bool were duplicate, diverging implementations; both now bind directly to the canonical pageindex.index.page_index_md._coerce_bool. - retrieve.py's _get_md_page_content delegated its own traversal instead of calling the canonical get_md_page_content; now a one-line delegation. - FileTypeError's docstring now calls out the except-ordering gotcha from also subclassing ValueError. 17 new regression tests (tests/test_review_fixes_2.py) plus 2 updated in tests/test_legacy_shims.py for the simplified md_to_tree shim. Full suite: 210 passed, 2 skipped. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
2026-07-08 21:56:41 +08:00
doc = self._require_document(collection, doc_id)
doc["structure"] = self._storage.get_document_structure(collection, doc_id)
if include_text:
pages = self._storage.get_pages(collection, doc_id) or []
page_map = {p["page"]: p["content"] for p in pages}
self._fill_node_text(doc["structure"], page_map)
return doc
@staticmethod
def _fill_node_text(nodes: list, page_map: dict) -> None:
fix: address xhigh code-review findings on 2d46d68..8f536cb Verified 12 findings from an xhigh-effort review of the prior review-fix batch; all confirmed real. Most trace back to one root cause: the build_index() text-stripping fix (8f536cb) correctly stopped Markdown from leaking full text by default, but broke every path that assumed text could be re-read later. Correctness: - LocalBackend._fill_node_text (get_document(include_text=True)) only handled PDF's start_index/end_index convention; Markdown nodes use line_num and got silently empty text. Now handles both. - get_page_content's Markdown fallback (triggered when a StorageEngine legitimately returns None from get_pages()) read from the now-text-stripped structure. It now re-derives from the source file, mirroring the PDF fallback, so it no longer depends on structure text at all. - add_document's PDF-only text-stripping branch (with the stale "markdown needs text in structure for fallback retrieval" comment) is now dead/wrong since build_index() already applies if_add_node_text uniformly — removed. - _validate_llm_provider's keyless-provider allowlist was missing several local LiteLLM providers (xinference, llamafile, triton, oobabooga, openai_like, docker_model_runner, custom, custom_openai, petals) that need no API key just like ollama/lm_studio; expanded. - The three agent-tool closures (get_document, get_document_structure, get_page_content) had three different not-found patterns; two bypassed the backend's DocumentNotFoundError entirely. Extracted LocalBackend. _require_document as the single existence check every method/tool now uses. - examples/agentic_vectorless_rag_demo.py's hand-rolled Agent() didn't apply the litellm/ prefix normalization the SDK does internally, so its own documented "any LiteLLM provider" claim broke for non-openai models. - cloud delete_collection's cache eviction removed the "folders unavailable" None sentinel too, forcing a wasted re-fetch; now only pops on a real id. Cleanup / altitude: - build_index() skips the remove_structure_text walk entirely when text was never added (content_based + if_add_node_summary=False + if_add_node_text= False) instead of a guaranteed no-op tree walk. - page_index()'s locals()-capture-as-kwargs (fragile by construction) replaced with an explicit dict of the named parameters. - run_pageindex.py's _cli_bool and the page_index_md.py legacy shim's _coerce_bool were duplicate, diverging implementations; both now bind directly to the canonical pageindex.index.page_index_md._coerce_bool. - retrieve.py's _get_md_page_content delegated its own traversal instead of calling the canonical get_md_page_content; now a one-line delegation. - FileTypeError's docstring now calls out the except-ordering gotcha from also subclassing ValueError. 17 new regression tests (tests/test_review_fixes_2.py) plus 2 updated in tests/test_legacy_shims.py for the simplified md_to_tree shim. Full suite: 210 passed, 2 skipped. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
2026-07-08 21:56:41 +08:00
"""Recursively fill 'text' on structure nodes from cached page content.
Two node conventions, one per indexing strategy: content_based (PDF)
nodes span a start_index..end_index page range; level_based (Markdown)
nodes map 1:1 to a single page keyed by line_num. Handling only the
first would silently leave Markdown nodes with no text.
"""
for node in nodes:
start = node.get("start_index")
end = node.get("end_index")
if start is not None and end is not None:
node["text"] = "\n".join(
page_map.get(p, "") for p in range(start, end + 1)
)
fix: address xhigh code-review findings on 2d46d68..8f536cb Verified 12 findings from an xhigh-effort review of the prior review-fix batch; all confirmed real. Most trace back to one root cause: the build_index() text-stripping fix (8f536cb) correctly stopped Markdown from leaking full text by default, but broke every path that assumed text could be re-read later. Correctness: - LocalBackend._fill_node_text (get_document(include_text=True)) only handled PDF's start_index/end_index convention; Markdown nodes use line_num and got silently empty text. Now handles both. - get_page_content's Markdown fallback (triggered when a StorageEngine legitimately returns None from get_pages()) read from the now-text-stripped structure. It now re-derives from the source file, mirroring the PDF fallback, so it no longer depends on structure text at all. - add_document's PDF-only text-stripping branch (with the stale "markdown needs text in structure for fallback retrieval" comment) is now dead/wrong since build_index() already applies if_add_node_text uniformly — removed. - _validate_llm_provider's keyless-provider allowlist was missing several local LiteLLM providers (xinference, llamafile, triton, oobabooga, openai_like, docker_model_runner, custom, custom_openai, petals) that need no API key just like ollama/lm_studio; expanded. - The three agent-tool closures (get_document, get_document_structure, get_page_content) had three different not-found patterns; two bypassed the backend's DocumentNotFoundError entirely. Extracted LocalBackend. _require_document as the single existence check every method/tool now uses. - examples/agentic_vectorless_rag_demo.py's hand-rolled Agent() didn't apply the litellm/ prefix normalization the SDK does internally, so its own documented "any LiteLLM provider" claim broke for non-openai models. - cloud delete_collection's cache eviction removed the "folders unavailable" None sentinel too, forcing a wasted re-fetch; now only pops on a real id. Cleanup / altitude: - build_index() skips the remove_structure_text walk entirely when text was never added (content_based + if_add_node_summary=False + if_add_node_text= False) instead of a guaranteed no-op tree walk. - page_index()'s locals()-capture-as-kwargs (fragile by construction) replaced with an explicit dict of the named parameters. - run_pageindex.py's _cli_bool and the page_index_md.py legacy shim's _coerce_bool were duplicate, diverging implementations; both now bind directly to the canonical pageindex.index.page_index_md._coerce_bool. - retrieve.py's _get_md_page_content delegated its own traversal instead of calling the canonical get_md_page_content; now a one-line delegation. - FileTypeError's docstring now calls out the except-ordering gotcha from also subclassing ValueError. 17 new regression tests (tests/test_review_fixes_2.py) plus 2 updated in tests/test_legacy_shims.py for the simplified md_to_tree shim. Full suite: 210 passed, 2 skipped. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
2026-07-08 21:56:41 +08:00
elif "line_num" in node:
node["text"] = page_map.get(node["line_num"], "")
if "nodes" in node:
LocalBackend._fill_node_text(node["nodes"], page_map)
def get_document_structure(self, collection: str, doc_id: str) -> list:
fix: address xhigh code-review findings on 2d46d68..8f536cb Verified 12 findings from an xhigh-effort review of the prior review-fix batch; all confirmed real. Most trace back to one root cause: the build_index() text-stripping fix (8f536cb) correctly stopped Markdown from leaking full text by default, but broke every path that assumed text could be re-read later. Correctness: - LocalBackend._fill_node_text (get_document(include_text=True)) only handled PDF's start_index/end_index convention; Markdown nodes use line_num and got silently empty text. Now handles both. - get_page_content's Markdown fallback (triggered when a StorageEngine legitimately returns None from get_pages()) read from the now-text-stripped structure. It now re-derives from the source file, mirroring the PDF fallback, so it no longer depends on structure text at all. - add_document's PDF-only text-stripping branch (with the stale "markdown needs text in structure for fallback retrieval" comment) is now dead/wrong since build_index() already applies if_add_node_text uniformly — removed. - _validate_llm_provider's keyless-provider allowlist was missing several local LiteLLM providers (xinference, llamafile, triton, oobabooga, openai_like, docker_model_runner, custom, custom_openai, petals) that need no API key just like ollama/lm_studio; expanded. - The three agent-tool closures (get_document, get_document_structure, get_page_content) had three different not-found patterns; two bypassed the backend's DocumentNotFoundError entirely. Extracted LocalBackend. _require_document as the single existence check every method/tool now uses. - examples/agentic_vectorless_rag_demo.py's hand-rolled Agent() didn't apply the litellm/ prefix normalization the SDK does internally, so its own documented "any LiteLLM provider" claim broke for non-openai models. - cloud delete_collection's cache eviction removed the "folders unavailable" None sentinel too, forcing a wasted re-fetch; now only pops on a real id. Cleanup / altitude: - build_index() skips the remove_structure_text walk entirely when text was never added (content_based + if_add_node_summary=False + if_add_node_text= False) instead of a guaranteed no-op tree walk. - page_index()'s locals()-capture-as-kwargs (fragile by construction) replaced with an explicit dict of the named parameters. - run_pageindex.py's _cli_bool and the page_index_md.py legacy shim's _coerce_bool were duplicate, diverging implementations; both now bind directly to the canonical pageindex.index.page_index_md._coerce_bool. - retrieve.py's _get_md_page_content delegated its own traversal instead of calling the canonical get_md_page_content; now a one-line delegation. - FileTypeError's docstring now calls out the except-ordering gotcha from also subclassing ValueError. 17 new regression tests (tests/test_review_fixes_2.py) plus 2 updated in tests/test_legacy_shims.py for the simplified md_to_tree shim. Full suite: 210 passed, 2 skipped. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
2026-07-08 21:56:41 +08:00
self._require_document(collection, doc_id)
return self._storage.get_document_structure(collection, doc_id)
def get_page_content(self, collection: str, doc_id: str, pages: str) -> list:
fix: address xhigh code-review findings on 2d46d68..8f536cb Verified 12 findings from an xhigh-effort review of the prior review-fix batch; all confirmed real. Most trace back to one root cause: the build_index() text-stripping fix (8f536cb) correctly stopped Markdown from leaking full text by default, but broke every path that assumed text could be re-read later. Correctness: - LocalBackend._fill_node_text (get_document(include_text=True)) only handled PDF's start_index/end_index convention; Markdown nodes use line_num and got silently empty text. Now handles both. - get_page_content's Markdown fallback (triggered when a StorageEngine legitimately returns None from get_pages()) read from the now-text-stripped structure. It now re-derives from the source file, mirroring the PDF fallback, so it no longer depends on structure text at all. - add_document's PDF-only text-stripping branch (with the stale "markdown needs text in structure for fallback retrieval" comment) is now dead/wrong since build_index() already applies if_add_node_text uniformly — removed. - _validate_llm_provider's keyless-provider allowlist was missing several local LiteLLM providers (xinference, llamafile, triton, oobabooga, openai_like, docker_model_runner, custom, custom_openai, petals) that need no API key just like ollama/lm_studio; expanded. - The three agent-tool closures (get_document, get_document_structure, get_page_content) had three different not-found patterns; two bypassed the backend's DocumentNotFoundError entirely. Extracted LocalBackend. _require_document as the single existence check every method/tool now uses. - examples/agentic_vectorless_rag_demo.py's hand-rolled Agent() didn't apply the litellm/ prefix normalization the SDK does internally, so its own documented "any LiteLLM provider" claim broke for non-openai models. - cloud delete_collection's cache eviction removed the "folders unavailable" None sentinel too, forcing a wasted re-fetch; now only pops on a real id. Cleanup / altitude: - build_index() skips the remove_structure_text walk entirely when text was never added (content_based + if_add_node_summary=False + if_add_node_text= False) instead of a guaranteed no-op tree walk. - page_index()'s locals()-capture-as-kwargs (fragile by construction) replaced with an explicit dict of the named parameters. - run_pageindex.py's _cli_bool and the page_index_md.py legacy shim's _coerce_bool were duplicate, diverging implementations; both now bind directly to the canonical pageindex.index.page_index_md._coerce_bool. - retrieve.py's _get_md_page_content delegated its own traversal instead of calling the canonical get_md_page_content; now a one-line delegation. - FileTypeError's docstring now calls out the except-ordering gotcha from also subclassing ValueError. 17 new regression tests (tests/test_review_fixes_2.py) plus 2 updated in tests/test_legacy_shims.py for the simplified md_to_tree shim. Full suite: 210 passed, 2 skipped. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
2026-07-08 21:56:41 +08:00
doc = self._require_document(collection, doc_id)
page_nums = parse_pages(pages)
# Try cached pages first (fast, no file I/O)
cached_pages = self._storage.get_pages(collection, doc_id)
if cached_pages:
return [p for p in cached_pages if p["page"] in page_nums]
fix: address xhigh code-review findings on 2d46d68..8f536cb Verified 12 findings from an xhigh-effort review of the prior review-fix batch; all confirmed real. Most trace back to one root cause: the build_index() text-stripping fix (8f536cb) correctly stopped Markdown from leaking full text by default, but broke every path that assumed text could be re-read later. Correctness: - LocalBackend._fill_node_text (get_document(include_text=True)) only handled PDF's start_index/end_index convention; Markdown nodes use line_num and got silently empty text. Now handles both. - get_page_content's Markdown fallback (triggered when a StorageEngine legitimately returns None from get_pages()) read from the now-text-stripped structure. It now re-derives from the source file, mirroring the PDF fallback, so it no longer depends on structure text at all. - add_document's PDF-only text-stripping branch (with the stale "markdown needs text in structure for fallback retrieval" comment) is now dead/wrong since build_index() already applies if_add_node_text uniformly — removed. - _validate_llm_provider's keyless-provider allowlist was missing several local LiteLLM providers (xinference, llamafile, triton, oobabooga, openai_like, docker_model_runner, custom, custom_openai, petals) that need no API key just like ollama/lm_studio; expanded. - The three agent-tool closures (get_document, get_document_structure, get_page_content) had three different not-found patterns; two bypassed the backend's DocumentNotFoundError entirely. Extracted LocalBackend. _require_document as the single existence check every method/tool now uses. - examples/agentic_vectorless_rag_demo.py's hand-rolled Agent() didn't apply the litellm/ prefix normalization the SDK does internally, so its own documented "any LiteLLM provider" claim broke for non-openai models. - cloud delete_collection's cache eviction removed the "folders unavailable" None sentinel too, forcing a wasted re-fetch; now only pops on a real id. Cleanup / altitude: - build_index() skips the remove_structure_text walk entirely when text was never added (content_based + if_add_node_summary=False + if_add_node_text= False) instead of a guaranteed no-op tree walk. - page_index()'s locals()-capture-as-kwargs (fragile by construction) replaced with an explicit dict of the named parameters. - run_pageindex.py's _cli_bool and the page_index_md.py legacy shim's _coerce_bool were duplicate, diverging implementations; both now bind directly to the canonical pageindex.index.page_index_md._coerce_bool. - retrieve.py's _get_md_page_content delegated its own traversal instead of calling the canonical get_md_page_content; now a one-line delegation. - FileTypeError's docstring now calls out the except-ordering gotcha from also subclassing ValueError. 17 new regression tests (tests/test_review_fixes_2.py) plus 2 updated in tests/test_legacy_shims.py for the simplified md_to_tree shim. Full suite: 210 passed, 2 skipped. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
2026-07-08 21:56:41 +08:00
# Fallback: re-derive from the source file, same as the PDF path below
# — never from the stored structure, whose 'text' field may have been
# stripped (if_add_node_text=False, the default). Reachable only for a
# custom StorageEngine that doesn't cache pages (the built-in
# SQLiteStorage always does).
if doc["doc_type"] == "pdf":
return get_pdf_page_content(doc["file_path"], page_nums)
else:
fix: address xhigh code-review findings on 2d46d68..8f536cb Verified 12 findings from an xhigh-effort review of the prior review-fix batch; all confirmed real. Most trace back to one root cause: the build_index() text-stripping fix (8f536cb) correctly stopped Markdown from leaking full text by default, but broke every path that assumed text could be re-read later. Correctness: - LocalBackend._fill_node_text (get_document(include_text=True)) only handled PDF's start_index/end_index convention; Markdown nodes use line_num and got silently empty text. Now handles both. - get_page_content's Markdown fallback (triggered when a StorageEngine legitimately returns None from get_pages()) read from the now-text-stripped structure. It now re-derives from the source file, mirroring the PDF fallback, so it no longer depends on structure text at all. - add_document's PDF-only text-stripping branch (with the stale "markdown needs text in structure for fallback retrieval" comment) is now dead/wrong since build_index() already applies if_add_node_text uniformly — removed. - _validate_llm_provider's keyless-provider allowlist was missing several local LiteLLM providers (xinference, llamafile, triton, oobabooga, openai_like, docker_model_runner, custom, custom_openai, petals) that need no API key just like ollama/lm_studio; expanded. - The three agent-tool closures (get_document, get_document_structure, get_page_content) had three different not-found patterns; two bypassed the backend's DocumentNotFoundError entirely. Extracted LocalBackend. _require_document as the single existence check every method/tool now uses. - examples/agentic_vectorless_rag_demo.py's hand-rolled Agent() didn't apply the litellm/ prefix normalization the SDK does internally, so its own documented "any LiteLLM provider" claim broke for non-openai models. - cloud delete_collection's cache eviction removed the "folders unavailable" None sentinel too, forcing a wasted re-fetch; now only pops on a real id. Cleanup / altitude: - build_index() skips the remove_structure_text walk entirely when text was never added (content_based + if_add_node_summary=False + if_add_node_text= False) instead of a guaranteed no-op tree walk. - page_index()'s locals()-capture-as-kwargs (fragile by construction) replaced with an explicit dict of the named parameters. - run_pageindex.py's _cli_bool and the page_index_md.py legacy shim's _coerce_bool were duplicate, diverging implementations; both now bind directly to the canonical pageindex.index.page_index_md._coerce_bool. - retrieve.py's _get_md_page_content delegated its own traversal instead of calling the canonical get_md_page_content; now a one-line delegation. - FileTypeError's docstring now calls out the except-ordering gotcha from also subclassing ValueError. 17 new regression tests (tests/test_review_fixes_2.py) plus 2 updated in tests/test_legacy_shims.py for the simplified md_to_tree shim. Full suite: 210 passed, 2 skipped. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
2026-07-08 21:56:41 +08:00
parser = self._resolve_parser(doc["file_path"])
parsed = parser.parse(doc["file_path"], model=self._model)
page_map = {n.index: n.content for n in parsed.nodes}
return [{"page": p, "content": page_map[p]} for p in page_nums if p in page_map]
def list_documents(self, collection: str) -> list[dict]:
return self._storage.list_documents(collection)
def delete_document(self, collection: str, doc_id: str) -> None:
fix: address xhigh code-review findings on 2d46d68..8f536cb Verified 12 findings from an xhigh-effort review of the prior review-fix batch; all confirmed real. Most trace back to one root cause: the build_index() text-stripping fix (8f536cb) correctly stopped Markdown from leaking full text by default, but broke every path that assumed text could be re-read later. Correctness: - LocalBackend._fill_node_text (get_document(include_text=True)) only handled PDF's start_index/end_index convention; Markdown nodes use line_num and got silently empty text. Now handles both. - get_page_content's Markdown fallback (triggered when a StorageEngine legitimately returns None from get_pages()) read from the now-text-stripped structure. It now re-derives from the source file, mirroring the PDF fallback, so it no longer depends on structure text at all. - add_document's PDF-only text-stripping branch (with the stale "markdown needs text in structure for fallback retrieval" comment) is now dead/wrong since build_index() already applies if_add_node_text uniformly — removed. - _validate_llm_provider's keyless-provider allowlist was missing several local LiteLLM providers (xinference, llamafile, triton, oobabooga, openai_like, docker_model_runner, custom, custom_openai, petals) that need no API key just like ollama/lm_studio; expanded. - The three agent-tool closures (get_document, get_document_structure, get_page_content) had three different not-found patterns; two bypassed the backend's DocumentNotFoundError entirely. Extracted LocalBackend. _require_document as the single existence check every method/tool now uses. - examples/agentic_vectorless_rag_demo.py's hand-rolled Agent() didn't apply the litellm/ prefix normalization the SDK does internally, so its own documented "any LiteLLM provider" claim broke for non-openai models. - cloud delete_collection's cache eviction removed the "folders unavailable" None sentinel too, forcing a wasted re-fetch; now only pops on a real id. Cleanup / altitude: - build_index() skips the remove_structure_text walk entirely when text was never added (content_based + if_add_node_summary=False + if_add_node_text= False) instead of a guaranteed no-op tree walk. - page_index()'s locals()-capture-as-kwargs (fragile by construction) replaced with an explicit dict of the named parameters. - run_pageindex.py's _cli_bool and the page_index_md.py legacy shim's _coerce_bool were duplicate, diverging implementations; both now bind directly to the canonical pageindex.index.page_index_md._coerce_bool. - retrieve.py's _get_md_page_content delegated its own traversal instead of calling the canonical get_md_page_content; now a one-line delegation. - FileTypeError's docstring now calls out the except-ordering gotcha from also subclassing ValueError. 17 new regression tests (tests/test_review_fixes_2.py) plus 2 updated in tests/test_legacy_shims.py for the simplified md_to_tree shim. Full suite: 210 passed, 2 skipped. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
2026-07-08 21:56:41 +08:00
doc = self._require_document(collection, doc_id)
fix(cloud): align cloud/local backend contracts and harden the HTTP layer Fixes the cloud/local contract mismatches from the PR #272 review (verified against the official API docs — the cloud API has no folder/collection endpoints publicly, GET /docs supports limit<=100 with offset): - query_stream: emit a terminal answer_done event with the full answer (same contract as the local backend); raise CloudAPIError instead of disguising HTTP errors as answer events; move the initial connect inside try so a connection failure can no longer strand the consumer awaiting a sentinel that never arrives - _request: rewind file objects before retrying so a transient 5xx/429 during upload no longer re-sends an empty multipart body; carry the HTTP status on CloudAPIError (status_code) and keep the last status in the max-retries error - list_documents: paginate with limit/offset instead of a hard-coded limit=100, so >100-doc collections are no longer silently truncated (whole-collection queries rely on this list) - folders: treat only 403/404 as "folders unavailable" (warned via warnings.warn instead of an invisible logger.warning, matched on status_code instead of a "403" substring); transient errors now propagate instead of being permanently cached as folder_id=None - error taxonomy parity: cloud doc endpoints map HTTP 404 to DocumentNotFoundError; local get_document raises DocumentNotFoundError instead of returning {}; local delete_document raises on missing doc_id instead of silently deleting nothing - cloud get_document warns that include_text is not supported instead of silently ignoring it Adds regression tests for each fix (11 new tests). Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
2026-07-07 09:40:09 +08:00
if doc.get("file_path"):
Path(doc["file_path"]).unlink(missing_ok=True)
# Clean up images directory: files/{collection}/{doc_id}/
doc_dir = self._files_dir / collection / doc_id
if doc_dir.exists():
shutil.rmtree(doc_dir)
self._storage.delete_document(collection, doc_id)
def get_agent_tools(self, collection: str, doc_ids: list[str] | None = None) -> AgentTools:
"""Build agent tools.
- doc_ids=None (open mode): includes ``list_documents``; agent picks docs itself.
- doc_ids=[...] (scoped mode): no ``list_documents``; the other tools
hard-enforce the whitelist and reject out-of-scope doc_ids.
"""
from agents import function_tool
import json
storage = self._storage
col_name = collection
backend = self
scope = set(doc_ids) if doc_ids else None
def _reject(doc_id: str) -> str | None:
if scope is not None and doc_id not in scope:
return json.dumps({
"error": f"doc_id '{doc_id}' is not in scope.",
"allowed_doc_ids": sorted(scope),
})
return None
@function_tool
def get_document(doc_id: str) -> str:
"""Get document metadata."""
rejection = _reject(doc_id)
if rejection:
return rejection
fix: address xhigh code-review findings on 2d46d68..8f536cb Verified 12 findings from an xhigh-effort review of the prior review-fix batch; all confirmed real. Most trace back to one root cause: the build_index() text-stripping fix (8f536cb) correctly stopped Markdown from leaking full text by default, but broke every path that assumed text could be re-read later. Correctness: - LocalBackend._fill_node_text (get_document(include_text=True)) only handled PDF's start_index/end_index convention; Markdown nodes use line_num and got silently empty text. Now handles both. - get_page_content's Markdown fallback (triggered when a StorageEngine legitimately returns None from get_pages()) read from the now-text-stripped structure. It now re-derives from the source file, mirroring the PDF fallback, so it no longer depends on structure text at all. - add_document's PDF-only text-stripping branch (with the stale "markdown needs text in structure for fallback retrieval" comment) is now dead/wrong since build_index() already applies if_add_node_text uniformly — removed. - _validate_llm_provider's keyless-provider allowlist was missing several local LiteLLM providers (xinference, llamafile, triton, oobabooga, openai_like, docker_model_runner, custom, custom_openai, petals) that need no API key just like ollama/lm_studio; expanded. - The three agent-tool closures (get_document, get_document_structure, get_page_content) had three different not-found patterns; two bypassed the backend's DocumentNotFoundError entirely. Extracted LocalBackend. _require_document as the single existence check every method/tool now uses. - examples/agentic_vectorless_rag_demo.py's hand-rolled Agent() didn't apply the litellm/ prefix normalization the SDK does internally, so its own documented "any LiteLLM provider" claim broke for non-openai models. - cloud delete_collection's cache eviction removed the "folders unavailable" None sentinel too, forcing a wasted re-fetch; now only pops on a real id. Cleanup / altitude: - build_index() skips the remove_structure_text walk entirely when text was never added (content_based + if_add_node_summary=False + if_add_node_text= False) instead of a guaranteed no-op tree walk. - page_index()'s locals()-capture-as-kwargs (fragile by construction) replaced with an explicit dict of the named parameters. - run_pageindex.py's _cli_bool and the page_index_md.py legacy shim's _coerce_bool were duplicate, diverging implementations; both now bind directly to the canonical pageindex.index.page_index_md._coerce_bool. - retrieve.py's _get_md_page_content delegated its own traversal instead of calling the canonical get_md_page_content; now a one-line delegation. - FileTypeError's docstring now calls out the except-ordering gotcha from also subclassing ValueError. 17 new regression tests (tests/test_review_fixes_2.py) plus 2 updated in tests/test_legacy_shims.py for the simplified md_to_tree shim. Full suite: 210 passed, 2 skipped. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
2026-07-08 21:56:41 +08:00
try:
# _require_document (not backend.get_document) deliberately:
# the metadata-only row, no 'structure' — keeps this tool's
# output small for the agent's context window.
doc = backend._require_document(col_name, doc_id)
except DocumentNotFoundError:
fix: address PR #272 review findings (directly-fixable items) Verified against current dev; the compat/behavior decisions (#7 api_key semantics, #10 CLI flags, #11 doc-description default) are deferred. Crashes: - page_index(): snapshot args before importing IndexConfig — locals() was capturing the imported class and IndexConfig(extra='forbid') made every call raise ValidationError. - process_none_page_numbers: pop('page', None) instead of del (a TOC item without 'page' raised KeyError mid-pipeline). - pipeline._run_async: guard only the loop detection, not the run, so a real RuntimeError from the coroutine isn't masked as "asyncio.run() cannot be called from a running event loop". Silent-wrong / robustness: - LocalBackend.get_document_structure and the agent get_document / get_document_structure tools now surface a missing doc (raise / error-JSON) instead of returning empty, matching get_page_content and the cloud backend. - cloud delete_collection drops the cached folder_id. - cloud query raises on an empty collection instead of POSTing doc_id:[]. - LocalClient skips the API-key check for keyless providers (ollama, lm_studio, …) so keyless LiteLLM models aren't rejected at construction. Compat / cleanup: - md_to_tree coerces legacy 'yes'/'no' string flags (a bare 'no' was truthy). - FileTypeError also subclasses ValueError (0.2.x raised ValueError). - _validate_llm_provider no longer mutates global litellm.model_cost_map_url. - __all__ re-includes legacy exports (page_index, md_to_tree, get_*). - Rewrite examples/agentic_vectorless_rag_demo.py to the Collection API and use the in-repo attention.pdf (the old workspace=/client.index/client.documents API no longer exists). Adds tests/test_review_fixes.py (10 regressions). Full suite: 189 passed. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
2026-07-08 18:56:51 +08:00
return json.dumps({"error": f"doc_id '{doc_id}' not found."})
return json.dumps(doc)
@function_tool
def get_document_structure(doc_id: str) -> str:
"""Get document tree structure (without text)."""
rejection = _reject(doc_id)
if rejection:
return rejection
fix: address xhigh code-review findings on 2d46d68..8f536cb Verified 12 findings from an xhigh-effort review of the prior review-fix batch; all confirmed real. Most trace back to one root cause: the build_index() text-stripping fix (8f536cb) correctly stopped Markdown from leaking full text by default, but broke every path that assumed text could be re-read later. Correctness: - LocalBackend._fill_node_text (get_document(include_text=True)) only handled PDF's start_index/end_index convention; Markdown nodes use line_num and got silently empty text. Now handles both. - get_page_content's Markdown fallback (triggered when a StorageEngine legitimately returns None from get_pages()) read from the now-text-stripped structure. It now re-derives from the source file, mirroring the PDF fallback, so it no longer depends on structure text at all. - add_document's PDF-only text-stripping branch (with the stale "markdown needs text in structure for fallback retrieval" comment) is now dead/wrong since build_index() already applies if_add_node_text uniformly — removed. - _validate_llm_provider's keyless-provider allowlist was missing several local LiteLLM providers (xinference, llamafile, triton, oobabooga, openai_like, docker_model_runner, custom, custom_openai, petals) that need no API key just like ollama/lm_studio; expanded. - The three agent-tool closures (get_document, get_document_structure, get_page_content) had three different not-found patterns; two bypassed the backend's DocumentNotFoundError entirely. Extracted LocalBackend. _require_document as the single existence check every method/tool now uses. - examples/agentic_vectorless_rag_demo.py's hand-rolled Agent() didn't apply the litellm/ prefix normalization the SDK does internally, so its own documented "any LiteLLM provider" claim broke for non-openai models. - cloud delete_collection's cache eviction removed the "folders unavailable" None sentinel too, forcing a wasted re-fetch; now only pops on a real id. Cleanup / altitude: - build_index() skips the remove_structure_text walk entirely when text was never added (content_based + if_add_node_summary=False + if_add_node_text= False) instead of a guaranteed no-op tree walk. - page_index()'s locals()-capture-as-kwargs (fragile by construction) replaced with an explicit dict of the named parameters. - run_pageindex.py's _cli_bool and the page_index_md.py legacy shim's _coerce_bool were duplicate, diverging implementations; both now bind directly to the canonical pageindex.index.page_index_md._coerce_bool. - retrieve.py's _get_md_page_content delegated its own traversal instead of calling the canonical get_md_page_content; now a one-line delegation. - FileTypeError's docstring now calls out the except-ordering gotcha from also subclassing ValueError. 17 new regression tests (tests/test_review_fixes_2.py) plus 2 updated in tests/test_legacy_shims.py for the simplified md_to_tree shim. Full suite: 210 passed, 2 skipped. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
2026-07-08 21:56:41 +08:00
try:
backend._require_document(col_name, doc_id)
except DocumentNotFoundError:
fix: address PR #272 review findings (directly-fixable items) Verified against current dev; the compat/behavior decisions (#7 api_key semantics, #10 CLI flags, #11 doc-description default) are deferred. Crashes: - page_index(): snapshot args before importing IndexConfig — locals() was capturing the imported class and IndexConfig(extra='forbid') made every call raise ValidationError. - process_none_page_numbers: pop('page', None) instead of del (a TOC item without 'page' raised KeyError mid-pipeline). - pipeline._run_async: guard only the loop detection, not the run, so a real RuntimeError from the coroutine isn't masked as "asyncio.run() cannot be called from a running event loop". Silent-wrong / robustness: - LocalBackend.get_document_structure and the agent get_document / get_document_structure tools now surface a missing doc (raise / error-JSON) instead of returning empty, matching get_page_content and the cloud backend. - cloud delete_collection drops the cached folder_id. - cloud query raises on an empty collection instead of POSTing doc_id:[]. - LocalClient skips the API-key check for keyless providers (ollama, lm_studio, …) so keyless LiteLLM models aren't rejected at construction. Compat / cleanup: - md_to_tree coerces legacy 'yes'/'no' string flags (a bare 'no' was truthy). - FileTypeError also subclasses ValueError (0.2.x raised ValueError). - _validate_llm_provider no longer mutates global litellm.model_cost_map_url. - __all__ re-includes legacy exports (page_index, md_to_tree, get_*). - Rewrite examples/agentic_vectorless_rag_demo.py to the Collection API and use the in-repo attention.pdf (the old workspace=/client.index/client.documents API no longer exists). Adds tests/test_review_fixes.py (10 regressions). Full suite: 189 passed. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
2026-07-08 18:56:51 +08:00
return json.dumps({"error": f"doc_id '{doc_id}' not found."})
structure = storage.get_document_structure(col_name, doc_id)
return json.dumps(remove_fields(structure, fields=["text"]), ensure_ascii=False)
@function_tool
def get_page_content(doc_id: str, pages: str) -> str:
"""Get page content. Use tight ranges: '5-7', '3,8', '12'."""
rejection = _reject(doc_id)
if rejection:
return rejection
fix: address PR #272 review findings (directly-fixable items) Verified against current dev; the compat/behavior decisions (#7 api_key semantics, #10 CLI flags, #11 doc-description default) are deferred. Crashes: - page_index(): snapshot args before importing IndexConfig — locals() was capturing the imported class and IndexConfig(extra='forbid') made every call raise ValidationError. - process_none_page_numbers: pop('page', None) instead of del (a TOC item without 'page' raised KeyError mid-pipeline). - pipeline._run_async: guard only the loop detection, not the run, so a real RuntimeError from the coroutine isn't masked as "asyncio.run() cannot be called from a running event loop". Silent-wrong / robustness: - LocalBackend.get_document_structure and the agent get_document / get_document_structure tools now surface a missing doc (raise / error-JSON) instead of returning empty, matching get_page_content and the cloud backend. - cloud delete_collection drops the cached folder_id. - cloud query raises on an empty collection instead of POSTing doc_id:[]. - LocalClient skips the API-key check for keyless providers (ollama, lm_studio, …) so keyless LiteLLM models aren't rejected at construction. Compat / cleanup: - md_to_tree coerces legacy 'yes'/'no' string flags (a bare 'no' was truthy). - FileTypeError also subclasses ValueError (0.2.x raised ValueError). - _validate_llm_provider no longer mutates global litellm.model_cost_map_url. - __all__ re-includes legacy exports (page_index, md_to_tree, get_*). - Rewrite examples/agentic_vectorless_rag_demo.py to the Collection API and use the in-repo attention.pdf (the old workspace=/client.index/client.documents API no longer exists). Adds tests/test_review_fixes.py (10 regressions). Full suite: 189 passed. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
2026-07-08 18:56:51 +08:00
try:
result = backend.get_page_content(col_name, doc_id, pages)
except DocumentNotFoundError:
return json.dumps({"error": f"doc_id '{doc_id}' not found."})
except (ValueError, AttributeError) as e:
# A malformed page spec ("all", "5-") is a recoverable bad tool
# argument: hand the model an actionable error it can correct
# (mirroring the legacy retrieval tool) rather than letting the
# ValueError surface as the agent SDK's generic tool-failure text.
return json.dumps({
"error": f"Invalid pages format: {pages!r}. Use '5-7', '3,8', or '12'. Error: {e}"
})
return json.dumps(result, ensure_ascii=False)
tools = [get_document, get_document_structure, get_page_content]
if scope is None:
@function_tool
def list_documents() -> str:
"""List all documents in the collection."""
return json.dumps(storage.list_documents(col_name))
tools.insert(0, list_documents)
return AgentTools(function_tools=tools)
def _scoped_docs(self, collection: str, doc_ids: list[str]) -> list[dict]:
"""Fetch metadata for the docs in scope; raise if any are missing."""
by_id = {d["doc_id"]: d for d in self._storage.list_documents(collection)}
missing = [did for did in doc_ids if did not in by_id]
if missing:
raise DocumentNotFoundError(
f"doc_ids not found in collection '{collection}': {missing}"
)
return [by_id[did] for did in doc_ids]
@staticmethod
def _normalize_doc_ids(doc_ids: str | list[str] | None) -> list[str] | None:
if isinstance(doc_ids, str):
return [doc_ids]
if doc_ids == []:
raise ValueError(
"doc_ids cannot be empty; pass None to query the whole collection"
)
return doc_ids
def query(self, collection: str, question: str,
doc_ids: str | list[str] | None = None) -> str:
from ..agent import AgentRunner, SCOPED_SYSTEM_PROMPT, wrap_with_doc_context
doc_ids = self._normalize_doc_ids(doc_ids)
tools = self.get_agent_tools(collection, doc_ids)
instructions = None
if doc_ids:
docs = self._scoped_docs(collection, doc_ids)
question = wrap_with_doc_context(docs, question)
instructions = SCOPED_SYSTEM_PROMPT
return AgentRunner(tools=tools, model=self._retrieve_model,
instructions=instructions).run(question)
async def query_stream(self, collection: str, question: str,
doc_ids: str | list[str] | None = None):
from ..agent import QueryStream, SCOPED_SYSTEM_PROMPT, wrap_with_doc_context
doc_ids = self._normalize_doc_ids(doc_ids)
tools = self.get_agent_tools(collection, doc_ids)
instructions = None
if doc_ids:
docs = self._scoped_docs(collection, doc_ids)
question = wrap_with_doc_context(docs, question)
instructions = SCOPED_SYSTEM_PROMPT
stream = QueryStream(tools=tools, question=question,
model=self._retrieve_model, instructions=instructions)
async for event in stream:
yield event