Add support for appending media (vision support)

[chrisboulton]

Media is carried with a append:

seq_command:
  data: 
    command: append
    media:
      uri: https://example.com/image.png
      blob: base64 encoded bytes if you have that instead
      mime_type: mime type if you have it
      hash: optional hash we'll verify if you pass it

Summary by CodeRabbit

• New Features

  • Added support for attaching media when creating a sequence entry, including media details like URI or blob data.
  • Sequence text submissions now handle missing text more flexibly.

• Bug Fixes

  • Improved compatibility with older sequence requests that don’t include media.
  • Ensured media attachments are sent and received correctly during sequence updates.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR adds optional media support to append requests, introduces SeqAppendMedia, updates serialization and tests, and adds Seq::append_media to send append commands with media payloads.

Changes

Append media support

Layer / File(s)	Summary
Append request media contract src/protocol.rs	SeqAppendReq now serializes text with a default and includes optional media; SeqAppendMedia defines optional uri, blob, hash, mime_type, and detail fields. Tests cover legacy deserialization and media round-trips.
Client append_media entrypoint src/client/seq.rs	Seq::append_media creates and sends SeqCommand::Append with media: Some(media), waits for the response, and returns Ok(()); the import list is reformatted for the new type usage.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I hop through bytes, both old and new,
With media tucked in, soft and true.
Append goes out on a moonlit breeze,
And tests bounce back with gentle ease.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: adding media append support for vision use cases.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch vision

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (2)

src/client/seq.rs (1)

380-406: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

append_media duplicates the append send/await boilerplate verbatim.

The cid allocation, channel registration, SeqCommand::Append send, and rx.recv() handling are identical to append (Lines 352-378). Extract a shared helper that takes a fully-built SeqAppendReq so the two entrypoints differ only in how they construct the request.

♻️ Proposed helper extraction

async fn send_append(&self, req: SeqAppendReq) -> Result<(), ModelSocketError> {
    let cid = Uuid::new_v4().to_string();
    let (tx, mut rx) = mpsc::channel(1);
    self.cmds.lock().await.insert(cid.clone(), tx);

    self.socket
        .send_request(MSRequest::SeqCommand {
            cid: cid.clone(),
            seq_id: self.seq_id.clone(),
            data: SeqCommand::Append(req),
        })
        .await?;

    rx.recv()
        .await
        .ok_or_else(|| ModelSocketError::Command("failed to receive response".into()))??;

    Ok(())
}

pub async fn append(
    &self,
    text: impl AsRef<str>,
    opts: AppendOpts,
) -> Result<(), ModelSocketError> {
    self.send_append(SeqAppendReq {
        text: text.as_ref().to_string(),
        role: opts.role,
        ..Default::default()
    })
    .await
}

pub async fn append_media(
    &self,
    media: SeqAppendMedia,
    opts: AppendOpts,
) -> Result<(), ModelSocketError> {
    self.send_append(SeqAppendReq {
        media: Some(media),
        role: opts.role,
        ..Default::default()
    })
    .await
}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/client/seq.rs` around lines 380 - 406, The append_media implementation
repeats the same cid setup, channel registration, MSRequest::SeqCommand send,
and rx.recv() error handling already present in append. Extract that shared flow
into a helper like send_append that accepts a fully built SeqAppendReq, then
have append and append_media only build their respective request payloads and
delegate to it so the duplication is removed.

src/protocol.rs (1)

233-245: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value

uri and blob are mutually exclusive but the type permits both/neither.

SeqAppendMedia lets callers set both uri and blob (or omit both, serializing to "media":{}). The model can't express the intended one-of contract. If the server doesn't strictly reject the ambiguous cases, this can silently produce surprising behavior. Consider either an enum (e.g. uri/blob as variants) or documenting the invariant and validating it at the client entrypoint.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/protocol.rs` around lines 233 - 245, SeqAppendMedia currently allows both
uri and blob to be present or absent, but the intended contract is one-of only.
Update SeqAppendMedia in protocol.rs so the uri/blob relationship is enforced or
clearly represented, ideally by replacing the struct shape with an enum variant
design around SeqAppendMedia or by adding validation at the client entrypoint
where SeqAppendMedia is constructed/used. If keeping the struct, ensure the
relevant builder/constructor/path that creates SeqAppendMedia rejects ambiguous
cases and documents the invariant using the SeqAppendMedia symbol.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/client/seq.rs`:
- Around line 380-406: The append_media implementation repeats the same cid
setup, channel registration, MSRequest::SeqCommand send, and rx.recv() error
handling already present in append. Extract that shared flow into a helper like
send_append that accepts a fully built SeqAppendReq, then have append and
append_media only build their respective request payloads and delegate to it so
the duplication is removed.

In `@src/protocol.rs`:
- Around line 233-245: SeqAppendMedia currently allows both uri and blob to be
present or absent, but the intended contract is one-of only. Update
SeqAppendMedia in protocol.rs so the uri/blob relationship is enforced or
clearly represented, ideally by replacing the struct shape with an enum variant
design around SeqAppendMedia or by adding validation at the client entrypoint
where SeqAppendMedia is constructed/used. If keeping the struct, ensure the
relevant builder/constructor/path that creates SeqAppendMedia rejects ambiguous
cases and documents the invariant using the SeqAppendMedia symbol.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a22f5fd1-e143-4e9d-9b25-839bc29ecff9

📥 Commits

Reviewing files that changed from the base of the PR and between c1a0d66 and aaae0da.

📒 Files selected for processing (2)

• src/client/seq.rs
• src/protocol.rs