UMS Task: https://katha.shikshalokam.org/story-view-1885.html - #7
Merged
kiranharidas187 merged 6 commits intoJun 29, 2026
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 19 minutes and 58 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR threads Elevate UMS profile data through websocket authentication, chat service execution, and profile-related endpoints. It also relaxes profile constraints, updates onboarding persistence, and adds bot vernacular uniqueness rules. ChangesElevate UMS Profile Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
chatbot/services/response_handlers/common_handler.py (1)
845-863:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPersist onboarding completion only after Elevate profile sync succeeds.
Line 846-Line 849 marks onboarding complete before the Elevate write at Line 853-Line 860, and the Elevate result is ignored. Since district/state are no longer stored locally, a failed
update_elevate_profile(...)can silently drop submitted profile context whileis_onboarding_completedis alreadyTrue.Proposed fix
- other_params = profile.other_params or {} - other_params['is_onboarding_completed'] = True - profile.other_params = other_params - profile.save(update_fields=['other_params']) - logger.info('[submit_user_context] marked onboarding complete for profile id=%s', profile_id) - - if access_token: + elevate_synced = True + if access_token: from chatbot.utils.elevate.profile_utils import update_elevate_profile - update_elevate_profile( + elevate_resp = update_elevate_profile( access_token=access_token, name=arguments.get('name'), role=arguments.get('role'), school_name=arguments.get('school_name'), district=arguments.get('district'), state=arguments.get('state'), ) + elevate_synced = bool(elevate_resp) + + if not elevate_synced: + logger.error('[submit_user_context] elevate sync failed; onboarding not marked complete for id=%s', profile_id) + return + + other_params = profile.other_params or {} + other_params['is_onboarding_completed'] = True + profile.other_params = other_params + profile.save(update_fields=['other_params']) + logger.info('[submit_user_context] marked onboarding complete for profile id=%s', profile_id)🤖 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 `@chatbot/services/response_handlers/common_handler.py` around lines 845 - 863, The onboarding completion is being persisted to the profile before the update_elevate_profile call completes. If update_elevate_profile fails, is_onboarding_completed is already set to True while the profile context may not have been synced to Elevate. Move the profile.other_params assignment and profile.save(update_fields=['other_params']) call to execute after the successful completion of update_elevate_profile, ensuring that onboarding is only marked complete when the Elevate profile sync succeeds.chatbot/models/profile_models.py (1)
17-20:⚠️ Potential issue | 🟡 MinorRemove or clarify the unused
get_file_upload_pathmethod.The method at lines 17–20 is never used by any FileField in the Profile model and appears to be dead code. Additionally, if it were called, it would crash when
companyisNone(line 18:self.company.slug) since the field is now nullable (line 32). Either remove this method or document why it's retained.🤖 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 `@chatbot/models/profile_models.py` around lines 17 - 20, The `get_file_upload_path` method is unused dead code that will crash when the company field is None (since it attempts to access self.company.slug without null checking). Either remove this method entirely if it is not needed, or if you must retain it, add a null check for self.company before accessing its slug attribute and include a clear comment documenting the intended use case and why it is being kept despite not currently being referenced by any FileField.
🧹 Nitpick comments (4)
shikshalokam/views/profile_views.py (1)
24-50: ⚡ Quick winFail fast on
DEFAULT_COMPANY_SLUGbefore calling Elevate.Line 47-Line 50 checks configuration after the Elevate call at Line 24. If env is misconfigured, this endpoint still does external work and then returns 500 on every request.
Proposed refactor
- profile_details = handle_elevate_profile(access_token=access_token) + company_slug = os.getenv('DEFAULT_COMPANY_SLUG') + if not company_slug: + logger.error('[read_elevate_profile] DEFAULT_COMPANY_SLUG is not set') + return Response({'status': 'error', 'message': 'Server misconfiguration.'}, status=500) + + profile_details = handle_elevate_profile(access_token=access_token) @@ - company_slug = os.getenv('DEFAULT_COMPANY_SLUG') - if not company_slug: - logger.error('[read_elevate_profile] DEFAULT_COMPANY_SLUG is not set') - return Response({'status': 'error', 'message': 'Server misconfiguration.'}, status=500) -🤖 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 `@shikshalokam/views/profile_views.py` around lines 24 - 50, The DEFAULT_COMPANY_SLUG environment variable check is occurring after the expensive handle_elevate_profile external API call, causing unnecessary work when the configuration is misconfigured. Move the DEFAULT_COMPANY_SLUG validation logic (which checks if the environment variable exists and returns a 500 error if not) to the beginning of the function before calling handle_elevate_profile, so the endpoint fails fast without making external API calls when the required configuration is missing.chatbot/consumers/async_consumer.py (1)
62-72: ⚡ Quick winElevate profile fetch is called even when
access_tokenisNone.
sync_elevate_profileis called unconditionally on line 62. If the client doesn't provide anaccess_token, this will make a request to Elevate with aNonetoken in theX-auth-tokenheader, likely returning 401. While this is handled, it's an unnecessary network call.Consider guarding:
♻️ Proposed optimization
- elevate_result = await self.sync_elevate_profile(self.access_token) + elevate_result = {} + if self.access_token: + elevate_result = await self.sync_elevate_profile(self.access_token)🤖 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 `@chatbot/consumers/async_consumer.py` around lines 62 - 72, The `sync_elevate_profile` method is called unconditionally on line 62 even when `self.access_token` is `None`, which results in unnecessary network requests to the Elevate server that will fail with a 401 response. Add a guard condition to check if `self.access_token` is not `None` before calling `sync_elevate_profile`. If the token is `None`, you should return early or handle the authentication failure directly without making the unnecessary network request.chatbot/models/profile_models.py (1)
62-70: ⚖️ Poor tradeoffApplication-level duplicate check is susceptible to race conditions.
Removing the
unique_togetherDB constraint and replacing it with an application-level check insave()allows concurrent requests to create duplicate(email, company_id)profiles if they pass the.exists()check simultaneously before either commits.If duplicates must be prevented reliably, consider adding a partial unique index at the database level for the traditional flow:
CREATE UNIQUE INDEX profile_email_company_uniq ON chatbot_profile (email, company_id) WHERE userid IS NULL;Alternatively, accept that duplicates are rare and handle them via error recovery.
🤖 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 `@chatbot/models/profile_models.py` around lines 62 - 70, The duplicate check in the Profile save() method using the .exists() query is vulnerable to race conditions where concurrent requests can both pass the check before either commits. To fix this reliably, add a database-level constraint by creating a migration that adds a partial unique index on the (email, company_id) columns where userid IS NULL. Alternatively, remove the application-level check in save() and wrap the super().save() call in a try-except block to catch IntegrityError exceptions (which would be raised by the database constraint), then handle the error appropriately by raising a ValueError with a user-friendly message or logging the conflict.chatbot/consumers/async_base_consumer.py (1)
39-42: 💤 Low valueConsider logging the swallowed exception for debugging.
Silently swallowing exceptions makes debugging harder. While the intent (avoiding double-close errors) is valid, logging at debug level would help diagnose unexpected issues.
♻️ Proposed improvement
finally: try: await self.close() - except Exception: - pass + except Exception as e: + logger.debug('close() raised during disconnect: %s', e)🤖 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 `@chatbot/consumers/async_base_consumer.py` around lines 39 - 42, The exception handler for the `await self.close()` call in the try-except block is silently swallowing the exception with `pass`, which makes debugging difficult. Add a debug-level logging statement in the except Exception block to capture and log the exception details before the pass statement, so that unexpected errors during the close operation can be investigated if needed.Source: Linters/SAST tools
🤖 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 `@chatbot/consumers/async_consumer.py`:
- Around line 191-206: Rename the variable JWT_PUBLIC_KEY to JWT_SECRET_KEY (or
JWT_SYMMETRIC_SECRET) throughout the codebase to accurately reflect that HS256
is a symmetric HMAC algorithm requiring a shared secret key, not a public key.
This includes updating the reference in the handle_access_token function where
it checks PUBLIC_KEY and in any configuration or environment variable
definitions. Additionally, locate and correct the misleading comment in
free_flow_consumer.py around line 160 that incorrectly references RS256 to
instead reference HS256, ensuring consistency with the actual algorithm
implementation at line 166.
In `@chatbot/services/core/base_service.py`:
- Around line 17-20: The CompanyBot model lacks a uniqueness constraint on the
route field, causing the fallback route-only lookup in the conditional block to
be unsafe in multi-tenant scenarios. Add a unique_together constraint to the
CompanyBot.Meta class that enforces uniqueness on the combination of company and
route fields, ensuring that the database prevents duplicate route values within
the same company. If a global route uniqueness is required instead, use
unique=True on the route field. Alternatively, if company context should always
be required, replace the fallback else block with a raise ValueError statement
instead of attempting the unsafe route-only get() lookup.
In `@chatbot/services/core/prompt_builder.py`:
- Around line 15-17: The address assignment in the prompt builder uses a
truthiness check on ums_profile which fails for empty dictionaries, causing the
code to incorrectly fall back to ProfileAddress lookup. Replace the truthiness
check `if ums_profile` with an explicit None check `if ums_profile is not None`
in the ternary conditional that assigns the address variable. This ensures that
even empty ums_profile dictionaries are properly recognized and prevent the
unwanted fallback to the profile.profile_address.all().first() lookup.
In `@chatbot/utils/elevate/profile_utils.py`:
- Around line 17-23: The handle_elevate_profile function constructs a URL using
elevate_base_url without verifying it has been properly set from the
ELEVATE_BASE_URL environment variable. Add a guard at the beginning of the
function to check if elevate_base_url is None, and raise a clear exception with
a descriptive error message indicating that the ELEVATE_BASE_URL environment
variable is not configured, rather than allowing the function to proceed and
create a malformed URL that results in confusing requests errors.
---
Outside diff comments:
In `@chatbot/models/profile_models.py`:
- Around line 17-20: The `get_file_upload_path` method is unused dead code that
will crash when the company field is None (since it attempts to access
self.company.slug without null checking). Either remove this method entirely if
it is not needed, or if you must retain it, add a null check for self.company
before accessing its slug attribute and include a clear comment documenting the
intended use case and why it is being kept despite not currently being
referenced by any FileField.
In `@chatbot/services/response_handlers/common_handler.py`:
- Around line 845-863: The onboarding completion is being persisted to the
profile before the update_elevate_profile call completes. If
update_elevate_profile fails, is_onboarding_completed is already set to True
while the profile context may not have been synced to Elevate. Move the
profile.other_params assignment and profile.save(update_fields=['other_params'])
call to execute after the successful completion of update_elevate_profile,
ensuring that onboarding is only marked complete when the Elevate profile sync
succeeds.
---
Nitpick comments:
In `@chatbot/consumers/async_base_consumer.py`:
- Around line 39-42: The exception handler for the `await self.close()` call in
the try-except block is silently swallowing the exception with `pass`, which
makes debugging difficult. Add a debug-level logging statement in the except
Exception block to capture and log the exception details before the pass
statement, so that unexpected errors during the close operation can be
investigated if needed.
In `@chatbot/consumers/async_consumer.py`:
- Around line 62-72: The `sync_elevate_profile` method is called unconditionally
on line 62 even when `self.access_token` is `None`, which results in unnecessary
network requests to the Elevate server that will fail with a 401 response. Add a
guard condition to check if `self.access_token` is not `None` before calling
`sync_elevate_profile`. If the token is `None`, you should return early or
handle the authentication failure directly without making the unnecessary
network request.
In `@chatbot/models/profile_models.py`:
- Around line 62-70: The duplicate check in the Profile save() method using the
.exists() query is vulnerable to race conditions where concurrent requests can
both pass the check before either commits. To fix this reliably, add a
database-level constraint by creating a migration that adds a partial unique
index on the (email, company_id) columns where userid IS NULL. Alternatively,
remove the application-level check in save() and wrap the super().save() call in
a try-except block to catch IntegrityError exceptions (which would be raised by
the database constraint), then handle the error appropriately by raising a
ValueError with a user-friendly message or logging the conflict.
In `@shikshalokam/views/profile_views.py`:
- Around line 24-50: The DEFAULT_COMPANY_SLUG environment variable check is
occurring after the expensive handle_elevate_profile external API call, causing
unnecessary work when the configuration is misconfigured. Move the
DEFAULT_COMPANY_SLUG validation logic (which checks if the environment variable
exists and returns a 500 error if not) to the beginning of the function before
calling handle_elevate_profile, so the endpoint fails fast without making
external API calls when the required configuration is missing.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8224272d-9ea3-4b72-8f4c-12f5da646cc5
📒 Files selected for processing (14)
chatbot/celery_tasks/flow_tasks.pychatbot/consumers/async_base_consumer.pychatbot/consumers/async_consumer.pychatbot/migrations/0086_alter_profile_unique_together_and_more.pychatbot/models/profile_models.pychatbot/services/core/base_service.pychatbot/services/core/orchestrator.pychatbot/services/core/prompt_builder.pychatbot/services/response_handlers/common_handler.pychatbot/utils/elevate/profile_utils.pychatbot/views/api_views.pychatbot/views/profile_views.pyobservability/migrations/0009_alter_companybottcrun_provider_and_more.pyshikshalokam/views/profile_views.py
💤 Files with no reviewable changes (1)
- chatbot/views/profile_views.py
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
KUNALTEMPEST
force-pushed
the
feature/ums
branch
from
June 23, 2026 12:34
428fe4f to
c246c9f
Compare
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@chatbot/consumers/async_consumer.py`:
- Around line 60-77: The sync_elevate_profile method is being called
unconditionally at line 62 regardless of whether self.access_token exists, which
causes non-Elevate flows without a token to fail with authentication errors.
Wrap the entire sync_elevate_profile call and its associated error handling
blocks (lines 62-75) in a conditional check that only executes when
self.access_token is truthy, allowing non-Elevate sessions without tokens to
proceed past the authentication step and reach the get_profile call on line 77.
In `@chatbot/models/profile_models.py`:
- Around line 62-77: The Profile model's Meta class has data-integrity gaps that
allow duplicate `userid` values and race-condition vulnerabilities in the
email/company_id uniqueness check. In the Meta class of the Profile model,
replace the current indexes list with a combination of a regular index on fields
email and phone, plus two UniqueConstraint definitions: one for userid with a
condition that userid is not null to prevent duplicates while allowing null
values, and another for the email and company_id combination with a condition
that userid is null and email is not null to ensure this constraint only applies
to traditional flow profiles. This moves validation from the error-prone save()
method checks to database-level enforcement, while keeping the save() method
checks as pre-validation only.
In `@shikshalokam/views/profile_views.py`:
- Around line 26-31: The error handling in the Elevate profile check is using
500 (Internal Server Error) as the fallback status code when status_code is not
provided, but 500 incorrectly represents a local server issue. For upstream
dependency failures like Elevate service outages, use 502 (Bad Gateway) as the
appropriate fallback to correctly indicate an external service failure. In the
Response return statement within the 'elevate_server_error' condition, change
the fallback value in the status parameter from 500 to 502 so that
profile_details.get('status_code') or 502 is used instead.
- Around line 17-43: The function calls handle_elevate_profile() without first
validating local preconditions, which causes unnecessary upstream calls and
potential data persistence when those preconditions aren't met. Move the
DEFAULT_COMPANY_SLUG environment variable check that currently happens after the
handle_elevate_profile() call (around line 39) to execute BEFORE the
handle_elevate_profile() call at line 17. Additionally, add a validation check
for the access_token parameter itself before calling handle_elevate_profile() to
ensure the token exists and is valid before making the upstream call. This
ensures all local preconditions are validated first, preventing wasted Elevate
service calls and Profile object updates when the request is missing required
configuration or credentials.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 23073713-d330-4f66-be6e-7c10dac8231d
📒 Files selected for processing (14)
chatbot/celery_tasks/flow_tasks.pychatbot/consumers/async_base_consumer.pychatbot/consumers/async_consumer.pychatbot/migrations/0086_alter_profile_unique_together_and_more.pychatbot/models/profile_models.pychatbot/services/core/base_service.pychatbot/services/core/orchestrator.pychatbot/services/core/prompt_builder.pychatbot/services/response_handlers/common_handler.pychatbot/utils/elevate/profile_utils.pychatbot/views/api_views.pychatbot/views/profile_views.pyobservability/migrations/0009_alter_companybottcrun_provider_and_more.pyshikshalokam/views/profile_views.py
💤 Files with no reviewable changes (1)
- chatbot/views/profile_views.py
🚧 Files skipped from review as they are similar to previous changes (6)
- chatbot/services/core/prompt_builder.py
- chatbot/services/core/base_service.py
- chatbot/views/api_views.py
- chatbot/celery_tasks/flow_tasks.py
- chatbot/services/core/orchestrator.py
- chatbot/services/response_handlers/common_handler.py
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
chatbot/admin/profile_admin.py (1)
42-49: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid
list_filteron the high-cardinalityuserid; usesearch_fieldsinstead.
useridis a free-textCharField(max_length=500)with one (near-)unique value per row. As alist_filterit renders Django'sAllValuesFieldListFilter, which runs aSELECT DISTINCT useridover the wholeProfiletable on every list-page render and produces an unusable dropdown containing every userid. Filtering by an identifier like this belongs insearch_fields.♻️ Proposed change
list_filter = ( CustomAdvanceDateFilter, 'email', - 'userid', 'phone', ProfileCompanyFilter, 'profile_type' ) actions = ['export_selected'] inlines = [ProfileAddressInline, ProfileMediaInline] - search_fields = ['first_name', 'email', 'phone'] + search_fields = ['first_name', 'email', 'phone', 'userid']🤖 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 `@chatbot/admin/profile_admin.py` around lines 42 - 49, Remove `userid` from the list_filter tuple in the ProfileAdmin class (the tuple starting at line 42 and containing CustomAdvanceDateFilter, email, userid, phone, ProfileCompanyFilter, and profile_type) and instead add userid to a search_fields attribute in the same ProfileAdmin class. This will allow filtering by userid through search functionality rather than an inefficient dropdown filter that queries all distinct userid values.chatbot/migrations/0087_remove_profile_chatbot_pro_userid_3d404e_idx_and_more.py (1)
17-24: 🩺 Stability & Availability | 🔵 TrivialVerify no pre-existing duplicate
userid/(company_id) rows before applying these constraints.
AddConstraint(UniqueConstraint(...))creates partial unique indexes. If production data already contains two rows with the same non-nulluserid(this column was previously non-unique and written viaupdate_or_create), the migration will fail mid-deploy with anIntegrityError. Same risk for duplicate(email, company_id)whereuserid IS NULL.Run a pre-check (and de-dupe if needed) before deploying:
SELECT userid, COUNT(*) FROM chatbot_profile WHERE userid IS NOT NULL GROUP BY userid HAVING COUNT(*) > 1; SELECT email, company_id, COUNT(*) FROM chatbot_profile WHERE userid IS NULL AND email IS NOT NULL GROUP BY email, company_id HAVING COUNT(*) > 1;🤖 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 `@chatbot/migrations/0087_remove_profile_chatbot_pro_userid_3d404e_idx_and_more.py` around lines 17 - 24, Before applying the UniqueConstraint migrations for the profile model, add a RunPython data migration step that validates there are no duplicate userid values (where userid IS NOT NULL) or duplicate (email, company_id) pairs (where userid IS NULL and email IS NOT NULL) in existing data. Use the provided SQL queries to detect duplicates and either raise an error with clear instructions for manual remediation or implement deduplication logic that keeps the most recent or highest priority record. This prevents the AddConstraint operations on the userid and email-company_id fields from failing with IntegrityError during deployment.
🤖 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.
Nitpick comments:
In `@chatbot/admin/profile_admin.py`:
- Around line 42-49: Remove `userid` from the list_filter tuple in the
ProfileAdmin class (the tuple starting at line 42 and containing
CustomAdvanceDateFilter, email, userid, phone, ProfileCompanyFilter, and
profile_type) and instead add userid to a search_fields attribute in the same
ProfileAdmin class. This will allow filtering by userid through search
functionality rather than an inefficient dropdown filter that queries all
distinct userid values.
In
`@chatbot/migrations/0087_remove_profile_chatbot_pro_userid_3d404e_idx_and_more.py`:
- Around line 17-24: Before applying the UniqueConstraint migrations for the
profile model, add a RunPython data migration step that validates there are no
duplicate userid values (where userid IS NOT NULL) or duplicate (email,
company_id) pairs (where userid IS NULL and email IS NOT NULL) in existing data.
Use the provided SQL queries to detect duplicates and either raise an error with
clear instructions for manual remediation or implement deduplication logic that
keeps the most recent or highest priority record. This prevents the
AddConstraint operations on the userid and email-company_id fields from failing
with IntegrityError during deployment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 181be378-e252-4ed5-97bc-5e44562d9355
📒 Files selected for processing (5)
chatbot/admin/profile_admin.pychatbot/consumers/async_consumer.pychatbot/migrations/0087_remove_profile_chatbot_pro_userid_3d404e_idx_and_more.pychatbot/models/profile_models.pyshikshalokam/views/profile_views.py
💤 Files with no reviewable changes (1)
- chatbot/consumers/async_consumer.py
🚧 Files skipped from review as they are similar to previous changes (1)
- shikshalokam/views/profile_views.py
✅ Action performedReview finished.
|
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
KUNALTEMPEST
changed the title
Switch from full profile DB storage to a minimal model where only userid is persisted for Elevate users. Non-PII profile data (designation, org, district, state) is fetched on every WS authenticate and threaded in-memory through the Celery task chain as ums_profile — never written to DB. Key changes: - Profile model: userid max_length 500, email/company nullable, unique_together removed, save() guard enforces UMS vs traditional flow rules, userid index added - handle_elevate_profile: lookup by userid, writes only userid + source to DB, returns ums_profile dict with session-scoped data - WS authenticate: calls Elevate after JWT decode, closes connection on auth/server errors, stores self.ums_profile - ums_profile threaded: flow_tasks → orchestrator → build_system_prompt (Jinja2 context includes ums_profile, skips ProfileAddress query) - _save_submitted_user_context: removed writes to first_name, designation, org_associated, ProfileAddress; keeps only is_onboarding_completed flag and update_elevate_profile call - get_profile_view: slimmed to id, is_tnc_accepted, is_profile_complete - CompanyBot lookup guards added for null company in async_consumer, async_base_consumer, base_service # Conflicts: # shikshalokam/views/profile_views.py
- Send auth error messages directly via self.send() instead of channel_layer to ensure delivery before connection closes - Guard base consumer disconnect() against double-close ASGI error - Fail loudly in read_elevate_profile if DEFAULT_COMPANY_SLUG is not set
KUNALTEMPEST
force-pushed
the
feature/ums
branch
from
June 29, 2026 04:07
c1333c9 to
d64d1d1
Compare
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@chatbot/consumers/async_consumer.py`:
- Line 293: The translate_message logging in async_consumer is leaking user chat
content because it logs the full transliteration response at INFO level. Update
the logger.info call in the transliteration path to record only non-sensitive
status or metadata (for example, success/failure, response type, or keys/length)
and avoid including the response payload itself. Keep the change scoped to the
translate_message flow so the log remains useful without exposing translated
message contents.
- Around line 61-76: The authentication flow in AsyncConsumer currently only
closes on explicit Elevate error values, but it can still proceed when
sync_elevate_profile() returns no bound profile. Update the authenticate path in
AsyncConsumer to treat a missing profileid from sync_elevate_profile() as a
failure: only assign self.profile_id and call get_profile() when elevate_result
contains a valid profileid, otherwise log and send the same auth failure
response, then close the socket. Use the existing sync_elevate_profile(),
self.profile_id, and get_profile() branch as the location to enforce this
fail-closed behavior.
- Around line 207-209: The sync_elevate_profile path in
chatbot/consumers/async_consumer.py is wrapping both the blocking Elevate HTTP
fetch and the ORM write in database_sync_to_async, which can tie up the DB
thread pool during auth bursts. Refactor handle_elevate_profile usage so the
requests.get call happens in a separate sync_to_async(thread_sensitive=False)
helper (or an async HTTP client), and keep only the update_or_create/database
write portion inside the database_sync_to_async wrapper in sync_elevate_profile.
In `@chatbot/models/profile_models.py`:
- Around line 77-87: The partial unique constraints in Profile.Meta still treat
blank strings as valid values, so `userid=''` can collide with the
`uniq_profile_userid` constraint and similar blank `email` values may affect
`uniq_profile_email_company`. Update the constraint conditions in `Profile` to
exclude empty strings as well as NULLs, or normalize blanks to `None` in
`Profile.save()` before persistence; use the existing `models.UniqueConstraint`
definitions and `Profile.save()` logic as the places to fix.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 84cafe3f-107b-47fb-ba95-a60c7a649d3c
📒 Files selected for processing (18)
chatbot/admin/profile_admin.pychatbot/celery_tasks/flow_tasks.pychatbot/consumers/async_base_consumer.pychatbot/consumers/async_consumer.pychatbot/migrations/0086_alter_profile_unique_together_and_more.pychatbot/migrations/0087_remove_profile_chatbot_pro_userid_3d404e_idx_and_more.pychatbot/migrations/0088_remove_botvernacular_bot_vernacu_company_483975_idx_and_more.pychatbot/models/bot_vernacular_model.pychatbot/models/profile_models.pychatbot/services/core/base_service.pychatbot/services/core/orchestrator.pychatbot/services/core/prompt_builder.pychatbot/services/response_handlers/common_handler.pychatbot/utils/elevate/profile_utils.pychatbot/views/api_views.pychatbot/views/profile_views.pyobservability/migrations/0009_alter_companybottcrun_provider_and_more.pyshikshalokam/views/profile_views.py
💤 Files with no reviewable changes (1)
- chatbot/views/profile_views.py
🚧 Files skipped from review as they are similar to previous changes (9)
- chatbot/services/core/prompt_builder.py
- chatbot/services/core/base_service.py
- chatbot/admin/profile_admin.py
- chatbot/celery_tasks/flow_tasks.py
- chatbot/models/bot_vernacular_model.py
- chatbot/views/api_views.py
- chatbot/services/core/orchestrator.py
- chatbot/services/response_handlers/common_handler.py
- shikshalokam/views/profile_views.py
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
✅ Action performedReview finished.
|
kiranharidas187
merged commit Jun 29, 2026
217e2fe
into
ELEVATE-Project:release-1.0.0
1 check passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Other Changes