diff --git a/internal/model/parse.go b/internal/model/parse.go index 5b2df1b..de22f8e 100644 --- a/internal/model/parse.go +++ b/internal/model/parse.go @@ -9,8 +9,10 @@ import ( ) // ParseError describes why a model's stdout couldn't be parsed. -// Kind narrows the cause for actionable error messages; Raw is a -// truncated copy of the offending output for debug surfaces. +// Kind narrows the cause for actionable error messages; Raw is the FULL +// offending output, retained untruncated so it can be surfaced for +// diagnosis (diffsmith-2xy). Error() prints only a bounded snippet of it; +// callers wanting the whole payload read Raw directly. type ParseError struct { Kind string // "prose_preamble" | "invalid_json" | "wrong_shape" Raw string @@ -18,10 +20,18 @@ type ParseError struct { } func (e *ParseError) Error() string { + msg := fmt.Sprintf("parse model output (%s)", e.Kind) if e.Cause != nil { - return fmt.Sprintf("parse model output (%s): %v", e.Kind, e.Cause) + msg += ": " + e.Cause.Error() } - return fmt.Sprintf("parse model output (%s)", e.Kind) + // Surface a bounded snippet of the raw output. Adapters wrap this + // error with %w and the dropped-model run summary prints it with %v, + // so without this a model that returned garbage leaves no trace of + // what it actually said. The full payload stays on Raw. + if e.Raw != "" { + msg += fmt.Sprintf(" [raw: %s]", truncate(e.Raw, 200)) + } + return msg } func (e *ParseError) Unwrap() error { return e.Cause } @@ -41,7 +51,7 @@ func ParseFindings(raw []byte) ([]review.FindingCandidate, error) { trimmed := stripWrapper(string(raw)) if !strings.HasPrefix(trimmed, "{") { - return nil, &ParseError{Kind: "prose_preamble", Raw: truncate(trimmed, 200)} + return nil, &ParseError{Kind: "prose_preamble", Raw: trimmed} } // Findings is a pointer so we can distinguish "key missing" (nil @@ -56,14 +66,14 @@ func ParseFindings(raw []byte) ([]review.FindingCandidate, error) { if err := json.Unmarshal([]byte(trimmed), &envelope); err != nil { return nil, &ParseError{ Kind: "invalid_json", - Raw: truncate(trimmed, 200), + Raw: trimmed, Cause: err, } } if envelope.Findings == nil { return nil, &ParseError{ Kind: "wrong_shape", - Raw: truncate(trimmed, 200), + Raw: trimmed, } } return *envelope.Findings, nil diff --git a/internal/model/parse_test.go b/internal/model/parse_test.go index 69ea21b..2992a66 100644 --- a/internal/model/parse_test.go +++ b/internal/model/parse_test.go @@ -2,9 +2,42 @@ package model import ( "errors" + "strings" "testing" ) +// TestParseErrorPreservesFullRawOutput is diffsmith-2xy: when a model +// emits unparseable output, the FULL raw payload must be retained (not +// truncated) so it can be surfaced for diagnosis. Previously Raw was +// clipped to 200 chars, discarding the rest of the evidence. +func TestParseErrorPreservesFullRawOutput(t *testing.T) { + raw := []byte(`{"findings":[` + strings.Repeat("x", 400) + `TAIL_MARKER`) + _, err := ParseFindings(raw) + var pe *ParseError + if !errors.As(err, &pe) { + t.Fatalf("want *ParseError, got %T: %v", err, err) + } + if !strings.Contains(pe.Raw, "TAIL_MARKER") { + t.Errorf("Raw must preserve the full output (len %d); the tail past 200 chars was dropped", len(pe.Raw)) + } +} + +// TestParseErrorMessageIncludesRawSnippet ensures the error STRING carries +// a snippet of the offending output. Adapters wrap this error with %w and +// the dropped-model run summary prints it with %v, so without a snippet a +// user whose model returned garbage sees no bytes of that garbage. +func TestParseErrorMessageIncludesRawSnippet(t *testing.T) { + raw := []byte(`{"oops": SNIPPET_MARKER not valid json`) + _, err := ParseFindings(raw) + var pe *ParseError + if !errors.As(err, &pe) { + t.Fatalf("want *ParseError, got %T: %v", err, err) + } + if !strings.Contains(pe.Error(), "SNIPPET_MARKER") { + t.Errorf("Error() should include a raw snippet for diagnosis; got %q", pe.Error()) + } +} + func TestParseFindingsHappy(t *testing.T) { raw := []byte(`{ "findings": [