Feat/traces logs enhance - #1
Open
AnoobFeng wants to merge 4 commits into
Open
Conversation
- 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
- 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
github-actions
Bot
added
area:backend
area:agents
area:docs
size/XL
risk:high
needs-validation
labels
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.
AnoobFeng
pushed a commit
that referenced
this pull request
Jul 27, 2026
…etries (bytedance#4294) * Create a feature of Process-global LLM concurrency cap * Added configuration of llm_call of max_concurrent_calls * Classify limit_burst_rate and expose retry params via config.yaml * refactor(middleware): encapsulate LLM concurrency state in a dataclass Address PR bytedance#4294 review feedback (github-code-quality bot): the bare module-level globals _GLOBAL_CONCURRENCY_LOOP / _GLOBAL_CONCURRENCY_LIMIT were flagged as unused - a false positive, since both are read on the recreate condition, but the `global`-declaration pattern tripped the analyzer. Replace the three globals + `global` declaration with a single _ConcurrencyState dataclass singleton mutated in place. Behavior is unchanged (lazy recreate when the running loop or configured limit changes); the state is now co-located and no longer relies on bare globals. dataclasses is already an established harness convention. Co-Authored-By: Claude <noreply@anthropic.com> * fix(middleware): make LLM concurrency limiter process-wide + jitter burst retry Addresses PR bytedance#4294 review (fancyboi999, CHANGES_REQUESTED) - two P1 issues. P1 #1: the asyncio.Semaphore limiter was loop-bound, so it recreated per event loop and the cap was NOT process-wide: lead-agent calls (main loop) and subagent calls (the isolated persistent loop in subagents/executor.py) each got their own semaphore, and the sync graph path (wrap_model_call) bypassed the cap entirely. Recreating on loop/limit change also abandoned permits held by the prior instance. Replace it with a _ProcessWideLimiter built on threading primitives (not loop-bound): one limiter shared across every event loop and both sync/async wrappers. The cap is mutable via set_limit (never recreates, so in-flight permits are never abandoned); permits release in finally and async waiters unregister on cancellation, so cancellation never leaks capacity. Wire it into wrap_model_call (sync) too - previously a direct handler() call. P1 #2: the first (and only) burst-rate retry was deterministic at 5000ms. prev_delay_ms was seeded from the 1000ms normal base, so for burst the window collapsed to randint(5000, max(5000, 1000*3)) = randint(5000, 5000) - a fleet that failed together realigned on the same 5s tick. Seed the first retry from the reason-specific base (prev_delay_ms=None on loop init) so the burst window is [burst_base, cap] = [5000, 8000], non-degenerate. Retry-After is still honored verbatim. Tests: rename semaphore tests -> limiter; add an autouse fixture resetting the process singleton; add regressions the reviewer asked for - cross-loop (lead + isolated-loop subagent), two concurrent sync calls, limit-change while a permit is held (same instance, permit preserved), cancellation no-leak, and burst first-retry non-degeneracy with default config (real and seeded RNG) plus a concurrent de-synchronization case. Verified the burst guard goes red on the old logic ({5000}) and green on the new. Co-Authored-By: Claude <noreply@anthropic.com> * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * Potential fix for pull request finding 'Statement has no effect' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * fix(middleware): lossless limiter handoff, generation-aware cap, burst-rate CB gate Address the open P1/P2 review findings on bytedance#4294: - P1 #1 (cancellation handoff): reserve the permit for a specific waiter at dequeue time (grant-at-dequeue, _AsyncWaiter.granted) so a waiter cancelled in the post-dequeue / pre-reacquire window hands its reservation to the next waiter (_handoff_granted_permit_locked) instead of stranding it. No cancellation window remains. - P1 #2 (hot-reload generation): move cap updates out of the per-attempt path; give the limiter one generation-aware owner (set_limit_if_newer with a monotonic instance seq proxy for config freshness) so a stale in-flight run cannot rewrite a freshly-lowered cap. max_concurrent_calls is now genuinely hot-reloadable, resolving the reload-boundary inconsistency by option (b) - no STARTUP_ONLY_FIELDS change (retry params truly hot-reload). - P2 (circuit breaker): gate _record_failure on reason != "burst_rate" so burst-rate (limit_burst_rate) exhaustion - a transient slope-throttle, not "provider down" - does not trip the CB and fast-fail ALL calls. - P3: clamp the jitter window to the cap before drawing (uniform spread instead of piling at the cap); document the per-process / GATEWAY_WORKERS cap semantics in config + the field description. Tests: add the reviewer-requested regressions (cancel-after-dequeue handoff; stale-instance-doesn't-overwrite-lowered-cap across sync + isolated-loop async; burst_rate-exhaustion-doesn't-trip-CB sync + async). Each is red on the prior buggy logic and green on the fix. _build_middleware now routes llm_call knobs through AppConfig so __init__ applies the cap. 71 middleware tests pass; 212 across the blast radius (1 pre-existing skip). Co-Authored-By: Claude <noreply@anthropic.com> * fix(middleware): startup-only LLM concurrency cap; report effective retry budget Addresses review feedback on bytedance#4294 (fancyboi999 CHANGES_REQUESTED on acfc761): P1 - the generation guard measured construction order, not config freshness, so a stale AppConfig(cap=3) constructed after a fresher AppConfig(cap=1) could restore the higher cap; and on a downscale 3->1 release() handed excess permits to queued waiters, keeping in_flight pegged at the old cap. Replace the pseudo-generation path with a startup-only cap: the first middleware __init__ resolves and freezes the cap; later instances (newer or older config) are no-ops. No runtime cap mutation means no downscale race and no freshness/construction-order race. Per-call gate is now `limiter is None` only, so a reloaded instance with max_concurrent_calls=0 cannot silently drop the frozen cap. Removes _owner_seq / set_limit_if_newer / _grant_to_queued_locked / _next_instance_seq; file 946 -> 926 lines. P2 - burst-rate calls are capped at 2 attempts but the retry log line, the llm_retry stream event max_attempts, and the user-facing message still used self.retry_max_attempts (3), so the frontend showed 1/3 then stopped after attempt 2. Thread the effective max_attempts (_max_attempts_for) through the logger, _emit_retry_event, and _build_retry_message. Also: document max_concurrent_calls as startup-only in the config field description and config.example.yaml (prose only - the startup-only: prefix is top-level AppConfig-field granularity and would mislabel the otherwise hot-reloadable llm_call section / break the reload_boundary drift test). Rewrite the cap-mutation tests for startup-only semantics; add P2 retry-budget event tests (sync+async, teeth-verified red on the bug); fix bot nits (empty except blocks -> gather(return_exceptions=True); bare await statements -> assigned+asserted). Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
AnoobFeng
pushed a commit
that referenced
this pull request
Jul 27, 2026
bytedance#4174) * feat(uploads): lazy-load historical files via list_uploaded_files tool Replace per-turn injection of all historical upload metadata with on-demand discovery via a new `list_uploaded_files` built-in tool, following the same deferred-discovery pattern used by skills. - Rename <uploaded_files> block to <current_uploads> (current-run files only) - Add list_uploaded_files tool with include_outline: bool|list[str] - Extract outline helpers to shared deerflow/utils/file_outline.py - Update system prompt to reflect lazy-loading behaviour - Historical file scan removed from UploadsMiddleware.before_agent() Co-Authored-By: Claude <noreply@anthropic.com> * fix(uploads): clear uploaded_files state when no new files in current turn When before_agent() returns None on empty turns, the LastValue uploaded_files field retains the previous turn's filenames. list_uploaded_files then incorrectly excludes those files as "current-run" files, making them invisible until the next upload. Fix: return {"uploaded_files": []} instead of None to explicitly clear state. Add two-turn regression test covering the exact scenario from review feedback. Co-Authored-By: Claude <noreply@anthropic.com> * fix: resolve CI lint errors and stale test assertion from merge - Split long prompt line to fit 240-char limit - Add missing `Any` import in list_uploaded_files_tool - Remove unused `re` import in file_conversion (outline code moved) - Remove unused `os` import in middleware test - Fix test assertion: <uploaded_files> → <current_uploads> after main merge Co-Authored-By: Claude <noreply@anthropic.com> * fix: resolve CI lint errors and stale test assertion from merge - Split long prompt line to fit 240-char limit - Add missing `Any` import in list_uploaded_files_tool - Remove unused `re` import in file_conversion (outline code moved) - Remove unused `os` import in middleware test - Fix test assertion: <uploaded_files> → <current_uploads> after main merge Co-Authored-By: Claude <noreply@anthropic.com> * fix: add current_uploads to input sanitization exempt tags The lazy-loading PR renamed <uploaded_files> to <current_uploads>. The anti-drift guard scans all framework XML blocks and requires each to be either blocked or explicitly exempted. current_uploads wraps trusted server-generated file metadata, not user input, so it belongs in the exempt set. Co-Authored-By: Claude <noreply@anthropic.com> * test: regenerate replay golden after uploaded_files state change before_agent now returns {"uploaded_files": []} instead of None, adding uploaded_files to SSE values events. Regenerated via DEERFLOW_WRITE_GOLDEN=1. Co-Authored-By: Claude <noreply@anthropic.com> * fix: review feedback — memory pipeline, stale tags, state clearing, nits - Match both tags in memory stripping pipeline (uploaded_files|current_uploads) - Remove stale uploaded_files from _BLOCKED_TAG_NAMES - Clear uploaded_files on all before_agent early-return paths - Fix ponytail: stray word in file_conversion re-export comment - Remove dead total_omitted branch in _format_omitted_summary - ruff format fixes Co-Authored-By: Claude <noreply@anthropic.com> * fix: block current_uploads, sanitize only original user content Per review feedback: instead of exempting <current_uploads> (which allows user forgery), move it to _BLOCKED_TAG_NAMES and change InputSanitizationMiddleware._process_request to scan only the original user content (ORIGINAL_USER_CONTENT_KEY) when available. Server-injected trusted blocks are no longer checked against the blocked-tag denylist. Co-Authored-By: Claude <noreply@anthropic.com> * docs: clarify fallback reason in input sanitization comment Co-Authored-By: Claude <noreply@anthropic.com> * @ fix: third-round review feedback — state visibility, sanitization, regex, nits - list_uploaded_files_tool: logger.warning instead of silent try/except on runtime.state read failure (High) - input_sanitization_middleware: _extract_text_from_content skips empty text blocks to match message_content_to_text behaviour; rfind fallback path logs warning for observability (Medium) - memory pipeline regexes: backreference (?P<tag>)(?P=tag) in message_processing.py and prompt.py (Low) - file_conversion.py: re-export moved to top of file (Low) - Tests: middleware→tool state bridge test; integrated forged-tag + multimodal sanitization tests PR bytedance#4174 — Follow-up issues: bytedance#4212, bytedance#4213, bytedance#4214 Co-Authored-By: Claude <noreply@anthropic.com> @ * @ fix: 4th-round review — denylist, sanitization, scandir, nits - Add "uploaded_files" back to _BLOCKED_TAG_NAMES (old tag still processed by deermem; user forgery must be escaped) (consistency) - Fix inaccurate rfind-fallback comment: UploadsMiddleware keeps string as string, fallback is unreachable for strings (doc fix) - Distinguish "empty string key" (upload without text) from "non-string key" (caller forgery) so empty-text uploads never escape the server block (edge) - Merge dual os.scandir(uploads_dir) calls into one list re-use (minor) - Add comment on .md sibling skip known limitation: user-uploaded .md files whose stem collides with a converted doc are hidden (boundary, no code change) Co-Authored-By: Claude <noreply@anthropic.com> @ * @ fix: tighten rfind-failure fallback — distinguish server blocks from user blocks When _extract_text_from_content and message_content_to_text disagree on multimodal list content and rfind fails, use content[0] (server-injected <current_uploads> block) vs content[1:] (user blocks) to sanitize only user blocks. Raw strings and non-standard dict blocks that _extract_text_from_content misses are now also sanitized. Non-distinguishable paths (< 2 text blocks, non-list content) still degrade to full sanitization (safe — server block may be escaped but user forgery never leaks). All fallback paths log via logger.warning. Decision 18 / willem-bd 4th-round comment #3 Co-Authored-By: Claude <noreply@anthropic.com> @ * @ fix: correct comments referencing text_blocks → content in rfind fallback Co-Authored-By: Claude <noreply@anthropic.com> @ * fix: 5th-round review — dead code, subagent gating, integration test, perf, consistency - Delete unreachable ORIGINAL_USER_CONTENT_KEY guard in rfind fallback branch (original_user_content guaranteed non-empty str at that point) - Remove list_uploaded_files from BUILTIN_TOOLS; add include_upload_tool param to get_available_tools(), default True; task_tool.py passes False so subagents no longer receive a tool whose state exclusion is broken - Add integration test exercising real create_agent graph (not mocked runtime.state) to verify LangGraph propagates before_agent state writes into ToolRuntime.state during same-turn tool calls - Cache DirEntry.stat() st_size in candidates tuple to avoid second per-file syscall in the rendering loop - Make the upload-tag pre-check case-insensitive (content_str.lower()) to match _UPLOAD_BLOCK_RE re.IGNORECASE PR bytedance#4174 — willem-bd 5th-round review items #1-bytedance#5 Co-Authored-By: Claude <noreply@anthropic.com> * fix(channels): pass files metadata through _human_input_message() for IM uploads _human_input_message() was not passing additional_kwargs.files to the downstream message. UploadsMiddleware read no files, wrote uploaded_files=[], and list_uploaded_files reported same-run IM attachments as historical files (fancyboi999 repro). Fix: add files parameter to _human_input_message(), call site passes files=uploaded. Regression test locks the contract. Co-Authored-By: Claude <noreply@anthropic.com> * fix(channels): remove legacy <uploaded_files> manual prepend to fix double-injection regression Commit 8d86dbf added files= pass-through to UploadsMiddleware but left the manual _format_uploaded_files_block() prepend in place. Every IM attachment reached the model twice — once via the legacy <uploaded_files> block and once via <current_uploads>. This commit removes the manual prepend and the now-dead _format_uploaded_files_block() function. UploadsMiddleware is the sole upload-context producer for both IM and web paths. Reported-by: fancyboi999 (PR review) Co-Authored-By: Claude <noreply@anthropic.com> * docs: update bytedance#4212 issue body to reflect completed fixes and narrowed remaining scope * chore: remove temporary scratch file * fix(middleware): neutralize user-derived values inside <current_uploads> block Upload-derived filenames, paths, outline titles, and preview text are interpolated verbatim inside the trusted <current_uploads> wrapper, which InputSanitizationMiddleware exempts from sanitization. A crafted filename or document heading containing blocked authority tags would bypass the guardrail and enter model context as trusted framework data. Fix: call neutralize_untrusted_tags() on all four user-derived values inside _format_file_entry(), preserving the outer <current_uploads> wrapper untouched. Reported-by: fancyboi999 (P1 security review) Co-Authored-By: Claude <noreply@anthropic.com> * fix(middleware): neutralize extension labels in omitted-file summary Files exceeding the 10-item context cap bypass _format_file_entry(). Their extensions, derived from user-controlled filenames via _extension_label(), were interpolated verbatim into the trusted <current_uploads> wrapper — another path for blocked authority tags to escape the guardrail. Fix: neutralize extension values inside _extension_label(), the single extraction point for all extension labels. Reported-by: fancyboi999 (P1 security review) Co-Authored-By: Claude <noreply@anthropic.com> * fix(tools): neutralize user-derived values in list_uploaded_files tool result Apply neutralize_untrusted_tags() to every model-visible user-derived value returned by list_uploaded_files: filename, virtual path, extension, outline titles, outline preview lines, and omitted-file extension summary. This closes the last remaining injection bypass in the upload lazy-loading path - the <current_uploads> block and its omitted summary were already neutralized (previous commits), but the list_uploaded_files tool produced a second exit for the same attacker-controlled metadata that ToolResultSanitizationMiddleware did not cover. Co-Authored-By: Claude <noreply@anthropic.com> * fix(tests): add missing include_upload_tool=False to task_tool mock assertions PR bytedance#4174 added include_upload_tool parameter to get_available_tools(). task_tool.py correctly passes include_upload_tool=False for subagents but 5 existing tests' assert_called_once_with expectations were not updated, causing CI failures. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #
Why
What changed
Surface area
frontend/backend/applanggraph.json, or prompt changedocker/or sandboxed executionskills/backend/pyproject.tomlorfrontend/package.json(say what it buys us)Screenshots / Recording
Bug fix verification
Validation
AI assistance
Tool(s) used:
How you used it: