Config: Temperature Always to 0.7 and Vecto Store Ids not being stored in Config Body#32
Config: Temperature Always to 0.7 and Vecto Store Ids not being stored in Config Body#32Prajna1999 merged 1 commit intomainfrom
Conversation
📝 WalkthroughWalkthroughThis PR renames the Changes
Sequence Diagram(s)sequenceDiagram
participant Frontend as Frontend (UI)
participant SimplifiedEditor as SimplifiedConfigEditor
participant Backend as Backend API
rect rgba(100, 150, 200, 0.5)
Note over Frontend,Backend: Loading Configuration
Backend->>SimplifiedEditor: Provide config with flattened<br/>knowledge_base_ids, max_num_results
SimplifiedEditor->>SimplifiedEditor: Unflatten: Construct tools array<br/>from knowledge_base_ids
SimplifiedEditor->>Frontend: Return SavedConfig with<br/>tools array for UI
Frontend->>Frontend: Render tool configuration UI
end
rect rgba(200, 150, 100, 0.5)
Note over Frontend,Backend: Saving Configuration
Frontend->>SimplifiedEditor: User saves configuration<br/>with updated tools array
SimplifiedEditor->>SimplifiedEditor: Flatten: Aggregate knowledge_base_ids<br/>from all tools, extract max_num_results
SimplifiedEditor->>Backend: Send payload with top-level<br/>knowledge_base_ids, max_num_results
Backend->>Backend: Persist flattened structure
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Prajna1999
requested review from
AkhileshNegi and
vprashrex
and removed request for
vprashrex
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/lib/useConfigs.ts (1)
112-118:⚠️ Potential issue | 🟠 MajorPreserve explicit temperature = 0.
Line 116 uses
|| 0.7, which coerces0to0.7and recreates the temperature reset bug. Use nullish coalescing instead.🛠️ Proposed fix
- temperature: params.temperature || 0.7, + temperature: params.temperature ?? 0.7,
🤖 Fix all issues with AI agents
In `@app/configurations/prompt-editor/page.tsx`:
- Line 247: The debug console.log that prints the entire configBlob
(console.log('[DEBUG] Config blob being sent:', JSON.stringify(configBlob, null,
2))) should be removed or gated to non-production; update the code around where
configBlob is prepared to either delete the debug log or wrap it in a runtime
check (e.g., only run when process.env.NODE_ENV !== 'production' or when a
specific DEBUG flag is enabled) so the full instructions/config are not exposed
in production logs.
🧹 Nitpick comments (3)
app/components/SimplifiedConfigEditor.tsx (3)
106-120: Consider using a fresh array instead of mutating a const.The
toolsvariable is declared asconstbut then mutated viapush(). While this is technically valid in JavaScript (const prevents reassignment, not mutation), it's cleaner to build a new array:♻️ Suggested refactor
- const tools: Tool[] = params.tools || []; - - // If no tools array but has flattened fields, create tools array from them - // Each knowledge_base_id becomes a separate tool for UI display - if (tools.length === 0 && params.knowledge_base_ids && params.knowledge_base_ids.length > 0) { - params.knowledge_base_ids.forEach((kbId: string) => { - tools.push({ - type: 'file_search', - knowledge_base_ids: [kbId], // Each tool gets one ID for UI - max_num_results: params.max_num_results || 20, - }); - }); - } + // Backend sends flattened fields - convert to tools array for frontend UI + let tools: Tool[] = params.tools || []; + + // If no tools array but has flattened fields, create tools array from them + if (tools.length === 0 && params.knowledge_base_ids && params.knowledge_base_ids.length > 0) { + tools = params.knowledge_base_ids.map((kbId: string) => ({ + type: 'file_search' as const, + knowledge_base_ids: [kbId], + max_num_results: params.max_num_results || 20, + })); + }
131-131: Consider using nullish coalescing for temperature fallback.Using
|| 0.7will fallback to 0.7 if the backend returnstemperature: 0, which is a valid (deterministic) setting. If supportingtemperature=0is intended, use??instead:♻️ Suggested fix
- temperature: params.temperature || 0.7, + temperature: params.temperature ?? 0.7,
252-259: Brittle condition for selectingmax_num_resultsfrom the first tool.The condition
allKnowledgeBaseIds.length === tool.knowledge_base_ids.lengthis only true on the first iteration (before any IDs are accumulated). While this works, it's non-obvious and fragile—iftools[0]has zeroknowledge_base_ids, the condition fails andmaxNumResultsstays at the default.♻️ Clearer approach using a flag or direct access
const allKnowledgeBaseIds: string[] = []; - let maxNumResults = 20; // default + const maxNumResults = tools[0]?.max_num_results ?? 20; tools.forEach((tool) => { - // Add all knowledge_base_ids from this tool allKnowledgeBaseIds.push(...tool.knowledge_base_ids); - // Use max_num_results from first tool (could be made configurable) - if (allKnowledgeBaseIds.length === tool.knowledge_base_ids.length) { - maxNumResults = tool.max_num_results; - } });
| }, | ||
| }; | ||
|
|
||
| console.log('[DEBUG] Config blob being sent:', JSON.stringify(configBlob, null, 2)); |
There was a problem hiding this comment.
Drop or gate the debug config blob log.
Line 247 logs the full config blob (including instructions) to the console. Consider removing or gating to non-production builds.
🧹 Suggested guard
- console.log('[DEBUG] Config blob being sent:', JSON.stringify(configBlob, null, 2));
+ if (process.env.NODE_ENV !== 'production') {
+ console.log('[DEBUG] Config blob being sent:', JSON.stringify(configBlob, null, 2));
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log('[DEBUG] Config blob being sent:', JSON.stringify(configBlob, null, 2)); | |
| if (process.env.NODE_ENV !== 'production') { | |
| console.log('[DEBUG] Config blob being sent:', JSON.stringify(configBlob, null, 2)); | |
| } |
🤖 Prompt for AI Agents
In `@app/configurations/prompt-editor/page.tsx` at line 247, The debug console.log
that prints the entire configBlob (console.log('[DEBUG] Config blob being
sent:', JSON.stringify(configBlob, null, 2))) should be removed or gated to
non-production; update the code around where configBlob is prepared to either
delete the debug log or wrap it in a runtime check (e.g., only run when
process.env.NODE_ENV !== 'production' or when a specific DEBUG flag is enabled)
so the full instructions/config are not exposed in production logs.
| if (allKnowledgeBaseIds.length === tool.knowledge_base_ids.length) { | ||
| maxNumResults = tool.max_num_results; | ||
| } | ||
| }); |
There was a problem hiding this comment.
I think iska easy way vo sakta hai no need of iteration and all.
const maxNumResults = tools[0].max_num_results || 20 // since u are just considering the from the first tool .. idk what configurable u can do while performing iteration but this one line is enough
const allKnwledgebaseids. = tools.flatMap(tool => tool.knoweldge_base_ids)
aisa karenge toh let maxNumResults = 20 ki phi jarurat nhi padeghi
This suggestion is for just code readability .. u can ignore it if it doesn't make sense
There was a problem hiding this comment.
I think you are correct but abhi lets merge it and refactor at a later time. Its possible all these code will be deleted entirely in 2 months.
Prajna1999
changed the title
2 participants
Target issues #35 #36 #37
Temperature was being reset to 0.7 and
knowledge_base_ids array was not being spread to match POST Config API contract.
Enhanced configuration selector to display additional details: temperature indicator, knowledge base count, and comprehensive configuration grid layout.
Summary by CodeRabbit
Release Notes
New Features
knowledge_base_ids array was not being spread to match POST Config API contract.
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.