Skip to content

fix: load-test script build paths and provider config - #4358

Merged
akshaydeo merged 2 commits into
maximhq:devfrom
G-XD:fix/load-test-script
Jun 13, 2026
Merged

fix: load-test script build paths and provider config#4358
akshaydeo merged 2 commits into
maximhq:devfrom
G-XD:fix/load-test-script

Conversation

@G-XD

@G-XD G-XD commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Summary

.github/workflows/scripts/load-test.sh could not run successfully due to several issues: wrong working directory for go build, wrong clone location for the bifrost-benchmarking mocker repo, an invalid openai provider config in the generated config.json, and a missing ui/ directory required by the //go:embed all:ui directive in transports/bifrost-http/main.go. This PR fixes the script so it can run end-to-end.

Changes

  • build_bifrost_http: cd "${BIFROST_HTTP_DIR}" (was ${TRANSPORTS_DIR}). go build must run from transports/bifrost-http/, which contains main.go.
  • build_bifrost_http: create transports/bifrost-http/ui/ with a placeholder file before building, to satisfy //go:embed all:ui. This mirrors what run-migration-tests.sh already does; the load test does not need real UI assets.
  • setup_mocker: git clone into ${REPO_ROOT}/.. instead of ${WORK_DIR}, so the cloned path matches MOCKER_DIR=${REPO_ROOT}/../bifrost-benchmarking/mocker.
  • create_config: add the required "models": ["*"] field to the openai key (per the schemas.Key whitelist requirement).
  • create_config: remove custom_provider_config from the openai provider — that block is only valid for custom providers (where base_provider_type is set) and causes validation errors under the standard openai provider.

No design trade-offs — all changes are mechanical bug fixes.

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs
  • CI scripts (.github/workflows/scripts/)

How to test

From the repo root:

bash .github/workflows/scripts/load-test.sh

Expected:

  • bifrost-http builds successfully from transports/bifrost-http/
  • bifrost-benchmarking is cloned to ../bifrost-benchmarking (sibling of repo root)
  • config.json is generated and accepted by bifrost-http at startup (no provider validation error)
  • The calibration phase, overhead phase, and stress phase run to completion and the script prints the final summary table

No new configs or environment variables introduced.

Screenshots/Recordings

N/A — CI-only shell script.

Breaking changes

  • Yes
  • No

Related issues

None.

Security considerations

None. Only changes a load-test shell script and its generated config; no auth, secrets, PII, or sandboxing changes.

Checklist

  • I read docs/contributing/README.md and followed the guidelines
  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

Summary by CodeRabbit

  • Chores
    • Improved local load-testing and build setup to ensure UI assets are present during local builds.
    • Updated mock provider configuration to permit all models for broader testing coverage.
    • Streamlined mock setup/clone process for more reliable initialization in local runs.
    • Minor script formatting tweak for consistency.

@G-XD
G-XD requested a review from a team as a code owner June 13, 2026 07:51
@CLAassistant

CLAassistant commented Jun 13, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@coderabbitai

coderabbitai Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ce57a757-5d02-40e9-a954-6ce1ad0e8c11

📥 Commits

Reviewing files that changed from the base of the PR and between f3056c9 and 9ce9b85.

📒 Files selected for processing (1)
  • .github/workflows/scripts/load-test.sh

📝 Walkthrough

Walkthrough

Load-test script updated to create a placeholder ui/ directory before building bifrost-http, change the mocker checkout to ${REPO_ROOT}/.., add models: ["*"] to the generated OpenAI-style mock provider key config, and add a trailing newline after main "$@".

Changes

Load Test Script Configuration

Layer / File(s) Summary
Build system asset directory setup
.github/workflows/scripts/load-test.sh
build_bifrost_http creates a ui/ directory with a .gitkeep placeholder before running go build so embedded UI assets are available during local builds.
Mocking infra checkout and provider config
.github/workflows/scripts/load-test.sh
setup_mocker performs cd "${REPO_ROOT}/.." before cloning bifrost-benchmarking. Generated OpenAI-style config.json adds models: ["*"] to key configuration and removes the prior custom provider/request-allowlist block.
Script final newline
.github/workflows/scripts/load-test.sh
A trailing blank line was added after the final main "$@" invocation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • akshaydeo

Poem

A rabbit hops through scripts so spry,
Drops a .gitkeep where UI should lie,
Tweaks the mock clone and models too,
Adds a newline — tidy and true. 🐇✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: load-test script build paths and provider config' clearly and concisely summarizes the main changes: fixing the load-test script's build paths and provider configuration issues.
Description check ✅ Passed The PR description follows the template closely with all critical sections completed: summary explaining the problem, detailed changes with rationale, type of change marked, affected areas selected, testing instructions provided, breaking changes addressed, and relevant checklist items marked.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

Safe to merge — all changes are isolated to a CI load-test shell script with no impact on production code paths.

Every change is a direct, verifiable correction: the build directory mismatch and missing ui/ placeholder would both cause a compile failure, the clone path mismatch would cause mocker build to fail, and the config changes match what the schema and provider validation actually require. No logic is shared with production code.

No files require special attention.

Important Files Changed

Filename Overview
.github/workflows/scripts/load-test.sh Five targeted bug fixes: correct go build working directory, placeholder ui/ dir for //go:embed, correct git clone target directory, required "models" field in key config, and removal of invalid custom_provider_config from standard openai provider.

Reviews (3): Last reviewed commit: "fix: ensure ui placeholder exists even w..." | Re-trigger Greptile

@G-XD
G-XD force-pushed the fix/load-test-script branch from e55f6f8 to f3056c9 Compare June 13, 2026 07:57

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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/scripts/load-test.sh:
- Around line 175-178: The script only creates a placeholder when the ui
directory is missing, but the Go embed rule (//go:embed all:ui in main.go)
requires at least one file even if the directory exists; update the shell
snippet that uses ${BIFROST_HTTP_DIR}/ui so that it: (1) always creates the
directory if missing, and (2) checks whether the directory contains any files
(e.g., test for glob "${BIFROST_HTTP_DIR}/ui/*") and writes a placeholder file
(e.g., .gitkeep) into ${BIFROST_HTTP_DIR}/ui when it is empty; reference the
BIFROST_HTTP_DIR/ui path and the .gitkeep placeholder in your change.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a48a8018-a230-499e-8da8-1b4a02463073

📥 Commits

Reviewing files that changed from the base of the PR and between e55f6f8 and f3056c9.

📒 Files selected for processing (1)
  • .github/workflows/scripts/load-test.sh

Comment thread .github/workflows/scripts/load-test.sh Outdated
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: G-XD <gxd0606@gmail.com>
@akshaydeo

Copy link
Copy Markdown
Contributor

yeah loadtest repo is not used on CI yet - it was internally used

@akshaydeo

Copy link
Copy Markdown
Contributor

❤️ for the PR

@akshaydeo
akshaydeo merged commit ff4c7d1 into maximhq:dev Jun 13, 2026
6 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Jun 13, 2026
akshaydeo pushed a commit that referenced this pull request Jun 14, 2026
* fix: load-test script build paths and provider config

* fix: ensure ui placeholder exists even when ui dir is empty

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: G-XD <gxd0606@gmail.com>

---------

Signed-off-by: G-XD <gxd0606@gmail.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@G-XD
G-XD deleted the fix/load-test-script branch June 15, 2026 02:11
akshaydeo pushed a commit that referenced this pull request Jun 15, 2026
* fix: load-test script build paths and provider config

* fix: ensure ui placeholder exists even when ui dir is empty

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: G-XD <gxd0606@gmail.com>

---------

Signed-off-by: G-XD <gxd0606@gmail.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
akhsaul pushed a commit to akhsaul/bifrost that referenced this pull request Aug 27, 2026
* fix: load-test script build paths and provider config

* fix: ensure ui placeholder exists even when ui dir is empty

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: G-XD <gxd0606@gmail.com>

---------

Signed-off-by: G-XD <gxd0606@gmail.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
occcat pushed a commit to occcat/bifrost that referenced this pull request Sep 2, 2026
* fix: load-test script build paths and provider config

* fix: ensure ui placeholder exists even when ui dir is empty

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: G-XD <gxd0606@gmail.com>

---------

Signed-off-by: G-XD <gxd0606@gmail.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.

3 participants