feat(plugins): add streaming output observer hooks - #64317
Conversation
a7a6d15 to
8447f41
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for implementing the off-token-path observer direction and documenting the payloads. The underlying streaming-hook gap remains present on current origin/main.
Problems
agent/plugin_stream_hooks.py:42drains one global queue viaplugins.invoke_hook().PluginManager.invoke_hook()runs registered callbacks serially (hermes_cli/plugins.py:1912-1927), so one slow plugin blocks every observer consumer. This does not meet the per-consumer queue + fan-out-worker contract stated by the maintainer on #64161.- The Bedrock call still gates
on_reasoning_deltaon UI callbacks (agent/chat_completion_helpers.py:2106), so plugin-only reasoning observers enabled throughplugins.stream_reasoning_deltasreceive no Bedrock reasoning deltas. agent/plugin_stream_hooks.py:22addsHERMES_PLUGIN_STREAM_HOOK_QUEUE_SIZE; behavioral configuration belongs inconfig.yaml, not a new user-facingHERMES_*variable.
Suggested changes
- Isolate each registered consumer with its own bounded queue/worker and test slow-consumer isolation.
- Include the reasoning opt-in in the Bedrock callback predicate and add a plugin-only Bedrock test.
- Remove the environment-variable tuning path or move it to supported config.
Automated hermes-sweeper review.
|
@teknium1 addressed the review on #64317:
Local verification: Pushed at |
SummaryOne PR addresses #64161. #64317 adds observer-only stream lifecycle, normalized delta, and interim-message hooks, with bounded off-token-path delivery, per-callback isolation, and opt-in reasoning deltas. Related pull requests
Suggested consolidationKeep #64317 open with a salvage path: preserve its observer-only hook API, off-token-path per-consumer dispatcher, reasoning opt-in, documentation, and regression tests while obtaining contributor confirmation that the revised diff satisfies the corrected v1 design. There are no duplicate PRs to close. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I64161(["issue #64161 (open)"])
P64317["PR #64317 (open)"]
P64317 -->|best fix| I64161
class I64161 open
class P64317 open
class P64317 best
class P64317 target
click I64161 "https://github.com/NousResearch/hermes-agent/issues/64161"
click P64317 "https://github.com/NousResearch/hermes-agent/pull/64317"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 40 kB of PR diffs, 7 kB of issue/PR text, 6 kB of discussion (9 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
Salvage of PR #64317 (@deaneeth) onto current main, implementing #64161: observer-only on_stream_start / on_stream_delta / on_stream_end / on_interim_message plugin hooks dispatched through a host-owned bounded queue (one worker per callback) so plugin callbacks never run inline on the token path. Reasoning deltas are opt-in via plugins.stream_reasoning_deltas.
|
Merged via #84924 with your commit intact (rebase-merge, authorship preserved) — the streaming observer hooks are on main with the bounded-queue contract and reasoning-delta opt-in exactly as you built them, adapted onto main's newer single-writer stream fencing. Thanks @deaneeth! Closing this original. (#64161) |
|
Huge thanks to @teknium1 for salvaging this PR and getting it merged! I’m really glad to have contributed to Hermes Agent, and I truly appreciate you taking the time to bring it across the finish line. |
Summary
Closes #64161 as part of #64182.
This adds observer-only plugin hooks for streaming LLM output without putting plugin callbacks on the token path:
on_stream_startwhen a streaming response beginson_stream_deltafor normalized text deltason_stream_endwhen streaming finishes or errorson_interim_messagefor mid-loop assistant messages surfaced before the final answerThe hook delivery path uses host-owned bounded queues with one background worker per registered callback. If one plugin stalls, only that callback's queue can fill and drop its oldest pending observer event; other observers continue receiving events independently. Hook return values are ignored and callback exceptions are isolated from the stream.
Reasoning deltas remain private by default and require explicit opt-in:
Implementation notes
hermes_cli.plugins.VALID_HOOKS.agent.plugin_stream_hooksas the async per-consumer dispatcher for stream observer hooks.on_stream_start,on_stream_delta, andon_stream_endas stream consumers so plugin-only streaming observers can activate streaming transport.on_interim_messageobservable without letting an interim-only plugin force streaming transport.HERMES_PLUGIN_STREAM_HOOK_QUEUE_SIZEenvironment variable path; the queue bound is internal.plugins.stream_reasoning_deltasis enabled.Non-goals
Verification
Results:
tests/run_agent/test_plugin_stream_hooks.py: 12 passedtests/run_agent/test_streaming.py+tests/agent/test_bedrock_adapter.py: 183 passedpy_compile: passedruff check: passedgit diff --check upstream/main...HEAD: passed