Repository navigation
Hub token identity tests: let the llama-backend stubs take the caller's token after #13005 - #13022
Conversation
…nslothai#12979 now passes unslothai#12979 (184e1fc) made `_llama_backend_active(model, token)` so the embedding resolver can gate the llama-server path on the caller's Hub token, and `_resolve_embedding_model_plan` now calls it with both. The two stub tables in `test_hub_token_caller_identity.py` still hand it a one-argument lambda, so every test built on `_st_resolver` / `_gguf_resolver` fails with `TypeError: <lambda>() takes 1 positional argument but 2 were given`: seven cases, red on Backend CI `(Python 3.13, a-k)` for 184e1fc and 04013e9. The stubs now accept the token and ignore it, which is what they did with the model. `tests/test_hub_token_caller_identity.py`: 197 passed. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_012DRiTpjxAymRtv9p8Xr2kd
…r's token unslothai#13005 made _resolve_embedding_model_plan pass the caller's Hub token to _llama_backend_active, because whether llama-server or sentence-transformers serves a private repo depends on files only that token can read. The stubs fixed above made the suite pass again but nothing checked the token still arrives; this test fails if the resolver drops it.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
danielhanchen
changed the title
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Review record for #13022, carried over from the mirror it was reviewed on: danielhanchen/unsloth-staging-review#556. Findings are quoted as posted and attributed to the account that posted them. The reactions shown are the triage recorded on the mirror; none were re-applied here. Commits are named as the mirror's, with this PR's equivalent where one was found. Round 1: reviewed mirror
|
What
Backend CI
(Python 3.13, a-k)has been red on main since #13005 (184e1fc3e):Cause
#13005 changed
_llama_backend_active(model)to_llama_backend_active(model, token)inroutes/settings.py, and_resolve_embedding_model_plannow passes the caller's Hub token. Whether llama-server or sentence-transformers serves a repo depends on reading itsmodules.json/config.json, which a private repo only shows to a token that reaches it. The product change is right.The two stub tables in
test_hub_token_caller_identity.py(_st_resolver,_gguf_resolver) still gave that name a one-argument lambda. Every test that reaches the resolver preamble raised before the behaviour under test ran.Change
_llama_backend_activewith a one-argument lambda;test_embedding_model_resolve.pyalready useslambda *_:.test_the_backend_probe_is_asked_with_the_callers_token: it pins that the resolver really passes the caller's token to the backend probe. The fixed stubs alone would also pass if a later change dropped the token.Verification
Run with Python 3.13, CPU torch, and the same installs as Backend CI.
test_hub_token_caller_identity.pytest_embedding_model_resolve.py+test_embedding_model_security_gate.py,-n 4