feat: [Orchestration] Configuration as JSON String

[MatKuhr]

Context

This PR adds a feature to directly use JSON as downloaded from AI Launchpad.

Definition of Done

• Functionality scope stated & covered
• Tests cover the scope above
• Error handling created / updated & covered by the tests above
• Aligned changes with the JavaScript SDK
• Documentation updated
• Release notes updated

[CharlesDuboisSAP]

An OrchestrationPrompt is not the right object then. Why not accept a map of input params?

[MatKuhr]

Because that is missing the (optional) messages history.

OrchestrationPrompt is not the right object

I'd argue it is. We will have further cases where passing messages is not allowed, e.g. when using TemplateRef. If we want we could create another Prompt object for that to make this distinction, but so far I figured it's not worth it.

[jjtang1985]

I left some comments.

[jjtang1985]

I would make the prompt as optional parameter.

[MatKuhr]

It's not optional, it has to be given. In this unit test I'm just skipping the effort to give params in the map

[jjtang1985]

It's not optional, it has to be given.

You are right, so we can discuss the API signature/design 😄

Please have a look at the following json, that I copied from the Launchpad.

{
    "module_configurations": {
        "llm_module_config": {
            "model_name": "mistralai--mistral-large-instruct",
            "model_params": {},
            "model_version": "2407"
        },
        "templating_module_config": {
            "template": [
                {
                    "role": "user",
                    "content": "How are you?"
                }
            ],
            "defaults": {}
        }
    }
}

I would assume, I've got everything from the JSON and should be able to run it with a function like

myFunction(json);

The second param (prompt in this case) is only needed (so it should be optional), when I want to pass a inputParam or provide message history like the following screenshot:

[MatKuhr]

Ah true, Launchpad doesn't force you to use any variables. Okay, will update 👍🏻

[MatKuhr]

After thinking about this again, I actually think the use case for this is not very strong. Because if there are no input params or messages history, prompt execution will always yield the same or very similar results (depending on the temperature setting).

The same is true when using template references. If we really want to support such usage, we could consider allowing new OrchestrationPrompt() for these cases. But making the prompt itself optional would mean we add 2-3 method overloads for non-productive cases..

[jjtang1985]

👍
Let's keep this as beta before the alignment with e.g., Launchpad.

[newtork]

Your suggested code change results in the following three methods on client:

OrchestrationClient client = new OrchestrationClient();

// execute completion request (exist)
CompletionPostRequest requestDto;
CompletionPostResponse responseDto = client
  .executeRequest(
    requestDto);

// execute prompt (exist)
OrchestrationPrompt promptConvenience;
OrchestrationModuleConfig configConvenience;
OrchestrationChatResponse responseConvenience = client
  .executeRequest(
    promptConvenience,
    configConvenience);

// execute prompt (new)
OrchestrationPrompt promptConvenience;
String configJson;
OrchestrationChatResponse responseConvenience = client
  .executeRequestFromJsonModuleConfig(
    promptConvenience,
    configJson);

(Question)

Is it not possible to derive reuse existing method and allow for OrchestrationModuleConfig to be created from String configJson? That would look a little cleaner, maybe(?)

[MatKuhr]

I also thought about this approach. But unfortunately not, because the JSON string is expected to contain things not yet supported by OrchestrationModuleConfig or it's individual config objects, i.e. throughout the entire JSON. That includes cases of oneOf, e.g. TemplateRef.

[newtork]

(Major/Question)

Not sure, but wouldn't it make more sense applying the module config JSON to client instead of per request?

Seems like the only place where orchestration_config is provided, it could also be an optional parameter that is passed along the convenience API(?)

(My hidden motivation is to challenge this method a little.)

[MatKuhr]

Hm not exactly sure what you mean.. Do you mean setting the config on the OrchestrationClient object instead? Or something else?

[newtork]

Yes.
Boiling this down to the question: Is orchestration_config to be expected request specific or client specific?

[MatKuhr]

Essentially same as the OrchestrationConfig object: It depends how much, if any, config changes per request. But the tendency is it is tied to the client, i.e. for typical use cases the majority of config will stay the same across multiple requests.

However, we decided to not allow config per client to make the API simpler. So the same strategy is applied for this method too. If we want to, we can still change this in the future though.

[newtork]

Understood.

To me, from a user perspective, it would feel weird to pass a String object along (over multiple controller classes potentially) just to pass to my request invocations.

But it's difficult to argue about client / request specific data handling, when I haven't yet seen a proper use-case "in the wild". Maybe we will discuss this again, after having users first interact with that.