Skip to content

fix(room-ops): read returns messages newest first by timestamp, as it declares - #6230

Open
mybelovedstar123 wants to merge 1 commit into
sonichi:mainfrom
mybelovedstar123:fix/6176-room-read-chronological
Open

mybelovedstar123 wants to merge 1 commit into
sonichi:mainfrom
mybelovedstar123:fix/6176-room-read-chronological

Conversation

@mybelovedstar123

Copy link
Copy Markdown
Contributor

Priority

Level: low
Reason: room_ops read declared order: newest_first while returning the gateway's reply-chain order, and a small limit then kept an old replied-to message instead of newer ones. That matters to every "has anyone replied?" probe that reads a few messages.

Closes

Refs #6176. The reordering itself comes from the gateway's /messages answer, which isn't in this repo. This makes the engine's reader return the order it declares, whatever the gateway sends.

Summary

read_room (skills/agent-room-ops/read.py) took the gateway's message order as-is and labelled it newest_first. The gateway pulls a replied-to message (and a referenced one) up behind its reply, so:

  • the declared order was false;
  • messages[:limit] kept the replied-to message and dropped newer ones.

Now, before the limit, _newest_first() sorts by ts, newest first:

  • Ties keep the gateway's order. Python's sort is stable, and that holds with reverse=True too.
  • Only when every message has a numeric ts. A page where any ts is missing or non-numeric keeps the gateway's order, rather than guessing where those messages go.
  • oldest_first still reverses only the rendering, now of the sorted list.

File: skills/agent-room-ops/read.py.

Checklist

  • Confirmed no other open PR closes the same issue
  • Git author email matches my CLA-signed email
  • Single concern per PR (the read's declared order)
  • Confirmed on upstream/main ("before" below)
  • Tests added: section 5b of tests/room-ops-read-order.test.py (5 checks)
  • Before/after evidence pasted below
  • Every claim in this PR body is verifiable from the diff or the pasted output
  • Live path: a read of a live AG2 Space room with a reply chain. Not run; the test drives the real read_room with the gateway's reply-chain order as the fake's answer.
  • Stacked PR: N/A
  • No host-specific paths added
  • No CI workflow changes

Before / after evidence

The gateway answer used: reply(400, in_reply_to $1), replied-to(100), between-2(300), between-1(200).

main:

FAIL a reply chain is read newest first by timestamp — got ['reply', 'replied-to', 'between-2', 'between-1']
FAIL ...and the limit keeps the newest messages, not the replied-to one — got ['reply', 'replied-to']
FAIL ...and oldest_first is the same, reversed — got ['between-1', 'between-2', 'replied-to', 'reply']
ok   equal timestamps keep the gateway's order
ok   CONTROL: with a message lacking a numeric ts, the gateway's order is kept, not guessed
3 failure(s)

This branch: every check in room-ops-read-order passes, sections 1–6.

Test plan

  • Linux (WSL2, Python 3.14): every test that reads agent-room-ops/read.py, read_room or room_ops (41 test files) passes, except two that this change doesn't touch: gateway-credential-sources (fails identically on main here) and hermetic-env-sentinels (hit my 300 s per-file timeout).
  • Static CI checks: ruff 0.15.22, check-python39-compat.py on Python 3.9.25, every lint-*.sh --diff against upstream/main, gen-src-map.py --check, agents-md-sync.sh --check and review-checks.sh. All pass.

Author: discord username :: slovertop :: agent-assisted (Claude)

… declares

The gateway's /messages answer pulls a replied-to (or referenced) message up
behind its reply, so the order is not chronological (sonichi#6176). read_room passed
that order through while declaring `order: newest_first`, and its `limit`
slice then kept the old replied-to message instead of newer ones.

read_room now sorts by timestamp, newest first, before the limit, when every
message carries a numeric ts; ties keep the gateway's order (stable), and a
page with any non-numeric ts keeps the gateway's order rather than guessing.

Refs sonichi#6176
@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 92%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

  • skills/agent-room-ops/read.py (100%)

Summary

  • Total: 5 lines
  • Missing: 0 lines
  • Coverage: 100%

@mybelovedstar123

Copy link
Copy Markdown
Contributor Author

@cla-assistant check

(agent-assisted, Claude for slovertop)

This branch has not been deployed

No deployments
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.

1 participant