feat(openai): add per-capability default model config via Builder#1925
feat(openai): add per-capability default model config via Builder#1925Buktal wants to merge 10 commits into
Conversation
Replace the single defaultModelName field with per-capability defaults (defaultVisionModel, defaultImageGenModel, defaultTtsModel, defaultSttModel) via a new Builder, matching OpenAIChatModel.Builder conventions. Existing constructors remain for backward compatibility. Default values unchanged: gpt-4o, dall-e-3, tts-1, whisper-1. Closes agentscope-ai#1924
When the outer timeout cancels the subscription, boundedElastic interrupts the sleeping thread via Future.cancel(true). The old code wrapped both the sleep and the file write in one try-catch, so the interrupt silently skipped the write. Split them so the write always executes even if sleep is interrupted. Closes agentscope-ai#1924 (part of PR agentscope-ai#1925)
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Nice work on the Builder — backward compat is well handled with the old constructor delegating to the new private one. One thing I noticed: the builder tests only do |
…igured default The optional model param was documented with OpenAI-specific examples (e.g. 'dall-e-3'), which lured the LLM into passing a model name and bypassing the configured defaultXxxModel. Remove the param from all four capabilities (text-to-image / image-to-text / text-to-audio / audio-to-text) so the configured default is always used. Adapt existing tests to the new signatures.
AgentScopeJavaBot
left a comment
There was a problem hiding this comment.
🤖 AI Review
Adds Builder pattern for OpenAIMultiModalTool with per-capability default models. Behavioral change: old 4-param constructor now only sets vision model. Consider marking old constructor @deprecated.
(inline comments could not be attached — line numbers fell outside PR hunks. See archived report.)
AgentScope-Java Version
2.0.0-SNAPSHOT
Description
1. Per-capability default model via Builder
OpenAIMultiModalToolpreviously had a singledefaultModelNamefield that only applied toopenai_image_to_text. The other three tools had their default models hardcoded.This adds a
BuildermatchingOpenAIChatModel.Builderconventions, exposing per-capability defaults:defaultVisionModel— image-to-text (falls back togpt-4o)defaultImageGenModel— text-to-image (falls back todall-e-3)defaultTtsModel— text-to-audio (falls back totts-1)defaultSttModel— audio-to-text (falls back towhisper-1)baseUrl— custom endpoint (optional)When not set via Builder, the existing hardcoded values are used — no change to current behavior.
2. Flaky test fix
SubAgentToolTimeoutRetryIntegrationTest.oldAgentIsStoppedwas flaky: when the timeout cancelled the subscription,boundedElasticinterrupted the sleeping thread viaFuture.cancel(true). The old code wrapped bothThread.sleepandFiles.writeStringin one try-catch, so the interrupt silently skipped the write, making the assertion fail. Fixed by splitting them.Related:
Checklist
mvn spotless:applymvn test)