Skip to content

Commit 607bd03

Browse files
temporal-spring-ai: make per-model options compositional via 'default' catch-all
Address reviewer feedback on #2855: the three-way implicit fallback (specific entry → library defaults) was ambiguous to readers of forDefault() / forModel(name) — nothing locally tells the caller which rung they land on. But a strict either-or (global OR exhaustive per-model) breaks compositionality when a third-party starter contributes a ChatModel bean your config didn't enumerate. Resolution: keep per-model overrides, and reserve ChatModelTypes.DEFAULT_MODEL_NAME as a user-declared catch-all key in perModelOptions. Lookup is now map[name] ?? map["default"] ?? library defaults. SpringAiPlugin validates that every perModelOptions key either matches a registered ChatModel bean or equals the catch-all name; typos fail at plugin construction, not silently at call time. Updates javadoc on SpringAiPlugin, ActivityChatModel.forModel(String), and ChatModelActivityOptions. Collapses the duplicate "Activity options" README section into one with an example of the catch-all pattern. Adds tests for the catch-all lookup, specific-wins-over- catch-all, and typo-key rejection. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 1cd021a commit 607bd03

6 files changed

Lines changed: 141 additions & 22 deletions

File tree

temporal-spring-ai/README.md

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,22 @@ ActivityChatModel chatModel = ActivityChatModel.forDefault(
6464
.build());
6565
```
6666

67+
For configuration-driven per-model overrides, declare a `ChatModelActivityOptions` bean and auto-configuration wires the map into the plugin. A key equal to `ChatModelTypes.DEFAULT_MODEL_NAME` (the literal `"default"`) acts as a global catch-all: any chat model that lacks a bean-name-specific entry — including models contributed by third-party starters that your application did not declare directly — picks up that entry.
68+
69+
```java
70+
@Bean
71+
ChatModelActivityOptions chatModelActivityOptions() {
72+
ActivityOptions fiveMinute = ActivityOptions.newBuilder(ActivityChatModel.defaultActivityOptions())
73+
.setStartToCloseTimeout(Duration.ofMinutes(5))
74+
.build();
75+
return new ChatModelActivityOptions(Map.of(
76+
ChatModelTypes.DEFAULT_MODEL_NAME, fiveMinute, // global baseline
77+
"claude", ActivityOptions.newBuilder(fiveMinute).setTaskQueue("claude-heavy").build())); // override
78+
}
79+
```
80+
81+
Keys that neither match a registered ChatModel bean name nor equal `"default"` cause plugin construction to fail, so a typo surfaces at startup.
82+
6783
`ActivityMcpClient.create()` / `create(ActivityOptions)` work the same way with a 30-second default timeout.
6884

6985
The Temporal UI labels chat and MCP rows with a short Summary (`chat: <model>`, `mcp: <client>.<tool>`). `ActivityChatModel` and `ActivityMcpClient` are constructed only via these factories — there is no public constructor, so users can't accidentally end up in a code path that skips UI labels. Prompt text is deliberately not included in chat summaries to avoid leaking user input (which may contain PII, credentials, or other sensitive data) into workflow history and server logs.
@@ -187,24 +203,6 @@ Raw `byte[]` media gets serialized into every chat activity's input *and* result
187203

188204
Override the cap by setting the system property `io.temporal.springai.maxMediaBytes` before your worker starts (pass a positive integer; `0` disables the check). For anything larger than a small thumbnail, the URI route is the right answer — have an activity write the bytes to blob storage, then pass only the URL into the conversation.
189205

190-
## Activity options and retry behavior
191-
192-
`ActivityChatModel.forDefault()` and `ActivityChatModel.forModel(name)` create the chat activity stub with sensible defaults: a 2-minute start-to-close timeout, 3 attempts, and `org.springframework.ai.retry.NonTransientAiException` + `java.lang.IllegalArgumentException` classified as non-retryable so a bad API key or invalid prompt fails fast.
193-
194-
Override with `ActivityChatModel.forModel(name, ActivityOptions)`:
195-
196-
```java
197-
ActivityOptions opts = ActivityOptions.newBuilder(ActivityChatModel.defaultActivityOptions())
198-
.setStartToCloseTimeout(Duration.ofMinutes(10))
199-
.setTaskQueue("reasoning-models")
200-
.build();
201-
ActivityChatModel chatModel = ActivityChatModel.forModel("reasoning", opts);
202-
```
203-
204-
For repeated per-model overrides, declare a `ChatModelActivityOptions` bean and auto-configuration wires the map into the plugin. See that class's javadoc for the pattern.
205-
206-
`ActivityMcpClient.create()` / `create(ActivityOptions)` behave the same way with a 30-second default timeout.
207-
208206
## Known limitations
209207

210208
- **Streaming (`chatClient.stream(...)`)** — not currently supported. Use `.call()` instead.

temporal-spring-ai/src/main/java/io/temporal/springai/autoconfigure/ChatModelActivityOptions.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@
3131
* that model. Use {@link ActivityChatModel#defaultActivityOptions()} as the baseline so the
3232
* plugin's non-retryable-error classification is preserved.
3333
*
34+
* <p>A key equal to {@link io.temporal.springai.model.ChatModelTypes#DEFAULT_MODEL_NAME} (the
35+
* literal {@code "default"}) acts as a global catch-all: any chat model that lacks a
36+
* bean-name-specific entry — including models contributed by third-party starters that your
37+
* application did not declare directly — picks up the {@code "default"} entry. Keys that neither
38+
* match a registered ChatModel bean nor equal {@code "default"} cause plugin construction to fail.
39+
*
3440
* @param byModelName per-model-bean-name {@link ActivityOptions} overrides; may be empty
3541
*/
3642
public record ChatModelActivityOptions(Map<String, ActivityOptions> byModelName) {

temporal-spring-ai/src/main/java/io/temporal/springai/model/ActivityChatModel.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,8 @@ public static ActivityChatModel forDefault(ActivityOptions options) {
152152
* <ol>
153153
* <li>An entry registered on {@link SpringAiPlugin} under {@code modelName} in the per-model
154154
* {@code ActivityOptions} map, if any.
155+
* <li>An entry registered under {@link ChatModelTypes#DEFAULT_MODEL_NAME} in the per-model map,
156+
* which acts as a user-declared catch-all for models without a specific entry.
155157
* <li>The plugin's default {@link ActivityOptions} (2-minute start-to-close, 3 attempts,
156158
* clearly permanent AI errors marked non-retryable).
157159
* </ol>
@@ -168,6 +170,7 @@ public static ActivityChatModel forDefault(ActivityOptions options) {
168170
public static ActivityChatModel forModel(String modelName) {
169171
ActivityOptions options =
170172
SpringAiPluginOptions.optionsFor(modelName)
173+
.or(() -> SpringAiPluginOptions.optionsFor(ChatModelTypes.DEFAULT_MODEL_NAME))
171174
.orElseGet(ActivityChatModel::defaultActivityOptions);
172175
return forModel(modelName, options);
173176
}

temporal-spring-ai/src/main/java/io/temporal/springai/plugin/SpringAiPlugin.java

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,14 +71,19 @@ public SpringAiPlugin(Map<String, ChatModel> chatModels, @Nullable ChatModel pri
7171
* Creates a new SpringAiPlugin with multiple ChatModels and per-model {@link ActivityOptions}.
7272
*
7373
* <p>Entries in {@code perModelOptions} are keyed by chat-model bean name and consulted by {@link
74-
* io.temporal.springai.model.ActivityChatModel#forModel(String)} (and by {@link
75-
* io.temporal.springai.model.ActivityChatModel#forDefault()} via {@link
76-
* ChatModelTypes#DEFAULT_MODEL_NAME}). Callers who pass explicit {@code ActivityOptions} to a
77-
* factory bypass this map entirely.
74+
* io.temporal.springai.model.ActivityChatModel#forModel(String)} and {@link
75+
* io.temporal.springai.model.ActivityChatModel#forDefault()}. An entry whose key equals {@link
76+
* ChatModelTypes#DEFAULT_MODEL_NAME} acts as a global catch-all: models without a
77+
* bean-name-specific entry pick it up, including models contributed by third-party starters that
78+
* the application did not declare directly. Keys that neither match a bean nor equal the
79+
* catch-all name cause plugin construction to fail, so a typo'd model name surfaces early.
80+
* Callers who pass explicit {@code ActivityOptions} to a factory bypass this map entirely.
7881
*
7982
* @param chatModels map of bean names to ChatModel instances
8083
* @param primaryChatModel the primary chat model (used to determine default), or null
8184
* @param perModelOptions per-model-name ActivityOptions overrides; may be empty
85+
* @throws IllegalArgumentException if a key in {@code perModelOptions} neither matches a
86+
* registered ChatModel bean name nor equals {@link ChatModelTypes#DEFAULT_MODEL_NAME}
8287
*/
8388
public SpringAiPlugin(
8489
Map<String, ChatModel> chatModels,
@@ -104,6 +109,20 @@ public SpringAiPlugin(
104109
this.defaultModelName = chatModels.keySet().iterator().next();
105110
}
106111

112+
if (perModelOptions != null) {
113+
for (String key : perModelOptions.keySet()) {
114+
if (!ChatModelTypes.DEFAULT_MODEL_NAME.equals(key) && !this.chatModels.containsKey(key)) {
115+
throw new IllegalArgumentException(
116+
"perModelOptions key '"
117+
+ key
118+
+ "' does not match any ChatModel bean. Registered models: "
119+
+ this.chatModels.keySet()
120+
+ ". Use '"
121+
+ ChatModelTypes.DEFAULT_MODEL_NAME
122+
+ "' as a catch-all for models without a specific entry.");
123+
}
124+
}
125+
}
107126
SpringAiPluginOptions.register(perModelOptions);
108127

109128
if (chatModels.size() > 1) {

temporal-spring-ai/src/test/java/io/temporal/springai/PerModelActivityOptionsTest.java

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import io.temporal.springai.activity.ChatModelActivityImpl;
1111
import io.temporal.springai.chat.TemporalChatClient;
1212
import io.temporal.springai.model.ActivityChatModel;
13+
import io.temporal.springai.model.ChatModelTypes;
1314
import io.temporal.springai.plugin.SpringAiPluginOptions;
1415
import io.temporal.testing.TestWorkflowEnvironment;
1516
import io.temporal.worker.Worker;
@@ -37,6 +38,7 @@ class PerModelActivityOptionsTest {
3738
private static final String TASK_QUEUE = "test-spring-ai-per-model-options";
3839
private static final Duration SLOW_TIMEOUT = Duration.ofMinutes(10);
3940
private static final Duration EXPLICIT_TIMEOUT = Duration.ofMinutes(7);
41+
private static final Duration CATCH_ALL_TIMEOUT = Duration.ofMinutes(4);
4042
private static final Duration DEFAULT_TIMEOUT = ActivityChatModel.DEFAULT_TIMEOUT;
4143

4244
private TestWorkflowEnvironment testEnv;
@@ -92,6 +94,48 @@ void registryMiss_fallsBackToDefault() {
9294
runAndAssertScheduledTimeout(UnknownModelWorkflow.class, DEFAULT_TIMEOUT);
9395
}
9496

97+
@Test
98+
void catchAllKey_appliesToUnknownModel() {
99+
// Only the DEFAULT_MODEL_NAME catch-all is registered; a lookup for a model without its own
100+
// entry should fall through to it (not to the library defaults).
101+
SpringAiPluginOptions.register(
102+
Map.of(
103+
ChatModelTypes.DEFAULT_MODEL_NAME,
104+
ActivityOptions.newBuilder(ActivityChatModel.defaultActivityOptions())
105+
.setStartToCloseTimeout(CATCH_ALL_TIMEOUT)
106+
.build()));
107+
108+
Worker worker = testEnv.newWorker(TASK_QUEUE);
109+
worker.registerWorkflowImplementationTypes(UnknownModelWorkflowImpl.class);
110+
worker.registerActivitiesImplementations(stubActivityImpl());
111+
testEnv.start();
112+
113+
runAndAssertScheduledTimeout(UnknownModelWorkflow.class, CATCH_ALL_TIMEOUT);
114+
}
115+
116+
@Test
117+
void specificEntry_winsOverCatchAll() {
118+
// Both the catch-all and a model-specific entry are registered; the model-specific one
119+
// must win for that model.
120+
SpringAiPluginOptions.register(
121+
Map.of(
122+
ChatModelTypes.DEFAULT_MODEL_NAME,
123+
ActivityOptions.newBuilder(ActivityChatModel.defaultActivityOptions())
124+
.setStartToCloseTimeout(CATCH_ALL_TIMEOUT)
125+
.build(),
126+
"slow",
127+
ActivityOptions.newBuilder(ActivityChatModel.defaultActivityOptions())
128+
.setStartToCloseTimeout(SLOW_TIMEOUT)
129+
.build()));
130+
131+
Worker worker = testEnv.newWorker(TASK_QUEUE);
132+
worker.registerWorkflowImplementationTypes(SlowModelWorkflowImpl.class);
133+
worker.registerActivitiesImplementations(stubActivityImpl());
134+
testEnv.start();
135+
136+
runAndAssertScheduledTimeout(SlowModelWorkflow.class, SLOW_TIMEOUT);
137+
}
138+
95139
@Test
96140
void explicitOptions_bypassRegistry() {
97141
SpringAiPluginOptions.register(

temporal-spring-ai/src/test/java/io/temporal/springai/plugin/SpringAiPluginTest.java

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@
33
import static org.junit.jupiter.api.Assertions.*;
44
import static org.mockito.Mockito.*;
55

6+
import io.temporal.activity.ActivityOptions;
67
import io.temporal.springai.activity.ChatModelActivityImpl;
78
import io.temporal.springai.activity.EmbeddingModelActivityImpl;
89
import io.temporal.springai.activity.VectorStoreActivityImpl;
10+
import io.temporal.springai.model.ActivityChatModel;
911
import io.temporal.springai.model.ChatModelTypes;
1012
import io.temporal.worker.Worker;
1113
import java.util.*;
@@ -108,6 +110,53 @@ void emptyChatModelsMap_throwsIllegalArgument() {
108110
IllegalArgumentException.class, () -> new SpringAiPlugin(new LinkedHashMap<>(), null));
109111
}
110112

113+
@Test
114+
void perModelOptions_keyMatchingBean_accepts() {
115+
ChatModel openai = mock(ChatModel.class);
116+
ChatModel anthropic = mock(ChatModel.class);
117+
Map<String, ChatModel> models = new LinkedHashMap<>();
118+
models.put("openai", openai);
119+
models.put("anthropic", anthropic);
120+
121+
// Should not throw — both keys match registered bean names.
122+
new SpringAiPlugin(
123+
models,
124+
null,
125+
Map.of(
126+
"openai", ActivityChatModel.defaultActivityOptions(),
127+
"anthropic", ActivityChatModel.defaultActivityOptions()));
128+
}
129+
130+
@Test
131+
void perModelOptions_catchAllKey_accepts() {
132+
ChatModel openai = mock(ChatModel.class);
133+
Map<String, ChatModel> models = Map.of("openai", openai);
134+
135+
// DEFAULT_MODEL_NAME is a valid key even when no bean is named "default" — it acts as the
136+
// catch-all for models that don't have a specific entry, including third-party additions.
137+
new SpringAiPlugin(
138+
models,
139+
null,
140+
Map.of(ChatModelTypes.DEFAULT_MODEL_NAME, ActivityChatModel.defaultActivityOptions()));
141+
}
142+
143+
@Test
144+
void perModelOptions_typoKey_throws() {
145+
ChatModel openai = mock(ChatModel.class);
146+
Map<String, ChatModel> models = Map.of("openai", openai);
147+
Map<String, ActivityOptions> typo =
148+
Map.of("openia", ActivityChatModel.defaultActivityOptions());
149+
150+
IllegalArgumentException ex =
151+
assertThrows(IllegalArgumentException.class, () -> new SpringAiPlugin(models, null, typo));
152+
assertTrue(
153+
ex.getMessage().contains("openia"),
154+
"message should name the offending key; got: " + ex.getMessage());
155+
assertTrue(
156+
ex.getMessage().contains(ChatModelTypes.DEFAULT_MODEL_NAME),
157+
"message should point users at the catch-all key; got: " + ex.getMessage());
158+
}
159+
111160
// --- Optional plugin tests ---
112161

113162
@Test

0 commit comments

Comments
 (0)