Skip to content
Open
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: 43 additions & 6 deletions cmd/entire/cli/summarize/claude.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,10 @@ type ClaudeGenerator struct {
}

// claudeCLIResponse represents the JSON response from the Claude CLI.
// Claude Code 1.x returns a single object: {"result": "..."}
// Claude Code 2.x returns an array: [{"type":"system",...}, {"type":"result", "result":"..."}]
type claudeCLIResponse struct {
Type string `json:"type"`
Result string `json:"result"`
}
Comment on lines 69 to 75
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JSON example in this comment isn’t valid JSON ({type:"system"} is missing quotes around keys). Since this is explaining a wire format, it’s worth using valid JSON in the example to avoid confusion.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — doc comment now uses valid JSON with quoted keys.


Expand Down Expand Up @@ -133,15 +136,14 @@ func (g *ClaudeGenerator) Generate(ctx context.Context, input Input) (*checkpoin
return nil, fmt.Errorf("failed to run claude CLI: %w", err)
}

// Parse the CLI response
var cliResponse claudeCLIResponse
if err := json.Unmarshal(stdout.Bytes(), &cliResponse); err != nil {
// Parse the CLI response.
// Claude Code 2.x returns a JSON array of messages; extract the "result" element.
// Claude Code 1.x returns a single JSON object; try that as a fallback.
resultJSON, err := parseClaudeCLIResult(stdout.Bytes())
if err != nil {
return nil, fmt.Errorf("failed to parse claude CLI response: %w", err)
}

// The result field contains the actual JSON summary
resultJSON := cliResponse.Result

// Try to extract JSON if it's wrapped in markdown code blocks
resultJSON = extractJSONFromMarkdown(resultJSON)

Expand All @@ -154,6 +156,41 @@ func (g *ClaudeGenerator) Generate(ctx context.Context, input Input) (*checkpoin
return &summary, nil
}

// parseClaudeCLIResult extracts the result string from the Claude CLI JSON output.
// It handles both formats:
// - Claude Code 2.x: JSON array with a "type":"result" element
// - Claude Code 1.x: single JSON object with a "result" field
func parseClaudeCLIResult(data []byte) (string, error) {
// Try array format first (Claude Code 2.x)
var responses []claudeCLIResponse
if err := json.Unmarshal(data, &responses); err == nil {
hasEmptyResultElement := false
for _, r := range responses {
if r.Type == "result" {
if r.Result == "" {
hasEmptyResultElement = true
continue
}
return r.Result, nil
}
}
Comment on lines +168 to +176
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the 2.x array path, a {"type":"result","result":""} element currently triggers the generic "array contained no result element" error because the loop only returns when Result != "". Consider treating an explicit result element with an empty result as a distinct error (e.g., "contained empty result") to match the single-object behavior and make debugging easier.

Suggested change
for _, r := range responses {
if r.Type == "result" && r.Result != "" {
return r.Result, nil
}
}
hasEmptyResultElement := false
for _, r := range responses {
if r.Type == "result" {
if r.Result == "" {
hasEmptyResultElement = true
continue
}
return r.Result, nil
}
}
if hasEmptyResultElement {
return "", errors.New("claude CLI response array contained empty result")
}

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — added hasEmptyResultElement tracking in the array path. Now returns 'contained empty result' (matching 1.x behavior) instead of the generic 'no result element' message.

if hasEmptyResultElement {
return "", errors.New("claude CLI response contained empty result")
}
return "", errors.New("claude CLI response array contained no result element")
}

// Fall back to single object (Claude Code 1.x)
var single claudeCLIResponse
if err := json.Unmarshal(data, &single); err != nil {
return "", fmt.Errorf("failed to parse claude CLI response: %w", err)
}
if single.Result == "" {
return "", errors.New("claude CLI response contained empty result")
}
return single.Result, nil
}

// buildSummarizationPrompt creates the prompt for the Claude CLI.
func buildSummarizationPrompt(transcriptText string) string {
return fmt.Sprintf(summarizationPromptTemplate, transcriptText)
Expand Down
130 changes: 130 additions & 0 deletions cmd/entire/cli/summarize/claude_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,67 @@ func TestClaudeGenerator_NonZeroExit(t *testing.T) {
}
}

func TestClaudeGenerator_ArrayResponse(t *testing.T) {
t.Parallel()

summaryJSON := `{\"intent\":\"Fix a bug\",\"outcome\":\"Bug fixed\",\"learnings\":{\"repo\":[],\"code\":[],\"workflow\":[]},\"friction\":[],\"open_items\":[]}`
// Claude Code 2.x array format with system, assistant, and result elements
response := `[{"type":"system","system":"..."},{"type":"assistant","message":"..."},{"type":"result","result":"` + summaryJSON + `"}]`

gen := &ClaudeGenerator{
CommandRunner: func(ctx context.Context, _ string, _ ...string) *exec.Cmd {
return exec.CommandContext(ctx, "sh", "-c", "printf '%s' '"+response+"'")
},
}

input := Input{
Transcript: []Entry{
{Type: EntryTypeUser, Content: "Fix the bug"},
},
}

summary, err := gen.Generate(context.Background(), input)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}

if summary.Intent != "Fix a bug" {
t.Errorf("unexpected intent: %s", summary.Intent)
}

if summary.Outcome != "Bug fixed" {
t.Errorf("unexpected outcome: %s", summary.Outcome)
}
}

func TestClaudeGenerator_ArrayResponseNoResult(t *testing.T) {
t.Parallel()

// Array with no "result" type element
response := `[{"type":"system","system":"..."},{"type":"assistant","message":"..."}]`

gen := &ClaudeGenerator{
CommandRunner: func(ctx context.Context, _ string, _ ...string) *exec.Cmd {
return exec.CommandContext(ctx, "sh", "-c", "printf '%s' '"+response+"'")
},
}

input := Input{
Transcript: []Entry{
{Type: EntryTypeUser, Content: "Hello"},
},
}

_, err := gen.Generate(context.Background(), input)
if err == nil {
t.Fatal("expected error when array has no result element")
}

if !strings.Contains(err.Error(), "no result element") {
t.Errorf("expected 'no result element' error, got: %v", err)
}
}

func TestClaudeGenerator_ErrorCases(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -247,6 +308,75 @@ func TestClaudeGenerator_MarkdownCodeBlock(t *testing.T) {
}
}

func TestParseClaudeCLIResult(t *testing.T) {
t.Parallel()

tests := []struct {
name string
input string
expected string
expectError string
}{
{
name: "single object (1.x format)",
input: `{"result": "the summary"}`,
expected: "the summary",
},
{
name: "array with result (2.x format)",
input: `[{"type":"system","system":"..."},{"type":"result","result":"the summary"}]`,
expected: "the summary",
},
{
name: "array picks result type over others",
input: `[{"type":"assistant","result":"wrong"},{"type":"result","result":"correct"}]`,
expected: "correct",
},
{
name: "array with no result element",
input: `[{"type":"system"},{"type":"assistant"}]`,
expectError: "no result element",
},
{
name: "invalid JSON",
input: `not json at all`,
expectError: "parse claude CLI response",
},
{
name: "single object with empty result",
input: `{"result": ""}`,
expectError: "empty result",
},
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TestParseClaudeCLIResult covers empty result only for the 1.x single-object format. Since parseClaudeCLIResult now has distinct logic for the 2.x array format, consider adding a test case for an array that includes a { "type": "result", "result": "" } element to lock in the intended behavior/error message.

Suggested change
},
},
{
name: "array with empty result element",
input: `[{"type":"system"},{"type":"result","result":""}]`,
expectError: "empty result",
},

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added — new test case "array with empty result element" covers this path and expects the "empty result" error.

{
name: "array with empty result element",
input: `[{"type":"system"},{"type":"result","result":""}]`,
expectError: "empty result",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
result, err := parseClaudeCLIResult([]byte(tt.input))
if tt.expectError != "" {
if err == nil {
t.Fatalf("expected error containing %q, got nil", tt.expectError)
}
if !strings.Contains(err.Error(), tt.expectError) {
t.Errorf("expected error containing %q, got: %v", tt.expectError, err)
}
return
}
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if result != tt.expected {
t.Errorf("expected %q, got %q", tt.expected, result)
}
})
}
}

func TestBuildSummarizationPrompt(t *testing.T) {
transcriptText := "[User] Hello\n\n[Assistant] Hi"

Expand Down