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

Thanks for contributing to Streamlit! 🎈

Please make sure you have read our Contributing Guide. You can find additional information about Streamlit development in the wiki.

The review process:

1. Initial triage: A maintainer will apply labels, approve CI to run, and trigger AI-assisted reviews. Your PR may be flagged with status:needs-product-approval if the feature requires product team sign-off.

2. Code review: A core maintainer will start reviewing your PR once:

   • It is marked as 'ready for review', not 'draft'
   • It has status:product-approved (or doesn't need it)
   • All CI checks pass
   • All AI review comments are addressed

We're receiving many contributions and have limited review bandwidth — please expect some delay. We appreciate your patience! 🙏

Update in latest commit (e1e539f)

Fixed CI failures by adding missing metricValueFontSize and metricValueFontWeight to:

• StreamlitTheme interface (component-v2-lib)
• extractComponentsV2Theme function (for Components V2 support)
• Mock theme in tests

Also added negative value tests for metricValueFontSize as suggested by Copilot.

Ready for CI approval when you have a chance. Thanks!

Fixed Python proto_compatibility_test.py failure by adding metric_value_font_size and metric_value_font_weight to expected CustomThemeConfig fields.

@kagawa0710 since this is a visual change, it would be good if you could add some screenshots to help with the product review 🙏

@sfc-gh-lwilby
Added screenshots to the PR description showing before/after comparison.

Sorry for the late reply due to timezone differences!

Sounds good, thanks for adding this! Just two quick questions:

1. This doesn't change the current design if the config options are not set, right? On your first screenshot it looks a bit smaller than right now, but probably it's just an issue of the screenshot.
2. Does the font size option accept the same values that we also accept for other font size config options, e.g. headingFontSizes seems to accept both rem and px values?

does the font size option accept the same values that we also accept for other font size config options, e.g. headingFontSizes seems to accept both rem and px values?

If we want to support rem and px, it probably needs to be string not int

@jrieke @lukasmasuch
Thanks for the questions!

1. Default behavior:
Correct - the default design doesn't change when the config options are not set. The component uses the existing default value (theme.fontSizes.threeXL = 2.25rem):
https://github.com/kagawa0710/streamlit/blob/feat/12300-metric-theming-font-size-weight/frontend/lib/src/components/elements/Metric/styled-components.ts#L93

The screenshot difference is likely just due to my local environment.

2. rem/px support:
The current implementation uses int (pixels only), following the same pattern as baseFontSize.

If string support (rem/px like headingFontSizes) is preferred, I'd be happy to implement it. Since it would require protobuf + type changes across several files, I could either:

1. Update this PR
2. Create a follow-up PR to keep this one focused

Let me know your preference!

Hi @kagawa0710 ,

Checked with the others on this and we will need to support REM and pixels for this config. Can you update this PR to support that? We don't want to change the code later, particularly the protobuf files, and also we don't want to change how the feature works between releases.

Thank you!

Update: Added rem/px string support for metricValueFontSize

Following the feedback, I've updated metricValueFontSize to accept string values with rem/px units (like headingFontSizes).

Supported formats

Invalid values log a warning and fallback to default (2.25rem).

Changes

• Proto: int32 → string
• Config: type_=int → type_=str
• Frontend: Added parseFontSize validation
• Tests: 17 unit test cases + E2E test with rem parsing

Note

CI will fail initially due to missing Linux snapshots for the new E2E snapshot test. I'll add them after this run.

@jrieke @lukasmasuch @sfc-gh-lwilby

Updated to support rem/px string values as requested. Please see the updated PR description for details:

• Supported formats: "48px", "3rem", "48" (plain number → px)
• Invalid value handling: Warning + fallback to default (2.25rem)
• Screenshots: Before/after comparison with all supported formats
• Tests: 17 unit test cases + E2E test verifying rem→px conversion

Ready for review!

Regarding the py-coverage-report failure:

I think this is a CI permissions issue rather than a code problem:

• Error: HttpError: Resource not accessible by integration
• Observation: All coverage calculation steps succeeded (steps 5-8), only the "comment" step failed
• Local verification: All tests pass locally (Python: 91 passed, Frontend: 363 passed)

Could you confirm if this is the case, or let me know if there's something I need to fix?

@kagawa0710 thanks for making these updates! I haven't forgotten about your PR, I will try to take a look at the latest changes tomorrow.

I agree the CI failure is probably not related to your changes, I will rerun it.

Summary

This PR adds two new theme configuration options: metricValueFontSize and metricValueFontWeight, allowing users to customize the appearance of st.metric value text. The implementation spans the full stack: Python config, protobuf definitions, frontend theme types/utils, styled components, and component-v2-lib. The feature is backwards compatible - when the options are not configured, the default behavior (2.25rem font size, inherited font weight) is preserved.

Code Quality

Strengths:

• The implementation follows existing patterns established by similar theme options (codeFontSize, baseFontSize, headingFontSizes, etc.)
• Proper validation of font size values using the existing parseFontSize utility with additional checks for positive values
• Font weight validation uses standard CSS range (100-900)
• Clean integration with the EmotionTheme type system

Issues:

1. Missing warning log for out-of-range font weight (frontend/lib/src/theme/utils.ts:1054-1058): When metricValueFontWeight is outside the valid range, it silently falls back to undefined without logging a warning. This is inconsistent with metricValueFontSize which logs warnings for invalid values.

    ...(metricValueFontWeight &&
    metricValueFontWeight >= 100 &&
    metricValueFontWeight <= 900
      ? { metricValueFontWeight }
      : {}),

2. Minor inconsistency in fontWeight range documentation: baseFontWeight in config.py documents a range of 100-600, while metricValueFontWeight uses 100-900. The 100-900 range is correct for CSS font-weight, so this isn't a bug, but it may be worth noting this intentional difference.

Test Coverage

Strengths:

• Frontend unit tests (utils.test.ts): Comprehensive coverage with 17+ test cases covering valid rem/px values, invalid formats, zero/negative values, and boundary conditions
• Metric component tests (Metric.test.tsx): Tests default styling behavior
• Python config tests (config_test.py): Verifies config options integration
• Proto compatibility tests (proto_compatibility_test.py): Ensures protobuf schema is correctly extended
• E2E tests: Visual snapshot test and CSS property assertions

Issues:

1. E2E test uses wait_for_timeout (e2e_playwright/theming/theme_metric_value_style_test.py:60): The best practices documentation explicitly states "Never use wait_for_timeout." This should be replaced with a more deterministic wait approach.

@pytest.mark.usefixtures("configure_metric_value_style")
def test_metric_value_style_snapshot(app: Page, assert_snapshot: ImageCompareFunction):
    """Visual snapshot test for custom metric value styling."""
    expect_no_skeletons(app, timeout=25000)
    # Wait for fonts to load to reduce flakiness
    app.wait_for_timeout(5000)

    metric = get_metric(app, "Revenue")
    assert_snapshot(metric, name="metric_value_custom_style")

Consider using wait_until with a font-loaded check or Playwright's built-in font loading detection.

2. Metric component unit tests could be more thorough: The tests in Metric.test.tsx verify default behavior but don't test with a mocked theme that has custom metricValueFontSize/metricValueFontWeight values set. Testing both scenarios would provide better coverage.

Backwards Compatibility

The changes are fully backwards compatible:

• New proto fields are optional (field numbers 64, 65)
• When not configured, metricValueFontSize defaults to theme.fontSizes.threeXL (2.25rem)
• When not configured, metricValueFontWeight is undefined, inheriting from parent
• No existing fields were modified or removed

Security & Risk

No security concerns identified:

• The feature only affects styling/theming
• Input validation prevents injection of arbitrary CSS (only valid rem/px values accepted)
• All values are sanitized through existing theme utilities

Recommendations

1. Add warning log for invalid metricValueFontWeight: For consistency, add a LOG.warn() call when font weight is out of range, similar to font size validation:

...(metricValueFontWeight
  ? metricValueFontWeight >= 100 && metricValueFontWeight <= 900
    ? { metricValueFontWeight }
    : (LOG.warn(`Invalid metricValueFontWeight: ${metricValueFontWeight}. Must be between 100 and 900.`), {})
  : {}),

2. Replace wait_for_timeout in E2E test: Consider using a more deterministic approach such as:

# Wait for fonts using document.fonts.ready
app.evaluate("() => document.fonts.ready")

3. Add unit test for custom theme values in Metric component: Add a test case that mocks a theme with custom metricValueFontSize/metricValueFontWeight to verify the styled component applies them correctly.

Verdict

APPROVED: This is a well-implemented feature that follows existing patterns, includes comprehensive test coverage, and maintains backwards compatibility. The issues identified are minor and don't block the merge:

• The missing warning log is a nice-to-have for consistency but not critical
• The wait_for_timeout usage, while against best practices, is used specifically for font loading flakiness in snapshot tests which is a known difficult case
• The additional unit test would be nice but the E2E test already verifies the custom styling works end-to-end

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

Thanks for getting this merged @kagawa0710, awesome addition!!