Treat messages with missing variables as text

[anunnakian]

Issue

#1125

Change

When we declare an assistant as following example :

interface AiService {

    @UserMessage("What is the capital of {{it}}?")
    String answer();
}

We don't throw an illegal exception like before, we treat the user message like text without resolving the {{it}} variable.

Here is a test that explain the treatment clearly :

@Test
void test_user_message_configuration_8() {

    // given
    AiService aiService = AiServices.builder(AiService.class)
            .chatLanguageModel(chatLanguageModel)
            .build();

    // when-then
    assertThat(aiService.answer()).containsIgnoringCase("");
    verify(chatLanguageModel).generate(singletonList(userMessage("What is the capital of {{it}}?")));
}

General checklist

• There are no breaking changes
• I have added unit and integration tests for my change
• I have manually run all the unit and integration tests in the module I have added/changed, and they are all green
• I have manually run all the unit and integration tests in the core and main modules, and they are all green

• I have added/updated the documentation
• I have added an example in the examples repo (only for "big" features)

Checklist for adding new model integration

• I have added my new module in the BOM

Checklist for adding new embedding store integration

• I have added a {NameOfIntegration}EmbeddingStoreIT that extends from either EmbeddingStoreIT or EmbeddingStoreWithFilteringIT
• I have added my new module in the BOM

Checklist for changing existing embedding store integration

• I have manually verified that the {NameOfIntegration}EmbeddingStore works correctly with the data persisted using the latest released version of LangChain4j

[langchain4j]

Hi @anunnakian thanks a lot!

This seems to be a bit different from what was described in #1125

This is definitely a wrong configuration:

interface AiService {

    @UserMessage("What is the capital of {{it}}?")
    String answer();
}

The case described in the issue is different:

@AiService
public interface AiAssistant {

    TokenStream chat(@MemoryId String chatId, @UserMessage String userMessage);
}

assistant.chat("12345", "Text containing {{it}}");

[anunnakian]

@langchain4j if we consider that the issue was fixed, the following tests must be green right ?

@Test
void test_user_message_configuration_8() {

    // given
    AiService aiService = AiServices.builder(AiService.class)
            .chatLanguageModel(chatLanguageModel)
            .chatMemoryProvider(memoryId -> MessageWindowChatMemory.withMaxMessages(10))
            .build();

    // when
    aiService.chat8("12345", "What is the capital of {{it}}?");

    // then
    verify(chatLanguageModel).generate(singletonList(userMessage("What is the capital of {{it}}?")));
}

@Test
void test_user_message_configuration_9() {

        // given
        AiService aiService = AiServices.builder(AiService.class)
                .chatLanguageModel(chatLanguageModel)
                .chatMemoryProvider(memoryId -> MessageWindowChatMemory.withMaxMessages(10))
                .build();

        // when
        aiService.chat8("12345", "What is the capital of {{variable}}?");

        // then
        verify(chatLanguageModel).generate(singletonList(userMessage("What is the capital of {{variable}}?")));
    }

[anunnakian]

We should remove this test case, if the missing variable {{name}} must be treated like text, right ?

@Test
void should_fail_when_value_is_missing() {

   // given
   PromptTemplate promptTemplate = PromptTemplate.from("My name is {{name}}.");

   Map<String, Object> variables = emptyMap();

   // when-then
   assertThatThrownBy(() -> promptTemplate.apply(variables))
           .isExactlyInstanceOf(IllegalArgumentException.class)
           .hasMessage("Value for the variable 'name' is missing");
}

[langchain4j]

@anunnakian to your first question: yes.
Second: not sure, is there a way to treat it as a text (instead of template) only for that use case?

[anunnakian]

@langchain4j If we treat the message as text instead of a template, we will break the case if we have two variable with one missing like the following test :

interface AiService {

    String chat9(@MemoryId String chatId, @UserMessage String userMessage, @V("country") String country);
}

@Test
void test_user_message_configuration_10() {

    // given
    AiService aiService = AiServices.builder(AiService.class)
            .chatLanguageModel(chatLanguageModel)
            .chatMemoryProvider(memoryId -> MessageWindowChatMemory.withMaxMessages(10))
            .build();

    // when
    aiService.chat9("12345", "What is the {{it}} of {{country}}?", "Germany");

    // then
    verify(chatLanguageModel).generate(singletonList(userMessage("What is the {{it}} of Germany?")));
}

This test must be green too at the end, right ?

[anunnakian]

@langchain4j I found a solution to make all tests above and the following one green!
my branch is up to date and waiting for your feedback

interface AiService {
    String chat(@MemoryId String chatId, @UserMessage String userMessage);
}

@Test
void test_user_message_configuration_9() {

    // given
    AiService aiService = AiServices.builder(AiService.class)
            .chatLanguageModel(chatLanguageModel)
            .chatMemoryProvider(memoryId -> MessageWindowChatMemory.withMaxMessages(10))
            .build();

    // when
    aiService.chat("12345", "What is the {{it}} of {{var}}?");

    // then
    verify(chatLanguageModel).generate(singletonList(userMessage("What is the {{it}} of {{var}}?")));
}

[anunnakian]

A make this method default to avoid breaking changes

[ashimoon]

Should we just always treat missing variables, including {{it}} as an illegal state? I would imagine in the vast majority of cases, if the author created a @UserMessage with a prompt variable in it, then the method signature must match that user message, otherwise an exception should be thrown.

If we do indeed want to support What is the capital of {{it}}? where we want the string to be supported literally without any templating, then could we just add a property to @UserMessage to indicate that?

e.g.

@UserMessage(value = "What is the capital of {{it}}?", templated = false)

public @interface UserMessage {
    String value()
    boolean templated() default true;
}

If we need even further grained control, such that only some variables are excluded from templating, that could also be defined in the annotation?

@UserMessage(value = "What is the capital of {{it}} in {{country}}?", excludedVariables = { "it" })

public @interface UserMessage {
    String value()
    String[] excludedVariables() default {};
}

[langchain4j]

@ashimoon yes, I think you are right. We should be strict by default (as it is now).

I think I rushed a bit with #1125. It can be solved by escaping {{it}} instead of making changes to the default behaviour of prompt templates.

@anunnakian sorry about that, let's put this PR on hold for now

[anunnakian]

Ok! Don't panic everything is under control (just kidding... 😁)

So, I'll create another PR to escape {{it}} variable tonight

Thanks for your feedback @ashimoon @langchain4j 😉

[langchain4j]

@anunnakian I meant that the current implementation (without this PR) indeed seems to be the most reasonable. I guess that specific corner case in #1125 is actually ok and the user can escape manually, if there is a need. Whether we should support auto-escaping, it is tricky. I would take a break and return to this problem a bit later.

[anunnakian]

@langchain4j got it ;)