Skip to content

feat(Segments): Write rules as plain JSON - #8245

Open
emyller wants to merge 17 commits into
mainfrom
feat/segments-rules-json
Open

feat(Segments): Write rules as plain JSON#8245
emyller wants to merge 17 commits into
mainfrom
feat/segments-rules-json

Conversation

@emyller

@emyller emyller commented Aug 8, 2026

Copy link
Copy Markdown
Contributor
  • 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

Closes #7815

This is the first step towards getting rid of SegmentRule and Condition models, and the tree complexity they have brought upon us thus far.

We achieve:

  • The Segment model earns a new rules_data attribute
    • Creating / updating segments will snapshot clean JSON rules to rules_data
    • Cloning / evolving segments carry JSON rules along with other attributes.
    • Importing segments (e.g. from LaunchDarkly) also stores rules_data.
    • All active segments are backfilled with rules_data.
  • Existing tests interacting with old models are marked for deletion — most are replaced with new surviving tests.
  • Code paths interacting with old models are marked for deletion.

Unplanned things we also achieve:

  • Overall test code and coverage improvements.
  • Segments API will now reject rules nested more than two levels deep.
    • We used to accept them, before we started to ignore them.
    • Unfortunately, active segments with more than two levels of rules exist, and all conditions are evaluated recursively, despite the front-end being unable to represent them visually in full.
  • Fix some leaking writable segment fields, now read-only.

Chose not to achieve:

Deleted LoC will look great in #7818!

How did you test this code?

Exhaustive unit testing. The migration was also benchmarked and optimised.

Important

This includes a data migration to backfill active segments with JSON rules. The migration code was exhaustively tested and tweaked. On a very limited Postgres local container (1 CPU, 2 GB memory), it took ~16s to rasterise rules of 100K segments with varying depth of rules.

@vercel

vercel Bot commented Aug 8, 2026

Copy link
Copy Markdown

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

3 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs Ignored Ignored Preview Aug 11, 2026 11:50pm
flagsmith-frontend-preview Ignored Ignored Preview Aug 11, 2026 11:50pm
flagsmith-frontend-staging Ignored Ignored Preview Aug 11, 2026 11:50pm

Request Review

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The change adds typed, persisted rules_data for segments. It introduces recursive validation and serialisation for segment writes, plus a migration that backfills active rules in batches. Rollout and LaunchDarkly integrations now populate the structured data while retaining legacy records. Tests cover validation, migration, cloning, rollout rules, LaunchDarkly imports, fallbacks, transactions, and large snapshots. API schemas and observability documentation now reflect current metadata and source locations.

Estimated code review effort: 4 (Complex) | ~60 minutes


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.

@emyller emyller self-assigned this Aug 8, 2026
@github-actions github-actions Bot added api Issue related to the REST API docs Documentation updates feature New feature or request and removed docs Documentation updates labels Aug 8, 2026
@emyller
emyller force-pushed the feat/segments-rules-json branch from e997681 to d1b4523 Compare August 8, 2026 01:01
@github-actions github-actions Bot added feature New feature or request docs Documentation updates and removed feature New feature or request docs Documentation updates labels Aug 8, 2026
@khvn26
khvn26 marked this pull request as ready for review August 8, 2026 10:53
@khvn26
khvn26 requested review from a team as code owners August 8, 2026 10:53
@khvn26
khvn26 requested review from khvn26 and removed request for a team August 8, 2026 10:53
@github-actions github-actions Bot added feature New feature or request and removed feature New feature or request docs Documentation updates labels Aug 8, 2026
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Docker builds report

Image Build Status Security report
ghcr.io/flagsmith/flagsmith-e2e:pr-8245 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-api-test:pr-8245 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-frontend:pr-8245 Finished ✅ Results
ghcr.io/flagsmith/flagsmith:pr-8245 Finished ✅ Results
ghcr.io/flagsmith/flagsmith-private-cloud:pr-8245 Finished ✅ Results
ghcr.io/flagsmith/flagsmith-api:pr-8245 Finished ✅ Results

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 5


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 405482df-5f69-4b7d-b284-074ab96f0eb7

📥 Commits

Reviewing files that changed from the base of the PR and between eeac6b3 and e9e27b2.

📒 Files selected for processing (15)
  • api/segments/migrations/0031_add_segment_rules_data.py
  • api/segments/models.py
  • api/segments/serializers.py
  • api/segments/services.py
  • api/segments/types.py
  • api/segments/validators.py
  • api/tests/conftest.py
  • api/tests/types.py
  • api/tests/unit/segments/conftest.py
  • api/tests/unit/segments/test_unit_segments_models.py
  • api/tests/unit/segments/test_unit_segments_services.py
  • api/tests/unit/segments/test_unit_segments_views.py
  • docs/docs/deployment-self-hosting/observability/_events-catalogue.md
  • mcp/src/flagsmith_mcp/openapi.json
  • openapi.yaml

Comment thread api/segments/serializers.py
Comment thread api/segments/validators.py Outdated
Comment thread api/segments/validators.py Outdated
Comment thread api/tests/unit/segments/conftest.py Outdated
Comment thread api/tests/unit/segments/test_unit_segments_views.py
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19227 (attempt 1)

Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)

passed  2 passed

Details

stats  2 tests across 2 suites
duration  37.4 seconds
commit  06f13bb
info  🔄 Run: #19227 (attempt 1)

🗂️ Previous results
✅ private-cloud · depot-ubuntu-latest-16 — run #19227 (attempt 1)

Playwright Test Results (private-cloud - depot-ubuntu-latest-16)

passed  3 passed

Details

stats  3 tests across 3 suites
duration  32.8 seconds
commit  06f13bb
info  🔄 Run: #19227 (attempt 1)

✅ oss · depot-ubuntu-latest-arm-16 — run #19227 (attempt 1)

Playwright Test Results (oss - depot-ubuntu-latest-arm-16)

passed  1 passed

Details

stats  1 test across 1 suite
duration  39.5 seconds
commit  06f13bb
info  🔄 Run: #19227 (attempt 1)

✅ oss · depot-ubuntu-latest-16 — run #19227 (attempt 1)

Playwright Test Results (oss - depot-ubuntu-latest-16)

passed  1 passed

Details

stats  1 test across 1 suite
duration  40 seconds
commit  06f13bb
info  🔄 Run: #19227 (attempt 1)

✅ private-cloud · depot-ubuntu-latest-16 — run #19225 (attempt 1)

Playwright Test Results (private-cloud - depot-ubuntu-latest-16)

passed  1 passed

Details

stats  1 test across 1 suite
duration  1 minute, 7 seconds
commit  080cde0
info  🔄 Run: #19225 (attempt 1)

✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19225 (attempt 1)

Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)

passed  2 passed

Details

stats  2 tests across 2 suites
duration  1 minute, 4 seconds
commit  080cde0
info  🔄 Run: #19225 (attempt 1)

✅ oss · depot-ubuntu-latest-arm-16 — run #19226 (attempt 1)

Playwright Test Results (oss - depot-ubuntu-latest-arm-16)

passed  1 passed

Details

stats  1 test across 1 suite
duration  47.6 seconds
commit  beacb94
info  🔄 Run: #19226 (attempt 1)

✅ oss · depot-ubuntu-latest-arm-16 — run #19225 (attempt 1)

Playwright Test Results (oss - depot-ubuntu-latest-arm-16)

passed  1 passed

Details

stats  1 test across 1 suite
duration  41.4 seconds
commit  080cde0
info  🔄 Run: #19225 (attempt 1)

✅ oss · depot-ubuntu-latest-16 — run #19226 (attempt 1)

Playwright Test Results (oss - depot-ubuntu-latest-16)

passed  1 passed

Details

stats  1 test across 1 suite
duration  33.6 seconds
commit  beacb94
info  🔄 Run: #19226 (attempt 1)

✅ oss · depot-ubuntu-latest-16 — run #19225 (attempt 1)

Playwright Test Results (oss - depot-ubuntu-latest-16)

passed  1 passed

Details

stats  1 test across 1 suite
duration  41.1 seconds
commit  080cde0
info  🔄 Run: #19225 (attempt 1)

✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19223 (attempt 1)

Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)

passed  4 passed

Details

stats  4 tests across 3 suites
duration  37 seconds
commit  6f0394b
info  🔄 Run: #19223 (attempt 1)

✅ private-cloud · depot-ubuntu-latest-16 — run #19223 (attempt 1)

Playwright Test Results (private-cloud - depot-ubuntu-latest-16)

passed  23 passed
skipped  1 skipped

Details

stats  24 tests across 18 suites
duration  1 minute, 29 seconds
commit  6f0394b
info  🔄 Run: #19223 (attempt 1)

Skipped tests

firefox › tests/onboarding-tests.pw.ts › Onboarding › New user connects via the single-page onboarding flow @oss

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Visual Regression

19 screenshots compared. See report for details.
View full report

@emyller
emyller marked this pull request as draft August 8, 2026 14:02
@github-actions github-actions Bot added the feature New feature or request label Aug 11, 2026
@emyller
emyller force-pushed the feat/segments-rules-json branch from ef93dd7 to 04b05a6 Compare August 11, 2026 20:32
@github-actions github-actions Bot added docs Documentation updates feature New feature or request and removed feature New feature or request docs Documentation updates labels Aug 11, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8f8878c4-fd1a-4a42-af9c-99b117204a52

📥 Commits

Reviewing files that changed from the base of the PR and between ef573e1 and 9eff8a3.

📒 Files selected for processing (7)
  • api/integrations/launch_darkly/services.py
  • api/segments/migrations/0031_add_segment_rules_data.py
  • api/segments/serializers.py
  • api/segments/types.py
  • api/tests/conftest.py
  • api/tests/unit/segments/test_unit_segments_migrations.py
  • docs/docs/deployment-self-hosting/observability/_events-catalogue.md
💤 Files with no reviewable changes (1)
  • api/tests/conftest.py

Comment thread api/integrations/launch_darkly/services.py Outdated
@emyller
emyller force-pushed the feat/segments-rules-json branch from 9eff8a3 to 2904c84 Compare August 11, 2026 21:51
@github-actions github-actions Bot added feature New feature or request and removed feature New feature or request docs Documentation updates labels Aug 11, 2026
@emyller
emyller marked this pull request as draft August 11, 2026 21:52
@github-actions github-actions Bot added the docs Documentation updates label Aug 11, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
api/integrations/launch_darkly/services.py (1)

1195-1198: 🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win

Persist root_rule instead of undefined subrules.

Line 1196 references subrules, but _create_segments_from_ld only defines root_rule. Every imported non-deleted LaunchDarkly segment reaches this statement and raises NameError. The import can then leave legacy rows without rules_data.

Proposed fix
-        segment.rules_data = [
-            {"type": constants.ALL_RULE, "conditions": [], "rules": subrules}
-        ]
+        segment.rules_data = [root_rule]

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ea285ce1-bc5a-44bb-9fdd-b9ceeba0171a

📥 Commits

Reviewing files that changed from the base of the PR and between 9eff8a3 and c68ad3c.

📒 Files selected for processing (7)
  • api/experimentation/services.py
  • api/integrations/launch_darkly/services.py
  • api/segments/types.py
  • api/tests/unit/experimentation/test_services.py
  • api/tests/unit/integrations/launch_darkly/test_services.py
  • api/tests/unit/segments/test_unit_segments_views.py
  • docs/docs/deployment-self-hosting/observability/_events-catalogue.md

Comment thread api/tests/unit/segments/test_unit_segments_views.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a1f34a67-e61b-4ecc-89b7-61014629105c

📥 Commits

Reviewing files that changed from the base of the PR and between c68ad3c and 6f0394b.

📒 Files selected for processing (5)
  • api/integrations/launch_darkly/services.py
  • api/segments/migrations/0031_add_segment_rules_data.py
  • api/tests/unit/integrations/launch_darkly/snapshots/test_process_import_request__large_segments__correctly_imported__rules_data.json
  • api/tests/unit/integrations/launch_darkly/test_services.py
  • api/tests/unit/segments/test_unit_segments_migrations.py
💤 Files with no reviewable changes (2)
  • api/tests/unit/segments/test_unit_segments_migrations.py
  • api/tests/unit/integrations/launch_darkly/snapshots/test_process_import_request__large_segments__correctly_imported__rules_data.json

Comment thread api/integrations/launch_darkly/services.py Outdated
@emyller

emyller commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Already reviewed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
api/segments/migrations/0031_add_segment_rules_data.py (1)

56-60: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift

Avoid per-rule queries below the prefetch depth.

When a rule is below the two prefetched rule levels, _rasterise_segment_rule() executes one query for conditions and one query for rules. The existing three-level test already triggers both fallback queries. Deeper trees add two queries per rule, and BATCH_SIZE limits segments, not descendant rules.

Load active rules and conditions once per batch, or prefetch the supported legacy depth. Add query-count coverage for three- and four-level trees.

Source: Learnings


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 3626a794-8918-439f-9ead-8038f03087f4

📥 Commits

Reviewing files that changed from the base of the PR and between c68ad3c and beacb94.

📒 Files selected for processing (9)
  • api/integrations/launch_darkly/services.py
  • api/segments/migrations/0031_add_segment_rules_data.py
  • api/segments/serializers.py
  • api/tests/unit/integrations/launch_darkly/snapshots/test_process_import_request__large_segments__correctly_imported__rules_data.json
  • api/tests/unit/integrations/launch_darkly/test_services.py
  • api/tests/unit/segments/test_unit_segments_migrations.py
  • docs/docs/deployment-self-hosting/observability/_events-catalogue.md
  • mcp/src/flagsmith_mcp/openapi.json
  • openapi.yaml
💤 Files with no reviewable changes (3)
  • api/tests/unit/segments/test_unit_segments_migrations.py
  • api/tests/unit/integrations/launch_darkly/snapshots/test_process_import_request__large_segments__correctly_imported__rules_data.json
  • api/segments/serializers.py

Comment thread api/tests/unit/integrations/launch_darkly/test_services.py
Comment thread openapi.yaml
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.

Store segment rule JSON alongside existing models

2 participants