OpenAdaptAI / OpenAdapt

AI-First Process Automation with Large ([Language (LLMs) / Action (LAMs) / Multimodal (LMMs)] / Visual Language (VLMs)) Models
https://www.OpenAdapt.AI
MIT License
735 stars 95 forks source link

Fix incorrect argument when visualizing from tray #780

Closed shashank40 closed 1 week ago

shashank40 commented 1 week ago

What kind of change does this PR introduce? Fixing user app replay

Summary When replaying the video in user app, it always fails with error type int as no value id, reason being that recording_id was passed to recording. Fixed that in this PR

Checklist

How can your code be run and tested?

Other information

shashank40 commented 1 week ago

@abrichr Can you please review this PR? I have explained the bug above that was causing replay errors in latest build

abrichr commented 1 week ago

@shashank40 can you please clarify exactly what error you were experiencing before this change? Console output and screenshots would be very helpful 🙏

shashank40 commented 1 week ago

@abrichr So basically when we click on replay recordings from the app-tray, our visualize function gets an argument recording_id, but out function definition has recording(of type Recording as the first argument)

Now due to this type mismatch, we tend to fetch id(or any other attribute) from variable of type Recording, but as recording_id(of type int) instead of recording, recording.id failed with an error type int does not have id.

Now after moving recording_id variable up in visualize main function, recording_id will be passed correctly and the app would work correctly.

Screenshot here :

image
abrichr commented 1 week ago

Thank you for clarifying @shashank40 !

I believe the fix should be here:

https://github.com/OpenAdaptAI/OpenAdapt/blob/main/openadapt/app/tray.py#L247

If you undo your changes to visualize.py, and modify the above line in tray.py to accept args=(recording) I will be happy to merge this!

shashank40 commented 1 week ago

Done @abrichr

shashank40 commented 1 week ago

Only you can merge the PR @abrichr 😅

Also if you can take a look at my other 2 PRs. Will merge these 2 functionalities as well 1) https://github.com/OpenAdaptAI/OpenAdapt/pull/771 2) https://github.com/OpenAdaptAI/OpenAdapt/pull/782

abrichr commented 1 week ago

@shashank40 was just waiting for tests to run, then got distracted 😅 . Will merge ASAP.