Studio: never pick a macOS AppleDouble sidecar as a GGUF by sts-change · Pull Request #8919 · unslothai/unsloth · GitHub
Skip to content

Studio: never pick a macOS AppleDouble sidecar as a GGUF - #8919

Merged
oobabooga merged 5 commits into
unslothai:mainfrom
sts-change:fix/appledouble-gguf-sidecars
Aug 19, 2026
Merged

Studio: never pick a macOS AppleDouble sidecar as a GGUF#8919
oobabooga merged 5 commits into
unslothai:mainfrom
sts-change:fix/appledouble-gguf-sidecars

Conversation

@sts-change

@sts-change sts-change commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Fixes #8566 — the GGUF-picker half that was left open after #8574.

Motivation

On volumes that cannot store Mac file metadata (exFAT, FAT32, many network shares), macOS writes a ~4 KB AppleDouble file beside every file, named ._<name> and holding magic 0x00051607. The companion carries the described file's extension, so model-Q4_K_M.gguf gets ._model-Q4_K_M.gguf. Studio accepted any name ending in .gguf and took the first match in sorted order, and . sorts ahead of alphanumerics, so the sidecar won every time.

llama-server then opened 4 KB of Finder metadata, found no GGUF header, and exited. The failure classifier had no branch for that output, so it fell through to its catch-all and blamed the file or the user's memory — which is why a 1–3B model on a 16 GB machine reported an out-of-memory-shaped error. The same selection bug picked ._mmproj-* for vision models, and every ._ part of a split model.

The companion answers every name-shaped question the way the real file does, so nothing about this is specific to GGUF. pathlib.Path.glob matches these names where the older glob.glob hid them, which is why some call sites were affected and others were not:

  • training runs died at data load, and LoRA training was refused over a file the user cannot see;
  • repo_ships_transformers_weights accepted ._consolidated.safetensors by extension, and the ignore-pattern builder then stripped every consolidated* file — a Mistral-style repo downloaded "successfully" with no weights on disk;
  • expected_files_from_snapshot_dir baked companions into the completion contract, so a finished download read as partial forever once macOS cleaned them up;
  • the RAG folder scan embedded AppleDouble bytes as retrievable chunks, cited in answers.

Changes

  • One rule, and it reads bytes. The earlier commits on this branch treated every ._-prefixed name as not-a-GGUF. That refuses a file for its name alone, and a user is free to name a real model ._model.gguf — on a volume with native xattrs nothing else would ever create that name, and the file is a perfectly good GGUF. The rule is now: a path is Finder metadata when its name carries the ._ prefix and its first four bytes are the AppleDouble magic. Only ._ names are ever opened, so a scan costs one extra read per companion rather than one per file. The predicates live in utils.paths.path_utils, beside the other path questions.
  • Names with no bytes are settled by pairing instead. A remote listing offers nothing to read, so a ._x is dropped only when its subject x is present in the same listing and the same directory; a sole candidate survives whatever it is called. GGUF passes a subject key so a split quant's shards count as one family.
  • Where consumers share a walk, the filter went into the walk. cached_repo_files for the cache inventory and the model routes, _safe_dataset_image_path for the dataset image serve, caption and delete endpoints, and dataset_files_in_dir for local dataset resolution across GPU, MLX and embedding training.
  • The failure message is named. invalid magic characters ... expected 'GGUF' is now classified ahead of the generic invalid-file/OOM fallback. llama.cpp formats the four bytes it found with %c, so a binary header arrives as unprintable characters and nothing matched the line. The message gives the model path for context without claiming that path is the bad file: llama-server reports this while opening the vision projector and the drafter too, and only the main model's path reaches the classifier. Memory is never mentioned.

Deliberately out of scope: size- and count-only cosmetics that break nothing — a few KB on a VRAM estimate, a progress bar reading 99.9%, a "last downloaded" sort order.

Tests

test_appledouble_guards.py covers what every consumer shares rather than one case per call site: the predicates, the shadow pairing including a split quant's shard family, the cache walk that follows a snapshot entry to the blob it links to, the dataset walk, and the two selections a sidecar used to win outright — a drafter budget priced off a shared basename, and consolidated* weights hidden by their companion. The classification path is covered in the suite that already owns it: the failure is reported as not-a-GGUF, it does not name an ordinary main model as the bad file, and a dyld failure still outranks it.

The one-line guards at individual call sites — upload block quota, dataset image gate, immediate weight file, RAG folder scan, remote code scan, delete cleanup, companion family names, named dataset file, and the cached-files walk in routes/models — are not each covered by a test here. Each was verified during development by reverting the guard and re-running the suite. They are visible in review, but CI will not notice if a later edit drops one; flagging that rather than leaving it implied.

Run on macOS from studio/backend:

  • the suites this change touches: 698 passed
  • hub/tests/: 554 passed, 1 pre-existing failure (a case-insensitive-filesystem assumption)
  • the full backend suite, this branch versus 6f443b5c, same invocation, run sequentially: 288 failures on each side, identical sets — the branch introduces none and fixes none. This environment carries a large pre-existing failure count (macOS host, blocked network), so a raw pass/fail number proves nothing; the set comparison is the instrument.

On volumes that cannot store Mac file metadata -- exFAT, FAT32, many network
shares -- macOS writes a ~4 KB AppleDouble file named `._<name>.gguf` beside
every model. Studio accepted any name ending in `.gguf` and took the first
match in sorted order, and `._` sorts ahead of letters, so the sidecar won.
llama-server then opened 4 KB of Finder metadata, found no GGUF header, and
exited. The failure classifier had no branch for that, so it fell through to
its catch-all and blamed the file or the user's memory -- which is why a 1-3B
model on a 16 GB machine reported an out-of-memory-shaped error. The same
selection bug picked `._mmproj-*` for vision models and every `._` part of a
split model.

Three changes, matching the three ways this surfaced:

- Filename rule: `._` prefixed names are no longer GGUFs. The canonical rule
  lives in `hub.utils.gguf.is_gguf_filename`; `utils.models.model_config` and
  `core.inference.llama_cpp` carry documented mirrors because core deliberately
  does not import from hub. Every scanner, variant resolver, companion picker
  and cache-reuse path now goes through one of the three.
- Header check: the metadata reader records whether the file actually starts
  with `GGUF`, so an invalid file is refused before llama-server is spawned.
  A definite non-GGUF header is kept distinct from an unreadable one, so a
  truncated but valid GGUF still falls through to llama-server as before.
- Failure message: `invalid magic characters ... expected 'GGUF'` is now
  classified ahead of the generic invalid-file/OOM fallback, and names Finder
  metadata explicitly when the path is AppleDouble. Memory is never mentioned.

Fixes unslothai#8566

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e50dd737e9

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread studio/backend/core/inference/llama_cpp.py Outdated
@Lyxot Lyxot self-assigned this Aug 15, 2026
@Lyxot

Lyxot commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b398867769

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread studio/backend/utils/paths/path_utils.py
@Lyxot
Lyxot force-pushed the fix/appledouble-gguf-sidecars branch from b398867 to e51f3a1 Compare August 15, 2026 17:18
@Lyxot

Lyxot commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: e51f3a1532

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Lyxot
Lyxot force-pushed the fix/appledouble-gguf-sidecars branch from e51f3a1 to 59ae87b Compare August 15, 2026 17:42
@Lyxot

Lyxot commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 59ae87b708

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread studio/backend/core/inference/diffusion_families.py Outdated
@Lyxot
Lyxot force-pushed the fix/appledouble-gguf-sidecars branch from 59ae87b to d586aea Compare August 15, 2026 18:03
@Lyxot

Lyxot commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d586aea8f8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread studio/backend/utils/security/remote_code_scan.py Outdated
@Lyxot
Lyxot force-pushed the fix/appledouble-gguf-sidecars branch from d586aea to 8f09175 Compare August 15, 2026 18:16
@Lyxot

Lyxot commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: 8f09175968

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@danielhanchen

Copy link
Copy Markdown
Member

Reviewed this against #8566 and it holds up well. The design is right, and I want to record the checks rather than just say "looks good".

Verified

Reproduced the bug on main first, so the fix had something concrete to be measured against:

pick_best_gguf(['._Muse-12B-Q4_K_M.gguf', 'Muse-12B-Q4_K_M.gguf']) -> '._Muse-12B-Q4_K_M.gguf'
pick_best_gguf(['._M-00001-of-00002.gguf', 'M-00001-of-00002.gguf', ...]) -> '._M-00001-of-00002.gguf'

On this branch both return the real file, and an end-to-end run over a directory containing actual AppleDouble bytes (00 05 16 07) leaves only the real model in _iter_ggufs.

The predicate is the part I was most prepared to argue with, and it is correct. is_appledouble_metadata is name and magic, not name alone, so a legitimate GGUF that happens to be called ._legit-Q4_K_M.gguf survives selection. I checked that specifically:

is_appledouble_metadata('._legit-Q4_K_M.gguf' with GGUF bytes) -> False   # kept, correct

Missing, empty and 2-byte files all return False rather than raising, so an unreadable candidate is not mistaken for metadata. Splitting the work between the magic-checking predicate for local paths and the name-only drop_shadowed_appledouble_names for remote listings is the right call, since you cannot read bytes off a Hub listing.

Full studio/backend/tests suite at this branch versus its merge base: 180 failures at both, the same 180, zero new and zero fixed, with 12 more tests passing here. No regressions.

One thing worth fixing

tests/test_appledouble_guards.py is order-dependent. Run it after hub/tests/test_model_services.py and two tests fail:

FAILED tests/test_appledouble_guards.py::test_a_folder_holding_only_metadata_is_not_a_model
FAILED tests/test_appledouble_guards.py::test_a_drafter_budget_prices_the_largest_file_of_a_shared_basename
E   ImportError: cannot import name 'File' from '<unknown module name>'
routes/training.py:16: ImportError

The function-level from routes.models import ... pulls routes/__init__ and then routes.training, which imports from a fastapi an earlier test has already stubbed. Nothing is wrong with the product code, and that import line is identical on your branch and the base, so this is the new test tripping over pre-existing stubbing rather than anything you introduced.

It matters because any job that runs "the PR's changed test files together" hits it. I ran exactly that across ubuntu-latest, macos-14 and windows-latest and got the same two failures on all three. Importing routes.models at module scope, or reaching the two helpers without dragging in routes/__init__, should settle it.

For completeness, the third failure in that batch, test_llama_cpp_start_failure_classification.py::TestDiffusionArchitectures::test_no_runnable_video_arch_is_tagged_unsupported_by_the_picker, reproduces at the merge base too, so it is not yours.

Smaller suggestion

The new classifier branch is a real improvement over the memory catch-all. The wording ends on "Re-download the model, or pick a different file", which is right for a generic non-GGUF but is not the remedy for the case that prompted this. For the exFAT reporter the fix is dot_clean -m <path>, or reformatting as APFS if the drive is Mac-only. Since the branch fires for any non-GGUF, I would keep the current text and add a targeted line when the basename starts with ._, rather than making the general message macOS-specific.

Not blocking

Windows Unsloth UI CI went red on my staging run, on the Playwright update-banner checks (8 of 1422, whisper card elements resolving to None). This PR touches no frontend or STT files, so I read that as unrelated flake.

Malicious scan clean, static and both LLM passes. Nice piece of work, and thanks for chasing the AppleDouble angle. Happy to merge once the test-isolation point is sorted.

On a volume without native extended attributes -- exFAT, FAT, most SMB and NFS shares -- macOS keeps a file's xattrs and resource fork in a companion beside it, named `._<name>` and holding AppleDouble data with magic `0x00051607`. The companion carries the described file's extension, so `model-Q4_K_M.gguf` gets `._model-Q4_K_M.gguf` and `train.jsonl` gets `._train.jsonl`. It answers every name-shaped question the way the real file does, and because `.` sorts ahead of alphanumerics, first-match-wins selection prefers it. `pathlib.Path.glob` matches these names where the older `glob.glob` hid them, which is why some call sites were affected and others were not.

The reported symptom is a GGUF model that will not load from a cache on exFAT, but the same defect reaches datasets, images, video, PDFs, `.py` sources and Arrow caches. Training runs died at data load; LoRA training was refused because of a file the user cannot see; `repo_ships_transformers_weights` accepted `._consolidated.safetensors` by extension and the ignore-pattern builder then stripped every `consolidated*` file, so a Mistral-style repo downloaded "successfully" with no weights on disk; `expected_files_from_snapshot_dir` baked companions into the completion contract, so a finished download read as partial forever once macOS cleaned them up; and the RAG folder scan embedded AppleDouble bytes as retrievable chunks cited in answers.

This replaces the filename rule this branch carried earlier, which treated every `._`-prefixed name as not-a-GGUF. That rule refuses a file for its name alone, and a user is free to name a real model `._model.gguf` -- on a volume with native xattrs nothing else would ever create that name, and the file is a perfectly good GGUF. Only the bytes can tell the two apart.

`utils/appledouble.py` holds the format-agnostic primitives. A path is metadata only when its name carries the `._` prefix *and* its first four bytes are the AppleDouble magic. The prefix alone is never enough: a user's own `._model.gguf` is a real model and must stay loadable. Only `._` names are ever opened, so a scan costs one extra read per companion rather than one per file.

Remote listings carry names with no bytes to read, so they are settled by pairing instead: a `._x` is dropped only when its subject `x` is present in the same listing, which leaves a sole candidate usable whatever it is called. GGUF passes a subject key so a split quant's shards count as one family.

Where consumers shared a walk, the filter went into the walk rather than into each caller: `cached_repo_files` for the cache inventory and model routes, `_safe_dataset_image_path` for the dataset image serve, caption and delete endpoints, and `dataset_files_in_dir` for local dataset resolution across GPU, MLX and embedding training.

One error message changes with it. llama.cpp prints `invalid magic characters: '<4 bytes>', expected 'GGUF'` when the file it opened is not a GGUF, formatting the bytes it found with `%c`, so a binary header arrives as unprintable characters. Nothing in the classifier matched that line, so it fell through to the generic "check that the GGUF file is valid and you have enough memory" -- which sent people to free memory that was never short, over a file that was never a model. That failure is now named. The message says the file is not a GGUF and gives the model path for context without claiming that path is the bad file: llama-server reports this while opening the vision projector and the drafter too, and only the main model's path reaches the classifier. The rule sits after the loader diagnostics, since a dyld failure explains why the wrong bytes were reached and stays the more specific answer when both appear.

Fixes unslothai#8566
@Lyxot
Lyxot force-pushed the fix/appledouble-gguf-sidecars branch from 8f09175 to c7ec308 Compare August 17, 2026 13:32
@Lyxot

Lyxot commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Thanks — the reproduction on main and the three-platform run are exactly the evidence I wanted and did not have.

Test isolation

Fixed, but not where you suggested, because the first suggestion made it worse and I want to show the measurement rather than assert it.

Hoisting to module scope turns the two test failures into a whole-module collection error:

tests/test_appledouble_guards.py:36: in <module>
    from routes.inference import _cached_repo_gguf_bytes
routes/__init__.py:8: in <module>
    from routes.training import router as training_router
E   ImportError: cannot import name 'File' from '<unknown module name>'

Reaching the helpers without routes/__init__ does not work either — I pre-seeded a namespace routes package so the submodules import directly, and they fail on their own imports: routes/inference.py on File, routes/models.py on status.

The cause is hub/tests/conftest.py, which did sys.modules.setdefault("fastapi", SimpleNamespace(...)) at module scope with a seven-symbol stub and never removed it. setdefault stubs whenever the module is merely not yet imported, not when it is absent, so collecting any hub test replaced fastapi process-wide for every later import.

That makes this pre-existing and repo-wide rather than specific to my file — 163 files under tests/ import routes.*. On the merge base 6f443b5cc, four of five sampled pre-existing files collapse identically:

tests/test_local_model_format.py             1 error
tests/test_inference_status_loaded_gguf.py   1 error
tests/test_diffusion_routes.py               1 error
tests/test_rag_preview.py                    1 error

So I fixed the cause: the stub now installs only when the real import genuinely fails. pydantic needed the same treatment first — stubbed ahead of fastapi, it made the real fastapi import fail, so guarding fastapi alone still fell through to the stub.

hub/tests/ alone                                554 passed, 1 pre-existing failure   (unchanged)
test_model_services.py + test_appledouble_guards.py    270 passed
all changed test files, hub first                      483 passed
the four pre-existing files above                      now pass

Reverting the conftest to bare setdefault brings the reported failure straight back, so the fix is load-bearing.

Classifier wording

Taken as you framed it. The generic text is unchanged and a targeted line replaces only the closing remedy when the model path starts with ._:

This volume has no native extended attributes, so macOS keeps them in "._" companions: run "dot_clean -m" on the folder to remove them, or keep models on an APFS disk.

Phrased about the volume rather than the file on purpose, since llama-server may have choked on the projector or the drafter while the main model merely happens to sit on the same disk — dot_clean is right either way. An ordinary path keeps "Re-download the model, or pick a different file". Both directions are asserted, and I mutation-checked them: forcing either remedy unconditionally fails exactly one test.

The two I am leaving

test_no_runnable_video_arch_is_tagged_unsupported_by_the_picker and the Windows Playwright banner failures — agreed, both reproduce independently of this branch and neither is in its scope.

Head is now c7ec30869.

@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Aug 19, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Aug 19, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Aug 19, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Aug 19, 2026
@danielhanchen

Copy link
Copy Markdown
Member

Confirmed this still hits is_gguf_filename in studio/backend/hub/utils/gguf.py, where an AppleDouble sidecar passes the extension test and sorts ahead of the real quant, and that the invalid-magic output has no classifier branch today. Will get this reviewed.

@danielhanchen

Copy link
Copy Markdown
Member

@Lyxot could you take a look at this one?

@oobabooga

Copy link
Copy Markdown
Member

Both points are addressed on the current head (4cf69be5). They were fixed in Lyxot's c7ec3086, which is dated after your review, so the state you tested is not the state on the branch now.

Test isolation. Reproduced exactly at 8f091759, the commit you reviewed:

$ pytest hub/tests/test_model_services.py tests/test_appledouble_guards.py -q
FAILED tests/test_appledouble_guards.py::test_a_drafter_budget_prices_the_largest_file_of_a_shared_basename
FAILED tests/test_appledouble_guards.py::test_a_folder_holding_only_metadata_is_not_a_model
routes/training.py:16: ImportError
2 failed, 269 passed

Same invocation at head: 274 passed.

The fix is in hub/tests/conftest.py rather than in the test. sys.modules.setdefault installed the stub whenever the module was merely not imported yet, and the fastapi stub carries no File, so the next test to import routes died on that name. _stub_unless_installed now imports the module first and stubs only on ImportError. studio/backend/requirements/studio.txt pins fastapi 0.141.1 and pydantic 2.13.4, so in a normal environment no stub is installed at all. Worth noting for scope: no workflow under .github/workflows runs hub/tests, and pytest tests/ never collects hub/tests/conftest.py, so this only ever affected an invocation that mixes the two, which is the one you used.

Classifier wording. That is what head does. The branch keeps "Re-download the model, or pick a different file." and substitutes the dot_clean -m / APFS remedy only when the basename starts with ._ (core/inference/llama_cpp.py:12002). TestANonGgufFile asserts both directions, including that an ordinary main model path keeps the generic remedy.

For the record on regressions: studio/backend/tests at head against the merge base e99dfe5a, same invocation on Linux, gives 27351 passed / 11 failed against 27340 passed / 10 failed. Identical error sets, and identical failure sets apart from test_stream_completion_timeout_is_absolute_despite_keepalives, an asyncio.wait_for(..., timeout=1) bound that loses its slot under 8 workers and passes 3/3 in isolation.

All four Codex items are correct catches and all four are fixed at head; I reproduced each one before agreeing to it. Separately, every org workflow on this PR is sitting at action_required, so nothing but pre-commit.ci has run against 4cf69be5.

@oobabooga

Copy link
Copy Markdown
Member

@oobabooga
oobabooga merged commit 8d6e969 into unslothai:main Aug 19, 2026
1 check passed
danielhanchen added a commit that referenced this pull request Aug 20, 2026
* Keep path_utils off PEP 604, which the 3.9 floor gate rejects

`Core` has been failing on main at 'python floor compatibility (HARD GATE)',
all three HF/TRL cells:

  36 studio files now evaluate PEP 604 unions on the floor, up from 35

The 36th is studio/backend/utils/paths/path_utils.py, from #8919. That change is
right; it just added

  subject_key: Callable[[str], object] | None = None

to a module with no `from __future__ import annotations`, so the annotation is
evaluated at import and `X | None` is a TypeError on the declared 3.9 floor.

Uses Optional[...], which the module already imports, rather than adding the
future import. Five other modules import this one, and deferring every
annotation in it is a wider change than the one union needs -- the ratchet's own
docstring warns that converting a file wants Studio booted, because FastAPI
resolves annotations when it builds endpoints.

Verified on main: offenders back to 35 against a debt of 35, the whole
test_python39_compatibility.py suite passes (8), and the AppleDouble guards the
original change added still pass (9).

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

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

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
meefs pushed a commit to meefs/unsloth that referenced this pull request Aug 20, 2026
…ard them (unslothai#9353)

* Put back the AppleDouble GGUF filters unslothai#9074 reverted, and guard them

Backend CI's 3.13 leg has six failures on main beyond the two in unslothai#9348. Three
separate causes, all of them in the tests or in a merge resolution rather than
in anything a PR meant to change.

1. llama_cpp.py lost every line of unslothai#8919
------------------------------------------------------------------------
unslothai#8919, "never pick a macOS AppleDouble sidecar as a GGUF", touched 49 files.
18b97f8 ("keep and search the turns rolling context evicts", unslothai#9074) reverted
all five of its hunks in core/inference/llama_cpp.py and nothing else. That is
the signature of a branch cut before unslothai#8919 landed and merged whole-file: unslothai#9074
is a rolling-context PR, its diff carries no replacement for any of this, and it
did not revert the tests, which is the only reason CI said anything at all.

Checked the rest of unslothai#8919 line by line against main: of the 49 files it changed,
llama_cpp.py is the only one that lost anything. All five hunks are restored
here, and the file now contains every line unslothai#8919 added.

Four of the five are the selection sites, and they are the half that was silent:

  _gguf_snapshot_files      a local walk now skips the companion on its bytes
  _pick_mmproj              "._mmproj-F16.gguf" satisfied the F16 preference and
                            sorted ahead of the real adapter
  _pick_dspark              every GGUF under dspark/ qualifies, so a sidecar
                            ranked equal to its sibling and sorted first; also
                            back at module level, where unslothai#8919 put it because it
                            is handed a live repo listing as well as a snapshot
  the HF list_repo_files    a repo listing has no bytes to read

The fifth is the one CI caught: the "invalid magic characters" branch in
_classify_llama_start_failure. Without it a user who points llama-server at a
"._model.gguf" sidecar gets "Check that the GGUF file is valid and you have
enough memory" and goes off to free memory they already have, which is issue
unslothai#8566 exactly. Restored verbatim at its original anchor, below the dyld branch
so test_a_dyld_failure_still_outranks_it keeps holding.

Five new tests in test_appledouble_guards.py cover the four selection sites
behaviourally, including that _pick_dspark is reachable from module scope, since
nesting it back inside the method is how it was reverted. Each also pins that a
file a user genuinely named "._something" still resolves: nothing may be refused
for its name alone.

Mutation-tested by restoring llama_cpp.py to its current main state: 7 failed,
all five new guards plus the two that were already red.

2. The refactor guard baseline
------------------------------------------------------------------------
unslothai#9074 added RAG_SEARCH_TOOLS to core/inference/tool_call_parser, which is one of
the two strict modules the guard runs with additions_matter, so a new public name
there is a deliberate re-baseline by design. The symbol is correct: three modules
import it, and test_conversation_recall_injection.py already pins its value.

Re-baselined through the tool's own `snapshot`, then trimmed to just this entry.
The full snapshot also absorbed 52 unreviewed new names in core.inference.llama_cpp,
3 in safetensors_agentic and 151 lines of patch_targets churn. Those are additive
drift the guard tolerates on purpose, so recording them fixes nothing and pins
symbols nobody looked at.

Mutation-tested: an added throwaway public symbol still turns both tests red, so
the strict-addition behaviour survived the re-baseline.

3. The research opt-out payload
------------------------------------------------------------------------
28b8880 ("compact a chat by resetting the epoch", unslothai#9162) added tools_withheld
to the generation kwargs. test_the_opt_out_changes_nothing_a_default_install_does
compared the two payloads whole, which was right when every kwarg was
model-facing.

tools_withheld is not. Its only consumer is _can_reset_epoch, which picks a
compaction strategy; it never reaches the prompt, the sampling params or the tool
catalogue. And it has to differ: without the opt-out a compacted thread can still
re-admit search_conversation through the checkpoint-repair branch, so resetting
the epoch is safe, while with the opt-out that repair is closed on this turn and
every identical turn after it, so a reset would strand the epoch behind a tool
that never arrives. Forcing the two equal is what would break Deep Research,
which sends a real thread_id.

The 17 model-facing fields are identical and neither side carries a `tools` key,
so the asymmetry this test exists to catch is not present. Rather than just
excluding the key, it is now pinned in both directions plus an explicit
no-tool-catalogue assertion, so the file catches more than before.

Mutation-tested: pinning tools_withheld = False in routes/inference.py fails the
new assertion for both parameters.

Verification
------------------------------------------------------------------------
test_appledouble_guards.py, test_llama_cpp_start_failure_classification.py,
test_refactor_guard.py, test_research_internal_call_tool_gate.py and
hub/tests/test_model_services.py: 508 passed, from 6 failed.
Wider sweep over tests/test_llama*.py and the hub model services: no collateral.

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

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

---------

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Unsloth Desktop on macOS M4: llama-server fails to start & excessive idle RAM usage

4 participants