Feat/rtl i18n improvements#2022
Open
mohmedofficial515 wants to merge 14 commits into
Open
Conversation
…me, and merge progress - Created `taskReview.json` for task review translations in Arabic. - Created `tasks.json` for task management translations in Arabic. - Created `terminal.json` for terminal interface translations in Arabic. - Created `welcome.json` for welcome screen translations in Arabic.
…ng defaults for cost efficiency
…icons for RTL Arabic (RTL) was already wired up (document.dir='rtl') but the layout never mirrored because the renderer used physical Tailwind utilities everywhere. - Codemod across 175 .tsx files: ml/mr→ms/me, pl/pr→ps/pe, text-left/right→ text-start/end, rounded-l/r/tl/tr/bl/br→logical, border-l/r→border-s/e, absolute left-/right-→start-/end- (698 replacements). Radix side-animation utilities (slide-in-from-left/right tied to data-[side]) preserved. - Flip 26 horizontal directional icons (Chevron/Arrow/Panel) with rtl:-scale-x-100; rotating expand/collapse chevrons left untouched. typecheck passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Icon-only Buttons/triggers lacked an accessible name for screen readers. Reuse existing common:accessibility.* keys where they fit; add 18 new keys (close/edit/remove/restore/discard/settings/newTask/toggleSidebar/sendMessage/ attachScreenshot/removeImage/queueAll/queueSettings/openInIde/runClaude/ resumeAllSessions/showPassword/hidePassword) with fluent en/fr/ar translations. Locale parity verified (62 accessibility keys in each of en/fr/ar). typecheck passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The entire changelog feature (11 components) was 100% hardcoded English with
zero useTranslation usage — Arabic/French users saw English throughout.
- Register new `changelog` i18n namespace (en/fr/ar) with 153 keys covering
every user-facing string: headers, filters, list/empty states, config,
preview, success, GitHub release, archive, actions — plus the enum-keyed
source-mode/format/audience/emoji-level/stage labels & descriptions that were
rendered straight from constants.
- Replace hardcoded MCP tool tooltips ("Test Connection"/"Edit"/"Delete") and
the "Test" button label in AgentTools with settings:/common: keys; add
settings:mcp.testConnectionTitle (en/fr/ar).
Locale parity verified (changelog: 153 keys each lang, 0 missing/extra).
typecheck passes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… FirstSpecStep These onboarding-flow components were hardcoded English (first-run experience shown to every new user). Extract ~73 strings into the onboarding namespace under new graphiti/authChoice/firstSpec sections; refactor GraphitiStep's module-level LLM_PROVIDERS/EMBEDDING_PROVIDERS arrays to carry i18n key references instead of literal name/description. onboarding locale parity: 309 keys each in en/fr/ar (0 missing/extra). typecheck passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…omponents
These settings components were hardcoded English. Extract ~109 strings into the
settings namespace: project-settings (Security/Memory, Integration, Linear &
GitHub sections, Notifications), settings/integrations/LinearIntegration, and
ThemeSelector. New keys grouped under integrations.{common,connection,linear,
github,memory} and theme. GitHub project-scope description uses <Trans> to
preserve the inline styled {{projectName}} span.
settings locale parity: 1009 keys each in en/fr/ar (0 missing/extra).
typecheck passes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
AuthChoiceStep now renders copy via t('onboarding:authChoice.*'); mock
useTranslation to resolve those keys to the English strings the existing
assertions expect (mirrors OnboardingWizard.test.tsx). 14/14 pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…AI points Add a 'direct' Vercel AI SDK provider that wraps the free, no-API-key web transports from ai-providers-direct (DeepSeek primary, ChatGPT assist), and route every AI feature through it when enabled. - providers/direct: LanguageModelV2 wrapper with prompt flattening, plain-text ReAct tool-call emulation (the web protocols have no native function calling), and streaming. Library bridged lazily so WASM/Playwright never bundle into the pure core. 13 unit tests. - factory/registry/types: register SupportedProvider.Direct + model-id prefixes (deepseek, chatgpt) for detection and registry resolution. - client/factory: resolveDirectModel() override at the single chokepoint both client factories share — covers planner, coder, QA, and every utility runner. Safe fallback to the configured provider when no DeepSeek token is present. - settings: restore DirectAiConnectionSettings type + DirectAiSettings UI panel, default enabled (transparently falls back without a token), i18n for en/fr/ar. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…sing - getDirectConnectionSettings now merges over DEFAULT_APP_SETTINGS so the shipped default applies even when the saved settings file predates the field (the accessor reads the raw file without merging defaults). - resolveDirectModel distinguishes enabled-but-tokenless (needs-setup) from disabled; when direct mode is on without a token and the paid fallback also fails, surface an actionable setup message instead of the cryptic priority-queue error. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a one-click "Capture token automatically" flow so users don't have to run the CLI extractor and paste the token by hand. - capture-token.ts: runs the bundled ai-providers-direct Playwright extractor via Electron-as-node. Headless first (existing session); if not signed in, opens a headed sign-in window then captures headlessly. Profile + .env land in userData, never the cwd. - direct-ai-handlers.ts: 'direct-ai-capture-token' IPC handler, registered in the IPC index alongside the other handlers. - preload + ipc types + browser mock: expose window.electronAPI.captureDeepSeekToken. - DirectAiSettings: capture button with loading/error states; fills the token field on success. i18n for en/fr/ar. Caveat: requires playwright + a Chrome/Chromium binary at runtime (available in dev; not bundled in packaged builds). Failures surface a structured error. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Node 24 defaults DNS to "verbatim" (often IPv6 first). chat.deepseek.com (and
api.github.com) have AAAA records whose IPv6 route hangs here, so undici `fetch`
hit its 10s connect timeout (UND_ERR_CONNECT_TIMEOUT) — breaking the Direct AI
(DeepSeek) connection and the GitHub app-updater.
Diagnosis: raw TCP + curl + `node` with family=4 all connect in <0.5s, but
default `fetch` failed at ~10.6s (3/3). `dns.setDefaultResultOrder('ipv4first')`
makes the same fetch succeed in <200ms.
Apply the IPv4-first default at the top of the main process and in the agent
worker thread (separate DNS default), so both the runner path and the build
pipeline reach DeepSeek.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…d screenshot support
Contributor
|
Important Review skippedToo many files! This PR contains 253 files, which is 103 over the limit of 150. To get a review, narrow the scope: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (253)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🎉 Thanks for your first PR!
A maintainer will review it soon. Please make sure:
- Your branch is synced with
develop - CI checks pass
- You've followed our contribution guide
Welcome to the Auto Claude community!
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
+135
to
+138
| if (isNotLoggedIn(result)) { | ||
| await runExtractor(scriptPath, true, consumerRoot); | ||
| result = await runExtractor(scriptPath, false, consumerRoot); | ||
| } |
There was a problem hiding this comment.
Bug: The result of the headed runExtractor call is discarded, so if it times out or fails, the user sees a generic "not logged in" error instead of the real one.
Severity: MEDIUM
Suggested Fix
Capture the result of the headed runExtractor call. Check if it was successful. If it failed, return the failure result immediately. Do not proceed with the final headless runExtractor call if the headed sign-in step failed.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: apps/desktop/src/main/ai/providers/direct/capture-token.ts#L135-L138
Potential issue: In `captureDeepSeekToken`, the result of the headed `runExtractor` call
is not assigned to any variable. If this operation fails, for instance due to a timeout
after 240 seconds, the error information is lost. The function then proceeds to another
headless `runExtractor` call which returns a `NOT_LOGGED_IN` error. This masks the
original timeout error, leading to a confusing user experience where the user waits a
long time only to see a misleading error message.
Did we get this right? 👍 / 👎 to inform future reviews.
| @@ -70,7 +70,23 @@ export const DEFAULT_APP_SETTINGS = { | |||
| // GPU acceleration for terminal rendering | |||
| // Default to 'off' until WebGL stability is proven across all GPU drivers. | |||
There was a problem hiding this comment.
Bug: New users without a configured provider will see a confusing error about 'Direct AI Connection' when trying to use AI features, because this connection type is enabled by default.
Severity: LOW
Suggested Fix
To improve the user experience, consider disabling directAiConnection by default. Alternatively, improve the error handling to present a more general and helpful message, such as guiding the user to the settings page to configure any available AI provider, rather than a specific one.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: apps/desktop/src/shared/constants/config.ts#L71
Potential issue: The `directAiConnection` feature is enabled by default. For a new user
without a pre-configured DeepSeek token or a fallback paid provider, attempting to use
an AI feature will throw an error. The error message, `DIRECT_NEEDS_TOKEN_MESSAGE`, is
confusing as it refers to a feature the user did not explicitly enable, creating a poor
onboarding experience.
Did we get this right? 👍 / 👎 to inform future reviews.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a free 'Direct AI Connection' feature to route agent tasks directly through DeepSeek and ChatGPT web protocols, adds Arabic language support with RTL layouts, implements Anthropic prompt caching, and fixes Windows CLI path quoting. Feedback from the review highlights critical compatibility issues with the Vercel AI SDK LanguageModelV2 interface in the prompt flattening and model implementation, such as using incorrect properties (input instead of args, and output instead of result) which will cause runtime crashes. Additionally, the reviewer noted that spinning up a new browser session on every generation call is highly inefficient, warned against using fragile relative file dependencies in package.json, recommended safer RTL detection and locale-aware date formatting, and requested the removal of a temporary stack dump file.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+86
to
+92
| } else if (part.type === 'tool-call') { | ||
| // Echo the assistant's prior tool call so the model sees the loop it | ||
| // is already in. Mirrors the ReAct block the parser emits/expects. | ||
| segments.push( | ||
| `<tool_call>\n${JSON.stringify({ name: part.toolName, arguments: part.input })}\n</tool_call>`, | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
In the Vercel AI SDK LanguageModelV2 prompt, the content part for a tool-call message uses the args property to store the arguments, not input. Using part.input will result in undefined, causing the tool arguments to be completely omitted when echoing the assistant's prior turns. This will break multi-turn tool use as the model won't see its previous arguments.
Suggested change
| } else if (part.type === 'tool-call') { | |
| // Echo the assistant's prior tool call so the model sees the loop it | |
| // is already in. Mirrors the ReAct block the parser emits/expects. | |
| segments.push( | |
| `<tool_call>\n${JSON.stringify({ name: part.toolName, arguments: part.input })}\n</tool_call>`, | |
| ); | |
| } | |
| } else if (part.type === 'tool-call') { | |
| // Echo the assistant's prior tool call so the model sees the loop it | |
| // is already in. Mirrors the ReAct block the parser emits/expects. | |
| segments.push( | |
| `<tool_call>\\n${JSON.stringify({ name: part.toolName, arguments: part.args })}\\n</tool_call>`, | |
| ); | |
| } |
Comment on lines
+32
to
+52
| function renderToolOutput(output: { type: string; value?: unknown }): string { | ||
| switch (output.type) { | ||
| case 'text': | ||
| case 'error-text': | ||
| return String(output.value ?? ''); | ||
| case 'json': | ||
| case 'error-json': | ||
| return JSON.stringify(output.value ?? null); | ||
| case 'content': { | ||
| // Array of text/media parts — keep text, mark media placeholders. | ||
| const parts = Array.isArray(output.value) ? output.value : []; | ||
| return parts | ||
| .map((p: { type: string; text?: string; mediaType?: string }) => | ||
| p.type === 'text' ? (p.text ?? '') : `[media: ${p.mediaType ?? 'unknown'}]`, | ||
| ) | ||
| .join('\n'); | ||
| } | ||
| default: | ||
| return JSON.stringify(output.value ?? null); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
The renderToolOutput function expects a structured { type, value } object, but the Vercel AI SDK passes the raw tool execution result directly as part.result (which can be a string, object, array, etc.). We should redefine this function to accept unknown and format it robustly to prevent runtime crashes.
function renderToolOutput(result: unknown): string {
if (result == null) return '';
if (typeof result === 'string') return result;
if (typeof result === 'object') {
try {
return JSON.stringify(result);
} catch {
return String(result);
}
}
return String(result);
}
Comment on lines
+102
to
+106
| for (const part of message.content) { | ||
| lines.push( | ||
| `Tool result (tool="${part.toolName}", id="${part.toolCallId}"):\n${renderToolOutput(part.output)}`, | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
In the Vercel AI SDK LanguageModelV2 prompt, the content part for a tool message contains result: unknown, NOT output. At runtime, part.output will be undefined, causing renderToolOutput(undefined) to throw a TypeError and crash the agent loop. Update this to use part.result.
Suggested change
| for (const part of message.content) { | |
| lines.push( | |
| `Tool result (tool="${part.toolName}", id="${part.toolCallId}"):\n${renderToolOutput(part.output)}`, | |
| ); | |
| } | |
| for (const part of message.content) { | |
| lines.push( | |
| `Tool result (tool="${part.toolName}", id="${part.toolCallId}"):\\n${renderToolOutput(part.result)}` | |
| ); | |
| } |
Comment on lines
+130
to
+138
| function toToolCallContent(calls: ParsedToolCall[]): LanguageModelV2Content[] { | ||
| return calls.map((call) => ({ | ||
| type: 'tool-call' as const, | ||
| toolCallId: crypto.randomUUID(), | ||
| toolName: call.toolName, | ||
| // AI SDK expects the arguments as a stringified JSON object. | ||
| input: JSON.stringify(call.args ?? {}), | ||
| })); | ||
| } |
Contributor
There was a problem hiding this comment.
In the Vercel AI SDK LanguageModelV2 output, the property for tool call arguments is args (which must be a parsed object), NOT input (which is a stringified JSON). Returning input as a stringified JSON will cause the AI SDK to fail to parse the tool call arguments or throw an error.
function toToolCallContent(calls: ParsedToolCall[]): LanguageModelV2Content[] {
return calls.map((call) => ({
type: 'tool-call' as const,
toolCallId: crypto.randomUUID(),
toolName: call.toolName,
args: call.args ?? {},
}));
}| "@xterm/addon-webgl": "^0.19.0", | ||
| "@xterm/xterm": "^6.0.0", | ||
| "ai": "^6.0.116", | ||
| "ai-providers-direct": "file:../../../ai-providers-direct", |
Contributor
There was a problem hiding this comment.
Using a relative file: dependency pointing outside the repository root (file:../../../ai-providers-direct) is highly fragile. It will break CI/CD pipelines, automated testing, and production packaging on other machines where this relative path does not exist. Consider publishing this package to a private registry, bundling it, or using a monorepo workspace protocol (e.g., workspace:* if using pnpm/yarn/npm workspaces) to reference it.
Comment on lines
188
to
190
| if (llmProvider === 'openai' || embeddingProvider === 'openai') { | ||
| if (!config.openaiApiKey.trim()) return 'OpenAI API key'; | ||
| if (!config.openaiApiKey.trim()) return t('graphiti.requiredKeys.openai'); | ||
| } |
Contributor
There was a problem hiding this comment.
Calling .trim() directly on config properties (like config.openaiApiKey.trim()) will throw a TypeError and crash the onboarding wizard if those properties are undefined (e.g., if they are not initialized in the state or are missing from a legacy settings file). Use optional chaining to safely handle potentially undefined values.
Suggested change
| if (llmProvider === 'openai' || embeddingProvider === 'openai') { | |
| if (!config.openaiApiKey.trim()) return 'OpenAI API key'; | |
| if (!config.openaiApiKey.trim()) return t('graphiti.requiredKeys.openai'); | |
| } | |
| if (llmProvider === 'openai' || embeddingProvider === 'openai') { | |
| if (!config.openaiApiKey?.trim()) return t('graphiti.requiredKeys.openai'); | |
| } |
Comment on lines
14
to
+15
| export function TaskCard({ task, isSelected, onToggle }: TaskCardProps) { | ||
| const { t } = useTranslation('changelog'); |
Contributor
There was a problem hiding this comment.
toLocaleDateString() formats the date using the system's default locale, which might not match the user's selected language in the app. To ensure a consistent internationalized experience, pass the current language from i18n to toLocaleDateString().
const { t, i18n } = useTranslation('changelog');
const completedDate = new Date(task.completedAt).toLocaleDateString(i18n.language);
Comment on lines
59
to
60
|
|
||
| export function CommitCard({ commit }: CommitCardProps) { |
Contributor
There was a problem hiding this comment.
toLocaleDateString() formats the date using the system's default locale, which might not match the user's selected language in the app. To ensure a consistent internationalized experience, pass the current language from i18n to toLocaleDateString().
const { t, i18n } = useTranslation('changelog');
const commitDate = new Date(commit.date).toLocaleDateString(i18n.language);
Comment on lines
+322
to
+326
| useEffect(() => { | ||
| const isRtl = i18n.language === 'ar'; | ||
| document.documentElement.dir = isRtl ? 'rtl' : 'ltr'; | ||
| document.documentElement.lang = i18n.language; | ||
| }, [i18n.language]); |
Contributor
There was a problem hiding this comment.
Checking i18n.language === 'ar' might fail if the language is set to a regional variant of Arabic (e.g., ar-EG, ar-SA). It is safer to use startsWith('ar') or import the RTL_LANGUAGES constant from src/shared/constants/i18n.ts to determine if the language is RTL.
Suggested change
| useEffect(() => { | |
| const isRtl = i18n.language === 'ar'; | |
| document.documentElement.dir = isRtl ? 'rtl' : 'ltr'; | |
| document.documentElement.lang = i18n.language; | |
| }, [i18n.language]); | |
| useEffect(() => { | |
| const isRtl = i18n.language.startsWith('ar'); | |
| document.documentElement.dir = isRtl ? 'rtl' : 'ltr'; | |
| document.documentElement.lang = i18n.language; | |
| }, [i18n.language]); |
| @@ -0,0 +1,28 @@ | |||
| Stack trace: | |||
Contributor
There was a problem hiding this comment.
The bash.exe.stackdump file appears to be a temporary crash dump file generated during local development. It should not be committed to the repository. Please remove this file and ensure it is added to your .gitignore to prevent future accidental commits.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Base Branch
developbranch (required for all feature/fix PRs)main(hotfix only - maintainers)Description
Related Issue
Closes #
Type of Change
Area
Commit Message Format
Follow conventional commits:
<type>: <subject>Types: feat, fix, docs, style, refactor, test, chore
Example:
feat: add user authentication systemAI Disclosure
Tool(s) used:
Testing level:
Untested -- AI output not yet verified
Lightly tested -- ran the app / spot-checked key paths
Fully tested -- all tests pass, manually verified behavior
I understand what this PR does and how the underlying code works
Checklist
developbranchPlatform Testing Checklist
CRITICAL: This project supports Windows, macOS, and Linux. Platform-specific bugs are a common source of breakage.
platform/module instead of directprocess.platformchecksfindExecutable()or platform abstractions)If you only have access to one OS: CI now tests on all platforms. Ensure all checks pass before submitting.
CI/Testing Requirements
Screenshots
Feature Toggle
use_feature_nameBreaking Changes
Breaking: Yes / No
Details: