From b65d629d490a6460fccfeff626df1181703545f9 Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Sat, 26 Jul 2025 18:52:30 -0500 Subject: [PATCH 01/20] feat: implement json output for review suggestions --- cmd/main/main.go | 68 ++++++++++++++++++++------------- internal/agents/reviewer.go | 26 ++++++++++++- internal/prompts/code-review.go | 32 ++++++++-------- internal/prompts/diff-review.go | 37 +++++++++--------- 4 files changed, 102 insertions(+), 61 deletions(-) diff --git a/cmd/main/main.go b/cmd/main/main.go index 1dc6571..f57e1c4 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -218,21 +218,32 @@ func (r *ReviewCmd) Run(cli *CLI) error { return fmt.Errorf("review failed: %w", err) } - // Format the output to include diffs - formatter := diff.NewFormatter() - formattedContent := formatter.Format(result.Content) + if len(result.Suggestions) == 0 { + fmt.Println("āœ… No issues found.") + return nil + } - // Apply glamour rendering if requested - if r.OutputStyle == "rich" { - rendered, err := renderRichOutput(formattedContent) - if err != nil { - log.Printf("Failed to initialize rich renderer: %v", err) - fmt.Println(formattedContent) + fmt.Printf("# šŸ² miso Code review for %s\n\n", filename) + + formatter := diff.NewFormatter() + for _, suggestion := range result.Suggestions { + // Format the body to render diffs correctly + formattedBody := formatter.Format(suggestion.Body) + fullSuggestion := fmt.Sprintf("## %s\n%s", suggestion.Title, formattedBody) + + // Apply glamour rendering if requested + if r.OutputStyle == "rich" { + rendered, err := renderRichOutput(fullSuggestion) + if err != nil { + log.Printf("Failed to initialize rich renderer: %v", err) + fmt.Println(fullSuggestion) // Fallback to plain + } else { + fmt.Print(rendered) + } } else { - fmt.Print(rendered) + fmt.Println(fullSuggestion) } - } else { - fmt.Println(formattedContent) + fmt.Println() // Add a newline for separation } // Display token usage if available @@ -435,18 +446,18 @@ func (gr *GitHubReviewPRCmd) Run(cli *CLI) error { continue } - // Format the output to include diffs - formattedContent := formatter.Format(result.Content) - - if formattedContent != "" { + if len(result.Suggestions) > 0 { reviewOutput.WriteString(fmt.Sprintf("
\n")) reviewOutput.WriteString( fmt.Sprintf( - "šŸ“ Review for %s\n\n", file, + "šŸ“ Review for %s (%d issues)\n\n", file, len(result.Suggestions), ), ) - reviewOutput.WriteString(formattedContent) - reviewOutput.WriteString(fmt.Sprintf("\n
\n")) + for _, suggestion := range result.Suggestions { + formattedBody := formatter.Format(suggestion.Body) + reviewOutput.WriteString(fmt.Sprintf("### %s\n%s\n\n", suggestion.Title, formattedBody)) + } + reviewOutput.WriteString("\n") } if result.TokensUsed > 0 { @@ -455,7 +466,12 @@ func (gr *GitHubReviewPRCmd) Run(cli *CLI) error { } // Post to GitHub - commentBody := fmt.Sprintf("# šŸ² miso Code review\n\n%s", reviewOutput.String()) + var commentBody string + if reviewOutput.Len() > 0 { + commentBody = fmt.Sprintf("# šŸ² miso Code review\n\n%s", reviewOutput.String()) + } else { + commentBody = "# šŸ² miso Code review\n\nāœ… No issues found." + } ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() if err := ghClient.PostOrUpdateComment(ctx, prNumber, commentBody); err != nil { @@ -591,15 +607,15 @@ func (d *DiffCmd) Run(cli *CLI) error { continue } - // Format the output to include diffs - formattedContent := formatter.Format(result.Content) - - if formattedContent != "" { + if len(result.Suggestions) > 0 { fmt.Printf("
\n") fmt.Printf( - "šŸ“ Review for %s\n\n", file, + "šŸ“ Review for %s (%d issues)\n\n", file, len(result.Suggestions), ) - fmt.Println(formattedContent) + for _, suggestion := range result.Suggestions { + formattedBody := formatter.Format(suggestion.Body) + fmt.Printf("### %s\n%s\n\n", suggestion.Title, formattedBody) + } fmt.Printf("\n
\n") } diff --git a/internal/agents/reviewer.go b/internal/agents/reviewer.go index 1b156f4..1c57c87 100644 --- a/internal/agents/reviewer.go +++ b/internal/agents/reviewer.go @@ -2,9 +2,11 @@ package agents import ( "context" + "encoding/json" "fmt" "net/http" "os" + "strings" "github.com/j0lvera/miso/internal/config" "github.com/j0lvera/miso/internal/git" @@ -33,10 +35,17 @@ func (t *headerTransport) RoundTrip(req *http.Request) (*http.Response, error) { return t.base.RoundTrip(req2) } +// Suggestion represents a single review comment from the LLM. +type Suggestion struct { + ID string `json:"id"` + Title string `json:"title"` + Body string `json:"body"` +} + // ReviewResult holds the review content and token usage information from an LLM call. // Provides details about the review content and associated costs. type ReviewResult struct { - Content string + Suggestions []Suggestion TokensUsed int InputTokens int OutputTokens int @@ -143,9 +152,22 @@ func (cr *CodeReviewer) callLLM(prompt string) (*ReviewResult, error) { content = resp.Choices[0].Content } + // Clean up potential markdown code blocks around the JSON + content = strings.TrimSpace(content) + if strings.HasPrefix(content, "```json") { + content = strings.TrimPrefix(content, "```json") + content = strings.TrimSuffix(content, "```") + content = strings.TrimSpace(content) + } + + var suggestions []Suggestion + if err := json.Unmarshal([]byte(content), &suggestions); err != nil { + return nil, fmt.Errorf("failed to parse LLM JSON response: %w\nRaw response:\n%s", err, content) + } + // Create result with content result := &ReviewResult{ - Content: content, + Suggestions: suggestions, } // Check if usage information is available in the response diff --git a/internal/prompts/code-review.go b/internal/prompts/code-review.go index 3feca0a..9214771 100644 --- a/internal/prompts/code-review.go +++ b/internal/prompts/code-review.go @@ -39,7 +39,7 @@ func CodeReview(cfg *config.Config, code string, filename string) ( } template := prompts.NewPromptTemplate( - `You are an expert code reviewer. Perform a two-pass review on the provided code. Do not add any introductory text, just the review. + `You are an expert code reviewer. Perform a two-pass review on the provided code. **FIRST PASS - General Code Health** Identify general issues based on the following criteria: @@ -51,22 +51,24 @@ Identify general issues based on the following criteria: **SECOND PASS - Architecture Compliance** Review the code against the provided Architecture Guides. If no guides are provided, skip this pass. -**Report Format:** -# šŸ² miso Code review - -## First Pass: General Issues -[šŸ”“ Critical | šŸŸ” Warning | šŸ’” Suggestion] - -## Second Pass: Architecture Violations -[āŒ Violation | āš ļø Deviation] - -For each issue provide: -- What's wrong and severity -- Why it matters -- How to fix. Use this specific format for the fix, with the original code and your suggested change: +**Output Format:** +Return your review as a JSON array of suggestion objects. Each object must have the following fields: +- "id": A unique identifier for the suggestion (e.g., "miso-1A", "miso-1B"). +- "title": A concise, one-line summary of the issue, including a severity emoji (e.g., "šŸ”“ Critical", "šŸŸ” Warning", "šŸ’” Suggestion", "āŒ Violation", "āš ļø Deviation"). +- "body": A detailed explanation of the issue in markdown format. The body must explain what's wrong, why it matters, and how to fix it. For code fixes, use this specific format: `+"```original\n"+`[the exact code to be replaced]`+"\n```\n"+"```suggestion\n"+`[the new code]`+"\n```"+` -Keep it concise - actionable issues only. +**Example JSON Output:** +[ + { + "id": "miso-1A", + "title": "šŸ”“ Critical: Lack of Error Handling", + "body": "The function `+"`doSomething`"+` can return an error that is not being checked. This could lead to unexpected behavior.\n\n`+"```original\n"+`result := doSomething()`+"\n```\n"+"```suggestion\n"+`result, err := doSomething()\nif err != nil {\n return err\n}`+"\n```"+`" + } +] + +If you find no issues, return an empty JSON array: []. +Do not add any introductory text or markdown formatting around the JSON array. Code to review: ''' diff --git a/internal/prompts/diff-review.go b/internal/prompts/diff-review.go index 6919f0a..55faf97 100644 --- a/internal/prompts/diff-review.go +++ b/internal/prompts/diff-review.go @@ -77,29 +77,30 @@ func DiffReview( - Check for proper error handling in new code - Verify imports and dependencies are appropriate +**Output Format:** +Return your review as a JSON array of suggestion objects. Each object must have the following fields: +- "id": A unique identifier for the suggestion (e.g., "miso-1A", "miso-1B"). +- "title": A concise, one-line summary of the issue, including a severity emoji (e.g., "šŸ”“ Breaking", "šŸŸ” Risky", "šŸŸ¢ Safe", "šŸ”“ Critical", "šŸŸ” Warning", "šŸ’” Suggestion", "āŒ Inconsistent", "āš ļø Minor Issue"). +- "body": A detailed explanation of the issue in markdown format. The body must explain what's wrong, why it matters, and how to fix it. For code fixes, use this specific format: +`+"```original\n"+`[the exact code to be replaced]`+"\n```\n"+"```suggestion\n"+`[the new code]`+"\n```"+` + +**Example JSON Output:** +[ + { + "id": "miso-1A", + "title": "šŸ”“ Breaking: Function signature changed", + "body": "The signature of `+"`calculateTotal`"+` was changed, which will break existing callers.\n\n`+"```original\n"+`-func calculateTotal(price int, quantity int)`+"\n```\n"+"```suggestion\n"+`+func calculateTotal(price float64, quantity int)`+"\n```"+`" + } +] + +If you find no issues, return an empty JSON array: []. +Do not add any introductory text or markdown formatting around the JSON array. + **DIFF TO REVIEW:** {{.changes_summary}} {{.formatted_diff}} -**REPORT FORMAT:** -## Change Impact Analysis -[šŸ”“ Breaking | šŸŸ” Risky | šŸŸ¢ Safe] - -## Code Quality Issues -[šŸ”“ Critical | šŸŸ” Warning | šŸ’” Suggestion] - -## Consistency & Patterns -[āŒ Inconsistent | āš ļø Minor Issue | āœ… Good] - -For each issue provide: -- Specific line numbers from the diff -- What's wrong and severity level -- Why it matters for this change -- How to fix (with code examples) - -Focus on actionable feedback for the specific changes shown. - File: {{.filename}}{{.guide}}`, []string{"changes_summary", "formatted_diff", "filename", "guide"}, ) From 0dec6abb7a83e5605e01509218a78a300be8c8a1 Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Sat, 26 Jul 2025 19:02:22 -0500 Subject: [PATCH 02/20] fix: escape newlines in prompt examples and format review output as markdown --- cmd/main/main.go | 51 ++++++++++++++++++--------------- internal/prompts/code-review.go | 2 +- internal/prompts/diff-review.go | 2 +- 3 files changed, 30 insertions(+), 25 deletions(-) diff --git a/cmd/main/main.go b/cmd/main/main.go index f57e1c4..1eea504 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -154,6 +154,24 @@ func (tp *TestPatternCmd) Run(cli *CLI) error { return nil } +func formatSuggestionsToMarkdown(suggestions []agents.Suggestion, filename string) string { + if len(suggestions) == 0 { + return "āœ… No issues found." + } + + var builder strings.Builder + builder.WriteString(fmt.Sprintf("# šŸ² miso Code review for %s\n\n", filename)) + + formatter := diff.NewFormatter() + for _, suggestion := range suggestions { + // Format the body to render diffs correctly + formattedBody := formatter.Format(suggestion.Body) + builder.WriteString(fmt.Sprintf("## %s\n%s\n\n", suggestion.Title, formattedBody)) + } + + return builder.String() +} + func (r *ReviewCmd) Run(cli *CLI) error { // Load configuration cfg, err := loadConfig(cli.Config, r.Verbose) @@ -218,32 +236,19 @@ func (r *ReviewCmd) Run(cli *CLI) error { return fmt.Errorf("review failed: %w", err) } - if len(result.Suggestions) == 0 { - fmt.Println("āœ… No issues found.") - return nil - } - - fmt.Printf("# šŸ² miso Code review for %s\n\n", filename) - - formatter := diff.NewFormatter() - for _, suggestion := range result.Suggestions { - // Format the body to render diffs correctly - formattedBody := formatter.Format(suggestion.Body) - fullSuggestion := fmt.Sprintf("## %s\n%s", suggestion.Title, formattedBody) + markdownReport := formatSuggestionsToMarkdown(result.Suggestions, filename) - // Apply glamour rendering if requested - if r.OutputStyle == "rich" { - rendered, err := renderRichOutput(fullSuggestion) - if err != nil { - log.Printf("Failed to initialize rich renderer: %v", err) - fmt.Println(fullSuggestion) // Fallback to plain - } else { - fmt.Print(rendered) - } + // Apply glamour rendering if requested + if r.OutputStyle == "rich" && len(result.Suggestions) > 0 { + rendered, err := renderRichOutput(markdownReport) + if err != nil { + log.Printf("Failed to initialize rich renderer: %v", err) + fmt.Println(markdownReport) // Fallback to plain } else { - fmt.Println(fullSuggestion) + fmt.Print(rendered) } - fmt.Println() // Add a newline for separation + } else { + fmt.Println(markdownReport) } // Display token usage if available diff --git a/internal/prompts/code-review.go b/internal/prompts/code-review.go index 9214771..1777dc6 100644 --- a/internal/prompts/code-review.go +++ b/internal/prompts/code-review.go @@ -63,7 +63,7 @@ Return your review as a JSON array of suggestion objects. Each object must have { "id": "miso-1A", "title": "šŸ”“ Critical: Lack of Error Handling", - "body": "The function `+"`doSomething`"+` can return an error that is not being checked. This could lead to unexpected behavior.\n\n`+"```original\n"+`result := doSomething()`+"\n```\n"+"```suggestion\n"+`result, err := doSomething()\nif err != nil {\n return err\n}`+"\n```"+`" + "body": "The function `+"`doSomething`"+` can return an error that is not being checked. This could lead to unexpected behavior.\\n\\n`+"```original\\n"+`result := doSomething()`+"\\n```\\n"+"```suggestion\\n"+`result, err := doSomething()\\nif err != nil {\\n return err\\n}`+"\\n```"+`" } ] diff --git a/internal/prompts/diff-review.go b/internal/prompts/diff-review.go index 55faf97..fb5c084 100644 --- a/internal/prompts/diff-review.go +++ b/internal/prompts/diff-review.go @@ -89,7 +89,7 @@ Return your review as a JSON array of suggestion objects. Each object must have { "id": "miso-1A", "title": "šŸ”“ Breaking: Function signature changed", - "body": "The signature of `+"`calculateTotal`"+` was changed, which will break existing callers.\n\n`+"```original\n"+`-func calculateTotal(price int, quantity int)`+"\n```\n"+"```suggestion\n"+`+func calculateTotal(price float64, quantity int)`+"\n```"+`" + "body": "The signature of `+"`calculateTotal`"+` was changed, which will break existing callers.\\n\\n`+"```original\\n"+`-func calculateTotal(price int, quantity int)`+"\\n```\\n"+"```suggestion\\n"+`+func calculateTotal(price float64, quantity int)`+"\\n```"+`" } ] From d940e9eb2a88e96a71c98ddf25d187be67e79b87 Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Sat, 26 Jul 2025 19:04:49 -0500 Subject: [PATCH 03/20] fix: escape newlines in json body field in prompts --- internal/prompts/code-review.go | 1 + internal/prompts/diff-review.go | 1 + 2 files changed, 2 insertions(+) diff --git a/internal/prompts/code-review.go b/internal/prompts/code-review.go index 1777dc6..ec447c2 100644 --- a/internal/prompts/code-review.go +++ b/internal/prompts/code-review.go @@ -57,6 +57,7 @@ Return your review as a JSON array of suggestion objects. Each object must have - "title": A concise, one-line summary of the issue, including a severity emoji (e.g., "šŸ”“ Critical", "šŸŸ” Warning", "šŸ’” Suggestion", "āŒ Violation", "āš ļø Deviation"). - "body": A detailed explanation of the issue in markdown format. The body must explain what's wrong, why it matters, and how to fix it. For code fixes, use this specific format: `+"```original\n"+`[the exact code to be replaced]`+"\n```\n"+"```suggestion\n"+`[the new code]`+"\n```"+` +The \"body\" field must be a valid JSON string, meaning all newlines inside the explanation must be escaped as \\n. **Example JSON Output:** [ diff --git a/internal/prompts/diff-review.go b/internal/prompts/diff-review.go index fb5c084..aa7137b 100644 --- a/internal/prompts/diff-review.go +++ b/internal/prompts/diff-review.go @@ -83,6 +83,7 @@ Return your review as a JSON array of suggestion objects. Each object must have - "title": A concise, one-line summary of the issue, including a severity emoji (e.g., "šŸ”“ Breaking", "šŸŸ” Risky", "šŸŸ¢ Safe", "šŸ”“ Critical", "šŸŸ” Warning", "šŸ’” Suggestion", "āŒ Inconsistent", "āš ļø Minor Issue"). - "body": A detailed explanation of the issue in markdown format. The body must explain what's wrong, why it matters, and how to fix it. For code fixes, use this specific format: `+"```original\n"+`[the exact code to be replaced]`+"\n```\n"+"```suggestion\n"+`[the new code]`+"\n```"+` +The \"body\" field must be a valid JSON string, meaning all newlines inside the explanation must be escaped as \\n. **Example JSON Output:** [ From 9d22147bd338e6ee9e68c8e935fd57b5f30d38b6 Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Sat, 26 Jul 2025 19:07:39 -0500 Subject: [PATCH 04/20] fix: unescape newlines in suggestion body before rendering --- cmd/main/main.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/cmd/main/main.go b/cmd/main/main.go index 1eea504..4d38d20 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -164,8 +164,10 @@ func formatSuggestionsToMarkdown(suggestions []agents.Suggestion, filename strin formatter := diff.NewFormatter() for _, suggestion := range suggestions { + // Unescape newlines from the JSON string body + unescapedBody := strings.ReplaceAll(suggestion.Body, "\\n", "\n") // Format the body to render diffs correctly - formattedBody := formatter.Format(suggestion.Body) + formattedBody := formatter.Format(unescapedBody) builder.WriteString(fmt.Sprintf("## %s\n%s\n\n", suggestion.Title, formattedBody)) } @@ -459,7 +461,9 @@ func (gr *GitHubReviewPRCmd) Run(cli *CLI) error { ), ) for _, suggestion := range result.Suggestions { - formattedBody := formatter.Format(suggestion.Body) + // Unescape newlines from the JSON string body + unescapedBody := strings.ReplaceAll(suggestion.Body, "\\n", "\n") + formattedBody := formatter.Format(unescapedBody) reviewOutput.WriteString(fmt.Sprintf("### %s\n%s\n\n", suggestion.Title, formattedBody)) } reviewOutput.WriteString("\n") @@ -618,7 +622,9 @@ func (d *DiffCmd) Run(cli *CLI) error { "šŸ“ Review for %s (%d issues)\n\n", file, len(result.Suggestions), ) for _, suggestion := range result.Suggestions { - formattedBody := formatter.Format(suggestion.Body) + // Unescape newlines from the JSON string body + unescapedBody := strings.ReplaceAll(suggestion.Body, "\\n", "\n") + formattedBody := formatter.Format(unescapedBody) fmt.Printf("### %s\n%s\n\n", suggestion.Title, formattedBody) } fmt.Printf("\n\n") From f501c28d3daf84caec90105339872423e8c772a3 Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Sat, 26 Jul 2025 19:10:13 -0500 Subject: [PATCH 05/20] feat: improve llm json parsing by separating code from body --- cmd/main/main.go | 31 ++++++++++++++++++++++--------- internal/agents/reviewer.go | 8 +++++--- internal/prompts/code-review.go | 12 ++++++++---- internal/prompts/diff-review.go | 12 ++++++++---- 4 files changed, 43 insertions(+), 20 deletions(-) diff --git a/cmd/main/main.go b/cmd/main/main.go index 4d38d20..0ab438c 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -154,6 +154,22 @@ func (tp *TestPatternCmd) Run(cli *CLI) error { return nil } +func buildSuggestionBody(suggestion agents.Suggestion) string { + var bodyBuilder strings.Builder + bodyBuilder.WriteString(strings.ReplaceAll(suggestion.Body, "\\n", "\n")) + + if suggestion.Original != "" || suggestion.Suggestion != "" { + bodyBuilder.WriteString("\n\n") + bodyBuilder.WriteString("```original\n") + bodyBuilder.WriteString(strings.ReplaceAll(suggestion.Original, "\\n", "\n")) + bodyBuilder.WriteString("\n```\n") + bodyBuilder.WriteString("```suggestion\n") + bodyBuilder.WriteString(strings.ReplaceAll(suggestion.Suggestion, "\\n", "\n")) + bodyBuilder.WriteString("\n```") + } + return bodyBuilder.String() +} + func formatSuggestionsToMarkdown(suggestions []agents.Suggestion, filename string) string { if len(suggestions) == 0 { return "āœ… No issues found." @@ -164,10 +180,9 @@ func formatSuggestionsToMarkdown(suggestions []agents.Suggestion, filename strin formatter := diff.NewFormatter() for _, suggestion := range suggestions { - // Unescape newlines from the JSON string body - unescapedBody := strings.ReplaceAll(suggestion.Body, "\\n", "\n") + fullBody := buildSuggestionBody(suggestion) // Format the body to render diffs correctly - formattedBody := formatter.Format(unescapedBody) + formattedBody := formatter.Format(fullBody) builder.WriteString(fmt.Sprintf("## %s\n%s\n\n", suggestion.Title, formattedBody)) } @@ -461,9 +476,8 @@ func (gr *GitHubReviewPRCmd) Run(cli *CLI) error { ), ) for _, suggestion := range result.Suggestions { - // Unescape newlines from the JSON string body - unescapedBody := strings.ReplaceAll(suggestion.Body, "\\n", "\n") - formattedBody := formatter.Format(unescapedBody) + fullBody := buildSuggestionBody(suggestion) + formattedBody := formatter.Format(fullBody) reviewOutput.WriteString(fmt.Sprintf("### %s\n%s\n\n", suggestion.Title, formattedBody)) } reviewOutput.WriteString("\n") @@ -622,9 +636,8 @@ func (d *DiffCmd) Run(cli *CLI) error { "šŸ“ Review for %s (%d issues)\n\n", file, len(result.Suggestions), ) for _, suggestion := range result.Suggestions { - // Unescape newlines from the JSON string body - unescapedBody := strings.ReplaceAll(suggestion.Body, "\\n", "\n") - formattedBody := formatter.Format(unescapedBody) + fullBody := buildSuggestionBody(suggestion) + formattedBody := formatter.Format(fullBody) fmt.Printf("### %s\n%s\n\n", suggestion.Title, formattedBody) } fmt.Printf("\n\n") diff --git a/internal/agents/reviewer.go b/internal/agents/reviewer.go index 1c57c87..4eaa454 100644 --- a/internal/agents/reviewer.go +++ b/internal/agents/reviewer.go @@ -37,9 +37,11 @@ func (t *headerTransport) RoundTrip(req *http.Request) (*http.Response, error) { // Suggestion represents a single review comment from the LLM. type Suggestion struct { - ID string `json:"id"` - Title string `json:"title"` - Body string `json:"body"` + ID string `json:"id"` + Title string `json:"title"` + Body string `json:"body"` + Original string `json:"original,omitempty"` + Suggestion string `json:"suggestion,omitempty"` } // ReviewResult holds the review content and token usage information from an LLM call. diff --git a/internal/prompts/code-review.go b/internal/prompts/code-review.go index ec447c2..d75447a 100644 --- a/internal/prompts/code-review.go +++ b/internal/prompts/code-review.go @@ -55,16 +55,20 @@ Review the code against the provided Architecture Guides. If no guides are provi Return your review as a JSON array of suggestion objects. Each object must have the following fields: - "id": A unique identifier for the suggestion (e.g., "miso-1A", "miso-1B"). - "title": A concise, one-line summary of the issue, including a severity emoji (e.g., "šŸ”“ Critical", "šŸŸ” Warning", "šŸ’” Suggestion", "āŒ Violation", "āš ļø Deviation"). -- "body": A detailed explanation of the issue in markdown format. The body must explain what's wrong, why it matters, and how to fix it. For code fixes, use this specific format: -`+"```original\n"+`[the exact code to be replaced]`+"\n```\n"+"```suggestion\n"+`[the new code]`+"\n```"+` -The \"body\" field must be a valid JSON string, meaning all newlines inside the explanation must be escaped as \\n. +- "body": A detailed explanation of the issue in markdown format. This should explain what's wrong and why it matters. +- "original": (Optional) The exact code to be replaced. +- "suggestion": (Optional) The new code. + +The "body", "original", and "suggestion" fields must be valid JSON strings, meaning all newlines inside them must be escaped as \\n. **Example JSON Output:** [ { "id": "miso-1A", "title": "šŸ”“ Critical: Lack of Error Handling", - "body": "The function `+"`doSomething`"+` can return an error that is not being checked. This could lead to unexpected behavior.\\n\\n`+"```original\\n"+`result := doSomething()`+"\\n```\\n"+"```suggestion\\n"+`result, err := doSomething()\\nif err != nil {\\n return err\\n}`+"\\n```"+`" + "body": "The function `+"`doSomething`"+` can return an error that is not being checked. This could lead to unexpected behavior.", + "original": "result := doSomething()", + "suggestion": "result, err := doSomething()\\nif err != nil {\\n return err\\n}" } ] diff --git a/internal/prompts/diff-review.go b/internal/prompts/diff-review.go index aa7137b..a773bcd 100644 --- a/internal/prompts/diff-review.go +++ b/internal/prompts/diff-review.go @@ -81,16 +81,20 @@ func DiffReview( Return your review as a JSON array of suggestion objects. Each object must have the following fields: - "id": A unique identifier for the suggestion (e.g., "miso-1A", "miso-1B"). - "title": A concise, one-line summary of the issue, including a severity emoji (e.g., "šŸ”“ Breaking", "šŸŸ” Risky", "šŸŸ¢ Safe", "šŸ”“ Critical", "šŸŸ” Warning", "šŸ’” Suggestion", "āŒ Inconsistent", "āš ļø Minor Issue"). -- "body": A detailed explanation of the issue in markdown format. The body must explain what's wrong, why it matters, and how to fix it. For code fixes, use this specific format: -`+"```original\n"+`[the exact code to be replaced]`+"\n```\n"+"```suggestion\n"+`[the new code]`+"\n```"+` -The \"body\" field must be a valid JSON string, meaning all newlines inside the explanation must be escaped as \\n. +- "body": A detailed explanation of the issue in markdown format. This should explain what's wrong and why it matters. +- "original": (Optional) The exact code to be replaced. +- "suggestion": (Optional) The new code. + +The "body", "original", and "suggestion" fields must be valid JSON strings, meaning all newlines inside them must be escaped as \\n. **Example JSON Output:** [ { "id": "miso-1A", "title": "šŸ”“ Breaking: Function signature changed", - "body": "The signature of `+"`calculateTotal`"+` was changed, which will break existing callers.\\n\\n`+"```original\\n"+`-func calculateTotal(price int, quantity int)`+"\\n```\\n"+"```suggestion\\n"+`+func calculateTotal(price float64, quantity int)`+"\\n```"+`" + "body": "The signature of `+"`calculateTotal`"+` was changed, which will break existing callers.", + "original": "-func calculateTotal(price int, quantity int)", + "suggestion": "+func calculateTotal(price float64, quantity int)" } ] From 9769515f4b949766ff9927a7a9bb0162b0b32da9 Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Sat, 26 Jul 2025 19:13:36 -0500 Subject: [PATCH 06/20] feat: update prompts to sort suggestions and focus on improvements --- internal/prompts/code-review.go | 6 +++++- internal/prompts/diff-review.go | 8 ++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/internal/prompts/code-review.go b/internal/prompts/code-review.go index d75447a..c7efcac 100644 --- a/internal/prompts/code-review.go +++ b/internal/prompts/code-review.go @@ -52,7 +52,11 @@ Identify general issues based on the following criteria: Review the code against the provided Architecture Guides. If no guides are provided, skip this pass. **Output Format:** -Return your review as a JSON array of suggestion objects. Each object must have the following fields: +Return your review as a JSON array of suggestion objects. +- Provide only actionable suggestions for improvement. Do not comment on code that is already good. +- Sort the suggestions in the final JSON array from most critical to least critical. + +Each object must have the following fields: - "id": A unique identifier for the suggestion (e.g., "miso-1A", "miso-1B"). - "title": A concise, one-line summary of the issue, including a severity emoji (e.g., "šŸ”“ Critical", "šŸŸ” Warning", "šŸ’” Suggestion", "āŒ Violation", "āš ļø Deviation"). - "body": A detailed explanation of the issue in markdown format. This should explain what's wrong and why it matters. diff --git a/internal/prompts/diff-review.go b/internal/prompts/diff-review.go index a773bcd..67aa6ff 100644 --- a/internal/prompts/diff-review.go +++ b/internal/prompts/diff-review.go @@ -78,9 +78,13 @@ func DiffReview( - Verify imports and dependencies are appropriate **Output Format:** -Return your review as a JSON array of suggestion objects. Each object must have the following fields: +Return your review as a JSON array of suggestion objects. +- Provide only actionable suggestions for improvement. Do not comment on code that is already good. +- Sort the suggestions in the final JSON array from most critical to least critical. + +Each object must have the following fields: - "id": A unique identifier for the suggestion (e.g., "miso-1A", "miso-1B"). -- "title": A concise, one-line summary of the issue, including a severity emoji (e.g., "šŸ”“ Breaking", "šŸŸ” Risky", "šŸŸ¢ Safe", "šŸ”“ Critical", "šŸŸ” Warning", "šŸ’” Suggestion", "āŒ Inconsistent", "āš ļø Minor Issue"). +- "title": A concise, one-line summary of the issue, including a severity emoji (e.g., "šŸ”“ Breaking", "šŸŸ” Risky", "šŸ”“ Critical", "šŸŸ” Warning", "šŸ’” Suggestion", "āŒ Inconsistent", "āš ļø Minor Issue"). - "body": A detailed explanation of the issue in markdown format. This should explain what's wrong and why it matters. - "original": (Optional) The exact code to be replaced. - "suggestion": (Optional) The new code. From 43faefaf49a88c58e4dafeed2576bf533088655c Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Sat, 26 Jul 2025 19:20:56 -0500 Subject: [PATCH 07/20] feat: add --one flag to review command to show single suggestion --- cmd/main/main.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cmd/main/main.go b/cmd/main/main.go index 0ab438c..28d0e7c 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -46,6 +46,7 @@ type ReviewCmd struct { Message string `short:"m" help:"Message to display while processing" default:"Thinking..."` DryRun bool `short:"d" help:"Show what would be reviewed without calling LLM"` OutputStyle string `short:"s" name:"output-style" help:"Output style: plain (default) or rich (formatted with colors and markdown)" enum:"plain,rich" default:"plain"` + One bool `short:"1" name:"one" help:"Show only the first suggestion."` } type VersionCmd struct{} @@ -253,6 +254,10 @@ func (r *ReviewCmd) Run(cli *CLI) error { return fmt.Errorf("review failed: %w", err) } + if r.One && len(result.Suggestions) > 0 { + result.Suggestions = result.Suggestions[:1] + } + markdownReport := formatSuggestionsToMarkdown(result.Suggestions, filename) // Apply glamour rendering if requested From a88f3ce32c03ec265e7931f2eea408612e3ea587 Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Sat, 26 Jul 2025 19:42:02 -0500 Subject: [PATCH 08/20] fix: improve llm json parsing robustness --- internal/agents/reviewer.go | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/internal/agents/reviewer.go b/internal/agents/reviewer.go index 4eaa454..a9de702 100644 --- a/internal/agents/reviewer.go +++ b/internal/agents/reviewer.go @@ -154,16 +154,22 @@ func (cr *CodeReviewer) callLLM(prompt string) (*ReviewResult, error) { content = resp.Choices[0].Content } - // Clean up potential markdown code blocks around the JSON - content = strings.TrimSpace(content) - if strings.HasPrefix(content, "```json") { - content = strings.TrimPrefix(content, "```json") - content = strings.TrimSuffix(content, "```") - content = strings.TrimSpace(content) + // Find the start of the JSON array to strip any leading text. + startIndex := strings.Index(content, "[") + if startIndex == -1 { + return nil, fmt.Errorf("failed to find start of JSON array in LLM response\nRaw response:\n%s", content) } + // Find the end of the JSON array + endIndex := strings.LastIndex(content, "]") + if endIndex == -1 { + return nil, fmt.Errorf("failed to find end of JSON array in LLM response\nRaw response:\n%s", content) + } + + jsonStr := content[startIndex : endIndex+1] + var suggestions []Suggestion - if err := json.Unmarshal([]byte(content), &suggestions); err != nil { + if err := json.Unmarshal([]byte(jsonStr), &suggestions); err != nil { return nil, fmt.Errorf("failed to parse LLM JSON response: %w\nRaw response:\n%s", err, content) } From 26f185896d9044d25f534cf8218400be2d849863 Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Sat, 26 Jul 2025 20:14:22 -0500 Subject: [PATCH 09/20] fix: update reviewer tests and prompt expectations --- internal/agents/reviewer_test.go | 22 +++++++++++++++------- internal/prompts/diff-review_test.go | 6 +++--- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/internal/agents/reviewer_test.go b/internal/agents/reviewer_test.go index b74e4b7..27fb816 100644 --- a/internal/agents/reviewer_test.go +++ b/internal/agents/reviewer_test.go @@ -101,8 +101,8 @@ func main() { if result == nil { t.Error("Expected non-nil result") } - if result.Content == "" { - t.Error("Expected non-empty content") + if len(result.Suggestions) == 0 { + t.Error("Expected non-empty suggestions") } } }, @@ -186,8 +186,8 @@ func TestCodeReviewer_ReviewDiff(t *testing.T) { if result == nil { t.Error("Expected non-nil result") } - if result.Content == "" { - t.Error("Expected non-empty content") + if len(result.Suggestions) == 0 { + t.Error("Expected non-empty suggestions") } } }, @@ -246,16 +246,24 @@ func TestCodeReviewer_callLLM(t *testing.T) { // Mock tests for unit testing without API calls func TestReviewResult_Structure(t *testing.T) { result := &ReviewResult{ - Content: "Test review content", + Suggestions: []Suggestion{ + { + Title: "Test review content", + }, + }, TokensUsed: 100, InputTokens: 60, OutputTokens: 40, Cost: 0.001, } - if result.Content != "Test review content" { + if len(result.Suggestions) != 1 { + t.Fatalf("Expected 1 suggestion, got %d", len(result.Suggestions)) + } + + if result.Suggestions[0].Title != "Test review content" { t.Errorf( - "Expected content 'Test review content', got %s", result.Content, + "Expected content 'Test review content', got %s", result.Suggestions[0].Title, ) } diff --git a/internal/prompts/diff-review_test.go b/internal/prompts/diff-review_test.go index 7ed91d8..3ed4bba 100644 --- a/internal/prompts/diff-review_test.go +++ b/internal/prompts/diff-review_test.go @@ -55,9 +55,9 @@ func TestDiffReview(t *testing.T) { "File: test.go", "@@ -1,2 +1,3 @@", "+import \"fmt\"", - "Change Impact Analysis", - "Code Quality Issues", - "Consistency & Patterns", + "**CHANGE ANALYSIS FOCUS:**", + "**REVIEW GUIDELINES:**", + "**Output Format:**", }, }, { From 47134676d6bc8ee3603bf30783bdaaaafb4d9efa Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Sat, 26 Jul 2025 21:34:41 -0500 Subject: [PATCH 10/20] feat: add --one option to diff command --- cmd/main/main.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cmd/main/main.go b/cmd/main/main.go index 28d0e7c..b22472d 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -298,6 +298,7 @@ type DiffCmd struct { Verbose bool `short:"v" help:"Enable verbose output"` Message string `short:"m" help:"Message to display while processing" default:"Analyzing changes..."` DryRun bool `short:"d" help:"Show what would be reviewed without calling LLM"` + One bool `short:"1" name:"one" help:"Show only the first suggestion per file."` } type ValidateConfigCmd struct { @@ -473,6 +474,10 @@ func (gr *GitHubReviewPRCmd) Run(cli *CLI) error { continue } + if d.One && len(result.Suggestions) > 0 { + result.Suggestions = result.Suggestions[:1] + } + if len(result.Suggestions) > 0 { reviewOutput.WriteString(fmt.Sprintf("
\n")) reviewOutput.WriteString( From 7ec5446b72efbe22ed8278d88321bced0aa4fefe Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Sat, 26 Jul 2025 21:37:39 -0500 Subject: [PATCH 11/20] fix: move --one option logic to diff command --- cmd/main/main.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/main/main.go b/cmd/main/main.go index b22472d..19040a4 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -474,10 +474,6 @@ func (gr *GitHubReviewPRCmd) Run(cli *CLI) error { continue } - if d.One && len(result.Suggestions) > 0 { - result.Suggestions = result.Suggestions[:1] - } - if len(result.Suggestions) > 0 { reviewOutput.WriteString(fmt.Sprintf("
\n")) reviewOutput.WriteString( @@ -640,6 +636,10 @@ func (d *DiffCmd) Run(cli *CLI) error { continue } + if d.One && len(result.Suggestions) > 0 { + result.Suggestions = result.Suggestions[:1] + } + if len(result.Suggestions) > 0 { fmt.Printf("
\n") fmt.Printf( From 75af579aaed2fce137f1138e4f8577ffa6648f40 Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Sat, 26 Jul 2025 21:45:19 -0500 Subject: [PATCH 12/20] feat: add rich output option to diff command --- cmd/main/main.go | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/cmd/main/main.go b/cmd/main/main.go index 19040a4..b4bc6cf 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -294,11 +294,12 @@ func (r *ReviewCmd) Run(cli *CLI) error { } type DiffCmd struct { - Range string `arg:"" optional:"" help:"Git range (e.g., main..HEAD, HEAD~1)" default:"HEAD~1"` - Verbose bool `short:"v" help:"Enable verbose output"` - Message string `short:"m" help:"Message to display while processing" default:"Analyzing changes..."` - DryRun bool `short:"d" help:"Show what would be reviewed without calling LLM"` - One bool `short:"1" name:"one" help:"Show only the first suggestion per file."` + Range string `arg:"" optional:"" help:"Git range (e.g., main..HEAD, HEAD~1)" default:"HEAD~1"` + Verbose bool `short:"v" help:"Enable verbose output"` + Message string `short:"m" help:"Message to display while processing" default:"Analyzing changes..."` + DryRun bool `short:"d" help:"Show what would be reviewed without calling LLM"` + One bool `short:"1" name:"one" help:"Show only the first suggestion per file."` + OutputStyle string `short:"s" name:"output-style" help:"Output style: plain (default) or rich (formatted with colors and markdown)" enum:"plain,rich" default:"plain"` } type ValidateConfigCmd struct { @@ -640,17 +641,19 @@ func (d *DiffCmd) Run(cli *CLI) error { result.Suggestions = result.Suggestions[:1] } - if len(result.Suggestions) > 0 { - fmt.Printf("
\n") - fmt.Printf( - "šŸ“ Review for %s (%d issues)\n\n", file, len(result.Suggestions), - ) - for _, suggestion := range result.Suggestions { - fullBody := buildSuggestionBody(suggestion) - formattedBody := formatter.Format(fullBody) - fmt.Printf("### %s\n%s\n\n", suggestion.Title, formattedBody) + markdownReport := formatSuggestionsToMarkdown(result.Suggestions, file) + + // Apply glamour rendering if requested + if d.OutputStyle == "rich" && len(result.Suggestions) > 0 { + rendered, err := renderRichOutput(markdownReport) + if err != nil { + log.Printf("Failed to initialize rich renderer: %v", err) + fmt.Println(markdownReport) // Fallback to plain + } else { + fmt.Print(rendered) } - fmt.Printf("\n
\n") + } else { + fmt.Println(markdownReport) } if result.TokensUsed > 0 { From 1e5eefd290529b98a30659b2dc34f65b20bcb35f Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Sat, 26 Jul 2025 21:48:17 -0500 Subject: [PATCH 13/20] fix: remove unused formatter variable in diff command --- cmd/main/main.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/cmd/main/main.go b/cmd/main/main.go index b4bc6cf..5445974 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -597,9 +597,6 @@ func (d *DiffCmd) Run(cli *CLI) error { return fmt.Errorf("failed to create reviewer: %w", err) } - // Initialize diff formatter - formatter := diff.NewFormatter() - // Review each changed file totalTokens := 0 for _, file := range reviewableFiles { From e5e5c4f309b6b9ac6107f147ad30541d382c1228 Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Sat, 26 Jul 2025 21:52:52 -0500 Subject: [PATCH 14/20] feat: change default diff range to main..head --- cmd/main/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/main/main.go b/cmd/main/main.go index 5445974..c69e83a 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -294,7 +294,7 @@ func (r *ReviewCmd) Run(cli *CLI) error { } type DiffCmd struct { - Range string `arg:"" optional:"" help:"Git range (e.g., main..HEAD, HEAD~1)" default:"HEAD~1"` + Range string `arg:"" optional:"" help:"Git range (e.g., main..HEAD, HEAD~1)" default:"main..HEAD"` Verbose bool `short:"v" help:"Enable verbose output"` Message string `short:"m" help:"Message to display while processing" default:"Analyzing changes..."` DryRun bool `short:"d" help:"Show what would be reviewed without calling LLM"` From d6a0c6d9ca0516ac3be6338025255b8b7cf78c51 Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Sat, 26 Jul 2025 21:56:00 -0500 Subject: [PATCH 15/20] fix: default diff range to main..head when not provided --- cmd/main/main.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/cmd/main/main.go b/cmd/main/main.go index c69e83a..6a4bb40 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -541,8 +541,14 @@ func (d *DiffCmd) Run(cli *CLI) error { return fmt.Errorf("failed to initialize git client: %w", err) } + // Manually apply default if no range is provided. + rangeStr := d.Range + if rangeStr == "" { + rangeStr = "main..HEAD" + } + // Parse git range - base, head := git.ParseGitRange(d.Range) + base, head := git.ParseGitRange(rangeStr) if d.Verbose { fmt.Printf("Reviewing changes between %s and %s\n", base, head) From 9193a22fdf4ba759c117e38961f45df5bdc64ac5 Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Sat, 26 Jul 2025 22:03:56 -0500 Subject: [PATCH 16/20] feat: allow diff command to accept an optional file path --- cmd/main/main.go | 65 +++++++++++++++++++++++++++++++++++++----------- 1 file changed, 51 insertions(+), 14 deletions(-) diff --git a/cmd/main/main.go b/cmd/main/main.go index 6a4bb40..517b011 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -294,12 +294,12 @@ func (r *ReviewCmd) Run(cli *CLI) error { } type DiffCmd struct { - Range string `arg:"" optional:"" help:"Git range (e.g., main..HEAD, HEAD~1)" default:"main..HEAD"` - Verbose bool `short:"v" help:"Enable verbose output"` - Message string `short:"m" help:"Message to display while processing" default:"Analyzing changes..."` - DryRun bool `short:"d" help:"Show what would be reviewed without calling LLM"` - One bool `short:"1" name:"one" help:"Show only the first suggestion per file."` - OutputStyle string `short:"s" name:"output-style" help:"Output style: plain (default) or rich (formatted with colors and markdown)" enum:"plain,rich" default:"plain"` + Args []string `arg:"" optional:"" name:"range-or-file" help:"A git range and/or a file path."` + Verbose bool `short:"v" help:"Enable verbose output"` + Message string `short:"m" help:"Message to display while processing" default:"Analyzing changes..."` + DryRun bool `short:"d" help:"Show what would be reviewed without calling LLM"` + One bool `short:"1" name:"one" help:"Show only the first suggestion per file."` + OutputStyle string `short:"s" name:"output-style" help:"Output style: plain (default) or rich (formatted with colors and markdown)" enum:"plain,rich" default:"plain"` } type ValidateConfigCmd struct { @@ -541,10 +541,25 @@ func (d *DiffCmd) Run(cli *CLI) error { return fmt.Errorf("failed to initialize git client: %w", err) } - // Manually apply default if no range is provided. - rangeStr := d.Range - if rangeStr == "" { + var rangeStr string + var targetFile string + + switch len(d.Args) { + case 0: rangeStr = "main..HEAD" + case 1: + // Could be a range or a file. + if _, err := os.Stat(d.Args[0]); err == nil { + targetFile = d.Args[0] + rangeStr = "main..HEAD" + } else { + rangeStr = d.Args[0] + } + case 2: + rangeStr = d.Args[0] + targetFile = d.Args[1] + default: + return fmt.Errorf("too many arguments for diff command, expected [range] [file]") } // Parse git range @@ -572,11 +587,33 @@ func (d *DiffCmd) Run(cli *CLI) error { // Filter files that should be reviewed res := resolver.NewResolver(cfg) var reviewableFiles []string - for _, file := range files { - if res.ShouldReview(file) { - reviewableFiles = append(reviewableFiles, file) - } else if d.Verbose { - fmt.Printf("Skipping %s (no matching patterns)\n", file) + + if targetFile != "" { + fileIsChanged := false + for _, f := range files { + if f == targetFile { + fileIsChanged = true + break + } + } + + if fileIsChanged { + if res.ShouldReview(targetFile) { + reviewableFiles = append(reviewableFiles, targetFile) + } else if d.Verbose { + fmt.Printf("Skipping %s (no matching patterns)\n", targetFile) + } + } else { + fmt.Printf("File '%s' was not changed in the specified range.\n", targetFile) + return nil + } + } else { + for _, file := range files { + if res.ShouldReview(file) { + reviewableFiles = append(reviewableFiles, file) + } else if d.Verbose { + fmt.Printf("Skipping %s (no matching patterns)\n", file) + } } } From c0cca7ae8c548c062fce82220ca52265a7e71c06 Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Sat, 26 Jul 2025 22:13:45 -0500 Subject: [PATCH 17/20] docs: clarify diff command argument order in help text --- cmd/main/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/main/main.go b/cmd/main/main.go index 517b011..3749caf 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -294,7 +294,7 @@ func (r *ReviewCmd) Run(cli *CLI) error { } type DiffCmd struct { - Args []string `arg:"" optional:"" name:"range-or-file" help:"A git range and/or a file path."` + Args []string `arg:"" optional:"" name:"range-or-file" help:"[range] [file]. Git range to review (defaults to 'main..HEAD'). Optionally, a file path to review within the range."` Verbose bool `short:"v" help:"Enable verbose output"` Message string `short:"m" help:"Message to display while processing" default:"Analyzing changes..."` DryRun bool `short:"d" help:"Show what would be reviewed without calling LLM"` From 8a3e2b6bc306ec4607095627cf7c7f286342f5aa Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Sat, 26 Jul 2025 22:20:35 -0500 Subject: [PATCH 18/20] refactor: use flags for range and file in diff command --- cmd/main/main.go | 25 ++++--------------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/cmd/main/main.go b/cmd/main/main.go index 3749caf..6793293 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -294,7 +294,8 @@ func (r *ReviewCmd) Run(cli *CLI) error { } type DiffCmd struct { - Args []string `arg:"" optional:"" name:"range-or-file" help:"[range] [file]. Git range to review (defaults to 'main..HEAD'). Optionally, a file path to review within the range."` + Range string `short:"r" help:"Git range to review." default:"main..HEAD"` + File string `short:"f" help:"A specific file path to review within the range." type:"existingfile"` Verbose bool `short:"v" help:"Enable verbose output"` Message string `short:"m" help:"Message to display while processing" default:"Analyzing changes..."` DryRun bool `short:"d" help:"Show what would be reviewed without calling LLM"` @@ -541,26 +542,8 @@ func (d *DiffCmd) Run(cli *CLI) error { return fmt.Errorf("failed to initialize git client: %w", err) } - var rangeStr string - var targetFile string - - switch len(d.Args) { - case 0: - rangeStr = "main..HEAD" - case 1: - // Could be a range or a file. - if _, err := os.Stat(d.Args[0]); err == nil { - targetFile = d.Args[0] - rangeStr = "main..HEAD" - } else { - rangeStr = d.Args[0] - } - case 2: - rangeStr = d.Args[0] - targetFile = d.Args[1] - default: - return fmt.Errorf("too many arguments for diff command, expected [range] [file]") - } + rangeStr := d.Range + targetFile := d.File // Parse git range base, head := git.ParseGitRange(rangeStr) From 824c212b3dc79f916c336700d16989e893ca20a7 Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Sat, 26 Jul 2025 22:27:29 -0500 Subject: [PATCH 19/20] fix: convert target file path to relative for accurate comparison --- cmd/main/main.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/cmd/main/main.go b/cmd/main/main.go index 6793293..3cb570c 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -572,19 +572,28 @@ func (d *DiffCmd) Run(cli *CLI) error { var reviewableFiles []string if targetFile != "" { + // Convert user-provided file path to a relative path for comparison with git output. + relTargetFile := targetFile + cwd, err := os.Getwd() + if err == nil { + if rel, err := filepath.Rel(cwd, targetFile); err == nil { + relTargetFile = rel + } + } + fileIsChanged := false for _, f := range files { - if f == targetFile { + if f == relTargetFile { fileIsChanged = true break } } if fileIsChanged { - if res.ShouldReview(targetFile) { - reviewableFiles = append(reviewableFiles, targetFile) + if res.ShouldReview(relTargetFile) { + reviewableFiles = append(reviewableFiles, relTargetFile) } else if d.Verbose { - fmt.Printf("Skipping %s (no matching patterns)\n", targetFile) + fmt.Printf("Skipping %s (no matching patterns)\n", relTargetFile) } } else { fmt.Printf("File '%s' was not changed in the specified range.\n", targetFile) From d8a53f14b0981a06cb3c12d73182b86bb4614cd9 Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Sat, 26 Jul 2025 22:32:15 -0500 Subject: [PATCH 20/20] docs: update changelog with diff command features and fixes --- CHANGELOG.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 741613e..ad14d51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- Add `--one` option to `diff` command to show only the first suggestion per file. +- Add `--output-style rich` option to `diff` command for formatted output. +- Add `--file` option to `diff` command to review a specific file within a range. + +### Changed +- Change default git range for `diff` command to `main..HEAD`. +- Refactor `diff` command to use `--range` and `--file` flags instead of positional arguments for clarity. + +### Fixed +- Fix `diff` command to correctly handle relative file paths. +- Update internal tests to align with current data structures and prompt formats. + ## [0.4.0] - 2025-07-21 ### Added