Repository navigation
feat: expose the upstream provider on chat responses and error metadata - #55
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughChat responses now expose an optional upstream provider name. Error decoding now accepts structured metadata values and represents non-string, non-null values as compact JSON strings. ChangesResponse and Error Metadata
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~12 minutes Change: Feature Merge Risk: ⚪ Minimal · up to The provider and error-metadata changes appear mergeable after normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The new fields make more upstream data available to callers. No security failure is established, but applications should not treat provider names or error metadata as verified or safe to log without review. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 4 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit reads the provider name, Comment |
OpenRouter puts a top-level `provider` string (Groq, Cerebras, ...) on chat completion responses, naming the upstream that served the call. `StreamingDelta` already decoded it, but `ChatResponse` did not, so non-streaming callers had no way to record which provider they got. The field is optional. OpenRouter's OpenAPI schema for ChatResult does not list it, so responses that omit it still decode. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
OpenRouter types `error.metadata` as `Record<string, unknown>`. Moderation errors carry `reasons: string[]`, guardrail blocks carry `patterns: string[]`, and provider errors carry the upstream's verbatim payload in `raw`, which is often an object. `ErrorDetail` decoded metadata as `[String: String]`, so any one of those values made the whole `ErrorResponse` decode throw. HTTPClient swallows that with `try?`, so the caller got "Unknown error occurred", `metadata == nil`, and no way to read `provider_name` short of parsing `rawResponseBody`. Metadata now decodes as arbitrary JSON and is flattened into the existing `[String: String]` shape. Strings pass through unchanged, other values become compact sorted-key JSON text, and nulls are dropped. The public type of `OpenRouterError.metadata` is unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
ad7aeb3 to
4c042b2
Compare
Lets callers record which upstream provider (Groq, Cerebras, Baseten, ...) served an OpenRouter call. maia-server needs this before it can add a
providercolumn toopenrouter_calls.Changes
ChatResponse.provider: String?: decodes OpenRouter's top-levelproviderstring, following the existingStreamingDelta.provider. The field is optional, so responses without it still decode. OpenRouter's OpenAPIChatResultschema doesn't list the field, but its mid-stream error chunk type does, and the streaming path already relies on it.OpenRouterError.metadatacould not survive real error bodies. OpenRouter typeserror.metadataasRecord<string, unknown>. Moderation errors carryreasons: string[], guardrail blocks carrypatterns: string[], and provider errors put the upstream's verbatim payload inraw, often as an object.ErrorDetaildecoded metadata as[String: String], so any one of those values made the wholeErrorResponsedecode throw.HTTPClientswallows that failure withtry?, so the caller got"Unknown error occurred"andmetadata == nil, andprovider_namewas unreachable. Metadata now decodes as arbitrary JSON and is flattened into the same[String: String]. Strings pass through, other values become compact sorted-key JSON text, and nulls are dropped. The public type is unchanged, so this is not a breaking change. Before the fix, both new tests failed withDecodingError.typeMismatch ... Path: error.metadata.reasons(and.attempt).Audio transcription: no provider to expose
AudioTranscriptionResponseis unchanged. OpenRouter's OpenAPI spec forPOST /audio/transcriptions(STTResponse) definestext,usage, and theverbose_jsonextras (task,language,duration,segments,words,confidence), with no provider field. The spec also documents no response headers for the endpoint. The onlyX-headers anywhere in the spec are request headers. I did not call the live endpoint to check for undocumented fields, because that spends a real key. If maia-server needs the transcription provider, confirming it will take one authorized live call, or a lookup through/generation, though that needs a generation id the transcription response doesn't return either.Review guide
Read closely
Sources/OpenRouterKit/Errors/OpenRouterError.swift: the customErrorDetail.init(from:)and themetadataStringflattening.Skim
Sources/OpenRouterKit/Models/Chat/ChatResponse.swift: one optional stored property, synthesizedCodable.Tests/OpenRouterKitTests/ChatResponseProviderTests.swift: unit tests decode with and withoutprovider, and a live integration test asserts that a real chat completion carries a non-emptyprovider. Both follow theUsageCostTestspattern.Tests/OpenRouterKitTests/OpenRouterErrorTests.swift: two fixtures modeled on the documented provider-error and moderation-error shapes.README.md: two short paragraphs.Verification
swift build: clean.swift test --parallellocally: 193 tests in 23 suites passed. Live integration suites were skipped becauseOPENROUTER_API_KEYis not set on the dev box.provider, since the OpenAPI schema doesn't list it.swiftlint(ghcr.io/realm/swiftlint image): no violations in changed files. The only warnings are two that already existed inNIOHTTPClient.swift:114andChatRequestParametersTests.swift:567.🤖 Generated with Claude Code