Feature 08 task 1: Provider interface, typed errors and Fake provider #33

Member

Implements Feature 08 task 1 (53fb6507). Done-condition: internal/ai/llm defines Provider (Generate/ListModels/Verify), Request/Response/Model types and typed ErrAuth/ErrRateLimited(RetryAfter)/ErrTimeout/ErrBadOutput/ErrProvider(status+redacted excerpt) usable with errors.Is/As, plus a scripted Fake provider that records requests; tests cover wrapping/matching and Fake scripting; quality floor passes.

Evidence (developer-run): gofmt empty; go vet clean; go test -race all packages pass; CGO_ENABLED=0 build ok; internal/ai/llm 100% coverage. Standard library only.

Implements Feature 08 task 1 (53fb6507). Done-condition: internal/ai/llm defines Provider (Generate/ListModels/Verify), Request/Response/Model types and typed ErrAuth/ErrRateLimited(RetryAfter)/ErrTimeout/ErrBadOutput/ErrProvider(status+redacted excerpt) usable with errors.Is/As, plus a scripted Fake provider that records requests; tests cover wrapping/matching and Fake scripting; quality floor passes. Evidence (developer-run): gofmt empty; go vet clean; go test -race all packages pass; CGO_ENABLED=0 build ok; internal/ai/llm 100% coverage. Standard library only.
dev_agent added 1 commit 2026-10-03 15:53:10 -04:00
Add LLM Provider interface, typed errors and Fake provider
Independent review Codex gpt-6-sol high: CHANGES_REQUIRED (1 high)
CI / ci (push) Successful in 1m28s
CI / ci (pull_request) Successful in 1m28s
cb925618bc
internal/ai/llm defines the single Provider interface (Generate,
ListModels, Verify) shared by every vendor adapter, with Request,
Message, JSONSchema, Response, Usage and Model types.

Typed errors work with errors.Is/As: sentinels ErrAuth, ErrTimeout and
ErrBadOutput; *RateLimitedError (RetryAfter) matching ErrRateLimited;
*ProviderError (status and body excerpt) matching ErrProvider.
NewProviderError redacts every given secret before truncating the body,
so no part of an API key reaches an error string.

Fake is a thread-safe scripted Provider for tests: it returns queued
results in order, fails unscripted calls, and records deep copies of
every Request.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
review_agent left a comment
Member

Independent review — Codex gpt-6-sol, reasoning high

Verdict: CHANGES_REQUIRED
Reviewed SHA: cb925618bc4881851c2032571dc0ccce0a2f8af3

{
  "verdict": "CHANGES_REQUIRED",
  "reviewed_sha": "cb925618bc4881851c2032571dc0ccce0a2f8af3",
  "summary": "The Provider API, Fake scripting and request copies meet the task requirements. Formatting, vet, and race tests pass. Provider error construction does not guarantee the required secret redaction.",
  "findings": [
    {
      "id": "F1",
      "severity": "high",
      "location": "internal/ai/llm/errors.go:63",
      "failure_case": "If a provider response echoes an API key and the caller omits the optional secrets argument, ProviderError.Error() and Body() expose the key.",
      "evidence": "NewProviderError accepts secrets as a variadic optional argument and otherwise stores the response body unchanged. Existing tests call it without a secret.",
      "suggested_correction": "Make redaction mandatory before storing a body excerpt. If the key is unavailable, omit the excerpt; add a test for that case."
    }
  ]
}

Posted unaltered by review_agent. This is transport, not a second review.

## Independent review — Codex `gpt-6-sol`, reasoning high Verdict: **CHANGES_REQUIRED** Reviewed SHA: `cb925618bc4881851c2032571dc0ccce0a2f8af3` ```json { "verdict": "CHANGES_REQUIRED", "reviewed_sha": "cb925618bc4881851c2032571dc0ccce0a2f8af3", "summary": "The Provider API, Fake scripting and request copies meet the task requirements. Formatting, vet, and race tests pass. Provider error construction does not guarantee the required secret redaction.", "findings": [ { "id": "F1", "severity": "high", "location": "internal/ai/llm/errors.go:63", "failure_case": "If a provider response echoes an API key and the caller omits the optional secrets argument, ProviderError.Error() and Body() expose the key.", "evidence": "NewProviderError accepts secrets as a variadic optional argument and otherwise stores the response body unchanged. Existing tests call it without a secret.", "suggested_correction": "Make redaction mandatory before storing a body excerpt. If the key is unavailable, omit the excerpt; add a test for that case." } ] } ``` --- Posted unaltered by `review_agent`. This is transport, not a second review.
dev_agent added 1 commit 2026-10-03 15:56:55 -04:00
Require secrets for ProviderError redaction and omit unredactable bodies
Independent review Codex gpt-6-sol high: APPROVED (0 findings)
CI / ci (push) Successful in 1m28s
CI / ci (pull_request) Successful in 1m27s
518e78db54
NewProviderError now takes the secrets to redact as a required []string
instead of an optional variadic. When no non-empty secret is given the
body cannot be redacted, so no excerpt is stored and Body()/Error() never
expose raw vendor output.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
review_agent left a comment
Member

Independent review — Codex gpt-6-sol, reasoning high

Verdict: APPROVED
Reviewed SHA: 518e78db54058b15e4909538a1b31b92a148a0a8

{
  "verdict": "APPROVED",
  "reviewed_sha": "518e78db54058b15e4909538a1b31b92a148a0a8",
  "summary": "The round 1 redaction issue is fixed: ProviderError omits the body when no nonempty secret is supplied. The required types, error matching, Fake scripting and request copying are present. gofmt, go vet, and go test -race -count=1 ./... passed.",
  "findings": []
}

Posted unaltered by review_agent. This is transport, not a second review.

## Independent review — Codex `gpt-6-sol`, reasoning high Verdict: **APPROVED** Reviewed SHA: `518e78db54058b15e4909538a1b31b92a148a0a8` ```json { "verdict": "APPROVED", "reviewed_sha": "518e78db54058b15e4909538a1b31b92a148a0a8", "summary": "The round 1 redaction issue is fixed: ProviderError omits the body when no nonempty secret is supplied. The required types, error matching, Fake scripting and request copying are present. gofmt, go vet, and go test -race -count=1 ./... passed.", "findings": [] } ``` --- Posted unaltered by `review_agent`. This is transport, not a second review.
approver_agent merged commit 72042516e2 into feature/5dd6e253-cc9b-41f3-9091-bb834f7a373a-llm-provider-layer 2026-10-03 16:14:13 -04:00
Sign in to join this conversation.
No Reviewers
No labels
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Maniacs_Mansion/MealQuest#33