Repository navigation
feat(core): tri-state is_mention, mention regex, Author.email/is_system, Message.reply_to (#192) - #274
Conversation
…l/is_system, Message.reply_to (#192) Ports the core halves of vercel/chat 2531a422 (#621), 46681f50 (#711), 80def3ab (#707), b547f458 (#761), 0f24cc30 (#802) and fcdc1c9e (#946). - _set_mention_flags: text detection only when is_mention is None; an adapter False is definitive. DM override kept as a routing rule. - Discord forwarded messages report True|None (isMentioned || undefined). - _detect_mention: (?<!\w)@name(?![\w-]) with ASCII classes spelled out and case-sensitive scoped guards (JS \w without u is ASCII-only). - Author.email / Author.is_system; Message.reply_to (also MessageData, SentMessage), serialized as email / isSystem / replyTo; recursion in from_json(_compat), the Chat reviver, set_message_adapter and _rehydrate_message; both history caches null raw along the chain. Part of #184
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 59 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (15)
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 |
…viver/history/dict-fallback reads of email, is_system, reply_to
|
Merge gate: CI green (Lint & Type Check, test (3.12), test (3.13), CodeQL, Analyze (python), Analyze (actions)); local Codex review (gpt-6-astra, xhigh, --base origin/main) on 313982d: "No actionable regressions found against the specified merge base. Validation passed: 6,622 tests, Ruff checks, formatting, and type checking."; 2 astra rounds (both clean); origin/main (2aaef13) already an ancestor of HEAD, so no re-merge needed; CodeRabbit rate-limited (no review posted), no other bot findings. Merging with --admin (Protect Main requires a code-owner approval). |
Summary
Ports four core changes from the 4.41 wave:
is_mention.Chat._set_mention_flagsruns text detection only whenis_mention is None(upstream??), for the dispatched message and eachcontext.skipped. An adapter'sFalseis now definitive. The DM override stays as a routing rule (with a comment saying so)._detect_mentionmatches(?<![A-Za-z0-9_])@name(?![A-Za-z0-9_-])(IGNORECASE). Emails, URL userinfo,@bot-devand@UBOT123-canaryno longer mention the bot. The Discord<@!?id>pattern is unchanged.Author.email/Author.is_system(defaultNone, last fields). Serialized asemail/isSystemonly when notNone.Message.reply_to(last field; also onMessageData/SentMessage). Serialized asreplyTo. It survivesto_json/from_json/from_json_compat, the Chat reviver (bottom-upobject_hook),set_message_adapter, queue rehydration (rehydrate_attachmentrecursion), both history caches (rawnulled along the chain) andcreate_sent_message_from_message.True if is_mentioned else None(upstreamisMentioned || undefined). This lands here so a literal@botnamekeeps routing through text detection.Upstream commits mapped
2531a422(#621, chat@4.34.0)detectMention(?![\w-])0701679e(#706, chat@4.35.0)stop_pollingcancels the task), documented46681f50(#711, chat@4.35.0)Author.email?80def3ab(#707, chat@4.35.0)Author.isSystem?USLACK→ #209)b547f458(#761, chat@4.36.0)(?<!\w)@name(?![\w-])0f24cc30(#802, chat@4.38.0)Message.replyTofcdc1c9e(#946, chat@4.41.0)isMention ?? detectMention(...)_set_mention_flags+ DiscordTrue | None(Linear → #232)Verify-first results
grep -rn 'parse_teams_webhook_body\|TeamsMessagePayload' src/finds nothing outsideadapters/teams/webhook/, and the adapter path only ever setsTrue.teams/webhook/parse.pyis unchanged (parity with upstreamisTeamsMention(): boolean). A regression test proves the text fallback.grep -rn is_mention src/chat_sdk/adapters/shows that only Discord built a definitiveFalsewhere upstream leaves it undefined. Telegram passes abool, as upstream. Linear and Messenger passTrue. Slack and Teams only setTrue.Tests ported
chat.test.ts(strict): "should keep a definitive non-mention reported by the adapter", "should keep a definitive mention reported by the adapter", "should keep a definitive non-mention on skipped queued messages", the two hyphen-suffix tests,it.each"should not treat %s as a mention (%s)" (4 rows) and "should still detect %s as a mention (%s)" (5 rows). "should call rehydrateAttachment on deserialized attachments missing fetchData" is extended with upstream'sreplyToassertions (attachment rehydrated,subjectresolved through the adapter with the reply's raw).serialization.test.ts: "should round-trip replied-to message context".thread-history.test.ts: renamed to "should strip raw fields on storage" and extended withreplyTo.raw.thread.test.ts: "should wrap a Message as a SentMessage with same fields" now assertssent.reply_to.message.test.ts(target tier →tests/test_types.py): "should preserve author.isSystem through a full JSON roundtrip", "should leave author.isSystem absent for non-system authors", "should preserve author email through serialization".isMention: isMentioned || undefinedexpectation of "keeps allowlisted forwarded messages in their Discord thread": an unmentioned message givesNoneand a mentioned one givesTrue. An end-to-end test runs a realChatwith the real Discord adapter, and a literal@mybotroutes toon_mention._handle_message_activityinto a realChat. ASCII guard sweep:é@, Kelvin sign, long s,äafter the name,x@,_@,_after the name, uppercase. Bottom-upobject_hookrevival through both revivers.from_json_compatwith snake_casereply_to/is_system/emailand an already-revived nestedMessage.chat._ThreadHistoryCachestripsrawalong a 3-deep chain and round-trips it. AFalseis_systemis emitted and preserved.set_message_adapterrecursion._to_messagekeepsreply_to.Mutation checks: reverting
_set_mention_flagstoorfails the 2 definitive-non-mention tests. Reverting Discord tois_mention=is_mentionedfails 2 of the 3 Discord tests. Dropping the(?-i:…)scoping fails the Kelvin and long-s rows.Fidelity
(The serialization and thread-history tests were already counted as matched by fuzzy names. The message.test.ts delta is −2 because one of the three was already fuzzy-matched.) Strict at the pin: 733/733.
Divergences
None from upstream behavior. One porting refinement goes beyond the issue's suggested pattern. The guards are wrapped in case-sensitive scoped groups (
(?-i:(?<![A-Za-z0-9_])),(?-i:(?![A-Za-z0-9_-]))), because Python's Unicode IGNORECASE otherwise makes[A-Za-z]match the Kelvin sign and long s. JS (iwithoutu) never folds those onto ASCII. This brings the guards to exact parity. The residual name-folding difference (@ſlack-bot) is the same one the Telegram row already documents. It is recorded indocs/UPSTREAM_SYNC.md("Mentions and message model (chat@4.34–4.41, #192)").Consumer impact (low)
Falsefor "not detected" must returnNone, or text detection no longer runs for them.@name-suffixno longer triggeron_mention.replyTo,author.emailandauthor.isSystem. New fields:Author.email,Author.is_system,Message.reply_to. All are appended last, so positional construction is unchanged.@botnamefallback keeps working.Validation
ruff check / format,
audit_test_quality.py(0 hard failures),--check-docs,--strict733/733, pytest 6622 passed / 24 skipped, pyrefly 0 errors (on final HEAD313982d).Merge gate
Review findings (2 independent reviewers, 4 findings): all 4 fixed, none declined.
_rehydrate_message): confirmed. In upstream chat@4.41.1,packages/chat/src/chat.ts:3451-3455assignsmsg = rawfor aMessage, then falls through torehydrateAttachment.0f24cc30(#802) removed the early return. Removed the row. Replaced the "Diverges from upstream" comment with a parity note. Added a line to the [4.41/C2a] Core mentions & message model: tri-state is_mention, mention regex, Author.email/is_system, Message.reply_to #192 section saying this divergence is closed.email/isSystemin_message_from_json(Chat reviver,chat._ThreadHistoryCache): fixed.test_reply_to_survives_bottom_up_object_hook_revivalnow gives both messages authors withemail/is_system, includingFalse.test_chat_history_cache_strips_raw_along_reply_chainchecks those fields afterget_messages. Mutation M16 now fails both tests.replyTo/reply_toin the_rehydrate_messagedict fallback: fixed. The newtest_plain_dict_fallback_reads_reply_to_and_author_fieldsis parametrized over camelCase and snake_case keys. It calls_rehydrate_messageon a plain dict and checks thatreply_tois aMessageand that its attachment went throughrehydrate_attachment. Mutation M4 now fails both cases.email/isSystem/is_systemin the dict-fallback author: fixed by the same test. It coversisSystem: Trueandis_system: False, and checks thatFalsestaysFalse. Mutation M15 now fails both cases.Fidelity after these tests:
Delta vs committed report (HEAD): missing 198 -> 198 (+0). They add one Python-only test, andfidelity_target.jsonis regenerated.gpt-6-astra: 1 round, on
313982d. Verdict: PASS ("No actionable regressions found against the specified merge base").Bots: nothing actionable. CodeRabbit skipped the PR while it was a draft and was rate-limited after it was marked ready. There are no inline comments or reviews.
CI on
313982d: all green: Lint & Type Check (including strict fidelity and pyrefly), test 3.12, test 3.13, CodeQL.Closes #192
Part of #184