Skip to content

feat: Cover numeric trait conditions with mixed types - #63

Merged
emyller merged 1 commit into
mainfrom
feat/mixed-numeric-ordering
Oct 30, 2026
Merged

emyller merged 1 commit into
mainfrom
feat/mixed-numeric-ordering

Conversation

@emyller

@emyller emyller commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

No cases cover numeric traits against rule values of another type, where the Ruby SDK raised or disagreed with flagsmith-engine.

Fixed in Flagsmith/flagsmith-ruby-client#118. All cases pass in flagsmith-engine.

Changes

  • Integer trait against a decimal rule value (95 > "90.5", 95 != "90.5") doesn't match.
  • Float trait against an integer rule value (95.5 > "90") matches.
  • CONTAINS on an integer trait doesn't match.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@emyller
emyller requested a review from a team as a code owner October 30, 2026 14:27
@emyller
emyller requested review from khvn26 and removed request for a team October 30, 2026 14:27

@khvn26 khvn26 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Confirmed against the Core API's pre-engine evaluator (check_integer_value / check_float_value) and flagsmith-engine v2.1.0 through v11.1.1. All 4 cases match.

@emyller emyller changed the title feat: Add numeric trait coercion test cases feat: Cover numeric trait conditions with mixed types Oct 30, 2026
@emyller
emyller merged commit ab57f95 into main Oct 30, 2026
2 checks passed
@emyller
emyller deleted the feat/mixed-numeric-ordering branch October 30, 2026 14:48
gagantrivedi added a commit to Flagsmith/flagsmith-nodejs-client that referenced this pull request Nov 6, 2026
Since Flagsmith/flagsmith#8532, the environment document carries
`metadata.experiment` on feature states. The SDK dropped it when
building engine models, so under local evaluation `flag.experiment` was
always undefined and `getExperimentFlag` never recorded an exposure.

### Changes
- `FeatureStateModel` keeps `metadata.experiment`; the mapper adds it to
the engine context's feature metadata for environment and segment
feature states. Identity overrides carry none, as on the API.
- `Flags.fromEvaluationResult` sets `experiment`, and also `variant`,
which local evaluation dropped until now, so local exposures carry the
variant key.
- Locally evaluated environment flags carry no experiment, matching the
API, which only sends it for identity flags.

### Tests
`getExperimentFlag` under local evaluation: an enrolled identity records
one exposure with its variant and experiment ID; an identity outside the
rollout, one with an identity override, and a feature with no experiment
record none. Environment flags carry no experiment.

Also checked against a local Flagsmith with the events pipeline: local
and remote evaluation give the same value, variant and experiment, and
one exposure reached the warehouse.

### Not included
Bumping `engine-test-data` to v3.13.0, which adds the experiment cases:
it also adds two numeric trait cases (Flagsmith/engine-test-data#63) the
engine already fails, to be fixed separately.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants