egullickson/motovaultpro
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: Migrate Gemini SDK to google-genai and enable Google Search grounding for VIN decode #231

New Issue
Closed
opened 2026-02-20 14:54:32 +00:00 by egullickson · 6 comments
egullickson commented 2026-02-20 14:54:32 +00:00
Owner
Copy Link
Copy Source

Context

The OCR service uses the deprecated vertexai.generative_models SDK from google-cloud-aiplatform for Gemini VIN decode and maintenance extraction. This SDK will be removed June 2026. The replacement google-genai package has first-class support for Google Search grounding, which lets Gemini cross-reference real vehicle data during VIN decode -- improving accuracy for make, model, trim, engine, and transmission fields.

The staging VIN decode for 1G1YE2D32P5602473 returned wrong data (1993 Corvette instead of 2023). The year is now fixed deterministically, but Google Search grounding will further improve field accuracy by giving Gemini access to real vehicle specifications.

Acceptance Criteria

  • OCR service migrated from google-cloud-aiplatform / vertexai.generative_models to google-genai
  • VIN decode calls Gemini with Tool(google_search=GoogleSearch()) enabled
  • Maintenance extraction and receipt extraction migrated to new SDK (no search grounding)
  • All existing OCR tests updated and passing
  • Docker build succeeds with new dependency
  • VIN decode returns correct data on staging (2023 Corvette for 1G1YE2D32P5602473)

Plan

1. Update dependency (ocr/requirements.txt)

Replace google-cloud-aiplatform>=1.40.0 with google-genai>=1.0.0

2. Migrate GeminiEngine (ocr/app/engines/gemini_engine.py)

  • Rename self._model to self._client, remove self._generation_config
  • Rewrite _get_model() as _get_client() using genai.Client(vertexai=True, project, location)
  • Rewrite extract_maintenance() to use client.models.generate_content() with GenerateContentConfig
  • Rewrite decode_vin() with Google Search: tools=[Tool(google_search=GoogleSearch())]
  • Part.from_data() becomes Part.from_bytes()

3. Migrate MaintenanceReceiptExtractor (ocr/app/extractors/maintenance_receipt_extractor.py)

Same SDK migration pattern as GeminiEngine (no Google Search for receipts)

4. Update tests (ocr/tests/test_gemini_engine.py)

  • Replace engine._model / mock_model.generate_content mocks with engine._client / mock_client.models.generate_content
  • Remove engine._generation_config mocks
  • Update sys.modules patches from vertexai to google.genai
  • test_vin_decode.py and test_manual_extractor.py mock at public API level -- no changes needed

API Mapping

Old (vertexai.generative_models) New (google.genai)
aiplatform.init(project, location) genai.Client(vertexai=True, project, location)
GenerativeModel(model_name) Client handles model per-call
model.generate_content([...], generation_config=...) client.models.generate_content(model=name, contents=[...], config=...)
GenerationConfig(...) GenerateContentConfig(...)
Part.from_data(data, mime_type) Part.from_bytes(data, mime_type)
N/A Tool(google_search=GoogleSearch())

Files to Modify

File Change
ocr/requirements.txt Swap dependency
ocr/app/engines/gemini_engine.py SDK migration + Google Search grounding
ocr/app/extractors/maintenance_receipt_extractor.py SDK migration
ocr/tests/test_gemini_engine.py Update mock patterns

Verification

  1. Docker build: docker build -t mvp-ocr-test ./ocr
  2. Run OCR tests in container
  3. Deploy to staging and decode VIN 1G1YE2D32P5602473
  4. Check logs for correct year resolution and improved field accuracy
## Context The OCR service uses the deprecated `vertexai.generative_models` SDK from `google-cloud-aiplatform` for Gemini VIN decode and maintenance extraction. This SDK will be removed June 2026. The replacement `google-genai` package has first-class support for Google Search grounding, which lets Gemini cross-reference real vehicle data during VIN decode -- improving accuracy for make, model, trim, engine, and transmission fields. The staging VIN decode for `1G1YE2D32P5602473` returned wrong data (1993 Corvette instead of 2023). The year is now fixed deterministically, but Google Search grounding will further improve field accuracy by giving Gemini access to real vehicle specifications. ## Acceptance Criteria - [ ] OCR service migrated from `google-cloud-aiplatform` / `vertexai.generative_models` to `google-genai` - [ ] VIN decode calls Gemini with `Tool(google_search=GoogleSearch())` enabled - [ ] Maintenance extraction and receipt extraction migrated to new SDK (no search grounding) - [ ] All existing OCR tests updated and passing - [ ] Docker build succeeds with new dependency - [ ] VIN decode returns correct data on staging (2023 Corvette for `1G1YE2D32P5602473`) ## Plan ### 1. Update dependency (`ocr/requirements.txt`) Replace `google-cloud-aiplatform>=1.40.0` with `google-genai>=1.0.0` ### 2. Migrate `GeminiEngine` (`ocr/app/engines/gemini_engine.py`) - Rename `self._model` to `self._client`, remove `self._generation_config` - Rewrite `_get_model()` as `_get_client()` using `genai.Client(vertexai=True, project, location)` - Rewrite `extract_maintenance()` to use `client.models.generate_content()` with `GenerateContentConfig` - Rewrite `decode_vin()` with Google Search: `tools=[Tool(google_search=GoogleSearch())]` - `Part.from_data()` becomes `Part.from_bytes()` ### 3. Migrate `MaintenanceReceiptExtractor` (`ocr/app/extractors/maintenance_receipt_extractor.py`) Same SDK migration pattern as GeminiEngine (no Google Search for receipts) ### 4. Update tests (`ocr/tests/test_gemini_engine.py`) - Replace `engine._model` / `mock_model.generate_content` mocks with `engine._client` / `mock_client.models.generate_content` - Remove `engine._generation_config` mocks - Update `sys.modules` patches from `vertexai` to `google.genai` - `test_vin_decode.py` and `test_manual_extractor.py` mock at public API level -- no changes needed ### API Mapping | Old (`vertexai.generative_models`) | New (`google.genai`) | |-------------------------------------|----------------------| | `aiplatform.init(project, location)` | `genai.Client(vertexai=True, project, location)` | | `GenerativeModel(model_name)` | Client handles model per-call | | `model.generate_content([...], generation_config=...)` | `client.models.generate_content(model=name, contents=[...], config=...)` | | `GenerationConfig(...)` | `GenerateContentConfig(...)` | | `Part.from_data(data, mime_type)` | `Part.from_bytes(data, mime_type)` | | N/A | `Tool(google_search=GoogleSearch())` | ## Files to Modify | File | Change | |------|--------| | `ocr/requirements.txt` | Swap dependency | | `ocr/app/engines/gemini_engine.py` | SDK migration + Google Search grounding | | `ocr/app/extractors/maintenance_receipt_extractor.py` | SDK migration | | `ocr/tests/test_gemini_engine.py` | Update mock patterns | ## Verification 1. Docker build: `docker build -t mvp-ocr-test ./ocr` 2. Run OCR tests in container 3. Deploy to staging and decode VIN `1G1YE2D32P5602473` 4. Check logs for correct year resolution and improved field accuracy
egullickson added the
status
in-progress
type
feature
 labels 2026-02-20 15:05:17 +00:00
egullickson added this to the Sprint 2026-02-02 milestone 2026-02-20 15:05:18 +00:00
egullickson commented 2026-02-20 15:11:53 +00:00
Author
Owner
Copy Link
Copy Source

Plan: Migrate Gemini SDK to google-genai (#231)

Phase: Planning | Agent: Orchestrator | Status: APPROVED
Revision: v4 -- addresses QR plan-completeness + TW plan-scrub + QR plan-code + QR plan-docs

Context

The OCR service uses the deprecated vertexai.generative_models SDK from google-cloud-aiplatform in two files (gemini_engine.py, maintenance_receipt_extractor.py). This SDK is EOL June 2026. The replacement google-genai package provides a client-per-call pattern and first-class Google Search grounding for VIN decode accuracy.

Milestone Index

Milestone Sub-Issue File(s) Summary
M1 #232 ocr/requirements.txt Swap dependency to google-genai>=1.0.0
M2 #233 ocr/app/engines/gemini_engine.py, ocr/app/config.py Full SDK migration + Google Search grounding for VIN decode
M3 #234 ocr/app/extractors/maintenance_receipt_extractor.py SDK migration (no search grounding) + fix RuntimeError bug
M4 #235 ocr/tests/test_gemini_engine.py Update mock patterns for new SDK
M5 (parent) ocr/CLAUDE.md, ocr/app/engines/CLAUDE.md, ocr/app/CLAUDE.md Doc-sync on affected directories

Each sub-issue contains its own self-contained plan with duplicated shared context (API migration map, internal state changes, authentication, error handling, risk assessment, review findings). An agent can execute from the sub-issue alone.

M5: Doc-sync (no sub-issue)

  • ocr/CLAUDE.md: update references from "Vertex AI SDK" to "google-genai SDK"
  • ocr/app/engines/CLAUDE.md: update BOTH line 6 (prose: "via Vertex AI") AND line 18 (table: "Vertex AI SDK") to reference google-genai
  • ocr/app/CLAUDE.md: update line 10 (config.py table entry: "Vertex AI" -> "Google GenAI")
  • Run doc-sync skill on affected directories

Verification

  1. docker build -t mvp-ocr-test ./ocr -- confirms new dependency resolves and Docker image builds
  2. Run OCR tests in container: docker run --rm mvp-ocr-test pytest -- confirms all unit tests pass
  3. Deploy to staging and decode VIN 1G1YE2D32P5602473 -- confirms correct 2023 Corvette data
  4. Check staging logs for Google Search grounding metadata in VIN decode responses

Pre-requisite

Commit untracked ocr/tests/test_resolve_vin_year.py before creating the feature branch to avoid loss during branch operations.

Verdict: APPROVED | Next: Create branch, begin execution. Milestone-specific review findings distributed to sub-issues.

## Plan: Migrate Gemini SDK to google-genai (#231) **Phase**: Planning | **Agent**: Orchestrator | **Status**: APPROVED **Revision**: v4 -- addresses QR plan-completeness + TW plan-scrub + QR plan-code + QR plan-docs --- ### Context The OCR service uses the deprecated `vertexai.generative_models` SDK from `google-cloud-aiplatform` in two files (`gemini_engine.py`, `maintenance_receipt_extractor.py`). This SDK is EOL June 2026. The replacement `google-genai` package provides a client-per-call pattern and first-class Google Search grounding for VIN decode accuracy. ### Milestone Index | Milestone | Sub-Issue | File(s) | Summary | |-----------|-----------|---------|---------| | M1 | #232 | `ocr/requirements.txt` | Swap dependency to `google-genai>=1.0.0` | | M2 | #233 | `ocr/app/engines/gemini_engine.py`, `ocr/app/config.py` | Full SDK migration + Google Search grounding for VIN decode | | M3 | #234 | `ocr/app/extractors/maintenance_receipt_extractor.py` | SDK migration (no search grounding) + fix RuntimeError bug | | M4 | #235 | `ocr/tests/test_gemini_engine.py` | Update mock patterns for new SDK | | M5 | (parent) | `ocr/CLAUDE.md`, `ocr/app/engines/CLAUDE.md`, `ocr/app/CLAUDE.md` | Doc-sync on affected directories | Each sub-issue contains its own self-contained plan with duplicated shared context (API migration map, internal state changes, authentication, error handling, risk assessment, review findings). An agent can execute from the sub-issue alone. ### M5: Doc-sync (no sub-issue) - `ocr/CLAUDE.md`: update references from "Vertex AI SDK" to "google-genai SDK" - `ocr/app/engines/CLAUDE.md`: update BOTH line 6 (prose: "via Vertex AI") AND line 18 (table: "Vertex AI SDK") to reference google-genai - `ocr/app/CLAUDE.md`: update line 10 (config.py table entry: "Vertex AI" -> "Google GenAI") - Run doc-sync skill on affected directories ### Verification 1. `docker build -t mvp-ocr-test ./ocr` -- confirms new dependency resolves and Docker image builds 2. Run OCR tests in container: `docker run --rm mvp-ocr-test pytest` -- confirms all unit tests pass 3. Deploy to staging and decode VIN `1G1YE2D32P5602473` -- confirms correct 2023 Corvette data 4. Check staging logs for Google Search grounding metadata in VIN decode responses ### Pre-requisite Commit untracked `ocr/tests/test_resolve_vin_year.py` before creating the feature branch to avoid loss during branch operations. --- *Verdict*: APPROVED | *Next*: Create branch, begin execution. Milestone-specific review findings distributed to sub-issues.
egullickson commented 2026-02-20 15:14:42 +00:00
Author
Owner
Copy Link
Copy Source

QR Review: plan-completeness

Phase: Plan-Review | Agent: Quality Reviewer | Status: PASS_WITH_CONCERNS

VERDICT: PASS_WITH_CONCERNS

Findings

[RULE 1] [HIGH]: Import count in analysis table is 4, not 3

  • Location: Codebase Analysis Summary, gemini_engine.py row
  • Issue: File has 4 distinct import statements (aiplatform, GenerationConfig+GenerativeModel, Part, GenerationConfig again), not 3
  • Suggested Fix: Correct count or enumerate explicitly

[RULE 1] [HIGH]: Untracked test_resolve_vin_year.py not mentioned in plan

  • Location: Analysis table and M4
  • Issue: Git shows ocr/tests/test_resolve_vin_year.py as untracked. Imports resolve_vin_year (pure function, no SDK deps) but should be acknowledged
  • Suggested Fix: Add to analysis table with "No changes (pure function, no SDK references)"; commit before migration

[RULE 1] [MEDIUM]: Config settings confirmation missing

  • Location: M2/M3
  • Issue: Plan should confirm settings.vertex_ai_project and settings.vertex_ai_location are still consumed by genai.Client(vertexai=True, project=..., location=...)
  • Suggested Fix: Add explicit note

[RULE 1] [MEDIUM]: CLAUDE.md doc updates not in milestones

  • Location: Milestones section
  • Issue: ocr/CLAUDE.md and ocr/app/engines/CLAUDE.md reference "Vertex AI SDK" -- will be stale
  • Suggested Fix: Add doc-sync step or M5

[RULE 1] [MEDIUM]: AC#3 traceability for manual_extractor

  • Location: M2 vs AC#3
  • Issue: "Maintenance extraction" in AC#3 flows through manual_extractor.py -> GeminiEngine.extract_maintenance() -- transitive coverage via M2 should be stated
  • Suggested Fix: Note in M2

[RULE 1] [MEDIUM]: Error messages reference old package name

  • Location: gemini_engine.py:265-266
  • Issue: ImportError message says "google-cloud-aiplatform is not installed" -- must update
  • Suggested Fix: Include in M2 scope

[RULE 2] [LOW]: docker-compose.yml env vars not verified

  • Location: Analysis table
  • Issue: VERTEX_AI_LOCATION: global may not be valid for new SDK
  • Suggested Fix: Confirm compatibility or add to analysis

Considered But Not Flagged

  • test_vin_decode.py / test_manual_extractor.py correctly marked as no changes
  • Dockerfile correctly marked as no changes (flows from requirements.txt)
  • Milestone ordering M1->M2->M3->M4 is correct
  • All 6 acceptance criteria have milestone coverage
  • Risk assessment for Google Search + response_schema is appropriate

Verdict: PASS_WITH_CONCERNS | Next: Update plan, then TW plan-scrub

## QR Review: plan-completeness **Phase**: Plan-Review | **Agent**: Quality Reviewer | **Status**: PASS_WITH_CONCERNS --- ### VERDICT: PASS_WITH_CONCERNS ### Findings #### [RULE 1] [HIGH]: Import count in analysis table is 4, not 3 - **Location**: Codebase Analysis Summary, `gemini_engine.py` row - **Issue**: File has 4 distinct import statements (aiplatform, GenerationConfig+GenerativeModel, Part, GenerationConfig again), not 3 - **Suggested Fix**: Correct count or enumerate explicitly #### [RULE 1] [HIGH]: Untracked `test_resolve_vin_year.py` not mentioned in plan - **Location**: Analysis table and M4 - **Issue**: Git shows `ocr/tests/test_resolve_vin_year.py` as untracked. Imports `resolve_vin_year` (pure function, no SDK deps) but should be acknowledged - **Suggested Fix**: Add to analysis table with "No changes (pure function, no SDK references)"; commit before migration #### [RULE 1] [MEDIUM]: Config settings confirmation missing - **Location**: M2/M3 - **Issue**: Plan should confirm `settings.vertex_ai_project` and `settings.vertex_ai_location` are still consumed by `genai.Client(vertexai=True, project=..., location=...)` - **Suggested Fix**: Add explicit note #### [RULE 1] [MEDIUM]: CLAUDE.md doc updates not in milestones - **Location**: Milestones section - **Issue**: `ocr/CLAUDE.md` and `ocr/app/engines/CLAUDE.md` reference "Vertex AI SDK" -- will be stale - **Suggested Fix**: Add doc-sync step or M5 #### [RULE 1] [MEDIUM]: AC#3 traceability for manual_extractor - **Location**: M2 vs AC#3 - **Issue**: "Maintenance extraction" in AC#3 flows through `manual_extractor.py` -> `GeminiEngine.extract_maintenance()` -- transitive coverage via M2 should be stated - **Suggested Fix**: Note in M2 #### [RULE 1] [MEDIUM]: Error messages reference old package name - **Location**: gemini_engine.py:265-266 - **Issue**: ImportError message says "google-cloud-aiplatform is not installed" -- must update - **Suggested Fix**: Include in M2 scope #### [RULE 2] [LOW]: docker-compose.yml env vars not verified - **Location**: Analysis table - **Issue**: `VERTEX_AI_LOCATION: global` may not be valid for new SDK - **Suggested Fix**: Confirm compatibility or add to analysis ### Considered But Not Flagged - test_vin_decode.py / test_manual_extractor.py correctly marked as no changes - Dockerfile correctly marked as no changes (flows from requirements.txt) - Milestone ordering M1->M2->M3->M4 is correct - All 6 acceptance criteria have milestone coverage - Risk assessment for Google Search + response_schema is appropriate --- *Verdict*: PASS_WITH_CONCERNS | *Next*: Update plan, then TW plan-scrub
egullickson commented 2026-02-20 15:17:25 +00:00
Author
Owner
Copy Link
Copy Source

TW Review: plan-scrub

Phase: Plan-Review | Agent: Technical Writer | Status: MINOR_EDITS

TW VERDICT: MINOR_EDITS

Findings

  • PRECISION: API Migration Map uses invalid Python shorthand genai.Client(vertexai=True, project, location) -- should be genai.Client(vertexai=True, project=..., location=...) to match keyword arg syntax
  • PRECISION: M4 description for test_valid_pdf_returns_structured_schedules should clarify it needs BOTH sys.modules patch update AND engine._model -> engine._client field reassignments
  • PRECISION: test_missing_sdk_raises_unavailable should specify exact patch mechanism: patch.dict("sys.modules", {"google.genai": None})
  • COMPLETENESS: M2 should explicitly state self._generation_config assignment (in _get_model()) is removed, not just that _get_model() becomes _get_client()
  • CONSISTENCY: "model" term collision -- MaintenanceExtractionResult.model field (model name string) is unrelated to self._model (GenerativeModel instance). Add note that MaintenanceExtractionResult.model is unaffected.
  • CLARITY: test_resolve_vin_year.py note should explain why commit is needed: "file is currently untracked; commit before beginning M1 to avoid loss during branch operations"
  • REDUNDANCY: AC#3 cross-reference in M2 is a dangling reference -- no numbered AC list in the plan. Use plain note instead.

Positive Notes

  • Codebase Analysis table is thorough, covering affected and unaffected files with rationale
  • API Migration Map is a strong implementer lookup reference
  • Internal State Changes before/after code block removes ambiguity
  • Milestone decomposition and dependency ordering is correct
  • "No changes" files with explanations save implementer investigation time

Verdict: MINOR_EDITS | Next: Apply edits, then QR plan-code

## TW Review: plan-scrub **Phase**: Plan-Review | **Agent**: Technical Writer | **Status**: MINOR_EDITS --- ### TW VERDICT: MINOR_EDITS ### Findings - **PRECISION**: API Migration Map uses invalid Python shorthand `genai.Client(vertexai=True, project, location)` -- should be `genai.Client(vertexai=True, project=..., location=...)` to match keyword arg syntax - **PRECISION**: M4 description for `test_valid_pdf_returns_structured_schedules` should clarify it needs BOTH `sys.modules` patch update AND `engine._model` -> `engine._client` field reassignments - **PRECISION**: `test_missing_sdk_raises_unavailable` should specify exact patch mechanism: `patch.dict("sys.modules", {"google.genai": None})` - **COMPLETENESS**: M2 should explicitly state `self._generation_config` assignment (in `_get_model()`) is removed, not just that `_get_model()` becomes `_get_client()` - **CONSISTENCY**: "model" term collision -- `MaintenanceExtractionResult.model` field (model name string) is unrelated to `self._model` (GenerativeModel instance). Add note that `MaintenanceExtractionResult.model` is unaffected. - **CLARITY**: `test_resolve_vin_year.py` note should explain why commit is needed: "file is currently untracked; commit before beginning M1 to avoid loss during branch operations" - **REDUNDANCY**: AC#3 cross-reference in M2 is a dangling reference -- no numbered AC list in the plan. Use plain note instead. ### Positive Notes - Codebase Analysis table is thorough, covering affected and unaffected files with rationale - API Migration Map is a strong implementer lookup reference - Internal State Changes before/after code block removes ambiguity - Milestone decomposition and dependency ordering is correct - "No changes" files with explanations save implementer investigation time --- *Verdict*: MINOR_EDITS | *Next*: Apply edits, then QR plan-code
egullickson commented 2026-02-20 15:22:31 +00:00
Author
Owner
Copy Link
Copy Source

QR Review: plan-code

Phase: Plan-Review | Agent: Quality Reviewer | Status: NEEDS_CHANGES

VERDICT: NEEDS_CHANGES

Findings

[RULE 0] CRITICAL: Response schema dict type values may require uppercase for google-genai SDK

  • Location: gemini_engine.py:119-154 (_VIN_DECODE_SCHEMA, _RESPONSE_SCHEMA), maintenance_receipt_extractor.py:56-78 (_RECEIPT_RESPONSE_SCHEMA)
  • Issue: All schemas use lowercase types ("string", "object", "number", "array"). The new SDK documentation shows uppercase ("STRING", "OBJECT", etc.). Will pass mocked unit tests but may fail at runtime.
  • Failure Mode: All three API calls fail with schema validation errors in production
  • Suggested Fix: Convert schema type strings to uppercase, or verify with integration test that lowercase is accepted

[RULE 0] HIGH: New SDK exception types not mapped to existing error hierarchy

  • Location: Plan section on _get_client() error handling
  • Issue: google-genai has its own exception hierarchy (APIError, ClientError, ServerError). Plan doesn't describe how these map to GeminiUnavailableError/GeminiProcessingError. The generic except Exception catch works but loses specific error info.
  • Suggested Fix: Document that generic except Exception intentionally catches new SDK errors, and add exception type to log messages

[RULE 0] MEDIUM: ADC environment variable setup ordering must be explicit

  • Location: Plan section on _get_client()
  • Issue: os.environ["GOOGLE_APPLICATION_CREDENTIALS"] must be set BEFORE genai.Client() construction. Plan doesn't confirm this.
  • Suggested Fix: Explicitly state env var setup is preserved before client construction

[RULE 0] MEDIUM: Google Search grounding adds new external failure mode

  • Location: Plan section on decode_vin() with Google Search
  • Issue: Introduces Google Search dependency into VIN decode. If Search is unavailable, error message says "Gemini VIN decode failed" when failure is in grounding.
  • Suggested Fix: Log specific error type to distinguish Gemini vs grounding failures

[RULE 1] HIGH: MaintenanceReceiptExtractor raises RuntimeError instead of GeminiUnavailableError

  • Location: maintenance_receipt_extractor.py:182
  • Issue: Pre-existing inconsistency: GeminiEngine raises GeminiUnavailableError but MaintenanceReceiptExtractor raises bare RuntimeError. Should fix during migration.
  • Suggested Fix: Change to GeminiUnavailableError in M3

[RULE 2] SHOULD_FIX: Test mocking has dead code (if False branch)

  • Location: test_gemini_engine.py:162
  • Issue: with (patch("...importlib_vertex_ai") if False else patch.dict(...)) -- the if False branch is dead code
  • Suggested Fix: Clean up during M4

Considered But Not Flagged

  • Lazy initialization pattern compatible with new SDK
  • response.text attribute exists on new SDK response objects
  • Part.from_data() -> Part.from_bytes() migration is correct
  • requirements.txt update already covered in M1
  • Duplication between GeminiEngine and MaintenanceReceiptExtractor is pre-existing; track as follow-up

Verdict: NEEDS_CHANGES | Next: Address findings, update plan

## QR Review: plan-code **Phase**: Plan-Review | **Agent**: Quality Reviewer | **Status**: NEEDS_CHANGES --- ### VERDICT: NEEDS_CHANGES ### Findings #### [RULE 0] CRITICAL: Response schema dict type values may require uppercase for google-genai SDK - **Location**: `gemini_engine.py:119-154` (`_VIN_DECODE_SCHEMA`, `_RESPONSE_SCHEMA`), `maintenance_receipt_extractor.py:56-78` (`_RECEIPT_RESPONSE_SCHEMA`) - **Issue**: All schemas use lowercase types (`"string"`, `"object"`, `"number"`, `"array"`). The new SDK documentation shows uppercase (`"STRING"`, `"OBJECT"`, etc.). Will pass mocked unit tests but may fail at runtime. - **Failure Mode**: All three API calls fail with schema validation errors in production - **Suggested Fix**: Convert schema type strings to uppercase, or verify with integration test that lowercase is accepted #### [RULE 0] HIGH: New SDK exception types not mapped to existing error hierarchy - **Location**: Plan section on `_get_client()` error handling - **Issue**: `google-genai` has its own exception hierarchy (`APIError`, `ClientError`, `ServerError`). Plan doesn't describe how these map to `GeminiUnavailableError`/`GeminiProcessingError`. The generic `except Exception` catch works but loses specific error info. - **Suggested Fix**: Document that generic `except Exception` intentionally catches new SDK errors, and add exception type to log messages #### [RULE 0] MEDIUM: ADC environment variable setup ordering must be explicit - **Location**: Plan section on `_get_client()` - **Issue**: `os.environ["GOOGLE_APPLICATION_CREDENTIALS"]` must be set BEFORE `genai.Client()` construction. Plan doesn't confirm this. - **Suggested Fix**: Explicitly state env var setup is preserved before client construction #### [RULE 0] MEDIUM: Google Search grounding adds new external failure mode - **Location**: Plan section on `decode_vin()` with Google Search - **Issue**: Introduces Google Search dependency into VIN decode. If Search is unavailable, error message says "Gemini VIN decode failed" when failure is in grounding. - **Suggested Fix**: Log specific error type to distinguish Gemini vs grounding failures #### [RULE 1] HIGH: MaintenanceReceiptExtractor raises RuntimeError instead of GeminiUnavailableError - **Location**: `maintenance_receipt_extractor.py:182` - **Issue**: Pre-existing inconsistency: `GeminiEngine` raises `GeminiUnavailableError` but `MaintenanceReceiptExtractor` raises bare `RuntimeError`. Should fix during migration. - **Suggested Fix**: Change to `GeminiUnavailableError` in M3 #### [RULE 2] SHOULD_FIX: Test mocking has dead code (`if False` branch) - **Location**: `test_gemini_engine.py:162` - **Issue**: `with (patch("...importlib_vertex_ai") if False else patch.dict(...))` -- the `if False` branch is dead code - **Suggested Fix**: Clean up during M4 ### Considered But Not Flagged - Lazy initialization pattern compatible with new SDK - `response.text` attribute exists on new SDK response objects - `Part.from_data()` -> `Part.from_bytes()` migration is correct - requirements.txt update already covered in M1 - Duplication between GeminiEngine and MaintenanceReceiptExtractor is pre-existing; track as follow-up --- *Verdict*: NEEDS_CHANGES | *Next*: Address findings, update plan
egullickson commented 2026-02-20 15:22:38 +00:00
Author
Owner
Copy Link
Copy Source

QR Review: plan-docs

Phase: Plan-Review | Agent: Quality Reviewer | Status: NEEDS_CHANGES

VERDICT: NEEDS_CHANGES

Findings

[RULE 2] SHOULD_FIX: GeminiEngine class docstring update missing from M2

  • Location: gemini_engine.py:209 -- "Uses lazy initialization: the Vertex AI client is not created until..."
  • Suggested Fix: M2 must include updating class docstring

[RULE 2] SHOULD_FIX: MaintenanceReceiptExtractor._get_model() docstring not in M3

  • Location: maintenance_receipt_extractor.py:173 -- "Lazy-initialize Vertex AI Gemini model."
  • Suggested Fix: M3 must include updating this docstring

[RULE 2] SHOULD_FIX: config.py inline comment not in plan scope

  • Location: config.py -- "# Vertex AI / Gemini configuration" comment
  • Suggested Fix: Add config.py comment update to scope (settings retained, comment updated)

[RULE 2] SHOULD_FIX: ocr/app/engines/CLAUDE.md has TWO Vertex AI references

  • Location: Line 6 (prose) and line 18 (table entry) -- plan only targets table entry
  • Suggested Fix: M5 must update both occurrences

[RULE 2] SHOULD_FIX: ocr/app/CLAUDE.md not in M5 scope

  • Location: Line 10 -- "Configuration settings (OCR engines, Vertex AI, Redis, Vision API limits)"
  • Suggested Fix: Add ocr/app/CLAUDE.md to M5

[RULE 2] SHOULD_FIX: Test file docstring will be stale

  • Location: test_gemini_engine.py:5 -- "All Vertex AI SDK calls are mocked."
  • Suggested Fix: M4 must include updating test module docstring

Considered But Not Flagged

  • No temporal contamination detected
  • Plan's risk assessment and verification sections read clearly
  • MaintenanceExtractionResult.model disambiguation note is precise and useful
  • Business logic comments (VIN year override) are SDK-agnostic, no update needed

Verdict: NEEDS_CHANGES | Next: Address findings, update plan to v4

## QR Review: plan-docs **Phase**: Plan-Review | **Agent**: Quality Reviewer | **Status**: NEEDS_CHANGES --- ### VERDICT: NEEDS_CHANGES ### Findings #### [RULE 2] SHOULD_FIX: GeminiEngine class docstring update missing from M2 - **Location**: `gemini_engine.py:209` -- "Uses lazy initialization: the Vertex AI client is not created until..." - **Suggested Fix**: M2 must include updating class docstring #### [RULE 2] SHOULD_FIX: MaintenanceReceiptExtractor._get_model() docstring not in M3 - **Location**: `maintenance_receipt_extractor.py:173` -- "Lazy-initialize Vertex AI Gemini model." - **Suggested Fix**: M3 must include updating this docstring #### [RULE 2] SHOULD_FIX: config.py inline comment not in plan scope - **Location**: `config.py` -- "# Vertex AI / Gemini configuration" comment - **Suggested Fix**: Add config.py comment update to scope (settings retained, comment updated) #### [RULE 2] SHOULD_FIX: ocr/app/engines/CLAUDE.md has TWO Vertex AI references - **Location**: Line 6 (prose) and line 18 (table entry) -- plan only targets table entry - **Suggested Fix**: M5 must update both occurrences #### [RULE 2] SHOULD_FIX: ocr/app/CLAUDE.md not in M5 scope - **Location**: Line 10 -- "Configuration settings (OCR engines, Vertex AI, Redis, Vision API limits)" - **Suggested Fix**: Add `ocr/app/CLAUDE.md` to M5 #### [RULE 2] SHOULD_FIX: Test file docstring will be stale - **Location**: `test_gemini_engine.py:5` -- "All Vertex AI SDK calls are mocked." - **Suggested Fix**: M4 must include updating test module docstring ### Considered But Not Flagged - No temporal contamination detected - Plan's risk assessment and verification sections read clearly - `MaintenanceExtractionResult.model` disambiguation note is precise and useful - Business logic comments (VIN year override) are SDK-agnostic, no update needed --- *Verdict*: NEEDS_CHANGES | *Next*: Address findings, update plan to v4
egullickson commented 2026-02-28 17:22:41 +00:00
Author
Owner
Copy Link
Copy Source

Milestone: All milestones complete

Phase: Execution | Agent: Orchestrator | Status: PASS

Summary

Milestone Sub-Issue Status Commit
M1 #232 PASS 398d673
M2 #233 PASS b7f472b
M3 #234 PASS 9f51e62
M4 #235 PASS 1464a0e
M5 (parent) PASS 96e1dde

Verification

  • Docker build: docker build -t mvp-ocr-test ./ocr -- SUCCESS
  • All 18 gemini engine tests: PASS
  • All VIN decode tests: PASS (unchanged, mock at router level)
  • All manual extractor tests: PASS (unchanged, mock at engine-instance level)
  • 2 pre-existing test_resolve_vin_year.py failures (unrelated to migration -- test assertions outdated vs VIN code)

Branch

issue-231-migrate-gemini-sdk-google-genai -- 5 commits, ready for PR.

Verdict: PASS | Next: Open PR, move to status/review

## Milestone: All milestones complete **Phase**: Execution | **Agent**: Orchestrator | **Status**: PASS --- ### Summary | Milestone | Sub-Issue | Status | Commit | |-----------|-----------|--------|--------| | M1 | #232 | PASS | `398d673` | | M2 | #233 | PASS | `b7f472b` | | M3 | #234 | PASS | `9f51e62` | | M4 | #235 | PASS | `1464a0e` | | M5 | (parent) | PASS | `96e1dde` | ### Verification - Docker build: `docker build -t mvp-ocr-test ./ocr` -- SUCCESS - All 18 gemini engine tests: PASS - All VIN decode tests: PASS (unchanged, mock at router level) - All manual extractor tests: PASS (unchanged, mock at engine-instance level) - 2 pre-existing `test_resolve_vin_year.py` failures (unrelated to migration -- test assertions outdated vs VIN code) ### Branch `issue-231-migrate-gemini-sdk-google-genai` -- 5 commits, ready for PR. --- *Verdict*: PASS | *Next*: Open PR, move to status/review
egullickson added
status
review
 and removed
status
in-progress
 labels 2026-02-28 17:23:00 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
status
backlog
status/blocked
status
done
status
in-progress
status
ready
status
review
type
bug
type
chore
type
docs
type
feature
No labels
status
review
type
feature
Milestone
No items
No Milestone Sprint 2026-02-02
Projects
Clear projects
No projects
Assignees
Clear assignees
egullickson
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: egullickson/motovaultpro#231