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

Visualizations updates are not stable and leaving visualizations blank #6561

Closed
7 tasks
sylwiabr opened this issue May 4, 2023 · 31 comments · Fixed by #6831
Closed
7 tasks

Visualizations updates are not stable and leaving visualizations blank #6561

sylwiabr opened this issue May 4, 2023 · 31 comments · Fixed by #6831
Assignees
Labels
--bug Type: bug p-highest Should be completed ASAP

Comments

@sylwiabr
Copy link
Member

sylwiabr commented May 4, 2023

Discord username

No response

What type of issue is this?

Transient – Occurring only once

Is this issue blocking you from using Enso?

  • Yes, I can't use Enso because of this issue.

Is this a regression?

  • Yes, previous version of Enso did not have this issue.

What issue are you facing?

During building up use-cases and demos we observed the decrease in visualizations stability. That include:

  • visualization is blanked after finishing editing a node
  • blank visualizations after nodes collapsing,
  • blank visualization on an existing node with viz opened while uploading the new file,
  • no update for the full screen visualization if it was opened before IDE got the data - it is never updated and needs reopening,
  • clicking on the drop-down to select a visualization is vanishing the visualization.

Expected behaviour

Visualizations should be updated with the data as soon as data are available.

How we can reproduce it?

No response

Screenshots or screencasts

No response

Enso Version

nightly 04.05

Browser or standalone distribution

Standalone distribution

Browser Version or standalone distribution

standalone

Operating System

MacOS

Operating System Version

No response

Hardware you are using

No response

@sylwiabr sylwiabr added --bug Type: bug triage labels May 4, 2023
@github-project-automation github-project-automation bot moved this to ❓New in Issues Board May 4, 2023
@sylwiabr sylwiabr added the p-highest Should be completed ASAP label May 4, 2023
@4e6 4e6 self-assigned this May 4, 2023
@wdanilo
Copy link
Member

wdanilo commented May 4, 2023

Question – we had so many issues about it already. Do we have any insights why it is happening? Why can't we just fix the GUI - backend communication once for all and prevent such issues from happening ever again? Basically, my question here is - what is exactly the cause of this / similar to this issues? Is it some kind of race condition / bugs in communication / something else?

CC @4e6 as you'd probably know the answer to these questions :)

@4e6
Copy link
Contributor

4e6 commented May 5, 2023

The IDE has basically two feedback channels from the engine - expression and visualization updates. And when anything goes wrong, the user can only observe it by seeing the wrong updates. Here are the last issues related to visualizations

All causes were completely different

@4e6
Copy link
Contributor

4e6 commented May 5, 2023

The issue I can reproduce on the current develop seems like on the IDE side

Peek.2023-05-05.15-37.mp4

@4e6 4e6 added the -gui label May 5, 2023
@sylwiabr
Copy link
Member Author

sylwiabr commented May 5, 2023

@4e6 thank you for checking. Package from 29.04 does not have those issues, package from 02.05 does. This means the regression was caused by #6458 or #6461. As #6461 was just touching debug scenes not application probably the issue was introduced with #6458 - @kazcw can you have a look at this?

@kazcw
Copy link
Contributor

kazcw commented May 5, 2023

Looking into it.

@kazcw kazcw assigned kazcw and unassigned 4e6 and farmaazon May 5, 2023
@4e6
Copy link
Contributor

4e6 commented May 5, 2023

May be related #6526

@kazcw
Copy link
Contributor

kazcw commented May 5, 2023

The bug still occurs on develop after reverting #6458 (and dependent PR #6569).

I suspect it's older than 02.05. The bug is sensitive to the exact testing procedure: Immediately after creating a new node the bug is present when editing the node, but if I open a project and edit an already existing node the bug isn't triggered. (This made attempts to narrow down the cause "interesting").

@kazcw kazcw assigned farmaazon and unassigned kazcw May 5, 2023
@vitvakatu vitvakatu moved this from ❓New to 📤 Backlog in Issues Board May 8, 2023
@MichaelMauderer MichaelMauderer moved this from 📤 Backlog to 🔧 Implementation in Issues Board May 8, 2023
@enso-bot
Copy link

enso-bot bot commented May 8, 2023

Michael Mauderer reports a new STANDUP for today (2023-05-08):

Progress: Started investigating why visualizations are blank by reproducing the issue and adding more logging. It should be finished by 2023-05-10.

Next Day: Next day I will be working on the #6561 task. Continue investigation Address question about recurring issues around visualisations.

@enso-bot
Copy link

enso-bot bot commented May 10, 2023

Michael Mauderer reports a new STANDUP for yesterday (2023-05-09):

Progress: Found that the blank visualization is caused by a spurious FRP update that sets the visualization path and thus re-creates the visualization. But since the node is not re-evaluated as it was already evaluated for preview, the engine sends no data update. It should be finished by 2023-05-10.

Next Day: Next day I will be working on the #6561 task. Check if this solves all described scenarios. Investigate missing colouring that also happens in this scenario.

@enso-bot
Copy link

enso-bot bot commented May 15, 2023

Michael Mauderer reports a new STANDUP for today (2023-05-15):

Progress: Still investigating the missing data update when full-screen visualization is opened before data arrives. So far it seems to not only be related to FRP sheninigans but an inconsistent state in the controller: the visualisaiton seems to dissapear from the execution context just before it is meant to receive an update. But I am not sure why yet. It should be finished by 2023-05-15.

Next Day: Next day I will be working on the #6561 task. Investigate remaining issues.

@enso-bot
Copy link

enso-bot bot commented May 16, 2023

Michael Mauderer reports a new 🔴 DELAY for today (2023-05-16):

Summary: There is 4 days delay in implementation of the Visualizations updates are not stable and leaving visualizations blank (#6561) task.
It will cause 0 days delay for the delivery of this weekly plan.

Delay Cause: Multiple bugs on this issue. The current investigation also seems to be caused by multiple issues: (1) a rare crash of the table visualization (2) a race condition that causes the visualization not to be parented correctly (3) a race condition that sometimes removes the visualization.

Possible solutions: Implement easier bugs.

@enso-bot
Copy link

enso-bot bot commented May 16, 2023

Michael Mauderer reports a new STANDUP for today (2023-05-16):

Progress: Still investigating the missing data update when full-screen visualization is opened before data arrives. So far, I solved an issue that the visualizations sometimes was not parented to the correct layer due to a race condition. Still investigating an issue where the visualization is not correctly restored after resetting it just before going full-screen. It should be finished by 2023-05-19.

Next Day: Next day I will be working on the #6561 task. Investigate remaining issues.

mergify bot pushed a commit that referenced this issue May 16, 2023
Addresses the issue described here: #6561 (comment) .
This was caused by FRP events that would re-set the visualization path when ending the node editing. This would eventually clear the visualization before setting it again, losing the already received data.


https://github.com/enso-org/enso/assets/1428930/6e324ddf-f365-48b8-bb2a-c68b2fbd24ef

also addresses the issue described here #6561 (comment)


https://github.com/enso-org/enso/assets/1428930/437f7822-7c35-48ba-a055-59d6f712a813

Note that now the default visualization is already shown on the first hover of the action bar, where before it was empty. This was caused by a faulty initialization.
@enso-bot
Copy link

enso-bot bot commented May 18, 2023

Michael Mauderer reports a new STANDUP for yesterday (2023-05-17):

Progress: After fixing some FRP bugs in the visualization pipeline, I found an issue with the controller not emitting the correct events during full-screen visualization. Investigating root cause. It should be finished by 2023-05-19.

Next Day: Next day I will be working on the #6561 task. Investigate remaining issues.

@enso-bot
Copy link

enso-bot bot commented May 22, 2023

Michael Mauderer reports a new STANDUP for the provided date (2023-05-19):

Progress: After some more investigation, I found that the actual root cause was a logic bug with the display/hide button, which w\s not triggered correctly when going full-screen. Starting to implement a + fix + some refactoring that will simplify the FRP logic around showing/hiding visualizations. It should be finished by 2023-05-19.

Next Day: Next day I will be working on the #6561 task. Investigate remaining issues.

@enso-bot
Copy link

enso-bot bot commented May 23, 2023

Michael Mauderer reports a new 🔴 DELAY for today (2023-05-23):

Summary: There is 3 days delay in implementation of the Visualizations updates are not stable and leaving visualizations blank (#6561) task.
It will cause 0 days delay for the delivery of this weekly plan.

Delay Cause: Multiple bugs on this issue caused by excess complexity in the FRP networks around the visualization logic. Logic is distributed around multiple components (action bar / nodes / graph editor) and thus prone to breaking as different usages make different assumptions. Cleaning this up should decrease future maintenance burden and bugs in this code.

Possible solutions: Implement easier bugs.

@enso-bot
Copy link

enso-bot bot commented May 23, 2023

Michael Mauderer reports a new 🔴 DELAY for today (2023-05-23):

Summary: There is 2 days delay in implementation of the Visualizations updates are not stable and leaving visualizations blank (#6561) task.
It will cause 0 days delay for the delivery of this weekly plan.

Delay Cause: Forgot to account for weekend days in the last delay.

@enso-bot
Copy link

enso-bot bot commented May 23, 2023

Michael Mauderer reports a new STANDUP for yesterday (2023-05-22):

Progress: Continued work on fixing FRP issues and simplifying the logic. Found an existing issue with the show/hide buttons not distinguishing between API events and user events, which sometimes lead to multiple show/hide events for visualizations. It should be finished by 2023-05-24.

Next Day: Next day I will be working on the #6561 task. Open PR for this round of fixes.

MichaelMauderer added a commit that referenced this issue May 24, 2023
…as well as some other undiscovered issues around the order of events that arrive at the visualisation container.
MichaelMauderer added a commit that referenced this issue May 24, 2023
…as well as some other undiscovered issues around the order of events that arrive at the visualisation container.
@enso-bot
Copy link

enso-bot bot commented May 24, 2023

Michael Mauderer reports a new STANDUP for yesterday (2023-05-23):

Progress: Finalised the changes cleaning up code and testing against regressions. It should be finished by 2023-05-24.

Next Day: Next day I will be working on the #6561 task. Starting next task.

MichaelMauderer added a commit that referenced this issue May 25, 2023
…as well as some other undiscovered issues around the order of events that arrive at the visualisation container.
@farmaazon farmaazon linked a pull request May 25, 2023 that will close this issue
5 tasks
@farmaazon farmaazon moved this from 🔧 Implementation to 👁️ Code review in Issues Board May 25, 2023
@MichaelMauderer MichaelMauderer moved this from 👁️ Code review to 🌟 Q/A review in Issues Board May 25, 2023
@MichaelMauderer MichaelMauderer moved this from 🌟 Q/A review to 👁️ Code review in Issues Board May 25, 2023
@enso-bot
Copy link

enso-bot bot commented May 25, 2023

Michael Mauderer reports a new STANDUP for yesterday (2023-05-24):

Progress: When testing, I noticed another regression that is now fixed. Opened PR. and started checking other open issues that could be resolved by this PR. It should be finished by 2023-05-24.

Next Day: Next day I will be working on the #6561 task. Starting next task.

@farmaazon farmaazon moved this from 👁️ Code review to 🌟 Q/A review in Issues Board May 26, 2023
@farmaazon farmaazon moved this from 🌟 Q/A review to 🔴 Changes requested in Issues Board Jun 1, 2023
@MichaelMauderer MichaelMauderer moved this from 🔴 Changes requested to 🌟 Q/A review in Issues Board Jun 1, 2023
@MichaelMauderer MichaelMauderer moved this from 🌟 Q/A review to 🟢 Accepted in Issues Board Jun 2, 2023
mergify bot pushed a commit that referenced this issue Jun 5, 2023
Fixes
* Empty Visualization when opening a full-screen visualization directly without opening the visualization before. #6770

https://github.com/enso-org/enso/assets/1428930/5812ed03-652c-4a27-8e33-b85512ca11b6

* Empty visualization when opening the full-screen visualization before the data for the visualization has arrived. #6561

https://github.com/enso-org/enso/assets/1428930/d8e58f2d-f1b6-4b70-84fa-e917f6c0af1f

* Visualization is reset to default when reconnecting nodes #6673

https://github.com/enso-org/enso/assets/1428930/ac6cf79a-7147-4f13-9045-52599fb39900


* Redundant internal open/lose events caused by logic loops around the show/hide button, as well as many redundant layer setting/unsetting issues internal to the visualization code.

Generally improves the logic around the visualization API by avoiding decentralized logic in different places and removing old code that is no longer needed.
@mergify mergify bot closed this as completed in #6831 Jun 5, 2023
@farmaazon farmaazon moved this from 🟢 Accepted to 🗄️ Archived in Issues Board Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
--bug Type: bug p-highest Should be completed ASAP
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants