feat: display model provider and default model throughout CLI#324
feat: display model provider and default model throughout CLI#324aidandaly24 merged 1 commit intomainfrom
Conversation
|
/strands review this pr, do not make any changes just review it and add comments |
|
failing test, remember to go throught this check list |
Code Review SummaryI've reviewed this PR and overall it's a well-implemented feature with good code organization and consistent UX improvements. Here are my findings: ✅ Strengths
🔍 Suggestions for Improvement1. Naming Clarity -
2. Inconsistent Capitalization
Consider standardizing the format for better consistency. 3. Comment Accuracy
This is slightly misleading since:
Suggested rewording: "Human-readable model names displayed in the CLI user interface" 4. Test Coverage
5. PR Description - Testing Checklist
Were these run? If so, please check them off for completeness. 📋 Minor Observations
✨ Overall AssessmentThis is a solid implementation that improves user visibility into model configuration. The suggestions above are minor refinements that would further improve code clarity and maintainability. Nice work! Let me know if you'd like me to clarify any of these points or if you'd like to discuss the suggestions! |
Additional Review NotesI've completed a thorough review of all the template files and can confirm: ✅ All SDK Templates Correctly Updated
Code Flow Verification ✅Verified the complete data flow:
Integration Points ✅
The implementation is solid and complete. My main suggestions from the previous comment still stand (naming clarity and test coverage), but these are minor improvements that don't affect functionality. Great work on the comprehensive implementation! 🎉 |
notgitika
left a comment
There was a problem hiding this comment.
left a comment but an overall nit: The gpt-4o -> gpt-4.1 and the gemini one changes affect what gets scaffolded for new projects (different cost/capability). Worth calling out separately in the PR description since they're bundled with the display-name feature but are independent behavioral changes.
I would also prefer that the actual full model ID be shown to the user at all times so they don't have to check inside the code each time but thats just my opinion
Coverage Report
|
ce2ed9b to
80c8142
Compare
- Add DEFAULT_MODEL_IDS constant mapping providers to model IDs - Show model in TUI provider selection (e.g., 'Anthropic (claude-sonnet-4-5-20250514)') - Show model in Review Configuration confirm screen - Show model info in success screens after create/add agent - Show provider in dev TUI, dev --logs, dev --invoke - Show provider in invoke TUI and invoke CLI - Add modelProvider field to AgentEnvSpec schema for persistence - Update OpenAI templates from gpt-4o to gpt-4.1 - Update Gemini templates from gemini-2.0-flash to gemini-2.5-flash
80c8142 to
1c45cad
Compare
Description
Display model provider and default model information throughout the CLI to improve user visibility into which models their agents will use.
Changes:
Related Issue
Closes #231
Documentation PR
N/A - UI/UX improvement, no doc changes needed
Type of Change
Testing
How have you tested the change?
Manual testing:
Checklist