• fix: preserve active tab selection after spinner reruns #14023 👈 (View in Graphite)
• develop

This stack of pull requests is managed by Graphite. Learn more about stacking.

✅ 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!

Summary

This PR fixes a bug (#14018) where tabs lose their selection state when rendered after a st.spinner context. The root cause is that AppRoot.addBlock looks up the existing node at a delta path to determine whether to inherit children (preserving widget/React state across reruns). When a spinner wraps a tab container, the existing node is a TransientNode, not a BlockNode, so the child-inheritance check (instanceof BlockNode) fails and children are discarded — resetting the tab selection.

The fix adds a 4-line unwrap in addBlock: if the node at the delta path is a TransientNode, it accesses the underlying anchor (the original BlockNode) before the inheritance check. The change is accompanied by a unit test in AppRoot.test.ts and a regression E2E test in st_spinner_test.py.

Code Quality

The fix is minimal, well-placed, and follows existing patterns in the codebase.

• AppRoot.ts (lines 426–435): The variable rename from existingNode to existingNodeAtPath + the derived existingNode is clear and self-documenting. The comment on lines 430–431 concisely explains the "why." The nullish coalescing fallback (anchor ?? existingNodeAtPath) is a safe default: if anchor is undefined, the TransientNode is used as-is and will fail the instanceof BlockNode check, resulting in an empty children array — the correct behavior when there's no prior block to inherit from.

• AppRoot.test.ts (lines 431–504): The unit test directly reproduces the scenario (tab container → transient wrap → new tab container) and validates that children are inherited. It also verifies the tab labels on the inherited children, confirming the inheritance was correct. Follows existing test patterns in the same file.

• st_spinner.py / st_spinner_test.py: The E2E test app uses session_state to persist the spinner-before-tabs scenario across reruns, which is a realistic reproduction of the bug. The test uses role-based locators (get_by_role("tab", ...), get_by_role("spinbutton", ...)), expect for auto-wait assertions, and includes both positive and negative aria-selected checks.

Test Coverage

The changes are well-covered:

• Unit test verifies the core logic: when addBlock encounters a TransientNode wrapping a BlockNode of the same type, the children are inherited correctly. The test covers the full flow — creating tabs, wrapping in a transient, then replacing with a new tab container in a different script run.

• E2E test validates the end-to-end user experience: clicking a tab, interacting with a widget, triggering a rerun, and confirming the tab selection persists. This directly captures the user-reported regression.

• Both tests include negative assertions (tab_one is not selected when tab_two should be), which aligns with the project's testing guidelines.

Backwards Compatibility

No breaking changes. The fix only changes behavior when a TransientNode wraps a BlockNode that matches the incoming block type — a scenario that previously lost children (bug behavior). Non-transient nodes are unaffected (the ternary passes them through as-is). The fallback to existingNodeAtPath when anchor is undefined preserves existing behavior for anchorless transient nodes.

Security & Risk

No security concerns. This is a client-side render-tree state management fix with no impact on data handling, authentication, or network communication. The change is narrow in scope and low risk.

Accessibility

The fix is a net positive for accessibility: tab selection state is now correctly preserved across reruns when tabs follow a spinner context. This means keyboard/screen-reader users who select a tab won't have their selection unexpectedly reset after a rerun.

Recommendations

No blocking issues. Minor observations:

1. The E2E test uses time.sleep(1) in the app script (st_spinner.py line 108) which is necessary to reproduce the spinner scenario but contributes to test runtime. This is acceptable given the nature of the test.

2. Consider whether arrowAddRows in AppRoot.ts (line 522) could be similarly affected if a transient node wraps an element targeted by arrowAddRows. This seems unlikely in practice (data elements aren't typically wrapped by spinners), but worth noting as a theoretical edge case for future reference.

Verdict

APPROVED: Clean, minimal fix for a real user-reported regression with strong unit and E2E test coverage. The change is safe, backwards-compatible, and follows existing codebase patterns.

This is an automated AI review by opus-4.6-thinking.