added a prompt fix for better vision+audio prompt, removed code that …#589
added a prompt fix for better vision+audio prompt, removed code that …#589HenryNdubuaku merged 1 commit intomainfrom
Conversation
…threw warnings on cmake
There was a problem hiding this comment.
Pull request overview
Updates Gemma4 test prompts and ChatMessage initializers to align with the current ChatMessage struct (including audio/tool-call related fields) and improves the vision+audio test prompt.
Changes:
- Update multiple tests to aggregate-initialize
ChatMessagewith the newly presentaudio_soft_token_countandtool_callsfields. - Adjust the vision+audio suite test to use a clearer prompt string (and reuse it for display + request JSON).
- Minor whitespace-only change in the suite test.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/test_gemma4_vision.cpp | Updates ChatMessage initialization to include new fields. |
| tests/test_gemma4_thinking.cpp | Updates ChatMessage initializers across thinking/prompt/cache tests. |
| tests/test_gemma4_suite.cpp | Updates vision message initializer; refines vision+audio prompt and builds JSON request with it. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| std::string messages = "[{\"role\":\"user\",\"content\":\"" + image_audio_prompt + "\"," | ||
| "\"images\":[\"" + escape_json(image_path) + "\"]," | ||
| "\"audio\":[\"" + escape_json(audio_path) + "\"]}]"; |
There was a problem hiding this comment.
The messages JSON string concatenates image_audio_prompt directly into a JSON string value. If the prompt ever contains quotes, backslashes, or newlines, this will produce invalid JSON and can cause cactus_complete to fail. Use the existing escape_json() helper for the prompt content as well (similar to how paths are handled).
| return false; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
There is trailing whitespace on this blank line. Please remove it to avoid unnecessary diffs and potential whitespace/lint issues.
…threw warnings on cmake