SEAB-7471: Move AI prompting code to utils.ai package#540
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #540 +/- ##
=============================================
+ Coverage 15.03% 15.31% +0.28%
+ Complexity 107 98 -9
=============================================
Files 50 40 -10
Lines 2475 2338 -137
Branches 196 186 -10
=============================================
- Hits 372 358 -14
+ Misses 2079 1956 -123
Partials 24 24
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
denis-yuen
left a comment
There was a problem hiding this comment.
Some comments on bom stuff
| <usedDependency>software.amazon.awssdk:auth</usedDependency> | ||
| <usedDependency>software.amazon.awssdk:aws-core</usedDependency> | ||
| <usedDependency>software.amazon.awssdk:sdk-core</usedDependency> | ||
| <usedDependency>org.apache.commons:commons-lang3</usedDependency> |
There was a problem hiding this comment.
are these two additions needed?
This https://maven.apache.org/plugins/maven-dependency-plugin/analyze-mojo.html#useddependencies is normally used when the dependency analyzer can't tell if a library is used and you need to force their use
Are they actually being used? (can explain more in stand-up)
There was a problem hiding this comment.
The analyzer thinks they are being used, and if I remove the usedDependency lines, the build dies with an
Unused declared dependencies found error.
Possibly, we could make this better. But, for now, suggest we go with what's here, given that it's dockstore-support and the new usedDependency lines are simply additions to the existing usedDependency block...
There was a problem hiding this comment.
I think the bulky usedDependencies section is part of what's setting off my spidey-sense. It's a lot bigger than the equivalent in the main Dockstore project
https://github.com/dockstore/dockstore/blob/develop/dockstore-webservice/pom.xml#L1119C28-L1130
https://github.com/dockstore/dockstore/blob/develop/dockstore-common/pom.xml#L426-L429
There's a bit of an explanation here https://stackoverflow.com/questions/63885408/maven-dependency-plugin-useddependency-vs-ignoredunuseddeclareddependencies so since we''re not close to a release, I think I'd like to look more into this. And I'm fine with looking into this myself.
By overriding the dependency analyzer too much, it does bloat both the size of the jar but more importantly increases the surface for dependabot/aws inspector complaints,
Quickly looking, I see a few suspicious elements, the version of the dependency plugin is older than the main project and on line 343 below it still says Java 17.
| <usedDependency>software.amazon.awssdk:aws-core</usedDependency> | ||
| <usedDependency>software.amazon.awssdk:auth</usedDependency> | ||
| <usedDependency>software.amazon.awssdk:bedrockruntime</usedDependency> | ||
| <usedDependency>com.google.code.gson:gson</usedDependency> |
There was a problem hiding this comment.
Answer similar to above.
…e/dockstore-support into feature/move-ai-code-into-utils
|
denis-yuen
left a comment
There was a problem hiding this comment.
Note for future selves:
Current belief is
a) Looks trivial that that lang3 and gson really are being used in metrics aggregator and topic generator (they show up in import statements)
b) This means we should not have to use the fancy usedDependencies workaround in enforcer which is meant for harder situations like reflection that it can have trouble with
c) Found a ticket that seems to say that the analyzer can get confused by duplicate declarations (which there probably are since we import parts of the main repo)
d) So re-arranged dependencies and that seems to have greatly reduced the number special declarations we need to do
| <artifactId>httpcore</artifactId> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>io.dockstore</groupId> |
There was a problem hiding this comment.
Think we're running into something like apache/maven-dependency-plugin#1216 which is confusing the maven analyzer, so let the "local" declarations win


Description
This PR moves the AI prompting code out of the topic generator and into the
io.dockstore.utils.aipackage of the utils submodule, so that our nascent "AI categorizer" can use it, too. It also removes the deprecated OpenAI prompting classes.I used Claude Code to write most of this PR. Past experience indicated that when I gave Claude "more than it could chew", potentially, it would go off and spend a lot of tokens doing something in the neighborhood of good, but not quite what I was looking for. Thusly, I broke the task into four subprompts:
I applied these one-by-one, spot-checking and committing each intermediate result, so I could easily revert an errant response from the next prompt, if necessary, (Note: actually, I didn't commit every intermediate result, but I did commit most of them.)
In general, Claude Code did a good job. My favorite thing is that it picked up on some topic generator-specific code that had worked its way into an
AIModelclass (to strip the<summary>tags from the response). Claude removed that code when it copied the classes, and later, added it back to the TopicGenerator itself, where it belonged.It didn't quite understand all of the subtleties of the poms, which I had to adjust by hand. Honestly, sometimes, I don't understand all of the subtleties of the poms, either.
Overall, it went well, but I couldn't shake the nagging unease at being on the outside looking in, playing a role much more akin to a code reviewer than a developer. Typically, the developer has been deep in the code, examining it to determine how to modify it, poking and prodding it, testing it along the way. Conversely, a reviewer lacks much of that context, especially regarding the subtleties, so it's much harder for them to verify correctness.
So, the concern is that, on the occasions when the AI goes sideways, and, for example, hallucinates something well-formed and plausible but wrong, that, despite their best efforts, the "reviewers" won't be able to flag all of it. In other words, unavoidably, some of the better-looking slop will get past us.
A possible countermeasure is testing. What exactly that means is an open question. During standup, Ben mentioned using AI to create the tests, and test-driven development, both of which could be fruitful.
Review Instructions
Confirm that the topic generator is running correctly.
Issue
https://ucsc-cgl.atlassian.net/browse/SEAB-7471
Security
If there are any concerns that require extra attention from the security team, highlight them here.
Please make sure that you've checked the following before submitting your pull request. Thanks!
mvn clean installin the project that you have modified (until https://ucsc-cgl.atlassian.net/browse/SEAB-5300 adds multi-module support properly)