Skip to content

RE1-T122 Bug fixes for audio tts#420

Merged
ucswift merged 1 commit into
masterfrom
develop
Jun 29, 2026
Merged

RE1-T122 Bug fixes for audio tts#420
ucswift merged 1 commit into
masterfrom
develop

Conversation

@ucswift

@ucswift ucswift commented Jun 29, 2026

Copy link
Copy Markdown
Member

Description

This PR fixes bugs in the voice dispatch TTS (text-to-speech) path where long dispatch text exceeding the TTS chunk limit would cause ArgumentException failures, breaking voice dispatch playback (Sentry: RESGRID-API-78).

Changes

  • Pre-warm path (PreWarmPromptAsync): Previously threw ArgumentException for any input spanning more than one TTS chunk. It now iterates over all chunks and pre-warms each individually, so long dispatch text no longer faults the pre-warm/redirect flow.

  • Playback path (TryAppendDispatchPlaybackAsync in TwilioController): Switched from GetPromptUrlAsync (single-chunk only) to the multi-chunk-aware AppendPromptAsync, which emits one <Play> element per chunk. This prevents failures when dispatch text (e.g., long notes or addresses) exceeds the chunk limit.

  • Tests: Added two regression tests verifying that PreWarmPromptAsync no longer throws for multi-chunk text and that AppendPromptAsync produces a <Play> per chunk for multi-chunk input.

@Resgrid-Bot

Resgrid-Bot commented Jun 29, 2026

Copy link
Copy Markdown

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

Kody Guide: Usage and Configuration
Interacting with Kody
  • Request a Review: Ask Kody to review your PR manually by adding a comment with the @kody start-review command at the root of your PR.

  • Validate Business Logic: Ask Kody to validate your code against business rules by adding a comment with the @kody -v business-logic command.

  • Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Current Kody Configuration
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug
Performance
Security
Business Logic

Access your configuration settings here.

@request-info

request-info Bot commented Jun 29, 2026

Copy link
Copy Markdown

Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details?

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6308b9f0-bab0-4622-9767-3c8c95fa0c9b

📥 Commits

Reviewing files that changed from the base of the PR and between 8f712f5 and 064bce8.

⛔ Files ignored due to path filters (1)
  • Tests/Resgrid.Tests/Web/Services/TwilioVoiceResponseServiceTests.cs is excluded by !**/Tests/**
📒 Files selected for processing (2)
  • Web/Resgrid.Web.Services/Controllers/TwilioController.cs
  • Web/Resgrid.Web.Services/Twilio/TwilioVoiceResponseService.cs

📝 Walkthrough

Walkthrough

PreWarmPromptAsync is updated to iterate all chunks from ChunkText and pre-warm each, removing the single-chunk restriction. TryAppendDispatchPlaybackAsync switches from GetPromptUrlAsync plus a manual <Play> append to AppendPromptAsync for chunk-aware TwiML rendering.

Multi-chunk Twilio dispatch prompt

Layer / File(s) Summary
PreWarmPromptAsync multi-chunk iteration
Web/Resgrid.Web.Services/Twilio/TwilioVoiceResponseService.cs
Removes the single-chunk enforcement and ArgumentException path; now loops over all chunks from ChunkText, calling GetOrCreatePromptUrlAsync for each with a fault-only logging continuation.
TryAppendDispatchPlaybackAsync chunk-aware append
Web/Resgrid.Web.Services/Controllers/TwilioController.cs
Replaces the GetPromptUrlAsync call and manual <Play> append with AppendPromptAsync, delegating multi-chunk TwiML rendering to the service.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • github-actions
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is broadly aligned with the Twilio text-to-speech fixes in this PR.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch develop

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

}

[Test]
public async Task pre_warm_prompt_async_should_not_throw_for_multi_chunk_text()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kody code-review Kody Rules low

.NET naming convention violation occurs because the test methods pre_warm_prompt_async_should_not_throw_for_multi_chunk_text and append_prompt_async_should_emit_a_play_per_chunk_for_multi_chunk_text use snake_case. Rename these methods to use PascalCase to comply with C# standards.

Kody rule violation: Use proper naming conventions

[Test]
public async Task PreWarmPromptAsyncShouldNotThrowForMultiChunkText()

[Test]
public async Task AppendPromptAsyncShouldEmitAPlayPerChunkForMultiChunkText()
Prompt for LLM

File Tests/Resgrid.Tests/Web/Services/TwilioVoiceResponseServiceTests.cs:

Line 137:

Violates rule 'Use proper naming conventions': the two new test methods use snake_case names (pre_warm_prompt_async_should_not_throw_for_multi_chunk_text and append_prompt_async_should_emit_a_play_per_chunk_for_multi_chunk_text) instead of PascalCase as required by the .NET naming convention for methods.

Suggested Code:

[Test]
public async Task PreWarmPromptAsyncShouldNotThrowForMultiChunkText()

[Test]
public async Task AppendPromptAsyncShouldEmitAPlayPerChunkForMultiChunkText()

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

@ucswift

ucswift commented Jun 29, 2026

Copy link
Copy Markdown
Member Author

Approve

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This PR is approved.

@ucswift ucswift merged commit ec4c6bf into master Jun 29, 2026
18 of 19 checks passed
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