fix: Storyline 自动提取后无法匹配已有故事线#192
Conversation
- Add fuzzy name matching in _resolve_storyline_by_name() - Implement exact, normalized, and substring match strategies - Auto-create missing storylines when not found - Add comprehensive test coverage for storyline resolution Closes shenminglinyi#187
📝 WalkthroughWalkthroughStateUpdater's chapter-to-storyline update now uses a fallback resolution strategy: attempt ID-based lookup, then fuzzy name matching (exact, normalized, substring); if no existing storyline is found but a name is provided, auto-create a new active storyline initialized with LLM-provided progress. Comprehensive tests validate all matching modes and edge cases. ChangesStoryline Resolution and Auto-Creation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
application/analyst/services/state_updater.py (1)
390-390: ⚡ Quick winConsider using
List[Storyline]for consistency.Line 390 uses
list[Storyline](PEP 585 syntax, Python 3.9+) while the file imports and usesListfromtypingelsewhere (line 4). For consistency with the rest of the file's type annotations, consider usingList[Storyline]instead.♻️ Proposed fix
- substring_matches: list[Storyline] = [] + substring_matches: List[Storyline] = []🤖 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/analyst/services/state_updater.py` at line 390, The declaration of substring_matches uses PEP 585 syntax "list[Storyline]" which is inconsistent with the rest of the file that imports and uses "List" from typing; change the type annotation for the variable "substring_matches" to use "List[Storyline]" so it matches other annotations and the imported typing symbol "List" (ensure "Storyline" remains the element type and no additional imports are required).
🤖 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 `@tests/unit/application/services/test_state_updater_storyline_resolution.py`:
- Line 27: The test helper _make_storyline currently uses a parameter named id
which shadows the Python builtin; rename that parameter to a more specific
identifier (e.g., storyline_id or sl_id) in the _make_storyline function
signature and update all uses within the function and its callers to use the new
name so the builtin is not shadowed (refer to symbol _make_storyline to locate
and change the parameter and its references).
---
Nitpick comments:
In `@application/analyst/services/state_updater.py`:
- Line 390: The declaration of substring_matches uses PEP 585 syntax
"list[Storyline]" which is inconsistent with the rest of the file that imports
and uses "List" from typing; change the type annotation for the variable
"substring_matches" to use "List[Storyline]" so it matches other annotations and
the imported typing symbol "List" (ensure "Storyline" remains the element type
and no additional imports are required).
🪄 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: 5c34330b-bb02-42fd-9b50-aa4f15fa7038
📒 Files selected for processing (2)
application/analyst/services/state_updater.pytests/unit/application/services/test_state_updater_storyline_resolution.py
| ) | ||
| self.novel_id = NovelId("novel-test") | ||
|
|
||
| def _make_storyline(self, id: str, name: str) -> Storyline: |
There was a problem hiding this comment.
Rename parameter to avoid shadowing Python builtin.
The parameter name id shadows the Python builtin function. Consider using a more specific name like storyline_id or sl_id.
🛠️ Proposed fix
- def _make_storyline(self, id: str, name: str) -> Storyline:
+ def _make_storyline(self, storyline_id: str, name: str) -> Storyline:
return Storyline(
- id=id,
+ id=storyline_id,
novel_id=self.novel_id,
storyline_type=StorylineType.GROWTH,
status=StorylineStatus.ACTIVE,
estimated_chapter_start=1,
estimated_chapter_end=50,
name=name,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _make_storyline(self, id: str, name: str) -> Storyline: | |
| def _make_storyline(self, storyline_id: str, name: str) -> Storyline: | |
| return Storyline( | |
| id=storyline_id, | |
| novel_id=self.novel_id, | |
| storyline_type=StorylineType.GROWTH, | |
| status=StorylineStatus.ACTIVE, | |
| estimated_chapter_start=1, | |
| estimated_chapter_end=50, | |
| name=name, | |
| ) |
🧰 Tools
🪛 Ruff (0.15.15)
[error] 27-27: Function argument id is shadowing a Python builtin
(A002)
🤖 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 `@tests/unit/application/services/test_state_updater_storyline_resolution.py`
at line 27, The test helper _make_storyline currently uses a parameter named id
which shadows the Python builtin; rename that parameter to a more specific
identifier (e.g., storyline_id or sl_id) in the _make_storyline function
signature and update all uses within the function and its callers to use the new
name so the builtin is not shadowed (refer to symbol _make_storyline to locate
and change the parameter and its references).
Source: Linters/SAST tools
1 participant
Summary
_resolve_storyline_by_name()methodProblem
Issue #187: LLM 提取的故事线名称与存储的名称不一致，导致 StateUpdater 无法找到对应故事线，造成故事线状态丢失和主线推进记录异常。
Solution
模糊名称匹配：在
state_updater.py中新增_resolve_storyline_by_name()方法，支持：自动创建缺失故事线：当模糊匹配仍无法找到时，自动创建新故事线并保存，避免故事线状态丢失。
测试覆盖：新增 6 个测试用例覆盖各种匹配场景。
Test plan
Closes #187
Summary by CodeRabbit
Improvements
Tests