diff --git a/CHANGELOG.md b/CHANGELOG.md index e55791e3..35e37050 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -300,6 +300,14 @@ ### Fixed +- **`context_for_task` asks each file's sensitivity once per call, so live packs + keep their affected tests.** The walk asks whether every node it reaches is in a + sensitive file, 600 to 2,000 times per pack over a hundred or two files, and + the daemon answers each by resolving the path's project on disk. A live pack + overran its 2 s graph deadline even on a warm index, losing the affected-tests + section and its run-first line in three of five real tasks. The answer is now + remembered per path for the call. + - **An edit to the global `config.toml` made while the daemon starts is no longer missed.** The daemon loads the file, then starts watching it; a write in between raised no event the watch could see, so the daemon kept the older diff --git a/internal/tools/context_collect.go b/internal/tools/context_collect.go index d33beade..b15eb5ad 100644 --- a/internal/tools/context_collect.go +++ b/internal/tools/context_collect.go @@ -6,6 +6,7 @@ import ( "path" "path/filepath" "strings" + "sync" "time" ) @@ -330,11 +331,30 @@ func (c *ContextCollector) walk(ctx, graphCtx context.Context, pack *contextPack // sensitiveFor adapts the injected decision to the index's root-relative paths. // It is nil when no decision is wired, which withholds nothing. +// +// The answer is remembered per path for the one call: a walk asks about every +// node it reaches, 600 to 2,000 times over a hundred or two files, and the +// daemon's decision resolves the path's project each time (workspace detection +// on disk), which made a live pack overrun its graph deadline. A path's +// sensitivity cannot change within the call that asks. Only the walk's asks go +// through this memo; the constraint, document and hint paths ask c.sensitive +// directly, a few times each. func (c *ContextCollector) sensitiveFor(ctx context.Context, root string) func(string) bool { if c.sensitive == nil { return nil } - return func(rel string) bool { return c.sensitive(ctx, absUnder(root, rel), "") } + var mu sync.Mutex + seen := map[string]bool{} + return func(rel string) bool { + mu.Lock() + defer mu.Unlock() + if v, ok := seen[rel]; ok { + return v + } + v := c.sensitive(ctx, absUnder(root, rel), "") + seen[rel] = v + return v + } } // markStale marks the relationships that come from a file whose bytes changed diff --git a/internal/tools/context_sensitive_memo_test.go b/internal/tools/context_sensitive_memo_test.go new file mode 100644 index 00000000..7103b6a4 --- /dev/null +++ b/internal/tools/context_sensitive_memo_test.go @@ -0,0 +1,70 @@ +package tools + +import ( + "context" + "path/filepath" + "strings" + "sync" + "testing" +) + +// TestContextForTask_AsksEachPathItsSensitivityOnce: a walk reaches many nodes of +// the same files, and the daemon's decision resolves the path's project on disk +// each time it is asked, so one call asks it once per path. The withheld control +// shows the remembered answer is still the decision's own: every node of the +// sensitive file is withheld, the repeated asks included, and none of its text +// reaches the pack. +func TestContextForTask_AsksEachPathItsSensitivityOnce(t *testing.T) { + s := newShop(t) + var mu sync.Mutex + asked := map[string]int{} + s.collector.WithSensitive(func(_ context.Context, p, _ string) bool { + mu.Lock() + asked[p]++ + mu.Unlock() + return strings.HasSuffix(filepath.ToSlash(p), "/cart/cart.go") + }) + pack := s.collect(t, map[string]any{"symbols": []string{"pricing/discount.go#Apply"}, "intent": contextIntentChange}) + + if len(asked) == 0 { + t.Fatal("control: the walk never asked about sensitivity, so the test proves nothing") + } + for p, n := range asked { + if n != 1 { + t.Errorf("%s was asked %d times in one call, want once", p, n) + } + } + // Every node from the sensitive file is withheld, not only the first one asked + // about: a memo that remembered the wrong answer would leak the later ones. + var fromCart int + for _, r := range pack.Related { + if r.Node.Path != "cart/cart.go" { + continue + } + fromCart++ + if !r.Withheld { + t.Errorf("%s from the sensitive cart/cart.go is not withheld: %+v", nodeSelector(r.Node), r) + } + } + if fromCart < 2 { + t.Fatalf("control: the walk reached %d node(s) of cart/cart.go, want at least 2, so a repeated answer is tested", fromCart) + } + out := renderContextPack(pack) + for _, body := range []string{"kept := c.items[:0]", "sub += it.Cents * it.Qty", "c.items[i].Qty += it.Qty"} { + if strings.Contains(out, body) { + t.Errorf("content of the sensitive cart/cart.go reached the pack: %q", body) + } + } + + // A second call asks again: the memory lasts one call, never across calls. + before := len(asked) + _ = s.collect(t, map[string]any{"symbols": []string{"pricing/discount.go#Apply"}, "intent": contextIntentChange}) + for p, n := range asked { + if n != 2 { + t.Errorf("%s was asked %d times over two calls, want twice (once per call)", p, n) + } + } + if len(asked) != before { + t.Errorf("the second call reached %d paths, the first %d: the walk is not deterministic", len(asked), before) + } +}