- Issue created by @marcus_johansson
- First commit to issue fork.
- 🇬🇧United Kingdom justanothermark
Created an MR for this.
I also want to suggest creating a follow-up ticket to remove the `$include_pre_prompt` parameter from `AssistantMessageBuilder::buildMessage()`. We already pass the AIAssistant to this parameter so the Message Builder can check this directly to see whether or not to include the pre prompt. In turn, this means we can simplify `AiAssistantApiRunner::process` and keep all references to the prompt files in a single place. However, this would introduce backwards compatibility issues so I haven't included it as part of the MR.
- 🇩🇪Germany marcus_johansson
Thanks Mark - I did realize one thing that I forgot to put in the proposed resolution that we should add here,
If they actual are overriding already, we can assume at the moment that they are using a custom prompt. We should create a update hook that outputs a warning about it, so they have seen it? Or some other way of letting them know that this has happened, otherwise I'm not sure we can add it into 1.0 branch, since it is changing running sites.
- 🇬🇧United Kingdom justanothermark
Ah yeah, that makes sense @marcus_johansson. I've added an update hook and implemented hook_requirements to provide warnings with links to the documentation if the site is using advanced mode but doesn't have custom prompts enabled.
- 🇮🇳India akulsaxena
The changes LGTM
But there seem to be some PHPCS error in the file. Seems like a single small error.
can you please fix it, moving it to NW for now - 🇬🇧United Kingdom justanothermark
My mistake, I'd fixed that but not pushed the change. Updated now.