Skip to content

[feature] Add on_change="ignore" mode to st.pills - #17307

Merged
lukasmasuch merged 2 commits into
developfrom
lukasmasuch/on-change-ignore-pills-implement
Oct 7, 2026
Merged

lukasmasuch merged 2 commits into
developfrom
lukasmasuch/on-change-ignore-pills-implement

Conversation

@lukasmasuch

@lukasmasuch lukasmasuch commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Describe your changes

Adds on_change="ignore" to st.pills. Selecting, replacing, or clearing a pill updates the widget and any bound query parameter without rerunning the app. Python receives the value on the next rerun, for example after a button click.

GitHub Issue Link (if applicable)

Testing Plan

  • Python unit and typing tests cover accepted modes, validation, forms, identity, and return types.
  • Frontend unit tests cover single- and multi-select commits, rerun suppression, forms, and required selections.
  • E2E tests cover UI and query-parameter updates, deferred delivery to Python, and reload persistence.
  • No additional manual testing is required.

Contribution License Agreement

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

Selecting or clearing a pill can update the widget and bound query
params without rerunning. Python receives the value on the next rerun.
Copilot AI balanced review requested due to automatic review settings October 7, 2026 14:58
@lukasmasuch lukasmasuch added change:feature PR contains new feature or enhancement implementation impact:users PR changes affect end users labels Oct 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@lukasmasuch

Copy link
Copy Markdown
Collaborator Author

Added Agent Docs

@lukasmasuch lukasmasuch added ai-review If applied to PR or issue will run AI review workflow change:feature PR contains new feature or enhancement implementation impact:users PR changes affect end users labels Oct 7, 2026
@snyk-io

snyk-io Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

✅ Snyk checks have passed. No issues have been found so far.

Status Scan Engine Image Critical Image High Image Medium Image 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.

@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

✅ PR preview is ready!

Name Link
📦 Wheel file https://core-previews.s3-us-west-2.amazonaws.com/pr-17307/streamlit-1.65.0-py3-none-any.whl
📦 @streamlit/component-v2-lib Download from artifacts

@github-actions github-actions Bot removed the ai-review If applied to PR or issue will run AI review workflow label Oct 7, 2026
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

⚠️ AI Review Failed

The AI review job failed to complete. Please check the workflow run for details.

You can retry by adding the ai-review label again or manually triggering the workflow.

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

⚠️ AI Review Failed

The AI review job failed to complete. Please check the workflow run for details.

You can retry by adding the ai-review label again or manually triggering the workflow.

@greptile-apps

greptile-apps Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Adds on_change ignore mode to pills widget.

The changes since the previous review appear safe to merge.

What we checked:

  • Segmented control still rejects modes: No. validate_on_change_mode rejects the mode before the code can set ignore_rerun.
Summary

Adds on_change="ignore" to st.pills. Selections update in the browser without rerunning the app, and bound query parameters still update.

  • Python receives the buffered selection on the next rerun.
  • st.segmented_control remains callback-only.
  • Adds Python, frontend, typing, and end-to-end coverage.
  • Changes since the previous review shorten documentation and remove a redundant guard. No new actionable issues were found.
Diagram
sequenceDiagram
    participant User
    participant Browser
    participant Python
    User->>Browser: Change pills selection
    Browser->>Browser: Store selection and update bound URL
    Note over Browser,Python: No immediate rerun
    User->>Browser: Trigger another rerun
    Browser->>Python: Send buffered selection
    Python->>Python: Run app with updated value
Loading

Reviews (2) · Last reviewed commit: "Clarify pills on_change commit docs" · Reviewed by Greptile

@lukasmasuch lukasmasuch added the ai-final-review If applied to PR or issue will run AI review workflow with the latest generation of AI models. label Oct 7, 2026
@github-actions github-actions Bot removed the ai-final-review If applied to PR or issue will run AI review workflow with the latest generation of AI models. label Oct 7, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

This PR adds on_change="ignore" to st.pills, and makes "rerun" the explicit default. Selecting, replacing, or clearing a pill updates the widget immediately. A bound query parameter updates on that same commit. Python receives the buffered value on the next rerun. The implementation adds bool ignore_rerun = 18 on ButtonGroup, accepts "rerun" and "ignore" only for pills, and passes triggerRerun: false from the shared syncWithWidgetManager path. st.segmented_control stays callback-only.

All four expected reviews completed: gpt-5.6-sol-high, claude-opus-5-5-high, gemini-3.8-flash-high, and grok-4.7-xhigh. None failed. All four approved. They agree the change follows the merged on-change-modes spec, matches widgets that already ship this mode, stays backwards compatible, and has unit, typing, frontend, and e2e coverage. The one material disagreement is whether query-param binding should require @pytest.mark.external_test coverage. That is resolved in External test recommendation: it should not.

Product Alignment

This is a user-facing public API change: st.pills gains mode strings on on_change, the default moves from None to "rerun", and a pill click can update the UI and URL without a script rerun.

The behavior is covered by specs/2026-04-14-on-change-modes/product-spec.md, already on develop. All four reviewers treated that spec as approved, and the pills implementation matches it:

  • on_change accepts "rerun" (default), "ignore", None (alias of "rerun"), or a callback.
  • "ignore" updates the UI without a rerun and delivers the value on the next rerun.
  • With bind="query-params", the URL updates on commit, including before Python has the value.
  • Inside st.form, the mode is accepted and form batching still owns commit timing.
  • The docstring documents the modes, the form caveat, and the bound-URL caveat.

The spec also lists st.segmented_control. This PR leaves that command callback-only. Reviewers agreed this is an incremental rollout of an approved spec, consistent with how st.number_input and st.text_input landed, and that runtime rejection of mode strings on the shared helper avoids a half-working API. The parameter name, placement, default, and behavior match st.radio, st.multiselect, and st.checkbox, which satisfies "Same Name, Same Behavior" and "Patterns Are Sacred" in specs/AGENTS.md. There is no product decision here that needs a new spec.

Code Quality

Reviewers agreed the change is small and follows the existing widget pattern: validate_on_change_mode, a callback registered only when on_change is callable, and ignore_rerun set only for "ignore". The frontend flag lives on the single syncWithWidgetManager path, so click and keyboard selection share it. WidgetStateManager already buffers triggerRerun: false, skips the rerun, and still writes bound query params. Public st.segmented_control overloads stay WidgetCallback | None, and runtime validation rejects mode strings there.

Complexity is proportionate. There is no new abstraction, dependency, or state machine. The only code-level disagreement is the style == "pills" conjunct next to the proto assignment. claude-opus-5-5-high would drop it because validation already rejects mode strings for segmented control. grok-4.7-xhigh would keep it as a local guard on the shared helper. Either form is correct. It is an optional nit, noted inline, and it does not block merge.

Performance

All four reviewers found no meaningful performance regression, and the diff supports that.

  • Backend work is one mode check and a proto bool while creating a pills element. Element-id calculation, rerun, session state, and message serialization are unchanged. The mode is excluded from the element id, so switching modes does not remount an unkeyed widget. Proto3 omits the default false, so existing pills messages stay the same on the wire.
  • Frontend work is a conditional field on an existing widget commit. It adds no render, remount, effect, traversal, or serialization pass. WidgetStateManager itself is untouched.
  • Apps that opt into "ignore" rerun less often.

No hot-path benchmark is warranted.

Test Coverage

Coverage matches the other on_change="ignore" widgets, and all four reviewers judged it sufficient.

  • Python unit tests cover the proto flag for "ignore", "rerun", None, and a callback; invalid strings and non-callables; "ignore" inside a form; segmented control rejecting both mode strings; and the mode staying out of the unkeyed element id.
  • Typing tests cover single-select, multi-select, and required=True return types for the new literals.
  • Frontend tests cover select, deselect, multi add, and multi remove with triggerRerun: false and no rerun message; the default rerun path when the flag is false or omitted; form pending-change batching; and a required pill that does not write when the current selection is clicked again.
  • The e2e scenario checks that single and multi edits leave the run counter unchanged, update or clear bound query params, flush both values on a later button rerun, and restore them after reload.

grok-4.7-xhigh noted that keyboard activation and fragments do not have their own cases. Keyboard selection uses the same onSelectionChange handler the frontend tests already exercise, and sibling ignore-mode e2e tests do not require a fragment case. That gap is optional.

Backwards Compatibility

All four reviewers agreed the change is backwards compatible.

  • The default moves from None to "rerun". Both still rerun and do not run a callback, which is the signature change the merged spec describes. None remains accepted.
  • Existing callbacks behave as before.
  • The mode is excluded from the element id, so widget identity and saved state stay stable. A unit test covers that.
  • st.segmented_control keeps its signature and still rejects mode strings.
  • ignore_rerun defaults to proto3 false. An older frontend ignores the field, and a newer frontend treats a missing field as false.

Security & Risk

No security concerns. The diff adds no endpoints, dependencies, HTML rendering, postMessage handling, auth, cookies, uploads, or code execution. Bound query params continue through the existing URL sync, which writes the widget's selected option strings. The main regression risk is a pill click still rerunning, or a buffered value never arriving on the next rerun. Frontend and e2e tests cover both. Reviewers agreed regression risk is low because ButtonGroup changes behavior only when ignoreRerun is true, and the backend sets that flag only for pills.

External test recommendation

  • Recommend external_test: No
  • Triggered categories: None
  • Evidence:
    • lib/streamlit/elements/widgets/button_group.py: validates on_change and sets a proto bool for pills. No routes, auth, cookies, or session transport.
    • frontend/lib/src/components/widgets/ButtonGroup/ButtonGroup.tsx: passes triggerRerun: false into the existing widget-state commit. No handshake, embedding, storage, or cross-origin changes.
    • proto/streamlit/proto/ButtonGroup.proto: adds ignore_rerun on the existing button-group message.
    • e2e_playwright/st_pills_test.py: local interaction and query-param assertions. Query params are outside cookies, localStorage, and sessionStorage.
  • Suggested external_test focus areas:
    • None. Local unit and e2e coverage is the right layer for this widget behavior.
  • Confidence: High
  • Assumptions and gaps: gpt-5.6-sol-high recommended Yes under category 1 (Routing and URL behavior) because "ignore" can update a bound query parameter without a rerun, and the local e2e app is not deployed externally. The other three reviewers recommended No. Category 1 is hit when a change modifies Starlette routes, server.baseUrlPath, catch-alls, request methods, URL resolution, redirects, or status codes. This diff does none of that. URL updates stay on the existing WidgetStateManager query-binding path, which this PR does not edit and which other ignore-mode widgets already use. Category 10 covers cookies, localStorage, and sessionStorage. A query-parameter update is outside that category. The hosted and iframe behavior of query binding is a pre-existing path, so a new external test is a coverage preference, not a triggered risk.

Accessibility

No accessibility change. Pills still render as the existing React Aria toggle group (radio in single-select, checkbox in multi-select), with the same names, keyboard activation, focus behavior, and imperative aria-required. on_change="ignore" only changes whether that selection schedules a rerun. All four reviewers agreed.

Readability

Public docstrings follow the established on_change pattern used by st.radio and st.multiselect. Names such as on_change_callback, PillsOnChangeModeTest, and test_segmented_control_rejects_on_change_modes say what they do. The frontend comment on triggerRerun explains why the flag is a no-op inside a form. Two reviewers flagged the same pills docstring parenthetical ("activating the same selection change from the keyboard") and the repeated list under bind. That wording suggestion is inline, so it is not repeated here.

e2e_playwright/st_pills_test.py (test_pills_on_change_ignore, docstring at line 772): the docstring already states the user-visible outcome (no rerun, query params update, value arrives on the next rerun). gpt-5.6-sol-high proposed a shorter version. The current text is clear enough to keep.

frontend/lib/src/components/widgets/ButtonGroup/ButtonGroup.test.tsx: claude-opus-5-5-high noted that "passes triggerRerun: false when $label" names an implementation detail. A behavior name such as "does not rerun when $label" would read more directly. Optional.

PR title. [feature] Add on_change="ignore" mode to st.pills stands alone and matches [type] Description. All reviewers agreed. No rewrite.

PR description. The opening states the user-facing change and the default change. Reviewers differed on how clear the segmented-control bullet is. gemini-3.8-flash-high found it sufficient. claude-opus-5-5-high and grok-4.7-xhigh wanted the deferral reason in plain language, without "button-group proto".

Proposed replacement for that bullet:

- Leaves `st.segmented_control` callback-only for now. It shares the pills widget, so this change rejects mode strings there. Support for that command can follow in a separate PR.

gpt-5.6-sol-high also noted that the Testing Plan checkboxes do not say what each layer verifies, and "Any manual testing needed?" is still unchecked. Proposed replacement:

## Testing Plan

- Python unit and typing tests cover accepted modes, validation, forms, identity, and return types.
- Frontend unit tests cover single- and multi-select commits, rerun suppression, forms, and required selections.
- E2E tests cover UI and query-parameter updates, deferred delivery to Python, and reload persistence.
- No additional manual testing is required.

Recommendations

  1. Optionally replace the st.segmented_control bullet with the wording above so the deferral reason is plain, and track that command as follow-up against the spec's affected-widget list.
  2. Optionally replace the Testing Plan with the layer-by-layer checklist above and mark manual testing as not required.
  3. Optionally apply the inline docstring and style == "pills" nits. Neither changes behavior.

Verdict

APPROVED: All four reviews approve, the pills implementation matches the merged on-change-modes spec and the existing widget pattern, and the only disagreements are optional wording and an external-test preference that the checklist does not trigger.


This is an automated consolidated AI review by grok-4.7-xhigh, synthesizing reviews from gpt-5.6-sol-high, claude-opus-5-5-high, gemini-3.8-flash-high, and grok-4.7-xhigh. Please verify the feedback and use your judgment.

This review also includes 2 inline comment(s) on specific code lines.

Comment thread lib/streamlit/elements/widgets/button_group.py Outdated
Comment thread lib/streamlit/elements/widgets/button_group.py Outdated
Mode strings other than pills already fail validation, so the
ignore flag no longer repeats that style check.
@lukasmasuch
lukasmasuch merged commit 94ad3fe into develop Oct 7, 2026
39 checks passed
@lukasmasuch
lukasmasuch deleted the lukasmasuch/on-change-ignore-pills-implement branch October 7, 2026 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:feature PR contains new feature or enhancement implementation impact:users PR changes affect end users

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants