feat(agents): Moved Persistence to AIAgentStorage#1828
feat(agents): Moved Persistence to AIAgentStorage#1828
Conversation
Ololoshechkin
left a comment
There was a problem hiding this comment.
@Rizzen please consider my comments about naming and Java.
Also please add documentation about the new functionality in this PR (both Kotlin and Java) to our Persistence docs -- that's also very important
| * @param key A uniquely identifying key of type `AIAgentStorageKey` used to store the feature. | ||
| * @param value The feature to be stored, which can be of any type. | ||
| */ | ||
| @Deprecated("Use context.storage.set() instead", level = DeprecationLevel.WARNING) |
There was a problem hiding this comment.
Do you think we should deprecate this?
| * | ||
| * @property storage Optional pre-populated storage to use for the session execution. | ||
| */ | ||
| public data class AIAgentSessionInputs( |
There was a problem hiding this comment.
nit: Let's either rename to AIAgentSessionAdditionalInputs or alternative move input: T to this class as well
There was a problem hiding this comment.
Probably, renaming to "AdditionalInputs" is a better option
There was a problem hiding this comment.
Also, maybe let's extend to the whole AIAgentContext that one can provide as input (not just storage that is actually a part of it)?
Then it will be also possible to share context between CLI Agents (Claude -> Junie -> Codex), WDYT @sdubov ?
| val session = agent.createSession("test-session") | ||
| val output = session.run( | ||
| input = "ignored", | ||
| sessionInputs = AIAgentSessionInputs(storage = AIAgentStorage()), |
There was a problem hiding this comment.
looking at the usage side, I'd probably name the parameter additionalContext or similar.
Also, have you updated the Java API for it? Let's please add a Java test too!
| * @param input The input data to set for the agent | ||
| */ | ||
| public fun setExecutionPoint( | ||
| public suspend fun setExecutionPoint( |
There was a problem hiding this comment.
And how to use it from Java now?
| * @param sessionId Optional session identifier. A random UUID is generated if not provided. | ||
| * @return The output produced by the agent after resuming from the checkpoint. | ||
| */ | ||
| public suspend fun <Input, Output> AIAgent<Input, Output>.runFromCheckpoint( |
There was a problem hiding this comment.
And how to use it from Java? Let's consider moving this either to AIAgent class (or one of it's sub-classes), or creating some helper object or static method for Java (I am thinking about something like Persistence.runFromCheckpoint(agent, input, checkpoint, ...) )
There was a problem hiding this comment.
Plase also create Java example or/and test
|
|
||
| AIAgent<String, String> agent = createAgent(); | ||
|
|
||
| String output = RunFromCheckpointJvm.runFromCheckpoint( |
There was a problem hiding this comment.
Let's move it from RunFromCheckpointJvm (which looks a bit too specific for an object name) to Persistence (as a JvmStatic method) so that it's Persistence.runFromCheckpoint(...) .
Or let's think of some other better place for it. RunFromCheckpointJvm looks a bit like a glue code coming from Kotlin equivalent
Storage-based persistence: Migrated the Persistence feature to use AIAgentStorage (on AIAgentContext) instead of direct context fields, making persistence state accessible through the standard storage API
External storage injection: Added AIAgentSessionInputs — an extensible container that allows pre-populating AIAgentStorage before session execution via session.run(input, AIAgentSessionInputs(storage =
...))
runFromCheckpoint API: New extension functions on AIAgent and AIAgentRunSession that restore agent execution from a saved AgentCheckpointData without requiring the Persistence feature to be installed.
Persistence setExecutionPoint* methods are now suspend to align with the storage API migration