Skip to content

Coverage graph zoom#64

Merged
Liam-DeVoe merged 13 commits intoZac-HD:masterfrom
Liam-DeVoe:dashboard
Mar 21, 2025
Merged

Coverage graph zoom#64
Liam-DeVoe merged 13 commits intoZac-HD:masterfrom
Liam-DeVoe:dashboard

Conversation

@Liam-DeVoe
Copy link
Copy Markdown
Collaborator

@Liam-DeVoe Liam-DeVoe commented Mar 9, 2025

Current UX is to allow arbitrary zoom with scroll, as opposed click + dragging to define the new viewport. I'm not entirely sold that scroll is always better yet (though I think it usually is). I think will depend on what zooming realistically gets used for.

@Zac-HD
Copy link
Copy Markdown
Owner

Zac-HD commented Mar 9, 2025

(noting here for ease of release, not because it's related)

clicking through to /patches in --dashboard-only mode reliably gives me an error:

  File ".../starlette/routing.py", line 72, in app
    response = await func(request)
  File ".../hypofuzz/dashboard.py", line 160, in api_patches
    patches = make_and_save_patches(PYTEST_ARGS, REPORTS, METADATA)
  File ".../hypofuzz/patching.py", line 30, in make_and_save_patches
    report = reports[nodeid][-1]
IndexError: list index out of range

Comment thread src/hypofuzz/dashboard.py
Comment on lines +281 to +283
# TODO we should really add the node id to hypofuzz-test-keys entries
node_id = reports[0]["nodeid"]
REPORTS[node_id] = SortedList(reports, key=lambda r: r["elapsed_time"])
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can pretty easily construct cases where there are multiple nodeids for a single DB key, or vice-versa. They're not tests that I'd expect anyone to write naturally and so I'm not proposing to try 'fixing' that, but I do want to treat database keys as the underlying source of truth and nodeids (which are also less stable!) as only a presentation-layer thing.

@Zac-HD
Copy link
Copy Markdown
Owner

Zac-HD commented Mar 9, 2025

quick notes

  • I'd love a way to zoom horizontally-only, or select a region of interest - often I want to inspect the recent part of the log-time curve, and that's a little trickier than it needs to be at the moment.
  • it'd be neat if the tooltips showed both number of inputs and elapsed time, regardless of which axis we're currently viewing (at least for the single-test views)
  • I haven't seen /patches working yet
  • browser and UI testing is always a PITA at best, but at some point this year we'll need to work something out.
    • Not soon though, and probably we should write tests against the http/websockets API first.

log-x-axis labels are tricky

image
presumably there's a nice way to get this working that doesn't immediately break again with zooming etc? Low prio.

initial load seems buggy

image
I saw a similar issue at one point from switching between time/inputs axes, which suggests that the frontend needs to resort when graphing?

@Liam-DeVoe
Copy link
Copy Markdown
Collaborator Author

  • I've changed zooming to only change the x axis, which feels better. We can bring back both-axis-zoom on a modifier key (probably cmd?) if we want. Also fixed a centering bug which caused zooming to not zoom onto the cursor
  • done
  • works for me, but I believe this...I'm hoping to push looking into this out to until I rewrite patches? (soon, hopefully)
  • api tests first once the ui is more stable...then we can think about ui testing 😅 not particularly looking forward to this one, but agree it needs to happen.
  • (log x axis) noticed this too, I wonder how other log graphs do this. Seems like a common problem. Making the labels diagonal unconditionally might be the best option, but it doesn't look as nice as flat labels
  • (initial load) hmm, I thought I fixed this in a force push here. I guess the frontend should be sorting for safety. I've been mostly relying on the backend to sort so far. If you're still seeing this let me know?

@Zac-HD
Copy link
Copy Markdown
Owner

Zac-HD commented Mar 10, 2025

  • graph y-axis bounds don't seem to account for initially-loaded data, so I start with a lot of lines cut off.
    • I also now can't zoom the y-axis at all, so if it's out of range I just can't see it at all.
    • x-zoom resets to full area on update, which I don't want
    • a box-select tool would be really nice as a y-zoom option, so I can make whatever small late changes fill the area
  • for the list of minimal-covering-examples on an individual test page, we're ommitting any later notes from data.draw(). We should fix that, and then skip inputs where the string repr is identical to an earlier input.
  • (for later, just so I don't forget) ability to sort the table by value in each column. Maybe also a search/filter box if there are many tests.
  • still seeing crazy line orderings, so I think the frontend needs to sort.

@Liam-DeVoe
Copy link
Copy Markdown
Collaborator Author

I'd advocate for merging now and following up in future pulls, thoughts?

@Liam-DeVoe Liam-DeVoe mentioned this pull request Mar 20, 2025
1 task
@Zac-HD
Copy link
Copy Markdown
Owner

Zac-HD commented Mar 21, 2025

I think this is actually a bug in Hypothesis (!!), but if I move the .hypothesis/ directory (to test on an empty db), I get this error:

  File ".../hypofuzz/dashboard.py", line 291, in run_dashboard
    db._db.add_listener(send_nowait_from_anywhere)
  File ".../hypothesis/database.py", line 187, in add_listener
    self._start_listening()
  File ".../hypothesis/database.py", line 998, in _start_listening
    self._db.add_listener(self._broadcast_change)
  File ".../hypothesis/database.py", line 187, in add_listener
    self._start_listening()
  File ".../hypothesis/database.py", line 513, in _start_listening
    self._observer.start()
  File ".../watchdog/observers/api.py", line 280, in start
    emitter.start()
  File ".../watchdog/utils/__init__.py", line 92, in start
    self.on_thread_start()
  File ".../watchdog/observers/inotify.py", line 125, in on_thread_start
    self._inotify = InotifyBuffer(path, self.watch.is_recursive, event_mask)
  File ".../watchdog/observers/inotify_buffer.py", line 37, in __init__
    self._inotify = Inotify(path, recursive, event_mask)
  File ".../watchdog/observers/inotify_c.py", line 176, in __init__
    self._add_watch(path, event_mask)
  File ".../watchdog/observers/inotify_c.py", line 401, in _add_watch
    Inotify._raise_error()
  File ".../watchdog/observers/inotify_c.py", line 417, in _raise_error
    raise OSError(err, os.strerror(err))
FileNotFoundError: [Errno 2] No such file or directory

In context that only takes out the dashboard, with the workers happily fuzzing away in other processes, and re-running the command brings the dashboard up just fine (because the database now exists)

(obviously this is yet another thing to split out, sorry)

@Zac-HD
Copy link
Copy Markdown
Owner

Zac-HD commented Mar 21, 2025

I think I've worked out (one reason) why my dashboard has intermittent chaos on the line charts: my habitual test set has fewer tests than my laptop has cores, so I've been accidentally stress-testing the presentation of data written by concurrent workers. Bumps up priority of the reconciliation logic mentioned in #3 and #45 I suppose.

Copy link
Copy Markdown
Owner

@Zac-HD Zac-HD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More generally, I've had a tough time testing this locally, with what seems to be a persistent stale build (no sorting, no box-select, etc). I'll hold other comments on the basis that they're probably stale.

I'm OK with merging this on trust that you've checked it out on your end, since I do think it improves on the status quo.

@Liam-DeVoe
Copy link
Copy Markdown
Collaborator Author

oh, sorry, one thing I definitely should have mentioned earlier: I don't have autobuilding set up yet, so for now whenever there's a source code change you have to run npm run build inside src/hypofuzz/frontend. I assume you're using hypothesis-jsonschema? I'll take another look with that to see if I can reproduce any weirdness.

re box select: this isn't surprising, I've commented out the icon for now until I fix the issues with it.

@Zac-HD
Copy link
Copy Markdown
Owner

Zac-HD commented Mar 21, 2025

Yep, specifically hypothesis fuzz ../hypothesis-jsonschema/tests/test_canonicalise.py as my default run.

Having also done src/hypofuzz/frontend ; npm run build ; cd -, a few notes

  • it looks like the graph now preserves zoom level on update, but jumps rightward - best guess is that the position anchor is to the upper bound of the x-axis, we probably want the lower bound (s.t. the viewport doesn't move relative to data coordinates)
  • Switching between inputs/time sorting, or zooming in and out, still somewhat-reliably messes up the ordering of lines in my plot, although only for what I'm pretty sure is the multiple-workers case.

approval stands for the reasons above 🙂

@Liam-DeVoe Liam-DeVoe merged commit 0f779b5 into Zac-HD:master Mar 21, 2025
13 checks passed
@Liam-DeVoe Liam-DeVoe deleted the dashboard branch March 21, 2025 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants