Introduce LanguageModel/TextProcessing OCP API #38854
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
ChristophWurst
added
enhancement
2. developing
There was a problem hiding this comment.
Noice!
- Setting default provider per task type
Any idea on how this would be done? We could start and avoid thinking about making this appear in the UI. How would the information be set and stored? occ command, OCS endpoint? Stored in a dedicated table? Consumed by the manager's runTask method so it would really reflect the current value?
I think it would make sense to do this in a new PR together with the settings for translations and STT. |
|
Looks nice, thank you! I've suggested replacing !empty with count() > 0. Backstory is https://localheinz.com/articles/2023/05/10/avoiding-empty-in-php/#content-array which recommends not to use empty when the code is properly typed. |
|
I've never written tests for server before, so please bear with me. I hope this first commit goes in the right direction :) |
|
Tests are in :) |
This comment was marked as off-topic.
This comment was marked as off-topic.
|
Hey :)
I'm somewhat familiar with unit and integration tests. I tend to not buy into unit tests and rather go with integration tests, as I feel unit tests have a low return on investment.
I did mock most if not all of the unrelated classes. Currently it takes 6s for me.
I will get to writing unit tests, then :) Can we finalize the API first, then? That would avoid blocking you for implementing the downstream stuff and me for the docs. |
This comment was marked as off-topic.
This comment was marked as off-topic.
| $this->jobList->add(TaskBackgroundJob::class, [ | ||
| 'taskId' => $task->getId() | ||
| ]); |
There was a problem hiding this comment.
Best case scenario, the job runs next time cron.php is executed, right?
If so, maybe we could think about allowing providers to say they will be fast and run the task immediately. All good for now, just thinking ahead.
| * | ||
| * @PublicPage | ||
| */ | ||
| public function taskTypes(): DataResponse { |
There was a problem hiding this comment.
As discussed, maybe it makes sense to have another endpoint to list the providers (maybe grouped by task type).
Also, maybe we could include the "selected provider" for each task type in the result of this taskTypes method.
WDYT?
There was a problem hiding this comment.
Do we really need the providers on the frontend though? I think we would usually want to hide them, no?
There was a problem hiding this comment.
Yeah they wouldn't be displayed in the text processing UI component but maybe it's useful for the settings part where admins can choose the "default" provider for each task type.
Also, maybe we could include the "selected provider" for each task type in the result of this taskTypes method.
What do you think about that other aspect? It might make sense to let advanced users know which provider is used when choosing a task type. This information would not necessarily be very visible in the UI. But just in case we find a nice way to give more details if users want to know.
There was a problem hiding this comment.
the "selected provider" for each task type
I tend to think this should be hidden from the user. Only the admin should see this.
maybe it's useful for the settings part where admins can choose the "default" provider for each task type
I'll deal with that in the follow up PR :)
Signed-off-by: Marcel Klehr <[email protected]>
Signed-off-by: Marcel Klehr <[email protected]>
Signed-off-by: Marcel Klehr <[email protected]>
|
🎉 🚢 |
|
@marcelklehr backport to 27 needed? |
|
@AndyScherzinger Yes! If we backport now, this is gonna land in 27.0.2, though. |
|
Ah, right 👍 Good point 🫣 |
|
/backport to stable27 |
|
The backport to stable27 failed. Please do this backport manually. # Switch to the target branch and update it
git checkout stable27
git pull origin stable27
# Create the new backport branch
git checkout -b fix/foo-stable27
# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123
# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable27More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport |
| * along with this program. If not, see <http://www.gnu.org/licenses/> | ||
| * | ||
| */ | ||
| namespace OCP\Common\Exception; |
There was a problem hiding this comment.
This is quite an anti pattern and we really try hard to get rid of the OC_Util and OCP\Util, so let's not start the next thing we can never change because it is used by multiple mechanisms in different ways.
Can we deprecate this (or even revert since it was never released apart from beta) and instead add a dedicated TaskNotFound exception inside the text processing namespace?
There was a problem hiding this comment.
It was suggested to me by other reviewers 🤷
I'm on vacation until end of August. Feel free to change this in my absence.
There was a problem hiding this comment.
Just read up on the comments... alright then :(
And enjoy your hols! 😎
| $taskEntity = $this->taskMapper->find($id); | ||
| return $taskEntity->toPublicTask(); | ||
| } catch (DoesNotExistException $e) { | ||
| throw new NotFoundException('Could not find task with the provided id'); |
There was a problem hiding this comment.
As per other comment, either (re)throw DoesNotExistException which is already a generic exception with the same meaning, or a dedicated TaskNotFound
Summary
This adds a LanguageModel namespace with anILanguageModelManagerthat queries registeredILanguageModelProviders to either synchronously run or asynchronously scheduleILanguageModelTasks. When scheduled asynchronously, the tasks will be run in a background job and either aTaskFailedEventor aTaskSuccessfulEventis emitted with error message or the output, respectively. Currently implemented are the following task types:SummarizeTask,HeadlineTask,TopicsTaskandFreePromptTask.In coordination with #julien-nc we reworked this PR to merge it with another upcoming API. Now, this adds a TextProcessing namespace with an
IManagerthat queries registeredIProviders to either synchronously run or asynchronously scheduleTasks. When scheduled asynchronously, the tasks will be run in a background job and either aTaskFailedEventor aTaskSuccessfulEventis emitted with error message or the output, respectively. Currently implemented are the following task types:SummaryTaskType,HeadlineTaskType,TopicsTaskTypeandFreePromptTaskType. Apps can register providers for existing task types in core or use their own task types, which we may pull into core at some point, if we want.TODO
Checklist