Skip to content

fix: replace queued write with latest state in StateManagerDisk - #6840

Open
honma89 wants to merge 1 commit into
reflex-dev:mainfrom
honma89:fix/disk-state-manager-stale-write-queue
Open

fix: replace queued write with latest state in StateManagerDisk#6840
honma89 wants to merge 1 commit into
reflex-dev:mainfrom
honma89:fix/disk-state-manager-stale-write-queue

Conversation

@honma89

@honma89 honma89 commented Aug 4, 2026

Copy link
Copy Markdown

All Submissions:

  • Have you followed the guidelines stated in CONTRIBUTING.md file?
  • Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Changes To Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

What does this PR do and why?

StateManagerDisk.set_state only enqueued a token for a debounced write if that token wasn't already in _write_queue:

if token not in self._write_queue:
    self._write_queue[token] = QueueItem(...)

If set_state was called again for the same token before the queued item flushed, the existing QueueItem was left untouched, so _flush_write_queue / _process_write_queue would later write the first queued value to disk instead of the latest one. Direct set_state calls with a replacement state object (as opposed to modify_state, which mutates the object in place) could silently lose data during the debounce window.

This PR changes set_state so that when a token is already queued, the queued QueueItem is replaced with one carrying the latest state via dataclasses.replace (since QueueItem is a frozen dataclass), while preserving the original timestamp so the flush still happens at its originally scheduled time — only the value that gets written changes.

A regression test (test_state_manager_disk_set_state_updates_queued_write) was added that mirrors the reproduction from the linked issue: it calls set_state twice for the same token with debouncing enabled, asserts the queued state reflects the latest call, then verifies the value persisted to disk after close() also reflects the latest call. Verified this test fails on the pre-fix code (1 == 2) and passes with the fix.

Also added a towncrier news fragment (news/+disk-state-manager-stale-write-queue.bugfix.md) per repo convention.

closes #6839

Review in cubic

@honma89
honma89 requested a review from a team as a code owner August 4, 2026 09:53
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@greptile-apps

greptile-apps Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes debounced disk-state persistence so repeated writes for the same token retain the latest state while preserving the original flush schedule.

  • Replaces an existing frozen queue item with its latest state using dataclasses.replace.
  • Adds regression coverage for the queued value and the value persisted during shutdown.

Confidence Score: 5/5

The PR appears safe to merge, with the replacement preserving the queue’s existing scheduling and persistence contracts.

The queue continues to use the original token and timestamp, while normal processing and shutdown flushing retrieve the latest replacement value from the queue.

Important Files Changed

Filename Overview
reflex/istate/manager/disk.py Correctly updates an existing queued item’s state without changing its token or debounce timestamp.
tests/units/test_state.py Adds focused coverage proving both the in-memory queue entry and flushed disk value use the latest state.

Reviews (1): Last reviewed commit: "fix: replace queued write with latest st..." | Re-trigger Greptile

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No issues found across 2 files

Tip: cubic could auto-approve low-risk PRs like this, if it thinks it's safe to merge. Learn more

Re-trigger cubic

@harsh21234i

harsh21234i commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

The implementation looks correct to me. Replacing the queued QueueItem state while preserving the original timestamp
fixes the stale flush without changing debounce timing.

One CI issue remains: pyright flags the test because it assigns through state_manager.__dict__. Replacing those two
assignments with object.__setattr__(..., "states_directory", tmp_path) fixes the type error locally.

Also, the PR body mentions a news fragment, but I don’t see one in the diff. Could you add the fragment .

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.

StateManagerDisk debounced writes can persist stale state after repeated set_state calls

2 participants