Skip to content

Define default buildRequestPrompt method in ChatModel interface#5841

Open
nicolaskrier wants to merge 1 commit intospring-projects:mainfrom
nicolaskrier:default-build-request-prompt
Open

Define default buildRequestPrompt method in ChatModel interface#5841
nicolaskrier wants to merge 1 commit intospring-projects:mainfrom
nicolaskrier:default-build-request-prompt

Conversation

@nicolaskrier
Copy link
Copy Markdown
Contributor

@nicolaskrier nicolaskrier commented Apr 20, 2026

Description

  • Remove existing implementation when possible,
  • Verify built ChatOptions when required by the vendor,
  • Use different ChatOptions types in OllamaChatModelIT,
  • Polish OllamaChatRequestTests unit tests.

Explanations

The goal is to align the behavior of ChatModel.buildRequestPrompt() across vendors where possible, and to avoid casting ChatOptions to vendor-specific types.

Related work

e3e8cca: fix MistralAiChatModel.buildRequestPrompt()

Testing scope

Warning: Integration tests were run locally for Mistral AI and Ollama vendors only.

@nicolaskrier nicolaskrier force-pushed the default-build-request-prompt branch from 1d6748b to ba79e4b Compare April 20, 2026 22:18
@nicolaskrier nicolaskrier marked this pull request as draft April 20, 2026 22:32
@nicolaskrier nicolaskrier force-pushed the default-build-request-prompt branch from ba79e4b to 86ff629 Compare April 21, 2026 22:16
- Remove existing implementation when possible
- Verify built ChatOptions when required by the vendor
- Use different ChatOptions types in OllamaChatModelIT
- Polish OllamaChatRequestTests unit tests

Signed-off-by: Nicolas Krier <7557886+nicolaskrier@users.noreply.github.com>
@nicolaskrier nicolaskrier force-pushed the default-build-request-prompt branch from 86ff629 to 77ad597 Compare April 21, 2026 22:27
assertThat(toolDefinitions.get(0).description()).isEqualTo("Overridden function description");
}

@Disabled("temporary disabled")
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Investigate why this unit test is failing.

@nicolaskrier nicolaskrier marked this pull request as ready for review April 21, 2026 22:54
@nicolaskrier
Copy link
Copy Markdown
Contributor Author

Hi @ilayaperumalg, I was wondering if you could take a quick look at this PR. It is a follow-up of your recent modifications for Mistral AI vendor.

@anuragg-saxenaa
Copy link
Copy Markdown

LGTM. Adds a sensible default buildRequestPrompt() to the ChatModel interface that handles the common pattern (merge defaults + runtime options, validate tool callbacks). Removes redundant overridden implementations from Anthropic, DeepSeek, Bedrock, Google, MiniMax, Mistral — all converged on the same logic, so the default makes sense. Ollama keeps its custom impl (removes model name from runtime options, which is Ollama-specific). API impact: low — default method means no breaking changes for existing implementers. All CI checks green. Ship it.

@redinside-dev
Copy link
Copy Markdown

LGTM. The interface default method approach is clean and avoids vendor-specific casting. The aligned buildRequestPrompt() behavior across Ollama/Mistral makes the interface contract more coherent. Warning: the IT-only scope is noted — worth watching during CI.

Copy link
Copy Markdown

@anuragg-saxenaa anuragg-saxenaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Clean refactor — adds a default method to ChatModel interface and removes matching boilerplate from 8 vendor implementations. Net: +71 / -140 lines across Anthropic, Azure OpenAI, Bedrock Converse, DeepSeek, Google GenAI, MiniMax, Mistral, Ollama, OpenAI. Good pattern. Approved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants