refactor(dashboard): split compare-runs/history-tab functions to clear complexity ratchets

CompareRunsPanel (92 lines, max-lines-per-function) and HistoryTab (85 lines,
same rule) exceeded the 80-line function cap; compareRuns.ts's a2aEventsFrom
exceeded the cognitive-complexity cap (16 > 15). Extract pure/presentational
helpers (ComparePanelHeaderBar, SideErrorRow, ComparisonMetrics, a2aEventFrom,
HistoryStatusRows, refreshNowMsOnActionDone) with no behavior, DOM, i18n or
aria change.
This commit is contained in:
diegosouzapw
2026-09-07 08:24:28 -03:00
parent a05da56ac6
commit 02da7232b3
3 changed files with 168 additions and 87 deletions

View File

@@ -56,21 +56,30 @@ function isNonEmptyString(value: unknown): value is string {
return typeof value === "string" && value.length > 0;
}
/** One `reconstituteHistoricalTask` event (`{ timestamp: string; state: string; message?: string
* }`) → `RunEvent`, or `null` if the raw shape does not match. Split out of `a2aEventsFrom` only
* to keep that function's cognitive complexity under the ratchet — no behavior change; the same
* field checks run in the same order. */
function a2aEventFrom(raw: unknown): RunEvent | null {
if (!isRecord(raw)) return null;
const { state, message, timestamp } = raw;
if (!isNonEmptyString(state)) return null;
if (message !== undefined && typeof message !== "string") return null;
if (timestamp !== undefined && timestamp !== null && typeof timestamp !== "string") return null;
return {
label: typeof message === "string" ? message : state,
timestamp: typeof timestamp === "string" ? timestamp : null,
};
}
/** `{ timestamp: string; state: string; message?: string }` — reconstituteHistoricalTask's shape. */
function a2aEventsFrom(detail: Record<string, unknown>): RunEvent[] {
const events = detail.events;
if (!Array.isArray(events)) return [];
const out: RunEvent[] = [];
for (const raw of events) {
if (!isRecord(raw)) continue;
const { state, message, timestamp } = raw;
if (!isNonEmptyString(state)) continue;
if (message !== undefined && typeof message !== "string") continue;
if (timestamp !== undefined && timestamp !== null && typeof timestamp !== "string") continue;
out.push({
label: typeof message === "string" ? message : state,
timestamp: typeof timestamp === "string" ? timestamp : null,
});
const event = a2aEventFrom(raw);
if (event) out.push(event);
}
return out;
}

View File

@@ -338,6 +338,99 @@ function SideErrorCell({
);
}
/** Title bar (panel label + close button). Split out only to keep `CompareRunsPanel` under the
* max-lines-per-function ratchet — no behavior change. */
function ComparePanelHeaderBar({ t, onClose }: { t: Translate; onClose: () => void }) {
return (
<div className="flex items-center justify-between mb-2 min-w-[480px]">
<span className="text-xs font-semibold">{t("compareTitle")}</span>
<button
type="button"
data-testid="orchestration-compare-close"
aria-label={t("drawerClose")}
className="text-muted"
onClick={onClose}
>
</button>
</div>
);
}
/** Per-side fetch-failure row — renders only when at least one side's fetch failed (`SideErrorCell`
* itself still renders an empty, role-less cell for a side that succeeded, so the two columns stay
* grid-aligned). Split out only to keep `CompareRunsPanel` under the max-lines-per-function
* ratchet — no behavior change. */
function SideErrorRow({
leftError,
rightError,
t,
}: {
leftError: string | null;
rightError: string | null;
t: Translate;
}) {
if (!leftError && !rightError) return null;
return (
<div className="grid grid-cols-2 gap-2 min-w-[480px] mb-2">
<SideErrorCell error={leftError} t={t} testId="orchestration-compare-error-left" />
<SideErrorCell error={rightError} t={t} testId="orchestration-compare-error-right" />
</div>
);
}
/** Delta-column legend (Task A4 review fix, Minor #4) plus the Duration/Cost/Events metric rows
* — column order is selection order, not chronology, so nothing else on the panel says which side
* a positive delta favors. Split out only to keep `CompareRunsPanel` under the
* max-lines-per-function ratchet — no behavior change. */
function ComparisonMetrics({
t,
left,
right,
comparison,
eventsBothOk,
}: {
t: Translate;
left: HistoryItem;
right: HistoryItem;
comparison: RunComparison;
eventsBothOk: boolean;
}) {
return (
<div className="flex flex-col gap-1">
<div className="grid grid-cols-[70px_1fr_1fr_70px] gap-2 text-[9px] items-center min-w-[480px] text-muted">
<span />
<span />
<span />
<span data-testid="orchestration-compare-delta-legend" className="text-right">
{t("compareDeltaLegend")}
</span>
</div>
<MetricRow
metricKey="duration"
label={t("compareDuration")}
leftText={formatDuration(left.durationMs)}
rightText={formatDuration(right.durationMs)}
deltaText={formatDurationDelta(comparison.deltas.durationMs)}
/>
<MetricRow
metricKey="cost"
label={t("compareCost")}
leftText={formatCost(left.cost)}
rightText={formatCost(right.cost)}
deltaText={formatCostDelta(comparison.deltas.cost)}
/>
<MetricRow
metricKey="events"
label={t("compareEvents")}
leftText={eventsBothOk ? String(comparison.left.events.length) : "—"}
rightText={eventsBothOk ? String(comparison.right.events.length) : "—"}
deltaText={eventsBothOk ? formatEventDelta(comparison.deltas.eventCount) : "—"}
/>
</div>
);
}
export function CompareRunsPanel({
left,
right,
@@ -366,34 +459,14 @@ export function CompareRunsPanel({
data-testid="orchestration-history-compare-panel"
className="border border-border rounded p-2 overflow-x-auto overflow-y-auto max-h-[45vh] shrink-0"
>
<div className="flex items-center justify-between mb-2 min-w-[480px]">
<span className="text-xs font-semibold">{t("compareTitle")}</span>
<button
type="button"
data-testid="orchestration-compare-close"
aria-label={t("drawerClose")}
className="text-muted"
onClick={onClose}
>
</button>
</div>
<ComparePanelHeaderBar t={t} onClose={onClose} />
<div className="grid grid-cols-2 gap-2 min-w-[480px] mb-2">
<RunHeader item={left} t={t} testId="orchestration-compare-header-left" />
<RunHeader item={right} t={t} testId="orchestration-compare-header-right" />
</div>
{(leftState.error || rightState.error) && (
<div className="grid grid-cols-2 gap-2 min-w-[480px] mb-2">
<SideErrorCell error={leftState.error} t={t} testId="orchestration-compare-error-left" />
<SideErrorCell
error={rightState.error}
t={t}
testId="orchestration-compare-error-right"
/>
</div>
)}
<SideErrorRow leftError={leftState.error} rightError={rightState.error} t={t} />
{/* Informational only — fires on every mount where the two picks are not the same
(source, identity) pair, never in response to an error condition, so `role="status"`
@@ -405,40 +478,13 @@ export function CompareRunsPanel({
</div>
)}
<div className="flex flex-col gap-1">
{/* Delta-column legend (Task A4 review fix, Minor #4): column order is selection order,
not chronology, so nothing else on the panel says which side a positive delta favors.
Shares the metric rows' grid template and right-aligns like the delta cells below. */}
<div className="grid grid-cols-[70px_1fr_1fr_70px] gap-2 text-[9px] items-center min-w-[480px] text-muted">
<span />
<span />
<span />
<span data-testid="orchestration-compare-delta-legend" className="text-right">
{t("compareDeltaLegend")}
</span>
</div>
<MetricRow
metricKey="duration"
label={t("compareDuration")}
leftText={formatDuration(left.durationMs)}
rightText={formatDuration(right.durationMs)}
deltaText={formatDurationDelta(comparison.deltas.durationMs)}
/>
<MetricRow
metricKey="cost"
label={t("compareCost")}
leftText={formatCost(left.cost)}
rightText={formatCost(right.cost)}
deltaText={formatCostDelta(comparison.deltas.cost)}
/>
<MetricRow
metricKey="events"
label={t("compareEvents")}
leftText={eventsBothOk ? String(comparison.left.events.length) : "—"}
rightText={eventsBothOk ? String(comparison.right.events.length) : "—"}
deltaText={eventsBothOk ? formatEventDelta(comparison.deltas.eventCount) : "—"}
/>
</div>
<ComparisonMetrics
t={t}
left={left}
right={right}
comparison={comparison}
eventsBothOk={eventsBothOk}
/>
<TimelineRows comparison={comparison} t={t} />
<MemorySection comparison={comparison} t={t} />

View File

@@ -339,6 +339,47 @@ function HistoryGridTable({
);
}
/** Loading indicator + "no rows" empty state. Mutually exclusive by construction (the empty
* message only ever renders once loading has finished), same as the two conditionals this
* replaces. Extracted only to keep `HistoryTab` under the max-lines-per-function ratchet. */
function HistoryStatusRows({
isLoading,
hasNoRows,
t,
tCommon,
}: {
isLoading: boolean;
hasNoRows: boolean;
t: ReturnType<typeof useTranslations>;
tCommon: ReturnType<typeof useTranslations>;
}) {
return (
<>
{isLoading && (
<div role="status" aria-live="polite" className="text-xs text-muted">
{tCommon("loading")}
</div>
)}
{hasNoRows && !isLoading && <div className="text-xs text-muted p-4">{t("historyEmpty")}</div>}
</>
);
}
/** Re-samples `nowMs` from inside a real event-driven callback (never during render — the
* `nowMs` note on `HistoryTab` explains why) so the drawer's `onActionDone` refetches the grid
* over a new range without closing the drawer: the drawer renders its own success toast right
* after calling `onActionDone`, so unmounting here would throw the confirmation away and the
* operator would see a repeat/cancel silently do nothing. The updater only ever picks the larger
* of the sampled clock and `prev + 1`, so the range always changes (and the refetch always
* happens) even when two samples land in the same millisecond. Extracted only to keep
* `HistoryTab` under the max-lines-per-function ratchet — no behavior change. */
function refreshNowMsOnActionDone(setNowMs: (updater: (prev: number) => number) => void) {
return () => {
const sampled = Date.now();
setNowMs((prev) => (sampled > prev ? sampled : prev + 1));
};
}
export function HistoryTab() {
const t = useTranslations("orchestration");
// `common.loading` is an already-translated global key — the history namespace has no
@@ -408,15 +449,12 @@ export function HistoryTab() {
<FailedSourcesList failedSources={failedSources} t={t} />
{isLoading && (
<div role="status" aria-live="polite" className="text-xs text-muted">
{tCommon("loading")}
</div>
)}
{grid.rows.length === 0 && !isLoading && (
<div className="text-xs text-muted p-4">{t("historyEmpty")}</div>
)}
<HistoryStatusRows
isLoading={isLoading}
hasNoRows={grid.rows.length === 0}
t={t}
tCommon={tCommon}
/>
{grid.rows.length > 0 && (
<HistoryGridTable
@@ -441,22 +479,10 @@ export function HistoryTab() {
/>
)}
{/* `onActionDone` must NOT close the drawer: the drawer renders its own success toast
right after calling it, so unmounting here threw the confirmation away and the
operator saw a repeat/cancel silently do nothing. Re-sampling `nowMs` instead
keeps the drawer mounted (the toast lands) and refreshes the grid through the new
range — the same "refetch, don't close" contract `OrchestrationPageClient` uses.
`Date.now()` is sampled inside a real event-driven callback, never during render
(see the `nowMs` note above), and the updater is pure — it only picks the larger of
the sampled clock and `prev + 1`, so the range always changes (and the refetch
always happens) even when two samples land in the same millisecond. */}
<OrchestrationDrawer
node={selected}
onClose={() => setSelected(null)}
onActionDone={() => {
const sampled = Date.now();
setNowMs((prev) => (sampled > prev ? sampled : prev + 1));
}}
onActionDone={refreshNowMsOnActionDone(setNowMs)}
/>
</div>
);