Repository navigation
feat: make View Instructions optional - #353
Conversation
📝 WalkthroughWalkthroughThe AiModelParameters component is updated to accept props via a new AiModelParametersProps interface, introducing an optional showViewInstructions boolean that controls whether the View Instructions button is rendered, replacing unconditional button display. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
![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
🤖 Fix all issues with AI agents
In `@packages/ai-settings/src/components/AiModelParameters.tsx`:
- Around line 17-23: The change defaulting showViewInstructions to false in the
AiModelParameters component will hide the "View Instructions" button for all
existing call sites that do not pass props; revert or mitigate by either
restoring the previous default (set showViewInstructions to true in the
AiModelParameters props/defaults) or updating every consumer to explicitly pass
showViewInstructions={true}; locate the AiModelParameters component (exported as
AiModelParameters) and choose one of the two fixes: (a) change the default back
to true in the component signature, or (b) update the 8 upstream call sites that
render AiModelParameters to include showViewInstructions={true} where the button
should remain visible.
🧹 Nitpick comments (1)
packages/ai-settings/src/components/AiModelParameters.tsx (1)
173-214: Consider conditionally rendering the dialog and its dependencies whenshowViewInstructionsis false.The
useDisclosurehook (line 41),getFullInstructionsselector (line 51-53),handleViewFullInstructions(line 118-120), and the entire<Dialog>block (lines 196-214) are still initialized even when the button is hidden. Wrapping the dialog JSX in the same conditional would avoid rendering a permanently-closed dialog:♻️ Suggested change
- {/* Full Instructions Modal */} - <Dialog open={isOpen} onOpenChange={onClose}> - ... - </Dialog> + {/* Full Instructions Modal */} + {showViewInstructions && ( + <Dialog open={isOpen} onOpenChange={onClose}> + ... + </Dialog> + )}Moving the
getFullInstructionsselector andhandleViewFullInstructionsbehind a guard isn't feasible due to the rules of hooks, but skipping the Dialog JSX is a small win.
| export interface AiModelParametersProps { | ||
| showViewInstructions?: boolean; | ||
| } | ||
|
|
||
| export const AiModelParameters: FC<AiModelParametersProps> = ({ | ||
| showViewInstructions = false, | ||
| }) => { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n --type=tsx --type=ts -C3 'AiModelParameters(?!Props)' --multiline-dotallRepository: sqlrooms/sqlrooms
Length of output: 307
🏁 Script executed:
rg --pcre2 -n --type=tsx --type=ts -C3 'AiModelParameters(?!Props)'Repository: sqlrooms/sqlrooms
Length of output: 88
🏁 Script executed:
rg -n 'AiModelParameters' --include='*.ts' --include='*.tsx' -C3Repository: sqlrooms/sqlrooms
Length of output: 502
🏁 Script executed:
rg 'AiModelParameters' -C3Repository: sqlrooms/sqlrooms
Length of output: 4524
🏁 Script executed:
cat packages/ai-settings/src/components/AiSettingsPanel.tsxRepository: sqlrooms/sqlrooms
Length of output: 1325
🏁 Script executed:
rg 'ModelParametersSettings' -C3Repository: sqlrooms/sqlrooms
Length of output: 6445
Defaulting showViewInstructions to false is a breaking change — all 8 existing call sites will lose the "View Instructions" button.
The component is currently used in 8 places across examples and apps (ai, ai-core, ai-rag, ai-nextjs, app-builder, ai-agent, and sqlrooms-cli-ui), all without passing props. These will silently lose the button with the new default. Update the call sites that need the button to pass showViewInstructions={true}, or reconsider the default value if the button should remain visible.
🤖 Prompt for AI Agents
In `@packages/ai-settings/src/components/AiModelParameters.tsx` around lines 17 -
23, The change defaulting showViewInstructions to false in the AiModelParameters
component will hide the "View Instructions" button for all existing call sites
that do not pass props; revert or mitigate by either restoring the previous
default (set showViewInstructions to true in the AiModelParameters
props/defaults) or updating every consumer to explicitly pass
showViewInstructions={true}; locate the AiModelParameters component (exported as
AiModelParameters) and choose one of the two fixes: (a) change the default back
to true in the component signature, or (b) update the 8 upstream call sites that
render AiModelParameters to include showViewInstructions={true} where the button
should remain visible.
Summary by CodeRabbit