Drop the code orphaned by the SQLite-only backend transition - #526
Merged
Conversation
The vector-index serialization path existed only for the daemon warmup snapshot, which no longer exists. Export/ImportVectorIndex on Indexer and MultiIndexer, the skipVectorBuild flag that gated re-embedding during warmup, and the VectorBackend Save/LoadFrom/SetCount frame codec had no remaining callers. EmbedderDims on the shared server was write-only.
The store-native FTS adapter is the production text backend, so the BM25 -> Bleve auto-upgrade never fires: its trigger is guarded on the active backend not already being the SymbolSearcher adapter, which every non-test store construction satisfies. Removes BleveBackend and the indexer's upgrade machinery (snapshot, goroutine spawn, once-gating fields), leaving BM25 as the fallback for stores without native search. Drops bleve and 25 satellite modules.
The cache could never serve a usable graph: the eval store is a fresh temp sqlite file removed on exit, so a cache hit only logged a path and skipped indexing, and the store call copied the whole source checkout into the cache directory. eval-server now always indexes; --cache-dir and the container-side flag that fed it are gone.
The storetest backend-resolver conformance helpers had exactly one reference in the tree: their own definition. No conformance runner ever invoked them, and no store in the repo implements the unscoped graph.BackendResolver interface, so wiring them up would pin nothing. The graph.BackendResolver interface and its resolver dispatch arm stay: both are exercised by a test fake in the resolver package.
The cap was threaded through five files and discarded on arrival: openSqliteBackend took it only to write `_ = bufferPoolMB`, since SQLite sizes its page cache via a pragma. Removes the resolver, the env read, the SharedServerConfig field and the OpenBackend parameter. --backend-buffer-pool-mb stays registered as a hidden deprecated no-op so existing start scripts and the detach re-exec keep working.
EncodeJSON, OverlayManager.SessionWorkspace, OverlayManager.DeleteFromBranch and Server.StartedAt had zero callers repo-wide. The uptime path reads the started field directly and session-memory scoping resolves the workspace slug through another route, so none of these were ever wired to a tool or verb.
ResolverLSPHelper had no callers: MultiIndexer mirrors the helper through its own resolverLSPHelper field, and RunDeferredPassesAll never reads the per-Indexer getter, so its doc comment described a consumer that does not exist. CloneIndexPending was likewise a readiness probe with no reader. The backing pending flag stays: MarkPending still feeds readyLocked.
Nothing constructs ZapReporter or TimingReporter — every reference to either lived inside its own file, and no Reporter slot is ever fed one, so implementing the interface did not keep them reachable. The Run/RunDisabled/runWith context wrappers have no call site, and Spinner.Enabled/Tracker plus PaletteAccent/PaletteErr are unread accessors; the surviving Spinner surface is what the CLI actually drives.
The file/line, qual-name and name-in-file node matchers plus the path and ID parsing helpers had no production callers; the go/analysis provider reimplements the file/line lookup locally over its own repo node set. PersistEdge and FindEdgeByTarget were likewise unreachable - production persists edge attributes through direct EdgePersister assertions.
The post-commit wrappers InstallPostCommit, UninstallPostCommit and HookPath had no production callers — cmd/gortex/githook.go reaches the package only through InstallHook, UninstallHook and HookPathFor. Status and StatusReport were likewise test-only, so they go too. The installer tests now drive the multi-hook entry points directly with the "post-commit" hook name, keeping every behavioural assertion.
The per-code constructor sugar and the errors.As-style unwrapper in internal/mcp/errors.go were never adopted by any handler; every error site builds results through NewStructuredErrorResult directly. Drops the now-orphaned invalid-argument sentinel and cross-workspace code along with them, plus the exported one-line delegate over isDedicatedFacadeTool, which only the unexported form ever reaches.
The CommunityCoupling type and the Couplings field on CrossCommunityWarning had no producer: computeCrossCommunityWarning only ever sets AffectedCommunities. The json tag was omitempty, so the field never serialized and its removal is wire-compatible for MCP clients. Two tests pinned "the impact path runs no graph-wide coupling scan" by asserting the field stayed empty; that guarantee is now structural and the rationale is recorded on the type instead.
EdgesForKindsLight was a thin wrapper over the still-live LightEdgeScanner capability with no callers; the package-level EvictConfigNodesByIDs dispatcher was bypassed by contracts, which asserts ConfigNodeBatchEvicter itself. IsExternalCallStub, IsModuleStub and StructuralEdgeDropCount had no readers either. The interfaces, methods and counter they wrapped stay.
NodeText, WrapNode and WrapTree had no callers left: extractors slice src[start:end] directly and the shim Node/Tree values are constructed inside tsitter itself. The per-language complexity wrappers and the Temporal env-helper configurator were likewise unreachable, so the stranded EnvHelperConfigurable interface goes with them.
Fourteen exported helpers had no caller left anywhere in the tree — only their own declaration and the doc comments that named them. Delete each symbol with its comment and let the compiler prune the imports that went dark, so the remaining exported surface is one every reader can trust has a consumer.
The graph now lives in a local SQLite store, but package docs, prompt text, the release package description and the backend-bench comment still described the retired in-memory backend. Wording only. The exporter doc also dropped its snapshot claim: it walks a graph.Store and no longer offers the consistency the old gob snapshot did.
The eval-server side of the index cache is gone, but the docker harness still carried the producer half: every container setup ran _restore_or_skip_cache, which resolved a cache key, mkdir'd /root/.gortex-cache and put_archive'd index.tar.gz into it, then logged "Cached index restored". Nothing inside the container ever read that tarball, so each eval instance paid the copy and emitted a cache-hit line that did not correspond to a cache hit. Remove the whole plumbing: the restore call and its method, _make_cache_key, DEFAULT_CACHE_DIR, the cache_dir constructor parameter, _get_repo_identity (its only caller was the restore path), the run_eval pass-through of env_cfg["cache_dir"], and the cache-key/cache_dir assertions in the eval tests. The module docstring no longer promises to mount cached indexes.
The mh-search-backends case (and its typo twin) listed
internal/search/bleve.go::BleveBackend among the expected IDs. That
symbol no longer exists, so `gortex eval recall` printed
"expected IDs missing from graph — fixture bug" on every default run and
the case could never score better than 4/5 no matter how good the ranker
was.
Replace the dead ID with internal/search/symbolsearcher_backend.go::
SymbolSearcherBackend — the store-native FTS backend that the indexer now
installs as the initial search.Backend, and the answer the case's query
("search backend implementations") is actually asking for. The expected
count stays at five.
Note for anyone comparing recall numbers across revisions: this case
stopped being answerable when the Bleve backend was deleted, not here.
Any regression on mh-search-backends belongs to that revision; this
change restores the case rather than altering its difficulty.
`gortex eval recall --fixture bench/fixtures/retrieval.yaml` now validates
the case cleanly. Five unrelated expected IDs in other cases
(cmd/gortex/init_doctor.go, cmd/gortex/server.go, pkg/wire/decoder.go)
are still reported missing; those predate the search-backend sweep and are
left alone.
SearchBackendStats still documented Name as "bm25" | "bleve-memory" | "bleve-disk", and DiskPath / DiskBytes as "set only for bleve-disk". No producer can emit any of that: resolveSearchBackend only ever returns "bm25", "sqlite-fts5" or "unknown", and the branch that populated DiskPath / DiskBytes went away with the on-disk backend. Everything downstream of those two fields was therefore unreachable — the `case sb.DiskPath != ""` arm of the search row, the node-proportional share written into MemoryBreakdown.DiskBytes, and the conditional disk_b column plus its header, row and footer cells (showDisk could never turn true). Both fields were json omitempty and never set, so they were already absent from every status payload on the wire; dropping them changes no observed JSON. Removed: SearchBackendStats.DiskPath / .DiskBytes, MemoryBreakdown.DiskBytes, the DiskPath render arm, the disk_b column plumbing, and the two disk-column tests. The render test that pinned a bleve-disk payload is rewritten onto the two payloads a live daemon can actually produce: the disk-resident sqlite-fts5 backend (no heap figure) and the in-process bm25 backend (heap figure present). The Name doc comment now lists only the names resolveSearchBackend can return. Follow-up, deliberately not done here: sqlite-fts5 reports no size at all, and the graph store's own file size would be a genuinely useful number to surface on that row. That is a new feature, not part of removing dead names, so it is left for its own change.
Four leftovers whose readers went away with the code they served. internal/graph: structuralWriteDrops was an atomic counter incremented by both write funnels (Graph.AddEdge and FilterStructuralEdgeViolations) and read by nobody — the audit helper that printed it is gone. The drops themselves still happen; only the uncounted counter is removed. internal/semantic: SymbolMap kept a gortexToExternal reverse map that no caller could observe once ExternalID and Size were deleted. Every enrichment provider only ever resolves external ID -> Gortex node ID, so the map, its write in Add, and the "bidirectional" claim in the type doc all go; scip's enrichFromIndexScoped uses Add + GortexID only and is unaffected. internal/indexer: Indexer.SetResolverLSPHelper stored the helper in idx.resolverLSPHelper after handing it to idx.resolver.SetLSPHelper. The setter itself is live — MultiIndexer calls it both when propagating to a newly created per-repo Indexer and when the helper is installed later, and serverstack wires it from the daemon's LSP registry — so the setter stays and only the shadow field goes. The resolver owns the helper; the Indexer copy was never read back. MultiIndexer's own resolverLSPHelper field is different: it is read on every propagation and on the master-resolver path, so it stays. internal/semantic/goanalysis: matchRepoNodeByFileLine's doc comment described itself as mirroring semantic.MatchNodeByFileLine, which no longer exists. It now states the innermost-then-smallest-span policy directly.
THIRD_PARTY_NOTICES.md documents its own source of truth: the module list is generated from `go list -m all`. Re-ran that command and rewrote the `## Modules` section from its output, keeping the file's existing case-insensitive path ordering so the diff is content, not churn. The bleve module family is what prompted this — bleve/v2, bleve_index_api, geo, go-faiss, go-metrics, goleveldb, gtreap, mmap-go, scorch_segment_api, segment, snowball, snowballstem, stempel, upsidedown_store_api, vellum and zapx v11-v17 all left the module graph with the backend, along with their exclusive dependents (couchbase/moss, couchbase/ghistogram, RoaringBitmap/roaring, golang/snappy, json-iterator/go, modern-go/concurrent, modern-go/reflect2, mschoch/smat, google/gofuzz, x448/float16, bbolt, gofail, mdempsky/unconvert). blevesearch/go-porterstemmer is still required and keeps its entry. Regenerating also picked up the drift that had accumulated since the last refresh: 34 modules present in the graph but never listed (modernc.org sqlite and its support tree, jackc/pgx, the gortexhq tree-sitter grammars, gomlx/compute, go-github, ledongthuc/pdf, golang-lru) and 31 version bumps. The file now matches `go list -m all` entry for entry. docs/04-evaluation/task-set.md described buildSearchIndex as "(BM25 / Bleve)". It is now BM25 in tests and evals, store-native FTS in production.
Nine comments still described the text search index as "BM25/Bleve" (or credited Bleve outright) after the backend was deleted. Each one is reworded to say what the code is now rather than dropping the word and leaving a half sentence: - config.go / manager.go: SkipSearch "controls what goes into the text search index (in-process BM25 in tests and evals, store-native FTS in production)" — the contrast with SkipEmbed's graph/vector scope is preserved. - query/engine.go: SearchSymbols uses the configured backend's ranking, named for both live backends, still contrasted against the substring fallback. - eval/recall/rankers.go: BM25Ranker takes a raw text backend (either live one) as opposed to a HybridBackend's text side; EngineRanker measures text-backend results plus the substring fallback. - graph/store.go, store_sqlite/schema.go, store_sqlite/store_fts.go: the parallel in-process index that engine-native FTS replaces is a BM25 index, so the Bleve half of the pair simply went. - agents/aider/adapter.go: the ignored paths are Gortex's graph store and cache artifacts, not scorer data. Comment-only: every changed line in the diff starts with // or --, and the store_sqlite one sits above CREATE VIRTUAL TABLE rather than inside any stored statement text, so no DDL or schema version is affected. The only surviving matches for `git grep -ni bleve -- '*.go'` are the bLevel loop counters in callpath.go (a substring accident) and the live blevesearch/go-porterstemmer import in search/fts_normalize.go.
zzet
force-pushed
the
chore/drop-code-orphaned-by-sqlite-only
branch
from
August 14, 2026 14:11
95b0365 to
7e44615
Compare
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
1 participant
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.
What
Dead-code sweep of everything orphaned by the sqlite-only backend transition (#473): 16 commits, 79 files, +212/−3,194.
Every candidate was verified three ways before deletion:
golang.org/x/tools/cmd/deadcodewith tests-as-roots, per-symbol reference greps, and a build+test cycle per commit. Deletion of a thing and the conversion of its last callers always share one commit.Removed
SetSkipVectorBuild/ImportVectorIndex/ExportVectorIndexand the frame codec ininternal/search/vector.go) — last callers died with the daemon snapshot subsystem.internal/search/bleve.go, the upgrade spawn machinery,AutoThreshold, thebleve-disk/bleve-memorystatus names,GORTEX_BLEVE_DISK_DIR) — the upgrade could never fire on any shipped configuration: every production store implements the native FTS interface, soinitialSearchBackendnever selects the in-heap path.go mod tidydrops bleve plus 25 transitive modules.internal/eval/cache.go+--cache-dironeval-server+ the container-side pass-through) — a cache hit restored a tarball nothing reads, serving an empty graph.storetest/backend_resolver.go, 272 LOC, zero callers since it was written).graph.BackendResolveritself stays and is covered by the resolver's own tests (resolve_all_pending_test.goasserts the backend-created-pending path).--backend-buffer-pool-mbknob — sqlite sizes its own cache; the flag remains registered as a hidden no-op so existing invocations keep working.CommunityCouplingresponse field, unadopted structured-error constructors, and ~30 smaller unreferenced helpers across daemon/indexer/progress/semantic/githooks/parser/graph — each grep-verified, with dedicated tests retargeted at the live equivalents rather than deleted where they covered live behavior.Deliberately spared (with reasons)
BM25Backendand its tokenizer side-indexes (~900 LOC): production-dead but referenced by 96 test files pluseval_stdbench's store-free ranking of foreign corpora. Dies only with the deferred*graph.Graphtest-fixture migration.search.NewAutoand theisSymbolSearcherBackendguards: the capability-fallback path every memory-fixture test exercises.deadcode's largest false positive — its callers exist under-tags llama.--backend memoryhard-error shim and legacy no-op flags: intentional migration surface.MultiIndexer.AutoDetectRepos+Multi.AutoDetectconfig: parsed, migrated, documented — but never read to drive behavior. Left in place; needs a product decision (wire it up vs retract the doc) rather than a silent deletion.Review round (last five commits)
Two adversarial review passes found no blockers; everything they raised is fixed in-branch: the eval docker harness no longer restores a cache tarball nothing reads; the two bench fixture cases that gold-answered the deleted
BleveBackendnow point at the live store-native FTS adapter (the cases broke at the deletion, not at the fix — noted in the commit body); the status protocol no longer documentsbleve-memory/bleve-diskand the producer-lessDiskPath/DiskBytesfields are gone (both were omitempty and never set — wire-compatible); the write-only residue (structuralWriteDrops, theSymbolMapreverse map, theresolverLSPHelperfield) is removed with every kept setter justified; andTHIRD_PARTY_NOTICES.mdwas regenerated from its documentedgo list -m allsource — which, beyond dropping the bleve tree, also picked up 34 modules the file had never listed and 31 stale versions.Known notes (pre-existing, flagged not fixed)
init_doctor.go,server.go,pkg/wire/decoder.gosymbols —mh-wire-formatscores 0 unconditionally); the fixture-bug warning keeps firing until a corpus-maintenance pass repoints them.make eval-test+ a venv that a fresh checkout lacks).Validation
go test -race -count=1 ./...green;golangci-lint0 issues;go vetclean; fence test green.deadcode -testre-run post-sweep: every deleted symbol gone, no new dead code introduced by the conversions; 28 remaining entries are pre-existing and untouched.-tags llamabuild and go.sum consistency).