Skip to content

fix(bible): 修复角色关系指数增长导致数据库爆炸的问题 #160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ws1065 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(bible): 修复角色关系指数增长导致数据库爆炸的问题 #160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 36 additions & 10 deletions application/world/services/bible_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from domain.bible.repositories.bible_repository import BibleRepository
from domain.novel.repositories.novel_repository import NovelRepository
from domain.novel.repositories.chapter_repository import ChapterRepository
from domain.shared.exceptions import EntityNotFoundError
from domain.shared.exceptions import EntityNotFoundError, InvalidOperationError
from application.world.dtos.bible_dto import BibleDTO, CharacterDTO

if TYPE_CHECKING:
Expand Down Expand Up @@ -105,7 +105,7 @@ def add_character(
description: str,
relationships: list = None
) -> BibleDTO:
"""添加人物
"""添加人物（增量写入，不触碰已有角色和关系）

Args:
novel_id: 小说 ID
Expand All @@ -124,15 +124,41 @@ def add_character(
if bible is None:
raise EntityNotFoundError("Bible", f"for novel {novel_id}")

character = Character(
id=CharacterId(character_id),
name=name,
description=description,
relationships=relationships or [],
)
bible.add_character(character)
self.bible_repository.save(bible)
# 检查角色是否已存在
if bible.get_character(CharacterId(character_id)) is not None:
raise InvalidOperationError(
f"Character with id '{character_id}' already exists"
)

# 增量写入：只 INSERT 新角色及其关系，不 DELETE 其他数据
repo = self.bible_repository
if hasattr(repo, "add_character_incremental"):
rel_dicts = []
for rel in (relationships or []):
if isinstance(rel, dict):
rel_dicts.append(rel)
elif hasattr(rel, "model_dump"):
rel_dicts.append(rel.model_dump())
elif hasattr(rel, "dict"):
rel_dicts.append(rel.dict())
else:
rel_dicts.append({"target": str(rel), "relation": "", "description": ""})
repo.add_character_incremental(
novel_id, character_id, name, description, rel_dicts,
)
else:
# fallback：非 SQLite 仓储走原路径
character = Character(
id=CharacterId(character_id),
name=name,
description=description,
relationships=relationships or [],
)
bible.add_character(character)
repo.save(bible)

# 重新加载最新状态返回
bible = self.bible_repository.get_by_novel_id(NovelId(novel_id))
return BibleDTO.from_domain(bible)
Comment on lines +160 to 162
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 8, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Potential NoneType crash: BibleDTO.from_domain(bible) is called without a null-check after the reload.

get_by_novel_id returns Optional[Bible]. If the Bible is concurrently deleted between add_character_incremental and the reload on line 161, bible is None and BibleDTO.from_domain(None) will raise an AttributeError.

🐛 Proposed fix 
-        # 重新加载最新状态返回
-        bible = self.bible_repository.get_by_novel_id(NovelId(novel_id))
-        return BibleDTO.from_domain(bible)
+        # 重新加载最新状态返回
+        bible = self.bible_repository.get_by_novel_id(NovelId(novel_id))
+        if bible is None:
+            raise EntityNotFoundError("Bible", f"for novel {novel_id}")
+        return BibleDTO.from_domain(bible)
🤖 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/bible_service.py` around lines 160 - 162, The code
calls BibleDTO.from_domain(bible) without checking that bible (from
self.bible_repository.get_by_novel_id(NovelId(novel_id))) may be None; add a
null-check after the get_by_novel_id call and handle the missing Bible
explicitly — e.g., if bible is None raise a clear NotFound/DomainError (or
return an appropriate error/None response) instead of calling
BibleDTO.from_domain, updating the method containing add_character_incremental
(or the function where this reload occurs) to reference get_by_novel_id and
BibleDTO.from_domain so the None case is handled.


def update_character_voice_anchors(
Expand Down
45 changes: 45 additions & 0 deletions infrastructure/persistence/database/sqlite_bible_repository.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,51 @@ def exists(self, bible_id: str) -> bool:
r = self.db.fetch_one("SELECT 1 AS o FROM bibles WHERE id = ?", (bible_id,))
return r is not None

def add_character_incremental(
self,
novel_id: str,
character_id: str,
name: str,
description: str,
relationships: List[Dict[str, str]],
*,
mental_state: str = "NORMAL",
verbal_tic: str = "",
idle_behavior: str = "",
) -> None:
"""增量添加单个角色及其关系（不触碰其他角色）。"""
now = self._now()
conn = self._conn()
try:
conn.execute(
"""
INSERT OR REPLACE INTO bible_characters (
id, novel_id, name, description,
mental_state, mental_state_reason, verbal_tic, idle_behavior,
created_at, updated_at
) VALUES (?, ?, ?, ?, ?, '', ?, ?, ?, ?)
""",
(character_id, novel_id, name, description,
mental_state, verbal_tic, idle_behavior, now, now),
)
for i, rel in enumerate(relationships):
rid = f"{character_id}-rel-{i}-{uuid.uuid4().hex[:6]}"
target_name = rel.get("target", "") or ""
relation = rel.get("relation", "") or ""
desc = rel.get("description", "") or ""
conn.execute(
"""
INSERT INTO bible_character_relationships
(id, character_id, target_name, relation, description)
VALUES (?, ?, ?, ?, ?)
""",
(rid, character_id, target_name, relation, desc),
)
Comment on lines +345 to +357
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 8, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

add_character_incremental is not idempotent — stale relationships are not purged before re-insertion.

INSERT OR REPLACE on bible_characters (line 334) performs a DELETE+INSERT internally. Because SQLite foreign-key enforcement is off by default and the schema most likely has no explicit ON DELETE CASCADE, the existing relationship rows for character_id in bible_character_relationships are not removed when the character row is replaced. The subsequent INSERT loop then appends new rows on top of the survivors, re-introducing a smaller version of the original duplication bug.

The fix is a targeted DELETE at the top of the relationship loop:

🛡️ Proposed fix 
+            conn.execute(
+                "DELETE FROM bible_character_relationships WHERE character_id = ?",
+                (character_id,),
+            )
             for i, rel in enumerate(relationships):
                 rid = f"{character_id}-rel-{i}-{uuid.uuid4().hex[:6]}"

This makes the method self-consistent regardless of whether the service-level duplicate guard fires first (e.g., TOCTOU, direct repo invocation).

🤖 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/persistence/database/sqlite_bible_repository.py` around lines
345 - 357, add_character_incremental is not idempotent because existing rows in
bible_character_relationships are not removed before re-inserting relationships;
add a targeted DELETE to remove all relationship rows for the given character_id
before the loop that INSERTs new rows. Locate the function
add_character_incremental and, just before the for i, rel in
enumerate(relationships) loop, execute a parameterized DELETE like "DELETE FROM
bible_character_relationships WHERE character_id = ?" using the same
conn/transaction so old relationships are purged atomically prior to inserting
the new ones. Ensure you reference character_id when deleting and keep the
subsequent INSERT logic unchanged.

conn.commit()
except Exception:
conn.rollback()
raise

def update_character_anchors(
self,
novel_id: str,
Expand Down
14 changes: 9 additions & 5 deletions tests/unit/application/services/test_bible_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,13 @@ def test_create_bible(self, service, mock_repository):

def test_add_character(self, service, mock_repository):
"""测试添加人物"""
# 准备 mock 数据
bible = Bible(id="bible-1", novel_id=NovelId("novel-1"))
mock_repository.get_by_novel_id.return_value = bible
# 准备 mock 数据：第一次返回空 Bible，第二次返回含角色的 Bible（模拟增量写入后）
bible_empty = Bible(id="bible-1", novel_id=NovelId("novel-1"))
bible_with_char = Bible(id="bible-1", novel_id=NovelId("novel-1"))
bible_with_char.add_character(
Character(id=CharacterId("char-1"), name="主角", description="主角描述")
)
mock_repository.get_by_novel_id.side_effect = [bible_empty, bible_with_char]

bible_dto = service.add_character(
novel_id="novel-1",
Expand All @@ -56,8 +60,8 @@ def test_add_character(self, service, mock_repository):
assert bible_dto.characters[0].id == "char-1"
assert bible_dto.characters[0].name == "主角"

# 验证调用了 save
mock_repository.save.assert_called_once()
# 验证调用了增量写入
mock_repository.add_character_incremental.assert_called_once()

def test_add_character_bible_not_found(self, service, mock_repository):
"""测试向不存在的 Bible 添加人物"""
Expand Down