Take the loaded-models indicator suite off the UI job's critical path by danielhanchen · Pull Request #9060 · unslothai/unsloth · GitHub
Skip to content

Take the loaded-models indicator suite off the UI job's critical path - #9060

Merged
danielhanchen merged 16 commits into
mainfrom
ci-ui-smoke-timeout
Aug 17, 2026
Merged

Take the loaded-models indicator suite off the UI job's critical path#9060
danielhanchen merged 16 commits into
mainfrom
ci-ui-smoke-timeout

Conversation

@danielhanchen

Copy link
Copy Markdown
Member

Take the indicator suite off the UI job's critical path

Unsloth UI CI / Chat UI Tests has been timing out at its 30 minute limit on
almost every open PR: 5 of the 6 currently open, at 30m09s to 30m24s. Nothing
was failing. The job was simply longer than the limit, and a limit is a
guillotine, so the steps after the cut never ran at all.

Where it goes: "Cross-browser loaded-models indicator" is 853s of the 30
minutes, three browser engines at about 4.7 minutes each, each booting its own
Unsloth. Everything downstream of it -- the other-engines update banner
regression, both image staged-download regressions, the model-picker
per-model-config suite and the IME / multilingual paste regression -- was
reported "skipped" and has been running on no PR at all.

It is now its own job rather than a step, so it runs beside the rest instead of
inside their budget. That leaves ui-smoke around 16 minutes and the new job
around 15, both with real margin, and the five skipped suites run again. The new
job carries no HF cache and no GGUF: the suite stubs the four /status reads with
page.route, so it needs no model, no GPU and no llama.cpp build.

Moving the work rather than the limit is deliberate. The limit here already went
25 -> 30 for this exact symptom, with the comment on it still describing that
round, so raising it again buys one more release of silence.

Also tests/studio/test_playwright_suites_run_in_ci.py, because these drivers are
standalone scripts that nothing collects: a suite runs only because a workflow
step or a .github/scripts helper names it, and deleting that line leaves every
job green. It fails if any tests/studio/playwright_*.py is named by no workflow
and no CI script. Deleting either the new step or the driver call inside
run-studio-indicator-browser.sh fails it.

It also found two suites already in that state, exempted with a reason each
rather than quietly ignored: playwright_tauri_python_tool_images.py needs the
Tauri desktop shell no runner here builds, and playwright_train_pickers.py needs
a dataset and model resolved through huggingface_hub while the UI workflows boot
API-only. Both are worth wiring up, separately from this.

test_playwright_suites_run_in_ci.py: 3 passed, and 2 failed under each mutation
test_stt_model_search_locator_contract.py + test_composer_rtl_bidi_attribute.py: 34 passed

Unsloth UI CI / Chat UI Tests has been timing out at its 30 minute limit on
almost every open PR: 5 of the 6 currently open, at 30m09s to 30m24s. Nothing
was failing. The job was simply longer than the limit, and a limit is a
guillotine, so the steps after the cut never ran at all.

Where it goes: "Cross-browser loaded-models indicator" is 853s of the 30
minutes, three browser engines at about 4.7 minutes each, each booting its own
Unsloth. Everything downstream of it -- the other-engines update banner
regression, both image staged-download regressions, the model-picker
per-model-config suite and the IME / multilingual paste regression -- was
reported "skipped" and has been running on no PR at all.

It is now its own job rather than a step, so it runs beside the rest instead of
inside their budget. That leaves ui-smoke around 16 minutes and the new job
around 15, both with real margin, and the five skipped suites run again. The new
job carries no HF cache and no GGUF: the suite stubs the four /status reads with
page.route, so it needs no model, no GPU and no llama.cpp build.

Moving the work rather than the limit is deliberate. The limit here already went
25 -> 30 for this exact symptom, with the comment on it still describing that
round, so raising it again buys one more release of silence.

Also tests/studio/test_playwright_suites_run_in_ci.py, because these drivers are
standalone scripts that nothing collects: a suite runs only because a workflow
step or a .github/scripts helper names it, and deleting that line leaves every
job green. It fails if any tests/studio/playwright_*.py is named by no workflow
and no CI script. Deleting either the new step or the driver call inside
run-studio-indicator-browser.sh fails it.

It also found two suites already in that state, exempted with a reason each
rather than quietly ignored: playwright_tauri_python_tool_images.py needs the
Tauri desktop shell no runner here builds, and playwright_train_pickers.py needs
a dataset and model resolved through huggingface_hub while the UI workflows boot
API-only. Both are worth wiring up, separately from this.

  test_playwright_suites_run_in_ci.py:  3 passed, and 2 failed under each mutation
  test_stt_model_search_locator_contract.py + test_composer_rtl_bidi_attribute.py: 34 passed
chatgpt-codex-connector[bot]

This comment was marked as resolved.

Reported on this PR and correct: reading every file under .github/scripts counts
an orphaned helper as coverage, so a driver named only by a script nothing calls
reads as covered while it runs nowhere, which is the regression this test exists
to catch.

The workflows now seed the text and a helper joins only once something already in
it names the helper, repeated to a fixed point because one helper can call
another. .github/actions is included the same way, since a workflow reaches a
composite action by uses: and the action then calls the script.

Verified by making the reference unreachable in all three UI workflows while
leaving run-studio-indicator-browser.sh in the tree. The new version reports the
driver as running nowhere; the scan-everything version reports it covered. That
one helper is named by the Linux, Windows and Mac UI workflows, which is why
removing it from one is correctly still covered.

  3 passed
chatgpt-codex-connector[bot]

This comment was marked as resolved.

Reported on this PR and correct. Reading a workflow whole counts a driver named
in on.pull_request.paths as an invocation, and studio-frontend-ci.yml names
playwright_strip_ansi_smoke.py in both its trigger list and its step, so deleting
the step alone left this guard green while the suite ran nowhere. Trigger paths
say when CI runs, not what it runs.

The scan now parses each workflow and reads only the executable fields: a job's
uses, and each step's run, uses and with. Helper reachability is followed from
that text as before.

Verified by deleting only the run line in studio-frontend-ci.yml and leaving the
trigger path: the guard now reports playwright_strip_ansi_smoke.py as running
nowhere, where before it stayed green.

  3 passed
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Aug 17, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Aug 17, 2026
chatgpt-codex-connector[bot]

This comment was marked as resolved.

Reported on this PR and correct. A composite action is used as
`./.github/actions/<name>`, never as the action.yml inside it, so the
reachability walk never opened one and a driver launched from inside an action
read as an orphan. An action.yml now also matches on its containing directory.

Pinned with a line from inside install-unsloth-local's own body rather than
something a workflow could also contain, so the pin fails if the walk matches the
uses: reference without opening the action. Dropping the directory match fails it.

  3 passed
chatgpt-codex-connector[bot]

This comment was marked as resolved.

Reported on this PR and correct: the guard could not catch the regression it
exists for. It ran only in studio-backend-ci.yml's Repo tests job, whose paths do
not include .github/workflows/**, .github/scripts/** or .github/actions/**, so a
PR that only deletes a Playwright invocation never collected it and stayed green
until the post-merge run on main.

It now runs in workflow-trigger-lint.yml instead, which carries no paths filter
at all and says so in its own header comment, deliberately, for the same reason:
a gate that only runs for some PRs does not gate workflow changes. Cost is the
same shape as the lint beside it, filesystem reads in seconds, and that job
already installs PyYAML, which is all this needs besides pytest.

  3 passed, run bare from the repo root as that job runs it
  the workflow-trigger lint itself still passes: 42 workflow files scanned
chatgpt-codex-connector[bot]

This comment was marked as resolved.

Reported on this PR and correct: the repo-wide check cannot see this job
disappear. run-studio-indicator-browser.sh is named by the Mac and Windows UI
workflows too, so deleting all three calls from the Linux workflow leaves every
guard green while the Chromium/Firefox/WebKit coverage this job exists for is
gone.

Asserted against the ui-indicator job itself rather than the file, so a step
moved back into ui-smoke fails it too: that is the 30-minute limit this change
moved the work out of.

  4 passed, and deleting the three calls fails the new test by naming all three
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Aug 17, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Aug 17, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Aug 17, 2026
chatgpt-codex-connector[bot]

This comment was marked as resolved.

danielhanchen and others added 3 commits August 17, 2026 05:46
Reported on this PR and correct, with a real instance: substring presence is not
coverage. .github/scripts/kaggle_studio_ci/report.py names playwright_chat_ui.py
inside a result description, and the workflows name drivers in comments and in
trigger paths, so deleting every real invocation could leave this guard green on
prose alone.

Every driver and helper in this repo is run the same way, as an argument to an
interpreter, so that is what is matched now: the name at the end of a path token
being handed to python, python3, node, bash or sh. Composite actions keep the
directory match, since a workflow reaches one through uses: rather than a command.

Verified by removing every real invocation of playwright_chat_ui.py while leaving
report.py's description in place. The substring is still present and the guard now
reports the driver as running nowhere; before, it stayed green.
chatgpt-codex-connector[bot]

This comment was marked as resolved.

Reported on this PR and correct: commenting a command out is how one gets
disabled, and the scan reads workflow run: bodies and helper scripts verbatim, so
`# python tests/studio/x.py` matched the invocation shape through the whitespace
before python.

Comments are stripped before matching. Shell, YAML and Python all take # to end of
line, and a # inside a string only ever appears in prose here, which is not an
invocation either way.

Verified by commenting out every real invocation of playwright_chat_ui.py: the
guard now reports it as running nowhere, where before it stayed green.

  4 passed
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Aug 17, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Aug 17, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

Comments and docstrings only. The code is byte-identical once docstrings are
stripped, checked by comparing the parsed trees, and the tests still pass.
chatgpt-codex-connector[bot]

This comment was marked as resolved.

The cross-engine check tested raw step bodies for the substring, so prefixing
all three invocations with # left it green while the job ran no browser suite,
which is the regression it exists to prevent. It uses the same _uncommented()
helper the repo-wide scan does, and a pin asserts the disabled form of one of
those lines does not read as coverage.
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Aug 17, 2026
@danielhanchen

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

@danielhanchen
danielhanchen merged commit d0e828f into main Aug 17, 2026
22 checks passed
@danielhanchen
danielhanchen deleted the ci-ui-smoke-timeout branch August 17, 2026 07:50
danielhanchen added a commit that referenced this pull request Aug 17, 2026
…#9060)

* Take the indicator suite off the UI job's critical path

Unsloth UI CI / Chat UI Tests has been timing out at its 30 minute limit on
almost every open PR: 5 of the 6 currently open, at 30m09s to 30m24s. Nothing
was failing. The job was simply longer than the limit, and a limit is a
guillotine, so the steps after the cut never ran at all.

Where it goes: "Cross-browser loaded-models indicator" is 853s of the 30
minutes, three browser engines at about 4.7 minutes each, each booting its own
Unsloth. Everything downstream of it -- the other-engines update banner
regression, both image staged-download regressions, the model-picker
per-model-config suite and the IME / multilingual paste regression -- was
reported "skipped" and has been running on no PR at all.

It is now its own job rather than a step, so it runs beside the rest instead of
inside their budget. That leaves ui-smoke around 16 minutes and the new job
around 15, both with real margin, and the five skipped suites run again. The new
job carries no HF cache and no GGUF: the suite stubs the four /status reads with
page.route, so it needs no model, no GPU and no llama.cpp build.

Moving the work rather than the limit is deliberate. The limit here already went
25 -> 30 for this exact symptom, with the comment on it still describing that
round, so raising it again buys one more release of silence.

Also tests/studio/test_playwright_suites_run_in_ci.py, because these drivers are
standalone scripts that nothing collects: a suite runs only because a workflow
step or a .github/scripts helper names it, and deleting that line leaves every
job green. It fails if any tests/studio/playwright_*.py is named by no workflow
and no CI script. Deleting either the new step or the driver call inside
run-studio-indicator-browser.sh fails it.

It also found two suites already in that state, exempted with a reason each
rather than quietly ignored: playwright_tauri_python_tool_images.py needs the
Tauri desktop shell no runner here builds, and playwright_train_pickers.py needs
a dataset and model resolved through huggingface_hub while the UI workflows boot
API-only. Both are worth wiring up, separately from this.

  test_playwright_suites_run_in_ci.py:  3 passed, and 2 failed under each mutation
  test_stt_model_search_locator_contract.py + test_composer_rtl_bidi_attribute.py: 34 passed

* Count only helper scripts a workflow can actually reach

Reported on this PR and correct: reading every file under .github/scripts counts
an orphaned helper as coverage, so a driver named only by a script nothing calls
reads as covered while it runs nowhere, which is the regression this test exists
to catch.

The workflows now seed the text and a helper joins only once something already in
it names the helper, repeated to a fixed point because one helper can call
another. .github/actions is included the same way, since a workflow reaches a
composite action by uses: and the action then calls the script.

Verified by making the reference unreachable in all three UI workflows while
leaving run-studio-indicator-browser.sh in the tree. The new version reports the
driver as running nowhere; the scan-everything version reports it covered. That
one helper is named by the Linux, Windows and Mac UI workflows, which is why
removing it from one is correctly still covered.

  3 passed

* Count only the workflow fields that run something

Reported on this PR and correct. Reading a workflow whole counts a driver named
in on.pull_request.paths as an invocation, and studio-frontend-ci.yml names
playwright_strip_ansi_smoke.py in both its trigger list and its step, so deleting
the step alone left this guard green while the suite ran nowhere. Trigger paths
say when CI runs, not what it runs.

The scan now parses each workflow and reads only the executable fields: a job's
uses, and each step's run, uses and with. Helper reachability is followed from
that text as before.

Verified by deleting only the run line in studio-frontend-ci.yml and leaving the
trigger path: the guard now reports playwright_strip_ansi_smoke.py as running
nowhere, where before it stayed green.

  3 passed

* Follow a composite action by the directory workflows reference it with

Reported on this PR and correct. A composite action is used as
`./.github/actions/<name>`, never as the action.yml inside it, so the
reachability walk never opened one and a driver launched from inside an action
read as an orphan. An action.yml now also matches on its containing directory.

Pinned with a line from inside install-unsloth-local's own body rather than
something a workflow could also contain, so the pin fails if the walk matches the
uses: reference without opening the action. Dropping the directory match fails it.

  3 passed

* Run the coverage guard from the workflow that has no paths filter

Reported on this PR and correct: the guard could not catch the regression it
exists for. It ran only in studio-backend-ci.yml's Repo tests job, whose paths do
not include .github/workflows/**, .github/scripts/** or .github/actions/**, so a
PR that only deletes a Playwright invocation never collected it and stayed green
until the post-merge run on main.

It now runs in workflow-trigger-lint.yml instead, which carries no paths filter
at all and says so in its own header comment, deliberately, for the same reason:
a gate that only runs for some PRs does not gate workflow changes. Cost is the
same shape as the lint beside it, filesystem reads in seconds, and that job
already installs PyYAML, which is all this needs besides pytest.

  3 passed, run bare from the repo root as that job runs it
  the workflow-trigger lint itself still passes: 42 workflow files scanned

* Pin the three browser engines to the job that runs them

Reported on this PR and correct: the repo-wide check cannot see this job
disappear. run-studio-indicator-browser.sh is named by the Mac and Windows UI
workflows too, so deleting all three calls from the Linux workflow leaves every
guard green while the Chromium/Firefox/WebKit coverage this job exists for is
gone.

Asserted against the ui-indicator job itself rather than the file, so a step
moved back into ui-smoke fails it too: that is the 30-minute limit this change
moved the work out of.

  4 passed, and deleting the three calls fails the new test by naming all three

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Count a driver as covered only where CI actually runs it

Reported on this PR and correct, with a real instance: substring presence is not
coverage. .github/scripts/kaggle_studio_ci/report.py names playwright_chat_ui.py
inside a result description, and the workflows name drivers in comments and in
trigger paths, so deleting every real invocation could leave this guard green on
prose alone.

Every driver and helper in this repo is run the same way, as an argument to an
interpreter, so that is what is matched now: the name at the end of a path token
being handed to python, python3, node, bash or sh. Composite actions keep the
directory match, since a workflow reaches one through uses: rather than a command.

Verified by removing every real invocation of playwright_chat_ui.py while leaving
report.py's description in place. The substring is still present and the guard now
reports the driver as running nowhere; before, it stayed green.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Stop a commented-out invocation counting as coverage

Reported on this PR and correct: commenting a command out is how one gets
disabled, and the scan reads workflow run: bodies and helper scripts verbatim, so
`# python tests/studio/x.py` matched the invocation shape through the whitespace
before python.

Comments are stripped before matching. Shell, YAML and Python all take # to end of
line, and a # inside a string only ever appears in prose here, which is not an
invocation either way.

Verified by commenting out every real invocation of playwright_chat_ui.py: the
guard now reports it as running nowhere, where before it stayed green.

  4 passed

* Tighten the comments in the coverage guard

Comments and docstrings only. The code is byte-identical once docstrings are
stripped, checked by comparing the parsed trees, and the tests still pass.

* Read the browser-engine guard through the comment stripper

The cross-engine check tested raw step bodies for the substring, so prefixing
all three invocations with # left it green while the job ran no browser suite,
which is the regression it exists to prevent. It uses the same _uncommented()
helper the repo-wide scan does, and a pin asserts the disabled form of one of
those lines does not read as coverage.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
danielhanchen added a commit that referenced this pull request Aug 17, 2026
…#9027)

* Stub unsloth in test_safetensors_reasoning_stream, and close the guard hole

Three tests in this file fail on main if the file runs first, or on its own,
against the dependency set Backend CI installs:

  ImportError: Unsloth: Please install unsloth_zoo via `pip install unsloth_zoo`

Which PR, and which side is wrong:

  #5620 (2026-07-06) added this file, with no stub, from the start
  #8342 (2026-08-10) added the stub guard, which does not look here

Neither is wrong about its own subject, and neither is reverted. #5620's tests
are correct; they just relied on another file having stubbed unsloth first.
#8342's guard exempts imports inside a function on purpose, because those are
lazy and cannot fail COLLECTION, which is the disaster it was written for.

The hole is that pytest.importorskip USED to make a lazy import safe and no
longer does. Since pytest 8.2 it skips on ModuleNotFoundError only, and
unsloth/_gpu_init.py raises a plain ImportError when unsloth_zoo is missing, so
the call fails instead of skipping. #8342's exemption was sound when written and
stopped being sound under a pytest it did not choose.

Fix: install the same stub the sibling files install, at module scope. Stubbing
rather than skipif keeps the coverage: the module under test is still the real
core.inference.inference, and only unsloth is faked. A skipif would have made
three tests silently stop running on the backend job.

Robustification: a second guard beside #8342's, for modules reached through
pytest.importorskip at any scope, with its own reason stated rather than folded
into the collection guard. Verified against main's version of this file:

  1 test module(s) reach a backend module that needs unsloth through
  pytest.importorskip without installing the stub ... ['test_safetensors_reasoning_stream.py']

It is pinned in both directions (an unstubbed sample is an offence, a stubbed one
and an importorskip of numpy are not) so it cannot pass by matching nothing.

I also swept the other five files that importorskip a core.* or unsloth* module
without stubbing. Under the no-unsloth_zoo condition all five pass, so this file
was the only live case:

  test_active_generations            84 passed
  test_bypass_permissions           117 passed
  test_diffusion_dataset_clips       22 passed
  test_mlx_inference_backend         80 passed, 10 skipped
  test_offline_gguf_cache_fallback  202 passed
  test_safetensors_reasoning_stream   3 FAILED, 10 passed

Verified by hiding unsloth_zoo's metadata to reproduce the CI shape:

  before: 3 failed, 10 passed
  after:  13 passed

* Drop the stubs once the module is built, so they do not outlive it

The first version of this fix installed the unsloth stub and left it in
sys.modules for the rest of the process. Staging CI caught it on all four
interpreters:

  tests/test_audio_type_inconclusive.py:159: in test_the_stubs_do_not_outlive_this_module
      assert name not in sys.modules, name
  E   AssertionError: unsloth

That test exists precisely to forbid what I did. Every other file installs its
stubs, performs its heavy import, then pops them, and a stub left behind is a
cross-file leak: every other _stub_if_missing returns early when the name is
already present, so its own bookkeeping never runs and its cleanup has nothing
to undo.

Traded this file's order dependency for a worse one, in other words.

Now it follows the same shape as test_audio_type_inconclusive.py: stub, build
core.inference.inference while the stubs are live, drop the stubs. The three
tests reach the module through pytest.importorskip and get it from sys.modules,
so the stubs only need to exist for that one import.

It did not reproduce locally, which is why it reached staging: run the file
alone and the victim's own _STUBBED list is empty, so its loop asserts nothing
and passes vacuously. It takes xdist interleaving the two files on one worker
to put the leak in front of that assertion.

* Close two holes in the importorskip guard

Both reported on the guard added by this PR, both confirmed by construction
against the guard's own helpers.

1. The bare-name form was invisible. "from pytest import importorskip" then a
   bare importorskip("core.inference.inference") is a call on an ast.Name, not
   an attribute, so matching only pytest.importorskip returned no targets:

     bare-name form detected as target: []

   Both call shapes are matched now.

2. A module-scope call was judged by the wrong boundary. The check scanned the
   whole module for a stub, so a file that calls importorskip at module scope
   and installs its stub BELOW that line read as safe:

     late-stub form judged stubbed (scanning whole module): True

   while the call runs during collection and the import has already raised. The
   boundary is now the call's own line at module scope, and the end of the
   module only inside a def, where the call really does run after the body.

The pinning test covers all four shapes and both boundaries, so neither hole
can reopen silently. The guard still names the file this PR fixes when run
against main's version of it.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Judge import-time calls and dropped stubs correctly in the guard

Two more gaps on the guard this PR adds, both verified against its own helpers
before changing anything.

1. Definition-time calls were given the wrong boundary. _runtime_nodes stops at
   every def and class, so a call in a class body, a decorator, an annotation or
   a default expression was treated as lazy and got the end-of-module boundary:

     class body, stub below    calls=[('core.inference.inference', 5)] judged_safe=True
     default arg, stub below   calls=[('core.inference.inference', 5)] judged_safe=True

   All of those run while the module is being imported. Only a function BODY is
   deferred, so the scope walk now keeps class bodies and definition-time
   expressions and excludes just the body.

   No file in this tree does this today; all five class-level importorskip calls
   are inside methods, which really are lazy. Fixed anyway, because a guard that
   is wrong about when Python runs things is not much of a guard.

2. A lazy call was judged only on whether a stub was ever installed, not on
   whether it was still there. A module that installs the stubs, drops them
   again, and then reaches the heavy module lazily passes that check and still
   raises at test time. This PR's own file is safe only because it imports the
   target eagerly while the stubs are live, and nothing made that a requirement,
   so a copy that omitted the eager import would have read as safe.

   Now: a lazy call is an offence when the module drops its stubs and does not
   import the target itself. A module that keeps its stubs installed is fine
   lazily, which is why the removal is part of the condition rather than the
   eager import alone.

The decision is one function used by both the guard and the test that pins it.
My first version of this test reimplemented the logic instead, and a mutation
that deleted the real rule left it green.

  scope walk narrowed back to _runtime_nodes  -> 1 failed
  eager-import rule deleted                   -> 1 failed
  stub-removal condition dropped              -> 1 failed

* Judge a dropped stub at import time too, not only for lazy calls

Reported on this PR and correct. The guard asked "were the stubs dropped again,
and was the target never imported eagerly" only in the lazy branch, so a module
that installs the stub, pops it, and then calls importorskip at module scope
satisfied the install check and read as safe, while collection still dies: the
stub is gone by the time the call runs and nothing put the target in sys.modules.

Both questions now take the same boundary as the install check. _drops_stubs and
_eagerly_imports stop at that line, so an import-time call is judged on the pops
and imports ABOVE it and a lazy call on the whole module body, which is what the
old lazy-only branch computed. Three cases pinned: stub-pop-call is an offence,
stub-call-pop is not, and stub-import-pop-call is not.

Mutating the branch back to the lazy-only form fails the new pin.
6 passed.

* Four precision fixes to the importorskip guard

All four reported on this PR, all four real, each pinned and each verified by
reverting the fix and watching the pin fail.

1. An eager import only counts if the stubs were live for it. A file that probes
   the target in try/except ImportError before installing the stubs, then stubs,
   pops, and calls importorskip lazily, was read as safe. On the dependency-light
   matrix that probe fails and Python removes the half-initialised module, so the
   later call still reaches the real dependency. The check now asks _stubs_before
   for the statement's own line, so there is one answer to "are the stubs live"
   in this file rather than two that can drift.

2. A drop has to be OUR stubs being dropped. Any x.modules.pop counted, so an
   unrelated sys.modules.pop("routes.foo", None) in a properly stubbed file got
   that file reported as an offender, and a pop on any object with a .modules
   counted as well. The receiver must now be sys.modules and the statement must
   name the required stub the same way an install does, directly or through a
   module-level list it reads, which is how the real files spell it.

3. importorskip(modname = "...") is the documented signature
   (modname, minversion=None, reason=None, *, exc_type=None), and node.args is
   empty for it, so a file written that way walked straight past the guard. The
   keyword is read now.

4. exc_type=ImportError is pytest's own opt-in to skipping on a plain
   ImportError, which is the exact failure this guard is about, so such a call is
   safe unstubbed and flagging it was a false report. Those calls are excluded.
   exc_type=ModuleNotFoundError is the default and stays flagged. Confirmed
   against the pytest reference: exc_type arrived in 8.2, "must be ImportError or
   a subclass", and defaults to ModuleNotFoundError.

  6 passed, and reverting each of the four in turn: 1 failed each time
  test_safetensors_reasoning_stream.py + the guard together: 19 passed

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Install peft on the backend job, and stop misreading unrelated cleanup as a stub drop

Two items from this round, both reported here.

1. The eager import in test_safetensors_reasoning_stream.py was failing on the
   backend matrix and the handler was swallowing it, so the three importorskip
   tests SKIPPED rather than ran: 10 passed and 3 skipped, green either way.
   core/inference/inference.py imports peft at module scope, and peft is listed
   only in extras-no-deps.txt and no-torch-runtime.txt, neither of which this job
   installs. Confirmed both ways: the install log for a real matrix leg lists
   torchao and transformers and no peft, and hiding peft behind a meta_path
   finder locally reproduces exactly 10 passed and 3 skipped.

   Fixed by installing it rather than stubbing it. A stub does not work here:
   transformers probes importlib.util.find_spec("peft") during its own import,
   which raises ValueError on a stub whose __spec__ is None, so stubbing turns a
   silent skip into a collection error. Installing it also means these tests
   exercise the real import chain, which is the coverage the file is for.

   The swallow now records the error and a new test reads it back, so a
   module-scope dependency added to inference.py that this job does not install
   fails by name instead of quietly removing three tests.

2. _drops_stubs accumulated every module-level assignment target, so an unrelated
   cleanup list popped from sys.modules read as the unsloth stubs being dropped
   and got properly stubbed files reported as offenders. It now follows the link:
   a module-scope call naming the required stub identifies the helper, and
   whatever module-level name that helper appends to is the record. Two cases
   pinned, and the pin for the real shape now spells out the helper body, because
   that is what the link is followed through.

  test_safetensors_reasoning_stream.py + the guard: 20 passed
  with peft hidden: the new test FAILS by name instead of 3 silent skips
  restoring the old accumulation: 1 failed

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Install peft after the CPU torch wheel, not before it

Reported on this PR and correct, and this was my mistake in the previous commit.
peft declares `torch >=1.13.0` with no index constraint (checked against its
wheel metadata), so placed above the CPU-index line it resolves torch from PyPI,
which on Linux is the CUDA build. That version satisfies the >=2.4,<2.11 the next
line asks for, so pip leaves it, and a nominally CPU-only job ends up carrying
the CUDA distribution and its nvidia-* dependencies on all four legs: minutes of
download against a limit this job was already cancelling at, and runner disk.

Moved below the torch and transformers lines, where the pinned CPU torch already
satisfies the dependency and nothing re-resolves.

* Judge eager imports and helper calls by what actually runs at import

Three items from this round, all reported here, all real.

1. An eager import under `if TYPE_CHECKING:` or `if False:` never runs, so it is
   not what left the target in sys.modules, and a file that stubs, "imports"
   there, then drops the stubs still raises. The walk pruned nothing and read it
   as safe. It now skips a branch whose test is a constant the interpreter will
   not take, or TYPE_CHECKING, and nothing beyond that is guessed at.

2. The opposite error in the same walk: a class body, a decorator, a default and
   an annotation all execute while the module is being imported, so an eager
   import written in one of them DOES cache the target. _runtime_nodes stopped at
   every def and class, so such a file was reported as an offender when it is
   safe. That is what _import_time_nodes was already built for, so the eager
   check uses it now, with the reachability pruning above on top.

3. A def is only deferred while nothing runs it. Where the MODULE BODY calls a
   helper, that helper executes during collection, so an importorskip inside it
   runs then too, and the end-of-module boundary let a stub installed BELOW the
   call site read as being in place. Those calls now take the line they are
   invoked from. Followed one level, which is the shape that occurs; a helper
   reached only through another helper keeps the deferred boundary rather than
   being guessed at.

Four cases pinned: TYPE_CHECKING import is an offence, a class-body import is
not, a helper called at import time takes the call site's line, and the same
helper never called keeps the end-of-module boundary. Reverting each of the three
fixes fails a pin.

  6 passed, and 20 passed with test_safetensors_reasoning_stream.py

* Resolve importorskip aliases, and prune the else of a constant-true test

Both reported on this PR, both real, both pinned and mutation-checked.

1. `from pytest import importorskip as ios` then `ios(...)` is a valid call, and
   matching the callee against the literal string missed it, so an unstubbed
   module written that way walked past the guard. Bare names bound to it are
   collected now, through the import and through a plain rebinding of the
   attribute form. `from mymod import importorskip` is not one of them and is
   pinned as such.

2. The other half of the unreachable-branch hole I closed last round: pruning the
   body of `if False:` while still descending into the `else:` of `if True:` left
   an import that never runs counting as the one that cached the target. The
   traversal now takes only the branch the interpreter takes, for either constant.
   Both directions pinned.

  6 passed, and reverting each fix fails its pin

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Follow import-time helper calls all the way down

Reported on this PR and correct, and my "conservative" one-level stop was
conservative in the wrong direction. Stopping hands the inner helper the
end-of-module boundary, which is the LENIENT answer: a stub installed anywhere in
the file then reads as in time, while collection has already run the inner import
and failed.

Reachability now propagates through the module-level call graph to a fixed point,
and a helper reached only through another helper inherits the outer call site's
line, which is when it actually runs.

Two cases pinned: the module calls the outer helper and the inner one holds the
importorskip, which is an offence at the outer call's line; and the same chain
with nothing calling it at import time, which keeps the deferred boundary.
Removing the closure fails the first.

  6 passed

* Propagate only through the calls the helper actually makes

Reported on this PR and correct, and this one fails a SAFE file rather than
passing an unsafe one. ast.walk visits a call under `if False:` and a call inside
a nested def that nothing invokes, and handing those the outer helper's
import-time boundary judged an importorskip as running before a later stub when
Python never executes that path during collection.

It now walks the helper's body under the same reachability rule the module body
uses, so the two cannot answer differently.

Two cases pinned, both safe: an inner call under a constant-false test, and one
inside a nested def that is returned rather than called. Reverting to ast.walk
fails them.

  6 passed

* Do not report a call the interpreter never reaches

Reported on this PR and correct, and it is the same false-positive direction as
the last one: an importorskip under `if False:` or `if TYPE_CHECKING:` was
collected as a module-scope call, so the guard failed a file that only
type-checks the import or deliberately disables it. The helper seeding had the
same gap for a helper invoked only inside such a branch.

Both traversals are reachability-aware now. The call walk prunes constant
branches wherever they appear, inside a function body as well as at module scope,
since a call under `if False:` never runs there either.

Three cases pinned, all safe: a TYPE_CHECKING call, an `if False:` call, and a
helper called only from a disabled branch. Reverting either traversal fails them.

  6 passed, and 20 passed with test_safetensors_reasoning_stream.py

* Attribute a call to the innermost function that holds it

Reported on this PR and correct, and the same false-positive direction as the
last two. ast.walk descends into a nested def, so an importorskip written there
was attributed to the enclosing module-level function and handed that function's
import-time boundary. A nested body cannot run until something calls it, by which
time a stub installed below the outer call is in place, so the guard rejected a
file that works.

Nested bodies are deferred now, and a call inside one falls through to the
end-of-module boundary, which is what deferred means.

Pinned: an importorskip inside a nested def that the outer helper returns rather
than calls, with the stub after, is safe. Restoring ast.walk fails it.

  6 passed, and 20 passed with test_safetensors_reasoning_stream.py

* Stub the heavy imports in the peft-gated TTS test

Reported on this PR, real, and a consequence of installing peft in the previous
commit. test_audio_tts_cancellation.py gates on pytest.importorskip("peft") and
then imports core.inference.inference in the test body without stubbing anything.
That gate used to skip, so the import never ran. With peft installed it opens,
and the import works only because collecting test_safetensors_reasoning_stream.py
has already cached that module: module-level code runs during COLLECTION, before
any test body, so the whole suite hides it. Running the file on its own does not.

That is the same order dependence this PR exists to remove, reintroduced by the
fix, which makes it worth more than the P2 it came in as.

The file now stubs unsloth and trl, imports core.inference.inference while they
are live, and drops them again, exactly as its siblings do. The peft gate stays,
so a machine without peft still skips rather than failing.

Verified: with peft hidden the file skips cleanly on its own (17 passed, 1
skipped), and 59 passed across it plus test_audio_type_inconclusive.py,
test_safetensors_reasoning_stream.py and the guard. Locally the eager import is
blocked only by a torch/torchao ScalingType mismatch inside peft, which is
present on main too and has nothing to do with unsloth; with the stubs installed
that is the sole remaining blocker, checked directly.

Worth flagging separately: seven more files are in the same class, importing a
heavy backend module lazily while never stubbing --
test_chat_eos_template_refresh.py, test_chat_template_continuation.py,
test_control_markup_neutralize_7066.py, test_generation_timing.py,
test_nudge_tool_calls_wiring.py, test_online_tokenization_wiring.py and
test_training_preflight.py. All are masked by the same collection-time caching.
They are latent rather than failing, and fixing eight files belongs in its own
change rather than inflating this one.

* Widen the stub guard to module-scope with/if blocks and helper-held stubs

_first_heavy_import_line only read the direct children of the module body, so a
heavy import inside a module-scope with or if was invisible to the guard: a file
written as 'with something(): from core.training.trainer import X' without stubs
would take collection down unseen. It now uses the import-time traversal, with
try/except ImportError exempt since that is a deliberate guard.

That widening needs the matching read on the other side. Two files hold their
stubs with 'with _stubbed():' and import the heavy module inside that block, so
the installing code is one call away from module scope and the statement spells
neither the module name nor sys.modules. _stubs_before now reads through a call
to a module-level helper, the same way the stub-record check already does, and
keeps asking what the helper DOES rather than merely what it names.

Pins added for both halves plus the helper that names the module without
installing anything; each is mutation-checked.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Take the backend CI peft version from the requirements pin

An unconstrained 'pip install peft' takes the newest release (0.20.0 on the last
staging run), while extras-no-deps.txt pins peft==0.18.1 deliberately, because
0.19.0 breaks Unsloth's export subprocess. That left this job exercising the
newly enabled inference tests against a peft that production never ships and no
other job installs.

The version is read out of extras-no-deps.txt rather than written here again, so
a change to the pin cannot leave this workflow behind. peft 0.18.1 declares
torch>=1.13.0, transformers unbounded and python>=3.10, so it holds across the
3.10 to 3.13 matrix and does not disturb the CPU torch installed above it.

* Make both sides of the stub guard reachability and order aware

Two follow-ons to widening the search into module-scope compound statements.

The widened search used the plain import-time traversal, which descends into
both branches of every conditional, so an import under if TYPE_CHECKING: or
if False: read as an import-time dependency. A file with a legitimate type-only
import would have failed this guard until someone added a stub it does not need.
It uses the reachability-aware traversal now.

And widening one side without narrowing the other read the whole enclosing
statement as preceding the import, so

    if True:
        import core.inference.inference
        _stub_if_missing("unsloth", ())

counted as stubbed while Python attempts that import first and collection dies.
Only the part of a statement that runs before the import line is read now.

The stub side gets the same reachability treatment, from the other direction: a
stub installed under if False: installs nothing, so counting it would report a
file safe that still raises.

Three pins added, each mutation-checked.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Follow the import's own branch, and match except clauses by what they catch

Line order alone merges branches that exclude each other:

    if enabled:
        _stub_if_missing("unsloth", ())
    else:
        import core.inference.inference

puts the stub above the import while the two can never both run, and the guard
called that file stubbed. What is walked now is the import's own chain: at each
level only the block CONTAINING the import is descended into, and only the
statements above it there. Those did run, because the import running means its
branch was taken. A stub that merely might have run does not count either.

That last rule needs one exemption, and the tree already had the case:
test_training_progress_callback.py installs its stubs under
'if not _TRAINER_PRE_IMPORTED', where the flag is
'"core.training.trainer" in sys.modules'. Skipping the stubs on the other branch
is not an omission, because the import resolves out of sys.modules there and
never reaches the real dependency. A test that asks sys.modules whether the
target is already imported keeps its branch, read through a module-level flag as
well as directly, since that is how it is written.

Separately, the ImportError exemption matched substrings, so 'except
MyImportError:' and 'except ExceptionGroup:' both exempted a try that catches
neither. Handler types are compared as whole names now, including tuple members
and the last component of a dotted name. ModuleNotFoundError is deliberately not
in the set: it is a subclass, so it catches strictly less.

Ten pins added across the four behaviours, each mutation-checked.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Take the loaded-models indicator suite off the UI job's critical path (#9060)

* Take the indicator suite off the UI job's critical path

Unsloth UI CI / Chat UI Tests has been timing out at its 30 minute limit on
almost every open PR: 5 of the 6 currently open, at 30m09s to 30m24s. Nothing
was failing. The job was simply longer than the limit, and a limit is a
guillotine, so the steps after the cut never ran at all.

Where it goes: "Cross-browser loaded-models indicator" is 853s of the 30
minutes, three browser engines at about 4.7 minutes each, each booting its own
Unsloth. Everything downstream of it -- the other-engines update banner
regression, both image staged-download regressions, the model-picker
per-model-config suite and the IME / multilingual paste regression -- was
reported "skipped" and has been running on no PR at all.

It is now its own job rather than a step, so it runs beside the rest instead of
inside their budget. That leaves ui-smoke around 16 minutes and the new job
around 15, both with real margin, and the five skipped suites run again. The new
job carries no HF cache and no GGUF: the suite stubs the four /status reads with
page.route, so it needs no model, no GPU and no llama.cpp build.

Moving the work rather than the limit is deliberate. The limit here already went
25 -> 30 for this exact symptom, with the comment on it still describing that
round, so raising it again buys one more release of silence.

Also tests/studio/test_playwright_suites_run_in_ci.py, because these drivers are
standalone scripts that nothing collects: a suite runs only because a workflow
step or a .github/scripts helper names it, and deleting that line leaves every
job green. It fails if any tests/studio/playwright_*.py is named by no workflow
and no CI script. Deleting either the new step or the driver call inside
run-studio-indicator-browser.sh fails it.

It also found two suites already in that state, exempted with a reason each
rather than quietly ignored: playwright_tauri_python_tool_images.py needs the
Tauri desktop shell no runner here builds, and playwright_train_pickers.py needs
a dataset and model resolved through huggingface_hub while the UI workflows boot
API-only. Both are worth wiring up, separately from this.

  test_playwright_suites_run_in_ci.py:  3 passed, and 2 failed under each mutation
  test_stt_model_search_locator_contract.py + test_composer_rtl_bidi_attribute.py: 34 passed

* Count only helper scripts a workflow can actually reach

Reported on this PR and correct: reading every file under .github/scripts counts
an orphaned helper as coverage, so a driver named only by a script nothing calls
reads as covered while it runs nowhere, which is the regression this test exists
to catch.

The workflows now seed the text and a helper joins only once something already in
it names the helper, repeated to a fixed point because one helper can call
another. .github/actions is included the same way, since a workflow reaches a
composite action by uses: and the action then calls the script.

Verified by making the reference unreachable in all three UI workflows while
leaving run-studio-indicator-browser.sh in the tree. The new version reports the
driver as running nowhere; the scan-everything version reports it covered. That
one helper is named by the Linux, Windows and Mac UI workflows, which is why
removing it from one is correctly still covered.

  3 passed

* Count only the workflow fields that run something

Reported on this PR and correct. Reading a workflow whole counts a driver named
in on.pull_request.paths as an invocation, and studio-frontend-ci.yml names
playwright_strip_ansi_smoke.py in both its trigger list and its step, so deleting
the step alone left this guard green while the suite ran nowhere. Trigger paths
say when CI runs, not what it runs.

The scan now parses each workflow and reads only the executable fields: a job's
uses, and each step's run, uses and with. Helper reachability is followed from
that text as before.

Verified by deleting only the run line in studio-frontend-ci.yml and leaving the
trigger path: the guard now reports playwright_strip_ansi_smoke.py as running
nowhere, where before it stayed green.

  3 passed

* Follow a composite action by the directory workflows reference it with

Reported on this PR and correct. A composite action is used as
`./.github/actions/<name>`, never as the action.yml inside it, so the
reachability walk never opened one and a driver launched from inside an action
read as an orphan. An action.yml now also matches on its containing directory.

Pinned with a line from inside install-unsloth-local's own body rather than
something a workflow could also contain, so the pin fails if the walk matches the
uses: reference without opening the action. Dropping the directory match fails it.

  3 passed

* Run the coverage guard from the workflow that has no paths filter

Reported on this PR and correct: the guard could not catch the regression it
exists for. It ran only in studio-backend-ci.yml's Repo tests job, whose paths do
not include .github/workflows/**, .github/scripts/** or .github/actions/**, so a
PR that only deletes a Playwright invocation never collected it and stayed green
until the post-merge run on main.

It now runs in workflow-trigger-lint.yml instead, which carries no paths filter
at all and says so in its own header comment, deliberately, for the same reason:
a gate that only runs for some PRs does not gate workflow changes. Cost is the
same shape as the lint beside it, filesystem reads in seconds, and that job
already installs PyYAML, which is all this needs besides pytest.

  3 passed, run bare from the repo root as that job runs it
  the workflow-trigger lint itself still passes: 42 workflow files scanned

* Pin the three browser engines to the job that runs them

Reported on this PR and correct: the repo-wide check cannot see this job
disappear. run-studio-indicator-browser.sh is named by the Mac and Windows UI
workflows too, so deleting all three calls from the Linux workflow leaves every
guard green while the Chromium/Firefox/WebKit coverage this job exists for is
gone.

Asserted against the ui-indicator job itself rather than the file, so a step
moved back into ui-smoke fails it too: that is the 30-minute limit this change
moved the work out of.

  4 passed, and deleting the three calls fails the new test by naming all three

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Count a driver as covered only where CI actually runs it

Reported on this PR and correct, with a real instance: substring presence is not
coverage. .github/scripts/kaggle_studio_ci/report.py names playwright_chat_ui.py
inside a result description, and the workflows name drivers in comments and in
trigger paths, so deleting every real invocation could leave this guard green on
prose alone.

Every driver and helper in this repo is run the same way, as an argument to an
interpreter, so that is what is matched now: the name at the end of a path token
being handed to python, python3, node, bash or sh. Composite actions keep the
directory match, since a workflow reaches one through uses: rather than a command.

Verified by removing every real invocation of playwright_chat_ui.py while leaving
report.py's description in place. The substring is still present and the guard now
reports the driver as running nowhere; before, it stayed green.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Stop a commented-out invocation counting as coverage

Reported on this PR and correct: commenting a command out is how one gets
disabled, and the scan reads workflow run: bodies and helper scripts verbatim, so
`# python tests/studio/x.py` matched the invocation shape through the whitespace
before python.

Comments are stripped before matching. Shell, YAML and Python all take # to end of
line, and a # inside a string only ever appears in prose here, which is not an
invocation either way.

Verified by commenting out every real invocation of playwright_chat_ui.py: the
guard now reports it as running nowhere, where before it stayed green.

  4 passed

* Tighten the comments in the coverage guard

Comments and docstrings only. The code is byte-identical once docstrings are
stripped, checked by comparing the parsed trees, and the tests still pass.

* Read the browser-engine guard through the comment stripper

The cross-engine check tested raw step bodies for the substring, so prefixing
all three invocations with # left it green while the job ran no browser suite,
which is the regression it exists to prevent. It uses the same _uncommented()
helper the repo-wide scan does, and a pin asserts the disabled form of one of
those lines does not read as coverage.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Keep the killed-by-signal exit status when release() fails (#9072)

* Keep the killed-by-signal exit status when release() fails

A cancelled launcher reported success. The signal handler deletes the kernels
and then re-raises the signal so the status still reads killed, but the delete
was outside any try: a raise there propagated out of the handler into whatever
the main thread was doing, main()'s except BaseException caught it, and finish()
called release() a second time. When that second call worked, which is what a
transient OSError out of a subprocess spawn on a loaded runner looks like, main
RETURNED 0 and the job read as completed.

Caught on a contended runner rather than by inspection, and reproduced by
failing the first delete only: returncode 0 instead of -SIGTERM.

The delete is now best effort, retried once, and the death by signal happens
either way. If both attempts fail the slugs stay in the registry and the next
launcher's orphan sweep reclaims them, which is the same path a kill -9 already
takes.

The subprocess waits in the suite move from 30s to a named budget of 120s. They
guard against a launcher that never dies, not against latency, and the repo
suite now runs four of these at once on a four-core runner.

* Make the stall outlast the death budget, and assert the signal did the killing

Raising the wait to 120s put it past the 60s stall the launcher sits in, which
defangs the tests it was meant to make robust: a handler that swallows its
signal leaves the process asleep and then resuming, so it wakes, runs finish(),
deletes the kernels through the ordinary path and exits INSIDE the wait. The
deletion assertions are satisfied by that, so those tests would have passed on a
launcher that ignored the signal entirely.

The stall is now a named 900s against the named 120s budget, and a test asserts
the relationship rather than leaving it to whoever next tunes one of the two
numbers.

The deletion test also asserts the exit status now. Without it, the deletion is
satisfied by finish() doing its usual work, so nothing there said the signal was
what caused it.

Verified by making the handler return instead of re-raising: with the stall at
900s that fails all four signal tests, and with it at 60s the relationship test
fails as well.

* Tighten the comments added here

* Read a stub helper only as far as it has run

Reading the whole helper body accepted a sys.modules write below the yield of a
context manager. That code runs on the way OUT of the with block, after the
import inside it has already been attempted, so it stubs nothing for that import
and the file still dies at collection while the guard stays green.

The scan now stops at the helper's first yield, and drops unreachable and merely
optional branches the way the module-scope side already does. The yield has to be
the helper's own: one belonging to a generator defined inside it says nothing
about when the helper suspends, and taking it cut the scan off above the install
the real files do perform.

Three pins, each mutation-checked.

* Count only the finally of a try as certain

A try body whose handler swallows the exception is exactly a block that may stop
part way: an optional import fails, the handler catches it, the stub call below
it never runs, and the heavy import after the try does. Counting the whole body
reported that file stubbed.

Only the finally counts now, which runs on both paths. The else and the handlers
are conditional by construction and were already excluded. No test module in the
tree installs its stubs inside a try body, so nothing legitimate changes.

Two pins, both mutation-checked.

* Read the polarity of the already-imported guard, and reach nested helpers

Three holes, all about when code runs rather than what it says.

The already-imported exemption accepted any branch whose test mentioned
sys.modules, either way up. Written as 'if _PRE:' the stubs are installed only
when they are not needed and skipped when they are, and the import then fails at
collection with the guard green. The polarity is read now, through a negation and
through a module-level flag, and only the branch that runs when the module is
ABSENT counts, since that is the one whose stubs the import needs.

A while-else was treated as certain. It is skipped when the loop leaves through
break, so it is not.

And nested defs were left out of the function map entirely. Deferring a nested
body is right while nothing calls it, but an outer helper the module body calls,
which defines and calls an inner one, runs that inner body during collection. The
call inside it was handed the end-of-module boundary, the lenient answer, so a
stub installed after the outer call read as being in time while the inner import
had already run. Nested defs are in the map now and inherit the boundary of the
call that reaches them; one nothing calls at import is still deferred.

Seven pins across the three, each mutation-checked.

* Judge handlers, helper exits and stub calls by what they do

Three more exemptions that were granted on shape rather than effect.

A handler naming ImportError does not necessarily absorb it: 'except ImportError:
raise', and one raising a replacement, both take collection down while the try
body read as guarded, which hid the import from the guard entirely. Any raise the
handler can reach disqualifies it now. A module-level pytest.skip is a call rather
than a raise, so the skip idiom these files use stays exempt.

A helper that can return before installing has not installed anything, so the scan
stops at an exit rather than reading on to the call below it. Two exits are benign
and both are already in the tree: a return under 'name in sys.modules', and one
under the importlib probe inside an absorbing try. On each of those paths the
module is AVAILABLE, so skipping the stub is the point of the branch. Without that
distinction five correct files became offenders, which is how the shape of the
exemption was found.

And a call was treated as installing a stub because its name contained 'stub',
which counted _remove_stub and _validate_stub. Where the module defines the
callee, what it does decides. The name stays as the fallback for a helper imported
from elsewhere, since a name defined nowhere would raise NameError at import and
never reach collection.

Nine pins, four mutation-checked behaviours.

* Honour handler dispatch order, and stop at deferred bodies and dead code

Python dispatches an exception to the FIRST handler that matches it, so 'except
Exception: raise' followed by 'except ImportError: pass' still propagates. Asking
whether ANY handler absorbs found the second one and exempted a try that does not
guard the import. Only the first handler that would catch it is asked now.

The re-raise check also descended into a def the handler merely DEFINES. That body
does not run while the exception is being handled, so a properly guarded file was
reported as an offender. The traversal stops at deferred bodies now.

And the call-graph closure walked past an unconditional exit, handing a helper
named below one the outer call's import-time boundary although it never ran, which
rejected a stub installed below that call. It stops at such an exit now, checked
AFTER the statement since 'return _inner()' runs its own expression, and only for
an unconditional one: the path that skips a conditional exit still reaches the
calls below it.

Five pins, three mutation-checked behaviours.

* Keep every definition of a helper name, and stop reachability at dead code

Two enclosing functions can each define a helper of the same name. Keeping only
the first dropped the second from the call graph, so its importorskip took the
end-of-module boundary and a stub installed after the enclosing call read as being
in time. All definitions of a name are kept now. Which one a given call names
cannot be told apart here, so all of them take the boundary, which is the strict
answer.

And the general reachability walk did not stop at an unconditional exit, so an
importorskip written after a bare return or raise was reported as an unstubbed
offence. Python cannot execute it, so that is a file failing CI over dead code.
The walk now ends a statement list at a return, raise, break or continue, which
the import-time traversal beside it already did.

Four pins, both mutation-checked.

* Judge every heavy import, not just the first

Separate branches can each import a heavy module, and only one of them needs to be
stubbed for the first to look fine. A file that stubs before the import in the if
and forgets the one in the else dies at collection whenever the else runs, while
reducing the module to a single line reported it safe.

Every import-time heavy import is collected now and each is judged against the
stubs that precede it on its own path. Any single unstubbed one is an offence,
whatever the others do.

Two pins, mutation-checked.

* Require the availability guard to name the module, and drop async installers

The already-imported exemption fired on any sys.modules membership test, so
'if "pytest" not in sys.modules:' decided whether the stubs under it counted.
An unrelated cached package says nothing about unsloth, and the branch is skipped
exactly when that package is present. The key now has to be the required stub or
a heavy backend module. A key this file cannot read gets that latitude only inside
a stub helper deciding about its own argument, which is the idiom
_stub_if_missing opens with, and never at module scope.

And calling an async def builds a coroutine and runs none of its body, so reading
through such a call credited the module with stubs it never installed. Only
synchronous helpers are read through now.

Six pins, three mutation-checked behaviours.

* Drop the review bookkeeping from the comments

Thirty-three comments ended in 'Reported on this PR', which says nothing to
anyone reading the file afterwards: once merged there is no this PR. The reasoning
each comment records is what matters and is kept.

* Name the file rather than the pull request

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
shimmyshimmer added a commit that referenced this pull request Aug 25, 2026
The vite install in the banner shard exists for playwright_overlay_rail.py and
nothing else, so with the harness gone it is dead. Nothing left in
studio-ui-smoke.yml touches npm, npx, node_modules or studio/frontend, so on a
frontend-dist cache hit it would run a clean `npm ci` for no consumer.

logs/playwright-overlay-rail goes with it: it was only ever written by the two
steps this branch removed.

Job-level setup-node stays. It predates the harness (#9060) and the installer
needs node to build the frontend on a cache miss.
shimmyshimmer added a commit that referenced this pull request Aug 25, 2026
)

* Studio: put the notification rail back in its bottom-right corner

The rail was anchored with `fixed bottom-4 right-4` until #8082, which added
the loaded models indicator and, with it, a frame store the rail read: the
Live monitor, the API monitor panel and the chat composer publish their boxes,
and `useStackGeometry` wrote `bottom` and `maxHeight` as inline styles that
dodged them.

Every input to that placement moves on its own. The composer grows with its
input and goes from centred to docked with no resize event, a download row
arrives, release notes expand, a banner is dismissed. Each change re-ran the
measurement, and when the room fell short the rail lifted over the obstacle at
`viewportHeight - frame.top + STACK_GAP`. On the welcome chat that is the cards
in the middle of the window; against a maximised monitor the lift clamped at
MIN_STACK_ROOM and parked them at the top. It reads as random because the
trigger is measurement timing.

The store also had to probe its own output, dropping `max-height` to read
`scrollHeight` and restoring it with transitions suppressed, because the
placement was feeding on the box it had just sized. It went from 184 lines to
701 over nine follow-up PRs, all of them editing the same arithmetic.

So this restores the anchor rather than adding another special case. The rail
is `fixed bottom-4 right-4` with the CSS cap again, on the named layer rather
than the old hard-coded z-index. The frame store keeps its map of boxes, which
api-monitor/panel-placement still needs to open the API panel clear of the
monitor and the composer, and loses the dodge arithmetic, the measurement loop
and the `coverable` licence. The composer still publishes for that panel's
sake. The indicator itself is untouched.

Known trade-off, and the pre-#8082 behaviour: in a short window a card can sit
over the composer's Send button rather than the rail dodging it. The indicator
is off by default, draggable and dismissible.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Studio: scroll the rail at its cap instead of spilling cards off screen

Review catch, and it reproduces. The cap alone does not contain anything: the
cards that cannot shrink lay out past the rail's bottom edge, which is pinned
16px off the floor, so the overflow goes under the bottom of the window with
no way to bring it back.

At 320x480 with --ui-font-scale 1.25, three cards up: the llama.cpp banner is
in flow with its bottom at 497 against a 480px viewport, and the card after it
is squeezed to 0px at 505. scrollHeight 501 against clientHeight 448, and
overflow-y visible. The Update button is below the fold for good.

Earlier sizes missed it because the download panel is the one min-h-0 card in
the rail and absorbed the squeeze on its own. Raising the type size puts the
two banner floors over the cap and the slack runs out.

So the rail scrolls again, in CSS only. No measurement, no placement: the
offset and the cap stay where they were. The gutter comes back with it, since
overflow-y forces overflow-x and the card shadows clip without it, and stays
horizontal so the bottom card keeps sitting on bottom-4.

The rail stays click-through. Verified that the fold is still reachable:
elementFromPoint over a card returns a node inside it, so the card is the hit
target despite the rail being pointer-events-none, and the rail is that node's
nearest scrollable ancestor, which is the chain a wheel scrolls. Focus works
too: the Update button measured at bottom 481 in a 480px viewport, and
focusing it scrolled the rail to 17 and brought it to 464.

* Studio: measure the rail's corner off a card, and drop the stale pointer contract

Two fixes to the Playwright suite, both mine to begin with.

The corner check measured `fromRight` off the rail's border box. The rail
carries the shadow gutter, `-mx-3 px-3`, so its border box sits 4px from the
viewport edge while the cards it pads sit at 16. It reported 4 and would have
failed at every inspected viewport. Measured at 320x480 and at 921x534, one of
the two the pass runs: rail 4, every in-flow card 16. It now takes the right
edge from the cards, filtered to the ones in flow, since a dragged loaded
models card is `fixed` somewhere else. The bottom still comes off the rail:
no vertical margin or padding, and its border box does not move when the cards
scroll. Checked at both scroll extremes.

The other is a contract this branch already invalidated. The suite asserted the
rail takes pointer input exactly when it scrolls, which was #9150 and needed the
JS that also placed it. The rail is click-through in every state now, so that
check failed whenever the rail scrolled. Verified at 320x480 with the rail
actually scrolling, scrollHeight 501 against clientHeight 448: pointer-events
none, and a click on the gutter lands on the thread viewport behind it. Both
assertions are now unconditional.

* CI: drop the install and the artifact dir the deleted rail harness owned

The vite install in the banner shard exists for playwright_overlay_rail.py and
nothing else, so with the harness gone it is dead. Nothing left in
studio-ui-smoke.yml touches npm, npx, node_modules or studio/frontend, so on a
frontend-dist cache hit it would run a clean `npm ci` for no consumer.

logs/playwright-overlay-rail goes with it: it was only ever written by the two
steps this branch removed.

Job-level setup-node stays. It predates the harness (#9060) and the installer
needs node to build the frontend on a cache miss.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
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