fix(langfuse): prefer gateway_session_key for session grouping - #71608
fix(langfuse): prefer gateway_session_key for session grouping#71608Slobaka wants to merge 1 commit into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for targeting the right separation: gateway_session_key is already the documented stable channel identity, while /v1/responses creates a fresh UUID when no previous_response_id is available (gateway/platforms/api_server.py:4974-4977).
Problems
- Current main routes the Langfuse request tracer through
pre_api_request(plugins/observability/langfuse/__init__.py:1128-1135), dispatched atagent/conversation_loop.py:2147. Its payload currently containssession_id(agent/conversation_loop.py:2151) but notgateway_session_key. Salvage the forwarding change onto that current hook seam; otherwise the plugin cannot receive the stable key on the active tracing path. - The proposed regression test calls the plugin handler directly, so it does not prove the API-server/agent/hook propagation chain required for this bug. Add a focused integration test for that boundary.
Suggested changes
- Forward
agent._gateway_session_keyin the currentpre_api_requestpayload and preserve the plugin’s existingsession_idfallback. - Test two distinct unchained Responses-style session IDs sharing one gateway key through the hook boundary.
Automated hermes-sweeper review.
| @@ -1891,6 +1892,7 @@ def run_conversation( | |||
| turn_id=turn_id, | |||
There was a problem hiding this comment.
Please carry this forwarding change onto current main’s active pre_api_request dispatch as well. Langfuse registers its request tracer on pre_api_request (plugins/observability/langfuse/__init__.py:1132), and current agent/conversation_loop.py:2147-2155 does not yet include gateway_session_key.
| def test_langfuse_session_id_prefers_gateway_key(self): | ||
| mod = self._fresh_plugin() | ||
| assert mod._langfuse_session_id("uuid-ephemeral", "agent:main:api:chat:1") == ( | ||
| "agent:main:api:chat:1" |
There was a problem hiding this comment.
This validates the plugin helper directly, but not the AIAgent → pre_api_request hook propagation. Please add a focused integration-level test that captures the active hook payload from an agent initialized with gateway_session_key.
SummaryOne PR addresses issue #71556. #71608 implements the correct stable-key preference in Langfuse, but its current diff does not forward gateway_session_key through the active pre_api_request dispatch and therefore does not yet complete the reported propagation path. Related pull requests
Suggested consolidationKeep #71608 open with a salvage path: forward agent._gateway_session_key in the active pre_api_request payload identified at agent/conversation_loop.py:2147-2155, retain the existing session_id fallback, and add the requested integration-level test capturing that hook payload for distinct unchained sessions sharing one gateway key. It remains the recorded best fix and sole candidate, but these contributor-requested changes are required before consolidation can advance. 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
I71556(["issue #71556 (open)"])
P71608["PR #71608 (open)"]
P71608 -->|best fix| I71556
class I71556 open
class P71608 open
class P71608 best
class P71608 target
click I71556 "https://github.com/NousResearch/hermes-agent/issues/71556"
click P71608 "https://github.com/NousResearch/hermes-agent/pull/71608"
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: 11 kB of PR diffs, 5 kB of issue/PR text, 3 kB of discussion (5 comments), 3 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
Forward agent._gateway_session_key on pre_api_request and sibling hooks, and group Langfuse traces by that stable channel key when present. Fixes NousResearch#71556
6e2dbea to
13f1f5c
Compare
|
Addressed review feedback on current
|
Summary
gateway_session_key(X-Hermes-Session-Key) over ephemeralsession_idwhen setting Langfuse session groupingagent._gateway_session_keyso the plugin can see it/v1/responses-style turns with different UUIDs but the same gateway key open one Langfuse sessionFixes #71556
Why
Open WebUI Responses mode often does not chain
previous_response_id. The API server then mints a fresh UUIDsession_idper request while the stable channel key lives only ongateway_session_key. Langfuse previously keyed solely onsession_id, so each turn became a disconnected session.Test plan
bash scripts/run_tests.sh tests/plugins/test_langfuse_plugin.py -- -k 'GatewaySession or TurnTrace or langfuse_session'