feat(feishu): implement thread history fetching for topic sessions#238
feat(feishu): implement thread history fetching for topic sessions#238badgerbees wants to merge 3 commits intonextlevelbuilder:mainfrom
Conversation
|
hmm CI error isnt from my changes |
viettranx
left a comment
There was a problem hiding this comment.
Review PR #238 — feat(feishu): implement thread history fetching for topic sessions
Tổng quan
PR giải quyết đúng vấn đề "mất ngữ cảnh" khi bot được mention giữa chừng trong thread Feishu/Lark. Hướng tiếp cận hợp lý — fetch 20 tin nhắn gần nhất từ API để có ngữ cảnh đầy đủ. Thread history từ API đã bao gồm các pending messages nên việc ưu tiên nó thay vì groupHistory.BuildContext() là đúng.
Cần fix trước khi merge
1. BUG — json.Unmarshal lỗi bị bỏ qua (Nghiêm trọng)
File: larkclient_messaging.go — GetMessage() và ListMessages()
json.Unmarshal(resp.Data, &data) // lỗi bị bỏ quaNếu Feishu API thay đổi format response, hàm trả về rỗng mà không báo lỗi — rất khó debug production. Nên check và return error.
2. BUG — Sender hiển thị raw open_id thay vì tên (Trung bình)
File: bot_parse.go — buildThreadHistoryContent()
sender := item.Sender.ID // "ou_xxxxx" — vô nghĩa với AIUser thường sẽ hiện dạng ou_xxxxx trong thread history. AI không hiểu đây là ai. Nên resolve tên user (dùng resolveSenderName có sẵn trong Channel), hoặc ít nhất hiển thị "User" thay vì raw ID.
3. THIẾU — Lỗi fetch history bị nuốt, không log (Nhỏ)
File: bot.go
if history, err := c.fetchFeishuThreadHistory(ctx, mc.RootID); err == nil && history != "" {
threadHistory = history
}Khi err != nil, lỗi hoàn toàn bị nuốt. Nên thêm slog.Warn để dễ debug production.
4. THIẾU — Mention placeholders không resolve trong thread history (Trung bình)
File: bot_parse.go — buildThreadHistoryContent()
parseMessageContent không xử lý mention placeholder (@_user_1, @_user_2). Thread history sẽ chứa raw placeholder thay vì tên người dùng.
Nên fix (không blocking)
5. PERFORMANCE — 2 API calls mỗi lần mention, không cache
Mỗi lần mention: GetMessage(rootID) + ListMessages(). Nên cache rootID → threadID mapping vì thread ID không thay đổi.
6. SECURITY — URL params không escape
File: larkclient_messaging.go — ListMessages()
containerIDType và containerID nối trực tiếp vào URL. Nên dùng url.QueryEscape().
Tóm tắt
| # | Vấn đề | Mức độ | Blocking? |
|---|---|---|---|
| 1 | json.Unmarshal lỗi bị bỏ qua |
Nghiêm trọng | ✅ |
| 2 | Sender hiện raw ID thay vì tên | Trung bình | ✅ |
| 3 | Lỗi fetch bị nuốt, không log | Nhỏ | ✅ |
| 4 | Mention placeholders không resolve | Trung bình | ✅ |
| 5 | Không cache threadID | Nhỏ | ❌ |
| 6 | URL params không escape | Nhỏ | ❌ |
Tổng thể PR tốt, giải quyết đúng pain point. Fix 4 items blocking rồi merge được 👍
viettranx
left a comment
There was a problem hiding this comment.
Review PR #238 — feat(feishu): implement thread history fetching for topic sessions
Overview
Good approach — fetching the last 20 messages from Feishu API solves the "memory gap" when bot is mentioned mid-thread. Thread history from API already includes pending messages, so prioritizing it over groupHistory.BuildContext() is correct.
Blocking (must fix before merge)
1. BUG — json.Unmarshal errors silently ignored (Critical)
File: larkclient_messaging.go — GetMessage() and ListMessages()
json.Unmarshal(resp.Data, &data) // error ignoredIf Feishu API changes response format, functions return empty results with no error — very hard to debug in production. Should check and return error.
2. BUG — Sender displays raw open_id instead of name (Medium)
File: bot_parse.go — buildThreadHistoryContent()
sender := item.Sender.ID // "ou_xxxxx" — meaningless to AIRegular users show as ou_xxxxx in thread history. AI can't understand who this is. Should resolve user name (using existing resolveSenderName in Channel), or at minimum display "User" instead of raw ID.
3. MISSING — Failed history fetch silently swallowed (Low)
File: bot.go
if history, err := c.fetchFeishuThreadHistory(ctx, mc.RootID); err == nil && history != "" {
threadHistory = history
}When err != nil, error is completely swallowed. Add slog.Warn for production debugging.
4. MISSING — Mention placeholders not resolved in thread history (Medium)
File: bot_parse.go — buildThreadHistoryContent()
parseMessageContent doesn't handle mention placeholders (@_user_1, @_user_2). Thread history will contain raw placeholders instead of user names.
Nice-to-have (non-blocking)
5. PERFORMANCE — 2 API calls per mention, no caching
Each mention triggers: GetMessage(rootID) + ListMessages(). Consider caching rootID → threadID mapping since thread ID never changes.
6. SECURITY — URL params not escaped
File: larkclient_messaging.go — ListMessages()
containerIDType and containerID concatenated directly into URL. Should use url.QueryEscape().
Summary
| # | Issue | Severity | Blocking? |
|---|---|---|---|
| 1 | json.Unmarshal errors ignored |
Critical | ✅ |
| 2 | Sender shows raw ID instead of name | Medium | ✅ |
| 3 | Failed fetch silently swallowed | Low | ✅ |
| 4 | Mention placeholders not resolved | Medium | ✅ |
| 5 | No threadID caching | Low | ❌ |
| 6 | URL params not escaped | Low | ❌ |
Overall solid PR, solves a real pain point. Fix the 4 blocking items and it's good to merge 👍
|
@viettranx Thanks for the thorough review! I’ve gone ahead and addressed all 4 blocking issues and decided to implement the suggested performance and security improvements as well to ensure the PR is fully robust. Summary of Changes:
Ready for another look! |
c4ecebb to
08b20fa
Compare
08b20fa to
0f6cbfd
Compare
This update fixes the "memory gap" in Feishu/Lark group discussions. When the bot is mentioned in a topic thread, it now proactively fetches the recent conversation history of that thread and passes it to the AI as context.
The Problem:
Previously, if a user started a thread and mentioned the bot later, or if the bot had restarted, it would "forget" everything said in that specific thread. This made multi-turn conversations in Feishu feel broken or "dumb" because the AI couldn't see its own previous replies or the users' earlier messages.
Key Improvements:
root_idandTopicSessionModeis active, the bot now calls the Feishu API (container_id_type=thread) to retrieve the last 20 messages.