Fill mode: a "grid" layout that plants individual plants in rows (#77)
Build image / build-and-push (push) Successful in 6s
Gadfly review (reusable) / review (pull_request) Successful in 12m51s
Adversarial Review (Gadfly) / review (pull_request) Successful in 12m51s

Steve chose option 3: keep clumps as the default primitive, add a grid/rows
fill mode, so sketching and planning are different operations with different
outputs rather than one model forced to be both.

A plop is a CLUMP, not a plant — great for "a few plops of garlic in a corner",
useless for drawing a plantable 8-rows-of-garlic bed (that came out as ~15
blobs, #77). FillLayout selects what a fill packs:
- clump (default, unchanged): radius 1.5×spacing, ~7 plants per plop.
- grid: radius spacing/2, pitch = spacing, ONE plant per plop — rows you could
  actually plant from.

The geometry is the SAME hexCenters lattice and the SAME #75 half-spacing edge
rule; only the radius→spacing relationship differs (plopRadiusFor). Grid keeps
no 15cm floor — its whole point is true spacing — while clump keeps it so a
tiny-spacing plant doesn't make invisible clumps.

Threaded through FillRegion/FillNamedRegion (empty layout = clump, so existing
callers are unchanged; unknown layout = ErrInvalidInput), the REST /fill
endpoint (`layout`), and the agent's fill_region tool (`mode`, enum clump|grid),
so "plant the bed in rows" works.

Tests: grid produces many more, single-plant plops than clump on the same bed
(radius spacing/2, derived count 1); unknown layout is refused at both the
service and the API. maxFillPlops still caps a grid fill of a huge bed.

No frontend fill affordance exists yet (fill is agent-only in the UI; the fill
UI was deferred in #82), so the mode toggle rides along when that's built —
noted. Docs: DESIGN placement-model decision.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ
This commit is contained in:
2026-07-22 00:01:27 -04:00
co-authored by Claude Opus 4.8
parent 1e2b763566
commit 7c1faa1515
9 changed files with 154 additions and 25 deletions
+64 -9
View File
@@ -59,7 +59,7 @@ func TestFillRegionCappedForHugeArea(t *testing.T) {
bed := seedFillBed(t, s, owner, g.ID, 6000, 6000) // ~46k lattice points at radius 15 → over the cap
plant := seedOwnPlant(t, s, owner, 10)
region, _ := NamedRegion(bed, "all")
if _, err := s.FillRegion(ctx, owner, bed.ID, region, plant.ID, nil); !errors.Is(err, domain.ErrInvalidInput) {
if _, err := s.FillRegion(ctx, owner, bed.ID, region, plant.ID, nil, FillClump); !errors.Is(err, domain.ErrInvalidInput) {
t.Errorf("oversized fill err = %v, want ErrInvalidInput (over maxFillPlops)", err)
}
}
@@ -199,7 +199,7 @@ func TestFillRegionRejectsNonFiniteRegion(t *testing.T) {
{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)
created, err := s.FillRegion(ctx, owner, bed.ID, r, plant.ID, nil, FillClump)
if !errors.Is(err, domain.ErrInvalidInput) {
t.Errorf("FillRegion(%+v) err = %v, want ErrInvalidInput", r, err)
}
@@ -223,7 +223,7 @@ func TestFillRegionOutsideObjectPlantsNothing(t *testing.T) {
plant := seedOwnPlant(t, s, owner, 10)
// Wholly east of the bed: clampTo gives MinX=500, MaxX=50.
created, err := s.FillRegion(ctx, owner, bed.ID, rect(500, -50, 600, 50), plant.ID, nil)
created, err := s.FillRegion(ctx, owner, bed.ID, rect(500, -50, 600, 50), plant.ID, nil, FillClump)
if err != nil {
t.Fatalf("FillRegion: %v", err)
}
@@ -256,7 +256,7 @@ func TestFillRegionDeterministicPacking(t *testing.T) {
plant := seedOwnPlant(t, s, owner, 10) // radius = max(15,15) = 15
region, _ := NamedRegion(bed, "all")
created, err := s.FillRegion(ctx, owner, bed.ID, region, plant.ID, nil)
created, err := s.FillRegion(ctx, owner, bed.ID, region, plant.ID, nil, FillClump)
if err != nil {
t.Fatalf("FillRegion: %v", err)
}
@@ -283,7 +283,7 @@ func TestFillRegionDeterministicPacking(t *testing.T) {
// Re-filling the same region skips everything (each candidate sits exactly on
// an existing plop → entirely inside it).
again, err := s.FillRegion(ctx, owner, bed.ID, region, plant.ID, nil)
again, err := s.FillRegion(ctx, owner, bed.ID, region, plant.ID, nil, FillClump)
if err != nil {
t.Fatalf("second FillRegion: %v", err)
}
@@ -292,6 +292,61 @@ func TestFillRegionDeterministicPacking(t *testing.T) {
}
}
// TestFillGridLaysOutIndividualPlants is the #77 grid mode: a grid fill packs one
// plant per plop at true spacing, so a bed becomes rows of plants rather than a
// few fat clumps. On the same bed it produces many more, smaller plops, each a
// single plant.
func TestFillGridLaysOutIndividualPlants(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, 60, 60)
plant := seedOwnPlant(t, s, owner, 10) // spacing 10
region, _ := NamedRegion(bed, "all")
clump, err := s.FillRegion(ctx, owner, bed.ID, region, plant.ID, nil, FillClump)
if err != nil {
t.Fatalf("clump: %v", err)
}
if _, err := s.ClearObject(ctx, owner, bed.ID); err != nil {
t.Fatalf("clear: %v", err)
}
grid, err := s.FillRegion(ctx, owner, bed.ID, region, plant.ID, nil, FillGrid)
if err != nil {
t.Fatalf("grid: %v", err)
}
// Grid packs at spacing 10 (radius 5, pitch 10); clump at radius 15 (pitch 30).
// Grid must produce many more plops.
if len(grid) <= len(clump) {
t.Errorf("grid produced %d plops, clump %d — grid should be denser", len(grid), len(clump))
}
// Each grid plop is one plant at radius spacing/2 = 5.
for _, p := range grid {
if p.RadiusCM != 5 {
t.Errorf("grid plop radius = %v, want 5 (spacing/2)", p.RadiusCM)
}
if p.DerivedCount != 1 {
t.Errorf("grid plop derived count = %d, want 1 (one plant per plop)", p.DerivedCount)
}
}
}
// TestFillRejectsUnknownLayout: a layout that isn't clump/grid is ErrInvalidInput.
func TestFillRejectsUnknownLayout(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, 60, 60)
plant := seedOwnPlant(t, s, owner, 10)
region, _ := NamedRegion(bed, "all")
if _, err := s.FillRegion(ctx, owner, bed.ID, region, plant.ID, nil, FillLayout("spiral")); !errors.Is(err, domain.ErrInvalidInput) {
t.Errorf("unknown layout err = %v, want ErrInvalidInput", err)
}
}
func TestFillRegionRotatedBedUsesLocalFrame(t *testing.T) {
ctx := context.Background()
s := newTestService(t, openConfig())
@@ -304,7 +359,7 @@ func TestFillRegionRotatedBedUsesLocalFrame(t *testing.T) {
plant := seedOwnPlant(t, s, owner, 20)
region, _ := NamedRegion(bed, "ne")
created, err := s.FillRegion(ctx, owner, bed.ID, region, plant.ID, nil)
created, err := s.FillRegion(ctx, owner, bed.ID, region, plant.ID, nil, FillClump)
if err != nil {
t.Fatalf("FillRegion: %v", err)
}
@@ -327,7 +382,7 @@ func TestClearObject(t *testing.T) {
bed := seedBed(t, s, owner, g.ID)
plant := seedOwnPlant(t, s, owner, 10)
region, _ := NamedRegion(bed, "all")
if _, err := s.FillRegion(ctx, owner, bed.ID, region, plant.ID, nil); err != nil {
if _, err := s.FillRegion(ctx, owner, bed.ID, region, plant.ID, nil, FillClump); err != nil {
t.Fatalf("fill: %v", err)
}
@@ -361,7 +416,7 @@ func TestOpsForbiddenForViewer(t *testing.T) {
}
region, _ := NamedRegion(bed, "all")
if _, err := s.FillRegion(ctx, viewer, bed.ID, region, plant.ID, nil); !errors.Is(err, domain.ErrForbidden) {
if _, err := s.FillRegion(ctx, viewer, bed.ID, region, plant.ID, nil, FillClump); !errors.Is(err, domain.ErrForbidden) {
t.Errorf("viewer fill = %v, want ErrForbidden", err)
}
if _, err := s.ClearObject(ctx, viewer, bed.ID); !errors.Is(err, domain.ErrForbidden) {
@@ -391,7 +446,7 @@ func TestFillScenario(t *testing.T) {
if err != nil {
t.Fatalf("region %q: %v", name, err)
}
if _, err := s.FillRegion(ctx, owner, bed.ID, region, plantID, nil); err != nil {
if _, err := s.FillRegion(ctx, owner, bed.ID, region, plantID, nil, FillClump); err != nil {
t.Fatalf("fill %q: %v", name, err)
}
}