User Onboarding#116
Conversation
…future suggestions
PR Review GuideCategory: Infrastructure + client, genai, server Review Rules
Changed domains
|
📝 WalkthroughWalkthroughThis PR introduces a student onboarding flow with three steps (program, documents, goals), CV upload and parsing via a new GenAI service, and profile schema expansion (studyProgramId, cvData, onboardingCompleted) across client, server, user-profile-service, and genai layers, plus routing/redirect wiring. ChangesOnboarding feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant APIControllerMe
participant ExternalServices
participant GenAI as genai cv.parse_cv
participant ProfileService
Client->>APIControllerMe: POST /me/cv/upload (file)
APIControllerMe->>ExternalServices: callCvParse(fileBytes)
ExternalServices->>GenAI: POST /v1/cv/parse
GenAI-->>ExternalServices: CvData
ExternalServices-->>APIControllerMe: Profile.CvData
APIControllerMe->>ProfileService: upsert(tumid, updated profile)
APIControllerMe-->>Client: CvData
sequenceDiagram
participant User
participant LoginPage
participant ProfileAPI
participant Router
participant OnboardingPage
participant Dashboard
User->>LoginPage: submit credentials
LoginPage->>ProfileAPI: getProfile()
ProfileAPI-->>LoginPage: profile.student.onboardingCompleted
alt onboardingCompleted === false
LoginPage->>Router: navigate(/onboarding)
Router->>OnboardingPage: render steps 1-3
OnboardingPage->>ProfileAPI: completeOnboarding(data)
OnboardingPage->>Router: navigate(/dashboard)
else onboardingCompleted true or unknown
LoginPage->>Router: navigate(/dashboard)
end
Router->>Dashboard: render
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (10)
services/server/src/main/java/tum/devops/http418/api/APIControllerMe.java-265-273 (1)
265-273: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winAdd a file-size/type guard before forwarding CV uploads
uploadCvreads the whole multipart into memory and forwards raw bytes without a local size or content-type check. Reject oversized/non-PDF uploads here, or stream them, to avoid unnecessary memory pressure and downstream failures.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/server/src/main/java/tum/devops/http418/api/APIControllerMe.java` around lines 265 - 273, The uploadCv handler currently forwards the entire MultipartFile to transcriptService.callCvParse without any local validation, which can cause memory pressure and bad downstream requests. Add a guard in uploadCv before reading bytes to reject oversized files and non-PDF content (using the MultipartFile metadata and/or size), and only call transcriptService.callCvParse when the file passes those checks; keep the ResponseEntity-based error handling in APIControllerMe consistent with the existing flow.services/genai/services/cv.py-1-1 (1)
1-1: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCI formatting check is failing.
Pipeline reports
ruff format --checkwould reformat this file. Runruff format services/cv.pyto fix before merge.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/genai/services/cv.py` at line 1, CI formatting is failing because this file is not compliant with Ruff’s formatter. Run the formatter on the module containing the import-only change in cv.py so the file matches ruff format output and the check passes before merge.Source: Pipeline failures
services/genai/routers/cv.py-1-1 (1)
1-1: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCI formatting check is failing.
Pipeline reports
ruff format --checkwould reformat this file. Runruff format routers/cv.pyto fix before merge.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/genai/routers/cv.py` at line 1, The CI formatting check is failing because the imports in the cv router file are not in the expected Ruff format. Update the module containing the APIRouter setup in cv.py by running Ruff formatting (or making the import layout match Ruff’s output) so the file passes ruff format --check. Keep the change limited to formatting around the existing fastapi import and any nearby symbols in the router module.Source: Pipeline failures
services/client/src/routes/register.tsx-115-115 (1)
115-115: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMissing
voidon floatingnavigate()promise.Every other
navigate(...)call in this cohort is prefixed withvoidto explicitly discard the promise. This line breaks that convention and may trip the same no-floating-promises lint rule failing elsewhere in this PR.- onSuccess: () => navigate({ to: "/onboarding" }), + onSuccess: () => void navigate({ to: "/onboarding" }),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/client/src/routes/register.tsx` at line 115, The onSuccess handler in register.tsx is calling navigate without explicitly discarding its promise, which is inconsistent with the rest of the cohort and may trigger no-floating-promises. Update the onSuccess callback in the register route to match the other navigate(...) usages by prefixing the navigate call with void, using the onSuccess handler inside the register flow as the locator.services/client/src/hooks/useOnboarding.ts-47-107 (1)
47-107: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winBiome formatting check is failing in CI for this file.
Pipeline logs show the formatter would reformat the
reducer/persistingReducersignatures and return statements.# Run locally to auto-fix npx biome format --write services/client/src/hooks/useOnboarding.ts🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/client/src/hooks/useOnboarding.ts` around lines 47 - 107, Biome formatting is out of sync in useOnboarding, especially around the reducer and persistingReducer function signatures and return statements. Reformat the file to match Biome’s style so CI passes, keeping the existing logic in reducer, persistingReducer, and useOnboarding unchanged while adjusting only whitespace and line wrapping as needed.Source: Pipeline failures
services/client/src/components/onboarding/GoalsStep.tsx-56-64 (1)
56-64: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winBiome formatter check failing in CI.
Pipeline reports useState initializer and JSX formatting differences.
npx biome format --write services/client/src/components/onboarding/GoalsStep.tsx🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/client/src/components/onboarding/GoalsStep.tsx` around lines 56 - 64, Biome formatting is failing in GoalsStep because the useState initializers and JSX style in this component do not match the formatter’s expected layout. Update the formatting in GoalsStep and its related JSX exactly as Biome would produce it, especially around the useState calls for industryPreference, rolePreference, careerGoals, interests, and preferredWorkload, so the file passes the formatter check.Source: Pipeline failures
services/client/src/components/onboarding/CvUploader.tsx-89-117 (1)
89-117: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winBiome formatter check failing in CI.
Pipeline reports JSX prop formatting (className array, style objects, conditional spans) differs from expected output.
npx biome format --write services/client/src/components/onboarding/CvUploader.tsx🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/client/src/components/onboarding/CvUploader.tsx` around lines 89 - 117, Biome formatting is out of sync in CvUploader’s JSX, specifically around the button className array, inline style objects, and conditional span rendering. Reformat the JSX in the onboarding CV uploader component to match Biome’s expected output by running the formatter or adjusting the affected return block in CvUploader so the prop and conditional element layout conforms to the project style.Source: Pipeline failures
services/client/src/components/onboarding/DocumentsStep.tsx-1-6 (1)
1-6: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winBiome import-order and formatting checks failing in CI.
npx biome check --write services/client/src/components/onboarding/DocumentsStep.tsx🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/client/src/components/onboarding/DocumentsStep.tsx` around lines 1 - 6, The import block in DocumentsStep is failing Biome import-order/format checks, so reorder and format the imports to match the project’s import sorting rules. Update the import section around DocumentsStep, keeping the same symbols (CheckCircle, CvData, TranscriptUploader, useTranscriptUpload, OnboardingStep2, CvUploader) but arranging them in the correct Biome order and formatting style so `biome check --write` passes.Source: Pipeline failures
services/client/src/components/onboarding/ProgramStep.tsx-1-8 (1)
1-8: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winBiome import-order and formatting checks are failing in CI.
Pipeline reports imports/exports are not sorted and array literal/JSX formatting differs from expected output.
npx biome check --write services/client/src/components/onboarding/ProgramStep.tsx🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/client/src/components/onboarding/ProgramStep.tsx` around lines 1 - 8, Biome is failing on import ordering and formatting in ProgramStep. Reorder the imports in ProgramStep.tsx so they match Biome’s expected sort order, and reformat the GRADUATION_OPTIONS array to the preferred multiline style used by the formatter. Use the existing ProgramStep component, useStudyPrograms, and OnboardingStep1 references to locate the affected section and then run the Biome formatter/check to normalize the file.Source: Pipeline failures
services/client/src/components/onboarding/WizardLayout.tsx-128-235 (1)
128-235: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winBiome formatter check failing in CI.
Pipeline reports boxShadow/ternary style-prop formatting differs from expected output.
npx biome format --write services/client/src/components/onboarding/WizardLayout.tsx🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/client/src/components/onboarding/WizardLayout.tsx` around lines 128 - 235, The Biome formatter output for WizardLayout is drifting from the checked-in style, specifically around the inline style object in WizardLayout and the btnDisabled ternary formatting. Re-run the formatter on the component and commit the exact Biome-generated formatting, keeping the style prop object and conditional expressions in the canonical layout expected by npx biome format so CI matches.Source: Pipeline failures
🧹 Nitpick comments (10)
services/genai/models/recommendations.py (1)
15-15: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
cv_datatyped as rawdictloses schema safety.
cv_data: dict | Noneaccepts an unvalidated blob; downstreamservices/recommendations.py(Lines 44, 47-49) reaches into it with.get("skills", []),.get("workExperience", []), and per-item.get('role', '')/.get('company', ''). Any mismatch between these hardcoded keys and the actual CV parsing output schema (defined elsewhere in the GenAI CV service) would silently degrade to empty context rather than fail loudly. Consider typing this as a proper nested Pydantic model (e.g., a sharedCvDatamodel) for compile-time/validation safety.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/genai/models/recommendations.py` at line 15, `cv_data` in the `Recommendations` model is currently a raw `dict`, which hides schema mismatches and makes downstream access in `services/recommendations.py` fragile. Replace it with a proper typed Pydantic model (for example a shared `CvData` nested model with typed fields for skills and workExperience) and update the `Recommendations` field to use that model so validation catches unexpected CV shapes instead of silently falling back to empty data.services/user-profile-service/src/main/java/tum/devops/http418/Profile.java (1)
18-20: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winType mismatch with server-side
Profile.Student.This
StudentusesList<String>forcareerGoals/interests, whileservices/server/.../dto/Profile.java'sStudentusesString[]for the same fields. See the companion comment on that file — recommend standardizing both DTOs onList<String>.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/user-profile-service/src/main/java/tum/devops/http418/Profile.java` around lines 18 - 20, The `Profile.Student` record in this module is inconsistent with the server-side DTO because `careerGoals` and `interests` are `List<String>` here but `String[]` elsewhere. Update the `Student` record to match the shared DTO shape and standardize both `Profile.Student` definitions on `List<String>` so the client and server serialize the same structure consistently.services/server/src/main/java/tum/devops/http418/api/dto/Profile.java (1)
18-20: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
careerGoals/intereststype diverges from the mirrored user-profile-service DTO.Here they're
String[], but the equivalentStudentrecord inservices/user-profile-service/.../Profile.java(same PR) usesList<String>. JSON serialization hides this at the wire level, but array components in a Javarecordbreak auto-generatedequals()/hashCode()(reference equality), and the two hand-duplicated DTOs will keep drifting without a shared contract.♻️ Suggested fix
- public record Student(String studyProgramId, int semester, String[] careerGoals, String[] interests, + public record Student(String studyProgramId, int semester, List<String> careerGoals, List<String> interests, int preferredWorkload, String expectedGraduation, String industryPreference, String rolePreference, CvData cvData, boolean onboardingCompleted) { }(Requires updating
APIControllerMe's.toArray(new String[0])merge logic and default-instance construction accordingly.)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/server/src/main/java/tum/devops/http418/api/dto/Profile.java` around lines 18 - 20, The Student DTO in Profile should match the mirrored user-profile-service contract by using List<String> for careerGoals and interests instead of String[], since the current record shape can break equals/hashCode and keep the duplicated DTOs drifting. Update the Student record in Profile, then adjust APIControllerMe so the .toArray(new String[0]) merge logic and default-instance creation use List<String> consistently.services/genai/models/cv.py (1)
4-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuecamelCase field names deviate from project's snake_case convention.
workExperience, unlikestudy_program_idused elsewhere in genai models, is camelCase — presumably to match Java'sProfile.CvDatadirectly without aliasing. Functionally fine (Pydantic handles the JSON boundary correctly either way), but inconsistent with the snake_case + alias pattern used in sibling genai models.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/genai/models/cv.py` around lines 4 - 22, The CvParseResponse model in cv.py uses camelCase field names that break the project’s snake_case convention. Rename the Python attributes to snake_case, especially work_experience, and add the appropriate Pydantic alias so the JSON/API shape still matches the Java Profile.CvData contract; keep the same pattern used by sibling genai models and preserve the existing WorkExperience and Education nested models.services/server/src/main/java/tum/devops/http418/api/dto/PatchProfileRequest.java (2)
6-13: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd input validation constraints.
None of the patch fields have validation (e.g.,
semester/preferredWorkloadcan be negative,careerGoals/interestslists are unbounded). Combined with thepatchProfilehandler shown in context (which applies these values directly with no checks), invalid data can be persisted into the profile.🛡️ Example validation additions
package tum.devops.http418.api.dto; import java.util.List; +import jakarta.validation.constraints.Min; +import jakarta.validation.constraints.Size; public record PatchProfileRequest( String studyProgramId, - Integer semester, - List<String> careerGoals, - List<String> interests, - Integer preferredWorkload, + `@Min`(0) Integer semester, + `@Size`(max = 20) List<String> careerGoals, + `@Size`(max = 20) List<String> interests, + `@Min`(0) Integer preferredWorkload, String expectedGraduation, String industryPreference, String rolePreference, Boolean onboardingCompleted) { }Requires
@Validon the controller parameter for these to take effect.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/server/src/main/java/tum/devops/http418/api/dto/PatchProfileRequest.java` around lines 6 - 13, The PatchProfileRequest DTO currently has no constraints, so invalid values can pass straight into patchProfile and be persisted. Add validation annotations to the fields in PatchProfileRequest, such as bounds for semester and preferredWorkload and size limits for careerGoals and interests, and ensure the controller parameter using patchProfile is annotated with `@Valid` so the checks are enforced.
14-14: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winClient can mark onboarding complete without any onboarding data.
Per the
patchProfilecontext snippet,onboardingCompletedis applied unconditionally whenever non-null, with no check thatstudyProgramId/cvData/goals are actually populated. A client can call PATCH with just{"onboardingCompleted": true}and skip the wizard entirely, leaving downstream recommendation features (which now depend onstudyProgramId/cvData) with empty inputs.Consider computing
onboardingCompletedserver-side based on required-field completeness rather than trusting the client flag directly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/server/src/main/java/tum/devops/http418/api/dto/PatchProfileRequest.java` at line 14, The PatchProfileRequest onboarding completion flag is being trusted directly, allowing clients to skip onboarding without providing the required profile data. Update the patchProfile flow and the PatchProfileRequest handling so that onboardingCompleted is derived server-side from the completeness of studyProgramId, cvData, and goals instead of applying the client-provided boolean unconditionally. Use the patchProfile logic and the onboardingCompleted field in PatchProfileRequest as the main touchpoints when implementing the validation/computation.services/genai/services/cv.py (1)
31-35: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider tolerating markdown-fenced JSON from the LLM.
The prompt asks for "no markdown code blocks", but LLMs frequently wrap JSON in
```jsonfences anyway. Today that would raisejson.JSONDecodeError→ 502, an avoidable failure for otherwise-valid output. Stripping common fences beforejson.loadswould reduce spurious failures.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/genai/services/cv.py` around lines 31 - 35, The CV parsing path in the exception handling around llm.ainvoke and json.loads currently rejects otherwise-valid JSON if the model wraps it in markdown fences. Update the parsing logic in cv.py to normalize result.content by stripping common ```json / ``` fences before calling json.loads, while keeping the existing logging and HTTPException fallback for truly malformed output.services/client/src/routes/login.tsx (1)
116-128: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated onboarding-status check/cast shared with
dashboard.tsx.The
raw?.student?.onboardingCompleted === falsecheck (plus the same unsafe cast) is duplicated here and indashboard.tsx. Consider extracting a small shared helper, e.g.getPostLoginDestination(profile), to keep this logic in one place.♻️ Suggested extraction
// `#/api/onboarding.ts` import type { StudentProfile } from "`#/api/types`"; export function postLoginDestination(profile: StudentProfile | undefined): "/onboarding" | "/dashboard" { return profile?.student?.onboardingCompleted === false ? "/onboarding" : "/dashboard"; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/client/src/routes/login.tsx` around lines 116 - 128, The post-login navigation logic is duplicated between login.tsx and dashboard.tsx, including the unsafe profile cast and onboarding check. Extract the shared decision into a helper such as getPostLoginDestination(profile) or postLoginDestination(profile) in a common API/util module, and have both onSuccess in login.tsx and the corresponding logic in dashboard.tsx call that helper instead of repeating the raw?.student?.onboardingCompleted === false check and cast. Keep the helper typed against the shared profile type so the navigation destination remains centralized and consistent.services/client/src/routes/_authenticated/dashboard.tsx (1)
160-171: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueRemove the redundant profile cast
useProfile()already returnsStudentProfile, soprofile.student?.onboardingCompletedcan be accessed directly here. Drop the same cast inlogin.tsxtoo.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/client/src/routes/_authenticated/dashboard.tsx` around lines 160 - 171, The dashboard effect is using a redundant type cast on profile even though useProfile already returns StudentProfile. Update the useEffect in dashboard.tsx to access profile.student?.onboardingCompleted directly after the null check, and apply the same cleanup in login.tsx where the same cast is used. Keep the navigation logic in the useEffect and the useProfile hook reference unchanged.services/client/src/components/onboarding/ProgramStep.tsx (1)
5-8: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded graduation term list will need periodic manual updates.
Consider generating the term list dynamically (e.g., current + next N semesters) instead of a fixed array that will go stale.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/client/src/components/onboarding/ProgramStep.tsx` around lines 5 - 8, The fixed GRADUATION_OPTIONS array in ProgramStep will go stale and requires manual updates. Replace the hardcoded list with logic that generates the graduation terms dynamically, using the existing ProgramStep component (and GRADUATION_OPTIONS usage) to derive the current and next N semesters at runtime so the options stay current without code changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@services/client/src/api/types.ts`:
- Around line 496-513: Remove the duplicated onboarding/profile fields from
StudentProfile so there is a single source of truth. In
services/client/src/api/types.ts, keep the nested student object as the
canonical shape and delete the flat expectedGraduation, industryPreference,
rolePreference, cvData, and onboardingCompleted properties if they are not used
elsewhere. Verify dashboard.tsx and login.tsx still compile against
StudentProfile.student and update any type references or mappings that assumed
the top-level copies.
In `@services/client/src/components/onboarding/StepIndicator.tsx`:
- Around line 20-47: The StepIndicator component’s inline style object
formatting is failing Biome, specifically around the nested style props and the
boxShadow conditional in StepIndicator. Reformat the JSX style block in the
StepIndicator render so it matches the formatter’s expected layout, then run the
project’s Biome format command to confirm the formatter passes cleanly.
In `@services/client/src/hooks/useOnboarding.ts`:
- Around line 11-15: The persisted onboarding snapshot is currently storing
sensitive parsed résumé data in sessionStorage via useOnboarding’s saveState
flow. Update the OnboardingStep2 shape and the save/restore logic so cvData is
not serialized at all, and only non-sensitive fields like transcriptUploaded and
cvUploaded are persisted. Ensure the loadState path in useOnboarding can
tolerate missing cvData and falls back cleanly after reload.
In `@services/client/src/routes/onboarding.tsx`:
- Around line 129-219: This route is duplicating the onboarding UI instead of
using the shared components, so replace the local StepIndicatorInline and
WizardShell implementations with imports from the existing StepIndicator and
WizardLayout components under components/onboarding. Update the onboarding page
to render those shared symbols directly, keep the currentStep/onBack props wired
through, and remove the duplicate local definitions so there is a single source
of truth.
- Around line 1-11: Biome is failing on this route file due to unsorted imports
and formatting issues, so run the project’s Biome format/organize-imports fix on
the onboarding route. Make sure the import block in onboarding.tsx is reordered
consistently, and reformat any affected JSX, arrow-function props, and style
object literals in the route component so the file matches Biome’s output.
- Around line 24-30: The finishMutation in onboarding.tsx only handles success,
so failures from completeOnboarding are silent and leave the user stuck on the
Goals step. Update the useMutation setup for finishMutation to include error
handling in its callbacks, and surface a clear failure message or UI state when
mutationFn fails so the user knows onboarding did not complete. Use the existing
finishMutation, completeOnboarding, and onSuccess flow as the place to add the
missing failure path.
- Around line 13-18: The onboarding route only checks authentication, so
completed users can reopen the wizard, and completeOnboarding() does not refresh
the cached profile state. Add an onboarding-completed guard in Route.beforeLoad
for OnboardingPage to redirect users who already finished onboarding, and update
or invalidate the ["profile"] query inside completeOnboarding() before
navigating to /dashboard so useProfile() sees the latest onboardingCompleted
value.
In `@services/genai/requirements.txt`:
- Line 9: Update the pinned pypdf dependency in requirements.txt from the
current 5.4.0 release to a current 6.x version. This affects the PDF
upload/parsing path that uses PdfReader, so make sure the version bump is
applied in the dependency declaration and keep any downstream compatibility in
mind when validating the genai service.
In `@services/genai/routers/cv.py`:
- Around line 9-14: The cv_parse handler currently reads the entire request body
with request.body() before any limit is enforced, so add an explicit maximum
upload size check in this endpoint before buffering the payload. Use cv_parse
and parse_cv as the key locations, and reject oversized PDFs with an
HTTPException (or equivalent) as soon as the body size exceeds the configured
threshold so memory cannot be exhausted before parse_cv runs.
In `@services/genai/services/cv.py`:
- Around line 28-40: The cv parsing flow in cv.py currently returns
json.loads(result.content) without checking it matches the CvData shape, so
malformed LLM output can slip through. Update the cv_parse path around get_llm()
/ llm.ainvoke() to validate the parsed payload against a strict schema or
Pydantic model that mirrors the expected CvData contract, including required
fields and allowed top-level keys. If validation fails, log a clear cv_parse
error and raise the existing 502 response from this handler instead of returning
invalid data.
- Around line 17-19: The synchronous PDF parsing in parse_cv is blocking the
event loop; move the PdfReader and page.extract_text() work off the async path
by running the extraction in a background thread or executor, then await the
result back in parse_cv. Keep the async method responsive by isolating the
CPU-bound work into a helper around PdfReader and the text-join loop, and only
return the extracted text once that helper completes.
- Around line 29-32: The CV generation path can hang because the LLM invocation
has no timeout protection. Update the code around get_llm() and
llm.ainvoke(prompt) in the CV service to enforce an explicit timeout, either by
wrapping the await in asyncio.wait_for(...) or by configuring a client timeout
in the provider setup that creates ChatOpenAI/ChatOllama. Keep the change
localized so the parsing flow in the CV function still handles the result and
json.loads(result.content) after the call completes or times out.
In `@services/server/src/main/java/tum/devops/http418/api/APIControllerMe.java`:
- Around line 233-296: The read-merge-upsert flow in patchProfile and uploadCv
can overwrite concurrent profile changes because both methods fetch a Profile,
build a new Profile in memory, and then upsert it without any concurrency check.
Fix this by adding optimistic concurrency to Profile/upsert in APIControllerMe,
using a version or ETag so conflicting updates are rejected, or by moving the
merge logic into PROFILE_SERVICE so the update happens atomically server-side.
Ensure the symbols patchProfile, uploadCv, and the PROFILE_SERVICE /upsert call
are updated together.
- Around line 260-262: The PROFILE_SERVICE upsert call in patchProfile and
uploadCv currently discards the result of retrieve().toEntity(String.class) and
has no error handling, so failures escape as an opaque 500. Update the code
around the restClient.post().uri(..."/upsert/" + tumid) call in both endpoints
to handle RestClientResponseException the same way getProfile does, mapping the
downstream status into a controlled ResponseEntity response instead of letting
the exception propagate. Keep the newProfile flow unchanged on success, but
ensure the upsert response is not ignored and any profile-service error is
caught and translated consistently.
---
Minor comments:
In `@services/client/src/components/onboarding/CvUploader.tsx`:
- Around line 89-117: Biome formatting is out of sync in CvUploader’s JSX,
specifically around the button className array, inline style objects, and
conditional span rendering. Reformat the JSX in the onboarding CV uploader
component to match Biome’s expected output by running the formatter or adjusting
the affected return block in CvUploader so the prop and conditional element
layout conforms to the project style.
In `@services/client/src/components/onboarding/DocumentsStep.tsx`:
- Around line 1-6: The import block in DocumentsStep is failing Biome
import-order/format checks, so reorder and format the imports to match the
project’s import sorting rules. Update the import section around DocumentsStep,
keeping the same symbols (CheckCircle, CvData, TranscriptUploader,
useTranscriptUpload, OnboardingStep2, CvUploader) but arranging them in the
correct Biome order and formatting style so `biome check --write` passes.
In `@services/client/src/components/onboarding/GoalsStep.tsx`:
- Around line 56-64: Biome formatting is failing in GoalsStep because the
useState initializers and JSX style in this component do not match the
formatter’s expected layout. Update the formatting in GoalsStep and its related
JSX exactly as Biome would produce it, especially around the useState calls for
industryPreference, rolePreference, careerGoals, interests, and
preferredWorkload, so the file passes the formatter check.
In `@services/client/src/components/onboarding/ProgramStep.tsx`:
- Around line 1-8: Biome is failing on import ordering and formatting in
ProgramStep. Reorder the imports in ProgramStep.tsx so they match Biome’s
expected sort order, and reformat the GRADUATION_OPTIONS array to the preferred
multiline style used by the formatter. Use the existing ProgramStep component,
useStudyPrograms, and OnboardingStep1 references to locate the affected section
and then run the Biome formatter/check to normalize the file.
In `@services/client/src/components/onboarding/WizardLayout.tsx`:
- Around line 128-235: The Biome formatter output for WizardLayout is drifting
from the checked-in style, specifically around the inline style object in
WizardLayout and the btnDisabled ternary formatting. Re-run the formatter on the
component and commit the exact Biome-generated formatting, keeping the style
prop object and conditional expressions in the canonical layout expected by npx
biome format so CI matches.
In `@services/client/src/hooks/useOnboarding.ts`:
- Around line 47-107: Biome formatting is out of sync in useOnboarding,
especially around the reducer and persistingReducer function signatures and
return statements. Reformat the file to match Biome’s style so CI passes,
keeping the existing logic in reducer, persistingReducer, and useOnboarding
unchanged while adjusting only whitespace and line wrapping as needed.
In `@services/client/src/routes/register.tsx`:
- Line 115: The onSuccess handler in register.tsx is calling navigate without
explicitly discarding its promise, which is inconsistent with the rest of the
cohort and may trigger no-floating-promises. Update the onSuccess callback in
the register route to match the other navigate(...) usages by prefixing the
navigate call with void, using the onSuccess handler inside the register flow as
the locator.
In `@services/genai/routers/cv.py`:
- Line 1: The CI formatting check is failing because the imports in the cv
router file are not in the expected Ruff format. Update the module containing
the APIRouter setup in cv.py by running Ruff formatting (or making the import
layout match Ruff’s output) so the file passes ruff format --check. Keep the
change limited to formatting around the existing fastapi import and any nearby
symbols in the router module.
In `@services/genai/services/cv.py`:
- Line 1: CI formatting is failing because this file is not compliant with
Ruff’s formatter. Run the formatter on the module containing the import-only
change in cv.py so the file matches ruff format output and the check passes
before merge.
In `@services/server/src/main/java/tum/devops/http418/api/APIControllerMe.java`:
- Around line 265-273: The uploadCv handler currently forwards the entire
MultipartFile to transcriptService.callCvParse without any local validation,
which can cause memory pressure and bad downstream requests. Add a guard in
uploadCv before reading bytes to reject oversized files and non-PDF content
(using the MultipartFile metadata and/or size), and only call
transcriptService.callCvParse when the file passes those checks; keep the
ResponseEntity-based error handling in APIControllerMe consistent with the
existing flow.
---
Nitpick comments:
In `@services/client/src/components/onboarding/ProgramStep.tsx`:
- Around line 5-8: The fixed GRADUATION_OPTIONS array in ProgramStep will go
stale and requires manual updates. Replace the hardcoded list with logic that
generates the graduation terms dynamically, using the existing ProgramStep
component (and GRADUATION_OPTIONS usage) to derive the current and next N
semesters at runtime so the options stay current without code changes.
In `@services/client/src/routes/_authenticated/dashboard.tsx`:
- Around line 160-171: The dashboard effect is using a redundant type cast on
profile even though useProfile already returns StudentProfile. Update the
useEffect in dashboard.tsx to access profile.student?.onboardingCompleted
directly after the null check, and apply the same cleanup in login.tsx where the
same cast is used. Keep the navigation logic in the useEffect and the useProfile
hook reference unchanged.
In `@services/client/src/routes/login.tsx`:
- Around line 116-128: The post-login navigation logic is duplicated between
login.tsx and dashboard.tsx, including the unsafe profile cast and onboarding
check. Extract the shared decision into a helper such as
getPostLoginDestination(profile) or postLoginDestination(profile) in a common
API/util module, and have both onSuccess in login.tsx and the corresponding
logic in dashboard.tsx call that helper instead of repeating the
raw?.student?.onboardingCompleted === false check and cast. Keep the helper
typed against the shared profile type so the navigation destination remains
centralized and consistent.
In `@services/genai/models/cv.py`:
- Around line 4-22: The CvParseResponse model in cv.py uses camelCase field
names that break the project’s snake_case convention. Rename the Python
attributes to snake_case, especially work_experience, and add the appropriate
Pydantic alias so the JSON/API shape still matches the Java Profile.CvData
contract; keep the same pattern used by sibling genai models and preserve the
existing WorkExperience and Education nested models.
In `@services/genai/models/recommendations.py`:
- Line 15: `cv_data` in the `Recommendations` model is currently a raw `dict`,
which hides schema mismatches and makes downstream access in
`services/recommendations.py` fragile. Replace it with a proper typed Pydantic
model (for example a shared `CvData` nested model with typed fields for skills
and workExperience) and update the `Recommendations` field to use that model so
validation catches unexpected CV shapes instead of silently falling back to
empty data.
In `@services/genai/services/cv.py`:
- Around line 31-35: The CV parsing path in the exception handling around
llm.ainvoke and json.loads currently rejects otherwise-valid JSON if the model
wraps it in markdown fences. Update the parsing logic in cv.py to normalize
result.content by stripping common ```json / ``` fences before calling
json.loads, while keeping the existing logging and HTTPException fallback for
truly malformed output.
In
`@services/server/src/main/java/tum/devops/http418/api/dto/PatchProfileRequest.java`:
- Around line 6-13: The PatchProfileRequest DTO currently has no constraints, so
invalid values can pass straight into patchProfile and be persisted. Add
validation annotations to the fields in PatchProfileRequest, such as bounds for
semester and preferredWorkload and size limits for careerGoals and interests,
and ensure the controller parameter using patchProfile is annotated with `@Valid`
so the checks are enforced.
- Line 14: The PatchProfileRequest onboarding completion flag is being trusted
directly, allowing clients to skip onboarding without providing the required
profile data. Update the patchProfile flow and the PatchProfileRequest handling
so that onboardingCompleted is derived server-side from the completeness of
studyProgramId, cvData, and goals instead of applying the client-provided
boolean unconditionally. Use the patchProfile logic and the onboardingCompleted
field in PatchProfileRequest as the main touchpoints when implementing the
validation/computation.
In `@services/server/src/main/java/tum/devops/http418/api/dto/Profile.java`:
- Around line 18-20: The Student DTO in Profile should match the mirrored
user-profile-service contract by using List<String> for careerGoals and
interests instead of String[], since the current record shape can break
equals/hashCode and keep the duplicated DTOs drifting. Update the Student record
in Profile, then adjust APIControllerMe so the .toArray(new String[0]) merge
logic and default-instance creation use List<String> consistently.
In `@services/user-profile-service/src/main/java/tum/devops/http418/Profile.java`:
- Around line 18-20: The `Profile.Student` record in this module is inconsistent
with the server-side DTO because `careerGoals` and `interests` are
`List<String>` here but `String[]` elsewhere. Update the `Student` record to
match the shared DTO shape and standardize both `Profile.Student` definitions on
`List<String>` so the client and server serialize the same structure
consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c8a7cad8-6db8-48ce-b6ed-758eb2e7e1a8
📒 Files selected for processing (33)
services/client/src/api/profile.tsservices/client/src/api/types.tsservices/client/src/components/onboarding/CvUploader.tsxservices/client/src/components/onboarding/DocumentsStep.tsxservices/client/src/components/onboarding/GoalsStep.tsxservices/client/src/components/onboarding/ProgramStep.tsxservices/client/src/components/onboarding/StepIndicator.tsxservices/client/src/components/onboarding/WizardLayout.tsxservices/client/src/hooks/useCvUpload.tsservices/client/src/hooks/useOnboarding.tsservices/client/src/routeTree.gen.tsservices/client/src/routes/_authenticated/dashboard.tsxservices/client/src/routes/login.tsxservices/client/src/routes/onboarding.tsxservices/client/src/routes/register.tsxservices/genai/main.pyservices/genai/models/advisor.pyservices/genai/models/cv.pyservices/genai/models/recommendations.pyservices/genai/models/roadmap.pyservices/genai/prompts/cv_parse.txtservices/genai/prompts/recommendations.txtservices/genai/requirements.txtservices/genai/routers/cv.pyservices/genai/services/advisor.pyservices/genai/services/cv.pyservices/genai/services/recommendations.pyservices/genai/services/roadmap.pyservices/server/src/main/java/tum/devops/http418/api/APIControllerMe.javaservices/server/src/main/java/tum/devops/http418/api/ExternalServices.javaservices/server/src/main/java/tum/devops/http418/api/dto/PatchProfileRequest.javaservices/server/src/main/java/tum/devops/http418/api/dto/Profile.javaservices/user-profile-service/src/main/java/tum/devops/http418/Profile.java
| expectedGraduation?: string; | ||
| industryPreference?: string; | ||
| rolePreference?: string; | ||
| cvData?: CvData; | ||
| onboardingCompleted?: boolean; | ||
| student?: { | ||
| studyProgramId?: string; | ||
| semester?: number; | ||
| careerGoals?: string[]; | ||
| interests?: string[]; | ||
| preferredWorkload?: number; | ||
| expectedGraduation?: string; | ||
| industryPreference?: string; | ||
| rolePreference?: string; | ||
| cvData?: CvData; | ||
| onboardingCompleted?: boolean; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Duplicate onboarding fields at both top level and nested student.
expectedGraduation, industryPreference, rolePreference, cvData, onboardingCompleted exist twice on StudentProfile — flat and inside student. Every consumer in this cohort (dashboard.tsx, login.tsx) only reads the nested student.* copy, so the flat top-level fields look unused and create an ambiguous source of truth.
♻️ Suggested cleanup
export type StudentProfile = {
...
createdAt: IsoDateString;
updatedAt: IsoDateString;
- expectedGraduation?: string;
- industryPreference?: string;
- rolePreference?: string;
- cvData?: CvData;
- onboardingCompleted?: boolean;
student?: {
studyProgramId?: string;
semester?: number;
careerGoals?: string[];
interests?: string[];
preferredWorkload?: number;
expectedGraduation?: string;
industryPreference?: string;
rolePreference?: string;
cvData?: CvData;
onboardingCompleted?: boolean;
};
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| expectedGraduation?: string; | |
| industryPreference?: string; | |
| rolePreference?: string; | |
| cvData?: CvData; | |
| onboardingCompleted?: boolean; | |
| student?: { | |
| studyProgramId?: string; | |
| semester?: number; | |
| careerGoals?: string[]; | |
| interests?: string[]; | |
| preferredWorkload?: number; | |
| expectedGraduation?: string; | |
| industryPreference?: string; | |
| rolePreference?: string; | |
| cvData?: CvData; | |
| onboardingCompleted?: boolean; | |
| }; | |
| }; | |
| expectedGraduation?: string; | |
| industryPreference?: string; | |
| rolePreference?: string; | |
| cvData?: CvData; | |
| onboardingCompleted?: boolean; | |
| student?: { | |
| studyProgramId?: string; | |
| semester?: number; | |
| careerGoals?: string[]; | |
| interests?: string[]; | |
| preferredWorkload?: number; | |
| expectedGraduation?: string; | |
| industryPreference?: string; | |
| rolePreference?: string; | |
| cvData?: CvData; | |
| onboardingCompleted?: boolean; | |
| }; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/client/src/api/types.ts` around lines 496 - 513, Remove the
duplicated onboarding/profile fields from StudentProfile so there is a single
source of truth. In services/client/src/api/types.ts, keep the nested student
object as the canonical shape and delete the flat expectedGraduation,
industryPreference, rolePreference, cvData, and onboardingCompleted properties
if they are not used elsewhere. Verify dashboard.tsx and login.tsx still compile
against StudentProfile.student and update any type references or mappings that
assumed the top-level copies.
| <div style={{ display: "flex", flexDirection: "column", alignItems: "center" }}> | ||
| <div | ||
| style={{ | ||
| width: 32, | ||
| height: 32, | ||
| borderRadius: "50%", | ||
| display: "flex", | ||
| alignItems: "center", | ||
| justifyContent: "center", | ||
| fontSize: 13, | ||
| fontWeight: 700, | ||
| background: done | ||
| ? "linear-gradient(135deg, #8A57E0, #2D6FB5)" | ||
| : active | ||
| ? "linear-gradient(135deg, #8A57E0, #2D6FB5)" | ||
| : "#E2E7EF", | ||
| color: done || active ? "#fff" : "#6E7E94", | ||
| boxShadow: active ? "0 2px 8px rgba(138,87,224,0.35)" : "none", | ||
| transition: "all 0.2s", | ||
| }} | ||
| > | ||
| {done ? "✓" : stepNum} | ||
| </div> | ||
| <span | ||
| style={{ | ||
| marginTop: 6, | ||
| fontSize: 11, | ||
| fontWeight: active ? 600 : 400, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Biome formatter failure on style props.
Pipeline reports the formatter would reformat this nested style object and the boxShadow ternary. Run biome format --write (or the project's format command) to resolve before merge.
🧰 Tools
🪛 GitHub Actions: Lint & Format / 3_lint-client.txt
[error] 20-47: Biome formatter failed. Formatter would have reformatted JSX style prop object for a nested div and wrapped a conditional ternary expression (boxShadow).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/client/src/components/onboarding/StepIndicator.tsx` around lines 20
- 47, The StepIndicator component’s inline style object formatting is failing
Biome, specifically around the nested style props and the boxShadow conditional
in StepIndicator. Reformat the JSX style block in the StepIndicator render so it
matches the formatter’s expected layout, then run the project’s Biome format
command to confirm the formatter passes cleanly.
Source: Pipeline failures
| export type OnboardingStep2 = { | ||
| transcriptUploaded: boolean; | ||
| cvUploaded: boolean; | ||
| cvData: CvData | null; | ||
| }; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Parsed CV data (PII) persisted unencrypted to sessionStorage.
OnboardingStep2.cvData (containing parsed résumé fields such as name, contact info, work/education history) is serialized into sessionStorage as plaintext via saveState. This is readable by any script on the page (XSS exposure) and browser devtools for the session's duration.
Consider excluding cvData from the persisted snapshot (re-fetch/re-upload if the user reloads) or storing only a non-sensitive flag (e.g., cvUploaded) in sessionStorage.
Also applies to: 87-97
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/client/src/hooks/useOnboarding.ts` around lines 11 - 15, The
persisted onboarding snapshot is currently storing sensitive parsed résumé data
in sessionStorage via useOnboarding’s saveState flow. Update the OnboardingStep2
shape and the save/restore logic so cvData is not serialized at all, and only
non-sensitive fields like transcriptUploaded and cvUploaded are persisted.
Ensure the loadState path in useOnboarding can tolerate missing cvData and falls
back cleanly after reload.
| import { useMutation } from "@tanstack/react-query"; | ||
| import { createFileRoute, redirect, useNavigate } from "@tanstack/react-router"; | ||
| import type { ReactNode } from "react"; | ||
| import { completeOnboarding } from "#/api/profile"; | ||
| import type { StudentProfileUpdate } from "#/api/types"; | ||
| import { DocumentsStep } from "#/components/onboarding/DocumentsStep"; | ||
| import { GoalsStep } from "#/components/onboarding/GoalsStep"; | ||
| import { ProgramStep } from "#/components/onboarding/ProgramStep"; | ||
| import type { OnboardingStep3 } from "#/hooks/useOnboarding"; | ||
| import { useOnboarding } from "#/hooks/useOnboarding"; | ||
| import { isAuthenticated } from "#/api"; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Biome lint/format failures block CI.
Pipeline reports unsorted imports and multiple formatting violations (arrow function props, JSX/style objects) across this file. Run the project's Biome format + organize-imports fix before merge.
Also applies to: 25-219
🧰 Tools
🪛 GitHub Actions: Lint & Format / 3_lint-client.txt
[error] 1-13: Biome import organization failed (assist/source/organizeImports). Imports/exports are not sorted. Safe fix: 'Organize Imports (Biome)'.
🪛 GitHub Actions: Lint & Format / lint-client
[error] 1-1: Biome organizeImports error: imports/exports are not sorted. Tool: assist/source/organizeImports.
[error] 1-1: Biome formatter error: formatting differs from expected output (e.g., multiline arrow function props, JSX element formatting, style object formatting).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/client/src/routes/onboarding.tsx` around lines 1 - 11, Biome is
failing on this route file due to unsorted imports and formatting issues, so run
the project’s Biome format/organize-imports fix on the onboarding route. Make
sure the import block in onboarding.tsx is reordered consistently, and reformat
any affected JSX, arrow-function props, and style object literals in the route
component so the file matches Biome’s output.
Source: Pipeline failures
| export const Route = createFileRoute("/onboarding")({ | ||
| beforeLoad: () => { | ||
| if (!isAuthenticated()) throw redirect({ to: "/login" }); | ||
| }, | ||
| component: OnboardingPage, | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect useProfile implementation for caching behavior
fd -a useProfile.ts services/client/src | xargs cat -nRepository: AET-DevOps26/team-http-418
Length of output: 456
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== onboarding route ==\n'
fd -a onboarding.tsx services/client/src | head -n 20
printf '\n== dashboard route ==\n'
fd -a dashboard.tsx services/client/src | head -n 20
printf '\n== profile hooks and api ==\n'
fd -a profile.ts services/client/src | head -n 20
fd -a useProfile.ts services/client/src | head -n 20Repository: AET-DevOps26/team-http-418
Length of output: 686
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in \
services/client/src/routes/onboarding.tsx \
services/client/src/routes/_authenticated/dashboard.tsx \
services/client/src/hooks/useUpdateProfile.ts \
services/client/src/api/profile.ts
do
echo "== $f =="
wc -l "$f"
cat -n "$f"
echo
doneRepository: AET-DevOps26/team-http-418
Length of output: 20207
Redirect completed users and refresh the profile cache
/onboarding only blocks unauthenticated users, so a completed user can reopen the wizard and patch their profile again. completeOnboarding() also navigates to /dashboard without updating ["profile"]; with useProfile() cached for 120s, the dashboard can still see onboardingCompleted === false and send the user back to onboarding. Add an onboarding-completed guard here and invalidate/update the profile query on success.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/client/src/routes/onboarding.tsx` around lines 13 - 18, The
onboarding route only checks authentication, so completed users can reopen the
wizard, and completeOnboarding() does not refresh the cached profile state. Add
an onboarding-completed guard in Route.beforeLoad for OnboardingPage to redirect
users who already finished onboarding, and update or invalidate the ["profile"]
query inside completeOnboarding() before navigating to /dashboard so
useProfile() sees the latest onboardingCompleted value.
| from pypdf import PdfReader | ||
| reader = PdfReader(io.BytesIO(pdf_bytes)) | ||
| text = "\n".join(page.extract_text() or "" for page in reader.pages) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Blocking PDF extraction inside an async function.
PdfReader(...) and extract_text() are synchronous/CPU-bound. Running them directly inside async def parse_cv blocks the event loop for the duration of extraction, stalling all other concurrent requests on this worker — especially costly for large, multi-page PDFs.
♻️ Suggested fix
+import asyncio
+
async def parse_cv(pdf_bytes: bytes) -> dict:
try:
from pypdf import PdfReader
- reader = PdfReader(io.BytesIO(pdf_bytes))
- text = "\n".join(page.extract_text() or "" for page in reader.pages)
+ def _extract():
+ reader = PdfReader(io.BytesIO(pdf_bytes))
+ return "\n".join(page.extract_text() or "" for page in reader.pages)
+ text = await asyncio.to_thread(_extract)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from pypdf import PdfReader | |
| reader = PdfReader(io.BytesIO(pdf_bytes)) | |
| text = "\n".join(page.extract_text() or "" for page in reader.pages) | |
| import asyncio | |
| from pypdf import PdfReader | |
| def _extract(): | |
| reader = PdfReader(io.BytesIO(pdf_bytes)) | |
| return "\n".join(page.extract_text() or "" for page in reader.pages) | |
| text = await asyncio.to_thread(_extract) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/genai/services/cv.py` around lines 17 - 19, The synchronous PDF
parsing in parse_cv is blocking the event loop; move the PdfReader and
page.extract_text() work off the async path by running the extraction in a
background thread or executor, then await the result back in parse_cv. Keep the
async method responsive by isolating the CPU-bound work into a helper around
PdfReader and the text-join loop, and only return the extracted text once that
helper completes.
| prompt = _CV_PARSE_PROMPT.format(cv_text=text[:8000]) | ||
| try: | ||
| llm = get_llm() | ||
| result = await llm.ainvoke(prompt) | ||
| parsed = json.loads(result.content) | ||
| except (json.JSONDecodeError, ValueError) as e: | ||
| logger.error("cv_parse | LLM response invalid: %s", e) | ||
| raise HTTPException(status_code=502, detail="LLM returned malformed CV parse response") | ||
| except Exception as e: | ||
| logger.error("cv_parse | LLM call failed: %s", e) | ||
| raise HTTPException(status_code=502, detail=f"LLM service unavailable: {e}") | ||
|
|
||
| return parsed |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Parsed LLM output isn't validated against the CvData contract before returning.
parsed is whatever JSON the LLM returns after json.loads, returned as-is. The server deserializes this response directly into Profile.CvData.class (per ExternalServices.callCvParse). If the LLM omits a field, uses the wrong type (e.g., a string instead of an array), or adds extra top-level keys, json.loads still succeeds here (so no 502 is raised), but the mismatch will only surface as an opaque deserialization failure on the Java side.
Validate the shape before returning — e.g., with a Pydantic model mirroring workExperience/skills/languages/education — so malformed LLM output is caught and reported here with a clear error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/genai/services/cv.py` around lines 28 - 40, The cv parsing flow in
cv.py currently returns json.loads(result.content) without checking it matches
the CvData shape, so malformed LLM output can slip through. Update the cv_parse
path around get_llm() / llm.ainvoke() to validate the parsed payload against a
strict schema or Pydantic model that mirrors the expected CvData contract,
including required fields and allowed top-level keys. If validation fails, log a
clear cv_parse error and raise the existing 502 response from this handler
instead of returning invalid data.
| try: | ||
| llm = get_llm() | ||
| result = await llm.ainvoke(prompt) | ||
| parsed = json.loads(result.content) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== file list ==\n'
git ls-files 'services/genai/services/*' 'services/genai/*' | sed -n '1,200p'
printf '\n== locate get_llm and timeout-related symbols ==\n'
rg -n "def get_llm|class .*LLM|get_llm\(|timeout|wait_for|client_timeout|request_timeout|read_timeout|connect_timeout" services/genai -S
printf '\n== cv.py excerpt ==\n'
sed -n '1,120p' services/genai/services/cv.pyRepository: AET-DevOps26/team-http-418
Length of output: 4800
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== provider.py ==\n'
sed -n '1,220p' services/genai/llm/provider.py
printf '\n== timeout handling in services ==\n'
rg -n "timeout|wait_for|Timeout|asyncio\.wait_for|raise .*timeout" services/genai/services services/genai/llm -SRepository: AET-DevOps26/team-http-418
Length of output: 3323
Add a timeout around the CV LLM call. services/genai/llm/provider.py builds ChatOpenAI/ChatOllama without any timeout settings, so await llm.ainvoke(prompt) can still hang indefinitely. Wrap this call in asyncio.wait_for(...) or pass an explicit client timeout.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/genai/services/cv.py` around lines 29 - 32, The CV generation path
can hang because the LLM invocation has no timeout protection. Update the code
around get_llm() and llm.ainvoke(prompt) in the CV service to enforce an
explicit timeout, either by wrapping the await in asyncio.wait_for(...) or by
configuring a client timeout in the provider setup that creates
ChatOpenAI/ChatOllama. Keep the change localized so the parsing flow in the CV
function still handles the result and json.loads(result.content) after the call
completes or times out.
| @PatchMapping("") | ||
| public ResponseEntity<Profile> patchProfile(@RequestBody PatchProfileRequest patch, | ||
| @AuthenticationPrincipal String tumid) { | ||
| final Profile current = transcriptService.fetchProfile(tumid); | ||
| final Profile.Student s = (current != null && current.student() != null) | ||
| ? current.student() | ||
| : new Profile.Student(null, 0, new String[0], new String[0], 0, null, null, null, null, false); | ||
| final Profile.Student updated = new Profile.Student( | ||
| patch.studyProgramId() != null ? patch.studyProgramId() : s.studyProgramId(), | ||
| patch.semester() != null ? patch.semester() : s.semester(), | ||
| patch.careerGoals() != null ? patch.careerGoals().toArray(new String[0]) | ||
| : (s.careerGoals() != null ? s.careerGoals() : new String[0]), | ||
| patch.interests() != null ? patch.interests().toArray(new String[0]) | ||
| : (s.interests() != null ? s.interests() : new String[0]), | ||
| patch.preferredWorkload() != null ? patch.preferredWorkload() : s.preferredWorkload(), | ||
| patch.expectedGraduation() != null ? patch.expectedGraduation() : s.expectedGraduation(), | ||
| patch.industryPreference() != null ? patch.industryPreference() : s.industryPreference(), | ||
| patch.rolePreference() != null ? patch.rolePreference() : s.rolePreference(), | ||
| s.cvData(), | ||
| patch.onboardingCompleted() != null ? patch.onboardingCompleted() : s.onboardingCompleted()); | ||
| final Profile newProfile = new Profile(updated, | ||
| current != null ? current.completedCourses() : List.of(), | ||
| current != null ? current.enrolledCourses() : List.of(), | ||
| current != null ? current.availableCourses() : List.of(), | ||
| current != null ? current.limit() : 0, | ||
| current != null ? current.category() : null, | ||
| current != null ? current.semesterKey() : null); | ||
| restClient.post().uri(PROFILE_SERVICE + "/upsert/" + tumid) | ||
| .contentType(MediaType.APPLICATION_JSON).body(newProfile).retrieve().toEntity(String.class); | ||
| return ResponseEntity.ok(newProfile); | ||
| } | ||
|
|
||
| @PostMapping("/cv/upload") | ||
| public ResponseEntity<Profile.CvData> uploadCv(@AuthenticationPrincipal String tumid, | ||
| @RequestParam("file") MultipartFile file) { | ||
| final Profile.CvData cvData; | ||
| try { | ||
| cvData = transcriptService.callCvParse(file.getBytes()); | ||
| } catch (Exception e) { | ||
| return ResponseEntity.status(HttpStatus.SERVICE_UNAVAILABLE).build(); | ||
| } | ||
| if (cvData == null) { | ||
| return ResponseEntity.status(HttpStatus.BAD_GATEWAY).build(); | ||
| } | ||
| final Profile current = transcriptService.fetchProfile(tumid); | ||
| final Profile.Student s = (current != null && current.student() != null) | ||
| ? current.student() | ||
| : new Profile.Student(null, 0, new String[0], new String[0], 0, null, null, null, null, false); | ||
| final Profile.Student updated = new Profile.Student( | ||
| s.studyProgramId(), s.semester(), s.careerGoals(), s.interests(), | ||
| s.preferredWorkload(), s.expectedGraduation(), s.industryPreference(), | ||
| s.rolePreference(), cvData, s.onboardingCompleted()); | ||
| final Profile newProfile = new Profile(updated, | ||
| current != null ? current.completedCourses() : List.of(), | ||
| current != null ? current.enrolledCourses() : List.of(), | ||
| current != null ? current.availableCourses() : List.of(), | ||
| current != null ? current.limit() : 0, | ||
| current != null ? current.category() : null, | ||
| current != null ? current.semesterKey() : null); | ||
| restClient.post().uri(PROFILE_SERVICE + "/upsert/" + tumid) | ||
| .contentType(MediaType.APPLICATION_JSON).body(newProfile).retrieve().toEntity(String.class); | ||
| return ResponseEntity.ok(cvData); | ||
| } | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Read-modify-write race can silently drop concurrent profile updates.
Both patchProfile and uploadCv fetch the current profile, merge in-memory, then upsert the whole record with no version/ETag check. Two overlapping calls (e.g. two onboarding steps submitted close together, a retry, or uploadCv racing a patchProfile) can both read the same current state; whichever upsert lands last wins and silently discards the other's changes.
A proper fix requires either optimistic concurrency (version/ETag on Profile, rejected on conflict) or moving the merge into PROFILE_SERVICE so the read-merge-write happens atomically server-side instead of in this API layer.
🧰 Tools
🪛 GitHub Actions: Lint & Format / 2_lint-server.txt
[error] 240-240: spotlessJavaCheck failed: formatting violations detected in this file. Run './gradlew :spotlessApply' to fix. Diff indicates conditional formatting changes required for patch.careerGoals() and patch.interests() ternary expressions.
🪛 GitHub Actions: Lint & Format / lint-server
[error] 240-240: spotlessJavaCheck (Spotless) failed due to formatting violations. Run './gradlew :spotlessApply' to fix.
[error] 240-240: Spotless formatting diff indicates spacing/line-break violations for null checks converting patch.careerGoals() and patch.interests() to arrays.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/server/src/main/java/tum/devops/http418/api/APIControllerMe.java`
around lines 233 - 296, The read-merge-upsert flow in patchProfile and uploadCv
can overwrite concurrent profile changes because both methods fetch a Profile,
build a new Profile in memory, and then upsert it without any concurrency check.
Fix this by adding optimistic concurrency to Profile/upsert in APIControllerMe,
using a version or ETag so conflicting updates are rejected, or by moving the
merge logic into PROFILE_SERVICE so the update happens atomically server-side.
Ensure the symbols patchProfile, uploadCv, and the PROFILE_SERVICE /upsert call
are updated together.
| restClient.post().uri(PROFILE_SERVICE + "/upsert/" + tumid) | ||
| .contentType(MediaType.APPLICATION_JSON).body(newProfile).retrieve().toEntity(String.class); | ||
| return ResponseEntity.ok(newProfile); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Unhandled failure on the PROFILE_SERVICE upsert call.
The upsert retrieve().toEntity(String.class) result is discarded and not wrapped in error handling in either endpoint. If the profile service is down or returns an error status, the exception propagates uncaught, producing an opaque 500 instead of a controlled response — unlike getProfile, which catches RestClientResponseException and maps the status code.
🔧 Suggested fix (apply to both patchProfile and uploadCv)
- restClient.post().uri(PROFILE_SERVICE + "/upsert/" + tumid)
- .contentType(MediaType.APPLICATION_JSON).body(newProfile).retrieve().toEntity(String.class);
- return ResponseEntity.ok(newProfile);
+ try {
+ restClient.post().uri(PROFILE_SERVICE + "/upsert/" + tumid)
+ .contentType(MediaType.APPLICATION_JSON).body(newProfile).retrieve().toEntity(String.class);
+ } catch (RestClientResponseException e) {
+ return ResponseEntity.status(e.getStatusCode()).build();
+ }
+ return ResponseEntity.ok(newProfile);Also applies to: 292-294
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/server/src/main/java/tum/devops/http418/api/APIControllerMe.java`
around lines 260 - 262, The PROFILE_SERVICE upsert call in patchProfile and
uploadCv currently discards the result of retrieve().toEntity(String.class) and
has no error handling, so failures escape as an opaque 500. Update the code
around the restClient.post().uri(..."/upsert/" + tumid) call in both endpoints
to handle RestClientResponseException the same way getProfile does, mapping the
downstream status into a controlled ResponseEntity response instead of letting
the exception propagate. Keep the newProfile flow unchanged on success, but
ensure the upsert response is not ignored and any profile-service error is
caught and translated consistently.
| } | ||
|
|
||
| export function completeOnboarding(data: Partial<StudentProfileUpdate>): Promise<StudentProfile> { | ||
| return patchProfile({ ...data, onboardingCompleted: true }); |
There was a problem hiding this comment.
this conflicts with my pr #114 . to update a profile, the client should send the complete profile so the server can just overwrite the existing one.
that makes storing the profile way easier as it can be done in a single table by making lists json strings
There was a problem hiding this comment.
this would then use POST instead of PATCH. since the client already has the full profile with all changes applied, there's no point in doing the same calculations on the server again, possibly introducing bugs and a mismatch between expected-actual stored data
|
also, I updated the StudentProfile type to match what the server returns. |
2 participants
Summary by CodeRabbit
New Features
Bug Fixes