Skip to content

Make plant details reachable without hover and mark a failed poll stale - #118

Merged
dmccoystephenson merged 5 commits into
mainfrom
fix/plant-detail-and-stale-poll
Aug 14, 2026
Merged

Make plant details reachable without hover and mark a failed poll stale#118
dmccoystephenson merged 5 commits into
mainfrom
fix/plant-detail-and-stale-poll

Conversation

@dmccoystephenson

@dmccoystephenson dmccoystephenson commented Aug 12, 2026

Copy link
Copy Markdown
Member

Summary

Two dashboard defects are addressed, both concerning facts the page holds
and does not surface.

A plant's detail is reachable without a hover (#114). Every per-repo
fact except the leaf name and a tend/error tally used to live in a title
attribute — a hover tooltip, on a stylesheet written phone-first, in the
view the page defaults to. Each plot cell is now a <button> that opens a
detail card beneath the plot: full owner/name, health, tends out of total
runs, errors, last successful tend, last attempt and its outcome, cost,
merge eligibility, and whether the repo is actually planted. The title is
kept unchanged for pointer users, since it costs nothing. last_run and
last_outcome were emitted by build_garden_rows and rendered by no view
at all; they are on the card, because "the most recent attempt errored" is a
different fact from "the last success was three days ago", and the more
urgent one.

The garden tablist implements what it declares (#116). aria-controls
is added to each tab, role="tabpanel" and an aria-labelledby
back-reference to each panel, and a roving tabindex with
Left/Right/Home/End handling is wired into the tab buttons. setTabState
sets tabIndex from the same selected flag it sets aria-selected from,
so the two cannot disagree.

A failed poll no longer renders as a healthy dashboard (#116). Every
panel keeps showing the last good snapshot after a failed poll, so changing
one header caption was not enough of a signal. res.ok is now checked
before the body is parsed — a 500 was previously detected only by way of
res.json() throwing on its error body, and a 2xx with a bad body was
indistinguishable from a dead server. Either failure marks
<body class="stale">: the content below the header is dimmed and
desaturated, and the heartbeat is replaced by the age of the displayed
snapshot (built from the payload's own generated_at, at
seconds/minutes granularity rather than fmtAge's day buckets) plus the
reason and a consecutive-failure count. The first successful poll clears
all of it; the existing visibilitychange fast path is untouched.

dashboard.py's module docstring and docs/DASHBOARD.md are updated to
match.

Test plan

  • python3 -m compileall -q gardener tests
  • python3 -m unittest discover -s tests -v — 645 tests, all passing
    (14 new TestPageHtmlInvariants cases covering the button markup, the
    two newly-rendered row fields, the delegated listeners, the detail
    card's change-only rewrite, focus surviving a plot rebuild, the
    tablist wiring, the roving tabindex, the staleness class, the
    res.ok check, each of the four poll-failure reasons, the
    non-overlapping poll guard, the age string in the plot signature, and
    the age-based caption)
  • PAGE_HTML parsed with html.parser to confirm the markup is
    balanced after the tab/panel/detail-card edits, and build_status
    re-checked against a temporary state dir
  • The new code's assumptions checked against the rest of the file in a
    second review pass: .plant's existing background/border/
    text-align correctly override the UA button defaults, generated_at
    is offset-aware so fmtSince's Date.parse is unambiguous,
    body.stale main and body.stale #updated both have real targets
    with #updated outside <main> as intended, and every row field and
    CSS class the detail card references exists.
  • Visual render check not performed. CLAUDE.md records that
    anything touching the plot is verified by rendering it against the
    real state db and looking at it, since stdlib-only Python leaves no JS
    test runner. This dispatch ran headlessly with no browser and no JS
    runtime available, so the in-page JavaScript here has never been
    executed — only its emitted source text was asserted against. Because
    that code shares one <script> block with the whole page, a syntax
    error in it would blank every panel rather than just the new card,
    which the Python-side assertions cannot detect. Review should include
    opening the dashboard and confirming: a plant taps open and closed,
    the detail card shows the right repo, arrow keys move between the Plot
    and Table tabs, and stopping the server dims the page with an aging
    caption.

dispatch.py, dev_loop.py and the prompt templates are untouched, so the
manual dispatch verification those require does not apply to this change.

Closes #114
Closes #116

This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

dmccoystephenson and others added 3 commits August 12, 2026 04:30
Draw each plot cell as a button opening a detail card under the plot,
carrying the facts that previously lived only in a title tooltip plus
last_run/last_outcome, which no view rendered. Complete the garden
tablist with aria-controls, tabpanel back-references and a roving
tabindex with arrow-key navigation. Check res.ok before parsing the
poll's body, and mark the whole page stale with the snapshot's age
when a poll fails.

Closes #114
Closes #116

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Rewrite the detail card only when its content changed, restore focus to
a plant after the plot is rebuilt, drop the aria-live region that would
have re-announced the card on every poll, look tab keys up in a Map
rather than an object literal, and mark the snapshot fresh only once
every panel has actually rendered it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Name a bad body and a throwing render as their own staleness reasons
rather than folding both into "fetch failed", guard against overlapping
polls re-marking a live page stale, restore focus to the detail card's
close button when the card's own content changes, restore plant focus
without scrolling the reader back to the plot, put the rendered age in
the plot signature so a plant's accessible name cannot freeze, and
desaturate rather than dim the stale page so the last known data stays
readable.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dmccoystephenson

Copy link
Copy Markdown
Member Author

Review

The diff was reviewed inline this session (no Agent tool is available to a
headless dispatch, and neither gh pr review nor the reviews API is on this
dispatch's allow-list, so the review is posted here as a comment). Seven
findings were raised and all seven were addressed in
4361679; the notes below record what each was and what was done, since the
before-state is no longer visible in the current diff.

  • gardener/dashboard.py:1082the detail card's focus protection only
    covered the unchanged case.
    The card was memoised so an unchanged
    render doesn't touch innerHTML, but the card's content genuinely does
    change on its own: in_flight flips when a tend starts or ends, and the
    counts and cost move when one finishes. A keyboard user sitting on the ✕
    close button for the repo currently being tended would have lost focus to
    <body> at exactly that moment. Fixed by capturing
    el.contains(document.activeElement) before the rewrite and restoring the
    close button after it.

  • gardener/dashboard.py:1036focusPlant scrolled the reader back to
    the plot.
    Restoring focus after a poll-driven plot rebuild called
    el.focus() with no preventScroll, so a reader who had Tabbed onto a
    plant and then scrolled down to the log tail would have been yanked back
    up to the garden panel — repeatedly, since the adjacent comment notes the
    rebuild happens often during a run. focusPlant now takes a
    preventScroll flag, set for the rebuild path.

  • gardener/dashboard.py:1212a throwing render left the page in
    neither state.
    markFresh had been moved to the end of refresh so the
    heartbeat is only claimed once the panels have actually rendered, but only
    the fetch and parse were inside a try. A payload that threw part-way
    through rendering would have escaped as an unhandled rejection, leaving
    the caption frozen at the last good time with no staleness class — the
    precise "looks live but isn't" state this PR exists to rule out. The
    render is now its own guarded step with a render failed reason.

  • gardener/dashboard.py:1237the docs claimed a distinction the code
    didn't make.
    Both this PR's prose and the module docstring stated that a
    2xx with an unparseable body is no longer indistinguishable from a dead
    server, but a res.json() failure landed in the same catch as a network
    failure and produced the identical fetch failed caption. A bad body now
    reports itself as bad response body, and docs/DASHBOARD.md tabulates
    all four failure reasons.

  • gardener/dashboard.py:1218overlapping polls could re-mark a live
    page stale.
    Both setInterval and the visibilitychange listener start
    polls, and nothing prevented a second from starting while the first was
    outstanding. Against a restarting server, a slow doomed request could
    resolve after a later successful one and dim a page that had just come
    back. A pollInFlight guard now drops overlapping polls, which costs
    nothing given the payload is a whole snapshot.

  • gardener/dashboard.py:1004a plant's accessible name could freeze.
    The plot re-renders only when its signature changes, and the signature
    carried healthOf(r) — 2/5/10-day buckets — but not the rendered age
    string. A dashboard left open overnight would show a plant still reading
    "just now" hours later while the table view, re-rendered unconditionally,
    read "3h ago" for the same repo. Pre-existing for the tooltip, but this
    diff promotes that same string into the button's aria-label, so it is
    now the accessible name going stale. fmtAge(daysSince(r.last_success))
    is now part of the signature.

  • gardener/dashboard.py:512the staleness treatment hid the data it
    was labelling.
    opacity: 0.42 was applied to the whole content area,
    including text already coloured var(--muted), which on the dark theme
    falls below readable contrast. When the server has died mid-overnight
    that stale snapshot is the only data there is. The signal is now carried
    by grayscale(1) — all the colour on the page drains at once, which is
    unmissable — with the opacity relaxed to 0.78.

Conventions checked and clean. Stdlib-only (no dependency added; the
change is CSS/JS inside PAGE_HTML plus its module docstring).
dispatch.py, dev_loop.py, MODE_SPECS and every prompt template are
untouched, so no claude invocation flag was added outside
_build_invocation and no path to bypassPermissions was opened or
altered. align_repo.md.tmpl is unchanged and still string.Template-substituted.
No convention rule was hardcoded into a prompt. docs/DASHBOARD.md's
plant/data mapping table still matches plantSvg, which was not modified,
and the doc gained the detail-card and staleness sections the new behaviour
requires.

One gap this review cannot close. CLAUDE.md records that anything
touching the plot is verified by rendering it against the real state db and
looking at it, because stdlib-only Python leaves no JS test runner here.
This dispatch was headless with no browser and no JS runtime available, so
the in-page JavaScript in this PR has never been executed — the eight new
TestPageHtmlInvariants cases assert its emitted source text, and
PAGE_HTML was parsed with html.parser to confirm the markup is balanced,
but neither is a substitute. A human should open the dashboard and confirm:
a plant taps open and closed and shows the right repo, arrow keys move
between the Plot and Table tabs, and stopping the server desaturates the
page with an aging caption.

This review was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

dmccoystephenson and others added 2 commits August 12, 2026 04:45
Describe TestPageHtmlInvariants in TESTING.md, retitle DASHBOARD.md now
that it covers more than the garden view, point README at both halves,
and add the reason-string check to CLAUDE.md's doc source-of-truth row.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dmccoystephenson

Copy link
Copy Markdown
Member Author

Review — second pass (continuing an interrupted dispatch)

This PR was found open and carrying gardener's own dispatch marker, meaning
the previous tend run was killed before it could report back. The branch
was resumed rather than duplicated. Local HEAD was confirmed identical to
the PR head before any work began, and the working tree was clean, so no
partial edits were lost to the interruption.

The review already posted above covers the diff up to 4361679. Commit
421c727 (docs) landed after it and had never been reviewed; it is
covered here, along with an independent re-check of the code.

Verification performed this session

  • python3 -m unittest discover -s tests — 645 tests, all passing.
  • gh pr checks 118test (3.10) and test (3.x) both green;
    mergeable: MERGEABLE, mergeStateStatus: CLEAN.
  • The load-bearing assumptions of the new code were checked against the
    rest of the file rather than taken on trust. All held:
    • gardener/dashboard.py:547 — promoting a plant from <div> to
      <button> inherits the UA's ButtonFace background, default border
      and centred text. .plant's existing background: shorthand resets
      background-color to transparent, border: 1px solid transparent
      overrides the UA border, and text-align: center was already set, so
      the plant renders unchanged on both themes. button.plant correctly
      only needs font/color: inherit.
    • gardener/dashboard.py:1244fmtSince computes an elapsed time via
      Date.parse. generated_at comes from state.now_iso(), which is
      datetime.now(timezone.utc).isoformat() and therefore offset-aware, so
      the parse is unambiguous. Had it been a naive timestamp, every stale
      caption would have been wrong by the viewer's UTC offset.
    • gardener/dashboard.py:512body.stale main and
      body.stale #updated both have real targets: <main> wraps the panels
      at line 673, and #updated sits at line 671, outside it. The stated
      intent (content desaturates, header keeps full contrast because it
      carries the explanation) is what the selectors actually do. --warn is
      defined in both the dark and light palettes.
    • gardener/dashboard.py:1077 — every field the detail card reads
      (runs, successes, errors, last_run, last_success,
      last_outcome, cost_usd, can_merge, in_garden, in_flight) is
      emitted by build_garden_rows, and every class it applies
      (outcome-error, outcome-tend, muted) is defined. .muted is a
      top-level rule, not one scoped inside the phone media block. The
      Errors cell's conditional styling matches the table's own at line 974.
    • gardener/dashboard.py:1317 — the document.hidden poll gate and the
      visibilitychange fast path are both intact, as the description
      claims; the pollInFlight guard was added around them, not in place
      of them.
    • The tab keydown arithmetic was traced for all four keys at both
      indices; ArrowLeft at the first tab and ArrowRight at the last both
      wrap correctly through the modulo.

On 421c727

The docs commit is accurate. docs/DASHBOARD.md's new failure table lists
exactly the four reason strings refresh passes to markStale, and the
new CLAUDE.md row makes that correspondence a maintained invariant rather
than a coincidence — which is the right shape, given the reasons are
otherwise only asserted as loose substrings. Retitling the doc to
"The dashboard" is warranted now that it covers more than the garden view,
and the README link text was updated to match rather than left stale.
docs/TESTING.md's description of TestPageHtmlInvariants matches what
those cases assert, including the honest note that anything about how the
plot looks is still verified by looking at it.

Findings

  • docs/DASHBOARD.md:105 — a trailing blank line was left at end of file.
    Fixed in cb9e6a4 so the branch is handed over clean.
  • tests/test_dashboard.py:659 — nit, no change requested. The tablist
    test asserts the four key names as bare substrings, so "Home" and
    "End" would still pass if the keydown handler were deleted and those
    strings survived anywhere else in PAGE_HTML. The narrower quoted forms
    used by the sibling assertions would be stronger. This matches the
    deliberately-narrow style the class docstring describes, so it is noted
    rather than raised as a defect.

No correctness defect was found in the JavaScript, the CSS, or the docs.

Why merging is being left to a human

This is not a reservation about the code. It is that the repo's own
CLAUDE.md states the dashboard's in-page JavaScript has no test runner
here and is verified by rendering it against the real state db and looking
at it. This PR is roughly two hundred lines of that JavaScript, and it has
never been executed — no browser and no JS runtime (node, deno, bun,
qjs were all checked for) exists in this dispatch, so the same gap the
first pass declared could not be closed by this one either.

The reason this matters more than a normal untested-change caveat: the new
code shares a single <script> block with the whole page. A syntax error
in it would not degrade the new detail card — it would blank every panel on
the dashboard, and the Python-side assertions could not detect that,
because they only inspect the emitted source text. PAGE_HTML has been
parsed with html.parser for balanced markup, which is not the same check.

Everything mechanically verifiable is verified: tests green, CI green,
conventions clean, docs consistent with the code, no dependency added, and
dispatch.py/dev_loop.py/MODE_SPECS/the prompt templates untouched, so
the manual dispatch verification those would require does not apply.
What remains is a short look in a browser:

  • a plant taps open and closed, and the card shows the right repo;
  • Left/Right/Home/End move between the Plot and Table tabs;
  • stopping the server desaturates the page and the caption ages.

Once that passes, this is ready to squash-merge.

This review was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

@dmccoystephenson
dmccoystephenson merged commit db51ea7 into main Aug 14, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant