refine response format#467
Merged
Merged
Conversation
PR Summary by QodoRefine response_format handling by moving it into LLM config Description
Diagram
High-Level Assessment
Files changed (5)
|
Code Review by Qodo
1. Template config shadows agent
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
91
to
97
| function changeResponseFormat(e) { | ||
| const value = e?.detail?.selecteds?.map((/** @type {any} */ x) => x.value)[0] || null; | ||
| template.response_format = value; | ||
| if (!template.llm_config) { | ||
| template.llm_config = { provider: null, model: null }; | ||
| } | ||
| template.llm_config.response_format = value; | ||
| handleAgentChange(); |
There was a problem hiding this comment.
1. Template config shadows agent 🐞 Bug ≡ Correctness
changeResponseFormat() now creates template.llm_config = { provider: null, model: null } just to
store response_format, making template.llm_config truthy even when the template should otherwise
inherit agent settings. Downstream, InstructionAgent prefers selectedTemplate.llm_config over
selectedAgent.llm_config, which can clear provider/model and break LLM selection for that
template.
Agent Prompt
## Issue description
Setting a template response format creates a non-null `template.llm_config` with `provider/model = null`, and other screens treat `template.llm_config` as a complete override (not a partial overlay). This causes agent-level provider/model to be lost when selecting that template.
## Issue Context
- `InstructionAgent` currently computes `llmConfig` via `selectedTemplate?.llm_config || selectedAgent?.llm_config`, so any non-null template config wins, even if it only contains `response_format`.
## Fix Focus Areas
- src/routes/page/agent/[agentId]/agent-components/templates/agent-template-config.svelte[91-97]
- src/routes/page/instruction/instruction-components/instruction-agent.svelte[37-69]
## Recommended fix
Pick one of these (prefer 1 or 2):
1) **Treat template.llm_config as a partial override** in `InstructionAgent` by merging:
- Start from `selectedAgent.llm_config`
- Overlay only non-null/non-undefined fields from `selectedTemplate.llm_config`
- Ensure `provider/model` fall back to agent values when template values are null
2) **Don’t instantiate `template.llm_config` when only response_format is set**, unless the rest of the app supports partial-template configs everywhere. (E.g., store template response format separately, or only set `template.llm_config` when provider/model/max tokens/etc are configured.)
3) Add an explicit “inherit agent llm_config” behavior for templates, and ensure `response_format` can override without forcing provider/model overrides.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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.
No description provided.