fix(focus,find): fix relationship type case, empty CalledBy/Imports, stale API#68
Conversation
All API relationship types are lowercase (calls, imports, defines_function, declares_class) but focus and find used uppercase literals, causing: - focus: Imports and CalledBy sections always empty - focus: Type extraction (--types flag) never returned results - find: Callers, callees, and defined_in always empty Also fixed: - callerFile lookup used "file"/"path" props; Function nodes store their file in "filePath" — causes empty file in CalledBy output - find/handler.go: nil dereference on defFile lookup when StartNode missing - focus/getGraph and find/getGraph: created zip before checking cache and called the old client.Analyze endpoint; now delegate to analyze.GetGraph() which uses git fingerprinting for fast cache hits and AnalyzeShards - Removed dead newAPIClient wrapper from focus/zip.go Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
WalkthroughGraph relation type strings were normalized from uppercase to lowercase and node property lookups were adjusted. Local ZIP/cache/getGraph logic was removed in favor of delegating to Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as "cmd/<command>"
participant Handler as "internal/*/handler.go"
participant Analyze as "internal/analyze (Analyze API)"
participant Zip as "local zip/cache (removed)"
CLI->>Handler: Run command (e.g., find/focus)
Handler->>Analyze: analyze.GetGraph(ctx, cfg, dir, force)
Analyze-->>Handler: graph data (nodes/relations)
Handler->>Handler: process relations (calls, defines, imports)
Note over Zip,Handler: previously Handler -> Zip -> cache -> Analyze flow removed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/find/handler_test.go (1)
200-203:⚠️ Potential issue | 🟡 MinorTest helper
makeGraph()still uses uppercase"CALLS"Heads up! The
makeGraph()helper on line 201 still uses uppercase"CALLS":{ID: "r1", Type: "CALLS", StartNode: "fn3", EndNode: "fn1"},The tests using this helper (
TestSearch_BasicMatch,TestSearch_CaseInsensitive, etc.) still pass because they don't assert onCallers/Calleesfields. But this inconsistency could trip someone up later if they add a test that does check those fields.Consider updating to lowercase for consistency with the API and the other test fixtures in this file:
Proposed fix
Relationships: []api.Relationship{ - {ID: "r1", Type: "CALLS", StartNode: "fn3", EndNode: "fn1"}, + {ID: "r1", Type: "calls", StartNode: "fn3", EndNode: "fn1"}, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/find/handler_test.go` around lines 200 - 203, Update the test helper makeGraph() so the Relationship Type uses lowercase "calls" (i.e., set Relationships: []api.Relationship{{ID: "r1", Type: "calls", StartNode: "fn3", EndNode: "fn1"}}) to match the API and other fixtures; modify the api.Relationship.Type value in makeGraph() and run the tests (including TestSearch_BasicMatch and TestSearch_CaseInsensitive) to ensure nothing else relies on the uppercase value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/find/handler.graph.go`:
- Line 29: The call-graph metadata is stale: update the generated metadata so it
reflects that getGraph now calls analyze.GetGraph(); regenerate the call-graph
metadata file (the handler.graph.go metadata) using the project's call-graph
generation tool/command so the entry becomes "getGraph → analyze.GetGraph()"
instead of the old "getGraph → Analyze". After regenerating, commit the updated
metadata file.
---
Outside diff comments:
In `@internal/find/handler_test.go`:
- Around line 200-203: Update the test helper makeGraph() so the Relationship
Type uses lowercase "calls" (i.e., set Relationships: []api.Relationship{{ID:
"r1", Type: "calls", StartNode: "fn3", EndNode: "fn1"}}) to match the API and
other fixtures; modify the api.Relationship.Type value in makeGraph() and run
the tests (including TestSearch_BasicMatch and TestSearch_CaseInsensitive) to
ensure nothing else relies on the uppercase value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0186f061-18db-4206-8c76-03bde52dcb3d
📒 Files selected for processing (13)
internal/find/doc.graph.gointernal/find/handler.gointernal/find/handler.graph.gointernal/find/handler_test.gointernal/find/handler_test.graph.gointernal/find/integration_test.graph.gointernal/find/zip.graph.gointernal/find/zip_test.graph.gointernal/focus/doc.graph.gointernal/focus/handler.gointernal/focus/handler.graph.gointernal/focus/zip.gointernal/focus/zip.graph.go
💤 Files with no reviewable changes (1)
- internal/focus/zip.go
| // Run → printMatches internal/find/handler.go:120 | ||
| // Run → ParseFormat internal/ui/output.go:24 | ||
| // getGraph ← Run internal/find/handler.go:36 | ||
| // getGraph → Analyze internal/api/client.go:41 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify what getGraph calls in the refactored handler.go
# Look for the getGraph function implementation and its call sites
rg -A 10 'func getGraph' internal/find/handler.goRepository: supermodeltools/cli
Length of output: 233
Regenerate the call-graph metadata file.
The metadata at line 29 is stale. Your code actually does what the PR says—getGraph now calls analyze.GetGraph() instead of the old deprecated endpoint. I verified the actual code:
func getGraph(ctx context.Context, cfg *config.Config, dir string, force bool) (*api.Graph, error) {
g, _, err := analyze.GetGraph(ctx, cfg, dir, force)
return g, err
}Perfect. But the generated metadata file still shows the old call path (getGraph → Analyze). Since this is an auto-generated documentation file, just regenerate it with whatever tool/command creates it, and it'll reflect the current call graph.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/find/handler.graph.go` at line 29, The call-graph metadata is stale:
update the generated metadata so it reflects that getGraph now calls
analyze.GetGraph(); regenerate the call-graph metadata file (the
handler.graph.go metadata) using the project's call-graph generation
tool/command so the entry becomes "getGraph → analyze.GetGraph()" instead of the
old "getGraph → Analyze". After regenerating, commit the updated metadata file.
focus/zip.go was entirely dead code after getGraph() was replaced with analyze.GetGraph(). golangci-lint flagged createZip, skipDirs, isGitRepo, gitArchive, walkZip as unused. Delete the file. Also run goimports on modified files to satisfy formatting check. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The focus package had zero test coverage. New tests cover: - pathMatches() helper (exact, suffix, dot-slash prefix) - extract() core logic: file lookup, imports BFS, functions/callees, CalledBy population, self-call exclusion - Relationship type case regression guard: verifies uppercase rel types (DEFINES_FUNCTION, CALLS) do not match, confirming the #68 fix - render() for both JSON and markdown output formats Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The focus package had zero test coverage. New tests cover: - pathMatches() helper (exact, suffix, dot-slash prefix) - extract() core logic: file lookup, imports BFS, functions/callees, CalledBy population, self-call exclusion - Relationship type case regression guard: verifies uppercase rel types (DEFINES_FUNCTION, CALLS) do not match, confirming the #68 fix - render() for both JSON and markdown output formats Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Three commands (
focus,find) were silently broken due to relationship type case mismatches and stale code paths.Relationship type case (all three)
The Supermodel API returns relationship types in lowercase (
calls,imports,defines_function,declares_class) but the handlers used uppercase literals. This caused:supermodel focus <file>— Imports and Called by sections always empty;--typesnever returned resultssupermodel find <symbol>— Callers, Callees, and Defined in always emptyfilePath property lookup (focus, find)
callerNode.Prop("file", "path")missed the actual property namefilePathused by Function nodes, causing caller file paths to appear as empty()in focus output and emptyfilefields in find JSON output.Stale getGraph implementations (focus, find)
Both packages had their own
getGraphthat:client.Analyzeendpoint instead ofAnalyzeShardsNow both delegate to
analyze.GetGraph(), which uses git fingerprinting for ~1ms cache checks and the currentAnalyzeShardsendpoint.Other
find/handler.godefFileassignmentnewAPIClientwrapper fromfocus/zip.goTest plan
supermodel focus internal/analyze/handler.goshows non-empty Imports and Called by sectionssupermodel find GetGraphshows non-empty Callers, Callees, Defined in fieldsgo test ./internal/find/... ./internal/focus/...passes🤖 Generated with Claude Code
Summary by CodeRabbit
Refactor
Chores