✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

✅ PR preview is ready!

@cursor review

Summary

This PR fixes issue #10907 where elements from previous dialogs were showing up as stale content in different dialogs. The bug occurred when:

1. User opens Dialog A (with content like text inputs)
2. User dismisses Dialog A
3. User opens Dialog B (which may take time to render)
4. During the loading of Dialog B, content from Dialog A would briefly appear

The fix introduces a dialog identity check that prevents the frontend from inheriting children from an existing dialog block when replacing it with a different dialog.

Changes:

• Python Backend (lib/streamlit/elements/lib/dialog.py): Always compute and assign a stable element ID to dialogs based on their attributes (title, dismissible, width, on_dismiss)
• TypeScript Frontend (frontend/lib/src/render-tree/AppRoot.ts): When replacing a dialog block, check if the dialog identity (ID) has changed and clear children if so

Code Quality

Python Changes (dialog.py)

The refactoring is clean and follows good practices:

• The element_id computation was moved from being conditional (only when on_dismiss is activated) to always being computed
• The ID is now set on block_proto.id which propagates to the frontend
• Comments clearly explain the purpose: distinguishing dialogs and preventing stale content
• The existing widget registration logic continues to work as before

TypeScript Changes (AppRoot.ts)

The fix is minimal and targeted:

const isDialogWithDifferentIdentity =
  block.dialog &&
  existingNode.deltaBlock.dialog &&
  block.id !== existingNode.deltaBlock.id

if (!isDialogWithDifferentIdentity) {
  children = existingNode.children
}

Strengths:

• ✅ Only affects dialog blocks - all other block types continue with existing behavior
• ✅ Defensive null checks ensure both blocks must be dialogs with dialog data
• ✅ Clear comment explaining the fix and referencing issue st.dialog shows stale elements from previous dialog #10907
• ✅ Follows the existing pattern in addBlock for handling block children inheritance

Test Coverage

Frontend Unit Tests (AppRoot.test.ts)

Two comprehensive tests added:

1. removes a dialog block's children when replacing with a different dialog identity

   • Creates Dialog 1 with children
   • Replaces with Dialog 2 (different identity)
   • Verifies Dialog 2 has zero children

2. preserves a dialog block's children when replacing with the same dialog identity

   • Creates a dialog with children
   • Replaces with same dialog (same identity)
   • Verifies children are preserved

Both tests follow the existing patterns in the test file and provide good coverage of the new behavior.

E2E Test (st_dialog_test.py)

Test: test_switching_dialogs_does_not_show_stale_content

The test accurately reproduces the issue:

1. Opens "Fast Dialog" (contains text input and specific content)
2. Dismisses it via Escape key
3. Opens "Slow Dialog" (has 1-second delay before rendering)
4. Immediately verifies no Fast Dialog content appears
5. Waits for Slow Dialog content to load
6. Re-verifies no stale content from Fast Dialog

Best practices followed:

• ✅ Uses expect for auto-wait assertions
• ✅ Uses stable locators (get_by_test_id, get_by_text)
• ✅ Descriptive docstring explaining the test scenario and issue reference
• ✅ No unnecessary screenshots
• ✅ Tests the specific bug behavior, not implementation details

Backwards Compatibility

✅ Fully backwards compatible

• Existing dialogs will continue to work exactly as before
• The only behavioral change is that switching between different dialogs no longer shows stale content
• Same dialog (same identity) still preserves children as before, maintaining widget state
• No protobuf changes required - the existing optional string id = 12 field in Block.proto is utilized

Security & Risk

✅ No security concerns

• This is purely a UI rendering fix
• The change is scoped specifically to dialog block handling
• No sensitive data handling changes
• No network or authentication changes

Risk Assessment: Low

• The conditional logic is defensive with proper null checks
• Both "positive" and "negative" cases are tested
• The fix only applies when both the existing and new blocks are dialog types
• All other block types are unaffected

Recommendations

No changes are required. The implementation is clean, well-tested, and addresses the issue appropriately.

Minor observations (not blocking):

1. The E2E test uses time.sleep(1) in the slow dialog - this is intentional and necessary to reproduce the race condition where stale content would appear. This is acceptable for this specific test scenario.

2. The test could optionally verify that the dialog title changes as well, but the current assertions are sufficient to validate the fix.

Verdict

APPROVED: This is a well-implemented, properly tested bug fix that addresses issue #10907 with minimal, targeted changes. The fix is backwards compatible and poses no security or regression risks. Both the frontend and backend changes are clean and follow existing code patterns in the repository.

This is an automated AI review. Please verify the feedback and use your judgment.

That's amazing, thanks for fixing this, Lukas!!