fix: the viewport reserves the real frame, and a fallback width counts cells

Copilot's two "previously missed" findings, both correct.

**`extent()` reserved one header row and one header column.** That was right
until this PR: the column header is now one line per level plus an optional
group line, and the row header is `row_levels` columns wide. Announcing a
viewport the page cannot show matters because `#grid` has `overflow: auto` —
the DOM would take a scroll position of its own while the client's cursor
offsets were computed from the larger window, putting the selected cell off
screen. The cost is measured off the painted model and re-announced when it
changes.

Inline group *rows* are deliberately **not** counted, although they occupy
space: they depend on how many rows the window asked for, so counting them
would be a feedback loop — a smaller window crosses fewer groups and asks
for a larger one. The three facts that are counted depend only on the axes.
The residue is a little over-fetch, the direction `ROW_PX` already errs in.

**`fit_to_labels` measured its fallback width in Unicode scalars.** It is
documented as the width a column is drawn at while `ColWidths` catches up,
and the authoritative widths are in display cells, so `日本` came out two
cells wide where it needs four. Now measured with `UnicodeWidthStr`, like
`header_widths` and like the TUI.
`a_column_without_its_width_yet_is_measured_in_display_cells` pins it, and
restoring `chars().count()` fails it with `[2]` against `[4]`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Edward Langley
2026-09-19 23:22:30 -07:00
parent 16ba617b27
commit 9aef855d5f
2 changed files with 62 additions and 4 deletions
+25 -1
View File
@@ -584,7 +584,16 @@ impl GridCache {
// column would make a re-fit quadratic in the column count.
let items: Vec<&Vec<String>> = self.col_frame.items().collect();
for levels in &items[self.col_widths.len()..cols] {
let width = levels.iter().map(|s| s.chars().count()).max().unwrap_or(0);
// Display cells, the same unit `header_widths` and the
// authoritative `ColWidths` use. `chars().count()` would draw
// `日本` two cells wide where it needs four, so a cache
// caught between `ColFrame` and `ColWidths` would render the
// new column too narrow.
let width = levels
.iter()
.map(|s| UnicodeWidthStr::width(s.as_str()))
.max()
.unwrap_or(0);
self.col_widths.push(width.min(u16::MAX as usize) as u16);
}
}
@@ -1269,6 +1278,21 @@ mod tests {
assert_eq!(wide.header_widths(), vec![MAX_ROW_HEADER_W]);
}
/// The fallback width a column is drawn at between its `ColFrame` and
/// its `ColWidths` is in **display cells**, the same unit the
/// authoritative widths are in. `chars().count()` would draw a wide
/// label half as wide as it needs (raised by Copilot on PR #23).
#[test]
fn a_column_without_its_width_yet_is_measured_in_display_cells() {
let mut grid = GridCache::default();
grid.apply(&GridUpdate::ColFrame(AxisFrame::flat(["\u{65e5}\u{672c}"])));
assert_eq!(
grid.col_widths,
vec![4],
"two scalars, four display cells — a scalar count would say 2"
);
}
/// An axis with no categories still draws one header line, and its item
/// tuples are empty — reading a level off one must answer `""` rather
/// than panic. This is the shape a view with nothing on Row produces.
+37 -3
View File
@@ -105,12 +105,26 @@ function setShellStatus(text) {
paintStatus();
}
/** The extent this window can show, in whole cells. */
// What the grid's own frame costs, in whole cells, as of the last repaint.
// One header row and one header column was right until improvise-anr9: the
// column header is now one row PER LEVEL plus an optional group row, the row
// header is `row_levels` columns wide, and inline group headers take a row
// each. Reserving one of each announces a viewport the page cannot show, and
// `#grid` has `overflow: auto`, so the DOM would acquire a scroll position of
// its own while the client's cursor offsets were computed from the larger
// window — putting the selected cell off screen.
//
// It is measured off the painted model rather than predicted, and defaults to
// the old one-and-one before the first paint, which is the only moment there
// is nothing to measure.
let frameCost = { rows: 1, cols: 1 };
/** The extent this window can show, in whole DATA cells. */
function extent() {
const box = el.grid.getBoundingClientRect();
return {
rows: Math.max(1, Math.floor(box.height / ROW_PX) - 1),
cols: Math.max(1, Math.floor(box.width / COL_PX) - 1),
rows: Math.max(1, Math.floor(box.height / ROW_PX) - frameCost.rows),
cols: Math.max(1, Math.floor(box.width / COL_PX) - frameCost.cols),
};
}
@@ -242,6 +256,26 @@ function repaint() {
el.viewName.textContent = model.view_name;
el.grid.replaceChildren(gridTable(model));
el.categories.replaceChildren(categoryList(model));
// What the frame costs, so the NEXT viewport announcement is honest: the
// header lines, plus a group line if there is one, plus the row header's
// width in columns.
//
// Inline group ROWS are deliberately not counted, even though they take
// space. They depend on how many rows the window asked for, so counting
// them would make this a feedback loop — a smaller window fetches fewer
// rows, which crosses fewer groups, which asks for a larger window. The
// three facts below depend only on the axes, so a repaint cannot chase
// its own tail. The cost is a little over-fetch, which is the direction
// ROW_PX already errs in.
const next = {
rows: model.col_levels.length + (model.col_groups.length > 0 ? 1 : 0),
cols: model.row_levels,
};
if (next.rows !== frameCost.rows || next.cols !== frameCost.cols) {
frameCost = next;
sendViewport();
}
}
/** The grid, painted from a model in which every frame decision is already