feat(x2a): Telemetry data for each job#2500
feat(x2a): Telemetry data for each job#2500mareklibra merged 4 commits intoredhat-developer:mainfrom
Conversation
Missing ChangesetsThe following package(s) are changed by this PR but do not have a changeset:
See CONTRIBUTING.md for more information about how to add changesets. Changed Packages
|
Review Summary by QodoAdd telemetry data visualization for agent job execution
WalkthroughsDescription• Add telemetry data visualization for LLM agent execution • Create formatDuration utility for human-readable time display • Display agent metrics including duration and tool calls • Add multilingual support for telemetry UI labels Diagramflowchart LR
A["Phase Data with Telemetry"] -->|"consumed by"| B["PhaseDetails Component"]
B -->|"renders"| C["PhaseTelemetry Component"]
C -->|"uses"| D["formatDuration Utility"]
C -->|"displays"| E["Agent Metrics Table"]
E -->|"shows"| F["Agent Name, Duration, Tool Calls"]
G["Translation Files"] -->|"provides labels"| C
File Changes1. workspaces/x2a/plugins/x2a/src/components/tools/formatDuration.ts
|
Code Review by Qodo
1. Invalid Box gap prop
|
![rhdh-qodo-merge[bot]](https://avatars.githubusercontent.com/in/1961108?s=60&v=4){kind=small}
| ); | ||
|
|
||
| return ( | ||
| <Box display="flex" flexWrap="wrap" gap={0.5}> |
There was a problem hiding this comment.
1. Invalid box gap prop 🐞 Bug ✓ Correctness
ToolCallsCell uses gap={0.5} on a Box imported from @material-ui/core (MUI v4). This prop is
not part of the v4 Box system props and may be forwarded as an invalid DOM attribute, leading to
React warnings and unreliable/absent chip spacing.
Agent Prompt
### Issue description
`ToolCallsCell` uses `<Box gap={0.5}>` while importing `Box` from `@material-ui/core` (MUI v4). `gap` is not a reliable prop for v4 `Box` and can be forwarded as an invalid DOM attribute, producing React warnings and inconsistent spacing.
### Issue Context
This plugin depends on `@material-ui/core` v4, and the component imports `Box` from that package. Use v4-supported spacing patterns.
### Fix Focus Areas
- workspaces/x2a/plugins/x2a/src/components/PhaseTelemetry.tsx[32-57]
### Suggested implementation direction
- Remove `gap={0.5}` from the `Box`.
- Add spacing via v4-supported props, e.g. wrap each `Chip` in a `<Box mr={0.5} mb={0.5}>...</Box>`, or apply a `className`/`style` with margins to the `Chip` wrapper.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
eloycoto
force-pushed
the
TelemetryInformation
branch
from
2a4a7fe to
1cf7ee8
Compare
Add a way to digest the telemetry data per each agent, so the user is able to understand what the LLM is doing. Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>
eloycoto
force-pushed
the
TelemetryInformation
branch
from
1cf7ee8 to
30398e3
Compare
mareklibra
left a comment
mareklibra
left a comment
There was a problem hiding this comment.
Rendering the data as chips prevents copy&paste and is hard to see/analyze otherwise.
@ShiranHi , any suggestions, please?
| * limitations under the License. | ||
| */ | ||
|
|
||
| export const formatDuration = (durationSeconds: number): string => { |
There was a problem hiding this comment.
please add tests, cover NaN, null or undefined
There was a problem hiding this comment.
Mind reusing the computeDuration() in the PhaseDetails.tsx
| return ( | ||
| <Box className={classes.chipContainer}> | ||
| {sortedToolCalls.map(([tool, count]) => ( | ||
| <Chip |
There was a problem hiding this comment.
The Chip component is not ideal way to represent this kind of data. Let's find another form which allows easier analysis of the data.
| }, [telemetry]); | ||
|
|
||
| const columns = useMemo((): TableColumn<AgentRow>[] => { | ||
| return [ |
There was a problem hiding this comment.
Shouldn't we render telemetry.summary as well?
I agree regarding the "Tool calls" column, the current chip layout makes the table cluttered and prevents quick comparisons. I suggest renaming the column to "Count of tool calls" and switching to a plain, vertically aligned list. This keeps the counts on the left for easy scanning while removing the visual noise of the chips. For example: What do you think? |
Make sense, let me implement it :) |
|
done, changes done: |
Co-authored-by: Marek Libra <marek.libra@gmail.com>
|
* feat(x2a): Telemetry data for each job Add a way to digest the telemetry data per each agent, so the user is able to understand what the LLM is doing. Signed-off-by: Eloy Coto <eloy.coto@acalustra.com> * fix(x2a): format duration example * feat(x2a): telemetry with ul list instead of chips * Apply suggestion from @mareklibra Co-authored-by: Marek Libra <marek.libra@gmail.com> --------- Signed-off-by: Eloy Coto <eloy.coto@acalustra.com> Co-authored-by: Marek Libra <marek.libra@gmail.com>
4 participants
Add a way to digest the telemetry data per each agent, so the user is able to understand what the LLM is doing.