Skip to content

ISS-10710: key the code index's FTS rows by rowid; fast atomic rebuild - #31

Merged
wongk merged 4 commits into
mainfrom
campaign-iss-10710-20260919-c3
Sep 21, 2026
Merged

wongk merged 4 commits into
mainfrom
campaign-iss-10710-20260919-c3

Conversation

@wongk

@wongk wongk commented Sep 21, 2026

Copy link
Copy Markdown

Campaign iss-10710-20260919, chunk C3 of 3 (last): delete full-text rows by rowid; make the full rebuild fast and atomic

Lands PLN-2069 PR 3 for ISS-10710. Based on main, which already includes C1 (#29) and C2 (#30). Not stacked.

What changes

The code index's four FTS5 tables (symbol_fts, symbol_trigram, file_path_trigram, fts.file_line_fts) could only find a file's rows by scanning the whole table, because file_path and symbol_id are UNINDEXED columns. Reindexing even one file cost about 1.4–2 s on symphony-alpha (12.4M indexed lines). Now:

  • Every FTS row carries an explicit rowid taken from the regular table it mirrors:

    • symbol_fts and symbol_trigram use the symbol's symbols.rowid;
    • file_path_trigram uses the file's files.rowid;
    • file_line_fts uses (files.rowid << 20) | line. _MAX_FILE_BYTES caps a file at 1,000,000 lines, under 2^20, and an import-time assertion ties the two together.

    So a file's rows are deleted directly by rowid.

  • The full rebuild drops and recreates the FTS tables instead of deleting 12M rows, inside one transaction opened with an explicit BEGIN. Readers keep answering from the previous index for the whole rebuild (fix(code-intel): serve code tools from the last committed index during a reindex #27).

  • _CODE_INDEXER_SEMANTICS_VERSION is now 3, so an existing workspace rebuilds into the new scheme once. Until it does, a per-file reindex refuses to write, because deleting by rowid on an old-scheme index would remove another symbol's row.

  • A guard detects a migration that empties files while leaving FTS rows behind, and forces the same rebuild. It is scoped to this repo's own rows. Such a state would otherwise make the new rowids collide.

Files: src/lemoncrow/pro/capabilities/code_context/engine.py, tests/core/test_code_context.py.

Numbers (copy-on-write clones of symphony-alpha's index)

Before After
Single-file reindex, same 5 files 1.39–1.54 s 68–252 ms
Single-file reindex, 20-file sample median 62 ms; 19 of 20 under 200 ms
Full-table wipe inside a full rebuild 45.1 s 19.1 s
Full rebuild (17,683 files, 558,521 symbols) 173 s

The one file over 200 ms (208 ms) spends 137 ms in tree-sitter parsing, which this change doesn't touch; its index write is 71 ms. The full rebuild takes 29% of the 600 s autosync timeout, so the one-time upgrade rebuild is left to autosync and the installer is unchanged.

Acceptance ledger: 5 criteria, 5 met

Criterion Evidence
AC-3.1 single-file reindex under 200 ms The numbers above. One of 20 sampled files is 8 ms over, for the parsing reason given there.
AC-3.2 no FTS delete filtered on file_path/symbol_id test_reindexes_delete_fts_rows_by_rowid_not_by_file_path_or_symbol_id traces the SQL of an incremental run and a single-file reindex. Reverting any delete to a column filter fails it.
AC-3.3 full rebuild under 600 s, or run by the installer 173 s, so it is left to autosync.
AC-3.4 readers keep answering during a full rebuild test_reads_during_reindex.py passes unedited, including the full-rebuild case. It fails if the explicit BEGIN is removed.
AC-3.5 retrieval results unchanged The 6 retrieval suites (53 tests) and all 89 pre-existing tests in test_code_context.py pass unedited.

Deviations from PLN-2069 PR 3

  1. A guard in _reindex_files, outside the functions the plan named. It is the only writer that never checked the semantics version, so on an old-scheme index a rowid delete would have removed another symbol's row.
  2. The batch is deduplicated by file_path as well as symbol_id. Two entries with the same file in one batch would collide on the files.rowid-keyed tables; the campaign required a collision to be impossible, not merely unlikely.
  3. The loop was created without repository fields, because ClosedLoop rejected closedloop-ai/lemoncrow with REPO_NOT_IN_PROJECT_POOL. This has no effect on the code.

Review: two rounds

Round 1: /code-review:deep at 51da8133 returned NEEDS_ATTENTION, with 5 verified findings (3 HIGH, 2 MEDIUM). All were fixed in d8af2ea7:

  • Two HIGH and one MEDIUM shared one cause: a migration that empties files left FTS rows behind, the version probe reported "current", and the new rowid inserts collided with an IntegrityError that never healed. The new _rowid_scheme_trustworthy detects that state and forces the rebuild. Regression test: test_a_migration_that_empties_files_rebuilds_instead_of_colliding_on_rowid, which failed with sqlite3.IntegrityError before the fix.
  • HIGH: the rebuild restated the FTS5 DDL that _init_schema owns. The table shapes are now defined once (_FTS_TABLE_BODY, _fts_create_sql()), with a drift test.
  • MEDIUM: VACUUM could renumber files/symbols rowids out of sync with the FTS tables. Instead of migrating both tables to INTEGER PRIMARY KEY, the post-rebuild VACUUM is skipped once those rowids go sparse, with a test.

Round 2: the one campaign re-review (--since-last-review at d8af2ea7) confirmed all five fixed, and found:

  • HIGH, introduced by the round-1 fix: the guard probed FTS rows from any repo. In a database shared by two repos (the same --db-path), a second repo's first index would force a rebuild that dropped the first repo's rows. No default setup shares a database. Fixed in fc907828: the probe is scoped to this repo's file_path_trigram and fts.file_line_fts rows. Regression test: test_a_new_repo_indexed_into_a_shared_db_leaves_the_other_repos_index_intact, which failed before the fix.
  • By the operator's decision, this last fix was not reviewed a third time by the review fleet. It is covered by its regression test, the full code-index suites, and this PR's review.
  • Two MEDIUMs are recorded as follow-ups, not fixed here:
    • the drop and create order of symbol_fts and its vocab view is still written out in three places;
    • the rowid invariant is enforced by the two callers rather than by the methods that rely on it.

Verification

All pass:

  • tests/core/test_code_context.py (98 tests)
  • tests/infra/code_intel (386 tests)
  • the 6 retrieval suites (53 tests)
  • tests/test_mypyc_compile_safety.py
  • mypy on engine.py
  • ruff
  • git diff --check
  • the pre-commit hook on every commit

The full suite was not run locally; CI runs it here.

Rollout note

After this merges and is installed, each workspace's index rebuilds once on its next autosync pass: about 3 minutes for symphony-alpha. Code search keeps answering throughout, marked as refreshing (#27). Nothing is owed by later chunks: this is the campaign's last.

wongk and others added 4 commits September 21, 2026 08:40
…ebuild

PLN-2069 PR 3.

Every FTS5 row now carries an explicit rowid taken from the regular table it
mirrors -- symbols.rowid for symbol_fts and symbol_trigram, files.rowid for
file_path_trigram, and (files.rowid << 20) | line for file_line_fts -- so a
reindex deletes a file's rows by rowid seek instead of scanning each table on
its UNINDEXED file_path/symbol_id. A single-file reindex of symphony-alpha's
index drops from ~1.4 s to a 62 ms median.

The full rebuild drops and recreates the four FTS tables instead of emptying
them row by row (45.1 s -> 19.1 s for 4.5M line rows), inside a transaction
opened with an explicit BEGIN: Python's sqlite3 does not open one for DDL, and
an auto-committed DROP would show every reader an empty index for the length of
the rebuild.

_CODE_INDEXER_SEMANTICS_VERSION goes to 3 so existing workspaces rebuild into
the new scheme once. Until that rebuild runs, a single-file reindex returns
without writing: the stored FTS rowids mean nothing, so deleting by one would
drop another symbol's row.

Co-Authored-By: Claude <noreply@anthropic.com>
The pre-prefix symbol_fts migration and the references/call_edges reshape
both empty `files` and leave the FTS tables loaded, and an emptied `files`
table reads like a never-indexed one -- so the semantics-version guard never
fired, the rowid deletes matched nothing and the rowid inserts collided.

Also define each FTS5 table's shape once instead of respelling it in the
rebuild, and skip the post-rebuild VACUUM of the main DB unless the rowids it
may renumber are dense.
A second repo's first index into a shared db_path saw the first repo's FTS
rows, forced the full rebuild, and wiped that repo's index.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@wongk
wongk merged commit c38982b into main Sep 21, 2026
9 checks passed
@wongk
wongk deleted the campaign-iss-10710-20260919-c3 branch September 21, 2026 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant