Repository navigation
Metrics explorer page design - #3348
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
david-crespo
marked this pull request as ready for review
August 25, 2026 20:48
| * | ||
| * Exported for tests; use {@link oxqlAutocomplete} in the editor. | ||
| */ | ||
| export const oxqlCompletionSource = |
…query scope, datum
No point leaving it behind.
They're stable.
There is no such thing as "one representative query". There are three distinct kinds of charts, to say nothing of layout!
Codemirror was putting it behind the active line, making it challenging to see what text you selected (and impossible to tell what whitespace you selected).
fakemonster
reviewed
Sep 14, 2026
This page is already tipped pretty far into "knowing everything about backend implementation details", so I'm just grabbing an easy win here.
Safari has subpixel precision on bounding boxes, so we need slightly more tolerance than exact matching for cursor<->placeholder comparisons. "Within a pixel" seems fair enough to me.
Collaborator
|
Relevant to the unit question — it would make things a lot easier if the metrics responses included units, but it looks like there's quite a lot required to get that done, at least according to the plan here: oxidecomputer/omicron#6696 It's possible we could do something clever client-side, though it would be fragile. I am not above fetching the units from the TOML in omicron and keeping a map of them. One issue would be that if you do some kind of math in your query and combine units, we might not be able to easily figure out what the resulting unit should be. |
Contributor
Yeah... unfortunately this is intrinsic to oximeter at the moment. We could kind of... huck the schema TOML into console and pull out units that way (or, similarly, do that within omicron), but it's not in the DB schema at the moment.
We do have a "looks like a badge but doesn't screw with the casing" component now. I agree it's odd but my main concern (that you can copy-paste it with the correct casing) is satisfied, so I'm personally neutral on it.
Was a bit of laziness on my part; I think if you're transforming the table name at all, you should format it. Which is relatively a pain given how many acronyms we got floating around in here. Mono might be a good middle ground then (though my understanding is that it hurts @benjaminleonard's feelings) |
Collaborator
|
Example showing more aggressive UUID truncation. I felt like with truncating UUIDs to 24 chars, we were getting the visual noise of truncation without enough of the benefit in terms of space saved. 
|
Collaborator
|
Yeehaw. 
|
david-crespo
added a commit
that referenced
this pull request
Oct 7, 2026
https://github.com/oxidecomputer/console/actions/runs/37562428363/job/112602625115?pr=3348 In an action menu test, it seems like the escape key fails to close the menu when we expect. This has happened only a few times and I couldn't repro it locally. This fix is the robots' best guess, appropriately explained and caveated in a comment. Bot found 9 failures in ~317 CI runs since Aug 17. <details> <summary>Full 🤖 investigation and failure to repro</summary> Goal: reproduce https://github.com/oxidecomputer/console/actions/runs/37562428363/job/112602625115?pr=3348 and determine whether Firefox fill/composition suppresses Escape. ## Summary "filters items and resets the search when dismissed" in `ActionMenu.browser.spec.tsx` has failed in 9 CI jobs since 2026-08-17. Every failure was in Firefox, and the menu was still open after Escape. The failure hasn't been reproduced in Vitest locally. The leading hypothesis is Base UI's IME composition guard. In Firefox, Playwright's `fill()` inserts text through a composition, so it emits `compositionstart`/`compositionend`. Base UI ignores Escape while composing, and only clears that state in a 0 ms `setTimeout` after `compositionend`. An Escape that arrives before that timer runs is discarded. Typing the text instead of filling it produces no composition events, which is the change in this PR. That mechanism is demonstrated with the real ActionMenu in a standalone Firefox repro, but the repro's trigger (timers deferred while the page is still loading) can't occur in a Vitest run. So it's still unknown why the timer would be late in CI, and whether typing fixes the flake. Focus loss to the parent page is an alternative that produces the same symptom, but fits the failure history less well. ## CI failure history Searched every run of the CI workflow since the browser specs landed on 2026-08-17 (#3287), 317 runs. Runs with a rerun had each attempt checked, because a rerun that passes hides the first attempt's failure. Kept jobs whose "Run Vitest browser tests" step failed and grepped their failed-step logs (`gh run view --job <id> --log-failed`). Two job listings returned GitHub 502s, so the count may be short by one or two. "filters items and resets the search when dismissed" failed in 9 jobs across 8 runs, all Firefox. It never failed in Chromium or WebKit: | Date | Branch | Run / attempt | Failure | | ----- | -------------------------------- | -------------------- | ------------------------------------------------------- | | 08-17 | main | 32076851713/1 | test timeout (15 s), no assertion location | | 08-17 | fix-combobox-blur | 32078312304/1 and /2 | after-Escape dialog assertion, failed on the rerun too | | 08-18 | main | 32153723452/1 | after-Escape dialog assertion | | 08-18 | support-bundles | 32154137346/1 | after-Escape dialog assertion | | 08-18 | accessible-tooltips | 32167049254/1 | after-Escape dialog assertion | | 08-26 | prototype-mutation-invalidations | 33015840325/1 | after-Escape dialog assertion (line 74, so after #3353) | | 10-07 | oxql-page-design (#3348) | 37562428363/1 | after-Escape dialog assertion | | 10-07 | oxql-page-design (#3348) | 37633457389/1 | after-Escape dialog assertion | That's 6 failures in the first two days and 3 in the six weeks after #3353 (2026-08-26) changed it to an input-targeted Escape. The rate dropped but the failure didn't go away. Over the same period Firefox had single failures in four different `Combobox.browser.spec.tsx` tests on 08-17/18. The other failed Vitest steps were consistent breakage on feature branches (SubscriptionsField, NumberInput) or unhandled errors. The two #3348 failures aren't explained by that branch missing something from main. #3348 is stacked on `oxql-page`, which is 13 commits behind main, but none of those touch ActionMenu, the Vitest config, or the CI workflow. Base UI, Vitest, Playwright and `vitest-browser-react` versions are the same in both lockfiles. #3348 doesn't add browser specs, and main itself failed on 08-17 and 08-18. ## Mechanism ### Composition guard, reproduced outside Vitest A standalone Firefox script serves the real ActionMenu in a small React harness (Vite, Firefox 151.0, Playwright 1.61.0, Base UI 1.1.0). It reproduces the stuck dialog without changing the component or the timer delay. It holds an async script request open so the document stays `interactive`, and keeps the browser event queue occupied with short MessageChannel tasks. It fills `db1`, checks the filtered options, focuses the input, and presses Escape. Firefox defers Base UI's composition-reset timer until the document finishes loading. Escape is discarded, and finishing the load afterward does not close the dialog. A second Escape closes it. One captured run: - 149 ms: compositionstart, compositionend, reset timer scheduled with delay 0. - 190 ms: Escape reaches the focused input, document still interactive; dialog remains data-open with db1. - 216 ms: after releasing the pending request, document becomes complete and the timer fires. - Dialog remains open. A second Escape closes it. The script reproduced the stuck dialog in 3/3 runs. Each of three controls closed it in 3/3 runs: type `db1` instead of filling, await page load before filling, or disable Firefox's `dom.timeout.defer_during_load` preference. ### Load-time timer deferral is ruled out for Vitest Vitest 5 waits for the test iframe's `load` event before preparing and executing tests. Injecting the same pending script into its initial HTML makes Vitest wait, and the test passes afterward. Firefox treats a window as loading only when its own document is `loading`/`interactive` or an ancestor is ([`Document::NotifyLoading`](https://github.com/mozilla-firefox/firefox/blob/cfd140a/dom/base/Document.cpp) mirrors the top-level state down to subdocuments; [`nsGlobalWindowInner::SetActiveLoadingState`](https://github.com/mozilla-firefox/firefox/blob/cfd140a/dom/base/nsGlobalWindowInner.cpp) passes it to `TimeoutManager::SetLoading`). Sibling iframes don't count. In a Firefox Vitest run, `document`, `parent` and `top` all report `complete` 200 ms into a test. The failing test is also the second in its file, so the iframe had already run a test. The standalone repro shows the guard in action, but its trigger can't occur here. ### Effect re-run cancellation is a real Base UI hole, but not this test's `useDismiss`'s effect cleanup calls `compositionTimeout.clear()` and never resets `isComposingRef`. If the effect re-runs between `compositionend` and the 0 ms timer, every later Escape is ignored until another composition happens. Instrumenting the deps shows one re-run per open, when `floatingElement` goes from `null` to the popup about 4 ms after mount. `vitest-browser-react`'s `render` wraps in `act()`, so that re-run finishes before `fill()` starts. The reopen at the end of the test triggers it again, but no composition follows. ### Focus loss as an alternative Moving focus elsewhere inside the test iframe should still let Base UI see Escape through its [document listener](https://github.com/mui/base-ui/blob/033199c/packages/react/src/floating-ui-react/hooks/useDismiss.ts#L569-L578). Losing focus to the parent page could make it miss the key entirely. Vitest's targeted `type(search, '{Escape}')` focuses the input, then sends a page-level key event, leaving a gap between those operations. A focus probe produced the same symptom with the actual menu and no composition events. It loads the real ActionMenu in an iframe, waits for loading to finish, and types `db1` rather than filling. Captured events show: - Focus moved to the Dismiss button inside the iframe: Escape reaches that button in the test document; dialog closes. - Focus deliberately moved to a parent-page input: Escape reaches the parent document; the test iframe receives no Escape; dialog remains `data-open`, search remains `db1`, and the test document reports `hasFocus() === false`. - Refocusing the search input and pressing Escape closes the dialog. The probe introduces the focus steal on purpose. It doesn't reproduce a spontaneous focus loss in Vitest or explain what would steal focus in CI, and typing doesn't protect against it. Focus loss fits the failure history less well than composition. Every failure is in the filter test, and the ones checked showed `value="db1"` and `data-open`. "dismisses with Escape" sends a global Escape to the same freshly rendered menu and has never failed in CI. The filter test has failed with a global Escape (before #3353) and with an input-targeted Escape (after), so the delivery path doesn't seem to matter. The only step the filter test adds is `fill()`, which is what produces composition events in Firefox. ### What's left The guard path with a timer that's late for an ordinary reason. Firefox dispatches a 0 ms, non-nested timeout as an immediate normal-priority runnable to the window's event target ([`TimeoutExecutor::ScheduleImmediate`](https://github.com/mozilla-firefox/firefox/blob/cfd140a/dom/base/TimeoutExecutor.cpp)). So Escape can only win if its keydown overtakes an already-queued runnable. That could happen if Juggler's key dispatch reaches the content process on a higher-priority queue than the window's timer queue, or if the per-window timer queue is backed up. Not verified. A CI failure would distinguish the hypotheses if the test recorded the composition-timer fire time, the Escape keydown time and target document, `document.hasFocus()`, and focus transitions, and attached them to the failure. The failure's DOM snapshot alone can't. ## Attempts that didn't reproduce it The original test sequence, instrumented for composition events, timer firing/cancellation, input focus, and Escape delivery, passed: - 200 baseline repetitions with failure-only tracing. - 100 repetitions under 12 CPU churn workers on a 14-core machine, with tracing. - 200 under the same CPU contention without tracing. - 50 under bounded browser MessageChannel contention without tracing. No timer cancellation or missed Escape was captured in those runs. A diagnostic-only 1000 ms delay of the composition-reset timer reproduced the original failed dialog-disappearance assertion. Typing passed 10 repetitions under that same delay. A 100 ms delay passed with tracing enabled, which shows how recording can mask short windows. A headless background-tab experiment did not make Firefox report the page hidden or unfocused. None of the release notes for [Base UI 1.1–1.8](https://base-ui.com/react/overview/releases), [Playwright 1.61–1.63](https://playwright.dev/docs/release-notes) or [Vitest 5.0–5.0.3](https://github.com/vitest-dev/vitest/releases) mention a fix for this. Base UI's latest release still has the [same composition-reset timer](https://github.com/mui/base-ui/blob/47b4052/packages/react/src/floating-ui-react/hooks/useDismiss.ts#L311-L327). ## Sources - [Playwright Firefox inserts text using commitCompositionWith](https://github.com/microsoft/playwright/blob/1cc5a90/browser_patches/firefox/juggler/content/PageAgent.js#L576-L579). - [Base UI Escape guard](https://github.com/mui/base-ui/blob/033199c/packages/react/src/floating-ui-react/hooks/useDismiss.ts#L199-L209) and [composition-reset timer](https://github.com/mui/base-ui/blob/033199c/packages/react/src/floating-ui-react/hooks/useDismiss.ts#L550-L566). - [Mozilla's explanation of timeout deferral during loading](https://blog.mozilla.org/performance/2020/06/12/improving-firefox-page-load/). </details>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WIP