feat(compare): side-by-side YAML diff for two resources#754
Conversation
Adds a `Compare ⇄` flow for diffing two K8s resources of the same kind. Two entry points converge on the same view: - **Drawer "Compare" button**: opens a picker (same-namespace promoted, alphabetical, ↑↓/Enter keyboard nav), navigates to `/compare?kind=&a=&b=`. - **Table compare mode**: header toggle flips ResourcesView into pick mode — leading A/B badge column, sticky bottom tray with 2 slots, cap-at-2 with replace-oldest, Esc exits. Diff view: Monaco DiffEditor (real impl of the existing `YamlDiffEditor` stub), side-by-side or unified, hide-unchanged collapses regions, Spec-only drops status. Per-side error rendering — failed side gets a red pill + banner, working side still renders. Swap A↔B updates URL. Resources are normalized before diffing (strip managedFields/uid/resourceVersion/last- applied/pod-template-hash) so the diff is signal not noise. Pure helpers extracted with tests: `parseRef`/`refToParam` (URL ref parsing), `togglePick`/`pickIndex` (cap-replace state machine), `sortCandidates`/`filterCandidates` (picker order), `normalizeForCompare`. 49 new tests pin the load-bearing behavior. The frontend `useResources` hook now gates on `Boolean(kind)` so the picker's lazy-on-open pattern doesn't fire a 404 for the empty kind.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| namespace, | ||
| name, | ||
| group: resourceGroup || undefined, | ||
| }) |
There was a problem hiding this comment.
CRD group omitted from compare
Medium Severity
The drawer compare launcher only passes group derived from loaded resource.apiVersion, not the group already on the selected resource. Opening Compare before fetch completes (or if fetch fails) omits the API group from list and navigation URLs, so CRD compare can hit the wrong kind or fail while the drawer still has the correct group.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 3554310. Configure here.
|
|
||
| useEffect(() => { | ||
| setHighlightIdx(prev => (prev >= filtered.length ? 0 : prev)) | ||
| }, [filtered.length]) |
There was a problem hiding this comment.
Search keeps stale list highlight
Medium Severity
Keyboard highlight index is only clamped when the filtered list length changes, not when the search query changes the filtered rows. After typing in the search box, Enter can select a different resource than the one visually highlighted, because highlightIdx still points at the old position.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 3554310. Configure here.
Address findings from the second review pass: - parseRef now rejects `?a=prod/` (empty name after slash) — was silently wedging callers in an indefinite loading state since useResource had nothing to fetch. - CompareViewRoute no longer flashes "Failed to load side A" for a refetch failure that has cached data — banner now only fires when the side has no data at all. Stale beats misleading. - Kind change in compare mode now also exits compare mode, not just clears picks — leaving the tray on with empty pills after the kind switch was the worst-of-both UX. - Tray render now gated on `compareEnabled` (mirrors the existing toolbar toggle gate) so library consumers without onNavigate can't get a tray whose Compare CTA silently no-ops. - Picker error prop typed `unknown` (was `Error | null`) — React Query emits unknown; renderer falls back through `String(err)` for non-Error throws. - togglePick cap-replace rewritten as `[...picks.slice(1), ref]` — clearer intent than the old slice arithmetic that only happened to be correct for cap=2. Type cleanup: - Single `NamespacedRef` shape replaces the three accidental duplicates (Pick / CompareTrayPick / ParsedRef / SortableCandidate). - `SIDE_TONES` const centralises the A/B palette used by the drawer pill, picker chip, tray pill, and table row badge. Palette changes touch one place instead of four. - rowHighlightClass extracted from a 4-deep nested ternary in ResourceRowCells (CLAUDE.md flag). Web cleanup: - useCompareCandidates lifts the shared `useResources` + map pattern out of useCompareLauncher and CompareViewRoute. Comments stripped (CLAUDE.md: no WHAT narration): A→B gradient ribbon, "see the design memo" pointer, DNS-1123 duplication in url.test.ts, verbose PINNING block in normalize.test.ts. 51 tests pass (+2 — empty-name URL rejection, slash-only URL rejection).
Two viewport-related issues caught on real-world wide screens (2000px+) that I missed in 1200px visual tests: - ResourceCompareView's root lacked `flex-1`, so on wide viewports the diff view collapsed to its content width and left half the screen empty. - CompareTray's Compare CTA and Exit X collided with the fixed-position debug + shortcut-help overlay buttons anchored bottom-right of the viewport. Added right padding to the tray content row so the buttons sit clear of the overlay. Verified at 2000x1100.
Playwright MCP defaults to ~1280px, which hides whole classes of layout bugs that only show up at desktop / ultrawide widths. The compare PR shipped two of them — a full-screen view that collapsed to content width without `flex-1`, and a sticky bar that collided with Radar's fixed bottom-right overlay buttons — both invisible at 1280 but obvious at 2000+. - /visual-test command now opens with a "set viewport FIRST" step, defaults to 1920x1080, and points at the 1280/1920/2560 sweep for layout-sensitive changes. - visual-test-start.sh prints the same reminder on launch so anyone driving the harness sees it before navigating. - "What to look for" checklist gains two wide-viewport bullets.
| function handleKeyDown(e: React.KeyboardEvent<HTMLInputElement>) { | ||
| if (e.key === 'ArrowDown') { | ||
| e.preventDefault() | ||
| setHighlightIdx(i => Math.min(i + 1, filtered.length - 1)) |
There was a problem hiding this comment.
Picker arrow sets invalid index
Low Severity
When the filtered candidate list is empty, ArrowDown computes Math.min(i + 1, filtered.length - 1), which becomes -1 because filtered.length - 1 is -1. Highlight index can leave the valid 0..length-1 range, so keyboard navigation no longer matches any row until the user changes the filter again.
Reviewed by Cursor Bugbot for commit 757d17c. Configure here.
- New "Raw metadata" toggle (off by default) — when on, normalize skips the metadata-noise strip pass, so resourceVersion, uid, managedFields, last-applied-configuration, pod-template-hash, etc. show up in the diff. For the rare case of debugging API-level differences. - Fixed the layout toggle to match the other three toolbar toggles: constant "Unified" label, highlight when active. Was changing both label and icon on click — inconsistent with Spec only / Diff only / Raw metadata. - README + docs screenshot updated to reflect the four-toggle toolbar.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 5 total unresolved issues (including 3 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 54d8789. Configure here.
| if (aQuery.error && !aQuery.data) errors.push({ side: 'a', message: aQuery.error instanceof Error ? aQuery.error.message : String(aQuery.error) }) | ||
| if (bQuery.error && !bQuery.data) errors.push({ side: 'b', message: bQuery.error instanceof Error ? bQuery.error.message : String(bQuery.error) }) | ||
|
|
||
| const source = pickerOpen === 'a' ? a : pickerOpen === 'b' ? b : null |
There was a problem hiding this comment.
Picker badge hardcoded as "A" when re-picking side B
Medium Severity
When the user clicks the pencil on the B pill to re-pick side B, pickerOpen is 'b' and source is set to b. But CompareResourcePicker hardcodes the badge as "A" (blue bg-blue-400/90) regardless of which side is being re-picked. The user sees the current B resource labeled as "A", which is visually misleading. Additionally, sortCandidates uses this source to promote same-namespace candidates — it promotes B's namespace instead of A's, which may not match user intent when replacing B.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 54d8789. Configure here.
| onPick, | ||
| }: CompareResourcePickerProps) { | ||
| const [query, setQuery] = useState('') | ||
| const [highlightIdx, setHighlightIdx] = useState(0) |
There was a problem hiding this comment.
Picker search query persists between opens in drawer
Low Severity
In the useCompareLauncher path (drawer entry point), CompareResourcePicker stays mounted and only toggles via the open prop. The query and highlightIdx state are never reset when the picker re-opens, so a previous search term leaks into the next open. The CompareViewRoute path avoids this by conditionally rendering ({source && ...}), which unmounts and remounts the picker. The inconsistency suggests the intent is a fresh picker each time.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 54d8789. Configure here.
1 participant
Summary
Adds a
Compare ⇄flow for diffing two Kubernetes resources of the same kind side-by-side. Modelled after Aptakube's resource-diff feature, scoped to v1: single cluster, same kind, two-way.Two entry points converge on the same diff view:
Comparebutton in the resource action bar opens a picker dialog. Same-namespace candidates are promoted to the top (the obvious target), alphabetical within each group, ↑↓ keyboard navigation, Enter to pick.Compare ⇄toggle in theResourcesViewheader flips the table into pick mode — leading A/B-badge column, sticky bottom tray with two pick slots, cap-at-2 with replace-oldest so a row click always has a visible effect, Esc exits.Both routes navigate to
/compare?kind=&group=&a=ns/name&b=ns/name(URL is shareable).What the diff view does
Monaco
DiffEditor(real implementation of the existingYamlDiffEditorstub atYamlEditor.tsx:250). Side-by-side or unified, hide-unchanged collapses regions,Spec onlydrops status fields, A↔B swap rewrites the URL, click the pencil on either pill to re-pick.Resources are normalised before diffing —
managedFields,uid,resourceVersion,creationTimestamp,kubectl.kubernetes.io/last-applied-configuration,pod-template-hashand similar noise stripped — so the diff shows intent, not server-assigned state.Per-side error rendering: if A succeeds and B 404s (stale share-link), the working side still renders, the failed side gets a red pill + warning icon + banner naming the side, and the pencil lets the user re-pick.
Implementation notes
packages/k8s-ui/src/components/compare/. Pure helpers (parseRef/refToParam,togglePick/pickIndex,sortCandidates/filterCandidates,normalizeForCompare) are exported and unit-tested — 49 new tests pin the load-bearing behaviour.CompareResourceRef), oneCompareSide = 'a' | 'b', one place for the cap constant (COMPARE_PICK_CAP = 2).useResourceshook now gates onBoolean(kind)so the picker's lazy-on-open pattern doesn't fire a 404 for the empty kind.onNavigate— embeds that don't pass it (hypothetical) get the button hidden rather than a dead click.Out of scope (deliberate)
Cross-kind compare and three-way compare — both worth more thought before adding. Cross-cluster compare needs Radar Hub multi-cluster anyway. Semantic ("by category") grouping over the raw line diff is the natural next step.
Note
Medium Risk
Adds a new compare workflow and route that changes core resource browsing interactions (table keyboard shortcuts, row click behavior) and introduces new diff rendering via Monaco
DiffEditor; main risk is UI regressions and increased query/load patterns when listing candidates.Overview
Adds a new resource compare feature that lets users diff two same-kind resources in a dedicated
/compareview, including side-by-side vs unified rendering, diff-only collapsing, spec-only mode, raw-metadata toggle, A↔B swap, and per-side error handling.Introduces two entry points: a
Compareaction in the resource drawer (with a searchable, keyboard-navigable picker) and a new Resources table compare mode that supports selecting two rows (A/B) with a sticky bottomCompareTray, row highlighting, andEscto exit.Implements normalization utilities to strip server-assigned metadata noise before diffing, adds unit tests for compare helpers, wires the new view into the app router, updates
YamlDiffEditorto use MonacoDiffEditor, and extendsuseResourceswith anenabledgate to support lazy candidate fetching.Reviewed by Cursor Bugbot for commit 54d8789. Bugbot is set up for automated code reviews on this repo. Configure here.