CI failed with cargo clippy --workspace --all-targets -- -D warnings,
not a test. The actual cause: I'd been validating locally against
rustc/clippy 1.95.0 (the toolchain this machine had installed before
now), while CI installs plain "stable" (1.99.0 as of this run) — two
lints new since 1.95.0 fired there and nowhere in my local checks:
- clippy::int_plus_one on `src.end.row + 1 <= cap` in grid.rs,
simplified to `src.end.row < cap` (same meaning, no overflow risk
change: row is a u32 and cap is already saturating-clamped below
MAX_ROWS - 1, so src.end.row + 1 can't wrap).
- clippy::disallowed_methods on the new fill_handle_perf example's
three Instant::now() calls — the project's own lint from the time-
panic fix (#125 history): reading the clock traps on wasm. This
example is native-only (a `cargo run --release` benchmark, never
built for wasm), so it gets the same #![allow(...)] perf.rs already
carries, with the same justification.
Updated the local toolchain to stable (rustup update stable) and
re-ran cargo clippy --workspace --all-targets -- -D warnings clean
before this push, to actually match what CI checks instead of
guessing again.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
cargo fmt reformatted the new example's while-loop condition onto its
own line; clippy's nonminimal_bool caught a negated is_some_and that
reads more plainly as is_none_or.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Follow-up to #210, which the reviewer correctly rejected: the stated
cause (a sparse column with a stray distant value) doesn't hold, since
the scan already stops at the first empty cell. This is a different,
narrower fix that doesn't change behaviour for any input — it never
truncates a long fill, unlike #210's cap — only how fast a genuinely
long, densely-populated run is scanned.
grid.rs's fill-handle double-click called Sheet::value_ref once per
row to find where the neighbouring column's data ends. Each call does
a band lookup + row lookup + column lookup (three BTreeMap probes) via
CellStore::get. For a real dense run (e.g. a filled column near the
1,048,576-row sheet limit) that's a lot of point lookups on the UI
thread in one frame: ~59ms for a 1,000,000-row run in a release build
(measured with the new fill_handle_perf example), likely worse under
wasm where GridCraft actually runs in the browser.
- crates/model/src/store.rs: CellStore::contiguous_value_run walks
each band's row BTreeMap with `range` (one in-order pass per band)
instead of one `get` per row, so a dense run costs roughly one
tree-traversal step per row instead of one binary search per row.
Deliberately narrower than `has`/`get`: a stored cell always decides
the row on its own (even with an empty value), matching what
`value_ref`'s callers actually test for, not `has`'s "formula counts
regardless of its cached value" (used for Ctrl+arrow navigation).
- crates/model/src/sheet.rs: Sheet::last_contiguous_nonempty layers
`spill` back in wherever contiguous_value_run finds no stored cell,
exactly where plain `value_ref` would fall through to it.
- crates/ui-egui/src/grid.rs: the fill-handle double-click now calls
this once instead of looping over value_ref.
Result: 999,999-row dense run, 59.2ms -> 24.7ms (2.40x), same answer
(asserted in the example). Four new tests in sheet.rs, including a
300-trial property test against a naive value_ref-loop reference
(spills, formula-with-empty-value, gaps, blanks) — all pass, plus the
full existing suite (`cargo test -p gridcraft-model -p gridcraft-ui-egui
-p gridcraft-engine`; the only failures are #210-era gridcraft-engine
CSV tests already failing on main, unrelated to this change).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Every edit goes through cmd::commit, which read std::time::Instant::now()
up front for GRIDCRAFT_PROFILE timing, whether profiling was on or not.
wasm32-unknown-unknown has no clock: Instant::now panics there, and with
panic=abort the browser reports `RuntimeError: unreachable`. So in the
web app, typing 1 into A1 and pressing Enter trapped (#118). The native
apps and the CLI have a clock and were unaffected.
commit now reads the clock only when GRIDCRAFT_PROFILE is set, and
never on wasm. The other clock reads (NOW(), the current year for typed
dates, the UI's frame timer) already skip wasm.
Native tests can't see this trap, so clippy.toml now disallows
Instant::now and SystemTime::now. A clock read has to be gated off wasm
and allowed at the call, and `cargo clippy -D warnings` fails on the old
commit code. The existing, already-gated reads and the native-only perf
example allow it.
Checked by building the engine for wasm32-unknown-unknown into a small
cdylib that runs cell.set A1=1 on a new workbook, and running it in
Node: before the fix it traps with `RuntimeError: unreachable`, after
it returns normally.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
