diff --git a/CLAUDE.md b/CLAUDE.md index c74f63e..c6ccf00 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -277,11 +277,24 @@ levels a two-field `group_by` does not have. `depth` is null until the first layout has been read back; the stepper shows an em dash rather than guessing a level. -Both are **1-based**: they count the levels to show, where the imperative -`view.set_depth()` counts the boundary below them. `server.cpp` does -`ctx1->set_depth(row_pivot_depth - 1)` and -`ctx2->set_depth(HEADER_COLUMN, column_pivot_depth - 1)`, so the toolbar sends -`d + 1` and subtracts one again when reading a layout back. +Both are **1-based counts of the levels to show**, and so is the stepper's own +`depth` — `DepthButtons` labels `d` as `levels[d - 1]`. Same unit on both +sides, so the value crosses unchanged in each direction. The engine agrees: +`server.cpp` turns `group_by_depth` into `set_depth(n - 1)` and +`t_traversal::set_depth` adds the 1 back, so `n` renders `n` levels, on both +axes (`ctx2->set_depth(HEADER_COLUMN, column_pivot_depth - 1)` is the same +arithmetic). + +The toolbar used to send `d + 1` and subtract the same 1 when reading a layout +back. That was self-consistent — the stepper always agreed with itself — and +off by one against the grid at every position: picking `partgroup` showed one +level past it. The `+1` was there to dodge `depth 0`, which reaches the engine +as `set_depth(-1)` on an unsigned `t_depth`, underflows, and clamps to *fully +expanded*. `applyDepth` now floors the value at 1 instead, so the stepper's +**"Total" slot still shows the first level rather than a bare total** — the +long-standing wart, kept deliberately rather than traded for the underflow. A +real total needs `group_rollup_mode: 'total'`, which is a field in +`ViewConfigUpdate` but is unverified against the local WASM engine. Because a depth lives in the config, it survives every view rebuild, rides into the persisted and named layouts through `viewer.save()`, and needs nothing diff --git a/ui/src/views/Forecast.jsx b/ui/src/views/Forecast.jsx index 9147eab..ac98fa1 100644 --- a/ui/src/views/Forecast.jsx +++ b/ui/src/views/Forecast.jsx @@ -929,12 +929,15 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio await viewer.restore(cfg) // split_full is the legacy key, from when collapsing truncated split_by adoptSplit(cfg.split_full?.length ? cfg.split_full : cfg.split_by, - cfg.split_by_depth != null ? cfg.split_by_depth - 1 : null) + cfg.split_by_depth != null ? cfg.split_by_depth : null) setGroupFull(cfg.group_by || []) // restore() has already applied group_by_depth; this only syncs the - // toolbar. expand_depth is the legacy key, from when depth was imperative + // toolbar. The config's count and the stepper's are the same unit, so + // it reads across as it is -- it used to subtract the 1 that applyDepth + // added, which kept the toolbar agreeing with itself and with nothing + // else. expand_depth is the legacy key, from when depth was imperative // and had to be stored beside the config rather than in it. - if (cfg.group_by_depth != null) setExpandDepth(cfg.group_by_depth - 1) + if (cfg.group_by_depth != null) setExpandDepth(cfg.group_by_depth) else if (cfg.expand_depth != null) await applyDepth(cfg.expand_depth) } else { // The default is a published layout flagged is_default. A version-scoped @@ -970,6 +973,7 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio // throw here would abort the rest of initViewer silently. try { await ensureRowLabelWidth() + await fitRowLabelsNow() } catch (err) { console.error('[pf-layout] threw', err) } @@ -982,7 +986,7 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio // collapse apart from the user rearranging the pivot. const live = await viewer.save() adoptSplit(live.split_by || [], - live.split_by_depth != null ? live.split_by_depth - 1 : null) + live.split_by_depth != null ? live.split_by_depth : null) setGroupFull(live.group_by || []) // the plugin element is replaced when the plugin changes, so re-assert hideSplitTotal() @@ -1097,9 +1101,9 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio if (!levels) return const depth = Math.max(0, Math.min(n, levels)) try { - // 1-based, as with group_by_depth: server.cpp does - // ctx2->set_depth(HEADER_COLUMN, column_pivot_depth - 1) - await viewer.restore({ ...cfg, split_by_depth: depth + 1 }) + // A count of levels to show, exactly as group_by_depth is -- the engine + // treats both axes identically. See applyDepth for why 0 is floored to 1. + await viewer.restore({ ...cfg, split_by_depth: Math.max(1, depth) }) setSplitDepth(depth) } catch (err) { console.error('[applySplitDepth]', err) @@ -1348,6 +1352,20 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio await grid.draw({ invalid_columns: true }) } + // fitRowLabels takes the grid; this finds it, so the restore paths can call it + // the same way they call ensureRowLabelWidth. + // + // Deliberately not routed through the Fit widths button: that one goes via + // regular-table's own auto-size, which measures the leaf header's rendered box + // over the rows currently in the viewport. fitRowLabels measures the label + // text on a canvas instead, so it is not blinded by a clipped cell and does + // not depend on what happens to be scrolled into view. + async function fitRowLabelsNow() { + const grid = (await viewerRef.current?.getPlugin())?.regular_table + if (!grid?.saveColumnSizes) return + await fitRowLabels(grid) + } + const GRID_CSS = ` /* Let headers and row labels size their own column. * @@ -1426,18 +1444,25 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio async function applyDepth(d) { const viewer = viewerRef.current if (!viewer) return - // group_by_depth counts levels, where view.set_depth() counts the boundary - // below them -- server.cpp does ctx->set_depth(row_pivot_depth - 1) for both - // the one- and two-sided contexts. So the toolbar's 0..3, which were written - // against the imperative call, are one less than the config wants: depth 0 - // (grand total only) is group_by_depth 1, and passing 0 asked the engine for - // set_depth(-1). + // group_by_depth is a count of levels to show, and so is the stepper's own + // d -- DepthButtons labels d as levels[d - 1]. Same unit, so d goes across + // unchanged. It used to be sent as d + 1, which showed one level more than + // was picked at every position: server.cpp turns group_by_depth into + // set_depth(n - 1), and t_traversal::set_depth adds the 1 back, so n levels + // is what n asks for. + // + // d === 0 is the stepper's "Total" slot, which has never worked: 0 reaches + // the engine as set_depth(-1) on an unsigned depth, underflows, and clamps + // to fully expanded. Sending 1 keeps it showing the first level, which is + // what it has always shown -- still wrong, but wrong in the small way + // rather than the large one. A bare total needs group_rollup_mode. // // The whole config goes back rather than a partial update, which had no // effect on its own. table is dropped: it is loaded by reference, and a stale // name in a restored config fails the lookup. + const target = Math.max(1, d) const { table: _t, ...cfg } = await viewer.save() - await viewer.restore({ ...cfg, group_by_depth: d + 1 }) + await viewer.restore({ ...cfg, group_by_depth: target }) // Until the engine carries the patch in ui/vendor, restore() drops the depth: // ViewConfig::apply_update applies ten fields and neither depth is among @@ -1452,9 +1477,11 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio // a button, which is predictable, and it becomes durable for free once the // engine is rebuilt. const after = await viewer.save() - if (after.group_by_depth !== d + 1) { + if (after.group_by_depth !== target) { const view = await viewer.getView() - await view.set_depth(d) + // set_depth counts the boundary below the levels shown, so it is one + // less than the config's count of them. + await view.set_depth(target - 1) const plugin = await viewer.getPlugin() await plugin.draw(view) } @@ -1545,12 +1572,13 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio cfg.plugin_config = { ...(cfg.plugin_config || {}), edit_mode: 'SELECT_REGION' } await viewer.restore(cfg) adoptSplit(cfg.split_full?.length ? cfg.split_full : cfg.split_by, - cfg.split_by_depth != null ? cfg.split_by_depth - 1 : null) + cfg.split_by_depth != null ? cfg.split_by_depth : null) setGroupFull(cfg.group_by || []) - if (cfg.group_by_depth != null) setExpandDepth(cfg.group_by_depth - 1) + if (cfg.group_by_depth != null) setExpandDepth(cfg.group_by_depth) else if (cfg.expand_depth != null) await applyDepth(cfg.expand_depth) await ensureRowLabelWidth() + await fitRowLabelsNow() // The persisted copy is taken after the restore, so it carries whatever // cleanLayout dropped on the way in rather than the stale original -- and