feat: [Admin Assistant] Voice Briefings (TTS Integration)#264
feat: [Admin Assistant] Voice Briefings (TTS Integration)#264
Conversation
- Add VoiceBriefing DataObject (model/VoiceBriefing.php) - Add BriefingScripts with 4 template types: morning, meeting, evening, urgent - Add VoiceBriefingService with ElevenLabs TTS integration - Add VoiceBriefingTask BuildTask for cron-scheduled generation - Add voicebriefings.yml configuration - Add PHPUnit test suite (BriefingScripts tests)
There was a problem hiding this comment.
Pull request overview
Adds an “Admin Assistant” voice briefing feature to the SilverStripe app, including script templating, persistence for generated briefings, a cron-invokable BuildTask, and YAML configuration for provider/quiet-hours settings.
Changes:
- Introduces
VoiceBriefingDataObject to store transcripts, audio paths, duration, and metadata. - Adds
BriefingScriptstemplates/helpers and aVoiceBriefingServicewith ElevenLabs TTS generation + caching. - Adds
VoiceBriefingTaskBuildTask and YAML config files for scheduling/provider settings, plus unit tests for script generation helpers.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/Model/VoiceBriefing.php | New DataObject for storing generated briefing records and cleanup logic. |
| app/src/Scripts/BriefingScripts.php | Adds briefing script templates and helper utilities (pluralize, duration estimate, formatting). |
| app/src/services/VoiceBriefingService.php | Implements briefing generation, ElevenLabs API call, audio storage, and cache lookup. |
| app/src/Tasks/VoiceBriefingTask.php | Adds BuildTask entrypoint for scheduled generation + cleanup. |
| app/src/Test/VoiceBriefings/VoiceBriefingTest.php | Adds unit tests for BriefingScripts helpers/templates. |
| app/_config/voicebriefings.yml | Adds YAML configuration for service/task/model (lowercase namespace keys). |
| app/_config/voice.yml | Adds a second YAML configuration for the same feature (different casing/Name). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| curl_setopt($ch, CURLOPT_HTTPHEADER, [ | ||
| 'Accept: audio/mpeg', | ||
| 'Content-Type: application/json', | ||
| 'xi-api-key: ' . $apiKey, | ||
| ]); |
There was a problem hiding this comment.
The request always sets Accept: audio/mpeg, but the saved file extension comes from the configurable $format (mp3/wav/ogg). If $format is set to wav/ogg you’ll still receive MP3 bytes and save them with the wrong extension. Map $format to the correct ElevenLabs output format/headers (or restrict $format to mp3 when using ElevenLabs).
| @@ -0,0 +1,223 @@ | |||
| <?php | |||
|
|
|||
| namespace App\services; | |||
There was a problem hiding this comment.
This file lives under app/src/Scripts but declares namespace App\services; which makes the class hard to find and inconsistent with the rest of the codebase layout. Consider moving it to app/src/services (or changing the namespace to match the directory), and standardising the namespace casing to App\Services.
| namespace App\services; | |
| namespace App\Scripts; |
| @@ -0,0 +1,184 @@ | |||
| <?php | |||
|
|
|||
| namespace App\tasks; | |||
There was a problem hiding this comment.
Namespace casing here (App\tasks) is inconsistent with existing task namespaces (e.g. App\ContentHub\Tasks) and with the config file app/_config/voice.yml which uses App\Tasks\VoiceBriefingTask. Standardise to App\Tasks (or update config/class references) to avoid config manifest / class resolution issues.
| namespace App\tasks; | |
| namespace App\Tasks; |
| protected function getRecipients(): array | ||
| { | ||
| return []; | ||
| } |
There was a problem hiding this comment.
getRecipients() currently always returns an empty array, so the task will always print “No recipients configured” and never generate/deliver any briefings. If this is intended as a placeholder, it should be wired up to the actual recipient source (e.g. SiteConfig/Member list) before merging, or guarded behind a feature flag so cron runs don’t silently no-op.
| @@ -0,0 +1,147 @@ | |||
| <?php | |||
|
|
|||
| namespace App\model; | |||
There was a problem hiding this comment.
Namespace casing here (App\model) doesn’t match other DataObjects (e.g. namespace App\Model; in app/src/Model/Product.php) and conflicts with app/_config/voice.yml which configures App\Model\VoiceBriefing. Please standardise the namespace to App\Model (and update all references/config keys).
| namespace App\model; | |
| namespace App\Model; |
| namespace App\test\VoiceBriefings; | ||
|
|
||
| use PHPUnit\Framework\TestCase; | ||
| use App\services\BriefingScripts; |
There was a problem hiding this comment.
Namespace casing here (App\\test) is inconsistent with other tests under app/src/Test (e.g. namespace App\\Test\\ClientPortal;). This can break test discovery/autoloading depending on tooling. Recommend using namespace App\\Test\\VoiceBriefings; and updating the BriefingScripts import to the standard service namespace casing as well.
| namespace App\test\VoiceBriefings; | |
| use PHPUnit\Framework\TestCase; | |
| use App\services\BriefingScripts; | |
| namespace App\Test\VoiceBriefings; | |
| use PHPUnit\Framework\TestCase; | |
| use App\Services\BriefingScripts; |
| @@ -0,0 +1,25 @@ | |||
| --- | |||
| Name: voice-briefings | |||
There was a problem hiding this comment.
There are two new config files (voice.yml and voicebriefings.yml) configuring the same classes with different Name headers and different class-name casing. Keeping both is likely to cause confusing/duplicate config merges and make it unclear which values are effective. Please consolidate into a single config file and ensure class keys match the actual namespaces used in code.
| Name: voice-briefings | |
| Name: voice |
| App\model\VoiceBriefing: | ||
| cache_ttl: 86400 | ||
|
|
||
| App\services\VoiceBriefingService: | ||
| provider: 'elevenlabs' |
There was a problem hiding this comment.
Config keys here use App\\model / App\\services (lowercase segments) while other code/config uses App\\Model / App\\Services. SilverStripe config is keyed by the exact class name string, so mixed casing can lead to config not applying. Align these keys with the canonical namespaces used in code (and remove the duplicate config file).
| echo "Generating {$type} briefings...\n"; | ||
|
|
||
| $service = new \App\services\VoiceBriefingService(); | ||
| $schedule = self::$schedule; |
There was a problem hiding this comment.
The task instantiates the service with new \App\services\VoiceBriefingService(). If VoiceBriefingService is made Configurable/Injectable (recommended), prefer getting it via Injector (or ::singleton()) so configured dependencies/settings are applied consistently.
| public function generateBriefing(string $type, array $data, string $deliveryMethod = 'telegram', ?string $recipientID = null): ?VoiceBriefing | ||
| { | ||
| // Check quiet hours (skip for urgent alerts) | ||
| if ($type !== 'urgent' && $this->isQuietHours()) { | ||
| return null; |
There was a problem hiding this comment.
New core behaviours in generateBriefing() (quiet-hours skipping, script-hash caching, and persistence of VoiceBriefing records) aren’t covered by tests. Given the potential for subtle regressions (e.g. cache key/recipient handling), it would be good to add unit tests around generateBriefing() using a stubbed TTS client/audio writer.
Implements #193
Changes
Briefing Types
Features
Fixes #193