Skip to content

feat(api): Evaluate identity flags through flag-engine - #8555

Draft
khvn26 wants to merge 30 commits into
mainfrom
feat/engine-evaluation-authority
Draft

khvn26 wants to merge 30 commits into
mainfrom
feat/engine-evaluation-authority

Conversation

@khvn26

@khvn26 khvn26 commented Sep 18, 2026

Copy link
Copy Markdown
Member

Thanks for submitting a PR! Please check the boxes below:

  • I have read the Contributing Guide.
  • I have added information to docs/ if required so people know about the feature.
  • I have filled in the "Changes" section below.
  • I have filled in the "How did you test this code" section below.

Changes

Contributes to #6654

In this PR, we route identity flag evaluation through get_evaluation_result, leaving Core API responsible only for building an EvaluationContext.

How did you test this code?

Added api/tests/unit/environments/identities/test_unit_identities_evaluation.py, and fixed existing test.

@vercel

vercel Bot commented Sep 18, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

3 Skipped Deployments
Project Deployment Actions Updated
docs Ignored Ignored Preview Sep 23, 2026 2:19am UTC
flagsmith-frontend-preview Ignored Ignored Preview Sep 23, 2026 2:19am UTC
flagsmith-frontend-staging Ignored Ignored Preview Sep 23, 2026 2:19am UTC

Request Review

@coderabbitai

coderabbitai Bot commented Sep 18, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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.

@github-actions github-actions Bot added api Issue related to the REST API feature New feature or request labels Sep 18, 2026
@codecov

codecov Bot commented Sep 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.83923% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 98.81%. Comparing base (47db6dc) to head (334c714).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
api/evaluation/mappers.py 99.08% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main    #8555    +/-   ##
========================================
  Coverage   98.80%   98.81%            
========================================
  Files        1631     1636     +5     
  Lines       66920    67090   +170     
========================================
+ Hits        66123    66292   +169     
- Misses        797      798     +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions Bot added feature New feature or request and removed feature New feature or request labels Sep 21, 2026
Core API resolved override precedence and multivariate bucketing itself,
in parallel with the engine it already used for segment matching. This
routes identity flag evaluation through `get_evaluation_result` instead,
leaving Core API responsible only for building an `EvaluationContext`.

`Identity.get_all_feature_states` now asks the engine which override won
rather than comparing feature states with `>`. It still returns Django
rows: `FlagResult.metadata.feature_state_id` maps the engine's verdict
back to the row it came from, so callers are unaffected for now.

Identity overrides reach the engine the way SDKs express them, as a
synthetic segment whose overrides sit at `priority = -inf`. That encodes
"no segment override outranks an identity override", which until now was
`FeatureState.__gt__` returning True unconditionally.

Every segment with an override in the environment is now evaluated,
rather than pre-filtering to segments the identity is known to match.
A single pass is what lets a segment condition depend on a flag that is
itself only resolved during evaluation (#1674).

Contributes to #6654.
Drop the redundant OR term on the transient path, and the module
docstring, comments and pragmas that restated what the code says.
…ypes

`evaluation.py` mixed a mapper, a service and two result types. Split it
along the lines the rest of the package already uses, and rename
`build_identity_evaluation_context` to `map_identity_to_evaluation_context`
to match the mapper naming convention.

Tests follow the split. No behaviour change.
@khvn26
khvn26 force-pushed the feat/engine-evaluation-authority branch from 02251d0 to e7192f5 Compare September 22, 2026 10:15
@github-actions github-actions Bot added feature New feature or request and removed feature New feature or request labels Sep 22, 2026
…n_context

The caller had to query feature states and hand them back in four
mutually-consistent arguments. Resolve them in the mapper instead, as
`map_environment_to_engine` alongside it already does, leaving one
argument per genuine caller choice.

`environments.identities.mappers` had nothing left to do and is gone;
`evaluate_identity` calls the engine mapper directly.

Feature states are now always resolved, so `Identity.get_segments` costs
one more query. That is the price of correctness once segment conditions
can read `$.flags.*`: a segments-only context cannot answer them.
@khvn26
khvn26 force-pushed the feat/engine-evaluation-authority branch from 6910685 to 6ac930f Compare September 22, 2026 11:06
@github-actions github-actions Bot added feature New feature or request and removed feature New feature or request labels Sep 22, 2026
Serialisers and integrations re-resolved a feature state's value per
identity, hashing multivariate allocation themselves, after the engine
had already decided it. Carry the engine's `FlagResult` on each feature
state `Identity.get_all_feature_states` returns, so the value and variant
travel with the row they came from.

Removes `FeatureState.get_multivariate_feature_state_value` and
`get_feature_state_value_by_hash_key`, and the `identity` argument to
`get_feature_state_value`, which now means only "this state's stored
value". `evaluated_value` is the value as evaluated, falling back to the
stored one where nothing evaluated it.

Edge identities keep their overrides in DynamoDB, so the rows the edge
serialiser reads never pass through an evaluation. Those resolve through
`features.evaluation.evaluate_feature_state` instead, which asks the
engine about a single feature state.
@github-actions github-actions Bot added feature New feature or request and removed feature New feature or request labels Sep 22, 2026
The edge serialiser resolved a feature state on its own, through a
helper calling `get_flag_result_from_context` — which flag-engine does
not export. Evaluate in `EdgeIdentity.get_all_feature_states` instead,
via `get_evaluation_result`, so its rows carry `flag_result` like the
Postgres ones and the serialiser has nothing left to resolve.

Removes `features.evaluation`, and with it the only use of a private
engine API.

Drops the mocks of the engine's internal hashing where a test can say
what it means instead: the multivariate seed is an assertion about the
mapped feature context, and a serialiser test can set `flag_result`
itself. The two left are end-to-end tests of allocation boundaries,
where the hash is the input being varied.
@github-actions github-actions Bot added feature New feature or request and removed feature New feature or request labels Sep 22, 2026
These are black box API tests, so they had no business patching the
engine's hashing to pin which variant an identity fell into.

Allocate the whole range to one variant instead, or to neither, which
makes the outcome deterministic through the API alone. Both cases the
old tests covered — the served value and the reported variant key — fold
into one parametrised test, including the fall-through to the control
value when nothing is allocated.
@github-actions github-actions Bot added the feature New feature or request label Sep 22, 2026
`EdgeIdentity.get_all_feature_states` asked DynamoDB which segments an
identity matched, then settled precedence itself by keying feature states
on name. Both are the engine's to decide, and it could not decide them
without the segments in the context.

`evaluation.services.get_edge_identity_feature_states` builds one, with
the identity taken from the DynamoDB document rather than an ORM row.
Its own overrides stay in DynamoDB, so they are laid over the result
afterwards, and nothing Pydantic enters the context.

The `identity_feature_names` set goes with it: the serialiser was told
which features an edge identity overrode because those states carry no
`identity_id`, but not being an ORM row says the same thing.

`DynamoIdentityWrapper.get_segment_ids` survives for the segments
endpoint, which is the last caller of the vendored segment evaluation.
@github-actions github-actions Bot added feature New feature or request and removed feature New feature or request labels Sep 23, 2026
`get_segment_ids` reached for `is_context_in_segment`, which flag-engine
does not export, and built its context with a vendored mapper. Ask
`get_evaluation_result` instead, through a context the engine is happy
to be handed.

That leaves nothing calling the vendored segment evaluation, so it and
the context mapper feeding it are gone. What remains of that module maps
a segment for `flagsmith-sql-flag-engine`, which translates rather than
evaluates.
@github-actions github-actions Bot added feature New feature or request and removed feature New feature or request labels Sep 23, 2026
An edge identity's own overrides were laid over the result once the
engine had ruled, so the engine neither knew about them nor said
anything about them: no reason named the identity-override segment, and
precedence over segment overrides rested on the order of a dict update.

Give them to the engine instead, as the synthetic segment every identity
override already reaches it through, and read the winners back off the
result.

`FeatureEngineMetadata` gains `edge_feature_state` to carry one back. A
stored model cannot be assigned a `flag_result`, so those states are
still resolved by the serialiser until the reason exposed for a
multivariate override names its segment — Flagsmith/flagsmith-engine#340.
@github-actions github-actions Bot added feature New feature or request and removed feature New feature or request labels Sep 23, 2026
`get_segment_ids` had become a shim around `get_edge_identity_segments`,
and matching segments was never DynamoDB's business. The segments view
reads the document and asks the evaluation service itself.

Its tests move with it, and stop reaching for DynamoDB on the way: what
they set up is an identity document and some segments, which is all
`get_edge_identity_segments` wants. The ones left behind covered the
method's own signature.
@github-actions github-actions Bot added feature New feature or request and removed feature New feature or request labels Sep 23, 2026
@github-actions github-actions Bot added feature New feature or request and removed feature New feature or request docs Documentation updates labels Sep 23, 2026
Suppressing `ObjectDoesNotExist` around the whole branch also swallowed
the `Environment.DoesNotExist` that `get_edge_identity_segments` raises
when reading the environment, reporting an identity as belonging to no
segments when something was actually wrong. Inherited from the method
this replaced.

Catch around the DynamoDB read alone, and cover the missing identity
where it is now handled.
@github-actions github-actions Bot added feature New feature or request and removed feature New feature or request docs Documentation updates labels Sep 23, 2026
@github-actions github-actions Bot added the docs Documentation updates label Sep 23, 2026
`GET /flags` read environment defaults straight from the ORM, filtering
segment overrides out of the query. So a segment that does not depend on
an identity — one reading `$.environment`, or another flag — was ignored,
though an SDK evaluating the same environment document locally honours
it.

Evaluate the environment instead, with segments and without an identity,
and let the engine decide. Disabled flags are now hidden after evaluating
rather than filtered from the query, so that a disabled override is no
longer passed over in favour of an enabled default.

This branch was successfully deployed

1 active (outdated) deployment
Preview – docs 4d060a58 Deployed Sep 23, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api Issue related to the REST API feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant