Repository navigation
Conversation
fkapsahili
left a comment
There was a problem hiding this comment.
@glaforge Thanks a lot again for the contribution, and sorry for the long delay! I was quite busy the last few weeks. I love this PR! :)
I ran the build and tests for both ADK modules locally and everything passed. I think this looks really good overall.
A few small suggestions here:
Can you maybe return Collections.unmodifiableMap(map) instead of Map.copyOf(map) at the end of toSchemaMap? Map.copyOf doesn't keep the insertion order, so the schema keys can come out in a different order between runs.
Could you also check if thoughtsTokenCount should be part of tokensOut? As far as I know, thinking tokens are billed as output, so the cost would be a bit too low for thinking models. If you want to keep it as is, no problem, a short Javadoc note would be enough for me.
One bigger thing, but not blocking from my side: AdkSupport is quite large (about 2.5k lines and ~100 public static methods), with many sync, async and measured variants of the same things. Since all of it becomes public API, maybe we could start with a smaller set and add the other overloads when someone needs them? Happy to merge as is and do this in a follow-up, what do you think?
One more question: do we really need GEMINI_API_KEY for the integration tests? We use OPENAI_API_KEY for the others. For Kotlin I think Gemini is the only option, so this is fine by me. For Java maybe google-adk-langchain4j with OpenAI could work, but I haven't tried it. I can also just add the secret on our side.
None of these are blocking from my side. Thanks again!
…egration tests - Add OpenAI integration test in AdkAgentIT (dokimos-adk-java) gated on OPENAI_API_KEY using com.google.adk:google-adk-langchain4j (LangChain4j wrapping OpenAiChatModel with OpenAiChatModelName.GPT_4_O_MINI), alongside the Gemini test gated on GEMINI_API_KEY. - Add OpenAI integration test in AdkKotlinAgentIT (dokimos-adk-kotlin) gated on OPENAI_API_KEY using com.google.adk:google-adk-kotlin-integrations-openai-jvm (ChatCompletions with gpt-4o-mini), alongside the Gemini test gated on GEMINI_API_KEY. - Document third-party model integrations in docs/docs/integrations/adk.md.
|
Thanks a lot for the thorough review @fkapsahili! I've addressed all four points across recent commits:
|
Relates to #198
Summary
Adds first-class Dokimos integration modules for Google Agent Development Kit (ADK) in both Java (
com.google.adk:google-adk) and Kotlin (com.google.adk:google-adk-kotlin-core-jvm):dokimos-adk-java(dev.dokimos.adk):AdkTraceCollector— an ADKBasePluginthat passively capturesEvents,ToolCalls,ToolDefinitions (including full descriptions and JSON schemas fromBaseTool.declaration()), multi-turnConversationTrajectorys, retrieved RAG contexts (GroundingMetadatachunks + retrieval tool outputs), andCallMetrics(tokens withthoughtsTokenCountincluded intokensOut, latency, and optionalPriceTablecost).AdkSupport— streamlined static factory methods forasJudge,simpleTask,agentTask,ragTask,asyncTask,asyncAgentTask,asyncRagTask,measuredTask,measuredAgentTask,measuredRagTask,toCallMetrics,toAgentTrace,toToolCalls,toToolCall,toToolDefinitions,toToolDefinition,toSchemaMap,attach,extractTexts,toConversationTrajectory, andasConversationalApp.dokimos-adk-kotlin(dev.dokimos.adk.kt):AdkTraceCollector— an ADK KotlinPluginwith coroutine-native callbacks (onUserMessage,beforeRun,onEvent,afterModel,beforeTool,afterTool,onToolError,afterRun).AdkSupport+ idiomatic Kotlin extension functions (Model.asJudge,Runner.asJudge,BaseAgent.asJudge,BaseAgent.toAgentTrace,BaseAgent.runTrace,Runner.toAgentTrace,Runner.runTrace,Session.toAgentTrace,List<Event>.toAgentTrace,BaseAgent.toToolDefinitions,BaseTool.toToolDefinition,FunctionDeclaration.toToolDefinition,Schema.toSchemaMap,Session.toConversationTrajectory,List<Event>.toConversationTrajectory,BaseAgent.asConversationalApp,Runner.asConversationalApp).dokimos-examples(AdkAgentEvaluationExample.javaandAdkKotlinAgentEvaluationExample.kt).docs/docs/integrations/adk.md) and installation instructions (docs/docs/getting-started/installation.md).plugins/evaluate-adk).Review Updates
toSchemaMapinsertion order: PreservedLinkedHashMapinsertion order by returningCollections.unmodifiableMap(map)instead ofMap.copyOf(map).thoughtsTokenCountintokensOut: IncludedthoughtsTokenCountintokensOut(candidatesTokenCount + thoughtsTokenCount) acrossAdkSupportandAdkTraceCollectorin both Java and Kotlin since thinking tokens are billed at the output token rate, and documented this in Javadoc/KDoc.AdkSupportpublic API surface (>1,050 net lines removed):AdkSupport.javafrom 88 public static methods (2,522 lines) to 39 public static methods (1,775 lines) by removing redundantinputKey/outputKey/toolCallsKeypermutations, customExecutoroverloads, duplicateList<String>vs varargsretrievalToolNamesoverloads, 2-argPriceTableoverloads,measuredAsync*methods,toToolCall(FunctionCall, String), andtoToolDefinitions(LlmAgent).AdkSupport.ktfrom 1,719 lines to 1,470 lines by removing customExecutoroverloads, removingmeasuredAsync*methods (foldingCallMetrics+ optionalmodelId/priceTabledirectly intoasyncTask/asyncAgentTask/asyncRagTask), dropping@JvmOverloadsfrom multi-parameter functions, and removingtoToolCall(FunctionCall, String?)andtoToolDefinitions(LlmAgent).GEMINI_API_KEYandOPENAI_API_KEY:AdkAgentIT(dokimos-adk-java): Runs against Gemini (gemini-3.8-flash) whenGEMINI_API_KEYis set, and against OpenAI (OpenAiChatModelName.GPT_4_O_MINI) whenOPENAI_API_KEYis set via ADK Java'scom.google.adk:google-adk-langchain4j(LangChain4j) integration.AdkKotlinAgentIT(dokimos-adk-kotlin): Runs against Gemini (gemini-3.8-flash) whenGEMINI_API_KEYis set, and against OpenAI (gpt-4o-mini) whenOPENAI_API_KEYis set via ADK Kotlin's first-partycom.google.adk:google-adk-kotlin-integrations-openai-jvm(ChatCompletions) integration.Testing & Verification
dokimos-adk-javaand 24 unit tests indokimos-adk-kotlincovering trace extraction, parallel tool call correlation, OpenAPI/GenAI schema conversion to JSON Schema (including insertion-order preservation), RAG context extraction, multi-turn conversation simulation,CallMetrics(thoughtsTokenCount+PriceTable) computation, andJudgeLMadapters.@Tag("integration")):AdkAgentITandAdkKotlinAgentITsupport bothOPENAI_API_KEY(for CI) andGEMINI_API_KEY(for local / Gemini runs).mvn spotless:checkandmvn javadoc:javadocpass cleanly on Java 17+.