Fix RAG exercise flows and authenticated AI E2E - #145
Conversation
|
Warning Review limit reached
Next review available in: 52 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe change makes OpenAI-compatible LLM usage the default, adds GenAI readiness/liveness handling and RAG validation, improves backend error and authorization behavior, updates frontend exercise flows, adjusts Compose and Helm deployment settings, and adds local authentication and AI QA documentation. ChangesLLM runtime and deployment
RAG and backend behavior
Frontend and QA
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Frontend
participant LearningService
participant GenAI
participant LLM
Frontend->>LearningService: request RAG learning plan
LearningService->>GenAI: submit topic, exercise types, and top_k
GenAI->>LLM: generate structured plan with timeout
LLM-->>GenAI: generated plan
GenAI-->>LearningService: normalized plan or error
LearningService-->>Frontend: validated plan or HTTP error
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
helm/team-drops/templates/genai.yaml (1)
67-76: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUse
/livefor the startup probe too.Changing only liveness leaves startup dependent on
/health. A missing or temporarily unavailable LLM configuration will therefore restart a healthy process instead of merely keeping it unready. Keep readiness on/health, but move startup to/live.Proposed fix
startupProbe: httpGet: - path: /health + path: /live port: app🤖 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 `@helm/team-drops/templates/genai.yaml` around lines 67 - 76, Update the startupProbe httpGet path in the genai deployment template to /live instead of /health, while preserving /health for the readiness probe and the existing livenessProbe configuration.
🧹 Nitpick comments (5)
backend/learning-service/src/main/java/de/tum/aet/devops26/learning_service/service/LessonService.java (1)
54-62: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPrefer batch inserts with
saveAll.Saving entities one by one inside a stream map results in multiple database round-trips. Consider mapping the elements to a list first and passing them to
saveAll()to allow the persistence provider to batch the inserts.♻️ Proposed refactor
- return IntStream.range(0, normalizedBlocks.size()) - .mapToObj(index -> lessonContentBlockRepository.save(LessonContentBlock.builder() - .lessonId(lessonId) - .orderNumber((int) existingBlockCount + index + 1) - .type("content") - .title("Lesson content") - .text(normalizedBlocks.get(index)) - .build())) - .toList(); + List<LessonContentBlock> blocks = IntStream.range(0, normalizedBlocks.size()) + .mapToObj(index -> LessonContentBlock.builder() + .lessonId(lessonId) + .orderNumber((int) existingBlockCount + index + 1) + .type("content") + .title("Lesson content") + .text(normalizedBlocks.get(index)) + .build()) + .toList(); + return lessonContentBlockRepository.saveAll(blocks);🤖 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 `@backend/learning-service/src/main/java/de/tum/aet/devops26/learning_service/service/LessonService.java` around lines 54 - 62, Update the lesson content creation flow to map normalizedBlocks into a list of LessonContentBlock entities first, then persist the entire list with lessonContentBlockRepository.saveAll(). Preserve the existing lessonId, orderNumber, type, title, and text values, and return the saved list.backend/learning-service/src/main/java/de/tum/aet/devops26/learning_service/integration/UserServiceClient.java (1)
31-37: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPreserve the auto-configured request factory.
Manually instantiating
SimpleClientHttpRequestFactoryreplaces the Spring Boot auto-configured request factory (which may use Apache HttpClient or the JDK HttpClient). This implicitly disables HTTP connection pooling and observability/tracing integrations.Consider applying timeouts programmatically using
ClientHttpRequestFactories(available in Spring Boot 3.2+), which applies the timeouts while retaining the optimal underlying factory type. Alternatively, configure timeouts globally via application properties (spring.http.client.connect-timeoutandspring.http.client.read-timeout).♻️ Proposed refactor using Spring Boot 3.2+ factory utilities
First, add the necessary imports:
+import org.springframework.boot.http.client.ClientHttpRequestFactorySettings; +import org.springframework.boot.web.client.ClientHttpRequestFactories;Then update the builder configuration:
- SimpleClientHttpRequestFactory requestFactory = new SimpleClientHttpRequestFactory(); - requestFactory.setConnectTimeout(CONNECT_TIMEOUT); - requestFactory.setReadTimeout(READ_TIMEOUT); this.restClient = restClientBuilder .baseUrl(baseUrl) - .requestFactory(requestFactory) + .requestFactory(ClientHttpRequestFactories.get( + ClientHttpRequestFactorySettings.DEFAULTS + .withConnectTimeout(CONNECT_TIMEOUT) + .withReadTimeout(READ_TIMEOUT) + )) .build();🤖 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 `@backend/learning-service/src/main/java/de/tum/aet/devops26/learning_service/integration/UserServiceClient.java` around lines 31 - 37, Update the UserServiceClient constructor to preserve Spring Boot’s auto-configured request factory instead of directly creating SimpleClientHttpRequestFactory. Apply CONNECT_TIMEOUT and READ_TIMEOUT through ClientHttpRequestFactories while configuring the existing restClientBuilder, or use the corresponding global HTTP client timeout properties if that is the project’s established configuration approach.backend/learning-service/src/main/resources/db/migration/V0_1__bootstrap_learning_schema.sql (1)
14-40: 🧹 Nitpick | 🔵 TrivialAdd indexes for foreign key columns.
The
lessons,exercises, andlesson_content_blockstables are heavily queried by their parent relations (e.g.,findByLessonIdOrderByOrderNumberAscandcountByLessonId). Without indexes on the foreign key columns (plan_idandlesson_id), the database will eventually fall back to full table scans, impacting performance as data volume grows.Consider defining indexes on these columns in this initial schema migration.
💡 Suggested SQL addition
CREATE INDEX idx_lessons_plan_id ON lessons(plan_id); CREATE INDEX idx_exercises_lesson_id ON exercises(lesson_id); CREATE INDEX idx_lesson_content_blocks_lesson_id ON lesson_content_blocks(lesson_id);🤖 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 `@backend/learning-service/src/main/resources/db/migration/V0_1__bootstrap_learning_schema.sql` around lines 14 - 40, Add indexes for the foreign-key columns in the bootstrap schema: create indexes on lessons.plan_id, exercises.lesson_id, and lesson_content_blocks.lesson_id alongside the table definitions, using clear unique names such as idx_lessons_plan_id, idx_exercises_lesson_id, and idx_lesson_content_blocks_lesson_id.frontend/src/pages/LearningPage.jsx (1)
1096-1098: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove redundant URL revocation.
Since there is already a dedicated
useEffect(lines 1105–1111) that revokes the object URL whenaudioPreviewUrlchanges or unmounts, explicitly callingURL.revokeObjectURL(audioPreviewUrl)here is redundant.Updating the state with
setAudioPreviewUrl('')will automatically trigger that existing cleanup effect.♻️ Proposed refactor
if (reviewed && !answerError) { - if (audioPreviewUrl) { - URL.revokeObjectURL(audioPreviewUrl); - } setSelectedAudio(null);🤖 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 `@frontend/src/pages/LearningPage.jsx` around lines 1096 - 1098, Remove the explicit URL.revokeObjectURL(audioPreviewUrl) call from the audio preview cleanup flow, leaving setAudioPreviewUrl('') to trigger the existing useEffect cleanup for audioPreviewUrl changes and unmounting.frontend/src/api/client.js (1)
56-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRecurse on nested error structures.
If the backend returns a complex object within the
messageordetailfield (e.g.,{"detail": {"field": "Invalid input"}}), returning it directly will cause the standardErrorconstructor to stringify it as[object Object].Consider recursing on the extracted value to ensure the fallback always resolves to a clean string format.
♻️ Proposed refactor
if (value && typeof value === 'object') { - return value.message ?? value.detail ?? JSON.stringify(value); + const nested = value.message ?? value.detail; + return nested !== undefined ? readableErrorMessage(nested) : JSON.stringify(value); }🤖 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 `@frontend/src/api/client.js` around lines 56 - 58, Update the object-handling logic in the error-formatting function around the message/detail extraction so nested values are passed back through the same formatter recursively, rather than returned directly. Preserve the existing precedence of value.message, value.detail, and JSON.stringify(value), while ensuring nested objects ultimately resolve to a clean string instead of “[object Object]”.
🤖 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
`@backend/learning-service/src/main/java/de/tum/aet/devops26/learning_service/integration/GenAiRagLearningPlanClient.java`:
- Around line 116-126: The GenAI error translation must sanitize upstream
failures while preserving timeout semantics: in GenAiRagLearningPlanClient.java
lines 116-126, map upstream HTTP 504 to GATEWAY_TIMEOUT with a fixed safe
timeout response message and log only sanitized status/context; in lines
160-176, allowlist only explicitly safe error contracts and stop returning
arbitrary message or detail fields. Update GenAiRagLearningPlanClientTests.java
lines 118-121 to remove the raw-detail assertion and add coverage confirming an
upstream 504 remains 504.
In `@docs/local-auth-ai-test-plan.md`:
- Around line 32-34: Remove the hard-coded password from the test credentials in
the local authentication test plan. Replace it with a local-only placeholder and
instruct testers to generate or inject the password at runtime, while preserving
the email and username values.
In `@docs/local-auth-ai-test-report.md`:
- Line 27: Remove machine-specific filesystem paths from the QA documentation:
in docs/local-auth-ai-test-report.md lines 27-27, describe the local .env source
generically; in docs/local-auth-ai-test-plan.md lines 17-17 and
docs/local-auth-ai-test-report.md lines 4-4, replace absolute worktree paths
with generic repository-relative worktree wording.
In `@genai/app/prompts/rag.py`:
- Around line 58-61: Update the interview-related instruction in the RAG prompt
construction to apply only when the topic or learning goal contains
software-engineering interview signals. For medical, hospitality, admissions,
and other non-software domains, preserve the domain established by the topic,
learning goal, and RAG context.
In `@genai/app/routers/rag.py`:
- Around line 359-375: Update the reading fallback in the exercise-generation
match so its question, answer choices, and expected_answer are generated in the
plan’s target language using the available lesson context, rather than hardcoded
English. Preserve the existing multiple-choice structure and ensure the repaired
exercise still tests the requested language for German, French, and other plans.
In `@genai/app/schemas/rag.py`:
- Around line 318-326: Update the lesson normalization loop in the lessons
processing block to enumerate valid dictionary lessons and overwrite each
lesson’s order_number with its sequential 1-based index. Preserve the existing
_plan_topic, _plan_level, and _plan_language metadata while ensuring
LLM-provided order_number values cannot create duplicates or gaps.
In `@genai/tests/test_auth_middleware.py`:
- Line 13: Update the response status assertion in the authentication middleware
health test to accept only 200 or the readiness-related 503 status, rejecting
401, 403, 404, 500, and other unexpected statuses.
In `@README.md`:
- Around line 70-73: Update the Requirements section in README.md to make the
LLM prerequisite conditional: require either an OpenAI-compatible API key in
LLM_API_KEY or the resources needed to run the local Ollama profile. Align the
nearby Ollama setup guidance in the lines covering the local profile so both
alternatives are clearly described.
---
Outside diff comments:
In `@helm/team-drops/templates/genai.yaml`:
- Around line 67-76: Update the startupProbe httpGet path in the genai
deployment template to /live instead of /health, while preserving /health for
the readiness probe and the existing livenessProbe configuration.
---
Nitpick comments:
In
`@backend/learning-service/src/main/java/de/tum/aet/devops26/learning_service/integration/UserServiceClient.java`:
- Around line 31-37: Update the UserServiceClient constructor to preserve Spring
Boot’s auto-configured request factory instead of directly creating
SimpleClientHttpRequestFactory. Apply CONNECT_TIMEOUT and READ_TIMEOUT through
ClientHttpRequestFactories while configuring the existing restClientBuilder, or
use the corresponding global HTTP client timeout properties if that is the
project’s established configuration approach.
In
`@backend/learning-service/src/main/java/de/tum/aet/devops26/learning_service/service/LessonService.java`:
- Around line 54-62: Update the lesson content creation flow to map
normalizedBlocks into a list of LessonContentBlock entities first, then persist
the entire list with lessonContentBlockRepository.saveAll(). Preserve the
existing lessonId, orderNumber, type, title, and text values, and return the
saved list.
In
`@backend/learning-service/src/main/resources/db/migration/V0_1__bootstrap_learning_schema.sql`:
- Around line 14-40: Add indexes for the foreign-key columns in the bootstrap
schema: create indexes on lessons.plan_id, exercises.lesson_id, and
lesson_content_blocks.lesson_id alongside the table definitions, using clear
unique names such as idx_lessons_plan_id, idx_exercises_lesson_id, and
idx_lesson_content_blocks_lesson_id.
In `@frontend/src/api/client.js`:
- Around line 56-58: Update the object-handling logic in the error-formatting
function around the message/detail extraction so nested values are passed back
through the same formatter recursively, rather than returned directly. Preserve
the existing precedence of value.message, value.detail, and
JSON.stringify(value), while ensuring nested objects ultimately resolve to a
clean string instead of “[object Object]”.
In `@frontend/src/pages/LearningPage.jsx`:
- Around line 1096-1098: Remove the explicit
URL.revokeObjectURL(audioPreviewUrl) call from the audio preview cleanup flow,
leaving setAudioPreviewUrl('') to trigger the existing useEffect cleanup for
audioPreviewUrl changes and unmounting.
🪄 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: acbb668f-6142-494f-84f4-1855c17274d3
📒 Files selected for processing (44)
.env.exampleREADME.mdapi/openapi.yamlapi/services/genai.yamlbackend/learning-service/src/main/java/de/tum/aet/devops26/learning_service/api/impl/ApiExceptionHandler.javabackend/learning-service/src/main/java/de/tum/aet/devops26/learning_service/integration/GenAiRagLearningPlanClient.javabackend/learning-service/src/main/java/de/tum/aet/devops26/learning_service/integration/UserServiceClient.javabackend/learning-service/src/main/java/de/tum/aet/devops26/learning_service/repository/LessonContentBlockRepository.javabackend/learning-service/src/main/java/de/tum/aet/devops26/learning_service/service/LearningPlanService.javabackend/learning-service/src/main/java/de/tum/aet/devops26/learning_service/service/LessonService.javabackend/learning-service/src/main/resources/application.propertiesbackend/learning-service/src/main/resources/db/migration/V0_1__bootstrap_learning_schema.sqlbackend/learning-service/src/test/java/de/tum/aet/devops26/learning_service/integration/GenAiRagLearningPlanClientTests.javabackend/learning-service/src/test/java/de/tum/aet/devops26/learning_service/integration/UserServiceClientTests.javabackend/learning-service/src/test/java/de/tum/aet/devops26/learning_service/service/LearningPlanServiceTests.javabackend/learning-service/src/test/java/de/tum/aet/devops26/learning_service/service/LessonServiceTests.javabackend/progress-feedback-service/src/main/java/de/tum/aet/devops26/progress_feedback_service/api/impl/ApiExceptionHandler.javabackend/progress-feedback-service/src/main/java/de/tum/aet/devops26/progress_feedback_service/api/impl/ProgressFeedbackServiceController.javabackend/progress-feedback-service/src/main/java/de/tum/aet/devops26/progress_feedback_service/service/UserAnswerService.javabackend/progress-feedback-service/src/main/resources/application.propertiesbackend/progress-feedback-service/src/test/java/de/tum/aet/devops26/progress_feedback_service/service/UserAnswerServiceTests.javadocker-compose.ymldocs/keycloak-authentication.mddocs/local-auth-ai-test-plan.mddocs/local-auth-ai-test-report.mdfrontend/src/api/client.jsfrontend/src/api/client.test.jsfrontend/src/api/mappers.jsfrontend/src/api/mappers.test.jsfrontend/src/pages/LearningPage.jsxgenai/app/config.pygenai/app/llm/client.pygenai/app/main.pygenai/app/middleware/auth.pygenai/app/prompts/rag.pygenai/app/routers/rag.pygenai/app/schemas/rag.pygenai/tests/test_auth_middleware.pygenai/tests/test_llm_config.pygenai/tests/test_rag.pyhelm/team-drops/templates/configmap.yamlhelm/team-drops/templates/genai.yamlhelm/team-drops/values.yamlkeycloak/realm-export.json
| if (httpResponse.statusCode() < 200 || httpResponse.statusCode() >= 300) { | ||
| LOGGER.warn("GenAI RAG learning-plan generation rejected: {}", httpResponse.body()); | ||
| String rejectionMessage = genAiErrorMessage(httpResponse.body()); | ||
| LOGGER.warn( | ||
| "GenAI RAG learning-plan generation rejected (status {}): {}", | ||
| httpResponse.statusCode(), | ||
| httpResponse.body() | ||
| ); | ||
| throw new ResponseStatusException( | ||
| HttpStatus.BAD_GATEWAY, | ||
| "GenAI service rejected RAG learning-plan generation: " + httpResponse.body() | ||
| rejectionMessage | ||
| ); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Sanitize downstream errors while preserving generation timeouts.
The current translation turns GenAI 504 responses into 502 and exposes arbitrary upstream diagnostics through logs and API responses.
backend/learning-service/src/main/java/de/tum/aet/devops26/learning_service/integration/GenAiRagLearningPlanClient.java#L116-L126: map upstream 504 toGATEWAY_TIMEOUT, return a fixed safe timeout message, and log only sanitized status/context.backend/learning-service/src/main/java/de/tum/aet/devops26/learning_service/integration/GenAiRagLearningPlanClient.java#L160-L176: stop returning arbitrarymessageordetailvalues; allowlist only explicitly safe error contracts.backend/learning-service/src/test/java/de/tum/aet/devops26/learning_service/integration/GenAiRagLearningPlanClientTests.java#L118-L121: replace the raw-detail assertion and add coverage that upstream 504 remains 504.
📍 Affects 2 files
backend/learning-service/src/main/java/de/tum/aet/devops26/learning_service/integration/GenAiRagLearningPlanClient.java#L116-L126(this comment)backend/learning-service/src/main/java/de/tum/aet/devops26/learning_service/integration/GenAiRagLearningPlanClient.java#L160-L176backend/learning-service/src/test/java/de/tum/aet/devops26/learning_service/integration/GenAiRagLearningPlanClientTests.java#L118-L121
🤖 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
`@backend/learning-service/src/main/java/de/tum/aet/devops26/learning_service/integration/GenAiRagLearningPlanClient.java`
around lines 116 - 126, The GenAI error translation must sanitize upstream
failures while preserving timeout semantics: in GenAiRagLearningPlanClient.java
lines 116-126, map upstream HTTP 504 to GATEWAY_TIMEOUT with a fixed safe
timeout response message and log only sanitized status/context; in lines
160-176, allowlist only explicitly safe error contracts and stop returning
arbitrary message or detail fields. Update GenAiRagLearningPlanClientTests.java
lines 118-121 to remove the raw-detail assertion and add coverage confirming an
upstream 504 remains 504.
| - Email: `qa+local-rag@example.com` | ||
| - Username: `qa-local-rag` | ||
| - Password: `QaLocalRag!2026` |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not commit a reusable QA password.
Even for a disposable account, storing the password in the repository creates a credential-leakage risk. Replace it with a local-only placeholder and instruct testers to generate or inject the password at runtime.
Proposed documentation change
- Email: `qa+local-rag@example.com`
- Username: `qa-local-rag`
-- Password: `QaLocalRag!2026`
+- Password: `<set locally; do not commit>`📝 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.
| - Email: `qa+local-rag@example.com` | |
| - Username: `qa-local-rag` | |
| - Password: `QaLocalRag!2026` | |
| - Email: `qa+local-rag@example.com` | |
| - Username: `qa-local-rag` | |
| - Password: `<set locally; do not commit>` |
🤖 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 `@docs/local-auth-ai-test-plan.md` around lines 32 - 34, Remove the hard-coded
password from the test credentials in the local authentication test plan.
Replace it with a local-only placeholder and instruct testers to generate or
inject the password at runtime, while preserving the email and username values.
|
|
||
| ## Fresh Final Rerun | ||
|
|
||
| After the final review fixes, the stack was rebuilt and recreated from this branch with auth enabled and the external OpenAI-compatible API-key runtime. The temp worktree does not have its own `.env`, so the run used `/Users/juliankraus/Coding/Coding-Uni/DevOps/team-drops/.env` plus `AUTH_ENABLED=true`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove machine-specific filesystem paths from the QA artifacts. Replace them with generic, repository-relative wording so the documents remain portable and do not disclose local workstation details.
docs/local-auth-ai-test-report.md#L27-L27: redact/Users/.../.envand describe the local.envsource generically.docs/local-auth-ai-test-plan.md#L17-L17: replace the absolute worktree path with a generic worktree reference.docs/local-auth-ai-test-report.md#L4-L4: replace the absolute worktree path with a generic worktree reference.
📍 Affects 2 files
docs/local-auth-ai-test-report.md#L27-L27(this comment)docs/local-auth-ai-test-plan.md#L17-L17docs/local-auth-ai-test-report.md#L4-L4
🤖 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 `@docs/local-auth-ai-test-report.md` at line 27, Remove machine-specific
filesystem paths from the QA documentation: in docs/local-auth-ai-test-report.md
lines 27-27, describe the local .env source generically; in
docs/local-auth-ai-test-plan.md lines 17-17 and
docs/local-auth-ai-test-report.md lines 4-4, replace absolute worktree paths
with generic repository-relative worktree wording.
| "When the topic or learning goal is interview-related, frame exercises around " | ||
| "software engineering interviews: project explanations, technical trade-offs, " | ||
| "behavioral answers, collaboration, debugging, system design, and clear spoken " | ||
| "or written engineering communication. " |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Limit software-engineering framing to software-engineering interview requests.
“Interview-related” also includes medical, hospitality, admissions, and other domains. Gate this instruction on software-engineering signals; otherwise preserve the domain supplied by the topic, goal, and RAG context.
🤖 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 `@genai/app/prompts/rag.py` around lines 58 - 61, Update the interview-related
instruction in the RAG prompt construction to apply only when the topic or
learning goal contains software-engineering interview signals. For medical,
hospitality, admissions, and other non-software domains, preserve the domain
established by the topic, learning goal, and RAG context.
| match exercise_type: | ||
| case "reading": | ||
| return RagLearningPlanExercise( | ||
| type="reading", | ||
| subtype="multiple_choice", | ||
| question=( | ||
| f"Which action best supports this learning goal about {topic}: {goal}?\n" | ||
| "A) Give a concise answer with a concrete example from the lesson context.\n" | ||
| "B) Ignore the topic and answer with unrelated personal details.\n" | ||
| "C) Focus only on memorized grammar rules without answering the question.\n" | ||
| "D) Change the subject instead of responding to the task." | ||
| ), | ||
| expected_answer=( | ||
| "A) Give a concise answer with a concrete example from the lesson context." | ||
| ), | ||
| difficulty=level, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Do not backfill target-language reading exercises in English.
When reading is missing, this creates an entirely English question and answer set—even for German or French plans. Generate the fallback from target-language lesson context so the repaired exercise still tests the requested language.
🤖 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 `@genai/app/routers/rag.py` around lines 359 - 375, Update the reading fallback
in the exercise-generation match so its question, answer choices, and
expected_answer are generated in the plan’s target language using the available
lesson context, rather than hardcoded English. Preserve the existing
multiple-choice structure and ensure the repaired exercise still tests the
requested language for German, French, and other plans.
| lessons = [] | ||
| for lesson in normalized.get("lessons") or []: | ||
| if isinstance(lesson, dict): | ||
| lesson = {**lesson, "_plan_topic": topic, "_plan_level": level} | ||
| lesson = { | ||
| **lesson, | ||
| "_plan_topic": topic, | ||
| "_plan_level": level, | ||
| "_plan_language": language, | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Renumber lessons instead of trusting LLM-provided values.
This forwards arbitrary positive order_number values, allowing duplicates or gaps to reach persistence. Enumerate the lessons and overwrite their order with 1..N.
Proposed fix
- for lesson in normalized.get("lessons") or []:
+ for order_number, lesson in enumerate(
+ normalized.get("lessons") or [], start=1
+ ):
if isinstance(lesson, dict):
lesson = {
**lesson,
+ "order_number": order_number,
"_plan_topic": topic,
"_plan_level": level,
"_plan_language": language,
}📝 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.
| lessons = [] | |
| for lesson in normalized.get("lessons") or []: | |
| if isinstance(lesson, dict): | |
| lesson = {**lesson, "_plan_topic": topic, "_plan_level": level} | |
| lesson = { | |
| **lesson, | |
| "_plan_topic": topic, | |
| "_plan_level": level, | |
| "_plan_language": language, | |
| } | |
| lessons = [] | |
| for order_number, lesson in enumerate( | |
| normalized.get("lessons") or [], start=1 | |
| ): | |
| if isinstance(lesson, dict): | |
| lesson = { | |
| **lesson, | |
| "order_number": order_number, | |
| "_plan_topic": topic, | |
| "_plan_level": level, | |
| "_plan_language": language, | |
| } |
🤖 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 `@genai/app/schemas/rag.py` around lines 318 - 326, Update the lesson
normalization loop in the lessons processing block to enumerate valid dictionary
lessons and overwrite each lesson’s order_number with its sequential 1-based
index. Preserve the existing _plan_topic, _plan_level, and _plan_language
metadata while ensuring LLM-provided order_number values cannot create
duplicates or gaps.
|
|
||
| response = client.get("/health") | ||
|
|
||
| assert response.status_code != 401 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Restrict the accepted health statuses.
!= 401 also passes for 403, 404, or 500. Assert 200 or readiness-related 503 so authentication and routing regressions fail this test.
Proposed fix
- assert response.status_code != 401
+ assert response.status_code in {200, 503}📝 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.
| assert response.status_code != 401 | |
| assert response.status_code in {200, 503} |
🤖 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 `@genai/tests/test_auth_middleware.py` at line 13, Update the response status
assertion in the authentication middleware health test to accept only 200 or the
readiness-related 503 status, rejecting 401, 403, 404, 500, and other unexpected
statuses.
| Requirements: | ||
|
|
||
| - Docker with Docker Compose v2 | ||
| - Enough memory and disk space to run the services and download the configured | ||
| Ollama model | ||
| - An OpenAI-compatible external LLM API key in `LLM_API_KEY`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Describe the API key and Ollama as alternative requirements.
Line 73 makes an external API key unconditional, but Lines 86-92 support running exclusively with local Ollama. Change the requirement to “an OpenAI-compatible API key, or resources for the local Ollama profile.”
Also applies to: 83-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 `@README.md` around lines 70 - 73, Update the Requirements section in README.md
to make the LLM prerequisite conditional: require either an OpenAI-compatible
API key in LLM_API_KEY or the resources needed to run the local Ollama profile.
Align the nearby Ollama setup guidance in the lines covering the local profile
so both alternatives are clearly described.
2 participants
Summary
Verification
qa-local-ragLLM_PROVIDER=openai,LLM_API_KEY,LLM_MODEL=openai/gpt-oss-120b, and configured base URL400 No speech was detected...backend/learning-service: focused Gradle tests passeddocs/local-auth-ai-test-report.mdNotes
Summary by CodeRabbit
New Features
Bug Fixes
Documentation