feat: add option to show one suggestion at the time

[juanbzz]

Includes an update on the prompts to receive the report as JSON from the LLM. This change makes it easier to grab the first suggestion and will help when we implement memory.

[github-actions]

🍲 miso Code review

📝 Review for cmd/main/main.go (4 issues)

🟡 Warning: Potential race condition in formatSuggestionsToMarkdown

The strings.Builder is being used in a function that could potentially be called concurrently. While this specific usage appears safe in the current context, it's good practice to document thread-safety assumptions or ensure proper synchronization.

+// Note: This function assumes single-threaded usage
 var builder strings.Builder

💡 Suggestion: Consider adding error return to buildSuggestionBody

The buildSuggestionBody function performs string operations that could potentially fail (though unlikely). Consider returning an error to maintain consistency with Go's error handling patterns and allow proper error propagation.

-func buildSuggestionBody(suggestion agents.Suggestion) string {
+func buildSuggestionBody(suggestion agents.Suggestion) (string, error) {

💡 Suggestion: Add context timeout constant

The 30-second timeout for the GitHub API call is hardcoded. Consider extracting this value to a named constant at package level for better maintainability and documentation.

-ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+const githubAPITimeout = 30 * time.Second
+
+// ...
+ctx, cancel := context.WithTimeout(context.Background(), githubAPITimeout)

⚠️ Minor Issue: Inconsistent error message formatting

Error messages should be consistent in their formatting. Some error messages use fmt.Errorf while others use direct string concatenation. Standardize on fmt.Errorf for consistency.

-return nil, fmt.Errorf("failed to find start of JSON array in LLM response
-Raw response:
-%s", content)
+return nil, fmt.Errorf("failed to find start of JSON array in LLM response: %s", content)

📝 Review for internal/agents/reviewer.go (3 issues)

🟡 Warning: Potential race condition in buildSuggestionBody

The buildSuggestionBody function uses strings.Builder but doesn't pre-allocate capacity. For large suggestions or high concurrency, this could lead to multiple memory allocations and potential race conditions.

Consider using builder.Grow() to pre-allocate capacity based on the expected size of the content.

 var bodyBuilder strings.Builder
+expectedSize := len(suggestion.Body) + len(suggestion.Original) + len(suggestion.Suggestion) + 100
+bodyBuilder.Grow(expectedSize)
 bodyBuilder.WriteString(strings.ReplaceAll(suggestion.Body, "
 ", "
 "))

💡 Suggestion: Add error handling for renderRichOutput

The code silently falls back to plain output when rich rendering fails. While this is a valid fallback strategy, it would be better to inform the user about the rendering failure.

Consider logging a warning message that's visible to the user when falling back to plain output.

-log.Printf("Failed to initialize rich renderer: %v", err)
-fmt.Println(markdownReport) // Fallback to plain
+fmt.Fprintf(os.Stderr, "Warning: Rich output rendering failed (%v), falling back to plain output
+", err)
+fmt.Println(markdownReport)

💡 Suggestion: Add constant for GitHub comment timeout

The 30-second timeout for GitHub comment posting is hardcoded. This value should be defined as a constant at package level for better maintainability and consistency.

-ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+const githubCommentTimeout = 30 * time.Second
+
+// Later in code:
+ctx, cancel := context.WithTimeout(context.Background(), githubCommentTimeout)

📝 Review for internal/agents/reviewer_test.go (3 issues)

🟡 Warning: Potential race condition in buildSuggestionBody

The strings.Builder is being used without proper initialization size hint. For large suggestions, this could lead to multiple memory allocations. Consider using builder.Grow() with an estimated size to improve performance.

-var bodyBuilder strings.Builder
+bodyBuilder := strings.Builder{}
+bodyBuilder.Grow(len(suggestion.Body) + len(suggestion.Original) + len(suggestion.Suggestion) + 100)

💡 Suggestion: Add error handling for filepath.Rel

The error from filepath.Rel() is silently ignored. While the code continues to work with the absolute path, it would be better to log the error for debugging purposes.

 if rel, err := filepath.Rel(cwd, targetFile); err == nil {
 				relTargetFile = rel
+			} else {
+				log.Printf("Warning: Could not get relative path: %v", err)
 			}

💡 Suggestion: Add context timeout constant

The 30-second timeout for GitHub API calls is hardcoded. Consider making this a configurable constant at the package level for better maintainability.

-ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+const githubAPITimeout = 30 * time.Second
+
+// Later in code:
+ctx, cancel := context.WithTimeout(context.Background(), githubAPITimeout)

📝 Review for internal/prompts/code-review.go (3 issues)

🟡 Warning: Inconsistent Error Handling

The error handling in buildSuggestionBody and formatSuggestionsToMarkdown doesn't follow the project's error wrapping guidelines. These functions could fail during string operations but don't return errors for proper handling upstream.

-func buildSuggestionBody(suggestion agents.Suggestion) string {
+func buildSuggestionBody(suggestion agents.Suggestion) (string, error) {
 	var bodyBuilder strings.Builder
-	bodyBuilder.WriteString(strings.ReplaceAll(suggestion.Body, "\
+	_, err := bodyBuilder.WriteString(strings.ReplaceAll(suggestion.Body, "\
 ", "
-"))
+"))
+	if err != nil {
+		return "", fmt.Errorf("failed to build suggestion body: %w", err)
+	}

💡 Suggestion: Extract Common Rendering Logic

The rendering logic for suggestions is duplicated between the DiffCmd and ReviewCmd. Consider extracting this into a shared function to maintain DRY principles and ensure consistent rendering behavior.

-// In DiffCmd
-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 {
+func renderOutput(content string, style string) error {
+	if style == "rich" {
+		rendered, err := renderRichOutput(content)
+		if err != nil {
+			return fmt.Errorf("failed to render rich output: %w", err)
+		}
 		fmt.Print(rendered)
+		return nil
 	}
-} else {
-	fmt.Println(markdownReport)
+	fmt.Println(content)
+	return nil
 }

💡 Suggestion: Add Constants for Output Styles

The output style values "plain" and "rich" are used as string literals in multiple places. Consider defining these as constants to prevent typos and make changes easier to maintain.

+const (
+	OutputStylePlain = "plain"
+	OutputStyleRich  = "rich"
+)
+
 OutputStyle string `short:"s" name:"output-style" help:"Output style: plain (default) or rich (formatted with colors and markdown)" enum:"plain,rich" default:"plain"`

📝 Review for internal/prompts/diff-review.go (3 issues)

🟡 Warning: Potential race condition in buildSuggestionBody

The buildSuggestionBody function uses strings.Builder but doesn't pre-allocate capacity. For large suggestions with multiple replacements, this could lead to multiple memory allocations and potential race conditions in concurrent usage.

Consider pre-allocating the builder capacity based on the expected string size.

-var bodyBuilder strings.Builder
+bodyBuilder := strings.Builder{}
+bodyBuilder.Grow(len(suggestion.Body) + len(suggestion.Original) + len(suggestion.Suggestion) + 100)
 bodyBuilder.WriteString(strings.ReplaceAll(suggestion.Body, "
 ", "
 "))

💡 Suggestion: Add error handling for WriteString operations

The WriteString operations in buildSuggestionBody and formatSuggestionsToMarkdown don't check for write errors. While strings.Builder write operations rarely fail, it's good practice to handle potential errors, especially in a code review tool.

-builder.WriteString(fmt.Sprintf("# 🍲 miso Code review for %s
+_, err := builder.WriteString(fmt.Sprintf("# 🍲 miso Code review for %s
 
-", filename))
+", filename))
+if err != nil {
+    return fmt.Sprintf("Error formatting review: %v", err)
+}

💡 Suggestion: Consider adding documentation for new public functions

The newly added public functions buildSuggestionBody and formatSuggestionsToMarkdown lack documentation comments. As per Go best practices, all exported functions should have proper documentation.

+// buildSuggestionBody formats a suggestion into a markdown-formatted string
+// including the original code and suggested changes if present.
 func buildSuggestionBody(suggestion agents.Suggestion) string {

📝 Review for internal/prompts/diff-review_test.go (3 issues)

🟡 Warning: Potential race condition in buildSuggestionBody

The buildSuggestionBody function uses strings.Builder but doesn't pre-allocate capacity. For concurrent usage, this could lead to unnecessary allocations and potential race conditions.

Consider using builder.Grow() to pre-allocate capacity based on the expected size of the content.

 var bodyBuilder strings.Builder
+expectedSize := len(suggestion.Body) + len(suggestion.Original) + len(suggestion.Suggestion) + 100
+bodyBuilder.Grow(expectedSize)
 bodyBuilder.WriteString(strings.ReplaceAll(suggestion.Body, "
 ", "
 "))

💡 Suggestion: Add error handling for renderRichOutput

The error from renderRichOutput is logged but execution continues with a fallback. Consider propagating the error up or adding metrics/monitoring for these rendering failures to track their frequency.

 rendered, err := renderRichOutput(markdownReport)
 if err != nil {
     log.Printf("Failed to initialize rich renderer: %v", err)
+    metrics.IncCounter("render_failures")
     fmt.Println(markdownReport) // Fallback to plain
 }

💡 Suggestion: Consider caching diff.NewFormatter()

The diff.NewFormatter() is created multiple times in loops. Consider creating it once and reusing the instance to improve performance, especially when processing multiple files.

+// Create formatter once at function start
 formatter := diff.NewFormatter()
-for _, suggestion := range result.Suggestions {
-    formattedBody := formatter.Format(fullBody)
+// ... rest of the function
+// Reuse formatter in loop