From dde7a26483b039d22271de5b4c9430f5093cf951 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Fri, 26 Jun 2026 05:35:13 +0000 Subject: [PATCH] docker: address second-round review feedback on the JupyterLab/Studio UX - studio_launch.sh: also gate the categorized-view landing URL on UNSLOTH_SKIP_NOTEBOOK_SYNC (the entrypoint skips building the view entirely in that mode), not just UNSLOTH_SKIP_NOTEBOOK_VIEW, so a no-sync container does not land on a missing folder. - Dockerfile.studio: scope the sticker-install "|| echo" fallback to only the sticker step via a { ...; } group. It was attached to the whole branding && chain, so a failure in a REQUIRED step (JS resolve, favicon/logo/login copy) was swallowed and the build continued with broken branding. - unsloth_nb_view.py: when creating the categorized symlinks, only replace our own stale symlinks; if a real user file already occupies that name, keep it and skip the link instead of os.remove-ing it. - overrides.json: drop doNotDisturbMode (it silenced ALL JupyterLab toasts, including kernel-restart / connection-drop feedback). The news/update prompts are already off via fetchNews / checkForUpdates. - Dockerfile: keep decord mandatory on amd64 (fail the build on a missing or incompatible wheel) and only fail-soft on arm64/other arches that have no wheel. - cellNav.ts: do not hijack ArrowUp/Down when focus is in an interactive output widget / form control, or while a completion popup is open, so ipywidgets controls and autocomplete at cell boundaries keep working. --- docker/Dockerfile | 15 ++++++++----- docker/Dockerfile.studio | 11 ++++++---- docker/jupyter/overrides.json | 3 +-- docker/jupyter/unsloth_labext/src/cellNav.ts | 23 ++++++++++++++++++++ docker/studio_launch.sh | 12 ++++++---- docker/unsloth_nb_view.py | 10 +++++++-- 6 files changed, 57 insertions(+), 17 deletions(-) diff --git a/docker/Dockerfile b/docker/Dockerfile index 5ea94b1134..a45bc04942 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -303,11 +303,16 @@ RUN ${VENV}/bin/uv pip install \ "omegaconf==2.3.1" "einx==0.4.3" "librosa==0.11.0" "ftfy==6.3.1" \ && ${VENV}/bin/python -c "import torch, numpy, numba; from packaging.version import Version; assert torch.__version__.startswith('2.10.0'), torch.__version__; assert Version(numpy.__version__) >= Version('2.3'), numpy.__version__; assert Version(numba.__version__) >= Version('0.65'), numba.__version__; print('notebook-deps pins OK:', torch.__version__, numpy.__version__, numba.__version__)" -# decord (ERNIE-VL video decode) publishes wheels only for x86_64 / win_amd64, -# so install it on its own and fail-soft: amd64 gets it; on arm64 the ERNIE-VL -# video path is skipped rather than breaking the whole image build. -RUN ${VENV}/bin/uv pip install --python ${VENV}/bin/python "decord==0.6.0" \ - || echo ">> decord skipped (no matching wheel for ${TARGETARCH:-amd64}); ERNIE-VL video decode unavailable" +# decord (ERNIE-VL video decode) publishes wheels only for x86_64 / win_amd64. +# Install it on its own: HARD on amd64 (a missing/incompatible wheel is a real +# regression there and must fail the build), fail-soft on arm64/other (no wheel +# exists, so drop the ERNIE-VL video path rather than break the image build). +RUN if [ "${TARGETARCH:-amd64}" = "amd64" ]; then \ + ${VENV}/bin/uv pip install --python ${VENV}/bin/python "decord==0.6.0"; \ + else \ + ${VENV}/bin/uv pip install --python ${VENV}/bin/python "decord==0.6.0" \ + || echo ">> decord skipped (no matching wheel for ${TARGETARCH:-}); ERNIE-VL video decode unavailable"; \ + fi # Audio decode out of the box: the TTS/STT notebooks feed datasets' Audio # features, which decode through torchcodec. Three traps, all defended: diff --git a/docker/Dockerfile.studio b/docker/Dockerfile.studio index 29ab702e8f..4b88c73ca2 100644 --- a/docker/Dockerfile.studio +++ b/docker/Dockerfile.studio @@ -220,12 +220,15 @@ RUN JS="$(/opt/unsloth-venv/bin/python -c 'import os, jupyter_server; print(os.p && cp /tmp/unsloth-branding/logo.png "${JS}/static/logo/logo.png" \ && cp /tmp/unsloth-branding/login.html "${JS}/templates/login.html" \ # Copy the curated Studio sloth stickers the login page rotates through into - # jupyter_server's static dir (sloth/NN.png). Fail-soft: if the Studio - # public folder ever moves, login.html's onerror falls back to the logo. - && /opt/unsloth-venv/bin/python /tmp/unsloth-branding/install_sloth_stickers.py \ + # jupyter_server's static dir (sloth/NN.png). Fail-soft -- but scoped to ONLY + # the sticker step via the { ...; } group so a failure in a REQUIRED branding + # step above (JS resolve, favicon/logo/login copy) still fails the build + # instead of being swallowed by this fallback. If the Studio public folder + # ever moves, login.html's onerror falls back to the logo. + && { /opt/unsloth-venv/bin/python /tmp/unsloth-branding/install_sloth_stickers.py \ --src "${UNSLOTH_STUDIO_HOME}/src/studio/frontend/public/Sloth emojis" \ --dest "${JS}/static/sloth" \ - || echo ">> sloth stickers not installed (login falls back to the Unsloth logo)" \ + || echo ">> sloth stickers not installed (login falls back to the Unsloth logo)"; } \ && rm -rf /tmp/unsloth-branding \ && /opt/unsloth-venv/bin/jupyter labextension disable @jupyterlab/application-extension:logo \ && /opt/unsloth-venv/bin/jupyter labextension lock @jupyterlab/application-extension:logo diff --git a/docker/jupyter/overrides.json b/docker/jupyter/overrides.json index 6ffc6704d8..4fe9590fdb 100644 --- a/docker/jupyter/overrides.json +++ b/docker/jupyter/overrides.json @@ -35,7 +35,6 @@ }, "@jupyterlab/apputils-extension:notification": { "fetchNews": "false", - "checkForUpdates": false, - "doNotDisturbMode": true + "checkForUpdates": false } } diff --git a/docker/jupyter/unsloth_labext/src/cellNav.ts b/docker/jupyter/unsloth_labext/src/cellNav.ts index 81eb9d840c..a2a84b204d 100644 --- a/docker/jupyter/unsloth_labext/src/cellNav.ts +++ b/docker/jupyter/unsloth_labext/src/cellNav.ts @@ -41,6 +41,19 @@ const cellNavPlugin: JupyterFrontEndPlugin = { if (!panel.node.contains(event.target as Node)) { return; } + // Never hijack arrows that belong to an interactive output (an ipywidgets + // slider / dropdown / text box created by a cell) or a plain form control; + // only the cell editor and the notebook's own command-mode cell nav. + const targetEl = event.target as HTMLElement | null; + if (targetEl) { + if (targetEl.closest('.jp-OutputArea')) { + return; + } + const tag = targetEl.tagName; + if (tag === 'INPUT' || tag === 'TEXTAREA' || tag === 'SELECT') { + return; + } + } const notebook = panel.content; const direction = event.key === 'ArrowDown' ? 1 : -1; const editing = notebook.mode === 'edit'; @@ -49,6 +62,16 @@ const cellNavPlugin: JupyterFrontEndPlugin = { if (!editor) { return; } + // While a completion / autocomplete popup is open, the arrows belong to + // it (moving through the suggestions) -- do not take over even at a cell + // boundary, which is common in one-line setup cells. + if ( + document.querySelector( + '.jp-Completer:not(.lm-mod-hidden), .cm-tooltip-autocomplete' + ) + ) { + return; + } const line = editor.getCursorPosition().line; // Only take over at the cell boundary; otherwise let CodeMirror move the // cursor within the editor as usual (do not preventDefault/stop). diff --git a/docker/studio_launch.sh b/docker/studio_launch.sh index 21a75a9a09..5ffffa1e4c 100644 --- a/docker/studio_launch.sh +++ b/docker/studio_launch.sh @@ -65,12 +65,16 @@ c.PasswordIdentityProvider.hashed_password = "${HASH}" EOF # Land straight in the categorized notebook view, but only when it is enabled # AND lives under root_dir (so it is expressible as a /lab/tree path). Mirror - # unsloth_sync_notebooks.sh's UNSLOTH_NOTEBOOKS_VIEW_DIR / UNSLOTH_SKIP_NOTEBOOK_VIEW - # so a relocated or disabled view never points JupyterLab at a missing dir; - # in those cases JupyterLab just opens on its default (/lab) over /workspace. + # unsloth_sync_notebooks.sh's gating -- UNSLOTH_NOTEBOOKS_VIEW_DIR plus both + # UNSLOTH_SKIP_NOTEBOOK_VIEW (no view built) and UNSLOTH_SKIP_NOTEBOOK_SYNC + # (entrypoint skips sync entirely, so nothing under the view dir exists) -- so + # a relocated, disabled, or unsynced view never points JupyterLab at a missing + # dir; in those cases JupyterLab just opens on its default (/lab) over /workspace. _root_dir="/workspace" _view_dir="${UNSLOTH_NOTEBOOKS_VIEW_DIR:-/workspace/Unsloth Notebooks}" - if [[ "${UNSLOTH_SKIP_NOTEBOOK_VIEW:-0}" != "1" && "${_view_dir}" == "${_root_dir}/"* ]]; then + if [[ "${UNSLOTH_SKIP_NOTEBOOK_VIEW:-0}" != "1" \ + && "${UNSLOTH_SKIP_NOTEBOOK_SYNC:-0}" != "1" \ + && "${_view_dir}" == "${_root_dir}/"* ]]; then _view_rel="${_view_dir#${_root_dir}/}" # default_url must be set on BOTH ServerApp and LabApp -- the lab # extension app otherwise overrides ServerApp's value back to /lab. diff --git a/docker/unsloth_nb_view.py b/docker/unsloth_nb_view.py index 9b8f35e3a3..6eb5655d5f 100644 --- a/docker/unsloth_nb_view.py +++ b/docker/unsloth_nb_view.py @@ -155,8 +155,14 @@ def build_view( target = os.path.join(nb_dir, fname) rel = os.path.relpath(target, folder) # ../../unsloth-notebooks/nb/ try: - if os.path.islink(link) or os.path.exists(link): - os.remove(link) + if os.path.islink(link): + os.remove(link) # replace our own stale symlink + elif os.path.exists(link): + # a real user file/dir already occupies this name -- never + # clobber it; leave it and skip linking this notebook. + print(f"[unsloth-nb] view: keep user file, skip link {fname}", + file = sys.stderr) + continue os.symlink(rel, link) n_links += 1 except OSError as e: