Skip to content

Fix console turn finish timestamps - #1858

Merged
gjkim42 merged 1 commit into
mainfrom
fix/console-finish-timestamps
Oct 29, 2026
Merged

gjkim42 merged 1 commit into
mainfrom
fix/console-finish-timestamps

Conversation

@gjkim42

@gjkim42 gjkim42 commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

What type of PR is this?

/kind bug

What this PR does / why we need it:

Console conversations only show how long a turn worked, leaving its finish time unavailable. The runtime also strips completion timestamps when preparing history for replay.

Show a readable separator such as Finished at 12:05 PM · Worked for 5m 19s. Today's turns show a short local time; earlier days include the date, with the year when it differs from the current year. The full date, seconds, and time zone are available on hover, keyboard focus, or tap. Enter toggles the details and Escape dismisses them. Failed and interrupted turns have explicit labels.

Preserve completion timestamps through recent and paginated history so finish times remain available after reconnecting. Retained history shows finish times without durations. Keep older-page replay separate from live-turn timing in the console and plain terminal, so loading earlier messages neither invents a duration for an older turn nor loses the active turn's duration. Omit unknown timestamps and recovery-generated interruption times. Update the console documentation and tests for formatting, rendering, dismissal, replay isolation, and history delivery.

Which issue(s) this PR is related to:

N/A

Special notes for your reviewer:

Validation:

  • make update and make verify passed.
  • make test TEST_FLAGS="-run 'TestApplication|TestSessionPlainTerminal|TestProjectHistory|TestServerPagesProjectedHistoryItems'" passed.
  • Replay-isolation tests verify that timestamped older completions have no duration and the active turn retains its correct duration.
  • Chromium checks on a conversation fixture passed for hover, keyboard focus, Enter/Escape, touch disclosure and dismissal, and a 390px mobile layout without horizontal overflow.
  • The earlier full make test run failed on TestCodexEntrypointUsesPersistentSessionHome and the Codex case of TestAgentEntrypointsPreserveSkillReferenceFiles. Both failures reproduce on the unchanged base commit 15eb48e in this environment.

Screenshots use the same completed-turn fixture, locale (en-US), time zone (UTC), clock (August 8, 2026), and desktop viewport, rendered with the base and PR versions of the console code and styles.

Before

Before: the completed turn shows only its duration

After

After: a readable finish time comes before the duration

Full timestamp and mobile views

Full timestamp with keyboard focus

Full date, seconds, and time zone exposed by the timestamp disclosure

Mobile (390px)

Compact timestamp and duration on mobile

Does this PR introduce a user-facing change?

The console displays readable turn finish times, including in retained history. Live turn separators show finish times before their durations. Full timestamps are accessible on hover, keyboard focus, or tap, and failed and interrupted turns are labeled explicitly.

Summary by cubic

Shows each console turn's finish time before its duration, with distinct labels for completed, failed, and interrupted turns, and preserves completion timestamps in history so the finish time stays visible after reconnecting.

  • Same-day turns show the time; older turns add the date, and the year when it differs from the current year.
  • Hover, focus, or tap the finish time for the full date, seconds, and time zone; press Escape to dismiss.
  • Finish times are omitted for unknown timestamps and for turns interrupted by runtime recovery.
  • Adds coverage for live rendering, replay, missing timestamps, and paginated history, and updates the console docs.

Written for commit 8a319b1. Summary will update on new commits.

View guided diff Turn on auto-fix

@gjkim42 gjkim42 added the kind/bug Categorizes issue or PR as related to a bug label Oct 29, 2026
@gjkim42

gjkim42 commented Oct 29, 2026

Copy link
Copy Markdown
Collaborator Author

/kelos review

@kelos-bot

kelos-bot Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

🤖 Kelos Task Status

Task kelos-reviewer-issue-comment-3edd4b0aceb5 has succeeded. ✅

@gjkim42
gjkim42 marked this pull request as ready for review October 29, 2026 21:39
@gjkim42

gjkim42 commented Oct 29, 2026

Copy link
Copy Markdown
Collaborator Author

/kelos review

@kelos-bot

kelos-bot Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

🤖 Kelos Reviewer Agent @gjkim42

Review Summary

Verdict: APPROVE
Overall correctness: patch is correct
Scope: Keeps turn.completed timestamps in projected and paginated history. Console turn separators show a "Finished/Failed/Interrupted at …" disclosure before "Worked for …". Older-page replay is kept out of live-turn timing in the console and the plain terminal.

Reviewed at head 06fdfbc.

Findings Overview

Priority Count File:Line Summary
P0 0 — none
P1 0 — none
P2 0 — none
P3 0 — none

Findings

No new findings. All three findings from the previous review (at 8a319b1) are fixed in the current head:

  • Plain terminal /history during an active turn (was P2): fixed. internal/cli/session_terminal.go:406 now skips the duration calculation and the activeTurn* reset for page events. The "Turn interrupted." warning still prints for older pages. TestSessionPlainTerminalIsolatesHistoryPagesFromLiveStream now replays a timestamped page completion while turn-1 is active. It asserts that only one Worked for appears and that it is the live turn's separator.
  • Web console older-page replay (was P2): fixed. replayOlderHistoryPage (internal/consoleserver/frontend/app.ts:5051-5053) clears activeTurn, activeTurnID and activeTurnStartedAt before replaying the page, and the existing live snapshot restores them afterward. Assistant segments are keyed by event.turnId || 'current' on fresh maps, not by activeTurnID, so clearing it does not change how page messages render. testOlderHistoryPageIsPrependedWithoutChangingLiveState checks that the older divider has a <time> and no .turn-duration, and that the live turn still shows Worked for 5m 19s.
  • Docs wording (was P3): fixed. docs/reference.md now says "Retained history shows finish times without durations."

Other paths checked, with nothing to flag:

  • Initial history and reconnects: projected history is sent only when since == 0 or bounds.Reset, and every client clears its active-turn state on those paths before replay. Projected history has no turn.started items, so none of the clients can compute a duration from a projected completion.
  • TUI: replayHistoryPage uses a fresh model with no active-turn state, so older pages never produce a separator.
  • Runtime recovery: in projected history, runtime.recovered still comes before the recovery-generated turn.completed (interrupted), so recoveredCompletion hides both the finish time and the duration.
  • Journal timestamps: normalizedHistoryEvent copies the event struct and only stops clearing Timestamp on completions. The journal sets a timestamp only when one is missing (journal.go:242), so the timestamps set in the server test fixtures are kept.

Key takeaways

  • Older-page replay is now isolated from live-turn timing in all three clients. The regression cases from the last review are covered by the plain-terminal and console tests.
  • Test coverage for the new rendering is good: formatting (today, an earlier day, and an earlier year), every status label in both live and replay, Escape and Enter dismissal, unknown timestamps, and recovery interruptions.

@gjkim42
gjkim42 force-pushed the fix/console-finish-timestamps branch from 8a319b1 to 06fdfbc Compare October 29, 2026 21:52
@gjkim42

gjkim42 commented Oct 29, 2026

Copy link
Copy Markdown
Collaborator Author

/kelos review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant