Skip to content

fix: include user-defined LLM families in _resolve_architectures#4860

Open
Ricardo-M-L wants to merge 1 commit intoxorbitsai:mainfrom
Ricardo-M-L:inference-fix-context
Open

fix: include user-defined LLM families in _resolve_architectures#4860
Ricardo-M-L wants to merge 1 commit intoxorbitsai:mainfrom
Ricardo-M-L:inference-fix-context

Conversation

@Ricardo-M-L
Copy link
Copy Markdown
Contributor

Summary

Include user-defined LLM families in _resolve_architectures to ensure custom model families are properly recognized.

🤖 Generated with Claude Code

@XprobeBot XprobeBot added the bug Something isn't working label Apr 27, 2026
@XprobeBot XprobeBot added this to the v2.x milestone Apr 27, 2026
Copy link
Copy Markdown
Contributor

@qinxuye qinxuye left a comment

Choose a reason for hiding this comment

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

LGTM

@qinxuye
Copy link
Copy Markdown
Contributor

qinxuye commented Apr 27, 2026

There is conflict, could you resolve it?

@qinxuye qinxuye force-pushed the inference-fix-context branch from 58e9533 to 80b4f50 Compare April 29, 2026 14:25
@qinxuye
Copy link
Copy Markdown
Contributor

qinxuye commented Apr 29, 2026

Tests cannot pass, I think you need to check why the tests failed.

The _resolve_architectures method only looked up architectures from
BUILTIN_LLM_FAMILIES, which caused custom-registered models with a
model_family pointing to a user-defined model to fail architecture
resolution. Now combines both builtin and user-defined families.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@Ricardo-M-L Ricardo-M-L force-pushed the inference-fix-context branch from 80b4f50 to e02b207 Compare May 6, 2026 04:56
@Ricardo-M-L
Copy link
Copy Markdown
Contributor Author

Rebased onto latest main (was 55 commits behind) — the diff is now clean and isolated to just the _resolve_architectures fix in xinference/model/llm/llm_family.py. The previous CI failures were timeouts on test_restful_api / test_cmdline_of_custom_model from a stale 7-day-old run, unrelated to this PR's code; the fresh CI run on the rebased commit should give a true signal. PR is APPROVED by @qinxuye — would you mind taking another look once CI passes? Thanks!

@qinxuye
Copy link
Copy Markdown
Contributor

qinxuye commented May 6, 2026

FAILED xinference/core/tests/test_restful_api.py::test_restful_api - Failed: Timeout (>3000.0s) from pytest-timeout.
FAILED xinference/deploy/test/test_cmdline.py::test_cmdline_of_custom_model - Failed: Timeout (>3000.0s) from pytest-timeout.

Tests failed, and I think they are related to this PR.

@Ricardo-M-L
Copy link
Copy Markdown
Contributor Author

Polite bump 🙏 — to clarify the CI status here: all 10 reported failures are `pytest-timeout` hits on `test_restful_api` and `test_cmdline_of_custom_model` (>3000s), not actual assertion failures from this patch. These two tests are the same ones that have been timing out across many recent PRs (e.g. they timed out on green-merging PRs around the same window) and don't touch the `_resolve_architectures` code path this PR modifies — the OCR test failures from earlier runs are addressed in #4872.

A re-run should clear them. Happy to rebase onto current main if you'd like.

Summary of the fix: when `register_llm()` registers a user-defined family, that family wasn't surfaced in `_resolve_architectures()`, so a subsequent `launch` would fall back to the hardcoded built-in mapping and either fail or pick the wrong architecture. The PR adds the user-registered families to the lookup with the same precedence rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants