Skip to content
Merged
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
24 changes: 17 additions & 7 deletions internal/model/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,29 @@ 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
Cause error
}

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 }
Expand All @@ -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
Expand All @@ -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
Expand Down
33 changes: 33 additions & 0 deletions internal/model/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand Down
Loading