Skip to content

chore: v2 - integrate update components#2242

Open
maxy-shpfy wants to merge 1 commit into
masterfrom
05-08-chore_v2_-_integrate_update_components
Open

chore: v2 - integrate update components#2242
maxy-shpfy wants to merge 1 commit into
masterfrom
05-08-chore_v2_-_integrate_update_components

Conversation

@maxy-shpfy
Copy link
Copy Markdown
Collaborator

@maxy-shpfy maxy-shpfy commented May 8, 2026

Description

Wires the Upgrade Components window in the V2 editor to real outdated-component data instead of always using mock candidates.

Key changes:

  • collectUsedComponentReferencesFromV2Spec – new utility that collects unique component references from V2 spec tasks (deduped by digest), mirroring the existing graph-based fetchUsedComponents.
  • buildUpgradeCandidateFromResolved – extracted shared logic for building an UpgradeCandidate from a resolved task spec and a target component ref (including lost-binding detection and issue prediction). Previously this logic was duplicated inside useMockUpgradeCandidates.
  • useUpgradeCandidatesFromOutdated – new hook that calls useOutdatedComponents with the V2 spec's used components and maps the outdated pairs to UpgradeCandidate objects using buildUpgradeCandidateFromResolved.
  • UpgradeComponentsContent – now accepts a dataSource prop ("real" | "mock"). "real" (the default) uses useUpgradeCandidatesFromOutdated; "mock" uses the existing mock hook. The inner rendering logic is extracted into UpgradeComponentsInner.
  • useUpgradeComponentsWindow – accepts an optional { useMock?: boolean } argument. When useMock is true (debug panel only), it opens with mock data; otherwise it opens UpgradeComponentsContent with dataSource="real" wrapped in a suspense boundary with a skeleton fallback.
  • UpgradeAvailableAlertBox – now renders two variants depending on whether a window context is present. Inside the V2 window chrome (e.g. docked Component Library), the Review button opens the Upgrade Components window via useUpgradeComponentsWindow. In the V1 sidebar (no window context), the existing nodes-overlay flow is preserved. Alert visibility and dismiss logic are extracted into a shared useUpgradeAlertVisibility hook.
  • useOutdatedComponents query key – changed from the full ComponentReference[] array to a sorted array of digest strings, preventing unnecessary cache misses caused by object reference changes.

Related Issue and Pull requests

Type of Change

  • Bug fix
  • New feature
  • Improvement
  • Cleanup/Refactor
  • Breaking change
  • Documentation update

Checklist

  • I have tested this does not break current pipelines / runs functionality
  • I have tested the changes on staging

Screenshots (if applicable)

Component Update.mov (uploaded via Graphite)

Test Instructions

  1. Open the V2 editor with a pipeline that has outdated components.
  2. Confirm the "Upgrades available" alert appears in the docked Component Library sidebar.
  3. Click Review — the Upgrade Components window should open and display the real outdated components with accurate diff previews (lost inputs/outputs highlighted).
  4. Open the Debug Panel and click Upgrade Components — confirm it opens with mock candidates as before.
  5. In the V1 sidebar, confirm the alert's Review button still triggers the nodes-overlay highlight flow and does not open the window.
  6. Dismiss the alert and confirm it is suppressed for 24 hours with a toast notification.

Additional Comments

The debug panel intentionally continues to use useMock: true so developers can exercise the upgrade UI without needing a pipeline with genuinely outdated components.

Copy link
Copy Markdown
Collaborator Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 8, 2026

🎩 Preview

A preview build has been created at: 05-08-chore_v2_-_integrate_update_components/c9b8e91

@maxy-shpfy maxy-shpfy marked this pull request as ready for review May 11, 2026 20:32
@maxy-shpfy maxy-shpfy requested a review from a team as a code owner May 11, 2026 20:32
*
* 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
* returns no candidates without doing any work.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 This is an AI-generated code review comment.

Nit: the doc says "returns no candidates without doing any work", but the if (!spec) branch is checked after useOutdatedComponents(EMPTY_USED_COMPONENTS) runs (hook rules require this). With an empty input the query function returns [] immediately so this is cheap in practice, but the comment overstates it.

Suggest rewording to something like "the null branch returns no candidates without iterating the spec".

@@ -0,0 +1,82 @@
import type { ComponentReference } from "@/models/componentSpec";
import type { ComponentSpec } from "@/models/componentSpec/entities/componentSpec";
import type { ComponentSpecJson } from "@/models/componentSpec/entities/types";
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 This is an AI-generated code review comment.

Minor: ComponentSpec and ComponentSpecJson are both re-exported from the @/models/componentSpec barrel, so the deep paths here are inconsistent with the other new files in this PR (which all use the barrel). Suggest:

import type {
  ComponentReference,
  ComponentSpec,
  ComponentSpecJson,
} from "@/models/componentSpec";

@@ -0,0 +1,21 @@
import type { ComponentSpec } from "@/models/componentSpec";
import type { ComponentReference } from "@/utils/componentSpec";
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 This is an AI-generated code review comment.

Nit: ComponentReference is re-exported from @/models/componentSpec too, so both imports can come from the same module. The mixed sources are unnecessary churn given the rest of the new files in this PR consistently use @/models/componentSpec.

import type { ComponentReference, ComponentSpec } from "@/models/componentSpec";


const usedComponents = spec
? collectUsedComponentReferencesFromV2Spec(spec)
: (usedComponentsFolder.components ?? []);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 This is an AI-generated code review comment.

This V2 branch only renders inside a window context, which only exists inside the V2 editor where useSpec() returns a non-null spec. The usedComponentsFolder.components ?? [] fallback path looks unreachable in practice, but the useComponentLibrary() call still subscribes this component to ComponentLibraryProvider updates — extra re-renders for behavior we never hit.

Two options:

  • If the invariant really is "spec is non-null in V2", drop the destructure (and the useComponentLibrary() call) and just use the spec.
  • If spec genuinely can be null here, a one-line comment explaining when would help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants