-
Notifications
You must be signed in to change notification settings - Fork 91
feat: Replace inline Qurator model ID with configurable JSON file #4562
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
- Add models.json with list of available Bedrock models (Nova Lite as default) - Update Assistant.tsx to load models from JSON configuration - Enhance DevTools with dropdown selector for preset models - Add option to enter custom model IDs when needed This makes it easier to manage and update available models without code changes. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #4562 +/- ##
==========================================
- Coverage 39.44% 39.34% -0.11%
==========================================
Files 852 852
Lines 36847 36957 +110
Branches 6013 6055 +42
==========================================
+ Hits 14536 14542 +6
- Misses 21087 21180 +93
- Partials 1224 1235 +11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Summary
This PR successfully replaces the hardcoded Bedrock model ID with a configurable JSON-based system. The changes enhance the DevTools UI with a user-friendly dropdown for model selection while maintaining backward compatibility through custom model input.
Key Improvements:
- New
models.jsonconfiguration file with structured model metadata - Enhanced DevTools UI with dropdown selector showing model names and descriptions
- Seamless switching between preset and custom model modes
- Improved default model selection (Amazon Nova Lite for better cost efficiency)
Issues Found:
- Logic error in state management causing potential inconsistency when toggling custom mode
- Accessibility concerns with Link component used as button
- Runtime safety could be improved with better array access patterns
Impact: Low-risk enhancement that modernizes the model selection interface while maintaining existing functionality.
Confidence Score: 4/5
- This PR is generally safe to merge with minor issues that should be addressed
- Score reflects well-structured changes with good error handling, but reduced due to state management inconsistency in DevTools and accessibility concerns that could cause user experience issues
- DevTools.tsx requires attention for state management fix and accessibility improvements
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| catalog/app/components/Assistant/Model/models.json | 5/5 | New JSON configuration file defining available Bedrock models with metadata - well-structured and safe |
| catalog/app/components/Assistant/Model/Assistant.tsx | 4/5 | Updated to use configurable models from JSON, includes fallback handling but has potential runtime issue with array access |
| catalog/app/components/Assistant/UI/Chat/DevTools.tsx | 3/5 | Enhanced UI with dropdown and custom mode switching, but has state management issues and accessibility concerns |
Sequence Diagram
sequenceDiagram
participant User as User
participant DevTools as DevTools Component
participant Assistant as Assistant.tsx
participant JSON as models.json
participant LocalStorage as LocalStorage
User->>DevTools: Opens DevTools UI
DevTools->>Assistant: Import MODELS constant
Assistant->>JSON: Import modelsConfig
JSON-->>Assistant: Return models array
Assistant-->>DevTools: Provide MODELS for dropdown
User->>DevTools: Select model from dropdown
DevTools->>Assistant: setValue(modelId)
Assistant->>LocalStorage: Store model override
User->>DevTools: Click "enter custom model ID"
DevTools->>DevTools: setCustomMode(true)
DevTools->>User: Show custom text input
User->>DevTools: Enter custom model ID
DevTools->>Assistant: setValue(customModelId)
Assistant->>LocalStorage: Store custom model override
User->>DevTools: Clear/Reset model
DevTools->>Assistant: setValue('')
DevTools->>DevTools: setCustomMode(false)
Assistant->>LocalStorage: Remove model override
Assistant-->>Assistant: Use DEFAULT_MODEL_ID
3 files reviewed, 3 comments
## Features Added: - **Real AWS Bedrock API testing**: Tests ALL models (preset + custom) for actual availability - **Better event handling**: Press Enter or click test button instead of onBlur - **Rich visual feedback**: Loading spinners, success/error modals with icons - **Smart error handling**: Parses AWS error codes (ValidationException, AccessDeniedException, etc.) - **Manual test button**: Explicit checkmark button for immediate testing ## PR Review Issues Fixed: - Fixed state management logic error in custom mode toggle - Replaced Link component with proper Button for accessibility - Improved runtime safety with explicit array length checks - All linting and TypeScript compilation passes ## User Experience: - Type freely without validation interruption - Press Enter or click test button to validate - Clear modal dialogs show availability status - Tests work for both preset and custom models - Handles throttling, access denied, and model not found errors 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Summary
Key Features Added
Real Model Validation
Better User Experience
DevTools Improvements
Technical Changes
models.jsonconfiguration file with available Bedrock modelsAssistant.tsxto load models from JSON and export validation functionsDevTools.tsxwith real AWS testing, better events, accessibility fixesPR Review Issues Fixed ✅
Test Plan
npm run lint)npm run build)🤖 Generated with Claude Code