Skip to content

GH-5971: Add failing regression test for streaming observation stop order#6317

Open
BK202503 wants to merge 1 commit into
spring-projects:mainfrom
BK202503:GH-5971-regression-test
Open

GH-5971: Add failing regression test for streaming observation stop order#6317
BK202503 wants to merge 1 commit into
spring-projects:mainfrom
BK202503:GH-5971-regression-test

Conversation

@BK202503

@BK202503 BK202503 commented Jun 6, 2026

Copy link
Copy Markdown

Adds a @Disabled regression test that encodes the expected observation lifecycle for #5971.

Background

Per #5971, ChatClient.prompt(...).stream() stops its observations in the wrong order: spring.ai.chat.client fires its .doFinally(... .stop()) before the inner spring.ai.advisor, instead of the LIFO order one would expect for nested observations.

What this PR adds

A new test in DefaultChatClientTests, streamingChatClientStopsObservationsInLifoOrder, that:

  • Registers a custom ObservationHandler on a TestObservationRegistry capturing every onStart and onStop event by name.
  • Runs ChatClient.prompt("hello").stream().chatResponse().blockLast() against a mocked ChatModel.
  • Asserts that both spring.ai.chat.client and spring.ai.advisor were started, and that spring.ai.advisor's stop event appears before spring.ai.chat.client's in the captured sequence.

Without the fix the test fails as expected:

[spring.ai.advisor must stop before spring.ai.chat.client]
Expecting actual:  1
to be less than:   0

i.e. the observed stopEvents is ["spring.ai.chat.client", "spring.ai.advisor", ...]chat.client stops at index 0 (first), advisor at index 1 (second).

Why @Disabled

The bug in #5971 has not yet been root-caused on main, so the test is kept @Disabled for now to avoid red CI. The @Disabled message references the issue and asks the next contributor who fixes the lifecycle to flip the annotation off.

Scope

  • One test class touched, one test method added (spring-ai-client-chat).
  • No production code changes.
  • No new dependencies.
  • The reverse-order assertion was confirmed to fail locally on main (commit at HEAD when the test was written) using the ChatModel mock pattern already established in this file.

Follow-ups

The other half of #5971spring.ai.tool observations getting null parent in streaming — is not covered by this PR. The tool-call path involves a Schedulers.boundedElastic() boundary inside ToolCallingAdvisor.handleToolCallRecursion, and the Reactor context that ToolCallReactiveContextHolder reads from there appears not to carry the parent observation that the call (blocking) path inherits via Observation.observe(...). Happy to fold a second @Disabled test for that on top of this PR if maintainers prefer one PR over two.

…ervation stop order)

Encodes the expected lifecycle described in spring-projects#5971 as an assertion that
under streaming `spring.ai.advisor` must stop before its parent
`spring.ai.chat.client` (LIFO). The test captures observation start/stop
events through a custom `ObservationHandler` registered on a
`TestObservationRegistry` and is currently `@Disabled` because the bug
in spring-projects#5971 makes it fail with the inverse order:

  [spring.ai.advisor must stop before spring.ai.chat.client]
  Expecting actual:  1
  to be less than:   0

`ChatClient.prompt(...).stream()` produces:

  stop spring.ai.chat.client   <- outer fires first under the bug
  stop spring.ai.advisor       <- inner fires second

while the inverse order is expected (`.doFinally` callbacks fire from
inner to outer along the operator chain). Once the lifecycle is fixed
the `@Disabled` can be removed and the test serves as a regression
guard.

Signed-off-by: BK202503 <199436087+BK202503@users.noreply.github.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.

1 participant