Add Kotlin snippets for Memory Bank and RAG memory - #2089
Conversation
The Kotlin examples were pinned to adk-kotlin 0.5.0, which predates the context caching, Vertex AI memory, and RAG retrieval APIs. Bump the pin to 0.7.0 so snippets for those features can be added. Three consequences of the bump are handled here: - `ExperimentalResumabilityFeature` was removed in 0.7.0, so RunConfigExample no longer opts into it. The annotation class is gone, not merely deprecated, so this is a hard compile break rather than a warning that could be deferred. - The Vertex AI session and memory services expose Ktor's `HttpClient` as a defaulted constructor parameter, so any snippet naming them needs Ktor on the compile classpath, not just at runtime. - The `resolutionStrategy` block forcing kotlin-stdlib 2.1.20 is now dead. It worked around 0.5.0 publishing a stdlib newer than this project's compiler; 0.6.0 fixed that upstream. Verified that the highest stdlib on the compile classpath is still 2.1.20 without it. Also fixes two unrelated snags found while validating the above: - `check_kotlin_snippets.sh` walked `examples/kotlin` without pruning build output, so after any local build it reported every generated KSP file as an unregistered snippet. CI only ever ran it against a clean checkout, so the bug was invisible there. - A transclusion path in logging.md was split across two lines, so the include never resolved and the code block rendered empty.
260a152 to
d465c81
Compare
7df4c6a to
673ad88
Compare
The three plugin sample links 404. adk-python renamed `contributing/samples/plugin/` to `contributing/samples/plugins/`; the directory contents are otherwise unchanged, so only the path segment moves. This is what the repo-wide `link-check` job has been failing on. It is unrelated to the 0.7.0 upgrade in this PR, but the check gates the merge and the fix is confined to the three URLs. Verified all three targets return 200.
The memory page documented `VertexAiMemoryBankService` and `VertexAiRagMemoryService` for Python and Java only; the Kotlin tab stopped at the in-memory service. Both are available in adk-kotlin as of 0.7.0, so add the two missing tabs. Both services declare an `internal` primary constructor, so the snippets use the public secondary one that takes project/location plus the engine or corpus id -- reading the primary signature alone gives a constructor callers cannot invoke. The page's language-support badge stays at Kotlin v0.1.0: it marks when Kotlin support for the page was introduced, and the other six Kotlin snippets on it have worked since then. The remaining diff in MemoryExample.kt is ktlint bringing pre-existing lines into line with the repo style, which the linter now gates on because the file is touched here.
673ad88 to
d97ec91
Compare
Three fixes from review:
- The `rag_memory` KDoc claimed `ragCorpus` accepts a bare id or a full
resource name. It is the opposite: `normalizeCorpusName` does
`require(!ragCorpus.startsWith("projects/"))` and throws on a full name. The
Python tab directly above this snippet passes a full resource name, so a
reader switching tabs would have hit an IllegalArgumentException with a
comment telling them it was fine. The note now states the bare-id rule and
calls out the divergence from Python explicitly.
- Dropped "the primary constructor is internal" from the rendered snippet. It
is a note for reviewers, not for readers, who cannot see that constructor.
It stays in the PR description.
- Both snippets stopped at a factory function while the surrounding prose says
"instantiating the service and passing it to the Runner" and the Python tab
shows exactly that. They now build the service and pass it to a Runner.
✅ Deploy Preview for adk-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Technical review reportadk-docs PR #2089 — Add Kotlin snippets for Memory Bank and RAG memory Verdict: technically correct, with two accuracy issues to fix before merge. The API surface in both new snippets matches adk-kotlin v0.7.0 exactly. One placeholder value will throw at runtime, and one inline comment mischaracterizes the Python contract. A note on scope: the PR adds Kotlin snippets, so the authoritative source is Verified correct1. Memory Bank constructor — The snippet's three named arguments match the public secondary constructor exactly, in declaration order:
The PR's reviewer note about the primary constructor being 2. RAG constructor — all five arguments, including types
3. The headline fix — This is the substantive correction in commit
internal fun normalizeCorpusName(ragCorpus: String, project: String, location: String): String {
validateSegment(project, "project")
validateSegment(location, "location")
require(!ragCorpus.startsWith("projects/")) {
"ragCorpus must be a bare corpus id, not a full resource name: '$ragCorpus'."
}
validateSegment(ragCorpus, "ragCorpus id")
return "projects/$project/locations/$location/ragCorpora/$ragCorpus"
}The KDoc agrees at 4. Both types satisfy Both declare 5. The "wire it to a Runner" change matches upstream's own examples The new snippets are near-identical to adk-kotlin's canonical examples, which is strong corroboration:
Line 132 is literally 6. "Available since 0.7.0" is correct Both paths return HTTP 404 at tag Issues to fixIssue 1 (should fix): adk-kotlin validates that the id is numeric:
internal fun validateAgentEngineId(agentEngineId: String): String {
require(agentEngineId.isNotBlank()) { "agentEngineId must not be blank." }
require(agentEngineId.all { it.isDigit() }) { ... }Python has no such check — it only rejects empty and warns on Worth noting the asymmetry: the other three placeholders are safe, because Suggest Issue 2 (minor accuracy): the The snippet says "unlike the Python tab above, which takes the full name". Python accepts both forms:
The Observation (no action required): both services live in On the non-API claimsThe The CI caveat in the description is worth heeding: the Kotlin build/lint job appears never to have run on this PR, and the two findings above are exactly the kind a compile alone wouldn't catch anyway — |
joefernandez
left a comment
joefernandez
left a comment
There was a problem hiding this comment.
Thanks for the update.
Have a look at the Technical review report issues, particularly Issue 2 to see if it worth mentioning. I don't consider that a required change, thought.
#2089 (comment)
2 participants
Summary
docs/sessions/memory.mddocumentedVertexAiMemoryBankServiceandVertexAiRagMemoryServicefor Python only — the Kotlin tab stopped at thein-memory service. Both have been available in adk-kotlin since 0.7.0, so this
adds the two missing tabs.
What's in it
memory.md: Kotlin tabs for the Memory Bank and RAG memory sections.MemoryExample.kt:memory_bankandrag_memorysnippet regions.Notes for reviewers
ragCorpustakes a bare corpus id, not a full resource name.normalizeCorpusNamedoesrequire(!ragCorpus.startsWith("projects/"))andthrows on a full name, expanding the bare id itself. This diverges from the
Python tab immediately above, which passes the full
projects/.../ragCorpora/...name, so the snippet calls the difference outinline. (An earlier revision of this PR claimed the opposite; fixed.)
internalprimary constructor. Thesnippets use the public secondary one taking project/location plus the engine
or corpus id. Reading the primary signature alone yields a constructor
callers cannot invoke — noting it here rather than in the rendered snippet,
where a reader can't see the constructor anyway.
Runner, matching the surrounding prose("instantiating the service and passing it to the
Runner") and the Pythontab. They previously stopped at a factory function.
Kotlin v0.1.0. It marks when Kotlin supportfor the page was introduced, not the newest API on it — the other six Kotlin
snippets have worked since 0.1.0.
[start:full_example]marker moved above the function's KDoc, so that KDocnow renders inside the code block at
memory.md:229. ktlint forces this —"an EOL comment may not be preceded by a KDoc; reversed order is allowed when
separated by a newline" — and the reversed order is the only compliant
arrangement short of deleting the KDoc.
Verification
./gradlew compileKotlinpasses; ktlint clean on changed files.