Repository navigation
pysimlin: start the journey's kernel before the notebook opens - #1074
Merged
Merged
Conversation
The JupyterLab journey failed intermittently with a cell that never produced its widget. JupyterLab starts a notebook's kernel session itself on open, and when that one request fails it raises a dialog and leaves the notebook with no kernel for good, so the journey waited out its timeout. The harness starts the session over the server's REST API first (Lab connects to an existing session), retries a refused start, and fails at once with the dialog's text if Lab raises one. The kernel wait read the execution indicator, which says idle before any kernel exists, so a cell could be run ahead of the session; it asks the application for the notebook's session state. Helpers acted on the visible notebook, which after Lab's workspace restore could be the previous test's; each notebook opens in its own workspace.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Reviewed the kernel-session changes. The pure parts (startNotebookSession, kernelState, canRunCells) look correct and are exhaustively tested in kernel-session.spec.ts; the imperative shell in waitForKernel/openNotebook reads session facts via window.jupyterapp (enabled by --LabApp.expose_app_in_browser=True) and polls the notebook's own path before acting. The retry loop in startNotebookSession correctly definitely-assigns refusal in both branches before use, and expect.poll stops at the first dialog match so the raised variable is reliably set when the post-poll throw fires. No blocking issues found. Overall correctness: CORRECT. No findings. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1074 +/- ##
=======================================
Coverage 92.40% 92.40%
=======================================
Files 304 304
Lines 184569 184569
=======================================
+ Hits 170555 170556 +1
+ Misses 14014 14013 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This was referenced Oct 2, 2026
bpowers
added a commit
that referenced
this pull request
Oct 3, 2026
## Why The JupyterLab journey kept failing after #1074, always the same way: a notebook's first cell was run and produced no output at all. It failed once on main and three times across open pull requests. #1074 fixed a real race in the harness (the kernel session start); this is a second cause, and it is in the kernel, not the harness. ipykernel through 7.3 writes shell replies raw on the shell ROUTER socket that its `ZMQStream` reads. The raw send drains the socket's edge-triggered read edge, so a request arriving at the same moment is never dispatched and the kernel sits idle for good ([ipython/ipykernel#1529](ipython/ipykernel#1529), first released in 7.4.0). JupyterLab sends a burst of shell requests when it connects a notebook (history, debug, the widget manager's comm), which makes a notebook's first cell the likely victim: Lab sends the `execute_request`, the prompt shows `[*]`, and nothing ever comes back. ## What - The `e2e` extra requires `ipykernel>=7.4` (the lock moves ipykernel 7.2.0 to 7.4.0, with jupyter-client and tornado at 7.4.0's own floors). - **Running a cell is an action with a checked effect.** `runCell` waits for a new execution count and otherwise fails at the run, saying which side never acted (`[*]`: Lab sent it and the kernel has not answered; no new count: Lab never ran it), with the kernel state and the server log's last lines. There is no retry: a wedged kernel answers nothing, so pressing again cannot help. - The setup waits out a partially written server runtime file instead of throwing on it (`jupyter_server` writes it in place; this failed the setup once in 121 runs). - **The failure artifact was always empty:** `actions/upload-artifact@v4` skips hidden files, and the journey writes to `e2e/.output`. The upload includes hidden files, so the trace, screenshots and server log reach the artifact. ## Evidence - **A reproduced failure** (four concurrent journeys sharing four CPUs): the websocket trace shows the `execute_request` sent on the notebook's own connection and never answered, and a stack dump of that kernel shows every thread idle in `select()`. - **A kernel-level reproducer** with no browser (`jupyter_client`, shell requests pipelined eight deep): ipykernel 7.2.0 wedged in 10 of 10 trials; 7.4.0 in 0 of 10 trials of 3,000 requests each. - **The journey under the same concurrent load:** 3 failures in 121 runs before (two kernel wedges, one runtime-file race), 120 of 120 after. - Unit specs in `kernel-session.spec.ts` cover every arm of the new pure decisions (every truncation of a runtime file, the prompt forms, each before-and-after pair of a cell run, both report verdicts). - The pre-commit hook passed in full. ## What this does not establish - The failure on main left no artifact (see above), so it is attributed to this wedge by its shape, not by a trace. - At a base rate of about 2%, 120 clean runs happen by chance about one time in eleven; the kernel reproducer carries the claim, not the loop. - Nothing here has run on GitHub's runners yet; this PR's own check is the first. - pysimlin users on ipykernel 7.0 to 7.3 with JupyterLab 4.4 or later can hit the same wedge in their own notebooks. This changes only the test extra, not pysimlin's runtime dependencies. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LYR7N9WZwQuFnZFW8JDUgx
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
1 participant
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.
Why
The
pysimlin notebook editor journey (JupyterLab)check fails intermittently on unrelated pull requests (#1070, #1071 three times, #1072), always the same way: a cell is run, and the widget never appears before the 90 s timeout. The cause is in the harness, not the widget. Three races, each reproduced under load before it was fixed:idleby default, so Shift+Enter could race Lab's session initialization.What
startNotebookSessionstarts the notebook's kernel session over the server's REST API before the browser opens the notebook (Lab connects to an existing session instead of starting its own), retries a refused start up to three times, and fails with the server's own words.waitForKernelasks the application for the notebook's session state (window.jupyterapp, enabled with--LabApp.expose_app_in_browser=True) instead of reading the indicator, and a dialog over the notebook fails the wait at once with the dialog's text.kernel-session.spec.tspins the pure parts (the request body, the retry, the state classification).Evidence
make -C src/pysimlin e2eon this branch: 7 passed each time (the three journeys and the four new unit specs).What this does not establish
🤖 Generated with Claude Code
https://claude.ai/code/session_01LYR7N9WZwQuFnZFW8JDUgx