Skip to content

perf(desktop): route a group round to the members it concerns - #98610

Closed
andredezzy wants to merge 1 commit into
NousResearch:mainfrom
andredezzy:perf/group-routing-by-relevance
Closed

andredezzy wants to merge 1 commit into
NousResearch:mainfrom
andredezzy:perf/group-routing-by-relevance

Conversation

@andredezzy

@andredezzy andredezzy commented Aug 30, 2026 •

Copy link
Copy Markdown

The cost

A group room with no @-mention wakes every member. Each specialist then pays a full model call to conclude the question is not its domain and answer (pass).

The domain gate itself is right — a health bot should not opine on a Notion schema. But it fires after the API bill, not before. Measured across a live five-bot room:

122 no-op calls · 490 seconds · 8% of all model time

Spent producing the word "(pass)".

Mentions are not the fix. Addressing a room by name on every message is exactly the ceremony a group chat exists to avoid, and the moment a user stops, the whole roster wakes again.

The change

A round with no mention now selects members whose domain vocabulary appears in the round's text. A question naming databases and blocks reaches the Notion specialist; one naming training and sleep reaches health.

Matching uses complete words, with explicit prefixes for inflected terms. Ambiguous Notion vocabulary requires two matches; notion alone is sufficient. A domain-neutral coordinator joins a matched round so it can redirect the task.

Why a wrong guess stays cheap

Relevance is a heuristic over words, so the design assumes it will sometimes miss:

  • The coordinator joins every routed round. It can hand the task to whoever actually owns it, so a miss costs a round rather than an answer.
  • A round matching no domain wakes everyone. Silence is a worse failure than an extra turn, so the heuristic never routes a question into the void.

@mentions and @everyone are untouched — an explicit address still selects exactly who was named.

Tests

Six cases covering both directions: mentions still win, a domain question routes to its specialist and away from the others, the coordinator survives a wrong guess, an unrecognisable question still reaches everyone, and routing reads the whole exchange since the last user message rather than one line.

apps/desktop/src/plugins/hermes-bots/   59 files   565 passed
tsc --noEmit                            clean

Verified on origin/main with no other local changes.

Scope note

The domain table lives next to the resolver as a plain constant. It could become configuration later, but shipping it as config first would be speculative — there is one consumer, and the right shape of that config is not yet known.

@alt-glitch alt-glitch added type/perf Performance improvement or optimization P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) area/usage-cost Token accounting, usage reporting, billing, cost tracking needs-decision Awaiting maintainer decision before any implementation labels Aug 30, 2026
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

Overall: Performance routing: group round now wakes only domain-relevant members instead of whole room, saving model calls.

Correctness:

  • apps/desktop/src/plugins/hermes-bots/group-rounds.ts:15-78 — GROUP_DOMAIN_TERMS narrow vocab (notion/finance/health ... with pt/en variants), matched against member name leading segment (notion-expert → notion). Deliberately narrow to avoid false filtering; unrecognized falls back to whole room (open wake whole room fallback in surrounding code not shown but implied by comment "fails to recognise falls back").
  • Comment explains trade-off: specialist (pass) still costs API bill vs unanswered question — routing cost analysis is sound.
  • parseGroupChatMentions unchanged for @-mention path.

Non-blocking nits:

  • Hardcoded term lists may need periodic update; consider config-driven or skill-derived terms. Current narrow set is safe (low miss-cost when fallback is whole room).
  • months in notion list seems broad — may trigger notion expert on finance "last months" — acceptable given fallback is whole room not miss, but worth watch.

Verdict: LGTM.

@andredezzy

Copy link
Copy Markdown
Author

A follow-up adversarial review found two routing failures in the first revision. Substring matching treated review as the Notion term view, and recovery depended on the exact profile name lifeos-coordinator.

Commit daa64020 now matches complete words, keeps intentional stems explicit, requires two matches for ambiguous Notion vocabulary unless notion itself appears, and recognises any domain-neutral coordinator without pulling specialist coordinators into unrelated rounds. It also removes the broad months term.

Verified: 10 routing tests passed, including the reproduced travel/review false positive and a non-LifeOS coordinator; ESLint and TypeScript passed.

@angel12

angel12 commented Sep 6, 2026

Copy link
Copy Markdown

Nice measurement — 8% of model time spent producing (pass) is a more concrete number than anything in the routing discussions so far.

I want to check a scope boundary before I write anything that would collide with this. In #95163 I've been scoping backend-hosted group rooms, and one of the open direction questions there (Q2) is where "who answers an unmentioned round" should live. Your PR answers that question client-side with domain-vocabulary matching. There's also #93003, which answers it client-side by falling back to a default owner.

Do you see vocabulary matching and designated-owner routing as one feature or two? My read is that they're different mechanisms that could compose — vocabulary matching narrows the roster, an owner catches what matches nothing — but they'd both be selecting in group-rounds.ts, so composing them accidentally is easy and composing them deliberately is better.

If you consider selection yours, I'll stay out of group-rounds.ts entirely and scope leader routing to the hosted driver in #95163. If you'd rather only own the perf half, I'm happy to take the fallback case and build on your selection point. Either is fine by me — I'd just rather know now than have us both discover it at rebase time.

One thing you may not have hit yet: #103298 (opened today) also rewrites the round loop, for reload durability. Worth a look before you rebase.

@andredezzy

Copy link
Copy Markdown
Author

I see vocabulary matching and designated-owner routing as separate policies that should share one responder-selection point. This PR owns the relevance filter. Please take the no-match fallback; I am not reserving group-rounds.ts.

My preferred order is explicit mentions / @everyone, then relevant members, then a configured owner when nothing matches, with the existing whole-room fallback for rooms without an owner. The owner should come from room configuration rather than a registry of personal room names. That is a proposed composition, not behavior this PR implements today.

I read #95163, #93003, and #103298. Hosted-room execution and reload recovery can reuse the selection contract while keeping their lifecycle state separate. #103298 remains open, so I will not pull its unmerged round-loop changes into this PR. The current revision uses whole-word matching and explicit prefixes; I corrected the stale substring description above.

A room drives every member on every round, so a six-bot room pays six model
calls for a question aimed at one specialist; the other five spend a full call
to answer "(pass)". Measured on my own rooms, that was 8% of model time.

resolveGroupResponders already narrows the round when the user @-mentions
someone. This extends the same seam to the unmentioned case: match each
member's domain vocabulary against the text since the last user message, and
drive only the members whose domain appears.

Two properties keep a wrong guess cheap rather than silent:

- No match wakes the whole room, so a question the table does not recognise is
  never routed into the void.
- The coordinator stays in every narrowed round, so a miss is handed to whoever
  owns the task and costs a round instead of an answer.

Stems are matched as whole words. Substring matching read "review" as the
Notion term "view"; intentional stems are listed explicitly instead.
@andredezzy
andredezzy force-pushed the perf/group-routing-by-relevance branch from daa6402 to b70ce40 Compare September 14, 2026 02:30
@andredezzy

Copy link
Copy Markdown
Author

Rebased onto current main (ee445299). The branch was 8123 commits behind, and the round driver was extracted into group-round-members.ts in the meantime, so this is a rewrite against the new structure rather than a replay.

What changed in the port: the relevance filter goes back inside resolveGroupResponders, which is still the one place that decides who speaks. The narrowing applies only when there is no @mention and no @everyone, so the existing paths are untouched.

Rechecked on b70ce40b:

  • 606 Bot Mode tests pass, 9 of them covering this filter.
  • Reverting the filter fails 2 of those 9, so they exercise the behaviour rather than the shape.
  • npm run typecheck clean; npm run lint reports the same 162 warnings / 0 errors as origin/main.

@angel12 — the boundary we agreed still holds: this PR owns the vocabulary filter, the no-match fallback is yours. The fallback here is deliberately the crudest thing that cannot lose a message (no match wakes the whole room, and the coordinator stays in every narrowed round), so replacing it should not require touching this code path.

CI has never run on this PR; all three workflows expired awaiting approval. Happy to rebase again whenever a maintainer has a window.

andredezzy pushed a commit to andredezzy/hermes-agent that referenced this pull request Sep 16, 2026
…d driver

The branch was 1444 commits behind. The conflicting hunks were the ones already
resolved on the fork today, so rerere replayed them: upstream's groupChatRoomKey
import sits beside getGroupChatLimits, and its failedMembers field beside the
limits carried on the drive context.

Routing (NousResearch#98610) deliberately stays out — these are two separate policies and
each PR owns one.
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks @andredezzy — declining this one on design grounds rather than quality: Bot Mode group rounds are serial round-robin by design, and routing a round to a subset of members changes who sees what, which the standing ruling keeps fixed (a faster round never licenses changing visibility or order). Latency work that keeps every member in the loop is welcome. Closing.

@teknium1 teknium1 closed this Sep 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/usage-cost Token accounting, usage reporting, billing, cost tracking comp/desktop Electron desktop app (apps/desktop/*) needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have type/perf Performance improvement or optimization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants