Development: AtlasAgent Tools Integration
#11482
Conversation
…tools (create competency, list competencies etc..)
github-actions
bot
added
server
WalkthroughAdds Spring AI tool support and a request-scoped AtlasAgentToolsService; extends AtlasAgentService to use ToolCallbackProvider, chat memory, and return an AgentChatResult with a competenciesModified flag; updates REST, DTOs, frontend service/component/tests to pass sessionId and propagate competency-change events; replaces the agent system prompt. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as AgentChatModal (frontend)
participant FService as AgentChatService (frontend)
participant API as AtlasAgentResource (REST)
participant Agent as AtlasAgentService (backend)
participant Tools as AtlasAgentToolsService (request-scoped)
participant AI as ChatClient (Spring AI)
UI->>FService: sendMessage(message, courseId)
FService->>FService: derive sessionId (course_{id}_user_{id})
FService->>API: POST /atlas/agent/courses/{courseId}/chat {message, sessionId}
API->>Agent: processChatMessage(message, courseId, sessionId)
Agent->>Agent: build system prompt, attach MessageChatMemoryAdvisor (sessionId)
Agent->>Agent: register ToolCallbackProvider (if present)
Agent->>AI: execute chat with tools & memory
AI->>Tools: invoke @Tool method (e.g., createCompetency)
Tools->>Tools: persist data, set wasCompetencyCreated flag
Tools-->>AI: return tool result (JSON)
AI-->>Agent: final response
Agent->>Tools: wasCompetencyCreated() -> competenciesModified
Agent-->>API: AgentChatResult{message, competenciesModified}
API-->>FService: AtlasAgentChatResponseDTO{message,...,competenciesModified}
FService-->>UI: AgentChatResponse
UI->>UI: emit competencyChanged if competenciesModified
UI-->>CompetencyMgmt: competencyChanged triggers refresh
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Pre-merge checks and finishing touches✅ Passed checks (3 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: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/java/de/tum/cit/aet/artemis/atlas/config/AtlasAgentToolConfig.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/dto/AtlasAgentChatResponseDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/dto/ChatHistoryMessageDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.java(3 hunks)src/main/resources/prompts/atlas/agent_system_prompt.st(1 hunks)src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat-modal.component.ts(5 hunks)src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.ts(3 hunks)src/main/webapp/app/atlas/manage/competency-management/competency-management.component.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/atlas/manage/competency-management/competency-management.component.tssrc/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.tssrc/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat-modal.component.ts
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
Files:
src/main/java/de/tum/cit/aet/artemis/atlas/config/AtlasAgentToolConfig.javasrc/main/java/de/tum/cit/aet/artemis/atlas/dto/AtlasAgentChatResponseDTO.javasrc/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.javasrc/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.javasrc/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.javasrc/main/java/de/tum/cit/aet/artemis/atlas/dto/ChatHistoryMessageDTO.java
🧬 Code graph analysis (3)
src/main/java/de/tum/cit/aet/artemis/atlas/config/AtlasAgentToolConfig.java (1)
src/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.java (1)
Conditional(40-140)
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java (2)
src/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.java (1)
Conditional(40-140)src/main/java/de/tum/cit/aet/artemis/atlas/config/AtlasEnabled.java (1)
AtlasEnabled(13-25)
src/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.java (1)
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.java (1)
Lazy(26-131)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.java
Outdated
Show resolved
Hide resolved
src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat-modal.component.ts
Outdated
Show resolved
Hide resolved
github-project-automation
bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
WalkthroughAdds Spring AI tool wiring and in-memory chat memory, introduces AtlasAgentToolsService with tool-annotated methods, extends chat API with session-based context and history retrieval, updates DTOs, adjusts system prompt, and modifies Angular client to fetch history, handle new response shape, and emit competency-changed events to refresh competencies. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Angular UI (Modal)
participant Svc as AgentChatService
participant API as AtlasAgentResource
participant Agent as AtlasAgentService
participant Tools as AtlasAgentToolsService
participant Mem as ChatMemory
participant LLM as ChatClient/LLM
rect rgb(245,248,255)
note over UI,Svc: Send chat message
User->>UI: Type message
UI->>Svc: sendMessage(message, courseId)
Svc->>API: POST /courses/{courseId}/chat
API->>Agent: processChatMessage(message, courseId, sessionId)
Agent->>Mem: store/retrieve history (by sessionId)
Agent->>LLM: prompt (with history, tool callbacks)
par Tool invocation (if needed)
LLM->>Agent: tool call
Agent->>Tools: execute tool
Tools-->>Agent: result (JSON/plain)
Tools->>Tools: mark competencies modified (ThreadLocal)
end
Agent-->>API: response text
API->>API: read ThreadLocal flag
API-->>Svc: { message, success, competenciesModified }
Svc-->>UI: AgentChatResponse
UI->>UI: emit competencyChanged if true
end
sequenceDiagram
autonumber
actor User
participant UI as Angular UI (Modal)
participant Svc as AgentChatService
participant API as AtlasAgentResource
participant Agent as AtlasAgentService
participant Mem as ChatMemory
rect rgb(245,248,255)
note over UI,API: Load conversation history
User->>UI: Open modal
UI->>Svc: getHistory(courseId)
Svc->>API: GET /courses/{courseId}/history
API->>Agent: getConversationHistory(sessionId)
Agent->>Mem: fetch messages (by sessionId)
Mem-->>Agent: message list
Agent-->>API: history (system msgs filtered)
API-->>Svc: [ { role, content } ]
Svc-->>UI: history
UI->>UI: render messages
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PMD (7.17.0)src/main/java/de/tum/cit/aet/artemis/atlas/dto/ChatHistoryMessageDTO.java[ERROR] Error at ruleset.xml:58:5 59| ... [truncated 11412 characters] ... prone.xml/InaccurateNumericLiteral instead of the deprecated Rule name category/ecmascript/errorprone.xml/InnaccurateNumericLiteral. PMD 8.0.0 will remove support for this deprecated Rule name usage. 185| src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.java[0.001s][warning][perf,memops] Cannot use file /tmp/hsperfdata_jailuser/1 because it is locked by another process (errno = 11) 59| ... [truncated 11539 characters] ... prone.xml/InaccurateNumericLiteral instead of the deprecated Rule name category/ecmascript/errorprone.xml/InnaccurateNumericLiteral. PMD 8.0.0 will remove support for this deprecated Rule name usage. 185| src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java[0.002s][warning][perf,memops] Cannot use file /tmp/hsperfdata_jailuser/1 because it is locked by another process (errno = 11) 59| ... [truncated 11539 characters] ... prone.xml/InaccurateNumericLiteral instead of the deprecated Rule name category/ecmascript/errorprone.xml/InnaccurateNumericLiteral. PMD 8.0.0 will remove support for this deprecated Rule name usage. 185|
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/java/de/tum/cit/aet/artemis/atlas/config/AtlasAgentToolConfig.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/dto/AtlasAgentChatResponseDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/dto/ChatHistoryMessageDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.java(3 hunks)src/main/resources/prompts/atlas/agent_system_prompt.st(1 hunks)src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat-modal.component.ts(5 hunks)src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.ts(3 hunks)src/main/webapp/app/atlas/manage/competency-management/competency-management.component.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/atlas/manage/competency-management/competency-management.component.tssrc/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat-modal.component.tssrc/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.ts
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
Files:
src/main/java/de/tum/cit/aet/artemis/atlas/dto/AtlasAgentChatResponseDTO.javasrc/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.javasrc/main/java/de/tum/cit/aet/artemis/atlas/dto/ChatHistoryMessageDTO.javasrc/main/java/de/tum/cit/aet/artemis/atlas/config/AtlasAgentToolConfig.javasrc/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.javasrc/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java
🧬 Code graph analysis (3)
src/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.java (1)
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.java (1)
Lazy(26-131)
src/main/java/de/tum/cit/aet/artemis/atlas/config/AtlasAgentToolConfig.java (1)
src/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.java (1)
Conditional(40-140)
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java (2)
src/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.java (1)
Conditional(40-140)src/main/java/de/tum/cit/aet/artemis/atlas/config/AtlasEnabled.java (1)
AtlasEnabled(13-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Build and Push Docker Image / Build linux/arm64 Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build linux/amd64 Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: client-style
- GitHub Check: Analyse
- GitHub Check: server-style
- GitHub Check: bean-instantiation-check
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.java
Outdated
Show resolved
Hide resolved
src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat-modal.component.ts
Outdated
Show resolved
Hide resolved
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
src/main/java/de/tum/cit/aet/artemis/atlas/dto/AtlasAgentChatResponseDTO.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/atlas/service/AgentChatResult.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.java
Outdated
Show resolved
Hide resolved
src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat-modal.component.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/atlas/manage/competency-management/competency-management.component.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.ts (1)
29-31: Session collision remains unresolved: userId fallback to 0.A previous major issue flagged that defaulting
userIdto0whenaccountService.userIdentityis not loaded causes session collision—multiple unauthenticated or early-load users share the samecourse_<courseId>_user_0session key, mixing their conversation histories and violating per-user isolation. The backend stores chat memory per course/user, so this breaks continuity once the real user ID becomes available.Gate the call until a real account ID is present by awaiting or subscribing to
accountService.identity(), or return an error if identity cannot be resolved. Do not construct the sessionId or call the backend with a zero user ID.src/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.java (1)
65-80: Unresolved: Error responses lose competency modification state.Previous reviews flagged that if tools already modified competencies before a timeout/interruption/execution error, these error handlers hardcode
competenciesModified = false, leaving the UI unaware of actual backend changes. Users see stale competency lists even though data was modified.Capture the actual modification state before constructing error responses. If
AtlasAgentToolsServiceorAgentChatResulttracks modifications via ThreadLocal or similar, read that flag into a local boolean before cleanup and pass it into each error DTO:catch (TimeoutException te) { log.warn("Chat timed out for course {}: {}", courseId, te.getMessage()); + boolean competenciesModified = /* read from ThreadLocal or service */; return ResponseEntity.status(HttpStatus.GATEWAY_TIMEOUT) - .body(new AtlasAgentChatResponseDTO("The agent timed out. Please try again.", request.sessionId(), ZonedDateTime.now(), false, false)); + .body(new AtlasAgentChatResponseDTO("The agent timed out. Please try again.", request.sessionId(), ZonedDateTime.now(), false, competenciesModified)); }Apply the same fix to the
InterruptedExceptionandExecutionExceptionbranches.
🧹 Nitpick comments (2)
src/main/java/de/tum/cit/aet/artemis/atlas/service/AgentChatResult.java (1)
1-10: Consider relocating record to dto package.AgentChatResult is used as an internal result object between service and web layers, functioning as a DTO. Per coding guidelines (
dtos:{java_records,no_entities,min_data,single_resp}), records serving as data transfer objects typically belong in adtopackage rather thanservice. While this works functionally, relocating tode.tum.cit.aet.artemis.atlas.dto.AgentChatResultwould improve consistency with the codebase's architectural patterns.src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java (1)
100-144: Add Java validation to title and description fields; authorization is properly inherited.The
@EnforceAtLeastInstructorInCourseannotation onAtlasAgentResource.sendChatMessage()protects the endpoint, and authorization context is inherited by tool methods through Spring Security's thread-local propagation—no explicit checks needed in the tool service.However, validation is insufficient.
BaseCompetency.titlehas only@Column(nullable=false)(database-level), anddescriptionhas no constraint. ThecreateCompetencymethod allows null or empty strings to be persisted. Add Java validation annotations:
title:@NotBlank(non-null, non-empty, non-whitespace)description:@NotNullor@NotBlank(depending on design)This prevents the AI agent from creating invalid competencies and provides immediate feedback without database errors.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/java/de/tum/cit/aet/artemis/atlas/dto/AtlasAgentChatResponseDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/service/AgentChatResult.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.java(3 hunks)src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat-modal.component.spec.ts(6 hunks)src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat-modal.component.ts(3 hunks)src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.spec.ts(6 hunks)src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.ts(3 hunks)src/main/webapp/app/atlas/manage/competency-management/competency-management.component.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/webapp/app/atlas/manage/competency-management/competency-management.component.ts
- src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.spec.ts
- src/main/java/de/tum/cit/aet/artemis/atlas/dto/AtlasAgentChatResponseDTO.java
🧰 Additional context used
📓 Path-based instructions (2)
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
Files:
src/main/java/de/tum/cit/aet/artemis/atlas/service/AgentChatResult.javasrc/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.javasrc/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.javasrc/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.java
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat-modal.component.tssrc/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat-modal.component.spec.tssrc/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.ts
🧬 Code graph analysis (1)
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java (4)
src/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.java (1)
Conditional(33-82)src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.java (1)
Lazy(23-100)src/main/java/de/tum/cit/aet/artemis/atlas/config/AtlasAgentToolConfig.java (1)
Lazy(17-35)src/main/java/de/tum/cit/aet/artemis/atlas/config/AtlasEnabled.java (1)
AtlasEnabled(13-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Build and Push Docker Image / Build linux/amd64 Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build linux/arm64 Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: client-style
- GitHub Check: client-tests-selected
- GitHub Check: server-tests
- GitHub Check: server-style
- GitHub Check: client-tests
- GitHub Check: Analyse
- GitHub Check: bean-instantiation-check
🔇 Additional comments (7)
src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat-modal.component.ts (2)
53-54: LGTM! Signal-based output addresses past feedback.The use of
output<void>()forcompetencyChangedcorrectly follows Angular's modern signal-based API, addressing the previous review feedback to migrate away from old decorators.
104-109: LGTM! Proper fallback and event propagation.The fallback for empty
response.messageprevents undefined display, and the conditional emission ofcompetencyChangedwhencompetenciesModifiedis true correctly notifies parent components to refresh. The logic is sound.src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.ts (1)
18-18: LGTM! Interface updated for competency tracking.The addition of
competenciesModified: booleantoAgentChatResponsecorrectly supports the new competency change detection flow.src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat-modal.component.spec.ts (1)
254-260: LGTM! Comprehensive test coverage for new response structure.The mock responses correctly include all fields from the updated
AgentChatResponseinterface (sessionId,timestamp,success,competenciesModified), and the new test suite at lines 524-575 thoroughly validates both the true and false paths for competency modification events.Also applies to: 534-541
src/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.java (1)
60-63: LGTM! Success path correctly propagates competency modification flag.The success response correctly extracts
result.competenciesModified()fromAgentChatResultand includes it in theAtlasAgentChatResponseDTO, ensuring the client receives accurate modification state.src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java (1)
215-223: LGTM! Robust error handling in JSON serialization.The
toJsonhelper correctly catchesJsonProcessingExceptionand returns a well-formed error JSON, preventing serialization failures from propagating as exceptions. The fallback message is clear and actionable.src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.java (1)
34-40: LGTM! Proper dependency injection for tool support.The constructor correctly injects
ToolCallbackProviderwith@Autowired(required = false), allowing the service to function with or without tool support. This follows Spring best practices for optional dependencies.
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/webapp/app/atlas/manage/competency-management/competency-management.component.spec.ts (1)
240-263: Consider verifying the competencyChanged subscription and auto-refresh behavior.The test mocks
competencyChanged.subscribebut never verifies that the component actually subscribes to it or callsloadCourseCompetencieswhen the event fires. According to the AI summary, the modal should trigger auto-refresh of course competencies via thecompetencyChangedevent, which is a key integration point.While the current test verifies the basic modal setup, consider adding assertions to verify:
- That the component subscribes to
competencyChanged- That when
competencyChangedemits,loadCourseCompetenciesis called with the correctcourseIdExample enhancement:
it('should open agent chat modal, subscribe to competencyChanged, and refresh competencies', async () => { // Arrange sessionStorageService.store<boolean>('alreadyVisitedCompetencyManagement', true); const competencyChangedCallback = jest.fn(); const modalRef = { componentInstance: { courseId: undefined, competencyChanged: { subscribe: jest.fn((cb) => { competencyChangedCallback.mockImplementation(cb); return { unsubscribe: jest.fn() }; }), }, }, } as any; const openModalSpy = jest.spyOn(modalService, 'open').mockReturnValue(modalRef); const loadCompetenciesSpy = jest.spyOn(component as any, 'loadCourseCompetencies'); fixture.detectChanges(); // Act component['openAgentChatModal'](); // Assert modal setup expect(openModalSpy).toHaveBeenCalledWith(AgentChatModalComponent, { size: 'lg', backdrop: true, }); expect(modalRef.componentInstance.courseId).toBe(1); expect(modalRef.componentInstance.competencyChanged.subscribe).toHaveBeenCalled(); // Act: simulate competencyChanged event competencyChangedCallback(); // Assert: verify refresh behavior expect(loadCompetenciesSpy).toHaveBeenCalledWith(1); });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/de/tum/cit/aet/artemis/atlas/service/AgentChatResult.java(1 hunks)src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.spec.ts(4 hunks)src/main/webapp/app/atlas/manage/competency-management/competency-management.component.spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/atlas/manage/competency-management/competency-management.component.spec.tssrc/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.spec.ts
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
Files:
src/main/java/de/tum/cit/aet/artemis/atlas/service/AgentChatResult.java
🧠 Learnings (3)
📚 Learning: 2024-10-08T15:35:48.767Z
Learnt from: JohannesStoehr
PR: ls1intum/Artemis#8947
File: src/main/java/de/tum/in/www1/artemis/service/ExerciseService.java:760-762
Timestamp: 2024-10-08T15:35:48.767Z
Learning: Even when the `NotNull` annotation is used, adding a null check in methods is recommended to catch potential issues early and provide clear error messages.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/atlas/service/AgentChatResult.java
📚 Learning: 2025-09-13T13:57:35.790Z
Learnt from: jfr2102
PR: ls1intum/Artemis#11145
File: src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIEventListenerService.java:193-197
Timestamp: 2025-09-13T13:57:35.790Z
Learning: In the LocalCIEventListenerService, NotNull annotations are used on event components like MapEntryRemovedEvent.oldValue() to guarantee non-null values rather than using defensive null checks in the event handlers.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/atlas/service/AgentChatResult.java
📚 Learning: 2025-01-28T17:32:42.886Z
Learnt from: magaupp
PR: ls1intum/Artemis#10204
File: src/main/java/de/tum/cit/aet/artemis/programming/service/localci/scaparser/strategy/sarif/RubocopMessageProcessor.java:13-25
Timestamp: 2025-01-28T17:32:42.886Z
Learning: In Java, parameters without Nullable annotation are implicitly non-null by contract, so adding explicit null checks for such parameters is redundant.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/atlas/service/AgentChatResult.java
🪛 GitHub Check: client-tests
src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.spec.ts
[failure] 212-212:
Type 'undefined' is not assignable to type 'number'.
[failure] 191-191:
Type 'null' is not assignable to type '{ id: number; login: string; }'.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Build and Push Docker Image / Build linux/amd64 Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build linux/arm64 Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: bean-instantiation-check
- GitHub Check: client-compilation
- GitHub Check: client-style
- GitHub Check: server-tests
- GitHub Check: Analyse
🔇 Additional comments (8)
src/main/java/de/tum/cit/aet/artemis/atlas/service/AgentChatResult.java (1)
1-10: LGTM! Clean and well-designed record.The record follows Java best practices with appropriate
@NotNullvalidation, clear JavaDoc, and minimal data. The design aligns with the DTO guidelines for using Java records.src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.spec.ts (7)
1-20: LGTM! Modern testing setup is correctly configured.The imports and mock setup appropriately use Angular's modern testing utilities and include the necessary AccountService mock for sessionId derivation testing.
22-49: LGTM! Test configuration follows Angular best practices.The TestBed configuration correctly uses the modern provider functions and includes all necessary mocks.
51-85: LGTM! Comprehensive test of successful response handling.The test properly verifies the request URL, method, body structure, and response mapping including the new
competenciesModifiedfield.
87-133: LGTM! Thorough error handling test coverage.The tests properly verify both HTTP errors and network failures, ensuring the catchError operator returns structured fallback responses as expected.
135-162: LGTM! Timeout handling is properly tested.The test correctly uses
fakeAsyncandtickto simulate the 30-second timeout and verifies the fallback response behavior.
232-278: LGTM! HTTP request details are thoroughly verified.The tests properly validate the request URL, method, and body structure including the derived sessionId format.
280-331: LGTM! Error response structure is comprehensively tested.The tests properly verify all fields in the error response object, including the sessionId, timestamp, and translated error message.
src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.spec.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/atlas/manage/competency-management/competency-management.component.spec.ts
Show resolved
Hide resolved
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
src/main/java/de/tum/cit/aet/artemis/atlas/config/AtlasAgentToolConfig.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/atlas/config/AtlasAgentToolConfig.java
Outdated
Show resolved
Hide resolved
| boolean success | ||
| boolean success, | ||
|
|
||
| boolean competenciesModified |
There was a problem hiding this comment.
Usually we dont indent DTOs like that. I dont know why the linter does not do it automatically here. Can we stay consistent? Have a look at CalendarEventDTO for example. We usually just write all properties in one line without newlines.
There was a problem hiding this comment.
resolved in both DTOs now
src/main/java/de/tum/cit/aet/artemis/atlas/dto/ChatHistoryMessageDTO.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.java
Outdated
Show resolved
Hide resolved
src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat-modal.component.spec.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat-modal.component.spec.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.spec.ts
Outdated
Show resolved
Hide resolved
| private accountService = inject(AccountService); | ||
|
|
||
| sendMessage(message: string, courseId: number): Observable<AgentChatResponse> { | ||
| const userId = this.accountService.userIdentity?.id ?? 0; |
There was a problem hiding this comment.
this was temporary and forgot to refactor it. I throw an error now as the agent needs authentication to be accessed.
| const fallbackMessage = this.translateService.instant('artemisApp.agent.chat.error'); | ||
| return of(fallbackMessage); | ||
| // Return error response on failure | ||
| return of({ |
There was a problem hiding this comment.
I suppose this is an intended dummy implementation that will be implemented later?
There was a problem hiding this comment.
exactly. its a placeholder for future conversation memory implementation.
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
helios-aet
bot
temporarily deployed
to
artemis-test3.artemis.cit.tum.de
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.java (1)
61-73: Hard-coded competenciesModified=false in error paths loses tool modifications.If a tool successfully creates or modifies a competency but the chat response times out or fails afterward, these error handlers return
competenciesModified: false, preventing the client from refreshing the competency list. This leaves the UI showing stale data even though the backend state changed.To fix this, capture the competenciesModified state from
AtlasAgentToolsServicebefore returning error responses:catch (TimeoutException te) { + boolean competenciesModified = atlasAgentToolsService != null && atlasAgentToolsService.wasCompetencyCreated(); return ResponseEntity.status(HttpStatus.GATEWAY_TIMEOUT) - .body(new AtlasAgentChatResponseDTO("The agent timed out. Please try again.", request.sessionId(), ZonedDateTime.now(), false, false)); + .body(new AtlasAgentChatResponseDTO("The agent timed out. Please try again.", request.sessionId(), ZonedDateTime.now(), false, competenciesModified)); } catch (InterruptedException ie) { Thread.currentThread().interrupt(); + boolean competenciesModified = atlasAgentToolsService != null && atlasAgentToolsService.wasCompetencyCreated(); return ResponseEntity.status(HttpStatus.SERVICE_UNAVAILABLE) - .body(new AtlasAgentChatResponseDTO("The request was interrupted. Please try again.", request.sessionId(), ZonedDateTime.now(), false, false)); + .body(new AtlasAgentChatResponseDTO("The request was interrupted. Please try again.", request.sessionId(), ZonedDateTime.now(), false, competenciesModified)); } catch (ExecutionException ee) { + boolean competenciesModified = atlasAgentToolsService != null && atlasAgentToolsService.wasCompetencyCreated(); return ResponseEntity.status(HttpStatus.BAD_GATEWAY) - .body(new AtlasAgentChatResponseDTO("Upstream error while processing your request.", request.sessionId(), ZonedDateTime.now(), false, false)); + .body(new AtlasAgentChatResponseDTO("Upstream error while processing your request.", request.sessionId(), ZonedDateTime.now(), false, competenciesModified)); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/main/java/de/tum/cit/aet/artemis/atlas/config/AtlasAgentToolConfig.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/dto/AtlasAgentChatResponseDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.java(3 hunks)src/main/resources/prompts/atlas/agent_system_prompt.st(1 hunks)src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat-modal.component.spec.ts(4 hunks)src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.spec.ts(3 hunks)src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.ts(3 hunks)src/main/webapp/app/atlas/manage/competency-management/competency-management.component.spec.ts(2 hunks)src/test/java/de/tum/cit/aet/artemis/atlas/agent/AtlasAgentIntegrationTest.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentServiceTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/de/tum/cit/aet/artemis/atlas/dto/AtlasAgentChatResponseDTO.java
- src/test/java/de/tum/cit/aet/artemis/atlas/agent/AtlasAgentIntegrationTest.java
🧰 Additional context used
📓 Path-based instructions (3)
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/atlas/manage/competency-management/competency-management.component.spec.tssrc/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat-modal.component.spec.tssrc/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.spec.tssrc/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.ts
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
Files:
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.javasrc/main/java/de/tum/cit/aet/artemis/atlas/config/AtlasAgentToolConfig.javasrc/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.javasrc/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.java
src/test/java/**/*.java
⚙️ CodeRabbit configuration file
test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true
Files:
src/test/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentServiceTest.java
🧬 Code graph analysis (2)
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java (2)
src/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.java (1)
Conditional(31-75)src/main/java/de/tum/cit/aet/artemis/atlas/config/AtlasEnabled.java (1)
AtlasEnabled(13-25)
src/main/java/de/tum/cit/aet/artemis/atlas/config/AtlasAgentToolConfig.java (2)
src/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.java (1)
Conditional(31-75)src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.java (1)
Lazy(22-107)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Build and Push Docker Image / Build linux/arm64 Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build linux/amd64 Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: client-style
- GitHub Check: client-tests-selected
- GitHub Check: server-tests
- GitHub Check: server-style
- GitHub Check: client-tests
- GitHub Check: bean-instantiation-check
- GitHub Check: Analyse
🔇 Additional comments (27)
src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.ts (2)
29-35: LGTM! Authentication check properly addresses previous concern.The authentication check now throws an error when userId is missing, preventing the fallback to userId=0 that would have caused session sharing across users. The sessionId derivation is correct and consistent.
42-53: LGTM! Error handling is comprehensive.The error handling properly returns a structured AgentChatResponse object with all required fields including sessionId, timestamp, success flag, and competenciesModified. The 30-second timeout aligns with the backend configuration.
src/main/webapp/app/atlas/manage/competency-management/competency-management.component.spec.ts (1)
241-261: LGTM! Past review feedback addressed.The test now explicitly imports and asserts against
AgentChatModalComponent(line 256), addressing the previous review comment about usingexpect.anything(). The test properly validates the modal configuration and courseId assignment.src/main/java/de/tum/cit/aet/artemis/atlas/config/AtlasAgentToolConfig.java (1)
12-28: LGTM! Clean Spring AI tool integration configuration.The configuration properly registers the tool callback provider using
MethodToolCallbackProvider, enabling Spring AI's tool-calling system to discover@Tool-annotated methods fromAtlasAgentToolsService. The JavaDoc clearly explains the purpose, and the conditional loading ensures proper feature gating.src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat-modal.component.spec.ts (6)
118-120: Test appropriately simplified for new architecture.The test now focuses on message presence rather than sessionId generation, which is correct since sessionId is now internally managed by the service. The service has its own dedicated tests for sessionId derivation.
222-242: LGTM! Test updated for new sendMessage signature.The test correctly calls
sendMessage(message, courseId)without the sessionId parameter, and the mock response includes all required fields includingcompetenciesModified. The assertion validates the full interaction flow.
446-491: LGTM! Comprehensive competency modification event tests.The new test suite properly validates that
competencyChangedevents are emitted only whencompetenciesModifiedis true, with both positive and negative test cases. This ensures the parent component will be notified to refresh competency lists when changes occur.
527-560: LGTM! Thorough computed signal tests.The tests comprehensively validate the computed signals for message length tracking and validation. The reactive nature of signals is properly tested by verifying updates when the underlying message changes.
562-630: LGTM! Complete message state management coverage.The tests thoroughly validate the message lifecycle including clearing input after sending, typing state transitions, and message array management. This ensures the UI state remains consistent throughout the chat interaction.
632-705: LGTM! Robust edge case coverage.The tests properly handle edge cases including null containers, empty/null responses, and scroll state management. The fallback to translated error messages when responses are empty/null ensures a good user experience.
src/test/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentServiceTest.java (2)
36-41: LGTM! Clean test setup for new service signature.The test setup properly initializes the service with null values for optional dependencies (ToolCallbackProvider, ChatMemory, AtlasAgentToolsService), maintaining proper test isolation for unit testing the core chat functionality.
43-169: LGTM! Tests properly updated for AgentChatResult.All test methods correctly call
processChatMessagewith the sessionId parameter and validate bothmessage()andcompetenciesModified()fields on the result. The new conversation isolation test effectively validates that different session IDs maintain separate contexts.src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.spec.ts (5)
18-35: LGTM! Modern Angular test setup with proper mocking.The test setup properly mocks AccountService with a flexible type that allows testing null/undefined scenarios, and uses the modern
provideHttpClientpattern instead of the deprecatedHttpClientTestingModule.
61-82: LGTM! Comprehensive request/response validation.The test validates the complete request flow including correct URL, HTTP method, request body structure with sessionId, and response structure with all required fields including
competenciesModified.
126-151: LGTM! Proper timeout handling test.The test correctly uses
fakeAsyncandtickto simulate a timeout scenario, validating that the catchError operator produces a properly structured fallback response withsuccess: falseandcompetenciesModified: false.
153-186: LGTM! Comprehensive sessionId generation tests.The tests properly validate sessionId generation with valid user IDs and correctly verify that authentication errors are thrown when userIdentity is null or userId is undefined, preventing unauthorized access.
230-275: LGTM! Thorough error response validation.The tests comprehensively validate that error responses include all required fields (message, sessionId, timestamp, success, competenciesModified), ensuring the client can properly handle error scenarios and maintain session context.
src/main/resources/prompts/atlas/agent_system_prompt.st (3)
17-33: LGTM! Clear competency creation workflow.The prompt provides clear instructions for immediate competency creation when all details are present, eliminating unnecessary confirmation steps. The step-by-step data collection for missing details ensures a good user experience while maintaining the streamlined workflow.
59-64: LGTM! Appropriate abstraction of technical details.The prompt correctly instructs the agent to hide technical implementation details (function calls, APIs, backend operations) from users, maintaining a professional and user-friendly interaction. The emphasis on plain language for error scenarios ensures accessibility.
81-86: LGTM! Strong safety and privacy guidelines.The prompt includes appropriate safety constraints preventing data invention, exposure of system internals, and ensuring pedagogically focused responses. These guidelines are essential for maintaining trust and security in an educational context.
src/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.java (1)
55-59: LGTM! Success path correctly handles structured result.The code properly awaits the
CompletableFuture<AgentChatResult>, extracts bothmessage()andcompetenciesModified()fields, and maps them to the response DTO. The timeout aligns with the configured value.src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java (4)
32-35: LGTM! Request-scoped design correctly tracks per-request state.Using
@RequestScopeensures each HTTP request gets its own instance of this service, allowing thecompetencyCreatedflag to track modifications within a single request lifecycle. This aligns well with the architecture whereAtlasAgentServicecheckswasCompetencyCreated()after tool execution.
63-86: LGTM! Clean implementation with proper validation and error handling.The method validates course existence before querying competencies and returns structured JSON responses for both success and error cases. Using
LinkedHashMapmaintains field order in the JSON output, which can improve readability for the LLM.
155-158: LGTM! Clean and concise implementation.Using
Optional.mapandorElseprovides a clean fallback for missing courses. Returning an empty string (rather than error JSON) is appropriate here since a missing description is not an error condition for the tool.
166-191: LGTM! Proper validation and comprehensive exercise data.The method validates course existence and returns structured exercise data including type, points, and dates. Using
getClass().getSimpleName()for the exercise type provides a human-readable class name that helps the LLM distinguish exercise types.src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.java (2)
37-45: LGTM! Proper constructor injection with optional dependencies.Marking the new dependencies as
@Autowired(required = false)allows the service to function in degraded mode when Atlas tools or chat memory are unavailable. This follows Spring best practices for optional feature dependencies.
56-92: LGTM! Clean integration of tool calling and chat memory.The method correctly:
- Conditionally wires
MessageChatMemoryAdvisorusingsessionIdas the conversation key for per-session memory (lines 69-71)- Attaches tool callbacks only when available (lines 74-76)
- Detects competency modifications via
atlasAgentToolsService.wasCompetencyCreated()(line 82), leveraging request-scoped state trackingThe approach is deterministic and avoids brittle keyword-based detection mentioned in past review comments. Since
AtlasAgentToolsServiceis request-scoped and this service holds a reference to it, thecompetencyCreatedflag correctly reflects modifications within the current HTTP request lifecycle.
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.spec.ts (1)
18-20: Reset mock state inbeforeEachfor better test isolation.The
mockAccountServiceis defined at module level and mutated by individual tests (lines 158, 176, 182, 193, 210, 236). ThebeforeEachhook clears Jest mocks but doesn't resetmockAccountService.userIdentity. This creates test interdependencies—particularly, the test at lines 257-275 relies on state set by the previous test rather than explicitly defining its expected state.Apply this diff to reset the mock in
beforeEach:beforeEach(() => { TestBed.configureTestingModule({ providers: [ provideHttpClient(), provideHttpClientTesting(), AgentChatService, { provide: TranslateService, useValue: mockTranslateService, }, { provide: AccountService, useValue: mockAccountService, }, ], }); service = TestBed.inject(AgentChatService); httpMock = TestBed.inject(HttpTestingController); translateService = TestBed.inject(TranslateService) as jest.Mocked<TranslateService>; // Reset mocks + mockAccountService.userIdentity = { id: 42, login: 'testuser' }; jest.clearAllMocks(); });Also applies to: 43-45
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java(1 hunks)src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.spec.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java
🧰 Additional context used
📓 Path-based instructions (1)
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Build and Push Docker Image / Build linux/arm64 Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build linux/amd64 Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: bean-instantiation-check
- GitHub Check: server-tests
- GitHub Check: client-style
- GitHub Check: client-tests
- GitHub Check: server-style
- GitHub Check: client-tests-selected
- GitHub Check: Analyse
🔇 Additional comments (1)
src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat.service.spec.ts (1)
175-185: No issues found. Tests correctly validate synchronous authentication error handling.The verification confirms that
AgentChatService.sendMessageperforms authentication checks synchronously before constructing the HTTP observable. The implementation extractsuserIdfromuserIdentity?.id, checksif (!userId), and throws immediately with the expected error message. Both test cases correctly expect this synchronous throw behavior:
- When
userIdentityisnull, optional chaining returnsundefined, triggering the throw- When
userIdentity.idisundefined, the condition directly triggers the throwThe tests are appropriate and will pass reliably.
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat-modal.component.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat-modal.component.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Analyse
- GitHub Check: Build and Push Docker Image / Build linux/amd64 Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build linux/arm64 Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: client-style
- GitHub Check: server-tests
- GitHub Check: client-tests
- GitHub Check: server-style
- GitHub Check: bean-instantiation-check
🔇 Additional comments (2)
src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat-modal.component.ts (2)
1-14: LGTM: Imports are correctly structured.The addition of
outputfrom Angular core aligns with the modern signal-based API for component outputs. All imports are properly organized and necessary for the component's functionality.
53-54: LGTM: Event emitter follows modern Angular patterns.The use of
output<void>()correctly implements the signal-based output API, addressing the previous feedback about migrating from decorators to signals. The naming and documentation are clear and follow the project's coding guidelines.
src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat-modal.component.ts
Show resolved
Hide resolved
src/main/webapp/app/atlas/manage/agent-chat-modal/agent-chat-modal.component.ts
Show resolved
Hide resolved
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java (1)
193-200: Add logging for serialization failures.The
toJsonmethod silently swallowsJsonProcessingExceptionwithout logging, making debugging difficult if serialization fails. While the fallback error JSON is appropriate for the LLM, developers should be alerted to serialization issues.Add a log statement:
private String toJson(Object object) { try { return objectMapper.writeValueAsString(object); } catch (JsonProcessingException e) { + log.warn("Failed to serialize object to JSON: {}", e.getMessage()); return "{\"error\": \"Failed to serialize response\"}"; } }You'll need to add the SLF4J logger field:
private static final Logger log = LoggerFactory.getLogger(AtlasAgentToolsService.class);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
Files:
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java
🧬 Code graph analysis (1)
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java (4)
src/main/java/de/tum/cit/aet/artemis/atlas/web/AtlasAgentResource.java (1)
Conditional(31-75)src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.java (1)
Lazy(22-107)src/main/java/de/tum/cit/aet/artemis/atlas/config/AtlasAgentToolConfig.java (1)
Lazy(12-29)src/main/java/de/tum/cit/aet/artemis/atlas/config/AtlasEnabled.java (1)
AtlasEnabled(13-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Build and Push Docker Image / Build linux/arm64 Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build linux/amd64 Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: client-style
- GitHub Check: client-tests
- GitHub Check: client-tests-selected
- GitHub Check: server-tests
- GitHub Check: server-style
- GitHub Check: Analyse
- GitHub Check: bean-instantiation-check
🔇 Additional comments (1)
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java (1)
27-53: LGTM! Well-structured request-scoped service.The class structure follows best practices:
@RequestScopecorrectly ensures per-request tool-call tracking, constructor injection is used throughout, and thecompetencyCreatedflag mechanism provides clean state management for detecting modifications during the request lifecycle.
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java
Show resolved
Hide resolved
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
There was a problem hiding this comment.
Tested on: https://artemis-test3.artemis.cit.tum.de/courses/147/competencies
- Agent can create competencies → works (auto-refresh OK)
- Listing competencies → works, new competency included
- Course description & exercises → works
- Chat memory persists per course → works
- UI clean, no console errors
- Tested with student account → permissions correctly blocked
5 participants
Checklist
General
Server
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Motivation and Context
The current Atlas Agent only exchanges plain messages with OpenAI:
Description
Tool Calling Integration: Implemented Spring AI's @tool annotation system to allow the agent to interact with the database. Created four tools that enable the agent to:
=>All tools use MethodToolCallbackProvider for automatic discovery and registration
-Competency Modification Detection: Added logic to detect when the agent creates or modifies competencies by parsing agent response . Returns a competenciesModified flag to the frontend so the UI can refresh the competency list automatically.
-Improved System Prompt: Updated agent instructions to prevent displaying technical implementation details (function calls, JSON payloads) to non-technical users, while maintaining all backend functionality.
-Competency Change Events: Emits competencyChanged event when the agent creates or modifies competencies (based on the "competenciesModified" flag), allowing parent components to refresh their competency lists automatically without manual reload.
Steps for Testing
Prerequisites:
Testserver States
You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.
Review Progress
Performance Review
Code Review
Manual Tests
Exam Mode Test
Performance Tests
Test Coverage
Screenshots
Summary by CodeRabbit
New Features
Tests