Feature/db write retry 保存小说（带写锁重试机制）#123
Conversation
📝 WalkthroughWalkthroughThis PR introduces multiple configuration and logic enhancements across the stack: simplifies the frontend Changes
Sequence DiagramssequenceDiagram
participant Client
participant get_service as get_service()
participant OpenAI as OpenAIProvider
participant Anthropic as AnthropicProvider
Client->>get_service: Initialize LLM service
get_service->>OpenAI: Read OPENAI_API_KEY & OPENAI_BASE_URL
alt OpenAI initialization succeeds
OpenAI-->>get_service: Provider ready
get_service-->>Client: OpenAI provider assigned
else OpenAI unavailable/fails
get_service->>Anthropic: Read ANTHROPIC_API_KEY/AUTH_TOKEN
alt Anthropic initialization succeeds
Anthropic-->>get_service: Provider ready
get_service-->>Client: Anthropic provider assigned
else Both fail
get_service-->>Client: RuntimeError with env var requirements
end
end
sequenceDiagram
participant App
participant sqlite_novel_repository as Repository.save()
participant SQLite as SQLite DB
App->>sqlite_novel_repository: save(novel)
sqlite_novel_repository->>sqlite_novel_repository: Build params (once)
loop For each retry attempt
sqlite_novel_repository->>SQLite: execute(...)
alt Success
SQLite-->>sqlite_novel_repository: OK
sqlite_novel_repository-->>App: Return
else OperationalError: "database is locked"
sqlite_novel_repository->>sqlite_novel_repository: Calculate backoff<br/>(base × 2^attempt + jitter)
sqlite_novel_repository->>sqlite_novel_repository: Wait (exponential backoff)
sqlite_novel_repository->>sqlite_novel_repository: Log warning
Note over sqlite_novel_repository: Retry next iteration
else Other error
sqlite_novel_repository-->>App: Re-raise exception
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
infrastructure/ai/providers/openai_provider.py (1)
38-47:⚠️ Potential issue | 🟠 MajorPass custom httpx client with
trust_env=Falseto match other providers.The OpenAI SDK's
AsyncOpenAIconstructor accepts anhttp_clientparameter for custom httpx configuration.AnthropicProviderandGeminiProviderboth explicitly passhttpx.AsyncClient(trust_env=False)to prevent system proxy interference that causes TLS errors (as documented in the AnthropicProvider comment).OpenAIProvidercurrently omits this, relying on the SDK's default httpx client withtrust_env=True, which will route requests through system HTTP(S)_PROXY and risks connection failures in proxy environments.Add:
client_kwargs["http_client"] = httpx.AsyncClient( timeout=httpx.Timeout(settings.timeout_seconds), trust_env=False )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/ai/providers/openai_provider.py` around lines 38 - 47, The OpenAIProvider currently constructs client_kwargs and instantiates AsyncOpenAI without supplying a custom httpx client, which leaves trust_env=True and can cause proxy/TLS issues; update the construction of client_kwargs (the dict passed to AsyncOpenAI in the OpenAIProvider) to include an "http_client" entry that is an instance of httpx.AsyncClient created with timeout=httpx.Timeout(settings.timeout_seconds) and trust_env=False so the AsyncOpenAI(... client_kwargs) call uses the custom client and matches AnthropicProvider/GeminiProvider behavior.
🧹 Nitpick comments (9)
interfaces/api/v1/core/novels.py (1)
20-20: Minor formatting: missing space after comma.✨ Proposed fix
-router = APIRouter(prefix="/novels",redirect_slashes=False, tags=["novels"]) +router = APIRouter(prefix="/novels", redirect_slashes=False, tags=["novels"])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interfaces/api/v1/core/novels.py` at line 20, The APIRouter instantiation line has a formatting issue: add a space after the comma between prefix="/novels" and redirect_slashes=False so the call to APIRouter(...) reads with proper spacing; update the router = APIRouter(...) expression (the router symbol and APIRouter call) to include that space.infrastructure/persistence/database/sqlite_novel_repository.py (1)
106-108: Move imports to module level.Importing
time,sqlite3, andrandominside the method adds overhead on each call and deviates from Python conventions. These are standard library modules and should be imported at the top of the file.♻️ Proposed fix
At the top of the file (around line 4):
import time import randomThen remove lines 106-108 from inside the method. Note:
sqlite3is already implicitly available via the connection module, but if you need to catchsqlite3.OperationalError, addimport sqlite3at module level.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/persistence/database/sqlite_novel_repository.py` around lines 106 - 108, The inline imports of time, sqlite3, and random inside the method should be moved to the module level to avoid per-call overhead and follow Python conventions: add "import time" and "import random" at the top of the file (and "import sqlite3" at module level only if you need to reference sqlite3 exceptions like sqlite3.OperationalError) and remove the corresponding import statements currently inside the method in sqlite_novel_repository.py so only top-level imports remain; ensure any exception handling or references in functions (e.g., where sqlite3.OperationalError is used) still work after the move.interfaces/api/v1/blueprint/continuous_planning_routes.py (5)
135-142: Prefer constructingSettingswith all values upfront.Mutating a dataclass after construction is not idiomatic and bypasses
__post_init__validation. Use the constructor to set all fields:♻️ Proposed fix
- settings = Settings() - settings.api_key = openai_api_key.strip() - settings.base_url = openai_base_url.strip() - settings.default_model = os.getenv('DEFAULT_LLM_MODEL_NAME', 'qwen/qwen3.5-397b-a17b') - settings.use_legacy_chat_completions = True # 关键：使用 /v1/chat/completions - settings.timeout_seconds = 300.0 + settings = Settings( + api_key=openai_api_key.strip(), + base_url=openai_base_url.strip(), + default_model=os.getenv('DEFAULT_LLM_MODEL_NAME', 'qwen/qwen3.5-397b-a17b'), + use_legacy_chat_completions=True, + timeout_seconds=300.0, + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interfaces/api/v1/blueprint/continuous_planning_routes.py` around lines 135 - 142, The code mutates a Settings instance after construction; instead instantiate Settings with all fields set to ensure __post_init__ runs and validations apply. Replace the current multi-line mutation with a single constructor call to Settings(...) passing api_key=openai_api_key.strip(), base_url=openai_base_url.strip(), default_model=os.getenv('DEFAULT_LLM_MODEL_NAME', 'qwen/qwen3.5-397b-a17b'), use_legacy_chat_completions=True, timeout_seconds=300.0 and then pass that Settings instance to OpenAIProvider(settings=...). Locate the Settings creation and llm_service = OpenAIProvider(...) in continuous_planning_routes (the block where settings is built) and update there.
37-41: Remove duplicate imports ofBaseModelandField.These are already imported at line 8.
♻️ Proposed fix
- -from pydantic import BaseModel, Field - - -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interfaces/api/v1/blueprint/continuous_planning_routes.py` around lines 37 - 41, Remove the duplicate import statement "from pydantic import BaseModel, Field" that was reintroduced at lines 37-41; keep the original import already declared earlier (line ~8) and delete this redundant import so only one "BaseModel, Field" import remains in the module.
49-52: Remove large blocks of commented-out code.Multiple sections of commented-out code (~100 lines total) add noise and reduce maintainability. If this code is no longer needed, remove it. If it's for reference, consider documenting the alternative approach in comments or an ADR instead.
Also applies to: 82-116, 186-230, 277-278
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interfaces/api/v1/blueprint/continuous_planning_routes.py` around lines 49 - 52, Remove the large commented-out blocks (e.g., the commented GenerateMacroPlanRequest class and other commented sections around the indicated ranges) from interfaces/api/v1/blueprint/continuous_planning_routes.py to reduce noise; if the code is meant to be preserved for reference, extract it into a short ADR or add a concise explanatory comment linking to the ADR instead of leaving multiline commented code in the file, and ensure any needed type names (like GenerateMacroPlanRequest, StructurePreference) are either restored as real definitions or omitted so there are no dead comments left in the module.
24-29: Remove duplicate import ofget_database.
get_databaseis imported twice: at line 22 (frominterfaces.api.dependencies) and at line 29 (frominfrastructure.persistence.database.connection). The import at line 29 shadows the first one.♻️ Proposed fix
import os from infrastructure.ai.providers.openai_provider import OpenAIProvider from infrastructure.ai.config.settings import Settings - - -from infrastructure.persistence.database.connection import get_database🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interfaces/api/v1/blueprint/continuous_planning_routes.py` around lines 24 - 29, The file imports get_database twice (once from interfaces.api.dependencies and again from infrastructure.persistence.database.connection), which causes the latter to shadow the former; remove the duplicate import by deleting the redundant "from infrastructure.persistence.database.connection import get_database" line and ensure all references in continuous_planning_routes.py use the single intended get_database import (verify usages in route handlers/methods that call get_database).
143-145: Replaceprint()statements with proper logging.Using
print()bypasses the logging configuration. Use a logger for consistency with the rest of the codebase.Also applies to: 158-158
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interfaces/api/v1/blueprint/continuous_planning_routes.py` around lines 143 - 145, Replace the print() calls inside the OpenAIProvider initialization try/except with the module logger so messages go through the app logging system: in the try block that references settings.default_model and settings.base_url, call logger.info(...) (or logger.debug as appropriate) instead of print for the success message, and in the except block call logger.error(..., exc_info=True) to include the exception details; also change the other print() at the later occurrence (around the same OpenAIProvider init) to use the same logger level for consistency.infrastructure/ai/providers/openai_provider.py (1)
105-105: Debug print statement should use logger.♻️ Proposed fix
- print(f"[LLM] 生成成功，内容长度: {len(content)}，输入tokens: {input_tokens}，输出tokens: {output_tokens}") + logger.debug("Generation successful, content_length: %d, input_tokens: %d, output_tokens: %d", len(content), input_tokens, output_tokens)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/ai/providers/openai_provider.py` at line 105, Replace the raw debug print call that logs generation metrics with the module logger (e.g., logger.info or logger.debug) so logs go through the configured logging system; change the print(f"[LLM] 生成成功，内容长度: {len(content)}，输入tokens: {input_tokens}，输出tokens: {output_tokens}") to logger.info(...) or logger.debug(...) using the same formatted message and the existing logger in openai_provider.py (referencing variables content, input_tokens, output_tokens) so it integrates with normal log handling.interfaces/main.py (1)
20-21: Remove dead commented-out code.This commented-out environment variable line serves no purpose and adds noise. If it's needed for debugging, consider using a proper environment variable check instead.
🧹 Proposed fix
-#os.environ['DISABLE_AUTO_DAEMON'] = '1' -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interfaces/main.py` around lines 20 - 21, Remove the dead commented-out line "#os.environ['DISABLE_AUTO_DAEMON'] = '1'" from the module to eliminate noise; if you need to toggle DISABLE_AUTO_DAEMON for debugging, implement a proper runtime check that reads os.environ (e.g., use os.getenv or a config loader) rather than leaving commented-out code in the source.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gitignore:
- Line 161: Remove the literal ".gitignore" entry from the .gitignore file so
Git can track and commit changes to that file; open the .gitignore, locate the
line containing ".gitignore" (exact string) and delete it, then save and commit
the updated .gitignore ensuring no other rules inadvertently exclude the file
(e.g., negate patterns or parent ignores) so the file remains tracked.
- Line 157: The .gitignore entry "%USERPROFILE%/" is incorrect because git
treats it as a literal path; replace this pattern with concrete ignore patterns
for the actual local-temp directories you want ignored (for example use names
like tmp/, temp/, build/, .cache/ or any project-specific temp folder names), or
move a custom local-only rule into .git/info/exclude; update the file to remove
"%USERPROFILE%/" and add the explicit directory patterns (or add guidance in
repo docs about adding OS-specific paths to .git/info/exclude) so the intended
local profile/temp files are actually ignored.
In `@frontend/vite.config.ts`:
- Around line 30-37: Remove the dead redirect logging in the Vite proxy config:
delete the conditional that checks proxyRes.statusCode === 307 || 308 and the
console.log inside onProxyRes (or set followRedirects to false if you intend to
handle redirects manually); update the onProxyRes handler to only contain live
error/response handling logic and ensure followRedirects remains true if you
want automatic redirect following.
In `@infrastructure/ai/providers/openai_provider.py`:
- Around line 83-91: Replace the three print() calls in the chat call block with
proper logger calls: change the pre-call debug print to logger.debug(...)
including model and max_tokens from request_kwargs, change the success print to
logger.info(...) and remove the unnecessary f-string prefix (no placeholders),
and replace the exception handling print/traceback with logger.exception(...) so
the exception and stacktrace are captured by the logging system before
re-raising; target the async_client.chat.completions.create call and the
surrounding try/except in the same function to locate the edits.
In `@infrastructure/persistence/database/sqlite_novel_repository.py`:
- Around line 106-124: The exponential backoff in sqlite_novel_repository.py
(variables max_retries and base_delay inside the for loop that calls
self.db.execute and self.db.get_connection().commit()) can grow far beyond the
SQLite connection busy_timeout; modify the retry logic to cap the exponential
delay (e.g., compute wait = min(base_delay * (2 ** attempt) + jitter,
max_delay)) and set max_delay to a value consistent with the connection
busy_timeout (≈30s), or reduce max_retries accordingly so retries occur within
the busy_handler window; ensure the "database is locked" branch uses the capped
wait and keeps the same logger.warning and time.sleep semantics.
In `@interfaces/api/v1/blueprint/continuous_planning_routes.py`:
- Around line 159-160: Replace the bare "except Exception: pass" in the
Anthropic fallback block with proper logging: add "import logging" and "logger =
logging.getLogger(__name__)" at the top of the file, then catch the exception
and call logger.exception(...) or logger.error(..., exc_info=True) inside that
except so the initialization error is recorded (locate the bare except in the
Anthropic fallback handling in continuous_planning_routes.py).
---
Outside diff comments:
In `@infrastructure/ai/providers/openai_provider.py`:
- Around line 38-47: The OpenAIProvider currently constructs client_kwargs and
instantiates AsyncOpenAI without supplying a custom httpx client, which leaves
trust_env=True and can cause proxy/TLS issues; update the construction of
client_kwargs (the dict passed to AsyncOpenAI in the OpenAIProvider) to include
an "http_client" entry that is an instance of httpx.AsyncClient created with
timeout=httpx.Timeout(settings.timeout_seconds) and trust_env=False so the
AsyncOpenAI(... client_kwargs) call uses the custom client and matches
AnthropicProvider/GeminiProvider behavior.
---
Nitpick comments:
In `@infrastructure/ai/providers/openai_provider.py`:
- Line 105: Replace the raw debug print call that logs generation metrics with
the module logger (e.g., logger.info or logger.debug) so logs go through the
configured logging system; change the print(f"[LLM] 生成成功，内容长度:
{len(content)}，输入tokens: {input_tokens}，输出tokens: {output_tokens}") to
logger.info(...) or logger.debug(...) using the same formatted message and the
existing logger in openai_provider.py (referencing variables content,
input_tokens, output_tokens) so it integrates with normal log handling.
In `@infrastructure/persistence/database/sqlite_novel_repository.py`:
- Around line 106-108: The inline imports of time, sqlite3, and random inside
the method should be moved to the module level to avoid per-call overhead and
follow Python conventions: add "import time" and "import random" at the top of
the file (and "import sqlite3" at module level only if you need to reference
sqlite3 exceptions like sqlite3.OperationalError) and remove the corresponding
import statements currently inside the method in sqlite_novel_repository.py so
only top-level imports remain; ensure any exception handling or references in
functions (e.g., where sqlite3.OperationalError is used) still work after the
move.
In `@interfaces/api/v1/blueprint/continuous_planning_routes.py`:
- Around line 135-142: The code mutates a Settings instance after construction;
instead instantiate Settings with all fields set to ensure __post_init__ runs
and validations apply. Replace the current multi-line mutation with a single
constructor call to Settings(...) passing api_key=openai_api_key.strip(),
base_url=openai_base_url.strip(),
default_model=os.getenv('DEFAULT_LLM_MODEL_NAME', 'qwen/qwen3.5-397b-a17b'),
use_legacy_chat_completions=True, timeout_seconds=300.0 and then pass that
Settings instance to OpenAIProvider(settings=...). Locate the Settings creation
and llm_service = OpenAIProvider(...) in continuous_planning_routes (the block
where settings is built) and update there.
- Around line 37-41: Remove the duplicate import statement "from pydantic import
BaseModel, Field" that was reintroduced at lines 37-41; keep the original import
already declared earlier (line ~8) and delete this redundant import so only one
"BaseModel, Field" import remains in the module.
- Around line 49-52: Remove the large commented-out blocks (e.g., the commented
GenerateMacroPlanRequest class and other commented sections around the indicated
ranges) from interfaces/api/v1/blueprint/continuous_planning_routes.py to reduce
noise; if the code is meant to be preserved for reference, extract it into a
short ADR or add a concise explanatory comment linking to the ADR instead of
leaving multiline commented code in the file, and ensure any needed type names
(like GenerateMacroPlanRequest, StructurePreference) are either restored as real
definitions or omitted so there are no dead comments left in the module.
- Around line 24-29: The file imports get_database twice (once from
interfaces.api.dependencies and again from
infrastructure.persistence.database.connection), which causes the latter to
shadow the former; remove the duplicate import by deleting the redundant "from
infrastructure.persistence.database.connection import get_database" line and
ensure all references in continuous_planning_routes.py use the single intended
get_database import (verify usages in route handlers/methods that call
get_database).
- Around line 143-145: Replace the print() calls inside the OpenAIProvider
initialization try/except with the module logger so messages go through the app
logging system: in the try block that references settings.default_model and
settings.base_url, call logger.info(...) (or logger.debug as appropriate)
instead of print for the success message, and in the except block call
logger.error(..., exc_info=True) to include the exception details; also change
the other print() at the later occurrence (around the same OpenAIProvider init)
to use the same logger level for consistency.
In `@interfaces/api/v1/core/novels.py`:
- Line 20: The APIRouter instantiation line has a formatting issue: add a space
after the comma between prefix="/novels" and redirect_slashes=False so the call
to APIRouter(...) reads with proper spacing; update the router = APIRouter(...)
expression (the router symbol and APIRouter call) to include that space.
In `@interfaces/main.py`:
- Around line 20-21: Remove the dead commented-out line
"#os.environ['DISABLE_AUTO_DAEMON'] = '1'" from the module to eliminate noise;
if you need to toggle DISABLE_AUTO_DAEMON for debugging, implement a proper
runtime check that reads os.environ (e.g., use os.getenv or a config loader)
rather than leaving commented-out code in the source.
🪄 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: aa4f9f8b-49d8-4ee6-a2a6-b03f081adb64
📒 Files selected for processing (8)
.gitignorefrontend/src/api/novel.tsfrontend/vite.config.tsinfrastructure/ai/providers/openai_provider.pyinfrastructure/persistence/database/sqlite_novel_repository.pyinterfaces/api/v1/blueprint/continuous_planning_routes.pyinterfaces/api/v1/core/novels.pyinterfaces/main.py
💤 Files with no reviewable changes (1)
- frontend/src/api/novel.ts
| # ════════════════════════════════════════════ | ||
| # 本地临时文件和目录（自定义，不入库） | ||
| # ════════════════════════════════════════════ | ||
| %USERPROFILE%/ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if literal %USERPROFILE% directories exist that justify this pattern
fd -t d '^%USERPROFILE%$' --max-depth 3Repository: shenminglinyi/PlotPilot
Length of output: 49
🏁 Script executed:
# Read .gitignore around lines 157-159 to see context and comments
sed -n '150,165p' .gitignoreRepository: shenminglinyi/PlotPilot
Length of output: 275
🌐 Web query:
Does git expand environment variables in .gitignore patterns like %USERPROFILE%
💡 Result:
No, Git does not expand environment variables like %USERPROFILE% in .gitignore patterns. .gitignore patterns are treated as literal static strings using fnmatch-style glob matching without shell expansion or variable substitution. Official Git documentation for gitignore(5) describes the PATTERN FORMAT exhaustively, listing supported syntax like wildcards (*, ?), character ranges, for recursive directories, negation (!), escapes (), and comments (#), but makes no mention of environment variable expansion. Patterns follow fnmatch(3) rules with Git-specific extensions for paths, confirming literal interpretation. Stack Overflow discussions confirm this: users attempting constructs like .$(echo %username%) or conditional env-based ignores are told patterns are "completely static" with no shell/language expansion. For user-specific ignores (e.g., Windows username files), solutions involve per-repo .git/info/exclude or local core.excludesfile with hardcoded paths, not variables in patterns. Even core.excludesfile paths support ~ expansion in git-config (e.g., ~/foo expands to $HOME/foo), but patterns within these files remain literal. No evidence of %VAR% or $VAR expansion inside patterns across Git versions up to 2.53 (2025). Workarounds for dynamic ignores: - Generate .gitignore via scripts/Makefile before Git operations. - Use repo-local .git/info/exclude for uncommitted user-specific rules. - Global core.excludesfile = %USERPROFILE%.gitignore (expands in config value, not pattern). To test: echo "%USERPROFILE%/temp" > .gitignore; touch "$HOME/temp.txt"; git status --ignored --others (temp.txt appears unless path literally matches %USERPROFILE%).
Citations:
- 1: https://web.mit.edu/git/git-doc/gitignore.html
- 2: https://stackoverflow.com/questions/17640160/is-it-possible-to-make-gitignore-configurable-based-on-environment-variables
- 3: https://stackoverflow.com/questions/26670878/gitignore-my-windows-username
- 4: https://git-scm.com/docs/gitignore.html
- 5: https://mirrors.edge.kernel.org/pub/software/scm/git/docs/gitignore.html
- 6: https://git-scm.com/docs/gitignore
- 7: https://git-scm.com/docs/gitignore/2.32.0.html
- 8: https://www.man7.org/linux/man-pages/man5/gitignore.5.html
- 9: https://stackoverflow.com/questions/7335420/can-i-use-a-global-user-profile-scope-gitignore-file
- 10: https://git-scm.com/docs/git-ls-files.html
Environment variable patterns won't work in .gitignore.
Git does not expand environment variables in .gitignore patterns; they are treated as literal static strings. These patterns will only match directories literally named %USERPROFILE%/, not the actual user profile path.
Based on the comment here, you intend to ignore local temporary directories. Use concrete path patterns instead, or consider using .git/info/exclude for local-only ignore rules.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.gitignore at line 157, The .gitignore entry "%USERPROFILE%/" is incorrect
because git treats it as a literal path; replace this pattern with concrete
ignore patterns for the actual local-temp directories you want ignored (for
example use names like tmp/, temp/, build/, .cache/ or any project-specific temp
folder names), or move a custom local-only rule into .git/info/exclude; update
the file to remove "%USERPROFILE%/" and add the explicit directory patterns (or
add guidance in repo docs about adding OS-specific paths to .git/info/exclude)
so the intended local profile/temp files are actually ignored.
| .history/ | ||
| frontend/%USERPROFILE%/ | ||
| test_llm.py | ||
| .gitignore |
There was a problem hiding this comment.
CRITICAL: Remove .gitignore from ignore patterns.
Adding .gitignore to the ignore list prevents Git from tracking changes to the .gitignore file itself. This means future updates to .gitignore cannot be committed or pushed, effectively breaking version control for this critical configuration file.
🚨 Proposed fix
frontend/%USERPROFILE%/
test_llm.py
-.gitignore🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.gitignore at line 161, Remove the literal ".gitignore" entry from the
.gitignore file so Git can track and commit changes to that file; open the
.gitignore, locate the line containing ".gitignore" (exact string) and delete
it, then save and commit the updated .gitignore ensuring no other rules
inadvertently exclude the file (e.g., negate patterns or parent ignores) so the
file remains tracked.
| // 🔥 关键：跟随重定向（让代理处理，而非浏览器） | ||
| followRedirects: true, | ||
| // 🔥 添加：错误处理 | ||
| onProxyRes: (proxyRes, req, res) => { | ||
| if (proxyRes.statusCode === 307 || proxyRes.statusCode === 308) { | ||
| console.log('⚠️ Redirect detected:', proxyRes.headers.location); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Vite dev server proxy followRedirects onProxyRes behavior http-proxy
💡 Result:
In Vite's dev server proxy (powered by http-proxy), the followRedirects option controls whether the proxy automatically follows HTTP redirects (3xx responses) from the target server. Default is false, meaning redirects are passed through to the client as-is. To enable automatic following of redirects, pass followRedirects: true in the proxy options via vite.config.js: export default defineConfig({ server: { proxy: { '/api': { target: 'http://example.com', followRedirects: true, // other options... } } } }) Regarding onProxyRes behavior: This event fires for every response from the target, including redirects (e.g., 301/302). If followRedirects: true, the proxy internally follows the redirect (using the follow-redirects dependency), so onProxyRes receives the final response after all redirects. If false (default), onProxyRes receives the initial redirect response, which you can inspect/modify (e.g., rewrite Location header) or handle manually. To customize onProxyRes, use the configure function to access the proxy instance: '/api': { target: 'http://example.com', configure: (proxy) => { proxy.on('proxyRes', (proxyRes, req, res) => { if (proxyRes.statusCode >= 300 && proxyRes.statusCode < 400) { // Handle redirect: e.g., rewrite Location const location = proxyRes.headers.location; proxyRes.headers.location = location.replace('old', 'new'); } }); } } Note: Setting selfHandleResponse: true requires manually ending the response in onProxyRes handlers. Redirects are not followed automatically unless followRedirects: true. Use options like hostRewrite, autoRewrite, or protocolRewrite for automatic Location header rewriting on redirects without following them.
Citations:
- 1: https://v3.vitejs.dev/config/server-options.html
- 2: https://stackoverflow.com/questions/64677212/how-to-configure-proxy-in-vite/77246318
- 3: https://vite.dev/config/server-options
- 4: https://www.npmjs.com/package/http-proxy
- 5: https://github.com/http-party/node-http-proxy
- 6: https://www.npmjs.com/package/http-proxy-middleware
Remove the redirect logging in onProxyRes—it will never execute with followRedirects: true.
When followRedirects: true is set, the proxy internally follows 307/308 redirects before onProxyRes fires. The callback receives the final response status, not the intermediate redirect status. The condition if (proxyRes.statusCode === 307 || proxyRes.statusCode === 308) will never be true and the logging is dead code.
Either remove the logging or set followRedirects: false and handle redirects manually in onProxyRes if redirect tracking is genuinely needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/vite.config.ts` around lines 30 - 37, Remove the dead redirect
logging in the Vite proxy config: delete the conditional that checks
proxyRes.statusCode === 307 || 308 and the console.log inside onProxyRes (or set
followRedirects to false if you intend to handle redirects manually); update the
onProxyRes handler to only contain live error/response handling logic and ensure
followRedirects remains true if you want automatic redirect following.
| print(f"[LLM] 准备调用 Chat API，模型: {request_kwargs.get('model')}，max_tokens: {request_kwargs.get('max_tokens')}") | ||
| try: | ||
| response = await self.async_client.chat.completions.create(**request_kwargs) | ||
| print(f"[LLM] 收到响应，status: success") | ||
| except Exception as e: | ||
| print(f"[LLM] 请求异常: {type(e).__name__}: {e}") | ||
| import traceback | ||
| traceback.print_exc() | ||
| raise |
There was a problem hiding this comment.
Replace print() debug statements with proper logging.
Using print() for debugging is inappropriate for production code. These statements bypass the logging configuration and will clutter stdout. Use logger.debug() or logger.info() instead.
Also, line 86 has an f-string without placeholders (flagged by static analysis).
🛠️ Proposed fix
- print(f"[LLM] 准备调用 Chat API，模型: {request_kwargs.get('model')}，max_tokens: {request_kwargs.get('max_tokens')}")
+ logger.debug("Calling Chat API, model: %s, max_tokens: %s", request_kwargs.get('model'), request_kwargs.get('max_tokens'))
try:
response = await self.async_client.chat.completions.create(**request_kwargs)
- print(f"[LLM] 收到响应，status: success")
+ logger.debug("Chat API response received successfully")
except Exception as e:
- print(f"[LLM] 请求异常: {type(e).__name__}: {e}")
- import traceback
- traceback.print_exc()
+ logger.error("Chat API request failed: %s: %s", type(e).__name__, e, exc_info=True)
raise🧰 Tools
🪛 Ruff (0.15.10)
[warning] 83-83: String contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF001)
[warning] 83-83: String contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF001)
[error] 86-86: f-string without any placeholders
Remove extraneous f prefix
(F541)
[warning] 86-86: String contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@infrastructure/ai/providers/openai_provider.py` around lines 83 - 91, Replace
the three print() calls in the chat call block with proper logger calls: change
the pre-call debug print to logger.debug(...) including model and max_tokens
from request_kwargs, change the success print to logger.info(...) and remove the
unnecessary f-string prefix (no placeholders), and replace the exception
handling print/traceback with logger.exception(...) so the exception and
stacktrace are captured by the logging system before re-raising; target the
async_client.chat.completions.create call and the surrounding try/except in the
same function to locate the edits.
| import time | ||
| import sqlite3 | ||
| import random | ||
|
|
||
| max_retries = 15 | ||
| base_delay = 1.0 | ||
|
|
||
| for attempt in range(max_retries): | ||
| try: | ||
| self.db.execute(sql, params) | ||
| self.db.get_connection().commit() | ||
| return | ||
| except sqlite3.OperationalError as e: | ||
| if "database is locked" in str(e) and attempt < max_retries - 1: | ||
| wait = base_delay * (2 ** attempt) + random.uniform(0, 1.0) | ||
| logger.warning(f"数据库写锁冲突，{wait:.2f}秒后重试 (attempt {attempt + 1}/{max_retries})") | ||
| time.sleep(wait) | ||
| continue | ||
| raise |
There was a problem hiding this comment.
Exponential backoff grows unbounded and inconsistent with connection timeout.
The retry logic uses base_delay * (2 ** attempt) which grows exponentially:
- Attempt 5: ~32s wait
- Attempt 10: ~1024s wait
- Attempt 14: ~16384s wait
This is inconsistent with the 30-second busy_timeout configured at the connection level (per connection.py:336-342). After SQLite's built-in busy handler gives up at 30s, waiting 16000+ seconds before retrying is counterproductive.
Consider capping the maximum delay or reducing max_retries to align with the connection timeout:
🛠️ Proposed fix with capped delay
max_retries = 15
base_delay = 1.0
+ max_delay = 30.0 # Cap to match busy_timeout
for attempt in range(max_retries):
try:
self.db.execute(sql, params)
self.db.get_connection().commit()
return
except sqlite3.OperationalError as e:
if "database is locked" in str(e) and attempt < max_retries - 1:
- wait = base_delay * (2 ** attempt) + random.uniform(0, 1.0)
+ wait = min(base_delay * (2 ** attempt), max_delay) + random.uniform(0, 1.0)
logger.warning(f"数据库写锁冲突，{wait:.2f}秒后重试 (attempt {attempt + 1}/{max_retries})")
time.sleep(wait)
continue
raise🧰 Tools
🪛 Ruff (0.15.10)
[error] 120-120: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
[warning] 121-121: String contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@infrastructure/persistence/database/sqlite_novel_repository.py` around lines
106 - 124, The exponential backoff in sqlite_novel_repository.py (variables
max_retries and base_delay inside the for loop that calls self.db.execute and
self.db.get_connection().commit()) can grow far beyond the SQLite connection
busy_timeout; modify the retry logic to cap the exponential delay (e.g., compute
wait = min(base_delay * (2 ** attempt) + jitter, max_delay)) and set max_delay
to a value consistent with the connection busy_timeout (≈30s), or reduce
max_retries accordingly so retries occur within the busy_handler window; ensure
the "database is locked" branch uses the capped wait and keeps the same
logger.warning and time.sleep semantics.
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Don't silently swallow exceptions in Anthropic fallback.
The bare except Exception: pass hides initialization failures, making debugging difficult. At minimum, log the error:
🛠️ Proposed fix
except Exception:
- pass
+ logger.warning("AnthropicProvider initialization failed", exc_info=True)Note: You'll need to add import logging and logger = logging.getLogger(__name__) at the top of the file.
🧰 Tools
🪛 Ruff (0.15.10)
[error] 159-160: try-except-pass detected, consider logging the exception
(S110)
[warning] 159-159: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@interfaces/api/v1/blueprint/continuous_planning_routes.py` around lines 159 -
160, Replace the bare "except Exception: pass" in the Anthropic fallback block
with proper logging: add "import logging" and "logger =
logging.getLogger(__name__)" at the top of the file, then catch the exception
and call logger.exception(...) or logger.error(..., exc_info=True) inside that
except so the initialization error is recorded (locate the bare except in the
Anthropic fallback handling in continuous_planning_routes.py).
|
感谢 @LuoFengXiaoXiao 的贡献！ 这个 DB 写锁重试机制的思路很有价值，但注意到 PR 描述中提到「运行起来有点儿怪」，同时代码中保留了 为了保持主分支稳定，暂时先关闭此 PR。建议：
您的贡献已被记录在核心贡献者名单中，后续完善后欢迎重新提交。再次感谢！ |
2 participants
变更类型
sqlite_novel_repository.py.save
变更说明
架构影响
infrastructure（删除不适用项）测试
python -m uvicorn ...）npm run dev）风险说明
Summary by CodeRabbit
Release Notes
Bug Fixes
API Changes
Chores