From 1617f69830a21761370c3ce67b46921fcae726c3 Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Fri, 4 Sep 2026 15:49:11 +0900 Subject: [PATCH 1/8] =?UTF-8?q?feat(index):=20incremental=20reindexing=20?= =?UTF-8?q?=E2=80=94=20reuse=20unchanged=20files'=20chunks,=20vectors,=20a?= =?UTF-8?q?nd=20BM25=20postings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Port upstream semble #225 (partial reindexing) to the Rust core. When the cached index's whole-tree content hash is stale, `load_or_build_index` now seeds the rebuild with the previous index instead of rebuilding from scratch: files whose per-file content hash is unchanged keep their chunks, vector rows, and BM25 postings; only changed files are re-chunked and re-embedded, and deleted files' postings are dropped. - `indexing/types.rs`: `FileManifestEntry {hash, start, count}`, `PreviousIndex::try_new` (alignment checks), `make_chunk_id`. - `sparse.rs`: `Bm25Index` becomes the id-keyed incremental index from upstream `bm25.py` (`add_document` / `remove_document` / `set_doc_order`); `bm25.json` v2 persists `{documents, docOrder}`. - `create.rs`: `create_index_from_path(.., previous)` reuse path; rows are moved (not copied) and reused rows are not re-normalised. - `cache_orchestrator.rs`: `load_previous_for_incremental` (fails closed on any structural inconsistency) + shared `manifest_compatible`. - `index.rs`: `files` manifest in `IndexManifest`/`CspIndex`, `from_path_with_previous`, `INDEX_SCHEMA_VERSION` 1 → 2, and `load_from_disk` rejects component count mismatches. - ADR-0005 records the per-file content hash (vs upstream `mtime_ns`) decision; `semble.md` and both READMEs updated. Refs #84 --- ...-file-hash-manifest-incremental-reindex.md | 75 +++ .please/docs/decisions/index.md | 1 + .please/docs/references/semble.md | 64 ++- README.ko.md | 2 +- README.md | 2 +- crates/csp/src/indexing/cache.rs | 8 + crates/csp/src/indexing/cache_orchestrator.rs | 88 +++- .../src/indexing/cache_orchestrator/tests.rs | 166 ++++++- crates/csp/src/indexing/create.rs | 303 ++++++++++-- crates/csp/src/indexing/dense/backend.rs | 23 + crates/csp/src/indexing/index.rs | 120 +++-- crates/csp/src/indexing/index/tests.rs | 36 +- crates/csp/src/indexing/mod.rs | 1 + crates/csp/src/indexing/sparse.rs | 452 +++++++++++++----- crates/csp/src/indexing/types.rs | 104 ++++ crates/csp/src/mcp.rs | 2 + 16 files changed, 1219 insertions(+), 228 deletions(-) create mode 100644 .please/docs/decisions/0005-per-file-hash-manifest-incremental-reindex.md create mode 100644 crates/csp/src/indexing/types.rs diff --git a/.please/docs/decisions/0005-per-file-hash-manifest-incremental-reindex.md b/.please/docs/decisions/0005-per-file-hash-manifest-incremental-reindex.md new file mode 100644 index 0000000..3933e66 --- /dev/null +++ b/.please/docs/decisions/0005-per-file-hash-manifest-incremental-reindex.md @@ -0,0 +1,75 @@ +# ADR 0005 — Incremental reindexing keyed on a per-file content hash (not `mtime_ns`) + +- **Status**: Accepted +- **Date**: 2026-09-04 +- **Deciders**: csp maintainers +- **Context**: [Issue #84](https://github.com/pleaseai/code-search/issues/84) — parity with upstream semble [#225](https://github.com/MinishLab/semble/pull/225) (partial / incremental reindexing) +- **Builds on**: [ADR 0002](0002-index-storage-cache-model.md) (global `~/.csp/index/` content-hash cache) + +## Context + +Upstream semble #225 added **partial reindexing**: when a cached index is stale, only the files +that changed since it was built are re-chunked and re-embedded; unchanged files keep their +chunks, their rows in the vector matrix, and their BM25 postings. Upstream detects change per +file with `mtime_ns` recorded in a `files` manifest (`{mtime_ns, start, count}`), rebuilt the +sparse side around an own incremental `BM25` class (`add_document` / `remove_document` / +`set_doc_order`, JSON persistence of per-document term counts + document order), and +validates the cached artifacts' alignment in `load_previous_for_incremental` before reusing them. + +Before this decision csp rebuilt the **whole** index whenever the whole-tree content hash in the +manifest mismatched the live tree (ADR 0002). csp never used mtimes: the cache-validity oracle +is a sha256 over `(path, bytes)` of every indexable file, chosen in ADR 0002 because mtimes are +unreliable across `git checkout`, copies, CI caches, and container mounts. + +## Decision + +Port the incremental machinery from #225, but key the per-file manifest on a **per-file content +hash** instead of `mtime_ns`: + +- `IndexManifest.files: {indexed_path → {hash, start, count}}` where `hash` is the sha256 (hex) + of the file bytes at index time (`indexing::cache::sha256_hex`). `start`/`count` are the + file's chunk range in the global chunk list, exactly as upstream. +- The whole-tree `contentHash` stays as the **fast path**: when it matches, the cache is loaded + as before with no per-file work. Only on a mismatch does `load_or_build_index` call + `load_previous_for_incremental` and seed `CspIndex::from_path_with_previous` with the + previous chunks / vectors / manifest / BM25 index. +- `create_index_from_path` reads every file once (it already had to for the whole-tree hash), + hashes it, and reuses the previous rows when the hash matches the manifest entry; otherwise it + re-chunks, re-embeds, and replaces that file's BM25 postings. Files missing from the new walk + have their postings removed. Rows are **moved** out of the previous index (no vector copies), + and reused rows are not re-normalised, so they stay bit-identical. +- `Bm25Index` becomes the id-keyed incremental index from upstream `bm25.py` with stable chunk + ids `"{indexed_path}:{slot}"` (`indexing::types::make_chunk_id`). `bm25.json` now persists + `{version: 2, documents, docOrder}`; postings are rebuilt on load, and a document order that + does not describe exactly the persisted documents is rejected. +- `INDEX_SCHEMA_VERSION` is bumped to **2**. `load_from_disk` rejects any other version (it + already did) and additionally rejects component count mismatches between chunks, vectors, and + the BM25 document order. `load_previous_for_incremental` fails closed on any structural + inconsistency (missing/empty `files`, non-tiling or overlapping ranges, chunk paths that do + not match their range, BM25 order ≠ manifest-derived ids, model/chunk-size/schema mismatch), + so a full rebuild is always the fallback. + +## Alternatives considered + +1. **`mtime_ns` like upstream** — rejected: it contradicts the ADR 0002 oracle and would make + the per-file decision disagree with the whole-tree decision (a `git checkout` that restores + identical bytes bumps mtimes and would force needless re-embedding; a same-second edit could + be missed). One oracle, one answer. +2. **Reuse the whole-tree hash only (status quo)** — rejected: any single-file edit re-embeds + the entire repository, which is the cost #225 exists to remove. +3. **Store per-file hashes only, drop the whole-tree hash** — rejected: the whole-tree hash is a + single string compare on the hit path and avoids loading chunks/vectors/BM25 at all when + nothing changed; keeping both costs one extra hex string per file in the manifest. + +## Consequences + +- A stale cache now costs roughly `O(changed files)` embedding work plus one hash pass over the + tree, instead of a full re-embed. The hit path is unchanged. +- Existing v1 caches are rebuilt once (schema bump), then benefit from incremental reuse. +- The BM25 **scoring** is intentionally unchanged from the previous csp implementation (Lucene + IDF with the `(k1+1)` numerator, de-duplicated query terms). Upstream's own class dropped the + `(k1+1)` factor and weights repeated query terms by their query frequency; both differences + scale scores without changing ranks, and ranks are all that Reciprocal Rank Fusion consumes. + Recorded as an intentional adaptation in `.please/docs/references/semble.md` §6.1. +- Git sources (`from_git`) are URL+ref keyed and never take the incremental path, matching + upstream (which only seeds `from_path`). diff --git a/.please/docs/decisions/index.md b/.please/docs/decisions/index.md index a6f846d..2431982 100644 --- a/.please/docs/decisions/index.md +++ b/.please/docs/decisions/index.md @@ -8,3 +8,4 @@ | [0002](0002-index-storage-cache-model.md) | Index Storage & Caching Model: Global `~/.csp/index/` Content-Hash Cache | 2026-06-18 | Accepted | | [0003](0003-rewrite-in-rust.md) | Rewrite `@pleaseai/csp` from TypeScript/Bun to Rust | 2026-06-18 | Proposed | | [0004](0004-rust-grammar-coverage-language-pack.md) | Rust grammar coverage via `tree-sitter-language-pack` (downloaded parsers) | 2026-06-20 | Accepted | +| [0005](0005-per-file-hash-manifest-incremental-reindex.md) | Incremental reindexing keyed on a per-file content hash (not `mtime_ns`) | 2026-09-04 | Accepted | diff --git a/.please/docs/references/semble.md b/.please/docs/references/semble.md index 632238a..829abb3 100644 --- a/.please/docs/references/semble.md +++ b/.please/docs/references/semble.md @@ -77,10 +77,11 @@ The full ranking in `ranking::{boosting,penalties}` is now **wired** into `searc | `index/file_walker.py` | `csp/src/indexing/file_walker.rs` | ported (`.cspignore`) | gitignore-aware recursive walk (`ignore` crate idioms) | | `index/files.py` | `csp/src/indexing/files.rs` | ported | ext→language map, content-type sets, file status checks | | `index/dense.py` | `csp/src/indexing/dense.rs` | ported (real + stub) | `Model` enum, `embed_chunks`, `SelectableBasicBackend` cosine | -| `index/sparse.py` | `csp/src/indexing/sparse.rs` | ported | `Bm25Index`, `enrich_for_bm25`, selector→mask | -| `index/create.py` | `csp/src/indexing/create.rs` | ported | build BM25 + dense + chunks from a path | +| `index/sparse.py` + `index/bm25.py` | `csp/src/indexing/sparse.rs` | ported (incremental) | id-keyed incremental `Bm25Index` (add/remove/doc order, `bm25.json` v2), `enrich_for_bm25`, selector→mask | +| `index/types.py` | `csp/src/indexing/types.rs` | adapted (hash, not mtime) | `FileManifestEntry {hash,start,count}`, `PreviousIndex::try_new` alignment checks, `make_chunk_id` | +| `index/create.py` | `csp/src/indexing/create.rs` | ported (incremental) | build BM25 + dense + chunks + `files` manifest from a path, reusing a `PreviousIndex`'s unchanged files | | `index/index.py` | `csp/src/indexing/index.rs` | ported | `CspIndex` orchestrator (from_path/from_git/search/find_related/save/load) + `load_or_build_index` | -| `cache.py` | `csp/src/indexing/cache.rs` | adapted | content-hash cache at `~/.csp/index/` (ADR-0002), 0700 perms | +| `cache.py` | `csp/src/indexing/cache.rs` + `cache_orchestrator.rs` | adapted | content-hash cache at `~/.csp/index/` (ADR-0002), 0700 perms; `try_reuse` + `load_previous_for_incremental` (ADR-0005) | | `search.py` | `csp/src/search.rs` | ported (ranking wired) | hybrid RRF + alpha blend; trait seams | | `ranking/weighting.py` | `csp/src/ranking/weighting.rs` | ported | adaptive alpha | | `ranking/boosting.py` | `csp/src/ranking/boosting.rs` | ported (wired) | query-type detection + definition/stem/embedded boosts | @@ -185,24 +186,43 @@ Same contract as semble `tokens.py`: ### 4.7 `indexing/sparse.rs` — BM25 + enrichment - `enrich_for_bm25(chunk)` → `"{content} {stem} {stem} {dir[-3:]}"` — stem repeated twice to - up-weight path matches; last 3 parent dir components. `Bm25Index` ports the BM25 scoring - (`get_scores(tokens, weight_mask)`). `selector_to_mask(selector, size)` → `Vec` mask. + up-weight path matches; last 3 parent dir components. `selector_to_mask(selector, size)` → + `Vec` mask. +- `Bm25Index` ports upstream's own incremental `BM25` class (`index/bm25.py`, #225): documents + are keyed on a stable chunk id (`"{path}:{slot}"`), `add_document` rejects duplicates, + `remove_document` drops a document's postings, and `set_doc_order(ids)` fixes the global + chunk-list order that `get_scores(tokens, weight_mask)` output is aligned to (ids not in the + corpus score 0). Internally ids are interned to `u32` slots (recycled on removal) and postings + are `term → {slot → tf}` so a removal is `O(terms in doc)`. `bm25.json` (v2) persists + `{documents: {id → {term → tf}}, docOrder}`; postings are rebuilt on load, and an order that + does not describe exactly the persisted documents is rejected. `build(docs)` remains as a + positional-id convenience for fixtures. ### 4.8 `indexing/create.rs` — index construction -`create_index_from_path`: walk → per file: `detect_language`, size/empty gate, read text, store -path **relative to `display_root`**, `chunk_source`. Then `embed_chunks` → dense matrix; build -`Bm25Index` over `tokenize(enrich_for_bm25(chunk))`; wrap dense in `SelectableBasicBackend`. -Empty → error. +`create_index_from_path(path, options, previous)`: walk → per file: `detect_language`, size +gate, read bytes, **sha256 the bytes**, store path **relative to `display_root`**. If `previous` +has a manifest entry for the path with the same hash, the file's chunks and (already-normalised) +vector rows are **moved** out of the previous index; otherwise the file is decoded, `chunk_source`d, +its old BM25 slots removed and new ones added (`reindex_file`), and its chunks embedded. Paths in +the previous manifest that the walk no longer yields have their postings removed. Finally +`set_doc_order(chunk_ids)` and the rows are wrapped via `SelectableBasicBackend::from_normalized`. +Returns chunks + BM25 + dense + a `files: FileManifest` (`{hash, start, count}` per file; a valid +file that yields no chunks gets `count = 0`). Empty → error. Divergence from upstream: the reuse +key is the content hash, not `mtime_ns` (ADR-0005); the "same vector layout → mutate in place" +special case is unnecessary because rows are moved, never copied. ### 4.9 `indexing/index.rs` — `CspIndex` orchestrator The public façade (parallels `SembleIndex`): -- `from_path`, `from_git` (git clone into a tempdir; repo-relative chunk paths), `search` - (`QueryOptions`), `find_related` (semantic kNN on the seed, same-language, excludes seed), - `save` / `load_from_disk` (persists chunks + bm25 + semantic + metadata). +- `from_path` (= `from_path_with_previous(path, options, None)`), `from_git` (git clone into a + tempdir; repo-relative chunk paths), `search` (`QueryOptions`), `find_related` (semantic kNN + on the seed, same-language, excludes seed), `save` / `load_from_disk` (persists chunks + bm25 + + semantic + `manifest.json` incl. the per-file `files` manifest; `INDEX_SCHEMA_VERSION = 2`; + load rejects other versions and chunk/vector/BM25-order count mismatches). - `load_or_build_index` (`LoadOrBuildOptions`) — the cache-aware entry the CLI/MCP use: load from - `~/.csp/index/` on a validated hit, else build and persist. + `~/.csp/index/` on a validated hit; on a miss for a local path, seed the rebuild with + `load_previous_for_incremental` so unchanged files are reused (ADR-0005), then persist. - Builds file→indices and language→indices maps for selectors and stats. ### 4.10 `search.rs` — hybrid retrieval & fusion @@ -277,7 +297,11 @@ Ported faithfully (`LazyLock` for the static patterns, `RefCell` `chunk_size` equals the current `DESIRED_CHUNK_LENGTH_CHARS` (a manifest predating the field → `None` → rebuild) **and**, for local sources, the live source-file content hash matches. This mirrors upstream `_metadata_matches`, which gained a `chunk_size` check so the 1500→750 - change auto-invalidates stale caches. + change auto-invalidates stale caches. The same `manifest_compatible` check (schema version, + chunk size, model id, model kind) gates `load_previous_for_incremental`, which then loads + chunks + vectors + BM25 and runs `PreviousIndex::try_new` alignment checks (counts agree, + manifest ranges tile the chunk list, chunk paths match their range, BM25 order == manifest- + derived ids). Any failure → `None` → full rebuild (mirrors upstream fail-closed behaviour). - **Divergence from upstream**: semble uses the OS cache dir (`~/Library/Caches/semble`, XDG, `%LOCALAPPDATA%`) + `SEMBLE_CACHE_LOCATION`; csp fixes a global `~/.csp/index/` per ADR-0002. @@ -359,6 +383,13 @@ Clean two-layer split: 7. **Storage** — fixed `~/.csp/index/` (0700) + `~/.csp/savings.jsonl` (ADR-0002), not the OS cache dir / `SEMBLE_CACHE_LOCATION`. `.cspignore` (not `.sembleignore`). 8. **CLI** — clap; `init` (not `install`/`uninstall`); explicit `mcp` subcommand; adds `index`. +9. **Incremental reindex keyed on per-file content hash** (ADR-0005) — upstream #225 records + `mtime_ns` per file; csp records the sha256 of the file bytes so the per-file decision uses + the same oracle as the whole-tree `contentHash` fast path (ADR-0002). +10. **BM25 scoring unchanged** — csp keeps the Lucene `(k1+1)` numerator and de-duplicates + query terms; upstream's own `BM25` (#225) dropped `(k1+1)` and multiplies by the query term + frequency. Both differ by a per-term scale factor only, so ranks — the only thing RRF + consumes — are identical. ### 6.2 Open stubs & gaps (verify before claiming runtime parity) @@ -381,6 +412,11 @@ Clean two-layer split: manifest as `chunk_size` and validated in `try_reuse`, so the change auto-invalidates stale caches (mirrors upstream's added metadata field + cache check). The TS source still uses 1500, but per the current direction Python upstream — not TS — is the source of truth. +- **Partial (incremental) reindexing (#225, `204ae4e`) — ported** ([#84](https://github.com/pleaseai/code-search/issues/84), + ADR-0005): per-file `files` manifest, id-keyed incremental `Bm25Index`, `PreviousIndex` + reuse in `create_index_from_path`, `load_previous_for_incremental` in the orchestrator, + `INDEX_SCHEMA_VERSION` 1 → 2. See §4.7 / §4.8 / §4.9 / §4.14 and §6.1 items 9–10 for the + two intentional deviations (hash vs mtime, scoring scale). --- diff --git a/README.ko.md b/README.ko.md index f82c33e..1421e46 100644 --- a/README.ko.md +++ b/README.ko.md @@ -397,7 +397,7 @@ csp find-related src/auth.ts 42 ./my-project `--content`는 `code` (기본), `docs`, `config`, `all`을 받습니다. `path`를 생략하면 현재 디렉터리를 사용합니다. git URL도 받습니다. `csp`가 `$PATH`에 없다면 `bunx @pleaseai/csp`로 대체하세요. -`csp search`나 `csp find-related`를 `--index` 없이 실행하면, `csp`는 소스와 콘텐츠 선택을 키로 하여 글로벌 캐시 `~/.csp/index/`에 자동으로 인덱싱·캐시합니다. 다음 실행 때 캐시를 재사용하며, 소스 파일이 바뀌면 콘텐츠 해시로 자동 무효화되므로 수동으로 다시 인덱싱할 필요가 없습니다. `--index <경로>`를 지정하면 그 경로를 그대로 사용하고 자동 캐시를 우회합니다. `csp index -o <경로>`는 명시적 영속화 전용(`-o` 필수)이며 자동 캐시와는 독립적입니다. +`csp search`나 `csp find-related`를 `--index` 없이 실행하면, `csp`는 소스와 콘텐츠 선택을 키로 하여 글로벌 캐시 `~/.csp/index/`에 자동으로 인덱싱·캐시합니다. 다음 실행 때 캐시를 재사용하며, 소스 파일이 바뀌면 콘텐츠 해시로 자동 무효화되므로 수동으로 다시 인덱싱할 필요가 없습니다. 무효화된 캐시는 증분으로 다시 만들어집니다. 내용이 바뀐 파일만 다시 청킹·임베딩하고, 바뀌지 않은 파일은 기존 청크·벡터·BM25 포스팅을 그대로 유지합니다. `--index <경로>`를 지정하면 그 경로를 그대로 사용하고 자동 캐시를 우회합니다. `csp index -o <경로>`는 명시적 영속화 전용(`-o` 필수)이며 자동 캐시와는 독립적입니다.
토큰 절약량 보기 diff --git a/README.md b/README.md index e764a79..d90607c 100644 --- a/README.md +++ b/README.md @@ -397,7 +397,7 @@ csp find-related src/auth.ts 42 ./my-project `--content` accepts `code` (default), `docs`, `config`, or `all`. `path` defaults to the current directory when omitted; git URLs are accepted. If `csp` is not on `$PATH`, use `bunx @pleaseai/csp` in its place. -When you run `csp search` or `csp find-related` **without** `--index`, `csp` automatically indexes and caches the source in a global cache at `~/.csp/index/`, keyed by the source and content selection. The cache is reused on the next run and invalidated automatically when the source files change (by content hash), so you do not need to reindex manually. Passing `--index ` uses that exact path instead and bypasses the auto-cache. `csp index -o ` is for explicit persistence only (`-o` is required) and is independent of the auto-cache. +When you run `csp search` or `csp find-related` **without** `--index`, `csp` automatically indexes and caches the source in a global cache at `~/.csp/index/`, keyed by the source and content selection. The cache is reused on the next run and invalidated automatically when the source files change (by content hash), so you do not need to reindex manually. A stale cache is rebuilt incrementally: only files whose content changed are re-chunked and re-embedded, and unchanged files keep their existing chunks, vectors, and BM25 postings. Passing `--index ` uses that exact path instead and bypasses the auto-cache. `csp index -o ` is for explicit persistence only (`-o` is required) and is independent of the auto-cache.
Savings diff --git a/crates/csp/src/indexing/cache.rs b/crates/csp/src/indexing/cache.rs index 422e073..f22f932 100644 --- a/crates/csp/src/indexing/cache.rs +++ b/crates/csp/src/indexing/cache.rs @@ -200,6 +200,14 @@ pub(crate) fn compute_content_hash_from_paths(mut files: Vec<(String, PathBuf)>) to_hex(&hasher.finalize()) } +/// sha256 (hex) of raw bytes — the per-file hash recorded in the index's file +/// manifest for incremental reindexing. +pub(crate) fn sha256_hex(bytes: &[u8]) -> String { + let mut hasher = Sha256::new(); + hasher.update(bytes); + to_hex(&hasher.finalize()) +} + fn update_content_hash(hasher: &mut Sha256, path: &str, content: &[u8]) { let len16 = path.encode_utf16().count(); hasher.update(format!("{len16}:{path}").as_bytes()); diff --git a/crates/csp/src/indexing/cache_orchestrator.rs b/crates/csp/src/indexing/cache_orchestrator.rs index 1bef6be..79114e0 100644 --- a/crates/csp/src/indexing/cache_orchestrator.rs +++ b/crates/csp/src/indexing/cache_orchestrator.rs @@ -1,5 +1,6 @@ -//! Build-or-reuse orchestration for the global on-disk index cache. Port of -//! `src/indexing/cache.ts`. +//! Build-or-reuse orchestration for the global on-disk index cache (the +//! `get_validated_cache` / `load_previous_for_incremental` half of semble +//! `cache.py`, adapted to csp's content-hash oracle — ADR-0002 / ADR-0005). use std::path::{Path, PathBuf}; @@ -8,10 +9,16 @@ use crate::indexing::cache::{ compute_content_hash_from_paths, ensure_cache_dir, resolve_cache_dir, CacheLocation, }; use crate::indexing::create::MAX_FILE_BYTES; +use crate::indexing::dense::SelectableBasicBackend; use crate::indexing::dense::DEFAULT_MODEL_NAME; use crate::indexing::file_walker::walk_files; use crate::indexing::files::get_extensions; -use crate::indexing::index::{normalize_content, parse_manifest, CspIndex, LoadOptions}; +use crate::indexing::index::{ + normalize_content, parse_manifest, read_chunks, CspIndex, IndexManifest, LoadOptions, + INDEX_SCHEMA_VERSION, +}; +use crate::indexing::sparse::Bm25Index; +use crate::indexing::types::PreviousIndex; use crate::types::ContentType; use crate::utils::is_git_url; @@ -91,46 +98,85 @@ pub fn load_or_build_index(source: &str, options: &LoadOrBuildOptions) -> Result let load_options = LoadOptions { model_path: options.model_path.clone(), - content: Some(content), + content: Some(content.clone()), }; let index = if is_git { CspIndex::from_git(source, &load_options, options.git_ref.as_deref())? } else { - CspIndex::from_path(Path::new(source), &load_options)? + // Stale (or absent) cache: seed the rebuild with the previous index so + // unchanged files keep their chunks, vectors, and BM25 postings. + let previous = load_previous_for_incremental(&cache_dir, expected_model, &content); + CspIndex::from_path_with_previous(Path::new(source), &load_options, previous)? }; index.save(&cache_dir, source_hash.as_deref())?; Ok(index) } -/// Reuse a cached index when present and valid, else `None`. -fn try_reuse( - cache_dir: &Path, - is_git: bool, - source_hash: Option<&str>, - expected_model: &str, -) -> Option { - let manifest_path = cache_dir.join("manifest.json"); - if !manifest_path.exists() { - return None; - } - let raw = std::fs::read_to_string(&manifest_path).ok()?; +/// Read and parse `/manifest.json`, or `None` when absent/malformed. +fn read_manifest(cache_dir: &Path) -> Option { + let raw = std::fs::read_to_string(cache_dir.join("manifest.json")).ok()?; let value: serde_json::Value = serde_json::from_str(&raw).ok()?; - let manifest = parse_manifest(&value).ok()?; + parse_manifest(&value).ok() +} + +/// Whether a persisted manifest describes an index the current build +/// parameters could reuse (mirrors upstream `_metadata_matches`). +fn manifest_compatible(manifest: &IndexManifest, expected_model: &str) -> bool { + // A schema change means the on-disk artifacts may be laid out differently. + if manifest.schema_version != INDEX_SCHEMA_VERSION { + return false; + } // A chunk_size change re-chunks every file, so a cache built with a different // target length is stale even if the source files are byte-identical. if manifest.chunk_size != Some(DESIRED_CHUNK_LENGTH_CHARS as u32) { - return None; + return false; } // A model change makes the persisted vectors incompatible with queries // embedded by the new model, so a cache built with a different model is stale. if manifest.model_id != expected_model { - return None; + return false; } // The persisted vectors and live query model must use the same runtime // implementation. This distinguishes a real Model2Vec cache from an offline // deterministic-stub cache even when both share the same requested model id. let (query_model, _) = crate::indexing::dense::load_model(Some(expected_model)); - if manifest.model_kind.as_deref() != Some(query_model.kind()) { + manifest.model_kind.as_deref() == Some(query_model.kind()) +} + +/// Load a compatible cached index as a seed for incremental reindexing, or +/// `None` when the cache is absent, incompatible, or structurally inconsistent +/// (fails closed — a full rebuild is always correct). Port of upstream +/// `load_previous_for_incremental`. +pub(crate) fn load_previous_for_incremental( + cache_dir: &Path, + expected_model: &str, + content: &[ContentType], +) -> Option { + let manifest = read_manifest(cache_dir)?; + if !manifest_compatible(&manifest, expected_model) { + return None; + } + let same_content = manifest.content.len() == content.len() + && content.iter().all(|c| manifest.content.contains(c)); + if !same_content || manifest.files.is_empty() { + return None; + } + + let chunks = read_chunks(cache_dir).ok()?; + let vectors = SelectableBasicBackend::load(cache_dir).ok()?.vectors; + let bm25_index = Bm25Index::load(cache_dir).ok()?; + PreviousIndex::try_new(chunks, vectors, manifest.files, bm25_index).ok() +} + +/// Reuse a cached index when present and valid, else `None`. +fn try_reuse( + cache_dir: &Path, + is_git: bool, + source_hash: Option<&str>, + expected_model: &str, +) -> Option { + let manifest = read_manifest(cache_dir)?; + if !manifest_compatible(&manifest, expected_model) { return None; } // Local sources additionally validate the live source-file hash; git sources diff --git a/crates/csp/src/indexing/cache_orchestrator/tests.rs b/crates/csp/src/indexing/cache_orchestrator/tests.rs index 9c5b440..1105ce9 100644 --- a/crates/csp/src/indexing/cache_orchestrator/tests.rs +++ b/crates/csp/src/indexing/cache_orchestrator/tests.rs @@ -1,6 +1,6 @@ use super::*; use crate::indexing::cache::{resolve_cache_dir, CacheLocation}; -use crate::indexing::dense::{make_stub_model, SelectableBasicBackend}; +use crate::indexing::dense::{make_stub_model, SelectableBasicBackend, DEFAULT_MODEL_NAME}; use crate::indexing::index::{CspIndexState, DEFAULT_CONTENT}; use crate::indexing::sparse::Bm25Index; use crate::types::Chunk; @@ -27,6 +27,7 @@ fn build_index(chunks: Vec) -> CspIndex { model_path: "test-model".to_string(), root: None, content: DEFAULT_CONTENT.to_vec(), + files: Default::default(), }) } @@ -145,3 +146,166 @@ fn load_or_build_miss_then_hit_then_invalidate() { assert!(third.chunks.iter().any(|c| c.file_path == "b.ts")); assert!(third.chunks.len() >= first.chunks.len()); } + +// --- load_previous_for_incremental (mirrors upstream tests/index/test_create.py) --- + +/// Build a real, well-formed cache for a two-file source and return +/// `(source dir, cache dir)`; `home` keeps the cache alive for the caller. +fn build_valid_cache(home: &Path) -> (tempfile::TempDir, PathBuf) { + let src = tempdir().unwrap(); + std::fs::write(src.path().join("a.ts"), "function alpha() { return 1 }\n").unwrap(); + std::fs::write(src.path().join("b.ts"), "function beta() { return 2 }\n").unwrap(); + let src_str = src.path().to_string_lossy().into_owned(); + let base = home.join(".csp"); + let opts = LoadOrBuildOptions { + base_dir: Some(base.clone()), + ..Default::default() + }; + load_or_build_index(&src_str, &opts).unwrap(); + let cache_dir = resolve_cache_dir( + &src_str, + DEFAULT_CONTENT, + &CacheLocation { + base_dir: Some(base), + git_ref: None, + }, + ); + (src, cache_dir) +} + +fn read_json(path: &Path) -> serde_json::Value { + serde_json::from_str(&std::fs::read_to_string(path).unwrap()).unwrap() +} + +fn write_json(path: &Path, value: &serde_json::Value) { + std::fs::write(path, value.to_string()).unwrap(); +} + +#[test] +fn load_previous_for_incremental_happy_path() { + let home = tempdir().unwrap(); + let (_src, cache_dir) = build_valid_cache(home.path()); + + let previous = + load_previous_for_incremental(&cache_dir, DEFAULT_MODEL_NAME, DEFAULT_CONTENT).unwrap(); + assert_eq!(previous.chunks.len(), previous.vectors.len()); + assert_eq!(previous.chunks.len(), previous.bm25_index.doc_order().len()); + assert!(previous.files.contains_key("a.ts")); + assert!(previous.files.contains_key("b.ts")); +} + +#[test] +fn load_previous_for_incremental_fails_closed() { + for corrupt in [ + "missing_cache", + "missing_files_key", + "metadata_mismatch", + "schema_version_mismatch", + "component_length_mismatch", + "length_mismatch", + "overlapping_entries", + "bm25_order_mismatch", + "corrupt_json", + ] { + let home = tempdir().unwrap(); + let cache_dir = if corrupt == "missing_cache" { + home.path().join("no-such-cache") + } else { + let (_src, cache_dir) = build_valid_cache(home.path()); + let manifest_path = cache_dir.join("manifest.json"); + let mut manifest = read_json(&manifest_path); + match corrupt { + "missing_files_key" => { + manifest.as_object_mut().unwrap().remove("files"); + } + "metadata_mismatch" => manifest["modelId"] = serde_json::json!("other/model"), + "schema_version_mismatch" => manifest["schemaVersion"] = serde_json::json!(1), + "component_length_mismatch" => { + let chunks_path = cache_dir.join("chunks.json"); + let mut chunks = read_json(&chunks_path); + chunks.as_array_mut().unwrap().pop(); + write_json(&chunks_path, &chunks); + } + "length_mismatch" => { + let count = manifest["files"]["a.ts"]["count"].as_u64().unwrap(); + manifest["files"]["a.ts"]["count"] = serde_json::json!(count + 5); + } + "overlapping_entries" => { + let start = manifest["files"]["a.ts"]["start"].clone(); + manifest["files"]["b.ts"]["start"] = start; + } + "bm25_order_mismatch" => { + let bm25_path = cache_dir.join("bm25.json"); + let mut bm25 = read_json(&bm25_path); + bm25["docOrder"].as_array_mut().unwrap().reverse(); + write_json(&bm25_path, &bm25); + } + "corrupt_json" => { + std::fs::write(&manifest_path, "{not json").unwrap(); + } + _ => unreachable!(), + } + if corrupt != "corrupt_json" { + write_json(&manifest_path, &manifest); + } + cache_dir + }; + + assert!( + load_previous_for_incremental(&cache_dir, DEFAULT_MODEL_NAME, DEFAULT_CONTENT) + .is_none(), + "case {corrupt} should fail closed" + ); + } +} + +#[test] +fn load_or_build_reuses_unchanged_files_on_incremental_rebuild() { + let home = tempdir().unwrap(); + let (src, cache_dir) = build_valid_cache(home.path()); + let src_str = src.path().to_string_lossy().into_owned(); + let opts = LoadOrBuildOptions { + base_dir: Some(home.path().join(".csp")), + ..Default::default() + }; + + // Plant a sentinel in the cached chunk for the file that will not change: it + // can only reach the rebuilt index by being reused from the previous index. + let chunks_path = cache_dir.join("chunks.json"); + let mut chunks = read_json(&chunks_path); + let a_chunk = chunks + .as_array_mut() + .unwrap() + .iter_mut() + .find(|c| c["filePath"] == "a.ts") + .unwrap(); + let content = format!("{}/*reused*/", a_chunk["content"].as_str().unwrap()); + a_chunk["content"] = serde_json::json!(content); + write_json(&chunks_path, &chunks); + let b_hash_before = + read_json(&cache_dir.join("manifest.json"))["files"]["b.ts"]["hash"].clone(); + + // Change b.ts → whole-tree hash mismatch → incremental rebuild. + std::fs::write(src.path().join("b.ts"), "function beta() { return 22 }\n").unwrap(); + let rebuilt = load_or_build_index(&src_str, &opts).unwrap(); + + let a_chunk = rebuilt + .chunks + .iter() + .find(|c| c.file_path == "a.ts") + .unwrap(); + assert!(a_chunk.content.ends_with("/*reused*/")); + let b_chunk = rebuilt + .chunks + .iter() + .find(|c| c.file_path == "b.ts") + .unwrap(); + assert!(b_chunk.content.contains("22")); + + // The persisted manifest now records b.ts's new hash and is a valid seed. + let manifest = read_json(&cache_dir.join("manifest.json")); + assert_ne!(manifest["files"]["b.ts"]["hash"], b_hash_before); + assert!( + load_previous_for_incremental(&cache_dir, DEFAULT_MODEL_NAME, DEFAULT_CONTENT).is_some() + ); +} diff --git a/crates/csp/src/indexing/create.rs b/crates/csp/src/indexing/create.rs index 0bf9965..0362b50 100644 --- a/crates/csp/src/indexing/create.rs +++ b/crates/csp/src/indexing/create.rs @@ -1,17 +1,23 @@ -//! Index orchestration. Port of `src/indexing/create.ts` -//! (← semble `index/create.py`). +//! Index orchestration. Port of semble `index/create.py` (incremental reuse +//! from upstream #225). //! //! Walks files matching the resolved extensions, chunks them, enriches + //! tokenizes text for BM25, embeds the chunks, and returns the populated -//! sparse/dense indexes alongside the chunk list. +//! sparse/dense indexes alongside the chunk list and a per-file manifest. +//! When a [`PreviousIndex`] is supplied, files whose content hash is unchanged +//! reuse their previous chunks, vector rows, and BM25 postings; only changed +//! files are re-chunked and re-embedded, and deleted files' postings are +//! dropped. use std::path::{Path, PathBuf}; use crate::chunking::source::chunk_source; +use crate::indexing::cache::sha256_hex; use crate::indexing::dense::{embed_chunks, Model, SelectableBasicBackend}; use crate::indexing::file_walker::walk_files; use crate::indexing::files::{detect_language, get_extensions}; use crate::indexing::sparse::{enrich_for_bm25, Bm25Index}; +use crate::indexing::types::{make_chunk_id, FileManifest, FileManifestEntry, PreviousIndex}; use crate::tokens::tokenize; use crate::types::{Chunk, ContentType}; @@ -35,12 +41,38 @@ pub struct CreateIndexResult { pub bm25_index: Bm25Index, pub semantic_index: SelectableBasicBackend, pub chunks: Vec, + /// Per-file content hash + chunk range, for the next incremental reindex. + pub files: FileManifest, } -/// Create an index from a resolved directory. Errors when no chunks are produced. +/// Replace a file's BM25 postings: remove its old slots (if any), then add its +/// new ones. +fn reindex_file( + bm25_index: &mut Bm25Index, + indexed_path: &str, + file_chunks: &[Chunk], + previous_entry: Option<&FileManifestEntry>, +) -> Result<(), String> { + if let Some(entry) = previous_entry { + for slot in 0..entry.count { + bm25_index.remove_document(&make_chunk_id(indexed_path, slot)); + } + } + for (slot, chunk) in file_chunks.iter().enumerate() { + bm25_index.add_document( + &make_chunk_id(indexed_path, slot), + &tokenize(&enrich_for_bm25(chunk)), + )?; + } + Ok(()) +} + +/// Create an index from a resolved directory, optionally reusing a previous +/// index's unchanged files. Errors when no chunks are produced. pub fn create_index_from_path( path: &Path, options: &CreateIndexOptions, + previous: Option, ) -> Result { let content = options .content @@ -49,7 +81,29 @@ pub fn create_index_from_path( let resolved = get_extensions(&content, options.extensions.as_deref()); let ext_refs: Vec<&str> = resolved.iter().map(String::as_str).collect(); + // The previous index is consumed: its BM25 index is mutated in place and its + // chunk/vector rows are moved out rather than copied. + let (mut bm25_index, previous_files, mut previous_chunks, mut previous_vectors) = match previous + { + Some(prev) => ( + prev.bm25_index, + prev.files, + prev.chunks.into_iter().map(Some).collect::>(), + prev.vectors.into_iter().map(Some).collect::>(), + ), + None => ( + Bm25Index::new(), + FileManifest::new(), + Vec::new(), + Vec::new(), + ), + }; + let mut chunks: Vec = Vec::new(); + let mut chunk_ids: Vec = Vec::new(); + let mut vectors: Vec> = Vec::new(); + let mut files = FileManifest::new(); + for file_path in walk_files(path, &ext_refs, &[]) { let language = detect_language(&file_path.to_string_lossy()); let size = match std::fs::metadata(&file_path) { @@ -59,14 +113,11 @@ pub fn create_index_from_path( if size > MAX_FILE_BYTES { continue; } - // Lossy UTF-8 decode (invalid bytes → U+FFFD) to match the TS oracle's - // `readFileSync(path, 'utf8')`, which decodes lossily and only skips on - // an IO error — `read_to_string` would instead drop the whole file. - let source = match std::fs::read(&file_path) { - Ok(bytes) => String::from_utf8_lossy(&bytes).into_owned(), - Err(_) => continue, + let Ok(bytes) = std::fs::read(&file_path) else { + continue; }; - let chunk_path = match &options.display_root { + let hash = sha256_hex(&bytes); + let indexed_path = match &options.display_root { Some(root) => file_path .strip_prefix(root) .unwrap_or(&file_path) @@ -74,7 +125,60 @@ pub fn create_index_from_path( .into_owned(), None => file_path.to_string_lossy().into_owned(), }; - chunks.extend(chunk_source(&source, &chunk_path, language)); + let previous_entry = previous_files.get(&indexed_path); + + // Unchanged file: move its previous chunk + vector rows out (each row is + // taken at most once because a validated manifest's ranges never overlap). + let reused = match previous_entry { + Some(entry) + if entry.hash == hash + && entry.end() <= previous_chunks.len() + && entry.end() <= previous_vectors.len() => + { + let rows: Option> = previous_chunks[entry.start..entry.end()] + .iter_mut() + .map(Option::take) + .collect(); + let vecs: Option>> = previous_vectors[entry.start..entry.end()] + .iter_mut() + .map(Option::take) + .collect(); + rows.zip(vecs) + } + _ => None, + }; + let (file_chunks, file_vectors) = match reused { + Some(reused) => reused, + None => { + // Lossy UTF-8 decode (invalid bytes → U+FFFD): only an IO error + // skips a file, never an encoding error. + let source = String::from_utf8_lossy(&bytes).into_owned(); + let file_chunks = chunk_source(&source, &indexed_path, language); + reindex_file(&mut bm25_index, &indexed_path, &file_chunks, previous_entry)?; + // Normalise through the backend so fresh rows match the reused + // (already-normalised) rows. + let file_vectors = SelectableBasicBackend::from_vectors(embed_chunks( + options.model, + &file_chunks, + ))? + .vectors; + (file_chunks, file_vectors) + } + }; + + let start = chunks.len(); + let count = file_chunks.len(); + chunk_ids.extend((0..count).map(|slot| make_chunk_id(&indexed_path, slot))); + chunks.extend(file_chunks); + vectors.extend(file_vectors); + files.insert(indexed_path, FileManifestEntry { hash, start, count }); + } + + // Files that vanished since the previous index: drop their postings. + for (indexed_path, entry) in &previous_files { + if !files.contains_key(indexed_path) { + reindex_file(&mut bm25_index, indexed_path, &[], Some(entry))?; + } } if chunks.is_empty() { @@ -84,18 +188,14 @@ pub fn create_index_from_path( )); } - let embeddings = embed_chunks(options.model, &chunks); - let documents: Vec> = chunks - .iter() - .map(|c| tokenize(&enrich_for_bm25(c))) - .collect(); - let bm25_index = Bm25Index::build(&documents); - let semantic_index = SelectableBasicBackend::from_vectors(embeddings)?; + bm25_index.set_doc_order(chunk_ids); + let semantic_index = SelectableBasicBackend::from_normalized(vectors)?; Ok(CreateIndexResult { bm25_index, semantic_index, chunks, + files, }) } @@ -103,6 +203,7 @@ pub fn create_index_from_path( mod tests { use super::*; use crate::indexing::dense::make_stub_model; + use crate::tokens::tokenize; use tempfile::tempdir; fn opts(model: &Model, display_root: Option) -> CreateIndexOptions<'_> { @@ -123,9 +224,12 @@ mod tests { ) .unwrap(); let model = make_stub_model(4); - let result = - create_index_from_path(dir.path(), &opts(&model, Some(dir.path().to_path_buf()))) - .unwrap(); + let result = create_index_from_path( + dir.path(), + &opts(&model, Some(dir.path().to_path_buf())), + None, + ) + .unwrap(); assert!(!result.chunks.is_empty()); assert_eq!(result.chunks[0].file_path, "sample.ts"); @@ -138,7 +242,7 @@ mod tests { let dir = tempdir().unwrap(); std::fs::write(dir.path().join("data.bin"), "binary").unwrap(); let model = make_stub_model(4); - let err = create_index_from_path(dir.path(), &opts(&model, None)).unwrap_err(); + let err = create_index_from_path(dir.path(), &opts(&model, None), None).unwrap_err(); assert!(err.contains("No supported files found")); } @@ -153,7 +257,7 @@ mod tests { content: Some(vec![ContentType::Docs]), display_root: Some(dir.path().to_path_buf()), }; - let result = create_index_from_path(dir.path(), &options).unwrap(); + let result = create_index_from_path(dir.path(), &options, None).unwrap(); assert_eq!(result.chunks.len(), 1); assert_eq!(result.chunks[0].file_path, "a.txt"); } @@ -164,9 +268,12 @@ mod tests { std::fs::write(dir.path().join("big.ts"), "a".repeat(2_000_000)).unwrap(); std::fs::write(dir.path().join("small.ts"), "export const x = 1\n").unwrap(); let model = make_stub_model(4); - let result = - create_index_from_path(dir.path(), &opts(&model, Some(dir.path().to_path_buf()))) - .unwrap(); + let result = create_index_from_path( + dir.path(), + &opts(&model, Some(dir.path().to_path_buf())), + None, + ) + .unwrap(); let paths: Vec<&str> = result.chunks.iter().map(|c| c.file_path.as_str()).collect(); assert!(paths.contains(&"small.ts")); assert!(!paths.contains(&"big.ts")); @@ -178,12 +285,148 @@ mod tests { std::fs::create_dir(dir.path().join("sub")).unwrap(); std::fs::write(dir.path().join("sub/nested.ts"), "const a = 1\n").unwrap(); let model = make_stub_model(4); - let result = - create_index_from_path(dir.path(), &opts(&model, Some(dir.path().to_path_buf()))) - .unwrap(); + let result = create_index_from_path( + dir.path(), + &opts(&model, Some(dir.path().to_path_buf())), + None, + ) + .unwrap(); assert!(result .chunks .iter() .any(|c| c.file_path.ends_with("nested.ts"))); } + + // --- incremental reindex (mirrors upstream tests/index/test_create.py) --- + + fn write_files(root: &Path, files: &[(&str, &str)]) { + for (rel, content) in files { + let path = root.join(rel); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, content).unwrap(); + } + } + + fn into_previous(result: CreateIndexResult) -> PreviousIndex { + PreviousIndex::try_new( + result.chunks, + result.semantic_index.vectors, + result.files, + result.bm25_index, + ) + .unwrap() + } + + fn score_sum(index: &Bm25Index, query: &str) -> f32 { + index.get_scores(&tokenize(query), None).iter().sum() + } + + #[test] + fn fresh_build_records_a_layout_valid_file_manifest() { + let dir = tempdir().unwrap(); + write_files( + dir.path(), + &[ + ("a.ts", "function stable_anchor() { return 1 }\n"), + ("sub/b.ts", "function other_value() { return 2 }\n"), + ], + ); + let model = make_stub_model(4); + let result = create_index_from_path( + dir.path(), + &opts(&model, Some(dir.path().to_path_buf())), + None, + ) + .unwrap(); + + assert_eq!(result.files.len(), 2); + let total: usize = result.files.values().map(|e| e.count).sum(); + assert_eq!(total, result.chunks.len()); + for (path, entry) in &result.files { + assert_eq!(entry.hash.len(), 64); + assert!(result.chunks[entry.start..entry.end()] + .iter() + .all(|c| c.file_path == *path)); + } + // The manifest, chunks, vectors, and BM25 order all agree. + assert!(into_previous(result).files.contains_key("a.ts")); + } + + #[test] + fn incremental_reindex_reuses_updates_and_prunes() { + let dir = tempdir().unwrap(); + let root = dir.path().to_path_buf(); + write_files( + &root, + &[ + ("a.ts", "function stable_anchor() { return 1 }\n"), + ("b.ts", "function changed_value() { return 2 }\n"), + ("c.ts", "function unique_gone() { return 3 }\n"), + ("emptying.ts", "function becomes_empty() { return 4 }\n"), + ], + ); + let model = make_stub_model(4); + let before = + create_index_from_path(&root, &opts(&model, Some(root.clone())), None).unwrap(); + let b_before = before.files["b.ts"].clone(); + let b_vectors_before = + before.semantic_index.vectors[b_before.start..b_before.end()].to_vec(); + let a_before = before.files["a.ts"].clone(); + + // Plant sentinels in the previous index for the unchanged file: they can + // only survive into the rebuilt index if its rows were reused, not + // re-chunked/re-embedded (the stub embedder is deterministic). + let mut previous = into_previous(before); + let sentinel_vector = vec![0.0, 1.0, 0.0, 0.0]; + previous.vectors[a_before.start] = sentinel_vector.clone(); + previous.chunks[a_before.start] + .content + .push_str("/*reused*/"); + + write_files( + &root, + &[("b.ts", "function changed_value() { return 999 }\n")], + ); + std::fs::remove_file(root.join("c.ts")).unwrap(); + write_files(&root, &[("emptying.ts", &" ".repeat(128))]); + write_files( + &root, + &[("d.ts", "function brand_new_term() { return 4 }\n")], + ); + + let after = + create_index_from_path(&root, &opts(&model, Some(root.clone())), Some(previous)) + .unwrap(); + + let a_after = &after.files["a.ts"]; + assert_eq!(after.semantic_index.vectors[a_after.start], sentinel_vector); + assert!(after.chunks[a_after.start].content.ends_with("/*reused*/")); + let b_after = &after.files["b.ts"]; + assert_ne!( + after.semantic_index.vectors[b_after.start..b_after.end()].to_vec(), + b_vectors_before + ); + assert!(!after.files.contains_key("c.ts")); + assert!(after.files.contains_key("d.ts")); + assert_eq!(after.files["emptying.ts"].count, 0); + + assert_eq!(score_sum(&after.bm25_index, "unique_gone"), 0.0); + assert_eq!(score_sum(&after.bm25_index, "becomes_empty"), 0.0); + assert!(score_sum(&after.bm25_index, "brand_new_term") > 0.0); + assert!(score_sum(&after.bm25_index, "changed_value") > 0.0); + + let mut expected_ids: Vec = after + .files + .iter() + .flat_map(|(path, entry)| (0..entry.count).map(move |slot| make_chunk_id(path, slot))) + .collect(); + expected_ids.sort(); + let mut doc_order = after.bm25_index.doc_order().to_vec(); + doc_order.sort(); + assert_eq!(doc_order, expected_ids); + assert_eq!(after.bm25_index.corpus_size(), after.chunks.len()); + assert_eq!(after.semantic_index.vectors.len(), after.chunks.len()); + // The rebuilt index is itself a valid seed for the next incremental pass. + into_previous(after); + } } diff --git a/crates/csp/src/indexing/dense/backend.rs b/crates/csp/src/indexing/dense/backend.rs index 4b88694..3b96186 100644 --- a/crates/csp/src/indexing/dense/backend.rs +++ b/crates/csp/src/indexing/dense/backend.rs @@ -87,6 +87,29 @@ impl SelectableBasicBackend { Self::new(vectors, BasicArgs::default()) } + /// Build from rows that are **already** L2-normalised (taken from a persisted + /// backend or a fresh [`new`](Self::new)), skipping the normalisation pass so + /// reused rows stay bit-identical. Errors on inconsistent dimensions. + pub fn from_normalized(vectors: Vec>) -> Result { + let dim = vectors.first().map(Vec::len).unwrap_or(0); + if !vectors.is_empty() && dim == 0 { + return Err( + "Vector dimension must be greater than 0 for a non-empty index".to_string(), + ); + } + if let Some(bad) = vectors.iter().find(|v| v.len() != dim) { + return Err(format!( + "Inconsistent vector dimensions: expected {dim}, got {}", + bad.len() + )); + } + Ok(Self { + vectors, + arguments: BasicArgs::default(), + dim, + }) + } + /// Batched k-NN query. Returns, per query, `[(chunk_index, cosine_distance)]` /// sorted by ascending distance. `selector` constrains results to a pool. pub fn query( diff --git a/crates/csp/src/indexing/index.rs b/crates/csp/src/indexing/index.rs index e11e028..b406fc4 100644 --- a/crates/csp/src/indexing/index.rs +++ b/crates/csp/src/indexing/index.rs @@ -1,23 +1,25 @@ -//! `CspIndex` — the hybrid (dense + BM25) search orchestrator. Port of -//! `src/indexing/index.ts` (← semble `index/index.py`). +//! `CspIndex` — the hybrid (dense + BM25) search orchestrator. Port of semble +//! `index/index.py`. use std::collections::{BTreeMap, HashSet}; -use std::fmt::Write as _; use std::path::Path; use std::process::Command; use serde::{Deserialize, Serialize}; -use sha2::{Digest, Sha256}; use crate::chunking::source::DESIRED_CHUNK_LENGTH_CHARS; +use crate::indexing::cache::sha256_hex; use crate::indexing::create::{create_index_from_path, CreateIndexOptions}; use crate::indexing::dense::{load_model, make_stub_model, Model, SelectableBasicBackend}; use crate::indexing::sparse::Bm25Index; +use crate::indexing::types::{FileManifest, FileManifestEntry, PreviousIndex}; use crate::search::{search as run_search, SearchOptions as RunSearchOptions, SearchResult}; use crate::types::{chunk_from_dict, chunk_to_dict, Chunk, ChunkDict, ContentType, IndexStats}; -/// On-disk index schema version. -pub const INDEX_SCHEMA_VERSION: u32 = 1; +/// On-disk index schema version. Bump when the persisted layout changes so +/// older caches are rebuilt rather than misread (v2: per-file `files` manifest +/// + id-keyed `bm25.json`, upstream #225). +pub const INDEX_SCHEMA_VERSION: u32 = 2; /// Default content selection (code-only). pub const DEFAULT_CONTENT: &[ContentType] = &[ContentType::Code]; @@ -42,6 +44,10 @@ pub struct IndexManifest { /// (mirrors semble `_metadata_matches`). `None` = built before this field /// existed → treated as a mismatch. pub chunk_size: Option, + /// Per-file content hash + chunk range, used for incremental reindexing + /// (mirrors upstream metadata `files`; hash-keyed instead of `mtime_ns`). + #[serde(default)] + pub files: FileManifest, } /// Query options for [`CspIndex::search`] / [`CspIndex::find_related`]. @@ -68,6 +74,8 @@ pub struct CspIndexState { pub model_path: String, pub root: Option, pub content: Vec, + /// Per-file content hash + chunk range (empty for hand-built fixtures). + pub files: FileManifest, } /// Hybrid (dense + BM25) code search index. @@ -80,6 +88,8 @@ pub struct CspIndex { pub model_path: String, pub root: Option, pub content: Vec, + /// Per-file content hash + chunk range, used for incremental reindexing. + pub files: FileManifest, } pub(crate) fn normalize_content(content: Option>) -> Vec { @@ -96,11 +106,24 @@ impl CspIndex { model_path: state.model_path, root: state.root, content: state.content, + files: state.files, } } /// Build an index from a local directory. pub fn from_path(path: &Path, options: &LoadOptions) -> Result { + Self::from_path_with_previous(path, options, None) + } + + /// Build an index from a local directory, reusing the unchanged files of a + /// compatible previous index (see + /// `cache_orchestrator::load_previous_for_incremental`). Only files whose + /// content hash changed are re-chunked and re-embedded. + pub fn from_path_with_previous( + path: &Path, + options: &LoadOptions, + previous: Option, + ) -> Result { let meta = std::fs::metadata(path) .map_err(|_| format!("Path does not exist: {}", path.display()))?; if !meta.is_dir() { @@ -118,6 +141,7 @@ impl CspIndex { content: Some(content.clone()), display_root: Some(path.to_path_buf()), }, + previous, )?; Ok(Self::new(CspIndexState { @@ -128,6 +152,7 @@ impl CspIndex { model_path, root: Some(path.to_string_lossy().into_owned()), content, + files: result.files, })) } @@ -159,6 +184,7 @@ impl CspIndex { model_path: index.model_path, root: Some(url.to_string()), content: index.content, + files: index.files, })) } @@ -287,12 +313,13 @@ impl CspIndex { schema_version: INDEX_SCHEMA_VERSION, content_hash: content_hash .map(str::to_string) - .unwrap_or_else(|| hash_chunks(&chunks_json)), + .unwrap_or_else(|| sha256_hex(chunks_json.as_bytes())), source_id: self.root.clone(), content: self.content.clone(), model_id: self.model_path.clone(), model_kind: Some(self.model.kind().to_string()), chunk_size: Some(DESIRED_CHUNK_LENGTH_CHARS as u32), + files: self.files.clone(), }; let manifest_json = serde_json::to_string(&manifest).map_err(|e| e.to_string())?; std::fs::write(dir.join("manifest.json"), manifest_json).map_err(|e| e.to_string()) @@ -328,17 +355,12 @@ impl CspIndex { } let manifest = parse_manifest(&value)?; - let chunks_raw = - std::fs::read_to_string(dir.join("chunks.json")).map_err(|e| e.to_string())?; - let chunk_values: Vec = - serde_json::from_str(&chunks_raw).map_err(|e| e.to_string())?; - let mut chunks = Vec::with_capacity(chunk_values.len()); - for v in &chunk_values { - chunks.push(chunk_from_dict(v).map_err(|e| e.to_string())?); - } - + let chunks = read_chunks(dir)?; let bm25_index = Bm25Index::load(dir).map_err(|e| e.to_string())?; let semantic_index = SelectableBasicBackend::load(dir)?; + if chunks.len() != bm25_index.num_docs() || chunks.len() != semantic_index.vectors.len() { + return Err("Persisted index components have inconsistent document counts".to_string()); + } let (model, model_path) = load_model(Some(&manifest.model_id)); // Align the query model's dim with the persisted vectors. @@ -356,10 +378,23 @@ impl CspIndex { model_path, root: manifest.source_id, content: manifest.content, + files: manifest.files, })) } } +/// Read and validate `/chunks.json`. +pub(crate) fn read_chunks(dir: &Path) -> Result, String> { + let chunks_raw = std::fs::read_to_string(dir.join("chunks.json")).map_err(|e| e.to_string())?; + let chunk_values: Vec = + serde_json::from_str(&chunks_raw).map_err(|e| e.to_string())?; + let mut chunks = Vec::with_capacity(chunk_values.len()); + for v in &chunk_values { + chunks.push(chunk_from_dict(v).map_err(|e| e.to_string())?); + } + Ok(chunks) +} + /// Shallow-clone `url` into `dir`, non-interactively. Rejects a ref starting /// with `-` (git-flag injection, CWE-88). fn clone_shallow(url: &str, dir: &Path, git_ref: Option<&str>) -> Result<(), String> { @@ -393,18 +428,6 @@ fn clone_shallow(url: &str, dir: &Path, git_ref: Option<&str>) -> Result<(), Str Ok(()) } -/// Deterministic sha256 (hex) of the serialized chunks JSON. -fn hash_chunks(chunks_json: &str) -> String { - let mut hasher = Sha256::new(); - hasher.update(chunks_json.as_bytes()); - let digest = hasher.finalize(); - let mut out = String::with_capacity(digest.len() * 2); - for byte in digest { - let _ = write!(out, "{byte:02x}"); - } - out -} - /// Parse and validate a persisted manifest (an on-disk trust boundary). pub fn parse_manifest(raw: &serde_json::Value) -> Result { let obj = raw.as_object().ok_or("Invalid manifest: not an object")?; @@ -459,6 +482,8 @@ pub fn parse_manifest(raw: &serde_json::Value) -> Result content.push(parsed); } + let files = parse_file_manifest(obj.get("files"))?; + Ok(IndexManifest { schema_version: u32::try_from(schema_version) .map_err(|_| "Invalid manifest: schemaVersion out of range")?, @@ -468,9 +493,48 @@ pub fn parse_manifest(raw: &serde_json::Value) -> Result model_id, model_kind, chunk_size, + files, }) } +/// Parse the per-file manifest. Absent/null → empty (a manifest without one +/// cannot seed an incremental rebuild); malformed → error. +fn parse_file_manifest(raw: Option<&serde_json::Value>) -> Result { + let Some(raw) = raw.filter(|v| !v.is_null()) else { + return Ok(FileManifest::new()); + }; + let obj = raw + .as_object() + .ok_or("Invalid manifest: files must be an object")?; + let mut files = FileManifest::new(); + for (indexed_path, entry) in obj { + let entry_obj = entry + .as_object() + .ok_or("Invalid manifest: files entries must be objects")?; + let hash = entry_obj + .get("hash") + .and_then(serde_json::Value::as_str) + .ok_or("Invalid manifest: files entry hash must be a string")? + .to_string(); + let range = |key: &str| -> Result { + entry_obj + .get(key) + .and_then(serde_json::Value::as_u64) + .and_then(|n| usize::try_from(n).ok()) + .ok_or_else(|| format!("Invalid manifest: files entry {key} must be a usize")) + }; + files.insert( + indexed_path.clone(), + FileManifestEntry { + hash, + start: range("start")?, + count: range("count")?, + }, + ); + } + Ok(files) +} + pub use crate::indexing::cache_orchestrator::{ load_or_build_index, source_fingerprint, LoadOrBuildOptions, }; diff --git a/crates/csp/src/indexing/index/tests.rs b/crates/csp/src/indexing/index/tests.rs index de9577a..38be70f 100644 --- a/crates/csp/src/indexing/index/tests.rs +++ b/crates/csp/src/indexing/index/tests.rs @@ -35,6 +35,7 @@ fn build_index(chunks: Vec) -> CspIndex { model_path: "test-model".to_string(), root: None, content: DEFAULT_CONTENT.to_vec(), + files: Default::default(), }) } @@ -188,7 +189,7 @@ fn save_writes_manifest_fields() { idx.save(dir.path(), None).unwrap(); let raw = std::fs::read_to_string(dir.path().join("manifest.json")).unwrap(); let value: serde_json::Value = serde_json::from_str(&raw).unwrap(); - assert_eq!(value["schemaVersion"], 1); + assert_eq!(value["schemaVersion"], 2); assert_eq!(value["modelId"], "test-model"); assert_eq!(value["content"], serde_json::json!(["code"])); assert!(value["contentHash"].as_str().unwrap().len() == 64); @@ -196,6 +197,39 @@ fn save_writes_manifest_fields() { value["chunkSize"].as_u64(), Some(u64::from(DESIRED_CHUNK_LENGTH_CHARS as u32)) ); + assert!(value["files"].is_object()); +} + +#[test] +fn save_load_roundtrip_preserves_file_manifest() { + let src = tempdir().unwrap(); + std::fs::write(src.path().join("a.ts"), "export const alpha = 1\n").unwrap(); + let idx = CspIndex::from_path(src.path(), &LoadOptions::default()).unwrap(); + assert!(idx.files.contains_key("a.ts")); + + let dir = tempdir().unwrap(); + idx.save(dir.path(), None).unwrap(); + let loaded = CspIndex::load_from_disk(dir.path()).unwrap(); + assert_eq!(loaded.files, idx.files); + assert_eq!(loaded.bm25_index.doc_order(), idx.bm25_index.doc_order()); +} + +#[test] +fn load_rejects_inconsistent_component_counts() { + let idx = build_index(vec![ + make_chunk("a.ts", 1, 10, Some("typescript"), "A"), + make_chunk("b.ts", 1, 5, Some("python"), "B"), + ]); + let dir = tempdir().unwrap(); + idx.save(dir.path(), None).unwrap(); + let chunks_path = dir.path().join("chunks.json"); + let mut chunks: Vec = + serde_json::from_str(&std::fs::read_to_string(&chunks_path).unwrap()).unwrap(); + chunks.pop(); + std::fs::write(&chunks_path, serde_json::to_string(&chunks).unwrap()).unwrap(); + + let err = CspIndex::load_from_disk(dir.path()).unwrap_err(); + assert!(err.contains("inconsistent document counts")); } #[test] diff --git a/crates/csp/src/indexing/mod.rs b/crates/csp/src/indexing/mod.rs index 771e6a4..cb2e766 100644 --- a/crates/csp/src/indexing/mod.rs +++ b/crates/csp/src/indexing/mod.rs @@ -12,3 +12,4 @@ pub mod file_walker; pub mod files; pub mod index; pub mod sparse; +pub mod types; diff --git a/crates/csp/src/indexing/sparse.rs b/crates/csp/src/indexing/sparse.rs index c4fea8d..09dfefa 100644 --- a/crates/csp/src/indexing/sparse.rs +++ b/crates/csp/src/indexing/sparse.rs @@ -1,19 +1,21 @@ -//! Minimal BM25 index + BM25 enrichment. Port of `src/indexing/sparse.ts` -//! (← semble `index/sparse.py`, standing in for Python's `bm25s`). +//! BM25 index + BM25 enrichment. Port of semble `index/bm25.py` (the own +//! incremental `BM25` class that replaced `bm25s` in upstream #225) plus +//! `index/sparse.py` (`enrich_for_bm25`). //! -//! Phase 1 covered the pure scoring core: `enrich_for_bm25`, `selector_to_mask`, -//! and `Bm25Index::{build, get_scores}`. Phase 3 (T014) adds on-disk -//! `save`/`load` to a `bm25.json` file whose shape matches the TS serialization -//! exactly (camelCase keys, `[[term, postings]]` entry arrays), so a Rust-written -//! index is byte-compatible with — and loadable by — the TS implementation. +//! `Bm25Index` keys documents on a stable chunk id (`"{path}:{slot}"`, see +//! `indexing::types::make_chunk_id`) and supports `add_document` / +//! `remove_document` so an incremental reindex can replace one file's postings +//! without rebuilding the corpus. `set_doc_order` fixes the global chunk-list +//! order that `get_scores` output is aligned to. Persistence (`bm25.json`) +//! stores the per-document term counts plus that order; postings are rebuilt +//! on load. //! -//! Float parity: the upstream stores scores in a `Float32Array`, so each -//! additive accumulation is rounded to `f32`. We reproduce that exactly — -//! `score = ((score as f64) + contrib) as f32` — and iterate unique query terms -//! in first-appearance order (JS `Set` insertion order), since `f32` -//! accumulation is order-sensitive. +//! Float parity: scores accumulate in `f32` (upstream uses a `float32` numpy +//! array), so each addition is rounded to `f32` and unique query terms are +//! visited in first-appearance order, since `f32` accumulation is +//! order-sensitive. -use std::collections::{HashMap, HashSet}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::path::Path; use serde::{Deserialize, Serialize}; @@ -68,90 +70,165 @@ pub fn selector_to_mask(selector: Option<&[u32]>, size: usize) -> Option }) } -/// Minimal in-memory BM25 index supporting `build` and `get_scores`. +/// One indexed document: its id, term counts, and token length. +#[derive(Debug, Clone)] +struct Doc { + chunk_id: String, + /// term → term frequency, in first-appearance order. + terms: Vec<(String, u32)>, + length: usize, +} + +/// Incremental in-memory BM25 index keyed on stable chunk ids. /// /// Documents are passed pre-tokenized (callers use -/// `tokenize(&enrich_for_bm25(chunk))`). `get_scores` returns per-document -/// scores in document order, matching `bm25s.BM25.get_scores`. -#[derive(Debug, Clone, Serialize, Deserialize)] +/// `tokenize(&enrich_for_bm25(chunk))`). `get_scores` returns one score per +/// entry of the current document order (see [`set_doc_order`](Self::set_doc_order)). +#[derive(Debug, Clone, Default)] pub struct Bm25Index { - num_docs: usize, - /// Token count per document, in document order. - doc_lengths: Vec, - avg_doc_length: f64, - /// term -> postings list of `(doc_id, term_freq)`. - postings: HashMap>, - /// term -> document frequency. - doc_freq: HashMap, + /// chunk id → internal slot. + ids: HashMap, + /// slot → document (`None` after removal; slots are recycled). + docs: Vec>, + free_slots: Vec, + /// term → (slot → term frequency). + postings: HashMap>, + total_doc_length: usize, + /// Global chunk-list order that `get_scores` output is aligned to. + doc_order: Vec, + /// slot → position in `doc_order` (`None` when not in the current order). + order_positions: Vec>, } impl Bm25Index { - /// Build an index from pre-tokenized documents. + /// Create an empty index. + pub fn new() -> Self { + Self::default() + } + + /// Build an index from pre-tokenized documents with positional ids + /// (`"0"`, `"1"`, …) and a matching document order. Convenience for callers + /// that do not track per-file chunk ids (tests, fixtures). pub fn build(documents: &[Vec]) -> Self { - let num_docs = documents.len(); - let mut doc_lengths = vec![0f32; num_docs]; - let mut postings: HashMap> = HashMap::new(); - let mut doc_freq: HashMap = HashMap::new(); - - let mut total_len = 0usize; - for (doc_id, tokens) in documents.iter().enumerate() { - doc_lengths[doc_id] = tokens.len() as f32; - total_len += tokens.len(); - - // Term frequencies for this document, in first-appearance order so - // the postings list order matches the upstream `Map` iteration. - let mut tf_order: Vec = Vec::new(); - let mut tf: HashMap<&str, u32> = HashMap::new(); - for token in tokens { - let entry = tf.entry(token.as_str()).or_insert(0); - if *entry == 0 { - tf_order.push(token.clone()); + let mut index = Self::new(); + let mut order = Vec::with_capacity(documents.len()); + for (i, tokens) in documents.iter().enumerate() { + let chunk_id = i.to_string(); + index + .add_document(&chunk_id, tokens) + .expect("positional ids are unique"); + order.push(chunk_id); + } + index.set_doc_order(order); + index + } + + /// Index one document, rejecting a duplicate chunk id. + pub fn add_document(&mut self, chunk_id: &str, tokens: &[String]) -> Result<(), String> { + if self.ids.contains_key(chunk_id) { + return Err(format!("chunk_id already indexed: {chunk_id}")); + } + // Term frequencies in first-appearance order. + let mut terms: Vec<(String, u32)> = Vec::new(); + let mut positions: HashMap<&str, usize> = HashMap::new(); + for token in tokens { + match positions.get(token.as_str()) { + Some(&i) => terms[i].1 += 1, + None => { + positions.insert(token.as_str(), terms.len()); + terms.push((token.clone(), 1)); } - *entry += 1; } + } - for term in tf_order { - let freq = tf[term.as_str()]; - postings - .entry(term.clone()) - .or_default() - .push((doc_id, freq)); - *doc_freq.entry(term).or_insert(0) += 1; + let slot = match self.free_slots.pop() { + Some(slot) => slot, + None => { + self.docs.push(None); + self.order_positions.push(None); + (self.docs.len() - 1) as u32 } + }; + for (term, freq) in &terms { + self.postings + .entry(term.clone()) + .or_default() + .insert(slot, *freq); } + self.total_doc_length += tokens.len(); + self.ids.insert(chunk_id.to_string(), slot); + self.order_positions[slot as usize] = None; + self.docs[slot as usize] = Some(Doc { + chunk_id: chunk_id.to_string(), + terms, + length: tokens.len(), + }); + Ok(()) + } - let avg_doc_length = if num_docs > 0 { - total_len as f64 / num_docs as f64 - } else { - 0.0 + /// Remove a document's postings; no-op when `chunk_id` is not indexed. + pub fn remove_document(&mut self, chunk_id: &str) { + let Some(slot) = self.ids.remove(chunk_id) else { + return; }; + let Some(doc) = self.docs[slot as usize].take() else { + return; + }; + self.total_doc_length -= doc.length; + for (term, _) in &doc.terms { + if let Some(docs) = self.postings.get_mut(term) { + docs.remove(&slot); + if docs.is_empty() { + self.postings.remove(term); + } + } + } + self.order_positions[slot as usize] = None; + self.free_slots.push(slot); + } - Self { - num_docs, - doc_lengths, - avg_doc_length, - postings, - doc_freq, + /// Set the global chunk-list order that `get_scores` output is aligned to. + /// Ids not (or no longer) indexed simply score 0 at their position. + pub fn set_doc_order(&mut self, chunk_ids: Vec) { + for position in self.order_positions.iter_mut() { + *position = None; + } + for (i, chunk_id) in chunk_ids.iter().enumerate() { + if let Some(&slot) = self.ids.get(chunk_id) { + self.order_positions[slot as usize] = Some(i); + } } + self.doc_order = chunk_ids; + } + + /// The current document order (aligned with `get_scores` output). + pub fn doc_order(&self) -> &[String] { + &self.doc_order } - /// Number of indexed documents. + /// Number of entries in the document order — the length of `get_scores`. pub fn num_docs(&self) -> usize { - self.num_docs + self.doc_order.len() + } + + /// Number of indexed documents (the BM25 corpus size). + pub fn corpus_size(&self) -> usize { + self.ids.len() } - /// Compute BM25 scores for the query tokens, in document order. + /// Compute BM25 scores for the query tokens, aligned with the document order. /// - /// When `weight_mask` is provided, documents with `mask[i] == 0` score 0 - /// (matching `bm25s.BM25.get_scores(..., weight_mask=mask)`). + /// When `weight_mask` is provided, positions with `mask[i] == 0` score 0 + /// (matching upstream `BM25.get_scores(..., weight_mask=mask)`). pub fn get_scores(&self, query_tokens: &[String], weight_mask: Option<&[u8]>) -> Vec { - let mut scores = vec![0f32; self.num_docs]; - if query_tokens.is_empty() || self.num_docs == 0 { + let mut scores = vec![0f32; self.doc_order.len()]; + let corpus_size = self.corpus_size(); + if query_tokens.is_empty() || corpus_size == 0 { return scores; } // De-duplicate query terms, preserving first-appearance order so the - // order-sensitive f32 accumulation matches the upstream `Set`. + // order-sensitive f32 accumulation is deterministic. let mut seen: HashSet<&str> = HashSet::new(); let mut unique: Vec<&str> = Vec::new(); for token in query_tokens { @@ -160,31 +237,33 @@ impl Bm25Index { } } + let avg = self.total_doc_length as f64 / corpus_size as f64; + let avg = if avg != 0.0 { avg } else { 1.0 }; for term in unique { - let Some(list) = self.postings.get(term) else { + let Some(docs) = self.postings.get(term) else { continue; }; - let df = self.doc_freq.get(term).copied().unwrap_or(0); + let df = docs.len() as f64; // Lucene/Robertson IDF: log(1 + (N - df + 0.5) / (df + 0.5)). - let idf = (1.0 + (self.num_docs as f64 - df as f64 + 0.5) / (df as f64 + 0.5)).ln(); + let idf = (1.0 + (corpus_size as f64 - df + 0.5) / (df + 0.5)).ln(); - for &(doc_id, freq) in list { + for (&slot, &freq) in docs { + let Some(position) = self.order_positions[slot as usize] else { + continue; + }; if let Some(mask) = weight_mask { - if mask.get(doc_id).copied().unwrap_or(0) == 0 { + if mask.get(position).copied().unwrap_or(0) == 0 { continue; } } - let dl = doc_lengths_get(&self.doc_lengths, doc_id); - let avg = if self.avg_doc_length != 0.0 { - self.avg_doc_length - } else { - 1.0 - }; + let dl = self.docs[slot as usize] + .as_ref() + .map_or(0.0, |doc| doc.length as f64); let denom = freq as f64 + K1 * (1.0 - B + (B * dl) / avg); let denom = if denom != 0.0 { denom } else { 1.0 }; let contrib = (idf * (freq as f64 * (K1 + 1.0))) / denom; - // Float32 accumulation (mirrors the Float32Array store). - scores[doc_id] = ((scores[doc_id] as f64) + contrib) as f32; + // Float32 accumulation (mirrors the float32 score array). + scores[position] = ((scores[position] as f64) + contrib) as f32; } } @@ -194,61 +273,77 @@ impl Bm25Index { /// Persist the index to `dir/bm25.json`, creating `dir` if needed. pub fn save(&self, dir: &Path) -> std::io::Result<()> { std::fs::create_dir_all(dir)?; + let documents: BTreeMap<&str, BTreeMap<&str, u32>> = self + .docs + .iter() + .flatten() + .map(|doc| { + let counts = doc + .terms + .iter() + .map(|(term, freq)| (term.as_str(), *freq)) + .collect(); + (doc.chunk_id.as_str(), counts) + }) + .collect(); let serialized = Bm25Serialized { - version: 1, - num_docs: self.num_docs, - avg_doc_length: self.avg_doc_length, - doc_lengths: self.doc_lengths.clone(), - postings: self - .postings - .iter() - .map(|(term, list)| (term.clone(), list.clone())) - .collect(), - doc_freq: self - .doc_freq - .iter() - .map(|(term, df)| (term.clone(), *df)) - .collect(), + version: BM25_FORMAT_VERSION, + documents, + doc_order: self.doc_order.iter().map(String::as_str).collect(), }; let json = serde_json::to_string(&serialized) .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?; std::fs::write(dir.join("bm25.json"), json) } - /// Load an index previously persisted with [`save`](Self::save). + /// Load an index previously persisted with [`save`](Self::save), + /// reconstructing its postings. Errors when the persisted document order + /// does not describe exactly the persisted documents. pub fn load(dir: &Path) -> std::io::Result { let raw = std::fs::read_to_string(dir.join("bm25.json"))?; - let parsed: Bm25Serialized = serde_json::from_str(&raw) + let parsed: Bm25Serialized = serde_json::from_str(&raw) .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?; - Ok(Self { - num_docs: parsed.num_docs, - doc_lengths: parsed.doc_lengths, - avg_doc_length: parsed.avg_doc_length, - postings: parsed.postings.into_iter().collect(), - doc_freq: parsed.doc_freq.into_iter().collect(), - }) + let invalid = |msg: &str| std::io::Error::new(std::io::ErrorKind::InvalidData, msg); + if parsed.version != BM25_FORMAT_VERSION { + return Err(invalid(&format!( + "Unsupported BM25 format {}; expected {BM25_FORMAT_VERSION}", + parsed.version + ))); + } + let order_set: HashSet<&str> = parsed.doc_order.iter().map(String::as_str).collect(); + let document_set: HashSet<&str> = parsed.documents.keys().map(String::as_str).collect(); + if order_set.len() != parsed.doc_order.len() || order_set != document_set { + return Err(invalid("Persisted BM25 document state is inconsistent")); + } + + let mut index = Self::new(); + for (chunk_id, counts) in parsed.documents { + let mut tokens: Vec = Vec::new(); + for (term, freq) in counts { + tokens.extend(std::iter::repeat_n(term, freq as usize)); + } + index + .add_document(&chunk_id, &tokens) + .map_err(|e| invalid(&e))?; + } + index.set_doc_order(parsed.doc_order); + Ok(index) } } -/// On-disk representation of [`Bm25Index`]. The keys are camelCase and the -/// maps are serialized as `[[key, value], ...]` entry arrays to match the TS -/// `bm25.json` format exactly. +/// On-disk format version of `bm25.json`. v1 was the positional +/// (`numDocs`/`docLengths`/`postings`) layout that predates stable chunk ids. +const BM25_FORMAT_VERSION: u32 = 2; + +/// On-disk representation of [`Bm25Index`]: per-document term counts plus the +/// document order (postings are derived on load). Maps are `BTreeMap` so the +/// serialized bytes are deterministic. #[derive(Serialize, Deserialize)] -struct Bm25Serialized { +struct Bm25Serialized { version: u32, - #[serde(rename = "numDocs")] - num_docs: usize, - #[serde(rename = "avgDocLength")] - avg_doc_length: f64, - #[serde(rename = "docLengths")] - doc_lengths: Vec, - postings: Vec<(String, Vec<(usize, u32)>)>, - #[serde(rename = "docFreq")] - doc_freq: Vec<(String, u32)>, -} - -fn doc_lengths_get(doc_lengths: &[f32], doc_id: usize) -> f64 { - doc_lengths.get(doc_id).copied().unwrap_or(0.0) as f64 + documents: BTreeMap>, + #[serde(rename = "docOrder")] + doc_order: Vec, } #[cfg(test)] @@ -413,19 +508,17 @@ mod tests { } #[test] - fn save_writes_ts_compatible_json() { - let index = Bm25Index::build(&docs(&[&["hello"]])); + fn save_writes_documents_and_doc_order() { + let index = Bm25Index::build(&docs(&[&["hello", "hello", "world"]])); let dir = tempfile::tempdir().unwrap(); index.save(dir.path()).unwrap(); let raw = std::fs::read_to_string(dir.path().join("bm25.json")).unwrap(); let value: serde_json::Value = serde_json::from_str(&raw).unwrap(); - assert_eq!(value["version"], 1); - assert_eq!(value["numDocs"], 1); - assert!(value["avgDocLength"].is_number()); - assert!(value["docLengths"].is_array()); - assert!(value["postings"].is_array()); - assert!(value["docFreq"].is_array()); + assert_eq!(value["version"], 2); + assert_eq!(value["documents"]["0"]["hello"], 2); + assert_eq!(value["documents"]["0"]["world"], 1); + assert_eq!(value["docOrder"], serde_json::json!(["0"])); } #[test] @@ -433,4 +526,101 @@ mod tests { let dir = tempfile::tempdir().unwrap(); assert!(Bm25Index::load(dir.path()).is_err()); } + + // --- incremental API (mirrors upstream tests/index/test_bm25.py) --- + + fn build_ids(input: &[(&str, &[&str])]) -> Bm25Index { + let mut index = Bm25Index::new(); + for (chunk_id, tokens) in input { + index.add_document(chunk_id, &query(tokens)).unwrap(); + } + index.set_doc_order(input.iter().map(|(id, _)| id.to_string()).collect()); + index + } + + #[test] + fn scoring_matches_lucene_formula() { + let index = build_ids(&[ + ("a", &["authenticate", "token"]), + ("b", &["login", "password"]), + ]); + let scores = index.get_scores(&query(&["authenticate"]), None); + // idf = ln(1 + 1.5/1.5); tf term = 1·(k1+1) / (1 + k1·(1 − b + b·dl/avgdl)) = 2.5/2.5. + let expected = (1.0f64 + 1.5 / 1.5).ln() as f32; + assert!( + (scores[0] - expected).abs() < 1e-6, + "{} vs {expected}", + scores[0] + ); + assert_eq!(scores[1], 0.0); + } + + #[test] + fn removed_and_unordered_documents_stop_scoring() { + let mut index = build_ids(&[("a", &["authenticate"]), ("b", &["login"])]); + index.remove_document("missing"); + index.set_doc_order(vec!["b".to_string()]); + assert_eq!(index.get_scores(&query(&["authenticate"]), None), vec![0.0]); + + index.remove_document("a"); + index.set_doc_order(vec!["a".to_string(), "b".to_string()]); + assert_eq!( + index.get_scores(&query(&["authenticate"]), None), + vec![0.0, 0.0] + ); + assert_eq!(index.corpus_size(), 1); + assert_eq!(index.num_docs(), 2); + } + + #[test] + fn duplicate_add_document_errors() { + let mut index = build_ids(&[("a", &["x"])]); + let err = index.add_document("a", &query(&["y"])).unwrap_err(); + assert!(err.contains("already indexed")); + } + + #[test] + fn removed_slot_is_recycled_without_stale_postings() { + let mut index = build_ids(&[("a", &["alpha"]), ("b", &["beta"])]); + index.remove_document("a"); + index.add_document("c", &query(&["gamma"])).unwrap(); + index.set_doc_order(vec!["b".to_string(), "c".to_string()]); + assert_eq!(index.get_scores(&query(&["alpha"]), None), vec![0.0, 0.0]); + let gamma = index.get_scores(&query(&["gamma"]), None); + assert_eq!(gamma[0], 0.0); + assert!(gamma[1] > 0.0); + } + + #[test] + fn save_load_preserves_scores_and_doc_order() { + let index = build_ids(&[ + ("empty", &[]), + ("a", &["authenticate", "token"]), + ("b", &["login", "password"]), + ]); + let dir = tempfile::tempdir().unwrap(); + index.save(dir.path()).unwrap(); + + let loaded = Bm25Index::load(dir.path()).unwrap(); + assert_eq!(loaded.doc_order(), index.doc_order()); + assert_eq!( + loaded.get_scores(&query(&["authenticate"]), None), + index.get_scores(&query(&["authenticate"]), None) + ); + } + + #[test] + fn load_rejects_inconsistent_document_order() { + let index = build_ids(&[("a", &["authenticate"])]); + let dir = tempfile::tempdir().unwrap(); + index.save(dir.path()).unwrap(); + let path = dir.path().join("bm25.json"); + let mut value: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap(); + value["docOrder"] = serde_json::json!(["other"]); + std::fs::write(&path, value.to_string()).unwrap(); + + let err = Bm25Index::load(dir.path()).unwrap_err(); + assert!(err.to_string().contains("document state")); + } } diff --git a/crates/csp/src/indexing/types.rs b/crates/csp/src/indexing/types.rs new file mode 100644 index 0000000..87d1f57 --- /dev/null +++ b/crates/csp/src/indexing/types.rs @@ -0,0 +1,104 @@ +//! Incremental-reindex types. Port of semble `index/types.py` (upstream #225). +//! +//! Upstream keys the per-file manifest on `mtime_ns`; csp keys it on a +//! per-file content hash instead, matching the whole-tree content-hash oracle +//! `cache_orchestrator` already uses (see ADR-0005). + +use std::collections::BTreeMap; + +use serde::{Deserialize, Serialize}; + +use crate::indexing::sparse::Bm25Index; +use crate::types::Chunk; + +/// Stable BM25 document id for a file chunk: `"{indexed_path}:{slot}"`. +pub fn make_chunk_id(indexed_path: &str, slot: usize) -> String { + format!("{indexed_path}:{slot}") +} + +/// A file's content hash and its chunk range within the global chunk list. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct FileManifestEntry { + /// sha256 (hex) of the file bytes at index time. + pub hash: String, + /// Index of the file's first chunk in the global chunk list. + pub start: usize, + /// Number of chunks produced for the file (may be 0). + pub count: usize, +} + +impl FileManifestEntry { + /// Exclusive end of the chunk range. + pub fn end(&self) -> usize { + self.start + self.count + } +} + +/// Per-file manifest keyed by the indexed (display-root-relative) path. +pub type FileManifest = BTreeMap; + +/// A previously built index, loaded for reuse during incremental reindexing. +#[derive(Debug)] +pub struct PreviousIndex { + pub chunks: Vec, + /// L2-normalised rows, aligned with `chunks`. + pub vectors: Vec>, + pub files: FileManifest, + pub bm25_index: Bm25Index, +} + +impl PreviousIndex { + /// Assemble a `PreviousIndex`, verifying that chunks, vectors, the file + /// manifest, and the BM25 document order all describe the same layout + /// (mirrors the alignment checks in upstream `load_previous_for_incremental`). + pub fn try_new( + chunks: Vec, + vectors: Vec>, + files: FileManifest, + bm25_index: Bm25Index, + ) -> Result { + let chunk_count = chunks.len(); + if chunk_count != vectors.len() || chunk_count != bm25_index.doc_order().len() { + return Err("Persisted index components have inconsistent document counts".into()); + } + if files.is_empty() { + return Err("Persisted index has no file manifest".into()); + } + + // Entries must tile [0, chunk_count) exactly, in ascending `start` order. + let mut entries: Vec<(&String, &FileManifestEntry)> = files.iter().collect(); + entries.sort_by_key(|(_, entry)| entry.start); + let mut expected_ids: Vec = Vec::with_capacity(chunk_count); + let mut next_start = 0usize; + for (indexed_path, entry) in entries { + if entry.start != next_start || entry.end() > chunk_count { + return Err(format!( + "File manifest entry for {indexed_path} does not tile the chunk list" + )); + } + if chunks[entry.start..entry.end()] + .iter() + .any(|chunk| chunk.file_path != *indexed_path) + { + return Err(format!( + "Chunks in the range recorded for {indexed_path} belong to another file" + )); + } + expected_ids.extend((0..entry.count).map(|slot| make_chunk_id(indexed_path, slot))); + next_start = entry.end(); + } + if next_start != chunk_count { + return Err("File manifest does not cover every chunk".into()); + } + if bm25_index.doc_order() != expected_ids.as_slice() { + return Err("BM25 document order does not match the file manifest".into()); + } + + Ok(Self { + chunks, + vectors, + files, + bm25_index, + }) + } +} diff --git a/crates/csp/src/mcp.rs b/crates/csp/src/mcp.rs index 89eab2d..3cc7aef 100644 --- a/crates/csp/src/mcp.rs +++ b/crates/csp/src/mcp.rs @@ -325,6 +325,7 @@ mod tests { model_path: "test".to_string(), root: None, content: vec![ContentType::Code], + files: Default::default(), }) } @@ -345,6 +346,7 @@ mod tests { model_path: "test".to_string(), root: None, content: vec![ContentType::Code], + files: Default::default(), }) } From b40edf4dcf5f939768be91e846ff22d610e0decd Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Fri, 4 Sep 2026 19:56:10 +0900 Subject: [PATCH 2/8] fix(index): harden incremental reindex after review - `PreviousIndex::try_new`: sort manifest entries by `(start, count)` so a zero-chunk file that ties with the following file no longer fails the tiling check (which silently disabled incremental reuse for that tree). Regression test `zero_chunk_file_does_not_break_manifest_tiling`. - `Bm25Index::load`: rebuild postings from the persisted term counts via `insert_document` instead of materialising `freq` copies of every term; sum lengths in u64 and reject out-of-range counts. Drop the duplicate `Doc.chunk_id`. - `create_index_from_path`: embed all changed files' chunks in one batched pass (`dense::embed_chunk_refs`) so a cold build keeps the tokenizer's batch parallelism. - `FileManifestEntry::end()`: saturating add so a corrupt manifest fails the range checks instead of overflowing. - `load_previous_for_incremental`: reject a seed whose vector rows do not match the live model's dimension, falling back to a full rebuild. - `parse_manifest`: read `files` through the `FileManifestEntry` serde derive that `save` writes with. - Docs: query-term de-duplication is a real ranking divergence from upstream's query-frequency weighting, not rank-neutral; record it as an open parity gap in ADR-0005 and `semble.md`. Refs #84 --- ...-file-hash-manifest-incremental-reindex.md | 7 +- .please/docs/references/semble.md | 13 +++- crates/csp/src/indexing/cache_orchestrator.rs | 8 +++ crates/csp/src/indexing/create.rs | 69 +++++++++++++++---- crates/csp/src/indexing/dense.rs | 6 ++ crates/csp/src/indexing/index.rs | 37 ++-------- crates/csp/src/indexing/sparse.rs | 49 ++++++++----- crates/csp/src/indexing/types.rs | 12 +++- 8 files changed, 129 insertions(+), 72 deletions(-) diff --git a/.please/docs/decisions/0005-per-file-hash-manifest-incremental-reindex.md b/.please/docs/decisions/0005-per-file-hash-manifest-incremental-reindex.md index 3933e66..142cea5 100644 --- a/.please/docs/decisions/0005-per-file-hash-manifest-incremental-reindex.md +++ b/.please/docs/decisions/0005-per-file-hash-manifest-incremental-reindex.md @@ -68,8 +68,11 @@ hash** instead of `mtime_ns`: - Existing v1 caches are rebuilt once (schema bump), then benefit from incremental reuse. - The BM25 **scoring** is intentionally unchanged from the previous csp implementation (Lucene IDF with the `(k1+1)` numerator, de-duplicated query terms). Upstream's own class dropped the - `(k1+1)` factor and weights repeated query terms by their query frequency; both differences - scale scores without changing ranks, and ranks are all that Reciprocal Rank Fusion consumes. + `(k1+1)` factor and weights repeated query terms by their query frequency. Only the first is + rank-neutral: `(k1+1)` is a global constant, but the query-frequency weight varies per term, + so for a query with a repeated token the two implementations can order documents differently. + That is a pre-existing parity gap, out of scope here and tracked in + `.please/docs/references/semble.md` §6.1 item 10. Recorded as an intentional adaptation in `.please/docs/references/semble.md` §6.1. - Git sources (`from_git`) are URL+ref keyed and never take the incremental path, matching upstream (which only seeds `from_path`). diff --git a/.please/docs/references/semble.md b/.please/docs/references/semble.md index 829abb3..56b6641 100644 --- a/.please/docs/references/semble.md +++ b/.please/docs/references/semble.md @@ -387,9 +387,16 @@ Clean two-layer split: `mtime_ns` per file; csp records the sha256 of the file bytes so the per-file decision uses the same oracle as the whole-tree `contentHash` fast path (ADR-0002). 10. **BM25 scoring unchanged** — csp keeps the Lucene `(k1+1)` numerator and de-duplicates - query terms; upstream's own `BM25` (#225) dropped `(k1+1)` and multiplies by the query term - frequency. Both differ by a per-term scale factor only, so ranks — the only thing RRF - consumes — are identical. + query terms; upstream's own `BM25` (#225) dropped `(k1+1)` and multiplies each term's + contribution by the query term frequency. The `(k1+1)` factor is a single global constant, + so dropping it is rank-neutral. The query-tf factor is **not** — it reweights terms against + one another whenever a query tokenises to a repeated token, which the identifier-aware + tokenizer makes common (`getUserById getUser` repeats `get` and `user`). Ranks can + therefore differ from upstream: for query tokens `[a, a, b]` with per-term contributions + `a → 1.0` (doc1) and `b → 1.8` (doc2), upstream scores doc1 `2.0` > doc2 `1.8` while csp + scores doc1 `1.0` < doc2 `1.8`. This is a **live parity gap**, not a rank-neutral + adaptation, and it predates #225; it is tracked here rather than fixed in the incremental + port. ### 6.2 Open stubs & gaps (verify before claiming runtime parity) diff --git a/crates/csp/src/indexing/cache_orchestrator.rs b/crates/csp/src/indexing/cache_orchestrator.rs index 79114e0..f097c3b 100644 --- a/crates/csp/src/indexing/cache_orchestrator.rs +++ b/crates/csp/src/indexing/cache_orchestrator.rs @@ -164,6 +164,14 @@ pub(crate) fn load_previous_for_incremental( let chunks = read_chunks(cache_dir).ok()?; let vectors = SelectableBasicBackend::load(cache_dir).ok()?.vectors; + // The persisted rows are concatenated with freshly embedded ones, so a model + // whose dimension changed under an unchanged id would make the merged matrix + // ragged — and `from_normalized` would then hard-error out of the rebuild + // instead of falling back to it. Fail closed here instead. + let (query_model, _) = crate::indexing::dense::load_model(Some(expected_model)); + if vectors.iter().any(|row| row.len() != query_model.dim()) { + return None; + } let bm25_index = Bm25Index::load(cache_dir).ok()?; PreviousIndex::try_new(chunks, vectors, manifest.files, bm25_index).ok() } diff --git a/crates/csp/src/indexing/create.rs b/crates/csp/src/indexing/create.rs index 0362b50..e8db76d 100644 --- a/crates/csp/src/indexing/create.rs +++ b/crates/csp/src/indexing/create.rs @@ -13,7 +13,7 @@ use std::path::{Path, PathBuf}; use crate::chunking::source::chunk_source; use crate::indexing::cache::sha256_hex; -use crate::indexing::dense::{embed_chunks, Model, SelectableBasicBackend}; +use crate::indexing::dense::{embed_chunk_refs, Model, SelectableBasicBackend}; use crate::indexing::file_walker::walk_files; use crate::indexing::files::{detect_language, get_extensions}; use crate::indexing::sparse::{enrich_for_bm25, Bm25Index}; @@ -101,7 +101,11 @@ pub fn create_index_from_path( let mut chunks: Vec = Vec::new(); let mut chunk_ids: Vec = Vec::new(); - let mut vectors: Vec> = Vec::new(); + // Reused rows land here directly; freshly chunked files leave a `None` hole + // that the single batched embed pass below fills, so the tokenizer keeps the + // whole-corpus batching it had before incremental reuse existed. + let mut vectors: Vec>> = Vec::new(); + let mut fresh_rows: Vec = Vec::new(); let mut files = FileManifest::new(); for file_path in walk_files(path, &ext_refs, &[]) { @@ -147,30 +151,27 @@ pub fn create_index_from_path( } _ => None, }; - let (file_chunks, file_vectors) = match reused { - Some(reused) => reused, + let start = chunks.len(); + let file_chunks = match reused { + Some((file_chunks, file_vectors)) => { + vectors.extend(file_vectors.into_iter().map(Some)); + file_chunks + } None => { // Lossy UTF-8 decode (invalid bytes → U+FFFD): only an IO error // skips a file, never an encoding error. let source = String::from_utf8_lossy(&bytes).into_owned(); let file_chunks = chunk_source(&source, &indexed_path, language); reindex_file(&mut bm25_index, &indexed_path, &file_chunks, previous_entry)?; - // Normalise through the backend so fresh rows match the reused - // (already-normalised) rows. - let file_vectors = SelectableBasicBackend::from_vectors(embed_chunks( - options.model, - &file_chunks, - ))? - .vectors; - (file_chunks, file_vectors) + fresh_rows.extend(start..start + file_chunks.len()); + vectors.extend(std::iter::repeat_n(None, file_chunks.len())); + file_chunks } }; - let start = chunks.len(); let count = file_chunks.len(); chunk_ids.extend((0..count).map(|slot| make_chunk_id(&indexed_path, slot))); chunks.extend(file_chunks); - vectors.extend(file_vectors); files.insert(indexed_path, FileManifestEntry { hash, start, count }); } @@ -188,6 +189,23 @@ pub fn create_index_from_path( )); } + // One batched embed for every changed file's chunks — the tokenizer + // parallelises per batch, so a call per file would serialise a cold build. + // Normalise through the backend so fresh rows match the reused + // (already-normalised) rows. + let fresh_chunks: Vec<&Chunk> = fresh_rows.iter().map(|&i| &chunks[i]).collect(); + let fresh_vectors = + SelectableBasicBackend::from_vectors(embed_chunk_refs(options.model, &fresh_chunks))? + .vectors; + if fresh_vectors.len() != fresh_rows.len() { + return Err("Embedder returned the wrong number of rows".to_string()); + } + for (&row, vector) in fresh_rows.iter().zip(fresh_vectors) { + vectors[row] = Some(vector); + } + let vectors: Option>> = vectors.into_iter().collect(); + let vectors = vectors.ok_or("Internal error: an embedding row was left unfilled")?; + bm25_index.set_doc_order(chunk_ids); let semantic_index = SelectableBasicBackend::from_normalized(vectors)?; @@ -429,4 +447,27 @@ mod tests { // The rebuilt index is itself a valid seed for the next incremental pass. into_previous(after); } + + #[test] + fn zero_chunk_file_does_not_break_manifest_tiling() { + let dir = tempdir().unwrap(); + let root = dir.path().to_path_buf(); + // `pkg/` is walked before `pkg.ts` (directory entries sort by file name), + // but "pkg.ts" < "pkg/z.ts" lexicographically ('.' 0x2E < '/' 0x2F). The + // empty file yields no chunks, so both entries share `start`. + write_files( + &root, + &[ + ("pkg/z.ts", " \n"), + ("pkg.ts", "function stable_anchor() { return 1 }\n"), + ], + ); + let model = make_stub_model(4); + let result = + create_index_from_path(&root, &opts(&model, Some(root.clone())), None).unwrap(); + assert_eq!(result.files["pkg/z.ts"].count, 0); + assert_eq!(result.files["pkg/z.ts"].start, result.files["pkg.ts"].start); + // A freshly built index must always be a valid seed for the next pass. + into_previous(result); + } } diff --git a/crates/csp/src/indexing/dense.rs b/crates/csp/src/indexing/dense.rs index c574822..00e6467 100644 --- a/crates/csp/src/indexing/dense.rs +++ b/crates/csp/src/indexing/dense.rs @@ -184,6 +184,12 @@ fn load_model_with( /// Embed chunks with the model — one row per chunk, `[]` for empty input. pub fn embed_chunks(model: &Model, chunks: &[Chunk]) -> Vec> { + embed_chunk_refs(model, &chunks.iter().collect::>()) +} + +/// [`embed_chunks`] over borrowed chunks, so a caller embedding a subset of a +/// chunk list does not have to clone the chunks to gather them. +pub fn embed_chunk_refs(model: &Model, chunks: &[&Chunk]) -> Vec> { if chunks.is_empty() { return Vec::new(); } diff --git a/crates/csp/src/indexing/index.rs b/crates/csp/src/indexing/index.rs index b406fc4..f4e36f9 100644 --- a/crates/csp/src/indexing/index.rs +++ b/crates/csp/src/indexing/index.rs @@ -12,7 +12,7 @@ use crate::indexing::cache::sha256_hex; use crate::indexing::create::{create_index_from_path, CreateIndexOptions}; use crate::indexing::dense::{load_model, make_stub_model, Model, SelectableBasicBackend}; use crate::indexing::sparse::Bm25Index; -use crate::indexing::types::{FileManifest, FileManifestEntry, PreviousIndex}; +use crate::indexing::types::{FileManifest, PreviousIndex}; use crate::search::{search as run_search, SearchOptions as RunSearchOptions, SearchResult}; use crate::types::{chunk_from_dict, chunk_to_dict, Chunk, ChunkDict, ContentType, IndexStats}; @@ -498,41 +498,14 @@ pub fn parse_manifest(raw: &serde_json::Value) -> Result } /// Parse the per-file manifest. Absent/null → empty (a manifest without one -/// cannot seed an incremental rebuild); malformed → error. +/// cannot seed an incremental rebuild); malformed → error. Deserialised through +/// `FileManifest`'s derive, the same one `save` serialises with, so the two +/// halves of the on-disk format cannot drift. fn parse_file_manifest(raw: Option<&serde_json::Value>) -> Result { let Some(raw) = raw.filter(|v| !v.is_null()) else { return Ok(FileManifest::new()); }; - let obj = raw - .as_object() - .ok_or("Invalid manifest: files must be an object")?; - let mut files = FileManifest::new(); - for (indexed_path, entry) in obj { - let entry_obj = entry - .as_object() - .ok_or("Invalid manifest: files entries must be objects")?; - let hash = entry_obj - .get("hash") - .and_then(serde_json::Value::as_str) - .ok_or("Invalid manifest: files entry hash must be a string")? - .to_string(); - let range = |key: &str| -> Result { - entry_obj - .get(key) - .and_then(serde_json::Value::as_u64) - .and_then(|n| usize::try_from(n).ok()) - .ok_or_else(|| format!("Invalid manifest: files entry {key} must be a usize")) - }; - files.insert( - indexed_path.clone(), - FileManifestEntry { - hash, - start: range("start")?, - count: range("count")?, - }, - ); - } - Ok(files) + serde_json::from_value(raw.clone()).map_err(|e| format!("Invalid manifest: files: {e}")) } pub use crate::indexing::cache_orchestrator::{ diff --git a/crates/csp/src/indexing/sparse.rs b/crates/csp/src/indexing/sparse.rs index 09dfefa..97d3681 100644 --- a/crates/csp/src/indexing/sparse.rs +++ b/crates/csp/src/indexing/sparse.rs @@ -70,10 +70,10 @@ pub fn selector_to_mask(selector: Option<&[u32]>, size: usize) -> Option }) } -/// One indexed document: its id, term counts, and token length. +/// One indexed document: its term counts and token length. The document's +/// chunk id lives once, as the `Bm25Index::ids` key. #[derive(Debug, Clone)] struct Doc { - chunk_id: String, /// term → term frequency, in first-appearance order. terms: Vec<(String, u32)>, length: usize, @@ -125,9 +125,6 @@ impl Bm25Index { /// Index one document, rejecting a duplicate chunk id. pub fn add_document(&mut self, chunk_id: &str, tokens: &[String]) -> Result<(), String> { - if self.ids.contains_key(chunk_id) { - return Err(format!("chunk_id already indexed: {chunk_id}")); - } // Term frequencies in first-appearance order. let mut terms: Vec<(String, u32)> = Vec::new(); let mut positions: HashMap<&str, usize> = HashMap::new(); @@ -140,6 +137,20 @@ impl Bm25Index { } } } + self.insert_document(chunk_id, terms, tokens.len()) + } + + /// Index one document from its term counts directly — the shape `bm25.json` + /// persists, so `load` never has to materialise a token stream. + fn insert_document( + &mut self, + chunk_id: &str, + terms: Vec<(String, u32)>, + length: usize, + ) -> Result<(), String> { + if self.ids.contains_key(chunk_id) { + return Err(format!("chunk_id already indexed: {chunk_id}")); + } let slot = match self.free_slots.pop() { Some(slot) => slot, @@ -155,14 +166,10 @@ impl Bm25Index { .or_default() .insert(slot, *freq); } - self.total_doc_length += tokens.len(); + self.total_doc_length += length; self.ids.insert(chunk_id.to_string(), slot); self.order_positions[slot as usize] = None; - self.docs[slot as usize] = Some(Doc { - chunk_id: chunk_id.to_string(), - terms, - length: tokens.len(), - }); + self.docs[slot as usize] = Some(Doc { terms, length }); Ok(()) } @@ -274,16 +281,16 @@ impl Bm25Index { pub fn save(&self, dir: &Path) -> std::io::Result<()> { std::fs::create_dir_all(dir)?; let documents: BTreeMap<&str, BTreeMap<&str, u32>> = self - .docs + .ids .iter() - .flatten() - .map(|doc| { + .filter_map(|(chunk_id, &slot)| { + let doc = self.docs[slot as usize].as_ref()?; let counts = doc .terms .iter() .map(|(term, freq)| (term.as_str(), *freq)) .collect(); - (doc.chunk_id.as_str(), counts) + Some((chunk_id.as_str(), counts)) }) .collect(); let serialized = Bm25Serialized { @@ -318,12 +325,18 @@ impl Bm25Index { let mut index = Self::new(); for (chunk_id, counts) in parsed.documents { - let mut tokens: Vec = Vec::new(); + // Sum in u64: `length` is derived from untrusted on-disk counts, and + // an overflowing document must be rejected, not silently wrapped. + let mut length = 0u64; + let mut terms: Vec<(String, u32)> = Vec::with_capacity(counts.len()); for (term, freq) in counts { - tokens.extend(std::iter::repeat_n(term, freq as usize)); + length += u64::from(freq); + terms.push((term, freq)); } + let length = usize::try_from(length) + .map_err(|_| invalid("Persisted BM25 document length is out of range"))?; index - .add_document(&chunk_id, &tokens) + .insert_document(&chunk_id, terms, length) .map_err(|e| invalid(&e))?; } index.set_doc_order(parsed.doc_order); diff --git a/crates/csp/src/indexing/types.rs b/crates/csp/src/indexing/types.rs index 87d1f57..a8e72e2 100644 --- a/crates/csp/src/indexing/types.rs +++ b/crates/csp/src/indexing/types.rs @@ -28,9 +28,11 @@ pub struct FileManifestEntry { } impl FileManifestEntry { - /// Exclusive end of the chunk range. + /// Exclusive end of the chunk range. Saturating: a corrupt on-disk manifest + /// must fail the range checks, not overflow (and then panic on a reversed + /// slice range in a release build). pub fn end(&self) -> usize { - self.start + self.count + self.start.saturating_add(self.count) } } @@ -66,8 +68,12 @@ impl PreviousIndex { } // Entries must tile [0, chunk_count) exactly, in ascending `start` order. + // A file that produced no chunks shares its `start` with the next file + // that did, so ties are broken by `count` to place the empty entries + // first — `files` is path-keyed and path order is not walk order + // (`pkg.ts` sorts before `pkg/z.ts`, but `pkg/` is walked first). let mut entries: Vec<(&String, &FileManifestEntry)> = files.iter().collect(); - entries.sort_by_key(|(_, entry)| entry.start); + entries.sort_by_key(|(_, entry)| (entry.start, entry.count)); let mut expected_ids: Vec = Vec::with_capacity(chunk_count); let mut next_start = 0usize; for (indexed_path, entry) in entries { From 862db725494354869885bb74d0825c606852d312 Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Fri, 4 Sep 2026 21:21:13 +0900 Subject: [PATCH 3/8] fix(index): skip files whose lossy display path collides with an indexed file On Unix, file names that differ only in invalid UTF-8 bytes collapse to the same `to_string_lossy` path. The BM25 chunk ids derived from that path would then collide and abort the whole build with "chunk_id already indexed". Keep the first such file, skip the rest with a warning, and add a Linux-only regression test (APFS rejects non-UTF-8 names). Refs #84 --- crates/csp/src/indexing/create.rs | 39 +++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/crates/csp/src/indexing/create.rs b/crates/csp/src/indexing/create.rs index e8db76d..ee22b39 100644 --- a/crates/csp/src/indexing/create.rs +++ b/crates/csp/src/indexing/create.rs @@ -129,6 +129,19 @@ pub fn create_index_from_path( .into_owned(), None => file_path.to_string_lossy().into_owned(), }; + // `to_string_lossy` is not injective: on Unix, file names that differ + // only in invalid UTF-8 bytes collapse to the same display path, and the + // BM25 chunk ids derived from it would then collide and abort the whole + // build. Keep the first such file and skip the rest, as a valid UTF-8 + // tree can never hit this. + if files.contains_key(&indexed_path) { + eprintln!( + "csp: skipping {}: its display path collides with an already indexed file \ + (non-UTF-8 file name)", + file_path.display() + ); + continue; + } let previous_entry = previous_files.get(&indexed_path); // Unchanged file: move its previous chunk + vector rows out (each row is @@ -470,4 +483,30 @@ mod tests { // A freshly built index must always be a valid seed for the next pass. into_previous(result); } + + /// Non-UTF-8 file names only exist on Unix filesystems that allow them + /// (APFS rejects them), so this runs on Linux only. + #[cfg(target_os = "linux")] + #[test] + fn colliding_lossy_paths_skip_the_duplicate_instead_of_aborting() { + use std::os::unix::ffi::OsStrExt; + let dir = tempdir().unwrap(); + let root = dir.path().to_path_buf(); + let first = root.join(std::ffi::OsStr::from_bytes(b"a\xff.ts")); + let second = root.join(std::ffi::OsStr::from_bytes(b"a\xfe.ts")); + std::fs::write(&first, "function first_file() { return 1 }\n").unwrap(); + std::fs::write(&second, "function second_file() { return 2 }\n").unwrap(); + assert_eq!( + first.to_string_lossy(), + second.to_string_lossy(), + "test precondition: both names must collapse to one display path" + ); + + let model = make_stub_model(4); + let result = + create_index_from_path(&root, &opts(&model, Some(root.clone())), None).unwrap(); + assert_eq!(result.files.len(), 1); + assert_eq!(result.bm25_index.corpus_size(), result.chunks.len()); + into_previous(result); + } } From de6ff29a391b6572f8dcff421bf466343c9cd9c0 Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Fri, 4 Sep 2026 21:23:35 +0900 Subject: [PATCH 4/8] chore: merge origin/main (#80 max_snippet_lines, #82 savings telemetry) into incremental reindexing --- README.ko.md | 8 + README.md | 8 + crates/csp/src/bin/csp/main.rs | 203 +++++++++++++++++++------ crates/csp/src/bin/csp/mcp_server.rs | 48 +++++- crates/csp/src/chunking/core.rs | 5 +- crates/csp/src/indexing/index.rs | 95 +++++++++++- crates/csp/src/indexing/index/tests.rs | 58 +++++++ crates/csp/src/mcp.rs | 131 +++++++++++++++- crates/csp/src/stats.rs | 85 ++++++++++- crates/csp/src/utils.rs | 136 ++++++++++++++--- 10 files changed, 694 insertions(+), 83 deletions(-) diff --git a/README.ko.md b/README.ko.md index 1421e46..2fc7437 100644 --- a/README.ko.md +++ b/README.ko.md @@ -130,6 +130,12 @@ csp search "database host port" ./my-project --content config csp search "authentication" ./my-project --content all ``` +`--max-snippet-lines N`으로 결과당 반환되는 코드를 제한할 수 있습니다. `10`은 시그니처+본문 앞부분 미리보기, `0`은 파일 경로와 라인 범위만 반환하며, 기본값은 전체 청크입니다. 파일을 열기 전 위치를 확인하는 데는 짧은 미리보기로 충분한 경우가 많아, 약간의 맥락을 토큰 절감과 맞바꿉니다. + +```bash +csp search "authentication" ./my-project --max-snippet-lines 10 +``` + `csp find-related`로 기존 위치와 비슷한 코드를 찾을 수 있습니다 (이전 검색 결과의 `file_path`와 `line`을 사용). ```bash @@ -349,6 +355,8 @@ args = [ | `search` | 자연어 또는 코드 쿼리로 코드베이스를 검색. `repo`는 로컬 디렉터리 경로 또는 https:// git URL. | | `find_related` | 파일 경로와 라인 번호를 받아, 해당 위치의 코드와 의미적으로 유사한 청크를 반환. | +두 도구 모두 결과당 반환 코드를 제한하는 `max_snippet_lines`를 받습니다. 기본값은 `10`으로, 시그니처+본문 앞부분 미리보기라 에이전트가 위치를 싸게 확인한 뒤 전체 맥락은 파일로 이동해 봅니다. 위치만 필요하면 `0`, 미리보기로 부족하면 `null`로 전체 청크를 받습니다. + 기본적으로 MCP 서버는 코드 파일만 인덱싱합니다. 문서/설정/전체를 함께 인덱싱하려면 명령에 `--content docs`, `--content config`, `--content all` 또는 조합(예: `--content code docs`)을 추가하세요. 예를 들어 Claude Code에서는 `claude mcp add csp -s user -- bunx @pleaseai/csp mcp --content all`. ## 서브 에이전트 설정 diff --git a/README.md b/README.md index d90607c..0dfbd70 100644 --- a/README.md +++ b/README.md @@ -130,6 +130,12 @@ csp search "database host port" ./my-project --content config csp search "authentication" ./my-project --content all ``` +Use `--max-snippet-lines N` to cap the code returned per result: `10` gives a signature-plus-first-lines preview, `0` returns only the file path and line range. The default returns the full chunk. A short preview is often enough to confirm a location before you open the file, so it trades a little context for fewer tokens. + +```bash +csp search "authentication" ./my-project --max-snippet-lines 10 +``` + Use `csp find-related` to discover code similar to a known location (pass `file_path` and `line` from a prior search result): ```bash @@ -349,6 +355,8 @@ Add to `~/.config/zed/settings.json` (or `.zed/settings.json` in your project): | `search` | Search a codebase with a natural-language or code query. Pass `repo` as a local directory path or an https:// git URL. | | `find_related` | Given a file path and line number, return chunks semantically similar to the code at that location. | +Both tools accept `max_snippet_lines` to cap the code returned per result. It defaults to `10` — a signature-plus-first-lines preview that lets an agent confirm a location cheaply, then navigate to the file for full context. Pass `0` for the location only, or `null` for the full chunk when the preview lacks context. + By default the MCP server indexes only code files. To also index documentation, config, or everything, append `--content docs`, `--content config`, or `--content all` to the server command, or a combination, e.g. `--content code docs`. For example, in Claude Code: `claude mcp add csp -s user -- bunx @pleaseai/csp mcp --content all`. ## Sub-agent setup diff --git a/crates/csp/src/bin/csp/main.rs b/crates/csp/src/bin/csp/main.rs index 50a1ede..1934708 100644 --- a/crates/csp/src/bin/csp/main.rs +++ b/crates/csp/src/bin/csp/main.rs @@ -14,9 +14,11 @@ use csp::indexing::cache::{clear_index_cache, CacheLocation}; use csp::indexing::index::{ load_or_build_index, CspIndex, LoadOptions, LoadOrBuildOptions, QueryOptions, }; -use csp::stats::{clear_savings, default_stats_file, format_savings_report, now_secs}; -use csp::types::ContentType; -use csp::utils::{format_results, is_git_url, resolve_chunk}; +use csp::stats::{ + clear_savings, default_stats_file, format_savings_report, now_secs, save_search_stats, +}; +use csp::types::{CallType, ContentType}; +use csp::utils::{format_results, is_git_url, resolve_chunk, resolve_snippet_lines}; #[derive(Parser)] #[command(name = "csp", version, about = "Instant local code search for agents")] @@ -56,6 +58,10 @@ enum Command { path: Option, #[arg(long = "top-k", short = 'k')] top_k: Option, + /// Lines of source per result (default: full chunk). 10 = signature + + /// body, 0 = no code. + #[arg(long = "max-snippet-lines", value_name = "N")] + max_snippet_lines: Option, #[arg(long, value_enum, num_args = 1..)] content: Vec, /// Path to a pre-built index (bypasses the auto-cache). @@ -73,6 +79,10 @@ enum Command { path: Option, #[arg(long = "top-k", short = 'k')] top_k: Option, + /// Lines of source per result (default: full chunk). 10 = signature + + /// body, 0 = no code. + #[arg(long = "max-snippet-lines", value_name = "N")] + max_snippet_lines: Option, #[arg(long, value_enum, num_args = 1..)] content: Vec, #[arg(long)] @@ -210,8 +220,15 @@ fn load_index( } } -/// JSON output for `search` (pure — testable without stdout capture). -fn search_output(index: &CspIndex, query: &str, top_k: usize) -> String { +/// JSON output for `search`. `stats_file` records token-savings telemetry when +/// `Some`; tests pass `None` to stay off the real `~/.csp` file. +fn search_output( + index: &CspIndex, + query: &str, + top_k: usize, + max_snippet_lines: Option, + stats_file: Option<&Path>, +) -> String { let results = index.search( query, &QueryOptions { @@ -219,10 +236,19 @@ fn search_output(index: &CspIndex, query: &str, top_k: usize) -> String { ..Default::default() }, ); + if let Some(stats_file) = stats_file { + save_search_stats( + stats_file, + &results, + CallType::Search, + &index.file_sizes, + max_snippet_lines, + ); + } let out = if results.is_empty() { serde_json::json!({ "error": "No results found." }) } else { - format_results(query, &results) + format_results(query, &results, max_snippet_lines) }; out.to_string() } @@ -233,6 +259,8 @@ fn find_related_output( file: &str, line: &str, top_k: usize, + max_snippet_lines: Option, + stats_file: Option<&Path>, ) -> Result { let Ok(line_num) = line.parse::() else { return Err(format!("line must be an integer, got: {line}")); @@ -254,10 +282,23 @@ fn find_related_output( ..Default::default() }, ); + if let Some(stats_file) = stats_file { + save_search_stats( + stats_file, + &related, + CallType::FindRelated, + &index.file_sizes, + max_snippet_lines, + ); + } let out = if related.is_empty() { serde_json::json!({ "error": format!("No related chunks found for {file}:{line_num}.") }) } else { - format_results(&format!("Chunks related to {file}:{line_num}"), &related) + format_results( + &format!("Chunks related to {file}:{line_num}"), + &related, + max_snippet_lines, + ) }; Ok(out.to_string()) } @@ -332,6 +373,13 @@ fn run() -> ExitCode { /// `ExitCode` — so the dispatch logic is unit-testable without going through /// `Cli::parse` (which reads argv) or an opaque, non-comparable `ExitCode`. fn dispatch(command: Command) -> u8 { + dispatch_with_stats(command, &default_stats_file()) +} + +/// [`dispatch`] with the token-savings file injected so tests can redirect the +/// telemetry that `search` / `find-related` append (keeping the real `~/.csp` +/// untouched). +fn dispatch_with_stats(command: Command, stats_file: &Path) -> u8 { match command { Command::Init { agent, force } => { let agent = agent.unwrap_or(Agent::Claude); @@ -382,6 +430,7 @@ fn dispatch(command: Command) -> u8 { query, path, top_k, + max_snippet_lines, content, index, git_ref, @@ -394,7 +443,16 @@ fn dispatch(command: Command) -> u8 { git_ref, ) { Ok(idx) => { - println!("{}", search_output(&idx, &query, top_k.unwrap_or(5))); + println!( + "{}", + search_output( + &idx, + &query, + top_k.unwrap_or(5), + resolve_snippet_lines(max_snippet_lines), + Some(stats_file), + ) + ); EXIT_SUCCESS } Err(e) => { @@ -408,6 +466,7 @@ fn dispatch(command: Command) -> u8 { line, path, top_k, + max_snippet_lines, content, index, git_ref, @@ -425,7 +484,14 @@ fn dispatch(command: Command) -> u8 { return EXIT_FAILURE; } }; - match find_related_output(&idx, &file, &line, top_k.unwrap_or(5)) { + match find_related_output( + &idx, + &file, + &line, + top_k.unwrap_or(5), + resolve_snippet_lines(max_snippet_lines), + Some(stats_file), + ) { Ok(out) => { println!("{out}"); EXIT_SUCCESS @@ -519,24 +585,56 @@ mod tests { fn search_output_shapes_results() { let dir = build_index_dir(); let idx = CspIndex::from_path(dir.path(), &LoadOptions::default()).unwrap(); - let out = search_output(&idx, "greet", 5); + let out = search_output(&idx, "greet", 5, None, None); let value: serde_json::Value = serde_json::from_str(&out).unwrap(); assert!(value.get("results").is_some() || value.get("error").is_some()); if let Some(results) = value.get("results").and_then(|r| r.as_array()) { if let Some(first) = results.first() { - let chunk = &first["chunk"]; - assert!(chunk.get("file_path").is_some()); - assert!(chunk.get("start_line").is_some()); - assert!(chunk.get("location").is_some()); + // Flat wire shape (semble#198): fields at the top level. + assert!(first.get("chunk").is_none()); + assert!(first.get("file_path").is_some()); + assert!(first.get("start_line").is_some()); + // CLI default (None) keeps the full content. + assert!(first.get("content").is_some()); } } } + #[test] + fn search_output_caps_snippet_lines() { + let dir = build_index_dir(); + let idx = CspIndex::from_path(dir.path(), &LoadOptions::default()).unwrap(); + let out = search_output(&idx, "greet", 5, Some(0), None); + let value: serde_json::Value = serde_json::from_str(&out).unwrap(); + if let Some(first) = value["results"].as_array().and_then(|r| r.first()) { + assert!(first.get("content").is_none()); + assert!(first.get("file_path").is_some()); + } + } + + #[test] + fn search_output_records_savings_when_stats_file_given() { + let dir = build_index_dir(); + let idx = CspIndex::from_path(dir.path(), &LoadOptions::default()).unwrap(); + // file_sizes is captured at build time from the source tree. + assert!(!idx.file_sizes.is_empty()); + + let stats = tempdir().unwrap(); + let stats_file = stats.path().join("savings.jsonl"); + let _ = search_output(&idx, "greet", 5, None, Some(&stats_file)); + + let content = std::fs::read_to_string(&stats_file).unwrap(); + let lines: Vec<&str> = content.lines().filter(|l| !l.is_empty()).collect(); + assert_eq!(lines.len(), 1); + assert!(lines[0].contains("\"call\":\"search\"")); + assert!(lines[0].contains("file_chars")); + } + #[test] fn find_related_rejects_non_integer_line() { let dir = build_index_dir(); let idx = CspIndex::from_path(dir.path(), &LoadOptions::default()).unwrap(); - let err = find_related_output(&idx, "sample.ts", "abc", 5).unwrap_err(); + let err = find_related_output(&idx, "sample.ts", "abc", 5, None, None).unwrap_err(); assert!(err.contains("line must be an integer")); } @@ -544,7 +642,7 @@ mod tests { fn find_related_no_chunk_at_location() { let dir = build_index_dir(); let idx = CspIndex::from_path(dir.path(), &LoadOptions::default()).unwrap(); - let err = find_related_output(&idx, "nope.ts", "1", 5).unwrap_err(); + let err = find_related_output(&idx, "nope.ts", "1", 5, None, None).unwrap_err(); assert!(err.contains("No chunk found")); } @@ -600,46 +698,64 @@ mod tests { #[test] fn dispatch_index_then_search_and_find_related() { // Keep everything on an explicit --index path so the test never writes - // to the global ~/.csp auto-cache. + // to the global ~/.csp auto-cache, and redirect savings telemetry to a + // temp file so it never touches the real ~/.csp/savings.jsonl. let src = build_index_dir(); let out = tempdir().unwrap(); + let stats_file = out.path().join("savings.jsonl"); assert_eq!(index_to(out.path(), src.path()), EXIT_SUCCESS); let idx_path = out.path().to_string_lossy().into_owned(); - let search = dispatch(Command::Search { - query: "greet".to_string(), - path: None, - top_k: Some(5), - content: vec![], - index: Some(idx_path.clone()), - git_ref: None, - }); + let search = dispatch_with_stats( + Command::Search { + query: "greet".to_string(), + path: None, + top_k: Some(5), + max_snippet_lines: None, + content: vec![], + index: Some(idx_path.clone()), + git_ref: None, + }, + &stats_file, + ); assert_eq!(search, EXIT_SUCCESS); // sample.ts:1 has an indexable chunk → find-related succeeds. - let related = dispatch(Command::FindRelated { - file: "sample.ts".to_string(), - line: "1".to_string(), - path: None, - top_k: Some(5), - content: vec![], - index: Some(idx_path.clone()), - git_ref: None, - }); + let related = dispatch_with_stats( + Command::FindRelated { + file: "sample.ts".to_string(), + line: "1".to_string(), + path: None, + top_k: Some(5), + max_snippet_lines: None, + content: vec![], + index: Some(idx_path.clone()), + git_ref: None, + }, + &stats_file, + ); assert_eq!(related, EXIT_SUCCESS); // A non-integer line is a caller error → failure exit. - let bad = dispatch(Command::FindRelated { - file: "sample.ts".to_string(), - line: "abc".to_string(), - path: None, - top_k: Some(5), - content: vec![], - index: Some(idx_path), - git_ref: None, - }); + let bad = dispatch_with_stats( + Command::FindRelated { + file: "sample.ts".to_string(), + line: "abc".to_string(), + path: None, + top_k: Some(5), + max_snippet_lines: None, + content: vec![], + index: Some(idx_path), + git_ref: None, + }, + &stats_file, + ); assert_eq!(bad, EXIT_FAILURE); + + // The two successful calls appended one savings record each. + let recorded = std::fs::read_to_string(&stats_file).unwrap(); + assert_eq!(recorded.lines().filter(|l| !l.is_empty()).count(), 2); } #[test] @@ -650,6 +766,7 @@ mod tests { query: "greet".to_string(), path: None, top_k: Some(1), + max_snippet_lines: None, content: vec![], index: Some( missing diff --git a/crates/csp/src/bin/csp/mcp_server.rs b/crates/csp/src/bin/csp/mcp_server.rs index d6d7a31..44d8c61 100644 --- a/crates/csp/src/bin/csp/mcp_server.rs +++ b/crates/csp/src/bin/csp/mcp_server.rs @@ -16,7 +16,15 @@ use rmcp::{tool, tool_handler, tool_router, ErrorData as McpError, ServerHandler use tokio::sync::Mutex; use csp::mcp::{find_related_tool, search_tool, IndexCache, SERVER_INSTRUCTIONS}; +use csp::stats::default_stats_file; use csp::types::ContentType; +use csp::utils::resolve_snippet_lines; + +/// MCP default: signature + first body lines, enough to confirm a location +/// while spending far fewer tokens than the full chunk (semble#198). +fn default_max_snippet_lines() -> Option { + Some(10) +} /// Parameters for the `search` tool (mirrors the TS MCP tool's args). #[derive(Debug, serde::Deserialize, schemars::JsonSchema)] @@ -28,6 +36,11 @@ pub struct SearchParams { pub repo: Option, /// Maximum number of results (default 5). pub top_k: Option, + /// Lines of source per result. Default 10 = signature + first body lines, + /// enough to confirm the location. 0 = file path and line range only. Pass + /// `null` for the full chunk when the snippet lacks context. + #[serde(default = "default_max_snippet_lines")] + pub max_snippet_lines: Option, } /// Parameters for the `find_related` tool. @@ -41,6 +54,10 @@ pub struct FindRelatedParams { pub repo: Option, /// Maximum number of results (default 5). pub top_k: Option, + /// Lines of source per result. Default 10 = signature + first body lines. + /// 0 = location only. Pass `null` for the full chunk. + #[serde(default = "default_max_snippet_lines")] + pub max_snippet_lines: Option, } /// MCP server holding the session index cache and the default source. @@ -49,6 +66,9 @@ pub struct CspMcpServer { cache: Arc>, default_source: Option, default_ref: Option, + /// Where token-savings telemetry is appended; `None` disables recording + /// (used by tests so they don't touch the real `~/.csp/savings.jsonl`). + stats_file: Option, tool_router: ToolRouter, } @@ -63,6 +83,7 @@ impl CspMcpServer { cache: Arc::new(Mutex::new(IndexCache::new(content))), default_source, default_ref, + stats_file: Some(default_stats_file()), tool_router: Self::tool_router(), } } @@ -82,6 +103,8 @@ impl CspMcpServer { &p.query, p.repo.as_deref(), p.top_k.unwrap_or(5) as usize, + resolve_snippet_lines(p.max_snippet_lines), + self.stats_file.as_deref(), ); Ok(CallToolResult::success(vec![Content::text(out)])) } @@ -102,6 +125,8 @@ impl CspMcpServer { p.line, p.repo.as_deref(), p.top_k.unwrap_or(5) as usize, + resolve_snippet_lines(p.max_snippet_lines), + self.stats_file.as_deref(), ); Ok(CallToolResult::success(vec![Content::text(out)])) } @@ -152,15 +177,30 @@ mod tests { assert_eq!(minimal.query, "greet"); assert!(minimal.repo.is_none()); assert!(minimal.top_k.is_none()); + // Absent max_snippet_lines → the MCP default of 10. + assert_eq!(minimal.max_snippet_lines, Some(10)); let full: SearchParams = serde_json::from_value(serde_json::json!({ "query": "greet", "repo": "./x", - "top_k": 3 + "top_k": 3, + "max_snippet_lines": 0 })) .unwrap(); assert_eq!(full.repo.as_deref(), Some("./x")); assert_eq!(full.top_k, Some(3)); + assert_eq!(full.max_snippet_lines, Some(0)); + + // Explicit null → None (full chunk), distinct from the absent default. + let nulled: SearchParams = serde_json::from_value(serde_json::json!({ + "query": "greet", + "max_snippet_lines": null + })) + .unwrap(); + assert!(nulled.max_snippet_lines.is_none()); + assert!(resolve_snippet_lines(nulled.max_snippet_lines).is_none()); + assert_eq!(resolve_snippet_lines(Some(3)), Some(3)); + assert_eq!(resolve_snippet_lines(Some(-4)), Some(0)); } #[test] @@ -198,16 +238,19 @@ mod tests { #[tokio::test] async fn search_tool_call_returns_json_payload() { let dir = sample_source(); - let server = CspMcpServer::new( + let mut server = CspMcpServer::new( Some(dir.path().to_string_lossy().into_owned()), None, vec![ContentType::Code], ); + // Don't append telemetry to the developer's real ~/.csp during tests. + server.stats_file = None; let result = server .search(Parameters(SearchParams { query: "greet".to_string(), repo: None, top_k: Some(5), + max_snippet_lines: None, })) .await .unwrap(); @@ -235,6 +278,7 @@ mod tests { line: 1, repo: None, top_k: Some(5), + max_snippet_lines: None, })) .await .unwrap(); diff --git a/crates/csp/src/chunking/core.rs b/crates/csp/src/chunking/core.rs index 7b46133..267e6e0 100644 --- a/crates/csp/src/chunking/core.rs +++ b/crates/csp/src/chunking/core.rs @@ -209,8 +209,9 @@ pub fn merge_node(node: &N, desired_length: usize) -> Vec Vec<&str> { +/// and bare `\r`. Shared with `utils::result_to_dict` so a snippet cap counts +/// lines exactly the way chunk `start_line`/`end_line` do. +pub fn split_lines_keep_ends(text: &str) -> Vec<&str> { if text.is_empty() { return Vec::new(); } diff --git a/crates/csp/src/indexing/index.rs b/crates/csp/src/indexing/index.rs index f4e36f9..e604b8c 100644 --- a/crates/csp/src/indexing/index.rs +++ b/crates/csp/src/indexing/index.rs @@ -1,7 +1,7 @@ //! `CspIndex` — the hybrid (dense + BM25) search orchestrator. Port of semble //! `index/index.py`. -use std::collections::{BTreeMap, HashSet}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::path::Path; use std::process::Command; @@ -90,6 +90,11 @@ pub struct CspIndex { pub content: Vec, /// Per-file content hash + chunk range, used for incremental reindexing. pub files: FileManifest, + /// Per-file character counts (repo-relative path → UTF-16 length) captured + /// at build time from the source tree, for token-savings telemetry. Empty + /// when the source files aren't available (e.g. a git index loaded from + /// cache). Derived metadata, not part of [`CspIndexState`]. + pub file_sizes: HashMap, } pub(crate) fn normalize_content(content: Option>) -> Vec { @@ -107,6 +112,7 @@ impl CspIndex { root: state.root, content: state.content, files: state.files, + file_sizes: HashMap::new(), } } @@ -144,16 +150,26 @@ impl CspIndex { previous, )?; - Ok(Self::new(CspIndexState { + let mut index = Self::new(CspIndexState { model, bm25_index: result.bm25_index, semantic_index: result.semantic_index, chunks: result.chunks, model_path, - root: Some(path.to_string_lossy().into_owned()), + // Absolute, like upstream's `path.resolve()`, so an index built from + // `.` still finds its source tree when loaded from another cwd. + root: Some( + std::path::absolute(path) + .unwrap_or_else(|_| path.to_path_buf()) + .to_string_lossy() + .into_owned(), + ), content, files: result.files, - })) + }); + // Capture file sizes now, while the source tree is on disk. + index.file_sizes = compute_file_sizes(path, &index.chunks); + Ok(index) } /// Build an index from a remote git URL (shallow clone into a temp dir). @@ -174,9 +190,12 @@ impl CspIndex { clone_shallow(url, dir.path(), git_ref)?; let index = Self::from_path(dir.path(), options)?; + // `from_path` already captured file sizes from the checkout; carry them + // over since the temp dir is removed when `dir` drops. + let file_sizes = index.file_sizes.clone(); // Re-root at the URL so a persisted manifest records a stable sourceId // (the temp checkout is removed when `dir` drops). - Ok(Self::new(CspIndexState { + let mut rerooted = Self::new(CspIndexState { model: index.model, bm25_index: index.bm25_index, semantic_index: index.semantic_index, @@ -185,7 +204,9 @@ impl CspIndex { root: Some(url.to_string()), content: index.content, files: index.files, - })) + }); + rerooted.file_sizes = file_sizes; + Ok(rerooted) } /// Aggregate index statistics. @@ -370,7 +391,7 @@ impl CspIndex { make_stub_model(semantic_index.dim) }; - Ok(Self::new(CspIndexState { + let mut index = Self::new(CspIndexState { model, bm25_index, semantic_index, @@ -379,8 +400,66 @@ impl CspIndex { root: manifest.source_id, content: manifest.content, files: manifest.files, - })) + }); + // Recompute file sizes from the source when it's a still-present local + // directory (mirrors semble reading sizes off `root` on load). A git URL + // or a moved source leaves this empty → `file_chars` is simply 0. + if let Some(root) = index.root.as_deref() { + let root_path = Path::new(root); + if root_path.is_dir() { + index.file_sizes = compute_file_sizes(root_path, &index.chunks); + } + } + Ok(index) + } +} + +/// Per-file UTF-16 character counts for the unique files referenced by `chunks`, +/// read from `root`. Mirrors semble `_compute_file_sizes` (unreadable files are +/// skipped). Feeds the `file_chars` side of token-savings telemetry; UTF-16 keeps +/// it consistent with `stats::save_search_stats`'s snippet accounting. +/// +/// Chunk paths are repo-relative by construction; a path that is absolute or +/// escapes `root` via `..` can only come from a tampered on-disk index, so it is +/// skipped rather than resolved (path traversal guard — a deliberate addition +/// over upstream, which joins the path unchecked). Only regular files are read: +/// the file walker never follows symlinks, and a path that has since become a +/// symlink, FIFO, or device must not be able to redirect or stall the read. +fn compute_file_sizes(root: &Path, chunks: &[Chunk]) -> HashMap { + let mut sizes: HashMap = HashMap::new(); + for chunk in chunks { + if sizes.contains_key(&chunk.file_path) { + continue; + } + let rel = Path::new(&chunk.file_path); + if !is_safe_relative_path(rel) { + continue; + } + let full = root.join(rel); + let is_regular_file = std::fs::symlink_metadata(&full) + .map(|m| m.is_file()) + .unwrap_or(false); + if !is_regular_file { + continue; + } + if let Ok(text) = std::fs::read_to_string(&full) { + sizes.insert(chunk.file_path.clone(), text.encode_utf16().count() as u64); + } } + sizes +} + +/// `true` when `path` is relative and contains no `..` or root component, so +/// joining it onto an index root cannot resolve outside that root. +fn is_safe_relative_path(path: &Path) -> bool { + use std::path::Component; + !path.is_absolute() + && !path.components().any(|c| { + matches!( + c, + Component::ParentDir | Component::RootDir | Component::Prefix(_) + ) + }) } /// Read and validate `/chunks.json`. diff --git a/crates/csp/src/indexing/index/tests.rs b/crates/csp/src/indexing/index/tests.rs index 38be70f..5b1d3f1 100644 --- a/crates/csp/src/indexing/index/tests.rs +++ b/crates/csp/src/indexing/index/tests.rs @@ -275,6 +275,20 @@ fn from_path_builds_index() { assert_eq!(idx.content, DEFAULT_CONTENT.to_vec()); } +#[test] +fn from_path_stores_absolute_root_for_relative_path() { + // Built from a cwd-relative path; `root` must still come out absolute so a + // reload from another cwd can find the source tree (upstream `path.resolve()`). + let dir = tempfile::Builder::new().tempdir_in(".").unwrap(); + std::fs::write(dir.path().join("sample.ts"), "export const x = 1\n").unwrap(); + let relative = Path::new(dir.path().file_name().unwrap()); + assert!(relative.is_relative()); + let idx = CspIndex::from_path(relative, &LoadOptions::default()).unwrap(); + let root = idx.root.as_deref().unwrap(); + assert!(Path::new(root).is_absolute(), "root was {root}"); + assert!(root.ends_with(relative.to_str().unwrap())); +} + // --- from_git --- #[test] @@ -331,3 +345,47 @@ fn from_git_clones_and_builds() { assert!(!idx.chunks.is_empty()); assert_eq!(idx.root.as_deref(), Some(url.as_str())); } + +#[test] +fn compute_file_sizes_skips_paths_that_escape_root() { + let outer = tempdir().unwrap(); + let root = outer.path().join("repo"); + std::fs::create_dir(&root).unwrap(); + std::fs::write(root.join("inside.ts"), "abc").unwrap(); + std::fs::write(outer.path().join("secret.txt"), "top secret").unwrap(); + let abs = root.join("inside.ts").to_string_lossy().into_owned(); + + let chunks = vec![ + make_chunk("inside.ts", 1, 1, None, "abc"), + make_chunk("../secret.txt", 1, 1, None, "x"), + make_chunk(&abs, 1, 1, None, "x"), + ]; + let sizes = compute_file_sizes(&root, &chunks); + + assert_eq!(sizes.get("inside.ts"), Some(&3)); + assert!(!sizes.contains_key("../secret.txt")); + assert!(!sizes.contains_key(abs.as_str())); +} + +#[cfg(unix)] +#[test] +fn compute_file_sizes_skips_symlinks_and_non_regular_files() { + let outer = tempdir().unwrap(); + let root = outer.path().join("repo"); + std::fs::create_dir(&root).unwrap(); + std::fs::write(root.join("real.ts"), "abcd").unwrap(); + std::fs::write(outer.path().join("secret.txt"), "top secret").unwrap(); + std::os::unix::fs::symlink(outer.path().join("secret.txt"), root.join("link.ts")).unwrap(); + std::fs::create_dir(root.join("dir.ts")).unwrap(); + + let chunks = vec![ + make_chunk("real.ts", 1, 1, None, "abcd"), + make_chunk("link.ts", 1, 1, None, "x"), + make_chunk("dir.ts", 1, 1, None, "x"), + ]; + let sizes = compute_file_sizes(&root, &chunks); + + assert_eq!(sizes.get("real.ts"), Some(&4)); + assert!(!sizes.contains_key("link.ts")); + assert!(!sizes.contains_key("dir.ts")); +} diff --git a/crates/csp/src/mcp.rs b/crates/csp/src/mcp.rs index 3cc7aef..8e4a262 100644 --- a/crates/csp/src/mcp.rs +++ b/crates/csp/src/mcp.rs @@ -9,6 +9,7 @@ //! testable. [`IndexCache`] holds `Arc` so it can be shared across the //! async server's tokio tasks. +use std::path::Path; use std::sync::Arc; use std::time::{Duration, Instant}; @@ -18,7 +19,8 @@ use serde_json::json; use crate::indexing::index::{ load_or_build_index, source_fingerprint, CspIndex, LoadOrBuildOptions, QueryOptions, }; -use crate::types::ContentType; +use crate::stats::save_search_stats; +use crate::types::{CallType, ContentType}; use crate::utils::{format_results, is_git_url, resolve_chunk}; /// Server instructions advertised to MCP clients (preserved for the transport). @@ -239,6 +241,10 @@ pub fn get_index( /// `search` tool handler. Returns a JSON string (results or `{error}`), or an /// error message string on failure (mirroring the TS handler's catch). +/// `stats_file`, when `Some`, records token-savings telemetry (tests pass `None`). +// Positional transport params mirror the MCP tool signature; a struct would just +// move the plumbing without clarifying it (same call as `find_related_tool`). +#[allow(clippy::too_many_arguments)] pub fn search_tool( cache: &mut IndexCache, default_source: Option<&str>, @@ -246,6 +252,8 @@ pub fn search_tool( query: &str, repo: Option<&str>, top_k: usize, + max_snippet_lines: Option, + stats_file: Option<&Path>, ) -> String { let index = match get_index(repo, default_source, default_ref, cache) { Ok(idx) => idx, @@ -258,14 +266,26 @@ pub fn search_tool( ..Default::default() }, ); + if let Some(stats_file) = stats_file { + save_search_stats( + stats_file, + &results, + CallType::Search, + &index.file_sizes, + max_snippet_lines, + ); + } if results.is_empty() { json!({ "error": "No results found." }).to_string() } else { - format_results(query, &results).to_string() + format_results(query, &results, max_snippet_lines).to_string() } } /// `find_related` tool handler. +// Positional transport params mirror the MCP tool signature; a struct would just +// move the plumbing without clarifying it. +#[allow(clippy::too_many_arguments)] pub fn find_related_tool( cache: &mut IndexCache, default_source: Option<&str>, @@ -274,6 +294,8 @@ pub fn find_related_tool( line: i64, repo: Option<&str>, top_k: usize, + max_snippet_lines: Option, + stats_file: Option<&Path>, ) -> String { let index = match get_index(repo, default_source, default_ref, cache) { Ok(idx) => idx, @@ -299,10 +321,24 @@ pub fn find_related_tool( ..Default::default() }, ); + if let Some(stats_file) = stats_file { + save_search_stats( + stats_file, + &results, + CallType::FindRelated, + &index.file_sizes, + max_snippet_lines, + ); + } if results.is_empty() { json!({ "error": format!("No related chunks found for {file_path}:{line}.") }).to_string() } else { - format_results(&format!("Chunks related to {file_path}:{line}"), &results).to_string() + format_results( + &format!("Chunks related to {file_path}:{line}"), + &results, + max_snippet_lines, + ) + .to_string() } } @@ -525,7 +561,16 @@ mod tests { #[test] fn search_tool_no_results() { let mut cache = IndexCache::with_seam(vec![ContentType::Code], Stub::new()); - let out = search_tool(&mut cache, Some("/tmp/repo"), None, "anything", None, 5); + let out = search_tool( + &mut cache, + Some("/tmp/repo"), + None, + "anything", + None, + 5, + None, + None, + ); assert_eq!(out, json!({ "error": "No results found." }).to_string()); } @@ -548,23 +593,95 @@ mod tests { #[test] fn search_tool_returns_results_json() { let mut cache = IndexCache::with_seam(vec![ContentType::Code], OneChunkSeam); - let out = search_tool(&mut cache, Some("/tmp/repo"), None, "main", None, 5); + let out = search_tool( + &mut cache, + Some("/tmp/repo"), + None, + "main", + None, + 5, + None, + None, + ); let value: serde_json::Value = serde_json::from_str(&out).unwrap(); assert!(value.get("query").is_some()); assert!(value["results"].as_array().is_some()); + // Full content by default (max_snippet_lines = None). + assert!(value["results"][0].get("content").is_some()); + } + + #[test] + fn search_tool_records_savings_when_stats_file_given() { + let mut cache = IndexCache::with_seam(vec![ContentType::Code], OneChunkSeam); + let dir = tempfile::tempdir().unwrap(); + let stats_file = dir.path().join("savings.jsonl"); + let _ = search_tool( + &mut cache, + Some("/tmp/repo"), + None, + "main", + None, + 5, + None, + Some(&stats_file), + ); + let content = std::fs::read_to_string(&stats_file).unwrap(); + let lines: Vec<&str> = content.lines().filter(|l| !l.is_empty()).collect(); + assert_eq!(lines.len(), 1); + assert!(lines[0].contains("\"call\":\"search\"")); + } + + #[test] + fn search_tool_respects_max_snippet_lines_zero() { + let mut cache = IndexCache::with_seam(vec![ContentType::Code], OneChunkSeam); + let out = search_tool( + &mut cache, + Some("/tmp/repo"), + None, + "main", + None, + 5, + Some(0), + None, + ); + let value: serde_json::Value = serde_json::from_str(&out).unwrap(); + let entry = &value["results"][0]; + // 0 lines → no content, but the location metadata is still present. + assert!(entry.get("content").is_none()); + assert_eq!(entry["file_path"], "a.ts"); } #[test] fn find_related_no_chunk_message() { let mut cache = IndexCache::with_seam(vec![ContentType::Code], OneChunkSeam); - let out = find_related_tool(&mut cache, Some("/tmp/repo"), None, "nope.ts", 1, None, 5); + let out = find_related_tool( + &mut cache, + Some("/tmp/repo"), + None, + "nope.ts", + 1, + None, + 5, + None, + None, + ); assert!(out.contains("No chunk found at nope.ts:1")); } #[test] fn find_related_returns_json_for_known_chunk() { let mut cache = IndexCache::with_seam(vec![ContentType::Code], OneChunkSeam); - let out = find_related_tool(&mut cache, Some("/tmp/repo"), None, "a.ts", 5, None, 5); + let out = find_related_tool( + &mut cache, + Some("/tmp/repo"), + None, + "a.ts", + 5, + None, + 5, + None, + None, + ); // Either related results or the no-related error — both valid JSON. let value: serde_json::Value = serde_json::from_str(&out).unwrap(); assert!(value.get("query").is_some() || value.get("error").is_some()); diff --git a/crates/csp/src/stats.rs b/crates/csp/src/stats.rs index 6be0370..89d56ec 100644 --- a/crates/csp/src/stats.rs +++ b/crates/csp/src/stats.rs @@ -82,14 +82,41 @@ fn utf16_len(s: &str) -> u64 { s.encode_utf16().count() as u64 } +/// Characters actually delivered to the caller for one result, honoring the +/// `max_snippet_lines` cap (mirrors semble#206): `None` → the whole chunk, +/// `Some(0)` → nothing, `Some(n)` → the first `n` lines. +fn delivered_chars(content: &str, max_snippet_lines: Option) -> u64 { + match max_snippet_lines { + None => utf16_len(content), + Some(0) => 0, + Some(n) => { + // Sum of the first `n` lines plus one `\n` between each pair, without + // materializing the joined snippet. + let (len, count) = content + .lines() + .take(n) + .fold((0u64, 0u64), |(len, count), line| { + (len + utf16_len(line), count + 1) + }); + len + count.saturating_sub(1) + } + } +} + /// Append one telemetry record. Best-effort: any I/O error is swallowed. +/// `max_snippet_lines` matches the cap applied to the returned snippet so the +/// recorded `snippet_chars` reflects what the caller actually received. pub fn save_search_stats( stats_file: &Path, results: &[SearchResult], call_type: CallType, file_sizes: &HashMap, + max_snippet_lines: Option, ) { - let snippet_chars: u64 = results.iter().map(|r| utf16_len(&r.chunk.content)).sum(); + let snippet_chars: u64 = results + .iter() + .map(|r| delivered_chars(&r.chunk.content, max_snippet_lines)) + .sum(); let mut unique_paths: Vec<&str> = Vec::new(); for r in results { if !unique_paths.contains(&r.chunk.file_path.as_str()) { @@ -476,6 +503,7 @@ mod tests { &results, CallType::Search, &sizes(&[("a.ts", 100), ("b.ts", 200)]), + None, ); let content = std::fs::read_to_string(&file).unwrap(); @@ -488,12 +516,55 @@ mod tests { assert_eq!(record.file_chars, 300); } + #[test] + fn save_caps_snippet_chars_by_max_snippet_lines() { + let dir = tempdir().unwrap(); + let file = dir.path().join("savings.jsonl"); + let results = vec![result("line1\nline2\nline3", "a.ts")]; + // The first 2 lines "line1\nline2" are 11 UTF-16 units (semble#206). + save_search_stats( + &file, + &results, + CallType::Search, + &sizes(&[("a.ts", 100)]), + Some(2), + ); + let content = std::fs::read_to_string(&file).unwrap(); + let record: StatsRecord = serde_json::from_str(content.lines().next().unwrap()).unwrap(); + assert_eq!(record.snippet_chars, 11); + assert_eq!(record.file_chars, 100); + } + + #[test] + fn save_zero_snippet_lines_records_no_snippet_chars() { + let dir = tempdir().unwrap(); + let file = dir.path().join("savings.jsonl"); + let results = vec![result("anything at all", "a.ts")]; + save_search_stats( + &file, + &results, + CallType::Search, + &sizes(&[("a.ts", 100)]), + Some(0), + ); + let content = std::fs::read_to_string(&file).unwrap(); + let record: StatsRecord = serde_json::from_str(content.lines().next().unwrap()).unwrap(); + assert_eq!(record.snippet_chars, 0); + assert_eq!(record.file_chars, 100); + } + #[test] fn save_dedups_file_chars_per_path() { let dir = tempdir().unwrap(); let file = dir.path().join("savings.jsonl"); let results = vec![result("abc", "a.ts"), result("def", "a.ts")]; - save_search_stats(&file, &results, CallType::Search, &sizes(&[("a.ts", 100)])); + save_search_stats( + &file, + &results, + CallType::Search, + &sizes(&[("a.ts", 100)]), + None, + ); let content = std::fs::read_to_string(&file).unwrap(); let record: StatsRecord = serde_json::from_str(content.lines().next().unwrap()).unwrap(); assert_eq!(record.file_chars, 100); @@ -505,7 +576,13 @@ mod tests { let dir = tempdir().unwrap(); let file = dir.path().join("savings.jsonl"); let results = vec![result("x", "a.ts"), result("y", "missing.ts")]; - save_search_stats(&file, &results, CallType::Search, &sizes(&[("a.ts", 100)])); + save_search_stats( + &file, + &results, + CallType::Search, + &sizes(&[("a.ts", 100)]), + None, + ); let content = std::fs::read_to_string(&file).unwrap(); let record: StatsRecord = serde_json::from_str(content.lines().next().unwrap()).unwrap(); assert_eq!(record.file_chars, 100); @@ -520,12 +597,14 @@ mod tests { &[result("a", "a.ts")], CallType::Search, &sizes(&[("a.ts", 10)]), + None, ); save_search_stats( &file, &[result("b", "b.ts")], CallType::FindRelated, &sizes(&[("b.ts", 10)]), + None, ); let content = std::fs::read_to_string(&file).unwrap(); let lines: Vec<&str> = content.lines().filter(|l| !l.is_empty()).collect(); diff --git a/crates/csp/src/utils.rs b/crates/csp/src/utils.rs index 5d3ecbe..a32641a 100644 --- a/crates/csp/src/utils.rs +++ b/crates/csp/src/utils.rs @@ -2,33 +2,69 @@ use serde_json::{json, Value}; +use crate::chunking::core::split_lines_keep_ends; use crate::search::SearchResult; -use crate::types::{chunk_location, Chunk}; +use crate::types::Chunk; -/// Serialize a search result to the CLI/MCP wire dict — **snake_case** chunk -/// fields plus a derived `location` (matching the TS `SearchResult.toDict`, which -/// differs from the camelCase `ChunkDict` used for on-disk persistence). -pub fn result_to_dict(result: &SearchResult) -> Value { +/// Map a CLI/MCP `max_snippet_lines` wire value (`--max-snippet-lines`, or the +/// tool's `max_snippet_lines` argument) to the `Option` cap that +/// [`format_results`] takes. Absent (`None`) → full chunk content; a negative +/// value clamps to `0` (no code); a value above `usize::MAX` (32-bit targets) +/// saturates instead of wrapping. +pub fn resolve_snippet_lines(value: Option) -> Option { + value.map(|n| usize::try_from(n.max(0)).unwrap_or(usize::MAX)) +} + +/// Serialize a search result to the flat CLI/MCP wire dict — **snake_case** +/// fields (`file_path`, `start_line`, `end_line`, `score`, optional `content`), +/// matching semble `utils.format_results` after semble#198. +/// +/// `max_snippet_lines` caps the `content` field so agents can spend fewer tokens +/// confirming a location before navigating to the file: +/// - `None` → full chunk content +/// - `Some(0)` → omit `content` entirely (path + line range only) +/// - `Some(n)` → the first `n` lines of content +pub fn result_to_dict(result: &SearchResult, max_snippet_lines: Option) -> Value { let c = &result.chunk; - json!({ - "chunk": { - "content": c.content, - "file_path": c.file_path, - "start_line": c.start_line, - "end_line": c.end_line, - "language": c.language, - "location": chunk_location(c), - }, + let mut entry = json!({ + "file_path": c.file_path, + "start_line": c.start_line, + "end_line": c.end_line, "score": result.score, - }) + }); + match max_snippet_lines { + None => { + entry["content"] = json!(c.content); + } + Some(0) => {} + Some(n) => { + // Python `splitlines()` also breaks on bare `\r`; `str::lines()` + // does not, so reuse the chunker's splitter for line parity. + let snippet: Vec<&str> = split_lines_keep_ends(&c.content) + .into_iter() + .take(n) + .map(|l| l.trim_end_matches(['\r', '\n'])) + .collect(); + entry["content"] = json!(snippet.join("\n")); + } + } + entry } /// Build the `{ query, results }` payload the CLI prints and the MCP server -/// returns. Port of `utils.formatResults`. -pub fn format_results(query: &str, results: &[SearchResult]) -> Value { +/// returns. Port of `utils.format_results`. `max_snippet_lines` is forwarded to +/// [`result_to_dict`] to cap each result's `content`. +pub fn format_results( + query: &str, + results: &[SearchResult], + max_snippet_lines: Option, +) -> Value { json!({ "query": query, - "results": results.iter().map(result_to_dict).collect::>(), + "results": results + .iter() + .map(|r| result_to_dict(r, max_snippet_lines)) + .collect::>(), }) } @@ -114,6 +150,21 @@ pub fn resolve_chunk<'a>(chunks: &'a [Chunk], file_path: &str, line: u32) -> Opt mod tests { use super::*; + #[test] + fn result_to_dict_cap_splits_crlf_and_bare_cr_like_python_splitlines() { + let r = result("a\r\nb\rc\nd"); + let d = result_to_dict(&r, Some(3)); + assert_eq!(d["content"], json!("a\nb\nc")); + } + + #[test] + fn resolve_snippet_lines_maps_wire_value_to_cap() { + assert!(resolve_snippet_lines(None).is_none()); + assert_eq!(resolve_snippet_lines(Some(3)), Some(3)); + assert_eq!(resolve_snippet_lines(Some(0)), Some(0)); + assert_eq!(resolve_snippet_lines(Some(-4)), Some(0)); + } + fn chunk(file_path: &str, start_line: u32, end_line: u32) -> Chunk { Chunk { content: String::new(), @@ -124,6 +175,55 @@ mod tests { } } + fn result(content: &str) -> SearchResult { + SearchResult { + chunk: Chunk { + content: content.to_string(), + file_path: "a.ts".to_string(), + start_line: 1, + end_line: 3, + language: Some("ts".to_string()), + }, + score: 0.5, + } + } + + #[test] + fn format_results_flat_shape_with_full_content() { + let out = format_results("q", &[result("line1\nline2\nline3")], None); + let entry = &out["results"][0]; + // Flat shape: fields live at the top level, no nested `chunk`. + assert!(entry.get("chunk").is_none()); + assert_eq!(entry["file_path"], "a.ts"); + assert_eq!(entry["start_line"], 1); + assert_eq!(entry["end_line"], 3); + assert_eq!(entry["score"], 0.5); + assert_eq!(entry["content"], "line1\nline2\nline3"); + assert_eq!(out["query"], "q"); + } + + #[test] + fn result_to_dict_truncates_to_n_lines() { + let entry = result_to_dict(&result("line1\nline2\nline3\nline4"), Some(2)); + assert_eq!(entry["content"], "line1\nline2"); + } + + #[test] + fn result_to_dict_omits_content_when_zero() { + let entry = result_to_dict(&result("line1\nline2"), Some(0)); + assert!(entry.get("content").is_none()); + // Location metadata is still present so the agent can navigate. + assert_eq!(entry["file_path"], "a.ts"); + assert_eq!(entry["start_line"], 1); + assert_eq!(entry["end_line"], 3); + } + + #[test] + fn result_to_dict_more_lines_than_content_returns_all() { + let entry = result_to_dict(&result("only\ntwo"), Some(10)); + assert_eq!(entry["content"], "only\ntwo"); + } + #[test] fn recognises_scheme_git_urls() { for url in [ From 83a438b82588ba9e8f23df1bf8e79e4b7fa5794a Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Fri, 4 Sep 2026 21:28:24 +0900 Subject: [PATCH 5/8] fix(index): reject zero BM25 term counts on load; take persisted vectors verbatim - Bm25Index::load rejects a zero term frequency (it would inflate the term's document frequency) so the cache falls back to a full rebuild. - SelectableBasicBackend::load no longer re-normalises rows that were normalised before save, keeping unchanged rows bit-identical across an incremental rebuild seeded from disk. Refs #84 --- crates/csp/src/indexing/dense/backend.rs | 14 +++++++++----- crates/csp/src/indexing/dense/tests.rs | 12 ++++++++++++ crates/csp/src/indexing/sparse.rs | 20 ++++++++++++++++++++ 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/crates/csp/src/indexing/dense/backend.rs b/crates/csp/src/indexing/dense/backend.rs index 3b96186..8d4d193 100644 --- a/crates/csp/src/indexing/dense/backend.rs +++ b/crates/csp/src/indexing/dense/backend.rs @@ -234,11 +234,15 @@ impl SelectableBasicBackend { } vectors.push(row); } - let mut backend = Self::new(vectors, meta.arguments)?; - if meta.rows == 0 { - backend.dim = meta.dim; - } - Ok(backend) + // Rows were normalised by `new` before they were saved. Re-normalising + // here can flip low bits, which would break bit-identical reuse of + // unchanged rows on an incremental rebuild — take them verbatim. Every + // row has exactly `meta.dim` elements by construction above. + Ok(Self { + vectors, + arguments: meta.arguments, + dim: meta.dim, + }) } } diff --git a/crates/csp/src/indexing/dense/tests.rs b/crates/csp/src/indexing/dense/tests.rs index 28255e1..3f1a3f9 100644 --- a/crates/csp/src/indexing/dense/tests.rs +++ b/crates/csp/src/indexing/dense/tests.rs @@ -253,6 +253,18 @@ fn save_load_round_trips() { assert_eq!(orig_hits, loaded_hits); } +#[test] +fn load_takes_persisted_rows_verbatim() { + // A deliberately non-unit row: re-normalising on load would turn it into + // [0.6, 0.8], so equality proves the persisted bytes are used as-is. + let backend = SelectableBasicBackend::from_normalized(vec![vec![3.0, 4.0]]).unwrap(); + let dir = tempfile::tempdir().unwrap(); + backend.save(dir.path()).unwrap(); + let loaded = SelectableBasicBackend::load(dir.path()).unwrap(); + assert_eq!(loaded.vectors, vec![vec![3.0, 4.0]]); + assert_eq!(loaded.dim, 2); +} + #[test] fn load_preserves_dimension_for_empty_index() { let dir = tempdir().unwrap(); diff --git a/crates/csp/src/indexing/sparse.rs b/crates/csp/src/indexing/sparse.rs index 97d3681..04f464c 100644 --- a/crates/csp/src/indexing/sparse.rs +++ b/crates/csp/src/indexing/sparse.rs @@ -330,6 +330,11 @@ impl Bm25Index { let mut length = 0u64; let mut terms: Vec<(String, u32)> = Vec::with_capacity(counts.len()); for (term, freq) in counts { + // A zero count would still create a posting and inflate the + // term's document frequency; treat it as corruption. + if freq == 0 { + return Err(invalid("Persisted BM25 term frequencies must be positive")); + } length += u64::from(freq); terms.push((term, freq)); } @@ -622,6 +627,21 @@ mod tests { ); } + #[test] + fn load_rejects_zero_term_frequency() { + let index = build_ids(&[("a", &["authenticate"])]); + let dir = tempfile::tempdir().unwrap(); + index.save(dir.path()).unwrap(); + let path = dir.path().join("bm25.json"); + let mut value: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap(); + value["documents"]["a"]["authenticate"] = serde_json::json!(0); + std::fs::write(&path, value.to_string()).unwrap(); + + let err = Bm25Index::load(dir.path()).unwrap_err(); + assert!(err.to_string().contains("must be positive")); + } + #[test] fn load_rejects_inconsistent_document_order() { let index = build_ids(&[("a", &["authenticate"])]); From 7c5f073006d3a99c2389da260f4c47631274b34f Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Fri, 4 Sep 2026 21:33:41 +0900 Subject: [PATCH 6/8] refactor(index): split create/sparse tests out, extract create_index_from_path helpers - create.rs / sparse.rs test modules move to create/tests.rs and sparse/tests.rs, matching the index/, dense/, cache_orchestrator/ layout. - create_index_from_path delegates to open_previous, display_path, take_previous_rows and embed_fresh_rows; behaviour unchanged. - load_previous_for_incremental compares the content selection as a set, so a duplicated request no longer matches a manifest that covers more. Refs #84 --- crates/csp/src/indexing/cache_orchestrator.rs | 9 +- .../src/indexing/cache_orchestrator/tests.rs | 22 +- crates/csp/src/indexing/create.rs | 440 ++++-------------- crates/csp/src/indexing/create/tests.rs | 272 +++++++++++ crates/csp/src/indexing/sparse.rs | 293 +----------- crates/csp/src/indexing/sparse/tests.rs | 290 ++++++++++++ 6 files changed, 690 insertions(+), 636 deletions(-) create mode 100644 crates/csp/src/indexing/create/tests.rs create mode 100644 crates/csp/src/indexing/sparse/tests.rs diff --git a/crates/csp/src/indexing/cache_orchestrator.rs b/crates/csp/src/indexing/cache_orchestrator.rs index f097c3b..9bee642 100644 --- a/crates/csp/src/indexing/cache_orchestrator.rs +++ b/crates/csp/src/indexing/cache_orchestrator.rs @@ -2,6 +2,7 @@ //! `get_validated_cache` / `load_previous_for_incremental` half of semble //! `cache.py`, adapted to csp's content-hash oracle — ADR-0002 / ADR-0005). +use std::collections::BTreeSet; use std::path::{Path, PathBuf}; use crate::chunking::source::DESIRED_CHUNK_LENGTH_CHARS; @@ -156,9 +157,11 @@ pub(crate) fn load_previous_for_incremental( if !manifest_compatible(&manifest, expected_model) { return None; } - let same_content = manifest.content.len() == content.len() - && content.iter().all(|c| manifest.content.contains(c)); - if !same_content || manifest.files.is_empty() { + // Compare as sets: a length check plus `contains` would accept + // `[Code, Code]` against `[Code, Docs]`. + let manifest_content: BTreeSet<&str> = manifest.content.iter().map(|c| c.as_str()).collect(); + let expected_content: BTreeSet<&str> = content.iter().map(|c| c.as_str()).collect(); + if manifest_content != expected_content || manifest.files.is_empty() { return None; } diff --git a/crates/csp/src/indexing/cache_orchestrator/tests.rs b/crates/csp/src/indexing/cache_orchestrator/tests.rs index 1105ce9..5b39490 100644 --- a/crates/csp/src/indexing/cache_orchestrator/tests.rs +++ b/crates/csp/src/indexing/cache_orchestrator/tests.rs @@ -3,7 +3,7 @@ use crate::indexing::cache::{resolve_cache_dir, CacheLocation}; use crate::indexing::dense::{make_stub_model, SelectableBasicBackend, DEFAULT_MODEL_NAME}; use crate::indexing::index::{CspIndexState, DEFAULT_CONTENT}; use crate::indexing::sparse::Bm25Index; -use crate::types::Chunk; +use crate::types::{Chunk, ContentType}; use tempfile::tempdir; fn make_chunk(file_path: &str, content: &str) -> Chunk { @@ -259,6 +259,26 @@ fn load_previous_for_incremental_fails_closed() { } } +#[test] +fn load_previous_for_incremental_compares_content_as_a_set() { + let home = tempdir().unwrap(); + let (_src, cache_dir) = build_valid_cache(home.path()); + let manifest_path = cache_dir.join("manifest.json"); + let mut manifest = read_json(&manifest_path); + let duplicated = [ContentType::Code, ContentType::Code]; + + // Same length, and every requested type is present in the manifest — but + // the manifest also covers docs the request does not ask for. + manifest["content"] = serde_json::json!(["code", "docs"]); + write_json(&manifest_path, &manifest); + assert!(load_previous_for_incremental(&cache_dir, DEFAULT_MODEL_NAME, &duplicated).is_none()); + + // Repetition on the request side is irrelevant once the sets agree. + manifest["content"] = serde_json::json!(["code"]); + write_json(&manifest_path, &manifest); + assert!(load_previous_for_incremental(&cache_dir, DEFAULT_MODEL_NAME, &duplicated).is_some()); +} + #[test] fn load_or_build_reuses_unchanged_files_on_incremental_rebuild() { let home = tempdir().unwrap(); diff --git a/crates/csp/src/indexing/create.rs b/crates/csp/src/indexing/create.rs index ee22b39..dabb83b 100644 --- a/crates/csp/src/indexing/create.rs +++ b/crates/csp/src/indexing/create.rs @@ -67,6 +67,95 @@ fn reindex_file( Ok(()) } +/// Split a previous index into the parts the rebuild consumes: its BM25 index +/// is mutated in place, and its chunk / vector rows are wrapped in `Option` so +/// unchanged files can move them out without copying. +type PreviousParts = ( + Bm25Index, + FileManifest, + Vec>, + Vec>>, +); + +fn open_previous(previous: Option) -> PreviousParts { + match previous { + Some(prev) => ( + prev.bm25_index, + prev.files, + prev.chunks.into_iter().map(Some).collect(), + prev.vectors.into_iter().map(Some).collect(), + ), + None => ( + Bm25Index::new(), + FileManifest::new(), + Vec::new(), + Vec::new(), + ), + } +} + +/// The path a file is indexed under: relative to `display_root` when set. +fn display_path(file_path: &Path, display_root: Option<&Path>) -> String { + match display_root { + Some(root) => file_path + .strip_prefix(root) + .unwrap_or(file_path) + .to_string_lossy() + .into_owned(), + None => file_path.to_string_lossy().into_owned(), + } +} + +/// Move an unchanged file's previous chunk + vector rows out, or `None` when +/// the file is new, changed, or its manifest range is out of bounds. Each row +/// is taken at most once because a validated manifest's ranges never overlap. +fn take_previous_rows( + entry: Option<&FileManifestEntry>, + hash: &str, + previous_chunks: &mut [Option], + previous_vectors: &mut [Option>], +) -> Option<(Vec, Vec>)> { + let entry = entry?; + if entry.hash != hash + || entry.end() > previous_chunks.len() + || entry.end() > previous_vectors.len() + { + return None; + } + let rows: Option> = previous_chunks[entry.start..entry.end()] + .iter_mut() + .map(Option::take) + .collect(); + let vecs: Option>> = previous_vectors[entry.start..entry.end()] + .iter_mut() + .map(Option::take) + .collect(); + rows.zip(vecs) +} + +/// Fill the `None` holes left for freshly chunked files with one batched embed +/// — the tokenizer parallelises per batch, so a call per file would serialise a +/// cold build. Fresh rows are normalised through the backend so they match the +/// reused (already-normalised) rows. +fn embed_fresh_rows( + model: &Model, + chunks: &[Chunk], + fresh_rows: &[usize], + mut vectors: Vec>>, +) -> Result>, String> { + let fresh_chunks: Vec<&Chunk> = fresh_rows.iter().map(|&i| &chunks[i]).collect(); + let fresh_vectors = + SelectableBasicBackend::from_vectors(embed_chunk_refs(model, &fresh_chunks))?.vectors; + if fresh_vectors.len() != fresh_rows.len() { + return Err("Embedder returned the wrong number of rows".to_string()); + } + for (&row, vector) in fresh_rows.iter().zip(fresh_vectors) { + vectors[row] = Some(vector); + } + let vectors: Option>> = vectors.into_iter().collect(); + vectors.ok_or_else(|| "Internal error: an embedding row was left unfilled".to_string()) +} + /// Create an index from a resolved directory, optionally reusing a previous /// index's unchanged files. Errors when no chunks are produced. pub fn create_index_from_path( @@ -81,23 +170,8 @@ pub fn create_index_from_path( let resolved = get_extensions(&content, options.extensions.as_deref()); let ext_refs: Vec<&str> = resolved.iter().map(String::as_str).collect(); - // The previous index is consumed: its BM25 index is mutated in place and its - // chunk/vector rows are moved out rather than copied. - let (mut bm25_index, previous_files, mut previous_chunks, mut previous_vectors) = match previous - { - Some(prev) => ( - prev.bm25_index, - prev.files, - prev.chunks.into_iter().map(Some).collect::>(), - prev.vectors.into_iter().map(Some).collect::>(), - ), - None => ( - Bm25Index::new(), - FileManifest::new(), - Vec::new(), - Vec::new(), - ), - }; + let (mut bm25_index, previous_files, mut previous_chunks, mut previous_vectors) = + open_previous(previous); let mut chunks: Vec = Vec::new(); let mut chunk_ids: Vec = Vec::new(); @@ -121,14 +195,7 @@ pub fn create_index_from_path( continue; }; let hash = sha256_hex(&bytes); - let indexed_path = match &options.display_root { - Some(root) => file_path - .strip_prefix(root) - .unwrap_or(&file_path) - .to_string_lossy() - .into_owned(), - None => file_path.to_string_lossy().into_owned(), - }; + let indexed_path = display_path(&file_path, options.display_root.as_deref()); // `to_string_lossy` is not injective: on Unix, file names that differ // only in invalid UTF-8 bytes collapse to the same display path, and the // BM25 chunk ids derived from it would then collide and abort the whole @@ -144,26 +211,12 @@ pub fn create_index_from_path( } let previous_entry = previous_files.get(&indexed_path); - // Unchanged file: move its previous chunk + vector rows out (each row is - // taken at most once because a validated manifest's ranges never overlap). - let reused = match previous_entry { - Some(entry) - if entry.hash == hash - && entry.end() <= previous_chunks.len() - && entry.end() <= previous_vectors.len() => - { - let rows: Option> = previous_chunks[entry.start..entry.end()] - .iter_mut() - .map(Option::take) - .collect(); - let vecs: Option>> = previous_vectors[entry.start..entry.end()] - .iter_mut() - .map(Option::take) - .collect(); - rows.zip(vecs) - } - _ => None, - }; + let reused = take_previous_rows( + previous_entry, + &hash, + &mut previous_chunks, + &mut previous_vectors, + ); let start = chunks.len(); let file_chunks = match reused { Some((file_chunks, file_vectors)) => { @@ -202,22 +255,7 @@ pub fn create_index_from_path( )); } - // One batched embed for every changed file's chunks — the tokenizer - // parallelises per batch, so a call per file would serialise a cold build. - // Normalise through the backend so fresh rows match the reused - // (already-normalised) rows. - let fresh_chunks: Vec<&Chunk> = fresh_rows.iter().map(|&i| &chunks[i]).collect(); - let fresh_vectors = - SelectableBasicBackend::from_vectors(embed_chunk_refs(options.model, &fresh_chunks))? - .vectors; - if fresh_vectors.len() != fresh_rows.len() { - return Err("Embedder returned the wrong number of rows".to_string()); - } - for (&row, vector) in fresh_rows.iter().zip(fresh_vectors) { - vectors[row] = Some(vector); - } - let vectors: Option>> = vectors.into_iter().collect(); - let vectors = vectors.ok_or("Internal error: an embedding row was left unfilled")?; + let vectors = embed_fresh_rows(options.model, &chunks, &fresh_rows, vectors)?; bm25_index.set_doc_order(chunk_ids); let semantic_index = SelectableBasicBackend::from_normalized(vectors)?; @@ -231,282 +269,4 @@ pub fn create_index_from_path( } #[cfg(test)] -mod tests { - use super::*; - use crate::indexing::dense::make_stub_model; - use crate::tokens::tokenize; - use tempfile::tempdir; - - fn opts(model: &Model, display_root: Option) -> CreateIndexOptions<'_> { - CreateIndexOptions { - model, - extensions: None, - content: None, - display_root, - } - } - - #[test] - fn builds_indexes_for_small_ts_file() { - let dir = tempdir().unwrap(); - std::fs::write( - dir.path().join("sample.ts"), - "export function greet(name: string) {\n return `hi ${name}`\n}\n", - ) - .unwrap(); - let model = make_stub_model(4); - let result = create_index_from_path( - dir.path(), - &opts(&model, Some(dir.path().to_path_buf())), - None, - ) - .unwrap(); - - assert!(!result.chunks.is_empty()); - assert_eq!(result.chunks[0].file_path, "sample.ts"); - assert_eq!(result.semantic_index.vectors.len(), result.chunks.len()); - assert_eq!(result.bm25_index.num_docs(), result.chunks.len()); - } - - #[test] - fn errors_when_no_supported_files() { - let dir = tempdir().unwrap(); - std::fs::write(dir.path().join("data.bin"), "binary").unwrap(); - let model = make_stub_model(4); - let err = create_index_from_path(dir.path(), &opts(&model, None), None).unwrap_err(); - assert!(err.contains("No supported files found")); - } - - #[test] - fn respects_extensions_override() { - let dir = tempdir().unwrap(); - std::fs::write(dir.path().join("a.txt"), "hello world").unwrap(); - let model = make_stub_model(4); - let options = CreateIndexOptions { - model: &model, - extensions: Some(vec![".txt".to_string()]), - content: Some(vec![ContentType::Docs]), - display_root: Some(dir.path().to_path_buf()), - }; - let result = create_index_from_path(dir.path(), &options, None).unwrap(); - assert_eq!(result.chunks.len(), 1); - assert_eq!(result.chunks[0].file_path, "a.txt"); - } - - #[test] - fn skips_files_over_max_bytes() { - let dir = tempdir().unwrap(); - std::fs::write(dir.path().join("big.ts"), "a".repeat(2_000_000)).unwrap(); - std::fs::write(dir.path().join("small.ts"), "export const x = 1\n").unwrap(); - let model = make_stub_model(4); - let result = create_index_from_path( - dir.path(), - &opts(&model, Some(dir.path().to_path_buf())), - None, - ) - .unwrap(); - let paths: Vec<&str> = result.chunks.iter().map(|c| c.file_path.as_str()).collect(); - assert!(paths.contains(&"small.ts")); - assert!(!paths.contains(&"big.ts")); - } - - #[test] - fn descends_into_subdirectories() { - let dir = tempdir().unwrap(); - std::fs::create_dir(dir.path().join("sub")).unwrap(); - std::fs::write(dir.path().join("sub/nested.ts"), "const a = 1\n").unwrap(); - let model = make_stub_model(4); - let result = create_index_from_path( - dir.path(), - &opts(&model, Some(dir.path().to_path_buf())), - None, - ) - .unwrap(); - assert!(result - .chunks - .iter() - .any(|c| c.file_path.ends_with("nested.ts"))); - } - - // --- incremental reindex (mirrors upstream tests/index/test_create.py) --- - - fn write_files(root: &Path, files: &[(&str, &str)]) { - for (rel, content) in files { - let path = root.join(rel); - std::fs::create_dir_all(path.parent().unwrap()).unwrap(); - std::fs::write(path, content).unwrap(); - } - } - - fn into_previous(result: CreateIndexResult) -> PreviousIndex { - PreviousIndex::try_new( - result.chunks, - result.semantic_index.vectors, - result.files, - result.bm25_index, - ) - .unwrap() - } - - fn score_sum(index: &Bm25Index, query: &str) -> f32 { - index.get_scores(&tokenize(query), None).iter().sum() - } - - #[test] - fn fresh_build_records_a_layout_valid_file_manifest() { - let dir = tempdir().unwrap(); - write_files( - dir.path(), - &[ - ("a.ts", "function stable_anchor() { return 1 }\n"), - ("sub/b.ts", "function other_value() { return 2 }\n"), - ], - ); - let model = make_stub_model(4); - let result = create_index_from_path( - dir.path(), - &opts(&model, Some(dir.path().to_path_buf())), - None, - ) - .unwrap(); - - assert_eq!(result.files.len(), 2); - let total: usize = result.files.values().map(|e| e.count).sum(); - assert_eq!(total, result.chunks.len()); - for (path, entry) in &result.files { - assert_eq!(entry.hash.len(), 64); - assert!(result.chunks[entry.start..entry.end()] - .iter() - .all(|c| c.file_path == *path)); - } - // The manifest, chunks, vectors, and BM25 order all agree. - assert!(into_previous(result).files.contains_key("a.ts")); - } - - #[test] - fn incremental_reindex_reuses_updates_and_prunes() { - let dir = tempdir().unwrap(); - let root = dir.path().to_path_buf(); - write_files( - &root, - &[ - ("a.ts", "function stable_anchor() { return 1 }\n"), - ("b.ts", "function changed_value() { return 2 }\n"), - ("c.ts", "function unique_gone() { return 3 }\n"), - ("emptying.ts", "function becomes_empty() { return 4 }\n"), - ], - ); - let model = make_stub_model(4); - let before = - create_index_from_path(&root, &opts(&model, Some(root.clone())), None).unwrap(); - let b_before = before.files["b.ts"].clone(); - let b_vectors_before = - before.semantic_index.vectors[b_before.start..b_before.end()].to_vec(); - let a_before = before.files["a.ts"].clone(); - - // Plant sentinels in the previous index for the unchanged file: they can - // only survive into the rebuilt index if its rows were reused, not - // re-chunked/re-embedded (the stub embedder is deterministic). - let mut previous = into_previous(before); - let sentinel_vector = vec![0.0, 1.0, 0.0, 0.0]; - previous.vectors[a_before.start] = sentinel_vector.clone(); - previous.chunks[a_before.start] - .content - .push_str("/*reused*/"); - - write_files( - &root, - &[("b.ts", "function changed_value() { return 999 }\n")], - ); - std::fs::remove_file(root.join("c.ts")).unwrap(); - write_files(&root, &[("emptying.ts", &" ".repeat(128))]); - write_files( - &root, - &[("d.ts", "function brand_new_term() { return 4 }\n")], - ); - - let after = - create_index_from_path(&root, &opts(&model, Some(root.clone())), Some(previous)) - .unwrap(); - - let a_after = &after.files["a.ts"]; - assert_eq!(after.semantic_index.vectors[a_after.start], sentinel_vector); - assert!(after.chunks[a_after.start].content.ends_with("/*reused*/")); - let b_after = &after.files["b.ts"]; - assert_ne!( - after.semantic_index.vectors[b_after.start..b_after.end()].to_vec(), - b_vectors_before - ); - assert!(!after.files.contains_key("c.ts")); - assert!(after.files.contains_key("d.ts")); - assert_eq!(after.files["emptying.ts"].count, 0); - - assert_eq!(score_sum(&after.bm25_index, "unique_gone"), 0.0); - assert_eq!(score_sum(&after.bm25_index, "becomes_empty"), 0.0); - assert!(score_sum(&after.bm25_index, "brand_new_term") > 0.0); - assert!(score_sum(&after.bm25_index, "changed_value") > 0.0); - - let mut expected_ids: Vec = after - .files - .iter() - .flat_map(|(path, entry)| (0..entry.count).map(move |slot| make_chunk_id(path, slot))) - .collect(); - expected_ids.sort(); - let mut doc_order = after.bm25_index.doc_order().to_vec(); - doc_order.sort(); - assert_eq!(doc_order, expected_ids); - assert_eq!(after.bm25_index.corpus_size(), after.chunks.len()); - assert_eq!(after.semantic_index.vectors.len(), after.chunks.len()); - // The rebuilt index is itself a valid seed for the next incremental pass. - into_previous(after); - } - - #[test] - fn zero_chunk_file_does_not_break_manifest_tiling() { - let dir = tempdir().unwrap(); - let root = dir.path().to_path_buf(); - // `pkg/` is walked before `pkg.ts` (directory entries sort by file name), - // but "pkg.ts" < "pkg/z.ts" lexicographically ('.' 0x2E < '/' 0x2F). The - // empty file yields no chunks, so both entries share `start`. - write_files( - &root, - &[ - ("pkg/z.ts", " \n"), - ("pkg.ts", "function stable_anchor() { return 1 }\n"), - ], - ); - let model = make_stub_model(4); - let result = - create_index_from_path(&root, &opts(&model, Some(root.clone())), None).unwrap(); - assert_eq!(result.files["pkg/z.ts"].count, 0); - assert_eq!(result.files["pkg/z.ts"].start, result.files["pkg.ts"].start); - // A freshly built index must always be a valid seed for the next pass. - into_previous(result); - } - - /// Non-UTF-8 file names only exist on Unix filesystems that allow them - /// (APFS rejects them), so this runs on Linux only. - #[cfg(target_os = "linux")] - #[test] - fn colliding_lossy_paths_skip_the_duplicate_instead_of_aborting() { - use std::os::unix::ffi::OsStrExt; - let dir = tempdir().unwrap(); - let root = dir.path().to_path_buf(); - let first = root.join(std::ffi::OsStr::from_bytes(b"a\xff.ts")); - let second = root.join(std::ffi::OsStr::from_bytes(b"a\xfe.ts")); - std::fs::write(&first, "function first_file() { return 1 }\n").unwrap(); - std::fs::write(&second, "function second_file() { return 2 }\n").unwrap(); - assert_eq!( - first.to_string_lossy(), - second.to_string_lossy(), - "test precondition: both names must collapse to one display path" - ); - - let model = make_stub_model(4); - let result = - create_index_from_path(&root, &opts(&model, Some(root.clone())), None).unwrap(); - assert_eq!(result.files.len(), 1); - assert_eq!(result.bm25_index.corpus_size(), result.chunks.len()); - into_previous(result); - } -} +mod tests; diff --git a/crates/csp/src/indexing/create/tests.rs b/crates/csp/src/indexing/create/tests.rs new file mode 100644 index 0000000..6d55fdf --- /dev/null +++ b/crates/csp/src/indexing/create/tests.rs @@ -0,0 +1,272 @@ +use super::*; +use crate::indexing::dense::make_stub_model; +use crate::tokens::tokenize; +use tempfile::tempdir; + +fn opts(model: &Model, display_root: Option) -> CreateIndexOptions<'_> { + CreateIndexOptions { + model, + extensions: None, + content: None, + display_root, + } +} + +#[test] +fn builds_indexes_for_small_ts_file() { + let dir = tempdir().unwrap(); + std::fs::write( + dir.path().join("sample.ts"), + "export function greet(name: string) {\n return `hi ${name}`\n}\n", + ) + .unwrap(); + let model = make_stub_model(4); + let result = create_index_from_path( + dir.path(), + &opts(&model, Some(dir.path().to_path_buf())), + None, + ) + .unwrap(); + + assert!(!result.chunks.is_empty()); + assert_eq!(result.chunks[0].file_path, "sample.ts"); + assert_eq!(result.semantic_index.vectors.len(), result.chunks.len()); + assert_eq!(result.bm25_index.num_docs(), result.chunks.len()); +} + +#[test] +fn errors_when_no_supported_files() { + let dir = tempdir().unwrap(); + std::fs::write(dir.path().join("data.bin"), "binary").unwrap(); + let model = make_stub_model(4); + let err = create_index_from_path(dir.path(), &opts(&model, None), None).unwrap_err(); + assert!(err.contains("No supported files found")); +} + +#[test] +fn respects_extensions_override() { + let dir = tempdir().unwrap(); + std::fs::write(dir.path().join("a.txt"), "hello world").unwrap(); + let model = make_stub_model(4); + let options = CreateIndexOptions { + model: &model, + extensions: Some(vec![".txt".to_string()]), + content: Some(vec![ContentType::Docs]), + display_root: Some(dir.path().to_path_buf()), + }; + let result = create_index_from_path(dir.path(), &options, None).unwrap(); + assert_eq!(result.chunks.len(), 1); + assert_eq!(result.chunks[0].file_path, "a.txt"); +} + +#[test] +fn skips_files_over_max_bytes() { + let dir = tempdir().unwrap(); + std::fs::write(dir.path().join("big.ts"), "a".repeat(2_000_000)).unwrap(); + std::fs::write(dir.path().join("small.ts"), "export const x = 1\n").unwrap(); + let model = make_stub_model(4); + let result = create_index_from_path( + dir.path(), + &opts(&model, Some(dir.path().to_path_buf())), + None, + ) + .unwrap(); + let paths: Vec<&str> = result.chunks.iter().map(|c| c.file_path.as_str()).collect(); + assert!(paths.contains(&"small.ts")); + assert!(!paths.contains(&"big.ts")); +} + +#[test] +fn descends_into_subdirectories() { + let dir = tempdir().unwrap(); + std::fs::create_dir(dir.path().join("sub")).unwrap(); + std::fs::write(dir.path().join("sub/nested.ts"), "const a = 1\n").unwrap(); + let model = make_stub_model(4); + let result = create_index_from_path( + dir.path(), + &opts(&model, Some(dir.path().to_path_buf())), + None, + ) + .unwrap(); + assert!(result + .chunks + .iter() + .any(|c| c.file_path.ends_with("nested.ts"))); +} + +// --- incremental reindex (mirrors upstream tests/index/test_create.py) --- + +fn write_files(root: &Path, files: &[(&str, &str)]) { + for (rel, content) in files { + let path = root.join(rel); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, content).unwrap(); + } +} + +fn into_previous(result: CreateIndexResult) -> PreviousIndex { + PreviousIndex::try_new( + result.chunks, + result.semantic_index.vectors, + result.files, + result.bm25_index, + ) + .unwrap() +} + +fn score_sum(index: &Bm25Index, query: &str) -> f32 { + index.get_scores(&tokenize(query), None).iter().sum() +} + +#[test] +fn fresh_build_records_a_layout_valid_file_manifest() { + let dir = tempdir().unwrap(); + write_files( + dir.path(), + &[ + ("a.ts", "function stable_anchor() { return 1 }\n"), + ("sub/b.ts", "function other_value() { return 2 }\n"), + ], + ); + let model = make_stub_model(4); + let result = create_index_from_path( + dir.path(), + &opts(&model, Some(dir.path().to_path_buf())), + None, + ) + .unwrap(); + + assert_eq!(result.files.len(), 2); + let total: usize = result.files.values().map(|e| e.count).sum(); + assert_eq!(total, result.chunks.len()); + for (path, entry) in &result.files { + assert_eq!(entry.hash.len(), 64); + assert!(result.chunks[entry.start..entry.end()] + .iter() + .all(|c| c.file_path == *path)); + } + // The manifest, chunks, vectors, and BM25 order all agree. + assert!(into_previous(result).files.contains_key("a.ts")); +} + +#[test] +fn incremental_reindex_reuses_updates_and_prunes() { + let dir = tempdir().unwrap(); + let root = dir.path().to_path_buf(); + write_files( + &root, + &[ + ("a.ts", "function stable_anchor() { return 1 }\n"), + ("b.ts", "function changed_value() { return 2 }\n"), + ("c.ts", "function unique_gone() { return 3 }\n"), + ("emptying.ts", "function becomes_empty() { return 4 }\n"), + ], + ); + let model = make_stub_model(4); + let before = create_index_from_path(&root, &opts(&model, Some(root.clone())), None).unwrap(); + let b_before = before.files["b.ts"].clone(); + let b_vectors_before = before.semantic_index.vectors[b_before.start..b_before.end()].to_vec(); + let a_before = before.files["a.ts"].clone(); + + // Plant sentinels in the previous index for the unchanged file: they can + // only survive into the rebuilt index if its rows were reused, not + // re-chunked/re-embedded (the stub embedder is deterministic). + let mut previous = into_previous(before); + let sentinel_vector = vec![0.0, 1.0, 0.0, 0.0]; + previous.vectors[a_before.start] = sentinel_vector.clone(); + previous.chunks[a_before.start] + .content + .push_str("/*reused*/"); + + write_files( + &root, + &[("b.ts", "function changed_value() { return 999 }\n")], + ); + std::fs::remove_file(root.join("c.ts")).unwrap(); + write_files(&root, &[("emptying.ts", &" ".repeat(128))]); + write_files( + &root, + &[("d.ts", "function brand_new_term() { return 4 }\n")], + ); + + let after = + create_index_from_path(&root, &opts(&model, Some(root.clone())), Some(previous)).unwrap(); + + let a_after = &after.files["a.ts"]; + assert_eq!(after.semantic_index.vectors[a_after.start], sentinel_vector); + assert!(after.chunks[a_after.start].content.ends_with("/*reused*/")); + let b_after = &after.files["b.ts"]; + assert_ne!( + after.semantic_index.vectors[b_after.start..b_after.end()].to_vec(), + b_vectors_before + ); + assert!(!after.files.contains_key("c.ts")); + assert!(after.files.contains_key("d.ts")); + assert_eq!(after.files["emptying.ts"].count, 0); + + assert_eq!(score_sum(&after.bm25_index, "unique_gone"), 0.0); + assert_eq!(score_sum(&after.bm25_index, "becomes_empty"), 0.0); + assert!(score_sum(&after.bm25_index, "brand_new_term") > 0.0); + assert!(score_sum(&after.bm25_index, "changed_value") > 0.0); + + let mut expected_ids: Vec = after + .files + .iter() + .flat_map(|(path, entry)| (0..entry.count).map(move |slot| make_chunk_id(path, slot))) + .collect(); + expected_ids.sort(); + let mut doc_order = after.bm25_index.doc_order().to_vec(); + doc_order.sort(); + assert_eq!(doc_order, expected_ids); + assert_eq!(after.bm25_index.corpus_size(), after.chunks.len()); + assert_eq!(after.semantic_index.vectors.len(), after.chunks.len()); + // The rebuilt index is itself a valid seed for the next incremental pass. + into_previous(after); +} + +#[test] +fn zero_chunk_file_does_not_break_manifest_tiling() { + let dir = tempdir().unwrap(); + let root = dir.path().to_path_buf(); + // `pkg/` is walked before `pkg.ts` (directory entries sort by file name), + // but "pkg.ts" < "pkg/z.ts" lexicographically ('.' 0x2E < '/' 0x2F). The + // empty file yields no chunks, so both entries share `start`. + write_files( + &root, + &[ + ("pkg/z.ts", " \n"), + ("pkg.ts", "function stable_anchor() { return 1 }\n"), + ], + ); + let model = make_stub_model(4); + let result = create_index_from_path(&root, &opts(&model, Some(root.clone())), None).unwrap(); + assert_eq!(result.files["pkg/z.ts"].count, 0); + assert_eq!(result.files["pkg/z.ts"].start, result.files["pkg.ts"].start); + // A freshly built index must always be a valid seed for the next pass. + into_previous(result); +} + +/// Non-UTF-8 file names only exist on Unix filesystems that allow them +/// (APFS rejects them), so this runs on Linux only. +#[cfg(target_os = "linux")] +#[test] +fn colliding_lossy_paths_skip_the_duplicate_instead_of_aborting() { + use std::os::unix::ffi::OsStrExt; + let dir = tempdir().unwrap(); + let root = dir.path().to_path_buf(); + let first = root.join(std::ffi::OsStr::from_bytes(b"a\xff.ts")); + let second = root.join(std::ffi::OsStr::from_bytes(b"a\xfe.ts")); + std::fs::write(&first, "function first_file() { return 1 }\n").unwrap(); + std::fs::write(&second, "function second_file() { return 2 }\n").unwrap(); + assert_eq!( + first.to_string_lossy(), + second.to_string_lossy(), + "test precondition: both names must collapse to one display path" + ); + + let model = make_stub_model(4); + let result = create_index_from_path(&root, &opts(&model, Some(root.clone())), None).unwrap(); + assert_eq!(result.files.len(), 1); + assert_eq!(result.bm25_index.corpus_size(), result.chunks.len()); + into_previous(result); +} diff --git a/crates/csp/src/indexing/sparse.rs b/crates/csp/src/indexing/sparse.rs index 04f464c..929f56a 100644 --- a/crates/csp/src/indexing/sparse.rs +++ b/crates/csp/src/indexing/sparse.rs @@ -365,295 +365,4 @@ struct Bm25Serialized { } #[cfg(test)] -mod tests { - use super::*; - - fn chunk(file_path: &str, content: &str) -> Chunk { - Chunk { - content: content.to_string(), - file_path: file_path.to_string(), - start_line: 1, - end_line: 1, - language: None, - } - } - - fn docs(input: &[&[&str]]) -> Vec> { - input - .iter() - .map(|d| d.iter().map(|s| s.to_string()).collect()) - .collect() - } - - fn query(tokens: &[&str]) -> Vec { - tokens.iter().map(|s| s.to_string()).collect() - } - - // --- enrich_for_bm25 (mirrors src/indexing/sparse.test.ts) --- - - #[test] - fn enrich_appends_repeated_stem_and_dir_parts() { - assert_eq!( - enrich_for_bm25(&chunk("src/utils/format.ts", "hello world")), - "hello world format format src utils" - ); - } - - #[test] - fn enrich_trims_to_last_3_dir_parts() { - assert_eq!( - enrich_for_bm25(&chunk("a/b/c/d/foo.py", "x")), - "x foo foo b c d" - ); - } - - #[test] - fn enrich_handles_top_level_file() { - assert_eq!(enrich_for_bm25(&chunk("foo.py", "x")), "x foo foo "); - } - - #[test] - fn enrich_drops_dot_segments() { - assert_eq!( - enrich_for_bm25(&chunk("./a/b/foo.ts", "x")), - "x foo foo a b" - ); - } - - #[test] - fn enrich_normalizes_backslashes() { - assert_eq!( - enrich_for_bm25(&chunk("src\\utils\\format.ts", "hello world")), - "hello world format format src utils" - ); - } - - // --- selector_to_mask --- - - #[test] - fn selector_builds_mask() { - let mask = selector_to_mask(Some(&[0, 2, 5]), 6).unwrap(); - assert_eq!(mask, vec![1, 0, 1, 0, 0, 1]); - } - - #[test] - fn selector_none_returns_none() { - assert_eq!(selector_to_mask(None, 6), None); - } - - #[test] - fn selector_ignores_out_of_bounds() { - let mask = selector_to_mask(Some(&[0, 10]), 3).unwrap(); - assert_eq!(mask, vec![1, 0, 0]); - } - - // --- Bm25Index --- - - #[test] - fn ranks_docs_with_query_term_higher() { - let index = Bm25Index::build(&docs(&[&["hello", "world"], &["hello"], &["world"]])); - let scores = index.get_scores(&query(&["hello"]), None); - assert_eq!(scores.len(), 3); - assert!(scores[0] > 0.0); - assert!(scores[1] > 0.0); - assert_eq!(scores[2], 0.0); - } - - #[test] - fn zero_scores_for_unknown_tokens() { - let index = Bm25Index::build(&docs(&[&["hello"], &["world"]])); - assert_eq!(index.get_scores(&query(&["unknown"]), None), vec![0.0, 0.0]); - } - - #[test] - fn empty_corpus_yields_empty_scores() { - let index = Bm25Index::build(&docs(&[])); - assert_eq!(index.get_scores(&query(&["anything"]), None).len(), 0); - } - - #[test] - fn empty_query_yields_zero_scores() { - let index = Bm25Index::build(&docs(&[&["hello"], &["world"]])); - assert_eq!(index.get_scores(&[], None), vec![0.0, 0.0]); - } - - #[test] - fn weight_mask_zeros_masked_docs() { - let index = Bm25Index::build(&docs(&[&["hello", "world"], &["hello"], &["world"]])); - let scores = index.get_scores(&query(&["hello"]), Some(&[1, 0, 1])); - assert!(scores[0] > 0.0); - assert_eq!(scores[1], 0.0); - assert_eq!(scores[2], 0.0); - } - - #[test] - fn full_mask_matches_baseline() { - let index = Bm25Index::build(&docs(&[&["hello", "world"], &["hello"], &["world"]])); - let baseline = index.get_scores(&query(&["hello"]), None); - let masked = index.get_scores(&query(&["hello"]), Some(&[1, 1, 1])); - assert_eq!(masked, baseline); - } - - #[test] - fn repeated_query_tokens_do_not_compound() { - let index = Bm25Index::build(&docs(&[&["hello"]])); - let single = index.get_scores(&query(&["hello"]), None); - let repeated = index.get_scores(&query(&["hello", "hello", "hello"]), None); - assert_eq!(repeated, single); - } - - // --- save / load (T014) --- - - #[test] - fn save_load_round_trips_scores() { - let index = Bm25Index::build(&docs(&[ - &["hello", "world"], - &["hello"], - &["world", "world"], - ])); - let dir = tempfile::tempdir().unwrap(); - index.save(dir.path()).unwrap(); - - let loaded = Bm25Index::load(dir.path()).unwrap(); - assert_eq!(loaded.num_docs(), index.num_docs()); - for q in [ - query(&["hello"]), - query(&["world"]), - query(&["hello", "world"]), - ] { - assert_eq!(loaded.get_scores(&q, None), index.get_scores(&q, None)); - } - } - - #[test] - fn save_writes_documents_and_doc_order() { - let index = Bm25Index::build(&docs(&[&["hello", "hello", "world"]])); - let dir = tempfile::tempdir().unwrap(); - index.save(dir.path()).unwrap(); - - let raw = std::fs::read_to_string(dir.path().join("bm25.json")).unwrap(); - let value: serde_json::Value = serde_json::from_str(&raw).unwrap(); - assert_eq!(value["version"], 2); - assert_eq!(value["documents"]["0"]["hello"], 2); - assert_eq!(value["documents"]["0"]["world"], 1); - assert_eq!(value["docOrder"], serde_json::json!(["0"])); - } - - #[test] - fn load_missing_file_is_err() { - let dir = tempfile::tempdir().unwrap(); - assert!(Bm25Index::load(dir.path()).is_err()); - } - - // --- incremental API (mirrors upstream tests/index/test_bm25.py) --- - - fn build_ids(input: &[(&str, &[&str])]) -> Bm25Index { - let mut index = Bm25Index::new(); - for (chunk_id, tokens) in input { - index.add_document(chunk_id, &query(tokens)).unwrap(); - } - index.set_doc_order(input.iter().map(|(id, _)| id.to_string()).collect()); - index - } - - #[test] - fn scoring_matches_lucene_formula() { - let index = build_ids(&[ - ("a", &["authenticate", "token"]), - ("b", &["login", "password"]), - ]); - let scores = index.get_scores(&query(&["authenticate"]), None); - // idf = ln(1 + 1.5/1.5); tf term = 1·(k1+1) / (1 + k1·(1 − b + b·dl/avgdl)) = 2.5/2.5. - let expected = (1.0f64 + 1.5 / 1.5).ln() as f32; - assert!( - (scores[0] - expected).abs() < 1e-6, - "{} vs {expected}", - scores[0] - ); - assert_eq!(scores[1], 0.0); - } - - #[test] - fn removed_and_unordered_documents_stop_scoring() { - let mut index = build_ids(&[("a", &["authenticate"]), ("b", &["login"])]); - index.remove_document("missing"); - index.set_doc_order(vec!["b".to_string()]); - assert_eq!(index.get_scores(&query(&["authenticate"]), None), vec![0.0]); - - index.remove_document("a"); - index.set_doc_order(vec!["a".to_string(), "b".to_string()]); - assert_eq!( - index.get_scores(&query(&["authenticate"]), None), - vec![0.0, 0.0] - ); - assert_eq!(index.corpus_size(), 1); - assert_eq!(index.num_docs(), 2); - } - - #[test] - fn duplicate_add_document_errors() { - let mut index = build_ids(&[("a", &["x"])]); - let err = index.add_document("a", &query(&["y"])).unwrap_err(); - assert!(err.contains("already indexed")); - } - - #[test] - fn removed_slot_is_recycled_without_stale_postings() { - let mut index = build_ids(&[("a", &["alpha"]), ("b", &["beta"])]); - index.remove_document("a"); - index.add_document("c", &query(&["gamma"])).unwrap(); - index.set_doc_order(vec!["b".to_string(), "c".to_string()]); - assert_eq!(index.get_scores(&query(&["alpha"]), None), vec![0.0, 0.0]); - let gamma = index.get_scores(&query(&["gamma"]), None); - assert_eq!(gamma[0], 0.0); - assert!(gamma[1] > 0.0); - } - - #[test] - fn save_load_preserves_scores_and_doc_order() { - let index = build_ids(&[ - ("empty", &[]), - ("a", &["authenticate", "token"]), - ("b", &["login", "password"]), - ]); - let dir = tempfile::tempdir().unwrap(); - index.save(dir.path()).unwrap(); - - let loaded = Bm25Index::load(dir.path()).unwrap(); - assert_eq!(loaded.doc_order(), index.doc_order()); - assert_eq!( - loaded.get_scores(&query(&["authenticate"]), None), - index.get_scores(&query(&["authenticate"]), None) - ); - } - - #[test] - fn load_rejects_zero_term_frequency() { - let index = build_ids(&[("a", &["authenticate"])]); - let dir = tempfile::tempdir().unwrap(); - index.save(dir.path()).unwrap(); - let path = dir.path().join("bm25.json"); - let mut value: serde_json::Value = - serde_json::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap(); - value["documents"]["a"]["authenticate"] = serde_json::json!(0); - std::fs::write(&path, value.to_string()).unwrap(); - - let err = Bm25Index::load(dir.path()).unwrap_err(); - assert!(err.to_string().contains("must be positive")); - } - - #[test] - fn load_rejects_inconsistent_document_order() { - let index = build_ids(&[("a", &["authenticate"])]); - let dir = tempfile::tempdir().unwrap(); - index.save(dir.path()).unwrap(); - let path = dir.path().join("bm25.json"); - let mut value: serde_json::Value = - serde_json::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap(); - value["docOrder"] = serde_json::json!(["other"]); - std::fs::write(&path, value.to_string()).unwrap(); - - let err = Bm25Index::load(dir.path()).unwrap_err(); - assert!(err.to_string().contains("document state")); - } -} +mod tests; diff --git a/crates/csp/src/indexing/sparse/tests.rs b/crates/csp/src/indexing/sparse/tests.rs new file mode 100644 index 0000000..daeff44 --- /dev/null +++ b/crates/csp/src/indexing/sparse/tests.rs @@ -0,0 +1,290 @@ +use super::*; + +fn chunk(file_path: &str, content: &str) -> Chunk { + Chunk { + content: content.to_string(), + file_path: file_path.to_string(), + start_line: 1, + end_line: 1, + language: None, + } +} + +fn docs(input: &[&[&str]]) -> Vec> { + input + .iter() + .map(|d| d.iter().map(|s| s.to_string()).collect()) + .collect() +} + +fn query(tokens: &[&str]) -> Vec { + tokens.iter().map(|s| s.to_string()).collect() +} + +// --- enrich_for_bm25 (mirrors src/indexing/sparse.test.ts) --- + +#[test] +fn enrich_appends_repeated_stem_and_dir_parts() { + assert_eq!( + enrich_for_bm25(&chunk("src/utils/format.ts", "hello world")), + "hello world format format src utils" + ); +} + +#[test] +fn enrich_trims_to_last_3_dir_parts() { + assert_eq!( + enrich_for_bm25(&chunk("a/b/c/d/foo.py", "x")), + "x foo foo b c d" + ); +} + +#[test] +fn enrich_handles_top_level_file() { + assert_eq!(enrich_for_bm25(&chunk("foo.py", "x")), "x foo foo "); +} + +#[test] +fn enrich_drops_dot_segments() { + assert_eq!( + enrich_for_bm25(&chunk("./a/b/foo.ts", "x")), + "x foo foo a b" + ); +} + +#[test] +fn enrich_normalizes_backslashes() { + assert_eq!( + enrich_for_bm25(&chunk("src\\utils\\format.ts", "hello world")), + "hello world format format src utils" + ); +} + +// --- selector_to_mask --- + +#[test] +fn selector_builds_mask() { + let mask = selector_to_mask(Some(&[0, 2, 5]), 6).unwrap(); + assert_eq!(mask, vec![1, 0, 1, 0, 0, 1]); +} + +#[test] +fn selector_none_returns_none() { + assert_eq!(selector_to_mask(None, 6), None); +} + +#[test] +fn selector_ignores_out_of_bounds() { + let mask = selector_to_mask(Some(&[0, 10]), 3).unwrap(); + assert_eq!(mask, vec![1, 0, 0]); +} + +// --- Bm25Index --- + +#[test] +fn ranks_docs_with_query_term_higher() { + let index = Bm25Index::build(&docs(&[&["hello", "world"], &["hello"], &["world"]])); + let scores = index.get_scores(&query(&["hello"]), None); + assert_eq!(scores.len(), 3); + assert!(scores[0] > 0.0); + assert!(scores[1] > 0.0); + assert_eq!(scores[2], 0.0); +} + +#[test] +fn zero_scores_for_unknown_tokens() { + let index = Bm25Index::build(&docs(&[&["hello"], &["world"]])); + assert_eq!(index.get_scores(&query(&["unknown"]), None), vec![0.0, 0.0]); +} + +#[test] +fn empty_corpus_yields_empty_scores() { + let index = Bm25Index::build(&docs(&[])); + assert_eq!(index.get_scores(&query(&["anything"]), None).len(), 0); +} + +#[test] +fn empty_query_yields_zero_scores() { + let index = Bm25Index::build(&docs(&[&["hello"], &["world"]])); + assert_eq!(index.get_scores(&[], None), vec![0.0, 0.0]); +} + +#[test] +fn weight_mask_zeros_masked_docs() { + let index = Bm25Index::build(&docs(&[&["hello", "world"], &["hello"], &["world"]])); + let scores = index.get_scores(&query(&["hello"]), Some(&[1, 0, 1])); + assert!(scores[0] > 0.0); + assert_eq!(scores[1], 0.0); + assert_eq!(scores[2], 0.0); +} + +#[test] +fn full_mask_matches_baseline() { + let index = Bm25Index::build(&docs(&[&["hello", "world"], &["hello"], &["world"]])); + let baseline = index.get_scores(&query(&["hello"]), None); + let masked = index.get_scores(&query(&["hello"]), Some(&[1, 1, 1])); + assert_eq!(masked, baseline); +} + +#[test] +fn repeated_query_tokens_do_not_compound() { + let index = Bm25Index::build(&docs(&[&["hello"]])); + let single = index.get_scores(&query(&["hello"]), None); + let repeated = index.get_scores(&query(&["hello", "hello", "hello"]), None); + assert_eq!(repeated, single); +} + +// --- save / load (T014) --- + +#[test] +fn save_load_round_trips_scores() { + let index = Bm25Index::build(&docs(&[ + &["hello", "world"], + &["hello"], + &["world", "world"], + ])); + let dir = tempfile::tempdir().unwrap(); + index.save(dir.path()).unwrap(); + + let loaded = Bm25Index::load(dir.path()).unwrap(); + assert_eq!(loaded.num_docs(), index.num_docs()); + for q in [ + query(&["hello"]), + query(&["world"]), + query(&["hello", "world"]), + ] { + assert_eq!(loaded.get_scores(&q, None), index.get_scores(&q, None)); + } +} + +#[test] +fn save_writes_documents_and_doc_order() { + let index = Bm25Index::build(&docs(&[&["hello", "hello", "world"]])); + let dir = tempfile::tempdir().unwrap(); + index.save(dir.path()).unwrap(); + + let raw = std::fs::read_to_string(dir.path().join("bm25.json")).unwrap(); + let value: serde_json::Value = serde_json::from_str(&raw).unwrap(); + assert_eq!(value["version"], 2); + assert_eq!(value["documents"]["0"]["hello"], 2); + assert_eq!(value["documents"]["0"]["world"], 1); + assert_eq!(value["docOrder"], serde_json::json!(["0"])); +} + +#[test] +fn load_missing_file_is_err() { + let dir = tempfile::tempdir().unwrap(); + assert!(Bm25Index::load(dir.path()).is_err()); +} + +// --- incremental API (mirrors upstream tests/index/test_bm25.py) --- + +fn build_ids(input: &[(&str, &[&str])]) -> Bm25Index { + let mut index = Bm25Index::new(); + for (chunk_id, tokens) in input { + index.add_document(chunk_id, &query(tokens)).unwrap(); + } + index.set_doc_order(input.iter().map(|(id, _)| id.to_string()).collect()); + index +} + +#[test] +fn scoring_matches_lucene_formula() { + let index = build_ids(&[ + ("a", &["authenticate", "token"]), + ("b", &["login", "password"]), + ]); + let scores = index.get_scores(&query(&["authenticate"]), None); + // idf = ln(1 + 1.5/1.5); tf term = 1·(k1+1) / (1 + k1·(1 − b + b·dl/avgdl)) = 2.5/2.5. + let expected = (1.0f64 + 1.5 / 1.5).ln() as f32; + assert!( + (scores[0] - expected).abs() < 1e-6, + "{} vs {expected}", + scores[0] + ); + assert_eq!(scores[1], 0.0); +} + +#[test] +fn removed_and_unordered_documents_stop_scoring() { + let mut index = build_ids(&[("a", &["authenticate"]), ("b", &["login"])]); + index.remove_document("missing"); + index.set_doc_order(vec!["b".to_string()]); + assert_eq!(index.get_scores(&query(&["authenticate"]), None), vec![0.0]); + + index.remove_document("a"); + index.set_doc_order(vec!["a".to_string(), "b".to_string()]); + assert_eq!( + index.get_scores(&query(&["authenticate"]), None), + vec![0.0, 0.0] + ); + assert_eq!(index.corpus_size(), 1); + assert_eq!(index.num_docs(), 2); +} + +#[test] +fn duplicate_add_document_errors() { + let mut index = build_ids(&[("a", &["x"])]); + let err = index.add_document("a", &query(&["y"])).unwrap_err(); + assert!(err.contains("already indexed")); +} + +#[test] +fn removed_slot_is_recycled_without_stale_postings() { + let mut index = build_ids(&[("a", &["alpha"]), ("b", &["beta"])]); + index.remove_document("a"); + index.add_document("c", &query(&["gamma"])).unwrap(); + index.set_doc_order(vec!["b".to_string(), "c".to_string()]); + assert_eq!(index.get_scores(&query(&["alpha"]), None), vec![0.0, 0.0]); + let gamma = index.get_scores(&query(&["gamma"]), None); + assert_eq!(gamma[0], 0.0); + assert!(gamma[1] > 0.0); +} + +#[test] +fn save_load_preserves_scores_and_doc_order() { + let index = build_ids(&[ + ("empty", &[]), + ("a", &["authenticate", "token"]), + ("b", &["login", "password"]), + ]); + let dir = tempfile::tempdir().unwrap(); + index.save(dir.path()).unwrap(); + + let loaded = Bm25Index::load(dir.path()).unwrap(); + assert_eq!(loaded.doc_order(), index.doc_order()); + assert_eq!( + loaded.get_scores(&query(&["authenticate"]), None), + index.get_scores(&query(&["authenticate"]), None) + ); +} + +#[test] +fn load_rejects_zero_term_frequency() { + let index = build_ids(&[("a", &["authenticate"])]); + let dir = tempfile::tempdir().unwrap(); + index.save(dir.path()).unwrap(); + let path = dir.path().join("bm25.json"); + let mut value: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap(); + value["documents"]["a"]["authenticate"] = serde_json::json!(0); + std::fs::write(&path, value.to_string()).unwrap(); + + let err = Bm25Index::load(dir.path()).unwrap_err(); + assert!(err.to_string().contains("must be positive")); +} + +#[test] +fn load_rejects_inconsistent_document_order() { + let index = build_ids(&[("a", &["authenticate"])]); + let dir = tempfile::tempdir().unwrap(); + index.save(dir.path()).unwrap(); + let path = dir.path().join("bm25.json"); + let mut value: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap(); + value["docOrder"] = serde_json::json!(["other"]); + std::fs::write(&path, value.to_string()).unwrap(); + + let err = Bm25Index::load(dir.path()).unwrap_err(); + assert!(err.to_string().contains("document state")); +} From bf0b54f717e36451857c095a61087651584c0f75 Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Fri, 4 Sep 2026 21:39:03 +0900 Subject: [PATCH 7/8] perf(index): compare the cached backend dim instead of scanning every row Refs #84 --- crates/csp/src/indexing/cache_orchestrator.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/csp/src/indexing/cache_orchestrator.rs b/crates/csp/src/indexing/cache_orchestrator.rs index 9bee642..3747eba 100644 --- a/crates/csp/src/indexing/cache_orchestrator.rs +++ b/crates/csp/src/indexing/cache_orchestrator.rs @@ -166,15 +166,17 @@ pub(crate) fn load_previous_for_incremental( } let chunks = read_chunks(cache_dir).ok()?; - let vectors = SelectableBasicBackend::load(cache_dir).ok()?.vectors; + let backend = SelectableBasicBackend::load(cache_dir).ok()?; // The persisted rows are concatenated with freshly embedded ones, so a model // whose dimension changed under an unchanged id would make the merged matrix // ragged — and `from_normalized` would then hard-error out of the rebuild - // instead of falling back to it. Fail closed here instead. + // instead of falling back to it. Fail closed here instead. `load` builds + // every row with exactly `dim` elements, so the backend's dim covers them all. let (query_model, _) = crate::indexing::dense::load_model(Some(expected_model)); - if vectors.iter().any(|row| row.len() != query_model.dim()) { + if !backend.vectors.is_empty() && backend.dim != query_model.dim() { return None; } + let vectors = backend.vectors; let bm25_index = Bm25Index::load(cache_dir).ok()?; PreviousIndex::try_new(chunks, vectors, manifest.files, bm25_index).ok() } From b0853c50ea05991d57f17e46ac82ca09ae6d3d8c Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Fri, 4 Sep 2026 21:39:54 +0900 Subject: [PATCH 8/8] test(index): build the manifest key with the platform separator Refs #84 --- crates/csp/src/indexing/create/tests.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/csp/src/indexing/create/tests.rs b/crates/csp/src/indexing/create/tests.rs index 6d55fdf..efd4e45 100644 --- a/crates/csp/src/indexing/create/tests.rs +++ b/crates/csp/src/indexing/create/tests.rs @@ -240,8 +240,13 @@ fn zero_chunk_file_does_not_break_manifest_tiling() { ); let model = make_stub_model(4); let result = create_index_from_path(&root, &opts(&model, Some(root.clone())), None).unwrap(); - assert_eq!(result.files["pkg/z.ts"].count, 0); - assert_eq!(result.files["pkg/z.ts"].start, result.files["pkg.ts"].start); + // `display_path` keeps the platform separator, so build the key with `join`. + let zero_path = Path::new("pkg").join("z.ts").to_string_lossy().into_owned(); + assert_eq!(result.files[zero_path.as_str()].count, 0); + assert_eq!( + result.files[zero_path.as_str()].start, + result.files["pkg.ts"].start + ); // A freshly built index must always be a valid seed for the next pass. into_previous(result); }