Development: Reuse compliance analysis across languages via snippet mapping - #2470
Conversation
…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
github-actions
Bot
added
server
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Complexity | 1 medium |
🟢 Metrics 8 complexity
Metric Results Complexity 8
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.
|
📊 Server Test Coverage Too Low 🔍 View coverage locally: ./gradlew test jacocoTestReport
open build/reports/jacoco/test/html/index.html🌐 View coverage from GitHub: |
|
📊 Server Test Coverage Too Low 🔍 View coverage locally: ./gradlew test jacocoTestReport
open build/reports/jacoco/test/html/index.html🌐 View coverage from GitHub: |
|
🤖 No OpenAPI or client changes needed. |
ge94zec
changed the title
Chore: Reuse compliance analysis across languages via snippet mapping Development: Reuse compliance analysis across languages via snippet mapping
- updated openapi - added comment to AiService - simplified AiService mapping - updated prompt
ge94zec
changed the title
Development: Reuse compliance analysis across languages via snippet mapping Development: Reuse compliance analysis across languages via snippet mapping
|
📊 Server Test Coverage Too Low 🔍 View coverage locally: ./gradlew test jacocoTestReport
open build/reports/jacoco/test/html/index.html🌐 View coverage from GitHub: |
|
📊 Client Test Coverage Too Low 🔍 View coverage locally: npm run test:ci
open build/test-results/vitest/coverage/index.html🌐 View coverage from GitHub: |
|
🤖 No OpenAPI or client changes needed. |
|
📊 Server Test Coverage Too Low 🔍 View coverage locally: ./gradlew test jacocoTestReport
open build/reports/jacoco/test/html/index.html🌐 View coverage from GitHub: |
|
🤖 No OpenAPI or client changes needed. |
|
📊 Client Test Coverage Too Low 🔍 View coverage locally: npm run test:ci
open build/test-results/vitest/coverage/index.html🌐 View coverage from GitHub: |
…or-compliance' into chore/2346-enhance-performance-for-compliance
|
📊 Server Test Coverage Too Low 🔍 View coverage locally: ./gradlew test jacocoTestReport
open build/reports/jacoco/test/html/index.html🌐 View coverage from GitHub: |
|
🤖 No OpenAPI or client changes needed. |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Complexity | 3 medium |
🟢 Metrics 10 complexity
Metric Results Complexity 10
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.
|
📊 Server Test Coverage Too Low 🔍 View coverage locally: ./gradlew test jacocoTestReport
open build/reports/jacoco/test/html/index.html🌐 View coverage from GitHub: |
|
🤖 No OpenAPI or client changes needed. |
az108
left a comment
az108
left a comment
There was a problem hiding this comment.
Reviewed on top of Cathy's existing review — none of the points below overlap with hers.
The core idea is right: snippet mapping instead of a second full-text analysis is exactly the lever worth pulling here, and applyJobChangeForAnalysis quietly adds the first ownership check on this path — updateAiAnalysis had none on main. Raw List → List<ComplianceIssue> is a good catch too.
The prompt rewrite that ships with it is broken, though.
Blocking
1. {format} breaks every compliance analysis (AnalyzeComplianceText.st:59) — unresolved template variable, StTemplateRenderer defaults to ValidationMode.THROW. Reproduced against the real Spring AI 2.0.0-M4 jars; details inline.
2. Output contract no longer matches ComplianceIssue (AnalyzeComplianceText.st:52) — the prompt asks for text, category, suggestion, but there is no suggestion field anywhere in the project, and article / explanation / action are no longer requested at all. The compliance popover renders nothing but article and explanation.
3. {userLang} silently dropped (AnalyzeComplianceText.st:9) — the parameter is still passed from client through resource to service, and the JavaDoc still promises it controls the explanation language. It no longer does anything.
4. jobId has no Bean Validation (MapComplianceIssuesRequestDTO.java:13) — a request without it runs the LLM mapping, returns 200, and persists nothing without any error or log.
A general note on 1–3: the prompt rewrite looks unrelated to "reuse compliance analysis across languages via snippet mapping". Splitting it into its own PR would have surfaced these — the mapping logic itself is sound.
Test coverage
62be17d06 "removed tests" deletes AiServiceTest.java (418 lines) including five tests for mapComplianceIssues, with no resource-level replacement. That follows the no-*ServiceTest rule, but it leaves the empty-source-issues early return, the LLM catch branch, the size/null mismatch branch, and — most importantly — the silent dropping of non-verbatim snippets (continue after SnippetMatcher.isVerbatim) completely uncovered. SnippetMatcherTest only covers the trivial helper.
Things I checked and found fine
Listing these so they don't come up in a later round:
- LLM call before the ownership check in
mapComplianceIssues— consistent withanalyzeJobDescriptionandextractPdfData, and this PR improves the situation rather than worsening it.isAdminOrMemberOfreturnsvoidand throwsAccessDeniedException, so nothing is being silently ignored. let finalContent: string | null = null— the type is forced by the pre-existingextractTranslatedTextFromStream(): string | null; no lint rule againstnull, and it appears in 77 of 267 client files.- The
if (!jobId) returnmoved to the top oftranslateAndStoreOtherLanguage— both callers run after an awaitedsaveDraftwith no await in between, so it is unreachable defence, not a behaviour change. void-ed analysis promise inprocessDescriptionWithAi—analyzeAndUpdateScorehas nothing throwable outside itstry, andmainhad the same shape withvoid Promise.all([...]).- Fully-qualified
java.util.stream.IntStream— 19 comparable spots insrc/main/java, no checkstyle or Spotless gate on imports. postAndRead(..., Void.class, 400)withoutassertThat—postInvalidasserts the status itself; 81 test methods in the repo do it this way.toHaveBeenCalledWithinstead oftoHaveBeenCalledOnce— the rule targets call-count assertions; 468 of 546toHaveBeenCalledWithin the repo carry no count assertion.
- add @NotNull for jobId - added tests in AiResource
|
🤖 OpenAPI spec and client code auto-updated and committed. |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Security | 1 high |
| Complexity | 1 medium |
🟢 Metrics 29 complexity
Metric Results Complexity 29
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.
|
🤖 No OpenAPI or client changes needed. |
az108
left a comment
az108
left a comment
There was a problem hiding this comment.
Two follow-up points on the already-resolved review threads — both are convention issues, not functional bugs.
The mapping logic itself still looks right, and the earlier blocking items ({format}, the output contract, {userLang}, @NotNull jobId) all check out as fixed.
| import java.util.List; | ||
| import java.util.UUID; | ||
|
|
||
| @JsonInclude |
There was a problem hiding this comment.
@JsonInclude without a value defaults to Include.ALWAYS, which is Jackson's built-in default — so this annotation is a no-op and doesn't actually omit anything. It's also the only valueless @JsonInclude in the repo; the other 124 all carry an explicit value.
Please use the documented default:
@JsonInclude(JsonInclude.Include.NON_EMPTY)Per server-development.mdx:104, NON_EMPTY is preferred; NON_NULL is only for when empty collections or strings are meaningful and should be sent. Matches AssignSlotRequestDTO and BookSlotRequestDTO.
There was a problem hiding this comment.
NON_EMPTY does not work here though because an empty complianceIssues list means “clear all issues for this language.” Omitting it makes the field null, causing @NotNull to return 400. The corresponding AiResourceTest reproduces this behavior.
This is the exception described in server-development.mdx: empty collections are meaningful and must be sent. I therefore switched to @JsonInclude(JsonInclude.Include.NON_NULL). I can also remove the annotation entirely if preferred for request DTOs.
az108
left a comment
az108
left a comment
There was a problem hiding this comment.
Its not 2 seconds as you describe it in the pr description but after you fix the two small code issues we can merge this PR
|
🤖 No OpenAPI or client changes needed. |
- avoid unnecessary null initialization
…or-compliance' into chore/2346-enhance-performance-for-compliance
|
🤖 No OpenAPI or client changes needed. |
|
📊 Server Test Coverage Too Low 🔍 View coverage locally: ./gradlew test jacocoTestReport
open build/reports/jacoco/test/html/index.html🌐 View coverage from GitHub: |
|
🤖 No OpenAPI or client changes needed. |
|
📊 Server Test Coverage Too Low 🔍 View coverage locally: ./gradlew test jacocoTestReport
open build/reports/jacoco/test/html/index.html🌐 View coverage from GitHub: |
1 similar comment
|
📊 Server Test Coverage Too Low 🔍 View coverage locally: ./gradlew test jacocoTestReport
open build/reports/jacoco/test/html/index.html🌐 View coverage from GitHub: |
|
🤖 No OpenAPI or client changes needed. |
3 participants
Checklist
General
Server
Client
Motivation and Context
Closes: #2346
Description
This change improves the bilingual AI compliance workflow for job descriptions by replacing the second target-language compliance analysis with a snippet mapping. The editor now analyzes the source language once in
analyzeJobDescription, streams and stores the translated text intranslateTextStream, and then maps the detected compliance snippets onto the translated version automaticallymapComplianceIssues. This reduces duplicate AI work, keeps translated highlights in sync with sourceIssues.The duplicated compliance issue persistence logic in JobService was extracted into a shared helper. Since the Score is currently a combined value derived from both gender and legal analysis, this ensures the score remains consistent and is only updated when a full, valid recalculation is possible.
LLM compliance calls per autosave cycle is now reduced from 2 to 1. The translation with second analysis is reduced from 11s to approximately 2.30s.
Steps for Testing
Prerequisites:
Review Progress
Code Review
Manual Tests
Screenshots
Test Coverage
Client
Server
Last updated: 2026-08-21 20:22:49 UTC