From d17b7a0c2310772102726e7bf32f56fa97cc18e8 Mon Sep 17 00:00:00 2001 From: Christopher Hlubek Date: Sun, 6 Sep 2026 23:06:43 +0200 Subject: [PATCH 1/3] fix(llmops): extract a single valid JSON response --- internal/llmops/json_response_test.go | 120 ++++++++++++++++++++++++++ internal/llmops/preflight.go | 45 +++++++--- internal/llmops/writingguide.go | 4 + 3 files changed, 155 insertions(+), 14 deletions(-) create mode 100644 internal/llmops/json_response_test.go diff --git a/internal/llmops/json_response_test.go b/internal/llmops/json_response_test.go new file mode 100644 index 00000000..c6002c0e --- /dev/null +++ b/internal/llmops/json_response_test.go @@ -0,0 +1,120 @@ +package llmops_test + +import ( + "context" + "strings" + "testing" + + "github.com/networkteam/sdd/internal/basefacts" + "github.com/networkteam/sdd/internal/llmops" + "github.com/networkteam/sdd/internal/model" + "github.com/networkteam/sdd/internal/viewlayout" + "github.com/networkteam/sdd/pkg/llm" +) + +type responseFactSource struct{ graph *model.Graph } + +func (s responseFactSource) FactBody(id string) (string, error) { + _, body, err := s.graph.FactBody(id) + return body, err +} + +func TestCheckersJSONResponse(t *testing.T) { + facts, err := basefacts.Entries(viewlayout.Vocabulary{}) + if err != nil { + t.Fatal(err) + } + graph := model.NewGraph(facts) + entry := &model.Entry{ + Type: model.TypeSignal, Kind: model.KindGap, Layer: model.LayerOperational, + Content: "The config ignores Options{Zebra: true}.", + } + checkers := []struct { + name string + payload string + run func(llm.Runner) (int, error) + }{ + { + name: "preflight", + payload: `{"findings":[{"severity":"high","category":"missing-ref","observation":"The affected configuration decision is not referenced."}]}`, + run: func(runner llm.Runner) (int, error) { + result, err := llmops.Preflight(t.Context(), runner, entry, graph, "") + if err != nil { + return 0, err + } + return len(result.Findings), nil + }, + }, + { + name: "writing_guide", + payload: `{"findings":[{"reasoning":"The configuration is not identified.","axis":"stranding","quote":"The config","repair":"write-in","severity":"substantive"}]}`, + run: func(runner llm.Runner) (int, error) { + result, err := llmops.WritingGuide(t.Context(), runner, entry, nil, llmops.ReferenceFacts{ + Source: responseFactSource{graph}, TypeSystemFactID: basefacts.OverviewFactID, + }) + if err != nil { + return 0, err + } + return len(result.Findings), nil + }, + }, + } + for _, checker := range checkers { + t.Run(checker.name, func(t *testing.T) { + cases := []struct { + name string + output string + want int + wantErr bool + }{ + {name: "plain", output: checker.payload, want: 1}, + {name: "fenced_with_prose", output: "Here is the review:\n```json\n" + checker.payload + "\n```\nDone.", want: 1}, + {name: "literal_in_json_string", output: strings.Replace(checker.payload, "The", "Options{Zebra: true} affects the", 1), want: 1}, + {name: "literal_after_payload", output: checker.payload + "\nReviewed Options{Zebra: true}.", want: 1}, + {name: "go_literal_before_payload", output: "Reviewed Options{Zebra: true}.\n" + checker.payload, want: 1}, + {name: "user_literal_before_payload", output: "Reviewed User{ID: userID}.\n" + checker.payload, want: 1}, + {name: "empty_object_before_payload", output: "Reviewed {}.\n" + checker.payload, wantErr: true}, + {name: "unrelated_json_before_payload", output: "Reviewed {\"Zebra\":true}.\n" + checker.payload, wantErr: true}, + {name: "nested_code_literal", output: "Reviewed Options{Child: Child{Enabled: true}}.\n" + checker.payload, want: 1}, + {name: "quoted_prose", output: "The \"review\" follows.\n" + checker.payload, want: 1}, + {name: "escaped_strings", output: strings.Replace(checker.payload, "The", `Quoted \"{x}\" and C:\\tmp affect the`, 1), want: 1}, + {name: "two_results", output: checker.payload + "\n" + checker.payload, wantErr: true}, + {name: "clean_result_after_findings", output: checker.payload + `{"findings":[]}`, wantErr: true}, + {name: "empty_object_after_payload", output: checker.payload + "\n{}", wantErr: true}, + {name: "nested_result_is_not_promoted", output: `{"wrapper":` + checker.payload + `}`, wantErr: true}, + {name: "invalid_outer_object", output: `{wrapper:` + checker.payload + `}`, wantErr: true}, + {name: "unclosed_prose_brace", output: "Reviewed {\n" + checker.payload, wantErr: true}, + {name: "truncated_second_result", output: checker.payload + `{"findings":[`, wantErr: true}, + {name: "findings_object", output: `{"findings":{}}`, wantErr: true}, + {name: "findings_string", output: `{"findings":"[]"}`, wantErr: true}, + {name: "invalid_finding", output: `{"findings":[{}]}`, wantErr: true}, + {name: "no_object", output: "No findings.", wantErr: true}, + {name: "empty_response", output: "", wantErr: true}, + {name: "explicit_empty_findings", output: `{"findings":[]}`}, + {name: "missing_findings", output: `{}`, wantErr: true}, + {name: "null_findings", output: `{"findings":null}`, wantErr: true}, + {name: "malformed_findings", output: `{"findings":[{severity:high}]}`, wantErr: true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + runner := llm.RunnerFunc(func(context.Context, llm.Request) (llm.Result, error) { + return llm.Result{Text: tc.output}, nil + }) + got, err := checker.run(runner) + if tc.wantErr { + if err == nil { + t.Fatalf("expected invalid response error, got %d findings", got) + } + return + } + if err != nil { + t.Fatal(err) + } + if got != tc.want { + t.Fatalf("got %d findings, want %d", got, tc.want) + } + }) + } + }) + } +} diff --git a/internal/llmops/preflight.go b/internal/llmops/preflight.go index 60d6acc8..29ce6fea 100644 --- a/internal/llmops/preflight.go +++ b/internal/llmops/preflight.go @@ -554,7 +554,7 @@ func renderPreflightPrompt(ct checkType, pctx *preflightContext) (llm.Request, e // // Empty findings array means "no findings". The parser tolerates prose // surrounding the JSON object (LLM preambles, code fences) by scanning for -// the first balanced {...}. Malformed JSON, missing keys, unknown severity +// a single valid JSON object. Malformed JSON, missing keys, unknown severity // values — all return errors so infrastructure failures stay distinct from // findings. func parsePreflightResult(output string) (*PreflightResult, error) { @@ -574,6 +574,10 @@ func parsePreflightResult(output string) (*PreflightResult, error) { return nil, fmt.Errorf("parsing pre-flight JSON: %w", err) } + if resp.Findings == nil { + return nil, fmt.Errorf("findings must be a non-null array") + } + findings := make([]Finding, 0, len(resp.Findings)) for i, f := range resp.Findings { sev, err := parseSeverity(f.Severity) @@ -596,27 +600,28 @@ func parsePreflightResult(output string) (*PreflightResult, error) { return &PreflightResult{Findings: findings}, nil } -// extractJSONObject returns the first balanced {...} in the input, skipping -// any surrounding prose or code fences. Returns an error if no object is -// found or braces are unbalanced. String-escape aware so braces inside -// JSON strings don't confuse the balance counter. Shared by every JSON-shaped -// LLM check (pre-flight, writing guide). +// Balanced non-JSON literals are prose; multiple JSON objects are ambiguous. +// Inspect only outermost groups so nested findings cannot become the result. func extractJSONObject(output string) (string, error) { output = strings.TrimSpace(output) if output == "" { return "", fmt.Errorf("empty LLM response") } - start := strings.Index(output, "{") - if start < 0 { - return "", fmt.Errorf("no JSON object found in LLM response: %q", output) - } - + start := 0 + var object string depth := 0 inString := false escape := false - for i := start; i < len(output); i++ { + for i := 0; i < len(output); i++ { c := output[i] + if depth == 0 { + if c == '{' { + start = i + depth = 1 + } + continue + } if escape { escape = false continue @@ -638,11 +643,23 @@ func extractJSONObject(output string) (string, error) { case '}': depth-- if depth == 0 { - return output[start : i+1], nil + candidate := output[start : i+1] + if json.Valid([]byte(candidate)) { + if object != "" { + return "", fmt.Errorf("multiple JSON objects found in LLM response") + } + object = candidate + } } } } - return "", fmt.Errorf("unbalanced JSON braces in LLM response: %q", output) + if depth != 0 { + return "", fmt.Errorf("unbalanced JSON braces in LLM response: %q", output) + } + if object == "" { + return "", fmt.Errorf("no JSON object found in LLM response: %q", output) + } + return object, nil } func parseSeverity(s string) (Severity, error) { diff --git a/internal/llmops/writingguide.go b/internal/llmops/writingguide.go index a9bcc60e..1f60c680 100644 --- a/internal/llmops/writingguide.go +++ b/internal/llmops/writingguide.go @@ -240,6 +240,10 @@ func parseWritingGuideResult(output string) (*WritingGuideResult, error) { return nil, fmt.Errorf("parsing writing-guide JSON: %w", err) } + if resp.Findings == nil { + return nil, fmt.Errorf("findings must be a non-null array") + } + findings := make([]GuideFinding, 0, len(resp.Findings)) for i, f := range resp.Findings { axis := strings.ToLower(strings.TrimSpace(f.Axis)) From 7d2dc2e15245fd0b3b893e18c32e35639113dfce Mon Sep 17 00:00:00 2001 From: Christopher Hlubek Date: Sun, 6 Sep 2026 23:17:13 +0200 Subject: [PATCH 2/3] fix(llmops): clarify checker JSON output instructions --- .../llmops/preflight_templates/verdict.tmpl | 16 ++++++++------- .../shared_templates/json_response_rules.tmpl | 3 +++ .../shared_templates/json_string_rules.tmpl | 2 +- .../writingguide_templates/writing_guide.tmpl | 20 ++++++++++--------- 4 files changed, 24 insertions(+), 17 deletions(-) create mode 100644 internal/llmops/shared_templates/json_response_rules.tmpl diff --git a/internal/llmops/preflight_templates/verdict.tmpl b/internal/llmops/preflight_templates/verdict.tmpl index 7e151b5c..07765431 100644 --- a/internal/llmops/preflight_templates/verdict.tmpl +++ b/internal/llmops/preflight_templates/verdict.tmpl @@ -1,25 +1,27 @@ {{ define "verdict" }} ## Output format -Respond with a single JSON object matching this schema — nothing before or after, no code fences: +{{ template "json_response_rules" }} + +Example with one finding: ``` { "findings": [ { - "observation": "one to three sentences working through what you noticed, ending in a conclusion", - "category": "short-hyphenated-tag", - "severity": "high" | "medium" | "low" + "observation": "The entry claims completion without addressing the plan's second acceptance criterion.", + "category": "ac-unaddressed", + "severity": "high" } ] } ``` -**Field order matters: reasoning first, verdict last.** The `observation` is your reasoning — work through what you noticed first. Then `category` names what the observation found, and `severity` scores it; both are conclusions, assigned only after the observation has reached its own. The severity must match where the observation lands, not where it started. If the observation concludes that the concern does not hold ("withdrawn", "not a finding", "no issue"), emit no finding at all — drop it from the array rather than emitting it with any severity. +**Field order matters: reasoning first, verdict last.** In `observation`, work through what you noticed in one to three sentences, ending in a conclusion. Then `category` names what the observation found, and `severity` scores it; both are conclusions, assigned only after the observation has reached its own. The severity must match where the observation lands, not where it started. If the observation concludes that the concern does not hold ("withdrawn", "not a finding", "no issue"), emit no finding at all — drop it from the array rather than emitting it with any severity. -All three fields are required on every finding — a finding without a `category` or without a `severity` is unparseable and fails the whole response. +All three fields are required on every finding. `severity` must be `high`, `medium`, or `low`. -Category is a short hyphenated tag (e.g. `type-mismatch`, `missing-ref`, `plan-coverage-ambiguity`, `ac-unaddressed`). When you have nothing to report, return `{"findings": []}`. +Category is a short hyphenated tag (e.g. `type-mismatch`, `missing-ref`, `plan-coverage-ambiguity`, `ac-unaddressed`). {{ template "json_string_rules" }} diff --git a/internal/llmops/shared_templates/json_response_rules.tmpl b/internal/llmops/shared_templates/json_response_rules.tmpl new file mode 100644 index 00000000..74922093 --- /dev/null +++ b/internal/llmops/shared_templates/json_response_rules.tmpl @@ -0,0 +1,3 @@ +{{ define "json_response_rules" -}} +Return exactly one JSON object with a required `findings` array. For no findings, return `{"findings":[]}`. Never omit `findings` or set it to `null`. Put all explanations and quoted source text inside the finding fields. Write no text outside the object and use no Markdown fences. +{{- end }} diff --git a/internal/llmops/shared_templates/json_string_rules.tmpl b/internal/llmops/shared_templates/json_string_rules.tmpl index 095625d7..193032f6 100644 --- a/internal/llmops/shared_templates/json_string_rules.tmpl +++ b/internal/llmops/shared_templates/json_string_rules.tmpl @@ -1,3 +1,3 @@ {{ define "json_string_rules" -}} -**JSON strings.** Every value is a JSON string with standard escaping: write a literal `"` as `\"`, a backslash as `\\`, and a line break as `\n`. Never put a raw line break inside a value. Reproduce cited text exactly and escape it; do not substitute or drop characters to avoid escaping. When a prose value cites an excerpt, set the excerpt off with single quotes (`'like this'`) or backticks. +**JSON strings.** Every field within a finding has a string value. Use standard JSON escaping: write a literal `"` as `\"`, a backslash as `\\`, and a line break as `\n`. Never put a raw line break inside a value. Reproduce cited text exactly and escape it; do not substitute or drop characters to avoid escaping. When a prose value cites an excerpt, set the excerpt off with single quotes (`'like this'`) or backticks. {{- end }} diff --git a/internal/llmops/writingguide_templates/writing_guide.tmpl b/internal/llmops/writingguide_templates/writing_guide.tmpl index 56ac7b2e..271f0784 100644 --- a/internal/llmops/writingguide_templates/writing_guide.tmpl +++ b/internal/llmops/writingguide_templates/writing_guide.tmpl @@ -38,23 +38,25 @@ Check only these axes: ## Output format -Respond with a single JSON object matching this schema — nothing before or after, no code fences: +{{ template "json_response_rules" }} + +Example with one finding: { "findings": [ { - "reasoning": "one to three sentences working through what you noticed and why it fails the pull, ending in a conclusion", - "axis": "stranding" | "dilution" | "conflation" | "pointing" | "form", - "quote": "the draft's words at issue, verbatim", - "repair": "cut" | "write-in" | "split" | "point" | "reword", - "severity": "substantive" | "minor" + "reasoning": "The draft refers to an earlier approach without identifying it, so a reader cannot tell what changed.", + "axis": "stranding", + "quote": "the earlier approach", + "repair": "write-in", + "severity": "substantive" } ] } -**Field order matters: reasoning first, verdict last.** The reasoning is your work — walk through what you noticed before naming the axis, repair, and severity; they are conclusions, assigned only after the reasoning has reached its own. If the reasoning concludes the concern does not hold, emit no finding at all — drop it from the array rather than emitting it at any severity. +**Field order matters: reasoning first, verdict last.** In `reasoning`, work through what you noticed and why it fails the pull in one to three sentences, ending in a conclusion. Then name the axis, repair, and severity; they are conclusions, assigned only after the reasoning has reached its own. If the reasoning concludes the concern does not hold, emit no finding at all — drop it from the array rather than emitting it at any severity. -All five fields are required on every finding. `severity` is `substantive` when the drafting dialogue should take the finding up, `minor` when folding it in or ignoring it is fine. +All five fields are required on every finding. `axis` must be `stranding`, `dilution`, `conflation`, `pointing`, or `form`. `repair` must be `cut`, `write-in`, `split`, `point`, or `reword`. `severity` must be `substantive` or `minor`. `severity` is `substantive` when the drafting dialogue should take the finding up, `minor` when folding it in or ignoring it is fine. {{ template "json_string_rules" }} @@ -62,7 +64,7 @@ All five fields are required on every finding. `severity` is `substantive` when ## Calibration -No findings is a correct outcome — return {"findings": []} without hesitation. Do not stretch: a finding you would not defend is noise that costs more than it saves. When unsure whether something fails, it does not. Anything outside the five axes is out of scope. +No findings is a correct outcome. Do not stretch: a finding you would not defend is noise that costs more than it saves. When unsure whether something fails, it does not. Anything outside the five axes is out of scope. {{- end }} {{ define "writing_guide_user" -}} From 791cb44f0606c54ee9a8964c9e5801dbfa463165 Mon Sep 17 00:00:00 2001 From: Christopher Hlubek Date: Sun, 6 Sep 2026 23:52:12 +0200 Subject: [PATCH 3/3] fix(llmops): restore prompts after inconclusive evals --- .../llmops/preflight_templates/verdict.tmpl | 16 +++++++-------- .../shared_templates/json_response_rules.tmpl | 3 --- .../shared_templates/json_string_rules.tmpl | 2 +- .../writingguide_templates/writing_guide.tmpl | 20 +++++++++---------- 4 files changed, 17 insertions(+), 24 deletions(-) delete mode 100644 internal/llmops/shared_templates/json_response_rules.tmpl diff --git a/internal/llmops/preflight_templates/verdict.tmpl b/internal/llmops/preflight_templates/verdict.tmpl index 07765431..7e151b5c 100644 --- a/internal/llmops/preflight_templates/verdict.tmpl +++ b/internal/llmops/preflight_templates/verdict.tmpl @@ -1,27 +1,25 @@ {{ define "verdict" }} ## Output format -{{ template "json_response_rules" }} - -Example with one finding: +Respond with a single JSON object matching this schema — nothing before or after, no code fences: ``` { "findings": [ { - "observation": "The entry claims completion without addressing the plan's second acceptance criterion.", - "category": "ac-unaddressed", - "severity": "high" + "observation": "one to three sentences working through what you noticed, ending in a conclusion", + "category": "short-hyphenated-tag", + "severity": "high" | "medium" | "low" } ] } ``` -**Field order matters: reasoning first, verdict last.** In `observation`, work through what you noticed in one to three sentences, ending in a conclusion. Then `category` names what the observation found, and `severity` scores it; both are conclusions, assigned only after the observation has reached its own. The severity must match where the observation lands, not where it started. If the observation concludes that the concern does not hold ("withdrawn", "not a finding", "no issue"), emit no finding at all — drop it from the array rather than emitting it with any severity. +**Field order matters: reasoning first, verdict last.** The `observation` is your reasoning — work through what you noticed first. Then `category` names what the observation found, and `severity` scores it; both are conclusions, assigned only after the observation has reached its own. The severity must match where the observation lands, not where it started. If the observation concludes that the concern does not hold ("withdrawn", "not a finding", "no issue"), emit no finding at all — drop it from the array rather than emitting it with any severity. -All three fields are required on every finding. `severity` must be `high`, `medium`, or `low`. +All three fields are required on every finding — a finding without a `category` or without a `severity` is unparseable and fails the whole response. -Category is a short hyphenated tag (e.g. `type-mismatch`, `missing-ref`, `plan-coverage-ambiguity`, `ac-unaddressed`). +Category is a short hyphenated tag (e.g. `type-mismatch`, `missing-ref`, `plan-coverage-ambiguity`, `ac-unaddressed`). When you have nothing to report, return `{"findings": []}`. {{ template "json_string_rules" }} diff --git a/internal/llmops/shared_templates/json_response_rules.tmpl b/internal/llmops/shared_templates/json_response_rules.tmpl deleted file mode 100644 index 74922093..00000000 --- a/internal/llmops/shared_templates/json_response_rules.tmpl +++ /dev/null @@ -1,3 +0,0 @@ -{{ define "json_response_rules" -}} -Return exactly one JSON object with a required `findings` array. For no findings, return `{"findings":[]}`. Never omit `findings` or set it to `null`. Put all explanations and quoted source text inside the finding fields. Write no text outside the object and use no Markdown fences. -{{- end }} diff --git a/internal/llmops/shared_templates/json_string_rules.tmpl b/internal/llmops/shared_templates/json_string_rules.tmpl index 193032f6..095625d7 100644 --- a/internal/llmops/shared_templates/json_string_rules.tmpl +++ b/internal/llmops/shared_templates/json_string_rules.tmpl @@ -1,3 +1,3 @@ {{ define "json_string_rules" -}} -**JSON strings.** Every field within a finding has a string value. Use standard JSON escaping: write a literal `"` as `\"`, a backslash as `\\`, and a line break as `\n`. Never put a raw line break inside a value. Reproduce cited text exactly and escape it; do not substitute or drop characters to avoid escaping. When a prose value cites an excerpt, set the excerpt off with single quotes (`'like this'`) or backticks. +**JSON strings.** Every value is a JSON string with standard escaping: write a literal `"` as `\"`, a backslash as `\\`, and a line break as `\n`. Never put a raw line break inside a value. Reproduce cited text exactly and escape it; do not substitute or drop characters to avoid escaping. When a prose value cites an excerpt, set the excerpt off with single quotes (`'like this'`) or backticks. {{- end }} diff --git a/internal/llmops/writingguide_templates/writing_guide.tmpl b/internal/llmops/writingguide_templates/writing_guide.tmpl index 271f0784..56ac7b2e 100644 --- a/internal/llmops/writingguide_templates/writing_guide.tmpl +++ b/internal/llmops/writingguide_templates/writing_guide.tmpl @@ -38,25 +38,23 @@ Check only these axes: ## Output format -{{ template "json_response_rules" }} - -Example with one finding: +Respond with a single JSON object matching this schema — nothing before or after, no code fences: { "findings": [ { - "reasoning": "The draft refers to an earlier approach without identifying it, so a reader cannot tell what changed.", - "axis": "stranding", - "quote": "the earlier approach", - "repair": "write-in", - "severity": "substantive" + "reasoning": "one to three sentences working through what you noticed and why it fails the pull, ending in a conclusion", + "axis": "stranding" | "dilution" | "conflation" | "pointing" | "form", + "quote": "the draft's words at issue, verbatim", + "repair": "cut" | "write-in" | "split" | "point" | "reword", + "severity": "substantive" | "minor" } ] } -**Field order matters: reasoning first, verdict last.** In `reasoning`, work through what you noticed and why it fails the pull in one to three sentences, ending in a conclusion. Then name the axis, repair, and severity; they are conclusions, assigned only after the reasoning has reached its own. If the reasoning concludes the concern does not hold, emit no finding at all — drop it from the array rather than emitting it at any severity. +**Field order matters: reasoning first, verdict last.** The reasoning is your work — walk through what you noticed before naming the axis, repair, and severity; they are conclusions, assigned only after the reasoning has reached its own. If the reasoning concludes the concern does not hold, emit no finding at all — drop it from the array rather than emitting it at any severity. -All five fields are required on every finding. `axis` must be `stranding`, `dilution`, `conflation`, `pointing`, or `form`. `repair` must be `cut`, `write-in`, `split`, `point`, or `reword`. `severity` must be `substantive` or `minor`. `severity` is `substantive` when the drafting dialogue should take the finding up, `minor` when folding it in or ignoring it is fine. +All five fields are required on every finding. `severity` is `substantive` when the drafting dialogue should take the finding up, `minor` when folding it in or ignoring it is fine. {{ template "json_string_rules" }} @@ -64,7 +62,7 @@ All five fields are required on every finding. `axis` must be `stranding`, `dilu ## Calibration -No findings is a correct outcome. Do not stretch: a finding you would not defend is noise that costs more than it saves. When unsure whether something fails, it does not. Anything outside the five axes is out of scope. +No findings is a correct outcome — return {"findings": []} without hesitation. Do not stretch: a finding you would not defend is noise that costs more than it saves. When unsure whether something fails, it does not. Anything outside the five axes is out of scope. {{- end }} {{ define "writing_guide_user" -}}