natsukium/felis

Watch

transcode::body_to_json renders RowDelta rows as a byte array, not structural JSON #193

New issue

Closed

opened 2026-09-07 20:43:30 +09:00 by natsukium · 1 comment

natsukium commented

2026-09-07 20:43:30 +09:00

Owner

Summary

felis_grid::transcode::body_to_json is documented as the shared engine that
re-exposes a daemon session "as JSON" for the satellite consumers
(felis-web-gateway, felis-fcast serve, the CLI's stdio bridge). For a
GridMsg::RowDelta it no longer produces readable JSON: the row lands as a
JSON array of byte values, not as the structural row object the module
doc and its tests describe.

The bytes are correct and round-trip cleanly — this is a fidelity, size, and
readability regression in the JSON view, not a corruption bug.

Where it comes from

body_to_json recodes the rows and then serializes the whole message
(crates/felis-grid/src/transcode.rs:40-46):

MessageKind::Grid => {
    let mut msg: GridMsg = codec::decode(body)?;
    rows_to_json(&mut msg)?;
    Ok(serde_json::to_value(msg)?)
}

rows_to_json fills each RowPayload with encode_row_json output, which is
JSON text as bytes (crates/felis-grid/src/wire.rs:642, serde_json::to_vec).
serde_json::to_value then serializes RowPayload, and since
crates/felis-protocol/src/row.rs declares

#[derive(… Serialize, Deserialize)]
#[serde(transparent)]
pub struct RowPayload(pub Vec<u8>);

the JSON text is re-encoded as [123, 34, 82, …].

Before the wire-baseline reset, RowPayload's serde impl was encoding-aware:
under a human-readable serializer it spliced the row text inline as a real
JSON value via serde_json::value::RawValue, so the row read as
{"Rle":{…}}. That branch's docs flagged its own future as an open question
("Whether this branch keeps earning its place now that no live wire path
drives it is an open decision"). It was dropped — but body_to_json's
human-readable consumers were exactly the path that drove it, so the JSON
view lost its readability along with it.

Reproduction

Any consumer of the JSON view shows it. Using felis-fcast, whose .fcast
frames are serde_json::to_value of the same GridMsg values (felis-fcast on
follow-felis-105b0899, felis pinned at 105b089979369fd310757375af3df7900ae7e523):

$ cargo run -p felis-fcast --example gen_hello_fcast > hello.fcast
$ grep -o '\[400000,"Grid",{"RowDelta".\{0,70\}' hello.fcast
[400000,"Grid",{"RowDelta":{"rows":[[0,[123,34,82,108,101,34,58,123,34,103,114

Decoding that byte array as UTF-8 gives what the JSON view is supposed to
show directly:

$ tail -n +2 hello.fcast \
    | jq -r 'select(.[2] | type == "object" and has("RowDelta"))
             | .[2].RowDelta.rows[][1] | implode' | head -1 | cut -c1-80
{"Rle":{"graphemes":[{"Ascii":102},{"Ascii":101},{"Ascii":108},{"Ascii":105},{"

Size cost for that one 48-column row: 3271 chars as stored, 948 chars as the
text it encodes — 3.5x.

Why the tests do not catch it

transcode::tests::row_delta_payload_survives_json_protobuf_json
(crates/felis-grid/src/transcode.rs:195) asserts
body_to_json(json_to_body(json)) == json and comments "row payload must come
back as structural JSON". Round-trip equality holds for the byte-array form,
so the test passes while the property its name claims does not hold. Nothing
asserts the shape of the produced value.

Relatedly, row::tests::the_serde_form_matches_a_plain_byte_vec
(crates/felis-protocol/src/row.rs) carries the comment "so stored .fcast
files and the JSON row view are unaffected by the newtype" — but it exercises
postcard (binary) only. Both are affected; the comment reasons about the
branch the test covers and generalizes past it.

Suggested direction

Not prescribing a fix, but the options as they look from downstream:

Restore the human-readable splice on RowPayload (RawValue under
is_human_readable()), which fixes every JSON consumer at once and needs
no downstream change.
Have body_to_json splice the row itself after rows_to_json, keeping
RowPayload transparent.
Accept the byte array as the contract, and correct the module doc, the two
test comments, and the "structural JSON" language in
docs/explanation/architecture/ipc.md:405 to match.

Option 3 is a real choice — but the doc and test comments should not keep
describing option 1's behavior either way.

Why this is out of scope for the felis-fcast follow-up

felis-fcast cannot fix it downstream. Its format rule is that a frame's msg
is the verbatim serde-JSON of a felis-protocol enum and that repo never
defines the msg schema; splicing the row inline there would fork the schema,
so a producer writing the spliced form and a consumer reading the transparent
one would disagree about a file both call valid. That PR documents the current
shape and ships a jq recipe to decode a row; if felis restores the readable
form, recordings pick it up with no format change.

## Summary `felis_grid::transcode::body_to_json` is documented as the shared engine that re-exposes a daemon session "as JSON" for the satellite consumers (`felis-web-gateway`, `felis-fcast serve`, the CLI's stdio bridge). For a `GridMsg::RowDelta` it no longer produces readable JSON: the row lands as a JSON **array of byte values**, not as the structural row object the module doc and its tests describe. The bytes are correct and round-trip cleanly — this is a fidelity, size, and readability regression in the JSON view, not a corruption bug. ## Where it comes from `body_to_json` recodes the rows and then serializes the whole message (`crates/felis-grid/src/transcode.rs:40-46`): ```rust MessageKind::Grid => { let mut msg: GridMsg = codec::decode(body)?; rows_to_json(&mut msg)?; Ok(serde_json::to_value(msg)?) } ``` `rows_to_json` fills each `RowPayload` with `encode_row_json` output, which is JSON **text as bytes** (`crates/felis-grid/src/wire.rs:642`, `serde_json::to_vec`). `serde_json::to_value` then serializes `RowPayload`, and since `crates/felis-protocol/src/row.rs` declares ```rust #[derive(… Serialize, Deserialize)] #[serde(transparent)] pub struct RowPayload(pub Vec<u8>); ``` the JSON text is re-encoded as `[123, 34, 82, …]`. Before the wire-baseline reset, `RowPayload`'s serde impl was encoding-aware: under a human-readable serializer it spliced the row text inline as a real JSON value via `serde_json::value::RawValue`, so the row read as `{"Rle":{…}}`. That branch's docs flagged its own future as an open question ("Whether this branch keeps earning its place now that no live wire path drives it is an open decision"). It was dropped — but `body_to_json`'s human-readable consumers were exactly the path that drove it, so the JSON view lost its readability along with it. ## Reproduction Any consumer of the JSON view shows it. Using felis-fcast, whose `.fcast` frames are `serde_json::to_value` of the same `GridMsg` values (felis-fcast on `follow-felis-105b0899`, felis pinned at `105b089979369fd310757375af3df7900ae7e523`): ```console $ cargo run -p felis-fcast --example gen_hello_fcast > hello.fcast $ grep -o '\[400000,"Grid",{"RowDelta".\{0,70\}' hello.fcast [400000,"Grid",{"RowDelta":{"rows":[[0,[123,34,82,108,101,34,58,123,34,103,114 ``` Decoding that byte array as UTF-8 gives what the JSON view is supposed to show directly: ```console $ tail -n +2 hello.fcast \ | jq -r 'select(.[2] | type == "object" and has("RowDelta")) | .[2].RowDelta.rows[][1] | implode' | head -1 | cut -c1-80 {"Rle":{"graphemes":[{"Ascii":102},{"Ascii":101},{"Ascii":108},{"Ascii":105},{" ``` Size cost for that one 48-column row: 3271 chars as stored, 948 chars as the text it encodes — **3.5x**. ## Why the tests do not catch it `transcode::tests::row_delta_payload_survives_json_protobuf_json` (`crates/felis-grid/src/transcode.rs:195`) asserts `body_to_json(json_to_body(json)) == json` and comments "row payload must come back as structural JSON". Round-trip equality holds for the byte-array form, so the test passes while the property its name claims does not hold. Nothing asserts the *shape* of the produced value. Relatedly, `row::tests::the_serde_form_matches_a_plain_byte_vec` (`crates/felis-protocol/src/row.rs`) carries the comment "so stored `.fcast` files and the JSON row view are unaffected by the newtype" — but it exercises postcard (binary) only. Both are affected; the comment reasons about the branch the test covers and generalizes past it. ## Suggested direction Not prescribing a fix, but the options as they look from downstream: 1. Restore the human-readable splice on `RowPayload` (`RawValue` under `is_human_readable()`), which fixes every JSON consumer at once and needs no downstream change. 2. Have `body_to_json` splice the row itself after `rows_to_json`, keeping `RowPayload` transparent. 3. Accept the byte array as the contract, and correct the module doc, the two test comments, and the "structural JSON" language in `docs/explanation/architecture/ipc.md:405` to match. Option 3 is a real choice — but the doc and test comments should not keep describing option 1's behavior either way. ## Why this is out of scope for the felis-fcast follow-up felis-fcast cannot fix it downstream. Its format rule is that a frame's `msg` is the verbatim serde-JSON of a `felis-protocol` enum and that repo never defines the `msg` schema; splicing the row inline there would fork the schema, so a producer writing the spliced form and a consumer reading the transparent one would disagree about a file both call valid. That PR documents the current shape and ships a jq recipe to decode a row; if felis restores the readable form, recordings pick it up with no format change.

natsukium referenced this issue from natsukium/felis-fcast

2026-09-07 20:44:08 +09:00

Follow felis to 105b0899: wire-baseline reset, .fcast envelope v2 #1

natsukium referenced this issue from natsukium/felis-web-component

2026-09-07 21:02:39 +09:00

deps: follow felis to 105b0899 (wire-baseline reset) #3

natsukium commented

2026-09-07 21:21:38 +09:00

Author

Owner

Triage plan (2026-09-07)

Verdict: accepted, priority/P2 by the rule (no wire byte and no CLI output contract changes: the bridge never emits a Grid frame, capture_row_json transcodes Region::Row which carries no RowPayload) — but landing before the tag, because every .fcast recorded against v0.1.0 would otherwise carry the byte-array shape and json_to_body plus every satellite reader would have to dual-accept two row shapes forever. Fixing now costs the satellites nothing (felis-fcast's rule is verbatim serde-JSON of the felis enum).

Verified: RowPayload is #[serde(transparent)] Vec<u8> (crates/felis-protocol/src/row.rs), serde_json has no bytes fast path, and body_to_json's Grid arm (crates/felis-grid/src/transcode.rs) fills the payload with encode_row_json bytes and then to_values the message, so the JSON text is re-encoded as [123,34,…]. 85cc1bad removed the is_human_readable() splice on the argument that no live path reached it; body_to_json (added later) is exactly such a path. The existing round-trip test builds its expected value with the same to_value, so it asserts the broken shape.

Approach (option 2 — splice in felis-grid::transcode, RowPayload stays transparent):

body_to_json: after rows_to_json + to_value, walk value["RowDelta"]["rows"][i][1] and replace each byte array with serde_json::from_slice::<Value>; error (do not skip) if the path is missing so a GridMsg rename fails loudly. json_to_body: inverse (to_vec into RowPayload before rows_to_wire); reject a byte-array input (no legacy recordings exist pre-tag).
Do not reintroduce a serializer-dependent serde impl in felis-protocol (undoes 85cc1bad's valid point); do not change the docs to bless the byte array (contradicts ipc.md, overview.md, and the module's reason to exist).
Tests: rewrite row_delta_payload_survives_json_protobuf_json with a non-empty row and assert shape (["RowDelta"]["rows"][0][1]["Rle"]["graphemes"] is an array, no Number elements at that position) plus round-trip equality. Drop "and the JSON row view" from the row.rs test comment (that test is postcard-only).
Cascade: CHANGELOG.md entry (.fcast-affecting, precedent exists); ipc.md/overview.md already describe the fixed behavior. No schema, no skills/felis change. Size ≈ +45/−10.

Downstream: the felis-fcast follow PR documents the byte-array shape with a jq implode recipe; that becomes stale when this lands — coordinate the felis bump there.

## Triage plan (2026-09-07) **Verdict:** accepted, `priority/P2` by the rule (no wire byte and no CLI output contract changes: the bridge never emits a `Grid` frame, `capture_row_json` transcodes `Region::Row` which carries no `RowPayload`) — but landing **before the tag**, because every `.fcast` recorded against v0.1.0 would otherwise carry the byte-array shape and `json_to_body` plus every satellite reader would have to dual-accept two row shapes forever. Fixing now costs the satellites nothing (felis-fcast's rule is verbatim serde-JSON of the felis enum). **Verified:** `RowPayload` is `#[serde(transparent)] Vec<u8>` (`crates/felis-protocol/src/row.rs`), serde_json has no bytes fast path, and `body_to_json`'s Grid arm (`crates/felis-grid/src/transcode.rs`) fills the payload with `encode_row_json` bytes and then `to_value`s the message, so the JSON text is re-encoded as `[123,34,…]`. `85cc1bad` removed the `is_human_readable()` splice on the argument that no live path reached it; `body_to_json` (added later) is exactly such a path. The existing round-trip test builds its expected value with the same `to_value`, so it asserts the broken shape. **Approach (option 2 — splice in `felis-grid::transcode`, `RowPayload` stays transparent):** - `body_to_json`: after `rows_to_json` + `to_value`, walk `value["RowDelta"]["rows"][i][1]` and replace each byte array with `serde_json::from_slice::<Value>`; error (do not skip) if the path is missing so a `GridMsg` rename fails loudly. `json_to_body`: inverse (`to_vec` into `RowPayload` before `rows_to_wire`); reject a byte-array input (no legacy recordings exist pre-tag). - Do not reintroduce a serializer-dependent serde impl in `felis-protocol` (undoes `85cc1bad`'s valid point); do not change the docs to bless the byte array (contradicts `ipc.md`, `overview.md`, and the module's reason to exist). - Tests: rewrite `row_delta_payload_survives_json_protobuf_json` with a non-empty row and assert shape (`["RowDelta"]["rows"][0][1]["Rle"]["graphemes"]` is an array, no `Number` elements at that position) plus round-trip equality. Drop "and the JSON row view" from the `row.rs` test comment (that test is postcard-only). - Cascade: `CHANGELOG.md` entry (`.fcast`-affecting, precedent exists); `ipc.md`/`overview.md` already describe the fixed behavior. No schema, no `skills/felis` change. Size ≈ +45/−10. **Downstream:** the felis-fcast follow PR documents the byte-array shape with a jq `implode` recipe; that becomes stale when this lands — coordinate the felis bump there.

natsukium added the

priority/P2

status/planned

labels

2026-09-07 21:21:39 +09:00

natsukium referenced this issue from a commit

2026-09-07 23:34:48 +09:00

grid: splice the row object into the JSON frame view

natsukium referenced this issue from a pull request that will close it,

2026-09-07 23:35:05 +09:00

grid: splice the row object into the JSON frame view #208

natsukium closed this issue