Skip to content

Commit 6d4d166

Browse files
donald-pinckneyclaude
donald-pinckney
and
claude
committed
Triage Copilot and DABH review comments into TASK_QUEUE
T16-T21: fixes (NPE guard, error message, multi-ChatModel bug, replay test, duplicate MCP names, embedding boxing) T22-T24: design discussions to have with Don (starter artifact, MCP caching, Object vs String) T25-T27: replies (docs, SandboxingAdvisor tests, ToolContext drop — last two likely moot if T15 lands) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 336cc7b commit 6d4d166

1 file changed

Lines changed: 120 additions & 0 deletions

File tree

‎TASK_QUEUE.json‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,126 @@
2020
"depends_on": [],
2121
"status": "blocked",
2222
"notes": "Blocked on PR review discussion with tconley1428. Agreed on direction but need to finalize details before implementing. Proposed design: https://github.com/temporalio/sdk-java/pull/2829#discussion_r3060711651"
23+
},
24+
{
25+
"id": "T16",
26+
"title": "Fix NPE on assistantMessage.getMedia() in ActivityChatModel",
27+
"description": "ASSISTANT branch in toActivityMessages() calls assistantMessage.getMedia().stream() without null guard. Other places in the module use CollectionUtils.isEmpty(). Add the guard.",
28+
"severity": "medium",
29+
"category": "bugfix",
30+
"depends_on": [],
31+
"status": "todo",
32+
"notes": "Copilot review comment: https://github.com/temporalio/sdk-java/pull/2829/files#r3053789704"
33+
},
34+
{
35+
"id": "T17",
36+
"title": "Fix error message in TemporalToolUtil to mention Nexus stubs",
37+
"description": "The IllegalArgumentException for unknown tool types doesn't mention Nexus service stubs as supported, even though they are. Update the message.",
38+
"severity": "low",
39+
"category": "bugfix",
40+
"depends_on": [],
41+
"status": "todo",
42+
"notes": "Copilot review comment: https://github.com/temporalio/sdk-java/pull/2829/files#r3053789727. May be partly superseded by T15 if the error message changes entirely."
43+
},
44+
{
45+
"id": "T18",
46+
"title": "Fix NoUniqueBeanDefinitionException with multiple ChatModels",
47+
"description": "SpringAiTemporalAutoConfiguration injects @Autowired(required=false) ChatModel primaryChatModel, which fails with NoUniqueBeanDefinitionException when multiple ChatModel beans exist without @Primary. Use ObjectProvider<ChatModel> instead.",
48+
"severity": "high",
49+
"category": "bugfix",
50+
"depends_on": [],
51+
"status": "todo",
52+
"notes": "Real bug — hit this in the multimodel sample, worked around with auto-config exclusions. Copilot review comment: https://github.com/temporalio/sdk-java/pull/2829/files#r3053789744"
53+
},
54+
{
55+
"id": "T19",
56+
"title": "Make replay test actually exercise tool calls",
57+
"description": "workflowWithTools_replaysDeterministically uses a StubChatModel that never requests tool calls, so tools are never invoked. Adjust the stub to return tool call requests first, then a final response, so the test actually validates tool execution determinism.",
58+
"severity": "medium",
59+
"category": "tests",
60+
"depends_on": [],
61+
"status": "todo",
62+
"notes": "Copilot review comment: https://github.com/temporalio/sdk-java/pull/2829/files#r3053789763"
63+
},
64+
{
65+
"id": "T20",
66+
"title": "Handle duplicate MCP client names in McpClientActivityImpl",
67+
"description": "Collectors.toMap() in the constructor throws IllegalStateException on duplicate keys. Two MCP clients with the same name() would crash. Should validate or handle gracefully.",
68+
"severity": "medium",
69+
"category": "bugfix",
70+
"depends_on": [],
71+
"status": "todo",
72+
"notes": "DABH review comment: https://github.com/temporalio/sdk-java/pull/2829/files#r3054031855"
73+
},
74+
{
75+
"id": "T21",
76+
"title": "Use primitive arrays in EmbeddingModelTypes instead of List<Double>",
77+
"description": "Converting float[] to List<Double> creates 1536+ boxed Double objects per embedding. Use float[] or double[] in EmbeddingModelTypes to avoid boxing overhead and reduce serialization size.",
78+
"severity": "low",
79+
"category": "improvement",
80+
"depends_on": [],
81+
"status": "todo",
82+
"notes": "DABH review comment: https://github.com/temporalio/sdk-java/pull/2829/files#r3054058971"
83+
},
84+
{
85+
"id": "T22",
86+
"title": "Discuss: temporal-spring-ai-starter artifact for easier onboarding",
87+
"description": "temporal-sdk is compileOnly, so users must declare it separately. DABH asks if a starter that pulls in the right transitive deps would reduce friction. Need to decide if this is worth it or if documentation is sufficient.",
88+
"severity": "medium",
89+
"category": "discussion",
90+
"depends_on": [],
91+
"status": "todo",
92+
"notes": "DABH review comment: https://github.com/temporalio/sdk-java/pull/2829/files#r3053808755. Discuss with Don."
93+
},
94+
{
95+
"id": "T23",
96+
"title": "Discuss: ActivityMcpClient capability caching and replay",
97+
"description": "ActivityMcpClient caches getServerCapabilities() after first call. DABH asks if stale cache is a replay concern. Probably fine (activity result is in history, so replay uses the original value — which is correct). But worth confirming the design intent.",
98+
"severity": "low",
99+
"category": "discussion",
100+
"depends_on": [],
101+
"status": "todo",
102+
"notes": "DABH review comment: https://github.com/temporalio/sdk-java/pull/2829/files#r3054027377. Discuss with Don."
103+
},
104+
{
105+
"id": "T24",
106+
"title": "Discuss: Change ChatModelTypes.rawContent from Object to String",
107+
"description": "rawContent is Object but always cast to String. DABH suggests making it String. Cleaner but changes the JSON schema. If we ever need non-string content (multimodal), we'd have to change it back.",
108+
"severity": "low",
109+
"category": "discussion",
110+
"depends_on": [],
111+
"status": "todo",
112+
"notes": "DABH review comment: https://github.com/temporalio/sdk-java/pull/2829/files#r3054049714. Discuss with Don."
113+
},
114+
{
115+
"id": "T25",
116+
"title": "Reply: compatibility matrix in docs",
117+
"description": "DABH suggests documenting the compatibility matrix (Java version, Spring Boot version, Spring AI version). Acknowledge and defer to a docs PR.",
118+
"severity": "low",
119+
"category": "reply",
120+
"depends_on": [],
121+
"status": "todo",
122+
"notes": "DABH review comment: https://github.com/temporalio/sdk-java/pull/2829/files#r3053818533"
123+
},
124+
{
125+
"id": "T26",
126+
"title": "Reply: SandboxingAdvisor lacks tests",
127+
"description": "DABH notes SandboxingAdvisor has no tests. Likely moot if T15 removes it. Reply explaining that.",
128+
"severity": "low",
129+
"category": "reply",
130+
"depends_on": ["T15"],
131+
"status": "todo",
132+
"notes": "DABH review comment: https://github.com/temporalio/sdk-java/pull/2829/files#r3053836427"
133+
},
134+
{
135+
"id": "T27",
136+
"title": "Reply: ToolContext silently dropped in LocalActivityToolCallbackWrapper",
137+
"description": "DABH notes ToolContext is silently ignored. Likely moot if T15 removes LocalActivityToolCallbackWrapper. Reply explaining that.",
138+
"severity": "low",
139+
"category": "reply",
140+
"depends_on": ["T15"],
141+
"status": "todo",
142+
"notes": "DABH review comment: https://github.com/temporalio/sdk-java/pull/2829/files#r3054036773"
23143
}
24144
]
25145
}

0 commit comments

Comments
 (0)