Add Atlas Cloud chat completion provider#9288
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for the Atlas Cloud chat completion provider. It adds the default configuration for Atlas Cloud, registers the provider adapter with dynamic import support, and implements the ProviderAtlasCloud class which inherits from the official OpenAI provider. Additionally, unit tests have been added to verify the default and custom endpoint/model configurations. I have no feedback to provide.
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.
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
ProviderAtlasCloud.get_models, catching a bareExceptionand silentlypassing makes debugging harder; consider narrowing the exception type and/or emitting a log so unexpected failures don't get completely swallowed. - The
ProviderAtlasCloudconstructor mutates the incomingprovider_configdict to set defaults; if callers reuse these dicts elsewhere, consider copying the config before modification to avoid surprising side effects.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `ProviderAtlasCloud.get_models`, catching a bare `Exception` and silently `pass`ing makes debugging harder; consider narrowing the exception type and/or emitting a log so unexpected failures don't get completely swallowed.
- The `ProviderAtlasCloud` constructor mutates the incoming `provider_config` dict to set defaults; if callers reuse these dicts elsewhere, consider copying the config before modification to avoid surprising side effects.
## Individual Comments
### Comment 1
<location path="astrbot/core/provider/sources/atlascloud_source.py" line_range="32-37" />
<code_context>
+
+ super().__init__(provider_config, provider_settings)
+
+ async def get_models(self) -> list[str]:
+ try:
+ models = await super().get_models()
+ if models:
+ return models
+ except Exception:
+ pass
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The broad exception handler in get_models silently swallows all errors, which could make debugging difficult.
Catching `Exception` and then `pass`-ing will mask real issues (auth, network, schema changes, etc.). Please either narrow the exception type or at least log the failure with enough context so provider-related problems can be diagnosed while still falling back to the static list.
Suggested implementation:
```python
async def get_models(self) -> list[str]:
try:
models = await super().get_models()
if models:
return models
except Exception as exc:
logger.warning(
"AtlasCloudSource.get_models: failed to fetch models from upstream; "
"falling back to static ATLASCLOUD_MODELS list.",
exc_info=exc,
)
return ATLASCLOUD_MODELS
```
To fully implement this change, ensure there is a module-level logger defined in this file, for example:
1. At the top of `astrbot/core/provider/sources/atlascloud_source.py`:
- Add `import logging` if it is not already present.
- Add `logger = logging.getLogger(__name__)` alongside other module-level definitions.
If this project uses a different logging convention (e.g., a shared logger utility), replace `logger = logging.getLogger(__name__)` and the `logger.warning(...)` call with the appropriate project-specific logger.
</issue_to_address>
### Comment 2
<location path="astrbot/core/provider/sources/atlascloud_source.py" line_range="5-7" />
<code_context>
+from .openai_source import ProviderOpenAIOfficial
+
+ATLASCLOUD_DEFAULT_API_BASE = "https://api.atlascloud.ai/v1"
+ATLASCLOUD_DEFAULT_MODEL = "qwen/qwen3.5-flash"
+ATLASCLOUD_MODELS = [
+ "qwen/qwen3.5-flash",
+ "deepseek-ai/deepseek-v4-pro",
+ "deepseek-ai/deepseek-v4-flash",
</code_context>
<issue_to_address>
**nitpick:** Keep the default model DRY by reusing ATLASCLOUD_DEFAULT_MODEL in ATLASCLOUD_MODELS.
The default model string is defined both in `ATLASCLOUD_DEFAULT_MODEL` and in `ATLASCLOUD_MODELS`, so any change would require updating two places. Referencing the constant in the list (e.g., `ATLASCLOUD_MODELS = [ATLASCLOUD_DEFAULT_MODEL, ...]`) keeps them in sync.
</issue_to_address>
### Comment 3
<location path="tests/test_openai_source.py" line_range="140-144" />
<code_context>
assert captured["httpx_module"] is openai_source_module.httpx
+def test_atlascloud_provider_sets_default_openai_compatible_endpoint():
+ provider = _make_atlascloud_provider()
+
+ assert str(provider.client.base_url).rstrip("/") == "https://api.atlascloud.ai/v1"
+ assert provider.get_model() == "qwen/qwen3.5-flash"
+
+
</code_context>
<issue_to_address>
**question (testing):** Consider adding tests for explicit-but-falsy api_base/model values (e.g. empty strings)
`ProviderAtlasCloud` treats `if not provider_config.get("api_base")` / `if not provider_config.get("model")` as “missing”, so values like `""` or `None` are replaced with defaults. Current tests only cover absent or non-empty values.
Please add tests that exercise falsy-but-explicit values, for example:
- `{"api_base": "", "model": ""}` to confirm that defaults are applied (if that’s the intended behavior), or
- A case that asserts empty strings are preserved, if that’s what we want.
This will clarify and lock in the expected behavior for falsy config values.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| async def get_models(self) -> list[str]: | ||
| try: | ||
| models = await super().get_models() | ||
| if models: | ||
| return models | ||
| except Exception: |
There was a problem hiding this comment.
suggestion (bug_risk): The broad exception handler in get_models silently swallows all errors, which could make debugging difficult.
Catching Exception and then pass-ing will mask real issues (auth, network, schema changes, etc.). Please either narrow the exception type or at least log the failure with enough context so provider-related problems can be diagnosed while still falling back to the static list.
Suggested implementation:
async def get_models(self) -> list[str]:
try:
models = await super().get_models()
if models:
return models
except Exception as exc:
logger.warning(
"AtlasCloudSource.get_models: failed to fetch models from upstream; "
"falling back to static ATLASCLOUD_MODELS list.",
exc_info=exc,
)
return ATLASCLOUD_MODELSTo fully implement this change, ensure there is a module-level logger defined in this file, for example:
- At the top of
astrbot/core/provider/sources/atlascloud_source.py:- Add
import loggingif it is not already present. - Add
logger = logging.getLogger(__name__)alongside other module-level definitions.
- Add
If this project uses a different logging convention (e.g., a shared logger utility), replace logger = logging.getLogger(__name__) and the logger.warning(...) call with the appropriate project-specific logger.
| ATLASCLOUD_DEFAULT_MODEL = "qwen/qwen3.5-flash" | ||
| ATLASCLOUD_MODELS = [ | ||
| "qwen/qwen3.5-flash", |
There was a problem hiding this comment.
nitpick: Keep the default model DRY by reusing ATLASCLOUD_DEFAULT_MODEL in ATLASCLOUD_MODELS.
The default model string is defined both in ATLASCLOUD_DEFAULT_MODEL and in ATLASCLOUD_MODELS, so any change would require updating two places. Referencing the constant in the list (e.g., ATLASCLOUD_MODELS = [ATLASCLOUD_DEFAULT_MODEL, ...]) keeps them in sync.
| def test_atlascloud_provider_sets_default_openai_compatible_endpoint(): | ||
| provider = _make_atlascloud_provider() | ||
|
|
||
| assert str(provider.client.base_url).rstrip("/") == "https://api.atlascloud.ai/v1" | ||
| assert provider.get_model() == "qwen/qwen3.5-flash" |
There was a problem hiding this comment.
question (testing): Consider adding tests for explicit-but-falsy api_base/model values (e.g. empty strings)
ProviderAtlasCloud treats if not provider_config.get("api_base") / if not provider_config.get("model") as “missing”, so values like "" or None are replaced with defaults. Current tests only cover absent or non-empty values.
Please add tests that exercise falsy-but-explicit values, for example:
{"api_base": "", "model": ""}to confirm that defaults are applied (if that’s the intended behavior), or- A case that asserts empty strings are preserved, if that’s what we want.
This will clarify and lock in the expected behavior for falsy config values.
Summary
Validation
No README changes. No sponsor, logo, credits, or partner placement.
Summary by Sourcery
Add an Atlas Cloud OpenAI-compatible chat completion provider and wire it into the default configuration and provider manager.
New Features:
Enhancements:
Tests: