Conversation
Conversation export sorted missing and ISO timestamps as zero, so a reply could be written before the message it answered. Record the Redis enqueue timestamp instead of the time send_and_wait returns. Co-authored-by: Cursor <[email protected]>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Non-finite numeric timestamps must be rejected before sorting.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Updates cooperative-message exports to preserve enqueue timestamps and sort mixed timestamp formats chronologically.
Changes:
- Records enqueue timestamps for normal and blocking sends.
- Parses ISO-8601 timestamps, including trailing
Z. - Adds regression tests for timestamp propagation and ordering.
| File | Description |
|---|---|
tests/runner/test_coop.py |
Tests timestamp ordering behavior. |
tests/agents/mini_swe_agent_v2/test_sent_timestamp.py |
Tests timestamp propagation. |
src/cooperbench/runner/coop.py |
Normalizes message timestamps for sorting. |
src/cooperbench/agents/mini_swe_agent_v2/connectors/messaging.py |
Stores enqueue timestamps. |
src/cooperbench/agents/mini_swe_agent_v2/agents/default.py |
Copies timestamps into sent-message records. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Extreme numeric and ISO timestamps can still raise exceptions and abort conversation export.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (1)
Resolved since last review (1)
Comment on lines
+41
to
+43
| if isinstance(ts, (int, float)): | ||
| value = float(ts) | ||
| return value if math.isfinite(value) else 0.0 |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

The bug
sent_messagesstorestoandcontentonly._extract_conversationthen copies a missing timestamp._message_timestamp_keydocuments ISO input, butfloat(ts)rejects those strings and returns 0. Missing timestamps sort first, and the stable sort groups by sender, so a reply can be written before the message it answers.send_and_waitreturns after the peer replies. A timestamp taken at that point records the reply, not the send. Redis already stores an ISO timestamp atrpush.The fix
On a successful
send, keep that enqueue timestamp on the connector and copy it ontosent_messagesfor both plain sends and--wait. The sort key parses ISO-8601, including a trailingZ, and rejects non-finite numeric values such asNaNand infinities in both numeric and string form. Missing, malformed, and non-finite values sort as 0; a historical record with a null timestamp stays null.Verification
Local validation on Python 3.13.13:
python -m pytest tests/runner/test_coop.py tests/agents/mini_swe_agent_v2/test_sent_timestamp.py— 20 passed. Covers enqueue-time propagation, blocking sends, ISO/UTC offsets, float/string non-finite values, overflow-to-infinity strings, and stable mixed ordering. The non-finite regressions fail before the follow-up fix.python -m pytest tests/ -v --tb=short— 442 passed, 84 skipped. Docker/Modal/GCP integration tests retain their default skips. For this run only, Git tag/commit signing was disabled viaGIT_CONFIG_COUNToverrides so personal signing settings do not alter temporary test repositories; no Git config file was changed.LITELLM_LOCAL_MODEL_COST_MAP=truewas set to avoid fetching a price map in unit tests.ruff check src/cooperbench/,ruff format --check src/cooperbench/, andmypy src/cooperbench/— all passed (76 source files).git diff --check— passed.The full-suite run emits two pre-existing unknown-marker warnings for
slowandtimeout. No remote CI result is available yet; local checks do not establish the Linux/Python matrix.Left out
The no-git prompt that tells an agent to fetch
origin/<peer>is a separate PR.Checklist
Made with Cursor