{{ message }}
Put back the AppleDouble GGUF filters #9074 reverted, and guard them - #9353
Merged
Conversation
Backend CI's 3.13 leg has six failures on main beyond the two in #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 #8919 ------------------------------------------------------------------------ #8919, "never pick a macOS AppleDouble sidecar as a GGUF", touched 49 files. 18b97f8 ("keep and search the turns rolling context evicts", #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 #8919 landed and merged whole-file: #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 #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 #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 #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 #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 ------------------------------------------------------------------------ #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", #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.
for more information, see https://pre-commit.ci
Member
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Put back the AppleDouble GGUF filters #9074 reverted, and guard them
Backend CI's 3.13 leg has six failures on main beyond the two in #9348. Three
separate causes, all of them in the tests or in a merge resolution rather than
in anything a PR meant to change.
#8919, "never pick a macOS AppleDouble sidecar as a GGUF", touched 49 files.
18b97f8 ("keep and search the turns rolling context evicts", #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 #8919 landed and merged whole-file: #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 #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 #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 #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
#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.
#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.
28b8880 ("compact a chat by resetting the epoch", #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
toolskey,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.