feat: add structured output support (added in ollama 0.5.0) #92
Conversation
WalkthroughThis pull request updates the OpenAPI specification for the Ollama API by replacing the simple Changes
Sequence Diagram(s)sequenceDiagram
participant T as Test Framework
participant CC as Chat Client
participant S as Server/API
T->>CC: Set response format with JSON schema
T->>CC: Send query ("current temperature in Dubai")
CC->>S: API request with structured payload
S-->>CC: Return structured response (temperature, unit, location)
CC-->>T: Deliver deserialized QueryResponse
T->>T: Assert temperature and location values
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/tests/Ollama.IntegrationTests/Test.StructuredOutputInChat.cs (3)
17-20: Make the system message more specific to weather queries.The system message could be more specific to guide the model's behavior for weather-related queries.
- systemMessage: "You are a helpful weather assistant." + systemMessage: "You are a weather assistant. Provide accurate temperature information in the requested format. Always respond with temperature in the specified unit and include the full location name."
22-39: Consider using a schema generation library or moving schema to a resource file.The JSON schema is currently hardcoded as a string. Consider:
- Using NewtonSoft.Json.Schema for auto-generation from the
QueryResponseclass.- Moving the schema to a resource file for better maintainability.
Example using NewtonSoft.Json.Schema:
using Newtonsoft.Json.Schema.Generation; // Generate schema from QueryResponse class var generator = new JSchemaGenerator(); var schema = generator.Generate(typeof(QueryResponse)); chat.ResponseFormat = new ResponseFormat(schema);
58-70: Add XML documentation and validation attributes to types.The types could benefit from:
- XML documentation for better IntelliSense support.
- Data validation attributes.
[JsonConverter(typeof(JsonStringEnumConverter))] + /// <summary> + /// Represents the unit of temperature measurement. + /// </summary> public enum TemperatureUnit { + /// <summary> + /// Temperature in Celsius. + /// </summary> Celsius, + /// <summary> + /// Temperature in Fahrenheit. + /// </summary> Fahrenheit } + /// <summary> + /// Represents a response to a temperature query. + /// </summary> public class QueryResponse { + /// <summary> + /// Gets or sets the temperature value. + /// </summary> + [Range(-50, 60)] public int Temperature { get; set; } + /// <summary> + /// Gets or sets the temperature unit. + /// </summary> + [Required] public TemperatureUnit Unit { get; set; } + /// <summary> + /// Gets or sets the location name. + /// </summary> + [Required] + [MinLength(2)] public string Location { get; set; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (16)
src/libs/Ollama/Generated/JsonConverters.ResponseFormat.g.csis excluded by!**/generated/**src/libs/Ollama/Generated/JsonConverters.ResponseFormatEnum.g.csis excluded by!**/generated/**src/libs/Ollama/Generated/JsonConverters.ResponseFormatEnumNullable.g.csis excluded by!**/generated/**src/libs/Ollama/Generated/JsonSerializerContext.g.csis excluded by!**/generated/**src/libs/Ollama/Generated/JsonSerializerContextTypes.g.csis excluded by!**/generated/**src/libs/Ollama/Generated/Ollama.ChatClient.GenerateChatCompletion.g.csis excluded by!**/generated/**src/libs/Ollama/Generated/Ollama.CompletionsClient.GenerateCompletion.g.csis excluded by!**/generated/**src/libs/Ollama/Generated/Ollama.IChatClient.GenerateChatCompletion.g.csis excluded by!**/generated/**src/libs/Ollama/Generated/Ollama.ICompletionsClient.GenerateCompletion.g.csis excluded by!**/generated/**src/libs/Ollama/Generated/Ollama.Models.GenerateChatCompletionRequest.g.csis excluded by!**/generated/**src/libs/Ollama/Generated/Ollama.Models.GenerateCompletionRequest.g.csis excluded by!**/generated/**src/libs/Ollama/Generated/Ollama.Models.ResponseFormat.Json.g.csis excluded by!**/generated/**src/libs/Ollama/Generated/Ollama.Models.ResponseFormat.g.csis excluded by!**/generated/**src/libs/Ollama/Generated/Ollama.Models.ResponseFormatEnum.g.csis excluded by!**/generated/**src/libs/Ollama/Generated/Ollama.Models.ResponseFormatEnum2.Json.g.csis excluded by!**/generated/**src/libs/Ollama/Generated/Ollama.Models.ResponseFormatEnum2.g.csis excluded by!**/generated/**
📒 Files selected for processing (2)
src/libs/Ollama/openapi.yaml(1 hunks)src/tests/Ollama.IntegrationTests/Test.StructuredOutputInChat.cs(1 hunks)
🔇 Additional comments (1)
src/libs/Ollama/openapi.yaml (1)
441-447: LGTM! Well-structured schema changes.The ResponseFormat schema changes:
- Maintain backward compatibility with the 'json' string format.
- Add support for structured output through JSON schema objects.
- Include clear descriptions for both formats.
|
Thank you very much for taking the time to look into this issue and raise it and offer a solution.
Let me explain a bit what's going on - this is where the FixOpenApiSpec project is launched to make edits to the OpenAPI being loaded - and this is the place to make your suggested changes. I fixed it in this commit - fd55bb8 |
openai.yamlto allow either the string 'json' or a json-serializable object that represents a json schema (no schema validation occurs).StructuredOutputInChatintegration test.Main outstanding issue is that the current
generate.shscript as it stands will blow away the changes I made to the reposopenapi.yamlfile. IMO, the generate.sh script should target the repos version of theopenapi.yamlfile and not try to redownload it from the other source.Addresses issue: #85
Summary by CodeRabbit
New Features
Tests