Skip to content

Fixing how repo is filtered in cross repo environment #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dejo1307 merged 1 commit into from
Jun 28, 2026
Merged

Fixing how repo is filtered in cross repo environment #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 58 additions & 8 deletions internal/server/insights_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,27 +35,77 @@ func sampleInsights() []facts.Insight {
func TestFilterInsights(t *testing.T) {
all := sampleInsights()

if got := filterInsights(all, "unused-routes", "", 0); len(got) != 1 || got[0].Source != "unused-routes" {
if got := filterInsights(all, "unused-routes", "", 0, true); len(got) != 1 || got[0].Source != "unused-routes" {
t.Errorf("explainer filter: want 1 unused-routes insight, got %+v", got)
}
if got := filterInsights(all, "UNUSED-ROUTES", "", 0); len(got) != 1 {
if got := filterInsights(all, "UNUSED-ROUTES", "", 0, true); len(got) != 1 {
t.Errorf("explainer filter should be case-insensitive; got %d", len(got))
}
if got := filterInsights(all, "", "", 0.8); len(got) != 1 || got[0].Source != "cycles" {
if got := filterInsights(all, "", "", 0.8, true); len(got) != 1 || got[0].Source != "cycles" {
t.Errorf("min_confidence filter: want only the 0.9 cycles insight, got %+v", got)
}
// repo matches via title ("in golf") and evidence file prefix.
if got := filterInsights(all, "", "golf-ui", 0); len(got) != 1 || got[0].Source != "cycles" {
// repo matches the cycles insight via its evidence path segment (golf-ui/src/a).
if got := filterInsights(all, "", "golf-ui", 0, true); len(got) != 1 || got[0].Source != "cycles" {
t.Errorf("repo filter golf-ui: want the cycles insight, got %+v", got)
}
if got := filterInsights(all, "", "nonexistent", 0); got != nil {
if got := filterInsights(all, "", "nonexistent", 0, true); got != nil {
t.Errorf("repo filter with no match should be empty; got %+v", got)
}
if got := filterInsights(all, "", "", 0); len(got) != 3 {
if got := filterInsights(all, "", "", 0, true); len(got) != 3 {
t.Errorf("no filters should return all; got %d", len(got))
}
}

// TestFilterInsightsNoCrossRepoLeak guards against the substring over-matching
// bug: repo="golf" must not return insights whose evidence lives under sibling
// repos that merely share the "golf" token (golf-ui, my-golf-journal-*).
func TestFilterInsightsNoCrossRepoLeak(t *testing.T) {
all := []facts.Insight{
{
Source: "unused-routes",
Title: "Unused endpoint candidates in golf",
Evidence: []facts.Evidence{
{Fact: "/api/x", File: "golf/internal/bootstrap/server.go"},
},
},
{
Source: "god-class",
Title: "MyGolfJournal.MyGolfJournalApp",
Evidence: []facts.Evidence{{File: "golf-ui/fitness-functions.js"}},
},
{
Source: "cycles",
Title: "Circular dependency",
Evidence: []facts.Evidence{{Fact: "my-golf-journal-ios/Sources/App"}},
},
}

got := filterInsights(all, "", "golf", 0, true)
if len(got) != 1 || got[0].Source != "unused-routes" {
t.Fatalf("repo filter golf should return only the golf insight, got %+v", got)
}
}

// TestFilterInsightsSingleRepoFallback verifies that single-repo snapshots
// (multiRepo=false), where evidence paths are not repo-prefixed, still match via
// the legacy title substring heuristic.
func TestFilterInsightsSingleRepoFallback(t *testing.T) {
all := []facts.Insight{
{
Source: "unused-routes",
Title: "562 route(s) in golf have no caller",
Evidence: []facts.Evidence{{Fact: "/api/x", File: "internal/bootstrap/server.go"}},
},
}

if got := filterInsights(all, "", "golf", 0, false); len(got) != 1 {
t.Errorf("single-repo fallback: want the golf insight via title match, got %+v", got)
}
if got := filterInsights(all, "", "golf", 0, true); got != nil {
t.Errorf("multi-repo strict match should not match unprefixed evidence; got %+v", got)
}
}

func TestRenderInsightsSummary(t *testing.T) {
out := renderInsightsSummary(sampleInsights())
for _, want := range []string{"Found **3** insight(s)", "## By explainer", "unused-routes", "0.60", "Unused endpoint candidates"} {
Expand All @@ -66,7 +116,7 @@ func TestRenderInsightsSummary(t *testing.T) {
}

func TestRenderInsightsCompact(t *testing.T) {
out := renderInsightsCompact(filterInsights(sampleInsights(), "unused-routes", "", 0))
out := renderInsightsCompact(filterInsights(sampleInsights(), "unused-routes", "", 0, true))
for _, want := range []string{"explainer: unused-routes", "confidence: 0.60", "no loaded client calls this route", "suggested actions"} {
if !strings.Contains(out, want) {
t.Errorf("compact missing %q; got:\n%s", want, out)
Expand Down
58 changes: 44 additions & 14 deletions internal/server/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,12 @@ func renderQuerySummary(results []facts.Fact, total int) string {
}

// filterInsights returns the insights matching all of the supplied filters.
// explainer is matched case-insensitively against Insight.Source; repo is a
// best-effort substring match over the title and evidence files (insights have
// no structured repo field); minConfidence keeps insights at or above the bar.
func filterInsights(insights []facts.Insight, explainer, repo string, minConfidence float64) []facts.Insight {
// explainer is matched case-insensitively against Insight.Source; repo matches
// the repo-prefix path segment of each insight's evidence files (insights have
// no structured repo field) — see insightBelongsToRepo; minConfidence keeps
// insights at or above the bar. multiRepo reports whether the snapshot spans
// more than one repo, which selects strict vs. legacy repo matching.
func filterInsights(insights []facts.Insight, explainer, repo string, minConfidence float64, multiRepo bool) []facts.Insight {
repoLC := strings.ToLower(strings.TrimSpace(repo))
var out []facts.Insight
for _, in := range insights {
Expand All @@ -214,24 +216,46 @@ func filterInsights(insights []facts.Insight, explainer, repo string, minConfide
if in.Confidence < minConfidence {
continue
}
if repoLC != "" && !insightMentionsRepo(in, repoLC) {
if repoLC != "" && !insightBelongsToRepo(in, repoLC, multiRepo) {
continue
}
out = append(out, in)
}
return out
}

// insightMentionsRepo reports whether an insight appears to be about repo (given
// lowercased). It checks the title (repo-scoped explainers name the repo there,
// e.g. "... route(s) in golf have no caller ...") and the evidence files, which
// are repo-prefixed in multi-repo snapshots.
func insightMentionsRepo(in facts.Insight, repoLC string) bool {
// pathInRepo reports whether a repo-prefixed evidence path (e.g.
// "golf/internal/x.go") belongs to repo (given lowercased). Matching is on the
// first path segment, so "golf" does not match "golf-ui/..." or
// "my-golf-journal-*".
func pathInRepo(path, repoLC string) bool {
p := strings.ToLower(strings.TrimSpace(path))
return p == repoLC || strings.HasPrefix(p, repoLC+"/")
}

// insightBelongsToRepo reports whether an insight is about repo (given
// lowercased). In multi-repo snapshots evidence paths are repo-prefixed, so we
// match the path-segment of each evidence File/Fact exactly. The title
// substring match is dropped there because titles aren't reliably repo-qualified
// and over-match shared tokens (e.g. "golf" in "golf-ui"). Single-repo snapshots
// don't prefix evidence paths, so there we keep the legacy substring heuristic —
// it can't leak across repos because there are no siblings.
func insightBelongsToRepo(in facts.Insight, repoLC string, multiRepo bool) bool {
for _, ev := range in.Evidence {
if pathInRepo(ev.File, repoLC) || pathInRepo(ev.Fact, repoLC) {
return true
}
}
if multiRepo {
return false
}
// Single-repo legacy fallback (unchanged behavior).
if strings.Contains(strings.ToLower(in.Title), repoLC) {
return true
}
for _, ev := range in.Evidence {
if strings.Contains(strings.ToLower(ev.File), repoLC) || strings.Contains(strings.ToLower(ev.Fact), repoLC) {
if strings.Contains(strings.ToLower(ev.File), repoLC) ||
strings.Contains(strings.ToLower(ev.Fact), repoLC) {
return true
}
}
Expand Down Expand Up @@ -994,7 +1018,7 @@ func (s *Server) registerTools() {
Name: "query_insights",
Description: "Return the architectural findings (insights) that explainers computed during generate_snapshot — the first-class answer to questions like \"which routes are unused?\", \"where are the dependency cycles?\", or \"which modules are god-classes?\". " +
"Each insight carries a title, the explainer that produced it, a description, a confidence (0-1; lower = candidate to verify, not a verdict), evidence (files/symbols/routes), and suggested actions. " +
"Filter by explainer= — one of: unused-routes (dead/uncalled HTTP routes), cycles, layers, crossrepo, coverage, god-class, hotspots, dependency-depth, exported-surface, complexity-outliers; repo= (best-effort substring match over each insight's title and evidence); and min_confidence=. " +
"Filter by explainer= — one of: unused-routes (dead/uncalled HTTP routes), cycles, layers, crossrepo, coverage, god-class, hotspots, dependency-depth, exported-surface, complexity-outliers; repo= (in multi-repo snapshots, matches the repo-prefix path segment of each insight's evidence — e.g. \"golf\" matches golf/... but not golf-ui/...; single-repo snapshots fall back to a substring match); and min_confidence=. " +
"output_mode ladder: 'summary' (DEFAULT — one row per insight: explainer, confidence, title) → 'compact' (adds description, an evidence sample, and suggested actions) → 'full' (complete JSON incl. all evidence and actions). Pass max_tokens to hard-cap output. " +
"All explainers populate insights, but route/cross-repo findings (unused-routes, crossrepo, coverage) only appear for multi-repo (append-mode) snapshots of a backend plus its clients. " +
"Prefer this over hand-diffing query_facts results: e.g. query_insights(explainer=\"unused-routes\") returns the per-repo dead-route candidates directly.",
Expand All @@ -1007,7 +1031,13 @@ func (s *Server) registerTools() {
return errorResult("No snapshot available. Run generate_snapshot first."), nil, nil
}

matched := filterInsights(snap.Insights, args.Explainer, args.Repo, args.MinConfidence)
repos := map[string]struct{}{}
for _, f := range snap.Facts {
if f.Repo != "" {
repos[strings.ToLower(f.Repo)] = struct{}{}
}
}
matched := filterInsights(snap.Insights, args.Explainer, args.Repo, args.MinConfidence, len(repos) > 1)
if len(matched) == 0 {
if len(snap.Insights) == 0 {
return textResult("No insights were produced for this snapshot."), nil, nil
Expand All @@ -1030,7 +1060,7 @@ func (s *Server) registerTools() {

type queryInsightsArgs struct {
Explainer string `json:"explainer,omitempty" jsonschema:"Filter to insights produced by this explainer. One of: unused-routes, cycles, layers, crossrepo, coverage, god-class, hotspots, dependency-depth, exported-surface, complexity-outliers. Empty = all."`
Repo string `json:"repo,omitempty" jsonschema:"Best-effort filter to insights about this repo label (substring match over each insight's title and evidence files). Empty = all repos."`
Repo string `json:"repo,omitempty" jsonschema:"Filter to insights about this repo label. In multi-repo snapshots this matches the repo-prefix path segment of each insight's evidence files (so 'golf' matches golf/... but not golf-ui/...); single-repo snapshots fall back to a substring match. Empty = all repos."`
MinConfidence float64 `json:"min_confidence,omitempty" jsonschema:"Only return insights with confidence >= this (0.0-1.0). Default 0 (all). Unused-routes is emitted at 0.6 as a review candidate."`
OutputMode string `json:"output_mode,omitempty" jsonschema:"'summary' (DEFAULT — one row per insight: explainer, confidence, title) → 'compact' (adds description, evidence sample, actions) → 'full' (complete JSON)."`
MaxTokens int `json:"max_tokens,omitempty" jsonschema:"Optional hard cap on output size (approx tokens). Default: no cap."`
Expand Down
Loading