The speaker travels with the plan — T524–T529 (gh-#772) - #820
Merged
Merged
Conversation
…ntract; SegmentRequest.Speaker additive (gh-#772) SpeakerSnapshot (PersonaId, PersonaName, Voice, Pace, Rules, Corrections, ContentHash) lands in GenWave.Abstractions per SPEC F189.1 so a planned speaker can travel with the plan instead of being re-read from the ambient caches at render time. SpeechCorrection moves from GenWave.Tts to the contract verbatim (Tts and its specs pick it up via usings; Story253 keeps a targeted alias so its PronunciationRule binding does not change). SegmentRequest.Speaker is an init-only nullable body property, not a 15th positional parameter: the published 14-arity ctor/Deconstruct is preserved (same precedent as CrosstalkAiredThisBreak). Nothing sets or reads it yet; T526 adds the reader and T527 the writer. The published-package surface fixture stays byte-identical to 5.7.0. The Story431 surface-diff fact tolerates exactly the 16 F189.1 lines through a named allowlist (PendingPublicationSurfaceAllowlist); any other addition or any removal still fails. T530 regenerates the fixture from the published package and deletes the allowlist. Story456 AC1: Speaker null by default, initializer round-trip, ctor arity pinned at 14.
…shotSource reads a card once (gh-#772) Core gains two seams: IPersonaCardByIdSource (one method, the card-by-id read Tts needs without depending on the whole IPersonaStore) and ISpeakerSnapshotSource (ForStationAsync / ForPersonaAsync). Tts implements the latter in SpeakerSnapshotSource: station snapshot from the identity provider; persona snapshot from the card (clamped pace, merged rules via PronunciationRuleResolver, card corrections, and the same fingerprint terms the ambient caches fold today, exposed by SpeakerSnapshotFingerprint as a plain joined string for T526 to hash). Unknown id or a store fault degrades to the station snapshot with one WARN naming the id. Host's PersonaCardByIdStore adapts IPersonaStore.GetCardByIdAsync (no new SQL), registered beside AddPersonaStore; Tts registers the snapshot source via TryAddSingleton. SEAMS.md regenerated (two additive rows). Specs: STORY-456 AC10 (Host, ephemeral Postgres, real AddPersonaStore factory) and AC12 (Tts, unknown-id fallback + WARN) green; T526 facts remain pending.
…hes untouched (gh-#772) TtsSegmentSource.RenderAsync now has two arms. When request.Speaker is null it takes ResolveFromAmbientAsync, a verbatim extraction of today's path, so every existing caller — auditions, PA/House Voice, plugin renders — is byte-identical (SPEC F189.6). When a snapshot is present ResolveFromSnapshot uses its pace and rules, carries its corrections through the new TtsRenderContext.Corrections, and reads no ambient cache at all (SPEC F189.3). The snapshot arm keys its cache with TtsRenderKey.ComputeSnapshotHash rather than splicing terms into the ambient formula. The two key spaces are deliberately disjoint, so a snapshot render never reuses a clip the ambient path wrote. The first snapshot-driven render of an evergreen clip therefore re-synthesizes once, an accepted cost now written down in TtsRenderKey. A snapshot never overrides request.Voice. When the two disagree the render logs one WARN naming both and airs the request's voice. Both voice terms are newline-stripped: a persona card's voice id is operator-uploaded and is accepted unverified when the speech engine is faulting, so it is untrusted free text on a log line (CWE-117). Doc corrections this branch made necessary: SpeakerSnapshot.Corrections is card-only and merged card-over-station downstream, not the merged set; its ContentHash matches the ambient path on invalidation timing but never on key identity; and SpeakerSnapshotFingerprint no longer reads as an instruction to reproduce the ambient digest, which this task deliberately does not do. Gate: full solution green, Tts 889 passed / 2 skipped, 0 failed. Smoke: Release Host booted against Postgres, /health 200 Healthy, station schema migrated.
…t through a per-plan memo (gh-#772) Every Render and Verbatim slot now carries a SpeakerSnapshot; Ready slots carry none, per SPEC F187.4. SpeakerResolution memoises the lookups for the life of one PlanAsync call: one ForStationAsync per plan and one ForPersonaAsync per distinct persona, reused across every slot that names them (F188.4, STORY-456 AC9). The memo is a local, never a field, so concurrent plans on the singleton planner get independent memos. The planner-resolved voice stays authoritative. A slot resolves its snapshot and then aligns the snapshot to that voice, rather than stamping the snapshot's voice onto the request. This matters because the persona ROW and the persona CARD are separate seams that provably diverge — an unvalidated card voiceId (F79.4), or the card-less default persona PersonaCardMigrator creates. Letting the card win would have silently retuned a break to a voice the planner never chose. BuildHandoffRequest applies the same alignment to a snapshot captured at arm time. Station-voiced slots now render with the station snapshot's pace and corrections where the pre-T527 ambient path applied the active persona's. That follows from F188.4 and is audible; the ear check is the T529 dev-station wire.
…S.md (gh-#772) STORY-456 AC11 asserts the committed seam index lists the two seams T525 registered: ISpeakerSnapshotSource (GenWave.Tts) and IPersonaCardByIdSource (GenWave.Host). SEAMS.md itself needed no change — T525 regenerated it, and tools/check-seam-index.sh confirms it still matches a fresh generation. AC11's byte-identity half is deliberately not re-asserted here. It is a global invariant over one generated file and is already owned twice: by Story294's TheCommittedSeamsFileMatchesAFreshGenerationByteForByte, which carries no Category trait and so runs in the PR tier, and by tools/check-seam-index.sh at ci.yml:39 — which is the owner T528's own acceptance line names. A third copy added no coverage and cost three WebApplicationFactory<Program> builds, two of them discarded unread; the scenario is now a plain File.ReadAllText. The header comment records where byte-identity lives so the copy does not come back. Architecture 172/12 -> 174/9. Full solution 0 failed, 0 warnings.
…y scoped (gh-#772) Records the manual dev-station run behind STORY-456's AC13 in the two `Skip =` consts on ScenarioTheDevStationWire, and — as importantly — records what that run does NOT establish. The run flips segment_schedule day 6 once per trial and holds, in both directions, landing each flip inside the narrow plan->render window by freezing the kokoro container the instant the outbound synth call starts and releasing it after. Four booth_log rows (94739/94740 under a beta plan, 94744/94745 under an alpha plan) each name the persona their break was PLANNED under, ~8s after the schedule was written to the other one, with TtsSegmentSource.LogRenderOutcome naming the same persona at the same instant. Scope, stated in the const rather than left for a reader to discover: an ~8s flip-to-row gap sits inside both the per-unit-plan ResolveAsync horizon and the 30s StalenessBound on the three card caches, so a still-re-resolving implementation would have produced identical rows. These rows are a plan-time-stamp regression guard, not timing evidence for which arm rendered them. What the run does establish is by construction: ISpeakerSnapshotSource is registered, BreakPlanner builds a SpeakerResolution, no card-degrade warnings fired, so request.Speaker was non-null and RenderCopyAsync took the snapshot arm — which returns rules, pace and hash as one tuple, fixing pace to the plan-time speaker.Pace rather than the live personaPace.Current that T527 closed. AC13's pace conjunct has no clean measured wire ratio. No production door drives arbitrary copy through TtsSegmentSource under a chosen persona, and the run's own artifacts could not be paired to copy text with confidence, so durations are reported without a derived ratio rather than dressed up as one. Both facts stay skipped with Assert.Fail guard bodies; the "manual: " prefix sits on each const so stack_gate.sh's per-file scan matches the names actually used in Skip= (SPEC F182.1).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎙️ PR-4 — the speaker travels with the plan (T524–T529)
Fourth PR of the orchestrator split (gh-#401), closing out STORY-456 of gh-#772. Follows #805 (PR-1), #808 (PR-2) and #818 (PR-3).
🎯 What changes
A break's speech now carries the speaker it was planned with, instead of re-reading whoever happens to be active when the render finally runs. That read was live: a persona switch between planning a break and rendering it could put the wrong voice, pace and pronunciation rules on segments that had already been scheduled under someone else.
SpeakerSnapshot+SpeechCorrectionon the contract;SegmentRequest.SpeakeradditiveISpeakerSnapshotSource+ card-by-id seam;SpeakerSnapshotSourcereads a card onceBreakPlannerresolves a speaker per slot through a per-plan memoSEAMS.mdThe branch is additive throughout:
RenderCopyAsynctakes the snapshot arm only whenSegmentRequest.Speakeris non-null and falls through to the existing ambient path otherwise, so nothing that does not stamp a speaker changes behaviour. The two arms use structurally disjoint cache keys, so they cannot collide.✅ Gate
Full solution, Release, warnings as errors:
Smoked on the Release binary against a real Postgres:
/health→200 Healthy, station schema migrated, no DI resolution failures building theOrchestrator/BreakPlannergraph.🔬 T529 — what the dev-station run proves, and what it does not
Worth reading before merging, because the honest scope is narrower than the task line implies.
Established. The run flips
segment_scheduleday 6 once per trial and holds, in both directions, landing each flip inside the plan→render window by freezing the kokoro container the moment the outbound synth call starts and releasing it after. Fourbooth_logrows — 94739/94740 under a beta plan, 94744/94745 under an alpha plan — each name the persona their break was planned under, ~8s after the schedule was written to the other one.LogRenderOutcomenames the same persona at the same instant.Not established. That ~8s gap sits inside both the per-unit-plan
ResolveAsynchorizon and the 30sStalenessBoundon the three card caches, so a still-re-resolving implementation would have produced identical rows. The rows are a plan-time-stamp regression guard, not timing evidence for which arm rendered them. The run's own 59s and 74s settle times are the measure of that.Pace has no measured wire ratio. No production door drives arbitrary copy through
TtsSegmentSourceunder a chosen persona —/api/tts/previewand/api/safe-segmentsboth callITtsSynthesizerdirectly, below the planner. Durations came out in the right direction but the artifacts could not be paired to copy text with confidence, so they are reported without a derived ratio rather than dressed up as one. Pace-freezing rests on construction instead: the snapshot arm returns rules, pace and hash as one tuple, so a non-nullSpeakerfixes pace to the plan-time value.All of this is written into the two
Skip =consts, so the limit travels with the code rather than living in a PR description.👂 Owed
An ear check on the demo box that the planned persona's pace actually airs — same shape as the T430 and T454 ear checks. That is the one claim in STORY-456's AC13 no automated or dev-station evidence in this PR covers.
📦 Next
T530 is the pins PR (compose.pinned + CHANGELOG), which also regenerates the Abstractions surface baseline and deletes
PendingPublicationSurfaceAllowlist. One ordering note found while scoping it:publish-nuget.ymltriggers onv*tags, so the 5.10.0 package will not exist until the tag. The surface fact reads the live-built assembly rather than a package, so T530 regenerates from the build at its own commit — which is the same tree the tag publishes. Nothing in PR-5 touchessrc/GenWave.Abstractions, so that baseline stays valid through the tag.