Update OpenAI Chat API #411
Conversation
- Add recommendations for using `Kotest-assertions-json` for JSON serialization testing
- Add structured content parts (`ContentPart`) for handling multimodal messages (text, images, and audio). - Introduce `MessageContent` abstraction for unified content representation (string or parts array). - Enhance serialization/deserialization logic for `MessageContent`. - Update affected builders, tests, and serializers to reflect the new structure. - Improve request body matchers for embeddings to support flexible input criteria.
- Replace manual formatting with `toLogString` for improved consistency in unmatched request logging. - Simplify verbose and failure log messages for better clarity.
kpavlov
added
enhancement
|
Caution Review failedThe pull request is closed. 📝 WalkthroughSummary by CodeRabbit
WalkthroughMessage.content changed from String to a structured MessageContent (with serializer and parts). Embeddings request spec moved to matcher-based lists with inputContains helpers. MockOpenai, builders, serializers, tests, docs, and logging/build flags updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Test as Test Code
participant Builder as ChatCompletionRequestBuilder
participant Model as Message/MessageContent
participant Serializer as MessageContentSerializer
Test->>Builder: addMessage(role, "hello")
Builder->>Model: create Message(role, MessageContent.Text("hello"))
Note right of Model #e3f2fd: New structured content type
Model->>Serializer: serialize content
alt Text content
Serializer-->>Test: JSON string "hello"
else Parts content
Serializer-->>Test: JSON array of ContentPart
end
sequenceDiagram
autonumber
actor Test as Test Code
participant Spec as OpenaiEmbedRequestSpecification
participant Matchers as OpenaiEmbeddingsMatchers
participant Server as MockOpenai Embeddings
Test->>Spec: inputContains("Hello")
Spec->>Matchers: build Matcher<CreateEmbeddingsRequest?>
Server->>Spec: evaluate matcher(s) against incoming request
alt any matcher matches
Server-->>Test: return configured embeddings response
else no matcher matches
Server-->>Test: 404 Not Found
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (5)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ai-mocks-openai/src/commonMain/kotlin/me/kpavlov/aimocks/openai/model/chat/ChatCompletionModels.kt (1)
478-480: Update delta content to structured type
Delta.contentremainsString?, so streaming responses that now ship content arrays (the very case this PR introduces for messages) will fail to deserialize with aSerializationException. Please align it withMessageContent(and reuseMessageContentSerializer) so both string and array payloads deserialize correctly.- val content: String? = null, + @Serializable(MessageContentSerializer::class) + val content: MessageContent? = null,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (15)
GUIDELINES.md(1 hunks)ai-mocks-openai/src/commonMain/kotlin/me/kpavlov/aimocks/openai/MockOpenai.kt(3 hunks)ai-mocks-openai/src/commonMain/kotlin/me/kpavlov/aimocks/openai/Model.kt(2 hunks)ai-mocks-openai/src/commonMain/kotlin/me/kpavlov/aimocks/openai/completions/OpenaiChatCompletionsBuildingStep.kt(2 hunks)ai-mocks-openai/src/commonMain/kotlin/me/kpavlov/aimocks/openai/embeddings/OpenaiEmbedRequestSpecification.kt(6 hunks)ai-mocks-openai/src/commonMain/kotlin/me/kpavlov/aimocks/openai/embeddings/OpenaiEmbeddingsMatcher.kt(0 hunks)ai-mocks-openai/src/commonMain/kotlin/me/kpavlov/aimocks/openai/embeddings/OpenaiEmbeddingsMatchers.kt(1 hunks)ai-mocks-openai/src/commonMain/kotlin/me/kpavlov/aimocks/openai/model/chat/ChatCompletionModels.kt(4 hunks)ai-mocks-openai/src/commonMain/kotlin/me/kpavlov/aimocks/openai/model/chat/ChatCompletionRequestBuilder.kt(1 hunks)ai-mocks-openai/src/commonTest/kotlin/me/kpavlov/aimocks/openai/completions/OpenaiCompletionsMatchersTest.kt(4 hunks)ai-mocks-openai/src/commonTest/kotlin/me/kpavlov/aimocks/openai/model/chat/ChatCompletionModelsTest.kt(9 hunks)ai-mocks-openai/src/jvmTest/kotlin/me/kpavlov/aimocks/openai/official/embeddings/EmbeddingsOpenaiTest.kt(3 hunks)buildSrc/src/main/kotlin/kotlin-convention.gradle.kts(1 hunks)docs/content/docs/ai-mocks/openai.md(5 hunks)mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/RequestHandler.kt(2 hunks)
💤 Files with no reviewable changes (1)
- ai-mocks-openai/src/commonMain/kotlin/me/kpavlov/aimocks/openai/embeddings/OpenaiEmbeddingsMatcher.kt
🧰 Additional context used
🧬 Code graph analysis (1)
ai-mocks-openai/src/jvmTest/kotlin/me/kpavlov/aimocks/openai/official/embeddings/EmbeddingsOpenaiTest.kt (3)
ai-mocks-openai/src/commonMain/kotlin/me/kpavlov/aimocks/openai/embeddings/OpenaiEmbedRequestSpecification.kt (1)
inputContains(117-119)ai-mocks-openai/src/commonMain/kotlin/me/kpavlov/aimocks/openai/embeddings/OpenaiEmbeddingsMatchers.kt (1)
inputContains(8-22)ai-mocks-openai/src/commonMain/kotlin/me/kpavlov/aimocks/openai/MockOpenai.kt (2)
embeddings(224-228)embeddings(267-312)
🪛 GitHub Check: Codacy Static Code Analysis
ai-mocks-openai/src/commonMain/kotlin/me/kpavlov/aimocks/openai/model/chat/ChatCompletionModels.kt
[warning] 248-248: ai-mocks-openai/src/commonMain/kotlin/me/kpavlov/aimocks/openai/model/chat/ChatCompletionModels.kt#L248
Method (anonymous) has 61 lines of code (limit is 50)
🪛 markdownlint-cli2 (0.18.1)
GUIDELINES.md
79-79: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build
🔇 Additional comments (20)
mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/RequestHandler.kt (1)
6-6: LGTM: Necessary import for Ktor logging utility.The import is required for the
toLogString()extension method used in the logging changes below.ai-mocks-openai/src/commonMain/kotlin/me/kpavlov/aimocks/openai/MockOpenai.kt (3)
81-103: LGTM: Variable rename improves clarity.The rename from
chatRequestSpectorequestSpecis cleaner and all references are updated consistently. This refactor maintains the same behavior while improving code readability.
237-260: Excellent documentation enhancement.The new KDoc examples clearly demonstrate both single-string and list-based embedding scenarios, including the new
inputContainsmatcher. This will significantly improve developer experience.
304-305: Good alignment with matcher-based validation.The switch from iterating
requestBodyStringelements to directly usingrequestBodyandrequestBodyStringcollections aligns with the new matcher-based approach and maintains consistency with the chat completions path.ai-mocks-openai/src/commonMain/kotlin/me/kpavlov/aimocks/openai/completions/OpenaiChatCompletionsBuildingStep.kt (2)
21-21: LGTM: Import added for new content type.The
MessageContentimport is required for the structured content wrapper used at line 102.
102-102: Correct usage of MessageContent.Text wrapper.The assistant content is properly wrapped in
MessageContent.Text(...)to align with the newMessage.contenttype. This maintains support for simple text responses while enabling structured content in the future.ai-mocks-openai/src/commonMain/kotlin/me/kpavlov/aimocks/openai/model/chat/ChatCompletionRequestBuilder.kt (2)
61-67: LGTM: Backward-compatible string overload.The existing
addMessage(role, String)method now wraps the string inMessageContent.Text, maintaining backward compatibility while delegating to the new structured content model.
69-82: Excellent API design with structured content support.The new
addMessage(role, MessageContent)overload enables structured message content (text, images, audio, etc.) while the string overload maintains convenience for simple text messages. This dual approach provides both flexibility and ease of use.ai-mocks-openai/src/jvmTest/kotlin/me/kpavlov/aimocks/openai/official/embeddings/EmbeddingsOpenaiTest.kt (3)
4-4: LGTM: Imports added for error scenario testing.The new imports (
NotFoundException,shouldContain,assertThrows) are required for the 404 error test added at lines 60-86.Also applies to: 11-11, 16-16
27-28: Good test coverage of inputContains matcher.Adding both partial (
"Hello") and full (input) substring matches demonstrates the flexibility of the newinputContainsmatcher and validates that the feature works correctly.
60-86: Excellent negative scenario coverage.This test validates that when the embeddings request input doesn't match the mock criteria (e.g.,
inputContains("Hello2")vs actual input"Hello world"), aNotFoundExceptionis thrown with a 404 status. This ensures proper error handling for unmatched requests.ai-mocks-openai/src/commonTest/kotlin/me/kpavlov/aimocks/openai/completions/OpenaiCompletionsMatchersTest.kt (2)
7-7: LGTM: Import added for new content type.The
MessageContentimport is required to wrap message content in the test data.
19-19: Tests correctly updated for MessageContent type.The test cases properly wrap message content in
MessageContent.Text(...)to align with the newMessage.contenttype. The existing matchers (systemMessageContains,userMessageContains) continue to work correctly, indicating they handle the MessageContent abstraction appropriately.Also applies to: 44-44, 65-65
ai-mocks-openai/src/commonMain/kotlin/me/kpavlov/aimocks/openai/Model.kt (1)
15-15: LGTM: Import added for new content type.The
MessageContentimport is required for the updatedMessage.contentproperty type.docs/content/docs/ai-mocks/openai.md (6)
115-140: Excellent addition: Custom Error Response documentation.This new section demonstrates how to mock custom error responses with non-standard HTTP status codes and body structures. The example is clear and practical for testing custom error handling scenarios.
142-168: LGTM: OpenAI-compatible error format documented.The updated example correctly demonstrates the standard OpenAI error response format with a nested
errorobject, distinguishing it from custom error responses.
202-280: Excellent streaming documentation expansion.The new documentation clearly distinguishes between list-based chunking (simple) and flow-based streaming (flexible), with a complete client-side example showing how to consume the stream. This significantly improves developer experience.
365-365: Good documentation of inputContains feature.The new "Advanced Input Matching" section clearly explains the
inputContains()matcher for substring-based request matching, with practical examples showing multiple matchers.Also applies to: 433-445
454-454: LGTM: Error example simplified.The updated error example uses
stringInput("boom")for conciseness and consistency with the test suite.
476-555: Excellent new section: Moderations API documentation.This comprehensive new section documents the OpenAI Moderations API support with clear examples covering both successful moderation responses and error scenarios. The examples follow established patterns and provide good coverage of the API features.
...penai/src/commonMain/kotlin/me/kpavlov/aimocks/openai/embeddings/OpenaiEmbeddingsMatchers.kt
Show resolved
Hide resolved
| * According to the OpenAI specification, the content field can be either: | ||
| * - A string for simple text messages | ||
| * - An array of content parts for multimodal messages (text, images, audio, etc.) | ||
| * | ||
| * @property role The role of the message author (system, user, assistant, or tool). | ||
| * @property content The content of the message. | ||
| * @property content The content of the message as either a string or array of content parts. | ||
| * @property refusal The refusal message if the model refused to generate a response. | ||
| * @property toolCalls The tool calls generated by the model. | ||
| * @see <a href="https://platform.openai.com/docs/api-reference/chat/create#chat-create-messages">Messages</a> | ||
| */ | ||
| @Serializable | ||
| public data class Message( | ||
| val role: ChatCompletionRole, | ||
| val content: String, | ||
| @Serializable(me.kpavlov.aimocks.openai.model.chat.MessageContentSerializer::class) | ||
| val content: MessageContent, |
There was a problem hiding this comment.
Important public API change: Message.content now uses MessageContent.
This change transitions from a plain String to a structured MessageContent type, enabling support for multimodal messages (text, images, audio, etc.) as per the OpenAI specification. The MessageContentSerializer annotation ensures proper JSON serialization.
Note: This is a breaking change for code that directly accesses Message.content expecting a String. Users will need to use .asText() or handle the MessageContent type appropriately.
🤖 Prompt for AI Agents
In ai-mocks-openai/src/commonMain/kotlin/me/kpavlov/aimocks/openai/Model.kt
around lines 248 to 262, the public API change replaced Message.content: String
with Message.content: MessageContent which breaks callers; add a short
compatibility helper and documentation: implement a public convenience function
or property on Message (e.g., fun asText(): String or val contentText: String)
that returns content.asText() and mark it @Deprecated("Use Message.content and
MessageContent APIs", ReplaceWith("content.asText()")) to ease migration, update
the KDoc on Message to mention the breaking change and the new helper, and add a
note to the CHANGELOG/migration docs explaining callers should switch to
Message.content.asText() or handle MessageContent directly.
...ks-openai/src/commonMain/kotlin/me/kpavlov/aimocks/openai/model/chat/ChatCompletionModels.kt
Show resolved
Hide resolved
| // Enable debug symbols and line number information | ||
| freeCompilerArgs.addAll( | ||
| "-Xdebug", | ||
| ) |
There was a problem hiding this comment.
Do not ship -Xdebug — it floods compiler logs without adding symbols.
-Xdebug only enables the compiler’s internal debug logging. On Kotlin 2.0 it is extremely verbose, slows builds, and still doesn’t affect bytecode debug symbols (those are already emitted by default). Please drop this flag instead of merging it.
🤖 Prompt for AI Agents
In buildSrc/src/main/kotlin/kotlin-convention.gradle.kts around lines 39 to 42,
the build script adds the compiler flag "-Xdebug" which only enables compiler
internal debug logging (very verbose and not needed) — remove the "-Xdebug"
entry from freeCompilerArgs so the flag is not passed to the Kotlin compiler;
ensure no other place reintroduces this flag and run a quick build to confirm
logs and build time improve.
| Remove obvious cases for simplicity. | ||
| - If multiple assertions are maid against nullable field, first check for null, e.g.: `params shoulNotBeNull { params.id shouldBe 1 }` | ||
| - If multiple assertions exist against nullable field, first check for null, e.g.: `params shoulNotBeNull { params.id shouldBe 1 }` | ||
| - For testing json serialization use [Kotest-assertions-json](https://kotest.io/docs/assertions/json/json-overview.html) |
There was a problem hiding this comment.
Trim trailing space flagged by markdownlint
markdownlint (MD009) reports a trailing space on this line; please remove it to keep the guidelines lint-clean.
Based on static analysis hints.
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
79-79: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
🤖 Prompt for AI Agents
In GUIDELINES.md around line 79, there is a trailing space at the end of the
line "For testing json serialization use
[Kotest-assertions-json](https://kotest.io/docs/assertions/json/json-overview.html)
"; remove the trailing whitespace so the line ends immediately after the closing
parenthesis to satisfy markdownlint MD009.
mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/RequestHandler.kt
Outdated
Show resolved
Hide resolved
| "No matched mapping for request:\n---\n${request.toLogString()}\n---", | ||
| ) | ||
| } | ||
| failure("No matched mapping for request: ${formatter.formatRequest(request)}") | ||
| failure("No matched mapping for request: ${request.toLogString()}") |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
Verify consistency between toLogString() and formatter.formatRequest().
The changes introduce an inconsistency in request formatting:
- Verbose logging (lines 43, 74, 108): uses
formatter.formatRequest(request)(custom formatter) - Non-verbose logging (lines 79, 82): uses
request.toLogString()(Ktor built-in)
Ensure this is intentional and that the different formats won't confuse users. If the formats differ significantly, consider either:
- Using a consistent format across verbose and non-verbose modes, or
- Documenting why different formats are used in different contexts.
Run the following script to examine the HttpFormatter.formatRequest() implementation to understand how it differs from Ktor's toLogString():
🏁 Script executed:
#!/bin/bash
# Description: Examine the HttpFormatter.formatRequest implementation to understand formatting differences.
# Find the HttpFormatter implementation
rg -n -A 20 'class HttpFormatter|interface HttpFormatter' --type kotlin
# Find formatRequest method implementation
rg -n -A 15 'fun formatRequest' --type kotlinLength of output: 4340
Unify or clarify request logging formats
Verbose logs use formatter.formatRequest(request) (full request line, headers, body), while non-verbose logs and failures use request.toLogString() (method + URI only). Confirm this difference is intentional or switch to a single formatter (or document the rationale for using two).
🤖 Prompt for AI Agents
In mokksy/src/commonMain/kotlin/me/kpavlov/mokksy/RequestHandler.kt around lines
79 to 82, the verbose log uses formatter.formatRequest(request) while the
failure log uses request.toLogString(), producing inconsistent request
representations; choose one approach and make it consistent (recommended: use
formatter.formatRequest(request) everywhere for full request line/headers/body)
or, if the shorter form is intended for failures, add a clear inline comment
documenting that rationale; then update the failure(...) call (or the verbose
log) to use the chosen formatter and adjust the surrounding log messages so both
code paths emit the same request format.
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
No description provided.