Drop 10 work item across 1851 and 1873 story - #19
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR adds company chat feedback storage and APIs, profile update handling, citation metadata, a chat export script, language choice updates, and Gotenberg deployment documentation. ChangesCompany chat feedback
Profile integration
Citation metadata
Chat export tooling
Platform setup updates
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant CompanyChatFeedbackCreateView
participant CompanyChatFeedbackSerializer
participant CompanyChatFeedback
participant CompanyChatSerializer
Client->>CompanyChatFeedbackCreateView: POST company chat feedback
CompanyChatFeedbackCreateView->>CompanyChatFeedbackSerializer: validate submission
CompanyChatFeedbackSerializer->>CompanyChatFeedback: create feedback history entry
CompanyChatFeedback-->>CompanyChatSerializer: provide latest reaction
CompanyChatSerializer-->>Client: return chat reaction fields
Possibly related PRs
🚥 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 |
|
@coderabbitai review, (you can ignore docs folder) |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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/admin/company_admin.py`:
- Around line 335-337: Update CompanyChatAdmin.get_queryset to apply the
moderator company restriction directly to the base queryset, following the
existing filtering pattern in BotAdmin and ChatSessionAdmin. Preserve the
sender__company and receiver__company prefetches while ensuring moderators
cannot see non-company CompanyChat rows in the default result.
In `@chatbot/serializer/profile_serializer.py`:
- Around line 96-100: Update CompanyChatSerializer._latest_feedback in
chatbot/serializer/profile_serializer.py:96-100 and the related feedback
prefetches in chatbot/views/drf_views.py:20-21 and
chatbot/views/drf_views.py:27-28 so each chat loads only its latest feedback
row, or supplies a precomputed latest-feedback value; preserve thumbs_up and
thumbs_down selection while avoiding full feedback-history materialization.
- Around line 129-138: Move the omitted-reaction-field fallback out of
CompanyChatFeedbackSerializer.validate and into the feedback creation path,
using an atomic transaction that locks the latest CompanyChatFeedback for the
company_chat before copying thumbs_up/thumbs_down and creating the new row.
Preserve explicit reaction values and ensure comment-only submissions cannot
overwrite newer reaction state.
In `@chatbot/views/api_views.py`:
- Around line 358-359: Update the profile update flow around
update_elevate_profile so personal profile values are not emitted in INFO logs;
replace full request/response logging in the called profile utility with field
names and status codes only, or use the project’s approved redaction helper,
while preserving the update behavior.
- Around line 358-359: Update update_profile_view to construct the
profile-update payload only from fields requested by the frontend, including
about only when explicitly provided; ensure update_elevate_profile does not
inject its placeholder about value. Preserve existing supported-field filtering
and add a mocked request test verifying a name-only update sends no about key.
- Around line 358-359: Update the access-token flow in the surrounding view to
check whether _get_access_token(request) returned None before calling
update_elevate_profile. Match the existing guard and 401 Unauthorized response
used by update_profile_view, ensuring requests without either token source
return immediately and never invoke update_elevate_profile with a missing token.
- Around line 346-350: Update the update_fields construction in the profile
update handler to include only values that are strings containing non-whitespace
characters, excluding False, 0, empty strings, and whitespace-only strings.
Ensure the response’s updated-field list is derived from this validated set so
rejected values are not reported as updated.
- Around line 381-386: Update the exception handler around the affected API view
to use logger.exception(...) for structured server-side diagnostics instead of
traceback.print_exc(), while retaining the existing logger context. Return a
generic 500 error message without exposing str(e) to clients, and address Ruff’s
blind except Exception warning using the project’s established
exception-handling pattern.
In `@chatbot/views/drf_views.py`:
- Around line 32-35: Restrict feedback creation in CompanyChatFeedbackCreateView
so callers cannot submit an arbitrary company_chat. Either scope the writable
company_chat field in CompanyChatFeedbackSerializer to chats authorized for the
caller, or make it read-only and resolve the authorized chat in perform_create
before saving.
🪄 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 Plus
Run ID: a68e9cdb-6765-485e-9e5a-bfa12b05cd17
📒 Files selected for processing (8)
chatbot/admin/company_admin.pychatbot/migrations/0089_alter_chatsession_language_alter_story_language_and_more.pychatbot/models/company_models.pychatbot/serializer/profile_serializer.pychatbot/urls.pychatbot/views/api_views.pychatbot/views/drf_views.pydocs/setup/gotenberg_server_setup.md
|
@coderabbitai review, please resolve the comments if they are fixed now |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="ELEVATE-Project/saathi-backend"
pr="19"
echo "== Pull request =="
gh pr view "$pr" --repo "$repo" --json number,state,headRefOid,baseRefOid,changedFiles,commits \
--jq '{number,state,headRefOid,baseRefOid,changedFiles,commitCount:(.commits|length)}'
echo "== Changed files excluding docs =="
gh pr diff "$pr" --repo "$repo" --name-only | awk '$0 !~ /^docs\//'
echo "== CodeRabbit review threads =="
gh api graphql \
-F owner="ELEVATE-Project" \
-F name="saathi-backend" \
-F number="$pr" \
-f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes {
id
isResolved
path
line
originalLine
comments(first: 20) {
nodes {
author { login }
body
createdAt
url
}
}
}
}
}
}
}' \
--jq '.data.repository.pullRequest.reviewThreads.nodes[]
| select(any(.comments.nodes[]; .author.login == "coderabbitai"))
| {id,isResolved,path,line,originalLine,comments:[.comments.nodes[] | select(.author.login == "coderabbitai") | {body,url,createdAt}]}'Length of output: 35884 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Current symbol map =="
ast-grep outline chatbot/admin/company_admin.py --match CompanyChatAdmin --view expanded
ast-grep outline chatbot/serializer/profile_serializer.py --match 'CompanyChatSerializer|CompanyChatFeedbackSerializer' --view expanded
ast-grep outline chatbot/views/api_views.py --match update_profile_view --view expanded
ast-grep outline chatbot/views/drf_views.py --match 'CompanyChatListCreateView|CompanyChatRetrieveUpdateDestroyView|CompanyChatFeedbackCreateView' --view expanded
ast-grep outline chatbot/utils/elevate/profile_utils.py --match update_elevate_profile --view expanded
echo "== Relevant current source =="
rg -n -C 12 --type py \
'class CompanyChatAdmin|def get_queryset|class CompanyChatSerializer|_latest_feedback|class CompanyChatFeedbackSerializer|def validate|def create' \
chatbot/admin/company_admin.py chatbot/serializer/profile_serializer.py
rg -n -C 12 --type py \
'def update_profile_view|update_fields|update_elevate_profile|logger\.|traceback|except Exception' \
chatbot/views/api_views.py chatbot/utils/elevate/profile_utils.py
rg -n -C 10 --type py \
'class CompanyChatListCreateView|class CompanyChatRetrieveUpdateDestroyView|class CompanyChatFeedbackCreateView|prefetch_related|Prefetch' \
chatbot/views/drf_views.py
echo "== Diff after base commit, excluding docs =="
git diff --no-ext-diff --unified=16 1bde8dbd3fd0898b7e8911e117156b728c2a6fbf..HEAD -- \
chatbot/admin/company_admin.py \
chatbot/serializer/profile_serializer.py \
chatbot/views/api_views.py \
chatbot/utils/elevate/profile_utils.py \
chatbot/views/drf_views.pyLength of output: 89941 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="ELEVATE-Project/saathi-backend"
pr="19"
thread_ids=(
"PRRT_kwDOSaCc8s6WLuC4"
"PRRT_kwDOSaCc8s6WLuC8"
"PRRT_kwDOSaCc8s6WLuC_"
"PRRT_kwDOSaCc8s6WLuDC"
"PRRT_kwDOSaCc8s6WLuDJ"
)
for thread_id in "${thread_ids[@]}"; do
gh api graphql \
-F threadId="$thread_id" \
-f query='
mutation($threadId: ID!) {
resolveReviewThread(input: {threadId: $threadId}) {
thread { id isResolved }
}
}' \
--jq '.data.resolveReviewThread.thread | {id,isResolved}'
done
echo "== Remaining unresolved CodeRabbit threads =="
gh api graphql \
-F owner="ELEVATE-Project" \
-F name="saathi-backend" \
-F number="$pr" \
-f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes {
id
isResolved
path
comments(first: 20) { nodes { author { login } url } }
}
}
}
}
}' \
--jq '.data.repository.pullRequest.reviewThreads.nodes[]
| select(.isResolved == false)
| select(any(.comments.nodes[]; .author.login == "coderabbitai"))
| {id,path,commentUrls:[.comments.nodes[] | select(.author.login == "coderabbitai") | .url]}'Length of output: 1949
One thread remains open in You are interacting with an AI system. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
chatbot/scripts/export_chats_by_userid.py (1)
131-142: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid evaluating each chat QuerySet twice.
chats.exists()performs a query without populating the QuerySet cache. The loop then performs the full query again. Materialize the result once and test the resulting list.Proposed change
- chats = CompanyChat.objects.filter( + chats = list(CompanyChat.objects.filter( Q(sender_id=profile.id) | Q(receiver_id=profile.id) - ).order_by('session', 'created_at') + ).order_by('session', 'created_at')) - if not chats.exists(): + if not chats:🤖 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/scripts/export_chats_by_userid.py` around lines 131 - 142, Materialize the QuerySet assigned to chats once before the empty-check, then test that materialized collection for emptiness and iterate over the same results. Update the chats handling in the profile-processing flow while preserving the existing no-chats statistics, output row, and populated-chat behavior.
🤖 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/scripts/export_chats_by_userid.py`:
- Around line 46-49: Update _s3_to_https so S3 values resolve through the
configured bucket endpoint, including the configured S3_BUCKET_NAME/domain and
region, by reusing the existing download URL resolver. If that resolver is
unavailable, preserve the raw s3:// URI instead of constructing an invalid
generic HTTPS URL.
In `@chatbot/utils/elevate/profile_utils.py`:
- Around line 176-185: Update update_profile_view to include about in the
request-field allowlist and validation message, while preserving the existing
fields and forwarding behavior through update_elevate_profile.
In `@chatbot/views/drf_views.py`:
- Around line 24-30: Ensure latest feedback selection is deterministic by
ordering the latest_feedback query in chatbot/views/drf_views.py lines 24-30 by
-created_at then -pk. Apply the same ordering to the fallback query in
chatbot/serializer/profile_serializer.py lines 103-104 and the reaction-state
carry-forward query in lines 153-155, preserving each query’s existing behavior.
---
Nitpick comments:
In `@chatbot/scripts/export_chats_by_userid.py`:
- Around line 131-142: Materialize the QuerySet assigned to chats once before
the empty-check, then test that materialized collection for emptiness and
iterate over the same results. Update the chats handling in the
profile-processing flow while preserving the existing no-chats statistics,
output row, and populated-chat behavior.
🪄 Autofix
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 Plus
Run ID: 8f3f52db-e035-45c3-acb2-4d50acf03660
📒 Files selected for processing (10)
chatbot/admin/company_admin.pychatbot/scripts/export_chats_by_userid.pychatbot/serializer/profile_serializer.pychatbot/services/response_handlers/base_response_handler.pychatbot/services/vector/vector_service.pychatbot/utils/elevate/profile_utils.pychatbot/views/api_views.pychatbot/views/drf_views.pydocs/setup/gotenberg_server_setup.mdshikshalokam/views/profile_views.py
🚧 Files skipped from review as they are similar to previous changes (3)
- chatbot/views/api_views.py
- chatbot/admin/company_admin.py
- docs/setup/gotenberg_server_setup.md
…known fields, duplicate JSON keys, and no-op thumbs+comment submissions with 400)
|
@coderabbitai reivew, review only lastes changes pushed |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
chatbot/views/drf_views.py (1)
50-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse an immutable parser configuration.
parser_classesis a mutable class attribute. A later mutation can affect all requests handled by this view. Ruff also reports RUF012 here.Use a tuple after confirming the project’s Django REST framework version accepts tuple-valued parser configurations.
Proposed fix
- parser_classes = [StrictJSONParser] + parser_classes = (StrictJSONParser,)🤖 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/views/drf_views.py` at line 50, Update the view’s parser_classes declaration to an immutable tuple containing StrictJSONParser, after confirming the project’s Django REST framework version supports tuple-valued parser configurations.Source: Linters/SAST tools
chatbot/parsers.py (1)
31-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winChain the decoder error explicitly.
The raise in
except ValueError as exclacks an explicit cause, which triggers Ruff B904. Usefrom excso theJSON parse errorpreserves the underlying decoder cause.Proposed fix
- raise ParseError('JSON parse error - %s' % str(exc)) + raise ParseError('JSON parse error - %s' % str(exc)) from exc🤖 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/parsers.py` at line 31, Update the ParseError raise in the ValueError exception handler to explicitly chain the caught decoder exception with from exc, preserving the existing JSON parse error message.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/parsers.py`:
- Line 2: Update the JSON import and the parser’s json.load call to use
rest_framework.utils.json, passing parse_constant=json.strict_constant when
self.strict is true and None otherwise. Preserve the existing duplicate-key
validation and parsing behavior.
---
Nitpick comments:
In `@chatbot/parsers.py`:
- Line 31: Update the ParseError raise in the ValueError exception handler to
explicitly chain the caught decoder exception with from exc, preserving the
existing JSON parse error message.
In `@chatbot/views/drf_views.py`:
- Line 50: Update the view’s parser_classes declaration to an immutable tuple
containing StrictJSONParser, after confirming the project’s Django REST
framework version supports tuple-valued parser configurations.
🪄 Autofix
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 Plus
Run ID: f0c55165-975b-4f76-8b06-6839dfc741a0
📒 Files selected for processing (3)
chatbot/parsers.pychatbot/serializer/profile_serializer.pychatbot/views/drf_views.py
🚧 Files skipped from review as they are similar to previous changes (1)
- chatbot/serializer/profile_serializer.py
Summary by CodeRabbit
New Features
Administration
Documentation