From 06cd2a6b9e4335784f3936d93b01cdb9ea6d3d50 Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Mon, 12 Oct 2026 21:58:34 +1100 Subject: [PATCH 1/7] feat(context): name the tests a call edge reaches, as a ready run_task filter The affected-tests estimate named packages only (e.g. "./internal/tools/... 1808 test(s)"), so every agent in the PLAN-543 arm D study hand-built its own -run filter or ran the whole package. Since #640/#646 resolve same-package calls, a test that calls a seed is reached by a real call edge, which tells it apart from a test that merely sits in the package. Under a package row, one line now names up to 8 of those edge-reached tests (Test/Example/Fuzz only), closest first by hop count then name, with the rest counted. With a Go package target it is a run_task call whose filter passes validateRunFilter. Co-located tests are never named; a stale index names none. The rows themselves are unchanged, and nothing claims a run. topology_affected's gather records each test's fewest hops from a root. Co-Authored-By: Claude Opus 5.5 Plumb-Session: scarlet-badger --- internal/tools/context_affected.go | 153 +++++++++++++++- internal/tools/context_affected_named_test.go | 167 ++++++++++++++++++ internal/tools/context_affected_test.go | 36 +++- internal/tools/topology_affected.go | 41 ++++- 4 files changed, 379 insertions(+), 18 deletions(-) create mode 100644 internal/tools/context_affected_named_test.go diff --git a/internal/tools/context_affected.go b/internal/tools/context_affected.go index 15cd7c4a..1ee05c3a 100644 --- a/internal/tools/context_affected.go +++ b/internal/tools/context_affected.go @@ -4,8 +4,11 @@ import ( "cmp" "context" "fmt" + "math" "path" "slices" + "strings" + "unicode" "github.com/plumbkit/plumb/internal/textfmt" "github.com/plumbkit/plumb/internal/topology" @@ -16,8 +19,15 @@ import ( // This is topology_affected's own gather (collectAffected) run over the pack's // seeds, kept to one row per test package: the directory (or the run_task target // the calling agent's test command accepts), how many tests live there and why the -// package is implicated. It names no individual test, and it runs none (Invariant -// 10): every word here says "estimate" and "implicated", never "passed" or "ran". +// package is implicated. It runs none (Invariant 10): every word here says +// "estimate" and "implicated", never "passed" or "ran". +// +// Under a package row it may name a few tests, but only the ones a call edge +// reaches: a test that calls a seed, or a declaration next to one, directly or +// through one caller. Co-location says only that a test sits in the package, so a +// co-located test is counted, never named. With a Go test target the names come as +// a ready run_task filter, which is the step agents otherwise spell by hand before +// their first test run (PLAN-543: every arm D agent built its own -run filter). // // The estimate rests on the index, so a missing, failing or foreign index gives // none at all (the index gap already says so), and the scope filter applies to @@ -30,6 +40,10 @@ const ( // contextAffectedNeighbours bounds the neighbouring declarations a change may // extend into that the estimate also starts from. contextAffectedNeighbours = 8 + // contextNamedPerPackage and contextNamedPackages bound the named tests: past a + // handful the line stops being a first run and becomes the package again. + contextNamedPerPackage = 8 + contextNamedPackages = 3 ) // contextAffected is the estimate and what bounded it. @@ -37,11 +51,12 @@ type contextAffected struct { Ran bool Packages []affectedPackage Scope TestScope - Neighbour map[string]bool // directories implicated only through a neighbour of a seed - Excluded int // tests left out by within, corpora or plumb's own state - Cut bool // more packages than the cap - GraphCut bool // the dependent walk hit its traversal budget - Deadline bool // the graph deadline passed, so the estimate is partial + Neighbour map[string]bool // directories implicated only through a neighbour of a seed + Named map[string]namedTests // per package directory: the tests a call edge reaches + Excluded int // tests left out by within, corpora or plumb's own state + Cut bool // more packages than the cap + GraphCut bool // the dependent walk hit its traversal budget + Deadline bool // the graph deadline passed, so the estimate is partial } // affectedTests gathers the estimate. ctx carries the call's graph deadline. @@ -64,14 +79,89 @@ func (c *ContextCollector) affectedTests(ctx context.Context, pack *contextPack, Ran: true, Packages: seedPackagesFirst(aggregateTestsByPackage(kept), nearDirs), Excluded: excluded, Neighbour: nearDirs, Cut: res.Truncated, GraphCut: res.GraphTruncated, Deadline: ctx.Err() != nil, } + pack.Affected.Named = edgeReachedTests(pack.Affected.Packages, indexFreshness(index.store.Health())) for i := range pack.Affected.Packages { - pack.Affected.Packages[i].Tests = nil // the estimate names packages, never tests + pack.Affected.Packages[i].Tests = nil // only edgeReachedTests' names survive } if c.testScope != nil { pack.Affected.Scope = c.testScope(ctx) } } +// namedTests are the tests of one package that a call edge reaches, closest +// first (fewest hops, then name), and how many there were. +type namedTests struct { + Names []string + Total int +} + +// edgeReachedTests names, per package, the tests the dependent walk reached by +// an edge rather than by sitting in an implicated directory. Only test functions +// go test -run can select are named; the first contextNamedPackages packages that +// have any are kept, in the rows' order. +// +// freshness is the index's (indexFreshness). A stale graph may name a test that +// no longer reaches the seed, and a wrong name costs the agent a run, so a stale +// index names none; its gap already says it is stale, and the package rows stay. +func edgeReachedTests(pkgs []affectedPackage, freshness string) map[string]namedTests { + out := map[string]namedTests{} + if freshness == "stale" { + return out + } + for _, pk := range pkgs { + if len(out) == contextNamedPackages { + break + } + var reached []affectedTest + seen := map[string]bool{} + for _, t := range pk.Tests { + if t.Confidence <= colocatedConfidence || !runnableTestName(t.Node.Name) || seen[t.Node.Name] { + continue + } + seen[t.Node.Name] = true + reached = append(reached, t) + } + if len(reached) == 0 { + continue + } + // Fewest hops first; a test whose connecting edge was filtered out of the + // subgraph has no hop count (0) and goes last. Name breaks ties, so the line + // is the same on every call. + slices.SortFunc(reached, func(a, b affectedTest) int { + if c := cmp.Compare(hopRank(a.Hops), hopRank(b.Hops)); c != 0 { + return c + } + return cmp.Compare(a.Node.Name, b.Node.Name) + }) + n := namedTests{Total: len(reached)} + for _, t := range reached[:min(len(reached), contextNamedPerPackage)] { + n.Names = append(n.Names, t.Node.Name) + } + out[pk.Dir] = n + } + return out +} + +// hopRank orders a known hop count before an unknown one (0). +func hopRank(h int) int { + if h == 0 { + return math.MaxInt + } + return h +} + +// runnableTestName is a top-level function go test -run selects: Test, Example or +// Fuzz followed by nothing or a non-lowercase rune, as the testing package +// requires. A benchmark needs -bench, and a helper is not a test at all. +func runnableTestName(name string) bool { + for _, p := range []string{"Test", "Example", "Fuzz"} { + if rest, ok := strings.CutPrefix(name, p); ok { + return rest == "" || !unicode.IsLower([]rune(rest)[0]) + } + } + return false +} + // affectedRoots lists the code symbols the estimate starts from, and the // directories whose co-located tests it may add. A symbol seed starts from its own // node, a covered code file from its declarations; a file the index cannot parse @@ -193,10 +283,57 @@ func (p *contextPack) affectedLines() []packLine { text := fmt.Sprintf(" %s — %d test(s); %s", textfmt.TerminalSafeLine(packageRunLabel(p.Affected.Scope, pk.Dir)), pk.Count, affectedReasonWords(pk.Reason, pk.Dir, p.Affected)) ls = append(ls, packLine{text: text, prio: prioAffected, section: secAffected, class: classAffected}) + if named, ok := p.Affected.Named[pk.Dir]; ok { + ls = append(ls, packLine{text: namedTestsLine(p.Affected.Scope, pk.Dir, named), prio: prioAffected, section: secAffected, class: classAffected}) + } } return ls } +// namedTestsReach is how a named test is connected to the seeds. It is the walk's +// own claim and no more: the edges are the index's, and nothing was run. +const namedTestsReach = "call a seed or a declaration next to one, directly or through one caller (index call edges)" + +// namedTestsLine renders one package's edge-reached tests. With a Go package +// target it is a run_task call ready to paste, its filter cut to the names that +// fit the filter's length limit; otherwise it is the names. The line never says +// the tests ran or passed. +func namedTestsLine(scope TestScope, dir string, n namedTests) string { + names := n.Names + if target, ok := testTarget(scope, dir); ok && scope.Style == TargetGoPackage { + for len(names) > 0 && validateRunFilter("run", runFilterFor(names)) != nil { + names = names[:len(names)-1] + } + if len(names) > 0 { + return fmt.Sprintf(" run first: run_task(slot:\"test\", target:%q, run:%q) — %s %s%s", + target, runFilterFor(names), namedCount(len(names), n.Total), namedTestsReach, namedMore(len(names), n.Total)) + } + names = n.Names + } + return fmt.Sprintf(" %s %s: %s%s", namedCount(len(names), n.Total), namedTestsReach, + textfmt.TerminalSafeLine(strings.Join(names, ", ")), namedMore(len(names), n.Total)) +} + +// runFilterFor anchors the names as one group: go test -run splits a pattern only +// at a top-level |, so the alternation inside the parentheses stays whole. +func runFilterFor(names []string) string { + return "^(" + strings.Join(names, "|") + ")$" +} + +func namedCount(shown, total int) string { + if shown == total { + return fmt.Sprintf("%d test(s)", total) + } + return fmt.Sprintf("%d of %d test(s) that", shown, total) +} + +func namedMore(shown, total int) string { + if shown == total { + return "" + } + return fmt.Sprintf("; %d more are not named", total-shown) +} + // affectedGaps words what bounded the estimate. An estimate with no test in it is // not proof that nothing exercises the seeds. func (p *contextPack) affectedGaps() []string { diff --git a/internal/tools/context_affected_named_test.go b/internal/tools/context_affected_named_test.go new file mode 100644 index 00000000..32a5c621 --- /dev/null +++ b/internal/tools/context_affected_named_test.go @@ -0,0 +1,167 @@ +package tools + +import ( + "context" + "fmt" + "maps" + "slices" + "strings" + "testing" + + "github.com/plumbkit/plumb/internal/topology" +) + +// context_affected_named_test.go — the named tests under an affected-package row +// (PLAN-543): only tests a call edge reaches, closest first, as a run_task filter. + +// C01's cart tests reach Total only through a same-package NAME match, which is no +// call edge, so the cart row names none; C02's pricing test calls Apply through a +// resolved edge and is named (the control: the same fixture, the same section). +func TestContextForTask_Affected_NamesOnlyTestsACallEdgeReaches(t *testing.T) { + s := newShop(t) + _, out := runCase(t, s, oracle(t, "C01")) + rows, sub := sectionRowsAndSubRows(out, affectedHeading) + if len(rows) != 1 || !strings.HasPrefix(rows[0], " ./cart/... — ") { + t.Fatalf("control: C01 should have the one cart row, got %q", rows) + } + if len(sub[rows[0]]) != 0 { + t.Errorf("tests no call edge reaches were named: %q\n%s", sub[rows[0]], out) + } + + _, out = runCase(t, s, oracle(t, "C02")) + rows, sub = sectionRowsAndSubRows(out, affectedHeading) + var pricing string + for _, r := range rows { + if strings.HasPrefix(r, " ./pricing/... — ") { + pricing = r + } + } + want := []string{` run first: run_task(slot:"test", target:"./pricing/...", run:"^(TestApplyPercent)$") — 1 test(s) ` + namedTestsReach} + if !slices.Equal(sub[pricing], want) { + t.Errorf("pricing's named tests = %q, want %q\n%s", sub[pricing], want, out) + } + for _, r := range rows { + if r != pricing && len(sub[r]) != 0 { + t.Errorf("row %q names tests no call edge reaches: %q", r, sub[r]) + } + } +} + +// Without a Go package target there is no run_task filter to offer: the names +// stand alone, and the line still claims only the call edges. +func TestContextForTask_Affected_NamedTestsWithoutATargetAreJustNames(t *testing.T) { + root := copyShopFixture(t) + s := newShopTool(t, openShopStore(t, root), root) + s.collector = s.collector.WithTestScope(func(context.Context) TestScope { return TestScope{} }) + s.tool = NewContextForTask(s.collector) + _, out := runCase(t, s, oracle(t, "C06")) + rows, sub := sectionRowsAndSubRows(out, affectedHeading) + if len(rows) == 0 || !strings.HasPrefix(rows[0], " pricing — ") { + t.Fatalf("control: C06's first row should name the pricing directory, got %q\n%s", rows, out) + } + want := []string{fmt.Sprintf(" 1 test(s) %s: TestApplyPercent", namedTestsReach)} + if !slices.Equal(sub[rows[0]], want) { + t.Errorf("named tests = %q, want %q", sub[rows[0]], want) + } +} + +func reachedTest(name string, conf float64, hops int) affectedTest { + return affectedTest{Node: topology.Node{Name: name, Kind: topology.KindTest, Path: "p/x_test.go"}, Confidence: conf, Hops: hops} +} + +// The naming rules, one at a time: edge-reached only, runnable names only, closest +// first with the name breaking ties and an unknown hop count last, one name per +// test, the per-package and package caps, and nothing at all from a stale index. +func TestEdgeReachedTests_Rules(t *testing.T) { + pkg := affectedPackage{Dir: "p", Tests: []affectedTest{ + reachedTest("TestCoLocated", colocatedConfidence, 0), // no edge: never named + reachedTest("TestFar", graphEdgeBaseline, 2), + reachedTest("TestUnknownHops", graphEdgeBaseline, 0), // edge filtered from the subgraph + reachedTest("TestNearB", 0.85, 1), + reachedTest("TestNearA", 0.85, 1), + reachedTest("TestNearA", 0.85, 1), // the same test twice: named once + reachedTest("BenchmarkNear", 0.85, 1), // needs -bench, not -run + reachedTest("Testify", 0.85, 1), // a lowercase rune after Test: not a test + reachedTest("helperNear", 0.85, 1), // a helper is not a test + reachedTest("Test", 0.85, 1), // bare Test is a test + reachedTest("ExampleNear", 0.85, 1), // go test -run selects examples + reachedTest("FuzzNear", 0.85, 1), // and fuzz targets + reachedTest("Test_underscore", 0.85, 1), // a non-letter after Test is fine + reachedTest("TestÉclair", 0.85, 1), // so is a non-lowercase letter + reachedTest("TestNearC", 0.85, 1), // ninth runnable: past the cap + reachedTest("TestNearD", 0.85, 1), // tenth + reachedTest("TestCoLocatedToo", colocatedConfidence, 0), + }} + got := edgeReachedTests([]affectedPackage{pkg}, "fresh")["p"] + // Byte order: "_" (0x5f) sorts before "É" (0xc3), so TestÉclair is the one cut. + wantNames := []string{"ExampleNear", "FuzzNear", "Test", "TestNearA", "TestNearB", "TestNearC", "TestNearD", "Test_underscore"} + if !slices.Equal(got.Names, wantNames) { + t.Errorf("names = %q, want %q", got.Names, wantNames) + } + if got.Total != 11 { + t.Errorf("total = %d, want 11 (9 one-hop runnable, TestFar, TestUnknownHops)", got.Total) + } + + far := edgeReachedTests([]affectedPackage{{Dir: "q", Tests: []affectedTest{ + reachedTest("TestUnknown", graphEdgeBaseline, 0), reachedTest("TestTwo", 0.85, 2), reachedTest("TestOne", 0.85, 1), + }}}, "building")["q"] + if want := []string{"TestOne", "TestTwo", "TestUnknown"}; !slices.Equal(far.Names, want) { + t.Errorf("hop order = %q, want %q (an index mid-build still names: its edges are real)", far.Names, want) + } + + var many []affectedPackage + for _, d := range []string{"a", "b", "c", "d"} { + many = append(many, affectedPackage{Dir: d, Tests: []affectedTest{reachedTest("TestX", 0.85, 1)}}) + } + many = append([]affectedPackage{{Dir: "none", Tests: []affectedTest{reachedTest("TestY", colocatedConfidence, 0)}}}, many...) + if got := slices.Sorted(maps.Keys(edgeReachedTests(many, "fresh"))); !slices.Equal(got, []string{"a", "b", "c"}) { + t.Errorf("packages named = %q, want the first %d that have any, in row order", got, contextNamedPackages) + } + + if got := edgeReachedTests([]affectedPackage{pkg}, "stale"); len(got) != 0 { + t.Errorf("a stale index named tests: %v", got) + } +} + +// The run filter must be one run_task accepts: names are dropped from the end +// until it fits, and the count says how many are left out. +func TestNamedTestsLine_FilterFitsWhatRunTaskAccepts(t *testing.T) { + names := make([]string, 0, contextNamedPerPackage) + for i := range contextNamedPerPackage { + names = append(names, fmt.Sprintf("TestAVeryLongNameThatDescribesBehaviourInDetail%02d", i)) + } + line := namedTestsLine(TestScope{Language: "go", Style: TargetGoPackage}, "pkg", namedTests{Names: names, Total: 20}) + _, rest, ok := strings.Cut(line, `run:"`) + if !ok { + t.Fatalf("no run filter in %q", line) + } + filter, _, _ := strings.Cut(rest, `")`) + if err := validateRunFilter("run", filter); err != nil { + t.Fatalf("the offered filter is one run_task refuses: %v\n%s", err, line) + } + kept := strings.Count(filter, "|") + 1 + if kept >= len(names) || kept == 0 { + t.Fatalf("control: %d names should not all fit in the filter limit, kept %d", len(names), kept) + } + if want := fmt.Sprintf("%d of 20 test(s) that %s; %d more are not named", kept, namedTestsReach, 20-kept); !strings.HasSuffix(line, want) { + t.Errorf("line = %q,\nwant the suffix %q", line, want) + } +} + +// A test reached from two roots keeps the shorter path; nodes the edges do not +// connect to the root have no hop count. +func TestInwardHops(t *testing.T) { + const root, a, b, c, loose = 1, 2, 3, 4, 5 + edges := []topology.Edge{ + {FromID: a, ToID: root}, // a depends on root: 1 hop + {FromID: b, ToID: a}, // b depends on a: 2 hops + {FromID: c, ToID: b}, // c: 3 hops through b ... + {FromID: c, ToID: root}, // ... but 1 directly + {FromID: loose, ToID: 99}, + } + got := inwardHops(root, edges) + want := map[int64]int{a: 1, b: 2, c: 1} + if !maps.Equal(got, want) { + t.Errorf("inwardHops = %v, want %v", got, want) + } +} diff --git a/internal/tools/context_affected_test.go b/internal/tools/context_affected_test.go index 14bbcffd..bab0b7a4 100644 --- a/internal/tools/context_affected_test.go +++ b/internal/tools/context_affected_test.go @@ -14,9 +14,16 @@ import ( // measured against the gold's test_packages and test_package_counts. // sectionRows are the two-space rows under heading in a rendered pack, up to the -// next line that is not one. +// next line that is neither a row nor a deeper-indented sub-row of one (the +// affected section's named tests sit under their package row). func sectionRows(out, heading string) []string { - var rows []string + rows, _ := sectionRowsAndSubRows(out, heading) + return rows +} + +// sectionRowsAndSubRows also returns each row's sub-rows, keyed by the row. +func sectionRowsAndSubRows(out, heading string) (rows []string, sub map[string][]string) { + sub = map[string][]string{} in := false for _, l := range strings.Split(out, "\n") { switch { @@ -24,11 +31,13 @@ func sectionRows(out, heading string) []string { in = true case in && strings.HasPrefix(l, " ") && !strings.HasPrefix(l, " "): rows = append(rows, l) + case in && len(rows) > 0 && strings.HasPrefix(l, " "): + sub[rows[len(rows)-1]] = append(sub[rows[len(rows)-1]], l) case in: - return rows + return rows, sub } } - return rows + return rows, sub } // goldCounts reads a gold key that maps names to counts. @@ -59,14 +68,16 @@ func affectedTargets(rows []string) []string { // executionWords would claim a test was run. Nothing in the estimate may say one. var executionWords = regexp.MustCompile(`(?i)\b(passed|passes|failed|fails|green|ran|results?|succeeded)\b`) -// C07: a broad test package is one row with its count, never its individual tests, -// and nothing in the estimate says a test was run (Invariant 10). +// C07: a broad test package is one row with its count, never a list of its tests, +// and nothing in the estimate says a test was run (Invariant 10). Under the row, +// one line names the closest of the tests a call edge reaches, as a ready run_task +// filter, and counts the rest (PLAN-543): 60 tests call Summarise, so 8 are named. func TestContextForTask_Affected_C07_OneRowWithTheCountAndNoTestNames(t *testing.T) { s := newShop(t) c := oracle(t, "C07") _, out := runCase(t, s, c) - rows := sectionRows(out, affectedHeading) + rows, sub := sectionRowsAndSubRows(out, affectedHeading) counts := c.goldCounts(t, "test_package_counts") want := []string{fmt.Sprintf(" ./reports/... — %d test(s); holds a seed", counts["./reports/..."])} if !slices.Equal(rows, want) { @@ -93,7 +104,16 @@ func TestContextForTask_Affected_C07_OneRowWithTheCountAndNoTestNames(t *testing if !strings.Contains(out, cut) { t.Errorf("the tests beyond the cap are not counted (want %q):\n%s", cut, out) } - for _, line := range append([]string{affectedHeading}, rows...) { + names := make([]string, 0, contextNamedPerPackage) + for i := range contextNamedPerPackage { + names = append(names, fmt.Sprintf("TestSummariseCase%02d", i)) + } + wantSub := []string{fmt.Sprintf(` run first: run_task(slot:"test", target:"./reports/...", run:%q) — %d of %d test(s) that %s; %d more are not named`, + runFilterFor(names), contextNamedPerPackage, counts["./reports/..."], namedTestsReach, counts["./reports/..."]-contextNamedPerPackage)} + if !slices.Equal(sub[want[0]], wantSub) { + t.Errorf("named tests under the row = %q,\nwant %q", sub[want[0]], wantSub) + } + for _, line := range append(append([]string{affectedHeading}, rows...), sub[want[0]]...) { if m := executionWords.FindString(line); m != "" { t.Errorf("the estimate implies a test was run (%q): %q", m, line) } diff --git a/internal/tools/topology_affected.go b/internal/tools/topology_affected.go index 827ac112..0c1ad9aa 100644 --- a/internal/tools/topology_affected.go +++ b/internal/tools/topology_affected.go @@ -109,6 +109,9 @@ type affectedTest struct { Node topology.Node Confidence float64 Reason string // "dependency edge", "changed package", "imports the changed package" + // Hops is the fewest edges from any root to this test in the dependent walk, + // or 0 for a test found only by co-location. + Hops int } // Reasons a directory is worth scanning for tests. They are ordered by how @@ -349,7 +352,7 @@ func declarationRoots(nodes []topology.Node) []topology.Node { } func collectAffected(ctx context.Context, store *topology.Store, roots []topology.Node, seedDirs []string, maxResults int) (*affectedResult, error) { - g := &affectedGather{store: store, maxResults: maxResults, seen: map[int64]bool{}, dirs: map[string]string{}} + g := &affectedGather{store: store, maxResults: maxResults, seen: map[int64]bool{}, dirs: map[string]string{}, testAt: map[int64]int{}} for _, d := range seedDirs { g.dirs[d] = reasonChanged } @@ -381,6 +384,7 @@ type affectedGather struct { store *topology.Store maxResults int seen map[int64]bool + testAt map[int64]int // test node ID -> its index in tests, to keep the fewest hops dirs map[string]string // directory -> why it is implicated dependents []topology.Node tests []affectedTest @@ -437,8 +441,13 @@ func (g *affectedGather) fromGraph(ctx context.Context, root topology.Node) { g.graphTruncated = true } conf := incidentConfidence(nb.DependedOnBy.Edges) + hops := inwardHops(root.ID, nb.DependedOnBy.Edges) for _, n := range nb.DependedOnBy.Nodes { if g.seen[n.ID] { + // Another root reached this test first; keep whichever path is shorter. + if i, ok := g.testAt[n.ID]; ok && hops[n.ID] > 0 && (g.tests[i].Hops == 0 || hops[n.ID] < g.tests[i].Hops) { + g.tests[i].Hops = hops[n.ID] + } continue } g.seen[n.ID] = true @@ -455,7 +464,8 @@ func (g *affectedGather) fromGraph(ctx context.Context, root topology.Node) { if g.dirs[filepath.Dir(n.Path)] == reasonChanged { reason = reasonChanged } - g.tests = append(g.tests, affectedTest{Node: n, Confidence: c, Reason: reason}) + g.testAt[n.ID] = len(g.tests) + g.tests = append(g.tests, affectedTest{Node: n, Confidence: c, Reason: reason, Hops: hops[n.ID]}) } else { g.dependents = append(g.dependents, n) // Do not demote a directory that is already the changed one: a package @@ -522,6 +532,33 @@ func (g *affectedGather) sortTests() { }) } +// inwardHops is the fewest inward edges from root to each node of an inward +// neighbourhood. An edge FromID -> ToID means FromID depends on ToID, so the walk +// steps from a node to everything that depends on it. A node the edges do not +// connect to root is absent (0). +func inwardHops(root int64, edges []topology.Edge) map[int64]int { + dependents := map[int64][]int64{} + for _, e := range edges { + dependents[e.ToID] = append(dependents[e.ToID], e.FromID) + } + hops := map[int64]int{root: 0} + frontier := []int64{root} + for len(frontier) > 0 { + var next []int64 + for _, id := range frontier { + for _, d := range dependents[id] { + if _, ok := hops[d]; !ok { + hops[d] = hops[id] + 1 + next = append(next, d) + } + } + } + frontier = next + } + delete(hops, root) + return hops +} + // incidentConfidence maps each node ID to the highest confidence of any edge // incident to it within the affected subgraph — an approximation of how // strongly that node is linked to the change. From ccdbb48f602f8f95eb7c23759c754fcb71e10ae1 Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Mon, 12 Oct 2026 22:07:28 +1100 Subject: [PATCH 2/7] fix(context): rank named tests that run the seed first, and fit more per filter Measured on the #571 task (seed buildMovePlans, base 8ac6b433), the first cut named TestApplyTextEdits_* and TestOffsetForPosition ahead of the TestMoveSymbol_* tests the arm D agent actually ran: tests of a CALLEE of the seed scored 1 hop from that neighbour, though they never run the seed. - Neighbours are walked one hop out (collectAffectedNear), and a neighbour that is not a caller of a seed marks its tests Aside; Aside tests rank after every test that runs a seed. A test reached twice keeps the better path (betterPath, with inwardHops in topology_affected_paths.go). - runFilterFor writes the names' common prefix once (^TestMoveSymbol_(Refusals|...)$), so 8 names fit run_task's limit where 5 did. #571 now: 8 of 16, all TestMoveSymbol_*; the pack grows by one line (27,162 -> 27,623 bytes) and nothing else in it changes. Co-Authored-By: Claude Opus 5.5 Plumb-Session: scarlet-badger --- internal/tools/context_affected.go | 62 ++++++++++++---- internal/tools/context_affected_named_test.go | 70 +++++++++++++++++-- internal/tools/topology_affected.go | 66 +++++++++-------- internal/tools/topology_affected_paths.go | 43 ++++++++++++ 4 files changed, 188 insertions(+), 53 deletions(-) create mode 100644 internal/tools/topology_affected_paths.go diff --git a/internal/tools/context_affected.go b/internal/tools/context_affected.go index 1ee05c3a..2d17f077 100644 --- a/internal/tools/context_affected.go +++ b/internal/tools/context_affected.go @@ -9,6 +9,7 @@ import ( "slices" "strings" "unicode" + "unicode/utf8" "github.com/plumbkit/plumb/internal/textfmt" "github.com/plumbkit/plumb/internal/topology" @@ -68,8 +69,8 @@ func (c *ContextCollector) affectedTests(ctx context.Context, pack *contextPack, if len(roots)+len(dirs) == 0 { return // no code seed: nothing to estimate from, and nothing to disclaim } - near := affectedNeighbours(pack) - res, err := collectAffected(ctx, index.store, append(roots, near...), dirs, contextAffectedPackages) + near, aside := affectedNeighbours(pack) + res, err := collectAffectedNear(ctx, index.store, roots, near, aside, dirs, contextAffectedPackages) if err != nil || res == nil { return } @@ -88,8 +89,9 @@ func (c *ContextCollector) affectedTests(ctx context.Context, pack *contextPack, } } -// namedTests are the tests of one package that a call edge reaches, closest -// first (fewest hops, then name), and how many there were. +// namedTests are the tests of one package that a call edge reaches, those that +// exercise a seed first and the closest first among them (fewest hops, then +// name), and how many there were. type namedTests struct { Names []string Total int @@ -124,10 +126,14 @@ func edgeReachedTests(pkgs []affectedPackage, freshness string) map[string]named if len(reached) == 0 { continue } - // Fewest hops first; a test whose connecting edge was filtered out of the - // subgraph has no hop count (0) and goes last. Name breaks ties, so the line - // is the same on every call. + // A test that exercises a seed comes before one that only reaches a callee or + // sibling of it; then fewest hops, where a test whose connecting edge was + // filtered out of the subgraph has no hop count (0) and goes last. Name breaks + // ties, so the line is the same on every call. slices.SortFunc(reached, func(a, b affectedTest) int { + if c := cmp.Compare(boolInt(a.Aside), boolInt(b.Aside)); c != 0 { + return c + } if c := cmp.Compare(hopRank(a.Hops), hopRank(b.Hops)); c != 0 { return c } @@ -190,20 +196,27 @@ func affectedRoots(ctx context.Context, store *topology.Store, seeds []contextSe // away) that a change to the seeds may extend into, so their tests are implicated // too. They matter only for intent=change: understanding code changes nothing. // Each already passed the scope filter in the walk. -func affectedNeighbours(p *contextPack) []topology.Node { +// +// aside holds the neighbours that are not callers of a seed (its callees and +// siblings): a test of one of those does not run the seed, so it ranks after +// every test that does. A caller's tests do run the seed, one hop further out. +func affectedNeighbours(p *contextPack) (near []topology.Node, aside map[int64]bool) { if p.Intent != contextIntentChange { - return nil + return nil, nil } - var out []topology.Node + aside = map[int64]bool{} for _, r := range p.Related { - if len(out) == contextAffectedNeighbours { + if len(near) == contextAffectedNeighbours { break } if r.Dist == 1 && !r.Gap && !r.Withheld && r.Evidence >= evidenceDerived && !isTestNode(r.Node) { - out = append(out, r.Node) + near = append(near, r.Node) + if r.CallerOf == 0 { + aside[r.Node.ID] = true + } } } - return out + return near, aside } // neighbourDirs are the directories only a neighbour of a seed puts in play: not @@ -315,9 +328,28 @@ func namedTestsLine(scope TestScope, dir string, n namedTests) string { } // runFilterFor anchors the names as one group: go test -run splits a pattern only -// at a top-level |, so the alternation inside the parentheses stays whole. +// at a top-level |, so the alternation inside the parentheses stays whole. The +// names' common prefix is written once (^TestMoveSymbol_(Refusals|CreateDestination)$), +// which fits more of them in run_task's filter limit. Test names are identifiers, +// so nothing in them needs escaping. func runFilterFor(names []string) string { - return "^(" + strings.Join(names, "|") + ")$" + if len(names) == 1 { + return "^(" + names[0] + ")$" + } + prefix := names[0] + for _, n := range names[1:] { + for !strings.HasPrefix(n, prefix) { + prefix = prefix[:len(prefix)-1] + } + } + for !utf8.ValidString(prefix) { // never split a rune between prefix and rest + prefix = prefix[:len(prefix)-1] + } + rests := make([]string, len(names)) + for i, n := range names { + rests[i] = n[len(prefix):] + } + return "^" + prefix + "(" + strings.Join(rests, "|") + ")$" } func namedCount(shown, total int) string { diff --git a/internal/tools/context_affected_named_test.go b/internal/tools/context_affected_named_test.go index 32a5c621..dcdacf6f 100644 --- a/internal/tools/context_affected_named_test.go +++ b/internal/tools/context_affected_named_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "maps" + "regexp" "slices" "strings" "testing" @@ -102,11 +103,14 @@ func TestEdgeReachedTests_Rules(t *testing.T) { t.Errorf("total = %d, want 11 (9 one-hop runnable, TestFar, TestUnknownHops)", got.Total) } + asideOne := reachedTest("TestAsideOne", 0.85, 1) + asideOne.Aside = true // reaches only a callee of the seed: it does not run the seed far := edgeReachedTests([]affectedPackage{{Dir: "q", Tests: []affectedTest{ - reachedTest("TestUnknown", graphEdgeBaseline, 0), reachedTest("TestTwo", 0.85, 2), reachedTest("TestOne", 0.85, 1), + asideOne, reachedTest("TestUnknown", graphEdgeBaseline, 0), reachedTest("TestThree", 0.85, 3), reachedTest("TestOne", 0.85, 1), }}}, "building")["q"] - if want := []string{"TestOne", "TestTwo", "TestUnknown"}; !slices.Equal(far.Names, want) { - t.Errorf("hop order = %q, want %q (an index mid-build still names: its edges are real)", far.Names, want) + if want := []string{"TestOne", "TestThree", "TestUnknown", "TestAsideOne"}; !slices.Equal(far.Names, want) { + t.Errorf("order = %q, want %q: tests that run a seed first, by hops, an aside test last however close "+ + "(and an index mid-build still names: its edges are real)", far.Names, want) } var many []affectedPackage @@ -128,7 +132,8 @@ func TestEdgeReachedTests_Rules(t *testing.T) { func TestNamedTestsLine_FilterFitsWhatRunTaskAccepts(t *testing.T) { names := make([]string, 0, contextNamedPerPackage) for i := range contextNamedPerPackage { - names = append(names, fmt.Sprintf("TestAVeryLongNameThatDescribesBehaviourInDetail%02d", i)) + // Distinct after "Test0", so the shared prefix saves almost nothing. + names = append(names, fmt.Sprintf("Test%02dAVeryLongNameThatDescribesBehaviourInDetail", i)) } line := namedTestsLine(TestScope{Language: "go", Style: TargetGoPackage}, "pkg", namedTests{Names: names, Total: 20}) _, rest, ok := strings.Cut(line, `run:"`) @@ -148,6 +153,63 @@ func TestNamedTestsLine_FilterFitsWhatRunTaskAccepts(t *testing.T) { } } +// The filter writes the common prefix once and selects exactly the names: go +// test's own matcher (one part, since the alternation sits in a group) accepts +// each name and rejects a near miss. +func TestRunFilterFor(t *testing.T) { + for _, tc := range []struct { + names []string + want string + miss string + }{ + {[]string{"TestMoveSymbol_Refusals", "TestMoveSymbol_CreateDestination"}, "^TestMoveSymbol_(Refusals|CreateDestination)$", "TestMoveSymbol_Refusals2"}, + {[]string{"TestOne"}, "^(TestOne)$", "TestOneMore"}, + {[]string{"TestA", "TestAB"}, "^TestA(|B)$", "TestABC"}, + {[]string{"ExampleX", "TestX"}, "^(ExampleX|TestX)$", "TestXY"}, + {[]string{"TestÉa", "TestÈb"}, "^Test(Éa|Èb)$", "TestÉb"}, // É and È share their first byte + } { + got := runFilterFor(tc.names) + if got != tc.want { + t.Errorf("runFilterFor(%q) = %q, want %q", tc.names, got, tc.want) + continue + } + re := regexp.MustCompile(got) + for _, n := range tc.names { + if !re.MatchString(n) { + t.Errorf("%q does not select %q", got, n) + } + } + if re.MatchString(tc.miss) { + t.Errorf("%q also selects %q", got, tc.miss) + } + } +} + +// A test reached twice keeps the better path: one that runs the change over one +// that does not, then fewer hops, and any count over none. +func TestBetterPath(t *testing.T) { + seed2 := affectedTest{Hops: 2} + aside1 := affectedTest{Hops: 1, Aside: true} + for _, tc := range []struct { + name string + have affectedTest + hops int + aside bool + want bool + }{ + {"a path that runs the change beats an aside one, even longer", aside1, 3, false, true}, + {"an aside path never beats one that runs the change", seed2, 1, true, false}, + {"fewer hops win", seed2, 1, false, true}, + {"equal hops do not", seed2, 2, false, false}, + {"more hops do not", seed2, 3, false, false}, + {"any count beats an unknown one", affectedTest{}, 4, false, true}, + } { + if got := betterPath(tc.have, tc.hops, tc.aside); got != tc.want { + t.Errorf("%s: betterPath = %v, want %v", tc.name, got, tc.want) + } + } +} + // A test reached from two roots keeps the shorter path; nodes the edges do not // connect to the root have no hop count. func TestInwardHops(t *testing.T) { diff --git a/internal/tools/topology_affected.go b/internal/tools/topology_affected.go index 0c1ad9aa..8016f319 100644 --- a/internal/tools/topology_affected.go +++ b/internal/tools/topology_affected.go @@ -109,9 +109,13 @@ type affectedTest struct { Node topology.Node Confidence float64 Reason string // "dependency edge", "changed package", "imports the changed package" - // Hops is the fewest edges from any root to this test in the dependent walk, + // Hops is the fewest edges from the change to this test in the dependent walk, // or 0 for a test found only by co-location. Hops int + // Aside marks a test reached only through a neighbour the change does not run + // (a callee or sibling of a root): it is implicated, but it does not exercise + // the change itself. + Aside bool } // Reasons a directory is worth scanning for tests. They are ordered by how @@ -352,6 +356,15 @@ func declarationRoots(nodes []topology.Node) []topology.Node { } func collectAffected(ctx context.Context, store *topology.Store, roots []topology.Node, seedDirs []string, maxResults int) (*affectedResult, error) { + return collectAffectedNear(ctx, store, roots, nil, nil, seedDirs, maxResults) +} + +// collectAffectedNear is collectAffected that also walks near: declarations one +// edge from a root, which a change to the roots may extend into. They are walked +// exactly as roots are, except that a test reached through one is a hop further +// from the change than the walk from that neighbour says, and a test reached only +// through a neighbour in aside is marked Aside. +func collectAffectedNear(ctx context.Context, store *topology.Store, roots, near []topology.Node, aside map[int64]bool, seedDirs []string, maxResults int) (*affectedResult, error) { g := &affectedGather{store: store, maxResults: maxResults, seen: map[int64]bool{}, dirs: map[string]string{}, testAt: map[int64]int{}} for _, d := range seedDirs { g.dirs[d] = reasonChanged @@ -366,7 +379,11 @@ func collectAffected(ctx context.Context, store *topology.Store, roots []topolog // bounded on packages in fromColocation. for _, root := range roots { g.dirs[filepath.Dir(root.Path)] = reasonChanged - g.fromGraph(ctx, root) + g.fromGraph(ctx, root, 0, false) + } + for _, n := range near { + g.dirs[filepath.Dir(n.Path)] = reasonChanged + g.fromGraph(ctx, n, 1, aside[n.ID]) } g.fromColocation(ctx) g.sortTests() @@ -420,7 +437,11 @@ func graphTraversalOpts(includeDerived bool) topology.ImpactOpts { // fromGraph adds inward (dependedOnBy) neighbours of root: tests are flagged // with their incident-edge confidence, other nodes become affected files and // seed more directories for the co-location pass. -func (g *affectedGather) fromGraph(ctx context.Context, root topology.Node) { +// +// away is how many hops root itself is from the change: 0 for a root, 1 for a +// neighbour of one; it is added to each test's hop count. aside marks the tests +// root reaches as not exercising the change (root is a callee or sibling of it). +func (g *affectedGather) fromGraph(ctx context.Context, root topology.Node, away int, aside bool) { // ImpactFrom, not Impact: root is ALREADY resolved, and Impact would throw it // away and re-resolve the bare name against the whole index. That lookup has no // tie-break, so a common name lands on an arbitrary row in an unrelated package @@ -442,11 +463,15 @@ func (g *affectedGather) fromGraph(ctx context.Context, root topology.Node) { } conf := incidentConfidence(nb.DependedOnBy.Edges) hops := inwardHops(root.ID, nb.DependedOnBy.Edges) + for id, h := range hops { + hops[id] = h + away + } for _, n := range nb.DependedOnBy.Nodes { if g.seen[n.ID] { - // Another root reached this test first; keep whichever path is shorter. - if i, ok := g.testAt[n.ID]; ok && hops[n.ID] > 0 && (g.tests[i].Hops == 0 || hops[n.ID] < g.tests[i].Hops) { - g.tests[i].Hops = hops[n.ID] + // Another root reached this test first; keep the better path: one that + // exercises the change over one that does not, then the shorter. + if i, ok := g.testAt[n.ID]; ok && hops[n.ID] > 0 && betterPath(g.tests[i], hops[n.ID], aside) { + g.tests[i].Hops, g.tests[i].Aside = hops[n.ID], aside } continue } @@ -465,7 +490,7 @@ func (g *affectedGather) fromGraph(ctx context.Context, root topology.Node) { reason = reasonChanged } g.testAt[n.ID] = len(g.tests) - g.tests = append(g.tests, affectedTest{Node: n, Confidence: c, Reason: reason, Hops: hops[n.ID]}) + g.tests = append(g.tests, affectedTest{Node: n, Confidence: c, Reason: reason, Hops: hops[n.ID], Aside: aside}) } else { g.dependents = append(g.dependents, n) // Do not demote a directory that is already the changed one: a package @@ -532,33 +557,6 @@ func (g *affectedGather) sortTests() { }) } -// inwardHops is the fewest inward edges from root to each node of an inward -// neighbourhood. An edge FromID -> ToID means FromID depends on ToID, so the walk -// steps from a node to everything that depends on it. A node the edges do not -// connect to root is absent (0). -func inwardHops(root int64, edges []topology.Edge) map[int64]int { - dependents := map[int64][]int64{} - for _, e := range edges { - dependents[e.ToID] = append(dependents[e.ToID], e.FromID) - } - hops := map[int64]int{root: 0} - frontier := []int64{root} - for len(frontier) > 0 { - var next []int64 - for _, id := range frontier { - for _, d := range dependents[id] { - if _, ok := hops[d]; !ok { - hops[d] = hops[id] + 1 - next = append(next, d) - } - } - } - frontier = next - } - delete(hops, root) - return hops -} - // incidentConfidence maps each node ID to the highest confidence of any edge // incident to it within the affected subgraph — an approximation of how // strongly that node is linked to the change. diff --git a/internal/tools/topology_affected_paths.go b/internal/tools/topology_affected_paths.go new file mode 100644 index 00000000..80268817 --- /dev/null +++ b/internal/tools/topology_affected_paths.go @@ -0,0 +1,43 @@ +package tools + +import "github.com/plumbkit/plumb/internal/topology" + +// How far a test is from the change in the dependent walk, and which of two +// paths to the same test to keep. context_for_task's named tests rank by these. + +// betterPath reports whether reaching t again in hops, aside or not, beats the +// path it was recorded with: exercising the change beats not, then fewer hops +// beat more, and any count beats none (0). +func betterPath(t affectedTest, hops int, aside bool) bool { + if t.Aside != aside { + return !aside + } + return t.Hops == 0 || hops < t.Hops +} + +// inwardHops is the fewest inward edges from root to each node of an inward +// neighbourhood. An edge FromID -> ToID means FromID depends on ToID, so the walk +// steps from a node to everything that depends on it. A node the edges do not +// connect to root is absent (0). +func inwardHops(root int64, edges []topology.Edge) map[int64]int { + dependents := map[int64][]int64{} + for _, e := range edges { + dependents[e.ToID] = append(dependents[e.ToID], e.FromID) + } + hops := map[int64]int{root: 0} + frontier := []int64{root} + for len(frontier) > 0 { + var next []int64 + for _, id := range frontier { + for _, d := range dependents[id] { + if _, ok := hops[d]; !ok { + hops[d] = hops[id] + 1 + next = append(next, d) + } + } + } + frontier = next + } + delete(hops, root) + return hops +} From f29c99657f7f58a0d0969ae8461475bb4788ab3a Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Mon, 12 Oct 2026 22:12:57 +1100 Subject: [PATCH 3/7] test(context): pin the named tests' order end to end over a real index Mutation left the gather's wiring unasserted (the one-hop offset for a neighbour, the aside mark, which neighbours count as callers, the second path to a test). A five-test fixture around one seed fixes the order each of them decides. Co-Authored-By: Claude Opus 5.5 Plumb-Session: scarlet-badger --- .../context_affected_named_wiring_test.go | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 internal/tools/context_affected_named_wiring_test.go diff --git a/internal/tools/context_affected_named_wiring_test.go b/internal/tools/context_affected_named_wiring_test.go new file mode 100644 index 00000000..ca56bdc9 --- /dev/null +++ b/internal/tools/context_affected_named_wiring_test.go @@ -0,0 +1,71 @@ +package tools + +import ( + "os" + "path/filepath" + "slices" + "testing" +) + +// context_affected_named_wiring_test.go — the named tests' order end to end, over +// a real index, so each piece of the gather that decides it is pinned where it is +// wired: the walk from a neighbour starting one hop out, the aside mark for a +// neighbour the seed only calls, and which neighbours count as callers. + +// namedFixture is one package around Seed: +// +// TestDirect -> Seed 1 hop +// TestZTwo -> X -> Seed 2 hops (X is a caller of Seed) +// TestADeep -> A -> B -> Seed 3 hops: past the walk's depth from Seed, +// reached only through the caller B +// TestAHelper -> helper <- Seed Seed's callee: never runs Seed (aside) +// TestBBoth -> helper, and -> A -> B -> Seed: reached both ways, it keeps +// the path that runs Seed (3 hops) +const namedFixtureSeed = `package seed + +func Seed() int { return helper() + 1 } + +func helper() int { return 1 } + +func B() int { return Seed() } + +func A() int { return B() } + +func X() int { return Seed() } +` + +const namedFixtureTests = `package seed + +import "testing" + +func TestDirect(t *testing.T) { Seed() } +func TestZTwo(t *testing.T) { X() } +func TestADeep(t *testing.T) { A() } +func TestAHelper(t *testing.T) { helper() } +func TestBBoth(t *testing.T) { helper(); A() } +` + +func TestContextForTask_Affected_NamedTestsRunTheSeedFirstAndClosestFirst(t *testing.T) { + root := t.TempDir() + for name, body := range map[string]string{ + "go.mod": "module example.com/fixture\n\ngo 1.22\n", + "seed/seed.go": namedFixtureSeed, + "seed/seed_test.go": namedFixtureTests, + } { + p := filepath.Join(root, filepath.FromSlash(name)) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + } + s := newShopTool(t, openContextStore(t, root, 2), root) + pack := s.collect(t, map[string]any{"symbols": []string{"seed/seed.go#Seed"}, "intent": contextIntentChange, "max_bytes": 32000}) + got := pack.Affected.Named["seed"] + want := []string{"TestDirect", "TestZTwo", "TestADeep", "TestBBoth", "TestAHelper"} + if !slices.Equal(got.Names, want) { + t.Errorf("named = %q, want %q: tests that run Seed first, closest first (TestADeep is three hops, "+ + "reached through the caller B, and TestBBoth by its path through B), and the test of Seed's callee last", got.Names, want) + } +} From 97dc292026ed85bce05e9d84e82f2073e56c2c45 Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Mon, 12 Oct 2026 22:17:24 +1100 Subject: [PATCH 4/7] fix(context): walk callers of a seed before aside neighbours, in name order The walk order of the seeds' neighbours came from the pack's ranking, so whether a test was first recorded through a callee (aside) or through a caller of the seed depended on that ranking. Neighbours now walk callers first, then by name: a path that runs the change is always recorded first, and betterPath only compares paths of the same kind. The fixture is rebuilt so that each wiring decision (the one-hop start, the aside mark, the caller test, the shorter second path) reorders the line when it is wrong. Co-Authored-By: Claude Opus 5.5 Plumb-Session: scarlet-badger --- internal/tools/context_affected.go | 10 +++++ internal/tools/context_affected_named_test.go | 11 ++--- .../context_affected_named_wiring_test.go | 45 +++++++++++-------- internal/tools/topology_affected.go | 6 +-- internal/tools/topology_affected_paths.go | 13 +++--- 5 files changed, 52 insertions(+), 33 deletions(-) diff --git a/internal/tools/context_affected.go b/internal/tools/context_affected.go index 2d17f077..944ab3fb 100644 --- a/internal/tools/context_affected.go +++ b/internal/tools/context_affected.go @@ -200,6 +200,10 @@ func affectedRoots(ctx context.Context, store *topology.Store, seeds []contextSe // aside holds the neighbours that are not callers of a seed (its callees and // siblings): a test of one of those does not run the seed, so it ranks after // every test that does. A caller's tests do run the seed, one hop further out. +// +// near is in walk order, callers first and then by name, whatever the ranking +// chose them in: a path that does not run the seed is then never recorded +// before one that does, and the same pack walks in the same order every time. func affectedNeighbours(p *contextPack) (near []topology.Node, aside map[int64]bool) { if p.Intent != contextIntentChange { return nil, nil @@ -216,6 +220,12 @@ func affectedNeighbours(p *contextPack) (near []topology.Node, aside map[int64]b } } } + slices.SortStableFunc(near, func(a, b topology.Node) int { + if c := cmp.Compare(boolInt(aside[a.ID]), boolInt(aside[b.ID])); c != 0 { + return c + } + return cmp.Compare(a.Name, b.Name) + }) return near, aside } diff --git a/internal/tools/context_affected_named_test.go b/internal/tools/context_affected_named_test.go index dcdacf6f..81e677a9 100644 --- a/internal/tools/context_affected_named_test.go +++ b/internal/tools/context_affected_named_test.go @@ -185,11 +185,11 @@ func TestRunFilterFor(t *testing.T) { } } -// A test reached twice keeps the better path: one that runs the change over one -// that does not, then fewer hops, and any count over none. +// A test reached twice keeps the shorter path of the same kind. The walk records +// paths that run the change before aside ones, so neither kind replaces the other. func TestBetterPath(t *testing.T) { seed2 := affectedTest{Hops: 2} - aside1 := affectedTest{Hops: 1, Aside: true} + aside3 := affectedTest{Hops: 3, Aside: true} for _, tc := range []struct { name string have affectedTest @@ -197,9 +197,10 @@ func TestBetterPath(t *testing.T) { aside bool want bool }{ - {"a path that runs the change beats an aside one, even longer", aside1, 3, false, true}, - {"an aside path never beats one that runs the change", seed2, 1, true, false}, + {"an aside path never replaces one that runs the change", seed2, 1, true, false}, + {"nor the other way: the walk never records the aside one first", aside3, 1, false, false}, {"fewer hops win", seed2, 1, false, true}, + {"fewer hops win among aside paths too", aside3, 2, true, true}, {"equal hops do not", seed2, 2, false, false}, {"more hops do not", seed2, 3, false, false}, {"any count beats an unknown one", affectedTest{}, 4, false, true}, diff --git a/internal/tools/context_affected_named_wiring_test.go b/internal/tools/context_affected_named_wiring_test.go index ca56bdc9..2e4841d9 100644 --- a/internal/tools/context_affected_named_wiring_test.go +++ b/internal/tools/context_affected_named_wiring_test.go @@ -12,20 +12,28 @@ import ( // wired: the walk from a neighbour starting one hop out, the aside mark for a // neighbour the seed only calls, and which neighbours count as callers. -// namedFixture is one package around Seed: +// namedFixture is one package around Seed. Callers of Seed: B, X. Its callees: +// helperA, helperB (aside: their tests never run Seed). Each name is chosen so +// that a wrong hop count or a wrong kind reorders the line. // -// TestDirect -> Seed 1 hop -// TestZTwo -> X -> Seed 2 hops (X is a caller of Seed) -// TestADeep -> A -> B -> Seed 3 hops: past the walk's depth from Seed, -// reached only through the caller B -// TestAHelper -> helper <- Seed Seed's callee: never runs Seed (aside) -// TestBBoth -> helper, and -> A -> B -> Seed: reached both ways, it keeps -// the path that runs Seed (3 hops) +// TestDirect -> Seed 1 hop +// TestATwo -> X -> Seed 2 hops; the walk from the caller X would say 1 +// if a neighbour did not start one hop out +// TestADeep -> A -> B -> Seed 3 hops: past the walk's depth from Seed, +// reached only through the caller B +// TestBBoth -> A -> B -> Seed, and -> helperA: runs Seed, 3 hops +// TestACallees -> f -> helperA (3), and -> helperB (2): aside, and the shorter +// path is found second (helperA walks first) +// TestBHelper -> helperA aside, 2 hops const namedFixtureSeed = `package seed -func Seed() int { return helper() + 1 } +func Seed() int { return helperA() + helperB() } -func helper() int { return 1 } +func helperA() int { return 1 } + +func helperB() int { return 1 } + +func f() int { return helperA() } func B() int { return Seed() } @@ -38,11 +46,12 @@ const namedFixtureTests = `package seed import "testing" -func TestDirect(t *testing.T) { Seed() } -func TestZTwo(t *testing.T) { X() } -func TestADeep(t *testing.T) { A() } -func TestAHelper(t *testing.T) { helper() } -func TestBBoth(t *testing.T) { helper(); A() } +func TestDirect(t *testing.T) { Seed() } +func TestATwo(t *testing.T) { X() } +func TestADeep(t *testing.T) { A() } +func TestBBoth(t *testing.T) { A(); helperA() } +func TestACallees(t *testing.T) { f(); helperB() } +func TestBHelper(t *testing.T) { helperA() } ` func TestContextForTask_Affected_NamedTestsRunTheSeedFirstAndClosestFirst(t *testing.T) { @@ -63,9 +72,9 @@ func TestContextForTask_Affected_NamedTestsRunTheSeedFirstAndClosestFirst(t *tes s := newShopTool(t, openContextStore(t, root, 2), root) pack := s.collect(t, map[string]any{"symbols": []string{"seed/seed.go#Seed"}, "intent": contextIntentChange, "max_bytes": 32000}) got := pack.Affected.Named["seed"] - want := []string{"TestDirect", "TestZTwo", "TestADeep", "TestBBoth", "TestAHelper"} + want := []string{"TestDirect", "TestATwo", "TestADeep", "TestBBoth", "TestACallees", "TestBHelper"} if !slices.Equal(got.Names, want) { - t.Errorf("named = %q, want %q: tests that run Seed first, closest first (TestADeep is three hops, "+ - "reached through the caller B, and TestBBoth by its path through B), and the test of Seed's callee last", got.Names, want) + t.Errorf("named = %q, want %q: tests that run Seed first and closest first, then the tests of Seed's "+ + "callees, closest first (see namedFixture)", got.Names, want) } } diff --git a/internal/tools/topology_affected.go b/internal/tools/topology_affected.go index 8016f319..8283f2a8 100644 --- a/internal/tools/topology_affected.go +++ b/internal/tools/topology_affected.go @@ -468,10 +468,10 @@ func (g *affectedGather) fromGraph(ctx context.Context, root topology.Node, away } for _, n := range nb.DependedOnBy.Nodes { if g.seen[n.ID] { - // Another root reached this test first; keep the better path: one that - // exercises the change over one that does not, then the shorter. + // Another root reached this test first; keep the shorter path of the same + // kind (betterPath). if i, ok := g.testAt[n.ID]; ok && hops[n.ID] > 0 && betterPath(g.tests[i], hops[n.ID], aside) { - g.tests[i].Hops, g.tests[i].Aside = hops[n.ID], aside + g.tests[i].Hops = hops[n.ID] } continue } diff --git a/internal/tools/topology_affected_paths.go b/internal/tools/topology_affected_paths.go index 80268817..b0a61b4e 100644 --- a/internal/tools/topology_affected_paths.go +++ b/internal/tools/topology_affected_paths.go @@ -5,14 +5,13 @@ import "github.com/plumbkit/plumb/internal/topology" // How far a test is from the change in the dependent walk, and which of two // paths to the same test to keep. context_for_task's named tests rank by these. -// betterPath reports whether reaching t again in hops, aside or not, beats the -// path it was recorded with: exercising the change beats not, then fewer hops -// beat more, and any count beats none (0). +// betterPath reports whether reaching t again in hops beats the path it was +// recorded with. Only a path of the same kind can: the walk visits roots, then +// callers of a seed, then the aside neighbours, so a path that runs the change +// is always recorded first, and an aside path never replaces one. Among paths of +// one kind, fewer hops beat more, and any count beats none (0). func betterPath(t affectedTest, hops int, aside bool) bool { - if t.Aside != aside { - return !aside - } - return t.Hops == 0 || hops < t.Hops + return t.Aside == aside && (t.Hops == 0 || hops < t.Hops) } // inwardHops is the fewest inward edges from root to each node of an inward From 234b15f959beae8ee51e91c7bb19a77fc607ea9c Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Mon, 12 Oct 2026 22:22:53 +1100 Subject: [PATCH 5/7] test(context): pin the neighbours' walk order; changelog for named tests Co-Authored-By: Claude Opus 5.5 Plumb-Session: scarlet-badger --- CHANGELOG.md | 9 ++++++ internal/tools/context_affected_named_test.go | 30 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e53d1b09..21df68d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,15 @@ ### Added +- **`context_for_task` names the tests that run your seed, as a ready + `run_task` call.** The affected-tests estimate named only packages + (`./internal/tools/... — 1808 test(s)`), so agents built their own `-run` + filter or ran the whole package. Under a package row, one line now names up to + eight tests a call edge reaches: tests that call a seed, directly or through a + caller, come first and closest first, then tests of the declarations it calls. + With a Go test target the line is a `run_task(slot:"test", target:…, run:…)` + call whose filter `run_task` accepts, with the rest counted. Tests that only + sit in the package are never named, and a stale index names none. - **The Go call graph resolves method calls on local variables of evident type.** `x.M()` now links to the method of `x`'s type when the enclosing function declares `x` once as `x := T{}`, `&T{}`, `new(T)`, `var x T`, `x := y.(T)`, or diff --git a/internal/tools/context_affected_named_test.go b/internal/tools/context_affected_named_test.go index 81e677a9..3ba60de8 100644 --- a/internal/tools/context_affected_named_test.go +++ b/internal/tools/context_affected_named_test.go @@ -228,3 +228,33 @@ func TestInwardHops(t *testing.T) { t.Errorf("inwardHops = %v, want %v", got, want) } } + +// The neighbours walk callers of a seed first, then by name, whatever order the +// ranking put them in: that is what keeps a path that runs the seed ahead of an +// aside one. Only resolved, non-test, one-hop neighbours are walked at all. +func TestAffectedNeighbours_WalkCallersFirstThenByName(t *testing.T) { + rel := func(id int64, name string, callerOf int64) contextRelated { + return contextRelated{Node: topology.Node{ID: id, Name: name, Path: "p/p.go"}, Dist: 1, Evidence: evidenceDerived, CallerOf: callerOf} + } + test := rel(9, "TestSeed", 1) + test.Node.Kind = topology.KindTest + far := rel(8, "Far", 1) + far.Dist = 2 + p := &contextPack{Intent: contextIntentChange, Related: []contextRelated{ + rel(2, "helperB", 0), rel(3, "X", 1), test, rel(4, "helperA", 0), far, rel(5, "B", 1), + }} + near, aside := affectedNeighbours(p) + names := make([]string, 0, len(near)) + for _, n := range near { + names = append(names, n.Name) + } + if want := []string{"B", "X", "helperA", "helperB"}; !slices.Equal(names, want) { + t.Errorf("walk order = %q, want %q", names, want) + } + if want := map[int64]bool{2: true, 4: true}; !maps.Equal(aside, want) { + t.Errorf("aside = %v, want the two callees %v", aside, want) + } + if n, a := affectedNeighbours(&contextPack{Intent: contextIntentUnderstand, Related: p.Related}); n != nil || a != nil { + t.Errorf("understanding code changes nothing, yet neighbours were walked: %v %v", n, a) + } +} From 88a035f7938129a35a2592a7425cee0728a96be3 Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Mon, 12 Oct 2026 22:24:18 +1100 Subject: [PATCH 6/7] test(context): neighbour names whose order disagrees with callers-first Co-Authored-By: Claude Opus 5.5 Plumb-Session: scarlet-badger --- internal/tools/context_affected_named_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/tools/context_affected_named_test.go b/internal/tools/context_affected_named_test.go index 3ba60de8..0a1d2ef2 100644 --- a/internal/tools/context_affected_named_test.go +++ b/internal/tools/context_affected_named_test.go @@ -241,14 +241,15 @@ func TestAffectedNeighbours_WalkCallersFirstThenByName(t *testing.T) { far := rel(8, "Far", 1) far.Dist = 2 p := &contextPack{Intent: contextIntentChange, Related: []contextRelated{ - rel(2, "helperB", 0), rel(3, "X", 1), test, rel(4, "helperA", 0), far, rel(5, "B", 1), + rel(2, "Beta", 0), rel(3, "Gamma", 1), test, rel(4, "Alpha", 0), far, rel(5, "Delta", 1), }} near, aside := affectedNeighbours(p) names := make([]string, 0, len(near)) for _, n := range near { names = append(names, n.Name) } - if want := []string{"B", "X", "helperA", "helperB"}; !slices.Equal(names, want) { + // Name order alone would walk the callees Alpha and Beta first. + if want := []string{"Delta", "Gamma", "Alpha", "Beta"}; !slices.Equal(names, want) { t.Errorf("walk order = %q, want %q", names, want) } if want := map[int64]bool{2: true, 4: true}; !maps.Equal(aside, want) { From 2bcfa1d09bcd1d5cf2bdae9097ae3ae7f1f03dea Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Mon, 12 Oct 2026 22:34:52 +1100 Subject: [PATCH 7/7] docs(context): a named test may reach the seed through several callers Review NIT (warm-cobra): a test reached through a caller of the seed can be two or more callers away, so the line says "through callers". Co-Authored-By: Claude Opus 5.5 Plumb-Session: scarlet-badger --- internal/tools/context_affected.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/tools/context_affected.go b/internal/tools/context_affected.go index 944ab3fb..9939c8e1 100644 --- a/internal/tools/context_affected.go +++ b/internal/tools/context_affected.go @@ -25,7 +25,7 @@ import ( // // Under a package row it may name a few tests, but only the ones a call edge // reaches: a test that calls a seed, or a declaration next to one, directly or -// through one caller. Co-location says only that a test sits in the package, so a +// through callers. Co-location says only that a test sits in the package, so a // co-located test is counted, never named. With a Go test target the names come as // a ready run_task filter, which is the step agents otherwise spell by hand before // their first test run (PLAN-543: every arm D agent built its own -run filter). @@ -315,7 +315,7 @@ func (p *contextPack) affectedLines() []packLine { // namedTestsReach is how a named test is connected to the seeds. It is the walk's // own claim and no more: the edges are the index's, and nothing was run. -const namedTestsReach = "call a seed or a declaration next to one, directly or through one caller (index call edges)" +const namedTestsReach = "call a seed or a declaration next to one, directly or through callers (index call edges)" // namedTestsLine renders one package's edge-reached tests. With a Go package // target it is a run_task call ready to paste, its filter cut to the names that