`Development`: Add action buttons in popover for recommendation system by ge94zec · Pull Request #2520 · ls1intum/doc-apply

ge94zec · 2026-05-14T19:43:59Z

Checklist

General

I tested all changes and their related features with all corresponding user types.
Language: I followed the guidelines for inclusive, diversity-sensitive, and appreciative language.
I chose a title conforming to the naming conventions for pull requests.

Server

Important: I implemented the changes with a very good performance and prevented too many (unnecessary) and too complex database calls.
I strictly followed the principle of data economy for all database calls.
I strictly followed the server coding and design guidelines.

Client

Important: I implemented the changes with a very good performance, prevented too many (unnecessary) REST calls and made sure the UI is responsive, even with large data (e.g. using paging).
I strictly followed the principle of data economy for all client-server REST calls.
I strictly followed the client coding and design guidelines.

Motivation and Context

Description

Steps for Testing

Prerequisites:

Log in to TUMApply as Prof
Nav to Create Position
Verify the action buttons behave correctly and text is saved across page switches

Review Progress

Code Review

Code Review 1

Manual Tests

Test 1

Screenshots

Test Coverage

Client

Class/File	Line Coverage	Lines	Expects	Ratio
job-creation-form.component.ts	74.93%	1406	114	8.1
editor.component.ts	44.29%	367	25	6.8
ai-compliance-popover.component.ts	47.61%	39	?	?
suggestion-system.component.ts	30.00%	17	?	?
compliance-suggestion.util.ts	66.66%	51	?	?

Server

Class/File	Line Coverage	Lines
ComplianceIssue.java	0.00%	27
ComplianceIssueDTO.java	100.00%	29
MapComplianceIssuesRequestDTO.java	100.00%	15
AiService.java	25.65%	400
ComplianceScoreService.java	not found (added)	0

Last updated: 2026-08-24 17:20:18 UTC

…rget text - added endpoint POST that maps source issues to a target language - added prompt that returns one mapped text snippet per line in source complianceIssues order, other fields carry over - wired compliance mapping through client → server API - separated compliance analysis from score calculation - extracted save flow in JobService into Consumer - improved performance by eliminating redundant analysis steps in job creation

- updated openapi - added comment to AiService - simplified AiService mapping - updated prompt

- show suggestions in the compliance popover - add suggestion actions for replace, add, remove, and dismiss - extend compliance prompt and model with suggestion field - migration suggestions field on liquibase

- make job authorization check reusable in JobService - check job access before mapping compliance issues - Clear target-language issues in DB when source list is empty - skip target mapping when source analysis was skipped/failed

…or-compliance' into chore/2346-enhance-performance-for-compliance

- added MapComplianceIssuesRequestDTO for explicit request validation - keep TranslateComplianceDTO focused on translation only. - track AI success and failure for the new mapping LLM call - update openapi

…or-compliance' into chore/2346-enhance-performance-for-compliance

…rformance-for-compliance # Conflicts: # src/main/java/de/tum/cit/aet/job/repository/JobRepository.java # src/main/java/de/tum/cit/aet/job/service/JobService.java # src/main/webapp/app/job/job-creation-form/job-creation-form.component.ts

…buttons-in-popover-for-recommendation-system # Conflicts: # src/main/resources/config/liquibase/master.xml

Removing supervisingProfessor and image from the @EntityGraph caused those associations to become lazy (the FETCH graph overrides entity-level EAGER). getJobById accesses both via the entity returned from assertCanManageJob, so they must be in the graph to avoid LazyInitializationException. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- Refined the compliance suggestion prompt for more accurate/correct output - Added client logic to insert, replace, or remove snippets at the correct index in the editor and persist the change via the form

…buttons-in-popover-for-recommendation-system # Conflicts: # src/main/resources/config/liquibase/master.xml

codacy-production · 2026-05-14T19:45:07Z

Not up to standards ⛔

🔴 Issues 19 high · 4 medium · 1 minor

Alerts:
⚠ 24 issues (≤ 0 issues of at least minor severity)

Results:
24 new issues

Category Results

ErrorProne 2 medium
1 high

Security 18 high

CodeStyle 1 minor

Complexity 2 medium

View in Codacy

🟢 Metrics 78 complexity

Metric Results

Complexity 78

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer}
_{TIP This summary will be updated as you push new changes.}

…lyses - map newly detected issues without retranslating synchronized descriptions - apply accepted suggestions to both language versions - queue actions until target-language mapping completes - insert ADD suggestions after sentence punctuation within list items - prevent duplicate in-flight compliance analyses - preserve mapped highlights and translated suggestions - keep compliance popovers above the sticky footer

github-actions · 2026-08-04T10:05:28Z

📊 Client Test Coverage Too Low

🔍 View coverage locally:

pnpm run test:ci
open build/test-results/vitest/coverage/index.html

🌐 View coverage from GitHub:
Download the "coverage-report-client" artifact from this workflow run.

github-actions · 2026-08-04T10:06:33Z

🤖 No OpenAPI or client changes needed.

codacy-production · 2026-08-04T10:16:02Z

Not up to standards ⛔

🔴 Issues 18 high · 3 medium · 1 minor

Alerts:
⚠ 22 issues (≤ 0 issues of at least minor severity)

Results:
22 new issues

Category Results

ErrorProne 2 medium
1 high

Security 17 high

CodeStyle 1 minor

Complexity 1 medium

View in Codacy

🟢 Metrics 34 complexity

Metric Results

Complexity 34

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer}
_{TIP This summary will be updated as you push new changes.}

github-actions · 2026-08-04T10:18:23Z

🤖 No OpenAPI or client changes needed.

github-actions · 2026-08-11T12:15:50Z

There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.

…recommendation-system

github-actions · 2026-08-17T12:16:16Z

🤖 No OpenAPI or client changes needed.

…buttons-in-popover-for-recommendation-system # Conflicts: # openapi/openapi.yaml # src/main/java/de/tum/cit/aet/ai/dto/MapComplianceIssuesRequestDTO.java # src/main/java/de/tum/cit/aet/ai/service/AiService.java # src/main/resources/prompts/SnippetMapping.st # src/main/webapp/app/generated/model/map-compliance-issues-request-dto.ts # src/main/webapp/app/job/job-creation-form/job-creation-form.component.ts # src/test/java/de/tum/cit/aet/ai/web/rest/AiResourceTest.java # src/test/webapp/app/job/job-creation-form/job-creation-form.component.spec.ts

github-actions · 2026-08-22T12:51:26Z

🤖 No OpenAPI or client changes needed.

- translate existing compliance suggestions during snippet mapping - preserve suggestion meaning across languages - require exact target-text matches for mapped snippets

…buttons-in-popover-for-recommendation-system # Conflicts: # src/main/java/de/tum/cit/aet/ai/service/AiService.java # src/main/java/de/tum/cit/aet/ai/service/ComplianceScoreService.java # src/main/resources/config/liquibase/master.xml # src/main/webapp/app/shared/components/atoms/editor/editor.component.ts # src/main/webapp/app/shared/components/molecules/ai-compliance-popover/ai-compliance-popover.component.ts # src/test/java/de/tum/cit/aet/ai/service/ComplianceScoreServiceTest.java # src/test/webapp/app/job/job-creation-form/job-creation-form.component.spec.ts

github-actions · 2026-08-24T17:12:26Z

📊 Server Test Coverage Too Low

🔍 View coverage locally:

./gradlew test jacocoTestReport
open build/reports/jacoco/test/html/index.html

🌐 View coverage from GitHub:
Download the "coverage-report-server" artifact from this workflow run.

github-actions · 2026-08-24T17:13:26Z

🤖 No OpenAPI or client changes needed.

github-actions · 2026-08-24T17:17:08Z

🤖 No OpenAPI or client changes needed.

az108

All ten earlier threads are resolved, and with #2465 and #2470 now merged the diff is finally a reviewable 28 files. Four things, one of them a real behaviour bug — details inline.

Two 0-byte files are committed

src/main/java/de/tum/cit/aet/ai/service/ComplianceScoreService.java      0 bytes
src/test/java/de/tum/cit/aet/ai/service/ComplianceScoreServiceTest.java  0 bytes

Both were moved to ai/util/ComplianceScoreCalculator by #2465 and deleted on main. Merge commit 936070dc4 hit a modify/delete conflict and resolved it by keeping the files and emptying them. ComplianceScoreCalculator.java and its test exist intact alongside, so no coverage is lost — this is pure dead weight.

It doesn't break anything (an empty .java is a valid compilation unit, and compileJava/compileTestJava pass), and grep finds zero references to ComplianceScoreService anywhere in the repo. They can just be deleted. I couldn't leave an inline comment since there are no lines to anchor to.

Unrelated, but worth knowing

PR #2638 (feat/2637-port-new-gender-contracts) also claims 00000000000053, with a file that is a byte-identical duplicate of the already-merged 052 and isn't registered in master.xml at all — so it would never run. No git conflict with this PR (different filenames, and #2638 doesn't touch master.xml), and the defect is on that side. Flagging it here only because it surfaced while checking the numbering.

az108 · 2026-08-25T09:18:15Z

+    const dismissedIssues = this.dismissedComplianceHighlights();
    const highlights = (compliance ?? []).flatMap(issue =>
-      hasText(issue.text) && issue.category !== undefined && (!hasText(issue.language) || issue.language === lang)
+      !dismissedIssues.includes(issue) &&


dismissedComplianceHighlights stores object references and this filters with .includes(issue) — SameValueZero, so reference equality on a generated plain interface with no identity field. That only holds while the array keeps the same references, and the early-return branch added in this PR at line 1847 breaks exactly that: translateAndStoreOtherLanguage now calls mapIssuesToTargetLanguage even when the text is unchanged, and line 1998 replaces the target-language issues with fresh objects from the mapComplianceIssues response.

Reachable flow, with no re-analysis anywhere in it:

Draft analyzed in EN, translated to DE, DE issues mapped

On the DE tab, dismiss a highlight — disappears correctly

Back on EN, edit any other field (title, location, workload) so autosave fires

performAnalysis('en') cache-returns: no HTTP, no set

The new early-return branch runs mapComplianceIssues, and line 1998 swaps the DE issues for fresh objects — invisible, since you are on EN

Switch to DE: languageChangeEffect → applyHighlights → the dismissed highlight is back

The dismissed object also stays in the array forever as a dead reference.

The four complianceIssues.set(...) call sites separate cleanly: 1467 and 1575 are load paths, 2050 is a real analysis (resetting dismissals there is defensible), but 1998 is pure re-mapping of unchanged text and should not clear anything.

Suggestion: key on something stable rather than identity, e.g. `${issue.language ?? ''}|${issue.id ?? issue.text ?? ''}`. AiService.java:561 carries sourceIssue.getId() through the mapping round-trip, so IDs survive mapping but not a fresh analysis — which is exactly the granularity you want. The text fallback matters because id is optional and the local gender-bias path does not always set it.

If reset-on-new-analysis is intended, worth doing explicitly at 2050 rather than as a side effect of reference equality — that also stops the array growing unbounded.

Dismiss is not covered by the new specs, so this would not have been caught.

az108 · 2026-08-25T09:18:15Z

+      const updatedTargetHtml = applyComplianceSuggestionToHtml(currentTargetHtml, acceptedIssue);
+      if (updatedTargetHtml === undefined) continue;
+
+      this.pendingMappedActions.delete(acceptedIssue.id!);


this.pendingMappedActions.delete(acceptedIssue.id!);

client-development.mdx:230: "Never use the TypeScript non-null assertion operator (!) to bypass type checks. It hides potential runtime errors and defeats the purpose of strict typing."

Worth noting this isn't an established pattern to follow — there is exactly one other real ! assertion in the entire client (document-cache.service.ts:33), so this would be the second.

hasText is a type guard (text.util.ts:8), but the && in the filter on line 1974 produces a plain boolean rather than a predicate on issue, so filter still returns ComplianceIssue[] and id is back to string | undefined in the loop. A local const plus a guard next to the continue you already have:

for (const acceptedIssue of acceptedMappedIssues) { const acceptedIssueId = acceptedIssue.id; if (!hasText(acceptedIssueId)) continue; ... this.pendingMappedActions.delete(acceptedIssueId);

(ESLint won't flag it — no-non-null-assertion is off in eslint.config.mjs:174 — so this is convention rather than a lint break.)

az108 · 2026-08-25T09:18:15Z

+  xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+  xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-latest.xsd">
+
+  <changeSet id="052_add_suggestion_field_to_complianceIssue" author="melissa">


The changeset id is prefixed 052_ but the file is numbered 053. Per liquibase-guidelines.mdx:41, the id should start with the same changelog number as the file:

<changeSet id="053_add_suggestion_field_to_compliance_issue" author="melissa">

No functional collision — Liquibase keys on id+author+filename, and there are zero duplicate ids repo-wide — but the convention is actively followed: 29 of the 31 files in the 023–053 range conform.

Two smaller things in the same file while you're in there:

The filename description is camelCase (complianceIssue). It's the only camelCase description among all 54 changelogs; liquibase-guidelines.mdx:20 asks for snake_case.

Missing <preConditions onFail="MARK_RAN"> — I checked and am not raising this as a real issue: only 7 of 54 changelogs use them, and a plain addColumn on a column that never existed runs exactly once via DATABASECHANGELOG anyway. Mentioning only so it doesn't get raised later.

Confirmed the migration itself applies: JobResourceTest runs the full changelog against Testcontainers MySQL, 37 tests pass.

az108 · 2026-08-25T10:01:30Z

@@ -560,6 +565,7 @@ public List<ComplianceIssue> mapComplianceIssues(MapComplianceIssuesRequestDTO r
                    sourceIssue.getArticle(),
                    sourceIssue.getExplanation(),
                    sourceIssue.getAction(),
+                    mappedTexts.get(i * 2 + 1).trim(),
                    request.toLang()


can you add a little bit of java doc explaining the i and the calculations here

az108 · 2026-08-25T10:33:46Z

+   * @param issue The compliance issue containing the action, snippet text, and suggestion.
+   * @returns The updated editor HTML for issued text snippet
+   */
+  public applyComplianceSuggestion(issue: ComplianceIssue): string | undefined {


Design point: this puts page-specific domain logic into a shared atom, and the coupling here is a different kind from what the editor carried before.

EditorComponent lives in shared/components/atoms/ and is consumed by eight features — research-group template edit, research-group info, research-group detail view, research-group creation form, review dialog, interviewee assessment, application creation page 3, and job creation. Only the last one has anything to do with compliance.

On main the editor imported exactly two compliance symbols:

ComplianceIssueDTOCategoryEnum // pick the highlight underline colour ComplianceIssueDTOCategoryEnumValues

That is narrow and defensible — the atom needs a category the same way it needs a font size. This PR adds ComplianceIssue (the whole DTO) and ComplianceIssueActionEnum, and applyComplianceSuggestion switches on issue.action to decide what REPLACE / REMOVE / ADD mean. The atom went from "knows compliance categories for styling" to "knows the compliance issue model and executes its business rules".

The PR already contains the better design. compliance-suggestion.util.ts implements applyComplianceSuggestionToHtml(html, issue) — the same rule (locate snippet, switch on action, Replace/Remove/Add with sentence-end insertion) as a pure function. This method even imports findSentenceEnd from it, so the extraction was started and then stopped halfway. The result is one rule with two implementations and two different effectors: the Quill API here, DOM Range there.

And they already disagree, which is the part that is a bug today rather than a preference. When the snippet is not found:

ADD action, snippet not found

editor.component.ts:435 insertAt = editor.getLength() — appends the sentence at the end of the document

compliance-suggestion.util.ts:28 if (start === -1) return undefined — no change at all

onComplianceSuggestionAccepted runs both paths for a single accept: this method for the active language, the util for the paired-language HTML. So accepting one ADD suggestion whose snippet is missing in the other language bolts a sentence onto the end of the active editor and silently does nothing to the other. That falls straight out of having the rule in two places.

Suggested shape — keep the atom's real job ("apply a text edit to the live Quill document") and drop the domain:

// atom, domain-free public applyTextEdit(edit: { index: number; deleteLength: number; insert: string }): string | undefined

The page then computes the edit from the issue via the util, which already knows how. That removes ComplianceIssue and ComplianceIssueActionEnum from the atom entirely, collapses the two implementations into one, and makes the EN/DE paths symmetric — which fixes the divergence above as a side effect rather than leaving it to surface later.

For context, the gender-decoder surface already on this component (showGenderDecoderButton, biasedAnalysis, genderDecoderClick, showAnalysisModal) is the same pattern and predates this PR, so this is a direction worth stopping rather than something introduced here from scratch.

ge94zec and others added 26 commits May 5, 2026 09:56

updated openapi

9adc3e9

refactor: code

7946480

- updated openapi - added comment to AiService - simplified AiService mapping - updated prompt

feat: add interactive compliance suggestions

941bd88

- show suggestions in the compliance popover - add suggestion actions for replace, add, remove, and dismiss - extend compliance prompt and model with suggestion field - migration suggestions field on liquibase

Merge branch 'main' into chore/2346-enhance-performance-for-compliance

7433eeb

updates:

5579180

- make job authorization check reusable in JobService - check job access before mapping compliance issues - Clear target-language issues in DB when source list is empty - skip target mapping when source analysis was skipped/failed

Merge remote-tracking branch 'origin/chore/2346-enhance-performance-f…

97f4653

…or-compliance' into chore/2346-enhance-performance-for-compliance

Merge branch 'main' into chore/2346-enhance-performance-for-compliance

bd80f96

updates:

2d02039

- added MapComplianceIssuesRequestDTO for explicit request validation - keep TranslateComplianceDTO focused on translation only. - track AI success and failure for the new mapping LLM call - update openapi

Merge remote-tracking branch 'origin/chore/2346-enhance-performance-f…

23549fd

…or-compliance' into chore/2346-enhance-performance-for-compliance

chore: update OpenAPI spec and generated client

102ac7b

Merge branch 'main' into chore/2346-enhance-performance-for-compliance

b0c19fb

Merge branch 'main' into chore/2346-enhance-performance-for-compliance

8d001be

Merge branch 'main' into chore/2346-enhance-performance-for-compliance

ee463f7

change requests

b7e0862

chore: update OpenAPI spec and generated client

11e553a

updated client

26f3cbd

added AiServiceTest for analyze and map

702dd6c

Merge remote-tracking branch 'origin/main' into feat/2348-add-action-…

b76eb11

…buttons-in-popover-for-recommendation-system # Conflicts: # src/main/resources/config/liquibase/master.xml

Merge branch 'main' into chore/2346-enhance-performance-for-compliance

9f21677

Merge branch 'main' into chore/2346-enhance-performance-for-compliance

630a36f

feat(compliance): apply AI suggestions to job description editor

fedcb06

- Refined the compliance suggestion prompt for more accurate/correct output - Added client logic to insert, replace, or remove snippets at the correct index in the editor and persist the change via the form

Merge remote-tracking branch 'origin/main' into feat/2348-add-action-…

f09b600

…buttons-in-popover-for-recommendation-system # Conflicts: # src/main/resources/config/liquibase/master.xml

fix tests

01a92c8

ge94zec linked an issue May 14, 2026 that may be closed by this pull request

Add action buttons in popover for recommendation system #2348

Open

ge94zec changed the title ~~Feat/2348 add action buttons in popover for recommendation system~~ Development: Add action buttons in popover for recommendation system May 14, 2026

ge94zec self-assigned this May 14, 2026

github-actions Bot removed the stale label Aug 2, 2026

ge94zec added 2 commits August 2, 2026 21:02

lint fix

39d10d0

github-actions Bot added the job Code changes in job module. label Aug 4, 2026

prettier

3c44084

github-actions Bot added the stale label Aug 11, 2026

az108 mentioned this pull request Aug 11, 2026

Bugfix: Fix excessive api requests triggering genderdecoder #2465

Merged

11 tasks

Merge branch 'main' into feat/2348-add-action-buttons-in-popover-for-…

00bb4ea

…recommendation-system

github-actions Bot removed the job Code changes in job module. label Aug 22, 2026

helios-aet Bot deployed to test-server August 22, 2026 13:05 Active

github-actions Bot removed the stale label Aug 23, 2026

ge94zec added 2 commits August 24, 2026 18:50

fix: add suggestion and improve compliance issue detection

a2454dc

- translate existing compliance suggestions during snippet mapping - preserve suggestion meaning across languages - require exact target-text matches for mapped snippets

fix tests

f012189

ge94zec requested a review from az108 August 24, 2026 18:15

az108 reviewed Aug 25, 2026

View reviewed changes

This was referenced Aug 25, 2026

Development: Show gender-inclusive wording feedback in the AI sidebar #2630

Closed

General: Align compliance rules and stabilize generation workflow #2631

Open

az108 requested changes Aug 25, 2026

View reviewed changes

	ADD action, snippet not found
`editor.component.ts:435`	`insertAt = editor.getLength()` — appends the sentence at the end of the document
`compliance-suggestion.util.ts:28`	`if (start === -1) return undefined` — no change at all

Uh oh!

Conversation

ge94zec commented May 14, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

General

Server

Client

Motivation and Context

Description

Steps for Testing

Review Progress

Code Review

Manual Tests

Screenshots

Test Coverage

Client

Server

Uh oh!

codacy-production Bot commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Not up to standards ⛔

Uh oh!

github-actions Bot commented Aug 4, 2026

Uh oh!

github-actions Bot commented Aug 4, 2026

Uh oh!

codacy-production Bot commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Not up to standards ⛔

Uh oh!

github-actions Bot commented Aug 4, 2026

Uh oh!

github-actions Bot commented Aug 11, 2026

Uh oh!

github-actions Bot commented Aug 17, 2026

Uh oh!

github-actions Bot commented Aug 22, 2026

Uh oh!

github-actions Bot commented Aug 24, 2026

Uh oh!

github-actions Bot commented Aug 24, 2026

Uh oh!

github-actions Bot commented Aug 24, 2026

Uh oh!

az108 left a comment

Choose a reason for hiding this comment

Two 0-byte files are committed

Unrelated, but worth knowing

Uh oh!

az108 Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

az108 Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

az108 Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

az108 Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

az108 Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ge94zec commented May 14, 2026 •

edited by github-actions Bot

Loading

codacy-production Bot commented May 14, 2026 •

edited

Loading

codacy-production Bot commented Aug 4, 2026 •

edited

Loading