diff --git a/src/lemoncrow/pro/capabilities/code_context/engine.py b/src/lemoncrow/pro/capabilities/code_context/engine.py index c42d54a0b..f72401c75 100644 --- a/src/lemoncrow/pro/capabilities/code_context/engine.py +++ b/src/lemoncrow/pro/capabilities/code_context/engine.py @@ -173,6 +173,69 @@ def _query_is_natural_language(query: str) -> bool: _MAX_FILE_BYTES = 1_000_000 +# file_line_fts.rowid is (files.rowid << _LINE_ROWID_BITS) | line, so one file's +# line rows occupy one contiguous rowid range and a reindex drops them with a +# single range delete instead of scanning the whole table on the UNINDEXED +# file_path. Every line number has to fit in the low bits, which a file capped at +# _MAX_FILE_BYTES bytes -- hence at most that many lines -- always does. +_LINE_ROWID_BITS = 20 +_LINE_ROWID_MASK = (1 << _LINE_ROWID_BITS) - 1 +assert _MAX_FILE_BYTES <= _LINE_ROWID_MASK, "_LINE_ROWID_BITS cannot address every line of a _MAX_FILE_BYTES file" +# The one definition of each FTS5 table's shape. `_init_schema`, the fts.sqlite +# bootstrap in `_init_secondary_schemas_locked` and the drop-and-recreate full +# rebuild in `_index_repo_unsafe` all build their DDL from here, so a column, +# tokenizer or prefix change reaches every one of them -- a rebuild that spelled +# out its own DDL could silently recreate a table `_schema_current` then rejects. +_FTS_TABLE_BODY: dict[str, str] = { + # prefix indexes turn the "term"* prefix-channel MATCH from a term-range scan + + # doclist merge into direct doclist seeks for prefixes of 2-6 chars (the bulk of + # what _fts_prefix_query_from_terms emits after IDF pruning). Results are + # identical; cost is index size. _schema_current greps the stored SQL for + # `prefix=`, so dropping it here also re-triggers that migration. + "symbol_fts": ( + "fts5(symbol_id UNINDEXED, name, qualified_name, signature," " file_path UNINDEXED, source, prefix='2 3 4 5 6')" + ), + # Read-only term->document-frequency view over symbol_fts (zero write cost, + # auto-maintained). Powers IDF pruning of common query tokens. + "symbol_fts_vocab": "fts5vocab(symbol_fts, 'row')", + # Trigram tokenizer handles substring matching natively; signature is omitted + # (5x size amplification, and symbol_fts already covers it). + "symbol_trigram": "fts5(symbol_id UNINDEXED, name, qualified_name, file_path, tokenize='trigram')", + # One row per FILE (not per symbol like symbol_trigram): the path channel in + # _search_symbols_local matches path patterns against this much smaller table + # (file-cardinality: thousands even on a huge repo) and joins back to symbols via + # idx_symbols_repo_file, instead of scanning symbol_trigram's one-row-per-symbol + # duplication of every file's path. + "file_path_trigram": "fts5(repo_id UNINDEXED, file_path, tokenize='trigram')", + "file_line_fts": "fts5(repo_id UNINDEXED, file_path UNINDEXED, line UNINDEXED, text)", +} + + +def _fts_create_sql(table: str, *, schema: str = "", if_not_exists: bool = False) -> str: + """``CREATE VIRTUAL TABLE`` for one FTS5 table, from the single definition of its shape.""" + exists = "IF NOT EXISTS " if if_not_exists else "" + qualified = f"{schema}.{table}" if schema else table + return f"CREATE VIRTUAL TABLE {exists}{qualified} USING {_FTS_TABLE_BODY[table]}" + + +def _rowid_tables_dense(conn: sqlite3.Connection) -> bool: + """True when ``files`` and ``symbols`` occupy rowids 1..N with no gaps. + + Both are rowid tables without an ``INTEGER PRIMARY KEY``, and SQLite is free to + renumber such a table's rowids during ``VACUUM`` -- while the FTS5 shadow tables + keyed on those rowids (``_apply_file_data_batch``) keep theirs, because every + shadow table does have one. The renumbering is the identity map only while the + rowids are already dense, which is exactly the state a full rebuild leaves + behind. Anywhere else a VACUUM would silently repoint every FTS row at a + different file or symbol, so the caller skips it: the cost is disk, not data. + """ + for table in ("files", "symbols"): + row = conn.execute(f"SELECT COUNT(*), COALESCE(MAX(rowid), 0) FROM {table}").fetchone() + if row is None or int(row[0]) != int(row[1]): + return False + return True + + logger = logging.getLogger(__name__) @@ -343,7 +406,8 @@ def close(self) -> None: _LINEAGE_INDEX_VERSION = 2 # Bump when source selection or symbol/text extraction semantics change in a way # an incremental mtime/hash check cannot see for unchanged files. -_CODE_INDEXER_SEMANTICS_VERSION = 2 +# 3: FTS5 rows carry explicit rowids derived from files.rowid / symbols.rowid. +_CODE_INDEXER_SEMANTICS_VERSION = 3 _LINEAGE_DEFAULT_SCORE_PENALTY = 0.1 # --- File-watcher constants --- @@ -3985,8 +4049,23 @@ def _apply_file_data_batch( conn: sqlite3.Connection, results: list[_FileIndexData], ) -> None: - """Batch-insert all extracted data using ``executemany`` (single writer).""" + """Batch-insert all extracted data using ``executemany`` (single writer). + + Every FTS5 row is written with an explicit rowid taken from the regular + table it mirrors, which is what lets :meth:`_delete_files_index` find it + again without a full-table scan. That makes the rowid a key: exactly one + FTS row may exist per ``files`` / ``symbols`` row, so a path or symbol_id + repeated inside one batch is written once. + """ # --- files --- + seen_rels: set[str] = set() + deduped: list[_FileIndexData] = [] + for d in results: + if d.rel in seen_rels: + continue + seen_rels.add(d.rel) + deduped.append(d) + results = deduped conn.executemany( """ INSERT INTO files(repo_id, file_path, language, content_hash, size_bytes, mtime_ns, indexed_at) @@ -4000,6 +4079,14 @@ def _apply_file_data_batch( """, [(self.repo_id, d.rel, d.language, d.content_hash, d.size_bytes, d.mtime_ns) for d in results], ) + file_rowids: dict[str, int] = { + str(row["file_path"]): int(row["rowid"]) + for row in conn.execute( + "SELECT rowid, file_path FROM files " + "WHERE repo_id = ? AND file_path IN (SELECT value FROM json_each(?))", + (self.repo_id, json.dumps(sorted(seen_rels))), + ) + } # --- symbols + FTS --- symbol_rows: list[ @@ -4023,10 +4110,18 @@ def _apply_file_data_batch( ] = [] fts_rows: list[tuple[str, str, str, str, str, str]] = [] trigram_rows: list[tuple[str, str, str, str]] = [] # (symbol_id, name_plain, qualified_name, file_path) + seen_symbol_ids: set[str] = set() for d in results: for i, sym in enumerate(d.symbols): raw_id = f"{self.repo_id}:{d.rel}:{sym.qualified_name}:{sym.start_byte}:{d.content_hash}" sid = hashlib.sha256(raw_id.encode("utf-8")).hexdigest()[:24] + if sid in seen_symbol_ids: + # Two extracted symbols collapsing to one symbol_id (same file, + # qualified name and start byte) yield one `symbols` row under + # INSERT OR IGNORE, hence one rowid -- and a second FTS row for + # that rowid would be a duplicate key, not a second symbol. + continue + seen_symbol_ids.add(sid) symbol_rows.append( ( sid, @@ -4068,25 +4163,46 @@ def _apply_file_data_batch( """, symbol_rows, ) + symbol_rowids: dict[str, int] = { + str(row["symbol_id"]): int(row["rowid"]) + for row in conn.execute( + "SELECT rowid, symbol_id FROM symbols WHERE symbol_id IN (SELECT value FROM json_each(?))", + (json.dumps(sorted(seen_symbol_ids)),), + ) + } conn.executemany( - "INSERT INTO symbol_fts(symbol_id, name, qualified_name, signature, file_path, source) VALUES (?, ?, ?, ?, ?, ?)", - fts_rows, + "INSERT INTO symbol_fts(rowid, symbol_id, name, qualified_name, signature, file_path, source) " + "VALUES (?, ?, ?, ?, ?, ?, ?)", + [(symbol_rowids[r[0]], r[0], r[1], r[2], r[3], r[4], r[5]) for r in fts_rows if r[0] in symbol_rowids], ) conn.executemany( - "INSERT INTO symbol_trigram(symbol_id, name, qualified_name, file_path) VALUES (?, ?, ?, ?)", - trigram_rows, + "INSERT INTO symbol_trigram(rowid, symbol_id, name, qualified_name, file_path) VALUES (?, ?, ?, ?, ?)", + [(symbol_rowids[r[0]], r[0], r[1], r[2], r[3]) for r in trigram_rows if r[0] in symbol_rowids], ) conn.executemany( - "INSERT INTO file_path_trigram(repo_id, file_path) VALUES (?, ?)", - [(self.repo_id, d.rel) for d in results], + "INSERT INTO file_path_trigram(rowid, repo_id, file_path) VALUES (?, ?, ?)", + [(file_rowids[d.rel], self.repo_id, d.rel) for d in results if d.rel in file_rowids], ) # --- line text + FTS --- - line_rows: list[tuple[str, str, int, str]] = [] + line_rows: list[tuple[int, str, str, int, str]] = [] for d in results: - line_rows.extend((self.repo_id, d.rel, line_no, text) for line_no, text in d.text_lines if text.strip()) + base = file_rowids.get(d.rel) + if base is None: + continue + base <<= _LINE_ROWID_BITS + for line_no, text in d.text_lines: + if not text.strip(): + continue + if line_no > _LINE_ROWID_MASK: + # Past the rowid range reserved for this file; packing it anyway + # would write into the next file's range. Only reachable if a file + # grew past _MAX_FILE_BYTES between its stat and its read. + logger.warning("context_engine: %s exceeds %d lines; indexing the first", d.rel, _LINE_ROWID_MASK) + break + line_rows.append((base | line_no, self.repo_id, d.rel, line_no, text)) conn.executemany( - "INSERT INTO file_line_fts(repo_id, file_path, line, text) VALUES (?, ?, ?, ?)", + "INSERT INTO file_line_fts(rowid, repo_id, file_path, line, text) VALUES (?, ?, ?, ?, ?)", line_rows, ) @@ -4270,15 +4386,32 @@ def _index_repo_unsafe( with self._connect() as conn: self._init_schema(conn) - if not force and self._stored_indexer_semantics_version(conn) != _CODE_INDEXER_SEMANTICS_VERSION: + if not force and not self._rowid_scheme_trustworthy(conn): force = True if force: # --- Full rebuild: wipe everything, then parallel-extract + batch-write --- - conn.execute("DELETE FROM file_line_fts") - conn.execute("DELETE FROM symbol_fts") - conn.execute("DELETE FROM symbol_trigram") - conn.execute("DELETE FROM file_path_trigram") + # DROP + CREATE rather than DELETE: emptying file_line_fts row by row + # is 12.4M deletes on a large repo. Python's sqlite3 opens a + # transaction implicitly for DML but never for DDL, so without an + # explicit BEGIN the first DROP would commit on its own and show every + # reader an empty index for the length of the rebuild. + if not conn.in_transaction: + conn.execute("BEGIN") + # file_line_fts lives in the attached fts database; the rest in main. + # symbol_fts_vocab is an fts5vocab view over symbol_fts, so it drops + # first and is recreated last -- hence the reversed second pass. + _rebuilt = ( + ("fts", "file_line_fts"), + ("main", "symbol_fts_vocab"), + ("main", "symbol_fts"), + ("main", "symbol_trigram"), + ("main", "file_path_trigram"), + ) + for _schema, _fts_table in _rebuilt: + conn.execute(f"DROP TABLE IF EXISTS {_schema}.{_fts_table}") + for _schema, _fts_table in reversed(_rebuilt): + conn.execute(_fts_create_sql(_fts_table, schema=_schema)) conn.execute("DELETE FROM symbols") conn.execute("DELETE FROM imports") conn.execute('DELETE FROM "references"') @@ -4454,7 +4587,13 @@ def _index_repo_unsafe( if _vac_db.exists(): with contextlib.suppress(Exception): _vc = sqlite3.connect(str(_vac_db)) - _vc.execute("VACUUM") + # VACUUM renumbers the rowids of a table with no INTEGER + # PRIMARY KEY, and `files`/`symbols` rowids are what every FTS5 + # row is keyed on. Dense after the rebuild that just ran, that + # renumbering is the identity map; sparse, it would repoint + # every FTS row at another file, so leave the free pages be. + if _vac_db != self.db_path or _rowid_tables_dense(_vc): + _vc.execute("VACUUM") _vc.close() with self._connect() as conn: @@ -4497,6 +4636,39 @@ def _stored_indexer_semantics_version(self, conn: sqlite3.Connection) -> int | N except (TypeError, ValueError): return None + def _rowid_scheme_trustworthy(self, conn: sqlite3.Connection) -> bool: + """True when this index's FTS5 rowids still identify the rows they mirror. + + ``_apply_file_data_batch`` keys every FTS row on a ``files``/``symbols`` rowid + and ``_delete_files_index`` finds it again by that rowid alone, so both sides + have to come from the same index generation. Two states break that, and the + drop-and-recreate rebuild this returning False forces heals both: an index + written before the scheme existed (a stale stored semantics version), and a + schema migration that empties ``files`` while leaving the FTS tables loaded + (``_init_schema``'s pre-prefix symbol_fts migration and + ``_init_secondary_schemas_locked``'s references/call_edges reshape both do). + In the second state the rowids handed back to the repopulated ``files`` rows + are the ones the surviving FTS rows already occupy, so the deletes no-op and + the inserts collide -- and ``_stored_indexer_semantics_version`` cannot see it, + because an emptied ``files`` table reads exactly like a never-indexed one. + """ + if self._stored_indexer_semantics_version(conn) != _CODE_INDEXER_SEMANTICS_VERSION: + return False + if conn.execute("SELECT 1 FROM files WHERE repo_id = ? LIMIT 1", (self.repo_id,)).fetchone() is not None: + return True + # Only the tables keyed on files.rowid can collide: those migrations leave + # `symbols`, and so the symbol_fts/symbol_trigram rowids, intact. The probe is + # scoped to this repo because a shared db_path holds other repos' rows too, and + # the rebuild a False forces wipes every repo's index, not just this one's. + for table in ("file_path_trigram", "fts.file_line_fts"): + with contextlib.suppress(sqlite3.Error): + if ( + conn.execute(f"SELECT 1 FROM {table} WHERE repo_id = ? LIMIT 1", (self.repo_id,)).fetchone() + is not None + ): + return False + return True + def _stamp_indexer_semantics_version(self, conn: sqlite3.Connection) -> None: conn.execute( """ @@ -4509,26 +4681,44 @@ def _stamp_indexer_semantics_version(self, conn: sqlite3.Connection) -> None: def _delete_files_index(self, conn: sqlite3.Connection, rels: list[str]) -> None: """Remove every indexed row for the files *rels*, in one pass per table. - The FTS5 tables are keyed by rowid alone -- ``file_path`` and ``symbol_id`` - are UNINDEXED columns -- so any delete filtered on them scans the whole - table. Issued once per file, that was ~2 s per file on a 12M-line index: - an incremental run after a pull (hundreds of changed files) took longer - than the autosync subprocess's 600 s timeout, was killed before it - committed, and the next run started over. Filtering every table on the - whole batch at once costs one scan per FTS table per run. The regular - tables are indexed on these columns, so the batch costs them nothing. + ``file_path`` and ``symbol_id`` are UNINDEXED columns of the FTS5 tables, so + a delete filtered on either scans the whole table -- ~2 s per scan on a + 12M-line index. Issued once per changed file, that made an incremental run + after a pull outlive the autosync subprocess's 600 s timeout: it was killed + before it committed, and the next run started over. + Every FTS row is therefore written under a rowid taken from the regular table + it mirrors (``_apply_file_data_batch``), which the regular tables index on + ``file_path``, so each delete below is a rowid seek. EXPLAIN QUERY PLAN on a + 12M-line index: ``SCAN symbol_fts VIRTUAL TABLE INDEX 0:=`` for the rowid IN + form and ``0:><`` for the file_line_fts range, against a constraint-free + ``0:`` for the file_path filter they replace. """ if not rels: return batch = json.dumps(rels) paths = "SELECT value FROM json_each(?)" symbols = f"SELECT symbol_id FROM symbols WHERE repo_id = ? AND file_path IN ({paths})" - conn.execute(f"DELETE FROM file_line_fts WHERE repo_id = ? AND file_path IN ({paths})", (self.repo_id, batch)) - conn.execute( - f"DELETE FROM file_path_trigram WHERE repo_id = ? AND file_path IN ({paths})", (self.repo_id, batch) + # Read the rowids before the regular tables lose the rows that carry them. + file_rowids = [ + int(row[0]) + for row in conn.execute( + f"SELECT rowid FROM files WHERE repo_id = ? AND file_path IN ({paths})", (self.repo_id, batch) + ) + ] + symbol_rowids = json.dumps( + [ + int(row[0]) + for row in conn.execute( + f"SELECT rowid FROM symbols WHERE repo_id = ? AND file_path IN ({paths})", (self.repo_id, batch) + ) + ] ) - conn.execute(f"DELETE FROM symbol_trigram WHERE symbol_id IN ({symbols})", (self.repo_id, batch)) - conn.execute(f"DELETE FROM symbol_fts WHERE symbol_id IN ({symbols})", (self.repo_id, batch)) + for file_rowid in file_rowids: + low = file_rowid << _LINE_ROWID_BITS + conn.execute("DELETE FROM file_line_fts WHERE rowid BETWEEN ? AND ?", (low, low | _LINE_ROWID_MASK)) + conn.execute(f"DELETE FROM file_path_trigram WHERE rowid IN ({paths})", (json.dumps(file_rowids),)) + conn.execute(f"DELETE FROM symbol_trigram WHERE rowid IN ({paths})", (symbol_rowids,)) + conn.execute(f"DELETE FROM symbol_fts WHERE rowid IN ({paths})", (symbol_rowids,)) # Prune persisted embeddings for these files' symbols *before* the symbols # themselves. symbol_id encodes the file content hash, so an edited or # removed file yields fresh ids -- without this the old vectors orphan @@ -10998,14 +11188,7 @@ def _init_secondary_schemas_locked(self) -> None: self.fts_db_path.parent.mkdir(parents=True, exist_ok=True) with sqlite3.connect(self.fts_db_path, timeout=30.0) as fc: fc.execute("PRAGMA journal_mode = WAL") - fc.executescript(""" - CREATE VIRTUAL TABLE IF NOT EXISTS file_line_fts USING fts5( - repo_id UNINDEXED, - file_path UNINDEXED, - line UNINDEXED, - text - ); - """) + fc.execute(_fts_create_sql("file_line_fts", if_not_exists=True)) # Recompute AFTER the DDL: the files may have just been created, so the # pre-DDL identity (with (-1, -1) placeholders) must not be cached. @@ -11293,40 +11476,6 @@ def _init_schema(self, conn: sqlite3.Connection) -> None: doc_summary TEXT, content_hash TEXT NOT NULL ); - -- prefix indexes turn the "term"* prefix-channel MATCH from a - -- term-range scan + doclist merge into direct doclist seeks for - -- prefixes of 2-6 chars (the bulk of what _fts_prefix_query_from_terms - -- emits after IDF pruning). Results are identical; cost is index size. - CREATE VIRTUAL TABLE IF NOT EXISTS symbol_fts USING fts5( - symbol_id UNINDEXED, - name, - qualified_name, - signature, - file_path UNINDEXED, - source, - prefix='2 3 4 5 6' - ); - -- Read-only term->document-frequency view over the FTS index (zero write - -- cost, auto-maintained). Powers IDF pruning of common query tokens. - CREATE VIRTUAL TABLE IF NOT EXISTS symbol_fts_vocab USING fts5vocab(symbol_fts, 'row'); - CREATE VIRTUAL TABLE IF NOT EXISTS symbol_trigram USING fts5( - symbol_id UNINDEXED, - name, - qualified_name, - file_path, - tokenize='trigram' - ); - -- One row per FILE (not per symbol like symbol_trigram): the path - -- channel in _search_symbols_local matches path patterns against this - -- much smaller table (file-cardinality: thousands even on a huge repo) - -- and joins back to symbols via idx_symbols_repo_file, instead of - -- scanning symbol_trigram's one-row-per-symbol duplication of every - -- file's path. - CREATE VIRTUAL TABLE IF NOT EXISTS file_path_trigram USING fts5( - repo_id UNINDEXED, - file_path, - tokenize='trigram' - ); CREATE INDEX IF NOT EXISTS idx_symbols_repo_name_nocase ON symbols(repo_id, symbol_name COLLATE NOCASE); CREATE INDEX IF NOT EXISTS idx_symbols_repo_qual_nocase @@ -11362,6 +11511,8 @@ def _init_schema(self, conn: sqlite3.Connection) -> None: CREATE INDEX IF NOT EXISTS idx_commit_author_date ON commit_chunks(author_date); CREATE INDEX IF NOT EXISTS idx_commit_files ON commit_chunks(files_touched); """) + for _fts_table in ("symbol_fts", "symbol_fts_vocab", "symbol_trigram", "file_path_trigram"): + conn.execute(_fts_create_sql(_fts_table, if_not_exists=True)) # Migration: older DBs predate the files.mtime_ns column used to fast-skip # unchanged files during incremental reindex. CREATE TABLE IF NOT EXISTS # never adds a column to an existing table, so add it here when absent. @@ -11376,25 +11527,22 @@ def _init_schema(self, conn: sqlite3.Connection) -> None: _trig_cols = {str(row[1]) for row in conn.execute("PRAGMA table_info(symbol_trigram)")} if "signature" in _trig_cols: conn.execute("DROP TABLE IF EXISTS symbol_trigram") - conn.execute( - "CREATE VIRTUAL TABLE symbol_trigram USING fts5(" - " symbol_id UNINDEXED, name, qualified_name, file_path," - " tokenize='trigram')" - ) + conn.execute(_fts_create_sql("symbol_trigram")) # Backfill the substring trigram index for DBs built before it existed, so the # substring/path channels use the index instead of full-scanning symbols. if conn.execute("SELECT 1 FROM symbol_trigram LIMIT 1").fetchone() is None: if conn.execute("SELECT 1 FROM symbols LIMIT 1").fetchone() is not None: conn.execute( - "INSERT INTO symbol_trigram(symbol_id, name, qualified_name, file_path) " - "SELECT symbol_id, symbol_name, qualified_name, file_path FROM symbols" + "INSERT INTO symbol_trigram(rowid, symbol_id, name, qualified_name, file_path) " + "SELECT rowid, symbol_id, symbol_name, qualified_name, file_path FROM symbols" ) # Backfill the file-level path trigram index for DBs built before it existed # (see _search_symbols_local's path channel). if conn.execute("SELECT 1 FROM file_path_trigram LIMIT 1").fetchone() is None: if conn.execute("SELECT 1 FROM files LIMIT 1").fetchone() is not None: conn.execute( - "INSERT INTO file_path_trigram(repo_id, file_path) SELECT DISTINCT repo_id, file_path FROM files" + "INSERT INTO file_path_trigram(rowid, repo_id, file_path) " + "SELECT rowid, repo_id, file_path FROM files" ) # Migration: symbol_fts built before the prefix indexes existed. FTS5 cannot # add prefix indexes to an existing table, so drop and recreate it empty @@ -11407,12 +11555,12 @@ def _init_schema(self, conn: sqlite3.Connection) -> None: if _fts_sql_row is not None and "prefix=" not in str(_fts_sql_row[0] or ""): conn.execute("DROP TABLE IF EXISTS symbol_fts_vocab") conn.execute("DROP TABLE IF EXISTS symbol_fts") - conn.execute( - "CREATE VIRTUAL TABLE symbol_fts USING fts5(" - " symbol_id UNINDEXED, name, qualified_name, signature," - " file_path UNINDEXED, source, prefix='2 3 4 5 6')" - ) - conn.execute("CREATE VIRTUAL TABLE symbol_fts_vocab USING fts5vocab(symbol_fts, 'row')") + conn.execute(_fts_create_sql("symbol_fts")) + conn.execute(_fts_create_sql("symbol_fts_vocab")) + # `files` is emptied so the change detector sees every file as new, which + # leaves file_path_trigram / file_line_fts holding rows under rowids the + # repopulated `files` rows will be handed again. _rowid_scheme_trustworthy + # recognises that state and forces the full rebuild that clears them. conn.execute("DELETE FROM files") conn.execute("INSERT OR IGNORE INTO engine_state(key, value) VALUES ('index_version', '0')") # Self-heal DBs built before planner statistics were collected: with data @@ -13494,6 +13642,12 @@ def _reindex_locked() -> None: return with self._connect() as conn: self._init_schema(conn) + if not self._rowid_scheme_trustworthy(conn): + # The stored index's FTS rowids mean nothing: deleting by one + # would drop another symbol's row, and inserting at one would + # collide. The full rebuild this state forces in + # _index_repo_unsafe picks these files up instead. + return self._delete_files_index(conn, rels) results = ( self._parallel_extract(existing_paths, total=len(existing_paths)) if existing_paths else [] diff --git a/tests/core/test_code_context.py b/tests/core/test_code_context.py index 2e103cce9..f837eb4ea 100644 --- a/tests/core/test_code_context.py +++ b/tests/core/test_code_context.py @@ -19,6 +19,7 @@ CallGraphNode, traverse_call_graph, ) +from lemoncrow.pro.capabilities.code_context.engine import _CODE_INDEXER_SEMANTICS_VERSION from lemoncrow.pro.capabilities.code_context.models import SymbolRecord, TextMatch from lemoncrow.pro.capabilities.code_context.output_policy import TRUNCATION_MARKER from lemoncrow.pro.code_intel.cross_lang.runner import CrossLangRunner @@ -369,7 +370,7 @@ def test_incremental_index_forces_rebuild_when_indexer_semantics_version_changes with sqlite3.connect(db_path) as conn: row = conn.execute("SELECT value FROM engine_state WHERE key = 'indexer_semantics_version'").fetchone() assert row is not None - assert int(row[0]) == 2 + assert int(row[0]) == _CODE_INDEXER_SEMANTICS_VERSION def test_search_symbols_refreshes_stale_line_numbers_after_external_edit(tmp_path: Path) -> None: @@ -2404,11 +2405,62 @@ def count(sql: str, *args: object) -> int: assert count("SELECT COUNT(*) FROM file_line_fts WHERE file_path = ?", untouched) == untouched_lines -def test_incremental_index_scans_each_fts_table_once_per_run(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: - """The FTS5 tables are rowid-keyed, so a delete filtered on file_path or symbol_id - scans the whole table. Issued per file, an incremental run cost one full scan per - changed file -- ~2 s each on a 12M-line index -- and a run after a pull outlived - the autosync subprocess's 600 s timeout, so it never committed. +_FTS_DELETE_BY_CONTENT_KEY = re.compile(r"\b(file_path|symbol_id)\b") + + +def _assert_every_fts_row_is_keyed_to_the_row_it_mirrors(engine: CodeContextEngine) -> None: + """Each FTS5 rowid identifies the ``files``/``symbols`` row the FTS row describes. + + This is the invariant the rowid-keyed deletes rely on: break it and a reindex + drops some other file's rows and leaves its own behind. + """ + from lemoncrow.pro.capabilities.code_context.engine import _LINE_ROWID_BITS, _LINE_ROWID_MASK + + with engine._connect() as conn: + + def count(sql: str) -> int: + return int(conn.execute(sql).fetchone()[0]) + + symbols = count("SELECT COUNT(*) FROM symbols") + files = count("SELECT COUNT(*) FROM files") + assert symbols > 0 and files > 0 + for table in ("symbol_fts", "symbol_trigram"): + assert count(f"SELECT COUNT(*) FROM {table}") == symbols + assert ( + count( + f"SELECT COUNT(*) FROM {table} t JOIN symbols s ON s.rowid = t.rowid " + "WHERE s.symbol_id = t.symbol_id" + ) + == symbols + ) + assert count("SELECT COUNT(*) FROM file_path_trigram") == files + assert ( + count( + "SELECT COUNT(*) FROM file_path_trigram t JOIN files f ON f.rowid = t.rowid " + "WHERE f.repo_id = t.repo_id AND f.file_path = t.file_path" + ) + == files + ) + lines = count("SELECT COUNT(*) FROM file_line_fts") + assert lines > 0 + assert ( + count( + "SELECT COUNT(*) FROM file_line_fts l " + f"JOIN files f ON f.rowid = (l.rowid >> {_LINE_ROWID_BITS}) " + f"WHERE f.file_path = l.file_path AND (l.rowid & {_LINE_ROWID_MASK}) = l.line" + ) + == lines + ) + + +def test_reindexes_delete_fts_rows_by_rowid_not_by_file_path_or_symbol_id( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """``file_path`` and ``symbol_id`` are UNINDEXED in every FTS5 table, so a delete + filtered on either scans the table whole -- ~2 s per run on a 12M-line index, and + an incremental run after a pull outlived the autosync subprocess's 600 s timeout, + was killed before it committed, and the next run started over. Both the batched + incremental run and the single-file reindex must seek by rowid instead. """ _write_fixture_repo(tmp_path) for i in range(6): @@ -2431,8 +2483,256 @@ def traced_connect(*args: object, **kwargs: object) -> sqlite3.Connection: stats = engine.index_repo(force=False) assert stats.files_indexed == 6 - deletes = [m.group(1) for s in statements if (m := _FTS_DELETE.match(s))] - assert sorted(deletes) == sorted(_FTS_TABLES), f"7 stale files cost {len(deletes)} FTS scans" + scans = [s for s in statements if _FTS_DELETE.match(s) and _FTS_DELETE_BY_CONTENT_KEY.search(s)] + assert [s for s in statements if _FTS_DELETE.match(s)], "the run issued no FTS delete at all" + assert scans == [], f"7 stale files cost {len(scans)} FTS scans" + + statements.clear() + orders = tmp_path / "src" / "orders.py" + orders.write_text("class RenamedService:\n pass\n", encoding="utf-8") + engine._reindex_files([str(orders)]) + + assert [s for s in statements if _FTS_DELETE.match(s)], "the single-file reindex issued no FTS delete at all" + assert [s for s in statements if _FTS_DELETE.match(s) and _FTS_DELETE_BY_CONTENT_KEY.search(s)] == [] + _assert_every_fts_row_is_keyed_to_the_row_it_mirrors(engine) + + +def test_fts_rowids_key_their_tables_after_a_full_and_an_incremental_index(tmp_path: Path) -> None: + _write_fixture_repo(tmp_path) + engine = CodeContextEngine(tmp_path, db_path=tmp_path / "code.sqlite", autosync_enabled=False) + engine.index_repo(force=True) + _assert_every_fts_row_is_keyed_to_the_row_it_mirrors(engine) + + (tmp_path / "src" / "orders.py").write_text("class RenamedService:\n pass\n", encoding="utf-8") + (tmp_path / "src" / "checkout.py").unlink() + (tmp_path / "src" / "added.py").write_text("def added() -> int:\n return 1\n", encoding="utf-8") + engine.index_repo(force=False) + _assert_every_fts_row_is_keyed_to_the_row_it_mirrors(engine) + + assert [h.qualified_name for h in engine.search_symbols("RenamedService", limit=5)] + assert [h.qualified_name for h in engine.search_symbols("added", limit=5)] + + +def test_a_symbol_id_repeated_in_one_batch_gets_exactly_one_fts_row(tmp_path: Path) -> None: + """``symbols`` keeps one row per symbol_id (INSERT OR IGNORE) and the FTS tables are + keyed on that row's rowid, so a second FTS row for the same id is not a second + symbol -- it is a duplicate key. + """ + from lemoncrow.pro.capabilities.code_context.engine import _ExtractedSymbol, _FileIndexData + + sym = _ExtractedSymbol( + name="dup", + qualified_name="dup", + kind="function", + signature="def dup() -> int", + start_byte=0, + end_byte=24, + start_line=1, + end_line=2, + ) + source = "def dup() -> int:\n return 1\n" + data = _FileIndexData( + rel="src/dup.py", + language="python", + content_hash="deadbeef", + size_bytes=len(source), + text_lines=[(1, "def dup() -> int:"), (2, " return 1")], + symbols=[sym, sym], + symbol_sources=[source, source], + imports=[], + references=[], + call_edges=[], + mtime_ns=1, + ) + engine = CodeContextEngine(tmp_path, db_path=tmp_path / "code.sqlite", autosync_enabled=False) + with engine._connect() as conn: + engine._init_schema(conn) + engine._apply_file_data_batch(conn, [data, data]) + + def count(sql: str) -> int: + return int(conn.execute(sql).fetchone()[0]) + + assert count("SELECT COUNT(*) FROM symbols") == 1 + assert count("SELECT COUNT(*) FROM symbol_fts") == 1 + assert count("SELECT COUNT(*) FROM symbol_trigram") == 1 + assert count("SELECT COUNT(*) FROM files") == 1 + assert count("SELECT COUNT(*) FROM file_path_trigram") == 1 + assert count("SELECT COUNT(*) FROM file_line_fts") == 2 + _assert_every_fts_row_is_keyed_to_the_row_it_mirrors(engine) + + +def test_an_index_at_the_previous_semantics_version_is_rebuilt_into_the_rowid_scheme(tmp_path: Path) -> None: + """Existing workspaces carry FTS rowids that mean nothing, so the version bump has + to rebuild them before anything deletes or inserts by rowid. + """ + _write_fixture_repo(tmp_path) + engine = CodeContextEngine(tmp_path, db_path=tmp_path / "code.sqlite", autosync_enabled=False) + first = engine.index_repo(force=True) + + with engine._connect() as conn: + conn.execute("UPDATE engine_state SET value = '2' WHERE key = 'indexer_semantics_version'") + for table in ("symbol_fts", "symbol_trigram", "file_path_trigram"): + conn.execute(f"UPDATE {table} SET rowid = rowid + 10000") + conn.execute("UPDATE file_line_fts SET rowid = rowid + 10000") + conn.commit() + with pytest.raises(AssertionError): + _assert_every_fts_row_is_keyed_to_the_row_it_mirrors(engine) + + # A single-file reindex must not touch a stale-scheme index: its rowid deletes + # would hit other files' rows. The pending rebuild picks the file up instead. + version_before = engine._current_index_version() + (tmp_path / "src" / "orders.py").write_text("class RenamedService:\n pass\n", encoding="utf-8") + engine._reindex_files([str(tmp_path / "src" / "orders.py")]) + assert engine._current_index_version() == version_before + + rebuilt = engine.index_repo(force=False) + + assert rebuilt.files_indexed == first.files_indexed + with engine._connect() as conn: + stored = conn.execute("SELECT value FROM engine_state WHERE key = 'indexer_semantics_version'").fetchone() + assert int(stored[0]) == _CODE_INDEXER_SEMANTICS_VERSION + _assert_every_fts_row_is_keyed_to_the_row_it_mirrors(engine) + assert [h.qualified_name for h in engine.search_symbols("RenamedService", limit=5)] + + +def test_a_migration_that_empties_files_rebuilds_instead_of_colliding_on_rowid(tmp_path: Path) -> None: + """Two schema migrations clear `files` and leave the FTS tables loaded so a normal + reindex repopulates them. The rowids handed back to the repopulated `files` rows + are the ones the surviving FTS rows already occupy: the rowid deletes match + nothing and the rowid inserts collide. An emptied `files` table also reads exactly + like a never-indexed one, so the stored semantics version cannot catch this. + """ + _write_fixture_repo(tmp_path) + engine = CodeContextEngine(tmp_path, db_path=tmp_path / "code.sqlite", autosync_enabled=False) + first = engine.index_repo(force=True) + + with engine._connect() as conn: + conn.execute("DELETE FROM engine_state WHERE key = 'indexer_semantics_version'") + conn.execute("DELETE FROM files") + conn.commit() + for table in ("file_path_trigram", "file_line_fts", "symbol_trigram"): + assert int(conn.execute(f"SELECT COUNT(*) FROM {table}").fetchone()[0]) > 0 + + recovered = engine.index_repo(force=False) + + assert recovered.files_indexed == first.files_indexed + _assert_every_fts_row_is_keyed_to_the_row_it_mirrors(engine) + assert [h.qualified_name for h in engine.search_symbols("OrderService", limit=5)] + + +def test_a_new_repo_indexed_into_a_shared_db_leaves_the_other_repos_index_intact(tmp_path: Path) -> None: + """`files` being empty for *this* repo while the FTS tables hold rows is the + migration signature only when those rows are this repo's. A second repo's first + index into a db_path another repo already filled must stay incremental: the full + rebuild wipes every repo's rows, not just its own. + """ + repo_a = tmp_path / "repo_a" + repo_b = tmp_path / "repo_b" + repo_a.mkdir() + (repo_b / "src").mkdir(parents=True) + _write_fixture_repo(repo_a) + (repo_b / "src" / "inventory.py").write_text( + "class InventoryLedger:\n def reserve(self) -> int:\n return 1\n", encoding="utf-8" + ) + db_path = tmp_path / "shared.sqlite" + engine_a = CodeContextEngine(repo_a, db_path=db_path, autosync_enabled=False) + engine_b = CodeContextEngine(repo_b, db_path=db_path, autosync_enabled=False) + assert engine_a.repo_id != engine_b.repo_id + + def a_rows() -> dict[str, int]: + with engine_a._connect() as conn: + + def count(sql: str) -> int: + return int(conn.execute(sql, (engine_a.repo_id,)).fetchone()[0]) + + return { + "files": count("SELECT COUNT(*) FROM files WHERE repo_id = ?"), + "symbols": count("SELECT COUNT(*) FROM symbols WHERE repo_id = ?"), + "symbol_fts": count( + "SELECT COUNT(*) FROM symbol_fts t JOIN symbols s ON s.rowid = t.rowid WHERE s.repo_id = ?" + ), + "file_path_trigram": count("SELECT COUNT(*) FROM file_path_trigram WHERE repo_id = ?"), + "file_line_fts": count("SELECT COUNT(*) FROM file_line_fts WHERE repo_id = ?"), + "symbol_match": count( + "SELECT COUNT(*) FROM symbol_fts t JOIN symbols s ON s.rowid = t.rowid " + "WHERE symbol_fts MATCH 'OrderService' AND s.repo_id = ?" + ), + "line_match": count( + "SELECT COUNT(*) FROM file_line_fts WHERE file_line_fts MATCH 'calculate_total' AND repo_id = ?" + ), + } + + engine_a.index_repo(force=False) + before = a_rows() + assert all(before.values()), before + + indexed_b = engine_b.index_repo(force=False) + + assert indexed_b.files_indexed == 1 + assert a_rows() == before + _assert_every_fts_row_is_keyed_to_the_row_it_mirrors(engine_a) + assert [h.qualified_name for h in engine_a.search_symbols("OrderService", limit=5)] + assert [h.qualified_name for h in engine_b.search_symbols("InventoryLedger", limit=5)] + + +def test_the_full_rebuild_recreates_the_fts_tables_exactly_as_the_schema_defines_them(tmp_path: Path) -> None: + """The rebuild drops and recreates the FTS5 tables. A column, tokenizer or prefix + set that drifts from `_init_schema`'s would be rebuilt into an index every later + `_schema_current` probe rejects, with nothing to re-migrate it. + """ + _write_fixture_repo(tmp_path) + engine = CodeContextEngine(tmp_path, db_path=tmp_path / "code.sqlite", autosync_enabled=False) + engine.index_repo(force=False) + + def fts_shapes() -> dict[str, str]: + with engine._connect() as conn: + rows = conn.execute( + "SELECT name, sql FROM main.sqlite_master WHERE sql LIKE 'CREATE VIRTUAL%' " + "UNION ALL SELECT name, sql FROM fts.sqlite_master WHERE sql LIKE 'CREATE VIRTUAL%'" + ).fetchall() + # Compare from USING onward, so a schema qualifier or IF NOT EXISTS does not + # read as a shape change; whitespace around the punctuation likewise. + shapes = {} + for name, sql in rows: + body = re.sub(r"\s+", " ", str(sql)[str(sql).index(" USING ") :]).strip() + shapes[str(name)] = body.replace("( ", "(").replace(" )", ")").replace(" ,", ",") + return shapes + + before = fts_shapes() + assert set(before) == {"symbol_fts", "symbol_fts_vocab", "symbol_trigram", "file_path_trigram", "file_line_fts"} + + engine.index_repo(force=True) + + assert fts_shapes() == before + + +def test_the_post_rebuild_vacuum_is_skipped_once_the_rowids_it_would_renumber_go_sparse(tmp_path: Path) -> None: + """`files`/`symbols` are rowid tables with no INTEGER PRIMARY KEY, so VACUUM may + renumber them while the FTS5 shadow tables keyed on those rowids keep theirs. That + renumbering is the identity map only while the rowids are dense -- the state a + full rebuild leaves behind, and the only state the VACUUM may run in. + """ + from lemoncrow.pro.capabilities.code_context.engine import _rowid_tables_dense + + _write_fixture_repo(tmp_path) + engine = CodeContextEngine(tmp_path, db_path=tmp_path / "code.sqlite", autosync_enabled=False) + engine.index_repo(force=True) + with engine._connect() as conn: + assert _rowid_tables_dense(conn) + first_indexed = str(conn.execute("SELECT file_path FROM files ORDER BY rowid LIMIT 1").fetchone()[0]) + + # Removing the lowest-rowid file leaves a hole at rowid 1, so COUNT no longer + # equals MAX(rowid) however the remaining files were numbered. + (tmp_path / first_indexed).unlink() + engine.index_repo(force=False) + with engine._connect() as conn: + assert not _rowid_tables_dense(conn) + + engine.index_repo(force=True) + + with engine._connect() as conn: + assert _rowid_tables_dense(conn) + _assert_every_fts_row_is_keyed_to_the_row_it_mirrors(engine) def test_dir_cached_relpath_resolves_exactly_like_safe_relpath(tmp_path: Path) -> None: