Skip to content

Conversation

@ayushjain17
Copy link
Collaborator

@ayushjain17 ayushjain17 commented Jan 16, 2026

Problem

Describe the problem you are trying to solve here

Solution

Provide a brief summary of your solution so that reviewers can understand your code

Environment variable changes

What ENVs need to be added or changed

Pre-deployment activity

Things needed to be done before deploying this change (if any)

Post-deployment activity

Things needed to be done after deploying this change (if any)

API changes

Endpoint Method Request body Response Body
API GET/POST, etc request response

Possible Issues in the future

Describe any possible issues that could occur because of this change

Summary by CodeRabbit

  • Refactor
    • Optimized internal cohort schema update mechanism for improved efficiency and streamlined data handling.

✏️ Tip: You can customize this high-level summary in your review settings.

@ayushjain17 ayushjain17 requested a review from a team as a code owner January 16, 2026 15:41
@semanticdiff-com
Copy link

semanticdiff-com bot commented Jan 16, 2026

Review changes with  SemanticDiff

Changed Files
File Status
  crates/frontend/src/components/cohort_schema.rs  58% smaller

@coderabbitai
Copy link

coderabbitai bot commented Jan 16, 2026

Walkthrough

Modified the schema update mechanism in the cohort schema component to use in-place updates via cohort_schema_format_rws.update instead of explicit mutations on cloned schemas, eliminating redundant r#enum list management in Add and Update handlers.

Changes

Cohort / File(s) Summary
Schema mutation refactoring
crates/frontend/src/components/cohort_schema.rs
Replaced explicit cloning and mutation of schema with definitions map in-place updates; removed manual r#enum list appends in Add/Update handlers; now relies solely on definitions map for tracking new enum entries

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 A rabbit's ode to cleaner mutations,
No cloning, no scattered distributions,
Just update the map with graceful care,
Let definitions float through the air!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(UI): Cohort enum update behaviour' is directly related to the main change, which refactors how enum updates are handled in the cohort schema component.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings


📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8355849 and 8bed68f.

📒 Files selected for processing (1)
  • crates/frontend/src/components/cohort_schema.rs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-05T12:37:05.828Z
Learnt from: ayushjain17
Repo: juspay/superposition PR: 815
File: crates/frontend/src/components/datetime.rs:70-95
Timestamp: 2026-01-05T12:37:05.828Z
Learning: Documentation should reflect build-target dependent behavior. For functions that differ between wasm32 and non-wasm32 (e.g., datetime parsing/normalization), explicitly document the intent and the platform-specific logic in the code and/or a nearby comment. Prefer using cfg attributes to clearly separate implementations for wasm32 vs non-wasm32 and add tests that cover both builds. In this case, the function should be noted as: wasm32 -> local midnight interpreted then converted to UTC; non-wasm32 (SSR) -> UTC midnight directly. Ensure any related behavior is consistent with the SSR/CSR architecture and that downstream code relies on the clarified semantics.

Applied to files:

  • crates/frontend/src/components/cohort_schema.rs
📚 Learning: 2026-01-07T20:38:53.153Z
Learnt from: ayushjain17
Repo: juspay/superposition PR: 827
File: crates/superposition_core/src/config.rs:154-154
Timestamp: 2026-01-07T20:38:53.153Z
Learning: Deprecate jsonlogic-based context conditions and migrate to simple key-value pair map conditions across the codebase. Replace jsonlogic::apply usage with superposition_types::apply for condition evaluation. Update all relevant modules (notably Rust files under crates) to use the new approach, remove jsonlogic dependencies where applicable, and run full compilation and tests to catch regressions. Ensure context evaluation logic consistently uses key-value maps and that the architectural change is reflected in unit/integration tests.

Applied to files:

  • crates/frontend/src/components/cohort_schema.rs
🔇 Additional comments (1)
crates/frontend/src/components/cohort_schema.rs (1)

316-319: LGTM! Correct fix for update behavior.

The change correctly updates only the definitions map without modifying the r#enum list. This is appropriate because:

  1. The cohort name is already present in r#enum from when it was first added
  2. The name input is disabled during edit (disabled=edit at line 469), preventing name changes
  3. Using update() is more efficient than the clone-mutate-set pattern

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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 and usage tips.

@ayushjain17 ayushjain17 added bug Something isn't working UI things relating to the UI quick win An item or task that can be quickly closed and gives us an easy win labels Jan 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working quick win An item or task that can be quickly closed and gives us an easy win UI things relating to the UI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants