Skip to content

fix: preserve explicit rerank api paths#7165

Open
whatevertogo wants to merge 3 commits intoAstrBotDevs:masterfrom
whatevertogo:fix/rerank-api-base-suffix
Open

fix: preserve explicit rerank api paths#7165
whatevertogo wants to merge 3 commits intoAstrBotDevs:masterfrom
whatevertogo:fix/rerank-api-base-suffix

Conversation

@whatevertogo
Copy link
Copy Markdown
Contributor

@whatevertogo whatevertogo commented Mar 30, 2026

This fixes the rerank API base URL handling for the vllm_rerank provider.
fix #5699

Modifications / 改动点

  • Preserve explicit rerank paths instead of always appending /v1/rerank

  • Append /v1/rerank only when rerank_api_base is just a host, and append /rerank only when it already ends with /v1

  • Surface upstream rerank API errors directly instead of falling back to a generic no results returned failure

  • Update config metadata hints and the Chinese/English knowledge-base docs to reflect the new behavior

  • This is NOT a breaking change. / 这不是一个破坏性变更。

Screenshots or Test Results / 运行截图或测试结果

Verification steps:

uv run ruff format astrbot/core/provider/sources/vllm_rerank_source.py astrbot/core/config/default.py
uv run ruff check astrbot/core/provider/sources/vllm_rerank_source.py astrbot/core/config/default.py

Local validation covered these cases for VLLMRerankProvider._resolve_rerank_endpoint:

  • http://127.0.0.1:8000 -> http://127.0.0.1:8000/v1/rerank
  • http://127.0.0.1:8000/v1 -> http://127.0.0.1:8000/v1/rerank
  • http://127.0.0.1:8000/v1/rerank -> http://127.0.0.1:8000/v1/rerank
  • https://example.com/api/v3/rerank -> https://example.com/api/v3/rerank
  • https://example.com/custom/path -> https://example.com/custom/path

Checklist / 检查清单

  • 😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
    / 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。

  • 👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
    / 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”

  • 🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in requirements.txt and pyproject.toml.
    / 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到 requirements.txtpyproject.toml 文件相应位置。

  • 😮 My changes do not introduce malicious code.
    / 我的更改没有引入恶意代码.

Summary by Sourcery

Improve vLLM rerank provider URL handling and error reporting while updating related configuration hints and docs.

Bug Fixes:

  • Correct rerank API endpoint resolution to respect explicit paths and only append default suffixes when the base URL is host-like.

Enhancements:

  • Log the resolved rerank API URL used by the vLLM rerank provider.
  • Raise clear errors when the rerank API returns error payloads instead of silently failing with a generic no-results condition.

Documentation:

  • Clarify in English and Chinese knowledge-base docs how AstrBot derives the rerank API endpoint from rerank api base.

Copilot AI review requested due to automatic review settings March 30, 2026 00:43
@auto-assign auto-assign bot requested review from Fridemn and anka-afk March 30, 2026 00:43
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Mar 30, 2026
@dosubot dosubot bot added the area:provider The bug / feature is about AI Provider, Models, LLM Agent, LLM Agent Runner. label Mar 30, 2026
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 3 issues, and left some high level feedback:

  • In rerank, response_data is assumed to be a dict (calling .get on it) but only type-checked as a dict inside the error branch; consider validating the JSON type once up front or handling non-dict responses explicitly to avoid AttributeError in unexpected cases.
  • The base URL is stripped twice (rstrip('/') in __init__ and removesuffix('/') in _resolve_rerank_endpoint); consolidating normalization in a single place (e.g., inside _resolve_rerank_endpoint) would simplify the logic and reduce the chance of subtle inconsistencies.
  • The info-level log [vLLM Rerank] Using API URL: ... will be emitted on every provider initialization; consider downgrading this to debug or making it conditional to avoid noisy logs in production deployments.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `rerank`, `response_data` is assumed to be a `dict` (calling `.get` on it) but only type-checked as a dict inside the error branch; consider validating the JSON type once up front or handling non-dict responses explicitly to avoid `AttributeError` in unexpected cases.
- The base URL is stripped twice (`rstrip('/')` in `__init__` and `removesuffix('/')` in `_resolve_rerank_endpoint`); consolidating normalization in a single place (e.g., inside `_resolve_rerank_endpoint`) would simplify the logic and reduce the chance of subtle inconsistencies.
- The info-level log `[vLLM Rerank] Using API URL: ...` will be emitted on every provider initialization; consider downgrading this to debug or making it conditional to avoid noisy logs in production deployments.

## Individual Comments

### Comment 1
<location path="astrbot/core/provider/sources/vllm_rerank_source.py" line_range="17-29" />
<code_context>
 )
 class VLLMRerankProvider(RerankProvider):
+    @staticmethod
+    def _resolve_rerank_endpoint(base_url: str) -> str:
+        normalized = base_url.strip().removesuffix("/")
+        if normalized.endswith("/rerank"):
+            return normalized
+        if normalized.endswith("/v1"):
+            return f"{normalized}/rerank"
+
+        parsed = urlsplit(normalized if "://" in normalized else f"//{normalized}")
+        if not parsed.path or parsed.path == "/":
+            return f"{normalized}/v1/rerank"
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Normalize and validate schemeless base URLs more explicitly to avoid subtle endpoint issues.

Because `urlsplit(normalized if '://' in normalized else f'//{normalized}')` keeps schemeless inputs unchanged (e.g. `example.com` or `example.com/api`), `self.endpoint_url` can end up without a scheme, which aiohttp will reject and which behaves differently depending on whether a path is present. Please either enforce/add a default scheme (e.g. `http`) for host-only inputs or fail fast when `base_url` lacks a scheme so this configuration issue is explicit and easier to debug.

```suggestion
class VLLMRerankProvider(RerankProvider):
    @staticmethod
    def _resolve_rerank_endpoint(base_url: str) -> str:
        """
        Normalize and resolve the rerank endpoint from a base URL.

        Rules:
        - If the base URL already ends with `/rerank`, use it as-is.
        - If it ends with `/v1`, append `/rerank`.
        - If it is host-only (with or without scheme), append `/v1/rerank`.
        - If it has a path but no scheme (e.g. `example.com/api`), fail fast.
        - For host-only schemeless URLs (e.g. `example.com`), default to `http://`.
        """
        normalized = base_url.strip().removesuffix("/")

        # Fast-path for already well-formed endpoints
        if normalized.endswith("/rerank"):
            return normalized
        if normalized.endswith("/v1"):
            return f"{normalized}/rerank"

        has_scheme = "://" in normalized
        # For schemeless inputs we still want to parse host vs path correctly
        parsed = urlsplit(normalized if has_scheme else f"//{normalized}")

        if not has_scheme:
            # Schemeless with a non-root path is ambiguous: require explicit scheme
            if parsed.path not in ("", "/"):
                raise ValueError(
                    "VLLMRerankProvider base_url must include a URL scheme when a path is provided "
                    f"(got {base_url!r})"
                )
            # Host-only input like `example.com` or `example.com:8000`: default to http
            normalized = f"http://{normalized}"
            has_scheme = True
            parsed = urlsplit(normalized)

        # If no path or root path, assume `/v1/rerank` under the given base
        if not parsed.path or parsed.path == "/":
            return f"{normalized}/v1/rerank"

        # Non-root path with scheme: use as-is (caller is responsible for correctness)
        return normalized
```
</issue_to_address>

### Comment 2
<location path="astrbot/core/provider/sources/vllm_rerank_source.py" line_range="65-69" />
<code_context>
             payload["top_n"] = top_n
         assert self.client is not None
         async with self.client.post(
-            f"{self.base_url}/v1/rerank",
+            self.endpoint_url,
             json=payload,
         ) as response:
+            response.raise_for_status()
             response_data = await response.json()
+            if isinstance(response_data, dict) and "error" in response_data:
</code_context>
<issue_to_address>
**suggestion:** Reconsider the order of `raise_for_status` vs JSON parsing to preserve API error details.

Because `response.raise_for_status()` runs first, any non-2xx response will raise an `aiohttp` error before your `

Suggested implementation:

```python
        async with self.client.post(
            self.endpoint_url,
            json=payload,
        ) as response:
            # Parse the JSON body first so we can surface any structured API
            # error payload before aiohttp raises on non-2xx status codes.
            response_data = await response.json()
            if isinstance(response_data, dict) and "error" in response_data:

```

```python
                error = response_data["error"]
                if isinstance(error, dict):
                    code = error.get("code", "unknown")
                    message = error.get("message", "Unknown rerank API error")
                    raise ValueError(f"Rerank API error {code}: {message}")
                raise ValueError(f"Rerank API error: {error}")
            # Only raise for HTTP status errors after we've inspected the body
            # for API-specific error details.
            response.raise_for_status()
            results = response_data.get("results", [])

```
</issue_to_address>

### Comment 3
<location path="astrbot/core/provider/sources/vllm_rerank_source.py" line_range="69-78" />
<code_context>
+            self.endpoint_url,
             json=payload,
         ) as response:
+            response.raise_for_status()
             response_data = await response.json()
+            if isinstance(response_data, dict) and "error" in response_data:
+                error = response_data["error"]
+                if isinstance(error, dict):
+                    code = error.get("code", "unknown")
+                    message = error.get("message", "Unknown rerank API error")
+                    raise ValueError(f"Rerank API error {code}: {message}")
+                raise ValueError(f"Rerank API error: {error}")
             results = response_data.get("results", [])

             if not results:
</code_context>
<issue_to_address>
**issue:** Guard against non-dict JSON responses before accessing `results`.

Currently you only special-case dicts with an `"error"` field, then always call `response_data.get("results", [])`. If the API ever returns a list or other non-dict JSON, this will raise `AttributeError`. Add an `isinstance(response_data, dict)` guard before accessing `"results"`, and raise a clear error if the schema is unexpected.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adjusts how AstrBot’s vllm_rerank provider resolves the rerank endpoint URL so user-supplied rerank API bases that already include a path (or a full endpoint) are preserved, while still supporting “host-only” bases via automatic suffixing.

Changes:

  • Add endpoint resolution logic (_resolve_rerank_endpoint) and use the resolved URL for requests in VLLMRerankProvider.
  • Improve error surfacing from the upstream rerank HTTP response.
  • Update dashboard config hints and docs (ZH/EN) describing the rerank API base behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
astrbot/core/provider/sources/vllm_rerank_source.py Adds URL resolution for rerank endpoint and uses it for POST requests; improves error handling.
astrbot/core/config/default.py Updates config metadata hint for rerank_api_base.
dashboard/src/i18n/locales/zh-CN/features/config-metadata.json Updates Chinese hint text for rerank_api_base.
dashboard/src/i18n/locales/en-US/features/config-metadata.json Updates English hint text for rerank_api_base.
dashboard/src/i18n/locales/ru-RU/features/config-metadata.json Updates Russian hint text for rerank_api_base.
docs/zh/use/knowledge-base.md Updates Chinese knowledge-base doc to describe rerank API base URL behavior.
docs/en/use/knowledge-base.md Updates English knowledge-base doc to describe rerank API base URL behavior.

Copy link
Copy Markdown
Contributor

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

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 enhances the vLLM rerank provider by implementing a more flexible API base URL resolution logic, which automatically appends /v1/rerank to domains while preserving full paths. It also introduces comprehensive error handling for API responses and updates configuration hints and documentation in multiple languages. Feedback was provided to refactor the initialization logic in VLLMRerankProvider to eliminate redundant string operations and unnecessary instance variables.

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

Labels

area:provider The bug / feature is about AI Provider, Models, LLM Agent, LLM Agent Runner. size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] 增加智谱Embedding模型和Rerank模型支持

2 participants