feat: 代码质量提升 - 修复异常处理、清理调试代码、合并PR#59增强#61
Merged
shenminglinyi merged 3 commits intoApr 16, 2026
Merged
Conversation
## 代码修复 - 修复13处裸except:捕获，改为具体异常类型 - 修复_call_llm_and_parse_with_retry未使用last_error问题 - 清理所有print()调试语句，改为logger - 删除重复的parse_json_from_response函数，统一使用导入 - 删除未使用的旧JSON解析函数（_sanitize_llm_json_output等） - 修复IndexError空指针风险（response.choices[0]） ## 合并PR#59增强 - LLM fallback持久化：update_profile_legacy_flag方法 - Settings添加profile_id字段支持 - ProviderFactory传递profile_id - OpenAIProvider添加_persist_legacy_flag方法 - TensionAnalyzer使用项目标准JSON解析 署名：我本凡尘
Contributor
📝 WalkthroughWalkthroughRefined broad exception handlers into explicit exception catches, centralized and hardened LLM JSON parsing, threaded profile_id through provider settings, added persistence for a legacy chat-completions flag, and replaced prints with structured logging across several modules. Changes
Sequence Diagram(s)sequenceDiagram
participant Provider as OpenAIProvider
participant Settings as Settings (with profile_id)
participant Control as LLMControlService
participant DB as llm_profiles DB
Provider->>Settings: initialized (includes profile_id)
Provider->>Provider: attempt Responses API generation
alt Responses unsupported / gateway error
Provider->>Control: _persist_legacy_flag(use_legacy=True) (if profile_id)
Control->>DB: UPDATE llm_profiles SET use_legacy_chat_completions=1 WHERE id=profile_id
DB-->>Control: OK
Control-->>Provider: confirmed
Provider->>Provider: fallback to Chat Completions
else Responses OK
Provider->>Provider: proceed with Responses flow
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
application/ai/knowledge_llm_contract.py (1)
109-114:⚠️ Potential issue | 🟠 MajorFenced-JSON parse failure can return
Noneprematurely instead of continuing fallback parsing.If decode fails at Line 109–110, Line 114 may return
None, which bypasses intended parse fallback and degrades into downstream validation noise.💡 Proposed fix
def parse_json_from_response(rsp: str): """从LLM响应中解析JSON，支持```json包裹格式""" pattern = r"```json(.*?)```" - rsp_json = None try: match = re.search(pattern, rsp, re.DOTALL) if match is not None: try: - rsp_json = json.loads(match.group(1).strip()) + return json.loads(match.group(1).strip()) except (json.JSONDecodeError, ValueError, TypeError): - pass - else: - rsp_json = json.loads(rsp) - return rsp_json + # fenced block failed, continue with raw text fallback + pass + return json.loads(rsp) except json.JSONDecodeError as e: try: match = re.search(r"\{(.*?)\}", rsp, re.DOTALL) if match: content = "{" + match.group(1) + "}" return json.loads(content) except (json.JSONDecodeError, ValueError, TypeError): pass raise e🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@application/ai/knowledge_llm_contract.py` around lines 109 - 114, The fenced-JSON parsing currently assigns rsp_json to None on failure which can cause returning None instead of trying the fallback raw parsing; update the block that searches pattern = r"```json(.*?)```" to return immediately when json.loads(match.group(1).strip()) succeeds, and if that loads raises (json.JSONDecodeError, ValueError, TypeError) simply continue to the raw-text fallback (i.e., do not set rsp_json to None or return), then call json.loads(rsp) as the primary fallback; reference the variables/pattern, the re.search call, match.group(1), json.loads(rsp) and the surrounding try/except in this function to locate where to change the control flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@application/world/services/auto_bible_generator.py`:
- Around line 585-586: The except blocks that currently catch (AttributeError,
TypeError, KeyError) must also include EntityNotFoundError so missing
DB/entities degrade to safe defaults; update both exception handlers (the one
returning {} and the one returning []) to catch (AttributeError, TypeError,
KeyError, EntityNotFoundError), and add an import for EntityNotFoundError from
the module that defines it if not already imported; ensure the handlers still
return the same safe defaults ({}/[]) after adding the new exception.
- Around line 12-13: The runtime/ retry break is caused by calls to removed
helpers (which cause NameError) and swallowing parse errors into {} so retries
are skipped; replace any remaining calls to the old helpers with
parse_json_from_response (imported at top) and change the parse failure handling
so it does NOT convert failures to an empty dict — instead let
parse_json_from_response raise or return a clear None/exception and propagate
that up so the existing retry logic runs (or explicitly re-raise) rather than
returning early when you see {}; update the code paths that check the parsed
value (the block that currently returns immediately on {}) to treat
None/exception as a parse failure and allow retry.
- Line 542: Replace f-strings that contain no interpolations with plain string
literals to fix F541 lint errors: update the logger.info call that currently
uses f"Worldbuilding saved to Worldbuilding table" (in the save flow for
Worldbuilding), change the log message f"Saving worldbuilding to
Bible.world_settings" (in the Bible.world_settings save path), and the message
f"所有重试都失败，返回空字典" (in the retry/failure handler) to regular quoted strings;
search for these exact string contents in auto_bible_generator.py and remove the
leading f from each f-string so they become normal string literals.
In `@infrastructure/ai/providers/openai_provider.py`:
- Around line 50-60: The fallback persistence for the legacy flag is incomplete:
_persist_legacy_flag(True) is only invoked in generate() but not when the first
fallback occurs inside stream_generate(), so the DB isn't updated and restarts
keep retrying the Responses API; update stream_generate() to call
_persist_legacy_flag(True) at the same fallback site where you switch from
Responses API to chat completions (mirror the generate() behavior), ensuring you
use the same control method names (call _persist_legacy_flag with the profile
context) so the profile record is updated when streaming falls back.
- Around line 73-83: The code currently calls self._persist_legacy_flag(True)
for broad/ambiguous exception text matches, which can permanently pin a profile
to legacy mode; change the exception handling in openai_provider.py so that only
definitive indicators of missing Responses support cause persistent storage:
keep calling self.__class__._fallback_to_chat_cache.add(base_url) for transient
or ambiguous errors but remove _persist_legacy_flag(True) for generic text
matches like "400" and other non-definitive messages; only call
_persist_legacy_flag(True) when the exception type is openai.NotFoundError or
openai.BadRequestError or when the error text explicitly contains clear endpoint
incompatibility markers such as "Not Found", "Account invalid", or
"INVALID_ARGUMENT". Ensure references: update the except Exception as e block,
the usage of self.__class__._fallback_to_chat_cache, and calls to
self._persist_legacy_flag to implement this behavior.
In `@interfaces/api/v1/engine/autopilot_routes.py`:
- Around line 667-671: The current try/except around queue.get_nowait() (where
sample_msg = queue.get_nowait(); queue.put(sample_msg)) is too broad; change it
to explicitly catch queue.Empty to handle the normal "no item" case, and add a
separate except Exception as e block that logs the unexpected error (using the
existing logger in this module) before re-raising or handling it appropriately.
Update the exception handling around the get_nowait()/put calls to import/use
queue.Empty and call logger.exception or logger.error with the exception object
for any unexpected errors so you don't silently swallow them.
---
Outside diff comments:
In `@application/ai/knowledge_llm_contract.py`:
- Around line 109-114: The fenced-JSON parsing currently assigns rsp_json to
None on failure which can cause returning None instead of trying the fallback
raw parsing; update the block that searches pattern = r"```json(.*?)```" to
return immediately when json.loads(match.group(1).strip()) succeeds, and if that
loads raises (json.JSONDecodeError, ValueError, TypeError) simply continue to
the raw-text fallback (i.e., do not set rsp_json to None or return), then call
json.loads(rsp) as the primary fallback; reference the variables/pattern, the
re.search call, match.group(1), json.loads(rsp) and the surrounding try/except
in this function to locate where to change the control flow.
🪄 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: 42bc12ad-2382-48c1-b252-38f77c3d10e4
📒 Files selected for processing (10)
application/ai/knowledge_llm_contract.pyapplication/ai/llm_control_service.pyapplication/analyst/services/llm_voice_analysis_service.pyapplication/world/services/auto_bible_generator.pyinfrastructure/ai/config/settings.pyinfrastructure/ai/provider_factory.pyinfrastructure/ai/providers/openai_provider.pyinterfaces/api/v1/blueprint/continuous_planning_routes.pyinterfaces/api/v1/engine/autopilot_routes.pyscripts/evaluation/macro_planning_evaluator.py
Comment on lines
73
to
83
| except (openai.NotFoundError, openai.BadRequestError, RuntimeError) as e: | ||
| logger.info(f"Responses API unsupported for {base_url}, falling back to chat completions: {str(e)}") | ||
| self.__class__._fallback_to_chat_cache.add(base_url) | ||
| self._persist_legacy_flag(True) | ||
| except Exception as e: | ||
| # 某些网关在路径错误时可能不抛严格的 404 而是抛出其他错误，如果消息含有明确路径错误也尝试降级 | ||
| if "404" in str(e) or "Not Found" in str(e) or "400" in str(e) or "Account invalid" in str(e) or "INVALID_ARGUMENT" in str(e): | ||
| logger.info(f"Gateway returned error for Responses API ({base_url}), falling back: {str(e)}") | ||
| self.__class__._fallback_to_chat_cache.add(base_url) | ||
| self._persist_legacy_flag(True) | ||
| else: |
Contributor
There was a problem hiding this comment.
Legacy-mode persistence is currently too eager.
The code persists use_legacy_chat_completions=True for broad fallback cases (including generic message matches like "400"), which can be transient/request-specific failures rather than true endpoint incompatibility. This can permanently pin a profile to legacy mode incorrectly.
Suggested fix
@@
- except (openai.NotFoundError, openai.BadRequestError, RuntimeError) as e:
+ except (openai.NotFoundError, openai.BadRequestError) as e:
logger.info(f"Responses API unsupported for {base_url}, falling back to chat completions: {str(e)}")
self.__class__._fallback_to_chat_cache.add(base_url)
self._persist_legacy_flag(True)
+ except RuntimeError:
+ # runtime/content anomalies: fallback once, but do not persist permanent legacy mode
+ logger.info("Responses API runtime anomaly, temporary fallback to chat completions")
except Exception as e:
- if "404" in str(e) or "Not Found" in str(e) or "400" in str(e) or "Account invalid" in str(e) or "INVALID_ARGUMENT" in str(e):
+ if "404" in str(e) or "Not Found" in str(e) or "INVALID_ARGUMENT" in str(e):
logger.info(f"Gateway returned error for Responses API ({base_url}), falling back: {str(e)}")
self.__class__._fallback_to_chat_cache.add(base_url)
self._persist_legacy_flag(True)
else:
raise🧰 Tools
🪛 Ruff (0.15.10)
[warning] 74-74: Use explicit conversion flag
Replace with conversion flag
(RUF010)
[warning] 78-78: Comment contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
[warning] 80-80: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@infrastructure/ai/providers/openai_provider.py` around lines 73 - 83, The
code currently calls self._persist_legacy_flag(True) for broad/ambiguous
exception text matches, which can permanently pin a profile to legacy mode;
change the exception handling in openai_provider.py so that only definitive
indicators of missing Responses support cause persistent storage: keep calling
self.__class__._fallback_to_chat_cache.add(base_url) for transient or ambiguous
errors but remove _persist_legacy_flag(True) for generic text matches like "400"
and other non-definitive messages; only call _persist_legacy_flag(True) when the
exception type is openai.NotFoundError or openai.BadRequestError or when the
error text explicitly contains clear endpoint incompatibility markers such as
"Not Found", "Account invalid", or "INVALID_ARGUMENT". Ensure references: update
the except Exception as e block, the usage of
self.__class__._fallback_to_chat_cache, and calls to self._persist_legacy_flag
to implement this behavior.
Comment on lines
667
to
671
| sample_msg = queue.get_nowait() | ||
| # 把消息放回去 | ||
| queue.put(sample_msg) | ||
| except: | ||
| except Exception: | ||
| pass |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
head -n 680 interfaces/api/v1/engine/autopilot_routes.py | tail -n 30Repository: shenminglinyi/PlotPilot
Length of output: 954
🏁 Script executed:
sed -n '650,675p' interfaces/api/v1/engine/autopilot_routes.pyRepository: shenminglinyi/PlotPilot
Length of output: 782
🏁 Script executed:
head -n 50 interfaces/api/v1/engine/autopilot_routes.pyRepository: shenminglinyi/PlotPilot
Length of output: 2011
🏁 Script executed:
rg -n "logger|import.*logging" interfaces/api/v1/engine/autopilot_routes.py | head -20Repository: shenminglinyi/PlotPilot
Length of output: 673
🏁 Script executed:
fd streaming_bus.pyRepository: shenminglinyi/PlotPilot
Length of output: 111
🏁 Script executed:
cat application/engine/services/streaming_bus.pyRepository: shenminglinyi/PlotPilot
Length of output: 7922
🏁 Script executed:
grep -n "multiprocessing\|Queue\|queue" application/engine/services/streaming_bus.py | head -20Repository: shenminglinyi/PlotPilot
Length of output: 918
🏁 Script executed:
python3 << 'EOF'
import multiprocessing as mp
import queue
# Check what exceptions multiprocessing.Queue.get_nowait() raises
q = mp.Queue()
# The multiprocessing.Queue should raise queue.Empty when empty
print("Testing multiprocessing.Queue exception behavior:")
print(f"queue.Empty exception: {queue.Empty}")
print(f"multiprocessing module also has Empty: {hasattr(mp, 'Empty')}")
# Check if they're the same
if hasattr(mp.Queue, 'get_nowait'):
print("multiprocessing.Queue has get_nowait method")
EOFRepository: shenminglinyi/PlotPilot
Length of output: 979
🏁 Script executed:
sed -n '1,25p' interfaces/api/v1/engine/autopilot_routes.pyRepository: shenminglinyi/PlotPilot
Length of output: 1059
🏁 Script executed:
grep -A5 -B5 "get_nowait\|queue.Empty\|Empty" application/engine/services/streaming_bus.py | head -40Repository: shenminglinyi/PlotPilot
Length of output: 1149
Explicitly handle queue.Empty instead of silently catching all exceptions.
At lines 670–671, catch the expected queue.Empty exception explicitly, and log unexpected exceptions for debugging:
Suggested fix
async def stream_debug(novel_id: str):
"""调试端点：检查流式队列状态"""
from application.engine.services.streaming_bus import _get_queue, _stream_queue, _injected_queue
import multiprocessing as mp
+ from queue import Empty
queue = _get_queue()
if queue is not None:
try:
sample_msg = queue.get_nowait()
# 把消息放回去
queue.put(sample_msg)
- except Exception:
+ except Empty:
pass
+ except Exception as e:
+ logger.debug("stream_debug queue probe failed: %s", e)🧰 Tools
🪛 Ruff (0.15.10)
[error] 670-671: try-except-pass detected, consider logging the exception
(S110)
[warning] 670-670: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@interfaces/api/v1/engine/autopilot_routes.py` around lines 667 - 671, The
current try/except around queue.get_nowait() (where sample_msg =
queue.get_nowait(); queue.put(sample_msg)) is too broad; change it to explicitly
catch queue.Empty to handle the normal "no item" case, and add a separate except
Exception as e block that logs the unexpected error (using the existing logger
in this module) before re-raising or handling it appropriately. Update the
exception handling around the get_nowait()/put calls to import/use queue.Empty
and call logger.exception or logger.error with the exception object for any
unexpected errors so you don't silently swallow them.
- 修复_call_llm_and_parse使用新的parse_json_from_response替代已删除的旧函数 - 增强parse_json_from_response支持截断的markdown JSON自动补全 - 修复重试逻辑：parse失败时raise异常而非返回空dict让重试生效 - 修复EntityNotFoundError缺失：在_load_worldbuilding和_load_characters中添加捕获 - 修复stream_generate fallback持久化：两处fallback点添加_persist_legacy_flag调用 - 修复docstring全角括号为半角（RUF002） - 修复blind exception：使用具体异常类型替代Exception（BLE001） - 修复f-string无占位符问题（F541） - 更新测试以反映新的异常传播设计
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
application/ai/knowledge_llm_contract.py (1)
108-114:⚠️ Potential issue | 🟠 MajorDon't swallow fenced-JSON parse failures.
When the ```json block is present but
json.loads()fails, this branch falls through to Line 114 and returns `None`. That bypasses the repair path below and downgrades a real parse failure into a generic `ValueError` in callers like `AutoBibleGenerator._call_llm_and_parse()`. Re-raise the original decode error here, or route this branch into the same recovery logic instead of `pass`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@application/ai/knowledge_llm_contract.py` around lines 108 - 114, The fenced-JSON branch swallows JSON decode exceptions causing callers to get None; in the block that handles match.group(1) (where rsp_json = json.loads(match.group(1).strip() is attempted), either re-raise the caught json.JSONDecodeError (or ValueError/TypeError) so callers see the original parse failure, or invoke the existing recovery/repair logic used for the unrouted branch (the same path taken when using rsp and the downstream repair routines) instead of using pass; update the exception handler around json.loads(match.group(1).strip()) to propagate the error or route to the repair function so rsp_json is not silently set to None.infrastructure/ai/providers/openai_provider.py (1)
153-158:⚠️ Potential issue | 🟠 MajorStreaming can now complete successfully with no content.
After the empty-
choicesguard in_extract_text_from_stream_chunk(), malformed chat stream chunks are silently skipped. If every chunk is empty/invalid,stream_generate()returns normally without yielding anything or raising, so callers see a blank completion instead of a retryable failure.💡 Suggested fix
stream = await self.async_client.chat.completions.create(**request_kwargs) + yielded_any = False async for chunk in stream: content = self._extract_text_from_stream_chunk(chunk) if content: + yielded_any = True yield content + if not yielded_any: + raise RuntimeError("OpenAI-compatible stream returned no content") except (openai.APIError, openai.APIConnectionError, openai.RateLimitError, openai.APITimeoutError) as e:Also applies to: 272-279
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/ai/providers/openai_provider.py` around lines 153 - 158, The stream generator (stream_generate) currently skips malformed/empty chunks via _extract_text_from_stream_chunk and can finish without yielding anything, causing callers to receive a blank completion; update stream_generate (and the analogous block at the other location) to detect when the stream completed with zero emitted content and raise a retryable error (or propagate an appropriate exception) instead of returning silently — track whether any content was yielded while iterating the async stream, and if none was produced after the stream ends, raise an error (e.g., a specific OpenAI-related or custom transient exception) so callers can handle retries.
♻️ Duplicate comments (1)
infrastructure/ai/providers/openai_provider.py (1)
73-82:⚠️ Potential issue | 🟠 MajorDon't persist legacy mode for ambiguous fallback signals.
This is still persisting
use_legacy_chat_completions=Truefor cases that do not prove endpoint incompatibility: Line 73 treats anyRuntimeErrorfrom_generate_via_responses()as permanent, and Lines 79-82 / 142-145 still persist on a plain"400"substring. Empty content or request-specific validation failures can permanently pin a profile to chat completions even though the endpoint itself is supported. Keep the in-process fallback for those cases, but only persist on definitive incompatibility signals.Also applies to: 137-145
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/ai/providers/openai_provider.py` around lines 73 - 82, The code is persisting legacy chat mode too eagerly (e.g., on RuntimeError or generic "400" substrings); change the logic in _generate_via_responses and the other responses-fallback block so that you still add base_url to the in-process _fallback_to_chat_cache for transient/ambiguous errors but only call self._persist_legacy_flag(True) when there is a definitive endpoint incompatibility (explicit exception types like openai.NotFoundError/openai.BadRequestError or clear, unambiguous messages such as "404", "Not Found", "INVALID_ARGUMENT" that indicate the endpoint itself is unsupported); remove persistent fallback for plain RuntimeError or generic "400" text and keep those as in-memory-only fallbacks.
🧹 Nitpick comments (1)
application/world/services/auto_bible_generator.py (1)
219-220: Demote these payload-inspection logs to DEBUG.These messages look like troubleshooting output rather than operator-facing events, so leaving them at INFO will add noise on every successful worldbuilding run.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@application/world/services/auto_bible_generator.py` around lines 219 - 220, The two informational logs that print payload inspection (logger.info(f"Worldbuilding generated, keys: {bible_data.keys()}") and logger.info(f"Has worldbuilding key: {'worldbuilding' in bible_data}")) should be demoted to DEBUG to avoid noisy operator-facing logs; locate these calls in auto_bible_generator.py (where logger and bible_data are used) and change logger.info to logger.debug for both messages so the details remain available when debug-level logging is enabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@application/ai/knowledge_llm_contract.py`:
- Around line 154-156: The code currently computes missing_braces = brace_count
+ bracket_count and appends only '}' characters to content, which corrupts
truncated JSON arrays; change the fix-up to append the correct closing
characters for each type by using bracket_count and brace_count separately
(append ']' repeated bracket_count and then '}' repeated brace_count to content)
instead of appending only '}' (references: missing_braces, brace_count,
bracket_count, content).
In `@infrastructure/ai/providers/openai_provider.py`:
- Around line 37-38: The fallback cache is currently keyed only by base_url
which causes profile-level persistence to be shared across accounts; change all
accesses and updates of _fallback_to_chat_cache (including where use_responses
reads it and the places noted around the existing uses) to use a profile-scoped
cache key derived from both base_url and self._profile_id (e.g., a tuple
(self._base_url, self._profile_id) or a string
f"{self._base_url}:{self._profile_id}"); update the code paths that set, get,
and delete entries in _fallback_to_chat_cache (referencing use_responses,
_fallback_to_chat_cache, and self._profile_id) so the lookup, assignment, and
eviction are done with this combined key to ensure per-profile behavior.
---
Outside diff comments:
In `@application/ai/knowledge_llm_contract.py`:
- Around line 108-114: The fenced-JSON branch swallows JSON decode exceptions
causing callers to get None; in the block that handles match.group(1) (where
rsp_json = json.loads(match.group(1).strip() is attempted), either re-raise the
caught json.JSONDecodeError (or ValueError/TypeError) so callers see the
original parse failure, or invoke the existing recovery/repair logic used for
the unrouted branch (the same path taken when using rsp and the downstream
repair routines) instead of using pass; update the exception handler around
json.loads(match.group(1).strip()) to propagate the error or route to the repair
function so rsp_json is not silently set to None.
In `@infrastructure/ai/providers/openai_provider.py`:
- Around line 153-158: The stream generator (stream_generate) currently skips
malformed/empty chunks via _extract_text_from_stream_chunk and can finish
without yielding anything, causing callers to receive a blank completion; update
stream_generate (and the analogous block at the other location) to detect when
the stream completed with zero emitted content and raise a retryable error (or
propagate an appropriate exception) instead of returning silently — track
whether any content was yielded while iterating the async stream, and if none
was produced after the stream ends, raise an error (e.g., a specific
OpenAI-related or custom transient exception) so callers can handle retries.
---
Duplicate comments:
In `@infrastructure/ai/providers/openai_provider.py`:
- Around line 73-82: The code is persisting legacy chat mode too eagerly (e.g.,
on RuntimeError or generic "400" substrings); change the logic in
_generate_via_responses and the other responses-fallback block so that you still
add base_url to the in-process _fallback_to_chat_cache for transient/ambiguous
errors but only call self._persist_legacy_flag(True) when there is a definitive
endpoint incompatibility (explicit exception types like
openai.NotFoundError/openai.BadRequestError or clear, unambiguous messages such
as "404", "Not Found", "INVALID_ARGUMENT" that indicate the endpoint itself is
unsupported); remove persistent fallback for plain RuntimeError or generic "400"
text and keep those as in-memory-only fallbacks.
---
Nitpick comments:
In `@application/world/services/auto_bible_generator.py`:
- Around line 219-220: The two informational logs that print payload inspection
(logger.info(f"Worldbuilding generated, keys: {bible_data.keys()}") and
logger.info(f"Has worldbuilding key: {'worldbuilding' in bible_data}")) should
be demoted to DEBUG to avoid noisy operator-facing logs; locate these calls in
auto_bible_generator.py (where logger and bible_data are used) and change
logger.info to logger.debug for both messages so the details remain available
when debug-level logging is enabled.
🪄 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: 8a1e0a9d-fc0e-49fd-8d20-003e90277217
📒 Files selected for processing (4)
application/ai/knowledge_llm_contract.pyapplication/world/services/auto_bible_generator.pyinfrastructure/ai/providers/openai_provider.pytests/unit/application/services/test_auto_bible_generator.py
Comment on lines
+154
to
+156
| missing_braces = brace_count + bracket_count | ||
| if missing_braces > 0: | ||
| content = content + '}' * missing_braces |
Contributor
There was a problem hiding this comment.
Close missing arrays with ], not }.
brace_count + bracket_count is correct for detecting imbalance, but appending only } corrupts truncated payloads that are missing a closing array. For inputs ending inside [...], this turns a recoverable response into invalid JSON.
Suggested fix
- missing_braces = brace_count + bracket_count
- if missing_braces > 0:
- content = content + '}' * missing_braces
+ if bracket_count > 0:
+ content += "]" * bracket_count
+ if brace_count > 0:
+ content += "}" * brace_count🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@application/ai/knowledge_llm_contract.py` around lines 154 - 156, The code
currently computes missing_braces = brace_count + bracket_count and appends only
'}' characters to content, which corrupts truncated JSON arrays; change the
fix-up to append the correct closing characters for each type by using
bracket_count and brace_count separately (append ']' repeated bracket_count and
then '}' repeated brace_count to content) instead of appending only '}'
(references: missing_braces, brace_count, bracket_count, content).
Comment on lines
+37
to
38
| self._profile_id: Optional[str] = getattr(settings, "profile_id", None) | ||
|
|
Contributor
There was a problem hiding this comment.
Per-profile persistence is undermined by the global fallback cache.
These changes make fallback state profile-specific, but use_responses still consults _fallback_to_chat_cache by base_url only. If one profile/account on a shared gateway hits an account-scoped fallback case, every other profile on the same base_url will skip Responses in-process even when their own persisted flag is still False. Key the cache by (base_url, profile_id) or another account-scoped identity.
Also applies to: 76-82, 139-145
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@infrastructure/ai/providers/openai_provider.py` around lines 37 - 38, The
fallback cache is currently keyed only by base_url which causes profile-level
persistence to be shared across accounts; change all accesses and updates of
_fallback_to_chat_cache (including where use_responses reads it and the places
noted around the existing uses) to use a profile-scoped cache key derived from
both base_url and self._profile_id (e.g., a tuple (self._base_url,
self._profile_id) or a string f"{self._base_url}:{self._profile_id}"); update
the code paths that set, get, and delete entries in _fallback_to_chat_cache
(referencing use_responses, _fallback_to_chat_cache, and self._profile_id) so
the lookup, assignment, and eviction are done with this combined key to ensure
per-profile behavior.
This was referenced Apr 17, 2026
Merged
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
变更说明
1. 代码质量修复（P0/P1优先级）
except:捕获 - 改为具体异常类型（json.JSONDecodeError,ValueError,TypeError,AttributeError,KeyError等），避免吞掉KeyboardInterrupt、SystemExit等系统异常_call_llm_and_parse_with_retry未使用last_error- 增强错误日志，记录最后一次失败原因print()调试语句 - 改为logger.debug()/logger.error()（共15处，分布在3个文件）parse_json_from_response函数 - 统一使用application.ai.knowledge_llm_contract中的版本_sanitize_llm_json_output,_extract_outer_json_object,_repair_json_string（约100行死代码）response.choices[0]和chunk.choices[0]增加长度检查2. 合并 PR #59 增强功能
update_profile_legacy_flag方法，自动保存降级状态到数据库profile_id字段：支持识别当前配置profile_id：确保provider能持久化配置_persist_legacy_flag方法：fallback时自动保存到数据库parse_llm_json_to_dict，增强容错性3. 已修改文件列表
application/ai/knowledge_llm_contract.py- 修复裸exceptapplication/ai/llm_control_service.py- 添加update_profile_legacy_flag（PR#59）application/analyst/services/llm_voice_analysis_service.py- 修复裸exceptapplication/analyst/services/tension_analyzer.py- 改进JSON解析（PR#59）application/world/services/auto_bible_generator.py- 修复裸except、清理调试代码、删除死代码infrastructure/ai/config/settings.py- 添加profile_id字段（PR#59）infrastructure/ai/provider_factory.py- 传递profile_id（PR#59）infrastructure/ai/providers/openai_provider.py- 修复空指针、添加_persist_legacy_flag（PR#59）interfaces/api/v1/blueprint/continuous_planning_routes.py- 清理print调试语句interfaces/api/v1/engine/autopilot_routes.py- 修复裸exceptscripts/evaluation/macro_planning_evaluator.py- 修复裸except测试
风险与回滚
署名：我本凡尘
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Tests