quarkiverse / quarkus-langchain4j

Quarkus Langchain4j extension
https://docs.quarkiverse.io/quarkus-langchain4j/dev/index.html
Apache License 2.0
148 stars 89 forks source link

Register GuardRails programmatically #1065

Open lordofthejars opened 2 weeks ago

lordofthejars commented 2 weeks ago

Currently, we can only (or as far as I know) register guardrails with annotations. This is great, but some guardrails might need to register programmatically for better reusing. For example:

new BanTopicGuardRail("politics");

In one model but in another model, we might want to add a GuardRail like

new BanTopicGuardRail("politics", "economics")

Both models are in the same project.

The same is true for a threshold. In one case, you might want a more restrictive threshold than in another, so the only thing you might change is the value of the constructor.

There are some workarounds, like creating one subclass for each topic to ban, but I think it is better to use a programmatic approach.

One option could be to provide a default method in the @RegisterAi interface, so if you provide an implementation, these guardrails are appended to the ones defined declaratively. I know that here, the default method affects all the defined methods, and you cannot programmatically create guardrails for specific methods, but at least you can register guardrails programmatically.

Moreover, this default method should add the beans into the CDI container.

It is just an idea on how to implement it, but I wanted to put the problem users might face.

cescoffier commented 2 weeks ago

I need to think about it. The programmatic approach you propose would add guardrails that are not CDI beans (you instantiate them), so I would instantiate them on every call (like a request scope guardrail).

I'm wondering if we cannot achieve something similar with pure configuration.

lordofthejars commented 2 weeks ago

I know, but why it is instantiate in every call, couldn't be just "produce" in application scope using a CDI method to register it?

geoand commented 1 week ago

One potential approach (that we use elsewhere in the AiService registration) is to use a supplier which is itself a CDI bean, this allowing you to create whatever you want easily

cescoffier commented 1 week ago

@geoand If we start taking two approaches (supplier and direct bean) to register guardrails, how can we preserve the order? (The order is very important.)

@lordofthejars If you can produce the guardrails like this, why do you need a programmatic approach? You just need a custom class name. No?

geoand commented 1 week ago

If we start taking two approaches (supplier and direct bean) to register guardrails, how can we preserve the order? (The order is very important.)

We could have a Supplier<SomeTypeThatIncludesAPriority> or something like that

lordofthejars commented 1 week ago

No because the class is always the same, what changes are the parameters. For example the threshold, the ban topics, ...

cescoffier commented 1 week ago

We could have a Supplier or something like that

It would require guardrails to also include a priority, and also provide a quite confusing experience:

@OutputGuardrails(a.class, b.class, c.class)

may end up being (based on priority):

c.class, a.class, b.class

That would require a large API change, as it would need a repeatable @OutputGuardrail taking a single parameter instead of an array with a defined order.

cescoffier commented 1 week ago

No because the class is always the same, what changes are the parameters. For example the threshold, the ban topics, ...

Then, I don't see how I can register them as CDI beans. It will be ambiguous for the given type (2 beans exposing the same classname).

geoand commented 1 week ago

IMO, the result, should not be a CDI bean - we should follow the same approach as we do with the suppliers that can be used in @RegisterAiService

geoand commented 1 week ago

That would require a large API change, as it would need a repeatable @OutputGuardrail taking a single parameter instead of an array with a defined order

But that shouldn't be a problem given the API is still experimental, right?

cescoffier commented 1 week ago

No it's not a problem to change it right now.

cescoffier commented 1 week ago

Ah, I remember something (well, my notebook did remind me) - I did not use priority to avoid having a dependency on jakarta common annotations (in order to contribute this back to langchain4j)

geoand commented 1 week ago

That shouldn't be a problem really because @Priority is in jakarta.annotation which only contains annotations, no logic.

lordofthejars commented 1 week ago

I said CDI as an idea but of course al of you know better the details on what is the best way to implement.

About Priority, is it a problem to create a custom annotation?