feat: configurable monitor latency display - #792
Open
123FLO321 wants to merge 22 commits into
Open
Conversation
…superpowers Shell scripts checked out with CRLF on Windows (core.autocrlf=true) broke the Docker entrypoint at container start. Force LF for all text files at the git layer, matching the existing .editorconfig and Prettier settings.
Add a workflow-level concurrency group keyed by workflow + ref with cancel-in-progress, so pushing a new commit cancels the still-running test job for the previous commit on that branch/PR.
The status icon sat at the left edge of a shrink-to-fit, right-pinned block whose width was driven by the (now multi-metric) latency line, so a longer avg|min|max line pushed the icon left and rows no longer aligned. Move the icon next to the uptime % and drop the latency onto its own line below, so the icon tracks only the % width (its pre-feature behavior).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughAdds configurable latency statistics to page settings and monitor bars, including current, average, minimum, and maximum values. Adds Vitest server/browser projects, Playwright setup, CI execution, and broad component and utility test coverage. ChangesConfigurable monitor latency display
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Administrator
participant PageSettings
participant MonitorPage
participant MonitorBar
Administrator->>PageSettings: select latency metrics
PageSettings->>MonitorPage: provide monitor_latency_display
MonitorPage->>MonitorBar: pass latencyDisplay
MonitorBar->>MonitorBar: build configured latency text
MonitorBar-->>Administrator: render latency display
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
| Filename | Overview |
|---|---|
| src/lib/components/GroupMonitorPopover.svelte | Passes the configured latency display to child monitor bars in every rendering branch. |
| src/lib/components/MonitorBar.svelte | Formats and displays the selected latency statistics with an average-only default. |
| src/lib/server/pageSettings.ts | Adds defaults, merging, validation, and API conversion for latency display settings. |
| src/lib/server/api-server/monitor-bar/shared.ts | Adds the latest latency value to monitor-bar API responses. |
Reviews (3): Last reviewed commit: "style(monitor-bar): use "label:" and mid..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/test.yml:
- Around line 20-41: Pin the third-party actions in the workflow, including
actions/checkout@v4.2.2, actions/setup-node@v4, and actions/cache@v4, to their
exact immutable commit SHAs. Preserve the current action versions and
configuration while replacing mutable version tags with the corresponding SHA
references.
- Around line 17-18: Correct the indentation of the contents property under the
permissions configuration in the workflow YAML so it is indented exactly two
spaces relative to permissions, preserving the existing contents: read value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3ecd3cda-18f0-4353-8b74-7082a408f25c
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (24)
.gitattributes.github/workflows/test.yml.gitignoreCLAUDE.mdpackage.jsonsrc/lib/clientTools.test.tssrc/lib/clientTools.tssrc/lib/components/CopyButton.svelte.test.tssrc/lib/components/MonitorBar.sveltesrc/lib/components/MonitorBar.svelte.test.tssrc/lib/components/StatusBarCalendar.svelte.test.tssrc/lib/locales/en.jsonsrc/lib/server/controllers/dashboardController.tssrc/lib/server/pageSettings.test.tssrc/lib/server/pageSettings.tssrc/lib/server/tool.test.tssrc/lib/server/types/db.tssrc/lib/types/api.tssrc/routes/(kener)/+page.sveltesrc/routes/(kener)/[page_path]/+page.sveltesrc/routes/(manage)/manage/app/pages/[page_id]/+page.sveltestatic/api-references/v4.jsonvite.config.tsvitest-setup-client.ts
GroupMonitorPopover rendered its child MonitorBar instances without latencyDisplay, so a group's included-monitors drawer always showed avg-only regardless of the page's configured min/max/avg setting. Thread the prop MonitorBar -> GroupMonitorPopover -> child MonitorBar. Also switch the prop type to the shared PageSettingsLatencyDisplay (removes the duplicated inline shape).
Adds a fourth per-page latency stat: the current/latest reading, shown as "now. 123ms". Fixed bar order is now current -> avg -> min -> max; default is off (unchanged behavior). - Server: expose currentLatency (latest monitoring row latency, formatted like the aggregates) in the monitor-bar response. Both the single and batch endpoints already fetch the latest row for status, so no extra queries. - Config: current flag added to PageSettingsLatencyDisplay + default/merge/ sanitize/validate in pageSettings, server + manage defaults, and the v4 OpenAPI schema. - UI: buildLatencyDisplay handles current (first); MonitorBar passes the value and "now." label; admin gets a Current checkbox; i18n keys "now."/"Current". - Tests updated (clientTools 11, pageSettings 8, MonitorBar 4); check 0/0.
Per feedback: labels now read "now:" / "avg:" / "min:" / "max:" (colon instead of period), and metrics are joined with a middle dot " · " instead of " | ", which reads lighter in the muted caption. Renders e.g. "now: 123ms · avg: 250ms · min: 10ms · max: 900ms".
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
1 participant
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.
The monitor bar on a status page always shows average latency in the top-right corner, with no way to change it.
Add a per-page setting to choose which latency stats appear (a checkbox for average, minimum, and maximum ) shown as a short labeled string:
e.g.
avg. 250ms | min. 10ms | max. 900ms | now 30msDefault stays average only (current behavior).
requires #789
Summary by CodeRabbit
Summary by CodeRabbit