Skip to content

[CHAIN] refactor(ui): normalize graph edge types and remove dead code#10701

Merged
pfe-nazaries merged 6 commits into
PROWLER-1273/react-flow-migrationfrom
PROWLER-1373/normalize-data
Apr 22, 2026
Merged

[CHAIN] refactor(ui): normalize graph edge types and remove dead code#10701
pfe-nazaries merged 6 commits into
PROWLER-1273/react-flow-migrationfrom
PROWLER-1373/normalize-data

Conversation

@pfe-nazaries

@pfe-nazaries pfe-nazaries commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

🔗 Part of Chained PRs

Field Value
Feature Branch PROWLER-1273/react-flow-migration
Main PR #10686
Sub-task PROWLER-1373
Chain Position 1 of 5
Status 🟢 Merged — unblocks PR1 (#10705)

Chain Overview

         ┌──────────────────────────┐
         │ 📍 PR0: Normalize data   │ ← THIS PR (#10701) 🟢
         └────────────┬─────────────┘
                      │
         ┌────────────▼─────────────┐
         │ PR1: React Flow core     │ ← #10705 🟡
         │   rendering              │
         └────────────┬─────────────┘
                      │
         ┌────────────▼─────────────┐
         │ PR2: Interactions        │ ← #10756 🟡
         │   🎯 MVP                 │
         └────────────┬─────────────┘
                      │
         ┌────────────▼─────────────┐
         │ PR3: Export + Minimap    │ ← #10800 🟡
         │   ✅ Phase 1 complete    │
         └────────────┬─────────────┘
                      │
         ┌────────────▼─────────────┐
         │ PR4: Test coverage       │ ← PROWLER-1405 ⚪
         │   🛡️ Hardening complete  │
         └────────────┬─────────────┘
                      │
         ┌────────────▼─────────────┐
         │ #10686 Main PR           │
         │   → master               │
         └──────────────────────────┘

Context

Preparatory cleanup for the React Flow migration (#10686). Normalizes the graph data layer and removes dead code before introducing any new library dependencies.

Part of chained PRs for Replace D3+Dagre attack path graph with React Flow. This sub-task ensures the data layer is clean and type-safe before PR1 introduces React Flow rendering.


Description

Net result: -198 lines, zero visual/functional change.

Changes:

Data normalization:

  • Narrow GraphEdge.source/target type from string | object to string — the adapter already guaranteed strings, but the type was unnecessarily wide
  • Remove 14 typeof guards across 5 files (adapter, graph component, utilities, node relationships, graph-utils)
  • Remove getEdgeNodeId() helper and EdgeNodeRef type — replaced by direct edge.source/edge.target access

Dead code removal:

  • Remove unused panX, panY, zoomLevel, setZoom(), setPan(), updateZoomAndPan() from Zustand graph state store (never consumed by any component)
  • Delete orphaned NodeRelationships component (exported but never imported by any parent component)

Steps to review

  1. Review type changes in ui/types/attack-paths.tssource and target narrowed to string
  2. Verify typeof guard removals in query-result.adapter.ts, attack-path-graph.tsx, graph-utils.ts
  3. Confirm dead store fields removed from use-graph-state.ts
  4. Verify NodeRelationships was indeed orphaned (search for imports — none exist outside its own barrel)
  5. pnpm typecheck passes
  6. Browser test: Navigate to Attack Paths → select a scan → execute a query → verify graph renders, interactions work, no console errors

Checklist

Community Checklist
  • This feature/issue is listed in here or roadmap.prowler.com
  • Is it assigned to me
  • Review if the code is being covered by tests.
  • Review if code is being documented
  • Review if backport is needed.
  • Review if is needed to change the Readme.md
  • Ensure new entries are added to CHANGELOG.md, if applicable.

SDK/CLI

  • Are there new checks included in this PR? No

UI

  • All issue/task requirements work as expected on the UI
  • N/A Screenshots — zero visual change (type-only refactor)
  • Ensure new entries are added to ui/CHANGELOG.md

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@pfe-nazaries pfe-nazaries requested a review from a team as a code owner April 15, 2026 11:48
@pfe-nazaries pfe-nazaries requested a review from a team April 15, 2026 12:08
@pfe-nazaries pfe-nazaries changed the title refactor(ui): normalize graph edge types and remove dead code [CHAIN] refactor(ui): normalize graph edge types and remove dead code Apr 15, 2026
Pablo F.G and others added 2 commits April 17, 2026 14:32
- Narrow GraphEdge.source/target from string | object to string
- Remove typeof guards in adapter, graph component, and utilities
- Remove unused panX, panY, zoomLevel fields from graph state store
- Delete orphaned NodeRelationships component (never integrated)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@pfe-nazaries pfe-nazaries force-pushed the PROWLER-1373/normalize-data branch from 105bc5f to 86bd153 Compare April 17, 2026 13:06
@alejandrobailo

Copy link
Copy Markdown
Contributor

Code review

A couple of questions before this lands — both non-blocking, just want to make sure the cleanup closes fully:

1. <Suspense key="feeds"> in navbar.tsx

ui/components/ui/nav-bar/navbar.tsx#L17-L20

What bug is this fixing? A static string key on Suspense is a no-op — React only remounts the boundary when the key value changes, so a literal never triggers anything. If the original intent was to reset the fallback on navigation (stale feeds loader), you probably want a dynamic key like key={pathname} or key={searchParams.toString()}. Otherwise this line can be dropped, since it also feels unrelated to the graph data normalization scope of this PR.

2. Orphaned incomingEdges / outgoingEdges on SelectedNodeState

ui/types/attack-paths.ts#L252-L255

After deleting NodeRelationships (its only consumer), these two fields on SelectedNodeState have no producer and no consumer left in the codebase. Since this PR already prunes the rest of that dead surface (barrel export, store fields, helper, guards), would you be up for dropping these two fields in the same PR so the cleanup lands as one coherent pass? Happy for it to go in a follow-up if you'd rather keep the diff scoped.

Pablo F.G added 3 commits April 22, 2026 15:56
A literal string key on <Suspense> never changes, so it does not remount
the boundary or reset the fallback. Removing the line.
NodeDetailData, RelatedFinding, FINDING_SEVERITIES and FINDING_STATUSES
have no remaining producers or consumers after prior cleanup. Dropping
the full chain.
@pfe-nazaries

Copy link
Copy Markdown
Contributor Author

Code review

A couple of questions before this lands — both non-blocking, just want to make sure the cleanup closes fully:

1. <Suspense key="feeds"> in navbar.tsx

ui/components/ui/nav-bar/navbar.tsx#L17-L20

What bug is this fixing? A static string key on Suspense is a no-op — React only remounts the boundary when the key value changes, so a literal never triggers anything. If the original intent was to reset the fallback on navigation (stale feeds loader), you probably want a dynamic key like key={pathname} or key={searchParams.toString()}. Otherwise this line can be dropped, since it also feels unrelated to the graph data normalization scope of this PR.

2. Orphaned incomingEdges / outgoingEdges on SelectedNodeState

ui/types/attack-paths.ts#L252-L255

After deleting NodeRelationships (its only consumer), these two fields on SelectedNodeState have no producer and no consumer left in the codebase. Since this PR already prunes the rest of that dead surface (barrel export, store fields, helper, guards), would you be up for dropping these two fields in the same PR so the cleanup lands as one coherent pass? Happy for it to go in a follow-up if you'd rather keep the diff scoped.

  1. You were right, now im almost sure that was something related to the next cache due my worktrunk setup cloning it on every worktree

  2. Done :)

@pfe-nazaries pfe-nazaries merged commit ba84b23 into PROWLER-1273/react-flow-migration Apr 22, 2026
1 check passed
@pfe-nazaries pfe-nazaries deleted the PROWLER-1373/normalize-data branch April 22, 2026 14:29

@Alan-TheGentleman Alan-TheGentleman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  1. The string-only edge contract needs test coverage. This refactor removes every fallback path for GraphEdge.source and GraphEdge.target, so we need one automated test that proves adaptQueryResultToGraphData() always emits string IDs and one test that exercises computeFilteredSubgraph() against that normalized shape. Without that guardrail, a legacy edge payload with { id } would break the graph silently.

Alan-TheGentleman pushed a commit that referenced this pull request May 8, 2026
…#10701)

Co-authored-by: Pablo F.G <pablo.fernandez@prowler.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.

3 participants