refactor/move-prompts-to-jinja-templates #2164

jeannotdamoiseaux · 2024-11-18T16:24:10Z

Purpose

This pull request addresses the request made by @pamelafox in #2123 to refactor the code for improved prompt management.
All prompts are now stored in a separate folder using .jinja files. This makes it easier to manage and update prompts across the application.

Does this introduce a breaking change?

[ ] Yes
[ X ] No

Does this require changes to learn.microsoft.com docs?

This repository is referenced by this tutorial
which includes deployment, settings and usage instructions. If text or screenshot need to change in the tutorial,
check the box below and notify the tutorial author. A Microsoft employee can do this for you if you're an external contributor.

[ ] Yes
[ X ] No

Type of change

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ X ] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

Code quality checklist

See CONTRIBUTING.md for more details.

The current tests all pass (python -m pytest).
I added tests that prove my fix is effective or that my feature works
I ran python -m pytest --cov to verify 100% coverage of added lines
I ran python -m mypy to check for type errors
I either used the pre-commit hooks or ran ruff and black manually on my code.

Additional Notes

This refactoring aims to improve the overall code structure without changing functionality. It should make the codebase more maintainable and easier to work with for future development.

pamelafox · 2024-11-18T20:51:19Z

app/backend/approaches/prompts/chat/system_message_vision.jinja

+{follow_up_questions_prompt}
+{injected_prompt}
+
+"You are an intelligent assistant helping analyze the Annual Financial Report of Contoso Ltd., The documents contain text, graphs, tables and images. "


This looks incorrect? I think you can remove lines 12 and onwards?

You're absolutely right. I originally pasted this here to compare the prompts but forgot to remove it afterward.

pamelafox · 2024-11-18T20:53:07Z

app/backend/approaches/prompts/chat/query_few_shots.jinja

@@ -0,0 +1,18 @@
+[
+    {


Interesting, I hadn't considered making the few shots a Jinja as well. Do you imagine passing template variables into it?

This makes more sense indeed. I converted it to .jinja initially to stay consistent with the file extensions in the prompts folder. I've now updated these to .json.

pamelafox · 2024-11-18T20:53:44Z

app/backend/approaches/prompts/ask/system_message.jinja

+Use 'you' to refer to the individual asking the questions even if they ask with 'I'. 
+Answer the following question using only the data provided in the sources below.
+Each source has a name followed by colon and the actual information, always include the source name for each fact you use in the response.
+If you cannot answer using the sources below, say you don't know. Use below example to answer


If you install pre-commit, it should fix the new lines. See CONTRIBUTING.md for installation instructions.

I'm not entirely sure what you mean here. Could you clarify?

I notice that several files don't have newlines at the ends of the file, which usually means that the pre-commit hasn't run, as the pre-commit hooks fix that issue (and others). There are instructions for installing pre-commit hooks here:
https://github.com/Azure-Samples/azure-search-openai-demo/blob/main/CONTRIBUTING.md#setting-up-the-development-environment

app/backend/approaches/chatapproach.py

pamelafox · 2024-11-18T20:56:12Z

app/backend/approaches/chatapproach.py

-    """
+    def __init__(self):
+        self._initialize_templates()
+        self.NO_RESPONSE = "0"


NO_RESPONSE can stay as a class variable, no? There's no particular need for it to be an instance variable.

You're right, I reverted the change.

pamelafox · 2024-11-18T20:57:23Z

app/backend/approaches/retrievethenread.py

+        self.system_chat_template = self.env.get_template('system_message.jinja').render()
+        self.few_shots = self._process_few_shots()
+
+    def _process_few_shots(self) -> Any:


Given the amount of processing here, I'd be inclined to leave the few shots as .json, but inside the prompts folder, and just load with json package.

Same here, I’ve converted the few_shots examples to JSON.

pamelafox

Thank you! My main suggestion is that we keep few shots as JSON files, while still keeping the nice organization into separate folder.

jeannotdamoiseaux · 2024-11-19T10:05:09Z

Thank you! My main suggestion is that we keep few shots as JSON files, while still keeping the nice organization into separate folder.

I’ve incorporated the above comments in the latest commit. Thanks for taking the time to review!

pamelafox · 2024-11-19T23:17:01Z

@jeannotdamoiseaux Thanks for the changes!
To give us one more idea for how we could approach this, I've put together this PR: #2178
It uses a file format called Prompty that was developed by Microsoft but is intended to be generically useful.
I'm 50/50 on which approach is best, would love to get some more feedback from the community on these branches. See my comments in that PR about benefits/drawbacks.

CCing recent contributors:
@bnodir @zedhaque @cforce @fujita-h @EMjetrot

jeannotdamoiseaux · 2024-11-20T08:21:38Z

@jeannotdamoiseaux Thanks for the changes! To give us one more idea for how we could approach this, I've put together this PR: #2178 It uses a file format called Prompty that was developed by Microsoft but is intended to be generically useful. I'm 50/50 on which approach is best, would love to get some more feedback from the community on these branches. See my comments in that PR about benefits/drawbacks.

CCing recent contributors: @bnodir @zedhaque @cforce @fujita-h @EMjetrot

My suggestion would be to move all prompts to a separate folder using Jinja files first to create a clearer overview and better structure. Once that’s in place, we could consider using Prompty later if it aligns well with the project's needs. For now, though, I don’t see this as something that would significantly push my project forward.

bnodir · 2024-11-20T10:06:44Z

Thanks for the PR! Based on the high Package Health Score of Jinja2, I favor sticking with it over Prompty, which has a lower score (details here).

That said, I’m open to hearing more about specific cases where Prompty might offer clear advantages.
Looking forward to hearing others' feedback on this!
CC: @zedhaque @cforce @fujita-h @EMjetrot

fujita-h · 2024-11-20T16:18:39Z

I understand that Jinja2 and Prompty are being compared as approaches to organizing prompts.

Jinja2 has a proven track record as a template engine for Flask, Django, etc., and I think it is a very good option.
On the other hand, Prompty is a tool designed for LLM apps, so I think it is suitable for this project.

This decision is very difficult. I think Jinja2 is sufficient as an approach to achieve the goal of organizing the current prompts. If I were forced to make this choice for my own production project, I would definitely choose Jinja2.

However, considering that this project is "Azure Samples", I think it is better to be adventurous, so I will vote for Prompty.

zedhaque · 2024-11-20T21:11:43Z

I suggest Prompty because it specializes in AI prompt creation and management. It offers structured workflows, better visibility, making debugging and evaluation (prompt evaluation and ab testing easier) While migrating to it might take some effort, its focus on flexibility and portability makes it a great fit for our AI needs. Jinja2 was designed for general templating, but Prompty is better designed for AI-specific tasks.

Thanks.

move-prompts-to-jinja-templates

f69861d

pamelafox reviewed Nov 18, 2024

View reviewed changes

app/backend/approaches/chatapproach.py Outdated Show resolved Hide resolved

pamelafox reviewed Nov 18, 2024

View reviewed changes

pamelafox requested changes Nov 18, 2024

View reviewed changes

refactor: convert few_shots to JSON format and clean up comments

eda6292

jeannotdamoiseaux added 2 commits November 19, 2024 10:06

Clean retreivethenread.py

e25468c

Merge branch 'main' into refactor/move-prompts-to-jinja-templates

4063767

jeannotdamoiseaux mentioned this pull request Nov 19, 2024

feat: make-rag-optional-but-default #2123

Open

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor/move-prompts-to-jinja-templates #2164

refactor/move-prompts-to-jinja-templates #2164

jeannotdamoiseaux commented Nov 18, 2024

pamelafox Nov 18, 2024

jeannotdamoiseaux Nov 19, 2024

pamelafox Nov 18, 2024

jeannotdamoiseaux Nov 19, 2024

pamelafox Nov 18, 2024

jeannotdamoiseaux Nov 19, 2024

pamelafox Nov 19, 2024

pamelafox Nov 18, 2024

jeannotdamoiseaux Nov 19, 2024

pamelafox Nov 18, 2024

jeannotdamoiseaux Nov 19, 2024

pamelafox left a comment

jeannotdamoiseaux commented Nov 19, 2024

pamelafox commented Nov 19, 2024

jeannotdamoiseaux commented Nov 20, 2024

bnodir commented Nov 20, 2024 •

edited

Loading

fujita-h commented Nov 20, 2024

zedhaque commented Nov 20, 2024

refactor/move-prompts-to-jinja-templates #2164

Are you sure you want to change the base?

refactor/move-prompts-to-jinja-templates #2164

Conversation

jeannotdamoiseaux commented Nov 18, 2024

Purpose

Does this introduce a breaking change?

Does this require changes to learn.microsoft.com docs?

Type of change

Code quality checklist

Additional Notes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pamelafox left a comment

Choose a reason for hiding this comment

jeannotdamoiseaux commented Nov 19, 2024

pamelafox commented Nov 19, 2024

jeannotdamoiseaux commented Nov 20, 2024

bnodir commented Nov 20, 2024 • edited Loading

fujita-h commented Nov 20, 2024

zedhaque commented Nov 20, 2024

bnodir commented Nov 20, 2024 •

edited

Loading