Pass LLM as a judge on the full transcript#1605
Conversation
Signed-off-by: David Gageot <david.gageot@docker.com>
There was a problem hiding this comment.
Code Review Summary
I reviewed the code changes in this PR and found the implementation to be generally well-structured with comprehensive test coverage. The buildTranscript function correctly preserves temporal ordering of events for LLM evaluation.
However, I identified one issue with defensive programming consistency that should be addressed.
| func getToolCallInfo(event map[string]any) (name, args string) { | ||
| tc, _ := event["tool_call"].(map[string]any) | ||
| fn, _ := tc["function"].(map[string]any) | ||
| name, _ = fn["name"].(string) |
There was a problem hiding this comment.
Unsafe type assertions that silently fail on malformed events
The getToolCallInfo function uses unchecked type assertions with the blank identifier _, which means it will silently return empty strings if the event structure is malformed:
tc, _ := event["tool_call"].(map[string]any)
fn, _ := tc["function"].(map[string]any)While Go handles nil map access safely (won't panic), this produces confusing output like [Agent root calls tool "" with arguments: ] when events are malformed, instead of indicating a data problem.
This is inconsistent with the defensive programming style used in parseContainerEvents (lines 481-489) which properly checks assertion results:
if tc, ok := event["tool_call"].(map[string]any); ok {
if fn, ok := tc["function"].(map[string]any); ok {
// ...
}
}Recommendation: Use the comma-ok idiom to check type assertions and either return empty strings explicitly when checks fail, or add a comment explaining that empty strings are the intended behavior for malformed events.
No description provided.