Conversation
|
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (22)
✅ Files skipped from review due to trivial changes (16)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughSummary by CodeRabbit
WalkthroughMultiple UI formatting and focused functional tweaks across dashboard and logs: typed tooltips, chart tick/formatting and bucket/selection math changes, new column visualizations (latency/tokens/cost), minor Sheet styling update, numerous whitespace/formatting edits, and Go toolchain bumps from 1.26.1 → 1.26.2 across many modules. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ 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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.4)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
Contributor
🧪 Test Suite AvailableThis PR can be tested by a repository admin. |
akshaydeo
force-pushed
the
04-19-logs_ux_updates
branch
from
a40bc38 to
2d1ffce
Compare
Contributor
Confidence Score: 5/5Safe to merge — all remaining findings are P2 style/UX suggestions with no crash or data-integrity risk. No P0/P1 issues found. Previously flagged crashes and null-guard bugs are resolved. The two remaining comments are minor display polish (End Timestamp showing start time when latency is null, and the linear latency bar scale). logDetailView.tsx — minor End Timestamp display when latency is null. Important Files Changed
Reviews (9): Last reviewed commit: "logs ux updates" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
ui/app/workspace/dashboard/components/overviewTab.tsx (1)
216-308: Extract the cost/usage legend trees beforeOverviewTabgrows further.These two header branches are doing a lot of nested tooltip and conditional work inline, which makes the tab harder to scan and maintain than it needs to be. Pulling them into small route-local view components would keep this file focused on composition.
As per coding guidelines, "Avoid deeply nested conditional rendering; break complex UI into smaller components."
Also applies to: 324-414
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/dashboard/components/overviewTab.tsx` around lines 216 - 308, The header legend markup inside OverviewTab (the headerActions JSX that builds the cost legend with nested Tooltips and conditional branches) is too large and should be extracted into small local view components to reduce nesting and improve readability; create two components (e.g., CostLegendAll and CostLegendSingle or a single CostLegend that handles both branches) that encapsulate the logic currently using costModel, costModels, getModelColor, and the Tooltip/TooltipTrigger/TooltipContent trees, then replace the inline JSX in OverviewTab with a simple <CostLegend ... /> call and pass props (costModel, costModels, getModelColor) so ModelFilterSelect and ChartTypeToggle remain untouched. Ensure you also refactor the other similar block (the one noted at lines ~324-414) the same way to keep OverviewTab focused on composition.ui/app/workspace/logs/views/columns.tsx (1)
1-5: Move these shared formatters out of the dashboard route.The logs table now depends on
@/app/workspace/dashboard/utils/chartUtils, which couples the logs feature to a route-local dashboard module. Since these helpers are shared UI formatters now, relocating them under a neutral shared utility module inui/libwould keep the feature boundaries cleaner.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/logs/views/columns.tsx` around lines 1 - 5, The imports formatCost, formatLatency, and formatTokens in columns.tsx are coming from a route-local module and should be moved to a neutral shared utility; create a new module (e.g. ui/lib/formatters or ui/lib/sharedFormatters) that re-exports formatCost, formatLatency, and formatTokens, copy or move the implementation there, update this file to import from the new module, and update any other files that import these helpers from "@/app/workspace/dashboard/utils/chartUtils" to use the new shared module (ensure named exports remain unchanged so function references like formatCost, formatLatency, formatTokens continue to work).ui/app/workspace/dashboard/components/charts/logVolumeChart.tsx (1)
29-33: Type the tooltip props instead of falling back toany.This tooltip is already consuming a known bucket shape, so keeping it as
anythrows away the type safety for one of the more fragile chart integration points. Recharts exposes tooltip prop types, so this can stay fully typed without extra casts.As per coding guidelines, "TypeScript/React must avoid using any type unless absolutely unavoidable; prefer strict typing and inference."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/dashboard/components/charts/logVolumeChart.tsx` around lines 29 - 33, The CustomTooltip currently types its props as any; replace this with Recharts' TooltipProps generic so the payload is strongly typed: import TooltipProps from 'recharts' and declare a bucket interface matching the chart data (e.g., LogVolumeBucket) then change the signature to function CustomTooltip(props: TooltipProps<LogVolumeBucket>) or function CustomTooltip({ active, payload }: TooltipProps<LogVolumeBucket>); this lets payload[0]?.payload be correctly typed and eliminates the use of any in CustomTooltip.ui/app/workspace/logs/views/logsVolumeChart.tsx (1)
158-164: Replace the Rechartsanypayloads with concrete types.The tooltip props, drag events, and bar click payloads are all on typed library boundaries, so leaving them as
anymakes this chart much easier to break silently during future data-shape changes.As per coding guidelines, "TypeScript/React must avoid using any type unless absolutely unavoidable; prefer strict typing and inference."
Also applies to: 329-343, 387-395, 494-504
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/logs/views/logsVolumeChart.tsx` around lines 158 - 164, Replace all uses of `any` for Recharts data with precise Recharts generics and your HistogramBucket type: update CustomTooltip signature to use TooltipProps<HistogramBucket & { formattedTime: string }, string> (import TooltipProps from 'recharts') and type the payload as TooltipProps<...>['payload'] (or Payload<T, K> if available) instead of any; do the same for the drag event handler and bar click handler by using Recharts' mouse/interaction event types (e.g., RechartsMouseEvent or the correct Bar event payload generic) and the HistogramBucket shape so payload[0].payload is strongly typed; ensure you import required types from 'recharts' and replace other `any` occurrences noted (lines ~329-343, ~387-395, ~494-504) with the corresponding TooltipProps/TooltipPayload and event payload generics referencing HistogramBucket.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/app/workspace/logs/views/logsTable.tsx`:
- Around line 152-155: When computing footer numbers, guard the calculations
with a totalItems===0 branch: if totalItems is zero, set startItem and endItem
to 0 (or both to 0) and set currentPage and totalPages to 0 (or otherwise clamp
to sensible minimums) instead of computing via
pagination.offset/pagination.limit; update the logic around currentPage,
totalPages, startItem, and endItem so the UI renders "0 of 0" and "Page 0 of 0"
(or the chosen clamped representation). Apply the same fix to the duplicate
calculation area where the same variables are computed (the second block that
repeats the pagination math).
- Around line 244-277: The row height is fixed via the TableRow/className "h-7"
and TableCell "py-1", which clips the new multi-line cells (timestamp,
model/provider, tokens); remove the hard-coded "h-7" and reduce/remove the fixed
"py-1" so rows size to content (or replace with a min-height like "min-h-7" or
"min-h-[desired]" if you want a floor), leaving pin style logic (buildPinStyle,
lastLeftPinId, firstRightPinId) intact; alternatively implement measuring to set
row height from the tallest visible cell in the TableRow render if uniform
height is required.
In `@ui/app/workspace/logs/views/logsVolumeChart.tsx`:
- Around line 404-434: The CollapsibleTrigger and the reset-zoom button lack
stable test selectors; add data-testid attributes following the pattern
data-testid="<entity>-<element>-<qualifier>" to make them reachable in tests:
add e.g. data-testid="logs-volume-chart-trigger" to the CollapsibleTrigger
component (the one rendering ChevronDown and "Request Volume") and add
data-testid="logs-volume-chart-reset-zoom" to the button that uses onResetZoom /
renders the RotateCcw icon; keep naming consistent with other dashboard testids.
---
Nitpick comments:
In `@ui/app/workspace/dashboard/components/charts/logVolumeChart.tsx`:
- Around line 29-33: The CustomTooltip currently types its props as any; replace
this with Recharts' TooltipProps generic so the payload is strongly typed:
import TooltipProps from 'recharts' and declare a bucket interface matching the
chart data (e.g., LogVolumeBucket) then change the signature to function
CustomTooltip(props: TooltipProps<LogVolumeBucket>) or function CustomTooltip({
active, payload }: TooltipProps<LogVolumeBucket>); this lets payload[0]?.payload
be correctly typed and eliminates the use of any in CustomTooltip.
In `@ui/app/workspace/dashboard/components/overviewTab.tsx`:
- Around line 216-308: The header legend markup inside OverviewTab (the
headerActions JSX that builds the cost legend with nested Tooltips and
conditional branches) is too large and should be extracted into small local view
components to reduce nesting and improve readability; create two components
(e.g., CostLegendAll and CostLegendSingle or a single CostLegend that handles
both branches) that encapsulate the logic currently using costModel, costModels,
getModelColor, and the Tooltip/TooltipTrigger/TooltipContent trees, then replace
the inline JSX in OverviewTab with a simple <CostLegend ... /> call and pass
props (costModel, costModels, getModelColor) so ModelFilterSelect and
ChartTypeToggle remain untouched. Ensure you also refactor the other similar
block (the one noted at lines ~324-414) the same way to keep OverviewTab focused
on composition.
In `@ui/app/workspace/logs/views/columns.tsx`:
- Around line 1-5: The imports formatCost, formatLatency, and formatTokens in
columns.tsx are coming from a route-local module and should be moved to a
neutral shared utility; create a new module (e.g. ui/lib/formatters or
ui/lib/sharedFormatters) that re-exports formatCost, formatLatency, and
formatTokens, copy or move the implementation there, update this file to import
from the new module, and update any other files that import these helpers from
"@/app/workspace/dashboard/utils/chartUtils" to use the new shared module
(ensure named exports remain unchanged so function references like formatCost,
formatLatency, formatTokens continue to work).
In `@ui/app/workspace/logs/views/logsVolumeChart.tsx`:
- Around line 158-164: Replace all uses of `any` for Recharts data with precise
Recharts generics and your HistogramBucket type: update CustomTooltip signature
to use TooltipProps<HistogramBucket & { formattedTime: string }, string> (import
TooltipProps from 'recharts') and type the payload as
TooltipProps<...>['payload'] (or Payload<T, K> if available) instead of any; do
the same for the drag event handler and bar click handler by using Recharts'
mouse/interaction event types (e.g., RechartsMouseEvent or the correct Bar event
payload generic) and the HistogramBucket shape so payload[0].payload is strongly
typed; ensure you import required types from 'recharts' and replace other `any`
occurrences noted (lines ~329-343, ~387-395, ~494-504) with the corresponding
TooltipProps/TooltipPayload and event payload generics referencing
HistogramBucket.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 81cf72ed-7c98-471e-b300-06a4fb04caac
📒 Files selected for processing (10)
ui/app/workspace/dashboard/components/charts/chartCard.tsxui/app/workspace/dashboard/components/charts/logVolumeChart.tsxui/app/workspace/dashboard/components/overviewTab.tsxui/app/workspace/logs/sheets/logDetailView.tsxui/app/workspace/logs/sheets/logDetailsSheet.tsxui/app/workspace/logs/views/columns.tsxui/app/workspace/logs/views/logsTable.tsxui/app/workspace/logs/views/logsVolumeChart.tsxui/lib/constants/icons.tsxui/lib/utils.ts
akshaydeo
force-pushed
the
04-19-logs_ux_updates
branch
from
2d1ffce to
716b86e
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ui/app/workspace/logs/views/columns.tsx (1)
477-486: Adddata-testidto the delete button.Per coding guidelines, interactive UI elements should have
data-testidattributes following the pattern<entity>-<element>-<qualifier>.♻️ Proposed fix
<Button variant="outline" size="icon" aria-label="Delete log" + data-testid="log-delete-button" className="text-secondary-foreground/30 hover:bg-destructive/10 hover:text-destructive border-destructive/10" onClick={() => onDelete(log)} disabled={!hasDeleteAccess} > <Trash2 strokeWidth={1.5} /> </Button>As per coding guidelines: "Add data-testid attributes to all interactive UI elements following the pattern: data-testid="--""
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/logs/views/columns.tsx` around lines 477 - 486, Add a data-testid to the delete Button in logs/views/columns.tsx so it follows the pattern <entity>-<element>-<qualifier>; specifically add data-testid="log-button-delete" to the Button component (the one with onClick={() => onDelete(log)} and disabled={!hasDeleteAccess}) so automated tests can reliably target the delete control.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/app/workspace/logs/views/columns.tsx`:
- Around line 318-333: The call to getProviderLabel(row.original.provider) can
throw if row.original.provider is null/undefined because getProviderLabel calls
provider.toLowerCase(); add a null guard in the cell renderer (where provider is
derived from row.original.provider) and pass a safe fallback (e.g., a default
string like "unknown" or an empty string) to getProviderLabel when provider is
falsy, or conditionally render the label only when provider is defined; update
the cell rendering logic (references: provider variable in the cell,
getProviderLabel, and RenderProviderIcon) to ensure provider is validated before
invoking getProviderLabel.
---
Nitpick comments:
In `@ui/app/workspace/logs/views/columns.tsx`:
- Around line 477-486: Add a data-testid to the delete Button in
logs/views/columns.tsx so it follows the pattern <entity>-<element>-<qualifier>;
specifically add data-testid="log-button-delete" to the Button component (the
one with onClick={() => onDelete(log)} and disabled={!hasDeleteAccess}) so
automated tests can reliably target the delete control.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 322589ec-c67e-4ad6-8649-c3b74069e257
📒 Files selected for processing (21)
framework/go.modplugins/compat/go.modplugins/governance/go.modplugins/jsonparser/go.modplugins/logging/go.modplugins/maxim/go.modplugins/mocker/go.modplugins/otel/go.modplugins/semanticcache/go.modplugins/telemetry/go.modtransports/go.modui/app/workspace/dashboard/components/charts/chartCard.tsxui/app/workspace/dashboard/components/charts/logVolumeChart.tsxui/app/workspace/dashboard/components/overviewTab.tsxui/app/workspace/logs/sheets/logDetailView.tsxui/app/workspace/logs/sheets/logDetailsSheet.tsxui/app/workspace/logs/views/columns.tsxui/app/workspace/logs/views/logsTable.tsxui/app/workspace/logs/views/logsVolumeChart.tsxui/lib/constants/icons.tsxui/lib/utils.ts
✅ Files skipped from review due to trivial changes (14)
- plugins/mocker/go.mod
- plugins/semanticcache/go.mod
- framework/go.mod
- plugins/otel/go.mod
- plugins/jsonparser/go.mod
- plugins/governance/go.mod
- plugins/telemetry/go.mod
- plugins/logging/go.mod
- transports/go.mod
- plugins/maxim/go.mod
- plugins/compat/go.mod
- ui/app/workspace/dashboard/components/charts/chartCard.tsx
- ui/app/workspace/dashboard/components/overviewTab.tsx
- ui/app/workspace/logs/sheets/logDetailsSheet.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- ui/lib/utils.ts
- ui/app/workspace/logs/views/logsTable.tsx
- ui/app/workspace/dashboard/components/charts/logVolumeChart.tsx
- ui/app/workspace/logs/views/logsVolumeChart.tsx
akshaydeo
force-pushed
the
04-19-logs_ux_updates
branch
from
716b86e to
64332dd
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ui/app/workspace/logs/views/columns.tsx (1)
288-305: Consider adding a fallback for unknown request types.If
row.original.objectis not a key inRequestTypeLabelsorRequestTypeColors, the badge will renderundefinedtext and no color class. You may want to provide a fallback to handle custom or unexpected object types gracefully.♻️ Suggested improvement
cell: ({ row }) => { + const objectType = row.original.object as keyof typeof RequestTypeLabels; + const label = RequestTypeLabels[objectType] ?? row.original.object; + const colorClass = RequestTypeColors[objectType] ?? ""; return ( <Badge variant="outline" className={cn( "font-mono text-[11px] py-0.5 px-1.5 uppercase", - RequestTypeColors[ - row.original.object as keyof typeof RequestTypeColors - ], + colorClass, )} > - { - RequestTypeLabels[ - row.original.object as keyof typeof RequestTypeLabels - ] - } + {label} </Badge> ); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/logs/views/columns.tsx` around lines 288 - 305, The badge cell renderer currently reads RequestTypeLabels and RequestTypeColors using row.original.object and can produce undefined for unknown keys; update the cell renderer in columns.tsx (the cell: ({ row }) => { ... } block) to compute a safe key and fallbacks: derive a lookupKey from row.original.object, check membership in RequestTypeLabels/RequestTypeColors, and use a default label (e.g., 'UNKNOWN' or String(row.original.object)) and a default color/class (e.g., a neutral gray class or RequestTypeColors['default']) when the key is missing before passing values to RequestTypeLabels/RequestTypeColors, Badge and cn so unknown/custom request types render sensibly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/app/workspace/logs/views/columns.tsx`:
- Around line 398-410: The bug is that total_tokens can be undefined and is
passed to formatTokens, so replace the declaration of total (const total =
tokenUsage.total_tokens) with a null-coalesced default (const total =
tokenUsage.total_tokens ?? 0) so it matches prompt and completion fallbacks;
update any uses of total (e.g., the formatTokens(total) call and any displays
relying on total) to rely on this non-null value.
---
Nitpick comments:
In `@ui/app/workspace/logs/views/columns.tsx`:
- Around line 288-305: The badge cell renderer currently reads RequestTypeLabels
and RequestTypeColors using row.original.object and can produce undefined for
unknown keys; update the cell renderer in columns.tsx (the cell: ({ row }) => {
... } block) to compute a safe key and fallbacks: derive a lookupKey from
row.original.object, check membership in RequestTypeLabels/RequestTypeColors,
and use a default label (e.g., 'UNKNOWN' or String(row.original.object)) and a
default color/class (e.g., a neutral gray class or RequestTypeColors['default'])
when the key is missing before passing values to
RequestTypeLabels/RequestTypeColors, Badge and cn so unknown/custom request
types render sensibly.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bc66a1a3-f160-4002-9f49-f2f190c5a2a0
📒 Files selected for processing (21)
framework/go.modplugins/compat/go.modplugins/governance/go.modplugins/jsonparser/go.modplugins/logging/go.modplugins/maxim/go.modplugins/mocker/go.modplugins/otel/go.modplugins/semanticcache/go.modplugins/telemetry/go.modtransports/go.modui/app/workspace/dashboard/components/charts/chartCard.tsxui/app/workspace/dashboard/components/charts/logVolumeChart.tsxui/app/workspace/dashboard/components/overviewTab.tsxui/app/workspace/logs/sheets/logDetailView.tsxui/app/workspace/logs/sheets/logDetailsSheet.tsxui/app/workspace/logs/views/columns.tsxui/app/workspace/logs/views/logsTable.tsxui/app/workspace/logs/views/logsVolumeChart.tsxui/lib/constants/icons.tsxui/lib/utils.ts
✅ Files skipped from review due to trivial changes (16)
- plugins/telemetry/go.mod
- plugins/otel/go.mod
- transports/go.mod
- ui/app/workspace/logs/sheets/logDetailsSheet.tsx
- plugins/mocker/go.mod
- ui/lib/utils.ts
- plugins/compat/go.mod
- plugins/governance/go.mod
- plugins/maxim/go.mod
- framework/go.mod
- plugins/jsonparser/go.mod
- plugins/semanticcache/go.mod
- ui/app/workspace/dashboard/components/charts/chartCard.tsx
- ui/app/workspace/dashboard/components/charts/logVolumeChart.tsx
- plugins/logging/go.mod
- ui/app/workspace/dashboard/components/overviewTab.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- ui/app/workspace/logs/views/logsTable.tsx
- ui/app/workspace/logs/views/logsVolumeChart.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
ui/app/workspace/logs/views/columns.tsx (1)
398-410:⚠️ Potential issue | 🟠 MajorDefault
total_tokensbefore formatting it.If the API sends prompt/completion counts but omits
total_tokens,formatTokens(total)will receiveundefinedand throw during cell render.🩹 Suggested fix
- const total = tokenUsage.total_tokens; + const total = tokenUsage.total_tokens ?? 0;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/logs/views/columns.tsx` around lines 398 - 410, The render uses total = tokenUsage.total_tokens which may be undefined; update the logic around tokenUsage/prompt/completion/total in the component (the variables tokenUsage, prompt, completion, total, hasSplit, splitBase and the call to formatTokens(total)) to default total to a numeric value (e.g., prompt + completion when both present, otherwise 0) before passing it to formatTokens so formatTokens never receives undefined and the cell render cannot throw.ui/app/workspace/logs/views/logsVolumeChart.tsx (1)
404-434:⚠️ Potential issue | 🟡 MinorAdd stable test ids to the header controls.
CollapsibleTriggerand the reset-zoom button are still interactive without stable selectors, so this updated chart surface remains harder to cover in E2E/UI tests.As per coding guidelines, "Add data-testid attributes to all interactive UI elements following the pattern: data-testid="--"."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/logs/views/logsVolumeChart.tsx` around lines 404 - 434, Add stable data-testid attributes to the interactive header controls so E2E tests can target them: add data-testid="logs-volume-chart-trigger" to the CollapsibleTrigger element and add data-testid="logs-volume-chart-reset-zoom" to the reset-zoom button rendered when isZoomed and onResetZoom are present (the button that calls onResetZoom and contains RotateCcw). Ensure the attributes follow the pattern data-testid="<entity>-<element>-<qualifier>" and are added to the JSX elements for CollapsibleTrigger and the reset button.
🧹 Nitpick comments (1)
ui/app/workspace/logs/views/columns.tsx (1)
1-5: Move these shared formatters out of the dashboard route.
columns.tsxis now depending on@/app/workspace/dashboard/utils/chartUtils, which couples the logs view to a sibling route’s private utility module. Since these helpers are now shared across surfaces, they should live under a neutral shared utility path inui/libinstead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/logs/views/columns.tsx` around lines 1 - 5, columns.tsx currently imports shared formatters formatCost, formatLatency, and formatTokens from a dashboard-only module, coupling the logs view to a sibling route; extract these helpers into a neutral shared utility under ui/lib (e.g. ui/lib/formatters or ui/lib/chartUtils), move the functions (formatCost, formatLatency, formatTokens) there, update their exports, and then change the import in columns.tsx to reference the new shared module so logs and other routes consume the common utilities instead of the dashboard private path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/app/workspace/logs/views/columns.tsx`:
- Around line 273-280: The table shell is forcing single-line clipping (class
h-7 and per-cell classes overflow-hidden truncate whitespace-nowrap) so your new
multi-line timestamp/model/tokens renderers in the LogsTable won't be visible;
locate the LogsTable component (references around the row/cell definitions where
h-7 and overflow-hidden truncate whitespace-nowrap are applied, e.g., the blocks
at 243-270, 327-333, 407-427) and remove the fixed row height (replace h-7 with
h-auto or min-h-7) and remove truncate/whitespace-nowrap/overflow-hidden on the
cell containers; instead use overflow-visible (or no overflow class),
whitespace-normal and break-words (or break-all if needed) so cells can wrap to
multiple lines and the inline bars render fully while preserving vertical
alignment and spacing.
In `@ui/app/workspace/logs/views/logsVolumeChart.tsx`:
- Around line 233-239: The loop that fills buckets currently includes a bucket
at time === maxTime (where maxTime = endTime * 1000) which treats endTime as
inclusive; change the fill logic so it treats maxTime as exclusive: compute
estimatedBuckets as Math.ceil((maxTime - minTime) / bucketSizeMs) (clamped as
before) but iterate buckets using a strict less-than condition (e.g., for (let i
= 0; i < estimatedBuckets; i++) or while (time < maxTime) instead of <= or time
=== maxTime), or alternatively subtract 1ms from maxTime before generating
buckets; update the fill loop (the block that uses minTime, maxTime,
bucketSizeMs and estimatedBuckets around the current fill and the similar block
at lines ~275-289) to stop before the exclusive end boundary so no phantom
bucket is created.
---
Duplicate comments:
In `@ui/app/workspace/logs/views/columns.tsx`:
- Around line 398-410: The render uses total = tokenUsage.total_tokens which may
be undefined; update the logic around tokenUsage/prompt/completion/total in the
component (the variables tokenUsage, prompt, completion, total, hasSplit,
splitBase and the call to formatTokens(total)) to default total to a numeric
value (e.g., prompt + completion when both present, otherwise 0) before passing
it to formatTokens so formatTokens never receives undefined and the cell render
cannot throw.
In `@ui/app/workspace/logs/views/logsVolumeChart.tsx`:
- Around line 404-434: Add stable data-testid attributes to the interactive
header controls so E2E tests can target them: add
data-testid="logs-volume-chart-trigger" to the CollapsibleTrigger element and
add data-testid="logs-volume-chart-reset-zoom" to the reset-zoom button rendered
when isZoomed and onResetZoom are present (the button that calls onResetZoom and
contains RotateCcw). Ensure the attributes follow the pattern
data-testid="<entity>-<element>-<qualifier>" and are added to the JSX elements
for CollapsibleTrigger and the reset button.
---
Nitpick comments:
In `@ui/app/workspace/logs/views/columns.tsx`:
- Around line 1-5: columns.tsx currently imports shared formatters formatCost,
formatLatency, and formatTokens from a dashboard-only module, coupling the logs
view to a sibling route; extract these helpers into a neutral shared utility
under ui/lib (e.g. ui/lib/formatters or ui/lib/chartUtils), move the functions
(formatCost, formatLatency, formatTokens) there, update their exports, and then
change the import in columns.tsx to reference the new shared module so logs and
other routes consume the common utilities instead of the dashboard private path.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3cc3f7c9-85df-4b1c-a362-8ae08d8b9317
📒 Files selected for processing (22)
framework/go.modplugins/compat/go.modplugins/governance/go.modplugins/jsonparser/go.modplugins/logging/go.modplugins/maxim/go.modplugins/mocker/go.modplugins/otel/go.modplugins/semanticcache/go.modplugins/telemetry/go.modtransports/go.modui/app/workspace/dashboard/components/charts/chartCard.tsxui/app/workspace/dashboard/components/charts/logVolumeChart.tsxui/app/workspace/dashboard/components/overviewTab.tsxui/app/workspace/logs/page.tsxui/app/workspace/logs/sheets/logDetailView.tsxui/app/workspace/logs/sheets/logDetailsSheet.tsxui/app/workspace/logs/views/columns.tsxui/app/workspace/logs/views/logsTable.tsxui/app/workspace/logs/views/logsVolumeChart.tsxui/lib/constants/icons.tsxui/lib/utils.ts
✅ Files skipped from review due to trivial changes (16)
- framework/go.mod
- plugins/telemetry/go.mod
- plugins/compat/go.mod
- plugins/otel/go.mod
- transports/go.mod
- plugins/jsonparser/go.mod
- plugins/mocker/go.mod
- ui/lib/utils.ts
- plugins/maxim/go.mod
- plugins/governance/go.mod
- ui/app/workspace/dashboard/components/overviewTab.tsx
- ui/app/workspace/logs/sheets/logDetailsSheet.tsx
- plugins/semanticcache/go.mod
- plugins/logging/go.mod
- ui/app/workspace/dashboard/components/charts/chartCard.tsx
- ui/app/workspace/logs/page.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- ui/app/workspace/dashboard/components/charts/logVolumeChart.tsx
- ui/app/workspace/logs/views/logsTable.tsx
akshaydeo
force-pushed
the
04-19-logs_ux_updates
branch
from
b9c7d56 to
daa4fff
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
ui/app/workspace/logs/views/logsVolumeChart.tsx (2)
431-435: Set an explicit button type for the reset action.Without
type="button", this can submit a parent form unintentionally.✅ Small hardening change
{isZoomed && onResetZoom && ( <button + type="button" data-testid="logs-volume-chart-reset-zoom" onClick={onResetZoom} className="text-muted-foreground hover:text-foreground flex items-center gap-1 text-xs transition-colors" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/logs/views/logsVolumeChart.tsx` around lines 431 - 435, The reset zoom button in LogsVolumeChart uses the onResetZoom handler but lacks an explicit type, which can cause accidental form submission; update the <button> element with data-testid "logs-volume-chart-reset-zoom" in logsVolumeChart.tsx to include type="button" so the onResetZoom click won't submit any parent form.
158-163: Replaceanywith narrow local types in tooltip and chart handlers.These
anyusages are avoidable and weaken type-safety on a critical interaction path.♻️ Suggested typing cleanup
+type ChartPoint = HistogramBucket & { formattedTime: string; index: number }; +type ChartMouseEvent = { activeTooltipIndex?: number }; +type TooltipDatum = { payload?: ChartPoint }; -function CustomTooltip({ active, payload }: any) { +function CustomTooltip({ + active, + payload, +}: { + active?: boolean; + payload?: TooltipDatum[]; +}) { - const data = payload[0]?.payload as HistogramBucket & { - formattedTime: string; - }; + const data = payload?.[0]?.payload; - const handleMouseDown = useCallback((e: any) => { + const handleMouseDown = useCallback((e: ChartMouseEvent) => { - (e: any) => { + (e: ChartMouseEvent) => { - (barData: any) => { + (barData?: ChartPoint) => { if (!data || !barData?.timestamp) return; - const bucket = barData as HistogramBucket; - const startTime = new Date(bucket.timestamp).getTime() / 1000; + const startTime = new Date(barData.timestamp).getTime() / 1000;As per coding guidelines, "TypeScript/React must avoid using any type unless absolutely unavoidable; prefer strict typing and inference".
Also applies to: 329-340, 387-398, 498-508
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/logs/views/logsVolumeChart.tsx` around lines 158 - 163, The tooltip and chart handler functions (starting with CustomTooltip) use `any`, weakening type-safety; replace `any` with specific local types (for example use Recharts' TooltipProps/TooltipPayload generics and a union/type for HistogramBucket with formattedTime) and similarly narrow the types in the other chart handlers referenced (around the blocks at 329-340, 387-398, 498-508) so payload, active, and event parameters are strongly typed; update the function signatures (e.g., CustomTooltip(...) and each chart handler) to use those concrete types and adjust local casts to a HistogramBucket & { formattedTime: string } instead of using `any`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/app/workspace/dashboard/components/charts/logVolumeChart.tsx`:
- Around line 29-33: Replace the loose any on CustomTooltip props with a
concrete TypeScript type: define an interface (e.g. TooltipPayloadItem and
CustomTooltipProps) describing payload shape (payload: { payload: { timestamp:
string; success: number; error: number; count: number; index?: number;
formattedTime?: string } }[] | undefined, active: boolean) and use it in the
CustomTooltip signature (function CustomTooltip(props: CustomTooltipProps) or
const CustomTooltip: React.FC<CustomTooltipProps>). Update the existing local
variable usages (data = payload[0]?.payload) to match the typed shape so the
compiler can check timestamp, success, error, count, index, and formattedTime
fields.
In `@ui/app/workspace/logs/views/logsVolumeChart.tsx`:
- Around line 365-373: The selection math in logsVolumeChart.tsx computes bucket
edges asymmetrically (only adds data.bucket_size_seconds to rightBucket),
causing reverse (right-to-left) drags to drop the last bucket; update the
computation so you compute each bucket's end edge before taking min/max — e.g.,
compute leftEdge = new Date(leftBucket.timestamp).getTime()/1000 +
data.bucket_size_seconds and rightEdge = new
Date(rightBucket.timestamp).getTime()/1000 + data.bucket_size_seconds (or at
minimum add bucket_size_seconds to both timestamps) then set selectionStart =
Math.min(leftEdge, rightEdge) and selectionEnd = Math.max(leftEdge, rightEdge)
(references: leftBucket, rightBucket, data.bucket_size_seconds, selectionStart,
selectionEnd).
---
Nitpick comments:
In `@ui/app/workspace/logs/views/logsVolumeChart.tsx`:
- Around line 431-435: The reset zoom button in LogsVolumeChart uses the
onResetZoom handler but lacks an explicit type, which can cause accidental form
submission; update the <button> element with data-testid
"logs-volume-chart-reset-zoom" in logsVolumeChart.tsx to include type="button"
so the onResetZoom click won't submit any parent form.
- Around line 158-163: The tooltip and chart handler functions (starting with
CustomTooltip) use `any`, weakening type-safety; replace `any` with specific
local types (for example use Recharts' TooltipProps/TooltipPayload generics and
a union/type for HistogramBucket with formattedTime) and similarly narrow the
types in the other chart handlers referenced (around the blocks at 329-340,
387-398, 498-508) so payload, active, and event parameters are strongly typed;
update the function signatures (e.g., CustomTooltip(...) and each chart handler)
to use those concrete types and adjust local casts to a HistogramBucket & {
formattedTime: string } instead of using `any`.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3be32179-eb9f-44f6-bbae-9fb5f7130048
📒 Files selected for processing (22)
framework/go.modplugins/compat/go.modplugins/governance/go.modplugins/jsonparser/go.modplugins/logging/go.modplugins/maxim/go.modplugins/mocker/go.modplugins/otel/go.modplugins/semanticcache/go.modplugins/telemetry/go.modtransports/go.modui/app/workspace/dashboard/components/charts/chartCard.tsxui/app/workspace/dashboard/components/charts/logVolumeChart.tsxui/app/workspace/dashboard/components/overviewTab.tsxui/app/workspace/logs/page.tsxui/app/workspace/logs/sheets/logDetailView.tsxui/app/workspace/logs/sheets/logDetailsSheet.tsxui/app/workspace/logs/views/columns.tsxui/app/workspace/logs/views/logsTable.tsxui/app/workspace/logs/views/logsVolumeChart.tsxui/lib/constants/icons.tsxui/lib/utils.ts
✅ Files skipped from review due to trivial changes (16)
- plugins/jsonparser/go.mod
- plugins/mocker/go.mod
- plugins/otel/go.mod
- plugins/semanticcache/go.mod
- ui/lib/utils.ts
- plugins/governance/go.mod
- plugins/maxim/go.mod
- plugins/logging/go.mod
- framework/go.mod
- ui/app/workspace/logs/sheets/logDetailsSheet.tsx
- transports/go.mod
- plugins/compat/go.mod
- ui/app/workspace/logs/page.tsx
- plugins/telemetry/go.mod
- ui/app/workspace/dashboard/components/overviewTab.tsx
- ui/app/workspace/dashboard/components/charts/chartCard.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- ui/app/workspace/logs/views/logsTable.tsx
- ui/app/workspace/logs/views/columns.tsx
akshaydeo
force-pushed
the
04-19-logs_ux_updates
branch
from
daa4fff to
05e33bf
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ui/app/workspace/logs/views/logsVolumeChart.tsx (1)
158-163: Replace the remaininganychart payloads with typed Recharts props.
CustomTooltip, the drag handlers, and the bar click callback all depend on specific fields likepayload,activeTooltipIndex, andtimestamp. Keeping them asanyremoves the type checks exactly where this chart’s interaction logic is getting more complex. As per coding guidelines, "TypeScript/React must avoid using any type unless absolutely unavoidable; prefer strict typing and inference."Also applies to: 329-344, 385-397, 497-507
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/logs/views/logsVolumeChart.tsx` around lines 158 - 163, CustomTooltip and several chart interaction handlers still use untyped any for Recharts props which removes compile-time checks; replace those anys with the proper Recharts types (e.g., TooltipProps, TooltipPayload, BarProps or XAxisProps where appropriate) and narrow payload items to the HistogramBucket + { formattedTime: string } shape so fields like payload[0].payload, activeTooltipIndex, and timestamp are strongly typed; update the function signatures for CustomTooltip, the drag handlers, and the bar click callback to accept the correct Recharts generic types and use type guards (or optional chaining) to safely access payload[0].payload as HistogramBucket to restore type safety.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/app/workspace/logs/views/logsVolumeChart.tsx`:
- Around line 29-37: formatRequest currently can produce "1000.0K" for values
just below 1_000_000; change the logic in formatRequest to detect when the
thousands branch would round up to 1000K and instead return "1M" (e.g., compute
valueK = requests / 1000 and if valueK >= 1000 return "1M", else return
`${valueK.toFixed(1)}K`), or alternatively replace the whole formatter with
Intl.NumberFormat(undefined, { notation: "compact", maximumFractionDigits: 1 })
to handle compact rounding consistently.
---
Nitpick comments:
In `@ui/app/workspace/logs/views/logsVolumeChart.tsx`:
- Around line 158-163: CustomTooltip and several chart interaction handlers
still use untyped any for Recharts props which removes compile-time checks;
replace those anys with the proper Recharts types (e.g., TooltipProps,
TooltipPayload, BarProps or XAxisProps where appropriate) and narrow payload
items to the HistogramBucket + { formattedTime: string } shape so fields like
payload[0].payload, activeTooltipIndex, and timestamp are strongly typed; update
the function signatures for CustomTooltip, the drag handlers, and the bar click
callback to accept the correct Recharts generic types and use type guards (or
optional chaining) to safely access payload[0].payload as HistogramBucket to
restore type safety.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6f7b8d13-752e-4be0-8bb0-18d775fdedb0
📒 Files selected for processing (22)
framework/go.modplugins/compat/go.modplugins/governance/go.modplugins/jsonparser/go.modplugins/logging/go.modplugins/maxim/go.modplugins/mocker/go.modplugins/otel/go.modplugins/semanticcache/go.modplugins/telemetry/go.modtransports/go.modui/app/workspace/dashboard/components/charts/chartCard.tsxui/app/workspace/dashboard/components/charts/logVolumeChart.tsxui/app/workspace/dashboard/components/overviewTab.tsxui/app/workspace/logs/page.tsxui/app/workspace/logs/sheets/logDetailView.tsxui/app/workspace/logs/sheets/logDetailsSheet.tsxui/app/workspace/logs/views/columns.tsxui/app/workspace/logs/views/logsTable.tsxui/app/workspace/logs/views/logsVolumeChart.tsxui/lib/constants/icons.tsxui/lib/utils.ts
✅ Files skipped from review due to trivial changes (15)
- plugins/mocker/go.mod
- transports/go.mod
- ui/lib/utils.ts
- plugins/otel/go.mod
- plugins/telemetry/go.mod
- plugins/governance/go.mod
- plugins/logging/go.mod
- plugins/maxim/go.mod
- plugins/jsonparser/go.mod
- framework/go.mod
- ui/app/workspace/dashboard/components/overviewTab.tsx
- plugins/semanticcache/go.mod
- plugins/compat/go.mod
- ui/app/workspace/logs/sheets/logDetailsSheet.tsx
- ui/app/workspace/logs/page.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- ui/app/workspace/dashboard/components/charts/chartCard.tsx
- ui/app/workspace/logs/views/logsTable.tsx
- ui/app/workspace/dashboard/components/charts/logVolumeChart.tsx
- ui/app/workspace/logs/views/columns.tsx
akshaydeo
force-pushed
the
04-19-logs_ux_updates
branch
from
05e33bf to
fd7c7c6
Compare
Contributor
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 `@ui/app/workspace/logs/views/logsVolumeChart.tsx`:
- Around line 157-163: Replace the untyped any usages by adding small local
interfaces and specific event types: type CustomTooltip props using Recharts'
TooltipPayload/TooltipProps (or a local interface matching { active: boolean;
payload?: Array<{ payload: HistogramBucket & { formattedTime: string } }> }),
type handleMouseDown/handleMove parameters with React.MouseEvent<HTMLDivElement
| SVGElement> (or PointerEvent) and the recharts chart's active payload type,
and type handleBarClick/onClick handlers with (event: React.MouseEvent, data:
HistogramBucket) or the appropriate Bar/Rectangle click signature; update
function signatures for CustomTooltip, handleMouseDown, handleMove,
handleBarClick and the onClick props to use these types so editor/typechecker
can validate payloads instead of any.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 963549de-bf36-4b37-a287-080152d7ce8e
📒 Files selected for processing (22)
framework/go.modplugins/compat/go.modplugins/governance/go.modplugins/jsonparser/go.modplugins/logging/go.modplugins/maxim/go.modplugins/mocker/go.modplugins/otel/go.modplugins/semanticcache/go.modplugins/telemetry/go.modtransports/go.modui/app/workspace/dashboard/components/charts/chartCard.tsxui/app/workspace/dashboard/components/charts/logVolumeChart.tsxui/app/workspace/dashboard/components/overviewTab.tsxui/app/workspace/logs/page.tsxui/app/workspace/logs/sheets/logDetailView.tsxui/app/workspace/logs/sheets/logDetailsSheet.tsxui/app/workspace/logs/views/columns.tsxui/app/workspace/logs/views/logsTable.tsxui/app/workspace/logs/views/logsVolumeChart.tsxui/lib/constants/icons.tsxui/lib/utils.ts
✅ Files skipped from review due to trivial changes (15)
- plugins/semanticcache/go.mod
- plugins/jsonparser/go.mod
- plugins/mocker/go.mod
- plugins/logging/go.mod
- plugins/otel/go.mod
- plugins/telemetry/go.mod
- framework/go.mod
- ui/app/workspace/logs/sheets/logDetailsSheet.tsx
- transports/go.mod
- ui/lib/utils.ts
- plugins/compat/go.mod
- plugins/maxim/go.mod
- plugins/governance/go.mod
- ui/app/workspace/logs/page.tsx
- ui/app/workspace/logs/views/columns.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- ui/app/workspace/dashboard/components/charts/chartCard.tsx
- ui/app/workspace/logs/views/logsTable.tsx
- ui/app/workspace/dashboard/components/overviewTab.tsx
akshaydeo
force-pushed
the
04-19-logs_ux_updates
branch
from
fd7c7c6 to
4dea2ff
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ui/app/workspace/logs/views/columns.tsx (1)
288-306: Consider adding a fallback for unknown request types.If
row.original.objectis not present inRequestTypeLabelsorRequestTypeColors, the Badge will render empty with potentially missing styling. While this may be unlikely if the API always returns valid types, a defensive fallback would improve robustness.♻️ Suggested fallback
return ( <Badge variant="outline" className={cn( "font-mono text-[11px] py-0.5 px-1.5 uppercase", RequestTypeColors[ row.original.object as keyof typeof RequestTypeColors - ], + ] ?? "bg-zinc-100 text-zinc-600 dark:bg-zinc-800 dark:text-zinc-400", )} > { RequestTypeLabels[ row.original.object as keyof typeof RequestTypeLabels - ] + ] ?? row.original.object ?? "Unknown" } </Badge> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/logs/views/columns.tsx` around lines 288 - 306, The Badge cell renderer uses RequestTypeLabels and RequestTypeColors keyed by row.original.object but has no fallback; update the cell function (in the cell renderer for the column) to derive a safe key from row.original.object, check existence in RequestTypeLabels and RequestTypeColors (e.g., via hasOwnProperty or in), and supply a default label (like "UNKNOWN" or "Other") and default color variant when the key is missing so the Badge always renders with text and styling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@ui/app/workspace/logs/views/columns.tsx`:
- Around line 288-306: The Badge cell renderer uses RequestTypeLabels and
RequestTypeColors keyed by row.original.object but has no fallback; update the
cell function (in the cell renderer for the column) to derive a safe key from
row.original.object, check existence in RequestTypeLabels and RequestTypeColors
(e.g., via hasOwnProperty or in), and supply a default label (like "UNKNOWN" or
"Other") and default color variant when the key is missing so the Badge always
renders with text and styling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ae5080f2-e1fe-4e7d-9f32-ba3525f19010
📒 Files selected for processing (22)
framework/go.modplugins/compat/go.modplugins/governance/go.modplugins/jsonparser/go.modplugins/logging/go.modplugins/maxim/go.modplugins/mocker/go.modplugins/otel/go.modplugins/semanticcache/go.modplugins/telemetry/go.modtransports/go.modui/app/workspace/dashboard/components/charts/chartCard.tsxui/app/workspace/dashboard/components/charts/logVolumeChart.tsxui/app/workspace/dashboard/components/overviewTab.tsxui/app/workspace/logs/page.tsxui/app/workspace/logs/sheets/logDetailView.tsxui/app/workspace/logs/sheets/logDetailsSheet.tsxui/app/workspace/logs/views/columns.tsxui/app/workspace/logs/views/logsTable.tsxui/app/workspace/logs/views/logsVolumeChart.tsxui/lib/constants/icons.tsxui/lib/utils.ts
✅ Files skipped from review due to trivial changes (17)
- plugins/otel/go.mod
- plugins/governance/go.mod
- plugins/mocker/go.mod
- plugins/semanticcache/go.mod
- plugins/maxim/go.mod
- ui/lib/utils.ts
- framework/go.mod
- plugins/logging/go.mod
- plugins/jsonparser/go.mod
- plugins/telemetry/go.mod
- plugins/compat/go.mod
- transports/go.mod
- ui/app/workspace/logs/sheets/logDetailsSheet.tsx
- ui/app/workspace/dashboard/components/overviewTab.tsx
- ui/app/workspace/logs/page.tsx
- ui/app/workspace/dashboard/components/charts/logVolumeChart.tsx
- ui/app/workspace/logs/views/logsVolumeChart.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- ui/app/workspace/logs/views/logsTable.tsx
- ui/app/workspace/dashboard/components/charts/chartCard.tsx
akshaydeo
force-pushed
the
04-19-logs_ux_updates
branch
2 times, most recently
from
5eb6c7f to
29a6be8
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/app/workspace/logs/views/columns.tsx`:
- Around line 319-326: Guard the optional row.original.provider before passing
it to RenderProviderIcon: check that provider (from row.original.provider) is
defined and only cast to ProviderIconType when present, otherwise render a safe
fallback (e.g., a default icon or null). Update the JSX around
RenderProviderIcon to conditionally render based on provider and avoid the
forced cast to ProviderIconType when provider is undefined.
- Around line 256-259: The Button components used for sorting toggles (the
Button wrapping column.toggleSorting with column.getIsSorted()), and the other
interactive Button instances in this file (the Buttons at the other noted
locations) are missing stable test selectors; add data-testid attributes
following the pattern data-testid="<entity>-<element>-<qualifier>" to each
interactive Button (e.g., for the sort toggle use something like
data-testid="logs-column-sort-<columnId>-toggle"), ensuring each test id
uniquely references the entity (logs), element (column), and qualifier
(sort/toggle/visibility/etc.) so tests can reliably select them; update every
Button instance referenced (including the ones around
column.toggleSorting/column.getIsSorted and the Buttons at the other noted
ranges) to include appropriate data-testid values.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f58c23ce-c3d8-4b41-8535-49dbfefb3bf4
📒 Files selected for processing (22)
framework/go.modplugins/compat/go.modplugins/governance/go.modplugins/jsonparser/go.modplugins/logging/go.modplugins/maxim/go.modplugins/mocker/go.modplugins/otel/go.modplugins/semanticcache/go.modplugins/telemetry/go.modtransports/go.modui/app/workspace/dashboard/components/charts/chartCard.tsxui/app/workspace/dashboard/components/charts/logVolumeChart.tsxui/app/workspace/dashboard/components/overviewTab.tsxui/app/workspace/logs/page.tsxui/app/workspace/logs/sheets/logDetailView.tsxui/app/workspace/logs/sheets/logDetailsSheet.tsxui/app/workspace/logs/views/columns.tsxui/app/workspace/logs/views/logsTable.tsxui/app/workspace/logs/views/logsVolumeChart.tsxui/lib/constants/icons.tsxui/lib/utils.ts
✅ Files skipped from review due to trivial changes (13)
- plugins/governance/go.mod
- framework/go.mod
- plugins/otel/go.mod
- plugins/maxim/go.mod
- plugins/telemetry/go.mod
- plugins/compat/go.mod
- plugins/jsonparser/go.mod
- ui/lib/utils.ts
- plugins/semanticcache/go.mod
- transports/go.mod
- ui/app/workspace/logs/page.tsx
- plugins/logging/go.mod
- plugins/mocker/go.mod
🚧 Files skipped from review as they are similar to previous changes (6)
- ui/app/workspace/dashboard/components/overviewTab.tsx
- ui/app/workspace/logs/sheets/logDetailsSheet.tsx
- ui/app/workspace/logs/views/logsTable.tsx
- ui/app/workspace/logs/views/logsVolumeChart.tsx
- ui/app/workspace/dashboard/components/charts/logVolumeChart.tsx
- ui/app/workspace/dashboard/components/charts/chartCard.tsx
akshaydeo
force-pushed
the
04-19-logs_ux_updates
branch
from
29a6be8 to
74778d1
Compare
Contributor
Author
Merge activity
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Improved UX for logs table and logs details view
Changes
Type of change
Affected areas
How to test
Describe the steps to validate this change. Include commands and expected outcomes.
If adding new configs or environment variables, document them here.
Screenshots/Recordings
If UI changes, add before/after screenshots or short clips.
Breaking changes
If yes, describe impact and migration instructions.
Related issues
Link related issues and discussions. Example: Closes #123
Security considerations
Note any security implications (auth, secrets, PII, sandboxing, etc.).
Checklist
docs/contributing/README.mdand followed the guidelines