Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
49 changes: 45 additions & 4 deletions cmd/opencodereview/review_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,17 @@ func runReview(args []string) error {
return runPreview(repoDir, opts, fileFilter)
}

mode := tool.ParseReviewMode(opts.from, opts.to, opts.commit)
ref, _ := mode.RefValue(opts.to, opts.commit)

toolEntries, err := toolsconfig.Load(opts.toolConfigPath)
if err != nil {
return fmt.Errorf("load tools: %w", err)
}
codeGraph := detectCodeGraphForReview(repoDir, ref)
if !codeGraph.Available {
toolEntries = toolsconfig.ExcludeByName(toolEntries, tool.CodeGraphContext.Name())
}
planToolDefs := agent.BuildToolDefs(toolEntries, true)
mainToolDefs := agent.BuildToolDefs(toolEntries, false)

Expand Down Expand Up @@ -101,15 +108,13 @@ func runReview(args []string) error {
gitRunner := gitcmd.New(opts.maxGitProcs)

collector := tool.NewCommentCollector()
mode := tool.ParseReviewMode(opts.from, opts.to, opts.commit)
ref, _ := mode.RefValue(opts.to, opts.commit)
fileReader := &tool.FileReader{
RepoDir: repoDir,
Mode: mode,
Ref: ref,
Runner: gitRunner,
}
tools := buildToolRegistry(collector, fileReader)
tools := buildToolRegistry(collector, fileReader, codeGraph)

ag := agent.New(agent.Args{
RepoDir: repoDir,
Expand Down Expand Up @@ -269,12 +274,48 @@ func runPreview(repoDir string, opts reviewOptions, fileFilter *rules.FileFilter
return nil
}

func buildToolRegistry(collector *tool.CommentCollector, fr *tool.FileReader) *tool.Registry {
func detectCodeGraphForReview(repoDir, ref string) tool.CodeGraphAvailability {
codeGraph := tool.DetectCodeGraph(repoDir)
if !codeGraph.Available || ref == "" {
return codeGraph
}

head, err := resolveCommit(repoDir, "HEAD")
if err != nil {
codeGraph.Available = false
codeGraph.Reason = "cannot resolve HEAD for CodeGraph ref compatibility check"
return codeGraph
}
target, err := resolveCommit(repoDir, ref)
if err != nil {
codeGraph.Available = false
codeGraph.Reason = "cannot resolve review target ref for CodeGraph compatibility check"
return codeGraph
}
if head != target {
codeGraph.Available = false
codeGraph.Reason = "CodeGraph index is for current checkout, which differs from review target ref"
}
return codeGraph
}

func resolveCommit(repoDir, ref string) (string, error) {
out, err := runGitCmd(repoDir, "rev-parse", "--verify", "--end-of-options", ref+"^{commit}")
if err != nil {
return "", err
}
return strings.TrimSpace(string(out)), nil
}

func buildToolRegistry(collector *tool.CommentCollector, fr *tool.FileReader, codeGraph tool.CodeGraphAvailability) *tool.Registry {
reg := tool.NewRegistry()
reg.Register(tool.NewFileRead(fr))
reg.Register(tool.NewFileFind(fr))
reg.Register(tool.NewFileReadDiff(tool.DiffMap{}))
reg.Register(tool.NewCodeSearch(fr))
if codeGraph.Available {
reg.Register(tool.NewCodeGraph(fr.RepoDir, codeGraph.BinPath))
}
reg.Register(&tool.CodeCommentProvider{Collector: collector})
return reg
}
46 changes: 45 additions & 1 deletion internal/config/toolsconfig/tools.json
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,50 @@
}
}
},
{
"name": "code_graph_context",
"plan_task": true,
"main_task": true,
"definition": {
"name": "code_graph_context",
"description": "Use this optional structural-code tool when text search is not enough and you need to understand symbols, callers, callees, or impact radius. Prefer this tool for changes to public functions, APIs, interfaces, method signatures, shared utilities, route handlers, data models, security/auth logic, concurrency/lifecycle code, or when a risk depends on how other files call the changed symbol. Do not use it for simple literal text lookup; use code_search instead. This tool is only available when a compatible CodeGraph index is detected for the repository.",
"parameters": {
"type": "object",
"properties": {
"mode": {
"type": "string",
"enum": [
"explore",
"search",
"callers",
"callees",
"impact"
],
"description": "Structural query mode. Use search to find symbols, explore for source plus relationship context, callers/callees for dependency direction, and impact to estimate what may be affected by changing a symbol. Defaults to explore."
},
"query": {
"type": "string",
"description": "A symbol name, file path, or concise structural query. Prefer exact function/type/class/interface names when available."
},
"kind": {
"type": "string",
"description": "Optional symbol kind filter for mode=search, such as function, method, class, interface, type, variable, route, or component."
},
"limit": {
"type": "integer",
"description": "Maximum number of symbols or relationships to return for search/callers/callees. Defaults to 12, maximum 30."
},
"max_files": {
"type": "integer",
"description": "Maximum number of source files to include for mode=explore. Defaults to 4, maximum 8."
}
},
"required": [
"query"
]
}
}
},
{
"name": "file_find",
"plan_task": true,
Expand All @@ -180,4 +224,4 @@
}
}
}
]
]
19 changes: 19 additions & 0 deletions internal/config/toolsconfig/toolsconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,25 @@ func Load(path string) ([]ToolConfigEntry, error) {
return tools, nil
}

// ExcludeByName returns entries excluding any tool whose name appears in names.
func ExcludeByName(entries []ToolConfigEntry, names ...string) []ToolConfigEntry {
if len(names) == 0 {
return entries
}
excluded := make(map[string]bool, len(names))
for _, name := range names {
excluded[name] = true
}
out := make([]ToolConfigEntry, 0, len(entries))
for _, entry := range entries {
if excluded[entry.Name] {
continue
}
out = append(out, entry)
}
return out
}

// ToolDefsByPhase returns the parsed tool definitions filtered by phase.
// planOnly=true returns only tools with plan_task:true.
// planOnly=false returns only tools with main_task:true.
Expand Down
21 changes: 21 additions & 0 deletions internal/config/toolsconfig/toolsconfig_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package toolsconfig

import "testing"

func TestExcludeByName(t *testing.T) {
entries := []ToolConfigEntry{
{Name: "code_search"},
{Name: "code_graph_context"},
{Name: "file_read"},
}

filtered := ExcludeByName(entries, "code_graph_context")
if len(filtered) != 2 {
t.Fatalf("expected 2 entries, got %d", len(filtered))
}
for _, entry := range filtered {
if entry.Name == "code_graph_context" {
t.Fatal("expected code_graph_context to be filtered")
}
}
}
Loading