-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FS-60: Add promptfoo test config for suggestions prompt #14
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
backend/src/prompts/README.md
Outdated
Then run `promptfoo eval` to run promptfooconfig.yaml | ||
ro run a specific file for example `generate_message_suggestions_config.yaml` run `promptfoo eval -c generate_message_suggestions_config.yaml` | ||
|
||
Afterwards, you can view the results by running `promptfoo view` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these live under backend/tests/prompts
instead? ... to be honest, I don't know what the tests in there are doing, or how they are supposed to be run...
It's becoming a bit of a mix...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests we have under "prompts" already aren't doing a great deal in my opinion. Looks to me like it's just duplicating the code.
e.g. if someone was to make a change to the prompt file, the test would fail and then they would update the prompt exactly the same in the prompts file. I suppose it might alert you if you changed a prompt that you didn't realise was being nested inside another prompt somewhere else?
I would be tempted to scrap all the tests under /backend/tests/prompts
and replace it with these...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That being said, the tests in that folder are python based, and this is an npm based test. So maybe mixing them isn't a good idea. Possibly we could have it under /backend/promptfoo
, I'm not sure...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because of how python does import you can't do that - at least I couldn't figure out how to
the problem line is from prompting import PromptEngine
from get_generate_message_suggestions_prompt.py
can show you if you want
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made some updates to fix some of these issues
Co-authored-by: Charlie Leopard <[email protected]>
…v file. Simplify test structure so less utility .py classes are needed
…cottLogic/InferESG into FS-60/set-up-promptfoo-example
If someone can test my changes on their machine before we merge, that would be ideal |
Description
https://scottlogic.atlassian.net/browse/FS-60
I have added an example of promptfoo testing on the suggestions prompt with the hope that this can easily be modified to work for other prompts when we come to work on those.