🌐 US-Proxy
class="logged-out env-production page-responsive" style="word-wrap: break-word;" >
Skip to content

[fix] Add AppTest support for st.audio_input - #17346

Merged
lukasmasuch merged 4 commits into
developfrom
feature/apptest-p3-2-audio-input
Oct 9, 2026
Merged

lukasmasuch merged 4 commits into
developfrom
feature/apptest-p3-2-audio-input

Conversation

@lukasmasuch

@lukasmasuch lukasmasuch commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

Describe your changes

AppTest can stage one WAV recording for st.audio_input and read the file the script receives.

  • at.audio_input exposes the committed recording as .value (UploadedFile or None). .set_value(), .upload(), and .clear() stage that recording for the next .run(). .upload() replaces any staged recording because the widget holds a single file. .sample_rate echoes the sample_rate argument: 16000 when the script omits it, and None only when the script passes sample_rate=None.
  • A filename that does not end in .wav, a MIME type other than audio/wav, or a list of recordings raises AppTestError. The browser widget only produces a single WAV, so a one-item list is rejected too.
  • A recording inside a form stays uncommitted until that form is submitted.

Implements wiki queue P3.2 from the App testing fixes backlog.

GitHub Issue Link (if applicable)

N/A

Testing Plan

  • Unit Tests (JS and/or Python): lib/tests/streamlit/testing/element_tree_test.py covers a WAV recording, persistence across reruns, on_change, disabled widgets, invalid files, and form submit / clear_on_submit.
  • E2E Tests: Not applicable (AppTest-only)
  • Any manual testing needed?: No

Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

Tests can stage one WAV recording and observe the file the script receives, including inside forms.
Copilot AI balanced review requested due to automatic review settings October 8, 2026 22:32
@lukasmasuch lukasmasuch added change:feature PR contains new feature or enhancement implementation impact:users PR changes affect end users feature:app-testing Related to `AppTest` testing framework labels Oct 8, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@snyk-io

snyk-io Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

✅ Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
✅ Open Source Security 0 0 0 0 0 issues
✅ Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

✅ PR preview is ready!

Name Link
📦 Wheel file https://core-previews.s3-us-west-2.amazonaws.com/pr-17346/streamlit-1.65.0-py3-none-any.whl
📦 @streamlit/component-v2-lib Download from artifacts

@lukasmasuch

Copy link
Copy Markdown
Collaborator Author

Added Agent Docs

@greptile-apps

greptile-apps Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no new actionable issue was found.

Summary

Adds AppTest support for staging, reading, replacing, and clearing one WAV recording through at.audio_input.

  • Shares file registration and form behavior with FileUploader.
  • Adds tests for recording persistence, callbacks, disabled widgets, invalid inputs, and form submission.
  • Since the last review, clarifies errors and expands checks for replacement, committed values, and MIME case.
  • The earlier form-recording finding was withdrawn. lukasmasuch explained that a non-submit .run() discards staged form values by design, matching FileUploader.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A["set_value / upload / clear"] --> B["Stage recording"]
  B --> C["AppTest.run"]
  C --> D{"Form submitted or no form?"}
  D -->|Yes| E["Register bytes and send widget state"]
  D -->|No| F["Keep committed value"]
  E --> G["Script receives UploadedFile or None"]
  F --> G
  G --> H["New tree exposes committed value"]
Loading

Reviews (4) · Last reviewed commit: "Clarify AppTest audio_input errors for n..." · Reviewed by Greptile

Comment thread lib/streamlit/testing/v1/element_tree.py
@lukasmasuch lukasmasuch removed the feature:app-testing Related to `AppTest` testing framework label Oct 8, 2026
clear() reuses set_value, and the widget value is typed as one UploadedFile or None.
@lukasmasuch lukasmasuch added ai-review If applied to PR or issue will run AI review workflow feature:app-testing Related to `AppTest` testing framework and removed feature:app-testing Related to `AppTest` testing framework labels Oct 8, 2026
@github-actions github-actions Bot removed the ai-review If applied to PR or issue will run AI review workflow label Oct 8, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Summary

This PR adds typed AppTest support for st.audio_input. at.audio_input is a WidgetList of AudioInput. .value is the committed UploadedFile or None. .set_value(), .upload(), and .clear() stage a single audio/wav recording for the next .run(). A non-.wav name, a MIME type other than audio/wav, a sequence of files, or raw bytes raises AppTestError. Form gating, clear_on_submit, disabled widgets, and persistence across reruns reuse the existing in-memory file registration path.

All three expected reviews completed (claude-opus-5-5-medium, gpt-5.6-terra-xhigh, grok-4.7-xhigh). They agree this is AppTest-only parity with FileUploader: no production behavior change, no spec required, no security or performance impact, and no external-test coverage. Two approve. gpt-5.6-terra-xhigh requested changes for a form-staging interaction that follows the existing AppTest form contract. That request is resolved under Code Quality and is not merge-blocking.

Product Alignment

This change is user-facing for the public st.testing.v1.AppTest API. Production st.audio_input is unchanged: the same UploadedFile | None return, WAV-only backend check, and form behavior.

All three reviewers agree a separate product spec is not required. No spec in specs/ covers AppTest widget parity, and the PR body does not record a product approval to skip one. specs/AGENTS.md exempts small, non-controversial enhancements. This is parity with at.file_uploader for a widget that already ships, in the same series as recent AppTest-support changes. It adds no st.* command, config, or default.

The implementation matches the PR body and the principles in specs/AGENTS.md:

  • The common case is upload(filename, content) with MIME type audio/wav by default.
  • Access, .value, .set_value(), .upload(), and .clear() follow FileUploader. .upload() replaces the staged recording because the widget holds one file. FileUploader.upload() appends because that widget can hold many. All three reviewers treat that difference as intentional, and the method docstring already states it.
  • .value is UploadedFile | None, the same type st.audio_input returns.
  • A bad name, MIME type, or multi-file payload fails immediately with AppTestError, matching the browser widget (one WAV) and AudioInputSerde (.wav only).
  • sample_rate is exposed for inspection, including None when the script passed sample_rate=None. An unset optional proto field would otherwise read as 0.

Code Quality

The new node plugs into the existing seams: _register_uploaded_files, _unset_value_marker, and parse_tree_from_messages. It reuses the _files sentinel (InitialValue / list / None), file_uploader_state_value, and MemoryUploadedFileManager. HasField("sample_rate") keeps an omitted rate as None. clear() delegates to set_value(None), so the disabled-widget check stays in one place. There are no new dependencies or extra abstractions.

AudioInput._widget_state and _get_files_to_register copy the FileUploader versions, aside from list values in session state. Two reviewers treat that copy as consistent with the other widget classes and optional to share later, ideally before st.camera_input needs the same path. The inline comment records that follow-up.

Form staging, where the reviews disagree. gpt-5.6-terra-xhigh flagged a non-submit .run() inside a form: the replacement tree is constructed with InitialValue(), so a recording staged before that run is gone, and a later submit sends None unless the test calls set_value again. claude-opus-5-5-medium and grok-4.7-xhigh read the second set_value as the correct way to stage on the new tree.

Verification agrees with the two approvals. ElementTree.get_widget_states ignores uncommitted form values until that form's submit button is triggered, then .run() builds a new element tree. The embedded testing guide states the same rule: stage with set_value, click submit, then .run(); a .run() without the click discards the staged values. test_form_file_uploader_applies_only_on_submit re-stages after the non-submit run in the same way as test_audio_input_form_applies_only_on_submit. Keeping pending form values across tree replacement would be new behavior for every form widget, and it is outside this PR.

Performance

There is no meaningful production or frontend performance impact. Element creation, ForwardMsg handling, the WebSocket, session state, and the frontend are untouched. AppTest copies at most one in-memory WAV per .run() when it registers the file or restores it from session state, which is the same cost st.file_uploader already pays in tests. All three reviewers agree. No benchmark is needed.

Test Coverage

Unit coverage in lib/tests/streamlit/testing/element_tree_test.py is sufficient, and all three reviewers agree Playwright is the wrong layer for an in-process harness. Typing tests were not added for earlier AppTest widgets either.

Covered: one WAV via set_value and upload (including .WAV and the default MIME), persistence across an extra .run() without a second on_change, clear(), sidebar sample_rate=None versus 48000, at.get("audio_input"), repr, rejected click(), disabled upload and clear, rejected extension / MIME / multi-file / raw bytes, and form submit plus clear_on_submit. The old inspect-only case correctly moved from st.audio_input to st.camera_input.

The form test matches test_form_file_uploader_applies_only_on_submit: a .run() without submit leaves the script value empty, then a new set_value plus the submit button commits it. gpt-5.6-terra-xhigh asked for a regression that preserves a staged form recording across that non-submit .run(). That assertion would lock in behavior the form contract discards.

Backwards Compatibility

App scripts and the production st.audio_input API are unchanged. at.audio_input is new. at.get("audio_input") now returns AudioInput instead of UnknownElement, and set_value records a WAV. click() still raises AppTestError, with the typed-widget message. Tests that asserted the old unsupported-element error need updating. All three reviewers agree that is the feature.

Security & Risk

Registration stays inside AppTest's in-process MemoryUploadedFileManager. Filenames are metadata on UploadedFile. The mock upload URL is the same test string FileUploader already uses and is not an HTTP route. No new dependencies, endpoints, cookies, HTML rendering, or dynamic code execution. Changes to the shared registration loop are covered by the existing file_uploader tests. All three reviewers agree.

External test recommendation

  • Recommend external_test: No
  • Triggered categories: None
  • Evidence:
    • lib/streamlit/testing/v1/element_tree.py: adds an AppTest node that stages one WAV and serializes file_uploader_state_value. No server route, websocket, cookie, storage, or embed change.
    • lib/streamlit/testing/v1/app_test.py: registers those bytes with the existing in-memory file manager before the test script runs. The mock upload URL is test-only and matches FileUploader.
    • lib/tests/streamlit/testing/element_tree_test.py: in-process unit tests only.
  • Suggested external_test focus areas:
    • None. The diff does not change hosted, proxied, or embedded runtime behavior.
  • Confidence: High
  • Assumptions and gaps: The PR diff is these three files. Category 6 (uploads) was considered because bytes are registered for st.audio_input; the upload HTTP endpoint, CSRF wrapping, and download attributes are unchanged. All three reviewers agree.

Accessibility

No frontend or rendered UI changes. Accessibility does not apply. All three reviewers agree.

Readability

Names and the main docstrings are clear. AudioInput, _parse_audio_input_file, and the new test names state the single-WAV contract. The value docstring distinguishes a committed recording from one that is only staged, and _register_uploaded_files explains why audio input shares the file-uploader path.

gpt-5.6-terra-xhigh reported no readability findings. The other two reviewers proposed optional wording edits. Line-level notes on the sample_rate attribute, the list-rejection error, the MIME-normalization comment, and the .upload() replacement contrast are in the inline comments. Two further optional edits:

  • lib/streamlit/testing/v1/element_tree.py, AudioInput.upload mime_type (around line 1631): set_value documents that any other MIME type raises, and this parameter does not. Proposed rewrite: "MIME type of the recording. Defaults to audio/wav. Any other value raises AppTestError."
  • lib/tests/streamlit/testing/element_tree_test.py, test_audio_input_form_applies_only_on_submit (around line 1931): "stay local" and "drops them" do not say who commits or clears the recording. Proposed rewrite: "A form recording stays uncommitted until submit. clear_on_submit clears it on the next submit."

PR title: [feature] Add AppTest support for st.audio_input stands alone, matches [type] Description, and fits the length guideline. No change. All three reviewers agree.

PR description: gpt-5.6-terra-xhigh found the description sufficient. The other two want a tighter opening and an explicit note that .upload() replaces. "Implements wiki queue P3.2" is also internal shorthand for a later reader of the commit log. Proposed rewrite of the summary:

`AppTest` can stage one recording for `st.audio_input` and read the file the script receives.

- `at.audio_input` exposes the committed recording as `.value` (`UploadedFile` or `None`). `.set_value()`, `.upload()`, and `.clear()` stage that recording for the next `.run()`. `.upload()` replaces any staged recording because the widget holds a single file.
- A filename that does not end in `.wav`, a MIME type other than `audio/wav`, or more than one file raises `AppTestError`. The browser widget only produces a single WAV.
- A recording inside a form stays uncommitted until that form is submitted.

Part of the [App testing fixes backlog](https://github.com/streamlit/streamlit/wiki/2026-07-22-app-testing-fixes) (item P3.2).

The testing-plan section already matches the pull-request template. These description edits are optional.

Recommendations

  1. Merge as-is. Form staging follows the documented AppTest contract shared with FileUploader: stage, click submit, then .run().
  2. Optional follow-ups, any of which can land later: share the file-widget state helpers before st.camera_input copies them, and apply the docstring and PR-description wording above.

Verdict

APPROVED: Two of three reviews approve, and the remaining change request describes the existing form contract. AppTest support for st.audio_input matches the file-uploader testing pattern, rejects payloads the browser widget cannot produce, and is covered by unit tests.


This is an automated consolidated AI review by grok-4.7-xhigh. Please verify the feedback and use your judgment.

This review also includes 5 inline comment(s) on specific code lines.

Comment thread lib/streamlit/testing/v1/element_tree.py
Comment thread lib/streamlit/testing/v1/element_tree.py
Comment thread lib/streamlit/testing/v1/element_tree.py
Comment thread lib/streamlit/testing/v1/element_tree.py
Comment thread lib/streamlit/testing/v1/element_tree.py Outdated
@lukasmasuch lukasmasuch added the feature:app-testing Related to `AppTest` testing framework label Oct 8, 2026
A list is not a recording, and sample_rate plus upload replacement were undocumented. Form staging stays on the existing AppTest contract.
@lukasmasuch lukasmasuch removed the feature:app-testing Related to `AppTest` testing framework label Oct 8, 2026
@lukasmasuch

Copy link
Copy Markdown
Collaborator Author

@greptile-apps The inline follow-up marked the form-recording finding a false positive under the documented AppTest form contract (a non-submit .run() discards staged values, same as file_uploader). The summary line that says the PR should not merge is stale relative to that reply.

@greptile-apps

greptile-apps Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Agreed. The inline follow-up supersedes the original finding: under the documented AppTest form contract, a non-submit .run() discards staged form values, so re-staging before the submit run is expected and consistent with file_uploader. The form-recording issue is a false positive, and the summary line saying the PR should not merge is stale. There is no remaining merge blocker from this finding.

@lukasmasuch lukasmasuch added ai-final-review If applied to PR or issue will run AI review workflow with the latest generation of AI models. feature:app-testing Related to `AppTest` testing framework labels Oct 8, 2026
@github-actions github-actions Bot removed the ai-final-review If applied to PR or issue will run AI review workflow with the latest generation of AI models. label Oct 8, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Summary

This PR adds typed AppTest support for st.audio_input. at.audio_input returns WidgetList[AudioInput]. Tests can stage one WAV with .set_value(), .upload(), or .clear(), read the committed UploadedFile from .value, and read .sample_rate from the proto (16000 when the script omits the argument, None only when the script passes sample_rate=None). Staged bytes go through the existing in-memory file manager, including form submit and clear_on_submit.

All four expected reviews completed (gpt-5.6-sol-high, claude-opus-5-5-high, gemini-3.8-flash-high, grok-4.7-xhigh). None failed to complete review. All four approved. They agree this is a small AppTest-only extension of an existing widget, with no production, security, performance, or product-spec blocker. They differ on extracting a shared file-widget helper now, and on how far to tighten the rejection message, docstrings, and a few extra assertions. Those differences are optional follow-ups.

Product Alignment

This PR has user-facing impact on the public testing API (streamlit.testing.v1): at.audio_input, plus .value, .set_value(), .upload(), .clear(), and .sample_rate. It does not change st.audio_input, the rendered widget, or any production default.

No product spec under specs/ covers AppTest widget coverage, and the PR body does not claim a separate product approval. A spec is not required. This is a small, non-controversial extension of the existing testing API so an already-public widget is no longer inspect-only, which specs/AGENTS.md treats as exempt from a new spec. The same backlog already landed typed AppTest coverage for other existing widgets.

The surface matches st.audio_input and the existing FileUploader testing methods. .value is one UploadedFile or None. .upload(filename, content) defaults the MIME type to audio/wav and replaces any staged recording, which matches a widget that holds a single file. A non-.wav name, a MIME type other than audio/wav, or a list raises AppTestError before the next .run(). The MIME check is stricter than the runtime deserializer, which enforces the .wav filename and leaves MIME to the browser; rejecting other MIME types keeps tests from staging a payload the browser widget does not send. Form values stay uncommitted until that form is submitted. .sample_rate is read-only metadata.

That keeps the common case small, fails with an actionable error, and adds no new st.* surface. No material product issue.

Code Quality

AudioInput follows FileUploader: stage (file_id, filename, content, mime_type), serialize file_uploader_state_value, and let _register_uploaded_files put the bytes in MemoryUploadedFileManager before the script runs. Parsing runs before _files is assigned, so a rejected set_value leaves a previously staged recording in place. optional int32 sample_rate is read with HasField, so an unset field stays None instead of proto3's 0. Form handling reuses _unset_value_marker and _use_form_clear_defaults.

AudioInput._widget_state and _get_files_to_register repeat the FileUploader versions. One review suggested a shared helper so the mock upload URLs cannot drift. The other reviews treat a second explicit copy as the right size: the single-file WAV rules stay visible on AudioInput, and a shared helper can wait until st.camera_input needs the same path. The duck-typed loop over file_uploader and audio_input is acceptable at two widgets. Leave the copy as it is.

Performance

No production or frontend impact. st.audio_input, element enqueueing, ForwardMsg caching, session-state hot paths, and the WebSocket flush path are untouched. The new work runs only inside AppTest: one extra linear scan of audio-input widgets per .run(), plus an in-memory byte copy for each recording that must stay registered. That matches the existing file-uploader testing path and does not need a benchmark.

Test Coverage

Unit coverage is sufficient for this AppTest-only change. element_tree_test.py covers a WAV recording, .type, session state, on_change once per change, persistence across a plain .run(), .upload() replacement (including .WAV), .clear(), click() rejection, disabled upload/clear, invalid extension, MIME, list, and raw bytes, sidebar sample_rate=None, and form submit with clear_on_submit. The inspect-only test now uses st.camera_input, which is still untyped.

Two reviewers suggested extra assertions: upload(..., mime_type="Audio/WAV") round-trips .type == "audio/wav", two .upload() calls before one .run() keep only the second recording, and .value stays on the last committed recording until .run(). Those paths are straight-line assignments and lookups. They are worthwhile follow-ups. E2E coverage does not apply; the browser widget is unchanged.

Backwards Compatibility

Existing st.audio_input apps keep the same behavior. AppTest gains at.audio_input.

at.get("audio_input") now returns AudioInput instead of UnknownElement. .value still reads the same session-state entry, so existing inspections keep working. set_value() stages a recording instead of raising the inspect-only error. click() still raises AppTestError, with the typed-widget message. Code that checks isinstance(..., UnknownElement) on this widget will see the new type; that pattern was unsupported.

A non-UploadedFile stuffed into at.session_state for a keyed microphone no longer survives the next .run(), because the harness now sends widget state for this element. That session-state path was the workaround while the widget was unmodeled. Modeled widgets use .set_value() / .upload().

Security & Risk

No security-sensitive area is touched. There are no new routes, auth, cookies, WebSocket handling, dependencies, filesystem writes, HTML rendering, or dynamic code execution. Filenames are metadata on UploadedFile. File ids are UUIDs. Mock upload URLs are the same literals FileUploader already uses and are not fetched. Bytes stay in MemoryUploadedFileManager for the test session. A filename containing a null byte still fails later inside enforce_filename_restriction, same as st.file_uploader.

The only shared production-adjacent code is the registration loop, which still serves st.file_uploader and is covered by the existing file-uploader tests.

External test recommendation

  • Recommend external_test: No
  • Triggered categories: None
  • Evidence:
    • lib/streamlit/testing/v1/element_tree.py: in-process AppTest node, WAV validation, and file_uploader_state_value staging. No routing, auth, websocket, embedding, asset serving, or storage change.
    • lib/streamlit/testing/v1/app_test.py: registers staged bytes with the existing in-memory test file manager. No Starlette route, upload endpoint, CSRF/JWT wrapping, or download-attribute change.
    • lib/tests/streamlit/testing/element_tree_test.py: headless unit tests only.
  • Suggested external_test focus areas: None. Embedded and hosted runs do not execute this testing API.
  • Confidence: High
  • Assumptions and gaps: Category 6 (service worker, uploads, and downloads) was considered because AppTest registers upload bytes. No service worker, upload/download endpoint, or browser download behavior changed. The diff is limited to the three files above.

Accessibility

Not applicable. There are no frontend, DOM, or keyboard changes.

Readability

Line-level notes on the rejection error and the .sample_rate docstring are inline comments.

lib/tests/streamlit/testing/element_tree_test.py

  • test_audio_input_disabled_rejects_update docstring, line 1919: "A disabled microphone cannot be recorded or cleared" describes a physical device and the phrasing is awkward. One review cited line 1913; that line is the pytest.raises in the invalid-recordings test. Proposed rewrite: """A disabled audio_input widget cannot accept a recording or be cleared."""

PR title and description

  • Title: [feature] Add AppTest support for st.audio_input stands alone and matches the repository format. No change.
  • Description: The opening and the replace-on-upload / form bullets are clear. Two wording gaps remain. "More than one file" can be read as allowing a one-item list; every list is rejected. P3.2 is backlog jargon, and .sample_rate (16000 vs None) is part of the public testing surface and is easy to misread. Proposed description:
`AppTest` can stage one WAV recording for `st.audio_input` and read the file the script receives.

- `at.audio_input` exposes the committed recording as `.value` (`UploadedFile` or `None`). `.set_value()`, `.upload()`, and `.clear()` stage that recording for the next `.run()`. `.upload()` replaces any staged recording because the widget holds a single file. `.sample_rate` echoes the `sample_rate` argument: `16000` when the script omits it, and `None` only when the script passes `sample_rate=None`.
- A filename that does not end in `.wav`, a MIME type other than `audio/wav`, or a list of recordings raises `AppTestError`. The browser widget only produces a single WAV, so a one-item list is rejected too.
- A recording inside a form stays uncommitted until that form is submitted.

Part of the [App testing fixes backlog](https://github.com/streamlit/streamlit/wiki/2026-07-22-app-testing-fixes) (audio input).

Recommendations

  1. Optionally assert that upload(..., mime_type="Audio/WAV") stores .type == "audio/wav", and that .value stays on the last committed recording until .run().
  2. Optionally apply the PR-description rewrite above.
  3. Optionally reword the disabled-widget test docstring at element_tree_test.py:1919.

Verdict

APPROVED: All four reviews approve this backwards-compatible AppTest addition; the remaining notes are wording and optional assertions, not merge blockers.


This is an automated AI review consolidated by grok-4.7-xhigh. Please verify the feedback and use your judgment.

This review also includes 2 inline comment(s) on specific code lines.

Comment thread lib/streamlit/testing/v1/element_tree.py Outdated
Comment thread lib/streamlit/testing/v1/element_tree.py Outdated
The rejection message said every bad value was a list, and the sample_rate docstring treated None as the widget default.
@lukasmasuch

Copy link
Copy Markdown
Collaborator Author

Applied the remaining final-review notes in beb3c80.

  • Rejection errors now say a list is not accepted, and name the received type for every other invalid value.
  • The sample_rate docstring states the 16000 default and that None is an explicit sample_rate=None.
  • The disabled-widget test docstring no longer describes a physical microphone.
  • The recording test checks that a second .upload() replaces the first before .run(), that .value stays on the committed file until .run(), and that mime_type="Audio/WAV" is stored as audio/wav.
  • The PR description now matches that contract.

Left the FileUploader copy unshared. This review agreed a shared helper can wait.

@lukasmasuch lukasmasuch changed the title [feature] Add AppTest support for st.audio_input [fix] Add AppTest support for st.audio_input Oct 9, 2026
@lukasmasuch lukasmasuch added change:bugfix PR contains bug fix implementation ai-final-review If applied to PR or issue will run AI review workflow with the latest generation of AI models. and removed feature:app-testing Related to `AppTest` testing framework change:feature PR contains new feature or enhancement implementation labels Oct 9, 2026
@github-actions github-actions Bot removed the ai-final-review If applied to PR or issue will run AI review workflow with the latest generation of AI models. label Oct 9, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Summary

This PR adds typed AppTest support for st.audio_input. Tests can stage one WAV recording, read the committed UploadedFile, and exercise callbacks and forms. st.audio_input previously parsed as an UnknownElement, so set_value() raised.

AudioInput follows the existing FileUploader test path: bytes are registered with MemoryUploadedFileManager, widget state is a file_uploader_state_value, and a form recording stays uncommitted until that form is submitted. Validation rejects filenames that do not end in .wav, MIME types other than audio/wav, and lists, which matches the browser widget (one audio/wav file). .sample_rate uses HasField, so an omitted proto field stays None instead of the proto default 0.

All four expected reviews completed (gpt-5.6-sol-high, claude-opus-5-5-high, gemini-3.8-flash-high, grok-4.7-xhigh). None failed. All four approve. They agree this is a small testing-API parity change with sufficient unit tests, no production hot-path impact, no security exposure, and no external-test trigger. They differ only on optional polish: a shared widget-state helper, whether upload() should keep mime_type, a default-sample_rate assertion, and a few docstring wordings. Those are resolved below and are not merge-blocking.

Product Alignment

This PR has user-facing impact for AppTest authors. It adds AppTest.audio_input, Block.audio_input, and the AudioInput node. It does not change the st.audio_input command, its defaults, the browser widget, or rendered UI.

No product spec for AppTest widget coverage is merged on develop, and the PR body does not claim a separate product approval. A spec is not required. This is a small, non-controversial extension of the existing testing API so an already-shipped widget can be driven the way at.file_uploader can. The same incremental pattern landed recently for st.pagination, link_button / page_link, and st.form without a spec. It is not a new st.* command or a significant public app API.

The surface matches st.audio_input and the analogous FileUploader AppTest API, which is what every reviewer checked against the API-design principles:

  • .value is UploadedFile | None, the value the script receives, and it reflects the last .run().
  • .set_value((filename, content, mime_type)), .upload(), and .clear() chain and return Self. .upload() replaces the staged recording because the widget holds one file. at.file_uploader.upload() appends, and that difference matches the widgets.
  • .sample_rate echoes the script argument: 16000 when omitted (audio_input sets the proto field whenever the argument is not None), and None only when the script passes sample_rate=None. Staged bytes are stored as given.
  • Invalid filenames, MIME types, and lists fail at staging time with AppTestError.
  • Form staging uses the existing AppTest rule, including clear_on_submit.

The common case stays small: at.audio_input[0].upload("clip.wav", wav_bytes).run().

One reviewer suggested dropping upload()'s mime_type parameter because the only accepted value is audio/wav. The other reviews treat the parameter as consistent with FileUploader.upload and with set_value's (filename, content, mime_type) tuple. Keep it. A non-audio/wav value raises AppTestError at staging time, so the parameter is a checked input, and removing it would split the two methods. That is an optional API-shape preference, not a material alignment issue.

Code Quality

AudioInput is wired through the same extension points as FileUploader: parse_tree_from_messages, the ElementTree / Block accessors, _unset_value_marker (so form gating treats _files / InitialValue correctly), and _register_uploaded_files. Validation is factored into _parse_audio_input_file. .value is typed as UploadedFile | None. Sentinel states distinguish an unstaged recording, a staged recording, and an explicit clear. The complexity matches the feature.

Reviewers differed on sharing _widget_state with FileUploader. The proto-building loop matches, including the mock /mock/upload/test session id/ URLs. The session-state fallback does not: audio input restores one UploadedFile, and file uploader also restores a list. Every other widget in this module owns its own _widget_state. Leaving the copy local is the right call at this size.

Checked against the runtime: AudioInputSerde.deserialize enforces .wav filenames, the frontend encoder emits audio/wav, and the backend sets sample_rate only when the argument is not None. The HasField check matches that.

Performance

There is no production or frontend performance impact. Element creation, ForwardMsg handling, WebSocket flushing, session state, and the frontend are untouched. During an AppTest rerun, _register_uploaded_files also walks audio_input nodes and copies at most one recording per widget into the existing in-memory file manager. That is the same per-run cost st.file_uploader already pays in AppTest, proportional to the test workload. No hot-path benchmark is warranted.

Test Coverage

Unit coverage in lib/tests/streamlit/testing/element_tree_test.py is sufficient, and every reviewer agreed e2e coverage is not needed because the browser widget and production integration are unchanged. The new tests cover a WAV round-trip (name, MIME, bytes), persistence across an extra .run() without a second on_change, upload() replacement and MIME case normalization, .clear(), repr, click() rejection, disabled widgets, invalid payloads that leave the widget empty, sidebar sample_rate=None, at.get("audio_input") returning the typed node, and form submit / clear_on_submit including an outside-the-form rerun that keeps the committed value.

The previous inspectable-element test now uses st.camera_input, which is still an UnknownElement, so that coverage remains.

All four reviews agree this is enough to merge. Three reviewers wanted one more assertion for the documented default sample_rate; gpt-5.6-sol-high called the suite adequate. That follow-up is optional. The explicit 48000 and None cases already cover both HasField branches, and 16000 is the production widget default written into the proto.

Backwards Compatibility

App scripts and st.audio_input behavior are unchanged.

For AppTest, at.audio_input is new. at.get("audio_input") still finds the widgets and now returns AudioInput instead of UnknownElement. .value on that node is the committed UploadedFile | None (previously the raw proto field on the inspectable node). set_value() / upload() stage a recording. click() raises the typed-widget AppTestError. A test that asserted the old UnknownElement type, or that matched the old "does not implement interactions" string, needs the new API. The in-repo test that did so was updated. All four reviews treat this as the intended fix for normal AppTest usage.

Security & Risk

Risk is low, and all four reviews agree. The new path runs only inside AppTest. Recording bytes go to MemoryUploadedFileManager under the test session id. Filenames are metadata on UploadedFile. MIME types other than audio/wav are rejected before registration. File ids are uuid4() values, and the upload URLs are the existing mock strings. No WebSocket, auth, cookie, CORS, CSP, upload endpoint, HTML rendering, dependency, or dynamic execution changes. Regression risk to FileUploader is limited to iterating one more widget collection in the shared registration loop.

External test recommendation

  • Recommend external_test: No
  • Triggered categories: None
  • Evidence:
    • lib/streamlit/testing/v1/element_tree.py: AppTest node that builds the same file_uploader_state_value proto FileUploader already builds, with mock /mock/upload/test session id/ URLs. No server route, handshake, or browser upload client.
    • lib/streamlit/testing/v1/app_test.py: _register_uploaded_files also registers audio_input bytes on the in-memory test file manager. Production upload endpoints, CSRF wrapping, and download behavior are unchanged.
    • lib/tests/streamlit/testing/element_tree_test.py: in-process AppTest tests only.
  • Suggested external_test focus areas:
    • None. Embedded or hosted runs do not exercise AppTest.
  • Confidence: High
  • Assumptions and gaps: Category 6 (service worker, uploads, and downloads) was considered because recording bytes use upload-state protobufs. No service worker, HTTP upload/download endpoint, CSRF/JWT wrapping, or download behavior changed. Assumes the production st.audio_input upload path is not modified by uncommitted work outside this diff.

Accessibility

This PR has no frontend, DOM, keyboard, or assistive-technology changes. Accessibility is unchanged.

Readability

Reviewers split here. gpt-5.6-sol-high found the code comments, docstrings, title, and description already clear. The other three proposed wording tweaks. The items below are the ones that survive that check. Lateral rewrites of comments that already state the rule were dropped: the _files field comment already distinguishes InitialValue (not staged) from None (cleared), and the HasField comment already says an unset sample_rate reads as 0.

lib/streamlit/testing/v1/element_tree.py

  • _parse_audio_input_file docstring (around line 1519). "Sequences and other types are rejected" overstates the special case: only list has its own error, and other values hit the generic AppTestError. Proposed rewrite:

    """Return one ``(filename, content, mime_type)`` WAV recording.
    
    Raise ``AppTestError`` for a list or any other value that is not one
    ``(filename, content, mime_type)`` tuple. The browser widget sends a
    single WAV file.
    """
  • AudioInput class docstring (around line 1556). FileUploader is the test-node class, and the sample_rate paragraph does not say that staged bytes are stored as given. Proposed rewrite:

    r"""A representation of ``st.audio_input``.
    
    The recording is one WAV file, ``(filename, content, mime_type)``, with
    MIME type ``audio/wav``. ``.upload()`` replaces any staged recording.
    Pass one recording tuple; ``at.file_uploader`` is the widget that accepts
    a list and appends files.
    
    ``sample_rate`` is the argument the script passed to ``st.audio_input``.
    The widget default is ``16000``. ``None`` means the script passed
    ``sample_rate=None``, so the browser chooses the rate. The staged
    ``content`` is stored as given.
    """

PR title and description

  • Title: [fix] Add AppTest support for st.audio_input stands alone, uses the [type] Description form, and fits the length guideline. No change.

  • Description: It leads with the change, explains replacement, the sample_rate rule, WAV-only rejection, and form commit, and includes the testing-plan and license sections. gpt-5.6-sol-high would leave it as written. The other reviews asked to split the first bullet and to name who raises AppTestError. That is optional polish. The second bullet is the one that hides the actor ("a filename ... raises"). Proposed replacement for that bullet:

    - `AppTest` raises `AppTestError` for a filename that does not end in `.wav`, a MIME type other than `audio/wav`, or a list of recordings. The browser widget produces one WAV, so a one-item list is rejected too.

Recommendations

  1. In lib/streamlit/.agents/skills/developing-with-streamlit/references/testing.md, add one interaction example next to the existing set_value lines: at.audio_input[0].upload("clip.wav", wav_bytes).run(), including that the filename ends in .wav and that .upload() replaces the single recording. The reference does not claim st.audio_input is unsupported, and its interaction examples also omit file_uploader, so this is follow-up rather than a hole in this PR. Only grok-4.7-xhigh raised it. The embedded-skill guide asks for a brief mention when a new testing behavior is easy to misuse, which the single-WAV rule is.

Line-level nits (default sample_rate assertion, the list error text, and the registration docstring) are inline comments. They are optional.

Verdict

APPROVED: All four reviews approve this AppTest enhancement; it matches the single-WAV widget and the file-uploader testing contract, and the remaining disagreements are optional wording, a default-value assertion, and a skill example.


This is an automated consolidated AI review by grok-4.7-xhigh. Please verify the feedback and use your judgment.

This review also includes 3 inline comment(s) on specific code lines.

)


def test_audio_input_records_one_wav() -> None:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion: This test asserts sample_rate only for explicit 48000 and sample_rate=None. The class docstring and PR description also say an omitted argument is 16000. Add st.audio_input("Default", key="default") and assert at.audio_input(key="default").sample_rate == 16000. The 48000 case already covers HasField; this locks the widget default. The name test_audio_input_records_one_wav also undersells replacement, on_change, .clear(), and click(). test_audio_input_set_upload_clear_and_persist matches the body.

return filename, bytes(content), _AUDIO_INPUT_MIME
if isinstance(file, list):
raise AppTestError(
"st.audio_input records one recording. A list is not accepted."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion: A one-item list is the call people will try, because at.file_uploader.set_value() accepts a list. This message rejects it without naming the tuple. The non-list error just below already does.

raise AppTestError(
    "st.audio_input records one recording. "
    "Pass one (filename, content, mime_type) tuple, not a list."
)

"""Register bytes for file-upload widgets with the file manager.

Covers ``st.file_uploader`` and ``st.audio_input``. Both widgets
deserialize ``file_uploader_state_value`` through

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nitpick: These widgets do not deserialize through MemoryUploadedFileManager. Their serdes look up file IDs on the uploaded-file manager, which AppTest backs with that class. Suggested docstring:

"""Register files for ``st.file_uploader`` and ``st.audio_input`` widgets.

Both widgets resolve ``file_uploader_state_value`` file IDs against the
uploaded-file manager, so each staged or committed file must be registered
before the script runs.
"""

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Auto-approved by AI final review (workflow run).

@lukasmasuch
lukasmasuch merged commit 7806870 into develop Oct 9, 2026
76 checks passed
@lukasmasuch
lukasmasuch deleted the feature/apptest-p3-2-audio-input branch October 9, 2026 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:bugfix PR contains bug fix implementation impact:users PR changes affect end users

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants