fix(engine): 修复截断检测误判 + 新增章节修复功能#161
Open
ws1065 wants to merge 6 commits into
Open
Conversation
bible_service.add_character() 原先每次调用都会全量加载 Bible、删除所有 子表数据后重新写入，多次调用会导致关系数据按 2^n 倍增。实测 bible_character_relationships 表膨胀至 406 万行（1.4GB），其中每条 关系被复制 131,072 次。 修复方案： - 新增 SqliteBibleRepository.add_character_incremental() 增量写入方法 - BibleService.add_character() 改为增量 INSERT，不再 DELETE + 全量重写 - 更新单元测试适配新逻辑 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
之前 _ensure_complete_ending() 通过正则检测内容是否以句号等符号结尾来判断 截断，导致大量误判（对话引号、逗号等正常结尾也会触发续写）。 现在从 LLM API 返回的 finish_reason / stop_reason / finishReason 中提取 真正的截断信号： - "length" / "max_tokens" / "MAX_TOKENS" → 确认截断，发起续写 - "stop" / "end_turn" / "STOP" 等 → 正常结束，跳过续写 - 为空时降级到原有文本匹配逻辑 改动涉及：GenerationResult 增加 stop_reason 字段，各 Provider（OpenAI / Anthropic / Gemini / Mock）提取 stop_reason，BaseProvider 增加 last_stream_stop_reason 用于流式场景。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
DynamicLLMService 每次调用 _resolve_provider() 获取 provider 实例， last_stream_stop_reason 设置在 provider 上，但 daemon 从 DynamicLLMService 上读取。新增 _last_provider 引用和 last_stream_stop_reason 属性，将读取委托给最后一次使用的 provider。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
中文 1 字 ≈ 1-2 token，之前 max_tokens = target_words × 1.1 导致模型 在 token 耗尽前就被截断。将倍率从 1.1 提高到 2.0，留足余量。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
不同模型 tokenizer 对中文的编码差异大（1 字 ≈ 1-2.5 token）， 用 target_words 估算 max_tokens 总是不准确。改为固定 8192， 让模型自然结束，依赖 stop_reason 判断是否真正截断。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
因数据库爆炸和截断检测误判导致大量章节内容过短，新增章节修复页面： - 新增 ChapterRepairService：扫描短章节、单章扩写、批量扩写 - 扩写时自动读取前后章节内容确保故事连贯 - SSE 流式输出扩写进度，支持批量顺序执行 - 前端新增 ChapterRepair.vue 页面，支持全选、一键审查续写 - 工作台 AI 工具菜单增加"章节修复"入口 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThis PR introduces a chapter repair feature that scans for short chapters and expands them using LLM streaming. It extends LLM stop-reason tracking across all providers, updates the autopilot beat generation to use stop reasons for completion decisions, adds a ChapterRepairService with scan/expand/batch operations, and provides a complete Vue workbench UI with streaming event handling. ChangesChapter Repair Feature Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
infrastructure/ai/provider_factory.py (1)
3-3:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd missing
Optionalimport.
Optionalis used in type annotations (lines 23, 26, 71, 73) but not imported fromtyping. Whilefrom __future__ import annotationsprevents import-time errors, the missing import should be added for code completeness and to avoid issues if annotations are introspected at runtime.Proposed fix
-from typing import AsyncIterator +from typing import AsyncIterator, Optional🤖 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/provider_factory.py` at line 3, The module infrastructure/ai/provider_factory.py uses Optional in type annotations (e.g., in the signatures around the provider creation functions and variables referenced in the file) but did not import Optional; add Optional to the typing imports (alongside AsyncIterator) so annotations like Optional[Something] resolve correctly during introspection and static checks; update the import line that currently reads "from typing import AsyncIterator" to include Optional.
🧹 Nitpick comments (5)
tests/unit/application/services/test_bible_service.py (1)
63-64: ⚡ Quick win建议校验
add_character_incremental的调用参数，而不只校验调用次数当前断言只能证明“被调用过”，不能证明“调用内容正确”。建议补
assert_called_once_with(...)覆盖novel_id/character_id/name/description/relationships。🤖 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_bible_service.py` around lines 63 - 64, Replace the call-count-only assertion on mock_repository.add_character_incremental with a parameter check using assert_called_once_with(...) so the test verifies the actual payload; specifically assert that mock_repository.add_character_incremental was called once with the expected novel_id, character_id, name, description and relationships values (use the test's expected variables or constructed dict/args present earlier in the test) to validate the call contents rather than only its occurrence.frontend/src/views/ChapterRepair.vue (1)
488-492: 💤 Low valueSeverity thresholds in
refreshChapterInTableduplicate backend constants.Lines 490–491 hardcode
1000and2500for severity, mirroring_CRITICAL_THRESHOLDand_WARNING_THRESHOLDinchapter_repair_service.py. If the backend values change, the table will show stale severity labels until a re-scan is triggered.The values are stable and the batch path always calls
scan()after completion, so this is only relevant when using single-chapter expand. Consider exporting the thresholds from the API scan response (e.g., asscan_thresholdsinChapterRepairScanResult) so the frontend can use authoritative values.🤖 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/views/ChapterRepair.vue` around lines 488 - 492, The frontend hardcodes severity thresholds inside refreshChapterInTable (using 1000 and 2500), duplicating backend constants (_CRITICAL_THRESHOLD and _WARNING_THRESHOLD in chapter_repair_service.py); change the API to include authoritative thresholds in the scan response (add scan_thresholds to ChapterRepairScanResult) and update refreshChapterInTable to read those values from the scan response instead of hardcoded literals so single-chapter expand uses the backend thresholds; ensure the new scan_thresholds shape (critical, warning) is documented and fallback to the old literals only if the response lacks the fields.infrastructure/ai/providers/gemini_provider.py (1)
161-208: ⚡ Quick winExtract shared SSE data-decoding into a private helper to remove duplication.
_parse_sse_eventand_extract_finish_reason_from_sseshare identical SSE data-extraction and JSON-decode logic (~10 lines each). Any future fix to the parsing (e.g. multi-linedata:joining,[DONE]handling) would need to be applied twice.♻️ Proposed refactor
+ def _decode_sse_payload(self, event_text: str) -> Any | None: + """Parse SSE event text, return the decoded JSON payload or None.""" + data_lines = [line[5:].strip() for line in event_text.splitlines() if line.startswith('data:')] + if not data_lines: + return None + raw = ''.join(data_lines).strip() + if not raw or raw == '[DONE]': + return None + try: + return json.loads(raw) + except json.JSONDecodeError: + logger.debug('Gemini SSE parse skip: %s', raw[:120]) + return None + def _parse_sse_event(self, event_text: str) -> str: - data_lines: list[str] = [] - for line in event_text.splitlines(): - if line.startswith('data:'): - data_lines.append(line[5:].strip()) - if not data_lines: - return '' - raw_payload = ''.join(data_lines).strip() - if not raw_payload or raw_payload == '[DONE]': - return '' - try: - payload = json.loads(raw_payload) - except json.JSONDecodeError: - logger.debug('Gemini SSE parse skip: %s', raw_payload[:120]) - return '' + payload = self._decode_sse_payload(event_text) + if payload is None: + return '' if isinstance(payload, list): return ''.join(self._extract_text(item) for item in payload if isinstance(item, dict)) if isinstance(payload, dict): return self._extract_text(payload) return '' def _extract_finish_reason_from_sse(self, event_text: str) -> str: - data_lines: list[str] = [] - for line in event_text.splitlines(): - if line.startswith('data:'): - data_lines.append(line[5:].strip()) - if not data_lines: - return '' - raw_payload = ''.join(data_lines).strip() - if not raw_payload or raw_payload == '[DONE]': - return '' - try: - payload = json.loads(raw_payload) - except json.JSONDecodeError: - return '' + payload = self._decode_sse_payload(event_text) + if payload is None: + return '' items = payload if isinstance(payload, list) else [payload] for item in items: if isinstance(item, dict): for candidate in item.get('candidates') or []: fr = candidate.get('finishReason') if fr: return fr return ''🤖 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/providers/gemini_provider.py` around lines 161 - 208, Create a private helper (e.g., _decode_sse_payload or similar) that encapsulates the duplicated SSE data extraction and JSON decoding used by _parse_sse_event and _extract_finish_reason_from_sse: it should collect lines starting with 'data:', join them, ignore empty/'[DONE]' payloads, attempt json.loads and return either the parsed object (dict/list) or None (or raise/log on JSONDecodeError). Replace the repeated blocks in _parse_sse_event and _extract_finish_reason_from_sse to call this helper and then keep only the payload-specific processing (_extract_text traversal in _parse_sse_event and finishReason extraction in _extract_finish_reason_from_sse); update return types/signatures accordingly and preserve existing logging/behavior for decode failures.frontend/src/api/chapterRepair.ts (2)
125-125: 💤 Low valueRedundant
raw === nullguard —!rawalready covers it.
if (!raw || typeof raw !== 'object' || raw === null)—!rawis truthy whenrawisnull, making the trailingraw === nullunreachable.🛠️ Suggested cleanup
- if (!raw || typeof raw !== 'object' || raw === null) continue + if (!raw || typeof raw !== 'object') continue🤖 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/api/chapterRepair.ts` at line 125, The condition in the loop redundantly checks raw === null because the earlier !raw already catches null; update the guard in frontend/src/api/chapterRepair.ts by removing the trailing "|| raw === null" from the if statement that currently reads with raw, so the check becomes if (!raw || typeof raw !== 'object') to cleanly skip non-objects; locate the conditional that references the variable raw in the chapter repair logic and simplify it accordingly.
84-158: ⚖️ Poor tradeoffDuplicated SSE-parsing loop between
consumeBatchExpandStreamand_consumeSse.Both functions implement the same
reader.read()→TextDecoder→buf.indexOf('\n\n')→parseSseDataLinepattern. The only difference is the event dispatch logic. Consider extracting the loop skeleton into_consumeSse(or a lower-level_readSseBlocksgenerator/async iterator) and passing a per-event handler callback, eliminating ~40 lines of duplication.Also applies to: 162-214
🤖 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/api/chapterRepair.ts` around lines 84 - 158, The SSE parsing loop in consumeBatchExpandStream duplicates the same reader/read/TextDecoder/buffer/\n\n split logic found in _consumeSse; extract that skeleton into a single reusable helper (e.g. _consumeSse or a lower-level async generator _readSseBlocks) that takes the Response.body (or ReadableStreamDefaultReader) and a per-event callback to receive each parsed SSE object (or raw block/line), preserve AbortError handling and top-level error propagation, then refactor consumeBatchExpandStream to call the new helper and implement only the event-dispatch logic (phase/chunk/chapter_start/chapter_done/session_done/error) in its callback; ensure the new helper returns/throws on stream end or fatal errors so existing handlers.onDone and handlers.onError behavior remains unchanged.
🤖 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/audit/services/chapter_repair_service.py`:
- Around line 260-274: The asyncio.create_task call in _schedule_aftermath may
be GC-cancelled because no strong reference is kept; fix by maintaining a
background-task set on the service (e.g., self._background_tasks or a
class-level set) so the created Task from asyncio.create_task(_run()) is added
to that set immediately, attach a done callback to remove the task from the set
and log/handle exceptions (inspect task.exception()) in the callback, and
replace the current plain create_task call in _schedule_aftermath so the task is
retained until completion; reference symbols: _schedule_aftermath,
self._aftermath.run_after_chapter_saved, and the new self._background_tasks (or
ChapterRepairService._background_tasks) for locating the change.
In `@application/engine/services/autopilot_daemon.py`:
- Line 1385: The warning call uses an unnecessary f-string prefix on a literal
with no placeholders; remove the leading "f" on the logger.warning call (the one
invoking logger.warning with the message "[截断检测] stop_reason 未知且内容未以结束符结尾，尝试续写")
so it becomes a normal string literal to resolve the Ruff F541 error.
In `@application/world/services/bible_service.py`:
- Around line 160-162: After reloading the bible with
self.bible_repository.get_by_novel_id(NovelId(novel_id)) add an explicit null
check and raise EntityNotFoundError if the repository returned None to avoid
passing None into BibleDTO.from_domain; i.e., in the method that calls
get_by_novel_id (the current function in bible_service.py) validate that bible
is not None and raise EntityNotFoundError("Bible not found" or similar) before
calling BibleDTO.from_domain(bible).
In `@frontend/src/api/chapterRepair.ts`:
- Around line 115-118: The SSE consumer loops in consumeBatchExpandStream and
_consumeSse read from reader and break on done without flushing the remaining
buf, so final events lacking trailing "\n\n" are dropped; after the reader loop
in both functions (where const { done, value } = await reader.read() is handled)
ensure you call the same buffer-flush logic you use for full chunks (decode
remaining bytes with dec.decode(..., { stream: false }) if needed, append to
buf, then run the existing event-parsing/emit routine to process any leftover
event(s)) so the final partial event (e.g., session_done/done) is emitted before
returning.
- Around line 111-157: The stream reader obtained via res.body.getReader() is
never cleaned up on early exits; wrap the read loop in a try/finally and in
finally call reader.cancel() (or reader.releaseLock()) to release the lock and
allow GC/reuse of the response body; do this in the function containing the
shown loop (references: reader, res.body.getReader(), parseSseDataLine,
handlers.onEvent/onChunk/onPhase/onDone/onError) and apply the same
finally-based cleanup to the internal _consumeSse implementation as well.
In `@infrastructure/ai/provider_factory.py`:
- Line 73: DynamicLLMService currently stores `_last_provider` on the shared
singleton which races across concurrent generate/stream calls; instead, make the
provider selection per-request by removing mutable `_last_provider` from the
instance and threading the chosen provider through the call stack (e.g., return
or pass the provider from the provider selection helper and use that local
variable inside `generate`/`stream`/`stop_stream`), and update uses of
`last_stream_stop_reason` to read the provider-specific stop reason from the
per-request/local provider object or from a request-scoped context rather than
the singleton field; update all references in DynamicLLMService (selection
helper, `generate`, `stream`, `stop_stream`) to use the local provider variable.
In `@infrastructure/ai/providers/openai_provider.py`:
- Around line 130-135: The Responses API returns resp.status == "incomplete"
when max_output_tokens is hit, but that value isn't normalized so downstream
_ensure_complete_ending (in autopilot_daemon.py) never treats it as a
truncation; update the provider to normalize that status: in
_generate_via_responses (and in the streaming path where event_type ==
"response.completed") set self.last_stream_stop_reason to a normalized
token-truncation value (e.g., "max_tokens" or "length") whenever getattr(resp,
"status", "") == "incomplete" (instead of leaving "incomplete" or an unmapped
string); ensure you reference and update last_stream_stop_reason assignment code
that currently reads resp = getattr(chunk, "response", None) /
self.last_stream_stop_reason = getattr(resp, "status", "") or "" so downstream
checks in _ensure_complete_ending will trigger continuation.
In `@infrastructure/persistence/database/sqlite_bible_repository.py`:
- Around line 336-344: This INSERT uses "INSERT OR REPLACE" for the
bible_characters table (parameters: character_id, novel_id, name, description,
mental_state, verbal_tic, idle_behavior, now) but the operation is for
"create/add new character" semantics; change the SQL to a strict "INSERT" so
conflicts fail instead of silently replacing existing rows, and update the
surrounding code in the function that performs this query (the method that binds
character_id/novel_id/etc.) to let the IntegrityError propagate or convert it
into the appropriate business exception (e.g., duplicate-character error) to be
handled by the caller rather than overwriting an existing record.
In `@interfaces/api/v1/audit/chapter_repair_routes.py`:
- Around line 25-27: The BatchExpandRequest model's chapter_numbers field lacks
an upper bound allowing huge lists; update the Field declaration for
chapter_numbers in class BatchExpandRequest to include a max_length (e.g.,
max_length=50 or a project-appropriate limit) so the schema enforces a sensible
upper bound and prevents large batch abuse; adjust any related validation tests
or docs to reflect the new limit.
- Around line 41-122: All three endpoints (scan_short_chapters, expand_chapter,
batch_expand_chapters) lack auth dependencies; add the application's auth
dependency (e.g. Depends(get_current_user) or whichever auth dependency is used
elsewhere) to each endpoint signature (or attach it as a router-level
dependencies=[...] on router) so FastAPI enforces authentication/authorization
before executing the handlers and before any SSE/streaming work runs; ensure the
chosen dependency returns the user/claims object (or performs authorization) and
keep it in the function params so expand_chapter and batch_expand_chapters still
stream correctly while being protected.
---
Outside diff comments:
In `@infrastructure/ai/provider_factory.py`:
- Line 3: The module infrastructure/ai/provider_factory.py uses Optional in type
annotations (e.g., in the signatures around the provider creation functions and
variables referenced in the file) but did not import Optional; add Optional to
the typing imports (alongside AsyncIterator) so annotations like
Optional[Something] resolve correctly during introspection and static checks;
update the import line that currently reads "from typing import AsyncIterator"
to include Optional.
---
Nitpick comments:
In `@frontend/src/api/chapterRepair.ts`:
- Line 125: The condition in the loop redundantly checks raw === null because
the earlier !raw already catches null; update the guard in
frontend/src/api/chapterRepair.ts by removing the trailing "|| raw === null"
from the if statement that currently reads with raw, so the check becomes if
(!raw || typeof raw !== 'object') to cleanly skip non-objects; locate the
conditional that references the variable raw in the chapter repair logic and
simplify it accordingly.
- Around line 84-158: The SSE parsing loop in consumeBatchExpandStream
duplicates the same reader/read/TextDecoder/buffer/\n\n split logic found in
_consumeSse; extract that skeleton into a single reusable helper (e.g.
_consumeSse or a lower-level async generator _readSseBlocks) that takes the
Response.body (or ReadableStreamDefaultReader) and a per-event callback to
receive each parsed SSE object (or raw block/line), preserve AbortError handling
and top-level error propagation, then refactor consumeBatchExpandStream to call
the new helper and implement only the event-dispatch logic
(phase/chunk/chapter_start/chapter_done/session_done/error) in its callback;
ensure the new helper returns/throws on stream end or fatal errors so existing
handlers.onDone and handlers.onError behavior remains unchanged.
In `@frontend/src/views/ChapterRepair.vue`:
- Around line 488-492: The frontend hardcodes severity thresholds inside
refreshChapterInTable (using 1000 and 2500), duplicating backend constants
(_CRITICAL_THRESHOLD and _WARNING_THRESHOLD in chapter_repair_service.py);
change the API to include authoritative thresholds in the scan response (add
scan_thresholds to ChapterRepairScanResult) and update refreshChapterInTable to
read those values from the scan response instead of hardcoded literals so
single-chapter expand uses the backend thresholds; ensure the new
scan_thresholds shape (critical, warning) is documented and fallback to the old
literals only if the response lacks the fields.
In `@infrastructure/ai/providers/gemini_provider.py`:
- Around line 161-208: Create a private helper (e.g., _decode_sse_payload or
similar) that encapsulates the duplicated SSE data extraction and JSON decoding
used by _parse_sse_event and _extract_finish_reason_from_sse: it should collect
lines starting with 'data:', join them, ignore empty/'[DONE]' payloads, attempt
json.loads and return either the parsed object (dict/list) or None (or raise/log
on JSONDecodeError). Replace the repeated blocks in _parse_sse_event and
_extract_finish_reason_from_sse to call this helper and then keep only the
payload-specific processing (_extract_text traversal in _parse_sse_event and
finishReason extraction in _extract_finish_reason_from_sse); update return
types/signatures accordingly and preserve existing logging/behavior for decode
failures.
In `@tests/unit/application/services/test_bible_service.py`:
- Around line 63-64: Replace the call-count-only assertion on
mock_repository.add_character_incremental with a parameter check using
assert_called_once_with(...) so the test verifies the actual payload;
specifically assert that mock_repository.add_character_incremental was called
once with the expected novel_id, character_id, name, description and
relationships values (use the test's expected variables or constructed dict/args
present earlier in the test) to validate the call contents rather than only its
occurrence.
🪄 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: f5f26ece-4cdb-4725-865c-e374366aa0e2
📒 Files selected for processing (20)
application/audit/dtos/chapter_repair_dto.pyapplication/audit/services/chapter_repair_service.pyapplication/engine/services/autopilot_daemon.pyapplication/world/services/bible_service.pydomain/ai/services/llm_service.pyfrontend/src/api/chapterRepair.tsfrontend/src/components/stats/StatsTopBar.vuefrontend/src/router/index.tsfrontend/src/views/ChapterRepair.vueinfrastructure/ai/provider_factory.pyinfrastructure/ai/providers/anthropic_provider.pyinfrastructure/ai/providers/base.pyinfrastructure/ai/providers/gemini_provider.pyinfrastructure/ai/providers/mock_provider.pyinfrastructure/ai/providers/openai_provider.pyinfrastructure/persistence/database/sqlite_bible_repository.pyinterfaces/api/dependencies.pyinterfaces/api/v1/audit/chapter_repair_routes.pyinterfaces/main.pytests/unit/application/services/test_bible_service.py
Comment on lines
+260
to
+274
| def _schedule_aftermath(self, novel_id: str, chapter_number: int, content: str) -> None: | ||
| """异步触发章后管线""" | ||
| if not self._aftermath or not content.strip(): | ||
| return | ||
|
|
||
| async def _run() -> None: | ||
| try: | ||
| await self._aftermath.run_after_chapter_saved(novel_id, chapter_number, content) | ||
| except Exception as e: | ||
| logger.warning(f"章节修复后管线失败 novel={novel_id} ch={chapter_number}: {e}") | ||
|
|
||
| try: | ||
| asyncio.create_task(_run()) | ||
| except Exception as e: | ||
| logger.warning(f"章节修复后管线未调度: {e}") |
Contributor
There was a problem hiding this comment.
asyncio.create_task result is discarded — aftermath task may be GC-cancelled before completion.
Ruff RUF006: if nothing holds a strong reference to the task object, CPython's cyclic GC may collect and cancel it before _aftermath.run_after_chapter_saved finishes. The failure is silent (no exception propagates to the caller).
🐛 Proposed fix — use a class-level background task set
Add a set to the class to hold strong references until completion:
class ChapterRepairService:
"""章节修复服务"""
+ _background_tasks: set[asyncio.Task] = set()
def _schedule_aftermath(self, novel_id: str, chapter_number: int, content: str) -> None:
"""异步触发章后管线"""
if not self._aftermath or not content.strip():
return
async def _run() -> None:
try:
await self._aftermath.run_after_chapter_saved(novel_id, chapter_number, content)
except Exception as e:
logger.warning(f"章节修复后管线失败 novel={novel_id} ch={chapter_number}: {e}")
try:
- asyncio.create_task(_run())
+ task = asyncio.create_task(_run())
+ ChapterRepairService._background_tasks.add(task)
+ task.add_done_callback(ChapterRepairService._background_tasks.discard)
except Exception as e:
logger.warning(f"章节修复后管线未调度: {e}")🧰 Tools
🪛 Ruff (0.15.12)
[warning] 268-268: Do not catch blind exception: Exception
(BLE001)
[warning] 272-272: Store a reference to the return value of asyncio.create_task
(RUF006)
[warning] 273-273: 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 `@application/audit/services/chapter_repair_service.py` around lines 260 - 274,
The asyncio.create_task call in _schedule_aftermath may be GC-cancelled because
no strong reference is kept; fix by maintaining a background-task set on the
service (e.g., self._background_tasks or a class-level set) so the created Task
from asyncio.create_task(_run()) is added to that set immediately, attach a done
callback to remove the task from the set and log/handle exceptions (inspect
task.exception()) in the callback, and replace the current plain create_task
call in _schedule_aftermath so the task is retained until completion; reference
symbols: _schedule_aftermath, self._aftermath.run_after_chapter_saved, and the
new self._background_tasks (or ChapterRepairService._background_tasks) for
locating the change.
| if re.search(ending_pattern, stripped): | ||
| return content | ||
| # 文本也不匹配结束符，可能是截断 | ||
| logger.warning(f"[截断检测] stop_reason 未知且内容未以结束符结尾，尝试续写") |
Contributor
There was a problem hiding this comment.
Remove extraneous f prefix from f-string without placeholders (Ruff F541 error).
- logger.warning(f"[截断检测] stop_reason 未知且内容未以结束符结尾，尝试续写")
+ logger.warning("[截断检测] stop_reason 未知且内容未以结束符结尾，尝试续写")📝 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
| logger.warning(f"[截断检测] stop_reason 未知且内容未以结束符结尾，尝试续写") | |
| logger.warning("[截断检测] stop_reason 未知且内容未以结束符结尾，尝试续写") |
🧰 Tools
🪛 Ruff (0.15.12)
[error] 1385-1385: f-string without any placeholders
Remove extraneous f prefix
(F541)
[warning] 1385-1385: String contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF001)
🤖 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/autopilot_daemon.py` at line 1385, The warning
call uses an unnecessary f-string prefix on a literal with no placeholders;
remove the leading "f" on the logger.warning call (the one invoking
logger.warning with the message "[截断检测] stop_reason 未知且内容未以结束符结尾，尝试续写") so it
becomes a normal string literal to resolve the Ruff F541 error.
Comment on lines
+160
to
162
| # 重新加载最新状态返回 | ||
| bible = self.bible_repository.get_by_novel_id(NovelId(novel_id)) | ||
| return BibleDTO.from_domain(bible) |
Contributor
There was a problem hiding this comment.
补上重载结果的空值保护，避免返回阶段抛出非预期异常
Line 161 重新加载后未判空，若并发删除或底层异常返回 None，Line 162 会触发 DTO 转换异常。建议显式抛 EntityNotFoundError。
建议修改
# 重新加载最新状态返回
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, After
reloading the bible with
self.bible_repository.get_by_novel_id(NovelId(novel_id)) add an explicit null
check and raise EntityNotFoundError if the repository returned None to avoid
passing None into BibleDTO.from_domain; i.e., in the method that calls
get_by_novel_id (the current function in bible_service.py) validate that bible
is not None and raise EntityNotFoundError("Bible not found" or similar) before
calling BibleDTO.from_domain(bible).
Comment on lines
+111
to
+157
| const reader = res.body.getReader() | ||
| const dec = new TextDecoder() | ||
| let buf = '' | ||
| try { | ||
| while (true) { | ||
| const { done, value } = await reader.read() | ||
| if (done) break | ||
| buf += dec.decode(value, { stream: true }) | ||
| let sep: number | ||
| while ((sep = buf.indexOf('\n\n')) >= 0) { | ||
| const block = buf.slice(0, sep) | ||
| buf = buf.slice(sep + 2) | ||
| for (const line of block.split('\n')) { | ||
| const raw = parseSseDataLine(line) | ||
| if (!raw || typeof raw !== 'object' || raw === null) continue | ||
| const o = raw as Record<string, unknown> | ||
| const typ = o.type as string | ||
| const ev = o as unknown as ChapterRepairStreamEvent | ||
| handlers.onEvent?.(ev) | ||
|
|
||
| if (typ === 'phase') { | ||
| handlers.onPhase?.(String(o.phase ?? '')) | ||
| } else if (typ === 'chunk') { | ||
| handlers.onChunk?.(String(o.text ?? ''), o.chapter_number as number | undefined) | ||
| } else if (typ === 'chapter_start') { | ||
| handlers.onChapterStart?.( | ||
| Number(o.chapter_number), Number(o.index), Number(o.total) | ||
| ) | ||
| } else if (typ === 'chapter_done') { | ||
| handlers.onChapterDone?.( | ||
| Number(o.chapter_number), Number(o.index), Number(o.total) | ||
| ) | ||
| } else if (typ === 'session_done') { | ||
| handlers.onDone?.() | ||
| return | ||
| } else if (typ === 'error') { | ||
| handlers.onError?.(String(o.message ?? '扩写失败')) | ||
| return | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } catch (e: unknown) { | ||
| if (e instanceof Error && e.name === 'AbortError') return | ||
| const msg = e instanceof Error ? e.message : '流式连接失败' | ||
| handlers.onError?.(msg) | ||
| } |
Contributor
There was a problem hiding this comment.
ReadableStreamDefaultReader is never cancelled on error paths.
reader.cancel() (or reader.releaseLock()) is never called in a finally block. On any early-exit path (error, session_done, done) the underlying stream stays locked, which prevents GC of the stream and can cause issues if the caller tries to re-use the response body.
♻️ Proposed fix: add a `finally` block
const reader = res.body.getReader()
const dec = new TextDecoder()
let buf = ''
try {
while (true) { ... }
} catch (e: unknown) {
if (e instanceof Error && e.name === 'AbortError') return
const msg = e instanceof Error ? e.message : '流式连接失败'
handlers.onError?.(msg)
+ } finally {
+ reader.cancel().catch(() => {})
}Apply the same fix to _consumeSse.
Also applies to: 172-213
🤖 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/api/chapterRepair.ts` around lines 111 - 157, The stream reader
obtained via res.body.getReader() is never cleaned up on early exits; wrap the
read loop in a try/finally and in finally call reader.cancel() (or
reader.releaseLock()) to release the lock and allow GC/reuse of the response
body; do this in the function containing the shown loop (references: reader,
res.body.getReader(), parseSseDataLine,
handlers.onEvent/onChunk/onPhase/onDone/onError) and apply the same
finally-based cleanup to the internal _consumeSse implementation as well.
Comment on lines
+115
to
+118
| while (true) { | ||
| const { done, value } = await reader.read() | ||
| if (done) break | ||
| buf += dec.decode(value, { stream: true }) |
Contributor
There was a problem hiding this comment.
Final SSE event dropped if stream ends without a trailing \n\n.
Both consumeBatchExpandStream (line 117) and _consumeSse (line 178) break on done before flushing the remaining buffer. If the server sends the last event without a closing \n\n, that event is silently lost — most critically the session_done/done event, which would leave the UI hanging.
🐛 Proposed fix: flush buffer after the reader loop
while (true) {
const { done, value } = await reader.read()
- if (done) break
+ if (done) {
+ // flush any trailing data not terminated by \n\n
+ if (buf.trim()) buf += '\n\n'
+ else break
+ }
buf += dec.decode(value, { stream: true })
...
}Apply the same pattern inside _consumeSse.
Also applies to: 176-179
🤖 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/api/chapterRepair.ts` around lines 115 - 118, The SSE consumer
loops in consumeBatchExpandStream and _consumeSse read from reader and break on
done without flushing the remaining buf, so final events lacking trailing "\n\n"
are dropped; after the reader loop in both functions (where const { done, value
} = await reader.read() is handled) ensure you call the same buffer-flush logic
you use for full chunks (decode remaining bytes with dec.decode(..., { stream:
false }) if needed, append to buf, then run the existing event-parsing/emit
routine to process any leftover event(s)) so the final partial event (e.g.,
session_done/done) is emitted before returning.
|
|
||
| def __init__(self, factory: Optional[LLMProviderFactory] = None): | ||
| self.factory = factory or LLMProviderFactory() | ||
| self._last_provider: Optional[LLMService] = None |
Contributor
There was a problem hiding this comment.
Shared _last_provider on singleton can race across concurrent requests.
DynamicLLMService is used as a long-lived shared instance; mutating _last_provider per call can be overwritten by other concurrent generate/stream calls, making last_stream_stop_reason read from the wrong request/provider.
💡 Directional fix (per-request/task isolation)
+from contextvars import ContextVar
+
class DynamicLLMService(LLMService):
def __init__(self, factory: Optional[LLMProviderFactory] = None):
self.factory = factory or LLMProviderFactory()
- self._last_provider: Optional[LLMService] = None
+ self._last_provider_ctx: ContextVar[Optional[LLMService]] = ContextVar(
+ "last_provider", default=None
+ )
@@
def last_stream_stop_reason(self) -> str:
- if self._last_provider and hasattr(self._last_provider, "last_stream_stop_reason"):
- return self._last_provider.last_stream_stop_reason # type: ignore[union-attr]
+ provider = self._last_provider_ctx.get()
+ if provider and hasattr(provider, "last_stream_stop_reason"):
+ return provider.last_stream_stop_reason # type: ignore[union-attr]
return ""
@@
async def generate(self, prompt: Prompt, config: GenerationConfig) -> GenerationResult:
provider = self._resolve_provider()
- self._last_provider = provider
+ self._last_provider_ctx.set(provider)
@@
async def stream_generate(self, prompt: Prompt, config: GenerationConfig) -> AsyncIterator[str]:
provider = self._resolve_provider()
- self._last_provider = provider
+ self._last_provider_ctx.set(provider)Also applies to: 79-82, 110-110, 116-116
🧰 Tools
🪛 Ruff (0.15.12)
[error] 73-73: Undefined name Optional
(F821)
🤖 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/provider_factory.py` at line 73, DynamicLLMService
currently stores `_last_provider` on the shared singleton which races across
concurrent generate/stream calls; instead, make the provider selection
per-request by removing mutable `_last_provider` from the instance and threading
the chosen provider through the call stack (e.g., return or pass the provider
from the provider selection helper and use that local variable inside
`generate`/`stream`/`stop_stream`), and update uses of `last_stream_stop_reason`
to read the provider-specific stop reason from the per-request/local provider
object or from a request-scoped context rather than the singleton field; update
all references in DynamicLLMService (selection helper, `generate`, `stream`,
`stop_stream`) to use the local provider variable.
Comment on lines
+130
to
+135
| # Responses API: response.completed 事件携带 status | ||
| event_type = getattr(chunk, "type", "") | ||
| if event_type == "response.completed": | ||
| resp = getattr(chunk, "response", None) | ||
| if resp: | ||
| self.last_stream_stop_reason = getattr(resp, "status", "") or "" |
Contributor
There was a problem hiding this comment.
OpenAI Responses API truncation signal "incomplete" is not recognized — truncation will be silently missed.
resp.status is "incomplete" when max_output_tokens is reached, but _ensure_complete_ending only checks stop_reason in ("length", "max_tokens", "MAX_TOKENS"). When stop_reason == "incomplete", the condition if stop_reason: on line 1377 of autopilot_daemon.py is truthy and returns the content unchanged — no continuation is triggered. This means the entire stop_reason fix is a no-op for users on the Responses API path.
🐛 Proposed fix — normalize at the provider level
- if event_type == "response.completed":
- resp = getattr(chunk, "response", None)
- if resp:
- self.last_stream_stop_reason = getattr(resp, "status", "") or ""
+ if event_type == "response.completed":
+ resp = getattr(chunk, "response", None)
+ if resp:
+ status = getattr(resp, "status", "") or ""
+ # Normalize Responses API status to the shared vocabulary:
+ # "incomplete" → "length" (max_output_tokens truncation)
+ self.last_stream_stop_reason = "length" if status == "incomplete" else statusThe same normalization should be applied in _generate_via_responses for the non-streaming path:
- if not stop_reason:
- stop_reason = getattr(response, "status", "") or ""
+ if not stop_reason:
+ raw_status = getattr(response, "status", "") or ""
+ stop_reason = "length" if raw_status == "incomplete" else raw_status🤖 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/providers/openai_provider.py` around lines 130 - 135, The
Responses API returns resp.status == "incomplete" when max_output_tokens is hit,
but that value isn't normalized so downstream _ensure_complete_ending (in
autopilot_daemon.py) never treats it as a truncation; update the provider to
normalize that status: in _generate_via_responses (and in the streaming path
where event_type == "response.completed") set self.last_stream_stop_reason to a
normalized token-truncation value (e.g., "max_tokens" or "length") whenever
getattr(resp, "status", "") == "incomplete" (instead of leaving "incomplete" or
an unmapped string); ensure you reference and update last_stream_stop_reason
assignment code that currently reads resp = getattr(chunk, "response", None) /
self.last_stream_stop_reason = getattr(resp, "status", "") or "" so downstream
checks in _ensure_complete_ending will trigger continuation.
Comment on lines
+336
to
+344
| 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), | ||
| ) |
Contributor
There was a problem hiding this comment.
避免在新增路径里使用 INSERT OR REPLACE，会放大并发丢写风险
Line 336 这里是“新增角色”语义，但 INSERT OR REPLACE 会在冲突时覆盖已有记录。并发请求下，前置存在性检查不是原子操作，后到请求可能把先到请求覆盖掉。建议改为严格 INSERT（冲突即失败），由上层转成业务异常。
建议修改
- INSERT OR REPLACE INTO bible_characters (
+ INSERT INTO bible_characters (
id, novel_id, name, description,
mental_state, mental_state_reason, verbal_tic, idle_behavior,
created_at, updated_at
) VALUES (?, ?, ?, ?, ?, '', ?, ?, ?, ?)📝 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
| 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), | |
| ) | |
| INSERT 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), | |
| ) |
🤖 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
336 - 344, This INSERT uses "INSERT OR REPLACE" for the bible_characters table
(parameters: character_id, novel_id, name, description, mental_state,
verbal_tic, idle_behavior, now) but the operation is for "create/add new
character" semantics; change the SQL to a strict "INSERT" so conflicts fail
instead of silently replacing existing rows, and update the surrounding code in
the function that performs this query (the method that binds
character_id/novel_id/etc.) to let the IntegrityError propagate or convert it
into the appropriate business exception (e.g., duplicate-character error) to be
handled by the caller rather than overwriting an existing record.
Comment on lines
+25
to
+27
| class BatchExpandRequest(BaseModel): | ||
| chapter_numbers: list[int] = Field(..., min_length=1, description="章节号列表") | ||
| target_words: int = Field(default=4000, ge=500, le=20000, description="目标字数") |
Contributor
There was a problem hiding this comment.
chapter_numbers has no upper bound — a single request can queue thousands of sequential LLM calls.
With no max_length, a client can submit all chapters of a novel (or an arbitrary large list) in one request. Since batch_expand_chapters is sequential and each expansion runs a full LLM stream, this is a straightforward resource-exhaustion vector.
- chapter_numbers: list[int] = Field(..., min_length=1, description="章节号列表")
+ chapter_numbers: list[int] = Field(..., min_length=1, max_length=50, description="章节号列表")Choose a reasonable upper bound (50 is a suggestion; adjust to match expected batch sizes).
🤖 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/audit/chapter_repair_routes.py` around lines 25 - 27, The
BatchExpandRequest model's chapter_numbers field lacks an upper bound allowing
huge lists; update the Field declaration for chapter_numbers in class
BatchExpandRequest to include a max_length (e.g., max_length=50 or a
project-appropriate limit) so the schema enforces a sensible upper bound and
prevents large batch abuse; adjust any related validation tests or docs to
reflect the new limit.
Comment on lines
+41
to
+122
| @router.get("/novels/{novel_id}/chapter-repair/scan") | ||
| async def scan_short_chapters( | ||
| novel_id: str, | ||
| threshold: int = Query(default=4000, ge=100, le=20000, description="字数阈值"), | ||
| service: ChapterRepairService = Depends(_get_service), | ||
| ): | ||
| """扫描字数不足的章节""" | ||
| result = service.scan_short_chapters(novel_id, threshold) | ||
| return { | ||
| "novel_id": result.novel_id, | ||
| "threshold": result.threshold, | ||
| "total_chapters": result.total_chapters, | ||
| "short_chapters": [ | ||
| { | ||
| "chapter_number": ch.chapter_number, | ||
| "title": ch.title, | ||
| "word_count": ch.word_count, | ||
| "status": ch.status, | ||
| "content_preview": ch.content_preview, | ||
| "severity": ch.severity, | ||
| } | ||
| for ch in result.short_chapters | ||
| ], | ||
| "summary": result.summary, | ||
| } | ||
|
|
||
|
|
||
| @router.post("/novels/{novel_id}/chapter-repair/expand/{chapter_number}") | ||
| async def expand_chapter( | ||
| novel_id: str, | ||
| chapter_number: int, | ||
| request: ExpandChapterRequest, | ||
| service: ChapterRepairService = Depends(_get_service), | ||
| ): | ||
| """SSE 流式扩写单个章节""" | ||
|
|
||
| async def event_gen(): | ||
| try: | ||
| async for event in service.expand_chapter(novel_id, chapter_number, request.target_words): | ||
| yield f"data: {json.dumps(event, ensure_ascii=False)}\n\n" | ||
| except Exception as e: | ||
| logger.error(f"扩写 SSE 异常: {e}") | ||
| yield f"data: {json.dumps({'type': 'error', 'message': str(e)}, ensure_ascii=False)}\n\n" | ||
|
|
||
| return StreamingResponse( | ||
| event_gen(), | ||
| media_type="text/event-stream", | ||
| headers={ | ||
| "Cache-Control": "no-cache", | ||
| "Connection": "keep-alive", | ||
| "X-Accel-Buffering": "no", | ||
| }, | ||
| ) | ||
|
|
||
|
|
||
| @router.post("/novels/{novel_id}/chapter-repair/batch-expand") | ||
| async def batch_expand_chapters( | ||
| novel_id: str, | ||
| request: BatchExpandRequest, | ||
| service: ChapterRepairService = Depends(_get_service), | ||
| ): | ||
| """SSE 流式批量扩写章节""" | ||
|
|
||
| async def event_gen(): | ||
| try: | ||
| async for event in service.batch_expand_chapters( | ||
| novel_id, request.chapter_numbers, request.target_words | ||
| ): | ||
| yield f"data: {json.dumps(event, ensure_ascii=False)}\n\n" | ||
| except Exception as e: | ||
| logger.error(f"批量扩写 SSE 异常: {e}") | ||
| yield f"data: {json.dumps({'type': 'error', 'message': str(e)}, ensure_ascii=False)}\n\n" | ||
|
|
||
| return StreamingResponse( | ||
| event_gen(), | ||
| media_type="text/event-stream", | ||
| headers={ | ||
| "Cache-Control": "no-cache", | ||
| "Connection": "keep-alive", | ||
| "X-Accel-Buffering": "no", | ||
| }, | ||
| ) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether auth middleware or router-level dependency is applied to chapter-repair routes
# 1. Look for how this router is registered (does the include_router call add dependencies?)
rg -n "chapter.repair\|chapter_repair_routes" --type py -C 5
# 2. Check for any global auth middleware or app-level dependency
rg -n "add_middleware\|Depends.*auth\|get_current_user\|verify_token" --type py -C 3
# 3. Check existing audit routers for comparison
fd -e py audit interfaces/api --exec grep -l "Depends"Repository: shenminglinyi/PlotPilot
Length of output: 49
🏁 Script executed:
# First, let's find the exact file and examine its imports and structure
find . -name "chapter_repair_routes.py" -type fRepository: shenminglinyi/PlotPilot
Length of output: 118
🏁 Script executed:
# Find the main app file and how routers are registered
find . -name "main.py" -o -name "app.py" -type f | head -20Repository: shenminglinyi/PlotPilot
Length of output: 88
🏁 Script executed:
# Search for all audit-related files in the interfaces/api directory
fd -e py "audit" interfaces/api/Repository: shenminglinyi/PlotPilot
Length of output: 49
🏁 Script executed:
# Examine the chapter_repair_routes.py file
cat -n ./interfaces/api/v1/audit/chapter_repair_routes.pyRepository: shenminglinyi/PlotPilot
Length of output: 4546
🏁 Script executed:
# Examine main.py to see router registration
cat -n ./interfaces/main.pyRepository: shenminglinyi/PlotPilot
Length of output: 26174
🏁 Script executed:
# Search for authentication/authorization patterns in the codebase
rg -n "get_current_user|verify_token|requires_auth|Depends.*[Aa]uth" --type py | head -50Repository: shenminglinyi/PlotPilot
Length of output: 49
🏁 Script executed:
# Check other audit routes to see if they have auth
find ./interfaces/api/v1/audit -name "*.py" -type fRepository: shenminglinyi/PlotPilot
Length of output: 303
🏁 Script executed:
# Check dependencies module
cat -n ./interfaces/api/dependencies.py 2>/dev/null | head -100Repository: shenminglinyi/PlotPilot
Length of output: 5384
🏁 Script executed:
# Search for any authentication middleware or security patterns
rg -n "SecurityScopes|HTTPBearer|HTTPBasic|JWTBearer" --type py | head -30Repository: shenminglinyi/PlotPilot
Length of output: 49
🏁 Script executed:
# Check the rest of dependencies.py to find get_chapter_repair_service
tail -n +100 ./interfaces/api/dependencies.py | head -200Repository: shenminglinyi/PlotPilot
Length of output: 5361
🏁 Script executed:
# Check another audit route for comparison
cat -n ./interfaces/api/v1/audit/chapter_review_routes.py | head -80Repository: shenminglinyi/PlotPilot
Length of output: 2608
🏁 Script executed:
# Check macro_refactor route
cat -n ./interfaces/api/v1/audit/macro_refactor.py | head -80Repository: shenminglinyi/PlotPilot
Length of output: 3663
🏁 Script executed:
# Search for get_chapter_repair_service in dependencies.py
rg -n "get_chapter_repair_service" ./interfaces/api/dependencies.py -A 10Repository: shenminglinyi/PlotPilot
Length of output: 558
🏁 Script executed:
# Find ChapterRepairService and check for write operations
find . -name "*chapter_repair*service*.py" -type fRepository: shenminglinyi/PlotPilot
Length of output: 122
🏁 Script executed:
# Check if there's ANY auth security pattern in the entire codebase
rg -n "auth|Auth|permission|Permission|access_control" ./interfaces/api/v1 --type py | head -30Repository: shenminglinyi/PlotPilot
Length of output: 802
🏁 Script executed:
# Examine ChapterRepairService implementation
cat -n ./application/audit/services/chapter_repair_service.py | head -150Repository: shenminglinyi/PlotPilot
Length of output: 6369
🏁 Script executed:
# Check if there's a global auth requirement or middleware anywhere
rg -n "APIRouter\|include_router" ./interfaces/api/v1 --type py -A 2 | grep -i "depend\|auth" | head -20Repository: shenminglinyi/PlotPilot
Length of output: 49
🏁 Script executed:
# Verify what the aftermath_pipeline does (mentioned in review)
find . -name "*aftermath*" -type fRepository: shenminglinyi/PlotPilot
Length of output: 127
🏁 Script executed:
# Continue reading ChapterRepairService to see the write operations
cat -n ./application/audit/services/chapter_repair_service.py | sed -n '145,250p'Repository: shenminglinyi/PlotPilot
Length of output: 4761
🏁 Script executed:
# Check the chapter_aftermath_pipeline to understand what it does
cat -n ./application/engine/services/chapter_aftermath_pipeline.py | head -100Repository: shenminglinyi/PlotPilot
Length of output: 4639
🏁 Script executed:
# Search comprehensively for any authentication/security middleware or decorators
rg -n "@.*auth|@.*Auth|@.*secure|@.*Secure|require.*auth|check.*auth" --type py | head -20Repository: shenminglinyi/PlotPilot
Length of output: 201
Add authentication/authorization to all three chapter-repair endpoints.
None of the three endpoints declare auth dependencies. The expand_chapter and batch_expand_chapters endpoints write modified content back to the chapter repository (lines 147–152) and trigger the aftermath pipeline, which performs narrative synchronization, vector indexing, style scoring, and knowledge graph inference. The router itself carries no auth, and no global auth middleware is applied in main.py, so these endpoints are unprotected if the application relies on route-level auth enforcement.
🧰 Tools
🪛 Ruff (0.15.12)
[warning] 45-45: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
[warning] 73-73: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
[warning] 81-81: Do not catch blind exception: Exception
(BLE001)
[warning] 100-100: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
[warning] 110-110: 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/audit/chapter_repair_routes.py` around lines 41 - 122, All
three endpoints (scan_short_chapters, expand_chapter, batch_expand_chapters)
lack auth dependencies; add the application's auth dependency (e.g.
Depends(get_current_user) or whichever auth dependency is used elsewhere) to
each endpoint signature (or attach it as a router-level dependencies=[...] on
router) so FastAPI enforces authentication/authorization before executing the
handlers and before any SSE/streaming work runs; ensure the chosen dependency
returns the user/claims object (or performs authorization) and keep it in the
function params so expand_chapter and batch_expand_chapters still stream
correctly while being protected.
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.
标题: fix(engine): 修复截断检测误判 + 新增章节修复功能
变更说明
架构影响
测试
pytest tests/unit -q --tb=short
706 passed, 48 failed（均为预先存在的失败，与本次改动无关）
风险说明
Summary by CodeRabbit
Release Notes
New Features
Improvements