fix(sidecar): force JSON response_format on OpenRouter calls#72
Merged
Conversation
When a brief mentioned an inaccessible resource (e.g. a local file path
like `story.md`), OpenRouter models replied with a plain-text refusal
("Je suis conçu pour générer uniquement des captions Instagram en
JSON — je ne peux pas analyser ou décrire des fichiers comme
`story.md`."), which poisoned `_parse_json_response` downstream and
surfaced as a malformed-JSON error in the Composer UI.
Add `response_format={"type": "json_object"}` to the three OpenAI-compat
methods that expect a JSON response (`_generate_openai_compat`,
`_carousel_openai_compat`, `_extract_visual_openai_compat`) so the model
is contractually forced to emit valid JSON regardless of the brief
content. The synthesize path (`_synthesize_openai_compat`) is left
untouched — it returns plain text on purpose (the ProductTruth gets
pasted directly into a textarea).
The flag is gated to `provider == "openrouter"` via a new
`_openrouter_json_kwargs()` helper so Ollama (local, support is patchy)
and Anthropic native (different request shape) are unaffected.
OpenRouter's docs list `json_mode` in the `supported_features` array
for Anthropic Claude models, so the routing is safe across the compat
matrix.
Add `TestOpenRouterJsonMode` (6 tests) that mock `ai_client.OpenAI` and
verify the gate behavior: param IS passed for OpenRouter, NOT for
Ollama, NOT for the synthesize path even on OpenRouter.
All 66 sidecar tests pass locally.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Guide du relecteurPour les appels OpenRouter compatibles OpenAI qui sont censés renvoyer du JSON, cette PR centralise et impose Modifications par fichier
Conseils et commandesInteragir avec Sourcery
Personnaliser votre expérienceAccédez à votre tableau de bord pour :
Obtenir de l’aide
Original review guide in EnglishReviewer's GuideFor OpenRouter-based OpenAI-compatible calls that are expected to return JSON, this PR centralizes and enforces File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - j'ai trouvé 1 problème
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="sidecar/tests/test_ai_client.py" line_range="577-586" />
<code_context>
+ call = client.chat.completions.create.call_args
+ assert call.kwargs.get("response_format") == {"type": "json_object"}
+
+ def test_generate_ollama_omits_response_format(self):
+ client = self._mock_client('{"caption": "ok", "hashtags": []}')
+ with patch("ai_client.OpenAI", return_value=client):
+ ai = AIClient(
+ provider="ollama",
+ api_key=None,
+ model="qwen2:0.5b",
+ base_url="http://localhost:11434/v1",
+ )
+ ai._generate_openai_compat("brief", "system", max_tokens=600)
+ call = client.chat.completions.create.call_args
+ assert "response_format" not in call.kwargs
+
+ def test_carousel_openrouter_sets_response_format(self):
</code_context>
<issue_to_address>
**suggestion (testing):** Ajouter un test négatif pour un fournisseur qui n’est ni Ollama ni OpenRouter mais qui utilise tout de même le chemin de compatibilité
Actuellement, nous vérifions uniquement l’omission de `response_format` pour `provider="ollama"` (ainsi que pour le cas `synthesize`). Merci d’ajouter également un test négatif pour un autre fournisseur qui utilise le même chemin `*_openai_compat` (par exemple `provider="openai"` ou un fournisseur compatible Anthropic) afin de vérifier qu’il ne reçoit pas non plus `response_format` dans `call.kwargs`. Cela verrouillera le contrat selon lequel seul `provider=="openrouter"` bénéficie du mode JSON et évitera d’élargir accidentellement ce comportement.
</issue_to_address>Sourcery est gratuit pour l’open source - si nos reviews vous plaisent, pensez à les partager ✨
Original comment in English
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="sidecar/tests/test_ai_client.py" line_range="577-586" />
<code_context>
+ call = client.chat.completions.create.call_args
+ assert call.kwargs.get("response_format") == {"type": "json_object"}
+
+ def test_generate_ollama_omits_response_format(self):
+ client = self._mock_client('{"caption": "ok", "hashtags": []}')
+ with patch("ai_client.OpenAI", return_value=client):
+ ai = AIClient(
+ provider="ollama",
+ api_key=None,
+ model="qwen2:0.5b",
+ base_url="http://localhost:11434/v1",
+ )
+ ai._generate_openai_compat("brief", "system", max_tokens=600)
+ call = client.chat.completions.create.call_args
+ assert "response_format" not in call.kwargs
+
+ def test_carousel_openrouter_sets_response_format(self):
</code_context>
<issue_to_address>
**suggestion (testing):** Add a negative test for a non-Ollama, non-OpenRouter provider that still uses the compat path
Currently we only assert omission of `response_format` for `provider="ollama"` (plus the `synthesize` case). Please also add a negative test for another provider that uses the same `*_openai_compat` path (e.g. `provider="openai"` or an Anthropic-compat provider) to verify it also does not receive `response_format` in `call.kwargs`. This will lock in the contract that only `provider=="openrouter"` gets JSON mode and prevent accidental broadening of this behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+577
to
+586
| def test_generate_ollama_omits_response_format(self): | ||
| client = self._mock_client('{"caption": "ok", "hashtags": []}') | ||
| with patch("ai_client.OpenAI", return_value=client): | ||
| ai = AIClient( | ||
| provider="ollama", | ||
| api_key=None, | ||
| model="qwen2:0.5b", | ||
| base_url="http://localhost:11434/v1", | ||
| ) | ||
| ai._generate_openai_compat("brief", "system", max_tokens=600) |
There was a problem hiding this comment.
suggestion (testing): Ajouter un test négatif pour un fournisseur qui n’est ni Ollama ni OpenRouter mais qui utilise tout de même le chemin de compatibilité
Actuellement, nous vérifions uniquement l’omission de response_format pour provider="ollama" (ainsi que pour le cas synthesize). Merci d’ajouter également un test négatif pour un autre fournisseur qui utilise le même chemin *_openai_compat (par exemple provider="openai" ou un fournisseur compatible Anthropic) afin de vérifier qu’il ne reçoit pas non plus response_format dans call.kwargs. Cela verrouillera le contrat selon lequel seul provider=="openrouter" bénéficie du mode JSON et évitera d’élargir accidentellement ce comportement.
Original comment in English
suggestion (testing): Add a negative test for a non-Ollama, non-OpenRouter provider that still uses the compat path
Currently we only assert omission of response_format for provider="ollama" (plus the synthesize case). Please also add a negative test for another provider that uses the same *_openai_compat path (e.g. provider="openai" or an Anthropic-compat provider) to verify it also does not receive response_format in call.kwargs. This will lock in the contract that only provider=="openrouter" gets JSON mode and prevent accidental broadening of this behavior.
…ponse_format (Sourcery on PR #72) Sourcery flagged that the gate behavior was only asserted negatively for `provider="ollama"`. Add a third negative case for `provider="openai"` to prevent a future OpenAI-direct route, Anthropic-compat proxy, or any other compat-path provider from accidentally inheriting JSON mode. Locks the contract: only the literal string `"openrouter"` triggers `response_format={"type": "json_object"}`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
response_format={\"type\": \"json_object\"}to the three OpenAI-compat methods that expect JSON (_generate_openai_compat,_carousel_openai_compat,_extract_visual_openai_compat).provider == \"openrouter\"via new_openrouter_json_kwargs()helper. Ollama, Anthropic native, and the plain-text_synthesize_openai_compatpath are intentionally untouched.TestOpenRouterJsonMode) mockai_client.OpenAIand pin the gate behavior.The bug
User report: Composer broke when a brief mentioned a local file (
story.md). The model returned a plain-text refusal instead of JSON:This poisoned
_parse_json_responseand surfaced as a malformed-JSON error in the UI. Forcingresponse_formatmakes the model contractually emit valid JSON regardless of what the brief asks for.Why this is safe across the compat matrix
OpenRouter's models endpoint advertises
json_modein thesupported_featuresarray for Anthropic Claude models (per docs). The CLAUDE.md compat-matrix entries (Sonnet 4.6, Haiku, Opus 4.7, Opus 4.6-fast, GPT-4o, GPT-4o-mini, DeepSeek, Gemini 2.0 Flash) all support it natively.Models flagged⚠️
jsonUnreliable(Mistral Small) and*:freeendpoints may surface a clearer 400 instead of returning malformed JSON downstream — acceptable trade.Why we keep
_synthesize_openai_compatplainProductTruth synthesis returns plain text that gets pasted into the Settings textarea. Forcing JSON mode there would defeat the UX.
Tests
Includes 6 new tests covering: OpenRouter sets the flag (3 methods), Ollama omits it (2 methods), synthesize never sets it even on OpenRouter.
Out of scope (deferred)
extra_body={\"provider\": {\"require_parameters\": True}}for strict routing — defense in depth; could restrict provider fallbacks for partial-support models. Consider in a follow-up.scheduler.rs:133bug_risk) — separate dedicated PR.🤖 Generated with Claude Code
Summary by Sourcery
Forcer les réponses en mode JSON pour les workflows JSON basés sur OpenRouter et ajouter une couverture de régression pour le contrôle spécifique au fournisseur.
Corrections de bugs :
Tests :
response_formatsur JSON pour les méthodes produisant du JSON, tandis que les appels Ollama et de synthèse l’omettent.Original summary in English
Summary by Sourcery
Force JSON-mode responses for OpenRouter-backed JSON workflows and add regression coverage for the provider-specific gating.
Bug Fixes:
Tests: