fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(wasm-client): one answer to what a typed string means

Browse Source 
`Client::set_cell_at` and `socket.rs::typed` gave opposite answers to the
same question, one level below the envelopes improvise-xks6 had just
consolidated. `set_cell_at`'s doc said classifying a typed string "is the
host's decision, not one to re-implement here" and took a serialized
`CellValue`; `typed`'s said the opposite and `typed_cell` takes a plain
string. Both composed the same `ClientFrame`, and they stayed apart only
because nothing called both.

The `set_cell_at` claim is false on every deployment. `ModelCommand::SetCell`
carries a `CellValue` on the wire, so no host ever sees the string. The
ticket's escape hatch -- that improvise-9x6's `App` in the browser might make
the pre-classified form right for the worker -- does not exist: that `App`
sits behind the frame boundary and is handed a value, not a string.

So `set_cell_at` and the `set_cell` it wrapped are deleted, not
re-documented. `set_cell` could not express `ClearCell`, which makes it a
strictly weaker duplicate of `typed_cell_frame` rather than a peer, and
`the_json_wrapper_composes_the_same_message` was precisely the compatibility
test doctrine says a finished consolidation does not keep.

Evidence: 1189 -> 1184. The five deleted tests are the two wrapper tests
(aimed at deleted code) and three None-shape tests already asserted against
`typed_cell_frame` in socket.rs. The fourth None shape -- ADR 001's absent
basis -- had no twin, so `a_client_with_no_basis_composes_nothing` was
retargeted rather than dropped, and proved discriminating by injecting a
published `Projection::Basis` into its fixture and watching it flip to Some.

Three doc comments elsewhere named the deleted function (keys.rs, state.rs,
and socket.rs's own None-shape test); all three re-point, per gotcha 48's
rule that changing a stated fact is a grep rather than an edit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Edward Langley
2026-09-19 22:36:30 -07:00
parent 97ea48957e
commit 18189c1dee
4 changed files with 46 additions and 167 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/improvise-wasm-client/src/keys.rs
+1 -1
View File
@@ -1080,7 +1080,7 @@ mod tests {
/// `t` is a document mutation, so it carries a basis — and until one has
/// been published there is nothing truthful to claim, which is the
/// fourth `None` shape `Client::set_cell` already has (ADR 001).
/// fourth `None` shape `Client::typed_cell_frame` already has (ADR 001).
#[test]
fn transpose_needs_a_published_basis() {
let state = in_mode(AppMode::Normal);
crates/improvise-wasm-client/src/lib.rs
+8 -159
View File
@@ -61,9 +61,7 @@ pub use keys::{BrowserKey, KeyOutcome, resolve};
pub use render::{RenderCatRow, RenderCell, RenderModel, RenderRow};
pub use state::ClientState;
use improvise_protocol::{
CellId, CellValue, ClientFrame, Command, ModelCommand, RecordCell, Snapshot,
};
use improvise_protocol::{CellId, ClientFrame, RecordCell, Snapshot};
use wasm_bindgen::prelude::*;
/// The handle a JS bootstrap holds for the lifetime of the tab.
@@ -131,20 +129,6 @@ impl Client {
self.state.revision
}
/// Compose a write to the cell at a viewport position.
///
/// `value_json` is a serialized `CellValue`; everything crossing this
/// boundary is JSON, and *what* a typed string means — a number or text
/// — is the host's decision, not one to re-implement here. Returns the
/// serialized [`ClientFrame`], or `None` when the position holds
/// nothing this client may write to.
#[wasm_bindgen]
pub fn set_cell_at(&self, row: usize, col: usize, value_json: &str) -> Option<String> {
let value: CellValue = serde_json::from_str(value_json).ok()?;
let message = self.set_cell(row, col, value)?;
serde_json::to_string(&message).ok()
}
// ── the transport surface (improvise-cr3) ───────────────────────────
//
// Three string calls, and they are the whole harness a page needs: send
@@ -254,42 +238,6 @@ impl Client {
self.state.bootstrap(snapshot);
}
/// Compose a write to the cell at a viewport position, addressed by the
/// surrogate the server projected there (ADR 002).
///
/// This is the composition `improvise-q08` found a thin client could not
/// do. It holds no `Model`, so it cannot build a `CellKey` — but it does
/// not need to: the id on the cell it is already drawing *is* the
/// address, and it is echoed back unread.
///
/// `None` on three shapes, and they are three different facts the cache
/// already carries: the position is outside the grid, the position holds
/// no settable cell (`id: None` — padding, or a records-mode column that
/// renames a coordinate rather than writing one), or the cell is an
/// **aggregate**, which `SetCell` cannot be composed for because
/// [`ModelCommand::SetCell`] does not take that kind of id. The last is
/// the one worth noting: it is not a check this function performs, it is
/// the pattern match failing to bind.
///
/// Unlike [`Client::key`]'s locally-resolved commands, nothing is folded
/// here. The new value comes back as a `GridUpdate::Cells` from the host,
/// which is the side that formats it; guessing it locally is how the two
/// ends drift apart.
/// A **fourth** shape returns `None`, and it is ADR 001's: this client
/// has not yet been told the document's basis, so it has nothing
/// truthful to say about what it composed against. `BasisCache::claim`
/// is the only constructor there is, and it withholds rather than
/// invents — a forged basis would be the check deleted with extra steps.
/// In practice a client that has folded its bootstrap always has one.
pub fn set_cell(&self, row: usize, col: usize, value: CellValue) -> Option<ClientFrame> {
let cell = self.record_cell_at(row, col)?;
let basis = self.state.basis.claim([cell.record])?;
Some(ClientFrame::command(Command::Model {
basis,
cmd: ModelCommand::SetCell { cell, value },
}))
}
/// Classify a keypress, folding it locally when this client can answer it.
pub fn key(&mut self, browser: &BrowserKey) -> Option<ClientFrame> {
let event = browser.to_key_event()?;
@@ -321,62 +269,17 @@ mod tests {
serde_json::from_str(&format!("\"{hex}\"")).expect("a surrogate is hex")
}
fn client_showing(id: Option<CellId>) -> Client {
let mut cell = improvise_protocol::CellDisplay::number("1200");
cell.id = id;
let mut client = init();
client.bootstrap(Snapshot {
view: ViewState::default(),
seed: vec![
Projection::Grid(GridUpdate::RowLabels(vec!["North".into()])),
Projection::Grid(GridUpdate::ColLabels(vec!["Revenue".into()])),
Projection::Grid(GridUpdate::Cells(vec![((0, 0), cell)])),
Projection::Basis(published_basis()),
],
});
client
}
/// What a server publishes for the one record this fixture's grid names
/// (ADR 001). A client that was never sent one composes nothing at all,
/// which `a_client_with_no_basis_composes_nothing` pins.
fn published_basis() -> improvise_protocol::Basis {
improvise_protocol::basis::published(
0,
0,
[(surrogate("fe4c8e5e9cbd578e0d388f4b2b5a7879"), 0)]
.into_iter()
.collect(),
)
}
/// The composition improvise-q08 found a thin client could not do. The
/// id goes back out **unchanged**: it is a token to echo, so the
/// assertion is equality with what arrived, not a re-derivation.
#[test]
fn a_client_composes_a_write_from_the_id_it_was_given() {
let id = improvise_protocol::RecordCell {
record: surrogate("fe4c8e5e9cbd578e0d388f4b2b5a7879"),
measure: Some("Revenue".into()),
};
let client = client_showing(Some(CellId::Record(id.clone())));
assert_eq!(
client.set_cell(0, 0, CellValue::Number(42.0)),
Some(ClientFrame::command(Command::Model {
basis: published_basis(),
cmd: ModelCommand::SetCell {
cell: id,
value: CellValue::Number(42.0),
},
}))
);
}
/// The fourth `None`, and the one ADR 001 adds: a client that has not
/// been told the document's basis has nothing truthful to say about what
/// it composed against, so it composes nothing. That is the property
/// that makes the basis check unforgeable from a browser —
/// `BasisCache::claim` is the only constructor, and it withholds.
///
/// The other three `None` shapes — an aggregate, `id: None`, and a
/// position outside the grid — are pinned against the same composition in
/// `socket.rs`'s `typing_into_a_cell_that_is_not_a_record_composes_nothing`.
/// They were asserted twice, once against each of two compositions, until
/// improvise-r4lo left only one.
#[test]
fn a_client_with_no_basis_composes_nothing() {
let id = improvise_protocol::RecordCell {
@@ -394,61 +297,7 @@ mod tests {
Projection::Grid(GridUpdate::Cells(vec![((0, 0), cell)])),
],
});
assert_eq!(client.set_cell(0, 0, CellValue::Number(42.0)), None);
}
/// The point of the enum: an aggregate carries an identity, and it is
/// still not one `SetCell` accepts. Nothing here checks anything — the
/// `let CellId::Record(..) = ... else` simply does not bind.
#[test]
fn a_client_cannot_compose_a_write_for_an_aggregate() {
let hash: improvise_protocol::AggregateHash =
serde_json::from_str("\"5b05bbcd2364099f68a49dbf9099e075\"").expect("hex");
let client = client_showing(Some(CellId::Aggregate(hash)));
assert_eq!(client.set_cell(0, 0, CellValue::Number(42.0)), None);
}
/// `id: None` is not "aggregate" — it is "no projected cell here", and a
/// position holding one is not writable either.
#[test]
fn a_cell_the_server_named_nothing_for_is_not_settable() {
let client = client_showing(None);
assert_eq!(client.set_cell(0, 0, CellValue::Number(42.0)), None);
assert_eq!(
client.set_cell(9, 9, CellValue::Number(42.0)),
None,
"and neither is a position outside the grid"
);
}
/// The JS-facing wrapper is the same path with JSON at both ends, so a
/// browser needs no structural bindings.
#[test]
fn the_json_wrapper_composes_the_same_message() {
let id = improvise_protocol::RecordCell {
record: surrogate("fe4c8e5e9cbd578e0d388f4b2b5a7879"),
measure: Some("Revenue".into()),
};
let client = client_showing(Some(CellId::Record(id)));
let json = client
.set_cell_at(0, 0, "{\"Number\":7.5}")
.expect("the wrapper must compose");
assert_eq!(
serde_json::from_str::<ClientFrame>(&json).expect("round trips"),
client.set_cell(0, 0, CellValue::Number(7.5)).unwrap()
);
}
/// A value that is not a `CellValue` composes nothing rather than
/// panicking across the wasm boundary.
#[test]
fn the_json_wrapper_refuses_a_value_it_cannot_read() {
let id = improvise_protocol::RecordCell {
record: surrogate("fe4c8e5e9cbd578e0d388f4b2b5a7879"),
measure: None,
};
let client = client_showing(Some(CellId::Record(id)));
assert_eq!(client.set_cell_at(0, 0, "not json"), None);
assert_eq!(client.typed_cell_frame(0, 0, "42"), None);
}
/// The end-to-end shape a JS harness drives: bootstrap, key, message.
crates/improvise-wasm-client/src/socket.rs
+36 -6
View File
@@ -65,6 +65,11 @@ fn rejection_message(rejection: &Rejection) -> String {
/// Rust rather than in the page's JavaScript is what makes it testable and
/// keeps one spelling of the rule per language boundary instead of two.
///
/// This is the **only** answer the client gives to "what does a typed string
/// mean" (improvise-r4lo). It holds on every deployment, including the worker:
/// a host's `App` is behind the frame boundary and receives a `CellValue`, so
/// no host anywhere ever sees the string to classify it.
///
/// `None` means "clear this cell", which is a different command, not a
/// different value — hence the `Option` rather than a `CellValue::Text("")`.
fn typed(text: &str) -> Option<CellValue> {
@@ -137,12 +142,37 @@ impl Client {
}
/// Compose the write a user made by typing `text` into the cell at a
/// viewport position.
/// viewport position — the **one** composition of a cell write there is
/// (improvise-r4lo).
///
/// `None` on exactly the shapes [`Client::set_cell`] refuses — outside
/// the grid, no projected cell, an aggregate, which `SetCell` cannot be
/// composed for because it does not take that kind of id (ADR 002), or
/// no basis yet (ADR 001).
/// This is the composition `improvise-q08` found a thin client could not
/// do. It holds no `Model`, so it cannot build a `CellKey` — but it does
/// not need to: the id on the cell it is already drawing *is* the
/// address, and it is echoed back unread (ADR 002).
///
/// It takes the user's **string**, not a `CellValue`, and [`typed`] above
/// says why: the classification is unavoidably this side of the wire,
/// because `ModelCommand::SetCell` already carries a `CellValue` by the
/// time any host sees it. That is true of the worker deployment too — the
/// `App` there sits *behind* the frame boundary and is handed a value, not
/// a string — so there is no deployment for which a pre-classified entry
/// point would be the right one. A sibling taking a serialized `CellValue`
/// used to sit beside this and claim the opposite; it was deleted rather
/// than re-documented, because it could not express `ClearCell` and was
/// therefore a strictly weaker duplicate of this.
///
/// `None` on four shapes, each a different fact the client already holds:
/// the position is outside the grid; the position holds no settable cell
/// (`id: None` — padding, or a records-mode column that renames a
/// coordinate rather than writing one); the cell is an **aggregate**,
/// which is the pattern failing to bind rather than a check (ADR 002);
/// or this client has not been told the document's basis and so has
/// nothing truthful to say about what it composed against (ADR 001 —
/// `BasisCache::claim` withholds rather than invents).
///
/// Nothing is folded here. The new value comes back as a
/// `GridUpdate::Cells` from the host, which is the side that formats it;
/// guessing it locally is how the two ends drift apart.
pub fn typed_cell_frame(&self, row: usize, col: usize, text: &str) -> Option<ClientFrame> {
let cell = self.record_cell_at(row, col)?;
let basis = self.state().basis.claim([cell.record])?;
@@ -366,7 +396,7 @@ mod tests {
}
/// Typing into a cell the client may not write is no frame at all, for
/// the same reason `set_cell` composes nothing: the id is of the wrong
/// the same reason any write composes nothing: the id is of the wrong
/// kind, or there is no id. `Drill` is not on the wire yet, so an
/// aggregate is simply not editable here.
#[test]
crates/improvise-wasm-client/src/state.rs
+1 -1
View File
@@ -36,7 +36,7 @@ pub struct ClientState {
pub tiles: TileBarCache,
/// The picture of the document this client may claim to have composed
/// against (ADR 001). Not drawn; folded like any other projection, and
/// read by `Client::set_cell` when it composes a write.
/// read by `Client::typed_cell_frame` when it composes a write.
#[serde(skip)]
pub basis: BasisCache,
/// The last basis rejection this client was told about, if any.