-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat(turns): let external-turn STT decide the turn start, retire provisional_vad #767
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7de48c7
537b7b4
c16f3d3
58e053e
0653c63
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| """retire the provisional_vad turn start strategy | ||
|
|
||
| Revision ID: c4e21b7f80a9 | ||
| Revises: f3a1c47b9e02 | ||
| Create Date: 2026-09-13 17:10:00.000000 | ||
|
|
||
| """ | ||
|
|
||
| import json | ||
| from typing import Sequence, Union | ||
|
|
||
| import sqlalchemy as sa | ||
| from alembic import op | ||
|
|
||
| # revision identifiers, used by Alembic. | ||
| revision: str = "c4e21b7f80a9" | ||
| down_revision: Union[str, None] = "f3a1c47b9e02" | ||
| branch_labels: Union[str, Sequence[str], None] = None | ||
| depends_on: Union[str, Sequence[str], None] = None | ||
|
|
||
| # `provisional_vad` gated the user turn start on a transcript, so the turn start | ||
| # resolved from a queued frame and the interruption it broadcast flushed the | ||
| # queued end-of-turn proposal behind it — the turn then never closed. It is | ||
| # retired in favour of `default`, which follows the STT's own turn boundaries | ||
| # when the STT reports them and falls back to transcript + VAD when it does not. | ||
| # | ||
| # `provisional_vad_pause_secs` only configured that strategy, so it goes too. | ||
| # | ||
| # Run history (workflow_runs and friends) records what actually executed and is | ||
| # deliberately not rewritten. `workflow_definitions` rows are immutable | ||
| # versions, but a run executes against the definition it points at, so they are | ||
| # rewritten here as well — otherwise a re-run of an old definition would still | ||
| # ask for the retired strategy. | ||
| _RETIRED_STRATEGY = "provisional_vad" | ||
| _REPLACEMENT_STRATEGY = "default" | ||
| _RETIRED_KEY = "provisional_vad_pause_secs" | ||
|
|
||
| _TARGETS = ( | ||
| ("workflows", "workflow_configurations"), | ||
| ("workflow_definitions", "workflow_configurations"), | ||
| ) | ||
|
|
||
|
|
||
| def _rewrite(config: dict) -> bool: | ||
| """Rewrite one configuration dict in place. True if anything changed.""" | ||
| changed = False | ||
| if config.get("turn_start_strategy") == _RETIRED_STRATEGY: | ||
| config["turn_start_strategy"] = _REPLACEMENT_STRATEGY | ||
| changed = True | ||
| if _RETIRED_KEY in config: | ||
| del config[_RETIRED_KEY] | ||
| changed = True | ||
| return changed | ||
|
|
||
|
|
||
| def upgrade() -> None: | ||
| conn = op.get_bind() | ||
| for table, column in _TARGETS: | ||
| # These are `json`, not `jsonb`, so key containment (`?`) is unavailable | ||
| # and a ::jsonb cast is avoided. | ||
| # | ||
| # The two operators are not interchangeable here. `->>` extracts the | ||
| # value as text, which is what the strategy comparison wants, but it | ||
| # renders a JSON null as SQL NULL — indistinguishable from a missing | ||
| # key. Stored configs do carry explicit nulls for keys the user never | ||
| # configured (see WorkflowConfigurationDefaults._treat_null_as_unset), | ||
| # so the key check uses `->`, which returns the JSON value and is SQL | ||
| # NULL only when the key is genuinely absent. | ||
| rows = conn.execute( | ||
| sa.text( | ||
| f"SELECT id, {column} FROM {table} " | ||
| f"WHERE {column} IS NOT NULL " | ||
| f"AND ({column}->>'turn_start_strategy' = :retired " | ||
| f" OR {column}->'{_RETIRED_KEY}' IS NOT NULL)" | ||
| ), | ||
| {"retired": _RETIRED_STRATEGY}, | ||
| ).fetchall() | ||
|
|
||
| for row_id, raw in rows: | ||
| config = json.loads(raw) if isinstance(raw, str) else raw | ||
| if not isinstance(config, dict) or not _rewrite(config): | ||
| continue | ||
| conn.execute( | ||
| sa.text(f"UPDATE {table} SET {column} = :cfg WHERE id = :id"), | ||
| {"cfg": json.dumps(config), "id": row_id}, | ||
| ) | ||
|
|
||
|
|
||
| def downgrade() -> None: | ||
| # One-way: the retired strategy no longer exists in the application, so | ||
| # restoring the value would leave rows the code cannot honour. Workflows | ||
| # that were on it now read as `default`, which is a valid configuration. | ||
| pass | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,6 @@ | |
| from api.schemas.workflow_configurations import ( | ||
| DEFAULT_MAX_CALL_DURATION_SECONDS, | ||
| DEFAULT_MAX_USER_IDLE_TIMEOUT_SECONDS, | ||
| DEFAULT_PROVISIONAL_VAD_PAUSE_SECS, | ||
| DEFAULT_SMART_TURN_STOP_SECS, | ||
| DEFAULT_TURN_START_MIN_WORDS, | ||
| DEFAULT_TURN_START_STRATEGY, | ||
|
|
@@ -104,7 +103,6 @@ | |
| from pipecat.turns.user_start import ( | ||
| ExternalUserTurnStartStrategy, | ||
| MinWordsUserTurnStartStrategy, | ||
| ProvisionalVADUserTurnStartStrategy, | ||
| ) | ||
| from pipecat.turns.user_start.transcription_user_turn_start_strategy import ( | ||
| TranscriptionUserTurnStartStrategy, | ||
|
|
@@ -197,22 +195,20 @@ def _resolve_turn_start_min_words(run_configs: dict) -> int: | |
| ) | ||
|
|
||
|
|
||
| def _resolve_provisional_vad_pause_secs(run_configs: dict) -> float: | ||
| return max( | ||
| 0.1, | ||
| float( | ||
| run_configs.get( | ||
| "provisional_vad_pause_secs", DEFAULT_PROVISIONAL_VAD_PAUSE_SECS | ||
| ) | ||
| ), | ||
| ) | ||
|
|
||
|
|
||
| def _create_non_realtime_user_turn_start_strategies( | ||
| run_configs: dict, *, uses_external_turns: bool | ||
| ): | ||
| """Return user turn start strategies for non-realtime pipelines.""" | ||
|
|
||
| # An STT that reports its own turn boundaries decides the turn start, | ||
| # whatever `turn_start_strategy` asks for. | ||
| # | ||
| # Local VAD is deliberately kept out of these start strategies too: it would | ||
| # win the race on raw voice activity and start the turn before the STT | ||
| # confirms a real turn. | ||
| if uses_external_turns: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: For external-turn STTs, an explicitly configured Prompt for AI agents |
||
| return [ExternalUserTurnStartStrategy(enable_interruptions=True)] | ||
|
|
||
| turn_start_strategy = run_configs.get( | ||
| "turn_start_strategy", DEFAULT_TURN_START_STRATEGY | ||
| ) | ||
|
|
@@ -224,20 +220,6 @@ def _create_non_realtime_user_turn_start_strategies( | |
| ) | ||
| ] | ||
|
|
||
| if turn_start_strategy == "provisional_vad": | ||
| return [ | ||
| ProvisionalVADUserTurnStartStrategy( | ||
| pause_secs=_resolve_provisional_vad_pause_secs(run_configs) | ||
| ), | ||
| ] | ||
|
|
||
| if uses_external_turns: | ||
| # The STT emits its own turn boundaries and owns interruptions. Local | ||
| # VAD is deliberately kept out of the default start strategies: it would | ||
| # win the race on raw voice activity and start the turn before the STT | ||
| # confirms a real turn. | ||
| return [ExternalUserTurnStartStrategy(enable_interruptions=True)] | ||
|
|
||
| return [TranscriptionUserTurnStartStrategy(), VADUserTurnStartStrategy()] | ||
|
|
||
|
|
||
|
|
@@ -1002,9 +984,12 @@ async def send_node_transition( | |
| turn_start_strategy = run_configs.get( | ||
| "turn_start_strategy", DEFAULT_TURN_START_STRATEGY | ||
| ) | ||
| # `requested` is what the workflow asked for; `resolved` is what the | ||
| # pipeline built, which differs whenever external turns override it. | ||
| logger.info( | ||
| f"[run {workflow_run_id}] Non-realtime interrupt strategy " | ||
| f"requested={turn_start_strategy} " | ||
| f"resolved={','.join(type(s).__name__ for s in user_turn_start_strategies)} " | ||
| f"uses_external_turns={uses_external_turns}" | ||
| ) | ||
|
|
||
|
|
||
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3: The migration calls
fetchall()on both target tables and then issues a separateUPDATEper matched row, materializing every matchingworkflow_configurationspayload in memory. With the versionedworkflow_definitionstable this can be thousands of payloads/statements during a locked migration window. Batch the rewrite into a single statement per table using a CASE/JSON rewrite lambda orjsonb_set-style expression (the columns arejson, so a->>+reconstruct in one UPDATE is possible for these flat keys) instead of the fetchall + per-row UPDATE loop.Prompt for AI agents