Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changes/unreleased/final-response-text-rule-divergence.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
category: Fixed
---

- `RunResult::final_response` treats a non-string `text` block (including
`null`) as `""`. That is a recorded divergence from the Python SDK, which
coerces a truthy non-string via `str()` (`42` → `"42"`, `true` → `"True"`).
There is no runtime behavior change.
14 changes: 10 additions & 4 deletions .mstar/specs/dsh-sdk-wire-parity-surface.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ The rustdoc "known variants" count MUST say six, not five.
- `assistant/attempt` was added (`packages/core/session/src/types.ts:319`).
- `assistant/chunk` was removed.

Because the crate keeps `session.event.event` untyped (§3.2), the run path is unaffected: `final_response` reads the last root `assistant/message`'s `data.message.content`, and `finish_reason` reads the last root `turn/end`'s `data.reason.kind`. The crate MUST NOT claim `assistant/chunk` support and MUST NOT parse the embedded v2 stream (non-goal). Documentation MUST state the v2 vocabulary.
Because the crate keeps `session.event.event` untyped (§3.2), the run path is unaffected: `final_response` keeps the reversed-scan derivation of §6.2, and `finish_reason` reads the last root `turn/end`'s `data.reason.kind`. The crate MUST NOT claim `assistant/chunk` support and MUST NOT parse the embedded v2 stream (non-goal). Documentation MUST state the v2 vocabulary.

---

Expand Down Expand Up @@ -138,9 +138,9 @@ Crate-only fields MUST NOT exist beyond `timeouts` (the close-ladder struct, a l

Evidence: `python/sdk/src/deepseek_harness/api.py:40-46`; `packages/sdk/client/src/types.ts:69-79`; upstream asserts the removal at `python/sdk/tests/test_client.py:880`.

Derivation algorithms MUST match Python exactly:
Derivation algorithms MUST match Python exactly except where §7 records a divergence:

- `final_response` — reversed scan for the last root `assistant/message` whose `data` is an object and whose resolved `content` (`data.message.content` when `data.message` is an object, else `data.content`) is an array; a malformed last `assistant/message` (non-object `data` or non-array `content`) is skipped via `continue` and the scan falls back to the next earlier `assistant/message` (Python `continue` inside `reversed()`, `python/sdk/src/deepseek_harness/api.py:211-228`); a non-string `text` contributes `""`; `""` when no usable `assistant/message` exists.
- `final_response` — reversed scan for the last root `assistant/message` whose `data` is an object and whose resolved `content` (`data.message.content` when `data.message` is an object, else `data.content`) is an array; a malformed last `assistant/message` (non-object `data` or non-array `content`) is skipped via `continue` and the scan falls back to the next earlier `assistant/message` (Python `continue` inside `reversed()`, `python/sdk/src/deepseek_harness/api.py:211-228`); a string `text` contributes its value while `null`, a missing `text`, or any other non-string `text` contributes `""` (Python coerces a *truthy* non-string `text` through `str()` instead — a recorded divergence, §7.6); `""` when no usable `assistant/message` exists.
- `finish_reason` — last root `turn/end`'s `data.reason.kind` inside the activity interval; no `turn/end` → `None`; a malformed last `turn/end` → protocol error with the exact message `turn/end event requires a string data.reason.kind`; malformedness is checked only on the last one (reversed scan) (`python/sdk/src/deepseek_harness/api.py:231-248`).
- The runtime's `turn/end` reason vocabulary is six kinds — `completed`, `aborted`, `blocked`, `error`, `max-tokens`, `interrupted` (`packages/core/session/src/types.ts:198-222`) — and MUST stay a string, not a closed enum.

Expand Down Expand Up @@ -202,7 +202,7 @@ Python exposes `next_request` / `respond` / `respond_error` / `notify` (`python/

The crate fails the run with a protocol error when a `session.event` or `session.status` payload fails its shape check, in receipt-wait, event-collection, and idle-detection paths (`src/api.rs:220-240,261-280,285-303`). Python silently skips a malformed `event` / `status` and only raises on a malformed last `turn/end` (`python/sdk/src/deepseek_harness/api.py:149-181,245-246`); TypeScript raises for a malformed `session.event` envelope but ignores a malformed `session.status` (`packages/sdk/client/src/api.ts:189,210-212,264-286`).

Verdict: **keep the strictness** (it converts a silent hang into a typed failure) but **do not claim it is Python parity**. The rustdoc MUST say the policy is intentionally stricter. This is the one place where "follows Python" does not hold, and the docs must say so.
Verdict: **keep the strictness** (it converts a silent hang into a typed failure) but **do not claim it is Python parity**. The rustdoc MUST say the policy is intentionally stricter. This is one place where "follows Python" does not hold, and the docs must say so.

Related: the crate's embedded stderr text is capped (8 KiB, newest lines first) while both references embed the whole 400-line tail; and the crate's broadcast buffer is bounded with a fail-fast on observed lag where Python's queue is unbounded. Both are documented local robustness choices, not parity claims.

Expand All @@ -214,6 +214,12 @@ The crate requires exact equality with `deepseek-harness-sdk-runtime` (`packages

No `DeepSeekHarness::run` convenience and no lazy start: the crate requires an explicit `DeepSeekHarness::start`. Python has both a `run` convenience and lazy start (`python/sdk/src/deepseek_harness/api.py:121,124-131`); TypeScript lazily starts inside `run` (`packages/sdk/client/src/api.ts:177`). The divergence is documented in the crate's README and stays a non-goal.

### 7.6 Non-string `text` in `final_response`

Python appends `str(block.get("text") or "")` (`python/sdk/src/deepseek_harness/api.py:226`), which **coerces** a *truthy* non-string `text`: `42` → `"42"`, `true` → `"True"`, `[1]` → `"[1]"`, `{"a": 1}` → `"{'a': 1}"`. The crate contributes `""` for **every** non-string `text` — `null`, a missing key, and any truthy non-string alike. The two therefore agree wherever `text` is a string, `null`, or absent (and on every falsy non-string, which Python also maps to `""`), and diverge only on a truthy non-string value, which a conformant runtime never emits: `text` blocks are strings (`packages/llm/llm/src/types.ts:55-58`).

Verdict: **keep the crate's rule**. Faithfully emulating `str()` for arbitrary JSON values would need a Python-`repr` formatter (bool → `True`, float `1.0` → `"1.0"`, dict → `{'a': 1}`, `inf`, …) to serve a path unreachable from a conformant runtime, which the crate's simplicity rules reject. The rustdoc on `RunResult::final_response` and `derive_final_response` states the real rule and points here; the `derive_final_response` unit table asserts both the shared-domain parity and these divergence rows.

---

## 8. Deferred and non-goal surface
Expand Down
103 changes: 75 additions & 28 deletions src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
//! tree, send `session/prompt`, wait for the durable `agent/inbox/spliced`
//! receipt of the returned message id, collect every tree notification until
//! the **root** session reports `idle`, then derive
//! [`RunResult::final_response`] and [`RunResult::finish_reason`] exactly as
//! the Python SDK does.
//! [`RunResult::final_response`] and [`RunResult::finish_reason`] as the
//! Python SDK does, except the recorded §7.6 text-coercion divergence.
//!
//! [`RunResult`] mirrors the **Python** SDK's five fields exactly
//! (`session_id`, `final_response`, `finish_reason`, `events`,
Expand Down Expand Up @@ -442,13 +442,16 @@ pub struct RunResult {
/// The SDK session id this turn ran on.
pub session_id: String,
/// Text concatenation of the last usable root `assistant/message`
/// event's text blocks (`text: null` or a non-string `text` contributes
/// `""`). The activity interval is scanned in reverse; a last
/// `assistant/message` whose `data` is not an object or whose resolved
/// `content` (`data.message.content` when `data.message` is an object,
/// else `data.content`) is not an array is skipped and the scan falls
/// back to the next earlier `assistant/message`. `""` when the interval
/// contains no usable `assistant/message` (Python algorithm,
/// event's text blocks: a string `text` contributes its value; `null`,
/// a missing `text`, or any other non-string `text` contributes `""`.
/// Python coerces a *truthy* non-string `text` through `str()` instead
/// (`42` → `"42"`, `true` → `"True"`) — a recorded divergence, not
/// parity (spec §7.6). The activity interval is scanned in reverse; a
/// last `assistant/message` whose `data` is not an object or whose
/// resolved `content` (`data.message.content` when `data.message` is an
/// object, else `data.content`) is not an array is skipped and the scan
/// falls back to the next earlier `assistant/message`. `""` when the
/// interval contains no usable `assistant/message` (Python algorithm,
/// `python/sdk/src/deepseek_harness/api.py:211-228`).
pub final_response: String,
/// The last root `turn/end` event's `data.reason.kind` inside the
Expand Down Expand Up @@ -485,17 +488,20 @@ pub fn extract_finish_reason(events: &[Value]) -> Result<Option<String>, Error>
Ok(None)
}

/// Python `final_response` verbatim: a reversed scan for the last root
/// `assistant/message` whose `data` is an object and whose resolved
/// `content` is an array. Content lives at `data.message.content` when
/// `data.message` is an object, else at `data.content` (Python `isinstance`
/// walk). A last `assistant/message` whose `data` is not an object, or whose
/// resolved `content` is not an array, is malformed and skipped (`continue`
/// inside the reversed loop), so the scan **falls back to the next earlier
/// `assistant/message`** (Python `api.py:211-228`). `""` when no usable
/// `assistant/message` exists. Blocks with `type == "text"` contribute
/// their string `text`; `text: null` (or a non-string `text`) contributes
/// `""` (Python parity).
/// Python `final_response`, except the recorded §7.6 divergence: a reversed
/// scan for the last root `assistant/message` whose `data` is an object and
/// whose resolved `content` is an array. Content lives at `data.message.content`
/// when `data.message` is an object, else at `data.content` (Python
/// `isinstance` walk). A last `assistant/message` whose `data` is not an
/// object, or whose resolved `content` is not an array, is malformed and
/// skipped (`continue` inside the reversed loop), so the scan **falls back to
/// the next earlier `assistant/message`** (Python `api.py:211-228`). `""` when
/// no usable `assistant/message` exists. Blocks with `type == "text"`
/// contribute their string `text`; `text: null`, a missing `text`, or any
/// other non-string `text` contributes `""`. Python coerces a *truthy*
/// non-string `text` through `str()` instead: the reversed scan and its
/// two fallbacks are parity, this one block-level rule is a recorded
/// divergence (spec §7.6).
fn derive_final_response(events: &[Value]) -> String {
for event in events.iter().rev() {
if event.get("type").and_then(Value::as_str) != Some("assistant/message") {
Expand All @@ -521,6 +527,10 @@ fn derive_final_response(events: &[Value]) -> String {
return blocks
.iter()
.filter(|block| block.get("type").and_then(Value::as_str) == Some("text"))
// A non-string `text` (including `null` and a missing key)
// contributes "". Python's `str(block.get("text") or "")` coerces
// a *truthy* non-string instead — the recorded divergence, spec
// §7.6; do not "fix" this back without a superseding decision.
.map(|block| block.get("text").and_then(Value::as_str).unwrap_or(""))
.collect();
}
Expand Down Expand Up @@ -634,12 +644,15 @@ mod tests {
}

// Cross-implementation parity with upstream Python `final_response`
// (`python/sdk/src/deepseek_harness/api.py:211-228` @ c389f96bf3). The
// edge cases and expected outputs are the ones the cited Python source
// produces, so the Rust port is locked to it line for line. The
// discriminating cases ("falls back when last content/data is null")
// guard against the bug: a malformed last `assistant/message` must
// fall back to an earlier one.
// (`python/sdk/src/deepseek_harness/api.py:211-228` @ c389f96bf3) **on
// the shared domain**: for a string (or absent/`null`) `text` the case
// table below is the Python source's output byte for byte. A *truthy*
// non-string `text` (`42` → "42", `true` → "True", `[1]` → "[1]",
// `{"a": 1}` → "{'a': 1}") is the one recorded divergence — Python
// coerces it through `str()`, the crate contributes `""` (spec §7.6;
// the two rows are marked inline). The discriminating cases ("falls back
// when last content/data is null") guard against the bug: a malformed
// last `assistant/message` must fall back to an earlier one.
#[test]
fn derive_final_response_matches_python_on_parity_edge_cases() {
fn am(content: Value) -> Value {
Expand Down Expand Up @@ -713,14 +726,48 @@ mod tests {
vec![am(json!(["str", 42, {"type": "text", "text": "ok"}, null]))],
"ok",
),
// Recorded divergence (spec §7.6): Python's
// `str(block.get("text") or "")` coerces a *truthy* non-string
// — `42` → "42", `true` → "True", `[1]` → "[1]", `{"a": 1}` →
// "{'a': 1}". The crate contributes "" for every non-string
// `text`, which the two rows below assert (number and object;
// a truthy non-string flips every one of them together).
(
"divergence: truthy non-string text (number) yields empty",
vec![am(json!([{"type": "text", "text": 42}]))],
"",
),
(
"divergence: truthy non-string text (object) yields empty",
vec![am(json!([{"type": "text", "text": {"a": 1}}]))],
"",
),
// Python's `isinstance(data.get("message"), dict)` is false for a
// non-object `message`, so content resolves at `data.content`.
(
"message present but not an object: data.content is used",
vec![am_data(json!({
"message": "not-an-object",
"content": [{"type": "text", "text": "flat"}],
}))],
"flat",
),
// Python fallback #1 with a non-object `data` that is not `null`.
(
"non-object data (not null) is skipped",
vec![am_data(json!([]))],
"",
),
];
for (id, events, expected) in cases {
assert_eq!(
derive_final_response(events),
*expected,
"{id}: Rust `derive_final_response` must match upstream Python \
`final_response` byte-for-byte (python/sdk/src/deepseek_harness/api.py:211-228 \
@ c389f96bf3)"
`final_response` byte-for-byte on the shared domain \
(python/sdk/src/deepseek_harness/api.py:211-228 @ c389f96bf3); \
the `divergence:` rows assert the crate's recorded rule instead \
(spec §7.6)"
);
}
}
Expand Down
38 changes: 38 additions & 0 deletions tests/run_semantics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,44 @@ async fn final_response_falls_back_when_last_assistant_message_data_is_not_an_ob
assert_eq!(result.finish_reason.as_deref(), Some("completed"));
}

#[tokio::test]
async fn final_response_is_empty_when_every_assistant_message_in_the_window_is_malformed() {
// The exhausted-scan complement of the fallback cases: when the reversed
// scan skips every `assistant/message` in the interval — here a non-array
// `content` and two non-object `data` values (`null` and an array, the
// latter covering a non-object `data` that is not `null`) — there is no
// earlier usable message to fall back to, so `final_response` is ""
// (spec §6.2). The scan is exhausted, not aborted: the run still
// completes and the last `turn/end` still yields its reason.
let mut script = run_prefix("msg-exhausted");
script.extend([
emit("session.event", root_event(receipt_event("msg-exhausted"))),
emit(
"session.event",
root_event(assistant_event(serde_json::Value::Null)),
),
emit(
"session.event",
root_event(json!({"type": "assistant/message", "data": null})),
),
emit(
"session.event",
root_event(json!({"type": "assistant/message", "data": []})),
),
emit("session.event", root_event(turn_end("completed"))),
emit("session.status", idle(ROOT_SESSION)),
exit(0),
]);
let result = run_once(&script, "hello").await.expect("run succeeds");

assert_eq!(
result.final_response, "",
"an interval whose every assistant/message is malformed must yield \
\"\" once the reversed scan is exhausted (spec §6.2)"
);
assert_eq!(result.finish_reason.as_deref(), Some("completed"));
}

#[tokio::test]
async fn prompt_error_propagates_and_client_stays_usable() {
let script = [
Expand Down
Loading