Two mobile wasted-space complaints, both in the editor/assistant context.
1. The editor's top banner was pure chrome
On a phone the global top bar (brand + account avatar) sat above the editor's own garden-name strip — a whole banner over a full-screen canvas, saying nothing the strip below it didn't. That's the same reasoning that already hides the bottom nav in the editor, so:
Hide the global header on mobile in the editor (AppShell), desktop keeps it.
Fold a leaf/back affordance into the editor's mobile strip so there's still a way back to the gardens list (the global brand link was the only one).
Reclaim the height: the editor's band shrinks 100dvh-8rem → 100dvh-4rem on mobile (no header to clear); desktop stays 8rem. The canvas gets the ~48px back, plus the ~32px of dead space the old 8rem left at the bottom.
2. The assistant felt cramped
The assistant / journal / history render inside the rail peek, capped at max-h-[50vh]. After the tab bar, panel header and message input, the assistant got ~200px of actual conversation. That cap is right for the inspector (you read it alongside the canvas) but wrong for a mode where reading and typing are the task.
Panel modes now take a taller slice (max-h-[78vh]) via a tall prop on EditorRail; the inspector keeps the 50vh peek so the canvas it describes stays visible.
Chat message spacing loosened gap-2 → gap-3.
Notes
100vh → 100dvh (issue #85 item 1) was already in; this builds on it.
No behaviour change on desktop.
tsc, vitest (108), and vite build all green.
Part of the #96 mobile-first pass; direct follow-up to Steve's "so much wasted space / assistant so compact" feedback.
Two mobile wasted-space complaints, both in the editor/assistant context.
## 1. The editor's top banner was pure chrome
On a phone the global top bar (brand + account avatar) sat *above* the editor's own garden-name strip — a whole banner over a full-screen canvas, saying nothing the strip below it didn't. That's the same reasoning that already hides the bottom nav in the editor, so:
- **Hide the global header on mobile in the editor** (`AppShell`), desktop keeps it.
- **Fold a leaf/back affordance into the editor's mobile strip** so there's still a way back to the gardens list (the global brand link was the only one).
- **Reclaim the height**: the editor's band shrinks `100dvh-8rem → 100dvh-4rem` on mobile (no header to clear); desktop stays `8rem`. The canvas gets the ~48px back, plus the ~32px of dead space the old `8rem` left at the bottom.
## 2. The assistant felt cramped
The assistant / journal / history render inside the rail *peek*, capped at `max-h-[50vh]`. After the tab bar, panel header and message input, the assistant got ~200px of actual conversation. That cap is right for the **inspector** (you read it alongside the canvas) but wrong for a mode where reading and typing *are* the task.
- **Panel modes now take a taller slice** (`max-h-[78vh]`) via a `tall` prop on `EditorRail`; the inspector keeps the 50vh peek so the canvas it describes stays visible.
- Chat message spacing loosened `gap-2 → gap-3`.
## Notes
- `100vh → 100dvh` (issue #85 item 1) was already in; this builds on it.
- No behaviour change on desktop.
- `tsc`, `vitest` (108), and `vite build` all green.
Part of the #96 mobile-first pass; direct follow-up to Steve's "so much wasted space / assistant so compact" feedback.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ
Two mobile complaints, both in the editor/assistant context:
- The global top bar (brand + account) sat above the editor's own
garden-name strip — a whole banner of pure chrome over a full-screen
canvas. Hide it on mobile in the editor (the same rationale that hides
the bottom nav there) and fold a leaf/back affordance into the garden
strip so there's still a way out. Desktop keeps the header. The editor's
height band shrinks 8rem → 4rem on mobile to hand that space to the
canvas; desktop stays 8rem since the header is still there.
- The assistant (and journal/history) rendered inside the rail peek
capped at max-h-[50vh]; after the tab bar, header and input, messages
got ~200px. That cap is right for the inspector (read alongside the
canvas) but not for a mode where reading/typing is the task. Panel
modes now take a taller slice (78vh) via a `tall` prop; the inspector
keeps the 50vh peek. Chat message spacing loosened gap-2 → gap-3.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ
Live status board. Findings are posted in each model's own comment. Advisory only — does not block merge.
<!-- gadfly-status-board -->
## 🪰 Gadfly — live review status
5/5 reviewers finished · updated 2026-07-23 01:25:44Z
#### `claude-code/sonnet` · claude-code — ✅ done
- ✅ **security** — No material issues found
- ✅ **correctness** — Minor issues
- ✅ **maintainability** — Minor issues
- ✅ **performance** — No material issues found
- ✅ **error-handling** — No material issues found
#### `glm-5.2:cloud` · ollama-cloud — ✅ done
- ✅ **security** — Minor issues
- ✅ **correctness** — Minor issues
- ✅ **maintainability** — Minor issues
- ✅ **performance** — No material issues found
- ✅ **error-handling** — No material issues found
#### `kimi-k2.6:cloud` · ollama-cloud — ✅ done
- ✅ **security** — No material issues found
- ✅ **correctness** — No material issues found
- ✅ **maintainability** — No material issues found
- ✅ **performance** — No material issues found
- ✅ **error-handling** — No material issues found
#### `opencode/glm-5.2:cloud` · opencode — ✅ done
- ✅ **security** — No material issues found
- ✅ **correctness** — Minor issues
- ✅ **maintainability** — Minor issues
- ✅ **performance** — No material issues found
- ✅ **error-handling** — Minor issues
#### `opencode/kimi-k2.6:cloud` · opencode — ✅ done
- ✅ **security** — No material issues found
- ⚠️ **correctness** — could not complete
- ✅ **maintainability** — No material issues found
- ✅ **performance** — No material issues found
- ✅ **error-handling** — No material issues found
<sub>Live status board. Findings are posted in each model's own comment. Advisory only — does not block merge.</sub>
🪰Gadfly consensus review — 5 inline findings on changed lines. See the consensus comment for the full ranked summary.
Advisory only — does not block merge.
<!-- gadfly-inline-review -->
🪰 **Gadfly consensus review** — 5 inline findings on changed lines. See the consensus comment for the full ranked summary.
<sub>Advisory only — does not block merge.</sub>
🟠Hiding the global header on mobile in the editor removes the only sign-out affordance (AccountMenu), weakening session control on shared devices
correctness, error-handling, maintainability, security · flagged by 2 models
Loss of the only sign-out affordance on mobile in the editor (web/src/components/layout/AppShell.tsx:51,57-61). The global header — which contains AccountMenu, the app's only "Sign out" control (AppShell.tsx:92/130-197) — is now hidden md:block whenever inEditor is true. The editor's replacement mobile strip (GardenEditorPage.tsx:608-630) adds only a 🌱Link to="/gardens" and garden/season/share controls — no account or sign-out control. showBottomNav is also false on th…
🪰 Gadfly · advisory
🟠 **Hiding the global header on mobile in the editor removes the only sign-out affordance (AccountMenu), weakening session control on shared devices**
_correctness, error-handling, maintainability, security · flagged by 2 models_
- **Loss of the only sign-out affordance on mobile in the editor** (`web/src/components/layout/AppShell.tsx:51,57-61`). The global header — which contains `AccountMenu`, the app's only "Sign out" control (`AppShell.tsx:92`/`130-197`) — is now `hidden md:block` whenever `inEditor` is true. The editor's replacement mobile strip (`GardenEditorPage.tsx:608-630`) adds only a 🌱 `Link to="/gardens"` and garden/season/share controls — no account or sign-out control. `showBottomNav` is also `false` on th…
<sub>🪰 Gadfly · advisory</sub>
🟠Mobile peek height cap uses static vh instead of dvh, risking overflow past the visible viewport (hiding the always-visible mode bar) when panel content is tall — worsened by raising the cap from 50vh to 78vh
correctness, maintainability · flagged by 2 models
web/src/editor/EditorRail.tsx:65 — The mobile peek's height cap uses vh (max-h-[50vh] / new max-h-[78vh]) rather than dvh, while the container it's nested inside (GardenEditorPage.tsx:553, h-[calc(100dvh-4rem)] ... md:h-[calc(100dvh-8rem)]) was deliberately switched to dvh for the exact reason documented in the adjacent comment at GardenEditorPage.tsx:544-546 (#85): mobile Safari/Chrome's 100vh is the largest viewport (address bar hidden), so with the bar showing, any…
🪰 Gadfly · advisory
🟠 **Mobile peek height cap uses static vh instead of dvh, risking overflow past the visible viewport (hiding the always-visible mode bar) when panel content is tall — worsened by raising the cap from 50vh to 78vh**
_correctness, maintainability · flagged by 2 models_
- **`web/src/editor/EditorRail.tsx:65`** — The mobile peek's height cap uses `vh` (`max-h-[50vh]` / new `max-h-[78vh]`) rather than `dvh`, while the container it's nested inside (`GardenEditorPage.tsx:553`, `h-[calc(100dvh-4rem)] ... md:h-[calc(100dvh-8rem)]`) was deliberately switched to `dvh` for the exact reason documented in the adjacent comment at `GardenEditorPage.tsx:544-546` (#85): mobile Safari/Chrome's `100vh` is the *largest* viewport (address bar hidden), so with the bar showing, any…
<sub>🪰 Gadfly · advisory</sub>
🟡Mobile height subtracts 4rem but only 3rem of chrome (main py-6) exists with header hidden; leaves ~16px dead space and comment math is wrong
correctness · flagged by 2 models
web/src/pages/GardenEditorPage.tsx:553 — Mobile height is h-[calc(100dvh-4rem)], but with the global header hidden on mobile (AppShell.tsx header is hidden md:block), the only chrome outside the editor's flex column is <main>'s px-4 py-6 — i.e. 3rem of vertical padding (1.5rem top + 1.5rem bottom), not 4rem. Subtracting 4rem leaves ~16px of dead space the viewport would otherwise allow. This contradicts the comment at GardenEditorPage.tsx:548-551 ("only <main>'s padding is…
🪰 Gadfly · advisory
🟡 **Mobile height subtracts 4rem but only 3rem of chrome (main py-6) exists with header hidden; leaves ~16px dead space and comment math is wrong**
_correctness · flagged by 2 models_
- `web/src/pages/GardenEditorPage.tsx:553` — Mobile height is `h-[calc(100dvh-4rem)]`, but with the global header hidden on mobile (`AppShell.tsx` header is `hidden md:block`), the only chrome outside the editor's flex column is `<main>`'s `px-4 py-6` — i.e. **3rem** of vertical padding (1.5rem top + 1.5rem bottom), not 4rem. Subtracting 4rem leaves ~16px of dead space the viewport would otherwise allow. This contradicts the comment at `GardenEditorPage.tsx:548-551` ("only `<main>`'s padding is…
<sub>🪰 Gadfly · advisory</sub>
🟡Back affordance is a bare emoji with only aria-label; sr-only text would be more robust
maintainability · flagged by 1 model
web/src/components/layout/AppShell.tsx:51 — hideHeaderOnMobile is a single-use alias for inEditor, with a five-line comment (lines 46–50) whose rationale substantially overlaps the file-top docblock (lines 24–30: "the editor is a full-screen context… the brand link is the way back to the gardens list"). Not a defect — the alias does aid readability of the cn(...) call — but the comment is heavier than the one-line decision needs. Trivial. - web/src/pages/GardenEditorPage.tsx:609 — Co…
🪰 Gadfly · advisory
🟡 **Back affordance is a bare emoji with only aria-label; sr-only text would be more robust**
_maintainability · flagged by 1 model_
- `web/src/components/layout/AppShell.tsx:51` — `hideHeaderOnMobile` is a single-use alias for `inEditor`, with a five-line comment (lines 46–50) whose rationale substantially overlaps the file-top docblock (lines 24–30: "the editor is a full-screen context… the brand link is the way back to the gardens list"). Not a defect — the alias does aid readability of the `cn(...)` call — but the comment is heavier than the one-line decision needs. Trivial. - `web/src/pages/GardenEditorPage.tsx:609` — Co…
<sub>🪰 Gadfly · advisory</sub>
⚪Duplicated 'tall' rationale comment already expressed in EditorRail prop doc
maintainability · flagged by 1 model
web/src/pages/GardenEditorPage.tsx:690 (duplicated rationale) — Verified, minor maintainability nit. The same "inspector reads alongside the canvas / panel modes want room" explanation is written out twice: as the tall prop's JSDoc in EditorRail.tsx:24-27 (and the inline prop doc at 50-51) and again as an inline comment at the call site in GardenEditorPage.tsx:690-694. The call-site comment adds nothing the prop doc doesn't already say; it's copy-paste that will drift if one is upd…
🪰 Gadfly · advisory
⚪ **Duplicated 'tall' rationale comment already expressed in EditorRail prop doc**
_maintainability · flagged by 1 model_
- **`web/src/pages/GardenEditorPage.tsx:690` (duplicated rationale)** — Verified, minor maintainability nit. The same "inspector reads alongside the canvas / panel modes want room" explanation is written out twice: as the `tall` prop's JSDoc in `EditorRail.tsx:24-27` (and the inline prop doc at 50-51) and again as an inline comment at the call site in `GardenEditorPage.tsx:690-694`. The call-site comment adds nothing the prop doc doesn't already say; it's copy-paste that will drift if one is upd…
<sub>🪰 Gadfly · advisory</sub>
Mobile peek height cap uses static vh instead of dvh, risking overflow past the visible viewport (hiding the always-visible mode bar) when panel content is tall — worsened by raising the cap from 50vh to 78vh
web/src/editor/EditorRail.tsx:65
2/5
correctness, maintainability
🟡
Stale class docstring: "the brand link is the way back" is now false on mobile editor after header hide
web/src/components/layout/AppShell.tsx:28
2/5
maintainability
🟡
Comment says editor and public-garden view both use a 100dvh-8rem canvas, but the editor's mobile band changed to 100dvh-4rem
web/src/components/layout/AppShell.tsx:37
2/5
maintainability
🟡
Mobile height subtracts 4rem but only 3rem of chrome (main py-6) exists with header hidden; leaves ~16px dead space and comment math is wrong
web/src/pages/GardenEditorPage.tsx:553
2/5
correctness
🟡
Comment says the mobile rail peek is capped at ≤50vh, but the new tall prop raises it to 78vh for all non-inspector tabs
web/src/pages/GardenEditorPage.tsx:683
2/5
maintainability
2 single-model findings (lower confidence)
Finding
Where
Model
Lens
🟡
Back affordance is a bare emoji with only aria-label; sr-only text would be more robust
web/src/pages/GardenEditorPage.tsx:609
glm-5.2:cloud
maintainability
⚪
Duplicated 'tall' rationale comment already expressed in EditorRail prop doc
web/src/pages/GardenEditorPage.tsx:690
opencode/glm-5.2:cloud
maintainability
Per-model detailclaude-code/sonnet (claude-code) — Minor issues
No new data flow, authn/authz logic, injection surface, or secret handling in this diff — it's Tailwind class/layout changes (hidden md:block on the header, a tall height-cap prop, a gap-2→gap-3 spacing tweak) plus one new <Link to="/gardens">. Verified in AppShell.tsx and GardenEditorPage.tsx that hiding the header on mobile doesn't remove any access-control check — canEdit/isOwner gating for edit actions lives elsewhere in the page body, untouched by this diff, and the account/sign-out control simply becomes desktop-only rather than being bypassed. EditorRail/ChatPanel changes are purely visual (height cap, gap).
VERDICT: No material issues found
No findings within the security lens — this PR touches only presentational Tailwind classes, a tall height-cap prop, and a static in-app navigation Link; no new input handling, no auth/authz logic, no external calls.
🎯 Correctness — Minor issues
VERDICT: Minor issues
web/src/editor/EditorRail.tsx:65 — The mobile peek's height cap uses vh (max-h-[50vh] / new max-h-[78vh]) rather than dvh, while the container it's nested inside (GardenEditorPage.tsx:553, h-[calc(100dvh-4rem)] ... md:h-[calc(100dvh-8rem)]) was deliberately switched to dvh for the exact reason documented in the adjacent comment at GardenEditorPage.tsx:544-546 (#85): mobile Safari/Chrome's 100vh is the largest viewport (address bar hidden), so with the bar showing, anything sized off vh overflows the actually-visible area.
Confirmed the flex structure directly: the outer editor container (GardenEditorPage.tsx:553) is flex-col with a fixed dvh-based height and no overflow-hidden; the canvas column (GardenEditorPage.tsx:603, flex-1 min-h-0) can shrink to accommodate siblings, but both the rail (EditorRail.tsx:64, shrink-0) and the always-visible mobile mode bar (GardenEditorPage.tsx:715, shrink-0) refuse to shrink. Because the rail's cap is computed against the largevh instead of the container's dvh, on a device where the address bar is showing, the rail can size itself larger than the space actually left after the fixed-height container and the shrink-0 mode bar are accounted for, pushing content (including the mode bar) past the bottom of the visible screen.
This mismatch already existed at max-h-[50vh], but raising the cap to 78vh (per EditorRail.tsx:44-51, GardenEditorPage.tsx:690-694 wiring tall={railTab !== 'inspector'} for journal/history/chat) increases the realistic overshoot, since panel content (a chat thread, journal entries, history list) is far more likely to actually reach a 78% cap than a 50% one.
Suggested fix: express the cap in dvh (e.g. max-h-[78dvh] / max-h-[50dvh]) to match the container it's nested in, consistent with the fix already applied one level up.
🧹 Code cleanliness & maintainability — Minor issues
VERDICT: Minor issues
web/src/components/layout/AppShell.tsx:29 — The docstring says "the app bottom bar hides there (the brand link is the way back to the gardens list)," but the diff adds hideHeaderOnMobile = inEditor (line 51) which hides the entire header — including the brand link — on mobile in the editor (line 57-61: hidden md:block). On mobile the brand link is no longer visible in the editor; the new 🌱 link in GardenEditorPage.tsx:609-615 is now the way back. The comment describes desktop-only behavior as if universal.
web/src/components/layout/AppShell.tsx:37 — The comment "the garden editor and the public shared-garden view both render a 100dvh-8rem canvas" is now false on mobile: GardenEditorPage.tsx:553 changed the editor's mobile band to h-[calc(100dvh-4rem)], keeping 8rem only at md:. The two canvas routes no longer match on mobile, but the comment still claims they do.
web/src/pages/GardenEditorPage.tsx:683 — The comment directly above the EditorRail usage still says the mobile peek is "≤50vh," but line 694 in the same diff introduces tall={railTab !== 'inspector'}, which raises the cap to 78vh (confirmed in EditorRail.tsx:65, tall ? 'max-h-[78vh]' : 'max-h-[50vh]') for every rail tab except the inspector — the common case. Notably, EditorRail.tsx's own docstring was correctly updated in this same diff to say "a capped-height panel," making this leftover GardenEditorPage.tsx comment the one stale outlier.
⚡ Performance — No material issues found
VERDICT: No material issues found
This diff is Tailwind class changes (a hidden md:block toggle, max-h-[50vh] → conditional max-h-[78vh], gap-2 → gap-3), a tall boolean prop threaded through EditorRail, and a new <Link> in the mobile strip. I read AppShell.tsx, EditorRail.tsx, and the surrounding GardenEditorPage.tsx context to confirm there's no hidden cost:
hideHeaderOnMobile (AppShell.tsx:51,57-61) uses CSS hidden md:block, not conditional mounting — the header's subtree (including AccountMenu's useLogout/useMe hooks) mounts exactly as it did before this change; visibility is display-only. No new render/mount cost.
tall (EditorRail.tsx:44-67) is a plain boolean prop picking between two static Tailwind classes via cn() — no computation, no re-render amplification beyond the normal prop-driven re-render already inherent to the component.
The Link to="/gardens" addition (GardenEditorPage.tsx:609-615) is a single static element, not inside any loop or list.
gap-2 → gap-3 in ChatPanel.tsx:146 is pure CSS.
No loops, no data-fetching changes, no unbounded growth, nothing on a hot path. Nothing in my lane here.
🧯 Error handling & edge cases — No material issues found
VERDICT: No material issues found
This PR is CSS/layout-only (header visibility via hidden md:block, a max-h cap swap, a gap-2→gap-3 tweak) plus one added navigation <Link>. None of it introduces new async operations, error paths, resource acquisition, or state that needs cleanup — nothing for this lens to bite into.
Specifically checked and found no issues:
AppShell.tsx:57-62 — hideHeaderOnMobile only hides the header via CSS (hidden md:block); the component doesn't unmount, so AccountMenu's effects/backdrop logic (AppShell.tsx:138) keep working unaffected. Confirmed via router.tsx that gardens/$gardenId always requires auth (requireAuth at router.tsx:132), so there's no edge case of an unauthenticated user losing the header's "Sign in" link on this route.
EditorRail.tsx:44,51,65 — new tall prop defaults to false, preserving prior behavior; confirmed via grep it has a single call site (GardenEditorPage.tsx:694), so no other caller is affected by the default change. Pre-existing tabs.find(...) ?? tabs[0] / if (!active) return null guard is untouched.
GardenEditorPage.tsx:608-615 — new back <Link to="/gardens"> renders only after the existing loading/gone-state guards higher in the component (verified gardenGone/loading handling precedes this JSX), so garden.name being dereferenced nearby is not a new null-access risk.
GardenEditorPage.tsx:553 — height calc changes only the subtracted constant (4rem vs 8rem); this reduces (not increases) the risk of a negative/degenerate height versus the prior 8rem on mobile.
ChatPanel.tsx:146 — spacing-only change, no logic touched.
Confirmed. The global header containing AccountMenu (the only sign-out control, AppShell.tsx:92/130-197) is now hidden md:block whenever hideHeaderOnMobile (inEditor, AppShell.tsx:51,57-61) is true; showBottomNav is false on the editor route (canvasRoute true, AppShell.tsx:45,121), so the bottom nav isn't a fallback either. The editor's mobile strip (GardenEditorPage.tsx:608-630) adds only the 🌱Link to="/gardens", garden name, view-only badge, season picker, and share button — no account/sign-out control. The only sign-out affordance lives in AccountMenu in the hidden header. Finding survives.
VERDICT: Minor issues
Loss of the only sign-out affordance on mobile in the editor (web/src/components/layout/AppShell.tsx:51,57-61). The global header — which contains AccountMenu, the app's only "Sign out" control (AppShell.tsx:92/130-197) — is now hidden md:block whenever inEditor is true. The editor's replacement mobile strip (GardenEditorPage.tsx:608-630) adds only a 🌱Link to="/gardens" and garden/season/share controls — no account or sign-out control. showBottomNav is also false on the editor route (canvasRoute, AppShell.tsx:45,121), so the bottom nav (which would otherwise carry account access) isn't there either. Net effect: an authenticated user on a phone inside the garden editor has no visible UI to end their session. On a shared/loaned device this weakens session control — the user can't sign out before handing the phone back and must know to navigate away to find the control. Concrete impact: degraded ability to terminate an authenticated session; not an authn/authz bypass. Suggested fix: surface a compact account/sign-out control in the editor's mobile strip (or make AccountMenu available via the editor), rather than only in the hidden header.
🎯 Correctness — Minor issues
Verdict: Minor issues
web/src/pages/GardenEditorPage.tsx:553 — Mobile height is h-[calc(100dvh-4rem)], but with the global header hidden on mobile (AppShell.tsx header is hidden md:block), the only chrome outside the editor's flex column is <main>'s px-4 py-6 — i.e. 3rem of vertical padding (1.5rem top + 1.5rem bottom), not 4rem. Subtracting 4rem leaves ~16px of dead space the viewport would otherwise allow. This contradicts the comment at GardenEditorPage.tsx:548-551 ("only <main>'s padding is above us — 4rem"). Impact is small: the canvas still renders correctly and reclaims most of the header room; it just leaves 16px on the table and the numerical justification is wrong. Fix: either use h-[calc(100dvh-3rem)] on mobile, or correct the comment to state the 1rem is an intentional buffer.
web/src/editor/EditorRail.tsx:65 — The tall peek cap uses max-h-[78vh] while the editor column is sized in dvh (100dvh-4rem). On mobile browsers with the URL bar visible 100dvh < 100vh, so a content-filled 78vh rail plus the ~5rem of strip + mode bar can approach or exceed the dvh-based column height. This is the same vh-vs-dvh class of issue (#85) the editor already corrected for its own height. Not certain to overflow in practice (depends on rail content and exact mode-bar height), but the unit inconsistency is real. Fix: max-h-[78dvh] to match the column's unit basis.
🧹 Code cleanliness & maintainability — Minor issues
Verdict: Minor issues found
The change is clean and works as intended. A few maintainability nits survive verification:
web/src/components/layout/AppShell.tsx:51 — hideHeaderOnMobile is a single-use alias for inEditor, with a five-line comment (lines 46–50) whose rationale substantially overlaps the file-top docblock (lines 24–30: "the editor is a full-screen context… the brand link is the way back to the gardens list"). Not a defect — the alias does aid readability of the cn(...) call — but the comment is heavier than the one-line decision needs. Trivial.
web/src/pages/GardenEditorPage.tsx:609 — Confirmed: the back affordance is a bare 🌱 emoji in a Link with aria-label="All gardens" and no visible/sr-only text, while the header brand it replaces (AppShell.tsx:67) renders 🌱 pansy with the wordmark. With the header hidden on mobile here, this leaf is the only brand/back affordance. Reliance on an emoji glyph plus aria-label alone is fragile across screen readers / platform glyph rendering; adding <span className="sr-only">All gardens</span> would be more robust. Minor.
web/src/editor/EditorRail.tsx:65 — Confirmed: the tall ? 'max-h-[78vh]' : 'max-h-[50vh]' ternary uses magic viewport percentages, and the "reading and typing are the task" rationale appears verbatim-ish in both the EditorRail docblock (line 26) and the call site comment (GardenEditorPage.tsx:692). Two copies of the same "why" will drift. Trivial.
⚡ Performance — No material issues found
No material issues found.
Through the performance lens, the change is purely CSS class / prop plumbing — no new allocations, queries, loops, or blocking work on any path. I verified:
EditorRail.tsx:64-66 — the tall toggle just swaps one Tailwind max-h class string via cn; no runtime cost beyond the class lookup. tall={railTab !== 'inspector'} (GardenEditorPage.tsx:694) is a cheap boolean prop recomputed on render — negligible.
AppShell.tsx — the hidden md:block header is a pure display toggle; the header subtree still mounts, but that was already the case (it was always rendered), so no render-cost regression.
GardenEditorPage.tsx — the Link and 🌱 glyph are static, render once; no list work, no effect added.
ChatPanel.tsx:146 — gap-2 → gap-3 is a layout spacing change, not a loop/allocation change.
No N+1, no unbounded growth, no hot-path regression introduced.
🧯 Error handling & edge cases — No material issues found
Through the error-handling & edge-cases lens, I verified the unhappy paths this diff touches:
EditorRail empty/degenerate input (EditorRail.tsx:53-54): active = tabs.find(...) ?? tabs[0]; if tabs is empty, tabs[0] is undefined and the if (!active) return null guard handles it — no panic. Verified by reading the file.
tall={railTab !== 'inspector'} (GardenEditorPage.tsx:694): EditorRail is only rendered inside {railTab && (...)} (line 685), so railTab is guaranteed truthy here. The tall prop has a = false default and is always a boolean. No null/undefined edge case. Verified.
hideHeaderOnMobile = inEditor (AppShell.tsx:51): boolean, !!-coerced from matchRoute. The cn(..., hideHeaderOnMobile && 'hidden md:block') safely yields false (no class) when off. No bad-input path.
Link to="/gardens" (GardenEditorPage.tsx:609): /gardens is an existing route (used in AppShell.tsx:66), so no resolution failure; the link is rendered unconditionally inside the mobile strip, which is fine for both owner and view-only viewers (verified the strip has no auth-gating around the link).
Mobile editor now has no account/sign-out surface (header hidden, bottom nav already suppressed for canvas routes at AppShell.tsx:45): this is a UX consideration, not an unhandled error — the 🌱 link returns to /gardens where the header reappears. Not an error-handling finding.
No swallowed errors, missing defer/cleanup, panics on bad input, or unhandled nil/empty/boundary conditions are introduced by this change.
Verdict: No material issues found.
kimi-k2.6:cloud (ollama-cloud) — No material issues found
Verdict: No material issues found — 5 reviewers: security, correctness, maintainability, performance, error-handling
🔒 Security — No material issues found
No material issues found
This is a purely presentational layout change. From a security lens:
No auth bypass: The editor route still enforces requireAuth in router.tsx:132; the header is only visually hidden with CSS (hidden md:block), not removed from an authenticated shell.
No new injection surface: The only rendered dynamic data, garden.name, was already present before this diff and is safely interpolated by React inside a <h1>. The new <Link to="/gardens"> has a hardcoded path and aria-label.
No trust boundary change: The tall prop and max-h-[78vh] cap are driven by a hardcoded local store value (railTab), not by user input or URL parameters.
No credential leakage: Hiding the global header on mobile removes the account menu from view, but this is a responsive design choice, not a data-leak vector — the DOM is hidden with display: none, not rendered in the public view (the public garden route /g/$token explicitly keeps its header).
🎯 Correctness — No material issues found
No material issues found
I verified the logic carefully:
AppShell.tsx: hideHeaderOnMobile is scoped to inEditor only, intentionally preserving the global header for the public garden view which has no back affordance of its own. The hidden md:block breakpoint correctly hides the header on mobile while keeping it on desktop.
GardenEditorPage.tsx: The Link import from @tanstack/react-router is present (diff confirmed). The back affordance (🌱 → /gardens) replaces the hidden brand link exactly where needed. The height switch 100dvh-4rem (mobile) / 100dvh-8rem (desktop) correctly pairs with the header visibility change.
EditorRail.tsx: The tall prop is typed boolean with a sensible default. The max-h-[78vh] / max-h-[50vh] cap is overridden on desktop by the existing md:max-h-none — no breakpoint conflict.
ChatPanel.tsx: The spacing change (gap-2 → gap-3) is a pure layout tweak with no semantic impact.
The tall={railTab !== 'inspector'} expression correctly maps to the PR's intent: inspector stays a peek, while journal/history/assistant expand.
All domain logic, conditional scoping, and arithmetic expressions align with the stated intent and with each other.
🧹 Code cleanliness & maintainability — No material issues found
No material issues found.
The change is clean and consistent with existing patterns:
The tall prop on EditorRail is a focused, well-documented abstraction with a sensible default (false), so existing behavior is preserved for any future callers. Only GardenEditorPage.tsx:694 passes it, and the railTab !== 'inspector' logic is clearly explained inline.
The hideHeaderOnMobile variable in AppShell.tsx:51 follows the same naming and conditional-class pattern already used for showBottomNav two lines above it.
The back affordance (🌱) in the mobile top strip (GardenEditorPage.tsx:609-614) reuses the existing Link component and text-accent-strong styling from the global header, avoiding visual drift.
Height values (4rem / 8rem, 50vh / 78vh) are paired with comments that explain why they differ by breakpoint, so the magic numbers are justified.
No dead code or duplication introduced; PublicGardenPage.tsx correctly keeps calc(100dvh-8rem) because it doesn't hide the global header.
⚡ Performance — No material issues found
No material issues found
This PR is purely presentational—CSS breakpoint tweaks, conditional header hiding, and a rail height prop. Nothing in my lane:
No new data fetching, subscriptions, or polling.useAgentHistory and streamChat were already there.
No work in hot loops or render-time allocations. The gap-2 → gap-3 in ChatPanel is static className churn; it doesn't recompute per-message.
No unbounded growth or missing pagination. The rail's max-h-[78vh] is a strict cap, higher than the old 50vh but still bounded by viewport.
No new blocking calls or synchronous work on the main thread. The added Link and tall prop are trivial React prop passes.
Layout recalculation is event-driven, not continuous. The calc(100dvh-4rem) height changes once on mount/resize and once per tab switch; it doesn't fire in a loop.
🧯 Error handling & edge cases — No material issues found
No material issues found
The tall prop correctly falls back to false when railTab === 'inspector', and the {railTab && ...} guard ensures it's only evaluated when railTab is truthy, so tall is never computed for null.
EditorRail handles the empty-tabs edge case gracefully (if (!active) return null).
The new mobile back Link has an explicit aria-label and points to an existing authenticated route.
The PublicGardenPage correctly retains 100dvh-8rem since hideHeaderOnMobile intentionally excludes it (the PR comment and code agree).
activeId not matching any tab falls back to tabs[0] — existing behavior, not newly introduced.
Authz of the new "🌱" back link to /gardens — the /gardens route is auth-protected via beforeLoad: requireAuth (web/src/router.tsx:120), so the link is not a way to bypass auth; it's guarded like any other authed navigation. Verified by reading router.tsx:119-120.
Hiding the global header on mobile editor — the header is purely presentational chrome (brand + account avatar); no security-relevant state or guards are removed. The auth-protected route guard (requireAuth) still runs in beforeLoad regardless of whether the header renders, so hiding it on mobile doesn't weaken auth enforcement. Verified in AppShell.tsx:51 and router.tsx:132 (the editor route's beforeLoad).
No tainted input, injection, SSRF, secret leakage, or unsafe deserialization introduced. All changes are static className strings, a hardcoded emoji label, and a relative internal route link (to="/gardens" rendered via TanStack Link, not a raw href), so no path/open-redirect surface is added.
No new credentials, tokens, or untrusted data handling.
Nothing in my lens is materially wrong with this change.
🎯 Correctness — Minor issues
I'll verify the findings against the actual code.
Both findings verified against the actual code.
Finding 1 confirmed:AccountMenu (the only sign-out affordance — grep confirms it's referenced solely at AppShell.tsx:92, inside the header) is hidden on mobile in the editor (hideHeaderOnMobile && 'hidden md:block', line 60). The bottom nav is also hidden in the editor (showBottomNav = !!user && !canvasRoute, line 45, with canvasRoute = inEditor || inPublicGarden). The new 🌱 leaf (GardenEditorPage.tsx:609-615) links only to /gardens, not to account/settings. From /gardens the header and bottom nav reappear, so those destinations are reachable via an extra hop — a real, if recoverable, UX regression.
Finding 2 confirmed: The comment at GardenEditorPage.tsx:549-551 claims "only <main>'s padding is above us — 4rem," but <main> uses py-6 (AppShell.tsx:106) = 1.5rem top + 1.5rem bottom = 3rem total, not 4rem. With the header at zero height on mobile, actual vertical chrome is 3rem; 100dvh-4rem leaves ~1rem of slack. No overflow (the editor is shorter than the content box), but the stated reasoning is arithmetically wrong.
Verdict: Minor issues found
web/src/components/layout/AppShell.tsx:51 / web/src/pages/GardenEditorPage.tsx:609 — account / global nav requires an extra hop from the mobile editor. The hidden header carried AccountMenu (the only sign-out affordance — grep confirms it exists solely in AppShell's header at line 92) plus the desktop section links. On mobile in the editor the bottom nav is also hidden (showBottomNav = !!user && !canvasRoute), so the editor has no in-context access to sign-out, Settings, or Plants. The new 🌱 → /gardens leaf only returns to the gardens list; from there the header and bottom nav reappear, so the destinations are recoverable with one extra hop rather than truly lost. The PR's framing of the header as "pure chrome" understates what it carried. Suggested fix: fold an account affordance into the mobile editor strip the way the back leaf was.
web/src/pages/GardenEditorPage.tsx:553 — the comment's height justification doesn't match the value. The comment states "only <main>'s padding is above us — 4rem," but <main> uses py-6 (AppShell.tsx:106) = 1.5rem + 1.5rem = 3rem total, not 4rem. With the header display:none on mobile (zero height), the real chrome above+below is 3rem, so 100dvh-4rem leaves ~1rem of unexplained slack. No overflow results (the editor is shorter than main's content box), but the stated reasoning is arithmetically wrong — a future maintainer "correcting" 4rem to 3rem would push the mode bar flush against main's bottom padding. Suggested fix: reconcile the comment to the value (note the deliberate 1rem breathing room) or set the value to match the reasoning.
🧹 Code cleanliness & maintainability — Minor issues
I'll verify each finding against the actual code.
All four findings verified against the actual code. Confirming each:
AppShell.tsx:28 — Line 28-29 docstring says "the app bottom bar hides there (the brand link is the way back to the gardens list)". PR hides the brand header on mobile in the editor (hideHeaderOnMobile = inEditor, hidden md:block at line 60), and the new 🌱 Link at GardenEditorPage.tsx:609-615 is the back affordance. So the rationale "the brand link is the way back" is now false on mobile. Confirmed.
AppShell.tsx:37 — Line 36-37 comment says "both render a 100dvh-8rem canvas". Editor now renders 100dvh-4rem on mobile (GardenEditorPage.tsx:553). Confirmed stale.
GardenEditorPage.tsx:683 — Line 682-684 comment says "in-flow PEEK (≤50vh)". With tall, journal/history/assistant now peek at ≤78vh (EditorRail.tsx:65). Confirmed stale.
GardenEditorPage.tsx:690 — Lines 690-694 comment duplicates the rationale already in EditorRail.tsx:24-27 and the prop doc at 50-51. Confirmed as a trivial maintainability nit.
Verdict: Minor issues
web/src/components/layout/AppShell.tsx:28 (stale class docstring) — Verified. The component's JSDoc says the editor hides the bottom bar because "the brand link is the way back to the gardens list". This PR hides the brand header itself on mobile in the editor (hideHeaderOnMobile = inEditor → hidden md:block at line 60), and the new 🌱 leaf Link in GardenEditorPage (lines 609-615) is now the back affordance, per the new comment at GardenEditorPage.tsx:605-607. So the docstring's stated rationale — "the brand link is the way back" — is now false on mobile. CLAUDE.md: "stale comments are worse than none — the next reader believes them." Suggested fix: update the sentence to reflect that the editor's own strip carries the back affordance on mobile (or drop the parenthetical, since it no longer explains the hide).
web/src/components/layout/AppShell.tsx:37 (stale inline comment) — Verified. The comment says "the garden editor and the public shared-garden view both render a 100dvh-8rem canvas." After this PR the editor renders 100dvh-4rem on mobile (GardenEditorPage.tsx:553), so the "both render a 100dvh-8rem canvas" statement is now only true on desktop / for the public view. Low severity but the comment was written as a general invariant and is now misleading. Suggested fix: qualify it ("...both render a near-full-height canvas; the editor's exact band differs by breakpoint — see GardenEditorPage").
web/src/pages/GardenEditorPage.tsx:683 (stale inline comment above the rail) — Verified. The comment still says "an in-flow PEEK (≤50vh)". With tall, journal/history/assistant now peek at ≤78vh (EditorRail.tsx:65). The ≤50vh is now only the inspector case. Minor, but it's the kind of "looks authoritative, is half-wrong" comment CLAUDE.md warns about. Suggested fix: "an in-flow PEEK (≤50vh for the inspector, ≤78vh for panel modes — see EditorRail)".
web/src/pages/GardenEditorPage.tsx:690 (duplicated rationale) — Verified, minor maintainability nit. The same "inspector reads alongside the canvas / panel modes want room" explanation is written out twice: as the tall prop's JSDoc in EditorRail.tsx:24-27 (and the inline prop doc at 50-51) and again as an inline comment at the call site in GardenEditorPage.tsx:690-694. The call-site comment adds nothing the prop doc doesn't already say; it's copy-paste that will drift if one is updated and not the other. Suggested fix: trim the call-site comment to // Inspector stays a short peek; panels take the tall slice. and let EditorRail's prop doc carry the "why".
No dead code, no naming problems, no leaky abstractions, no copy-pasted logic. The tall prop is a clean single-purpose boolean and the conditional tall ? 'max-h-[78vh]' : 'max-h-[50vh]' reads clearly. The hideHeaderOnMobile flag and the emoji-leaf back Link are reasonable. The duplicated rationale text and the three stale comments are the only cleanliness concerns.
⚡ Performance — No material issues found
No material issues found
Through the performance lens this is a CSS/className and layout-only change — no queries, no new allocations, no work added to hot paths or render loops. The new tall prop is a boolean computed once per render (railTab !== 'inspector'), the Link is a single static element, and the gap-2 → gap-3 and max-h swaps are pure style strings. Nothing here regresses efficiency.
🧯 Error handling & edge cases — Minor issues
Verdict: Minor issues
Mobile editor loses the account/sign-out affordance (incl. the sign-out-retry error path).AppShell.tsx:51,60 now hides the global header on mobile in the editor (hideHeaderOnMobile = inEditor, applied as hidden md:block), and showBottomNav is already false for canvas routes (AppShell.tsx:45). The header's AccountMenu (AppShell.tsx:130-204) is the only sign-out surface — grep across web/src confirms no other useLogout/sign-out path exists in the UI. Its onLogout deliberately stays open on failure so the "Retry sign out" button (driven by logout.isError) remains visible (AppShell.tsx:140-149), the code comment explicitly calling it "the only retry affordance." On mobile in the editor that affordance is now gone until the user taps the new 🌱 link (GardenEditorPage.tsx:607) to leave for /gardens, where the header/bottom nav reappear. Recoverable, but a real edge-case regression: an error path the codebase engineered to keep reachable is unreachable on this route. Suggested fix: surface a compact account/sign-out entry on the editor's mobile strip (next to the 🌱 back link), or keep a slim account control visible when the header is hidden.
opencode/kimi-k2.6:cloud (opencode) — No material issues found
Verdict: No material issues found · ⚠️ 1/5 lens(es) errored — 5 reviewers: security, correctness, maintainability, performance, error-handling
🔒 Security — No material issues found
No material issues found.
This PR is a pure frontend styling/layout change (header visibility, height calculations, rail peek height, chat spacing). There are no authn/authz changes, no new data inputs, no injection surfaces, no secret handling, no route parameter handling, and no cross-origin or trust-boundary crossings. The Link added in GardenEditorPage.tsx is a hardcoded client-side navigation to /gardens with no query parameter construction or URL interpolation. The AccountMenu being hidden on mobile in the editor is a UX decision, not an authorization gap — the service layer still enforces permissions per CLAUDE.md.
🎯 Correctness — ⚠️ could not complete
⚠️ This reviewer failed to complete: opencode run context deadline exceeded
🧹 Code cleanliness & maintainability — No material issues found
No material issues found.
Wait — let me re-check one thing. The comment in GardenEditorPage.tsx at line 683 is part of the diff context and says "≤50vh", but with the tall prop the rail can now be 78vh. That's a stale comment in changed code.
Actually, I'll verify: is that comment in the hunks or just context? Looking at the diff:
@@ -673,6 +687,11 @@ export function GardenEditorPage() {
{/* The rail sits between the canvas and the mode bar. On mobile it's an
in-flow PEEK (≤50vh), so the garden stays visible above it and the mode
bar below (see EditorRail); on desktop it's the right-hand column. */}
This is context lines (not prefixed with + or -), so it's unchanged by the PR but adjacent to the change. However, it's now factually stale because the rail is no longer always ≤50vh. Since my lens is maintainability and stale comments are worse than none, I'll report it.
Minor issues
web/src/pages/GardenEditorPage.tsx:683 — The comment describing the rail as an "in-flow PEEK (≤50vh)" is now stale: with the tall={railTab !== 'inspector'} prop, panel modes (journal/history/assistant) render at 78vh. Future readers relying on this comment will be misled about the actual height behavior. Fix: Update the comment to reflect the new conditional height, e.g. a capped-height panel (inspector ≤50vh, panels ≤78vh).
That's the only finding in my lane. The tall prop is cleanly added, the Link import is properly placed, the hideHeaderOnMobile logic mirrors the existing showBottomNav pattern, and the Tailwind class composition is consistent with the codebase.
⚡ Performance — No material issues found
No material issues found
This PR is purely CSS/layout surface changes. Through the Performance lens:
EditorRail.tsx:65 — The new tall prop toggles between max-h-[78vh] and max-h-[50vh]. This is static class-name switching; it does not introduce extra re-renders beyond what already happens on tab change (activeId already mutates), nor does it trigger layout thrashing.
ChatPanel.tsx:146 — gap-2 → gap-3 is a spacing token change with no runtime cost.
GardenEditorPage.tsx:553/609 — The height calc change (4rem vs 8rem) and the added Link are layout-level only; no new data fetching, no effect hooks, and no hot-path work.
No N+1 queries, no unbounded growth, no missed memoization, and no work inside hot loops was introduced.
🧯 Error handling & edge cases — No material issues found
No material issues found
I examined the diff carefully through the error-handling and edge-cases lens:
Nil / empty collections:EditorRail already safely handles an empty tabs array (tabs[0] is undefined, caught by the if (!active) return null guard). No new nil-dereference paths are introduced.
Boundary / overflow: The tall prop correctly defaults to false, preserving the existing 50vh cap for any caller that omits it. The 78vh cap is relative to the viewport and the parent flex container has min-h-0 on children, so there’s no risk of negative-height overflow or CSS clamp bugs. The 100dvh-4rem / 100dvh-8rem heights correctly account for the header presence/absence, and the child fits inside <main>’s padded content box with room to spare (verified by re-deriving the box model).
Missing cleanup / rollback: No new effects, timers, or subscriptions are added; the existing nudgeTimer / nudgeFire cleanup is untouched.
Swallowed errors: No error-handling paths were modified.
Panics on bad input: The new Link uses a static to="/gardens" route that exists elsewhere in the app; no dynamic input is passed.
The landscape-shrunk canvas is an intentional design trade-off documented in the PR (“the canvas above just needs to stay glanceable”), not an unhandled error.
Automated adversarial review by Gadfly — consensus across the model swarm. Advisory only — does not block merge.
<!-- gadfly-consensus -->
## 🪰 Gadfly review — consensus across 5 models
**Verdict: Minor issues** · 8 findings (6 with multi-model agreement)
| | Finding | Where | Models | Lens |
|--|--|--|--|--|
| 🟠 | Hiding the global header on mobile in the editor removes the only sign-out affordance (AccountMenu), weakening session control on shared devices | `web/src/components/layout/AppShell.tsx:51` | 2/5 | correctness, error-handling, maintainability, security |
| 🟠 | Mobile peek height cap uses static vh instead of dvh, risking overflow past the visible viewport (hiding the always-visible mode bar) when panel content is tall — worsened by raising the cap from 50vh to 78vh | `web/src/editor/EditorRail.tsx:65` | 2/5 | correctness, maintainability |
| 🟡 | Stale class docstring: "the brand link is the way back" is now false on mobile editor after header hide | `web/src/components/layout/AppShell.tsx:28` | 2/5 | maintainability |
| 🟡 | Comment says editor and public-garden view both use a 100dvh-8rem canvas, but the editor's mobile band changed to 100dvh-4rem | `web/src/components/layout/AppShell.tsx:37` | 2/5 | maintainability |
| 🟡 | Mobile height subtracts 4rem but only 3rem of chrome (main py-6) exists with header hidden; leaves ~16px dead space and comment math is wrong | `web/src/pages/GardenEditorPage.tsx:553` | 2/5 | correctness |
| 🟡 | Comment says the mobile rail peek is capped at ≤50vh, but the new tall prop raises it to 78vh for all non-inspector tabs | `web/src/pages/GardenEditorPage.tsx:683` | 2/5 | maintainability |
<details><summary>2 single-model findings (lower confidence)</summary>
| | Finding | Where | Model | Lens |
|--|--|--|--|--|
| 🟡 | Back affordance is a bare emoji with only aria-label; sr-only text would be more robust | `web/src/pages/GardenEditorPage.tsx:609` | glm-5.2:cloud | maintainability |
| ⚪ | Duplicated 'tall' rationale comment already expressed in EditorRail prop doc | `web/src/pages/GardenEditorPage.tsx:690` | opencode/glm-5.2:cloud | maintainability |
</details>
<details><summary>Per-model detail</summary>
<details><summary><b>claude-code/sonnet</b> (claude-code) — Minor issues</summary>
**Verdict: Minor issues** — 5 reviewers: security, correctness, maintainability, performance, error-handling
<details><summary><b>🔒 Security</b> — No material issues found</summary>
No new data flow, authn/authz logic, injection surface, or secret handling in this diff — it's Tailwind class/layout changes (`hidden md:block` on the header, a `tall` height-cap prop, a `gap-2→gap-3` spacing tweak) plus one new `<Link to="/gardens">`. Verified in `AppShell.tsx` and `GardenEditorPage.tsx` that hiding the header on mobile doesn't remove any access-control check — `canEdit`/`isOwner` gating for edit actions lives elsewhere in the page body, untouched by this diff, and the account/sign-out control simply becomes desktop-only rather than being bypassed. `EditorRail`/`ChatPanel` changes are purely visual (height cap, gap).
VERDICT: No material issues found
No findings within the security lens — this PR touches only presentational Tailwind classes, a `tall` height-cap prop, and a static in-app navigation `Link`; no new input handling, no auth/authz logic, no external calls.
</details>
<details><summary><b>🎯 Correctness</b> — Minor issues</summary>
## VERDICT: Minor issues
- **`web/src/editor/EditorRail.tsx:65`** — The mobile peek's height cap uses `vh` (`max-h-[50vh]` / new `max-h-[78vh]`) rather than `dvh`, while the container it's nested inside (`GardenEditorPage.tsx:553`, `h-[calc(100dvh-4rem)] ... md:h-[calc(100dvh-8rem)]`) was deliberately switched to `dvh` for the exact reason documented in the adjacent comment at `GardenEditorPage.tsx:544-546` (#85): mobile Safari/Chrome's `100vh` is the *largest* viewport (address bar hidden), so with the bar showing, anything sized off `vh` overflows the actually-visible area.
Confirmed the flex structure directly: the outer editor container (`GardenEditorPage.tsx:553`) is `flex-col` with a fixed `dvh`-based height and no `overflow-hidden`; the canvas column (`GardenEditorPage.tsx:603`, `flex-1 min-h-0`) can shrink to accommodate siblings, but both the rail (`EditorRail.tsx:64`, `shrink-0`) and the always-visible mobile mode bar (`GardenEditorPage.tsx:715`, `shrink-0`) refuse to shrink. Because the rail's cap is computed against the *large* `vh` instead of the container's `dvh`, on a device where the address bar is showing, the rail can size itself larger than the space actually left after the fixed-height container and the shrink-0 mode bar are accounted for, pushing content (including the mode bar) past the bottom of the visible screen.
This mismatch already existed at `max-h-[50vh]`, but raising the cap to `78vh` (per `EditorRail.tsx:44-51`, `GardenEditorPage.tsx:690-694` wiring `tall={railTab !== 'inspector'}` for journal/history/chat) increases the realistic overshoot, since panel content (a chat thread, journal entries, history list) is far more likely to actually reach a 78% cap than a 50% one.
Suggested fix: express the cap in `dvh` (e.g. `max-h-[78dvh]` / `max-h-[50dvh]`) to match the container it's nested in, consistent with the fix already applied one level up.
</details>
<details><summary><b>🧹 Code cleanliness & maintainability</b> — Minor issues</summary>
**VERDICT: Minor issues**
- `web/src/components/layout/AppShell.tsx:29` — The docstring says "the app bottom bar hides there (the brand link is the way back to the gardens list)," but the diff adds `hideHeaderOnMobile = inEditor` (line 51) which hides the entire header — including the brand link — on mobile in the editor (line 57-61: `hidden md:block`). On mobile the brand link is no longer visible in the editor; the new `🌱` link in `GardenEditorPage.tsx:609-615` is now the way back. The comment describes desktop-only behavior as if universal.
- `web/src/components/layout/AppShell.tsx:37` — The comment "the garden editor and the public shared-garden view both render a 100dvh-8rem canvas" is now false on mobile: `GardenEditorPage.tsx:553` changed the editor's mobile band to `h-[calc(100dvh-4rem)]`, keeping `8rem` only at `md:`. The two canvas routes no longer match on mobile, but the comment still claims they do.
- `web/src/pages/GardenEditorPage.tsx:683` — The comment directly above the `EditorRail` usage still says the mobile peek is "≤50vh," but line 694 in the same diff introduces `tall={railTab !== 'inspector'}`, which raises the cap to 78vh (confirmed in `EditorRail.tsx:65`, `tall ? 'max-h-[78vh]' : 'max-h-[50vh]'`) for every rail tab except the inspector — the common case. Notably, `EditorRail.tsx`'s own docstring was correctly updated in this same diff to say "a capped-height panel," making this leftover `GardenEditorPage.tsx` comment the one stale outlier.
</details>
<details><summary><b>⚡ Performance</b> — No material issues found</summary>
**VERDICT: No material issues found**
This diff is Tailwind class changes (a `hidden md:block` toggle, `max-h-[50vh]` → conditional `max-h-[78vh]`, `gap-2` → `gap-3`), a `tall` boolean prop threaded through `EditorRail`, and a new `<Link>` in the mobile strip. I read `AppShell.tsx`, `EditorRail.tsx`, and the surrounding `GardenEditorPage.tsx` context to confirm there's no hidden cost:
- `hideHeaderOnMobile` (`AppShell.tsx:51,57-61`) uses CSS `hidden md:block`, not conditional mounting — the header's subtree (including `AccountMenu`'s `useLogout`/`useMe` hooks) mounts exactly as it did before this change; visibility is display-only. No new render/mount cost.
- `tall` (`EditorRail.tsx:44-67`) is a plain boolean prop picking between two static Tailwind classes via `cn()` — no computation, no re-render amplification beyond the normal prop-driven re-render already inherent to the component.
- The `Link to="/gardens"` addition (`GardenEditorPage.tsx:609-615`) is a single static element, not inside any loop or list.
- `gap-2 → gap-3` in `ChatPanel.tsx:146` is pure CSS.
No loops, no data-fetching changes, no unbounded growth, nothing on a hot path. Nothing in my lane here.
</details>
<details><summary><b>🧯 Error handling & edge cases</b> — No material issues found</summary>
**VERDICT: No material issues found**
This PR is CSS/layout-only (header visibility via `hidden md:block`, a `max-h` cap swap, a `gap-2`→`gap-3` tweak) plus one added navigation `<Link>`. None of it introduces new async operations, error paths, resource acquisition, or state that needs cleanup — nothing for this lens to bite into.
Specifically checked and found no issues:
- `AppShell.tsx:57-62` — `hideHeaderOnMobile` only hides the header via CSS (`hidden md:block`); the component doesn't unmount, so `AccountMenu`'s effects/backdrop logic (`AppShell.tsx:138`) keep working unaffected. Confirmed via `router.tsx` that `gardens/$gardenId` always requires auth (`requireAuth` at `router.tsx:132`), so there's no edge case of an unauthenticated user losing the header's "Sign in" link on this route.
- `EditorRail.tsx:44,51,65` — new `tall` prop defaults to `false`, preserving prior behavior; confirmed via grep it has a single call site (`GardenEditorPage.tsx:694`), so no other caller is affected by the default change. Pre-existing `tabs.find(...) ?? tabs[0]` / `if (!active) return null` guard is untouched.
- `GardenEditorPage.tsx:608-615` — new back `<Link to="/gardens">` renders only after the existing loading/gone-state guards higher in the component (verified `gardenGone`/loading handling precedes this JSX), so `garden.name` being dereferenced nearby is not a new null-access risk.
- `GardenEditorPage.tsx:553` — height calc changes only the subtracted constant (`4rem` vs `8rem`); this reduces (not increases) the risk of a negative/degenerate height versus the prior `8rem` on mobile.
- `ChatPanel.tsx:146` — spacing-only change, no logic touched.
</details>
</details>
<details><summary><b>glm-5.2:cloud</b> (ollama-cloud) — Minor issues</summary>
**Verdict: Minor issues** — 5 reviewers: security, correctness, maintainability, performance, error-handling
<details><summary><b>🔒 Security</b> — Minor issues</summary>
Confirmed. The global header containing `AccountMenu` (the only sign-out control, `AppShell.tsx:92`/`130-197`) is now `hidden md:block` whenever `hideHeaderOnMobile` (`inEditor`, `AppShell.tsx:51,57-61`) is true; `showBottomNav` is `false` on the editor route (`canvasRoute` true, `AppShell.tsx:45,121`), so the bottom nav isn't a fallback either. The editor's mobile strip (`GardenEditorPage.tsx:608-630`) adds only the 🌱 `Link to="/gardens"`, garden name, view-only badge, season picker, and share button — no account/sign-out control. The only sign-out affordance lives in `AccountMenu` in the hidden header. Finding survives.
**VERDICT: Minor issues**
- **Loss of the only sign-out affordance on mobile in the editor** (`web/src/components/layout/AppShell.tsx:51,57-61`). The global header — which contains `AccountMenu`, the app's only "Sign out" control (`AppShell.tsx:92`/`130-197`) — is now `hidden md:block` whenever `inEditor` is true. The editor's replacement mobile strip (`GardenEditorPage.tsx:608-630`) adds only a 🌱 `Link to="/gardens"` and garden/season/share controls — no account or sign-out control. `showBottomNav` is also `false` on the editor route (`canvasRoute`, `AppShell.tsx:45,121`), so the bottom nav (which would otherwise carry account access) isn't there either. Net effect: an authenticated user on a phone inside the garden editor has no visible UI to end their session. On a shared/loaned device this weakens session control — the user can't sign out before handing the phone back and must know to navigate away to find the control. Concrete impact: degraded ability to terminate an authenticated session; not an authn/authz bypass. Suggested fix: surface a compact account/sign-out control in the editor's mobile strip (or make `AccountMenu` available via the editor), rather than only in the hidden header.
</details>
<details><summary><b>🎯 Correctness</b> — Minor issues</summary>
Verdict: Minor issues
- `web/src/pages/GardenEditorPage.tsx:553` — Mobile height is `h-[calc(100dvh-4rem)]`, but with the global header hidden on mobile (`AppShell.tsx` header is `hidden md:block`), the only chrome outside the editor's flex column is `<main>`'s `px-4 py-6` — i.e. **3rem** of vertical padding (1.5rem top + 1.5rem bottom), not 4rem. Subtracting 4rem leaves ~16px of dead space the viewport would otherwise allow. This contradicts the comment at `GardenEditorPage.tsx:548-551` ("only `<main>`'s padding is above us — 4rem"). Impact is small: the canvas still renders correctly and reclaims most of the header room; it just leaves 16px on the table and the numerical justification is wrong. Fix: either use `h-[calc(100dvh-3rem)]` on mobile, or correct the comment to state the 1rem is an intentional buffer.
- `web/src/editor/EditorRail.tsx:65` — The `tall` peek cap uses `max-h-[78vh]` while the editor column is sized in `dvh` (`100dvh-4rem`). On mobile browsers with the URL bar visible `100dvh < 100vh`, so a content-filled 78vh rail plus the ~5rem of strip + mode bar can approach or exceed the `dvh`-based column height. This is the same `vh`-vs-`dvh` class of issue (#85) the editor already corrected for its own height. Not certain to overflow in practice (depends on rail content and exact mode-bar height), but the unit inconsistency is real. Fix: `max-h-[78dvh]` to match the column's unit basis.
</details>
<details><summary><b>🧹 Code cleanliness & maintainability</b> — Minor issues</summary>
**Verdict: Minor issues found**
The change is clean and works as intended. A few maintainability nits survive verification:
- `web/src/components/layout/AppShell.tsx:51` — `hideHeaderOnMobile` is a single-use alias for `inEditor`, with a five-line comment (lines 46–50) whose rationale substantially overlaps the file-top docblock (lines 24–30: "the editor is a full-screen context… the brand link is the way back to the gardens list"). Not a defect — the alias does aid readability of the `cn(...)` call — but the comment is heavier than the one-line decision needs. Trivial.
- `web/src/pages/GardenEditorPage.tsx:609` — Confirmed: the back affordance is a bare `🌱` emoji in a `Link` with `aria-label="All gardens"` and no visible/`sr-only` text, while the header brand it replaces (`AppShell.tsx:67`) renders `🌱 pansy` with the wordmark. With the header hidden on mobile here, this leaf is the only brand/back affordance. Reliance on an emoji glyph plus `aria-label` alone is fragile across screen readers / platform glyph rendering; adding `<span className="sr-only">All gardens</span>` would be more robust. Minor.
- `web/src/editor/EditorRail.tsx:65` — Confirmed: the `tall ? 'max-h-[78vh]' : 'max-h-[50vh]'` ternary uses magic viewport percentages, and the "reading and typing are the task" rationale appears verbatim-ish in both the `EditorRail` docblock (line 26) and the call site comment (`GardenEditorPage.tsx:692`). Two copies of the same "why" will drift. Trivial.
</details>
<details><summary><b>⚡ Performance</b> — No material issues found</summary>
**No material issues found.**
Through the performance lens, the change is purely CSS class / prop plumbing — no new allocations, queries, loops, or blocking work on any path. I verified:
- `EditorRail.tsx:64-66` — the `tall` toggle just swaps one Tailwind `max-h` class string via `cn`; no runtime cost beyond the class lookup. `tall={railTab !== 'inspector'}` (`GardenEditorPage.tsx:694`) is a cheap boolean prop recomputed on render — negligible.
- `AppShell.tsx` — the `hidden md:block` header is a pure display toggle; the header subtree still mounts, but that was already the case (it was always rendered), so no render-cost regression.
- `GardenEditorPage.tsx` — the `Link` and `🌱` glyph are static, render once; no list work, no effect added.
- `ChatPanel.tsx:146` — `gap-2 → gap-3` is a layout spacing change, not a loop/allocation change.
No N+1, no unbounded growth, no hot-path regression introduced.
</details>
<details><summary><b>🧯 Error handling & edge cases</b> — No material issues found</summary>
Through the error-handling & edge-cases lens, I verified the unhappy paths this diff touches:
- **`EditorRail` empty/degenerate input** (`EditorRail.tsx:53-54`): `active = tabs.find(...) ?? tabs[0]`; if `tabs` is empty, `tabs[0]` is `undefined` and the `if (!active) return null` guard handles it — no panic. Verified by reading the file.
- **`tall={railTab !== 'inspector'}`** (`GardenEditorPage.tsx:694`): `EditorRail` is only rendered inside `{railTab && (...)}` (line 685), so `railTab` is guaranteed truthy here. The `tall` prop has a `= false` default and is always a boolean. No null/undefined edge case. Verified.
- **`hideHeaderOnMobile = inEditor`** (`AppShell.tsx:51`): boolean, `!!`-coerced from `matchRoute`. The `cn(..., hideHeaderOnMobile && 'hidden md:block')` safely yields `false` (no class) when off. No bad-input path.
- **`Link to="/gardens"`** (`GardenEditorPage.tsx:609`): `/gardens` is an existing route (used in `AppShell.tsx:66`), so no resolution failure; the link is rendered unconditionally inside the mobile strip, which is fine for both owner and view-only viewers (verified the strip has no auth-gating around the link).
- **Mobile editor now has no account/sign-out surface** (header `hidden`, bottom nav already suppressed for canvas routes at `AppShell.tsx:45`): this is a UX consideration, not an unhandled error — the 🌱 link returns to `/gardens` where the header reappears. Not an error-handling finding.
No swallowed errors, missing `defer`/cleanup, panics on bad input, or unhandled nil/empty/boundary conditions are introduced by this change.
Verdict: No material issues found.
</details>
</details>
<details><summary><b>kimi-k2.6:cloud</b> (ollama-cloud) — No material issues found</summary>
**Verdict: No material issues found** — 5 reviewers: security, correctness, maintainability, performance, error-handling
<details><summary><b>🔒 Security</b> — No material issues found</summary>
No material issues found
This is a purely presentational layout change. From a security lens:
- **No auth bypass**: The editor route still enforces `requireAuth` in `router.tsx:132`; the header is only visually hidden with CSS (`hidden md:block`), not removed from an authenticated shell.
- **No new injection surface**: The only rendered dynamic data, `garden.name`, was already present before this diff and is safely interpolated by React inside a `<h1>`. The new `<Link to="/gardens">` has a hardcoded path and `aria-label`.
- **No trust boundary change**: The `tall` prop and `max-h-[78vh]` cap are driven by a hardcoded local store value (`railTab`), not by user input or URL parameters.
- **No credential leakage**: Hiding the global header on mobile removes the account menu from view, but this is a responsive design choice, not a data-leak vector — the DOM is hidden with `display: none`, not rendered in the public view (the public garden route `/g/$token` explicitly keeps its header).
</details>
<details><summary><b>🎯 Correctness</b> — No material issues found</summary>
**No material issues found**
I verified the logic carefully:
- **`AppShell.tsx`**: `hideHeaderOnMobile` is scoped to `inEditor` only, intentionally preserving the global header for the public garden view which has no back affordance of its own. The `hidden md:block` breakpoint correctly hides the header on mobile while keeping it on desktop.
- **`GardenEditorPage.tsx`**: The `Link` import from `@tanstack/react-router` is present (diff confirmed). The back affordance (`🌱` → `/gardens`) replaces the hidden brand link exactly where needed. The height switch `100dvh-4rem` (mobile) / `100dvh-8rem` (desktop) correctly pairs with the header visibility change.
- **`EditorRail.tsx`**: The `tall` prop is typed `boolean` with a sensible default. The `max-h-[78vh]` / `max-h-[50vh]` cap is overridden on desktop by the existing `md:max-h-none` — no breakpoint conflict.
- **`ChatPanel.tsx`**: The spacing change (`gap-2 → gap-3`) is a pure layout tweak with no semantic impact.
- The `tall={railTab !== 'inspector'}` expression correctly maps to the PR's intent: inspector stays a peek, while journal/history/assistant expand.
All domain logic, conditional scoping, and arithmetic expressions align with the stated intent and with each other.
</details>
<details><summary><b>🧹 Code cleanliness & maintainability</b> — No material issues found</summary>
No material issues found.
The change is clean and consistent with existing patterns:
- The `tall` prop on `EditorRail` is a focused, well-documented abstraction with a sensible default (`false`), so existing behavior is preserved for any future callers. Only `GardenEditorPage.tsx:694` passes it, and the `railTab !== 'inspector'` logic is clearly explained inline.
- The `hideHeaderOnMobile` variable in `AppShell.tsx:51` follows the same naming and conditional-class pattern already used for `showBottomNav` two lines above it.
- The back affordance (`🌱`) in the mobile top strip (`GardenEditorPage.tsx:609-614`) reuses the existing `Link` component and `text-accent-strong` styling from the global header, avoiding visual drift.
- Height values (`4rem` / `8rem`, `50vh` / `78vh`) are paired with comments that explain *why* they differ by breakpoint, so the magic numbers are justified.
- No dead code or duplication introduced; `PublicGardenPage.tsx` correctly keeps `calc(100dvh-8rem)` because it doesn't hide the global header.
</details>
<details><summary><b>⚡ Performance</b> — No material issues found</summary>
**No material issues found**
This PR is purely presentational—CSS breakpoint tweaks, conditional header hiding, and a rail height prop. Nothing in my lane:
- **No new data fetching, subscriptions, or polling.** `useAgentHistory` and `streamChat` were already there.
- **No work in hot loops or render-time allocations.** The `gap-2 → gap-3` in `ChatPanel` is static className churn; it doesn't recompute per-message.
- **No unbounded growth or missing pagination.** The rail's `max-h-[78vh]` is a strict cap, higher than the old `50vh` but still bounded by viewport.
- **No new blocking calls or synchronous work on the main thread.** The added `Link` and `tall` prop are trivial React prop passes.
- **Layout recalculation is event-driven, not continuous.** The `calc(100dvh-4rem)` height changes once on mount/resize and once per tab switch; it doesn't fire in a loop.
</details>
<details><summary><b>🧯 Error handling & edge cases</b> — No material issues found</summary>
**No material issues found**
- The `tall` prop correctly falls back to `false` when `railTab === 'inspector'`, and the `{railTab && ...}` guard ensures it's only evaluated when `railTab` is truthy, so `tall` is never computed for `null`.
- `EditorRail` handles the empty-tabs edge case gracefully (`if (!active) return null`).
- The new mobile back `Link` has an explicit `aria-label` and points to an existing authenticated route.
- The `PublicGardenPage` correctly retains `100dvh-8rem` since `hideHeaderOnMobile` intentionally excludes it (the PR comment and code agree).
- `activeId` not matching any tab falls back to `tabs[0]` — existing behavior, not newly introduced.
</details>
</details>
<details><summary><b>opencode/glm-5.2:cloud</b> (opencode) — Minor issues</summary>
**Verdict: Minor issues** — 5 reviewers: security, correctness, maintainability, performance, error-handling
<details><summary><b>🔒 Security</b> — No material issues found</summary>
# Verdict: No material issues found
Reviewed through the 🔒 Security lens.
- **Authz of the new "🌱" back link to `/gardens`** — the `/gardens` route is auth-protected via `beforeLoad: requireAuth` (`web/src/router.tsx:120`), so the link is not a way to bypass auth; it's guarded like any other authed navigation. Verified by reading `router.tsx:119-120`.
- **Hiding the global header on mobile editor** — the header is purely presentational chrome (brand + account avatar); no security-relevant state or guards are removed. The auth-protected route guard (`requireAuth`) still runs in `beforeLoad` regardless of whether the header renders, so hiding it on mobile doesn't weaken auth enforcement. Verified in `AppShell.tsx:51` and `router.tsx:132` (the editor route's `beforeLoad`).
- **No tainted input, injection, SSRF, secret leakage, or unsafe deserialization** introduced. All changes are static className strings, a hardcoded emoji label, and a relative internal route link (`to="/gardens"` rendered via TanStack `Link`, not a raw `href`), so no path/open-redirect surface is added.
- No new credentials, tokens, or untrusted data handling.
Nothing in my lens is materially wrong with this change.
</details>
<details><summary><b>🎯 Correctness</b> — Minor issues</summary>
I'll verify the findings against the actual code.
Both findings verified against the actual code.
- **Finding 1 confirmed:** `AccountMenu` (the only sign-out affordance — grep confirms it's referenced solely at `AppShell.tsx:92`, inside the header) is hidden on mobile in the editor (`hideHeaderOnMobile && 'hidden md:block'`, line 60). The bottom nav is also hidden in the editor (`showBottomNav = !!user && !canvasRoute`, line 45, with `canvasRoute = inEditor || inPublicGarden`). The new 🌱 leaf (`GardenEditorPage.tsx:609-615`) links only to `/gardens`, not to account/settings. From `/gardens` the header and bottom nav reappear, so those destinations are reachable via an extra hop — a real, if recoverable, UX regression.
- **Finding 2 confirmed:** The comment at `GardenEditorPage.tsx:549-551` claims "only `<main>`'s padding is above us — 4rem," but `<main>` uses `py-6` (`AppShell.tsx:106`) = 1.5rem top + 1.5rem bottom = 3rem total, not 4rem. With the header at zero height on mobile, actual vertical chrome is 3rem; `100dvh-4rem` leaves ~1rem of slack. No overflow (the editor is shorter than the content box), but the stated reasoning is arithmetically wrong.
## Verdict: Minor issues found
- **`web/src/components/layout/AppShell.tsx:51` / `web/src/pages/GardenEditorPage.tsx:609` — account / global nav requires an extra hop from the mobile editor.** The hidden header carried `AccountMenu` (the only sign-out affordance — grep confirms it exists solely in `AppShell`'s header at line 92) plus the desktop section links. On mobile in the editor the bottom nav is also hidden (`showBottomNav = !!user && !canvasRoute`), so the editor has no in-context access to sign-out, Settings, or Plants. The new `🌱 → /gardens` leaf only returns to the gardens list; from there the header and bottom nav reappear, so the destinations are recoverable with one extra hop rather than truly lost. The PR's framing of the header as "pure chrome" understates what it carried. Suggested fix: fold an account affordance into the mobile editor strip the way the back leaf was.
- **`web/src/pages/GardenEditorPage.tsx:553` — the comment's height justification doesn't match the value.** The comment states "only `<main>`'s padding is above us — 4rem," but `<main>` uses `py-6` (`AppShell.tsx:106`) = 1.5rem + 1.5rem = 3rem total, not 4rem. With the header `display:none` on mobile (zero height), the real chrome above+below is 3rem, so `100dvh-4rem` leaves ~1rem of unexplained slack. No overflow results (the editor is shorter than main's content box), but the stated reasoning is arithmetically wrong — a future maintainer "correcting" 4rem to 3rem would push the mode bar flush against main's bottom padding. Suggested fix: reconcile the comment to the value (note the deliberate 1rem breathing room) or set the value to match the reasoning.
</details>
<details><summary><b>🧹 Code cleanliness & maintainability</b> — Minor issues</summary>
I'll verify each finding against the actual code.
All four findings verified against the actual code. Confirming each:
1. **AppShell.tsx:28** — Line 28-29 docstring says "the app bottom bar hides there (the brand link is the way back to the gardens list)". PR hides the brand header on mobile in the editor (`hideHeaderOnMobile = inEditor`, `hidden md:block` at line 60), and the new 🌱 Link at GardenEditorPage.tsx:609-615 is the back affordance. So the rationale "the brand link is the way back" is now false on mobile. Confirmed.
2. **AppShell.tsx:37** — Line 36-37 comment says "both render a 100dvh-8rem canvas". Editor now renders `100dvh-4rem` on mobile (GardenEditorPage.tsx:553). Confirmed stale.
3. **GardenEditorPage.tsx:683** — Line 682-684 comment says "in-flow PEEK (≤50vh)". With `tall`, journal/history/assistant now peek at ≤78vh (EditorRail.tsx:65). Confirmed stale.
4. **GardenEditorPage.tsx:690** — Lines 690-694 comment duplicates the rationale already in EditorRail.tsx:24-27 and the prop doc at 50-51. Confirmed as a trivial maintainability nit.
**Verdict: Minor issues**
- **`web/src/components/layout/AppShell.tsx:28` (stale class docstring)** — Verified. The component's JSDoc says the editor hides the bottom bar because *"the brand link is the way back to the gardens list"*. This PR hides the brand header itself on mobile in the editor (`hideHeaderOnMobile = inEditor` → `hidden md:block` at line 60), and the new 🌱 leaf `Link` in `GardenEditorPage` (lines 609-615) is now the back affordance, per the new comment at `GardenEditorPage.tsx:605-607`. So the docstring's stated rationale — "the brand link is the way back" — is now false on mobile. CLAUDE.md: "stale comments are worse than none — the next reader believes them." Suggested fix: update the sentence to reflect that the editor's own strip carries the back affordance on mobile (or drop the parenthetical, since it no longer explains the hide).
- **`web/src/components/layout/AppShell.tsx:37` (stale inline comment)** — Verified. The comment says "the garden editor and the public shared-garden view both render a `100dvh-8rem` canvas." After this PR the editor renders `100dvh-4rem` on mobile (`GardenEditorPage.tsx:553`), so the "both render a 100dvh-8rem canvas" statement is now only true on desktop / for the public view. Low severity but the comment was written as a general invariant and is now misleading. Suggested fix: qualify it ("...both render a near-full-height canvas; the editor's exact band differs by breakpoint — see GardenEditorPage").
- **`web/src/pages/GardenEditorPage.tsx:683` (stale inline comment above the rail)** — Verified. The comment still says "an in-flow PEEK (≤50vh)". With `tall`, journal/history/assistant now peek at ≤78vh (EditorRail.tsx:65). The ≤50vh is now only the inspector case. Minor, but it's the kind of "looks authoritative, is half-wrong" comment CLAUDE.md warns about. Suggested fix: "an in-flow PEEK (≤50vh for the inspector, ≤78vh for panel modes — see EditorRail)".
- **`web/src/pages/GardenEditorPage.tsx:690` (duplicated rationale)** — Verified, minor maintainability nit. The same "inspector reads alongside the canvas / panel modes want room" explanation is written out twice: as the `tall` prop's JSDoc in `EditorRail.tsx:24-27` (and the inline prop doc at 50-51) and again as an inline comment at the call site in `GardenEditorPage.tsx:690-694`. The call-site comment adds nothing the prop doc doesn't already say; it's copy-paste that will drift if one is updated and not the other. Suggested fix: trim the call-site comment to `// Inspector stays a short peek; panels take the tall slice.` and let `EditorRail`'s prop doc carry the "why".
No dead code, no naming problems, no leaky abstractions, no copy-pasted *logic*. The `tall` prop is a clean single-purpose boolean and the conditional `tall ? 'max-h-[78vh]' : 'max-h-[50vh]'` reads clearly. The `hideHeaderOnMobile` flag and the emoji-leaf back Link are reasonable. The duplicated *rationale text* and the three stale comments are the only cleanliness concerns.
</details>
<details><summary><b>⚡ Performance</b> — No material issues found</summary>
No material issues found
Through the performance lens this is a CSS/className and layout-only change — no queries, no new allocations, no work added to hot paths or render loops. The new `tall` prop is a boolean computed once per render (`railTab !== 'inspector'`), the `Link` is a single static element, and the `gap-2 → gap-3` and `max-h` swaps are pure style strings. Nothing here regresses efficiency.
</details>
<details><summary><b>🧯 Error handling & edge cases</b> — Minor issues</summary>
Verdict: Minor issues
- **Mobile editor loses the account/sign-out affordance (incl. the sign-out-retry error path).** `AppShell.tsx:51,60` now hides the global header on mobile in the editor (`hideHeaderOnMobile = inEditor`, applied as `hidden md:block`), and `showBottomNav` is already false for canvas routes (`AppShell.tsx:45`). The header's `AccountMenu` (`AppShell.tsx:130-204`) is the only sign-out surface — grep across `web/src` confirms no other `useLogout`/sign-out path exists in the UI. Its `onLogout` deliberately stays open on failure so the "Retry sign out" button (driven by `logout.isError`) remains visible (`AppShell.tsx:140-149`), the code comment explicitly calling it "the only retry affordance." On mobile in the editor that affordance is now gone until the user taps the new 🌱 link (`GardenEditorPage.tsx:607`) to leave for `/gardens`, where the header/bottom nav reappear. Recoverable, but a real edge-case regression: an error path the codebase engineered to keep reachable is unreachable on this route. Suggested fix: surface a compact account/sign-out entry on the editor's mobile strip (next to the 🌱 back link), or keep a slim account control visible when the header is hidden.
</details>
</details>
<details><summary><b>opencode/kimi-k2.6:cloud</b> (opencode) — No material issues found</summary>
**Verdict: No material issues found · ⚠️ 1/5 lens(es) errored** — 5 reviewers: security, correctness, maintainability, performance, error-handling
<details><summary><b>🔒 Security</b> — No material issues found</summary>
No material issues found.
This PR is a pure frontend styling/layout change (header visibility, height calculations, rail peek height, chat spacing). There are no authn/authz changes, no new data inputs, no injection surfaces, no secret handling, no route parameter handling, and no cross-origin or trust-boundary crossings. The `Link` added in `GardenEditorPage.tsx` is a hardcoded client-side navigation to `/gardens` with no query parameter construction or URL interpolation. The `AccountMenu` being hidden on mobile in the editor is a UX decision, not an authorization gap — the service layer still enforces permissions per `CLAUDE.md`.
</details>
<details><summary><b>🎯 Correctness</b> — ⚠️ could not complete</summary>
⚠️ This reviewer failed to complete: opencode run context deadline exceeded
</details>
<details><summary><b>🧹 Code cleanliness & maintainability</b> — No material issues found</summary>
No material issues found.
Wait — let me re-check one thing. The comment in `GardenEditorPage.tsx` at line 683 is part of the diff context and says "≤50vh", but with the `tall` prop the rail can now be 78vh. That's a stale comment in changed code.
Actually, I'll verify: is that comment in the hunks or just context? Looking at the diff:
```
@@ -673,6 +687,11 @@ export function GardenEditorPage() {
{/* The rail sits between the canvas and the mode bar. On mobile it's an
in-flow PEEK (≤50vh), so the garden stays visible above it and the mode
bar below (see EditorRail); on desktop it's the right-hand column. */}
```
This is context lines (not prefixed with `+` or `-`), so it's unchanged by the PR but adjacent to the change. However, it's now factually stale because the rail is no longer always ≤50vh. Since my lens is maintainability and stale comments are worse than none, I'll report it.
**Minor issues**
- **`web/src/pages/GardenEditorPage.tsx:683`** — The comment describing the rail as an "in-flow PEEK (≤50vh)" is now stale: with the `tall={railTab !== 'inspector'}` prop, panel modes (journal/history/assistant) render at 78vh. Future readers relying on this comment will be misled about the actual height behavior.
**Fix:** Update the comment to reflect the new conditional height, e.g. `a capped-height panel (inspector ≤50vh, panels ≤78vh)`.
That's the only finding in my lane. The `tall` prop is cleanly added, the `Link` import is properly placed, the `hideHeaderOnMobile` logic mirrors the existing `showBottomNav` pattern, and the Tailwind class composition is consistent with the codebase.
</details>
<details><summary><b>⚡ Performance</b> — No material issues found</summary>
**No material issues found**
This PR is purely CSS/layout surface changes. Through the Performance lens:
- **`EditorRail.tsx:65`** — The new `tall` prop toggles between `max-h-[78vh]` and `max-h-[50vh]`. This is static class-name switching; it does not introduce extra re-renders beyond what already happens on tab change (`activeId` already mutates), nor does it trigger layout thrashing.
- **`ChatPanel.tsx:146`** — `gap-2 → gap-3` is a spacing token change with no runtime cost.
- **`GardenEditorPage.tsx:553/609`** — The height calc change (`4rem` vs `8rem`) and the added `Link` are layout-level only; no new data fetching, no effect hooks, and no hot-path work.
- No N+1 queries, no unbounded growth, no missed memoization, and no work inside hot loops was introduced.
</details>
<details><summary><b>🧯 Error handling & edge cases</b> — No material issues found</summary>
No material issues found
I examined the diff carefully through the error-handling and edge-cases lens:
- **Nil / empty collections:** `EditorRail` already safely handles an empty `tabs` array (`tabs[0]` is `undefined`, caught by the `if (!active) return null` guard). No new nil-dereference paths are introduced.
- **Boundary / overflow:** The `tall` prop correctly defaults to `false`, preserving the existing `50vh` cap for any caller that omits it. The `78vh` cap is relative to the viewport and the parent flex container has `min-h-0` on children, so there’s no risk of negative-height overflow or CSS clamp bugs. The `100dvh-4rem` / `100dvh-8rem` heights correctly account for the header presence/absence, and the child fits inside `<main>`’s padded content box with room to spare (verified by re-deriving the box model).
- **Missing cleanup / rollback:** No new effects, timers, or subscriptions are added; the existing `nudgeTimer` / `nudgeFire` cleanup is untouched.
- **Swallowed errors:** No error-handling paths were modified.
- **Panics on bad input:** The new `Link` uses a static `to="/gardens"` route that exists elsewhere in the app; no dynamic input is passed.
The landscape-shrunk canvas is an intentional design trade-off documented in the PR (“the canvas above just needs to stay glanceable”), not an unhandled error.
</details>
</details>
</details>
<sub>Automated adversarial review by Gadfly — consensus across the model swarm. Advisory only — does not block merge.</sub>
- Fold the account menu into the editor's mobile strip. Hiding the global
header removed the only sign-out on mobile in the editor; the strip now
carries it, so the space win stays but sign-out is one tap away.
- EditorRail peek cap vh → dvh, matching the dvh-bounded editor column, so
it can't overrun the visible viewport and push the mode bar off-screen.
- Mobile editor height 4rem → 3rem: with the header hidden, only <main>'s
py-6 (3rem) is outside the editor, so 4rem left ~16px dead. Comment
corrected.
- Trim two comments that duplicated nearby docs.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ
steve
merged commit 9e227e29eb into main2026-07-23 01:32:59 +00:00
steve
deleted branch feat/mobile-space-polish2026-07-23 01:32:59 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Two mobile wasted-space complaints, both in the editor/assistant context.
1. The editor's top banner was pure chrome
On a phone the global top bar (brand + account avatar) sat above the editor's own garden-name strip — a whole banner over a full-screen canvas, saying nothing the strip below it didn't. That's the same reasoning that already hides the bottom nav in the editor, so:
AppShell), desktop keeps it.100dvh-8rem → 100dvh-4remon mobile (no header to clear); desktop stays8rem. The canvas gets the ~48px back, plus the ~32px of dead space the old8remleft at the bottom.2. The assistant felt cramped
The assistant / journal / history render inside the rail peek, capped at
max-h-[50vh]. After the tab bar, panel header and message input, the assistant got ~200px of actual conversation. That cap is right for the inspector (you read it alongside the canvas) but wrong for a mode where reading and typing are the task.max-h-[78vh]) via atallprop onEditorRail; the inspector keeps the 50vh peek so the canvas it describes stays visible.gap-2 → gap-3.Notes
100vh → 100dvh(issue #85 item 1) was already in; this builds on it.tsc,vitest(108), andvite buildall green.Part of the #96 mobile-first pass; direct follow-up to Steve's "so much wasted space / assistant so compact" feedback.
🤖 Generated with Claude Code
https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ
🪰 Gadfly — live review status
5/5 reviewers finished · updated 2026-07-23 01:25:44Z
claude-code/sonnet· claude-code — ✅ doneglm-5.2:cloud· ollama-cloud — ✅ donekimi-k2.6:cloud· ollama-cloud — ✅ doneopencode/glm-5.2:cloud· opencode — ✅ doneopencode/kimi-k2.6:cloud· opencode — ✅ doneLive status board. Findings are posted in each model's own comment. Advisory only — does not block merge.
🪰 Gadfly consensus review — 5 inline findings on changed lines. See the consensus comment for the full ranked summary.
Advisory only — does not block merge.
@@ -46,0 +48,4 @@// above the garden, so hide it too and let the editor's own strip carry a back// affordance; desktop keeps the header. Only the editor, not the public view,// which has no strip of its own to fall back on.const hideHeaderOnMobile = inEditor🟠 Hiding the global header on mobile in the editor removes the only sign-out affordance (AccountMenu), weakening session control on shared devices
correctness, error-handling, maintainability, security · flagged by 2 models
web/src/components/layout/AppShell.tsx:51,57-61). The global header — which containsAccountMenu, the app's only "Sign out" control (AppShell.tsx:92/130-197) — is nowhidden md:blockwheneverinEditoris true. The editor's replacement mobile strip (GardenEditorPage.tsx:608-630) adds only a 🌱Link to="/gardens"and garden/season/share controls — no account or sign-out control.showBottomNavis alsofalseon th…🪰 Gadfly · advisory
@@ -58,1 +63,3 @@'flex max-h-[50vh] min-h-0 shrink-0 flex-col rounded-t-xl border-t border-border bg-surface shadow-lg',// fixed-width column beside the canvas (the cap doesn't apply there).'flex min-h-0 shrink-0 flex-col rounded-t-xl border-t border-border bg-surface shadow-lg',tall ? 'max-h-[78vh]' : 'max-h-[50vh]',🟠 Mobile peek height cap uses static vh instead of dvh, risking overflow past the visible viewport (hiding the always-visible mode bar) when panel content is tall — worsened by raising the cap from 50vh to 78vh
correctness, maintainability · flagged by 2 models
web/src/editor/EditorRail.tsx:65— The mobile peek's height cap usesvh(max-h-[50vh]/ newmax-h-[78vh]) rather thandvh, while the container it's nested inside (GardenEditorPage.tsx:553,h-[calc(100dvh-4rem)] ... md:h-[calc(100dvh-8rem)]) was deliberately switched todvhfor the exact reason documented in the adjacent comment atGardenEditorPage.tsx:544-546(#85): mobile Safari/Chrome's100vhis the largest viewport (address bar hidden), so with the bar showing, any…🪰 Gadfly · advisory
@@ -547,2 +551,3 @@// desktop the header is present, so keep the original 8rem.return (<div className="flex h-[calc(100dvh-8rem)] flex-col gap-3 md:flex-row"><div className="flex h-[calc(100dvh-4rem)] flex-col gap-3 md:h-[calc(100dvh-8rem)] md:flex-row">🟡 Mobile height subtracts 4rem but only 3rem of chrome (main py-6) exists with header hidden; leaves ~16px dead space and comment math is wrong
correctness · flagged by 2 models
web/src/pages/GardenEditorPage.tsx:553— Mobile height ish-[calc(100dvh-4rem)], but with the global header hidden on mobile (AppShell.tsxheader ishidden md:block), the only chrome outside the editor's flex column is<main>'spx-4 py-6— i.e. 3rem of vertical padding (1.5rem top + 1.5rem bottom), not 4rem. Subtracting 4rem leaves ~16px of dead space the viewport would otherwise allow. This contradicts the comment atGardenEditorPage.tsx:548-551("only<main>'s padding is…🪰 Gadfly · advisory
@@ -601,1 +606,4 @@mobile here (AppShell), so this leaf is the only route back to thegardens list — it can't be dropped. md:hidden. */}<div className="flex items-center gap-2 md:hidden"><Link🟡 Back affordance is a bare emoji with only aria-label; sr-only text would be more robust
maintainability · flagged by 1 model
web/src/components/layout/AppShell.tsx:51—hideHeaderOnMobileis a single-use alias forinEditor, with a five-line comment (lines 46–50) whose rationale substantially overlaps the file-top docblock (lines 24–30: "the editor is a full-screen context… the brand link is the way back to the gardens list"). Not a defect — the alias does aid readability of thecn(...)call — but the comment is heavier than the one-line decision needs. Trivial. -web/src/pages/GardenEditorPage.tsx:609— Co…🪰 Gadfly · advisory
@@ -673,6 +687,11 @@ export function GardenEditorPage() {tabs={railTabs}activeId={railTab}onActivate={setRailTab}// The inspector is read alongside the canvas, so it stays a half-height⚪ Duplicated 'tall' rationale comment already expressed in EditorRail prop doc
maintainability · flagged by 1 model
web/src/pages/GardenEditorPage.tsx:690(duplicated rationale) — Verified, minor maintainability nit. The same "inspector reads alongside the canvas / panel modes want room" explanation is written out twice: as thetallprop's JSDoc inEditorRail.tsx:24-27(and the inline prop doc at 50-51) and again as an inline comment at the call site inGardenEditorPage.tsx:690-694. The call-site comment adds nothing the prop doc doesn't already say; it's copy-paste that will drift if one is upd…🪰 Gadfly · advisory
🪰 Gadfly review — consensus across 5 models
Verdict: Minor issues · 8 findings (6 with multi-model agreement)
web/src/components/layout/AppShell.tsx:51web/src/editor/EditorRail.tsx:65web/src/components/layout/AppShell.tsx:28web/src/components/layout/AppShell.tsx:37web/src/pages/GardenEditorPage.tsx:553web/src/pages/GardenEditorPage.tsx:6832 single-model findings (lower confidence)
web/src/pages/GardenEditorPage.tsx:609web/src/pages/GardenEditorPage.tsx:690Per-model detail
claude-code/sonnet (claude-code) — Minor issues
Verdict: Minor issues — 5 reviewers: security, correctness, maintainability, performance, error-handling
🔒 Security — No material issues found
No new data flow, authn/authz logic, injection surface, or secret handling in this diff — it's Tailwind class/layout changes (
hidden md:blockon the header, atallheight-cap prop, agap-2→gap-3spacing tweak) plus one new<Link to="/gardens">. Verified inAppShell.tsxandGardenEditorPage.tsxthat hiding the header on mobile doesn't remove any access-control check —canEdit/isOwnergating for edit actions lives elsewhere in the page body, untouched by this diff, and the account/sign-out control simply becomes desktop-only rather than being bypassed.EditorRail/ChatPanelchanges are purely visual (height cap, gap).VERDICT: No material issues found
No findings within the security lens — this PR touches only presentational Tailwind classes, a
tallheight-cap prop, and a static in-app navigationLink; no new input handling, no auth/authz logic, no external calls.🎯 Correctness — Minor issues
VERDICT: Minor issues
web/src/editor/EditorRail.tsx:65— The mobile peek's height cap usesvh(max-h-[50vh]/ newmax-h-[78vh]) rather thandvh, while the container it's nested inside (GardenEditorPage.tsx:553,h-[calc(100dvh-4rem)] ... md:h-[calc(100dvh-8rem)]) was deliberately switched todvhfor the exact reason documented in the adjacent comment atGardenEditorPage.tsx:544-546(#85): mobile Safari/Chrome's100vhis the largest viewport (address bar hidden), so with the bar showing, anything sized offvhoverflows the actually-visible area.Confirmed the flex structure directly: the outer editor container (
GardenEditorPage.tsx:553) isflex-colwith a fixeddvh-based height and nooverflow-hidden; the canvas column (GardenEditorPage.tsx:603,flex-1 min-h-0) can shrink to accommodate siblings, but both the rail (EditorRail.tsx:64,shrink-0) and the always-visible mobile mode bar (GardenEditorPage.tsx:715,shrink-0) refuse to shrink. Because the rail's cap is computed against the largevhinstead of the container'sdvh, on a device where the address bar is showing, the rail can size itself larger than the space actually left after the fixed-height container and the shrink-0 mode bar are accounted for, pushing content (including the mode bar) past the bottom of the visible screen.This mismatch already existed at
max-h-[50vh], but raising the cap to78vh(perEditorRail.tsx:44-51,GardenEditorPage.tsx:690-694wiringtall={railTab !== 'inspector'}for journal/history/chat) increases the realistic overshoot, since panel content (a chat thread, journal entries, history list) is far more likely to actually reach a 78% cap than a 50% one.Suggested fix: express the cap in
dvh(e.g.max-h-[78dvh]/max-h-[50dvh]) to match the container it's nested in, consistent with the fix already applied one level up.🧹 Code cleanliness & maintainability — Minor issues
VERDICT: Minor issues
web/src/components/layout/AppShell.tsx:29— The docstring says "the app bottom bar hides there (the brand link is the way back to the gardens list)," but the diff addshideHeaderOnMobile = inEditor(line 51) which hides the entire header — including the brand link — on mobile in the editor (line 57-61:hidden md:block). On mobile the brand link is no longer visible in the editor; the new🌱link inGardenEditorPage.tsx:609-615is now the way back. The comment describes desktop-only behavior as if universal.web/src/components/layout/AppShell.tsx:37— The comment "the garden editor and the public shared-garden view both render a 100dvh-8rem canvas" is now false on mobile:GardenEditorPage.tsx:553changed the editor's mobile band toh-[calc(100dvh-4rem)], keeping8remonly atmd:. The two canvas routes no longer match on mobile, but the comment still claims they do.web/src/pages/GardenEditorPage.tsx:683— The comment directly above theEditorRailusage still says the mobile peek is "≤50vh," but line 694 in the same diff introducestall={railTab !== 'inspector'}, which raises the cap to 78vh (confirmed inEditorRail.tsx:65,tall ? 'max-h-[78vh]' : 'max-h-[50vh]') for every rail tab except the inspector — the common case. Notably,EditorRail.tsx's own docstring was correctly updated in this same diff to say "a capped-height panel," making this leftoverGardenEditorPage.tsxcomment the one stale outlier.⚡ Performance — No material issues found
VERDICT: No material issues found
This diff is Tailwind class changes (a
hidden md:blocktoggle,max-h-[50vh]→ conditionalmax-h-[78vh],gap-2→gap-3), atallboolean prop threaded throughEditorRail, and a new<Link>in the mobile strip. I readAppShell.tsx,EditorRail.tsx, and the surroundingGardenEditorPage.tsxcontext to confirm there's no hidden cost:hideHeaderOnMobile(AppShell.tsx:51,57-61) uses CSShidden md:block, not conditional mounting — the header's subtree (includingAccountMenu'suseLogout/useMehooks) mounts exactly as it did before this change; visibility is display-only. No new render/mount cost.tall(EditorRail.tsx:44-67) is a plain boolean prop picking between two static Tailwind classes viacn()— no computation, no re-render amplification beyond the normal prop-driven re-render already inherent to the component.Link to="/gardens"addition (GardenEditorPage.tsx:609-615) is a single static element, not inside any loop or list.gap-2 → gap-3inChatPanel.tsx:146is pure CSS.No loops, no data-fetching changes, no unbounded growth, nothing on a hot path. Nothing in my lane here.
🧯 Error handling & edge cases — No material issues found
VERDICT: No material issues found
This PR is CSS/layout-only (header visibility via
hidden md:block, amax-hcap swap, agap-2→gap-3tweak) plus one added navigation<Link>. None of it introduces new async operations, error paths, resource acquisition, or state that needs cleanup — nothing for this lens to bite into.Specifically checked and found no issues:
AppShell.tsx:57-62—hideHeaderOnMobileonly hides the header via CSS (hidden md:block); the component doesn't unmount, soAccountMenu's effects/backdrop logic (AppShell.tsx:138) keep working unaffected. Confirmed viarouter.tsxthatgardens/$gardenIdalways requires auth (requireAuthatrouter.tsx:132), so there's no edge case of an unauthenticated user losing the header's "Sign in" link on this route.EditorRail.tsx:44,51,65— newtallprop defaults tofalse, preserving prior behavior; confirmed via grep it has a single call site (GardenEditorPage.tsx:694), so no other caller is affected by the default change. Pre-existingtabs.find(...) ?? tabs[0]/if (!active) return nullguard is untouched.GardenEditorPage.tsx:608-615— new back<Link to="/gardens">renders only after the existing loading/gone-state guards higher in the component (verifiedgardenGone/loading handling precedes this JSX), sogarden.namebeing dereferenced nearby is not a new null-access risk.GardenEditorPage.tsx:553— height calc changes only the subtracted constant (4remvs8rem); this reduces (not increases) the risk of a negative/degenerate height versus the prior8remon mobile.ChatPanel.tsx:146— spacing-only change, no logic touched.glm-5.2:cloud (ollama-cloud) — Minor issues
Verdict: Minor issues — 5 reviewers: security, correctness, maintainability, performance, error-handling
🔒 Security — Minor issues
Confirmed. The global header containing
AccountMenu(the only sign-out control,AppShell.tsx:92/130-197) is nowhidden md:blockwheneverhideHeaderOnMobile(inEditor,AppShell.tsx:51,57-61) is true;showBottomNavisfalseon the editor route (canvasRoutetrue,AppShell.tsx:45,121), so the bottom nav isn't a fallback either. The editor's mobile strip (GardenEditorPage.tsx:608-630) adds only the 🌱Link to="/gardens", garden name, view-only badge, season picker, and share button — no account/sign-out control. The only sign-out affordance lives inAccountMenuin the hidden header. Finding survives.VERDICT: Minor issues
web/src/components/layout/AppShell.tsx:51,57-61). The global header — which containsAccountMenu, the app's only "Sign out" control (AppShell.tsx:92/130-197) — is nowhidden md:blockwheneverinEditoris true. The editor's replacement mobile strip (GardenEditorPage.tsx:608-630) adds only a 🌱Link to="/gardens"and garden/season/share controls — no account or sign-out control.showBottomNavis alsofalseon the editor route (canvasRoute,AppShell.tsx:45,121), so the bottom nav (which would otherwise carry account access) isn't there either. Net effect: an authenticated user on a phone inside the garden editor has no visible UI to end their session. On a shared/loaned device this weakens session control — the user can't sign out before handing the phone back and must know to navigate away to find the control. Concrete impact: degraded ability to terminate an authenticated session; not an authn/authz bypass. Suggested fix: surface a compact account/sign-out control in the editor's mobile strip (or makeAccountMenuavailable via the editor), rather than only in the hidden header.🎯 Correctness — Minor issues
Verdict: Minor issues
web/src/pages/GardenEditorPage.tsx:553— Mobile height ish-[calc(100dvh-4rem)], but with the global header hidden on mobile (AppShell.tsxheader ishidden md:block), the only chrome outside the editor's flex column is<main>'spx-4 py-6— i.e. 3rem of vertical padding (1.5rem top + 1.5rem bottom), not 4rem. Subtracting 4rem leaves ~16px of dead space the viewport would otherwise allow. This contradicts the comment atGardenEditorPage.tsx:548-551("only<main>'s padding is above us — 4rem"). Impact is small: the canvas still renders correctly and reclaims most of the header room; it just leaves 16px on the table and the numerical justification is wrong. Fix: either useh-[calc(100dvh-3rem)]on mobile, or correct the comment to state the 1rem is an intentional buffer.web/src/editor/EditorRail.tsx:65— Thetallpeek cap usesmax-h-[78vh]while the editor column is sized indvh(100dvh-4rem). On mobile browsers with the URL bar visible100dvh < 100vh, so a content-filled 78vh rail plus the ~5rem of strip + mode bar can approach or exceed thedvh-based column height. This is the samevh-vs-dvhclass of issue (#85) the editor already corrected for its own height. Not certain to overflow in practice (depends on rail content and exact mode-bar height), but the unit inconsistency is real. Fix:max-h-[78dvh]to match the column's unit basis.🧹 Code cleanliness & maintainability — Minor issues
Verdict: Minor issues found
The change is clean and works as intended. A few maintainability nits survive verification:
web/src/components/layout/AppShell.tsx:51—hideHeaderOnMobileis a single-use alias forinEditor, with a five-line comment (lines 46–50) whose rationale substantially overlaps the file-top docblock (lines 24–30: "the editor is a full-screen context… the brand link is the way back to the gardens list"). Not a defect — the alias does aid readability of thecn(...)call — but the comment is heavier than the one-line decision needs. Trivial.web/src/pages/GardenEditorPage.tsx:609— Confirmed: the back affordance is a bare🌱emoji in aLinkwitharia-label="All gardens"and no visible/sr-onlytext, while the header brand it replaces (AppShell.tsx:67) renders🌱 pansywith the wordmark. With the header hidden on mobile here, this leaf is the only brand/back affordance. Reliance on an emoji glyph plusaria-labelalone is fragile across screen readers / platform glyph rendering; adding<span className="sr-only">All gardens</span>would be more robust. Minor.web/src/editor/EditorRail.tsx:65— Confirmed: thetall ? 'max-h-[78vh]' : 'max-h-[50vh]'ternary uses magic viewport percentages, and the "reading and typing are the task" rationale appears verbatim-ish in both theEditorRaildocblock (line 26) and the call site comment (GardenEditorPage.tsx:692). Two copies of the same "why" will drift. Trivial.⚡ Performance — No material issues found
No material issues found.
Through the performance lens, the change is purely CSS class / prop plumbing — no new allocations, queries, loops, or blocking work on any path. I verified:
EditorRail.tsx:64-66— thetalltoggle just swaps one Tailwindmax-hclass string viacn; no runtime cost beyond the class lookup.tall={railTab !== 'inspector'}(GardenEditorPage.tsx:694) is a cheap boolean prop recomputed on render — negligible.AppShell.tsx— thehidden md:blockheader is a pure display toggle; the header subtree still mounts, but that was already the case (it was always rendered), so no render-cost regression.GardenEditorPage.tsx— theLinkand🌱glyph are static, render once; no list work, no effect added.ChatPanel.tsx:146—gap-2 → gap-3is a layout spacing change, not a loop/allocation change.No N+1, no unbounded growth, no hot-path regression introduced.
🧯 Error handling & edge cases — No material issues found
Through the error-handling & edge-cases lens, I verified the unhappy paths this diff touches:
EditorRailempty/degenerate input (EditorRail.tsx:53-54):active = tabs.find(...) ?? tabs[0]; iftabsis empty,tabs[0]isundefinedand theif (!active) return nullguard handles it — no panic. Verified by reading the file.tall={railTab !== 'inspector'}(GardenEditorPage.tsx:694):EditorRailis only rendered inside{railTab && (...)}(line 685), sorailTabis guaranteed truthy here. Thetallprop has a= falsedefault and is always a boolean. No null/undefined edge case. Verified.hideHeaderOnMobile = inEditor(AppShell.tsx:51): boolean,!!-coerced frommatchRoute. Thecn(..., hideHeaderOnMobile && 'hidden md:block')safely yieldsfalse(no class) when off. No bad-input path.Link to="/gardens"(GardenEditorPage.tsx:609):/gardensis an existing route (used inAppShell.tsx:66), so no resolution failure; the link is rendered unconditionally inside the mobile strip, which is fine for both owner and view-only viewers (verified the strip has no auth-gating around the link).hidden, bottom nav already suppressed for canvas routes atAppShell.tsx:45): this is a UX consideration, not an unhandled error — the 🌱 link returns to/gardenswhere the header reappears. Not an error-handling finding.No swallowed errors, missing
defer/cleanup, panics on bad input, or unhandled nil/empty/boundary conditions are introduced by this change.Verdict: No material issues found.
kimi-k2.6:cloud (ollama-cloud) — No material issues found
Verdict: No material issues found — 5 reviewers: security, correctness, maintainability, performance, error-handling
🔒 Security — No material issues found
No material issues found
This is a purely presentational layout change. From a security lens:
requireAuthinrouter.tsx:132; the header is only visually hidden with CSS (hidden md:block), not removed from an authenticated shell.garden.name, was already present before this diff and is safely interpolated by React inside a<h1>. The new<Link to="/gardens">has a hardcoded path andaria-label.tallprop andmax-h-[78vh]cap are driven by a hardcoded local store value (railTab), not by user input or URL parameters.display: none, not rendered in the public view (the public garden route/g/$tokenexplicitly keeps its header).🎯 Correctness — No material issues found
No material issues found
I verified the logic carefully:
AppShell.tsx:hideHeaderOnMobileis scoped toinEditoronly, intentionally preserving the global header for the public garden view which has no back affordance of its own. Thehidden md:blockbreakpoint correctly hides the header on mobile while keeping it on desktop.GardenEditorPage.tsx: TheLinkimport from@tanstack/react-routeris present (diff confirmed). The back affordance (🌱→/gardens) replaces the hidden brand link exactly where needed. The height switch100dvh-4rem(mobile) /100dvh-8rem(desktop) correctly pairs with the header visibility change.EditorRail.tsx: Thetallprop is typedbooleanwith a sensible default. Themax-h-[78vh]/max-h-[50vh]cap is overridden on desktop by the existingmd:max-h-none— no breakpoint conflict.ChatPanel.tsx: The spacing change (gap-2 → gap-3) is a pure layout tweak with no semantic impact.tall={railTab !== 'inspector'}expression correctly maps to the PR's intent: inspector stays a peek, while journal/history/assistant expand.All domain logic, conditional scoping, and arithmetic expressions align with the stated intent and with each other.
🧹 Code cleanliness & maintainability — No material issues found
No material issues found.
The change is clean and consistent with existing patterns:
tallprop onEditorRailis a focused, well-documented abstraction with a sensible default (false), so existing behavior is preserved for any future callers. OnlyGardenEditorPage.tsx:694passes it, and therailTab !== 'inspector'logic is clearly explained inline.hideHeaderOnMobilevariable inAppShell.tsx:51follows the same naming and conditional-class pattern already used forshowBottomNavtwo lines above it.🌱) in the mobile top strip (GardenEditorPage.tsx:609-614) reuses the existingLinkcomponent andtext-accent-strongstyling from the global header, avoiding visual drift.4rem/8rem,50vh/78vh) are paired with comments that explain why they differ by breakpoint, so the magic numbers are justified.PublicGardenPage.tsxcorrectly keepscalc(100dvh-8rem)because it doesn't hide the global header.⚡ Performance — No material issues found
No material issues found
This PR is purely presentational—CSS breakpoint tweaks, conditional header hiding, and a rail height prop. Nothing in my lane:
useAgentHistoryandstreamChatwere already there.gap-2 → gap-3inChatPanelis static className churn; it doesn't recompute per-message.max-h-[78vh]is a strict cap, higher than the old50vhbut still bounded by viewport.Linkandtallprop are trivial React prop passes.calc(100dvh-4rem)height changes once on mount/resize and once per tab switch; it doesn't fire in a loop.🧯 Error handling & edge cases — No material issues found
No material issues found
tallprop correctly falls back tofalsewhenrailTab === 'inspector', and the{railTab && ...}guard ensures it's only evaluated whenrailTabis truthy, sotallis never computed fornull.EditorRailhandles the empty-tabs edge case gracefully (if (!active) return null).Linkhas an explicitaria-labeland points to an existing authenticated route.PublicGardenPagecorrectly retains100dvh-8remsincehideHeaderOnMobileintentionally excludes it (the PR comment and code agree).activeIdnot matching any tab falls back totabs[0]— existing behavior, not newly introduced.opencode/glm-5.2:cloud (opencode) — Minor issues
Verdict: Minor issues — 5 reviewers: security, correctness, maintainability, performance, error-handling
🔒 Security — No material issues found
Verdict: No material issues found
Reviewed through the 🔒 Security lens.
/gardens— the/gardensroute is auth-protected viabeforeLoad: requireAuth(web/src/router.tsx:120), so the link is not a way to bypass auth; it's guarded like any other authed navigation. Verified by readingrouter.tsx:119-120.requireAuth) still runs inbeforeLoadregardless of whether the header renders, so hiding it on mobile doesn't weaken auth enforcement. Verified inAppShell.tsx:51androuter.tsx:132(the editor route'sbeforeLoad).to="/gardens"rendered via TanStackLink, not a rawhref), so no path/open-redirect surface is added.Nothing in my lens is materially wrong with this change.
🎯 Correctness — Minor issues
I'll verify the findings against the actual code.
Both findings verified against the actual code.
AccountMenu(the only sign-out affordance — grep confirms it's referenced solely atAppShell.tsx:92, inside the header) is hidden on mobile in the editor (hideHeaderOnMobile && 'hidden md:block', line 60). The bottom nav is also hidden in the editor (showBottomNav = !!user && !canvasRoute, line 45, withcanvasRoute = inEditor || inPublicGarden). The new 🌱 leaf (GardenEditorPage.tsx:609-615) links only to/gardens, not to account/settings. From/gardensthe header and bottom nav reappear, so those destinations are reachable via an extra hop — a real, if recoverable, UX regression.GardenEditorPage.tsx:549-551claims "only<main>'s padding is above us — 4rem," but<main>usespy-6(AppShell.tsx:106) = 1.5rem top + 1.5rem bottom = 3rem total, not 4rem. With the header at zero height on mobile, actual vertical chrome is 3rem;100dvh-4remleaves ~1rem of slack. No overflow (the editor is shorter than the content box), but the stated reasoning is arithmetically wrong.Verdict: Minor issues found
web/src/components/layout/AppShell.tsx:51/web/src/pages/GardenEditorPage.tsx:609— account / global nav requires an extra hop from the mobile editor. The hidden header carriedAccountMenu(the only sign-out affordance — grep confirms it exists solely inAppShell's header at line 92) plus the desktop section links. On mobile in the editor the bottom nav is also hidden (showBottomNav = !!user && !canvasRoute), so the editor has no in-context access to sign-out, Settings, or Plants. The new🌱 → /gardensleaf only returns to the gardens list; from there the header and bottom nav reappear, so the destinations are recoverable with one extra hop rather than truly lost. The PR's framing of the header as "pure chrome" understates what it carried. Suggested fix: fold an account affordance into the mobile editor strip the way the back leaf was.web/src/pages/GardenEditorPage.tsx:553— the comment's height justification doesn't match the value. The comment states "only<main>'s padding is above us — 4rem," but<main>usespy-6(AppShell.tsx:106) = 1.5rem + 1.5rem = 3rem total, not 4rem. With the headerdisplay:noneon mobile (zero height), the real chrome above+below is 3rem, so100dvh-4remleaves ~1rem of unexplained slack. No overflow results (the editor is shorter than main's content box), but the stated reasoning is arithmetically wrong — a future maintainer "correcting" 4rem to 3rem would push the mode bar flush against main's bottom padding. Suggested fix: reconcile the comment to the value (note the deliberate 1rem breathing room) or set the value to match the reasoning.🧹 Code cleanliness & maintainability — Minor issues
I'll verify each finding against the actual code.
All four findings verified against the actual code. Confirming each:
AppShell.tsx:28 — Line 28-29 docstring says "the app bottom bar hides there (the brand link is the way back to the gardens list)". PR hides the brand header on mobile in the editor (
hideHeaderOnMobile = inEditor,hidden md:blockat line 60), and the new 🌱 Link at GardenEditorPage.tsx:609-615 is the back affordance. So the rationale "the brand link is the way back" is now false on mobile. Confirmed.AppShell.tsx:37 — Line 36-37 comment says "both render a 100dvh-8rem canvas". Editor now renders
100dvh-4remon mobile (GardenEditorPage.tsx:553). Confirmed stale.GardenEditorPage.tsx:683 — Line 682-684 comment says "in-flow PEEK (≤50vh)". With
tall, journal/history/assistant now peek at ≤78vh (EditorRail.tsx:65). Confirmed stale.GardenEditorPage.tsx:690 — Lines 690-694 comment duplicates the rationale already in EditorRail.tsx:24-27 and the prop doc at 50-51. Confirmed as a trivial maintainability nit.
Verdict: Minor issues
web/src/components/layout/AppShell.tsx:28(stale class docstring) — Verified. The component's JSDoc says the editor hides the bottom bar because "the brand link is the way back to the gardens list". This PR hides the brand header itself on mobile in the editor (hideHeaderOnMobile = inEditor→hidden md:blockat line 60), and the new 🌱 leafLinkinGardenEditorPage(lines 609-615) is now the back affordance, per the new comment atGardenEditorPage.tsx:605-607. So the docstring's stated rationale — "the brand link is the way back" — is now false on mobile. CLAUDE.md: "stale comments are worse than none — the next reader believes them." Suggested fix: update the sentence to reflect that the editor's own strip carries the back affordance on mobile (or drop the parenthetical, since it no longer explains the hide).web/src/components/layout/AppShell.tsx:37(stale inline comment) — Verified. The comment says "the garden editor and the public shared-garden view both render a100dvh-8remcanvas." After this PR the editor renders100dvh-4remon mobile (GardenEditorPage.tsx:553), so the "both render a 100dvh-8rem canvas" statement is now only true on desktop / for the public view. Low severity but the comment was written as a general invariant and is now misleading. Suggested fix: qualify it ("...both render a near-full-height canvas; the editor's exact band differs by breakpoint — see GardenEditorPage").web/src/pages/GardenEditorPage.tsx:683(stale inline comment above the rail) — Verified. The comment still says "an in-flow PEEK (≤50vh)". Withtall, journal/history/assistant now peek at ≤78vh (EditorRail.tsx:65). The ≤50vh is now only the inspector case. Minor, but it's the kind of "looks authoritative, is half-wrong" comment CLAUDE.md warns about. Suggested fix: "an in-flow PEEK (≤50vh for the inspector, ≤78vh for panel modes — see EditorRail)".web/src/pages/GardenEditorPage.tsx:690(duplicated rationale) — Verified, minor maintainability nit. The same "inspector reads alongside the canvas / panel modes want room" explanation is written out twice: as thetallprop's JSDoc inEditorRail.tsx:24-27(and the inline prop doc at 50-51) and again as an inline comment at the call site inGardenEditorPage.tsx:690-694. The call-site comment adds nothing the prop doc doesn't already say; it's copy-paste that will drift if one is updated and not the other. Suggested fix: trim the call-site comment to// Inspector stays a short peek; panels take the tall slice.and letEditorRail's prop doc carry the "why".No dead code, no naming problems, no leaky abstractions, no copy-pasted logic. The
tallprop is a clean single-purpose boolean and the conditionaltall ? 'max-h-[78vh]' : 'max-h-[50vh]'reads clearly. ThehideHeaderOnMobileflag and the emoji-leaf back Link are reasonable. The duplicated rationale text and the three stale comments are the only cleanliness concerns.⚡ Performance — No material issues found
No material issues found
Through the performance lens this is a CSS/className and layout-only change — no queries, no new allocations, no work added to hot paths or render loops. The new
tallprop is a boolean computed once per render (railTab !== 'inspector'), theLinkis a single static element, and thegap-2 → gap-3andmax-hswaps are pure style strings. Nothing here regresses efficiency.🧯 Error handling & edge cases — Minor issues
Verdict: Minor issues
AppShell.tsx:51,60now hides the global header on mobile in the editor (hideHeaderOnMobile = inEditor, applied ashidden md:block), andshowBottomNavis already false for canvas routes (AppShell.tsx:45). The header'sAccountMenu(AppShell.tsx:130-204) is the only sign-out surface — grep acrossweb/srcconfirms no otheruseLogout/sign-out path exists in the UI. ItsonLogoutdeliberately stays open on failure so the "Retry sign out" button (driven bylogout.isError) remains visible (AppShell.tsx:140-149), the code comment explicitly calling it "the only retry affordance." On mobile in the editor that affordance is now gone until the user taps the new 🌱 link (GardenEditorPage.tsx:607) to leave for/gardens, where the header/bottom nav reappear. Recoverable, but a real edge-case regression: an error path the codebase engineered to keep reachable is unreachable on this route. Suggested fix: surface a compact account/sign-out entry on the editor's mobile strip (next to the 🌱 back link), or keep a slim account control visible when the header is hidden.opencode/kimi-k2.6:cloud (opencode) — No material issues found
Verdict: No material issues found · ⚠️ 1/5 lens(es) errored — 5 reviewers: security, correctness, maintainability, performance, error-handling
🔒 Security — No material issues found
No material issues found.
This PR is a pure frontend styling/layout change (header visibility, height calculations, rail peek height, chat spacing). There are no authn/authz changes, no new data inputs, no injection surfaces, no secret handling, no route parameter handling, and no cross-origin or trust-boundary crossings. The
Linkadded inGardenEditorPage.tsxis a hardcoded client-side navigation to/gardenswith no query parameter construction or URL interpolation. TheAccountMenubeing hidden on mobile in the editor is a UX decision, not an authorization gap — the service layer still enforces permissions perCLAUDE.md.🎯 Correctness — ⚠️ could not complete
⚠️ This reviewer failed to complete: opencode run context deadline exceeded
🧹 Code cleanliness & maintainability — No material issues found
No material issues found.
Wait — let me re-check one thing. The comment in
GardenEditorPage.tsxat line 683 is part of the diff context and says "≤50vh", but with thetallprop the rail can now be 78vh. That's a stale comment in changed code.Actually, I'll verify: is that comment in the hunks or just context? Looking at the diff:
This is context lines (not prefixed with
+or-), so it's unchanged by the PR but adjacent to the change. However, it's now factually stale because the rail is no longer always ≤50vh. Since my lens is maintainability and stale comments are worse than none, I'll report it.Minor issues
web/src/pages/GardenEditorPage.tsx:683— The comment describing the rail as an "in-flow PEEK (≤50vh)" is now stale: with thetall={railTab !== 'inspector'}prop, panel modes (journal/history/assistant) render at 78vh. Future readers relying on this comment will be misled about the actual height behavior.Fix: Update the comment to reflect the new conditional height, e.g.
a capped-height panel (inspector ≤50vh, panels ≤78vh).That's the only finding in my lane. The
tallprop is cleanly added, theLinkimport is properly placed, thehideHeaderOnMobilelogic mirrors the existingshowBottomNavpattern, and the Tailwind class composition is consistent with the codebase.⚡ Performance — No material issues found
No material issues found
This PR is purely CSS/layout surface changes. Through the Performance lens:
EditorRail.tsx:65— The newtallprop toggles betweenmax-h-[78vh]andmax-h-[50vh]. This is static class-name switching; it does not introduce extra re-renders beyond what already happens on tab change (activeIdalready mutates), nor does it trigger layout thrashing.ChatPanel.tsx:146—gap-2 → gap-3is a spacing token change with no runtime cost.GardenEditorPage.tsx:553/609— The height calc change (4remvs8rem) and the addedLinkare layout-level only; no new data fetching, no effect hooks, and no hot-path work.🧯 Error handling & edge cases — No material issues found
No material issues found
I examined the diff carefully through the error-handling and edge-cases lens:
EditorRailalready safely handles an emptytabsarray (tabs[0]isundefined, caught by theif (!active) return nullguard). No new nil-dereference paths are introduced.tallprop correctly defaults tofalse, preserving the existing50vhcap for any caller that omits it. The78vhcap is relative to the viewport and the parent flex container hasmin-h-0on children, so there’s no risk of negative-height overflow or CSS clamp bugs. The100dvh-4rem/100dvh-8remheights correctly account for the header presence/absence, and the child fits inside<main>’s padded content box with room to spare (verified by re-deriving the box model).nudgeTimer/nudgeFirecleanup is untouched.Linkuses a staticto="/gardens"route that exists elsewhere in the app; no dynamic input is passed.The landscape-shrunk canvas is an intentional design trade-off documented in the PR (“the canvas above just needs to stay glanceable”), not an unhandled error.
Automated adversarial review by Gadfly — consensus across the model swarm. Advisory only — does not block merge.