Skip to content

Conversation

@haydenrear
Copy link
Contributor

Added annotations for retry for qos provider, and also properties, with nested for action names #1221 - related to #1215

"embabel.agent.platform.action-qos.default.idempotent=true",
"embabel.agent.platform.action-qos.default.backoff-multiplier=1",
"embabel.agent.platform.action-qos.default.backoff-millis=1",
"embabel.agent.platform.action-qos.agents.agent.method.max-attempts=6",
Copy link

@drewlakee drewlakee Dec 31, 2025

Choose a reason for hiding this comment

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

Hi, again, and thanks for the PR!

Do you think the agent retry configuration (e.g., embabel.agent.platform.action-qos.agents.agent.method.max-attempts=6) would be more suitable in this form?

For example, I’ve defined my agents like this:

@Agent(
    name = "Subscription availability verification agent",
    description = "Explains the availability of any subscription for a user's subscription state"
)

However, there are no validations for the @Agent.name parameter. This configuration approach would force me to rewrite my agent names to ensure safe and correct usage of Spring properties.

While unification is generally beneficial, I’m concerned that such agent names might be vulnerable at requested prompts, potentially compromising the quality of generative results from the LLM. Autonomy could use @Agent.name for the ranking operation, if user message was passed in a free form, not programmatically via the AgentInvocation class.

Maybe it would be enough to code the agent retry configuration at the annotation level? What are your thoughts on this?

val toolGroups: Array<String> = [],
val toolGroupRequirements: Array<ToolGroup> = [],
val trigger: KClass<*> = Unit::class,
val retry: Array<Retry> = []
Copy link

@drewlakee drewlakee Dec 31, 2025

Choose a reason for hiding this comment

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

Could you clarify why Array<Retry> was used instead of a single Retry? Since the Action annotation applies per action, I’m not sure I understand the reasoning behind this choice.

The same thing to Retry.maxAttempts[] and others, it goes as several nested arrays Action.Retry[].maxAttempts[], but at DefaultActionQosProvider you always take the first.

By the way, I’m just curious in which cases I could use this in my project 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @drewlakee - thanks for looking at this!

I made it an array because I found that they can't be null in Kotlin, and I didn't want to match over the default value because I'm overriding per parameter, so the empty array would signify the value not being set. However it does seem a bit confusing. I suppose we can set it to the default value in ActionQos instead to make it more clear.

However, if the user provides the same as the default, and then provides a property also, we won't know that the user provided it in the action. You know what I mean? There would be no way to know if we match over the default value. So I was thinking it's probably better to do it like this? Unless there's a more idiomatic approach in Kotlin for this?

Thank you,
Hayden

Choose a reason for hiding this comment

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

@haydenrear thanks for explanation, I just didn't think about it in that way. Considering this, looks like a good workaround for compile-time with optional parameters. Also, I looked into how Kotlin designs its annotations, and it looks like it's the same.

Copy link

@drewlakee drewlakee Jan 5, 2026

Choose a reason for hiding this comment

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

Hi again! I’m back with a fresh proposal about a (potentially) more consistent action retry configuration for agents in the framework :)

Don’t you think that the @Retry configuration looks a bit messy? It causes the RetryTemplate to leak its parameters into the Embabel's high-end @Action, including all the verbose details. I’ve been thinking about this and trying to challenge it in various forms, so maybe my idea could help give more space to think about.

Currently, we have a default behaviour for Action retries. Perhaps we could reuse it as a «compromise» by introducing some kind of enum‑like configuration: @Action(retryPolicy = ActionRetryPolicy.DEFAULT). This approach could benefit the encapsulation of the RetryTemplate implementation. Considering your thoughts about the confusing property matching — whether defaults come from the annotation level or from Spring’s default properties — perhaps we could introduce something like @Action(retryPolicy = ActionRetryPolicy.NONE)?

In this model, ActionRetryPolicy would only define framework‑preconfigured profiles. There could also be something like ActionRetryPolicy.FIRE_ONCE. However, if we want to override it more precisely, we could introduce a custom retry policy, such as @Action(retryPolicy = "subscription.agent"). This would force the framework to scan Spring properties like <subscription.agent>.action.retry.policy.max-attempts and others. Otherwise, it would partially adopt the default profile parameters for the retry policy — in this case, from ActionRetryPolicy.DEFAULT.

If we would like to override action retry policies per agent, we could introduce something like @Agent(actionRetryPolicy = "my.agent.name"). This would function in the same way as Spring’s property scanning for per‑action configurations.

To sum up:

  • ActionRetryPolicy (e.g., NONE, DEFAULT, FIRE_ONCE, etc.) — provides framework‑preconfigured retry profiles for actions.
  • @Action.retryPolicy — offers the same functionality per action, with support for Spring properties.
  • @Agent.actionRetryPolicy — provides the same capability per agent.
  • Default retry policy per action & per agent at spring properties (the values from ActionQos)

Just wanted to share my thoughts, maybe it would be useful in some way 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @drewlakee

Do you think we should support any Spel expression? Should it only support ${property.value} only? In that case should we drop the spring idiom? Or keep the property placeholder syntax? Or should I just strip the property placeholder manually?

Thanks!
Hayden

Choose a reason for hiding this comment

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

@haydenrear Honestly, I don’t have that much extensive experience with SpEL, so I can’t tell whether this is the most idiomatic spring approach for such configuration. However, after reviewing your latest commit, I believe you’ve nailed the idea perfectly 👍 I pulled the PR and experimented with the tests — the solution feels more declarative and Spring‑friendly to me. Thank you!

Considering SpEL docs, maybe at some point it would come in handy for such project needs

@Action(actionRetryPolicyExpression = "condition ? 'one' : 'two'")
fun perform() { }

@Agent(actionRetryPolicyExpression = "\${project.custom.action.retry.policy:until.success.action.retry.policy}")

At least for now, it can resolve the configuration via spring properties in a convenient way — pretty useful!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review @drewlakee, great ideas!

@alexheifetz alexheifetz requested a review from johnsonr January 3, 2026 04:28
@haydenrear haydenrear force-pushed the action-qos-annotation-props branch from 2378579 to 169b932 Compare January 6, 2026 00:05
…ith nested for action names embabelgh-1221

Signed-off-by: hayden.rear <hayden.rear@gmail.com>
@haydenrear haydenrear force-pushed the action-qos-annotation-props branch from 169b932 to 0a10dca Compare January 7, 2026 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants