Targets the uncovered branches the coverage report named, around the
formula dependency graph and incremental recalculation:
- Formula replacement/removal edge cases: set_cell("") removes the
cell and its edges, formula->value and formula->formula rewiring
drop stale dependency edges.
- No-op removal paths: set_cell("") on a missing cell records nothing.
- Dependency-graph cleanup after formula deletion: remove_cell on a
formula cell, and update_dependency_graph's defensive path when a
dependent has no dependents entry.
- Affected-cell recalculation error paths: non-formula cells inside a
cycle-affected set keep their raw value; the ="" empty-result
regression; the recursive eval slow path (cached AST, parse
fallback, and CycleDetected); parse_cell_computed_value arms; parse
errors propagating through a dependent and through a large range.
Also covers named-range unary expansion (=-Total), the >64-cell range
fast path, non-numeric number-format fallbacks, and the demo_q3 /
demo_roi constructors.
Engine line coverage: data.rs 91.17% -> 97.90%; engine total
90.09% -> 93.24%. Unit tests 260 -> 279 (19 new); 9 integration tests
unchanged.
`recalculate_incremental` treated "empty computed_value on a changed
formula cell" as proof that the topological sort dropped the cell, and
debug_asserted on it. But a formula may legitimately evaluate to the
empty string (`=""`, `=IF(FALSE, "x", "")`), leaving computed_value
empty through no fault of the dep-graph walk. In a debug build that
edit panicked the engine.
The invariant now checks what it actually meant to check: whether the
topological sort *visited* every changed formula cell, using the
`visited` set built from `sorted`. Emptiness is no longer used as the
error signal, so legitimate empty-string results flow through (the
display falls back to the raw formula text, as already documented for
empty computed values).
Nine tests against `workbook_api.rs`, the weakest file in the engine at
77.80%. Now 88.80%; engine total 88.97% -> 90.09%, tests 259 -> 260.
One of them pins a trap rather than a bug. `set_cell` calls
`begin_recording()` before `apply()`; the other eight public mutators
(`remove_cell`, `put_cell`, `set_col_width`, `set_row_height`,
`toggle_bold`, `set_number_format`, `set_alignment`, `set_bg_color`) do
not. Calling one of those directly and then `undo()` reverts the
*previous* recorded edit, not the one just made.
Nothing ships broken: every `spreadsheet-ui` call site takes its own
`data.snapshot()` first, checked one by one in `grid.rs`. But the
asymmetry is invisible at the call site and the next caller will not know
to snapshot. `only_set_cell_records_its_own_undo_step` states the current
contract so a change to it is a deliberate decision rather than an
accident.
The coverage script measured the engine only, and it cherry-picked four
source files to report on, which flattered the number: 91.15% against a
hand-picked subset versus 88.97% for the whole of `src/`.
Rewritten to cover both crates honestly, with per-crate floors and a
listing of uncovered lines. Two bugs in the script itself:
- The ignore regex contained the work-directory name, so it excluded the
very sources being measured and reported a confident 0%. The work dir
also cannot live inside the repo, or Cargo treats the copied crates as
workspace members and refuses to build them.
- `llvm-cov show` filename headers carry no trailing colon, so the awk
matcher never fired and the uncovered-line listing was always empty.
`spreadsheet-ui/src/{grid,ui,workspace}.rs` and `src/bin/` are excluded:
the first three are `script_mod!` generated DSL and the last is desktop
startup, neither of which a unit test can reach.
UI controllers now measure 94.55%: `event_router.rs` 70.59% -> 97.96%,
`selection.rs` 80.65% -> 100%. UI tests 9 -> 17.
The repo has a CI gate requiring full-length revs, added deliberately in
5e71457 with a comment explaining that an abbreviated rev resolves only
while no other object shares its prefix -- a property of the repository's
current object count, not a guarantee. Git's abbreviation length grows as
a repo grows, so a short pin silently becomes ambiguous, and an attacker
able to push to the fork can try to manufacture a colliding prefix.
That gate has been failing. 42 declarations across 34 crates used
abbreviated revs:
41x rev = "ecf5a572" (the current makepad pin)
1x rev = "5efe6e24c" (map/tests/makepad_test_app, left behind
by the ce0eaae bump)
Resolved both against the remote and rewrote them:
ecf5a572 -> ecf5a572ab62a1c1598909971f602f99083671cc
5efe6e24c -> 5efe6e24c9f732e9f11b783757f196f4f1c402b2
Verified this changes the LABEL and not the dependency: Cargo.lock holds
exactly one makepad commit id and zero references to the old one, so
nothing was silently upgraded. The stray makepad_test_app pin did move to
the current rev, which is the intent -- it pointed at a stale branch head.
Cargo.lock also picks up unrelated churn (brotli et al in,
makepad-android-state/jni-sys out). That staleness is PRE-EXISTING, not
caused by this change: confirmed by stashing every edit and running
`cargo metadata` on a pristine tree, which produces the identical diff.
Gate now passes:
$ grep -rn 'rev = ' --include=Cargo.toml . | grep -vE 'rev = "[0-9a-f]{40}"'
(no output)
86c9595 synced the fork to upstream/dev at abd70f4, which dropped three
fork-local optional dependencies from widgets/Cargo.toml and their
re-exports from lib.rs. They were fork additions, so the merge lost them.
Every Makepad UI target then failed to resolve:
package `nigig-pdf-makepad` depends on `makepad-widgets` with feature
`test` but `makepad-widgets` does not have that feature.
help: available features: default, serde
failed to select a version for `makepad-widgets`
The "available features" list is misleading: with no `test` feature on
widgets 2.0.0, cargo falls back to the stale old/widgets copy, which is
1.0.0 and offers only default and serde. Same fallback that produced the
bogus makepad-fonts-chinese-bold error in an earlier sync.
libs/makepad_test was never removed - only the manifest entries and the
re-export. The fork's ecf5a572 restores both. This bumps all 34 crates.
Verified against the real fork, not a local copy:
TEST_TARGET=pdf-ui 682 passing (was: failed to resolve)
TEST_TARGET=pdf 637 passing
Pin bump only: every hunk changes the rev and nothing else.
Updated makepad fork to include all latest APIs needed by map widget:
- pack_vector_vertices and VECTOR_PACKED_FLOATS_PER_VERTEX
- TileArchiveReader for MKMap archive support
- get_tile_decoded method on MbtilesReader
- set_trust_fill_winding and fill_fringe_into on Tessellator
- retain_queued method on TagThreadPool
- set_camera_delta method on DrawRotatedText
This resolves all compilation errors in the map widget code.
All 35 Cargo.toml pins move from d82756a to 5efe6e24c on the gitdab fork
(portallist base + makepad_test Android adb / standalone terminal wiring).
Lockfile regenerated; pdf crates compile against the new rev.
