Skip to content

fix(adk): Always inject headers on execute activity#337

Merged
declan-scale merged 1 commit intonextfrom
declan-scale/update-context-interceptor
Apr 30, 2026
Merged

fix(adk): Always inject headers on execute activity#337
declan-scale merged 1 commit intonextfrom
declan-scale/update-context-interceptor

Conversation

@declan-scale
Copy link
Copy Markdown
Contributor

@declan-scale declan-scale commented Apr 30, 2026

Greptile Summary

This PR removes the activity-name allowlist from ContextWorkflowOutboundInterceptor.start_activity, so task_id, trace_id, and parent_span_id headers are now injected into every outbound activity, not just invoke_model_activity / run_claude_agent_activity. This fixes cases where other activities in a workflow needed the same context propagation.

Confidence Score: 5/5

Safe to merge; the only finding is a minor log-level concern that does not affect correctness.

The change is a straightforward removal of an overly-narrow allowlist. The core propagation logic is unchanged and correct. The single finding is a P2 warning-log noise issue that doesn't impact behavior or data integrity.

No files require special attention.

Important Files Changed

Filename Overview
src/agentex/lib/core/temporal/plugins/openai_agents/interceptors/context_interceptor.py Removes the activity-name allowlist filter so that task_id/trace_id/parent_span_id headers are injected on every outbound activity start, not just named model activities. The logic itself is correct, but the warning log that fires when context attributes are absent will now trigger for all non-model workflows, potentially causing log noise.

Fix All in Cursor Fix All in Claude Code Fix All in Codex

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
src/agentex/lib/core/temporal/plugins/openai_agents/interceptors/context_interceptor.py:103
**Warning log noise for non-context workflows**

Now that headers are injected for _all_ activities (not just model activities), workflows that legitimately have no `_task_id`/`_trace_id`/`_parent_span_id` will emit a `WARNING` for every single activity they start. This can flood logs with spurious warnings that aren't actionable. Consider downgrading this branch to `DEBUG` since "no context found" is now expected for non-model workflows.

Reviews (2): Last reviewed commit: "fix(adk): Always inject headers on execu..." | Re-trigger Greptile

@declan-scale declan-scale force-pushed the declan-scale/update-context-interceptor branch from ddcb12c to d52c73f Compare April 30, 2026 19:40
@declan-scale declan-scale merged commit 9d80e0b into next Apr 30, 2026
9 checks passed
@declan-scale declan-scale deleted the declan-scale/update-context-interceptor branch April 30, 2026 19:59
@stainless-app stainless-app Bot mentioned this pull request Apr 30, 2026
declan-scale added a commit that referenced this pull request Apr 30, 2026
* feat(api): api update

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* chore(internal): more robust bootstrap script

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* fix: use correct field name format for multipart file arrays

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* feat: support setting headers via env

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* codegen metadata

* fix: allow litellm security patch (#336)

* fix(adk): Always inject headers on execute activity (#337)

* perf(streaming): coalesce per-token publishes to Redis (50ms / 128-char window) (#333)

* perf(streaming): coalesce per-token publishes to Redis (50ms / 128-char window)

Per-token Redis publishes from TemporalStreamingModel were adding ~45s
(56-62%) overhead to agent response latency, mostly from head-of-line
blocking on the model's event loop: each `await streaming_context.stream_update(...)`
inside the OpenAI stream `async for` paused token consumption until the
publish round-trip completed.

This change introduces a `CoalescingBuffer` driven by an `asyncio.Event`,
so the producer never awaits on Redis. Deltas are merged consecutive-only
(preserving character order in every (type, index) channel) and flushed
on a 50ms timer, on a 128-char size threshold, or immediately for the
first delta to keep perceived responsiveness high. The buffer's `close()`
drains remaining deltas before the DONE event, so consumers see the full
sequence in order.

A new `StreamingMode = Literal["off", "per_token", "coalesced"]` lives
in `streaming.py` as the single source of truth and is plumbed through
the adk streaming module, `StreamingService.streaming_task_message_context`,
and `StreamingTaskMessageContext`. Default is `"coalesced"` everywhere,
so all 13+ existing context callers (claude_agents, langgraph, litellm
provider, openai sync provider, etc.) benefit automatically.

* chore(streaming): fix import ordering (ruff I001)

* fix(streaming): address greptile review findings

- _run: when CancelledError is raised mid-flush in the for-loop, re-enqueue
  the in-flight item plus any remaining items in the local `drained` list
  back into self._buf so close()'s final drain can recover them. Previously
  the local `drained` list was unreachable after CancelledError exited the
  for-loop, causing the last coalesced batch to be silently dropped on
  close-during-flush races. Trade-off: the in-flight item may be duplicated
  on the consumer side (Redis pub may have completed before cancel was
  delivered), which is preferable to silent loss for streaming UX.

- _merge_pair: replace `return b` fallback with AssertionError. All six
  current TaskMessageDelta variants have explicit isinstance branches, so
  the fallback is unreachable today. But _can_merge returns True for any
  same-type pair, so adding a 7th delta variant without updating
  _merge_pair would silently drop `a`'s accumulated content. Asserting
  turns a future silent data-loss into an immediate, diagnosable crash.

* test(streaming): add coalescing-layer tests; loosen one model assertion

After merging the test-suite repair from main (#334) into this branch, one
model test (test_responses_api_streaming) regressed because its
assert_called_with strict-matched all kwargs of streaming_task_message_context
and didn't tolerate the new `streaming_mode='coalesced'` kwarg this PR
adds. Switched to assert_called() + targeted kwarg checks so the test
verifies what it cares about (task_id threading) without locking in
implementation details.

Replaced the ad-hoc smoke scripts that lived in conversation with a real
pytest module at tests/lib/core/services/adk/test_streaming.py covering:

- _delta_char_len, _can_merge, _merge_pair: per-channel correctness +
  None-handling
- _merge_consecutive: pure-text collapse, cross-channel order preservation,
  per-channel reconstruction matches per-token semantics
- CoalescingBuffer: first-delta-immediate flush within ~20ms,
  size-threshold flush before timer fires, multi-delta coalescing within
  one window, idle close, add-after-close no-op
- CoalescingBuffer cancel-during-flush regression test for the P1 fix:
  five queued chunks must all surface across publishes when close()
  cancels mid-flush (asserts substring presence rather than exact
  ordering, since the documented trade-off allows duplicates of the
  in-flight item)
- StreamingTaskMessageContext mode dispatch: "off" suppresses publishes
  but persists full content, "per_token" publishes each delta synchronously,
  "coalesced" batches and persists full content

* chore(streaming): route TemporalStreamingModel logger through make_logger

The model file used raw ``logging.getLogger("agentex.temporal.streaming")``,
which returns a logger with no handler attached and no level configured —
so the existing ``[TemporalStreamingModel] Initialized ... streaming_mode=...``
INFO log was silently dropped, making it impossible to verify at runtime
that a coalesced (or any) streaming mode was actually wired.

Switch to the SDK's ``make_logger`` helper (level=INFO, RichHandler in
local mode, StreamHandler otherwise) used everywhere else in the SDK.
The explicit logger name ``agentex.temporal.streaming`` is preserved so
any external logging configuration targeting that name keeps working.

* codegen metadata

* feat(api): api update

* release: 0.10.3

---------

Co-authored-by: stainless-app[bot] <142633134+stainless-app[bot]@users.noreply.github.com>
Co-authored-by: Brandon Allen <brandon.allen@scale.com>
Co-authored-by: Declan Brady <declan.brady@scale.com>
Co-authored-by: Stas Moreinis <stas.moreinis@scale.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants