From 7d1264f8d58ee3db6c367e6660e352c747ec6f6e Mon Sep 17 00:00:00 2001 From: Morgan Wowk Date: Fri, 5 Jun 2026 13:44:39 -0700 Subject: [PATCH] Lineage-aware Upgrade panel: subgraph recursion + off-mainline grouping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three complementary improvements to the Upgrade feature that make it lineage- aware: - Recurse subgraphs in collectUsedComponentReferencesFromV2Spec and the candidate-building walk so tasks nested inside group tasks are now considered for upgrades (they were silently skipped before) - Attach the task's lineage originId to each UpgradeCandidate; add a secondary walk that finds 'edited-off-mainline' tasks — instances whose digests diverged via local edits so they fell outside the published supersession chain, but whose lineage still traces back to a component family that has a catalog upgrade available. These are surfaced with an 'Edited locally' badge. - Group candidates by lineage origin in the panel when meaningful (multiple tasks share an origin, or any off-mainline candidate present). A component- name header separates each family; unrelated candidates render flat as before. groupCandidatesByOrigin returns null to avoid visual noise when no grouping adds value. --- .../UpgradeComponentsContent.tsx | 74 ++++++++++++--- .../components/UpgradeCandidateRow.tsx | 24 ++++- .../hooks/useUpgradeCandidatesFromOutdated.ts | 94 +++++++++++++++---- .../components/UpgradeComponents/types.ts | 8 ++ ...tUsedComponentReferencesFromV2Spec.test.ts | 39 ++++++++ ...ollectUsedComponentReferencesFromV2Spec.ts | 16 +++- .../utils/groupCandidatesByOrigin.test.ts | 78 +++++++++++++++ .../utils/groupCandidatesByOrigin.ts | 62 ++++++++++++ 8 files changed, 353 insertions(+), 42 deletions(-) create mode 100644 src/routes/v2/pages/Editor/components/UpgradeComponents/utils/collectUsedComponentReferencesFromV2Spec.test.ts create mode 100644 src/routes/v2/pages/Editor/components/UpgradeComponents/utils/groupCandidatesByOrigin.test.ts create mode 100644 src/routes/v2/pages/Editor/components/UpgradeComponents/utils/groupCandidatesByOrigin.ts diff --git a/src/routes/v2/pages/Editor/components/UpgradeComponents/UpgradeComponentsContent.tsx b/src/routes/v2/pages/Editor/components/UpgradeComponents/UpgradeComponentsContent.tsx index e5d22c66d..b9ab0a983 100644 --- a/src/routes/v2/pages/Editor/components/UpgradeComponents/UpgradeComponentsContent.tsx +++ b/src/routes/v2/pages/Editor/components/UpgradeComponents/UpgradeComponentsContent.tsx @@ -4,6 +4,7 @@ import { useState } from "react"; import { BlockStack } from "@/components/ui/layout"; import { VerticalResizeHandle } from "@/components/ui/resize-handle"; import { Separator } from "@/components/ui/separator"; +import { Text } from "@/components/ui/typography"; import { useTaskActions } from "@/routes/v2/pages/Editor/store/actions/useTaskActions"; import { useSpec } from "@/routes/v2/shared/providers/SpecContext"; import { useOptionalWindowContext } from "@/routes/v2/shared/windows/ContentWindowStateContext"; @@ -18,9 +19,61 @@ import { useSelectionSet } from "./hooks/useSelectionSet"; import { useUpgradeCandidatesFromOutdated } from "./hooks/useUpgradeCandidatesFromOutdated"; import { useUpgradePreviewOverlay } from "./hooks/useUpgradePreviewOverlay"; import { candidateHasIssues, type UpgradeCandidate } from "./types"; +import { groupCandidatesByOrigin } from "./utils/groupCandidatesByOrigin"; const DEFAULT_LEFT_PANEL_WIDTH = 340; +/** Flat or grouped list of candidates depending on whether grouping is helpful. */ +function CandidateList({ + candidates, + selection, + focusedId, + onToggle, + onFocus, +}: { + candidates: UpgradeCandidate[]; + selection: Set; + focusedId: string | null; + onToggle: (id: string, checked: boolean) => void; + onFocus: (id: string) => void; +}) { + const groups = groupCandidatesByOrigin(candidates); + + const renderRow = (candidate: UpgradeCandidate) => ( + onToggle(candidate.taskId, checked)} + onSelect={() => onFocus(candidate.taskId)} + /> + ); + + if (!groups) { + return ( + {candidates.map(renderRow)} + ); + } + + return ( + + {groups.map((group) => ( + + {group.componentName && ( +
+ + {group.componentName} + +
+ )} + {group.candidates.map(renderRow)} + + ))} + + ); +} + type UpgradeComponentsDataSource = "real" | "mock"; interface UpgradeComponentsContentProps { @@ -83,20 +136,13 @@ const UpgradeComponentsInner = observer(function UpgradeComponentsInner({ style={{ width: DEFAULT_LEFT_PANEL_WIDTH }} >
- - {candidates.map((candidate) => ( - - toggle(candidate.taskId, checked) - } - onSelect={() => setFocusedId(candidate.taskId)} - /> - ))} - +
diff --git a/src/routes/v2/pages/Editor/components/UpgradeComponents/components/UpgradeCandidateRow.tsx b/src/routes/v2/pages/Editor/components/UpgradeComponents/components/UpgradeCandidateRow.tsx index c661aa579..bfa213c8b 100644 --- a/src/routes/v2/pages/Editor/components/UpgradeComponents/components/UpgradeCandidateRow.tsx +++ b/src/routes/v2/pages/Editor/components/UpgradeComponents/components/UpgradeCandidateRow.tsx @@ -87,10 +87,26 @@ export function UpgradeCandidateRow({ blockAlign="center" data-testid="upgrade-candidate-row-digest" > - - {truncateDigest(candidate.currentDigest)} - - + {candidate.isEditedOffMainline ? ( + + Edited locally + + ) : ( + <> + + {truncateDigest(candidate.currentDigest)} + + + + )} {truncateDigest(candidate.newComponentRef.digest ?? "")} diff --git a/src/routes/v2/pages/Editor/components/UpgradeComponents/hooks/useUpgradeCandidatesFromOutdated.ts b/src/routes/v2/pages/Editor/components/UpgradeComponents/hooks/useUpgradeCandidatesFromOutdated.ts index b33785273..7a04657c2 100644 --- a/src/routes/v2/pages/Editor/components/UpgradeComponents/hooks/useUpgradeCandidatesFromOutdated.ts +++ b/src/routes/v2/pages/Editor/components/UpgradeComponents/hooks/useUpgradeCandidatesFromOutdated.ts @@ -1,5 +1,5 @@ import { useOutdatedComponents } from "@/components/shared/ManageComponent/hooks/useOutdatedComponents"; -import type { ComponentReference } from "@/models/componentSpec"; +import type { ComponentReference, ComponentSpec } from "@/models/componentSpec"; import type { UpgradeCandidate } from "@/routes/v2/pages/Editor/components/UpgradeComponents/types"; import { buildUpgradeCandidateFromResolved } from "@/routes/v2/pages/Editor/components/UpgradeComponents/utils/buildUpgradeCandidateFromResolved"; import { @@ -7,10 +7,15 @@ import { EMPTY_USED_COMPONENTS, } from "@/routes/v2/pages/Editor/components/UpgradeComponents/utils/collectUsedComponentReferencesFromV2Spec"; import { useSpec } from "@/routes/v2/shared/providers/SpecContext"; +import { LINEAGE_ORIGIN_ANNOTATION } from "@/utils/lineage"; /** * Upgrade candidates derived from {@link useOutdatedComponents} for the * current graph's used components, aligned with {@link replaceTask} previews. + * Recurses through subgraphs so nested tasks are included. Also surfaces + * "edited-off-mainline" tasks: instances whose digests were changed locally + * so they fell out of the catalog's supersession chain, but whose lineage + * still traces back to a component family that has an upgrade available. * * The window is only opened from the V2 editor, so {@link useSpec} should * always return a non-null spec here; the null branch is defensive and @@ -35,27 +40,78 @@ export function useUpgradeCandidatesFromOutdated(): UpgradeCandidate[] { } const candidates: UpgradeCandidate[] = []; - for (const task of spec.tasks) { - const digest = task.componentRef.digest; - if (!digest) { - continue; - } - const newComponentRef = digestToMrc.get(digest); - if (!newComponentRef) { - continue; + const seenTaskIds = new Set(); + // originId → newRef for the off-mainline secondary pass. + const originIdToNewRef = new Map(); + + // Primary walk: tasks whose digest is directly in the supersession chain. + // `s` is each task's immediately-owning spec so lost-binding checks are scoped + // to the bindings at that nesting level (not the root). + function walkPrimary(s: ComponentSpec) { + for (const task of s.tasks) { + const digest = task.componentRef.digest; + const newComponentRef = digest ? digestToMrc.get(digest) : undefined; + const originId = task.annotations.get( + LINEAGE_ORIGIN_ANNOTATION, + )?.originId; + + if (newComponentRef && digest) { + candidates.push({ + ...buildUpgradeCandidateFromResolved( + task.$id, + task.name, + digest, + task.resolvedComponentSpec, + newComponentRef, + s, + ), + originId, + }); + seenTaskIds.add(task.$id); + if (originId) originIdToNewRef.set(originId, newComponentRef); + } + + if (task.subgraphSpec) walkPrimary(task.subgraphSpec); } + } - candidates.push( - buildUpgradeCandidateFromResolved( - task.$id, - task.name, - digest, - task.resolvedComponentSpec, - newComponentRef, - spec, - ), - ); + walkPrimary(spec); + + // Secondary walk: tasks that were locally edited (digest off-chain) but share + // a lineage origin with a family that does have an upgrade available. + function walkOffMainline(s: ComponentSpec) { + for (const task of s.tasks) { + if (!seenTaskIds.has(task.$id)) { + const originId = task.annotations.get( + LINEAGE_ORIGIN_ANNOTATION, + )?.originId; + const newComponentRef = originId + ? originIdToNewRef.get(originId) + : undefined; + + if (originId && newComponentRef) { + const digest = task.componentRef.digest ?? ""; + candidates.push({ + ...buildUpgradeCandidateFromResolved( + task.$id, + task.name, + digest, + task.resolvedComponentSpec, + newComponentRef, + s, + ), + originId, + isEditedOffMainline: true, + }); + seenTaskIds.add(task.$id); + } + } + + if (task.subgraphSpec) walkOffMainline(task.subgraphSpec); + } } + walkOffMainline(spec); + return candidates; } diff --git a/src/routes/v2/pages/Editor/components/UpgradeComponents/types.ts b/src/routes/v2/pages/Editor/components/UpgradeComponents/types.ts index 58fa9583e..d94846ffd 100644 --- a/src/routes/v2/pages/Editor/components/UpgradeComponents/types.ts +++ b/src/routes/v2/pages/Editor/components/UpgradeComponents/types.ts @@ -26,6 +26,14 @@ export interface UpgradeCandidate { outputDiff: EntityDiff; lostBindings: LostBinding[]; predictedIssues: ValidationIssue[]; + /** Lineage origin id shared by all tasks descended from the same component. */ + originId?: string; + /** + * True when this task was locally edited so its digest fell outside the + * published supersession chain, but its lineage still traces back to a + * component family that has a catalog upgrade available. + */ + isEditedOffMainline?: boolean; } export function candidateHasIssues(candidate: UpgradeCandidate): boolean { diff --git a/src/routes/v2/pages/Editor/components/UpgradeComponents/utils/collectUsedComponentReferencesFromV2Spec.test.ts b/src/routes/v2/pages/Editor/components/UpgradeComponents/utils/collectUsedComponentReferencesFromV2Spec.test.ts new file mode 100644 index 000000000..144277be4 --- /dev/null +++ b/src/routes/v2/pages/Editor/components/UpgradeComponents/utils/collectUsedComponentReferencesFromV2Spec.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from "vitest"; + +import { ComponentSpec } from "@/models/componentSpec/entities/componentSpec"; +import { Task } from "@/models/componentSpec/entities/task"; + +import { collectUsedComponentReferencesFromV2Spec } from "./collectUsedComponentReferencesFromV2Spec"; + +const task = ($id: string, digest: string, subgraphSpec?: ComponentSpec) => + new Task({ $id, name: $id, componentRef: { digest }, subgraphSpec }); + +describe("collectUsedComponentReferencesFromV2Spec", () => { + it("collects unique digests from root tasks", () => { + const spec = new ComponentSpec({ + name: "Root", + tasks: [task("a", "d1"), task("b", "d2"), task("c", "d1")], + }); + + const refs = collectUsedComponentReferencesFromV2Spec(spec); + expect(refs.map((r) => r.digest).sort()).toEqual(["d1", "d2"]); + }); + + it("recurses into subgraphs and collects their digests too", () => { + const sub = new ComponentSpec({ + name: "Sub", + tasks: [task("nested", "d-nested")], + }); + const spec = new ComponentSpec({ + name: "Root", + tasks: [task("root", "d-root"), task("group", "d-group", sub)], + }); + + const refs = collectUsedComponentReferencesFromV2Spec(spec); + expect(refs.map((r) => r.digest).sort()).toEqual([ + "d-group", + "d-nested", + "d-root", + ]); + }); +}); diff --git a/src/routes/v2/pages/Editor/components/UpgradeComponents/utils/collectUsedComponentReferencesFromV2Spec.ts b/src/routes/v2/pages/Editor/components/UpgradeComponents/utils/collectUsedComponentReferencesFromV2Spec.ts index 18bf920c0..59ebdacc9 100644 --- a/src/routes/v2/pages/Editor/components/UpgradeComponents/utils/collectUsedComponentReferencesFromV2Spec.ts +++ b/src/routes/v2/pages/Editor/components/UpgradeComponents/utils/collectUsedComponentReferencesFromV2Spec.ts @@ -4,19 +4,25 @@ export const EMPTY_USED_COMPONENTS: ComponentReference[] = []; /** * Unique component references used by tasks in the current V2 editor spec - * (deduped by digest). Mirrors {@link fetchUsedComponents} for graph tasks. + * (deduped by digest). Recurses through subgraphs so tasks nested inside + * a group task are also considered for upgrades. + * Mirrors {@link fetchUsedComponents} for graph tasks. */ export function collectUsedComponentReferencesFromV2Spec( spec: ComponentSpec, ): ComponentReference[] { const usedComponentsMap = new Map(); - for (const task of spec.tasks) { - const key = task.componentRef.digest; - if (key && !usedComponentsMap.has(key)) { - usedComponentsMap.set(key, { ...task.componentRef }); + function walk(s: ComponentSpec) { + for (const task of s.tasks) { + const key = task.componentRef.digest; + if (key && !usedComponentsMap.has(key)) { + usedComponentsMap.set(key, { ...task.componentRef }); + } + if (task.subgraphSpec) walk(task.subgraphSpec); } } + walk(spec); return Array.from(usedComponentsMap.values()); } diff --git a/src/routes/v2/pages/Editor/components/UpgradeComponents/utils/groupCandidatesByOrigin.test.ts b/src/routes/v2/pages/Editor/components/UpgradeComponents/utils/groupCandidatesByOrigin.test.ts new file mode 100644 index 000000000..4761022d4 --- /dev/null +++ b/src/routes/v2/pages/Editor/components/UpgradeComponents/utils/groupCandidatesByOrigin.test.ts @@ -0,0 +1,78 @@ +import { describe, expect, it } from "vitest"; + +import type { UpgradeCandidate } from "@/routes/v2/pages/Editor/components/UpgradeComponents/types"; + +import { groupCandidatesByOrigin } from "./groupCandidatesByOrigin"; + +function candidate( + taskId: string, + opts: { originId?: string; isEditedOffMainline?: boolean } = {}, +): UpgradeCandidate { + return { + taskId, + taskName: taskId, + currentDigest: "old", + newComponentRef: { name: "Component", digest: "new" }, + inputDiff: { lostEntities: [], newEntities: [], changedEntities: [] }, + outputDiff: { lostEntities: [], newEntities: [], changedEntities: [] }, + lostBindings: [], + predictedIssues: [], + ...opts, + }; +} + +describe("groupCandidatesByOrigin", () => { + it("returns null when all candidates have different origins (no meaningful grouping)", () => { + const candidates = [ + candidate("a", { originId: "https://x/a.yaml" }), + candidate("b", { originId: "https://x/b.yaml" }), + ]; + expect(groupCandidatesByOrigin(candidates)).toBeNull(); + }); + + it("returns null when no candidates have an originId", () => { + expect( + groupCandidatesByOrigin([candidate("a"), candidate("b")]), + ).toBeNull(); + }); + + it("groups candidates sharing the same originId", () => { + const candidates = [ + candidate("a", { originId: "https://x/c.yaml" }), + candidate("b", { originId: "https://x/c.yaml" }), + candidate("c", { originId: "https://x/other.yaml" }), + ]; + + const groups = groupCandidatesByOrigin(candidates); + expect(groups).not.toBeNull(); + expect(groups).toHaveLength(2); + expect(groups![0].candidates.map((c) => c.taskId)).toEqual(["a", "b"]); + expect(groups![0].componentName).toBe("Component"); + }); + + it("triggers grouping when any candidate is edited-off-mainline", () => { + const candidates = [ + candidate("a", { originId: "https://x/c.yaml" }), + candidate("b", { + originId: "https://x/c.yaml", + isEditedOffMainline: true, + }), + ]; + + const groups = groupCandidatesByOrigin(candidates); + expect(groups).not.toBeNull(); + }); + + it("places candidates without originId in an ungrouped section at the end", () => { + const candidates = [ + candidate("a", { originId: "https://x/c.yaml" }), + candidate("b", { originId: "https://x/c.yaml" }), + candidate("c"), + ]; + + const groups = groupCandidatesByOrigin(candidates)!; + const last = groups[groups.length - 1]; + expect(last.originId).toBeNull(); + expect(last.candidates.map((c) => c.taskId)).toEqual(["c"]); + }); +}); diff --git a/src/routes/v2/pages/Editor/components/UpgradeComponents/utils/groupCandidatesByOrigin.ts b/src/routes/v2/pages/Editor/components/UpgradeComponents/utils/groupCandidatesByOrigin.ts new file mode 100644 index 000000000..095fc677c --- /dev/null +++ b/src/routes/v2/pages/Editor/components/UpgradeComponents/utils/groupCandidatesByOrigin.ts @@ -0,0 +1,62 @@ +import type { UpgradeCandidate } from "@/routes/v2/pages/Editor/components/UpgradeComponents/types"; + +export interface CandidateGroup { + /** Null for candidates that have no lineage origin (pre-lineage tasks). */ + originId: string | null; + /** Component name from the upgrade target, for the group header. */ + componentName: string; + candidates: UpgradeCandidate[]; +} + +/** + * Group upgrade candidates by their lineage origin. Returns groups only when + * there is meaningful grouping — at least one origin with multiple candidates, + * or any off-mainline candidate. Otherwise returns null (flat render unchanged). + * + * Groups are ordered: multi-candidate families first, then singletons, then + * ungrouped (no origin). Within each group candidates are in their original order. + */ +export function groupCandidatesByOrigin( + candidates: UpgradeCandidate[], +): CandidateGroup[] | null { + const hasOffMainline = candidates.some((c) => c.isEditedOffMainline); + const byOrigin = new Map(); + const noOrigin: UpgradeCandidate[] = []; + + for (const c of candidates) { + if (c.originId) { + const group = byOrigin.get(c.originId) ?? []; + group.push(c); + byOrigin.set(c.originId, group); + } else { + noOrigin.push(c); + } + } + + const hasMultiGroup = [...byOrigin.values()].some((g) => g.length > 1); + if (!hasMultiGroup && !hasOffMainline) return null; + + const groups: CandidateGroup[] = []; + + for (const [originId, groupCandidates] of byOrigin) { + groups.push({ + originId, + componentName: + groupCandidates[0].newComponentRef.name ?? groupCandidates[0].taskName, + candidates: groupCandidates, + }); + } + + // Sort: multi-candidate families first, then singletons. + groups.sort((a, b) => b.candidates.length - a.candidates.length); + + if (noOrigin.length > 0) { + groups.push({ + originId: null, + componentName: "", + candidates: noOrigin, + }); + } + + return groups; +}