docs(test-evidence): what this wave's persistence and view tests are evidence of
Keyed by test name per design-principles section 7, recording for each one the bug it reproduced, the fault injected to watch it fail, and -- where a test has a neighbour that looks like it -- the input that separates the two. Three entries are about that separation rather than about a bug: - a_view_that_does_not_prune_writes_no_prune_line is the direction the default supplies, so a writer emitting "prune: true" unconditionally passes the real test and fails this one. - an_interleaved_category_keeps_its_item_order explains why the ticket's own two-item reproduction cannot discriminate: with one item of each kind, both the right and the wrong order are a single bare run followed by a single grouped one. - slash_in_hidden_name_survives_roundtrip records that improvise-nvq4 does NOT reproduce, and is kept as a passing control so the claim cannot be re-filed without someone first watching it pass. core.md is touched for the improvise-qx1z entries, which are View-level and belong there rather than in formula-and-io.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
@@ -351,6 +351,41 @@ test flips the category back to `Page` afterwards, so it distinguishes "omits
|
||||
The fixture puts a category on each of `Page`, `Filter`, `Row` and `None` so the
|
||||
result separates the two questions rather than merely counting.
|
||||
|
||||
### on_category_removed_forgets_a_collapsed_group
|
||||
|
||||
improvise-qx1z arm 2. `View::on_category_removed` cleared `category_axes`,
|
||||
`page_selections` and `hidden_items` — but not `collapsed_groups`. The other
|
||||
three lines being right there is what makes this an omission rather than a
|
||||
decision: collapse `Month/Q1`, delete the category, re-add it with `Jan[Q1]`,
|
||||
and the group comes back **pre-collapsed** with nothing saying why. Reachable
|
||||
from shipped keys, through the category panel.
|
||||
|
||||
Fault injected: delete the added line; the test fails at
|
||||
`is_group_collapsed("Month", "Q1")`. The test re-adds the category after
|
||||
removing it, because a view that merely *forgot the axis* would also report
|
||||
`false` for a category it no longer knows — asking the question after the
|
||||
re-add is what separates "cleared" from "not currently registered".
|
||||
|
||||
### on_item_removed_forgets_a_hidden_item
|
||||
|
||||
improvise-qx1z arm 1, at the `View` level. `Model::remove_item` removes the item
|
||||
and its cells and **touches no view at all**, so `hidden_items["Region"]` kept
|
||||
`"East"`: re-adding the item put it in the model and in the category panel while
|
||||
leaving it absent from the grid, with no indication why.
|
||||
|
||||
The fixture hides **two** items and removes one. Without the second, a fix that
|
||||
dropped the whole category entry — `hidden_items.remove(cat_name)` — would pass
|
||||
while destroying unrelated state; the surviving `West` assertion is the input
|
||||
that separates "forget this item" from "forget this category".
|
||||
|
||||
### on_item_removed_forgets_a_page_selection_naming_it
|
||||
|
||||
The third question improvise-qx1z asked to check while there, and the answer was
|
||||
the same: a page selection naming a removed item survived it. Asserted in both
|
||||
directions in one test — removing a *different* item must leave the selection
|
||||
alone — because a hook that cleared the selection unconditionally would satisfy
|
||||
the removal half and silently reset the user's page on every unrelated delete.
|
||||
|
||||
## `view/layout.rs`
|
||||
|
||||
### prune_axis_items_indexes_data_positions_not_visual_ones
|
||||
|
||||
@@ -442,6 +442,118 @@ honest — without it, "the writer quotes it" would be consistent with the reade
|
||||
also accepting the unquoted form. The trailing precondition (the same line
|
||||
quoted *does* parse) stops it passing because of some unrelated syntax error.
|
||||
|
||||
### prune_empty_survives_the_roundtrip
|
||||
|
||||
improvise-qu4a. `View::prune_empty` was never written to disk: `format.rs` wrote
|
||||
`format:`, `hidden:` and `collapsed:` for a view and nothing else, and
|
||||
`improv.pest`'s `view_entry` had no `prune` alternative. Pressing `P`, saving
|
||||
and reopening brought every pruned row and column back — and the toggle marks
|
||||
the document dirty, so the user is invited to save the setting that is about to
|
||||
be discarded.
|
||||
|
||||
The reason nothing caught it is the shape worth remembering: it is **invisible
|
||||
to `fmt(parse(x)) == fmt(parse(fmt(parse(x))))`, because both sides drop it**
|
||||
(`doctrine(a_roundtrip_equality_is_blind_to_a_loss_both_sides_share)`). So this
|
||||
test asserts the **flag**, never the output's stability. Fault injected: delete
|
||||
the two-line `w!` in `format_md`; the test fails at the `prune: true` assertion,
|
||||
and the two controls below still pass — which is exactly why they are controls
|
||||
and not the evidence.
|
||||
|
||||
#### a_view_that_does_not_prune_writes_no_prune_line
|
||||
|
||||
The control, stated separately because it is the direction the default supplies.
|
||||
A writer that emitted `prune: true` unconditionally passes the test above and
|
||||
fails this one. Not evidence of the bug; evidence the fix is not one-directional.
|
||||
|
||||
#### a_file_without_a_prune_line_does_not_prune
|
||||
|
||||
The compatibility reading, decided rather than defaulted: a file written before
|
||||
the line existed means `false`, which is what happens today. `improv.pest`
|
||||
spells `prune: true|false` rather than making the line's presence the boolean,
|
||||
so the grammar-walking generator can emit `prune: false` and it round-trips to
|
||||
`false` through a writer that omits it.
|
||||
|
||||
### a_file_naming_its_own_views_gains_no_default
|
||||
|
||||
improvise-m4rv. `Workbook::new` seeds a `Default` view and `on_category_added`
|
||||
assigns every parsed category into **every** existing view, so a file naming
|
||||
only `Plan` parsed into *two* views — the second auto-laid-out and never in the
|
||||
file. Two consequences, the second worse: the shipped demo's view panel showed a
|
||||
phantom row **at index 0**, so the first `V`+`Enter` a visitor pressed took them
|
||||
off the curated view; and open-and-save with no edits rewrote the user's
|
||||
document, appending the whole `## View: Default` block permanently.
|
||||
|
||||
`examples/demo.improv` was unaffected only because its view *is* named
|
||||
`Default`, which is why the bug survived the shipped-document tests.
|
||||
|
||||
Fault injected: remove the `drop_unnamed_default` call in `build_workbook`; the
|
||||
test fails with `["Default", "Plan"]`. The fix is narrow on purpose — it drops
|
||||
`Default` only when the file named **at least one** view and none of them was
|
||||
`Default`, so the two cases in the control below are untouched.
|
||||
|
||||
#### a_file_naming_no_view_keeps_the_seeded_default
|
||||
|
||||
The control the fix must not break, in both directions: a file with no `## View`
|
||||
section still opens on the seeded view (the legacy reading), and a file that
|
||||
names `Default` itself keeps it. Without this, "drop Default" could have been
|
||||
implemented as "always drop Default" and the test above would not have noticed.
|
||||
|
||||
### the_aggregation_section_is_written_in_a_stable_order
|
||||
|
||||
improvise-ppru's companion. `Model::measure_agg` is a `HashMap`, and an index
|
||||
whose iteration order is hash order is not an order (gotcha 18) — writing it
|
||||
unsorted would give a different `.improv` file per process, the same class of
|
||||
defect as `analyze_records`. The test pins the three lines in name order and
|
||||
then asserts `format(parse(text)) == text`. Fault injected: drop the
|
||||
`sort_by_key`; the assertion fails on most runs, which is itself the argument
|
||||
for pinning it rather than trusting a single green run.
|
||||
|
||||
### an_interleaved_category_keeps_its_item_order
|
||||
|
||||
improvise-3yv3. `format_md` collected every ungrouped item into `bare` and
|
||||
emitted that whole list *before* any grouped item, so a category built as
|
||||
`[grouped, ungrouped]` came back `[ungrouped, grouped]`. Grid axis ordering
|
||||
consumes item insertion order (gotcha 18), so reopening a document silently
|
||||
rearranged its rows and columns.
|
||||
|
||||
The fixture interleaves deliberately — `a, b, g1[G], c, g2[G]` — because the
|
||||
ticket's own two-item reproduction cannot tell "writes grouped items last" from
|
||||
"writes each run in place": with one item of each kind, *both* orders are a
|
||||
single bare run followed by a single grouped one. The test asserts the four
|
||||
emitted lines, not just the parsed order, so a writer that happened to re-sort
|
||||
correctly by accident still fails. Fault injected: restore the two-vector split;
|
||||
the line assertion fails with `["- a, b, c", "- g1[G]", "- g2[G]"]`.
|
||||
|
||||
### a_cell_with_no_coordinates_round_trips
|
||||
|
||||
improvise-pnf5, the P1. A cell keyed by the empty `CellKey` is a legal model
|
||||
state — it is what `AddRecordRow` leaves while a records row is unfinished, and
|
||||
it is reachable from the shipped `r` then `o` keys. `format_md` wrote it as
|
||||
`" = 0"` and `coord_list` required at least one coordinate, so **saving in
|
||||
records mode overwrote a good document with one the application could not
|
||||
reopen**. `CleanEmptyRecords` runs on leaving records mode, not before `Save`.
|
||||
|
||||
The fix is a spelling rather than a discard, because the ticket is explicit that
|
||||
legitimate data must not be dropped: `data_coords = _{ (coord_list ~ " ")? }`
|
||||
gives the scalar cell the line `= 42`. It is unambiguous under PEG because
|
||||
`coord` needs a `name`, and `bare_name` cannot start with `=`, so the optional
|
||||
group fails cleanly and the `"= "` literal matches.
|
||||
|
||||
Fault injected: revert the writer's `match` to the old `"{} = {}"`; the test
|
||||
fails at the `\n= 42\n` assertion, and `parse_md` then rejects its own writer's
|
||||
output. The trailing `format_md(&back) == text` is what makes this a round trip
|
||||
rather than a parse check.
|
||||
|
||||
#### slash_in_hidden_name_survives_roundtrip
|
||||
|
||||
**Not a bug.** improvise-nvq4 (external report) claimed a `/` in an item or
|
||||
group name corrupts the `hidden:`/`collapsed:` line, whose format is
|
||||
`Cat/Item`. It does not reproduce: `hidden_line` reads two `name`s, a `name` is
|
||||
`pipe_quoted | bare_name`, and `bare_name` excludes `/` — so `quote_name`
|
||||
already writes `hidden: C/|A/B|` and `extract_name_pair` reads it back. The test
|
||||
is kept as a **passing control** over both maps, so the claim cannot be re-filed
|
||||
without someone first watching this pass.
|
||||
|
||||
---
|
||||
|
||||
## `improvise-io/src/persistence/export.rs`
|
||||
|
||||
Reference in New Issue
Block a user