- Fix ChatBubble to handle non-string content with String() wrapper - Fix API route to use generateText for non-streaming requests - Add @ai-sdk/openai-compatible for non-OpenAI providers (DeepSeek, etc.) - Use Chat Completions API instead of Responses API for compatible providers - Update ChatBubble tests and fix component exports to kebab-case - Remove stale PascalCase ChatBubble.tsx file
2.0 KiB
2.0 KiB
🔥 CODE REVIEW FINDINGS, Max!
Story: 4-3-model-selection-configuration.md Git vs Story Discrepancies: 0 found (Repo has no commits, verified file content) Issues Found: 0 High, 2 Medium, 4 Low
🔴 CRITICAL ISSUES
None. Acceptance Criteria are met.
🟡 MEDIUM ISSUES
- UX Data Loss in ProviderForm: Selecting a provider preset (OpenAI/DeepSeek/etc.) immediately overwrites the "Model Name" field. If a user enters a custom model first and then clicks a preset (e.g., to set the URL), their custom model name is lost of the user has not saved it yet.
- File:
src/components/features/settings/provider-form.tsx
- File:
- Weak Input Validation: The "Model Name" field accepts any non-empty string. There is no pattern validation (e.g., preventing whitespace-only strings if trimmed check fails differently, or weird chars) or reasonable length limits.
- File:
src/services/settings-service.ts
- File:
🟢 LOW ISSUES
- Weak Security: API Keys are "encrypted" using
btoa()(Base64). This is trivial to decode and provides only visual obfuscation, not real security.- File:
src/store/use-settings.ts
- File:
- Connection Validation Robustness:
LLMService.validateConnectionrequestsmax_tokens: 1with message "hello". Some models or provider safety filters might reject this pattern, leading to false negatives in validation.- File:
src/services/llm-service.ts
- File:
- Documentation Gap: The story's "Files Modified" list misses
src/services/settings-service.ts(where validation logic lives) andsrc/services/provider-management-service.ts(used by ChatService), which are critical for the integration verification.- File:
_bmad-output/implementation-artifacts/4-3-model-selection-configuration.md
- File:
- Test Fragility:
resetStore()in tests clears alllocalStorage. While fine now, this is aggressive and makes tests brittle if we add other persistent features later.- File:
src/components/features/settings/provider-form.model-selection.test.tsx
- File: