Skip to content

implement html output, fix log scaling in charts #218

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

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

DaltheCow
Copy link
Collaborator

@DaltheCow DaltheCow commented Jul 1, 2025

To clarify all of the UI changes here, there was a previous version of the UI that worked with an older version of guidellm and the code used to generate the html report provided the UI with a different data format. Most of the UI changes here are needed to get things up to date with the current version of guidellm and how the html reporting logic generates data.

There are a few specific changes in these files to get logarithmic scaling working which I found was broken after making these changes, in hindsight I would have split these out into another PR:
src/ui/lib/components/Charts/DashedLine/DashedLine.component.tsx
src/ui/lib/components/Charts/DashedLine/helpers.ts
src/ui/lib/components/Charts/MetricLine/MetricLine.component.tsx
tests/ui/unit/components/Charts/DashedLine/helpers.test.ts

There were a few other tiny bug fixes as well.

@DaltheCow DaltheCow added the UI Front-end workstream label Jul 1, 2025

📦 Build Artifacts Available
The build artifacts (.whl and .tar.gz) have been successfully generated and are available for download: https://github.com/neuralmagic/guidellm/actions/runs/16010163088/artifacts/3444433981.
They will be retained for up to 30 days.

📦 Build Artifacts Available
The build artifacts (.whl and .tar.gz) have been successfully generated and are available for download: https://github.com/neuralmagic/guidellm/actions/runs/16010161293/artifacts/3444434269.
They will be retained for up to 30 days.

Copy link

github-actions bot commented Jul 1, 2025

🎉 Live Preview: Click here to view the live version
Last updated: aa176a0

github-actions bot pushed a commit that referenced this pull request Jul 1, 2025
github-actions bot pushed a commit that referenced this pull request Jul 1, 2025

📦 Build Artifacts Available
The build artifacts (.whl and .tar.gz) have been successfully generated and are available for download: https://github.com/neuralmagic/guidellm/actions/runs/16128343191/artifacts/3480921434.
They will be retained for up to 30 days.

github-actions bot pushed a commit that referenced this pull request Jul 7, 2025
Copy link
Collaborator

@jaredoconnell jaredoconnell left a comment

Choose a reason for hiding this comment

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

I didn't notice any major red flags from the code review.
I took the time to run a new benchmark using this branch, since the output format changed from the addition of the 50th percentile. I merged in my from-file changes for testing, and converted the JSON output to HTML, and I encountered two things.

First, I got these warnings:

Saving benchmarks report...
/Users/joconnel/Documents/projects/ai/guidellm/.venv/lib/python3.13/site-packages/pydantic/main.py:463: UserWarning: Pydantic serializer warnings:
  PydanticSerializationUnexpectedValue(Expected `float` - serialized value may not be as expected [input_value='p50', input_type=str])
  PydanticSerializationUnexpectedValue(Expected `float` - serialized value may not be as expected [input_value='p90', input_type=str])
  PydanticSerializationUnexpectedValue(Expected `float` - serialized value may not be as expected [input_value='p95', input_type=str])
  PydanticSerializationUnexpectedValue(Expected `float` - serialized value may not be as expected [input_value='p99', input_type=str])
  PydanticSerializationUnexpectedValue(Expected `float` - serialized value may not be as expected [input_value='p50', input_type=str])
  PydanticSerializationUnexpectedValue(Expected `float` - serialized value may not be as expected [input_value='p90', input_type=str])
  PydanticSerializationUnexpectedValue(Expected `float` - serialized value may not be as expected [input_value='p95', input_type=str])
  PydanticSerializationUnexpectedValue(Expected `float` - serialized value may not be as expected [input_value='p99', input_type=str])
  PydanticSerializationUnexpectedValue(Expected `float` - serialized value may not be as expected [input_value='p50', input_type=str])
  PydanticSerializationUnexpectedValue(Expected `float` - serialized value may not be as expected [input_value='p90', input_type=str])
  PydanticSerializationUnexpectedValue(Expected `float` - serialized value may not be as expected [input_value='p95', input_type=str])
  PydanticSerializationUnexpectedValue(Expected `float` - serialized value may not be as expected [input_value='p99', input_type=str])
  PydanticSerializationUnexpectedValue(Expected `float` - serialized value may not be as expected [input_value='p50', input_type=str])
  PydanticSerializationUnexpectedValue(Expected `float` - serialized value may not be as expected [input_value='p90', input_type=str])
  PydanticSerializationUnexpectedValue(Expected `float` - serialized value may not be as expected [input_value='p95', input_type=str])
  PydanticSerializationUnexpectedValue(Expected `float` - serialized value may not be as expected [input_value='p99', input_type=str])
  return self.__pydantic_serializer__.to_python(
Benchmarks report saved to benchmarks_html.html

Second, the HTML file displays nearly blank. I'll upload my benchmarks file for comparison. It's possible that I did something wrong. The HTML cannot be included in this comment, so here is a picture of the output on Firefox:
image
benchmarks_html.json

Base automatically changed from ui-build-pipeline-and-ci to main July 10, 2025 20:55
github-actions bot pushed a commit that referenced this pull request Jul 15, 2025
github-actions bot pushed a commit that referenced this pull request Jul 15, 2025
github-actions bot pushed a commit that referenced this pull request Jul 15, 2025
github-actions bot pushed a commit that referenced this pull request Jul 15, 2025
@DaltheCow DaltheCow force-pushed the html-report branch 2 times, most recently from 5bd84d8 to b346719 Compare July 15, 2025 23:07
DaltheCow and others added 13 commits July 15, 2025 19:25
Bumps [next](https://github.com/vercel/next.js) from 15.3.2 to 15.3.4.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/vercel/next.js/releases">next's
releases</a>.</em></p>
<blockquote>
<h2>v15.3.4</h2>
<blockquote>
<p>[!NOTE]<br />
This release is backporting bug fixes. It does <strong>not</strong>
include all pending features/changes on canary.</p>
</blockquote>
<h3>Core Changes</h3>
<ul>
<li>[metadata] render streaming metadata on the top level (<a
href="https://redirect.github.com/vercel/next.js/issues/80566">#80566</a>)</li>
<li>[fix] clone the config module to avoid mutation (<a
href="https://redirect.github.com/vercel/next.js/issues/80573">#80573</a>)</li>
</ul>
<h3>Credits</h3>
<p>Huge thanks to <a
href="https://github.com/huozhi"><code>@​huozhi</code></a> for
helping!</p>
<h2>v15.3.3</h2>
<blockquote>
<p>[!NOTE]<br />
This release is backporting bug fixes. It does <strong>not</strong>
include all pending features/changes on canary.</p>
</blockquote>
<h3>Core Changes</h3>
<ul>
<li>Reinstate <code>vary</code> (<a
href="https://redirect.github.com/vercel/next.js/issues/79939">#79939</a>)</li>
<li>fix(next-swc): Fix interestingness detection for React Compiler (<a
href="https://redirect.github.com/vercel/next.js/issues/79558">#79558</a>)</li>
<li>fix(next-swc): Fix react compiler usefulness detector (<a
href="https://redirect.github.com/vercel/next.js/issues/79480">#79480</a>)</li>
<li>fix(dev-overlay): Better handle edge-case file paths in launchEditor
(<a
href="https://redirect.github.com/vercel/next.js/issues/79526">#79526</a>)</li>
<li>Client router should discard stale prefetch entries for static pages
(<a
href="https://redirect.github.com/vercel/next.js/issues/79362">#79362</a>)</li>
</ul>
<h3>Credits</h3>
<p>Huge thanks to <a
href="https://github.com/gaojude"><code>@​gaojude</code></a>, <a
href="https://github.com/kdy1"><code>@​kdy1</code></a>, <a
href="https://github.com/bgw"><code>@​bgw</code></a>, and <a
href="https://github.com/unstubbable"><code>@​unstubbable</code></a> for
helping!</p>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https://github.com/vercel/next.js/commit/f9e98b9ce39d69fac6841faeaa327d3848e0122c"><code>f9e98b9</code></a>
v15.3.4</li>
<li><a
href="https://github.com/vercel/next.js/commit/2557fba3996007f642df8a2b0c98b36c9ce9a343"><code>2557fba</code></a>
[fix] clone the config module to avoid mutation (<a
href="https://redirect.github.com/vercel/next.js/issues/80573">#80573</a>)</li>
<li><a
href="https://github.com/vercel/next.js/commit/616f6fa0908552126de3190e663e15491a267e13"><code>616f6fa</code></a>
[backport][metadata] render streaming metadata on the top level (<a
href="https://redirect.github.com/vercel/next.js/issues/80566">#80566</a>)</li>
<li><a
href="https://github.com/vercel/next.js/commit/3ab8db7383ae904af872e14c26a9bd8a43879819"><code>3ab8db7</code></a>
v15.3.3</li>
<li><a
href="https://github.com/vercel/next.js/commit/18c8113ebd90cfb54902c429a7c1138ed7d9f97d"><code>18c8113</code></a>
[backport] Reinstate <code>vary</code> (<a
href="https://redirect.github.com/vercel/next.js/issues/79939">#79939</a>)</li>
<li><a
href="https://github.com/vercel/next.js/commit/e18212f54668eba21ef8ac33f5db5a94f2ba3499"><code>e18212f</code></a>
re-enable vary header deploy test (<a
href="https://redirect.github.com/vercel/next.js/issues/79753">#79753</a>)</li>
<li><a
href="https://github.com/vercel/next.js/commit/ec202eccf05820b60c6126d6411fe16766ecc066"><code>ec202ec</code></a>
Revert &quot;[next-server] skip setting vary header for basic
routes&quot; (<a
href="https://redirect.github.com/vercel/next.js/issues/79426">#79426</a>)</li>
<li><a
href="https://github.com/vercel/next.js/commit/e2f264fdce0472001675418f7524ed355fa184a0"><code>e2f264f</code></a>
fix(next-swc): Fix interestingness detection for React Compiler (15.3)
(<a
href="https://redirect.github.com/vercel/next.js/issues/79558">#79558</a>)</li>
<li><a
href="https://github.com/vercel/next.js/commit/562fac78dab5656c6b8cbf6248a44a8f18da3075"><code>562fac7</code></a>
fix(next-swc): Fix react compiler usefulness detector (15.3) (<a
href="https://redirect.github.com/vercel/next.js/issues/79480">#79480</a>)</li>
<li><a
href="https://github.com/vercel/next.js/commit/06097fd7bb74f437fdb52d42a82280bce206f9d3"><code>06097fd</code></a>
fix(dev-overlay): Better handle edge-case file paths in launchEditor (<a
href="https://redirect.github.com/vercel/next.js/issues/79526">#79526</a>)</li>
<li>Additional commits viewable in <a
href="https://github.com/vercel/next.js/compare/v15.3.2...v15.3.4">compare
view</a></li>
</ul>
</details>
<br />

[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=next&package-manager=npm_and_yarn&previous-version=15.3.2&new-version=15.3.4)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)
You can disable automated security fix PRs for this repo from the
[Security Alerts
page](https://github.com/neuralmagic/guidellm/network/alerts).

</details>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: dalthecow <[email protected]>
Add container build jobs alongside the existing wheel jobs. Tag
containers based on which workflow was triggered.

Tags:
* PR: `pr-{{ github.event.number }}` ~`{{ github.sha }}`~
* Nightly: `nightly` ~`{{ github.sha }}`~
* Release candidate: `latest` ~`{{ github.sha }}`~
* Release:  `latest` `stable` ~`{{ github.sha }}`~

Edit: To save space, don't use unique tags.

- - -

* Tag RC and releases with their versions. (e.g. 0.3.0-rc, 0.3.0)
* Reuse wheel builds in container

---------

Co-authored-by: Mark Kurtz <[email protected]>
Signed-off-by: dalthecow <[email protected]>
Co-authored-by: Mark Kurtz <[email protected]>
Co-authored-by: Copilot <[email protected]>
Signed-off-by: dalthecow <[email protected]>
Small issue in ui-unit-test job is causing failures across all PRs since
we use pull_request_target instead of pull_request for the trigger in
development.yml which means that actions run against the workflow in
main instead of the workflow files that might have been updated in the
PR.

The last UI PR was merged and brought in this buggy ui-unit-test job.

I also updated the checkout action to v4 since the other non ui jobs are
using v4 instead of v3

Ended up updating package-lock as well since I was getting more failures
than expected, thought maybe that was the issue. Regardless, this PR
passes the checks when I temporarily update the trigger from
pull_request_target to pull_request

Signed-off-by: dalthecow <[email protected]>
`pull_request_target` has not been the drop-in replacement it promises
to be. Revert its addition and disable build jobs for external PRs.

---------

Co-authored-by: Copilot <[email protected]>
Signed-off-by: dalthecow <[email protected]>
when using `pull_request_target:` the default ref is main so all checks
are run from the main branch rather than the PR. This re-targets all
checkouts to a generated `merge_commit` which matches the behavior of
`pull_request:`. ([ref](actions/checkout#518))

Signed-off-by: dalthecow <[email protected]>
Bumps [next](https://github.com/vercel/next.js) from 15.3.2 to 15.3.4.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/vercel/next.js/releases">next's
releases</a>.</em></p>
<blockquote>
<h2>v15.3.4</h2>
<blockquote>
<p>[!NOTE]<br />
This release is backporting bug fixes. It does <strong>not</strong>
include all pending features/changes on canary.</p>
</blockquote>
<h3>Core Changes</h3>
<ul>
<li>[metadata] render streaming metadata on the top level (<a
href="https://redirect.github.com/vercel/next.js/issues/80566">#80566</a>)</li>
<li>[fix] clone the config module to avoid mutation (<a
href="https://redirect.github.com/vercel/next.js/issues/80573">#80573</a>)</li>
</ul>
<h3>Credits</h3>
<p>Huge thanks to <a
href="https://github.com/huozhi"><code>@​huozhi</code></a> for
helping!</p>
<h2>v15.3.3</h2>
<blockquote>
<p>[!NOTE]<br />
This release is backporting bug fixes. It does <strong>not</strong>
include all pending features/changes on canary.</p>
</blockquote>
<h3>Core Changes</h3>
<ul>
<li>Reinstate <code>vary</code> (<a
href="https://redirect.github.com/vercel/next.js/issues/79939">#79939</a>)</li>
<li>fix(next-swc): Fix interestingness detection for React Compiler (<a
href="https://redirect.github.com/vercel/next.js/issues/79558">#79558</a>)</li>
<li>fix(next-swc): Fix react compiler usefulness detector (<a
href="https://redirect.github.com/vercel/next.js/issues/79480">#79480</a>)</li>
<li>fix(dev-overlay): Better handle edge-case file paths in launchEditor
(<a
href="https://redirect.github.com/vercel/next.js/issues/79526">#79526</a>)</li>
<li>Client router should discard stale prefetch entries for static pages
(<a
href="https://redirect.github.com/vercel/next.js/issues/79362">#79362</a>)</li>
</ul>
<h3>Credits</h3>
<p>Huge thanks to <a
href="https://github.com/gaojude"><code>@​gaojude</code></a>, <a
href="https://github.com/kdy1"><code>@​kdy1</code></a>, <a
href="https://github.com/bgw"><code>@​bgw</code></a>, and <a
href="https://github.com/unstubbable"><code>@​unstubbable</code></a> for
helping!</p>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https://github.com/vercel/next.js/commit/f9e98b9ce39d69fac6841faeaa327d3848e0122c"><code>f9e98b9</code></a>
v15.3.4</li>
<li><a
href="https://github.com/vercel/next.js/commit/2557fba3996007f642df8a2b0c98b36c9ce9a343"><code>2557fba</code></a>
[fix] clone the config module to avoid mutation (<a
href="https://redirect.github.com/vercel/next.js/issues/80573">#80573</a>)</li>
<li><a
href="https://github.com/vercel/next.js/commit/616f6fa0908552126de3190e663e15491a267e13"><code>616f6fa</code></a>
[backport][metadata] render streaming metadata on the top level (<a
href="https://redirect.github.com/vercel/next.js/issues/80566">#80566</a>)</li>
<li><a
href="https://github.com/vercel/next.js/commit/3ab8db7383ae904af872e14c26a9bd8a43879819"><code>3ab8db7</code></a>
v15.3.3</li>
<li><a
href="https://github.com/vercel/next.js/commit/18c8113ebd90cfb54902c429a7c1138ed7d9f97d"><code>18c8113</code></a>
[backport] Reinstate <code>vary</code> (<a
href="https://redirect.github.com/vercel/next.js/issues/79939">#79939</a>)</li>
<li><a
href="https://github.com/vercel/next.js/commit/e18212f54668eba21ef8ac33f5db5a94f2ba3499"><code>e18212f</code></a>
re-enable vary header deploy test (<a
href="https://redirect.github.com/vercel/next.js/issues/79753">#79753</a>)</li>
<li><a
href="https://github.com/vercel/next.js/commit/ec202eccf05820b60c6126d6411fe16766ecc066"><code>ec202ec</code></a>
Revert &quot;[next-server] skip setting vary header for basic
routes&quot; (<a
href="https://redirect.github.com/vercel/next.js/issues/79426">#79426</a>)</li>
<li><a
href="https://github.com/vercel/next.js/commit/e2f264fdce0472001675418f7524ed355fa184a0"><code>e2f264f</code></a>
fix(next-swc): Fix interestingness detection for React Compiler (15.3)
(<a
href="https://redirect.github.com/vercel/next.js/issues/79558">#79558</a>)</li>
<li><a
href="https://github.com/vercel/next.js/commit/562fac78dab5656c6b8cbf6248a44a8f18da3075"><code>562fac7</code></a>
fix(next-swc): Fix react compiler usefulness detector (15.3) (<a
href="https://redirect.github.com/vercel/next.js/issues/79480">#79480</a>)</li>
<li><a
href="https://github.com/vercel/next.js/commit/06097fd7bb74f437fdb52d42a82280bce206f9d3"><code>06097fd</code></a>
fix(dev-overlay): Better handle edge-case file paths in launchEditor (<a
href="https://redirect.github.com/vercel/next.js/issues/79526">#79526</a>)</li>
<li>Additional commits viewable in <a
href="https://github.com/vercel/next.js/compare/v15.3.2...v15.3.4">compare
view</a></li>
</ul>
</details>
<br />

[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=next&package-manager=npm_and_yarn&previous-version=15.3.2&new-version=15.3.4)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)
You can disable automated security fix PRs for this repo from the
[Security Alerts
page](https://github.com/neuralmagic/guidellm/network/alerts).

</details>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: dalthecow <[email protected]>
Co-authored-by: Mark Kurtz <[email protected]>
Co-authored-by: Copilot <[email protected]>
Signed-off-by: dalthecow <[email protected]>
Small issue in ui-unit-test job is causing failures across all PRs since
we use pull_request_target instead of pull_request for the trigger in
development.yml which means that actions run against the workflow in
main instead of the workflow files that might have been updated in the
PR.

The last UI PR was merged and brought in this buggy ui-unit-test job.

I also updated the checkout action to v4 since the other non ui jobs are
using v4 instead of v3

Ended up updating package-lock as well since I was getting more failures
than expected, thought maybe that was the issue. Regardless, this PR
passes the checks when I temporarily update the trigger from
pull_request_target to pull_request

Signed-off-by: dalthecow <[email protected]>
`pull_request_target` has not been the drop-in replacement it promises
to be. Revert its addition and disable build jobs for external PRs.

---------

Co-authored-by: Copilot <[email protected]>
Signed-off-by: dalthecow <[email protected]>
Signed-off-by: dalthecow <[email protected]>
github-actions bot pushed a commit that referenced this pull request Jul 15, 2025
github-actions bot pushed a commit that referenced this pull request Jul 15, 2025
github-actions bot pushed a commit that referenced this pull request Jul 16, 2025
github-actions bot pushed a commit that referenced this pull request Jul 16, 2025
github-actions bot pushed a commit that referenced this pull request Jul 16, 2025
@markurtz markurtz requested review from markurtz and Copilot July 17, 2025 13:14
Copilot

This comment was marked as outdated.

github-actions bot pushed a commit that referenced this pull request Jul 17, 2025
Signed-off-by: dalthecow <[email protected]>
github-actions bot pushed a commit that referenced this pull request Jul 17, 2025
@markurtz markurtz requested a review from Copilot July 18, 2025 18:20
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements HTML output functionality for GuideLLM and fixes logarithmic scaling issues in charts. The PR adds the ability to generate HTML reports from benchmark data, updates UI components to work with the new data format, and fixes chart scaling problems. The majority of UI changes are necessary to update from an older data format to the current structure.

Reviewed Changes

Copilot reviewed 43 out of 46 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/guidellm/presentation/ New module for handling HTML report generation and data models
src/guidellm/config.py Added ReportGenerationSettings configuration
src/guidellm/benchmark/output.py Added HTML output support to save_html method
src/ui/lib/components/Charts/ Fixed logarithmic scaling in DashedLine and MetricLine components
tests/ Updated tests for new p50 percentile and data format changes
Comments suppressed due to low confidence (2)

@@ -0,0 +1,232 @@
import random
Copy link
Preview

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

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

Using the 'random' module for sampling in production code may not be cryptographically secure. Consider using 'secrets' module or documenting that this is for non-security purposes.

Copilot uses AI. Check for mistakes.

successful_requests = [
req for bm in benchmarks for req in bm.requests.successful
]
sample_indices = random.sample(
Copy link
Preview

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

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

Using random.sample() for selecting benchmark samples could be predictable. If this data could contain sensitive information, consider using secrets.SystemRandom() for sampling.

Suggested change
sample_indices = random.sample(
sample_indices = secrets.SystemRandom().sample(

Copilot uses AI. Check for mistakes.

:return: the html with the json data injected
:rtype: str
"""
head_match = re.search(r"<head[^>]*>(.*?)</head>", html, re.DOTALL | re.IGNORECASE)
Copy link
Preview

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

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

Using regex to parse HTML can be vulnerable to malformed input. Consider using an HTML parser library like BeautifulSoup or lxml for more robust HTML manipulation.

Copilot uses AI. Check for mistakes.

Comment on lines +100 to +105
let i = 1;
if (values[0] === 0) {
i++;
}
const valuesRatios = [];
for (let i = 1; i < values.length; i++) {
for (i; i < values.length; i++) {
Copy link
Preview

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

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

Same issue as in the helper function - the variable 'i' is reused and creates unclear control flow. Use a more descriptive name and clearer initialization.

Copilot uses AI. Check for mistakes.

@DaltheCow
Copy link
Collaborator Author

@jaredoconnell found that the app blanks out on firefox, looking into that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI Front-end workstream
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants