Skip to content

docs(api): record the final_response text-coercion divergence and pin it with tests - #21

Merged
btspoony merged 2 commits into
mainfrom
fix/pr-review-followups-parity
Sep 14, 2026
Merged

btspoony merged 2 commits into
mainfrom
fix/pr-review-followups-parity

Conversation

@btspoony

Copy link
Copy Markdown
Member

What

Makes the final_response text rule truthful and pins it with tests, without changing runtime behaviour.

Follow-up from the 2026-09-14 review of #19 (residual pr-review-followups-2026-09-14-3).

Why

Python's final_response coerces text with str(block.get("text") or "") (python/sdk/src/deepseek_harness/api.py:225), so a truthy non-string text becomes a string: 42 → "42", true → "True", [1] → "[1]". The Rust port maps text with as_str().unwrap_or(""), so every non-string contributes "". #19 rewrote the derivation correctly but re-asserted the old claim — spec §6.2 and the public rustdoc both said "" is Python parity — and its parity table had no row that could detect the difference.

Decision

Rust keeps "" for non-string text; the documentation stops claiming parity. Faithfully emulating Python's str() for arbitrary JSON 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 — rejected under the crate's simplicity rules. The divergence is now recorded instead of denied.

Changes

  • .mstar/specs/dsh-sdk-wire-parity-surface.md — §6.2 states the real rule (string text contributes its value; null or any non-string contributes "") and points at the new entry; §6.2's intro is scoped to "except where §7 records a divergence"; new §7.6 records the coercion examples, the reachability argument and the one-line rationale in the established §7 entry shape; §5.3's stale "last assistant/message's data.message.content" clause now points at §6.2 instead of contradicting it; §7.3's exclusivity wording relaxed so it does not contradict §7.6.
  • src/api.rs — dropped the (Python parity) mislabel from both rustdocs and stated the actual rule citing §7.6; a comment at the mapping names the divergence. Function bodies are unchanged (comment-stripped comparison of derive_final_response and extract_finish_reason against the pre-change revision is identical).
  • src/api.rs tests — six new rows: four truthy non-string text values (42, true, [1], {"a":1} → "", marked as the recorded divergence), the message-present-but-not-an-object arm of the isinstance walk, and a non-object non-null data. The table's assertion message is scoped to the shared domain so it no longer promises byte-for-byte parity across the divergence.
  • tests/run_semantics.rs — new end-to-end case: an interval where every assistant/message is malformed yields final_response == "" with finish_reason == Some("completed").
  • one new changelog fragment (consumer-facing).

Verification

  • cargo test --lib derive_final_response → 1 passed (all new rows included).
  • cargo test --test run_semantics final_response → 4 passed / 0 failed.
  • cargo test --test run_semantics final_response_is_empty_when_every_assistant_message_in_the_window_is_malformed → 1 passed.
  • grep -rn "Python parity" src/api.rs .mstar/specs/dsh-sdk-wire-parity-surface.md → no surviving non-string-text parity claim.

Notes

  • The spec §7 preamble requires a divergence be stated in rustdoc and, where user-visible, in the README pair. This one is not user-visible: the README's RunResult table lists final_response as a field without describing its derivation, so no README statement became false. No bilingual edit is required; recorded here as the deliberate judgment.

… it with tests

The spec and rustdoc claimed the non-string-`text` rule was Python parity.
Python's `str(block.get("text") or "")` coerces a truthy non-string, while the
crate contributes "". Keep the crate's rule (emulating `str()` would need a
Python-repr formatter for a path no conformant runtime reaches) and document it
as spec §7.6: §6.2 now states the real rule, §5.3 points at §6.2 instead of the
pre-PR algorithm, and the unit table asserts both the shared-domain parity and
the divergence rows. New exhausted-scan end-to-end case: every malformed
`assistant/message` in the window yields "" with finish_reason completed.
No behavior change to derive_final_response or extract_finish_reason.
@cursor

cursor Bot commented Sep 14, 2026 •

Copy link
Copy Markdown

PR Summary

Low Risk
No runtime behavior change—only spec/rustdoc accuracy and test coverage for documented Python divergence on malformed text blocks.

Overview
Documentation-only correction for how RunResult::final_response treats text blocks: string values are kept; null, missing, or any non-string text maps to "". The wire-parity spec and rustdoc stop claiming full Python parity because Python’s str(block.get("text") or "") still stringifies truthy non-strings (42 → "42", etc.); that gap is now §7.6 with an explicit “keep Rust behavior” verdict. derive_final_response logic is unchanged.

Tests are tightened to match: unit rows for the four divergence cases plus data.content / non-object data edges; integration test that an interval where every assistant/message is malformed yields final_response == "" while the run still completes. Changelog fragment notes the doc fix for consumers.

Reviewed by Cursor Bugbot for commit 7e36e1e. Configure here.

F-001: the module doc and the `derive_final_response` doc opener no longer
claim unqualified Python parity; each now names the recorded §7.6
text-coercion divergence.

F-002: §7.6 cited `api.py:225` for `parts.append(str(block.get("text") or ""))`;
verified against `git show c389f96bf3:python/sdk/src/deepseek_harness/api.py`
that `def final_response` starts at 211 and the append sits at 226. The
`api.py:211-228` range citations are unaffected.

F-003: the divergence rows collapse to two (a number and an object) — all four
values flip together under any text-mapping mutation — with the full
`42`/`true`/`[1]`/`{"a": 1}` set kept as comment examples and named in the
table header.

F-004: the changelog fragment leads with the consumer fact instead of naming
internal artifacts.
@btspoony
btspoony merged commit 01682aa into main Sep 14, 2026
6 checks passed
@btspoony
btspoony deleted the fix/pr-review-followups-parity branch September 14, 2026 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant