Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Spring Boot Project Starter for Google Gemini API model - ChatLangauge, Streaming model and Embedding Model #74

Open
wants to merge 40 commits into
base: main
Choose a base branch
from

Conversation

Suhas-Koheda
Copy link

This pull request is related to the issue #2103 of langchain4j project

@Suhas-Koheda
Copy link
Author

@langchain4j hey can you please have a look at this

Copy link
Owner

@langchain4j langchain4j left a comment

Choose a reason for hiding this comment

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

@Suhas-Koheda thanks a lot!

langchain4j-google-ai-gemini/pom.xml Outdated Show resolved Hide resolved
langchain4j-google-ai-gemini/pom.xml Outdated Show resolved Hide resolved
.topP(chatModelProperties.getTopP())
.topK(chatModelProperties.getTopK())
.maxOutputTokens(chatModelProperties.getMaxOutputTokens())
.responseFormat(ResponseFormat.JSON)
Copy link
Owner

Choose a reason for hiding this comment

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

why is it json?

Copy link

@franglopez franglopez Nov 14, 2024

Choose a reason for hiding this comment

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

Hi. Related to your question. AutoConfig it's not injecting logging properties and json format from the properties -> https://docs.langchain4j.dev/tutorials/logging/ is not working when this AutoConfig is used. I will wait until this PR is merged to create a PR to avoid conflicts or If you want to add this configuration in this PR. Whatever you consider better.
Thank you!

Copy link
Author

@Suhas-Koheda Suhas-Koheda Nov 14, 2024

Choose a reason for hiding this comment

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

Hey! can you provide me with some references
As i did not find the logging used in other starters
Or maybe i overlooked
Thank you!
@franglopez

.topP(chatModelProperties.getTopP())
.topK(chatModelProperties.getTopK())
.maxOutputTokens(chatModelProperties.getMaxOutputTokens())
.responseFormat(ResponseFormat.JSON)
Copy link
Owner

Choose a reason for hiding this comment

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

same here

Copy link
Author

Choose a reason for hiding this comment

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

In the docs it was using JSON format itself
And also mostly in spring boot we will prefer to transfer data in the form of json right i did continue without thinking?
If not JSON then what's the format to be used?
And if you don't mind... why?

Copy link
Owner

Choose a reason for hiding this comment

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

Here we need to configure only those properties that users have defined themselves, so please avoid setting default values.
The most common format is TEXT (String). JSON is used only for structured outputs, but there user has to explicitly specify the Java class.

@Suhas-Koheda
Copy link
Author

@langchain4j also in the timeout values i had doubt
Will it be set by the user? Or should i keep the condition if its not set then set to 0

At present i have hardcoded used 60 secs

@langchain4j
Copy link
Owner

@Suhas-Koheda you can see how timeouts are handled in OpenAI starter here, the same for .logRequests(chatModelProperties.logRequests()).

@ddobrin
Copy link

ddobrin commented Nov 14, 2024

@Suhas-Koheda
when you are looking at logging requests and responses, please be aware that the GogleAiGeminiChatModel in the module langchain4j-google-ai-gemini does not make a distinction in the constructor between logRequests() and log Responses() and uses a single Boolean to enable/disable the logging functionality link

@Suhas-Koheda
Copy link
Author

Suhas-Koheda commented Nov 14, 2024

Yeah sure I'll take care of it!
Thank you!
@ddobrin

@Suhas-Koheda
Copy link
Author

@ddobrin hey for the response format i have kept the type of prop as ResponseFormat only which is user defined - not a primitive type?
is this ok ? or does it cause any problem?

@Suhas-Koheda
Copy link
Author

@langchain4j
Hey! Hi!
I've completed writing tests and both of the tests passed!
Also review the code once and let me know if any changes are to be made :)

@ddobrin
Copy link

ddobrin commented Nov 16, 2024

@Suhas-Koheda on a 📱 let me come back to it at the beginning of the week

@ddobrin
Copy link

ddobrin commented Nov 18, 2024

Hi @Suhas-Koheda
could I suggest that you add:

  • in the ChatModelProperties: maxRetries, timeout

  • in the Properties a @NestedConfigurationProperties with GeminiSafetySetting and one for GeminiFunctionCallingConfig

  • in the GeminiSafetySetting 2 more properties: GeminiHarmCategory and GeminiHarmBlockThreshold, and they would have to be comma-separated values

  • tests for this, with acceptable values from here and here

  • for GeminiFunctionCallingConfig, it should allow a list of function names, comma-separated, a GeminiMode and 2 properties for allowCodeExecution, includeCodeExecutionOutput

@ddobrin
Copy link

ddobrin commented Nov 18, 2024

@Suhas-Koheda - if you find this useful, and to test the starter, before a merge, I have this sample on Langchain4J Function Calling with Google AI for Developers and Gemini models available, on 0.37 SNAPSHOT

@Suhas-Koheda
Copy link
Author

I'll look into it and make the suggested changes !

@Suhas-Koheda
Copy link
Author

@ddobrin hey th egemini safe setting function safetySettings has Map<GeminiHarmCategory, GeminiHarmBlockThreshold> safetySettingMap as args so i will be taking the args as two different properties and will be passing into the function as args
is it ok?
Hey can you check the recent commit and
one more thing
the arguments in streamign chat model and chatmodel differs in arguments
can you please tell me how to proceed further?

thank you!

@ddobrin
Copy link

ddobrin commented Nov 19, 2024

On mobile.

  • map is a <k,v> pair of harm category respectively threshold
  • the args are the same if I remember correctly, just the maxRetries was in a different position (again, on mobile)

Could you please add to the IT class, in order to have full coverage of all configured parameters?

@Suhas-Koheda
Copy link
Author

Suhas-Koheda commented Nov 19, 2024

@ddobrin i have written tests for the chat language model with updated properties and it runs!

the inconsistency in code i was mentioning about
-> public GoogleAiGeminiStreamingChatModelBuilder safetySettings(final List safetySettings) {
this.safetySettings = safetySettings;
return this;
}
-> public GoogleAiGeminiChatModelBuilder safetySettings(Map<GeminiHarmCategory, GeminiHarmBlockThreshold> safetySettingMap) {
this.safetySettings = (List)safetySettingMap.entrySet().stream().map((entry) -> new GeminiSafetySetting((GeminiHarmCategory)entry.getKey(), (GeminiHarmBlockThreshold)entry.getValue())).collect(Collectors.toList());
return this;
}

one method has list args and the other has map args
i continued with map args and didnot write tests for streaming model or update the properties in configuration file!

actually i have seen just now
the thing is that this piece of code

       public GoogleAiGeminiChatModelBuilder toolConfig(GeminiMode mode, String... allowedFunctionNames) {
           this.toolConfig = new GeminiFunctionCallingConfig(mode, Arrays.asList(allowedFunctionNames));
           return this;
       }

       public GoogleAiGeminiChatModelBuilder safetySettings(Map<GeminiHarmCategory, GeminiHarmBlockThreshold> safetySettingMap) {
           this.safetySettings = safetySettingMap.entrySet().stream()
               .map(entry -> new GeminiSafetySetting(entry.getKey(), entry.getValue())
               ).collect(Collectors.toList());
           return this;
       }
   }

for the builder method of GoogleAiGeminiChatModelBuilder has been written explicitly which was not written for the GoogleAiGeminiStreamingChatModelBuilder

@Suhas-Koheda
Copy link
Author

@langchain4j hey i have written tests for embedding model hope they are correct and suggest if any changes
@ddobrin hey the config and test for embedding model is done, also would like you to confirm the ChatLanguageModel config and test with safety setting and further and checkout the StreamingChatLanguageModel
Thank you!

@Suhas-Koheda Suhas-Koheda changed the title Add Spring Boot Project Starter for Google Gemini API model - ChatLangauge and Streaming model Add Spring Boot Project Starter for Google Gemini API model - ChatLangauge, Streaming model and Embedding Model Nov 20, 2024
Copy link
Owner

@langchain4j langchain4j left a comment

Choose a reason for hiding this comment

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

@Suhas-Koheda thanks a lot!

@Suhas-Koheda
Copy link
Author

@langchain4j hey i have resolved the issues and also can you please check the issue with streaming chat model
since we dont have any builder for safety settings for streaming chat model

@langchain4j
Copy link
Owner

@Suhas-Koheda thank you!
@ddobrin could you please help with the review and questions?

@ddobrin
Copy link

ddobrin commented Nov 27, 2024

@langchain4j - yes, Friday. Task overload until then

@ddobrin
Copy link

ddobrin commented Nov 29, 2024

Hi @Suhas-Koheda
I have looked at the code and can run your tests.

Re: streaming chat model not having a builder for safety settings

I have revisited streaming, as well as the Vertex AI Gemini model. While that is missing, rather than making the starter config different between chat and streaming chat, can I suggest that we add this small missing bit in the /langchain4j/langchain4j-google-ai-gemini module first, and the starter would then come after.

What do you think?

@Suhas-Koheda
Copy link
Author

Suhas-Koheda commented Nov 29, 2024

Hi @Suhas-Koheda
I have looked at the code and can run your tests.

Yeah! Happy to hear that!

Re: streaming chat model not having a builder for safety settings

I have revisited streaming, as well as the Vertex AI Gemini model. While that is missing, rather than making the starter config different between chat and streaming chat, can I suggest that we add this small missing bit in the /langchain4j/langchain4j-google-ai-gemini module first, and the starter would then come after.

What do you think?

Yeah sure! It works!
Actually i can work on that if you are okay with it!

@ddobrin
Copy link

ddobrin commented Nov 30, 2024

If you like to add it and send a PR, that would be great

@Suhas-Koheda
Copy link
Author

@ddobrin hey i have opened a pr for the above in the main repo and also i have chnaged the tests here also to support the same !
langchain4j/langchain4j#2217

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.

4 participants