修复了人物，地图，不按照自己设定大纲生成的bug，角色锚点始终为空，流式地点生成父子层级全部丢失#182
Open
QishanHe wants to merge 1 commit into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR enhances prompt variable handling infrastructure and restructures character/location generation prompts to use consistent dotted-key naming, enables protagonist computation in location workflows, optimizes location persistence via batch-saving after streaming completes, and includes minor cleanups in import and API response logic. ChangesPrompt Variable Structure and Location Persistence Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@application/world/services/auto_bible_generator.py`:
- Line 1218: The protagonist selection falls back to the first character because
_load_characters returns only name/description (no "role"), so the protagonist =
next((c for c in characters if c.get("role") == "主角"), ...) in the protagonist
selection (used at protagonist assignment and the similar selection at Line
1261) never matches; fix by changing _load_characters to include the role key
for each character (ensure returned character dicts include "role") or,
alternatively, change the selection logic in protagonist and the other
occurrence to match using available fields (e.g., check a configured
protagonist_name or match on c.get("name")) so the selector can succeed even
with persisted character shapes.
In `@infrastructure/ai/prompt_template_engine.py`:
- Around line 491-494: When unflattening dotted keys ensure you never overwrite
an existing scalar: in the loop that walks parts (variables parts, part, d),
change the logic so if d.get(part) exists and is not a dict you stop creating
deeper dicts and skip adding the nested value (preserving the original scalar),
instead of doing d[part] = {} as currently done; only assign d[parts[-1]] =
value when the traversal successfully yields a dict at that position. This keeps
the "existing key wins" contract and prevents silently discarding scalar keys
like novel when novel.title is present.
In `@interfaces/api/v1/world/bible.py`:
- Around line 915-917: The deferred batch save using
bible_generator._prepare_locations_for_save over locs_payload delays persistence
until streaming completes, risking loss if the SSE task is cancelled; modify the
streaming handler to persist incrementally and on cancellation by (a) flushing
prepared locations periodically inside the streaming loop (use the same
bible_generator._prepare_locations_for_save(novel_id, locs_payload) call on each
chunk and clear locs_payload after successful save) or (b) wrapping the stream
in a try/except/finally that catches asyncio.CancelledError (and other
exceptions) and invokes the save logic to flush any remaining locs_payload for
novel_id before re-raising; ensure dedup/atomicity of raw_to_final mapping is
preserved when saving partial batches.
- Around line 928-929: The except Exception: pass in the SSE location
persistence batch-save block silently swallows write failures; change this to
catch the exception, log the error (include exception details) and update/track
a saved_count based on successful writes (or decrement generated_count on
failure) instead of always reporting the generated_count; ensure the SSE
completion/reporting logic uses the actual saved_count and/or emits an error
event when saves fail (referencing the batch-save block that contains "except
Exception: pass", the generated_count variable, and the final
completion/reporting code that currently reports completion).
🪄 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: ce7a8ab0-b51b-44a8-b34f-cbffbcc32a2b
📒 Files selected for processing (5)
application/engine/services/context_budget_allocator.pyapplication/world/services/auto_bible_generator.pyinfrastructure/ai/prompt_template_engine.pyinterfaces/api/v1/core/settings.pyinterfaces/api/v1/world/bible.py
💤 Files with no reviewable changes (1)
- application/engine/services/context_budget_allocator.py
| wb_fields = build_worldbuilding_prompt_fields(worldbuilding_slices=worldbuilding) | ||
| wb_summary = wb_fields.get("worldbuilding_full", "") | ||
| char_summary = "\n".join([f"- {c['name']}: {c['description'][:50]}..." for c in characters]) | ||
| protagonist = next((c for c in characters if c.get("role") == "主角"), characters[0] if characters else {}) |
Contributor
There was a problem hiding this comment.
Protagonist selection currently degrades to “first character” in persisted flows.
At Line 1218 and Line 1261, selection depends on c.get("role") == "主角", but _load_characters (Line 845) only returns name and description. In stage=locations and SSE location generation, this means role matching never works and always falls back to index 0.
🔧 Suggested fix (root cause: loader shape)
def _load_characters(self, novel_id: str) -> list:
"""加载已有人物"""
try:
bible = self.bible_service.get_bible_by_novel(novel_id)
if bible is None:
return []
- return [{"name": c.name, "description": c.description} for c in bible.characters]
+ return [
+ {
+ "name": c.name,
+ "description": c.description,
+ # Prefer explicit field when available; fallback parses legacy "角色 - 描述"
+ "role": getattr(c, "role", None) or str(c.description or "").split(" - ", 1)[0],
+ }
+ for c in bible.characters
+ ]
except Exception:
return []Also applies to: 1261-1261
🤖 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/world/services/auto_bible_generator.py` at line 1218, The
protagonist selection falls back to the first character because _load_characters
returns only name/description (no "role"), so the protagonist = next((c for c in
characters if c.get("role") == "主角"), ...) in the protagonist selection (used at
protagonist assignment and the similar selection at Line 1261) never matches;
fix by changing _load_characters to include the role key for each character
(ensure returned character dicts include "role") or, alternatively, change the
selection logic in protagonist and the other occurrence to match using available
fields (e.g., check a configured protagonist_name or match on c.get("name")) so
the selector can succeed even with persisted character shapes.
Comment on lines
+491
to
+494
| if part not in d or not isinstance(d[part], dict): | ||
| d[part] = {} | ||
| d = d[part] | ||
| d[parts[-1]] = value |
Contributor
There was a problem hiding this comment.
Do not overwrite scalar keys during dotted-key unflattening.
At Line 491–Line 493, a pre-existing non-dict key is replaced with {}. If both novel and novel.title are provided, this silently discards novel, which contradicts the function contract (“已有 key 优先”) and can change rendered prompts unexpectedly.
🔧 Suggested fix
for key, value in variables.items():
if "." not in key:
result[key] = value
continue
parts = key.split(".")
d = result
+ conflict = False
for part in parts[:-1]:
- if part not in d or not isinstance(d[part], dict):
- d[part] = {}
+ if part not in d:
+ d[part] = {}
+ elif not isinstance(d[part], dict):
+ # Keep existing key as-is (existing key wins)
+ conflict = True
+ break
d = d[part]
- d[parts[-1]] = value
+ if conflict:
+ continue
+ if parts[-1] not in d:
+ d[parts[-1]] = value🤖 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 `@infrastructure/ai/prompt_template_engine.py` around lines 491 - 494, When
unflattening dotted keys ensure you never overwrite an existing scalar: in the
loop that walks parts (variables parts, part, d), change the logic so if
d.get(part) exists and is not a dict you stop creating deeper dicts and skip
adding the nested value (preserving the original scalar), instead of doing
d[part] = {} as currently done; only assign d[parts[-1]] = value when the
traversal successfully yields a dict at that position. This keeps the "existing
key wins" contract and prevents silently discarding scalar keys like novel when
novel.title is present.
Comment on lines
+915
to
+917
| # 流式收集完毕后批量落库，保证 raw_to_final 映射完整，parent_id 层级正确 | ||
| for pd in bible_generator._prepare_locations_for_save(novel_id, locs_payload): | ||
| try: |
Contributor
There was a problem hiding this comment.
Deferred save introduces a cancellation window where all locations can be lost.
At Line 915–Line 917, persistence is postponed until streaming ends. If the SSE task is cancelled/disconnected before completion, collected locs_payload is never flushed, so generated locations may be entirely dropped.
🧰 Tools
🪛 Ruff (0.15.15)
[warning] 915-915: Comment contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
[warning] 915-915: Comment contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
🤖 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 `@interfaces/api/v1/world/bible.py` around lines 915 - 917, The deferred batch
save using bible_generator._prepare_locations_for_save over locs_payload delays
persistence until streaming completes, risking loss if the SSE task is
cancelled; modify the streaming handler to persist incrementally and on
cancellation by (a) flushing prepared locations periodically inside the
streaming loop (use the same
bible_generator._prepare_locations_for_save(novel_id, locs_payload) call on each
chunk and clear locs_payload after successful save) or (b) wrapping the stream
in a try/except/finally that catches asyncio.CancelledError (and other
exceptions) and invokes the save logic to flush any remaining locs_payload for
novel_id before re-raising; ensure dedup/atomicity of raw_to_final mapping is
preserved when saving partial batches.
Comment on lines
+928
to
+929
| except Exception: | ||
| pass |
Contributor
There was a problem hiding this comment.
Do not swallow batch-save failures in SSE location persistence.
At Line 928–Line 929, except Exception: pass hides write failures; then Line 935 still reports completion using generated count, not saved count. This can produce false-success UX and silent data loss.
🔧 Suggested fix
+ failed_locations: list[str] = []
for pd in bible_generator._prepare_locations_for_save(novel_id, locs_payload):
try:
bible_generator.bible_service.add_location(
novel_id=novel_id,
location_id=pd["location_id"],
@@
)
location_ids.append((pd["location_id"], pd))
- except Exception:
- pass
+ except Exception as e:
+ failed_locations.append(pd.get("location_id", ""))
+ logger.warning("Failed to save streamed location %s: %s", pd.get("location_id"), e)
+
+ if failed_locations:
+ yield _sse_fmt("error", {
+ "message": f"部分地点保存失败: {len(failed_locations)} 个",
+ "failed_location_ids": failed_locations,
+ })Also applies to: 935-935
🧰 Tools
🪛 Ruff (0.15.15)
[error] 928-929: try-except-pass detected, consider logging the exception
(S110)
[warning] 928-928: Do not catch blind exception: Exception
(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 `@interfaces/api/v1/world/bible.py` around lines 928 - 929, The except
Exception: pass in the SSE location persistence batch-save block silently
swallows write failures; change this to catch the exception, log the error
(include exception details) and update/track a saved_count based on successful
writes (or decrement generated_count on failure) instead of always reporting the
generated_count; ensure the SSE completion/reporting logic uses the actual
saved_count and/or emits an error event when saves fail (referencing the
batch-save block that contains "except Exception: pass", the generated_count
variable, and the final completion/reporting code that currently reports
completion).
Source: Linters/SAST tools
Author
|
CI 失败与本次改动无关。test_auto_novel_generation_workflow.py 尝试从 application.workflows.auto_novel_generation_workflow 导入 ChapterPromptTemplateUnavailable，但该名称在原仓库 master 分支上同样不存在，属于原仓库已有的 broken test，并非本 PR 引入。 |
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
1 participant
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.
问题描述
首次真实运行时发现的 5 个 Bug，均在本次修复。
Fix 1 — PUT /settings/embedding 返回 500
文件： interfaces/api/v1/core/settings.py
update_embedding_config 将 svc.update_config() 的返回值（EmbeddingConfigModel，Pydantic 模型）误当服务对象调用了不存在的 .to_api_dict() 方法，导致保存 embedding 配置时 500。
Before
return updated.to_api_dict()
After
return svc.to_api_dict()
Fix 2 — Jinja2 渲染失败：'novel' is undefined
文件： infrastructure/ai/prompt_template_engine.py
CPMS 模板使用 {{ novel.title }} 点号命名空间，但调用方传的是扁平字符串 key（"novel.title": "x"）。Jinja2 找不到顶层 novel 对象，触发 UndefinedError 并回退 format_map，导致 {% if %}/{% for %} 控制块作为字面文本传给 LLM，生成质量严重下降。
新增 _unflatten_variables 方法，渲染前将扁平 key 展开为嵌套 dict：
{"novel.title": "x", "novel.premise": "y"} → {"novel": {"title": "x", "premise": "y"}}
Fix 3 — 人物 / 地点生成内容混乱
文件： application/world/services/auto_bible_generator.py
_generate_characters、_stream_generate_characters、_generate_locations、_stream_generate_locations 传给模板的变量名是旧式平名（premise、existing_characters、character_context 等），与 CPMS 模板声明的 novel.、worldbuilding.、characters.* 命名空间不匹配。Jinja2 渲染失败后 LLM 收到满是 {{ novel.* }} 占位符的 prompt，人物和地点生成结果不可用。
四处调用统一改为与模板变量声明一致的点号命名空间。
Fix 4 — 流式地点生成父子层级全部丢失
文件： interfaces/api/v1/world/bible.py
流式端点对每个 location chunk 单独调用 _prepare_locations_for_save(novel_id, [单条])，函数内部的 raw_to_final 映射表每次只包含当前一条，子地点引用的父 ID 永远查不到，全部降级为根节点，位置树变为完全扁平结构。
改为等流式收集全部完成后整批调用 _prepare_locations_for_save(novel_id, locs_payload)，确保父子关系正确保存。SSE 进度事件仍实时推送。
Fix 5 — 角色锚点始终为空
文件： application/engine/services/context_budget_allocator.py
_get_character_anchors 内部有一行多余的局部 import NovelId。Python 规则：函数体内任何地方对名字赋值（import 也是赋值），该名字在整个函数范围内视为局部变量，遮蔽了模块顶部的同名 import。导致在局部 import 执行之前访问 NovelId 时触发 UnboundLocalError，被 except 吞掉后角色锚点始终返回空字符串，写作上下文中角色心理状态、隐藏信息、地点提示全部失效。
删除该局部 import 一行即可修复。
架构影响
涉及层级：application / infrastructure / interfaces
未新增数据库表/字段
未修改 API 路径或类型契约
风险说明
潜在风险：无
Fix 4 将地点落库从流式即时改为批量，延迟约等于所有地点生成完毕的时间（通常数秒），SSE 进度显示不受影响
Summary by CodeRabbit
Release Notes
Bug Fixes
Improvements