Adaptive learning: Replace string-based delegation markers with Spring AI tool calls#12373
Adaptive learning: Replace string-based delegation markers with Spring AI tool calls#12373
Adaptive learning: Replace string-based delegation markers with Spring AI tool calls#12373Conversation
jaylann
added
enhancement
code quality
refactoring
atlas
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d91384e33
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughReplaces marker-based delegation and embedded-preview plumbing with explicit tool-based delegation and a centralized delegation flow: adds delegation and tool-callback services, per-request ThreadLocal context and new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AtlasAgent as AtlasAgentService
participant Tools as AtlasAgentToolsService
participant ToolFactory as AtlasAgentToolCallbackService
participant Delegation as AtlasAgentDelegationService
participant SubAgent as Sub-Agent Model
participant Cache as AtlasAgentSessionCacheService
User->>AtlasAgent: processChatMessage(input)
AtlasAgent->>Tools: set ThreadLocal courseId/sessionId
AtlasAgent->>Delegation: delegateToAgent(systemPromptPath, userMsg, courseId, sessionId, saveToMemory?, provider?)
Delegation->>ToolFactory: request ToolCallbackProvider
ToolFactory-->>Delegation: ToolCallbackProvider
Delegation->>SubAgent: invoke chat model (system + user + tools)
SubAgent-->>Delegation: response (may call tools)
SubAgent->>Tools: invoke tool (e.g., delegateToCompetencyExpert(...))
Tools->>Tools: read ThreadLocal courseId/sessionId
Tools->>Delegation: delegateToAgent(subPromptPath, brief, courseId, sessionId, false, subProvider)
Delegation->>SubAgent: run sub-agent prompt
SubAgent-->>Delegation: sub-agent response
Delegation-->>Tools: delegated result
Delegation-->>AtlasAgent: final response
AtlasAgent->>Cache: store/read previews by assistant message index
AtlasAgent->>Tools: clear ThreadLocal courseId/sessionId
AtlasAgent-->>User: final response (previews populated from cache)
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/test/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsServiceTest.java (1)
102-130: Consider adding test fordelegateToCompetencyMapperbrief formatting.The
DelegationBriefFormattingnested class tests brief construction fordelegateToCompetencyExpertanddelegateToExerciseMapper, butdelegateToCompetencyMapperis missing. While it uses the sameformatBriefhelper, having explicit coverage ensures the correct agent type and parameters are passed.📝 Suggested test addition
`@Test` void shouldFormatCompetencyMapperBriefCorrectly() { when(atlasAgentService.delegateToAgent(any(), anyString(), anyLong(), anyString(), anyBoolean())).thenReturn("ok"); toolsService.delegateToCompetencyMapper("A extends B", "Create EXTENDS relation", "None", "A builds on B"); verify(atlasAgentService).delegateToAgent(eq(AtlasAgentService.AgentType.COMPETENCY_MAPPER), eq("TOPIC: A extends B\nREQUIREMENTS: Create EXTENDS relation\nCONSTRAINTS: None\nCONTEXT: A builds on B"), eq(42L), eq("test-session"), eq(false)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsServiceTest.java` around lines 102 - 130, Add a missing unit test to DelegationBriefFormatting that verifies toolsService.delegateToCompetencyMapper builds and forwards the correct brief and agent type; specifically, mock atlasAgentService.delegateToAgent to return "ok", call toolsService.delegateToCompetencyMapper(...) with sample inputs, and verify atlasAgentService.delegateToAgent was called with AtlasAgentService.AgentType.COMPETENCY_MAPPER, the expected formatted brief string (matching formatBrief output), the current course/session via AtlasAgentToolsService.getCurrentCourseId()/getCurrentSessionId() (set in setThreadLocals), and false for the final boolean.src/main/resources/prompts/atlas/agent_system_prompt.st (1)
106-109: Clarify instruction on exclusive tool call vs text output.Lines 108-109 instruct the agent to "call the tool directly - do NOT output any additional text" and "NEVER output both a plan description AND a tool call together". This is important for consistent behavior, but Spring AI's tool calling typically allows the model to provide both text and tool calls in the same turn. Verify that your downstream processing correctly handles or strips any accompanying text when tool calls are present.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/prompts/atlas/agent_system_prompt.st` around lines 106 - 109, The prompt rule for delegateToCompetencyExpert forbids any accompanying text when calling the tool, but downstream Spring AI tooling can return both text and tool calls; update either the prompt or runtime: (A) clarify or relax the prompt text around delegateToCompetencyExpert to allow accompanying explanatory text if your runtime supports it, or (B) enforce the existing rule by changing the agent runtime that handles deployToolCalls to strip/ignore any non-tool text when a delegateToCompetencyExpert tool call is present; locate the delegateToCompetencyExpert guidance in the agent_system_prompt (symbol: delegateToCompetencyExpert) and adjust the prompt wording or the tool-calling handler (the component that processes tool calls) accordingly so behavior is deterministic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/main/resources/prompts/atlas/agent_system_prompt.st`:
- Around line 106-109: The prompt rule for delegateToCompetencyExpert forbids
any accompanying text when calling the tool, but downstream Spring AI tooling
can return both text and tool calls; update either the prompt or runtime: (A)
clarify or relax the prompt text around delegateToCompetencyExpert to allow
accompanying explanatory text if your runtime supports it, or (B) enforce the
existing rule by changing the agent runtime that handles deployToolCalls to
strip/ignore any non-tool text when a delegateToCompetencyExpert tool call is
present; locate the delegateToCompetencyExpert guidance in the
agent_system_prompt (symbol: delegateToCompetencyExpert) and adjust the prompt
wording or the tool-calling handler (the component that processes tool calls)
accordingly so behavior is deterministic.
In
`@src/test/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsServiceTest.java`:
- Around line 102-130: Add a missing unit test to DelegationBriefFormatting that
verifies toolsService.delegateToCompetencyMapper builds and forwards the correct
brief and agent type; specifically, mock atlasAgentService.delegateToAgent to
return "ok", call toolsService.delegateToCompetencyMapper(...) with sample
inputs, and verify atlasAgentService.delegateToAgent was called with
AtlasAgentService.AgentType.COMPETENCY_MAPPER, the expected formatted brief
string (matching formatBrief output), the current course/session via
AtlasAgentToolsService.getCurrentCourseId()/getCurrentSessionId() (set in
setThreadLocals), and false for the final boolean.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 56d25dfb-2b70-4426-8393-df51d722a427
📒 Files selected for processing (6)
src/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.javasrc/main/resources/prompts/atlas/agent_system_prompt.stsrc/test/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentServiceTest.javasrc/test/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsServiceTest.java
github-actions
bot
added
tests
server
|
@jaylann Test coverage could not be fully measured because some tests failed. Please check the workflow logs for details. |
End-to-End Test Results
Test Strategy: Running all tests (configuration or infrastructure changes detected) Overall: ❌ E2E tests failed 🔗 Workflow Run · 📊 Test Report |
…cture test violations
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentDelegationService.java (1)
63-70: Potential null return fromgetSubAgentToolCallbackProvider.The method can return
nullif aToolCallbackProviderwasn't configured. WhiledelegateToAgenthandles nulltoolCallbackProvidergracefully (line 112-114), callers should be aware this can happen when the corresponding tools service isn't available.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentDelegationService.java` around lines 63 - 70, The getSubAgentToolCallbackProvider method can return null when a ToolCallbackProvider wasn't configured; change its signature to return Optional<ToolCallbackProvider> and return Optional.ofNullable(...) for competencyExpertToolCallbackProvider, competencyMapperToolCallbackProvider and exerciseMapperToolCallbackProvider (keep the IllegalArgumentException for MAIN_AGENT). Update callers (notably delegateToAgent) to accept Optional by calling isPresent()/orElseThrow()/ifPresent() or map() as appropriate so null is no longer propagated silently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentDelegationService.java`:
- Around line 97-117: The method delegateToAgent currently returns the result of
promptSpec.call().content() which is annotated `@Nullable`; add explicit null
handling by capturing promptSpec.call().content() into a local variable, check
for null, and handle it (e.g., throw an IllegalStateException with context
including sessionId/courseId and the promptResourcePath or alternatively return
null and annotate the delegateToAgent method with `@Nullable`); update callers if
you choose the `@Nullable` route. Ensure you reference delegateToAgent and the
promptSpec.call().content() call when making the change so the null
check/annotation is applied to the correct return path.
---
Nitpick comments:
In
`@src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentDelegationService.java`:
- Around line 63-70: The getSubAgentToolCallbackProvider method can return null
when a ToolCallbackProvider wasn't configured; change its signature to return
Optional<ToolCallbackProvider> and return Optional.ofNullable(...) for
competencyExpertToolCallbackProvider, competencyMapperToolCallbackProvider and
exerciseMapperToolCallbackProvider (keep the IllegalArgumentException for
MAIN_AGENT). Update callers (notably delegateToAgent) to accept Optional by
calling isPresent()/orElseThrow()/ifPresent() or map() as appropriate so null is
no longer propagated silently.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 79ab3a26-ee21-4b1d-8d87-bead4b9c2fe0
📒 Files selected for processing (5)
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentDelegationService.javasrc/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.javasrc/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.javasrc/test/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentServiceTest.javasrc/test/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsServiceTest.java
✅ Files skipped from review due to trivial changes (1)
- src/test/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsServiceTest.java
github-project-automation
bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
|
@jaylann Test coverage has been automatically updated in the PR description. |
…cture test violations
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.java (1)
144-145: Comment is slightly misleading — tool calls happen during, not after, this invocation.The comment says "delegation to sub-agents happens via tool calls inside this invocation" but then the code calls
delegateToAgent(AgentType.MAIN_AGENT, ...). The tool-based delegation occurs during the AI model's execution when it decides to call delegation tools, not explicitly here. Consider clarifying that the main agent may internally invoke delegation tools during its response generation.✏️ Suggested comment clarification
- // Single main agent call — delegation to sub-agents happens via tool calls inside this invocation + // Call main agent — it may invoke delegation tools (delegateToCompetencyExpert, etc.) during response generation String response = delegateToAgent(AgentType.MAIN_AGENT, message, courseId, sessionId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.java` around lines 144 - 145, Update the misleading comment above the delegateToAgent call: clarify that calling delegateToAgent(AgentType.MAIN_AGENT, message, courseId, sessionId) invokes the main agent, and that any delegation to sub-agents happens internally during the main agent's execution via tool calls invoked by the model (not as a separate call after delegateToAgent returns); reference delegateToAgent and AgentType.MAIN_AGENT in the comment so readers know the main agent may trigger tool-based delegation while generating its response.src/test/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentServiceTest.java (1)
158-160: Consider extracting repeated deployment configuration to constants.The values
"gpt-4o"and0.2are duplicated across multiple test instantiations ofAtlasAgentDelegationService. Extracting these to constants would reduce duplication and make it easier to update if these values change.♻️ Suggested refactor
Add constants at the class level:
private static final String TEST_DEPLOYMENT_NAME = "gpt-4o"; private static final double TEST_TEMPERATURE = 0.2;Then replace usages:
-new AtlasAgentDelegationService(ChatClient.create(chatModel), templateService, null, "gpt-4o", 0.2) +new AtlasAgentDelegationService(ChatClient.create(chatModel), templateService, null, TEST_DEPLOYMENT_NAME, TEST_TEMPERATURE)Also applies to: 261-263, 643-644, 664-665, 671-673
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentServiceTest.java` around lines 158 - 160, Several tests repeatedly inline the deployment name "gpt-4o" and temperature 0.2 when constructing AtlasAgentDelegationService instances (e.g., in the AtlasAgentService test creating AtlasAgentService and AtlasAgentDelegationService); extract these literals into class-level constants (e.g., private static final String TEST_DEPLOYMENT_NAME = "gpt-4o"; private static final double TEST_TEMPERATURE = 0.2) and replace all occurrences used in AtlasAgentDelegationService constructor calls (and any other test instantiations) with TEST_DEPLOYMENT_NAME and TEST_TEMPERATURE to remove duplication and centralize configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentDelegationService.java`:
- Around line 60-81: The method delegateToAgent calls chatClient.mutate()
without verifying chatClient isn't null; add a null-safety check at the start of
delegateToAgent (or before calling chatClient.mutate()) to handle the nullable
ChatClient: if chatClient is null, either throw a clear IllegalStateException
(or return an appropriate error) with a descriptive message like "ChatClient not
configured" so callers see the problem, otherwise proceed to call
chatClient.mutate() and build sessionClient as before.
---
Nitpick comments:
In `@src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.java`:
- Around line 144-145: Update the misleading comment above the delegateToAgent
call: clarify that calling delegateToAgent(AgentType.MAIN_AGENT, message,
courseId, sessionId) invokes the main agent, and that any delegation to
sub-agents happens internally during the main agent's execution via tool calls
invoked by the model (not as a separate call after delegateToAgent returns);
reference delegateToAgent and AgentType.MAIN_AGENT in the comment so readers
know the main agent may trigger tool-based delegation while generating its
response.
In
`@src/test/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentServiceTest.java`:
- Around line 158-160: Several tests repeatedly inline the deployment name
"gpt-4o" and temperature 0.2 when constructing AtlasAgentDelegationService
instances (e.g., in the AtlasAgentService test creating AtlasAgentService and
AtlasAgentDelegationService); extract these literals into class-level constants
(e.g., private static final String TEST_DEPLOYMENT_NAME = "gpt-4o"; private
static final double TEST_TEMPERATURE = 0.2) and replace all occurrences used in
AtlasAgentDelegationService constructor calls (and any other test
instantiations) with TEST_DEPLOYMENT_NAME and TEST_TEMPERATURE to remove
duplication and centralize configuration.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b3898216-7491-4b8b-9a9f-43f654769400
📒 Files selected for processing (5)
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentDelegationService.javasrc/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.javasrc/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.javasrc/test/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentServiceTest.javasrc/test/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsServiceTest.java
✅ Files skipped from review due to trivial changes (1)
- src/test/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentToolsService.java
github-project-automation
bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
github-actions
bot
added
the
config-change
Claudia-Anthropica
left a comment
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@jaylann Clean refactoring — replacing the fragile marker-based delegation with Spring AI tool calls and moving preview persistence to Hazelcast cache is a solid architectural improvement. One config issue inline: the temperature property key doesn't match the YAML, so the config value is silently ignored.
|
@jaylann Test coverage has been automatically updated in the PR description. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentDelegationService.java`:
- Around line 70-72: The code configures MessageChatMemoryAdvisor when
saveToMemory is true without validating sessionId, which can merge distinct
conversations; update the block that calls clientBuilder.defaultAdvisors(...) to
first check that sessionId is non-null and non-blank (e.g. use sessionId != null
&& !sessionId.isBlank() or a StringUtils.hasText check) and only build and set
MessageChatMemoryAdvisor.builder(chatMemory).conversationId(sessionId).build()
when the sessionId passes that validation; if invalid, skip configuring the
advisor (and optionally log a warning).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ae1250fc-62e6-41be-ae1c-39ff08963a14
⛔ Files ignored due to path filters (1)
src/main/resources/config/application-artemis.ymlis excluded by!**/*.yml
📒 Files selected for processing (3)
src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentDelegationService.javasrc/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.javasrc/main/resources/prompts/atlas/exercise_mapper_system_prompt.st
✅ Files skipped from review due to trivial changes (1)
- src/main/resources/prompts/atlas/exercise_mapper_system_prompt.st
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/de/tum/cit/aet/artemis/atlas/service/AtlasAgentService.java
Claudia-Anthropica
left a comment
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@jaylann Clean refactoring — replacing the fragile marker-based delegation with Spring AI tool calls and moving preview persistence to Hazelcast cache is a solid architectural improvement. All previous feedback addressed (temperature config fix, spotless formatting, provider caching, sub-agent prompt cleanup, exerciseId passthrough). Nice work.
|
@jaylann Test coverage has been automatically updated in the PR description. |
|
@jaylann Test coverage has been automatically updated in the PR description. |
MarkusPaulsen
left a comment
MarkusPaulsen
left a comment
There was a problem hiding this comment.
Tested on TS2.
Before entering "Change the taxonomy to UNDERSTAND", I got the competency preview card:
Recursive Problem Solving
Apply recursion to solve algorithmic problems
Identify base and recursive cases
Implement recursive solutions correctly
After entering "Change the taxonomy to UNDERSTAND", I got the competency preview card:
Recursive Problem Solving
Apply recursion to solve algorithmic problems
Identify base and recursive cases
Implement recursive solutions correctly
So there was no textual change.
|
@jaylann Test coverage has been automatically updated in the PR description. |
This is preexisting behavior caused by the prompt. A large prompt refactoring is out of scope for this PR but will be addressed in a followup |
jaylann
dismissed stale reviews from coderabbitai[bot] and Claudia-Anthropica
via
642dab4
|
@jaylann Test coverage has been automatically updated in the PR description. |
Claudia-Anthropica
left a comment
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@jaylann Clean refactoring — the migration from string-based delegation markers to Spring AI tool calls is solid, and the Hazelcast-backed preview cache is well-designed. The per-session locking in storePreviewForMessage properly prevents the read-modify-write race, and the concurrency test is a nice addition. All previous feedback addressed, nice work.
Claudia-Anthropica
left a comment
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@jaylann Clean refactoring — the migration from string-based delegation markers to Spring AI tool calls is well-executed, and the Hazelcast-backed preview cache is a solid replacement for the fragile marker embedding approach. All previous feedback addressed (spotless formatting, provider caching, config property alignment, prompt cleanup, per-session locking, ?? coalescing tests). Nice work.
|
@jaylann Test coverage has been automatically updated in the PR description. |
alretum
left a comment
alretum
left a comment
There was a problem hiding this comment.
Tested locally and everything works as described
louis-heinrich
left a comment
louis-heinrich
left a comment
There was a problem hiding this comment.
Code LGTM. Tested locally and worked as expected.
6 participants
Summary
Replace the string-based delegation markers in the Atlas Agent orchestrator with Spring AI tool-based programmatic routing, and replace marker-based preview data embedding with Hazelcast cache-backed preview history. The main agent now delegates to sub-agents (Competency Expert, Competency Mapper, Exercise Mapper) via
@Toolmethods instead of embedding%%ARTEMIS_DELEGATE_TO_*%%text markers in LLM output. Preview data (competency, relation, exercise mapping) is now stored in a dedicated Hazelcast cache keyed by assistant message index instead of being serialized as%%PREVIEW_DATA_START%%/%%PREVIEW_DATA_END%%JSON markers in chat memory text. This eliminates all fragile string parsing, enables multi-delegation per request, produces cleaner conversation history, and aligns with Spring AI's native tool calling framework.Checklist
General
Server
Motivation and Context
The Atlas Agent's multi-agent orchestration relied on text markers embedded in LLM output for two purposes: (1) delegation routing via
%%ARTEMIS_DELEGATE_TO_*%%markers, and (2) preview data persistence via%%PREVIEW_DATA_START%%/%%PREVIEW_DATA_END%%markers serialized into chat memory.Delegation markers had several problems:
extractBriefFromDelegationMarker()method was sensitive to LLM formatting variations (extra whitespace, missing colons, partial markers).Preview data markers had similar issues:
embed*InResponse()methods serialized DTOs as JSON wrapped in%%PREVIEW_DATA%%markers appended to message text.extractPreviewDataFromMessage()usedindexOf()/substring()parsing to recover the JSON, with type-sniffing viaJsonNode.has()to distinguish competency vs. relation vs. exercise previews.updateChatMemoryWithEmbeddedData()had to clear and re-add all messages to splice embedded data into the last assistant message.getConversationHistoryAsDTO()had to extract and strip markers from every assistant message on every history load.Spring AI 1.1.3 provides native tool calling for delegation, and Hazelcast provides a natural cache layer for preview persistence — both eliminate the need for text-based markers entirely.
Description
AtlasAgentToolsService.java(delegation):@Tooldelegation methods:delegateToCompetencyExpert(),delegateToCompetencyMapper(),delegateToExerciseMapper()courseId/sessionIdfrom ThreadLocal, constructs the brief, callsdelegateToAgent()withsaveToMemory=falseAtlasAgentService.java(main simplification):extractBriefFromDelegationMarker()and 4 marker constantsAtlasAgentSessionCacheServicekeyed by assistant message index instead of being embedded as markers in chat memorygetConversationHistoryAsDTO()reconstructs previews from cache instead of parsing markers from message textdelegateToAgent()with switch expressionsAtlasAgentPreviewService.java(major reduction, -189 lines):embedPreviewDataInResponse(),embedRelationPreviewDataInResponse(),embedExerciseMappingPreviewDataInResponse(),extractPreviewDataFromMessage(),updateChatMemoryWithEmbeddedData()PreviewDataContainer,RelationPreviewDataContainer,ExerciseMappingPreviewDataContainer,PreviewDataResultconvertToRelationPreviewsList(),addAssistantMessageToMemory()AtlasAgentSessionCacheService.java(+69 lines):MessagePreviewDatarecord holding all preview types per messagestorePreviewForMessage(),getPreviewHistory(),clearPreviewHistory()methodsatlas-session-preview-historyAtlasAgentChatResponseDTO.java:@JsonIncludefromNON_EMPTYtoNON_NULL(allows empty string messages while still omitting null fields)@NotBlankfrom message field (tool-only delegation responses may have null message)AtlasAgentDelegationService.java:atlas.chat-model→artemis.atlas.chat-model,atlas.chat-temperature→artemis.atlas.chat-temperatureCompetencyMappingToolsService.java:rel.id() == 0L) alongside null check to handle default long valuesagent-chat-modal.component.ts:||to??for null coalescing on response messages (preserves empty strings)HazelcastConfiguration.java:atlas-session-preview-historymap configagent_system_prompt.st(rewritten):%%ARTEMIS_DELEGATE_TO_*%%:markersAtlasAgentServiceTest.java(updated):AtlasAgentPreviewServiceTest.java(major reduction, -194 lines):AtlasAgentIntegrationTest.java(updated):AtlasAgentSessionCacheService.storePreviewForMessage()instead of embedding markers in chat memoryNo changes to: sub-agent tool services (
CompetencyExpertToolsService,CompetencyMappingToolsService,ExerciseMappingToolsService), sub-agent system prompts, approval flow (CREATE_APPROVED_*markers), plan execution system (ExecutionPlanStateManagerService).Steps for Testing
Prerequisites:
Test competency creation (delegateToCompetencyExpert):
4. Type "Create a competency about recursion"
5. Verify the agent delegates and shows a competency preview card (not a text description)
6. Type "Change the taxonomy to UNDERSTAND"
7. Verify the preview card updates
8. Type "Looks good, create it"
9. Verify the competency is created
Test relation mapping (delegateToCompetencyMapper):
10. Type "Make [Competency A] a prerequisite for [Competency B]" (using actual competency names)
11. Verify the agent delegates and shows a relation preview with graph visualization
12. Approve the relation
Test exercise mapping (delegateToExerciseMapper):
13. Type "Map exercises to competencies"
14. Verify the agent lists exercises and asks for selection
15. Select an exercise by number
16. Verify the exercise-competency mapping preview appears
17. Approve the mapping
Test non-delegation flow:
18. Type "What can you do?"
19. Verify the agent responds conversationally without delegating (no preview cards)
Test conversation history persistence:
20. After completing steps 4-9 (with preview cards visible), reload the page
21. Verify conversation history displays cleanly with preview cards restored from cache
22. Verify no internal markers or briefing text visible in history
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
Code Review
Manual Tests
Test Coverage
Client
Server
Last updated: 2026-04-16 11:17:16 UTC
Screenshots
No UI changes. The delegation mechanism is internal to the server; the frontend receives the same DTO structure (
AtlasAgentChatResponseDTO) with identical fields.Summary by CodeRabbit
Refactor
Bug Fixes
Chores
Tests