feat(user_cohort): add user cohort#1116
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughChangesUser cohort dimension support
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new USER_COHORT dimension type to support merchant-specific cohort definitions (via context overrides on a generated, system-owned default-config key), and updates resolution/validation so cohorts can be evaluated using merchant-resolved definitions in a second pass. The change spans the Smithy model + generated SDKs, backend validation + CRUD rules around the generated key, core evaluation logic, and frontend UI/UX for creating/viewing user cohorts.
Changes:
- Add
USER_COHORTto the SmithyDimensionTypeunion and propagate it through OpenAPI/docs and all generated SDKs (Rust/JS/Python/Java/Haskell). - Implement user cohort definition validation + two-pass cohort evaluation (resolve definitions first, then evaluate cohorts), and remove generated definition keys from resolved config output.
- Enforce “generated key is dimension-owned” behavior: create/update/delete generated default-config keys only via dimension operations; block default-config APIs from mutating user-cohort-owned keys; update frontend to reflect locked ownership.
Reviewed changes
Copilot reviewed 56 out of 57 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| smithy/models/dimension.smithy | Adds USER_COHORT to the Smithy DimensionType union. |
| docs/docs/api/update-dimension.StatusCodes.json | Updates API schema docs to include USER_COHORT. |
| docs/docs/api/update-dimension.api.mdx | Regenerates API docs (patch payload updated). |
| docs/docs/api/Superposition.openapi.json | Updates OpenAPI schema to include USER_COHORT variant. |
| docs/docs/api/list-versions.StatusCodes.json | Updates API schema docs to include USER_COHORT. |
| docs/docs/api/list-versions.api.mdx | Regenerates API docs (patch payload updated). |
| docs/docs/api/list-dimensions.StatusCodes.json | Updates API schema docs to include USER_COHORT. |
| docs/docs/api/list-dimensions.api.mdx | Regenerates API docs (patch payload updated). |
| docs/docs/api/get-version.StatusCodes.json | Updates API schema docs to include USER_COHORT. |
| docs/docs/api/get-version.api.mdx | Regenerates API docs (patch payload updated). |
| docs/docs/api/get-dimension.StatusCodes.json | Updates API schema docs to include USER_COHORT. |
| docs/docs/api/get-dimension.api.mdx | Regenerates API docs (patch payload updated). |
| docs/docs/api/get-config.StatusCodes.json | Updates API schema docs to include USER_COHORT. |
| docs/docs/api/get-config.api.mdx | Regenerates API docs (patch payload updated). |
| docs/docs/api/create-dimension.StatusCodes.json | Updates API schema docs to include USER_COHORT. |
| docs/docs/api/create-dimension.RequestSchema.json | Updates create-dimension request schema docs to include USER_COHORT. |
| docs/docs/api/create-dimension.api.mdx | Regenerates API docs (patch payload updated). |
| crates/superposition_types/src/logic/tests.rs | Updates tests for renamed start-dimension function and local cohort evaluation helper. |
| crates/superposition_types/src/logic.rs | Adds user cohort evaluation/validation helpers and two-pass cohort evaluation support. |
| crates/superposition_types/src/database/models/cac.rs | Adds DimensionType::UserCohort, parsing/display updates, and tests for strict parsing. |
| crates/superposition_types/src/config.rs | Ensures prefix filtering accounts for user cohort generated keys during filtering. |
| crates/superposition_sdk/src/types/_dimension_type.rs | Adds Rust SDK DimensionType::UserCohort variant helpers. |
| crates/superposition_sdk/src/protocol_serde/shape_dimension_type.rs | Adds serde (de)serialization support for USER_COHORT. |
| crates/superposition_provider/src/utils.rs | Switches provider-side evaluation to core eval_cac and adds SDK→types conversion for UserCohort. |
| crates/superposition_core/tests/format_integration.rs | Adds JSON import/export round-trip tests for user cohort + generated key pairing and override validation. |
| crates/superposition_core/src/format/tests/toml.rs | Adjusts error expectations for malformed cohort dimension types. |
| crates/superposition_core/src/format/tests/json.rs | Adjusts error expectations for malformed cohort dimension types. |
| crates/superposition_core/src/format.rs | Adds import/export validation for user cohort dimensions and generated-key constraints + override validation. |
| crates/superposition_core/src/config.rs | Implements two-pass resolution entrypoints and removes generated keys from resolved output. |
| crates/frontend/src/types.rs | Adds frontend enum option for “User Cohort”. |
| crates/frontend/src/pages/dimension.rs | Updates dimension view to handle/display user cohort similarly to local cohort. |
| crates/frontend/src/pages/default_config.rs | Locks default-config edit/delete UI for system-owned user cohort definition keys. |
| crates/frontend/src/components/dimension_form.rs | Adds form support for user cohort type; disables value-compute fn selection for cohort types. |
| crates/context_aware_config/src/helpers.rs | Updates cohort evaluation entrypoints to new start-dimension function; skips local/user cohort in remote cohort dependency evaluation. |
| crates/context_aware_config/src/api/dimension/validations.rs | Adds user cohort override validation and updates cohort base-dimension validation rules. |
| crates/context_aware_config/src/api/dimension/handlers.rs | Creates/updates/deletes generated default-config keys for user cohort dimensions and validates overrides. |
| crates/context_aware_config/src/api/default_config/handlers.rs | Blocks default-config create/update/delete when key is owned by a user cohort dimension. |
| crates/context_aware_config/src/api/default_config.rs | Re-exports key-usage lookup for dimension deletion checks. |
| crates/context_aware_config/src/api/context/validations.rs | Validates user cohort definition overrides placed in contexts. |
| crates/context_aware_config/src/api/context/helpers.rs | Replaces cac_client::merge usage with core merge and updates start-dimension function reference. |
| crates/context_aware_config/src/api/config/helpers.rs | Switches to core eval functions and resolves user cohorts for “explain” flows. |
| crates/context_aware_config/Cargo.toml | Removes cac_client dependency. |
| clients/python/sdk/superposition_sdk/models.py | Adds Python SDK union variant model for USER_COHORT. |
| clients/python/sdk/superposition_sdk/_private/schemas.py | Adds Python SDK schema member for USER_COHORT. |
| clients/javascript/sdk/src/models/models_0.ts | Adds JS SDK DimensionType.USER_COHORTMember and visitor support. |
| clients/javascript/sdk/src/commands/UpdateDimensionCommand.ts | Updates JS SDK docs/examples to include USER_COHORT. |
| clients/javascript/sdk/src/commands/ListVersionsCommand.ts | Updates JS SDK docs/examples to include USER_COHORT. |
| clients/javascript/sdk/src/commands/ListDimensionsCommand.ts | Updates JS SDK docs/examples to include USER_COHORT. |
| clients/javascript/sdk/src/commands/GetVersionCommand.ts | Updates JS SDK docs/examples to include USER_COHORT. |
| clients/javascript/sdk/src/commands/GetDimensionCommand.ts | Updates JS SDK docs/examples to include USER_COHORT. |
| clients/javascript/sdk/src/commands/GetConfigCommand.ts | Updates JS SDK docs/examples to include USER_COHORT. |
| clients/javascript/sdk/src/commands/CreateDimensionCommand.ts | Updates JS SDK docs/examples to include USER_COHORT. |
| clients/java/sdk/src/main/java/io/juspay/superposition/model/DimensionType.java | Adds Java SDK UserCohortMember union member and builder wiring. |
| clients/java/sdk/src/main/java/io/juspay/superposition/client/SuperpositionClientImpl.java | Minor ordering change in Java client type registry. |
| clients/java/sdk/src/main/java/io/juspay/superposition/client/SuperpositionAsyncClientImpl.java | Minor ordering change in Java async client type registry. |
| clients/haskell/sdk/Io/Superposition/Model/DimensionType.hs | Adds Haskell SDK UserCohort variant and parser error message update. |
| Cargo.lock | Removes cac_client lock entry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pub fn filter_by_prefix(self, prefix_list: &HashSet<String>) -> Self { | ||
| let filtered_default_config = self.filter_default_by_prefix(prefix_list); | ||
| let mut effective_prefixes = prefix_list.clone(); | ||
| effective_prefixes.extend(user_cohort_dimension_names(&self.dimensions)); | ||
| let filtered_default_config = self.filter_default_by_prefix(&effective_prefixes); | ||
|
|
| let rows = contexts::dsl::contexts | ||
| .schema_name(schema_name) | ||
| .load::<Context>(conn)?; | ||
|
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/context_aware_config/src/api/dimension/handlers.rs`:
- Around line 790-796: Update the generated default-config deletion in the
is_user_cohort branch to use the same optional-result handling as the preceding
dimension deletion, so an absent row does not propagate Diesel’s NotFound error.
Preserve the existing delete filter, schema_name, and transaction flow.
In `@crates/frontend/src/pages/default_config.rs`:
- Around line 37-41: Update the dimension ownership check around dimensions::get
so lookup errors are not collapsed into false: treat only a confirmed missing
dimension as non-user-owned, while propagating transient, authorization, and
other failures. Preserve the existing UserCohort dimension match for successful
lookups and adjust the surrounding async flow to return the error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 85bb8eb0-c5cb-4b05-b847-dd7aa5259ba2
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (56)
clients/haskell/sdk/Io/Superposition/Model/DimensionType.hsclients/java/sdk/src/main/java/io/juspay/superposition/client/SuperpositionAsyncClientImpl.javaclients/java/sdk/src/main/java/io/juspay/superposition/client/SuperpositionClientImpl.javaclients/java/sdk/src/main/java/io/juspay/superposition/model/DimensionType.javaclients/javascript/sdk/src/commands/CreateDimensionCommand.tsclients/javascript/sdk/src/commands/GetConfigCommand.tsclients/javascript/sdk/src/commands/GetDimensionCommand.tsclients/javascript/sdk/src/commands/GetVersionCommand.tsclients/javascript/sdk/src/commands/ListDimensionsCommand.tsclients/javascript/sdk/src/commands/ListVersionsCommand.tsclients/javascript/sdk/src/commands/UpdateDimensionCommand.tsclients/javascript/sdk/src/models/models_0.tsclients/python/sdk/superposition_sdk/_private/schemas.pyclients/python/sdk/superposition_sdk/models.pycrates/context_aware_config/Cargo.tomlcrates/context_aware_config/src/api/config/helpers.rscrates/context_aware_config/src/api/context/helpers.rscrates/context_aware_config/src/api/context/validations.rscrates/context_aware_config/src/api/default_config.rscrates/context_aware_config/src/api/default_config/handlers.rscrates/context_aware_config/src/api/dimension/handlers.rscrates/context_aware_config/src/api/dimension/validations.rscrates/context_aware_config/src/helpers.rscrates/frontend/src/components/dimension_form.rscrates/frontend/src/pages/default_config.rscrates/frontend/src/pages/dimension.rscrates/frontend/src/types.rscrates/superposition_core/src/config.rscrates/superposition_core/src/format.rscrates/superposition_core/src/format/tests/json.rscrates/superposition_core/src/format/tests/toml.rscrates/superposition_core/tests/format_integration.rscrates/superposition_provider/src/utils.rscrates/superposition_sdk/src/protocol_serde/shape_dimension_type.rscrates/superposition_sdk/src/types/_dimension_type.rscrates/superposition_types/src/config.rscrates/superposition_types/src/database/models/cac.rscrates/superposition_types/src/logic.rscrates/superposition_types/src/logic/tests.rsdocs/docs/api/Superposition.openapi.jsondocs/docs/api/create-dimension.RequestSchema.jsondocs/docs/api/create-dimension.StatusCodes.jsondocs/docs/api/create-dimension.api.mdxdocs/docs/api/get-config.StatusCodes.jsondocs/docs/api/get-config.api.mdxdocs/docs/api/get-dimension.StatusCodes.jsondocs/docs/api/get-dimension.api.mdxdocs/docs/api/get-version.StatusCodes.jsondocs/docs/api/get-version.api.mdxdocs/docs/api/list-dimensions.StatusCodes.jsondocs/docs/api/list-dimensions.api.mdxdocs/docs/api/list-versions.StatusCodes.jsondocs/docs/api/list-versions.api.mdxdocs/docs/api/update-dimension.StatusCodes.jsondocs/docs/api/update-dimension.api.mdxsmithy/models/dimension.smithy
💤 Files with no reviewable changes (1)
- crates/context_aware_config/Cargo.toml
| if is_user_cohort { | ||
| diesel::delete( | ||
| default_configs::table.filter(default_configs::key.eq(&name)), | ||
| ) | ||
| .schema_name(&workspace_context.schema_name) | ||
| .get_result::<DefaultConfig>(transaction_conn)?; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Use .optional() when deleting the generated default config.
get_result::<DefaultConfig>(transaction_conn)? hard-fails with NotFound if the generated row is ever absent (e.g. data drift), rolling back the whole transaction and leaving the dimension undeletable. The dimension deletion just above (L785-788) already uses .optional(); mirror it here.
🛡️ Proposed fix
if is_user_cohort {
diesel::delete(
default_configs::table.filter(default_configs::key.eq(&name)),
)
.schema_name(&workspace_context.schema_name)
- .get_result::<DefaultConfig>(transaction_conn)?;
+ .get_result::<DefaultConfig>(transaction_conn)
+ .optional()?;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if is_user_cohort { | |
| diesel::delete( | |
| default_configs::table.filter(default_configs::key.eq(&name)), | |
| ) | |
| .schema_name(&workspace_context.schema_name) | |
| .get_result::<DefaultConfig>(transaction_conn)?; | |
| } | |
| if is_user_cohort { | |
| diesel::delete( | |
| default_configs::table.filter(default_configs::key.eq(&name)), | |
| ) | |
| .schema_name(&workspace_context.schema_name) | |
| .get_result::<DefaultConfig>(transaction_conn) | |
| .optional()?; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/context_aware_config/src/api/dimension/handlers.rs` around lines 790 -
796, Update the generated default-config deletion in the is_user_cohort branch
to use the same optional-result handling as the preceding dimension deletion, so
an absent row does not propagate Diesel’s NotFound error. Preserve the existing
delete filter, schema_name, and transaction flow.
| dimensions::get(key, workspace, org_id) | ||
| .await | ||
| .is_ok_and(|dimension| { | ||
| matches!(dimension.dimension_type, DimensionType::UserCohort(_)) | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not treat dimension lookup failures as user-owned.
is_ok_and converts every dimensions::get error into false, so a transient/authorization failure exposes Edit and Delete controls for a user-cohort-owned key. Treat only a confirmed missing dimension as non-system-owned; propagate other failures.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/frontend/src/pages/default_config.rs` around lines 37 - 41, Update the
dimension ownership check around dimensions::get so lookup errors are not
collapsed into false: treat only a confirmed missing dimension as
non-user-owned, while propagating transient, authorization, and other failures.
Preserve the existing UserCohort dimension match for successful lookups and
adjust the surrounding async flow to return the error.
71c6483 to
2dd5a67
Compare
Problem
Superposition currently supports cohorts whose definitions are shared across the workspace. This does not support cases where cohort boundaries differ by merchant.
For example,
amount_bucketmay be derived fromamount, but merchantsauravmay definelow_valueas an amount below1,000, while another merchant may use2,000.The existing single-pass resolution cannot first select the merchant-specific definitions and then use those definitions to calculate the cohort.
Solution
Introduce a new
USER_COHORTdimension type with an explicitbased_ondimension.For example:
Creating an
amount_bucketuser cohort:amount_bucket.Resolution now happens in two passes:
amount_bucketusing the resolved definitions and then resolve the final configuration.Generated definition keys remain internal and are removed from the final resolved response.
The implementation also adds:
based_ondimension.USER_COHORTvariant.Cohort enum order defines priority. If multiple definitions match, the first matching option wins.
otherwiseremains the final fallback.Environment variable changes
No environment variables need to be added or changed.
Pre-deployment activity
No database migration or data backfill is required.
The Smithy-generated SDKs, provider, backend, and frontend should be released together because they must all understand the new
USER_COHORTdimension variant.Post-deployment activity
No mandatory post-deployment activity is required.
Recommended smoke test:
amount_bucketuser cohort derived fromamount.Summary by CodeRabbit
New Features
Bug Fixes