tests:harness tests with compat and fixes - #4379
Conversation
|
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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a ChangesCompat mode harness infrastructure
stop_sequences model parameter mapping
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
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 `@Makefile`:
- Line 1912: The COMPAT variable matching filter at Makefile line 1912 (anchor)
and its sibling locations at lines 1995 and 2019 inconsistently handle truthy
values and miss common uppercase variants like TRUE and ON. Update the filter
expressions in all three locations to normalize the truthy value matching by
adding uppercase equivalents (TRUE, ON) to the current set of accepted values
(on, true, 1, yes, YES, y, Y). This ensures that COMPAT=TRUE and COMPAT=ON are
treated the same as their lowercase counterparts and properly trigger compat
header injection.
🪄 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: f681a219-4d61-47bf-94a9-d36a0ecb137d
📒 Files selected for processing (3)
Makefileframework/modelcatalog/datasheet/types.gotests/e2e/api/collections/provider-harness.json
👮 Files not reviewed due to content moderation or server errors (1)
- tests/e2e/api/collections/provider-harness.json
Confidence Score: 5/5Safe to merge — changes are limited to the test harness Makefile, a Postman pre-request script, and a one-line parameter mapping fix with no impact on production code paths. All three changes are confined to test infrastructure and a small data-extraction helper. The Makefile COMPAT logic correctly gates compat header injection behind an explicit opt-in filter and propagates non-zero exit codes. The stop_sequences to stop mapping is additive and only affects the model catalog parameter surface list. No production request handling, streaming, pooling, or auth paths are touched. No files require special attention. Important Files Changed
Reviews (3): Last reviewed commit: "tests:harness tests with compat and fixe..." | Re-trigger Greptile |
4c3eb3d to
68d4de2
Compare
d652ac6 to
a968a6e
Compare
Merge activity
|
68d4de2 to
bbfc7a3
Compare
## Summary
Adds `COMPAT` mode support to the provider harness test runner, allowing the harness to be executed with the `x-bf-compat` header injected on every request. This enables side-by-side comparison of provider behavior with compat on vs. off without manually re-running the suite.
## Changes
- Added a `COMPAT` Makefile variable that accepts `on`, `off`, or `both`. When set to `both`, the harness runs sequentially with compat off then on, saving separate report artifacts (`newman-report-compat-{off,on}.{json,html}`, `harness-failures-compat-{off,on}.md`) under `tmp/`.
- When `COMPAT=on` (or `true`/`1`/`yes`/`y`), `--env-var "compat=true"` is passed to all three Newman invocation paths in the harness target.
- Added a pre-request script in the harness Postman collection that reads the `compat` environment variable and, when present, upserts the `x-bf-compat` header on every outgoing request.
- Fixed `stop_sequences` → `stop` mapping in the model catalog datasheet parameter extractor so that `stop` is correctly surfaced as a supported top-level request parameter.
## Type of change
- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [ ] Core (Go)
- [ ] Transports (HTTP)
- [x] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs
## How to test
Run the harness in each mode and verify separate report files are produced:
```sh
# Run with compat off (baseline)
make run-provider-harness-test COMPAT=off
# Run with compat on
make run-provider-harness-test COMPAT=on
# Run both sequentially and produce split reports
make run-provider-harness-test COMPAT=both
# Verify artifacts
ls tmp/newman-report-compat-{off,on}.{json,html}
ls tmp/harness-failures-compat-{off,on}.md
```
`COMPAT` is unset by default, preserving existing harness behavior.
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
The `x-bf-compat` header is injected only during test runs via Newman and is controlled by an explicit opt-in variable. No secrets or PII are involved.
## 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
Adds `COMPAT` mode support to the provider harness test runner, allowing the harness to be executed with the `x-bf-compat` header injected on every request. This enables side-by-side comparison of provider behavior with compat on vs. off without manually re-running the suite.
## Changes
- Added a `COMPAT` Makefile variable that accepts `on`, `off`, or `both`. When set to `both`, the harness runs sequentially with compat off then on, saving separate report artifacts (`newman-report-compat-{off,on}.{json,html}`, `harness-failures-compat-{off,on}.md`) under `tmp/`.
- When `COMPAT=on` (or `true`/`1`/`yes`/`y`), `--env-var "compat=true"` is passed to all three Newman invocation paths in the harness target.
- Added a pre-request script in the harness Postman collection that reads the `compat` environment variable and, when present, upserts the `x-bf-compat` header on every outgoing request.
- Fixed `stop_sequences` → `stop` mapping in the model catalog datasheet parameter extractor so that `stop` is correctly surfaced as a supported top-level request parameter.
## Type of change
- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [ ] Core (Go)
- [ ] Transports (HTTP)
- [x] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs
## How to test
Run the harness in each mode and verify separate report files are produced:
```sh
# Run with compat off (baseline)
make run-provider-harness-test COMPAT=off
# Run with compat on
make run-provider-harness-test COMPAT=on
# Run both sequentially and produce split reports
make run-provider-harness-test COMPAT=both
# Verify artifacts
ls tmp/newman-report-compat-{off,on}.{json,html}
ls tmp/harness-failures-compat-{off,on}.md
```
`COMPAT` is unset by default, preserving existing harness behavior.
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
The `x-bf-compat` header is injected only during test runs via Newman and is controlled by an explicit opt-in variable. No secrets or PII are involved.
## 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
Adds `COMPAT` mode support to the provider harness test runner, allowing the harness to be executed with the `x-bf-compat` header injected on every request. This enables side-by-side comparison of provider behavior with compat on vs. off without manually re-running the suite.
## Changes
- Added a `COMPAT` Makefile variable that accepts `on`, `off`, or `both`. When set to `both`, the harness runs sequentially with compat off then on, saving separate report artifacts (`newman-report-compat-{off,on}.{json,html}`, `harness-failures-compat-{off,on}.md`) under `tmp/`.
- When `COMPAT=on` (or `true`/`1`/`yes`/`y`), `--env-var "compat=true"` is passed to all three Newman invocation paths in the harness target.
- Added a pre-request script in the harness Postman collection that reads the `compat` environment variable and, when present, upserts the `x-bf-compat` header on every outgoing request.
- Fixed `stop_sequences` → `stop` mapping in the model catalog datasheet parameter extractor so that `stop` is correctly surfaced as a supported top-level request parameter.
## Type of change
- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [ ] Core (Go)
- [ ] Transports (HTTP)
- [x] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs
## How to test
Run the harness in each mode and verify separate report files are produced:
```sh
# Run with compat off (baseline)
make run-provider-harness-test COMPAT=off
# Run with compat on
make run-provider-harness-test COMPAT=on
# Run both sequentially and produce split reports
make run-provider-harness-test COMPAT=both
# Verify artifacts
ls tmp/newman-report-compat-{off,on}.{json,html}
ls tmp/harness-failures-compat-{off,on}.md
```
`COMPAT` is unset by default, preserving existing harness behavior.
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
The `x-bf-compat` header is injected only during test runs via Newman and is controlled by an explicit opt-in variable. No secrets or PII are involved.
## 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
Adds
COMPATmode support to the provider harness test runner, allowing the harness to be executed with thex-bf-compatheader injected on every request. This enables side-by-side comparison of provider behavior with compat on vs. off without manually re-running the suite.Changes
COMPATMakefile variable that acceptson,off, orboth. When set toboth, the harness runs sequentially with compat off then on, saving separate report artifacts (newman-report-compat-{off,on}.{json,html},harness-failures-compat-{off,on}.md) undertmp/.COMPAT=on(ortrue/1/yes/y),--env-var "compat=true"is passed to all three Newman invocation paths in the harness target.compatenvironment variable and, when present, upserts thex-bf-compatheader on every outgoing request.stop_sequences→stopmapping in the model catalog datasheet parameter extractor so thatstopis correctly surfaced as a supported top-level request parameter.Type of change
Affected areas
How to test
Run the harness in each mode and verify separate report files are produced:
COMPATis unset by default, preserving existing harness behavior.Screenshots/Recordings
N/A
Breaking changes
Related issues
N/A
Security considerations
The
x-bf-compatheader is injected only during test runs via Newman and is controlled by an explicit opt-in variable. No secrets or PII are involved.Checklist
docs/contributing/README.mdand followed the guidelines