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

New bundle is added at a random place #3989

Closed
9 tasks done
pranavjain opened this issue Feb 7, 2022 · 25 comments · Fixed by #4168
Closed
9 tasks done

New bundle is added at a random place #3989

pranavjain opened this issue Feb 7, 2022 · 25 comments · Fixed by #4168
Assignees
Labels
frontend p2 Do it this quarter.

Comments

@pranavjain
Copy link
Contributor

pranavjain commented Feb 7, 2022

Implementation

  • Prevent a worksheet row from having a null sort key when rows are created using the cli
    • Note: cl commands should always add bundles to the end of the worksheet
  • Check for null sort keys when we initially fetch a worksheet and fix them on the spot if needed
  • Code documentation
  • Write tests if needed
  • QA
    • Ensure that source editing is not impacted
    • Test large worksheets that have lots of rows with null sort keys
    • Test a worksheet with a mixture of bundles created in web and bundles created in CLI
    • Remove test console logs

Background

The bundle should be added at the same place where the user has selected. Currently, it is getting placed at random places in the worksheet.

Screen.Recording.2022-02-07.at.1.02.48.PM.mov
@pranavjain
Copy link
Contributor Author

Estimate: 02/14

@pranavjain
Copy link
Contributor Author

@jzwang43 Is there any update on this one?

@jzwang43
Copy link
Contributor

I have narrowed it down to after_sort_key and getAfterSortKey function. The after_sort_key determines where the new run should be inserted. Still working on figuring out why this after_sort_key is incorrect.

@jzwang43
Copy link
Contributor

jzwang43 commented Feb 17, 2022

looks like sometimes sort key is set to null for some rows:

image

This is returned from the backend. could it be some issue with the backend? The api is rest/interpret/worksheet/

@epicfaace @pranavjain do you have any insights on this?

@epicfaace
Copy link
Member

@jzwang43 Can you please send me the worksheet link?

@jzwang43
Copy link
Contributor

@jzwang43 Can you please send me the worksheet link?

@epicfaace try 0x411c4f80346b4732a1aacb8402ec4d04 on codalab.worksheets.org. It's also pretty easy to reproduce locally.

@epicfaace
Copy link
Member

I'm not sure why this is happening -- would take more time to debug

@epicfaace
Copy link
Member

I'm not prioritizing working on this atm, but let me know if I should

@epicfaace
Copy link
Member

@jzwang43 is this something maybe you'd be able to debug? I'm not very familiar with the sort key logic, if you have bandwidth could you take some time and step through the code to figure out what's happening?

@leilenah
Copy link
Collaborator

leilenah commented Jun 23, 2022

Investigation

  • view affected is worksheet view
  • the user should be able to select and determine where the bundle is to be added
  • likely an issue with after_sort_key and getAfterSortKey function
    • The after_sort_key determines where the new run should be inserted
  • sometimes sort key is set to null for some rows
    • potentially could be an issue with the way the backend is sending data over
    • we could adjust that, or re-sort on the frontend, TBD

@leilenah
Copy link
Collaborator

Unable to Repro

@pranavjain @epicfaace I am unable to reproduce this bug. Are either of you able to repro on your end? I'm wondering if this was fixed with some other issue.

Screen Recording

Screen.Recording.2022-06-23.at.11.10.25.AM.mov

@leilenah
Copy link
Collaborator

@percyliang do you happen to know if this was resolved with a different issue? Are you able to repro on your end?

@jzwang43
Copy link
Contributor

jzwang43 commented Jun 25, 2022 via email

@leilenah
Copy link
Collaborator

@jzwang43 I just attempted that flow and am still unable to repro. Are you able to point me to a worksheet on production where this is happening?

@leilenah
Copy link
Collaborator

Removing myself from this issue for now, as I am unable to repro it. If anyone is able to repro or if a user reports this issue, feel free to re-assign me.

@leilenah leilenah removed their assignment Jun 28, 2022
@epicfaace
Copy link
Member

@jzwang43 Can you please send me the worksheet link?

@epicfaace try 0x411c4f80346b4732a1aacb8402ec4d04 on codalab.worksheets.org. It's also pretty easy to reproduce locally.

@leilenah did you try this worksheet?

If you can't reproduce, please close this issue.

@leilenah
Copy link
Collaborator

@epicfaace I don't have access to modify that worksheet. Are you able to grant me access?

@epicfaace
Copy link
Member

@pranavjain pranavjain added this to the User Experience milestone Jun 29, 2022
@pranavjain pranavjain removed the active label Jun 29, 2022
@leilenah leilenah self-assigned this Jun 29, 2022
@pranavjain
Copy link
Contributor Author

@leilenah, I was able to repro it. Please select a bundle on your bottom sheet before clicking on the run button. This new bundle should be added below the selected one, but it goes to a random place.

@leilenah
Copy link
Collaborator

leilenah commented Jul 4, 2022

@pranavjain @epicfaace below is a screen recording of me trying to repro the issue on worksheet 0x411c4f80346b4732a1aacb8402ec4d04 while logged in as the codalab admin user. I am still unable to repro (even with Pranav's latest instructions).

I'm going to close this issue for now. Please let me know if anyone comes across this issue again.

Screen Recording

Screen.Recording.2022-07-03.at.6.53.31.PM.mov

@leilenah leilenah closed this as completed Jul 4, 2022
@jzwang43 jzwang43 reopened this Jul 4, 2022
@jzwang43
Copy link
Contributor

jzwang43 commented Jul 4, 2022

@leilenah
only the last two items in the worksheet have issues, try clicking on one of the last two items. You will see the new run show up above them instead of below

@jzwang43
Copy link
Contributor

jzwang43 commented Jul 4, 2022

@leilenah The reason why this happens is that the items have sort-key as "null", check out my previous pull request for this issue and the conversations for more context.

https://worksheets.codalab.org/rest/interpret/worksheet/0x411c4f80346b4732a1aacb8402ec4d04?brief=1

This is the API request for the worksheet items and then search for sort_key and you will see the nulls.

@leilenah
Copy link
Collaborator

leilenah commented Jul 4, 2022

@jzwang43 here's an updated screen recording of me only creating runs below the last two items. It looks like they appear in the correct place. Am I missing a step? Or perhaps am I misunderstanding where the bundles should end up in the UI?

Browser Used: Chrome
Worksheet Used: https://worksheets.codalab.org/worksheets/0x411c4f80346b4732a1aacb8402ec4d04

Screen.Recording.2022-07-04.at.11.44.08.AM.mov

@jzwang43
Copy link
Contributor

jzwang43 commented Jul 4, 2022

@leilenah Hi, when you were trying to reproduce, there were no null sort_keys in the worksheet for some reason.
Sort keys were created by creating run bundles via CLI.

I created several null sort key items at the bottom, you can try again to reproduce, or manually create null sort_keys items by running cl run "xxxxx" in the command line on the web UI.

Screen.Recording.2022-07-04.at.12.51.38.PM.mov

@leilenah
Copy link
Collaborator

leilenah commented Jul 4, 2022

Ah, thanks for the additional context @jzwang43.

@leilenah leilenah added p2 Do it this quarter. and removed p1 Do it in the next two weeks. labels Jul 4, 2022
@leilenah leilenah removed their assignment Jul 6, 2022
@leilenah leilenah self-assigned this Jul 20, 2022
@leilenah leilenah changed the title New bundle is added at the random place New bundle is added at a random place Jul 27, 2022
@leilenah leilenah linked a pull request Aug 1, 2022 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment