Skip to content

docs(sdk): describe typed provider boundaries and Responses streaming - #891

Merged
neubig merged 2 commits into
OpenHands:mainfrom
vedjoshi1:refactor/4976-typed-llm-boundaries
Oct 6, 2026
Merged

neubig merged 2 commits into
OpenHands:mainfrom
vedjoshi1:refactor/4976-typed-llm-boundaries

Conversation

@vedjoshi1

@vedjoshi1 vedjoshi1 commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor
  • I have read and reviewed the documentation changes to the best of my ability.
  • If the change is significant, I have run the documentation site locally and confirmed it renders as expected.

Summary of changes

  • Describe private typed provider boundaries and optional chat-template tokenizer
    loading/fallback behavior in the SDK LLM architecture page.
  • Document Responses streaming, sync/async callbacks, required-stream endpoints,
    generic iterator support, and yielded-versus-wrapper completion precedence.

Companion implementation: OpenHands/software-agent-sdk#5504
Tracking issue: OpenHands/software-agent-sdk#4976
Completion-selection behavior follows alanhuangyoo's SDK PR #4772.

Validation / limitations

  • Fresh local docs checks after the example correction: uv run --with pytest --with requests --with pyyaml pytest -q tests/ produced 35 passed and 2 errors. The errors were pricing
    fixtures fetching a removed upstream file (HTTP 404); these pages do not change
    pricing documentation or tests.
  • Companion SDK validation includes real local HTTP/SSE parsing and packaged
    server startup; detailed results are in the SDK PR.
  • The updated Responses example was syntax-checked and executed against the
    installed SDK with a recording stand-in for the request, confirming imports,
    streaming arguments, and system-before-user message order. No live model call
    was made. Mintlify rendering has not been verified locally.
  • The human contributor reviewed the documentation changes. Both linked PRs
    are ready for maintainer review; the SDK branch has been rebased and validated.

Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: openhands <openhands@all-hands.dev>
@vedjoshi1
vedjoshi1 marked this pull request as ready for review October 5, 2026 03:50
@vedjoshi1
vedjoshi1 requested a review from enyst as a code owner October 5, 2026 03:50

@neubig neubig left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving on behalf of @neubig.

Reviewed the diff and verified every documented claim against the merged SDK code (main @ 69eb315, the squash commit of #5504):

  • Example is correct and runnable. Ran the new responses() snippet verbatim against a real model (gpt-5.5 via the Responses endpoint, stream=True, on_token): it streamed output and returned an LLMResponse. ModelResponseStream is genuinely re-exported from openhands.sdk.llm.streaming, so the import resolves.
  • Removed warning was accurate to remove. responses()/aresponses() do accept stream=True; the old 'chat completion endpoint only' caveat was stale. No other stale limitation text remains on the page.
  • Behavioral prose matches implementation: non-streaming fallback when on_token is None (llm.py:1790, 2048); requires_streaming/is_subscription draining without a callback (llm.py:961); LLMNoResponseError on a stream with no completed response (llm.py:1337, _response_stream.py:57); wrapper completed_response precedence and generic sync/async iterable support (_response_stream.py); optional Transformers tokenizer with LiteLLM fallback (_tokenizer.py).
  • Docs tests reproduce exactly as described: 35 passed, 2 errors. I confirmed the 2 errors are identical on unmodified main (pricing fixtures 404 on a removed upstream file) — pre-existing and unrelated to this PR.
  • Rendering/structure: code fences balanced, both pages present in docs.json nav, the new /sdk/guides/llm-streaming link and #ready-to-run-example anchor both resolve. The new code block carries no file-reference metadata, so the daily sync-docs-code-blocks job will not overwrite it.

Note: the broken-links workflow is still gated as action_required (fork PR); I could not approve it with the available token, so link validation above was done manually.

This review was produced by an AI agent (OpenHands) on behalf of @neubig.

@neubig
neubig merged commit 2ea54e6 into OpenHands:main Oct 6, 2026
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.

2 participants