fix(template): skip stale character selections during generation#266
fix(template): skip stale character selections during generation#266RachelForster wants to merge 5 commits into
Conversation
Reviewer's Guide by Ameath小爱整理了一条清晰的核对路径，顺着看就好。 审查从 文件级变更
Tips and commands
Original review guide in EnglishThe review traced |
|
Closes RachelForster/Shinsekai/#265 |
ameath-review
Bot
changed the title
![ameath-review[bot]](https://avatars.githubusercontent.com/in/4276791?s=60&v=4){kind=small}
There was a problem hiding this comment.
小爱会把需要留意的地方逐一标出来，避免遗漏。
小爱已完成全部变更审查，共发布 3 条行级建议（高风险 0，中风险 2）。
合并前关注点（风险 1 · 建议测试 0）
风险
- PR 元数据未链接现有 issue；
CONTRIBUTING.md要求每个 PR 在描述中链接已有 issue。
Prompt for AI Agents
请逐项处理以下审查问题；修改前先核实上下文，修改后运行相关测试。
1. 全失效选择被当作成功结果 / All-stale selection is treated as success
llm/template_generator.py:310 (RIGHT)
<issue_to_address>恢复的选择只包含已删除或改名角色时，这里返回 (err_no_characters, "")。/api/templates/generate 会将第一个值写入 TemplateSummary.system 并以 200 返回，因此模板编辑器不会进入错误路径；它只检查 selectedCharacters.length，用户点击生成后，onSuccess 会用“未选择角色”文案覆盖恢复的系统提示词。请把“无有效角色”作为可区分的失败结果传给 API/UI，或在应用该响应前保留现有草稿。
Original in English
When a restored selection contains only deleted or renamed characters, this returns (err_no_characters, ""). /api/templates/generate stores the first value in TemplateSummary.system and returns HTTP 200, so the template editor does not take its error path. It checks only selectedCharacters.length; after the user clicks Generate, onSuccess replaces the restored system prompt with the “no characters selected” text. Return “no valid characters” as a distinguishable failure to the API/UI, or preserve the current draft before applying this response.
2. 解析后的角色选择未同步到会话 / Resolved selection is not synchronized
llm/template_generator.py:314 (RIGHT)
<issue_to_address>这里仅重绑定了生成器的局部变量，恢复会话中的 selectedCharacters 没有更新。例如 ['Deleted', 'Alice'] 会生成只允许 Alice 的提示词，但模板编辑器随后仍保存并启动原数组；桥接层把首项设为 characterName，初始立绘按首项解析，子进程参数和历史标识也保留 Deleted。因此这个新成功路径会让运行时角色集、默认初始立绘和模板提示词失去一致性。请将规范化后的有效角色名回传并替换会话及启动载荷中的选择。
Original in English
This only rebinds a local generator variable; the restored session’s selectedCharacters is not updated. For example, ['Deleted', 'Alice'] generates a prompt that permits only Alice, but the template editor later saves and launches the original array. The bridge uses the first entry as characterName, resolves the default initial sprite from that first entry, and preserves Deleted in the child-process arguments and history identity. This newly successful path leaves the runtime character set, default initial sprite, and template prompt inconsistent. Return the canonical resolved names and replace the selection in the session and launch payload.
3. 角色去重规则与配置查询不一致 / Character deduplication differs from config lookup
llm/template_generator.py:295 (RIGHT)
<issue_to_address>配置层的 get_character_by_name() 以及创建时的重名判断都使用 .lower()，因此 Straße 和 STRASSE 可以作为两个独立角色存在并分别解析；但这里的 .casefold() 会把它们视为同一角色，导致其中一个被跳过。由于 requested_names 先经过 set 且两者排序键相同，保留下来的角色还会随进程的集合迭代顺序变化。请复用配置层的角色身份归一化规则。
Original in English
get_character_by_name() and duplicate detection in the configuration layer use .lower(), so Straße and STRASSE can exist and resolve as separate characters. This new .casefold() key treats them as one and drops one selection. Because requested_names is first made into a set and their sort keys tie, which character survives can also vary with set iteration order. Reuse the configuration layer’s identity normalization.
| "missing_characters": missing_characters, | ||
| }, | ||
| ) | ||
| if not resolved_characters: |
There was a problem hiding this comment.
[MEDIUM · bug] 全失效选择被当作成功结果 / All-stale selection is treated as success
恢复的选择只包含已删除或改名角色时，这里返回 (err_no_characters, "")。/api/templates/generate 会将第一个值写入 TemplateSummary.system 并以 200 返回，因此模板编辑器不会进入错误路径；它只检查 selectedCharacters.length，用户点击生成后，onSuccess 会用“未选择角色”文案覆盖恢复的系统提示词。请把“无有效角色”作为可区分的失败结果传给 API/UI，或在应用该响应前保留现有草稿。
Original in English
When a restored selection contains only deleted or renamed characters, this returns (err_no_characters, ""). /api/templates/generate stores the first value in TemplateSummary.system and returns HTTP 200, so the template editor does not take its error path. It checks only selectedCharacters.length; after the user clicks Generate, onSuccess replaces the restored system prompt with the “no characters selected” text. Return “no valid characters” as a distinguishable failure to the API/UI, or preserve the current draft before applying this response.
置信度 99% · 可直接回复讨论 · 👍/👎 会影响后续审查
There was a problem hiding this comment.
小爱已经把这边的内容接住了，接下来就顺着往下处理吧。
尚未能从当前改动确认已修复。
generate_chat_template() 在全失效选择时仍返回：
return no_valid_characters_message(), ""这仍是普通的二元成功返回形状。现有线程所述问题发生在 API 层将第一个返回值写入 TemplateSummary.system 并返回 HTTP 200；当前提供的 llm/template_generator.py 上下文没有显示 API 或前端据此区分失败，因此该行为并未在这里被改变。
建议将“没有有效角色”显式建模为失败，例如：
- 生成器抛出/返回可判别的领域错误（如
NoValidCharactersError或GenerateTemplateResult(ok=False, error_code="no_valid_characters")）； /api/templates/generate将其映射为非 2xx（建议 422）及稳定的错误码；- 前端在该错误时展示提示且不覆盖当前
system草稿。
至少补充端到端或 API/UI 测试，覆盖“恢复的选择非空、但全部角色已删除/改名；点击生成后原有系统提示词保持不变”。
There was a problem hiding this comment.
这边的回复已经接上啦，小爱陪你把后面的事慢慢说清楚。
生成器侧已做出关键修复：当选择为空或解析后没有任何有效角色时，generate_chat_template() 现在会抛出 NoValidCharactersError，而不再返回可被误当作模板正文的 (err_no_characters, "")：
if not selected_characters:
raise NoValidCharactersError()
resolved_characters = self.resolve_chat_template_characters(selected_characters)
if not resolved_characters:
raise NoValidCharactersError()这使“无有效角色”成为可判别的失败信号，因此原评论所指出的生成器返回形状问题已修复。
但仅凭当前提供的 llm/template_generator.py 上下文，尚不能确认端到端完全修复：仍需确认 /api/templates/generate 专门捕获 NoValidCharactersError，返回非 2xx（例如 422）或稳定错误码，并且前端在该失败响应下不执行成功回调、不覆盖已有 system 草稿。
建议补充或确认以下回归测试：
- 恢复的角色列表非空但全部已删除/改名时，API 返回明确失败而非 200 的
TemplateSummary。 - 模板编辑器点击生成后展示错误提示，已有系统提示词保持不变。
|
|
||
| # 人物排序保证生成内容稳定；聊天记录默认文件名由设置页「用户情景」哈希决定。 | ||
| selected_characters = sorted(selected_characters) | ||
| selected_characters = [name for name, _character in resolved_characters] |
There was a problem hiding this comment.
[MEDIUM · correctness] 解析后的角色选择未同步到会话 / Resolved selection is not synchronized
这里仅重绑定了生成器的局部变量，恢复会话中的 selectedCharacters 没有更新。例如 ['Deleted', 'Alice'] 会生成只允许 Alice 的提示词，但模板编辑器随后仍保存并启动原数组；桥接层把首项设为 characterName，初始立绘按首项解析，子进程参数和历史标识也保留 Deleted。因此这个新成功路径会让运行时角色集、默认初始立绘和模板提示词失去一致性。请将规范化后的有效角色名回传并替换会话及启动载荷中的选择。
Original in English
This only rebinds a local generator variable; the restored session’s selectedCharacters is not updated. For example, ['Deleted', 'Alice'] generates a prompt that permits only Alice, but the template editor later saves and launches the original array. The bridge uses the first entry as characterName, resolves the default initial sprite from that first entry, and preserves Deleted in the child-process arguments and history identity. This newly successful path leaves the runtime character set, default initial sprite, and template prompt inconsistent. Return the canonical resolved names and replace the selection in the session and launch payload.
置信度 99% · 可直接回复讨论 · 👍/👎 会影响后续审查
There was a problem hiding this comment.
这边已经接上了，小爱把话题稳稳收住。
尚未修复。当前改动只是将解析逻辑抽到 resolve_chat_template_characters()，并在 generate_chat_template() 内部把局部 selected_characters 替换为规范名列表（第 326–331 行）；该列表仍未作为返回值或其他可消费的结果传给模板编辑器、会话状态及启动载荷。
因此 ['Deleted', 'Alice'] 的生成提示词会只包含 Alice，但恢复的会话和后续启动路径若仍使用原始 selectedCharacters，问题依旧存在：Deleted 仍可能被用作首角色、初始立绘解析目标、子进程参数和历史标识。
建议让角色解析的结果成为生成接口的显式输出（至少返回规范化后的名称列表），并在调用方成功生成后：
- 用该列表覆盖会话中的
selectedCharacters； - 用同一列表构建保存和启动载荷；
- 在启动前以规范化列表的首项计算
characterName与默认初始立绘。
还应补充端到端或调用方测试：恢复 ['Deleted', 'Alice'] 后，持久化会话、启动参数、characterName 和初始立绘均只对应 Alice。
There was a problem hiding this comment.
这边的回复已经整理好了，小爱陪你把话题稳稳接下去。
尚未确认修复；基于当前提供的代码，上述一致性问题仍然存在。
generate_chat_template() 仍然只在函数内部执行：
resolved_characters = self.resolve_chat_template_characters(selected_characters)
selected_characters = [name for name, _character in resolved_characters]随后用这个局部变量生成提示词，但从现有上下文看：
generate_chat_template()没有将规范化后的名称列表作为返回值返回；resolve_chat_template_characters()仅返回(canonical_name, character)元组，未见调用方消费其名称列表；- 未见模板编辑器会话的
selectedCharacters被覆盖； - 未见保存载荷、启动载荷、
characterName或默认初始立绘改为使用解析后的首个有效角色。
因此恢复 ['Deleted', 'Alice'] 时，模板内容可能只约束 Alice，但只要编辑器和桥接层仍保留原始数组，Deleted 仍会进入保存/启动流程并可能被用作首角色。这正是原评论指出的不一致。
建议修改接口或调用链，使规范化选择成为显式结果。例如让生成结果同时返回模板和：
resolved_names = [name for name, _ in resolved_characters]调用方在生成成功后必须统一：
session.selectedCharacters = resolved_names；- 保存与启动载荷使用
resolved_names，而非恢复出的原始数组； characterName、默认初始立绘、子进程参数和历史标识均从同一resolved_names派生；- 当列表为空时维持现有“无有效角色”错误路径，且不得保存或启动会话。
建议补充调用方/端到端回归测试：恢复 ['Deleted', 'Alice'] 后，断言会话持久化值、启动参数、characterName 及初始立绘均只使用 Alice。
1 participant
Summary
sprites: nullas having no spritesRoot cause
The template generator assumed every persisted character name still resolved to a character object and directly accessed
char_detail.sprites. A restored selection can outlive a deleted or renamed character, causingget_character_by_name()to returnNoneand the/api/templates/generaterequest to fail with anAttributeError.Impact
Template generation now continues when at least one selected character is valid. If every selected character is stale, it returns the existing no-character message instead of an HTTP 500. Missing character names are logged under
template.characters.missingfor diagnostics.Validation
Summary by Ameath
小爱先把这次核对的重点收拢一下，方便继续查看。
此 PR 在
TemplateGenerator中一次性解析角色选择、跳过失效条目并容忍sprites: null；审查发现全失效选择会被前端当作模板正文，解析后的选择未同步到会话启动，且去重规则与配置层的角色身份规则不一致。Enhancements
sprites为null时按零张立绘渲染。Tests
Original summary in English
This PR resolves selected characters once in
TemplateGenerator, skips stale entries, and toleratessprites: null; the review found that all-stale selections are treated as template content, resolved selections are not synchronized to session launch, and deduplication disagrees with the configuration layer’s character identity rules.Enhancements
sprites: nullas zero sprites.Tests