diff --git a/internal/agent/tools_test.go b/internal/agent/tools_test.go index ce3928f..a1d33c8 100644 --- a/internal/agent/tools_test.go +++ b/internal/agent/tools_test.go @@ -10,6 +10,7 @@ import ( "gitea.stevedudenhoeffer.com/steve/majordomo/llm" "gitea.stevedudenhoeffer.com/steve/pansy/internal/config" + "gitea.stevedudenhoeffer.com/steve/pansy/internal/domain" "gitea.stevedudenhoeffer.com/steve/pansy/internal/service" "gitea.stevedudenhoeffer.com/steve/pansy/internal/store" ) @@ -51,9 +52,9 @@ func TestToolboxScenario(t *testing.T) { if err != nil { t.Fatalf("garden: %v", err) } - garlic, _ := svc.CreatePlant(ctx, owner.ID, service.PlantInput{Name: "Garlic", Category: "vegetable", SpacingCM: 15, Color: "#d9d2c5", Icon: "🧄"}) - basil, _ := svc.CreatePlant(ctx, owner.ID, service.PlantInput{Name: "Basil", Category: "herb", SpacingCM: 25, Color: "#4a7c3f", Icon: "🌿"}) - beans, _ := svc.CreatePlant(ctx, owner.ID, service.PlantInput{Name: "Beans", Category: "vegetable", SpacingCM: 10, Color: "#6b8e23", Icon: "🫘"}) + garlic := mustPlant(t, svc, owner.ID, "Garlic", 15, "🧄") + basil := mustPlant(t, svc, owner.ID, "Basil", 25, "🌿") + beans := mustPlant(t, svc, owner.ID, "Beans", 10, "🫘") // create_object → a 400×400 bed. res := call("create_object", map[string]any{ @@ -114,12 +115,15 @@ func TestToolboxScenario(t *testing.T) { } // ACL: a viewer's fill_region is refused (the toolbox runs as that actor). - viewerUser, _ := svc.Register(ctx, service.RegisterInput{Email: "v@example.com", DisplayName: "V", Password: "password123"}) - if _, err := svc.AddShare(ctx, owner.ID, g.ID, "v@example.com", "viewer"); err != nil { + viewerUser, err := svc.Register(ctx, service.RegisterInput{Email: "v@example.com", DisplayName: "V", Password: "password123"}) + if err != nil { + t.Fatalf("register viewer: %v", err) + } + if _, err := svc.AddShare(ctx, owner.ID, g.ID, "v@example.com", domain.RoleViewer); err != nil { t.Fatalf("share: %v", err) } viewerBox := NewToolbox(svc, viewerUser.ID) - vr := viewerBox.Execute(ctx, llm.ToolCall{ID: "2", Name: "fill_region", Arguments: mustJSON(map[string]any{ + vr := viewerBox.Execute(ctx, llm.ToolCall{ID: "2", Name: "fill_region", Arguments: mustJSON(t, map[string]any{ "objectId": bed.ID, "region": "all", "plantId": garlic.ID, })}) if !vr.IsError { @@ -127,7 +131,22 @@ func TestToolboxScenario(t *testing.T) { } } -func mustJSON(v any) json.RawMessage { - b, _ := json.Marshal(v) +func mustJSON(t *testing.T, v any) json.RawMessage { + t.Helper() + b, err := json.Marshal(v) + if err != nil { + t.Fatalf("marshal: %v", err) + } return b } + +func mustPlant(t *testing.T, svc *service.Service, owner int64, name string, spacing float64, icon string) *domain.Plant { + t.Helper() + p, err := svc.CreatePlant(context.Background(), owner, service.PlantInput{ + Name: name, Category: domain.CategoryVegetable, SpacingCM: spacing, Color: "#4a7c3f", Icon: icon, + }) + if err != nil { + t.Fatalf("create plant %s: %v", name, err) + } + return p +} diff --git a/internal/service/ops.go b/internal/service/ops.go index 62241f6..70e3f42 100644 --- a/internal/service/ops.go +++ b/internal/service/ops.go @@ -15,23 +15,31 @@ import ( // requireGardenRole. Geometry is in each object's LOCAL frame (origin at the // object's center, +x east, +y south, so -y is NORTH). -// Region is an area in an object's local frame: an axis-aligned rectangle, or a -// circle when Circle is set. NamedRegion produces the common ones. +// maxFillPlops bounds a single FillRegion so a huge bed with tiny spacing can't +// generate a runaway number of inserts. A bed with thousands of plops is already +// far past any real garden; over the cap we refuse rather than silently truncate. +const maxFillPlops = 5000 + +// Region is an axis-aligned rectangle in an object's local frame. (Circle/polygon +// regions are post-v1, like polygon objects; NamedRegion produces only rects.) type Region struct { - MinX, MinY, MaxX, MaxY float64 // rectangle (also the circle's bounding box) - Circle bool - CX, CY, Radius float64 // used when Circle + MinX, MinY, MaxX, MaxY float64 } // contains reports whether a local point lies in the region. func (r Region) contains(x, y float64) bool { - if r.Circle { - dx, dy := x-r.CX, y-r.CY - return dx*dx+dy*dy <= r.Radius*r.Radius - } return x >= r.MinX && x <= r.MaxX && y >= r.MinY && y <= r.MaxY } +// clampTo intersects the region with an object's local bounds (±halfW, ±halfH), +// so an oversized caller-supplied region can't make hexCenters loop forever. +func (r Region) clampTo(halfW, halfH float64) Region { + return Region{ + MinX: math.Max(r.MinX, -halfW), MinY: math.Max(r.MinY, -halfH), + MaxX: math.Min(r.MaxX, halfW), MaxY: math.Min(r.MaxY, halfH), + } +} + // rect builds a rectangular region. func rect(minX, minY, maxX, maxY float64) Region { return Region{MinX: minX, MinY: minY, MaxX: maxX, MaxY: maxY} @@ -43,13 +51,16 @@ func rect(minX, minY, maxX, maxY float64) Region { // A trailing "corner"/"half" word is ignored ("NE corner", "south half"). North // is -y (see the file header). Unknown names return ErrInvalidInput. func NamedRegion(o *domain.GardenObject, name string) (Region, error) { + if o == nil { + return Region{}, domain.ErrInvalidInput + } hw, hh := o.WidthCM/2, o.HeightCM/2 key := strings.ToLower(strings.TrimSpace(name)) key = strings.TrimSpace(strings.TrimSuffix(key, "corner")) key = strings.TrimSpace(strings.TrimSuffix(key, "half")) switch key { - case "all", "": + case "all": return rect(-hw, -hh, hw, hh), nil case "north", "top": return rect(-hw, -hh, hw, 0), nil @@ -72,8 +83,8 @@ func NamedRegion(o *domain.GardenObject, name string) (Region, error) { } } -// defaultPlopRadius is #15's formula for a placed plop's radius: max(1.5×spacing, -// 15cm). Reused here (don't duplicate the constant elsewhere). +// defaultPlopRadius is the radius a freshly-placed plop gets from its plant's +// spacing: max(1.5×spacing, 15cm) — matching the editor's placement default (#15). func defaultPlopRadius(spacingCM float64) float64 { return math.Max(1.5*spacingCM, 15) } @@ -89,6 +100,14 @@ func (s *Service) FillRegion(ctx context.Context, actorID, objectID int64, regio if err != nil { return nil, err } + return s.fillLoaded(ctx, actorID, o, region, plantID, spacingOverride) +} + +// fillLoaded is the shared body of FillRegion/FillNamedRegion given an object +// already loaded and authorized (roleEditor). It clamps the region to the +// object's bounds, refuses fills over maxFillPlops, and inserts the whole batch +// in one transaction rather than one round-trip per plop. +func (s *Service) fillLoaded(ctx context.Context, actorID int64, o *domain.GardenObject, region Region, plantID int64, spacingOverride *float64) ([]domain.Planting, error) { if !o.Plantable { return nil, domain.ErrInvalidInput } @@ -104,33 +123,36 @@ func (s *Service) FillRegion(ctx context.Context, actorID, objectID int64, regio spacing = *spacingOverride } radius := defaultPlopRadius(spacing) + if !isFinite(radius) || radius <= 0 { + return nil, domain.ErrInvalidInput + } - existing, err := s.store.ListActivePlantingsForObject(ctx, objectID) + region = region.clampTo(o.WidthCM/2, o.HeightCM/2) + centers := hexCenters(region, radius) + if len(centers) > maxFillPlops { + return nil, domain.ErrInvalidInput // region too large for this spacing; ask for less + } + + existing, err := s.store.ListActivePlantingsForObject(ctx, o.ID) if err != nil { return nil, err } - today := s.now().UTC().Format(dateLayout) - halfW, halfH := o.WidthCM/2, o.HeightCM/2 - created := []domain.Planting{} - for _, c := range hexCenters(region, radius) { - // The center must be inside the object (region ⊆ bounds already, but guard). - if math.Abs(c.x) > halfW || math.Abs(c.y) > halfH { - continue - } + batch := make([]*domain.Planting, 0, len(centers)) + for _, c := range centers { if coveredByExisting(c.x, c.y, radius, existing) { continue } - p := &domain.Planting{ObjectID: objectID, PlantID: plantID, XCM: c.x, YCM: c.y, RadiusCM: radius, PlantedAt: &today} - stored, err := s.store.CreatePlanting(ctx, p) - if err != nil { - return nil, err - } - stored.DerivedCount = derivedCount(stored.RadiusCM, spacing) - created = append(created, *stored) - // Count what we just placed so later candidates in this same fill don't - // stack on top of it. - existing = append(existing, *stored) + p := &domain.Planting{ObjectID: o.ID, PlantID: plantID, XCM: c.x, YCM: c.y, RadiusCM: radius, PlantedAt: &today} + batch = append(batch, p) + existing = append(existing, *p) // so later candidates in THIS fill don't stack on it + } + created, err := s.store.CreatePlantings(ctx, batch) + if err != nil { + return nil, err + } + for i := range created { + created[i].DerivedCount = derivedCount(created[i].RadiusCM, spacing) } return created, nil } @@ -188,11 +210,14 @@ func (s *Service) FillNamedRegion(ctx context.Context, actorID, objectID int64, if err != nil { return nil, err } - return s.FillRegion(ctx, actorID, objectID, region, plantID, spacingOverride) + return s.fillLoaded(ctx, actorID, o, region, plantID, spacingOverride) } // ClearObject soft-removes every active plop in an object the actor can edit (one // UPDATE), returning how many were cleared. Distinct from deleting the object. +// Unlike FillRegion it does NOT require the object be plantable: an object toggled +// non-plantable after it was planted must still be clearable (you can always +// remove existing plops, only not add new ones). func (s *Service) ClearObject(ctx context.Context, actorID, objectID int64) (int, error) { if _, _, err := s.objectForRole(ctx, actorID, objectID, roleEditor); err != nil { return 0, err diff --git a/internal/service/ops_test.go b/internal/service/ops_test.go index 660ac7e..733ac95 100644 --- a/internal/service/ops_test.go +++ b/internal/service/ops_test.go @@ -41,6 +41,25 @@ func TestNamedRegion(t *testing.T) { if _, err := NamedRegion(o, "middle-ish"); !errors.Is(err, domain.ErrInvalidInput) { t.Errorf("unknown region err = %v, want ErrInvalidInput", err) } + if _, err := NamedRegion(o, ""); !errors.Is(err, domain.ErrInvalidInput) { + t.Errorf("empty region err = %v, want ErrInvalidInput", err) + } + if _, err := NamedRegion(nil, "all"); !errors.Is(err, domain.ErrInvalidInput) { + t.Errorf("nil object err = %v, want ErrInvalidInput", err) + } +} + +func TestFillRegionCappedForHugeArea(t *testing.T) { + ctx := context.Background() + s := newTestService(t, openConfig()) + owner := seedUser(t, s, "a@example.com") + g, _ := s.CreateGarden(ctx, owner, GardenInput{Name: "Huge", WidthCM: 8000, HeightCM: 8000}) + 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) { + t.Errorf("oversized fill err = %v, want ErrInvalidInput (over maxFillPlops)", err) + } } func TestDefaultPlopRadius(t *testing.T) { diff --git a/internal/store/plantings.go b/internal/store/plantings.go index 28525b6..d47b15e 100644 --- a/internal/store/plantings.go +++ b/internal/store/plantings.go @@ -132,6 +132,36 @@ func (d *DB) CreatePlanting(ctx context.Context, p *domain.Planting) (*domain.Pl return created, nil } +// CreatePlantings inserts many plops in a single transaction (one commit), for +// bulk fills. Returns the stored rows in order. An empty input is a no-op. +func (d *DB) CreatePlantings(ctx context.Context, plantings []*domain.Planting) ([]domain.Planting, error) { + if len(plantings) == 0 { + return []domain.Planting{}, nil + } + tx, err := d.sql.BeginTx(ctx, nil) + if err != nil { + return nil, fmt.Errorf("store: begin plantings tx: %w", err) + } + defer tx.Rollback() //nolint:errcheck // no-op after a successful commit + + const stmt = `INSERT INTO plantings (object_id, plant_id, x_cm, y_cm, radius_cm, count, label, planted_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?) + RETURNING ` + plantingColumns + out := make([]domain.Planting, 0, len(plantings)) + for _, p := range plantings { + created, err := scanPlanting(tx.QueryRowContext(ctx, stmt, + p.ObjectID, p.PlantID, p.XCM, p.YCM, p.RadiusCM, p.Count, p.Label, p.PlantedAt)) + if err != nil { + return nil, fmt.Errorf("store: insert planting (batch): %w", err) + } + out = append(out, *created) + } + if err := tx.Commit(); err != nil { + return nil, fmt.Errorf("store: commit plantings: %w", err) + } + return out, nil +} + // UpdatePlanting applies a version-guarded update of all mutable columns (the // service merges partial patches first). Returns the updated row, or // (current row, ErrVersionConflict) / ErrNotFound — the same contract as the