-
Notifications
You must be signed in to change notification settings - Fork 118
Add session-aware evaluation logging #54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| package com.alibaba.assistant.agent.evaluation.util; | ||
|
|
||
| import com.alibaba.assistant.agent.evaluation.model.EvaluationContext; | ||
| import com.alibaba.cloud.ai.graph.RunnableConfig; | ||
|
|
||
| /** | ||
| * 评估链路日志上下文提取工具。 | ||
| */ | ||
| public final class EvaluationLogContextHelper { | ||
|
|
||
| private EvaluationLogContextHelper() { | ||
| } | ||
|
|
||
| public static String getSessionId(EvaluationContext context) { | ||
| if (context == null) { | ||
| return null; | ||
| } | ||
| Object sessionId = context.getEnvironmentValue("sessionId"); | ||
| if (sessionId == null) { | ||
| sessionId = context.getEnvironmentValue("threadId"); | ||
| } | ||
| return sessionId != null ? String.valueOf(sessionId) : null; | ||
| } | ||
|
|
||
| public static String getSessionId(RunnableConfig config) { | ||
| if (config == null) { | ||
| return null; | ||
| } | ||
| return config.threadId().orElse(null); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,6 +19,7 @@ | |||||||||||||
| import com.alibaba.assistant.agent.evaluation.model.CriterionExecutionContext; | ||||||||||||||
| import com.alibaba.assistant.agent.evaluation.model.CriterionResult; | ||||||||||||||
| import com.alibaba.assistant.agent.evaluation.model.CriterionStatus; | ||||||||||||||
| import com.alibaba.assistant.agent.evaluation.util.EvaluationLogContextHelper; | ||||||||||||||
| import com.alibaba.assistant.agent.extension.experience.model.Experience; | ||||||||||||||
| import com.alibaba.assistant.agent.extension.experience.model.ExperienceQuery; | ||||||||||||||
| import com.alibaba.assistant.agent.extension.experience.model.ExperienceQueryContext; | ||||||||||||||
|
|
@@ -114,23 +115,29 @@ private static RuleBasedEvaluator createEvaluator( | |||||||||||||
| try { | ||||||||||||||
| // 优先使用 enhanced_user_input，否则使用原始 userInput | ||||||||||||||
| String queryText = extractEnhancedOrOriginalInput(ctx); | ||||||||||||||
| String sessionId = EvaluationLogContextHelper.getSessionId(ctx.getInputContext()); | ||||||||||||||
| log.info("ExperienceRetrievalEvaluatorFactory#{} - reason=开始检索经验, sessionId={}, queryText={}, experienceTypes={}", | ||||||||||||||
| evaluatorId, sessionId, queryText, experienceTypes); | ||||||||||||||
|
Comment on lines
+119
to
+120
|
||||||||||||||
| log.info("ExperienceRetrievalEvaluatorFactory#{} - reason=开始检索经验, sessionId={}, queryText={}, experienceTypes={}", | |
| evaluatorId, sessionId, queryText, experienceTypes); | |
| int queryTextLength = StringUtils.hasLength(queryText) ? queryText.length() : 0; | |
| log.info("ExperienceRetrievalEvaluatorFactory#{} - reason=开始检索经验, sessionId={}, queryTextLength={}, experienceTypes={}", | |
| evaluatorId, sessionId, queryTextLength, experienceTypes); |
Copilot
AI
Apr 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
experiences.stream().map(...).collect(toList()) is computed multiple times (for logging and for metadata). Consider computing experienceIds once and reusing it to avoid duplicate work and ensure the log/metadata always refer to the same list.
Copilot
AI
Apr 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Session ID propagation into ExperienceQueryContext only checks environment.sessionId, but EvaluationLogContextHelper also falls back to environment.threadId. This can cause logs to show a non-null sessionId while downstream experience queries receive a null sessionId. Consider using EvaluationLogContextHelper.getSessionId(executionContext.getInputContext()) (or duplicating the same fallback logic) when setting queryContext.sessionId.
| Object sessionId = executionContext.getInputContext().getEnvironmentValue("sessionId"); | |
| if (sessionId != null) { | |
| queryContext.setSessionId(String.valueOf(sessionId)); | |
| String sessionId = EvaluationLogContextHelper.getSessionId(executionContext.getInputContext()); | |
| if (StringUtils.hasText(sessionId)) { | |
| queryContext.setSessionId(sessionId); |
Copilot
AI
Apr 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New behavior propagates sessionId/tenantId/userId from CriterionExecutionContext into ExperienceQueryContext, but there’s no test asserting that these fields are passed through to ExperienceProvider.query(...). Since this module already has unit tests around ExperienceQueryContext, consider adding a focused test that mocks ExperienceProvider and verifies the constructed ExperienceQueryContext contains the expected IDs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The log message labels the last field as
result={}, but the code passesresult.getValue(). This is confusing when debugging; either log the fullCriterionResult(if safe) or rename the label tovalue={}.