Smaller rough edges: 100vh on mobile, unbounded history growth, agent toolbox gaps, and four UI/doc fixes #85

Open
opened 2026-07-21 22:10:25 +00:00 by steve · 2 comments
Owner

Collected from a sweep. Each is small and independent; grouped so they don't clutter the tracker as seven issues. Split any of them out if it grows.

1. 100vh breaks the editor on mobile — small

web/src/pages/GardenEditorPage.tsx:402h-[calc(100vh-8rem)].

On iOS Safari and Chrome Android, 100vh is the largest viewport height (URL bar retracted). With the bar visible the editor is taller than the visible area, pushing the canvas bottom, the Fit button (GardenCanvas.tsx:326), and the top of the bottom-sheet rail under the browser chrome. 100dvh is the fix.

DESIGN.md claims "mobile-usable (touch drag, pinch zoom)" and the roadmap says to test on a phone "at the end of it, not later". This is also the one item still listed as open on epic #58.

Related, needs a decision rather than a fix: AppShell.tsx:82 wraps <Outlet/> in mx-auto w-full max-w-5xl, capping the editor canvas at 1024px on any monitor. Every other page wants that cap; the editor probably doesn't.

2. Revisions grow forever with no retention policy — medium

No pruning exists anywhere (grep -rn "prune|retention|DELETE FROM revisions|DELETE FROM change_sets" → nothing). Sessions get swept every 6h (main.go:24,99); revisions never do.

Two shapes:

  • A single fill_region can write up to maxFillPlops = 5000 revision rows, each carrying a full JSON row snapshot (internal/service/revisions.go).
  • Ordinary editing writes a change set per drag, including debounced keyboard nudges (GardenEditorPage.tsx:191-200, 400ms).

So the table grows with usage, not garden size, forever — in the same SQLite file serving the editor's hot /full read. Not urgent at household scale, but it's the only table with no ceiling. Wants either a retention policy (keep N days or N change sets per garden) or coalescing of consecutive same-entity moves by the same actor within a short window.

3. Agent toolbox is missing the corrective half of the service layer — medium

internal/agent/tools.go exposes 10 tools. Gaps against internal/service/*:

Missing Service method Why it bites
resize / rotate / rename / delete object UpdateObject (full ObjectPatch), DeleteObject (objects.go:162,185) move_object (tools.go:102) wires only XCM/YCM. "Make that bed 60cm wider" and "delete the old grow bag" both fail.
remove / update a single planting UpdatePlanting, DeletePlanting (plantings.go:125,197) The agent's only removal is clear_object — all-or-nothing. "Pull the basil out of the corner" requires clearing the whole bed.
read the journal ListJournal (journal.go:54) It can write entries (tools.go:61) but never read them. "What did I note about the west bed?" is unanswerable.
seed lots ListSeedLots / CreateSeedLot (seed_lots.go:77,152) find_plant reports remaining seed (tools.go:53-54) but the agent can't record a purchase or explain the number.
undo RevertChangeSet (revisions.go:295) "Undo that" — the change set is right there in the turn.

The journal read/write asymmetry and the object create-but-not-delete asymmetry are the most user-visible.

4. Journal date filtering is built but unreachable — small

from/to exist in internal/service/journal.go:44-46, in the API, and in web/src/lib/journal.ts:35-37 (JournalFilter) — but JournalPanel.tsx:45 only ever passes { objectId }. "Show me last spring" needs zero backend work.

5. Plop-level journal entries can't be created from the UI — small

journal_entries.planting_id is modelled, accepted by the API, and JournalPanel.tsx:275 renders a planting badge for such entries — but nothing creates one. Inspector.tsx has onAddNote (GardenEditorPage.tsx:344-349); PlopInspector.tsx has no equivalent. A rendered badge for a state the UI cannot produce.

6. Session expiry mid-chat reads as "the assistant is broken" — small

web/src/lib/agent.ts:155-161 maps everything that isn't a 404 to "The assistant is not available right now." A 401 should route to /login, which the rest of the app already does via ApiError.isUnauthorized (api.ts:30).

Related: toasts auto-dismiss at 4s with no manual dismiss (web/src/components/ui/toast.tsx:36-38). Error toasts are the primary failure surface for mutations, so looking away loses the only report that something failed.

7. DESIGN.md's API listing is stale — small

DESIGN.md:55-75 omits four registered routes:

  • GET/POST/DELETE /gardens/:id/share-link (api.go:112-114)
  • GET /public/gardens/:token (api.go:193)

The public share link is an unauthenticated surface — arguably the most security-relevant route in the app — and it's absent from the architecture doc. CLAUDE.md says DESIGN.md gets updated when "a new API surface" appears. README's env table is accurate; this is the only doc drift found.

Checked and clean, for the record

No N+1 queries in internal/store (everything batches with IN (placeholders(n)); ListGardensForActor resolves my_role in one UNION). /journal and /history paginate properly with limit+1 and are wired to useInfiniteQuery; /gardens and /plants have documented defensive caps. Every FK and hot filter has an index. Every catch {} in web/src has a justifying comment that checks out. The context.WithoutCancel uses are the two intentional ones.

Collected from a sweep. Each is small and independent; grouped so they don't clutter the tracker as seven issues. Split any of them out if it grows. ## 1. `100vh` breaks the editor on mobile — small `web/src/pages/GardenEditorPage.tsx:402` — `h-[calc(100vh-8rem)]`. On iOS Safari and Chrome Android, `100vh` is the *largest* viewport height (URL bar retracted). With the bar visible the editor is taller than the visible area, pushing the canvas bottom, the Fit button (`GardenCanvas.tsx:326`), and the top of the bottom-sheet rail under the browser chrome. `100dvh` is the fix. DESIGN.md claims "mobile-usable (touch drag, pinch zoom)" and the roadmap says to test on a phone "at the end of it, not later". This is also the one item still listed as open on epic #58. Related, needs a decision rather than a fix: `AppShell.tsx:82` wraps `<Outlet/>` in `mx-auto w-full max-w-5xl`, capping the editor canvas at 1024px on any monitor. Every other page wants that cap; the editor probably doesn't. ## 2. Revisions grow forever with no retention policy — medium No pruning exists anywhere (`grep -rn "prune|retention|DELETE FROM revisions|DELETE FROM change_sets"` → nothing). Sessions get swept every 6h (`main.go:24,99`); revisions never do. Two shapes: - A single `fill_region` can write up to `maxFillPlops` = 5000 revision rows, each carrying a full JSON row snapshot (`internal/service/revisions.go`). - Ordinary editing writes a change set per drag, including debounced keyboard nudges (`GardenEditorPage.tsx:191-200`, 400ms). So the table grows with *usage*, not garden size, forever — in the same SQLite file serving the editor's hot `/full` read. Not urgent at household scale, but it's the only table with no ceiling. Wants either a retention policy (keep N days or N change sets per garden) or coalescing of consecutive same-entity moves by the same actor within a short window. ## 3. Agent toolbox is missing the corrective half of the service layer — medium `internal/agent/tools.go` exposes 10 tools. Gaps against `internal/service/*`: | Missing | Service method | Why it bites | |---|---|---| | resize / rotate / rename / delete object | `UpdateObject` (full `ObjectPatch`), `DeleteObject` (`objects.go:162,185`) | `move_object` (`tools.go:102`) wires only `XCM`/`YCM`. "Make that bed 60cm wider" and "delete the old grow bag" both fail. | | remove / update a single planting | `UpdatePlanting`, `DeletePlanting` (`plantings.go:125,197`) | The agent's only removal is `clear_object` — all-or-nothing. "Pull the basil out of the corner" requires clearing the whole bed. | | read the journal | `ListJournal` (`journal.go:54`) | It can *write* entries (`tools.go:61`) but never read them. "What did I note about the west bed?" is unanswerable. | | seed lots | `ListSeedLots` / `CreateSeedLot` (`seed_lots.go:77,152`) | `find_plant` *reports* remaining seed (`tools.go:53-54`) but the agent can't record a purchase or explain the number. | | undo | `RevertChangeSet` (`revisions.go:295`) | "Undo that" — the change set is right there in the turn. | The journal read/write asymmetry and the object create-but-not-delete asymmetry are the most user-visible. ## 4. Journal date filtering is built but unreachable — small `from`/`to` exist in `internal/service/journal.go:44-46`, in the API, and in `web/src/lib/journal.ts:35-37` (`JournalFilter`) — but `JournalPanel.tsx:45` only ever passes `{ objectId }`. "Show me last spring" needs zero backend work. ## 5. Plop-level journal entries can't be created from the UI — small `journal_entries.planting_id` is modelled, accepted by the API, and `JournalPanel.tsx:275` renders a `planting` badge for such entries — but nothing creates one. `Inspector.tsx` has `onAddNote` (`GardenEditorPage.tsx:344-349`); `PlopInspector.tsx` has no equivalent. A rendered badge for a state the UI cannot produce. ## 6. Session expiry mid-chat reads as "the assistant is broken" — small `web/src/lib/agent.ts:155-161` maps everything that isn't a 404 to *"The assistant is not available right now."* A 401 should route to `/login`, which the rest of the app already does via `ApiError.isUnauthorized` (`api.ts:30`). Related: toasts auto-dismiss at 4s with **no manual dismiss** (`web/src/components/ui/toast.tsx:36-38`). Error toasts are the primary failure surface for mutations, so looking away loses the only report that something failed. ## 7. DESIGN.md's API listing is stale — small `DESIGN.md:55-75` omits four registered routes: - `GET`/`POST`/`DELETE /gardens/:id/share-link` (`api.go:112-114`) - `GET /public/gardens/:token` (`api.go:193`) The public share link is an **unauthenticated** surface — arguably the most security-relevant route in the app — and it's absent from the architecture doc. CLAUDE.md says DESIGN.md gets updated when "a new API surface" appears. README's env table is accurate; this is the only doc drift found. ## Checked and clean, for the record No N+1 queries in `internal/store` (everything batches with `IN (placeholders(n))`; `ListGardensForActor` resolves `my_role` in one UNION). `/journal` and `/history` paginate properly with limit+1 and are wired to `useInfiniteQuery`; `/gardens` and `/plants` have documented defensive caps. Every FK and hot filter has an index. Every `catch {}` in `web/src` has a justifying comment that checks out. The `context.WithoutCancel` uses are the two intentional ones.
steve closed this issue 2026-07-23 01:42:03 +00:00
Author
Owner

Swept this. Status of the seven:

# Item Status
1 100vh100dvh on the editor already done (editor uses 100dvh; PR #121 tightened it further + reclaimed the mobile header band)
2 Revisions grow forever still open — see below
3 Agent toolbox missing the corrective half PR #122read_journal, update_object, delete_object, remove_planting, list_seed_lots, record_seed_lot
4 Journal date filtering unreachable already done (JournalPanel has From/To inputs)
5 Plop-level journal entries not creatable from the UI PR #123PlopInspector "add note", plop-scoped journal
6 Session expiry reads as "assistant broken" + toast has no manual dismiss already done (agent.ts routes 401 → /login; error toasts persist with a ✕)
7 DESIGN.md API listing stale already done (share-link, /public, scan/from-packet, settings all listed)

So only item 2 remains. I did not ship a retention policy blind, because it's the one item with a real design fork and it sits on the undo substrate CLAUDE.md flags as delicate. Two things to decide:

The gotcha: change_sets.reverts_id is a self-FK with no ON DELETE clause → SQLite NO ACTION. Pruning an old change set that a kept revert points at would violate the FK. So a naive "delete oldest" needs either a table-rebuild migration to make it ON DELETE SET NULL, or pruning in dependency order.

My recommendation (a dedicated PR when you want it): a per-garden retention cap — keep the most recent N change sets per garden (say 500–1000, generous enough that no real household hits it), pruned in the existing 6h sweep (main.go), deleting whole change sets so revisions cascade. Pair it with the reverts_id → ON DELETE SET NULL migration so a surviving revert whose target aged out just loses its back-pointer. Coalescing consecutive same-entity nudges is the alternative but it touches the hot write path, which I'd avoid.

The knob is N (how much undo history to keep) — that's your call, which is why I'm leaving it for you rather than picking a number. Everything else here is merged.

Swept this. Status of the seven: | # | Item | Status | |---|---|---| | 1 | `100vh` → `100dvh` on the editor | ✅ already done (editor uses `100dvh`; PR #121 tightened it further + reclaimed the mobile header band) | | 2 | Revisions grow forever | ⏳ **still open — see below** | | 3 | Agent toolbox missing the corrective half | ✅ **PR #122** — `read_journal`, `update_object`, `delete_object`, `remove_planting`, `list_seed_lots`, `record_seed_lot` | | 4 | Journal date filtering unreachable | ✅ already done (`JournalPanel` has From/To inputs) | | 5 | Plop-level journal entries not creatable from the UI | ✅ **PR #123** — `PlopInspector` "add note", plop-scoped journal | | 6 | Session expiry reads as "assistant broken" + toast has no manual dismiss | ✅ already done (`agent.ts` routes 401 → `/login`; error toasts persist with a ✕) | | 7 | DESIGN.md API listing stale | ✅ already done (share-link, `/public`, scan/from-packet, settings all listed) | So only **item 2** remains. I did **not** ship a retention policy blind, because it's the one item with a real design fork *and* it sits on the undo substrate CLAUDE.md flags as delicate. Two things to decide: **The gotcha:** `change_sets.reverts_id` is a self-FK with no `ON DELETE` clause → SQLite `NO ACTION`. Pruning an old change set that a *kept* revert points at would violate the FK. So a naive "delete oldest" needs either a table-rebuild migration to make it `ON DELETE SET NULL`, or pruning in dependency order. **My recommendation** (a dedicated PR when you want it): a **per-garden retention cap** — keep the most recent *N* change sets per garden (say 500–1000, generous enough that no real household hits it), pruned in the existing 6h sweep (`main.go`), deleting whole change sets so `revisions` cascade. Pair it with the `reverts_id → ON DELETE SET NULL` migration so a surviving revert whose target aged out just loses its back-pointer. Coalescing consecutive same-entity nudges is the alternative but it touches the hot write path, which I'd avoid. The knob is *N* (how much undo history to keep) — that's your call, which is why I'm leaving it for you rather than picking a number. Everything else here is merged.
steve reopened this issue 2026-07-23 01:44:12 +00:00
Author
Owner

Reopened — #123's "Closes #85" auto-closed this, but per the status above only item 2 (revision retention) is actually done-pending-your-call. Leaving it open for that.

Reopened — #123's "Closes #85" auto-closed this, but per the status above only **item 2 (revision retention)** is actually done-pending-your-call. Leaving it open for that.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: steve/pansy#85