Merge remote-tracking branch 'gh/main' into collapse-fu/gw18

# Conflicts:
#	context/test-evidence/clients.md
This commit is contained in:
Edward Langley
2026-09-22 22:38:14 -07:00
16 changed files with 313 additions and 81 deletions
+6
View File
@@ -226,3 +226,9 @@ learns no coordinate space, and an invalid operation is unconstructible rather
than rejected. What the extension adds is that the enum is now **four** answers
to "what may be done here", so `None` finally means only what this ADR always
said it meant — no projected cell at all.
**A fifth kind (2026-09-22, improvise-exdg).** The records-mode coordinate cell
was the last `None` that named a visible cell. It is now `CellId::Coordinate`.
The variant carries no operand, because no client composes a coordinate rename.
The previous paragraph's "four answers" is therefore five. `None` now means
only a padded or blanked slot. Reasoning: `context/code/protocol.md` §3.
+25
View File
@@ -1700,6 +1700,31 @@ watchdog must be DISARMED and not merely pre-empted**: it fires later and would
overwrite the better message with "did not answer" (PR #13 review). The watchdog
is for silence only, and `setShellFailure` clears the timer.
**Success is claimed by the first frame, and only if nothing failed first**
(improvise-i0v9). The worker transport used to write `in-page worker · <doc>`
the moment it spawned the worker — 13ms after navigation, before anything had
answered — so a worker that could not start showed a success-looking status
over a blank page. It now writes `starting the worker · <doc>…`, and
`createShell().arm(message, { answer })` holds the claim until `sawFrame`. The
rule the claim obeys is **the watchdog's own state**: it is written only if the
watchdog is still armed when the first frame lands. That is not a convenience.
`worker.js` posts a `ReadOnly` (or `SeedMissing`) notice straight after
`start()` and **before** the Snapshot that answers the handshake, so an
unconditional "first frame ⇒ say we are up" would overwrite the precise reason
the notice just wrote. `fail` already disarms the watchdog, so "armed" answers
"has anything been said since arming" without a second flag to keep in step.
`fail` disarms **directly** rather than through `sawFrame`, or it would
briefly flash the success text on its way to the failure. The starting text
deliberately does not contain `in-page worker`, so the older specs asserting
that substring now need the first frame to pass. The socket path passes no
`answer`: `open` is already a real fact, and its status was never premature.
One consequence changes what the watchdog guards. In the read-only case a
Snapshot always follows the notice, and `sawFrame` disarms the watchdog either
way, so a notice that forgot to disarm would **not** be overwritten eight
seconds later. It is overwritten **at once**, by the claim. The eight-second
overwrite survives only where no frame follows: `BootFailed`.
**Did this socket ever open?** The page has to record that itself, because
nothing on the close event carries it: `readyState` is already CLOSED by the time
a close listener runs, whichever happened, and a refused connection and a
+43 -13
View File
@@ -614,14 +614,37 @@ broader than its reason — improvise-gcvb, which also records the two things
that make it more than a one-liner: an inserted `+cat` column means the TUI
does **not** advance, and `AddCategory` can abort.
### `CellId::Coordinate` — a drawn coordinate is not "no cell" (improvise-exdg)
A records-mode coordinate cell (the `Region` column holding `North`) used to
project `None`. That was a **third** meaning of `None`, next to the padded slot
and the blanked position: a visible cell that holds text and whose write
belongs to the host. So absence was encoding a permission, the same conflation
`Create` removed for the creating columns (improvise-poia). The pinning test,
`a_coordinate_column_is_projected_with_no_identity`, asserted the bug and is
gone.
`Coordinate` carries **nothing**. Editing a coordinate renames it across every
measure cell in the row (gotcha 1), and no client composes that operation: it
needs `record_col`, and the browser declines every records-mode edit before
reaching a cell. Giving the variant a `RecordHash` would hand a client a
surrogate with nothing to spend it on. The operand belongs with the command
that consumes it, whenever that command exists. Nothing a client can do has
changed. `render.rs::editable` is now an exhaustive `match` rather than a
`matches!`, so the next variant has to be classified. `typed_cell_command`,
`record_at_cursor` and `record_cell_at_cursor` still answer `None` for it.
So paste onto a coordinate is still a silent `Upstream` in the browser. The
TUI has declined it out loud since PR #184. That divergence is
improvise-kkcy, and a named kind is what lets the browser decline deliberately
now.
### `CellDisplay::id` — `None` is not "aggregate"
That is `Some(CellId::Aggregate(..))`, and the distinction is the whole point
of the enum. `None` means the slot holds **no projected cell**: a `GridCache`
position padded by `fit_to_labels`, blanked by a reframing, or a
records-mode column that is not a cell to write into at all (a category column
renames a coordinate across the whole row, gotcha 1, which is a different
intent from `SetCell`). Conflating the two either hides a cell or offers a
position padded by `fit_to_labels`, or one blanked by a reframing. Every
records-mode column is some kind of cell (`Record`, `Derived`, `Create` or
`Coordinate`). Conflating the two either hides a cell or offers a
write the type system exists to make unconstructible.
### `CellDisplay::edit_text` — the difference corrupts data
@@ -637,8 +660,8 @@ and an editor's seed is exactly a feedback path.
It is the projection of `App::display_value_at_cursor`, which is what
`EditOrDrill` seeds the TUI's buffer from, so both front ends start from one
string rather than each deriving one. `None` where there is nothing to edit:
the same set of positions `id` is `None` for, plus cells whose value is
absent. An error cell carries none, because its text is a diagnostic and not
positions whose `id` is `None`, plus pivot cells with no key. A records-mode
cell always carries one, even an empty one. An error cell carries none, because its text is a diagnostic and not
a value.
A browser found this: `tests/lockstep.rs` drove `i` through the host and the
@@ -785,9 +808,16 @@ one**. `AxisFrame::flat` leaves `cats` empty on purpose, so it is not a way to
smuggle a multi-level axis in flattened.
`FrameGroup::collapsed` is on the wire rather than left to the client because
it is a fact about the *view*, which only the server holds. `FrameGroup::cat`
travels so a client can echo it back in `ToggleGroup` without ever working out
which category a label came from — which slash-joined labels made impossible.
it is a fact about the *view*, which only the server holds. There is no
`FrameGroup::cat` any more (improvise-7xyx). Its doc comment promised that a
client echoes it back in `ToggleGroup`, but no client ever did. `resolve.rs`
has no `toggle-group-under-cursor` arm (`z` is on the still-needs-the-host
list), and neither renderer read the field. A promise about an interaction no
code performs is the stale-sibling defect, so the field went with it. It is
not a loss for `z`. The TUI's `ToggleGroupAtCursor` also places the cursor on
the layout the toggle *will produce*, which a thin client cannot compute. So
composing `z` in a browser needs its own design, and the operand should come
back with that design rather than wait for it.
### `AxisFrame`: four accessors that are **not for a draw loop**
@@ -1294,13 +1324,13 @@ frozen behaviour reads as decided afterwards and nothing says it was only frozen
for the duration of the move.
**The ticket's own proposed fix does not work, and that is worth more than the
fix.** h0iv suggested comparing the `(cat, name)` pair, `FrameGroup` carrying
`cat` precisely so a client need not work out which category a label came from.
fix.** h0iv suggested comparing the `(cat, name)` pair; `FrameGroup` carried
`cat` then (improvise-7xyx has since removed it).
But its own reachability case is *one* category: `Month` grouped into `Q1`,
nested under `Region`, giving `North/Q1` and `South/Q1` adjacent — the two
headers are `(Month, Q1)` and `(Month, Q1)`, **equal as values**, so the pair
separates nothing. `the_two_headers_are_equal_as_values_so_comparing_cat_and_name_cannot_split_them`
pins that, so the next reader does not re-propose it. What separates them is
separates nothing. `the_two_headers_are_equal_as_values_so_comparing_them_cannot_split_them`
pins that as whole-value equality, so the next reader does not re-propose it. What separates them is
that they are two different `FrameEntry::Group` entries, and `groups()` already
borrows each from `self.entries` — so the comparison is `std::ptr::eq` on the
header, which is the literal statement of "the same header opened both".
+49
View File
@@ -3054,6 +3054,23 @@ The third is the gotcha 48 property, and it needs `timesArmed() > 0` first or it
denies an unreachable event: the precise reason must still be on screen after
the 8s window, not replaced by the watchdog. `page.clock` walks past it.
**Strengthened by improvise-i0v9's second pass.** Every assertion above read the
*final* `#status`, so two failures could still pass it. A watchdog that fired
first and was then replaced by a late notice passed. So did a success claim
shown before the notice. The test now records every status with `logStatus` and
asserts, **before** the page clock is walked, that the log ends in `read-only`
and never contained `did not answer` or the exact `in-page worker · <doc>`. A
reason that arrived after the watchdog would sit in that log behind "did not
answer"; that is the "well before 8s" property, stated without a wall-clock
bound. Measured failing against `gh/main` at `da2e5ab`, whose log was
`["in-page worker · two-tab-….improv", "read-only — …"]`: the premature claim
the ticket measured at 13ms is still there once 1wft's notice exists. The
brief's rival, `onNotice` calling `setShell` instead of `fail` (the watchdog not
disarmed), fails it at the first `toContainText("read-only")`. The Snapshot's
claim replaced the reason at once, which is why the eight-second form of this
rival is now exercised by the invalid-`?doc=` spec instead (see
`context/code/clients.md`, "The watchdog").
It runs on Chromium and Firefox — the two engines with OPFS — rather than
Chromium alone, because the exclusivity semantics are the engine's and the two
word the refusal completely differently.
@@ -4458,6 +4475,38 @@ a client that never writes cannot pass. The fold case,
at row 2. Rival for all three: the host's paste/clear door restored to
`ctx.cell_key()`, which was `gh/main`'s code. All three fail.
### the worker is not called up until its first frame has arrived
improvise-i0v9. The worker transport wrote `in-page worker · <doc>` when it
spawned the worker, before anything had answered. A worker that then failed
therefore showed a success-looking status over a blank page. The spec records
every `#status` with `logStatus` and requires the whole log to be exactly
`["starting the worker · <doc>…", "in-page worker · <doc>"]`. It pins the order
as well as the final text, since a final read cannot see a claim made too early.
Measured failing against `gh/main` at `da2e5ab`. The rival, restoring the
immediate `setShell(\`in-page worker · ${DOC}\`)` in `workerTransport`, fails it
on the log's first entry. The final read is `toHaveText`, not `toContainText`.
The older specs' `toContainText("in-page worker")` could not tell the starting
text from the claim if the one contained the other, and the starting text
deliberately does not.
### only the first frame claims the peer answered, and not before it
improvise-i0v9, the unit half of the spec above. `arm(message, { answer })`
must write nothing until `sawFrame`, then write `answer` once, however many
frames follow. Fails against `gh/main`'s `createShell`, which has no `answer`
and so never writes the claim at all.
### a reason given before the first frame is not replaced by the frame's claim
The order `worker.js` really produces on a locked document: the `ReadOnly`
notice is posted after `start()` and **before** the Snapshot. So `fail` then
`sawFrame` must leave the notice's text as the only thing ever shown. The rival
is the obvious fix, `sawFrame` writing `answer` unconditionally. It fails both
this test and the one above: the first on its exact log, which is then written
twice. This test passes against `gh/main` by construction, since no claim exists
there. It is not evidence of the bug; it guards against the naive fix.
## Numbers across the serialized wire (improvise-6oy4)
No manifest enabled serde_json's `float_roundtrip`, so its default f64 parser
+26 -10
View File
@@ -1978,12 +1978,27 @@ Two measure columns of one row are two cells of one record, and the next row is
different record — the same shape the pivot case has, checked here because records
mode reaches it by a different path.
### a_coordinate_column_is_projected_with_no_identity
### a_coordinate_cell_is_named_rather_than_left_as_no_cell
A coordinate column is not a cell to write into: editing it renames that coordinate
across *every* measure cell in the row (gotcha 1), which is a different intent from
`SetCell`. So it is projected with no id at all, and a client cannot compose a
write against it.
improvise-exdg. This entry replaces `a_coordinate_column_is_projected_with_no_identity`,
a test that **pinned the bug**. It asserted that a records-mode coordinate cell
projects `id: None`. ADR 002 and gotcha 42 define `None` as "no projected cell",
yet the Region column is drawn and holds text. So a client could not tell it
from a padded slot. Absence was being used to encode a permission ("the host's"),
which is the conflation ADR 002's rejected `Option<CellId>` draft was about.
`improvise-poia` removed the same conflation for the creating columns.
The cell now projects `CellId::Coordinate`. That keeps the old test's real
property: it is still not a `SetCell` target, because `typed_cell_command`,
`record_cell_at_cursor` and `render.rs::editable` all match `Record` alone.
The test seats the cursor on the **second** row, so the first-row and
first-item cases cannot coincide. It then enumerates every cell of the records
grid and requires each to carry an id, which is the ticket's claim with no
case picked by hand. Before the fix it failed at `a drawn coordinate projected
as no cell`. Rival injected afterwards: `RecordCol::Category(_) => return None`
restored. This test failed at the `Some(CellId::Coordinate)` assertion, and
`a_creating_column_is_named_rather_than_left_as_no_cell` (whose `None`-count
now expects zero) failed too.
### a_write_moves_the_version_under_the_clients_own_surrogate
@@ -3298,11 +3313,12 @@ aggregates, arriving a third time.
The test walks **every** column of a records grid and counts, rather than
asserting the two creating columns alone. That matters in the direction a
weaker test fails: a producer that answered `Create` for everything, or that
made the `Region` coordinate column nameable, would pass a two-position check
and fails this one. The fault injected — returning `None` for both creating
columns, which is the pre-fix code — reports three nameless columns where one
is allowed.
weaker test fails: a producer that answered `Create` for everything would pass
a two-position check and fails this one. The fault injected — returning `None`
for both creating columns, which is the pre-fix code — reported three nameless
columns where one was allowed. Since improvise-exdg the coordinate column is
named too (`CellId::Coordinate`), so the count of nameless columns is now zero.
See `a_coordinate_cell_is_named_rather_than_left_as_no_cell`.
### a_records_coordinate_is_text_however_it_is_spelled
+7 -5
View File
@@ -557,7 +557,7 @@ the evidence that nothing was relying on the permissive parse.
### render::tests::two_runs_under_two_headers_of_one_name_are_two_runs
### render::tests::the_two_headers_are_equal_as_values_so_comparing_cat_and_name_cannot_split_them
### render::tests::the_two_headers_are_equal_as_values_so_comparing_them_cannot_split_them
improvise-h0iv. `group_runs` coalesced adjacent positions by group **name**, so
two runs under two different headers sharing a name read as one and both front
@@ -571,10 +571,12 @@ and nothing ungrouped between the runs. Before the fix it returns
at the precondition rather than pass with a single run.
The second test exists because the ticket proposed the wrong fix and a reader
will propose it again. h0iv says `FrameGroup` carries `cat` and "comparing the
pair would fix it" — but its own reachability case is a single category nested
under an outer one, so both headers are `(Month, Q1)`, equal as values. The test
asserts that equality directly. What actually separates them is that they are
will propose it again. h0iv said `FrameGroup` carried `cat` and that "comparing
the pair would fix it". But its own reachability case is a single category
nested under an outer one, so both headers are `(Month, Q1)`, equal as values.
The test asserts that equality directly. Since improvise-7xyx removed
`FrameGroup::cat`, it asserts equality of the whole header, which is stronger:
no field-wise comparison can separate two equal values. What actually separates them is that they are
two distinct entries in `self.entries`, which `std::ptr::eq` on the borrowed
header states exactly.
@@ -100,7 +100,7 @@ impl<'a> CmdContext<'a> {
match self.cursor_cell_id() {
Some(CellId::Aggregate(_)) => Err(self.aggregate_refusal()),
Some(CellId::Derived { target }) => Err(derived_refusal(&target)),
Some(CellId::Record(_) | CellId::Create(_)) | None => {
Some(CellId::Record(_) | CellId::Create(_) | CellId::Coordinate) | None => {
self.cell_key().ok_or_else(|| NO_CELL.to_string())
}
}
@@ -113,7 +113,7 @@ impl<'a> CmdContext<'a> {
Some(CellId::Record(_)) => self.stored_cell_key(),
Some(CellId::Aggregate(_)) => Err(self.aggregate_refusal()),
Some(CellId::Derived { target }) => Err(derived_refusal(&target)),
Some(CellId::Create(_)) => Err(NO_VALUE_REFUSAL.to_string()),
Some(CellId::Create(_) | CellId::Coordinate) => Err(NO_VALUE_REFUSAL.to_string()),
None => Err(match self.cell_key() {
Some(_) => NO_VALUE_REFUSAL,
None => NO_CELL,
+32 -21
View File
@@ -222,7 +222,6 @@ fn axis_frame(
pending.clear();
}
pending.push(FrameEntry::Group(FrameGroup {
cat: cat_name.clone(),
name: group_name.clone(),
collapsed: view.is_group_collapsed(cat_name, group_name),
}));
@@ -328,7 +327,7 @@ pub(crate) fn cell_id(
RecordCol::NewCategory => return Some(CellId::Create(CreateKind::Category)),
// Renaming a coordinate moves every measure cell in the row, which is a
// different intent from `SetCell` and still the host's (gotcha 1).
RecordCol::Category(_) => return None,
RecordCol::Category(_) => return Some(CellId::Coordinate),
};
return Some(CellId::Record(RecordCell {
record: RecordHash::of(record),
@@ -497,7 +496,10 @@ pub fn published_basis(app: &App, grid: &GridCache) -> Basis {
.flatten()
.filter_map(|cell| match cell.id.as_ref()? {
CellId::Record(rc) => Some((rc.record, app.record_version_of(rc.record))),
CellId::Aggregate(_) | CellId::Derived { .. } | CellId::Create(_) => None,
CellId::Aggregate(_)
| CellId::Derived { .. }
| CellId::Create(_)
| CellId::Coordinate => None,
})
.collect();
basis::published(app.document_generation(), app.layout_version(), records)
@@ -4857,20 +4859,6 @@ mod cell_identity {
);
}
/// A coordinate column is not a cell to write into (gotcha 1), so it is projected
/// with no id and a client cannot compose a write against it.
#[test]
fn a_coordinate_column_is_projected_with_no_identity() {
let app = records_app();
let region = column_named(&app, "Region");
assert_eq!(
grid_of(&app, WHOLE)
.cell(0, region)
.and_then(|c| c.id.clone()),
None
);
}
/// A document where `Profit` is BOTH an explicit `_Measure` item and a
/// formula target — which the `+` column lets a user build.
fn shadowed_measure_records_app() -> App {
@@ -5142,10 +5130,9 @@ mod cell_identity {
let ids: Vec<Option<CellId>> = (0..app.layout.col_count())
.map(|c| grid.cell(0, c).and_then(|cell| cell.id.clone()))
.collect();
assert_eq!(
ids.iter().filter(|id| id.is_none()).count(),
1,
"only the one Region coordinate column may be nameless: {ids:?}"
assert!(
ids.iter().all(Option::is_some),
"no records column is nameless since improvise-exdg: {ids:?}"
);
assert_eq!(
ids.iter()
@@ -5166,6 +5153,30 @@ mod cell_identity {
);
}
/// improvise-exdg: a coordinate cell is drawn and holds text, so it is not
/// "no cell". `context/test-evidence/command.md`.
#[test]
fn a_coordinate_cell_is_named_rather_than_left_as_no_cell() {
let app = records_app();
let grid = grid_of(&app, WHOLE);
let region = column_named(&app, "Region");
let north = grid.cell(1, region).expect("the second row's coordinate");
assert_eq!(north.text, "North", "setup: a visible coordinate cell");
assert_eq!(
north.id,
Some(CellId::Coordinate),
"a drawn coordinate is named"
);
let nameless: Vec<(usize, usize)> = (0..app.layout.row_count())
.flat_map(|r| (0..app.layout.col_count()).map(move |c| (r, c)))
.filter(|&(r, c)| grid.cell(r, c).is_some_and(|cell| cell.id.is_none()))
.collect();
assert!(
nameless.is_empty(),
"every records cell is something: {nameless:?}"
);
}
/// The version counter is keyed by the surrogate the client holds, so one
/// identity answers ADR 002's question and ADR 001's.
#[test]
+8 -13
View File
@@ -55,6 +55,9 @@ pub enum CellId {
/// A records-mode creating column: typing a **name** into it adds that
/// measure or category and writes no cell at all (gotcha 1, improvise-poia).
Create(CreateKind),
/// A records-mode coordinate. Renaming it moves every measure cell in the
/// row (gotcha 1), which no client composes: `protocol.md` §3 (improvise-exdg).
Coordinate,
}
/// What a creating column creates. The name typed into it is the whole
@@ -194,9 +197,6 @@ pub const MAX_ROW_HEADER_W: u16 = 24;
/// because it is a fact about the *view*, which only the server holds.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct FrameGroup {
/// The category the group belongs to, echoed back in `ToggleGroup` so a
/// client never works out which category a label came from.
pub cat: String,
pub name: String,
pub collapsed: bool,
}
@@ -1137,7 +1137,6 @@ mod tests {
FrameEntry::Item(vec!["A".into(), "x".into()]),
FrameEntry::Item(vec!["A".into(), "y".into()]),
FrameEntry::Group(FrameGroup {
cat: "Type".into(),
name: "Late".into(),
collapsed: true,
}),
@@ -1150,7 +1149,6 @@ mod tests {
/// ungrouped between them — improvise-h0iv's reachable shape.
fn repeated_group_frame() -> AxisFrame {
let q1 = || FrameGroup {
cat: "Month".into(),
name: "Q1".into(),
collapsed: false,
};
@@ -1176,7 +1174,6 @@ mod tests {
cats: vec!["Month".into()],
entries: vec![
FrameEntry::Group(FrameGroup {
cat: "Month".into(),
name: "Q1".into(),
collapsed: false,
}),
@@ -1214,7 +1211,6 @@ mod tests {
cats: vec!["Month".into()],
entries: vec![
FrameEntry::Group(FrameGroup {
cat: "Month".into(),
name: "Q1".into(),
collapsed: true,
}),
@@ -1266,7 +1262,7 @@ mod tests {
/// The pair `FrameGroup` carries cannot separate them — h0iv's own proposed
/// fix. See `context/test-evidence/protocol.md`.
#[test]
fn the_two_headers_are_equal_as_values_so_comparing_cat_and_name_cannot_split_them() {
fn the_two_headers_are_equal_as_values_so_comparing_them_cannot_split_them() {
let frame = repeated_group_frame();
let headers: Vec<&FrameGroup> = frame
.entries
@@ -1277,10 +1273,9 @@ mod tests {
})
.collect();
assert_eq!(
(&headers[0].cat, &headers[0].name),
(&headers[1].cat, &headers[1].name),
"both headers name the same category AND the same group, so the \
ticket's (cat, name) comparison would still merge them"
headers[0], headers[1],
"both headers are the same group in the same state, so no \
comparison of their fields can split them"
);
}
@@ -1344,7 +1339,7 @@ mod tests {
assert_eq!(frame.item(2), Some(&vec!["B".to_string(), "z".to_string()]));
assert!(frame.group_of(0).is_none(), "[A,x] is in no group");
let group = frame.group_of(2).expect("[B,z] sits under Late");
assert_eq!((group.cat.as_str(), group.name.as_str()), ("Type", "Late"));
assert_eq!(group.name, "Late");
assert!(group.collapsed, "the collapse state travels with the group");
}
+7 -2
View File
@@ -756,6 +756,7 @@ fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option<Vec<Command>> {
// Both creating columns are records-only, so the guard above takes
// them; declining rather than `None` keeps that a door, not a hole.
CellId::Create(_) => Some(vec![Command::SetStatus(not_yet("creating a column"))]),
CellId::Coordinate => Some(vec![Command::SetStatus(not_yet("renaming a coordinate"))]),
}
}
@@ -949,7 +950,9 @@ fn advanced(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option
fn record_at_cursor(state: &ClientState) -> Option<crate::RecordHash> {
match cell_at_cursor(state)?.id.as_ref()? {
CellId::Record(cell) => Some(cell.record),
CellId::Aggregate(_) | CellId::Derived { .. } | CellId::Create(_) => None,
CellId::Aggregate(_) | CellId::Derived { .. } | CellId::Create(_) | CellId::Coordinate => {
None
}
}
}
@@ -1016,7 +1019,9 @@ fn model(state: &ClientState, cmd: ModelCommand) -> Option<Command> {
fn record_cell_at_cursor(state: &ClientState) -> Option<RecordCell> {
match cell_at_cursor(state)?.id.as_ref()? {
CellId::Record(cell) => Some(cell.clone()),
CellId::Aggregate(_) | CellId::Derived { .. } | CellId::Create(_) => None,
CellId::Aggregate(_) | CellId::Derived { .. } | CellId::Create(_) | CellId::Coordinate => {
None
}
}
}
+4 -1
View File
@@ -172,7 +172,10 @@ impl Client {
pub fn record_cell_at(&self, row: usize, col: usize) -> Option<RecordCell> {
match self.state.grid.cell(row, col)?.id.clone()? {
CellId::Record(cell) => Some(cell),
CellId::Aggregate(_) | CellId::Derived { .. } | CellId::Create(_) => None,
CellId::Aggregate(_)
| CellId::Derived { .. }
| CellId::Create(_)
| CellId::Coordinate => None,
}
}
+22 -6
View File
@@ -144,7 +144,16 @@ impl RenderCell {
text: cell.text.clone(),
numeric: cell.numeric,
error: cell.error,
editable: matches!(cell.id, Some(CellId::Record(_))),
editable: match cell.id {
Some(CellId::Record(_)) => true,
Some(
CellId::Aggregate(_)
| CellId::Derived { .. }
| CellId::Create(_)
| CellId::Coordinate,
)
| None => false,
},
selected,
edit_text: cell.edit_text.clone().unwrap_or_default(),
}
@@ -563,9 +572,8 @@ mod tests {
/// A 2x3 grid whose cells cover the four id shapes plus a padded slot, so
/// one fixture answers every editability question.
fn group(cat: &str, name: &str, collapsed: bool) -> FrameEntry {
fn group(name: &str, collapsed: bool) -> FrameEntry {
FrameEntry::Group(improvise_protocol::FrameGroup {
cat: cat.into(),
name: name.into(),
collapsed,
})
@@ -583,11 +591,11 @@ mod tests {
first: 0,
cats: vec!["Month".into()],
entries: vec![
group("Month", "Q1", false),
group("Q1", false),
item("Jan"),
group("Fiscal", "Q1", true),
group("Q1", true),
item("Feb"),
group("Month", "Q2", false),
group("Q2", false),
item("Apr"),
],
};
@@ -628,6 +636,10 @@ mod tests {
}),
),
((1, 0), CellDisplay::text("pending")),
(
(1, 2),
CellDisplay::text("North").with_id(CellId::Coordinate),
),
])),
],
});
@@ -659,6 +671,10 @@ mod tests {
!model.rows[1].cells()[1].editable,
"and neither is a slot fit_to_labels padded"
);
assert!(
!model.rows[1].cells()[2].editable,
"renaming a coordinate moves the whole row, which is not a cell write"
);
}
/// The numeric/error flags are the server's answers, echoed — so the test
+29
View File
@@ -187,6 +187,21 @@ test.describe("the standalone deployment", () => {
await expect(page.locator("#grid td.editable").first()).toBeVisible();
});
// improvise-i0v9: "in-page worker · <doc>" was written at 13ms, before the
// worker had answered anything, and read as success over a blank page.
test("the worker is not called up until its first frame has arrived", async ({ page }) => {
const doc = `first-frame-${Date.now()}.improv`;
await page.addInitScript(logStatus);
await page.goto(`/app/index.html?doc=${doc}`);
await expect(page.locator("#grid table")).toBeVisible({ timeout: 20_000 });
await expect(status(page)).toHaveText(`in-page worker · ${doc}`);
const log = await page.evaluate(() => window.__statusLog);
expect(log, `#status said: ${json(log)}`).toEqual([
`starting the worker · ${doc}…`,
`in-page worker · ${doc}`,
]);
});
// improvise-9cma: a `?doc=` this backend cannot hold used to reach the
// generic watchdog 8s later. The REASON now crosses the port (improvise-1wft).
test("an invalid ?doc= is named on screen, and not blamed on the browser", async ({
@@ -235,6 +250,7 @@ test.describe("the standalone deployment", () => {
const second = await context.newPage();
await second.clock.install();
await second.addInitScript(logStatus);
await second.goto(`/app/index.html?doc=${doc}`);
// It PAINTS. That is the whole point: a locked document is read-only, not
@@ -242,6 +258,15 @@ test.describe("the standalone deployment", () => {
await expect(second.locator("#grid table")).toBeVisible({ timeout: 20_000 });
await expect(second.locator("#status")).toContainText("read-only");
await expect(second.locator("#status")).toContainText("another tab");
// The page's clock has not been walked, so a reason that arrived AFTER the
// watchdog would have been logged behind "did not answer". Nor was success
// ever claimed: the Snapshot that follows the notice must not overwrite it.
const early = await second.evaluate(() => window.__statusLog);
expect(early.at(-1), `#status said: ${json(early)}`).toContain("read-only");
expect(
early.filter((t) => t.includes("did not answer") || t === `in-page worker · ${doc}`),
`#status said: ${json(early)}`,
).toEqual([]);
// And the reason STANDS: it is not replaced 8s later by a watchdog
// blaming the browser for something the code knew precisely.
@@ -252,6 +277,10 @@ test.describe("the standalone deployment", () => {
await second.clock.runFor(HANDSHAKE_TIMEOUT_MS + 2_000);
await expect(second.locator("#status")).toContainText("read-only");
await expect(second.locator("#status")).not.toContainText("did not answer");
const late = await second.evaluate(() => window.__statusLog);
expect(late.some((t) => t.includes("did not answer")), `#status said: ${json(late)}`).toBe(
false,
);
await second.close();
await first.close();
+3 -1
View File
@@ -181,7 +181,8 @@ function workerTransport() {
shell.fail(`worker failed to start: ${event.message ?? "see console"}`);
console.error("improvise: worker error", event);
});
shell.setShell(`in-page worker · ${DOC}`);
// Not yet a claim that it works: the first frame makes that (improvise-i0v9).
shell.setShell(`starting the worker · ${DOC}…`);
// Frames posted before the worker has finished `start()` are held by
// worker.js in arrival order, so the handshake can go out immediately.
return { send: (text) => worker.postMessage(text), ready: Promise.resolve() };
@@ -797,6 +798,7 @@ shell.arm(
WS_URL
? `${WS_URL} never answered — is improvise-ws-server running, and did it send a document?`
: "the in-page worker did not answer (see console)",
{ answer: WS_URL ? null : `in-page worker · ${DOC}` },
);
await transport.ready;
sendViewport(); // the handshake: the snapshot is built at this extent
+13 -7
View File
@@ -339,7 +339,12 @@ export function createShell({
clearTimer = clearTimeout,
}) {
let watchdog = null;
let answered = null;
let arms = 0;
const disarm = () => {
clearTimer(watchdog);
watchdog = null;
};
return {
setShell(text) {
@@ -350,24 +355,25 @@ export function createShell({
},
/** A peer that never answers the handshake. Deliberately about the *first
* frame*, not either peer's own failure modes — clients.md. */
arm(message) {
arm(message, { answer = null } = {}) {
arms += 1;
clearTimer(watchdog);
answered = answer;
disarm();
watchdog = setTimer(() => {
watchdog = null;
this.setShell(message);
}, timeoutMs);
},
/** A frame arrived, so the peer answered: the watchdog has no question
* left to ask. */
/** A frame arrived, so the peer answered. Only the FIRST, and only if no
* failure spoke first, may say so (improvise-i0v9, clients.md). */
sawFrame() {
clearTimer(watchdog);
watchdog = null;
if (watchdog !== null && answered !== null) this.setShell(answered);
disarm();
},
/** A peer that failed *loudly* has already said something more precise, so
* the watchdog must be DISARMED and not merely pre-empted (PR #13). */
fail(text) {
this.sawFrame();
disarm();
this.setShell(text);
},
armed: () => watchdog !== null,
+37
View File
@@ -351,6 +351,43 @@ test("a notice DISARMS the watchdog rather than being replaced by it", () => {
);
});
// improvise-i0v9: "in-page worker · <doc>" was written before the worker had
// answered anything, and read as success over a blank page for eight seconds.
test("only the first frame claims the peer answered, and not before it", () => {
const clock = fakeClock();
const seen = [];
const shell = createShell({
onShell: (t) => seen.push(t),
timeoutMs: 8000,
setTimer: clock.setTimer,
clearTimer: clock.clearTimer,
});
shell.arm("did not answer", { answer: "in-page worker · a.improv" });
assert.deepEqual(seen, [], "the claim was made before any frame arrived");
shell.sawFrame();
shell.sawFrame();
assert.deepEqual(seen, ["in-page worker · a.improv"]);
});
// The order worker.js really produces on a locked document: the ReadOnly
// notice is posted after `start()` and BEFORE the Snapshot answers the handshake.
test("a reason given before the first frame is not replaced by the frame's claim", () => {
const clock = fakeClock();
const seen = [];
const shell = createShell({
onShell: (t) => seen.push(t),
timeoutMs: 8000,
setTimer: clock.setTimer,
clearTimer: clock.clearTimer,
});
shell.arm("did not answer", { answer: "in-page worker · a.improv" });
shell.fail(noticeText({ kind: "ReadOnly", document: "a.improv", detail: "" }));
shell.sawFrame();
clock.fire();
assert.equal(seen.length, 1, `#status said: ${JSON.stringify(seen)}`);
assert.match(seen[0], /read-only/);
});
// The notice's remembered state. The value crosses `localStorage`, which is
// allowed to forget and allowed to throw, so the interesting inputs are the
// ones that are NOT either spelling.