Skip to content

Conversation

@rodrigomalara
Copy link
Contributor

@rodrigomalara rodrigomalara commented Jul 2, 2025

No description provided.

import io.micrometer.observation.contextpropagation.ObservationThreadLocalAccessor;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.ai.vertexai.gemini.api.VertexAiGeminiApi;
Copy link
Contributor

@dev-jonghoonpark dev-jonghoonpark Jul 2, 2025

Choose a reason for hiding this comment

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

Please move this line between lines 87 and 88.

...
import org.springframework.ai.tool.definition.ToolDefinition;
import org.springframework.ai.vertexai.gemini.api.VertexAiGeminiApi;
import org.springframework.ai.vertexai.gemini.common.VertexAiGeminiConstants;
...

this.frequencyPenalty, this.presencePenalty, this.maxOutputTokens, this.model, this.responseMimeType,
this.toolCallbacks, this.toolNames, this.googleSearchRetrieval, this.safetySettings,
this.internalToolExecutionEnabled, this.toolContext);
this.internalToolExecutionEnabled, this.toolContext, this.toolContext, this.logprobs,
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please confirm whether the use of this.toolContext twice is intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! It's not - just removed the extra one.

@@ -0,0 +1,16 @@
package org.springframework.ai.vertexai.gemini.api;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a license header.

@ilayaperumalg
Copy link
Member

@rodrigomalara Thanks for the PR!

@ilayaperumalg ilayaperumalg self-assigned this Jul 2, 2025
@ilayaperumalg ilayaperumalg added this to the 1.1.x milestone Jul 2, 2025
@ilayaperumalg
Copy link
Member

@rodrigomalara Thanks for the PR. Rebased and merged as e557081

@rodrigomalara
Copy link
Contributor Author

@rodrigomalara Thanks for the PR. Rebased and merged as e557081

Thanks @ilayaperumalg - I see the PR was closed instead of merged but it seems the change is going with 1.1.0.

Is it normal procedure or do you have any suggestion so the PR can be merged the next time?

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants