feat(web): add <TimestampLabel> shared atom to unify timestamp rendering #1184

Merged

charles merged 2 commits from feat/1161-timestamp-label into main 2026-05-14 23:06:42 +00:00

Conversation 2 Commits 2 Files changed 6 +63 -14

dev commented 2026-05-14 22:39:10 +00:00

Collaborator

Copy link

Closes #1161.

Summary

• Introduces <TimestampLabel ts={number} showRelative? /> in components/timestamp-label.tsx
• Displays HH:MM:SS local time via the existing fmtTime() helper
• Full ISO-8601 datetime shown in a <Tooltip> on hover
• When showRelative is set the tooltip appends a relative age suffix e.g. "14:23:01 · 3m ago" via fmtAgo()
• fmtTime() in lib/format.ts kept unchanged (non-React callers)

Adoption

File	What changed
event-log.tsx	fmtTime(ev.ts) span → <TimestampLabel ts={ev.ts} />
task-list.tsx	elapsed-duration text → <TimestampLabel ts={task.started_at} showRelative />
run-header-meters.tsx	added optional started_at prop; renders <TimestampLabel> chip in metrics bar
session-scrubber.tsx	fmtTime(tick.ts) in hover popover → <TimestampLabel ts={tick.ts} />
roster/instance-history-drawer.tsx	fmtRelative() call → <TimestampLabel ts={e.at} showRelative />; exported fmtRelative kept for existing tests

Test plan

• just qa passes (typecheck + lint + format + tests)
• Event log rows show HH:MM:SS; hovering shows ISO-8601 tooltip
• Task list rows show start timestamp; hover tooltip shows ISO + relative age
• Run header metrics bar shows start timestamp chip
• Scrubber popover tick shows HH:MM:SS with ISO tooltip on hover
• Failover history drawer shows timestamps; tooltip shows relative age

Closes #1161. ## Summary - Introduces `<TimestampLabel ts={number} showRelative? />` in `components/timestamp-label.tsx` - Displays `HH:MM:SS` local time via the existing `fmtTime()` helper - Full ISO-8601 datetime shown in a `<Tooltip>` on hover - When `showRelative` is set the tooltip appends a relative age suffix e.g. "14:23:01 · 3m ago" via `fmtAgo()` - `fmtTime()` in `lib/format.ts` kept unchanged (non-React callers) ## Adoption | File | What changed | |---|---| | `event-log.tsx` | `fmtTime(ev.ts)` span → `<TimestampLabel ts={ev.ts} />` | | `task-list.tsx` | elapsed-duration text → `<TimestampLabel ts={task.started_at} showRelative />` | | `run-header-meters.tsx` | added optional `started_at` prop; renders `<TimestampLabel>` chip in metrics bar | | `session-scrubber.tsx` | `fmtTime(tick.ts)` in hover popover → `<TimestampLabel ts={tick.ts} />` | | `roster/instance-history-drawer.tsx` | `fmtRelative()` call → `<TimestampLabel ts={e.at} showRelative />`; exported `fmtRelative` kept for existing tests | ## Test plan - [ ] `just qa` passes (typecheck + lint + format + tests) - [ ] Event log rows show `HH:MM:SS`; hovering shows ISO-8601 tooltip - [ ] Task list rows show start timestamp; hover tooltip shows ISO + relative age - [ ] Run header metrics bar shows start timestamp chip - [ ] Scrubber popover tick shows `HH:MM:SS` with ISO tooltip on hover - [ ] Failover history drawer shows timestamps; tooltip shows relative age

dev added 1 commit 2026-05-14 22:39:10 +00:00

feat(web): add <TimestampLabel> shared atom to unify timestamp rendering ... d4c91c0b03

Closes #1161.

Introduces `<TimestampLabel ts={number} showRelative? />` in
`components/timestamp-label.tsx`. Displays `HH:MM:SS` local time via
the existing `fmtTime()` helper and surfaces the full ISO-8601 datetime
on hover via `<Tooltip>`. When `showRelative` is set the tooltip appends
a relative age suffix ("14:23:01 · 3m ago").

Adoption across all five call sites:
- `event-log.tsx` — replaces bare `fmtTime(ev.ts)` span in event rows
- `task-list.tsx` — replaces elapsed-duration text with start timestamp
- `run-header-meters.tsx` — adds optional `started_at` prop rendered as
  a `<TimestampLabel>` in the metrics bar header
- `session-scrubber.tsx` — replaces `fmtTime(tick.ts)` in the hover
  popover
- `roster/instance-history-drawer.tsx` — replaces `fmtRelative()` call
  with `<TimestampLabel showRelative>` (exported `fmtRelative` kept for
  existing tests)

Co-authored-by: Cursor <cursoragent@cursor.com>

dev requested review from reviewer 2026-05-14 22:45:12 +00:00

claude-desktop referenced this pull request 2026-05-14 22:47:54 +00:00

feat(web): add &lt;TimestampLabel&gt; shared atom to unify timestamp rendering #1181

reviewer requested changes 2026-05-14 22:47:59 +00:00

Dismissed

reviewer left a comment

Copy link

Review — <TimestampLabel> shared atom (PR #1184)

Note: There is an earlier competing implementation for the same issue in PR #1181 (CSS group-hover / <time> element approach). Both are open. One of them needs to be closed before the other merges. This review is for #1184.

The idea is sound and the adoption across five components is consistent. Three issues need addressing before merge.

---

🔴 Critical — tooltip content is wrong when showRelative=true

// timestamp-label.tsx
const tooltipContent = showRelative ? `${absolute} · ${fmtAgo(ts)}` : iso;

When showRelative=true the tooltip shows e.g. "14:23:01 · 3m ago". The problem: 14:23:01 is already the label text visible without hovering. The hover tooltip repeats the absolute time and adds a relative suffix — but the full ISO-8601 datetime is never reachable when showRelative=true. Users who hover to get a precise, copy-pasteable timestamp for debugging get nothing useful.

Fix — always expose ISO-8601, append relative when showRelative:

const tooltipContent = showRelative ? `${iso} · ${fmtAgo(ts)}` : iso;
// → "2026-05-14T14:23:01.000Z · 3m ago"

This also makes both modes symmetric: the tooltip is always the authoritative absolute value.

---

🔴 Should use <time dateTime={iso}> not <span>

PR #1181 (the other open attempt) uses <time dateTime={iso} title={iso}>. That is the correct semantic element for machine-readable datetime — screen readers announce it with the display text, parsers index it, and native browser tooltips expose the ISO value for free even without JS-powered tooltips.

<span> is a regression. The <Tooltip> content rendered via Base UI portals is typically not exposed to ARIA unless explicitly wired, so assistive technology may get no datetime information at all.

return (
  <Tooltip content={tooltipContent} side="top" delay={300}>
    <time dateTime={iso} className={className}>
      {absolute}
    </time>
  </Tooltip>
);

<time> is an inline element; it slots into the Tooltip trigger naturally via Base UI's render prop mechanism (same as <span>).

---

🟡 task-list.tsx — silently drops the "running…" status text

Before this PR, task rows showed:

State	Text
Finished	dev · 5m 23s (duration)
Running	dev · running…
No data	dev · —

After this PR, all states show dev · 14:23:01 · 3m ago (start time + relative age). The "running…" indicator is gone with no discussion. The duration on finished tasks is also gone — replaced by the start time, which is less scannable for "how long did this take?"

If the intent is to replace elapsed-duration display with start-time display, that should be called out explicitly in the PR summary as a deliberate UX tradeoff. Right now it reads as an accidental side-effect of the refactor. Consider at minimum keeping "running…" for the active-task state.

---

🟡 No tests — all test-plan items unchecked

PR #1182 (<TurnMeter>) shipped with turn-meter.test.tsx (13 tests covering all size variants, the null/undefined/number cost branches, and color-tier escalation). <TimestampLabel> is simpler but the tooltip-content logic, the showRelative branch, and the fmtTime(0) → "—" edge case should have coverage. A basic Vitest browser-mode test file would suffice.

---

🟢 What's good

• className prop is the right ergonomic call — it removes the wrapper <span> at most callsites (event-log, session-scrubber) and keeps JSX clean.
• run-header-meters.tsx adds started_at as optional (started_at?: number in the intersection) rather than required — backward-compatible, good.
• The early-return guard uses == null (catches both null and undefined) — correct.
• Tooltip delay={300} is a deliberate, sensible reduction from the 500 ms default for info-dense rows.
• The fmtAgo / fmtTime split (React callers vs. non-React callers) is correctly documented and respected.

## Review — `<TimestampLabel>` shared atom (PR #1184) **Note:** There is an earlier competing implementation for the same issue in PR #1181 (CSS `group-hover` / `<time>` element approach). Both are open. One of them needs to be closed before the other merges. This review is for #1184. The idea is sound and the adoption across five components is consistent. Three issues need addressing before merge. --- ### 🔴 Critical — tooltip content is wrong when `showRelative=true` ```ts // timestamp-label.tsx const tooltipContent = showRelative ? `${absolute} · ${fmtAgo(ts)}` : iso; ``` When `showRelative=true` the tooltip shows e.g. `"14:23:01 · 3m ago"`. The problem: `14:23:01` is **already the label text** visible without hovering. The hover tooltip repeats the absolute time and adds a relative suffix — but the full ISO-8601 datetime is **never reachable** when `showRelative=true`. Users who hover to get a precise, copy-pasteable timestamp for debugging get nothing useful. Fix — always expose ISO-8601, append relative when `showRelative`: ```ts const tooltipContent = showRelative ? `${iso} · ${fmtAgo(ts)}` : iso; // → "2026-05-14T14:23:01.000Z · 3m ago" ``` This also makes both modes symmetric: the tooltip is always the authoritative absolute value. --- ### 🔴 Should use `<time dateTime={iso}>` not `<span>` PR #1181 (the other open attempt) uses `<time dateTime={iso} title={iso}>`. That is the correct semantic element for machine-readable datetime — screen readers announce it with the display text, parsers index it, and native browser tooltips expose the ISO value for free even without JS-powered tooltips. `<span>` is a regression. The `<Tooltip>` content rendered via Base UI portals is typically **not** exposed to ARIA unless explicitly wired, so assistive technology may get no datetime information at all. ```tsx return ( <Tooltip content={tooltipContent} side="top" delay={300}> <time dateTime={iso} className={className}> {absolute} </time> </Tooltip> ); ``` `<time>` is an inline element; it slots into the Tooltip trigger naturally via Base UI's `render` prop mechanism (same as `<span>`). --- ### 🟡 `task-list.tsx` — silently drops the "running…" status text Before this PR, task rows showed: | State | Text | |---|---| | Finished | `dev · 5m 23s` (duration) | | Running | `dev · running…` | | No data | `dev · —` | After this PR, all states show `dev · 14:23:01 · 3m ago` (start time + relative age). The "running…" indicator is gone with no discussion. The duration on finished tasks is also gone — replaced by the start time, which is less scannable for "how long did this take?" If the intent is to replace elapsed-duration display with start-time display, that should be called out explicitly in the PR summary as a deliberate UX tradeoff. Right now it reads as an accidental side-effect of the refactor. Consider at minimum keeping "running…" for the active-task state. --- ### 🟡 No tests — all test-plan items unchecked PR #1182 (`<TurnMeter>`) shipped with `turn-meter.test.tsx` (13 tests covering all size variants, the `null`/`undefined`/`number` cost branches, and color-tier escalation). `<TimestampLabel>` is simpler but the tooltip-content logic, the `showRelative` branch, and the `fmtTime(0) → "—"` edge case should have coverage. A basic Vitest browser-mode test file would suffice. --- ### 🟢 What's good - `className` prop is the right ergonomic call — it removes the wrapper `<span>` at most callsites (event-log, session-scrubber) and keeps JSX clean. - `run-header-meters.tsx` adds `started_at` as **optional** (`started_at?: number` in the intersection) rather than required — backward-compatible, good. - The early-return guard uses `== null` (catches both `null` and `undefined`) — correct. - Tooltip `delay={300}` is a deliberate, sensible reduction from the 500 ms default for info-dense rows. - The `fmtAgo` / `fmtTime` split (React callers vs. non-React callers) is correctly documented and respected.

charles added 1 commit 2026-05-14 22:52:14 +00:00

fix(web): use semantic <time> element in TimestampLabel ... 37fb6de36f

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

claude-desktop requested review from reviewer 2026-05-14 22:52:24 +00:00

reviewer requested changes 2026-05-14 22:55:05 +00:00

reviewer left a comment

Copy link

Review — <TimestampLabel> shared atom

The new component itself is clean — semantic <time dateTime>, locale-aware via fmtTime(), tooltip on hover. Two functional issues need fixing before merge, plus one missing test.

---

❌ 1. task-list.tsx removes elapsed duration for completed tasks (regression)

File: apps/web/src/components/task-list.tsx

The elapsed variable was the primary at-a-glance metric for history-tab rows:

// Before
const elapsed = task.finished_at
    ? fmtDuration(task.finished_at - task.started_at)  // "2m 45s"
    : status === "running" ? "running…" : "—";

Replacing it wholesale with <TimestampLabel ts={task.started_at} showRelative /> means a completed task now shows "dev · 14:23:01" (when it started) instead of "dev · 2m 45s" (how long it took). For the history tab, elapsed duration is the more useful signal — start time doesn't tell you whether the task ran for 30 seconds or 3 hours.

Running and queued rows are arguably improved by the change. Please restore elapsed duration for completed tasks. The simplest fix keeps both signals — elapsed for finished_at != null, start-time label otherwise:

const isDone = task.finished_at != null;
const elapsed = isDone ? fmtDuration(task.finished_at! - task.started_at) : null;

// in the render:
<div className="ch-duration text-meta text-text-dim">
    {agent} · {elapsed ?? <TimestampLabel ts={task.started_at} showRelative />}
</div>

---

❌ 2. showRelative tooltip drops the ISO-8601 string

File: apps/web/src/components/timestamp-label.tsx, line 25

const tooltipContent = showRelative ? `${absolute} · ${fmtAgo(ts)}` : iso;

When showRelative=true the tooltip shows "14:23:01 · 3m ago" — the ISO timestamp is gone. The top-of-file docstring even promises "full ISO-8601 datetime shown in a <Tooltip> on hover" for all modes. A user hovering on a start-time chip in the run-header to copy the exact timestamp gets short-changed.

Prefer:

const tooltipContent = showRelative ? `${iso} · ${fmtAgo(ts)}` : iso;

Or if you intentionally want the short clock in the tooltip:

const tooltipContent = showRelative ? `${absolute} · ${fmtAgo(ts)} (${iso})` : iso;

Either way, ISO should always be present — update the JSDoc to match whatever you decide.

---

⚠️ 3. No test for the new component

<TimestampLabel> is a shared atom that will be reached from multiple callsites. The codebase uses Vitest browser mode for component tests; a minimal test would cover the two render paths:

• <TimestampLabel ts={ts} /> — body renders HH:MM:SS, tooltip contains the ISO string.
• <TimestampLabel ts={ts} showRelative /> — tooltip contains the relative suffix.

The existing task-list.test.tsx doesn't assert on .ch-duration content, so nothing will catch a future regression on this component. Please add apps/web/src/components/timestamp-label.test.tsx.

---

🔍 Minor / non-blocking

delay={300} on the Tooltip — the design-system default is 500 ms (tooltip.tsx line 60). Using 300 ms is fine for dense timestamp displays but worth a one-line comment so the next reader knows it's intentional, not an off-by-one from the constant.

fmtTime(0) edge case — fmtTime(0) returns "—" (!0 is truthy), but new Date(0).toISOString() produces the epoch string, so <time dateTime="1970-01-01T00:00:00.000Z">—</time> is technically inconsistent. Practically ts=0 shouldn't appear in real data, but if you want to be defensive, guard both: const iso = ts ? new Date(ts).toISOString() : "";.

---

Summary: fix the task-list regression (#1) and the tooltip ISO omission (#2), add a component test (#3), then this is good to merge.

## Review — `<TimestampLabel>` shared atom The new component itself is clean — semantic `<time dateTime>`, locale-aware via `fmtTime()`, tooltip on hover. Two functional issues need fixing before merge, plus one missing test. --- ### ❌ 1. `task-list.tsx` removes elapsed duration for completed tasks (regression) **File:** `apps/web/src/components/task-list.tsx` The `elapsed` variable was the primary at-a-glance metric for history-tab rows: ```tsx // Before const elapsed = task.finished_at ? fmtDuration(task.finished_at - task.started_at) // "2m 45s" : status === "running" ? "running…" : "—"; ``` Replacing it wholesale with `<TimestampLabel ts={task.started_at} showRelative />` means a completed task now shows `"dev · 14:23:01"` (when it started) instead of `"dev · 2m 45s"` (how long it took). For the history tab, elapsed duration is the more useful signal — start time doesn't tell you whether the task ran for 30 seconds or 3 hours. Running and queued rows are arguably improved by the change. **Please restore elapsed duration for completed tasks.** The simplest fix keeps both signals — elapsed for `finished_at != null`, start-time label otherwise: ```tsx const isDone = task.finished_at != null; const elapsed = isDone ? fmtDuration(task.finished_at! - task.started_at) : null; // in the render: <div className="ch-duration text-meta text-text-dim"> {agent} · {elapsed ?? <TimestampLabel ts={task.started_at} showRelative />} </div> ``` --- ### ❌ 2. `showRelative` tooltip drops the ISO-8601 string **File:** `apps/web/src/components/timestamp-label.tsx`, line 25 ```tsx const tooltipContent = showRelative ? `${absolute} · ${fmtAgo(ts)}` : iso; ``` When `showRelative=true` the tooltip shows `"14:23:01 · 3m ago"` — the ISO timestamp is gone. The top-of-file docstring even promises "full ISO-8601 datetime shown in a `<Tooltip>` on hover" for all modes. A user hovering on a start-time chip in the run-header to copy the exact timestamp gets short-changed. Prefer: ```tsx const tooltipContent = showRelative ? `${iso} · ${fmtAgo(ts)}` : iso; ``` Or if you intentionally want the short clock in the tooltip: ```tsx const tooltipContent = showRelative ? `${absolute} · ${fmtAgo(ts)} (${iso})` : iso; ``` Either way, ISO should always be present — update the JSDoc to match whatever you decide. --- ### ⚠️ 3. No test for the new component `<TimestampLabel>` is a shared atom that will be reached from multiple callsites. The codebase uses Vitest browser mode for component tests; a minimal test would cover the two render paths: - `<TimestampLabel ts={ts} />` — body renders `HH:MM:SS`, tooltip contains the ISO string. - `<TimestampLabel ts={ts} showRelative />` — tooltip contains the relative suffix. The existing `task-list.test.tsx` doesn't assert on `.ch-duration` content, so nothing will catch a future regression on this component. Please add `apps/web/src/components/timestamp-label.test.tsx`. --- ### 🔍 Minor / non-blocking **`delay={300}` on the Tooltip** — the design-system default is 500 ms (`tooltip.tsx` line 60). Using 300 ms is fine for dense timestamp displays but worth a one-line comment so the next reader knows it's intentional, not an off-by-one from the constant. **`fmtTime(0)` edge case** — `fmtTime(0)` returns `"—"` (`!0` is truthy), but `new Date(0).toISOString()` produces the epoch string, so `<time dateTime="1970-01-01T00:00:00.000Z">—</time>` is technically inconsistent. Practically `ts=0` shouldn't appear in real data, but if you want to be defensive, guard both: `const iso = ts ? new Date(ts).toISOString() : "";`. --- **Summary:** fix the `task-list` regression (#1) and the tooltip ISO omission (#2), add a component test (#3), then this is good to merge.

charles force-pushed feat/1161-timestamp-label from 37fb6de36f to 4f70ba4dd4 2026-05-14 23:06:22 +00:00 Compare

charles merged commit 8ac12d059e into main 2026-05-14 23:06:42 +00:00

charles deleted branch feat/1161-timestamp-label 2026-05-14 23:06:43 +00:00

charles referenced this pull request from a commit 2026-05-14 23:06:43 +00:00

feat(web): add &lt;TimestampLabel&gt; shared atom to unify timestamp rendering (#1184)

claude-desktop referenced this pull request 2026-05-14 23:27:16 +00:00

feat(web): add <TimestampLabel> shared atom to unify timestamp rendering #1171

Sign in to join this conversation.

Reviewers

No reviewers

reviewer

Labels

Clear labels   

area:agents

Agent types, pool scheduling, per-instance config

  

area:dashboard

Dashboard UI and observability surfaces

  

area:database

DB layer — schema, migrations, ORM, raw SQL

  

area:design

UI/UX mockup work — routes to designer agent

  

area:design-review

Design review dispatch — routes to design-reviewer agent

  

area:flows

Flow runner — YAML loader, executor, op registry, expression eval

  

area:infra

Deployment, isolation, containers, systemd units

  

area:meta

Tracking, scaffolding, project setup

  

area:security

Security — routes to reviewer-security (opus)

  

area:sessions

Session-id store, Claude SDK resume logic

  

area:webhook

Forgejo webhook routing and handlers

  

area:workdir

Clone cache, worktrees, git identity

  

security

Security-sensitive issue

  

type:bug

Bug

  

type:chore

Chore

  

type:meta

Tracking or decisions, not implementation work

  

type:user-story

User story

No labels

area:agents

area:dashboard

area:database

area:design

area:design-review

area:flows

area:infra

area:meta

area:security

area:sessions

area:webhook

area:workdir

security

type:bug

type:chore

type:meta

type:user-story

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

3 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

charles/agent-hooks!1184

No description provided.