Skip to content

Commit e80f34e

Browse files
tae2089claude
andauthored
chore: remove dead Analyze/PruneTree APIs, migrate tests to live paths (#22)
Two internal APIs were unused in production and exercised only by their own tests: - changes.Analyze (non-paged) + its private helpers sortRiskEntries / computeRiskScores / computeRiskCandidates. Production uses AnalyzePage. The 8 test call-sites now go through a small analyzeAll helper backed by AnalyzePage (full window), so the risk-score / namespace / ordering coverage moves onto the live path instead of a dead parallel implementation. - ragindex.PruneTree + pruneNode + their TestPruneTree_* tests — never used in production. The pagination-ordering test was rewritten to pin concrete expected node IDs (descending risk) rather than compare against AnalyzePage itself, keeping it a real, non-tautological check (per review). ragindex.LoadIndex is kept — it verifies live wikiindex builder output in tests. No behavior change; -150 lines. Independently reviewed. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent ab5723e commit e80f34e

4 files changed

Lines changed: 25 additions & 171 deletions

File tree

internal/analysis/changes/service.go

Lines changed: 0 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -55,35 +55,6 @@ func New(db *gorm.DB, git GitClient) *Service {
5555
return &Service{db: db, git: git}
5656
}
5757

58-
// Analyze detects changed functions and calculates risk scores.
59-
// Called from review_changes and pre_merge_check MCP prompts.
60-
//
61-
// @param repoDir git repository root path
62-
// @param baseRef git base reference for diff comparison
63-
// @return risk entries with hunk count and risk score per changed function
64-
// @intent identify high-risk code changes before merge
65-
// @domainRule risk score equals hunk count multiplied by outgoing edge count plus one
66-
// @sideEffect executes git diff via GitClient
67-
// @see impact.Analyzer.ImpactRadius
68-
func (s *Service) Analyze(ctx context.Context, repoDir, baseRef string) ([]RiskEntry, error) {
69-
hunksByFile, files, err := s.collectDiffHunks(ctx, repoDir, baseRef)
70-
if err != nil || hunksByFile == nil {
71-
return nil, err
72-
}
73-
74-
hits, err := matchHunksToNodes(s.db, ctx, files, hunksByFile)
75-
if err != nil || len(hits) == 0 {
76-
return nil, err
77-
}
78-
79-
risks, err := computeRiskScores(s.db, ctx, hits)
80-
if err != nil {
81-
return nil, err
82-
}
83-
sortRiskEntries(risks)
84-
return risks, nil
85-
}
86-
8758
// AnalyzePage detects changed functions and returns one bounded page of risk entries.
8859
// @intent bound handler response allocation while preserving the same sorted risk window that legacy Analyze would expose.
8960
// @domainRule pagination limits returned items, but compute cost still scales with scoring every changed node to preserve legacy ordering.
@@ -149,23 +120,6 @@ func (s *Service) changedNodeHits(ctx context.Context, repoDir, baseRef string)
149120
return matchHunksToNodes(s.db, ctx, files, hunksByFile)
150121
}
151122

152-
// sortRiskEntries orders entries deterministically for stable pagination windows.
153-
// @intent guarantee identical limit/offset slices regardless of map iteration order in computeRiskScores.
154-
func sortRiskEntries(entries []RiskEntry) {
155-
sort.SliceStable(entries, func(i, j int) bool {
156-
if entries[i].RiskScore != entries[j].RiskScore {
157-
return entries[i].RiskScore > entries[j].RiskScore
158-
}
159-
if entries[i].Node.FilePath != entries[j].Node.FilePath {
160-
return entries[i].Node.FilePath < entries[j].Node.FilePath
161-
}
162-
if entries[i].Node.StartLine != entries[j].Node.StartLine {
163-
return entries[i].Node.StartLine < entries[j].Node.StartLine
164-
}
165-
return entries[i].Node.QualifiedName < entries[j].Node.QualifiedName
166-
})
167-
}
168-
169123
// sortNodesForChangeOrder orders changed nodes deterministically for downstream set consumers.
170124
// @intent prevent flow lookups from depending on database or map iteration order.
171125
func sortNodesForChangeOrder(nodes []model.Node) {
@@ -262,22 +216,6 @@ type riskCandidate struct {
262216
riskScore float64
263217
}
264218

265-
// computeRiskScores calculates risk for each hit node based on outgoing edge count.
266-
// @intent weight changed nodes by both diff overlap and graph connectivity to prioritize risky edits.
267-
func computeRiskScores(db *gorm.DB, ctx context.Context, hits map[uint]*hitInfo) ([]RiskEntry, error) {
268-
candidates, err := computeRiskCandidates(db, ctx, hits)
269-
if err != nil {
270-
return nil, err
271-
}
272-
return riskCandidatesToEntries(candidates), nil
273-
}
274-
275-
// computeRiskCandidates calculates sortable risk candidates for each hit node.
276-
// @intent allow paged callers to sort and slice before converting candidates into wire-facing risk entries.
277-
func computeRiskCandidates(db *gorm.DB, ctx context.Context, hits map[uint]*hitInfo) ([]riskCandidate, error) {
278-
return computeTopRiskCandidates(db, ctx, hits, len(hits))
279-
}
280-
281219
// selectTopRiskCandidates returns the best N risk candidates without materializing the full candidate slice.
282220
// @intent preserve AnalyzePage ordering while capping page-path memory and sort work to the requested window.
283221
func selectTopRiskCandidates(db *gorm.DB, ctx context.Context, hits map[uint]*hitInfo, limit int) ([]riskCandidate, error) {

internal/analysis/changes/service_test.go

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ func TestAnalyze_ChangedFunction(t *testing.T) {
8989
hunks: []Hunk{{FilePath: "a.go", StartLine: 15, EndLine: 20}},
9090
}
9191
svc := New(db, git)
92-
got, err := svc.Analyze(context.Background(), ".", "main")
92+
got, err := analyzeAll(svc, context.Background())
9393
if err != nil {
9494
t.Fatal(err)
9595
}
@@ -110,7 +110,7 @@ func TestAnalyze_NoOverlap(t *testing.T) {
110110
hunks: []Hunk{{FilePath: "a.go", StartLine: 1, EndLine: 5}},
111111
}
112112
svc := New(db, git)
113-
got, err := svc.Analyze(context.Background(), ".", "main")
113+
got, err := analyzeAll(svc, context.Background())
114114
if err != nil {
115115
t.Fatal(err)
116116
}
@@ -132,7 +132,7 @@ func TestAnalyze_MultipleHunks(t *testing.T) {
132132
},
133133
}
134134
svc := New(db, git)
135-
got, err := svc.Analyze(context.Background(), ".", "main")
135+
got, err := analyzeAll(svc, context.Background())
136136
if err != nil {
137137
t.Fatal(err)
138138
}
@@ -159,7 +159,7 @@ func TestAnalyze_RiskScoreCalculation(t *testing.T) {
159159
},
160160
}
161161
svc := New(db, git)
162-
got, err := svc.Analyze(context.Background(), ".", "main")
162+
got, err := analyzeAll(svc, context.Background())
163163
if err != nil {
164164
t.Fatal(err)
165165
}
@@ -177,7 +177,7 @@ func TestAnalyze_EmptyDiff(t *testing.T) {
177177

178178
git := &mockGit{files: nil, hunks: nil}
179179
svc := New(db, git)
180-
got, err := svc.Analyze(context.Background(), ".", "main")
180+
got, err := analyzeAll(svc, context.Background())
181181
if err != nil {
182182
t.Fatal(err)
183183
}
@@ -198,7 +198,7 @@ func TestAnalyze_RespectsNamespace(t *testing.T) {
198198
svc := New(db, git)
199199

200200
ctxA := ctxns.WithNamespace(context.Background(), "ns-a")
201-
got, err := svc.Analyze(ctxA, ".", "main")
201+
got, err := analyzeAll(svc, ctxA)
202202
if err != nil {
203203
t.Fatal(err)
204204
}
@@ -288,10 +288,9 @@ func TestAnalyzePage_PreservesAnalyzeOrderingForPagedConsumers(t *testing.T) {
288288
}
289289
svc := New(db, &mockGit{files: files, hunks: hunks})
290290

291-
legacy, err := svc.Analyze(context.Background(), ".", "main")
292-
if err != nil {
293-
t.Fatal(err)
294-
}
291+
// Node i has i outgoing edges, so risk increases with i; the full descending
292+
// order is 4,3,2,1. The {Limit:2, Offset:1} window must therefore be [3, 2],
293+
// pinned to concrete node IDs so the check is independent of AnalyzePage itself.
295294
page, err := svc.AnalyzePage(context.Background(), ".", "main", paging.Request{Limit: 2, Offset: 1})
296295
if err != nil {
297296
t.Fatal(err)
@@ -300,10 +299,15 @@ func TestAnalyzePage_PreservesAnalyzeOrderingForPagedConsumers(t *testing.T) {
300299
if len(page.Items) != 2 {
301300
t.Fatalf("items = %d, want 2", len(page.Items))
302301
}
302+
if page.Items[0].Node.ID != 3 || page.Items[1].Node.ID != 2 {
303+
t.Fatalf("window node IDs = [%d, %d], want [3, 2]", page.Items[0].Node.ID, page.Items[1].Node.ID)
304+
}
305+
if !(page.Items[0].RiskScore > page.Items[1].RiskScore) {
306+
t.Fatalf("expected descending risk across the window, got %.1f then %.1f", page.Items[0].RiskScore, page.Items[1].RiskScore)
307+
}
303308
for i, item := range page.Items {
304-
want := legacy[i+1]
305-
if item.Node.ID != want.Node.ID || item.RiskScore != want.RiskScore || item.HunkCount != want.HunkCount {
306-
t.Fatalf("page item %d = %+v, want legacy slice item %+v", i, item, want)
309+
if item.HunkCount != 1 {
310+
t.Fatalf("item %d HunkCount = %d, want 1", i, item.HunkCount)
307311
}
308312
}
309313
}
@@ -328,7 +332,7 @@ func TestAnalyzePage_MatchesAnalyzeAcrossRepresentativeWindows(t *testing.T) {
328332
}
329333
svc := New(db, &mockGit{files: files, hunks: hunks})
330334

331-
legacy, err := svc.Analyze(context.Background(), ".", "main")
335+
legacy, err := analyzeAll(svc, context.Background())
332336
if err != nil {
333337
t.Fatal(err)
334338
}
@@ -417,3 +421,10 @@ func TestAnalyzePage_OffsetBeyondTotalReturnsEmpty(t *testing.T) {
417421
t.Fatal("has_more = true, want false")
418422
}
419423
}
424+
425+
// analyzeAll returns the full, unpaged risk-entry ordering via AnalyzePage,
426+
// used as the reference for verifying paged windows.
427+
func analyzeAll(svc *Service, ctx context.Context) ([]RiskEntry, error) {
428+
page, err := svc.AnalyzePage(ctx, ".", "main", paging.Request{Limit: 500, Offset: 0})
429+
return page.Items, err
430+
}

internal/ragindex/builder.go

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -196,44 +196,6 @@ func FindNode(root *TreeNode, id string) *TreeNode {
196196
return nil
197197
}
198198

199-
// PruneTree는 root 트리를 maxDepth 깊이까지만 포함한 새 트리를 반환한다.
200-
// maxDepth <= 0이면 전체 트리를 반환한다. 원본 트리는 변경하지 않는다.
201-
// depth 계산: root는 depth 0, root의 직계 자식은 depth 1.
202-
// @intent 대형 인덱스를 요약 응답에 맞게 깊이 제한된 복사본으로 축약한다.
203-
// @ensures 원본 트리는 수정되지 않는다.
204-
func PruneTree(root *TreeNode, maxDepth int) *TreeNode {
205-
if root == nil {
206-
return nil
207-
}
208-
return pruneNode(root, 0, maxDepth)
209-
}
210-
211-
// pruneNode copies one subtree while enforcing the depth limit.
212-
// @intent PruneTree의 재귀 복사 작업을 깊이 기준으로 수행한다.
213-
// @return 자식이 제한된 새로운 TreeNode 복사본을 반환한다.
214-
func pruneNode(n *TreeNode, currentDepth, maxDepth int) *TreeNode {
215-
copied := &TreeNode{
216-
ID: n.ID,
217-
Label: n.Label,
218-
Kind: n.Kind,
219-
Summary: n.Summary,
220-
DocPath: n.DocPath,
221-
SearchText: n.SearchText,
222-
HasChildren: len(n.Children) > 0 || n.HasChildren,
223-
FieldTexts: n.FieldTexts,
224-
Details: n.Details,
225-
}
226-
if maxDepth <= 0 || currentDepth < maxDepth {
227-
copied.Children = make([]*TreeNode, 0, len(n.Children))
228-
for _, child := range n.Children {
229-
copied.Children = append(copied.Children, pruneNode(child, currentDepth+1, maxDepth))
230-
}
231-
} else {
232-
copied.Children = []*TreeNode{}
233-
}
234-
return copied
235-
}
236-
237199
// LoadIndex reads a persisted Index (e.g. wiki-index.json) from disk.
238200
// @intent let the wiki index round-trip its written tree without the RAG-index builder.
239201
func LoadIndex(path string) (*Index, error) {

internal/ragindex/builder_test.go

Lines changed: 0 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -34,63 +34,6 @@ func setupDB(t *testing.T) *gorm.DB {
3434
return db
3535
}
3636

37-
// TestPruneTree_Depth1: depth=1이면 root와 직계 자식만 반환, 손자 노드 없음.
38-
func TestPruneTree_Depth1(t *testing.T) {
39-
root := &ragindex.TreeNode{
40-
ID: "root",
41-
Label: "Root",
42-
Children: []*ragindex.TreeNode{
43-
{
44-
ID: "c1",
45-
Label: "Community 1",
46-
Children: []*ragindex.TreeNode{
47-
{ID: "f1", Label: "file.go"},
48-
},
49-
},
50-
},
51-
}
52-
53-
result := ragindex.PruneTree(root, 1)
54-
if len(result.Children) != 1 {
55-
t.Fatalf("expected 1 child, got %d", len(result.Children))
56-
}
57-
if len(result.Children[0].Children) != 0 {
58-
t.Fatalf("expected 0 grandchildren at depth=1, got %d", len(result.Children[0].Children))
59-
}
60-
// 원본 트리는 변경되지 않아야 함
61-
if len(root.Children[0].Children) != 1 {
62-
t.Fatal("PruneTree must not modify the original tree")
63-
}
64-
}
65-
66-
// TestPruneTree_NegativeDepth: depth <= 0이면 트리 전체를 반환한다.
67-
func TestPruneTree_NegativeDepth(t *testing.T) {
68-
root := &ragindex.TreeNode{
69-
ID: "root",
70-
Children: []*ragindex.TreeNode{
71-
{ID: "c1", Children: []*ragindex.TreeNode{{ID: "f1"}}},
72-
},
73-
}
74-
75-
for _, depth := range []int{0, -1} {
76-
result := ragindex.PruneTree(root, depth)
77-
if len(result.Children) != 1 {
78-
t.Fatalf("depth=%d: expected 1 child, got %d", depth, len(result.Children))
79-
}
80-
if len(result.Children[0].Children) != 1 {
81-
t.Fatalf("depth=%d: expected 1 grandchild (unlimited), got %d", depth, len(result.Children[0].Children))
82-
}
83-
}
84-
}
85-
86-
// TestPruneTree_NilRoot: nil 입력 → nil 반환.
87-
func TestPruneTree_NilRoot(t *testing.T) {
88-
result := ragindex.PruneTree(nil, 2)
89-
if result != nil {
90-
t.Fatal("expected nil for nil root")
91-
}
92-
}
93-
9437
// TestSearch_MatchesLabel: query가 label에 매칭되면 결과를 반환한다.
9538
func TestSearch_MatchesLabel(t *testing.T) {
9639
root := &ragindex.TreeNode{

0 commit comments

Comments
 (0)