#new TUI feedback

1 messages · Page 1 of 1 (latest)

modest hedge
#

Goals: keep it light, minimize bells and whistles, low CPU usage. When everything is done, get out of your way and just print Unixy output.

#

Feels a bit noisier than I'd like, I think mostly due to the addition of the foo.bar.baz. chains, but without them it's not really clear what's actually running. thinkspin They can also get pretty long which ain't great.

#

Also tried rendering those on their own lines, but the verbosity escalates pretty quickly

modest hedge
fair frigate
#

Do you have any example of what happens when some failure occurs? I would love to see it treat that part better.

modest hedge
#

Sure - if you have a specific failure example I'm happy to run it too

modest hedge
#

Changed it to show failed pipelines on exit, and changed the Go SDK to silence the inner error since it will have already manifested itself elsewhere: https://asciinema.org/a/8PPe3AZkhyfoNMEPugwWjxZ7m
Note: this 'show failed pipelines' only applies to top-level pipelines; they won't bubble-up out of context. If the top-level call swallows the error, it won't be shown.

#

Would be nice to just hide these exec /runtime vertices entirely

modest hedge
#

Ok, cleaned things up a bit more - thanks for the suggestion @fair frigate, this was a goldmine of things to fix! Now is a great time to focus on error messages.

  1. Fixed an issue that was causing the API error message to be printed twice
  2. Replaced split Stdout:/Stderr: with a single combined Output: which should be more grokkable, and looks less cluttered. We've wanted that as an API for a while, it just never got triaged, might add that too now that the plumbing is done.
    https://asciinema.org/a/IzDNOR0siq1cQTgiezzrzsBg1
proven spire
#

Looking great! Love the errors output. My personal feedback from what I see.

  1. The timers on every step(vertex?) is overwhelming. I may only care about the total time (top level timer) and maybe the timer on the current step. Not necessarily parent steps. Maybe showing just seconds instead of seconds and milis my make this better? It's rare that things actually happen in milliseconds in CI/CD pipelines these days for it to matter. Seconds are good enough IMO. When the step stops you can always print the secs & millis.

  2. Same as above for the left spinning animations. A lot of things spinning on the output is distracting to me. I am trying to focus on the output which in itself is moving very fast and the animations in my peripheral view are a distraction.

The more complex the pipeline more animation clutter, might actually not be an issue for quick smaller pipelines.

modest hedge
modest hedge
#

How's this? https://asciinema.org/a/af2FeHdNJ5LsqotNNpLT7AkGB

  • No more spinners
  • Removed [] from the durations so they're less noisy
  • Truncated durations to tenths of seconds so they're less frantic
    Also tried without the timers, but I think I do like having them; otherwise it's hard to tell if something is stuck.
modest hedge
proven spire
# modest hedge How's this? <https://asciinema.org/a/af2FeHdNJ5LsqotNNpLT7AkGB> * No more spinne...

Better!! Much less distraction. When things start up I see a ton of timers still so I can't fully judge by the video. I'll test this myself this week and give more feedback. I think we are getting to a good place. There may be more feedback from other community members.

One thing that's kind of bothering me is there are a lot of flashing logs/info on the screen. I am not referring to the animations but the actual information on the screen. Most of it is flashing too fast for me to gleam what's going on. I know it's by the nature of DAGs and dagger. IDK if there's a good solution either but it's just what I feel. It would be nice to have a "skim" version of the TUI where only the most important pieces are shown. Anything that's going on too fast where it needs to flash for less than a second I don't want to see. Hopefully I'm making sense.

modest hedge
#

Ha that's also something I tried in an earlier iteration, basically if duration is <100ms don't even show it. Will try bringing that back

#

I originally wanted to use that heuristic to trim out boring completed steps, current iteration just doesn't show completed things at all

modern sky
# modest hedge Ha that's also something I tried in an earlier iteration, basically if duration ...

Just to clarify - I think this is maybe fine, but I think we should aim to try and really trim the number of progress events we actually end up getting, so we don't have this problem in the first place.
If the vertex is important, it should be displayed fully (and not go away), if it's not, it should never even be shown on screen (unless it fails). The flicker of "not quite important info" is distracting, and this feels like a bit of a workaround from that.

Just thinking out loud, I could be wrong about that.

#

I think it looks a lot tidier than what we previously had though

modest hedge
# modern sky Just to clarify - I think this is *maybe* fine, but I think we should aim to try...

Thanks for the feedback!

I don't think we can determine what's important at event emission time. What's important to see in this TUI isn't the same as what's important to see in Cloud. They're designed with similar principles and may even share code in the future, but the medium for presentation and the way that they're used is different. Definitely don't want to drop valuable info on the floor and make the use case for the data too narrow. Seeing the full trace in all its glory in Cloud is a great way to understand how everything works, and can be a very helpful for advanced use cases like building an SDK.

One constraint we're designing backwards from, and that I agree with, is that we want everything to go away at the end. The only output should be the data or --help text you requested, which should just be printed to stdout/stderr in a nice Unixy format as if none of the complicated TUI output ever happened. Until we get that result, I think it's most important to show whatever you're currently waiting on, which is basically a question of "what's taking time." Today I want to try also showing "what has taken time" so interesting things don't disappear, but if it makes it hard to see what is currently taking time because the scrollback becomes too cluttered, then IMO it's not worth it. Displaying vertexes "fully" is also an expensive decision for the same reason; some of these vertices are function calls with large complicated args, some of them have terminal output, etc.

Deciding what to show based on how long it's taking may sound like a workaround, but it's a pretty solid heuristic for what users will actually care to look at, even in unpredicable scenarios where your pipeline gets stuck somewhere you didn't predict. The worst thing a TUI can do is hang without showing me what it's waiting on, so it's nice that this simple heuristic inherently avoids that issue (as opposed to some system where the user tells us what's important, like withFocus).

#

wow sorry for the wall of text

modest hedge
#

Trying this out:

func (row *TraceRow) IsInteresting() bool {
    vtx := row.Step.FirstVertex()
    if vtx.Error != nil {
        // show errors always (TODO: make sure encapsulation is possible)
        return true
    }
    if vtx.Duration() < 100*time.Millisecond {
        // ignore fast steps; signal:noise is too poor
        return false
    }
    if row.IsRunning {
        // show things once they've been running for a while
        return true
    }
    if vtx.Completed != nil && time.Since(vtx.Completed.AsTime()) < time.Second {
        // show things that just completed, to reduce flicker
        return true
    }
    return false
}

https://asciinema.org/a/FC7837ivxEW54ggX0398y5jTp

#

A 100ms limit alone doesn't really help with flicker because then you'd just have it for steps that took 101ms. But I still think it's a useful first pass. And then if you combine that with removing things after 1 second instead of immediately, it feels a bit less frantic and doesn't seem to get too crowded.
Here's up: https://asciinema.org/a/z8uVglEOewzvOm9nAgVSlnPh7

modern sky
#

Oh this looks significantly better ❤️ ❤️ ❤️

#

So, I'm really struggling to explain what it is that still bothers me.
I think I agree that when we hit the end of the command (like running a call), we should collapse to only the result, and not show the logs.

But, while it's running, I think I'd like to preserve a little bit of the previous logs - not everything should disappear. Unfortunately, today I don't think it's very simple to define what these are - but I'd imagine "top-level" operations (that should ideally be at the top of the progress tree) to not disappear:

  • Load module "x"
  • Running function "y"
  • Bringing service "z" up (if using dagger call ... up or something)
    Genuinely not a lot - but I think something like this would help to give context on what's happening at each stage. Something where I can glance back and get a brief idea of what's already been done.

I think 1. this is useful from an overview perspective of what's actually being done, and 2. useful so that users can see that progress is being made - if all the progress vanishes after completing, for a super long build (maybe with a ton of module dependencies, or lots of chained functions), it might not feel clear that progress is actually being made at all.

#

As a more nitpicky comment - these lines are very long, and not particularly clear 😢
I get these are the graphql nested fields, but I would really love to work out how to have something that was a bit more understandable.

At one point I though we could try and get snippets from the language the module is written in? But this doesn't work as soon as you're cross-module, and mixing that up is weird (and also language shouldn't matter really).

Also, off-topic, but I remember a discussion somewhere about changing exec /runtime to be named something better... like the name of the module? That swould go some way I think as well towards making this more readable.

image.png
#

Sorry, this is really unstructured, but I'm still struggling to read this output - I tend to always put dagger into --progress=plain mode (like buildkit as well actually). These are just kinda my guesses for why I think it doesn't clock with me (but I also might not be the intended target audience, so you might want to ignore me 😆)

modern sky
modest hedge
# modern sky As a more nitpicky comment - these lines are *very* long, and not particularly c...

Would love to work that out too 😛 unfortunately don't have any ideas. I already tried rendering them "fully" (i.e. showing them as a stack of chained API calls, recursing into parents etc) but it just makes the output wildly verbose and didn't feel worth it. Thought about just hashing + color-coding their ancestry so it doesn't grow indefinitely, but then that's even less useful. I think a lot of this could actually be fixed by supporting intra-module calls, because then rather than huge chains of APIs you'd just see the small call you made within your own module, like daggerverse.withInitdb or something, and you could tidy up the output by using your API internally.

modest hedge
#

for comparison, here's how bad it looks without any modules at all; this TUI leans very hard on Zenith function calls to encapsulate progress

image.png
pine vigil
#

Looking pretty awesome! But I agree, as much as possible it'd be great to reduce to higher level concepts like functions from own module, and hide (or collapse in one line) the rest.

modest hedge
#

Trying another thing:

  • No more massive ancestry paths. It just shows the field being selected. Much easier to read.
  • Added module names (in magenta). This seems to be high bang-for-your-buck in terms of info conveyed, offsetting the info lost from the previous change.
    https://asciinema.org/a/Ee8cnNKErZhXgK5eog2JuSlDg
#

Feedback re: higher level grouping is heard, but I really want to keep doubling down on function-call-based visualization. It'd be great if in the end that's all we need. For example, I don't want to see Running function unit > bass unit() - I just want to see bass unit() because of course that's what a function call looks like. For other things like module initialization, which definitely contribute a lot of noise, it'd be great if we could push those into an actual function call like init() so it's not anything special either. The end goal here is to not feel the need to annotate our logs with a bunch of extra info, and instead have everything represented as function calls, and get to the point where reading that feels as natural as reading a stack trace (though with 1000x better DX and readability), since both are always representing one thing.

proven spire
proven spire
modest hedge
proven spire
#

That's better although I can still see/feel movement in my peripheral vision when I'm trying to focus on my logs (in this case 4-6 ticking timers). Do we have to show the millis? Can it just be seconds? I feel like that would be better. Maybe this is some kind of an OCD issue only I have 😄

modest hedge
#

Trying that now 😛

modest hedge
#

I think some kind of animation is important so the user knows it isn't stuck, at least. And making it that slow is almost a different kind of distraction.

proven spire
#

hmm, I can see people being confused about the 0s ones too. IDK.. Maybe I'll have to live with the tickers. I need some kind of a distraction free mode (cli option?) that's between this and --silent. Only thing that moves is the log from my function. Which is what I primarily care about. I will look at the other stuff only when something fails.

#

Definitely something you can keep iterating on based on community feedback also. I like the current iteration much much better than what we had so kudos on that.

modest hedge
#

Glad you like it overall! And the feedback is totally fair, I appreciate hearing different perspectives on this stuff. Would be worth throwing ideas around for flags. Don't want to go crazy with them, but there's no reason it has to be one-size-fits-all

modern sky
#

Woo! This looks so neat now actually ❤️
I still would love if we do more higher level grouping, but yes, this is hard 😄 I'd be happy if we left that to the side for now, and came back and fixed it after getting this in soon 🎉

r.e. timers, I think having the point is really important - without it, it feels like things get stuck. maybe that's just learnt behavior from buildkit/etc. There does seem to be a case in the recordings where a timer hits a number and then stops going up - when the job is finished. This definitely feels stuck. Maybe we could highlight the finished timers somehow (removing them entirely would be weird though).

#

@modest hedge r.e. merging this - i know we want to have this in for v0.9.9.
what's left from your perspective? are we in a good place to merge a first pass later today?

does this rely on a progrock update as well? is this even using progrock anymore?

I see we also add in support for CombinedOutput 🎉 looking forward to that!

modest hedge
modest hedge
modern sky
#

a > b > c

proven spire
#

After going through all of them again I agree with ^^^ too

pine vigil
#

Feedback in #daggernauts message re: a handled failure in a function leaking. Going to test it today, wondering if errors created by test suites show.

#

Just realized the "Not sure if this is covered by the new TUI" above 🙂

pine vigil
#

@modest hedge, can you rebase asap so I can test with Erik's changes to the module commands? There's some conflicts to solve.

modest hedge
modest hedge
modest hedge
#

@pine vigil done

fair frigate
#

I’m just coming back from PTO and checking out the new TUI on 0.9.11 and one thing I really miss was the behavior of —focus=false

Especially when things go wrong being able to scroll back through the terminal was super valuable

modest hedge