Skip to content

DO NOT MERGE: discussion needed #368

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

DO NOT MERGE: discussion needed #368

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions src/app/endpoints/query.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,13 +171,14 @@ def select_model_and_provider_id(
},
) from e

llama_stack_model_id = f"{provider_id}/{model_id}"
# Validate that the model_id and provider_id are in the available models
logger.debug("Searching for model: %s, provider: %s", model_id, provider_id)
if not any(
m.identifier == llama_stack_model_id and m.provider_id == provider_id
for m in models
):

def check_model(m):
logger.debug("Available model - model_identifier: %s, provider_model_id: %s, provider_id: %s", m.identifier, m.provider_resource_id, m.provider_id)
return m.identifier == model_id and m.provider_id == provider_id

if not any(check_model(m) for m in models):
Comment on lines +176 to +181
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Trailing whitespace and mypy untyped local function

  • Trailing whitespace on Line 176 and 180 (pylint C0303).
  • Local function check_model lacks annotations and is called in a typed context (mypy no-untyped-def, no-untyped-call).

Either annotate it (param type and return type) or remove it. The proposed refactor removes it entirely; alternatively:

-    def check_model(m):
+    def check_model(m: object) -> bool:
         logger.debug(
             "Available model - model_identifier: %s, provider_model_id: %s, provider_id: %s",
             m.identifier, m.provider_resource_id, m.provider_id
         )
         return m.identifier == model_id and m.provider_id == provider_id
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def check_model(m):
logger.debug("Available model - model_identifier: %s, provider_model_id: %s, provider_id: %s", m.identifier, m.provider_resource_id, m.provider_id)
return m.identifier == model_id and m.provider_id == provider_id
if not any(check_model(m) for m in models):
def check_model(m: object) -> bool:
logger.debug("Available model - model_identifier: %s, provider_model_id: %s, provider_id: %s",
m.identifier, m.provider_resource_id, m.provider_id)
return m.identifier == model_id and m.provider_id == provider_id
🧰 Tools
🪛 GitHub Actions: Type checks

[error] 177-177: mypy: Function is missing a type annotation. (no-untyped-def)

[error] 181-181: mypy: Call to untyped function "check_model" in typed context. (no-untyped-call)

🪛 GitHub Actions: Python linter

[error] 176-176: Pylint: Trailing whitespace (C0303)

[error] 178-178: Pylint: Line too long (155/100) (C0301)

[error] 180-180: Pylint: Trailing whitespace (C0303)

🤖 Prompt for AI Agents 
In src/app/endpoints/query.py around lines 176 to 181, remove trailing
whitespace on lines 176 and 180 to fix pylint C0303. Add type annotations to the
local function check_model by specifying the parameter type and return type to
satisfy mypy no-untyped-def and no-untyped-call checks. Alternatively, consider
removing the local function if it is not necessary.

message = f"Model {model_id} from provider {provider_id} not found in available models"
logger.error(message)
raise HTTPException(
Expand All @@ -188,7 +189,7 @@ def select_model_and_provider_id(
},
)

return llama_stack_model_id, provider_id
return model_id, provider_id


def _is_inout_shield(shield: Shield) -> bool:
Expand Down
Loading