Skip to content

fix: use event timestamps for user messages in SessionFromEvents#2158

Merged
dgageot merged 1 commit intodocker:mainfrom
dgageot:fix-timestamps
Mar 18, 2026
Merged

fix: use event timestamps for user messages in SessionFromEvents#2158
dgageot merged 1 commit intodocker:mainfrom
dgageot:fix-timestamps

Conversation

@dgageot
Copy link
Member

@dgageot dgageot commented Mar 18, 2026

SessionFromEvents was creating user messages with time.Now(), which runs after the container has finished. This gave user messages a timestamp after the last assistant message, causing Session.Duration() to return ~0 and breaking the longest-first eval sorting.

Now user messages use the timestamp from the "user_message" event in the container output stream. Falls back to the first "agent_choice" timestamp when no "user_message" event is present.

Assisted-By: docker-agent

SessionFromEvents was creating user messages with time.Now(), which runs
after the container has finished. This gave user messages a timestamp
after the last assistant message, causing Session.Duration() to return
~0 and breaking the longest-first eval sorting.

Now user messages use the timestamp from the "user_message" event in the
container output stream. Falls back to the first "agent_choice" timestamp
when no "user_message" event is present.

Assisted-By: docker-agent
@dgageot dgageot requested a review from a team as a code owner March 18, 2026 13:27
Copy link

@docker-agent docker-agent bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

No issues found in the changed code. The timestamp fix correctly addresses the Duration() calculation problem by using event timestamps instead of time.Now().

Review Summary:

  • ✅ User message timestamps now use event stream timestamps
  • ✅ Fallback to agent_choice timestamp when user_message event is absent
  • ✅ Multi-turn evaluation logic correctly increments questionIdx
  • ✅ parseEventTimestamp fallback to time.Now() is intentional error handling

The implementation properly fixes the bug where user messages had timestamps after assistant messages, which was breaking longest-first eval sorting.

@dgageot dgageot merged commit 57c4c2b into docker:main Mar 18, 2026
8 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

Development

Successfully merging this pull request may close these issues.

2 participants