Repository navigation
feat(chat): feedback APIs — rate a bot reply, answer the end-of-chat card - #1417
Dev-Bhumika03 wants to merge 2 commits into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
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 |
a09d1c9 to
edab068
Compare
Adds the storage and config for customer feedback in the widget, with no
behaviour yet (the APIs land in the next PR):
- 085: nullable feedback jsonb on chat_message (per-reply thumbs) and
chat_session (end-of-chat card). Metadata-only ALTERs.
- template configurations.feedback: message_feedback.enabled and
chat_feedback {enabled, title, pills (2-6, unique ids), allow_comment,
end_on {context rule, idle_seconds, host_event}}. Both off by default.
- widget surface block gains feedback; a disabled end card never ships.
- ChatMessage / ChatSession decode the new column (missing column = None).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…card
Two widget routes (POST, widget token, per-IP limit, CORS preflight):
- POST /widget/session/{id}/messages/{idx}/feedback {rating: up|down|null}
Sets or clears the rating on one bot reply (chat_message.feedback).
404 when idx is not a bot reply in the session.
- POST /widget/session/{id}/feedback {pills, comment, trigger}
Stores the end-card answer (chat_session.feedback). Pills must be ids
from the template; empty answers and unknown ids are 422.
Both are 403 when the template has that feedback off, and both work on an
ENDED session. The resume payload carries the card answer so the embed
does not show the card twice; per-reply ratings already ride on each
message. Chat analytics summary gains thumbs_up, thumbs_down and
chat_feedback_count.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
edab068 to
cd53aff
Compare
raghvendradhakar
left a comment
raghvendradhakar
left a comment
There was a problem hiding this comment.
Review at cd53aff2, top commit only. The first commit is an older copy of #1416.
- Rebase: #1416 now rejects an enabled end card with no end trigger. See the inline comment on the test fixture.
- Rating race on quick taps: it is best fixed in the SDK, juspay/loom#400. The server has no request order to compare, so this PR needs no change for it.
| return FeedbackConfig.model_validate( | ||
| { | ||
| "message_feedback": {"enabled": True}, | ||
| "chat_feedback": {"enabled": True}, |
There was a problem hiding this comment.
| class ChatFeedbackRequest(BaseModel): | ||
| """Body of ``POST /widget/session/{id}/feedback`` (the end-of-chat card).""" | ||
|
|
||
| pills: List[str] = Field( |
There was a problem hiding this comment.
The list is capped at 6, but each id has no length or format limit. A long string passes the schema, fails the allowed-id check, and the 422 detail echoes the whole string back. Use the same constraint as FeedbackPill.id, ^[a-z0-9_]{1,40}$, on each item.
| # --------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| async def _feedback_session( |
There was a problem hiding this comment.
Both callers discard the session this returns, so it can return only the feedback config. The later row is None 404 in set_chat_feedback_handler can only happen if the row is deleted between two statements, so it can go too.
| monkeypatch.setattr(handlers, "resolve_session_widget_config", config) | ||
| monkeypatch.setattr(handlers, "enforce_widget_ip_limit", ip_limit) | ||
| monkeypatch.setattr(handlers, "get_chat_session_by_id", session) | ||
| monkeypatch.setattr(handlers, "assert_widget_session_ownership", lambda *_: None) |
There was a problem hiding this comment.
This stub and the enforce_widget_ip_limit stub mean no test checks that the routes call them. Removing either guard still passes.
Please add:
- a test where ownership fails and the route returns 404
- a test that the IP bucket name is
feedback - a test that the resume payload returns
session.feedback
test_summary_exposes_feedback_counts only checks alias strings, so a broken FILTER would also pass.
What
Two widget routes for customer feedback. Both use POST, the widget token, the per-IP limit (
feedbackbucket) and a CORS preflight.POST /widget/session/{id}/messages/{idx}/feedbackwith{"rating": "up" | "down" | null}Sets or clears the rating on one bot reply (
chat_message.feedback = {rating, at}). Returns 404 whenidxis not a bot reply in the session.POST /widget/session/{id}/feedbackwith{"pills": [...], "comment": "...", "trigger": "context" | "idle" | "new_chat"}Stores the end-card answer (
chat_session.feedback). Pills must be ids from the template. Unknown ids or an empty answer return 422. A resend replaces the answer.Both return 403 when the template has that feedback off, and both work on an ENDED session (the card is shown after the chat ends).
Also:
feedback), so the embed doesn't show the card twice. Per-reply ratings already ride on each message.thumbs_up,thumbs_downandchat_feedback_count.POST rather than PUT because the widget CORS preflight allows GET/POST only.
Tests
tests/test_chat_feedback_api.py(18): storing, clearing, the off/404/422 paths, ended sessions, the SQL guards, routes mounted with preflight, and the analytics aliases. pyrefly 0 errors; 617 chat/widget/analytics tests pass.🤖 Generated with Claude Code