feat(tui): add optional message timestamps #1206
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,13 @@ | ||
| import type { AssistantMessage } from "@earendil-works/pi-ai"; | ||
| import { | ||
| type Component, | ||
| Container, | ||
| Markdown, | ||
| type MarkdownTheme, | ||
| Spacer, | ||
| Text, | ||
| visibleWidth, | ||
| } from "@earendil-works/pi-tui"; | ||
| import type { MarkdownTransformer } from "../../../core/extensions/types.ts"; | ||
| import { getMarkdownTheme, theme } from "../theme/theme.ts"; | ||
| import { type AssistantRenderDescriptor, createAssistantRenderDescriptors } from "./assistant-render-descriptors.ts"; | ||
|
|
@@ -28,6 +36,11 @@ export class AssistantMessageComponent extends Container { | |
| private hasToolCalls = false; | ||
| private expanded = false; | ||
| private isStreaming = false; | ||
| private showTimestamps: boolean; | ||
| // Stamped once when the message arrives, never at render time: render() is | ||
| // cache-backed and re-runs on resize, so reading the clock there would make a | ||
| // past message silently change its own timestamp. | ||
| private arrivedAt?: Date; | ||
|
|
||
| constructor( | ||
| message?: AssistantMessage, | ||
|
|
@@ -36,9 +49,11 @@ export class AssistantMessageComponent extends Container { | |
| hiddenThinkingLabel = "Thinking...", | ||
| outputPad = 1, | ||
| markdownTransformers: readonly MarkdownTransformer[] = [], | ||
| showTimestamps = false, | ||
| ) { | ||
| super(); | ||
|
|
||
| this.showTimestamps = showTimestamps; | ||
| this.hideThinkingBlock = hideThinkingBlock; | ||
| this.markdownTheme = markdownTheme; | ||
| this.hiddenThinkingLabel = hiddenThinkingLabel; | ||
|
|
@@ -83,13 +98,35 @@ export class AssistantMessageComponent extends Container { | |
| this.refreshContent(); | ||
| } | ||
|
|
||
| setShowTimestamps(show: boolean): void { | ||
| if (this.showTimestamps === show) return; | ||
| this.showTimestamps = show; | ||
| this.invalidate(); | ||
| } | ||
|
|
||
| private timestampPrefix(): string { | ||
| if (!this.showTimestamps) return ""; | ||
| const at = this.arrivedAt ?? new Date(); | ||
| const hh = String(at.getHours()).padStart(2, "0"); | ||
| const mm = String(at.getMinutes()).padStart(2, "0"); | ||
| const ss = String(at.getSeconds()).padStart(2, "0"); | ||
| return theme.fg("dim", `${hh}:${mm}:${ss} `); | ||
| } | ||
|
|
||
| override render(width: number): string[] { | ||
| const signature = this.lastMessageSignature ?? ""; | ||
| if (this.renderCache?.width === width && this.renderCache.signature === signature) { | ||
| return [...this.renderCache.lines]; | ||
| } | ||
|
|
||
| const prefix = this.timestampPrefix(); | ||
| const prefixWidth = visibleWidth(prefix); | ||
| const showTimestamp = prefixWidth > 0 && width > prefixWidth; | ||
| const lines = super.render(showTimestamp ? width - prefixWidth : width); | ||
|
There was a problem hiding this comment.
When timestamps are enabled, this subtracts the nine-column prefix from the width used to render the entire assistant component, but the prefix is added only to the first non-empty line. Every continuation line therefore wraps nine columns early while still starting at column zero, unnecessarily increasing transcript height and narrowing tables or code blocks; only the prefixed line should surrender that width. AGENTS.md reference: packages/coding-agent/src/modes/interactive/components/AGENTS.md:L34-L34 Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. The narrower Markdown render is intentional and follows #1205's stated contract to reserve the visible prefix width before Markdown rendering. Using one width for the component keeps Markdown/table/code wrapping internally consistent and provides a simple hard width bound; the prefix is still attached only to the first non-empty line. Boundary tests cover widths 9–12 and real TUI QA covers 120x34 to 80x24 resize. |
||
| const firstContentLineIndex = lines.findIndex((line) => visibleWidth(line) > 0); | ||
| if (firstContentLineIndex >= 0 && showTimestamp) { | ||
| lines[firstContentLineIndex] = prefix + lines[firstContentLineIndex]; | ||
| } | ||
| if (this.hasToolCalls || lines.length === 0) { | ||
| this.cacheRender(width, signature, lines); | ||
| return lines; | ||
|
|
@@ -102,6 +139,10 @@ export class AssistantMessageComponent extends Container { | |
| } | ||
|
|
||
| updateContent(message: AssistantMessage, isStreaming = this.isStreaming): void { | ||
| // First delta only. updateContent runs once per streaming chunk, so stamping | ||
| // unconditionally would march the clock forward while the message is still | ||
| // being written and land on the completion time rather than the start. | ||
| this.arrivedAt ??= new Date(); | ||
|
There was a problem hiding this comment.
When a saved session is resumed, reloaded, or otherwise rebuilt with timestamps enabled, constructing each component immediately calls Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. This is intentional for the contract in #1205: |
||
| const previousMessage = this.lastMessage; | ||
| const streamingChanged = this.isStreaming !== isStreaming; | ||
| this.isStreaming = isStreaming; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When this setting is toggled in a long transcript,
InteractiveModecalls this method for every assistant component, andinvalidate()clearsrenderDescriptorsbeforerefreshContent(), causing reconciliation to dispose and recreate every Markdown/Text child even though only the prefix changed. This makes a simple settings toggle synchronously rebuild the complete assistant tree and can stall large sessions; clear only the outer render cache here rather than invoking the full content invalidation path.AGENTS.md reference: packages/coding-agent/src/modes/interactive/components/AGENTS.md:L41-L41
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in the current head.
setShowTimestamps()andsetTimestampEligible()now invalidate only the outer render cache; they no longer clear or reconcile content descriptors. The regression test captures the memoized content children, toggles both states, and asserts object identity is unchanged.