Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a "Structured outputs" demo to the OllamaSharp console demo application. It demonstrates how to constrain a model's response to a predefined JSON schema by having the user enter a dish name and receiving a fully structured RecipeSchema object in return. The ToggleThink logic in OllamaConsole was also refactored to extract a new SetThink(object?) helper method.
Changes:
- A new
StructuredOutputConsoledemo that sends a JSON schema as theformatparameter and renders the result as a formatted recipe card. - A new
SetThink(object? value)helper method extracted from the existingToggleThinklogic inOllamaConsole. - A new "Structured outputs" entry added to the main demo selection menu in
Program.cs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
demo/Demos/StructuredOutputConsole.cs |
New demo class: prompts for a dish name, calls the model with a JSON schema format constraint, streams the output, pretty-prints JSON, and renders a recipe card. |
demo/OllamaConsole.cs |
Extracted SetThink(object?) from ToggleThink to allow callers to set think mode to a specific value programmatically. |
demo/Program.cs |
Added "Structured outputs" to the menu and switch statement to route to the new demo. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private static readonly JsonSerializerOptions SERIALIZER_OPTIONS = new() | ||
| { | ||
| PropertyNameCaseInsensitive = true, | ||
| DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull | ||
| }; |
There was a problem hiding this comment.
The field SERIALIZER_OPTIONS uses UPPER_CASE naming, which in this codebase is reserved for const fields (e.g., MULTILINE_OPEN, MULTILINE_CLOSE, START_NEW_COMMAND). This is a private static readonly field, not a constant. It should follow the naming convention for non-const fields. The src project uses _camelCase for private readonly statics (e.g., _schemaTransformCache in AbstractionMapper.cs). Renaming to _serializerOptions would align with the established convention.
| } | ||
|
|
||
| /// <summary> | ||
| /// Toggles the think mode between null, false, and true. |
There was a problem hiding this comment.
The XML doc comment for SetThink reads "Toggles the think mode between null, false, and true." — this is a copy-paste of the ToggleThink summary and does not accurately describe what this method does. SetThink sets think mode to a specific value passed as the argument, rather than cycling through states. The summary should instead reflect the actual behavior, for example: "Sets the think mode to the specified value and prints the updated state to the console."
| /// Toggles the think mode between null, false, and true. | |
| /// Sets the think mode to the specified value and prints the updated state to the console. |
| AnsiConsole.WriteLine(); | ||
|
|
||
| Ollama.SelectedModel = await SelectModel("Select a model you want to use:"); | ||
| SetThink(new ThinkValue(false)); |
There was a problem hiding this comment.
SetThink accepts object? and internally wraps it with new ThinkValue(value). The call SetThink(new ThinkValue(false)) therefore double-wraps the value: a ThinkValue struct is passed as object?, boxed, and then re-parsed by the ThinkValue(object? value) constructor via bool.TryParse(value.ToString(), ...). While the end result happens to be semantically correct (because ThinkValue(false).ToString() returns "false" which parses correctly), the intent is clearer and simpler as SetThink(false), which uses the existing implicit conversion from bool to ThinkValue.
| SetThink(new ThinkValue(false)); | |
| SetThink(false); |
| SetThink(new ThinkValue(false)); | ||
|
|
||
| if (string.IsNullOrEmpty(Ollama.SelectedModel)) | ||
| return; | ||
|
|
There was a problem hiding this comment.
SetThink(new ThinkValue(false)) is called before the guard that checks whether a model was selected (line 34). If the user presses "back" at the model selection prompt, Ollama.SelectedModel will be empty and the demo exits immediately, but "Think mode is false." will still be printed to the console. This produces a confusing message when the user never actually entered the demo. The SetThink call should be moved to after the empty-model guard, consistent with the flow in other demos that do not emit side-effect output before checking a precondition.
| SetThink(new ThinkValue(false)); | |
| if (string.IsNullOrEmpty(Ollama.SelectedModel)) | |
| return; | |
| if (string.IsNullOrEmpty(Ollama.SelectedModel)) | |
| return; | |
| SetThink(new ThinkValue(false)); |
No description provided.