improvement(container-runtime): Further simplifications for dirty/saved logic #24703
Conversation
github-actions
bot
added
base: main
Justification: Of all the calls to `updateDocumentDirtyState` that we were blocking emit for, the only one that could be hit during synchronous `replayPendingStates` is under `submit`. This can never result in transitioning from dirty -> saved, so doing one transition from "before state" to "after state" will not be distinguishable from letting the transition happen naturally as ops are resubmitted (or not)
markfields
force-pushed
the
cr/is-dirty-followups
branch
from
d9ca16c to
18685d5
Compare
…e in isDirty getter" This reverts commit 18685d5.
There was a problem hiding this comment.
Pull Request Overview
This pull request simplifies the dirty/saved state handling logic in the container runtime. Key changes include removing the temporary suppression of dirty document events during replay operations, eliminating the unused flag emitDirtyDocumentEvent, and adjusting the invocation of updateDocumentDirtyState in relevant code paths.
Comments suppressed due to low confidence (1)
packages/runtime/container-runtime/src/containerRuntime.ts:2528
- Ensure that the removal of the temporary suppression block for dirty events does not inadvertently affect state consistency during op replay. Verify that updateDocumentDirtyState fully replicates the intended behavior of the removed logic.
// Replaying is an internal operation and we don't want to generate noise while doing it.
| @@ -2832,6 +2819,9 @@ export class ContainerRuntime | |||
| localOpMetadata?: unknown; | |||
| }[] = this.pendingStateManager.processInboundMessages(inboundResult, local); | |||
|
|
|||
| // This message could have been the last pending local (dirtyable) message, in which case we need to update dirty state to "saved" | |||
There was a problem hiding this comment.
Review the revised placement and frequency of updateDocumentDirtyState calls to ensure that dirty state transitions remain consistent across all scenarios.
Copilot uses AI. Check for mistakes.
|
Converting to a draft, not expecting to prioritize this before going on Leave. Will finish the tests and pick it up in August. |
Description
Some follow-ups to #24646:
updateDocumentDirtyStatewhen processing a non-modern-runtime message, since we don't even check for its presence in PendingStateManagerreplayPendingStates. See commit message on feef9a1 for detailsTODO: