From c694a10fa17b69f3511a2650208c47343167741d Mon Sep 17 00:00:00 2001 From: Felipe Bergamin Date: Fri, 25 Sep 2026 14:10:21 -0300 Subject: [PATCH 1/5] fix(tree-compare): default A to the older revision The compare page defaulted side A to the branch head and side B to the previous revision, while deriveCompareChange reads A as the 'from' side. A test that started failing on the newest revision was therefore classified as 'fixed' instead of a regression. Default side B to the branch head and side A to the revision before it, and pre-fill the current revision as side B when opening compare from Tree Details, so the default A -> B direction is oldest -> newest. Closes #2130 --- .../src/pages/TreeCompare/TreeCompareLink.tsx | 2 +- .../src/pages/TreeCompare/TreeComparePage.tsx | 14 +++---- dashboard/src/utils/treeCompareDiff.test.ts | 38 +++++++++++++++++++ dashboard/src/utils/treeCompareDiff.ts | 19 ++++++++++ docs/tree-compare.md | 2 +- 5 files changed, 66 insertions(+), 9 deletions(-) diff --git a/dashboard/src/pages/TreeCompare/TreeCompareLink.tsx b/dashboard/src/pages/TreeCompare/TreeCompareLink.tsx index 48806f30b..3aa565489 100644 --- a/dashboard/src/pages/TreeCompare/TreeCompareLink.tsx +++ b/dashboard/src/pages/TreeCompare/TreeCompareLink.tsx @@ -24,7 +24,7 @@ export function TreeCompareLink({ s} > diff --git a/dashboard/src/pages/TreeCompare/TreeComparePage.tsx b/dashboard/src/pages/TreeCompare/TreeComparePage.tsx index 078b279c5..c04e80bd5 100644 --- a/dashboard/src/pages/TreeCompare/TreeComparePage.tsx +++ b/dashboard/src/pages/TreeCompare/TreeComparePage.tsx @@ -45,6 +45,7 @@ import { mapBootOrTestDiffRows, mapBuildDiffRows, readStoredStatusPairs, + resolveCompareHashes, resolveStatusPairs, serializeStatusPairs, writeStoredStatusPairs, @@ -88,11 +89,10 @@ const TreeComparePage = (): JSX.Element => { [commitsQuery.data], ); - const resolvedHashA = hashA || revisions[0]?.hash || ''; - const resolvedHashB = - hashB || - revisions.find(revision => revision.hash !== resolvedHashA)?.hash || - ''; + const { hashA: resolvedHashA, hashB: resolvedHashB } = useMemo( + () => resolveCompareHashes(revisions, hashA, hashB), + [revisions, hashA, hashB], + ); const canCompare = Boolean(resolvedHashA && resolvedHashB); @@ -317,7 +317,7 @@ const TreeComparePage = (): JSX.Element => { params={{ treeName, branch, - hash: resolvedHashA, + hash: resolvedHashB, }} state={s => s} > @@ -362,7 +362,7 @@ const TreeComparePage = (): JSX.Element => { s} > diff --git a/dashboard/src/utils/treeCompareDiff.test.ts b/dashboard/src/utils/treeCompareDiff.test.ts index 46d388d83..95a77df6b 100644 --- a/dashboard/src/utils/treeCompareDiff.test.ts +++ b/dashboard/src/utils/treeCompareDiff.test.ts @@ -7,6 +7,7 @@ import { mapBuildDiffRows, compareRowNav, parseStatusPairs, + resolveCompareHashes, resolveStatusPairs, serializeStatusPairs, toggleChangeTypePairs, @@ -44,6 +45,43 @@ describe('deriveCompareChange', () => { }); }); +describe('resolveCompareHashes', () => { + const revisions = [{ hash: 'new' }, { hash: 'mid' }, { hash: 'old' }]; + + it('defaults to the head on B and the revision before it on A', () => { + expect(resolveCompareHashes(revisions, '', '')).toEqual({ + hashA: 'mid', + hashB: 'new', + }); + }); + + it('keeps A one revision older than an explicit B', () => { + expect(resolveCompareHashes(revisions, '', 'mid')).toEqual({ + hashA: 'old', + hashB: 'mid', + }); + }); + + it('never defaults B to the same revision as A', () => { + expect(resolveCompareHashes(revisions, 'new', '')).toEqual({ + hashA: 'new', + hashB: 'mid', + }); + }); + + it('leaves explicit hashes alone and copes with too few revisions', () => { + expect(resolveCompareHashes(revisions, 'old', 'new')).toEqual({ + hashA: 'old', + hashB: 'new', + }); + expect(resolveCompareHashes([{ hash: 'only' }], '', '')).toEqual({ + hashA: '', + hashB: 'only', + }); + expect(resolveCompareHashes([], '', '')).toEqual({ hashA: '', hashB: '' }); + }); +}); + describe('applyStatusPairFilter', () => { const rows = [ { id: '1', sideA: 'PASS' as const, sideB: 'FAIL' as const }, diff --git a/dashboard/src/utils/treeCompareDiff.ts b/dashboard/src/utils/treeCompareDiff.ts index cb313c7cd..e47e4cc9a 100644 --- a/dashboard/src/utils/treeCompareDiff.ts +++ b/dashboard/src/utils/treeCompareDiff.ts @@ -300,6 +300,25 @@ export function applyStatusPairFilter< ); } +/** + * Fill in missing sides so the default comparison runs oldest → newest: + * B defaults to the branch head, A to the revision right before B. + * `revisions` is newest-first. + */ +export function resolveCompareHashes( + revisions: readonly { hash: string }[], + hashA: string, + hashB: string, +): { hashA: string; hashB: string } { + const resolvedB = + hashB || revisions.find(revision => revision.hash !== hashA)?.hash || ''; + const indexB = revisions.findIndex(revision => revision.hash === resolvedB); + return { + hashA: hashA || revisions[indexB + 1]?.hash || '', + hashB: resolvedB, + }; +} + /** Next/prev over the currently visible (searched/sorted) table rows. */ export function compareRowNav( rows: T[], diff --git a/docs/tree-compare.md b/docs/tree-compare.md index ca455cdaa..919382620 100644 --- a/docs/tree-compare.md +++ b/docs/tree-compare.md @@ -15,7 +15,7 @@ Entry point: **Compare revisions** on Tree Details (`TreeCompareLink`), which op ## User flow -1. Open compare from Tree Details (current revision pre-fills as side A). +1. Open compare from Tree Details (current revision pre-fills as side B; side A defaults to the revision before it, so A → B reads oldest → newest). 2. Choose / swap revisions via the revision selector (commit history + shortcuts: previous commit, branch head, swap sides). 3. Read the summary matrix (fixes, regressions, pass/fail/other counts per builds / boots / tests). 4. Drill into **Changed results** tabs (Builds / Boots / Tests). Quick change-type chips add or remove their status pairs; custom From/To pairs can be added too (default: `PASS → FAIL` and `FAIL → PASS`). Last edited pairs are remembered in localStorage when the URL omits `statusPair`. From 92fdc26843423351d81943aa78e865ab3e75bd55 Mon Sep 17 00:00:00 2001 From: Felipe Bergamin Date: Fri, 25 Sep 2026 14:14:26 -0300 Subject: [PATCH 2/5] fix(tree-compare): label revisions as Base and Target Side A/B hid the comparison direction. Use Base (from) and Target (to) in the selector, tables, and drawer; URL params stay hashA/hashB. --- dashboard/src/locales/messages/index.ts | 14 +++++++------- .../TreeCompare/components/CompareDetailSheet.tsx | 6 +++--- .../components/CompareFailuresTables.tsx | 8 ++++---- .../TreeCompare/components/RevisionSelector.tsx | 6 +++--- dashboard/src/utils/treeCompareDiff.ts | 2 +- docs/tree-compare.md | 12 ++++++------ 6 files changed, 24 insertions(+), 24 deletions(-) diff --git a/dashboard/src/locales/messages/index.ts b/dashboard/src/locales/messages/index.ts index 5d2dada70..b5bd6bfd4 100644 --- a/dashboard/src/locales/messages/index.ts +++ b/dashboard/src/locales/messages/index.ts @@ -417,6 +417,7 @@ export const messages = { 'tree.path': 'Trees', 'tree.searchPlaceholder': 'Search by tree, branch or tag with a regex', 'treeCompare.backToDetails': 'Back to tree details', + 'treeCompare.base': 'Base', 'treeCompare.breadcrumb': 'Compare', 'treeCompare.breakdownTitle': 'Changed results', 'treeCompare.change.appeared': 'Appeared', @@ -432,10 +433,10 @@ export const messages = { 'treeCompare.change.unchanged': 'Unchanged', 'treeCompare.changed': 'Changed', 'treeCompare.description': - 'Compare pass/fail counts between two revisions on the same tree and branch.', - 'treeCompare.detail.missingSide': 'No result on this side', + 'Compare pass/fail counts from a base revision to a target revision on the same tree and branch.', + 'treeCompare.detail.missingSide': 'No result on this revision', 'treeCompare.drilldownHint': - 'Individual builds, boots, and tests that changed between Side A and Side B.', + 'Individual builds, boots, and tests that changed from the base revision to the target revision.', 'treeCompare.failures.change': 'Change', 'treeCompare.failures.configArch': 'Config / Arch', 'treeCompare.failures.pathArch': 'Path / Arch', @@ -454,8 +455,6 @@ export const messages = { 'Select two different revisions to compare. This tree needs at least two commits with results.', 'treeCompare.openCompare': 'Compare revisions', 'treeCompare.selectRevision': 'Select a revision', - 'treeCompare.sideA': 'Side A', - 'treeCompare.sideB': 'Side B', 'treeCompare.statusPairFilter.active': 'Active status pair filters', 'treeCompare.statusPairFilter.add': 'Add', 'treeCompare.statusPairFilter.from': 'From', @@ -463,15 +462,16 @@ export const messages = { 'PASS — completed successfully{br}' + 'FAIL — completed with a failure{br}' + 'INCONCLUSIVE — no definitive pass or fail result{br}' + - '— — absent on this side', + '— — absent on this revision', 'treeCompare.statusPairFilter.remove': 'Remove {from} to {to} filter', 'treeCompare.statusPairFilter.select': 'Select status', 'treeCompare.statusPairFilter.to': 'To', 'treeCompare.suggestion.branchHead': 'Branch head', 'treeCompare.suggestion.previous': 'Previous commit', - 'treeCompare.suggestion.swap': 'Swap sides', + 'treeCompare.suggestion.swap': 'Swap base and target', 'treeCompare.suggestions': 'Suggestions', 'treeCompare.summaryTitle': 'Tree summary', + 'treeCompare.target': 'Target', 'treeDetails.bootsHistory': 'Boots History', 'treeDetails.branch': 'Branch', 'treeDetails.buildsHistory': 'Builds History', diff --git a/dashboard/src/pages/TreeCompare/components/CompareDetailSheet.tsx b/dashboard/src/pages/TreeCompare/components/CompareDetailSheet.tsx index fba25970d..348fc5f8b 100644 --- a/dashboard/src/pages/TreeCompare/components/CompareDetailSheet.tsx +++ b/dashboard/src/pages/TreeCompare/components/CompareDetailSheet.tsx @@ -64,7 +64,7 @@ function SideColumn({ logData, isLoading, }: { - labelId: 'treeCompare.sideA' | 'treeCompare.sideB'; + labelId: 'treeCompare.base' | 'treeCompare.target'; status: CompareItemStatus; id: string | null; logType: LogType; @@ -154,7 +154,7 @@ export function CompareDetailSheet({
setSort(current => cycleSort(current, key))} @@ -276,7 +276,7 @@ export function CompareBuildsFailuresTable({ setSort(current => cycleSort(current, key))} @@ -408,7 +408,7 @@ export function CompareBootsFailuresTable({ /> setSort(current => cycleSort(current, key))} @@ -416,7 +416,7 @@ export function CompareBootsFailuresTable({ setSort(current => cycleSort(current, key))} diff --git a/dashboard/src/pages/TreeCompare/components/RevisionSelector.tsx b/dashboard/src/pages/TreeCompare/components/RevisionSelector.tsx index ca68b07fb..ec7a1c823 100644 --- a/dashboard/src/pages/TreeCompare/components/RevisionSelector.tsx +++ b/dashboard/src/pages/TreeCompare/components/RevisionSelector.tsx @@ -68,11 +68,11 @@ function RevisionCard({ side === 'A' ? 'bg-blue' : 'bg-dim-gray', )} > - {side} + {side === 'A' ? 'B' : 'T'}
@@ -180,7 +180,7 @@ export function RevisionSelectorBar({ type="button" onClick={onSwap} className="bg-medium-gray flex h-10 w-10 items-center justify-center rounded-full" - aria-label="Swap sides" + aria-label="Swap base and target" > diff --git a/dashboard/src/utils/treeCompareDiff.ts b/dashboard/src/utils/treeCompareDiff.ts index e47e4cc9a..87dbde194 100644 --- a/dashboard/src/utils/treeCompareDiff.ts +++ b/dashboard/src/utils/treeCompareDiff.ts @@ -28,7 +28,7 @@ export function apiStatusToItemStatus( return status; } -/** Mirror backend _CHANGE_COUNT_SELECT categories for A→B transitions. */ +/** Mirror backend _CHANGE_COUNT_SELECT categories for base→target transitions. */ export function deriveCompareChange( statusA: CompareItemStatus, statusB: CompareItemStatus, diff --git a/docs/tree-compare.md b/docs/tree-compare.md index 919382620..90bd8efbf 100644 --- a/docs/tree-compare.md +++ b/docs/tree-compare.md @@ -4,9 +4,9 @@ High-level overview of the Tree Compare feature: side-by-side comparison of buil ## Purpose -Given a tree name and branch, pick two commit hashes (side A and side B) and answer: +Given a tree name and branch, pick two commit hashes (base and target) and answer: -- How did overall pass/fail/inconclusive counts move from A → B? +- How did overall pass/fail/inconclusive counts move from base → target? - Which individual builds / boots / tests changed category (regressions, fixes, new failures, etc.)? Entry point: **Compare revisions** on Tree Details (`TreeCompareLink`), which opens: @@ -15,14 +15,14 @@ Entry point: **Compare revisions** on Tree Details (`TreeCompareLink`), which op ## User flow -1. Open compare from Tree Details (current revision pre-fills as side B; side A defaults to the revision before it, so A → B reads oldest → newest). -2. Choose / swap revisions via the revision selector (commit history + shortcuts: previous commit, branch head, swap sides). +1. Open compare from Tree Details (current revision pre-fills as **Target**, **Base** defaults to the revision before it, so base → target reads oldest → newest). URL params stay `hashA` (base) and `hashB` (target). +2. Choose / swap revisions via the revision selector (commit history + shortcuts: previous commit, branch head, swap base and target). 3. Read the summary matrix (fixes, regressions, pass/fail/other counts per builds / boots / tests). 4. Drill into **Changed results** tabs (Builds / Boots / Tests). Quick change-type chips add or remove their status pairs; custom From/To pairs can be added too (default: `PASS → FAIL` and `FAIL → PASS`). Last edited pairs are remembered in localStorage when the URL omits `statusPair`. URL search state owns: `hashA`, `hashB`, `origin`, `currentPageTab`, and optional `statusPair`. -## Change categories (A → B) +## Change categories (base → target) Statuses are grouped into **PASS**, **FAIL**, and **INCONCLUSIVE** (everything else, including null). Absent on one side is treated as missing (`null` / `—`). @@ -34,7 +34,7 @@ Statuses are grouped into **PASS**, **FAIL**, and **INCONCLUSIVE** (everything e | `stillFailing` | FAIL → FAIL | | `newPass` | missing/INCONCLUSIVE → PASS | | `appeared` | missing → INCONCLUSIVE | -| `disappeared` | present on A, missing on B | +| `disappeared` | present on base, missing on target | | `unchanged` | same status on both sides (except FAIL → FAIL); frontend-only, has no backend count and no filter chip | Backend SQL aggregates (`_CHANGE_COUNT_SELECT` in `queries/tree.py`) and the frontend `deriveCompareChange` helper must stay in sync. From 2eb1efe45368630b85911f28f858d7b8b511e754 Mon Sep 17 00:00:00 2001 From: Felipe Bergamin Date: Fri, 2 Oct 2026 16:13:26 -0300 Subject: [PATCH 3/5] fixup! fix(tree-compare): label revisions as Base and Target Signed-off-by: Felipe Bergamin Co-authored-by: Cursor --- dashboard/src/locales/messages/index.ts | 8 ++-- .../src/pages/TreeCompare/TreeComparePage.tsx | 21 ++++---- .../components/CompareDetailSheet.tsx | 4 +- .../components/CompareFailuresTables.tsx | 4 +- .../components/RevisionSelector.tsx | 48 +++++++++---------- docs/tree-compare.md | 12 ++--- 6 files changed, 50 insertions(+), 47 deletions(-) diff --git a/dashboard/src/locales/messages/index.ts b/dashboard/src/locales/messages/index.ts index b5bd6bfd4..7804ebcfd 100644 --- a/dashboard/src/locales/messages/index.ts +++ b/dashboard/src/locales/messages/index.ts @@ -432,11 +432,12 @@ export const messages = { 'treeCompare.change.stillFailing': 'Still failing', 'treeCompare.change.unchanged': 'Unchanged', 'treeCompare.changed': 'Changed', + 'treeCompare.compare': 'Compare', 'treeCompare.description': - 'Compare pass/fail counts from a base revision to a target revision on the same tree and branch.', + 'Compare pass/fail counts of a revision against a base revision on the same tree and branch.', 'treeCompare.detail.missingSide': 'No result on this revision', 'treeCompare.drilldownHint': - 'Individual builds, boots, and tests that changed from the base revision to the target revision.', + 'Individual builds, boots, and tests that changed from the base revision to the compared revision.', 'treeCompare.failures.change': 'Change', 'treeCompare.failures.configArch': 'Config / Arch', 'treeCompare.failures.pathArch': 'Path / Arch', @@ -468,10 +469,9 @@ export const messages = { 'treeCompare.statusPairFilter.to': 'To', 'treeCompare.suggestion.branchHead': 'Branch head', 'treeCompare.suggestion.previous': 'Previous commit', - 'treeCompare.suggestion.swap': 'Swap base and target', + 'treeCompare.suggestion.swap': 'Swap base and compare', 'treeCompare.suggestions': 'Suggestions', 'treeCompare.summaryTitle': 'Tree summary', - 'treeCompare.target': 'Target', 'treeDetails.bootsHistory': 'Boots History', 'treeDetails.branch': 'Branch', 'treeDetails.buildsHistory': 'Builds History', diff --git a/dashboard/src/pages/TreeCompare/TreeComparePage.tsx b/dashboard/src/pages/TreeCompare/TreeComparePage.tsx index c04e80bd5..0131f3b83 100644 --- a/dashboard/src/pages/TreeCompare/TreeComparePage.tsx +++ b/dashboard/src/pages/TreeCompare/TreeComparePage.tsx @@ -58,7 +58,10 @@ import { } from './components/CompareFailuresTables'; import { CompareTestsGroupedTable } from './components/CompareTestsGroupedTable'; import { CompareSummary } from './components/CompareSummary'; -import { RevisionSelectorBar } from './components/RevisionSelector'; +import { + RevisionSelectorBar, + type RevisionSide, +} from './components/RevisionSelector'; const SHORT_HASH_LENGTH = 7; @@ -145,8 +148,8 @@ const TreeComparePage = (): JSX.Element => { }, [resolvedHashA, resolvedHashB, updateSearch]); const handleSideAction = useCallback( - (side: 'A' | 'B', action: 'previous' | 'branchHead') => { - const currentHash = side === 'A' ? resolvedHashA : resolvedHashB; + (side: RevisionSide, action: 'previous' | 'branchHead') => { + const currentHash = side === 'base' ? resolvedHashA : resolvedHashB; const currentIndex = revisions.findIndex(r => r.hash === currentHash); if (action === 'previous') { @@ -155,7 +158,7 @@ const TreeComparePage = (): JSX.Element => { Math.max(currentIndex, 0) + 1, ); const nextHash = revisions[previousIndex]?.hash ?? currentHash; - if (side === 'A') { + if (side === 'base') { updateSearch({ hashA: nextHash }); } else { updateSearch({ hashB: nextHash }); @@ -164,7 +167,7 @@ const TreeComparePage = (): JSX.Element => { } const headHash = revisions[0]?.hash ?? currentHash; - if (side === 'A') { + if (side === 'base') { updateSearch({ hashA: headHash }); } else { updateSearch({ hashB: headHash }); @@ -375,11 +378,11 @@ const TreeComparePage = (): JSX.Element => { error={commitsQuery.error} > updateSearch({ hashA: value })} - onHashBChange={value => updateSearch({ hashB: value })} + onBaseChange={value => updateSearch({ hashA: value })} + onCompareChange={value => updateSearch({ hashB: value })} onSideAction={handleSideAction} onSwap={handleSwap} /> diff --git a/dashboard/src/pages/TreeCompare/components/CompareDetailSheet.tsx b/dashboard/src/pages/TreeCompare/components/CompareDetailSheet.tsx index 348fc5f8b..013309b6b 100644 --- a/dashboard/src/pages/TreeCompare/components/CompareDetailSheet.tsx +++ b/dashboard/src/pages/TreeCompare/components/CompareDetailSheet.tsx @@ -64,7 +64,7 @@ function SideColumn({ logData, isLoading, }: { - labelId: 'treeCompare.base' | 'treeCompare.target'; + labelId: 'treeCompare.base' | 'treeCompare.compare'; status: CompareItemStatus; id: string | null; logType: LogType; @@ -162,7 +162,7 @@ export function CompareDetailSheet({ isLoading={logA.isLoading} /> setSort(current => cycleSort(current, key))} @@ -416,7 +416,7 @@ export function CompareBootsFailuresTable({ setSort(current => cycleSort(current, key))} diff --git a/dashboard/src/pages/TreeCompare/components/RevisionSelector.tsx b/dashboard/src/pages/TreeCompare/components/RevisionSelector.tsx index ec7a1c823..07ff831a4 100644 --- a/dashboard/src/pages/TreeCompare/components/RevisionSelector.tsx +++ b/dashboard/src/pages/TreeCompare/components/RevisionSelector.tsx @@ -16,7 +16,7 @@ import type { CompareRevision } from '@/types/tree/TreeCompare'; import { cn } from '@/lib/utils'; -type RevisionSide = 'A' | 'B'; +export type RevisionSide = 'base' | 'compare'; function TagChips({ tags }: { tags: string[] }): JSX.Element | null { if (tags.length === 0) { @@ -58,21 +58,21 @@ function RevisionCard({
- {side === 'A' ? 'B' : 'T'} + {side === 'base' ? 'B' : 'C'}
@@ -145,21 +145,21 @@ function RevisionCard({ } interface RevisionSelectorBarProps { - hashA: string; - hashB: string; + baseHash: string; + compareHash: string; revisions: CompareRevision[]; - onHashAChange: (hash: string) => void; - onHashBChange: (hash: string) => void; + onBaseChange: (hash: string) => void; + onCompareChange: (hash: string) => void; onSideAction: (side: RevisionSide, action: 'previous' | 'branchHead') => void; onSwap: () => void; } export function RevisionSelectorBar({ - hashA, - hashB, + baseHash, + compareHash, revisions, - onHashAChange, - onHashBChange, + onBaseChange, + onCompareChange, onSideAction, onSwap, }: RevisionSelectorBarProps): JSX.Element { @@ -167,12 +167,12 @@ export function RevisionSelectorBar({
onSideAction('A', 'previous')} - onBranchHead={() => onSideAction('A', 'branchHead')} + onSelect={onBaseChange} + onPrevious={() => onSideAction('base', 'previous')} + onBranchHead={() => onSideAction('base', 'branchHead')} />
@@ -180,7 +180,7 @@ export function RevisionSelectorBar({ type="button" onClick={onSwap} className="bg-medium-gray flex h-10 w-10 items-center justify-center rounded-full" - aria-label="Swap base and target" + aria-label="Swap base and compare" > @@ -190,12 +190,12 @@ export function RevisionSelectorBar({
onSideAction('B', 'previous')} - onBranchHead={() => onSideAction('B', 'branchHead')} + onSelect={onCompareChange} + onPrevious={() => onSideAction('compare', 'previous')} + onBranchHead={() => onSideAction('compare', 'branchHead')} />
diff --git a/docs/tree-compare.md b/docs/tree-compare.md index 90bd8efbf..9eb7fa0b2 100644 --- a/docs/tree-compare.md +++ b/docs/tree-compare.md @@ -4,9 +4,9 @@ High-level overview of the Tree Compare feature: side-by-side comparison of buil ## Purpose -Given a tree name and branch, pick two commit hashes (base and target) and answer: +Given a tree name and branch, pick two commit hashes (base and compare) and answer: -- How did overall pass/fail/inconclusive counts move from base → target? +- How did overall pass/fail/inconclusive counts move from base → compare? - Which individual builds / boots / tests changed category (regressions, fixes, new failures, etc.)? Entry point: **Compare revisions** on Tree Details (`TreeCompareLink`), which opens: @@ -15,14 +15,14 @@ Entry point: **Compare revisions** on Tree Details (`TreeCompareLink`), which op ## User flow -1. Open compare from Tree Details (current revision pre-fills as **Target**, **Base** defaults to the revision before it, so base → target reads oldest → newest). URL params stay `hashA` (base) and `hashB` (target). -2. Choose / swap revisions via the revision selector (commit history + shortcuts: previous commit, branch head, swap base and target). +1. Open compare from Tree Details (current revision pre-fills as **Compare**, **Base** defaults to the revision before it, so base → compare reads oldest → newest). URL params stay `hashA` (base) and `hashB` (compare). +2. Choose / swap revisions via the revision selector (commit history + shortcuts: previous commit, branch head, swap base and compare). 3. Read the summary matrix (fixes, regressions, pass/fail/other counts per builds / boots / tests). 4. Drill into **Changed results** tabs (Builds / Boots / Tests). Quick change-type chips add or remove their status pairs; custom From/To pairs can be added too (default: `PASS → FAIL` and `FAIL → PASS`). Last edited pairs are remembered in localStorage when the URL omits `statusPair`. URL search state owns: `hashA`, `hashB`, `origin`, `currentPageTab`, and optional `statusPair`. -## Change categories (base → target) +## Change categories (base → compare) Statuses are grouped into **PASS**, **FAIL**, and **INCONCLUSIVE** (everything else, including null). Absent on one side is treated as missing (`null` / `—`). @@ -34,7 +34,7 @@ Statuses are grouped into **PASS**, **FAIL**, and **INCONCLUSIVE** (everything e | `stillFailing` | FAIL → FAIL | | `newPass` | missing/INCONCLUSIVE → PASS | | `appeared` | missing → INCONCLUSIVE | -| `disappeared` | present on base, missing on target | +| `disappeared` | present on base, missing on compare | | `unchanged` | same status on both sides (except FAIL → FAIL); frontend-only, has no backend count and no filter chip | Backend SQL aggregates (`_CHANGE_COUNT_SELECT` in `queries/tree.py`) and the frontend `deriveCompareChange` helper must stay in sync. From c0c748ca85f5cd862408a5352ff002677c16f8ac Mon Sep 17 00:00:00 2001 From: Felipe Bergamin Date: Fri, 2 Oct 2026 16:24:45 -0300 Subject: [PATCH 4/5] refactor(tree-compare): use range=.. URL param Replace the hashA/hashB search params with a single git-style `range` param, `..`, so the URL reads the same way the page is labelled. Either side may be empty; the page still fills the missing side (compare defaults to the branch head, base to the revision before it) and rewrites the URL once both are known. The API request params (hash_a/hash_b) are unchanged. Part of #2130 Signed-off-by: Felipe Bergamin Co-authored-by: Cursor --- dashboard/e2e/tree-compare.spec.ts | 4 +- .../src/pages/TreeCompare/TreeCompareLink.tsx | 7 +- .../src/pages/TreeCompare/TreeComparePage.tsx | 70 +++++++++--------- dashboard/src/types/tree/TreeCompare.ts | 7 +- dashboard/src/utils/treeCompareDiff.test.ts | 74 ++++++++++++------- dashboard/src/utils/treeCompareDiff.ts | 35 ++++++--- docs/tree-compare.md | 6 +- 7 files changed, 122 insertions(+), 81 deletions(-) diff --git a/dashboard/e2e/tree-compare.spec.ts b/dashboard/e2e/tree-compare.spec.ts index 3d2fddcb2..19a4fd1d6 100644 --- a/dashboard/e2e/tree-compare.spec.ts +++ b/dashboard/e2e/tree-compare.spec.ts @@ -138,7 +138,7 @@ test('loads comparison data and opens a side-by-side details drawer', async ({ ); await page.goto( - `/tree/linux/master/compare?hashA=${HASH_A}&hashB=${HASH_B}&origin=maestro`, + `/tree/linux/master/compare?range=${HASH_A}..${HASH_B}&origin=maestro`, ); await expect(page.getByText('Tree summary')).toBeVisible(); @@ -304,7 +304,7 @@ test('drawer next stays on searched rows', async ({ page }) => { ); await page.goto( - `/tree/linux/master/compare?hashA=${HASH_A}&hashB=${HASH_B}&origin=maestro`, + `/tree/linux/master/compare?range=${HASH_A}..${HASH_B}&origin=maestro`, ); await page.getByPlaceholder('Search').fill('keep-'); diff --git a/dashboard/src/pages/TreeCompare/TreeCompareLink.tsx b/dashboard/src/pages/TreeCompare/TreeCompareLink.tsx index 3aa565489..14931daf0 100644 --- a/dashboard/src/pages/TreeCompare/TreeCompareLink.tsx +++ b/dashboard/src/pages/TreeCompare/TreeCompareLink.tsx @@ -6,6 +6,8 @@ import { FormattedMessage } from 'react-intl'; import { Button } from '@/components/ui/button'; +import { formatCompareRange } from '@/utils/treeCompareDiff'; + interface TreeCompareLinkProps { treeName: string; branch: string; @@ -24,7 +26,10 @@ export function TreeCompareLink({ s} > diff --git a/dashboard/src/pages/TreeCompare/TreeComparePage.tsx b/dashboard/src/pages/TreeCompare/TreeComparePage.tsx index 0131f3b83..840d15af9 100644 --- a/dashboard/src/pages/TreeCompare/TreeComparePage.tsx +++ b/dashboard/src/pages/TreeCompare/TreeComparePage.tsx @@ -41,11 +41,14 @@ import { } from '@/types/tree/TreeCompare'; import type { PossibleTabs } from '@/types/tree/TreeDetails'; import { + type CompareRange, applyStatusPairFilter, + formatCompareRange, mapBootOrTestDiffRows, mapBuildDiffRows, + parseCompareRange, readStoredStatusPairs, - resolveCompareHashes, + resolveCompareRange, resolveStatusPairs, serializeStatusPairs, writeStoredStatusPairs, @@ -68,9 +71,10 @@ const SHORT_HASH_LENGTH = 7; const TreeComparePage = (): JSX.Element => { const { formatMessage } = useIntl(); const { treeName, branch } = useParams({ from: compareRouteName }); - const { hashA, hashB, origin, currentPageTab, statusPair } = useSearch({ + const { range, origin, currentPageTab, statusPair } = useSearch({ from: compareRouteName, }); + const { base: baseHash, compare: compareHash } = parseCompareRange(range); const navigate = useNavigate({ from: compareNavigateFrom }); const commitsQuery = useCommits({ @@ -92,18 +96,19 @@ const TreeComparePage = (): JSX.Element => { [commitsQuery.data], ); - const { hashA: resolvedHashA, hashB: resolvedHashB } = useMemo( - () => resolveCompareHashes(revisions, hashA, hashB), - [revisions, hashA, hashB], + const { base: resolvedBase, compare: resolvedCompare } = useMemo( + () => + resolveCompareRange(revisions, { base: baseHash, compare: compareHash }), + [revisions, baseHash, compareHash], ); - const canCompare = Boolean(resolvedHashA && resolvedHashB); + const canCompare = Boolean(resolvedBase && resolvedCompare); const compareParams = { treeName, branch, - hashA: resolvedHashA, - hashB: resolvedHashB, + hashA: resolvedBase, + hashB: resolvedCompare, origin, }; @@ -114,16 +119,17 @@ const TreeComparePage = (): JSX.Element => { const updateSearch = useCallback( (updates: { - hashA?: string; - hashB?: string; + range?: Partial; currentPageTab?: PossibleTabs; statusPair?: string[]; }) => { navigate({ search: previous => ({ ...previous, - hashA: updates.hashA ?? previous.hashA, - hashB: updates.hashB ?? previous.hashB, + range: formatCompareRange({ + ...parseCompareRange(previous.range), + ...updates.range, + }), currentPageTab: updates.currentPageTab ?? previous.currentPageTab ?? @@ -138,18 +144,18 @@ const TreeComparePage = (): JSX.Element => { ); useEffect(() => { - if ((!hashA || !hashB) && resolvedHashA && resolvedHashB) { - updateSearch({ hashA: resolvedHashA, hashB: resolvedHashB }); + if ((!baseHash || !compareHash) && resolvedBase && resolvedCompare) { + updateSearch({ range: { base: resolvedBase, compare: resolvedCompare } }); } - }, [hashA, hashB, resolvedHashA, resolvedHashB, updateSearch]); + }, [baseHash, compareHash, resolvedBase, resolvedCompare, updateSearch]); const handleSwap = useCallback(() => { - updateSearch({ hashA: resolvedHashB, hashB: resolvedHashA }); - }, [resolvedHashA, resolvedHashB, updateSearch]); + updateSearch({ range: { base: resolvedCompare, compare: resolvedBase } }); + }, [resolvedBase, resolvedCompare, updateSearch]); const handleSideAction = useCallback( (side: RevisionSide, action: 'previous' | 'branchHead') => { - const currentHash = side === 'base' ? resolvedHashA : resolvedHashB; + const currentHash = side === 'base' ? resolvedBase : resolvedCompare; const currentIndex = revisions.findIndex(r => r.hash === currentHash); if (action === 'previous') { @@ -158,22 +164,14 @@ const TreeComparePage = (): JSX.Element => { Math.max(currentIndex, 0) + 1, ); const nextHash = revisions[previousIndex]?.hash ?? currentHash; - if (side === 'base') { - updateSearch({ hashA: nextHash }); - } else { - updateSearch({ hashB: nextHash }); - } + updateSearch({ range: { [side]: nextHash } }); return; } const headHash = revisions[0]?.hash ?? currentHash; - if (side === 'base') { - updateSearch({ hashA: headHash }); - } else { - updateSearch({ hashB: headHash }); - } + updateSearch({ range: { [side]: headHash } }); }, - [revisions, resolvedHashA, resolvedHashB, updateSearch], + [revisions, resolvedBase, resolvedCompare, updateSearch], ); const buildRows = useMemo( @@ -320,7 +318,7 @@ const TreeComparePage = (): JSX.Element => { params={{ treeName, branch, - hash: resolvedHashB, + hash: resolvedCompare, }} state={s => s} > @@ -365,7 +363,7 @@ const TreeComparePage = (): JSX.Element => {
s} > @@ -378,11 +376,13 @@ const TreeComparePage = (): JSX.Element => { error={commitsQuery.error} > updateSearch({ hashA: value })} - onCompareChange={value => updateSearch({ hashB: value })} + onBaseChange={value => updateSearch({ range: { base: value } })} + onCompareChange={value => + updateSearch({ range: { compare: value } }) + } onSideAction={handleSideAction} onSwap={handleSwap} /> diff --git a/dashboard/src/types/tree/TreeCompare.ts b/dashboard/src/types/tree/TreeCompare.ts index c77b7005c..19f506496 100644 --- a/dashboard/src/types/tree/TreeCompare.ts +++ b/dashboard/src/types/tree/TreeCompare.ts @@ -138,15 +138,14 @@ export type CompareFailureRow = | CompareTestFailureRow; export const compareDefaultValues = { - hashA: '', - hashB: '', + range: '', origin: 'maestro', currentPageTab: 'global.builds' as const, }; export const compareSearchSchema = z.object({ - hashA: z.string().catch(''), - hashB: z.string().catch(''), + /** `..` commit hashes; see `parseCompareRange`. */ + range: z.string().catch(''), origin: z .string() .default(compareDefaultValues.origin) diff --git a/dashboard/src/utils/treeCompareDiff.test.ts b/dashboard/src/utils/treeCompareDiff.test.ts index 95a77df6b..4e6e53a9e 100644 --- a/dashboard/src/utils/treeCompareDiff.test.ts +++ b/dashboard/src/utils/treeCompareDiff.test.ts @@ -1,13 +1,16 @@ import { describe, expect, it } from 'vitest'; import { + type CompareRange, deriveCompareChange, applyStatusPairFilter, + formatCompareRange, mapBootOrTestDiffRows, mapBuildDiffRows, compareRowNav, + parseCompareRange, parseStatusPairs, - resolveCompareHashes, + resolveCompareRange, resolveStatusPairs, serializeStatusPairs, toggleChangeTypePairs, @@ -45,40 +48,59 @@ describe('deriveCompareChange', () => { }); }); -describe('resolveCompareHashes', () => { +describe('compare range URL param', () => { + it('round-trips .., with either side empty', () => { + expect(parseCompareRange('old..new')).toEqual({ + base: 'old', + compare: 'new', + }); + expect(parseCompareRange('..new')).toEqual({ base: '', compare: 'new' }); + expect(parseCompareRange('old..')).toEqual({ base: 'old', compare: '' }); + expect(parseCompareRange('')).toEqual({ base: '', compare: '' }); + expect(formatCompareRange({ base: 'old', compare: 'new' })).toBe( + 'old..new', + ); + expect(formatCompareRange({ base: '', compare: 'new' })).toBe('..new'); + }); + + it('formats an empty range as empty so the param is stripped', () => { + expect(formatCompareRange({ base: '', compare: '' })).toBe(''); + }); +}); + +describe('resolveCompareRange', () => { const revisions = [{ hash: 'new' }, { hash: 'mid' }, { hash: 'old' }]; + const range = (base: string, compare: string): CompareRange => ({ + base, + compare, + }); - it('defaults to the head on B and the revision before it on A', () => { - expect(resolveCompareHashes(revisions, '', '')).toEqual({ - hashA: 'mid', - hashB: 'new', - }); + it('defaults compare to the head and base to the revision before it', () => { + expect(resolveCompareRange(revisions, range('', ''))).toEqual( + range('mid', 'new'), + ); }); - it('keeps A one revision older than an explicit B', () => { - expect(resolveCompareHashes(revisions, '', 'mid')).toEqual({ - hashA: 'old', - hashB: 'mid', - }); + it('keeps base one revision older than an explicit compare', () => { + expect(resolveCompareRange(revisions, range('', 'mid'))).toEqual( + range('old', 'mid'), + ); }); - it('never defaults B to the same revision as A', () => { - expect(resolveCompareHashes(revisions, 'new', '')).toEqual({ - hashA: 'new', - hashB: 'mid', - }); + it('never defaults compare to the same revision as base', () => { + expect(resolveCompareRange(revisions, range('new', ''))).toEqual( + range('new', 'mid'), + ); }); it('leaves explicit hashes alone and copes with too few revisions', () => { - expect(resolveCompareHashes(revisions, 'old', 'new')).toEqual({ - hashA: 'old', - hashB: 'new', - }); - expect(resolveCompareHashes([{ hash: 'only' }], '', '')).toEqual({ - hashA: '', - hashB: 'only', - }); - expect(resolveCompareHashes([], '', '')).toEqual({ hashA: '', hashB: '' }); + expect(resolveCompareRange(revisions, range('old', 'new'))).toEqual( + range('old', 'new'), + ); + expect(resolveCompareRange([{ hash: 'only' }], range('', ''))).toEqual( + range('', 'only'), + ); + expect(resolveCompareRange([], range('', ''))).toEqual(range('', '')); }); }); diff --git a/dashboard/src/utils/treeCompareDiff.ts b/dashboard/src/utils/treeCompareDiff.ts index 87dbde194..726c175a1 100644 --- a/dashboard/src/utils/treeCompareDiff.ts +++ b/dashboard/src/utils/treeCompareDiff.ts @@ -300,22 +300,37 @@ export function applyStatusPairFilter< ); } +export type CompareRange = { base: string; compare: string }; + +/** `range` URL param is git-style `..`; either side may be empty. */ +const COMPARE_RANGE_SEPARATOR = '..'; + +export function parseCompareRange(range: string): CompareRange { + const [base = '', compare = ''] = range.split(COMPARE_RANGE_SEPARATOR, 2); + return { base, compare }; +} + +export function formatCompareRange({ base, compare }: CompareRange): string { + return base || compare ? `${base}${COMPARE_RANGE_SEPARATOR}${compare}` : ''; +} + /** * Fill in missing sides so the default comparison runs oldest → newest: - * B defaults to the branch head, A to the revision right before B. + * compare defaults to the branch head, base to the revision right before it. * `revisions` is newest-first. */ -export function resolveCompareHashes( +export function resolveCompareRange( revisions: readonly { hash: string }[], - hashA: string, - hashB: string, -): { hashA: string; hashB: string } { - const resolvedB = - hashB || revisions.find(revision => revision.hash !== hashA)?.hash || ''; - const indexB = revisions.findIndex(revision => revision.hash === resolvedB); + { base, compare }: CompareRange, +): CompareRange { + const resolvedCompare = + compare || revisions.find(revision => revision.hash !== base)?.hash || ''; + const compareIndex = revisions.findIndex( + revision => revision.hash === resolvedCompare, + ); return { - hashA: hashA || revisions[indexB + 1]?.hash || '', - hashB: resolvedB, + base: base || revisions[compareIndex + 1]?.hash || '', + compare: resolvedCompare, }; } diff --git a/docs/tree-compare.md b/docs/tree-compare.md index 9eb7fa0b2..73a9455a5 100644 --- a/docs/tree-compare.md +++ b/docs/tree-compare.md @@ -11,16 +11,16 @@ Given a tree name and branch, pick two commit hashes (base and compare) and answ Entry point: **Compare revisions** on Tree Details (`TreeCompareLink`), which opens: -`/tree/{treeName}/{branch}/compare?hashA=…&hashB=…&origin=…` +`/tree/{treeName}/{branch}/compare?range={baseHash}..{compareHash}&origin=…` ## User flow -1. Open compare from Tree Details (current revision pre-fills as **Compare**, **Base** defaults to the revision before it, so base → compare reads oldest → newest). URL params stay `hashA` (base) and `hashB` (compare). +1. Open compare from Tree Details (current revision pre-fills as **Compare**, **Base** defaults to the revision before it, so base → compare reads oldest → newest). The `range` URL param is git-style `..`. 2. Choose / swap revisions via the revision selector (commit history + shortcuts: previous commit, branch head, swap base and compare). 3. Read the summary matrix (fixes, regressions, pass/fail/other counts per builds / boots / tests). 4. Drill into **Changed results** tabs (Builds / Boots / Tests). Quick change-type chips add or remove their status pairs; custom From/To pairs can be added too (default: `PASS → FAIL` and `FAIL → PASS`). Last edited pairs are remembered in localStorage when the URL omits `statusPair`. -URL search state owns: `hashA`, `hashB`, `origin`, `currentPageTab`, and optional `statusPair`. +URL search state owns: `range` (`..`), `origin`, `currentPageTab`, and optional `statusPair`. ## Change categories (base → compare) From ae9a94ae9e6d7a86c1ff502ed13e04e2dd09da47 Mon Sep 17 00:00:00 2001 From: Felipe Bergamin Date: Fri, 2 Oct 2026 16:47:36 -0300 Subject: [PATCH 5/5] fix(tree-compare): label grouped tests and kci-dev as Base/Compare Main's grouped tests table and tree-compare kci-dev button still referenced the old side names after the Base/Compare rename, so the frontend typecheck failed on the merge with main. Signed-off-by: Felipe Bergamin Co-authored-by: Cursor --- dashboard/src/pages/TreeCompare/TreeComparePage.tsx | 4 ++-- .../pages/TreeCompare/components/CompareTestsGroupedTable.tsx | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dashboard/src/pages/TreeCompare/TreeComparePage.tsx b/dashboard/src/pages/TreeCompare/TreeComparePage.tsx index 840d15af9..aea738ac8 100644 --- a/dashboard/src/pages/TreeCompare/TreeComparePage.tsx +++ b/dashboard/src/pages/TreeCompare/TreeComparePage.tsx @@ -346,8 +346,8 @@ const TreeComparePage = (): JSX.Element => { origin, gitUrl: compareQuery.data?.gitUrl, branch, - hashA: resolvedHashA || undefined, - hashB: resolvedHashB || undefined, + hashA: resolvedBase || undefined, + hashB: resolvedCompare || undefined, omittedFilters: statusPairs.length > 0 ? ['status-pair filter'] : [], })} diff --git a/dashboard/src/pages/TreeCompare/components/CompareTestsGroupedTable.tsx b/dashboard/src/pages/TreeCompare/components/CompareTestsGroupedTable.tsx index 7de4d463c..79956cef3 100644 --- a/dashboard/src/pages/TreeCompare/components/CompareTestsGroupedTable.tsx +++ b/dashboard/src/pages/TreeCompare/components/CompareTestsGroupedTable.tsx @@ -232,7 +232,7 @@ function createCompareSideColumns( sideStatus(row, 'sideA'), header: ({ column }): JSX.Element => (
- +
), cell: ({ row }): JSX.Element | null => { @@ -273,7 +273,7 @@ function createCompareSideColumns( sideStatus(row, 'sideB'), header: ({ column }): JSX.Element => (
- +
), cell: ({ row }): JSX.Element | null => {