diff --git a/CLAUDE.md b/CLAUDE.md index 1d9b23a..ff148e5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -129,6 +129,24 @@ Conventions that follow from it: a constraint between neighbouring plants; a bed edge is nobody's neighbour. - **Soft removal**: "clear bed" sets `removed_at`; the editor reads `removed_at IS NULL`. Hard delete is a different operation. +- **Length fields keep centimeters as the source of truth.** A dialog field + that takes a length is a `LengthField` (`web/src/lib/units.ts`): the text is + a view, `cm` changes only when the person types. Never re-parse the display + string on save — "29′ 6.3″" is the nearest tenth of an inch, and parsing it + back is how a no-change Save turned 900 cm into 899.922 (and bumped the + version, and wrote a bogus history entry). The inspector still keeps display + strings but gets the same result by refusing to commit text that still equals + the formatted original (`commitDim`); either way, a no-op save sends exactly + what was loaded — or nothing. +- **"Today" is the browser's local day**, from `today()` in + `web/src/lib/dates.ts`, and the UI always sends it: journal `observedAt`, + plop/fill `plantedAt`, `removedAt`. The server's UTC default is only for + API callers and the agent. A gardener placing at 9 pm in Ohio planted today, + not tomorrow — don't add a UI path that leaves the date to the server. +- **A wrapped `ErrInvalidInput` is shown to the person verbatim.** + `fmt.Errorf("%w: chat model %q: unknown provider", domain.ErrInvalidInput, spec)` + reaches the client as the 400's message (minus the sentinel prefix); the bare + sentinel reads "invalid input". Write the reason for the keyboard, not the log. - **Migrations** are numbered `.sql` files in `internal/store/migrations/`, run at startup, embedded. Never edit one that has shipped. - **Every service mutation lands in history** (#48). If you add one, record it — diff --git a/DESIGN.md b/DESIGN.md index a3e7ac4..f6e5cff 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -64,7 +64,7 @@ POST /change-sets/:id/revert ← undo an operation; 201, or 409 + the conflicts POST /gardens/:id/copy ← deep-copy a garden you own (objects + active plops; not shares/link) POST /gardens/:id/objects PATCH,DELETE /objects/:id POST /objects/:id/plantings PATCH,DELETE /plantings/:id -POST /objects/:id/fill ← hex-pack a region with one plant; region by compass name or rect +POST /objects/:id/fill ← hex-pack a region with one plant; region by compass name or rect; optional plantedAt (default UTC today) POST /objects/:id/clear ← soft-remove every active plop, as ONE change set GET,POST /plants PATCH,DELETE /plants/:id (own plants only) GET,POST /seed-lots GET,PATCH,DELETE /seed-lots/:id (own lots only; private) diff --git a/internal/agent/runtime_test.go b/internal/agent/runtime_test.go index a25945d..e452d10 100644 --- a/internal/agent/runtime_test.go +++ b/internal/agent/runtime_test.go @@ -60,7 +60,7 @@ func TestTurnIsOneChangeSet(t *testing.T) { if err != nil { t.Fatalf("bed: %v", err) } - if _, err := svc.FillNamedRegion(ctx, owner, bed.ID, "all", garlic.ID, nil, service.FillClump); err != nil { + if _, err := svc.FillNamedRegion(ctx, owner, bed.ID, "all", garlic.ID, nil, service.FillClump, nil); err != nil { t.Fatalf("seed garlic: %v", err) } diff --git a/internal/agent/tools.go b/internal/agent/tools.go index 43571dc..a62d951 100644 --- a/internal/agent/tools.go +++ b/internal/agent/tools.go @@ -163,7 +163,9 @@ func (a *adapter) fillRegion(ctx context.Context, args struct { SpacingOverride *float64 `json:"spacingOverrideCm" description:"optional in-row spacing override in cm; omit to use the plant's spacing"` Mode string `json:"mode" enum:"clump,grid" description:"clump (default) drops a few fat clumps for a quick sketch; grid lays out individual plants in rows at true spacing, a layout you could plant from"` }) (any, error) { - return a.svc.FillNamedRegion(ctx, a.actor, args.ObjectID, args.Region, args.PlantID, args.SpacingOverride, service.FillLayout(args.Mode)) + // nil: the agent runs server-side with no local day, so the fill dates + // plops UTC-today like its create_planting does. + return a.svc.FillNamedRegion(ctx, a.actor, args.ObjectID, args.Region, args.PlantID, args.SpacingOverride, service.FillLayout(args.Mode), nil) } func (a *adapter) findPlant(ctx context.Context, args struct { diff --git a/internal/agent/tools_test.go b/internal/agent/tools_test.go index cf19ae1..2324285 100644 --- a/internal/agent/tools_test.go +++ b/internal/agent/tools_test.go @@ -169,7 +169,7 @@ func TestGarlicBedToCucumbers(t *testing.T) { if err != nil { t.Fatalf("bed: %v", err) } - if _, err := svc.FillNamedRegion(ctx, owner, bed.ID, "all", garlic.ID, nil, service.FillClump); err != nil { + if _, err := svc.FillNamedRegion(ctx, owner, bed.ID, "all", garlic.ID, nil, service.FillClump, nil); err != nil { t.Fatalf("seed the garlic: %v", err) } diff --git a/internal/api/errors.go b/internal/api/errors.go index 83bc538..d4417df 100644 --- a/internal/api/errors.go +++ b/internal/api/errors.go @@ -6,6 +6,7 @@ import ( "log/slog" "net/http" "strconv" + "strings" "github.com/gin-gonic/gin" @@ -52,7 +53,7 @@ func writeServiceError(c *gin.Context, err error) { case errors.Is(err, domain.ErrOIDCIdentityConflict): writeAPIError(c, http.StatusConflict, "OIDC_IDENTITY_CONFLICT", "this identity conflicts with an existing account") case errors.Is(err, domain.ErrInvalidInput): - writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "invalid input") + writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", inputMessage(err)) default: slog.Error("api: unhandled service error", "error", err) writeAPIError(c, http.StatusInternalServerError, "INTERNAL", "internal error") @@ -114,3 +115,18 @@ func parseIDParam(c *gin.Context, name string) (int64, bool) { } return id, true } + +// inputMessage is the text a 400 carries for an ErrInvalidInput. The bare +// sentinel reads "invalid input"; a service that wraps it with a reason — +// fmt.Errorf("%w: chat model %q: unknown provider", domain.ErrInvalidInput, spec) +// — has that reason shown to the person verbatim, minus the sentinel prefix. +// So anything wrapped this way is written for the keyboard, not the log (see +// the note on domain.ErrInvalidInput). +func inputMessage(err error) string { + msg := err.Error() + base := domain.ErrInvalidInput.Error() + if msg == base { + return msg + } + return strings.TrimPrefix(msg, base+": ") +} diff --git a/internal/api/errors_test.go b/internal/api/errors_test.go new file mode 100644 index 0000000..5ab138d --- /dev/null +++ b/internal/api/errors_test.go @@ -0,0 +1,30 @@ +package api + +import ( + "errors" + "fmt" + "testing" + + "gitea.stevedudenhoeffer.com/steve/pansy/internal/domain" +) + +// TestInputMessage: the bare sentinel stays generic; a wrapped reason reaches +// the person without the "invalid input: " prefix in front of it. +func TestInputMessage(t *testing.T) { + cases := []struct { + err error + want string + }{ + {domain.ErrInvalidInput, "invalid input"}, + {fmt.Errorf("%w: chat model %q: unknown provider", domain.ErrInvalidInput, "nonesuch/model"), `chat model "nonesuch/model": unknown provider`}, + {fmt.Errorf("loading: %w", domain.ErrInvalidInput), "loading: invalid input"}, + } + for _, c := range cases { + if !errors.Is(c.err, domain.ErrInvalidInput) { + t.Fatalf("%v should still be an ErrInvalidInput", c.err) + } + if got := inputMessage(c.err); got != c.want { + t.Errorf("inputMessage(%v) = %q, want %q", c.err, got, c.want) + } + } +} diff --git a/internal/api/ops.go b/internal/api/ops.go index e152109..cf72063 100644 --- a/internal/api/ops.go +++ b/internal/api/ops.go @@ -55,6 +55,10 @@ type objectFillRequest struct { // (individual plants in rows at true spacing). Empty = clump. An unknown value // is refused by the service (#77). Layout string `json:"layout"` + // PlantedAt dates every plop the fill makes (YYYY-MM-DD). The UI sends its + // local day; omitted, the server uses UTC today — which is tomorrow for an + // evening gardener west of Greenwich, so clients that know better say so. + PlantedAt *string `json:"plantedAt"` } func (h *handlers) fillObject(c *gin.Context) { @@ -88,9 +92,9 @@ func (h *handlers) fillObject(c *gin.Context) { return } region := service.Region{MinX: rect.MinX, MinY: rect.MinY, MaxX: rect.MaxX, MaxY: rect.MaxY} - created, err = h.svc.FillRegion(c.Request.Context(), actor, id, region, req.PlantID, req.SpacingOverrideCM, service.FillLayout(req.Layout)) + created, err = h.svc.FillRegion(c.Request.Context(), actor, id, region, req.PlantID, req.SpacingOverrideCM, service.FillLayout(req.Layout), req.PlantedAt) } else { - created, err = h.svc.FillNamedRegion(c.Request.Context(), actor, id, req.Region, req.PlantID, req.SpacingOverrideCM, service.FillLayout(req.Layout)) + created, err = h.svc.FillNamedRegion(c.Request.Context(), actor, id, req.Region, req.PlantID, req.SpacingOverrideCM, service.FillLayout(req.Layout), req.PlantedAt) } if err != nil { writeServiceError(c, err) diff --git a/internal/api/settings_test.go b/internal/api/settings_test.go index 4dc304a..3198376 100644 --- a/internal/api/settings_test.go +++ b/internal/api/settings_test.go @@ -1,7 +1,9 @@ package api import ( + "encoding/json" "net/http" + "strings" "testing" "github.com/gin-gonic/gin" @@ -159,10 +161,22 @@ func TestSettingsRejectsBadModel(t *testing.T) { admin := registerAndCookie(t, r, "admin@example.com") v := settingsVersion(t, r, admin) - if w := doJSON(t, r, http.MethodPatch, "/api/v1/settings", - map[string]any{"agentModel": "nonesuch/model", "version": v}, admin); w.Code != http.StatusBadRequest { + w := doJSON(t, r, http.MethodPatch, "/api/v1/settings", + map[string]any{"agentModel": "nonesuch/model", "version": v}, admin) + if w.Code != http.StatusBadRequest { t.Errorf("bad model: status %d, want 400", w.Code) } + // The message says which field and which spec, so the page can show a reason + // rather than a bare "invalid input". + var body struct { + Error struct{ Code, Message string } `json:"error"` + } + if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil { + t.Fatalf("decode bad-model body: %v", err) + } + if body.Error.Code != "INVALID_INPUT" || !strings.Contains(body.Error.Message, "chat model") || !strings.Contains(body.Error.Message, "nonesuch/model") { + t.Errorf("bad model error = %+v, want INVALID_INPUT naming the chat model and spec", body.Error) + } // agentEnabled must be a bool or null, not a string. if w := doJSON(t, r, http.MethodPatch, "/api/v1/settings", map[string]any{"agentModel": "", "agentEnabled": "yes", "version": v}, admin); w.Code != http.StatusBadRequest { diff --git a/internal/domain/domain.go b/internal/domain/domain.go index 2fa7c34..ffbee70 100644 --- a/internal/domain/domain.go +++ b/internal/domain/domain.go @@ -33,7 +33,10 @@ var ( ErrShareExists = errors.New("garden already shared with that user") // ErrInvalidInput means the caller supplied structurally invalid data (empty - // required field, malformed value). Mapped to 400. + // required field, malformed value). Mapped to 400. Wrap it with the reason — + // fmt.Errorf("%w: plantedAt must be a YYYY-MM-DD date", ErrInvalidInput) — + // and the API shows that reason to the person verbatim, so write it for + // them, not for a log; the bare sentinel reads as just "invalid input". ErrInvalidInput = errors.New("invalid input") // ErrInvalidCredentials means a login attempt failed. It is deliberately // identical for an unknown email and a wrong password so neither can be diff --git a/internal/service/instance_settings.go b/internal/service/instance_settings.go index 6a121c5..5ea56fb 100644 --- a/internal/service/instance_settings.go +++ b/internal/service/instance_settings.go @@ -2,6 +2,8 @@ package service import ( "context" + "errors" + "fmt" "strings" "gitea.stevedudenhoeffer.com/steve/pansy/internal/agentmodel" @@ -61,12 +63,15 @@ func (s *Service) UpdateInstanceSettings(ctx context.Context, actorID int64, pat model := strings.TrimSpace(patch.AgentModel) vision := strings.TrimSpace(patch.VisionModel) // Validate non-empty specs up front. An empty one is the "inherit env" - // sentinel and needs no check — the env value was validated at boot. - for _, spec := range []string{model, vision} { - if spec != "" { - if err := agentmodel.Validate(s.cfg.Agent.OllamaCloudAPIKey, spec); err != nil { - return nil, domain.ErrInvalidInput - } + // sentinel and needs no check — the env value was validated at boot. The + // reason rides on the sentinel so the 400 can show it: "unknown provider" + // is something a person can act on, "invalid input" is not. + for _, f := range []struct{ label, spec string }{{"chat model", model}, {"vision model", vision}} { + if f.spec == "" { + continue + } + if err := agentmodel.Validate(s.cfg.Agent.OllamaCloudAPIKey, f.spec); err != nil { + return nil, fmt.Errorf("%w: %s %q: %v", domain.ErrInvalidInput, f.label, f.spec, specReason(err)) } } return s.store.UpdateInstanceSettings(ctx, &domain.InstanceSettings{ @@ -166,3 +171,12 @@ func (s *Service) EffectiveConfig(ctx context.Context) (EffectiveAgent, Effectiv } return s.agentOver(st), s.visionOver(st), nil } + +// specReason strips agentmodel's own "resolve %q:" wrapping so the message +// reads "unknown provider …" rather than repeating the spec twice. +func specReason(err error) string { + if u := errors.Unwrap(err); u != nil { + return u.Error() + } + return err.Error() +} diff --git a/internal/service/ops.go b/internal/service/ops.go index b648fe4..743134f 100644 --- a/internal/service/ops.go +++ b/internal/service/ops.go @@ -175,13 +175,15 @@ func validFillLayout(l FillLayout) (FillLayout, bool) { // in from each edge by edgeInset — a half-spacing for grid, radius-less-a-half- // spacing for a clump (see edgeInset for the why). A candidate is skipped when its // plop would sit entirely 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, layout FillLayout) ([]domain.Planting, error) { +// stack duplicates). Every plop is dated plantedAt (YYYY-MM-DD), or UTC today +// when nil — the UI always sends its local day, so the default is for API and +// agent callers. Returns the plops it created. +func (s *Service) FillRegion(ctx context.Context, actorID, objectID int64, region Region, plantID int64, spacingOverride *float64, layout FillLayout, plantedAt *string) ([]domain.Planting, error) { o, _, err := s.objectForRole(ctx, actorID, objectID, roleEditor) if err != nil { return nil, err } - return s.fillLoaded(ctx, actorID, o, region, plantID, spacingOverride, layout) + return s.fillLoaded(ctx, actorID, o, region, plantID, spacingOverride, layout, plantedAt) } // fillLoaded is the shared body of FillRegion/FillNamedRegion given an object @@ -189,10 +191,13 @@ func (s *Service) FillRegion(ctx context.Context, actorID, objectID int64, regio // non-finite region, 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, layout FillLayout) ([]domain.Planting, error) { +func (s *Service) fillLoaded(ctx context.Context, actorID int64, o *domain.GardenObject, region Region, plantID int64, spacingOverride *float64, layout FillLayout, plantedAt *string) ([]domain.Planting, error) { if !o.Plantable { return nil, domain.ErrInvalidInput } + if !validDatePtr(plantedAt) { + return nil, fmt.Errorf("%w: plantedAt must be a YYYY-MM-DD date", domain.ErrInvalidInput) + } layout, ok := validFillLayout(layout) if !ok { return nil, domain.ErrInvalidInput @@ -235,7 +240,10 @@ func (s *Service) fillLoaded(ctx context.Context, actorID int64, o *domain.Garde if err != nil { return nil, err } - today := s.now().UTC().Format(dateLayout) + plantedOn := s.now().UTC().Format(dateLayout) + if plantedAt != nil { + plantedOn = *plantedAt + } batch := make([]*domain.Planting, 0, len(centers)) // Only the plops that were ALREADY here can cover a candidate: every plop this // fill makes shares one radius and sits on a distinct lattice point, and a plop @@ -247,7 +255,7 @@ func (s *Service) fillLoaded(ctx context.Context, actorID int64, o *domain.Garde if coveredByExisting(c.x, c.y, radius, existing) { continue } - batch = append(batch, &domain.Planting{ObjectID: o.ID, PlantID: plantID, XCM: c.x, YCM: c.y, RadiusCM: radius, PlantedAt: &today}) + batch = append(batch, &domain.Planting{ObjectID: o.ID, PlantID: plantID, XCM: c.x, YCM: c.y, RadiusCM: radius, PlantedAt: &plantedOn}) } created, err := s.store.CreatePlantings(ctx, batch) if err != nil { @@ -376,7 +384,7 @@ func coveredByExisting(x, y, radius float64, existing []domain.Planting) bool { // FillNamedRegion is FillRegion addressed by a compass name ("ne", "south half") // instead of a resolved Region — the ergonomic form for agent tools, which don't // hold the object's geometry. It resolves the name against the object, then fills. -func (s *Service) FillNamedRegion(ctx context.Context, actorID, objectID int64, regionName string, plantID int64, spacingOverride *float64, layout FillLayout) ([]domain.Planting, error) { +func (s *Service) FillNamedRegion(ctx context.Context, actorID, objectID int64, regionName string, plantID int64, spacingOverride *float64, layout FillLayout, plantedAt *string) ([]domain.Planting, error) { o, _, err := s.objectForRole(ctx, actorID, objectID, roleEditor) if err != nil { return nil, err @@ -385,7 +393,7 @@ func (s *Service) FillNamedRegion(ctx context.Context, actorID, objectID int64, if err != nil { return nil, err } - return s.fillLoaded(ctx, actorID, o, region, plantID, spacingOverride, layout) + return s.fillLoaded(ctx, actorID, o, region, plantID, spacingOverride, layout, plantedAt) } // ClearObject soft-removes every active plop in an object the actor can edit (one diff --git a/internal/service/ops_test.go b/internal/service/ops_test.go index ba2810a..1fa58b4 100644 --- a/internal/service/ops_test.go +++ b/internal/service/ops_test.go @@ -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, FillClump); !errors.Is(err, domain.ErrInvalidInput) { + if _, err := s.FillRegion(ctx, owner, bed.ID, region, plant.ID, nil, FillClump, nil); !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, FillClump) + created, err := s.FillRegion(ctx, owner, bed.ID, r, plant.ID, nil, FillClump, nil) 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, FillClump) + created, err := s.FillRegion(ctx, owner, bed.ID, rect(500, -50, 600, 50), plant.ID, nil, FillClump, nil) 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, FillClump) + created, err := s.FillRegion(ctx, owner, bed.ID, region, plant.ID, nil, FillClump, nil) 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, FillClump) + again, err := s.FillRegion(ctx, owner, bed.ID, region, plant.ID, nil, FillClump, nil) if err != nil { t.Fatalf("second FillRegion: %v", err) } @@ -305,14 +305,14 @@ func TestFillGridLaysOutIndividualPlants(t *testing.T) { 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) + clump, err := s.FillRegion(ctx, owner, bed.ID, region, plant.ID, nil, FillClump, nil) 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) + grid, err := s.FillRegion(ctx, owner, bed.ID, region, plant.ID, nil, FillGrid, nil) if err != nil { t.Fatalf("grid: %v", err) } @@ -351,7 +351,7 @@ func TestFillRejectsUnknownLayout(t *testing.T) { 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) { + if _, err := s.FillRegion(ctx, owner, bed.ID, region, plant.ID, nil, FillLayout("spiral"), nil); !errors.Is(err, domain.ErrInvalidInput) { t.Errorf("unknown layout err = %v, want ErrInvalidInput", err) } } @@ -368,7 +368,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, FillClump) + created, err := s.FillRegion(ctx, owner, bed.ID, region, plant.ID, nil, FillClump, nil) if err != nil { t.Fatalf("FillRegion: %v", err) } @@ -391,7 +391,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, FillClump); err != nil { + if _, err := s.FillRegion(ctx, owner, bed.ID, region, plant.ID, nil, FillClump, nil); err != nil { t.Fatalf("fill: %v", err) } @@ -425,7 +425,7 @@ func TestOpsForbiddenForViewer(t *testing.T) { } region, _ := NamedRegion(bed, "all") - if _, err := s.FillRegion(ctx, viewer, bed.ID, region, plant.ID, nil, FillClump); !errors.Is(err, domain.ErrForbidden) { + if _, err := s.FillRegion(ctx, viewer, bed.ID, region, plant.ID, nil, FillClump, nil); !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) { @@ -455,7 +455,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, FillClump); err != nil { + if _, err := s.FillRegion(ctx, owner, bed.ID, region, plantID, nil, FillClump, nil); err != nil { t.Fatalf("fill %q: %v", name, err) } } @@ -507,3 +507,34 @@ func seedNamedPlant(t *testing.T, s *Service, owner int64, name string, spacingC } return p } + +// TestFillRegionPlantedAt: a fill dates its plops as told and refuses a date +// that isn't one. The UI sends its local day, so an evening fill isn't stamped +// with UTC's tomorrow; API and agent callers that omit it still get UTC today. +func TestFillRegionPlantedAt(t *testing.T) { + ctx := context.Background() + s := newTestService(t, openConfig()) + owner := seedUser(t, s, "a@example.com") + g, _ := s.CreateGarden(ctx, owner, GardenInput{Name: "Dated", WidthCM: 2000, HeightCM: 2000}) + bed := seedFillBed(t, s, owner, g.ID, 200, 100) + plant := seedOwnPlant(t, s, owner, 30) + + day := "2026-04-01" + created, err := s.FillNamedRegion(ctx, owner, bed.ID, "all", plant.ID, nil, FillClump, &day) + if err != nil { + t.Fatalf("fill: %v", err) + } + if len(created) == 0 { + t.Fatal("fill created nothing") + } + for _, p := range created { + if p.PlantedAt == nil || *p.PlantedAt != day { + t.Errorf("planting %d plantedAt = %v, want %s", p.ID, p.PlantedAt, day) + } + } + + bad := "April 1st" + if _, err := s.FillNamedRegion(ctx, owner, bed.ID, "all", plant.ID, nil, FillClump, &bad); !errors.Is(err, domain.ErrInvalidInput) { + t.Errorf("bad date err = %v, want ErrInvalidInput", err) + } +} diff --git a/internal/service/revisions_test.go b/internal/service/revisions_test.go index ecc500c..751ae2f 100644 --- a/internal/service/revisions_test.go +++ b/internal/service/revisions_test.go @@ -73,7 +73,7 @@ func TestFillRegionIsOneChangeSet(t *testing.T) { plant := seedOwnPlant(t, s, owner, 15) ctx := context.Background() - created, err := s.FillNamedRegion(ctx, owner, bed.ID, "all", plant.ID, nil, FillClump) + created, err := s.FillNamedRegion(ctx, owner, bed.ID, "all", plant.ID, nil, FillClump, nil) if err != nil { t.Fatalf("FillNamedRegion: %v", err) } @@ -320,7 +320,7 @@ func TestRevertClearObject(t *testing.T) { plant := seedOwnPlant(t, s, owner, 15) ctx := context.Background() - if _, err := s.FillNamedRegion(ctx, owner, bed.ID, "all", plant.ID, nil, FillClump); err != nil { + if _, err := s.FillNamedRegion(ctx, owner, bed.ID, "all", plant.ID, nil, FillClump, nil); err != nil { t.Fatalf("fill: %v", err) } before, _ := s.store.ListActivePlantingsForObject(ctx, bed.ID) @@ -688,7 +688,7 @@ func TestClearObjectOnlyClearsWhatItSnapshotted(t *testing.T) { plant := seedOwnPlant(t, s, owner, 15) ctx := context.Background() - if _, err := s.FillNamedRegion(ctx, owner, bed.ID, "all", plant.ID, nil, FillClump); err != nil { + if _, err := s.FillNamedRegion(ctx, owner, bed.ID, "all", plant.ID, nil, FillClump, nil); err != nil { t.Fatalf("fill: %v", err) } before, _ := s.store.ListActivePlantingsForObject(ctx, bed.ID) @@ -725,7 +725,7 @@ func TestRevertResultCarriesItsCounts(t *testing.T) { plant := seedOwnPlant(t, s, owner, 15) ctx := context.Background() - if _, err := s.FillNamedRegion(ctx, owner, bed.ID, "all", plant.ID, nil, FillClump); err != nil { + if _, err := s.FillNamedRegion(ctx, owner, bed.ID, "all", plant.ID, nil, FillClump, nil); err != nil { t.Fatalf("fill: %v", err) } // A second, different kind of change, so the breakdown has more than one row @@ -830,7 +830,7 @@ func TestSucceededTurnRecordsEvenIfTheCallerWentAway(t *testing.T) { cs, err := s.WithChangeSet(ctx, owner, g.ID, ChangeSetOptions{ Source: domain.SourceAgent, Summary: "plant beans in the second bed", }, func(ctx context.Context) error { - if _, err := s.FillNamedRegion(ctx, owner, bed.ID, "all", plant.ID, nil, FillClump); err != nil { + if _, err := s.FillNamedRegion(ctx, owner, bed.ID, "all", plant.ID, nil, FillClump, nil); err != nil { return err } cancel() // the client disconnects, mid-turn, after the work landed diff --git a/internal/service/seed_lots_test.go b/internal/service/seed_lots_test.go index a9cb75d..a2f0059 100644 --- a/internal/service/seed_lots_test.go +++ b/internal/service/seed_lots_test.go @@ -99,9 +99,12 @@ func TestRemainingReturnsWhenAPlantingIsRemoved(t *testing.T) { lot := seedLot(t, s, owner, plant.ID, 100, nil) ctx := context.Background() - ten := 10 + // Dated explicitly: left to default, plantedAt is the real UTC day, and the + // removal below has to come after it — a test that only passed before + // 2026-08-01 is the kind of clock bomb this avoids. + ten, planted := 10, "2026-07-01" pl, err := s.CreatePlanting(ctx, owner, bed.ID, PlantingInput{ - PlantID: plant.ID, XCM: 0, YCM: 0, RadiusCM: 20, Count: &ten, SeedLotID: &lot.ID, + PlantID: plant.ID, XCM: 0, YCM: 0, RadiusCM: 20, Count: &ten, SeedLotID: &lot.ID, PlantedAt: &planted, }) if err != nil { t.Fatalf("CreatePlanting: %v", err) diff --git a/web/src/components/gardens/CopyDialog.tsx b/web/src/components/gardens/CopyDialog.tsx index 04fbd57..fdc107d 100644 --- a/web/src/components/gardens/CopyDialog.tsx +++ b/web/src/components/gardens/CopyDialog.tsx @@ -1,4 +1,4 @@ -import { useState, type FormEvent } from 'react' +import { useEffect, useState, type FormEvent } from 'react' import { useNavigate } from '@tanstack/react-router' import { Alert } from '@/components/ui/Alert' import { Button } from '@/components/ui/Button' @@ -6,24 +6,37 @@ import { Dialog } from '@/components/ui/Dialog' import { TextField } from '@/components/ui/Field' import { toast } from '@/components/ui/toast' import { errorMessage } from '@/lib/api' -import { useCopyGarden, type Garden } from '@/lib/gardens' -import { parsePlanName, planNameFor } from '@/lib/plan' +import { useCopyGarden, useGardens, type Garden } from '@/lib/gardens' +import { nextPlanYear, parsePlanName, planNameFor } from '@/lib/plan' /** * Duplicate a garden — the way to scheme a season: the copy is a separate * garden you rearrange freely while this one stays put. Beds and everything * currently planted come along; the share link and shares don't. The name is - * prefilled as " — ", which is what the editor's season - * control and the `plan` tag read back (see lib/plan.ts). On success we land in - * the copy, since the point of copying is to start editing it. + * prefilled as " — " for the next year that doesn't already have a + * plan, which is what the editor's season control and the `plan` tag read back + * (see lib/plan.ts). On success we land in the copy, since the point of copying + * is to start editing it. */ export function CopyDialog({ garden, onClose }: { garden: Garden; onClose: () => void }) { const copy = useCopyGarden() const navigate = useNavigate() + const gardens = useGardens() + const names = (gardens.data ?? []).map((g) => g.name) const base = parsePlanName(garden.name)?.base ?? garden.name - const year = (parsePlanName(garden.name)?.year ?? new Date().getFullYear()) + 1 + const from = (parsePlanName(garden.name)?.year ?? new Date().getFullYear()) + 1 + const year = nextPlanYear(base, names, from) const [name, setName] = useState(() => planNameFor(base, year)) + const [touched, setTouched] = useState(false) const [error, setError] = useState(null) + // The gardens list can still be loading when this opens; until the person + // edits the name, keep the proposal in step with what the list says is free. + useEffect(() => { + if (!touched) setName(planNameFor(base, year)) + }, [base, year, touched]) + // The API allows duplicate names; say so rather than let two gardens read as + // the same season's plan. + const taken = names.some((n) => n.trim() === name.trim()) async function onSubmit(e: FormEvent) { e.preventDefault() @@ -45,8 +58,19 @@ export function CopyDialog({ garden, onClose }: { garden: Garden; onClose: () => A copy of {garden.name} to scheme in — rearrange freely, the original stays put. Beds and what's planted come along; shares and the public link don't.

- setName(e.target.value)} /> + { + setTouched(true) + setName(e.target.value) + }} + />

Keep the “— {year}” and it shows up as that season's plan in the editor.

+ {taken && You already have a garden called “{name.trim()}” — pick another name so the two don't read as the same plan.} {error && {error}}
- + diff --git a/web/src/components/plants/PlantDialog.tsx b/web/src/components/plants/PlantDialog.tsx index fcd5ebb..2adc3fa 100644 --- a/web/src/components/plants/PlantDialog.tsx +++ b/web/src/components/plants/PlantDialog.tsx @@ -15,7 +15,7 @@ import { type PlantInput, } from '@/lib/plants' import { safeExternalUrl } from '@/lib/seedLots' -import { cmFromSpacing, spacingFromCm, spacingUnitLabel, type UnitPref } from '@/lib/units' +import { editSpacingField, spacingField, spacingUnitLabel, type LengthField, type UnitPref } from '@/lib/units' import { ColorSwatches, CURATED_SWATCHES, expandHex } from './ColorSwatches' // Markers are monograms now, but the API still carries an icon per plant; a @@ -26,8 +26,10 @@ const categoryOptions = PLANT_CATEGORIES.map((c) => ({ value: c, label: CATEGORY /** * "A new plant" — or edit (`plant`), or a fresh create prefilled from another * (`template`, the Duplicate action; the only way to customize a built-in). - * Spacing is typed in the page's unit and stored in centimeters. A 409 rebases - * onto the server's current row. + * Spacing is typed in the page's unit and stored in centimeters — as a + * LengthField, so a Save that didn't touch it sends the centimeters that were + * loaded rather than re-parsing "17.7 in" into 44.958. A 409 rebases onto the + * server's current row. */ export function PlantDialog({ plant, @@ -48,7 +50,7 @@ export function PlantDialog({ const [name, setName] = useState(source ? (isEdit ? source.name : `${source.name} (copy)`) : '') const [category, setCategory] = useState(source?.category ?? 'vegetable') - const [spacing, setSpacing] = useState(String(spacingFromCm(source?.spacingCm ?? 30, unit))) + const [spacing, setSpacing] = useState(() => spacingField(source?.spacingCm ?? 30, unit)) const [color, setColor] = useState(expandHex(source?.color ?? CURATED_SWATCHES[0])) const [days, setDays] = useState(source?.daysToMaturity != null ? String(source.daysToMaturity) : '') const [vendor, setVendor] = useState(source?.vendor ?? '') @@ -68,8 +70,8 @@ export function PlantDialog({ setFormError('Give the plant a name.') return } - const spacingCm = cmFromSpacing(parseFloat(spacing), unit) - if (!Number.isFinite(spacingCm) || spacingCm < 1) { + const spacingCm = spacing.cm + if (spacingCm === null || spacingCm < 1) { setFormError(`Spacing must be at least 1 ${unitLabel}.`) return } @@ -97,6 +99,12 @@ export function PlantDialog({ vendor: vendor.trim(), notes: notes.trim(), } + // Nothing changed: close without a request, so a look-and-Save doesn't bump + // the version for every garden that shares the plant. + if (isEdit && (Object.keys(input) as (keyof PlantInput)[]).every((k) => input[k] === (k === 'color' ? expandHex(plant.color) : plant[k]))) { + onClose() + return + } try { if (isEdit) await update.mutateAsync({ id: plant.id, ...input, version }) else await create.mutateAsync(input) @@ -107,7 +115,7 @@ export function PlantDialog({ setVersion(current.version) setName(current.name) setCategory(current.category) - setSpacing(String(spacingFromCm(current.spacingCm, unit))) + setSpacing(spacingField(current.spacingCm, unit)) setColor(expandHex(current.color)) setDays(current.daysToMaturity != null ? String(current.daysToMaturity) : '') setVendor(current.vendor) @@ -142,8 +150,8 @@ export function PlantDialog({ step="any" min="1" required - value={spacing} - onChange={(e) => setSpacing(e.target.value)} + value={spacing.text} + onChange={(e) => setSpacing(editSpacingField(e.target.value, unit))} wrapperClassName="flex-1" />
diff --git a/web/src/editor/Canvas.tsx b/web/src/editor/Canvas.tsx index c3f2331..3ff9058 100644 --- a/web/src/editor/Canvas.tsx +++ b/web/src/editor/Canvas.tsx @@ -10,8 +10,9 @@ import { type PointerEvent as ReactPointerEvent, } from 'react' import { clampScale, type Point } from '@/lib/geometry' +import { monogramInk } from '@/lib/monogram' import { useCreateObject, useCreatePlanting, useUpdateObject, useUpdatePlanting } from '@/lib/objects' -import type { Plant } from '@/lib/plants' +import { FALLBACK_PLANT_COLOR, type Plant } from '@/lib/plants' import type { EditorPlanting } from '@/lib/plantings' import { formatSize } from '@/lib/units' import { kindDef, objectStyle, rectRadius } from './kinds' @@ -589,7 +590,7 @@ export const Canvas = forwardRef< > {/* A fingertip-sized hit area so a tiny plop at low zoom is still grabbable. */} - + ) })} @@ -642,7 +643,7 @@ export const Canvas = forwardRef< textAnchor="middle" dominantBaseline="central" fontSize={r * 1.05} - fill="var(--color-paper)" + fill={monogramInk(plant?.color ?? FALLBACK_PLANT_COLOR)} style={{ fontFamily: 'var(--font-heading)' }} > {letters.get(p.plantId) ?? '?'} diff --git a/web/src/editor/Inspector.tsx b/web/src/editor/Inspector.tsx index d8e6061..b34f21c 100644 --- a/web/src/editor/Inspector.tsx +++ b/web/src/editor/Inspector.tsx @@ -1,13 +1,14 @@ import { useEffect, useMemo, useRef, useState, type ReactNode } from 'react' import { ColorDot } from '@/components/plants/Monogram' import { Button, IconButton } from '@/components/ui/Button' +import { ConfirmDialog } from '@/components/ui/ConfirmDialog' import { TextAreaField, TextField } from '@/components/ui/Field' import { Icon } from '@/components/ui/Icon' import { Tag } from '@/components/ui/Tag' import { Toggle } from '@/components/ui/Toggle' import { cn } from '@/lib/cn' import { useDeleteObject, useRemovePlanting, useUpdateObject, useUpdatePlanting } from '@/lib/objects' -import type { Plant } from '@/lib/plants' +import { FALLBACK_PLANT_COLOR, type Plant } from '@/lib/plants' import type { EditorPlanting } from '@/lib/plantings' import { cmFromSpacing, @@ -22,7 +23,7 @@ import { spacingUnitLabel, type UnitPref, } from '@/lib/units' -import { kindDef, kindPlural } from './kinds' +import { kindDef, kindPlural, objectDisplayName } from './kinds' import { MIN_OBJECT_CM, plopCount } from './shared' import { useEditorStore } from './store' import type { EditorObject } from './types' @@ -40,6 +41,13 @@ export function rosterText(o: EditorObject, plantings: EditorPlanting[], plantsB return 'Growing: ' + [...roster].map(([n, c]) => `${c} ${n}`).join(' · ') } +/** How many plants an object holds right now (its plops × their counts). */ +export function plantCountIn(o: EditorObject, plantings: EditorPlanting[], plantsById: Map): number { + let n = 0 + for (const p of plantings) if (p.objectId === o.id) n += plopCount(p, plantsById.get(p.plantId)) + return n +} + /** A collapsible block of the less-often-needed fields. */ function Details({ open, onToggle, children }: { open: boolean; onToggle: () => void; children: ReactNode }) { return ( @@ -66,6 +74,7 @@ export function ObjectInspector({ canEdit, focused, roster, + plantCount, noteCount, large, onPlantThis, @@ -79,6 +88,8 @@ export function ObjectInspector({ /** Already inside this bed (so "Plant this" is redundant). */ focused: boolean roster: string + /** Live plants in it (see plantCountIn) — Remove asks first when this is > 0. */ + plantCount: number noteCount: number /** Phone: 16px inputs, 44px targets. */ large?: boolean @@ -89,6 +100,7 @@ export function ObjectInspector({ const update = useUpdateObject(gardenId) const del = useDeleteObject(gardenId) const rootRef = useRef(null) + const [confirmRemove, setConfirmRemove] = useState(false) const [name, setName] = useState(object.name) const [details, setDetails] = useState(false) const [width, setWidth] = useState(formatDimensionInput(object.widthCm, unit)) @@ -178,6 +190,13 @@ export function ObjectInspector({ iconClassName="text-accent-700" disabled={del.isPending} onClick={() => { + // An empty object goes straight away (one Undo brings it back); a + // planted one takes its plants with it, which is worth a question — + // on the phone this button sits right beside "Plant this". + if (plantCount > 0) { + setConfirmRemove(true) + return + } onDeleted() del.mutate(object.id) }} @@ -259,6 +278,22 @@ export function ObjectInspector({ setNotes(e.target.value)} onBlur={() => notes !== object.notes && patch({ notes })} /> + {confirmRemove && ( + { + await del.mutateAsync(object.id) + onDeleted() + }} + onClose={() => setConfirmRemove(false)} + > + It has {plantCount === 1 ? 'one plant' : `${plantCount} plants`} in it, and they go with it. One Undo brings + everything back. + + )} ) } @@ -321,7 +356,7 @@ export function PlopInspector({ return (
- + {plant?.name ?? 'Unknown plant'} {noteCount > 0 && (