Skip to content

Conversation

@andyjakubowski
Copy link
Contributor

@andyjakubowski andyjakubowski commented Sep 15, 2025

Signed-off-by: Andy Jakubowski hello@andyjakubowski.com

Summary by CodeRabbit

  • Style
    • Notebook picker now truncates long names with an ellipsis to improve layout; no functional changes.
  • Documentation
    • Streamlined README by removing outdated sections and badges.
    • Added a dedicated guide for running integration (UI) tests.
  • Tests
    • Introduced end-to-end UI testing setup and a basic activation test.
  • Chores
    • Removed obsolete PR label enforcement workflow.
    • Temporarily disabled CI integration tests; minor formatting updates to CI configuration.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 15, 2025

📝 Walkthrough

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "fix(ui): Set max width on notebook dropdown" clearly and concisely describes the primary UI change in src/index.tsx (adding an id and inline max-width/ellipsis styles to the notebook selector). The rest of the changes (test docs and CI/workflow edits) are present but the title focuses on the developer-intended UI fix and is informative for reviewers.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Ruff (0.12.2)
ui-tests/jupyter_server_test_config.py

�[1;31mruff failed�[0m
�[1mCause:�[0m Failed to load configuration /ruff.toml
�[1mCause:�[0m Failed to parse /ruff.toml
�[1mCause:�[0m TOML parse error at line 26, column 3
|
26 | "RSE100", # Use of assert detected
| ^^^^^^^^
Unknown rule selector: RSE100


Comment @coderabbitai help to get the list of available commands and usage tips.

Signed-off-by: Andy Jakubowski <hello@andyjakubowski.com>
@andyjakubowski andyjakubowski force-pushed the andy/oss-93-set-max-width-on-notebook-dropdown branch from f20a912 to 7cbb8a9 Compare September 15, 2025 13:33
@andyjakubowski andyjakubowski marked this pull request as ready for review September 15, 2025 13:33
It causes Check Links errors, possibly because CI can’t access private repo URLs?

Signed-off-by: Andy Jakubowski <hello@andyjakubowski.com>
We’re not using these right now

Signed-off-by: Andy Jakubowski <hello@andyjakubowski.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ca53a42 and 7cbb8a9.

📒 Files selected for processing (1)
  • src/index.tsx (1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T12:52:09.726Z
Learning: Applies to src/notebooks/deepnote/deepnoteNotebookSelector.ts : deepnoteNotebookSelector.ts implements UI selection logic
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/notebooks.instructions.md:0-0
Timestamp: 2025-09-03T12:59:14.489Z
Learning: Applies to src/notebooks/export/**/*.ts : Add UI elements for the new format to the export quick pick
📚 Learning: 2025-09-12T12:52:09.726Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T12:52:09.726Z
Learning: Applies to src/notebooks/deepnote/deepnoteNotebookSelector.ts : deepnoteNotebookSelector.ts implements UI selection logic

Applied to files:

  • src/index.tsx
📚 Learning: 2025-09-03T12:59:14.489Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/notebooks.instructions.md:0-0
Timestamp: 2025-09-03T12:59:14.489Z
Learning: Applies to src/notebooks/export/**/*.ts : Add UI elements for the new format to the export quick pick

Applied to files:

  • src/index.tsx
📚 Learning: 2025-09-12T12:52:09.726Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T12:52:09.726Z
Learning: Applies to src/notebooks/deepnote/deepnoteNotebookManager.ts : deepnoteNotebookManager.ts handles state management

Applied to files:

  • src/index.tsx
📚 Learning: 2025-09-12T12:52:09.726Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T12:52:09.726Z
Learning: Applies to src/notebooks/deepnote/deepnoteTypes.ts : deepnoteTypes.ts should contain only type definitions

Applied to files:

  • src/index.tsx

andyjakubowski and others added 4 commits September 15, 2025 15:58
Signed-off-by: Andy Jakubowski <hello@andyjakubowski.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: Andy Jakubowski <hello@andyjakubowski.com>
Signed-off-by: Andy Jakubowski <hello@andyjakubowski.com>
@andyjakubowski andyjakubowski force-pushed the andy/oss-93-set-max-width-on-notebook-dropdown branch from 674849e to 13bb29f Compare September 15, 2025 14:15
Signed-off-by: Andy Jakubowski <hello@andyjakubowski.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 674849e and 13bb29f.

⛔ Files ignored due to path filters (1)
  • ui-tests/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (5)
  • ui-tests/README.md (1 hunks)
  • ui-tests/jupyter_server_test_config.py (1 hunks)
  • ui-tests/package.json (1 hunks)
  • ui-tests/playwright.config.js (1 hunks)
  • ui-tests/tests/jupyterlab_deepnote.spec.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/notebooks.instructions.md:0-0
Timestamp: 2025-09-03T12:59:14.489Z
Learning: Applies to src/notebooks/export/**/*.ts : Add UI elements for the new format to the export quick pick
📚 Learning: 2025-09-03T12:54:35.368Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/interactiveWindow.instructions.md:0-0
Timestamp: 2025-09-03T12:54:35.368Z
Learning: Applies to src/interactive-window/**/*.ts : Add unit and integration tests for new features and modifications; include performance and cross-platform test coverage

Applied to files:

  • ui-tests/tests/jupyterlab_deepnote.spec.ts
  • ui-tests/README.md
📚 Learning: 2025-09-12T07:58:04.493Z
Learnt from: Artmann
PR: deepnote/vscode-deepnote#12
File: src/notebooks/deepnote/deepnoteActivationService.unit.test.ts:31-45
Timestamp: 2025-09-12T07:58:04.493Z
Learning: In src/notebooks/deepnote/deepnoteActivationService.ts, the activate() method is synchronous with signature `public activate()` and returns void. It should be called without await in tests.

Applied to files:

  • ui-tests/tests/jupyterlab_deepnote.spec.ts
📚 Learning: 2025-09-14T18:25:57.101Z
Learnt from: jamesbhobbs
PR: deepnote/deepnote#18401
File: scripts/check-chromatic-enabled/pr-template.test.ts:1-6
Timestamp: 2025-09-14T18:25:57.101Z
Learning: In the deepnote/deepnote repository, colocated tests use Node's built-in test runner (node:test, node:assert/strict) rather than Jest.

Applied to files:

  • ui-tests/package.json
📚 Learning: 2025-09-03T13:01:49.096Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-09-03T13:01:49.096Z
Learning: Use `npm run test:integration:web` to run integration tests with Remote Jupyter Kernels and VS Code in browser (`--grep` not supported)

Applied to files:

  • ui-tests/README.md
📚 Learning: 2025-09-03T13:01:49.096Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-09-03T13:01:49.096Z
Learning: Use `npm run test:integration` to run integration tests with local Jupyter Kernels (use `-- -- --grep <pattern>` to filter)

Applied to files:

  • ui-tests/README.md
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Use real Jupyter servers for integration tests and mock jupyterlab/services for unit tests; cover both success and failure scenarios and verify cleanup

Applied to files:

  • ui-tests/README.md
  • ui-tests/jupyter_server_test_config.py
🪛 markdownlint-cli2 (0.17.2)
ui-tests/README.md

32-32: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1

(MD029, ol-prefix)


41-41: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1

(MD029, ol-prefix)


69-69: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1

(MD029, ol-prefix)


78-78: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1

(MD029, ol-prefix)


105-105: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1

(MD029, ol-prefix)


114-114: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1

(MD029, ol-prefix)


121-121: Ordered list item prefix
Expected: 1; Actual: 4; Style: 1/1/1

(MD029, ol-prefix)


143-143: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1

(MD029, ol-prefix)


152-152: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1

(MD029, ol-prefix)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Integration tests
🔇 Additional comments (2)
ui-tests/playwright.config.js (1)

6-14: LGTM! Clean Galata integration.

The configuration correctly extends the base Galata config and adds a web server setup with appropriate timeout and CI detection.

ui-tests/README.md (1)

161-167: Verify Yarn command.

jlpm up "@playwright/test" may not exist depending on Yarn version (Yarn 1 typically uses upgrade). Confirm toolchain; adjust docs if needed.

Comment on lines +3 to +6
!! Never use this configuration in production because it
opens the server to the world and provide access to JupyterLab
JavaScript objects through the global window variable.
"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Docstring nits.

-!! Never use this configuration in production because it
-opens the server to the world and provide access to JupyterLab
-JavaScript objects through the global window variable.
+!! Never use this configuration in production because it
+opens the server to the world and provides access to JupyterLab
+JavaScript objects through the global window variable.
📝 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.

Suggested change
!! Never use this configuration in production because it
opens the server to the world and provide access to JupyterLab
JavaScript objects through the global window variable.
"""
!! Never use this configuration in production because it
opens the server to the world and provides access to JupyterLab
JavaScript objects through the global window variable.
"""
🤖 Prompt for AI Agents
In ui-tests/jupyter_server_test_config.py around lines 3 to 6, the module
docstring is informal and uses "!!" and inconsistent punctuation; replace it
with a proper triple-quoted docstring sentence starting with a capital letter,
without leading exclamation marks, and ending with a period. Keep the warning
concise (e.g., "Never use this configuration in production because it opens the
server to the world and exposes JupyterLab JavaScript objects via the global
window variable.") and ensure it is the first statement in the file.

Comment on lines +7 to +12
from jupyterlab.galata import configure_jupyter_server

configure_jupyter_server(c)

# Uncomment to set server log level to debug level
# c.ServerApp.log_level = "DEBUG"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

NameError: c is undefined.

The config object c isn’t created before use. This will break jlpm start.

-from jupyterlab.galata import configure_jupyter_server
+from jupyterlab.galata import configure_jupyter_server
+from traitlets.config import get_config
+
+c = get_config()
 
 configure_jupyter_server(c)
 
 # Uncomment to set server log level to debug level
 # c.ServerApp.log_level = "DEBUG"
📝 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.

Suggested change
from jupyterlab.galata import configure_jupyter_server
configure_jupyter_server(c)
# Uncomment to set server log level to debug level
# c.ServerApp.log_level = "DEBUG"
from jupyterlab.galata import configure_jupyter_server
from traitlets.config import get_config
c = get_config()
configure_jupyter_server(c)
# Uncomment to set server log level to debug level
# c.ServerApp.log_level = "DEBUG"
🤖 Prompt for AI Agents
In ui-tests/jupyter_server_test_config.py around lines 7 to 12, the variable `c`
is used before being defined; add the missing import and initialization by
importing get_config from traitlets.config and calling `c = get_config()` before
passing `c` to configure_jupyter_server(c) so the config object exists when
used.

Comment on lines +6 to +14
"scripts": {
"start": "jupyter lab --config jupyter_server_test_config.py",
"test": "jlpm playwright test",
"test:update": "jlpm playwright test --update-snapshots"
},
"devDependencies": {
"@jupyterlab/galata": "^5.0.5",
"@playwright/test": "^1.37.0"
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Pin test deps to reduce CI drift; add engines and report script.

Caret ranges can cause flaky upgrades. Pin versions and declare runtime expectations.

   "scripts": {
     "start": "jupyter lab --config jupyter_server_test_config.py",
     "test": "jlpm playwright test",
-    "test:update": "jlpm playwright test --update-snapshots"
+    "test:update": "jlpm playwright test --update-snapshots",
+    "report": "jlpm playwright show-report"
   },
+  "engines": {
+    "node": ">=18",
+    "yarn": ">=1.22"
+  },
   "devDependencies": {
-    "@jupyterlab/galata": "^5.0.5",
-    "@playwright/test": "^1.37.0"
+    "@jupyterlab/galata": "5.0.5",
+    "@playwright/test": "1.37.0"
   }
📝 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.

Suggested change
"scripts": {
"start": "jupyter lab --config jupyter_server_test_config.py",
"test": "jlpm playwright test",
"test:update": "jlpm playwright test --update-snapshots"
},
"devDependencies": {
"@jupyterlab/galata": "^5.0.5",
"@playwright/test": "^1.37.0"
}
"scripts": {
"start": "jupyter lab --config jupyter_server_test_config.py",
"test": "jlpm playwright test",
"test:update": "jlpm playwright test --update-snapshots",
"report": "jlpm playwright show-report"
},
"engines": {
"node": ">=18",
"yarn": ">=1.22"
},
"devDependencies": {
"@jupyterlab/galata": "5.0.5",
"@playwright/test": "1.37.0"
}
🤖 Prompt for AI Agents
In ui-tests/package.json around lines 6 to 14, the devDependencies use caret
ranges and there are no engines or test-report scripts: pin the dependency
versions by removing caret ranges (e.g., set "@jupyterlab/galata": "5.0.5" and
"@playwright/test": "1.37.0"), add an "engines" field declaring supported
runtimes (for example "node": ">=18 <21" and "npm": ">=9" or your project's
supported ranges), and add a script to produce test reports (for example a
"test:report" or "report" script that runs Playwright with --reporter or
generates the HTML report). Ensure the script names and engine ranges match your
CI/runtime policy.


> Check the extension is installed in JupyterLab.
2. Install test dependencies (needed only once):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Fix ordered-list numbering (MD029).

Use "1." for all items to satisfy markdownlint and keep auto-numbering consistent.

-2. Install test dependencies (needed only once):
+1. Install test dependencies (needed only once):
-3. Execute the [Playwright](https://playwright.dev/docs/intro) tests:
+1. Execute the [Playwright](https://playwright.dev/docs/intro) tests:
-2. Install test dependencies (needed only once):
+1. Install test dependencies (needed only once):
-3. Execute the [Playwright](https://playwright.dev/docs/intro) command:
+1. Execute the [Playwright](https://playwright.dev/docs/intro) command:
-2. Install test dependencies (needed only once):
+1. Install test dependencies (needed only once):
-3. Start the server:
+1. Start the server:
-4. Execute the [Playwright code generator](https://playwright.dev/docs/codegen) in **another terminal**:
+1. Execute the [Playwright code generator](https://playwright.dev/docs/codegen) in **another terminal**:
-2. Install test dependencies (needed only once):
+1. Install test dependencies (needed only once):
-3. Execute the Playwright tests in [debug mode](https://playwright.dev/docs/debug):
+1. Execute the Playwright tests in [debug mode](https://playwright.dev/docs/debug):

Also applies to: 41-41, 69-69, 78-78, 105-105, 114-114, 121-121, 143-143, 152-152

🧰 Tools
🪛 markdownlint-cli2 (0.17.2)

32-32: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1

(MD029, ol-prefix)

🤖 Prompt for AI Agents
In ui-tests/README.md around lines 32 (and also apply same change at lines 41,
69, 78, 105, 114, 121, 143, 152), the ordered list uses explicit incremental
numbers which triggers MD029; change each ordered-list item’s leading number to
"1." so the markdown uses auto-numbering consistently and satisfies
markdownlint.

Comment on lines +85 to +88
> Some discrepancy may occurs between the snapshots generated on your computer and
> the one generated on the CI. To ease updating the snapshots on a PR, you can
> type `please update playwright snapshots` to trigger the update by a bot on the CI.
> Once the bot has computed new snapshots, it will commit them to the PR branch.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Tighten wording/grammar.

Minor copy fixes.

-> Some discrepancy may occurs between the snapshots generated on your computer and
-> the one generated on the CI. To ease updating the snapshots on a PR, you can
+> Some discrepancies may occur between the snapshots generated on your computer and
+> the ones generated in CI. To ease updating snapshots on a PR, you can
📝 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.

Suggested change
> Some discrepancy may occurs between the snapshots generated on your computer and
> the one generated on the CI. To ease updating the snapshots on a PR, you can
> type `please update playwright snapshots` to trigger the update by a bot on the CI.
> Once the bot has computed new snapshots, it will commit them to the PR branch.
> Some discrepancies may occur between the snapshots generated on your computer and
> the ones generated in CI. To ease updating snapshots on a PR, you can
> type `please update playwright snapshots` to trigger the update by a bot on the CI.
> Once the bot has computed new snapshots, it will commit them to the PR branch.
🤖 Prompt for AI Agents
In ui-tests/README.md around lines 85 to 88, the wording and grammar in the
paragraph about snapshot discrepancies is awkward; rewrite it for clarity and
correctness. Replace "Some discrepancy may occurs between the snapshots
generated on your computer and the one generated on the CI." with a
grammatically correct sentence (e.g., "Snapshots generated locally may differ
from those produced on CI."), clarify that typing `please update playwright
snapshots` will trigger a bot to update snapshots on the CI, and make the final
sentence concise (e.g., "When the bot generates updated snapshots, it will
commit them to the PR branch."). Ensure subject-verb agreement, consistent
plurality, and concise phrasing.

Comment on lines +16 to +22
await page.goto();

expect(
logs.filter(
s => s === 'JupyterLab extension jupyterlab-deepnote is activated!'
)
).toHaveLength(1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Wait for the log to avoid flakiness.

page.goto() can resolve before the activation log is emitted. Poll the in-memory log until it stabilizes.

-  expect(
-    logs.filter(
-      s => s === 'JupyterLab extension jupyterlab-deepnote is activated!'
-    )
-  ).toHaveLength(1);
+  await expect
+    .poll(
+      () =>
+        logs.filter(
+          s => s === 'JupyterLab extension jupyterlab-deepnote is activated!'
+        ).length,
+      { timeout: 10000 }
+    )
+    .toBe(1);
📝 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.

Suggested change
await page.goto();
expect(
logs.filter(
s => s === 'JupyterLab extension jupyterlab-deepnote is activated!'
)
).toHaveLength(1);
await page.goto();
await expect
.poll(
() =>
logs.filter(
s => s === 'JupyterLab extension jupyterlab-deepnote is activated!'
).length,
{ timeout: 10000 }
)
.toBe(1);
🤖 Prompt for AI Agents
In ui-tests/tests/jupyterlab_deepnote.spec.ts around lines 16–22, the test
checks the activation log immediately after page.goto(), which is flaky because
page.goto() can resolve before the activation message is emitted; change the
assertion to poll the in-memory logs until the expected string appears (or until
a timeout) and stabilizes — e.g. use a wait-for helper or page.waitForFunction
that repeatedly evaluates the logs and returns once logs.filter(s => s ===
'JupyterLab extension jupyterlab-deepnote is activated!').length === 1
(optionally ensure no further changes for a short stabilization window) to make
the test deterministic.

coderabbitai[bot]
coderabbitai bot previously requested changes Sep 15, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
ui-tests/tests/jupyterlab_deepnote.spec.ts (1)

24-24: Add a UI test for the notebook picker width/ellipsis (covers the PR’s stated fix).

Validates CSS on #deepnote-notebook-picker.

Apply this diff (append new test):

+test('notebook picker should constrain width and ellipsize', async ({ page }) => {
+  await page.goto();
+  const picker = page.locator('#deepnote-notebook-picker');
+  await expect(picker).toBeVisible();
+  await expect(picker).toHaveCSS('white-space', 'nowrap');
+  await expect(picker).toHaveCSS('overflow', 'hidden');
+  await expect(picker).toHaveCSS('text-overflow', 'ellipsis');
+  // Optional: assert a max-width is set (value may vary if using rems)
+  const maxWidth = await picker.evaluate(el => getComputedStyle(el).maxWidth);
+  expect(maxWidth).not.toBe('none');
+});
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 674849e and cdd8054.

⛔ Files ignored due to path filters (1)
  • ui-tests/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (6)
  • .github/workflows/build.yml (1 hunks)
  • ui-tests/README.md (1 hunks)
  • ui-tests/jupyter_server_test_config.py (1 hunks)
  • ui-tests/package.json (1 hunks)
  • ui-tests/playwright.config.js (1 hunks)
  • ui-tests/tests/jupyterlab_deepnote.spec.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-09-03T13:01:49.096Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-09-03T13:01:49.096Z
Learning: Use `npm run test:integration:web` to run integration tests with Remote Jupyter Kernels and VS Code in browser (`--grep` not supported)

Applied to files:

  • ui-tests/README.md
📚 Learning: 2025-09-03T13:01:49.096Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-09-03T13:01:49.096Z
Learning: Use `npm run test:integration` to run integration tests with local Jupyter Kernels (use `-- -- --grep <pattern>` to filter)

Applied to files:

  • ui-tests/README.md
📚 Learning: 2025-09-03T12:54:35.368Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/interactiveWindow.instructions.md:0-0
Timestamp: 2025-09-03T12:54:35.368Z
Learning: Applies to src/interactive-window/**/*.ts : Add unit and integration tests for new features and modifications; include performance and cross-platform test coverage

Applied to files:

  • ui-tests/README.md
  • ui-tests/tests/jupyterlab_deepnote.spec.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Use real Jupyter servers for integration tests and mock jupyterlab/services for unit tests; cover both success and failure scenarios and verify cleanup

Applied to files:

  • ui-tests/README.md
  • ui-tests/jupyter_server_test_config.py
📚 Learning: 2025-09-12T07:58:04.493Z
Learnt from: Artmann
PR: deepnote/vscode-deepnote#12
File: src/notebooks/deepnote/deepnoteActivationService.unit.test.ts:31-45
Timestamp: 2025-09-12T07:58:04.493Z
Learning: In src/notebooks/deepnote/deepnoteActivationService.ts, the activate() method is synchronous with signature `public activate()` and returns void. It should be called without await in tests.

Applied to files:

  • ui-tests/tests/jupyterlab_deepnote.spec.ts
📚 Learning: 2025-09-14T18:25:57.101Z
Learnt from: jamesbhobbs
PR: deepnote/deepnote#18401
File: scripts/check-chromatic-enabled/pr-template.test.ts:1-6
Timestamp: 2025-09-14T18:25:57.101Z
Learning: In the deepnote/deepnote repository, colocated tests use Node's built-in test runner (node:test, node:assert/strict) rather than Jest.

Applied to files:

  • ui-tests/package.json
🪛 markdownlint-cli2 (0.17.2)
ui-tests/README.md

32-32: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1

(MD029, ol-prefix)


41-41: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1

(MD029, ol-prefix)


69-69: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1

(MD029, ol-prefix)


78-78: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1

(MD029, ol-prefix)


105-105: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1

(MD029, ol-prefix)


114-114: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1

(MD029, ol-prefix)


121-121: Ordered list item prefix
Expected: 1; Actual: 4; Style: 1/1/1

(MD029, ol-prefix)


143-143: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1

(MD029, ol-prefix)


152-152: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1

(MD029, ol-prefix)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test_isolated
  • GitHub Check: check_release
🔇 Additional comments (2)
.github/workflows/build.yml (1)

96-155: Integration tests disabled in CI — confirm intent and rollout plan.

The entire job is commented out. If temporary, track re‑enablement (label, issue, or TODO) so the new UI harness doesn’t silently bit‑rot.

ui-tests/package.json (1)

11-14: Pin devDependencies and align Galata/Playwright with the repo's JupyterLab major

Pin exact versions for reproducible UI runs — current devDependencies show @jupyterlab/galata ^5.0.5 and @playwright/test ^1.37.0.

  • If this repo targets JupyterLab 4.x: use @jupyterlab/galata 4.4.0 and @playwright/test 1.46.1 (pin exact).
  • If this repo targets JupyterLab 5.x: pin @jupyterlab/galata 5.0.5 and update @playwright/test to the Playwright version required by Galata 5; confirm and pin that exact version.

Location: ui-tests/package.json — devDependencies (lines 11–14).

@andyjakubowski andyjakubowski dismissed coderabbitai[bot]’s stale review September 15, 2025 14:48

CodeRabbit commented on code unrelated to the PR.

@andyjakubowski andyjakubowski merged commit 61ad4cc into main Sep 15, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants