Skip to content

Bugfix: Fix excessive api requests triggering genderdecoder - #2465

Open
ge94zec wants to merge 125 commits into
mainfrom
bugfix/2379-fix-excessive-api-requests-triggering-genderdecoder
Open

Bugfix: Fix excessive api requests triggering genderdecoder#2465
ge94zec wants to merge 125 commits into
mainfrom
bugfix/2379-fix-excessive-api-requests-triggering-genderdecoder

Conversation

@ge94zec

@ge94zec ge94zec commented May 3, 2026

Copy link
Copy Markdown
Contributor

Checklist

General

Server

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

Closes: #2379

Description

This PR refactors and improves the gender bias analysis and across page reloads and api requests. It introduces a new INCLUSIVE / NON_INCLUSIVE classification and replaces the previous response DTO GenderResponse with BiasedIssues, simplifying data handling and decoupling bias detection from language.

Bias analysis is now persisted directly in the database using @ElementCollection with a Liquibase migration, and the server-side flow has been streamlined by refactoring services, improving how language-specific results are merged and stored. Additional repository changes prevent Hibernate reload issues.

On client-side, duplicate/numerous API requests during job creation are reduced through better deduplication and improved execution flow. The job creation flow now analyzes the genderDecoder and after that the source language while running translation and secondary analysis in the background.

Steps for Testing

Prerequisites:

  1. Log in to TUMApply as Prof
  2. Nav to Create Position
  3. Verify wit dev tool network that biasedIssues are included in jobId and are persistent across page reloads or 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 73.54% 1285 103 8.0
editor.component.ts 83.67% 335 25 7.5
ai-assistant-card.component.ts 71.42% 117 4 3.4
ai-compliance-popover.component.ts 100.00% 14 ? ?
gender-bias-analysis-dialog.ts 100.00% 72 15 20.8
gender-bias-analysis.ts not found (deleted) ? 2 ?
gender-bias-analysis.utils.ts 100.00% 9 ? ?

Server

Class/File Line Coverage Lines
BiasedIssue.java 0.00% 22
AnalyzeJobDescriptionRequestDTO.java 100.00% 4
BiasedIssueDTO.java 100.00% 10
ComplianceIssueDTO.java 100.00% 27
JobAnalysisDTO.java 100.00% 16
TranslateComplianceDTO.java 100.00% 5
AiService.java 10.33% 334
ComplianceScoreService.java not found (deleted) ?
GenderBiasAnalysisService.java 100.00% 63
ComplianceScoreCalculator.java 92.68% 86
AiResource.java 43.48% 89
GenderBiasWordLists.java 91.67% 258
GenderCategory.java 100.00% 5
BiasedWordDTO.java not found (deleted) ?
GenderBiasAnalysisRequest.java not found (deleted) ?
GenderBiasAnalysisResponse.java not found (deleted) ?
GenderBiasAnalysisService.java 100.00% ?
GenderBiasAnalyzer.java 94.12% 60
GenderBiasAnalysisResource.java not found (deleted) ?
Job.java 100.00% 90
JobDTO.java 100.00% 41
JobFormDTO.java 93.33% 78
JobRepository.java 85.71% 243
JobService.java 82.68% 430
JobResource.java 90.63% 121

Last updated: 2026-08-17 21:39:26 UTC

ge94zec and others added 30 commits April 19, 2026 20:11
…or-not-shown' into bugfix/2321-title-compliance-error-not-shown
…or-not-shown' into bugfix/2321-title-compliance-error-not-shown
prettier
fix naming
update openapi
…egories-in-sidebar' into feat/2345-add-new-compliance-categories-in-sidebar
fix namings
…egories-in-sidebar' into feat/2345-add-new-compliance-categories-in-sidebar
@github-actions

Copy link
Copy Markdown
Contributor

📊 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

Copy link
Copy Markdown
Contributor

🤖 No OpenAPI or client changes needed.

- split rule-based gender analysis from the AI endpoint
- guard LLM compliance analysis on client and server
- preserve gender scoring when AI is unavailable
- prevent duplicate compliance issues
- add consent and gender-analysis regression tests
@github-actions

Copy link
Copy Markdown
Contributor

📊 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

Copy link
Copy Markdown
Contributor

🤖 No OpenAPI or client changes needed.

@github-actions

Copy link
Copy Markdown
Contributor

📊 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.

fix server tests
@github-actions

Copy link
Copy Markdown
Contributor

📊 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

Copy link
Copy Markdown
Contributor

🤖 No OpenAPI or client changes needed.

@github-actions

Copy link
Copy Markdown
Contributor

📊 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

Copy link
Copy Markdown
Contributor

🤖 No OpenAPI or client changes needed.

@az108 az108 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two follow-ups that keep getting lost in old threads, re-raised as fresh comments so they're actually visible.

Everything else from my previous rounds is now resolved — I've gone through and closed out the threads that are genuinely done (the dedup fix, the wildcard imports, the shared originalText, the JavaDoc rewrite, the unused import, the ai_score rename, and the removed service test). The only other thing still open is the consent leak on job-creation-form.component.ts:1684.

Comment thread src/test/webapp/app/job/job-creation-form/job-creation-form.component.spec.ts Outdated
@helios-aet
helios-aet Bot deployed to test-server August 17, 2026 08:25 Active
fix tests
@codacy-production

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 2 high · 1 medium

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

Results:
3 new issues

Category Results
Security 2 high
Complexity 1 medium

View in Codacy

🟢 Metrics 2 complexity

Metric Results
Complexity 2

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

Copy link
Copy Markdown
Contributor

🤖 No OpenAPI or client changes needed.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 No OpenAPI or client changes needed.

@helios-aet
helios-aet Bot deployed to test-server August 17, 2026 09:03 Active
@github-actions

Copy link
Copy Markdown
Contributor

🤖 No OpenAPI or client changes needed.

@az108

az108 commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Remaining items @ge94zec

Everything from my previous reviews is now resolved — 37 threads closed, and several of the fixes were better than what I proposed (moving calculateCombinedAiScore behind a ComplianceScoreIssue record, and the consent split landing as a separate /api/jobs/analyze-gender-bias endpoint plus a server-side check rather than trusting the client gate). The editor spec rewrite in particular went from ten as unknown as casts to zero.

Two things are still open, both on Cathy's threads:


1. Replace JobRepository.findByIdForAiUpdate with the inherited findById

#2465 (comment)

Your element-collection explanation is correct for findComplianceIssuesByJobId and findBiasedIssuesByJobId — I've confirmed that and closed both of those threads. It does not apply to this one:

@Query("SELECT j FROM Job j WHERE j.jobId = :jobId")
Optional<Job> findByIdForAiUpdate(@Param("jobId") UUID jobId);

This returns the root entity by primary key with no entity graph. Job's @Id is UUID jobId and the repository is DocApplyJpaRepository<Job, UUID>, so it is exactly the inherited findById(UUID). The JavaDoc's "deliberately without an entity graph" is just describing findById's default.

There is one caller, JobService:604:

Job job = jobRepository.findById(jobId).orElseThrow(() -> EntityNotFoundException.forId("Job", jobId));

Deleting the method also settles Cathy's naming point in the same move — findByIdForAiUpdate encodes one caller's use case into the repository API, which is what she was flagging. Dropping it beats renaming it.

Worth a quick re-test of the analysis flow after this change, since it sits on the write path.


2. Answer Cathy's package-placement question

#2465 (comment)

This one is from May and never got a reply. It may need no code change at all — GenderBiasAnalyzer is in core/service, so the literal question is already moot and you can just say so.

The part that is worth a real answer: GenderBiasAnalysisService sits in ai/service but does purely rule-based dictionary work with no model involved. That reads odd now that the new /api/jobs/analyze-gender-bias endpoint was deliberately placed outside the ai package, and given GenderBiasWordLists was explicitly kept in core for the same reason. Either move it to core/service next to the analyzer, or explain why it belongs where it is — but the thread shouldn't just be closed silently.


Nothing else is outstanding from my side. My CHANGES_REQUESTED is still standing and I'll clear it once manual testing on the test server is done.

@ge94zec

ge94zec commented Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

Remaining items @ge94zec

Everything from my previous reviews is now resolved — 37 threads closed, and several of the fixes were better than what I proposed (moving calculateCombinedAiScore behind a ComplianceScoreIssue record, and the consent split landing as a separate /api/jobs/analyze-gender-bias endpoint plus a server-side check rather than trusting the client gate). The editor spec rewrite in particular went from ten as unknown as casts to zero.

Two things are still open, both on Cathy's threads:

1. Replace JobRepository.findByIdForAiUpdate with the inherited findById

#2465 (comment)

Your element-collection explanation is correct for findComplianceIssuesByJobId and findBiasedIssuesByJobId — I've confirmed that and closed both of those threads. It does not apply to this one:

@Query("SELECT j FROM Job j WHERE j.jobId = :jobId")
Optional<Job> findByIdForAiUpdate(@Param("jobId") UUID jobId);

This returns the root entity by primary key with no entity graph. Job's @Id is UUID jobId and the repository is DocApplyJpaRepository<Job, UUID>, so it is exactly the inherited findById(UUID). The JavaDoc's "deliberately without an entity graph" is just describing findById's default.

There is one caller, JobService:604:

Job job = jobRepository.findById(jobId).orElseThrow(() -> EntityNotFoundException.forId("Job", jobId));

Deleting the method also settles Cathy's naming point in the same move — findByIdForAiUpdate encodes one caller's use case into the repository API, which is what she was flagging. Dropping it beats renaming it.

Worth a quick re-test of the analysis flow after this change, since it sits on the write path.

2. Answer Cathy's package-placement question

#2465 (comment)

This one is from May and never got a reply. It may need no code change at all — GenderBiasAnalyzer is in core/service, so the literal question is already moot and you can just say so.

The part that is worth a real answer: GenderBiasAnalysisService sits in ai/service but does purely rule-based dictionary work with no model involved. That reads odd now that the new /api/jobs/analyze-gender-bias endpoint was deliberately placed outside the ai package, and given GenderBiasWordLists was explicitly kept in core for the same reason. Either move it to core/service next to the analyzer, or explain why it belongs where it is — but the thread shouldn't just be closed silently.

Nothing else is outstanding from my side. My CHANGES_REQUESTED is still standing and I'll clear it once manual testing on the test server is done.
GenderBiasAnalyzer and GenderBiasWordLists already live in core because they implement the reusable, deterministic dictionary matching without any AI dependencies.

I kept GenderBiasAnalysisService separate from AiService so the rule-based /api/jobs/analyze-gender-bias path remains available without AI consent and does not introduce a JobService ↔ AiService bean cycle.
I considered moving that service to core.service, but it orchestrates AI-owned analysis types and scoring (AnalyzeJobDescriptionRequestDTO, BiasedIssue, and ComplianceScoreCalculator). Moving only the service would make core depend back on the ai package. A clean move would therefore require relocating those shared analysis contracts as well, which is broader than a package-only change.
The two endpoints intentionally share this service: the jobs endpoint performs only deterministic gender analysis, while the AI endpoint uses the same result as one part of the consent-protected combined compliance analysis.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 No OpenAPI or client changes needed.

@az108

az108 commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Status update — one item left @ge94zec

Updating my earlier summary, since one of the two items is now closed out and the discussion for it sits in a resolved thread where it is easy to miss.

Closed since then

Package placement of the gender-bias analysis — answered on Cathy's thread and resolved. Short version: GenderBiasAnalyzer is already in core/service, and keeping GenderBiasAnalysisService in ai/service is justified for this PR — it avoids a JobServiceAiService cycle, and moving it alone would make core depend on ai for the first time (core currently has zero imports from ai; ai imports core in 10 files).

The underlying split is still inconsistent though — GenderCategory, GenderBiasWordLists and GenderBiasAnalyzer are in core, while BiasedIssue and the gender scoring inside ComplianceScoreCalculator are in ai, none of which involve a model. That is wider than a bugfix, so it is tracked in #2635 and assigned to you. Nothing to do here.

Still open

Replace JobRepository.findByIdForAiUpdate with the inherited findByIdthread

This is the last outstanding item on the PR. @Query("SELECT j FROM Job j WHERE j.jobId = :jobId") returning Optional<Job> with no entity graph is exactly what findById(UUID) already does on DocApplyJpaRepository<Job, UUID>. One caller, JobService:604:

Job job = jobRepository.findById(jobId).orElseThrow(() -> EntityNotFoundException.forId("Job", jobId));

That also settles Cathy's naming point, since the method disappears rather than needing a better name. Worth re-running the analysis flow afterwards as it sits on the write path.

Review state

All 37 of my threads and 4 of Cathy's 5 are resolved. My CHANGES_REQUESTED is still standing — I will clear it once manual testing on the test server is done, independently of the item above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

client Pull requests that update TypeScript code. (Added Automatically!) database job Code changes in job module. ready for review server Pull requests that update Java code. (Added Automatically!) tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix excessive API requests triggering genderDecoder

3 participants