feat(cohorts): apply membership deltas to edge identities in batches by gagantrivedi · Pull Request #8213 · Flagsmith/flagsmith

gagantrivedi · 2026-08-04T09:26:25Z

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

Third PR of the cohort sync feature: the task-processor applier that materialises pending ledger rows as system_traits on DynamoDB identity documents. Stacked on #8211 (merged) and #8212 (open — its commits appear in this diff until it merges, after which this will be rebased).

DynamoIdentityWrapper.set_system_trait / unset_system_trait: idempotent single-attribute conditional writes (SET/REMOVE system_traits.<key>, trait_value param defaulting to True) — concurrent SDK writes to other attributes are never clobbered, and deleted identities never resurrect as ghost documents
apply_cohort_membership_deltas task: lock-free batches of 100, up to 10 batches per run then self-re-enqueues; DynamoDB throttling (all three throttle codes) maps to TaskBackoffError
State-guarded ledger flips: a row that changes state mid-batch stays pending and is corrected by a later batch
cohorts.membership.* events and a deltas-applied counter; docgen catalogues regenerated

Not production-reachable yet: nothing enqueues this task until the ingestion PRs land, and Edge must tolerate system_traits before real cohorts ship (#8212). Identifier byte-length validation is deliberately an ingestion-boundary concern, not an applier one.

How did you test this code?

18 cohort unit tests plus 14 wrapper tests against a real moto DynamoDB table: add/remove/merge paths, condition-failure retries and exhaustion, guarded flips, throttle backoff, batch-loop + re-enqueue drain, metric and structured-event assertions.

for more information, see https://pre-commit.ci

# Conflicts: # api/cohorts/models.py

vercel · 2026-08-04T09:26:31Z

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

3 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
docs	Ignored	Preview	Aug 5, 2026 10:21am
flagsmith-frontend-preview	Ignored	Preview	Aug 5, 2026 10:21am
flagsmith-frontend-staging	Ignored	Preview	Aug 5, 2026 10:21am

coderabbitai · 2026-08-04T09:26:34Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4840da43-b7a0-41ef-8812-75623cfedf54

📥 Commits

Reviewing files that changed from the base of the PR and between ecd836d and d03e36c.

📒 Files selected for processing (1)

api/util/mappers/sdk.py

📝 Walkthrough

Walkthrough

The change adds optional system traits to identity models and DynamoDB mappings. It adds conditional DynamoDB methods to set and unset system traits with retries. It adds services and a task to apply pending cohort additions and removals in bounded batches. The task handles eligibility checks, throttling backoff, re-queuing, metrics, and structured logs. Unit tests cover trait writes, mapping, membership processing, task errors, and batch continuation. Observability catalogues document the new events and metric.

Estimated code review effort: 4 (Complex) | ~45 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.}

for more information, see https://pre-commit.ci

codecov · 2026-08-04T09:33:20Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.72%. Comparing base (fc33002) to head (d03e36c).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #8213    +/-   ##
========================================
  Coverage   98.71%   98.72%            
========================================
  Files        1531     1542    +11     
  Lines       61263    61679   +416     
========================================
+ Hits        60475    60891   +416     
  Misses        788      788

☔ 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.

…into feat/cohort-membership-applier

…p-applier # Conflicts: # api/cohorts/models.py # api/tests/unit/cohorts/test_models.py

for more information, see https://pre-commit.ci

github-actions · 2026-08-05T08:43:13Z

Docker builds report

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

gagantrivedi · 2026-08-05T08:45:12Z

@themis-blindfold review

coderabbitai

Actionable comments posted: 3

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 562b1faa-9d78-4bbc-baff-dd83b4f4a90d

📥 Commits

Reviewing files that changed from the base of the PR and between d6da2ff and 931e8d5.

📒 Files selected for processing (19)

api/cohorts/constants.py
api/cohorts/metrics.py
api/cohorts/models.py
api/cohorts/services.py
api/cohorts/tasks.py
api/environments/dynamodb/constants.py
api/environments/dynamodb/wrappers/exceptions.py
api/environments/dynamodb/wrappers/identity_wrapper.py
api/tests/unit/cohorts/conftest.py
api/tests/unit/cohorts/test_services.py
api/tests/unit/cohorts/test_tasks.py
api/tests/unit/environments/dynamodb/wrappers/test_unit_dynamodb_identity_wrapper.py
api/tests/unit/util/mappers/test_unit_mappers_dynamodb.py
api/tests/unit/util/mappers/test_unit_mappers_sdk.py
api/util/engine_models/identities/models.py
api/util/mappers/dynamodb.py
api/util/mappers/sdk.py
docs/docs/deployment-self-hosting/observability/_events-catalogue.md
docs/docs/deployment-self-hosting/observability/_metrics-catalogue.md

github-actions · 2026-08-05T08:49:17Z

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

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

3 passed

Details

3 tests across 3 suites
39.3 seconds
d03e36c
🔄 Run: #19020 (attempt 1)

🗂️ Previous results

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

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

3 passed

Details

3 tests across 3 suites
32.4 seconds
d03e36c
🔄 Run: #19020 (attempt 1)

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

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

1 passed

Details

1 test across 1 suite
40.5 seconds
d03e36c
🔄 Run: #19020 (attempt 1)

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

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

1 passed

Details

1 test across 1 suite
33.6 seconds
d03e36c
🔄 Run: #19020 (attempt 1)

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

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

3 passed

Details

3 tests across 3 suites
33 seconds
ecd836d
🔄 Run: #19018 (attempt 1)

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

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

1 passed

Details

1 test across 1 suite
40.4 seconds
ecd836d
🔄 Run: #19018 (attempt 1)

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

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

23 passed
1 skipped

Details

24 tests across 18 suites
1 minute, 23 seconds
ecd836d
🔄 Run: #19018 (attempt 1)

Skipped tests

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

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

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

1 passed

Details

1 test across 1 suite
41.1 seconds
ecd836d
🔄 Run: #19018 (attempt 1)

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

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

3 passed

Details

3 tests across 3 suites
39.3 seconds
931e8d5
🔄 Run: #19017 (attempt 1)

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

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

3 passed

Details

3 tests across 3 suites
32.4 seconds
931e8d5
🔄 Run: #19017 (attempt 1)

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

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

1 passed

Details

1 test across 1 suite
46.2 seconds
931e8d5
🔄 Run: #19017 (attempt 1)

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

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

1 passed

Details

1 test across 1 suite
33.4 seconds
931e8d5
🔄 Run: #19017 (attempt 1)

github-actions · 2026-08-05T08:50:12Z

Visual Regression

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

themis-blindfold · 2026-08-05T08:51:43Z

⚖️ Themis review: ✅ Ship it

Third PR in the cohort sync stack: a task-processor applier that drains pending ledger rows into DynamoDB system_traits on Edge identity documents. The DynamoDB write layer is the heart of the change — three-branch conditional writes (document missing / system_traits missing / system_traits present) handle every concurrent-writer scenario without clobbering user traits or resurrecting deleted identities, and the retry loop with ConsistentRead makes those conditions meaningful. The service layer's state-guarded bulk flips ensure a row that transitions mid-batch stays pending for correction in a later pass. SDK document exclusion via Pydantic's __all__ exclude pattern keeps system_traits out of local-eval documents. Not production-reachable yet — nothing enqueues the task until the ingestion PRs land.

Area	Score
🎯 Correctness	4/5
🧪 Test coverage	5/5
📐 Code quality	5/5
🚀 Product impact	3/5

📝 Walkthrough

DynamoDB identity wrapper — set_system_trait and unset_system_trait methods with conditional writes, retry loop, ghost-document prevention, and idempotent no-ops when the value already matches.
Cohort service — apply_pending_memberships fetches a batch of pending rows, writes each to DynamoDB, then bulk-flips DB state with guards against mid-batch transitions.
Cohort task — apply_cohort_membership_deltas loops up to 10 batches per run, maps DynamoDB throttle errors to TaskBackoffError, and self-re-enqueues when more work remains.
SDK document exclusion — SDK_DOCUMENT_EXCLUDE converted from set to dict to support nested Pydantic exclude; system_traits stripped from every identity override in the local-eval document.
Identity model & mapper — system_traits field added to IdentityModel; _NULLABLE_IDENTITY_KEY_ATTRIBUTES updated to omit the key from DynamoDB documents when unset.
Observability — Prometheus counter with operation label, structured log events for applied/skipped/throttled, doc catalogue entries for all three.

🧪 How to verify

Run the cohort unit tests: pytest api/tests/unit/cohorts/ -v — all 18 tests should pass.
Run the wrapper tests: pytest api/tests/unit/environments/dynamodb/wrappers/test_unit_dynamodb_identity_wrapper.py -v -k system_trait — all 14 new tests should pass.
Run the mapper tests: pytest api/tests/unit/util/mappers/test_unit_mappers_dynamodb.py api/tests/unit/util/mappers/test_unit_mappers_sdk.py -v -k system_trait — round-trip and SDK exclusion tests should pass.
Verify mypy strict passes: mypy api/cohorts/ api/environments/dynamodb/wrappers/identity_wrapper.py api/util/mappers/sdk.py --strict.

Automate: all of the above are already covered by the CI unit test matrix (3.11/3.12/3.13).

Product take: Internal infrastructure for cohort-based segment membership on Edge. No user-visible change yet — the task is registered but unreachable until ingestion PRs land. When it ships, it enables syncing uploaded identifier lists to Edge identities without polling, which is a meaningful capability for customers with large cohort-driven segments.

🧭 Assumptions & unverified claims

The review assumes TaskBackoffError triggers the task processor's built-in retry with backoff, as described in the PR. The task processor's retry semantics were not independently verified.
The __all__ Pydantic exclude key is assumed to work correctly for list fields in the pinned Pydantic v2 version; the test covers it end-to-end but the Pydantic version was not checked.
The PR description states SystemTraitWriteRaceError propagating as an unhandled task exception is acceptable because the task processor will retry. This was accepted at face value — the task processor's unhandled-exception retry policy was not verified.

A conditional write that never has to fire is the best kind of defensive code — may your races remain theoretical. · reviewed at 931e8d5

…utes

coderabbitai

Caution

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

⚠️ Outside diff range comments (1)

api/environments/dynamodb/wrappers/identity_wrapper.py (1)

125-167: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Do not recreate an identity after a delete race.

If the first read finds an identity and a concurrent delete occurs before the conditional write, the write fails and line 167 retries. The next strongly consistent read returns no document, so lines 125-135 successfully put_item and recreate the deleted identity.

Track that this invocation previously observed a document. If a later retry finds it absent, return without creating a replacement document. Add a regression test for read → delete → conditional-write failure → retry.

Based on PR objectives: system-trait writes must not resurrect deleted identities.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 91db7765-a725-48c0-ac87-c8de88fda6af

📥 Commits

Reviewing files that changed from the base of the PR and between 931e8d5 and ecd836d.

📒 Files selected for processing (3)

api/environments/dynamodb/constants.py
api/environments/dynamodb/wrappers/identity_wrapper.py
api/tests/unit/environments/dynamodb/wrappers/test_unit_dynamodb_identity_wrapper.py

💤 Files with no reviewable changes (1)

api/environments/dynamodb/constants.py

…o feat/identity-system-traits

…p-applier

Zaimwa9

Solid work and crystal clear, nothing to add!

gagantrivedi and others added 8 commits August 4, 2026 10:28

feat(cohorts): add cohorts app with membership ledger models

3f1cf1f

feat(identities): add system_traits to engine identity document model

60bfba4

fix(identities): exclude system_traits from SDK environment document

4a55cee

[pre-commit.ci] auto fixes from pre-commit.com hooks

3d1fbef

for more information, see https://pre-commit.ci

chore: merge system_traits branch into applier branch

ce7b24e

feat(cohorts): apply membership deltas to edge identities in batches

4ebaaa0

fix(cohorts): enforce one active cohort per segment

5fa8c99

Merge branch 'feat/cohort-sync' into feat/cohort-membership-applier

6c502b9

# Conflicts: # api/cohorts/models.py

[pre-commit.ci] auto fixes from pre-commit.com hooks

2a36df6

for more information, see https://pre-commit.ci

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 4, 2026

gagantrivedi added 3 commits August 4, 2026 15:04

test(cohorts): pin partial unique condition via direct deleted_at update

1d09e0a

Merge branch 'feat/cohort-sync' into feat/cohort-membership-applier

35bfec4

Merge remote-tracking branch 'origin/feat/cohort-membership-applier' …

75b4d38

…into feat/cohort-membership-applier

github-actions Bot added docs Documentation updates feature New feature or request and removed feature New feature or request docs Documentation updates labels Aug 4, 2026

Base automatically changed from feat/cohort-sync to main August 4, 2026 09:54

Merge remote-tracking branch 'origin/main' into feat/cohort-membershi…

96679df

…p-applier # Conflicts: # api/cohorts/models.py # api/tests/unit/cohorts/test_models.py

vercel Bot had a problem deploying to Preview – docs August 4, 2026 10:56 Failure

[pre-commit.ci] auto fixes from pre-commit.com hooks

ed368a6

for more information, see https://pre-commit.ci

github-actions Bot added docs Documentation updates and removed feature New feature or request docs Documentation updates labels Aug 4, 2026

gagantrivedi requested a review from a team as a code owner August 5, 2026 08:42

gagantrivedi requested review from Holmus, Zaimwa9 and emyller and removed request for a team, Holmus and emyller August 5, 2026 08:42

github-actions Bot removed the feature New feature or request label Aug 5, 2026

github-actions Bot added the feature New feature or request label Aug 5, 2026

coderabbitai Bot reviewed Aug 5, 2026

View reviewed changes

Comment thread api/environments/dynamodb/constants.py Outdated

Comment thread api/environments/dynamodb/wrappers/identity_wrapper.py

Comment thread api/util/mappers/sdk.py Outdated

refactor(identities): confirm system_traits init from returned attrib…

ecd836d

…utes

github-actions Bot added docs Documentation updates feature New feature or request and removed feature New feature or request docs Documentation updates labels Aug 5, 2026

coderabbitai Bot reviewed Aug 5, 2026

View reviewed changes

gagantrivedi added 3 commits August 5, 2026 15:49

fix(identities): drop private pydantic IncEx import

77dd067

Merge remote-tracking branch 'origin/feat/identity-system-traits' int…

62c2b9d

…o feat/identity-system-traits

Merge branch 'feat/identity-system-traits' into feat/cohort-membershi…

d03e36c

…p-applier

github-actions Bot added docs Documentation updates and removed feature New feature or request docs Documentation updates labels Aug 5, 2026

Zaimwa9 approved these changes Aug 5, 2026

View reviewed changes

Uh oh!

Conversation

gagantrivedi commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

How did you test this code?

Uh oh!

vercel Bot commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Walkthrough

Uh oh!

codecov Bot commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Docker builds report

Uh oh!

gagantrivedi commented Aug 5, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

Uh oh!

github-actions Bot commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Visual Regression

Uh oh!

themis-blindfold Bot commented Aug 5, 2026

⚖️ Themis review: ✅ Ship it

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Zaimwa9 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gagantrivedi commented Aug 4, 2026 •

edited

Loading

vercel Bot commented Aug 4, 2026 •

edited

Loading

coderabbitai Bot commented Aug 4, 2026 •

edited

Loading

codecov Bot commented Aug 4, 2026 •

edited

Loading

github-actions Bot commented Aug 5, 2026 •

edited

Loading

github-actions Bot commented Aug 5, 2026 •

edited

Loading

github-actions Bot commented Aug 5, 2026 •

edited

Loading