- Refactor the builder methods to remove
with
as the prefix. - Introduce new methods with updated naming conventions.
- Deprecate the existing
with*
methods to maintain backward compatibility.
Comment From: ilayaperumalg
Hi @apappascs , Thanks for the PR. We also need the documentation to be updated accordingly.
Comment From: apappascs
Hi @apappascs , Thanks for the PR. We also need the documentation to be updated accordingly.
Thank you for the feedback, @ilayaperumalg. I have updated the documentation and rebased the branch with main.
I also have a suggestion: I think it would make sense to introduce an abstract class to implement the chat options and avoid code duplication across all ChatOptions model classes.
Here’s an example for discussion:
/**
* Abstract base class for {@link ChatOptions}, providing common implementation for its methods.
*/
public abstract class AbstractChatOptions implements ChatOptions {
@JsonProperty("model")
private final String model;
@JsonProperty("frequency_penalty")
private final Double frequencyPenalty;
@JsonProperty("max_tokens")
private final Integer maxTokens;
@JsonProperty("presence_penalty")
private final Double presencePenalty;
@JsonProperty("stop_sequences")
private final List<String> stopSequences;
@JsonProperty("temperature")
private final Double temperature;
@JsonProperty("top_k")
private final Integer topK;
@JsonProperty("top_p")
private final Double topP;
protected AbstractChatOptions(@Nullable String model,
@Nullable Double frequencyPenalty,
@Nullable Integer maxTokens,
@Nullable Double presencePenalty,
@Nullable List<String> stopSequences,
@Nullable Double temperature,
@Nullable Integer topK,
@Nullable Double topP) {
this.model = model;
this.frequencyPenalty = frequencyPenalty;
this.maxTokens = maxTokens;
this.presencePenalty = presencePenalty;
this.stopSequences = stopSequences;
this.temperature = temperature;
this.topK = topK;
this.topP = topP;
}
@Override
public String getModel() {
return model;
}
@Override
public Double getFrequencyPenalty() {
return frequencyPenalty;
}
@Override
@Nullable
public Integer getMaxTokens() {
return maxTokens;
}
@Override
public Double getPresencePenalty() {
return presencePenalty;
}
@Override
public List<String> getStopSequences() {
return stopSequences;
}
@Override
public Double getTemperature() {
return temperature;
}
@Override
public Integer getTopK() {
return topK;
}
@Override
@Nullable
public Double getTopP() {
return topP;
}
@Override
public abstract ChatOptions copy();
/**
* Generic Abstract Builder for {@link AbstractChatOptions}.
* Ensures fluent API in subclasses with proper typing.
*/
public abstract static class Builder<T extends Builder<T>> implements ChatOptions.Builder {
protected String model;
protected Double frequencyPenalty;
protected Integer maxTokens;
protected Double presencePenalty;
protected List<String> stopSequences;
protected Double temperature;
protected Integer topK;
protected Double topP;
protected abstract T self();
@Override
public T model(String model) {
this.model = model;
return self();
}
@Override
public T frequencyPenalty(Double frequencyPenalty) {
this.frequencyPenalty = frequencyPenalty;
return self();
}
@Override
public T maxTokens(Integer maxTokens) {
this.maxTokens = maxTokens;
return self();
}
@Override
public T presencePenalty(Double presencePenalty) {
this.presencePenalty = presencePenalty;
return self();
}
@Override
public T stopSequences(List<String> stopSequences) {
this.stopSequences = stopSequences;
return self();
}
@Override
public T temperature(Double temperature) {
this.temperature = temperature;
return self();
}
@Override
public T topK(Integer topK) {
this.topK = topK;
return self();
}
@Override
public T topP(Double topP) {
this.topP = topP;
return self();
}
@Override
public abstract AbstractChatOptions build();
}
}
with concrete implementation e.g.:
public class WatsonxAiChatOptions extends AbstractChatOptions {
@JsonProperty("decoding_method")
private final String decodingMethod;
@JsonProperty("max_new_tokens")
private final Integer maxNewTokens;
@JsonProperty("min_new_tokens")
private final Integer minNewTokens;
@JsonProperty("repetition_penalty")
private final Double repetitionPenalty;
@JsonProperty("random_seed")
private final Integer randomSeed;
private WatsonxAiChatOptions(Builder builder) {
super(builder.model, builder.frequencyPenalty, builder.maxTokens,
builder.presencePenalty, builder.stopSequences, builder.temperature,
builder.topK, builder.topP);
this.decodingMethod = builder.decodingMethod;
this.maxNewTokens = builder.maxNewTokens;
this.minNewTokens = builder.minNewTokens;
this.repetitionPenalty = builder.repetitionPenalty;
this.randomSeed = builder.randomSeed;
}
@Override
public WatsonxAiChatOptions copy() {
return builder()
.model(getModel())
.frequencyPenalty(getFrequencyPenalty())
.maxTokens(getMaxTokens())
.presencePenalty(getPresencePenalty())
.stopSequences(getStopSequences())
.temperature(getTemperature())
.topK(getTopK())
.topP(getTopP())
.decodingMethod(this.decodingMethod)
.maxNewTokens(this.maxNewTokens)
.minNewTokens(this.minNewTokens)
.repetitionPenalty(this.repetitionPenalty)
.randomSeed(this.randomSeed)
.build();
}
public static class Builder extends AbstractChatOptions.Builder<Builder> {
private String decodingMethod;
private Integer maxNewTokens;
private Integer minNewTokens;
private Double repetitionPenalty;
private Integer randomSeed;
@Override
protected Builder self() {
return this;
}
public Builder decodingMethod(@Nullable String decodingMethod) {
this.decodingMethod = decodingMethod;
return self();
}
public Builder maxNewTokens(@Nullable Integer maxNewTokens) {
this.maxNewTokens = maxNewTokens;
return self();
}
public Builder minNewTokens(@Nullable Integer minNewTokens) {
this.minNewTokens = minNewTokens;
return self();
}
public Builder repetitionPenalty(@Nullable Double repetitionPenalty) {
this.repetitionPenalty = repetitionPenalty;
return self();
}
public Builder randomSeed(@Nullable Integer randomSeed) {
this.randomSeed = randomSeed;
return self();
}
@Override
public WatsonxAiChatOptions build() {
return new WatsonxAiChatOptions(this);
}
public static Builder from(WatsonxAiChatOptions options) {
return new Builder()
.model(options.getModel())
.frequencyPenalty(options.getFrequencyPenalty())
.maxTokens(options.getMaxTokens())
.presencePenalty(options.getPresencePenalty())
.stopSequences(options.getStopSequences())
.temperature(options.getTemperature())
.topK(options.getTopK())
.topP(options.getTopP())
.decodingMethod(options.decodingMethod)
.maxNewTokens(options.maxNewTokens)
.minNewTokens(options.minNewTokens)
.repetitionPenalty(options.repetitionPenalty)
.randomSeed(options.randomSeed);
}
}
public static Builder builder() {
return new Builder();
}
public static Builder builder(WatsonxAiChatOptions options) {
return Builder.from(options);
}
}
Likewise, we already have similar patterns in the codebase, like AbstractObservationVectorStore
implementing VectorStore
and AbstractVectorStoreBuilder
implementing VectorStore.Builder
. Introducing this abstraction here would help keep things consistent and reduce redundancy.
Let me know what you think!
Comment From: ilayaperumalg
@apappascs That's a good suggestion and I agree with you.
Comment From: apappascs
@apappascs That's a good suggestion and I agree with you.
@ilayaperumalg Thanks! I’d be happy to take this task and create a new PR to introduce the abstraction. I’ll work on this and submit it soon.
Comment From: ilayaperumalg
@apappascs Great! thank you for your interest and contributions!
Comment From: ilayaperumalg
Rebased and merged as de29df7c5