chore(mypy): enable strict type checking for llama_stack_api module#5480
Open
Elbehery wants to merge 2 commits intoogx-ai:mainfrom
Open
chore(mypy): enable strict type checking for llama_stack_api module#5480Elbehery wants to merge 2 commits intoogx-ai:mainfrom
Elbehery wants to merge 2 commits intoogx-ai:mainfrom
Conversation
Elbehery
requested review from
ashwinb,
bbrowning,
cdoern,
ehhuang,
franciscojavierarceo,
leseb,
mattf and
raghotham
as code owners
meta-cla
Bot
added
the
CLA Signed
Elbehery
force-pushed
the
20260408_mypy_type_checks_lls_api
branch
5 times, most recently
from
4b184ea to
fa12f93
Compare
leseb
requested changes
Apr 13, 2026
Contributor
|
This pull request has merge conflicts that must be resolved before it can be merged. @Elbehery please rebase it. https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork |
Elbehery
force-pushed
the
20260408_mypy_type_checks_lls_api
branch
from
fa12f93 to
ece6543
Compare
cdoern
reviewed
Apr 13, 2026
| else: | ||
| if llama_model not in ALL_HUGGINGFACE_REPOS_TO_MODEL_DESCRIPTOR: | ||
| # Filter out None keys for the error message | ||
| valid_models = [k for k in ALL_HUGGINGFACE_REPOS_TO_MODEL_DESCRIPTOR.keys() if k is not None] |
Collaborator
There was a problem hiding this comment.
would a more proper solution here be to make sure the list doesn't have None entries? I feel like it shouldn't.
| "psycopg2", | ||
| "psycopg2.extras", | ||
| "psycopg2.extensions", | ||
| "sentence_transformers", |
Contributor
Author
There was a problem hiding this comment.
sentence_transformers was added to the mypy ignore list because:
- embedding_mixin.py was removed from the mypy exclude list as part of Phase 1 type checking improvements
- That file imports from the sentence_transformers library (line 16: from sentence_transformers import SentenceTransformer)
- The sentence_transformers library lacks proper type stubs/annotations
Adding it to [[tool.mypy.overrides]] allows mypy to:
- ✅ Type-check the rest of embedding_mixin.py
- ✅ Skip type checking for imports from sentence_transformers (treating them as Any)
This is a standard pattern when enabling strict type checking for code that depends on third-party libraries without type
annotations. The alternative would be keeping embedding_mixin.py in the exclude list, which defeats the purpose of the mypy
improvements.
Elbehery
force-pushed
the
20260408_mypy_type_checks_lls_api
branch
from
ece6543 to
b09bd81
Compare
leseb
approved these changes
Apr 16, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to a conflict with the base branch
Elbehery
force-pushed
the
20260408_mypy_type_checks_lls_api
branch
from
b09bd81 to
c0890c7
Compare
Contributor
Author
|
@leseb rebased and updated, please re-approve and re-add 👍🏽 |
github-merge-queue
Bot
removed this pull request from the merge queue due to a conflict with the base branch
Contributor
|
This pull request has merge conflicts that must be resolved before it can be merged. @Elbehery please rebase it. https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork |
Elbehery
force-pushed
the
20260408_mypy_type_checks_lls_api
branch
from
c0890c7 to
e5b62e2
Compare
Remove 12 files from mypy exclude list (Phase 1 complete): - llama_stack_api: models.py, prompts.py (2 files) - core/utils: serialize.py, exec.py (2 files) - providers/utils/inference: 6 files (model_registry, embedding_mixin, etc.) - log.py (1 file) Key changes: - model_registry.py: Add assertion to narrow provider_resource_id type - embedding_mixin.py: Add config attribute type annotation - pyproject.toml: Add sentence_transformers to mypy ignore list Signed-off-by: Mustafa Elbehery <melbeher@redhat.com>
…del registry. This change filters out models without HuggingFace repos at dictionary creation time rather than at every usage site, ensuring the registry only contains valid string keys and eliminating the need for None checks when accessing the dictionary. Signed-off-by: Mustafa Elbehery <melbeher@redhat.com>
Elbehery
force-pushed
the
20260408_mypy_type_checks_lls_api
branch
from
e5b62e2 to
2859530
Compare
Contributor
Author
|
cc @leseb |
Contributor
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
3 participants
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.
What does this PR do?
Enables strict type checking for llama_stack_api module
Key changes: