-
Notifications
You must be signed in to change notification settings - Fork 614
feat: remove python code run on js #617
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis update removes all support for local Python code execution using Pyodide, including dependency management and code runner logic. It eliminates related dependencies and configuration, and cleans up the codebase by removing schemas, handlers, and tool registrations for Python execution. Additional changes are limited to code formatting and style improvements across several files. Changes
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/main/presenter/mcpPresenter/inMemoryServers/appleServer.ts (2)
760-765:this.CONFIGin static context trips lint – use the class nameStatic methods rely on
this.CONFIG, triggeringnoThisInStaticerrors.
Swap to an explicit reference so it’s unambiguous and linter-clean:- maxEventsPerCalendar: this.CONFIG.MAX_EVENTS_PER_CALENDAR + maxEventsPerCalendar: CalendarUtils.CONFIG.MAX_EVENTS_PER_CALENDARApply the same change in
getEventsandcreateEvent.
604-606: Downgrade expected flow logs fromerrortoinfo/debug
console.error('searchEvents - Processing calendars …')is not an error.
Use an appropriate level to avoid log fatigue; reserveerrorfor exceptional paths.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
electron.vite.config.ts(2 hunks)package.json(0 hunks)src/main/presenter/configPresenter/mcpConfHelper.ts(2 hunks)src/main/presenter/mcpPresenter/inMemoryServers/appleServer.ts(46 hunks)src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts(0 hunks)src/main/presenter/mcpPresenter/pythonRunner/index.ts(0 hunks)src/main/presenter/mcpPresenter/pythonRunner/prepareEnv.ts(0 hunks)src/main/presenter/upgradePresenter/index.ts(1 hunks)src/renderer/src/components/message/MessageItemUser.vue(2 hunks)src/renderer/src/stores/upgrade.ts(1 hunks)
💤 Files with no reviewable changes (4)
- package.json
- src/main/presenter/mcpPresenter/pythonRunner/prepareEnv.ts
- src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
- src/main/presenter/mcpPresenter/pythonRunner/index.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/development-setup.mdc
src/main/presenter/**/*.ts
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/electron-best-practices.mdc
{src/main/presenter/**/*.ts,src/renderer/stores/**/*.ts}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/electron-best-practices.mdc
**/*.{ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/error-logging.mdc
src/main/**
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/project-structure.mdc
src/renderer/src/**/*
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/i18n.mdc
src/renderer/**
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/project-structure.mdc
src/renderer/src/stores/**/*.{vue,ts,tsx,js,jsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/pinia-best-practices.mdc
🧠 Learnings (6)
src/main/presenter/upgradePresenter/index.ts (3)
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-06-30T12:24:03.565Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Provider files should include provider-specific helper functions such as `formatMessages`, `convertToProviderTools`, `parseFunctionCalls`, and `prepareFunctionCallPrompt` as needed.
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/electron-best-practices.mdc:0-0
Timestamp: 2025-06-30T12:23:13.338Z
Learning: Applies to src/renderer/src/composables/usePresenter.ts : The IPC in the renderer process is implemented in usePresenter.ts, allowing direct calls to the presenter-related interfaces exposed by the main process
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/electron-best-practices.mdc:0-0
Timestamp: 2025-06-30T12:23:13.338Z
Learning: Applies to src/main/presenter/**/*.ts : Use context isolation for improved security
src/renderer/src/components/message/MessageItemUser.vue (5)
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-06-23T13:06:15.335Z
Learning: Use arrow functions for methods and computed properties to ensure concise and consistent syntax.
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/i18n.mdc:0-0
Timestamp: 2025-06-30T12:23:45.479Z
Learning: Applies to src/renderer/src/**/* : Do not hardcode user-facing text in code; always use the translation system (vue-i18n) for all user-visible strings
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/i18n.mdc:0-0
Timestamp: 2025-06-30T12:23:45.479Z
Learning: Applies to src/renderer/src/**/* : All user-facing strings in the renderer must use i18n keys (do not hardcode user-visible text in code; use vue-i18n translation keys instead)
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/pinia-best-practices.mdc:0-0
Timestamp: 2025-06-30T12:24:10.749Z
Learning: Applies to src/renderer/src/stores/**/*.{vue,ts,tsx,js,jsx} : Utilize actions for side effects and asynchronous operations
Learnt from: neoragex2002
PR: ThinkInAIXYZ/deepchat#550
File: src/renderer/src/stores/chat.ts:1011-1035
Timestamp: 2025-06-21T15:49:17.044Z
Learning: In src/renderer/src/stores/chat.ts, the user prefers to keep both `text` and `content` properties in the `handleMeetingInstruction` function's `sendMessage` call, even though they are redundant, rather than removing the `content` property.
electron.vite.config.ts (1)
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/development-setup.mdc:0-0
Timestamp: 2025-06-30T12:23:01.752Z
Learning: Node.js >= 22
src/renderer/src/stores/upgrade.ts (1)
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/error-logging.mdc:0-0
Timestamp: 2025-06-30T12:23:33.814Z
Learning: Applies to **/*.{ts,tsx} : 优雅降级处理
src/main/presenter/configPresenter/mcpConfHelper.ts (7)
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/electron-best-practices.mdc:0-0
Timestamp: 2025-06-30T12:23:13.338Z
Learning: Applies to src/main/presenter/**/*.ts : Use context isolation for improved security
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-06-30T12:24:03.565Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Provider files should include provider-specific helper functions such as `formatMessages`, `convertToProviderTools`, `parseFunctionCalls`, and `prepareFunctionCallPrompt` as needed.
Learnt from: neoragex2002
PR: ThinkInAIXYZ/deepchat#550
File: src/main/presenter/mcpPresenter/inMemoryServers/meetingServer.ts:250-252
Timestamp: 2025-06-21T15:48:29.950Z
Learning: In the meeting server implementation (src/main/presenter/mcpPresenter/inMemoryServers/meetingServer.ts), when multiple tabs have the same title, the user prefers to let the code silently select the first match without adding warnings or additional ambiguity handling.
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-06-30T12:24:03.565Z
Learning: Applies to src/main/presenter/llmProviderPresenter/index.ts : `src/main/presenter/llmProviderPresenter/index.ts` should manage the overall Agent loop, conversation history, tool execution via `McpPresenter`, and communication with the frontend via `eventBus`.
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/electron-best-practices.mdc:0-0
Timestamp: 2025-06-30T12:23:13.338Z
Learning: Applies to {src/main/presenter/**/*.ts,src/renderer/stores/**/*.ts} : Implement proper inter-process communication (IPC) patterns
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/error-logging.mdc:0-0
Timestamp: 2025-06-30T12:23:33.814Z
Learning: Applies to **/*.{ts,tsx} : 优雅降级处理
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-06-23T13:06:15.335Z
Learning: Use TypeScript for all code, preferring types over interfaces, and avoid enums in favor of const objects.
src/main/presenter/mcpPresenter/inMemoryServers/appleServer.ts (4)
Learnt from: neoragex2002
PR: ThinkInAIXYZ/deepchat#550
File: src/main/presenter/mcpPresenter/inMemoryServers/meetingServer.ts:250-252
Timestamp: 2025-06-21T15:48:29.950Z
Learning: In the meeting server implementation (src/main/presenter/mcpPresenter/inMemoryServers/meetingServer.ts), when multiple tabs have the same title, the user prefers to let the code silently select the first match without adding warnings or additional ambiguity handling.
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-06-30T12:24:03.565Z
Learning: Maintain separation of concerns by centralizing Agent loop logic in `index.ts` and keeping provider files focused on API interaction and event standardization.
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-06-30T12:24:03.565Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Provider files should include provider-specific helper functions such as `formatMessages`, `convertToProviderTools`, `parseFunctionCalls`, and `prepareFunctionCallPrompt` as needed.
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/development-setup.mdc:0-0
Timestamp: 2025-06-30T12:23:01.752Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Log和注释使用英文书写
🪛 Biome (1.9.4)
src/main/presenter/mcpPresenter/inMemoryServers/appleServer.ts
[error] 145-145: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 147-147: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 617-617: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 783-783: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 802-802: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 974-974: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 1873-1875: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (11)
electron.vite.config.ts (2)
15-15: LGTM! Clean removal of Pyodide from build exclusions.The removal of 'pyodide' from the
excludearray inexternalizeDepsPluginis consistent with the PR objective to remove Pyodide support. This ensures that Pyodide is no longer treated as an exception during dependency externalization.
26-26: LGTM! Proper cleanup of Pyodide from external dependencies.The removal of 'pyodide' from the
externalarray in the Rollup build options is appropriate and aligns with the broader cleanup of Pyodide-related code. This ensures that Pyodide is no longer marked as an external dependency in the build process.src/renderer/src/stores/upgrade.ts (1)
100-100: Good formatting improvement!Adding the space after the
ifkeyword improves code consistency and follows standard JavaScript/TypeScript formatting conventions.src/main/presenter/upgradePresenter/index.ts (1)
233-236: Documentation formatting improvement!Removing trailing spaces in JSDoc comments improves code cleanliness and consistency.
src/renderer/src/components/message/MessageItemUser.vue (2)
143-143: Consistent arrow function parameter syntax!Adding parentheses around the single parameter improves code consistency and follows the project's preferred arrow function style.
161-161: Consistent arrow function parameter syntax!Adding parentheses around the single parameter improves code consistency and follows the project's preferred arrow function style.
src/main/presenter/configPresenter/mcpConfHelper.ts (4)
34-47: Improved readability with multi-line formatting!The multi-line formatting of the macOS platform-specific server configuration makes the code much more readable and easier to understand.
50-63: Improved readability with multi-line formatting!The multi-line formatting of the Windows platform-specific server configuration (even though commented out) maintains consistency and readability.
66-79: Improved readability with multi-line formatting!The multi-line formatting of the Linux platform-specific server configuration maintains consistency with the other platform configurations.
344-347: Improved readability with multi-line conditional formatting!Breaking the complex conditional statement into multiple lines with proper indentation makes the logic much easier to follow and understand.
src/main/presenter/mcpPresenter/inMemoryServers/appleServer.ts (1)
120-126: Placeholder parsing logic – add TODO or implement real parserThe AppleScript result handling still returns hard-coded sample data.
Either implement proper parsing now or clearly mark with a TODO so callers don’t assume real data.
| console.error( | ||
| 'Cannot access Contacts app. Please grant access in System Preferences > Security & Privacy > Privacy > Contacts.' | ||
| ) | ||
| return false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Replace ad-hoc console.error with structured logging
Guidelines require structured logs with timestamp, level, stack-trace, etc. Direct console.error calls (here and throughout the file) break that rule and also emit error-level noise for normal control-flow situations.
Introduce a central logger (e.g. @electron/log, pino, or the project’s chosen wrapper) and emit at info/warn/error levels as appropriate.
🤖 Prompt for AI Agents
In src/main/presenter/mcpPresenter/inMemoryServers/appleServer.ts around lines
45 to 48, replace the direct console.error call with the project's centralized
structured logger. Import the logger used in the project (such as @electron/log
or pino), then log the message at an appropriate level like warn or info instead
of error, including any relevant metadata if needed. This ensures consistent,
structured logging with timestamps and levels, avoiding misuse of error logs for
normal control flow.
| text: `Found ${listReminders.length} reminders in list ${parsedArgs.listId}:\n\n${listReminders | ||
| .map( | ||
| (reminder) => | ||
| `• ${reminder.name} - ${reminder.completed ? 'Completed' : 'Pending'}` | ||
| ) | ||
| .join('\n')}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Variable leaks across switch clauses – wrap case in its own block
Lint flags that listReminders can bleed into other cases.
Wrap the entire clause in braces to constrain scope:
- case 'listById':
+ case 'listById': {
if (!parsedArgs.listId) {
@@
}
return {
@@
}
+ }This prevents accidental access from sibling branches and silences noSwitchDeclarations.
📝 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.
| text: `Found ${listReminders.length} reminders in list ${parsedArgs.listId}:\n\n${listReminders | |
| .map( | |
| (reminder) => | |
| `• ${reminder.name} - ${reminder.completed ? 'Completed' : 'Pending'}` | |
| ) | |
| .join('\n')}` | |
| case 'listById': { | |
| if (!parsedArgs.listId) { | |
| throw new Error('List ID is required'); | |
| } | |
| const listReminders = /* existing logic to fetch reminders */; | |
| return { | |
| text: `Found ${listReminders.length} reminders in list ${parsedArgs.listId}:\n\n${listReminders | |
| .map( | |
| (reminder) => | |
| `• ${reminder.name} - ${reminder.completed ? 'Completed' : 'Pending'}` | |
| ) | |
| .join('\n')}` | |
| }; | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 1873-1875: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🤖 Prompt for AI Agents
In src/main/presenter/mcpPresenter/inMemoryServers/appleServer.ts around lines
1873 to 1878, the variable listReminders declared in a switch case can leak into
other cases, causing scope issues. To fix this, wrap the entire switch case
block containing listReminders in curly braces {} to create a new block scope,
preventing the variable from being accessible outside this case and resolving
the noSwitchDeclarations lint warning.
去掉pyodide支持,改用e2b 沙盒
Summary by CodeRabbit
Bug Fixes
Style
Chores