Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
github-actions
Bot
added
test:uipath-langchain
There was a problem hiding this comment.
🟡 Changes recommended
Some conversational content is silently dropped, and attachment-backed output remains unresolved in the newer trajectory evaluator.
2 open issues
What changed in this PR
Adds agents’ final responses to trajectory judge histories, addressing incomplete evaluations for runs with or without tool calls.
Changes:
- Adds shared rendering for conversational and structured output.
- Updates both trajectory evaluators to append final responses.
- Adds regression tests and bumps
uipathto 2.14.42.
| File | Description |
|---|---|
| packages/uipath/uv.lock | Updates the locked package version. |
| packages/uipath/tests/evaluators/test_trajectory_agent_output.py | Tests rendering and evaluator integration. |
| packages/uipath/src/uipath/eval/evaluators/llm_judge_trajectory_evaluator.py | Appends final output to trajectory history. |
| packages/uipath/src/uipath/eval/evaluators/legacy_trajectory_evaluator.py | Appends resolved output and handles empty traces. |
| packages/uipath/src/uipath/eval/_helpers/agent_output.py | Adds shared output-rendering helpers. |
| packages/uipath/pyproject.toml | Bumps the package version. |
Review effort: Balanced
| if isinstance(part.data, UiPathInlineValue) and part.data.inline: | ||
| lines.append(f"[{message.role}] {part.data.inline}") |
There was a problem hiding this comment.
Fixed in 5e50d03. External parts now render as Attachment (<mime>): <uri>, and falsy inline values like false and 0 are kept. Only None and empty strings are skipped. Added a regression test with mixed text, attachment and falsy parts.
🤖 Addressed by Claude Code
| # trace_to_str only renders tool spans; the agent's own answer lives in the output. | ||
| return append_agent_output( | ||
| trace_to_str(workload_execution.workload_trace), | ||
| workload_execution.workload_output, | ||
| ) |
There was a problem hiding this comment.
Fixed in 5e50d03. _get_actual_output now downloads a job-attachment URI before appending it, same idiom as output_evaluator. Test mocks the download.
🤖 Addressed by Claude Code
![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: a1307f4370
ℹ️ 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".
| for call in message.tool_calls: | ||
| lines.append( | ||
| f"[{message.role}] Tool call {call.name}: {_to_json(call.input)}" | ||
| ) |
There was a problem hiding this comment.
When a conversational output includes toolCalls, the same executed calls are already rendered from workload_trace by trace_to_str, so appending them again under Agent Final Response makes the judge see each call twice. This can distort trajectory evaluations involving tool selection, sequencing, or call counts; only the final response content should be appended, or calls already represented by spans should be deduplicated.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed, dropped. toolCalls on the output messages are no longer rendered, the executed calls already come from the spans via trace_to_str.
🤖 Addressed by Claude Code
| for part in message.content_parts: | ||
| if isinstance(part.data, UiPathInlineValue) and part.data.inline: | ||
| lines.append(f"[{message.role}] {part.data.inline}") |
There was a problem hiding this comment.
When an assistant response contains both inline text and an external content part, such as a generated file attachment, this loop emits the text and silently discards the external part because only UiPathInlineValue is handled. Since the nonempty text prevents the JSON fallback, the trajectory judge cannot see that the agent supplied the attachment and may incorrectly fail behavior that requires producing or sharing a file.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Same fix as the Copilot thread on this line, external parts are rendered now in 5e50d03.
🤖 Addressed by Claude Code
… [FOR-1629] trace_to_str renders only tool spans, so zero-tool runs reached the trajectory judge with an empty AgentRunHistory. Append the rendered final output in both trajectory evaluators, with conversational messages rendered as text. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…se [FOR-1629] Render external content parts and falsy inline values, skip toolCalls (already rendered from spans), and resolve job-attachment output in the LLM judge trajectory evaluator. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
a1307f4 to
3cfb670
Compare
🚨 Heads up:
|
|
🚨 Heads up:
|
AAgnihotry
left a comment
AAgnihotry
left a comment
There was a problem hiding this comment.
Code review
Clean fix with solid test coverage (14 new tests). One performance nit below -- not blocking.
Checked for bugs, CLAUDE.md compliance, git history context, prior PR comments, and code comment adherence across 5 parallel review agents + deep review + adversarial review. Slop meter: 2/100 slop, 0/100 tunnel-vision.
| # | Sev | File:Line | Headline |
|---|---|---|---|
| 1 | P1 | llm_judge_trajectory_evaluator.py:87 |
_get_actual_output called twice per eval -- double attachment download |
🤖 Generated with Claude Code
If this review was useful, please react with 👍. Otherwise, react with 👎.
| return trace_to_str(workload_execution.workload_trace) | ||
| output = workload_execution.workload_output | ||
| if isinstance(output, str) and is_job_attachment_uri(output): | ||
| output = download_attachment_as_string(extract_attachment_id(output)) |
There was a problem hiding this comment.
P1 -- Performance: _get_actual_output called twice per evaluation, causing double attachment download
The base class LLMJudgeMixin.evaluate calls _get_actual_output at two sites:
Before this PR, _get_actual_output was pure computation (trace_to_str()). Now it calls download_attachment_as_string -- synchronous HTTP download + temp file write/read. Every trajectory evaluation with an attachment URI will download the file twice and block the event loop twice.
Fix: Override evaluate() in BaseLLMTrajectoryEvaluator to call _get_actual_output once and pass the cached result, or use @functools.lru_cache.
Not blocking since attachment-backed trajectory evals are uncommon, but worth a follow-up.
Trajectory evaluators were judging runs without ever seeing what the agent said.
trace_to_stronly renders spans that carrytool.name, and neither trajectory prompt has a slot for the final output, so a run that answers with no tool calls sends the judge an emptyAgentRunHistory. Follow up to #1901, taking the Part 2 fix and the review feedback from that PR.What the trace shows
Teodor's cloud trace (
f54be95e, Default Trajectory Evaluator) has 8 eval items against the same low-code conversational agent:bed87113,c3f157bbTools_Main_GetUserContext195d3d99,1e8cd149,1aed828d""60aa9c86,245302ad""8b4c11a5So whether a run "works" depends only on one thing: did the agent call a tool. Every zero-tool run got an empty history, and the agent did answer in all of them. The text is right there in
output.uipath__agent_response_messages[].contentParts[].data.inlineon theAgent outputspan, it just never reaches the prompt. Whether the judge then gave it 0 or 100 comes down to how the expected behavior is worded. "Must not disclose X" gets passed on nothing (the 100s are vacuous), "should report X" gets failed. The tool runs are not fully healthy either. Thec3f157bbjustification says "the agent's final response is not shown", so the judge is guessing there too.Why it shows up more in cloud: these are identity-scoped refusal scenarios where the agent answers straight from the prompt without calling
GetUserContext. Nothing environment-specific in the evaluator, the cloud runs just hit the zero-tool path more often.Change
eval/_helpers/agent_output.pywithrender_agent_output/append_agent_output. Conversational output is parsed withUiPathConversationMessageData, so camelCase and snake_case both work, and comes out as[role] textplus tool calls. Any structured fields next to the messages are kept as JSON. Everything else is indented JSON instead ofstr(dict).LLMJudgeTrajectoryEvaluatorandLegacyTrajectoryEvaluatoralways append anAgent Final Response:section after the tool history, not just when the history is empty. Thec3f157bbcase above is why._get_actual_output, sotarget_output_keyand job-attachment resolution still apply. An empty span list now renders as an empty history, not"[]".Noneoutput leaves the history alone, and{}is still rendered.Not in this PR
execute()already flushes in itsfinallybefore the eval runtime reads, and none of these runs is "tool calls happened, history empty".rootspan closes at suspend, a second one opens on resume ~60s later). After resume theEvaluationandEvaluation Set Runspans never get the scores ({"scores": {}}), and the evaluator span losesexecution.id.8b4c11a5never resumed at all. That one is its own bug and I will pick it up separately.Testing
tests/evaluators/test_trajectory_agent_output.py, 14 tests using the conversational payload shape from the trace. The 4 evaluator-level tests fail onmain.packages/uipathsuite: 2642 passed, 1 xfailed. ruff and mypy clean on the touched files.🤖 Generated with Claude Code