Repository navigation
fix(core): bump session time_updated on step lifecycle events - #49105
Conversation
|
The following comment was made by an LLM, it may be inaccurate: Potential Duplicate FoundPR #43847 - Why it might be related:
These may be complementary fixes for different event types, or there could be overlapping scope depending on the implementation approach. |
holny
left a comment
There was a problem hiding this comment.
Verified locally — the new test passes, and this matches what I found poking at the dev tree: neither insertMessage nor applyUsage advances SessionTable.time_updated (applyUsage explicitly preserves it), so step activity never moved a session in the list. Bumping through the projector is the right layer too, since time_updated then re-derives deterministically on a projection rebuild.
Two thoughts:
-
touchonly fires on step boundaries, so a prompt that never reachesStep.Started(auth error, invalid model before the LLM call) still leaves the session sitting at its old position. Bumping on the user-message append would cover that, though the step-level fix already handles the reported case. -
Nit: the update isn't monotonic (
set time_updated = timestampunconditionally). Events arrive ordered so it works out in practice, but something likemax(time_updated, ?)would make it immune to any out-of-order timestamp.
Test covers the right sequence — good that it asserts the Failed branch too.
|
Thanks for checking it locally.
|
|
Makes sense on both. Events on a single stream are ordered, so I agree max() is cheap insurance rather than a fix — fine to leave it out if you'd rather keep the change small. The user-message bump is the maintainer's scope call either way; step-only does match what the report describes. |
time_updated was set when a prompt was enqueued but never while the turn ran, so a busy session kept an old timestamp and sorted wrong in recency lists. Also touch the session row on step started, ended and failed. Fixes anomalyco#36893
bf318b7 to
0ca4289
Compare
Issue for this PR
Closes #36893
Type of change
What does this PR do?
On v2, time_updated gets set when a prompt is enqueued, but nothing touches it while the turn runs. So a session that's been busy for a while still sorts as old in the app and tui.
Added a small touch() in projector.ts that writes time_updated from the event's created time, and call it on step started/ended/failed. Execution succeeded/failed still keep time_updated as is, same as before. Didn't change the list sort order.
(Moved from dev to v2 per feedback on Discord.)
How did you verify your code works?
Screenshots / recordings
n/a
Checklist