Skip to content

fix(FunctionResult): tap direction is speak/listen/both, and always emitted - #86

Closed
grandcamel wants to merge 1 commit into
signalwire:mainfrom
grandcamel:fix/tap-direction
Closed

fix(FunctionResult): tap direction is speak/listen/both, and always emitted#86
grandcamel wants to merge 1 commit into
signalwire:mainfrom
grandcamel:fix/tap-direction

Conversation

@grandcamel

Copy link
Copy Markdown
Contributor

Summary

FunctionResult.tap() disagreed with the SWML schema (and the live platform docs, https://signalwire.com/docs/swml/reference/tap) in two ways, both of which made it emit taps that don't do what the caller asked:

  1. "hear" is not a SWML direction. The verb's enum is speak | listen | both; the helper accepted and emitted "hear". A tap(uri, direction="hear") produced SWML the platform rejects. record_call() in the same class already uses the correct ("speak", "listen", "both") set — tap was the outlier.

  2. The omit-if-default logic assumed the platform default is "both"; it's "speak". The helper only emitted direction when it differed from its own "both" default, so tap(uri) — documented as tapping both directions — actually produced a speak-only tap. The codec/rtp_ptime omissions are unaffected (their helper defaults match the schema defaults).

Fix

  • direction is now Literal["speak", "listen", "both"], matching record_call and the schema. "hear" now raises the existing ValueError — it could never produce a working tap, so no working caller regresses; a broken silent tap becomes a loud error.
  • direction is always emitted, so the helper's documented "both" default is what the platform actually executes.
  • docs/swaig_reference.md and docs/api_reference.md updated to match (api_reference had a third wrong enum — "inbound"/"outbound" — plus a G722 codec and sip: URIs the helper never accepted).

Behavior note (please review)

For existing callers of tap(uri) with no explicit direction, emitted SWML changes from (no direction ⇒ platform speak-only) to an explicit "direction": "both" — i.e. behavior now matches what the helper has always documented. If anyone has been relying on the accidental speak-only behavior, this PR changes what their tap captures. The alternative (documenting "speak" as the default instead) preserves accidental behavior but breaks the documented contract; happy to switch if that's preferred.

Cross-repo note

tap.direction's Literal feeds the port-audit oracle (python_signatures.json in porting-sdk) as an enum<...>; the oracle needs a regen alongside this change (test_validated_closed_sets_declared_as_literal pins the tuple in-repo and is updated here).

🤖 Generated with Claude Code

https://claude.ai/code/session_01MgA3KeCEPMKMJVvroZY1wV

…mitted

The tap() helper disagreed with the SWML schema (and the platform docs)
twice over. It accepted and emitted "hear", which is not a tap
direction — the verb's enum is speak/listen/both, exactly the set
record_call() already uses — so direction="hear" produced SWML the
platform rejects; it now raises the existing ValueError instead of
silently emitting a dead tap. And its omit-if-default logic assumed the
verb's default matches the helper's "both" when the verb actually
defaults to "speak", so tap(uri) — documented as tapping both
directions — produced a speak-only tap. direction is now always
emitted; codec/rtp_ptime omissions stay (their helper defaults match
the verb defaults).

Note for the port audit: tap.direction's Literal feeds
python_signatures.json in porting-sdk as enum<...>, so the oracle needs
a regen alongside this change.

docs/swaig_reference.md and docs/api_reference.md updated to the real
enum (api_reference had a third variant — inbound/outbound plus a G722
codec and sip: URIs that the helper never accepted).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MgA3KeCEPMKMJVvroZY1wV
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.

1 participant