fix: repair unescaped quotes in LLM JSON responses (#45)
CI / test (push) Successful in 14s
CI / review (/anthropic/v1, claude-sonnet-4-6, sonnet, anthropic, SONNET_REVIEW_TOKEN) (push) Has been skipped
CI / review (/openai/v1, gpt-4.1, gpt41, openai, GPT_REVIEW_TOKEN) (push) Has been skipped
CI / review (/openai/v1, gpt-4.1-mini, gpt41-mini, openai, GPT_REVIEW_TOKEN) (push) Has been skipped
CI / review (/openai/v1, gpt-5, gpt, openai, GPT_REVIEW_TOKEN) (push) Has been skipped
CI / review (/openai/v1, gpt-5, security, openai, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped
CI / review (/openai/v1, gpt-5-mini, gpt5-mini, openai, GPT_REVIEW_TOKEN) (push) Has been skipped
Release / release (push) Successful in 34s

fix: repair unescaped quotes in LLM JSON responses

Add repairJSON fallback that handles unescaped quotes in LLM string
values using first-valid-candidate heuristic with structural lookahead.

Reviewed-by: sonnet-review-bot
Reviewed-by: gpt-review-bot
Reviewed-by: security-review-bot
This commit was merged in pull request #45.
This commit is contained in:
2026-05-05 12:40:39 +00:00
parent 55cf3fd4b9
commit cabbb5a55a
2 changed files with 343 additions and 1 deletions
+110
View File
@@ -1,6 +1,7 @@
package review
import (
"encoding/json"
"testing"
)
@@ -112,3 +113,112 @@ func TestParseResponse_MarkdownFencesNoLang(t *testing.T) {
t.Errorf("expected APPROVE, got %q", result.Verdict)
}
}
func TestParseResponse_UnescapedQuotesInStrings(t *testing.T) {
// Real failure from CI: Sonnet puts unescaped quotes like (e.g. "28") in findings
input := `{"verdict": "APPROVE", "summary": "Clean PR", "findings": [{"severity": "NIT", "file": "ci/Dockerfile", "line": 14, "finding": "The comment says OTP_VERSION is the major version (e.g. \"28\") but it actually contains unescaped quotes like (e.g. "28") which breaks JSON"}], "recommendation": "Ship it"}`
result, err := ParseResponse(input)
if err != nil {
t.Fatalf("expected repair to handle unescaped quotes, got error: %v", err)
}
if result.Verdict != "APPROVE" {
t.Errorf("expected APPROVE, got %q", result.Verdict)
}
if len(result.Findings) != 1 {
t.Fatalf("expected 1 finding, got %d", len(result.Findings))
}
}
func TestRepairJSON_NoOpOnValid(t *testing.T) {
valid := `{"key": "value", "num": 42}`
result := repairJSON(valid)
if result != valid {
t.Errorf("repairJSON should not modify valid JSON\n got: %s\n want: %s", result, valid)
}
}
func TestRepairJSON_FixesUnescapedQuotes(t *testing.T) {
// Interior quote followed by non-structural character
input := `{"msg": "use "foo" here"}`
result := repairJSON(input)
// Should be parseable now
var m map[string]interface{}
if err := json.Unmarshal([]byte(result), &m); err != nil {
t.Fatalf("repaired JSON should parse, got: %v\nrepaired: %s", err, result)
}
}
func TestRepairJSON_InteriorQuoteBeforeComma(t *testing.T) {
// Bug reported by reviewer: interior quoted word immediately before a comma
input := `{"msg": "say "yes", and go"}`
result := repairJSON(input)
var m map[string]interface{}
if err := json.Unmarshal([]byte(result), &m); err != nil {
t.Fatalf("repaired JSON should parse, got: %v\nrepaired: %s", err, result)
}
// The full string content should be preserved
msg, ok := m["msg"].(string)
if !ok {
t.Fatal("msg field missing or not a string")
}
if msg != `say "yes", and go` {
t.Errorf("unexpected msg content: %q", msg)
}
}
func TestRepairJSON_InteriorQuoteBeforeCloseBrace(t *testing.T) {
// Bug reported by reviewer: JSON-shaped syntax inside string values
input := `{"msg": "input map {"key": "val"} caused error"}`
result := repairJSON(input)
var m map[string]interface{}
if err := json.Unmarshal([]byte(result), &m); err != nil {
t.Fatalf("repaired JSON should parse, got: %v\nrepaired: %s", err, result)
}
}
func TestRepairJSON_MultipleFields(t *testing.T) {
// Multiple string fields with unescaped quotes in different positions
input := `{"a": "hello "world"", "b": "foo"}`
result := repairJSON(input)
var m map[string]interface{}
if err := json.Unmarshal([]byte(result), &m); err != nil {
t.Fatalf("repaired JSON should parse, got: %v\nrepaired: %s", err, result)
}
if _, ok := m["b"]; !ok {
t.Error("expected 'b' field to be preserved")
}
}
func TestRepairJSON_PreservesEscapedQuotes(t *testing.T) {
// Already-escaped quotes should not be double-escaped
input := `{"msg": "already \"escaped\" here"}`
result := repairJSON(input)
if result != input {
t.Errorf("repairJSON should not modify already-escaped quotes\n got: %s\n want: %s", result, input)
}
var m map[string]interface{}
if err := json.Unmarshal([]byte(result), &m); err != nil {
t.Fatalf("repaired JSON should parse, got: %v\nrepaired: %s", err, result)
}
}
func TestRepairJSON_ComplexNestedContent(t *testing.T) {
// Combines both reviewer bugs: quoted words before commas AND JSON-like content
input := `{"verdict": "APPROVE", "findings": [{"finding": "The map {"key": "val"} and (e.g. "28") and say "yes", then stop"}]}`
result := repairJSON(input)
var parsed map[string]interface{}
if err := json.Unmarshal([]byte(result), &parsed); err != nil {
t.Fatalf("repaired JSON should parse, got: %v\nrepaired: %s", err, result)
}
if parsed["verdict"] != "APPROVE" {
t.Errorf("expected verdict APPROVE, got %v", parsed["verdict"])
}
}