Skip to content

Studio: auto-recover when shadowed 'unsloth' on PATH hides the frontend dist#5782

Merged
danielhanchen merged 3 commits into
mainfrom
fix/studio-frontend-resolution
May 26, 2026
Merged

Studio: auto-recover when shadowed 'unsloth' on PATH hides the frontend dist#5782
danielhanchen merged 3 commits into
mainfrom
fix/studio-frontend-resolution

Conversation

@danielhanchen

Copy link
Copy Markdown
Member

Summary

Fixes a silent 404 on / when another unsloth binary on PATH shadows the installer's shim. The CLI derives _PACKAGE_ROOT from where unsloth_cli was imported, and studio/backend/run.py derives its default frontend_path as a sibling of __file__. When a non-installer unsloth wins on PATH (a workspace venv with pip install unsloth, a system install, etc.), both resolve into a site-packages tree that ships frontend source but no vite-built dist/. The backend logged [WARNING] Frontend not found at ... and then served 200 on every /api/* route while returning {"detail":"Not Found"} on /. The 404 was invisible to users since /api/health looked fine.

This patch adds runtime auto-discovery, a loud error when discovery fails, and an installer self-check that warns about PATH shadowing.

Repro before the fix

# Pre-existing venv with `unsloth` already on PATH (e.g. workspace tooling):
which unsloth
# /path/to/other-venv/bin/unsloth   <-- this wins

curl -fsSL https://unsloth.ai/install.sh | sh    # or `./install.sh --local`
# Installer succeeds. Auto-launch works (it runs its own binary).

# But a new shell hits the shadowing binary:
unsloth studio -H 0.0.0.0 -p 8888
curl -s http://127.0.0.1:8888/                   # {"detail":"Not Found"}
curl -s http://127.0.0.1:8888/api/health         # {"status":"healthy",...}

What changes

Layer C: runtime auto-discovery

unsloth_cli/commands/studio.py gains _find_frontend_dist() and _iter_editable_studio_source_roots(). Before spawning run.py, the CLI resolves --frontend explicitly by probing:

  1. _PACKAGE_ROOT/studio/frontend/dist
  2. $UNSLOTH_STUDIO_HOME/unsloth_studio/lib/python*/site-packages/studio/frontend/dist (POSIX)
  3. $UNSLOTH_STUDIO_HOME/unsloth_studio/Lib/site-packages/studio/frontend/dist (Windows)
  4. Editable source roots read from __editable___*_finder.py MAPPING dicts in the installer venv

studio/backend/run.py gains the same probe as a backstop for direct python run.py invocations.

Layer E: loud structured error

The silent [WARNING] becomes a SystemExit that lists every candidate tried and the four one-line fixes (absolute path, --frontend, --api-only, reinstall). Skipped in --api-only mode where no UI is served.

Layer F: installer self-check

install.sh and install.ps1 compare command -v unsloth / Get-Command unsloth against the just-installed binary at the end of install. If a different path wins, both print a yellow warning block naming the shadowing binary plus the alias / absolute-path / PATH-reorder fixes. install.sh uses the freshly-created venv Python for path canonicalization (BSD readlink on macOS has no -f, so the previous attempt with readlink -f would have degraded on Mac).

Cross-platform compat

  • Glob patterns probe both lib/python*/site-packages (POSIX) and Lib/site-packages (Windows).
  • Canonical-binary path branches on sys.platform == "win32" to pick unsloth.exe over unsloth.
  • install.sh covers macOS / Linux / WSL; install.ps1 is the Windows analog. Both use existing in-file step / substep helpers.

Tests

studio/backend/tests/test_frontend_resolution.py adds 5 tests via AST-loading the helpers (same style as test_host_defaults.py, no uvicorn / FastAPI imports needed):

  1. Resolver returns None when nothing exists anywhere.
  2. Resolver picks the first existing candidate when the default works.
  3. Fallback to $UNSLOTH_STUDIO_HOME site-packages dist when the default is missing.
  4. Fallback to an editable-install source root via MAPPING parsing.
  5. Resolver tolerates a non-existent $UNSLOTH_STUDIO_HOME.

All 5 new + 2 existing tests pass:

studio/backend/tests/test_frontend_resolution.py ........... [ 5 passed ]
studio/backend/tests/test_host_defaults.py ................. [ 2 passed ]

Test plan

  • pytest studio/backend/tests/test_frontend_resolution.py studio/backend/tests/test_host_defaults.py passes (7 tests).
  • pre-commit run --files install.sh install.ps1 studio/backend/run.py unsloth_cli/commands/studio.py studio/backend/tests/test_frontend_resolution.py passes.
  • End-to-end reproduced the silent-404 bug on a real box, applied the patch, confirmed curl -s http://127.0.0.1:PORT/ now returns 200 with the real index.html via the shadowed binary -- no PATH or alias change required.
  • [OK] Frontend loaded from /path/to/editable/clone/studio/frontend/dist appears in the log, confirming the editable-install fallback path fires correctly.
  • Smoke test on macOS (Apple Silicon) installer path.
  • Smoke test on Windows installer.

Files touched

install.ps1                                          | +23 lines
install.sh                                           | +28 lines
studio/backend/run.py                                | +112 / -8
unsloth_cli/commands/studio.py                       |  +76 / -2
studio/backend/tests/test_frontend_resolution.py     | new (+116)

…nd dist

The CLI launcher derives `_PACKAGE_ROOT` from where `unsloth_cli` imports
from, and `studio/backend/run.py` derives its default `frontend_path` from
`Path(__file__).resolve().parent.parent / "frontend" / "dist"`. When
another `unsloth` (a separate venv with `pip install unsloth`, a system
install, an older venv earlier on PATH) wins `which unsloth`, both
resolve into a site-packages tree that ships frontend source files but no
vite-built `dist/`. The backend warned `[WARNING] Frontend not found at
...` and then happily served 200 on every `/api/*` route while returning
`{"detail":"Not Found"}` on `/`. The 404 was silent to users -- the
process was healthy, the log line scrolled by, and the only symptom was a
blank browser tab.

This is a real situation: many devboxes carry a workspace venv with
`unsloth` installed years before the user runs `curl|sh` to install
Studio. The installer-managed binary at `~/.local/bin/unsloth` exists
but loses to the older venv on PATH order.

Three layers of fix, additive:

Layer C -- runtime auto-discovery (unsloth_cli + run.py)
The CLI now resolves `--frontend` explicitly before spawning `run.py`,
probing in order: package-local default, installer venv site-packages
(`$STUDIO_HOME/unsloth_studio/lib/python*/site-packages/...` and the
Windows `Lib/site-packages/...` equivalent), and editable-install source
roots read from `__editable___*_finder.py` MAPPING dicts in the installer
venv. `run.py` does the same probe as a backstop for direct `python
run.py` invocations.

Layer E -- loud structured error
The silent `[WARNING]` is replaced with a `SystemExit` that names every
candidate path tried and lists the four one-line fixes (run the absolute
path, pass `--frontend`, pass `--api-only`, reinstall). Suppressed only
in `--api-only` mode where no UI is served by design.

Layer F -- installer self-check (install.sh + install.ps1)
At the tail of install, both installers compare `command -v unsloth`
(POSIX) / `Get-Command unsloth` (PowerShell) against the just-installed
binary. If a different path wins, a yellow `warning` block names the
shadowing binary and prints the alias / absolute-path / PATH-reorder
fixes. install.sh uses the venv Python for path canonicalization so it
also works on macOS (BSD `readlink` has no `-f`).

Cross-platform notes:
- Glob patterns probe both `lib/python*/site-packages` (POSIX) and
  `Lib/site-packages` (Windows).
- Canonical-binary path branches on `sys.platform == "win32"` to pick
  `unsloth.exe` over `unsloth`.
- install.sh fixed for macOS; install.ps1 is the Windows analog.

Tests: `studio/backend/tests/test_frontend_resolution.py` covers five
cases via AST-load of the helpers (no uvicorn / FastAPI import needed,
matching `test_host_defaults.py`'s style):
1. Resolver returns None when nothing exists anywhere.
2. Resolver picks the first existing candidate when the default works.
3. Fallback to `$UNSLOTH_STUDIO_HOME` site-packages dist when the default
   is missing.
4. Fallback to an editable-install source root via MAPPING parsing.
5. Resolver tolerates a non-existent `$UNSLOTH_STUDIO_HOME`.

All 5 new + 2 existing host-default tests pass.
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces fallback resolution logic for locating the built frontend dist directory in both the backend (run.py) and CLI (studio.py). This addresses issues where another unsloth binary on the PATH shadows the installer's binary, leading to missing frontend assets or silent 404 errors. It also adds warnings during installation (install.sh and install.ps1) when a PATH conflict is detected. The review feedback points out a potential crash in both run.py and studio.py if ast.literal_eval parses a non-dictionary literal, and suggests adding explicit type checks (isinstance(mapping, dict)) before calling .get().

Comment thread studio/backend/run.py
Comment on lines +553 to +559
try:
mapping = ast.literal_eval(m.group(1))
except (SyntaxError, ValueError):
continue
studio_pkg = mapping.get("studio")
if studio_pkg:
out.append(Path(studio_pkg) / "frontend" / "dist")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

If ast.literal_eval parses a literal that is not a dictionary (such as a set, list, or None), calling .get() on it will raise an AttributeError and crash the backend startup. We should explicitly verify that mapping is a dictionary before attempting to access its keys.

Suggested change
try:
mapping = ast.literal_eval(m.group(1))
except (SyntaxError, ValueError):
continue
studio_pkg = mapping.get("studio")
if studio_pkg:
out.append(Path(studio_pkg) / "frontend" / "dist")
try:
mapping = ast.literal_eval(m.group(1))
except (SyntaxError, ValueError):
continue
if isinstance(mapping, dict):
studio_pkg = mapping.get("studio")
if studio_pkg:
out.append(Path(studio_pkg) / "frontend" / "dist")

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 499e7d8. Both call sites (studio/backend/run.py:558 and unsloth_cli/commands/studio.py:234) now guard with isinstance(mapping, dict) and continue on a non-dict literal, so the resolver keeps probing the remaining finder files. Setuptools's template only emits dict literals today, so this is defensive against a future change rather than a live bug, but the guard is one line and prevents the crash mode. Added a regression test (test_resolver_does_not_crash_on_non_dict_mapping_literal) that writes a bad finder (MAPPING is a set literal) alongside a good one and asserts the resolver returns the good finder's dist path. Without the guard the test crashes with AttributeError; with it, 11/11 tests pass.

Comment on lines +229 to +235
try:
mapping = ast.literal_eval(m.group(1))
except (SyntaxError, ValueError):
continue
studio_pkg = mapping.get("studio")
if studio_pkg:
yield Path(studio_pkg).parent

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

If ast.literal_eval parses a literal that is not a dictionary (such as a set, list, or None), calling .get() on it will raise an AttributeError and crash the CLI. We should explicitly verify that mapping is a dictionary before attempting to access its keys.

Suggested change
try:
mapping = ast.literal_eval(m.group(1))
except (SyntaxError, ValueError):
continue
studio_pkg = mapping.get("studio")
if studio_pkg:
yield Path(studio_pkg).parent
try:
mapping = ast.literal_eval(m.group(1))
except (SyntaxError, ValueError):
continue
if isinstance(mapping, dict):
studio_pkg = mapping.get("studio")
if studio_pkg:
yield Path(studio_pkg).parent

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 499e7d8 (see reply on the run.py thread for the full rationale; same one-line guard, same regression test covers both call sites).

…h hint, broader tests)

Four parallel platform reviews (Windows, Linux, macOS, general) on the
initial commit surfaced a small batch of correctness items, all addressed
here:

Windows install.ps1 (medium severity, false positive on every install):
The user-facing shim at $StudioHome\bin\unsloth.exe is a hardlink to
$VenvDir\Scripts\unsloth.exe (created at line 1582). Resolve-Path does not
de-duplicate hardlinks, so the previous string compare always saw the two
paths as different and the new "another 'unsloth' wins on PATH" warning
would fire on every fresh Windows install. Switched to content-hash
equality via Get-FileHash, which collapses hardlinks, symlinks, and
identical copies to a single identity. Also restricted the probe to
Get-Command -CommandType Application so PowerShell aliases / functions /
scripts named "unsloth" don't false-trigger.

Windows run.py SystemExit hint (medium severity, defeats the recovery UX):
The structured error printed Path(STUDIO_HOME)/"unsloth_studio"/"bin"/
"unsloth.exe" on every platform, but on Windows the installer places the
shim at $STUDIO_HOME/bin/unsloth.exe (no unsloth_studio segment) and the
venv binary at $STUDIO_HOME/unsloth_studio/Scripts/unsloth.exe. The hint
pointed at a non-existent path on Windows. Branch on sys.platform ==
"win32" to emit the real shim location; Linux / macOS keep the unsloth_
studio/bin/unsloth layout.

MAPPING regex robustness (low):
[^\n]* silently failed if a future setuptools / black reformat wrapped
the MAPPING dict across multiple lines. Tightened to [^}]* + re.DOTALL,
which still rejects nested dicts (setuptools never emits those for
editable installs) but tolerates either single- or multi-line literals.

install.sh broken-venv edge case (low, macOS reviewer):
Previously _canon fell back to echoing the raw input when the venv python
failed, which would make two symlinked-but-identical paths look different
and false-trigger the warning. Now _canon returns empty on failure and
the caller skips the whole comparison if either side is unresolvable.

argparse default + log readability (nits):
run.py's argparse --frontend default now reuses the module-level
_DEFAULT_FRONTEND_PATH constant so it stays in lockstep with run_server's
default. The [OK] log message resolves the chosen path so support output
is always absolute.

Tests grow from 5 to 8 in studio/backend/tests/test_frontend_resolution.
py (10/10 with the existing host-default tests):

- Windows-layout fallback: Lib/site-packages with capital L.
- Multi-line MAPPING dict: locks in the [^}]* + re.DOTALL behaviour.
- SystemExit message contract: every actionable fix string and the
  attempted-paths list must appear; pins the user-facing recovery
  message so a future refactor doesn't drop a bullet.

End-to-end re-verified on this box: shadowing workspace_22/bin/unsloth
still serves 200 on / through the editable-finder fallback, with the
follow-up resolve-then-log change yielding [OK] Frontend loaded from
/mnt/disks/unslothai/ubuntu/unsloth/studio/frontend/dist.

Out of scope (called out by reviewers but deferred):

- _resolve_frontend_path candidate ordering still tries _PACKAGE_ROOT
  first. For the rare case where a shadowing install carries an older
  built dist, this serves the stale UI instead of the fresh one. Fix is
  non-trivial (the --local workflow intentionally wants _PACKAGE_ROOT to
  win when the cloned repo is the source of truth), so leaving it for a
  follow-up.
- studio/backend/colab.py still bails out on missing frontend instead of
  routing through the new resolver. Pre-existing behaviour, separate PR.
- _resolve_frontend_path is duplicated across run.py and unsloth_cli/
  commands/studio.py. Minor maintenance concern; consolidation is
  natural in a later refactor.
@danielhanchen

Copy link
Copy Markdown
Member Author

Follow-up commit c87162eb addresses the items surfaced by the four parallel platform reviews.

What changed

# Severity Reviewer File Fix
1 medium Windows install.ps1 The user-facing shim is created as a hardlink to the venv binary (line 1582), so Resolve-Path saw two distinct paths and the warning false-fired on every standard Windows install. Switched to Get-FileHash -Algorithm SHA256 content-equality, which collapses hardlinks, symlinks, and identical copies into one identity.
2 medium Windows studio/backend/run.py The SystemExit "fix one of" hint pointed at $STUDIO_HOME/unsloth_studio/bin/unsloth.exe, which doesn't exist on Windows. Branch on sys.platform == "win32" to emit $STUDIO_HOME/bin/unsloth.exe (the actual shim) instead.
3 low Windows install.ps1 Get-Command unsloth also matched aliases / functions / scripts. Restricted to -CommandType Application.
4 low Linux + General both Python files [^\n]* silently failed on a multi-line MAPPING dict. Tightened to [^}]* + re.DOTALL (still rejects nested dicts, which setuptools never emits).
5 low macOS install.sh If the venv python failed to canonicalize, _canon previously fell back to echoing the raw path, which could false-trigger for symlinked-but-identical paths. Now returns empty on failure and the caller skips the whole check.
6 nit General studio/backend/run.py argparse --frontend default reuses _DEFAULT_FRONTEND_PATH. [OK] Frontend loaded from ... .resolve()s the path for unambiguous support output.

Tests

Three new tests, 8 total in test_frontend_resolution.py (10/10 with existing host-default tests):

  • test_resolver_falls_back_to_windows_layout_site_packages pins the Lib/site-packages Windows venv layout.
  • test_resolver_handles_multiline_mapping_dict writes a multi-line setuptools-style MAPPING and asserts the resolver still finds the editable source.
  • test_systemexit_message_contains_actionable_fixes treats the recovery message as a contract: attempted paths plus all four bullet fixes (--frontend, --api-only, reinstall, absolute binary path) must appear.

Out of scope (called out, deferred)

  • _resolve_frontend_path still tries _PACKAGE_ROOT first. If a shadowing install carries an older built dist, that wins over the installer venv's fresh one. The --local workflow intentionally wants _PACKAGE_ROOT to win when the cloned repo is the source of truth, so a smarter selection needs more design.
  • studio/backend/colab.py still bails out on missing frontend instead of routing through the new resolver. Pre-existing behavior, separate PR.
  • _iter_frontend_fallback_candidates is duplicated across run.py and unsloth_cli/commands/studio.py. Minor maintenance concern; consolidation fits a later refactor.

End-to-end re-verified locally: shadowing workspace_22/bin/unsloth still serves 200 on / via the editable-finder fallback. Pre-commit + tests green.

Addresses gemini-code-assist[bot] high-priority inline review on PR 5782
flagging that `mapping.get('studio')` could raise AttributeError if the
MAPPING regex matched a brace-delimited literal that ast.literal_eval
parsed as a non-dict (set, list, None). The regex `\{[^}]*\}` happily
matches `{1, 2, 3}` and literal_eval returns a set; the previous code
then crashed on .get().

Setuptools's editable-install template only emits dict literals so this
is defensive rather than a live bug, but the guard is one line per call
site and prevents a future template change from taking out backend
startup or CLI invocation.

Both call sites (studio/backend/run.py:558 and
unsloth_cli/commands/studio.py:234) now bail out on the finder file when
isinstance(mapping, dict) is False; the resolver keeps probing the
remaining finders, so a malformed entry in one finder cannot poison the
discovery of a good one elsewhere.

Adds test_resolver_does_not_crash_on_non_dict_mapping_literal to
test_frontend_resolution.py, which writes one bad finder (MAPPING is a
set literal) alongside one good finder (MAPPING is a real dict) and
asserts the resolver returns the good finder's dist path. Without the
guard this test crashes with AttributeError; with the guard it passes.

11/11 tests green.
@danielhanchen danielhanchen merged commit c49cc6d into main May 26, 2026
36 checks passed
@danielhanchen danielhanchen deleted the fix/studio-frontend-resolution branch May 26, 2026 12:29
rhsCZ pushed a commit to rhsCZ/unsloth that referenced this pull request May 27, 2026
Resolve studio.py conflict by keeping both:
- PR unslothai#5737's `_PARALLEL_*` constants block
- main's `_iter_editable_studio_source_roots` + `_find_frontend_dist`
  helpers (PR unslothai#5782).

run.py auto-merged cleanly. 215 PR tests still pass.
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.

1 participant