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

@cursor review

Summary

This PR enables dynamic option changes for st.pills and st.segmented_control widgets without triggering a widget identity change or state reset. The implementation migrates from index-based widget values to string-based values (using formatted option strings), following the same pattern already implemented for st.selectbox, st.multiselect, and st.radio.

Key changes:

• Added raw_values field to ButtonGroup.proto for string-based value representation
• New _ButtonGroupSerde class for unified string-based serialization/deserialization
• Changed key_as_main_identity from {"options", "click_mode"} to just {"click_mode"} for pills/segmented_control
• Frontend updated to use setStringArrayValue and read from rawValues
• When current selection is not in the new options, the widget resets to the default value; otherwise, selection is preserved

Code Quality

The code quality is high and follows Streamlit's established patterns well:

Strengths:

• The implementation correctly mirrors the approach used for st.selectbox, st.multiselect, and st.radio, ensuring consistency across option-based widgets
• The _ButtonGroupSerde class is well-structured with clear separation between single and multi-select modes
• Good use of type hints throughout the Python code
• Frontend code properly uses useMemo and useCallback for performance optimization
• Helper functions (getFormattedOption, stringValuesToIndices, etc.) are well-named and have clear purposes

Minor observations:

1. In button_group.py lines 152-163, the _serialize_single method falls back to str(v) when format_func fails. While this is defensive, it could lead to subtle mismatches if a value can be formatted on one render but not another. Consider logging a warning in this case.

2. The feedback widget's format includes the button index ({icon} {content}|{button_idx}) to handle identical icons (stars), while pills/segmented_control use just the formatted label. This asymmetry is intentional and correct, but could benefit from a brief code comment explaining why.

Test Coverage

Test coverage is excellent and comprehensive:

Python Unit Tests (button_group_test.py):

• ✅ New TestButtonGroupSerde class thoroughly tests the serde for single/multi modes, serialization, deserialization, defaults, and unknown values
• ✅ Tests for ID stability with different parameters (options change without ID change, selection_mode change with ID change)
• ✅ Proper use of @parameterized.expand for test variations
• ✅ Includes negative assertions (e.g., testing that options change does NOT change ID)

Frontend Unit Tests (ButtonGroup.test.tsx):

• ✅ Updated to verify setStringArrayValue calls with correct string format
• ✅ Tests both single and multi-select modes with string values
• ✅ Tests rawValues proto field usage
• ✅ Uses proper RTL queries and userEvent

E2E Tests (st_pills_test.py, st_segmented_control_test.py):

• ✅ Tests both key scenarios: selection reset when value removed, selection preserved when value exists
• ✅ Excellent docstrings explaining test scenarios
• ✅ Proper use of expect for auto-wait assertions
• ✅ Key-based locators via get_element_by_key
• ✅ Negative assertions (expect(dynamic_pills).not_to_contain_text("Banana"))
• ✅ Visual snapshot testing for before/after states

Backwards Compatibility

Analysis:

• The protobuf change adds a new raw_values field (field 14) - this is a safe, additive change
• The existing value field (field 7) remains but is no longer actively used by the frontend
• The default field continues to use index-based values for initial state

Risk Assessment:

• Old frontend + New backend: Old frontend would read from the value field which is no longer populated. Widgets would fall back to defaults. This is acceptable since Streamlit requires matching frontend/backend versions.
• New frontend + Old backend: New frontend reads from raw_values which old backend doesn't populate. Widgets fall back to defaults.
• Both new: Works as intended with dynamic options support.

Recommendation: The value field in ButtonGroup.proto should have a deprecation comment added:

// DEPRECATED: Use raw_values instead for string-based value handling.
// Kept for backwards compatibility with older frontends.
repeated uint32 value = 7 [deprecated=true];

Security & Risk

No security concerns identified. The changes are well-contained to the button group widget implementation and follow established patterns in the codebase.

Regression risks are minimal:

• The change to key_as_main_identity only affects pills/segmented_control (not feedback), which is correct
• String-based serialization has been battle-tested in selectbox/multiselect/radio
• Session state updates use reset_state_value to avoid the "cannot be modified after widget instantiated" error

Recommendations

1. Consider adding deprecation marker to proto field: Add [deprecated=true] and a comment to the value field in ButtonGroup.proto to signal it's no longer the preferred approach. This is a minor documentation improvement.

2. Documentation note: Consider adding a note to the widget docstrings mentioning that when using dynamic options with a key, selection is preserved based on the formatted label string. If format_func changes such that the same option has a different label, selection may be lost.

3. Minor code comment: In button_group.py around line 488-501, consider adding a brief comment explaining why feedback uses a different format string (with index) compared to pills/segmented_control - to handle identical icons (stars).

Verdict

APPROVED: This is a well-implemented feature that follows established Streamlit patterns, has comprehensive test coverage across all testing layers (unit, frontend, e2e), and poses minimal backwards compatibility risk. The code quality is high and the changes are well-documented through tests. The minor recommendations above are suggestions for improvement but do not block merging.

This is an automated AI review using opus-4.5-thinking. Please verify the feedback and use your judgment.

📉 Python coverage change detected

The Python unit test coverage has decreased by 0.0407%

• Current PR: 93.0444% (23420 statements, 1629 missed)
• Latest develop: 93.0851% (23341 statements, 1614 missed)

✅ Coverage change is within normal range.

📊 View detailed coverage comparison

📉 Frontend coverage change detected

The frontend unit test (vitest) coverage has decreased by 0.0100%

• Current PR: 86.4500% (13862 lines, 1877 missed)
• Latest develop: 86.4600% (13843 lines, 1874 missed)

✅ Coverage change is within normal range.

📊 View detailed coverage comparison

Summary

This PR enables dynamic option changes for st.pills and st.segmented_control widgets when a user-provided key is present. Previously, changing options would reset the widget state; now selections are preserved when the selected value exists in the new options list, and gracefully reset to the default when the selected value is no longer available.

The implementation switches from index-based to string-based widget values, aligning with the approach already used by st.selectbox, st.multiselect, and st.radio. This is a significant but well-executed change that improves the developer experience for dynamic UIs.

Key changes:

• Backend: New _ButtonGroupSerde class with string-based serialization
• Frontend: State now stores content strings (e.g., ["Apple"]) instead of indices, with indices derived via useMemo
• Proto: Replaced value (uint32 indices) with raw_values (string values)
• Widget ID now only includes click_mode in key_as_main_identity, allowing options to change without ID reset

Code Quality

The code is well-structured and follows established patterns in the codebase:

Strengths:

• The _ButtonGroupSerde class (lines 83-260 in button_group.py) is well-organized with clear separation between single and multi-select logic
• Good reuse of existing utilities from options_selector_utils.py (validate_and_sync_value_with_options, validate_and_sync_multiselect_value_with_options)
• Frontend correctly uses useMemo to derive indices from content strings (line 331-334 in ButtonGroup.tsx)
• Helpful comments explaining design decisions (e.g., lines 976-979 in button_group.py explaining key_as_main_identity)

Minor observations:

1. The index suffix format {content}|{index} (line 829 in button_group.py) is an implementation detail for uniqueness. While functional, content containing |{digit} at the end could theoretically cause edge cases. The implementation handles this correctly with rsplit("|", 1), but it's worth noting.

2. The frontend stripIndexSuffix function (lines 78-81 in ButtonGroup.tsx) assumes the index is always at the end after a |. This mirrors the backend logic correctly.

Test Coverage

The test coverage is excellent and comprehensive:

Python Unit Tests (button_group_test.py):

• New TestButtonGroupSerde class with 10 tests covering single/multi-select serialization and deserialization
• Tests for ID stability with key (test_stable_id_with_key_pills, test_stable_id_with_key_segmented_control)
• Tests verifying options can change without changing ID (test_options_change_does_not_change_id_pills, test_options_change_does_not_change_id_segmented_control)
• Tests confirming selection_mode changes still trigger ID changes

Frontend Unit Tests (ButtonGroup.test.tsx):

• Updated to use setStringArrayValue instead of setIntArrayValue
• Tests for single-select and multi-select click behavior with string values
• Tests for form reset behavior

E2E Tests (st_pills_test.py, st_segmented_control_test.py):

• test_dynamic_pills_props and test_dynamic_segmented_control_props comprehensively test:
  • Selection reset when value is removed from options
  • Selection preservation when value exists in both option sets
  • Negative assertions to prevent regressions
• Snapshot tests for both initial and updated states across all browsers (chromium, firefox, webkit)

The tests follow best practices from .cursor/rules/e2e_playwright.mdc and .cursor/rules/python_tests.mdc, including negative assertions and clear docstrings explaining test scenarios.

Backwards Compatibility

Protocol changes are properly handled:

• Field 7 (value) is now reserved, not reused
• New field 14 (raw_values) is added with clear documentation
• The next id comment is updated to 15

Impact on existing users:

• Users upgrading Streamlit will see their pills/segmented_control widgets work seamlessly
• No breaking changes to the public API (st.pills, st.segmented_control)
• The change is backwards compatible from a user's perspective - existing code continues to work

Note: This is a wire format change, so mixed-version deployments (old frontend + new backend or vice versa) would have issues. This is expected and acceptable for Streamlit upgrades.

Security & Risk

No security concerns identified:

• No user input is executed or evaluated unsafely
• The format_func is already validated and used consistently
• String values are properly escaped/handled in protobuf

Low regression risk:

• The implementation mirrors the proven approach from st.selectbox/st.multiselect/st.radio
• Comprehensive test coverage protects against regressions
• E2E tests verify end-to-end behavior

Recommendations

No blocking issues identified. A few optional suggestions:

1. Documentation (optional): Consider adding a note to the docstrings for st.pills and st.segmented_control mentioning that when a key is provided, options can be changed dynamically while preserving valid selections.

2. Edge case handling (optional): The test app comments (e.g., line 211-214 in st_pills.py) explain that format_func changes affect selection preservation. This is documented in comments but users might benefit from explicit documentation.

Verdict

APPROVED: This is a well-implemented feature that aligns with existing patterns in the codebase, has comprehensive test coverage, and properly handles backwards compatibility. The change addresses user needs (issues #11277 and #12392) while maintaining code quality standards.

This is an automated AI review using opus-4.5-thinking. Please verify the feedback and use your judgment.