Skip to content

fix(planning): handle empty LLM response in macro plan parsing (#162)#171

Closed
liaoyl830 wants to merge 1 commit into
shenminglinyi:masterfrom
liaoyl830:fix/empty-llm-response-json-parse-error
Closed

fix(planning): handle empty LLM response in macro plan parsing (#162)#171
liaoyl830 wants to merge 1 commit into
shenminglinyi:masterfrom
liaoyl830:fix/empty-llm-response-json-parse-error

Conversation

@liaoyl830

@liaoyl830 liaoyl830 commented May 22, 2026

Copy link
Copy Markdown

变更类型

  • fix Bug 修复

变更说明

修复使用 glm-4.5-air 等模型时,LLM 偶尔返回空响应导致的 JSONDecodeError: Expecting value: line 1 column 1 (char 0) 错误。

根因

_parse_llm_response() 方法在清理 LLM 返回内容后,直接调用 json.loads()。当模型返回空字符串时(可能由于 API 限流、网络问题或模型自身问题),json.loads("") 会抛出难以理解的 JSONDecodeError。

修复

json.loads() 之前增加空内容检查,返回描述性 ValueError,便于 autopilot_daemon 的现有错误处理逻辑识别并优雅降级。


架构影响

  • 涉及层级:application
  • 是否修改现有 API 契约:否
  • 是否新增数据库表/字段:否

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced error handling with clearer messages when content processing fails
    • Improved EPUB export generation with proper navigation document inclusion for better file compatibility
    • Added diagnostic logging for content-related export issues

Review Change Stack

…inglinyi#162)

When using glm-4.5-air or other models that occasionally return empty
responses, _parse_llm_response would throw a cryptic JSONDecodeError
instead of a clear error message.

Add a check for empty cleaned content before attempting json.loads(),
raising a descriptive ValueError that autopilot_daemon can handle
gracefully via its existing error handling.
@liaoyl830 liaoyl830 requested a review from shenminglinyi as a code owner May 22, 2026 16:14
@coderabbitai

coderabbitai Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This PR adds defensive error handling and improves EPUB export robustness. LLM response parsing now validates and fails early with a user-friendly error when content is empty. EPUB export adds diagnostic logging for missing chapter content and explicitly generates proper EPUB 3 navigation documents.

Changes

Error Handling and Export Improvements

Layer / File(s) Summary
LLM response empty validation
application/blueprint/services/continuous_planning_service.py
_parse_llm_response checks for empty sanitized JSON content and immediately raises a descriptive ValueError to fail fast instead of proceeding with downstream parsing logic.
EPUB export diagnostics and navigation
application/core/services/export_service.py
export_novel scans chapters for empty content and logs warnings for diagnostics. EPUB generation now explicitly adds an EpubNav item to ensure EPUB 3 compliance with proper navigation documents.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • shenminglinyi/PlotPilot#40: Both PRs modify _parse_llm_response JSON parsing—the earlier PR adds sanitization/extraction/repair, and this PR adds an explicit empty-content guard to fail early with a user-friendly error message.

Suggested reviewers

  • shenminglinyi

Poem

🐰 A rabbit hops through parsing code,
Catching empty strings before they load!
EPUB pages dance in proper form,
Navigation blooming, shipshape and warm.
Early guards and logs so bright—
Keep the exports running right! 📚✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(planning): handle empty LLM response in macro plan parsing (#162)' directly matches the main change in the changeset, which is adding a guard to handle empty LLM responses in the _parse_llm_response method.
Description check ✅ Passed The description includes the required sections (变更类型, 变更说明, 架构影响) with complete information about the bug fix, root cause, and solution. However, the 测试 section with test commands and 风险说明 section are missing.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
application/core/services/export_service.py (1)

200-203: ⚡ Quick win

Include nav in spine for EPUB 3 reader compatibility.

At Line 203, EpubNav() is added, but the spine still excludes "nav". Some readers rely on nav being in reading order; adding it to spine improves compatibility.

Proposed change
-        book.spine = spine_items
+        book.spine = ["nav", *spine_items]
🤖 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 `@application/core/services/export_service.py` around lines 200 - 203, The EPUB
spine currently assigns book.spine = spine_items after adding
book.add_item(epub.EpubNav()), but it never includes the "nav" item id in the
spine; modify the spine construction so the nav id is included in the reading
order (e.g., insert or append "nav" into the spine_items list before assigning
book.spine) so the epub.EpubNav() item added by book.add_item(epub.EpubNav()) is
present in the spine for EPUB 3 compatibility.
🤖 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.

Nitpick comments:
In `@application/core/services/export_service.py`:
- Around line 200-203: The EPUB spine currently assigns book.spine = spine_items
after adding book.add_item(epub.EpubNav()), but it never includes the "nav" item
id in the spine; modify the spine construction so the nav id is included in the
reading order (e.g., insert or append "nav" into the spine_items list before
assigning book.spine) so the epub.EpubNav() item added by
book.add_item(epub.EpubNav()) is present in the spine for EPUB 3 compatibility.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f5b19975-a32b-4796-90ec-19ad7b0969cd

📥 Commits

Reviewing files that changed from the base of the PR and between 1008f81 and 3efe025.

📒 Files selected for processing (2)
  • application/blueprint/services/continuous_planning_service.py
  • application/core/services/export_service.py

@shenminglinyi

Copy link
Copy Markdown
Owner

感谢修复,这个问题当时确实会让宏观规划在模型空响应时直接掉进 JSONDecodeError,体验很差。

这块后续主线已经加入了空响应判断、输出清洗和 JSON repair 兜底,并且相关规划流程也有更大调整。当前 PR 基于旧上下文,继续合并会有比较高的冲突和重复实现风险,所以我先关闭。

如果后面还想继续参与,欢迎联系我加入 dev 仓库。我们可以先在 dev 分支/仓库里把具体模型兼容问题修稳、跑通,再合进开源 master。感谢你帮忙补这个坑。

@shenminglinyi

Copy link
Copy Markdown
Owner

再补充说明一下:这次关闭 PR 只是基于当前 master 稳定性、冲突风险和合并策略做的整理,不代表否定大家的贡献。

大家已经投入的工作我都看到了,也会统一计入项目贡献者名单/贡献记录里。很多思路后续仍然会被吸收,只是现在为了尽量避免开源 master 出问题,我会更倾向于先在 dev 仓库里拆小块开发、验证、跑通,再合并到 master。

如果后续还愿意一起推进,欢迎直接联系我,我可以开放 dev 仓库权限。我们按模块拆 PR,一块一块稳着来。再次感谢大家愿意在这么早期、不成熟的阶段帮忙共建,真的感谢。

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants