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

Summary

This PR fixes issue #13410 where Altair charts containing nested vconcat and hconcat compositions (e.g., scatter plots with marginal histograms using the pattern top_hist & (points | right_hist)) were rendering as empty elements with "Infinite extent" errors in Streamlit v1.42+.

Key changes:

1. Added a new helper function _has_nested_composition() to detect when a vconcat spec contains nested composition operators
2. Modified _prepare_vega_lite_spec() to use "pad" autosize instead of "fit-x" for nested compositions
3. Updated the default width logic to use "content" width for nested vconcat+hconcat charts
4. Added comprehensive unit tests and E2E tests with visual snapshots

Code Quality

Strengths

1. Well-documented code (lib/streamlit/elements/vega_charts.py lines 282-307): The _has_nested_composition() function has clear docstrings with proper NumPy-style documentation explaining parameters and return values.

2. Clear trade-off documentation (lines 345-353): The code comments clearly explain the known limitation regarding container overflow and the trade-off being made (accept overflow to ensure charts render correctly).

3. Focused changes: The fix is minimal and targeted, avoiding over-engineering. The detection function checks exactly what's needed to fix the issue.

4. Consistent with existing patterns: The handling of nested compositions follows the same pattern already used for facet, hconcat, and repeat charts.

Minor Observations

1. Detection depth (lib/streamlit/elements/vega_charts.py lines 300-306): The _has_nested_composition() function only checks direct children of vconcat. Deeply nested compositions (3+ levels) are not detected. However, this is a reasonable scope for the current fix and can be extended if needed.

    # Check if vconcat contains nested compositions
    if "vconcat" in spec and isinstance(spec["vconcat"], list):
        for item in spec["vconcat"]:
            # Check if this item is a dict containing any composition operator
            if isinstance(item, dict) and any(
                k in item for k in ["hconcat", "vconcat", "concat", "layer"]
            ):
                return True

2. Symmetric handling: The fix doesn't handle the symmetric case where hconcat contains nested vconcat. While this isn't mentioned in issue Bug: Altair charts with VConcat and HConcat broken on st v1.42 onwards #13410, it could exhibit similar behavior. This is acceptable for now since the issue specifically involves vconcat+hconcat patterns.

Test Coverage

Python Unit Tests ✅

The test coverage is comprehensive and follows best practices:

1. NestedCompositionTest class: Tests the new _has_nested_composition() helper function with:

   • Simple vconcat (returns False)
   • Vconcat with hconcat (returns True)
   • Vconcat with layer (returns True)
   • Vconcat with nested vconcat (returns True)
   • No vconcat in spec (returns False)

2. VegaLiteAutosizeTest class: Tests autosize configuration for various scenarios:

   • Simple vconcat with use_container_width=True gets fit-x
   • Nested vconcat+hconcat with use_container_width=True gets pad
   • Nested vconcat+hconcat with use_container_width=False gets pad
   • Issue Bug: Altair charts with VConcat and HConcat broken on st v1.42 onwards #13410 exact scenario with width='stretch' and width='content'
   • Altair nested charts with width='stretch'
   • Default width behavior for nested compositions

3. Parameterized tests updated: Added test cases to existing parameterized tests for default width behavior.

Tests follow the guidelines:

• ✅ Brief docstring comments
• ✅ Testing with both st.vega_lite_chart and st.altair_chart
• ✅ Tests are parameterized where appropriate

E2E Tests ✅

The E2E tests are appropriate and follow best practices:

1. Test app (e2e_playwright/st_altair_chart.py lines 145-195): Added a new chart that reproduces the exact pattern from issue Bug: Altair charts with VConcat and HConcat broken on st v1.42 onwards #13410 (scatter plot with marginal histograms using the iris dataset).

2. Snapshot test (e2e_playwright/st_altair_chart_test.py line 58): Added visual snapshot test for the new chart.

3. Updated chart count: NUM_CHARTS correctly updated from 8 to 9.

4. Follows E2E best practices:

   • ✅ Uses themed_app fixture for both light and dark theme coverage
   • ✅ Waits for stability before taking snapshots
   • ✅ Uses descriptive snapshot name: st_altair_chart-marginal_histogram
   • ✅ Added snapshots for all browser/theme combinations (6 new snapshot files)

Minor note: The E2E test imports from vega_datasets import data (line 18). This is an existing test dependency used elsewhere in the codebase, so no new dependencies are introduced.

Backwards Compatibility

Low risk of breaking changes:

1. Charts that were broken now work: Nested vconcat+hconcat charts that were rendering empty now display correctly.

2. Existing charts unaffected:

   • Simple vconcat charts continue to use fit-x autosize with container width
   • Hconcat, facet, and repeat charts continue their existing behavior

3. Conservative default behavior: Nested compositions now default to "content" width instead of "stretch". This is a safer default that avoids overflow issues.

4. Explicit width still works: Users can still explicitly set width="stretch" if needed, and the autosize will be set to "pad" to prevent infinite extent errors.

Security & Risk

No security concerns identified.

Low regression risk:

• The change is well-scoped to specific chart types (nested compositions)
• The autosize change only affects charts that were previously broken
• The detection logic is simple and unlikely to produce false positives/negatives for common use cases

Recommendations

No blocking issues found. The following are minor suggestions for future consideration (not required for this PR):

1. Consider symmetric handling: In a future PR, consider whether hconcat containing nested vconcat should also receive special handling.

2. Consider recursive detection: For very complex charts with deep nesting (3+ levels), the current detection might not catch all cases. This could be addressed if such issues are reported.

3. Documentation update: Consider updating the st.altair_chart and st.vega_lite_chart docstrings to mention that nested compositions default to width="content" (lines 1826-1834 in vega_charts.py).

Verdict

APPROVED: This PR provides a well-implemented fix for issue #13410 with comprehensive test coverage, clear documentation of trade-offs, and minimal risk of regressions. The changes follow existing patterns in the codebase and are appropriately scoped.

This is an automated AI review. Please verify the feedback and use your judgment.

Just a heads up that there have been flickering issues with these charts in connection with the width sizing options because some options from vega lead to a looping calculation of the width in conjunction with our element sizing. They typically show up when you go into full screen and then return back to the regular view and might not show up in the regression tests since it is hard to capture a flicker consistently. I don't think this will necessarily trigger the flicker, but just recommend manual testing.

📈 Frontend coverage change detected

The frontend unit test (vitest) coverage has increased by 0.0200%

• Current PR: 86.5100% (12766 lines, 1721 missed)
• Latest develop: 86.4900% (12754 lines, 1723 missed)

✅ Coverage change is within normal range.

📊 View detailed coverage comparison