feat: 增加 GCP Vertex AI 企业级接口支持与向导页 UI/UX 优化#156
Open
professorczh wants to merge 26 commits into
Open
Conversation
…server invocation
…ntegrate LLM provider infrastructure with context budgeting services.
… stability patches
…gration, and Black-Gold theme harmonization
… for prompt management
…implement continuous planning API routes and theme management.
…ex AI integration, and frontend configuration management
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Vertex AI as a new LLM protocol/provider (google-genai), wires it through presets, factory, client, and DB schema/migration; exposes Vertex-related env vars and optional requirements file; updates frontend UI to select Vertex AI and onboarding/workbench UX; includes supporting tooling, CLI, and gitignore adjustments. ChangesVertex AI + Integration Flow
Sequence Diagram(s)sequenceDiagram
participant User as Frontend/User
participant UI as LLMControlPanel
participant Service as LLMControlService
participant Factory as LLMProviderFactory
participant Provider as VertexAIProvider
participant GCP as Vertex AI API
User->>UI: Select "Vertex AI / GCP"
UI->>Service: Request presets/profiles
Service-->>UI: Return presets (includes vertex-ai)
User->>Service: Trigger generation (with credentials present)
Service->>Factory: create_from_profile(vertex-ai profile)
Factory->>Provider: Instantiate VertexAIProvider(settings)
Provider->>GCP: Initialize genai.Client (vertexai=True, billing header)
Service->>Provider: generate(prompt, config)
Provider->>GCP: models.generate_content (async)
GCP-->>Provider: response + usage
Provider-->>Service: GenerationResult
Service-->>UI: Rendered response to User
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
frontend/src/components/onboarding/NovelSetupGuide.vue (1)
260-313:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStep 6 is unreachable, and the progress header is now out of sync.
This adds a sixth panel, but the wizard still behaves like a 5-step flow: the header at Line 11 still renders 5
n-steps,handleNext()only advances whilecurrentStep < 5, and the Step 5 CTA closes the modal instead of advancing.detectWizardProgress()also still resumes fully completed novels at Step 5, so they reopen on the “设计情节弧线” screen instead of the completion screen.Suggested fix
- <n-step title="故事线" description="主线支线" /> - <n-step title="开始" description="进入工作台" /> + <n-step title="故事线" description="主线支线" /> + <n-step title="情节弧线" description="起承转合" /> + <n-step title="开始" description="进入工作台" />- <n-button v-if="currentStep === 5" type="primary" `@click`="handleComplete"> - 进入工作台 - </n-button> + <n-button v-if="currentStep === 5" type="primary" `@click`="handleNext"> + 下一步 + </n-button> + <n-button v-if="currentStep === 6" type="primary" `@click`="handleComplete"> + 进入工作台 + </n-button>And outside this hunk, align the state machine as well:
// handleNext } else if (currentStep.value < 6) { currentStep.value++ } // detectWizardProgress resumedFromStep.value = 6 return 6🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/onboarding/NovelSetupGuide.vue` around lines 260 - 313, The wizard was extended to six panels but state/controls still assume five steps; update the step count and transitions so currentStep can reach 6: adjust the progress header to render 6 n-steps (instead of 5), change handleNext to increment currentStep while currentStep < 6 (so Next advances from 5→6), change the Step 5 CTA behavior (it should call handleNext instead of closing the modal) or ensure handleComplete only runs when on step 6, and update detectWizardProgress to return/resume at step 6 (set resumedFromStep.value = 6). Locate and modify the symbols currentStep, handleNext, handleComplete (or the Step 5 CTA), the progress header n-steps rendering, and detectWizardProgress to keep the state machine consistent.interfaces/api/v1/analyst/voice.py (1)
83-114:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winType declaration allows
nullbut implementation never returns it.The
response_model=Optional[VoiceFingerprintResponse]and return type-> Optional[VoiceFingerprintResponse]declare thatnullis a valid response. However, the implementation always returns aVoiceFingerprintResponseobject—either the found fingerprint (lines 116-122) or a default empty fingerprint (lines 108-114).Either:
- Remove
OptionalsinceNoneis never returned, or- If
nullis the intended behavior for missing fingerprints, change lines 106-114 toreturn None.🔧 Option 1: Remove Optional (matches current behavior)
`@router.get`( "/novels/{novel_id}/voice/fingerprint", - response_model=Optional[VoiceFingerprintResponse], + response_model=VoiceFingerprintResponse, status_code=200, summary="获取文风指纹", description="获取小说的文风指纹统计数据" ) def get_voice_fingerprint( novel_id: str = Path(..., description="小说 ID"), pov_character_id: Optional[str] = Query(None, description="POV 角色 ID"), service=Depends(get_voice_fingerprint_service) -) -> Optional[VoiceFingerprintResponse]: +) -> VoiceFingerprintResponse:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interfaces/api/v1/analyst/voice.py` around lines 83 - 114, The route declares Optional but always returns a VoiceFingerprintResponse; update get_voice_fingerprint to remove Optional: change response_model=Optional[VoiceFingerprintResponse] to response_model=VoiceFingerprintResponse and change the function return annotation from -> Optional[VoiceFingerprintResponse] to -> VoiceFingerprintResponse (ensure any imports/type hints are adjusted and keep the existing default-empty VoiceFingerprintResponse return path).application/ai/llm_control_service.py (1)
390-399:⚠️ Potential issue | 🟠 Major | ⚡ Quick winVertex AI ADC profiles show incorrect mock-provider warnings and are blocked from connection testing.
get_runtime_summary(line 390) andtest_profile_model(line 418) both gate onnot profile.api_key.strip(). For ADC-based Vertex AI profiles this is always empty, so:
- The runtime summary incorrectly reports
using_mock=Truewith the "缺少 API Key" reason, misleading users.- The test endpoint returns
ok=Falsewith "请先填写 API Key 与模型名后再测试" before ever attempting a connection.Both checks need the same protocol-aware fix applied in
provider_factory.py:🐛 Proposed fix (same pattern for both occurrences)
- if not profile.api_key.strip() or not profile.model.strip(): + uses_api_key = profile.protocol != 'vertex-ai' + if (uses_api_key and not profile.api_key.strip()) or not profile.model.strip():Also applies to: 418-424
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@application/ai/llm_control_service.py` around lines 390 - 399, The runtime-summary and profile-test checks wrongly treat ADC-based Vertex AI profiles as missing API keys because they only test profile.api_key.strip(); update both get_runtime_summary and test_profile_model to be protocol-aware (same pattern used in provider_factory.py) so that when profile.protocol indicates Vertex/ADC (e.g., protocol == 'vertex' or provider_factory helper like supports_adc(profile.protocol)), you skip the api_key-empty gate and allow model/base_url validation and connection testing; apply this exact change to both get_runtime_summary and test_profile_model so Vertex ADC profiles do not get using_mock=True or premature "fill API Key" test failures.infrastructure/ai/provider_factory.py (1)
32-33:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winVertex AI (ADC) profiles always silently fall back to
MockProvider— the feature is effectively broken.The
api_keyguard at line 32 returnsMockProviderwheneverapi_keyis empty. For Vertex AI with ADC authentication,api_keyis intentionally blank — the profile is populated by theGCP_PROJECT_ID/GOOGLE_APPLICATION_CREDENTIALSdetection branch in_build_initial_config, which never setsapi_key. Meanwhile,VertexAIProvider.__init__constructsgenai.Client(vertexai=True, ...)using ADC and never readssettings.api_key. So every ADC-configured Vertex AI profile is permanently routed toMockProviderand no generation request ever reachesVertexAIProvider.The fix is to make the
api_keycheck protocol-aware:🐛 Proposed fix
- if not resolved.api_key.strip() or not resolved.model.strip(): - return MockProvider() + uses_api_key = resolved.protocol != 'vertex-ai' + if (uses_api_key and not resolved.api_key.strip()) or not resolved.model.strip(): + return MockProvider()The same pattern must also be fixed in
llm_control_service.py'sget_runtime_summary(line 390) andtest_profile_model(line 418), which will otherwise report "will use MockProvider" and refuse to run connection tests for ADC users.
🧹 Nitpick comments (9)
scripts/utils/download_model_via_modelscope.py (1)
12-15: Usereconfigure()as the idiomatic alternative.Python 3.7+ (this project uses Python 3.11) provides
sys.stdout.reconfigure(encoding='utf-8'), which mutates the existing wrapper in-place and is more concise than manually re-wrapping withio.TextIOWrapper.♻️ Proposed refactor
if sys.platform == 'win32': - import io - sys.stdout = io.TextIOWrapper(sys.stdout.buffer, encoding='utf-8') - sys.stderr = io.TextIOWrapper(sys.stderr.buffer, encoding='utf-8') + sys.stdout.reconfigure(encoding='utf-8') + sys.stderr.reconfigure(encoding='utf-8')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/utils/download_model_via_modelscope.py` around lines 12 - 15, Replace the manual re-wrapping of stdout/stderr in the Windows branch with the idiomatic reconfigure() call: locate the sys.platform == 'win32' block that currently imports io and reassigns sys.stdout/sys.stderr using io.TextIOWrapper, and instead call sys.stdout.reconfigure(encoding='utf-8') and sys.stderr.reconfigure(encoding='utf-8') to mutate the existing streams in-place (remove the io import and the TextIOWrapper reassignment).frontend/src/stores/novelStore.ts (2)
47-54: ⚡ Quick win
genreis always''; consider mappinglocked_genreor dropping the field.
NovelDTO.locked_genre(seefrontend/src/api/novel.tsline 86) carries the parsed genre, but Line 52 hardcodesgenre: ''. TheBookinterface exports this field as non-optionalstring, so any consumer that rendersbook.genrewill silently receive an empty string.♻️ Proposed fix
- genre: '', + genre: novel.locked_genre ?? '',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/stores/novelStore.ts` around lines 47 - 54, The mapping for books.value currently hardcodes genre: '' — update the books.value mapping in novelStore.ts to populate Book.genre from NovelDTO.locked_genre (e.g., set genre: novel.locked_genre || '') so consumers receive the parsed genre; alternatively, if you prefer to drop the field, remove Book.genre from the Book interface and omit it from the books.value mapping, but do not leave the non-optional Book.genre always empty.
76-76: ⚡ Quick winReplace
payload: anywith the typed parameter fromnovelApi.createNovel.
novelApi.createNovelalready declares a precise parameter shape (seefrontend/src/api/novel.tslines 115-128). Usinganyhere silently drops that safety at the store boundary.♻️ Proposed refactor
- const createBook = async (payload: any) => { + const createBook = async (payload: Parameters<typeof novelApi.createNovel>[0]) => {Or import and re-export the type explicitly if it's needed elsewhere.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/stores/novelStore.ts` at line 76, The createBook function currently accepts an untyped payload (payload: any); replace that with the exact parameter type from novelApi.createNovel by changing the signature to use Parameters<typeof novelApi.createNovel>[0] (or import the concrete request type from frontend/src/api/novel.ts and use it) so the store preserves the API's type safety; update any imports if necessary and adjust any call sites to satisfy the new typed parameter.frontend/package.json (1)
24-24: Migrate to@lucide/vueinstead of the deprecatedlucide-vue-next.
lucide-vue-nextv1.0.0 is the final published version under this package name. As of Lucide v1, the package was renamed to@lucide/vue, and all future updates (bug fixes, new icons, security patches) will only be released under the new name. The current^1.0.0range is effectively pinned to that exact version and will never receive updates.Note: The package does not appear to be imported anywhere in the codebase, so consider whether it should be removed entirely or if it's intended for future use.
♻️ Proposed fix: migrate to `@lucide/vue`
- "lucide-vue-next": "^1.0.0", + "@lucide/vue": "^1.0.0",If using lucide icons in component files, update imports:
-import { SomeIcon } from 'lucide-vue-next'; +import { SomeIcon } from '@lucide/vue';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/package.json` at line 24, Update the dependency entry for lucide in package.json by replacing the deprecated package name "lucide-vue-next" with the maintained scope package "@lucide/vue" (preserve or update the version specifier, e.g. to a caret version like "^1.x" or to the latest appropriate release), and then run your package manager to install; if the package is not imported anywhere (search for imports of "lucide-vue-next" or icon usages), remove the dependency entirely instead of migrating.infrastructure/ai/config/settings.py (1)
27-33: 💤 Low valueConsider aligning type annotations with the
Nonechecks.The fields
default_temperature: floatanddefault_max_tokens: intare not typed asOptional, yet the validation now guards againstNone. While this defensive check is harmless, the type annotations could be updated toOptional[float]andOptional[int]ifNoneis a valid state, or theis not Nonechecks could be removed if the default values guarantee non-null.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/ai/config/settings.py` around lines 27 - 33, The type annotations for the fields used in __post_init__ (default_temperature and default_max_tokens) are inconsistent with the None checks; either update their annotations to Optional[float] and Optional[int] (and add from typing import Optional) if None is a valid state, or remove the "is not None" guards in __post_init__ and ensure the dataclass provides non-null defaults so the fields remain plain float/int; pick one approach and make the corresponding change to the field declarations and imports, referencing default_temperature, default_max_tokens, and __post_init__.infrastructure/ai/providers/gemini_provider.py (1)
74-82: 💤 Low valueConsider catching a more specific exception type.
The broad
except Exceptioncould mask programming errors beyond JSON parsing issues (e.g.,AttributeErroriftarget_schemais misconfigured). Consider catchingpydantic.ValidationErrorspecifically.♻️ Proposed refinement
+from pydantic import BaseModel, ValidationError ... if target_schema and isinstance(target_schema, type) and issubclass(target_schema, BaseModel): try: validated_obj = target_schema.model_validate_json(content) content = validated_obj.model_dump_json() - except Exception as e: + except ValidationError as e: logger.warning(f"Gemini output Pydantic check failed (returning raw): {e}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/ai/providers/gemini_provider.py` around lines 74 - 82, The current broad except Exception around the Pydantic validation (for target_schema.model_validate_json and model_dump_json) can hide unrelated errors; change it to catch pydantic.ValidationError specifically (import ValidationError from pydantic) and use "except ValidationError as e:" to log the validation failure via logger.warning; if you still want to surface unexpected programming errors, add a separate generic except that re-raises or logs with logger.exception so AttributeError/ImportError/etc. aren't silently swallowed.application/ai/llm_control_service.py (1)
502-508: ⚡ Quick win
_normalize_base_urlmissingvertex-aibranch — falls through to OpenAI normalization.Vertex AI profiles have an empty
base_url. Ifnormalize_openai_base_url('')ever returns a non-empty default (e.g.,https://api.openai.com/v1),resolve_profilewould silently embed an incorrect URL in the resolved profile. Adding an explicit passthrough prevents this ambiguity.♻️ Proposed fix
if protocol == 'anthropic': return normalize_anthropic_base_url(base_url) or '' if protocol == 'gemini': return normalize_gemini_base_url(base_url) or '' + if protocol == 'vertex-ai': + return base_url # Vertex AI uses ADC; base_url may be empty or a custom endpoint return normalize_openai_base_url(base_url) or ''🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@application/ai/llm_control_service.py` around lines 502 - 508, _normalize_base_url incorrectly falls through to OpenAI normalization for vertex-ai profiles; add an explicit vertex-ai branch to passthrough the provided base_url. Update _normalize_base_url (the static method) to check if protocol == 'vertex-ai' and return base_url or '' directly, keeping the existing branches for 'anthropic', 'gemini' (normalize_anthropic_base_url, normalize_gemini_base_url) and the default OpenAI normalization (normalize_openai_base_url) unchanged.requirements-vertex.txt (1)
5-7: Removegoogle-cloud-aiplatformfrom requirements — it is completely unused.
vertex_ai_provider.pyonly imports fromgoogle.genai(provided bygoogle-genai). No references togoogle.cloud.aiplatformexist in the codebase. Sincegoogle-genaiis Google's recommended replacement for generative AI features, including the oldergoogle-cloud-aiplatformpackage adds unnecessary weight to the optional install without any benefit.♻️ Proposed cleanup
google-genai>=1.0.0 google-auth>=2.26.0 -google-cloud-aiplatform>=1.70.0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@requirements-vertex.txt` around lines 5 - 7, Remove the unnecessary dependency "google-cloud-aiplatform" from requirements-vertex.txt (leave google-genai and google-auth intact); confirm there are no imports referencing google.cloud.aiplatform (e.g., check vertex_ai_provider.py which uses google.genai only), then update any lock/CI dependency manifests if present and run the test/installation step to ensure no breakage.infrastructure/ai/providers/vertex_ai_provider.py (1)
10-10: Usetypes.HttpOptions()instead of plain dict for better type safety and IDE support.The
HttpOptionsimport at line 10 is unused; the code passeshttp_optionsas a plaindict. While the SDK internally converts plain dicts toHttpOptionsand the headers will work correctly, using the typedtypes.HttpOptions()is the recommended approach per SDK documentation, providing better validation, IDE autocompletion, and consistency with SDK examples.Suggested refactor
- from google.genai.types import HttpOptions + from google.genai import typesself.client = genai.Client( vertexai=True, project=self.project_id, location=self.region if self.region != "us-central1" else "global", - http_options={ - 'headers': {'x-goog-user-project': self.project_id} - } + http_options=types.HttpOptions( + headers={'x-goog-user-project': self.project_id} + ) )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/ai/providers/vertex_ai_provider.py` at line 10, The code currently constructs http_options as a plain dict and doesn't use the imported HttpOptions type; update the construction to instantiate and pass a typed HttpOptions object (from google.genai.types.HttpOptions) instead of a raw dict so you get IDE/type support and validation—locate where http_options is built and passed (the variable named http_options used when calling the Vertex AI client or request) and replace the dict with types.HttpOptions(...) setting the headers field (e.g., headers={"x-goog-user-project": ...}) accordingly, remove any redundant dict usage and ensure the existing HttpOptions import is used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/onboarding/NovelSetupGuide.vue`:
- Around line 1061-1065: handlePrev currently only decrements currentStep and
can re-trigger generation when stepping back into Step 2/3; update handlePrev to
first prevent/back out when a generation is in-flight or to cancel/invalidate
that generation: check the reactive flags generatingCharacters and
generatingLocations and if either is true either (a) return early (do not
decrement) or (b) call an existing cancelGeneration or set a new invalidation
flag (e.g., invalidateStep(stepKey) or set
generatingCharacters=false/generatingLocations=false plus a requestId/token to
ignore results) before decrementing currentStep; also disable the footer back
button when generatingCharacters or generatingLocations is true so users cannot
navigate back during generation.
In `@frontend/src/components/stats/StatsSidebar.vue`:
- Around line 139-148: The settings button (.action-settings) is orphaned in a
2-column grid; update the scoped CSS in StatsSidebar.vue to make that button
span both columns (e.g., target .actions-grid .action-settings or
.action-settings inside the component and set grid-column: 1 / -1 or
grid-column: span 2) so the fifth button becomes full-width and removes the
empty right cell.
In `@frontend/src/stores/novelStore.ts`:
- Around line 64-73: The catch is mis-attributing a stats refresh failure as a
deletion failure in deleteBook: after calling novelApi.deleteNovel and updating
books, move the await statsStore.loadGlobalStats(true) into its own try/catch
(or fire-and-forget) so any network error from statsStore.loadGlobalStats(true)
is logged or handled but not re-thrown; keep the original deleteBook rejection
path only for novelApi.deleteNovel failures and reference the functions/vars
novelApi.deleteNovel, books, deleteBook, and statsStore.loadGlobalStats when
making the change.
In `@infrastructure/ai/llm_client.py`:
- Line 4: The module uses Union[Prompt, str] (e.g., in the annotation on line
using Union at symbol: Union[Prompt, str]) but Union is not imported, causing a
NameError at module import; fix by adding Union to the typing imports on the top
import line (update "from typing import AsyncIterator, Optional, Any, Dict" to
include Union) so the annotation resolves eagerly, then run the linter/tests to
confirm the F821 is resolved.
In `@infrastructure/ai/providers/vertex_ai_provider.py`:
- Around line 88-92: The code dereferences response.usage_metadata without a
guard in the VertexAI provider (the block that builds token_usage using
TokenUsage(input_tokens=usage.prompt_token_count...,
output_tokens=usage.candidates_token_count...)), which will raise AttributeError
if usage_metadata is None; update the logic to safely handle a missing
usage_metadata by checking if response.usage_metadata is truthy (or use getattr
with defaults) and default input_tokens/output_tokens to 0 when absent before
constructing TokenUsage so TokenUsage always receives integers.
- Around line 43-53: The constructor currently calls
settings.extra_body.get('...') without guarding extra_body and ends up calling
.strip() on a None for project_id; update the init to safely read extra_body
(use self.settings.extra_body or {} as done in _build_genai_config) and provide
a default fallback for project_id (e.g., fallback to DEFAULT_PROJECT or empty
string) before calling .strip(); specifically, change the expressions that set
self.region and self.project_id so they first read from
(self.settings.extra_body or {}).get('region') / .get('project_id') and ensure
the project_id or-region chain ends with a non-None default (like DEFAULT_REGION
for region and a safe default for project_id) prior to calling .strip().
- Around line 140-144: The inline comment and handling for thinking_level are
incorrect: change the comment to say thinking_level is Gemini 3-only and
document that the SDK expects types.ThinkingLevel enum values (e.g.,
types.ThinkingLevel.HIGH) or lowercase strings like "low"/"medium"/"high"; do
not claim "Gemini 2.0+". Update the code that sets params["thinking_config"]
(the block using types.ThinkingConfig and eb["thinking_level"]) to first guard
by the active model version/name (only set thinking_config when the selected
model is a Gemini 3 series) and, for Gemini 2.5 models, instead prefer using
thinking_budget (an integer token count) or skip setting thinking_config to
avoid API errors. Ensure you reference types.ThinkingConfig,
eb["thinking_level"], and the active model variable used in this module when
adding the guard.
- Around line 56-70: The try/except around genai.Client in VertexAIProvider
currently swallows initialization errors so self.client is never set, causing
AttributeError later; either rethrow the error after logging or set a safe
sentinel and make _ensure_sdk validate the actual client. Update the init block
around genai.Client (reference: self.client) to on failure log the error and
raise a RuntimeError (or assign self.client = None), and modify _ensure_sdk to
check both HAS_GENAI and that self.client is truthy (and raise a clear
RuntimeError mentioning VertexAIProvider and ADC/credentials issues) so
generate() and stream_generate() fail with a helpful message instead of
AttributeError.
In `@infrastructure/persistence/database/schema.sql`:
- Line 615: The llm_profiles table's protocol CHECK constraint was expanded to
include 'vertex-ai' but existing databases keep the old constraint, causing
SQLITE_CONSTRAINT on INSERT; add a one-time migration that renames llm_profiles
(e.g., llm_profiles_v1), creates a new llm_profiles table with the updated
protocol CHECK including 'vertex-ai' (all other columns identical), copies data
from llm_profiles_v1 into the new table, then drops llm_profiles_v1; implement
this migration as a separate upgrade script (not in schema.sql) and ensure it
runs once during upgrade to preserve existing rows and enable saving Vertex AI
profiles.
In `@interfaces/api/v1/workbench/llm_control.py`:
- Around line 116-125: The environment variable assignments inside the
api_format == 'gemini' branch (setting HTTPX_HTTP2, HTTPS_PROXY, HTTP_PROXY) are
ineffective because the httpx.AsyncClient is created with trust_env=False (and
mirror code in GeminiProvider uses an explicit transport); remove those dead
environment-setting lines or replace them by wiring an explicit proxy transport
when creating the client. Specifically, edit the gemini branch around the
api_format == 'gemini' block to either delete the
os.environ["HTTPX_HTTP2"/"HTTPS_PROXY"/"HTTP_PROXY"] assignments, or
construct/pass an httpx proxy/transport (as done in GeminiProvider) when
instantiating httpx.AsyncClient so proxy settings are actually used rather than
ignored.
In `@scripts/utils/download_model_via_modelscope.py`:
- Around line 14-15: The re-wrapping of sys.stdout and sys.stderr with
io.TextIOWrapper disables line buffering and can delay incremental progress
prints; update the two reassignments (sys.stdout =
io.TextIOWrapper(sys.stdout.buffer, encoding='utf-8') and sys.stderr =
io.TextIOWrapper(sys.stderr.buffer, encoding='utf-8')) to pass
line_buffering=True so output is flushed per line (keep encoding='utf-8' as
before).
---
Outside diff comments:
In `@application/ai/llm_control_service.py`:
- Around line 390-399: The runtime-summary and profile-test checks wrongly treat
ADC-based Vertex AI profiles as missing API keys because they only test
profile.api_key.strip(); update both get_runtime_summary and test_profile_model
to be protocol-aware (same pattern used in provider_factory.py) so that when
profile.protocol indicates Vertex/ADC (e.g., protocol == 'vertex' or
provider_factory helper like supports_adc(profile.protocol)), you skip the
api_key-empty gate and allow model/base_url validation and connection testing;
apply this exact change to both get_runtime_summary and test_profile_model so
Vertex ADC profiles do not get using_mock=True or premature "fill API Key" test
failures.
In `@frontend/src/components/onboarding/NovelSetupGuide.vue`:
- Around line 260-313: The wizard was extended to six panels but state/controls
still assume five steps; update the step count and transitions so currentStep
can reach 6: adjust the progress header to render 6 n-steps (instead of 5),
change handleNext to increment currentStep while currentStep < 6 (so Next
advances from 5→6), change the Step 5 CTA behavior (it should call handleNext
instead of closing the modal) or ensure handleComplete only runs when on step 6,
and update detectWizardProgress to return/resume at step 6 (set
resumedFromStep.value = 6). Locate and modify the symbols currentStep,
handleNext, handleComplete (or the Step 5 CTA), the progress header n-steps
rendering, and detectWizardProgress to keep the state machine consistent.
In `@interfaces/api/v1/analyst/voice.py`:
- Around line 83-114: The route declares Optional but always returns a
VoiceFingerprintResponse; update get_voice_fingerprint to remove Optional:
change response_model=Optional[VoiceFingerprintResponse] to
response_model=VoiceFingerprintResponse and change the function return
annotation from -> Optional[VoiceFingerprintResponse] to ->
VoiceFingerprintResponse (ensure any imports/type hints are adjusted and keep
the existing default-empty VoiceFingerprintResponse return path).
---
Nitpick comments:
In `@application/ai/llm_control_service.py`:
- Around line 502-508: _normalize_base_url incorrectly falls through to OpenAI
normalization for vertex-ai profiles; add an explicit vertex-ai branch to
passthrough the provided base_url. Update _normalize_base_url (the static
method) to check if protocol == 'vertex-ai' and return base_url or '' directly,
keeping the existing branches for 'anthropic', 'gemini'
(normalize_anthropic_base_url, normalize_gemini_base_url) and the default OpenAI
normalization (normalize_openai_base_url) unchanged.
In `@frontend/package.json`:
- Line 24: Update the dependency entry for lucide in package.json by replacing
the deprecated package name "lucide-vue-next" with the maintained scope package
"@lucide/vue" (preserve or update the version specifier, e.g. to a caret version
like "^1.x" or to the latest appropriate release), and then run your package
manager to install; if the package is not imported anywhere (search for imports
of "lucide-vue-next" or icon usages), remove the dependency entirely instead of
migrating.
In `@frontend/src/stores/novelStore.ts`:
- Around line 47-54: The mapping for books.value currently hardcodes genre: '' —
update the books.value mapping in novelStore.ts to populate Book.genre from
NovelDTO.locked_genre (e.g., set genre: novel.locked_genre || '') so consumers
receive the parsed genre; alternatively, if you prefer to drop the field, remove
Book.genre from the Book interface and omit it from the books.value mapping, but
do not leave the non-optional Book.genre always empty.
- Line 76: The createBook function currently accepts an untyped payload
(payload: any); replace that with the exact parameter type from
novelApi.createNovel by changing the signature to use Parameters<typeof
novelApi.createNovel>[0] (or import the concrete request type from
frontend/src/api/novel.ts and use it) so the store preserves the API's type
safety; update any imports if necessary and adjust any call sites to satisfy the
new typed parameter.
In `@infrastructure/ai/config/settings.py`:
- Around line 27-33: The type annotations for the fields used in __post_init__
(default_temperature and default_max_tokens) are inconsistent with the None
checks; either update their annotations to Optional[float] and Optional[int]
(and add from typing import Optional) if None is a valid state, or remove the
"is not None" guards in __post_init__ and ensure the dataclass provides non-null
defaults so the fields remain plain float/int; pick one approach and make the
corresponding change to the field declarations and imports, referencing
default_temperature, default_max_tokens, and __post_init__.
In `@infrastructure/ai/providers/gemini_provider.py`:
- Around line 74-82: The current broad except Exception around the Pydantic
validation (for target_schema.model_validate_json and model_dump_json) can hide
unrelated errors; change it to catch pydantic.ValidationError specifically
(import ValidationError from pydantic) and use "except ValidationError as e:" to
log the validation failure via logger.warning; if you still want to surface
unexpected programming errors, add a separate generic except that re-raises or
logs with logger.exception so AttributeError/ImportError/etc. aren't silently
swallowed.
In `@infrastructure/ai/providers/vertex_ai_provider.py`:
- Line 10: The code currently constructs http_options as a plain dict and
doesn't use the imported HttpOptions type; update the construction to
instantiate and pass a typed HttpOptions object (from
google.genai.types.HttpOptions) instead of a raw dict so you get IDE/type
support and validation—locate where http_options is built and passed (the
variable named http_options used when calling the Vertex AI client or request)
and replace the dict with types.HttpOptions(...) setting the headers field
(e.g., headers={"x-goog-user-project": ...}) accordingly, remove any redundant
dict usage and ensure the existing HttpOptions import is used.
In `@requirements-vertex.txt`:
- Around line 5-7: Remove the unnecessary dependency "google-cloud-aiplatform"
from requirements-vertex.txt (leave google-genai and google-auth intact);
confirm there are no imports referencing google.cloud.aiplatform (e.g., check
vertex_ai_provider.py which uses google.genai only), then update any lock/CI
dependency manifests if present and run the test/installation step to ensure no
breakage.
In `@scripts/utils/download_model_via_modelscope.py`:
- Around line 12-15: Replace the manual re-wrapping of stdout/stderr in the
Windows branch with the idiomatic reconfigure() call: locate the sys.platform ==
'win32' block that currently imports io and reassigns sys.stdout/sys.stderr
using io.TextIOWrapper, and instead call
sys.stdout.reconfigure(encoding='utf-8') and
sys.stderr.reconfigure(encoding='utf-8') to mutate the existing streams in-place
(remove the io import and the TextIOWrapper reassignment).
🪄 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: abbf22e1-13aa-4693-a92d-4bab2153dafb
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (26)
.env.example.gitignoreapplication/ai/llm_control_service.pycli.pyfrontend/index.htmlfrontend/package.jsonfrontend/src/api/llmControl.tsfrontend/src/components/onboarding/NovelSetupGuide.vuefrontend/src/components/stats/StatsSidebar.vuefrontend/src/components/workbench/LLMControlPanel.vuefrontend/src/stores/novelStore.tsfrontend/src/views/Cast.vuefrontend/src/views/Home.vueinfrastructure/ai/config/settings.pyinfrastructure/ai/llm_client.pyinfrastructure/ai/provider_factory.pyinfrastructure/ai/providers/__init__.pyinfrastructure/ai/providers/gemini_provider.pyinfrastructure/ai/providers/vertex_ai_provider.pyinfrastructure/persistence/database/schema.sqlinterfaces/api/v1/analyst/voice.pyinterfaces/api/v1/workbench/llm_control.pyload_env.pyrequirements-vertex.txtrequirements.txtscripts/utils/download_model_via_modelscope.py
Comment on lines
+140
to
+144
| # 针对 Gemini 2.0+ 的思维配置 (Thinking) | ||
| if "thinking_level" in eb: | ||
| params["thinking_config"] = types.ThinkingConfig( | ||
| thinking_level=eb["thinking_level"] # e.g., "LOW", "MEDIUM", "HIGH" | ||
| ) |
Contributor
There was a problem hiding this comment.
Misleading comment: thinking_level is Gemini 3-only, not "Gemini 2.0+".
"No models (apart from Gemini 2.5/3 series) support thinking or thinking budgets APIs." The inline comment on Line 140 says "Gemini 2.0+" but thinking_level in ThinkingConfig is specific to Gemini 3 models; Gemini 2.5 models use thinking_budget (an integer token count) instead. If a user sets thinking_level in extra_body while the active model is the default gemini-1.5-flash (or any 2.x model), the API will return an error.
Additionally, the official Python SDK uses types.ThinkingLevel.HIGH enum values or lowercase string literals such as "low" — document this expectation in the comment alongside the uppercase examples.
📝 Suggested comment/guard update
- # 针对 Gemini 2.0+ 的思维配置 (Thinking)
+ # Thinking config: only valid for Gemini 3+ models (thinking_level)
+ # and Gemini 2.5 models (use thinking_budget instead).
+ # Accepted string values: "low" | "medium" | "high" (or types.ThinkingLevel enum).
if "thinking_level" in eb:
params["thinking_config"] = types.ThinkingConfig(
- thinking_level=eb["thinking_level"] # e.g., "LOW", "MEDIUM", "HIGH"
+ thinking_level=eb["thinking_level"] # e.g., "low", "medium", "high"
)📝 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
| # 针对 Gemini 2.0+ 的思维配置 (Thinking) | |
| if "thinking_level" in eb: | |
| params["thinking_config"] = types.ThinkingConfig( | |
| thinking_level=eb["thinking_level"] # e.g., "LOW", "MEDIUM", "HIGH" | |
| ) | |
| # Thinking config: only valid for Gemini 3+ models (thinking_level) | |
| # and Gemini 2.5 models (use thinking_budget instead). | |
| # Accepted string values: "low" | "medium" | "high" (or types.ThinkingLevel enum). | |
| if "thinking_level" in eb: | |
| params["thinking_config"] = types.ThinkingConfig( | |
| thinking_level=eb["thinking_level"] # e.g., "low", "medium", "high" | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@infrastructure/ai/providers/vertex_ai_provider.py` around lines 140 - 144,
The inline comment and handling for thinking_level are incorrect: change the
comment to say thinking_level is Gemini 3-only and document that the SDK expects
types.ThinkingLevel enum values (e.g., types.ThinkingLevel.HIGH) or lowercase
strings like "low"/"medium"/"high"; do not claim "Gemini 2.0+". Update the code
that sets params["thinking_config"] (the block using types.ThinkingConfig and
eb["thinking_level"]) to first guard by the active model version/name (only set
thinking_config when the selected model is a Gemini 3 series) and, for Gemini
2.5 models, instead prefer using thinking_budget (an integer token count) or
skip setting thinking_config to avoid API errors. Ensure you reference
types.ThinkingConfig, eb["thinking_level"], and the active model variable used
in this module when adding the guard.
| name TEXT NOT NULL DEFAULT '', | ||
| preset_key TEXT NOT NULL DEFAULT 'custom-openai-compatible', | ||
| protocol TEXT NOT NULL DEFAULT 'openai' CHECK(protocol IN ('openai', 'anthropic', 'gemini')), | ||
| protocol TEXT NOT NULL DEFAULT 'openai' CHECK(protocol IN ('openai', 'anthropic', 'gemini', 'vertex-ai')), |
Contributor
There was a problem hiding this comment.
Schema migration required — CREATE TABLE IF NOT EXISTS silently ignores the updated CHECK constraint on existing databases.
Any deployment whose llm_profiles table was created before this change retains the old constraint CHECK(protocol IN ('openai', 'anthropic', 'gemini')). Trying to INSERT a row with protocol='vertex-ai' will raise SQLITE_CONSTRAINT, so Vertex AI profiles can never be saved on upgraded deployments.
SQLite does not support ALTER TABLE … MODIFY COLUMN, so updating the constraint requires a table-rename migration, e.g.:
-- Migration: extend llm_profiles.protocol CHECK to include 'vertex-ai'
ALTER TABLE llm_profiles RENAME TO llm_profiles_v1;
CREATE TABLE IF NOT EXISTS llm_profiles (
id TEXT PRIMARY KEY,
name TEXT NOT NULL DEFAULT '',
preset_key TEXT NOT NULL DEFAULT 'custom-openai-compatible',
protocol TEXT NOT NULL DEFAULT 'openai' CHECK(protocol IN ('openai', 'anthropic', 'gemini', 'vertex-ai')),
-- ... (all remaining columns identical) ...
sort_order INTEGER NOT NULL DEFAULT 0,
created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP
);
INSERT INTO llm_profiles SELECT * FROM llm_profiles_v1;
DROP TABLE llm_profiles_v1;This migration script should be run once on upgrade and is separate from schema.sql.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@infrastructure/persistence/database/schema.sql` at line 615, The llm_profiles
table's protocol CHECK constraint was expanded to include 'vertex-ai' but
existing databases keep the old constraint, causing SQLITE_CONSTRAINT on INSERT;
add a one-time migration that renames llm_profiles (e.g., llm_profiles_v1),
creates a new llm_profiles table with the updated protocol CHECK including
'vertex-ai' (all other columns identical), copies data from llm_profiles_v1 into
the new table, then drops llm_profiles_v1; implement this migration as a
separate upgrade script (not in schema.sql) and ensure it runs once during
upgrade to preserve existing rows and enable saving Vertex AI profiles.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
infrastructure/ai/providers/vertex_ai_provider.py (2)
60-74:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winSwallowed init exception leaves
self.clientunset;_ensure_sdkdoesn't guard it — downstream calls crash withAttributeError.If
genai.Client(...)raises (bad ADC credentials, network error, etc.), theexcept Exceptionat line 72 logs and continues without settingself.client. Becauseself.client = Noneis never initialized before thetry, the attribute does not exist on the object._ensure_sdk()only checksHAS_GENAI(line 143), so the first call togenerate()orstream_generate()proceeds past the guard and fails withAttributeError: 'VertexAIProvider' object has no attribute 'client'— not a meaningful error for the caller.🐛 Proposed fix (option A — fail fast, preferred)
+ self.client = None try: self.client = genai.Client( vertexai=True, project=self.project_id, location=self.region if self.region != "us-central1" else "global", http_options={ 'headers': {'x-goog-user-project': self.project_id} } ) logger.info(f"Vertex AI (GenAI SDK) initialized: project={self.project_id}") - except Exception as e: + except Exception as e: # noqa: BLE001 logger.error(f"Failed to initialize GenAI Client: {e}") + raise RuntimeError(f"Vertex AI client initialization failed: {e}") from e🐛 Proposed fix (option B — soft failure with clear guard)
+ self.client = None try: self.client = genai.Client(...) ... - except Exception as e: + except Exception as e: # noqa: BLE001 logger.error(f"Failed to initialize GenAI Client: {e}") def _ensure_sdk(self): if not HAS_GENAI: raise ImportError("Please install 'google-genai' to use VertexAIProvider.") + if not getattr(self, 'client', None): + raise RuntimeError( + "Vertex AI client failed to initialize. Check logs for the root cause." + )Also applies to: 142-144
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/ai/providers/vertex_ai_provider.py` around lines 60 - 74, The constructor may leave self.client undefined if genai.Client(...) raises, causing AttributeError later; initialize self.client = None before the try in class VertexAIProvider and on except either re-raise the exception to fail fast or explicitly set self.client = None and log the error; then update _ensure_sdk() to check both HAS_GENAI and that self.client is not None (and raise a clear RuntimeError mentioning VertexAIProvider and misconfigured/failed GenAI init) so generate() and stream_generate() fail with a meaningful message instead of AttributeError.
43-57:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
settings.extra_bodyguard missing in__init__; emptyproject_idstill unvalidated.Two residual issues from the prior review:
extra_bodyguard: Lines 44 and 52 callsettings.extra_body.get(...)directly._build_genai_configalready applies the safeeb = self.settings.extra_body or {}pattern, but__init__does not. Ifprofile.extra_bodyisNone(e.g., nullable DB column not set), construction crashes withAttributeError.Empty
project_id: Theor ""at line 55 prevents theNone.strip()crash, but there is no earlyValueErrorwhenproject_idresolves to an empty string.genai.Client(project="", ...)will then either fail with an opaque SDK error or silently misconfigure the billing target.🐛 Proposed fix
- raw_region = ( - settings.extra_body.get('region') + eb = settings.extra_body or {} + raw_region = ( + eb.get('region') or os.getenv("GOOGLE_CLOUD_LOCATION") or os.getenv("GCP_REGION") or DEFAULT_REGION ) self.region = raw_region.strip() if raw_region else DEFAULT_REGION raw_project_id = ( - settings.extra_body.get('project_id') + eb.get('project_id') or os.getenv("GOOGLE_CLOUD_PROJECT") or os.getenv("GCP_PROJECT_ID") or "" ) self.project_id = raw_project_id.strip() + if not self.project_id: + raise ValueError( + "Vertex AI: project_id is required. Set 'project_id' in the profile " + "extra_body, or set GOOGLE_CLOUD_PROJECT / GCP_PROJECT_ID env var." + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/ai/providers/vertex_ai_provider.py` around lines 43 - 57, The constructor (__init__) accesses settings.extra_body without a null guard and allows an empty project_id; update __init__ to first normalize eb = self.settings.extra_body or {} and use eb.get('region') / eb.get('project_id') instead of directly calling settings.extra_body.get(...), trim the results into self.region and self.project_id as before, and add a validation after trimming that raises a ValueError (with a clear message) if self.project_id is empty so callers don't construct genai.Client with an empty project string; reference the __init__ method, settings.extra_body, self.region, self.project_id, and ensure behavior matches the _build_genai_config safe eb pattern.
🧹 Nitpick comments (4)
infrastructure/persistence/database/connection.py (1)
293-293: ⚡ Quick win避免在迁移里用
SELECT *复制表数据。
INSERT INTO llm_profiles SELECT * FROM llm_profiles_old依赖两张表的列数量和顺序完全一致，但这个 helper 本身就是给“旧 schema”准备的，后续只要llm_profiles再增一列，这里就会复制失败或把数据错位。建议改成显式列清单，或先用PRAGMA table_info取交集列再复制。一种更稳妥的实现思路
+ target_cols = [ + "id", "name", "preset_key", "protocol", "base_url", "api_key", "model", + "temperature", "max_tokens", "timeout_seconds", "extra_headers", + "extra_query", "extra_body", "notes", + "use_legacy_chat_completions", "sort_order", "created_at", "updated_at", + ] + source_cols = { + row[1] for row in conn.execute("PRAGMA table_info(llm_profiles_old)").fetchall() + } + shared_cols = [col for col in target_cols if col in source_cols] + cols_sql = ", ".join(shared_cols) + conn.execute( + f"INSERT INTO llm_profiles ({cols_sql}) " + f"SELECT {cols_sql} FROM llm_profiles_old" + ) - conn.execute("INSERT INTO llm_profiles SELECT * FROM llm_profiles_old")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/persistence/database/connection.py` at line 293, The migration currently uses conn.execute("INSERT INTO llm_profiles SELECT * FROM llm_profiles_old"), which breaks if column order/counts diverge; change it to explicitly copy only the matching columns between llm_profiles and llm_profiles_old by querying PRAGMA table_info for both tables, computing the intersection of column names, then executing an INSERT INTO llm_profiles (<col_list>) SELECT <col_list> FROM llm_profiles_old using that explicit column list (or hardcode the safe column list) so you never rely on SELECT *; reference the conn.execute call and the table names llm_profiles and llm_profiles_old when making this change.frontend/src/components/onboarding/NovelSetupGuide.vue (3)
30-32: 💤 Low valueError message appears in both title and body.
The alert now shows
bibleErrorin both the:titleattribute and the inner<div>(line 31). This creates duplicate display of the same error text. Consider either removing the title binding or the inner div content.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/onboarding/NovelSetupGuide.vue` around lines 30 - 32, The n-alert currently renders the same message twice by binding bibleError to both :title and the inner div; remove one of them to avoid duplication — either delete the :title="bibleError" attribute on the <n-alert> component or remove the inner <div class="wizard-error-text">{{ bibleError }}</div>, keeping the other so the error is shown only once; update any tests/styles that relied on the removed element if needed.
278-288: 💤 Low valueStep 6 panel appears unreachable.
The Step 6 panel exists but there's no navigation path to it:
handleNextonly incrementscurrentStepwhen< 5, stopping at step 5- At step 5, the "进入工作台" button calls
handleComplete()which closes the modalEither this is intentional placeholder code for future use, or the step indicator and navigation logic need adjustment.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/onboarding/NovelSetupGuide.vue` around lines 278 - 288, The Step 6 panel is unreachable because currentStep is only incremented while < 5 and the step-5 button calls handleComplete which closes the modal; update the navigation logic so the UI can actually reach step 6: modify the step advancement condition in handleNext (referencing currentStep and handleNext) to allow increment up to 6 (e.g., change the guard from currentStep < 5 to currentStep < 6 or adjust the comparison accordingly) or change the step-5 button’s handler (where handleComplete is invoked) to call handleNext instead so the flow proceeds to the panel for step 6; ensure any step-indicator/limits that reference max steps are consistent with the new max.
531-569:syncGenerationState()is defined but never called anywhere in the component.Remove this unused function or integrate it into the component's initialization logic if it was intended for a future feature.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/onboarding/NovelSetupGuide.vue` around lines 531 - 569, The function syncGenerationState is defined but never invoked; either remove it or call it during component initialization and when inputs change — specifically, invoke syncGenerationState() from the component's setup lifecycle (e.g., onMounted) and add watchers that call it when props.show or props.novelId change (and also call it after any generation actions that update bibleData); reference the syncGenerationState function name to locate it and ensure bibleData.value, bibleGenerated, charactersGenerated, locationsGenerated, generatingCharacters, generatingLocations and related worldbuildingApi/bibleApi logic remain consistent when wiring the calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/onboarding/NovelSetupGuide.vue`:
- Around line 260-276: The template references an undefined IconChart component
(used in the step panel rendering), so add a render-function definition named
IconChart alongside the other icon renderers (the same area where IconBook,
IconPeople, IconMap, IconTimeline, IconCheck are defined); implement IconChart
to return an SVG element (matching the pattern used by the other icons) with an
appropriate path for a chart/plot arc, or simply alias IconChart = IconTimeline
if that icon is acceptable—this will eliminate the runtime warning and render
the icon in the step panel.
In `@infrastructure/persistence/database/connection.py`:
- Around line 297-299: The migration except block that currently logs the error
and calls conn.rollback() is silently swallowing failures for the llm_profiles
schema; update the handler in connection.py so that after logger.error(...) and
conn.rollback() you either re-raise the caught exception (raise) or perform a
post-rollback validation that the CHECK constraint on llm_profiles.protocol was
actually updated (and raise if not), ensuring failures surface at startup
instead of later when application/ai/llm_control_service.py writes
protocol='vertex-ai'. Reference the existing identifiers logger.error,
conn.rollback, the llm_profiles schema/check constraint and the
protocol='vertex-ai' write path when implementing the change.
In `@interfaces/api/v1/workbench/llm_control.py`:
- Around line 128-129: The proxy troubleshooting text is incorrect for the
client configured with trust_env=True; update the RequestError handling message
that currently says environment proxies are disabled to instead state that the
HTTP client inherits system/environment proxy settings and advise checking
environment proxy variables and system-wide proxy configuration (e.g.,
HTTP_PROXY/HTTPS_PROXY) and unwinding/clearing them if needed; locate the async
client instantiation "async with httpx.AsyncClient(timeout=timeout,
trust_env=True) as client" and the corresponding RequestError except block and
replace the misleading guidance with this corrected troubleshooting text.
- Around line 117-118: The current construction of url =
f"{actual_base.rstrip('/')}/models?key={api_key}" can leak the Gemini API key in
HTTPException.detail strings; keep using url for the request but create a
safe_url variant (e.g., remove the ?key=... query or replace the value with
?key=[REDACTED]) and replace uses of url in all error
messages/HTTPException.detail outputs (the error paths around where url is
embedded) with safe_url so the real api_key from actual_base/api_key is never
echoed back to callers.
- Around line 116-119: The Gemini branch currently builds a correct request
(api_format == 'gemini' -> actual_base, url, headers) but the response is later
normalized by _normalize_model_items which expects data["data"]; update the
Gemini handling to detect responses from models.list and transform
data["models"] into the shape _normalize_model_items expects (map each item
using name -> id (strip "models/" prefix if needed) and displayName -> friendly
name, or default to name if displayName missing) before calling
_normalize_model_items so successful Gemini responses no longer return count=0;
locate the api_format == 'gemini' branch and the call sites of
_normalize_model_items to insert this conversion logic (referencing actual_base,
url, _normalize_model_items, and the Gemini response fields "models", "name",
"displayName").
---
Duplicate comments:
In `@infrastructure/ai/providers/vertex_ai_provider.py`:
- Around line 60-74: The constructor may leave self.client undefined if
genai.Client(...) raises, causing AttributeError later; initialize self.client =
None before the try in class VertexAIProvider and on except either re-raise the
exception to fail fast or explicitly set self.client = None and log the error;
then update _ensure_sdk() to check both HAS_GENAI and that self.client is not
None (and raise a clear RuntimeError mentioning VertexAIProvider and
misconfigured/failed GenAI init) so generate() and stream_generate() fail with a
meaningful message instead of AttributeError.
- Around line 43-57: The constructor (__init__) accesses settings.extra_body
without a null guard and allows an empty project_id; update __init__ to first
normalize eb = self.settings.extra_body or {} and use eb.get('region') /
eb.get('project_id') instead of directly calling settings.extra_body.get(...),
trim the results into self.region and self.project_id as before, and add a
validation after trimming that raises a ValueError (with a clear message) if
self.project_id is empty so callers don't construct genai.Client with an empty
project string; reference the __init__ method, settings.extra_body, self.region,
self.project_id, and ensure behavior matches the _build_genai_config safe eb
pattern.
---
Nitpick comments:
In `@frontend/src/components/onboarding/NovelSetupGuide.vue`:
- Around line 30-32: The n-alert currently renders the same message twice by
binding bibleError to both :title and the inner div; remove one of them to avoid
duplication — either delete the :title="bibleError" attribute on the <n-alert>
component or remove the inner <div class="wizard-error-text">{{ bibleError
}}</div>, keeping the other so the error is shown only once; update any
tests/styles that relied on the removed element if needed.
- Around line 278-288: The Step 6 panel is unreachable because currentStep is
only incremented while < 5 and the step-5 button calls handleComplete which
closes the modal; update the navigation logic so the UI can actually reach step
6: modify the step advancement condition in handleNext (referencing currentStep
and handleNext) to allow increment up to 6 (e.g., change the guard from
currentStep < 5 to currentStep < 6 or adjust the comparison accordingly) or
change the step-5 button’s handler (where handleComplete is invoked) to call
handleNext instead so the flow proceeds to the panel for step 6; ensure any
step-indicator/limits that reference max steps are consistent with the new max.
- Around line 531-569: The function syncGenerationState is defined but never
invoked; either remove it or call it during component initialization and when
inputs change — specifically, invoke syncGenerationState() from the component's
setup lifecycle (e.g., onMounted) and add watchers that call it when props.show
or props.novelId change (and also call it after any generation actions that
update bibleData); reference the syncGenerationState function name to locate it
and ensure bibleData.value, bibleGenerated, charactersGenerated,
locationsGenerated, generatingCharacters, generatingLocations and related
worldbuildingApi/bibleApi logic remain consistent when wiring the calls.
In `@infrastructure/persistence/database/connection.py`:
- Line 293: The migration currently uses conn.execute("INSERT INTO llm_profiles
SELECT * FROM llm_profiles_old"), which breaks if column order/counts diverge;
change it to explicitly copy only the matching columns between llm_profiles and
llm_profiles_old by querying PRAGMA table_info for both tables, computing the
intersection of column names, then executing an INSERT INTO llm_profiles
(<col_list>) SELECT <col_list> FROM llm_profiles_old using that explicit column
list (or hardcode the safe column list) so you never rely on SELECT *; reference
the conn.execute call and the table names llm_profiles and llm_profiles_old when
making this change.
🪄 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: 092ce778-92bc-4fad-9180-c987da25c614
📒 Files selected for processing (5)
frontend/src/components/onboarding/NovelSetupGuide.vueinfrastructure/ai/llm_client.pyinfrastructure/ai/providers/vertex_ai_provider.pyinfrastructure/persistence/database/connection.pyinterfaces/api/v1/workbench/llm_control.py
…d create onboarding setup guide component.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
interfaces/api/v1/workbench/llm_control.py (1)
103-126:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
vertex-aiprotocol not handled — misleading 400 error for ADC users.
vertex-aiis a validLLMProtocol(defined alongsideopenai,anthropic,gemini), andprovider_factory.pyalready has an explicit branch for it. However,list_modelshas novertex-aibranch. For users following the standard ADC path (no API key), the request is rejected at lines 104–106 with "API key is required to fetch model list" — which is misleading because Vertex AI doesn't use API keys.Add an early return specific to
vertex-aiso users get a clear, actionable message instead of a confusing 400:🐛 Proposed fix
api_format = (candidate.get('protocol') or '').strip().lower() api_key = (candidate.get('api_key') or '').strip() + if api_format == 'vertex-ai': + # Vertex AI authenticates via ADC and does not support model listing + # through this endpoint. Users should enter the model name manually. + raise HTTPException( + status_code=400, + detail='Vertex AI 使用 ADC 认证，不支持通过此接口列举模型，请直接填写模型名称（如 gemini-2.0-flash-001）。', + ) if not api_key: raise HTTPException(status_code=400, detail='API key is required to fetch model list')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interfaces/api/v1/workbench/llm_control.py` around lines 103 - 126, The code currently treats any candidate without api_key as an error before handling vertex-ai; update the logic in the list_models flow that reads api_format (variable api_format) so that if api_format == 'vertex-ai' you short-circuit before the api_key check and return a clear, actionable response (or raise an HTTPException with a message explaining Vertex AI uses ADC/credentials rather than an API key). Keep the existing branches for 'anthropic', 'gemini', and the OpenAI-compatible path intact (referencing api_format, api_key, base_url, and _openai_compatible_models_base) and ensure the vertex-ai branch does not require api_key so ADC users are not shown the misleading "API key is required to fetch model list" error.
♻️ Duplicate comments (1)
interfaces/api/v1/workbench/llm_control.py (1)
78-89:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGemini entry field mapping still broken after partial fix —
idis always empty.The change at line 79 correctly extracts
data.get('models', [])for Gemini responses, but the per-entry mapping at lines 85–88 still usesentry.get('id', '')for bothModelItem.idandModelItem.name. Gemini'smodels.listresponse identifies each model by itsnamefield (e.g.,"models/gemini-1.5-flash-001"), notid— so every normalizedModelItemwill haveid=''andname='', still yielding an effectively unusable model list despite the correct count.To fix this, the per-entry mapping must account for the Gemini schema:
🐛 Proposed fix
for entry in raw_list: if not isinstance(entry, dict): continue + raw_id = str(entry.get('id', '')) + raw_name = str(entry.get('name', '')) # Gemini: "models/gemini-1.5-flash-001" + raw_display = str(entry.get('display_name') or entry.get('displayName') or '') + effective_id = raw_id or raw_name # prefer OpenAI `id`, fall back to Gemini `name` + effective_name = raw_display or effective_id # prefer human-readable displayName items.append(ModelItem( - id=str(entry.get('id', '')), - name=str(entry.get('id', '')), # 多数网关不返回 name，回退到 id - owned_by=str(entry.get('owned_by', '')), + id=effective_id, + name=effective_name, + owned_by=str(entry.get('owned_by', '')), ))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interfaces/api/v1/workbench/llm_control.py` around lines 78 - 89, The per-entry mapping currently always uses entry.get('id') so Gemini responses (which use "name") produce empty ids; update the loop that builds ModelItem to set id = str(entry.get('name') or entry.get('id', '')) and name = str(entry.get('name') or entry.get('id', '')) (keep the existing owned_by mapping as-is), so ModelItem.id and ModelItem.name prefer the Gemini "name" field but fall back to "id".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@infrastructure/ai/providers/vertex_ai_provider.py`:
- Around line 99-111: After reading response.text into content, detect if
content is None or empty and handle the safety-blocked case explicitly: do not
pass an empty string into GenerationResult; instead raise or return a clear,
attributed error describing that Vertex AI safety filters suppressed all output
(include identifying info from response such as any id or usage_metadata when
available). Update the code around content/response usage (the variables
response, content, and usage_metadata and the GenerationResult construction) so
generate() returns a descriptive ValueError (or a GenerationResult with a clear
"safety_blocked" sentinel) rather than the opaque ValueError from
GenerationResult.__post_init__; also ensure TokenUsage is still constructed from
response.usage_metadata if present.
---
Outside diff comments:
In `@interfaces/api/v1/workbench/llm_control.py`:
- Around line 103-126: The code currently treats any candidate without api_key
as an error before handling vertex-ai; update the logic in the list_models flow
that reads api_format (variable api_format) so that if api_format == 'vertex-ai'
you short-circuit before the api_key check and return a clear, actionable
response (or raise an HTTPException with a message explaining Vertex AI uses
ADC/credentials rather than an API key). Keep the existing branches for
'anthropic', 'gemini', and the OpenAI-compatible path intact (referencing
api_format, api_key, base_url, and _openai_compatible_models_base) and ensure
the vertex-ai branch does not require api_key so ADC users are not shown the
misleading "API key is required to fetch model list" error.
---
Duplicate comments:
In `@interfaces/api/v1/workbench/llm_control.py`:
- Around line 78-89: The per-entry mapping currently always uses entry.get('id')
so Gemini responses (which use "name") produce empty ids; update the loop that
builds ModelItem to set id = str(entry.get('name') or entry.get('id', '')) and
name = str(entry.get('name') or entry.get('id', '')) (keep the existing owned_by
mapping as-is), so ModelItem.id and ModelItem.name prefer the Gemini "name"
field but fall back to "id".
🪄 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: a820078c-f33f-4d4b-843c-77e17b5e3fa0
📒 Files selected for processing (3)
frontend/src/components/onboarding/NovelSetupGuide.vueinfrastructure/ai/providers/vertex_ai_provider.pyinterfaces/api/v1/workbench/llm_control.py
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/onboarding/NovelSetupGuide.vue
… novel management UI components
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (3)
infrastructure/ai/providers/vertex_ai_provider.py (3)
98-115:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle empty model responses before constructing
GenerationResult.
response.text or ""still falls through toGenerationResult.__post_init__, which raises an opaque empty-content error if Vertex returns a blank / no-candidate response that is not flaggedSAFETY. Check for blank content here and raise a provider-specific error before building the result.🛠️ Proposed fix
content = response.text or "" + if not content.strip(): + finish_reason = candidate.finish_reason if candidate else "UNKNOWN" + raise ValueError( + f"Vertex AI returned empty content (finish_reason={finish_reason})." + ) # 提取 Token 使用量 usage = getattr(response, 'usage_metadata', None)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/ai/providers/vertex_ai_provider.py` around lines 98 - 115, The code currently sets content = response.text or "" but may still pass an empty string into GenerationResult which triggers an opaque __post_init__ error; before constructing GenerationResult (after reading response.candidates and response.text and before creating TokenUsage/GenerationResult), detect blank responses (e.g., candidate is None and not response.text, or response.text.strip() == ""), and raise a provider-specific error (ValueError or a custom ProviderError) with a clear message like "Vertex AI returned an empty response — no content or candidates" so callers see a meaningful error; keep using response.candidates, response.text, TokenUsage and GenerationResult names to locate the fix.
43-75:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard Vertex AI init configuration and fail fast on client setup errors.
settings.extra_bodyis read directly here, andproject_idcan still end up blank. That leaves the provider half-configured and pushes the failure to a later call site. Please use a safeextra_bodyfallback, reject emptyproject_idup front, and re-raisegenai.Client(...)init failures instead of only logging them.🛠️ Proposed fix
- raw_region = ( - settings.extra_body.get('region') + eb = settings.extra_body or {} + raw_region = ( + eb.get('region') or os.getenv("GOOGLE_CLOUD_LOCATION") or os.getenv("GCP_REGION") or DEFAULT_REGION ) self.region = raw_region.strip() if raw_region else DEFAULT_REGION raw_project_id = ( - settings.extra_body.get('project_id') + eb.get('project_id') or os.getenv("GOOGLE_CLOUD_PROJECT") or os.getenv("GCP_PROJECT_ID") or "" ) self.project_id = raw_project_id.strip() + if not self.project_id: + raise ValueError("Vertex AI requires project_id.") @@ except Exception as e: logger.error(f"Failed to initialize GenAI Client: {e}") + raise RuntimeError("Vertex AI client initialization failed") from e🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/ai/providers/vertex_ai_provider.py` around lines 43 - 75, Read settings.extra_body safely (e.g. settings.extra_body or {}) when extracting 'project_id' and ensure you validate the resulting self.project_id after .strip() is non-empty; if it's empty, raise a clear ValueError (or custom exception) immediately instead of allowing a blank project to be used. When initializing self.client via genai.Client (vertexai=True, project=..., location=..., http_options=...), do not swallow exceptions — log the error with context using logger.error and then re-raise the exception so the caller fails fast; keep references to settings.extra_body, self.project_id, genai.Client, self.client, and logger to locate and update the code.
86-90:⚠️ Potential issue | 🟠 Major | ⚡ Quick winOnly send
thinking_configfor supported Gemini models.This block injects
ThinkingConfigwheneverthinking_levelis present, even though the resolved model can still be an unsupported default likegemini-1.5-flash. That will turn an otherwise valid request into an API error. Thread the resolved model into_build_genai_configand gate this field by model family; for Gemini 2.5, preferthinking_budgetor omit the config.🛠️ Proposed fix
- gen_config = self._build_genai_config(config, prompt.system) + gen_config = self._build_genai_config(config, prompt.system, model_id) @@ - gen_config = self._build_genai_config(config, prompt.system) + gen_config = self._build_genai_config(config, prompt.system, model_id) @@ - def _build_genai_config(self, config: GenerationConfig, system_instruction: Optional[str]) -> types.GenerateContentConfig: + def _build_genai_config(self, config: GenerationConfig, system_instruction: Optional[str], model_id: str) -> types.GenerateContentConfig: @@ - if "thinking_level" in eb: + if "thinking_level" in eb and model_id.startswith("gemini-3"):Also applies to: 127-130, 163-183
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/ai/providers/vertex_ai_provider.py` around lines 86 - 90, Thread the resolved model into _build_genai_config and only attach a ThinkingConfig when the resolved model is a Gemini variant that actually supports it; change _build_genai_config to accept the resolved model (from _get_resolved_model) and conditionally construct ThinkingConfig (based on model family/version), using thinking_budget or omitting thinking_config for gemini-2.5/default unsupported models instead of always sending ThinkingConfig when thinking_level is present. Apply this same conditional logic wherever _build_genai_config is called (including the other occurrences noted) so ThinkingConfig is only included for supported Gemini models and requests to unsupported defaults (e.g., gemini-1.5-flash) are not mutated into API errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@infrastructure/ai/providers/vertex_ai_provider.py`:
- Around line 98-115: The code currently sets content = response.text or "" but
may still pass an empty string into GenerationResult which triggers an opaque
__post_init__ error; before constructing GenerationResult (after reading
response.candidates and response.text and before creating
TokenUsage/GenerationResult), detect blank responses (e.g., candidate is None
and not response.text, or response.text.strip() == ""), and raise a
provider-specific error (ValueError or a custom ProviderError) with a clear
message like "Vertex AI returned an empty response — no content or candidates"
so callers see a meaningful error; keep using response.candidates,
response.text, TokenUsage and GenerationResult names to locate the fix.
- Around line 43-75: Read settings.extra_body safely (e.g. settings.extra_body
or {}) when extracting 'project_id' and ensure you validate the resulting
self.project_id after .strip() is non-empty; if it's empty, raise a clear
ValueError (or custom exception) immediately instead of allowing a blank project
to be used. When initializing self.client via genai.Client (vertexai=True,
project=..., location=..., http_options=...), do not swallow exceptions — log
the error with context using logger.error and then re-raise the exception so the
caller fails fast; keep references to settings.extra_body, self.project_id,
genai.Client, self.client, and logger to locate and update the code.
- Around line 86-90: Thread the resolved model into _build_genai_config and only
attach a ThinkingConfig when the resolved model is a Gemini variant that
actually supports it; change _build_genai_config to accept the resolved model
(from _get_resolved_model) and conditionally construct ThinkingConfig (based on
model family/version), using thinking_budget or omitting thinking_config for
gemini-2.5/default unsupported models instead of always sending ThinkingConfig
when thinking_level is present. Apply this same conditional logic wherever
_build_genai_config is called (including the other occurrences noted) so
ThinkingConfig is only included for supported Gemini models and requests to
unsupported defaults (e.g., gemini-1.5-flash) are not mutated into API errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d8851b98-6903-443c-9f10-0de0a0059bc1
📒 Files selected for processing (5)
frontend/src/components/onboarding/NovelSetupGuide.vuefrontend/src/components/stats/StatsSidebar.vuefrontend/src/stores/novelStore.tsinfrastructure/ai/providers/vertex_ai_provider.pyinfrastructure/persistence/database/connection.py
✅ Files skipped from review due to trivial changes (1)
- frontend/src/stores/novelStore.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/components/stats/StatsSidebar.vue
- frontend/src/components/onboarding/NovelSetupGuide.vue
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
infrastructure/ai/providers/vertex_ai_provider.py (1)
43-58:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
project_idearly (empty string should fail fast).
self.project_idis allowed to become""(fallback chain ends withor "", then.strip()), and thengenai.Client(... project=self.project_id ...)is attempted. That can shift failures to request-time with confusing auth/404 errors.Add an explicit check in
__init__after computingself.project_idso the user gets a clear “project_id required” message.Suggested fix
@@ raw_project_id = ( settings.extra_body.get('project_id') or os.getenv("GOOGLE_CLOUD_PROJECT") or os.getenv("GCP_PROJECT_ID") or "" ) self.project_id = raw_project_id.strip() + + if not self.project_id: + raise ValueError( + "Vertex AI: project_id is required. Set it in profile extra_body['project_id'], " + "or via GOOGLE_CLOUD_PROJECT / GCP_PROJECT_ID." + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/ai/providers/vertex_ai_provider.py` around lines 43 - 58, After computing raw_project_id and assigning self.project_id (from raw_project_id.strip()), add a fast-fail validation that raises a clear error when project_id is empty; e.g., after self.project_id = raw_project_id.strip() check if not self.project_id and raise ValueError("project_id is required for Vertex AI provider") (or a custom/configured exception) so code that later constructs genai.Client(...) fails early with a clear message; reference raw_project_id and self.project_id in the check inside the class __init__ that builds the provider from settings.extra_body.
🧹 Nitpick comments (5)
infrastructure/ai/providers/vertex_ai_provider.py (5)
24-34: ⚡ Quick winRuff FULLWIDTH punctuation warnings: normalize to ASCII punctuation.
Static analysis hints show RUF002/RUF003 “ambiguous FULLWIDTH punctuation” warnings in docstrings/comments (e.g.,
：，（）). If CI treats Ruff warnings as failures, this will block merges.Please replace FULLWIDTH punctuation with ASCII equivalents in affected docstrings/comments.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/ai/providers/vertex_ai_provider.py` around lines 24 - 34, Replace FULLWIDTH punctuation in the module/class docstrings and comments (e.g., the docstring on VertexAIProvider) with ASCII equivalents to silence Ruff RUF002/RUF003 warnings: update characters like `：` `，` `（` `）` to `:` `,` `(` `)` in the docstring and any nearby comments referencing DEFAULT_MODEL or DEFAULT_REGION so the strings and comments use standard ASCII punctuation.
61-75: ⚡ Quick winPreserve the underlying init failure cause (don’t lose actionable context).
You swallow exceptions in
genai.Client(...)and justlogger.error(...)then return, relying on_ensure_sdk()later. While_ensure_sdk()will preventAttributeError, the later error message (“client not ready…”) won’t include the original exception details.At minimum, store
self._init_error = eand include it in_ensure_sdk()’s RuntimeError message (or re-raise during init).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/ai/providers/vertex_ai_provider.py` around lines 61 - 75, The except block swallowing the genai.Client init loses the original error context; capture and preserve it by assigning self._init_error = e inside the except before logging (and keep the logger.error), and then update _ensure_sdk() to include self._init_error in the raised RuntimeError (or re-raise the original exception from __init__); reference genai.Client initialization, self.client, self._init_error, and _ensure_sdk() so reviewers can locate and surface the underlying init failure in later errors.
197-209: ⚖️ Poor tradeoffVerify
types.SafetySetting.threshold="OFF"is the correct value for this SDK.The code sets
threshold="OFF"with a comment implying it maps to “BLOCK_NONE”. If the google-genai SDK expects a different enum/string (e.g.,BLOCK_NONE), this could either:
- raise a client-side/validation error, or
- silently not apply the intended safety configuration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/ai/providers/vertex_ai_provider.py` around lines 197 - 209, The _build_safety_settings function currently sets types.SafetySetting(threshold="OFF"), which may be the wrong token for the google-genai SDK; check the SDK docs or the types.SafetySetting enum to determine the correct value (for example BLOCK_NONE or types.SafetySetting.Threshold.BLOCK_NONE) and replace the literal "OFF" with the proper enum/constant or string expected by the SDK so the safety settings are applied/validated correctly; update the threshold in the list comprehension inside _build_safety_settings accordingly.
7-14: 💤 Low valueBlind exception catch (
except Exception) may be flagged by linting (BLE001).Ruff static hints indicate BLE001 for “Do not catch blind exception:
Exception”. Since this try/except is specifically for SDK initialization, consider either:
- catching a narrower exception type, or
- explicitly documenting why
Exceptionis acceptable (Ruff noqa comment on that line).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/ai/providers/vertex_ai_provider.py` around lines 7 - 14, The import block currently uses a broad exception handler; narrow it to catch ImportError (e.g., replace "except Exception" with "except ImportError") when importing google.genai (symbols: genai, types, HttpOptions) and set HAS_GENAI accordingly, or if you intentionally need to catch all exceptions, add an explicit Ruff noqa with a brief justification (e.g., "# noqa: BLE001 # allow broad catch during optional SDK init") next to the except so HAS_GENAI and the import logic remain correct and linters stop flagging the blind exception.
177-189: ⚖️ Poor tradeoffVerify accepted type/value for
thinking_levelintypes.ThinkingConfig.You convert
thinking_leveltostr(...).lower()and pass it intotypes.ThinkingConfig(thinking_level=level, ...). Depending on google-genai’s schema, it may require:
- a specific enum value (e.g.,
types.ThinkingLevel.HIGH), or- exact lowercase literals (e.g.,
"high"|"medium"|"low").This is worth verifying to avoid runtime API errors when users set
extra_body.thinking_level.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/ai/providers/vertex_ai_provider.py` around lines 177 - 189, The code coerces eb["thinking_level"] to a lowercase string and passes it into types.ThinkingConfig as thinking_level which may expect a specific enum member rather than a free-form string; update the block that builds params["thinking_config"] (referencing eb, level, budget, and types.ThinkingConfig) to validate/normalize the input: if types exposes an enum (e.g., types.ThinkingLevel), map the lowercased string to the corresponding enum member (e.g., {"low": types.ThinkingLevel.LOW, "medium": types.ThinkingLevel.MEDIUM, "high": types.ThinkingLevel.HIGH}); otherwise, ensure the exact literal format required by the API is used (or raise a clear ValueError for invalid values) and only pass thinking_level when budget is not provided.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@infrastructure/ai/providers/vertex_ai_provider.py`:
- Around line 76-116: In generate (async def generate) you must guard against a
missing/empty response.text and missing candidate before creating the
GenerationResult; currently you only raise when candidate.finish_reason ==
"SAFETY" but then do content = response.text or "" which can later fail
validation. Change the logic after retrieving response and candidate to: if
response.text is None or (not candidate and not response.text): raise a
descriptive ValueError (e.g., "Vertex AI returned no content") so we fail fast
and clearly; still compute token_usage from response.usage_metadata (TokenUsage)
and only construct and return GenerationResult when content is present.
---
Duplicate comments:
In `@infrastructure/ai/providers/vertex_ai_provider.py`:
- Around line 43-58: After computing raw_project_id and assigning
self.project_id (from raw_project_id.strip()), add a fast-fail validation that
raises a clear error when project_id is empty; e.g., after self.project_id =
raw_project_id.strip() check if not self.project_id and raise
ValueError("project_id is required for Vertex AI provider") (or a
custom/configured exception) so code that later constructs genai.Client(...)
fails early with a clear message; reference raw_project_id and self.project_id
in the check inside the class __init__ that builds the provider from
settings.extra_body.
---
Nitpick comments:
In `@infrastructure/ai/providers/vertex_ai_provider.py`:
- Around line 24-34: Replace FULLWIDTH punctuation in the module/class
docstrings and comments (e.g., the docstring on VertexAIProvider) with ASCII
equivalents to silence Ruff RUF002/RUF003 warnings: update characters like `：`
`，` `（` `）` to `:` `,` `(` `)` in the docstring and any nearby comments
referencing DEFAULT_MODEL or DEFAULT_REGION so the strings and comments use
standard ASCII punctuation.
- Around line 61-75: The except block swallowing the genai.Client init loses the
original error context; capture and preserve it by assigning self._init_error =
e inside the except before logging (and keep the logger.error), and then update
_ensure_sdk() to include self._init_error in the raised RuntimeError (or
re-raise the original exception from __init__); reference genai.Client
initialization, self.client, self._init_error, and _ensure_sdk() so reviewers
can locate and surface the underlying init failure in later errors.
- Around line 197-209: The _build_safety_settings function currently sets
types.SafetySetting(threshold="OFF"), which may be the wrong token for the
google-genai SDK; check the SDK docs or the types.SafetySetting enum to
determine the correct value (for example BLOCK_NONE or
types.SafetySetting.Threshold.BLOCK_NONE) and replace the literal "OFF" with the
proper enum/constant or string expected by the SDK so the safety settings are
applied/validated correctly; update the threshold in the list comprehension
inside _build_safety_settings accordingly.
- Around line 7-14: The import block currently uses a broad exception handler;
narrow it to catch ImportError (e.g., replace "except Exception" with "except
ImportError") when importing google.genai (symbols: genai, types, HttpOptions)
and set HAS_GENAI accordingly, or if you intentionally need to catch all
exceptions, add an explicit Ruff noqa with a brief justification (e.g., "# noqa:
BLE001 # allow broad catch during optional SDK init") next to the except so
HAS_GENAI and the import logic remain correct and linters stop flagging the
blind exception.
- Around line 177-189: The code coerces eb["thinking_level"] to a lowercase
string and passes it into types.ThinkingConfig as thinking_level which may
expect a specific enum member rather than a free-form string; update the block
that builds params["thinking_config"] (referencing eb, level, budget, and
types.ThinkingConfig) to validate/normalize the input: if types exposes an enum
(e.g., types.ThinkingLevel), map the lowercased string to the corresponding enum
member (e.g., {"low": types.ThinkingLevel.LOW, "medium":
types.ThinkingLevel.MEDIUM, "high": types.ThinkingLevel.HIGH}); otherwise,
ensure the exact literal format required by the API is used (or raise a clear
ValueError for invalid values) and only pass thinking_level when budget is not
provided.
🪄 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: e10f7a7c-45b7-490c-bf43-c02e9933ec9d
📒 Files selected for processing (2)
infrastructure/ai/providers/vertex_ai_provider.pyscripts/utils/download_model_via_modelscope.py
Comment on lines
+76
to
+116
| async def generate(self, prompt: Prompt, config: GenerationConfig) -> GenerationResult: | ||
| """执行单次文本生成任务。 | ||
|
|
||
| Args: | ||
| prompt: 包含系统指令和用户输入的 Prompt 对象。 | ||
| config: 包含模型 ID、温度等参数的生成配置。 | ||
|
|
||
| Returns: | ||
| GenerationResult: 包含生成文本及 Token 使用量元数据的结果。 | ||
| """ | ||
| self._ensure_sdk() | ||
| model_id = self._get_resolved_model(config) | ||
|
|
||
| gen_config = self._build_genai_config(config, prompt.system) | ||
|
|
||
| # 使用 aio 异步客户端 | ||
| response = await self.client.aio.models.generate_content( | ||
| model=model_id, | ||
| contents=prompt.user, | ||
| config=gen_config | ||
| ) | ||
|
|
||
| # 提取内容 (CodeRabbit: 处理安全拦截，防止 response.text 为空时崩溃) | ||
| candidate = response.candidates[0] if response.candidates else None | ||
| if candidate and candidate.finish_reason == "SAFETY": | ||
| raise ValueError("内容生成被 Vertex AI 安全过滤器拦截。请尝试修改输入或降低敏感度。") | ||
|
|
||
| content = response.text or "" | ||
|
|
||
| # 提取 Token 使用量 | ||
| usage = getattr(response, 'usage_metadata', None) | ||
| if usage: | ||
| token_usage = TokenUsage( | ||
| input_tokens=usage.prompt_token_count or 0, | ||
| output_tokens=usage.candidates_token_count or 0 | ||
| ) | ||
| else: | ||
| token_usage = TokenUsage(input_tokens=0, output_tokens=0) | ||
|
|
||
| return GenerationResult(content=content, token_usage=token_usage) | ||
|
|
Contributor
There was a problem hiding this comment.
Guard against empty response.text before GenerationResult (safety-blocked case).
You already handle finish_reason == "SAFETY" when response.candidates[0] exists, but you still do content = response.text or "" and then always construct GenerationResult(content=content, ...). If Vertex AI returns response.text=None (or empty) without finish_reason=="SAFETY" (e.g., candidates missing/partial response), this will likely trip GenerationResult’s validation and produce an opaque error.
Suggested fix
@@
- content = response.text or ""
+ content = response.text or ""
+ if not content.strip():
+ finish_reason = (
+ candidate.finish_reason
+ if candidate and getattr(candidate, "finish_reason", None)
+ else "UNKNOWN"
+ )
+ raise ValueError(
+ "Vertex AI returned empty content. "
+ f"(finish_reason={finish_reason}) "
+ "This may be caused by safety filters or blocked output."
+ )
@@
return GenerationResult(content=content, token_usage=token_usage)🧰 Tools
🪛 Ruff (0.15.12)
[warning] 98-98: Comment contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@infrastructure/ai/providers/vertex_ai_provider.py` around lines 76 - 116, In
generate (async def generate) you must guard against a missing/empty
response.text and missing candidate before creating the GenerationResult;
currently you only raise when candidate.finish_reason == "SAFETY" but then do
content = response.text or "" which can later fail validation. Change the logic
after retrieving response and candidate to: if response.text is None or (not
candidate and not response.text): raise a descriptive ValueError (e.g., "Vertex
AI returned no content") so we fail fast and clearly; still compute token_usage
from response.usage_metadata (TokenUsage) and only construct and return
GenerationResult when content is present.
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.
变更类型
feat新功能fixBug 修复chore构建/工具链变更说明
架构影响
application/infrastructure/interfaces/frontend测试
后端测试：已通过自定义脚本验证 VertexAIProvider 在 Enterprise 模式下的流式生成逻辑，成功连接
global端点。前端测试：已在本地通过
npm run dev验证向导页在亮色/暗色模式下的视觉表现，弹窗交互功能正常。启动验证：本地后端与前端服务均可正常启动并完成小说创建流程。
本地后端启动正常
本地前端启动正常
风险说明
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores