Skip to content

temporal-spring-ai: per-model ActivityOptions registry#2855

Open
donald-pinckney wants to merge 20 commits intomasterfrom
spring-ai/per-model-timeouts
Open

temporal-spring-ai: per-model ActivityOptions registry#2855
donald-pinckney wants to merge 20 commits intomasterfrom
spring-ai/per-model-timeouts

Conversation

@donald-pinckney
Copy link
Copy Markdown
Contributor

@donald-pinckney donald-pinckney commented Apr 21, 2026

Based on

Merge those in order first; this PR will retarget each time an ancestor lands.


What was changed

  • New SpringAiPluginOptions static registry (package io.temporal.springai.plugin). SpringAiPlugin publishes a Map<String, ActivityOptions> to it on construction; ActivityChatModel.forModel(String) / forDefault() consult it before falling back to the plugin defaults.
  • SpringAiPlugin gains a third constructor that accepts the per-model map; the two existing constructors delegate with an empty map. No constructor changes to existing SpringAiPlugin(ChatModel) / SpringAiPlugin(Map, ChatModel) call sites.
  • New ChatModelActivityOptions record (package io.temporal.springai.autoconfigure) — a thin wrapper around Map<String, ActivityOptions> — exists so SpringAiTemporalAutoConfiguration can inject user options by type rather than by bean name. Raw Map<String, ActivityOptions> injection would trigger Spring's collection-of-beans autowiring and sweep in any unrelated ActivityOptions bean in the context. Using a dedicated wrapper type avoids that collision and keeps the design consistent with temporal-spring-ai: discover MCP clients by type, not by bean name #2859 (MCP bean lookup by type, not by name).
  • SpringAiTemporalAutoConfiguration injects ObjectProvider<ChatModelActivityOptions> and forwards the wrapped map to the plugin. No magic bean name, no @Qualifier.
  • Callers who pass explicit ActivityOptions via forModel(name, options) / forDefault(options) bypass the registry entirely. The (timeout, maxAttempts) convenience factory is unaffected — it still builds options from its arguments.
  • Tests: PerModelActivityOptionsTest covers the three cases in the plan — registry hit uses the registered startToCloseTimeout, registry miss falls back to the 2-minute default, explicit options bypass a populated registry entry.

Why?

A single default (2 min start-to-close, 3 attempts) doesn't fit every model. Reasoning and thinking-mode models need more time; fast models want shorter timeouts so retries recover quickly. The Temporal AI integration guide specifically calls this out as a capability partners should expose. With this change, users register one bean and never have to hand-build activity stubs again.

Example user config:

@Bean
ChatModelActivityOptions chatModelActivityOptions() {
    return new ChatModelActivityOptions(Map.of(
        "reasoning", ActivityOptions.newBuilder(ActivityChatModel.defaultActivityOptions())
            .setStartToCloseTimeout(Duration.ofMinutes(15))
            .build()));
}

donald-pinckney and others added 4 commits April 21, 2026 15:52
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Narrows the activity-summaries scope to cases where the plugin itself owns
activity-stub creation. Activity-backed tool calls, Nexus tool calls, and
@SideEffectTool calls are now explicitly out of scope; the first two would
require per-call option overrides on user-owned stubs (no clean API), and
the third writes MarkerRecorded events which have no Summary field.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Runs a workflow that drives a single chat call through ActivityChatModel,
fetches the resulting history, and asserts the ActivityTaskScheduled event
for callChatModel carries a userMetadata Summary that starts with
"chat: default" and includes the user prompt. Intentionally fails against
unmodified chat code — the implementation follows in a subsequent commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ActivityChatModel.forModel() now stores the ActivityOptions it built and,
on each chat call, rebuilds the stub with a per-call Summary of the form
"chat: <model> · <first 60 chars of user prompt>". When a caller passes a
pre-built stub directly via the public constructors, behavior is unchanged
(no options known → no summary overlay).

ActivityMcpClient.create() does the same and adds a callTool(clientName,
request, summary) overload. McpToolCallback passes "mcp: <client>.<tool>".

Also fixes the activity-type-name casing in ActivitySummaryTest — Temporal
capitalizes the first character of method-name-derived activity types, so
the event carries "CallChatModel", not "callChatModel".

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@donald-pinckney donald-pinckney force-pushed the spring-ai/per-model-timeouts branch from 44777c8 to 316d52b Compare April 22, 2026 15:44
Planning scratchpad — not part of the shipped artifact. Removed before merge.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@donald-pinckney donald-pinckney force-pushed the spring-ai/retry-and-options branch from f967316 to 1310ba4 Compare April 22, 2026 16:07
@donald-pinckney donald-pinckney force-pushed the spring-ai/per-model-timeouts branch from 316d52b to ba6a88a Compare April 22, 2026 16:08
The Summary now carries only the model label ("chat: <model>") instead
of "chat: <model> · <first 60 chars of user prompt>". Including even a
truncated prompt leaks whatever the prompt contains — PII, secrets,
internal identifiers — into workflow history, server logs, and the
Temporal UI, which is a surprising default for an observability label.

An opt-in API for callers who explicitly want the prompt in the
Summary can be added later if there's demand.

ActivitySummaryTest.chatActivity_carriesModelOnlySummary_neverLeaksUserPrompt
asserts the Summary equals "chat: default" exactly and defensively
checks that no part of the prompt leaked in.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@donald-pinckney donald-pinckney force-pushed the spring-ai/retry-and-options branch from 1310ba4 to 9bb3866 Compare April 22, 2026 16:57
@donald-pinckney donald-pinckney force-pushed the spring-ai/per-model-timeouts branch 2 times, most recently from bad7fd6 to e8405cc Compare April 22, 2026 17:54
@donald-pinckney donald-pinckney force-pushed the spring-ai/retry-and-options branch from f038d46 to 9528294 Compare April 22, 2026 19:10
@donald-pinckney donald-pinckney force-pushed the spring-ai/per-model-timeouts branch from e8405cc to 1a2adb2 Compare April 22, 2026 19:11
@donald-pinckney donald-pinckney marked this pull request as ready for review April 22, 2026 19:50
@donald-pinckney donald-pinckney requested a review from a team as a code owner April 22, 2026 19:50
donald-pinckney and others added 8 commits April 22, 2026 19:32
Addresses review feedback (thread on #2852) preferring typesafe
Optional<T> over nullable fields and raw null delegation. Three
sites changed:

- ActivityChatModel: modelName and baseOptions fields + private ctor
  params are now Optional<String> / Optional<ActivityOptions>.
  getModelName() returns Optional<String>. Public ctors and factories
  still accept nullable String modelName at the API boundary (matches
  prior javadoc: "null for default"); they normalize via
  Optional.ofNullable before storing. Internal readers use .map /
  .orElse instead of null checks.
- ActivityMcpClient: the 3-arg callTool's summary parameter is now
  Optional<String>. The 2-arg convenience overload passes
  Optional.empty(). The rebuild-with-summary branch uses
  .isPresent() + .get() instead of null checks.
- McpToolCallback.call(...) wraps its generated summary string in
  Optional.of(...) before passing to callTool.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reverses the previous Optional<> commit in favor of @nullable on the
nullable fields/params. Matches the dominant convention in
temporal-sdk (431+ @nullable usages on public API surface) and avoids
a thicker runtime story for what is fundamentally a documentation /
IDE-hint concern (no NullAway in the build either way).

- ActivityChatModel: modelName and baseOptions fields + private ctor
  params are now @nullable String / @nullable ActivityOptions. Public
  ctors/factories accept nullable String modelName at the API
  boundary directly. getModelName() returns @nullable String.
- ActivityMcpClient: baseOptions field and callTool(..., summary)
  param use @nullable instead of Optional<>. 2-arg callTool passes
  null; McpToolCallback passes a plain String.
- ChatModelTypes.ChatModelActivityInput record: @nullable on
  modelName and modelOptions fields so deserialized readers see the
  nullability in the signature (per the reviewer's concern about the
  deserialization-side typesafety).

Consistent with org.springframework.lang.Nullable already used
elsewhere in this module (TemporalChatClient, SpringAiPlugin).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Switch the five files in this module that imported
org.springframework.lang.Nullable over to javax.annotation.Nullable
to match the dominant convention in sdk-java (197 usages vs. 7).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e error classification

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ies UX wart

The activity-summaries branch only overlays Summaries when the factories
(forModel/create) built the stub. Users who need custom timeouts today fall
back to the public constructor, which silently drops UI Summaries. The
ActivityOptions overloads planned here are the proper fix: they let users
customize the stub and keep Summary labels. Plan now also covers deprecating
the public constructors with javadoc pointing at the factories.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… AI errors

- ActivityChatModel.forDefault(ActivityOptions) and forModel(String,
  ActivityOptions) overloads added. New public defaultActivityOptions()
  returns the plugin's default bundle so callers can tweak one field
  without losing the other sensible defaults.
- ActivityMcpClient.create(ActivityOptions) + defaultActivityOptions()
  added, mirroring the chat side.
- Default RetryOptions for chat calls now mark
  org.springframework.ai.retry.NonTransientAiException and
  java.lang.IllegalArgumentException non-retryable. Default options for
  MCP calls mark IllegalArgumentException non-retryable. User-supplied
  ActivityOptions pass through verbatim — the plugin does not augment
  them.
- new ActivityChatModel(...) and new ActivityMcpClient(activity)
  constructors are @deprecated with javadoc pointing at the factories —
  they still work at runtime but skip the UI Summary labels the
  plugin-owned stub path attaches, which is now called out explicitly.
- README: new "Activity options and retry behavior" section documents
  the defaults, how to customize, and the Summary/factory connection.
- Tests: two new suites — ActivityOptionsAndRetryTest covers the
  non-retryable classification (1 attempt for NonTransientAiException,
  3 attempts for transient RuntimeException, custom task queue landing
  on the scheduled activity); ActivitySummaryTest gains a regression
  test asserting forDefault(customOptions) still emits UI Summaries.
- build.gradle: spring-ai-retry added as a testImplementation so tests
  can reference NonTransientAiException directly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Planning scratchpad — not part of the shipped artifact. Removed before merge.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Now that forDefault(ActivityOptions) / forModel(String, ActivityOptions)
/ create(ActivityOptions) exist, the (Duration, int) convenience
overloads are asymmetric dead weight — they expose two of N ActivityOptions
fields as positional parameters, and callers wanting anything else
(heartbeats, task queue, custom retry backoff, ...) have to drop to the
ActivityOptions path anyway. Removed pre-release so the API surface is
consistent: no-arg → plugin defaults; ActivityOptions arg → caller options.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@donald-pinckney donald-pinckney force-pushed the spring-ai/retry-and-options branch from 9528294 to 006d8ad Compare April 23, 2026 14:10
donald-pinckney and others added 3 commits April 23, 2026 10:12
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
SpringAiPlugin now accepts an optional Map<String, ActivityOptions>
keyed by chat-model bean name. On construction the plugin publishes
the map to a new package-public static registry SpringAiPluginOptions,
which ActivityChatModel.forModel(name) and forDefault() consult when
building the activity stub. Entries resolve by bean name; the reserved
key SpringAiPlugin.DEFAULT_MODEL_NAME ("default") covers forDefault().

Callers who pass explicit ActivityOptions via forModel(name, options)
or forDefault(options) bypass the registry entirely — explicit options
always win. The registry has no effect on the (timeout, maxAttempts)
convenience factory either; that still builds options from its args.

Auto-configuration picks up a user bean named
"chatModelActivityOptions" (constant
SpringAiTemporalAutoConfiguration.CHAT_MODEL_ACTIVITY_OPTIONS_BEAN) of
type Map<String, ActivityOptions>. The explicit bean-name qualifier
avoids Spring's collection-of-beans auto-wiring for Map<String, T>.

Tests: PerModelActivityOptionsTest covers the three cases called out
in the plan — registry hit, registry miss (falls back to default
2-minute timeout), and explicit options bypass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Planning scratchpad — not part of the shipped artifact. Removed before merge.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@donald-pinckney donald-pinckney force-pushed the spring-ai/per-model-timeouts branch from 1a2adb2 to d9baf99 Compare April 23, 2026 14:14
Base automatically changed from spring-ai/retry-and-options to master April 23, 2026 20:03
@brianstrauch
Copy link
Copy Markdown
Contributor

brianstrauch commented Apr 24, 2026

How would you feel about having two ways to initialize? (1) default activity options, or (2) explicit options. Combining the default options with explicit options makes it confusing what the final options are.

…-timeouts

# Conflicts:
#	temporal-spring-ai/build.gradle
#	temporal-spring-ai/src/main/java/io/temporal/springai/model/ActivityChatModel.java
…-timeouts

# Conflicts:
#	temporal-spring-ai/src/main/java/io/temporal/springai/plugin/SpringAiPlugin.java
@donald-pinckney
Copy link
Copy Markdown
Contributor Author

donald-pinckney commented Apr 24, 2026

@brianstrauch Just so we're clear, default options and explicit options don't get merged together ever, but what I think is confusing at the moment is that there is a fallthrough path of: no matching entry in perModelOptions -> use plugin builtin defaults. I don't think thats clear, and also isn't controllable by the user.

I suggest a very simple tweak to this PR, where perModelOptions can have a special ChatModelTypes.DEFAULT_MODEL_NAME key, which will act as a user-provided global fallback. This makes sense, as its a nice parallel structure to ChatModelTypes.DEFAULT_MODEL_NAME being the key for the default model to fallback to.

Specifically:

  • perModelOptions keys may be:
    • a ChatModel bean name: per-model override
    • the literal ChatModelTypes.DEFAULT_MODEL_NAME ("default"): acts as a user-declared global fallback
    • Any other key: IllegalArgumentException at plugin construction (typo safety).
  • Lookup of options in forModel(name):
    a. perModelOptions[name] if present
    b. else perModelOptions["default"] if present
    c. else library defaults
  • Lookup in forDefault():
    a. perModelOptions["default"] if present
    b. else library defaults

…' 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>
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