Skip to content

Commit 647f517

Browse files
committed
refactor(ChatRequest): immutable + wither/append pattern
Convert ChatRequest from a mutable fluent builder into a fully immutable value class with a functional "wither / appender" API: ChatRequest.empty() .appendMessage("user", "hi") .withMaxToolRounds(2) .withInferenceCustomizer(p -> p.setSeed(1)); Each modification routes through a single private all-args constructor with one field replaced, allocating a new ChatRequest. The original is never touched, so a caller can safely hold an intermediate request and derive variants without hidden state changes. Notable side effect: this fixes a hidden mutation bug in LlamaModel.chatWithTools(). The previous agent loop mutated the caller's ChatRequest across rounds (adding the assistant turn + each tool result to the same builder). The loop is now rebound to a local `current` that is replaced on every append, preserving the caller's request. Tests: - New ChatRequestTest documents the immutability + value-equality contract in 18 cases across four @nested groups (immutability, equality, validation, JSON-build read-only). - LlamaModelTest and ChatResponseTest call sites migrated from `new ChatRequest()...addMessage(...)` to `ChatRequest.empty()...appendMessage(...)`. SpotBugs: - IMC_NO_EQUALS goal cleared at source (the class is now a true value object with Lombok @EqualsAndHashCode by value). - EI_EXPOSE_REP on getMessages()/getTools() suppressed with explicit rationale: the fields ARE Collections.unmodifiableList views; the test suite verifies that mutation attempts throw UnsupportedOperationException, so the finding is a false positive.
1 parent eb55f58 commit 647f517

6 files changed

Lines changed: 404 additions & 97 deletions

File tree

spotbugs-exclude.xml

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,4 +233,24 @@ SPDX-License-Identifier: MIT
233233
<Bug pattern="OPM_OVERLY_PERMISSIVE_METHOD"/>
234234
</Match>
235235

236+
<!--
237+
ChatRequest is an immutable value class. Its messages/tools fields
238+
are stored as Collections.unmodifiableList(...) views, so the
239+
getters CANNOT actually leak the internal representation: any
240+
attempt to mutate the returned list throws UnsupportedOperationException
241+
(covered by ChatRequestTest.messagesAccessorIsUnmodifiable /
242+
toolsAccessorIsUnmodifiable). SpotBugs flags every "return this.field"
243+
from a non-array reference field as EI_EXPOSE_REP without tracking
244+
whether the field was unmodifiable-wrapped at construction time;
245+
the wrapping is verified by tests, so the finding is a false positive.
246+
-->
247+
<Match>
248+
<Class name="net.ladenthin.llama.ChatRequest"/>
249+
<Bug pattern="EI_EXPOSE_REP"/>
250+
<Or>
251+
<Method name="getMessages"/>
252+
<Method name="getTools"/>
253+
</Or>
254+
</Match>
255+
236256
</FindBugsFilter>

src/main/java/net/ladenthin/llama/ChatRequest.java

Lines changed: 175 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -13,149 +13,253 @@
1313
import java.util.List;
1414
import java.util.Optional;
1515
import java.util.function.Consumer;
16+
import lombok.EqualsAndHashCode;
1617
import lombok.ToString;
1718
import org.jspecify.annotations.Nullable;
1819

1920
/**
20-
* Builder for a typed chat completion call.
21-
* <p>
22-
* Bundles the conversation messages, optional tool definitions, an optional
23-
* {@code tool_choice} hint, and an {@link InferenceParameters} customizer that gets
24-
* applied to the underlying request just before invocation. Built with the fluent
25-
* setters; consumed by {@link LlamaModel#chat(ChatRequest)} and
21+
* Immutable typed chat-completion request, populated through a functional
22+
* "wither / appender" API.
23+
*
24+
* <h2>Design</h2>
25+
*
26+
* <p>The request carries the conversation messages, optional tool definitions,
27+
* an optional {@code tool_choice} hint, and an {@link InferenceParameters}
28+
* customiser applied to the underlying request just before invocation. The
29+
* type is consumed by {@link LlamaModel#chat(ChatRequest)} and
2630
* {@link LlamaModel#chatWithTools(ChatRequest, java.util.Map)}.
27-
* </p>
2831
*
29-
* <p>{@code toString} is generated by Lombok over the request state fields. The
30-
* {@code paramsCustomizer} {@link Consumer} is excluded because lambda equality is
31-
* implementation-defined (compiler-synthesized class identity), not value-shaped,
32-
* and the rendered identity hash is noise in a request dump. {@code equals}/
33-
* {@code hashCode} are intentionally NOT generated: this is a mutable builder, not
34-
* a value object.
32+
* <p>All instances are <b>immutable</b>: every field is {@code final} and the
33+
* stored lists are wrapped with {@link Collections#unmodifiableList(List)}.
34+
* Modification methods return a <b>new</b> {@code ChatRequest} instance with
35+
* the requested change applied; the original is untouched. This makes
36+
* {@code ChatRequest} safe to share across threads and gives it a meaningful
37+
* value-equality semantics (two requests with the same content compare
38+
* equal regardless of identity).
39+
*
40+
* <h2>Construction patterns</h2>
41+
*
42+
* <p>Use {@link #empty()} as the entry point, then chain {@code append*}
43+
* (for list fields) and {@code with*} (for scalar fields):
44+
*
45+
* <pre>{@code
46+
* ChatRequest req = ChatRequest.empty()
47+
* .appendMessage("system", "be terse")
48+
* .appendMessage("user", "two plus two?")
49+
* .withMaxToolRounds(2)
50+
* .withInferenceCustomizer(p -> p.setNPredict(8).setSeed(1));
51+
* }</pre>
52+
*
53+
* <p>Each call allocates a new {@code ChatRequest}. The cost is intentional:
54+
* the API is functional, so a caller can hold an intermediate request and
55+
* derive variants without worrying about hidden state changes.
56+
*
57+
* <h2>Equality</h2>
58+
*
59+
* <p>{@code @EqualsAndHashCode} compares messages, tools, {@code toolChoice},
60+
* and {@code maxToolRounds} by value. The {@code paramsCustomizer}
61+
* {@link Consumer} is <b>excluded</b> from equality: lambdas have
62+
* compiler-synthesised identity equality which is not value-shaped, so
63+
* including it would mean two structurally-identical requests with the same
64+
* customiser source code rarely compare equal — surprising for the typical
65+
* snapshot-testing and caching use cases. The customiser is also excluded
66+
* from {@link ToString} for the same reason (the rendered hash is noise).
3567
*/
3668
@ToString
69+
@EqualsAndHashCode
3770
public final class ChatRequest {
3871

3972
private static final ObjectMapper MAPPER = new ObjectMapper();
4073

41-
private final List<ChatMessage> messages = new ArrayList<ChatMessage>();
42-
private final List<ToolDefinition> tools = new ArrayList<ToolDefinition>();
43-
private @Nullable String toolChoice;
44-
private int maxToolRounds = 8;
74+
/**
75+
* Default {@code maxToolRounds} when the caller does not override it via
76+
* {@link #withMaxToolRounds(int)}. Mirrors the prior mutable builder's default.
77+
*/
78+
public static final int DEFAULT_MAX_TOOL_ROUNDS = 8;
79+
80+
private static final ChatRequest EMPTY = new ChatRequest(
81+
Collections.<ChatMessage>emptyList(),
82+
Collections.<ToolDefinition>emptyList(),
83+
null,
84+
DEFAULT_MAX_TOOL_ROUNDS,
85+
null);
86+
87+
private final List<ChatMessage> messages;
88+
private final List<ToolDefinition> tools;
89+
private final @Nullable String toolChoice;
90+
private final int maxToolRounds;
4591

4692
// Lambda Consumer — toString is the implementation hash, not useful in logs;
47-
// equality is compiler-synthesized class identity, not value-shaped.
93+
// equality is compiler-synthesised class identity, not value-shaped.
4894
@ToString.Exclude
49-
private @Nullable Consumer<InferenceParameters> paramsCustomizer;
95+
@EqualsAndHashCode.Exclude
96+
private final @Nullable Consumer<InferenceParameters> paramsCustomizer;
97+
98+
/**
99+
* All-args constructor. Private because callers should enter via {@link #empty()}
100+
* and derive variants via the {@code append*} / {@code with*} methods. Each
101+
* variant call routes through this same constructor with one field replaced.
102+
*/
103+
private ChatRequest(
104+
List<ChatMessage> messages,
105+
List<ToolDefinition> tools,
106+
@Nullable String toolChoice,
107+
int maxToolRounds,
108+
@Nullable Consumer<InferenceParameters> paramsCustomizer) {
109+
this.messages = messages;
110+
this.tools = tools;
111+
this.toolChoice = toolChoice;
112+
this.maxToolRounds = maxToolRounds;
113+
this.paramsCustomizer = paramsCustomizer;
114+
}
50115

51-
/** Construct an empty request; populate via the setters. */
52-
public ChatRequest() {
53-
// empty
116+
/**
117+
* Returns the empty request — no messages, no tools, {@code toolChoice}
118+
* absent, {@code maxToolRounds} = {@value #DEFAULT_MAX_TOOL_ROUNDS}, no
119+
* customiser. Acts as the starting point for chained derivations.
120+
*
121+
* @return the empty request
122+
*/
123+
public static ChatRequest empty() {
124+
return EMPTY;
54125
}
55126

127+
// -----------------------------------------------------------------------
128+
// List appends — each returns a new request with one entry added.
129+
// -----------------------------------------------------------------------
130+
56131
/**
57-
* Append a message to the conversation.
132+
* Returns a new request with {@code message} appended to the conversation.
133+
*
58134
* @param message the message to append
59-
* @return this builder
135+
* @return a new request with the appended message; this request is unchanged
60136
*/
61-
public ChatRequest addMessage(ChatMessage message) {
62-
messages.add(message);
63-
return this;
137+
public ChatRequest appendMessage(ChatMessage message) {
138+
List<ChatMessage> next = new ArrayList<ChatMessage>(messages.size() + 1);
139+
next.addAll(messages);
140+
next.add(message);
141+
return new ChatRequest(
142+
Collections.unmodifiableList(next),
143+
tools,
144+
toolChoice,
145+
maxToolRounds,
146+
paramsCustomizer);
64147
}
65148

66149
/**
67-
* Convenience for adding a system/user/assistant turn.
68-
* @param role the role
69-
* @param content the content
70-
* @return this builder
150+
* Convenience for {@link #appendMessage(ChatMessage)} that wraps a role +
151+
* content pair into a new {@link ChatMessage} and appends it.
152+
*
153+
* @param role the role (e.g. {@code "system"}, {@code "user"}, {@code "assistant"})
154+
* @param content the message content
155+
* @return a new request with the appended message; this request is unchanged
71156
*/
72-
public ChatRequest addMessage(String role, String content) {
73-
messages.add(new ChatMessage(role, content));
74-
return this;
157+
public ChatRequest appendMessage(String role, String content) {
158+
return appendMessage(new ChatMessage(role, content));
75159
}
76160

77161
/**
78-
* Append a tool definition.
79-
* @param tool the tool definition to expose to the model
80-
* @return this builder
162+
* Returns a new request with {@code tool} added to the tool registry.
163+
*
164+
* @param tool the tool to expose to the model
165+
* @return a new request with the appended tool; this request is unchanged
81166
*/
82-
public ChatRequest addTool(ToolDefinition tool) {
83-
tools.add(tool);
84-
return this;
167+
public ChatRequest appendTool(ToolDefinition tool) {
168+
List<ToolDefinition> next = new ArrayList<ToolDefinition>(tools.size() + 1);
169+
next.addAll(tools);
170+
next.add(tool);
171+
return new ChatRequest(
172+
messages,
173+
Collections.unmodifiableList(next),
174+
toolChoice,
175+
maxToolRounds,
176+
paramsCustomizer);
85177
}
86178

179+
// -----------------------------------------------------------------------
180+
// Scalar withers — each returns a new request with one field replaced.
181+
// -----------------------------------------------------------------------
182+
87183
/**
88-
* Set the {@code tool_choice} hint: typically {@code "auto"}, {@code "none"}, or
89-
* {@code "required"}. Defaults to absent (server default applies).
184+
* Returns a new request with the {@code tool_choice} hint replaced.
90185
*
91-
* @param toolChoice the hint string, or {@code null} to clear
92-
* @return this builder
186+
* @param newToolChoice the hint string (typically {@code "auto"}, {@code "none"}, or
187+
* {@code "required"}), or {@code null} to clear
188+
* @return a new request with the hint replaced; this request is unchanged
93189
*/
94-
public ChatRequest setToolChoice(@Nullable String toolChoice) {
95-
this.toolChoice = toolChoice;
96-
return this;
190+
public ChatRequest withToolChoice(@Nullable String newToolChoice) {
191+
return new ChatRequest(messages, tools, newToolChoice, maxToolRounds, paramsCustomizer);
97192
}
98193

99194
/**
100-
* Set the maximum number of agent-loop rounds for
101-
* {@link LlamaModel#chatWithTools(ChatRequest, java.util.Map)}. A round is one
102-
* model call followed by zero or more tool invocations. Default {@code 8}.
195+
* Returns a new request with the agent-loop round cap replaced.
103196
*
104-
* @param maxToolRounds the round cap (must be positive)
105-
* @return this builder
197+
* @param newMaxToolRounds the new round cap (must be {@code > 0})
198+
* @return a new request with the cap replaced; this request is unchanged
199+
* @throws IllegalArgumentException if {@code newMaxToolRounds} is non-positive
106200
*/
107-
public ChatRequest setMaxToolRounds(int maxToolRounds) {
108-
if (maxToolRounds <= 0) {
109-
throw new IllegalArgumentException("maxToolRounds must be > 0 but was " + maxToolRounds);
201+
public ChatRequest withMaxToolRounds(int newMaxToolRounds) {
202+
if (newMaxToolRounds <= 0) {
203+
throw new IllegalArgumentException(
204+
"maxToolRounds must be > 0 but was " + newMaxToolRounds);
110205
}
111-
this.maxToolRounds = maxToolRounds;
112-
return this;
206+
return new ChatRequest(messages, tools, toolChoice, newMaxToolRounds, paramsCustomizer);
113207
}
114208

115209
/**
116-
* Register a callback that customizes the {@link InferenceParameters} (e.g.
117-
* {@code setNPredict}, {@code setTemperature}) right before each request is sent.
210+
* Returns a new request with the inference-parameter customiser replaced.
118211
*
119-
* @param customizer the customizer; {@code null} clears any prior customizer
120-
* @return this builder
212+
* @param newCustomizer the customiser; {@code null} clears any prior customiser
213+
* @return a new request with the customiser replaced; this request is unchanged
121214
*/
122-
public ChatRequest setInferenceCustomizer(@Nullable Consumer<InferenceParameters> customizer) {
123-
this.paramsCustomizer = customizer;
124-
return this;
215+
public ChatRequest withInferenceCustomizer(@Nullable Consumer<InferenceParameters> newCustomizer) {
216+
return new ChatRequest(messages, tools, toolChoice, maxToolRounds, newCustomizer);
125217
}
126218

219+
// -----------------------------------------------------------------------
220+
// Accessors.
221+
// -----------------------------------------------------------------------
222+
127223
/**
128224
* Messages accessor.
129-
* @return an unmodifiable view of the messages added so far
225+
*
226+
* @return an unmodifiable view of the messages accumulated so far
130227
*/
131228
public List<ChatMessage> getMessages() {
132-
return Collections.unmodifiableList(messages);
229+
return messages;
133230
}
134231

135232
/**
136233
* Tools accessor.
137-
* @return an unmodifiable view of the tool definitions added so far
234+
*
235+
* @return an unmodifiable view of the tool definitions accumulated so far
138236
*/
139237
public List<ToolDefinition> getTools() {
140-
return Collections.unmodifiableList(tools);
238+
return tools;
141239
}
142240

143241
/**
144-
* Tool choice accessor.
242+
* Tool-choice hint accessor.
243+
*
145244
* @return the {@code tool_choice} hint, or {@link Optional#empty()} when unset
146245
*/
147246
public Optional<String> getToolChoice() {
148247
return Optional.ofNullable(toolChoice);
149248
}
150249

151250
/**
152-
* Max rounds accessor.
251+
* Agent-loop round cap accessor.
252+
*
153253
* @return the agent-loop round cap
154254
*/
155255
public int getMaxToolRounds() {
156256
return maxToolRounds;
157257
}
158258

259+
// -----------------------------------------------------------------------
260+
// JSON build helpers — read-only, do not mutate this request.
261+
// -----------------------------------------------------------------------
262+
159263
/**
160264
* Build the OAI-style {@code messages} array as a JSON string. Each entry carries
161265
* role and content; assistant tool-call turns add a {@code tool_calls} array; tool-
@@ -215,7 +319,7 @@ public Optional<String> buildToolsJson() {
215319
}
216320

217321
/**
218-
* Apply the optional customizer to an {@link InferenceParameters} instance.
322+
* Apply the optional customiser to an {@link InferenceParameters} instance.
219323
* Package-private; called by {@link LlamaModel}.
220324
*
221325
* @param params the parameters to mutate

src/main/java/net/ladenthin/llama/LlamaModel.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,8 @@ public ChatResponse chatWithTools(ChatRequest request, java.util.Map<String, Too
563563
throw new IllegalArgumentException("ChatRequest.maxToolRounds must be >= 1 (got " + maxRounds + "); "
564564
+ "chatWithTools always issues at least one chat call.");
565565
}
566-
ChatResponse last = chat(request);
566+
ChatRequest current = request;
567+
ChatResponse last = chat(current);
567568
for (int round = 1; round < maxRounds; round++) {
568569
Optional<ChatMessage> assistantOpt = last.getFirstMessage();
569570
// NOTE: inline !isPresent() here (not compatibilityHelper.isEmpty) so NullAway's
@@ -572,7 +573,7 @@ public ChatResponse chatWithTools(ChatRequest request, java.util.Map<String, Too
572573
return last;
573574
}
574575
ChatMessage assistant = assistantOpt.get();
575-
request.addMessage(assistant);
576+
current = current.appendMessage(assistant);
576577
for (ToolCall call : assistant.getToolCalls()) {
577578
ToolHandler handler = handlers.get(call.getName());
578579
String result;
@@ -588,9 +589,9 @@ public ChatResponse chatWithTools(ChatRequest request, java.util.Map<String, Too
588589
+ "}";
589590
}
590591
}
591-
request.addMessage(ChatMessage.toolResult(call.getId(), result));
592+
current = current.appendMessage(ChatMessage.toolResult(call.getId(), result));
592593
}
593-
last = chat(request);
594+
last = chat(current);
594595
}
595596
return last;
596597
}

0 commit comments

Comments
 (0)