fix(langfuse): include system prompt in generation input (Anthropic system + Codex instructions) - #64292
Conversation
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
This PR includes the system prompt in generation input for Langfuse.
Looks Good
- Targeted fix for Langfuse integration.
- Well-scoped: 4 files.
No Issues Found
Reviewed by Hermes Agent
teknium1
left a comment
There was a problem hiding this comment.
Thanks for extending the Langfuse fix to the Codex/Responses request shape; the current Anthropic and Codex premise is verified.
Problems
agent/bedrock_adapter.py:580-583also removes the system prompt from messages, encoding it as Bedrock blocks shaped{"text": content}and sending it through a separatesystemkwarg atagent/bedrock_adapter.py:951-962. The new check atplugins/observability/langfuse/__init__.py:497only accepts dict blocks wheretype == "text", so the proposed helper returns no system entry for Bedrock and leaves the same trace gap on that provider.
Suggested changes
- Accept Bedrock text blocks in
_serialize_system_prompt()and add aBedrockTransport().build_kwargs()regression through the hook-boundary helper, alongside the Anthropic and Codex cases.
Automated hermes-sweeper review.
| if isinstance(system_prompt, list): | ||
| parts: list[str] = [] | ||
| for block in system_prompt: | ||
| if isinstance(block, dict) and block.get("type") == "text": |
There was a problem hiding this comment.
Bedrock Converse sends separate system blocks as {"text": ...} (agent/bedrock_adapter.py:580-583,951-962), not {"type": "text", ...}. This condition drops every Bedrock system prompt, leaving the same Langfuse gap there; please accept that block shape and cover it with a transport-boundary regression.
Include Anthropic, Codex/Responses, and Bedrock system prompts in Langfuse generation input, with regression coverage across provider call paths. Co-authored-by: Dan Benyamin <db@project-aeon.com> Co-authored-by: Cursor <cursoragent@cursor.com>
dd55a07 to
009d803
Compare
Providers that move the system prompt out of messages made it vanish from traces: Anthropic Messages carries it as a separate system kwarg (str or content-block list) and the Responses/Codex API as top-level instructions, so generation inputs showed conversations without the agent's instructions, skills, or memory. conversation_loop now derives the system prompt as actually sent to the provider and forwards it to hooks; the plugin prepends a role: system entry when messages don't already carry one. Serialization routes through _capture_content so capture modes apply to system prompts too. Adopted from NousResearch#64292, which extends NousResearch#32175's Anthropic fix to the Codex/Responses path — thanks @FnExpress and @db-aeon. Co-authored-by: FnExpress <37214785+FnExpress@users.noreply.github.com> Co-authored-by: Dan Benyamin <db@project-aeon.com>
|
adopted into #83437 with co-author credit — thanks for extending the fix across Anthropic, Codex, and Bedrock shapes. integration changes: system-prompt serialization routes through the branch's capture modes, and the Bedrock premise test tolerates the transport's cachePoint block on current main. |
… fan-out Salvaged from PR #83437 by @erosika, with adopted fixes from @bgodlin (#81054), @aldoeliacim (#82332), @nftpoetrist (#42326), @rodboev (#39653), @FnExpress (#64292, supersedes #32175 by @db-aeon), @Per0-1 (#61166), @NaMinhyeok (#64797), and @liuhao1024 (#43130). Widens the bundled Langfuse plugin from 6 to 11 hooks and fixes two attribution bugs. Also adopts shutdown/atexit lifecycle fixes and composes 8 prior community PRs with interaction-fix follow-ups. Model attribution: on_pre_llm_request and on_post_llm_call now prefer the wire value (request body model, response model) over the agent attribute, which goes stale after /model switch or provider fallback. Cost total: both cost paths now send a summed total alongside the per-type breakdown, since Langfuse does not derive calculatedTotalCost from cost_details keys. Subscription-included routes send no cost keys at all. New coverage: api_request_error closes failed generations with ERROR level; on_session_finalize/on_session_end close dangling traces for tool-only and interrupted turns; subagent_start/subagent_stop trace delegated children as spans; MoA advisor fan-out emits one generation per advisor priced at the advisor's own model. Capture modes: HERMES_LANGFUSE_CAPTURE=metadata|sanitized|full (default sanitized). Sanitized mode redacts secret patterns before truncation. Adopted lifecycle fixes: shutdown client at session finalize when reason=shutdown (not on session rotation); atexit finalizer ends open root spans for short-lived processes; root context manager exited to prevent interpreter-teardown TypeError; TOCTOU on _get_langfuse() fixed with lock; reasoning_content surfaced in traces; system prompt included in generation input for Anthropic/Codex/Bedrock; SDK v3 update_trace replaces set_trace_io. Closes #29482, #43129, #72661. Supersedes #81054, #82332, #42326, #39653, #64292, #32175, #61166, #64797, #43130. Partially addresses #67544 (capture modes + secret redaction; user_id remains open).
|
Merged via #85439 — your fix was adopted and composed into the wider Langfuse tracing PR by @erosika. Your contribution is credited in the commit body. Thanks @FnExpress! |
Summary
Langfuse LLM call generation observations show conversations without the agent's system prompt (skills, memory, SOUL, tool guidance) for every provider that moves the system prompt out of
messages:systemkwarg (agent/anthropic_adapter.py), reported in fix(langfuse): include Anthropic system prompt in generation input #32175instructions(agent/transports/codex.py:132-139extractsmessages[0]intoinstructionsand drops it from the payload) — the sibling call path, previously uncoveredThe
pre_api_requesthook only forwardedrequest_messages, so traces for both paths looked like conversations with no instructions at all — e.g. a Codex-backedHermes turnwhoseLLM call 1shows 18k prompt tokens but only a one-line user message as input.This PR salvages #32175 by @db-aeon onto current
main(cherry-picked so authorship survives, per the AGENTS.md salvage etiquette), then fixes the whole bug class:agent/conversation_loop.pyderives the system prompt the provider actually receives —api_kwargs["system"]→api_kwargs["instructions"]→messages[0]fallback, extracted into a_system_prompt_for_hooks()helper — and passes it topre_api_requestassystem_promptplugins/observability/langfuseprepends a serializedrole: systementry to the generation input (after the last-12 window, so long conversations never drop it), flattens Anthropic content-block lists, skips the prepend whenmessages[0]already carries the system role (chat_completions), and recordssystem_prompt_charsmetadataCommits
ResponsesApiTransport().build_kwargs()/AnthropicTransport().build_kwargs()output flows through the loop's derivation into the plugin hook with no Hermes internals mocked (only the Langfuse client is faked)_system_prompt_for_hooks()extractionTest plan
tests/plugins/test_langfuse_plugin.py— 59 passed (48 pre-existing + 9 new + 2 from fix(langfuse): include Anthropic system prompt in generation input #32175)tests/plugins/,tests/hermes_cli/test_plugins.py,tests/run_agent/test_run_agent_codex_responses.py— no regressions vs cleanmainscripts/run_tests.shfull suite — remaining failures reproduce identically on cleanmain(environment-specific, macOS), zero delta from this changeobservability/langfuse, ranhermes chaton anopenai-codexmodel — generation input now starts withrole: system(102,659 chars,system_prompt_charsmetadata matches); chat_completions sessions unchanged (single system entry, no duplication)Closes #32175 (supersedes it while preserving its commit; happy to rebase if @db-aeon prefers to land his PR first — this one then shrinks to the Codex path + tests).