Skip to content

Fix hot keys returning empty on first MONITOR cycle - #434

Merged
ravjotbrar merged 2 commits into
mainfrom
fix/hotkeys-rendering
Jul 30, 2026
Merged

Fix hot keys returning empty on first MONITOR cycle#434
ravjotbrar merged 2 commits into
mainfrom
fix/hotkeys-rendering

Conversation

@ravjotbrar

@ravjotbrar ravjotbrar commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Summary

Prior to this change, the server waits until checkAt (collection end time) then immediately re-fetches /hot-keys. But monitor data is collected in memory for the full duration, then written to disk after collectLogs returns. The re-fetch arrives before the write completes, returning empty results.

This PR adds a 1-second buffer to the hot keys re-fetch delay after checkAt to account for the asynchronous disk write that happens after monitor collection ends. The 1-second buffer accounts for: promise resolution + event listener cleanup + JSON serialization of rows + fs.appendFile.

Before the fix

Screen.Recording.2026-07-29.at.5.25.01.PM.mov

After the fix

Screen.Recording.2026-07-29.at.5.15.40.PM.mov

Comment thread apps/server/src/actions/hotkeys.ts Outdated
// Reads monitor data and returns when to fetch results (`checkAt`).
if (initialParsedResponse.checkAt) {
const delay = initialParsedResponse.checkAt - Date.now()
// Add 1s buffer: checkAt marks when collection ends, but data is written

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

wondering, what if the disk takes longer than 1s (it is under load, or it is a slow disk)? the refetch will still return empty.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good question. Initially, I thought 1s delay is a good tradeoff. Now that I think about it maybe we should do a limit of retries until we hit that limit or until we get data back.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

good point

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Made the changes

Signed-off-by: ravjotb <ravjot.brar@improving.com>
@ravjotbrar
ravjotbrar force-pushed the fix/hotkeys-rendering branch from 264495e to d7011b4 Compare July 30, 2026 16:40
Signed-off-by: ravjotb <ravjot.brar@improving.com>

@ArgusLi ArgusLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The "best" solution would somehow be to send a signal after disk write, then we wait for that before getting the result, but that would be completely overkill. This works.

@ravjotbrar
ravjotbrar merged commit e6a6e80 into main Jul 30, 2026
8 checks passed
ravjotbrar added a commit that referenced this pull request Jul 30, 2026
* Fix hot keys returning empty on first MONITOR cycle

Signed-off-by: ravjotb <ravjot.brar@improving.com>

* Retry hot keys fetch with 1s interval instead of single buffer

Signed-off-by: ravjotb <ravjot.brar@improving.com>

---------

Signed-off-by: ravjotb <ravjot.brar@improving.com>
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.

Hot keys returns empty on first MONITOR cycle due to disk write race

3 participants