Skip to content

Commit ac01b6d

Browse files
greynewellclaude
andauthored
Fix incremental update: stale callee shards + false HEAD-change on startup (#99)
* Fix two incremental-update correctness bugs 1. watcher.go: trim SHA at startup to avoid false HEAD-change on first poll w.lastCommitSHA was set from raw git output (includes trailing newline). On the first poll, currentSHA (trimmed) != lastCommitSHA (untrimmed), so headChanged was always true. The downstream git-diff call would fail silently (invalid ref), but the spurious headChanged = true was wasteful and misleading. 2. daemon.go: track old callees so removed calls re-render callee shards computeAffectedFiles walked current callers but never tracked pre-merge callees. When funcA (in file A) stopped calling funcB (in file B), file B was not marked affected, so its shard kept a stale "funcB ← funcA" entry until the next full rebuild. Fix: snapshot d.cache.Callees for functions in changed files before the merge (parallel to the existing oldImports snapshot), pass them as oldCalleeFiles to computeAffectedFiles, and mark those files affected. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix lint: combine map params in computeAffectedFiles signature gocritic paramTypeCombine: two consecutive map[string][]string parameters can be declared together as "oldImports, oldCalleeFiles map[string][]string". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent cbb5680 commit ac01b6d

3 files changed

Lines changed: 109 additions & 5 deletions

File tree

internal/shards/daemon.go

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -257,19 +257,34 @@ func (d *Daemon) incrementalUpdate(ctx context.Context, changedFiles []string) {
257257
return
258258
}
259259

260-
// Snapshot old import relationships before the merge so we can re-render
261-
// files that lost an "imported-by" reference after A stops importing B.
260+
// Snapshot old import and call relationships before the merge so we can
261+
// re-render files that lost a reference after A stops importing/calling B.
262262
oldImports := make(map[string][]string, len(changedFiles))
263+
oldCalleeFiles := make(map[string][]string) // funcID -> callee file paths
263264
func() {
264265
d.mu.Lock()
265266
defer d.mu.Unlock()
267+
changedSet := make(map[string]bool, len(changedFiles))
268+
for _, f := range changedFiles {
269+
changedSet[f] = true
270+
}
266271
for _, f := range changedFiles {
267272
if deps := d.cache.Imports[f]; len(deps) > 0 {
268273
cp := make([]string, len(deps))
269274
copy(cp, deps)
270275
oldImports[f] = cp
271276
}
272277
}
278+
for id, fn := range d.cache.FnByID {
279+
if !changedSet[fn.File] {
280+
continue
281+
}
282+
for _, callee := range d.cache.Callees[id] {
283+
if callee.File != "" {
284+
oldCalleeFiles[id] = append(oldCalleeFiles[id], callee.File)
285+
}
286+
}
287+
}
273288
}()
274289

275290
var affected []string
@@ -280,7 +295,7 @@ func (d *Daemon) incrementalUpdate(ctx context.Context, changedFiles []string) {
280295
d.mergeGraph(ir, changedFiles)
281296
d.cache = NewCache()
282297
d.cache.Build(d.ir)
283-
affected = d.computeAffectedFiles(changedFiles, oldImports)
298+
affected = d.computeAffectedFiles(changedFiles, oldImports, oldCalleeFiles)
284299
cacheSnapshot = d.cache
285300
}()
286301

@@ -592,7 +607,10 @@ func (d *Daemon) assignNewFilesToDomains(newNodes []api.Node) {
592607
// oldImports maps each changed file to the set of files it imported BEFORE
593608
// the merge, so that files whose "imported-by" sections lost a reference
594609
// are also re-rendered.
595-
func (d *Daemon) computeAffectedFiles(changedFiles []string, oldImports map[string][]string) []string {
610+
// oldCalleeFiles maps each function ID (in a changed file) to the set of files
611+
// its callees lived in BEFORE the merge, so that callee files whose
612+
// "← caller" entries were removed are also re-rendered.
613+
func (d *Daemon) computeAffectedFiles(changedFiles []string, oldImports, oldCalleeFiles map[string][]string) []string {
596614
affected := make(map[string]bool)
597615

598616
for _, f := range changedFiles {
@@ -625,6 +643,12 @@ func (d *Daemon) computeAffectedFiles(changedFiles []string, oldImports map[stri
625643
affected[caller.File] = true
626644
}
627645
}
646+
// Files that functions in f used to call (old callees): if the call was
647+
// removed, those files' shards still show "funcB ← funcA" and need
648+
// re-rendering to drop the stale back-reference.
649+
for _, calleeFile := range oldCalleeFiles[id] {
650+
affected[calleeFile] = true
651+
}
628652
}
629653
}
630654

internal/shards/daemon_test.go

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,3 +359,83 @@ func TestMergeGraph_DomainsPreservedEvenWhenIncrementalHasMore(t *testing.T) {
359359
t.Errorf("expected original domain preserved, got %v", result.Domains)
360360
}
361361
}
362+
363+
// ── computeAffectedFiles tests ───────────────────────────────────────────────
364+
365+
// TestComputeAffectedFiles_OldCalleeIncluded verifies that when a function in a
366+
// changed file used to call a function in another file, that other file is
367+
// included in the affected set even if the call was removed by the update.
368+
//
369+
// Before the fix, computeAffectedFiles only walked d.cache.Callers (new state)
370+
// and oldImports. It never consulted pre-merge callee relationships. As a result,
371+
// if funcA stopped calling funcB, file B was not marked affected and its shard
372+
// kept the stale "funcB ← funcA" line.
373+
func TestComputeAffectedFiles_OldCalleeIncluded(t *testing.T) {
374+
ir := buildIR(
375+
[]api.Node{
376+
newNode("file-a", []string{"File"}, "filePath", "a.go"),
377+
newNode("fn-a", []string{"Function"}, "filePath", "a.go", "name", "FuncA"),
378+
newNode("file-b", []string{"File"}, "filePath", "b.go"),
379+
newNode("fn-b", []string{"Function"}, "filePath", "b.go", "name", "FuncB"),
380+
},
381+
[]api.Relationship{
382+
// FuncA calls FuncB (before the incremental update).
383+
newRel("calls-1", "calls", "fn-a", "fn-b"),
384+
},
385+
)
386+
d := NewTestDaemon(ir)
387+
d.cache = NewCache()
388+
d.cache.Build(ir)
389+
390+
// Simulate pre-merge callees snapshot: fn-a used to call fn-b (in b.go).
391+
oldCalleeFiles := map[string][]string{
392+
"fn-a": {"b.go"},
393+
}
394+
395+
// After the incremental update, FuncA no longer calls FuncB, so the new
396+
// cache has no callee relationship. Only a.go is in changedFiles.
397+
affected := d.computeAffectedFiles([]string{"a.go"}, nil, oldCalleeFiles)
398+
399+
affectedSet := make(map[string]bool, len(affected))
400+
for _, f := range affected {
401+
affectedSet[f] = true
402+
}
403+
404+
if !affectedSet["a.go"] {
405+
t.Error("expected a.go (changed file) in affected set")
406+
}
407+
if !affectedSet["b.go"] {
408+
t.Error("expected b.go (old callee) in affected set — shard needs to drop stale '← FuncA' entry")
409+
}
410+
}
411+
412+
// TestComputeAffectedFiles_CurrentCallersIncluded verifies that files currently
413+
// calling a function in the changed file are marked affected (existing behaviour).
414+
func TestComputeAffectedFiles_CurrentCallersIncluded(t *testing.T) {
415+
ir := buildIR(
416+
[]api.Node{
417+
newNode("file-a", []string{"File"}, "filePath", "a.go"),
418+
newNode("fn-a", []string{"Function"}, "filePath", "a.go", "name", "FuncA"),
419+
newNode("file-c", []string{"File"}, "filePath", "c.go"),
420+
newNode("fn-c", []string{"Function"}, "filePath", "c.go", "name", "FuncC"),
421+
},
422+
[]api.Relationship{
423+
// FuncC (in c.go) calls FuncA (in a.go).
424+
newRel("calls-2", "calls", "fn-c", "fn-a"),
425+
},
426+
)
427+
d := NewTestDaemon(ir)
428+
d.cache = NewCache()
429+
d.cache.Build(ir)
430+
431+
affected := d.computeAffectedFiles([]string{"a.go"}, nil, nil)
432+
433+
affectedSet := make(map[string]bool, len(affected))
434+
for _, f := range affected {
435+
affectedSet[f] = true
436+
}
437+
438+
if !affectedSet["c.go"] {
439+
t.Error("expected c.go (current caller) in affected set")
440+
}
441+
}

internal/shards/watcher.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func (w *Watcher) Run(ctx context.Context) {
5454
defer close(w.eventCh)
5555

5656
w.lastIndexMod = w.gitIndexMtime()
57-
w.lastCommitSHA = w.runGit("rev-parse", "HEAD")
57+
w.lastCommitSHA = strings.TrimSpace(w.runGit("rev-parse", "HEAD"))
5858

5959
for {
6060
select {

0 commit comments

Comments
 (0)