Skip to content

Commit 453a72f

Browse files
aksOpsclaude
andcommitted
feat(review): MR-review pipeline + review_changes MCP tool + codeiq review CLI
Plan Phase 3 — `codeiq review` and the `review_changes` MCP tool: index + LLM review of a PR diff against the indexed graph. Pieces: - internal/review/diff.go — ParseDiff + GitDiff: shells `git diff` and parses the unified output into per-file ChangedFile{Path, Hunks, AddedLines, RemovedLines}. - internal/review/config.go — Config + DefaultConfig. Targets local Ollama by default; OLLAMA_API_KEY flips to Ollama Cloud (gpt-oss:20b). - internal/review/client.go — HTTP wrapper over the OpenAI-compatible /chat/completions endpoint Ollama (and most LLM proxies) expose. Single hard-coded system prompt; user prompt is the assembled diff + evidence. Strict JSON response shape: {summary, findings:[{file,line,severity,comment}]}. - internal/review/service.go — Orchestrator. Diff → prompt → Client.Review. GraphContext interface lets cli/mcp inject graph evidence; nil means diff-only. - internal/cli/review.go — `codeiq review [path]` subcommand with --base/--head/--model/--out/--format=markdown|json/--focus. - internal/mcp/tools_review.go — `review_changes` MCP tool (consolidated alongside the other 6 phase-2 tools). Tests (TDD per CLAUDE.md): - TestParseDiff_FileWithSingleHunk / MultipleFiles / Empty - TestClient_Review_HappyPath / NoBearerWhenKeyEmpty / NonJSON / HTTPError (all stub the LLM via httptest) - TestService_BuildPrompt_HasFilesAndEvidence - TestService_Review_EndToEnd_FixtureRepo (builds a 2-commit git fixture in t.TempDir(), stubs the LLM, asserts the report flows end to end) Strict read-only-graph invariant: the MCP tool path never mutates the cache or Kuzu store. `codeiq review` from the CLI runs index + enrich before review when the graph is stale. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent df68978 commit 453a72f

10 files changed

Lines changed: 879 additions & 0 deletions

File tree

go/internal/cli/review.go

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
package cli
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
"fmt"
7+
"os"
8+
"path/filepath"
9+
"strings"
10+
"time"
11+
12+
"github.com/randomcodespace/codeiq/go/internal/review"
13+
14+
"github.com/spf13/cobra"
15+
)
16+
17+
func init() {
18+
registerSubcommand(func() *cobra.Command {
19+
var (
20+
base string
21+
head string
22+
model string
23+
outFile string
24+
format string
25+
focus []string
26+
)
27+
cmd := &cobra.Command{
28+
Use: "review [path]",
29+
Short: "LLM-driven review of a PR diff against the indexed graph.",
30+
Long: `Run an LLM review of git diff base..head, using the codeiq graph
31+
as evidence context. Defaults: base=HEAD~1, head=HEAD, model=gpt-oss:20b
32+
via local Ollama (set OLLAMA_API_KEY for Ollama Cloud).
33+
34+
Output formats:
35+
--format=markdown (default) human-readable review
36+
--format=json structured Report for piping into other tools
37+
38+
Plan §3 — Phase 3 of the optimization plan.`,
39+
Example: ` codeiq review --base origin/main --head HEAD
40+
OLLAMA_API_KEY=... codeiq review --model gpt-oss:120b
41+
codeiq review --base v1.0 --head v1.1 --out review.md`,
42+
Args: cobra.MaximumNArgs(1),
43+
RunE: func(cmd *cobra.Command, args []string) error {
44+
path := "."
45+
if len(args) == 1 {
46+
path = args[0]
47+
}
48+
abs, err := filepath.Abs(path)
49+
if err != nil {
50+
return err
51+
}
52+
cfg := review.DefaultConfig()
53+
if model != "" {
54+
cfg.Model = model
55+
}
56+
client := review.NewClient(cfg)
57+
svc := review.NewService(client, nil)
58+
59+
ctx, cancel := context.WithTimeout(cmd.Context(), cfg.Timeout+30*time.Second)
60+
defer cancel()
61+
rep, err := svc.Review(ctx, abs, base, head, focus)
62+
if err != nil {
63+
return fmt.Errorf("review: %w", err)
64+
}
65+
66+
var rendered string
67+
if format == "json" {
68+
b, _ := json.MarshalIndent(rep, "", " ")
69+
rendered = string(b) + "\n"
70+
} else {
71+
rendered = renderMarkdown(rep)
72+
}
73+
if outFile == "" {
74+
fmt.Fprint(cmd.OutOrStdout(), rendered)
75+
return nil
76+
}
77+
return os.WriteFile(outFile, []byte(rendered), 0644)
78+
},
79+
}
80+
cmd.Flags().StringVar(&base, "base", "", "Base git ref (default: HEAD~1)")
81+
cmd.Flags().StringVar(&head, "head", "", "Head git ref (default: HEAD)")
82+
cmd.Flags().StringVar(&model, "model", "", "Override LLM model (default: from config)")
83+
cmd.Flags().StringVarP(&outFile, "out", "o", "", "Write output to file instead of stdout")
84+
cmd.Flags().StringVar(&format, "format", "markdown", "Output format: markdown | json")
85+
cmd.Flags().StringSliceVar(&focus, "focus", nil, "Limit review to these file paths")
86+
return cmd
87+
})
88+
}
89+
90+
func renderMarkdown(rep *review.Report) string {
91+
var b strings.Builder
92+
fmt.Fprintf(&b, "# Code Review (model: %s)\n\n", rep.Model)
93+
fmt.Fprintf(&b, "## Summary\n\n%s\n\n", rep.Summary)
94+
if len(rep.Findings) == 0 {
95+
b.WriteString("## Findings\n\nNo findings.\n")
96+
return b.String()
97+
}
98+
b.WriteString("## Findings\n\n")
99+
for _, f := range rep.Findings {
100+
loc := f.File
101+
if f.Line > 0 {
102+
loc = fmt.Sprintf("%s:%d", f.File, f.Line)
103+
}
104+
fmt.Fprintf(&b, "- **[%s] %s** — %s\n", strings.ToUpper(f.Severity), loc, f.Comment)
105+
}
106+
return b.String()
107+
}

go/internal/mcp/tools_consolidated.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ func RegisterConsolidated(srv *Server, d *Deps) error {
3232
toolTraceRelationships(d),
3333
toolAnalyzeImpact(d),
3434
toolTopologyView(d),
35+
toolReviewChanges(d),
3536
} {
3637
if err := srv.Register(t); err != nil {
3738
return fmt.Errorf("mcp: register consolidated tool %q: %w", t.Name, err)

go/internal/mcp/tools_review.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
package mcp
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
"fmt"
7+
8+
"github.com/randomcodespace/codeiq/go/internal/review"
9+
)
10+
11+
// toolReviewChanges — Plan §3.3. LLM-driven review of base..head against
12+
// the indexed graph. Strictly read-only against the graph: the tool does
13+
// not mutate the cache or Kuzu store.
14+
//
15+
// The graph must already be enriched at the PR HEAD before this tool is
16+
// called — for that flow, use `codeiq review` from the CLI which runs
17+
// index + enrich + review in sequence.
18+
func toolReviewChanges(d *Deps) Tool {
19+
return Tool{
20+
Name: "review_changes",
21+
Description: "Run an LLM-driven review of a git diff (base_ref..head_ref) " +
22+
"against the indexed code graph. Computes evidence per changed " +
23+
"file (graph blast radius, dependencies) and asks the configured " +
24+
"LLM (default: local Ollama, set OLLAMA_API_KEY for Ollama Cloud) " +
25+
"for review comments. The graph must already be enriched at the PR " +
26+
"HEAD — run 'codeiq review' for the end-to-end flow.",
27+
Schema: json.RawMessage(`{"type":"object","properties":{"base_ref":{"type":"string"},"head_ref":{"type":"string"},"model":{"type":"string"},"focus_files":{"type":"array","items":{"type":"string"}}}}`),
28+
Handler: func(ctx context.Context, raw json.RawMessage) (any, error) {
29+
var p struct {
30+
BaseRef string `json:"base_ref"`
31+
HeadRef string `json:"head_ref"`
32+
Model string `json:"model"`
33+
FocusFiles []string `json:"focus_files"`
34+
}
35+
_ = json.Unmarshal(raw, &p)
36+
if d.RootPath == "" {
37+
return NewErrorEnvelope(CodeInternalError, fmt.Errorf("review_changes requires a working tree root; set RootPath when wiring the MCP server"), RequestID(ctx)), nil
38+
}
39+
cfg := review.DefaultConfig()
40+
if p.Model != "" {
41+
cfg.Model = p.Model
42+
}
43+
svc := review.NewService(review.NewClient(cfg), nil)
44+
rep, err := svc.Review(ctx, d.RootPath, p.BaseRef, p.HeadRef, p.FocusFiles)
45+
if err != nil {
46+
return NewErrorEnvelope(CodeInternalError, fmt.Errorf("review: %w", err), RequestID(ctx)), nil
47+
}
48+
rep.RequestID = RequestID(ctx)
49+
return rep, nil
50+
},
51+
}
52+
}

go/internal/review/client.go

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
package review
2+
3+
import (
4+
"bytes"
5+
"context"
6+
"encoding/json"
7+
"fmt"
8+
"io"
9+
"net/http"
10+
)
11+
12+
// Finding is one structured review comment from the LLM.
13+
type Finding struct {
14+
File string `json:"file"`
15+
Line int `json:"line,omitempty"`
16+
Severity string `json:"severity"` // info | low | medium | high | critical
17+
Comment string `json:"comment"`
18+
}
19+
20+
// Report is the structured LLM output. Both CLI (markdown) and MCP (JSON)
21+
// paths consume this shape.
22+
type Report struct {
23+
Summary string `json:"summary"`
24+
Findings []Finding `json:"findings"`
25+
Model string `json:"model"`
26+
RequestID string `json:"request_id,omitempty"`
27+
}
28+
29+
// SystemPrompt is the single system message we use for every review.
30+
// Plan §3.1 — "use the structured graph evidence to find correctness,
31+
// security, and architectural issues".
32+
const SystemPrompt = `You are reviewing a pull request. Use the structured graph evidence to find correctness, security, and architectural issues. ` +
33+
`Return strictly JSON in this shape: ` +
34+
`{"summary": "<one-paragraph overview>", "findings": [{"file": "<path>", "line": <int>, "severity": "info|low|medium|high|critical", "comment": "<message>"}]}. ` +
35+
`No prose before or after the JSON. ` +
36+
`If the diff is trivial, return an empty findings array — do NOT invent issues.`
37+
38+
// Client wraps the OpenAI-compatible /chat/completions endpoint exposed
39+
// by Ollama, Ollama Cloud, and proxies. The HTTPClient field is exported
40+
// so tests can inject a stub.
41+
type Client struct {
42+
Config Config
43+
HTTPClient *http.Client
44+
}
45+
46+
// NewClient returns a Client with cfg and a default *http.Client.
47+
func NewClient(cfg Config) *Client {
48+
return &Client{
49+
Config: cfg,
50+
HTTPClient: &http.Client{Timeout: cfg.Timeout},
51+
}
52+
}
53+
54+
type chatRequest struct {
55+
Model string `json:"model"`
56+
Messages []chatMessage `json:"messages"`
57+
Stream bool `json:"stream"`
58+
}
59+
60+
type chatMessage struct {
61+
Role string `json:"role"`
62+
Content string `json:"content"`
63+
}
64+
65+
type chatResponse struct {
66+
ID string `json:"id"`
67+
Model string `json:"model"`
68+
Choices []struct {
69+
Message chatMessage `json:"message"`
70+
} `json:"choices"`
71+
}
72+
73+
// Review sends the assembled prompt to the LLM and parses the structured
74+
// reply into a Report. The user prompt should already include the diff
75+
// + evidence pack rendering.
76+
func (c *Client) Review(ctx context.Context, userPrompt string) (*Report, error) {
77+
body, err := json.Marshal(chatRequest{
78+
Model: c.Config.Model,
79+
Stream: false,
80+
Messages: []chatMessage{
81+
{Role: "system", Content: SystemPrompt},
82+
{Role: "user", Content: userPrompt},
83+
},
84+
})
85+
if err != nil {
86+
return nil, fmt.Errorf("marshal request: %w", err)
87+
}
88+
req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.Config.Endpoint+"/chat/completions", bytes.NewReader(body))
89+
if err != nil {
90+
return nil, err
91+
}
92+
req.Header.Set("Content-Type", "application/json")
93+
if c.Config.APIKey != "" {
94+
req.Header.Set("Authorization", "Bearer "+c.Config.APIKey)
95+
}
96+
resp, err := c.HTTPClient.Do(req)
97+
if err != nil {
98+
return nil, fmt.Errorf("LLM call: %w", err)
99+
}
100+
defer resp.Body.Close()
101+
raw, err := io.ReadAll(resp.Body)
102+
if err != nil {
103+
return nil, err
104+
}
105+
if resp.StatusCode >= 400 {
106+
return nil, fmt.Errorf("LLM HTTP %d: %s", resp.StatusCode, snippet(string(raw)))
107+
}
108+
var cr chatResponse
109+
if err := json.Unmarshal(raw, &cr); err != nil {
110+
return nil, fmt.Errorf("decode chat response: %w (body: %s)", err, snippet(string(raw)))
111+
}
112+
if len(cr.Choices) == 0 {
113+
return nil, fmt.Errorf("LLM returned no choices: %s", snippet(string(raw)))
114+
}
115+
var rep Report
116+
content := cr.Choices[0].Message.Content
117+
if err := json.Unmarshal([]byte(content), &rep); err != nil {
118+
return nil, fmt.Errorf("LLM did not return strict JSON: %w (content: %s)", err, snippet(content))
119+
}
120+
if rep.Model == "" {
121+
rep.Model = cr.Model
122+
}
123+
return &rep, nil
124+
}
125+
126+
func snippet(s string) string {
127+
const max = 500
128+
if len(s) > max {
129+
return s[:max] + "..."
130+
}
131+
return s
132+
}

0 commit comments

Comments
 (0)