Summary

This PR fixes an issue where bound query parameters (bind="query-params") are lost from the URL when navigating between pages in a multi-page app (MPA). It ensures that the widget's value is preserved in the session state across page transitions and properly restored to the URL when the widget is remounted. Additionally, it normalizes boolean query parameters to use lowercase true/false instead of True/False to better align with standard URL conventions.

Code Quality

The code changes are well-structured, clean, and well-documented.

• In lib/streamlit/runtime/state/session_state.py, the logic to preserve bound widget values under their user keys before _remove_stale_widgets filters them out is a clever and robust solution.
• The addition of _query_param_bound_widget_ids to track bound-intent across page transitions is implemented safely with proper cleanup in clear() and pruning in _remove_stale_widgets().
• The URL sync logic in register_widget is carefully guarded to only trigger for values explicitly preserved during remounts, avoiding conflicts with programmatic session state updates.

Test Coverage

The changes are excellently tested.

• E2E Tests: A new test test_bound_widget_query_param_restored_after_page_switch in e2e_playwright/multipage_apps/mpa_basics_test.py thoroughly verifies the core issue (navigating away and back restores the URL param) and also checks that default-value collapsing remains intact.
• Unit Tests: Extensive unit tests have been added in lib/tests/streamlit/runtime/state/session_state_test.py (e.g., PreserveBoundWidgetValuesTest, RegisterWidgetUrlSyncTest, ConditionalRemountBoundBehaviorTest) to cover the new logic in _remove_stale_widgets and register_widget. query_params_test.py is also updated.

Backwards Compatibility

The PR introduces a slight behavioral change for boolean query parameters:

• Before: Bound boolean widgets (like st.checkbox or st.toggle) would write True or False to the URL.
• After: They will write true or false.
  This is a positive change that aligns with standard web conventions. However, if any users were manually reading st.query_params.get("my_checkbox") and strictly comparing it to "True", their code might need a minor update. Given that bind="query-params" is a very recent feature, this is an acceptable and necessary correction.

Security & Risk

No security concerns or regression risks identified. The changes are confined to state management and URL synchronization for bound widgets.

External test recommendation

• Recommend external_test: No
• Triggered categories: None
• Evidence:
  • lib/streamlit/runtime/state/query_params.py: Changes only affect how query parameters are synced to the URL string (e.g., lowercasing booleans).
  • lib/streamlit/runtime/state/session_state.py: Changes only affect internal state preservation (_old_state) during MPA page transitions and re-syncing to the URL.
• Suggested external_test focus areas: N/A
• Confidence: High
• Assumptions and gaps: The changes do not touch core routing, auth, embedding boundaries, or asset serving. They only affect the query string of the frontend URL, which is already handled by existing mechanisms.

Accessibility

No frontend changes were made in this PR, so accessibility is not impacted.

Recommendations

The PR is solid and ready to be merged. Great job on the detailed comments explaining the "why" behind the state management logic!

Verdict

APPROVED: The changes elegantly fix the MPA query param bug with comprehensive test coverage and no apparent risks.

This is an automated AI review by gemini-3.1-pro. Please verify the feedback and use your judgment.

Summary

This PR fixes a real multipage regression where query-param-bound widget values persist in session state but disappear from the URL after navigating away and back. The implementation preserves bound stale widget values across page transitions and restores missing non-default URL params during remount, while keeping default-value URL collapsing behavior.

Code Quality

The changes are well-structured and localized:

• lib/streamlit/runtime/state/session_state.py adds durable bound-intent tracking and remount URL resync logic without widening public API surface.
• lib/streamlit/runtime/state/query_params.py adds focused helpers (has_param, discard_param_no_forward_msg) and a clearer correction API (set_corrected_value, with private alias retained for compatibility).
• Tests are organized around behavior-level scenarios (stale cleanup preservation, remount sync, programmatic session-state exceptions), which makes intent and regression coverage clear.

No blocking code-quality issues identified.

Test Coverage

Coverage is strong for this scope:

• Python unit tests in lib/tests/streamlit/runtime/state/session_state_test.py exercise the key new edge cases: stale cleanup ordering, remount restoration, default collapse, and exclusion of programmatic session-state sets.
• Python unit tests in lib/tests/streamlit/runtime/state/query_params_test.py validate helper semantics and bool URL normalization.
• E2E coverage in e2e_playwright/multipage_apps/mpa_basics_test.py now verifies round-trip multipage restore behavior plus a negative check for default-value collapse.

Given the feature scope, unit + e2e coverage is adequate.

Backwards Compatibility

Backwards compatibility looks good:

• No public API removals.
• Internal _set_corrected_value behavior is preserved through a compatibility alias.
• URL bool formatting now normalizes to lowercase (true/false) for corrected values; this is standards-aligned and should be non-breaking for consumers.

Security & Risk

No direct security issues found in this diff.

• No changes to auth, cookies, CSRF/XSRF, websocket handshake, headers, asset serving, or external network calls.
• No unsafe dynamic code execution patterns introduced.
• Main risk is behavioral regression in URL/session synchronization logic; added tests substantially reduce that risk.

External test recommendation

• Recommend external_test: Yes
• Triggered categories: 1. Routing and URL behavior
• Evidence:
  • lib/streamlit/runtime/state/session_state.py: adds URL restore/removal behavior during widget registration and stale cleanup for multipage transitions.
  • lib/streamlit/runtime/state/query_params.py: modifies backend query-param cache/update semantics that affect URL synchronization behavior.
  • e2e_playwright/multipage_apps/mpa_basics_test.py: new multipage URL restoration scenario indicates user-visible URL behavior changed.
• Suggested external_test focus areas:
  • Validate bound query-param restore across multipage navigation when app is externally hosted behind a non-root base path/reverse proxy.
  • Validate the same flow in iframe-embedded mode to ensure URL updates and default-value collapse remain consistent.
• Confidence: Medium
• Assumptions and gaps: Assessment is based on static diff review only (no runtime execution in external modes), and current coverage does not include @pytest.mark.external_test for this scenario.

Accessibility

No frontend UI/component changes were introduced in this PR; accessibility impact is minimal and no new a11y issues were identified from the changed files.

Recommendations

1. Add one focused @pytest.mark.external_test for the multipage bound query-param restore flow (external app and/or embedded iframe mode), since URL behavior changed and could diverge under proxy/embed environments.

Verdict

APPROVED: The fix is well-targeted, regression-tested, and appears safe to merge, with external-mode coverage recommended as a follow-up hardening step.

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 bug where widget-bound query parameters (bind="query-params") are lost in the URL when navigating away from and back to a page in a multi-page app (MPA), even though the widget's value persists in session state. The fix addresses two root causes: (1) _remove_stale_widgets deleting bound widget values during page transitions, and (2) register_widget lacking a code path to restore a missing URL parameter from a persisted non-default value.

Key changes:

• Value preservation (session_state.py): A durable _query_param_bound_widget_ids set tracks bound intent across page transitions. Before stale cleanup, bound keyed widget values are saved under their user keys (which survive the element-ID filter).
• URL sync (session_state.py): After value resolution in register_widget, the URL parameter is restored when bind="query-params" is active, the param is missing, the value is non-default, and no programmatic st.session_state set occurred. Default-value collapsing is preserved.
• Bool normalization (query_params.py): set_corrected_value writes true/false instead of True/False for bool_value types to match URL conventions.
• API cleanup (query_params.py): _set_corrected_value renamed to set_corrected_value (public), with a backward-compatible alias. New has_param() and discard_param_no_forward_msg() methods added.

Code Quality

The code is well-structured and follows existing Streamlit patterns. Comments clearly explain the "why" behind each change, which is valuable given the complexity of MPA state management.

Strengths:

• The _query_param_bound_widget_ids set is a clean solution for tracking bound intent across page transitions where bindings and metadata may be gone.
• The pruning logic (intersection_update(wid_key_map.keys())) prevents unbounded growth of the tracking set.
• The discard_param_no_forward_msg method correctly avoids unnecessary forward messages for backend-only cache cleanup.
• Multiple guard conditions in the URL sync block (register_widget lines 1050-1063) are well-designed to prevent incorrect syncing: checking non-default, user-key presence in _old_state, param absence, and no programmatic set.

Minor observations:

• The @patch added to the existing test_remove_stale_widgets (line 886 in session_state_test.py) is technically unnecessary since DeltaGeneratorTestCase already provides a real ScriptRunContext with fragment_ids_this_run=None. It's harmless but slightly inconsistent with the pre-existing test style for that class.
• The _set_corrected_value alias at query_params.py:711 is good for backward compatibility. Since no external callers exist in the codebase (only the new public name is used), the alias could be removed in a future cleanup pass.

Test Coverage

Test coverage is thorough and well-organized:

Unit tests (session_state_test.py):

• RemoveStaleWidgetsPreservationTest: Tests value preservation for bound widgets, non-preservation for unbound widgets (good negative check), and pruning of unmapped IDs.
• RegisterWidgetUrlSyncTest: Tests URL sync on remount, skip when param exists, skip for default values, removal of stale default params, skip on programmatic set, and skip for compacted programmatic sets. Each test isolates a specific condition branch.
• ConditionalRemountBoundBehaviorTest: Integration-style tests comparing bound vs. unbound remount behavior end-to-end.

Unit tests (query_params_test.py):

• Tests for has_param, discard_param_no_forward_msg, and bool normalization in set_corrected_value.

E2E test (mpa_basics_test.py):

• test_bound_widget_query_param_restored_after_page_switch: Full round-trip scenario (set param → navigate away → navigate back → verify param restored). Includes a negative check verifying default-value collapsing remains intact after round-trip navigation.

The tests follow the AGENTS.md guidelines well: docstrings on all test functions, anti-regression negative checks, appropriate use of expect with timeouts for E2E, and use of existing DeltaGeneratorTestCase infrastructure.

Backwards Compatibility

No breaking changes:

• _set_corrected_value preserved as a backward-compatible alias.
• _query_param_bound_widget_ids is a new internal field with no public API exposure.
• Bool normalization (True→true, False→false in URL) is non-breaking because parse_url_param already handles case-insensitive boolean parsing (val.lower()).
• Existing widget behavior (unbound widgets, fragment runs) is unaffected—the new logic only activates for bind="query-params" widgets.

Security & Risk

No security concerns identified:

• No new server endpoints, routes, or external dependencies.
• No changes to authentication, session management, CORS, CSP, or security headers.
• No file system operations, subprocess calls, or eval/exec usage.
• The discard_param_no_forward_msg method operates on an internal dict and doesn't expose new attack surface.
• URL parameter restoration is guarded by multiple conditions (non-default, user-key presence, no existing param, no programmatic set) that prevent unintended URL manipulation.

Accessibility

No frontend changes are included in this PR. All changes are to Python backend code and tests. No accessibility impact.

Recommendations

1. Consider removing the unnecessary @patch on the existing test_remove_stale_widgets test (session_state_test.py line 886). The DeltaGeneratorTestCase base class already provides a ScriptRunContext with fragment_ids_this_run=None via add_script_run_ctx. The patch works but is inconsistent with the test's pre-existing style and other tests in the same class that don't mock the context. If kept, a brief comment explaining why the override is preferred would be helpful.

2. Minor nit: The _set_corrected_value backward-compat alias at query_params.py:711 has no remaining callers in the codebase. Consider adding a # TODO: remove in a future release comment or simply removing it if internal-only.

Verdict

APPROVED: This is a well-crafted bug fix with thorough test coverage, clean implementation, and no backwards-compatibility or security concerns. The multi-layered approach (value preservation + URL sync + default collapsing) correctly addresses the root causes of the MPA query-param loss issue.

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