9 Commits

Author	SHA1	Message	Date
Hyeonsang Cho	9146bfa03d	feature(gateway): issue request trace ids unconditionally (#5119) ... * refactor(gateway): issue request trace ids unconditionally The request trace id was gated behind logging.enhance.enabled at every entry point, so downstream code had to keep asking whether one existed: a header-provenance flag in its own ContextVar, a precedence resolver, and three-level carrier fallbacks at each consumer. Bind one unconditionally instead. TraceMiddleware covers Gateway HTTP; ensure_trace_context covers the entry points that never touch ASGI -- scheduled occurrences, MCP task notification runs, IM channel messages, and the embedded client -- each scoped to one unit of work so a long-lived worker task cannot leak one occurrence's id into the next. The ContextVar becomes the only source; the response header, runtime context, run metadata and log records are derived outputs. Consumers now use ensure_trace_id() or resolve_trace_id(*carriers) and drop their presence guards. Removed: resolve_deerflow_trace_id, the header-provenance flag and its three helpers, set/reset_current_trace_id, is_trace_correlation_enabled and its gateway alias. BREAKING CHANGE: every Gateway HTTP response now carries X-Trace-Id and it cannot be turned off; logging.enhance.enabled controls log output only. Installations on the default enabled: false will start seeing the header. No config keys were added or removed. * fix(gateway): stop persisting a caller-supplied trace id on the run record body.metadata forks two ways: through build_run_config into the live run config, which the run worker restamps, and through create_or_reject into the run record that the runs API echoes verbatim. Only the first was covered, so a client sending metadata.deerflow_trace_id made the most durable and most visible surface of a run disagree with the X-Trace-Id and the log lines the same request produced -- a correlation id that does not match the logs is worse than none. Stamp the server-issued id once at the trust boundary so both forks receive it, preserving the caller's own metadata keys. Close the same gap on config.context, which reaches the runtime context by a separate path: _build_runtime_context no longer merges server-owned keys from the caller, and _install_runtime_context assigns rather than setdefaults. A thread's metadata is no longer seeded with the run-scoped id of whichever run created it -- one thread spans many runs and as many trace ids. Found by driving a real run through the Gateway and reading the run back from the runs API; every unit test built its metadata by hand and so could not see it. * fix(gateway): expose X-Trace-Id to split-origin browser clients X-Trace-Id is not on the CORS safelist, so a browser client served from a separate origin could not read it -- and those are exactly the clients that cannot read the Gateway's logs either, leaving them with nothing to quote in a bug report. Same-origin nginx deployments were unaffected, which is why this stayed hidden. Add it to CORS_EXPOSED_HEADERS beside Content-Location, referencing TRACE_ID_HEADER rather than repeating the literal. * fix(gateway): keep X-Trace-Id on unhandled-exception 500s Starlette's ServerErrorMiddleware sits outside every user middleware and emits unhandled-exception 500s through the raw send, so those responses never pass TraceMiddleware's header-writing wrapper. The 500 for a server bug is exactly the response a user most needs to correlate with a log line, and it was the one response that shipped without the id. TraceMiddleware now tracks whether http.response.start has been sent. On an exception with no response started it emits its own plain 500 carrying the header, then re-raises: the outer ServerErrorMiddleware sees the response already started and only re-raises too, so the server's exception logging is untouched. An exception mid-stream keeps propagating unchanged — a second response start cannot be sent, and the already-written header stands. The trace id is printable ASCII by construction (normalize_trace_id / generate_trace_id), which is what makes the raw latin-1 header encoding safe. * fix(gateway): strip the forged trace id from the persisted request echo The run-record fix stopped a forged metadata.deerflow_trace_id on the authoritative metadata surface, but the raw request echo still carried one: create_or_reject persists body.config verbatim as runs.kwargs_json, which the runs API serves back. A client posting config.context.deerflow_trace_id therefore still got its forged value stored and echoed on one API surface while the header, logs, run metadata, and checkpoint all carried the real id — the id is ignored as input there, so echoing it back only manufactures disagreement. Two changes close it. redact_config_secrets — already the shared scrub for that echo, applied at admission and again at serve time, so historical records are covered too — now also drops deerflow_trace_id from config.metadata and config.context. And build_run_config now merges run metadata onto a copy of the caller's config["metadata"] instead of updating it in place: the nested values of the request config are reference copies, so the in-place merge was writing the server-stamped key through into body.config, contaminating the "what the client sent" record before it was persisted (and incidentally masking the forged-value echo on the metadata container). The regression test posts a forged id through body.metadata, config.metadata, and config.context at once and reads the kwargs echo back off the run record, failing if either leak returns. * docs(harness): record the trace-echo scrub, 500 fallback, and accepted retry divergence The trace section of the harness AGENTS.md now covers the two fixes that close the derived-output rule (the kwargs-echo scrub in redact_config_secrets plus build_run_config's copy merge, and TraceMiddleware's own 500 for unhandled exceptions), and CHANGELOG gains their Fixed entries. It also writes down the one accepted divergence: a crash-recovered scheduled launch reuses the durable run through its idempotency key, and start_run returns early on idempotency_reused without restamping — so the run record keeps the first attempt's deerflow_trace_id while the retry's own log lines carry the freshly minted id of its ensure_trace_context binding. The divergence is confined to the crash-recovery window and is accepted rather than fixed: restamping on reuse would rewrite a persisted record for a run that already exists, which is worse than two ids that each correlate their own attempt's logs. Written down so the next reader of the scheduler recovery path does not diagnose it as a bug. * docs(config): align the logging.enhance schema note with the unconditional trace id The config-module AGENTS.md still described logging.enhance as the gate for the Gateway X-Trace-Id header and Langfuse deerflow_trace_id. That model is gone: ids are issued unconditionally and this block decides log output only. Left as-is, the stale wording invites an agent to "restore" a header gate it believes was lost. Reworded to match the sibling AGENTS.md files and config.example.yaml, with a pointer to the Request Trace Context section that owns the full model. * docs(changelog): link the trace entries to #5119 The five new entries pointed at the ([#XXXX]) placeholder with no reference definition, rendering as literal text instead of a link — and RELEASING.md step 2 relies on those references when the section becomes release notes. All five now point at #5119, with the definition appended to the reference block. * refactor(harness): rename _stream_without_trace_context to _stream_turn The name asserted the opposite of what the method now does. It was accurate while logging.enhance.enabled could route stream() around the trace scope; with the gate gone it is the only stream implementation left, and it binds the id itself via ensure_trace_id(). Private, so the rename touches only the definition and the one stream() call site. * docs(harness): fit the trace-context guidance inside the AGENTS.md chain budget The expanded Request Trace Context section pushed the effective AGENTS.md chain for agents/middlewares to 99,815 bytes, past the 98,304 hard limit scripts/check_agent_guidance.py enforces in CI (AG002). Compressed the section from 7,359 to 4592 bytes with no facts removed: the entry-point table, the derived-output rule and its enforcement points, the accepted scheduled-retry divergence, the two resolution helpers, the stream() binding rationale, the log-output-only gate, the CORS listing, the 500 fallback, and the test map all remain. Sized against the merge, not just the branch: current main grew the same chain by ~724 bytes, so the check was verified on the merged tree as well (97,772 bytes; branch tree 97,048). * fix(gateway): declare content-length on the fallback 500 The pre-response 500 declared content-type but no content-length, leaving the framing to the ASGI server: chunked on HTTP/1.1, close-delimited on HTTP/1.0 — the one wire difference from the ServerErrorMiddleware response it replaces, which sends content-length: 21. The explicit header keeps the fallback byte-identical to what clients saw before. * docs(readme): drop the trace-correlation condition from the translations The zh/ja/fr/ru Langfuse sections still said metadata.deerflow_trace_id matches X-Trace-Id "when request trace correlation is enabled". The id now always matches and that condition no longer exists, so each bullet states the unconditional match and that logging.enhance.enabled only controls whether the id is printed into logs — the one piece of the feature a user can still configure. * test(gateway): pin TraceMiddleware wiring through create_app() Every X-Trace-Id test exercised a hand-built four-route app, so the real stack's add_middleware(TraceMiddleware) line was pinned by nothing: deleting it — or short-circuiting above it — passed CI while silently dropping both the response header and the ambient id the run-record stamp and enhanced log records derive from. One case now drives /health through create_app() and asserts the inbound id round-trips; mutation-checked by removing the wiring line, which fails exactly this test. * docs(gateway): note the fallback 500 is CORS-opaque The pre-response 500 is emitted outside CORSMiddleware — the exception has already unwound past it — so it carries no Access-Control-Allow-Origin and a split-origin browser client cannot read the id on this one response, unchanged from the ServerErrorMiddleware 500 it replaces. Documented on the class and in the CHANGELOG entry rather than fixed: replicating the origin allowlist outside CORSMiddleware would let the two policies drift. * fix(harness): keep abandoned-stream cleanup inside the trace binding stream() binds the turn's id around each next(inner) and resets it before yielding, but the finally's inner.close() ran after that binding was gone. Abandoning the stream therefore drove the inner LangGraph generator's GeneratorExit/finally path with no trace id — or an unrelated ambient one from whichever context ran the close — so cancellation and finalization logs and callbacks did not correlate with the turn they belong to. inner.close() is now wrapped in a local bind/reset of the same turn id. The token is set and reset in the same frame, never across a yield, so the per-step cross-context safety is preserved even when GC closes the generator from another Context — pinned by the existing copy_context close test, which now exercises this path. The regression test records the id from the inner generator's finally and fails without the binding. * test(harness): teach the worker-trace fake about RunManager.cleanup Upstream #5112 (bound gateway memory after terminal runs) added a run_manager.cleanup(run_id) call to run_agent's finalization, so the merge-commit CI run failed all five worker-trace-binding tests with AttributeError on this PR's _FakeRunManager. The fake gains the same no-op shape as its other methods. * docs(gateway): bring the gateway AGENTS.md back under its soft budget Upstream #5092 grew backend/app/gateway/AGENTS.md to 40,966 bytes, 6 over the 40,960 soft budget that test_agent_guidance_check.py::test_repository_guidance_stays_below_soft_budgets_and_avoids_doc_indexes enforces — its Unit Tests run on main was cancelled by push concurrency, so main is currently red on that test and every PR merge-run inherits the failure. Two whitespace/wording trims in the row #5092 touched (a doubled space, and "its configured `context_window`" → "its `context_window`") bring the file to 40,953 with no content change. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>	2026-09-01 16:49:39 +08:00
Janlay	45adb8fbb5	perf(runtime): bound gateway memory after terminal runs (#5112) ... * fix(runtime): clean up terminal run records * perf(sandbox): bound local path caches * perf(runtime): release terminal run cycles * fix(runtime): address terminal cleanup review * fix(runtime): clean up after end publish failure * fix(runtime): guard terminal cleanup from cancellation * fix(runtime): discard fenced journal buffers * fix(runtime): harden abort and teardown paths	2026-09-01 10:56:43 +08:00
MiaoRuidx	8a78c264b7	fix(runtime): cancel runs across live gateway workers (#4500) ... * docs(runtime): design cross-worker cancellation * fix(runtime): cancel runs across gateway workers * fix(runtime): harden cross-worker cancellation races 让取消请求与 owner 终态写入通过持久化 CAS 决定先后，保证首次取消 action 在不同 worker 路由下保持一致。\n\n将 heartbeat 收敛为续租后仅发送本地中止信号，并补齐完成竞态与路由重试的回归用例。 * docs(runtime): drop implementation plan from PR 移除仅用于实现过程的跨 worker 取消设计记录，保留 README 和 backend/AGENTS.md 中面向最终行为的文档。 * fix(runtime): preserve local cancel fallback * test(runtime): adapt worker run manager fakes * docs(runtime): fix run cancel migration registry --------- Co-authored-by: MiaoRuidx <12540796+MiaoRuidx@users.noreply.github.com>	2026-07-28 21:45:20 +08:00
MiaoRuidx	735f67a5b2	fix: guard pending run startup cancellation (#4450) ... * fix: guard pending run startup cancellation * fix(run): address startup review feedback * fix(run): narrow start_run store contract --------- Co-authored-by: MiaoRuidx <12540796+MiaoRuidx@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com>	2026-07-25 23:50:21 +08:00
yym36991	0f088033fe	fix(gateway): prefer X-Trace-Id over metadata.deerflow_trace_id when header is set (#4283) ... When TraceMiddleware binds a valid inbound X-Trace-Id, worker resolution prefers that id over config.metadata.deerflow_trace_id so logs, response headers, Langfuse, and runtime context stay aligned. Also add trace-context marker reset coverage for the inbound-header flag.	2026-07-19 09:13:51 +08:00
d33kayyy	84bbdf6e00	fix(langfuse): resolve trace user from runtime context (#3794) ... * fix(langfuse): resolve trace user from runtime context The worker built langfuse_user_id from get_effective_user_id(), which reads the request-scoped _current_user ContextVar. For runs invoked over an internal token on behalf of an end user, that ContextVar is never the end user, so traces recorded langfuse_user_id="default". Switch to resolve_runtime_user_id(runtime), matching the sandbox middleware/tools sites: it reads runtime.context["user_id"] (the owner carried in the run request's context, which survives background-task boundaries) and falls back to get_effective_user_id() for no-auth / browser paths. Caller-supplied metadata still wins via inject_langfuse_metadata's setdefault. * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>	2026-07-05 00:07:27 +08:00
AnoobFeng	e3e5c73b03	feat(observability): add trace-id correlation and enhanced logging (#3902) ... * feat(observability): add trace-id correlation and enhanced logging - add opt-in gateway request trace correlation via X-Trace-Id - enhance logging with configurable trace_id-aware formatting - propagate deerflow_trace_id into runtime context and Langfuse metadata - keep enhanced logging disabled by default to preserve existing behavior * fix: harden trace correlation wiring - Make logging enhancement a restart-required startup snapshot and remove per-request config reads from TraceMiddleware - Restrict trace ids to printable ASCII before writing them to response headers, logs, and Langfuse metadata - Gate implicit DeerFlowClient trace-id creation behind logging.enhance.enabled while preserving explicit caller opt-in - Bind embedded client trace context per stream step to avoid generator ContextVar leaks and cross-context reset errors - Rebind memory update trace ids in Timer/executor worker paths so enhanced logs keep the captured correlation id - Remove unrelated __run_journal context overwrite from the trace-correlation change set * fix(gateway): avoid eager app construction on package import * fix(gateway): avoid config load during app import Keep Gateway app construction import-safe when config.yaml is absent by disabling TraceMiddleware only for that construction-time fallback path. Startup lifespan still performs strict config loading before serving.	2026-07-03 08:01:46 +08:00
xiawiie	2e15e3fe0d	fix: generate fallback title for interrupted first-turn runs (#3874) ... * fix: generate title for interrupted first turn * test(title): cover partial-exchange + dict-form messages Harden the interrupted-run fallback path added in 19fc34fd: - TitleMiddleware._should_generate_title now accepts a lone first-turn user message when allow_partial_exchange=True, so the worker can still derive a title if cancellation lands before any AI chunk is checkpointed. - runtime/runs/worker._ensure_interrupted_title computes the next checkpoint step defensively (treat missing/non-int step as 0) and renames a shadowed ckpt_config local for readability. - Add four unit tests in tests/test_title_middleware_core_logic.py: partial-exchange allows user-only, partial-exchange still respects an existing title, dict-form messages are recognized, and the sync fallback path derives a title from dict-form messages — matching what channel_values stores in the checkpoint. Refs #3859. * fix: persist interrupted-title via channel_versions bump Address PR #3874 review feedback: ``_ensure_interrupted_title`` previously called ``aput(..., new_versions={})``. LangGraph's DB-backed savers (``PostgresSaver`` and the v4 ``SqliteSaver`` blob layout) strip inline ``channel_values`` from ``put`` and only persist blobs for channels named in ``new_versions`` — so the fallback ``title`` channel was dropped on read-back and ``threads_meta.display_name`` stayed ``"Untitled"`` after refresh on those backends. The original in-memory e2e passed because ``InMemorySaver`` keeps the inline snapshot verbatim. Fix mirrors ``_rollback_to_pre_run_checkpoint`` in the same file: bump ``channel_versions["title"]`` (via ``checkpointer.get_next_version`` when available, else int/string fallbacks), persist the new version on the checkpoint, and declare it in ``new_versions`` so the DB savers actually write the blob. Regression coverage in ``tests/test_run_worker_rollback.py``: - ``test_ensure_interrupted_title_bumps_channel_version_and_declares_it_in_new_versions`` — exact ``aput`` invariants: ``new_versions == {"title": 1}``, the written checkpoint's ``channel_versions["title"]`` is bumped, and the pre-existing ``messages`` version is preserved. - ``test_ensure_interrupted_title_bumps_existing_string_version`` — string-shaped prior version (some savers use UUID-style versions); bumped value must differ from the prior, no overwrite-in-place. - ``test_ensure_interrupted_title_skips_when_title_already_set`` — title short-circuit; no extra ``aput``. - ``test_ensure_interrupted_title_returns_none_when_no_checkpoint`` — no checkpoint yet; returns ``None`` without writing. - ``test_ensure_interrupted_title_round_trip_with_real_sqlite_checkpointer`` — full round-trip against a real ``AsyncSqliteSaver`` on a disk-backed DB, then closes and re-opens the saver to simulate a fresh connection. The fallback title must still be present on the second ``aget_tuple``. This is the exact scenario the review flagged. Validated locally with the full backend suite: 5195 passed, 18 skipped. Refs #3859. Addresses review on #3874. * test(worker, title): harden interrupted-title fallback for every saver Defensive coverage on top of the channel_versions fix (commit 05253957), addressing edge cases surfaced during a second-pass review of #3874. Worker: - Extract version bump into ``_bump_channel_version(checkpointer, current)`` with explicit fallbacks for int / float / numeric-string / UUID-shaped string / None / bool, AND a wrap-around defense when the saver's ``get_next_version`` raises or returns an unchanged value. The invariant is: returned version MUST differ from the prior. Without this, a saver bug (or a custom backend) could leave ``new_versions={"title": v}`` no-op on DB savers — the very class of bug the original review pointed out. Title middleware: - Coerce ``state.get("messages")`` from ``None`` to ``[]`` on both ``_should_generate_title`` and ``_build_title_prompt``. A partially-initialized checkpoint can carry ``messages=None`` on the channel_values channel (the worker reads raw channel_values, not BaseMessages), and the default kwarg only protects against a missing key. Repro: ``TypeError: 'NoneType' object is not iterable`` from the next() generator — confirmed by reverting the fix and watching ``test_*_handles_none_messages_channel`` go red. Tests (TDD-verified red→green for the new asserts): - ``test_run_worker_rollback.py``: * ``_bump_channel_version`` — 8 tests covering every version type (int, float, numeric string, UUID-style string, None, bool) and every saver-side fault mode (no ``get_next_version`` / raising / stuck on identity). * ``test_ensure_interrupted_title_*`` — 5 additional helper boundary tests: title.enabled=false short-circuit; empty messages list; messages=None; aput-error propagation (helper contract: caller swallows, not the helper); idempotency on a real InMemorySaver across two invocations. * ``test_ensure_interrupted_title_preserves_non_title_channel_versions`` — pins that ``new_versions`` only contains ``"title"`` and that other channels' versions are untouched (regression anchor for a sloppier draft that bumped every channel). * ``test_worker_finally_block_swallows_helper_exceptions`` — pins the integration contract: even if the helper raises, the worker's threads_meta status sync still runs and ``publish_end`` is still awaited so the SSE stream closes cleanly. - ``test_title_middleware_core_logic.py``: * 4 additional tests: ``messages=None`` on both ``_should_generate_title`` and ``_build_title_prompt``; the ``role: user`` / ``role: assistant`` (OpenAI-style) dict normalization; partial-exchange path with a dict-form message. Verification: - ``PYTHONPATH=. uv run pytest tests/ -x --ignore=tests/blocking_io -q`` → 5215 passed, 18 skipped. - ``ruff check`` + ``ruff format --check`` clean on every touched file. - Red/green TDD verification: temporarily reverted the ``new_versions={}`` fix → 4 new tests went red as expected; restored and the suite is green again. Same red/green dance for the ``messages=None`` coercion. Refs #3859. Addresses second-pass review on #3874. * fix(title): ignore dict context reminders in fallback * fix(worker): link interrupted-title checkpoint to its parent The title-bump checkpoint written by ``_ensure_interrupted_title`` was landing without a ``parent_checkpoint_id`` — a real orphan in the LangGraph history graph. Reproduction (disk-backed AsyncSqliteSaver): [seed] checkpoint_id = 1f173dbc... [helper] wrote title = "Why is the sky blue?" [issue 1] new checkpoint = 1f173dbc..., parent = None [issue 1] is new checkpoint orphaned? True Root cause: ``_ensure_interrupted_title`` built ``write_config`` as ``{"thread_id": ..., "checkpoint_ns": ...}`` only. ``BaseCheckpointSaver`` implementations read ``configurable.checkpoint_id`` from that config as the *parent* id when inserting (see ``langgraph/checkpoint/sqlite/aio.py`` ``aput``: ``config["configurable"].get("checkpoint_id")`` becomes the ``parent_checkpoint_id`` column). With no value, the saver writes NULL — the new checkpoint is a tree root. Consequences: - Any future LangGraph ``runs.resume_from`` / time-travel feature has no backward edge to walk past the title-bump. - History-visualization UIs built on ``alist()`` render the title-bump as a sibling of the prior checkpoint, not its descendant. Fix: read ``checkpoint_id`` off the tuple's own config and thread it into ``write_config["configurable"]["checkpoint_id"]`` before calling ``aput``, the same pattern every middleware-driven write uses. Three new regression tests against real ``AsyncSqliteSaver`` (disk-backed, fresh connections so we exercise the on-disk read path): - ``test_ensure_interrupted_title_links_new_checkpoint_to_its_parent`` — asserts ``latest.parent_config["configurable"]["checkpoint_id"]`` equals the seeded checkpoint id. TDD red-green verified: reverting the fix flips this test red with ``AssertionError: title-bump checkpoint must have a parent_config``. - ``test_ensure_interrupted_title_appears_in_history_with_audit_marker`` — pins the audit contract: the title-bump entry in ``alist()`` carries ``metadata.source == "update"`` and ``metadata.writes`` contains ``runtime_interrupt_title``. This is a deliberate design choice — we do NOT hide the entry from history (audit trail belongs in the saver), but its source and writes marker MUST be unambiguous so UIs/tools can identify it. - ``test_ensure_interrupted_title_survives_immediate_next_turn`` — cancel → immediate user follow-up scenario. Simulates the agent's next turn appending a (user, ai) pair without touching the title channel, then opens a fresh saver and verifies the title is still present after the next-turn checkpoint write. Pins the channel-version-blob invariant established by commit 05253957 — without the ``new_versions={"title": v}`` declaration there, the title blob would vanish from the DB and this test would read back ``None``. Verification: - ``PYTHONPATH=. uv run pytest tests/ -x --ignore=tests/blocking_io -q`` → 5222 passed, 15 skipped. - ``ruff check`` + ``ruff format --check`` clean on every touched file. - Reproduction script confirms ``parent_checkpoint_id`` is now non-null and the next-turn read-back preserves the fallback title. Refs #3859. * Revert "fix(worker): link interrupted-title checkpoint to its parent" This reverts commit c763ed9334781db1acdce0f5f33d663d8d5f80ff. * test: trim over-engineered test coverage Reduce review surface area on PR #3874 by dropping defensive tests that don't pin a real invariant. After self-review: - ``_bump_channel_version``: 8 tests → 2 (happy path + saver-error fallback). Dropped float / bool / numeric-string / UUID-string / missing-get-next-version / stuck-get-next-version branches — those are speculative scaffolding for savers we don't ship. - ``_ensure_interrupted_title``: dropped ``returns_none_when_title_disabled``, ``returns_none_with_no_user_message``, ``returns_none_when_no_checkpoint`` — boundary guards already exercised by the e2e test and the ``handles_none_messages_channel`` regression anchor. Net: -107 lines of test code. Remaining coverage still pins every red-green-verified invariant (channel_versions bump, string-version bump, idempotency, sqlite round-trip, non-title channel preservation, aput-error contract, worker finally swallowing, partial-exchange). Verification: 5209 passed, 15 skipped. * fix: harden interrupted title finalization * fix: serialize interrupted title finalization * fix: preserve interrupt semantics during title finalization * fix: preserve delayed interrupted title recovery --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>	2026-07-02 00:13:01 +08:00
Xinmin Zeng	df95154282	fix(tracing): propagate session_id and user_id into Langfuse traces (#2944) ... * fix(tracing): propagate session_id and user_id into Langfuse traces Adds Langfuse v4 reserved trace attributes (langfuse_session_id, langfuse_user_id, langfuse_trace_name, langfuse_tags) to RunnableConfig.metadata inside the run worker, so the langchain CallbackHandler can lift them onto the root trace. - New deerflow.tracing.metadata.build_langfuse_trace_metadata() returns the reserved keys when Langfuse is in the enabled providers, else {}. - worker.run_agent merges them with setdefault so caller-supplied keys win, allowing per-request overrides from upstream metadata. - session_id mirrors the LangGraph thread_id; user_id reads get_effective_user_id() (falls back to "default" in no-auth mode). - trace_name defaults to "lead-agent"; tags carry env and model name when DEER_FLOW_ENV (or ENVIRONMENT) and a model name are present. Closes #2930 * fix(tracing): attach Langfuse callback at graph root so metadata propagates The first commit injected ``langfuse_session_id`` / ``langfuse_user_id`` / ``langfuse_trace_name`` / ``langfuse_tags`` into ``RunnableConfig.metadata``, but on ``main`` the Langfuse callback is attached at *model* level (``models/factory.py``). LangChain still threads ``parent_run_id`` through the contextvar, so the handler sees the model as a nested observation and ``__on_llm_action`` strips the ``langfuse_*`` keys (``keep_langfuse_trace_attributes=False``). The trace's top-level ``sessionId`` / ``userId`` therefore stayed empty in deer-flow's LangGraph runtime — confirmed live against a real Langfuse instance. This commit moves the callback to the **graph invocation root** so the handler fires ``on_chain_start(parent_run_id=None)`` and runs the ``propagate_attributes`` path that actually lifts ``session_id`` / ``user_id`` onto the trace: - ``models/factory.py``: add ``attach_tracing`` keyword (default ``True``) so standalone callers (``MemoryUpdater``, etc.) keep their direct model-level tracing. - ``agents/lead_agent/agent.py``: call ``build_tracing_callbacks()`` once inside ``_make_lead_agent`` and append the result to ``config["callbacks"]``; the four in-graph ``create_chat_model`` sites (bootstrap, default agent, sync + async summarization) pass ``attach_tracing=False`` to avoid duplicate spans. - ``agents/middlewares/title_middleware.py``: same ``attach_tracing=False`` for the title-generation model, since it inherits the graph's RunnableConfig via ``_get_runnable_config``. Test updates: - ``tests/test_lead_agent_model_resolution.py`` and ``tests/test_title_middleware_core_logic.py``: extend the fake ``create_chat_model`` signatures / mock assertions to accept the new ``attach_tracing`` kwarg. - ``tests/test_worker_langfuse_metadata.py``: switch the no-user fallback test from direct ContextVar mutation to ``monkeypatch.setattr`` on ``get_effective_user_id`` to avoid pollution across the langfuse OTel global tracer provider. - ``tests/conftest.py``: add an autouse fixture that resets ``deerflow.config.title_config._title_config`` to its pristine default after every test. Any test that loads the real ``config.yaml`` (via ``get_app_config()``) calls ``load_title_config_from_dict`` and mutates the module-level singleton, which previously poisoned the title-middleware suite when run after, e.g., the new ``test_worker_langfuse_metadata.py`` cases. The fixture is independent of this PR's main change but unblocks the cross-file test run. Live verification (same Langfuse instance as before): - Drove ``worker.run_agent`` against the real ``make_lead_agent`` + ``gpt-4o-mini`` for three distinct ``user_context`` identities (``fancy-engineer``, ``alice-pm``, ``bob-designer``). - Each run produced one ``lead-agent`` trace whose top-level ``sessionId`` / ``userId`` / ``tags`` carry the expected values, e.g. ``session=e2e-2930-8f347c-alice-pm user=alice-pm name='lead-agent' tags=['model:gpt-4o-mini']``. Refs #2930. * fix(tracing): extend root-callback + metadata injection to the embedded client Addresses Copilot review on PR #2944. Commit 2 disabled model-level tracing for ``TitleMiddleware`` and ``_create_summarization_middleware`` because ``_make_lead_agent`` now attaches the tracing callbacks at the graph invocation root. But the embedded ``DeerFlowClient`` does not call ``_make_lead_agent`` — it calls ``_build_middlewares`` directly and never appends the tracing handlers to its ``RunnableConfig``. So under the embedded path, title-generation and summarization LLM calls were left untraced — a regression introduced by this PR. This commit mirrors the gateway worker's injection in ``DeerFlowClient.stream``: - Append ``build_tracing_callbacks()`` to ``config["callbacks"]`` so the Langfuse handler sees ``on_chain_start(parent_run_id=None)`` at the graph root and runs the ``propagate_attributes`` path. - Merge ``build_langfuse_trace_metadata(...)`` into ``config["metadata"]`` with ``setdefault`` so caller-supplied keys still win. - ``_ensure_agent`` now creates its main model with ``attach_tracing=False`` to avoid duplicate spans now that the callback lives at the graph root. Docs: - ``backend/CLAUDE.md`` Tracing section rewritten to describe the graph-root attachment model (replacing the inaccurate "at model-creation time" wording). - ``README.md`` Langfuse section now lists both injection points (worker + client) instead of only the worker path. Tests: - ``tests/test_client_langfuse_metadata.py`` (new, 3 cases): callbacks + metadata are injected when Langfuse is enabled, caller-supplied metadata overrides win via ``setdefault``, and the injection is inert when Langfuse is disabled. Live verification on the real Langfuse instance: === user=fancy-client === id=cbd22847.. session=client-2930-6b9491-fancy-client user=fancy-client name='lead-agent' === user=alice-client === id=b4f6f576.. session=client-2930-6b9491-alice-client user=alice-client name='lead-agent' Refs #2930. * refactor(tracing): address maintainer review on PR #2944 Addresses @WillemJiang's 5 comments. 1. Duplicated metadata-injection code between worker.py and client.py New ``deerflow.tracing.inject_langfuse_metadata(config, ...)`` helper takes the 10-line build + merge + setdefault logic that was duplicated in ``runtime/runs/worker.py`` and ``client.py``. Both callers now share a single source of truth, so the two paths cannot drift. 2. Direct private-attribute mutation in conftest.py and tests Added public ``reset_tracing_config()`` / ``reset_title_config()`` functions. ``tests/conftest.py`` and every test that previously did ``tracing_module._tracing_config = None`` or ``title_module._title_config = TitleConfig()`` now goes through the public API. A future internal rename will surface as an ImportError instead of a silent no-op. 3. client.py reading os.environ directly ``DeerFlowClient.__init__`` grows an optional ``environment`` parameter so programmatic callers can pass the deployment label explicitly. ``stream()`` consults ``self._environment`` first and only falls back to ``DEER_FLOW_ENV`` / ``ENVIRONMENT`` env vars when nothing was passed in. Backwards compatible — env-var behaviour preserved for callers that opt to keep using it. 4. build_tracing_callbacks() cached on hot path Not implemented. Inspected the langfuse v4 ``langchain.CallbackHandler`` constructor: it only resolves the module-level singleton client via ``get_client()`` and initialises a few dicts (no I/O, no env parsing at construction time). The build is essentially free. Caching would trade a non-measurable speedup for two real risks: handler instances carry per-run state internally (``_run_states``, ``_root_run_states``, ``last_trace_id``), and tracing config can be reloaded by env-var changes between runs. Will revisit if profiling ever shows it as a hot spot. 5. attach_tracing=False easy to forget at new in-graph call sites - Module docstring at the top of ``lead_agent/agent.py`` documents the invariant ("every in-graph ``create_chat_model`` MUST pass ``attach_tracing=False``") and enumerates the current sites. - New regression test ``test_make_lead_agent_attaches_tracing_callbacks_at_graph_root`` in ``tests/test_lead_agent_model_resolution.py`` locks both halves of the invariant: ``config["callbacks"]`` carries the tracing handler after ``_make_lead_agent``, AND every ``create_chat_model`` call captured by the test passes ``attach_tracing=False``. A future in-graph site that forgets the flag will fail this test. Lint clean. Full touched-suite bundle: 246 passed. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>	2026-05-21 16:49:31 +08:00