Summary

This PR fixes a regression where clearing a transient node (for example, a spinner) could restore a stale anchor element from a previous script run. The core change in frontend/lib/src/render-tree/visitors/ClearStaleNodeVisitor.ts now re-runs stale filtering on the anchor before restoration, and the test suite adds coverage for both current and stale anchor cases.

Code Quality

The implementation is minimal and targeted, with good locality and clear intent:

• frontend/lib/src/render-tree/visitors/ClearStaleNodeVisitor.ts:153 changes return node.anchor to return node.anchor.accept(this), which aligns behavior with the visitor's stale-node contract.
• frontend/lib/src/render-tree/visitors/ClearStaleNodeVisitor.ts:144-147 adds explanatory comments documenting the stale-anchor edge case.
• frontend/lib/src/render-tree/visitors/ClearStaleNodeVisitor.test.ts:270-295 adds explicit positive/negative test cases for current vs stale anchors.

No maintainability or pattern concerns found.

Test Coverage

Unit coverage for the modified behavior is good:

• The existing "anchor restored" test is preserved for a current-run anchor.
• A new regression test verifies stale anchors are dropped when a transient is cleared in the current run.

E2E coverage was not added in this PR. Given the change is in internal render-tree stale filtering and is now directly unit-tested at the decision point, this is acceptable; residual risk is low but non-zero for full UI flow interactions.

Backwards Compatibility

No public API changes or protocol changes were introduced. The behavior change is corrective (removing stale UI artifacts) and should be backwards compatible for users relying on expected rerun semantics.

Security & Risk

No security-sensitive surfaces were modified (no auth/session, transport, routing, storage, CSP/CORS, upload/asset handling, or dynamic code execution changes). Regression risk appears low and localized to transient-node cleanup behavior.

External test recommendation

• Recommend external_test: No
• Triggered categories: None
• Key evidence from changed files:
  • frontend/lib/src/render-tree/visitors/ClearStaleNodeVisitor.ts: internal render-tree visitor logic only; no external hosting/embedding/network boundary behavior touched.
  • frontend/lib/src/render-tree/visitors/ClearStaleNodeVisitor.test.ts: unit tests only, focused on stale-node filtering behavior.
• Suggested external test focus areas: None required for this patch.
• Confidence: High
• Assumptions and gaps: Assessment is based on the two-file diff only; no hidden runtime-path changes outside this PR.

Accessibility

No UI component markup, keyboard interaction, ARIA semantics, or focus behavior changed. No accessibility concerns identified.

Recommendations

1. Optional follow-up: add an e2e regression test reproducing the issue scenario (transient spinner + tab/section switching) to guard against future integration regressions across render-tree updates.

Verdict

APPROVED: The fix addresses the reported root cause with targeted code and solid unit regression coverage, with no blocking quality, compatibility, or security concerns.

This is an automated AI review by gpt-5.3-codex-high. Please verify the feedback and use your judgment.

Summary

This PR fixes a regression introduced in #13849 where stale elements from previous script runs could persist after spinner (transient node) completion. The root cause was that ClearStaleNodeVisitor.visitTransientNode() returned a transient node's anchor directly when the transient was cleared in the current run, without checking whether the anchor itself was stale. The fix changes return node.anchor to return node.anchor.accept(this) so the anchor is recursively validated for staleness.

Only two files are changed:

• frontend/lib/src/render-tree/visitors/ClearStaleNodeVisitor.ts (1 functional line change + 2 comment lines)
• frontend/lib/src/render-tree/visitors/ClearStaleNodeVisitor.test.ts (1 test corrected + 1 new test added)

Code Quality

The fix is minimal, surgical, and idiomatic. It leverages the existing visitor pattern — node.anchor.accept(this) delegates staleness checking to the same visitor that already handles ElementNode, BlockNode, and TransientNode, so the behavior is consistent regardless of anchor type. No new abstractions or complexity introduced.

The updated comments at lines 144-147 of ClearStaleNodeVisitor.ts clearly explain why the anchor must be checked for staleness, which is appropriate for non-obvious logic.

The test changes are well-structured:

• The existing test (line 270) was correctly updated — it previously asserted that a stale anchor ("old_run") would be returned, which was actually validating the buggy behavior. It now uses currentRunId for the anchor, testing the valid case.
• The new test (line 281) explicitly verifies the bug fix scenario: a stale anchor is properly filtered out, returning undefined.

Test Coverage

Unit tests: The new test at line 281-295 directly covers the regression scenario. Together with the corrected existing test, both positive (current anchor restored) and negative (stale anchor filtered) paths are covered for the early-return branch.

E2E tests: The existing E2E test test_simple_transient_spinner_does_not_leave_stale_elements in e2e_playwright/appnode_hierarchy_test.py exercises the exact user-facing scenario from issue #14249 — a spinner wrapping content, followed by a rerun without a spinner, verifying no stale elements remain. Additional E2E tests cover complex and chat-based transient spinner scenarios. No new E2E tests are needed.

The test coverage is adequate for this change.

Backwards Compatibility

This change is fully backwards compatible. The previous behavior (returning stale anchors) was a bug, not a feature. There are no API changes, no protobuf changes, and no changes to the public st.* interface. The fix restores the correct semantic: stale elements should never survive a stale-clearing pass.

Security & Risk

No security concerns. The change is entirely within the frontend render tree visitor logic and does not touch:

• WebSocket handling, authentication, or session management
• File upload/download, asset serving, or path traversal
• Cookies, XSRF, CORS, or security headers
• External dependencies, runtime JS execution, or command injection
• HTML/Markdown rendering or sanitization

Regression risk is low. The accept(this) call delegates to the same well-tested visitor methods (visitElementNode, visitBlockNode, visitTransientNode) that handle all other nodes in the tree. The only behavioral change is in the specific code path where a transient node is cleared in the current run and has an anchor — previously the anchor was returned unchecked, now it is validated.

External test recommendation

• Recommend external_test: No
• Triggered categories: None
• Evidence:
  • ClearStaleNodeVisitor.ts: Internal render tree visitor logic only; no routing, auth, embedding, asset serving, CORS, storage, or security header changes.
  • ClearStaleNodeVisitor.test.ts: Unit test changes only.
• Suggested external_test focus areas: N/A
• Confidence: High
• Assumptions and gaps: None. This is a pure frontend render tree management fix with no external integration surface.

Accessibility

No UI changes, no new components, and no DOM structure modifications. This fix only affects which render tree nodes survive the stale-clearing pass. Accessibility is not impacted.

Recommendations

No changes requested. The fix is correct, minimal, well-tested, and follows existing patterns.

Verdict

APPROVED: A clean, minimal one-line fix for a regression in transient node staleness checking, with proper unit test coverage and existing E2E test coverage for the user-facing scenario.

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

Summary

This PR fixes a stale-render regression in transient-node cleanup by ensuring a cleared transient's anchor is also passed through stale-node filtering before restoration. The implementation change in frontend/lib/src/render-tree/visitors/ClearStaleNodeVisitor.ts is small and focused, and the accompanying test updates in frontend/lib/src/render-tree/visitors/ClearStaleNodeVisitor.test.ts cover both the reported stale-anchor case and a fragment-run edge case.

Code Quality

The code change is precise and follows existing visitor patterns: replacing direct anchor return with node.anchor.accept(this) ensures consistent staleness handling across node types. Test additions are clear, scenario-driven, and improve maintainability by documenting expected transient/fragment behavior.

No code quality issues were identified.

Test Coverage

Unit coverage is strong for this fix:

• Adds a regression test for a stale anchor on a cleared transient (expects undefined).
• Preserves behavior coverage for current-run anchor restoration.
• Adds explicit fragment-run coverage where an anchor without fragmentId should remain preserved.

No e2e tests were added. Given the tightly scoped render-tree visitor logic and direct unit-level regression coverage, this is acceptable for this PR, though a future user-flow e2e around spinner clear behavior could further harden against integration regressions.

Backwards Compatibility

No public API or protocol changes are introduced. Behavior change is corrective: stale anchors that were previously (incorrectly) retained are now filtered, aligning with expected stale-node semantics. This should be backwards compatible for users and reduces unintended stale UI persistence.

Security & Risk

No security-sensitive surfaces were modified (no auth/session, websocket transport, routing, cookies/XSRF, asset serving, external networking, or dynamic code execution changes). Regression risk appears low due to:

• Minimal logic delta in a single branch.
• Added targeted unit tests for both main regression and edge-case behavior.

External test recommendation

• Recommend external_test: No
• Triggered categories: None
• Evidence:
  • frontend/lib/src/render-tree/visitors/ClearStaleNodeVisitor.ts: Internal render-tree stale filtering logic only; no routing/auth/websocket/embed/cross-origin/storage/security-header paths changed.
  • frontend/lib/src/render-tree/visitors/ClearStaleNodeVisitor.test.ts: Unit-test-only additions validating transient/fragment stale-node behavior.
• Suggested external_test focus areas:
  • No additional @pytest.mark.external_test coverage is required for this branch.
  • Optional future hardening: a non-external e2e regression flow around spinner/transient clear to validate end-user behavior.
• Confidence: High
• Assumptions and gaps: Assessment is based on the PR diff (two frontend files) and assumes no related runtime behavior changes exist outside this branch.

Accessibility

No accessibility-impacting UI markup, interaction handlers, semantics, focus behavior, or styling were changed. A11y risk is effectively none for this PR scope.

Recommendations

1. Optional: add a focused e2e regression test for spinner/transient anchor cleanup in a realistic app flow to complement the existing unit coverage.

Verdict

APPROVED: The fix is targeted, regression-tested at unit level, low risk, and ready to merge.

This is an automated AI review by gpt-5.3-codex-high. Please verify the feedback and use your judgment.

Summary

This PR fixes a regression introduced in #13849 where stale elements from previous script runs could persist after spinner (transient node) completion. The root cause was in ClearStaleNodeVisitor.visitTransientNode: when a transient node was explicitly cleared in the current run, its anchor was restored directly via return node.anchor without checking whether the anchor itself was stale. The fix changes this to return node.anchor.accept(this) so the anchor passes through the same staleness-checking visitor logic.

The change is a single-line fix in ClearStaleNodeVisitor.ts with three well-targeted unit tests.

Code Quality

The fix is minimal, precise, and follows the established visitor pattern already used elsewhere in the same file. Specifically:

• Line 153 of ClearStaleNodeVisitor.ts: return node.anchor.accept(this) is consistent with how the anchor is handled in the non-early-return path (line 157: const anchorNode = node.anchor?.accept(this)). The early-return path was the only place that skipped the staleness check.
• The updated comment on lines 146-147 clearly explains the reasoning behind the additional check.
• The test file changes are well-structured with descriptive test names and useful inline comments explaining each scenario.

The pre-existing test ("restores anchor for transient cleared in current run") was correctly updated to use currentRunId for the anchor, since the old test was inadvertently relying on the buggy behavior (returning a stale anchor without checking).

Test Coverage

Unit tests are good and cover the critical code paths:

1. Happy path (line 270): Anchor from the current run is correctly restored when transient is cleared.
2. Bug fix path (line 281): Stale anchor is correctly filtered out (returns undefined) when transient is cleared — this is the core regression test.
3. Fragment edge case (line 297): During a fragment run, a stale anchor without a fragmentId is preserved (not cleared), since visitElementNode intentionally keeps such elements during fragment runs.

E2E test coverage: While the existing st_spinner_test.py has extensive tests for spinner scenarios (including fragments, containers, and tabs), it does not have a test that exactly reproduces the #14249 scenario (segmented control switching + cached function with spinner between tab creation and content writing). However, this is a minor gap — the unit tests directly cover the buggy code path, and the existing e2e suite covers the broader spinner/transient behavior. Adding a dedicated e2e test for this specific scenario would further guard against future regressions in this area, but is not a blocker.

Backwards Compatibility

This change is fully backwards compatible. The previous behavior was a bug — stale elements were unintentionally preserved. Users affected by the regression (#14249) will see the correct behavior restored: elements from previous script runs will no longer "leak through" after spinner completion. There is no API surface change.

Security & Risk

No security concerns. The change is entirely within frontend render-tree visitor logic. It does not touch:

• WebSocket/server endpoints, authentication, or session management
• File handling, asset serving, or path traversal
• CORS, CSP, XSRF, or cookie handling
• HTML/Markdown rendering or sanitization
• External services or dependencies
• Runtime JavaScript execution (eval, Function(), etc.)

Regression risk is low. The fix makes the early-return path consistent with the general path that already calls node.anchor.accept(this). The only behavioral change is that stale anchors are now correctly filtered out instead of being incorrectly preserved.

External test recommendation

Recommendation: No — @pytest.mark.external_test coverage is not needed.

• Triggered categories: None
• Key evidence: The change is purely in the frontend render-tree visitor (ClearStaleNodeVisitor.ts), which is internal logic for managing the DOM render tree. It does not involve external services, network requests, embedding, or any infrastructure that differs between local and external deployment.
• Confidence: High. The code path is entirely client-side and has no external dependencies.

Accessibility

No accessibility impact. The change only affects which elements are included in or excluded from the render tree (stale element cleanup). It does not modify any UI components, ARIA attributes, focus management, or keyboard handling.

Recommendations

No blocking issues. One minor suggestion:

1. Consider adding an e2e test for the exact reproduction scenario from UI element appearing unexpectedly since v1.55.0 #14249 (segmented control + cached function with spinner + tabs). This would serve as an end-to-end regression guard for this specific user-reported issue. The app script would look similar to the reproduction code in the issue. However, this is optional since the unit tests directly cover the buggy code path.

Verdict

APPROVED: Clean, minimal one-line bug fix that correctly applies staleness checking to transient node anchors, with well-targeted unit tests covering the fix and relevant edge cases.

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