feat(network): single Node Identity filter + selectable edge-colour modes#556
Conversation
…odes Node Types filter (#499/#537 follow-up): the panel rendered two overlapping taxonomies (nodeType + deviceType) side by side, so Router, Web Server, DNS Server and Unknown each appeared twice. Collapse them into one taxonomy keyed on the adjudicated identity via a new nodeIdentityKey() (identityLabel → deviceType → L2_DEVICE → UNKNOWN), rename the section to "Node Identity", and switch every filter site to a single id: key namespace. Mobile/IoT/Laptop now stay distinct (finer than the rendered nodeType, which collapses them to "client"). Edge colour: the old "Protocol"/"Volume" toggle becomes Transport / Application / Volume in the same "Color edges by" dropdown. - Transport: base protocol (TCP/UDP/ICMP…), the previous behaviour. - Application: the nDPI application (WhatsApp, YouTube…) via getAppColor, with a matching legend (buildAppLegend); flows with no identified app fall to the grey "Other" bucket. This fixes edges reading as grey TCP when the identity lived only in appName. UI fixes: - Dark mode: contested "confirm" buttons (outline-primary on a warning alert) kept their unreadable purple; give outline-primary buttons inside alerts a legible light foreground in dark mode. - Monitor: the snapshot-detail network diagram gains a fullscreen button like the analysis diagram, using an over-modal z-index variant so it stacks above the dialog, with Escape-to-exit and tab-change cleanup. Tests: nodeIdentityKey resolution/dedup and buildAppLegend. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe frontend replaces separate node-type and device-type filters with canonical node identities, adds transport/application edge coloring, and introduces fullscreen diagram support in the monitor modal. Dark alert buttons receive additional contrast styling. ChangesNetwork identity and filtering
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SnapshotDetailModal
participant NetworkGraph
participant NetworkControls
participant networkService
User->>SnapshotDetailModal: Select edge mode or identity filter
SnapshotDetailModal->>NetworkGraph: Render diagram with selected mode
SnapshotDetailModal->>NetworkControls: Provide presentIdentities
NetworkControls->>networkService: Apply id-prefixed activeNodeFilters
networkService->>NetworkGraph: Return filtered graph data
User->>SnapshotDetailModal: Toggle diagram fullscreen
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request consolidates the node filtering taxonomy from two separate dimensions (nodeType and deviceType) into a single canonical 'Node Identity' taxonomy, and introduces a new 'Application' edge-coloring mode alongside the existing 'Transport' and 'Volume' modes. Additionally, it adds a fullscreen toggle for the network diagram inside the snapshot detail modal and improves dark mode button styling within alerts. The review feedback suggests adding a defensive guard check to nodeIdentityKey to prevent potential runtime crashes, and casting string arguments to any in several places to resolve strict TypeScript compilation errors where DeviceType is expected.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/network/NetworkControls/NetworkControls.tsx (1)
42-51: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
DEVICE_TYPES.includes(k)does not type-check here (frontend/src/components/network/NetworkControls/NetworkControls.tsx:122).DEVICE_TYPESisDeviceType[], whilekisstring, so this blocks strict TS compilation. NarrowktoDeviceTypefirst or widen the array type if arbitrary strings are intended.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/network/NetworkControls/NetworkControls.tsx` around lines 42 - 51, Update the identity-filter logic in NetworkControls to resolve the DEVICE_TYPES.includes call’s string-versus-DeviceType type mismatch. Narrow each key to DeviceType before calling includes, while preserving separate handling for the L2_DEVICE identity and existing filter behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/monitor/SnapshotDetailModal/SnapshotDetailModal.tsx`:
- Around line 141-148: Update the Escape-key handler in SnapshotDetailModal to
prioritize closing showLabelModal, showFilterModal, and selectedPair before
handling isDiagramFullscreen. Only exit diagram fullscreen and stop propagation
when no modal or node-detail selection is active, matching the priority order
used by NetworkDiagramPage.
---
Outside diff comments:
In `@frontend/src/components/network/NetworkControls/NetworkControls.tsx`:
- Around line 42-51: Update the identity-filter logic in NetworkControls to
resolve the DEVICE_TYPES.includes call’s string-versus-DeviceType type mismatch.
Narrow each key to DeviceType before calling includes, while preserving separate
handling for the L2_DEVICE identity and existing filter behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f018b2f0-e23d-47c9-a9d3-45135a2ff331
📒 Files selected for processing (13)
frontend/src/assets/styles/sgds-overrides.cssfrontend/src/components/monitor/SnapshotDetailModal/SnapshotDetailModal.tsxfrontend/src/components/network/NetworkControls/NetworkControls.tsxfrontend/src/components/network/NetworkGraph/NetworkGraph.cssfrontend/src/components/network/NetworkGraph/NetworkGraph.tsxfrontend/src/features/network/__tests__/protocolLegend.test.tsfrontend/src/features/network/constants.tsfrontend/src/features/network/services/networkService.tsfrontend/src/pages/Compare/ComparePage.tsxfrontend/src/pages/NetworkDiagram/NetworkDiagramPage.tsxfrontend/src/pages/NetworkIntelligence/NetworkIntelligencePage.tsxfrontend/src/utils/__tests__/deviceType.test.tsfrontend/src/utils/deviceType.ts
| const [showFilterModal, setShowFilterModal] = useState(false); | ||
| const [showLabelModal, setShowLabelModal] = useState(false); | ||
| const isDark = useResolvedDark(); | ||
| const [edgeColorMode, setEdgeColorMode] = useState<EdgeColorMode>('protocol'); | ||
| const [edgeColorMode, setEdgeColorMode] = useState<EdgeColorMode>('transport'); | ||
| const [isDiagramFullscreen, setIsDiagramFullscreen] = useState(false); | ||
| const [showHeatmap, setShowHeatmap] = useState(false); | ||
| const [isHeatmapFullscreen, setIsHeatmapFullscreen] = useState(false); | ||
| const [selectedPair, setSelectedPair] = useState<VolumePair | null>(null); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Escape while diagram-fullscreen doesn't prioritize closing open modals/node-details first.
Unlike the analogous handler in NetworkDiagramPage.tsx (which checks showLabelModal → showFilterModal → selectedNode before exiting fullscreen), this new isDiagramFullscreen branch exits fullscreen unconditionally and calls e.stopPropagation(). Since showFilterModal/showLabelModal remain openable while fullscreen (the Filters/tag buttons stay active), pressing Escape with one of those open will exit fullscreen and swallow the keypress that would otherwise close the modal — the modal is left open, requiring a second Escape press to dismiss it.
🐛 Proposed fix
if (e.key === 'Escape' && isDiagramFullscreen) {
e.preventDefault();
e.stopPropagation();
+ if (showLabelModal) { setShowLabelModal(false); return; }
+ if (showFilterModal) { setShowFilterModal(false); return; }
+ if (selectedNode) { setSelectedNode(null); return; }
setIsDiagramFullscreen(false);
return;
}Also applies to: 269-301
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/monitor/SnapshotDetailModal/SnapshotDetailModal.tsx`
around lines 141 - 148, Update the Escape-key handler in SnapshotDetailModal to
prioritize closing showLabelModal, showFilterModal, and selectedPair before
handling isDiagramFullscreen. Only exit diagram fullscreen and stop propagation
when no modal or node-detail selection is active, matching the priority order
used by NetworkDiagramPage.
1 participant
Summary
Follow-up to #537 ("Identity owns the verdict") plus edge-colour and UX fixes on the network diagram.
Node Identity filter (was two overlapping taxonomies)
The Node Types filter rendered
nodeTypeanddeviceTypepills side by side, so Router / Web Server / DNS Server / Unknown each showed twice. Collapsed into one taxonomy keyed on the adjudicated identity:nodeIdentityKey()—identityLabel → deviceType → L2_DEVICE → UNKNOWN.id:key namespace (NetworkControls, NetworkDiagram, Compare, SnapshotDetailModal, NetworkIntelligence, networkService, report labels).nodeType, which collapses them to client).Edge colour — user-selectable mode
The old Protocol / Volume toggle becomes Transport / Application / Volume in the same "Color edges by" dropdown:
getAppColor, with a matching legend (buildAppLegend); un-identified flows fall to a grey "Other" bucket. Fixes edges reading as grey TCP when the identity lived only inappName.UI fixes
outline-primaryon a warning alert) kept an unreadable purple — now a legible light foreground inside alerts.Testing
tsc -bclean;vitest143 passing (new:nodeIdentityKey,buildAppLegend).🤖 Generated with Claude Code
Summary by CodeRabbit