spec: start timestamp on collapsed agent reasoning phases (#10292)#10461
spec: start timestamp on collapsed agent reasoning phases (#10292)#10461lonexreb wants to merge 1 commit intowarpdotdev:masterfrom
Conversation
|
I'm starting a first review of this spec-only pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This spec defines timestamp behavior for collapsed and expanded agent reasoning, tool-call, and plan-step sub-blocks, including settings, accessibility, update cadence, tests, and implementation pointers.
Concerns
- The absolute-time examples conflict on whether 12-hour formats include seconds.
- Forced
relativemode is underspecified for starts older than 60 minutes. - The implementation pointers reference paths that do not exist in this checkout.
- The setting name and UI label are narrower than the documented scope across all agent sub-blocks.
Verdict
Found: 0 critical, 3 important, 1 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| - RELATIVE if the start was within the last 60 minutes | ||
| (e.g. `just now`, `34s ago`, `2m ago`). | ||
| - ABSOLUTE if older | ||
| (e.g. `11:42:07` for 24h locale, `11:42 AM` for 12h locale). |
There was a problem hiding this comment.
11:42:07 AM; choose one seconds policy so UI behavior and tests cannot diverge.
|
|
||
| - `"off"` — no timestamp on collapsed or expanded rows. | ||
| - `"absolute"` — always absolute time. | ||
| - `"relative"` — always relative time. |
There was a problem hiding this comment.
relative formats starts older than 60 minutes; B2 only defines relative strings below 60 minutes and auto switches to absolute after that.
| - Collapsed row component: `app/src/agent/conversation/reasoning_block.rs`. | ||
| - Tool-call block: `app/src/agent/conversation/tool_call_block.rs`. | ||
| - Plan-step block: `app/src/agent/conversation/plan_step.rs`. |
There was a problem hiding this comment.
app/src/agent/conversation/...); point implementers at the actual agent conversation/block modules or remove path-specific guidance.
| | `agent.reasoning_phase_timestamp_format` | enum `"off" \| "absolute" \| "relative" \| "auto"` | `"auto"` | Drives collapsed-row format. | | ||
| | `agent.reasoning_phase_timestamp_show_in_expanded` | bool | `true` | When `false`, B3 is suppressed. | | ||
|
|
||
| UI placement: Settings → Agents → "Reasoning phase timestamps": |
There was a problem hiding this comment.
💡 [SUGGESTION] The setting label says Reasoning phase timestamps, but the behavior applies to tool calls and plan steps too; use broader naming or document that this control covers all agent sub-blocks.
Spec-only PR. References #10292.
Summary
Adds a start timestamp to collapsed agent reasoning/thinking phase headers (and the same treatment to other agent sub-blocks: tool calls, plan steps), with start time + duration when expanded. Format is configurable (
off/absolute/relative/auto, defaultauto— relative ≤60min, absolute beyond), locale + 12h/24h aware, and accessible viaaria-label.Why
Long agent runs accumulate many collapsed sub-block rows with no time information. Users cannot correlate phases with terminal actions, judge how long an in-progress phase has been running, or audit a transcript after the fact without expanding each row.
Spec
See
specs/GH10292/SPEC.mdfor the full behavior contract, settings surface, acceptance criteria, implementation pointers (reasoning_block.rs,tool_call_block.rs,plan_step.rs, plus a coalesced ticker), tests, and open questions.Related
Builds on per-message timestamp work in PR #10128. That PR delivers message-level timing; this spec extends timestamp coverage into sub-blocks within a single agent message. The two share the same coalesced timer infrastructure (single ticker per conversation list, fan-out to subscribed rows — no per-row timers).
Labels
ready-to-spec