From d9c2de4882bc0e81a952ff5e2212b06c86b0a161 Mon Sep 17 00:00:00 2001
From: Steve Dudenhoeffer <steve@stevedudenhoeffer.com>
Date: Wed, 22 Jul 2026 13:06:23 -0400
Subject: [PATCH] Address review: rescan staleness + escapable slow scan
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

From the Gadfly review of #119:

- Reset the cost field on a new scan — it was the one review field not reseeded
  in the success handler (there's no cost on a packet), so a value typed before a
  Rescan lingered into the next proposal.
- Clear the review-phase error when returning to capture via Rescan, so a failed
  confirm doesn't leave a stale error showing on the capture screen.
- Make the scan escapable: thread an AbortController through the scan mutation and
  wire it to Cancel (no longer disabled while scanning), so a slow or hung vision
  call — the server allows up to 120s — isn't a trap the user can only leave by
  reloading. An aborted scan is treated as a cancel, not an error.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ
---
 web/src/components/plants/ScanPacketModal.tsx | 80 +++++++++++++------
 web/src/lib/seedPacket.ts                     | 10 ++-
 2 files changed, 63 insertions(+), 27 deletions(-)

diff --git a/web/src/components/plants/ScanPacketModal.tsx b/web/src/components/plants/ScanPacketModal.tsx
index 037df7b..804a5fc 100644
--- a/web/src/components/plants/ScanPacketModal.tsx
+++ b/web/src/components/plants/ScanPacketModal.tsx
@@ -43,6 +43,9 @@ export function ScanPacketModal({ unit, onClose }: { unit: UnitPref; onClose: ()
   const scan = useScanPacket()
   const create = useCreateFromPacket()
   const fileInput = useRef<HTMLInputElement>(null)
+  // Lets Cancel abort a slow/hung scan (the server allows up to 120s) so the
+  // dialog is never a trap the user can only escape by reloading the page.
+  const scanAbort = useRef<AbortController | null>(null)
 
   const [proposal, setProposal] = useState<PacketProposal | null>(null)
   const [error, setError] = useState<string | null>(null)
@@ -73,28 +76,40 @@ export function ScanPacketModal({ unit, onClose }: { unit: UnitPref; onClose: ()
     e.target.value = ''
     if (!file) return
     setError(null)
-    scan.mutate(file, {
-      onSuccess: (p) => {
-        const plant = newPlantDefaults(p)
-        const lot = lotDefaults(p.packet)
-        setProposal(p)
-        // Default to the top candidate when there is one — the likely case is the
-        // packet is a variety already in the catalog — else create a new one.
-        setSelection(p.candidates[0]?.plant.id ?? 'new')
-        setName(plant.name)
-        setCategory(plant.category)
-        setSpacing(String(spacingFromCm(plant.spacingCm, unit)))
-        setDays(plant.daysToMaturity != null ? String(plant.daysToMaturity) : '')
-        setVendor(lot.vendor)
-        setQuantity(String(lot.quantity))
-        setLotUnit(lot.unit)
-        setSku(lot.sku)
-        setLotCode(lot.lotCode)
-        setPackedForYear(lot.packedForYear != null ? String(lot.packedForYear) : '')
+    const controller = new AbortController()
+    scanAbort.current = controller
+    scan.mutate(
+      { file, signal: controller.signal },
+      {
+        onSuccess: (p) => {
+          const plant = newPlantDefaults(p)
+          const lot = lotDefaults(p.packet)
+          setProposal(p)
+          // Default to the top candidate when there is one — the likely case is
+          // the packet is a variety already in the catalog — else create a new one.
+          setSelection(p.candidates[0]?.plant.id ?? 'new')
+          setName(plant.name)
+          setCategory(plant.category)
+          setSpacing(String(spacingFromCm(plant.spacingCm, unit)))
+          setDays(plant.daysToMaturity != null ? String(plant.daysToMaturity) : '')
+          setVendor(lot.vendor)
+          setQuantity(String(lot.quantity))
+          setLotUnit(lot.unit)
+          setSku(lot.sku)
+          setLotCode(lot.lotCode)
+          setPackedForYear(lot.packedForYear != null ? String(lot.packedForYear) : '')
+          // Cost isn't on a packet, so it's the one field not reseeded from the
+          // proposal; clear it so a value typed before a Rescan doesn't linger.
+          setCost('')
+        },
+        onError: (err) => {
+          // An aborted scan is a user cancel, not a failure — and Cancel also
+          // closes the dialog, so there's nothing to report.
+          if ((err as Error)?.name === 'AbortError') return
+          setError(errorMessage(err, "Couldn't read that photo. Try a clearer, well-lit shot of the packet."))
+        },
       },
-      onError: (err) =>
-        setError(errorMessage(err, "Couldn't read that photo. Try a clearer, well-lit shot of the packet.")),
-    })
+    )
   }
 
   async function onConfirm(e: FormEvent) {
@@ -224,7 +239,16 @@ export function ScanPacketModal({ unit, onClose }: { unit: UnitPref; onClose: ()
           {error && <Alert>{error}</Alert>}
 
           <div className="flex justify-end">
-            <Button type="button" variant="ghost" onClick={onClose} disabled={busy}>
+            {/* Not disabled while scanning — this is the way out of a slow scan.
+                Aborting a settled/absent request is a harmless no-op. */}
+            <Button
+              type="button"
+              variant="ghost"
+              onClick={() => {
+                scanAbort.current?.abort()
+                onClose()
+              }}
+            >
               Cancel
             </Button>
           </div>
@@ -360,7 +384,17 @@ export function ScanPacketModal({ unit, onClose }: { unit: UnitPref; onClose: ()
           {error && <Alert>{error}</Alert>}
 
           <div className="mt-1 flex justify-between gap-2">
-            <Button type="button" variant="ghost" onClick={() => setProposal(null)} disabled={busy}>
+            <Button
+              type="button"
+              variant="ghost"
+              onClick={() => {
+                // Clear the review-phase error too, or it would show on the
+                // capture screen we're returning to.
+                setError(null)
+                setProposal(null)
+              }}
+              disabled={busy}
+            >
               Rescan
             </Button>
             <Button type="submit" disabled={busy}>
diff --git a/web/src/lib/seedPacket.ts b/web/src/lib/seedPacket.ts
index 7a42fde..b3d4ae2 100644
--- a/web/src/lib/seedPacket.ts
+++ b/web/src/lib/seedPacket.ts
@@ -65,14 +65,16 @@ export interface FromPacketBody {
 }
 
 /** Scan a packet photo into a proposal. Multipart upload; the vision call can
- *  take several seconds, so callers surface `isPending` as a scanning state. Not
- *  cached — every photo is a fresh one-shot. */
+ *  take several seconds (the server extends its deadline to 120s), so a `signal`
+ *  can be threaded through to abort a slow/hung scan — the caller wires it to a
+ *  Cancel button so the dialog is never a trap. Not cached — every photo is a
+ *  fresh one-shot. */
 export function useScanPacket() {
   return useMutation({
-    mutationFn: async (file: File): Promise<PacketProposal> => {
+    mutationFn: async ({ file, signal }: { file: File; signal?: AbortSignal }): Promise<PacketProposal> => {
       const form = new FormData()
       form.append('image', file)
-      return packetProposalSchema.parse(await api.postForm('/seed-lots/scan', form))
+      return packetProposalSchema.parse(await api.postForm('/seed-lots/scan', form, { signal }))
     },
   })
 }