Skip to content

Commit ed50c90

Browse files
stn1slvstn1slv
committed
fix(provider): fix output parsing and separate stderr from stdout
- Extract parseOutputLines() helper into common.go, shared by both Anthropic and Gemini providers, replacing four copies of identical logic - Fix multi-digit numbered list stripping (e.g. "10. message" was broken) - Separate stderr from stdout in CLI exec; only stdout is parsed for messages, stderr is included in error output on failure - Add table-driven tests covering all parsing edge cases
1 parent 9112452 commit ed50c90

4 files changed

Lines changed: 204 additions & 243 deletions

File tree

‎internal/provider/anthropic.go‎

Lines changed: 28 additions & 122 deletions
Original file line numberDiff line numberDiff line change
@@ -54,79 +54,12 @@ func (a *AnthropicProvider) GenerateCommitMessages(ctx context.Context, diff str
5454
fullPrompt := fmt.Sprintf("%s\n\nUser request: %s\n\nIMPORTANT: Generate exactly %d commit messages, one per line. Do not include any other text, explanations, or formatting - just the commit messages.",
5555
systemMsg, userPrompt, a.numSuggestions)
5656

57-
// Execute claude CLI with haiku model
58-
// Using -p flag for print mode and --model for model selection
59-
// Pipe prompt via stdin to avoid Windows command line length limits (8191 chars)
60-
cmd := exec.CommandContext(ctx, "claude", "--model", a.model, "-p", "-")
61-
62-
stdin, err := cmd.StdinPipe()
57+
output, err := a.runCLI(ctx, fullPrompt)
6358
if err != nil {
64-
return nil, fmt.Errorf("error creating stdin pipe: %w", err)
65-
}
66-
67-
var outputBuf strings.Builder
68-
cmd.Stdout = &outputBuf
69-
cmd.Stderr = &outputBuf
70-
71-
if err := cmd.Start(); err != nil {
72-
return nil, fmt.Errorf("error starting claude CLI: %w", err)
73-
}
74-
75-
_, writeErr := stdin.Write([]byte(fullPrompt))
76-
stdin.Close()
77-
78-
waitErr := cmd.Wait()
79-
80-
if writeErr != nil {
81-
return nil, fmt.Errorf("error writing to claude CLI stdin: %w", writeErr)
82-
}
83-
84-
if waitErr != nil {
85-
return nil, fmt.Errorf("error executing claude CLI: %w\nOutput: %s", waitErr, outputBuf.String())
86-
}
87-
88-
output := []byte(outputBuf.String())
89-
90-
// Parse the output - split by newlines and clean
91-
content := string(output)
92-
lines := strings.Split(content, "\n")
93-
94-
var commitMessages []string
95-
for _, line := range lines {
96-
trimmed := strings.TrimSpace(line)
97-
// Skip empty lines and lines that look like explanatory text
98-
if trimmed == "" {
99-
continue
100-
}
101-
// Skip lines that are clearly not commit messages (too long, contain certain patterns)
102-
if len(trimmed) > 200 {
103-
continue
104-
}
105-
// Skip markdown formatting or numbered lists
106-
if strings.HasPrefix(trimmed, "#") || strings.HasPrefix(trimmed, "-") || strings.HasPrefix(trimmed, "*") {
107-
// Try to extract the actual commit message
108-
parts := strings.SplitN(trimmed, " ", 2)
109-
if len(parts) == 2 {
110-
trimmed = strings.TrimSpace(parts[1])
111-
}
112-
}
113-
// Remove numbered list formatting like "1. " or "1) "
114-
if len(trimmed) > 3 {
115-
if (trimmed[0] >= '0' && trimmed[0] <= '9') && (trimmed[1] == '.' || trimmed[1] == ')') {
116-
trimmed = strings.TrimSpace(trimmed[2:])
117-
}
118-
}
119-
120-
if trimmed != "" {
121-
commitMessages = append(commitMessages, trimmed)
122-
}
123-
124-
// Stop once we have enough messages
125-
if len(commitMessages) >= a.numSuggestions {
126-
break
127-
}
59+
return nil, err
12860
}
12961

62+
commitMessages := parseOutputLines(output, a.numSuggestions)
13063
if len(commitMessages) == 0 {
13164
return nil, fmt.Errorf("no valid commit messages generated from Claude output")
13265
}
@@ -163,77 +96,50 @@ func (a *AnthropicProvider) GeneratePRTitles(ctx context.Context, diff string) (
16396
fullPrompt := fmt.Sprintf("%s\n\nUser request: %s\n\nIMPORTANT: Generate exactly %d pull request titles, one per line. Do not include any other text, explanations, or formatting - just the PR titles.",
16497
systemMsg, userPrompt, a.numSuggestions)
16598

99+
output, err := a.runCLI(ctx, fullPrompt)
100+
if err != nil {
101+
return nil, err
102+
}
103+
104+
prTitles := parseOutputLines(output, a.numSuggestions)
105+
if len(prTitles) == 0 {
106+
return nil, fmt.Errorf("no valid PR titles generated from Claude output")
107+
}
108+
109+
return prTitles, nil
110+
}
111+
112+
// runCLI executes the claude CLI with the given prompt via stdin and returns stdout.
113+
func (a *AnthropicProvider) runCLI(ctx context.Context, prompt string) (string, error) {
114+
// Using -p flag for print mode and --model for model selection
166115
// Pipe prompt via stdin to avoid Windows command line length limits (8191 chars)
167116
cmd := exec.CommandContext(ctx, "claude", "--model", a.model, "-p", "-")
168117

169118
stdin, err := cmd.StdinPipe()
170119
if err != nil {
171-
return nil, fmt.Errorf("error creating stdin pipe: %w", err)
120+
return "", fmt.Errorf("error creating stdin pipe: %w", err)
172121
}
173122

174-
var outputBuf strings.Builder
175-
cmd.Stdout = &outputBuf
176-
cmd.Stderr = &outputBuf
123+
var stdoutBuf, stderrBuf strings.Builder
124+
cmd.Stdout = &stdoutBuf
125+
cmd.Stderr = &stderrBuf
177126

178127
if err := cmd.Start(); err != nil {
179-
return nil, fmt.Errorf("error starting claude CLI: %w", err)
128+
return "", fmt.Errorf("error starting claude CLI: %w", err)
180129
}
181130

182-
_, writeErr := stdin.Write([]byte(fullPrompt))
131+
_, writeErr := stdin.Write([]byte(prompt))
183132
stdin.Close()
184133

185134
waitErr := cmd.Wait()
186135

187136
if writeErr != nil {
188-
return nil, fmt.Errorf("error writing to claude CLI stdin: %w", writeErr)
137+
return "", fmt.Errorf("error writing to claude CLI stdin: %w", writeErr)
189138
}
190139

191140
if waitErr != nil {
192-
return nil, fmt.Errorf("error executing claude CLI: %w\nOutput: %s", waitErr, outputBuf.String())
193-
}
194-
195-
output := []byte(outputBuf.String())
196-
197-
// Parse the output - same logic as commit message generation
198-
content := string(output)
199-
lines := strings.Split(content, "\n")
200-
201-
var prTitles []string
202-
for _, line := range lines {
203-
trimmed := strings.TrimSpace(line)
204-
if trimmed == "" {
205-
continue
206-
}
207-
if len(trimmed) > 200 {
208-
continue
209-
}
210-
// Skip markdown formatting or numbered lists
211-
if strings.HasPrefix(trimmed, "#") || strings.HasPrefix(trimmed, "-") || strings.HasPrefix(trimmed, "*") {
212-
parts := strings.SplitN(trimmed, " ", 2)
213-
if len(parts) == 2 {
214-
trimmed = strings.TrimSpace(parts[1])
215-
}
216-
}
217-
// Remove numbered list formatting like "1. " or "1) "
218-
if len(trimmed) > 3 {
219-
if (trimmed[0] >= '0' && trimmed[0] <= '9') && (trimmed[1] == '.' || trimmed[1] == ')') {
220-
trimmed = strings.TrimSpace(trimmed[2:])
221-
}
222-
}
223-
224-
if trimmed != "" {
225-
prTitles = append(prTitles, trimmed)
226-
}
227-
228-
// Stop once we have enough titles
229-
if len(prTitles) >= a.numSuggestions {
230-
break
231-
}
232-
}
233-
234-
if len(prTitles) == 0 {
235-
return nil, fmt.Errorf("no valid PR titles generated from Claude output")
141+
return "", fmt.Errorf("error executing claude CLI: %w\nStderr: %s", waitErr, stderrBuf.String())
236142
}
237143

238-
return prTitles, nil
144+
return stdoutBuf.String(), nil
239145
}

‎internal/provider/common.go‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,50 @@ import (
44
"context"
55
"fmt"
66
"strings"
7+
"unicode"
78

89
"github.com/openai/openai-go"
910
)
1011

12+
// parseOutputLines parses raw LLM output into clean lines, stripping markdown
13+
// formatting, numbered/bulleted list prefixes, and skipping empty or overly long lines.
14+
// It returns at most maxLines results.
15+
func parseOutputLines(raw string, maxLines int) []string {
16+
lines := strings.Split(raw, "\n")
17+
18+
var result []string
19+
for _, line := range lines {
20+
trimmed := strings.TrimSpace(line)
21+
if trimmed == "" || len(trimmed) > 200 {
22+
continue
23+
}
24+
// Strip markdown heading, bullet, or asterisk prefix
25+
if strings.HasPrefix(trimmed, "#") || strings.HasPrefix(trimmed, "-") || strings.HasPrefix(trimmed, "*") {
26+
parts := strings.SplitN(trimmed, " ", 2)
27+
if len(parts) == 2 {
28+
trimmed = strings.TrimSpace(parts[1])
29+
}
30+
}
31+
// Strip numbered list prefix like "1. ", "10) ", "3. "
32+
if len(trimmed) > 0 && trimmed[0] >= '0' && trimmed[0] <= '9' {
33+
i := 0
34+
for i < len(trimmed) && unicode.IsDigit(rune(trimmed[i])) {
35+
i++
36+
}
37+
if i < len(trimmed) && (trimmed[i] == '.' || trimmed[i] == ')') {
38+
trimmed = strings.TrimSpace(trimmed[i+1:])
39+
}
40+
}
41+
if trimmed != "" {
42+
result = append(result, trimmed)
43+
}
44+
if len(result) >= maxLines {
45+
break
46+
}
47+
}
48+
return result
49+
}
50+
1151
// commonProvider holds the common fields and methods for OpenAI-compatible providers.
1252
type commonProvider struct {
1353
client *openai.Client

‎internal/provider/common_test.go‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
package provider
2+
3+
import (
4+
"strings"
5+
"testing"
6+
)
7+
8+
func TestParseOutputLines(t *testing.T) {
9+
tests := []struct {
10+
name string
11+
input string
12+
maxLines int
13+
expected []string
14+
}{
15+
{
16+
name: "plain lines",
17+
input: "feat: add login\nfix: typo in readme\nchore: update deps",
18+
maxLines: 10,
19+
expected: []string{"feat: add login", "fix: typo in readme", "chore: update deps"},
20+
},
21+
{
22+
name: "single-digit numbered list with dot",
23+
input: "1. feat: add login\n2. fix: typo\n3. chore: deps",
24+
maxLines: 10,
25+
expected: []string{"feat: add login", "fix: typo", "chore: deps"},
26+
},
27+
{
28+
name: "single-digit numbered list with paren",
29+
input: "1) feat: add login\n2) fix: typo",
30+
maxLines: 10,
31+
expected: []string{"feat: add login", "fix: typo"},
32+
},
33+
{
34+
name: "multi-digit numbered list",
35+
input: "10. feat: add login\n11. fix: typo\n12. chore: deps",
36+
maxLines: 10,
37+
expected: []string{"feat: add login", "fix: typo", "chore: deps"},
38+
},
39+
{
40+
name: "markdown bullet dashes",
41+
input: "- feat: add login\n- fix: typo",
42+
maxLines: 10,
43+
expected: []string{"feat: add login", "fix: typo"},
44+
},
45+
{
46+
name: "markdown bullet asterisks",
47+
input: "* feat: add login\n* fix: typo",
48+
maxLines: 10,
49+
expected: []string{"feat: add login", "fix: typo"},
50+
},
51+
{
52+
name: "markdown headings stripped",
53+
input: "# feat: add login\n## fix: typo",
54+
maxLines: 10,
55+
expected: []string{"feat: add login", "fix: typo"},
56+
},
57+
{
58+
name: "empty lines skipped",
59+
input: "feat: add login\n\n\nfix: typo\n\n",
60+
maxLines: 10,
61+
expected: []string{"feat: add login", "fix: typo"},
62+
},
63+
{
64+
name: "lines over 200 chars skipped",
65+
input: "feat: add login\n" + strings.Repeat("x", 201) + "\nfix: typo",
66+
maxLines: 10,
67+
expected: []string{"feat: add login", "fix: typo"},
68+
},
69+
{
70+
name: "respects maxLines limit",
71+
input: "line1\nline2\nline3\nline4\nline5",
72+
maxLines: 3,
73+
expected: []string{"line1", "line2", "line3"},
74+
},
75+
{
76+
name: "whitespace-only input",
77+
input: " \n \n\t\n",
78+
maxLines: 10,
79+
expected: nil,
80+
},
81+
{
82+
name: "mixed formatting",
83+
input: "1. feat: login\n- fix: typo\n* chore: deps\n## docs: readme\nplain message",
84+
maxLines: 10,
85+
expected: []string{"feat: login", "fix: typo", "chore: deps", "docs: readme", "plain message"},
86+
},
87+
{
88+
name: "number at start but no list separator",
89+
input: "3rd attempt at fixing auth",
90+
maxLines: 10,
91+
expected: []string{"3rd attempt at fixing auth"},
92+
},
93+
}
94+
95+
for _, tt := range tests {
96+
t.Run(tt.name, func(t *testing.T) {
97+
got := parseOutputLines(tt.input, tt.maxLines)
98+
if len(got) != len(tt.expected) {
99+
t.Fatalf("got %d lines %v, want %d lines %v", len(got), got, len(tt.expected), tt.expected)
100+
}
101+
for i := range got {
102+
if got[i] != tt.expected[i] {
103+
t.Errorf("line %d: got %q, want %q", i, got[i], tt.expected[i])
104+
}
105+
}
106+
})
107+
}
108+
}

0 commit comments

Comments
 (0)