Skip to content

Move all ModelClient classes into client module.#551

Open
GdoongMathew wants to merge 33 commits into
AsyncFuncAI:mainfrom
GdoongMathew:feat/my_branch
Open

Move all ModelClient classes into client module.#551
GdoongMathew wants to merge 33 commits into
AsyncFuncAI:mainfrom
GdoongMathew:feat/my_branch

Conversation

@GdoongMathew

Copy link
Copy Markdown
Contributor

Summary

  • Restructures the client integration layer by consolidating all provider clients
    under api/clients/ (bedrock, dashscope, litellm, openrouter, ...).
  • Removes the monolithic openai_client and azureai_client modules.

Pros

  • Unifies the client import path.
  • Cleaner, more consistent repo structure that's easier to navigate and extend.

GdoongMathew and others added 30 commits July 18, 2026 12:01
* refactor chat streaming

* update `_is_token_limit_error` logic

* test: add chat provider unittests

* move `_is_token_limit_error` to module level

* remove unnecessary TYPE_CHECKING

* add EOF

* update prompt function
* patch: patch adalflow OllamaClient to support batch embedding api.

related to SylphAI-Inc/AdalFlow#496

* remove `OllamaDocumentProcessor`
* refactor chat streaming

* update `_is_token_limit_error` logic

* test: add chat provider unittests

* move `_is_token_limit_error` to module level

* remove unnecessary TYPE_CHECKING

* add EOF

* update prompt function

* refactor `websocket_wiki.py` using `ChatStreamer`

* remove unnecessary imports

* add litellm test

* update kwargs in `GoogleGenerativeChatStreamer`
# Conflicts:
#	api/websocket_wiki.py
# Conflicts:
#	api/chat/_stream.py
#	api/config.py
#	api/simple_chat.py
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the project's client integrations by removing local client implementations and unifying all Adalflow-compatible clients under a new centralized api/clients module. This simplifies and standardizes imports across api/chat/_stream.py and api/config.py. The review feedback recommends improving PEP 8 import grouping in api/clients/__init__.py, importing OpenAIClient from the public namespace instead of the internal submodule in api/clients/litellm.py, and sorting the imported clients alphabetically in api/config.py to enhance readability.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread api/clients/__init__.py Outdated
Comment on lines +5 to +15
from .google_embedder import GoogleEmbedderClient
from .bedrock import BedrockClient
from .dashscope import DashscopeClient
from .ollama import OllamaClient
from .litellm import LiteLLMClient
from .openrouter import OpenRouterClient
from adalflow.components.model_client import (
AzureAIClient,
OpenAIClient,
GoogleGenAIClient,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

According to PEP 8, imports should be grouped in the following order:

  1. Standard library imports.
  2. Related third-party imports.
  3. Local application/library specific imports.

Currently, the third-party import from adalflow is placed after the local relative imports (e.g., .google_embedder). Placing third-party imports before local relative imports improves readability and adheres to PEP 8 guidelines.

Suggested change
from .google_embedder import GoogleEmbedderClient
from .bedrock import BedrockClient
from .dashscope import DashscopeClient
from .ollama import OllamaClient
from .litellm import LiteLLMClient
from .openrouter import OpenRouterClient
from adalflow.components.model_client import (
AzureAIClient,
OpenAIClient,
GoogleGenAIClient,
)
from adalflow.components.model_client import (
AzureAIClient,
OpenAIClient,
GoogleGenAIClient,
)
from .google_embedder import GoogleEmbedderClient
from .bedrock import BedrockClient
from .dashscope import DashscopeClient
from .ollama import OllamaClient
from .litellm import LiteLLMClient
from .openrouter import OpenRouterClient

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed in 9ca0ae5

Comment thread api/clients/litellm.py
from openai import AsyncOpenAI, OpenAI

from api.openai_client import OpenAIClient
from adalflow.components.model_client.openai_client import OpenAIClient

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Importing OpenAIClient directly from the public namespace adalflow.components.model_client is preferred over importing from the internal submodule adalflow.components.model_client.openai_client. This ensures consistency with other imports in the project and guards against potential internal refactoring of the library.

Suggested change
from adalflow.components.model_client.openai_client import OpenAIClient
from adalflow.components.model_client import OpenAIClient

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OpenAIClient in scope model_client is wrapped as an LazyImport instance, which could not be subclassed.

https://github.com/SylphAI-Inc/AdalFlow/blob/810de99d86191b3aa0c939aa6d6d1a21977555aa/adalflow/adalflow/utils/lazy_import.py#L133-L139

Thus in this scenario, the OpenAIClient must be explicitly imported from openai_client so that LiteLLMClient could inherit from it.

Comment thread api/config.py
Comment on lines +10 to +20
from api.clients import (
LiteLLMClient,
OpenRouterClient,
BedrockClient,
GoogleGenAIClient,
GoogleEmbedderClient,
AzureAIClient,
DashscopeClient,
OpenAIClient,
OllamaClient,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Sorting imported names alphabetically within a multi-line import block improves readability and makes it easier to find specific clients as the list grows.

Suggested change
from api.clients import (
LiteLLMClient,
OpenRouterClient,
BedrockClient,
GoogleGenAIClient,
GoogleEmbedderClient,
AzureAIClient,
DashscopeClient,
OpenAIClient,
OllamaClient,
)
from api.clients import (
AzureAIClient,
BedrockClient,
DashscopeClient,
GoogleEmbedderClient,
GoogleGenAIClient,
LiteLLMClient,
OllamaClient,
OpenAIClient,
OpenRouterClient,
)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed in 9ca0ae5

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.

1 participant