Merge gh/main (666de199) into fix/fy9o-reopened-editor-reseeded
This head already carries #272 (p6v6), merged at d9c65181.
Conflicts:
- resolve.rs advanced: main's `reopened_at` (decline an aggregate or a
formula's result) now takes this branch's `writes` and answers the
decline arm first, then `awaiting_seed` or `opened_on(state, ..)`, as
the #282 review prescribed for this combination.
- context/test-evidence/{clients,command}.md: both sides' entries kept.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
@@ -227,6 +227,14 @@ checked by adding a `probe_field` to `Model` and to `Model::new`. The build then
|
||||
failed with `E0027: pattern does not mention field probe_field` at the `Debug`
|
||||
impl, and nowhere else.
|
||||
|
||||
**`Model.measure_agg` needed the same treatment (improvise-m6op).** `Model`'s
|
||||
`Debug` sorts `measure_agg` through a `BTreeMap`, but the fixture's
|
||||
`measure_agg` was empty, so nothing saw the sort. With the sort replaced by the
|
||||
raw `HashMap` (`.field("measure_agg", &measure_agg)`), this test stayed green in
|
||||
3 of 3 runs. The fixture now carries four entries: `Revenue` Avg, `Cost` Min,
|
||||
`Units` Max and `Orders` Count. Under the same injection it is red in 3 of 3
|
||||
runs, and it is green with the sort restored.
|
||||
|
||||
### a_stale_wire_model_command_cannot_be_applied
|
||||
|
||||
A wire `Command::Model` cannot reach the document without its basis being checked
|
||||
@@ -4539,6 +4547,8 @@ Two faults injected:
|
||||
assertion (`left: Some("0.50") right: Some("0.5000")`).
|
||||
- Dropping only the `col_formats` write in `viewport::grid_body`, leaving the
|
||||
cells correct, fails the totals assertion (`left: "1.50" right: "1.5000"`).
|
||||
Those numbers predate improvise-wm20. The total is now the formula over the
|
||||
pooled rows, `0.5000`, and the same rival reads `0.50`.
|
||||
|
||||
The second is the one that earns the test its length: without it, the totals
|
||||
strip could round every formula column to the view's format and no assertion in
|
||||
@@ -5349,11 +5359,67 @@ deliberate over-refusal, taken to keep one predicate.
|
||||
### enter_advancing_onto_a_fold_commits_nothing_there
|
||||
|
||||
The write half of improvise-enz6, closed by the commit door. `i` on Feb, then
|
||||
`Enter`, which commits and advances onto the Q2 fold and re-opens the editor
|
||||
there. That entry is still enz6's open half. Then `7` and `Enter`. At the
|
||||
previous commit door (`cell_key`) the second commit stored `Month=Q2`. Rival:
|
||||
`commit_at_cursor` on `cell_key` again. This test and
|
||||
`tab_advancing_onto_a_formula_cell_commits_nothing_under_it` fail.
|
||||
`Enter`, which commits and advances onto the Q2 fold. When this test was
|
||||
written, the advance also re-opened the editor there. That was enz6's open
|
||||
half, and `enter_advancing_onto_a_fold_does_not_reopen_the_editor_there` now
|
||||
closes it. Then `7` and `Enter`. At the previous commit door (`cell_key`) the
|
||||
second commit stored `Month=Q2`. Rival: `commit_at_cursor` on `cell_key` again.
|
||||
This test and `tab_advancing_onto_a_formula_cell_commits_nothing_under_it` fail.
|
||||
|
||||
**That rival no longer fails either test**, measured after enz6's entry half
|
||||
closed. The advance now leaves the user in normal mode on the fold, so `7` and
|
||||
`Enter` never reach the commit door. Both tests still pin the end-to-end
|
||||
property that the key path writes nothing there. The commit door itself is now
|
||||
pinned by `an_editor_already_open_on_a_fold_commits_nothing_there` and its
|
||||
formula twin, which that rival fails. The door is still reachable: a
|
||||
`:commit-cell-edit` line runs it at the cursor, and a browser editor can open
|
||||
past its window (`context/code/clients.md`).
|
||||
|
||||
### an_editor_already_open_on_a_fold_commits_nothing_there
|
||||
|
||||
The commit door alone, once improvise-enz6 closed the route the tests above
|
||||
reached it by. The test seats the editor on the fold directly: mode `Editing`,
|
||||
buffer `7`, cursor on the fold at (2, 1). Then `Enter`. Nothing may reach `## Data`
|
||||
with `Q2`, and the status names the collapsed group. Rival: `commit_at_cursor`
|
||||
back on `cell_key`. This test and the formula twin fail, and the four
|
||||
advance-path tests pass.
|
||||
|
||||
### an_editor_already_open_on_a_formula_cell_commits_nothing_under_it
|
||||
|
||||
The same door on `formula_app`'s Profit (row 1, column 2): `7` then `Enter`
|
||||
stores nothing under the formula. The rival is the one above.
|
||||
|
||||
### enter_advancing_onto_a_fold_does_not_reopen_the_editor_there
|
||||
|
||||
The entry half of improvise-enz6. `CommitAndAdvance` ends in `EnterEditAtCursor`,
|
||||
which opened the editor on whatever cell the advance landed on. `i` declines a
|
||||
fold, and the re-entry did not. The fixture is `app_on_a_fold`, whose fold sits
|
||||
at row 2, off row 0. `i` on Jan (row 0, column 1), then `Enter` to Feb. That
|
||||
first step is the control: Feb is an item, and the editor must re-open there,
|
||||
so a re-entry that never opens cannot pass. The second `Enter` lands on the
|
||||
fold, and the test asserts the mode is not editing and that the status names
|
||||
the collapsed group.
|
||||
|
||||
It was red at `202e7eac` (gh/main) on the mode assertion: `Editing`. A first
|
||||
version also asserted that `opened_editor` was `None`. It failed after the fix,
|
||||
because Feb's `OpenedEditor` outlives the commit. Nothing clears it in normal
|
||||
mode, and every opener overwrites it, so that is not this bug. The assertion
|
||||
was dropped rather than widening the fix.
|
||||
|
||||
Rivals:
|
||||
- `EnterEditAtCursor` opening everywhere (the `uneditable` check replaced by
|
||||
`None`): this test, its formula twin and the lockstep test fail.
|
||||
- `uneditable` answering `None` for a formula's result: the formula twin,
|
||||
`tab_advancing_onto_a_formula_cell_commits_nothing_under_it` and the lockstep
|
||||
test fail. That confirms the entry and the commit door ask one function.
|
||||
|
||||
### tab_advancing_onto_a_formula_cell_does_not_reopen_the_editor_there
|
||||
|
||||
enz6's entry half on the second arm `i` declines, a formula's result. This is the
|
||||
case improvise-jmti filed separately. `formula_app`, `i` on West/Revenue (row 1,
|
||||
column 0). `Tab` to Cost is the control, and the editor re-opens there. The
|
||||
second `Tab` lands on Profit, and the test asserts no editor and the formula's
|
||||
refusal text. It was red at `202e7eac` on the mode assertion. Rivals: as above.
|
||||
|
||||
### x_in_records_mode_clears_the_real_record_under_the_cursor
|
||||
|
||||
@@ -5385,7 +5451,10 @@ A door the tickets did not list, found by working the enumeration. `i` declines
|
||||
to edit a formula's cell, but `Tab` from West/Cost commits and advances onto
|
||||
Profit, `EnterEditAtCursor` re-opens the editor there, and `Enter` stored
|
||||
`7` under the formula. Found by this test failing at `90b93d4`, before
|
||||
`committable_cell_key` existed. The rival is the one above.
|
||||
`committable_cell_key` existed. The rival is the one above. Since
|
||||
improvise-enz6 the advance no longer opens that editor, so this test is
|
||||
end-to-end only and the rival passes it. The door is
|
||||
`an_editor_already_open_on_a_formula_cell_commits_nothing_under_it`.
|
||||
|
||||
### ui::host::tests::every_failing_model_command_refuses_its_whole_frame
|
||||
|
||||
@@ -5924,8 +5993,10 @@ is stored, so the stored arm accepted it without asking the projection. The test
|
||||
runs with `Money` collapsed, where the host draws that cell as the fold, and
|
||||
expanded, where the host draws no such cell. It asserts `MutationRefused` and an
|
||||
unchanged document in both, so a rule that asks the host's layout cannot pass
|
||||
the expanded half. Red at `5eaafbae`. The fix is `App::group_refusal`, which asks
|
||||
the document whether any coordinate names a group rather than an item. Rival:
|
||||
the expanded half. Red at `5eaafbae`. The fix was `App::group_refusal`, which
|
||||
asked the document whether any coordinate names a group rather than an item.
|
||||
Since improvise-nacd it is `App::non_item_refusal`, which asks whether every
|
||||
coordinate names an item. Rival:
|
||||
the group rule removed. Injected into a full workspace run, it fails this test
|
||||
and nothing else.
|
||||
|
||||
@@ -5946,6 +6017,72 @@ improvise-b114 rule is: no coordinate names a group, asked of the document.
|
||||
Rival: the group rule removed. Injected into a full workspace run, it fails this
|
||||
test and the b114 test, and nothing else.
|
||||
|
||||
### a_wire_write_to_an_item_named_like_its_categorys_group_is_accepted
|
||||
|
||||
improvise-pr8t, found in the adversarial review of PR #235 at `5c19fdd4`.
|
||||
`Category::add_item` checks only the items, so a category may hold an item
|
||||
`Money` and a group `Money` at once. The b114 rule exempted that case with
|
||||
`!category.items.contains_key(name)`, and nothing pinned the exemption: with it
|
||||
removed, the whole workspace stayed green while a caught-up write to
|
||||
`{Month=Feb, _Measure=Money}`, a real item's cell, was refused.
|
||||
|
||||
The test builds exactly that case on the stored record `{Month=Feb}`. It asserts
|
||||
`Accepted` and the stored value, so a rule that refuses and a rule that accepts
|
||||
but drops the write both fail.
|
||||
|
||||
The test passed on the unfixed code, as a pin should. Two rivals were run against it:
|
||||
|
||||
- On the b114 code, the exemption removed (`let group = true && …`): red,
|
||||
`a real item was refused: … Rejected(MutationRefused)`.
|
||||
- On the nacd code, a rule that also refuses any name that is a group, even
|
||||
when it is an item: red, and in a full `improvise-command` run this test is
|
||||
the only failure.
|
||||
|
||||
### a_wire_write_naming_no_item_of_its_category_is_rejected
|
||||
|
||||
improvise-nacd, found in the same review. The b114 rule refused a coordinate
|
||||
that *is a group*. The invariant it protects is wider: every coordinate names an
|
||||
item (improvise-gw18). Two writes fell in the gap. The fixture puts Revenue and
|
||||
Cost in `Money`, puts `Money` under the parent group `Totals`, and stores
|
||||
`{Month=Feb}`.
|
||||
|
||||
- `_Measure=Bogus` is no item and no group.
|
||||
- `_Measure=Totals` is a parent group that holds no item directly, so
|
||||
`items_in_group("Totals")` is empty and the group test did not see it.
|
||||
|
||||
Both were answered `Accepted` and stored the coordinate. The test asserts
|
||||
`Rejected(MutationRefused)` and an unchanged document for each. Both were
|
||||
confirmed red separately at `202e7eac`: the loop stops at its first failure, so
|
||||
the second name was checked by swapping the order.
|
||||
|
||||
The fix is `App::non_item_refusal`: every coordinate must be one of
|
||||
`Model::effective_item_names` of its category. The argument for a positive rule
|
||||
is in `context/code/command.md`, in the section on wire writes.
|
||||
|
||||
Rival: restrict the rule to `_Measure`, the ticket's first suggested fix shape.
|
||||
It fails `a_wire_write_to_a_fold_record_with_an_undrawn_measure_is_rejected`,
|
||||
because `Month=Q2` is a group of a regular category.
|
||||
|
||||
The fix also moved two fixtures in `ui/reduce.rs`. `model_app` and
|
||||
`a_wire_model_command_marks_the_document_dirty` stored or wrote
|
||||
`_Measure=Revenue` without adding `Revenue` as an item. So the cell they wrote
|
||||
was exactly the non-item coordinate this rule refuses. Both now add the item.
|
||||
That changes nothing they assert. It is the fixture being made a document that
|
||||
could exist.
|
||||
|
||||
### a_write_under_a_formula_is_refused_for_the_formula_not_as_a_non_item
|
||||
|
||||
improvise-nacd. A formula target is a `_Measure` name but not an item of the
|
||||
category, so the non-item rule could refuse a write under a formula before the
|
||||
`Derived` rule saw it. The write would still be refused, but for a reason that
|
||||
is false. The rule uses `Model::effective_item_names`, which includes formula
|
||||
targets, so the formula rule gives the reason.
|
||||
|
||||
Rival: the rule written with the category's raw `items`. Before this test, that
|
||||
rival passed the whole workspace, because every other formula test asserts only
|
||||
that the write is refused. It fails this test with
|
||||
`refused for another reason: Profit is no item of _Measure, …`.
|
||||
|
||||
### a_stored_record_the_hosts_view_aggregates_is_writable
|
||||
|
||||
improvise-sjdb, from the #217 review. South/Revenue is stored, then `Product` is
|
||||
@@ -6183,6 +6320,45 @@ of (North, Revenue). Typing `5` and pressing Enter must then store `25`. Before
|
||||
the fix the buffer was `""`, so the typed digit replaced the value instead of
|
||||
extending it.
|
||||
|
||||
## `ui/project.rs` — the totals strip (improvise-wm20)
|
||||
|
||||
### a_ratio_columns_total_is_the_ratio_of_the_totalled_inputs
|
||||
|
||||
improvise-wm20. `Margin = (Revenue - Cost) / Revenue` over North (10, 5) and
|
||||
South (30, 3). The rows read 0.50 and 0.90, so the three candidate rules give
|
||||
three different answers: summed **1.40**, averaged 0.70, and pooled
|
||||
(40 − 8) / 40 = **0.80**. A fixture whose rows share a ratio cannot tell them
|
||||
apart; the old `a_formulas_format_reaches_its_own_column_and_no_other` is one
|
||||
such fixture. **Red at `60179150`** with 1.40. Rival: the red commit itself,
|
||||
where the walk sums.
|
||||
|
||||
### an_average_measures_total_is_the_mean_over_its_records
|
||||
|
||||
improvise-wm20. `Price` under Avg, with `Product` on None, so North's row is the
|
||||
mean of two records (10, 20) and South's of one (30). Summing the rows gives
|
||||
**45**, the mean of the rows' means 22.5, and the mean of the records **20**.
|
||||
The ruling is the last. **Red at `60179150`** with 45.
|
||||
|
||||
### a_hidden_rows_values_stay_out_of_the_total
|
||||
|
||||
improvise-wm20. `East` (100, 100) is hidden, so both the Revenue total (40) and
|
||||
the pooled Margin (0.80) must leave it out. Pooled in, the Margin would read
|
||||
32/140. This test earns its place through two rivals, each injected into a full
|
||||
workspace run where each fails this test and nothing else:
|
||||
|
||||
- The span ignores `covers`, so hidden rows are pooled in.
|
||||
- A formula's references are read **unspanned**. The ratio test cannot see this
|
||||
one: an unspanned reference reads the column key over every record, which is
|
||||
the right answer whenever nothing is hidden. Only a hidden row separates
|
||||
"every drawn row" from "every record".
|
||||
|
||||
### a_max_measures_total_does_not_move_when_a_row_group_collapses
|
||||
|
||||
improvise-wm20, from the #203 re-review's comment on the bead. `Revenue` under
|
||||
Max, with Region on None and Month's Jan and Feb in Q1. Expanded, the rows are
|
||||
5 and 7, which a summing strip reads as **12**. Collapsed, the fold reads 7. The
|
||||
total must be 7 both ways. **Red at `60179150`** with 12.
|
||||
|
||||
#### improvise-i3o5: four declines that `set_status` hid from every sweep
|
||||
|
||||
`every_refusal_site_refuses_its_failure_and_only_its_failure` gained four cases.
|
||||
@@ -6212,6 +6388,29 @@ runs `cargo test --workspace --no-fail-fast`. Four of four fail this test, and
|
||||
nothing else fails. None of the four needs an abort: each is the only effect its
|
||||
command emits, and `p`/`i` are single bindings, not sequences.
|
||||
|
||||
### a_formula_rows_error_does_not_poison_the_pooled_total
|
||||
|
||||
improvise-b5dp, the user's ruling on the conflict between wm20's rulings (1) and
|
||||
(2): a formula column's total is the **pooled ratio**. A row's formula error
|
||||
does not poison it, and a stored error still does
|
||||
(`an_error_in_any_row_makes_the_total_an_error`). Rows North 25/30, South 27/30
|
||||
and East -4/0 draw 0.83, 0.90 and `div/0`. The total must read **0.80**
|
||||
(48/60), not an error. The test asserts that East's row really draws the error,
|
||||
so a fixture where no row errors cannot pass it.
|
||||
|
||||
It was **green at `9948aaf5`**, because #239 already implemented the choice the
|
||||
user then ratified. It is a pin, not a reproduction. Rival: in `grid_body`'s
|
||||
walk, a drawn cell's `Error` overwrote its column's total. That is ruling (2)
|
||||
applied to every row. This test and its Share sibling fail on the total
|
||||
assertion. The other seven `column_totals` tests pass.
|
||||
|
||||
### a_share_columns_total_pools_past_a_rows_div_zero
|
||||
|
||||
improvise-b5dp, the second case the bead named. `Share = Revenue /
|
||||
SUM(Revenue)`: East's own `SUM` is 0, so its row draws `div/0`. The pooled
|
||||
total is 60/60 = **1.00**. It was green at `9948aaf5` and fails under the same
|
||||
rival as the test above.
|
||||
|
||||
### a_frame_that_reframes_and_then_places_the_cursor_leaves_the_asker_placed
|
||||
|
||||
improvise-wdmp. A browser frame that reframes the grid and then places the cursor,
|
||||
@@ -6235,3 +6434,57 @@ case fails. Second, the ticket's suggested fix, which drops the asker's
|
||||
reset and the client never heard. That rival also fails
|
||||
`a_geometry_command_relays_the_cursor_reset_to_the_asker_too` and the
|
||||
ws-server's `the_other_document_would_also_have_a_cursor_to_invalidate`.
|
||||
|
||||
## `ui/host.rs` — the abort arm of `Host::mutating_frame` (improvise-we1a)
|
||||
|
||||
### every_failing_model_effect_that_aborts_also_refuses
|
||||
|
||||
What the gap was: `Host::mutating_frame` rejects a frame when
|
||||
`last_batch_refused() || last_batch_aborted()`. Every model effect that aborted
|
||||
also refused, so deleting the `|| last_batch_aborted()` arm left all tests
|
||||
passing. The wire table (`every_failing_model_command_refuses_its_whole_frame`)
|
||||
cannot hold the invariant, because that arm rejects a frame that aborted
|
||||
without refusing. The test applies each failing `ModelCommand`'s own effects,
|
||||
from `model_effects`, straight to an `App`, and asserts that the batch refused.
|
||||
Each case also carries whether it aborts. That catches a table in which nothing
|
||||
aborts, and a site that quietly stops aborting.
|
||||
|
||||
Faults injected:
|
||||
- `AddCategory`'s failure arm set `status_msg` and aborted, but did not refuse.
|
||||
- This test failed, and so did the key-path table.
|
||||
- The wire table passed, which is the gap.
|
||||
- With the arm also deleted, `a_refused_mutation_rolls_back_its_predecessor_and_the_abort_flag`
|
||||
failed as well. So the arm is what makes the wire table look correct under
|
||||
this fault.
|
||||
- `refuse_and_stop` wrote the status and aborted without refusing. This test
|
||||
failed, as did the key-path table,
|
||||
`a_failed_load_or_import_does_not_run_the_quit_after_it` and
|
||||
`a_formula_that_does_not_parse_keeps_the_user_in_the_editor_with_their_text`.
|
||||
The wire table passed.
|
||||
|
||||
What this table cannot see (improvise-lasd, found in review of #229): a site it
|
||||
does not list. The fault injected was `on_view`'s missing-view branch changed to
|
||||
write the status and abort, without refusing. The workspace stayed green, and
|
||||
only `Host::mutating_frame`'s `|| aborted` arm caught it. That is why
|
||||
`App::apply_effects` now carries a `debug_assert!`: a `Model` effect that aborts
|
||||
must have refused. Under the same fault, the assertion fails two existing tests:
|
||||
- `a_frame_writing_a_view_its_own_first_command_deleted_is_refused`, with
|
||||
`SetNumberFormat(Named("Scratch"), ",.2f")`;
|
||||
- `a_command_naming_a_view_the_document_has_not_got_writes_nothing`, with
|
||||
`SetAxis { view: Named("NoSuchView"), .. }`.
|
||||
|
||||
With the assertion removed, the same fault fails nothing.
|
||||
|
||||
### every_refusal_site_refuses_its_failure_and_only_its_failure — three cases (improvise-rtud)
|
||||
|
||||
This entry covers the rows `load of an absent file`, `import-json of an absent
|
||||
JSON` and `a formula that does not parse`. Each has a succeeding twin
|
||||
(`load doc.improv` from `MemoryFs`, `import-json good.json`, `Total = 1`).
|
||||
Each row was checked by a rival at its own site: the `refuse_and_stop` call in
|
||||
`AddFormula`'s parse arm, in `LoadModel`, or in the JSON read arm of
|
||||
`ImportJsonHeadless`, replaced by a status write plus the abort. Under each
|
||||
rival, the table fails at that row. The matching row of
|
||||
`every_failing_model_effect_that_aborts_also_refuses` fails too. A rival in
|
||||
`refuse_and_stop` itself is no use here, because the table stops at its first
|
||||
failing row, which is the older `import-json of an absent CSV`. The rtud prose correction is landing separately, so these
|
||||
rows are coverage only.
|
||||
|
||||
Reference in New Issue
Block a user