fix: address console review findings

This commit is contained in:
lda
2026-07-03 08:27:36 +07:00 Verified
parent 47ac422965
commit b0b2cba7ab
23 changed files with 351 additions and 107 deletions
@@ -150,4 +150,55 @@ describe("LifecycleExplorer", () => {
render(<LifecycleExplorer controller={controller} />);
expect(screen.getAllByText(/no artifacts/i)[0]).toBeVisible();
});
it("keeps previous records visible while showing loading and error status", () => {
const controller = createMockController({
artifactList: {
phase: "loading",
previous: {
items: [
{
key: "report@1",
artifactId: "report",
version: 1,
kind: "workflow",
displayName: "Report",
description: null,
outcomes: ["ok"],
requiredSources: ["local.report"],
diagnosticCount: 0,
},
],
total: 1,
nextCursor: null,
},
},
runList: {
phase: "error",
message: "network down",
previous: {
items: [
{
runId: "run_1",
deploymentId: "report.default",
artifactId: "report",
artifactVersion: 1,
status: "completed",
resumeReadiness: "not_needed",
diagnosticCount: 0,
},
],
total: 1,
nextCursor: null,
},
},
});
render(<LifecycleExplorer controller={controller} />);
expect(screen.getByText(/loading artifacts/i)).toBeVisible();
expect(screen.getByRole("alert")).toHaveTextContent(/could not load runs: network down/i);
expect(screen.getByRole("option", { name: /Report version 1/i })).toBeVisible();
expect(screen.getByRole("option", { name: /run_1 completed/i })).toBeVisible();
});
});
@@ -6,6 +6,7 @@ import { buildWorkflowGraph } from "../graph/graph-model.js";
import { WorkflowGraph } from "../graph/WorkflowGraph.js";
import { buildTraceFrames } from "../execution/trace-model.js";
import { ExecutionView } from "../execution/ExecutionView.js";
import type { LoadState } from "./state.js";
type LifecycleExplorerProps = {
readonly controller: LifecycleExplorerController;
@@ -13,18 +14,24 @@ type LifecycleExplorerProps = {
type FocusMode = "lifecycle" | "graph" | "execution" | "raw";
const loadedItems = <T,>(
state: LoadState<{ readonly items: ReadonlyArray<T> }>,
): ReadonlyArray<T> =>
state.phase === "loaded" ? state.value.items : state.phase === "loading" || state.phase === "error" ? state.previous?.items ?? [] : [];
const listStatus = <T,>(label: string, state: LoadState<T>) => {
if (state.phase === "loading") return <p role="status">Loading {label}...</p>;
if (state.phase === "error") return <p role="alert">Could not load {label}: {state.message}</p>;
return null;
};
export const LifecycleExplorer = ({ controller }: LifecycleExplorerProps) => {
const { state } = controller;
const [focusMode, setFocusMode] = useState<FocusMode>("lifecycle");
const artifacts =
state.artifactList.phase === "loaded" ? state.artifactList.value.items : [];
const deployments =
state.deploymentList.phase === "loaded"
? state.deploymentList.value.items
: [];
const runs =
state.runList.phase === "loaded" ? state.runList.value.items : [];
const artifacts = loadedItems(state.artifactList);
const deployments = loadedItems(state.deploymentList);
const runs = loadedItems(state.runList);
const graphModel = useMemo(() => {
if (!state.artifactDetail?.plan) return null;
@@ -74,6 +81,16 @@ export const LifecycleExplorer = ({ controller }: LifecycleExplorerProps) => {
{focusMode === "lifecycle" && (
<div className="lifecycle-content">
<div className="lifecycle-status">
{listStatus("artifacts", state.artifactList)}
{listStatus("deployments", state.deploymentList)}
{listStatus("runs", state.runList)}
{state.errors.map((error) => (
<p role="alert" key={`${error.operation}-${error.timestamp}`}>
{error.operation}: {error.message}
</p>
))}
</div>
<RecordColumns
artifacts={artifacts}
deployments={deployments}
@@ -1,9 +1,9 @@
import type { ArtifactSummary, DeploymentSummary, RunSummary } from "./models.js";
type RecordColumnsProps = {
readonly artifacts: ArtifactSummary[];
readonly deployments: DeploymentSummary[];
readonly runs: RunSummary[];
readonly artifacts: ReadonlyArray<ArtifactSummary>;
readonly deployments: ReadonlyArray<DeploymentSummary>;
readonly runs: ReadonlyArray<RunSummary>;
readonly selectedArtifactId: string | null;
readonly selectedDeploymentId: string | null;
readonly selectedRunId: string | null;
@@ -1,4 +1,4 @@
import { useReducer, useEffect, useRef, useCallback } from "react";
import { useReducer, useEffect, useRef, useCallback, type MutableRefObject } from "react";
import { callOperation } from "../connection/api.js";
import type { OperationName } from "../connection/contracts.js";
import {
@@ -26,15 +26,7 @@ export type LifecycleExplorerController = {
export const useLifecycleExplorer = (
target: string | null,
recordEvidence: (record: {
id: string;
operation: string;
label: string;
equivalentCli: string;
request: unknown;
response: unknown;
durationMs: number;
}) => void,
recordEvidence: (record: EvidenceRecord) => void,
): LifecycleExplorerController => {
const [state, dispatch] = useReducer(lifecycleReducer, initialLifecycleState);
const generationRef = useRef(0);
@@ -49,8 +41,9 @@ export const useLifecycleExplorer = (
operation: OperationName,
params: unknown,
generation: number,
checkGenerationRef: React.MutableRefObject<number>,
checkGenerationRef: MutableRefObject<number>,
onSuccess: (interpreted: unknown) => void,
onFailure?: (message: string) => void,
) => {
if (!target) return;
try {
@@ -83,6 +76,7 @@ export const useLifecycleExplorer = (
});
}
} else {
onFailure?.(result.error.message);
const seq = evidenceSeqRef.current++;
const record: EvidenceRecord = {
id: `${operation}-${seq}`,
@@ -106,11 +100,14 @@ export const useLifecycleExplorer = (
});
}
} catch (rpcError) {
if (generation !== checkGenerationRef.current) return;
const message = rpcError instanceof Error ? rpcError.message : String(rpcError);
onFailure?.(message);
dispatch({
type: "pushError",
error: {
operation,
message: rpcError instanceof Error ? rpcError.message : String(rpcError),
message,
timestamp: Date.now(),
},
});
@@ -122,20 +119,32 @@ export const useLifecycleExplorer = (
useEffect(() => {
if (!target) return;
generationRef.current++;
artifactGenerationRef.current++;
deploymentGenerationRef.current++;
runGenerationRef.current++;
const generation = generationRef.current;
rawEvidenceRef.current = [];
dispatch({ type: "targetChanged" });
dispatch({ type: "setArtifactListPhase", phase: "loading" });
dispatch({ type: "setDeploymentListPhase", phase: "loading" });
dispatch({ type: "setRunListPhase", phase: "loading" });
executeOperation("workflow.artifacts.list", { limit: 50 }, generation, generationRef, (interpreted) => {
dispatch({ type: "setArtifactListPhase", phase: "loaded", value: decodeArtifactList(interpreted) });
}, (message) => {
dispatch({ type: "setArtifactListPhase", phase: "error", message });
});
executeOperation("workflow.deployments.list", {}, generation, generationRef, (interpreted) => {
dispatch({ type: "setDeploymentListPhase", phase: "loaded", value: decodeDeploymentList(interpreted) });
}, (message) => {
dispatch({ type: "setDeploymentListPhase", phase: "error", message });
});
executeOperation("workflow.runs.list", { limit: 50 }, generation, generationRef, (interpreted) => {
dispatch({ type: "setRunListPhase", phase: "loaded", value: decodeRunList(interpreted) });
}, (message) => {
dispatch({ type: "setRunListPhase", phase: "error", message });
});
}, [target, executeOperation]);
@@ -223,28 +232,42 @@ export const useLifecycleExplorer = (
const refresh = useCallback(() => {
if (!target) return;
generationRef.current++;
const generation = generationRef.current;
executeOperation("workflow.artifacts.list", { limit: 50 }, generation, generationRef, (interpreted) => {
artifactGenerationRef.current++;
deploymentGenerationRef.current++;
runGenerationRef.current++;
const artifactGeneration = artifactGenerationRef.current;
const deploymentGeneration = deploymentGenerationRef.current;
const runGeneration = runGenerationRef.current;
dispatch({ type: "setArtifactListPhase", phase: "loading" });
dispatch({ type: "setDeploymentListPhase", phase: "loading" });
dispatch({ type: "setRunListPhase", phase: "loading" });
executeOperation("workflow.artifacts.list", { limit: 50 }, artifactGeneration, artifactGenerationRef, (interpreted) => {
dispatch({ type: "setArtifactListPhase", phase: "loaded", value: decodeArtifactList(interpreted) });
}, (message) => {
dispatch({ type: "setArtifactListPhase", phase: "error", message });
});
executeOperation("workflow.deployments.list", {}, generation, generationRef, (interpreted) => {
executeOperation("workflow.deployments.list", {}, deploymentGeneration, deploymentGenerationRef, (interpreted) => {
dispatch({ type: "setDeploymentListPhase", phase: "loaded", value: decodeDeploymentList(interpreted) });
}, (message) => {
dispatch({ type: "setDeploymentListPhase", phase: "error", message });
});
executeOperation("workflow.runs.list", { limit: 50 }, generation, generationRef, (interpreted) => {
executeOperation("workflow.runs.list", { limit: 50 }, runGeneration, runGenerationRef, (interpreted) => {
dispatch({ type: "setRunListPhase", phase: "loaded", value: decodeRunList(interpreted) });
}, (message) => {
dispatch({ type: "setRunListPhase", phase: "error", message });
});
}, [target, executeOperation]);
const loadMoreArtifacts = useCallback(() => {
const current = state.artifactList;
if (current.phase !== "loaded" || !current.value.nextCursor || !target) return;
generationRef.current++;
const generation = generationRef.current;
artifactGenerationRef.current++;
const generation = artifactGenerationRef.current;
executeOperation(
"workflow.artifacts.list",
{ cursor: current.value.nextCursor, limit: 50 },
generation,
generationRef,
artifactGenerationRef,
(interpreted) => {
dispatch({ type: "appendArtifactList", value: decodeArtifactList(interpreted) });
},
@@ -254,13 +277,13 @@ export const useLifecycleExplorer = (
const loadMoreRuns = useCallback(() => {
const current = state.runList;
if (current.phase !== "loaded" || !current.value.nextCursor || !target) return;
generationRef.current++;
const generation = generationRef.current;
runGenerationRef.current++;
const generation = runGenerationRef.current;
executeOperation(
"workflow.runs.list",
{ cursor: current.value.nextCursor, limit: 50 },
generation,
generationRef,
runGenerationRef,
(interpreted) => {
dispatch({ type: "appendRunList", value: decodeRunList(interpreted) });
},