From c8ffe2626fb1cefc3c76151166a5d06c8e2c925a Mon Sep 17 00:00:00 2001 From: Ponshu Developer Date: Sun, 12 Apr 2026 00:24:57 +0900 Subject: [PATCH] refactor: apply critical code review fixes - draft_service: GeminiService is now passed as a required parameter to analyzeDraft/analyzeAllDrafts instead of being directly instantiated, ensuring consistent Riverpod-managed injection - gemini_provider: correct misleading comment (rate limiting is due to static field, not Provider; Provider enables future safe refactoring) - analysis_cache_service: cleanupExpired now also removes orphaned brand index entries from _brandIndexBox after deleting expired _box entries - analysis_cache_service: keysToDelete type corrected from List to List, removing unnecessary as String cast - analysis_cache_service: _normalizeBrandName comment clarified to note that .toLowerCase() only affects ASCII characters, not Japanese text - camera_screen: add explicit ignore comment with rationale for showDialog after async gap (mounted check immediately precedes it) - camera_screen: remove leaked Cursor instruction comment from line 96 Made-with: Cursor --- lib/providers/gemini_provider.dart | 8 +++++--- lib/screens/camera_screen.dart | 4 ++-- lib/screens/pending_analysis_screen.dart | 2 ++ lib/services/analysis_cache_service.dart | 25 +++++++++++++++++++++--- lib/services/draft_service.dart | 24 +++++++++++------------ 5 files changed, 42 insertions(+), 21 deletions(-) diff --git a/lib/providers/gemini_provider.dart b/lib/providers/gemini_provider.dart index fe15d02..cc7cd50 100644 --- a/lib/providers/gemini_provider.dart +++ b/lib/providers/gemini_provider.dart @@ -3,9 +3,11 @@ import '../services/gemini_service.dart'; /// GeminiService のシングルトンプロバイダー /// -/// アプリ全体で同一インスタンスを共有する。 -/// レート制限の状態（_lastApiCallTime）がインスタンス間で共有されるため、 -/// 複数画面から同時に呼び出しても連打防止が正しく機能する。 +/// アプリ全体で同一インスタンスを共有し、依存関係の注入を明示化する。 +/// レート制限の状態（_lastApiCallTime）は GeminiService の static フィールドの +/// ため、Provider を経由しなくても共有される。しかし Provider を通じて注入する +/// ことで「誰が GeminiService を使うか」を追跡可能にし、将来の static 除去時に +/// 安全にリファクタリングできる設計を維持する。 /// /// 使用例: /// ```dart diff --git a/lib/screens/camera_screen.dart b/lib/screens/camera_screen.dart index 21bf574..a0f0cf9 100644 --- a/lib/screens/camera_screen.dart +++ b/lib/screens/camera_screen.dart @@ -93,8 +93,6 @@ class _CameraScreenState extends ConsumerState with SingleTickerPr super.dispose(); } - // ... (Keep existing methods: _capturedImages, _takePicture, _analyzeImages) - double? _pendingExposureValue; bool _isUpdatingExposure = false; @@ -424,6 +422,8 @@ class _CameraScreenState extends ConsumerState with SingleTickerPr // オンライン時: 通常の解析フロー if (!mounted) return; + // ignore: use_build_context_synchronously + // 直前の mounted チェックにより BuildContext の有効性は保証されている showDialog( context: context, barrierDismissible: false, diff --git a/lib/screens/pending_analysis_screen.dart b/lib/screens/pending_analysis_screen.dart index 7b966e8..3036f41 100644 --- a/lib/screens/pending_analysis_screen.dart +++ b/lib/screens/pending_analysis_screen.dart @@ -3,6 +3,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:lucide_icons/lucide_icons.dart'; import '../models/sake_item.dart'; +import '../providers/gemini_provider.dart'; import '../services/draft_service.dart'; import '../services/network_service.dart'; import '../theme/app_colors.dart'; @@ -79,6 +80,7 @@ class _PendingAnalysisScreenState extends ConsumerState { try { final result = await DraftService.analyzeAllDrafts( + geminiService: ref.read(geminiServiceProvider), onProgress: (progress, total) { if (mounted) { setState(() { diff --git a/lib/services/analysis_cache_service.dart b/lib/services/analysis_cache_service.dart index f4e4e99..0d33675 100644 --- a/lib/services/analysis_cache_service.dart +++ b/lib/services/analysis_cache_service.dart @@ -137,12 +137,13 @@ class AnalysisCacheService { /// /// アプリ起動時またはバックグラウンドで呼び出す。 /// 旧形式エントリ（タイムスタンプなし）は対象外として保持する。 + /// _brandIndexBox の孤立エントリ（削除されたハッシュを参照するもの）も合わせて削除する。 static Future cleanupExpired({int ttlDays = 30}) async { await init(); if (_box == null) return; final cutoff = DateTime.now().subtract(Duration(days: ttlDays)); - final keysToDelete = []; + final keysToDelete = []; for (final key in _box!.keys) { try { @@ -163,6 +164,22 @@ class AnalysisCacheService { if (keysToDelete.isNotEmpty) { await _box!.deleteAll(keysToDelete); debugPrint('Cache cleanup: ${keysToDelete.length} expired entries removed'); + + // 削除したハッシュを参照している _brandIndexBox の孤立エントリも削除する + if (_brandIndexBox != null) { + final deletedHashes = keysToDelete.toSet(); + final brandKeysToDelete = []; + for (final brandKey in _brandIndexBox!.keys) { + final hash = _brandIndexBox!.get(brandKey as String); + if (hash != null && deletedHashes.contains(hash)) { + brandKeysToDelete.add(brandKey); + } + } + if (brandKeysToDelete.isNotEmpty) { + await _brandIndexBox!.deleteAll(brandKeysToDelete); + debugPrint('Cache cleanup: ${brandKeysToDelete.length} orphaned brand index entries removed'); + } + } } } @@ -225,12 +242,14 @@ class AnalysisCacheService { /// 銘柄名の正規化 /// - /// スペース除去、小文字化で表記ゆれを吸収 + /// 全角・半角スペースを除去し、ASCII文字を小文字に統一する。 /// 例: "獺祭 純米大吟醸" → "獺祭純米大吟醸" + /// 例: "DASSAI" → "dassai" + /// 日本語文字には大文字・小文字の区別がないため、.toLowerCase() は ASCII のみに作用する。 static String _normalizeBrandName(String name) { return name .replaceAll(RegExp(r'\s+'), '') // 全角・半角スペース除去 - .toLowerCase(); + .toLowerCase(); // ASCII ローマ字の表記ゆれを吸収（日本語には無効） } /// 銘柄名インデックスをクリア（デバッグ用） diff --git a/lib/services/draft_service.dart b/lib/services/draft_service.dart index 7747066..e18e18c 100644 --- a/lib/services/draft_service.dart +++ b/lib/services/draft_service.dart @@ -122,7 +122,10 @@ class DraftService { /// print('解析失敗: $e'); /// } /// ``` - static Future analyzeDraft(dynamic itemKey) async { + static Future analyzeDraft( + dynamic itemKey, { + required GeminiService geminiService, + }) async { final box = Hive.box('sake_items'); final item = box.get(itemKey); @@ -139,14 +142,11 @@ class DraftService { throw Exception('Draft has no photo path: $itemKey'); } - debugPrint('🔍 Analyzing draft: $itemKey (${item.displayData.displayName})'); + debugPrint('Analyzing draft: $itemKey (${item.displayData.displayName})'); - // 🔧 FIX: 複数画像がある場合はすべて使用 final imagePaths = item.displayData.imagePaths; final pathsToAnalyze = imagePaths.isNotEmpty ? imagePaths : [photoPath]; - // Gemini Vision API呼び出し - final geminiService = GeminiService(); final result = await geminiService.analyzeSakeLabel(pathsToAnalyze); debugPrint('✅ Analysis completed: ${result.name}'); @@ -193,13 +193,14 @@ class DraftService { /// print('成功: ${result['success']}, 失敗: ${result['failed']}'); /// ``` static Future> analyzeAllDrafts({ + required GeminiService geminiService, Function(int progress, int total)? onProgress, }) async { final drafts = await getPendingDrafts(); final total = drafts.length; if (total == 0) { - debugPrint('📭 No pending drafts to analyze'); + debugPrint('No pending drafts to analyze'); return {'success': 0, 'failed': 0, 'errors': []}; } @@ -207,27 +208,24 @@ class DraftService { int failedCount = 0; final List errors = []; - debugPrint('🚀 Analyzing $total drafts...'); - for (int i = 0; i < total; i++) { final draft = drafts[i]; final itemKey = draft.key; try { onProgress?.call(i + 1, total); - await analyzeDraft(itemKey); + await analyzeDraft(itemKey, geminiService: geminiService); successCount++; - debugPrint('✅ [${i+1}/$total] Success: ${draft.displayData.displayName}'); + debugPrint('[${i+1}/$total] Success: ${draft.displayData.displayName}'); } catch (e) { failedCount++; final errorMsg = '[${i+1}/$total] ${draft.displayData.displayName}: $e'; errors.add(errorMsg); - debugPrint('❌ $errorMsg'); - // 失敗してもループは継続（他のDraftを解析） + debugPrint(errorMsg); } } - debugPrint('🎉 Batch analysis completed: $successCount成功, $failedCount失敗'); + debugPrint('Batch analysis completed: $successCount success, $failedCount failed'); return { 'success': successCount,