Repository navigation
Feat/custom system prompts - #437
Conversation
edward-ly
left a comment
edward-ly
left a comment
There was a problem hiding this comment.
Hello, thanks a lot for tackling this! We've been discussing the changes internally, and while we agree that it would be nice for admins to be able to customize the system prompts, we're not sure if we can say the same for normal users. Not only does it add additional, unneeded complexity for users, we don't see any meaningful use cases for it either. Would you be able to remove the user part from this PR or, at the very least, split the user part into a separate PR and add more justifications to it?
@edward-ly Hi, thanks for your feedback. I removed user-defined summary system prompt option from this PR. I added a new PR #444 with user-defined summary system prompt option and justification - it is stacked on #437 |
edward-ly
left a comment
edward-ly
left a comment
There was a problem hiding this comment.
Works great in my testing so far! Just a few small changes and we can get this in. On top of my other comments, the initial revert commit is missing a sign-off trailer, so that should be fixed too.
Also, since this is your first contribution, I recommend reviewing our AI Contributions Policy in case you haven't seen it yet. I don't know for certain if you've used any AI assistance for this PR, but I would appreciate any disclosures if you did (or if you didn't, an explicit mention stating that).
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
75dc75b to
11e7074
Compare
That's nice!
I applied code changes that you suggested. What is more:
|
This reverts commit f9dfdf8. Signed-off-by: Arkadiusz Sitkiewicz <a.sitkiewicz@wbgroup.com>
…n/default Assisted-by: OpenCode:DeepSeek-V4.1-Flash Signed-off-by: Arkadiusz Sitkiewicz <a.sitkiewicz@wbgroup.com>
…dmin/default Assisted-by: OpenCode:DeepSeek-V4.1-Flash Signed-off-by: Arkadiusz Sitkiewicz <a.sitkiewicz@wbgroup.com>
…er/admin Signed-off-by: Arkadiusz Sitkiewicz <a.sitkiewicz@wbgroup.com>
…pt - admin Signed-off-by: Arkadiusz Sitkiewicz <a.sitkiewicz@wbgroup.com>
Signed-off-by: Arkadiusz Sitkiewicz <a.sitkiewicz@wbgroup.com>
…emove unrelevant doc-comments Signed-off-by: Arkadiusz Sitkiewicz <a.sitkiewicz@wbgroup.com>
11e7074 to
b028591
Compare
edward-ly
left a comment
edward-ly
left a comment
There was a problem hiding this comment.
Hey, sorry for not being able to get back to you earlier, but awesome work! I'll take care of rebasing the branch again at this point before merging. Thanks again!
|
Some CI checks are failing due to some upstream errors, not related to this PR (all checks passed before the latest rebase). |
No problem at all. I would also want to thank you for working through this with me and helping me out 🤝 🎉 |
Configurable system prompts for summary and translation tasks
Earlier work:
php occ, was rejected because oftalk_meetingflag. The idea was to usetext2textprovider for Talk. I made a PR tospreedand it was merged feat(talk): support configurable call summary prompts spreed#19111Related work:
There is already merged functionality for configurable system prompts set by admin #433 (@Abhijeet-035, @lukasdotcom) introduces that, so I think that I should explain myself and my work a little, considering
SummaryProviderI started working on this PR before #433 was merged, so those two PRs overlap. I implemented the prompt per service using
ServiceConfigwhich is why instance-wide setting from #433 is reverted. I reverted it to keep one place in UI instead of two. I am happy to keep global default as fallback if you prefer.Summary system prompts
There are 3 types of system prompts configuration:
user system prompt(Advanced options in Assistant UI)admin system prompt(that can be set in Admin Settings > Assistant)default system prompt(hard-coded in repo:You are a helpful assistant that summarizes text in the same language as the text. You should only return the summary without any additional information.How it works:
user system prompt(if set) overridesadmin system prompt(if set) anddefault system prompt. It also ignoresformatandcomplexityoptions. The idea is that user knows what they want and the can define thiers output format however they wantsadmin system prompt(if set) overwritesdefault system promptand can be overwritten byuser system prompt(if set). It appendsformatandcomplexityoption, so the idea is that prompt should be more general and should not defineformatandcomplexitydefault system promptwill be used only ifuser system promptandadmin system promptare not setTranslation system prompt
I introduce that because I had troubles with translation. Sometimes model outputs only translated text and sometimes it outputs orginal text + translation line by line. It was not coherent
UI look
Admin system prompts for summarization and translation
System promptsText generationso if we toggle that off,System prompts will not be visibleUser defined system prompt for summary
🤖 AI (if applicable)