Fix ghost tabs issue in fragments (streamlit#9186)

## Describe your changes

Closes streamlit#9158

So far we seemed to have missed the case where we can have stale widgets
in the same fragment we are currently running which stem from a previous
run. This PR closes this gap by removing widget nodes that belong to the
current fragment but an old run.
It looks like the reason why this is happening for tabs (and probably
other containers) and not for other elements is that in for a tab delta,
the `scriptRunId` is updated to the current run, so the existing pruning
logic does not work; the new approach also considers the child's
`scriptRunId`!

## GitHub Issue Link (if applicable)

## Testing Plan

- Explanation of why no additional tests are needed
- Unit Tests (JS and/or Python)
  - Extended unit test to simulate this issue
- E2E Tests
  - Add e2e test with example app from streamlit#9158
- Any manual testing needed?

---

**Contribution License Agreement**

By submitting this pull request you agree that all contributions to this
project are made under the Apache 2.0 license.

e2e_playwright/st_fragment_dynamic_containers.py

@@ -0,0 +1,28 @@
+# Copyright (c) Streamlit Inc. (2018-2022) Snowflake Inc. (2022-2024)
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import streamlit as st
+
+
+@st.fragment
+def show_page():
+    checkmark = st.checkbox("Yes or No")
+
+    if checkmark:
+        st.tabs(["Tab A", "Tab B", "Tab C"])
+    else:
+        st.tabs(["Tab 1", "Tab 2"])
+
+
+show_page()

e2e_playwright/st_fragment_dynamic_containers_test.py

@@ -0,0 +1,52 @@
+# Copyright (c) Streamlit Inc. (2018-2022) Snowflake Inc. (2022-2024)
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from playwright.sync_api import Page, expect
+
+from e2e_playwright.shared.app_utils import click_checkbox
+
+
+def _expect_numeric_tabs(app: Page):
+    tabs = app.get_by_test_id("stTabs")
+    expect(tabs).to_have_count(1)
+    tab_buttons = tabs.locator("button")
+    expect(tab_buttons).to_have_count(2)
+    expect(tab_buttons.nth(0)).to_have_text("Tab 1")
+    expect(tab_buttons.nth(1)).to_have_text("Tab 2")
+
+
+def _expect_letter_tabs(app: Page):
+    tabs = app.get_by_test_id("stTabs")
+    expect(tabs).to_have_count(1)
+    tab_buttons = tabs.locator("button")
+    expect(tab_buttons).to_have_count(3)
+    expect(tab_buttons.nth(0)).to_have_text("Tab A")
+    expect(tab_buttons.nth(1)).to_have_text("Tab B")
+    expect(tab_buttons.nth(2)).to_have_text("Tab C")
+
+
+def test_correct_tabs_are_shown_and_no_ghost_tabs(app: Page):
+    """When we render a different amount of tabs, we want the
+    correct tabs to show and no tabs from the previous fragment
+    run (see issue https://github.com/streamlit/streamlit/issues/9158).
+    """
+    _expect_numeric_tabs(app)
+
+    # Ensure that this works for multiple runs
+    for _ in range(10):
+        click_checkbox(app, "Yes or No")
+        _expect_letter_tabs(app)
+
+        click_checkbox(app, "Yes or No")
+        _expect_numeric_tabs(app)

frontend/lib/src/AppNode.test.ts

@@ -1064,6 +1064,20 @@ describe("AppRoot.clearStaleNodes", () => {
   })
 
   it("handles currentFragmentId correctly", () => {
+    const tabContainerProto = makeProto(DeltaProto, {
+      addBlock: { tabContainer: {}, allowEmpty: false },
+      fragmentId: "my_fragment_id",
+    })
+    const tab1 = makeProto(DeltaProto, {
+      addBlock: { tab: { label: "tab1" }, allowEmpty: true },
+      fragmentId: "my_fragment_id",
+    })
+    const tab2 = makeProto(DeltaProto, {
+      addBlock: { tab: { label: "tab2" }, allowEmpty: true },
+      fragmentId: "my_fragment_id",
+    })
+
+    // const BLOCK = block([text("1"), block([text("2")])])
     const root = AppRoot.empty(FAKE_SCRIPT_HASH)
       // Block not corresponding to my_fragment_id. Should be preserved.
       .applyDelta(
@@ -1124,6 +1138,22 @@ describe("AppRoot.clearStaleNodes", () => {
         }),
         forwardMsgMetadata([0, 1, 1])
       )
+      // New element container related to my_fragment_id, having children which will be handled individually
+      // Create a tab container with two tabs in the old session; then send new delta with the container and
+      // only one tab. The second tab with the old_session_id should be pruned.
+      .applyDelta(
+        "old_session_id",
+        tabContainerProto,
+        forwardMsgMetadata([0, 2])
+      )
+      .applyDelta("old_session_id", tab1, forwardMsgMetadata([0, 2, 0]))
+      .applyDelta("old_session_id", tab2, forwardMsgMetadata([0, 2, 1]))
+      .applyDelta(
+        "new_session_id",
+        tabContainerProto,
+        forwardMsgMetadata([0, 2])
+      )
+      .applyDelta("new_session_id", tab1, forwardMsgMetadata([0, 2, 0]))
 
     const pruned = root.clearStaleNodes("new_session_id", ["my_fragment_id"])
 
@@ -1136,6 +1166,12 @@ describe("AppRoot.clearStaleNodes", () => {
     expect(pruned.main.getIn([1])).toBeInstanceOf(BlockNode)
     expect((pruned.main.getIn([1]) as BlockNode).children).toHaveLength(1)
     expect(pruned.main.getIn([1, 0])).toBeTextNode("newElement!")
+
+    expect(pruned.main.getIn([2])).toBeInstanceOf(BlockNode)
+    expect((pruned.main.getIn([2]) as BlockNode).children).toHaveLength(1)
+    expect(
+      (pruned.main.getIn([2, 0]) as BlockNode).deltaBlock.tab?.label
+    ).toContain("tab1")
   })
 })
 

frontend/lib/src/AppNode.ts

@@ -506,6 +506,12 @@ export class BlockNode implements AppNode {
           return this
         }
 
+        // This blocks belong to our fragment, but it was modified in a previous script run.
+        // This means it is stale now!
+        if (this.scriptRunId !== currentScriptRunId) {
+          return undefined
+        }
+
         // If this BlockNode *does* correspond to a currently running fragment,
         // we recurse into it below and set the fragmentIdOfBlock parameter to
         // keep track of which fragment this BlockNode belongs to.
@@ -518,13 +524,13 @@ export class BlockNode implements AppNode {
 
     // Recursively clear our children.
     const newChildren = this.children
-      .map(child =>
-        child.clearStaleNodes(
+      .map(child => {
+        return child.clearStaleNodes(
           currentScriptRunId,
           fragmentIdsThisRun,
           fragmentIdOfBlock
         )
-      )
+      })
       .filter(notUndefined)
 
     return new BlockNode(