Skip to content

feat: Evaluate flags that depend on other flags - #343

Draft
khvn26 wants to merge 2 commits into
mainfrom
feat/dependent-flags
Draft

feat: Evaluate flags that depend on other flags#343
khvn26 wants to merge 2 commits into
mainfrom
feat/dependent-flags

Conversation

@khvn26

@khvn26 khvn26 commented Aug 27, 2026

Copy link
Copy Markdown
Member

@khvn26
khvn26 force-pushed the feat/dependent-flags branch from 4e7cf34 to b0b6007 Compare August 27, 2026 18:00
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown

File Coverage Missing
All files 100%

Minimum allowed coverage is 100%

Generated by 🐒 cobertura-action against 63716d9

@codspeed-hq

codspeed-hq Bot commented Aug 27, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 1 untouched benchmark


Comparing feat/dependent-flags (63716d9) with main (cc74d0a)

Open in CodSpeed

khvn26 added 2 commits August 28, 2026 18:17
Adds support for segments conditioned on another flag's result, via a
`$.flags.<feature name>` condition property, as the evaluation half of
dependent flags.

Segment conditions are already JSONPath, so a dependency needs no new
operator or condition type. What it does need is for the flag to be
resolved by the time the condition reads it, and evaluation resolved
every segment before any flag.

Rather than establish up front which segments depend on which flags,
`$.flags` is a mapping that resolves a flag when a condition first
reads it. Resolving a flag evaluates the segments overriding it, which
in turn resolves whatever flags their conditions read, memoised
throughout. The existing single pass is otherwise untouched: a context
whose segments read no flag never enters the resolver, and pays only
for one empty mapping.

Resolving on read rather than in advance means there is no need to
recognise a dependency in a condition property, and so no need to
reimplement enough of the JSONPath grammar to tell that
`$.flags.a['enabled']` and `$.flags.a.enabled` are the same query.
Every spelling works because resolution is triggered by the read
itself. It also costs only what is read: rule short-circuiting means a
condition that is never evaluated resolves nothing.

Measured against main across the benchmark contexts, interleaved to
cancel drift: +3.4%, from the mapping and a shallow copy of the context
to hold it. Establishing up front that an environment has no
dependencies would remove that residual entirely, but needs a field on
the context to carry it.

A flag caught in a dependency cycle is left unresolved rather than
raising, so a cycle reaching the engine degrades to a non-matching
condition instead of breaking evaluation for the whole context. Such a
flag serves its environment default and reports
`ERROR; code=CIRCULAR_DEPENDENCY`, so that a flag which could not be
resolved is distinguishable from one that was never gated. Only flags
in the cycle are reported that way; a flag merely depending on one
resolves normally. Cycles are still expected to be rejected where
dependencies are written.

Behaviour is covered by test cases in Flagsmith/engine-test-data#59
rather than by unit tests here, so that every engine is held to it.
REVERT BEFORE MERGE.

Dependent flags behaviour is covered by test cases in
Flagsmith/engine-test-data#59 rather than by unit tests here, so that
every engine is held to it. Those cases aren't in a release yet, so
without this the new code is exercised by nothing and CI's 100%
coverage gate fails.

Once #59 is merged and tagged, this goes back to a semver tag, which
Renovate now tracks as of #337.
@khvn26
khvn26 force-pushed the feat/dependent-flags branch from ba4a8e0 to 63716d9 Compare August 28, 2026 17:17
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.

1 participant