Terms and condition now comes from elevate - #15
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughTerms and conditions state now flows through Elevate profile APIs and access-token-based views. Profile responses use boolean defaults, and knowledge-base results include explicit provenance wrappers. ChangesElevate TNC State Propagation
Knowledge-Base Result Provenance
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant apiViews as api_views
participant profileUtils as profile_utils
participant Elevate
Client->>apiViews: accept_tnc_view request
apiViews->>apiViews: _get_access_token()
alt token missing
apiViews-->>Client: 400 error response
else token present
apiViews->>profileUtils: update_elevate_profile(has_accepted_terms_and_conditions=True)
profileUtils->>Elevate: update profile request
Elevate-->>profileUtils: response
profileUtils-->>apiViews: result
apiViews-->>Client: is_tnc_accepted True
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
@CodeRabbit review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
chatbot/views/api_views.py (1)
13-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting
_get_access_tokento a shared utility.The same access-token retrieval logic (cookie fallback to
X-auth-tokenheader) is duplicated inline inshikshalokam/views/profile_views.py:13-15. Extracting_get_access_tokento a shared module would eliminate the duplication and ensure consistent token-retrieval behavior across both view files.🤖 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/api_views.py` around lines 13 - 28, The access-token lookup logic in _get_access_token is duplicated in multiple view modules, so move this helper into a shared utility and have both api_views and profile_views call the same function. Keep the existing behavior of checking the ACCESS_TOKEN_COOKIE_KEY cookie first and then falling back to the X-auth-token header so token retrieval stays consistent across the codebase.
🤖 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/views/api_views.py`:
- Around line 295-303: The accept_tnc_view flow is treating Elevate updates as
successful even when update_elevate_profile fails. Update accept_tnc_view to
inspect the return value from update_elevate_profile and only return the success
response when the Elevate profile update actually succeeds; if it returns an
empty result or otherwise indicates failure, respond with an error instead. Use
the existing update_elevate_profile and accept_tnc_view symbols to locate the
change, and consider making update_elevate_profile surface failures more
explicitly so callers can distinguish real success from a swallowed exception.
---
Nitpick comments:
In `@chatbot/views/api_views.py`:
- Around line 13-28: The access-token lookup logic in _get_access_token is
duplicated in multiple view modules, so move this helper into a shared utility
and have both api_views and profile_views call the same function. Keep the
existing behavior of checking the ACCESS_TOKEN_COOKIE_KEY cookie first and then
falling back to the X-auth-token header so token retrieval stays consistent
across the codebase.
🪄 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: 34ed2adb-dab3-4a22-b05d-8bdbec661e5e
📒 Files selected for processing (3)
chatbot/utils/elevate/profile_utils.pychatbot/views/api_views.pyshikshalokam/views/profile_views.py
…"repository|none"> provenance markers before they enter the LLM transcript.
|
@coderabbitai review please check new pushed changes |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
chatbot/services/response_handlers/base_response_handler.py (1)
540-542: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider escaping
textto prevent premature wrapper closure.If retrieved chunk text contains the literal
</retrieved_content>, it would prematurely close the provenance wrapper in the LLM transcript. While this is only a prompt hint (not parsed as structured data), a malformed wrapper could confuse the LLM about source boundaries. This is low-risk given current data sources but worth noting for future robustness.🛡️ Optional defensive escaping
def _wrap_retrieved_content(self, text, source): """Wrap tool-retrieved text in an explicit provenance marker before it enters the transcript.""" + # Prevent chunk text from prematurely closing the wrapper + safe_text = text.replace('</retrieved_content>', '</retrieved_content_>') return f'<retrieved_content source="{source}">\n{text}\n</retrieved_content>'🤖 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/base_response_handler.py` around lines 540 - 542, Update _wrap_retrieved_content to defensively escape or neutralize any literal </retrieved_content> sequence in text before interpolating it into the provenance wrapper, while preserving the retrieved content as much as possible. Ensure the generated opening and closing markers remain unambiguous in the LLM transcript.
🤖 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/services/response_handlers/base_response_handler.py`:
- Around line 529-533: Update the fallback text passed to
_wrap_retrieved_content in the source='none' branch so it does not encourage an
immediate general-knowledge response before web search is attempted. Instruct
the LLM to use web search if available, and only otherwise respond from general
knowledge when appropriate, while preserving the no-hallucination guidance.
---
Nitpick comments:
In `@chatbot/services/response_handlers/base_response_handler.py`:
- Around line 540-542: Update _wrap_retrieved_content to defensively escape or
neutralize any literal </retrieved_content> sequence in text before
interpolating it into the provenance wrapper, while preserving the retrieved
content as much as possible. Ensure the generated opening and closing markers
remain unambiguous in the LLM transcript.
🪄 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: 3f96e82b-e784-4ea9-b8f8-a0a8e28da024
📒 Files selected for processing (2)
chatbot/services/response_handlers/base_response_handler.pychatbot/views/api_views.py
🚧 Files skipped from review as they are similar to previous changes (1)
- chatbot/views/api_views.py
|
✅ Action performedReview finished.
|
|
@coderabbitai review please check new pushed changes and resolve any comments that are left opened |
|
✅ Action performedComments resolved. Approval is disabled; enable Review finished.
|
c5f0b58
into
ELEVATE-Project:release-1.1.0
Summary by CodeRabbit
X-auth-tokenrequest header.falsewhen Elevate data is unavailable (instead of an incorrect placeholder).