Multion change transcript to memory #909
Conversation
|
WalkthroughThe changes primarily focus on enhancing the Changes
Uplevel your code reviews with Entelligence.AI ProEntelligence.AI ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, Entelligence.AI Pro is free for open source projects. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Files selected (1)
- plugins/example/_multion/router.py (3)
Files ignored due to filter (1)
- community-plugins.json
Review comments generated (3)
- Review: 2
- LGTM: 1
Tips
Chat with Entelligence.AI Bot (@Entelligence.AI)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@Entelligence.AIin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@Entelligence.AI: ignoreanywhere in the PR description to pause further reviews from the bot.
plugins/example/_multion/router.py
Outdated
| @router.post("/multion", response_model=EndpointResponse, tags=['multion']) | ||
| async def multion_endpoint(memory: Memory, uid: str = Query(...)): | ||
| user_id = db.get_multion_user_id(uid) | ||
| if not user_id: | ||
| raise HTTPException(status_code=400, detail="Invalid UID or USERID not found.") | ||
|
|
||
| session_id = 'multion-' + data.session_id | ||
| db.clean_all_transcripts_except(uid, session_id) | ||
| transcript: List[TranscriptSegment] = db.append_segment_to_transcript(uid, session_id, data.segments) | ||
| transcript_str = TranscriptSegment.segments_as_string(transcript) | ||
| if len(transcript_str) > 100: | ||
| transcript_str = transcript_str[-100:] | ||
|
|
||
| try: | ||
| books = retrieve_books_to_buy(transcript_str) | ||
| if not books: | ||
| return {'message': ''} | ||
| db.remove_transcript(uid, data.session_id) | ||
| result = await asyncio.wait_for(call_multion(books, user_id), timeout=120) | ||
| except asyncio.TimeoutError: | ||
| print("Timeout error occurred") | ||
| db.remove_transcript(uid, data.session_id) | ||
| return | ||
| except Exception as e: | ||
| print(f"Error calling Multion API: {str(e)}") | ||
| db.remove_transcript(uid, data.session_id) | ||
| return | ||
| books = await retrieve_books_to_buy(memory.get_transcript()) | ||
| if not books: | ||
| return EndpointResponse(message='No books were suggested or mentioned.') | ||
|
|
||
| result = await call_multion(books, user_id) | ||
| if isinstance(result, bytes): | ||
| result = result.decode('utf-8') | ||
| return {"message": result} | ||
|
|
||
| return {"message": str(result)} | ||
|
|
||
| # @router.post("/multion", response_model=EndpointResponse, tags=['multion']) | ||
| # async def multion_endpoint(memory: Memory, uid: str = Query(...)): | ||
| # user_id = db.get_multion_user_id(uid) | ||
| # if not user_id: | ||
| # raise HTTPException(status_code=400, detail="Invalid UID or USERID not found.") | ||
| # | ||
| # books = await retrieve_books_to_buy(memory) | ||
| # if not books: | ||
| # return EndpointResponse(message='No books were suggested or mentioned.') | ||
| # | ||
| # result = await call_multion(books, user_id) | ||
| # if isinstance(result, bytes): | ||
| # result = result.decode('utf-8') | ||
| # | ||
| # return EndpointResponse(message=result) | ||
|
|
||
| return EndpointResponse(message=result) |
There was a problem hiding this comment.
The new implementation of multion_endpoint seems to have removed some error handling that was present in the old version. Specifically, the old version handled asyncio.TimeoutError and general exceptions with appropriate logging and cleanup operations. This should be reintroduced to ensure robustness.
Also, the new implementation assumes that call_multion will always return either a string or bytes. If call_multion can potentially return other types (like None or an error object), this could cause issues. It would be better to add a check for the type of result before decoding it.
Here's a suggested fix:
@router.post("/multion", response_model=EndpointResponse, tags=['multion'])
async def multion_endpoint(memory: Memory, uid: str = Query(...)):
user_id = db.get_multion_user_id(uid)
if not user_id:
raise HTTPException(status_code=400, detail="Invalid UID or USERID not found.")
books = await retrieve_books_to_buy(memory.get_transcript())
if not books:
return EndpointResponse(message='No books were suggested or mentioned.')
- 172: result = await call_multion(books, user_id)
+ 172: try:
+ 173: result = await asyncio.wait_for(call_multion(books, user_id), timeout=120)
+ 174: except asyncio.TimeoutError:
+ 175: print("Timeout error occurred")
+ 176: return EndpointResponse(message="Timeout error occurred.")
+ 177: except Exception as e:
+ 178: print(f"Error calling Multion API: {str(e)}")
+ 179: return EndpointResponse(message=f"Error calling Multion API: {str(e)}")
if isinstance(result, bytes):
result = result.decode('utf-8')
return EndpointResponse(message=result)
There was a problem hiding this comment.
Files selected (1)
- plugins/example/_multion/router.py (2)
Review comments generated (2)
- Review: 2
- LGTM: 0
Tips
Chat with Entelligence.AI Bot (@Entelligence.AI)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@Entelligence.AIin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@Entelligence.AI: ignoreanywhere in the PR description to pause further reviews from the bot.
plugins/example/_multion/router.py
Outdated
| @router.post("/multion", response_model=EndpointResponse, tags=['multion']) | ||
| async def multion_endpoint(memory: Memory, uid: str = Query(...)): | ||
| user_id = db.get_multion_user_id(uid) | ||
| if not user_id: | ||
| raise HTTPException(status_code=400, detail="Invalid UID or USERID not found.") | ||
|
|
||
| session_id = 'multion-' + data.session_id | ||
| db.clean_all_transcripts_except(uid, session_id) | ||
| transcript: List[TranscriptSegment] = db.append_segment_to_transcript(uid, session_id, data.segments) | ||
| transcript_str = TranscriptSegment.segments_as_string(transcript) | ||
| if len(transcript_str) > 100: | ||
| transcript_str = transcript_str[-100:] | ||
|
|
||
| try: | ||
| books = retrieve_books_to_buy(transcript_str) | ||
| if not books: | ||
| return {'message': ''} | ||
| db.remove_transcript(uid, data.session_id) | ||
| result = await asyncio.wait_for(call_multion(books, user_id), timeout=120) | ||
| except asyncio.TimeoutError: | ||
| print("Timeout error occurred") | ||
| db.remove_transcript(uid, data.session_id) | ||
| return | ||
| except Exception as e: | ||
| print(f"Error calling Multion API: {str(e)}") | ||
| db.remove_transcript(uid, data.session_id) | ||
| return | ||
| books = retrieve_books_to_buy(memory.get_transcript()) | ||
| if not books: | ||
| return EndpointResponse(message='No books were suggested or mentioned.') | ||
|
|
||
| result = await call_multion(books, user_id) | ||
| if isinstance(result, bytes): | ||
| result = result.decode('utf-8') | ||
| return {"message": result} | ||
|
|
||
| return {"message": str(result)} | ||
|
|
||
| # @router.post("/multion", response_model=EndpointResponse, tags=['multion']) | ||
| # async def multion_endpoint(memory: Memory, uid: str = Query(...)): | ||
| # user_id = db.get_multion_user_id(uid) | ||
| # if not user_id: | ||
| # raise HTTPException(status_code=400, detail="Invalid UID or USERID not found.") | ||
| # | ||
| # books = await retrieve_books_to_buy(memory) | ||
| # if not books: | ||
| # return EndpointResponse(message='No books were suggested or mentioned.') | ||
| # | ||
| # result = await call_multion(books, user_id) | ||
| # if isinstance(result, bytes): | ||
| # result = result.decode('utf-8') | ||
| # | ||
| # return EndpointResponse(message=result) | ||
|
|
||
| return EndpointResponse(message=result) |
There was a problem hiding this comment.
The changes in this hunk are significant. The function signature has been modified to accept a Memory object instead of a RealtimePluginRequest. The logic for retrieving books from the transcript has been simplified by directly calling memory.get_transcript(). Error handling has been improved by adding checks for the absence of books and the type of result. The response model EndpointResponse is now consistently used for all return statements. These changes improve readability and maintainability.
However, there's a potential issue with error handling. In the old code, exceptions during the call to call_multion were caught and handled, but in the new code, these exceptions will propagate up the stack. It might be better to keep the try-except block around the call to call_multion to handle potential errors.
- try:
- books = retrieve_books_to_buy(transcript_str)
- if not books:
- return {'message': ''}
- db.remove_transcript(uid, data.session_id)
- result = await asyncio.wait_for(call_multion(books, user_id), timeout=120)
- except asyncio.TimeoutError:
- print("Timeout error occurred")
- db.remove_transcript(uid, data.session_id)
- return
- except Exception as e:
- print(f"Error calling Multion API: {str(e)}")
- db.remove_transcript(uid, data.session_id)
- return
+ books = retrieve_books_to_buy(memory.get_transcript())
+ if not books:
+ return EndpointResponse(message='No books were suggested or mentioned.')
+
+ try:
+ result = await call_multion(books, user_id)
+ except Exception as e:
+ print(f"Error calling Multion API: {str(e)}")
+ return EndpointResponse(message=f"Error calling Multion API: {str(e)}")
There was a problem hiding this comment.
Files selected (1)
- plugins/example/_multion/router.py (2)
Review comments generated (2)
- Review: 2
- LGTM: 0
Tips
Chat with Entelligence.AI Bot (@Entelligence.AI)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@Entelligence.AIin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@Entelligence.AI: ignoreanywhere in the PR description to pause further reviews from the bot.
| @@ -11,7 +11,7 @@ | |||
| from langchain_groq import ChatGroq | |||
|
|
|||
| import db | |||
| from models import RealtimePluginRequest, TranscriptSegment | |||
| from models import Memory, EndpointResponse | |||
There was a problem hiding this comment.
The import of RealtimePluginRequest, TranscriptSegment has been replaced with Memory, EndpointResponse. This change seems to be in line with the PR's goal of changing the transcript to memory. However, ensure that all instances where RealtimePluginRequest and TranscriptSegment were used have been properly updated to use Memory and EndpointResponse instead.
- from models import RealtimePluginRequest, TranscriptSegment
+ from models import Memory, EndpointResponse| @router.post("/multion", response_model=EndpointResponse, tags=['multion']) | ||
| async def multion_endpoint(memory: Memory, uid: str = Query(...)): | ||
| user_id = db.get_multion_user_id(uid) | ||
| if not user_id: | ||
| raise HTTPException(status_code=400, detail="Invalid UID or USERID not found.") | ||
|
|
||
| session_id = 'multion-' + data.session_id | ||
| db.clean_all_transcripts_except(uid, session_id) | ||
| transcript: List[TranscriptSegment] = db.append_segment_to_transcript(uid, session_id, data.segments) | ||
| transcript_str = TranscriptSegment.segments_as_string(transcript) | ||
| if len(transcript_str) > 100: | ||
| transcript_str = transcript_str[-100:] | ||
| books = retrieve_books_to_buy(memory.get_transcript()) | ||
| if not books: | ||
| return EndpointResponse(message='No books were suggested or mentioned.') | ||
|
|
||
| try: | ||
| books = retrieve_books_to_buy(transcript_str) | ||
| if not books: | ||
| return {'message': ''} | ||
| db.remove_transcript(uid, data.session_id) | ||
| result = await asyncio.wait_for(call_multion(books, user_id), timeout=120) | ||
| except asyncio.TimeoutError: | ||
| print("Timeout error occurred") | ||
| db.remove_transcript(uid, data.session_id) | ||
| return | ||
| return EndpointResponse(message="Timeout error occurred.") | ||
| except Exception as e: | ||
| print(f"Error calling Multion API: {str(e)}") | ||
| db.remove_transcript(uid, data.session_id) | ||
| return | ||
|
|
||
| return EndpointResponse(message=f"Error calling Multion API: {str(e)}") | ||
| if isinstance(result, bytes): | ||
| result = result.decode('utf-8') | ||
| return {"message": result} | ||
|
|
||
| return {"message": str(result)} | ||
|
|
||
| # @router.post("/multion", response_model=EndpointResponse, tags=['multion']) | ||
| # async def multion_endpoint(memory: Memory, uid: str = Query(...)): | ||
| # user_id = db.get_multion_user_id(uid) | ||
| # if not user_id: | ||
| # raise HTTPException(status_code=400, detail="Invalid UID or USERID not found.") | ||
| # | ||
| # books = await retrieve_books_to_buy(memory) | ||
| # if not books: | ||
| # return EndpointResponse(message='No books were suggested or mentioned.') | ||
| # | ||
| # result = await call_multion(books, user_id) | ||
| # if isinstance(result, bytes): | ||
| # result = result.decode('utf-8') | ||
| # | ||
| # return EndpointResponse(message=result) | ||
|
|
||
| return EndpointResponse(message=result) |
There was a problem hiding this comment.
The changes made in this hunk are significant and improve the function's modularity and error handling. The function now accepts a Memory object instead of a transcript string or RealtimePluginRequest, which simplifies the logic for retrieving books from the transcript. The error handling mechanisms have been enhanced, and the response model is consistently used throughout the function. However, there are still some improvements that can be made:
-
Line 175 and 179: Instead of just printing the errors, it would be better to log them using a logging library. This will provide more control over the output format, destination, and level of detail.
-
Line 168: Ensure that the
retrieve_books_to_buyfunction can handle the input frommemory.get_transcript(). If the function was originally designed to work with a different data type (like a string), it may need to be updated or replaced. -
Line 173: The timeout value of 120 seconds is hardcoded. It would be better to make this a configurable parameter, allowing it to be easily adjusted based on the environment or use case.
Here are the suggested changes:
- try:
- result = await asyncio.wait_for(call_multion(books, user_id), timeout=120)
- except asyncio.TimeoutError:
- print("Timeout error occurred")
- return EndpointResponse(message="Timeout error occurred.")
- except Exception as e:
- print(f"Error calling Multion API: {str(e)}")
- return EndpointResponse(message=f"Error calling Multion API: {str(e)}")
+ try:
+ result = await asyncio.wait_for(call_multion(books, user_id), timeout=CONFIG.TIMEOUT)
+ except asyncio.TimeoutError:
+ logger.error("Timeout error occurred")
+ return EndpointResponse(message="Timeout error occurred.")
+ except Exception as e:
+ logger.error(f"Error calling Multion API: {str(e)}")
+ return EndpointResponse(message=f"Error calling Multion API: {str(e)}")Where CONFIG.TIMEOUT is a new configuration parameter and logger is an instance of a logger from a logging library.
Lets pray it does not explode
Summary by Entelligence.AI