[AGE-4027] fix(api): Stop session rename from bumping the activity timestamp - #5634
[AGE-4027] fix(api): Stop session rename from bumping the activity timestamp#5634bekossy wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
Walkthrough
ChangesStream header timestamp behavior
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5bd23d85f8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # `updated_at` is the heartbeat timestamp (see SessionStreamDBE docstring), read by the | ||
| # frontend as last-activity time for sorting -- a rename must not move it, or an | ||
| # unrelated/delayed header write (e.g. the fire-and-forget auto-title call) can reorder | ||
| # the session list ahead of a session with a genuinely more recent message. |
There was a problem hiding this comment.
Condense the explanatory comment
This four-line block duplicates details already documented by SessionStreamDBE and the frontend, making the DAO harder to scan and creating another explanation that can become stale. Condense it to a terse invariant about preserving the heartbeat timestamp; api/AGENTS.md explicitly requires minimal comments and prohibits multi-line narration of what the code does.
AGENTS.md reference: api/AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
Summary
Fixes #5579. In the playground's Session history list, sending a message in session A then session B correctly shows B on top (most recent first) — but reloading the page can flip the order back, even though B still has the more recent message.
Root cause: the session list has one sort function on both the live and reload paths, keyed on
lastMessageAt. Live updates stamp it precisely client-side when a turn settles. On reload, the value is folded in from the server'sSessionStream.updated_atcolumn — documented as "the heartbeat timestamp." Butupdate_header()in the API also bumped that same column on every rename, including the frontend's fire-and-forget auto-title call that fires on every session's first message. That call is unawaited, so its write can land arbitrarily late — including after a different session's entire exchange has finished — silently reordering the list on the next reconcile (mount/reload/focus/60s poll).Changes
update_header()(api/oss/src/dbs/postgres/sessions/streams/dao.py) no longer touchesupdated_at. That's the real heartbeat path's job (update(), a separate method) — a rename/auto-title is not activity. Checked: on the frontend,updated_athas exactly one consumer (the session-list sort), and nothing else on the backend depends on rename bumping it.Testing
Verified locally
ruff format/ruff checkclean.Added or updated tests
api/oss/tests/pytest/unit/sessions/test_stream_header_no_activity_bump.py: a fake-engine unit test (no live DB) asserting a rename leavesupdated_atuntouched. Verified it fails against the pre-fix code and passes against the fix.api/oss/tests/pytest/unit/sessions/suite (142 tests) passes.QA follow-up
Demo
Checklist
Contributor Resources