Conversation
`podName` is `runtimeName`. That collided in the two toolbars, where a local state called `runtimeName` already held the label a person types into the New Agent dialog — a different thing from the runtime's identity, and until now spelled the same way. The label is `givenName`, as it is on the wire. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved dependency peer conflicts and runtime identifier issues block safe approval.
Review effort: Lite
Findings: 3
Open (7)
What changed in this PR
Updates Datalayer dependencies and migrates runtime identifiers from pod names to runtimeName across the Electron app.
Changes:
- Refreshes Datalayer, React, Lexical, Vite, and tooling dependencies.
- Updates runtime APIs, IPC, UI components, fixtures, and tests.
- Adjusts module resolution, RequireJS behavior, and documentation.
| File | Summary |
|---|---|
tests/mocks/datalayer-api.ts |
Updates runtime mock parameters. |
tests/fixtures/index.ts |
Updates runtime fixture fields. |
src/renderer/theme/themeStore.ts |
Updates theme store typing. |
src/renderer/services/runtime/RuntimeService.ts |
Migrates runtime lifecycle handling. |
src/renderer/services/interfaces/IRuntimeService.ts |
Updates runtime service API. |
src/renderer/polyfills/requirejs.js |
Adjusts AMD behavior. |
src/renderer/pages/Runtimes.tsx |
Updates runtime termination usage. |
src/renderer/pages/NotebookEditor.tsx |
Migrates notebook runtime state. |
src/renderer/pages/Environments.tsx |
Updates environment terminology. |
src/renderer/pages/DocumentEditor.tsx |
Migrates document runtime state. |
src/renderer/components/runtimes/utils.ts |
Updates runtime formatting. |
src/renderer/components/runtimes/RuntimeItem.tsx |
Updates runtime display. |
src/renderer/components/runtime/RuntimeToolbar.tsx |
Updates runtime controls. |
src/renderer/components/runtime/RuntimeSelector.tsx |
Updates runtime selection. |
src/renderer/components/runtime/RuntimeProgressBar.tsx |
Updates runtime lookup. |
src/renderer/components/runtime/__tests__/RuntimeProgressBar.test.tsx |
Updates progress-bar tests. |
src/renderer/components/notebook/Toolbar.tsx |
Updates notebook runtime controls. |
src/renderer/components/notebook/NotebookControls.tsx |
Updates notebook control props. |
src/renderer/components/notebook/__tests__/Toolbar.test.tsx |
Updates toolbar tests. |
src/renderer/components/lexical/LexicalEditor.tsx |
Migrates runtime props. |
src/renderer/components/document/Header.tsx |
Updates runtime display. |
src/renderer/components/app/NavigationTabs.tsx |
Updates navigation terminology. |
src/preload/index.ts |
Updates IPC runtime APIs. |
src/main/services/__tests__/datalayer-sdk-bridge.integration.test.ts |
Updates bridge fixtures. |
src/main/index.ts |
Updates runtime IPC handlers. |
package.json |
Updates dependencies and toolchain versions. |
electron.vite.config.ts |
Adds workspace dependency resolution. |
DEVELOPMENT.md |
Updates runtime documentation. |
CLAUDE.md |
Updates runtime examples and guidance. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| "electron-builder": "^25.1.8", | ||
| "electron-vite": "^4.0.1", | ||
| "eslint": "^9.36.0", | ||
| "eslint": "^10.9.1", |
Member
Author
There was a problem hiding this comment.
Fixed in 491e72f: eslint and @eslint/js are back on 9 (eslint-plugin-jsx-a11y and eslint-plugin-react stop at 9); a clean npm install now resolves.
| "typescript": "^5.7.2", | ||
| "vite": "^5.4.11", | ||
| "typescript": "5.9.3", | ||
| "vite": "7.3.6", |
Member
Author
There was a problem hiding this comment.
@vitejs/plugin-react resolves to 4.7.0, which declares vite ^7; the range now says ^4.7.0 so the lockstep is explicit.
| uid: result.uid, | ||
| givenName: runtimeName, | ||
| podName: result.podName, | ||
| runtimeName: result.runtimeName, |
Member
Author
There was a problem hiding this comment.
Fixed: terminateRuntime() now calls deleteRuntime(notebookRuntime.runtime.runtimeName).
| uid: 'runtime-123', | ||
| given_name: 'test-notebook-runtime', | ||
| pod_name: 'datalayer-runtime-abc123', | ||
| runtime_name: 'datalayer-runtime-abc123', |
Member
Author
There was a problem hiding this comment.
Fixed: the fixtures carry the camelCase RuntimeJSON shape the SDK bridge hands the renderer (runtimeName, givenName, environmentName, ...).
Comment on lines
+57
to
+60
| "@lexical/code": "0.49.0", | ||
| "@lexical/link": "0.49.0", | ||
| "@lexical/list": "0.49.0", | ||
| "@lexical/react": "0.49.0", |
Member
Author
There was a problem hiding this comment.
The PR description now states the full scope: CI without the core fork, dependency bumps, the runtime identifier rename and the docs rename.
| * Manually notify all subscribers that a runtime has been terminated. | ||
| * This propagates the termination to ALL editors connected to that runtime. | ||
| * @param podName - The pod name of the terminated runtime | ||
| * @param runtimeName - The pod name of the terminated runtime |
Member
Author
There was a problem hiding this comment.
Fixed: the JSDoc says "the name of the terminated runtime".
…kout; eslint 9 (plugins have no 10 support); Prettier pass; CLAUDE.md becomes AGENTS.md
echarles
changed the title
… fixtures, Runtime label and JSDoc; production build: SWC pinned for vite-plugin-top-level-await, plugin-react range that declares Vite 7
… and never gets a runner)
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
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.
goanpeca/core@sdk/core-updates: every workflow checks out this repository alone and.github/actions/setup-environmentonly sets up Node 22 and runsnpm install.@datalayer/core,@datalayer/agent-runtimesand the other Datalayer packages are the published npm releases pinned inpackage.json(runtime features come from@datalayer/agent-runtimes;@datalayer/coreprovides the state store and naming utilities).eslintand@eslint/jsstay on 9 becauseeslint-plugin-jsx-a11yandeslint-plugin-reactdo not support 10 (npm installfailed with ERESOLVE).CLAUDE.mdrenamed toAGENTS.md, with the intro and dependency notes realigned;DEVELOPMENT.mdandREADME.mdupdated accordingly.Verified from a clean export:
npm install,format:check,lint,type-check(0 errors) andtest:unit(224 tests) all pass.