Skip to content

docs(bidi): refine model provider guides [replica of harness-sdk#4894] - #27

Open
poshinchen wants to merge 1 commit into
base-pr-4894from
strandly-v3-pr-4894
Open

poshinchen wants to merge 1 commit into
base-pr-4894from
strandly-v3-pr-4894

Conversation

@poshinchen

@poshinchen poshinchen commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Replica of https://github.com/strands-agents/harness-sdk/pull/4894 (URL in backticks to avoid a cross-reference on the original PR) for reviewer evaluation. Branches mirror the original PR's base and head commits.

The review below was produced by the optimized Strandly reviewer (variant v3-reason, optimization run aeb3ee, agent model Opus 4.8) during validation on PRs it never saw in training. Compare with the human and Strandly reviews on the original PR.

@poshinchen poshinchen left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Strandly v3-reason (optimized) review

Assessment: comment

Assessment: Comment

This is a solid documentation refactor that standardizes the setup flow (Installation → Credentials → Configuration) across the three bidi providers, trims redundant voice examples, and routes readers to the quickstart, I/O guide, and API references. The structure and tone improvements are clear wins.

Review Categories
  • Link integrity: The rewrite shifts to reference-style links and internal anchors (e.g., google.mdx#connection-restarts); please confirm all labels resolve and anchors exist.
  • Content fidelity: Several technical claims were added/changed (us-east-1 region fallback, Gemini 16kHz/24kHz defaults and auto-transcripts, session-resumption fallback) — verify against current implementation.
  • Lost detail: The OpenAI table removed timeout_s, organization, and project; the timeout_s 60-minute-limit workaround is non-obvious and should remain discoverable.
  • API surface: Confirm the newly referenced AudioCapable / get_audio_config() are intentionally public.

Overall a readable, well-organized improvement — the main asks are verification rather than rework.

File-level comments:

  • site/src/content/docs/user-guide/sdk/bidi/models/index.mdx: Suggestion

Issue: The session resumption link now points to google.mdx#connection-restarts instead of the external Gemini docs, and adds a "falling back to history replay" claim.
Suggestion: Please verify the #connection-restarts anchor actually exists on the Gemini page (the google.mdx diff shows a "Session settings" / connection section but the anchor isn't confirmed in the visible diff), and that the fallback-to-history-replay behavior is accurate for the SDK.

  • site/src/content/docs/user-guide/sdk/bidi/models/bedrock.mdx: Suggestion

Issue: The Credentials example drops the region argument in favor of a boto_session, and the prose states the region fallback is us-east-1. The removed caution box previously listed the specific supported regions (us-east-1, us-west-2, eu-north-1, ap-northeast-1).
Suggestion: Please verify the [nova-regions] reference link preserves that region list so readers who relied on the explicit callout can still find it. Also confirm the us-east-1 default-region fallback claim matches the model's actual behavior.

  • site/src/content/docs/user-guide/sdk/bidi/models/google.mdx: Suggestion

Issue: The example no longer passes client_args={"api_key": ...} and instead relies on the GOOGLE_API_KEY env var, which is clearer. The prose states "16 kHz input by default and fixed 24 kHz output" and "The adapter enables transcripts for both user speech and model responses."
Suggestion: Please verify these audio-rate and automatic-transcript claims against the current GoogleGeminiLiveModel defaults so the docs don't drift from implementation.

  • site/src/content/docs/user-guide/sdk/bidi/models/openai.mdx: Important

Issue: The removed table documented timeout_s (the 60-minute session limit workaround) and organization/project auth params. The new prose directs readers to the "constructor reference" but these fields are no longer described inline.
Suggestion: timeout_s carries non-obvious behavior (no OpenAI warning before the 60-min limit). Please verify this guidance survives somewhere reachable — either keep a short note or confirm the [model-api] reference clearly surfaces it, since dropping it risks users hitting silent session closures.

  • site/src/content/docs/user-guide/sdk/bidi/models/openai.mdx: Suggestion

Issue: These guides use many reference-style link labels ([nova-sonic], [nova-regions], [boto3-credentials], [credentials-setup], [realtime], [api-keys], [voices], [transcription-model], [model-api], [gemini-live], [client-api], [session-settings], [vad]). The matching URL definitions appear below the truncated portion of each file.
Suggestion: Since the PR mentions link verification was run, please confirm every reference label has a corresponding definition and no dangling labels remain after the rewrite — unresolved reference links render as literal text.

Inline comments (could not anchor to the diff):

  • site/src/content/docs/user-guide/sdk/bidi/models/index.mdx:108: Important

Issue: A new reference-style link definition [audio-capable] is added here, and the body references [AudioCapable][audio-capable]. Please verify AudioCapable and get_audio_config() are actually part of the public API at strands.bidi.models so the API-doc anchor resolves.
Suggestion: If AudioCapable is a new/public abstraction surfaced to customers, confirm the API reference anchor builds and that this symbol is intentionally public (consider whether it needs API bar-raising review).

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

Documentation Preview Failed

The documentation deployment encountered an error. Please check the deployment logs for more details.

This branch had an error being deployed

1 failed deployment
auto-approve — 7bde20c4 Deployed Oct 9, 2026 by poshinchen via Build and Deploy Preview #8
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