-
Notifications
You must be signed in to change notification settings - Fork 4
General: Align compliance rules and stabilize generation workflow
#2631
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
Open
ge94zec
wants to merge
14
commits into
main
Choose a base branch
from
chore/2535-modularize-compliance-rules-across-prompts
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
1bcaf8c
chore: modular prompting
ge94zec 8edf1a3
prettier
ge94zec 4c10896
Merge branch 'main' into chore/2535-modularize-compliance-rules-acros…
ge94zec 785a8a1
Merge branch 'main' into chore/2535-modularize-compliance-rules-acros…
ge94zec c89c8e1
refactored compliance rules
ge94zec 6ea2877
Merge branch 'main' into chore/2535-modularize-compliance-rules-acros…
ge94zec c8bf831
- add compliance rules to generation prompt
ge94zec b924b67
Merge remote-tracking branch 'origin/chore/2535-modularize-compliance…
ge94zec 95234d9
fix(ai): prioritize generation and cancel stale AI workflows
ge94zec 60cec24
chore: update OpenAPI spec and generated client
github-actions[bot] 5d97168
updated transparency policy in JobDescriptionGeneration.st
ge94zec fba1add
Merge remote-tracking branch 'origin/chore/2535-modularize-compliance…
ge94zec 5676856
Merge branch 'main' into chore/2535-modularize-compliance-rules-acros…
ge94zec e133704
- changed prompt back to origin
ge94zec File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
36 changes: 36 additions & 0 deletions
36
src/main/java/de/tum/cit/aet/ai/dto/ComplianceIssueDTO.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| package de.tum.cit.aet.ai.dto; | ||
|
|
||
| import com.fasterxml.jackson.annotation.JsonInclude; | ||
| import de.tum.cit.aet.ai.constants.ComplianceAction; | ||
| import de.tum.cit.aet.ai.constants.ComplianceCategory; | ||
| import de.tum.cit.aet.ai.domain.ComplianceIssue; | ||
|
|
||
| /** Response DTO for one compliance issue. */ | ||
| @JsonInclude(JsonInclude.Include.NON_EMPTY) | ||
| public record ComplianceIssueDTO( | ||
| String id, | ||
| ComplianceCategory category, | ||
| String text, | ||
| String article, | ||
| String explanation, | ||
| ComplianceAction action, | ||
| String language | ||
| ) { | ||
| /** | ||
| * Creates an API response from the persisted compliance value. | ||
| * | ||
| * @param issue the persisted compliance issue | ||
| * @return the response DTO | ||
| */ | ||
| public static ComplianceIssueDTO from(ComplianceIssue issue) { | ||
| return new ComplianceIssueDTO( | ||
| issue.getId(), | ||
| issue.getCategory(), | ||
| issue.getText(), | ||
| issue.getArticle(), | ||
| issue.getExplanation(), | ||
| issue.getAction(), | ||
| issue.getLanguage() | ||
| ); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| package de.tum.cit.aet.ai.dto; | ||
|
|
||
| import com.fasterxml.jackson.annotation.JsonInclude; | ||
| import de.tum.cit.aet.ai.domain.ComplianceIssue; | ||
| import java.util.List; | ||
|
|
||
| /** Response DTO for job-description analysis. */ | ||
| @JsonInclude(JsonInclude.Include.NON_EMPTY) | ||
| public record JobAnalysisDTO(int score, List<ComplianceIssueDTO> issues) { | ||
| /** | ||
| * Creates the response while keeping persistence models behind the DTO boundary. | ||
| * | ||
| * @param score the persisted combined score | ||
| * @param issues the detected compliance issues | ||
| * @return the analysis response | ||
| */ | ||
| public static JobAnalysisDTO from(int score, List<ComplianceIssue> issues) { | ||
| return new JobAnalysisDTO(score, issues.stream().map(ComplianceIssueDTO::from).toList()); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
72 changes: 72 additions & 0 deletions
72
src/main/java/de/tum/cit/aet/ai/service/AiPriorityService.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| package de.tum.cit.aet.ai.service; | ||
|
|
||
| import java.util.Map; | ||
| import java.util.Set; | ||
| import java.util.UUID; | ||
| import java.util.concurrent.CancellationException; | ||
| import java.util.concurrent.ConcurrentHashMap; | ||
| import org.springframework.stereotype.Service; | ||
| import reactor.core.publisher.Flux; | ||
| import reactor.core.publisher.Mono; | ||
| import reactor.core.publisher.Sinks; | ||
|
|
||
| /** Coordinates foreground generation and cancellable background AI work per job. */ | ||
| @Service | ||
| public class AiPriorityService { | ||
|
|
||
| private final Map<UUID, Set<Sinks.Empty<Void>>> backgroundCancellations = new ConcurrentHashMap<>(); | ||
|
|
||
| /** | ||
| * Cancels existing background work for the job before generation starts. | ||
| * | ||
| * @param jobId the job owning the AI workflow | ||
| * @param source the generation stream | ||
| * @return the generation stream | ||
| * @param <T> the streamed response type | ||
| */ | ||
| public <T> Flux<T> foreground(UUID jobId, Flux<T> source) { | ||
| if (jobId == null) { | ||
| return source; | ||
| } | ||
| return Flux.defer(() -> { | ||
| cancelBackground(jobId); | ||
| return source; | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Registers background work so foreground generation for the same job can cancel it. | ||
| * | ||
| * @param jobId the job owning the AI workflow | ||
| * @param source the background stream | ||
| * @return the cancellable background stream | ||
| * @param <T> the streamed response type | ||
| */ | ||
| public <T> Flux<T> background(UUID jobId, Flux<T> source) { | ||
| if (jobId == null) { | ||
| return source; | ||
| } | ||
| return Flux.defer(() -> { | ||
| Sinks.Empty<Void> cancellation = Sinks.empty(); | ||
| backgroundCancellations.computeIfAbsent(jobId, _ -> ConcurrentHashMap.newKeySet()).add(cancellation); | ||
| Mono<T> cancellationError = cancellation | ||
| .asMono() | ||
| .then(Mono.error(new CancellationException("AI request superseded by generation"))); | ||
| return source.takeUntilOther(cancellationError).doFinally(_ -> unregister(jobId, cancellation)); | ||
| }); | ||
| } | ||
|
|
||
| private void cancelBackground(UUID jobId) { | ||
| Set<Sinks.Empty<Void>> cancellations = backgroundCancellations.remove(jobId); | ||
| if (cancellations != null) { | ||
| cancellations.forEach(Sinks.Empty::tryEmitEmpty); | ||
| } | ||
| } | ||
|
|
||
| private void unregister(UUID jobId, Sinks.Empty<Void> cancellation) { | ||
| backgroundCancellations.computeIfPresent(jobId, (_, cancellations) -> { | ||
| cancellations.remove(cancellation); | ||
| return cancellations.isEmpty() ? null : cancellations; | ||
| }); | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Check-then-act:
computeIfAbsentreturns the set, then.add(cancellation)runs outside any map lock — whilecancelBackground(line 60) doesbackgroundCancellations.remove(jobId). If the remove lands between the two, the new cancellation goes into a set that is no longer in the map, so that background stream is never cancelled — which is the whole purpose of this class.Being straight about severity: the window is a few bytecode instructions, so a hit is unlikely. Worth fixing anyway because the fix is a risk-free one-liner and the consequence is not cosmetic — an orphaned cancellation means a full background LLM call runs alongside the generation, and in the compliance path
jobService.updateAiAnalysis(AiService.java:523) then writes a score computed from the pre-generation text.computeholds the bin lock across the entire mapping function, so it is mutually exclusive withremoveon the same key:No memory leak either way —
unregisterusescomputeIfPresent, so in the race case it degrades to a safe no-op and the orphaned set is collected.Separate, lower-severity point on the same class:
cancelBackgroundruns exactly once, at foreground subscribe, and leaves no marker behind. Generation streams for seconds, so background work registered after that point runs unimpeded. The client closes the obvious path —executeAutoSavebails onisGeneratingDraft()(job-creation-form.component.ts:1662) and re-checksrun.isStale()after the save round-trip — but two paths remain, both with millisecond rather than nanosecond windows: two tabs on the same draft (no sharedactiveAiRun, which is presumably why this state is keyed byjobIdserver-side), and latency skew where generate is sent later but arrives first. Note thecomputefix above does not close this one. A foreground marker thatbackgroundchecks at registration would.