Merge pull request #241 from fiddlerwoaroof/fix/djcn-canonical-debug

Make Model's Debug exhaustive and the Debug fingerprint canonical across instances (improvise-djcn)
This commit is contained in:
Ed Langley
2026-09-23 10:48:23 -07:00
committed by GitHub
7 changed files with 176 additions and 16 deletions
+24
View File
@@ -516,6 +516,30 @@ change. `recomputing_formulas_leaves_the_model_fingerprint_alone` pins this.
`view_only_effects_leave_shared_state_alone` now runs over a formula-bearing
fixture, for the same reason.
**That fix was canonical within one instance, not across two (improvise-djcn).**
It fixed `Model`'s own maps but not the maps inside its fields. `DataStore` has
`index`, `records` and the symbol table, and `View` has `page_selections`,
`hidden_items` and `collapsed_groups`. All still printed through a derived
`Debug`, in hash order. Each `HashMap` gets its own `RandomState`, so two equal
documents built separately printed differently in 32 of 32 runs.
- The effect-classification tests compare one instance with itself before and
after, so they were unaffected.
- `a_model_command_lands_where_its_keypress_lands` compares **two** `App`s. It
passed only because its fixture had no cells and no multi-entry view maps.
Now:
- `DataStore`, `SymbolTable` and `View` have hand-written `Debug` impls. Each
sorts wherever its storage is a hash map.
- `DataStore` resolves symbols to their strings. A `Symbol`'s number is interning
order, not content, so printing it would make two equal stores differ again.
- `SymbolTable` prints its interned strings, sorted.
- `cells` keeps its `IndexMap` order, because that order is part of the document
(gotcha 18).
- Every hand-written `Debug`, `Model`'s included, **destructures** `self`. A field
added later is then a compile error there, where a `debug_struct` that names
fields one by one would skip it silently. The pc3b version of `Model`'s impl
had exactly that hole.
`parse_number_cell` returns `CellValue::Error` naming the offending text rather
than a fabricated `0.0`. No token the current grammar admits actually fails
`f64::from_str` (overflow saturates to ±inf), but if the two ever drift the