Skip to content

fix(agent): pass lane metadata to post-llm hooks - #54452

Open
Tosko4 wants to merge 3 commits into
NousResearch:mainfrom
Tosko4:fix/lcm-post-hook-metadata
Open

Tosko4 wants to merge 3 commits into
NousResearch:mainfrom
Tosko4:fix/lcm-post-hook-metadata

Conversation

@Tosko4

@Tosko4 Tosko4 commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Refresh status — 2026-07-15

Rebased onto current main (569b912d7d0931c7256e9f5fb326609e9deda377). Current head: e5c16a5ac84a44802a4e44909b1034fcd14e5494.

The original gap still exists on current main: post_llm_call receives the generic turn/session fields but not the gateway lane metadata already retained on the agent. This refresh also documents the extended hook contract and avoids exposing the live context-engine/compressor object to observer plugins.

Exact-head local validation:

  • Relevant context-engine, turn-finalizer, hook, NeMo Relay, model-tool, and gateway suites: 172 passed.
  • python3 -m ruff check agent/turn_finalizer.py tests/test_post_llm_call_hook_metadata.py — passed.
  • python3 -m py_compile agent/turn_finalizer.py tests/test_post_llm_call_hook_metadata.py — passed.
  • git diff --check origin/main...HEAD — passed.
  • cd website && npm run build — passed; generated all locales. Existing site-wide broken-anchor warnings remain unrelated.
  • Exact-head GitHub CI passes all PR-related checks but is currently blocked by a base regression in tests/agent/test_codex_responses_adapter.py, tracked and fixed in test(codex): pin codex_backend issuer in xai-scoped salvage test #64844.

What does this PR do?

Extends the existing post_llm_call plugin hook with optional, generic metadata:

  • safe context_engine name (for example lcm), never the live compressor object
  • conversation_id / gateway_session_key
  • sender/chat/thread fields already present on gateway-backed agents

This closes a host-contract gap for plugins that persist completed turns by lane. The same payload works for Discord channels, Discord threads/forum topics, Telegram DMs, and other gateway conversations, while CLI/non-gateway sessions receive empty strings for unavailable gateway fields.

The change is intentionally small and generic. It does not add a core tool, vendor a plugin, change prompt construction, or rebuild tools/system prompts mid-conversation.

Related Issue

No issue number.

Duplicate searches for existing post_llm_call context-engine/lane-metadata work returned no matching implementation before this refresh.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • agent/turn_finalizer.py
    • Adds context_engine, conversation_id, gateway_session_key, sender_id, chat_id, chat_name, chat_type, and thread_id to successful post_llm_call payloads.
    • Reads the real private gateway lane attributes first, with public-attribute fallbacks for compatibility.
    • Sends only the engine name instead of the live compressor object. Compressor instances may hold provider credentials, and observer plugins may serialize hook kwargs.
  • tests/test_post_llm_call_hook_metadata.py
    • Covers Discord thread metadata, Telegram private-session compatibility, and public lane-attribute fallbacks.
    • Guards against exposing a compressor credential through the hook payload.
  • Hook documentation
    • Documents every optional field and its type.
    • Clarifies empty/unavailable values outside gateway sessions and the safe engine-name contract.

How to Test

python3 -m pytest \
  tests/test_post_llm_call_hook_metadata.py \
  tests/agent/test_turn_finalizer_interrupt_alternation.py \
  tests/agent/test_turn_finalizer_cleanup_guard.py \
  tests/agent/test_context_engine_host_contract.py \
  tests/agent/test_context_engine.py \
  tests/run_agent/test_plugin_context_engine_init.py \
  tests/run_agent/test_commit_memory_session_context_engine.py \
  tests/plugins/test_nemo_relay_plugin.py \
  tests/hermes_cli/test_plugins.py::TestPluginHooks::test_hook_none_returns_excluded \
  tests/test_model_tools.py \
  tests/gateway/test_pii_redaction.py \
  tests/gateway/test_pre_gateway_dispatch.py \
  -q -o addopts=

python3 -m ruff check agent/turn_finalizer.py tests/test_post_llm_call_hook_metadata.py
python3 -m py_compile agent/turn_finalizer.py tests/test_post_llm_call_hook_metadata.py
git diff --check origin/main...HEAD

cd website
npm run build

Local result: 172 passed; lint, compile, diff check, and documentation build passed.

The full tests/hermes_cli/test_plugins.py file also discovers the locally installed Noisegate entrypoint and therefore has two exact-count failures in this environment. The same two failures reproduce unchanged on origin/main; the scoped hook test from that file passes and the PR does not modify plugin discovery.

Tested on Linux 7.0.0-27-generic, Python 3.11.15.

Scope ledger

  • Valid but independent: tests/agent/test_codex_responses_adapter.py::test_normalize_codex_response_salvage_is_xai_scoped fails unchanged on clean base 569b912d7d0931c7256e9f5fb326609e9deda377. This PR does not touch the adapter or test. Existing fix: test(codex): pin codex_backend issuer in xai-scoped salvage test #64844. No patch or duplicate issue added here.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs/issues to make sure this isn't a duplicate
  • My PR contains only related changes
  • I've run pytest tests/ -q locally and all tests pass
    • Focused and affected-area validation is green; exact-head GitHub CI provides the repository-wide matrix.
  • I've added tests for my changes
  • I've tested on my platform: Linux 7.0.0-27-generic, Python 3.11.15

Documentation & Housekeeping

  • I've updated the hook and plugin documentation
  • cli-config.yaml.example is unchanged; no config keys were added
  • CONTRIBUTING.md / AGENTS.md are unchanged; no architecture or workflow contract changed
  • I've considered cross-platform impact; no OS-specific behavior, paths, or process handling changed
  • Tool descriptions/schemas are unchanged; this does not change model tools

For New Skills

N/A.

Screenshots / Logs

Companion LCM plugin PR: stephenschoettler/hermes-lcm#292

The LCM-side implementation owns storage/filtering and active-engine resolution; this PR only exposes the generic, safe host metadata contract.

@alt-glitch alt-glitch added type/feature New feature or request comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have labels Jun 28, 2026
@Tosko4

Tosko4 commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

CI follow-up: all required checks are green, including Python tests, lints, attribution, uv lock, supply-chain scan, and amd64 Docker build. The only red check is optional Build&Test Docker image / build-arm64, and its log fails while exporting cache to GHCR with denied: installation not allowed to Write organization package; that is a package-permission/export issue, not a test or code failure from this PR.

@Tosko4

Tosko4 commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

Updated the Agent PR.

  • post_llm_call now reads the real AIAgent lane attrs first: _chat_id, _chat_name, _chat_type, _thread_id.
  • It falls back to the public attr names for compatibility.
  • Tests now mirror the real private attrs for Discord and Telegram, plus a public-attr fallback case.

Validation run on 84c258de1b6be6afeff4bddbe544d5b195bcfa3b:

python -m pytest tests/test_post_llm_call_hook_metadata.py tests/agent/test_turn_finalizer_interrupt_alternation.py tests/agent/test_turn_finalizer_cleanup_guard.py -q -o addopts=
python -m ruff check agent/turn_finalizer.py tests/test_post_llm_call_hook_metadata.py
python -m py_compile agent/turn_finalizer.py tests/test_post_llm_call_hook_metadata.py
git diff --check

Result: 13 passed, ruff passed, py_compile passed, diff check passed.

@Tosko4
Tosko4 force-pushed the fix/lcm-post-hook-metadata branch from 84c258d to 353e1f9 Compare July 5, 2026 08:56
@Tosko4

Tosko4 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed this against current main (708b57e009fc1819f00bd2c9ad4ace0091473a0c).

Relevance check:

  • main still does not pass context_compressor / conversation_id / gateway_session_key / gateway chat/thread metadata into post_llm_call.
  • The underlying lane data is still present on AIAgent, and context-engine session start already gets conversation_id, so this PR is still the narrow missing post-turn hook contract rather than stale/obsolete code.
  • Duplicate/supersession search only found this PR for the same post_llm_call + context-engine/lane-metadata gap.

Refresh work:

  • rebased cleanly; current head is 353e1f967a559570c97f30e01178e5329c9ad972
  • PR is mergeable and no longer behind main
  • local validation: 13 focused finalizer/hook tests passed, 14 context-engine+hook tests passed, 66 affected-area tests passed; ruff, py_compile, and git diff --check passed
  • GitHub CI: all required checks pass, no failing check runs

@teknium1 this should be ready for another look.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused host-contract fix. The premise is verified on current main: agent/turn_finalizer.py:394-403 emits post_llm_call without the lane/context-engine data that agent/agent_init.py:413-420 already retains and gateway/run.py:18311-18320 supplies.

Problems

  • The new plugin-facing payload is undocumented. website/docs/user-guide/features/hooks.md:597-613 lists the post_llm_call callback contract, and website/docs/developer-guide/plugins/index.md:594-603 points readers to that page as the full hook reference. Neither documents the new optional lane/context-engine fields.

Suggested changes

  • Extend those hook references with the optional metadata fields and clarify that gateway-specific values are empty or unavailable for non-gateway sessions.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 15, 2026
@Tosko4
Tosko4 force-pushed the fix/lcm-post-hook-metadata branch from 353e1f9 to 00735d3 Compare July 15, 2026 08:50
@Tosko4
Tosko4 force-pushed the fix/lcm-post-hook-metadata branch from 00735d3 to e5c16a5 Compare July 15, 2026 08:54
@Tosko4

Tosko4 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

@teknium1 addressed this in e5c16a5ac84a44802a4e44909b1034fcd14e5494.

The optional post_llm_call payload is now documented in both requested locations, including the empty/unavailable semantics outside gateway sessions.

I also tightened the context-engine field while validating the contract: hooks now receive only context_engine as a safe engine-name string, not the live context_compressor object. Compressor instances can contain provider credentials, and the bundled NeMo Relay observer serializes hook kwargs. The regression test now verifies that a compressor credential does not reach the hook payload.

Local exact-tree validation is green:

  • 172 relevant tests passed, including the NeMo Relay suite
  • ruff, py_compile, and diff check passed
  • documentation build passed

Exact-head CI currently has one unrelated failure: test_normalize_codex_response_salvage_is_xai_scoped. I reproduced it unchanged on clean base 569b912d7, and it is already fixed by #64844. I kept it out of this diff and added it to the PR scope ledger. Once #64844 lands on main, I’ll rebase and confirm the new exact head is green.

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

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants