add modality-aware Instructions with audio/text variants#4987
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
chenghao-mou
left a comment
chenghao-mou
left a comment
There was a problem hiding this comment.
LGTM. Tested locally with different modalities.
| """ | ||
| return self._text_variant if self._text_variant is not None else self._audio_variant | ||
|
|
||
| def for_modality(self, modality: Literal["audio", "text"]) -> Instructions: |
There was a problem hiding this comment.
nit: should we use to_modality, similar to our to_provider_format?
There was a problem hiding this comment.
the for_ prefix seems unusual. as_ or to_ are a bit more pythonian.
There was a problem hiding this comment.
renamed to as_modality
davidzhao
left a comment
davidzhao
left a comment
There was a problem hiding this comment.
nice! this feels pretty clean
| """ | ||
| return self._text_variant if self._text_variant is not None else self._audio_variant | ||
|
|
||
| def for_modality(self, modality: Literal["audio", "text"]) -> Instructions: |
There was a problem hiding this comment.
the for_ prefix seems unusual. as_ or to_ are a bit more pythonian.
| async def book_appointment(self, date: str, time: str) -> None: | ||
| """Book an appointment. | ||
|
|
||
| Args: | ||
| date: The date of the appointment in the format YYYY-MM-DD | ||
| time: The time of the appointment in the format HH:MM | ||
| """ | ||
| logger.info(f"booking appointment for {date} at {time}") | ||
| return f"Appointment booked for {date} at {time}" |
There was a problem hiding this comment.
🔴 book_appointment return type annotation is None but function returns a string
The book_appointment method at line 71 declares -> None but actually returns f"Appointment booked for {date} at {time}" at line 79. Because the function_tool decorator inspects the return type annotation to decide how to handle tool output, a -> None annotation may cause the framework to discard the return value, meaning the LLM never receives the booking confirmation string. This would make the agent unable to confirm to the user that the booking succeeded.
| async def book_appointment(self, date: str, time: str) -> None: | |
| """Book an appointment. | |
| Args: | |
| date: The date of the appointment in the format YYYY-MM-DD | |
| time: The time of the appointment in the format HH:MM | |
| """ | |
| logger.info(f"booking appointment for {date} at {time}") | |
| return f"Appointment booked for {date} at {time}" | |
| @function_tool | |
| async def book_appointment(self, date: str, time: str) -> str: | |
| """Book an appointment. | |
| Args: | |
| date: The date of the appointment in the format YYYY-MM-DD | |
| time: The time of the appointment in the format HH:MM | |
| """ | |
| logger.info(f"booking appointment for {date} at {time}") | |
| return f"Appointment booked for {date} at {time}" |
Was this helpful? React with 👍 or 👎 to provide feedback.
3 participants
No description provided.