Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { render, screen } from "@testing-library/react";
import { describe, expect, it } from "vitest";

import type { EntityDiff } from "@/utils/componentSpecDiff";

import { ComponentEditSummary } from "./ComponentEditSummary";

const empty: EntityDiff<{ name: string }> = {
lostEntities: [],
newEntities: [],
changedEntities: [],
};

const BREAKING = "Some inputs or outputs will be removed";

describe("ComponentEditSummary", () => {
it("shows the breaking-change warning and removed input when inputs are lost", () => {
render(
<ComponentEditSummary
inputDiff={{ ...empty, lostEntities: [{ name: "threshold" }] }}
outputDiff={empty}
/>,
);

expect(screen.getByText(new RegExp(BREAKING))).toBeInTheDocument();
expect(screen.getByText("Removed: threshold")).toBeInTheDocument();
});

it("omits the breaking-change warning when nothing is removed", () => {
render(
<ComponentEditSummary
inputDiff={{ ...empty, newEntities: [{ name: "max_rows" }] }}
outputDiff={empty}
/>,
);

expect(screen.queryByText(new RegExp(BREAKING))).not.toBeInTheDocument();
expect(screen.getByText("Added: max_rows")).toBeInTheDocument();
});
});
80 changes: 80 additions & 0 deletions src/components/shared/ComponentDiff/ComponentEditSummary.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import { DiffSection } from "@/components/shared/ComponentDiff/DiffSection";
import { TrimmedDigest } from "@/components/shared/ManageComponent/TrimmedDigest";
import { Icon } from "@/components/ui/icon";
import { BlockStack, InlineStack } from "@/components/ui/layout";
import { Text } from "@/components/ui/typography";
import type { EntityDiff } from "@/utils/componentSpecDiff";

interface ComponentEditSummaryProps {
/** Digest of the component currently on the task. */
currentDigest?: string;
/** Digest of the edited component being applied. */
newDigest?: string;
inputDiff: EntityDiff<{ name: string }>;
outputDiff: EntityDiff<{ name: string }>;
/**
* When false, the lost/new/changed `DiffSection`s are omitted (the digest
* transition + breaking warning still show). Used where a richer preview
* renders the per-port diff instead. Defaults to true.
*/
showDiffList?: boolean;
}

/**
* Summarizes what an edited component changes, in the same visual language as
* the component-upgrade / replace flows: a current → new digest transition, a
* breaking-change warning when inputs/outputs are removed, and the shared
* lost/new/changed `DiffSection`s. Shared by the legacy and v2 save modals.
*/
export function ComponentEditSummary({
currentDigest,
newDigest,
inputDiff,
outputDiff,
showDiffList = true,
}: ComponentEditSummaryProps) {
const hasBreakingChanges =
inputDiff.lostEntities.length > 0 || outputDiff.lostEntities.length > 0;
const showDigests =
!!currentDigest && !!newDigest && currentDigest !== newDigest;

return (
<BlockStack gap="3">
{showDigests && (
<InlineStack gap="1" blockAlign="center" wrap="nowrap">
<TrimmedDigest
digest={currentDigest}
className="text-muted-foreground"
/>
<Icon
name="ArrowRight"
size="xs"
className="shrink-0 text-muted-foreground"
/>
<TrimmedDigest digest={newDigest} />
</InlineStack>
)}

{hasBreakingChanges && (
<InlineStack gap="1" blockAlign="center" wrap="nowrap">
<Icon
name="TriangleAlert"
size="sm"
className="shrink-0 text-amber-500"
/>
<Text size="xs" tone="subdued">
Some inputs or outputs will be removed, and their connections will
be lost.
</Text>
</InlineStack>
)}

{showDiffList && (
<>
<DiffSection label="Input" diff={inputDiff} />
<DiffSection label="Output" diff={outputDiff} />
</>
)}
</BlockStack>
);
}
29 changes: 19 additions & 10 deletions src/components/shared/ComponentEditor/SaveActionsView.tsx
Original file line number Diff line number Diff line change
@@ -1,20 +1,27 @@
import type { ReactNode } from "react";

import { DiffSection } from "@/components/shared/ComponentDiff/DiffSection";
import { ComponentEditSummary } from "@/components/shared/ComponentDiff/ComponentEditSummary";
import { Button } from "@/components/ui/button";
import { Icon, type IconName } from "@/components/ui/icon";
import { BlockStack } from "@/components/ui/layout";
import { Text } from "@/components/ui/typography";
import { type EntityDiff, hasIODiff } from "@/utils/componentSpecDiff";
import type { EntityDiff } from "@/utils/componentSpecDiff";

type ChooseableAction = "update" | "import" | "place";

export interface SaveActionsViewProps {
taskName: string;
currentDigest?: string;
newDigest?: string;
inputDiff: EntityDiff<{ name: string }>;
outputDiff: EntityDiff<{ name: string }>;
/** Whether to offer "Place as a new task". */
allowPlace?: boolean;
/**
* When false, the lost/new/changed diff list is hidden (e.g. the v2 editor
* shows a ghost-diff preview card instead). Defaults to true.
*/
showDiffList?: boolean;
/** Extra content shown above the actions (e.g. v2 predicted issues + preview). */
children?: ReactNode;
onChoose: (action: ChooseableAction) => void;
Expand All @@ -29,27 +36,29 @@ export interface SaveActionsViewProps {
*/
export function SaveActionsView({
taskName,
currentDigest,
newDigest,
inputDiff,
outputDiff,
allowPlace = false,
showDiffList = true,
children,
onChoose,
}: SaveActionsViewProps) {
const showDiff = hasIODiff(inputDiff, outputDiff);

return (
<div className="mx-auto w-full max-w-xl overflow-y-auto p-4">
<BlockStack gap="4">
<Text tone="subdued">
Choose what to do with your changes to “{taskName}”.
</Text>

{showDiff && (
<BlockStack gap="2">
<DiffSection label="Input" diff={inputDiff} />
<DiffSection label="Output" diff={outputDiff} />
</BlockStack>
)}
<ComponentEditSummary
currentDigest={currentDigest}
newDigest={newDigest}
inputDiff={inputDiff}
outputDiff={outputDiff}
showDiffList={showDiffList}
/>

{children}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,39 @@ describe("replaceTaskComponentRef", () => {
expect(updatedGraphSpec.tasks.train.arguments).toEqual({});
});

it("seeds default arguments for newly added inputs", () => {
const graphSpec = baseGraphSpec();
const refWithNewInput: ComponentReference = {
name: "Chicago Taxi Trips dataset",
digest: "new-digest",
spec: {
name: "Chicago Taxi Trips dataset",
inputs: [
{ name: "Limit", type: "Integer", default: "1000" },
{ name: "Select", type: "String" },
{ name: "Format", type: "String", default: "csv" },
{ name: "NoDefault", type: "String" },
],
outputs: [{ name: "Table" }],
implementation: { container: { image: "alpine/curl" } },
},
};

const { updatedGraphSpec } = replaceTaskComponentRef(
"dataset",
refWithNewInput,
graphSpec,
);

// New input with a default is seeded; one without a default is not; and
// existing arguments are preserved.
expect(updatedGraphSpec.tasks.dataset.arguments).toEqual({
Limit: { graphInput: { inputName: "Input" } },
Select: "tips,trip_seconds",
Format: "csv",
});
});

it("returns the graph unchanged when the task does not exist", () => {
const graphSpec = baseGraphSpec();
const fixedRef = { ...taxiRef("fixed"), digest: "new-digest" };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ import { deepClone } from "@/utils/deepClone";
* renaming would re-key the task to a new unique id derived from the component
* name and is surprising for an in-place edit.
*
* Wiring is preserved with two safe exceptions, mirroring the upgrade flow:
* Wiring is reconciled the same way the upgrade flow's `replaceTask` does:
* - argument bindings on this task for inputs that no longer exist in the
* edited component are dropped (and reported via `lostInputs`);
* - newly added inputs are seeded with their default argument value;
* - argument bindings on downstream tasks that consume an output this
* component no longer produces are dropped, to avoid dangling references.
*
Expand All @@ -41,9 +42,9 @@ export const replaceTaskComponentRef = (
}

const oldInputs = task.componentRef.spec?.inputs ?? [];
const newInputNames = new Set(
(newComponentRef.spec?.inputs ?? []).map((input) => input.name),
);
const oldInputNames = new Set(oldInputs.map((input) => input.name));
const newInputs = newComponentRef.spec?.inputs ?? [];
const newInputNames = new Set(newInputs.map((input) => input.name));
const newOutputNames = new Set(
(newComponentRef.spec?.outputs ?? []).map((output) => output.name),
);
Expand All @@ -68,6 +69,21 @@ export const replaceTaskComponentRef = (
);
}

// Seed default argument values for newly added inputs (parity with the
// upgrade flow's `replaceTask`).
const newlyAddedInputs = newInputs.filter(
(input) => !oldInputNames.has(input.name),
);
if (newlyAddedInputs.some((input) => input.default !== undefined)) {
const args = task.arguments ?? {};
for (const input of newlyAddedInputs) {
if (input.default !== undefined && args[input.name] === undefined) {
args[input.name] = input.default;
}
}
task.arguments = args;
}

// Drop downstream argument bindings that consume an output this component no
// longer produces. Outputs that still exist keep working because the taskId
// is unchanged.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type { Bounds } from "@/components/shared/ReactFlow/FlowCanvas/utils/geom
import { replaceTaskComponentRef } from "@/components/shared/ReactFlow/FlowCanvas/utils/replaceTaskComponentRef";
import { useNodesOverlay } from "@/components/shared/ReactFlow/NodesOverlay/NodesOverlayProvider";
import useToastNotification from "@/hooks/useToastNotification";
import { useAnalytics } from "@/providers/AnalyticsProvider";
import { useComponentSpec } from "@/providers/ComponentSpecProvider";
import { extractPositionFromAnnotations } from "@/utils/annotations";
import {
Expand All @@ -16,6 +17,7 @@ import {
type TaskSpec,
} from "@/utils/componentSpec";
import { diffComponentIO } from "@/utils/componentSpecDiff";
import { componentMetadata } from "@/utils/componentTracking";
import { DEFAULT_NODE_DIMENSIONS } from "@/utils/constants";
import { taskIdToNodeId } from "@/utils/nodes/nodeIdUtils";
import { tracking } from "@/utils/tracking";
Expand All @@ -39,6 +41,7 @@ export const EditComponentButton = ({
}: EditComponentButtonProps) => {
const [isEditDialogOpen, setIsEditDialogOpen] = useState(false);
const notify = useToastNotification();
const { track } = useAnalytics();
const { currentGraphSpec, updateGraphSpec } = useComponentSpec();
const { getNodes } = useReactFlow();
const { fitNodeIntoView, selectNode, notifyNode } = useNodesOverlay();
Expand All @@ -62,6 +65,12 @@ export const EditComponentButton = ({

updateGraphSpec(updatedGraphSpec);

track("pipeline_editor.component.edited", {
...componentMetadata(hydratedComponent, "user"),
action: "update",
lost_inputs_count: lostInputs.length,
});

if (lostInputs.length > 0) {
const inputNames = lostInputs.map((input) => input.name).join(", ");
notify(
Expand Down Expand Up @@ -147,6 +156,12 @@ export const EditComponentButton = ({
}

updateGraphSpec(updatedWrapper.implementation.graph);

track("pipeline_editor.component.edited", {
...componentMetadata(hydratedComponent, "user"),
action: "place",
});

notify("Task added", "success");

// The new node mounts asynchronously; wait for it, then reveal + spotlight.
Expand Down Expand Up @@ -194,6 +209,8 @@ export const EditComponentButton = ({
return (
<SaveActionsView
taskName={componentRef.name}
currentDigest={editedTask?.componentRef.digest}
newDigest={hydratedComponent.digest}
inputDiff={inputDiff}
outputDiff={outputDiff}
allowPlace
Expand Down
Loading