Repository navigation
feat!: revision-aware incremental indexing, share locations, streamable-http transport and a docs split - #41
Merged
Merged
Conversation
An explicit-path load_config() now becomes the process-wide active configuration that cfgload.get() answers from, so `--config X` no longer silently takes timeouts, batching, parallelism and the .doc scratch location from an auto-discovered file. The CLI activates the given path before importing the index and search modules, and every duplicated per-call-site fallback default (including the 8-vs-4 parallel_workers disagreement) goes away. Schema changes: - indexing.timeout.* flattened to indexing.per_file_seconds (0 disables) and indexing.scan_item_seconds; the heartbeat and .doc conversion budgets are internal now - indexing.enable_adaptive_batching / max_memory_mb folded into batch_size (an upper bound over the adaptive sizer); enable_parallel_loading folded into parallel_workers (1 = serial) - indexing.logging.slow_file_threshold_seconds flattened to indexing.slow_file_threshold_seconds; log_slow_files was never read - retrieval.bm25_similarity_top_k removed (the BM25 top-k was hardcoded) - include/exclude pattern lists defined once, in cfgload - an empty YAML value (chunk_size:) keeps the default instead of loading None, and the returned config never aliases the defaults - a config file that still sets a removed option gets a warning naming its replacement BREAKING CHANGE: the indexing.timeout, indexing.logging, enable_parallel_loading, enable_adaptive_batching, max_memory_mb and retrieval.bm25_similarity_top_k options are gone; the loader warns about each one it finds and names the replacement. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Yp9BfFC7MRYZ4WCKSobE6
…models through the CLI The manylinux bundle installed as the previous version: prepare-common.sh copies pyproject.toml and src/ straight from the checkout, where main still carries the last release's number (the bump lands only after a release finishes), so the tarball name and VERSION file said one thing while pip recorded another. The script now stamps VERSION into the copied sources and verifies it, so a mismatch fails the build instead of shipping. The release scripts and CI also invoked the module-level `python -m chunksilo.index` entry point, a second CLI that duplicated `chunksilo --download-models` with its own --model-cache-dir flag. That entry point and the build_index(model_cache_dir=...) override are gone; the scripts download models through the real CLI with a throwaway config naming the cache location, which the active-config loader honours end to end. setup.sh points at `chunksilo --build-index` as well. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Yp9BfFC7MRYZ4WCKSobE6
…adings by the original path Legacy .doc conversion wrote LibreOffice's output into one shared scratch directory: two parallel workers converting files with the same stem raced for one output path, so one file's content could be indexed under the other file's path. LibreOffice was also handed the original document, which let it drop a .~lock.*# file next to a source inside an indexed directory, and it used the shared profile under $HOME. Each conversion now runs in its own directory under storage on a copy of the source, with a private LibreOffice profile, and the whole directory is removed afterwards. Headings extracted from a converted .doc are stored under the original .doc path, which is where the query side looks them up; previously they were keyed by the temporary .docx path and never found. A new regression test builds an index twice over a populated tree (txt/md/docx/legacy doc, subdirectories with spaces) from a neutral working directory and asserts the tree is byte-for-byte unchanged, with no logs or lock files added. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Yp9BfFC7MRYZ4WCKSobE6
… could not verify Untouched files were re-indexed on every run, by three separate routes. A file that extracted no text never got a state row - load_files_parallel dropped it and build_index only recorded non-empty results - while its old chunks had already been deleted, so it looked new for ever; an image-only .docx hit this every time. A directory whose availability check was merely slow was dropped from the source list, and every file under it then looked deleted, wiping the tree from the index and forcing a full rebuild next run. A single file that could not be stat'd or hashed was pruned the same way, and once pruned it lost the mtime fast path, so the next scan had to hash it again - self-sustaining. Now every attempted file gets an outcome: an empty load is a final answer and is recorded with no doc ids; a failure is not recorded and is retried. Each source tracks the health of its scan, deletions are withheld for anything a scan could not see (with the count reported and the reason logged), and each reprocessed file logs why. The scan now runs in two phases - a stat-only enumeration, then hashing - with reason-returning variants of the include/exclude matchers and an optional per-event callback, so diagnostics can reuse the exact scan the indexer performs. This also fixes a pre-existing race: directory pruning ran on the consumer side of the walk while the os.walk producer thread walked ahead, so excluded directories were often descended into anyway. Pruning now happens inside the walker thread before descent. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Yp9BfFC7MRYZ4WCKSobE6
Document trees often hold many ad-hoc revisions of one document (revision-named directories like .../PB3/, filename tokens like 'Spec PB3.docx' or 'Spec_v2.pdf') plus copies made to collect review comments, and every one of them was indexed. - A new revisions module groups files that are revisions of one document: by a configurable doc-ID regex first, else by revision-stripped filename and path. Only the newest member (by file mtime - labels are never ordered) is indexed; review copies are never indexed. Skipped files are pruned by the existing deletion loop and resurface automatically when they become newest again. No state-schema change. - The two-phase scan hashes only the keepers, so skipped files cost no MD5 work. - indexing.versioning.index_superseded keeps old revisions indexed, stamped superseded so the query side can demote them. - Chunks and BM25 file-name nodes carry doc_group / doc_id / superseded metadata for the query side. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Yp9BfFC7MRYZ4WCKSobE6
chunksilo --check-files [PATH] runs the indexer's exact phase-1 scan (availability, directory pruning, include/exclude globs, revision grouping, review copies) and reports a per-file verdict with the deciding reason - read-only: no models, no hashing, no state-DB writes. With PATH it explains one file, including its revision group. Exit codes make it scriptable; --json emits the full report. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Yp9BfFC7MRYZ4WCKSobE6
…before anything else mcp.log was written to the current working directory, which for a service started from the wrong place can be inside an indexed tree, and rotated by renaming into mcp_<timestamp>_<pid>.log files that accumulated forever. It now lives in storage.storage_dir, is created mode 0600, and rotates with a bounded backup count. Because the log location comes from the configuration, run_server now loads - and thereby activates - the configuration first, then configures logging, then imports the search module. The CHUNKSILO_CONFIG environment workaround that used to smuggle --config into the modules that read the config at import time is gone. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Yp9BfFC7MRYZ4WCKSobE6
…esystem _resolve_file_uri called Path.exists() and Path.resolve() on every result path - for each BM25 candidate and for each formatted chunk, so 10-15 paths per query. resolve() readlinks every path component, and on a network mount that is a round trip per component, serialized through one userspace daemon: a single query against a few dozen documents could take the better part of a minute. It was also wrong twice over: resolve() rewrote a path the user recognises into the mount's internal form, which no application will open, and the URI was never percent-encoded, so any path containing a space produced a broken URI. The URI is now built from the stored path alone (a relative path is joined to the first configured directory) with quote(). Result URIs are consequently percent-encoded where they previously were not. The URI tests now exercise the real builder and pin the contract: no stat, readlink or resolve during URI building. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Yp9BfFC7MRYZ4WCKSobE6
Search results carried file:// URIs built from the indexer's own mount paths, which a client on another machine can neither open nor act on. A new `shares` configuration lists where each mounted tree really lives (local prefix -> //server/share), and results for covered files now name that location: an smb:// URI plus the Windows UNC path, in a new `unc` field next to `uri`. The mapping is derived purely from configuration with component-anchored, post-normpath matching (longest prefix wins, and `..` cannot cross into another share), and like the rest of result formatting it never touches the filesystem on the query path. Files not covered by any mapping - including everything in a local single-machine setup, which typically configures none - keep their file:// URIs and a null `unc`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Yp9BfFC7MRYZ4WCKSobE6
…ings The four lazy loaders (index, embedding model, reranker, BM25) were unguarded module globals. A server that dispatches tool calls into a thread pool with no session affinity lets N concurrent first calls each cold-start their own copy - N times the work and memory, N-1 discarded - and every one of them waits for the slowest. They are now double-checked-locked, the reranker is published only after it loaded successfully (a failed load is retried, not cached), and a new search.warm_up() lets a server pay the cost once at startup instead of inside a user's query. Per-stage timings (load, vector, bm25, remote, filter, rerank, format) are logged for every search, because "the query took 56 seconds" is not actionable on its own. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Yp9BfFC7MRYZ4WCKSobE6
run_search returned the raw exception text in its error field. That text can name filesystem paths and storage locations, which is not the caller's business - an MCP client gets a generic message and the operator gets the detail, with traceback, in the server log. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Yp9BfFC7MRYZ4WCKSobE6
Search results were dominated by one document's chunks (or one document's many revisions): the reranker truncated to rerank_top_k and handed back whatever ranked highest, which for a long manual is five chunks of the same file. Results are now capped per document (retrieval.max_chunks_per_doc, default 2, 0 disables) with rank-order backfill, so the top-k spans documents instead of one document's every chunk; the reranker returns the full ranked list so the diversifier has candidates to backfill from. Chunks stamped superseded by the revision-aware indexer are demoted below current documents at both the candidate cap and the final cut, and BM25 file-name matches are deduped per document group with superseded revisions dropped. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Yp9BfFC7MRYZ4WCKSobE6
chunksilo-mcp can now serve MCP over streamable-http as well as stdio: server.transport selects it (or --transport overrides the config), and server.host / server.port give the bind address, loopback by default. On the HTTP path the server warms up the index, the models and the BM25 index before accepting the first request, so concurrent first calls do not all wait on the slowest cold start. The transport performs no authentication of its own. That is deliberate: login, TLS termination and access policy belong to a reverse proxy in front of the server, which is why the default bind is 127.0.0.1 and the log line at startup says so. The stdio server is unchanged apart from its MCP name, which is now "chunksilo". Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Yp9BfFC7MRYZ4WCKSobE6
…ch and server behaviour The README grew into a 400-line manual. It is now a short overview - pitch, installation, a config sketch, the search_docs result shape and a documentation index - with the reference material in docs/: the full configuration reference (including the flattened indexing options, revision handling, share locations, the HTTP transport and an upgrade table for 2.x configs), CLI usage with --check-files, the tool result fields, MCP client setup for Claude Code, Claude Desktop, Cline, Roo Code and opencode over stdio or HTTP, and troubleshooting notes for the scan-health, revision and re-indexing behaviour. AGENTS.md records the invariants the code now relies on: the config activation contract, indexing never writing inside an indexed directory, withheld deletions, one state row per attempted file, the dry-run scan reusing the indexer's own scan, filesystem-free result formatting, the locked lazy loaders, and the HTTP transport authenticating nobody. The release bundle now ships docs/, and the test docs no longer point at a test/requirements.txt that does not exist. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Yp9BfFC7MRYZ4WCKSobE6
… proxy Behind a reverse proxy the MCP transport rejected every request: the SDK enables DNS-rebinding protection for a loopback bind and then accepts only loopback Host headers (421 otherwise, 403 for a foreign Origin), while a proxy forwards the public hostname. - server.public_url names the URL the proxy serves the instance on; its host and origin are added to the transport's allow-list, and server.allowed_hosts / allowed_origins take any further names. Without them the SDK's loopback-only default is kept. - server.authorization_servers publishes RFC 9728 protected-resource metadata at /.well-known/oauth-protected-resource (and the path-suffixed form) naming the authorization server MCP clients should obtain a token from. ChunkSilo only publishes the document; verifying tokens remains the proxy's job. - GET /health answers once the server is warmed up and listening. - A forwarded X-Forwarded-User / X-Forwarded-Email is logged with each search, for an audit trail per instance; it is never used for a decision. docs/reverse-proxy.md walks through one instance per body of documents behind oauth2-proxy, with membership of a KeyCloak group deciding who may search it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Yp9BfFC7MRYZ4WCKSobE6
server.py imports mcp.server.transport_security, which the SDK added in 1.10.0, so chunksilo-mcp failed at import - even over stdio - in an environment that updated the wheel but kept an older mcp: the previous floor of 1.0.0 let pip leave it in place. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Yp9BfFC7MRYZ4WCKSobE6
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9809125706
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+2025
to
+2032
| for file_path, outcome in file_docs.items(): | ||
| if outcome.failed: | ||
| failed_files.append(file_path) | ||
| continue | ||
| doc_ids = file_doc_ids.get(file_path, []) | ||
| if not doc_ids: | ||
| empty_files.append(file_path) | ||
| state_updates.append((outcome.file_info, doc_ids)) |
There was a problem hiding this comment.
When a loader reports a timeout or operational failure by returning []—as load_file does for a disappeared file, DOCX timeout, failed LibreOffice conversion, and generic reader timeout—outcome.failed remains false, so this block stores an empty state row. Subsequent builds then consider the file unchanged and never retry it, even after the transient problem is fixed or LibreOffice is installed; these failure paths must produce a failed outcome rather than being persisted as successfully empty.
AGENTS.md reference: AGENTS.md:L40-L40
Useful? React with 👍 / 👎.
| try: | ||
| for entry in os.walk(self.base_dir): | ||
| q.put(entry) | ||
| for root, dirs, files in os.walk(self.base_dir): |
There was a problem hiding this comment.
When a recursive scan encounters an unreadable or disconnected descendant directory, os.walk ignores the OSError by default because no onerror callback is supplied, then reaches the sentinel with scan_status.complete still true. Previously indexed files in that unseen subtree consequently enter deleted_files and have their chunks and state removed; pass an error callback that marks the scan incomplete so deletions under this root are withheld.
AGENTS.md reference: AGENTS.md:L39-L39
Useful? React with 👍 / 👎.
Comment on lines
+170
to
+174
| directories = _get_configured_directories(config) | ||
| base = str(directories[0]) if directories else "" | ||
| absolute = os.path.join(base, raw) if base else raw | ||
|
|
||
| return os.path.normpath(absolute) |
There was a problem hiding this comment.
For an older index containing relative metadata and the default relative directory ./data, this join normalizes to data/docs/file.pdf, so _file_uri emits file://data/docs/file.pdf; URI parsers interpret data as the host rather than as a local path, and absolute shares prefixes cannot match it. Convert the configured-directory join to an absolute path using string/CWD operations before formatting, without probing or resolving the filesystem.
AGENTS.md reference: AGENTS.md:L44-L44
Useful? React with 👍 / 👎.
…n could not list Two ways the incremental indexer could quietly lose content: - A loader that hit a timeout, a failed .doc conversion or a file that vanished returned an empty list, which build_index persisted as "holds no text". The file was then never tried again - a .doc indexed before LibreOffice was installed stayed unindexed for ever. Those paths now raise (FileProcessingTimeoutError / FileLoadError), so the file gets no state row and is retried next run, as the LoadOutcome contract says. - os.walk skips a directory it cannot list and carries on, so a subdirectory that lost permissions or a stale mount finished the walk "cleanly" and everything indexed under it was pruned, then re-indexed once readable again. The walk now records such directories on ScanStatus.unreadable_dirs; unscanned_roots() reports them, so deletions under them are withheld like under a stalled root, and --check-files lists them. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Yp9BfFC7MRYZ4WCKSobE6
Metadata written by old index versions can hold a relative path. Joined to a relative configured directory such as ./data it stayed relative, and file://data/x.pdf reads "data" as a host. os.path.abspath is string arithmetic against the CWD, so the no-filesystem rule still holds. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Yp9BfFC7MRYZ4WCKSobE6
Owner
Author
|
Addressed the three automated review findings in the two commits above: loader timeouts, failed .doc conversions and vanished files now raise so the file gets no state row and is retried; os.walk gets an onerror callback so files under a subdirectory the scan could not list are withheld from deletion (and reported by --check-files); legacy relative result paths are absolutised before the file:// URI is built. Full suite: 506 passed. |
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
1 participant
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.
Highlights
.docconversion happens in a private work directory so indexing never writes inside an indexed tree (test/test_never_mutates.pyenforces this).chunksilo --check-filesexplains why a file is or is not indexed.smb://URI plus UNC path, newshares:config); results are spread across documents (retrieval.max_chunks_per_doc); models load once under concurrency; per-stage timings are logged; internal error details are hidden from callers.storage.storage_dir; new unauthenticatedstreamable-httptransport (loopback by default) withpublic_url/allowed_hosts/authorization_serversfor running behind an authenticating reverse proxy;GET /health.docs/(configuration, CLI, tools, MCP clients, reverse proxy, troubleshooting).Breaking changes
indexing.timeout.*,logging.*,retrieval.bm25_similarity_top_k, and others) now log a warning naming their replacement instead of being silently ignored. Seedocs/configuration.md#upgrading-from-2x.mcp>=1.10.0is now required (the transport-security settings the server uses arrived there).Release as a major bump.
Verification
ruff check src/clean,test/run_tests.sh: 501 passed, 2 skipped.🤖 Generated with Claude Code
https://claude.ai/code/session_011Yp9BfFC7MRYZ4WCKSobE6