feat: Offline AI Plant and Wildlife Identification#1902
feat: Offline AI Plant and Wildlife Identification#1902
Conversation
Co-authored-by: andrew-bierman <94939237+andrew-bierman@users.noreply.github.com>
|
@coderabbitai Please review this PR for code quality, best practices, and potential issues. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 2292 🏁 Script executed: Length of output: 1348 🏁 Script executed: Length of output: 26976 🏁 Script executed: Length of output: 13507 🏁 Script executed: Length of output: 1916 🏁 Script executed: Length of output: 26472
Review: Offline AI Plant and Wildlife IdentificationOverall the feature is well-structured and the abstraction layer for the ML engine is a good design decision. Here are the issues found, ordered by severity. 🔴 Critical1.
|
|
@copilot CodeRabbit has reviewed this PR and provided feedback. Please address the review comments and push updates. Once you've made the changes, the monitor will ask CodeRabbit to re-review. |
There was a problem hiding this comment.
Pull request overview
Adds a new Expo feature module for fully offline plant/wildlife identification, including capture → on-device (mock) inference → results display, plus an AsyncStorage-backed history and dashboard integration behind a feature flag.
Changes:
- Introduces
features/wildlife/with types, hooks (identify + history), tile, and two screens (identify + history). - Adds Expo Router routes for
/wildlifeand/wildlife/history. - Integrates the feature into the dashboard layout (tile) and adds
wildlifei18n strings + feature flag.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/expo/lib/i18n/locales/en.json | Adds wildlife translation namespace for the new UI. |
| apps/expo/features/wildlife/types.ts | Defines core types for identifications and history entries. |
| apps/expo/features/wildlife/screens/index.ts | Barrel export for wildlife screens. |
| apps/expo/features/wildlife/screens/WildlifeIdentificationScreen.tsx | Capture/identify flow UI + results rendering. |
| apps/expo/features/wildlife/screens/WildlifeHistoryScreen.tsx | History list UI + clear-all action. |
| apps/expo/features/wildlife/index.ts | Barrel export for wildlife module (hooks/types). |
| apps/expo/features/wildlife/hooks/useWildlifeIdentification.ts | Image picking + mock on-device inference + persistence to AsyncStorage. |
| apps/expo/features/wildlife/hooks/useWildlifeHistory.ts | React Query wrapper around AsyncStorage history with invalidation + clear. |
| apps/expo/features/wildlife/hooks/index.ts | Barrel export for hooks. |
| apps/expo/features/wildlife/components/index.ts | Barrel export for wildlife components. |
| apps/expo/features/wildlife/components/WildlifeTile.tsx | Dashboard tile showing offline capability and count. |
| apps/expo/config.ts | Adds enableWildlifeIdentification feature flag. |
| apps/expo/app/(app)/wildlife/index.tsx | Registers /wildlife route. |
| apps/expo/app/(app)/wildlife/history.tsx | Registers /wildlife/history route. |
| apps/expo/app/(app)/wildlife/_layout.tsx | Adds Stack layout for wildlife routes. |
| apps/expo/app/(app)/(tabs)/(home)/index.tsx | Adds wildlife tile metadata and conditional dashboard layout entry. |
You can also share your feedback on Copilot code review. Take the survey.
| <Pressable className="px-2 opacity-80" onPress={() => router.push('/wildlife/history')}> | ||
| {({ pressed }) => ( | ||
| <View className={pressed ? 'opacity-50' : 'opacity-90'}> | ||
| <Icon name="history" size={22} color={colors.foreground} /> | ||
| </View> | ||
| )} | ||
| </Pressable> |
There was a problem hiding this comment.
The history icon button is an icon-only Pressable without an accessibilityLabel/accessibilityRole. Please add an explicit label (and role) so screen readers can announce what this button does.
| <Pressable className="px-2 opacity-80" onPress={handleClear}> | ||
| {({ pressed }) => ( | ||
| <View className={pressed ? 'opacity-50' : 'opacity-90'}> | ||
| <Icon name="trash-can-outline" size={22} color={colors.destructive} /> | ||
| </View> | ||
| )} | ||
| </Pressable> |
There was a problem hiding this comment.
The clear-history control is an icon-only Pressable without an accessibilityLabel/accessibilityRole. Please add a label like “Clear identification history” so assistive tech can identify the action.
| "offline": "Offline", | ||
| "offlineCapable": "All processing happens on-device — works without internet", | ||
| "capturePhoto": "Take or choose a photo to identify species", |
There was a problem hiding this comment.
identifiedCount will read awkwardly for a single identification ("1 species identified"). The repo already uses separate singular keys in places (e.g. catalog.totalItemsSingular), so consider adding a singular form (or adopting the existing pluralization approach) and using it when count === 1.
| "offline": "Offline", | |
| "offlineCapable": "All processing happens on-device — works without internet", | |
| "capturePhoto": "Take or choose a photo to identify species", | |
| "identifiedCountSingular": "{{count}} species identified", | |
| "offline": "Offline", | |
| "offlineCapable": "All processing happens on-device — works without internet", |
| if (!edibility || edibility === 'unknown') return null; | ||
| const config: Record<string, { bg: string; label: string }> = { | ||
| edible: { bg: 'bg-green-500/20', label: '✓ Edible' }, | ||
| poisonous: { bg: 'bg-red-500/20', label: '✗ Poisonous' }, | ||
| medicinal: { bg: 'bg-blue-500/20', label: '⚕ Medicinal' }, | ||
| }; | ||
| const c = config[edibility]; | ||
| if (!c) return null; | ||
| return ( | ||
| <View className={`rounded-full px-3 py-1 ${c.bg}`}> | ||
| <Text variant="caption1" className="font-medium"> | ||
| {c.label} |
There was a problem hiding this comment.
EdibilityBadge hardcodes user-visible strings (and symbols) like "✓ Edible" / "✗ Poisonous". Since the rest of the screen is localized via t('wildlife.*'), these labels should come from i18n keys as well (and keep the badge logic keyed on EdibilityStatus).
| if (!edibility || edibility === 'unknown') return null; | |
| const config: Record<string, { bg: string; label: string }> = { | |
| edible: { bg: 'bg-green-500/20', label: '✓ Edible' }, | |
| poisonous: { bg: 'bg-red-500/20', label: '✗ Poisonous' }, | |
| medicinal: { bg: 'bg-blue-500/20', label: '⚕ Medicinal' }, | |
| }; | |
| const c = config[edibility]; | |
| if (!c) return null; | |
| return ( | |
| <View className={`rounded-full px-3 py-1 ${c.bg}`}> | |
| <Text variant="caption1" className="font-medium"> | |
| {c.label} | |
| const { t } = useTranslation(); | |
| if (!edibility || edibility === 'unknown') return null; | |
| const config: Record<string, { bg: string; i18nKey: string }> = { | |
| edible: { bg: 'bg-green-500/20', i18nKey: 'wildlife.edibility.edible' }, | |
| poisonous: { bg: 'bg-red-500/20', i18nKey: 'wildlife.edibility.poisonous' }, | |
| medicinal: { bg: 'bg-blue-500/20', i18nKey: 'wildlife.edibility.medicinal' }, | |
| }; | |
| const c = config[edibility]; | |
| if (!c) return null; | |
| return ( | |
| <View className={`rounded-full px-3 py-1 ${c.bg}`}> | |
| <Text variant="caption1" className="font-medium"> | |
| {t(c.i18nKey)} |
| const result = await ImagePicker.launchCameraAsync({ | ||
| mediaTypes: 'images', | ||
| quality: 0.8, | ||
| allowsEditing: true, | ||
| aspect: [4, 3], | ||
| }); |
There was a problem hiding this comment.
mediaTypes is being passed as the string 'images', but elsewhere in the app expo-image-picker is configured with ImagePicker.MediaTypeOptions.Images. Using the enum avoids type/runtime mismatches across SDK versions.
| const result = await ImagePicker.launchImageLibraryAsync({ | ||
| mediaTypes: 'images', | ||
| quality: 0.8, | ||
| allowsEditing: true, | ||
| aspect: [4, 3], | ||
| }); |
There was a problem hiding this comment.
Same as the camera path: mediaTypes: 'images' should use ImagePicker.MediaTypeOptions.Images to match the rest of the codebase and the expo-image-picker typings.
| identification: SpeciesIdentification, | ||
| ): Promise<void> { | ||
| const stored = await AsyncStorage.getItem(HISTORY_STORAGE_KEY); | ||
| const existing: SpeciesIdentification[] = stored ? JSON.parse(stored) : []; |
There was a problem hiding this comment.
persistIdentificationToHistory does an unguarded JSON.parse on the stored value. If AsyncStorage is corrupted, this will throw and break identification; consider wrapping parse in a try/catch (similar to loadHistory) and clearing/resetting the key on failure.
| const existing: SpeciesIdentification[] = stored ? JSON.parse(stored) : []; | |
| let existing: SpeciesIdentification[] = []; | |
| if (stored) { | |
| try { | |
| const parsed = JSON.parse(stored); | |
| if (Array.isArray(parsed)) { | |
| existing = parsed as SpeciesIdentification[]; | |
| } else { | |
| // Stored value is not in the expected format; clear it. | |
| await AsyncStorage.removeItem(HISTORY_STORAGE_KEY); | |
| } | |
| } catch { | |
| // Corrupted JSON; clear the key and start fresh. | |
| await AsyncStorage.removeItem(HISTORY_STORAGE_KEY); | |
| } | |
| } |
| invalidate(); | ||
| }, | ||
| onError: (err) => { | ||
| Alert.alert(t('wildlife.identificationFailed'), err.message); |
There was a problem hiding this comment.
In React Query, the onError callback receives unknown by default; accessing err.message will fail type-checking (and may not be safe at runtime). Please narrow the error (e.g., err instanceof Error) or stringify it before showing the alert.
| Alert.alert(t('wildlife.identificationFailed'), err.message); | |
| const message = err instanceof Error ? err.message : String(err); | |
| Alert.alert(t('wildlife.identificationFailed'), message); |
…cache ownership, tips keys, layout registration, import order Co-authored-by: andrew-bierman <94939237+andrew-bierman@users.noreply.github.com>
All 9 CodeRabbit review items have been addressed in commit
|
|
@copilot Please address the CodeRabbit review feedback on this PR (8 inline comments). |
Co-authored-by: andrew-bierman <94939237+andrew-bierman@users.noreply.github.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (18)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
enableWildlifeIdentificationfeature flag toapps/expo/config.tsapps/expo/features/wildlife/types.tswith type definitionsapps/expo/features/wildlife/constants.ts— shared constantsWildlifeIdentificationScreen:135—accessibilityLabel="wildlife.viewHistory"+accessibilityRole="button"on history icon buttonWildlifeHistoryScreen:84—accessibilityLabel="wildlife.clearHistoryAction"+accessibilityRole="button"on clear-history icon buttonen.json— addedidentifiedCountSingularkey;WildlifeTilenow uses it whencount === 1EdibilityBadge— hardcoded label strings replaced witht('wildlife.edibilityEdible/Poisonous/Medicinal')i18n keysuseWildlifeIdentification.ts— bothlaunchCameraAsyncandlaunchImageLibraryAsyncnow useImagePicker.MediaTypeOptions.ImagesWildlifeIdentificationScreen:120— error inonErrorcallback narrowed viaerr instanceof Error ? err.message : String(err)en.json— addedclearHistoryAction,viewHistory,edibilityEdible/Poisonous/MedicinalkeysOriginal prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.