Skip to content

feat(core): tri-state is_mention, mention regex, Author.email/is_system, Message.reply_to (#192) - #274

Merged
patrick-chinchill merged 3 commits into
mainfrom
sync/4.41-c2a
Oct 1, 2026
Merged

patrick-chinchill merged 3 commits into
mainfrom
sync/4.41-c2a

Conversation

@patrick-chinchill

@patrick-chinchill patrick-chinchill commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Ports four core changes from the 4.41 wave:

  • Tri-state is_mention. Chat._set_mention_flags runs text detection only when is_mention is None (upstream ??), for the dispatched message and each context.skipped. An adapter's False is now definitive. The DM override stays as a routing rule (with a comment saying so).
  • Stricter mention regex. _detect_mention matches (?<![A-Za-z0-9_])@name(?![A-Za-z0-9_-]) (IGNORECASE). Emails, URL userinfo, @bot-dev and @UBOT123-canary no longer mention the bot. The Discord <@!?id> pattern is unchanged.
  • Author.email / Author.is_system (default None, last fields). Serialized as email / isSystem only when not None.
  • Message.reply_to (last field; also on MessageData / SentMessage). Serialized as replyTo. It survives to_json/from_json/from_json_compat, the Chat reviver (bottom-up object_hook), set_message_adapter, queue rehydration (rehydrate_attachment recursion), both history caches (raw nulled along the chain) and create_sent_message_from_message.
  • Discord forwarded messages now report True if is_mentioned else None (upstream isMentioned || undefined). This lands here so a literal @botname keeps routing through text detection.

Upstream commits mapped

Commit Upstream What landed here
2531a422 (#621, chat@4.34.0) detectMention (?![\w-]) core trailing guard (Telegram half was #225)
0701679e (#706, chat@4.35.0) Telegram regex cache + abortable sleep no core change; abortable sleep N/A (stop_polling cancels the task), documented
46681f50 (#711, chat@4.35.0) Author.email? core field + serialization (Teams hydration → #218)
80def3ab (#707, chat@4.35.0) Author.isSystem? core field + serialization (Slack USLACK → #209)
b547f458 (#761, chat@4.36.0) (?<!\w)@name(?![\w-]) leading guard
0f24cc30 (#802, chat@4.38.0) Message.replyTo core plumbing (Telegram population → #228)
fcdc1c9e (#946, chat@4.41.0) isMention ?? detectMention(...) _set_mention_flags + Discord True | None (Linear → #232)

Verify-first results

  • Teams coupling: confirmed. grep -rn 'parse_teams_webhook_body\|TeamsMessagePayload' src/ finds nothing outside adapters/teams/webhook/, and the adapter path only ever sets True. teams/webhook/parse.py is unchanged (parity with upstream isTeamsMention(): boolean). A regression test proves the text fallback.
  • Other adapters: grep -rn is_mention src/chat_sdk/adapters/ shows that only Discord built a definitive False where upstream leaves it undefined. Telegram passes a bool, as upstream. Linear and Messenger pass True. Slack and Teams only set True.

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's replyTo assertions (attachment rehydrated, subject resolved 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 with replyTo.raw.
  • thread.test.ts: "should wrap a Message as a SentMessage with same fields" now asserts sent.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".
  • Discord: mirrors the isMention: isMentioned || undefined expectation of "keeps allowlisted forwarded messages in their Discord thread": an unmentioned message gives None and a mentioned one gives True. An end-to-end test runs a real Chat with the real Discord adapter, and a literal @mybot routes to on_mention.
  • Python-specific: Teams channel text-fallback routing through _handle_message_activity into a real Chat. ASCII guard sweep: é@, Kelvin sign, long s, ä after the name, x@, _@, _ after the name, uppercase. Bottom-up object_hook revival through both revivers. from_json_compat with snake_case reply_to / is_system / email and an already-revived nested Message. chat._ThreadHistoryCache strips raw along a 3-deep chain and round-trips it. A False is_system is emitted and preserved. set_message_adapter recursion. _to_message keeps reply_to.

Mutation checks: reverting _set_mention_flags to or fails the 2 definitive-non-mention tests. Reverting Discord to is_mention=is_mentioned fails 2 of the 3 Discord tests. Dropping the (?-i:…) scoping fails the Kelvin and long-s rows.

Fidelity

Delta vs committed report (HEAD): missing 207 -> 198 (-9)
  packages/chat/src/chat.test.ts: 15 -> 8 (-7)
  packages/chat/src/message.test.ts: 13 -> 11 (-2)

(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 (i without u) 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 in docs/UPSTREAM_SYNC.md ("Mentions and message model (chat@4.34–4.41, #192)").

Consumer impact (low)

  • Custom adapters that return False for "not detected" must return None, or text detection no longer runs for them.
  • Emails, URL userinfo and @name-suffix no longer trigger on_mention.
  • Serialized messages gain the optional keys replyTo, author.email and author.isSystem. New fields: Author.email, Author.is_system, Message.reply_to. All are appended last, so positional construction is unchanged.
  • No routing change for Slack or Teams (the Teams text fallback is kept). Discord's literal @botname fallback keeps working.

Validation

ruff check / format, audit_test_quality.py (0 hard failures), --check-docs, --strict 733/733, pytest 6622 passed / 24 skipped, pyrefly 0 errors (on final HEAD 313982d).

Merge gate

Review findings (2 independent reviewers, 4 findings): all 4 fixed, none declined.

  • Stale non-parity row (docs/UPSTREAM_SYNC.md, comment in chat.py _rehydrate_message): confirmed. In upstream chat@4.41.1, packages/chat/src/chat.ts:3451-3455 assigns msg = raw for a Message, then falls through to rehydrateAttachment. 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.
  • No test for email / isSystem in _message_from_json (Chat reviver, chat._ThreadHistoryCache): fixed. test_reply_to_survives_bottom_up_object_hook_revival now gives both messages authors with email / is_system, including False. test_chat_history_cache_strips_raw_along_reply_chain checks those fields after get_messages. Mutation M16 now fails both tests.
  • No test for replyTo / reply_to in the _rehydrate_message dict fallback: fixed. The new test_plain_dict_fallback_reads_reply_to_and_author_fields is parametrized over camelCase and snake_case keys. It calls _rehydrate_message on a plain dict and checks that reply_to is a Message and that its attachment went through rehydrate_attachment. Mutation M4 now fails both cases.
  • No test for email / isSystem / is_system in the dict-fallback author: fixed by the same test. It covers isSystem: True and is_system: False, and checks that False stays False. 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, and fidelity_target.json is 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

…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
@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You'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.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: f9ff3aed-a5a4-498c-bd0e-f3776d3d0d17

📥 Commits

Reviewing files that changed from the base of the PR and between 2aaef13 and 313982d.

📒 Files selected for processing (15)
  • CHANGELOG.md
  • docs/UPSTREAM_SYNC.md
  • scripts/fidelity_target.json
  • src/chat_sdk/adapters/discord/adapter.py
  • src/chat_sdk/chat.py
  • src/chat_sdk/thread.py
  • src/chat_sdk/thread_history.py
  • src/chat_sdk/types.py
  • tests/test_chat_faithful.py
  • tests/test_discord_extended.py
  • tests/test_serialization.py
  • tests/test_teams_extended.py
  • tests/test_thread_faithful.py
  • tests/test_thread_history.py
  • tests/test_types.py
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@patrick-chinchill
patrick-chinchill marked this pull request as ready for review October 1, 2026 04:57
@patrick-chinchill

Copy link
Copy Markdown
Collaborator Author

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[4.41/C2a] Core mentions & message model: tri-state is_mention, mention regex, Author.email/is_system, Message.reply_to

1 participant