Summary

This PR adds callable on_change callback support to st.popover, including optional args/kwargs, while preserving the existing "ignore" and "rerun" behavior. The backend wiring in lib/streamlit/elements/layouts.py is consistent with existing widget callback patterns, and coverage was expanded with Python unit tests, typing tests, and e2e scenarios.

Code Quality

The implementation is clean and localized:

• st.popover now accepts on_change as "ignore" | "rerun" | callable and registers callback metadata via register_widget when callable.
• Stateful behavior is correctly derived from on_change != "ignore" and callback policy checks are enabled only for callable callbacks.
• Docstring updates are aligned with the new API contract and expected session state behavior.

No blocking code quality issues were identified.

Test Coverage

Coverage is strong for the changed behavior:

• Unit tests in lib/tests/streamlit/elements/layouts_test.py cover callback registration, open/close triggering, initial render behavior, args/kwargs plumbing, key/session-state interaction, and invalid on_change input.
• Typing tests in lib/tests/streamlit/typing/popover_container_types.py validate the widened API typing for callable callbacks and callback args/kwargs.
• E2E tests in e2e_playwright/st_popover_test.py verify callback firing on open/close, args/kwargs behavior, and fragment usage.

Overall, tests are adequate for merge.

Backwards Compatibility

No breaking change detected:

• Existing on_change="ignore" and on_change="rerun" paths remain intact.
• New parameters (args, kwargs) are additive and optional.
• Existing behavior for keyless popovers and state tracking semantics appears preserved.

Security & Risk

No new security concerns identified.
Regression risk is low and mainly limited to popover state/callback wiring; this is mitigated by the added unit and e2e coverage.

Accessibility

No frontend component logic was changed. Accessibility impact is minimal and neutral for this PR.

Recommendations

1. Consider adding one explicit test for callable on_change inside st.form to lock in the intended callback policy behavior in lib/streamlit/elements/layouts.py (around the check_widget_policies callback-rule gate).

Verdict

APPROVED: The change is well-scoped, backwards compatible, and sufficiently tested for the new popover callback functionality.

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

Summary

This PR adds on_change callback support to st.popover, allowing users to pass a callable (in addition to the existing "ignore" / "rerun" string literals). It also introduces optional args and kwargs parameters that are forwarded to the callback, consistent with the callback pattern used by other Streamlit widgets (st.checkbox, st.toggle, etc.). The callback fires on any popover state change (open or close), and users can inspect st.session_state[key] inside the callback to distinguish between open (True) and close (False).

Files changed:

• lib/streamlit/elements/layouts.py — Core implementation: widened on_change type, added args/kwargs params, wired callback through register_widget.
• lib/tests/streamlit/elements/layouts_test.py — 12 new unit tests covering callback registration, firing, args/kwargs, session state, and error handling.
• lib/tests/streamlit/typing/popover_container_types.py — Typing tests for the new parameter signatures.
• e2e_playwright/st_popover.py — E2E app script with callback scenarios.
• e2e_playwright/st_popover_test.py — 3 new E2E tests.

Code Quality

The implementation is clean and follows existing patterns well:

• Validation logic (layouts.py:1318): if not callable(on_change) and on_change not in {"ignore", "rerun"} correctly gates both string literals and callables before falling through to error.
• Stateful check (layouts.py:1324): is_stateful = on_change != "ignore" is a clean simplification — both "rerun" and callable values enable state tracking.
• Conditional callback forwarding (layouts.py:1333, 1336, 1362): The repeated callable(on_change) guard ensures the handler is only passed when it's actually a function. This is correct.
• Docstring (layouts.py:1240–1269): Well-written Numpydoc-style documentation that clearly explains the three modes and the new args/kwargs parameters.

Minor style items:

• Imports inside test functions (layouts_test.py): WidgetState, WidgetStates, and get_script_run_ctx are imported inside individual test functions (lines 913, 926, 948, 988, 1009). Per lib/tests/AGENTS.md, "Imports should be at the top-level of the test file. Only use imports inside test functions when there is a specific reason." These proto/state imports don't appear to have circular-import constraints and are used by 6+ tests — moving them to the top level would be cleaner.
• StreamlitValueError message (layouts.py:1320): The value "a callback function" is unquoted while the others ('rerun', 'ignore') are quoted. The resulting message reads fine ("Supported values: 'rerun', 'ignore', a callback function."), so this is cosmetic.

Test Coverage

Test coverage is strong:

Unit tests (12 new tests):

• Callback enables state tracking, sets proto ID, registers metadata
• Callback fires on state change (open), fires on both open and close
• Callback does not fire on initial render (important anti-regression)
• Args and kwargs forwarded correctly
• Callback with key accessible via session state
• Callback without key still works
• Invalid on_change value raises StreamlitAPIException

E2E tests (3 new tests):

• test_popover_callback_fires_on_open_and_close — open/close cycle with counter verification and "must NOT happen" check (content not visible when closed)
• test_popover_callback_with_args_kwargs — verifies args/kwargs reach the callback
• test_popover_callback_in_fragment — callback inside @st.fragment

Typing tests:

• Covers all three on_change modes, callback with args/kwargs, and callback with/without key.

One observation per E2E AGENTS.md guidance ("Prefer a single aggregated scenario test over many micro-tests when they share the same setup and page state"): test_popover_callback_with_args_kwargs is small and could be merged into test_popover_callback_fires_on_open_and_close to reduce browser loads, though this is a minor optimization concern.

Missing coverage (non-blocking): There is no test verifying that passing args or kwargs when on_change="rerun" is either rejected or documented as a no-op. Currently these values would be silently stored in widget metadata without being used. This is consistent with how register_widget works elsewhere, but could be confusing for users.

Backwards Compatibility

No breaking changes. This is purely additive:

• on_change retains its default "ignore", so all existing code continues to work.
• args and kwargs are new optional keyword-only parameters with None defaults.
• The type of on_change is widened from Literal["ignore", "rerun"] to Literal["ignore", "rerun"] | WidgetCallback, which is a non-breaking type expansion.
• The popover count in the snapshot test is correctly updated from 18 to 21 to reflect the 3 new popover elements.

Security & Risk

No security concerns. Callbacks are user-provided functions that execute in the same Python process, consistent with how all other widget callbacks work. No new attack surface is introduced.

The risk of regression is low:

• The is_stateful logic change from on_change == "rerun" to on_change != "ignore" is correct and well-tested.
• The previously-TODO'd check_widget_policies and enable_check_callback_rules are now properly set, which is an improvement over the previous placeholder state.

Accessibility

No frontend changes are included in this PR. The feature is entirely backend (Python callback plumbing), so there are no accessibility implications.

Recommendations

1. Move repeated imports to top-level in layouts_test.py: WidgetState, WidgetStates (from streamlit.proto.WidgetStates_pb2), and get_script_run_ctx (from streamlit.runtime.state.session_state) are imported inside 6+ test functions. Moving them to the top of the file would follow the AGENTS.md guidance and reduce duplication.

2. Consider validating args/kwargs when on_change is not callable: Currently, st.popover("label", on_change="rerun", args=("x",)) silently ignores the args. A warning or error would help prevent user confusion. This is not a blocker since the behavior is consistent with how register_widget handles these internally.

3. Consider consolidating E2E tests: test_popover_callback_with_args_kwargs is small enough to merge into test_popover_callback_fires_on_open_and_close, saving a browser load per E2E AGENTS.md guidance.

Verdict

APPROVED: Clean, well-tested feature addition that extends st.popover with callback support following established widget patterns. Minor import and test consolidation suggestions are non-blocking.

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