Multi-Repository Shared Database Support & Branch Isolation - #7
Open
dtor wants to merge 37 commits into
Open
Conversation
insert_lore_emails() returns Ok(()) unconditionally, even when individual merge_insert calls fail in the chunked fallback path. The pipeline inserter treats Ok(()) as full success: it records every commit SHA from the batch in lore_indexed_commits and credits the full batch size to the inserted counter. Emails that failed to insert are therefore marked as indexed and filtered out on all subsequent runs. Because reconcile_lore_indexed_commits() runs only during specific schema migrations, the orphaned SHA entries are never cleaned up in normal operation. Over time the set of processable new commits shrinks to near zero. Change insert_lore_emails() to return the indices of emails that could not be stored. The pipeline inserter now excludes failed emails when recording indexed commit SHAs, so they remain eligible for retry on the next run. The inserted counter reflects actual insertions. Fixes: 01b9399 ("semcode: add --lore for email indexing (database schema change)") Signed-off-by: Chuck Lever <cel@kernel.org>
Compact, Prune, and OptimizeAction::Index each create new manifest versions that drop FTS index references, destroying full-text search capability until the next semcode-index --lore rebuild. Skip the lore table entirely during optimize_single_table and perform only a checkout_latest to release stale handles. Fixes: 01b9399 ("semcode: add --lore for email indexing (database schema change)") Signed-off-by: Chuck Lever <cel@kernel.org>
Lance 3.0.1's merge_insert resolves columns by position during its internal join/write phase rather than by name. The lore table's on-disk schema has date_timestamp at position 10 (appended by the add_columns migration), but insert_lore_emails builds its RecordBatch with date_timestamp at position 3. The resulting column misalignment causes nullable column values (in_reply_to) to appear in non-nullable slots (subject), producing a spurious "field subject contained null values" error on every insert. table.add() resolves columns by name and is not affected by column-order differences between the insert batch and the on-disk schema. Replace merge_insert with table.add(), and add a filter_existing_lore_ids() check that queries the message_id BTree index before inserting to prevent duplicates from partial-failure retries. This is also cheaper than merge_insert, which performs a full read-modify-write cycle on every call. Fixes: 391b9d2 ("update deps to fix lance recursion_limit build failure") Signed-off-by: Chuck Lever <cel@kernel.org>
FTS pattern normalization strips all non-alphanumeric characters, so purely symbolic regexes like ".*" produce an empty FTS query string. An empty FTS query returns zero candidates, and the regex post-filter never gets a chance to match, causing the search to silently return no results. When the normalized FTS pattern is empty, bypass FTS and issue a plain table scan with date filtering, allowing the regex post-filter to operate on the full candidate set. Both search_lore_emails (single-field path) and query_field_impl (multi-field intersection path) are affected. Fixes: 01b9399 ("semcode: add --lore for email indexing (database schema change)") Signed-off-by: Chuck Lever <cel@kernel.org>
The MCP host process controls the semcode-mcp command line, so there is no way to pass a -d flag to override the database location. An environment variable is the only mechanism available for directing semcode-mcp to a database that lives outside the current working directory. This also enables semcode to share the same database (and thus the same lore archives and source code commits) across multiple copies of the same source code base, avoiding the storage gigabytes of data. Add SEMCODE_DB to the database path resolution order, between the -d flag and the source-dir / current-directory fallbacks. The variable receives the same path normalization as the flag: a path ending in .semcode.db is used as-is, an existing directory gets .semcode.db appended, and anything else is taken literally. The change is contained in process_database_path(), so all binaries that call it (semcode-index, semcode, semcode-mcp) pick up the new behavior without modification. semcode-lsp resolves its database path before calling that function, so it receives an explicit SEMCODE_DB check in its own resolution chain. Signed-off-by: Chuck Lever <cel@kernel.org>
Commit b66cb3c replaced rev_walk + with_hidden with a first-parent walk to avoid gix 0.81's slow graph painting. However, the first-parent walk only stops when it encounters the exact boundary commit ID. When the range start is a tag (e.g., v7.0-rc6) that was merged rather than being on the first-parent chain, the walk never encounters it and traverses the entire repository history. For the linux kernel with v7.0-rc6..HEAD, this produced 75,454 commits instead of 397, causing every file to be re-processed across all commits and inflating the indexing time from ~92 seconds to 3+ hours. Restore rev_walk + with_hidden which correctly computes the set difference regardless of graph topology. Fixes: b66cb3c (fix startup performance regression from gix 0.81 upgrade) Signed-off-by: Chris Mason <clm@meta.com>
Add tests that create a temporary git repo with a side branch and merge commit, then verify list_shas_in_range correctly handles: - Tags on side branches (not on the first-parent chain) - Range boundaries on the first-parent chain - No-range mode (all reachable commits) The tag-on-side-branch test specifically catches the regression from b66cb3c where a first-parent-only walk would miss the boundary and traverse the entire history. Signed-off-by: Chris Mason <clm@meta.com>
tower-lsp is unmaintained (no updates in 3 years). Migrate to the community fork tower-lsp-server, which brings lsp-types 0.97/ls-types, native async traits (no more async_trait), and fluent-uri based Uri. Key changes: - tower_lsp -> tower_lsp_server, lsp_types -> ls_types - Drop #[tower_lsp::async_trait] (RPITIT in 0.21+) - url::Url -> ls_types::Uri with inherent to_file_path/from_file_path - Prefer workspace_folders over deprecated root_uri - Add offset_encoding field to InitializeResult (new in ls-types) - Remove url crate dependency (no longer needed) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Chris Mason <clm@meta.com>
Signed-off-by: Chris Mason <clm@meta.com>
Signed-off-by: Chris Mason <clm@meta.com>
The lore search multi-field intersection had three bugs that caused queries to return wrong or empty results: 1. Multiple patterns for the same field (e.g., two from_patterns) were joined into a single string with spaces and passed as one FTS query + one regex. Both FTS and regex treated the combined string as an AND (all terms must match), not OR as documented. Fix by querying each pattern independently and taking the set union. 2. The result limit was applied before date filtering. Since HashSet iteration order is arbitrary, taking the first N message_ids before checking dates caused recent emails to be missed when the candidate set was large. 3. Date filtering used SQL string comparison on RFC 2822 date strings via only_if(), which produces lexicographic rather than temporal ordering (e.g., "Wed" > "Tue" alphabetically). Fix by selecting the date column in the FTS query and comparing parsed DateTime values in the post-filter loop alongside the regex check. 4. The new date_timestamp column was added to the lore table schema but existing databases were not migrated. merge_insert of 11-column batches into a 10-column table silently failed, and the fallback logic in insert_lore_emails returned Ok(()) after skipping individual rows, so commit SHAs were recorded as indexed while no email data was stored. Add a migration step that creates the column via add_columns and reconciles lore_indexed_commits by purging SHAs whose emails were never persisted. 5. The vector search path (vlore_similar_emails) used get_column() for date_timestamp, which returns an error when the column is absent. semcode-mcp runs the schema migration in a background task, so a query arriving before migration completes hits this path on pre-migration databases. Treat the column as optional and fall back to parsing the RFC 2822 date string, matching the approach used in the non-vector query path.
The vector-based lore search (vlore) applied the result limit before date filtering: intersection_ids was truncated to `limit` entries before passing them to fetch_emails_by_ids where date checks occurred. Because intersection order is arbitrary, recent emails could be excluded when the candidate set was large, producing short result sets. Push date filtering into query_lore_fields_intersection, matching the approach already used in the non-vector lore search path (connection.rs). Each per-field FTS query now selects the "date" column and filters candidates by parsed DateTime comparison in the post-processing loop, so the intersection set is already date-bounded before the limit is applied. Remove the dead date_filter SQL string construction that was built but never used in any only_if() clause. Fix clippy warnings: replace map_or(false, ...) with is_some_and() in both connection.rs and search.rs, and suppress too_many_arguments on the intersection functions whose signatures grew with the new date parameters.
Every lore indexing run dropped and rebuilt all five FTS indices from scratch, even when only a handful of new emails were inserted. On large archives the rebuild dominated the total indexing time. LanceDB's native FTS engine (lance >= 0.19.2) supports incremental updates: newly-inserted rows are served via a brute-force fallback at query time, then merged into the inverted index structure by OptimizeAction::Index. The bug where OptimizeAction::Index destroyed FTS index references was fixed in January 2025 and lance 3.0.1 is well past that fix. Split the indexing path into ensure + optimize: ensure_lore_fts_indices() creates the five indices only when they are absent (first run or after --clear), and optimize_lore_fts_indices() merges unindexed rows into the existing indices. The full drop-and-rebuild path remains available for schema migrations. Remove the lore-table skip in optimize_single_table() since OptimizeAction::Index no longer destroys FTS references.
Signed-off-by: Chris Mason <clm@meta.com>
Signed-off-by: Venkatesh Srinivas <venkateshs@chromium.org>
NomicBertModel is missing get_input_embeddings/set_input_embeddings,
which are required by model2vec's distilation pipeline. Patch in the
two methods.
Before:
...
HuggingFace tokenizer defines a pad_token, but the Skeletoken model does not. Setting it to '<pad>'.
Full traceback of the error:
Traceback (most recent call last):
File "/home/vsrinivas/WORK/semcode/./scripts/nomic2vec.py", line 501, in main
m2v = distill_from_model(
model=model,
...<3 lines>...
device=args.device
)
File "/home/vsrinivas/semcode-vectors2/lib/python3.13/site-packages/model2vec/distill/distillation.py", line 107, in distill_from_model
model = reshape_embeddings(model, original_tokenizer_model)
File "/home/vsrinivas/semcode-vectors2/lib/python3.13/site-packages/skeletoken/external/transformers.py", line 58, in reshape_embeddings
embedding = model.get_input_embeddings()
File "/home/vsrinivas/semcode-vectors2/lib/python3.13/site-packages/transformers/modeling_utils.py", line 1036, in get_input_embeddings
raise NotImplementedError(
f"`get_input_embeddings` not auto‑handled for {self.__class__.__name__}; please override in the subclass."
)
NotImplementedError: `get_input_embeddings` not auto‑handled for NomicBertModel; please override in the subclass.
After:
HuggingFace tokenizer defines a pad_token, but the Skeletoken model does not. Setting it to '<pad>'.
Encoding tokens: 100%|████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 249999/249999 [2:12:08<00:00, 31.53 tokens/s]
✓ Saved Model2Vec static model to /home/vsrinivas/WORK/semcode/nomic_v2_m2v
Fixing tokenizer configuration for semcode compatibility...
Adding [UNK] token to vocabulary
Set unk_id to 1 for [UNK] token
✓ Updated tokenizer configuration
✓ Updated tokenizer_config.json
⚠ Verification warning: Number of tokens (250000) does not match number of vectors (249999). Please provide a token mapping or ensure the number of tokens matches the number of vectors.
The model was saved successfully but may need additional configuration for some tools
All done. The static model is ready for high‑throughput CPU embedding.
Assisted-by: Claude:gemma-4-26B-A4B
Signed-off-by: Venkatesh Srinivas <venkateshs@chromium.org>
Add SEMCODE_GIT_REPO environment variable support for specifying the git repository path in semcode-mcp and semcode. This allows the MCP host process to provide the repository location via the environment when command line arguments cannot be easily modified. The priority order is: 1. --git-repo command line argument 2. SEMCODE_GIT_REPO environment variable 3. Current directory (default) Enable the env feature for clap in Cargo.toml to support the env attribute in the Args struct. Add test code to verify that the environment variable is correctly supported and respects the priority order. Signed-off-by: Guenter Roeck <linux@roeck-us.net>
"cargo fmt" adjusted a bit of code that was added by commit 37f4b7a ("switch LSP server from tower-lsp 0.20 to tower-lsp-server 0.23").
Commit 8ac9f79 ("lore: Use incremental FTS index updates instead of full rebuilds") removed an early-return guard in optimize_single_table() that previously skipped the lore table entirely. The guard had been documented as protecting FTS index references, and that protection was no longer needed once ensure_lore_fts_indices() + optimize_lore_fts_indices() became the canonical FTS update path. Removing the guard exposed a different, previously-dormant issue: compaction of the 290k-row lore table now runs on every --lore invocation. lance/index/append.rs:merge_indices() opens every delta index fragment for a column before merging any of them, and for the scalar/FTS path indices_merged is hard-coded to 1, so the num_indices_to_merge option has no effect on how many fragments are touched per call. On a host with enough memory the cost is acceptable. On a 6GB system the resident set grows linearly with the per-column fragment count, bleeds into swap, and the OOM killer eventually terminates semcode-index. The run then leaves behind fresh delta fragments that the next run will also have to walk, so the problem is monotonic. Two paths now reach the expensive merge_indices walk: 1. compact_lore_tables() -> optimize_single_table("lore"), which runs Compact + Prune + Index (step 3 is the index optimize that walks all fragments). 2. optimize_lore_fts_indices() called directly from the --lore pipeline after compact_lore_tables() returns. Restore the early-return skip in optimize_single_table() for the lore table so path (1) is a no-op again, and guard optimize_lore_fts_indices() with a _indices/ fragment-count threshold so path (2) bails out cleanly when the backlog is already pathologically large. Query correctness is preserved in both cases: LanceDB's native FTS engine serves unindexed rows via a brute-force fallback, so searches still return correct results while compaction is deferred to a host with enough memory to complete it. Fixes: 8ac9f79 ("lore: Use incremental FTS index updates instead of full rebuilds")
After inserting emails, the --lore handler called optimize_database() which runs compact_and_cleanup() across every table in the database — functions, types, 16 content shards, and several metadata tables. When a database already contains a code index, those tables carry thousands of fragments with full function bodies and type definitions. Compacting them loads hundreds of megabytes of data that the lore run never modified, and on a 6 GB system the combined working set triggers the OOM killer. Add compact_lore_tables() which processes only the lore and lore_indexed_commits tables, sequentially, and call it from both --lore code paths instead of optimize_database(). Peak memory during post-pipeline cleanup is now proportional to the lore data alone.
Adds the model2vec [distill] extra to scripts/requirements.txt so nomic2vec.py's distillation imports resolve. Signed-off-by: Chris Mason <clm@meta.com>
Monkey-patches get_input_embeddings/set_input_embeddings onto NomicBertModel so model2vec's distill_from_model pipeline runs. Signed-off-by: Chris Mason <clm@meta.com>
Adds SEMCODE_GIT_REPO env var support to semcode and semcode-mcp via clap's env attribute, with arg > env > default priority. Signed-off-by: Chris Mason <clm@meta.com>
The three #[test] fns introduced for SEMCODE_GIT_REPO each mutate the
process-wide env var. cargo runs tests in a binary in parallel by
default, so the three tests interleave their set_var/remove_var calls
and 2 of 3 fail under `cargo test`:
test tests::test_git_repo_default ... FAILED
test tests::test_git_repo_env_var ... FAILED
test tests::test_git_repo_priority ... ok
Combine the three cases into a single test fn so all env-var mutation
happens on one thread, restoring `cargo test` (and the pre-commit /
pre-push hooks that wrap it).
Signed-off-by: Chris Mason <clm@meta.com>
clippy under -D warnings flags `clippy::items-after-test-module` because
the #[cfg(test)] mod tests block introduced for SEMCODE_GIT_REPO sits
mid-file with non-test items below it:
error: items after a test module
--> src/bin/query.rs:86:1
Move the tests module to the bottom of the file. No functional change
intended.
Signed-off-by: Chris Mason <clm@meta.com>
`cargo fmt --check` flags this multi-line method-chain call in SemcodeLspBackend::initialize as malformed; rustfmt collapses it onto a single line. Apply the formatting so the pre-commit hook's `cargo fmt --check` passes. No functional change intended. Signed-off-by: Chris Mason <clm@meta.com>
Limits --lore post-pipeline compaction to the lore tables and adds fragment-count guards that skip the compaction paths whose peak memory OOM-kills semcode-index on small hosts. Signed-off-by: Chris Mason <clm@meta.com>
Refresh Cargo.lock with latest compatible versions of all dependencies. Build, clippy, and tests all pass after the update. Signed-off-by: Chris Mason <clm@meta.com>
Signed-off-by: Chris Mason <clm@meta.com>
… config
When unit tests in `src/indexer.rs` instantiate temporary Git
repositories via `std::process::Command::new("git")`, global or system
Git configurations (such as custom ref templates or hooks) can bleed into
test execution and produce unexpected ref structures like `.invalid`.
Set `GIT_CONFIG_GLOBAL=/dev/null` and `GIT_CONFIG_SYSTEM=/dev/null` in the
`git()` test helper command invocation to ensure test execution is fully
isolated from the host environment.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Repository paths referenced in database schemas must be consistent across operating systems and independent of individual user home directory mount points (e.g. /home/user vs /usr/local/google/home/user). Introduce `normalize_repo_path()` in `src/git.rs` to: - Convert backslashes (`\`) to forward slashes (`/`) for cross-platform OS compatibility. - Trim `$HOME` directory prefixes to `~` when the path resides under the user's home directory. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Linked git worktrees created via `git worktree add` share the same commit history and branch ref namespaces as their parent repository. If worktrees are treated as separate repository paths in database tables, branches get redundantly indexed for each worktree. Introduce `get_repo_root()` in `src/git.rs` to extract `repo.common_dir()` from `gix::Repository`. This ensures standard checkouts, bare repositories, and linked worktrees all resolve to their single underlying common Git directory identity key. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
A database can be operated in two modes: Local (embedded in a repo directory) or Shared/External (e.g. central ~/.cache/semcode.db). To select between position-independent NULL repo paths (Local) vs scoped repository paths (Shared), the connection initialization layer must classify the target database path. Introduce `determine_db_locality()` in `src/git.rs` to: - Collect canonical root directories for standard repos, active linked worktrees, and bare repositories. - Handle symlinked database targets and follow symlink targets even when the DB file does not exist yet. - Canonicalize non-existent database paths safely via a multi-level ancestor walk loop. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Branch store query routines must distinguish between records belonging to
different repositories in shared database mode while matching NULL
repo_path records when in local database mode.
Update `IndexedBranchStore` in `src/database/branches.rs` and
`DatabaseManager` in `src/database/connection.rs`:
- Add nullable `repo_path: Option<String>` to `IndexedBranchInfo` and
`IndexedBranchInfoJson`.
- Store `repo_identity` and `is_local_db` inside `IndexedBranchStore`.
- Introduce `build_sql_filter()` to automatically supply the default
SQL filter for query operations:
- Local DB mode: `(repo_path = '{repo_identity}' OR repo_path IS NULL)`
- Shared DB mode: `repo_path = '{repo_identity}'`
- Order branch query results by `indexed_at DESC` so the newest indexed
record is returned when both a legacy NULL record and a repo-scoped
record exist.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
In shared database mode, repositories deleted from the local filesystem
leave behind stale `indexed_branches` records matching
`repo_path = '{deleted_path}'`.
Implement `DatabaseManager::garbage_collect()` and `GcStats`:
- Query distinct `repo_path` values where `repo_path IS NOT NULL`.
- Expand `~/` home directory path prefixes using `dirs::home_dir()`.
- Purge stale records from deleted repositories via LanceDB
`table.delete()`.
- Purge un-scoped legacy `repo_path IS NULL` records when the database
directory resides outside all Git repositories on disk (standalone
database mode).
- Invoke `garbage_collect()` automatically at the end of indexing in
`src/bin/index.rs`.
- Return `GcStats` with count of deleted branch records.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Document Semcode's shared database architecture, detailing multi-repository co-indexing, global content deduplication vs. repo-scoped branch isolation, database locality detection (local vs. shared), Git worktree consolidation, and automatic garbage collection. Update docs/schema.md to include the repo_path column and BTree index on the indexed_branches table. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
NOTE: You need to sync your main branch from the main repo and then I can rebase as well to only include relevant commits.
Summary & Motivation
When working on large projects like the Linux kernel, developers frequently maintain multiple repository checkouts (e.g.
linux-mainline,bpf-next,net-next,stable) or multiple linked Git worktrees (git worktree add ...). Since repositories share a lot of the same objects it is very tempting to use a shared database with them.Previously,
semcodestoredindexed_brancheswithout repository identity scoping. When indexing multiple repositories into a single shared central database (SEMCODE_DB=~/.cache/semcode.db), branch names likemain,master, orworkcollided, polluting branch lookups across repositories.This PR improves support for multiple repositories indexed in the same database:
indexed_branches) per repository viarepo_pathfiltering, preventing branch name collisions between trees..semcode.dbinside a repository folder).Architectural Highlights
1. Repository Identity Resolution & Worktree Consolidation
git::get_repo_root(): Resolves any repository directory, subfolder, bare repo, or linked worktree to its canonical repository identity usingcommon_dir(.git)..gitcommit graph as their parent repository, transparently sharing the same branch index without creating duplicate worktree records.git::normalize_repo_path): Replaces absolute home directory prefixes (/home/user,/usr/local/google/home/user) with~/, ensuring repository identity keys are portable across machines with different home mount paths.2. Dual Database Locality Modes (
determine_db_locality).git/or.semcode.db):(repo_path = '{repo_identity}' OR repo_path IS NULL).~/.cache/semcode.dbor external path):repo_path = '{repo_identity}'.3. On-Demand Lazy Schema Migration
IndexedBranchStore::record_branch_indexed()lazily adds therepo_pathcolumn and its scalar BTree index on the first write operation.4. Automatic Garbage Collection (
DatabaseManager::garbage_collect)semcode-indexruns.indexed_branchesrecords for repository paths that no longer exist on the local filesystem.NULLbranch records in standalone database mode.Documentation & Test Coverage
indexed_branchestable schema specification and indices.git::tests::test_normalize_repo_pathgit::tests::test_determine_db_locality_*(local, external, bare repo, worktree)database::branches::tests::test_shared_db_branch_isolationdatabase::branches::tests::test_local_db_matches_null_and_activedatabase::branches::tests::test_garbage_collect_null_records_in_standalone_dbdatabase::branches::tests::test_read_old_indexed_branches_table_does_not_alter_schema