Address second round of Gadfly findings on #76

- FillRegion's doc still said "half-pitch inset", left over from the first
  draft of the fix; the inset is radius - spacing/2. Two docs on the same
  function disagreeing is worse than either being terse.
- Reject non-finite region bounds. They survive clamping and the inverted-
  region guard (NaN compares false both ways). Nothing corrupt reached the
  table — SQLite stores NaN as NULL and NOT NULL refuses it — but NaN
  surfaced as a raw store error and +Inf as a silent zero-plop success.
- TestHexCentersTinyRegion used a region symmetric about the origin, so it
  could not distinguish "the middle of the region" from "the origin" and
  would have passed for an implementation that just returned (0,0). Added an
  off-centre case.

Not taken: the finding that `make(..., rows*cols)` over-allocates ~12%
because staggered rows hold cols-1. True, but the slice is capped at
maxFillPlops (5000) and the exact count needs a ceil/floor split for no
measurable gain.

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

internal/service/ops.go

@@ -92,10 +92,11 @@ func defaultPlopRadius(spacingCM float64) float64 {
 // FillRegion lays a hex-packed field of plops of one plant across a region of a
 // plantable object the actor can edit. Plop radius comes from the plant's spacing
 // (or spacingOverride) via defaultPlopRadius; centers sit on a hex lattice at 2×
-// radius pitch, centered in the region with a half-pitch inset at each edge (see
+// radius pitch, centered in the region, and set in from each edge by the plop's
-// hexCenters for why that inset and not a full one). A candidate is skipped when
+// radius less half a spacing — see hexCenters for why that half-spacing is what
-// its plop would sit entirely inside an existing active plop (so re-filling
+// the edge is owed. A candidate is skipped when its plop would sit entirely
-// doesn't stack duplicates). Returns the plops it created.
+// inside an existing active plop (so re-filling doesn't stack duplicates).
+// Returns the plops it created.
 func (s *Service) FillRegion(ctx context.Context, actorID, objectID int64, region Region, plantID int64, spacingOverride *float64) ([]domain.Planting, error) {
 	o, _, err := s.objectForRole(ctx, actorID, objectID, roleEditor)
 	if err != nil {
@@ -128,6 +129,18 @@ func (s *Service) fillLoaded(ctx context.Context, actorID int64, o *domain.Garde
 		return nil, domain.ErrInvalidInput
 	}
 
+	// A caller-supplied region is arbitrary floats, and non-finite ones survive
+	// everything downstream: clamping keeps them, the inverted-region guard can't
+	// see NaN (it compares false both ways), and fitAxis centres on them happily.
+	// Nothing corrupt reaches the table — SQLite stores NaN as NULL and the NOT
+	// NULL constraint refuses it — but the caller gets an opaque store error for
+	// NaN, and for +Inf a silent zero-plop success. Both are lies about what went
+	// wrong; say "bad input" here instead.
+	if !isFinite(region.MinX) || !isFinite(region.MinY) ||
+		!isFinite(region.MaxX) || !isFinite(region.MaxY) {
+		return nil, domain.ErrInvalidInput
+	}
+
 	region = region.clampTo(o.WidthCM/2, o.HeightCM/2)
 	centers := hexCenters(region, radius, spacing)
 	if len(centers) > maxFillPlops {

internal/service/ops_test.go

@@ -154,10 +154,55 @@ func TestHexCentersEdgeInset(t *testing.T) {
 
 // TestHexCentersTinyRegion covers a region too small to hold a plop at the
 // half-pitch inset: planting one in the middle beats refusing to plant at all.
+//
+// The off-centre case earns its place — a region symmetric about the origin
+// can't tell "the middle of the region" from "the origin", so on its own it
+// would pass for an implementation that just returned (0,0).
 func TestHexCentersTinyRegion(t *testing.T) {
-	pts := hexCenters(rect(-5, -5, 5, 5), 15, 10)
+	for _, tc := range []struct {
-	if len(pts) != 1 || pts[0].x != 0 || pts[0].y != 0 {
+		name         string
-		t.Errorf("tiny region gave %+v, want a single centered plop", pts)
+		r            Region
+		wantX, wantY float64
+	}{
+		{"centred on the origin", rect(-5, -5, 5, 5), 0, 0},
+		{"off in a corner", rect(20, -40, 30, -30), 25, -35},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			pts := hexCenters(tc.r, 15, 10)
+			if len(pts) != 1 || pts[0].x != tc.wantX || pts[0].y != tc.wantY {
+				t.Errorf("got %+v, want one plop at (%v,%v)", pts, tc.wantX, tc.wantY)
+			}
+		})
+	}
+}
+
+// TestFillRegionRejectsNonFiniteRegion: non-finite bounds survive clamping and
+// the inverted-region guard (NaN compares false both ways). Without the explicit
+// check, NaN surfaced as a raw store error ("NOT NULL constraint failed") and
+// +Inf as a silent success that planted nothing — neither of which tells the
+// caller what it actually did wrong.
+func TestFillRegionRejectsNonFiniteRegion(t *testing.T) {
+	ctx := context.Background()
+	s := newTestService(t, openConfig())
+	owner := seedUser(t, s, "[email protected]")
+	g, _ := s.CreateGarden(ctx, owner, GardenInput{Name: "Big", WidthCM: 2000, HeightCM: 2000})
+	bed := seedFillBed(t, s, owner, g.ID, 100, 100)
+	plant := seedOwnPlant(t, s, owner, 10)
+
+	nan := math.NaN()
+	for _, r := range []Region{
+		{MinX: nan, MinY: -50, MaxX: 50, MaxY: 50},
+		{MinX: -50, MinY: -50, MaxX: 50, MaxY: math.Inf(1)},
+	} {
+		created, err := s.FillRegion(ctx, owner, bed.ID, r, plant.ID, nil)
+		if !errors.Is(err, domain.ErrInvalidInput) {
+			t.Errorf("FillRegion(%+v) err = %v, want ErrInvalidInput", r, err)
+		}
+		for _, p := range created {
+			if !isFinite(p.XCM) || !isFinite(p.YCM) {
+				t.Errorf("persisted a plop with non-finite coordinates: %+v", p)
+			}
+		}
 	}
 }