diff --git a/CLAUDE.md b/CLAUDE.md index 1b7aaa1..dc46238 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -173,10 +173,22 @@ Conventions that follow from it: garden made the old per-plop describe ~450 entries on every turn. A tool that needs individual ids uses `list_plantings`; bulk work takes (object, plant) — `remove_plantings`, `ClearPlantings`. Don't add a tool that lists plops. -- **The assistant can't undo and must not pretend to.** Asked to "undo the - beets", the live model replied "Done!" and changed nothing. The prompt now - forbids claiming a change no tool made and points at the Undo button; keep - both rules when editing `systemPrompt`. + With a `year` it is the season view (`GardenFull(year)`: every plop whose + time in the ground overlapped the year, pulled ones included, with `removed` + / `removedAt` per group) — that is how "what was here last year?" is answered. +- **The assistant undoes through `undo_change`, never by claiming.** Asked to + "undo the beets", the live model once replied "Done!" and changed nothing. + `undo_change` wraps `RevertChangeSet(source=agent)`; the prompt still forbids + claiming a change no tool made. A revert is its own change set (it points at + what it undid), so it never joins the turn's scope — `Run` reports the last + revert as the turn's `ChangeSetID` when the turn made no other change, so the + reply's "Undo this" is a redo. Keep that fallback: without it an undo-only + turn is the one change in the conversation with no undo button. +- **Garden notes are the assistant's memory.** `systemPrompt` quotes + `Garden.Notes` (owner-written, `%q`) as standing context, and `update_garden` + is how the model adds "we're in zone 6a" to them. Notes are replaced whole, + so the tool description tells the model to merge; don't add a second store + for "things the assistant remembers". - **Request deadlines are extended through `responseController(c)`, never `http.NewResponseController(c.Writer)`.** A controller built in a handler diff --git a/DESIGN.md b/DESIGN.md index 94fde36..e73136a 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -149,7 +149,7 @@ React 19 + TypeScript + Vite + Tailwind 4 (`@tailwindcss/vite`), `@tanstack/reac 7. **Sharing** — invite by email, roles, viewer read-only mode. 8. **Polish** — imperial toggle, mobile ergonomics, clear-bed, keyboard nudging. 9. **Agent seam** — `ops.go` bulk ops + `internal/agent` DefineTool wrappers. -10. **Garden assistant** — majordomo in-process, Ollama Cloud, streaming chat. Each turn runs inside ONE change set (`source='agent'`), so a turn that clears a bed and replants it undoes as one action; that is what makes acting without a confirmation prompt defensible. Bounded by a step cap and a timeout — loop safety, not spend control. The `majordomo` build tag is gone: a tag that keeps the agent out of the binary only earns its keep if you'd ship a build without it, and the agent is the point. What a day of live use added: the turn carries the gardener's **local day** (`today` in the chat body) into the prompt and every dated tool default, because the model's own idea of the date was a year stale and the server's is UTC; `describe_garden` **groups plops by plant** (count, where, planted date, days to maturity — `DescribeGroup`) and lists ids only for small groups, with `list_plantings` for the rest and `remove_plantings` to act on a whole group; `move_planting` relocates a plop (`MovePlanting`, within or across beds) keeping its planting date; `fill_region` takes an explicit local rectangle and a `seedLotId`; `update_plant`, `read_history` and `copy_garden` (the " — " plan convention) round out what the model kept reaching for. A mutation on another garden inside a turn is recorded under THAT garden (`record` refuses to file revisions into a scope for a different garden), so undo always finds them where the person is looking. +10. **Garden assistant** — majordomo in-process, Ollama Cloud, streaming chat. Each turn runs inside ONE change set (`source='agent'`), so a turn that clears a bed and replants it undoes as one action; that is what makes acting without a confirmation prompt defensible. Bounded by a step cap and a timeout — loop safety, not spend control. The `majordomo` build tag is gone: a tag that keeps the agent out of the binary only earns its keep if you'd ship a build without it, and the agent is the point. What a day of live use added: the turn carries the gardener's **local day** (`today` in the chat body) into the prompt and every dated tool default, because the model's own idea of the date was a year stale and the server's is UTC; `describe_garden` **groups plops by plant** (count, where, planted date, days to maturity — `DescribeGroup`) and lists ids only for small groups, with `list_plantings` for the rest and `remove_plantings` to act on a whole group; `move_planting` relocates a plop (`MovePlanting`, within or across beds) keeping its planting date; `fill_region` takes an explicit local rectangle and a `seedLotId`; `update_plant`, `read_history` and `copy_garden` (the " — " plan convention) round out what the model kept reaching for. A mutation on another garden inside a turn is recorded under THAT garden (`record` refuses to file revisions into a scope for a different garden), so undo always finds them where the person is looking. The record-keeping round (2026-08-23): `undo_change` exposes `RevertChangeSet` with `source=agent` — the revert is its own change set, so an undo-only turn reports it as the turn's handle and "Undo this" becomes a redo; `describe_garden` takes a `year` (the season view, pulled plops included, `removed`/`removedAt` per group) with `list_years` beside it, for rotation questions; `update_planting`, `update_journal_entry`/`delete_journal_entry` and `update_garden` correct records in place, and `remove_planting`/`remove_plantings`/`clear_object` take a `removedAt` so a harvest can be backdated; the garden's **notes go into the system prompt** as the gardener's standing facts, and `update_garden` is how the assistant remembers what it is told. ## Deliberate v1 limits diff --git a/internal/agent/runtime.go b/internal/agent/runtime.go index 1692869..c7a1cf6 100644 --- a/internal/agent/runtime.go +++ b/internal/agent/runtime.go @@ -120,13 +120,15 @@ func (r *Runner) Run(ctx context.Context, actorID, gardenID int64, message, toda result *agent.Result runErr error truncErr bool + tools *adapter ) changeSet, err := r.svc.WithChangeSet(ctx, actorID, gardenID, service.ChangeSetOptions{ Source: domain.SourceAgent, Summary: turnSummary(message), AgentRunID: &runID, }, func(ctx context.Context) error { - box := NewToolbox(r.svc, actorID, today) + var box *llm.Toolbox + box, tools = newToolbox(r.svc, actorID, today) a := agent.New(r.model, systemPrompt(garden, today), agent.WithMaxSteps(maxSteps), agent.WithToolErrorLimits(maxConsecutiveToolErrors, maxSameCallRepeats), @@ -156,6 +158,12 @@ func (r *Runner) Run(ctx context.Context, actorID, gardenID int64, message, toda turn := &Turn{Truncated: truncErr} if changeSet != nil { turn.ChangeSetID = &changeSet.ID + } else if tools != nil { + // An undo is its own change set, outside the turn's scope (it has to + // point back at what it reverted). A turn that did nothing BUT undo + // would otherwise come back with no handle, and the reply would lose + // the "Undo this" that every other change gets — here it is a redo. + turn.ChangeSetID = tools.lastRevert() } if result != nil { turn.Reply = result.Output @@ -216,7 +224,8 @@ func turnSummary(message string) string { } // systemPrompt gives the model the conventions it cannot infer, the day it is, -// and the rules of conduct the live instance showed it needs. +// the gardener's standing notes, and the rules of conduct the live instance +// showed it needs. // // The compass convention in particular is not guessable: -y is north because // screen y grows downward, and a model that assumes otherwise plants the south @@ -226,6 +235,11 @@ func turnSummary(message string) string { // testing: reported a change it never made, narrated every planting into the // journal, swapped four beds on an ambiguous sentence, and answered an imperial // gardener in centimeters. +// +// The garden's notes are the assistant's memory. They are the owner's own text +// (only the owner can edit them), so they are given as background the gardener +// wrote — zone, frost dates, soil, how they like things done — and update_garden +// is how the assistant adds to them when told something worth keeping. func systemPrompt(g *domain.Garden, today string) string { units := "The gardener works in meters and centimeters; answer in those." size := fmt.Sprintf("%.0f x %.0f cm", g.WidthCM, g.HeightCM) @@ -234,10 +248,19 @@ func systemPrompt(g *domain.Garden, today string) string { "(1 ft = 30.48 cm, 1 in = 2.54 cm) and answer in feet and inches, never in centimeters." size = fmt.Sprintf("%.1f x %.1f ft (%.0f x %.0f cm)", g.WidthCM/30.48, g.HeightCM/30.48, g.WidthCM, g.HeightCM) } + notes := "The gardener has written no notes about this garden yet." + if n := strings.TrimSpace(g.Notes); n != "" { + // %q: the notes are the gardener's own words, but they are data, not + // prompt — quoting keeps a line in them from reading as an instruction + // to someone the garden is shared with. + notes = "The gardener's notes about this garden — their standing facts about the place, to use as " + + "context (zone, frost dates, soil, sun, how they like things done): " + fmt.Sprintf("%q", n) + } return fmt.Sprintf(`You are pansy's garden assistant. You help plan and edit a real garden by calling tools. The garden you are working on is %q (id %d), %s. Today is %s — the gardener's local date. %s +%s Conventions you cannot guess and must not assume: - Every measurement a tool takes or returns is in CENTIMETERS. @@ -263,15 +286,29 @@ How to work: middle third, a strip along one edge) give fill_region a rectangle instead of placing plops by hand. - A garden named %s is this garden's plan for that year; copy_garden with that name makes one. Never use a different real garden as a scratch space. +- Past seasons: describe_garden with a year shows what was in each bed that year, pulled plants + included; list_years says which years have records. Check it before advising on rotation or + answering "what was here last year?" — do not guess from what is growing now. +- To undo something — yours or anyone's — find the change in read_history and call undo_change + with its id. It reverts as a new change that can itself be undone. "Undo the beets" means the + change that planted the beets, not pulling them out today; do not re-create what you can + revert. A change already marked undone stays undone. +- To correct a record rather than change the garden — a planting date, a plant count, a journal + entry's text or date, the garden's notes — use update_planting, update_journal_entry and + update_garden instead of removing and re-adding. +- When the gardener tells you something worth keeping about the place — their zone, usual + frost dates, soil, a standing preference — add it to the garden's notes with update_garden + (keeping what is already there), and say you did. You will see those notes in every later + conversation. - When a tool refuses (for example, the user only has view access to this garden), explain what happened in plain words. Do not retry it. How to behave: - Only claim what a tool actually did. If a tool failed, or there is no tool for what was asked, - say so plainly — never describe a change you did not make. -- You cannot undo. Every reply of yours that changed the garden has an "Undo this" button under - it, and the History panel can revert any change; point the gardener there, or offer to reverse - the change by hand with tools. + say so plainly — never describe a change you did not make, and never say something is undone + unless undo_change did it. +- Every reply of yours that changed the garden has an "Undo this" button under it, and the + History panel can revert any change; mention that when it helps. - When a request could mean materially different things — "swap the cucumbers and the melons" with two beds of each — say what you would do and ask, rather than clearing beds on a guess. When it is clear, just do it. @@ -283,5 +320,5 @@ How to behave: // %q throughout for the garden's name: any editor can rename a garden, and // a name is data, not prompt — quoting keeps a newline or a stray quote // in it from reading as a new instruction. - g.Name, g.ID, size, today, units, fmt.Sprintf("%q", g.Name+" — ")) + g.Name, g.ID, size, today, units, notes, fmt.Sprintf("%q", g.Name+" — ")) } diff --git a/internal/agent/runtime_test.go b/internal/agent/runtime_test.go index 55b3657..d53a9b8 100644 --- a/internal/agent/runtime_test.go +++ b/internal/agent/runtime_test.go @@ -335,7 +335,7 @@ func TestPartialWorkSurvivesATimeout(t *testing.T) { if _, conflicts, rerr := svc.RevertChangeSet(ctx, owner, after[0].ID, domain.SourceUI); rerr != nil || len(conflicts) != 0 { t.Fatalf("the partial turn should be undoable: err=%v conflicts=%+v", rerr, conflicts) } - o, _ := svc.DescribeGarden(ctx, owner, g.ID) + o, _ := svc.DescribeGarden(ctx, owner, g.ID, nil) if len(o.Objects) > 0 && o.Objects[0].XCM != bed.XCM { t.Errorf("undo left the bed at %v, want %v", o.Objects[0].XCM, bed.XCM) } @@ -368,7 +368,7 @@ func TestSystemPromptKnowsTheDayAndTheGardenersUnits(t *testing.T) { "feet and inches", "24.0 x 24.0 ft", "never describe a change you did not make", - "You cannot undo", + "undo_change", "Undo this", "rather than clearing beds on a guess", "not to narrate your own planting", @@ -495,8 +495,96 @@ func TestTurnOnAnotherGardenFilesHistoryThere(t *testing.T) { if _, conflicts, err := svc.RevertChangeSet(ctx, owner, afterB[0].ID, domain.SourceUI); err != nil || len(conflicts) != 0 { t.Fatalf("undo from B: err=%v conflicts=%+v", err, conflicts) } - d, _ := svc.DescribeGarden(ctx, owner, b.ID) + d, _ := svc.DescribeGarden(ctx, owner, b.ID, nil) if len(d.Objects) != 1 || d.Objects[0].Name != "Bed" { t.Errorf("after undo B's bed is %+v, want its original name back", d.Objects) } } + +// TestTurnThatOnlyUndoesIsItselfUndoable — a revert is its own change set, +// outside the turn's scope, so a turn that did nothing but undo would come back +// with no change of its own; the reply would then be the one change in the +// conversation without an "Undo this". It gets the revert instead — a redo. +func TestTurnThatOnlyUndoesIsItselfUndoable(t *testing.T) { + ctx := context.Background() + svc, owner := newAgentTestService(t) + g, err := svc.CreateGarden(ctx, owner, service.GardenInput{Name: "Plot", WidthCM: 2000, HeightCM: 2000}) + if err != nil { + t.Fatalf("garden: %v", err) + } + beets := mustPlant(t, svc, owner, "Beets", 10, "🫜") + bed, err := svc.CreateObject(ctx, owner, g.ID, service.ObjectInput{Kind: domain.KindBed, Name: "Bed", XCM: 1000, YCM: 1000, WidthCM: 400, HeightCM: 400}) + if err != nil { + t.Fatalf("bed: %v", err) + } + // The beets went in by hand in the editor: the change the person wants undone. + if _, err := svc.FillNamedRegion(ctx, owner, bed.ID, "all", beets.ID, nil, service.FillClump, nil); err != nil { + t.Fatalf("plant beets: %v", err) + } + history, _, err := svc.GardenHistory(ctx, owner, g.ID, 0, 0) + if err != nil || len(history) == 0 { + t.Fatalf("history: %v (%d entries)", err, len(history)) + } + planted := history[0] + + r := scriptedRunner(t, svc, + toolCall("read_history", map[string]any{"gardenId": g.ID}), + toolCall("undo_change", map[string]any{"changeSetId": planted.ID}), + fake.Reply("Undone — the beets are out of the bed again."), + ) + turn, err := r.Run(ctx, owner, g.ID, "undo the beets", "", nil, nil) + if err != nil { + t.Fatalf("Run: %v", err) + } + full, err := svc.GardenFull(ctx, owner, g.ID, nil) + if err != nil { + t.Fatalf("GardenFull: %v", err) + } + if len(full.Plantings) != 0 { + t.Fatalf("%d beets still in the bed after the undo", len(full.Plantings)) + } + after, _, _ := svc.GardenHistory(ctx, owner, g.ID, 0, 0) + if len(after) != len(history)+1 { + t.Fatalf("history grew by %d, want exactly the revert", len(after)-len(history)) + } + revert := after[0] + if revert.Source != domain.SourceAgent || revert.RevertsID == nil || *revert.RevertsID != planted.ID { + t.Errorf("newest entry = %+v; want the agent's revert of %d", revert, planted.ID) + } + if turn.ChangeSetID == nil || *turn.ChangeSetID != revert.ID { + t.Fatalf("turn.ChangeSetID = %v, want the revert %d so the reply can offer a redo", turn.ChangeSetID, revert.ID) + } + // And "Undo this" on that reply is a redo. + if _, conflicts, err := svc.RevertChangeSet(ctx, owner, *turn.ChangeSetID, domain.SourceUI); err != nil || len(conflicts) != 0 { + t.Fatalf("redo: err=%v conflicts=%+v", err, conflicts) + } + full, _ = svc.GardenFull(ctx, owner, g.ID, nil) + if len(full.Plantings) == 0 { + t.Error("redoing the turn did not put the beets back") + } +} + +// TestSystemPromptCarriesTheGardenersNotes — the notes are the assistant's +// memory: what the gardener told it about the place comes back on every turn, +// quoted as their words rather than pasted as instructions. +func TestSystemPromptCarriesTheGardenersNotes(t *testing.T) { + with := systemPrompt(&domain.Garden{ID: 1, Name: "Plot", WidthCM: 500, HeightCM: 400, UnitPref: domain.UnitMetric, + Notes: "Zone 6a.\nLast frost \"usually\" May 10."}, "2026-08-23") + for _, want := range []string{ + `"Zone 6a.\nLast frost \"usually\" May 10."`, + "update_garden", + "undo_change", + "describe_garden with a year", + } { + if !strings.Contains(with, want) { + t.Errorf("prompt is missing %q", want) + } + } + if strings.Contains(with, "You cannot undo") { + t.Error("the prompt still says the assistant cannot undo") + } + without := systemPrompt(&domain.Garden{ID: 1, Name: "Plot", WidthCM: 500, HeightCM: 400, UnitPref: domain.UnitMetric}, "2026-08-23") + if !strings.Contains(without, "no notes") { + t.Error("a garden without notes doesn't say so") + } +} diff --git a/internal/agent/tools.go b/internal/agent/tools.go index aa631d4..0a88923 100644 --- a/internal/agent/tools.go +++ b/internal/agent/tools.go @@ -3,7 +3,10 @@ package agent import ( "context" "fmt" + "strconv" "strings" + "sync" + "time" "gitea.stevedudenhoeffer.com/steve/majordomo/llm" @@ -24,19 +27,35 @@ import ( // client, because the server's UTC day is tomorrow by nine in the evening in // Ohio. Empty falls back to the service's UTC today. func NewToolbox(svc *service.Service, actorID int64, today string) *llm.Toolbox { + box, _ := newToolbox(svc, actorID, today) + return box +} + +// newToolbox is NewToolbox plus the adapter behind it, which Run keeps hold of: +// the adapter remembers what undo_change reverted, and a turn that only undid +// something has no other handle to offer as its change. +func newToolbox(svc *service.Service, actorID int64, today string) (*llm.Toolbox, *adapter) { a := &adapter{svc: svc, actor: actorID, today: strings.TrimSpace(today)} return llm.NewToolbox("pansy", llm.DefineTool("list_gardens", "List the gardens the user can see (owned and shared), with the user's role on each.", a.listGardens), llm.DefineTool("describe_garden", - "Summarize a garden: its dimensions, objects (with sizes/positions/version), and each "+ - "object's active plantings grouped by plant — how many, roughly where, when they went in, "+ + "Summarize a garden: its dimensions, notes, version, objects (with sizes/positions/version), "+ + "and each object's plantings grouped by plant — how many, roughly where, when they went in, "+ "and days to maturity when known. A small group lists its plops individually (id + "+ - "version for move_planting/remove_planting, and xCm/yCm in the object's local frame so a "+ - "move can keep their layout); a large one (a grid-filled bed) does not — use "+ - "list_plantings for those, or act on the whole group with remove_plantings.", + "version for move_planting/remove_planting/update_planting, and xCm/yCm in the object's "+ + "local frame so a move can keep their layout); a large one (a grid-filled bed) does not — "+ + "use list_plantings for those, or act on the whole group with remove_plantings. Without a "+ + "year it describes what is growing now; with one it is that season's view — every plop "+ + "whose time in the ground overlapped the year, pulled ones included, each group saying how "+ + "many were removed and when. That is how to answer \"what was in this bed last year?\" and "+ + "to check rotation before replanting. list_years says which years have data.", a.describeGarden), + llm.DefineTool("list_years", + "List the years this garden has planting records for, newest first — the years "+ + "describe_garden can show as a season view.", + a.listYears), llm.DefineTool("list_plantings", "List one object's active plops one by one, each with its id, version, position (xCm/yCm "+ "in the object's local frame), location, count and planting date — the detail "+ @@ -71,21 +90,30 @@ func NewToolbox(svc *service.Service, actorID int64, today string) *llm.Toolbox "is how to relocate plants; removing and re-placing them would lose when they were planted. "+ "Needs the plop's id and version (describe_garden or list_plantings).", a.movePlanting), + llm.DefineTool("update_planting", + "Correct ONE plop's record without moving it: the date it was planted (plantedAt), its "+ + "plant count (count, or clearCount to go back to deriving it from area and spacing), "+ + "its label, its radius, or the seed lot it came from. Use for \"those tomatoes actually "+ + "went in on May 20\" or \"that clump is five plants\". Needs the plop's id and version "+ + "(describe_garden or list_plantings). Only the fields you pass change.", + a.updatePlanting), llm.DefineTool("remove_planting", "Remove ONE plop from a bed, leaving the rest — the single-plant answer to clear_object's "+ "all-or-nothing. Soft-removes it (kept for planting history, undoable), like clearing a "+ "bed does. Needs the plop's id and version (describe_garden or list_plantings). Use "+ - "for \"pull the basil out of the corner\".", + "for \"pull the basil out of the corner\". Dated today unless removedAt says when it "+ + "actually came out (\"I harvested the garlic on Aug 1\").", a.removePlanting), llm.DefineTool("remove_plantings", "Remove every plop of ONE plant from an object, leaving the other plants in it — \"take the "+ "beets out of the south bed\". Soft-removes them (kept for planting history, undoable as "+ - "one change). Use this rather than many remove_planting calls.", + "one change). Use this rather than many remove_planting calls. Dated today unless "+ + "removedAt says when they actually came out.", a.removePlantings), llm.DefineTool("clear_object", "Remove all plants from an object. They are soft-removed, so the planting history for past "+ "seasons is kept and the change can be undone. Use this before replanting a bed with "+ - "something else.", + "something else. Dated today unless removedAt says when the bed was actually cleared.", a.clearObject), llm.DefineTool("find_plant", "Look up plants in the user's catalog by name or category, to get the plantId that "+ @@ -115,16 +143,34 @@ func NewToolbox(svc *service.Service, actorID int64, today string) *llm.Toolbox llm.DefineTool("read_journal", "Read back the garden's grow journal — the observations add_journal_entry wrote. "+ "Narrow it with objectId (one bed), or a from/to date range (YYYY-MM-DD). Most "+ - "recently observed first. Use this to answer \"what did I note about the west bed?\" "+ - "or \"what happened last spring?\".", + "recently observed first. Each entry carries the id and version that "+ + "update_journal_entry and delete_journal_entry need. Use this to answer \"what did I "+ + "note about the west bed?\" or \"what happened last spring?\".", a.readJournal), + llm.DefineTool("update_journal_entry", + "Correct a journal entry the user wrote — its text, or the date it describes — instead "+ + "of adding a second entry that contradicts the first: \"that note was about the "+ + "cucumbers, not the cantaloupe\". Needs the entry's id and version from read_journal. "+ + "Only the user's own entries can be edited.", + a.updateJournalEntry), + llm.DefineTool("delete_journal_entry", + "Delete a journal entry, by its id from read_journal. This is permanent — the journal is "+ + "not in the undo history — so delete only the entry the user pointed at.", + a.deleteJournalEntry), llm.DefineTool("read_history", "Read the garden's change history: every change anyone made — by hand in the editor, or "+ - "in an earlier conversation with you — newest first, with what it changed and whether it "+ - "was undone. Use it to answer \"what changed this week?\" or \"what did you do last time?\" "+ - "rather than reciting from memory. You cannot undo from here; the person has an Undo "+ - "button on each change.", + "in an earlier conversation with you — newest first, with its id, what it changed and "+ + "whether it was undone. Use it to answer \"what changed this week?\" or \"what did you do "+ + "last time?\" rather than reciting from memory, and to find the id undo_change needs.", a.readHistory), + llm.DefineTool("undo_change", + "Undo one change from the history by its id (from read_history): it reverts everything "+ + "that change did, as a new change that can itself be undone. This is how to do \"undo "+ + "the beets\" or \"put it back the way it was\" — find the change in read_history, then "+ + "undo it; never claim to have undone something without calling this. A change already "+ + "marked undone needs no second undo. Anything edited since that change is left alone "+ + "and reported under conflicts; tell the user about those.", + a.undoChange), llm.DefineTool("update_object", "Change an existing object: resize it (widthCm/heightCm), rotate it (rotationDeg), "+ "rename it (name), or toggle whether it can hold plants (plantable). Only the fields "+ @@ -154,7 +200,17 @@ func NewToolbox(svc *service.Service, actorID int64, today string) *llm.Toolbox "(with an em dash) is that garden's plan for the year, and the editor offers it as such. "+ "Use it for \"set up next year's plan\"; never use another real garden as a scratch space.", a.copyGarden), - ) + llm.DefineTool("update_garden", + "Change a garden the user owns: rename it, resize it (widthCm/heightCm), switch its units "+ + "(metric|imperial), set its grid (gridSizeCm, snapToGrid), or rewrite its notes. Only the "+ + "fields you pass change. Needs the garden's current version from describe_garden. The "+ + "notes are the gardener's standing facts about the place — zone, frost dates, soil, sun, "+ + "how they like things done — and you are given them at the start of every conversation, "+ + "so when the user tells you something worth remembering (\"we're in zone 6a\", \"last "+ + "frost is usually around May 10\"), add it here. notes replaces the WHOLE text: take the "+ + "current notes from describe_garden, add the new line, and pass all of it.", + a.updateGarden), + ), a } // adapter carries the service, the acting user and their local day for the @@ -163,6 +219,25 @@ type adapter struct { svc *service.Service actor int64 today string + + mu sync.Mutex + // reverts is every change set undo_change produced this turn. A revert is + // its own change set (it points back at the one it undid, and the target is + // marked undone), so it never joins the turn's scope — which leaves a turn + // that only undid something with no change of its own. Run reports the last + // revert as that turn's handle, so "Undo this" under the reply can redo it. + reverts []int64 +} + +// lastRevert is the newest change set undo_change produced this turn, if any. +func (a *adapter) lastRevert() *int64 { + a.mu.Lock() + defer a.mu.Unlock() + if len(a.reverts) == 0 { + return nil + } + id := a.reverts[len(a.reverts)-1] + return &id } // day is the date a tool stamps: the one the model passed, else the gardener's @@ -184,8 +259,45 @@ func (a *adapter) listGardens(ctx context.Context, _ struct{}) (any, error) { func (a *adapter) describeGarden(ctx context.Context, args struct { GardenID int64 `json:"gardenId" description:"id of the garden to describe"` + Year *int `json:"year" description:"optional: describe that year's season instead of what is growing now — every plop in the ground at any point in the year, pulled ones included"` }) (any, error) { - return a.svc.DescribeGarden(ctx, a.actor, args.GardenID) + return a.svc.DescribeGarden(ctx, a.actor, args.GardenID, args.Year) +} + +func (a *adapter) listYears(ctx context.Context, args struct { + GardenID int64 `json:"gardenId" description:"garden whose planting years to list"` +}) (any, error) { + years, err := a.svc.GardenYears(ctx, a.actor, args.GardenID) + if err != nil { + return nil, err + } + // The service pads the list with ITS current year (UTC); the gardener's may + // differ around New Year. Theirs is the one "this year" means to them. + if y, ok := yearOf(a.today); ok { + present := false + for _, have := range years { + if have == y { + present = true + break + } + } + if !present { + years = append([]int{y}, years...) + } + } + return map[string]any{"years": years}, nil +} + +// yearOf is the year of a YYYY-MM-DD date, or false for anything else. +func yearOf(date string) (int, bool) { + if len(date) < 4 { + return 0, false + } + y, err := strconv.Atoi(date[:4]) + if err != nil { + return 0, false + } + return y, true } func (a *adapter) listPlantings(ctx context.Context, args struct { @@ -344,9 +456,10 @@ func (a *adapter) addJournalEntry(ctx context.Context, args struct { } func (a *adapter) clearObject(ctx context.Context, args struct { - ObjectID int64 `json:"objectId" description:"object to remove all plants from"` + ObjectID int64 `json:"objectId" description:"object to remove all plants from"` + RemovedAt string `json:"removedAt" description:"optional date the plants came out, YYYY-MM-DD; defaults to today"` }) (any, error) { - n, err := a.svc.ClearPlantings(ctx, a.actor, args.ObjectID, service.ClearOptions{RemovedAt: a.day("")}) + n, err := a.svc.ClearPlantings(ctx, a.actor, args.ObjectID, service.ClearOptions{RemovedAt: a.day(args.RemovedAt)}) if err != nil { return nil, err } @@ -354,21 +467,75 @@ func (a *adapter) clearObject(ctx context.Context, args struct { } func (a *adapter) removePlantings(ctx context.Context, args struct { - ObjectID int64 `json:"objectId" description:"object to remove the plant from"` - PlantID int64 `json:"plantId" description:"the plant to remove every plop of (from describe_garden)"` + ObjectID int64 `json:"objectId" description:"object to remove the plant from"` + PlantID int64 `json:"plantId" description:"the plant to remove every plop of (from describe_garden)"` + RemovedAt string `json:"removedAt" description:"optional date they came out, YYYY-MM-DD; defaults to today"` }) (any, error) { if args.PlantID == 0 { // Left out, it would "remove" plant 0 — nothing — and report success. return nil, fmt.Errorf("%w: plantId is required — say which plant to remove, or use clear_object for all of them", domain.ErrInvalidInput) } n, err := a.svc.ClearPlantings(ctx, a.actor, args.ObjectID, - service.ClearOptions{PlantID: &args.PlantID, RemovedAt: a.day("")}) + service.ClearOptions{PlantID: &args.PlantID, RemovedAt: a.day(args.RemovedAt)}) if err != nil { return nil, err } return map[string]int{"removed": n}, nil } +func (a *adapter) updatePlanting(ctx context.Context, args struct { + PlantingID int64 `json:"plantingId" description:"plop to correct (its id from describe_garden or list_plantings)"` + Version int64 `json:"version" description:"the plop's current version"` + PlantedAt *string `json:"plantedAt" description:"optional corrected planting date, YYYY-MM-DD"` + Count *int `json:"count" description:"optional explicit plant count for the plop"` + ClearCount bool `json:"clearCount" description:"optional: drop an explicit count and derive it from area and spacing again"` + Label *string `json:"label" description:"optional label for the plop; empty clears it"` + RadiusCM *float64 `json:"radiusCm" description:"optional new radius in cm"` + SeedLotID *int64 `json:"seedLotId" description:"optional seed lot (from list_seed_lots) to attribute it to"` + ClearLot bool `json:"clearSeedLot" description:"optional: detach it from its seed lot"` +}) (any, error) { + patch := service.PlantingPatch{RadiusCM: args.RadiusCM} + if args.PlantedAt != nil { + if _, err := parseDay(*args.PlantedAt); err != nil { + return nil, err + } + patch.SetPlantedAt, patch.PlantedAt = true, args.PlantedAt + } + switch { + case args.ClearCount && args.Count != nil: + return nil, fmt.Errorf("%w: give a count or clearCount, not both", domain.ErrInvalidInput) + case args.ClearCount: + patch.SetCount = true + case args.Count != nil: + patch.SetCount, patch.Count = true, args.Count + } + if args.Label != nil { + patch.SetLabel = true + if strings.TrimSpace(*args.Label) != "" { + patch.Label = args.Label + } + } + switch { + case args.ClearLot && args.SeedLotID != nil: + return nil, fmt.Errorf("%w: give a seedLotId or clearSeedLot, not both", domain.ErrInvalidInput) + case args.ClearLot: + patch.SetSeedLotID = true + case args.SeedLotID != nil: + patch.SetSeedLotID, patch.SeedLotID = true, args.SeedLotID + } + return a.svc.UpdatePlanting(ctx, a.actor, args.PlantingID, patch, args.Version) +} + +// parseDay checks a date the model typed, so a malformed one fails with a +// message about the date rather than as a bare "invalid input" from the store. +func parseDay(s string) (string, error) { + s = strings.TrimSpace(s) + if _, err := time.Parse(dateLayout, s); err != nil { + return "", fmt.Errorf("%w: %q is not a YYYY-MM-DD date", domain.ErrInvalidInput, s) + } + return s, nil +} + func (a *adapter) readJournal(ctx context.Context, args struct { GardenID int64 `json:"gardenId" description:"garden whose journal to read"` ObjectID *int64 `json:"objectId" description:"optional bed to narrow to; omit for the whole garden"` @@ -391,6 +558,75 @@ func (a *adapter) readJournal(ctx context.Context, args struct { return map[string]any{"entries": entries, "hasMore": hasMore}, nil } +func (a *adapter) updateJournalEntry(ctx context.Context, args struct { + EntryID int64 `json:"entryId" description:"journal entry to correct (its id from read_journal)"` + Version int64 `json:"version" description:"the entry's current version (from read_journal)"` + Body *string `json:"body" description:"optional corrected text"` + ObservedAt *string `json:"observedAt" description:"optional corrected date it happened, YYYY-MM-DD"` +}) (any, error) { + if args.Body == nil && args.ObservedAt == nil { + return nil, fmt.Errorf("%w: say what to change — the body, the date, or both", domain.ErrInvalidInput) + } + if args.ObservedAt != nil { + if _, err := parseDay(*args.ObservedAt); err != nil { + return nil, err + } + } + return a.svc.UpdateJournalEntry(ctx, a.actor, args.EntryID, + service.JournalPatch{Body: args.Body, ObservedAt: args.ObservedAt}, args.Version) +} + +func (a *adapter) deleteJournalEntry(ctx context.Context, args struct { + EntryID int64 `json:"entryId" description:"journal entry to delete (its id from read_journal)"` +}) (any, error) { + if err := a.svc.DeleteJournalEntry(ctx, a.actor, args.EntryID); err != nil { + return nil, err + } + return map[string]any{"deleted": args.EntryID}, nil +} + +// undoResult is what undo_change reports: the change it reverted, the new +// change set that did so (undoable in turn), and what it had to leave alone. +type undoResult struct { + UndoneID int64 `json:"undoneId"` + // ChangeSet is the revert itself — the history entry that can be undone + // to redo — and Summary its row in the history ("Undid: …"). + ChangeSet *int64 `json:"changeSetId,omitempty"` + Summary string `json:"summary,omitempty"` + Changes string `json:"changes"` + Conflicts []domain.RevertConflict `json:"conflicts"` + Note string `json:"note,omitempty"` +} + +func (a *adapter) undoChange(ctx context.Context, args struct { + ChangeSetID int64 `json:"changeSetId" description:"the change to undo — its id from read_history"` +}) (any, error) { + if args.ChangeSetID == 0 { + return nil, fmt.Errorf("%w: changeSetId is required — find the change in read_history first", domain.ErrInvalidInput) + } + cs, conflicts, err := a.svc.RevertChangeSet(ctx, a.actor, args.ChangeSetID, domain.SourceAgent) + if err != nil { + return nil, err + } + res := undoResult{UndoneID: args.ChangeSetID, Conflicts: conflicts} + if cs == nil { + // Nothing applied: every revision was a conflict, or the set was empty. + res.Changes = "nothing" + res.Note = "Nothing was reverted — everything that change touched has been edited since, or there was nothing left to undo." + return res, nil + } + res.ChangeSet = &cs.ID + res.Summary = cs.Summary + res.Changes = describeCounts(cs.Counts) + if len(conflicts) > 0 { + res.Note = "Part of the change was left alone because it had been edited since; see conflicts." + } + a.mu.Lock() + a.reverts = append(a.reverts, cs.ID) + a.mu.Unlock() + return res, nil +} + // historyEntry is one change set as read_history reports it: the row a person // would read in the History panel, not the revision snapshots behind it. type historyEntry struct { @@ -471,12 +707,14 @@ func (a *adapter) deleteObject(ctx context.Context, args struct { } func (a *adapter) removePlanting(ctx context.Context, args struct { - PlantingID int64 `json:"plantingId" description:"plop to remove (its id from describe_garden)"` - Version int64 `json:"version" description:"the plop's current version (from describe_garden)"` + PlantingID int64 `json:"plantingId" description:"plop to remove (its id from describe_garden)"` + Version int64 `json:"version" description:"the plop's current version (from describe_garden)"` + RemovedAt string `json:"removedAt" description:"optional date it came out, YYYY-MM-DD; defaults to today"` }) (any, error) { - // Soft-remove via the service, dated the gardener's local day like every - // other tool here (the service clock's UTC day when that isn't known). - return a.svc.RemovePlanting(ctx, a.actor, args.PlantingID, args.Version, a.day("")) + // Soft-remove via the service, dated the day the gardener said, else their + // local day like every other tool here (the service clock's UTC day when + // that isn't known). + return a.svc.RemovePlanting(ctx, a.actor, args.PlantingID, args.Version, a.day(args.RemovedAt)) } func (a *adapter) listSeedLots(ctx context.Context, args struct { @@ -507,3 +745,53 @@ func (a *adapter) copyGarden(ctx context.Context, args struct { }) (any, error) { return a.svc.CopyGarden(ctx, a.actor, args.GardenID, args.Name) } + +func (a *adapter) updateGarden(ctx context.Context, args struct { + GardenID int64 `json:"gardenId" description:"garden to change (the user must own it)"` + Version int64 `json:"version" description:"the garden's current version (from describe_garden)"` + Name *string `json:"name" description:"optional new name"` + WidthCM *float64 `json:"widthCm" description:"optional new width in cm"` + HeightCM *float64 `json:"heightCm" description:"optional new height in cm"` + UnitPref *string `json:"units" description:"optional: metric | imperial — how the gardener wants lengths shown"` + Notes *string `json:"notes" description:"optional replacement for the WHOLE notes text (merge the current notes in yourself); empty clears them"` + GridSizeCM *float64 `json:"gridSizeCm" description:"optional grid spacing for the editor, in cm"` + SnapToGrid *bool `json:"snapToGrid" description:"optional: whether objects snap to that grid"` +}) (any, error) { + if args.Name == nil && args.WidthCM == nil && args.HeightCM == nil && args.UnitPref == nil && + args.Notes == nil && args.GridSizeCM == nil && args.SnapToGrid == nil { + return nil, fmt.Errorf("%w: say what to change about the garden", domain.ErrInvalidInput) + } + // UpdateGarden takes the whole row, so start from the current one and + // overlay what the model passed — the same merge the editor's settings + // dialog does, and the only way a one-field change leaves the rest alone. + g, err := a.svc.GetGarden(ctx, a.actor, args.GardenID) + if err != nil { + return nil, err + } + in := service.GardenInput{ + Name: g.Name, WidthCM: g.WidthCM, HeightCM: g.HeightCM, UnitPref: g.UnitPref, + Notes: g.Notes, GridSizeCM: g.GridSizeCM, SnapToGrid: g.SnapToGrid, + } + if args.Name != nil { + in.Name = *args.Name + } + if args.WidthCM != nil { + in.WidthCM = *args.WidthCM + } + if args.HeightCM != nil { + in.HeightCM = *args.HeightCM + } + if args.UnitPref != nil { + in.UnitPref = strings.ToLower(strings.TrimSpace(*args.UnitPref)) + } + if args.Notes != nil { + in.Notes = *args.Notes + } + if args.GridSizeCM != nil { + in.GridSizeCM = *args.GridSizeCM + } + if args.SnapToGrid != nil { + in.SnapToGrid = *args.SnapToGrid + } + return a.svc.UpdateGarden(ctx, a.actor, args.GardenID, in, args.Version) +} diff --git a/internal/agent/tools_test.go b/internal/agent/tools_test.go index 02b5803..de4ef2c 100644 --- a/internal/agent/tools_test.go +++ b/internal/agent/tools_test.go @@ -735,3 +735,173 @@ func TestToolsDefaultToTheServiceDayWithoutOne(t *testing.T) { t.Errorf("an explicit day beats the default: %v", d) } } + +// TestRecordKeepingTools covers the tools that correct the record rather than +// change the garden — and the one that undoes a change for real. Each answers a +// thing the live assistant could not do: fix a planting date, backdate a +// harvest, correct a journal note, remember the gardener's zone, see last +// season, and undo without pretending. +func TestRecordKeepingTools(t *testing.T) { + ctx := context.Background() + svc, owner := newAgentTestService(t) + box := NewToolbox(svc, owner, "2026-08-23") + + call := func(name string, args any) llm.ToolResult { + t.Helper() + return box.Execute(ctx, llm.ToolCall{ID: "1", Name: name, Arguments: mustJSON(t, args)}) + } + mustCall := func(name string, args any, into any) { + t.Helper() + res := call(name, args) + if res.IsError { + t.Fatalf("%s: %s", name, res.Content) + } + if into != nil { + if err := json.Unmarshal([]byte(res.Content), into); err != nil { + t.Fatalf("decode %s: %v (%s)", name, err, res.Content) + } + } + } + + g, err := svc.CreateGarden(ctx, owner, service.GardenInput{Name: "Home", WidthCM: 1200, HeightCM: 800, Notes: "Zone 6a."}) + if err != nil { + t.Fatalf("garden: %v", err) + } + beet := mustPlant(t, svc, owner, "Beet", 10, "🫜") + bed, err := svc.CreateObject(ctx, owner, g.ID, service.ObjectInput{Kind: domain.KindBed, Name: "South bed", XCM: 600, YCM: 400, WidthCM: 400, HeightCM: 200}) + if err != nil { + t.Fatalf("bed: %v", err) + } + + // --- update_garden: one field changes, the rest survive, notes merge by hand. + var desc service.DescribeResult + mustCall("describe_garden", map[string]any{"gardenId": g.ID}, &desc) + if desc.Notes != "Zone 6a." || desc.Version != g.Version { + t.Fatalf("describe carries notes %q v%d, want %q v%d", desc.Notes, desc.Version, "Zone 6a.", g.Version) + } + if r := call("update_garden", map[string]any{"gardenId": g.ID, "version": desc.Version}); !r.IsError || !strings.Contains(r.Content, "what to change") { + t.Errorf("update_garden with nothing to change = %q, want a refusal that says so", r.Content) + } + var updated domain.Garden + mustCall("update_garden", map[string]any{ + "gardenId": g.ID, "version": desc.Version, "notes": desc.Notes + "\nLast frost is usually around May 10.", + }, &updated) + if updated.Name != "Home" || updated.WidthCM != 1200 || updated.HeightCM != 800 || updated.UnitPref != domain.UnitMetric { + t.Errorf("a notes-only update changed other fields: %+v", updated) + } + if !strings.HasPrefix(updated.Notes, "Zone 6a.") || !strings.Contains(updated.Notes, "May 10") { + t.Errorf("notes = %q, want the old note kept and the new line added", updated.Notes) + } + if r := call("update_garden", map[string]any{"gardenId": g.ID, "version": desc.Version, "name": "Stale"}); !r.IsError { + t.Error("update_garden with a stale version succeeded") + } + mustCall("update_garden", map[string]any{"gardenId": g.ID, "version": updated.Version, "units": "Imperial"}, &updated) + if updated.UnitPref != domain.UnitImperial { + t.Errorf("units = %q after asking for imperial", updated.UnitPref) + } + + // --- update_planting: correct a plop's record without touching its position. + var plop domain.Planting + mustCall("place_planting", map[string]any{"objectId": bed.ID, "plantId": beet.ID, "xCm": 50, "yCm": -30, "radiusCm": 20, "plantedAt": "2026-05-01"}, &plop) + mustCall("update_planting", map[string]any{"plantingId": plop.ID, "version": plop.Version, "count": 5, "label": "from the market"}, &plop) + if plop.Count == nil || *plop.Count != 5 || plop.Label == nil || *plop.Label != "from the market" || plop.XCM != 50 { + t.Errorf("after count+label: %+v", plop) + } + // Decoded into a fresh value: a field the response omits must read as + // cleared, not as whatever the previous decode left in the pointer. + cleared, version := domain.Planting{}, plop.Version + mustCall("update_planting", map[string]any{"plantingId": plop.ID, "version": version, "clearCount": true, "plantedAt": "2026-05-20", "label": ""}, &cleared) + plop = cleared + if plop.Count != nil || plop.Label != nil || plop.PlantedAt == nil || *plop.PlantedAt != "2026-05-20" { + t.Errorf("after clearCount/plantedAt/empty label: %+v", plop) + } + if r := call("update_planting", map[string]any{"plantingId": plop.ID, "version": plop.Version, "plantedAt": "May 20"}); !r.IsError || !strings.Contains(r.Content, "YYYY-MM-DD") { + t.Errorf("a prose date = %q, want a refusal naming the format", r.Content) + } + if r := call("update_planting", map[string]any{"plantingId": plop.ID, "version": plop.Version, "count": 3, "clearCount": true}); !r.IsError { + t.Error("count and clearCount together were accepted") + } + + // --- remove_planting on the day the gardener said, not today. + mustCall("remove_planting", map[string]any{"plantingId": plop.ID, "version": plop.Version, "removedAt": "2026-08-01"}, &plop) + if plop.RemovedAt == nil || *plop.RemovedAt != "2026-08-01" { + t.Errorf("removedAt = %v, want the harvest date 2026-08-01", plop.RemovedAt) + } + + // --- the season view sees it, the live view does not. + var years struct{ Years []int } + mustCall("list_years", map[string]any{"gardenId": g.ID}, &years) + if len(years.Years) == 0 || years.Years[0] != 2026 { + t.Errorf("years = %v, want 2026 first", years.Years) + } + mustCall("describe_garden", map[string]any{"gardenId": g.ID}, &desc) + if len(desc.Objects[0].Plantings) != 0 { + t.Errorf("the live describe still lists the pulled beet: %+v", desc.Objects[0].Plantings) + } + mustCall("describe_garden", map[string]any{"gardenId": g.ID, "year": 2026}, &desc) + if desc.Year == nil || *desc.Year != 2026 || len(desc.Objects[0].Plantings) != 1 { + t.Fatalf("2026 describe = year %v, %d groups; want the beet group", desc.Year, len(desc.Objects[0].Plantings)) + } + if gr := desc.Objects[0].Plantings[0]; gr.Removed != 1 || gr.RemovedAt != "2026-08-01" || gr.PlantedAt != "2026-05-20" { + t.Errorf("2026 beet group = %+v; want 1 removed 2026-08-01, planted 2026-05-20", gr) + } + + // --- undo_change: the removal is the newest history entry; undoing it puts + // the beet back, as a change that is itself in the history and undoable. + var hist struct { + Entries []historyEntry `json:"entries"` + } + mustCall("read_history", map[string]any{"gardenId": g.ID, "limit": 5}, &hist) + if len(hist.Entries) == 0 || !strings.HasPrefix(hist.Entries[0].Summary, "Removed Beet") { + t.Fatalf("history[0] = %+v, want the beet's removal", hist.Entries) + } + removal := hist.Entries[0].ID + if r := call("undo_change", map[string]any{}); !r.IsError || !strings.Contains(r.Content, "read_history") { + t.Errorf("undo_change without an id = %q, want a refusal pointing at read_history", r.Content) + } + var undone undoResult + mustCall("undo_change", map[string]any{"changeSetId": removal}, &undone) + if undone.ChangeSet == nil || undone.UndoneID != removal || len(undone.Conflicts) != 0 || !strings.Contains(undone.Changes, "1 planting updated") { + t.Errorf("undo result = %+v; want a new change set, no conflicts, one planting updated", undone) + } + mustCall("describe_garden", map[string]any{"gardenId": g.ID}, &desc) + if len(desc.Objects[0].Plantings) != 1 || desc.Objects[0].Plantings[0].Each[0].ID != plop.ID { + t.Errorf("after the undo the beet is not back: %+v", desc.Objects[0].Plantings) + } + mustCall("read_history", map[string]any{"gardenId": g.ID, "limit": 5}, &hist) + if e := hist.Entries[0]; e.ID != *undone.ChangeSet || e.UndoOf == nil || *e.UndoOf != removal || e.Source != domain.SourceAgent { + t.Errorf("history[0] after undo = %+v; want the agent's revert of %d", e, removal) + } + if !hist.Entries[1].Undone { + t.Error("the removal is not marked undone") + } + if got := (&adapter{}).lastRevert(); got != nil { + t.Errorf("a fresh adapter remembers a revert: %v", *got) + } + + // --- the journal: correct an entry in place, then delete it. + var entry domain.JournalEntry + mustCall("add_journal_entry", map[string]any{"gardenId": g.ID, "body": "Aphids on the cantaloupe.", "observedAt": "2026-08-20"}, &entry) + if r := call("update_journal_entry", map[string]any{"entryId": entry.ID, "version": entry.Version}); !r.IsError { + t.Error("update_journal_entry with nothing to change succeeded") + } + if r := call("update_journal_entry", map[string]any{"entryId": entry.ID, "version": entry.Version, "observedAt": "yesterday"}); !r.IsError || !strings.Contains(r.Content, "YYYY-MM-DD") { + t.Errorf("a prose date = %q, want a refusal naming the format", r.Content) + } + mustCall("update_journal_entry", map[string]any{"entryId": entry.ID, "version": entry.Version, "body": "Aphids on the cucumbers.", "observedAt": "2026-08-19"}, &entry) + if entry.Body != "Aphids on the cucumbers." || entry.ObservedAt != "2026-08-19" { + t.Errorf("corrected entry = %+v", entry) + } + var journal struct { + Entries []domain.JournalEntry `json:"entries"` + } + mustCall("read_journal", map[string]any{"gardenId": g.ID}, &journal) + if len(journal.Entries) != 1 || journal.Entries[0].Body != "Aphids on the cucumbers." { + t.Errorf("journal after the correction = %+v, want the one corrected entry", journal.Entries) + } + mustCall("delete_journal_entry", map[string]any{"entryId": entry.ID}, nil) + mustCall("read_journal", map[string]any{"gardenId": g.ID}, &journal) + if len(journal.Entries) != 0 { + t.Errorf("journal after the delete = %+v, want empty", journal.Entries) + } +} diff --git a/internal/service/ops.go b/internal/service/ops.go index 5c7340d..50f952c 100644 --- a/internal/service/ops.go +++ b/internal/service/ops.go @@ -550,13 +550,22 @@ func (s *Service) ClearPlantings(ctx context.Context, actorID, objectID int64, o } // DescribeResult is a structured summary of a garden for prompting an agent. +// Version and Notes are here for update_garden: the version is its guard, and +// the notes are the whole text a new note has to be merged into. type DescribeResult struct { - GardenID int64 `json:"gardenId"` - Name string `json:"name"` - WidthCM float64 `json:"widthCm"` - HeightCM float64 `json:"heightCm"` - UnitPref string `json:"unitPref"` - Objects []DescribeObject `json:"objects"` + GardenID int64 `json:"gardenId"` + Name string `json:"name"` + WidthCM float64 `json:"widthCm"` + HeightCM float64 `json:"heightCm"` + UnitPref string `json:"unitPref"` + GridSizeCM float64 `json:"gridSizeCm"` + Notes string `json:"notes,omitempty"` + Version int64 `json:"version"` + // Year is set on a season view: the plantings are then every plop whose time + // in the ground overlapped that year, pulled ones included, rather than what + // is growing now. + Year *int `json:"year,omitempty"` + Objects []DescribeObject `json:"objects"` } // DescribeObject is one object plus its active plantings grouped by plant, for @@ -605,6 +614,11 @@ type DescribeGroup struct { // DaysToMaturity is the plant's, when the catalog knows it — with PlantedAt, // enough to say when the harvest is due. DaysToMaturity *int `json:"daysToMaturity,omitempty"` + // Removed counts the plops in the group that have been pulled, and RemovedAt + // is when ("first…last" when they differ). Only a season view lists pulled + // plops, so both are absent from a describe of what is growing now. + Removed int `json:"removed,omitempty"` + RemovedAt string `json:"removedAt,omitempty"` // Each lists the plops individually (id, version, position, location) only // when the group has at most maxListedPlops of them. Each []DescribePlanting `json:"each,omitempty"` @@ -626,14 +640,18 @@ type DescribePlanting struct { Location string `json:"location"` RadiusCM float64 `json:"radiusCm"` PlantedAt string `json:"plantedAt,omitempty"` + // RemovedAt is set on a pulled plop, which only a season view lists. + RemovedAt string `json:"removedAt,omitempty"` } // DescribeGarden returns a structured summary — dimensions, objects, and each -// object's active plantings grouped by plant (count, rough location, planting -// date) — for a garden the actor can view. Built on GardenFull so it inherits -// the ACL check. -func (s *Service) DescribeGarden(ctx context.Context, actorID, gardenID int64) (*DescribeResult, error) { - full, err := s.GardenFull(ctx, actorID, gardenID, nil) +// object's plantings grouped by plant (count, rough location, planting date) — +// for a garden the actor can view. year nil describes what is growing now; a +// year is the season view, every plop whose time in the ground overlapped it, +// pulled ones included — what "what was in this bed last year?" needs. Built +// on GardenFull so it inherits the ACL check and the year's bounds. +func (s *Service) DescribeGarden(ctx context.Context, actorID, gardenID int64, year *int) (*DescribeResult, error) { + full, err := s.GardenFull(ctx, actorID, gardenID, year) if err != nil { return nil, err } @@ -648,12 +666,16 @@ func (s *Service) DescribeGarden(ctx context.Context, actorID, gardenID int64) ( } res := &DescribeResult{ - GardenID: full.Garden.ID, - Name: full.Garden.Name, - WidthCM: full.Garden.WidthCM, - HeightCM: full.Garden.HeightCM, - UnitPref: full.Garden.UnitPref, - Objects: make([]DescribeObject, 0, len(full.Objects)), + GardenID: full.Garden.ID, + Name: full.Garden.Name, + WidthCM: full.Garden.WidthCM, + HeightCM: full.Garden.HeightCM, + UnitPref: full.Garden.UnitPref, + GridSizeCM: full.Garden.GridSizeCM, + Notes: full.Garden.Notes, + Version: full.Garden.Version, + Year: year, + Objects: make([]DescribeObject, 0, len(full.Objects)), } for i := range full.Objects { o := &full.Objects[i] @@ -722,9 +744,13 @@ func describeGroups(o *domain.GardenObject, plops []domain.Planting, plantByID m PlantID: pid, Plant: plant.Name, Plops: len(members), Where: summarizeWhere(o, members), PlantedAt: dateRange(members), DaysToMaturity: plant.DaysToMaturity, + RemovedAt: dateRangeOf(members, func(pl domain.Planting) *string { return pl.RemovedAt }), } for _, pl := range members { g.Plants += effectiveCount(pl) + if pl.RemovedAt != nil { + g.Removed++ + } } if len(members) <= maxListedPlops { g.Each = make([]DescribePlanting, 0, len(members)) @@ -746,6 +772,9 @@ func describePlanting(pl domain.Planting, plantName string) DescribePlanting { if pl.PlantedAt != nil { d.PlantedAt = *pl.PlantedAt } + if pl.RemovedAt != nil { + d.RemovedAt = *pl.RemovedAt + } return d } @@ -759,19 +788,26 @@ func effectiveCount(pl domain.Planting) int { } // dateRange is the planting date shared by a group's plops, "first…last" when -// they were planted on different days, or "" when none is dated. ISO dates -// order as strings, so min/max need no parsing. +// they were planted on different days, or "" when none is dated. func dateRange(plops []domain.Planting) string { + return dateRangeOf(plops, func(pl domain.Planting) *string { return pl.PlantedAt }) +} + +// dateRangeOf summarizes one date field across a group's plops: the one date +// they share, "first…last" when they differ, or "" when none is set. ISO dates +// order as strings, so min/max need no parsing. +func dateRangeOf(plops []domain.Planting, pick func(domain.Planting) *string) string { first, last := "", "" for _, pl := range plops { - if pl.PlantedAt == nil || *pl.PlantedAt == "" { + d := pick(pl) + if d == nil || *d == "" { continue } - if first == "" || *pl.PlantedAt < first { - first = *pl.PlantedAt + if first == "" || *d < first { + first = *d } - if *pl.PlantedAt > last { - last = *pl.PlantedAt + if *d > last { + last = *d } } if first == last { diff --git a/internal/service/ops_test.go b/internal/service/ops_test.go index 8ba0982..81da834 100644 --- a/internal/service/ops_test.go +++ b/internal/service/ops_test.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "math" + "reflect" "sort" "testing" @@ -439,7 +440,7 @@ func TestOpsForbiddenForViewer(t *testing.T) { t.Errorf("viewer clear = %v, want ErrForbidden", err) } // But a viewer can DescribeGarden (read). - if _, err := s.DescribeGarden(ctx, viewer, g.ID); err != nil { + if _, err := s.DescribeGarden(ctx, viewer, g.ID, nil); err != nil { t.Errorf("viewer describe = %v, want ok", err) } } @@ -470,7 +471,7 @@ func TestFillScenario(t *testing.T) { fill("nw", basil.ID) fill("south", beans.ID) - desc, err := s.DescribeGarden(ctx, owner, g.ID) + desc, err := s.DescribeGarden(ctx, owner, g.ID, nil) if err != nil { t.Fatalf("DescribeGarden: %v", err) } @@ -585,7 +586,7 @@ func TestDescribeGardenGroupsByPlant(t *testing.T) { } } - desc, err := s.DescribeGarden(ctx, owner, g.ID) + desc, err := s.DescribeGarden(ctx, owner, g.ID, nil) if err != nil { t.Fatalf("DescribeGarden: %v", err) } @@ -825,3 +826,97 @@ func TestFillByRectangleAttributesSeed(t *testing.T) { t.Errorf("overhanging rectangle: %d plops, %v; want some", len(created), err) } } + +// TestDescribeGardenByYear — "what was in this bed last year?" is the question +// rotation advice hangs on, and a describe of what is growing now cannot answer +// it. With a year, describe is the season view: every plop whose time in the +// ground overlapped the year, pulled ones included, each group saying how many +// came out and when. +func TestDescribeGardenByYear(t *testing.T) { + ctx := context.Background() + s := newTestService(t, openConfig()) + owner := seedUser(t, s, "a@example.com") + g, err := s.CreateGarden(ctx, owner, GardenInput{Name: "Seasons", WidthCM: 2000, HeightCM: 2000, Notes: "Zone 6a"}) + if err != nil { + t.Fatalf("garden: %v", err) + } + bed := seedFillBed(t, s, owner, g.ID, 400, 400) + garlic := seedNamedPlant(t, s, owner, "Garlic", 15) + beans := seedNamedPlant(t, s, owner, "Beans", 10) + basil := seedNamedPlant(t, s, owner, "Basil", 25) + + plantAndPull := func(plantID int64, x float64, planted, pulled string) { + t.Helper() + pl, err := s.CreatePlanting(ctx, owner, bed.ID, PlantingInput{PlantID: plantID, XCM: x, YCM: -100, RadiusCM: 20, PlantedAt: &planted}) + if err != nil { + t.Fatalf("plant %d: %v", plantID, err) + } + if pulled != "" { + if _, err := s.RemovePlanting(ctx, owner, pl.ID, pl.Version, &pulled); err != nil { + t.Fatalf("pull %d: %v", pl.ID, err) + } + } + } + plantAndPull(garlic.ID, -100, "2025-10-15", "2026-07-01") // overwintered: in both years + plantAndPull(beans.ID, 0, "2025-05-01", "2025-09-01") // 2025 only + plantAndPull(basil.ID, 100, "2026-06-01", "") // growing now + + groupsOf := func(year *int) map[string]DescribeGroup { + t.Helper() + desc, err := s.DescribeGarden(ctx, owner, g.ID, year) + if err != nil { + t.Fatalf("DescribeGarden(%v): %v", year, err) + } + if (year == nil) != (desc.Year == nil) || (year != nil && *desc.Year != *year) { + t.Errorf("describe(%v) reports year %v", year, desc.Year) + } + if desc.Notes != "Zone 6a" || desc.Version != g.Version { + t.Errorf("describe carries notes %q version %d; want the garden's (%q, %d)", desc.Notes, desc.Version, "Zone 6a", g.Version) + } + out := map[string]DescribeGroup{} + for _, gr := range desc.Objects[0].Plantings { + out[gr.Plant] = gr + } + return out + } + names := func(m map[string]DescribeGroup) []string { + var out []string + for n := range m { + out = append(out, n) + } + sort.Strings(out) + return out + } + + now := groupsOf(nil) + if got := names(now); !reflect.DeepEqual(got, []string{"Basil"}) { + t.Errorf("now = %v, want only the basil still growing", got) + } + if b := now["Basil"]; b.Removed != 0 || b.RemovedAt != "" || b.Each[0].RemovedAt != "" { + t.Errorf("a live plop reports a removal: %+v", b) + } + + y2025 := 2025 + last := groupsOf(&y2025) + if got := names(last); !reflect.DeepEqual(got, []string{"Beans", "Garlic"}) { + t.Errorf("2025 = %v, want the beans and the overwintered garlic", got) + } + if b := last["Beans"]; b.Removed != 1 || b.RemovedAt != "2025-09-01" || b.PlantedAt != "2025-05-01" { + t.Errorf("2025 beans = %+v; want 1 removed on 2025-09-01, planted 2025-05-01", b) + } + if gl := last["Garlic"]; gl.Removed != 1 || gl.RemovedAt != "2026-07-01" || len(gl.Each) != 1 || gl.Each[0].RemovedAt != "2026-07-01" { + t.Errorf("2025 garlic = %+v; want its 2026 removal on the group and the plop", gl) + } + + y2026 := 2026 + this := groupsOf(&y2026) + if got := names(this); !reflect.DeepEqual(got, []string{"Basil", "Garlic"}) { + t.Errorf("2026 = %v, want the basil and the garlic pulled in July", got) + } + + // A typo'd year is refused, not an empty garden. + bad := 20026 + if _, err := s.DescribeGarden(ctx, owner, g.ID, &bad); !errors.Is(err, domain.ErrInvalidInput) { + t.Errorf("describe(20026) err = %v, want ErrInvalidInput", err) + } +} diff --git a/internal/service/revisions_test.go b/internal/service/revisions_test.go index c5a2b08..cb5b53c 100644 --- a/internal/service/revisions_test.go +++ b/internal/service/revisions_test.go @@ -936,7 +936,7 @@ func TestRecordOutsideTheOpenScopeFilesUnderItsOwnGarden(t *testing.T) { if _, conflicts, err := s.RevertChangeSet(ctx, owner, got.ID, domain.SourceUI); err != nil || len(conflicts) != 0 { t.Fatalf("undo from B: err=%v conflicts=%+v", err, conflicts) } - if d, err := s.DescribeGarden(ctx, owner, b.ID); err != nil || len(d.Objects) != 1 || d.Objects[0].Name != "Bed" { + if d, err := s.DescribeGarden(ctx, owner, b.ID, nil); err != nil || len(d.Objects) != 1 || d.Objects[0].Name != "Bed" { t.Errorf("after undo B is %+v (%v), want the bed's name back", d, err) } } diff --git a/web/src/lib/agent.ts b/web/src/lib/agent.ts index dd39350..508ef1d 100644 --- a/web/src/lib/agent.ts +++ b/web/src/lib/agent.ts @@ -98,6 +98,7 @@ export interface AgentStep { const TOOL_LABELS: Record = { list_gardens: 'Looking at your gardens', describe_garden: 'Reading the garden', + list_years: 'Checking past seasons', list_plantings: 'Reading a bed', create_object: 'Adding a bed', move_object: 'Moving a bed', @@ -106,6 +107,7 @@ const TOOL_LABELS: Record = { place_planting: 'Planting', fill_region: 'Filling a bed', move_planting: 'Moving a plant', + update_planting: 'Correcting a planting', remove_planting: 'Pulling a plant', remove_plantings: 'Pulling plants', clear_object: 'Clearing a bed', @@ -114,10 +116,14 @@ const TOOL_LABELS: Record = { update_plant: 'Updating your catalog', add_journal_entry: 'Writing a journal note', read_journal: 'Reading the journal', + update_journal_entry: 'Correcting a journal note', + delete_journal_entry: 'Deleting a journal note', read_history: 'Reading the history', + undo_change: 'Undoing a change', list_seed_lots: 'Checking your seed', record_seed_lot: 'Recording seed', copy_garden: 'Copying the garden', + update_garden: 'Changing the garden', } export function describeStep(step: AgentStep): string {