Skip to content

Fix: Preserve local and custom model names in parse_model_name#1648

Closed
ps2program wants to merge 4 commits intoconfident-ai:mainfrom
ps2program:fix/preserve-local-model-names-in-parse_model_name
Closed

Fix: Preserve local and custom model names in parse_model_name#1648
ps2program wants to merge 4 commits intoconfident-ai:mainfrom
ps2program:fix/preserve-local-model-names-in-parse_model_name

Conversation

@ps2program
Copy link
Copy Markdown
Contributor

Summary

This PR improves the parse_model_name utility in deepeval.models.utils by ensuring that model names with non-standard or custom prefixes like local/llama-3 are preserved and not truncated.

Problem

The current implementation of parse_model_name assumes that any string before a / is a provider name, and always strips it. This causes issues for locally hosted or custom models like local/llama-3, which get incorrectly truncated to just llama-3.

Fix

  • Added logic to preserve the full model name if the prefix is not in a known list of providers (e.g., "openai", "anthropic", "cohere").
  • Updated docstring for clarity.
  • Updated tests to:
    • Expect None for None input instead of raising TypeError.
    • Add new test case to verify local/custom model names are preserved (local/llama-3, custom/model-xyz).

Example Behavior

parse_model_name("openai/gpt-4o")       # "gpt-4o"
parse_model_name("local/llama-3")       # "local/llama-3"
parse_model_name("gpt-4o")              # "gpt-4o"
parse_model_name(None)                  # None

@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 2, 2025

Someone is attempting to deploy a commit to the Confident AI Team on Vercel.

A member of the Team first needs to authorize it.

- Adjust tests to expect None return for None input instead of error
- Add test cases for local/custom model prefixes that should be preserved
- Modify existing tests to align with updated logic for known vs unknown providers
@penguine-ip
Copy link
Copy Markdown
Contributor

hey @ps2program actually we're thinking about removing this functino entirely. The reason why this was introduced is because we wanted to fit the litellm format of "provider/model" but i'd better for us to just support lite llm directly. Was wondering can you do that PR instead?

@ps2program
Copy link
Copy Markdown
Contributor Author

Hi @penguine-ip,

Thanks for the context and the opportunity! I’ll go ahead and handle the removal of the parse_model_name function and update all related usages and tests.

Before proceeding, I wanted to confirm a few things:

  • Should we remove all usages of this function across the codebase?
  • Will removing this function affect LiteLLM compatibility in any way? Or are we now supporting LiteLLM directly at a different layer?
  • Is the system now fully ready to support full "provider/model" strings everywhere — including validation, UI display, logging, etc.?
  • Are there any places where we still need to parse or normalize model names?
  • Are there any legacy test cases or downstream integrations that rely on the parsed (provider-less) model names?

If we're removing the function entirely, here are the areas that currently rely on it and would need cleanup:

  • base_model.py
  • azure_model.py
  • anthropic_model.py
  • openai_model.py
  • Corresponding unit tests

Please confirm if it’s okay to proceed with removing it from all of the above as part of the PR.

@ps2program
Copy link
Copy Markdown
Contributor Author

ps2program commented Jun 3, 2025

Hi @penguine-ip , please have a look at
Updated PR:
Removed parse_model_name function and updated usages and tests to support full provider/model format

Description:

  • Removed the parse_model_name utility function, as per recent decision to fully support LiteLLM’s provider/model naming format directly without parsing or stripping prefixes.
  • Updated all code references to use model names in full format without modifications.
  • Modified and removed related tests for parse_model_name. Added new test suite to ensure model names are accepted as-is in both provider/model and simple model name formats.
  • Cleaned up usages in base_model.py, azure_model.py, anthropic_model.py, openai_model.py, and their corresponding tests.
  • Confirmed that the system now treats model names as full strings, consistent with LiteLLM integration expectations.

Please review and let me know if there are any edge cases or legacy concerns to address.

@penguine-ip
Copy link
Copy Markdown
Contributor

Hey @ps2program i was thinking more of having litellm integration directly, so that it appears as one of the providers in the llm module, do you think you can get that working?

@ps2program
Copy link
Copy Markdown
Contributor Author

Hi@penguine-ip , sure. I can get this working. Thanks for the clarification.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants