feat(mattermost): seed prior thread posts into first-turn context - #64270
feat(mattermost): seed prior thread posts into first-turn context#64270wernerhp wants to merge 7 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds first-turn Mattermost thread-history seeding so when Hermes is mentioned in an existing Mattermost thread for the first time, prior thread posts are fetched (GET /api/v4/posts/{root_id}/thread) and injected via MessageEvent.channel_context to provide the LLM with conversation context.
Changes:
- Add Mattermost-side thread context fetching, formatting, allowlist-aware
[unverified]tagging, and TTL caching, wired into the WS receive path for first-turn-only seeding. - Expose a config/ENV toggle (
mattermost.thread_context↔MATTERMOST_THREAD_CONTEXT) and document the behavior. - Add Mattermost gateway tests covering formatting, exclusions, caching, first-turn-only behavior, and toggle disabling.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/platforms/mattermost/adapter.py | Fetch and inject prior thread posts into channel_context, with caching and authorization tagging, plus YAML→env bridging. |
| tests/gateway/test_mattermost.py | Adds tests for thread-context seeding behavior and configuration bridging. |
| website/docs/user-guide/messaging/mattermost.md | Documents the new thread context behavior and its configuration knob. |
| hermes_cli/config.py | Registers MATTERMOST_THREAD_CONTEXT as a user-facing optional messaging setting. |
| cli-config.yaml.example | Adds an example mattermost.thread_context config block. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Fixed in YAML 1.1 parses |
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Comment (prior COMMENT exists)
This PR seeds prior thread posts into first-turn context for Mattermost (5 files, 584 additions).
Observations
- Has prior COMMENT review.
- Thread history seeding is valuable for context quality.
Suggestion
Consider storage implications for large threads; ensure truncation/sampling.
Reviewed by Hermes Agent
teknium1
left a comment
There was a problem hiding this comment.
Thanks for carrying the Mattermost thread-context path through channel_context; current main still lacks this behavior (plugins/platforms/mattermost/adapter.py:867-960), so the feature remains useful. The YAML boolean follow-up in 1c46fae addresses the earlier review concern.
Problems
plugins/platforms/mattermost/adapter.py:274fetchesposts/{root_id}/threadwithout applyinglimit; only after materializing and sorting the whole response does line 286 slice the tail. Line 296 also passes each complete post body into the prompt. This does not provide the bounded fetch or per-post cap described in the discussion.plugins/platforms/mattermost/adapter.py:1452addsMATTERMOST_STRICT_MENTION, but current main has no Mattermost runtime reader for that variable. Please keep this unrelated dead bridge out of the change.
Suggested changes
- Bound retrieval before materializing posts, cap per-post and total rendered context, and test both limits.
- Remove the strict-mention bridge unless it is completed as a separate feature.
Automated hermes-sweeper review.
a10f0b5 to
7603dee
Compare
47b141c to
2525f04
Compare
When Hermes is drawn into an existing Mattermost thread for the first
time (a reply carrying root_id, with no session yet), it had no context
of the conversation it just joined: the receive path never called
GET /api/v4/posts/{root_id}/thread, so the LLM saw only the triggering
message. Slack already has this via _fetch_thread_context(); Mattermost
was the gap tracked by NousResearch#37695.
This adds first-turn thread-history seeding for the Mattermost adapter:
- _fetch_thread_context() pulls prior thread posts and formats them as
[username]: message, attached through MessageEvent.channel_context so
the gateway keeps per-author attribution and frames the trigger as
[New message] (rather than prepending into text, which would let the
triggering sender's prefix swallow the whole block).
- _has_active_session_for_thread() guards seeding to the first turn only,
using build_session_key() as the single source of truth so isolation
settings are honoured; once a session exists the history is already
carried and is not refetched.
- Senders not on the allowlist are tagged [unverified] via the base
_is_sender_authorized() hook so their content is treated as background
reference, not authoritative input, instead of being dropped.
- Command posts and the bot's own prior replies are excluded; a short
per-root TTL cache avoids refetching on bursts.
- Policy via config.yaml mattermost.thread_context (on/off), bridged to
MATTERMOST_THREAD_CONTEXT through the existing _apply_yaml_config hook.
Fixes NousResearch#37695
Copilot review (PR NousResearch#64270) caught that YAML 1.1 parses a bare `thread_context: off` as Python bool False, so `str(False).lower()` became 'false' — which the literal-'off' check did not treat as disabled, silently keeping the feature on when a user tried to turn it off via off/false/0/no. Add _normalize_onoff() to coerce the whole falsy family (off/false/0/ no/none/empty, plus Python bool False) to canonical 'on'/'off', and use it in both the adapter constructor and the _apply_yaml_config bridge. Tests cover the YAML-boolean cases for the helper, the config bridge, and the adapter constructor.
…ead strict_mention bridge Addresses review on NousResearch#64270: - Bound retrieval at the request (perPage + direction=up) so a long thread is never fully materialized before slicing; keep the defensive in-memory tail slice as a fallback. - Add explicit prompt-input caps independent of MAX_POST_LENGTH (which governs outbound sends only): per-post truncation (MAX_THREAD_CONTEXT_POST_CHARS) and a total-context budget (MAX_THREAD_CONTEXT_TOTAL_CHARS) that trims oldest posts first. - Remove the unrelated MATTERMOST_STRICT_MENTION YAML->env bridge; no Mattermost runtime reader exists for it on main (Slack-only feature). - Tests: request-bound assertion, per-post cap, total cap keeping newest, and strict_mention non-bridging. 91 passed.
d8e0008 to
9f95811
Compare
What does this PR do?
When Hermes is drawn into an existing Mattermost thread for the first time (a reply carrying
root_id, in a thread it has no session for yet), it had no context of the conversation it just joined. The receive path inplugins/platforms/mattermost/adapter.pyreadroot_idfor session keying and reply routing only; it never calledGET /api/v4/posts/{root_id}/thread, so the LLM saw just the triggering message.Slack already solves this with
_fetch_thread_context(); Mattermost was the remaining gap tracked by #37695. This adds first-turn thread-history seeding for the Mattermost adapter, mirroring the Slack approach and reusing existing infrastructure.Design notes:
MessageEvent.channel_context, not prepended into the message text. The gateway (gateway/run.py) applies the triggering sender's prefix totextonly, then prependschannel_contextahead of a[New message]marker. Routing history throughchannel_contextkeeps each prior post's own[username]attribution instead of letting the trigger sender's prefix swallow the whole block._has_active_session_for_thread(), which usesbuild_session_key()as the single source of truth so thegroup_sessions_per_user/thread_sessions_per_userisolation settings are honoured. Once a session exists the thread history is already carried in it, so no refetch and no duplication.[unverified](via the base_is_sender_authorized()hook) so the model treats their content as background reference rather than authoritative input. This preserves context without dropping it, and the header switches to a security-aware variant that tells the model not to act on unverified content. Without this, injectedchannel_contextwould bypass the triggering-author authorization check.main, so a leading-space/newis correctly classified as a command and never receives injected context.config.yamlmattermost.thread_context(ondefault /off), bridged toMATTERMOST_THREAD_CONTEXTthrough the existing_apply_yaml_confighook — no new bespoke.env-only setting.Related Issue
Fixes #37695
Type of Change
Changes Made
plugins/platforms/mattermost/adapter.py: add_fetch_thread_context(),_has_active_session_for_thread(), and_resolve_thread_user_name(); wire first-turn seeding into_handle_ws_event()viaMessageEvent.channel_context; add per-root TTL cache and user-name cache; bridgethread_contextin_apply_yaml_config().hermes_cli/config.py: registerMATTERMOST_THREAD_CONTEXTas a user-facing optional messaging setting.website/docs/user-guide/messaging/mattermost.md: document the thread-context behaviour and thethread_context/MATTERMOST_THREAD_CONTEXTknob.cli-config.yaml.example: add amattermost:block includingthread_context.tests/gateway/test_mattermost.py: 10 tests covering formatting/attribution, bot-and-command exclusion,[unverified]tagging, empty-root and cache behaviour, first-turn seeding via_handle_ws_event, no-seed on root posts,offdisabling, and the YAML->env bridge.How to Test
website/docs/user-guide/messaging/mattermost.md). In a channel, have two users exchange a few posts in a thread without mentioning the bot.@mentionthe bot in a reply to that thread. The bot's first response reflects awareness of the prior posts; posts from non-allowlisted users appear as background tagged[unverified].pytest tests/gateway/test_mattermost.py -q— 69 passed.git stash push -- plugins/platforms/mattermost/adapter.pythen rerun the thread-context tests — 7 behaviour-pinning tests fail (the remaining negative-guard tests trivially hold with the feature absent);git stash poprestores green.Checklist
Code
feat(scope):)pytest tests/gateway/test_mattermost.py -qand all tests pass (69 passed)Documentation & Housekeeping
docs/, docstrings)cli-config.yaml.examplefor the new config keyCONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/A