fix: 修复因错误合并导致的bug - #621
Conversation
📝 WalkthroughWalkthroughThe pull request consolidates LLM connection testing into one throttled request, standardizes response validation, changes fallback expressions for three configuration values, and simplifies a tuple return statement. ChangesConnection and Configuration Updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
api/answer.py (1)
1318-1320: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid hiding response-shape bugs as connection failures.
except Exceptionalso catches local programming and parsing errors, logs only the error text, and makes them indistinguishable from network failures. Catch expected transport/API exceptions and uselogger.exceptionfor unexpected failures. Static analysis also reports BLE001.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/answer.py` around lines 1318 - 1320, Update the exception handling around the connection check in the relevant method of the answer class: catch only the expected transport/API exception types for connection failures, and log unexpected exceptions with logger.exception before returning False. Preserve the existing failure result while preventing local response-shape or programming errors from being silently classified as connection failures and address the BLE001 warning.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api/answer.py`:
- Around line 1308-1313: Align check_llm_connection() with AI._query_locked() by
requiring non-empty message.content before reporting a successful provider
connection; do not treat reasoning_content alone as sufficient unless
_query_locked() is also updated to use it as the answer content.
---
Nitpick comments:
In `@api/answer.py`:
- Around line 1318-1320: Update the exception handling around the connection
check in the relevant method of the answer class: catch only the expected
transport/API exception types for connection failures, and log unexpected
exceptions with logger.exception before returning False. Preserve the existing
failure result while preventing local response-shape or programming errors from
being silently classified as connection failures and address the BLE001 warning.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 92dda8ce-fdf8-4787-8369-4fb69456036c
📒 Files selected for processing (3)
api/answer.pyapi/base.pymain.py
| # 统一检查响应 | ||
| if completion.choices: | ||
| msg = completion.choices[0].message | ||
| if msg.content or getattr(msg, 'reasoning_content', None): | ||
| logger.info(f'{self.name} 连接检查成功') | ||
| return True |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | rg '(^|/)api/answer\.py$|(^|/)main\.py$' || true
echo
echo "api/answer.py relevant snippets:"
sed -n '1240,1290p' api/answer.py || true
sed -n '1298,1320p' api/answer.py || true
echo
echo "main.py relevant snippets:"
sed -n '200,245p' main.py || true
echo
echo "Search for _query_locked and connection check usages:"
rg -n "_query_locked|连接检查|reason" -S api/answer.py main.py || true
echo
echo "Git diff summary/name-only:"
git diff --stat || true
git diff -- api/answer.py main.py | sed -n '1,220p' || trueRepository: Samueli924/chaoxing
Length of output: 6218
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Behavioral probe for the string-validation logic: parse_answer_like logic from _query_locked
# and the connection-check condition as written, over a small sample of response variants.
python3 - <<'PY'
from pathlib import Path
import re
answer_py = Path('api/answer.py').read_text()
m = re.search(r'_query_locked\(.*?\):(?:\n\s+.*)*?(?=\n def \w|\Z)', answer_py, re.S)
print("FOUND_QUERY_LOCKED=", m is not None)
if m:
snippet = m.group(0)
print(snippet)
print("contains_reasoning_in_parse=", bool(re.search(r'reasoning_content', snippet, re.I)))
print("contains_text_content=", bool(re.search(r'\.content\b', snippet, re.I)))
else:
print(answer_py[1240*80:1270*80])
conn = re.search(r'if \s+completion\.choices:\s*\n((?:\s+.+\n)+)', answer_py)
if conn:
print("FOUND_CONN_CHECK=true")
print(conn.group(0))
PYRepository: Samueli924/chaoxing
Length of output: 27070
🌐 Web query:
OpenAI Python ChatCompletionMessage reasoning_content attribute Content
💡 Result:
In the standard official OpenAI Python SDK, the ChatCompletionMessage object does not include a native reasoning_content attribute [1]. While some third-party model providers (such as DeepSeek or Grok) and local inference servers like vLLM include a reasoning_content field in their API responses to expose "thinking" or "reasoning" tokens, OpenAI has explicitly declined to incorporate this field as a standard part of their Python SDK's ChatCompletionMessage object [2][3][4][5]. Key technical points: 1. SDK Absence: The official openai-python library's ChatCompletionMessage class definition does not define reasoning_content [1]. Attempts to introduce this field into the official SDK via pull requests have been declined [2]. 2. Third-Party Usage: You may encounter this field when using the OpenAI Python client to communicate with third-party APIs that follow the OpenAI Chat Completions API format but extend it to include reasoning tokens [4][5]. In these scenarios, developers often access the field dynamically (e.g., via getattr or casting) because the SDK's type-hinting will not recognize it [6][4]. 3. OpenAI's Reasoning Approach: OpenAI handles reasoning models via their own specific APIs and architectural patterns (e.g., the Responses API or specific reasoning contexts) rather than by exposing raw reasoning tokens in the standard ChatCompletionMessage structure [7]. If you are working with a third-party service that returns reasoning_content, you can access it by treating the response message as a dictionary or using dynamic attribute access, but you should not expect it to be a permanent or supported feature of the official OpenAI Python SDK [6][4].
Citations:
- 1: https://github.com/openai/openai-python/blob/main/src/openai/types/chat/chat_completion_message.py
- 2: ChatCompletionMessage add reasoning_content openai/openai-python#2192
- 3: https://reference.langchain.com/python/langchain-openai/chat_models/base
- 4: https://docs.vllm.ai/en/v0.8.4/getting_started/examples/openai_chat_completion_with_reasoning.html
- 5: Reasoning content standardcompletions/rfcs#3
- 6: https://github.com/openai/openai-agents-python/blob/3a11cf52/tests/models/test_reasoning_content.py
- 7: https://developers.openai.com/api/docs/guides/reasoning
Keep the AI provider startup check aligned with the real answer parsing.
check_llm_connection() returns success when only reasoning_content is present, because AI._query_locked() continues to parse completion.choices[0].message.content. A reasoning-only response can clear startup validation but fail as None when real questions are answered. Require usable answer content in the check, or parse reasoning_content in _query_locked() the same way.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@api/answer.py` around lines 1308 - 1313, Align check_llm_connection() with
AI._query_locked() by requiring non-empty message.content before reporting a
successful provider connection; do not treat reasoning_content alone as
sufficient unless _query_locked() is also updated to use it as the answer
content.
2 participants
Summary by CodeRabbit
Bug Fixes
Improvements