parser: fix persistent cache staleness and semanticdb_index order dependence - #162
Merged
Conversation
…endence Two related correctness fixes for -scala_gazelle_cache_file: 1. Cached rules no longer enter the symbol scope at startup. Previously readScalaRuleCacheFile loaded every cached rule's symbols before the walk, so rules deleted from the tree kept providing symbols, kept their semanticdb_index membership, and never aged out of the cache file. The cache now seeds the memo only (SeedScalaRule); a rule's symbols load on its first ParseScalaRule hit, in walk order like a fresh parse, and ScalaRules() returns only rules seen this run so deletions age out of a persistent cache. 2. semanticdb_index membership now traverses Symbol.Conflicts. TrieScope.Put is first-wins per symbol name, so which label holds a duplicated name depends on symbol insertion order; membership computed from scope winners differed between walk-order runs and sorted cache preloads. Including conflict losers makes the generated deps order-independent.
zackerydev
force-pushed
the
cache-correctness
branch
from
August 19, 2026 14:49
36fe578 to
983e12f
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
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.
Two related correctness fixes for the persistent cache (
-scala_gazelle_cache_file), found while adopting it in a large private monorepo (~3,100 scala rules) where CI restores the cache across runs while developers regenerate with-scala_fileset_file.1. Deleted rules never leave the cache and keep resolving symbols
readScalaRuleCacheFileloaded every cached rule's symbols into the resolution scope before the walk. Two consequences:semanticdb_indexmembership. In our setup this made cache-mode runs disagree with fileset-mode runs: a PR deleting a scala rule regenerates cleanly for the author, then CI (with a restored cache) re-adds the dead rule's_semanticdbentry. Reproduced with a 4-day-old cache: 9 phantomsemanticdb_indexentries.writeScalaRuleCacheFilewritesScalaRules()= the whole memo, so deleted rules persist in the cache file forever and it grows without bound.Fix: the cache now seeds the memo only (new
MemoParser.SeedScalaRule, no symbol loading). A rule's symbols load on its firstParseScalaRulesha256 hit — in walk order, exactly like a fresh parse — andScalaRules()returns only rules seen this run, so deleted rules age out of the cache on the next write. Memo hits still skip parsing entirely.2.
semanticdb_indexmembership depends on symbol insertion orderTrieScope.Putis first-wins per symbol name; a duplicate from another label becomes aConflicton the incumbent and never enters the trie.semanticdbIndexRule.Resolvecomputed membership from scope winners only, so which labels appear indepsdepends on insertion order — package-walk order (fileset / fresh parse) and sorted cache preload produced different winners for duplicated symbol names, i.e. different generated BUILD files from the same tree.Fix: membership traversal includes
Symbol.Conflicts, making the generated deps the order-independent union. (With fix 1, cache hits load symbols in walk order anyway, but membership should not depend on provider order at all.)Validation
pkg/parser/memo_parser_test.go(seed loads no symbols; hit loads symbols exactly once and never re-parses; sha mismatch re-parses; seeded-but-unvisited rules are dropped fromScalaRules()) andpkg/semanticdb/semanticdb_index_test.go(conflict losers are members; kind filter and external-label exclusion still apply)._semanticdbentry exactly like a fileset run, and steady-state runs still report 0 runtime parses.go test ./pkg/parser/ ./pkg/semanticdb/ ./pkg/resolver/passes; thelanguage/scaladiff-test failures under plaingo testare identical before and after this change (they appear to want bazel-provided assets).Happy to split this into two PRs or reshape the API (e.g. keep
ScalaRules()returning everything behind an option) if you'd prefer — for our use the pruning behavior is the point, since CI restores the cache indefinitely.