vrr/unsloth
2
0
Fork 0
mirror of https://github.com/unslothai/unsloth.git synced 2026-08-25 08:42:25 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 29
unsloth/studio/backend/tests/test_llama_cpp_no_context_shift.py
Daniel Han 96cf275d71
Run the Backend CI matrix in parallel, like its sibling job already does (#9095) 
* Run the Backend CI matrix in parallel, like its sibling job already does

The matrix leg is the longest job in the repo at 23.3 minutes, and it was the only large
pytest run still serial. repo-cpu-tests has run -n 4 since it measured 806s -> 220s.

Measured over the same tree in the same environment, serial against -n 4: 1322.6s ->
343.0s, and the results are identical -- 51 failed / 26190 passed serial, 51 failed /
26188 passed parallel, with the two failure sets compared name for name and equal. So
nothing in the backend suite depends on the order it runs in. (Those 51 are one local
environment missing peft and a diffusers pin. The point is that the two modes agree.)

This is CPU bound and not memory bound: the suite loads no model, unlike the inference
smoke workflows where four workers on one runner would not fit.

The existing isolation guard caught this change, correctly: it assumed exactly one
parallel pytest run, and the two now run over different trees. The repo-root job runs
tests/ from the checkout, the matrix job runs the backend's own suite from
studio/backend, so tests/studio/load_freeze is not a path that exists for it and
demanding those ignores would be nonsense. The guard now tells the two apart and applies
the isolation rules to the repo-root run only, and a new test pins the matrix leg as
parallel so losing the flag shows up as a failure rather than as CI slowly getting
slower again.

* Keep the relative-timing tests off the parallel workers

Staging caught what the local comparison could not: the 3.10 leg reported 'early markup
cost 1.354s against the reference's 0.854s' and 'incremental cost grew 7.0x vs the
reference's 11.5x', while 3.13 passed the same commit in 9 minutes against the 23 it used
to take.

test_streaming_stripper times itself against a reference implementation measured in the
same process. Under four workers on four vCPUs one side of that ratio gets descheduled
and the other does not, so the comparison stops being between two implementations. It is
the same reason repo-cpu-tests already keeps load_freeze out of its parallel run, and it
does not reproduce on a machine with cores to spare, which is why the local run agreed
serially and in parallel.

So it is ignored from the parallel run and runs again in its own serial step, and the
isolation guard now holds that pair together for the backend run the way it already does
for the repo-root one: putting the file back in the parallel run fails one test, deleting
the serial step fails another.

* Find the tight elapsed-time bounds by scanning, not by remembering

Two more files assert ABSOLUTE elapsed time, and tightly: 50ms for a short-circuit that
should not run the probe at all, and 100ms for a regex backtracking guard. Bounds that
small sit inside one scheduler quantum, so under four workers on four vCPUs they measure
the scheduler as much as the code. Both passed on staging, which is the problem: they
would have flaked later, on somebody else's change.

Twenty-two backend files assert some elapsed bound, and serialising all of them would
give back most of what -n 4 buys. So the line is drawn at 0.1s, where the measurement
stops being about the code, and the three files at or below it are ignored from the
parallel run and rerun serially, which costs 2.2s and 1.7s.

The guard now finds them by scanning rather than by listing. It reads with ast, so the
name has to be assigned from a difference of two clock readings: grepping for '< 0.05'
would match a float tolerance, and grepping for 'elapsed' would match anything. A new
test asserting a 20ms bound fails that guard instead of buying a flake, which is verified
by adding one.

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

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

* Run the Backend CI matrix in parallel, like its sibling job already does

The matrix leg is the longest job in the repo at 23.3 minutes, and it was the only large
pytest run still serial. repo-cpu-tests has run -n 4 since it measured 806s -> 220s.

Measured over the same tree in the same environment, serial against -n 4: 1322.6s ->
343.0s, and the results are identical -- 51 failed / 26190 passed serial, 51 failed /
26188 passed parallel, with the two failure sets compared name for name and equal. So
nothing in the backend suite depends on the order it runs in. (Those 51 are one local
environment missing peft and a diffusers pin. The point is that the two modes agree.)

This is CPU bound and not memory bound: the suite loads no model, unlike the inference
smoke workflows where four workers on one runner would not fit.

The existing isolation guard caught this change, correctly: it assumed exactly one
parallel pytest run, and the two now run over different trees. The repo-root job runs
tests/ from the checkout, the matrix job runs the backend's own suite from
studio/backend, so tests/studio/load_freeze is not a path that exists for it and
demanding those ignores would be nonsense. The guard now tells the two apart and applies
the isolation rules to the repo-root run only, and a new test pins the matrix leg as
parallel so losing the flag shows up as a failure rather than as CI slowly getting
slower again.

* Keep the relative-timing tests off the parallel workers

Staging caught what the local comparison could not: the 3.10 leg reported 'early markup
cost 1.354s against the reference's 0.854s' and 'incremental cost grew 7.0x vs the
reference's 11.5x', while 3.13 passed the same commit in 9 minutes against the 23 it used
to take.

test_streaming_stripper times itself against a reference implementation measured in the
same process. Under four workers on four vCPUs one side of that ratio gets descheduled
and the other does not, so the comparison stops being between two implementations. It is
the same reason repo-cpu-tests already keeps load_freeze out of its parallel run, and it
does not reproduce on a machine with cores to spare, which is why the local run agreed
serially and in parallel.

So it is ignored from the parallel run and runs again in its own serial step, and the
isolation guard now holds that pair together for the backend run the way it already does
for the repo-root one: putting the file back in the parallel run fails one test, deleting
the serial step fails another.

* Find the tight elapsed-time bounds by scanning, not by remembering

Two more files assert ABSOLUTE elapsed time, and tightly: 50ms for a short-circuit that
should not run the probe at all, and 100ms for a regex backtracking guard. Bounds that
small sit inside one scheduler quantum, so under four workers on four vCPUs they measure
the scheduler as much as the code. Both passed on staging, which is the problem: they
would have flaked later, on somebody else's change.

Twenty-two backend files assert some elapsed bound, and serialising all of them would
give back most of what -n 4 buys. So the line is drawn at 0.1s, where the measurement
stops being about the code, and the three files at or below it are ignored from the
parallel run and rerun serially, which costs 2.2s and 1.7s.

The guard now finds them by scanning rather than by listing. It reads with ast, so the
name has to be assigned from a difference of two clock readings: grepping for '< 0.05'
would match a float tolerance, and grepping for 'elapsed' would match anything. A new
test asserting a 20ms bound fails that guard instead of buying a flake, which is verified
by adding one.

* Teach the timing scan the two shapes it was blind to

The scan only recognised a comparison whose left operand was a name assigned from a clock
difference. Two forms in this suite are written differently and were silently skipped:

  time.monotonic() - started < 0.2        the difference inline (test_stt_download_followups)
  _elapsed(big) < 8 * _elapsed(small)     a helper returning one (test_diffusion_checkpoint_resume)

The second is not a near miss. It compares two wall-clock measurements taken in sequence,
so descheduling one side and not the other breaks it at ANY magnitude, with no threshold to
be under, which is the same reason test_streaming_stripper came out of the parallel run. It
was still running under -n 4.

So the scan now asks whether an expression IS a duration, however it was spelled: a name
assigned from a difference, a difference written inline, or a call to a function that
returns one, found by walking for a return of a clock difference at any nesting depth --
the helper in question is defined inside its own test. And a relative comparison is fragile
regardless of magnitude, while an absolute one still has to be at or below the threshold.

test_diffusion_checkpoint_resume joins the serial step, costing 8.9s. Adding either shape
to a file that is not isolated fails the guard, both verified.

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

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

* Follow the clock value through a container, and record what is benign

The scan tracked names assigned a clock DIFFERENCE. test_tool_output_streaming compares
first_seen_at[0] - started against finished - started - 0.5, where every term is an
instant, no single name ever holds a duration, and one of them is parked in a list by a
callback. Nothing in it looked timed, so the file kept running under -n 4 while asserting
that a callback fired at least 0.5s before the child exited -- which collapses if the
worker is descheduled while the child sleeps.

So instants count now, not just differences, including one appended to a container, and
the check walks the expression rather than reading its top node.

That widened net found four more files, and only two are real: test_web_fetch_extraction
compares parse time at two input sizes, and test_tool_output_streaming is the above. The
other three are not performance claims at all. A sandwich, "before <= recorded <= after",
cannot be falsified by widening the gap; a poll deadline inside a wait-for-condition loop
is the pattern that replaces a guessed sleep; and "stamp < 0.0" compares against a
sentinel. Those are in BENIGN_TIMING with their reasons, keyed on the enclosing function
so an edit above them does not move the exemption onto something else.

Keeping the net wide means a new benign pattern lands here too, so the failure message now
says which of the three ways out applies rather than assuming the test is wrong.

Verified: a stored-instant comparison added to an unisolated file fails the guard, and the
two new files cost 39.5s and 13.3s in the serial step.

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

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

* Follow a duration back through the name a helper returned it in

The helper check required the return expression itself to read a clock.
test_tool_call_parser_strict has

    def best_ms(depth):
        best = float("inf")
        for _ in range(5):
            t0 = time.perf_counter()
            ...
            best = min(best, time.perf_counter() - t0)
        return best

    t200 = best_ms(200)
    t400 = best_ms(400)
    assert t400 < t200 * 3.0

where the return reads no clock, and neither does the assignment that holds the result.
Two links were missing, not one: a function counts as a timing helper if it returns any of
its OWN timed names, and a name counts as timed if it was assigned from a call to such a
helper. Helpers are resolved first for that reason, and to a fixpoint, so a helper built on
another helper is found on the next pass rather than missed.

Worth noting as a check on the scan rather than on this test: test_streaming_stripper is
now found by the scan on its own, having been in the isolation list by hand since the run
that started this. The rule and the list agree where before only the list knew.

test_tool_call_parser_strict joins the serial step at 2.0s. A helper returning a duration
through a local name, added to a file that is not isolated, fails the guard.

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

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

* Isolate the keepalive test staging caught, and say what the scan cannot see

test_tunnel_safe_long_post::test_the_route_still_pads_the_same_slow_load failed on a
staging 3.13 leg that had been green. It patches the keepalive threshold to 0.05s and makes
the work sleep 0.2s, then asserts the response starts with padding. Whether it passes turns
on which of two timers fired first, and four times the threshold was not enough margin
under four workers on four vCPUs.

The scan did not find it and structurally cannot: it looks for assertions COMPARING
clock-derived values, and this one has no clock in it at all. The dependency is implicit,
between a patched constant and a sleep, and the assertion is on the result.

Ten backend files pair a sub-second sleep with a small threshold. Since 4x margin already
proved insufficient, the ratio is not a usable rule, and flagging all ten would serialise a
large part of the suite on a guess. So that class stays with staging, which is the only
thing that has ever caught one, and the limit is written next to BACKEND_ISOLATED rather
than left for the next person to discover the same way.

The file joins the serial step at 3.9s.

* Isolate the heartbeat-count test, and correct the note about how this class is found

test_scan_loras_off_event_loop counts how many times a heartbeat coroutine ticked during a
0.3s sleep and requires at least three. Descheduling the worker costs ticks without the
scan being wrong, so it fails for a reason that has nothing to do with the code under test.

Same class as the keepalive test in the previous commit, from the other direction: there
the assertion was on a result, here it is on a COUNT, and either way there is no duration
in the expression for the scan to find.

The previous commit said this class was left to staging, "the only thing that has ever
detected one". That was true when written and is not now: this one came from review, on a
file staging had not yet failed on. Found by reading, not by scanning, is the accurate
statement, and the note says that instead.

The file joins the serial step at 1.6s.

* Isolate the anthropic keepalive counts from the xdist workers

Codex found test_scan_loras_off_event_loop by reading, not by running: it counts
heartbeats across a 0.3s sleep, and the assertion compares a count rather than
anything clock-derived, so the AST scan in this file is structurally unable to
see it. That is a class, not a one-off, so I read the rest of the suite for the
same shape instead of waiting for staging to hit it.

One more: test_anthropic_messages asserts len(keepalives) >= 3 and >= 2 across a
_time.sleep(0.24) stall past several shortened keepalive windows. A descheduled
worker loses keepalives exactly as the heartbeat test loses ticks. It costs 2.2s
to run serially.

One false positive worth naming, because the grep that finds these is crude:
test_diffusion_backend asserts len(staged) > 1 near a 0.2s sleep, but staged is a
list comprehension over cached filenames with no timing in it. It also costs
152s, so matching on the pattern alone would have been expensive as well as
wrong.

* Read every link of a chained comparison, not just the first

A tight bound is often written as a sandwich, and a sandwich is one Compare node
whose leftmost operand is the literal floor. Requiring that operand to be timed
made the scan skip the upper link entirely, so a file could hold an assertion of
the shape it exists to find and stay in the -n 4 run with the guard green.
test_llama_cpp_wait_for_vram_settle already writes a bound that way. Verified by
running the scan over a file with 0.3 <= elapsed < 0.05, which it now reports and
previously did not.

The walk also reads Gt and GtE by swapping the operands, since a bound written
backwards bounds the same thing. That turned up one live case, an access token
asserted to expire after the wall clock. The margin is 600 seconds, so reading
both sides late by whole seconds still leaves it true, and it goes in
BENIGN_TIMING rather than into the serial step.

* Stop the test stubs shadowing httpx once the suite runs in small pieces

The 3.10 leg failed collection on two of the ten files in the new serial step,
on module 'httpx' has no attribute 'Response', and it is worth being precise
about what that is: httpx is installed on that job. Thirteen backend modules
build a fake one and install it with sys.modules.setdefault, which reads as
deferring to the real library and does not. sys.modules holds what has been
IMPORTED, not what is installed, so in a process where nothing has touched httpx
yet the stub wins and shadows it for the rest of the session. The stubs have no
Response, starlette.testclient reads httpx.Response at import, and everything
collected afterwards that reaches fastapi.testclient or routes.inference dies.

In a 26,000-test run something always imports httpx before any of them is
collected, which is why this has been invisible for as long as the suite ran as
one process. Splitting the timing tests out removed the accident rather than
introduced the bug, and any future split would have found the same landmine.

All thirteen now try the real import first, the form test_llama_cpp_placement.py
already uses. Reproduced before the change by collecting wait_for_vram_settle and
diffusion_checkpoint_resume together, which errored, and after, which collects
154 tests. The full suite still collects 26391.

The guard is scoped to the isolated files. Around fifty other modules stub
structlog the same way and are load-bearing in a run that also imports the real
one, so rewriting those is a separate change with its own risk. What has to hold
here is that anything moved OUT of the parallel run stands on its own.

* Propagate helpers through assigned results, and isolate one more tick count

Two from review, both real.

The fixpoint over timing helpers called _timed_names without the helpers it had
already found, so a wrapper that assigns value = base() and returns value never
learned that value was timed. base was discovered, the wrapper was not, and any
relative benchmark built on the wrapper stayed invisible. The pass that learns a
helper is not the pass that reads its callers, which is the whole reason this
runs to a fixpoint, so the set has to go in as well as come out. Verified on a
base/wrapper pair the scan now reports and did not before.

test_profile_stats counts event-loop ticks during a 0.5s blocking call and needs
more than ten of the roughly fifty that fit. That is the same shape as the two
tick counts already isolated: no clock appears in the assertion, so the scan
cannot see it, and a descheduled worker loses ticks. 12.8s serially.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
2026-08-17 08:23:36 -07:00

169 lines
6.9 KiB
Python
Raw Permalink Blame History

# SPDX-License-Identifier: AGPL-3.0-only
# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0
"""``--no-context-shift`` launch-flag contract.
With llama-server's default context-shift behavior, the UI cannot tell the user
the KV cache was rotated -- earlier turns silently vanish from the conversation.
The Unsloth backend always passes ``--no-context-shift`` so the server returns a
clean error instead, and the chat adapter can point the user at the
``Context Length`` input in the settings panel.
This file statically reads the launch command: we ask ``LlamaCppBackend`` to
assemble its ``cmd`` list and assert the flag is present. Testing via the real
subprocess would need an actual GGUF on disk, out of scope for the fast suite.
"""
from __future__ import annotations
import inspect
import sys
import types as _types
from pathlib import Path
import pytest
# ---------------------------------------------------------------------------
# Same external-dep stubs as the other llama_cpp tests.
# ---------------------------------------------------------------------------
_BACKEND_DIR = str(Path(__file__).resolve().parent.parent)
if _BACKEND_DIR not in sys.path:
sys.path.insert(0, _BACKEND_DIR)
_loggers_stub = _types.ModuleType("loggers")
_loggers_stub.get_logger = lambda name: __import__("logging").getLogger(name)
sys.modules.setdefault("loggers", _loggers_stub)
_structlog_stub = _types.ModuleType("structlog")
sys.modules.setdefault("structlog", _structlog_stub)
_httpx_stub = _types.ModuleType("httpx")
for _exc in (
"ConnectError",
"TimeoutException",
"ReadTimeout",
"ReadError",
"RemoteProtocolError",
"CloseError",
):
setattr(_httpx_stub, _exc, type(_exc, (Exception,), {}))
_httpx_stub.Timeout = type("T", (), {"__init__": lambda s, *a, **k: None})
_httpx_stub.Client = type(
"C",
(),
{
"__init__": lambda s, **kw: None,
"__enter__": lambda s: s,
"__exit__": lambda s, *a: None,
},
)
# Only when the real library is absent. sys.modules holds what has been IMPORTED, not
# what is installed, so setdefault does not defer to a real httpx that nothing in this
# process has touched yet: the stub wins and shadows it for the whole session. This stub
# has no Response, and starlette.testclient reads httpx.Response at import, so every
# module collected afterwards that reaches fastapi.testclient or routes.inference dies.
try:
import httpx # noqa: F401
except ImportError:
sys.modules.setdefault("httpx", _httpx_stub)
from core.inference import llama_cpp as llama_cpp_module
def _load_model_source() -> str:
"""Return the source of ``LlamaCppBackend.load_model``.
Using ``inspect.getsource`` instead of reading the file scopes the assertions
to the function that launches llama-server, so neither the presence nor the
location check can be fooled by a stray ``"--no-context-shift"`` elsewhere in
the module.
"""
return inspect.getsource(llama_cpp_module.LlamaCppBackend.load_model)
def test_no_context_shift_is_in_load_model():
"""The flag is part of the static launch-command template.
We check the source of ``load_model`` rather than mocking the whole call
chain (GPU probing, GGUF stat, etc.): the flag is a literal in one place and
any regression must delete it, which a text search catches.
"""
assert '"--no-context-shift"' in _load_model_source(), (
"llama-server must be launched with --no-context-shift so the "
"UI can surface a clean 'context full' error instead of silently "
"losing old turns to a KV-cache rotation."
)
def test_the_flag_is_emitted_unless_the_build_lacks_it():
"""The gate replaces the old "must be a literal in the base list" pin.
It used to sit unconditionally inside ``cmd = [...]``, which meant a stale
or user-supplied LLAMA_SERVER_PATH without the flag got it anyway and
exited on an unknown argument. It is now gated, but the gate FAILS OPEN:
the capability defaults to True everywhere, so an unreadable --help keeps
today's command and only a build whose help positively lacks the flag
drops it.
"""
source = _load_model_source()
assert 'cmd.append("--no-context-shift")' in source
assert (
'if _caps.get("supports_no_context_shift", True):' in source
), "the gate must default to True, so a failed probe still emits the flag"
# And the default really is True in both places the probe can return.
probe_src = inspect.getsource(llama_cpp_module.LlamaCppBackend.probe_server_capabilities)
assert '"supports_no_context_shift": True' in probe_src
assert "supports_no_context_shift = True" in probe_src
def test_the_base_cmd_list_still_leads_straight_into_the_context_flag():
"""-c must stay grouped with the base list.
auto-fit must omit -c entirely, because "-c 0" pins the full native context
and disables --fit's VRAM-based sizing, so the emission needs to stay where
that reasoning is visible.
"""
source = _load_model_source()
start = source.find("cmd = [")
assert start >= 0, "could not find the base cmd = [...] block"
rest = source[start:]
end_rel = -1
for line_start, line in _iter_lines_with_offset(rest):
if line_start == 0:
continue
if line.strip() == "]":
end_rel = line_start
break
assert end_rel > 0, "could not find end of cmd = [...] block"
# Wide enough to span the gated flags and their comments that now sit between
# the base list and -c; the point is that -c is still emitted here rather than
# somewhere else entirely.
after = rest[end_rel : end_rel + 2400]
assert '"-c"' in after, (
"-c must still be emitted near the base cmd list (omitted only in "
"auto-fit, where --fit sizes context)."
)
def test_flash_attention_drops_its_value_only_for_a_boolean_build():
"""Older builds take -fa as a bare boolean and read "on" as a positional.
That is an immediate "invalid argument" exit, not a degraded launch.
"""
value_form = "-fa, --flash-attn [on|off|auto] set flash attention"
boolean_form = "-fa, --flash-attn enable flash attention"
assert llama_cpp_module.LlamaCppBackend._flash_attn_takes_value(value_form) is True
assert llama_cpp_module.LlamaCppBackend._flash_attn_takes_value(boolean_form) is False
# Fail open when the help says nothing about it, since the pinned prebuilt
# is the value form and guessing wrong there breaks the supported path.
assert llama_cpp_module.LlamaCppBackend._flash_attn_takes_value("-m, --model FNAME") is True
assert llama_cpp_module.LlamaCppBackend._flash_attn_takes_value("") is True
def _iter_lines_with_offset(text: str):
"""Yield (offset, line) pairs over ``text`` without losing offsets."""
offset = 0
for line in text.splitlines(keepends = True):
yield offset, line
offset += len(line)
Reference in a new issue View git blame Copy permalink