perf(backend,frontend): 优化核心性能，添加缓存，调整项目配置#164
Open
evil-ponsion wants to merge 4 commits into
Open
Conversation
详细改动如下： - 重构SQLite WAL checkpoint策略，拆分高频非阻塞PASSIVE检查点与低频收缩TRUNCATE逻辑 - 优化小说列表查询，将原1+3N次SQL查询优化为3次批量查询，添加模块级TTL缓存减少重复请求 - 重构全局统计接口，改用SQL直接聚合替代逐章遍历查询，添加TTL缓存，大幅提升大数据量场景下的查询速度 - 为提示词统计接口添加TTL缓存，降低重复调用开销 - 替换日志文件处理器为轮转式Handler，支持10MB自动轮转并保留3个备份文件 - 新增启动时自动安全清理SQLite WAL残留文件的逻辑 - 补充Python `__future__ annotations`导入适配现代类型提示 - 将项目版本从1.0.2升级至1.1.0
Contributor
📝 WalkthroughWalkthroughAdds TTL caches and optimized DB aggregation for novel lists and global stats, refines SQLite WAL checkpoint cadence, strengthens logging/startup imports, and hardens frontend autopilot polling and telemetry with aborts, timeouts, and visibility-aware scheduling. ChangesPerformance Optimization via Caching and Query Batching
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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
🧹 Nitpick comments (2)
interfaces/api/v1/workbench/llm_control.py (1)
313-332: ⚡ Quick winConsider invalidating prompt stats cache on write operations.
The new TTL cache improves performance, but write operations (create/update/delete prompts) only invalidate
_plaza_cachevia_invalidate_plaza_cache(), not the new_prompt_stats_cache. This can cause the/prompts/statsendpoint to return stale counts for up to 30 seconds after modifications.🔄 Proposed fix
Update
_invalidate_plaza_cache()to also clear the stats cache:def _invalidate_plaza_cache() -> None: """写操作后使缓存失效。""" - global _plaza_cache, _plaza_cache_ts + global _plaza_cache, _plaza_cache_ts, _prompt_stats_cache, _prompt_stats_cache_ts _plaza_cache = {} _plaza_cache_ts = 0.0 + _prompt_stats_cache = None + _prompt_stats_cache_ts = 0.0Alternatively, rename the function to reflect broader scope:
def _invalidate_prompt_caches() -> None: """写操作后使所有提示词相关缓存失效。""" global _plaza_cache, _plaza_cache_ts, _prompt_stats_cache, _prompt_stats_cache_ts _plaza_cache = {} _plaza_cache_ts = 0.0 _prompt_stats_cache = None _prompt_stats_cache_ts = 0.0🤖 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/workbench/llm_control.py` around lines 313 - 332, The prompt stats TTL cache (_prompt_stats_cache and _prompt_stats_cache_ts) is not being cleared on prompt write operations; update the invalidation logic so that _invalidate_plaza_cache() also resets _prompt_stats_cache and _prompt_stats_cache_ts (or rename to _invalidate_prompt_caches and clear both _plaza_cache/_plaza_cache_ts and _prompt_stats_cache/_prompt_stats_cache_ts) so /prompts/stats returns fresh data immediately after create/update/delete.application/engine/services/beat_middleware.py (1)
112-113: ⚡ Quick winRemove redundant
Anyimport.
Anyis already imported on line 36, making this duplicate import unnecessary. The original intent to avoid circular imports is now moot.♻️ Proposed fix
-# 避免循环导入 -from typing import Any -🤖 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/engine/services/beat_middleware.py` around lines 112 - 113, Remove the redundant "from typing import Any" import (the duplicate import that appears alongside the "# 避免循环导入" comment); keep the single existing "Any" import already declared earlier and delete this later duplicate import so there are no repeated imports of Any in beat_middleware.py.
🤖 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/core/services/novel_service.py`:
- Around line 211-213: Cached DTOs/lists are being stored and returned by
reference (cached["data"]) which allows callers to mutate shared objects; change
the cache writes and reads in NovelService so you store and return defensive
copies instead of the original objects — e.g., when setting cached["data"] wrap
the DTO/list in a deep copy or reconstruct DTOs (use copy.deepcopy or
DTO.from_dict/map over items) and when returning cached["data"] return a copy as
well (the same change needed for the other cache access at the second
occurrence, the code that reads/writes cached["data"] around line 239); ensure
timestamps (cached["ts"]) remain unchanged and only the data payload is cloned.
- Around line 235-236: The getattr calls eagerly evaluate the default
expressions causing _check_has_bible/_check_has_outline to run even when
novel._has_bible/_has_outline exist; change the pattern to use a unique sentinel
(or hasattr/try-except) so you first retrieve value = getattr(novel,
"_has_bible", SENTINEL) and only call
self._check_has_bible(novel.novel_id.value) if value is SENTINEL, then assign
dto.has_bible = value_or_fallback; do the same for "_has_outline" with
self._check_has_outline to avoid unnecessary immediate evaluation and preserve
batch-query optimization.
In `@interfaces/api/stats/services/stats_service.py`:
- Around line 87-89: The change sets total_words = total_characters in
get_global_stats(), which breaks semantics for English/mixed text; restore the
original word-count logic by computing total_words with the same
tokenizer/counting used in the fallback (call
self.repository.count_words(content) or call the internal tokenizer routine used
elsewhere) instead of using total_characters, or move that exact
tokenizer/counting rule into the DB layer so the SQL path returns the same
total_words value; ensure you update the logic in get_global_stats() where
total_words and total_characters are computed so both repository-backed and
SQL-backed paths produce identical word counts.
In `@interfaces/main.py`:
- Around line 181-182: The comment above _checkpoint_sqlite_wal_safe()
inaccurately claims PASSIVE mode but the function issues PRAGMA
wal_checkpoint(TRUNCATE); either update the comment to state that TRUNCATE mode
is used and may block/require exclusive access, or modify the function to use
PRAGMA wal_checkpoint(PASSIVE) if truly non-blocking behavior is desired; locate
the PRAGMA call in _checkpoint_sqlite_wal_safe() and make the corresponding
change to either the comment text or the PRAGMA mode to keep code and comment
consistent.
---
Nitpick comments:
In `@application/engine/services/beat_middleware.py`:
- Around line 112-113: Remove the redundant "from typing import Any" import (the
duplicate import that appears alongside the "# 避免循环导入" comment); keep the single
existing "Any" import already declared earlier and delete this later duplicate
import so there are no repeated imports of Any in beat_middleware.py.
In `@interfaces/api/v1/workbench/llm_control.py`:
- Around line 313-332: The prompt stats TTL cache (_prompt_stats_cache and
_prompt_stats_cache_ts) is not being cleared on prompt write operations; update
the invalidation logic so that _invalidate_plaza_cache() also resets
_prompt_stats_cache and _prompt_stats_cache_ts (or rename to
_invalidate_prompt_caches and clear both _plaza_cache/_plaza_cache_ts and
_prompt_stats_cache/_prompt_stats_cache_ts) so /prompts/stats returns fresh data
immediately after create/update/delete.
🪄 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: 1b6106c4-416e-4797-9486-3b6bb0d3e970
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
application/core/services/novel_service.pyapplication/engine/services/beat_middleware.pyapplication/engine/services/shared_state_repository.pyapplication/engine/services/state_publisher.pyinfrastructure/persistence/database/connection.pyinfrastructure/persistence/database/query_optimizations.pyinterfaces/api/middleware/logging_config.pyinterfaces/api/stats/services/stats_service.pyinterfaces/api/v1/workbench/llm_control.pyinterfaces/main.py
Comment on lines
+211
to
+213
| if cached is not None and _time.time() - cached["ts"] < self._NOVELS_LIST_CACHE_TTL: | ||
| return cached["data"] | ||
|
|
Contributor
There was a problem hiding this comment.
Do not return/store mutable cached DTO objects by reference.
Line 211-Line 213 and Line 239 currently share the same mutable list/DTO instances across requests. Any in-place mutation by one caller can pollute later responses.
Proposed fix
+import copy
...
- if cached is not None and _time.time() - cached["ts"] < self._NOVELS_LIST_CACHE_TTL:
- return cached["data"]
+ if cached is not None and _time.time() - cached["ts"] < self._NOVELS_LIST_CACHE_TTL:
+ return copy.deepcopy(cached["data"])
...
- self._novels_list_cache[cache_key] = {"data": dtos, "ts": _time.time()}
+ self._novels_list_cache[cache_key] = {
+ "data": copy.deepcopy(dtos),
+ "ts": _time.time(),
+ }Also applies to: 239-239
🤖 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/novel_service.py` around lines 211 - 213, Cached
DTOs/lists are being stored and returned by reference (cached["data"]) which
allows callers to mutate shared objects; change the cache writes and reads in
NovelService so you store and return defensive copies instead of the original
objects — e.g., when setting cached["data"] wrap the DTO/list in a deep copy or
reconstruct DTOs (use copy.deepcopy or DTO.from_dict/map over items) and when
returning cached["data"] return a copy as well (the same change needed for the
other cache access at the second occurrence, the code that reads/writes
cached["data"] around line 239); ensure timestamps (cached["ts"]) remain
unchanged and only the data payload is cloned.
Comment on lines
+235
to
+236
| dto.has_bible = getattr(novel, "_has_bible", self._check_has_bible(novel.novel_id.value)) | ||
| dto.has_outline = getattr(novel, "_has_outline", self._check_has_outline(novel.novel_id.value)) |
Contributor
There was a problem hiding this comment.
Avoid eager fallback evaluation in getattr here.
At Line 235 and Line 236, _check_has_bible(...) / _check_has_outline(...) are executed even when _has_bible / _has_outline exist, because getattr’s default is evaluated eagerly. This negates the batch-query optimization and can reintroduce N+1-style I/O.
Proposed fix
- dto.has_bible = getattr(novel, "_has_bible", self._check_has_bible(novel.novel_id.value))
- dto.has_outline = getattr(novel, "_has_outline", self._check_has_outline(novel.novel_id.value))
+ dto.has_bible = (
+ novel._has_bible
+ if hasattr(novel, "_has_bible")
+ else self._check_has_bible(novel.novel_id.value)
+ )
+ dto.has_outline = (
+ novel._has_outline
+ if hasattr(novel, "_has_outline")
+ else self._check_has_outline(novel.novel_id.value)
+ )📝 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.
Suggested change
| dto.has_bible = getattr(novel, "_has_bible", self._check_has_bible(novel.novel_id.value)) | |
| dto.has_outline = getattr(novel, "_has_outline", self._check_has_outline(novel.novel_id.value)) | |
| dto.has_bible = ( | |
| novel._has_bible | |
| if hasattr(novel, "_has_bible") | |
| else self._check_has_bible(novel.novel_id.value) | |
| ) | |
| dto.has_outline = ( | |
| novel._has_outline | |
| if hasattr(novel, "_has_outline") | |
| else self._check_has_outline(novel.novel_id.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 `@application/core/services/novel_service.py` around lines 235 - 236, The
getattr calls eagerly evaluate the default expressions causing
_check_has_bible/_check_has_outline to run even when
novel._has_bible/_has_outline exist; change the pattern to use a unique sentinel
(or hasattr/try-except) so you first retrieve value = getattr(novel,
"_has_bible", SENTINEL) and only call
self._check_has_bible(novel.novel_id.value) if value is SENTINEL, then assign
dto.has_bible = value_or_fallback; do the same for "_has_outline" with
self._check_has_outline to avoid unnecessary immediate evaluation and preserve
batch-query optimization.
Comment on lines
+87
to
+89
| # 估算总字数：中文为主时 ≈ 字符数；有英文时需加回英文单词 | ||
| # 作为近似，用 总字符数 作为 total_words（对中文写作已是准确值） | ||
| total_words = total_characters |
Contributor
There was a problem hiding this comment.
Don't replace total_words with character count.
The fallback path still uses self.repository.count_words(content), but the SQL path now returns total_words = total_characters. That changes the API’s semantics and will overcount badly for English or mixed-language chapters, so get_global_stats() returns different answers depending on repository type. Please preserve the existing word-count logic for total_words here as well, or move that exact tokenizer/counting rule into the DB layer instead of approximating it.
🧰 Tools
🪛 Ruff (0.15.13)
[warning] 87-87: Comment contains ambiguous ： (FULLWIDTH COLON). Did you mean : (COLON)?
(RUF003)
[warning] 87-87: Comment contains ambiguous ； (FULLWIDTH SEMICOLON). Did you mean ; (SEMICOLON)?
(RUF003)
[warning] 88-88: Comment contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
[warning] 88-88: Comment contains ambiguous （ (FULLWIDTH LEFT PARENTHESIS). Did you mean ( (LEFT PARENTHESIS)?
(RUF003)
[warning] 88-88: Comment contains ambiguous ） (FULLWIDTH RIGHT PARENTHESIS). Did you mean ) (RIGHT PARENTHESIS)?
(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/stats/services/stats_service.py` around lines 87 - 89, The
change sets total_words = total_characters in get_global_stats(), which breaks
semantics for English/mixed text; restore the original word-count logic by
computing total_words with the same tokenizer/counting used in the fallback
(call self.repository.count_words(content) or call the internal tokenizer
routine used elsewhere) instead of using total_characters, or move that exact
tokenizer/counting rule into the DB layer so the SQL path returns the same
total_words value; ensure you update the logic in get_global_stats() where
total_words and total_characters are computed so both repository-backed and
SQL-backed paths produce identical word counts.
Comment on lines
+181
to
+182
| # 清理上次运行时残留的 WAL（PASSIVE 模式不阻塞，安全执行） | ||
| _checkpoint_sqlite_wal_safe() |
Contributor
There was a problem hiding this comment.
Comment inaccurately describes checkpoint mode.
The comment states "PASSIVE 模式不阻塞，安全执行" (PASSIVE mode doesn't block, safe to execute), but the implementation in _checkpoint_sqlite_wal_safe() (line 229) actually uses PRAGMA wal_checkpoint(TRUNCATE), not PASSIVE. TRUNCATE mode requires exclusive access and can block if readers are present, whereas PASSIVE is truly non-blocking.
📝 Proposed fix
Update the comment to reflect the actual behavior:
- # 清理上次运行时残留的 WAL（PASSIVE 模式不阻塞，安全执行）
+ # 清理上次运行时残留的 WAL（TRUNCATE 模式在启动时安全，已停止所有运行中小说）
_checkpoint_sqlite_wal_safe()Or, if non-blocking behavior is truly required, change the implementation to use PASSIVE mode:
def _checkpoint_sqlite_wal_safe() -> None:
"""桌面端优雅退出时尽量将 WAL 落盘，降低异常断电时的损坏概率。"""
try:
import sqlite3
from application.paths import get_db_path
dbp = get_db_path()
conn = sqlite3.connect(dbp, timeout=2.0)
try:
conn.execute("PRAGMA journal_mode=WAL")
conn.execute("PRAGMA busy_timeout=2000")
- conn.execute("PRAGMA wal_checkpoint(TRUNCATE)")
+ conn.execute("PRAGMA wal_checkpoint(PASSIVE)")
finally:
conn.close()
except Exception as e:
logger.warning("WAL checkpoint 失败（可忽略）: %s", e)📝 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.
Suggested change
| # 清理上次运行时残留的 WAL（PASSIVE 模式不阻塞，安全执行） | |
| _checkpoint_sqlite_wal_safe() | |
| # 清理上次运行时残留的 WAL（TRUNCATE 模式在启动时安全，已停止所有运行中小说） | |
| _checkpoint_sqlite_wal_safe() |
🧰 Tools
🪛 Ruff (0.15.13)
[warning] 181-181: Comment contains ambiguous （ (FULLWIDTH LEFT PARENTHESIS). Did you mean ( (LEFT PARENTHESIS)?
(RUF003)
[warning] 181-181: Comment contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
[warning] 181-181: Comment contains ambiguous ） (FULLWIDTH RIGHT PARENTHESIS). Did you mean ) (RIGHT PARENTHESIS)?
(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/main.py` around lines 181 - 182, The comment above
_checkpoint_sqlite_wal_safe() inaccurately claims PASSIVE mode but the function
issues PRAGMA wal_checkpoint(TRUNCATE); either update the comment to state that
TRUNCATE mode is used and may block/require exclusive access, or modify the
function to use PRAGMA wal_checkpoint(PASSIVE) if truly non-blocking behavior is
desired; locate the PRAGMA call in _checkpoint_sqlite_wal_safe() and make the
corresponding change to either the comment text or the PRAGMA mode to keep code
and comment consistent.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
application/core/services/novel_service.py (2)
211-213:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDefensively copy cached DTO payloads to prevent shared mutable state leakage.
At Line 213 and Line 240, cache reads/writes pass the same mutable list/DTO objects by reference. Any caller-side mutation can pollute subsequent responses.
Proposed fix
+import copy ... cached = self._novels_list_cache.get(cache_key) if cached is not None and _time.time() - cached["ts"] < self._NOVELS_LIST_CACHE_TTL: - return cached["data"] + return copy.deepcopy(cached["data"]) ... - self._novels_list_cache[cache_key] = {"data": dtos, "ts": _time.time()} + self._novels_list_cache[cache_key] = { + "data": copy.deepcopy(dtos), + "ts": _time.time(), + }Also applies to: 240-240
🤖 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/novel_service.py` around lines 211 - 213, The cached DTO list is being returned and stored by reference (using _novels_list_cache with cache_key and variables cached/_NOVELS_LIST_CACHE_TTL), which can lead to shared mutable-state leaks; update the cache read and write paths in NovelService (novel_service.py) to use defensive copies (e.g., copy.deepcopy or constructing new lists/dicts) so that when returning cached["data"] you return a copy, and when setting the cache (the write at the other location around line 240) you store a copy of the payload instead of the original object.
236-237:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid eager fallback evaluation in
getattrdefaults.At Line 236 and Line 237,
_check_has_bible(...)/_check_has_outline(...)are executed even when_has_bible/_has_outlineexists, because default arguments are evaluated eagerly.Proposed fix
+ _sentinel = object() dtos = [] for novel in novels: dto = NovelDTO.from_domain(novel) # 批量查询已预计算 has_bible / has_outline - dto.has_bible = getattr(novel, "_has_bible", self._check_has_bible(novel.novel_id.value)) - dto.has_outline = getattr(novel, "_has_outline", self._check_has_outline(novel.novel_id.value)) + has_bible = getattr(novel, "_has_bible", _sentinel) + dto.has_bible = ( + has_bible + if has_bible is not _sentinel + else self._check_has_bible(novel.novel_id.value) + ) + has_outline = getattr(novel, "_has_outline", _sentinel) + dto.has_outline = ( + has_outline + if has_outline is not _sentinel + else self._check_has_outline(novel.novel_id.value) + ) dtos.append(dto)🤖 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/novel_service.py` around lines 236 - 237, The current getattr calls eagerly evaluate self._check_has_bible(...) and self._check_has_outline(...) even when novel already has _has_bible/_has_outline; change to lazy evaluation by first checking for the attribute (e.g., use hasattr(novel, "_has_bible") ? novel._has_bible : self._check_has_bible(novel.novel_id.value)) and similarly for _has_outline, or use a sentinel pattern (sentinel = object(); v = getattr(novel, "_has_bible", sentinel); if v is sentinel: v = self._check_has_bible(novel.novel_id.value)); assign those results to dto.has_bible and dto.has_outline instead of the current getattr lines.
🧹 Nitpick comments (1)
application/core/services/novel_service.py (1)
227-230: ⚡ Quick winDon’t silently swallow optimized-path failures.
Line 227 catches all exceptions and suppresses diagnostics. Keep fallback behavior, but log the failure (and ideally narrow exception types) so optimization regressions are observable.
Proposed fix
+import logging ... - except Exception: + except Exception as e: + logging.getLogger(__name__).warning( + "Optimized novel listing failed; falling back to repository path: %s", e + ) novels = self.novel_repository.list_all() for novel in novels: self._hydrate_chapters(novel)🤖 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/novel_service.py` around lines 227 - 230, The except block around the optimized-path should not swallow all exceptions; update the try/except to catch only expected error types (or re-raise unexpected ones) and log the failure before falling back to the existing behavior. Specifically, in the method that calls the optimized path (the block that currently calls novel_repository.list_all and _hydrate_chapters on fallback), replace the broad "except Exception:" with a narrower exception list if possible, and add a logging call (e.g., self.logger.exception(...) or module logger.exception(...)) that includes the caught exception and context, then proceed to call novel_repository.list_all() and _hydrate_chapters(novel) as the fallback. Ensure unexpected exceptions are not silently suppressed.
🤖 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/core/services/novel_service.py`:
- Around line 402-404: Remove the redundant full save that occurs before the
incremental patch to avoid overwriting untouched fields: delete the call to
novel_repository.save(novel) (the full-save) and instead call
novel_repository.patch(...) directly after updating the novel model, keeping the
existing self._invalidate_novels_cache() behavior; apply the same change where a
save is followed by a patch in the other location (the similar save/patch pair
around lines 420-421) so only the incremental patch is used to persist changes
and avoid clobbering autopilot_status and related fields.
---
Duplicate comments:
In `@application/core/services/novel_service.py`:
- Around line 211-213: The cached DTO list is being returned and stored by
reference (using _novels_list_cache with cache_key and variables
cached/_NOVELS_LIST_CACHE_TTL), which can lead to shared mutable-state leaks;
update the cache read and write paths in NovelService (novel_service.py) to use
defensive copies (e.g., copy.deepcopy or constructing new lists/dicts) so that
when returning cached["data"] you return a copy, and when setting the cache (the
write at the other location around line 240) you store a copy of the payload
instead of the original object.
- Around line 236-237: The current getattr calls eagerly evaluate
self._check_has_bible(...) and self._check_has_outline(...) even when novel
already has _has_bible/_has_outline; change to lazy evaluation by first checking
for the attribute (e.g., use hasattr(novel, "_has_bible") ? novel._has_bible :
self._check_has_bible(novel.novel_id.value)) and similarly for _has_outline, or
use a sentinel pattern (sentinel = object(); v = getattr(novel, "_has_bible",
sentinel); if v is sentinel: v = self._check_has_bible(novel.novel_id.value));
assign those results to dto.has_bible and dto.has_outline instead of the current
getattr lines.
---
Nitpick comments:
In `@application/core/services/novel_service.py`:
- Around line 227-230: The except block around the optimized-path should not
swallow all exceptions; update the try/except to catch only expected error types
(or re-raise unexpected ones) and log the failure before falling back to the
existing behavior. Specifically, in the method that calls the optimized path
(the block that currently calls novel_repository.list_all and _hydrate_chapters
on fallback), replace the broad "except Exception:" with a narrower exception
list if possible, and add a logging call (e.g., self.logger.exception(...) or
module logger.exception(...)) that includes the caught exception and context,
then proceed to call novel_repository.list_all() and _hydrate_chapters(novel) as
the fallback. Ensure unexpected exceptions are not silently suppressed.
🪄 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: 1fcbc6dc-8f50-4187-ac15-5d7bdcb7cbb9
📒 Files selected for processing (1)
application/core/services/novel_service.py
Comment on lines
+402
to
404
| self.novel_repository.save(novel) | ||
| self._invalidate_novels_cache() | ||
| # 增量 patch：避免全量 save 把 autopilot_status 等未改字段写回 stopped |
Contributor
There was a problem hiding this comment.
Remove redundant full save before patch to avoid unintended field overwrite.
Line 402 performs save(novel) before the incremental patch(...), which contradicts the Line 404 rationale and can reintroduce overwrite of non-updated fields (e.g., autopilot-related state).
Proposed fix
- self.novel_repository.save(novel)
- self._invalidate_novels_cache()
# 增量 patch：避免全量 save 把 autopilot_status 等未改字段写回 stopped
patch_fields: Dict[str, Any] = {}
...
if patch_fields:
self.novel_repository.patch(NovelId(novel_id), **patch_fields)
+ self._invalidate_novels_cache()Also applies to: 420-421
🧰 Tools
🪛 Ruff (0.15.13)
[warning] 404-404: Comment contains ambiguous ： (FULLWIDTH COLON). Did you mean : (COLON)?
(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 `@application/core/services/novel_service.py` around lines 402 - 404, Remove
the redundant full save that occurs before the incremental patch to avoid
overwriting untouched fields: delete the call to novel_repository.save(novel)
(the full-save) and instead call novel_repository.patch(...) directly after
updating the novel model, keeping the existing self._invalidate_novels_cache()
behavior; apply the same change where a save is followed by a patch in the other
location (the similar save/patch pair around lines 420-421) so only the
incremental patch is used to persist changes and avoid clobbering
autopilot_status and related fields.
1. 新增组件卸载标记，防止僵尸轮询累积 2. 针对stopped/completed/error状态延长轮询间隔 3. 为轮询请求添加超时和去重处理 4. 优化DAG视图的轮询策略和可见性处理 5. 解决前端因大量高速请求导致偶发性崩溃
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
frontend/src/components/autopilot/AutopilotDAGView.vue (2)
165-170:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard the post-await listener registration.
If the component unmounts during the awaited setup,
onUnmounted()runs first and this code still addsvisibilitychangeafterward, leaving a leaked document listener behind.Minimal fix
onMounted(async () => { await dagStore.hydrateDagForNovel(props.novelId) await runStore.fetchStatus(props.novelId) await fetchAutopilotStatus() + if (dagViewUnmounted) return document.addEventListener('visibilitychange', handleDAGVisibilityChange) })🤖 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 `@frontend/src/components/autopilot/AutopilotDAGView.vue` around lines 165 - 170, The onMounted callback adds document.addEventListener('visibilitychange', handleDAGVisibilityChange) after several awaits which can run after the component has unmounted; guard this by tracking mount state (e.g., a local let isMounted = true set in onMounted and set to false in onUnmounted) and only add the listener if isMounted is still true, or alternatively register the listener before awaits and remove it in onUnmounted; ensure handleDAGVisibilityChange, onMounted and onUnmounted are used so the listener is never added when the component has already been torn down.
231-268:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWire the timeout controller into the
/statusrequest.The controller created at line 239 never reaches line 245, so
abort()only flips local state. A slow/statuscall can keep running afterclearAutopilotPoll(), and its stale response can still overwrite a newerautopilotStatus.Suggested fix
async function fetchAutopilotStatus() { if (dagViewUnmounted) return if (autopilotFetchInFlight) return autopilotFetchInFlight = true if (autopilotFetchAbort) { autopilotFetchAbort.abort() } const ac = new AbortController() autopilotFetchAbort = ac const timeoutId = window.setTimeout(() => ac.abort(), AUTOPILOT_FETCH_TIMEOUT_MS) try { const { apiClient } = await import('`@/api/config`') - const result = await apiClient.get(`/autopilot/${props.novelId}/status`) as Record<string, unknown> + const result = await apiClient.get( + `/autopilot/${props.novelId}/status`, + { signal: ac.signal }, + ) as Record<string, unknown>🤖 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 `@frontend/src/components/autopilot/AutopilotDAGView.vue` around lines 231 - 268, The AbortController created as ac in fetchAutopilotStatus is never passed to the network request so abort() only flips local state; modify the call to apiClient.get(`/autopilot/${props.novelId}/status`) inside fetchAutopilotStatus to include the abort signal (ac.signal) so the request can actually be cancelled, and ensure any fetch/HTTP client path used by apiClient.get respects the signal; keep the existing timeoutId/clearTimeout, autopilotFetchAbort assignment, and existing try/catch/finally flow (including scheduleAutopilotPoll and resetting autopilotFetchInFlight) so aborted requests won't overwrite a newer autopilotStatus.frontend/src/components/workbench/WorkArea.vue (1)
1206-1220:⚠️ Potential issue | 🟠 Major | ⚡ Quick winProtect assisted-mode
/statusfrom stale cross-page responses.This poller still has no abort or request token. If
slugorworkModechanges while a request is in flight, the old response can still callapplyAutopilotStatusPayload()and incorrectly flip the new page into a stale read-only/status state.Suggested direction
+let assistedStatusFetchSeq = 0 +let assistedStatusFetchAbort: AbortController | null = null + async function pollAutopilotStatusWhileAssisted() { if (assistedAutopilot404) return + assistedStatusFetchSeq += 1 + const seq = assistedStatusFetchSeq + assistedStatusFetchAbort?.abort() + const ac = new AbortController() + assistedStatusFetchAbort = ac try { - const res = await fetch(resolveHttpUrl(`/api/v1/autopilot/${props.slug}/status`)) + const res = await fetch(resolveHttpUrl(`/api/v1/autopilot/${props.slug}/status`), { + signal: ac.signal, + }) + if (seq !== assistedStatusFetchSeq || assistedUnmounted || workMode.value !== 'assisted') { + return + } if (res.status === 404) { assistedAutopilot404 = true clearAssistedAutopilotPoll() assistAutopilotPollFailures = 0 return } if (res.ok) { assistAutopilotPollFailures = 0 const json = await res.json() + if (seq !== assistedStatusFetchSeq) return lastAssistedAutopilotStatus = String(json.autopilot_status ?? '') applyAutopilotStatusPayload(json as Record<string, unknown>)Also abort
assistedStatusFetchAbortinsideclearAssistedAutopilotPoll()so the old request is canceled whenslug, visibility, or mode changes.🤖 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 `@frontend/src/components/workbench/WorkArea.vue` around lines 1206 - 1220, The poller pollAutopilotStatusWhileAssisted lacks an AbortController and can apply stale responses to the new page; create/assign an AbortController (assistedStatusFetchAbort) for each fetch, pass its signal into fetch(resolveHttpUrl(...)), and ensure clearAssistedAutopilotPoll() calls assistedStatusFetchAbort.abort() and clears the variable; additionally, after awaiting the response but before calling applyAutopilotStatusPayload(), verify the request wasn't aborted and that props.slug/workMode still match the values when the request was started (or otherwise guard with a request token) to prevent applying stale payloads.
🤖 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 `@frontend/src/components/autopilot/NodeDetailPanel.vue`:
- Around line 270-275: The panel never surfaces AbortError timeouts because the
AbortError branch only logs to console; update the fetchWritingTelemetry
handling so that when an AbortError is detected (in the branch that currently
logs the timeout) you set writingPollError.value to a user-visible message
(e.g., '请求超时' or localized equivalent) and clear any loading flag so the UI
leaves the "加载中…" state; keep the existing seq check (writingFetchSeq) and other
error branch behavior intact and only augment the AbortError branch to set
writingPollError and stop any loading indicator.
---
Outside diff comments:
In `@frontend/src/components/autopilot/AutopilotDAGView.vue`:
- Around line 165-170: The onMounted callback adds
document.addEventListener('visibilitychange', handleDAGVisibilityChange) after
several awaits which can run after the component has unmounted; guard this by
tracking mount state (e.g., a local let isMounted = true set in onMounted and
set to false in onUnmounted) and only add the listener if isMounted is still
true, or alternatively register the listener before awaits and remove it in
onUnmounted; ensure handleDAGVisibilityChange, onMounted and onUnmounted are
used so the listener is never added when the component has already been torn
down.
- Around line 231-268: The AbortController created as ac in fetchAutopilotStatus
is never passed to the network request so abort() only flips local state; modify
the call to apiClient.get(`/autopilot/${props.novelId}/status`) inside
fetchAutopilotStatus to include the abort signal (ac.signal) so the request can
actually be cancelled, and ensure any fetch/HTTP client path used by
apiClient.get respects the signal; keep the existing timeoutId/clearTimeout,
autopilotFetchAbort assignment, and existing try/catch/finally flow (including
scheduleAutopilotPoll and resetting autopilotFetchInFlight) so aborted requests
won't overwrite a newer autopilotStatus.
In `@frontend/src/components/workbench/WorkArea.vue`:
- Around line 1206-1220: The poller pollAutopilotStatusWhileAssisted lacks an
AbortController and can apply stale responses to the new page; create/assign an
AbortController (assistedStatusFetchAbort) for each fetch, pass its signal into
fetch(resolveHttpUrl(...)), and ensure clearAssistedAutopilotPoll() calls
assistedStatusFetchAbort.abort() and clears the variable; additionally, after
awaiting the response but before calling applyAutopilotStatusPayload(), verify
the request wasn't aborted and that props.slug/workMode still match the values
when the request was started (or otherwise guard with a request token) to
prevent applying stale payloads.
🪄 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: 81398cd5-6b11-4737-86f2-2398daeb4e00
📒 Files selected for processing (4)
frontend/src/components/autopilot/AutopilotDAGView.vuefrontend/src/components/autopilot/AutopilotPanel.vuefrontend/src/components/autopilot/NodeDetailPanel.vuefrontend/src/components/workbench/WorkArea.vue
Comment on lines
+270
to
+275
| if (seq !== writingFetchSeq) return | ||
| if (e instanceof Error && e.name === 'AbortError') { | ||
| console.warn('[NodeDetailPanel] fetchWritingTelemetry 超时') | ||
| } else { | ||
| writingPollError.value = e instanceof Error ? e.message : '网络错误' | ||
| } |
Contributor
There was a problem hiding this comment.
Surface timed-out telemetry polls to the user.
AbortError only logs here, so repeated timeouts keep the panel stuck on “加载中…” with no visible failure state.
Minimal fix
} catch (e) {
if (seq !== writingFetchSeq) return
if (e instanceof Error && e.name === 'AbortError') {
+ writingPollError.value = '遥测拉取超时，请稍后重试'
console.warn('[NodeDetailPanel] fetchWritingTelemetry 超时')
} else {
writingPollError.value = e instanceof Error ? e.message : '网络错误'
}
} finally {🤖 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 `@frontend/src/components/autopilot/NodeDetailPanel.vue` around lines 270 -
275, The panel never surfaces AbortError timeouts because the AbortError branch
only logs to console; update the fetchWritingTelemetry handling so that when an
AbortError is detected (in the branch that currently logs the timeout) you set
writingPollError.value to a user-visible message (e.g., '请求超时' or localized
equivalent) and clear any loading flag so the UI leaves the "加载中…" state; keep
the existing seq check (writingFetchSeq) and other error branch behavior intact
and only augment the AbortError branch to set writingPollError and stop any
loading indicator.
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.
变更类型
perf性能优化变更说明
详细改动如下：
__future__ annotations导入适配现代类型提示架构影响
风险说明
写"无"
Summary by CodeRabbit
Performance Improvements
UI Improvements / Bug Fixes
Infrastructure