fix(monitoring): WeChat Work feedback recording bugs#2108
fix(monitoring): WeChat Work feedback recording bugs#21086mvp6 wants to merge 2 commits intolangbot-app:masterfrom
Conversation
- Fix feedback events silently dropped when stream session expires: dispatch feedback handlers regardless of session availability - Fix IntegrityError on repeated feedback (like→dislike) for same message: implement UPSERT logic in record_feedback() - Fix cancel feedback (type=3) not removing records: add delete logic - Fix inaccurate_reasons validation error: convert int reason codes to strings before creating FeedbackEvent (Pydantic expects List[str]) - Fix feedback timestamps 8 hours off in frontend: use parseUTCTimestamp instead of new Date() for UTC timestamp parsing - Fix StreamSessionManager.cleanup missing _feedback_index cleanup Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
dosubot
bot
added
size:L
There was a problem hiding this comment.
Pull request overview
Fixes multiple issues in the WeChat Work (WeCom) bot feedback pipeline so feedback isn’t dropped after stream session expiry, supports updating existing feedback records, supports cancel feedback events, normalizes negative feedback reason types, and corrects frontend timestamp parsing.
Changes:
- Backend: always dispatch feedback events even when the stream session has expired; clean up
StreamSessionManager._feedback_indexduring session cleanup. - Backend: implement update/insert behavior for repeated feedback IDs and delete-on-cancel (
type=3) in monitoring persistence. - Platform + Web: normalize
inaccurate_reasonsto strings and parse backend timestamps as UTC on the monitoring UI.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/langbot/libs/wecom_ai_bot_api/api.py |
Dispatch feedback handlers without requiring an active stream session; cleanup feedback index on session expiry. |
src/langbot/pkg/api/http/service/monitoring.py |
Add cancel-delete and update/insert logic for MonitoringFeedback records. |
src/langbot/pkg/platform/sources/wecombot.py |
Convert WeCom integer reason codes to strings before constructing FeedbackEvent. |
web/src/app/home/monitoring/hooks/useFeedbackData.ts |
Parse feedback timestamps as UTC via shared parseUTCTimestamp() helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| existing_row = existing_result.first() | ||
|
|
||
| if existing_row: | ||
| # UPDATE existing record | ||
| existing = existing_row[0] if isinstance(existing_row, tuple) else existing_row |
There was a problem hiding this comment.
existing_result.first() returns a SQLAlchemy Row (not a tuple), so existing = existing_row[0] if isinstance(existing_row, tuple) else existing_row will set existing to a Row. Accesses like existing.bot_id / existing.id will then raise AttributeError, breaking feedback updates. Prefer existing = existing_row[0] here (since the query is select(MonitoringFeedback)) or use existing = existing_result.scalars().first() and then read fields from the model instance.
| existing_row = existing_result.first() | |
| if existing_row: | |
| # UPDATE existing record | |
| existing = existing_row[0] if isinstance(existing_row, tuple) else existing_row | |
| existing = existing_result.scalars().first() | |
| if existing: | |
| # UPDATE existing record |
| # Handle cancel feedback (type=3): delete existing record | ||
| if feedback_type == 3: | ||
| await self.ap.persistence_mgr.execute_async( | ||
| sqlalchemy.delete(MonitoringFeedback).where( | ||
| MonitoringFeedback.feedback_id == feedback_id | ||
| ) | ||
| ) | ||
| return None | ||
|
|
There was a problem hiding this comment.
record_feedback is annotated as returning str, but the cancel path (feedback_type == 3) returns None. Please update the return type annotation/docstring (e.g., str | None) and ensure callers (if any) handle the None return value consistently.
| try: | ||
| await self.ap.persistence_mgr.execute_async( | ||
| sqlalchemy.insert(MonitoringFeedback).values(record_data) | ||
| ) | ||
| return record_id | ||
| except Exception: | ||
| # UNIQUE constraint conflict (concurrent feedback for same feedback_id) | ||
| await self.ap.persistence_mgr.execute_async( | ||
| sqlalchemy.update(MonitoringFeedback) | ||
| .where(MonitoringFeedback.feedback_id == feedback_id) | ||
| .values( | ||
| timestamp=now, | ||
| feedback_type=feedback_type, | ||
| feedback_content=feedback_content, | ||
| inaccurate_reasons=reasons_json, | ||
| ) | ||
| ) | ||
| return feedback_id |
There was a problem hiding this comment.
The insert conflict handling uses except Exception, which will also swallow non-unique DB failures (connection issues, SQL syntax, etc.) and incorrectly treat them as a UNIQUE conflict. Catch sqlalchemy.exc.IntegrityError specifically (and ideally verify it’s the feedback_id unique constraint) and re-raise/log other exceptions. Also, the fallback currently return feedback_id, which is not the record’s primary key id (and differs from the documented return value). Consider re-selecting the row to return existing.id, or use a real UPSERT that can return the row id.
| # Dispatch feedback event regardless of session availability | ||
| for handler in self._message_handlers.get('feedback', []): | ||
| try: | ||
| await handler( | ||
| feedback_id=feedback_id, | ||
| feedback_type=feedback_type, | ||
| feedback_content=feedback_content, | ||
| inaccurate_reasons=inaccurate_reasons, | ||
| session=session, | ||
| ) |
There was a problem hiding this comment.
Feedback handlers are now dispatched even when no session is found (passing session=None). This is a behavior change vs. the previous implementation and can break handlers/callbacks that assume session fields are always present. Consider documenting this explicitly (or adjusting handler contracts) so downstream handlers know session may be None and should guard accordingly.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
概要
修复 #2078 引入的企业微信智能机器人反馈功能的 3 个 Bug，以及测试中发现的 2 个额外问题：
Bug 1 & 2 — 反馈事件在会话过期后被静默丢弃：
StreamSessionManager的 TTL 为 60 秒，而_handle_feedback_event()仅在session存在时才分发处理函数。用户通常在阅读完回复后才给出反馈（往往超过 60 秒），此时会话已过期，反馈被静默丢弃。此外，对同一消息更改反馈（点赞→点踩）时，由于feedback_id的 UNIQUE 约束，原始代码只执行 INSERT 而不做 UPSERT，导致IntegrityError。Bug 3 — 取消反馈（type=3）不生效：缺少处理取消反馈事件的逻辑，反馈记录从未被删除。
Bug 4 —
inaccurate_reasons校验错误：企业微信 API 返回整数原因代码（如[2, 4]），但FeedbackEvent.inaccurate_reasons期望List[str]类型。Pydantic v2 抛出ValidationError，导致用户选择负反馈原因时整个反馈记录失败。Bug 5 — 前端时间戳偏差 8 小时：
useFeedbackData.ts使用new Date(item.timestamp)将服务端的 UTC 时间字符串当作本地时间解析。改用parseUTCTimestamp()，与其它监控 hooks 保持一致。修改内容
src/langbot/libs/wecom_ai_bot_api/api.pycleanup()遗漏的_feedback_index清理src/langbot/pkg/api/http/service/monitoring.pyrecord_feedback()：UPSERT 逻辑 + 取消（type=3）删除 + IntegrityError 防御src/langbot/pkg/platform/sources/wecombot.pyFeedbackEvent前将整数原因代码转换为字符串web/src/app/home/monitoring/hooks/useFeedbackData.tsparseUTCTimestamp()正确解析 UTC 时间戳测试计划
["1","3"]）