fix: improve GLM presets, ideation auth, and Insights env #648
Conversation
|
Warning Rate limit exceeded@StillKnotKnown has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 12 minutes and 16 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughInsights env assembly is now asynchronous and merges API-profile, OAuth-clear vars, and normalized PYTHONPATH; runner environment is propagated into GitHub-related Python subprocesses. Ideation auth can resolve API profiles via IPC. Multiple settings UIs were localized, presets added, tests and Sentry mocks included. Changes
Sequence Diagram(s)sequenceDiagram
rect rgb(250,250,255)
participant Executor as InsightsExecutor
participant Config as InsightsConfig
participant PyMgr as PythonEnvManager
participant APIProf as APIProfileService
end
Executor->>Config: await getProcessEnv()
activate Config
Config->>PyMgr: fetch pythonEnv (async)
Config->>APIProf: fetch apiProfileEnv & oauthModeClearVars (async)
APIProf-->>Config: apiProfileEnv + oauthModeClearVars
PyMgr-->>Config: pythonEnv
Config->>Config: merge envs, normalize & compute PYTHONPATH, append auto-build path if needed
Config-->>Executor: resolved env map
deactivate Config
Executor->>Executor: spawn Python runner with merged env
sequenceDiagram
actor User
participant Hook as useIdeationAuth
participant Store as IdeationStore
participant IPC as ElectronAPI
User->>Hook: checkAuth()/performCheck()
activate Hook
Hook->>Store: read activeProfileId
alt Store has activeProfileId
Hook->>Hook: resolveHasAPIProfile = true
else
Hook->>IPC: getAPIProfiles()
activate IPC
IPC-->>Hook: { profiles, activeProfileId }
deactivate IPC
Hook->>Hook: resolveHasAPIProfile from IPC result
end
Hook->>Hook: compute hasToken = hasSourceOAuthToken || hasAPIProfile
Hook->>Hook: set isLoading = false
Hook-->>User: return { hasToken, isLoading }
deactivate Hook
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
Summary of ChangesHello @StillKnotKnown, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines core functionalities related to API profile management and the Insights environment. It introduces user-friendly presets for API configurations and improves the dialog's layout. The authentication process for ideation features is now more robust, leveraging API profiles. Crucially, the Insights environment setup has been upgraded to handle Python paths more effectively, ensuring proper execution of Python-dependent features. These changes are backed by new and updated tests to maintain stability and correctness. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several valuable improvements. It adds GLM presets for API profiles and refactors the profile creation dialog for a better user experience. The authentication flow for the ideation feature is now aligned with API profiles, including a fallback mechanism for robustness. The environment setup for Insights is enhanced to correctly merge PYTHONPATH, and new tests have been added to cover these changes. My review includes one suggestion to refactor duplicated code in the useIdeationAuth hook for better maintainability.
| let hasAPIProfile = Boolean(activeProfileId && activeProfileId !== ''); | ||
| if (!hasAPIProfile) { | ||
| const profilesResult = await window.electronAPI.getAPIProfiles(); | ||
| hasAPIProfile = Boolean( | ||
| profilesResult.success && | ||
| profilesResult.data?.activeProfileId && | ||
| profilesResult.data.activeProfileId !== '' | ||
| ); | ||
| } |
There was a problem hiding this comment.
The logic for checking hasAPIProfile is duplicated here and in the useEffect hook's performCheck function (lines 35-45). To improve maintainability and follow the DRY (Don't Repeat Yourself) principle, this logic should be extracted into a single helper function within the useIdeationAuth hook. This new function can then be called from both checkAuth and performCheck.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/frontend/src/renderer/components/settings/ProfileList.tsx (1)
191-191: Use the computedisActivevariable.Line 177 already computes
isActive = activeProfileId === profile.id. For consistency and to avoid duplication, use theisActivevariable here instead of rechecking.🔎 Suggested fix
- {activeProfileId === profile.id && ( + {isActive && ( <span className="flex items-center text-xs text-primary"> <Check className="h-3 w-3 mr-1" /> Active </span> )}apps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx (1)
232-232: Missing i18n for user-facing text.Multiple hardcoded user-facing strings should use translation keys instead:
- Line 232:
"Enter model name (e.g., claude-3-5-sonnet-20241022)"- Line 268:
"Search models..."- Line 279:
"No models match your search"- Line 310:
"Model discovery not available. Enter model name manually."As per coding guidelines, all user-facing text in frontend components must use i18n translation keys with the
useTranslation()hook.🔎 Recommended approach
Add translation hook and replace hardcoded strings:
+import { useTranslation } from 'react-i18next'; + export function ModelSearchableSelect({ value, onChange, placeholder = 'Select a model or type manually', baseUrl, apiKey, disabled = false, className }: ModelSearchableSelectProps) { + const { t } = useTranslation('settings'); const discoverModels = useSettingsStore((state) => state.discoverModels);Then update the strings to use translation keys like:
t('settings:modelSelect.enterModelName')t('settings:modelSelect.searchPlaceholder')- etc.
Based on coding guidelines, all user-facing text must use i18n.
Also applies to: 268-268, 279-279, 310-310
apps/frontend/src/renderer/components/settings/ProfileEditDialog.tsx (1)
308-311: Missing i18n for user-facing text.Multiple hardcoded user-facing strings violate the i18n requirement:
- Line 308:
'Edit Profile','Add API Profile'- Lines 309-311: Dialog description text
- Line 319:
'Name'- Line 333:
'Preset'- Line 336:
'Choose a provider preset'- Line 347:
'Presets fill the base URL...'- Line 357:
'Base URL'- Line 368-369: Placeholder and helper text
- Line 375:
'API Key'- Line 392:
'Change'- Line 417:
'Cancel'- Lines 479-482: Model mappings section labels
- Lines 487, 500, 513, 526: Model field labels
As per coding guidelines, use
useTranslation()hook with namespace prefixes (e.g.,'settings:profile.title') for all user-facing text.🔎 Recommended approach
Add translation hook at component level:
+import { useTranslation } from 'react-i18next'; + export function ProfileEditDialog({ open, onOpenChange, onSaved, profile }: ProfileEditDialogProps) { + const { t } = useTranslation('settings'); const { saveProfile,Then replace all hardcoded strings with translation keys, for example:
{isEditMode ? t('settings:profile.editTitle') : t('settings:profile.addTitle')}t('settings:profile.description')- etc.
Based on coding guidelines, all frontend user-facing text must use i18n translation keys.
Also applies to: 319-320, 333-333, 336-336, 347-348, 357-358, 369-369, 375-375, 487-488, 500-501, 513-514, 526-527
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (13)
apps/frontend/src/__tests__/setup.tsapps/frontend/src/main/__tests__/insights-config.test.tsapps/frontend/src/main/insights/config.tsapps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeation.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsxapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsxapps/frontend/src/renderer/components/settings/ProfileList.tsx
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.tsapps/frontend/src/main/insights/config.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsxapps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.tsapps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeation.tsapps/frontend/src/main/__tests__/insights-config.test.tsapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsxapps/frontend/src/__tests__/setup.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsxapps/frontend/src/renderer/components/settings/ProfileList.tsx
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.tsapps/frontend/src/main/insights/config.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsxapps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.tsapps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeation.tsapps/frontend/src/main/__tests__/insights-config.test.tsapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsxapps/frontend/src/__tests__/setup.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsxapps/frontend/src/renderer/components/settings/ProfileList.tsx
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.tsapps/frontend/src/main/insights/config.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsxapps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.tsapps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeation.tsapps/frontend/src/main/__tests__/insights-config.test.tsapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsxapps/frontend/src/__tests__/setup.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsxapps/frontend/src/renderer/components/settings/ProfileList.tsx
🧠 Learnings (2)
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx} : Use `useTranslation()` hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Applied to files:
apps/frontend/src/renderer/components/ideation/hooks/useIdeation.ts
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/backend/.env* : Enable Electron MCP for E2E testing by setting `ELECTRON_MCP_ENABLED=true` in `.env` and starting the Electron app with `npm run dev`
Applied to files:
apps/frontend/src/main/__tests__/insights-config.test.ts
🧬 Code graph analysis (8)
apps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.ts (2)
apps/frontend/src/renderer/stores/settings-store.ts (1)
useSettingsStore(45-295)apps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.ts (1)
useIdeationAuth(16-89)
apps/frontend/src/main/insights/config.ts (2)
apps/frontend/src/main/agent/env-utils.ts (1)
getOAuthModeClearVars(22-39)apps/frontend/src/main/python-env-manager.ts (1)
pythonEnvManager(690-690)
apps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.ts (2)
apps/frontend/src/shared/types/insights.ts (1)
IdeationGenerationStatus(140-146)apps/frontend/src/renderer/components/ideation/hooks/useIdeation.ts (1)
useIdeation(28-262)
apps/frontend/src/renderer/components/ideation/hooks/useIdeation.ts (1)
apps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.ts (1)
useIdeationAuth(16-89)
apps/frontend/src/main/__tests__/insights-config.test.ts (1)
apps/frontend/src/main/insights/config.ts (1)
InsightsConfig(15-141)
apps/frontend/src/renderer/components/settings/ProfileEditDialog.tsx (2)
apps/frontend/src/renderer/lib/profile-utils.ts (1)
maskApiKey(11-16)apps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx (1)
ModelSearchableSelect(57-318)
apps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsx (1)
apps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx (1)
ModelSearchableSelect(57-318)
apps/frontend/src/renderer/components/settings/ProfileList.tsx (2)
.design-system/src/lib/utils.ts (1)
cn(4-6).design-system/src/components/Button.tsx (1)
Button(10-44)
🔇 Additional comments (20)
apps/frontend/src/renderer/components/settings/ProfileList.tsx (2)
176-187: LGTM! Clear refactor with isActive computation.The refactor to compute
isActiveat the start of the map callback improves code clarity and enables consistent styling based on the profile's active state. The conditional styling appropriately highlights the active profile.
252-252: Good UX improvement!Disabling the delete button when the profile is active prevents accidental deletion of the active profile and works well with the updated tooltip guidance.
apps/frontend/src/renderer/components/ideation/hooks/useIdeation.ts (1)
18-18: LGTM!The migration from
useClaudeTokenChecktouseIdeationAuthis clean and correctly implemented. The hook usage and method calls are properly aligned with the new authentication API.Also applies to: 52-52, 88-88
apps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.ts (1)
1-94: LGTM! Good test coverage for authentication flow.The test suite correctly validates the integration between
useIdeationand the newuseIdeationAuthhook. The mock setup is comprehensive, and the two test cases cover the key authentication scenarios:
- Prompting for env config when token is missing
- Proceeding with generation when token is present
apps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.ts (3)
21-21: LGTM! Mock setup for IPC profile retrieval.The mock setup for
getAPIProfilesis properly configured with a default implementation that returns an empty profile list. This aligns well with the fallback logic added touseIdeationAuth.Also applies to: 41-41, 50-57
201-235: LGTM! Validates IPC fallback behavior.This test correctly validates that when the store's
activeProfileIdis missing, the hook falls back to fetching profiles via IPC and correctly determineshasTokenbased on the IPC result.
237-255: LGTM! Validates optimization path.This test ensures that the IPC call is skipped when the store already has an
activeProfileId, confirming the "prefer store state" optimization works as intended.apps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx (1)
257-260: LGTM: Dropdown positioning change is well-tested.The repositioning of the dropdown from below to above the input is intentional and covered by test cases.
apps/frontend/src/renderer/components/settings/ProfileEditDialog.tsx (4)
144-160: LGTM: Preset logic is correct.The preset application logic correctly:
- Finds the preset by ID
- Updates the baseUrl
- Conditionally sets the name only when empty (prevents overwriting user input)
This behavior is well-tested in ProfileEditDialog.test.tsx.
71-71: LGTM: Preset state management is correct.The
presetIdstate is properly initialized and reset in both edit and create mode flows.Also applies to: 121-121, 132-132
315-350: Consider accessibility: preset selection keyboard navigation.The preset selector is well-implemented, but ensure keyboard navigation works smoothly when transitioning between Name input → Preset select → Base URL input in the two-column grid layout.
Test keyboard navigation flow:
- Tab from Name field
- Verify preset dropdown opens with keyboard (Space/Enter/Arrow keys)
- Verify focus moves correctly to Base URL field after preset selection
- Test in both desktop and mobile viewport sizes where grid collapses
This is particularly important since the preset field only appears in create mode, affecting the tab order.
377-421: LGTM: API key edit mode logic is correct.The Change/Cancel flow for API key editing is well-implemented:
- Masked display when not editing
- Password input when editing or in create mode
- Cancel button properly resets state
- Validation only applies when changing key
apps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsx (2)
273-289: LGTM: Preset test verifies core functionality.Test correctly validates that applying the "GLM (Global)" preset sets both the baseUrl and name fields as expected.
291-310: LGTM: Test verifies name preservation behavior.This test correctly validates the important edge case where applying a preset should not overwrite a user-entered name. This matches the implementation logic at lines 157-159 in ProfileEditDialog.tsx.
apps/frontend/src/main/insights/insights-executor.ts (1)
88-88: LGTM: Async environment retrieval is correct.The change from synchronous to asynchronous
getProcessEnv()call is properly implemented:
- The
executemethod is already async (line 61)- The
awaitkeyword is correctly used- The result is used synchronously afterwards (line 123), which is safe
This aligns with the broader PR changes to make environment configuration asynchronous.
apps/frontend/src/__tests__/setup.ts (1)
31-37: LGTM: scrollIntoView polyfill is correctly implemented.The polyfill for
scrollIntoViewis necessary for Radix Select components to work in jsdom test environments. The implementation is correct:
- Conditional check prevents overwriting if already defined
- Uses
vi.fn()for proper mock tracking- Set as writable to allow test-specific overrides
- Follows standard jsdom polyfill patterns
apps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsx (1)
135-158: LGTM! Test correctly validates dropdown positioning.The test properly verifies that the dropdown renders with the
bottom-fullclass, confirming it appears above the input. The approach of focusing the input to trigger the dropdown and usingwaitForfor async rendering is consistent with the other tests in this file.Minor note: the dropdown is queried twice (lines 153 and 156), but this is a stylistic choice and doesn't affect correctness.
apps/frontend/src/main/insights/config.ts (2)
117-126: Good use ofpath.delimiterfor cross-platform PYTHONPATH.The PYTHONPATH composition correctly uses
path.delimiter(:on POSIX,;on Windows) for cross-platform compatibility. The conditional spread at line 138 correctly avoids setting an empty PYTHONPATH.
110-140: Async refactor correctly implemented with all callers properly awaiting.The change to async correctly integrates the API profile environment. The merge order ensures that:
apiProfileEnvtakes precedence overprofileEnvoauthModeClearVarsclears ANTHROPIC_* vars only when API profile is absentPYTHONPATHis explicitly set last to combine both sourcesAll callers of
getProcessEnv()have been updated to await the Promise:InsightsExecutor(line 88) and test files properly await the result.apps/frontend/src/main/__tests__/insights-config.test.ts (1)
16-29: Note:getOAuthModeClearVarsuses the real implementation.The mock setup correctly allows the real
getOAuthModeClearVarsto be called, which returns{}whenapiProfileEnvcontainsANTHROPIC_*keys. This effectively tests the integration between these modules. If you want to test the OAuth-mode branch where vars are cleared, you'll need to mockgetAPIProfileEnvto return{}.
apps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.ts
Outdated
Show resolved
Hide resolved
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
Merge Verdict: 🟠 NEEDS REVISION
4 issue(s) must be addressed (2 required, 2 recommended), 2 suggestions
Risk Assessment
| Factor | Level | Notes |
|---|---|---|
| Complexity | High | Based on lines changed |
| Security Impact | None | Based on security findings |
| Scope Coherence | Good | Based on structural review |
Findings Summary
- High: 2 issue(s)
- Medium: 2 issue(s)
- Low: 2 issue(s)
Generated by Auto Claude PR Review
Findings (6 selected of 6 total)
🟠 [HIGH] Hardcoded strings violate i18n requirements
📁 apps/frontend/src/renderer/components/settings/ProfileList.tsx:134
ProfileList.tsx contains 20+ hardcoded English strings including 'API Profiles', 'No API profiles configured', 'Add Profile', 'Switch to OAuth', 'Delete Profile?', and tooltip text. Per CLAUDE.md: 'CRITICAL: Always use i18n translation keys for all user-facing text in the frontend.' Other settings components (AgentProfileSettings, GeneralSettings, DevToolsSettings) consistently use useTranslation.
Suggested fix:
Add useTranslation hook and replace all hardcoded strings with translation keys from settings.json. Add corresponding keys to en/settings.json and fr/settings.json under a 'profiles' section.
🟠 [HIGH] Dialog contains 25+ hardcoded user-facing strings
📁 apps/frontend/src/renderer/components/settings/ProfileEditDialog.tsx:308
ProfileEditDialog.tsx lacks i18n support. Hardcoded strings include: 'Edit Profile', 'Add API Profile', all form labels ('Name', 'Preset', 'Base URL', 'API Key'), placeholders, button text ('Test Connection', 'Save Profile', 'Cancel'), validation errors ('Name is required', 'Invalid URL format'), and toast messages. This violates the project's explicit i18n requirement in CLAUDE.md.
Suggested fix:
Import useTranslation from 'react-i18next', add translation keys for all user-facing strings to en/settings.json and fr/settings.json, and replace hardcoded strings with t() function calls.
🟡 [MEDIUM] Duplicated authentication check logic
📁 apps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.ts:34
The fallback logic for checking API profiles is duplicated between performCheck (lines 34-45) and checkAuth (lines 69-77). Both functions perform identical steps: check store activeProfileId, if empty call IPC getAPIProfiles, compute hasAPIProfile. This creates maintenance burden where bug fixes must be applied twice.
Suggested fix:
Extract the duplicated logic into a shared helper function:
const checkAPIProfile = async (profileId: string | null): Promise<boolean> => {
let hasAPIProfile = Boolean(profileId && profileId !== '');
if (!hasAPIProfile) {
const profilesResult = await window.electronAPI.getAPIProfiles();
hasAPIProfile = Boolean(profilesResult.success && profilesResult.data?.activeProfileId && profilesResult.data.activeProfileId !== '');
}
return hasAPIProfile;
};
🟡 [MEDIUM] Provider presets should be in shared/constants
📁 apps/frontend/src/renderer/components/settings/ProfileEditDialog.tsx:144
The presets array containing provider configurations (Anthropic, OpenRouter, Groq, GLM Global, GLM China) is defined inline within the component. The codebase pattern for configuration data is to define it in apps/frontend/src/shared/constants/ (e.g., AVAILABLE_MODELS, THINKING_LEVELS, DEFAULT_AGENT_PROFILES are in models.ts).
Suggested fix:
Create apps/frontend/src/shared/constants/api-profiles.ts exporting API_PROVIDER_PRESETS, then import in ProfileEditDialog.tsx. Add export to apps/frontend/src/shared/constants/index.ts.
🔵 [LOW] Test file has limited coverage - only one test case
📁 apps/frontend/src/main/__tests__/insights-config.test.ts:47
The insights-config.test.ts file contains only a single test case verifying getProcessEnv() merges environment variables correctly. Missing edge cases: error handling when getAPIProfileEnv fails, behavior when Python environment unavailable, path delimiter handling on Windows vs Unix, behavior when loadAutoBuildEnv returns empty object.
Suggested fix:
Add additional test cases for error scenarios and edge cases, particularly for getAPIProfileEnv failures and missing Python environment.
🔵 [LOW] Test file has limited coverage - only two test cases
📁 apps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.ts:62
The useIdeation.test.ts file contains only two basic test cases (auth modal prompt when token missing, generate when token present). Missing: error handling during generation, loading state management, config changes triggering regeneration, listener setup/cleanup, edge cases where auth state changes during generation.
Suggested fix:
Add comprehensive test cases including: cleanup listeners on unmount test, generation error handling test, loading state transitions.
This review was generated by Auto Claude.
StillKnotKnown
force-pushed
the
local/glm-insights-debug
branch
from
fef4398 to
656fd8b
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx (1)
18-316: Critical: Replace all hardcoded user-facing strings with i18n translation keys.The component contains multiple hardcoded user-facing strings that violate the coding guidelines. All text displayed to users must use i18n translation keys via the
useTranslation()hook.Hardcoded strings found:
- Line 60:
'Select a model or type manually'- Line 232:
'Enter model name (e.g., claude-3-5-sonnet-20241022)'- Line 268:
'Search models...'- Line 279:
'No models match your search'- Line 310:
'Model discovery not available. Enter model name manually.'As per coding guidelines: "Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings" and "Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key')".🔎 Proposed fix to add i18n support
First, import the useTranslation hook at the top of the file:
import { useState, useEffect, useRef } from 'react'; +import { useTranslation } from 'react-i18next'; import { Loader2, AlertCircle, ChevronDown, Search, Check, Info } from 'lucide-react';Then, add the hook inside the component and replace hardcoded strings:
export function ModelSearchableSelect({ value, onChange, - placeholder = 'Select a model or type manually', + placeholder, baseUrl, apiKey, disabled = false, className }: ModelSearchableSelectProps) { + const { t } = useTranslation(); const discoverModels = useSettingsStore((state) => state.discoverModels);Replace the hardcoded strings with translation keys:
- placeholder={modelDiscoveryNotSupported ? 'Enter model name (e.g., claude-3-5-sonnet-20241022)' : placeholder} + placeholder={modelDiscoveryNotSupported + ? t('settings:modelSelect.placeholderManual') + : placeholder || t('settings:modelSelect.placeholder')}- placeholder="Search models..." + placeholder={t('settings:modelSelect.searchPlaceholder')}<div className="p-3 text-center text-sm text-muted-foreground"> - No models match your search + {t('settings:modelSelect.noResults')} </div><p className="text-sm text-muted-foreground mt-1 flex items-center gap-1"> <Info className="h-3 w-3" /> - Model discovery not available. Enter model name manually. + {t('settings:modelSelect.discoveryNotAvailable')} </p>You'll also need to add these keys to your locale files (e.g.,
apps/frontend/src/shared/i18n/locales/en/settings.json):{ "modelSelect": { "placeholder": "Select a model or type manually", "placeholderManual": "Enter model name (e.g., claude-3-5-sonnet-20241022)", "searchPlaceholder": "Search models...", "noResults": "No models match your search", "discoveryNotAvailable": "Model discovery not available. Enter model name manually." } }
♻️ Duplicate comments (1)
apps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.ts (1)
24-39: LGTM! Helper function successfully addresses past review feedback.The
resolveHasAPIProfilehelper effectively eliminates the code duplication identified in previous reviews. The logic is sound:
- Optimizes by trusting store state when available
- Falls back to IPC lookup when store is not populated
- Gracefully handles errors by returning false
Based on past review comments requesting extraction of duplicate API profile checking logic into a helper function.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (17)
apps/frontend/src/__tests__/setup.tsapps/frontend/src/main/__tests__/insights-config.test.tsapps/frontend/src/main/insights/config.tsapps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeation.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsxapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsxapps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/shared/constants/api-profiles.tsapps/frontend/src/shared/constants/index.tsapps/frontend/src/shared/i18n/locales/en/settings.jsonapps/frontend/src/shared/i18n/locales/fr/settings.json
🧰 Additional context used
📓 Path-based instructions (4)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/__tests__/setup.tsapps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsxapps/frontend/src/shared/constants/api-profiles.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.tsapps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/renderer/components/ideation/hooks/useIdeation.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsxapps/frontend/src/main/insights/config.tsapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsxapps/frontend/src/main/__tests__/insights-config.test.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.tsapps/frontend/src/shared/constants/index.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsxapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/__tests__/setup.tsapps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsxapps/frontend/src/shared/constants/api-profiles.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.tsapps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/renderer/components/ideation/hooks/useIdeation.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsxapps/frontend/src/main/insights/config.tsapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsxapps/frontend/src/main/__tests__/insights-config.test.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.tsapps/frontend/src/shared/constants/index.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsxapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.ts
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/__tests__/setup.tsapps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsxapps/frontend/src/shared/constants/api-profiles.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.tsapps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/renderer/components/ideation/hooks/useIdeation.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsxapps/frontend/src/main/insights/config.tsapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsxapps/frontend/src/main/__tests__/insights-config.test.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.tsapps/frontend/src/shared/constants/index.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsxapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.ts
apps/frontend/src/shared/i18n/locales/**/*.json
📄 CodeRabbit inference engine (CLAUDE.md)
apps/frontend/src/shared/i18n/locales/**/*.json: Store translation strings in namespace-organized JSON files atapps/frontend/src/shared/i18n/locales/{lang}/*.jsonfor each supported language
When implementing new frontend features, add translation keys to all language files (minimum: en/.json and fr/.json)
Files:
apps/frontend/src/shared/i18n/locales/fr/settings.jsonapps/frontend/src/shared/i18n/locales/en/settings.json
🧠 Learnings (4)
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : When implementing new frontend features, add translation keys to all language files (minimum: en/*.json and fr/*.json)
Applied to files:
apps/frontend/src/shared/i18n/locales/fr/settings.jsonapps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/shared/i18n/locales/en/settings.json
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : Store translation strings in namespace-organized JSON files at `apps/frontend/src/shared/i18n/locales/{lang}/*.json` for each supported language
Applied to files:
apps/frontend/src/shared/i18n/locales/fr/settings.jsonapps/frontend/src/shared/i18n/locales/en/settings.json
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx,jsx} : Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Applied to files:
apps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsx
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx} : Use `useTranslation()` hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Applied to files:
apps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsx
🧬 Code graph analysis (7)
apps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsx (1)
apps/frontend/src/renderer/components/settings/ProfileEditDialog.tsx (1)
ProfileEditDialog(50-586)
apps/frontend/src/renderer/components/settings/ProfileList.tsx (3)
apps/frontend/src/renderer/hooks/use-toast.ts (1)
toast(192-192).design-system/src/components/Button.tsx (1)
Button(10-44).design-system/src/lib/utils.ts (1)
cn(4-6)
apps/frontend/src/renderer/components/ideation/hooks/useIdeation.ts (1)
apps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.ts (1)
useIdeationAuth(16-87)
apps/frontend/src/main/insights/config.ts (2)
apps/frontend/src/main/agent/env-utils.ts (1)
getOAuthModeClearVars(22-39)apps/frontend/src/main/python-env-manager.ts (1)
pythonEnvManager(690-690)
apps/frontend/src/main/__tests__/insights-config.test.ts (1)
apps/frontend/src/main/insights/config.ts (1)
InsightsConfig(15-141)
apps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.ts (2)
apps/frontend/src/shared/types/insights.ts (2)
IdeationGenerationStatus(140-146)IdeationType(13-19)apps/frontend/src/renderer/components/ideation/hooks/useIdeation.ts (1)
useIdeation(28-262)
apps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.ts (2)
apps/frontend/src/renderer/stores/settings-store.ts (1)
useSettingsStore(45-295)apps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.ts (1)
useIdeationAuth(16-87)
🔇 Additional comments (26)
apps/frontend/src/__tests__/setup.ts (1)
31-37: LGTM! Clean polyfill for jsdom compatibility.The
scrollIntoViewpolyfill is properly guarded and necessary for testing Radix Select components in jsdom. The implementation correctly checks for both the existence ofHTMLElementand whether the method is already defined before adding the mock.apps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx (1)
257-260: LGTM! Dropdown positioning improved.The dropdown now renders above the input with proper spacing and testability. This is well-tested in the accompanying test file.
apps/frontend/src/main/insights/insights-executor.ts (1)
88-88: LGTM! Async environment retrieval is correctly implemented.The change from synchronous to asynchronous environment retrieval is appropriate and aligns with the broader refactoring where
getProcessEnv()now returns a Promise. The await is correctly placed within the asyncexecute()method.apps/frontend/src/shared/constants/index.ts (1)
33-34: LGTM! Clean barrel export.The new export for API profile presets follows the existing pattern and includes a descriptive comment.
apps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsx (1)
135-158: LGTM! Good test coverage for dropdown positioning.The test properly verifies that the dropdown renders above the input with the correct
bottom-fullclass and is accessible via the test ID. Clear and focused test case.apps/frontend/src/renderer/components/ideation/hooks/useIdeation.ts (1)
18-18: LGTM! Clean migration to the new authentication hook.The refactoring consistently updates the authentication flow from
useClaudeTokenChecktouseIdeationAuth, and renamescheckTokentocheckAuththroughout. The changes maintain the same behavior while aligning with the broader API profile authentication architecture.Also applies to: 52-52, 88-88
apps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.ts (1)
1-173: LGTM! Comprehensive test coverage for the authentication flow.The test suite effectively covers the key scenarios:
- Listener lifecycle management
- Auth-gated generation/refresh/append flows
- Env config modal triggering when auth is missing
- Retry logic after auth configuration
The mock setup correctly simulates the
useIdeationAuthhook behavior and allows for testing different auth states.apps/frontend/src/shared/i18n/locales/fr/settings.json (1)
53-167: LGTM! Comprehensive French translations for API profiles.The
apiProfilestranslation block is complete and covers all UI strings for the API profile management feature, including presets, validation messages, and toast notifications. This aligns with the coding guidelines requirement to add translation keys to all language files (minimum: en and fr).As per coding guidelines, translation keys for new features must be added to all language files.
apps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsx (3)
274-290: LGTM! Preset application test is well-structured.The test correctly verifies that selecting the "GLM (Global)" preset populates both the base URL and name fields as expected. The test flow properly simulates user interaction with the preset dropdown.
292-311: LGTM! Important UX test for name preservation.This test verifies that applying a preset after manually entering a name preserves the user's custom name while updating the base URL. This prevents unexpected overwrites and ensures a good user experience.
313-330: LGTM! Good accessibility test for focus management.This test verifies that focus automatically moves to the Base URL input after selecting a preset, supporting smooth keyboard navigation and improving the user experience.
apps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.ts (1)
51-51: LGTM! Consistent usage of the helper function.The
resolveHasAPIProfilehelper is used consistently in both the automatic check (useEffect) and manual re-check (checkAuth) paths. TheBoolean()wrapper ensures proper type coercion for reliable boolean evaluation.Also applies to: 75-77
apps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.ts (3)
21-21: LGTM: Mock setup for IPC profile retrieval.The mock structure correctly follows the existing pattern for
mockCheckSourceTokenand provides an appropriate default implementation returning an empty profiles list with null activeProfileId.Also applies to: 41-41, 50-57
201-235: LGTM: IPC fallback test validates async profile resolution.This test correctly validates that when
activeProfileIdis missing from the store, the hook falls back to IPC-based profile retrieval viagetAPIProfiles, aligning with the implementation inuseIdeationAuth.ts.
237-255: LGTM: Optimization path test confirms IPC is skipped when profile is known.This test correctly validates that the IPC call to
getAPIProfilesis avoided whenactiveProfileIdis already present in the store, confirming the optimization logic in the hook.apps/frontend/src/renderer/components/settings/ProfileList.tsx (1)
10-10: Excellent i18n implementation throughout the component.All user-facing strings now use translation keys with proper namespace prefixing (
settings:apiProfiles.*), dynamic interpolation is correctly implemented using{{name}}and{{models}}syntax, and theisActiveflag properly controls conditional UI states (disabled delete button, conditional tooltips).This fully addresses the previous review feedback about hardcoded strings.
Based on coding guidelines and learnings.
Also applies to: 36-36, 62-330
apps/frontend/src/main/insights/config.ts (2)
5-7: LGTM: Async refactor enables API profile integration.The method signature change to
asyncis necessary to awaitgetAPIProfileEnv()from IPC. The new imports support the expanded environment composition logic for API profiles and Python environment integration.Also applies to: 110-110
128-139: LGTM: Environment merge order correctly prioritizes API profile settings.The spread order ensures
apiProfileEnv(line 134) takes precedence overoauthModeClearVars(line 132), allowing active API profiles to override cleared ANTHROPIC_* variables. Python-specific environment variables are appropriately set unconditionally.apps/frontend/src/main/__tests__/insights-config.test.ts (1)
1-99: Excellent test coverage addressing previous feedback.This test suite comprehensively covers:
- Full environment composition with all sources merged correctly
- OAuth mode clearing of ANTHROPIC_* variables when no API profile is active
- PYTHONPATH composition edge cases (python env only, backend path only)
The mock setup is clean with proper cleanup, and all tests correctly await the async
getProcessEnv()method. This implementation directly addresses the suggestions from the previous review comment.apps/frontend/src/shared/i18n/locales/en/settings.json (2)
53-167: LGTM: Comprehensive and well-structured i18n translations for API profiles.The
apiProfilestranslation block is thorough and properly organized, covering all UI elements including:
- GLM (Global) and GLM (China) presets (mentioned in PR title)
- Complete CRUD flow with validation messages
- Dynamic interpolation using correct
{{variable}}syntax- Accessible ARIA labels
- Toast notifications with nested structure
The structure aligns with the usage patterns in
ProfileList.tsxand supports the full API profile management workflow.Based on coding guidelines.
53-167: French translations are complete and verified.The
fr/settings.jsonfile contains all requiredapiProfileskeys matching the English version. No action needed.apps/frontend/src/renderer/components/settings/ProfileEditDialog.tsx (5)
16-86: LGTM! Proper i18n and preset infrastructure.The imports, hook setup, and new state management (presetId, baseUrlInputRef) correctly support the preset selector feature and comprehensive internationalization. The
useTranslationhook follows the coding guidelines.
160-199: LGTM! Comprehensive validation with proper i18n.All validation error messages use translation keys (
settings:apiProfiles.validation.*), correctly following the coding guidelines. The conditional API key validation (only in create mode or when changing in edit mode) is appropriate.
201-301: LGTM! Handlers properly internationalized with sound logic.Test connection and save handlers correctly use i18n for all user-facing messages, including toast notifications with proper interpolation. The conditional API key handling (edit vs create mode) is implemented correctly.
303-436: LGTM! Excellent i18n coverage and UX improvements.The dialog structure, form fields, and preset selector are fully internationalized per coding guidelines:
- All labels, placeholders, hints use
t()with proper namespace prefixes (settings:apiProfiles.*)- Responsive grid layout (
md:grid-cols-2) adapts well across screen sizes- Focus management (
onCloseAutoFocusat lines 345-348) provides smooth UX when selecting presets- API key visibility logic correctly handles edit mode (masked with Change button) vs create mode (password input)
438-582: LGTM! Complete i18n implementation across all UI sections.Test connection display, model mappings grid, and dialog footer buttons are fully internationalized:
- All button labels and status messages use proper translation keys
- Model field labels and placeholders follow consistent
settings:apiProfiles.models.*pattern- Responsive grid layout maintains visual consistency
- Translation namespace prefixes correctly applied throughout
apps/frontend/src/renderer/components/settings/ProfileEditDialog.tsx
Outdated
Show resolved
Hide resolved
| export const API_PROVIDER_PRESETS = [ | ||
| { | ||
| id: 'anthropic', | ||
| baseUrl: 'https://api.anthropic.com', | ||
| labelKey: 'settings:apiProfiles.presets.anthropic' | ||
| }, | ||
| { | ||
| id: 'openrouter', | ||
| baseUrl: 'https://openrouter.ai/api/v1', | ||
| labelKey: 'settings:apiProfiles.presets.openrouter' | ||
| }, | ||
| { | ||
| id: 'groq', | ||
| baseUrl: 'https://api.groq.com/openai/v1', | ||
| labelKey: 'settings:apiProfiles.presets.groq' | ||
| }, | ||
| { | ||
| id: 'glm-global', | ||
| baseUrl: 'https://api.z.ai/api/anthropic', | ||
| labelKey: 'settings:apiProfiles.presets.glmGlobal' | ||
| }, | ||
| { | ||
| id: 'glm-cn', | ||
| baseUrl: 'https://open.bigmodel.cn/api/paas/v4', | ||
| labelKey: 'settings:apiProfiles.presets.glmChina' | ||
| } | ||
| ] as const; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Optional: Consider defining an explicit type for better maintainability.
While as const provides type safety, adding an explicit TypeScript type would improve code documentation and make the structure more reusable if other parts of the codebase need to reference preset types.
🔎 Suggested type definition
+/**
+ * API provider preset configuration
+ */
+export interface ApiProviderPreset {
+ readonly id: string;
+ readonly baseUrl: string;
+ readonly labelKey: string;
+}
+
-export const API_PROVIDER_PRESETS = [
+export const API_PROVIDER_PRESETS: readonly ApiProviderPreset[] = [
{
id: 'anthropic',
baseUrl: 'https://api.anthropic.com',
labelKey: 'settings:apiProfiles.presets.anthropic'
},
// ... rest of presets
] as const;This provides better IntelliSense and documentation while maintaining type safety.
📝 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.
| export const API_PROVIDER_PRESETS = [ | |
| { | |
| id: 'anthropic', | |
| baseUrl: 'https://api.anthropic.com', | |
| labelKey: 'settings:apiProfiles.presets.anthropic' | |
| }, | |
| { | |
| id: 'openrouter', | |
| baseUrl: 'https://openrouter.ai/api/v1', | |
| labelKey: 'settings:apiProfiles.presets.openrouter' | |
| }, | |
| { | |
| id: 'groq', | |
| baseUrl: 'https://api.groq.com/openai/v1', | |
| labelKey: 'settings:apiProfiles.presets.groq' | |
| }, | |
| { | |
| id: 'glm-global', | |
| baseUrl: 'https://api.z.ai/api/anthropic', | |
| labelKey: 'settings:apiProfiles.presets.glmGlobal' | |
| }, | |
| { | |
| id: 'glm-cn', | |
| baseUrl: 'https://open.bigmodel.cn/api/paas/v4', | |
| labelKey: 'settings:apiProfiles.presets.glmChina' | |
| } | |
| ] as const; | |
| /** | |
| * API provider preset configuration | |
| */ | |
| export interface ApiProviderPreset { | |
| readonly id: string; | |
| readonly baseUrl: string; | |
| readonly labelKey: string; | |
| } | |
| export const API_PROVIDER_PRESETS: readonly ApiProviderPreset[] = [ | |
| { | |
| id: 'anthropic', | |
| baseUrl: 'https://api.anthropic.com', | |
| labelKey: 'settings:apiProfiles.presets.anthropic' | |
| }, | |
| { | |
| id: 'openrouter', | |
| baseUrl: 'https://openrouter.ai/api/v1', | |
| labelKey: 'settings:apiProfiles.presets.openrouter' | |
| }, | |
| { | |
| id: 'groq', | |
| baseUrl: 'https://api.groq.com/openai/v1', | |
| labelKey: 'settings:apiProfiles.presets.groq' | |
| }, | |
| { | |
| id: 'glm-global', | |
| baseUrl: 'https://api.z.ai/api/anthropic', | |
| labelKey: 'settings:apiProfiles.presets.glmGlobal' | |
| }, | |
| { | |
| id: 'glm-cn', | |
| baseUrl: 'https://open.bigmodel.cn/api/paas/v4', | |
| labelKey: 'settings:apiProfiles.presets.glmChina' | |
| } | |
| ] as const; |
🤖 Prompt for AI Agents
In apps/frontend/src/shared/constants/api-profiles.ts around lines 1 to 27, add
an explicit TypeScript type for the preset entries (e.g. export type
ApiProviderPreset = { id: string; baseUrl: string; labelKey: string }) and
annotate the exported constant with that type (e.g. export const
API_PROVIDER_PRESETS: readonly ApiProviderPreset[] = [...]); export the type if
it will be reused elsewhere and keep the existing array values (you can remove
or keep as const but ensure the constant is typed to the new ApiProviderPreset
type for clearer documentation and reuse).
🧩 Analysis chain
🏁 Script executed:
cat -n apps/frontend/src/shared/constants/api-profiles.tsRepository: AndyMik90/Auto-Claude
Length of output: 950
🌐 Web query:
GLM API official endpoint documentation Anthropic compatible base URL
💡 Result:
Official GLM (Zhipu/BigModel) Anthropic-compatible base URL and endpoint:
- Base URL (Anthropic-compatible): https://open.bigmodel.cn/api/anthropic.
- Example endpoint for the Messages API: https://open.bigmodel.cn/api/anthropic/v1/messages (use POST with Anthropic Messages payload).
- Auth: pass your Zhipu/BigModel API key (examples use x-api-key or ANTHROPIC_AUTH_TOKEN / ANTHROPIC_API_KEY). (docs.bigmodel.cn)
Citations:
🌐 Web query:
Bigmodel.cn API endpoint zhipu ai official documentation
💡 Result:
Official Zhipu (BigModel) API docs and endpoint:
- Base (general) API endpoint: https://open.bigmodel.cn/api/paas/v4. [1]
- Coding-specific endpoint (for GLM coding packages): https://open.bigmodel.cn/api/coding/paas/v4. [1]
- Common chat completions path (example): POST https://open.bigmodel.cn/api/paas/v4/chat/completions. [1][2]
Minimal cURL example:
curl -X POST "https://open.bigmodel.cn/api/paas/v4/chat/completions"
-H "Content-Type: application/json"
-H "Authorization: Bearer YOUR_API_KEY"
-d '{"model":"glm-4.7","messages":[{"role":"user","content":"Hello"}]}'.
Sources: Zhipu/BigModel official docs. [1] https://docs.bigmodel.cn/cn/api/introduction
[2] https://docs.bigmodel.cn/cn/guide/platform/model-migration
Update GLM Global endpoint to official URL.
The GLM Global endpoint is incorrect. Official GLM documentation confirms the Anthropic-compatible endpoint should be https://open.bigmodel.cn/api/anthropic, not https://api.z.ai/api/anthropic. The GLM China endpoint (https://open.bigmodel.cn/api/paas/v4) is correct. All i18n labelKey patterns are properly implemented with namespace prefixes and follow the coding guidelines.
🤖 Prompt for AI Agents
In apps/frontend/src/shared/constants/api-profiles.ts around lines 1 to 27, the
GLM Global preset uses the wrong baseUrl; replace the glm-global entry's baseUrl
value from "https://api.z.ai/api/anthropic" to the official endpoint
"https://open.bigmodel.cn/api/anthropic" so the Anthropic-compatible GLM Global
preset points to the correct URL.
There was a problem hiding this comment.
There are two variant of the China and Global endpoints, OpenAI compatable and Anthropic compatable. endpoints implemented in code is aligned to claude code cli usage using the Anthropic endpoints
There was a problem hiding this comment.
@StillKnotKnown, thank you for the clarification! I understand now—the GLM endpoints have both OpenAI-compatible and Anthropic-compatible variants, and you've intentionally chosen the Anthropic-compatible endpoints to align with the claude code CLI usage. The implementation is correct as-is.
My apologies for the confusion. The https://api.z.ai/api/anthropic endpoint for GLM Global and https://open.bigmodel.cn/api/paas/v4 for GLM China are the appropriate choices for your use case. 👍
StillKnotKnown
force-pushed
the
local/glm-insights-debug
branch
from
656fd8b to
b1d59c1
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx (1)
232-232: Use i18n translation keys instead of hardcoded strings.The placeholders and informational text should use translation keys via
useTranslation()hook. According to coding guidelines, all user-facing text in frontend files must use i18n translation keys with namespace prefixes.Affected lines:
- Line 232:
'Enter model name (e.g., claude-3-5-sonnet-20241022)'- Line 268:
'Search models...'- Lines 308-310:
'Model discovery not available. Enter model name manually.'Based on coding guidelines requiring i18n for all user-facing text in
apps/frontend/src/**/*.{ts,tsx,jsx}.🔎 Suggested implementation pattern
+import { useTranslation } from 'react-i18next'; export function ModelSearchableSelect({ // ...props }: ModelSearchableSelectProps) { + const { t } = useTranslation(); const discoverModels = useSettingsStore((state) => state.discoverModels); // ... rest of component - placeholder={modelDiscoveryNotSupported ? 'Enter model name (e.g., claude-3-5-sonnet-20241022)' : placeholder} + placeholder={modelDiscoveryNotSupported ? t('settings:model.manualInputPlaceholder') : placeholder} - placeholder="Search models..." + placeholder={t('settings:model.searchPlaceholder')} - Model discovery not available. Enter model name manually. + {t('settings:model.discoveryNotAvailable')}Note: Add corresponding keys to
apps/frontend/src/shared/i18n/locales/en/settings.jsonand other locale files.Also applies to: 268-268, 308-310
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (17)
apps/frontend/src/__tests__/setup.tsapps/frontend/src/main/__tests__/insights-config.test.tsapps/frontend/src/main/insights/config.tsapps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeation.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsxapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsxapps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/shared/constants/api-profiles.tsapps/frontend/src/shared/constants/index.tsapps/frontend/src/shared/i18n/locales/en/settings.jsonapps/frontend/src/shared/i18n/locales/fr/settings.json
🧰 Additional context used
📓 Path-based instructions (4)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsxapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsxapps/frontend/src/__tests__/setup.tsapps/frontend/src/shared/constants/api-profiles.tsapps/frontend/src/shared/constants/index.tsapps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeation.tsapps/frontend/src/main/insights/config.tsapps/frontend/src/main/__tests__/insights-config.test.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.tsapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsx
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsxapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsxapps/frontend/src/__tests__/setup.tsapps/frontend/src/shared/constants/api-profiles.tsapps/frontend/src/shared/constants/index.tsapps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeation.tsapps/frontend/src/main/insights/config.tsapps/frontend/src/main/__tests__/insights-config.test.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.tsapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsx
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsxapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsxapps/frontend/src/__tests__/setup.tsapps/frontend/src/shared/constants/api-profiles.tsapps/frontend/src/shared/constants/index.tsapps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeation.tsapps/frontend/src/main/insights/config.tsapps/frontend/src/main/__tests__/insights-config.test.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.tsapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsx
apps/frontend/src/shared/i18n/locales/**/*.json
📄 CodeRabbit inference engine (CLAUDE.md)
apps/frontend/src/shared/i18n/locales/**/*.json: Store translation strings in namespace-organized JSON files atapps/frontend/src/shared/i18n/locales/{lang}/*.jsonfor each supported language
When implementing new frontend features, add translation keys to all language files (minimum: en/.json and fr/.json)
Files:
apps/frontend/src/shared/i18n/locales/en/settings.jsonapps/frontend/src/shared/i18n/locales/fr/settings.json
🧠 Learnings (4)
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx,jsx} : Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Applied to files:
apps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsx
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx} : Use `useTranslation()` hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Applied to files:
apps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsx
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : When implementing new frontend features, add translation keys to all language files (minimum: en/*.json and fr/*.json)
Applied to files:
apps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/shared/i18n/locales/en/settings.jsonapps/frontend/src/shared/i18n/locales/fr/settings.json
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : Store translation strings in namespace-organized JSON files at `apps/frontend/src/shared/i18n/locales/{lang}/*.json` for each supported language
Applied to files:
apps/frontend/src/shared/i18n/locales/en/settings.jsonapps/frontend/src/shared/i18n/locales/fr/settings.json
🧬 Code graph analysis (9)
apps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsx (1)
apps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx (1)
ModelSearchableSelect(57-318)
apps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsx (1)
apps/frontend/src/renderer/components/settings/ProfileEditDialog.tsx (1)
ProfileEditDialog(50-584)
apps/frontend/src/renderer/components/settings/ProfileList.tsx (4)
apps/frontend/src/renderer/hooks/use-toast.ts (1)
toast(192-192).design-system/src/components/Button.tsx (1)
Button(10-44).design-system/src/lib/icons.ts (2)
Plus(30-30)Check(21-21).design-system/src/lib/utils.ts (1)
cn(4-6)
apps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.ts (2)
apps/frontend/src/shared/types/insights.ts (2)
IdeationGenerationStatus(140-146)IdeationType(13-19)apps/frontend/src/renderer/components/ideation/hooks/useIdeation.ts (1)
useIdeation(28-262)
apps/frontend/src/renderer/components/ideation/hooks/useIdeation.ts (1)
apps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.ts (1)
useIdeationAuth(16-87)
apps/frontend/src/main/insights/config.ts (2)
apps/frontend/src/main/agent/env-utils.ts (1)
getOAuthModeClearVars(22-39)apps/frontend/src/main/python-env-manager.ts (1)
pythonEnvManager(690-690)
apps/frontend/src/main/__tests__/insights-config.test.ts (1)
apps/frontend/src/main/insights/config.ts (1)
InsightsConfig(15-153)
apps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.ts (2)
apps/frontend/src/renderer/stores/settings-store.ts (1)
useSettingsStore(45-295)apps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.ts (1)
useIdeationAuth(16-87)
apps/frontend/src/renderer/components/settings/ProfileEditDialog.tsx (3)
apps/frontend/src/shared/constants/api-profiles.ts (1)
API_PROVIDER_PRESETS(7-33)apps/frontend/src/renderer/lib/profile-utils.ts (3)
isValidUrl(21-32)isValidApiKey(38-49)maskApiKey(11-16)apps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx (1)
ModelSearchableSelect(57-318)
🔇 Additional comments (29)
apps/frontend/src/renderer/components/ideation/hooks/useIdeation.ts (1)
18-18: LGTM! Clean integration with the new authentication hook.The changes cleanly integrate the new
useIdeationAuthhook, replacing the previous token check mechanism with the unified authentication flow that considers both OAuth tokens and API profiles.Also applies to: 52-52, 88-88
apps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.ts (1)
1-173: Excellent test coverage for the refactored authentication flow!The test suite comprehensively covers all critical paths:
- Listener lifecycle management
- Environment configuration prompts when authentication is missing
- Generation/refresh/append flows with authentication checks
- Verification that
checkAuthis called after environment configurationThe use of hoisted mocks and clear test organization makes this suite maintainable and easy to understand.
apps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.ts (2)
21-21: Good mock setup for IPC-based profile resolution.The mock infrastructure properly supports testing both the optimized path (when
activeProfileIdis already in the store) and the fallback path (IPC call togetAPIProfiles).Also applies to: 41-41, 50-57
201-255: Excellent test coverage for the IPC fallback optimization.These tests validate both scenarios:
- Lines 201-235: When the store's
activeProfileIdis missing, the hook correctly falls back to IPC and retrieves the active profile- Lines 237-255: When the store already has
activeProfileId, the IPC call is skipped (optimization)This ensures the
resolveHasAPIProfilehelper behaves correctly in both code paths.apps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.ts (3)
24-39: Excellent refactoring that addresses the DRY concern from past reviews!The
resolveHasAPIProfilehelper successfully eliminates the duplicate logic previously flagged in review comments. The implementation is clean:
- Early return optimization when
profileIdis already available- Graceful IPC fallback when the store hasn't loaded profiles yet
- Proper error handling with safe fallback to
falseThe function accepts
profileIdas a parameter rather than directly closing over it, which improves testability and makes dependencies explicit.
51-51: Good use of the helper and improved state management.The integration of
resolveHasAPIProfilesimplifies the authentication check logic. The explicitBoolean()coercion ensures type safety, and thefinallyblock guarantees that the loading state is always cleared, even if an error occurs.Also applies to: 54-54, 58-60
67-84: Consistent application of the refactored pattern incheckAuth.The manual re-check function follows the same clean pattern as the effect-based check, using the shared helper and ensuring proper state cleanup with the
finallyblock.apps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx (1)
257-260: LGTM! Dropdown positioning improved.The dropdown now appears above the input using
bottom-full mb-1, which prevents overflow issues. The addition ofdata-testid="model-select-dropdown"enhances testability.apps/frontend/src/__tests__/setup.ts (1)
31-37: LGTM! Necessary jsdom polyfill for Radix Select.The
scrollIntoViewpolyfill is correctly guarded and addresses a common limitation in jsdom environments. This enables components using Radix UI primitives (which callscrollIntoViewinternally) to render without errors during testing.apps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsx (1)
135-158: LGTM! Test validates dropdown positioning change.The test case correctly verifies that the dropdown renders above the input by checking for the
bottom-fullclass and the presence of themodel-select-dropdowntest ID. The test follows proper async patterns and complements the production code change.apps/frontend/src/main/insights/config.ts (3)
117-136: PYTHONPATH deduplication logic is well-implemented.The code properly addresses the concern from previous reviews by:
- Splitting the existing
PYTHONPATHby platform delimiter- Normalizing paths with
path.resolve()- Performing case-insensitive comparison on Windows
- Only adding
autoBuildSourceif not already presentThis prevents duplicate entries and handles platform-specific path comparison correctly.
140-151: Environment merging order is correct for precedence.The environment sources are merged in the correct order:
- Base system environment
- Python environment (venv-specific)
- Auto-build
.envfile- OAuth mode clear vars (empties ANTHROPIC_* when no profile)
- Claude profile (OAuth token)
- API profile (ANTHROPIC_BASE_URL, ANTHROPIC_AUTH_TOKEN)
- Python-specific flags
- Computed PYTHONPATH
This ensures that API profile settings take precedence over OAuth settings, and OAuth mode correctly clears API credentials when no profile is active.
110-110: Breaking change: getProcessEnv() is now async.All callers have been properly updated to await this method. Production code in
insights-executor.tsand all test cases correctly useawait this.config.getProcessEnv()andawait config.getProcessEnv().apps/frontend/src/renderer/components/settings/ProfileList.tsx (1)
10-10: LGTM! Comprehensive i18n integration completed.All user-facing text has been properly replaced with translation keys using the
useTranslation()hook with namespace prefixes (settings:apiProfiles.*). The implementation correctly handles:
- Dynamic interpolation for profile names (e.g.,
name: profile.name)- Toast messages for all user actions
- UI labels, tooltips, and ARIA labels
- Dialog content and button labels
- Empty state and header text
This fully addresses the coding guidelines requirement for i18n in frontend files and resolves the past review comment on this file.
Also applies to: 36-36, 62-330
apps/frontend/src/main/insights/insights-executor.ts (1)
88-88: LGTM! Correctly awaits async environment assembly.The change properly awaits the now-asynchronous
getProcessEnv()method, ensuring the complete environment is resolved before spawning the Python process. This aligns with the expanded environment assembly that now merges multiple sources asynchronously.apps/frontend/src/shared/constants/index.ts (1)
33-34: LGTM! Clean re-export following established patterns.The API profile presets export is properly documented and follows the same structure as other constant re-exports in the file.
apps/frontend/src/shared/i18n/locales/fr/settings.json (1)
53-167: Comprehensive French translations for API profiles feature.The translation block is complete and properly structured, covering all UI elements for the API profiles feature. The translations follow the established i18n organization pattern and comply with the coding guideline requiring translation keys in all language files (minimum EN and FR).
Based on coding guidelines: Translation strings are properly stored in namespace-organized JSON files with complete coverage for the new API profiles feature.
apps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsx (4)
13-13: Good practice: i18n initialization for tests.Importing the shared i18n module ensures proper localization setup for component tests that use translation hooks.
274-290: LGTM! Test correctly verifies preset application in create mode.The test properly validates that selecting a preset (GLM Global) auto-fills both the baseUrl and name fields, using appropriate async patterns and keyboard interaction simulation.
292-311: LGTM! Test validates name preservation logic.The test correctly verifies that when a user has manually entered a profile name, selecting a preset only updates the baseUrl and preserves the custom name. This aligns with the conditional preset application behavior described in the component implementation.
313-330: LGTM! Test validates proper focus management.The test correctly verifies that after selecting a preset, focus automatically moves to the Base URL input, ensuring smooth keyboard navigation flow in the create profile workflow.
apps/frontend/src/shared/constants/api-profiles.ts (1)
1-33: LGTM! Well-typed preset definitions with proper i18n integration.The type definition and preset array are cleanly implemented:
- Explicit
ApiProviderPresettype provides clear documentation and reusability- All presets follow consistent structure with valid HTTPS endpoints
- labelKey values properly reference i18n translation keys with namespace prefixes
- GLM endpoints confirmed as correct Anthropic-compatible variants (per previous review discussion)
Based on coding guidelines: Translation keys use proper namespace prefixes (e.g., 'settings:apiProfiles.presets.anthropic') matching the i18n locale structure.
apps/frontend/src/main/__tests__/insights-config.test.ts (1)
1-99: Excellent test coverage for the async environment refactor.The test suite comprehensively validates the
getProcessEnvrefactor with proper edge-case coverage. The four tests correctly verify:
- Environment source merging (process.env, autoBuild, profile, API profile, pythonEnv)
- PYTHONPATH composition with platform-specific delimiter
- OAuth mode clearing of ANTHROPIC_* variables
- PYTHONPATH fallback behavior when sources are missing
The mock setup is clean with proper cleanup in
afterEach, and the tests align with the edge cases requested in previous reviews.apps/frontend/src/shared/i18n/locales/en/settings.json (1)
53-167: Well-structured i18n implementation for API profiles feature.The
apiProfilestranslation block is comprehensive and properly organized, covering all UI elements including the new GLM presets mentioned in the PR objectives. The structure includes:
- Provider presets (Anthropic, OpenRouter, Groq, GLM Global, GLM China)
- Form fields, validation, and actions
- Model mappings and test connection flows
- Toast notifications and dialog text
Based on the AI summary, the French locale file also includes a corresponding
apiProfilesblock, satisfying the coding guideline requirement for multi-language support.Based on coding guidelines and learnings, multi-language translation support is properly implemented.
apps/frontend/src/renderer/components/settings/ProfileEditDialog.tsx (5)
18-18: LGTM: Clean i18n and preset integration setup.The imports and hook initialization properly introduce internationalization (
useTranslation) and API provider presets (API_PROVIDER_PRESETS), following the coding guideline requirement to use theuseTranslation()hook with namespace prefixes for React components.As per coding guidelines, proper i18n setup for frontend components.
Also applies to: 30-30, 37-37, 51-51
148-156: Solid preset application logic with i18n integration.The
applyPresetfunction correctly:
- Applies the selected provider's base URL
- Auto-fills the profile name using the translated preset label only when empty (preserving user input)
- Directly uses
API_PROVIDER_PRESETSwithout intermediate variablesThe conditional name assignment prevents overwriting user-provided names, which is good UX.
159-234: Complete i18n coverage for validation and form checks.All validation error messages and form validation logic properly use translation keys with the correct namespace prefix (
settings:apiProfiles.validation.*). No hardcoded user-facing strings remain in the validation flow.As per coding guidelines, all user-facing validation text uses i18n translation keys.
303-547: Comprehensive i18n coverage throughout the dialog UI.All user-facing text elements are properly internationalized with translation keys:
- Dialog titles, descriptions, and actions
- Form field labels, placeholders, and hints
- Validation errors and status messages
- Preset selection options (using translated
labelKeyfrom presets)- API key handling UI (masked display, Change/Cancel buttons)
- Model mapping labels and placeholders
- Test connection flow messages
The responsive grid layouts (
md:grid-cols-2) maintain good UX while organizing the increased content. The DialogContent width constraint (w-[min(92vw,720px)]) appears to resolve the past review concern about conflicting width values.As per coding guidelines, all frontend user-facing text uses i18n translation keys with proper namespace prefixes.
236-299: Toast notifications properly internationalized.Both create and update success toasts correctly use translation keys with dynamic name interpolation (
t('settings:apiProfiles.toast.*.description', { name })), providing localized user feedback.
StillKnotKnown
force-pushed
the
local/glm-insights-debug
branch
from
b1d59c1 to
94cb8df
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (18)
apps/frontend/src/__tests__/setup.tsapps/frontend/src/main/__tests__/insights-config.test.tsapps/frontend/src/main/insights/config.tsapps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeation.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsxapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsxapps/frontend/src/renderer/components/settings/ProfileList.test.tsxapps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/shared/constants/api-profiles.tsapps/frontend/src/shared/constants/index.tsapps/frontend/src/shared/i18n/locales/en/settings.jsonapps/frontend/src/shared/i18n/locales/fr/settings.json
🧰 Additional context used
📓 Path-based instructions (4)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.tsapps/frontend/src/main/insights/config.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeation.tsapps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsxapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.tsapps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/renderer/components/settings/ProfileList.test.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsxapps/frontend/src/main/__tests__/insights-config.test.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsxapps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/__tests__/setup.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsxapps/frontend/src/shared/constants/index.tsapps/frontend/src/shared/constants/api-profiles.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.tsapps/frontend/src/main/insights/config.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeation.tsapps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsxapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.tsapps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/renderer/components/settings/ProfileList.test.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsxapps/frontend/src/main/__tests__/insights-config.test.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsxapps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/__tests__/setup.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsxapps/frontend/src/shared/constants/index.tsapps/frontend/src/shared/constants/api-profiles.ts
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.tsapps/frontend/src/main/insights/config.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeation.tsapps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsxapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.tsapps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/renderer/components/settings/ProfileList.test.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsxapps/frontend/src/main/__tests__/insights-config.test.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsxapps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/__tests__/setup.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsxapps/frontend/src/shared/constants/index.tsapps/frontend/src/shared/constants/api-profiles.ts
apps/frontend/src/shared/i18n/locales/**/*.json
📄 CodeRabbit inference engine (CLAUDE.md)
apps/frontend/src/shared/i18n/locales/**/*.json: Store translation strings in namespace-organized JSON files atapps/frontend/src/shared/i18n/locales/{lang}/*.jsonfor each supported language
When implementing new frontend features, add translation keys to all language files (minimum: en/.json and fr/.json)
Files:
apps/frontend/src/shared/i18n/locales/fr/settings.jsonapps/frontend/src/shared/i18n/locales/en/settings.json
🧠 Learnings (4)
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx,jsx} : Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Applied to files:
apps/frontend/src/renderer/components/settings/ProfileList.test.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsxapps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx} : Use `useTranslation()` hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Applied to files:
apps/frontend/src/renderer/components/settings/ProfileList.test.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsxapps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : When implementing new frontend features, add translation keys to all language files (minimum: en/*.json and fr/*.json)
Applied to files:
apps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/shared/i18n/locales/fr/settings.jsonapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsxapps/frontend/src/shared/i18n/locales/en/settings.json
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : Store translation strings in namespace-organized JSON files at `apps/frontend/src/shared/i18n/locales/{lang}/*.json` for each supported language
Applied to files:
apps/frontend/src/shared/i18n/locales/fr/settings.jsonapps/frontend/src/shared/i18n/locales/en/settings.json
🧬 Code graph analysis (7)
apps/frontend/src/main/insights/config.ts (2)
apps/frontend/src/main/agent/env-utils.ts (1)
getOAuthModeClearVars(22-39)apps/frontend/src/main/python-env-manager.ts (1)
pythonEnvManager(690-690)
apps/frontend/src/renderer/components/ideation/hooks/useIdeation.ts (1)
apps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.ts (1)
useIdeationAuth(16-87)
apps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsx (1)
apps/frontend/src/renderer/components/settings/ProfileEditDialog.tsx (1)
ProfileEditDialog(50-584)
apps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.ts (2)
apps/frontend/src/renderer/stores/settings-store.ts (1)
useSettingsStore(45-295)apps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.ts (1)
useIdeationAuth(16-87)
apps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.ts (2)
apps/frontend/src/shared/types/insights.ts (2)
IdeationGenerationStatus(140-146)IdeationType(13-19)apps/frontend/src/renderer/components/ideation/hooks/useIdeation.ts (1)
useIdeation(28-262)
apps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsx (1)
apps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx (1)
ModelSearchableSelect(58-323)
apps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx (1)
.design-system/src/components/Input.tsx (1)
Input(4-24)
🔇 Additional comments (48)
apps/frontend/src/renderer/components/ideation/hooks/useIdeation.ts (1)
18-18: LGTM! Clean integration with the new auth hook.The refactoring from
useClaudeTokenChecktouseIdeationAuthis straightforward and maintains the same behavior. The rename fromcheckTokentocheckAuthis clear and consistent.Also applies to: 52-52, 88-88
apps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.ts (1)
21-21: Excellent test coverage for IPC fallback logic.The new tests properly verify the optimization where:
- IPC is called when the store doesn't have an
activeProfileId(fallback scenario)- IPC is skipped when the store already has an
activeProfileId(performance optimization)This aligns well with the implementation's trust-based approach and ensures the fallback mechanism works correctly.
Also applies to: 41-41, 50-57, 201-255
apps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.ts (1)
1-173: Well-structured test file with good coverage.The tests properly cover the main flows:
- Listener lifecycle
- Environment configuration gating
- Authentication checks before actions
- Retry logic after configuration
The use of hoisted mocks and clear test descriptions makes the suite maintainable.
apps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.ts (1)
51-60: Excellent addition of finally blocks.The
finallyblocks ensure thatisLoadingis always reset tofalse, even when errors occur. This prevents the hook from getting stuck in a loading state, which would block the UI. This is a critical reliability improvement.Also applies to: 75-83
apps/frontend/src/shared/constants/api-profiles.ts (1)
1-33: LGTM! Well-structured API provider presets.The implementation is clean and follows best practices:
- Explicit
ApiProviderPresettype improves maintainability- Readonly array prevents accidental mutations
- All
labelKeyvalues follow proper i18n namespace format- GLM endpoints align with Anthropic-compatible variants as clarified in previous review
apps/frontend/src/shared/constants/index.ts (1)
33-35: LGTM! Proper re-export pattern.The re-export follows the existing convention in this file and properly exposes the API provider presets to consumers.
apps/frontend/src/main/insights/config.ts (4)
5-7: LGTM! Necessary imports for API profile and Python environment handling.The imports support the new async environment assembly that includes API profile variables and enhanced Python path management.
115-138: LGTM! Robust PYTHONPATH composition with proper deduplication.The implementation correctly:
- Normalizes and resolves all path entries
- Handles Windows case-insensitive path comparison (lines 125-130)
- Deduplicates the autoBuildSource path to prevent duplicates
- Filters empty entries
- Uses platform-specific path delimiter
140-152: LGTM! Proper environment variable merging with correct precedence.The environment assembly is well-structured:
- Logical merge order ensures proper variable precedence
- OAuth mode clear vars positioned correctly to clear ANTHROPIC_* vars before profile/API profile vars
- Python-specific variables ensure proper UTF-8 handling and unbuffered output
- PYTHONPATH conditionally added only when non-empty
110-114: Async implementation is correct and properly awaited.The method signature correctly changes to async to support awaiting
getAPIProfileEnv(), andInsightsExecutorproperly awaits the call at line 88. The environment variable merging logic is sound.apps/frontend/src/__tests__/setup.ts (1)
31-37: LGTM! Proper jsdom polyfill for Radix Select.The
scrollIntoViewpolyfill is well-implemented:
- Guards prevent overwriting existing implementations
- Uses
vi.fn()for a test-friendly no-op mock- Writable property allows test-specific overrides if needed
- Solves the common issue of Radix UI components requiring this DOM API in jsdom
apps/frontend/src/shared/i18n/locales/fr/settings.json (1)
53-167: Verify that corresponding English translations exist in apps/frontend/src/shared/i18n/locales/en/settings.json.The French translations for the API profiles feature are comprehensive and well-structured. Per coding guidelines, all translation keys must exist in both English and French locale files (minimum: en/.json and fr/.json). Confirm the apiProfiles section and all nested keys are present in the English translations to satisfy the i18n requirements.
apps/frontend/src/main/insights/insights-executor.ts (1)
88-88: LGTM! Correct async/await pattern.The change properly awaits the asynchronous
getProcessEnv()call, ensuring the environment is fully resolved before spawning the Python process. Error handling is already in place via the Promise constructor.apps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsx (2)
11-11: Good practice: i18n import added.The i18n import ensures the test suite has access to localized strings used by the component, aligning with the frontend coding guidelines.
136-159: LGTM! Well-structured dropdown positioning test.The test correctly verifies that the dropdown renders above the input with the
bottom-fullclass. Proper async/await patterns are used, and the test aligns with the component's implementation.apps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsx (2)
13-13: Good practice: i18n import added.Ensures the test suite has access to localized strings, following the frontend i18n guidelines.
274-330: Excellent preset functionality coverage.These three tests comprehensively verify preset behavior:
- Preset values populate Base URL and name
- Existing name values are preserved when applying a preset
- Focus management enhances UX by moving to Base URL after selection
All tests use proper async/await patterns and appropriate assertions. Well done!
apps/frontend/src/renderer/components/settings/ProfileList.test.tsx (2)
16-16: Good adherence to i18n guidelines.The i18n import enables the test suite to use translation keys, aligning with the frontend coding guidelines.
208-210: Excellent i18n integration in tests.All user-facing strings have been correctly replaced with
i18n.t()calls using proper translation keys. The test at line 230 was appropriately made async to accommodatefindByText, and the mock setup ensures proper test isolation. This fully aligns with the coding guidelines for i18n usage.Also applies to: 221-226, 230-248, 266-267, 280-280, 291-294, 303-303
apps/frontend/src/main/__tests__/insights-config.test.ts (1)
1-99: Comprehensive test coverage for async environment composition.This test suite effectively validates the
InsightsConfig.getProcessEnv()refactor with:
- Proper async/await patterns throughout
- Comprehensive mocking of all dependencies (electron, profile service, python-env-manager)
- Four well-structured scenarios covering composite environment merging and PYTHONPATH logic
- Edge-case coverage for OAuth mode and varying PYTHONPATH configurations
The tests address the previous review feedback by including the suggested edge cases. Great work on ensuring the environment assembly logic is thoroughly validated!
apps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx (7)
20-20: LGTM! Proper i18n hook integration.The import and usage of
useTranslation()follows the coding guidelines, using namespace-prefixed keys (settings:modelSelect.placeholder). TheresolvedPlaceholderlogic correctly falls back to the translation when no explicit placeholder is provided.As per coding guidelines, all user-facing text must use i18n translation keys.
Also applies to: 67-68
235-237: LGTM! Conditional placeholder properly internationalized.Both branches of the ternary operator use translated strings—either
placeholderManualfor manual input mode or theresolvedPlaceholderfor dropdown mode. This aligns with i18n requirements.As per coding guidelines, all user-facing text must use i18n translation keys.
273-273: LGTM! Search placeholder uses i18n.The search input placeholder correctly uses the translation key
settings:modelSelect.searchPlaceholder.As per coding guidelines, all user-facing text must use i18n translation keys.
284-284: LGTM! No results message uses i18n.The empty state message correctly uses the translation key
settings:modelSelect.noResults.As per coding guidelines, all user-facing text must use i18n translation keys.
315-315: LGTM! Discovery unavailable message uses i18n.The informational message correctly uses the translation key
settings:modelSelect.discoveryNotAvailable.As per coding guidelines, all user-facing text must use i18n translation keys.
20-20: No action required — French translations already exist for all modelSelect keys.The
apps/frontend/src/shared/i18n/locales/fr/settings.jsonfile contains complete translations for all modelSelect entries (placeholder, placeholderManual, searchPlaceholder, noResults, discoveryNotAvailable), matching the English locale.Likely an incorrect or invalid review comment.
262-265: The dropdown positioning is correctly implemented. The component uses a relative parent container (line 221) that establishes the positioning context, so thebottom-full mb-1positioning is relative to the parent element, not the viewport. This works correctly in scrollable containers and near the top of the view—the dropdown will scroll naturally with its parent container. The change frommt-1(below input) tobottom-full(above input) is intentional and the implementation is sound.apps/frontend/src/renderer/components/settings/ProfileList.tsx (8)
10-10: LGTM! Proper i18n hook integration.The import and usage of
useTranslation()follows the coding guidelines.As per coding guidelines, all user-facing text must use i18n translation keys.
Also applies to: 36-36
62-76: LGTM! Delete toast messages properly internationalized.Both success and error toast messages use i18n keys with correct namespace prefixes. The dynamic name interpolation follows the correct pattern.
As per coding guidelines, all user-facing text must use i18n translation keys.
98-121: LGTM! Switch toast messages properly internationalized.All switch-related toast messages (OAuth and profile) use i18n keys with correct namespace prefixes and proper name interpolation.
As per coding guidelines, all user-facing text must use i18n translation keys.
140-148: LGTM! Header section properly internationalized.The title, description, and add button text all use i18n keys with correct namespace prefixes.
As per coding guidelines, all user-facing text must use i18n translation keys.
155-161: LGTM! Empty state properly internationalized.The empty state title, description, and action button all use i18n keys with correct namespace prefixes.
As per coding guidelines, all user-facing text must use i18n translation keys.
184-283: LGTM! Per-profile rendering properly internationalized.The refactored profile list rendering correctly uses i18n keys for all user-facing text including:
- Active badge
- Custom models display with interpolation
- Set Active button states
- Tooltips (edit and conditional delete)
- ARIA labels with name interpolation
The introduction of the local
isActivevariable improves code clarity.As per coding guidelines, all user-facing text must use i18n translation keys.
311-330: LGTM! Delete dialog properly internationalized.The delete confirmation dialog uses i18n keys for all text including title, description, and buttons. The name interpolation correctly includes a fallback value.
As per coding guidelines, all user-facing text must use i18n translation keys.
36-330: French translations are complete for all apiProfiles keys.Verification confirms that
apps/frontend/src/shared/i18n/locales/fr/settings.jsoncontains the fullapiProfilesblock with all 19 top-level keys and 77 translation strings, matching the English locale exactly. No missing translations detected.apps/frontend/src/shared/i18n/locales/en/settings.json (4)
53-167: LGTM! Comprehensive apiProfiles translation block.The new
apiProfilesblock is well-structured with:
- Logical nested organization (presets, fields, actions, toast, dialog)
- Consistent camelCase naming conventions
- Correct i18next interpolation syntax (
{{name}},{{models}})- Comprehensive coverage for all UI states (loading, errors, success, tooltips, ARIA labels)
As per coding guidelines, translation strings should be stored in namespace-organized JSON files.
168-174: LGTM! Complete modelSelect translation block.The new
modelSelectblock provides all necessary translations for the ModelSearchableSelect component with clear, descriptive keys.As per coding guidelines, translation strings should be stored in namespace-organized JSON files.
53-174: French translations are complete.Both
apiProfilesandmodelSelectblocks are present inapps/frontend/src/shared/i18n/locales/fr/settings.jsonwith matching key counts (77 keys for apiProfiles in both EN and FR files).
53-174: JSON syntax is valid and both language files are properly updated.Verification confirms that
en/settings.jsonandfr/settings.jsonboth contain valid JSON syntax and include the newapiProfilesandmodelSelecttranslation sections, meeting the requirement to add translation keys to all supported language files.apps/frontend/src/renderer/components/settings/ProfileEditDialog.tsx (9)
18-18: LGTM! Proper imports and i18n hook integration.The imports are correct and the
useTranslation()hook follows the coding guidelines with namespace-prefixed keys.As per coding guidelines, all user-facing text must use i18n translation keys.
Also applies to: 30-30, 37-37, 51-51
148-156: LGTM! Preset application logic is correct.The
applyPresetfunction correctly:
- Finds the preset by ID
- Populates
baseUrlfrom the preset- Translates and sets the profile name only when empty (good UX)
164-197: LGTM! Form validation properly internationalized.All validation error messages use i18n keys with correct namespace prefixes. The validation logic correctly handles both create and edit modes, with conditional API key validation.
As per coding guidelines, all user-facing text must use i18n translation keys.
208-212: LGTM! Test connection validation messages properly internationalized.The test connection error messages correctly use i18n keys, consistent with the main validation logic.
As per coding guidelines, all user-facing text must use i18n translation keys.
262-297: LGTM! Save toast messages properly internationalized.Both create and update success toast messages use i18n keys with correct name interpolation.
As per coding guidelines, all user-facing text must use i18n translation keys.
303-316: LGTM! Dialog header properly internationalized.The dialog title and description correctly use i18n keys with conditional rendering for create vs. edit modes.
As per coding guidelines, all user-facing text must use i18n translation keys.
319-433: LGTM! Form fields properly internationalized with good UX.All form fields correctly use i18n keys for:
- Labels and placeholders
- Hints and validation errors
- Preset selector options (translating preset.labelKey)
The
onCloseAutoFocuscallback on the preset selector (lines 342-346) that prevents default and focuses the base URL input is a nice UX touch.As per coding guidelines, all user-facing text must use i18n translation keys.
447-485: LGTM! Test connection UI properly internationalized.The test connection button and result display correctly use i18n keys for all UI text. The
testConnectionResult.messagefrom the backend is displayed as-is, which is expected for technical error messages.As per coding guidelines, all user-facing text must use i18n translation keys.
489-577: LGTM! Model mappings and dialog footer properly internationalized.The optional model mappings section and dialog footer buttons correctly use i18n keys throughout:
- Section title and description
- All model labels and placeholders
- Cancel and Save button labels with loading states
The responsive 2-column grid layout for model selectors is appropriate.
As per coding guidelines, all user-facing text must use i18n translation keys.
StillKnotKnown
force-pushed
the
local/glm-insights-debug
branch
from
94cb8df to
4d35b11
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (18)
apps/frontend/src/__tests__/setup.tsapps/frontend/src/main/__tests__/insights-config.test.tsapps/frontend/src/main/insights/config.tsapps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeation.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.tsapps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsxapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsxapps/frontend/src/renderer/components/settings/ProfileList.test.tsxapps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/shared/constants/api-profiles.tsapps/frontend/src/shared/constants/index.tsapps/frontend/src/shared/i18n/locales/en/settings.jsonapps/frontend/src/shared/i18n/locales/fr/settings.json
🧰 Additional context used
📓 Path-based instructions (4)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/shared/constants/api-profiles.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.tsapps/frontend/src/__tests__/setup.tsapps/frontend/src/main/__tests__/insights-config.test.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeation.tsapps/frontend/src/shared/constants/index.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.tsapps/frontend/src/main/insights/config.tsapps/frontend/src/renderer/components/settings/ProfileList.test.tsxapps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsxapps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsxapps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsxapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/shared/constants/api-profiles.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.tsapps/frontend/src/__tests__/setup.tsapps/frontend/src/main/__tests__/insights-config.test.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeation.tsapps/frontend/src/shared/constants/index.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.tsapps/frontend/src/main/insights/config.tsapps/frontend/src/renderer/components/settings/ProfileList.test.tsxapps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsxapps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsxapps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsxapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/shared/constants/api-profiles.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.tsapps/frontend/src/__tests__/setup.tsapps/frontend/src/main/__tests__/insights-config.test.tsapps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeation.tsapps/frontend/src/shared/constants/index.tsapps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.tsapps/frontend/src/main/insights/config.tsapps/frontend/src/renderer/components/settings/ProfileList.test.tsxapps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsxapps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsxapps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsxapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx
apps/frontend/src/shared/i18n/locales/**/*.json
📄 CodeRabbit inference engine (CLAUDE.md)
apps/frontend/src/shared/i18n/locales/**/*.json: Store translation strings in namespace-organized JSON files atapps/frontend/src/shared/i18n/locales/{lang}/*.jsonfor each supported language
When implementing new frontend features, add translation keys to all language files (minimum: en/.json and fr/.json)
Files:
apps/frontend/src/shared/i18n/locales/fr/settings.jsonapps/frontend/src/shared/i18n/locales/en/settings.json
🧠 Learnings (4)
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : When implementing new frontend features, add translation keys to all language files (minimum: en/*.json and fr/*.json)
Applied to files:
apps/frontend/src/shared/i18n/locales/fr/settings.jsonapps/frontend/src/shared/i18n/locales/en/settings.jsonapps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : Store translation strings in namespace-organized JSON files at `apps/frontend/src/shared/i18n/locales/{lang}/*.json` for each supported language
Applied to files:
apps/frontend/src/shared/i18n/locales/fr/settings.jsonapps/frontend/src/shared/i18n/locales/en/settings.json
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx,jsx} : Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Applied to files:
apps/frontend/src/renderer/components/settings/ProfileList.test.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsxapps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx} : Use `useTranslation()` hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Applied to files:
apps/frontend/src/renderer/components/settings/ProfileList.test.tsxapps/frontend/src/renderer/components/settings/ProfileEditDialog.tsxapps/frontend/src/renderer/components/settings/ProfileList.tsxapps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx
🧬 Code graph analysis (10)
apps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.ts (2)
apps/frontend/src/shared/types/insights.ts (2)
IdeationGenerationStatus(140-146)IdeationType(13-19)apps/frontend/src/renderer/components/ideation/hooks/useIdeation.ts (1)
useIdeation(28-262)
apps/frontend/src/main/__tests__/insights-config.test.ts (1)
apps/frontend/src/main/insights/config.ts (1)
InsightsConfig(15-153)
apps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.ts (2)
apps/frontend/src/renderer/stores/settings-store.ts (1)
useSettingsStore(45-295)apps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.ts (1)
useIdeationAuth(16-88)
apps/frontend/src/renderer/components/ideation/hooks/useIdeation.ts (1)
apps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.ts (1)
useIdeationAuth(16-88)
apps/frontend/src/main/insights/config.ts (2)
apps/frontend/src/main/agent/env-utils.ts (1)
getOAuthModeClearVars(22-39)apps/frontend/src/main/python-env-manager.ts (1)
pythonEnvManager(690-690)
apps/frontend/src/renderer/components/settings/ProfileList.test.tsx (1)
apps/frontend/src/renderer/stores/settings-store.ts (1)
useSettingsStore(45-295)
apps/frontend/src/renderer/components/settings/ProfileEditDialog.tsx (2)
apps/frontend/src/shared/constants/api-profiles.ts (1)
API_PROVIDER_PRESETS(7-33)apps/frontend/src/renderer/lib/profile-utils.ts (3)
isValidUrl(21-32)isValidApiKey(38-49)maskApiKey(11-16)
apps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsx (2)
apps/frontend/src/renderer/components/ui/error-boundary.tsx (1)
render(40-73)apps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx (1)
ModelSearchableSelect(58-323)
apps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsx (1)
apps/frontend/src/renderer/components/settings/ProfileEditDialog.tsx (1)
ProfileEditDialog(50-584)
apps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx (1)
.design-system/src/components/Input.tsx (1)
Input(4-24)
🔇 Additional comments (29)
apps/frontend/src/shared/constants/index.ts (1)
33-34: LGTM!The new re-export follows the established pattern and properly exposes the API profile presets for use throughout the application.
apps/frontend/src/shared/constants/api-profiles.ts (1)
1-33: LGTM!The implementation properly addresses past review feedback with an explicit
ApiProviderPresettype and typed constant. All i18n label keys follow the namespace pattern required by coding guidelines.apps/frontend/src/renderer/components/ideation/hooks/useIdeation.ts (3)
18-18: LGTM!The import update aligns with the new authentication flow that supports both OAuth tokens and API profiles.
52-52: LGTM!The hook destructuring correctly uses the renamed
checkAuthmethod fromuseIdeationAuth.
88-88: LGTM!The call site properly invokes the renamed
checkAuth()method to refresh authentication state after environment configuration.apps/frontend/src/main/insights/config.ts (1)
110-151: All callers properly await the asyncgetProcessEnv()method.The refactoring to async
getProcessEnv()is well-implemented with proper PYTHONPATH deduplication and correct environment merge order. Integration of API profile environment and OAuth clear variables aligns with the new authentication flow. Verification confirms all callers—both in tests (insights-config.test.ts) and production code (insights-executor.ts)—correctly await the Promise.apps/frontend/src/shared/i18n/locales/en/settings.json (1)
53-174: French translations are complete and properly maintained.Verification confirms that the French translation file (
fr/settings.json) contains all required sections with complete key coverage: apiProfiles (77 keys) and modelSelect (5 keys) match the English structure exactly. The coding guideline requirement to add translation keys to all language files (minimum: en and fr) has been satisfied.apps/frontend/src/__tests__/setup.ts (1)
31-37: LGTM! Proper jsdom polyfill for Radix Select.The scrollIntoView polyfill is correctly implemented with defensive checks to avoid overriding existing implementations. This is a standard requirement for Radix UI components in jsdom test environments.
apps/frontend/src/renderer/components/settings/ModelSearchableSelect.test.tsx (2)
11-11: LGTM! Necessary i18n import for test setup.The i18n import ensures translation strings are available during test execution, consistent with the component's use of internationalized text.
136-159: LGTM! Well-structured test for dropdown positioning.The test properly validates the dropdown's position and test ID, using appropriate async patterns and assertions.
apps/frontend/src/main/insights/insights-executor.ts (1)
88-88: LGTM! Correct async/await pattern.The change properly awaits the environment configuration before spawning the Python process, aligning with the async refactor in the config module.
apps/frontend/src/renderer/components/settings/ProfileEditDialog.test.tsx (4)
13-13: LGTM! Required i18n import for component tests.The i18n import ensures translation strings are available, enabling the ProfileEditDialog component to render correctly during tests.
274-290: LGTM! Comprehensive preset application test.The test properly validates that selecting the "GLM (Global)" preset populates both the base URL and name fields with correct values.
292-311: LGTM! Important edge case validation.This test correctly verifies that applying a preset after the user has entered a custom name preserves the user's name while updating the base URL. This is the expected behavior per the component's design.
313-330: LGTM! Good UX validation for focus management.The test properly validates that focus moves to the Base URL input after selecting a preset, ensuring a smooth keyboard navigation experience.
apps/frontend/src/shared/i18n/locales/fr/settings.json (1)
53-174: English locale is complete with all matching translation keys.The English locale file (
apps/frontend/src/shared/i18n/locales/en/settings.json) contains all corresponding translation keys from the French locale additions, including theapiProfilesandmodelSelectsections with GLM presets. No action required.apps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeation.test.ts (2)
1-66: LGTM! Well-structured test setup.The test file setup is comprehensive with properly hoisted mocks for auth state, IPC handlers, and the ideation store. The mock structure ensures correct initialization order and covers all necessary dependencies.
68-172: Comprehensive test coverage for the main flows.The test cases cover the essential scenarios: listener lifecycle, auth-gated generation/refresh/append, and retry flows after environment configuration. The use of
act()is correct, and assertions properly verify both state changes and function invocations.apps/frontend/src/renderer/components/ideation/hooks/__tests__/useIdeationAuth.test.ts (2)
21-57: LGTM! IPC mock setup aligns with test requirements.The
mockGetApiProfilesmock is properly initialized and wired intowindow.electronAPI. The default mock implementation returns a realistic empty-state response, ensuring tests can override it as needed for specific scenarios.
201-255: Excellent test coverage for the IPC fallback logic.These tests properly verify both paths of the
resolveHasAPIProfilehelper:
- IPC fallback when the store's
activeProfileIdis missing- Trusting the store to avoid redundant IPC calls when
activeProfileIdis presentThe assertions correctly validate call counts and resulting
hasTokenvalues.apps/frontend/src/renderer/components/settings/ProfileList.test.tsx (2)
16-16: LGTM! i18n integration follows coding guidelines.The test now uses
i18n.t()with proper namespace prefixes (settings:apiProfiles.dialog.*) for all user-facing strings in the delete confirmation dialog, including dynamic profile name interpolation. This aligns with the coding guideline requiring i18n translation keys instead of hardcoded strings.As per coding guidelines, user-facing text should use i18n.
Also applies to: 208-226
230-303: Consistent i18n usage throughout remaining tests.All remaining test assertions correctly use
i18n.t()for user-facing text, including cancel button, delete actions, and OAuth switch button labels. The test logic remains intact while ensuring all UI strings reference translation keys.apps/frontend/src/renderer/components/ideation/hooks/useIdeationAuth.ts (2)
24-40: LGTM! Helper function eliminates code duplication effectively.The
resolveHasAPIProfilehelper successfully addresses previous review feedback by consolidating the API profile detection logic. The "trust the store" optimization is well-documented in the comment, and the safe error handling (returningfalseon exceptions) ensures robust fallback behavior.
52-55: Consistent and correct helper usage.Both
performCheckandcheckAuthcorrectly invokeresolveHasAPIProfileand wrap the combined authentication check withBoolean()to ensure a consistent boolean type. This eliminates the previous code duplication while maintaining the same logic.Also applies to: 75-78
apps/frontend/src/main/__tests__/insights-config.test.ts (2)
1-48: LGTM! Comprehensive test setup with proper cleanup.The mock setup covers all necessary dependencies (Electron app, rate-limit-detector, profile service, python-env-manager) and the
beforeEach/afterEachhooks ensure proper environment initialization and cleanup between tests. This provides a solid foundation for testingInsightsConfig.getProcessEnv().
50-98: Excellent test coverage addressing previous feedback.The test cases comprehensively cover:
- Composite environment merging from all sources with proper PYTHONPATH composition
- OAuth mode behavior (clearing ANTHROPIC vars when no API profile)
- PYTHONPATH edge cases (python env without entries, missing auto-build path)
These tests address the previous review feedback requesting edge-case coverage and properly validate the async
getProcessEnv()behavior.apps/frontend/src/renderer/components/settings/ProfileList.tsx (1)
10-10: LGTM! Comprehensive i18n implementation.The internationalization implementation is thorough and follows the coding guidelines correctly:
- All user-facing strings now use translation keys with the
settings:apiProfiles.*namespace- Dynamic values (profile names) are properly interpolated using i18n parameters
- The local
isActivevariable (line 185) improves code readability within the map functionBased on coding guidelines, all user-facing text must use i18n translation keys, and this implementation achieves that requirement.
Also applies to: 36-36, 62-330
apps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx (1)
20-20: LGTM! Proper i18n implementation with fallback handling.The internationalization implementation correctly:
- Uses
useTranslationhook to access translation function- Provides fallback placeholder via
resolvedPlaceholder = placeholder ?? t('settings:modelSelect.placeholder')- Replaces all user-facing strings with namespaced translation keys
Based on coding guidelines, this properly implements i18n for all user-facing text.
Also applies to: 67-68, 235-237, 273-273, 284-284, 315-315
apps/frontend/src/renderer/components/settings/ProfileEditDialog.tsx (1)
18-18: LGTM! Excellent i18n implementation with preset integration.The changes implement comprehensive internationalization and add well-designed preset functionality:
- All user-facing strings use
settings:apiProfiles.*translation keys with proper namespace prefixes- Dynamic interpolation correctly passes profile names and other values to translation functions
- The
applyPresetfunction (lines 148-156) properly applies preset baseUrl and auto-fills name only when empty, avoiding overwrites- Focus management with
onCloseAutoFocus(lines 343-346) provides good keyboard UX by focusing the baseUrl input after preset selection- Validation messages, toast notifications, and form labels all use translation keys consistently
Based on coding guidelines, this properly implements i18n for all user-facing text and follows React best practices.
Also applies to: 51-51, 148-156, 164-188, 262-293, 308-315, 447-450, 473-474, 489-546, 564-577
apps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx
Show resolved
Hide resolved
Add GLM presets and improve profile dialog layout. Align ideation auth flow with API profiles. Expand Insights env setup and add tests.
StillKnotKnown
force-pushed
the
local/glm-insights-debug
branch
from
c52ed11 to
16d7818
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
apps/frontend/src/main/ipc-handlers/github/__tests__/runner-env-handlers.test.tsapps/frontend/src/main/ipc-handlers/github/autofix-handlers.tsapps/frontend/src/main/ipc-handlers/github/pr-handlers.tsapps/frontend/src/main/ipc-handlers/github/triage-handlers.tsapps/frontend/src/main/ipc-handlers/github/utils/__tests__/runner-env.test.tsapps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/main/ipc-handlers/github/triage-handlers.tsapps/frontend/src/main/ipc-handlers/github/__tests__/runner-env-handlers.test.tsapps/frontend/src/main/ipc-handlers/github/pr-handlers.tsapps/frontend/src/main/ipc-handlers/github/utils/runner-env.tsapps/frontend/src/main/ipc-handlers/github/autofix-handlers.tsapps/frontend/src/main/ipc-handlers/github/utils/__tests__/runner-env.test.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/main/ipc-handlers/github/triage-handlers.tsapps/frontend/src/main/ipc-handlers/github/__tests__/runner-env-handlers.test.tsapps/frontend/src/main/ipc-handlers/github/pr-handlers.tsapps/frontend/src/main/ipc-handlers/github/utils/runner-env.tsapps/frontend/src/main/ipc-handlers/github/autofix-handlers.tsapps/frontend/src/main/ipc-handlers/github/utils/__tests__/runner-env.test.ts
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/main/ipc-handlers/github/triage-handlers.tsapps/frontend/src/main/ipc-handlers/github/__tests__/runner-env-handlers.test.tsapps/frontend/src/main/ipc-handlers/github/pr-handlers.tsapps/frontend/src/main/ipc-handlers/github/utils/runner-env.tsapps/frontend/src/main/ipc-handlers/github/autofix-handlers.tsapps/frontend/src/main/ipc-handlers/github/utils/__tests__/runner-env.test.ts
🧬 Code graph analysis (6)
apps/frontend/src/main/ipc-handlers/github/triage-handlers.ts (3)
apps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts (1)
getRunnerEnv(4-15)apps/frontend/src/main/ipc-handlers/github/utils/subprocess-runner.ts (1)
runPythonSubprocess(54-193)apps/backend/runners/github/models.py (1)
TriageResult(570-643)
apps/frontend/src/main/ipc-handlers/github/__tests__/runner-env-handlers.test.ts (1)
apps/frontend/src/shared/types/project.ts (1)
Project(5-13)
apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts (1)
apps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts (1)
getRunnerEnv(4-15)
apps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts (1)
apps/frontend/src/main/agent/env-utils.ts (1)
getOAuthModeClearVars(22-39)
apps/frontend/src/main/ipc-handlers/github/autofix-handlers.ts (1)
apps/frontend/src/main/ipc-handlers/github/utils/subprocess-runner.ts (1)
runPythonSubprocess(54-193)
apps/frontend/src/main/ipc-handlers/github/utils/__tests__/runner-env.test.ts (1)
apps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts (1)
getRunnerEnv(4-15)
🔇 Additional comments (9)
apps/frontend/src/main/ipc-handlers/github/triage-handlers.ts (1)
22-22: LGTM! Centralized environment construction improves consistency.The introduction of
getRunnerEnv()to centralize subprocess environment assembly is a solid refactoring. This ensures API profile variables and OAuth clear vars are consistently propagated to Python runners across all GitHub IPC handlers.Also applies to: 258-264
apps/frontend/src/main/ipc-handlers/github/autofix-handlers.ts (3)
281-287: LGTM! Environment construction properly centralized.The
checkNewIssueshandler correctly adopts the centralizedgetRunnerEnv()pattern, ensuring consistent API profile and OAuth environment propagation.
613-621: LGTM! Batch handler environment construction properly centralized.Consistent application of the
getRunnerEnv()pattern ensures the batch auto-fix subprocess receives the correct API profile and OAuth environment.
736-743: LGTM! Analyze-preview handler environment construction properly centralized.The
analyze-previewhandler correctly usesgetRunnerEnv(), completing the consistent environment propagation pattern across all autofix subprocess invocations.apps/frontend/src/main/ipc-handlers/github/utils/__tests__/runner-env.test.ts (1)
1-55: LGTM! Comprehensive test coverage for the centralized environment utility.The test suite properly validates the
getRunnerEnv()utility:
- Verifies API profile environment and OAuth clear vars are merged correctly
- Confirms OAuth clear vars override API profile vars (e.g., clearing ANTHROPIC_AUTH_TOKEN)
- Validates extra environment values are included in the final result
- Uses proper Vitest mocking patterns and assertions
The tests cover the main scenarios and ensure the centralized environment construction behaves as expected.
apps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts (1)
4-15: LGTM! Clean centralization of subprocess environment construction.The
getRunnerEnv()utility provides a well-structured, single-responsibility function that:
- Asynchronously retrieves API profile environment variables
- Applies OAuth mode clear variables when needed
- Allows optional extra environment overrides via the
extraEnvparameter- Uses correct merge precedence (API profile → OAuth clear vars → extra env)
This centralization eliminates duplication and ensures consistent environment propagation across all GitHub Python subprocess invocations (PR review, triage, autofix).
apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts (2)
632-640: LGTM! PR review environment construction properly centralized with project settings.The
runPRReviewfunction correctly adoptsgetRunnerEnv()while preserving project-specific settings. The conditional logic appropriately passesUSE_CLAUDE_MD: 'true'whenproject.settings?.useClaudeMdis not explicitly false, maintaining backward compatibility.
1492-1500: LGTM! Follow-up review environment construction consistent with initial review.The follow-up review handler correctly mirrors the environment construction pattern from
runPRReview, ensuring consistent behavior between initial and follow-up reviews while preserving project-specificuseClaudeMdsettings.apps/frontend/src/main/ipc-handlers/github/__tests__/runner-env-handlers.test.ts (1)
123-219: Well-structured integration tests with good coverage.The test suite effectively verifies that runner environment is correctly propagated to Python subprocesses across all three GitHub operations (PR review, triage, and autofix). The tests follow a consistent pattern and properly validate the integration between IPC handlers and the runner-env utility.
Key verification points:
- PR review correctly passes
USE_CLAUDE_MD: 'true'togetRunnerEnv- All operations receive
ANTHROPIC_AUTH_TOKENin subprocess environment- Mock assertions confirm both
getRunnerEnvinvocation and subprocess environment setup
apps/frontend/src/main/ipc-handlers/github/__tests__/runner-env-handlers.test.ts
Show resolved
Hide resolved
apps/frontend/src/main/ipc-handlers/github/__tests__/runner-env-handlers.test.ts
Outdated
Show resolved
Hide resolved
MikeeBuilds
added
bug
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts:
- Around line 634-636: The conditional checking project.settings?.useClaudeMd
!== false is duplicated; extract it into a small helper (e.g., getClaudeMdEnv)
that returns Record<string,string>|undefined and document the opt-out default,
then replace the inline ternary passed to getRunnerEnv at the current call in
pr-handlers.ts (the const subprocessEnv = await getRunnerEnv(...) expression)
and the other occurrence referenced (the duplicate at lines ~1494-1496) to call
getClaudeMdEnv(project) instead so both sites share the same logic and intent.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/frontend/src/main/insights/config.ts (1)
140-161: Critical: Duplicate/malformed code block will cause syntax errors.The code between lines 140-161 contains what appears to be remnants of an incomplete merge or refactoring. There are two separate return statement blocks and duplicate variable declarations for
pythonEnv:
- Lines 140-142 begin a return statement with
...process.envand...pythonEnv- Lines 143-160 contain another code block with comments, a second
const pythonEnvdeclaration,getAugmentedEnv(), and a second return statementThis will not compile. The first return block (lines 140-142) should be removed, keeping only the properly structured return block that uses
getAugmentedEnv().🔎 Proposed fix
if (!hasAutoBuildSource) { pythonPathParts.push(normalizedAutoBuildSource); } } const combinedPythonPath = pythonPathParts.join(path.delimiter); - return { - ...process.env as Record<string, string>, - ...pythonEnv, // Get Python environment (PYTHONPATH for bundled packages like python-dotenv) const pythonEnv = pythonEnvManager.getPythonEnv(); // Use getAugmentedEnv() to ensure common tool paths (claude, dotnet, etc.) // are available even when app is launched from Finder/Dock const augmentedEnv = getAugmentedEnv(); return { ...augmentedEnv, ...pythonEnv, // Include PYTHONPATH for bundled site-packages ...autoBuildEnv, ...oauthModeClearVars, ...profileEnv, ...apiProfileEnv, PYTHONUNBUFFERED: '1', PYTHONIOENCODING: 'utf-8', PYTHONUTF8: '1', ...(combinedPythonPath ? { PYTHONPATH: combinedPythonPath } : {}) }; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
apps/frontend/src/main/insights/config.tsapps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/main/ipc-handlers/github/pr-handlers.tsapps/frontend/src/shared/i18n/locales/en/settings.jsonapps/frontend/src/shared/i18n/locales/fr/settings.json
🧰 Additional context used
📓 Path-based instructions (4)
apps/frontend/src/shared/i18n/locales/**/*.json
📄 CodeRabbit inference engine (CLAUDE.md)
apps/frontend/src/shared/i18n/locales/**/*.json: Store translation strings in namespace-organized JSON files atapps/frontend/src/shared/i18n/locales/{lang}/*.jsonfor each supported language
When implementing new frontend features, add translation keys to all language files (minimum: en/.json and fr/.json)
Files:
apps/frontend/src/shared/i18n/locales/en/settings.jsonapps/frontend/src/shared/i18n/locales/fr/settings.json
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/main/ipc-handlers/github/pr-handlers.tsapps/frontend/src/main/insights/config.tsapps/frontend/src/main/insights/insights-executor.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/main/ipc-handlers/github/pr-handlers.tsapps/frontend/src/main/insights/config.tsapps/frontend/src/main/insights/insights-executor.ts
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/main/ipc-handlers/github/pr-handlers.tsapps/frontend/src/main/insights/config.tsapps/frontend/src/main/insights/insights-executor.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:45.209Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings that should be localized) can be deferred to future i18n cleanup passes rather than requiring immediate fixes in PRs that don't introduce new i18n violations.
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : When implementing new frontend features, add translation keys to all language files (minimum: en/*.json and fr/*.json)
Applied to files:
apps/frontend/src/shared/i18n/locales/en/settings.jsonapps/frontend/src/shared/i18n/locales/fr/settings.json
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : Store translation strings in namespace-organized JSON files at `apps/frontend/src/shared/i18n/locales/{lang}/*.json` for each supported language
Applied to files:
apps/frontend/src/shared/i18n/locales/en/settings.jsonapps/frontend/src/shared/i18n/locales/fr/settings.json
🧬 Code graph analysis (2)
apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts (1)
apps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts (1)
getRunnerEnv(4-15)
apps/frontend/src/main/insights/config.ts (2)
apps/frontend/src/main/agent/env-utils.ts (1)
getOAuthModeClearVars(22-39)apps/frontend/src/main/python-env-manager.ts (1)
pythonEnvManager(707-707)
🔇 Additional comments (6)
apps/frontend/src/main/insights/config.ts (1)
110-114: Async refactoring and OAuth mode handling look correct.The conversion to async is appropriate since
getAPIProfileEnv()is asynchronous. The OAuth mode clear vars are correctly derived after awaiting the API profile environment, ensuring proper sequencing.apps/frontend/src/main/insights/insights-executor.ts (1)
88-88: LGTM!The await is correctly added to consume the now-async
getProcessEnv(). Sinceexecute()was already async, this integrates cleanly without requiring further signature changes.apps/frontend/src/shared/i18n/locales/fr/settings.json (1)
53-174: LGTM! Comprehensive French localization for API profiles and model selection.The additions properly localize all UI strings for the new API profiles feature and model selection functionality. The translations are professionally done and include:
- Complete French translations for the
apiProfilessection covering presets (including new GLM Global and China variants), fields, validation, actions, connection testing, model mappings, empty states, OAuth switching, badges, tooltips, toasts, and dialogs- Model selection UI strings in the
modelSelectsectionThe structure is well-organized and all nested keys are properly defined.
As per coding guidelines, translation keys have been added to both en/.json and fr/.json files as required.
apps/frontend/src/shared/i18n/locales/en/settings.json (1)
53-174: LGTM! Complete English localization matching the French translations.The additions provide comprehensive English UI strings for the API profiles and model selection features. The implementation:
- Defines all required keys in the
apiProfilessection with clear, user-friendly English text- Includes the new GLM (Global) and GLM (China) provider presets
- Provides all necessary strings for the
modelSelectcomponent- Maintains structural parity with the French locale file
The translations are clear, professional, and appropriately match the corresponding French keys.
As per coding guidelines, translation keys have been correctly added to both language files (en/.json and fr/.json).
apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts (2)
23-23: LGTM - Clean import for centralized environment handling.The import of
getRunnerEnvsupports the refactoring that centralizes subprocess environment construction across PR review flows.
1494-1496: Consistent with PR review flow.The environment setup for follow-up reviews correctly mirrors the PR review logic, ensuring consistent behavior across both review types.
|
Lint fix pushed: repaired malformed env return in insights config. Local eslint passes with "npm run lint -- --quiet". |
|
Tests run locally:\n- npm run lint (0 errors, 346 warnings)\n- npm run typecheck\n- npm test\n\nAdded Sentry module mocks for Vitest + type stubs to unblock renderer tests. |
|
|
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @apps/frontend/src/__mocks__/sentry-electron-main.ts:
- Around line 1-26: Mocks for main and renderer are identical; extract shared
types and functions (ErrorEvent, Scope, InitOptions, init, captureException,
withScope) into a new module (e.g., sentry-electron-shared) and replace the
contents of sentry-electron-main.ts (and similarly sentry-electron-renderer.ts
if desired) with a single re-export like "export * from
'./sentry-electron-shared'"; ensure the shared module exports the same types and
stub implementations (setContext noop, init/captureException/withScope
signatures) so existing tests keep working.
In @apps/frontend/src/types/sentry-electron.d.ts:
- Around line 1-49: The two identical module declarations for
'@sentry/electron/main' and '@sentry/electron/renderer' should reuse shared type
definitions: extract ErrorEvent, Scope, and InitOptions into single top-level
interfaces (e.g., SentryErrorEvent, SentryScope, SentryInitOptions) and then
reference those types inside each module block instead of redefining them; keep
the exported functions init, captureException, and withScope signatures but
change their parameter/return types to use the new shared interfaces (e.g.,
init(options: SentryInitOptions), withScope(callback: (scope: SentryScope) =>
void)), ensuring names like ErrorEvent, Scope, and InitOptions are removed from
the duplicated blocks and replaced with the shared types.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
apps/frontend/src/__mocks__/sentry-electron-main.tsapps/frontend/src/__mocks__/sentry-electron-renderer.tsapps/frontend/src/main/insights/config.tsapps/frontend/src/main/ipc-handlers/github/__tests__/runner-env-handlers.test.tsapps/frontend/src/shared/i18n/locales/en/settings.jsonapps/frontend/src/shared/i18n/locales/fr/settings.jsonapps/frontend/src/types/sentry-electron.d.tsapps/frontend/vitest.config.ts
🧰 Additional context used
📓 Path-based instructions (4)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/main/insights/config.tsapps/frontend/src/main/ipc-handlers/github/__tests__/runner-env-handlers.test.tsapps/frontend/src/types/sentry-electron.d.tsapps/frontend/src/__mocks__/sentry-electron-renderer.tsapps/frontend/src/__mocks__/sentry-electron-main.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/main/insights/config.tsapps/frontend/src/main/ipc-handlers/github/__tests__/runner-env-handlers.test.tsapps/frontend/src/types/sentry-electron.d.tsapps/frontend/src/__mocks__/sentry-electron-renderer.tsapps/frontend/src/__mocks__/sentry-electron-main.ts
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/main/insights/config.tsapps/frontend/vitest.config.tsapps/frontend/src/main/ipc-handlers/github/__tests__/runner-env-handlers.test.tsapps/frontend/src/types/sentry-electron.d.tsapps/frontend/src/__mocks__/sentry-electron-renderer.tsapps/frontend/src/__mocks__/sentry-electron-main.ts
apps/frontend/src/shared/i18n/locales/**/*.json
📄 CodeRabbit inference engine (CLAUDE.md)
apps/frontend/src/shared/i18n/locales/**/*.json: Store translation strings in namespace-organized JSON files atapps/frontend/src/shared/i18n/locales/{lang}/*.jsonfor each supported language
When implementing new frontend features, add translation keys to all language files (minimum: en/.json and fr/.json)
Files:
apps/frontend/src/shared/i18n/locales/fr/settings.jsonapps/frontend/src/shared/i18n/locales/en/settings.json
🧠 Learnings (4)
📓 Common learnings
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:45.209Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings that should be localized) can be deferred to future i18n cleanup passes rather than requiring immediate fixes in PRs that don't introduce new i18n violations.
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/backend/.env* : Enable Electron MCP for E2E testing by setting `ELECTRON_MCP_ENABLED=true` in `.env` and starting the Electron app with `npm run dev`
Applied to files:
apps/frontend/vitest.config.ts
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : When implementing new frontend features, add translation keys to all language files (minimum: en/*.json and fr/*.json)
Applied to files:
apps/frontend/src/shared/i18n/locales/fr/settings.jsonapps/frontend/src/shared/i18n/locales/en/settings.json
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : Store translation strings in namespace-organized JSON files at `apps/frontend/src/shared/i18n/locales/{lang}/*.json` for each supported language
Applied to files:
apps/frontend/src/shared/i18n/locales/fr/settings.jsonapps/frontend/src/shared/i18n/locales/en/settings.json
🧬 Code graph analysis (3)
apps/frontend/src/main/insights/config.ts (3)
apps/frontend/src/main/agent/env-utils.ts (1)
getOAuthModeClearVars(22-39)apps/frontend/src/main/python-env-manager.ts (1)
pythonEnvManager(707-707)apps/frontend/src/main/env-utils.ts (1)
getAugmentedEnv(98-147)
apps/frontend/src/main/ipc-handlers/github/__tests__/runner-env-handlers.test.ts (1)
apps/frontend/src/shared/types/project.ts (1)
Project(5-13)
apps/frontend/src/types/sentry-electron.d.ts (2)
apps/frontend/src/__mocks__/sentry-electron-main.ts (6)
ErrorEvent(1-1)Scope(3-5)InitOptions(7-16)init(18-18)captureException(20-20)withScope(22-26)apps/frontend/src/__mocks__/sentry-electron-renderer.ts (6)
ErrorEvent(1-1)Scope(3-5)InitOptions(7-16)init(18-18)captureException(20-20)withScope(22-26)
🔇 Additional comments (14)
apps/frontend/src/__mocks__/sentry-electron-renderer.ts (1)
1-26: LGTM! Well-structured mock for testing.The no-op implementations are appropriate for isolating Sentry from unit tests. The
withScopecallback pattern correctly mimics the real API behavior by immediately invoking the callback with a mock scope.apps/frontend/vitest.config.ts (1)
17-21: LGTM! Alias configuration correctly wires up the Sentry mocks.The aliases follow the existing pattern and correctly resolve to the mock implementations, ensuring tests import mocked Sentry modules without side effects.
apps/frontend/src/shared/i18n/locales/en/settings.json (1)
53-174: LGTM! Comprehensive localization for new API profile features.The new
apiProfilesandmodelSelectsections provide complete UI strings for the API profile management feature, including the GLM presets mentioned in the PR objectives. The structure is well-organized with proper hierarchical keys covering all user flows (CRUD, validation, testing, OAuth switching, toast notifications). Based on coding guidelines, verifying that French locale has matching keys.apps/frontend/src/shared/i18n/locales/fr/settings.json (1)
53-174: LGTM! Complete French localization matches English structure.The French locale file has identical key structure to the English version with appropriate translations for all new
apiProfilesandmodelSelectsections. All ~90+ translation strings are present, including GLM presets, validation messages, toast notifications, and accessibility labels. This follows the coding guidelines requirement to add translation keys to all language files (minimum: en/.json and fr/.json).Based on coding guidelines, which require translation keys in both English and French locale files for new frontend features.
apps/frontend/src/main/ipc-handlers/github/__tests__/runner-env-handlers.test.ts (5)
1-9: LGTM!Imports are appropriate and well-organized. Type-only imports are correctly used for types that shouldn't affect the runtime bundle.
11-47: LGTM!The
HoistedMockIpcMainclass provides a clean abstraction for testing IPC communication patterns. The hoisted mock ensures it's available before ES module mocking takes effect.
119-143: Good implementation of temp directory cleanup.The
createProjecthelper now properly tracks temporary directories in thetempDirsarray, and theafterEachhook ensures cleanup. This addresses the resource leak concern from the previous review.Also applies to: 154-163
165-196: LGTM!The PR review test correctly verifies that:
getRunnerEnvis called with{ USE_CLAUDE_MD: 'true' }matching the project settings- The subprocess receives the environment with
ANTHROPIC_AUTH_TOKENThe dynamic import pattern ensures mocks are established before handler registration.
198-221: Confirm that the difference ingetRunnerEnvcall signatures is intentional.The test assertions are accurate—PR handlers call
getRunnerEnv(getClaudeMdEnv(project))to pass theUSE_CLAUDE_MDsetting from project preferences, while triage and autofix handlers callgetRunnerEnv()without arguments, ignoring project settings. This difference is intentional:getClaudeMdEnvis only defined in pr-handlers.ts and not exported. However, ensure this design decision (Claude MD support only for PR reviews, not for triage/autofix operations) is documented or confirmed in the PR description.apps/frontend/src/main/insights/config.ts (5)
5-7: LGTM!The new imports for
getAPIProfileEnvandgetOAuthModeClearVarsalign with the environment augmentation logic added togetProcessEnv().
110-114: LGTM!The async signature change is appropriate given
getAPIProfileEnv()is async. The ordering correctly retrieves the API profile environment first, then derives OAuth mode clear vars from it.
117-138: PYTHONPATH deduplication is well implemented.The composition logic correctly:
- Normalizes paths using
path.resolve()- Handles Windows case-insensitivity with
.toLowerCase()- Filters empty entries
- Prevents duplicate
autoBuildSourceentriesThis properly addresses the previous review feedback.
144-155: Environment composition order is correct.The spread ordering ensures proper precedence:
oauthModeClearVarsclears ANTHROPIC_* vars only in OAuth mode (returns{}when API profile is active)apiProfileEnvfollows, so API profile vars take effect when configured- Final
PYTHONPATHspread correctly overwrites the original frompythonEnvThe conditional
combinedPythonPath ? {...} : {}at line 154 correctly avoids setting an empty PYTHONPATH.
113-113: No action needed.getAPIProfileEnv()has complete error handling through its dependency chain.loadProfilesFile()wraps all I/O and parsing operations in a try-catch that returns a safe default, ensuring no unhandled exceptions propagate to the caller.
|
Sentry mock/type refactor pushed: shared mock module + shared type definitions in d.ts.\n\nTests run locally:\n- npm run lint (0 errors, 346 warnings)\n- npm run typecheck\n- npm test |
) * fix: improve api profile presets and ideation auth Add GLM presets and improve profile dialog layout. Align ideation auth flow with API profiles. Expand Insights env setup and add tests. * github: pass api profile env to python runners * test: clean up github runner env temp dirs * refactor: centralize Claude.md env helper * fix: repair insights env return * test: fix typecheck and vitest sentry mocks * refactor: share sentry mocks and types --------- Co-authored-by: StillKnotKnown <[email protected]> Co-authored-by: Andy <[email protected]>
) * fix: improve api profile presets and ideation auth Add GLM presets and improve profile dialog layout. Align ideation auth flow with API profiles. Expand Insights env setup and add tests. * github: pass api profile env to python runners * test: clean up github runner env temp dirs * refactor: centralize Claude.md env helper * fix: repair insights env return * test: fix typecheck and vitest sentry mocks * refactor: share sentry mocks and types --------- Co-authored-by: StillKnotKnown <[email protected]> Co-authored-by: Andy <[email protected]>
4 participants
Summary
Testing
Screenshots
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.