Skip to content
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

update transfer call to pass in the TN to transfer to #514

Closed
wants to merge 16 commits into from

Conversation

rjheeta
Copy link
Contributor

@rjheeta rjheeta commented Mar 5, 2024

All TransferCall action to accept to_phone as a parameter so the LLM can feed the TN to transfer to at runtime.

@rjheeta
Copy link
Contributor Author

rjheeta commented Mar 7, 2024

@ajar98 Do you agree with this? Slight modification to allow LLM to pass in the phone number.

…rectly passed to actions

* This uses a more reliable method of obtaining these IDs (via ConversationStateManager)
Copy link
Contributor

@arpagon arpagon left a comment

Choose a reason for hiding this comment

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

LGTM!

@arpagon arpagon requested a review from ajar98 May 18, 2024 00:18
@arpagon arpagon self-assigned this May 18, 2024
Copy link
Contributor

@ajar98 ajar98 left a comment

Choose a reason for hiding this comment

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

@rjheeta actually, I think the user should be able to configure whether or not the LLM can choose the transfer number. Couple of ways to approach this:

  • Make a separate vocode action with this config change (poor DRY)
  • Make the configuration value optional (which would translate to sending GPT a schema where it is not required) - imo too much control to the LLM when this is something that is easily configurable pre call
  • [preferred] somehow making TransferCallParameters only get the property if to_phone is None - likely requires a deeper change to the actions system

@rjheeta
Copy link
Contributor Author

rjheeta commented Jun 3, 2024

@ajar98 Thanks for the feedback! If you're open to giving a bit of guidance / direction on your indicated preferred route, I'm happy to take a stab at it.

@ajar98
Copy link
Contributor

ajar98 commented Jun 3, 2024

@rjheeta awesome! Really excited to work with you on this -

BaseAction has a parameters_type method[0] that is usually implemented statically like this: https://github.com/vocodedev/vocode-python/blob/main/vocode/streaming/action/transfer_call.py#L37

We can override this for TransferCall to point to a TransferCallParameters that has a to_phone or not depending on self.action_config - i.e. if self.action_config.to_phone is None, then use the TransferCallParameters where to_phone is required.

[0] https://github.com/vocodedev/vocode-python/blob/main/vocode/streaming/action/base_action.py#L44-L46

@rjheeta
Copy link
Contributor Author

rjheeta commented Jun 4, 2024

@ajar98 Ajay, I just pushed an update. Could you have a look and advise if this is what you had in mind? Or let me know where/how I can improve it.

Copy link
Contributor

@ajar98 ajar98 left a comment

Choose a reason for hiding this comment

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

hey @rjheeta , this is exactly what I had in mind 🎉

mind fixing the merge conflict? then should be g2g :)

@rjheeta rjheeta force-pushed the extend-transfercall-action branch from a445ce1 to e79522a Compare June 14, 2024 20:29
@rjheeta rjheeta closed this Jun 14, 2024
@rjheeta
Copy link
Contributor Author

rjheeta commented Jun 14, 2024

my history got mangled. Will re-do this on new branch

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.

4 participants