Repository navigation
fix(index): preserve symbol word matches through full reindexing - #2252
Closed
rudycelekli wants to merge 3 commits into
Closed
rudycelekli wants to merge 3 commits into
rudycelekli wants to merge 3 commits into
Conversation
Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
rudycelekli
force-pushed
the
fix/index-rebuild-name-vocabulary-20261001
branch
from
October 1, 2026 12:02
03e08bc to
d05c911
Compare
Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
Author
|
Updated this PR against current main Tested signed head |
Resolve conflict while preserving the focused fix and upstream history. Native focused regressions pass; full-suite failures remain disclosed.
colbymchenry
added a commit
that referenced
this pull request
Oct 29, 2026
…'t change A library indexAll() over an existing index cleared the name-segment vocabulary (the words in symbol names that the prompt hook and codegraph_explore match plain-language questions against) before extraction. The store skips files whose content hash is unchanged, so their names were never written back: a second run with no edits left the table empty, and a run after an edit kept only the edited files' names. Sync and healSegmentVocabIfEmpty() repair only a completely empty table, so a partial one stayed that way. A cancelled run emptied the table as well, because the clear ran before the abort check. The CLI was not affected: `codegraph index` recreates the database first. The table is now cleared up front only when the database is fresh. Over an existing graph it is kept, and once a run completes without being cancelled, one DELETE drops the rows of names that no symbol carries any more (file and import nodes don't count, as on the write path). That keeps the orphan cleanup the clear used to do. The prune also resets the session's set of names already written, so a pruned name that comes back later in the same session gets its rows again instead of being skipped. Validation: - Kit repro with the PR's fixture: on main a second indexAll() took the table from 11 rows to 0, an edit plus a third run left 4, and a cancelled run emptied it. With the fix it stays at 11 rows, the renamed class's 3 rows give way to the new name's 3, and a cancel keeps them all. - 5 new tests in segment-vocab.test.ts; 4 fail on main's source, and the fresh-database guard passes on both. Three mutations are caught: no prune, no reset of the written-names set, no clear on a fresh database. - CLI: `codegraph init` then `codegraph index` write the same 11 rows on main and with the fix. - Real repo, vscode src/vs/base + src/vs/editor (1,388 files, 56,406 nodes, 66,250 vocabulary rows), a library indexAll() over the existing index, 3 interleaved runs per arm: main empties the table (66,250 to 0 rows); the fix keeps all 66,250. The prune takes 40 ms of each run, and the runs take the same time (median 8.37 s on main, 8.13 s with the fix). - Cost at scale: the prune statement takes 288 ms on a 10x replica of that index (564,060 nodes), and about 1.2 s on a synthetic graph with 505k vocabulary rows (170k distinct names); a full re-index of that size re-parses every file. Reimplements PR #2252 by @rudycelekli. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
colbymchenry
added a commit
that referenced
this pull request
Oct 29, 2026
…'t change (#3017) A library indexAll() over an existing index cleared the name-segment vocabulary (the words in symbol names that the prompt hook and codegraph_explore match plain-language questions against) before extraction. The store skips files whose content hash is unchanged, so their names were never written back: a second run with no edits left the table empty, and a run after an edit kept only the edited files' names. Sync and healSegmentVocabIfEmpty() repair only a completely empty table, so a partial one stayed that way. A cancelled run emptied the table as well, because the clear ran before the abort check. The CLI was not affected: `codegraph index` recreates the database first. The table is now cleared up front only when the database is fresh. Over an existing graph it is kept, and once a run completes without being cancelled, one DELETE drops the rows of names that no symbol carries any more (file and import nodes don't count, as on the write path). That keeps the orphan cleanup the clear used to do. The prune also resets the session's set of names already written, so a pruned name that comes back later in the same session gets its rows again instead of being skipped. Validation: - Kit repro with the PR's fixture: on main a second indexAll() took the table from 11 rows to 0, an edit plus a third run left 4, and a cancelled run emptied it. With the fix it stays at 11 rows, the renamed class's 3 rows give way to the new name's 3, and a cancel keeps them all. - 5 new tests in segment-vocab.test.ts; 4 fail on main's source, and the fresh-database guard passes on both. Three mutations are caught: no prune, no reset of the written-names set, no clear on a fresh database. - CLI: `codegraph init` then `codegraph index` write the same 11 rows on main and with the fix. - Real repo, vscode src/vs/base + src/vs/editor (1,388 files, 56,406 nodes, 66,250 vocabulary rows), a library indexAll() over the existing index, 3 interleaved runs per arm: main empties the table (66,250 to 0 rows); the fix keeps all 66,250. The prune takes 40 ms of each run, and the runs take the same time (median 8.37 s on main, 8.13 s with the fix). - Cost at scale: the prune statement takes 288 ms on a 10x replica of that index (564,060 nodes), and about 1.2 s on a synthetic graph with 505k vocabulary rows (170k distinct names); a full re-index of that size re-parses every file. Reimplements PR #2252 by @rudycelekli. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Owner
|
Thanks for tracking this down, @rudycelekli: a library |
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
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.
Repeating
CodeGraph.indexAll()on an existing graph cleared the word vocabulary before extraction, while unchanged content hashes skipped the node writes that populate it. A successful second full index therefore kept the symbols but lostgetSegmentMatches()results. Reindexing one edited file left a partial vocabulary that a no-op sync could not repair; a cancelled reindex also erased existing matches.Keep the fresh graph's write-based population, and rebuild vocabulary from all surviving names after a successful full index of an existing graph using the existing batched helper. Cancelled runs retain the previous vocabulary. Add three regressions for repeated indexing, mixed changed/unchanged files, and cancellation.
Validation on macOS with Node 22.19.0 and
--liftoff-only:segment-vocab.test.ts,index-command.test.ts, andindex-store-failure.test.ts.node:sqlitefailed before the fix and pass against this branch's built library. Unchanged graph nodes stay at six; vocabulary rows now stay at eight instead of falling to zero. Both changed and unchanged symbols remain matched after reindexing and no-op sync, and cancellation preserves the original matches.The full suite was not run. The minimal context fixture retains its other retrieval fallback; the demonstrated regression is the public word-matching API.
Current-main maintenance (2026-10-03)
Updated this PR against current main
6560052a6f856855d3f71eee838fd66ccfa4285d(release 1.6.2). The only conflict was the release-note promotion: the pending fix stays under Unreleased, and the entire published 1.6.2 changelog section is byte-identical to upstream. Production and test files retain Git's automatic merge result, with no manual changes.Tested signed head
3927aa88cfc626b916a33916f0afef747fa90866on Node 22.19.0 with native SQLite and the repository's normal Vitest/global build setup: 22 passed across segment vocabulary, index command and store-failure suites. The completenpm run buildalso passed, including the viewer and all 29 copied grammars. Validation remains scoped to those relevant suites; I have not claimed the full test suite or broader language/framework efficacy. No release/version/tag changes.October 21 conflict reconciliation and validation
Normal merge of
f66d064279c02f483c33602abec2e5ce9e13522a, preserving the focused fix and upstream history. Exact head:b4fab616f82299cbe600cca987fe85be0c72ff0f. The resulting tree also integrates cleanly with current main81fd007850ca8af8b88819d2d33ede4f59aa7361; native validation below is specifically on the frozen merge tree, not that newer integration.All 3 added regression cases passed on both Linux and Windows. Build, library build and all eight exact-tree identity guards passed across the four repairs. Full suites are not green:
Native full-suite run. The Linux dedup failure also reproduces on exact main: 25 passed, 1 failed. A matched Windows baseline 93-case diagnostic reproduced 12 failure pairs with identical test bytes (78 passed, 12 failed, 3 skipped). Other Windows timeouts/daemon teardown failures remain unclassified; the Windows dedup failure was not directly baselined. These are failed tests, not waived or green checks.
Windows proof used a hosted x64 Windows VM, not the maintainer ARM64 VM. Linux proof used an Ubuntu 24.04 hosted VM, not the Docker recipe. New upstream CI and maintainer review remain pending; no merge-readiness claim. AI assistance was used for reconciliation, investigation and validation.