feat(protocol): make image transfers canonical and stateful #81

Merged

natsukium merged 2 commits from feat/image-transfer-state-machine-51 into main 2026-09-04 19:39:01 +09:00

Conversation 0 Commits 2 Files changed 25 +1102 -840

natsukium commented 2026-09-04 19:27:50 +09:00

Owner

Copy link

Fixes #51.

The image wire carried two encodings of the root frame with different meanings — frame: None replaced the image and dropped its animation frames, frame: Some(0) edited the root's pixels in place — and both were reachable from real Kitty traffic (a=t versus a=f, r=1). A frame header also repeated width/height/format the receiver ignored, chunks claimed a random access nothing verified, and Complete marked a buffer renderable whatever had arrived, painting a dropped chunk as transparent pixels the producer never sent.

What changed

• The header names one target: New { width, height, format } starts a fresh image, Frame { number } addresses one frame in Kitty's own 1-based numbering (ShowFrame counts the same way), so the two meanings have two shapes and frame 0 is not expressible.
• total_bytes and a chunk's offset are gone: every buffer of an image is width × height × bytes_per_pixel, and the receiver runs one transfer at a time, appending chunks at its own byte count.
• ImageShadow::apply returns a typed error for every sequence the machine has no transition for, which the window turns into the same connection-local teardown a refused claim already got.
• The proto break (deleted ImageHeader.frame) is acknowledged in crates/felis-protocol/proto/BREAKING.md; it rides the current pre-release major.

Doc cascade

docs/reference/ipc.md and docs/reference/protocols/kitty-graphics.md record the resulting facts (one target per meaning, ordered chunks, one transfer at a time, the malformed list); docs/explanation/architecture/ipc.md and docs/explanation/protocols/kitty-graphics.md own the argument, including the two rejected shapes — offset-addressed chunks and a 0-based optional frame index — and what would make either worth revisiting. docs/explanation/data-model/image-store.md, docs/reference/testing.md, and CHANGELOG.md follow.

Verified: just check green (fmt, clippy, nextest, deny); reviewed by plan/correctness/docs lenses and pi sol

Deferred

• Debug assertion in materialize_image_events that no Header is queued while a prior transfer's Complete is pending — the plan calls it a cheap future-proofing catch, but no acceptance criterion asks for a producer-side guard and the receiver already refuses the sequence.
• ImageShadowError::DimensionsOverCap as a distinct variant — #49 landed already; its caps surface as the existing ImageBytes/SessionBytes/FrameIndex variants, so this would be a rename with no new behavior.
• A felis-daemon integration test shipping a two-frame image through the codec onto a live ImageShadow — the existing image_outbox_materialization.rs suite already round-trips producer output through the shadow and passes unchanged; a second end-to-end fixture would duplicate it without covering a criterion the shadow unit tests miss.
• Reworking docs/explanation/architecture/ipc.md's older "let the client trust the daemon" paragraph beyond the one sentence that named chunk offsets — its argument is untouched by this change; a fuller rewrite is prose work no criterion needs.

Fixes #51. The image wire carried two encodings of the root frame with different meanings — `frame: None` replaced the image and dropped its animation frames, `frame: Some(0)` edited the root's pixels in place — and both were reachable from real Kitty traffic (`a=t` versus `a=f, r=1`). A frame header also repeated width/height/format the receiver ignored, chunks claimed a random access nothing verified, and `Complete` marked a buffer renderable whatever had arrived, painting a dropped chunk as transparent pixels the producer never sent. ## What changed - The header names one target: `New { width, height, format }` starts a fresh image, `Frame { number }` addresses one frame in Kitty's own 1-based numbering (`ShowFrame` counts the same way), so the two meanings have two shapes and frame `0` is not expressible. - `total_bytes` and a chunk's `offset` are gone: every buffer of an image is `width × height × bytes_per_pixel`, and the receiver runs one transfer at a time, appending chunks at its own byte count. - `ImageShadow::apply` returns a typed error for every sequence the machine has no transition for, which the window turns into the same connection-local teardown a refused claim already got. - The proto break (deleted `ImageHeader.frame`) is acknowledged in `crates/felis-protocol/proto/BREAKING.md`; it rides the current pre-release major. ## Doc cascade `docs/reference/ipc.md` and `docs/reference/protocols/kitty-graphics.md` record the resulting facts (one target per meaning, ordered chunks, one transfer at a time, the malformed list); `docs/explanation/architecture/ipc.md` and `docs/explanation/protocols/kitty-graphics.md` own the argument, including the two rejected shapes — offset-addressed chunks and a 0-based optional frame index — and what would make either worth revisiting. `docs/explanation/data-model/image-store.md`, `docs/reference/testing.md`, and `CHANGELOG.md` follow. Verified: just check green (fmt, clippy, nextest, deny); reviewed by plan/correctness/docs lenses and pi sol ## Deferred - Debug assertion in `materialize_image_events` that no Header is queued while a prior transfer's Complete is pending — the plan calls it a cheap future-proofing catch, but no acceptance criterion asks for a producer-side guard and the receiver already refuses the sequence. - `ImageShadowError::DimensionsOverCap` as a distinct variant — #49 landed already; its caps surface as the existing `ImageBytes`/`SessionBytes`/`FrameIndex` variants, so this would be a rename with no new behavior. - A `felis-daemon` integration test shipping a two-frame image through the codec onto a live `ImageShadow` — the existing `image_outbox_materialization.rs` suite already round-trips producer output through the shadow and passes unchanged; a second end-to-end fixture would duplicate it without covering a criterion the shadow unit tests miss. - Reworking `docs/explanation/architecture/ipc.md`'s older "let the client trust the daemon" paragraph beyond the one sentence that named chunk offsets — its argument is untouched by this change; a fuller rewrite is prose work no criterion needs.

natsukium added 2 commits 2026-09-04 19:27:50 +09:00

feat(protocol): make image transfers canonical and stateful ... 68fe54cf72

The image wire carried two encodings of the root frame with different
meanings — `frame: None` replaced the image and dropped its animation
frames, `frame: Some(0)` edited the root's pixels in place — and both
were reachable from real Kitty traffic (`a=t` versus `a=f, r=1`). A
frame header also repeated width/height/format the receiver ignored,
chunks claimed a random access nothing verified, and `Complete` marked
a buffer renderable whatever had arrived, painting a dropped chunk as
transparent pixels the producer never sent.

The header now names one target: `New { width, height, format }` starts
a fresh image, `Frame { number }` addresses one frame in Kitty's own
1-based numbering (`ShowFrame` counts the same way), so the two
meanings have two shapes and frame `0` is not expressible. `total_bytes`
and a chunk's `offset` are gone: every buffer of an image is
`width × height × bytes_per_pixel`, and the receiver runs one transfer
at a time, appending chunks at its own byte count. `ImageShadow::apply`
returns a typed error for every sequence the machine has no transition
for, which the window turns into the same connection-local teardown a
refused claim already got.

Refs #51

docs: state the image transfer state machine ... 760384137f

The reference twin described the shape that just changed, and the
argument for it was nowhere: chunks were documented as reorderable
with no receiver that could verify the claim, and the root's two
encodings were stated as one rule in `ipc.md` and another in the
protocol source. Record the resulting facts in the reference pages
(one target per meaning, ordered chunks, one transfer at a time, the
malformed list) and the argument in the owning explanation doc, with
the two rejected shapes — offset-addressed chunks and a 0-based
optional frame index — and what would make either worth revisiting.

Refs #51

natsukium merged commit 760384137f into main 2026-09-04 19:39:01 +09:00

natsukium deleted branch feat/image-transfer-state-machine-51 2026-09-04 19:39:02 +09:00

natsukium referenced this pull request 2026-09-07 16:09:12 +09:00

[v0.1/Proto Review] Freeze protobuf and IPC contracts (7 issues) #52

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

priority/P0

Must be resolved before the v0.1.0 tag

  

priority/P1

Must be settled before the first public contract freezes

  

priority/P2

Explicitly deferred until after v0.1.0

  

release/v0.1.0

Work tracked for the first tagged release

  

status/blocked

Cannot start now: waits on an upstream release, another issue, or a recorded revisit trigger

  

status/planned

Triaged with a source-grounded plan and a passed pi review; ready to implement

  

type/bug

Shipped behavior that contradicts the docs or the code's own intent; needs a product code change

  

type/design

An open scope or trade-off question; needs a recorded decision in the owning explanation doc before code

  

type/test-gap

Documented or reviewed behavior that no test pins; usually split from a review finding, needs no product code change

  

type/tracker

Parent issue that tracks child work

No labels

priority/P0

priority/P1

priority/P2

release/v0.1.0

status/blocked

status/planned

type/bug

type/design

type/test-gap

type/tracker

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

natsukium/felis!81

No description provided.