diff --git a/.github/workflows/evals.yaml b/.github/workflows/evals.yaml new file mode 100644 index 000000000..5ddc87ff8 --- /dev/null +++ b/.github/workflows/evals.yaml @@ -0,0 +1,113 @@ +name: evals + +# Live evals of pre-flight, the writing guide and summarize through Opper, one +# path per model at its production setting, reporting each case's result and +# the cost (20261101-010604-d-tac-769, 20261102-101250-d-tac-s98). The job +# holds OPPER_API_KEY, so it runs only on manual dispatch or when a pull +# request from this repository gets the `eval` label: never on push, never for +# a fork. +on: + workflow_dispatch: + inputs: + runs: + description: Runs per pass-rate case (empty keeps each case's own count) + default: "" + pull_request: + types: [labeled] + +permissions: + contents: read + +concurrency: + group: evals-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + evals: + if: >- + github.event_name == 'workflow_dispatch' || + (github.event.label.name == 'eval' && + github.event.pull_request.head.repo.full_name == github.repository) + runs-on: ubuntu-latest + timeout-minutes: 90 + strategy: + fail-fast: false + matrix: + include: + # Opper's OpenAI-compatible route takes the full chat completions path. + # GLM is pinned to a route with structured output; the pooled id also + # reaches providers that list none. + - name: glm-5.3-flash + provider: openai + model: greenpt/glm-5.3-flash + params: reasoning_effort=low + endpoint: https://api.opper.ai/v3/compat/chat/completions + - name: gpt-5.6-luna + provider: openai + model: openai/gpt-5.6-luna + params: "" + endpoint: https://api.opper.ai/v3/compat/chat/completions + - name: claude-haiku-5-5 + provider: anthropic + model: anthropic/claude-haiku-5-5 + params: "" + endpoint: https://api.opper.ai/v3/compat + steps: + - uses: actions/checkout@v5 + - uses: actions/setup-go@v6 + with: + go-version-file: go.mod + - name: Write the eval config + env: + OPPER_API_KEY: ${{ secrets.OPPER_API_KEY }} + ENDPOINT: ${{ matrix.endpoint }} + run: | + test -n "$OPPER_API_KEY" || { echo "OPPER_API_KEY is not set"; exit 1; } + cat > "$RUNNER_TEMP/eval-config.yaml" <> "$GITHUB_STEP_SUMMARY" + - name: Keep the raw responses + if: always() + uses: actions/upload-artifact@v4 + with: + name: evals-${{ matrix.name }} + path: eval-out/ + if-no-files-found: warn diff --git a/.sdd/graph/2026/11/01-002900-s-tac-3q0.md b/.sdd/graph/2026/11/01-002900-s-tac-3q0.md new file mode 100644 index 000000000..aaa6a40e5 --- /dev/null +++ b/.sdd/graph/2026/11/01-002900-s-tac-3q0.md @@ -0,0 +1,30 @@ +--- +type: signal +layer: tactical +kind: gap +refs: + - id: 20261031-141038-d-tac-p8l + kind: surfaced-by + desc: its format evals surfaced the deviation; the plan counts Ollama among the providers that support structured output + - id: 20260901-093840-s-tac-ahc + kind: grounded-in + desc: the recommendation of glm-5.3-flash on Ollama Cloud for all three purposes, the expected side + - id: 20261004-135748-d-tac-ndh + kind: grounded-in + desc: the rule that an endpoint ignoring the schema fails loudly, and its revisit clause +participants: + - Christopher +confidence: high +topics: + - llm/providers + - reliability/pre-flight +summary: Ollama Cloud ignores a request's response schema for `glm-5.3-flash:cloud`, returning prose on every route probed, so pre-flight and the writing guide fail to parse on the configuration the provider evaluation (20260901-093840-s-tac-ahc) recommends; Ollama tracks it as ollama/ollama#12362. The structured-output plan's format evals (20261031-141038-d-tac-p8l) surfaced the deviation, the native-enforcement requirement (20261004-135748-d-tac-ndh) stays in force, and through Opper on an EU route the same model returned parseable responses in every run. +--- + +Ollama Cloud ignores a request's response schema, so pre-flight and the writing guide, which now send a schema instead of describing JSON in the prompt, fail to parse on `glm-5.3-flash:cloud`, the configuration the provider evaluation recommends. + +**Actual.** On 2026-11-01 `https://ollama.com` returned prose for `glm-5.3-flash:cloud` on `/api/generate` and `/api/chat` with `format` and on `/v1/chat/completions` with a strict `response_format`, whatever the thinking setting; a local Ollama daemon returned schema-valid JSON for the same request. On this model the format evals of the structured-output plan (20261031-141038-d-tac-p8l) returned mostly prose, where nearly every response parsed before the change ([probe record](./01-002900-s-tac-3q0/probe-record.md)). Ollama tracks the behavior as [ollama/ollama#12362](https://github.com/ollama/ollama/issues/12362), "JSON reply schema is ignored by Cloud model", open since 2025-09-21. + +**Expected.** The plan counts Ollama among the providers that support structured output, and the provider evaluation (20260901-093840-s-tac-ahc) recommends `glm-5.3-flash` at `think=low` on Ollama Cloud for all three purposes. + +Christopher kept the native-enforcement requirement (20261004-135748-d-tac-ndh) rather than revisiting it. Remedy for this model: through Opper on an EU route (`greenpt/glm-5.3-flash`, `reasoning_effort: low`) the same evals returned parseable responses in every run on 2026-11-01. diff --git a/.sdd/graph/2026/11/01-002900-s-tac-3q0/probe-record.md b/.sdd/graph/2026/11/01-002900-s-tac-3q0/probe-record.md new file mode 100644 index 000000000..0ddb2e0fe --- /dev/null +++ b/.sdd/graph/2026/11/01-002900-s-tac-3q0/probe-record.md @@ -0,0 +1,26 @@ +# Ollama Cloud schema probe, 2026-11-01 + +Request body, sent to `https://ollama.com` with the account's bearer key (cloud) and to `http://localhost:11434` (local daemon): + +- schema: `{"type":"object","properties":{"greeting":{"type":"string"},"language":{"type":"string","enum":["de","en"]}},"required":["greeting","language"],"additionalProperties":false}` +- prompt: "Write two sentences about the weather in Hamburg, in German, as plain prose." (a prompt that pulls away from JSON, so only enforcement yields JSON) +- `stream: false` + +| Endpoint | Route | Schema field | Thinking | Model | Result | +|---|---|---|---|---|---| +| ollama.com | `/api/generate` | `format` | unset | glm-5.3-flash:cloud | prose | +| ollama.com | `/api/generate` | `format` | `low` | glm-5.3-flash:cloud | prose | +| ollama.com | `/api/generate` | `format` | `false` | glm-5.3-flash:cloud | prose (leaked reasoning) | +| ollama.com | `/api/chat` (system + user) | `format` | unset | glm-5.3-flash:cloud | prose | +| ollama.com | `/api/chat` (system + user) | `format` | `low` | glm-5.3-flash:cloud | prose | +| ollama.com | `/v1/chat/completions` | strict `response_format` | unset | glm-5.3-flash:cloud | prose | +| localhost | `/api/generate` | `format` | unset | gemma4:26b | schema-valid JSON | +| localhost | `/api/generate` | `format` | `low` | gemma4:26b | schema-valid JSON | +| localhost | `/api/generate` | `format` | `false` | gemma4:26b | schema-valid JSON | + +Format evals (`TestWritingGuideEval_Format`, `TestPreflightEval_Format`, 3 runs per specimen and operation, 60 runs) on `ollama glm-5.3-flash:cloud think=low`: + +- before the structured-output change (prompts ask for JSON, no schema sent): 58 of 60 parseable; both failures were a second JSON object after leaked reasoning +- after it (schema sent as `format`, no JSON instructions): 5 of 60 parseable; the rest were Markdown prose, some opening with "Findings: []" + +For comparison after the change: claude-sonnet-4-6 (Anthropic API) 60 of 60, gpt-5-mini (OpenAI API) 60 of 60, claude CLI sonnet 60 of 60. diff --git a/.sdd/graph/2026/11/01-010604-d-tac-769.md b/.sdd/graph/2026/11/01-010604-d-tac-769.md new file mode 100644 index 000000000..feaaa389e --- /dev/null +++ b/.sdd/graph/2026/11/01-010604-d-tac-769.md @@ -0,0 +1,29 @@ +--- +type: decision +layer: tactical +kind: directive +refs: + - id: 20261031-141038-d-tac-p8l + kind: refines + desc: sets the models its GitHub Actions workflow runs and keeps it to model coverage + - id: 20261031-174216-d-tac-p83 + kind: related + desc: the eval workflow is the one sdd eval later takes over +participants: + - Christopher +confidence: high +intent: pending +topics: + - dev/ci + - llm/evaluation + - reliability/testing +summary: 'The structured-output plan''s GitHub Actions eval workflow runs three models through Opper, one path per model at production settings: `glm-5.3-flash` at `reasoning_effort: low`, and `openai/gpt-5.6-luna` and `anthropic/claude-haiku-5-5` at their default `medium` effort, reporting schema validity and the cost per model and setting; Claude Sonnet 4.6 is left out for poor cost-performance. It refines the plan (20261031-141038-d-tac-p8l) with the models it left open, pins GLM to an Opper route with structured output, defers lower-effort comparisons to the judge of `sdd eval`, which takes the workflow over (20261031-174216-d-tac-p83), and sets aside a provider smoke workflow, since checks through Opper say little about the vendors'' own APIs and the matrix already runs both adapters.' +--- + +The structured-output plan's GitHub Actions workflow evaluates three models through Opper, one path per model, each at the setting it would run with in production: `glm-5.3-flash` at `reasoning_effort: low` and `openai/gpt-5.6-luna` at its default effort (`medium`) on the OpenAI-compatible route, and `anthropic/claude-haiku-5-5` at its default effort (`medium`) on the Messages route. Each run reports schema validity and the cost per model and setting. This refines the plan (20261031-141038-d-tac-p8l), which named Opper's two routes but no models; the workflow is the one `sdd eval` later takes over (20261031-174216-d-tac-p83). + +All models run through Opper, so evaluation needs one key and one bill. GLM is pinned to an Opper route that supports structured output, because the pooled `glm-5.3-flash` also routes to providers that list none, and a format failure there would not be SDD's. Claude Sonnet 4.6 is left out, performing too poorly for its cost (Christopher). Data residency plays no role, since the eval material is anonymized or public. + +Lower efforts are not compared yet: effort levels do not match across model families, and whether quality holds at a cheaper setting needs the judge `sdd eval` brings. Each combination is recorded by its variant, so cost and latency already separate per setting. The cost is the one Opper reports with every response. + +A separate provider smoke workflow, one job per adapter, was set aside. Through Opper it checks the adapters against a translation layer that can be more tolerant than the vendors' own APIs: claude-haiku-5-5 accepted a temperature through Opper that Anthropic documents as rejected for that model. Covering the vendors and Ollama would take more keys and a runner with a local model. And the matrix already runs both adapters, so a broken one shows as every run of its model failing. Request shapes stay covered by the deterministic wire tests; live checks against the vendors' APIs and Ollama stay local runs. diff --git a/.sdd/graph/2026/11/01-110510-s-tac-buz.md b/.sdd/graph/2026/11/01-110510-s-tac-buz.md new file mode 100644 index 000000000..baaa1713c --- /dev/null +++ b/.sdd/graph/2026/11/01-110510-s-tac-buz.md @@ -0,0 +1,31 @@ +--- +type: signal +layer: tactical +kind: done +refs: + - id: 20261031-141038-d-tac-p8l + kind: addresses + desc: 'delivers its criteria except the merge and closing #20 and #21, so it stays open' + - id: 20261101-010604-d-tac-769 + kind: addresses + desc: carries out its model matrix and cost report + - id: 20261101-002900-s-tac-3q0 + kind: surfaces + desc: the live evals found Ollama Cloud ignoring response schemas +participants: + - Christopher +confidence: high +topics: + - llm/providers + - reliability/testing + - dev/ci +summary: 'Structured output for pre-flight and the writing guide is built on branch `structured-llm-responses` and open as unmerged PR #26: both checks send a JSON schema each adapter enforces natively and the prompts no longer teach JSON; after the change the format evals returned valid JSON on the Anthropic API, the OpenAI API, the Claude CLI and three models through Opper, and a pre-flight calibration run scored unchanged. It delivers the plan''s criteria (20261031-141038-d-tac-p8l) except the merge and closing #20/#21, carries out the model-matrix and cost-report directive (20261101-010604-d-tac-769), and surfaced that Ollama Cloud ignores response schemas (20261101-002900-s-tac-3q0).' +--- + +Structured output for pre-flight and the writing guide is built on branch `structured-llm-responses` and open as PR #26, unmerged: both checks send a JSON schema that each adapter enforces natively, and the prompts no longer teach JSON. This covers the structured-output plan (20261031-141038-d-tac-p8l) except its closing steps, the merge and closing GitHub #20 and #21, so the plan stays open. + +**Delivered.** In sdd: `4fb9af75` format evals over a hostile-character example set in data files, with #21's cases anonymized; `55186d3f` `ResponseSchema` on `pkg/llm.Request` and the adapters (Claude CLI `--json-schema`; strict `response_format`, `output_config.format` and `format` through the gollm fork; `llm.endpoint` for the anthropic provider); `62cec3f9` the schemas as ordered JSON files, prompts without JSON construction rules, quotes that only locate their passage; `dca0a76b` capture's instruction to locate the passage from a quote; `0421a997` and `d332ed4b` the format-evals workflow; `b12440f6` and `58607295` the cost Opper reports, in usage and eval results. In networkteam/gollm `generate-with-usage`, pinned at `698361b`: `14f7165` the configurable Anthropic base URL, `c460d23` the system prompt sent once and per call, `30e7a78` OpenAI parameters without model-name matching, `25ba06d` the schema path on the regular request path, `698361b` the reported cost. + +**Checked.** Before the change the format evals failed on the Anthropic API and on Ollama Cloud. After it every run returned valid JSON on the Anthropic API, the OpenAI API, the Claude CLI and `greenpt/glm-5.3-flash` through Opper, and local Ollama returned no invalid response. The workflow's first run on PR #26 returned valid JSON in every run for glm-5.3-flash, gpt-5.6-luna and claude-haiku-5-5, and reported the cost per model. A paired run of the pre-flight calibration suite on glm through Opper scored the same before and after. + +**Along the way.** Reading the fork turned up two defects: it sent the system prompt twice on OpenAI and Anthropic, and overlapping calls could send each other's system prompt. Both were reproduced by failing tests and fixed on all request paths (Christopher). The Claude CLI keeps its single stdin payload, so it has no prompt split to keep. Ollama Cloud ignores response schemas (20261101-002900-s-tac-3q0). The workflow's models and cost report were settled in 20261101-010604-d-tac-769, which this work carries out. diff --git a/.sdd/graph/2026/11/01-112131-s-tac-nk8.md b/.sdd/graph/2026/11/01-112131-s-tac-nk8.md new file mode 100644 index 000000000..baff8f4a5 --- /dev/null +++ b/.sdd/graph/2026/11/01-112131-s-tac-nk8.md @@ -0,0 +1,38 @@ +--- +type: signal +layer: tactical +kind: gap +refs: + - id: 20261101-110510-s-tac-buz + kind: surfaced-by + desc: holds the actual side + - id: 20261031-141038-d-tac-p8l + kind: related + desc: asks for validity only and leaves content checks to sdd eval + - id: 20261101-010604-d-tac-769 + kind: related + desc: sets the models; recorded the narrow reading + - id: 20261031-174216-d-tac-p83 + kind: grounded-in + desc: replaces these tests, so the fix stays simple + - id: 20260919-144857-s-prc-a1x + kind: related + desc: the narrowing pattern this run's part fits +participants: + - Christopher +confidence: high +topics: + - llm/evaluation + - reliability/testing + - dev/ci + - reliability/discipline +summary: The live evals of the structured-output run only assert that responses parse against the schema, and the GitHub Actions workflow runs nothing else, so whether findings and quotes are right goes unchecked. Christopher wants a few specimens with problem characters in the pre-flight and writing-guide eval tests, asserting expected findings and passage-locating quotes, and the workflow to run the pre-flight, writing-guide and summarize eval tests for the models of 20261101-010604-d-tac-769, kept simple since `sdd eval` replaces these tests (20261031-174216-d-tac-p83). The plan (20261031-141038-d-tac-p8l) asked for validity only and left content judgment to `sdd eval`; the run built to it, and answering Christopher's objection with the format evals again fits the narrowing pattern (20260919-144857-s-prc-a1x). The miss surfaced at the done's playback (20261101-110510-s-tac-buz). +--- + +The structured-output run's live evals only check that responses parse, and its GitHub Actions workflow runs nothing else; Christopher wants a few specimens in the existing eval tests that check content, run in the workflow for each decided model. + +**Actual.** The format evals (`4fb9af75`) make about 60 calls per model and assert only that responses parse against the schema; the workflow (`d332ed4b`) runs only them. With the schema enforced by the provider, that repeats a guarantee and says nothing about whether findings and quotes are right. + +**Expected** (Christopher, 2026-11-01). A few specimens with problem characters in the pre-flight and writing-guide eval tests, asserting the expected findings and quotes that locate their passage; the workflow runs the pre-flight, writing-guide and summarize eval tests for the models of 20261101-010604-d-tac-769. Kept simple, since `sdd eval` replaces these tests (20261031-174216-d-tac-p83). + +**How it was missed.** The plan (20261031-141038-d-tac-p8l) asks for validity only and leaves "judgment tests, whether findings hold and quotes point to the right passage" to `sdd eval`, a split the planning agent proposed and Christopher confirmed. The run built to the plan. When Christopher objected that he expected the eval tests to run per model, the run answered with the format evals again and recorded 20261101-010604-d-tac-769 that way; this part fits the narrowing pattern of 20260919-144857-s-prc-a1x. The miss surfaced at the done's playback (20261101-110510-s-tac-buz). diff --git a/.sdd/graph/2026/11/02-101250-d-tac-s98.md b/.sdd/graph/2026/11/02-101250-d-tac-s98.md new file mode 100644 index 000000000..fa40c9109 --- /dev/null +++ b/.sdd/graph/2026/11/02-101250-d-tac-s98.md @@ -0,0 +1,35 @@ +--- +type: decision +layer: tactical +kind: directive +refs: + - id: 20261031-141038-d-tac-p8l + kind: refines + desc: its live evals also check content, and its workflow runs the eval tests + - id: 20261101-010604-d-tac-769 + kind: refines + desc: the workflow runs the eval tests instead of validity checks only + - id: 20261031-174216-d-tac-p83 + kind: grounded-in + desc: percentages and thresholds wait for sdd eval +closes: + - 20261101-112131-s-tac-nk8 +participants: + - Christopher +confidence: high +intent: pending +topics: + - llm/evaluation + - reliability/testing + - dev/ci +summary: 'This decision commits the structured-output plan''s live evals to checking content, not just schema validity: specimens with problem characters join the pre-flight and writing-guide eval tests, asserting expected findings and quotes that locate their passage, and the GitHub Actions workflow runs the pre-flight, writing-guide and summarize eval tests per model. It refines the plan (20261031-141038-d-tac-p8l) and the workflow directive (20261101-010604-d-tac-769), which both earlier covered validity checks only, and defers percentages and thresholds to `sdd eval` (20261031-174216-d-tac-p83). It closes the content-check gap signal (20261101-112131-s-tac-nk8).' +--- + +The structured-output plan's live evals check content, not only validity: a few specimens with problem characters join the pre-flight and writing-guide eval tests and assert the expected findings and quotes that locate their passage, and the GitHub Actions workflow runs the pre-flight, writing-guide and summarize eval tests for each model of 20261101-010604-d-tac-769. + +- Specimens: the German draft from GitHub #21 with „…“ and a planted defect, for the writing guide; the two past incidents from the plan's example set, for pre-flight, one of them carrying braces, a backslash path and a regex. They stay data files with the expected outcome added. The eight synthetic variants of that draft and the validity-only format tests are dropped as not useful (Christopher). +- A quote locates its passage when its parts, split at "…", appear in the draft in order once quote marks and whitespace are normalized; a deterministic unit test covers the check. +- Summarize's judge runs through Opper, its endpoint set by `SDD_EVAL_JUDGE_ENDPOINT` beside `SDD_EVAL_JUDGE_PROVIDER`, `_MODEL` and `_PARAMS`. +- The job summary lists each case's result and the cost per model; raw responses are still uploaded. A failing case turns the job red without blocking a merge; percentages and thresholds come with `sdd eval` (20261031-174216-d-tac-p83). + +This refines the plan (20261031-141038-d-tac-p8l), which asked for validity only and left content checks to `sdd eval`, and 20261101-010604-d-tac-769, whose workflow ran validity checks only. It closes 20261101-112131-s-tac-nk8 by committing to the content checks that gap expects. diff --git a/AGENTS.md b/AGENTS.md index 430bec19f..6d51b6975 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -25,6 +25,7 @@ The `sdd` binary lives at `./bin/sdd` (gitignored — rebuild locally with `devb - `devbox run test` — run all tests: the root module and the separate `examples/extendingsdd` module. Never `go test ./...` alone — `./...` prunes the nested example module (a `go.work` does not change that), so its drift surfaces only in CI. - `UPDATE_SNAPSHOTS=true go test ` — regenerate the readable snapshots a package keeps under `.snapshots/` (cupaloy). Read the resulting git diff as the review of what the response changed; a snapshot test never stands alone but sits behind a typed assertion that names the intent (20260921-190614-d-cpt-t8i). - `go test -tags=eval -run TestPreflightEval ./internal/llmops/...` — pre-flight prompt calibration eval (live `claude` CLI, slow + paid; model via `SDD_EVAL_MODEL`, default `sonnet`). Capture full output to a file and grep the file — `… -v 2>&1 | tee /tmp/eval.log` — never filter the live stream, or a failure shows no findings and forces a costly re-run. +- `go test -tags=eval -run 'Eval_Specimens' -parallel 4 ./internal/llmops/...` — the specimens in `internal/llmops/testdata/eval/specimens`: drafts with problem characters whose expected finding must fire with a quote that locates its passage (live, paid; provider via `SDD_EVAL_PROVIDER`/`SDD_EVAL_MODEL`/`SDD_EVAL_CONFIG`, summarize's judge via `SDD_EVAL_JUDGE_*`). Every eval call appends raw to `$SDD_EVAL_RESPONSES_DIR/responses.jsonl` for later scoring. The `evals` workflow runs the pre-flight, writing-guide and summarize evals against Opper on manual dispatch or the `eval` label on a pull request from this repository. - `go fmt ./...` — format code - `devbox run lint` — lint (must be clean; CI enforces). Convention findings print as warnings without failing the run — see `scripts/lint.sh` for the mechanism. Use the wrapper, not bare `golangci-lint run`, which fails on warnings too. - `devbox run validate-skills` — validate the rendered Codex skills under `.agents/skills/` against the Agent Skills standard (`uvx skills-ref@0.1.1 agentskills validate`, managed via the devbox `uv` package). Requires a Codex render present (i.e. `codex` in `supported_agents`). diff --git a/go.mod b/go.mod index c4e4e3b2a..fdd253472 100644 --- a/go.mod +++ b/go.mod @@ -92,4 +92,4 @@ require ( // source and honor replace; only `go install pkg@version` (which sdd does not // use) would break. Remove this directive once the change lands upstream and // bump the require above to the released teilomillet/gollm version. -replace github.com/teilomillet/gollm => github.com/networkteam/gollm v0.0.0-20260919121334-aa9e2d1faebc +replace github.com/teilomillet/gollm => github.com/networkteam/gollm v0.0.0-20261101000732-698361b13832 diff --git a/go.sum b/go.sum index 95ef0c472..b9739bd8a 100644 --- a/go.sum +++ b/go.sum @@ -107,8 +107,8 @@ github.com/modelcontextprotocol/go-sdk v1.7.0 h1:yqjY2dsbKAC0LSuWZVBMrHgiG8ukXv6 github.com/modelcontextprotocol/go-sdk v1.7.0/go.mod h1:dL7u98E/zjJTGzEq+j30jQ8K2k1mb6LeAH4inEcSGts= github.com/muesli/cancelreader v0.2.2 h1:3I4Kt4BQjOR54NavqnDogx/MIoWBFa0StPA8ELUXHmA= github.com/muesli/cancelreader v0.2.2/go.mod h1:3XuTXfFS2VjM+HTLZY9Ak0l6eUKfijIfMUZ4EgX0QYo= -github.com/networkteam/gollm v0.0.0-20260919121334-aa9e2d1faebc h1:3SID2v1Qy85u5mchX+wlNUctJLa5NjRWMfj8HkeXcAo= -github.com/networkteam/gollm v0.0.0-20260919121334-aa9e2d1faebc/go.mod h1:ba4KWPPrYoc7PbPkcjTD6uMi2s5yXVWVsE5uzP+mvCg= +github.com/networkteam/gollm v0.0.0-20261101000732-698361b13832 h1:b85TIL8Kephr2WwAJjcEd9TkOHE5dunm3IbiEHQAri0= +github.com/networkteam/gollm v0.0.0-20261101000732-698361b13832/go.mod h1:ba4KWPPrYoc7PbPkcjTD6uMi2s5yXVWVsE5uzP+mvCg= github.com/networkteam/slogutils v0.4.0 h1:tf6Gf//H/a3EUmNszWEw8fhQkscrDdKt7HEdb2wBUhM= github.com/networkteam/slogutils v0.4.0/go.mod h1:j6siLHGfAkUpfhNUXw43EUdZv/HBaGd/RW7ToPwiJZo= github.com/philippgille/chromem-go v0.7.0 h1:4jfvfyKymjKNfGxBUhHUcj1kp7B17NL/I1P+vGh1RvY= diff --git a/internal/baseprocedures/entries/20260703-094500-d-prc-cap.md b/internal/baseprocedures/entries/20260703-094500-d-prc-cap.md index f1d39431f..ee798486f 100644 --- a/internal/baseprocedures/entries/20260703-094500-d-prc-cap.md +++ b/internal/baseprocedures/entries/20260703-094500-d-prc-cap.md @@ -227,6 +227,8 @@ The writing guide read the draft as a stranger to this dialogue — deliberately {{range .guideFindings}}- [{{.severity}}] {{.axis}} on '{{.quote}}' — {{.reasoning}} (repair: {{.repair}}) {{end}} +Each quote points to a passage in the draft you hold: locate the passage from it instead of matching it, since the guide may shorten the passage or change characters such as quotation marks. + A `substantive` finding you act on — or dismiss — is named to the user in your own words when the draft is played back; a `minor` finding is yours to fold or ignore silently. The guide runs once per capture: it judged the draft, and it will not re-judge your repairs — a second pass reviews repairs instead of the draft and drifts toward escalation. Choose **revise** to fold findings in and continue to playback, or **proceed** to play the draft back as it stands. Only when you have reworked the draft so substantially that it is effectively a new one is **recheck** the right choice — it discards this run and judges the new draft fresh. ## unit: playback diff --git a/internal/llm/claude/claude.go b/internal/llm/claude/claude.go index d72752d56..e286c56c0 100644 --- a/internal/llm/claude/claude.go +++ b/internal/llm/claude/claude.go @@ -32,9 +32,14 @@ func (r *Runner) identity() llm.Identity { // Run executes claude -p --output-format json and parses the JSON response. // The claude CLI accepts a single stdin payload, so SystemPrompt and // UserPrompt are concatenated — prompt caching is not available through this -// transport. +// transport. A ResponseSchema goes to --json-schema, and the structured output +// the CLI validated against it becomes the text. func (r *Runner) Run(ctx context.Context, req llm.Request) (llm.Result, error) { - cmd := exec.CommandContext(ctx, "claude", "-p", "--model", r.model, "--output-format", "json") + args := []string{"-p", "--model", r.model, "--output-format", "json"} + if len(req.ResponseSchema) > 0 { + args = append(args, "--json-schema", string(req.ResponseSchema)) + } + cmd := exec.CommandContext(ctx, "claude", args...) cmd.Stdin = strings.NewReader(req.Combined()) out, err := cmd.Output() if err != nil { @@ -50,6 +55,13 @@ func (r *Runner) Run(ctx context.Context, req llm.Request) (llm.Result, error) { if err := json.Unmarshal(out, &resp); err != nil { return llm.Result{}, &llm.Error{Identity: r.identity(), Err: fmt.Errorf("parsing claude JSON response: %w", err)} } + text := resp.Result + if len(req.ResponseSchema) > 0 { + if len(resp.StructuredOutput) == 0 || string(resp.StructuredOutput) == "null" { + return llm.Result{}, &llm.Error{Identity: r.identity(), Err: fmt.Errorf("claude -p returned no structured output for the response schema")} + } + text = string(resp.StructuredOutput) + } // The CLI is the one transport reporting per-model usage (a call may span // e.g. a haiku sub-model); the common Usage carries the aggregate, so the @@ -69,7 +81,7 @@ func (r *Runner) Run(ctx context.Context, req llm.Request) (llm.Result, error) { } return llm.Result{ - Text: resp.Result, + Text: text, Identity: r.identity(), Usage: llm.Usage{ InputTokens: resp.Usage.InputTokens, @@ -83,14 +95,15 @@ func (r *Runner) Run(ctx context.Context, req llm.Request) (llm.Result, error) { // claudeResponse maps the JSON output of claude -p --output-format json. type claudeResponse struct { - Result string `json:"result"` - TotalCostUSD float64 `json:"total_cost_usd"` - DurationMs int64 `json:"duration_ms"` - DurationAPIMs int64 `json:"duration_api_ms"` - NumTurns int `json:"num_turns"` - IsError bool `json:"is_error"` - Usage claudeUsage `json:"usage"` - ModelUsage map[string]claudeModelUsage `json:"modelUsage"` + Result string `json:"result"` + StructuredOutput json.RawMessage `json:"structured_output"` + TotalCostUSD float64 `json:"total_cost_usd"` + DurationMs int64 `json:"duration_ms"` + DurationAPIMs int64 `json:"duration_api_ms"` + NumTurns int `json:"num_turns"` + IsError bool `json:"is_error"` + Usage claudeUsage `json:"usage"` + ModelUsage map[string]claudeModelUsage `json:"modelUsage"` } type claudeUsage struct { diff --git a/internal/llm/claude/claude_test.go b/internal/llm/claude/claude_test.go new file mode 100644 index 000000000..9ac5d845e --- /dev/null +++ b/internal/llm/claude/claude_test.go @@ -0,0 +1,134 @@ +package claude_test + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "slices" + "strings" + "testing" + + "github.com/networkteam/sdd/internal/llm/claude" + "github.com/networkteam/sdd/pkg/llm" +) + +// problemChars are the characters that broke the checks' JSON output before. +const problemChars = "„Code einlösen“ «zitiert» ‚einfach‘ “English” an ASCII \" quote, Options{Zebra: true}, C:\\Projekte\\de.json, ^Code\\s+[A-Z0-9]{8}$, a line\nbreak, & 🎉" + +const wireSchema = `{"type":"object","properties":{"reasoning":{"type":"string"},"axis":{"type":"string","enum":["form"]}},"required":["reasoning","axis"],"additionalProperties":false}` + +// fakeCLI puts a claude executable on PATH that records its arguments and +// stdin into dir and prints response as the CLI's JSON output. +func fakeCLI(t *testing.T, response string) string { + t.Helper() + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "response.json"), []byte(response), 0o644); err != nil { + t.Fatal(err) + } + script := "#!/bin/sh\n" + + "for a in \"$@\"; do printf '%s\\0' \"$a\"; done > '" + dir + "/args'\n" + + "cat > '" + dir + "/stdin'\n" + + "cat '" + dir + "/response.json'\n" + if err := os.WriteFile(filepath.Join(dir, "claude"), []byte(script), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + return dir +} + +func recordedArgs(t *testing.T, dir string) []string { + t.Helper() + raw, err := os.ReadFile(filepath.Join(dir, "args")) + if err != nil { + t.Fatal(err) + } + return strings.Split(strings.TrimSuffix(string(raw), "\x00"), "\x00") +} + +// A structured request hands the schema to --json-schema byte for byte and +// returns the structured output the CLI validated, problem characters +// unchanged both ways; the prompt goes to stdin and usage is reported. +func TestRunWithResponseSchema(t *testing.T) { + answer := `{"reasoning":` + mustJSON(problemChars) + `,"axis":"form"}` + dir := fakeCLI(t, `{"type":"result","result":"ignored when a schema is set","structured_output":`+answer+ + `,"total_cost_usd":0.25,"usage":{"input_tokens":11,"output_tokens":22,"cache_read_input_tokens":3,"cache_creation_input_tokens":4}}`) + + req := llm.Request{ + SystemPrompt: "SYSTEM " + problemChars, + UserPrompt: "USER " + problemChars, + ResponseSchema: json.RawMessage(wireSchema), + } + res, err := claude.NewRunner("sonnet").Run(context.Background(), req) + if err != nil { + t.Fatalf("Run: %v", err) + } + + args := recordedArgs(t, dir) + i := slices.Index(args, "--json-schema") + if i < 0 || i+1 >= len(args) || args[i+1] != wireSchema { + t.Errorf("--json-schema not passed byte for byte: %q", args) + } + stdin, err := os.ReadFile(filepath.Join(dir, "stdin")) + if err != nil { + t.Fatal(err) + } + if string(stdin) != req.Combined() { + t.Errorf("stdin = %q, want the combined prompt", stdin) + } + if !jsonEqual(t, []byte(res.Text), []byte(answer)) { + t.Errorf("text = %q, want the structured output %q", res.Text, answer) + } + var decoded struct{ Reasoning string } + if err := json.Unmarshal([]byte(res.Text), &decoded); err != nil || decoded.Reasoning != problemChars { + t.Errorf("problem characters changed on the way back: %q (%v)", decoded.Reasoning, err) + } + want := llm.Usage{InputTokens: 11, OutputTokens: 22, CacheReadTokens: 3, CacheCreateTokens: 4, CostUSD: 0.25} + if res.Usage != want { + t.Errorf("usage = %+v, want %+v", res.Usage, want) + } +} + +// A CLI answer without structured output fails loudly rather than handing on +// the free-text result. +func TestRunWithResponseSchemaRequiresStructuredOutput(t *testing.T) { + fakeCLI(t, `{"type":"result","result":"{\"reasoning\":\"r\"}","usage":{}}`) + _, err := claude.NewRunner("sonnet").Run(context.Background(), llm.Request{UserPrompt: "U", ResponseSchema: json.RawMessage(wireSchema)}) + if err == nil || !strings.Contains(err.Error(), "no structured output") { + t.Fatalf("want a missing structured output error, got %v", err) + } +} + +// A free-text request passes no schema and returns the CLI's result. +func TestRunWithoutResponseSchema(t *testing.T) { + dir := fakeCLI(t, `{"type":"result","result":"SERVED","usage":{}}`) + res, err := claude.NewRunner("sonnet").Run(context.Background(), llm.Request{UserPrompt: "U"}) + if err != nil { + t.Fatalf("Run: %v", err) + } + if slices.Contains(recordedArgs(t, dir), "--json-schema") { + t.Error("free-text request passed --json-schema") + } + if res.Text != "SERVED" { + t.Errorf("text = %q, want SERVED", res.Text) + } +} + +func mustJSON(s string) string { + b, _ := json.Marshal(s) + return string(b) +} + +func jsonEqual(t *testing.T, a, b []byte) bool { + t.Helper() + var x, y any + if err := json.Unmarshal(a, &x); err != nil { + t.Fatalf("decoding %s: %v", a, err) + } + if err := json.Unmarshal(b, &y); err != nil { + t.Fatalf("decoding %s: %v", b, err) + } + ax, _ := json.Marshal(x) + by, _ := json.Marshal(y) + return string(ax) == string(by) +} diff --git a/internal/llm/gollm/gollm.go b/internal/llm/gollm/gollm.go index 61c4fbb1e..3873cb3f6 100644 --- a/internal/llm/gollm/gollm.go +++ b/internal/llm/gollm/gollm.go @@ -12,6 +12,7 @@ import ( "time" upstream "github.com/teilomillet/gollm" + upstreamllm "github.com/teilomillet/gollm/llm" "github.com/networkteam/sdd/internal/model" "github.com/networkteam/sdd/pkg/llm" @@ -87,8 +88,13 @@ func NewRunner(cfg model.LLMConfig) (*Runner, error) { } } - if cfg.Provider == "openai" && cfg.Endpoint != "" { - opts = append(opts, upstream.SetOpenAIEndpoint(cfg.Endpoint)) + if cfg.Endpoint != "" { + switch cfg.Provider { + case "openai": + opts = append(opts, upstream.SetOpenAIEndpoint(cfg.Endpoint)) + case "anthropic": + opts = append(opts, upstream.SetAnthropicEndpoint(cfg.Endpoint)) + } } // Enable Anthropic prompt caching — sends the anthropic-beta header so @@ -169,8 +175,9 @@ func (r *Runner) identity() llm.Identity { // Run sends the request to the underlying gollm client. When caching is // enabled (Anthropic), the SystemPrompt is tagged ephemeral so the server // caches the prefix. Other providers ignore the cache hint but still see -// the split system/user message pair. Failures carry the identity through the -// typed llm.Error so they measure like successes. +// the split system/user message pair. A ResponseSchema goes to the provider's +// native structured output. Failures carry the identity through the typed +// llm.Error so they measure like successes. func (r *Runner) Run(ctx context.Context, req llm.Request) (llm.Result, error) { var opts []upstream.PromptOption if req.SystemPrompt != "" { @@ -183,7 +190,12 @@ func (r *Runner) Run(ctx context.Context, req llm.Request) (llm.Result, error) { prompt := upstream.NewPrompt(req.UserPrompt, opts...) - resp, err := r.client.GenerateWithUsage(ctx, prompt) + var genOpts []upstreamllm.GenerateOption + if len(req.ResponseSchema) > 0 { + genOpts = append(genOpts, upstream.WithResponseSchema(req.ResponseSchema)) + } + + resp, err := r.client.GenerateWithUsage(ctx, prompt, genOpts...) if err != nil { if ctx.Err() == context.DeadlineExceeded { err = fmt.Errorf("gollm %s: timed out", r.provider) @@ -219,6 +231,7 @@ func usageFromResponse(u *upstream.Usage) llm.Usage { OutputTokens: out, CacheReadTokens: u.CacheReadInputTokens, CacheCreateTokens: u.CacheCreationInputTokens, + CostUSD: u.CostUSD, } } diff --git a/internal/llm/gollm/gollm_test.go b/internal/llm/gollm/gollm_test.go index 82de63e84..6fc28d051 100644 --- a/internal/llm/gollm/gollm_test.go +++ b/internal/llm/gollm/gollm_test.go @@ -99,6 +99,12 @@ func TestUsageFromResponse(t *testing.T) { if u2.InputTokens != 100 || u2.OutputTokens != 20 { t.Errorf("openai fallback wrong: %+v", u2) } + + // A cost the provider reports (Opper) passes through as reported. + u3 := usageFromResponse(&upstream.Usage{PromptTokens: 18, CompletionTokens: 21, CostUSD: 1.25e-05}) + if u3.CostUSD != 1.25e-05 { + t.Errorf("reported cost not mapped: %+v", u3) + } } func TestCanonicalVariant(t *testing.T) { diff --git a/internal/llm/gollm/wire_test.go b/internal/llm/gollm/wire_test.go index 06b4dc790..3cf5a92d4 100644 --- a/internal/llm/gollm/wire_test.go +++ b/internal/llm/gollm/wire_test.go @@ -1,6 +1,7 @@ package gollm_test import ( + "bytes" "context" "encoding/json" "io" @@ -16,127 +17,257 @@ import ( gollmrunner "github.com/networkteam/sdd/internal/llm/gollm" ) -// What actually reaches the provider is not inferable from the adapter code — -// gollm copies its option map into the request body verbatim, so a key we set -// may land under a name the provider ignores. This drives a real request at a -// local listener and asserts on the bytes. -func TestOllamaRequestWire(t *testing.T) { - var mu sync.Mutex - var body map[string]any - - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path != "/api/generate" { - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(`{"models":[]}`)) - return - } - raw, _ := io.ReadAll(r.Body) - mu.Lock() - _ = json.Unmarshal(raw, &body) - mu.Unlock() - _, _ = w.Write([]byte(`{"model":"m","response":"{}","done":true,"prompt_eval_count":11,"eval_count":22}`)) - })) - defer srv.Close() - - runner, err := gollmrunner.NewRunner(model.LLMConfig{ - Provider: "ollama", Model: "m", OllamaEndpoint: srv.URL, - Params: map[string]string{"think": "high"}, - }) - if err != nil { - t.Fatalf("NewRunner: %v", err) - } - res, err := runner.Run(context.Background(), llm.Request{ - SystemPrompt: "SYSTEM-BLOCK-MARKER", - UserPrompt: "USER-BLOCK-MARKER", - }) - if err != nil { - t.Fatalf("Run: %v", err) - } +// problemChars are the characters that broke the checks' JSON output before: +// typographic quotes of several languages, an ASCII quote, braces, a +// backslash path, a regex, a line break. +const problemChars = "„Code einlösen“ «zitiert» ‚einfach‘ “English” an ASCII \" quote, Options{Zebra: true}, C:\\Projekte\\de.json, ^Code\\s+[A-Z0-9]{8}$, a line\nbreak, & 🎉" - mu.Lock() - defer mu.Unlock() - t.Logf("request keys: %v", keysOf(body)) - t.Logf("full body: %+v", body) +// Out of alphabetical order, so a schema that went through a Go map shows. +const wireSchema = `{"type":"object","properties":{"reasoning":{"type":"string"},"axis":{"type":"string","enum":["form"]}},"required":["reasoning","axis"],"additionalProperties":false}` - if body["think"] != "high" { - t.Errorf("think = %v, want high", body["think"]) - } - prompt, _ := body["prompt"].(string) - if !strings.Contains(prompt, "SYSTEM-BLOCK-MARKER") { - t.Errorf("system block missing from the flattened prompt: %q", prompt) - } - // Ollama flattens through Prompt.String(), which used to render the input - // both bare and again under a Messages header — every call paid twice. - if n := strings.Count(prompt, "USER-BLOCK-MARKER"); n != 1 { - t.Errorf("user block sent %d times, want 1: %q", n, prompt) - } - if res.Usage.InputTokens != 11 || res.Usage.OutputTokens != 22 { - t.Errorf("usage not parsed: %+v", res.Usage) - } - if res.Identity.Variant != "think=high" { - t.Errorf("variant = %q, want think=high", res.Identity.Variant) - } +// wireCase drives a real request at a local listener: what reaches the +// provider is not inferable from the adapter code, since gollm copies option +// maps into the request body verbatim. +type wireCase struct { + name string + cfg func(url string) model.LLMConfig + path string + response func(text string) string + system func(t *testing.T, body map[string]any) string + user func(t *testing.T, body map[string]any) string + schema func(t *testing.T, body map[string]any) any +} + +var wireCases = []wireCase{ + { + name: "openai", + // A key OpenAI itself would reject must still reach a gateway that issued it. + cfg: func(url string) model.LLMConfig { + return model.LLMConfig{Provider: "openai", Model: "m", Endpoint: url, APIKeys: map[string]string{"openai": "op-notarealkeyaaaaaaaaaa"}} + }, + path: "/v1/chat/completions", + response: func(text string) string { + return `{"choices":[{"message":{"content":` + jsonString(text) + `}}],"usage":{"prompt_tokens":11,"completion_tokens":22}}` + }, + system: func(t *testing.T, body map[string]any) string { return messageContent(t, body, "system", "developer") }, + user: func(t *testing.T, body map[string]any) string { return messageContent(t, body, "user") }, + schema: func(t *testing.T, body map[string]any) any { + js := asMap(t, asMap(t, body["response_format"])["json_schema"]) + if js["strict"] != true { + t.Errorf("json_schema.strict = %v, want true", js["strict"]) + } + return js["schema"] + }, + }, + { + name: "anthropic", + cfg: func(url string) model.LLMConfig { + return model.LLMConfig{Provider: "anthropic", Model: "m", Endpoint: url, APIKeys: map[string]string{"anthropic": "op-notarealkeyaaaaaaaaaa"}} + }, + path: "/v1/messages", + response: func(text string) string { + return `{"content":[{"type":"text","text":` + jsonString(text) + `}],"usage":{"input_tokens":11,"output_tokens":22}}` + }, + system: func(t *testing.T, body map[string]any) string { + var parts []string + for _, block := range asSlice(t, body["system"]) { + parts = append(parts, asMap(t, block)["text"].(string)) + } + return strings.Join(parts, "\n\n") + }, + user: func(t *testing.T, body map[string]any) string { + var parts []string + for _, m := range asSlice(t, body["messages"]) { + for _, block := range asSlice(t, asMap(t, m)["content"]) { + parts = append(parts, asMap(t, block)["text"].(string)) + } + } + return strings.Join(parts, "") + }, + schema: func(t *testing.T, body map[string]any) any { + return asMap(t, asMap(t, body["output_config"])["format"])["schema"] + }, + }, + { + name: "ollama", + cfg: func(url string) model.LLMConfig { + return model.LLMConfig{Provider: "ollama", Model: "m", OllamaEndpoint: url, Params: map[string]string{"think": "high"}} + }, + path: "/api/generate", + response: func(text string) string { + return `{"model":"m","response":` + jsonString(text) + `,"done":true,"prompt_eval_count":11,"eval_count":22}` + }, + system: func(t *testing.T, body map[string]any) string { s, _ := body["system"].(string); return s }, + user: func(t *testing.T, body map[string]any) string { s, _ := body["prompt"].(string); return s }, + schema: func(t *testing.T, body map[string]any) any { return body["format"] }, + }, } -// The openai provider is the one whose target is configurable, so where a -// request actually lands is a property worth asserting on the bytes: a base URL -// only serves an OpenAI-compatible gateway if the path is appended to it. -func TestOpenAIRequestWire(t *testing.T) { - var mu sync.Mutex - var body map[string]any - var path string - - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - raw, _ := io.ReadAll(r.Body) - mu.Lock() - path = r.URL.Path - _ = json.Unmarshal(raw, &body) - mu.Unlock() - _, _ = w.Write([]byte(`{"choices":[{"message":{"content":"SERVED"}}],"usage":{"prompt_tokens":33,"completion_tokens":44}}`)) - })) - defer srv.Close() - - runner, err := gollmrunner.NewRunner(model.LLMConfig{ - Provider: "openai", Model: "m", Endpoint: srv.URL, - APIKeys: map[string]string{"openai": "op-notarealkeyaaaaaaaaaa"}, - }) - if err != nil { - t.Fatalf("NewRunner: %v", err) +// A structured request reaches each provider with the schema in its native +// slot, byte for byte, the system prompt once in its own slot and the user +// prompt alone in the user content, and the problem characters unchanged on +// the way out and back; the result reports the provider's usage. +func TestRequestWire(t *testing.T) { + for _, c := range wireCases { + t.Run(c.name, func(t *testing.T) { + var mu sync.Mutex + var raw []byte + var path string + answer := `{"reasoning":` + jsonString(problemChars) + `,"axis":"form"}` + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/api/tags" { // Ollama's reachability probe + _, _ = w.Write([]byte(`{"models":[]}`)) + return + } + body, _ := io.ReadAll(r.Body) + mu.Lock() + raw, path = body, r.URL.Path + mu.Unlock() + _, _ = w.Write([]byte(c.response(answer))) + })) + defer srv.Close() + + runner, err := gollmrunner.NewRunner(c.cfg(srv.URL)) + if err != nil { + t.Fatalf("NewRunner: %v", err) + } + res, err := runner.Run(context.Background(), llm.Request{ + SystemPrompt: "SYSTEM-BLOCK-MARKER " + problemChars, + UserPrompt: "USER-BLOCK-MARKER " + problemChars, + ResponseSchema: json.RawMessage(wireSchema), + }) + if err != nil { + t.Fatalf("Run: %v", err) + } + + mu.Lock() + defer mu.Unlock() + if path != c.path { + t.Errorf("request path = %q, want %q", path, c.path) + } + if !bytes.Contains(raw, []byte(wireSchema)) { + t.Errorf("schema not sent byte for byte: %s", raw) + } + if n := bytes.Count(raw, []byte("SYSTEM-BLOCK-MARKER")); n != 1 { + t.Errorf("system prompt sent %d times, want once", n) + } + var body map[string]any + if err := json.Unmarshal(raw, &body); err != nil { + t.Fatalf("request body: %v", err) + } + if got, _ := json.Marshal(c.schema(t, body)); !jsonEqual(t, got, []byte(wireSchema)) { + t.Errorf("schema in the provider's slot = %s", got) + } + if got := c.system(t, body); got != "SYSTEM-BLOCK-MARKER "+problemChars { + t.Errorf("system slot = %q", got) + } + if got := c.user(t, body); got != "USER-BLOCK-MARKER "+problemChars { + t.Errorf("user content = %q", got) + } + if res.Text != answer { + t.Errorf("text = %q, want %q", res.Text, answer) + } + if res.Usage.InputTokens != 11 || res.Usage.OutputTokens != 22 { + t.Errorf("usage not parsed: %+v", res.Usage) + } + }) } - res, err := runner.Run(context.Background(), llm.Request{ - SystemPrompt: "SYSTEM-BLOCK-MARKER", - UserPrompt: "USER-BLOCK-MARKER", - }) - if err != nil { - t.Fatalf("Run: %v", err) +} + +// A request without a schema asks for free text: no provider's schema slot is +// filled. Behaviour-affecting params reach the provider and the variant. +func TestRequestWireWithoutSchema(t *testing.T) { + for _, c := range wireCases { + t.Run(c.name, func(t *testing.T) { + var raw []byte + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/api/tags" { + _, _ = w.Write([]byte(`{"models":[]}`)) + return + } + raw, _ = io.ReadAll(r.Body) + _, _ = w.Write([]byte(c.response("SERVED"))) + })) + defer srv.Close() + + cfg := c.cfg(srv.URL) + runner, err := gollmrunner.NewRunner(cfg) + if err != nil { + t.Fatalf("NewRunner: %v", err) + } + res, err := runner.Run(context.Background(), llm.Request{SystemPrompt: "S", UserPrompt: "U"}) + if err != nil { + t.Fatalf("Run: %v", err) + } + for _, key := range []string{`"response_format"`, `"output_config"`, `"format"`} { + if bytes.Contains(raw, []byte(key)) { + t.Errorf("free-text request carries %s: %s", key, raw) + } + } + if res.Text != "SERVED" { + t.Errorf("text = %q, want SERVED", res.Text) + } + if cfg.Params["think"] != "" { + var body map[string]any + _ = json.Unmarshal(raw, &body) + if body["think"] != "high" { + t.Errorf("think = %v, want high", body["think"]) + } + if res.Identity.Variant != "think=high" { + t.Errorf("variant = %q, want think=high", res.Identity.Variant) + } + } + }) } +} - mu.Lock() - defer mu.Unlock() - t.Logf("request keys: %v", keysOf(body)) +func jsonString(s string) string { + b, _ := json.Marshal(s) + return string(b) +} - if path != "/v1/chat/completions" { - t.Errorf("request path = %q, want /v1/chat/completions", path) +func jsonEqual(t *testing.T, a, b []byte) bool { + t.Helper() + var x, y any + if err := json.Unmarshal(a, &x); err != nil { + t.Fatalf("decoding %s: %v", a, err) } - // A key OpenAI itself would reject must still reach a gateway that issued it. - sent, _ := json.Marshal(body["messages"]) - for _, marker := range []string{"SYSTEM-BLOCK-MARKER", "USER-BLOCK-MARKER"} { - if !strings.Contains(string(sent), marker) { - t.Errorf("%s missing from the messages sent: %s", marker, sent) - } + if err := json.Unmarshal(b, &y); err != nil { + t.Fatalf("decoding %s: %v", b, err) } - if res.Text != "SERVED" { - t.Errorf("text = %q, want SERVED", res.Text) + ax, _ := json.Marshal(x) + by, _ := json.Marshal(y) + return bytes.Equal(ax, by) +} + +func messageContent(t *testing.T, body map[string]any, roles ...string) string { + t.Helper() + var parts []string + for _, m := range asSlice(t, body["messages"]) { + msg := asMap(t, m) + for _, role := range roles { + if msg["role"] == role { + parts = append(parts, msg["content"].(string)) + } + } } - if res.Usage.InputTokens != 33 || res.Usage.OutputTokens != 44 { - t.Errorf("usage not parsed: %+v", res.Usage) + return strings.Join(parts, "\n\n") +} + +func asSlice(t *testing.T, v any) []any { + t.Helper() + s, ok := v.([]any) + if !ok { + t.Fatalf("want a JSON array, got %T", v) } + return s } -func keysOf(m map[string]any) []string { - out := make([]string, 0, len(m)) - for k := range m { - out = append(out, k) +func asMap(t *testing.T, v any) map[string]any { + t.Helper() + m, ok := v.(map[string]any) + if !ok { + t.Fatalf("want a JSON object, got %T", v) } - return out + return m } diff --git a/internal/llmops/eval_stats_test.go b/internal/llmops/eval_stats_test.go index 39fc204f5..7aec1e1d4 100644 --- a/internal/llmops/eval_stats_test.go +++ b/internal/llmops/eval_stats_test.go @@ -85,6 +85,7 @@ func (m multiSink) RecordCall(ctx context.Context, stat pkgllm.CallStat) { func TestMain(m *testing.M) { code := m.Run() printEvalUsageSummary() + printCostSummary() os.Exit(code) } diff --git a/internal/llmops/json_response_test.go b/internal/llmops/json_response_test.go index c6002c0e5..abc1e1775 100644 --- a/internal/llmops/json_response_test.go +++ b/internal/llmops/json_response_test.go @@ -78,6 +78,8 @@ func TestCheckersJSONResponse(t *testing.T) { {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: "typographic_quotes", output: strings.Replace(checker.payload, "The", "„Quoted“ and «cited» affect the", 1), want: 1}, + {name: "typographic_quote_normalized_unescaped", output: strings.Replace(checker.payload, "The", `„Quoted" affects the`, 1), wantErr: true}, {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}, diff --git a/internal/llmops/preflight.go b/internal/llmops/preflight.go index 29ce6fea4..2a98617de 100644 --- a/internal/llmops/preflight.go +++ b/internal/llmops/preflight.go @@ -482,9 +482,6 @@ var ( func parsedPreflightTemplates() (*template.Template, error) { preflightTmplOnce.Do(func() { preflightTmpl, preflightTmplErr = template.ParseFS(preflightTemplates, "preflight_templates/*.tmpl") - if preflightTmplErr == nil { - preflightTmpl, preflightTmplErr = preflightTmpl.ParseFS(sharedPromptTemplates, "shared_templates/*.tmpl") - } }) return preflightTmpl, preflightTmplErr } @@ -541,18 +538,16 @@ func renderPreflightPrompt(ct checkType, pctx *preflightContext) (llm.Request, e } return llm.Request{ - Purpose: llm.PurposePreflight, - SystemPrompt: strings.TrimSpace(sysB.String()), - UserPrompt: strings.TrimSpace(userB.String()), + Purpose: llm.PurposePreflight, + SystemPrompt: strings.TrimSpace(sysB.String()), + UserPrompt: strings.TrimSpace(userB.String()), + ResponseSchema: preflightSchema, }, nil } -// parsePreflightResult parses the validator's JSON response into a -// structured result. The LLM is asked to respond with: -// -// {"findings": [{"severity": "high|medium|low", "category": "...", "observation": "..."}]} -// -// Empty findings array means "no findings". The parser tolerates prose +// parsePreflightResult parses the validator's JSON response, shaped by +// response_schemas/preflight.json, into a structured result. Empty findings +// array means "no findings". The parser tolerates prose // surrounding the JSON object (LLM preambles, code fences) by scanning for // a single valid JSON object. Malformed JSON, missing keys, unknown severity // values — all return errors so infrastructure failures stay distinct from diff --git a/internal/llmops/preflight_eval_test.go b/internal/llmops/preflight_eval_test.go index 6a2089fe7..157c23b66 100644 --- a/internal/llmops/preflight_eval_test.go +++ b/internal/llmops/preflight_eval_test.go @@ -19,6 +19,7 @@ package llmops_test import ( "context" + "errors" "fmt" "os" "strconv" @@ -35,11 +36,14 @@ import ( pkgllm "github.com/networkteam/sdd/pkg/llm" ) -// capturingRunner wraps a real llm.Runner and records the last raw response. +// capturingRunner wraps a real llm.Runner and keeps the last raw response. // llm.Preflight returns only the parsed result, so the wrapper preserves the // raw model output for failure logging while the eval still runs the real -// orchestrator end to end. +// orchestrator end to end. Every call is also recorded raw with its cost +// (specimen_eval_test.go) under the name of the test that built the runner, +// so a case with subtests builds one per subtest. type capturingRunner struct { + t *testing.T inner llm.Runner lastText string } @@ -49,6 +53,20 @@ func (r *capturingRunner) Run(ctx context.Context, req llm.Request) (llm.Result, if res.Text != "" { r.lastText = res.Text } + identity := res.Identity + if attributed, ok := errors.AsType[*llm.Error](err); ok { + identity = attributed.Identity + } + row := responseRow{ + Time: time.Now().UTC(), Test: r.t.Name(), Purpose: string(req.Purpose), + Provider: identity.Provider, Model: identity.Model, Variant: identity.Variant, + PromptSHA256: promptSHA256(req.SystemPrompt, req.UserPrompt), ResponseSchema: len(req.ResponseSchema) > 0, + Raw: res.Text, InputTokens: res.Usage.InputTokens, OutputTokens: res.Usage.OutputTokens, CostUSD: res.Usage.CostUSD, + } + if err != nil { + row.Error = err.Error() + } + recordResponse(r.t, row) return res, err } @@ -61,7 +79,7 @@ func evalRunner(t *testing.T) *capturingRunner { if err != nil { t.Fatalf("building eval runner: %v", err) } - return &capturingRunner{inner: pkgllm.Observed(runner, multiSink{evalFileSink(t), tLogSink{t}})} + return &capturingRunner{t: t, inner: pkgllm.Observed(runner, multiSink{evalFileSink(t), tLogSink{t}})} } // evalConfig resolves the candidate configuration for this run. SDD_EVAL_CONFIG @@ -173,6 +191,13 @@ func runEval(t *testing.T, graph *model.Graph, proposed *model.Entry) (*llmops.P // aborting the test, so a single flaky response counts as a failed run rather // than killing the whole measurement (s-prc-vvd's run 5 was exactly that). func runEvalOnce(t *testing.T, graph *model.Graph, proposed *model.Entry) (*llmops.PreflightResult, string, error) { + t.Helper() + // English default — the language-drift rubric only fires for non-empty locales. + return runEvalOnceIn(t, graph, proposed, "") +} + +// runEvalOnceIn runs pre-flight under the configured graph language. +func runEvalOnceIn(t *testing.T, graph *model.Graph, proposed *model.Entry, language string) (*llmops.PreflightResult, string, error) { t.Helper() runner := evalRunner(t) @@ -182,8 +207,7 @@ func runEvalOnce(t *testing.T, graph *model.Graph, proposed *model.Entry) (*llmo ctx, cancel := context.WithTimeout(context.Background(), 240*time.Second) defer cancel() - // English default — the language-drift rubric only fires for non-empty locales. - result, err := llmops.Preflight(ctx, runner, proposed, evalGraphWithBase(t, graph), "") + result, err := llmops.Preflight(ctx, runner, proposed, evalGraphWithBase(t, graph), language) return result, runner.lastText, err } @@ -203,6 +227,20 @@ func evalGraphWithBase(t *testing.T, graph *model.Graph) *model.Graph { return model.NewGraph(model.MergeEmbedded(graph.Entries, onDisk, base)) } +// TestPreflightEval_Specimens runs the pre-flight specimens +// (specimen_eval_test.go), each under its configured language. +func TestPreflightEval_Specimens(t *testing.T) { + for _, s := range loadSpecimens(t, "preflight") { + t.Run(s.ID, func(t *testing.T) { + t.Parallel() + draft := s.Draft.entry(t) + runEvalPassRateIn(t, s.graph(t), draft, s.Language, s.Expect.tier(), func(r *llmops.PreflightResult) error { + return s.Expect.checkPreflight(draft.Content, r) + }) + }) + } +} + // --- N-run pass-rate support (plan d-tac-tph, AC 2) --- // passRate is a per-case pass-rate requirement: at least MinPasses of Runs @@ -240,11 +278,17 @@ func (p passRate) withRunsOverride() passRate { // least MinPasses runs where check returns nil. Infrastructure errors count as // failed runs (logged), not aborts. func runEvalPassRate(t *testing.T, graph *model.Graph, proposed *model.Entry, rate passRate, check func(*llmops.PreflightResult) error) { + t.Helper() + runEvalPassRateIn(t, graph, proposed, "", rate, check) +} + +// runEvalPassRateIn is runEvalPassRate under the configured graph language. +func runEvalPassRateIn(t *testing.T, graph *model.Graph, proposed *model.Entry, language string, rate passRate, check func(*llmops.PreflightResult) error) { t.Helper() rate = rate.withRunsOverride() passes := 0 for i := 1; i <= rate.Runs; i++ { - result, raw, err := runEvalOnce(t, graph, proposed) + result, raw, err := runEvalOnceIn(t, graph, proposed, language) if err != nil { t.Logf("run %d/%d FAIL (infrastructure): %v\nRaw output:\n%s", i, rate.Runs, err, raw) continue diff --git a/internal/llmops/preflight_internal_test.go b/internal/llmops/preflight_internal_test.go index 21a2fda06..a3f3ad0ed 100644 --- a/internal/llmops/preflight_internal_test.go +++ b/internal/llmops/preflight_internal_test.go @@ -727,21 +727,20 @@ func Test_renderPreflightPrompt_AllCheckTypes(t *testing.T) { if !strings.Contains(result.Combined(), "Test content") { t.Errorf("renderPreflightPrompt(%s) missing proposed entry content", ct) } - // Verdict partial must be embedded — the JSON output format and + // Verdict partial must be embedded — the finding fields and // severity semantics are the single source of truth for all checks. - if !strings.Contains(result.Combined(), `"findings"`) { - t.Errorf("renderPreflightPrompt(%s) missing JSON schema with findings key", ct) - } - if !strings.Contains(result.Combined(), `"severity"`) { - t.Errorf("renderPreflightPrompt(%s) missing severity field in schema", ct) + if !strings.Contains(result.SystemPrompt, "## Findings") { + t.Errorf("renderPreflightPrompt(%s) missing the findings section", ct) } // Category and severity are conclusions, not opening bids: the // schema must ask for the observation (reasoning) first, so a // finding cannot commit a verdict its own prose then retracts — // and category-first proved to get skipped by the model entirely. - obsIdx := strings.Index(result.Combined(), `"observation"`) - catIdx := strings.Index(result.Combined(), `"category"`) - sevIdx := strings.Index(result.Combined(), `"severity"`) + // Strict decoding generates in schema order. + schema := string(result.ResponseSchema) + obsIdx := strings.Index(schema, `"observation"`) + catIdx := strings.Index(schema, `"category"`) + sevIdx := strings.Index(schema, `"severity"`) if obsIdx < 0 || catIdx < 0 || sevIdx < 0 || obsIdx > catIdx || catIdx > sevIdx { t.Errorf("renderPreflightPrompt(%s) schema must order observation < category < severity (obs %d, cat %d, sev %d)", ct, obsIdx, catIdx, sevIdx) } @@ -988,9 +987,9 @@ func Test_renderPreflightPrompt_StructuralChecksKeepLightSystem(t *testing.T) { t.Errorf("%s system block should stay light but contains %q", ct, heavy) } } - // It must still carry the verdict format, which every check needs. - if !strings.Contains(req.SystemPrompt, `"findings"`) { - t.Errorf("%s system block should still carry the verdict output format", ct) + // It must still carry the verdict section, which every check needs. + if !strings.Contains(req.SystemPrompt, "## Findings") { + t.Errorf("%s system block should still carry the verdict section", ct) } }) } @@ -1014,26 +1013,6 @@ func Test_refKindVocabulary_GraphBacked(t *testing.T) { } } -func Test_renderPreflightPrompt_JSONStringRulesShared(t *testing.T) { - // Regression for 20260827-224853-s-tac-giv: the output format teaches - // standard JSON escaping, never quote avoidance, on every system template. - pctx := &preflightContext{ProposedEntry: "ID: test\nType: signal\n\nproposed"} - for _, ct := range []checkType{checkSignalCapture, checkAnnotationCapture, checkFocusCapture} { - t.Run(ct.String(), func(t *testing.T) { - req, err := renderPreflightPrompt(ct, pctx) - if err != nil { - t.Fatal(err) - } - if !strings.Contains(req.SystemPrompt, "**JSON strings.**") || !strings.Contains(req.SystemPrompt, "write a literal `\"` as `\\\"`") { - t.Errorf("%s system block missing the shared JSON escaping rule:\n%s", ct, req.SystemPrompt) - } - if strings.Contains(req.SystemPrompt, "never a literal") { - t.Errorf("%s system block still teaches quote avoidance", ct) - } - }) - } -} - func Test_renderPreflightPrompt_InvalidCheckType(t *testing.T) { pctx := &preflightContext{ProposedEntry: "test"} _, err := renderPreflightPrompt(checkType(99), pctx) diff --git a/internal/llmops/preflight_templates/verdict.tmpl b/internal/llmops/preflight_templates/verdict.tmpl index 7e151b5c9..cf909daea 100644 --- a/internal/llmops/preflight_templates/verdict.tmpl +++ b/internal/llmops/preflight_templates/verdict.tmpl @@ -1,27 +1,15 @@ {{ define "verdict" }} -## Output format +## Findings -Respond with a single JSON object matching this schema — nothing before or after, no code fences: +Each finding carries: -``` -{ - "findings": [ - { - "observation": "one to three sentences working through what you noticed, ending in a conclusion", - "category": "short-hyphenated-tag", - "severity": "high" | "medium" | "low" - } - ] -} -``` +- `observation` — one to three sentences working through what you noticed, ending in a conclusion +- `category` — a short hyphenated tag naming what the observation found (e.g. `type-mismatch`, `missing-ref`, `plan-coverage-ambiguity`, `ac-unaddressed`) +- `severity` — `high`, `medium` or `low`, as defined below -**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. +**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 rather than one at any severity. When you have nothing to report, return an empty list. -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`). When you have nothing to report, return `{"findings": []}`. - -{{ template "json_string_rules" }} +**Citing an excerpt.** When an observation cites the proposed entry or a related entry, set the excerpt off in quotes or backticks and take enough of its words to locate the passage; they need not match it character for character. ## Severity diff --git a/internal/llmops/prompt_contract_test.go b/internal/llmops/prompt_contract_test.go new file mode 100644 index 000000000..38597970f --- /dev/null +++ b/internal/llmops/prompt_contract_test.go @@ -0,0 +1,153 @@ +package llmops_test + +import ( + "context" + "encoding/json" + "slices" + "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" +) + +// The checks declare their result shape as a response schema the provider +// enforces (20261004-134853-d-cpt-sw8), so the prompts describe what each +// field means and teach no JSON construction (20261004-135748-d-tac-ndh); a +// quote only has to locate its passage (20261005-170145-d-tac-gch). +func TestCheckerPromptContract(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 loader drops the second option."} + + checkers := []struct { + name string + run func(llm.Runner) error + fields []string + severities []string + quoteRule []string + }{ + { + name: "preflight", + run: func(r llm.Runner) error { + _, err := llmops.Preflight(t.Context(), r, entry, graph, "") + return err + }, + fields: []string{"observation", "category", "severity"}, + severities: []string{string(llmops.SeverityHigh), string(llmops.SeverityMedium), string(llmops.SeverityLow)}, + quoteRule: []string{"take enough of its words to locate the passage", "need not match it character for character"}, + }, + { + name: "writing_guide", + run: func(r llm.Runner) error { + _, err := llmops.WritingGuide(t.Context(), r, entry, nil, llmops.ReferenceFacts{ + Source: responseFactSource{graph}, TypeSystemFactID: basefacts.OverviewFactID, + }) + return err + }, + fields: []string{"reasoning", "axis", "quote", "repair", "severity"}, + severities: []string{string(llmops.GuideSubstantive), string(llmops.GuideMinor)}, + quoteRule: []string{ + "enough of the draft's own words to locate it unambiguously", + "need not match the draft character for character", + "the quote `gains this as a fixed section` locates the passage", + }, + }, + } + for _, c := range checkers { + t.Run(c.name, func(t *testing.T) { + var req llm.Request + runner := llm.RunnerFunc(func(_ context.Context, r llm.Request) (llm.Result, error) { + req = r + return llm.Result{Text: `{"findings":[]}`}, nil + }) + if err := c.run(runner); err != nil { + t.Fatal(err) + } + + schema := findingSchema(t, req.ResponseSchema) + if !slices.Equal(schema.order, c.fields) { + t.Errorf("finding properties = %v, want %v (strict decoding generates in schema order, reasoning first)", schema.order, c.fields) + } + if !slices.Equal(schema.required, c.fields) { + t.Errorf("required = %v, want every field %v", schema.required, c.fields) + } + if !slices.Equal(schema.enum["severity"], c.severities) { + t.Errorf("severity enum = %v, want the parser's %v", schema.enum["severity"], c.severities) + } + + prompt := req.SystemPrompt + "\n" + req.UserPrompt + for _, rule := range []string{"JSON", "escap", "code fence", "nothing before or after", `{"findings"`} { + if strings.Contains(prompt, rule) { + t.Errorf("prompt still teaches JSON construction: contains %q", rule) + } + } + for _, want := range c.quoteRule { + if !strings.Contains(req.SystemPrompt, want) { + t.Errorf("system prompt missing the quote rule %q", want) + } + } + }) + } +} + +type parsedFindingSchema struct { + order []string + required []string + enum map[string][]string +} + +// findingSchema reads a findings schema's item properties in document order, +// which a decode into a Go map would lose. +func findingSchema(t *testing.T, raw json.RawMessage) parsedFindingSchema { + t.Helper() + var doc struct { + AdditionalProperties *bool `json:"additionalProperties"` + Properties struct { + Findings struct { + Items struct { + Properties json.RawMessage `json:"properties"` + Required []string `json:"required"` + AdditionalProperties *bool `json:"additionalProperties"` + } `json:"items"` + } `json:"findings"` + } `json:"properties"` + } + if err := json.Unmarshal(raw, &doc); err != nil { + t.Fatalf("response schema is not valid JSON: %v\n%s", err, raw) + } + items := doc.Properties.Findings.Items + if doc.AdditionalProperties == nil || *doc.AdditionalProperties || items.AdditionalProperties == nil || *items.AdditionalProperties { + t.Errorf("every object must forbid additional properties, as strict mode requires: %s", raw) + } + + dec := json.NewDecoder(strings.NewReader(string(items.Properties))) + if _, err := dec.Token(); err != nil { + t.Fatalf("finding properties: %v", err) + } + out := parsedFindingSchema{required: items.Required, enum: map[string][]string{}} + for dec.More() { + key, err := dec.Token() + if err != nil { + t.Fatalf("finding properties: %v", err) + } + var prop struct { + Enum []string `json:"enum"` + } + if err := dec.Decode(&prop); err != nil { + t.Fatalf("finding property %v: %v", key, err) + } + name := key.(string) + out.order = append(out.order, name) + if prop.Enum != nil { + out.enum[name] = prop.Enum + } + } + return out +} diff --git a/internal/llmops/quote_locate_test.go b/internal/llmops/quote_locate_test.go new file mode 100644 index 000000000..e3ab7a002 --- /dev/null +++ b/internal/llmops/quote_locate_test.go @@ -0,0 +1,143 @@ +package llmops_test + +import ( + "regexp" + "slices" + "strings" + "testing" +) + +// The eval specimens check quotes against the quote contract +// (20261005-170145-d-tac-gch): a quote need not match its passage character +// for character, only locate it. + +var quoteNormalizer = strings.NewReplacer( + "„", "", "“", "", "”", "", "‚", "", "‘", "", "’", "", "«", "", "»", "", + "‹", "", "›", "", `"`, "", "'", "", "`", "", "—", "-", "–", "-", +) + +func normalizeQuoted(s string) string { + return strings.Join(strings.Fields(quoteNormalizer.Replace(strings.ToLower(s))), " ") +} + +// quoteParts splits a quote shortened with an ellipsis into the parts that +// must appear in order. +func quoteParts(quote string) []string { + var parts []string + for p := range strings.SplitSeq(strings.ReplaceAll(quote, "...", "…"), "…") { + if n := normalizeQuoted(p); n != "" { + parts = append(parts, n) + } + } + return parts +} + +// quoteLocatesIn reports whether quote locates a passage of text that overlaps +// passage; an empty passage accepts any location. +func quoteLocatesIn(text, quote, passage string) bool { + nt := normalizeQuoted(text) + from, to := 0, len(nt) + if passage != "" { + np := normalizeQuoted(passage) + i := strings.Index(nt, np) + if np == "" || i < 0 { + return false + } + from, to = i, i+len(np) + } + parts := quoteParts(quote) + if len(parts) == 0 { + return false + } + for start := 0; ; { + i := strings.Index(nt[start:], parts[0]) + if i < 0 { + return false + } + first := start + i + end := first + len(parts[0]) + for _, p := range parts[1:] { + j := strings.Index(nt[end:], p) + if j < 0 { + return false + } + end += j + len(p) + } + if first < to && end > from { + return true + } + start = first + 1 + } +} + +var excerptPattern = regexp.MustCompile("“([^”]+)”|„([^“”]+)[“”]|\"([^\"]+)\"|«([^»]+)»|‚([^‘’]+)[‘’]|‘([^’]+)’|`([^`]+)`|(?:^|[^\\pL\\pN])'([^']+)'(?:$|[^\\pL\\pN])") + +// citedExcerpts returns the excerpts an observation sets off in quotes or +// backticks. A straight single quote counts only next to a non-letter, so an +// apostrophe does not open an excerpt. +func citedExcerpts(observation string) []string { + var out []string + for _, m := range excerptPattern.FindAllStringSubmatch(observation, -1) { + for _, g := range m[1:] { + if g != "" { + out = append(out, g) + break + } + } + } + return out +} + +func TestQuoteLocatesIn(t *testing.T) { + const draft = "Die Fehlermeldung nutzt jetzt die zweite Variante aus dem Call, nicht mehr „Einladungslink“.\n" + + "The release procedure gains this as a fixed section of its notes template. It gains this again." + tests := []struct { + name, quote, passage string + want bool + }{ + {"exact words", "gains this as a fixed section", "", true}, + {"ASCII quotes for typographic ones", `nicht mehr "Einladungslink"`, "", true}, + {"other case and whitespace", "THE RELEASE procedure\ngains", "", true}, + {"shortened with an ellipsis", "release procedure … notes template", "", true}, + {"shortened with three dots", "release procedure ... notes template", "", true}, + {"parts out of order", "notes template … release procedure", "", false}, + {"words not in the draft", "gains this as a permanent section", "", false}, + {"empty quote", " „“ ", "", false}, + {"inside the passage", "zweite Variante aus dem Call", "Die Fehlermeldung nutzt jetzt die zweite Variante aus dem Call, nicht mehr „Einladungslink“.", true}, + {"outside the passage", "fixed section", "Die Fehlermeldung nutzt jetzt die zweite Variante aus dem Call, nicht mehr „Einladungslink“.", false}, + {"second occurrence inside the passage", "gains this", "It gains this again.", true}, + {"passage not in the draft", "gains this", "A sentence the draft lacks.", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := quoteLocatesIn(draft, tt.quote, tt.passage); got != tt.want { + t.Errorf("quoteLocatesIn(%q, passage %q) = %v, want %v", tt.quote, tt.passage, got, tt.want) + } + }) + } +} + +func TestCitedExcerpts(t *testing.T) { + tests := []struct { + name, observation string + want []string + }{ + { + "typographic, double quotes and backticks", + "The ref `grounded-in` says “extends the contract” while the body says \"retires the immutability contract\"; „Partner-Code“ and «Link» stay.", + []string{"grounded-in", "extends the contract", "retires the immutability contract", "Partner-Code", "Link"}, + }, + { + "single quotes beside apostrophes", + "The draft's body states 'This directive retires the contract', but the desc doesn't: 'extends it'.", + []string{"This directive retires the contract", "extends it"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := citedExcerpts(tt.observation); !slices.Equal(got, tt.want) { + t.Errorf("citedExcerpts = %q, want %q", got, tt.want) + } + }) + } +} diff --git a/internal/llmops/response_schema.go b/internal/llmops/response_schema.go new file mode 100644 index 000000000..04c8643e3 --- /dev/null +++ b/internal/llmops/response_schema.go @@ -0,0 +1,64 @@ +package llmops + +import ( + "bytes" + "embed" + "encoding/json" + "fmt" +) + +// The response schemas pre-flight and the writing guide declare on their +// requests (20261004-134853-d-cpt-sw8). Kept as JSON files, not Go values: +// strict decoding generates properties in schema order, which puts each +// finding's reasoning before its verdict, and a Go map would sort the keys. +// +//go:embed response_schemas/*.json +var responseSchemas embed.FS + +var ( + preflightSchema = mustSchema("preflight.json") + writingGuideSchema = mustSchema("writing_guide.json") +) + +// mustSchema loads an embedded response schema, compacted with its property +// order intact. The files ship in the binary, so a malformed one is a build +// defect and panics at init. +func mustSchema(name string) json.RawMessage { + raw, err := responseSchemas.ReadFile("response_schemas/" + name) + if err != nil { + panic(fmt.Sprintf("reading response schema %s: %v", name, err)) + } + var buf bytes.Buffer + if err := json.Compact(&buf, raw); err != nil { + panic(fmt.Sprintf("compacting response schema %s: %v", name, err)) + } + return json.RawMessage(buf.Bytes()) +} + +// findingEnum returns the closed value set a findings schema declares for one +// property of a finding, so the parser accepts exactly what the schema allows. +func findingEnum(schema json.RawMessage, property string) map[string]bool { + var s struct { + Properties struct { + Findings struct { + Items struct { + Properties map[string]struct { + Enum []string `json:"enum"` + } `json:"properties"` + } `json:"items"` + } `json:"findings"` + } `json:"properties"` + } + if err := json.Unmarshal(schema, &s); err != nil { + panic(fmt.Sprintf("decoding response schema: %v", err)) + } + values := s.Properties.Findings.Items.Properties[property].Enum + if len(values) == 0 { + panic(fmt.Sprintf("response schema declares no enum for finding property %q", property)) + } + set := make(map[string]bool, len(values)) + for _, v := range values { + set[v] = true + } + return set +} diff --git a/internal/llmops/response_schemas/preflight.json b/internal/llmops/response_schemas/preflight.json new file mode 100644 index 000000000..3a99e0a10 --- /dev/null +++ b/internal/llmops/response_schemas/preflight.json @@ -0,0 +1,20 @@ +{ + "type": "object", + "properties": { + "findings": { + "type": "array", + "items": { + "type": "object", + "properties": { + "observation": {"type": "string"}, + "category": {"type": "string"}, + "severity": {"type": "string", "enum": ["high", "medium", "low"]} + }, + "required": ["observation", "category", "severity"], + "additionalProperties": false + } + } + }, + "required": ["findings"], + "additionalProperties": false +} diff --git a/internal/llmops/response_schemas/writing_guide.json b/internal/llmops/response_schemas/writing_guide.json new file mode 100644 index 000000000..81ff5b4c0 --- /dev/null +++ b/internal/llmops/response_schemas/writing_guide.json @@ -0,0 +1,22 @@ +{ + "type": "object", + "properties": { + "findings": { + "type": "array", + "items": { + "type": "object", + "properties": { + "reasoning": {"type": "string"}, + "axis": {"type": "string", "enum": ["stranding", "dilution", "conflation", "pointing", "form"]}, + "quote": {"type": "string"}, + "repair": {"type": "string", "enum": ["cut", "write-in", "split", "point", "reword"]}, + "severity": {"type": "string", "enum": ["substantive", "minor"]} + }, + "required": ["reasoning", "axis", "quote", "repair", "severity"], + "additionalProperties": false + } + } + }, + "required": ["findings"], + "additionalProperties": false +} diff --git a/internal/llmops/shared_templates.go b/internal/llmops/shared_templates.go deleted file mode 100644 index 6392e198b..000000000 --- a/internal/llmops/shared_templates.go +++ /dev/null @@ -1,11 +0,0 @@ -package llmops - -import "embed" - -// Partials included by more than one prompt set. Static text only: every -// including block is a byte-stable cacheable prefix (20260604-164527-d-tac-fah), -// and the shared JSON escaping rule exists so no prompt teaches avoidance where -// JSON requires escaping (20260827-224853-s-tac-giv). -// -//go:embed shared_templates/*.tmpl -var sharedPromptTemplates embed.FS diff --git a/internal/llmops/shared_templates/json_string_rules.tmpl b/internal/llmops/shared_templates/json_string_rules.tmpl deleted file mode 100644 index 095625d7f..000000000 --- a/internal/llmops/shared_templates/json_string_rules.tmpl +++ /dev/null @@ -1,3 +0,0 @@ -{{ 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. -{{- end }} diff --git a/internal/llmops/specimen_eval_test.go b/internal/llmops/specimen_eval_test.go new file mode 100644 index 000000000..f69f6c650 --- /dev/null +++ b/internal/llmops/specimen_eval_test.go @@ -0,0 +1,324 @@ +//go:build eval + +// Eval specimens in testdata/eval/specimens (20261102-101250-d-tac-s98): +// drafts with problem characters and the content a response must carry. Every +// eval call is also recorded raw, so the judge of `sdd eval` +// (20261031-174216-d-tac-p83) can score it later without new model calls. +// +// SDD_EVAL_RESPONSES_DIR selects where the rows append (/responses.jsonl) +// and where the cost summary goes (/cost-summary.md); unset, the run +// writes to a fresh temp dir and prints its path. + +package llmops_test + +import ( + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "os" + "path/filepath" + "slices" + "strings" + "sync" + "testing" + "time" + + "gopkg.in/yaml.v3" + + "github.com/networkteam/sdd/internal/llmops" + "github.com/networkteam/sdd/internal/model" +) + +const specimenDir = "testdata/eval/specimens" + +type evalSpecimen struct { + ID string `yaml:"id"` + Origin string `yaml:"origin"` + Class string `yaml:"class"` + Language string `yaml:"language"` + Operation string `yaml:"operation"` + Expect specimenExpect `yaml:"expect"` + Graph []specimenEntry `yaml:"graph"` + Draft specimenEntry `yaml:"draft"` +} + +// specimenExpect is the finding a response must carry and the draft passage +// its quote, or an excerpt its observation cites, must locate. +type specimenExpect struct { + Axis string `yaml:"axis"` + Severity string `yaml:"severity"` + Passage string `yaml:"passage"` + NoBlocking bool `yaml:"no_blocking"` +} + +type specimenEntry struct { + ID string `yaml:"id"` + Type string `yaml:"type"` + Kind string `yaml:"kind"` + Layer string `yaml:"layer"` + Refs []model.Ref `yaml:"refs"` + Content string `yaml:"content"` +} + +// loadSpecimens returns the specimens for operation, failing the test on any +// malformed file so a typo cannot silently shrink the set. +func loadSpecimens(t *testing.T, operation string) []evalSpecimen { + t.Helper() + paths, err := filepath.Glob(filepath.Join(specimenDir, "*.yaml")) + if err != nil { + t.Fatalf("listing specimens: %v", err) + } + var out []evalSpecimen + for _, path := range paths { + raw, err := os.ReadFile(path) + if err != nil { + t.Fatalf("reading %s: %v", path, err) + } + dec := yaml.NewDecoder(strings.NewReader(string(raw))) + dec.KnownFields(true) + var s evalSpecimen + if err := dec.Decode(&s); err != nil { + t.Fatalf("parsing %s: %v", path, err) + } + if want := strings.TrimSuffix(filepath.Base(path), ".yaml"); s.ID != want { + t.Fatalf("%s: id %q does not match the file name", path, s.ID) + } + if err := s.validate(); err != nil { + t.Fatalf("%s: %v", path, err) + } + if s.Operation == operation { + out = append(out, s) + } + } + if len(out) == 0 { + t.Fatalf("no specimens for %s in %s", operation, specimenDir) + } + return out +} + +func (s evalSpecimen) validate() error { + e := s.Expect + switch s.Operation { + case "writing-guide": + if e.Axis == "" || e.Passage == "" || e.Severity != "" || e.NoBlocking { + return fmt.Errorf("a writing-guide specimen expects an axis and a passage") + } + case "preflight": + clean := e.NoBlocking && e.Severity == "" && e.Passage == "" + finding := !e.NoBlocking && e.Severity != "" && e.Passage != "" + if e.Axis != "" || !(clean || finding) { + return fmt.Errorf("a preflight specimen expects either no_blocking or a severity and a passage") + } + default: + return fmt.Errorf("unknown operation %q", s.Operation) + } + if e.Passage != "" && !quoteLocatesIn(s.Draft.Content, e.Passage, "") { + return fmt.Errorf("passage %q is not in the draft", e.Passage) + } + return nil +} + +// tier holds a clean draft to the blocking bar and an expected finding to the +// advisory one. +func (e specimenExpect) tier() passRate { + if e.NoBlocking { + return blockingTier + } + return advisoryTier +} + +func (e specimenExpect) checkGuide(content string, r *llmops.WritingGuideResult) error { + for _, f := range r.Findings { + if f.Axis == e.Axis && quoteLocatesIn(content, f.Quote, e.Passage) { + return nil + } + } + return fmt.Errorf("no %s finding whose quote locates %q", e.Axis, e.Passage) +} + +func (e specimenExpect) checkPreflight(content string, r *llmops.PreflightResult) error { + if e.NoBlocking { + return noBlocking(r) + } + for _, f := range r.Findings { + if string(f.Severity) != e.Severity { + continue + } + for _, excerpt := range citedExcerpts(f.Observation) { + if quoteLocatesIn(content, excerpt, e.Passage) { + return nil + } + } + } + return fmt.Errorf("no %s finding citing an excerpt that locates %q", e.Severity, e.Passage) +} + +func (e specimenEntry) entry(t *testing.T) *model.Entry { + t.Helper() + typ := model.EntryType(e.Type) + if _, ok := model.TypeAbbrev[typ]; !ok { + t.Fatalf("specimen entry %q: unknown type %q", e.ID, e.Type) + } + return &model.Entry{ + ID: e.ID, + Type: typ, + Kind: model.Kind(e.Kind), + Layer: model.Layer(e.Layer), + Refs: e.Refs, + Content: e.Content, + Time: time.Date(2026, 4, 8, 12, 0, 0, 0, time.UTC), + } +} + +func (s evalSpecimen) graph(t *testing.T) *model.Graph { + t.Helper() + entries := make([]*model.Entry, 0, len(s.Graph)) + for _, e := range s.Graph { + entries = append(entries, e.entry(t)) + } + return model.NewGraph(entries) +} + +// responseRow is one recorded call: what the judge of `sdd eval` needs to +// score it later, keyed to the test and the prompt revision it ran under. +type responseRow struct { + Time time.Time `json:"time"` + Test string `json:"test"` + Purpose string `json:"purpose"` + Provider string `json:"provider"` + Model string `json:"model"` + Variant string `json:"variant,omitempty"` + PromptSHA256 string `json:"prompt_sha256"` + ResponseSchema bool `json:"response_schema"` + Raw string `json:"raw"` + Error string `json:"error,omitempty"` + InputTokens int `json:"input_tokens"` + OutputTokens int `json:"output_tokens"` + // CostUSD is the cost the provider reported for the call; absent when it + // reported none (20261101-010604-d-tac-769). + CostUSD float64 `json:"cost_usd,omitempty"` +} + +var ( + responsesOnce sync.Once + responsesMu sync.Mutex + responsesFile *os.File + responsesDir string + responsesErr error + costs = map[costKey]*costRow{} +) + +// costKey is one evaluated combination: the role it played, what served the +// call, and at which setting. +type costKey struct{ role, provider, model, variant string } + +type costRow struct { + calls, failed, costed int + inTokens, outTokens int + costUSD float64 +} + +func recordResponse(t *testing.T, row responseRow) { + responsesOnce.Do(func() { + dir := os.Getenv("SDD_EVAL_RESPONSES_DIR") + if dir == "" { + dir, responsesErr = os.MkdirTemp("", "sdd-eval-responses-") + if responsesErr != nil { + return + } + } else if responsesErr = os.MkdirAll(dir, 0o755); responsesErr != nil { + return + } + path := filepath.Join(dir, "responses.jsonl") + responsesFile, responsesErr = os.OpenFile(path, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o644) + if responsesErr == nil { + responsesDir = dir + fmt.Printf("eval responses: %s\n", path) + } + }) + if responsesErr != nil { + t.Errorf("opening eval responses file: %v", responsesErr) + return + } + line, err := json.Marshal(row) + if err != nil { + t.Errorf("encoding response row: %v", err) + return + } + responsesMu.Lock() + defer responsesMu.Unlock() + if _, err := responsesFile.Write(append(line, '\n')); err != nil { + t.Errorf("writing response row: %v", err) + } + role := "candidate" + if row.Purpose == "eval-judge" { + role = "judge" + } + key := costKey{role, row.Provider, row.Model, row.Variant} + c := costs[key] + if c == nil { + c = &costRow{} + costs[key] = c + } + c.calls++ + if row.Error != "" { + c.failed++ + } + if row.CostUSD > 0 { + c.costed++ + c.costUSD += row.CostUSD + } + c.inTokens += row.InputTokens + c.outTokens += row.OutputTokens +} + +// printCostSummary reports calls and cost per evaluated combination, on stdout +// and as cost-summary.md beside the responses (the workflow's job summary). +// Cost reads "not reported" when no call of a combination carried one; a +// partial sum is marked as such. +func printCostSummary() { + responsesMu.Lock() + defer responsesMu.Unlock() + if len(costs) == 0 { + return + } + keys := make([]costKey, 0, len(costs)) + for k := range costs { + keys = append(keys, k) + } + slices.SortFunc(keys, func(a, b costKey) int { + return strings.Compare(a.role+a.provider+a.model+a.variant, b.role+b.provider+b.model+b.variant) + }) + var b strings.Builder + b.WriteString("| Role | Provider | Model | Setting | Calls (failed) | Tokens in / out | Cost (USD) |\n") + b.WriteString("|---|---|---|---|---|---|---|\n") + for _, k := range keys { + c := costs[k] + cost := "not reported" + if c.costed > 0 { + cost = fmt.Sprintf("%.4f", c.costUSD) + if c.costed < c.calls { + cost += fmt.Sprintf(" (%d of %d calls)", c.costed, c.calls) + } + } + setting := k.variant + if setting == "" { + setting = "defaults" + } + fmt.Fprintf(&b, "| %s | %s | %s | %s | %d (%d) | %d / %d | %s |\n", + k.role, k.provider, k.model, setting, c.calls, c.failed, c.inTokens, c.outTokens, cost) + } + fmt.Printf("\n=== eval cost — this run ===\n%s", b.String()) + if responsesDir != "" { + path := filepath.Join(responsesDir, "cost-summary.md") + if err := os.WriteFile(path, []byte("## Cost\n\n"+b.String()), 0o644); err != nil { + fmt.Printf("writing %s: %v\n", path, err) + } + } +} + +func promptSHA256(system, user string) string { + sum := sha256.Sum256([]byte(system + "\x00" + user)) + return hex.EncodeToString(sum[:]) +} diff --git a/internal/llmops/summarize_eval_test.go b/internal/llmops/summarize_eval_test.go index 9f7507e49..04c49e594 100644 --- a/internal/llmops/summarize_eval_test.go +++ b/internal/llmops/summarize_eval_test.go @@ -136,18 +136,21 @@ type judgeVerdict struct { // candidate env knobs, so every candidate is measured with the same yardstick. // Default judge: Ollama Cloud's glm-5.3-flash:cloud at high effort (flat-rate, // no per-token cost); override via SDD_EVAL_JUDGE_PROVIDER / -// SDD_EVAL_JUDGE_MODEL / SDD_EVAL_JUDGE_PARAMS. +// SDD_EVAL_JUDGE_MODEL / SDD_EVAL_JUDGE_PARAMS / SDD_EVAL_JUDGE_ENDPOINT, the +// last because a candidate on another route of the same gateway needs another +// endpoint. func judgeRunner(t *testing.T) llm.Runner { t.Helper() cfg := evalBaseConfig(t) cfg.Provider = getenvOr("SDD_EVAL_JUDGE_PROVIDER", "ollama") cfg.Model = getenvOr("SDD_EVAL_JUDGE_MODEL", "glm-5.3-flash:cloud") cfg.Params = parseEvalParams(t, getenvOr("SDD_EVAL_JUDGE_PARAMS", "think=high")) + cfg.Endpoint = getenvOr("SDD_EVAL_JUDGE_ENDPOINT", cfg.Endpoint) runner, err := factory.New(cfg) if err != nil { t.Fatalf("building judge runner: %v", err) } - return pkgllm.Observed(runner, multiSink{evalFileSink(t), tLogSink{t}}) + return &capturingRunner{t: t, inner: pkgllm.Observed(runner, multiSink{evalFileSink(t), tLogSink{t}})} } // judgeSummary asks the judge to verify one summary against the exact source @@ -243,7 +246,6 @@ func checkSummaryMechanics(entryID, summary string) []string { // and expects fail verdicts — a judge that rubber-stamps these would make every // candidate measure capable. Judge-only calls; no candidate involved. func TestSummarizeJudgeCalibration(t *testing.T) { - judge := judgeRunner(t) tc := summarizeCases()[0] graph := model.NewGraph(append([]*model.Entry{tc.entry}, tc.related...)) req, err := llmops.RenderSummaryPrompt(tc.entry, graph, "") @@ -271,6 +273,7 @@ func TestSummarizeJudgeCalibration(t *testing.T) { for _, bad := range badSummaries { t.Run(bad.name, func(t *testing.T) { + judge := judgeRunner(t) verdict, err := judgeSummary(t, judge, req.UserPrompt, bad.summary, tc.wantRelationships) if err != nil { t.Fatalf("judge unavailable: %v", err) @@ -284,11 +287,9 @@ func TestSummarizeJudgeCalibration(t *testing.T) { } func TestSummarizeEval(t *testing.T) { - candidate := evalRunner(t) - judge := judgeRunner(t) - for _, tc := range summarizeCases() { t.Run(tc.name, func(t *testing.T) { + candidate, judge := evalRunner(t), judgeRunner(t) graph := model.NewGraph(append([]*model.Entry{tc.entry}, tc.related...)) ctx, cancel := context.WithTimeout(context.Background(), 240*time.Second) diff --git a/internal/llmops/testdata/eval/specimens/README.md b/internal/llmops/testdata/eval/specimens/README.md new file mode 100644 index 000000000..d46be8324 --- /dev/null +++ b/internal/llmops/testdata/eval/specimens/README.md @@ -0,0 +1,17 @@ +# Eval specimens + +Drafts with characters that broke the checks' JSON output before, each with +the content a response must carry. `TestWritingGuideEval_Specimens` and +`TestPreflightEval_Specimens` run them. One specimen per file; external +material is anonymized. + +Fields: `id` (the file name without extension), `origin` (where the case +comes from), `class` (the characters it carries), `language` (the configured +graph language for pre-flight), `operation` (`writing-guide` or `preflight`), +`expect`, `graph` (entries the draft's refs resolve to) and `draft` (`type`, +`kind`, `layer`, `refs`, `content`). + +`expect` names a finding and the draft passage it must point to: `axis` for +the writing guide, `severity` for pre-flight, and `passage`, which the +finding's quote, or an excerpt its observation cites, must locate. Instead, +`no_blocking: true` requires pre-flight to return no high finding. diff --git a/internal/llmops/testdata/eval/specimens/de-done-quotes-german.yaml b/internal/llmops/testdata/eval/specimens/de-done-quotes-german.yaml new file mode 100644 index 000000000..5aa7540da --- /dev/null +++ b/internal/llmops/testdata/eval/specimens/de-done-quotes-german.yaml @@ -0,0 +1,17 @@ +id: de-done-quotes-german +origin: GitHub #20 and #21, anonymized; the error-message line is a planted stranding +class: German typographic quotes „…“ +operation: writing-guide +expect: + axis: stranding + passage: Die Fehlermeldung nutzt jetzt die zweite Variante aus dem Call, nicht mehr „Einladungslink“. +draft: + type: signal + kind: done + layer: tactical + content: |- + Die nutzersichtbaren Partner-Code-Texte sind umformuliert und über MR !42 nach `main` gemergt: Kein Button spricht mehr von „Code einlösen“ — die Wording-Direktive ist umgesetzt, und der Einstieg in die Bezahlseite verspricht keine Gutschein-Einlösung mehr. + + - Auf Wunsch aus dem Review wurden die begleitenden Fließtexte angeglichen: In `EmptyState.tsx` heißt es jetzt „… oder nutze einen Partner-Code …“ statt „löse … ein“. + - Die Fehlermeldung nutzt jetzt die zweite Variante aus dem Call, nicht mehr „Einladungslink“. + - Die Schreibweise ist auf „Partner-Code“ mit Bindestrich vereinheitlicht. diff --git a/internal/llmops/testdata/eval/specimens/en-directive-ref-desc-excerpt.yaml b/internal/llmops/testdata/eval/specimens/en-directive-ref-desc-excerpt.yaml new file mode 100644 index 000000000..8d58e737e --- /dev/null +++ b/internal/llmops/testdata/eval/specimens/en-directive-ref-desc-excerpt.yaml @@ -0,0 +1,25 @@ +id: en-directive-ref-desc-excerpt +origin: the pre-flight eval case whose cited excerpt broke the JSON, 20260606-125155-s-tac-sk1 +class: ref desc and body excerpts cited in the observation +language: "" +operation: preflight +expect: + severity: high + passage: This directive retires the immutability contract d-prc-iom. +graph: + - id: 20260408-120000-d-prc-iom + type: decision + kind: contract + layer: process + content: |- + Documents in the graph are never modified after creation. Updates land as new entries that supersede or close the prior one. This immutability is the precondition for the graph as durable record. +draft: + type: decision + kind: directive + layer: process + refs: + - id: 20260408-120000-d-prc-iom + kind: grounded-in + desc: extends the immutability contract + content: |- + This directive retires the immutability contract d-prc-iom. We will allow in-place edits to entry bodies when the change is purely editorial (typo, link repair) and recorded in a `revision_history` field on the entry. The original immutability framing was correct at the time but proved too rigid for low-stakes edits that don't change semantic content. diff --git a/internal/llmops/testdata/eval/specimens/en-gap-brace-literal.yaml b/internal/llmops/testdata/eval/specimens/en-gap-brace-literal.yaml new file mode 100644 index 000000000..46aab5ba0 --- /dev/null +++ b/internal/llmops/testdata/eval/specimens/en-gap-brace-literal.yaml @@ -0,0 +1,13 @@ +id: en-gap-brace-literal +origin: GitHub #6, 20260906-231136-s-tac-ynt; the path and the regex were added +class: brace literals, a backslash path and a regex in prose +language: "" +operation: preflight +expect: + no_blocking: true +draft: + type: signal + kind: gap + layer: operational + content: |- + The config loader ignores Options{Zebra: true} when it is passed after Options{Child: Child{Enabled: true}}, so the second literal silently wins and the zebra mode never activates. The same happens when both come from C:\sdd\config\zebra.yaml, whose keys the loader matches with ^options\.(zebra|child)$. Expected is that both literals apply. diff --git a/internal/llmops/writingguide.go b/internal/llmops/writingguide.go index 1f60c680c..e3f542341 100644 --- a/internal/llmops/writingguide.go +++ b/internal/llmops/writingguide.go @@ -26,27 +26,15 @@ const ( GuideMinor GuideSeverity = "minor" ) -// GuideAxis is the closed set of writing-guide check axes (d-cpt-7zr rendered -// for the guide; see writing_guide.tmpl). Closed on purpose: stable identifiers -// are what the calibration corpus banks against — pre-flight's free-form -// categories proved untrackable across runs. -var guideAxes = map[string]bool{ - "stranding": true, - "dilution": true, - "conflation": true, - "pointing": true, - "form": true, -} +// guideAxes is the closed set of writing-guide check axes (d-cpt-7zr rendered +// for the guide; see writing_guide.tmpl), declared in the response schema. +// Closed on purpose: stable identifiers are what the calibration corpus banks +// against — pre-flight's free-form categories proved untrackable across runs. +var guideAxes = findingEnum(writingGuideSchema, "axis") // guideRepairs is the closed set of repair directions — direction only, // never replacement text. -var guideRepairs = map[string]bool{ - "cut": true, - "write-in": true, - "split": true, - "point": true, - "reword": true, -} +var guideRepairs = findingEnum(writingGuideSchema, "repair") // GuideFinding is a single writing-guide observation. Reasoning leads — // it is the finding's work; axis, repair, and severity are its conclusions. @@ -134,9 +122,6 @@ func renderWritingGuidePrompt(entry *model.Entry, closureTargets []model.Closure if err != nil { return llm.Request{}, fmt.Errorf("parsing writing-guide templates: %w", err) } - if tmpl, err = tmpl.ParseFS(sharedPromptTemplates, "shared_templates/*.tmpl"); err != nil { - return llm.Request{}, fmt.Errorf("parsing shared prompt templates: %w", err) - } sysData := struct{ TypeSystemFactID string }{TypeSystemFactID: refFacts.TypeSystemFactID} userData := struct{ Draft, Kind, KindFactID string }{ @@ -154,9 +139,10 @@ func renderWritingGuidePrompt(entry *model.Entry, closureTargets []model.Closure } return llm.Request{ - Purpose: llm.PurposeWritingGuide, - SystemPrompt: strings.TrimSpace(sysB.String()), - UserPrompt: strings.TrimSpace(userB.String()), + Purpose: llm.PurposeWritingGuide, + SystemPrompt: strings.TrimSpace(sysB.String()), + UserPrompt: strings.TrimSpace(userB.String()), + ResponseSchema: writingGuideSchema, }, nil } @@ -212,12 +198,9 @@ func formatDraftForWritingGuide(e *model.Entry, closureTargets []model.ClosureTa return b.String() } -// parseWritingGuideResult parses the guide's JSON response. The LLM is asked -// to respond with: -// -// {"findings": [{"reasoning": "...", "axis": "...", "quote": "...", "repair": "...", "severity": "substantive|minor"}]} -// -// Empty findings array means the draft passed clean. Tolerates prose around +// parseWritingGuideResult parses the guide's JSON response, shaped by +// response_schemas/writing_guide.json. Empty findings array means the draft +// passed clean. Tolerates prose around // the JSON object (shared extractJSONObject); malformed JSON, values outside // the closed axis/repair/severity sets, and empty required fields are errors, // so infrastructure failures stay distinct from findings. diff --git a/internal/llmops/writingguide_eval_test.go b/internal/llmops/writingguide_eval_test.go index d61492383..33987018c 100644 --- a/internal/llmops/writingguide_eval_test.go +++ b/internal/llmops/writingguide_eval_test.go @@ -138,6 +138,20 @@ func TestWritingGuideEval_StrandedDraftYieldsStranding(t *testing.T) { }) } +// TestWritingGuideEval_Specimens runs the writing-guide specimens +// (specimen_eval_test.go). GitHub #21's case is de-done-quotes-german. +func TestWritingGuideEval_Specimens(t *testing.T) { + for _, s := range loadSpecimens(t, "writing-guide") { + t.Run(s.ID, func(t *testing.T) { + t.Parallel() + draft := s.Draft.entry(t) + runGuideEvalPassRate(t, draft, s.Expect.tier(), func(r *llmops.WritingGuideResult) error { + return s.Expect.checkGuide(draft.Content, r) + }) + }) + } +} + // TestWritingGuideEval_ConflatedDraftYieldsConflation pins the split test: // two commitments with independent lifecycles fused into one body. func TestWritingGuideEval_ConflatedDraftYieldsConflation(t *testing.T) { diff --git a/internal/llmops/writingguide_internal_test.go b/internal/llmops/writingguide_internal_test.go index 4ff2693d0..8217e127c 100644 --- a/internal/llmops/writingguide_internal_test.go +++ b/internal/llmops/writingguide_internal_test.go @@ -129,22 +129,6 @@ func TestRenderWritingGuidePrompt_SystemByteStableAcrossDrafts(t *testing.T) { } } -func TestRenderWritingGuidePrompt_JSONStringRulesShared(t *testing.T) { - // Regression for 20260827-224853-s-tac-giv: the output format teaches - // standard JSON escaping, never quote avoidance. - e := &model.Entry{Type: model.TypeSignal, Kind: model.KindGap, Layer: model.LayerTactical, Content: "Body."} - req, err := renderWritingGuidePrompt(e, nil, testReferenceFacts("")) - if err != nil { - t.Fatal(err) - } - if !strings.Contains(req.SystemPrompt, "**JSON strings.**") || !strings.Contains(req.SystemPrompt, "write a literal `\"` as `\\\"`") { - t.Errorf("system prompt missing the shared JSON escaping rule:\n%s", req.SystemPrompt) - } - if strings.Contains(req.SystemPrompt, "never a literal") { - t.Error("system prompt still teaches quote avoidance") - } -} - // TestRenderWritingGuidePrompt_ReferenceFacts pins the framework-knowledge // slots: the type-system overview renders into the system block with its // headings demoted under the section, and the drafted kind's authoring fact diff --git a/internal/llmops/writingguide_templates/writing_guide.tmpl b/internal/llmops/writingguide_templates/writing_guide.tmpl index 56ac7b2e0..8abd36cd1 100644 --- a/internal/llmops/writingguide_templates/writing_guide.tmpl +++ b/internal/llmops/writingguide_templates/writing_guide.tmpl @@ -36,33 +36,23 @@ Check only these axes: **form** — the first sentence works alone as a summary of the entry. -## Output format +## Findings -Respond with a single JSON object matching this schema — nothing before or after, no code fences: +Each finding carries: -{ - "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` — one to three sentences working through what you noticed and why it fails the pull, ending in a conclusion +- `axis` — the check it fails: `stranding`, `dilution`, `conflation`, `pointing` or `form` +- `quote` — words from the draft that locate the passage at issue +- `repair` — the direction of the fix, never replacement text: `cut`, `write-in`, `split`, `point` or `reword` +- `severity` — `substantive` when the drafting dialogue should take the finding up, `minor` when folding it in or ignoring it is fine -**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. +**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 rather than one 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. - -{{ template "json_string_rules" }} - -**Quoting.** `quote` carries the draft's words verbatim, escaped as JSON requires. +**Quoting.** The quote points the drafting agent, who holds the draft, to the passage. Take enough of the draft's own words to locate it unambiguously — a few distinctive words do — and take them from the draft, never from a ref or a closure edge. They need not match the draft character for character. For a draft containing "The release procedure gains this as a fixed section of its notes template.", the quote `gains this as a fixed section` locates the passage. ## 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 — return an empty list 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" -}} diff --git a/internal/model/config.go b/internal/model/config.go index ba9bd285a..c4f490a33 100644 --- a/internal/model/config.go +++ b/internal/model/config.go @@ -243,8 +243,9 @@ type LLMConfig struct { // Concurrency bounds the worker pool for batch operations. Zero means // "use DefaultLLMConcurrency". Concurrency int `yaml:"concurrency,omitempty"` - // Endpoint overrides the OpenAI-compatible base URL for the openai - // provider. Empty defaults to the OpenAI public endpoint. Useful for a + // Endpoint overrides the base URL of the openai provider (OpenAI chat + // completions protocol) or the anthropic provider (Anthropic Messages + // API). Empty defaults to the vendor's public endpoint. Useful for a // gateway or a self-hosted service that implements the same protocol. Endpoint string `yaml:"endpoint,omitempty"` // OllamaEndpoint overrides the default Ollama URL for the gollm adapter. diff --git a/internal/proctest/capture_test.go b/internal/proctest/capture_test.go index 720be2d42..214ebd943 100644 --- a/internal/proctest/capture_test.go +++ b/internal/proctest/capture_test.go @@ -364,6 +364,26 @@ func TestCapture_WritingGuideFindingsServeReviewThenProceed(t *testing.T) { proctest.RequireStep(t, serve, "playback") } +// A guide quote locates its passage and need not match it +// (20261005-170145-d-tac-gch); the serve tells the drafting agent so, and +// renders a quote with characters the draft does not have as it came. +func TestCapture_WritingGuideQuoteLocatesThePassage(t *testing.T) { + world, session := newCaptureWorld(t, "core-guide-quote") + world.LLM.GuideFindings = []proctest.GuideFinding{{ + Reasoning: "the passage names a button label without saying where it appears", + Axis: "stranding", Quote: `von "Code einlösen"`, Repair: "write-in", Severity: "minor", + }} + + serve := session.Start(t, "capture", nil) + serve = session.Report(t, serve.Instance, captureDraft()) + proctest.RequireStep(t, serve, "guideReview") + for _, want := range []string{`von "Code einlösen"`, "locate the passage from it instead of matching it"} { + if !strings.Contains(serve.Instructions, want) { + t.Errorf("guideReview unit missing %q:\n%s", want, serve.Instructions) + } + } +} + func TestCapture_WritingGuideRunsOncePerCapture(t *testing.T) { world, session := newCaptureWorld(t, "core-guide-once") world.LLM.GuideFindings = []proctest.GuideFinding{{ diff --git a/pkg/llm/llm.go b/pkg/llm/llm.go index 606f66a7e..bf8a9d6de 100644 --- a/pkg/llm/llm.go +++ b/pkg/llm/llm.go @@ -10,7 +10,10 @@ // because implementations may route. package llm -import "context" +import ( + "context" + "encoding/json" +) // Purpose names what a call is for: a routing key for implementations and an // observability dimension, the same value for both so they cannot drift. The @@ -25,13 +28,19 @@ const ( ) // Request carries everything an implementation may route on: ctx (who) and -// Purpose (what for), plus the two-part prompt. +// Purpose (what for), plus the two-part prompt and the shape of the answer. type Request struct { Purpose Purpose // SystemPrompt is the stable prefix, cacheable by providers that can. SystemPrompt string // UserPrompt is the per-call variable part. UserPrompt string + // ResponseSchema is the JSON Schema Result.Text must conform to; nil asks + // for free text. An implementation enforces it through the provider's + // native structured output, never by asking for JSON in the prompt + // (20261004-134853-d-cpt-sw8). The bytes go to the provider as given: + // constrained decoding generates properties in schema order. + ResponseSchema json.RawMessage } // Combined returns SystemPrompt followed by UserPrompt separated by a blank