fix: harness test fixes - #3887
Conversation
|
Warning Review limit reached
More reviews will be available in 7 minutes and 13 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR updates test harness configuration and model identifiers across two files. The Makefile now passes Bedrock guardrail environment variables to newman test execution in both parallel and sequential paths. The provider-harness JSON test collection updates Azure embeddings and audio model identifiers to newer variants, adjusts a Bedrock performance test entry, and updates an OpenAI stop-sequence test entry. ChangesTest Harness and Configuration Updates
Possibly related PRs
Suggested reviewers
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
tejas ghatte seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Confidence Score: 5/5Safe to merge — all changes are confined to test configuration files with no impact on production code paths. The diff touches only the Newman collection JSON and Makefile test targets. Model name corrections and env-var passthrough are self-contained, and the bash conditional expansion syntax in the Makefile is correct. No logic changes to core, transports, or plugins. No files require special attention. Important Files Changed
Reviews (3): Last reviewed commit: "fix: harness test fixes" | Re-trigger Greptile |
Merge activity
|
e2dac17 to
fc8c127
Compare
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 `@Makefile`:
- Around line 1941-1942: The non-interactive sequential branch of the Makefile
(the `PARALLEL=0` / sequential `newman run` command) is missing passthrough of
the BEDROCK_GUARDRAIL_IDENTIFIER and BEDROCK_GUARDRAIL_VERSION env flags; update
the sequential `newman run` invocation to include the same conditional flags
used in the TTY/monitor path (i.e., add
$${BEDROCK_GUARDRAIL_IDENTIFIER:+--env-var
"bedrockGuardrailIdentifier=$$BEDROCK_GUARDRAIL_IDENTIFIER"} and
$${BEDROCK_GUARDRAIL_VERSION:+--env-var
"bedrockGuardrailVersion=$$BEDROCK_GUARDRAIL_VERSION"} to the command) so
CI/non-TTY runs honor guardrail settings.
In `@tests/e2e/api/collections/provider-harness.json`:
- Line 679: The test JSON has a model name "azure/text-embedding-ada-002" but
the URL.path and raw URL still point to "text-embedding-3-small:embedContent";
update the URL.path segment and the url.raw string to use
"text-embedding-ada-002:embedContent" so the request target matches the "name"
field (adjust the URL.path array element and the raw URL string in the same JSON
object).
🪄 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
Run ID: 354bc099-2485-4b56-b50d-a2d6ca973915
📒 Files selected for processing (2)
Makefiletests/e2e/api/collections/provider-harness.json
fc8c127 to
ef74d3f
Compare
## Summary
Updates the provider harness test suite to use correct and available Azure model names, adds support for passing AWS Bedrock Guardrail identifiers and versions into the Newman test runner, and pins the Bedrock performance config test to a model that supports it.
## Changes
- Added `BEDROCK_GUARDRAIL_IDENTIFIER` and `BEDROCK_GUARDRAIL_VERSION` environment variable passthrough to the Newman invocations in the `Makefile`, allowing guardrail-aware Bedrock tests to be run from the harness
- Replaced `azure/text-embedding-2-ada` with the correct model name `azure/text-embedding-ada-002` across all embedding test entries (OpenAI, OpenAI-compat, and GenAI endpoints)
- Replaced `azure/whisper-1` with `azure/gpt-4o-transcribe` for audio transcription tests
- Replaced `azure/tts-1` with `azure/gpt-4o-mini-tts` for text-to-speech tests
- Changed the "Bedrock Converse: performance config optimized" test to target `us.amazon.nova-pro-v1:0` directly instead of the generic `{{bedrockModel}}` variable, since `performanceConfig` is only supported on specific models
- Replaced `openai/gpt-5` with `openai/gpt-4o-mini` in the stop sequences cross-cut test
## Type of change
- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [x] Chore/CI
## Affected areas
- [ ] Core (Go)
- [ ] Transports (HTTP)
- [x] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs
## How to test
Run the provider harness with optional Bedrock guardrail variables:
```sh
# Without guardrails
make run-provider-harness-test
# With guardrails
BEDROCK_GUARDRAIL_IDENTIFIER=my-guardrail-id \
BEDROCK_GUARDRAIL_VERSION=1 \
make run-provider-harness-test
```
Verify that:
- Azure embedding, transcription, and TTS tests pass with the updated model names
- The Bedrock performance config test passes when targeting `us.amazon.nova-pro-v1:0`
- The stop sequences cross-cut test passes with `openai/gpt-4o-mini`
- Guardrail env vars are forwarded to Newman when set
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
## Security considerations
`BEDROCK_GUARDRAIL_IDENTIFIER` and `BEDROCK_GUARDRAIL_VERSION` are passed as Newman environment variables. These values are not secrets but should be treated as infrastructure configuration and kept out of public logs if they reference sensitive guardrail resources.
## Checklist
- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit
* **Chores**
* Updated test harnesses to use newer Azure model identifiers for embeddings and audio services, including GPT-4o variants.
* Enhanced test execution to support Bedrock guardrail configuration parameters.
* Updated test matrices with latest model versions for improved compatibility testing.
<!-- review_stack_entry_start -->
[](https://app.coderabbit.ai/change-stack/maximhq/bifrost/pull/3887?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)
<!-- review_stack_entry_end -->
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary
Updates the provider harness test suite to use correct and available Azure model names, adds support for passing AWS Bedrock Guardrail identifiers and versions into the Newman test runner, and pins the Bedrock performance config test to a model that supports it.
## Changes
- Added `BEDROCK_GUARDRAIL_IDENTIFIER` and `BEDROCK_GUARDRAIL_VERSION` environment variable passthrough to the Newman invocations in the `Makefile`, allowing guardrail-aware Bedrock tests to be run from the harness
- Replaced `azure/text-embedding-2-ada` with the correct model name `azure/text-embedding-ada-002` across all embedding test entries (OpenAI, OpenAI-compat, and GenAI endpoints)
- Replaced `azure/whisper-1` with `azure/gpt-4o-transcribe` for audio transcription tests
- Replaced `azure/tts-1` with `azure/gpt-4o-mini-tts` for text-to-speech tests
- Changed the "Bedrock Converse: performance config optimized" test to target `us.amazon.nova-pro-v1:0` directly instead of the generic `{{bedrockModel}}` variable, since `performanceConfig` is only supported on specific models
- Replaced `openai/gpt-5` with `openai/gpt-4o-mini` in the stop sequences cross-cut test
## Type of change
- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [x] Chore/CI
## Affected areas
- [ ] Core (Go)
- [ ] Transports (HTTP)
- [x] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs
## How to test
Run the provider harness with optional Bedrock guardrail variables:
```sh
# Without guardrails
make run-provider-harness-test
# With guardrails
BEDROCK_GUARDRAIL_IDENTIFIER=my-guardrail-id \
BEDROCK_GUARDRAIL_VERSION=1 \
make run-provider-harness-test
```
Verify that:
- Azure embedding, transcription, and TTS tests pass with the updated model names
- The Bedrock performance config test passes when targeting `us.amazon.nova-pro-v1:0`
- The stop sequences cross-cut test passes with `openai/gpt-4o-mini`
- Guardrail env vars are forwarded to Newman when set
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
## Security considerations
`BEDROCK_GUARDRAIL_IDENTIFIER` and `BEDROCK_GUARDRAIL_VERSION` are passed as Newman environment variables. These values are not secrets but should be treated as infrastructure configuration and kept out of public logs if they reference sensitive guardrail resources.
## Checklist
- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit
* **Chores**
* Updated test harnesses to use newer Azure model identifiers for embeddings and audio services, including GPT-4o variants.
* Enhanced test execution to support Bedrock guardrail configuration parameters.
* Updated test matrices with latest model versions for improved compatibility testing.
<!-- review_stack_entry_start -->
[](https://app.coderabbit.ai/change-stack/maximhq/bifrost/pull/3887?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)
<!-- review_stack_entry_end -->
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Summary
Updates the provider harness test suite to use correct and available Azure model names, adds support for passing AWS Bedrock Guardrail identifiers and versions into the Newman test runner, and pins the Bedrock performance config test to a model that supports it.
Changes
BEDROCK_GUARDRAIL_IDENTIFIERandBEDROCK_GUARDRAIL_VERSIONenvironment variable passthrough to the Newman invocations in theMakefile, allowing guardrail-aware Bedrock tests to be run from the harnessazure/text-embedding-2-adawith the correct model nameazure/text-embedding-ada-002across all embedding test entries (OpenAI, OpenAI-compat, and GenAI endpoints)azure/whisper-1withazure/gpt-4o-transcribefor audio transcription testsazure/tts-1withazure/gpt-4o-mini-ttsfor text-to-speech testsus.amazon.nova-pro-v1:0directly instead of the generic{{bedrockModel}}variable, sinceperformanceConfigis only supported on specific modelsopenai/gpt-5withopenai/gpt-4o-miniin the stop sequences cross-cut testType of change
Affected areas
How to test
Run the provider harness with optional Bedrock guardrail variables:
Verify that:
us.amazon.nova-pro-v1:0openai/gpt-4o-miniBreaking changes
Related issues
Security considerations
BEDROCK_GUARDRAIL_IDENTIFIERandBEDROCK_GUARDRAIL_VERSIONare passed as Newman environment variables. These values are not secrets but should be treated as infrastructure configuration and kept out of public logs if they reference sensitive guardrail resources.Checklist
docs/contributing/README.mdand followed the guidelinesSummary by CodeRabbit