Skip to content

Commit 35f9306

Browse files
temporal-spring-ai: amend plan — ActivityOptions overloads fix summaries 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>
1 parent 6432b41 commit 35f9306

1 file changed

Lines changed: 66 additions & 7 deletions

File tree

temporal-spring-ai/PLAN.md

Lines changed: 66 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,32 @@ Two related production-readiness gaps:
1414
have to bypass the factories entirely and construct their own
1515
`Workflow.newActivityStub(ChatModelActivity.class, ...)`.
1616

17+
### Interaction with the `spring-ai/activity-summaries` branch
18+
19+
The summaries branch landed a plugin-owned `ActivityOptions` store on
20+
`ActivityChatModel` and `ActivityMcpClient`: the factories (`forModel`,
21+
`create`) now retain the options they built the stub with, and the per-call
22+
path overlays a Summary on top. The bare public constructors
23+
(`new ActivityChatModel(stub)`, `new ActivityMcpClient(activity)`) receive
24+
no options and therefore emit no summary — silently. That's a real UX wart:
25+
anyone who rolls their own stub today loses Summary labels on the chat /
26+
MCP rows in the Temporal UI.
27+
28+
The `ActivityOptions` factory overloads this branch adds **are the proper
29+
fix** for that wart. Users who need non-default timeouts / retries / task
30+
queue / heartbeats today fall back to the public constructor to get those;
31+
giving them a factory that takes `ActivityOptions` means they keep
32+
summaries while still customizing the stub. This branch should therefore:
33+
34+
- Explicitly document (in `ActivityChatModel` and `ActivityMcpClient`
35+
javadocs, and the README) that Summary labels appear only when the stub
36+
is built via one of the factories, and point at the new overloads as the
37+
recommended path for customization.
38+
- Leave the public constructors in place for backward compatibility, but
39+
mark them `@Deprecated` or, at minimum, add a javadoc note that they
40+
skip UI summaries. Decision: ship the deprecation alongside the new
41+
factory so the doc and the warning point to the same replacement.
42+
1743
## Files to change
1844

1945
- `src/main/java/io/temporal/springai/model/ActivityChatModel.java`
@@ -23,11 +49,30 @@ Two related production-readiness gaps:
2349
an `ActivityOptions` internally that includes a default
2450
`RetryOptions.setDoNotRetry(...)` list (see §Retry classification below)
2551
and delegate to the new `ActivityOptions` overload.
26-
- Keep all existing overloads working — purely additive API.
52+
- The new overloads pass their `ActivityOptions` into the existing
53+
private `(stub, modelName, baseOptions)` constructor added by the
54+
summaries branch, so Summary labels work out of the box — no extra
55+
wiring.
56+
- Javadoc on the public `ActivityChatModel(ChatModelActivity[, String])`
57+
constructors: add a note that callers who want UI Summaries should
58+
prefer the new `forDefault(ActivityOptions)` / `forModel(String,
59+
ActivityOptions)` factories. Mark the constructors `@Deprecated` with
60+
a message pointing at the factory replacement.
61+
- Keep all existing overloads working at runtime — purely additive API.
2762

2863
- `src/main/java/io/temporal/springai/mcp/ActivityMcpClient.java`
2964
- Same treatment: add `create(ActivityOptions options)` overload; existing
3065
`create(timeout, maxAttempts)` delegates.
66+
- Thread the passed `ActivityOptions` through the private
67+
`(activity, baseOptions)` constructor added by the summaries branch so
68+
`callTool(..., summary)` overlays work.
69+
- `@Deprecated` the public `new ActivityMcpClient(activity)` constructor
70+
with a javadoc note recommending the factory for Summary support.
71+
72+
- `README.md`
73+
- Brief note in the quick-start and the "Tool Types" section that chat
74+
and MCP rows in the Temporal UI are labeled with Summaries when (and
75+
only when) built via the factories.
3176

3277
### Retry classification
3378

@@ -54,6 +99,11 @@ own, so no extra entries strictly required).
5499
`NonTransientAiException` does not trigger retry.
55100
- Unit test: default factory still retries on a transient `RuntimeException`
56101
(ensure the doNotRetry list doesn't over-reach).
102+
- Extend `ActivitySummaryTest` (added on the summaries branch) to assert a
103+
workflow built via `ActivityChatModel.forDefault(customOptions)` still
104+
carries the expected `chat: …` summary on its scheduled chat activity.
105+
This guards against a regression where the new overloads silently drop
106+
the `baseOptions` wiring.
57107
- `WorkflowReplayer` replay check.
58108

59109
## PR
@@ -70,15 +120,24 @@ own, so no extra entries strictly required).
70120
- Default RetryOptions for chat and MCP activities now include
71121
`doNotRetry` entries for clearly non-transient Spring AI failures
72122
(`NonTransientAiException`, `IllegalArgumentException`).
73-
- Existing factory signatures are preserved; they now delegate to the new
123+
- Public `new ActivityChatModel(...)` and `new ActivityMcpClient(...)`
124+
constructors are `@Deprecated` with javadoc pointing at the factory
125+
replacement. They still work at runtime but skip UI Summaries, which is
126+
now called out explicitly.
127+
- Existing factory signatures are preserved; they delegate to the new
74128
`ActivityOptions` overloads.
75129
76130
## Why?
77-
Previously the only way to customize the chat or MCP activity stub was via
78-
`(timeout, maxAttempts)` — no heartbeats, task queue override, Summary,
131+
Previously the only way to customize the chat or MCP activity stub was
132+
via `(timeout, maxAttempts)` — no heartbeats, task queue override,
79133
priority, or `doNotRetry`. Users hitting those needs had to bypass the
80-
factories entirely. Also: transient-vs-permanent classification matters a
81-
lot for LLM calls (a 401 shouldn't retry for minutes), and the integration
82-
guide specifically calls out "tell Temporal which errors are retryable and
134+
factories and call the public constructor with a hand-built stub, which
135+
as of the summaries branch silently drops the Temporal UI labels for
136+
chat and MCP rows. The new `ActivityOptions` overloads close that gap:
137+
users get full customization *and* keep the Summary labels.
138+
139+
Also: transient-vs-permanent classification matters a lot for LLM calls
140+
(a 401 shouldn't retry for minutes), and the integration guide
141+
specifically calls out "tell Temporal which errors are retryable and
83142
which are not." This brings the plugin in line.
84143
```

0 commit comments

Comments
 (0)