Address Gadfly review on the seed shelf UI

The correctness finding is a real bug and the interesting one: arming a plant
from the Seed Tray never attributed a seed lot, not even in the unambiguous
single-lot case. The tray is the QUICKEST way to place a plant, so the fastest
path was the one that silently lost which packet the seed came from — and it
would have looked fine, because attribution is invisible until you go looking at
a remaining count that's wrong. armPlant now does the same single-lot
auto-attribution the picker does. Several lots stays unattributed rather than
guessing; the picker is where that choice belongs.

A single exhausted lot was also indistinguishable from having no lot, because
the "available" filter dropped it before the count was taken. That silently
dropped attribution with no signal. attributableLots now offers exhausted lots
too, just not first: the count records what was written down, not a fact about
the packet, so refusing to attribute a planting because the number says zero
makes a wrong number harder to correct rather than easier. It's also now one
rule in one place instead of the same filter written twice.

The seed lot form had no 409 handling, alone among the version-guarded forms
here. A concurrent edit made you retype everything; it now rebases onto the
current row and says so, like the garden and plant forms do.

Deduplication: the outbound-link anchor was duplicated verbatim between the
plant card and the lot rows, including the security-relevant rel attribute — now
one SourceLink that re-checks the URL and renders nothing when it isn't safe, so
callers can't forget to guard. remainingLabel had its own copy of the
unit-agreement rule that summarizeLots already owns; it now calls it.
formatQuantity moved out of a component file into lib/seedLots.ts with the other
formatters.

STATE_CLASS.over and .low were identical. "Over-planted" is a discrepancy to
look at, not a shortage to act on, so it now reads as its own warning.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ

web/src/components/plants/DeleteSeedLotModal.tsx

@@ -3,8 +3,7 @@ import { Alert } from '@/components/ui/Alert'
 import { Button } from '@/components/ui/Button'
 import { Modal } from '@/components/ui/Modal'
 import { errorMessage } from '@/lib/api'
-import { useDeleteSeedLot, type SeedLot } from '@/lib/seedLots'
-import { formatQuantity } from './SeedLotList'
+import { formatQuantity, useDeleteSeedLot, type SeedLot } from '@/lib/seedLots'
 
 /**
  * Retire a lot. Worth confirming because it's the one place cost and germination

web/src/components/plants/PlantCard.tsx

@@ -1,9 +1,10 @@
 import { useState } from 'react'
 import { PlantIcon } from './PlantIcon'
-import { LotStateChip, SeedLotList, formatQuantity } from './SeedLotList'
+import { LotStateChip, SeedLotList } from './SeedLotList'
+import { SourceLink } from './SourceLink'
 import { cardActionClass, cardDangerClass } from '@/components/ui/cardActions'
 import { CATEGORY_LABELS, isBuiltin, type Plant } from '@/lib/plants'
-import { safeExternalUrl, summarizeLots, type SeedLot } from '@/lib/seedLots'
+import { formatQuantity, summarizeLots, type SeedLot } from '@/lib/seedLots'
 import { formatSpacing, type UnitPref } from '@/lib/units'
 
 /**
@@ -37,7 +38,7 @@ export function PlantCard({
   const builtin = isBuiltin(plant)
   const [showLots, setShowLots] = useState(false)
   const summary = summarizeLots(lots)
-  const sourceUrl = safeExternalUrl(plant.sourceUrl)
+
   return (
     <div className="flex flex-col rounded-xl border border-border bg-surface">
       <div className="flex items-start gap-3 p-4">
@@ -54,21 +55,10 @@ export function PlantCard({
           <p className="mt-0.5 text-sm text-muted">
             {CATEGORY_LABELS[plant.category]} · {formatSpacing(plant.spacingCm, unit)} spacing
           </p>
-          {(plant.vendor || sourceUrl) && (
+          {(plant.vendor || plant.sourceUrl) && (
             <p className="mt-0.5 flex flex-wrap items-center gap-1.5 text-xs text-muted">
               {plant.vendor && <span>{plant.vendor}</span>}
-              {sourceUrl && (
-                <a
-                  href={sourceUrl}
-                  target="_blank"
-                  // The href is a URL somebody pasted; noopener keeps it from
-                  // getting a handle back to this window.
-                  rel="noopener noreferrer"
-                  className="underline decoration-dotted underline-offset-2 hover:text-fg"
-                >
-                  source ↗
-                </a>
-              )}
+              <SourceLink url={plant.sourceUrl} />
             </p>
           )}
           {plant.notes && <p className="mt-1 line-clamp-2 text-xs text-muted">{plant.notes}</p>}

web/src/components/plants/SeedLotList.tsx

@@ -1,10 +1,11 @@
 import { Button } from '@/components/ui/Button'
 import { cn } from '@/lib/cn'
+import { SourceLink } from './SourceLink'
 import {
   formatCost,
+  formatQuantity,
   formatUnitCost,
   lotState,
-  safeExternalUrl,
   type LotState,
   type SeedLot,
 } from '@/lib/seedLots'
@@ -20,7 +21,9 @@ const STATE_LABEL: Record<LotState, string> = {
 // Encoded in colour AND words, because this is the thing you skim down a list of
 // twenty packets deciding what to order — a bare number doesn't survive that.
 const STATE_CLASS: Record<LotState, string> = {
-  over: 'bg-amber-500/20 text-amber-800 dark:text-amber-300',
+  // "over" is a discrepancy to look at rather than a shortage to act on, so it
+  // reads as a distinct warning rather than sharing "low"'s styling.
+  over: 'bg-orange-500/25 text-orange-900 dark:text-orange-200',
   empty: 'bg-red-500/15 text-red-800 dark:text-red-300',
   low: 'bg-amber-500/20 text-amber-800 dark:text-amber-300',
   ok: 'bg-accent/20 text-accent-strong',
@@ -110,7 +113,6 @@ function LotRow({
   onEdit: () => void
   onDelete: () => void
 }) {
-  const url = safeExternalUrl(lot.sourceUrl)
   const cost = formatCost(lot.costCents)
   const unitCost = formatUnitCost(lot)
 
@@ -130,18 +132,7 @@ function LotRow({
             {lot.purchasedAt && <span>bought {lot.purchasedAt}</span>}
             {lot.germinationPct != null && <span>{lot.germinationPct}% germ.</span>}
             {cost && <span>{unitCost ? `${cost} (${unitCost})` : cost}</span>}
-            {url && (
-              <a
-                href={url}
-                target="_blank"
-                // noopener/noreferrer on every outbound link: the target is a URL
-                // someone pasted, and window.opener would hand it a handle back.
-                rel="noopener noreferrer"
-                className="underline decoration-dotted underline-offset-2 hover:text-fg"
-              >
-                source ↗
-              </a>
-            )}
+            <SourceLink url={lot.sourceUrl} />
           </p>
           <RemainingBar lot={lot} />
         </div>
@@ -168,9 +159,3 @@ function LotRow({
     </div>
   )
 }
-
-/** Quantities are stored as REAL because grams and ounces are fractional, but a
- *  whole number of seeds shouldn't render as "100.0". */
-export function formatQuantity(n: number): string {
-  return Number.isInteger(n) ? String(n) : n.toFixed(1)
-}

web/src/components/plants/SeedLotModal.tsx

@@ -7,6 +7,7 @@ import { TextArea } from '@/components/ui/TextArea'
 import { TextField } from '@/components/ui/TextField'
 import { errorMessage } from '@/lib/api'
 import {
+  conflictSeedLot,
   LOT_UNITS,
   safeExternalUrl,
   useCreateSeedLot,
@@ -49,11 +50,14 @@ export function SeedLotModal({
   const [sku, setSku] = useState(lot?.sku ?? '')
   const [lotCode, setLotCode] = useState(lot?.lotCode ?? '')
   const [notes, setNotes] = useState(lot?.notes ?? '')
+  const [version, setVersion] = useState(lot?.version ?? 0)
+  const [conflict, setConflict] = useState<string | null>(null)
   const [error, setError] = useState<string | null>(null)
 
   async function onSubmit(e: FormEvent) {
     e.preventDefault()
     setError(null)
+    setConflict(null)
 
     const qty = quantity.trim() === '' ? 0 : Number(quantity)
     if (!Number.isFinite(qty) || qty < 0) {
@@ -109,12 +113,32 @@ export function SeedLotModal({
 
     try {
       if (isEdit) {
-        await update.mutateAsync({ id: lot.id, version: lot.version, ...input })
+        await update.mutateAsync({ id: lot.id, version, ...input })
       } else {
         await create.mutateAsync(input)
       }
       onClose()
     } catch (err) {
+      const current = conflictSeedLot(err)
+      if (current) {
+        // Someone edited this lot elsewhere. Rebase onto the fresh row so a
+        // re-save applies, rather than making them retype everything — the same
+        // contract every other version-guarded form here honours.
+        setVersion(current.version)
+        setVendor(current.vendor)
+        setSourceUrl(current.sourceUrl)
+        setQuantity(String(current.quantity))
+        setUnit(current.unit)
+        setPurchasedAt(current.purchasedAt ?? '')
+        setPackedForYear(current.packedForYear != null ? String(current.packedForYear) : '')
+        setCost(current.costCents != null ? (current.costCents / 100).toFixed(2) : '')
+        setGermination(current.germinationPct != null ? String(current.germinationPct) : '')
+        setSku(current.sku)
+        setLotCode(current.lotCode)
+        setNotes(current.notes)
+        setConflict('This lot changed elsewhere. The latest values are shown — review and save again.')
+        return
+      }
       setError(errorMessage(err, isEdit ? 'Could not save the lot.' : 'Could not record the lot.'))
     }
   }
@@ -122,6 +146,8 @@ export function SeedLotModal({
   return (
     <Modal title={isEdit ? 'Edit seed lot' : `Seed lot — ${plant.name}`} onClose={onClose} busy={pending}>
       <form onSubmit={onSubmit} className="flex flex-col gap-3">
+        {conflict && <Alert tone="info">{conflict}</Alert>}
+
         <div className="grid grid-cols-2 gap-3">
           <TextField
             label="Quantity"

web/src/components/plants/SourceLink.tsx

@@ -0,0 +1,27 @@
+import { safeExternalUrl } from '@/lib/seedLots'
+
+/**
+ * A link out to where seed came from.
+ *
+ * The href is a URL somebody pasted, so it is re-checked here before rendering
+ * and carries rel="noopener noreferrer" — the server scheme-checks it too (#50),
+ * but a link is rendered from whatever the client was handed, and "the backend
+ * validated it" is not a reason to hand javascript: to an anchor tag.
+ *
+ * Renders nothing at all when the URL is absent or unsafe, so callers don't each
+ * have to remember to guard.
+ */
+export function SourceLink({ url, label = 'source' }: { url: string; label?: string }) {
+  const safe = safeExternalUrl(url)
+  if (!safe) return null
+  return (
+    <a
+      href={safe}
+      target="_blank"
+      rel="noopener noreferrer"
+      className="underline decoration-dotted underline-offset-2 hover:text-fg"
+    >
+      {label} ↗
+    </a>
+  )
+}

web/src/editor/PlantPicker.tsx

@@ -4,8 +4,16 @@ import { fieldControlClass } from '@/components/ui/field'
 import { CategoryChips } from '@/components/plants/CategoryChips'
 import { PlantIcon } from '@/components/plants/PlantIcon'
 import { CATEGORY_LABELS, filterPlants, usePlants, type CategoryFilter, type Plant } from '@/lib/plants'
-import { lotsByPlant, lotState, useSeedLots, type SeedLot } from '@/lib/seedLots'
-import { LotStateChip, formatQuantity } from '@/components/plants/SeedLotList'
+import {
+  attributableLots,
+  formatQuantity,
+  lotsByPlant,
+  lotState,
+  summarizeLots,
+  useSeedLots,
+  type SeedLot,
+} from '@/lib/seedLots'
+import { LotStateChip } from '@/components/plants/SeedLotList'
 import { formatSpacing, type UnitPref } from '@/lib/units'
 
 const RECENT_KEY = 'pansy:recent-plants'
@@ -85,12 +93,15 @@ export function PlantPicker({
   }, [all, recent, query])
 
   function choose(p: Plant) {
-    const lots = (lotsFor.get(p.id) ?? []).filter((l) => l.remaining > 0)
+    const lots = attributableLots(lotsFor.get(p.id) ?? [])
     if (lots.length > 1) {
       setChoosingLotFor(p)
       return
     }
     setRecent(recordRecent(p.id))
+    // A single lot attributes even when its count says empty. The count records
+    // what was written down, not a fact about the packet — dropping attribution
+    // there would make a wrong number harder to correct rather than easier.
     onSelect(p, lots[0])
   }
 
@@ -121,12 +132,12 @@ export function PlantPicker({
 
   // What's left, shown where you're deciding what to plant. Silent when there
   // are no lots — most plants won't have any, and "0 left" on all of them would
-  // be noise that trains you to ignore the number.
+  // be noise that trains you to ignore the number. summarizeLots owns the
+  // unit-agreement rule; a second copy of it here would drift.
   function remainingLabel(lots: SeedLot[]): string {
     if (lots.length === 0) return ''
-    const unitName = lots.every((l) => l.unit === lots[0].unit) ? ` ${lots[0].unit}` : ''
-    const total = lots.reduce((sum, l) => sum + l.remaining, 0)
-    return ` · ${formatQuantity(total)}${unitName} left`
+    const { remaining, unit } = summarizeLots(lots)
+    return ` · ${formatQuantity(remaining)}${unit ? ` ${unit}` : ''} left`
   }
 
   return (
@@ -169,9 +180,7 @@ export function PlantPicker({
             <p className="px-3 pb-1 pt-2 text-xs font-semibold uppercase tracking-wide text-muted">
               Which lot of {choosingLotFor.name}?
             </p>
-            {(lotsFor.get(choosingLotFor.id) ?? [])
-              .filter((l) => l.remaining > 0)
-              .map((lot) => (
+            {attributableLots(lotsFor.get(choosingLotFor.id) ?? []).map((lot) => (
                 <button
                   key={lot.id}
                   type="button"
@@ -189,7 +198,7 @@ export function PlantPicker({
                   </span>
                   <LotStateChip state={lotState(lot)} />
                 </button>
-              ))}
+            ))}
             <button
               type="button"
               onClick={() => chooseLot(choosingLotFor, undefined)}

web/src/lib/seedLots.test.ts

@@ -1,6 +1,8 @@
 import { describe, expect, it } from 'vitest'
 import {
+  attributableLots,
   formatCost,
+  formatQuantity,
   formatUnitCost,
   lotState,
   lotsByPlant,
@@ -119,3 +121,29 @@ describe('lotsByPlant', () => {
     expect(lotsByPlant(undefined).size).toBe(0)
   })
 })
+
+describe('attributableLots', () => {
+  // An exhausted lot is still offered: the count records what was written down,
+  // not a fact about the packet, so refusing to attribute a planting because the
+  // number says zero would make a wrong number harder to correct, not easier.
+  it('offers exhausted lots, just not first', () => {
+    const empty = lot({ id: 1, remaining: 0 })
+    const full = lot({ id: 2, remaining: 100 })
+    expect(attributableLots([empty, full]).map((l) => l.id)).toEqual([2, 1])
+    expect(attributableLots([empty]).length).toBe(1)
+  })
+
+  it("doesn't mutate its input", () => {
+    const lots = [lot({ id: 1, remaining: 0 }), lot({ id: 2, remaining: 100 })]
+    attributableLots(lots)
+    expect(lots.map((l) => l.id)).toEqual([1, 2])
+  })
+})
+
+describe('formatQuantity', () => {
+  it('keeps whole counts whole and fractional ones readable', () => {
+    expect(formatQuantity(100)).toBe('100')
+    expect(formatQuantity(12.5)).toBe('12.5')
+    expect(formatQuantity(-15)).toBe('-15') // over-planted lots go negative
+  })
+})

web/src/lib/seedLots.ts

@@ -6,7 +6,7 @@
 
 import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query'
 import { z } from 'zod'
-import { api } from './api'
+import { ApiError, api } from './api'
 
 export const lotUnitSchema = z.enum(['seeds', 'grams', 'ounces', 'packets', 'bulbs', 'plants'])
 export type LotUnit = z.infer<typeof lotUnitSchema>
@@ -104,6 +104,27 @@ export function useDeleteSeedLot() {
   })
 }
 
+/**
+ * The lots a placement could be attributed to, best first.
+ *
+ * Lots with seed left come first, but an exhausted one is still offered: the
+ * count is a record of what you wrote down, not a fact about the packet, and
+ * refusing to attribute a planting because the number says zero would make the
+ * number harder to correct rather than easier.
+ */
+export function attributableLots(lots: SeedLot[]): SeedLot[] {
+  return [...lots].sort((a, b) => Number(b.remaining > 0) - Number(a.remaining > 0))
+}
+
+/** The current server row carried by a 409, so an edit form can rebase onto it
+ *  and the user can re-save rather than losing what they typed. Every other
+ *  entity's form does this; a lot is no different. */
+export function conflictSeedLot(err: unknown): SeedLot | null {
+  if (!(err instanceof ApiError) || !err.isConflict) return null
+  const parsed = seedLotSchema.safeParse((err.body as { current?: unknown })?.current)
+  return parsed.success ? parsed.data : null
+}
+
 /** Group lots by the plant they belong to. */
 export function lotsByPlant(lots: SeedLot[] | undefined): Map<number, SeedLot[]> {
   const map = new Map<number, SeedLot[]>()
@@ -149,6 +170,12 @@ export function summarizeLots(lots: SeedLot[]): { remaining: number; unit: LotUn
   return { remaining, unit, state }
 }
 
+/** Quantities are stored as REAL because grams and ounces are fractional, but a
+ *  whole number of seeds shouldn't render as "100.0". */
+export function formatQuantity(n: number): string {
+  return Number.isInteger(n) ? String(n) : n.toFixed(1)
+}
+
 /** Whole currency from cents, e.g. 499 → "$4.99". */
 export function formatCost(cents: number | undefined): string | null {
   if (cents == null) return null

web/src/pages/GardenEditorPage.tsx

@@ -31,7 +31,7 @@ import {
 import { toEditorPlanting } from '@/lib/plantings'
 import type { Plant } from '@/lib/plants'
 import { useJournalCounts } from '@/lib/journal'
-import type { SeedLot } from '@/lib/seedLots'
+import { attributableLots, lotsByPlant, useSeedLots, type SeedLot } from '@/lib/seedLots'
 import { useSeedTray } from '@/lib/seedTray'
 import { usePageTitle } from '@/lib/usePageTitle'
 
@@ -75,6 +75,8 @@ export function GardenEditorPage() {
   const updateObject = useUpdateObject(gid)
   const ensurePlant = useEnsurePlantInFull(gid)
   const { trayPlants, add: addToTray, remove: removeFromTray } = useSeedTray(gid)
+  const seedLots = useSeedLots()
+  const lotsByPlantId = useMemo(() => lotsByPlant(seedLots.data), [seedLots.data])
 
   // Which plant-picker flow is open: 'place' arms a plant for repeat placement;
   // 'change' swaps the selected plop's plant.
@@ -278,8 +280,17 @@ export function GardenEditorPage() {
   // Arm a plant for tap-to-place. The full catalog carries plants not yet in this
   // garden's /full payload, so make sure the chosen one is in the cache first —
   // otherwise its placed plops render without an icon/color until a refetch.
+  // The Seed Tray arms a plant directly, without going through the picker, so it
+  // has to do the picker's single-lot auto-attribution itself — otherwise the
+  // quickest way to place a plant is the one path that silently loses which
+  // packet it came from. Several lots is genuinely ambiguous, so that case stays
+  // unattributed rather than guessing; the picker is where you choose.
   function armPlant(plant: Plant, lot?: SeedLot) {
     ensurePlant(plant)
+    if (lot === undefined) {
+      const lots = attributableLots(lotsByPlantId.get(plant.id) ?? [])
+      lot = lots.length === 1 ? lots[0] : undefined
+    }
     setArmedPlant(plant, lot?.id ?? null)
   }