feat: Add `BoolTokens` mixin for strict/fgpyo-style bool parsing by msto · Pull Request #78 · fg-labs/fgmetric

msto · 2026-06-17T00:37:36Z

Summary

Closes #75

One of two features to bring Metric's parsing to parity with fgpyo.

Pydantic accepts a broad set of strings as booleans (true/t/yes/y/on/1 and false counterparts). This PR updates Metric's boolean parsing to be consistent with fgpyo by default - {true, t, 1}/{false, f, 0} are case-insensitively matched to True/False, and any other values raise a ValidationError.

The package held `DelimitedList` and `CounterPivotTable` — Pydantic mixins that transform `Metric` fields to and from their delimited-text representation, not collection types in their own right. Rename to `converters` to name them by what they do. `Metric` is the only importer; the move is behavior-preserving. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Extract the previous `_empty_field_to_none` validator from `Metric` into a reusable `NullSentinels` mixin that exposes a `null_sentinels: frozenset[str]` ClassVar. Users can now configure additional string sentinels (e.g., "NA", "None") to be treated as null on Optional fields. Scoped to Optional fields to keep the mixin composable with `DelimitedList`'s `list[T]` handling. `Metric` defaults to `frozenset({""})` to preserve fgpyo-aligned behavior. Closes #8. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Address review feedback. The Optional-field key collection handled only plain string aliases, so a field reachable solely via `AliasChoices` was never sentinel-substituted. Extract a `_validation_keys` helper that also expands the string members of an `AliasChoices`; path-shaped `AliasPath` stays out of scope for flat delimited rows. Document that `serialization_alias` is output-only and therefore excluded. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

coderabbitai · 2026-06-17T00:37:44Z

Warning

Review limit reached

@msto, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 29 minutes and 15 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 586b509c-0fa3-4cea-8a4d-2683811d2126

📥 Commits

Reviewing files that changed from the base of the PR and between 2ecac17 and 39b0638.

📒 Files selected for processing (12)

CHANGELOG.md
fgmetric/_typing_extensions.py
fgmetric/collections/__init__.py
fgmetric/converters/__init__.py
fgmetric/converters/_bool_tokens.py
fgmetric/converters/_counter_pivot_table.py
fgmetric/converters/_delimited_list.py
fgmetric/converters/_null_sentinels.py
fgmetric/metric.py
tests/test_bool_tokens.py
tests/test_null_sentinels.py
tests/test_typing_extensions.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

Restrict boolean parsing on `Metric` fields typed `bool`/`bool | None` to a narrow, configurable token set, defaulting to fgpyo's `{true, t, 1}`/ `{false, f, 0}` (case-insensitive) and raising a `ValidationError` on anything else — including `yes`/`y`/`on`/`no`/`n`/`off`, which Pydantic would otherwise accept. A `field_validator("*", mode="before")` resolves string cells before Pydantic's broader coercion; `_bool_fieldnames` and the case-folded token sets are computed once in `__pydantic_init_subclass__`, and overlapping true/false tokens are rejected at class-definition time. Adds an `is_bool` typing helper. The `BoolTokens` converter is mixed into `Metric` by default. Read-side only: serialization is unchanged (`bool` still serializes to `"True"`/`"False"`). Closes #75 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

ameynert

list[bool] / list[bool | None] element values silently bypass the strict token set. Confirmed empirically against the branch:

  class M(Metric):
      flags: list[bool]
      flag: bool

  M.model_validate({"flag": "yes"})                 # ValidationError  (strict, correct)
  M.model_validate({"flags": "yes,on,no"}).flags    # -> [True, True, False]  ← broad coercion!

is_bool(list[bool]) is False, so list-of-bool fields never enter _bool_fieldnames; DelimitedList._split_lists splits the cell and pydantic's default broad coercion validates each element — accepting exactly the yes/on/off tokens this PR exists to reject. So a scalar bool column and a list[bool] column on the same Metric disagree on what's a valid boolean.

Options:

Extend token parsing to bool-typed list elements (apply tokenization per element when has_origin(annotation, list) and the element type is bool), or
Explicitly document the limitation in the BoolTokens docstring and add a test pinning the current list[bool] behavior so it's a deliberate, visible choice
rather than an accident.

Extend BoolTokens to apply strict token resolution to the elements of list[bool] / list[bool | None] fields, so a scalar bool column and a list-of-bool column on the same Metric agree on what counts as a valid boolean. Previously list[bool] elements bypassed the token set via Pydantic's broad coercion (e.g. "yes,on,no" -> [True, True, False]). Adds an is_bool_list typing helper and applies the docstring review suggestions (customize-example output; non-string/list handling note). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

ameynert · 2026-06-25T23:24:50Z

+    Fields whose elements are bool - `list[bool]` and `list[bool | None]` - are tokenized per
+    element, so a list-of-bool column and a scalar `bool` column on the same model agree on what
+    counts as a valid boolean. Non-string values (and non-string list elements, such as a `None`
+    from an empty `list[bool | None]` cell) are left to Pydantic.


How would something weird list[bool | str] be treated? Is that even allowed?

ameynert · 2026-06-25T23:29:28Z

    @classmethod
    def _parse_bool_tokens(cls, value: Any, info: ValidationInfo) -> Any:
-        """Resolve string values on bool fields to `True`/`False` from the configured tokens."""
+        """Resolve string values on bool (and bool-list) fields from the configured tokens."""


Claude spotted this - worth a comment to ensure future refactors keep _split_list running before this.

# On `Metric`, `DelimitedList` runs first and has already split a bool-list cell into a # list of string elements by the time this validator sees it; resolve each element. Real # `None` elements (e.g. an empty cell in a `list[bool | None]`) are left for Pydantic. # # NB: This correctness hinges on `DelimitedList`'s before-validator running ahead of this # one (it must split the cell into a list before we can tokenize elements). If that order # ever flips, a `list[bool]` cell reaches Pydantic as an unsplit string and falls back to # broad coercion (silently accepting `yes`/`on`/...). `test_metric_bool_list_is_strict` # is the regression guard.

ameynert · 2026-06-25T23:31:04Z

+@pytest.mark.parametrize(
+    "annotation",
+    [
+        bool,  # a scalar bool is not a list


Add a test where the input TypeAnnotation is None (same with test_is_not_bool above) - that should return False for both.

ameynert · 2026-06-25T23:33:10Z

        true_tokens: The set of input strings parsed as `True` on `bool` fields during validation.
            Defaults to `frozenset({"true", "t", "1"})`.
        false_tokens: The set of input strings parsed as `False` on `bool` fields during
            validation. Defaults to `frozenset({"false", "f", "0"})`.


Suggested change

true_tokens: The set of input strings parsed as `True` on `bool` fields and `list[bool]`

elements during validation. Defaults to `frozenset({"true", "t", "1"})`.

false_tokens: The set of input strings parsed as `False` on `bool` fields and `list[bool]`

elements during validation. Defaults to `frozenset({"false", "f", "0"})`.

msto · 2026-06-26T00:17:27Z

After discussing with Alison, going to close this until someone asks for it!

msto and others added 3 commits June 16, 2026 11:43

msto self-assigned this Jun 17, 2026

msto linked an issue Jun 17, 2026 that may be closed by this pull request

Add BoolTokens mixin for strict/fgpyo-style bool parsing #75

Open

msto force-pushed the msto/bool-tokens-mixin branch from fceff5f to a76d1f7 Compare June 17, 2026 00:46

msto commented Jun 17, 2026

View reviewed changes

Comment thread fgmetric/converters/_bool_tokens.py Outdated

msto commented Jun 17, 2026

View reviewed changes

Comment thread fgmetric/converters/_bool_tokens.py Outdated

msto force-pushed the msto/bool-tokens-mixin branch from 14e5229 to 1765f4c Compare June 17, 2026 12:41

msto requested review from ameynert, clintval and znorgaard and removed request for ameynert and clintval June 17, 2026 12:51

msto assigned ameynert and unassigned msto Jun 17, 2026

msto marked this pull request as ready for review June 17, 2026 12:51

ameynert reviewed Jun 17, 2026

View reviewed changes

Comment thread fgmetric/converters/_bool_tokens.py

Comment thread fgmetric/converters/_bool_tokens.py

Comment thread fgmetric/converters/_bool_tokens.py

ameynert requested changes Jun 17, 2026

View reviewed changes

ameynert assigned msto and unassigned ameynert Jun 17, 2026

Base automatically changed from msto/null-sentinels-mixin to main June 24, 2026 23:32

msto mentioned this pull request Jun 24, 2026

Generalize DelimitedList to DelimitedCollection and add DelimitedMapping #76

Open

msto requested a review from ameynert June 25, 2026 00:02

ameynert approved these changes Jun 25, 2026

View reviewed changes

msto closed this Jun 26, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: Add `BoolTokens` mixin for strict/fgpyo-style bool parsing#78

feat: Add `BoolTokens` mixin for strict/fgpyo-style bool parsing#78
msto wants to merge 5 commits into
mainfrom
msto/bool-tokens-mixin

msto commented Jun 17, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jun 17, 2026 •

edited

Loading

Review limit reached

Uh oh!

Uh oh!

Uh oh!

ameynert left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ameynert Jun 25, 2026

Uh oh!

ameynert Jun 25, 2026

Uh oh!

ameynert Jun 25, 2026

Uh oh!

ameynert Jun 25, 2026

Uh oh!

msto commented Jun 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

+        true_tokens: The set of input strings parsed as `True` on `bool` fields and `list[bool]`
+            elements during validation. Defaults to `frozenset({"true", "t", "1"})`.
+        false_tokens: The set of input strings parsed as `False` on `bool` fields and `list[bool]`
+            elements during validation. Defaults to `frozenset({"false", "f", "0"})`.

Uh oh!

Conversation

msto commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

coderabbitai Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

Uh oh!

Uh oh!

ameynert left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ameynert Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

ameynert Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

ameynert Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

ameynert Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

msto commented Jun 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

msto commented Jun 17, 2026 •

edited

Loading

coderabbitai Bot commented Jun 17, 2026 •

edited

Loading