From 9cfaaba3b68f8a5322a50221859e4af10a8fcbca Mon Sep 17 00:00:00 2001 From: majdyz Date: Tue, 21 Apr 2026 23:06:07 +0700 Subject: [PATCH] fix(backend/copilot): anchor Kimi reasoning-route match to reject hakimi false positives MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../backend/copilot/baseline/reasoning.py | 30 +++++++++++++------ .../copilot/baseline/reasoning_test.py | 16 ++++++++-- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/autogpt_platform/backend/backend/copilot/baseline/reasoning.py b/autogpt_platform/backend/backend/copilot/baseline/reasoning.py index e5f941b805..e2511b34eb 100644 --- a/autogpt_platform/backend/backend/copilot/baseline/reasoning.py +++ b/autogpt_platform/backend/backend/copilot/baseline/reasoning.py @@ -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: diff --git a/autogpt_platform/backend/backend/copilot/baseline/reasoning_test.py b/autogpt_platform/backend/backend/copilot/baseline/reasoning_test.py index e429969b3a..4d6d3c5623 100644 --- a/autogpt_platform/backend/backend/copilot/baseline/reasoning_test.py +++ b/autogpt_platform/backend/backend/copilot/baseline/reasoning_test.py @@ -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):