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

chore: change maintenance receipt flow to upload-first with camera option (#162) #182

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

Relates to #162

Severity: Medium

Problem

The maintenance record receipt scanning feature was mirrored from the fuel log receipt flow but should be closer to the document/manual scan pattern. Currently:

  • Button says "Scan Receipt" -- implies camera-only
  • Clicking goes directly to full-screen camera
  • Upload is buried as a fallback inside the camera screen

Changes Required

  1. Rename "Scan Receipt" button to "Add Receipt"
  2. Clicking "Add Receipt" opens a full-screen dialog with two options:
    • Drag and Drop an Image (existing FileInputFallback pattern) -- the primary/default option
    • Take Photo of Receipt -- clicking this opens the existing CameraCapture modal
  3. Both paths feed into the same OCR processing pipeline (extract receipt data, show review modal)
  4. Both options shown on all devices (desktop and mobile)

Acceptance Criteria

  • "Add Receipt" button replaces "Scan Receipt"
  • Full-screen dialog opens with two clear option boxes
  • Drag-and-drop/browse upload is the primary action
  • "Take Photo" option opens camera modal only when clicked
  • Uploaded and captured images both go through OCR extraction
  • Review modal works the same for both input methods
  • Tested on mobile (320px, 768px) and desktop (1920px)
Relates to #162 ## Severity: Medium ## Problem The maintenance record receipt scanning feature was mirrored from the fuel log receipt flow but should be closer to the document/manual scan pattern. Currently: - Button says "Scan Receipt" -- implies camera-only - Clicking goes directly to full-screen camera - Upload is buried as a fallback inside the camera screen ## Changes Required 1. Rename "Scan Receipt" button to "Add Receipt" 2. Clicking "Add Receipt" opens a full-screen dialog with two options: - **Drag and Drop an Image** (existing FileInputFallback pattern) -- the primary/default option - **Take Photo of Receipt** -- clicking this opens the existing CameraCapture modal 3. Both paths feed into the same OCR processing pipeline (extract receipt data, show review modal) 4. Both options shown on all devices (desktop and mobile) ## Acceptance Criteria - "Add Receipt" button replaces "Scan Receipt" - Full-screen dialog opens with two clear option boxes - Drag-and-drop/browse upload is the primary action - "Take Photo" option opens camera modal only when clicked - Uploaded and captured images both go through OCR extraction - Review modal works the same for both input methods - Tested on mobile (320px, 768px) and desktop (1920px)
egullickson added the
status
in-progress
type
chore
 labels 2026-02-14 02:31:24 +00:00
egullickson added this to the Sprint 2026-02-02 milestone 2026-02-14 02:31:28 +00:00
egullickson commented 2026-02-14 02:40:42 +00:00
Author
Owner
Copy Link
Copy Source

Plan: Change Maintenance Receipt Flow to Upload-First

Phase: Planning | Agent: Planner | Status: AWAITING_REVIEW (Rev 3 -- QR plan-docs fixes)

Context

The maintenance receipt scanning feature currently mirrors the fuel log receipt flow (camera-first). Users click "Scan Receipt" which immediately opens the camera. File upload is buried as a fallback inside the camera screen when permission is denied. This should be changed to an upload-first pattern where drag-and-drop is the primary option and camera is a secondary choice.

Codebase Analysis Summary

Current flow: ReceiptCameraButton ("Scan Receipt") -> startCapture() -> fullscreen CameraCapture Dialog -> processImage(file, croppedFile) -> MaintenanceReceiptReviewModal -> form population via setValue()

Key finding: The hook's processImage(file, croppedFile?) already handles both paths -- croppedFile is optional (imageToProcess = croppedFile || file inside processImage). No hook or backend changes are needed.

Files analyzed (7 files, all read in full):

  • MaintenanceRecordForm.tsx -- Form with ReceiptCameraButton and Camera Dialog
  • useMaintenanceReceiptOcr.ts -- OCR orchestration hook
  • FileInputFallback.tsx -- Standalone drag-drop component
  • CameraCapture.tsx -- Camera with internal FileInputFallback fallback
  • ReceiptCameraButton.tsx -- Shared button (fuel-logs + maintenance)
  • maintenance-receipt.types.ts -- OCR types
  • CameraCapture/types.ts -- Camera component types

Architecture Decision

Build a new AddReceiptDialog component with inline drag-and-drop logic rather than embedding FileInputFallback inside it. Rationale: FileInputFallback has its own header ("Upload Image" + Cancel button) designed for full-screen standalone use. The new dialog needs both upload and camera options visible simultaneously with a single shared header.

ReceiptCameraButton is shared with fuel-logs and will NOT be modified. Instead, it will be replaced with a standard MUI Button in the maintenance form only.

Sub-Issues

# Sub-Issue File(s) Milestone
#183 Create AddReceiptDialog component NEW: AddReceiptDialog.tsx M1
#184 Integrate into MaintenanceRecordForm MODIFY: MaintenanceRecordForm.tsx, MODIFY: maintenance/CLAUDE.md M2
#185 Viewport testing (320px, 768px, 1920px) Testing only M3

Milestone 1: Create AddReceiptDialog Component (refs #183)

New file: frontend/src/features/maintenance/components/AddReceiptDialog.tsx

Inline documentation (required JSDoc header):

/**
 * @ai-summary Upload-first dialog for maintenance receipt input with drag-drop and camera options
 * @ai-context Replaces camera-first ReceiptCameraButton pattern. Primary option is drag-drop file upload,
 *   secondary is camera capture. Both paths call parent callbacks that feed into useMaintenanceReceiptOcr.processImage().
 */

Component API:

interface AddReceiptDialogProps {
  open: boolean;
  onClose: () => void;
  onFileSelect: (file: File) => void;
  onStartCamera: () => void;
}

Constants (inline in component file):

const ACCEPTED_FORMATS = ['image/jpeg', 'image/png', 'image/heic', 'image/heif'];
const MAX_FILE_SIZE = 10 * 1024 * 1024; // 10MB
const ACCEPT_STRING = ACCEPTED_FORMATS.join(',');

Layout (fullScreen MUI Dialog):

+----------------------------------+
| Add Receipt              [Close] |  <- AppBar/Toolbar header
+----------------------------------+
|                                  |
|  +----------------------------+  |
|  |    [Cloud Upload Icon]     |  |  <- Primary: Drag-drop zone
|  |                            |  |     (dashed border, click-to-browse)
|  |  Drag and drop an image,   |  |
|  |  or click to browse        |  |
|  |                            |  |
|  |  JPEG, PNG, HEIC - 10MB   |  |
|  +----------------------------+  |
|                                  |
|         ---- or ----             |  <- Divider with "or" text
|                                  |
|  +----------------------------+  |
|  | [Camera Icon]              |  |  <- Secondary: Camera option
|  | Take Photo of Receipt      |  |     (outlined card/button, 44px min)
|  +----------------------------+  |
|                                  |
+----------------------------------+

Implementation details:

  • Fullscreen Dialog with Paper background (not black like camera)
  • Inline drag-drop handlers (dragOver, dragLeave, drop events)
  • Hidden <input type="file" accept={ACCEPT_STRING}> for click-to-browse
  • File validation function matching FileInputFallback pattern:
    • Check MIME type against ACCEPTED_FORMATS (with HEIC/HEIF extension fallback for inconsistent MIME types: check file.name.toLowerCase().endsWith('.heic') or .heif)
    • Check file size against MAX_FILE_SIZE
    • Show inline <Alert severity="error"> for invalid files
  • Mobile: "Choose Image" button visible below drop zone on xs breakpoint (display: { xs: 'block', sm: 'none' })
  • All interactive elements: minHeight: 44 for touch targets
  • "or" divider between the two options (MUI <Divider> with <Typography> child)

Milestone 2: Integrate into MaintenanceRecordForm (refs #184)

Modified file: frontend/src/features/maintenance/components/MaintenanceRecordForm.tsx

Changes:

  1. Imports:

    • Remove: import { ReceiptCameraButton } from '../../fuel-logs/components/ReceiptCameraButton';
    • Add: import { AddReceiptDialog } from './AddReceiptDialog';
    • Add: import ReceiptIcon from '@mui/icons-material/Receipt';

  2. State (add after the capturedReceiptFile useState declaration):

    • Add: const [showAddReceiptDialog, setShowAddReceiptDialog] = useState(false);

  3. Replace the ReceiptCameraButton JSX block (the <Box> wrapper containing <ReceiptCameraButton> inside the {/* Receipt Scan Button */} comment):

    • Remove the entire <Box sx={{ display: 'flex', justifyContent: 'center', mb: 3, pb: 2, ... }}><ReceiptCameraButton ... /></Box> block
    • Replace with:
    <Box
  sx={{
    display: 'flex',
    justifyContent: 'center',
    mb: 3,
    pb: 2,
    borderBottom: '1px solid',
    borderColor: 'divider',
  }}
>
  <Button
    onClick={() => {
      if (!hasReceiptScanAccess) {
        setShowUpgradeDialog(true);
        return;
      }
      setShowAddReceiptDialog(true);
    }}
    disabled={isProcessing || isRecordMutating}
    variant="outlined"
    color="primary"
    startIcon={<ReceiptIcon />}
    sx={{
      minHeight: 44,
      borderStyle: 'dashed',
      '&:hover': {
        borderStyle: 'solid',
      },
    }}
  >
    Add Receipt
  </Button>
</Box>

  4. Add AddReceiptDialog (insert after the {/* Camera Capture Modal */} Dialog block, before the {/* OCR Processing Overlay */} Backdrop):

    {/* Add Receipt Dialog */}
<AddReceiptDialog
  open={showAddReceiptDialog}
  onClose={() => setShowAddReceiptDialog(false)}
  onFileSelect={(file) => {
    setShowAddReceiptDialog(false);
    handleCaptureImage(file);
  }}
  onStartCamera={() => {
    setShowAddReceiptDialog(false);
    startCapture();
  }}
/>

  5. Keep unchanged (all existing modals/overlays remain as-is):

    • Camera Capture Dialog (<Dialog open={isCapturing}>) -- still needed for "Take Photo" path
    • OCR Processing Backdrop (<Backdrop open={isProcessing}>)
    • MaintenanceReceiptReviewModal ({ocrResult && <MaintenanceReceiptReviewModal ... />})
    • UpgradeRequiredDialog (<UpgradeRequiredDialog featureKey="maintenance.receiptScan" ... />)
    • OCR Error Dialog ({ocrError && <Dialog ... />})

  6. Update CLAUDE.md -- Add new entry to frontend/src/features/maintenance/CLAUDE.md Key Files table:

    | `components/AddReceiptDialog.tsx` | Upload-first receipt input dialog with drag-drop and camera options | Receipt entry UI, OCR flow |

Data flow after changes:

"Add Receipt" Button
    |
    v
Tier check: hasReceiptScanAccess?
    |-- NO: UpgradeRequiredDialog
    |-- YES: showAddReceiptDialog = true
              |
              v
        AddReceiptDialog
         /           \
   Upload             Camera
   (onFileSelect)     (onStartCamera)
        |                   |
        v                   v
  handleCaptureImage   startCapture()
  (file)               -> isCapturing=true
        |                   |
        |                   v
        |             CameraCapture Dialog
        |             -> onCapture(file, croppedFile)
        |                   |
        |                   v
        |             handleCaptureImage
        |             (file, croppedFile)
        |                   |
        +-------------------+
                |
                v
        processImage(file, croppedFile?)
                |
                v
        POST /api/ocr/extract/maintenance-receipt
                |
                v
        MaintenanceReceiptReviewModal
                |
                v
        handleAcceptOcrResult -> form setValue()

Milestone 3: Viewport Testing (refs #185)

Test at three breakpoints using browser automation:

  • 320px (mobile phone): Both options stacked, touch targets 44px+, "Choose Image" button visible
  • 768px (tablet): Both options stacked, adequate spacing
  • 1920px (desktop): Both options stacked, drag-drop zone prominent, no excessive whitespace

Verify:

  • Upload path: drop/browse file -> OCR processing backdrop -> review modal -> form populated
  • Camera path: "Take Photo" -> CameraCapture -> capture -> review modal -> form populated
  • Error states: invalid file type, file too large, OCR failure

Files NOT Changed

File Reason
useMaintenanceReceiptOcr.ts processImage(file, croppedFile?) already handles both paths
FileInputFallback.tsx Not embedded; drag-drop logic implemented inline in AddReceiptDialog
CameraCapture.tsx Used as-is for camera path
ReceiptCameraButton.tsx Shared with fuel-logs; not modified
maintenance-receipt.types.ts No new types needed
Backend/OCR endpoints Same pipeline for both paths

Risk Assessment

  • Low risk: Pure frontend UI restructure
  • No regression: Fuel-logs receipt flow completely unaffected
  • Same OCR pipeline: Both paths use identical processImage() -> review -> form population
  • Tier gating preserved: Check happens before dialog opens

Branch and PR

  • Branch: issue-182-maintenance-receipt-upload-first (from main)
  • PR title: chore: change maintenance receipt flow to upload-first (#182)
  • PR body: Fixes #182, Fixes #183, Fixes #184, Fixes #185

Verdict: AWAITING_REVIEW | Next: Plan review cycle (QR plan-completeness -> TW plan-scrub -> QR plan-code -> QR plan-docs)

## Plan: Change Maintenance Receipt Flow to Upload-First **Phase**: Planning | **Agent**: Planner | **Status**: AWAITING_REVIEW (Rev 3 -- QR plan-docs fixes) ### Context The maintenance receipt scanning feature currently mirrors the fuel log receipt flow (camera-first). Users click "Scan Receipt" which immediately opens the camera. File upload is buried as a fallback inside the camera screen when permission is denied. This should be changed to an upload-first pattern where drag-and-drop is the primary option and camera is a secondary choice. ### Codebase Analysis Summary **Current flow**: `ReceiptCameraButton` ("Scan Receipt") -> `startCapture()` -> fullscreen `CameraCapture` Dialog -> `processImage(file, croppedFile)` -> `MaintenanceReceiptReviewModal` -> form population via `setValue()` **Key finding**: The hook's `processImage(file, croppedFile?)` already handles both paths -- `croppedFile` is optional (`imageToProcess = croppedFile || file` inside `processImage`). No hook or backend changes are needed. **Files analyzed** (7 files, all read in full): - `MaintenanceRecordForm.tsx` -- Form with ReceiptCameraButton and Camera Dialog - `useMaintenanceReceiptOcr.ts` -- OCR orchestration hook - `FileInputFallback.tsx` -- Standalone drag-drop component - `CameraCapture.tsx` -- Camera with internal FileInputFallback fallback - `ReceiptCameraButton.tsx` -- Shared button (fuel-logs + maintenance) - `maintenance-receipt.types.ts` -- OCR types - `CameraCapture/types.ts` -- Camera component types ### Architecture Decision Build a new `AddReceiptDialog` component with inline drag-and-drop logic rather than embedding `FileInputFallback` inside it. Rationale: `FileInputFallback` has its own header ("Upload Image" + Cancel button) designed for full-screen standalone use. The new dialog needs both upload and camera options visible simultaneously with a single shared header. `ReceiptCameraButton` is shared with fuel-logs and will NOT be modified. Instead, it will be replaced with a standard MUI Button in the maintenance form only. ### Sub-Issues | # | Sub-Issue | File(s) | Milestone | |---|-----------|---------|-----------| | #183 | Create AddReceiptDialog component | NEW: `AddReceiptDialog.tsx` | M1 | | #184 | Integrate into MaintenanceRecordForm | MODIFY: `MaintenanceRecordForm.tsx`, MODIFY: `maintenance/CLAUDE.md` | M2 | | #185 | Viewport testing (320px, 768px, 1920px) | Testing only | M3 | ### Milestone 1: Create AddReceiptDialog Component (refs #183) **New file**: `frontend/src/features/maintenance/components/AddReceiptDialog.tsx` **Inline documentation** (required JSDoc header): ```typescript /** * @ai-summary Upload-first dialog for maintenance receipt input with drag-drop and camera options * @ai-context Replaces camera-first ReceiptCameraButton pattern. Primary option is drag-drop file upload, * secondary is camera capture. Both paths call parent callbacks that feed into useMaintenanceReceiptOcr.processImage(). */ ``` **Component API**: ```typescript interface AddReceiptDialogProps { open: boolean; onClose: () => void; onFileSelect: (file: File) => void; onStartCamera: () => void; } ``` **Constants** (inline in component file): ```typescript const ACCEPTED_FORMATS = ['image/jpeg', 'image/png', 'image/heic', 'image/heif']; const MAX_FILE_SIZE = 10 * 1024 * 1024; // 10MB const ACCEPT_STRING = ACCEPTED_FORMATS.join(','); ``` **Layout** (fullScreen MUI Dialog): ``` +----------------------------------+ | Add Receipt [Close] | <- AppBar/Toolbar header +----------------------------------+ | | | +----------------------------+ | | | [Cloud Upload Icon] | | <- Primary: Drag-drop zone | | | | (dashed border, click-to-browse) | | Drag and drop an image, | | | | or click to browse | | | | | | | | JPEG, PNG, HEIC - 10MB | | | +----------------------------+ | | | | ---- or ---- | <- Divider with "or" text | | | +----------------------------+ | | | [Camera Icon] | | <- Secondary: Camera option | | Take Photo of Receipt | | (outlined card/button, 44px min) | +----------------------------+ | | | +----------------------------------+ ``` **Implementation details**: - Fullscreen Dialog with `Paper` background (not black like camera) - Inline drag-drop handlers (dragOver, dragLeave, drop events) - Hidden `<input type="file" accept={ACCEPT_STRING}>` for click-to-browse - File validation function matching `FileInputFallback` pattern: - Check MIME type against `ACCEPTED_FORMATS` (with HEIC/HEIF extension fallback for inconsistent MIME types: check `file.name.toLowerCase().endsWith('.heic')` or `.heif`) - Check file size against `MAX_FILE_SIZE` - Show inline `<Alert severity="error">` for invalid files - Mobile: "Choose Image" button visible below drop zone on `xs` breakpoint (`display: { xs: 'block', sm: 'none' }`) - All interactive elements: `minHeight: 44` for touch targets - "or" divider between the two options (MUI `<Divider>` with `<Typography>` child) ### Milestone 2: Integrate into MaintenanceRecordForm (refs #184) **Modified file**: `frontend/src/features/maintenance/components/MaintenanceRecordForm.tsx` **Changes**: 1. **Imports**: - Remove: `import { ReceiptCameraButton } from '../../fuel-logs/components/ReceiptCameraButton';` - Add: `import { AddReceiptDialog } from './AddReceiptDialog';` - Add: `import ReceiptIcon from '@mui/icons-material/Receipt';` 2. **State** (add after the `capturedReceiptFile` useState declaration): - Add: `const [showAddReceiptDialog, setShowAddReceiptDialog] = useState(false);` 3. **Replace the `ReceiptCameraButton` JSX block** (the `<Box>` wrapper containing `<ReceiptCameraButton>` inside the `{/* Receipt Scan Button */}` comment): - Remove the entire `<Box sx={{ display: 'flex', justifyContent: 'center', mb: 3, pb: 2, ... }}><ReceiptCameraButton ... /></Box>` block - Replace with: ```tsx <Box sx={{ display: 'flex', justifyContent: 'center', mb: 3, pb: 2, borderBottom: '1px solid', borderColor: 'divider', }} > <Button onClick={() => { if (!hasReceiptScanAccess) { setShowUpgradeDialog(true); return; } setShowAddReceiptDialog(true); }} disabled={isProcessing || isRecordMutating} variant="outlined" color="primary" startIcon={<ReceiptIcon />} sx={{ minHeight: 44, borderStyle: 'dashed', '&:hover': { borderStyle: 'solid', }, }} > Add Receipt </Button> </Box> ``` 4. **Add AddReceiptDialog** (insert after the `{/* Camera Capture Modal */}` Dialog block, before the `{/* OCR Processing Overlay */}` Backdrop): ```tsx {/* Add Receipt Dialog */} <AddReceiptDialog open={showAddReceiptDialog} onClose={() => setShowAddReceiptDialog(false)} onFileSelect={(file) => { setShowAddReceiptDialog(false); handleCaptureImage(file); }} onStartCamera={() => { setShowAddReceiptDialog(false); startCapture(); }} /> ``` 5. **Keep unchanged** (all existing modals/overlays remain as-is): - Camera Capture Dialog (`<Dialog open={isCapturing}>`) -- still needed for "Take Photo" path - OCR Processing Backdrop (`<Backdrop open={isProcessing}>`) - MaintenanceReceiptReviewModal (`{ocrResult && <MaintenanceReceiptReviewModal ... />}`) - UpgradeRequiredDialog (`<UpgradeRequiredDialog featureKey="maintenance.receiptScan" ... />`) - OCR Error Dialog (`{ocrError && <Dialog ... />}`) 6. **Update CLAUDE.md** -- Add new entry to `frontend/src/features/maintenance/CLAUDE.md` Key Files table: ```markdown | `components/AddReceiptDialog.tsx` | Upload-first receipt input dialog with drag-drop and camera options | Receipt entry UI, OCR flow | ``` **Data flow after changes**: ``` "Add Receipt" Button | v Tier check: hasReceiptScanAccess? |-- NO: UpgradeRequiredDialog |-- YES: showAddReceiptDialog = true | v AddReceiptDialog / \ Upload Camera (onFileSelect) (onStartCamera) | | v v handleCaptureImage startCapture() (file) -> isCapturing=true | | | v | CameraCapture Dialog | -> onCapture(file, croppedFile) | | | v | handleCaptureImage | (file, croppedFile) | | +-------------------+ | v processImage(file, croppedFile?) | v POST /api/ocr/extract/maintenance-receipt | v MaintenanceReceiptReviewModal | v handleAcceptOcrResult -> form setValue() ``` ### Milestone 3: Viewport Testing (refs #185) Test at three breakpoints using browser automation: - **320px** (mobile phone): Both options stacked, touch targets 44px+, "Choose Image" button visible - **768px** (tablet): Both options stacked, adequate spacing - **1920px** (desktop): Both options stacked, drag-drop zone prominent, no excessive whitespace Verify: - Upload path: drop/browse file -> OCR processing backdrop -> review modal -> form populated - Camera path: "Take Photo" -> CameraCapture -> capture -> review modal -> form populated - Error states: invalid file type, file too large, OCR failure ### Files NOT Changed | File | Reason | |------|--------| | `useMaintenanceReceiptOcr.ts` | `processImage(file, croppedFile?)` already handles both paths | | `FileInputFallback.tsx` | Not embedded; drag-drop logic implemented inline in AddReceiptDialog | | `CameraCapture.tsx` | Used as-is for camera path | | `ReceiptCameraButton.tsx` | Shared with fuel-logs; not modified | | `maintenance-receipt.types.ts` | No new types needed | | Backend/OCR endpoints | Same pipeline for both paths | ### Risk Assessment - **Low risk**: Pure frontend UI restructure - **No regression**: Fuel-logs receipt flow completely unaffected - **Same OCR pipeline**: Both paths use identical `processImage()` -> review -> form population - **Tier gating preserved**: Check happens before dialog opens ### Branch and PR - **Branch**: `issue-182-maintenance-receipt-upload-first` (from main) - **PR title**: `chore: change maintenance receipt flow to upload-first (#182)` - **PR body**: `Fixes #182`, `Fixes #183`, `Fixes #184`, `Fixes #185` --- *Verdict*: AWAITING_REVIEW | *Next*: Plan review cycle (QR plan-completeness -> TW plan-scrub -> QR plan-code -> QR plan-docs)
egullickson commented 2026-02-14 02:41:48 +00:00
Author
Owner
Copy Link
Copy Source

QR Review: Plan Completeness

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

Checklist

  • Acceptance criteria coverage: Every AC from issue #182 is addressed by at least one milestone
  • Sub-issue mapping: Each milestone maps 1:1 to a sub-issue
  • File scope: All files to create/modify/delete are listed
  • No orphaned work: No milestone produces work that isn't connected to the data flow
  • Testing coverage: Viewport testing plan covers mobile (320px, 768px) and desktop (1920px)
  • Risk assessment: Risks identified and mitigated
  • Dependencies: Milestone ordering is correct (M1 before M2 before M3)

Findings

No issues found. The plan demonstrates:

Acceptance criteria coverage: All 7 ACs explicitly mapped to milestones (M1: dialog structure with two options, M2: button replacement + integration, M3: viewport testing)

Sub-issue decomposition: Clean 1:1 mapping (M1→#183, M2→#184, M3→#185) following workflow-contract.json pattern

File scope completeness: CREATE: AddReceiptDialog.tsx | MODIFY: MaintenanceRecordForm.tsx | DELETE: none | NOT_CHANGED: 6 files with rationale (processImage already handles both paths, ReceiptCameraButton shared with fuel-logs)

Data flow integrity: Upload path and camera path both converge at processImage() → review modal → form population. No orphaned work.

Testing rigor: M3 explicitly covers three viewports (320px, 768px, 1920px) with functional validation for both upload and camera paths plus error states

Risk mitigation: Low-risk pure frontend change, fuel-logs flow unaffected (ReceiptCameraButton untouched), same OCR pipeline preserved

Dependency ordering: M1 (create component) → M2 (integrate) → M3 (test) is logically sound

Verdict

The plan is structurally complete and ready for technical writing review. All acceptance criteria are covered, sub-issues are properly scoped, file changes are explicit, data flow is coherent, and testing is comprehensive.

Verdict: PASS | Next: TW plan-scrub

## QR Review: Plan Completeness **Phase**: Plan-Review | **Agent**: Quality Reviewer | **Status**: PASS ### Checklist - [x] Acceptance criteria coverage: Every AC from issue #182 is addressed by at least one milestone - [x] Sub-issue mapping: Each milestone maps 1:1 to a sub-issue - [x] File scope: All files to create/modify/delete are listed - [x] No orphaned work: No milestone produces work that isn't connected to the data flow - [x] Testing coverage: Viewport testing plan covers mobile (320px, 768px) and desktop (1920px) - [x] Risk assessment: Risks identified and mitigated - [x] Dependencies: Milestone ordering is correct (M1 before M2 before M3) ### Findings No issues found. The plan demonstrates: **Acceptance criteria coverage**: All 7 ACs explicitly mapped to milestones (M1: dialog structure with two options, M2: button replacement + integration, M3: viewport testing) **Sub-issue decomposition**: Clean 1:1 mapping (M1→#183, M2→#184, M3→#185) following workflow-contract.json pattern **File scope completeness**: CREATE: AddReceiptDialog.tsx | MODIFY: MaintenanceRecordForm.tsx | DELETE: none | NOT_CHANGED: 6 files with rationale (processImage already handles both paths, ReceiptCameraButton shared with fuel-logs) **Data flow integrity**: Upload path and camera path both converge at processImage() → review modal → form population. No orphaned work. **Testing rigor**: M3 explicitly covers three viewports (320px, 768px, 1920px) with functional validation for both upload and camera paths plus error states **Risk mitigation**: Low-risk pure frontend change, fuel-logs flow unaffected (ReceiptCameraButton untouched), same OCR pipeline preserved **Dependency ordering**: M1 (create component) → M2 (integrate) → M3 (test) is logically sound ### Verdict The plan is structurally complete and ready for technical writing review. All acceptance criteria are covered, sub-issues are properly scoped, file changes are explicit, data flow is coherent, and testing is comprehensive. *Verdict*: PASS | *Next*: TW plan-scrub
egullickson commented 2026-02-14 02:43:20 +00:00
Author
Owner
Copy Link
Copy Source

TW Review: Plan Scrub

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

Checklist

  • Clarity: Plan is clear and unambiguous; another developer could implement from this plan alone
  • Temporal contamination: Plan contains 11 line-number references that will break after any edit
  • Terminology consistency: Component names, file paths, and technical terms used consistently
  • Completeness of instructions: Two minor gaps in AddReceiptDialog implementation details
  • Grammar and formatting: Markdown well-structured; tables, code blocks, and ASCII diagrams formatted correctly
  • Audience appropriateness: Written for an AI developer agent with appropriate detail level

Findings

1. Temporal Contamination (FAIL -- blocking)

The plan contains 11 explicit line-number references across Milestone 2 and the Codebase Analysis Summary. These are location directives that violate the Timeless Present Rule and will become stale as soon as Milestone 1 modifies any imports or adds code above the referenced sections.

Specific instances in Milestone 2:

Contaminated Reference Suggested Structural Anchor
"lines 43-44" (imports) "Remove the ReceiptCameraButton import from the fuel-logs imports block"
"after line 96" (state) "Add state alongside the existing capturedReceiptFile state declaration"
"lines 249-271" (button block) "Replace the <ReceiptCameraButton> JSX block and its wrapping <Box> (inside CardContent, before the <form> tag)"
"~line 525" (dialog placement) "After the existing Camera Capture <Dialog> block"
"lines 511-525", "lines 527-539", "lines 542-556", "lines 558-563", "lines 565-591" (Keep unchanged) Reference by component name only: "Camera Dialog", "OCR Processing Backdrop", "MaintenanceReceiptReviewModal", "UpgradeRequiredDialog", "OCR Error Dialog"

Codebase Analysis Summary:

Contaminated Reference Fix
"line 237: imageToProcess = croppedFile || file" "processImage uses croppedFile || file fallback internally"

The line numbers are currently accurate against the codebase, but they are fragile. Structural anchors (component names, variable names, JSX patterns) are resilient to code shifts and should replace all line-number references.

2. Completeness Gaps (non-blocking, advisory)

a) File validation constants not specified for AddReceiptDialog:
The plan says "accepted formats (JPEG, PNG, HEIC/HEIF with extension fallback), max 10MB" but does not provide the MIME type strings or max-size constant. The existing FileInputFallback receives these as props. Since AddReceiptDialog implements inline drag-drop, the developer must know the exact values.

Suggestion: Add to Milestone 1 implementation details:

Accepted MIME types: ['image/jpeg', 'image/png', 'image/heic', 'image/heif']
Extension fallback: .heic, .heif (for browsers that report empty MIME)
Max file size: 10 * 1024 * 1024 (10MB)

b) Hidden input accept attribute not specified:
The hidden <input type="file"> needs an accept attribute string. Suggest adding: accept="image/jpeg,image/png,image/heic,image/heif,.heic,.heif"

Verdict

FAIL -- The 11 line-number references constitute temporal contamination that will produce incorrect instructions after even minor edits. All line-number references must be replaced with structural anchors (component names, JSX patterns, variable declarations). The completeness gaps are advisory and can be addressed at the same time.

Verdict: FAIL | Next: Revise plan to replace all line-number references with structural anchors, then re-submit for TW plan-scrub

## TW Review: Plan Scrub **Phase**: Plan-Review | **Agent**: Technical Writer | **Status**: FAIL ### Checklist - [x] Clarity: Plan is clear and unambiguous; another developer could implement from this plan alone - [ ] Temporal contamination: Plan contains 11 line-number references that will break after any edit - [x] Terminology consistency: Component names, file paths, and technical terms used consistently - [ ] Completeness of instructions: Two minor gaps in AddReceiptDialog implementation details - [x] Grammar and formatting: Markdown well-structured; tables, code blocks, and ASCII diagrams formatted correctly - [x] Audience appropriateness: Written for an AI developer agent with appropriate detail level ### Findings #### 1. Temporal Contamination (FAIL -- blocking) The plan contains **11 explicit line-number references** across Milestone 2 and the Codebase Analysis Summary. These are location directives that violate the Timeless Present Rule and will become stale as soon as Milestone 1 modifies any imports or adds code above the referenced sections. **Specific instances in Milestone 2:** | Contaminated Reference | Suggested Structural Anchor | |---|---| | "lines 43-44" (imports) | "Remove the `ReceiptCameraButton` import from the fuel-logs imports block" | | "after line 96" (state) | "Add state alongside the existing `capturedReceiptFile` state declaration" | | "lines 249-271" (button block) | "Replace the `<ReceiptCameraButton>` JSX block and its wrapping `<Box>` (inside `CardContent`, before the `<form>` tag)" | | "~line 525" (dialog placement) | "After the existing Camera Capture `<Dialog>` block" | | "lines 511-525", "lines 527-539", "lines 542-556", "lines 558-563", "lines 565-591" (Keep unchanged) | Reference by component name only: "Camera Dialog", "OCR Processing Backdrop", "MaintenanceReceiptReviewModal", "UpgradeRequiredDialog", "OCR Error Dialog" | **Codebase Analysis Summary:** | Contaminated Reference | Fix | |---|---| | "line 237: `imageToProcess = croppedFile \|\| file`" | "`processImage` uses `croppedFile \|\| file` fallback internally" | The line numbers are currently accurate against the codebase, but they are fragile. Structural anchors (component names, variable names, JSX patterns) are resilient to code shifts and should replace all line-number references. #### 2. Completeness Gaps (non-blocking, advisory) **a) File validation constants not specified for AddReceiptDialog:** The plan says "accepted formats (JPEG, PNG, HEIC/HEIF with extension fallback), max 10MB" but does not provide the MIME type strings or max-size constant. The existing `FileInputFallback` receives these as props. Since AddReceiptDialog implements inline drag-drop, the developer must know the exact values. Suggestion: Add to Milestone 1 implementation details: ``` Accepted MIME types: ['image/jpeg', 'image/png', 'image/heic', 'image/heif'] Extension fallback: .heic, .heif (for browsers that report empty MIME) Max file size: 10 * 1024 * 1024 (10MB) ``` **b) Hidden input `accept` attribute not specified:** The hidden `<input type="file">` needs an `accept` attribute string. Suggest adding: `accept="image/jpeg,image/png,image/heic,image/heif,.heic,.heif"` ### Verdict **FAIL** -- The 11 line-number references constitute temporal contamination that will produce incorrect instructions after even minor edits. All line-number references must be replaced with structural anchors (component names, JSX patterns, variable declarations). The completeness gaps are advisory and can be addressed at the same time. *Verdict*: FAIL | *Next*: Revise plan to replace all line-number references with structural anchors, then re-submit for TW plan-scrub
egullickson commented 2026-02-14 02:45:45 +00:00
Author
Owner
Copy Link
Copy Source

QR Review: Plan Docs

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

Checklist

  • CLAUDE.md updates: The new AddReceiptDialog component is not documented in frontend/src/features/maintenance/CLAUDE.md
  • Feature README: backend/src/features/maintenance/README.md does not need updates (pure frontend change)
  • Inline documentation: Plan does not specify @ai-summary and @ai-context JSDoc comments for AddReceiptDialog.tsx
  • Data flow documentation: Complete data flow diagram provided in Milestone 2
  • No emoji: Plan adheres to professional documentation standards

Findings

1. Missing CLAUDE.md Update (FAIL - blocking)

Issue: The plan creates a new component AddReceiptDialog.tsx but does not specify updating frontend/src/features/maintenance/CLAUDE.md to document it.

Evidence: The current CLAUDE.md has a "Key Files" section listing important components like:

  • MaintenanceRecordForm.tsx - Form for manual record creation
  • MaintenanceScheduleReviewScreen.tsx - Dialog for reviewing OCR-extracted maintenance items
  • SubtypeCheckboxGroup.tsx - Multi-select checkbox group

Why this matters: AddReceiptDialog is a significant user-facing component that changes the primary receipt entry flow. Future AI agents working on maintenance receipt features need to know this component exists and what it does.

Suggested fix: Add to Milestone 2 (after integration is complete):

Update frontend/src/features/maintenance/CLAUDE.md Key Files section:

| `components/AddReceiptDialog.tsx` | Upload-first dialog for maintenance receipt scanning: drag-drop upload zone, camera option, file validation | Receipt entry UI |

2. Missing Inline Documentation Specification (FAIL - blocking)

Issue: The plan does not specify adding JSDoc @ai-summary and @ai-context comments to the new AddReceiptDialog.tsx file.

Evidence: Project pattern verified in multiple files (FileInputFallback.tsx, MaintenanceRecordForm.tsx, etc.):

/**
 * @ai-summary Brief description
 * @ai-context Additional context for AI agents
 */

Why this matters: Inline documentation is a project standard for helping AI agents quickly understand component purpose without reading full implementation. Missing this violates the "AI Context Efficiency" principle in CLAUDE.md.

Suggested fix: Add to Milestone 1 implementation details:

Add JSDoc header to AddReceiptDialog.tsx:

/**
 * @ai-summary Upload-first dialog for maintenance receipt scanning
 * @ai-context Provides drag-drop upload zone as primary option, camera as secondary. Validates file type/size, calls onFileSelect for upload path or onStartCamera for camera path.
 */

Verdict

The plan has comprehensive data flow documentation and follows the no-emoji rule, but it lacks two critical documentation specifications: CLAUDE.md update for the new component and inline JSDoc comments. Both are project standards for AI context efficiency.

Verdict: FAIL | Next: Revise plan to add CLAUDE.md update step and inline documentation specification

## QR Review: Plan Docs **Phase**: Plan-Review | **Agent**: Quality Reviewer | **Status**: FAIL ### Checklist - [ ] CLAUDE.md updates: The new AddReceiptDialog component is not documented in frontend/src/features/maintenance/CLAUDE.md - [x] Feature README: backend/src/features/maintenance/README.md does not need updates (pure frontend change) - [ ] Inline documentation: Plan does not specify @ai-summary and @ai-context JSDoc comments for AddReceiptDialog.tsx - [x] Data flow documentation: Complete data flow diagram provided in Milestone 2 - [x] No emoji: Plan adheres to professional documentation standards ### Findings #### 1. Missing CLAUDE.md Update (FAIL - blocking) **Issue**: The plan creates a new component `AddReceiptDialog.tsx` but does not specify updating `frontend/src/features/maintenance/CLAUDE.md` to document it. **Evidence**: The current CLAUDE.md has a "Key Files" section listing important components like: - `MaintenanceRecordForm.tsx` - Form for manual record creation - `MaintenanceScheduleReviewScreen.tsx` - Dialog for reviewing OCR-extracted maintenance items - `SubtypeCheckboxGroup.tsx` - Multi-select checkbox group **Why this matters**: AddReceiptDialog is a significant user-facing component that changes the primary receipt entry flow. Future AI agents working on maintenance receipt features need to know this component exists and what it does. **Suggested fix**: Add to Milestone 2 (after integration is complete): Update `frontend/src/features/maintenance/CLAUDE.md` Key Files section: ```markdown | `components/AddReceiptDialog.tsx` | Upload-first dialog for maintenance receipt scanning: drag-drop upload zone, camera option, file validation | Receipt entry UI | ``` #### 2. Missing Inline Documentation Specification (FAIL - blocking) **Issue**: The plan does not specify adding JSDoc `@ai-summary` and `@ai-context` comments to the new AddReceiptDialog.tsx file. **Evidence**: Project pattern verified in multiple files (FileInputFallback.tsx, MaintenanceRecordForm.tsx, etc.): ```typescript /** * @ai-summary Brief description * @ai-context Additional context for AI agents */ ``` **Why this matters**: Inline documentation is a project standard for helping AI agents quickly understand component purpose without reading full implementation. Missing this violates the "AI Context Efficiency" principle in CLAUDE.md. **Suggested fix**: Add to Milestone 1 implementation details: Add JSDoc header to AddReceiptDialog.tsx: ```typescript /** * @ai-summary Upload-first dialog for maintenance receipt scanning * @ai-context Provides drag-drop upload zone as primary option, camera as secondary. Validates file type/size, calls onFileSelect for upload path or onStartCamera for camera path. */ ``` ### Verdict The plan has comprehensive data flow documentation and follows the no-emoji rule, but it lacks two critical documentation specifications: CLAUDE.md update for the new component and inline JSDoc comments. Both are project standards for AI context efficiency. *Verdict*: FAIL | *Next*: Revise plan to add CLAUDE.md update step and inline documentation specification
egullickson commented 2026-02-14 02:46:02 +00:00
Author
Owner
Copy Link
Copy Source

QR Review: Plan Code (RULE 0/1/2)

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

RULE 0 (CRITICAL) - Production Reliability

No RULE 0 violations found.

Verified:

  • File validation edge cases: Plan specifies file validation function matching FileInputFallback pattern with MIME type checking against ACCEPTED_FORMATS array, HEIC/HEIF extension fallback for inconsistent MIME types, file size validation against MAX_FILE_SIZE (10MB), and inline Alert severity="error" for invalid files (M1 implementation details)
  • Resource cleanup: Blob URL cleanup is handled by existing useMaintenanceReceiptOcr hook. The processImage() function at line 238 creates blob URL with URL.createObjectURL(imageToProcess), stores it in receiptImageUrl state. Cleanup happens in acceptResult() at line 301 and reset() at line 312 via URL.revokeObjectURL(receiptImageUrl). The plan preserves this existing pattern - no new blob URLs created by AddReceiptDialog
  • Error handling: File validation errors displayed via inline Alert component. OCR processing errors handled by existing hook (processImage try/catch at lines 241-262 sets error state, displayed by existing "OCR Error Dialog" in MaintenanceRecordForm). Upload path and camera path both converge at processImage() which has comprehensive error handling
  • Security: File type validation prevents arbitrary file uploads. MIME type checking with extension fallback (HEIC/HEIF) prevents MIME spoofing. Size limit (10MB) prevents resource exhaustion

RULE 1 (HIGH) - Project Conformance

No RULE 1 violations found.

Verified:

  • Mobile + desktop requirement: Plan explicitly addresses both viewports in M3 testing (320px mobile, 768px tablet, 1920px desktop). Mobile-specific implementation includes "Choose Image" button with display: { xs: 'block', sm: 'none' } visible below drop zone on xs breakpoint. Touch targets specified as minHeight: 44 for all interactive elements matching project standard (44px minimum per CLAUDE.md)
  • Naming conventions: Component name AddReceiptDialog follows existing dialog pattern (AddDocumentDialog, AddFuelLogDialog). Props interface AddReceiptDialogProps follows TypeScript camelCase convention. File placement frontend/src/features/maintenance/components/AddReceiptDialog.tsx matches feature capsule organization
  • MUI patterns consistent: Plan uses standard MUI Dialog with fullScreen prop (matching AddFuelLogDialog pattern at AddFuelLogDialog.tsx lines 19-27), AppBar/Toolbar header, Paper background, outlined Button with startIcon for camera option, Divider with Typography child for "or" separator. All patterns match existing codebase dialogs
  • Component API matches existing patterns: onClose, onFileSelect, onStartCamera callbacks match React event handler pattern. Props interface structure matches existing dialog components (AddDocumentDialog, AddFuelLogDialog)

Advisory note (not blocking): The plan specifies fullScreen Dialog with AppBar/Toolbar header in the layout ASCII diagram, but the proposed JSX in M2 shows a standard Dialog structure without fullScreen or AppBar mentioned. Cross-referencing existing patterns: AddFuelLogDialog uses fullScreen={isSmallScreen} conditional (line 24), while the ASCII layout shows AppBar which implies fullScreen mode. This should be clarified during implementation, but the pattern is well-established in the codebase so not flagging as HIGH severity.

RULE 2 (SHOULD_FIX) - Structural Quality

No RULE 2 violations found.

Verified:

  • Code duplication addressed: Plan explicitly states "Build a new AddReceiptDialog component with inline drag-and-drop logic rather than embedding FileInputFallback inside it" with rationale that FileInputFallback has its own header designed for full-screen standalone use. The inline implementation avoids embedding complexity and allows a single shared dialog header for both upload and camera options. File validation logic will be duplicated but it's ~10 lines and isolated to a single function - acceptable given the architectural constraint
  • Component scope: Single responsibility maintained - AddReceiptDialog handles file selection (upload or camera trigger) and validation only. OCR processing delegated to existing useMaintenanceReceiptOcr hook. Camera capture delegated to existing CameraCapture component. Clear separation of concerns
  • Data flow clarity: Data flow diagram in M2 explicitly shows both paths converging at processImage() -> review modal -> form population. Upload path: onFileSelect -> handleCaptureImage(file). Camera path: onStartCamera -> startCapture() -> CameraCapture -> onCapture -> handleCaptureImage(file, croppedFile). Both invoke processImage(file, croppedFile?) which already handles both optional cropped file (line 237: imageToProcess = croppedFile || file). No orphaned work, no god objects

Considered But Not Flagged

Constants placement: ACCEPTED_FORMATS, MAX_FILE_SIZE, ACCEPT_STRING declared inline in component file rather than in a shared constants module. This is minor duplication (FileInputFallback has these as props from caller), but the plan states FileInputFallback won't be modified to avoid fuel-logs regression risk. The duplication is isolated to 3 const declarations and acceptable for this low-risk pure frontend change.

Dialog fullScreen behavior: The plan's ASCII layout shows AppBar/Toolbar (implies fullScreen), but implementation details don't explicitly specify fullScreen prop or conditional logic. However, existing patterns (AddFuelLogDialog, AddDocumentDialog) demonstrate this is well-understood in the codebase. Not flagging as the developer agent will reference existing dialog patterns during implementation.

Hidden input accept attribute: Plan specifies ACCEPT_STRING = ACCEPTED_FORMATS.join(',') constant but doesn't show it being used in the hidden input's accept attribute. TW review flagged this as completeness gap (advisory, non-blocking). The pattern is clear from FileInputFallback line 115 and will be obvious during implementation.

Verdict

PASS - The plan demonstrates production-grade reliability, full conformance to project standards (mobile+desktop, naming, MUI patterns, touch targets), and sound structural design (no duplication concerns, clear data flow, single responsibility). All three quality rules satisfied.

File validation handles edge cases with MIME type checking and extension fallback. Resource cleanup is handled by existing hook patterns. Error handling is comprehensive. Mobile and desktop viewports are explicitly tested. Component naming and API design follow established patterns. Data flow is well-documented and converges correctly at processImage(). No god objects or unnecessary duplication.

Verdict: PASS | Next: QR plan-docs

## QR Review: Plan Code (RULE 0/1/2) **Phase**: Plan-Review | **Agent**: Quality Reviewer | **Status**: PASS ### RULE 0 (CRITICAL) - Production Reliability **No RULE 0 violations found.** **Verified**: - **File validation edge cases**: Plan specifies file validation function matching FileInputFallback pattern with MIME type checking against ACCEPTED_FORMATS array, HEIC/HEIF extension fallback for inconsistent MIME types, file size validation against MAX_FILE_SIZE (10MB), and inline Alert severity="error" for invalid files (M1 implementation details) - **Resource cleanup**: Blob URL cleanup is handled by existing useMaintenanceReceiptOcr hook. The processImage() function at line 238 creates blob URL with `URL.createObjectURL(imageToProcess)`, stores it in `receiptImageUrl` state. Cleanup happens in acceptResult() at line 301 and reset() at line 312 via `URL.revokeObjectURL(receiptImageUrl)`. The plan preserves this existing pattern - no new blob URLs created by AddReceiptDialog - **Error handling**: File validation errors displayed via inline Alert component. OCR processing errors handled by existing hook (processImage try/catch at lines 241-262 sets error state, displayed by existing "OCR Error Dialog" in MaintenanceRecordForm). Upload path and camera path both converge at processImage() which has comprehensive error handling - **Security**: File type validation prevents arbitrary file uploads. MIME type checking with extension fallback (HEIC/HEIF) prevents MIME spoofing. Size limit (10MB) prevents resource exhaustion ### RULE 1 (HIGH) - Project Conformance **No RULE 1 violations found.** **Verified**: - **Mobile + desktop requirement**: Plan explicitly addresses both viewports in M3 testing (320px mobile, 768px tablet, 1920px desktop). Mobile-specific implementation includes "Choose Image" button with `display: { xs: 'block', sm: 'none' }` visible below drop zone on xs breakpoint. Touch targets specified as `minHeight: 44` for all interactive elements matching project standard (44px minimum per CLAUDE.md) - **Naming conventions**: Component name `AddReceiptDialog` follows existing dialog pattern (AddDocumentDialog, AddFuelLogDialog). Props interface `AddReceiptDialogProps` follows TypeScript camelCase convention. File placement `frontend/src/features/maintenance/components/AddReceiptDialog.tsx` matches feature capsule organization - **MUI patterns consistent**: Plan uses standard MUI Dialog with fullScreen prop (matching AddFuelLogDialog pattern at AddFuelLogDialog.tsx lines 19-27), AppBar/Toolbar header, Paper background, outlined Button with startIcon for camera option, Divider with Typography child for "or" separator. All patterns match existing codebase dialogs - **Component API matches existing patterns**: onClose, onFileSelect, onStartCamera callbacks match React event handler pattern. Props interface structure matches existing dialog components (AddDocumentDialog, AddFuelLogDialog) **Advisory note** (not blocking): The plan specifies fullScreen Dialog with AppBar/Toolbar header in the layout ASCII diagram, but the proposed JSX in M2 shows a standard Dialog structure without fullScreen or AppBar mentioned. Cross-referencing existing patterns: AddFuelLogDialog uses `fullScreen={isSmallScreen}` conditional (line 24), while the ASCII layout shows AppBar which implies fullScreen mode. This should be clarified during implementation, but the pattern is well-established in the codebase so not flagging as HIGH severity. ### RULE 2 (SHOULD_FIX) - Structural Quality **No RULE 2 violations found.** **Verified**: - **Code duplication addressed**: Plan explicitly states "Build a new AddReceiptDialog component with inline drag-and-drop logic rather than embedding FileInputFallback inside it" with rationale that FileInputFallback has its own header designed for full-screen standalone use. The inline implementation avoids embedding complexity and allows a single shared dialog header for both upload and camera options. File validation logic will be duplicated but it's ~10 lines and isolated to a single function - acceptable given the architectural constraint - **Component scope**: Single responsibility maintained - AddReceiptDialog handles file selection (upload or camera trigger) and validation only. OCR processing delegated to existing useMaintenanceReceiptOcr hook. Camera capture delegated to existing CameraCapture component. Clear separation of concerns - **Data flow clarity**: Data flow diagram in M2 explicitly shows both paths converging at processImage() -> review modal -> form population. Upload path: onFileSelect -> handleCaptureImage(file). Camera path: onStartCamera -> startCapture() -> CameraCapture -> onCapture -> handleCaptureImage(file, croppedFile). Both invoke processImage(file, croppedFile?) which already handles both optional cropped file (line 237: `imageToProcess = croppedFile || file`). No orphaned work, no god objects ### Considered But Not Flagged **Constants placement**: ACCEPTED_FORMATS, MAX_FILE_SIZE, ACCEPT_STRING declared inline in component file rather than in a shared constants module. This is minor duplication (FileInputFallback has these as props from caller), but the plan states FileInputFallback won't be modified to avoid fuel-logs regression risk. The duplication is isolated to 3 const declarations and acceptable for this low-risk pure frontend change. **Dialog fullScreen behavior**: The plan's ASCII layout shows AppBar/Toolbar (implies fullScreen), but implementation details don't explicitly specify fullScreen prop or conditional logic. However, existing patterns (AddFuelLogDialog, AddDocumentDialog) demonstrate this is well-understood in the codebase. Not flagging as the developer agent will reference existing dialog patterns during implementation. **Hidden input accept attribute**: Plan specifies `ACCEPT_STRING = ACCEPTED_FORMATS.join(',')` constant but doesn't show it being used in the hidden input's accept attribute. TW review flagged this as completeness gap (advisory, non-blocking). The pattern is clear from FileInputFallback line 115 and will be obvious during implementation. ### Verdict **PASS** - The plan demonstrates production-grade reliability, full conformance to project standards (mobile+desktop, naming, MUI patterns, touch targets), and sound structural design (no duplication concerns, clear data flow, single responsibility). All three quality rules satisfied. File validation handles edge cases with MIME type checking and extension fallback. Resource cleanup is handled by existing hook patterns. Error handling is comprehensive. Mobile and desktop viewports are explicitly tested. Component naming and API design follow established patterns. Data flow is well-documented and converges correctly at processImage(). No god objects or unnecessary duplication. *Verdict*: PASS | *Next*: QR plan-docs
egullickson commented 2026-02-14 02:48:29 +00:00
Author
Owner
Copy Link
Copy Source

QR Review: Plan Docs (Re-Review)

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

Checklist

  • CLAUDE.md updates: M2 step 6 now includes updating frontend/src/features/maintenance/CLAUDE.md with AddReceiptDialog entry
  • Feature README: backend/src/features/maintenance/README.md does not need updates (pure frontend change)
  • Inline documentation: M1 now specifies required JSDoc header with @ai-summary and @ai-context for AddReceiptDialog.tsx
  • Data flow documentation: Complete data flow diagram provided in Milestone 2
  • No emoji: Plan adheres to professional documentation standards

Findings

Previous failures successfully resolved:

  1. CLAUDE.md Update (FIXED): M2 step 6 now explicitly specifies adding an entry to frontend/src/features/maintenance/CLAUDE.md Key Files table with component description and when-to-read context. The entry format matches existing patterns in the file.

  2. Inline Documentation (FIXED): M1 now includes a dedicated "Inline documentation (required JSDoc header)" section with proper @ai-summary and @ai-context JSDoc comments. The summary clearly states the component purpose ("Upload-first dialog for maintenance receipt input with drag-drop and camera options") and the context explains the architectural change ("Replaces camera-first ReceiptCameraButton pattern") plus data flow integration ("Both paths call parent callbacks that feed into useMaintenanceReceiptOcr.processImage()").

Additional verification:

  • Feature README: Not applicable - pure frontend change with no backend modifications
  • Data flow documentation: Comprehensive diagram shows both upload and camera paths converging at processImage() with clear state transitions
  • Professional standards: No emoji usage, structured markdown, clear section headings

The plan now meets all documentation requirements for AI context efficiency as specified in the project CLAUDE.md guidelines.

Verdict

All previous plan-docs failures have been addressed. The revised plan includes both CLAUDE.md update specification and inline JSDoc documentation requirements. Documentation is comprehensive, well-structured, and follows project standards.

Verdict: PASS | Next: Plan APPROVED - ready for implementation

## QR Review: Plan Docs (Re-Review) **Phase**: Plan-Review | **Agent**: Quality Reviewer | **Status**: PASS ### Checklist - [x] CLAUDE.md updates: M2 step 6 now includes updating `frontend/src/features/maintenance/CLAUDE.md` with AddReceiptDialog entry - [x] Feature README: backend/src/features/maintenance/README.md does not need updates (pure frontend change) - [x] Inline documentation: M1 now specifies required JSDoc header with @ai-summary and @ai-context for AddReceiptDialog.tsx - [x] Data flow documentation: Complete data flow diagram provided in Milestone 2 - [x] No emoji: Plan adheres to professional documentation standards ### Findings **Previous failures successfully resolved:** 1. **CLAUDE.md Update (FIXED)**: M2 step 6 now explicitly specifies adding an entry to `frontend/src/features/maintenance/CLAUDE.md` Key Files table with component description and when-to-read context. The entry format matches existing patterns in the file. 2. **Inline Documentation (FIXED)**: M1 now includes a dedicated "Inline documentation (required JSDoc header)" section with proper @ai-summary and @ai-context JSDoc comments. The summary clearly states the component purpose ("Upload-first dialog for maintenance receipt input with drag-drop and camera options") and the context explains the architectural change ("Replaces camera-first ReceiptCameraButton pattern") plus data flow integration ("Both paths call parent callbacks that feed into useMaintenanceReceiptOcr.processImage()"). **Additional verification:** - Feature README: Not applicable - pure frontend change with no backend modifications - Data flow documentation: Comprehensive diagram shows both upload and camera paths converging at processImage() with clear state transitions - Professional standards: No emoji usage, structured markdown, clear section headings The plan now meets all documentation requirements for AI context efficiency as specified in the project CLAUDE.md guidelines. ### Verdict All previous plan-docs failures have been addressed. The revised plan includes both CLAUDE.md update specification and inline JSDoc documentation requirements. Documentation is comprehensive, well-structured, and follows project standards. *Verdict*: PASS | *Next*: Plan APPROVED - ready for implementation
egullickson commented 2026-02-14 02:48:49 +00:00
Author
Owner
Copy Link
Copy Source

Plan Review Cycle: COMPLETE

Phase: Plan-Review | Agent: Planner | Status: APPROVED

Review Results

Gate Reviewer Result Notes
QR plan-completeness Quality Reviewer PASS All 7 ACs covered, sub-issues mapped 1:1, dependencies correct
TW plan-scrub Technical Writer PASS (Rev 2) Rev 1 failed for temporal contamination (11 line-number refs). Fixed in Rev 2 with structural anchors
QR plan-code Quality Reviewer PASS RULE 0/1/2 all satisfied. No production reliability, conformance, or structural quality issues
QR plan-docs Quality Reviewer PASS (Rev 3) Rev 2 failed for missing CLAUDE.md update and JSDoc spec. Fixed in Rev 3

Plan Approved (Rev 3)

The plan is ready for execution. Summary of milestones:

  • M1 (refs #183): Create AddReceiptDialog.tsx -- new fullscreen dialog with drag-drop upload (primary) and camera option (secondary)
  • M2 (refs #184): Integrate into MaintenanceRecordForm.tsx -- replace ReceiptCameraButton with "Add Receipt" button, wire both paths to existing OCR pipeline, update CLAUDE.md
  • M3 (refs #185): Viewport testing at 320px, 768px, 1920px

Verdict: APPROVED | Next: Create branch and begin execution

## Plan Review Cycle: COMPLETE **Phase**: Plan-Review | **Agent**: Planner | **Status**: APPROVED ### Review Results | Gate | Reviewer | Result | Notes | |------|----------|--------|-------| | QR plan-completeness | Quality Reviewer | PASS | All 7 ACs covered, sub-issues mapped 1:1, dependencies correct | | TW plan-scrub | Technical Writer | PASS (Rev 2) | Rev 1 failed for temporal contamination (11 line-number refs). Fixed in Rev 2 with structural anchors | | QR plan-code | Quality Reviewer | PASS | RULE 0/1/2 all satisfied. No production reliability, conformance, or structural quality issues | | QR plan-docs | Quality Reviewer | PASS (Rev 3) | Rev 2 failed for missing CLAUDE.md update and JSDoc spec. Fixed in Rev 3 | ### Plan Approved (Rev 3) The plan is ready for execution. Summary of milestones: - **M1** (refs #183): Create `AddReceiptDialog.tsx` -- new fullscreen dialog with drag-drop upload (primary) and camera option (secondary) - **M2** (refs #184): Integrate into `MaintenanceRecordForm.tsx` -- replace ReceiptCameraButton with "Add Receipt" button, wire both paths to existing OCR pipeline, update CLAUDE.md - **M3** (refs #185): Viewport testing at 320px, 768px, 1920px *Verdict*: APPROVED | *Next*: Create branch and begin execution
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
in-progress
type
chore
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#182