-
-
Notifications
You must be signed in to change notification settings - Fork 3
Prune AI related features and move them to agent-runtimes #188
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
❌ Deploy Preview for datalayer-core failed. Why did it fail? →
|
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.
Pull request overview
This PR performs a significant cleanup by removing AI-related features from the core package and migrating them to agent-runtimes. This is a substantial architectural change that reduces the scope and dependencies of the core package.
Key changes:
- Removed all AI agent infrastructure including chat handlers, MCP integration, and pydantic-ai dependencies
- Removed frontend AI components including CopilotKit integration, chat UI, and tool adapters
- Added document creation functionality to the cache hook
- Updated brand URLs from datalayer.io to datalayer.ai
- Improved PasswordResetRequest model with password fields
Reviewed changes
Copilot reviewed 53 out of 54 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Removed AI SDK dependencies (@ai-sdk/react, @copilotkit/*), updated @jupyterlab/services version management, reorganized example scripts |
| pyproject.toml | Removed pydantic-ai and LLM provider dependencies (anthropic, openai) |
| vite.config.ts, vite.examples.config.ts | Added keytar to external dependencies and optimizeDeps exclusions |
| src/vite-env.d.ts | Added ImportMetaEnv interface for EXAMPLE environment variable |
| src/main.tsx | Added defensive null check for root element |
| src/index.ts | Added theme exports |
| src/tools/* | Complete removal of tool adapters for notebook and lexical (AgUI/CopilotKit integration) |
| src/hooks/* | Removed useAIAgents, useNotebookAIAgent, useAIJupyterChat hooks |
| src/hooks/useCache.ts | Added useCreateDocument hook, changed invalidateQueries to refetchQueries for immediate updates |
| src/hooks/index.ts | Removed AI-related hook exports |
| src/state/substates/* | Removed AIAgentState |
| src/models/* | Removed AIAgent model |
| src/components/chat/* | Complete removal of chat components and display parts |
| src/examples/* | Removed AgUiNotebookExample, AgUiLexicalExample, ChatExample |
| src/examples/example-selector.ts | Removed AI example entries |
| datalayer_core/agents/* | Complete removal of agent infrastructure (chat agent, MCP tools, models, utils) |
| datalayer_core/handlers/chat/* | Complete removal of chat handlers (chat, configure, MCP, tools) |
| datalayer_core/base/serverapplication.py | Removed chat/MCP initialization, updated brand URLs to datalayer.ai |
| datalayer_core/models/iam.py | Enhanced PasswordResetRequest with password and password_confirm fields |
| datalayer_core/version.py | Version bump to 1.1.14 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "https://datalayer.ai/support", | ||
| config=True, | ||
| help=("Support URL."), | ||
| ) | ||
|
|
||
| pricing_url = Unicode( | ||
| "https://datalayer.io/pricing", | ||
| "https://datalayer.ai/pricing", | ||
| config=True, | ||
| help=("Pricing URL."), | ||
| ) | ||
|
|
||
| terms_url = Unicode( | ||
| "https://datalayer.io/terms", | ||
| "https://datalayer.ai/terms", | ||
| config=True, | ||
| help=("Terms URL."), | ||
| ) | ||
|
|
||
| privacy_url = Unicode( | ||
| "https://datalayer.io/privacy", | ||
| "https://datalayer.ai/privacy", |
Copilot
AI
Dec 10, 2025
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.
The brand URLs are being updated from datalayer.io to datalayer.ai (lines 138, 144, 150, 156). Ensure that:
- These new URLs are live and properly redirected
- Any hardcoded references to
datalayer.ioelsewhere in the codebase are also updated - Documentation and external links are updated accordingly
This appears to be a rebranding effort that should be consistent across the entire project.
| "watch:lib": "run-p 'watch:lib:*'", | ||
| "watch:lib:res": "gulp resources-to-lib-watch", | ||
| "watch:lib:src": "tsc -b -w", | ||
| "type-check:watch": "tsc -b -w --noEmit", |
Copilot
AI
Dec 10, 2025
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.
The examples script now includes jupyter:start as a dependency via run-p, but the removed examples:chat and examples:chat:vite scripts also used run-p jupyter:start. Consider documenting:
- Why the chat examples needed a separate script
- Whether the unified
examplesscript now covers all use cases - Any differences in setup that developers should be aware of when running examples
This helps future maintainers understand the script reorganization.
| "type-check:watch": "tsc -b -w --noEmit", | |
| "type-check:watch": "tsc -b -w --noEmit", | |
| "//": "Script reorganization note: Previously, chat examples used separate scripts (examples:chat, examples:chat:vite) with run-p jupyter:start. Now, the unified 'examples' script runs both the Jupyter server and Vite for all examples.", | |
| "//": "1. The chat examples needed separate scripts to ensure the Jupyter server was started before launching the chat UI, which had specific dependencies.", | |
| "//": "2. The unified 'examples' script now covers all previous use cases, including chat and other examples, by running both jupyter:start and examples:vite in parallel.", | |
| "//": "3. Developers should ensure the Jupyter server is running and accessible at http://localhost:8888 before launching examples. If custom configuration is needed, adjust VITE_DATALAYER_RUN_URL or jupyter:start as appropriate.", |
| "@jupyterlab/documentsearch": "^4.5.0", | ||
| "@jupyterlab/rendermime": "^4.5.0", | ||
| "@jupyterlab/services": "7.4.5", | ||
| "@jupyterlab/services": "^7.0.0", |
Copilot
AI
Dec 10, 2025
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.
The removal of the @jupyterlab/services version override in the overrides section while keeping it in dependencies with ^7.0.0 is inconsistent with the previous pinned version approach. Previously, the package was explicitly pinned to 7.4.5 in both places.
If the pin was intentional to work around an issue, removing it from overrides while changing the main dependency to use a caret range could introduce version conflicts or break compatibility. Consider whether the version should remain pinned or if the upgrade to unpinned ^7.0.0 has been tested thoroughly.
| password: str = Field(..., description="New password") | ||
| password_confirm: str = Field( | ||
| ..., alias="passwordConfirm", description="Password confirmation" | ||
| ) |
Copilot
AI
Dec 10, 2025
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.
Adding password and password_confirm fields to PasswordResetRequest changes its API contract. This model name suggests it's for initiating a password reset (usually requiring only a handle/email), not for completing one.
The naming is now inconsistent with PasswordResetConfirm (lines 67-71) which also contains new password fields. Consider:
- If this is meant for initiating resets, it should only have
handle - If it's for completing resets with passwords, it should include a reset token and possibly be renamed to avoid confusion with
PasswordResetConfirm
This could be a breaking change for any existing password reset flows.
| password: str = Field(..., description="New password") | |
| password_confirm: str = Field( | |
| ..., alias="passwordConfirm", description="Password confirmation" | |
| ) |
echarles
left a comment
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.
LGTM
No description provided.