mirror of
https://github.com/Significant-Gravitas/AutoGPT.git
synced 2026-04-30 03:00:41 -04:00
fix(backend/copilot): anchor Kimi reasoning-route match to reject hakimi false positives
Sentry review on #12871 flagged the `"kimi" in lowered` substring check in `_is_reasoning_route` as too broad — a hypothetical `some-provider/hakimi-large` would match and get a `reasoning` payload appended to its request. Some providers silently drop unknown fields, others 400, so this is a correctness-not-just-tidy fix. Replace the substring check with an anchored match: accept the `moonshotai/` provider prefix, or a bare `kimi-` model id (either at string start or immediately after a `/` provider prefix). `claude` / `anthropic` branches unchanged. Adds regression coverage for `hakimi`, `some-provider/hakimi-large`, `akimi-7b` and keeps the existing Kimi variants passing.
This commit is contained in:
@@ -1,7 +1,8 @@
|
||||
"""Extended-thinking wire support for the baseline (OpenRouter) path.
|
||||
|
||||
Anthropic routes on OpenRouter expose extended thinking through
|
||||
non-OpenAI extension fields that the OpenAI Python SDK doesn't model:
|
||||
OpenRouter routes that support extended thinking (Anthropic Claude and
|
||||
Moonshot Kimi today) expose reasoning through non-OpenAI extension fields
|
||||
that the OpenAI Python SDK doesn't model:
|
||||
|
||||
* ``reasoning`` (legacy string) — enabled by ``include_reasoning: true``.
|
||||
* ``reasoning_content`` — DeepSeek / some OpenRouter routes.
|
||||
@@ -17,7 +18,8 @@ This module keeps the wire-level concerns in one place:
|
||||
one streaming round and emits ``StreamReasoning*`` events so the caller
|
||||
only has to plumb the events into its pending queue.
|
||||
* :func:`reasoning_extra_body` builds the ``extra_body`` fragment for the
|
||||
OpenAI client call. Returns ``None`` on non-Anthropic routes.
|
||||
OpenAI client call. Returns ``None`` for routes without reasoning
|
||||
support (see :func:`_is_reasoning_route`).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -159,14 +161,24 @@ def _is_reasoning_route(model: str) -> bool:
|
||||
Kept separate from :func:`backend.copilot.baseline.service._is_anthropic_model`
|
||||
because ``cache_control`` is strictly Anthropic-specific (Moonshot does
|
||||
its own auto-caching), so the two gates must not conflate.
|
||||
|
||||
The Kimi match anchors on the ``moonshotai/`` provider prefix or on a
|
||||
bare / OpenRouter-prefixed ``kimi-`` model id (``kimi-k2.6``,
|
||||
``moonshotai/kimi-k2-thinking``, ``openrouter/kimi-k2.6``), so unrelated
|
||||
models that happen to contain ``kimi`` as a substring (e.g. a
|
||||
hypothetical ``some-provider/hakimi-large``) are not treated as
|
||||
reasoning routes.
|
||||
"""
|
||||
lowered = model.lower()
|
||||
return (
|
||||
"claude" in lowered
|
||||
or lowered.startswith("anthropic")
|
||||
or lowered.startswith("moonshotai/")
|
||||
or "kimi" in lowered
|
||||
)
|
||||
if "claude" in lowered or lowered.startswith("anthropic"):
|
||||
return True
|
||||
if lowered.startswith("moonshotai/"):
|
||||
return True
|
||||
# Match a ``kimi-`` model id at string start or immediately after a
|
||||
# provider prefix ``/`` — avoids substring false positives like
|
||||
# ``hakimi``.
|
||||
bare = lowered.rsplit("/", 1)[-1]
|
||||
return bare.startswith("kimi-")
|
||||
|
||||
|
||||
def reasoning_extra_body(model: str, max_thinking_tokens: int) -> dict[str, Any] | None:
|
||||
|
||||
@@ -150,9 +150,13 @@ class TestIsReasoningRoute:
|
||||
assert _is_reasoning_route("moonshotai/kimi-k2.6")
|
||||
assert _is_reasoning_route("moonshotai/kimi-k2-thinking")
|
||||
assert _is_reasoning_route("moonshotai/kimi-k2.5")
|
||||
# Direct (non-OpenRouter) model ids also resolve via the ``kimi``
|
||||
# substring so a future bare ``kimi-k3`` id would still match.
|
||||
# Direct (non-OpenRouter) model ids also resolve via the ``kimi-``
|
||||
# prefix so a future bare ``kimi-k3`` id would still match.
|
||||
assert _is_reasoning_route("kimi-k2-instruct")
|
||||
# Provider-prefixed bare kimi ids (without the ``moonshotai/``
|
||||
# prefix) are also recognised — the match anchors on the final
|
||||
# path segment.
|
||||
assert _is_reasoning_route("openrouter/kimi-k2.6")
|
||||
|
||||
def test_other_providers_rejected(self):
|
||||
assert not _is_reasoning_route("openai/gpt-4o")
|
||||
@@ -161,6 +165,14 @@ class TestIsReasoningRoute:
|
||||
assert not _is_reasoning_route("meta-llama/llama-3.3-70b-instruct")
|
||||
assert not _is_reasoning_route("deepseek/deepseek-r1")
|
||||
|
||||
def test_kimi_substring_false_positives_rejected(self):
|
||||
# Regression: the previous implementation matched any model whose
|
||||
# name contained the substring ``kimi`` — including unrelated model
|
||||
# ids like ``hakimi``. The anchored match below rejects them.
|
||||
assert not _is_reasoning_route("some-provider/hakimi-large")
|
||||
assert not _is_reasoning_route("hakimi")
|
||||
assert not _is_reasoning_route("akimi-7b")
|
||||
|
||||
|
||||
class TestReasoningExtraBody:
|
||||
def test_anthropic_route_returns_fragment(self):
|
||||
|
||||
Reference in New Issue
Block a user