refactor: refactor BaseLlmFlow to make code more readable #310

praveen97uma · 2025-07-31T06:45:42Z

changed optional handling to be more functional
moved one of the lambdas to a private method in the same class to reduce the size of the function

google-cla · 2025-07-31T06:45:46Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

gemini-code-assist

Summary of Changes

Hello @praveen97uma, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request focuses on refactoring the BaseLlmFlow class to enhance code readability and maintainability. The changes involve adopting a more functional approach for handling Optional values and extracting a substantial inline lambda into a dedicated private method, thereby simplifying the main runLive method.

Highlights

Refactoring: Improved Optional handling within the runLive method by replacing isPresent() ? get() : ... with the more functional and concise orElse(...) pattern.
Code Organization: Extracted a large, complex lambda expression responsible for sending history data into a new private helper method named sendHistory. This significantly reduces the size and improves the readability of the runLive method.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request refactors BaseLlmFlow to improve readability by extracting a lambda into a private method and using a more functional style for handling Optional values.

My review includes a critical fix for the Optional handling, which currently uses orElse instead of orElseGet, potentially causing runtime exceptions and performance issues. I've also suggested improvements to the newly extracted method to reduce code duplication and improve formatting for better maintainability.

gemini-code-assist · 2025-07-31T06:47:16Z

core/src/main/java/com/google/adk/flows/llmflows/BaseLlmFlow.java

+              BaseLlm llm = agent.resolvedModel().model()
+                      .orElse(LlmRegistry.getLlm(agent.resolvedModel().modelName()
+                              .orElseThrow()));


The use of orElse here can lead to a runtime exception and unnecessary work. orElse evaluates its argument eagerly, meaning LlmRegistry.getLlm(...) will be executed even if agent.resolvedModel().model() is present.

If agent.resolvedModel().model() is present but agent.resolvedModel().modelName() is not, orElseThrow() will throw a NoSuchElementException, causing a crash.

To fix this, you should use orElseGet, which evaluates its Supplier argument lazily (only when the Optional is empty).

Suggested change

BaseLlm llm = agent.resolvedModel().model()

.orElse(LlmRegistry.getLlm(agent.resolvedModel().modelName()

.orElseThrow()));

BaseLlm llm = agent.resolvedModel().model()

.orElseGet(() -> LlmRegistry.getLlm(agent.resolvedModel().modelName()

.orElseThrow()));

gemini-code-assist · 2025-07-31T06:47:16Z

core/src/main/java/com/google/adk/flows/llmflows/BaseLlmFlow.java

+    private static Completable sendHistory(final InvocationContext invocationContext, final BaseLlmConnection connection, final LlmRequest llmRequestAfterPreprocess, final String eventIdForSendData) {
+        Span sendDataSpan =
+            Telemetry.getTracer().spanBuilder("send_data").startSpan();
+        try (Scope scope = sendDataSpan.makeCurrent()) {
+            return connection
+                    .sendHistory(llmRequestAfterPreprocess.contents())
+                    .doOnComplete(
+                            () -> {
+                                try (Scope innerScope = sendDataSpan.makeCurrent()) {
+                                    Telemetry.traceSendData(
+                                            invocationContext,
+                                            eventIdForSendData,
+                                            llmRequestAfterPreprocess.contents());
+                                }
+                            })
+                    .doOnError(
+                            error -> {
+                                sendDataSpan.setStatus(
+                                        StatusCode.ERROR, error.getMessage());
+                                sendDataSpan.recordException(error);
+                                try (Scope innerScope = sendDataSpan.makeCurrent()) {
+                                    Telemetry.traceSendData(
+                                            invocationContext,
+                                            eventIdForSendData,
+                                            llmRequestAfterPreprocess.contents());
+                                }
+                            })
+                    .doFinally(sendDataSpan::end);
+        }
+    }


This new helper method is a good step towards improving readability. However, there are a few areas for improvement:

Code Duplication: The Telemetry.traceSendData(...) call and its surrounding try-with-resources block are duplicated in doOnComplete and doOnError. You can consolidate this logic using doOnEvent to handle both success and error cases in one place.

Line Length: The method signature exceeds the standard 100-character line limit, making it harder to read. It should be wrapped for better readability.

Formatting: The indentation for the chained method calls is inconsistent. Following a standard indentation style (e.g., +4 spaces for each level of continuation indent) would improve readability.

private static Completable sendHistory( final InvocationContext invocationContext, final BaseLlmConnection connection, final LlmRequest llmRequestAfterPreprocess, final String eventIdForSendData) { Span sendDataSpan = Telemetry.getTracer().spanBuilder("send_data").startSpan(); try (Scope scope = sendDataSpan.makeCurrent()) { return connection .sendHistory(llmRequestAfterPreprocess.contents()) .doOnEvent( (result, error) -> { if (error != null) { sendDataSpan.setStatus(StatusCode.ERROR, error.getMessage()); sendDataSpan.recordException(error); } try (Scope innerScope = sendDataSpan.makeCurrent()) { Telemetry.traceSendData( invocationContext, eventIdForSendData, llmRequestAfterPreprocess.contents()); } }) .doFinally(sendDataSpan::end); } }

Poggecci · 2025-08-11T17:29:28Z

Hey, thank you for your contribution!

Would you mind updating your branch? we've enabled formatting on build so this should help you get your files formatted & pass the CI.

Poggecci

Changes look good to me. Need the files to be formatted so CI passes. Once passing, good to merge these changes in

gemini-code-assist bot reviewed Jul 31, 2025

View reviewed changes

refactor: refactor BaseLlmFlow to make code more readable

acb6b7d

praveen97uma force-pushed the refactor_base_llm_flow branch from c6c51f1 to acb6b7d Compare July 31, 2025 06:59

vorburger assigned Poggecci Aug 11, 2025

vorburger requested a review from Poggecci August 11, 2025 17:16

Poggecci requested changes Aug 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: refactor BaseLlmFlow to make code more readable #310

refactor: refactor BaseLlmFlow to make code more readable #310

Uh oh!

praveen97uma commented Jul 31, 2025

Uh oh!

google-cla bot commented Jul 31, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Jul 31, 2025

Uh oh!

gemini-code-assist bot Jul 31, 2025

Uh oh!

Poggecci commented Aug 11, 2025

Uh oh!

Poggecci left a comment

Uh oh!

Uh oh!

refactor: refactor BaseLlmFlow to make code more readable #310

Are you sure you want to change the base?

refactor: refactor BaseLlmFlow to make code more readable #310

Uh oh!

Conversation

praveen97uma commented Jul 31, 2025

Uh oh!

google-cla bot commented Jul 31, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

Poggecci commented Aug 11, 2025

Uh oh!

Poggecci left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!