diff --git a/README.md b/README.md index 3e67e46..0fdf26d 100644 --- a/README.md +++ b/README.md @@ -48,7 +48,7 @@ Builds are **unsigned**, so SmartScreen and Gatekeeper warn on first launch (the | ------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | **Compare** | Two files or pasted text, split or inline, word-level highlights, in-view search, and a live re-diff when a file changes on disk. | | **Understand structure** | JSON, YAML and XML compared as _data_: reordering keys or reformatting stops counting, and unchanged keys collapse away. | -| **Excel** | `.xlsx` workbooks as aligned grids — sheet tabs and cell-level highlights, with inserted rows and columns that don't cascade into false changes. Dates read as dates, hidden sheets and rows are marked, and formulas are compared as well as their results, so a total pasted over the formula behind it is caught rather than shown as unchanged. Set a tolerance — one of the presets or a threshold of your own, percentage or raw — and rounding noise stops counting; export the whole change list as a CSV. | +| **Excel** | `.xlsx` workbooks as aligned grids — sheet tabs and cell-level highlights, with inserted rows and columns that don't cascade into false changes. Dates read as dates, hidden sheets and rows are marked, and formulas are compared as well as their results, so a total pasted over the formula behind it is caught rather than shown as unchanged. Headers are found under a title row rather than assumed to be row 1, and rows can be paired by the columns that name them — one column or several — so the same export sorted differently reads as the one figure that moved instead of as a rewrite. Set a tolerance — one of the presets or a threshold of your own, percentage or raw — and rounding noise stops counting; export the whole change list as a CSV. | | **CSV** | `.csv` and `.tsv` compare as text or, one toggle away, as the same grid — rows aligned by their first column, quoted fields kept whole. | | **Huge files** | Past 32 MB a file is indexed by line instead of loaded, and the rows you're looking at are read from disk as you scroll — a multi-gigabyte log opens in seconds. Marked as streamed, with the few actions that need the whole text saying so rather than half-working. | | **Keep** | Saved diffs: encrypted, tagged, optionally auto-expiring. Drag a row onto another to arrange the list yourself; starred rows stay above the rest. Your open tabs come back on the next launch, and the strip can be told to close the oldest comparison to make room for a new one. | @@ -105,7 +105,7 @@ flowchart LR
- Two multi-sheet Excel workbooks compared as aligned grids: sheet tabs with per-sheet change counts, a tolerance control and a change-register export, changed cells boxed, and an added row and an inserted column each shown as a striped gap on the side without them + Two multi-sheet Excel workbooks compared as aligned grids: sheet tabs with per-sheet change counts, a tolerance control, a row-matching control and a change-register export, changed cells boxed, and an added row and an inserted column each shown as a striped gap on the side without them

Excel workbooks as aligned grids.

 diff --git a/docs/brand/roadmap.svg b/docs/brand/roadmap.svg index 6f4985d..4e90c40 100644 --- a/docs/brand/roadmap.svg +++ b/docs/brand/roadmap.svg @@ -5,7 +5,7 @@ sidebar. Track hues are real theme accents from src/renderer/src/utils/themes.js (Dim, Bloom, Neon, Beacon) — colour depth encodes sequence: solid now, faded later. Keep this in step with the items in docs/roadmap.md. --> - + @@ -34,32 +34,26 @@ font-family="ui-monospace, SFMono-Regular, Menlo, monospace" fill="#a99a7f">V0.4.27 · PLANNED - + Spreadsheet · finance 6 + font-family="ui-monospace, SFMono-Regular, Menlo, monospace" fill="#8d8168">4 - Row identity · key columns + Amounts read as amounts - Header row offset + Δ and net variance - Amounts read as amounts + Reading a big diff - Δ and net variance + Caps that announce themselves - - Reading a big diff - - - Caps that announce themselves - - - alignRows.js:72 + + numfmt.js diff --git a/docs/glossary.md b/docs/glossary.md index f0ecdd2..832cd16 100644 --- a/docs/glossary.md +++ b/docs/glossary.md @@ -83,7 +83,15 @@ where the concept lives in this repo. - **Shared strings** — an `.xlsx` de-duplicated text table (`sharedStrings.xml`) that cells reference by index. - **LCS** — _Longest Common Subsequence_, the classic diff algorithm; used to - align spreadsheet rows and to build the copy-as-patch output. + align spreadsheet rows by default and to build the copy-as-patch output. +- **Key column** — a column whose value names a row, so the two sides pair by + identity rather than by position or whole-row contents; several together make + a **composite key** (an account plus a cost centre). Chosen per sheet in the + grid's Match-rows panel and applied by `utils/matchRowsByKey.js`. It is what + lets the same export, sorted differently, read as the figures that moved. +- **Header row** — the row whose labels name the columns. Found by scanning the + first few rows (`headerPairing`), because an exported pack opens with a title + and a date before it; without one, columns pair by position. - **Monaco** — the VS Code editor component, used for the text diff view. - **Mermaid** — the text-to-diagram library used to render `mermaid` snippets. diff --git a/docs/roadmap.md b/docs/roadmap.md index 94d3e97..2f32b11 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -1,7 +1,7 @@ # Roadmap Roadmap board — four tracks. Spreadsheet · finance: row identity by key columns, header row offset, amounts read as amounts, delta and net variance, reading a big diff, caps that announce themselves. Diagrams: sequence · gantt · pie, click a change to pan to it. Comparing more: folder compare, image pairs, three-way merge — a decision first. Signing: macOS Developer ID, Windows deferred. + alt="Roadmap board — four tracks. Spreadsheet · finance: amounts read as amounts, delta and net variance, reading a big diff, caps that announce themselves. Diagrams: sequence · gantt · pie, click a change to pan to it. Comparing more: folder compare, image pairs, three-way merge — a decision first. Signing: macOS Developer ID, Windows deferred."> Board is `docs/brand/roadmap.svg` — hand-authored, edit it alongside the sections below. @@ -12,7 +12,9 @@ sections below. **Built** — formulas captured and normalised to R1C1 (`r1c1.js`), number formats (`numfmt.js`), materiality tolerance, change register, hidden state + error -cells, columns paired by header, `.csv`/`.tsv` through the same grid. +cells, columns paired by header — found under a title row, not assumed to be +row 1 — rows paired by the columns that name them, `.csv`/`.tsv` through the +same grid. ```mermaid flowchart TB @@ -27,9 +29,9 @@ flowchart TB sh --> ad["adapters/xlsxAdapter.js
adapters/csvAdapter.js — delimited text"] subgraph rend["renderer"] direction TB - ad --> ac["utils/alignColumns.js — pair columns by header"] + ad --> ac["utils/alignColumns.js — headerPairing
scans past a title row"] ac --> dw["utils/spreadsheetDiff.js — diffWorkbooks
utils/sheetCells.js — cell identity"] - dw --> al["utils/alignRows.js — rows + tolerance"] + dw --> al["utils/alignRows.js — signatures + tolerance
utils/matchRowsByKey.js — key columns"] al --> g["SpreadsheetGrid.vue"] al --> cr["utils/changeRegister.js --> diff:exportFile"] end @@ -44,30 +46,31 @@ trial balance, a GL export, a board pack — hits the gaps below. ```mermaid flowchart LR subgraph now["now"] - direction TB - a["1 · row identity
key columns · re-sorted rows"] - b["2 · header row offset"] - end - subgraph next["next"] direction TB c["3 · amounts read as amounts"] d["4 · Δ and net variance"] end - subgraph later["later"] + subgraph next["next"] direction TB e["5 · reading a big diff"] f["6 · caps that announce themselves"] end - now --> next --> later + now --> next ``` -- **1 · row identity** — `opts.keyColumn` exists (`spreadsheetDiff.js:78`) with - no UI, takes one column, and rows pair by LCS over row signatures: the same - export sorted differently reads as 100% changed. Key-based matching, - composite keys, duplicate-key detection -- **2 · header row offset** — `alignColumns.js:16` reads `rows[0]`. A title row - above the header fails `usable()` and drops silently to positional pairing, - which is the failure it was written to prevent +**1 · row identity** and **2 · header row offset** are done. + +- `matchRowsByKey.js` pairs rows by one column or several, wherever they sit, so + a re-sorted export reads as the figures that moved. Duplicate keys pair in + order of occurrence and are COUNTED, never hidden — the panel says so +- `headerPairing` scans for the first row that reads as a header (capped at 10) + instead of reading `rows[0]` and dropping to positional pairing; the band + names the row it used when it was not the first +- The hovered row is marked in BOTH grids — they are two ``s, so `:hover` + in one cannot reach the row aligned with it in the other + +**Open.** + - **3 · amounts read as amounts** — `numfmt.js` renders date, time and percent; everything else falls through to the raw float, so a P&L shows `1234567.891` and never `(1,234)`. A currency or rounding change is invisible today @@ -79,7 +82,7 @@ flowchart LR formulas sharing a prefix compare EQUAL; `maxMetaCells` (`sheet.js:137`) drops formula and format comparison past 100k cells; `csvAdapter` sets `truncated` and nothing renders it. A cap that hides is worse than a cap -- **Tolerance** takes a threshold of your own now (`useSpreadsheetDiff.js:7`, +- **Tolerance** takes a threshold of your own now (`useToleranceChoice.js`, percentage or raw), but it is still global and still `abs` OR `pct` (`alignRows.js:39-40`); materiality is "under €100 AND under 0.5%", per column. Date serials are now exempt (`meta.dt` — a percentage of 45870 is @@ -140,6 +143,9 @@ flowchart LR u --> v["DiagramDiffViewer.vue
+ change rail"] ``` +- A `.mmd` pair opens AS a picture — every load path takes the view from the + files, and a restored snapshot that recorded none does too rather than + falling back to text (`viewChrome.js` `restoredSemanticView`) - Status is encoded twice — colour AND stroke pattern — and the three tokens are held to a contrast floor and a pairwise ΔE floor on all 20 themes by `check-theme-depth.mjs` diff --git a/docs/screenshots/spreadsheet-diff.png b/docs/screenshots/spreadsheet-diff.png index 8a4abe7..018109a 100644 Binary files a/docs/screenshots/spreadsheet-diff.png and b/docs/screenshots/spreadsheet-diff.png differ diff --git a/e2e/spreadsheet.spec.mjs b/e2e/spreadsheet.spec.mjs index 20a093f..178acbb 100644 --- a/e2e/spreadsheet.spec.mjs +++ b/e2e/spreadsheet.spec.mjs @@ -333,3 +333,104 @@ test('exports the spreadsheet grid as a stitched picture', async ({ app, page }) rmSync(dir, { recursive: true, force: true }) } }) + +// A ledger with a title row above its header, re-sorted between the two files: +// changed almost end to end under whole-row matching, one moved amount and one +// dropped line when keyed on Account + Centre. +const ledgerRow = (r, account, centre, amount) => + `${inlineStr(`A${r}`, account)}${inlineStr(`B${r}`, centre)}` + + `${num(`C${r}`, amount)}` +const LEDGER_HEAD = + `${inlineStr('A1', 'Trial balance as at 30 Jun 2025')}` + + `${inlineStr('A2', 'Account')}${inlineStr('B2', 'Centre')}` + + `${inlineStr('C2', 'Amount')}` +const LEDGER_LEFT = buildXlsx( + LEDGER_HEAD + + ledgerRow(3, '1001', 'EMEA', 500) + + ledgerRow(4, '1001', 'APAC', 300) + + ledgerRow(5, '1200', 'EMEA', 120) + + ledgerRow(6, '4000', 'EMEA', 900) +) +const LEDGER_RIGHT = buildXlsx( + LEDGER_HEAD + + ledgerRow(3, '4000', 'EMEA', 900) + + ledgerRow(4, '1200', 'EMEA', 155) + + ledgerRow(5, '1001', 'APAC', 300) +) + +async function openLedger(app, page, dir) { + const leftPath = join(dir, 'ledger-before.xlsx') + const rightPath = join(dir, 'ledger-after.xlsx') + writeFileSync(leftPath, LEDGER_LEFT) + writeFileSync(rightPath, LEDGER_RIGHT) + await stubOpenDialog(app, [leftPath]) + await page.locator('.slot[data-side="left"]').click() + await stubOpenDialog(app, [rightPath]) + await page.locator('.slot[data-side="right"]').click() + await expect(page.locator('.grids')).toBeVisible() +} + +test('pairs re-sorted rows once the key columns are named', async ({ app, page }) => { + const dir = mkdtempSync(join(tmpdir(), 'diffbro-xlsx-key-')) + try { + await openLedger(app, page, dir) + + await expect(page.locator('.status-band')).toContainText('Header row 2') + + // Whole-row matching cannot see past the re-sort. + const band = page.locator('.status-band') + await expect(band).toContainText('3 removed') + + await page.getByTestId('row-match').click() + const panel = page.locator('.popover') + await expect(panel).toContainText('Rows pair by their whole contents') + // Account alone leaves two rows sharing a key, and the panel says so. + await panel.getByRole('checkbox').nth(0).check() + await expect(panel).toContainText('appears more than once') + + await panel.getByRole('checkbox').nth(1).check() + await expect(panel).not.toContainText('appears more than once') + await expect(panel).toContainText('Rows pair by the ticked columns') + await expect(page.getByTestId('row-match').locator('.btn-count')).toHaveText('2') + + await page.keyboard.press('Escape') + // One amount moved, one line gone — not a wholesale rewrite. + await expect(band).toContainText('1 changed') + await expect(band).toContainText('1 removed') + await expect(band).toContainText('0 added') + } finally { + rmSync(dir, { recursive: true, force: true }) + } +}) + +// The two grids are two
s: :hover in one cannot reach the row aligned +// with it in the other, so the mark has to be driven from the viewer. +test('hovering a row marks the same row in both grids', async ({ app, page }) => { + const dir = mkdtempSync(join(tmpdir(), 'diffbro-xlsx-hover-')) + try { + await openLedger(app, page, dir) + + const leftRows = page.locator('.grids > :first-child tbody tr') + const rightRows = page.locator('.grids > :nth-child(2) tbody tr') + await expect(page.locator('tbody tr.hover')).toHaveCount(0) + + await leftRows.nth(2).hover() + await expect(leftRows.nth(2)).toHaveClass(/hover/) + await expect(rightRows.nth(2)).toHaveClass(/hover/) + await expect(page.locator('tbody tr.hover')).toHaveCount(2) + + // Same height on both sides — carrying a figure across the divider is the point. + const l = await leftRows.nth(2).boundingBox() + const r = await rightRows.nth(2).boundingBox() + expect(Math.abs(l.y - r.y)).toBeLessThan(1) + + // The cue is painted, not merely classed. + const outline = await rightRows.nth(2).evaluate((el) => getComputedStyle(el).outlineWidth) + expect(parseFloat(outline)).toBeGreaterThan(0) + + await page.locator('.sheet-bar').hover() + await expect(page.locator('tbody tr.hover')).toHaveCount(0) + } finally { + rmSync(dir, { recursive: true, force: true }) + } +}) diff --git a/e2e/toolbar-view-menu.spec.mjs b/e2e/toolbar-view-menu.spec.mjs index d67a847..1dcd7bc 100644 --- a/e2e/toolbar-view-menu.spec.mjs +++ b/e2e/toolbar-view-menu.spec.mjs @@ -51,7 +51,7 @@ test('an unavailable option is disabled and says why', async ({ page }) => { // untouched comparison and mean nothing. test('the count chip tracks changed options, not enabled ones', async ({ page }) => { await pasteCompare(page, 'a\nb', 'a\nc') - const chip = page.locator('.count') + const chip = page.locator('.btn-count') await expect(chip).toBeHidden() // nothing changed yet, though Split view is on await openViewMenu(page) diff --git a/scripts/lib/legacySize.mjs b/scripts/lib/legacySize.mjs index d041208..09c4403 100644 --- a/scripts/lib/legacySize.mjs +++ b/scripts/lib/legacySize.mjs @@ -19,7 +19,7 @@ export const LEGACY_SIZE = { 'src/renderer/src/composables/useQuickLookKeys.js': { fn: 97 }, 'src/renderer/src/composables/useSnippetDraft.js': { fn: 182 }, 'src/renderer/src/composables/useSnippetPreview.js': { fn: 73 }, - 'src/renderer/src/composables/useSpreadsheetDiff.js': { fn: 73 }, + 'src/renderer/src/composables/useSpreadsheetDiff.js': { fn: 69 }, 'src/renderer/src/composables/useStreamedDiff.js': { fn: 99 }, 'src/renderer/src/composables/useTabContextMenu.js': { fn: 70 }, 'src/renderer/src/composables/useTagInput.js': { fn: 84 }, diff --git a/scripts/seed-local.mjs b/scripts/seed-local.mjs index 302ed64..14bf4f4 100644 --- a/scripts/seed-local.mjs +++ b/scripts/seed-local.mjs @@ -147,6 +147,44 @@ const FILES = { ] } ]), + // A title row above the header and the same ledger re-sorted: nearly all + // "changed" under Auto, one moved amount and one dropped line when keyed on + // Account + Centre. Account alone is a duplicate key, which the panel says. + 'ledger-before.xlsx': makeXlsx([ + { + name: 'Ledger', + rows: [ + ['Trial balance as at 30 Jun 2025'], + ['Account', 'Centre', 'Description', 'Amount'], + ['1001', 'EMEA', 'Bank', 128400.5], + ['1001', 'APAC', 'Bank', 41200], + ['1200', 'EMEA', 'Receivables', 96750.25], + ['1200', 'APAC', 'Receivables', 30110], + ['2100', 'EMEA', 'Payables', -74300], + ['4000', 'EMEA', 'Revenue', -412900], + ['5000', 'EMEA', 'Cost of sales', 233150.75], + ['6100', 'EMEA', 'Payroll', 118000], + ['6200', 'APAC', 'Travel', 9430.4] + ] + } + ]), + 'ledger-after.xlsx': makeXlsx([ + { + name: 'Ledger', + rows: [ + ['Trial balance as at 31 Jul 2025'], + ['Account', 'Centre', 'Description', 'Amount'], + ['6200', 'APAC', 'Travel', 9430.4], + ['4000', 'EMEA', 'Revenue', -412900], + ['1200', 'APAC', 'Receivables', 30110], + ['1001', 'APAC', 'Bank', 41200], + ['5000', 'EMEA', 'Cost of sales', 233150.75], + ['1200', 'EMEA', 'Receivables', 91020.25], + ['1001', 'EMEA', 'Bank', 128400.5], + ['2100', 'EMEA', 'Payables', -74300] + ] + } + ]), // A workbook wide enough that the grid scrolls sideways — what the image // export reports as columns it cannot reach. 'wide-metrics-a.xlsx': makeXlsx([ diff --git a/specs/2026-08-09-spreadsheet-row-identity/plan.md b/specs/2026-08-09-spreadsheet-row-identity/plan.md new file mode 100644 index 0000000..ec88aad --- /dev/null +++ b/specs/2026-08-09-spreadsheet-row-identity/plan.md @@ -0,0 +1,334 @@ +# Row identity and the header row — the Spreadsheet track's "now" + +| | | +|---|---| +| **Status** | shipped | +| **Progress** | 14 / 14 steps | +| **Branch** | `improvement/spreadsheet-row-identity` | +| **Started** | 2026-08-09 | +| **Finished** | 2026-08-09 | +| **Bugs found and fixed this iteration** | 3 code · 5 more found by `/validate` | +| **Token baseline** | 2026-08-09T17:38:10Z | +| **Claude tokens used** | 34,849,636 processed (171 requests) | + +## Problem + +`docs/roadmap.md` reopened the Spreadsheet track against _data_ rather than a +model, and put two items in **now**. Both are live defects, not wishes. + +**1 · row identity.** `opts.keyColumn` exists at `spreadsheetDiff.js:78` with no +UI, takes exactly one column, and is only consulted _inside an LCS gap_ +(`alignRows.js:114`). The outer pairing is an LCS over whole-row signatures +(`alignRows.js:202`), so re-sorting an export re-orders every signature and the +LCS finds almost no common subsequence: + +``` +left right (same data, sorted by amount) +1001, 500 1003, 120 +1002, 300 1002, 300 +1003, 120 1001, 500 +``` + +Today: `same, changed(1002 stays), same` at best, and on a real 400-row trial +balance sorted by a different column it reads as near-100% changed. There is no +way to say "row identity is the account code" — nor to say it is +*account + cost centre*, which is what a GL export actually keys on. + +**2 · header row offset.** `alignColumns.js:16` reads `rows[0]`. A workbook whose +first row is a title — `Trial balance as at 31 Dec 2025` in A1 and nothing else — +produces labels `['Trial balance…', '', '', '']`, which fails `usable()` +(`alignColumns.js:27`, blank label) and falls to `positional()`. Positional +pairing is exactly the failure the file's own header comment says it was written +to prevent: one inserted column then shifts every column after it. It fails +**silently** — nothing in the UI says the columns were paired by position. + +Neither has a test that would catch it, because neither was ever expressible. + +## Solution + +Two independent changes plus one UI surface. + +**Header row (2).** `alignColumns` scans the first `MAX_HEADER_SCAN` rows for the +first row whose labels are `usable()`, instead of reading `rows[0]` and giving +up. Each side scans independently — a title row added on one side only is the +common case. The detected index is exported so the viewer can say which row it +used when it is not row 1. The existing `paired * 2 > narrower` guard still +rejects a data row that happened to look usable, so the scan is strictly better +than the current behaviour: it only runs where the current code already gave up. + +**Row identity (1).** A new pure `utils/matchRowsByKey.js` pairs rows by a +composite key across the WHOLE sheet, bypassing the LCS. `spreadsheetDiff` +branches on `opts.keyColumns` (an array of paired-column indices): empty → today's +`alignRows`, non-empty → key matching. Duplicate keys pair in order of +occurrence and are counted, so the answer is never silently wrong. + +**Output order** follows the RIGHT file — the one the reader has open — with a +removed row emitted just before the first surviving left row that followed it. + +**UI.** A `RowMatchMenu.vue` in the grid-tools band: a `.btn btn-sm` with a count +chip, opening the shared `.popover` layer with one checkbox per paired column, +plus the duplicate-key warning. Per sheet, because column headers are. + +| option | why not | +|---|---| +| key matching inside `alignRows` | the file is 214 lines against a 250 cap, and the two matchers share nothing but the entry shape — one `if` at the top of a second matcher is honest, a mode flag threaded through `lcsOps`/`emitGap` is not | +| sort both sides by key, then align positionally | throws away the reader's own row order; a diff they cannot line up against the file on their screen | +| auto-detect the key column (first column with unique values) | a plausible guess that is wrong invisibly — an `id` column that is unique on the left and duplicated on the right silently changes what "changed" means. Recorded as out of scope, not rejected forever | +| header row as a user-set number | the detection is unambiguous where it matters (blank/duplicate labels vs. a clean header). A control for it is chrome for a case the scan already covers; revisit if the scan is ever seen picking wrong | +| a `BaseDialog` for the key picker | it is a one-of-N-columns choice made while looking at the grid, which is what `.popover` is for (`ViewOptionsMenu`) — a modal would cover the columns being chosen | + +## Scope + +**In:** + +- header-row scan in `alignColumns.js`, exported index, surfaced in the status band +- `utils/matchRowsByKey.js` — composite keys, duplicate detection, right-file order +- `spreadsheetDiff` branch on `opts.keyColumns`; `headerRows` + `duplicateKeys` on the sheet result +- `useSpreadsheetDiff` — per-sheet key selection +- `RowMatchMenu.vue` + its stylesheet; the count chip lifted to `ui.css` as `.btn-count` +- i18n catalogue + `en-XA`; seed pair; unit + e2e tests; roadmap prose + SVG +- **added mid-build, on request:** the hovered row marked in BOTH grids — see Decisions + +**Out:** *(recorded, not drifted into)* + +- **Δ and net variance** (roadmap item 4) — the register keeps its six columns +- **amounts read as amounts** (item 3) — `numfmt.js` untouched +- auto-detecting a key when none is chosen — Auto stays exactly today's LCS +- case-folding or numeric coercion of key values — see Decisions +- `keyColumn` (singular) on `alignRows` stays as-is; it is the in-gap pairing and + is still what Auto uses + +## Design + +The trigger is `.btn .btn-sm` from `ui.css` — the resting three-cue face +(`--btn-face` / `--btn-edge` / `--shadow-1`), never `.btn-ghost`: it stands alone +in the band with no primary beside it. Panel is the shared `.popover` +(`--bg-elevated`, `1px solid var(--border)`, `--radius-lg`, `--shadow-2`), rows +are `.popover-row` at `--control-h`. The count chip is `--chip-h` with a +`--btn-face-press` face and `--text` ink — moved verbatim out of +`ViewOptionsMenu.css` into `ui.css`, because a second component now needs it and +a scoped copy is this repo's recurring drift. + +Secondary text inside the panel (the Auto explanation, the duplicate-key warning) +is `--text-hint`, **not** `--text-dim`: dim's 3.0 floor is held against +`--bg-panel` and this panel is `--bg-elevated`, where `make theme-sweep` measured +2.92 on nord and 2.82 on sepia. `ui.css` already records that. + +The header-row note in the status band is plain `--text-dim` body text in the +existing `.status-band`, which is `--bg-panel` — the floored pair. + +No new colour, radius or font-size literal; nothing accent-filled behind a label; +no glow. + +### Theme verdict — all 20 + +Parsed from `styles/themes.css` (`--bg` for the ground, `--border`, `--accent`). +Every surface here is an existing `ui.css` layer already held to floors by +`check-theme-depth.mjs`, so the verdict is about what the new markup composes, +not a new palette. + +| theme | ground (`--bg`) | verdict | note | +|---|---|---|---| +| light | `#ffffff` (canvas inverted) | ok | popover is `--bg-elevated`, floats on the tinted canvas as every other panel does | +| dark | `#0d1117` | ok | | +| solar | `#fffdf6` | ok | | +| neon | `#090d18` | ok | accent `#22d3ee` only on the checkbox tick and `:focus-visible` — no fill under a label, nothing to halo | +| nord | `#2e3440` | ok | `--text-hint` chosen for exactly this theme (dim measured 2.92 here) | +| sepia | `#e9dcbe` | ok | as nord — dim measured 2.82 | +| dim | `#1b1917` | ok | accent `#d9a441` | +| beacon | `#000000` | ok | hard keyline `#e0e0e0` — popover keeps its `1px solid var(--border)`; nothing here removes a border | +| meridian | `#f5f7f4` | ok | | +| linen | `#faf7f0` | ok | | +| bloom | `#f9f4f5` | ok | | +| nyan | `#160a20` | ok | accent `#ff2ecb` — tick and focus ring only | +| matrix | `#020a04` | ok | accent `#00ff41` — same | +| contrast | `#ffffff` | ok | hard keyline `#111111`, kept | +| volcano | `#000000` | ok | border `#ffc9a4`, accent `#ff5c33` | +| amber | `#0f0a02` | ok | accent `#ffb000` | +| tide | `#0b1a1e` | ok | | +| ember | `#1a1013` | ok | | +| graphite | `#161616` | ok | achromatic; chip face is `--text` at 30%, so it steps toward the ink rather than picking a grey | +| vector | `#ffffff` | ok | | + +**The row-hover mark** is the one genuinely new painted surface, so it was read +off real frames rather than reasoned about: a 2px `--accent` outline inset into +the row, plus `--bg-hover` on the gutter. Captured from a running app on +**light · matrix · beacon · contrast · sepia · nyan · volcano** — the two hard-keyline +contracts, the three high-chroma accents, and one light warm ground. At 1px it +read as "a slightly different border" on beacon (`#4cc2ff` beside a `#e0e0e0` +keyline) and contrast; 2px at `-2px` offset separates the mark from the keyline +and reads on all of them. It never removes a border, never fills behind a label +and never glows, so the three rules the high-chroma themes exist to catch do not +apply. + +Verified by `npm run check:themes` (token floors, 20 themes, green) and re-read in +the running app before the plan closed. + +## Security rules touched + +**None crossed.** No IPC handler, no fs, no crypto, no `shell.*`, no new +dependency, no external link, no `v-html`. Everything added is renderer-side pure +JS over data the main process already parsed and validated. + +Two adjacent rules stay honoured: + +- **Rule 6 (untrusted input is hostile).** Key values come from cells in a file a + user chose. They are used only as `Map` keys and rendered through Vue text + interpolation — never as a selector, a path or markup. The composite key joins + on `\u0000` (NUL), which cannot appear in a spreadsheet cell's text, so two different + key tuples cannot collide into one. +- **Rule 6 again, caps.** The header scan is capped (`MAX_HEADER_SCAN`), the key + picker only lists paired columns and the popover scrolls rather than growing + without bound. Key matching is O(n) hashing — cheaper than the LCS it replaces, + so no new budget is needed. + +## Test plan + +Written before the code; each of the two defects gets its failing test first. + +- **unit** — `tests/renderer/utils/alignColumns.test.js`: a title row above the + header pairs by header, not by position (**red first**: today it returns + `positional()`); a header found at row 2 on the left and row 0 on the right + still pairs; nothing usable in the first ten rows still falls back; a data-only + sheet is unchanged. +- **unit** — `tests/renderer/utils/matchRowsByKey.test.js` (new): re-sorted rows + all pair (**red first**: `alignRows` on the same input reports changes); + composite key over two columns; a duplicate key pairs in order of occurrence + and is counted; unmatched left → removed, unmatched right → added; tolerance is + respected; the emitted order follows the right file with removed rows in place; + a blank key is a key, not a wildcard. +- **unit** — `tests/renderer/utils/spreadsheetDiff.test.js`: `opts.keyColumns` + reaches the matcher and the reported `changed` indices are still in DISPLAY + column space; `headerRows` and `duplicateKeys` on the sheet result. +- **unit** — `tests/renderer/composables/useSpreadsheetDiff.test.js`: key columns + are per sheet and survive a sheet switch; clearing returns to Auto. +- **e2e** — `e2e/spreadsheet.spec.mjs`: build a re-sorted pair with a title row, + open it, assert the grid reports the sheets as near-fully-changed under Auto, + pick the key column in the Match-rows popover, assert the change count collapses + to the one real edit. The popover, the per-sheet memory and the real parse only + exist in a launch. +- **red → green** — recorded per test in the Validation section with the actual + failure text. +- **seed fixtures** — `scripts/seed-local.mjs` `FILES` gains + `ledger-before.xlsx` / `ledger-after.xlsx`: a title row in A1, a header on row + 2, the same twelve ledger lines re-sorted, one amount moved and one line + dropped. Without it neither defect is reproducible by hand on the host Mac. + Keeps the `seed` tag; `local-seed-clean` removes it by the same glob. + +## Docs impact + +| surface | needed? | what changes | +|---|---|---| +| `README.md` | **yes** | the spreadsheet feature list says columns pair by header; it now also says rows can pair by key columns | +| `docs/screenshots/*.png` | **yes** — planned as "no", corrected on check | the plan guessed the tools band was off-frame. It is not: `spreadsheet-diff` shows the whole window, tolerance control included, so Match rows landed in it. Recaptured with `make screenshots SHOTS=spreadsheet-diff` (the container — the script drives the in-app menu bar, which macOS replaces with the native one), and the README `alt` updated with it | +| `docs/roadmap.md` | **yes** | items 1 and 2 leave **now**; 3 and 4 move up. Mermaid + terse bullets, no prose | +| `docs/brand/roadmap.svg` | **yes** | the Spreadsheet card's chips count the rows on the card — hand-authored, edited in the same change | +| `docs/*.md` | **no** | no IPC surface (`ipc-security.md`), no crypto (`security.md`), no new standard (`standards.md`). `glossary.md` — checked: "key column" is worth an entry, added | + +## Implementation plan + +- [x] 1. Branch `improvement/spreadsheet-row-identity`, record the token baseline +- [x] 2. **Red:** header-offset cases in `alignColumns.test.js`; watch them fail +- [x] 3. **Green:** header-row scan in `alignColumns.js`, `headerRowIndex` exported +- [x] 4. **Red:** `matchRowsByKey.test.js`; watch it fail (module absent, then behaviour) +- [x] 5. **Green:** `utils/matchRowsByKey.js` +- [x] 6. `spreadsheetDiff.js` — branch on `opts.keyColumns`, expose `headerRows` + and `duplicateKeys`; extend `spreadsheetDiff.test.js` +- [x] 7. `useSpreadsheetDiff.js` — per-sheet key columns; extend its test +- [x] 8. `.count` chip → `ui.css`; `ViewOptionsMenu.css` drops its copy +- [x] 9. `RowMatchMenu.vue` + `styles/RowMatchMenu.css` on `usePopover` +- [x] 10. `SpreadsheetDiffViewer.vue` — mount it, add the header-row note +- [x] 11. i18n keys in `en.json`, regenerate `en-XA`; `check:i18n` + `check:rawtext` clean +- [x] 12. Seed pair in `seed-local.mjs`; `make local-seed` opens it by hand +- [x] 13. e2e in `spreadsheet.spec.mjs` +- [x] 14. Docs: `README.md`, `docs/roadmap.md`, `docs/brand/roadmap.svg`, `glossary.md` + +## Decisions + +| date | decision | why | rejected | +|---|---|---|---| +| 2026-08-09 | key values are `String(value).trim()` — no case-folding, no numeric coercion | trimming fixes an invisible defect (a trailing space in an export). Folding case would silently merge `ACC-1` and `acc-1`, which in a ledger are two accounts | case-insensitive keys; `Number()` coercion so `'1001'` matches `1001` — cross-format keying is a separate decision with its own failure mode | +| 2026-08-09 | display order follows the RIGHT file | it is the file the reader has open; a removed row is emitted just before the first surviving left row that followed it, so it keeps its context | sorting by key (normalises away the reader's own order); left order (the "after" file is what is being reviewed) | +| 2026-08-09 | key columns are per sheet, held in the composable | the columns differ per sheet, so one global choice is wrong on the second tab | one workbook-wide setting; persisting to the store — the choice is about a comparison, not about the app | +| 2026-08-09 | duplicate keys pair in occurrence order and are COUNTED, never hidden | roadmap item 6 is "a cap that hides is worse than a cap"; the same holds for an ambiguity | refusing to match on a duplicate; matching many-to-one (that is reconciliation, an explicitly undecided track) | +| 2026-08-09 | the header scan is independent per side | the failure case is a title row added on ONE side; a shared index cannot express it | one index for both | +| 2026-08-09 | Auto keeps today's LCS untouched | changing the default matcher is a behaviour change for every existing diff, with no failing case behind it | auto-detecting a key column | +| 2026-08-09 | **scope added on request:** the hovered row is marked in BOTH grids | asked for mid-build. It belongs to this change: keyed rows no longer sit opposite their own position, so carrying a figure across the divider by eye is exactly what got harder | — | +| 2026-08-09 | the hover mark is an `outline` on the `tr`, plus the gutter face | every row and cell state here is already a `background`, so a veil would REPLACE the ghost's stripes and the changed cell's tint; and a wider border would move the rows the virtualization measures (`--grid-row-h` is one number shared by the CSS and the spacer arithmetic) | a background veil; a 2px border; an inset `box-shadow` (collides with `cell-chg` and `err`, which already use one) | +| 2026-08-09 | 2px outline at `-2px` offset, not 1px | 1px read as "a slightly different border" on beacon and contrast, whose hard `--border` is a contract. Measured on rendered frames, not guessed | 1px | +| 2026-08-09 | `RowMatchMenu` renders its backdrop inline, NOT teleported to body | `.content` carries `isolation: isolate`, so a body-level backdrop at z-index 20 outranks the whole diff subtree and swallowed every click meant for the panel. The backdrop is `position: fixed`, so it covers the window from where it is | `Teleport to="body"` (what `ViewOptionsMenu` does — it lives outside `.content`) | +| 2026-08-09 | two choice-composables split out of `useSpreadsheetDiff` | the function hit its 73-line ratchet. `useToleranceChoice` and `useKeyColumns` are the two decisions in it; the diff itself is what is left | raising the cap | + +## Validation + +- [x] `/validate` — four passes run; nine comment trims, two untyped boundary + props, one missing test mirror and one stale screenshot found and fixed in + this change. `quality-audit.md` closes with **no open findings** +- [x] `npm run check` — **green.** lint · style tokens (109 stylesheets) · theme + depth (20 themes) · structure · i18n (1224 keys, 1224 used) · raw text + (0, held at baseline) · 3077 tests passed, 2 skipped · build. Coverage + **95.34 / 88.47 / 95.75 / 96.33** against floors 93 / 86 / 92 / 95 +- [x] e2e on the host, `E2E_HIDDEN=1 E2E_WORKERS=1`: `spreadsheet` (9/9, two of + them new) plus `csv-grid`, `toolbar-view-menu`, `view-toggles`, + `virtual-rows`, `diff-zoom`, `locale`, `ui-affordances` — 63 passed +- [x] UI seen running: the grid, the panel and the hover mark captured off a real + launch on **light, matrix, beacon, contrast, sepia, nyan, volcano**; the + board SVG rendered offscreen and read back before it was committed +- [x] every Docs-impact "yes" done — `README.md`, `docs/roadmap.md`, + `docs/brand/roadmap.svg`, `docs/glossary.md` +- [ ] `make local-seed` — **not run.** It writes into the user's real DiffBro + install (userData: saved diffs, snippets, trusted keys), which is not mine + to change without being asked. The fixture itself is proven instead: the + e2e builds the same title-row-then-header, re-sorted shape and drives it + end to end, and `makeXlsx` already writes the four existing pairs +- [x] token usage measured, header row filled + +### Red → green, recorded + +| test | failure seen first | +|---|---| +| `alignColumns.test.js` — header below a title row (7 cases) | `expected [ 'Trial balance as at 31 Dec 2024', '', '' ] to deeply equal [ 'Account', 'Debit', 'Credit' ]`, and `headerPairing is not a function` | +| `matchRowsByKey.test.js` (14 cases) | `Failed to resolve import ".../utils/matchRowsByKey"` | +| `useSpreadsheetDiff.test.js` — "starts a new pair of files on Auto" | re-verified by deleting the `watch` and watching it fail alone | +| `alignColumns.test.js` — "gives up rather than scanning an unbounded preamble" | a cap test cannot fail before the cap exists, so it was proven by raising `MAX_HEADER_SCAN` to 20 and watching it fail alone | + +Three of the seven header cases **passed** on the first run — positional pairing +happened to give the same answer. They were rewritten to insert a column, so +header pairing and positional pairing disagree and the assertion can tell them +apart. A test that passes before the fix guards nothing. + +### Bugs found and fixed + +1. **`keyColumnsOf` accepted an out-of-range column.** `c?.left !== null` reads + `undefined !== null` → true, so `columns[9]` on a 3-column sheet reached + `picked.map((c) => c.left)` and threw. Caught by the "ignores a key column + only one side has" test; fixed to `c && c.left !== null && c.right !== null`. +2. **The Match-rows panel could not be clicked.** `.content` carries + `isolation: isolate`; the body-teleported backdrop at z-index 20 outranked the + whole subtree, so every checkbox click hit the backdrop and closed the panel. + Caught by the new e2e, fixed by rendering the backdrop inline. +3. **`e2e/toolbar-view-menu.spec.mjs` selected `.count`,** which the chip lost + when it moved to `ui.css` as `.btn-count`. Caught by running the affected + specs rather than only the new ones. + +### Token usage + +Window `2026-08-09T17:38:10Z` → now, 171 requests. + +| category | tokens | +|---|---:| +| input | 342 | +| output | 109,136 | +| cache write | 206,072 | +| cache read | 34,534,086 | +| **total** | **34,849,636** | + +Cache read dominates, so the total is tokens *processed*, not a bill. The window +is wall-clock and the session worked on nothing else. + +**Outcome:** both "now" items on the Spreadsheet track are done, and the board +moves 3 and 4 up. A re-sorted trial balance now reads as the one figure that +moved instead of a wholesale rewrite, a header found under a title row is named +in the band rather than silently abandoned for positional pairing, and the +hovered row is marked in both grids. Three bugs were found and fixed on the way, +two of them by tests written before the code. diff --git a/src/renderer/src/components/RowMatchMenu.vue b/src/renderer/src/components/RowMatchMenu.vue new file mode 100644 index 0000000..3ed772d --- /dev/null +++ b/src/renderer/src/components/RowMatchMenu.vue @@ -0,0 +1,89 @@ + + + + + diff --git a/src/renderer/src/components/SpreadsheetDiffViewer.vue b/src/renderer/src/components/SpreadsheetDiffViewer.vue index 9962ec5..156b612 100644 --- a/src/renderer/src/components/SpreadsheetDiffViewer.vue +++ b/src/renderer/src/components/SpreadsheetDiffViewer.vue @@ -1,7 +1,8 @@ + + diff --git a/src/renderer/src/components/ViewOptionsMenu.vue b/src/renderer/src/components/ViewOptionsMenu.vue index 6634ee9..fcd5056 100644 --- a/src/renderer/src/components/ViewOptionsMenu.vue +++ b/src/renderer/src/components/ViewOptionsMenu.vue @@ -72,7 +72,7 @@ defineExpose({ close }) @click="toggle" > {{ $t('appToolbar.view') }} - {{ changedCount }} + {{ changedCount }}
diff --git a/src/renderer/src/components/styles/RowMatchMenu.css b/src/renderer/src/components/styles/RowMatchMenu.css new file mode 100644 index 0000000..e9cfd14 --- /dev/null +++ b/src/renderer/src/components/styles/RowMatchMenu.css @@ -0,0 +1,24 @@ +/* The Match-rows button and its panel (RowMatchMenu.vue). The shell — surface, + keyline, rows, the hint ink — is the shared .popover layer in ui.css; only + what is genuinely local lives here. */ + +.popover { + min-width: 240px; +} +/* A sheet can carry more columns than a panel can be tall, so the list scrolls + rather than the panel growing past the window. */ +.key-list { + max-height: 260px; + overflow-y: auto; +} +/* The two hint lines are direct children of the panel, so they take the row's + own inline padding to line up with the checkboxes. */ +.popover-why { + padding: var(--space-1) var(--space-3); +} +.lead { + padding-top: 0; +} +.clear { + color: var(--text-hint); +} diff --git a/src/renderer/src/components/styles/SpreadsheetGrid.css b/src/renderer/src/components/styles/SpreadsheetGrid.css index 2b9042f..ed87520 100644 --- a/src/renderer/src/components/styles/SpreadsheetGrid.css +++ b/src/renderer/src/components/styles/SpreadsheetGrid.css @@ -102,6 +102,24 @@ box-shadow: inset 2px 0 0 color-mix(in srgb, var(--text-dim) 70%, transparent); } +/* The row under the pointer, marked in BOTH grids — they are two tables, so + :hover in one cannot reach the row opposite it. Neither cue touches layout or + the row's own background: every row and cell state here is ALREADY a + background, so a veil would replace the ghost's stripes rather than sit over + them, and a wider border would move the rows the virtualization measures. */ +.grid tr.hover { + outline: 2px solid var(--accent); + /* Fully INSIDE the row, so the mark sits beside the cell keyline rather than + on top of it — beacon and contrast carry a hard --border by contract, and a + hairline sharing that edge was the one theme pair where the mark read as a + slightly different border rather than as a mark. */ + outline-offset: -2px; +} +.grid tr.hover .rownum { + background: var(--bg-hover); + color: var(--text); +} + /* Ghost: the aligned gap opposite an added/removed row or column, striped so it reads as "nothing here" while keeping both grids cell-for-cell aligned. */ .grid tr.ghost td, diff --git a/src/renderer/src/components/styles/ViewOptionsMenu.css b/src/renderer/src/components/styles/ViewOptionsMenu.css index 50d25da..4cff31b 100644 --- a/src/renderer/src/components/styles/ViewOptionsMenu.css +++ b/src/renderer/src/components/styles/ViewOptionsMenu.css @@ -17,20 +17,3 @@ min-height: auto; padding-top: var(--space-1); } -/* How many options the reader has changed from their defaults. Height comes from - --chip-h, never from padding — that is how the Esc chip drifted to 19px beside - a 26px button. Face is the theme's own ink at 30%, the same step .btn.active - takes: --accent as a fill under a label scores under 4.5:1 on five themes. */ -.count { - min-width: var(--chip-h); - height: var(--chip-h); - padding: 0 var(--space-1); - border-radius: var(--radius-pill); - background: var(--btn-face-press); - color: var(--text); - font-size: var(--font-xs); - font-weight: 700; - display: inline-flex; - align-items: center; - justify-content: center; -} diff --git a/src/renderer/src/composables/useKeyColumns.js b/src/renderer/src/composables/useKeyColumns.js new file mode 100644 index 0000000..ef2479e --- /dev/null +++ b/src/renderer/src/composables/useKeyColumns.js @@ -0,0 +1,36 @@ +import { computed, ref, watch } from 'vue' + +/** + * Which columns name a row, held per sheet — the columns differ per sheet, so + * one workbook-wide choice would be wrong on the second tab. A Map rather than + * an object: the sheet name comes from the file, and `__proto__` must not + * resolve to anything. + * @param {() => string|undefined} sheetName the sheet the choice applies to + * @param {Array<() => unknown>} resetOn sources that mean "a different pair of + * files": keys are held by NAME, and two workbooks routinely share one. + */ +export function useKeyColumns(sheetName, resetOn = []) { + const bySheet = ref(new Map()) + + if (resetOn.length) watch(resetOn, () => bySheet.value.clear()) + + const activeKeyColumns = computed(() => bySheet.value.get(sheetName()) ?? []) + + function toggleKeyColumn(index) { + const name = sheetName() + if (!name) return + const current = activeKeyColumns.value + const next = current.includes(index) + ? current.filter((i) => i !== index) + : [...current, index].sort((a, b) => a - b) + if (next.length) bySheet.value.set(name, next) + else bySheet.value.delete(name) + } + + function clearKeyColumns() { + const name = sheetName() + if (name) bySheet.value.delete(name) + } + + return { bySheet, activeKeyColumns, toggleKeyColumn, clearKeyColumns } +} diff --git a/src/renderer/src/composables/useSpreadsheetDiff.js b/src/renderer/src/composables/useSpreadsheetDiff.js index 4378c89..5ea420e 100644 --- a/src/renderer/src/composables/useSpreadsheetDiff.js +++ b/src/renderer/src/composables/useSpreadsheetDiff.js @@ -1,49 +1,26 @@ import { computed, ref } from 'vue' import { useDiffStore } from '../stores/diffStore' import { diffWorkbooks } from '../utils/spreadsheetDiff' - -// An absolute floor kills float-rounding noise; a percentage is what -// materiality means. Neither covers the other, so both are offered. -export const TOLERANCES = [ - { value: 'exact', label: 'Exact', tolerance: null }, - { value: 'abs', label: '±0.01', tolerance: { abs: 0.01 } }, - { value: 'half', label: '±0.5%', tolerance: { pct: 0.5 } }, - { value: 'one', label: '±1%', tolerance: { pct: 1 } }, - // The threshold the engagement set, which is never one of four numbers. - { value: 'custom', label: 'Custom', tolerance: null } -] - -// Explicit, never sniffed from the text: "50" meaning half a percent or fifty -// pounds is not something to guess at. -export const TOLERANCE_UNITS = [ - { value: 'pct', label: '%' }, - { value: 'abs', label: 'abs' } -] +import { useKeyColumns } from './useKeyColumns' +import { useToleranceChoice } from './useToleranceChoice' // Per-sheet diff of the two loaded spreadsheets + the active tab. export function useSpreadsheetDiff() { const store = useDiffStore() const active = ref(0) - const showFormulas = ref(false) - const toleranceId = ref('exact') - const customValue = ref('') - const customUnit = ref('pct') - // Empty, unparseable or non-positive reads as Exact: a zero threshold forgives - // nothing while claiming a tolerance is set. - const customTolerance = computed(() => { - const n = Number(String(customValue.value).trim()) - if (!Number.isFinite(n) || n <= 0) return null - return customUnit.value === 'abs' ? { abs: n } : { pct: n } - }) - const tolerance = computed(() => - toleranceId.value === 'custom' - ? customTolerance.value - : (TOLERANCES.find((t) => t.value === toleranceId.value)?.tolerance ?? null) + + const { toleranceId, customValue, customUnit, tolerance } = useToleranceChoice() + const { bySheet, activeKeyColumns, toggleKeyColumn, clearKeyColumns } = useKeyColumns( + () => activeSheet.value?.name, + [() => store.gridSheets.left, () => store.gridSheets.right] ) const sheets = computed(() => - diffWorkbooks(store.gridSheets.left, store.gridSheets.right, { tolerance: tolerance.value }) + diffWorkbooks(store.gridSheets.left, store.gridSheets.right, { + tolerance: tolerance.value, + keyColumns: bySheet.value + }) ) // Clamp to the last sheet if the active index falls out of range. @@ -91,6 +68,9 @@ export function useSpreadsheetDiff() { toleranceId, customValue, customUnit, - tolerance + tolerance, + activeKeyColumns, + toggleKeyColumn, + clearKeyColumns } } diff --git a/src/renderer/src/composables/useToleranceChoice.js b/src/renderer/src/composables/useToleranceChoice.js new file mode 100644 index 0000000..e1e0cac --- /dev/null +++ b/src/renderer/src/composables/useToleranceChoice.js @@ -0,0 +1,40 @@ +import { computed, ref } from 'vue' + +// An absolute floor kills float-rounding noise; a percentage is what +// materiality means. Neither covers the other, so both are offered. +export const TOLERANCES = [ + { value: 'exact', label: 'Exact', tolerance: null }, + { value: 'abs', label: '±0.01', tolerance: { abs: 0.01 } }, + { value: 'half', label: '±0.5%', tolerance: { pct: 0.5 } }, + { value: 'one', label: '±1%', tolerance: { pct: 1 } }, + // The threshold the engagement set, which is never one of four numbers. + { value: 'custom', label: 'Custom', tolerance: null } +] + +// Explicit, never sniffed from the text: "50" meaning half a percent or fifty +// pounds is not something to guess at. +export const TOLERANCE_UNITS = [ + { value: 'pct', label: '%' }, + { value: 'abs', label: 'abs' } +] + +/** How close two numbers have to be to count as the same figure. */ +export function useToleranceChoice() { + const toleranceId = ref('exact') + const customValue = ref('') + const customUnit = ref('pct') + // Empty, unparseable or non-positive reads as Exact: a zero threshold forgives + // nothing while claiming a tolerance is set. + const customTolerance = computed(() => { + const n = Number(String(customValue.value).trim()) + if (!Number.isFinite(n) || n <= 0) return null + return customUnit.value === 'abs' ? { abs: n } : { pct: n } + }) + const tolerance = computed(() => + toleranceId.value === 'custom' + ? customTolerance.value + : (TOLERANCES.find((t) => t.value === toleranceId.value)?.tolerance ?? null) + ) + + return { toleranceId, customValue, customUnit, tolerance } +} diff --git a/src/renderer/src/stores/diffStore.js b/src/renderer/src/stores/diffStore.js index 778370c..57d8ad2 100644 --- a/src/renderer/src/stores/diffStore.js +++ b/src/renderer/src/stores/diffStore.js @@ -4,7 +4,7 @@ import { structureAdapter } from '../adapters/structureAdapter' import { csvAdapter } from '../adapters/csvAdapter' import { diffStructures, structuredKind } from '../utils/structuralDiff' import { delimitedKind } from '../utils/csv' -import { shouldOpenSemantic } from '../utils/viewChrome' +import { restoredSemanticView, shouldOpenSemantic } from '../utils/viewChrome' import { useVaultStore } from './vaultStore' import { useSnippetStore } from './snippetStore' import { isSecret } from '../utils/secretSnippet' @@ -669,7 +669,7 @@ export const useDiffStore = defineStore('diff', { this.pasteRightName = payload.pasteRightName ?? '' this.renderSideBySide = payload.renderSideBySide ?? true this.ignoreTrimWhitespace = payload.ignoreTrimWhitespace ?? false - this.semanticView = payload.semanticView === true + this.semanticView = restoredSemanticView(payload, this) this.mode = payload.mode ?? 'files' // Opened from a saved diff: it already exists in the vault, so replacing // it later needs no "you'll lose it" prompt. diff --git a/src/renderer/src/stores/tabsStore.js b/src/renderer/src/stores/tabsStore.js index ec8e38b..5fda8a4 100644 --- a/src/renderer/src/stores/tabsStore.js +++ b/src/renderer/src/stores/tabsStore.js @@ -114,15 +114,15 @@ export const useTabsStore = defineStore('tabs', { this._capture() this._show(next) }, - // A snapshot that says nothing about the view keeps the one in use: only a - // saved diff, which recorded its own, should change how the panes are set. + // A snapshot that says nothing about the PANES keeps the ones in use. NOT + // semanticView: it belongs to the comparison, not to the last tab, so an + // unrecorded one is left for restore() to take from the files. _withCurrentView(snapshot) { const diff = useDiffStore() return { ...snapshot, renderSideBySide: snapshot.renderSideBySide ?? diff.renderSideBySide, - ignoreTrimWhitespace: snapshot.ignoreTrimWhitespace ?? diff.ignoreTrimWhitespace, - semanticView: snapshot.semanticView === true + ignoreTrimWhitespace: snapshot.ignoreTrimWhitespace ?? diff.ignoreTrimWhitespace } }, _fill(tab, snapshot, { diffSaved, entryId, name }) { diff --git a/src/renderer/src/styles/ui.css b/src/renderer/src/styles/ui.css index 385aad2..342f726 100644 --- a/src/renderer/src/styles/ui.css +++ b/src/renderer/src/styles/ui.css @@ -262,6 +262,24 @@ color: var(--danger-text); filter: brightness(1.08); } +/* How many of the things behind this button are set — the View options a reader + has changed, the columns keying the grid's rows. Height comes from --chip-h, + never from padding: that is how the Esc chip drifted to 19px beside a 26px + button. Face is the theme's own ink at 30%, the same step .btn.active takes — + --accent as a fill under a label scores under 4.5:1 on five themes. */ +.btn-count { + min-width: var(--chip-h); + height: var(--chip-h); + padding: 0 var(--space-1); + border-radius: var(--radius-pill); + background: var(--btn-face-press); + color: var(--text); + font-size: var(--font-xs); + font-weight: 700; + display: inline-flex; + align-items: center; + justify-content: center; +} /* --- Popover layer ------------------------------------------------------ A panel anchored to a button, dismissed by Escape or a press outside — the View options and the toolbar's overflow menu. Shared because there were diff --git a/src/renderer/src/types.js b/src/renderer/src/types.js index 2933e49..ee4a593 100644 --- a/src/renderer/src/types.js +++ b/src/renderer/src/types.js @@ -36,6 +36,15 @@ * @property {boolean} [truncated] the row cap was hit before the end */ +/** + * One column of the grid's display order. `left`/`right` index into that side's + * own rows; null means the column is absent there and renders as a ghost. + * @typedef {object} AlignedColumn + * @property {number|null} left + * @property {number|null} right + * @property {string} name the header label, or '' where the pairing was positional + */ + /** * What one cell carries beyond its value. Absent fields mean "nothing to say". * @typedef {object} CellMeta diff --git a/src/renderer/src/utils/alignColumns.js b/src/renderer/src/utils/alignColumns.js index a7f4dd0..2997d54 100644 --- a/src/renderer/src/utils/alignColumns.js +++ b/src/renderer/src/utils/alignColumns.js @@ -4,6 +4,9 @@ // the columns that survived; everything downstream works on those pairs. Pure. const MAX_COLUMNS = 4096 +// Bounded: past a handful of rows, a "header" is just the first row that happens +// to look like one. +const MAX_HEADER_SCAN = 10 // Widest row, not the header: a sheet may carry data past its own header. function widthOf(rows) { @@ -12,8 +15,8 @@ function widthOf(rows) { return Math.min(width, MAX_COLUMNS) } -function headerLabels(rows, width) { - const header = rows[0] ?? [] +function headerLabels(rows, width, at = 0) { + const header = rows[at] ?? [] const out = [] for (let i = 0; i < width; i++) { const v = header[i] @@ -22,10 +25,10 @@ function headerLabels(rows, width) { return out } -// Blank or repeated labels would pair columns by coincidence, and a lone row is -// data — a header describes the rows beneath it, and there are none. -function usable(labels, rowCount) { - if (!labels.length || rowCount < 2) return false +// Blank or repeated labels would pair columns by coincidence, and a header +// describes the rows beneath it, so it needs at least one. +function usable(labels, rowsBeneath) { + if (!labels.length || rowsBeneath < 1) return false const seen = new Set() for (const l of labels) { if (!l || seen.has(l)) return false @@ -34,6 +37,15 @@ function usable(labels, rowCount) { return true } +/** The first row that reads as a header, or null within `MAX_HEADER_SCAN`. */ +function headerRowIndex(rows, width) { + const limit = Math.min(rows.length, MAX_HEADER_SCAN) + for (let at = 0; at < limit; at++) { + if (usable(headerLabels(rows, width, at), rows.length - at - 1)) return at + } + return null +} + function lcsOps(a, b) { const n = a.length const m = b.length @@ -74,23 +86,37 @@ function positional(leftLabels, rightLabels) { } /** - * Pair the two sheets' columns into one display order. - * @returns {Array<{left: number|null, right: number|null, name: string}>} - * `left`/`right` index into that side's own rows; null means the column is - * absent there and renders as a ghost. + * Pair the two sheets' columns, and say which row the labels came from. + * @returns {{columns: Array<{left: number|null, right: number|null, name: string}>, + * headerRows: {left: number, right: number}|null}} `headerRows` is null when + * the pairing fell back to position — naming a row it did not use would claim + * the columns were matched by name. */ -export function alignColumns(leftRows = [], rightRows = []) { - const leftLabels = headerLabels(leftRows, widthOf(leftRows)) - const rightLabels = headerLabels(rightRows, widthOf(rightRows)) - if (!usable(leftLabels, leftRows.length) || !usable(rightLabels, rightRows.length)) { - return positional(leftLabels, rightLabels) - } +export function headerPairing(leftRows = [], rightRows = []) { + const leftAt = headerRowIndex(leftRows, widthOf(leftRows)) + const rightAt = headerRowIndex(rightRows, widthOf(rightRows)) + const leftLabels = headerLabels(leftRows, widthOf(leftRows), leftAt ?? 0) + const rightLabels = headerLabels(rightRows, widthOf(rightRows), rightAt ?? 0) + const byPosition = { columns: positional(leftLabels, rightLabels), headerRows: null } + if (leftAt === null || rightAt === null) return byPosition const byHeader = lcsOps(leftLabels, rightLabels) // Pairing a minority of the columns means the row was data, not headers — // `row-0 | v0` against `row-0 | w0` dropped column B out of the diff entirely. const paired = pairedColumns(byHeader).length const narrower = Math.min(leftLabels.length, rightLabels.length) - return paired * 2 > narrower ? byHeader : positional(leftLabels, rightLabels) + return paired * 2 > narrower + ? { columns: byHeader, headerRows: { left: leftAt, right: rightAt } } + : byPosition +} + +/** + * Pair the two sheets' columns into one display order. + * @returns {Array<{left: number|null, right: number|null, name: string}>} + * `left`/`right` index into that side's own rows; null means the column is + * absent there and renders as a ghost. + */ +export function alignColumns(leftRows = [], rightRows = []) { + return headerPairing(leftRows, rightRows).columns } /** The aligned columns present on BOTH sides — the ones a diff can compare. */ diff --git a/src/renderer/src/utils/matchRowsByKey.js b/src/renderer/src/utils/matchRowsByKey.js new file mode 100644 index 0000000..afb3d22 --- /dev/null +++ b/src/renderer/src/utils/matchRowsByKey.js @@ -0,0 +1,113 @@ +// Row pairing by the value a reader recognises the row BY, rather than by the +// whole row's contents. alignRows matches signatures in order, so the same +// export sorted differently reads as changed end to end; keying pairs across the +// whole sheet instead. Pure. +import { changedCells } from './alignRows' + +// A cell's text cannot contain it, so ['a','b'] and ['a b',''] stay two keys. +const SEPARATOR = '\u0000' + +function cellKey(value) { + return value === null || value === undefined ? '' : String(value).trim() +} + +/** + * One pairing key per row, built from that side's own column indices. + * Trimmed — a trailing space in an export is not a different account — but + * never case-folded, because `ACC-1` and `acc-1` are two accounts. + */ +export function compositeKeys(rows = [], columns = []) { + return rows.map((row) => columns.map((c) => cellKey(row?.[c])).join(SEPARATOR)) +} + +function queuesByKey(keys) { + const map = new Map() + for (let i = 0; i < keys.length; i++) { + const queue = map.get(keys[i]) + if (queue) queue.push(i) + else map.set(keys[i], [i]) + } + return map +} + +// A repeated key cannot say WHICH row is which, so the nth occurrence takes the +// nth — counted rather than hidden, because the reader has to know that. +function duplicateCount(leftQueues, rightQueues) { + const keys = new Set() + for (const [key, queue] of leftQueues) if (queue.length > 1) keys.add(key) + for (const [key, queue] of rightQueues) if (queue.length > 1) keys.add(key) + return keys.size +} + +function pairing(leftQueues, rightKeys, leftCount) { + const taken = new Map() + const leftFor = new Array(rightKeys.length).fill(null) + const matchedLeft = new Array(leftCount).fill(false) + for (let j = 0; j < rightKeys.length; j++) { + const queue = leftQueues.get(rightKeys[j]) + const at = taken.get(rightKeys[j]) ?? 0 + if (!queue || at >= queue.length) continue + taken.set(rightKeys[j], at + 1) + leftFor[j] = queue[at] + matchedLeft[queue[at]] = true + } + return { leftFor, matchedLeft } +} + +function pairEntry(leftRows, rightRows, at, tolerance) { + const changed = changedCells(leftRows[at.i], rightRows[at.j], tolerance) + return { + status: changed.length ? 'changed' : 'same', + left: leftRows[at.i], + right: rightRows[at.j], + leftIndex: at.i, + rightIndex: at.j, + changed + } +} + +const oneSided = (status, row, index) => ({ + status, + left: status === 'removed' ? row : null, + right: status === 'removed' ? null : row, + leftIndex: status === 'removed' ? index : null, + rightIndex: status === 'removed' ? null : index, + changed: [] +}) + +/** + * Pair two sheets' rows by key. Display order follows the RIGHT file — the one + * the reader has open — with a removed row emitted just before the first + * surviving left row that followed it, so it keeps its context. + * @returns {{rows: Array<{status:'same'|'changed'|'added'|'removed', left, right, + * leftIndex:number|null, rightIndex:number|null, changed:number[]}>, + * duplicateKeys:number}} + */ +export function matchRowsByKey(leftRows = [], rightRows = [], opts = {}) { + const leftKeys = opts.leftKeys ?? [] + const rightKeys = opts.rightKeys ?? [] + const tolerance = opts.tolerance ?? null + const leftQueues = queuesByKey(leftKeys) + const { leftFor, matchedLeft } = pairing(leftQueues, rightKeys, leftRows.length) + + const rows = [] + let cursor = 0 + const flushTo = (limit) => { + while (cursor < limit) { + if (!matchedLeft[cursor]) rows.push(oneSided('removed', leftRows[cursor], cursor)) + cursor++ + } + } + for (let j = 0; j < rightRows.length; j++) { + const i = leftFor[j] + if (i === null) { + rows.push(oneSided('added', rightRows[j], j)) + continue + } + flushTo(i) + rows.push(pairEntry(leftRows, rightRows, { i, j }, tolerance)) + } + flushTo(leftRows.length) + + return { rows, duplicateKeys: duplicateCount(leftQueues, queuesByKey(rightKeys)) } +} diff --git a/src/renderer/src/utils/session.js b/src/renderer/src/utils/session.js index acd45ec..0064797 100644 --- a/src/renderer/src/utils/session.js +++ b/src/renderer/src/utils/session.js @@ -60,7 +60,7 @@ export function readSnapshot(raw) { pasteRightName: str(raw.pasteRightName), renderSideBySide: raw.renderSideBySide !== false, ignoreTrimWhitespace: raw.ignoreTrimWhitespace === true, - semanticView: raw.semanticView === true + semanticView: typeof raw.semanticView === 'boolean' ? raw.semanticView : undefined } } diff --git a/src/renderer/src/utils/spreadsheetDiff.js b/src/renderer/src/utils/spreadsheetDiff.js index 240b5d9..b7ebb7c 100644 --- a/src/renderer/src/utils/spreadsheetDiff.js +++ b/src/renderer/src/utils/spreadsheetDiff.js @@ -1,7 +1,9 @@ -// Workbook diff: pair sheets by name, pair their columns (alignColumns), align -// each pair's rows (alignRows), roll up per-sheet stats. Pure. +// Workbook diff: pair sheets by name, pair their columns (headerPairing), align +// each pair's rows — by key where the reader named one, otherwise by whole-row +// signature (alignRows) — and roll up per-sheet stats. Pure. import { alignRows, rowKeys, valuesEqual } from './alignRows' -import { alignColumns, pairedColumns } from './alignColumns' +import { headerPairing, pairedColumns } from './alignColumns' +import { compositeKeys, matchRowsByKey } from './matchRowsByKey' import { comparableCell, comparableRows, metaAt, metaIndex } from './sheetCells' // 0 -> "A", 25 -> "Z", 26 -> "AA" (bijective base-26), for the grid's column @@ -73,30 +75,76 @@ function project(rows, indices) { return rows.map((row) => indices.map((i) => row[i])) } -function alignedRows(left, right, columns, opts) { - const pairs = pairedColumns(columns) - const keyColumn = opts.keyColumn ?? 0 - return alignRows( - project( +// Keys are chosen per sheet, because the columns are. A Map rather than an +// object: the sheet name comes from the file, and `__proto__` must not resolve +// to anything. +const keyColumnsFor = (keyColumns, name) => + keyColumns instanceof Map ? (keyColumns.get(name) ?? []) : [] + +// The columns a reader chose as the row's identity, as each side's own indices. +// A column only one side has cannot key both, so it is dropped rather than +// keying one side off a column the other does not have. +function keyColumnsOf(columns, chosen) { + const picked = (chosen ?? []) + .map((i) => columns[i]) + .filter((c) => c && c.left !== null && c.right !== null) + return { left: picked.map((c) => c.left), right: picked.map((c) => c.right) } +} + +function projected(left, right, pairs) { + return { + left: project( comparableRows(left), pairs.map((c) => c.left) ), - project( + right: project( comparableRows(right), pairs.map((c) => c.right) - ), - { - ...opts, - leftKeys: rowKeys(left.rows ?? [], pairs[keyColumn]?.left ?? 0), - rightKeys: rowKeys(right.rows ?? [], pairs[keyColumn]?.right ?? 0) - } - ) + ) + } +} + +function keyedRows(rows, keys, ctx) { + return matchRowsByKey(rows.left, rows.right, { + leftKeys: compositeKeys(ctx.leftRows, keys.left), + rightKeys: compositeKeys(ctx.rightRows, keys.right), + tolerance: ctx.tolerance + }) +} + +function signatureRows(rows, pairs, ctx) { + const keyColumn = ctx.opts.keyColumn ?? 0 + return { + rows: alignRows(rows.left, rows.right, { + ...ctx.opts, + leftKeys: rowKeys(ctx.leftRows, pairs[keyColumn]?.left ?? 0), + rightKeys: rowKeys(ctx.rightRows, pairs[keyColumn]?.right ?? 0) + }), + duplicateKeys: 0 + } +} + +function alignedRows(left, right, columns, opts) { + const pairs = pairedColumns(columns) + const rows = projected(left, right, pairs) + const ctx = { + leftRows: left.rows ?? [], + rightRows: right.rows ?? [], + tolerance: opts.tolerance ?? null, + opts + } + const keys = keyColumnsOf(columns, opts.keyColumns) + return keys.left.length ? keyedRows(rows, keys, ctx) : signatureRows(rows, pairs, ctx) } function bothSides(name, left, right, opts) { - const columns = alignColumns(left.rows ?? [], right.rows ?? []) + const { columns, headerRows } = headerPairing(left.rows ?? [], right.rows ?? []) const tolerance = opts.tolerance ?? null - const rows = alignedRows(left, right, columns, { ...opts, tolerance }) + const { rows, duplicateKeys } = alignedRows(left, right, columns, { + ...opts, + tolerance, + keyColumns: keyColumnsFor(opts.keyColumns, name) + }) // Where each paired column sits in the DISPLAY order, so a changed cell is // reported at the index the grid actually renders. const pairAt = columns.reduce((acc, c, i) => { @@ -122,6 +170,8 @@ function bothSides(name, left, right, opts) { columnsAdded: columns.filter((c) => c.left === null).length, columnsRemoved: columns.filter((c) => c.right === null).length, changes: total(stats), + headerRows, + duplicateKeys, ...state } } @@ -138,7 +188,7 @@ function oneSide(name, sheet, side) { formulaChanged: [] })) const own = sheet.rows ?? [] - const columns = side === 'left' ? alignColumns(own, []) : alignColumns([], own) + const { columns } = side === 'left' ? headerPairing(own, []) : headerPairing([], own) const stats = statsOf(rows) const state = side === 'left' ? sheetState(sheet, null) : sheetState(null, sheet) return { @@ -150,6 +200,8 @@ function oneSide(name, sheet, side) { columnsAdded: 0, columnsRemoved: 0, changes: total(stats), + headerRows: null, + duplicateKeys: 0, ...state } } @@ -163,6 +215,7 @@ function total(stats) { * stats:{changed:number,added:number,removed:number}, * columns:Array<{left:number|null,right:number|null,name:string}>, * columnsAdded:number, columnsRemoved:number, changes:number, + * headerRows:{left:number,right:number}|null, duplicateKeys:number, * hidden:boolean, hasFormulas:boolean, leftMeta:Map, rightMeta:Map, * leftHidden:Set, rightHidden:Set}>} */ diff --git a/src/renderer/src/utils/viewChrome.js b/src/renderer/src/utils/viewChrome.js index f16c736..01cbab1 100644 --- a/src/renderer/src/utils/viewChrome.js +++ b/src/renderer/src/utils/viewChrome.js @@ -50,3 +50,11 @@ export const showsWhitespaceToggle = (store) => * @returns {boolean} */ export const shouldOpenSemantic = (store) => !!store?.canCompareDiagram || !!store?.delimitedFormat + +/** + * The view a restored snapshot opens in: the one it recorded, or — where it + * recorded none, having been saved before the diagram view or by a bundle that + * never wrote it — whatever the files themselves ask for. + */ +export const restoredSemanticView = (payload, store) => + typeof payload?.semanticView === 'boolean' ? payload.semanticView : shouldOpenSemantic(store) diff --git a/src/shared/i18n/en-XA.json b/src/shared/i18n/en-XA.json index 38d3908..9331d4a 100644 --- a/src/shared/i18n/en-XA.json +++ b/src/shared/i18n/en-XA.json @@ -973,6 +973,14 @@ "noSnippetsMatchTryRemoving": "[Ńō şńĩƥƥéţş ɱàţçĥ — ţřŷ řéɱōṽĩńğ à ƒĩłţéř. ·øé·øé·øé·øé·ø]", "newSnippetCta": "[Ńéŵ şńĩƥƥéţ ·øé·]" }, + "rowMatchMenu": { + "matchRows": "[Ṁàţçĥ řōŵş ·øé·]", + "tip": "[Çĥōōşé ţĥé çōłūɱńş ţĥàţ ńàɱé à řōŵ ·øé·øé·øé·øé]", + "byWholeRow": "[Řōŵş ƥàĩř ƀŷ ţĥéĩř ŵĥōłé çōńţéńţş. Řé-şōřţĩńğ ţĥé ƒĩłé đéƒéàţş ţĥàţ. ·øé·øé·øé·øé·øé·øé·øé·]", + "byKey": "[Řōŵş ƥàĩř ƀŷ ţĥé ţĩçķéđ çōłūɱńş, ŵĥéřéṽéř ţĥéŷ şĩţ ĩń ţĥé ƒĩłé. ·øé·øé·øé·øé·øé·øé·ø]", + "duplicates": "[{n} ķéŷ àƥƥéàřş ɱōřé ţĥàń ōńçé — ţĥōşé řōŵş ƥàĩř ĩń ţĥé ōřđéř ţĥéŷ àƥƥéàř | {n} ķéŷş àƥƥéàř ɱōřé ţĥàń ōńçé — ţĥōşé řōŵş ƥàĩř ĩń ţĥé ōřđéř ţĥéŷ àƥƥéàř ·øé·øé·øé·øé·øé·øé·øé·øé·øé·øé·øé·øé·øé·øé·øé]", + "useAuto": "[Ɓàçķ ţō ŵĥōłé-řōŵ ɱàţçĥĩńğ ·øé·øé·øé]" + }, "spreadsheetDiffViewer": { "customTolerance": "[Çūşţōɱ ţōłéřàńçé ·øé·øé]", "numbersCloserThanThisCount": "[Ńūɱƀéřş çłōşéř ţĥàń ţĥĩş çōūńţ àş ţĥé şàɱé ƒĩğūřé ·øé·øé·øé·øé·øé·ø]", @@ -988,7 +996,9 @@ "columns": "[Çōłūɱńş ·øé]", "backToTheValues": "[Ɓàçķ ţō ţĥé ṽàłūéş éàçĥ ƒōřɱūłà ƥřōđūçéđ ·øé·øé·øé·øé·ø]", "showFormulasNotResults": "[Šĥōŵ ƒōřɱūłàş, ńōţ řéşūłţş ·øé·øé·øé]", - "sheetOnlyIn": "[“{name}” ĩş ōńłŷ ĩń ţĥé {side} ƒĩłé ·øé·øé·øé·]" + "sheetOnlyIn": "[“{name}” ĩş ōńłŷ ĩń ţĥé {side} ƒĩłé ·øé·øé·øé·]", + "headerRow": "[Ĥéàđéř řōŵ {row} ·øé·ø]", + "headerRowEachSide": "[Ĥéàđéř řōŵ {left} · {right} ·øé·øé·ø]" }, "storageSettings": { "dataFolder": "[Đàţà ƒōłđéř ·øé·]", diff --git a/src/shared/i18n/en.json b/src/shared/i18n/en.json index 3202b6e..2f28f6a 100644 --- a/src/shared/i18n/en.json +++ b/src/shared/i18n/en.json @@ -973,6 +973,14 @@ "noSnippetsMatchTryRemoving": "No snippets match — try removing a filter.", "newSnippetCta": "New snippet" }, + "rowMatchMenu": { + "matchRows": "Match rows", + "tip": "Choose the columns that name a row", + "byWholeRow": "Rows pair by their whole contents. Re-sorting the file defeats that.", + "byKey": "Rows pair by the ticked columns, wherever they sit in the file.", + "duplicates": "{n} key appears more than once — those rows pair in the order they appear | {n} keys appear more than once — those rows pair in the order they appear", + "useAuto": "Back to whole-row matching" + }, "spreadsheetDiffViewer": { "customTolerance": "Custom tolerance", "numbersCloserThanThisCount": "Numbers closer than this count as the same figure", @@ -988,7 +996,9 @@ "columns": "Columns", "backToTheValues": "Back to the values each formula produced", "showFormulasNotResults": "Show formulas, not results", - "sheetOnlyIn": "“{name}” is only in the {side} file" + "sheetOnlyIn": "“{name}” is only in the {side} file", + "headerRow": "Header row {row}", + "headerRowEachSide": "Header row {left} · {right}" }, "storageSettings": { "dataFolder": "Data folder", diff --git a/tests/renderer/composables/useKeyColumns.test.js b/tests/renderer/composables/useKeyColumns.test.js new file mode 100644 index 0000000..0982c2c --- /dev/null +++ b/tests/renderer/composables/useKeyColumns.test.js @@ -0,0 +1,69 @@ +import { describe, expect, it } from 'vitest' +import { nextTick, ref } from 'vue' +import { useKeyColumns } from '../../../src/renderer/src/composables/useKeyColumns' + +describe('useKeyColumns', () => { + it('holds a choice per sheet name', () => { + const name = ref('Ledger') + const keys = useKeyColumns(() => name.value) + keys.toggleKeyColumn(0) + keys.toggleKeyColumn(2) + expect(keys.activeKeyColumns.value).toEqual([0, 2]) + name.value = 'Regions' + expect(keys.activeKeyColumns.value).toEqual([]) + name.value = 'Ledger' + expect(keys.activeKeyColumns.value).toEqual([0, 2]) + }) + + it('keeps the columns ascending however they were picked', () => { + const keys = useKeyColumns(() => 'S') + keys.toggleKeyColumn(3) + keys.toggleKeyColumn(1) + expect(keys.activeKeyColumns.value).toEqual([1, 3]) + }) + + it('drops the sheet entirely once its last column is toggled off', () => { + const keys = useKeyColumns(() => 'S') + keys.toggleKeyColumn(1) + keys.toggleKeyColumn(1) + expect(keys.activeKeyColumns.value).toEqual([]) + expect(keys.bySheet.value.has('S')).toBe(false) + }) + + it('clears only the active sheet', () => { + const name = ref('A') + const keys = useKeyColumns(() => name.value) + keys.toggleKeyColumn(0) + name.value = 'B' + keys.toggleKeyColumn(1) + keys.clearKeyColumns() + expect(keys.activeKeyColumns.value).toEqual([]) + name.value = 'A' + expect(keys.activeKeyColumns.value).toEqual([0]) + }) + + // The picker is mounted only for a real sheet, but a toggle arriving between + // one comparison and the next must not mint a key under `undefined`. + it('ignores a toggle with no sheet to attach it to', () => { + const keys = useKeyColumns(() => undefined) + keys.toggleKeyColumn(0) + keys.clearKeyColumns() + expect(keys.bySheet.value.size).toBe(0) + }) + + it('starts a new pair of files on Auto', async () => { + const files = ref('first') + const keys = useKeyColumns(() => 'Ledger', [() => files.value]) + keys.toggleKeyColumn(0) + files.value = 'second' + await nextTick() + expect(keys.activeKeyColumns.value).toEqual([]) + }) + + it('does not watch anything when given no reset sources', async () => { + const keys = useKeyColumns(() => 'Ledger') + keys.toggleKeyColumn(0) + await nextTick() + expect(keys.activeKeyColumns.value).toEqual([0]) + }) +}) diff --git a/tests/renderer/composables/useSpreadsheetDiff.test.js b/tests/renderer/composables/useSpreadsheetDiff.test.js index 28309bc..26576bc 100644 --- a/tests/renderer/composables/useSpreadsheetDiff.test.js +++ b/tests/renderer/composables/useSpreadsheetDiff.test.js @@ -99,3 +99,89 @@ describe('a tolerance of your own', () => { expect(tolerance.value).toBeNull() }) }) + +describe('useSpreadsheetDiff — key columns', () => { + const ledger = (rows) => [{ name: 'Ledger', rows }] + const header = ['Account', 'Centre', 'Amount'] + const left = ledger([header, ['1001', 'EMEA', 500], ['1002', 'APAC', 300]]) + const right = ledger([header, ['1002', 'APAC', 300], ['1001', 'EMEA', 500]]) + + const loaded = () => { + const store = useDiffStore() + store.left = book(left) + store.right = book(right) + return useSpreadsheetDiff() + } + + it('reads a re-sorted sheet as changed until a key column is named', () => { + const diff = loaded() + expect(diff.identical.value).toBe(false) + diff.toggleKeyColumn(0) + expect(diff.activeKeyColumns.value).toEqual([0]) + expect(diff.identical.value).toBe(true) + }) + + it('toggles a column back off and returns to Auto', () => { + const diff = loaded() + diff.toggleKeyColumn(0) + diff.toggleKeyColumn(0) + expect(diff.activeKeyColumns.value).toEqual([]) + expect(diff.identical.value).toBe(false) + }) + + it('keeps the columns in ascending order however they were picked', () => { + const diff = loaded() + diff.toggleKeyColumn(1) + diff.toggleKeyColumn(0) + expect(diff.activeKeyColumns.value).toEqual([0, 1]) + }) + + it('holds a choice per sheet', () => { + const store = useDiffStore() + const rows = [header, ['1001', 'EMEA', 500]] + store.left = book([ + { name: 'Ledger', rows }, + { name: 'Other', rows } + ]) + store.right = book([ + { name: 'Ledger', rows }, + { name: 'Other', rows } + ]) + const diff = useSpreadsheetDiff() + diff.toggleKeyColumn(0) + diff.select(1) + expect(diff.activeKeyColumns.value).toEqual([]) + diff.select(0) + expect(diff.activeKeyColumns.value).toEqual([0]) + }) + + it('clears the active sheet’s keys', () => { + const diff = loaded() + diff.toggleKeyColumn(0) + diff.clearKeyColumns() + expect(diff.activeKeyColumns.value).toEqual([]) + }) + + // Two workbooks routinely share a sheet name, so a key held by name would + // otherwise carry silently into the next comparison. + it('starts a new pair of files on Auto', async () => { + const store = useDiffStore() + const diff = loaded() + diff.toggleKeyColumn(0) + store.left = book(ledger([header, ['9001', 'EMEA', 1]])) + await Promise.resolve() + expect(diff.activeKeyColumns.value).toEqual([]) + }) + + it('reports the duplicate keys the choice leaves ambiguous', () => { + const store = useDiffStore() + const rows = [header, ['1001', 'EMEA', 500], ['1001', 'APAC', 300]] + store.left = book([{ name: 'Ledger', rows }]) + store.right = book([{ name: 'Ledger', rows }]) + const diff = useSpreadsheetDiff() + diff.toggleKeyColumn(0) + expect(diff.activeSheet.value.duplicateKeys).toBe(1) + diff.toggleKeyColumn(1) + expect(diff.activeSheet.value.duplicateKeys).toBe(0) + }) +}) diff --git a/tests/renderer/stores/diffStore.diagram.test.js b/tests/renderer/stores/diffStore.diagram.test.js index d6b2185..6420998 100644 --- a/tests/renderer/stores/diffStore.diagram.test.js +++ b/tests/renderer/stores/diffStore.diagram.test.js @@ -65,3 +65,47 @@ describe('diagram comparison from pasted text', () => { expect(diff.comparableKind).toBe('diagram') }) }) + +// A saved or shared diff carries the view it was saved in. One that carries +// NOTHING — written before the diagram view existed, or by a bundle that never +// recorded it — used to come back as text, which is the one place a Mermaid +// pair did not open as a picture. +describe('restoring a diagram comparison', () => { + const mmd = (body) => `flowchart TD\n${body}\n` + const payload = (extra) => ({ + left: { path: null, name: 'a.mmd', content: mmd(' A --> B') }, + right: { path: null, name: 'b.mmd', content: mmd(' A --> C') }, + mode: 'files', + ...extra + }) + + it('opens the picture when the snapshot says nothing about the view', () => { + const diff = useDiffStore() + diff.restore(payload()) + expect(diff.semanticView).toBe(true) + expect(diff.comparableKind).toBe('diagram') + }) + + it('honours a snapshot that explicitly recorded text', () => { + const diff = useDiffStore() + diff.restore(payload({ semanticView: false })) + expect(diff.semanticView).toBe(false) + expect(diff.comparableKind).toBe('text') + }) + + it('keeps the picture when the snapshot recorded it', () => { + const diff = useDiffStore() + diff.restore(payload({ semanticView: true })) + expect(diff.semanticView).toBe(true) + }) + + it('leaves a plain-text pair alone', () => { + const diff = useDiffStore() + diff.restore({ + left: { path: null, name: 'a.txt', content: 'one' }, + right: { path: null, name: 'b.txt', content: 'two' }, + mode: 'files' + }) + expect(diff.semanticView).toBe(false) + }) +}) diff --git a/tests/renderer/stores/tabsStore.test.js b/tests/renderer/stores/tabsStore.test.js index ef77ebd..cc061b2 100644 --- a/tests/renderer/stores/tabsStore.test.js +++ b/tests/renderer/stores/tabsStore.test.js @@ -785,6 +785,25 @@ describe('the structure toggle belongs to its comparison', () => { expect(diff.semanticView).toBe(true) }) + // ...and a comparison that IS a picture opens as one, in a new tab as much as + // in the first: the toggle is taken from the files, never from the tab before. + it('opens a Mermaid pair as a diagram in a fresh tab', () => { + const diff = useDiffStore() + const tabs = useTabsStore() + const mmd = (body) => ({ + mode: 'files', + left: { path: '/tmp/a.mmd', name: 'a.mmd', content: `flowchart TD\n${body}` }, + right: { path: '/tmp/b.mmd', name: 'b.mmd', content: 'flowchart TD\n A --> C' } + }) + tabs.init() + tabs.open(json('{"a":1}', '{"a":2}')) + expect(diff.semanticView).toBe(false) + + tabs.open(mmd(' A --> B')) + expect(diff.semanticView).toBe(true) + expect(diff.comparableKind).toBe('diagram') + }) + it('comes back with a restored session, like the other view toggles', async () => { window.api = { vaultEncrypt: async (text, aad) => vaultEncrypt(SESSION_KEY, text, aad), diff --git a/tests/renderer/utils/alignColumns.test.js b/tests/renderer/utils/alignColumns.test.js index 0bee5e0..1ab69ce 100644 --- a/tests/renderer/utils/alignColumns.test.js +++ b/tests/renderer/utils/alignColumns.test.js @@ -1,5 +1,9 @@ import { describe, expect, it } from 'vitest' -import { alignColumns, pairedColumns } from '../../../src/renderer/src/utils/alignColumns' +import { + alignColumns, + headerPairing, + pairedColumns +} from '../../../src/renderer/src/utils/alignColumns' const pairs = (cols) => cols.map((c) => [c.left, c.right]) @@ -143,6 +147,124 @@ describe('alignColumns — positional fallback', () => { }) }) +// A title row above the header is the shape every exported pack has, and it used +// to drop the whole sheet to positional pairing — the exact failure header +// pairing exists to prevent. +describe('alignColumns — header below a title row', () => { + const left = [ + ['Trial balance as at 31 Dec 2024'], + ['Account', 'Debit', 'Credit'], + ['1001', 500, 0], + ['1002', 0, 300] + ] + + it('finds the header under a title row and keeps an insert from shifting', () => { + const right = [ + ['Trial balance as at 31 Dec 2025'], + ['Account', 'Debit', 'Note', 'Credit'], + ['1001', 500, '', 0], + ['1002', 0, '', 300] + ] + expect(pairs(alignColumns(left, right))).toEqual([ + [0, 0], + [1, 1], + [null, 2], + [2, 3] + ]) + }) + + // Each of these inserts a column, so header pairing and positional pairing + // give different answers and the assertion can tell them apart. + it('pairs when only one side carries the title row', () => { + const right = [ + ['Account', 'Debit', 'Note', 'Credit'], + ['1001', 500, '', 0], + ['1002', 0, '', 300] + ] + expect(pairs(alignColumns(left, right))).toEqual([ + [0, 0], + [1, 1], + [null, 2], + [2, 3] + ]) + }) + + it('skips blank rows on the way down', () => { + const l = [['Q3 pack'], [], ['Region', 'Q1', 'Q2'], ['North', 1, 2], ['South', 3, 4]] + const r = [ + ['Region', 'New', 'Q1', 'Q2'], + ['North', 9, 1, 2], + ['South', 9, 3, 4] + ] + expect(pairs(alignColumns(l, r))).toEqual([ + [0, 0], + [null, 1], + [1, 2], + [2, 3] + ]) + }) + + it('gives up rather than scanning an unbounded preamble', () => { + const preamble = Array.from({ length: 12 }, (_, i) => [`note ${i}`]) + const l = [...preamble, ['Region', 'Q1', 'Q2'], ['North', 1, 2]] + const r = [...preamble, ['Region', 'New', 'Q1', 'Q2'], ['North', 9, 1, 2]] + expect(pairs(alignColumns(l, r))).toEqual([ + [0, 0], + [1, 1], + [2, 2], + [null, 3] + ]) + }) + + it('names the columns from the header it found, not from the title row', () => { + const right = [ + ['Trial balance as at 31 Dec 2025'], + ['Account', 'Debit', 'Credit'], + ['1001', 500, 0], + ['1002', 0, 300] + ] + expect(alignColumns(left, right).map((c) => c.name)).toEqual(['Account', 'Debit', 'Credit']) + }) +}) + +describe('headerPairing', () => { + it('reports the row each side was read from', () => { + const l = [['Pack'], ['Region', 'Q1'], ['North', 1], ['South', 2]] + const r = [ + ['Region', 'Q1'], + ['North', 1], + ['South', 2] + ] + expect(headerPairing(l, r).headerRows).toEqual({ left: 1, right: 0 }) + }) + + // Naming a header row the pairing did not use would tell the reader the + // columns were matched by name when they were matched by position. + it('reports no header row when the pairing fell back to position', () => { + const l = [ + ['row-0', 'v0'], + ['row-1', 'v1'] + ] + const r = [ + ['row-0', 'w0'], + ['row-1', 'w1'] + ] + expect(headerPairing(l, r).headerRows).toBeNull() + }) + + it('returns the same columns alignColumns does', () => { + const l = [ + ['Region', 'Q1'], + ['North', 1] + ] + const r = [ + ['Region', 'Q2'], + ['North', 2] + ] + expect(headerPairing(l, r).columns).toEqual(alignColumns(l, r)) + }) +}) + describe('pairedColumns', () => { it('keeps only the columns a diff can compare', () => { const cols = [ diff --git a/tests/renderer/utils/matchRowsByKey.test.js b/tests/renderer/utils/matchRowsByKey.test.js new file mode 100644 index 0000000..d1610bc --- /dev/null +++ b/tests/renderer/utils/matchRowsByKey.test.js @@ -0,0 +1,165 @@ +import { describe, expect, it } from 'vitest' +import { compositeKeys, matchRowsByKey } from '../../../src/renderer/src/utils/matchRowsByKey' +import { alignRows } from '../../../src/renderer/src/utils/alignRows' + +const statuses = (result) => result.rows.map((e) => e.status) +const keysOf = (result) => result.rows.map((e) => (e.left ?? e.right)[0]) + +const byColumn = (left, right, columns = [0], opts = {}) => + matchRowsByKey(left, right, { + leftKeys: compositeKeys(left, columns), + rightKeys: compositeKeys(right, columns), + ...opts + }) + +describe('matchRowsByKey', () => { + // The reopened-track defect: the same export sorted differently reads as + // changed end to end, because the LCS matches whole-row signatures in order. + const ledger = [ + ['1001', 500], + ['1002', 300], + ['1003', 120] + ] + const resorted = [ + ['1003', 120], + ['1002', 300], + ['1001', 500] + ] + + it('pairs re-sorted rows that the row-signature LCS cannot', () => { + expect(statuses({ rows: alignRows(ledger, resorted) })).not.toEqual(['same', 'same', 'same']) + expect(statuses(byColumn(ledger, resorted))).toEqual(['same', 'same', 'same']) + }) + + it('reports the cell that moved, not the row that moved', () => { + const right = [ + ['1003', 120], + ['1002', 999], + ['1001', 500] + ] + const { rows } = byColumn(ledger, right) + expect(statuses({ rows })).toEqual(['same', 'changed', 'same']) + expect(rows[1].changed).toEqual([1]) + expect(rows[1].leftIndex).toBe(1) + expect(rows[1].rightIndex).toBe(1) + }) + + it('keys on more than one column', () => { + const left = [ + ['1001', 'EMEA', 500], + ['1001', 'APAC', 300] + ] + const right = [ + ['1001', 'APAC', 300], + ['1001', 'EMEA', 500] + ] + expect(statuses(byColumn(left, right, [0]))).not.toEqual(['same', 'same']) + expect(statuses(byColumn(left, right, [0, 1]))).toEqual(['same', 'same']) + }) + + it('marks a row only one side has', () => { + const right = [ + ['1003', 120], + ['1004', 80] + ] + const result = byColumn(ledger, right) + expect(statuses(result)).toEqual(['removed', 'removed', 'same', 'added']) + expect(keysOf(result)).toEqual(['1001', '1002', '1003', '1004']) + }) + + // A removed row belongs beside the rows it sat next to, not in a heap at the + // end: it is emitted just before the first surviving left row that followed it. + it('follows the right file’s order and keeps a removed row in place', () => { + const left = [ + ['A', 1], + ['B', 2], + ['C', 3], + ['D', 4] + ] + const right = [ + ['D', 4], + ['B', 2], + ['A', 1] + ] + const result = byColumn(left, right) + expect(keysOf(result)).toEqual(['C', 'D', 'B', 'A']) + expect(statuses(result)).toEqual(['removed', 'same', 'same', 'same']) + }) + + it('pairs duplicate keys in order of occurrence and counts them', () => { + const left = [ + ['X', 1], + ['X', 2], + ['Y', 3] + ] + const right = [ + ['X', 1], + ['Y', 3] + ] + const result = byColumn(left, right) + expect(result.duplicateKeys).toBe(1) + expect(statuses(result)).toEqual(['same', 'removed', 'same']) + expect(result.rows[1].leftIndex).toBe(1) + }) + + it('counts a key duplicated on the right too', () => { + const left = [['X', 1]] + const right = [ + ['X', 1], + ['X', 2] + ] + const result = byColumn(left, right) + expect(result.duplicateKeys).toBe(1) + expect(statuses(result)).toEqual(['same', 'added']) + }) + + // A blank key is a key. Letting it match anything would pair a spacer row with + // whatever happened to be unmatched. + it('treats a blank key as a key, not a wildcard', () => { + const left = [ + ['', 1], + ['', 2], + ['A', 3] + ] + const right = [ + ['', 1], + ['A', 3] + ] + const result = byColumn(left, right) + expect(statuses(result)).toEqual(['same', 'removed', 'same']) + }) + + it('applies the tolerance to a matched pair', () => { + const left = [['1001', 500]] + const right = [['1001', 500.004]] + expect(statuses(byColumn(left, right))).toEqual(['changed']) + expect(statuses(byColumn(left, right, [0], { tolerance: { abs: 0.01 } }))).toEqual(['same']) + }) + + it('handles an empty side', () => { + expect(statuses(byColumn([], ledger))).toEqual(['added', 'added', 'added']) + expect(statuses(byColumn(ledger, []))).toEqual(['removed', 'removed', 'removed']) + }) +}) + +describe('compositeKeys', () => { + it('trims, so a trailing space in an export is not a different account', () => { + expect(compositeKeys([['1001 ']], [0])).toEqual(compositeKeys([['1001']], [0])) + }) + + it('does not fold case — two accounts are two accounts', () => { + expect(compositeKeys([['ACC-1']], [0])).not.toEqual(compositeKeys([['acc-1']], [0])) + }) + + // Joining on a separator a cell can contain would let ['a','b'] and ['a b'] + // collide into one key. + it('cannot collide two different key tuples into one', () => { + const [ab] = compositeKeys([['a', 'b']], [0, 1]) + const [joined] = compositeKeys([['a b', '']], [0, 1]) + expect(ab).not.toBe(joined) + }) + + it('reads a missing cell as blank', () => { + expect(compositeKeys([[null], [undefined], []], [0])).toEqual(['', '', '']) + }) +}) diff --git a/tests/renderer/utils/session.test.js b/tests/renderer/utils/session.test.js index a2b23b4..96266f7 100644 --- a/tests/renderer/utils/session.test.js +++ b/tests/renderer/utils/session.test.js @@ -318,3 +318,28 @@ describe('packSession — a half-loaded tab is reset, not blanked', () => { expect(packed.tabs[0].customTitle).toBe('prod vs staging') }) }) + +// A stored session says what the reader chose; a session that predates the +// choice must stay SILENT about it, so the store can take the view from the +// files rather than being told "text" by a field that was never written. +describe('readSession — the view a snapshot never recorded', () => { + const withView = (view) => { + const raw = JSON.parse(JSON.stringify(packSession([tab('tab-1')], 'tab-1'))) + if (view === undefined) delete raw.tabs[0].snapshot.semanticView + else raw.tabs[0].snapshot.semanticView = view + return readSession(JSON.stringify(raw)).tabs[0].snapshot.semanticView + } + + it('leaves an unrecorded view undefined, not false', () => { + expect(withView(undefined)).toBeUndefined() + }) + + it('keeps a recorded view, either way', () => { + expect(withView(true)).toBe(true) + expect(withView(false)).toBe(false) + }) + + it('still refuses a value that is not a boolean', () => { + expect(withView('yes')).toBeUndefined() + }) +}) diff --git a/tests/renderer/utils/spreadsheetDiff.test.js b/tests/renderer/utils/spreadsheetDiff.test.js index cdac8a7..50fdfa3 100644 --- a/tests/renderer/utils/spreadsheetDiff.test.js +++ b/tests/renderer/utils/spreadsheetDiff.test.js @@ -208,3 +208,133 @@ describe('diffWorkbooks — sheet state', () => { expect(s.rightHidden.has(1)).toBe(false) }) }) + +describe('diffWorkbooks — row identity by key columns', () => { + // Keys are named per sheet, so the option is keyed by sheet name. + const keys = (columns) => new Map([['Ledger', columns]]) + + // The same ledger, re-sorted, with one amount moved and one line dropped. + const left = [ + sheet('Ledger', [ + ['Account', 'Cost centre', 'Amount'], + ['1001', 'EMEA', 500], + ['1002', 'EMEA', 300], + ['1002', 'APAC', 120], + ['1003', 'APAC', 80] + ]) + ] + const right = [ + sheet('Ledger', [ + ['Account', 'Cost centre', 'Amount'], + ['1002', 'APAC', 120], + ['1003', 'APAC', 999], + ['1001', 'EMEA', 500] + ]) + ] + + it('reads a re-sorted export as changed when no key is named', () => { + const [s] = diffWorkbooks(left, right) + expect(s.stats.changed + s.stats.added + s.stats.removed).toBeGreaterThan(1) + }) + + it('pairs by the named key columns instead', () => { + const [s] = diffWorkbooks(left, right, { keyColumns: keys([0, 1]) }) + expect(s.stats).toEqual({ changed: 1, added: 0, removed: 1 }) + }) + + // The grid renders the display order, so a changed cell reported in paired + // space would point at the wrong column the moment a column is one-sided. + it('reports the changed cell in DISPLAY column space', () => { + const withGap = [ + sheet('Ledger', [ + ['Account', 'Note', 'Amount'], + ['1001', 'x', 500] + ]) + ] + const noGap = [ + sheet('Ledger', [ + ['Account', 'Amount'], + ['1001', 900] + ]) + ] + const [s] = diffWorkbooks(withGap, noGap, { keyColumns: keys([0]) }) + expect(s.columns.map((c) => c.name)).toEqual(['Account', 'Note', 'Amount']) + expect(s.rows[1].changed).toEqual([2]) + }) + + it('ignores a key column only one side has', () => { + const [s] = diffWorkbooks(left, right, { keyColumns: keys([0, 1, 9]) }) + expect(s.stats).toEqual({ changed: 1, added: 0, removed: 1 }) + }) + + it('counts duplicate keys rather than hiding the ambiguity', () => { + const [s] = diffWorkbooks(left, right, { keyColumns: keys([0]) }) + expect(s.duplicateKeys).toBe(1) + const [exact] = diffWorkbooks(left, right, { keyColumns: keys([0, 1]) }) + expect(exact.duplicateKeys).toBe(0) + }) + + it('leaves the tolerance working through the key path', () => { + const near = [ + sheet('Ledger', [ + ['Account', 'Amount'], + ['1001', 500.004] + ]) + ] + const base = [ + sheet('Ledger', [ + ['Account', 'Amount'], + ['1001', 500] + ]) + ] + expect(diffWorkbooks(base, near, { keyColumns: keys([0]) })[0].stats.changed).toBe(1) + const forgiving = diffWorkbooks(base, near, { + keyColumns: keys([0]), + tolerance: { abs: 0.01 } + }) + expect(forgiving[0].stats.changed).toBe(0) + }) +}) + +describe('diffWorkbooks — header row', () => { + const withTitle = (note) => + sheet('S', [[note], ['Region', 'Q1', 'Q2'], ['North', 1, 2], ['South', 3, 4]]) + + it('reports the row each side’s headers came from', () => { + const [s] = diffWorkbooks( + [withTitle('Pack')], + [ + sheet('S', [ + ['Region', 'Q1', 'Q2'], + ['North', 1, 2], + ['South', 3, 4] + ]) + ] + ) + expect(s.headerRows).toEqual({ left: 1, right: 0 }) + }) + + it('reports none when the columns fell back to positional pairing', () => { + const [s] = diffWorkbooks( + [ + sheet('S', [ + ['row-0', 'v0'], + ['row-1', 'v1'] + ]) + ], + [ + sheet('S', [ + ['row-0', 'w0'], + ['row-1', 'w1'] + ]) + ] + ) + expect(s.headerRows).toBeNull() + }) + + it('leaves a one-sided sheet without a header claim', () => { + const [s] = diffWorkbooks([sheet('Gone', [['a'], ['b']])], []) + expect(s.headerRows).toBeNull() + expect(s.duplicateKeys).toBe(0) + }) +}) diff --git a/tests/renderer/utils/viewChrome.test.js b/tests/renderer/utils/viewChrome.test.js index 4b714aa..5f8947c 100644 --- a/tests/renderer/utils/viewChrome.test.js +++ b/tests/renderer/utils/viewChrome.test.js @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest' import { + restoredSemanticView, shouldOpenSemantic, hasStatusBand, showsSplitView, @@ -124,3 +125,23 @@ describe('shouldOpenSemantic', () => { expect(shouldOpenSemantic(undefined)).toBe(false) }) }) + +describe('restoredSemanticView', () => { + const diagram = { canCompareDiagram: true } + + it('keeps the view the snapshot recorded, either way', () => { + expect(restoredSemanticView({ semanticView: false }, diagram)).toBe(false) + expect(restoredSemanticView({ semanticView: true }, {})).toBe(true) + }) + + it('takes it from the files when the snapshot recorded none', () => { + expect(restoredSemanticView({}, diagram)).toBe(true) + expect(restoredSemanticView({}, { delimitedFormat: 'csv' })).toBe(true) + expect(restoredSemanticView({}, { structuredFormat: 'json' })).toBe(false) + }) + + it('treats a non-boolean as no record at all', () => { + expect(restoredSemanticView({ semanticView: 'yes' }, diagram)).toBe(true) + expect(restoredSemanticView(null, diagram)).toBe(true) + }) +})