Skip to content

Commit 3e43a7d

Browse files
temporal-spring-ai: reframe replay-test messages around plugin behavior
Reviewer pointed out that "ChatModel must not be re-invoked during replay" reads as testing Temporal's activity replay guarantee rather than the plugin's behavior. Reword class-level javadoc and assertion messages so the property under test is explicit: the plugin places ChatModel calls, activity-stub tool calls, and @SideEffectTool bodies behind the correct Temporal boundary. Temporal's replay/memoization semantics are assumed correct. The counter-stays-at-1 observation is the signal we use to detect a plugin regression, not the thing being tested. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent f6b29d0 commit 3e43a7d

3 files changed

Lines changed: 45 additions & 24 deletions

File tree

temporal-spring-ai/src/test/java/io/temporal/springai/replay/ActivityToolSideEffectTest.java

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,15 @@
3535
import org.springframework.ai.tool.annotation.Tool;
3636

3737
/**
38-
* Asserts that a workflow replay does not re-invoke activity-backed tools. The {@link AddActivity}
39-
* impl holds a counter that increments on each tool call. After the initial run the counter is 1;
40-
* after replaying the captured history, it must still be 1 — activity results for the tool call
41-
* come from history, not from re-invoking the activity impl.
38+
* Asserts that {@link TemporalChatClient#defaultTools} routes activity-stub tools through the
39+
* activity boundary the user already set up (via {@code Workflow.newActivityStub}) rather than
40+
* invoking the underlying impl directly from workflow code. This is a plugin property: the plugin
41+
* must recognize the stub as an activity-backed tool and invoke it as such, not unwrap it into a
42+
* plain in-workflow Java method call. Temporal's replay semantics for activities are assumed
43+
* correct.
44+
*
45+
* <p>If the plugin regressed to invoking a stub's backing impl directly, replay would re-run that
46+
* call and the counter would exceed 1.
4247
*/
4348
class ActivityToolSideEffectTest {
4449

@@ -82,7 +87,9 @@ void activityTool_notReInvokedOnReplay() throws Exception {
8287
WorkflowOptions.newBuilder().setTaskQueue(TASK_QUEUE).build());
8388
assertEquals("The answer is 5", workflow.chat("What is 2+3?"));
8489
assertEquals(
85-
1, addActivity.callCount.get(), "Tool activity should run once during the initial run");
90+
1,
91+
addActivity.callCount.get(),
92+
"sanity check: the tool activity impl ran exactly once for one workflow invocation");
8693

8794
WorkflowExecutionHistory history =
8895
client.fetchHistory(WorkflowStub.fromTyped(workflow).getExecution().getWorkflowId());
@@ -91,7 +98,9 @@ void activityTool_notReInvokedOnReplay() throws Exception {
9198
assertEquals(
9299
1,
93100
addActivity.callCount.get(),
94-
"Tool activity must not be re-invoked during replay — results come from history");
101+
"TemporalChatClient must invoke activity-stub tools as activities; a counter above 1"
102+
+ " means the plugin unwrapped the stub and called the impl directly from workflow"
103+
+ " code");
95104
}
96105

97106
@WorkflowInterface

temporal-spring-ai/src/test/java/io/temporal/springai/replay/ChatModelSideEffectTest.java

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,17 @@
2828
import org.springframework.ai.chat.prompt.Prompt;
2929

3030
/**
31-
* Asserts that a workflow replay does not re-invoke the underlying {@link ChatModel}. The counter
32-
* lives on the activity's backing ChatModel, which is only reached when the {@code CallChatModel}
33-
* activity is scheduled by the workflow. On replay, the activity result is fetched from history;
34-
* the impl is not re-invoked. If we ever regress by dropping that guarantee — say by adding an
35-
* in-workflow cache that falls back to invoking the model directly — the counter will advance to 2
36-
* and this test will fail.
31+
* Asserts that {@link ActivityChatModel} routes every {@link ChatModel} invocation through an
32+
* activity boundary rather than calling the underlying {@code ChatModel} directly from workflow
33+
* code. This is a property of the plugin, not of Temporal: Temporal guarantees activity results are
34+
* replayed from history, but that only helps if the plugin actually scheduled an activity in the
35+
* first place.
36+
*
37+
* <p>Concretely, if a regression routed chat calls directly (e.g. an in-workflow cache whose miss
38+
* path invokes the {@code ChatModel} inline), replay would re-run that inline call and the counter
39+
* would advance past 1. Caching is disabled in {@link #setUp()} so the worker replays on every
40+
* workflow task — making this failure mode observable during the initial run, not only via the
41+
* explicit {@code WorkflowReplayer} pass.
3742
*/
3843
class ChatModelSideEffectTest {
3944

@@ -76,7 +81,9 @@ void chatModel_notReInvokedOnReplay() throws Exception {
7681
ChatWorkflow.class, WorkflowOptions.newBuilder().setTaskQueue(TASK_QUEUE).build());
7782
assertEquals("pong", workflow.chat("ping"));
7883
assertEquals(
79-
1, model.callCount.get(), "ChatModel should be called once during the initial run");
84+
1,
85+
model.callCount.get(),
86+
"sanity check: the ChatModel ran exactly once for one workflow invocation");
8087

8188
WorkflowExecutionHistory history =
8289
client.fetchHistory(WorkflowStub.fromTyped(workflow).getExecution().getWorkflowId());
@@ -85,7 +92,9 @@ void chatModel_notReInvokedOnReplay() throws Exception {
8592
assertEquals(
8693
1,
8794
model.callCount.get(),
88-
"ChatModel must not be re-invoked during replay — activity results come from history");
95+
"ActivityChatModel must place ChatModel calls behind an activity boundary; a counter"
96+
+ " above 1 means the plugin invoked the ChatModel directly from workflow code"
97+
+ " and replay re-ran it");
8998
}
9099

91100
@WorkflowInterface

temporal-spring-ai/src/test/java/io/temporal/springai/replay/SideEffectToolReplayTest.java

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,15 @@
3131
import org.springframework.ai.tool.annotation.Tool;
3232

3333
/**
34-
* Asserts that {@code Workflow.sideEffect(...)} memoization works for {@link SideEffectTool}
35-
* bodies. Unlike activity-backed tools, the {@code SideEffectToolCallback} wrapper runs on the
36-
* workflow side, so <em>every</em> workflow statement re-runs during replay — including the call
37-
* into {@code SideEffectToolCallback.call()}. What must NOT re-run is the inner tool body (the
38-
* lambda passed to {@code Workflow.sideEffect}); that result is fetched from the recorded marker.
34+
* Asserts that the plugin's {@link SideEffectTool} callback actually wraps the user's {@code @Tool}
35+
* method body in {@code Workflow.sideEffect(...)}. This is a plugin property: the {@code
36+
* SideEffectToolCallback} itself runs on the workflow side and gets re-entered on every replay, so
37+
* if the callback were to invoke the tool body directly instead of via {@code Workflow.sideEffect},
38+
* the body would run again on each replay. Temporal's memoization of {@code sideEffect} markers is
39+
* assumed correct.
3940
*
40-
* <p>A regression that dropped the sideEffect wrapper (e.g., directly invoking the delegate) would
41-
* bump this counter to 2 on replay.
41+
* <p>A regression that drops the {@code Workflow.sideEffect} wrap (calling the delegate directly)
42+
* would bump this counter past 1.
4243
*/
4344
class SideEffectToolReplayTest {
4445

@@ -83,7 +84,9 @@ void sideEffectTool_notReInvokedOnReplay() throws Exception {
8384
TimestampWorkflow.class, WorkflowOptions.newBuilder().setTaskQueue(TASK_QUEUE).build());
8485
assertEquals("got: 2026-04-21T00:00:00Z", workflow.chat("what time is it?"));
8586
assertEquals(
86-
1, CALL_COUNT.get(), "@SideEffectTool body should run once during the initial run");
87+
1,
88+
CALL_COUNT.get(),
89+
"sanity check: the @SideEffectTool body ran exactly once for one workflow invocation");
8790

8891
WorkflowExecutionHistory history =
8992
client.fetchHistory(WorkflowStub.fromTyped(workflow).getExecution().getWorkflowId());
@@ -92,8 +95,8 @@ void sideEffectTool_notReInvokedOnReplay() throws Exception {
9295
assertEquals(
9396
1,
9497
CALL_COUNT.get(),
95-
"@SideEffectTool body must not re-run on replay — result comes from the recorded"
96-
+ " sideEffect marker");
98+
"SideEffectToolCallback must wrap the @Tool body in Workflow.sideEffect; a counter above"
99+
+ " 1 means the plugin invoked the tool body directly and replay re-ran it");
97100
}
98101

99102
@WorkflowInterface

0 commit comments

Comments
 (0)