improvise-sjdb: judge an unstored record at its own cell, not its first column
RecordHash drops _Measure, so every column of a row carries the same record, and with a collapsed _Measure group the first is the fold. aggregate_refusal now finds the position by the whole key the write names. Rival for the stored-record test: stored records through cell_id fails it alone. 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:
@@ -3422,8 +3422,15 @@ The question it asks is the projection's own, never a second derivation of it:
|
||||
- A record found **in the store** is a record, whatever view anyone is on.
|
||||
`RecordHash` is document-scoped (gotcha 44).
|
||||
- A record found **only through the layout** is asked of `project::cell_id` at
|
||||
the position where it was found. `Aggregate`, a fold or a sum over a None-axis
|
||||
category, is refused.
|
||||
the position of **its cell**: the one whose key is the whole key the write
|
||||
names, record and measure together. `Aggregate`, a fold or a sum over a
|
||||
None-axis category, is refused. It must be the whole key, not the first
|
||||
position carrying the record (improvise-sjdb). `RecordHash` drops `_Measure`,
|
||||
so every column of a row carries the same record, and with a collapsed
|
||||
`_Measure` group the first of them can be the fold. That refused the empty
|
||||
`Units` cell beside a `Money` fold, which the host itself projects as a
|
||||
`Record`. A write whose whole key the host's grid does not draw is not
|
||||
refused here, as it was not before this rule existed.
|
||||
|
||||
**Why the `vec![]` arm stays.** A reviewer asked whether a `SetCell` whose cell
|
||||
does not resolve at all, which `model_effects` turns into `vec![]` and so
|
||||
|
||||
@@ -5611,3 +5611,29 @@ it and when a hand-rolled basis omits it. The second half was expected to be
|
||||
`ChainBroken`, and the test said otherwise: the create exemption covers only
|
||||
commands after the first in a frame. That surprise is why it is asserted rather
|
||||
than argued.
|
||||
|
||||
### an_empty_cell_beside_a_collapsed_measure_group_is_writable
|
||||
|
||||
improvise-sjdb, a regression PR #217 introduced and its review found. `_Measure`
|
||||
is on Column with Revenue and Cost in a collapsed group `Money` and `Units`
|
||||
ungrouped, and the Feb row is empty. The host projects Feb × Units as a
|
||||
`Record`. A caught-up `SetCell` on it was answered `Rejected(MutationRefused)`
|
||||
at `bc060f67`. It was accepted before #217. `aggregate_refusal` found the
|
||||
unstored record by `RecordHash` alone, which drops `_Measure`, so it judged the
|
||||
record at the first column on Feb's row, the `Money` fold, which `cell_id` calls
|
||||
`Aggregate`. The setup asserts that the fold comes before `Units` on the row and
|
||||
that the target projects as `Record`, so a fixture where the first column is
|
||||
harmless cannot pass. The fix looks the cell up by its whole key. The rival is
|
||||
the old lookup by record, which is this test's red commit, `6e021699`.
|
||||
|
||||
### a_stored_record_the_hosts_view_aggregates_is_writable
|
||||
|
||||
improvise-sjdb, from the #217 review. South/Revenue is stored, then `Product` is
|
||||
added on `Axis::None` in the host's view, so the host projects South's position
|
||||
as `Aggregate`. A stored record is a record wherever it is drawn, because
|
||||
`RecordHash` is document-scoped (gotcha 44). A records-mode client, or one on
|
||||
another view, may write it. The setup asserts the host really does project
|
||||
`Aggregate` there, so the test cannot pass on a view that draws South plainly.
|
||||
Rival: stored records sent through `cell_id` like unstored ones. That rival
|
||||
fails this test and nothing else in `improvise-command`. Before this test,
|
||||
nothing in the suite saw it.
|
||||
|
||||
@@ -583,24 +583,25 @@ impl App {
|
||||
f.target
|
||||
)
|
||||
});
|
||||
derived.or_else(|| self.aggregate_refusal(cell.record))
|
||||
derived.or_else(|| self.aggregate_refusal(cell.record, &key))
|
||||
}
|
||||
|
||||
/// A record the store does not hold is known only by its layout position, and
|
||||
/// is a record only where the projection there says `Record`, never `Aggregate`.
|
||||
fn aggregate_refusal(&self, hash: RecordHash) -> Option<String> {
|
||||
/// is a record only where the projection at ITS CELL says so (improvise-sjdb).
|
||||
fn aggregate_refusal(&self, hash: RecordHash, key: &CellKey) -> Option<String> {
|
||||
let stored = self.model_state.workbook.model.data.resolve_record(hash);
|
||||
let (row, col) = stored.map_or_else(|| self.record_position(hash), |_| None)?;
|
||||
let (row, col) = stored.map_or_else(|| self.cell_position(key), |_| None)?;
|
||||
let projected = crate::ui::project::cell_id(self, &self.layout, row, col);
|
||||
matches!(projected, Some(CellId::Aggregate(_))).then(|| {
|
||||
"an aggregate combines several records, so there is no one cell to write".into()
|
||||
})
|
||||
}
|
||||
|
||||
/// Where in the current layout a cell of record `hash` sits, if anywhere.
|
||||
fn record_position(&self, hash: RecordHash) -> Option<(usize, usize)> {
|
||||
/// Where in the current layout the cell `key` sits, if anywhere. By the whole
|
||||
/// key: a `RecordHash` drops `_Measure`, so a row's columns all share one.
|
||||
fn cell_position(&self, key: &CellKey) -> Option<(usize, usize)> {
|
||||
self.layout_cells()
|
||||
.find(|(_, _, key)| RecordHash::of(key) == hash)
|
||||
.find(|(_, _, at)| at == key)
|
||||
.map(|(row, col, _)| (row, col))
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user