Add endpoint to upload AI topic & increase max topic length #5847
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5847 +/- ##
=============================================
- Coverage 74.48% 73.33% -1.15%
+ Complexity 5247 5171 -76
=============================================
Files 366 367 +1
Lines 18974 18986 +12
Branches 2020 2016 -4
=============================================
- Hits 14132 13924 -208
- Misses 3886 4074 +188
- Partials 956 988 +32
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Belated thought: should the endpoint automatically set the |
Makes sense, I guess they wouldn't be visible otherwise unless we did something tricky with elastic |
|
|
Updated the PR so that if there is no manual or automatic topic, the topic selection is automatically set to AI. Furthermore, I modified the updateWorkflow and updateContainer endpoint so that it does not update the topicAI field since AI topics are updated by the topic generator, not by end users. |
Does this mean that in those cases, the AI topic automatically appears as the topic in search and/or on the entry page, without entry owner approval? |
Yeah, that's correct, but it should only be for published entries. Are you thinking it should be opt-in instead of opt-out? |
Mixed feelings, automatic inclusion is good for usability, but I feel like there's the possibility that owners might not always approve. Acceptable risk? Not sure. |
Well, this was the intent, to have the topic sentences show up as a last resort but be labelled as AI Many workflows should have a topic one way or another, harvested from the github repo, from the .dockstore.yml, or from the workflow descriptor itself |
| public class UpdateAITopicRequest { | ||
|
|
||
| @NotNull | ||
| @Schema(description = "The AI topic", requiredMode = RequiredMode.REQUIRED) |
There was a problem hiding this comment.
Should the maxLength be set, which would cause requests with longer than TOPIC_LENGTH to fail, or do we want it to get silently truncated, when you invoke abbreviateTopic in Entry.java?
Also, WorkflowResource.updateWorkflow(), which takes a workflow as a parameter, will I believe enforce a TOPIC_LIMIT by not deserializing a workflow with a greater than TOPIC_LENGTH ai topic.
There was a problem hiding this comment.
I went with the silent truncation to match the behaviour of topicAutomatic, which silently truncates topics greater than TOPIC_LENGTH. I'll add a description saying that topics > TOPIC_LENGTH are truncated.
Also, WorkflowResource.updateWorkflow(), which takes a workflow as a parameter, will I believe enforce a TOPIC_LIMIT by not deserializing a workflow with a greater than TOPIC_LENGTH ai topic.
That's true if maxLength is set, so perhaps it's another reason why we shouldn't set it since the AI topic can't be updated with WorkflowResource.updateWorkflow() anymore
Description
Related PRs:
This PR adds an extended TRS endpoint that uploads an AI topic for the entry identified by the TRS ID. If the manual and automatic topics are empty, then the topic selection is automatically set to AI. This new endpoint requires an admin or curator token.
I also removed for end users to update the
topicAIfield using theupdateendpoint since AI topics are updated by the topic generator.This PR also increases the maximum topic length from 150 to 250 characters because there were AI generated topics that exceeded 150 characters. I don't see any harm in doing this because the UI truncates the topics if they exceed 2 lines on the search page. I created a UI PR to update the maxLength allowed for the manual topic to 250 dockstore/dockstore-ui2#1950.
Review Instructions
See instructions in dockstore/dockstore-support#489
Issue
SEAB-6007
Security and Privacy
If there are any concerns that require extra attention from the security team, highlight them here and check the box when complete.
e.g. Does this change...
Please make sure that you've checked the following before submitting your pull request. Thanks!
mvn clean install@RolesAllowedannotation