Repository navigation
Conversation
Core inspection infrastructure: - Inspection types, prompt tree (8 prompts with adaptive branching) - Step engine with full/quick mode, swarm-risk branch insertion - Inspection Zustand store with MMKV persistence for crash recovery - GraphQL operations for all inspection lifecycle mutations Inspection screens: - Entry screen with scope confirmation (full vs quick), resume detection - Guided step screen with progress bar, observation options, classification badges (normal/cautionary/urgent), voice hint, free-text fallback - Summary screen with observation list, status counts, colony improvement signal, save flow Voice/media capture scaffolded via types and GraphQL operations: - VoiceLogCapture states (idle/listening/processing/confirm) - Image capture, compression, upload via signed URL - Vision AI and acoustic analysis result types and polling queries 315 tests passing (38 suites), 16 new tests for step engine and inspection store. Lint clean, Go tests passing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Implements Epic 8’s guided inspection flow in the mobile app, including local/persisted inspection state, prompt sequencing with branching, GraphQL operation scaffolding, and the entry/step/summary screens.
Changes:
- Added a persisted Zustand inspection store (MMKV-backed) with lifecycle actions and tests.
- Added inspection domain types, prompt tree configuration, and a step engine (+ tests) to compute sequences/totals.
- Added GraphQL operations for inspection lifecycle and media/analysis scaffolding, plus new inspection entry/step/summary screens.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/mobile/src/store/inspection-store.ts | New persisted inspection lifecycle store (Zustand + MMKV). |
| apps/mobile/src/store/inspection-store.test.ts | Unit tests for inspection store lifecycle and mutations. |
| apps/mobile/src/services/graphql/inspection.ts | GraphQL queries/mutations for inspection lifecycle + analysis scaffolding. |
| apps/mobile/src/features/inspection/types/index.ts | New inspection/prompt/observation domain types. |
| apps/mobile/src/features/inspection/services/step-engine.ts | Prompt sequencing + branching logic and step calculations. |
| apps/mobile/src/features/inspection/services/step-engine.test.ts | Tests for sequencing, branching behavior, and totals. |
| apps/mobile/src/features/inspection/constants/prompt-tree.ts | Full/quick prompt configuration + branch override metadata. |
| apps/mobile/app/(tabs)/apiaries/[id]/hives/[hiveId]/inspect/index.tsx | Inspection entry (type selection, offline banner, resume detection). |
| apps/mobile/app/(tabs)/apiaries/[id]/hives/[hiveId]/inspect/step.tsx | Guided step screen (single prompt, options/free-text, progress). |
| apps/mobile/app/(tabs)/apiaries/[id]/hives/[hiveId]/inspect/summary.tsx | Summary screen (counts, observation list, save/clear flow). |
| _bmad-output/implementation-artifacts/sprint-status.yaml | Updates Epic 8 sprint status tracking. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const startInspection = useInspectionStore((s) => s.startInspection); | ||
| const hasActive = useInspectionStore((s) => s.hasActiveInspection); | ||
| const isOnline = useConnectivityStore((s) => s.isOnline); |
There was a problem hiding this comment.
hasActive is selected as the hasActiveInspection function, so this screen won’t re-render when inspectionId/status changes (the function reference is stable). This can leave the UI stuck showing the start UI even after an inspection becomes active (or vice versa) while the screen remains mounted in the navigation stack. Select a derived boolean from store state instead (e.g., via a selector that returns inspectionId !== null && status !== 'completed', or expose hasActiveInspection as a computed value in the store).
There was a problem hiding this comment.
Addressed in apps/mobile/app/(tabs)/apiaries/[id]/hives/[hiveId]/inspect/index.tsx: the component now selects a derived boolean ((s) => s.inspectionId !== null && s.status !== 'completed') into hasActiveInspection instead of selecting the stable hasActiveInspection function reference, so the screen re-renders whenever inspectionId/status change. The if (hasActive()) call site was updated to if (hasActiveInspection) accordingly.
| const basePrompts = getPromptsForType(type); | ||
| const result: InspectionPrompt[] = []; | ||
|
|
||
| for (const prompt of basePrompts) { | ||
| result.push(prompt); | ||
|
|
||
| // Check if any observation for this prompt triggers a branch | ||
| const obs = observations.find((o) => o.promptId === prompt.id); | ||
| if (obs && prompt.options) { | ||
| const selected = prompt.options.find((o) => o.id === obs.value); | ||
| if (selected?.nextPromptOverride) { | ||
| const branchPrompt = basePrompts.find((p) => p.id === selected.nextPromptOverride); | ||
| if (branchPrompt && !result.includes(branchPrompt)) { | ||
| // Insert branch prompt after current | ||
| result.push(branchPrompt); | ||
| } |
There was a problem hiding this comment.
Branch insertion currently searches basePrompts (already filtered by inspection type) to find nextPromptOverride. This means a branch target that’s not in the base list (e.g., a quickMode: false prompt like swarm_risk) can never be inserted for quick inspections, even when an option explicitly requests it. Consider keeping an unfiltered prompt lookup (e.g., from the full tree) for override targets, while still starting from the type-filtered base sequence.
There was a problem hiding this comment.
Addressed in apps/mobile/src/features/inspection/services/step-engine.ts: getPromptSequence now resolves nextPromptOverride targets against the unfiltered ALL_PROMPTS list (imported from the prompt tree) rather than the type-filtered basePrompts. This lets a branch target that is not in the base sequence (e.g. a quickMode: false prompt like swarm_risk) be inserted for quick inspections when an option explicitly requests it. The dedupe guard was also changed to compare by p.id (!result.some((p) => p.id === branchPrompt.id)).
- Replace misleading `as 'checkmark-circle'` casts with proper `as keyof typeof Ionicons.glyphMap` across 3 files - Add updateObservation test to inspection store (covers public API) - Fix swarm_risk test: assert exact count (1) and verify placement immediately after queen_cells when branching triggers - Add branch ordering test for swarm risk insertion Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Superseded by automated re-review at
|
|
@claude Please address all open review comments on this PR from CodeRabbit and Copilot. |
|
Claude finished @don-petry's task in 11m 10s —— View job Addressing Review Comments — DoneAll open Copilot and cycle-1 review findings addressed in commit Changes made
|
…ion, screen tests Addresses all open Copilot and cycle-1 review findings: - prompt-tree: remove swarm_risk from base sequence; add ALL_PROMPTS export so the step engine can find branch-only prompts without them appearing in every full inspection unconditionally (Copilot: prompt-tree.ts:60) - step-engine: use ALL_PROMPTS for nextPromptOverride lookup so quick-mode inspections can also branch to full-only prompts like swarm_risk (Copilot: step-engine.ts:23) - step.tsx: fix stale memoised prompts in handleNext by recomputing the sequence with the new observation before calculating the next index, so swarm_risk insertion is reflected before setPromptIndex is called (Review: step.tsx:64) - step.tsx: move router.replace out of render body into useEffect to avoid side-effects during render / strict-mode double-navigation (Review: step.tsx:38) - classification.ts: extract shared CLASSIFICATION_STYLES constant from step.tsx and CLASSIFICATION_CONFIG from summary.tsx into a single source of truth, eliminating the ~15-line duplication (Review: step.tsx:13) - index.tsx: select derived boolean for hasActiveInspection rather than the function reference so the component re-renders on state changes (Copilot: index.tsx:18); derive step counts from getTotalSteps instead of hardcoding them (Copilot: index.tsx:129) - inspect-screens.test.tsx: 28 new tests covering all three inspect screens, including swarm branching integration, to improve SonarCloud new-code coverage Co-authored-by: Don Petry <don-petry@users.noreply.github.com>
Review — fix requested (cycle 2/3)The automated review identified the following issues. Please address each one: Findings to fix[Findings would be inserted here] Additional tasks
The review cascade will automatically re-review after new commits are pushed. |
* Epic 9: Recommendation Engine (Stories 9.1–9.5) Story 9.1: Context assembly service — assembles hive context from inspection history, seasonal phase, region, telemetry with freshness timestamps and missing source flagging Story 9.2: Confidence penalty system — adjusts recommendation confidence based on stale/missing data sources, downgrades confidence type appropriately (HIGH→MODERATE→LOW→INSUFFICIENT_DATA) Story 9.3: Conservative defaults — generates safe fallback recommendations when data is insufficient, season-aware (winter: food stores, spring: swarm prep) Story 9.4: Degraded mode — confidence floor at 0.1, INSUFFICIENT_DATA capped at 0.5, configurable staleness thresholds (weather 24h, flora 7d) Story 9.5: Skill mismatch detection — detects sustained behavioral patterns (5+ signals in 14-day window) suggesting misconfigured skill level, returns gentle suggestion message 14 new Go tests covering all service methods. All tests passing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix Epic 9 review: confidence downgrade cascade, conservative default ID - Reorder confidence type downgrade to use else-if chain, preventing MODERATE from being immediately overwritten by LOW when penalty is in the 0.2-0.3 range. Most severe condition (missing sources) now checked first. - Give conservative default recommendation a proper ID instead of empty string, and handle NewRecommendation error explicitly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix Epic 9 review: monotonic confidence downgrade, propagate errors, clean AssembleContext stub - ApplyConfidencePenalty: guard HIGH→LOW downgrade so INSUFFICIENT_DATA and other non-standard types are never upgraded; reorder INSUFFICIENT_DATA cap before universal floor with inline invariant comments (Copilot + cycle-1 review) - AssembleContext: remove unconditional stub that marked all three sources as missing on every call, making HIGH/MODERATE confidence unreachable; return clean context — integration hooks documented in godoc (cycle-1 major) - GenerateConservativeDefault: return (*domain.Recommendation, error) instead of silently swallowing the error and returning nil (Copilot + cycle-1 minor) - Tests: update AssembleContext test for corrected stub behavior; add INSUFFICIENT_DATA-not-upgraded-to-LOW test; update GenerateConservativeDefault tests to handle error return (Copilot) - sonar-project.properties: add sonar.go.coverage.reportPaths so SonarCloud picks up the coverage.out emitted by the CI Go job (cycle-1 major) Co-authored-by: Don Petry <don-petry@users.noreply.github.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Don Petry <don-petry@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Review — fix requested (cycle 2/3)The automated review identified the following issues. Please address each one: Findings to fix[Findings would be inserted here] Additional tasks
The review cascade will automatically re-review after new commits are pushed. |
Review — fix requested (cycle 2/3)The automated review identified the following issues. Please address each one: Findings to fix[Findings would be inserted here] Additional tasks
The review cascade will automatically re-review after new commits are pushed. |
CI Failure: SonarCloud Code AnalysisStep: SonarCloud Scan
Suggested fix: Add a Go test step to |
This comment has been minimized.
This comment has been minimized.
CI Failure: SonarCloud Code AnalysisStep: SonarCloud Quality Gate This PR adds Suggested fix: Add a step in the SonarCloud workflow to generate the coverage report before analysis runs: |
|
This PR has been inactive for 32 days and will be closed in 7 days if there is no activity. |
|
Auto-rebase is skipping this PR. It has merge conflicts with To unblock, do one of:
|
|
Auto-rebase failed — merge conflict — this branch has conflicts with dev-lead will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
This PR implements the Guided Inspection Flow (Epic 8, Stories 8.1–8.7) and has been inactive for 34 days. It will be automatically closed in 7 days if there's no new activity — feel free to leave a comment or push an update to keep it open! |
|
No description provided. |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
|
No description provided. |
**Coverage on New Code (0% → 80.7%)** - Updated sonarcloud.yml to generate Go coverage before scanning - Added 'Generate Go coverage for SonarCloud' step that runs 'go test -coverprofile=coverage.out ./...' - This ensures coverage.out file exists when SonarCloud scans, matching sonar-project.properties config **Duplication on New Code (7.9% → reduced)** - Extracted createQueenCellObservation helper in step-engine.test.ts to eliminate duplicate observation objects - Extracted createObservation helper in inspect-screens.test.tsx to reduce observation creation duplication - Refactored 5 test cases in step-engine.test.ts to use helper function - Refactored 3 test case groups in inspect-screens.test.tsx to use helper function This eliminates the duplicated object literal patterns that were triggering SonarCloud's duplication metrics. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Dev-Lead — fix-bot-comment (applied)Changes committed and pushed. |
|
No description provided. |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
|
Auto-rebase failed — merge conflict — this branch has conflicts with dev-lead will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
Auto-rebase failed — merge conflict — this branch has conflicts with dev-lead will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
🤖 Dev-Lead Rebase: Aborted — unsafe to rebase automaticallyI attempted to rebase this branch onto What I found
Why a blind rebase is dangerous here
Replaying this branch's 72 commits onto Recommended path forward (needs a human call)These histories need an intentional reconciliation, not an automated rebase. Options:
I did not modify, force-push, or alter history. The branch is untouched. Please advise on the preferred reconciliation strategy and I can proceed. |
Dev-Lead — rebase (no-changes)Agent reasoning |
|
No description provided. |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
|
No description provided. |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
|
No description provided. |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
|
No description provided. |
|
No description provided. |
|
Auto-rebase failed — merge conflict — this branch has conflicts with dev-lead will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
Dev-Lead — rebase (failed)Engine failed to resolve the rebase conflict (exit 1). |
|
No description provided. |
|
This CodeRabbit comment is the auto-generated "Draft PR not reviewed" skip notice (CodeRabbit does not auto-review draft PRs). It contains no review findings and requires no code change; the PR has since progressed through dedicated Copilot review. Acknowledged as informational. |
|
This SonarCloud Quality Gate failure (0% coverage on new code; duplication on new code above 3%) is a real finding but is out of scope for the two Copilot review threads on this PR, which are TypeScript-only. The root cause (the SonarCloud workflow not generating the Go coverage report) plus the new-code duplication cleanup are tracked in #591 for a dedicated follow-up. |
|
This SonarCloud Quality Gate failure (0% coverage on new code; 10.5% duplication on new code) is a real finding but is out of scope for the two Copilot review threads on this PR, which are TypeScript-only. The missing Go coverage report step in the SonarCloud workflow and the new-code duplication cleanup are tracked in #591. |
Summary
Stacked on #33 (Epic 7).
Core Infrastructure
Screens
Design Compliance
Test plan
🤖 Generated with Claude Code