From ac4571ea522320ca5c1c17e03b668e8070e69c5b Mon Sep 17 00:00:00 2001 From: Craig Walls Date: Fri, 25 Oct 2024 21:32:22 -0600 Subject: [PATCH] Fix StabilityAI options merge precedence This fix ensures proper inheritance of defaults while maintaining the correct precedence order for both generic and StabilityAI-specific options. Added tests to verify the merge behavior for runtime options, default options, and generic ImageOptions cases. --- .../ai/stabilityai/StabilityAiImageModel.java | 29 +++- .../ai/stabilityai/api/StabilityAiApi.java | 2 +- .../StabilityAiImageOptionsTests.java | 160 ++++++++++++++++++ 3 files changed, 183 insertions(+), 8 deletions(-) create mode 100644 models/spring-ai-stability-ai/src/test/java/org/springframework/ai/stabilityai/StabilityAiImageOptionsTests.java diff --git a/models/spring-ai-stability-ai/src/main/java/org/springframework/ai/stabilityai/StabilityAiImageModel.java b/models/spring-ai-stability-ai/src/main/java/org/springframework/ai/stabilityai/StabilityAiImageModel.java index 35a980593..cf0686f8c 100644 --- a/models/spring-ai-stability-ai/src/main/java/org/springframework/ai/stabilityai/StabilityAiImageModel.java +++ b/models/spring-ai-stability-ai/src/main/java/org/springframework/ai/stabilityai/StabilityAiImageModel.java @@ -115,14 +115,14 @@ public class StabilityAiImageModel implements ImageModel { /** * Merge runtime and default {@link ImageOptions} to compute the final options to use - * in the request. + * in the request. Protected access for testing purposes, though maybe useful for + * future subclassing as options change. */ - private StabilityAiImageOptions mergeOptions(ImageOptions runtimeOptions, StabilityAiImageOptions defaultOptions) { + StabilityAiImageOptions mergeOptions(ImageOptions runtimeOptions, StabilityAiImageOptions defaultOptions) { if (runtimeOptions == null) { return defaultOptions; } - - return StabilityAiImageOptions.builder() + StabilityAiImageOptions.Builder builder = StabilityAiImageOptions.builder() // Handle portable image options .withModel(ModelOptionsUtils.mergeOption(runtimeOptions.getModel(), defaultOptions.getModel())) .withN(ModelOptionsUtils.mergeOption(runtimeOptions.getN(), defaultOptions.getN())) @@ -131,14 +131,29 @@ public class StabilityAiImageModel implements ImageModel { .withWidth(ModelOptionsUtils.mergeOption(runtimeOptions.getWidth(), defaultOptions.getWidth())) .withHeight(ModelOptionsUtils.mergeOption(runtimeOptions.getHeight(), defaultOptions.getHeight())) .withStylePreset(ModelOptionsUtils.mergeOption(runtimeOptions.getStyle(), defaultOptions.getStyle())) - // Handle Stability AI specific image options + // Always set the stability-specific defaults .withCfgScale(defaultOptions.getCfgScale()) .withClipGuidancePreset(defaultOptions.getClipGuidancePreset()) .withSampler(defaultOptions.getSampler()) .withSeed(defaultOptions.getSeed()) .withSteps(defaultOptions.getSteps()) - .withStylePreset(defaultOptions.getStylePreset()) - .build(); + .withStylePreset(defaultOptions.getStylePreset()); + if (runtimeOptions instanceof StabilityAiImageOptions) { + StabilityAiImageOptions stabilityOptions = (StabilityAiImageOptions) runtimeOptions; + // Handle Stability AI specific image options + builder + .withCfgScale( + ModelOptionsUtils.mergeOption(stabilityOptions.getCfgScale(), defaultOptions.getCfgScale())) + .withClipGuidancePreset(ModelOptionsUtils.mergeOption(stabilityOptions.getClipGuidancePreset(), + defaultOptions.getClipGuidancePreset())) + .withSampler(ModelOptionsUtils.mergeOption(stabilityOptions.getSampler(), defaultOptions.getSampler())) + .withSeed(ModelOptionsUtils.mergeOption(stabilityOptions.getSeed(), defaultOptions.getSeed())) + .withSteps(ModelOptionsUtils.mergeOption(stabilityOptions.getSteps(), defaultOptions.getSteps())) + .withStylePreset(ModelOptionsUtils.mergeOption(stabilityOptions.getStylePreset(), + defaultOptions.getStylePreset())); + } + + return builder.build(); } } diff --git a/models/spring-ai-stability-ai/src/main/java/org/springframework/ai/stabilityai/api/StabilityAiApi.java b/models/spring-ai-stability-ai/src/main/java/org/springframework/ai/stabilityai/api/StabilityAiApi.java index 5b3b7f546..8c30e3034 100644 --- a/models/spring-ai-stability-ai/src/main/java/org/springframework/ai/stabilityai/api/StabilityAiApi.java +++ b/models/spring-ai-stability-ai/src/main/java/org/springframework/ai/stabilityai/api/StabilityAiApi.java @@ -100,7 +100,7 @@ public class StabilityAiApi { @JsonProperty("cfg_scale") Float cfgScale, @JsonProperty("clip_guidance_preset") String clipGuidancePreset, @JsonProperty("sampler") String sampler, @JsonProperty("samples") Integer samples, @JsonProperty("seed") Long seed, @JsonProperty("steps") Integer steps, - @JsonProperty("style_present") String stylePreset) { + @JsonProperty("style_preset") String stylePreset) { public static Builder builder() { return new Builder(); diff --git a/models/spring-ai-stability-ai/src/test/java/org/springframework/ai/stabilityai/StabilityAiImageOptionsTests.java b/models/spring-ai-stability-ai/src/test/java/org/springframework/ai/stabilityai/StabilityAiImageOptionsTests.java new file mode 100644 index 000000000..0cd242324 --- /dev/null +++ b/models/spring-ai-stability-ai/src/test/java/org/springframework/ai/stabilityai/StabilityAiImageOptionsTests.java @@ -0,0 +1,160 @@ +/* + * Copyright 2024 - 2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.ai.stabilityai; + +import org.junit.jupiter.api.Test; +import org.springframework.ai.image.ImageOptions; +import org.springframework.ai.stabilityai.api.StabilityAiApi; +import org.springframework.ai.stabilityai.api.StabilityAiImageOptions; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; + +public class StabilityAiImageOptionsTests { + + @Test + void shouldPreferRuntimeOptionsOverDefaultOptions() { + + StabilityAiApi stabilityAiApi = mock(StabilityAiApi.class); + // Default options + StabilityAiImageOptions defaultOptions = StabilityAiImageOptions.builder() + .withN(1) + .withModel("default-model") + .withWidth(512) + .withHeight(512) + .withResponseFormat("image/png") + .withCfgScale(7.0f) + .withClipGuidancePreset("FAST_BLUE") + .withSampler("DDIM") + .withSeed(1234L) + .withSteps(30) + .withStylePreset("3d-model") + .build(); + + // Runtime options with different values + StabilityAiImageOptions runtimeOptions = StabilityAiImageOptions.builder() + .withN(2) + .withModel("runtime-model") + .withWidth(1024) + .withHeight(768) + .withResponseFormat("application/json") + .withCfgScale(14.0f) + .withClipGuidancePreset("FAST_GREEN") + .withSampler("DDPM") + .withSeed(5678L) + .withSteps(50) + .withStylePreset("anime") + .build(); + + StabilityAiImageModel imageModel = new StabilityAiImageModel(stabilityAiApi, defaultOptions); + + StabilityAiImageOptions mergedOptions = imageModel.mergeOptions(runtimeOptions, defaultOptions); + + assertThat(mergedOptions).satisfies(options -> { + // Verify that all options match the runtime values, not the defaults + assertThat(options.getN()).isEqualTo(2); + assertThat(options.getModel()).isEqualTo("runtime-model"); + assertThat(options.getWidth()).isEqualTo(1024); + assertThat(options.getHeight()).isEqualTo(768); + assertThat(options.getResponseFormat()).isEqualTo("application/json"); + assertThat(options.getCfgScale()).isEqualTo(14.0f); + assertThat(options.getClipGuidancePreset()).isEqualTo("FAST_GREEN"); + assertThat(options.getSampler()).isEqualTo("DDPM"); + assertThat(options.getSeed()).isEqualTo(5678L); + assertThat(options.getSteps()).isEqualTo(50); + assertThat(options.getStylePreset()).isEqualTo("anime"); + }); + } + + @Test + void shouldUseDefaultOptionsWhenRuntimeOptionsAreNull() { + + StabilityAiApi stabilityAiApi = mock(StabilityAiApi.class); + StabilityAiImageOptions defaultOptions = StabilityAiImageOptions.builder() + .withN(1) + .withModel("default-model") + .withCfgScale(7.0f) + .build(); + + StabilityAiImageModel imageModel = new StabilityAiImageModel(stabilityAiApi, defaultOptions); + + StabilityAiImageOptions mergedOptions = imageModel.mergeOptions(null, defaultOptions); + + assertThat(mergedOptions).satisfies(options -> { + assertThat(options.getN()).isEqualTo(1); + assertThat(options.getModel()).isEqualTo("default-model"); + assertThat(options.getCfgScale()).isEqualTo(7.0f); + }); + } + + @Test + void shouldHandleGenericImageOptionsCorrectly() { + + StabilityAiApi stabilityAiApi = mock(StabilityAiApi.class); + StabilityAiImageOptions defaultOptions = StabilityAiImageOptions.builder() + .withN(1) + .withModel("default-model") + .withWidth(512) + .withCfgScale(7.0f) + .build(); + + // Create a non-StabilityAi ImageOptions implementation + ImageOptions genericOptions = new ImageOptions() { + @Override + public Integer getN() { + return 2; + } + + @Override + public String getModel() { + return "generic-model"; + } + + @Override + public Integer getWidth() { + return 1024; + } + + @Override + public Integer getHeight() { + return null; + } + + @Override + public String getResponseFormat() { + return null; + } + + @Override + public String getStyle() { + return null; + } + }; + + StabilityAiImageModel imageModel = new StabilityAiImageModel(stabilityAiApi, defaultOptions); + + StabilityAiImageOptions mergedOptions = imageModel.mergeOptions(genericOptions, defaultOptions); + + // Generic options should override defaults + assertThat(mergedOptions.getN()).isEqualTo(2); + assertThat(mergedOptions.getModel()).isEqualTo("generic-model"); + assertThat(mergedOptions.getWidth()).isEqualTo(1024); + + // Stability-specific options should retain default values + assertThat(mergedOptions.getCfgScale()).isEqualTo(7.0f); + } + +}