fixes skipped framework tests by sammaji · Pull Request #5611 · maximhq/bifrost

sammaji · 2026-07-28T09:19:22Z

Summary

Re-enables previously skipped pricing override tests and updates them to work with InputCostPerToken and OutputCostPerToken now being pointer types (*float64) rather than plain float64 values. Also removes a stale skipped test for a removed Azure API version column.

Changes

Removed TestEncryptedColumns_AzureAPIVersion_FitsAfterWidening which was already skipped with a note that the underlying column no longer exists.
Re-enabled multiple t.Skip()-guarded pricing override tests that were previously deferred.
Updated assertions in those tests to dereference pointer fields (*pricing.InputCostPerToken, *pricing.OutputCostPerToken) and added require.NotNil guards before each dereference.
Added RequestTypes fields to override fixtures in several tests to satisfy updated matching logic that requires request type scoping.

Type of change

Affected areas

How to test

go test ./framework/modelcatalog/datasheet/...
go test ./framework/configstore/tables/...

All previously skipped tests should now run and pass.

Breaking changes

Yes
No

Related issues

Security considerations

None.

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

coderabbitai · 2026-07-28T09:19:31Z

📝 Walkthrough

Walkthrough

The PR enables previously skipped pricing override tests, strengthens pointer-backed pricing assertions across resolution and patch scenarios, and removes an obsolete Azure encryption-width regression test.

Changes

Pricing override test coverage

Layer / File(s)	Summary
Override resolution assertions `framework/modelcatalog/datasheet/overrides_test.go`	Pricing resolution tests now execute and validate request-type precedence, fallback resolution, provider-prefixed model handling, and unchanged base pricing fields.
Override deletion and patch behavior `framework/modelcatalog/datasheet/overrides_test.go`	Tests now cover override removal, request-type-scoped wildcard specificity, and partial patches that preserve unspecified pricing fields.

Encryption test cleanup

Layer / File(s)	Summary
Obsolete encryption regression cleanup `framework/configstore/tables/encryption_test.go`	The obsolete Azure API version width test is removed; the legacy alias round-trip test retains its assertions.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: impoiler, pratham-mishra04

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 15.38% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title is related to the PR, but it is broad and doesn’t capture the specific test re-enablement and pointer-field updates.
Description check	✅ Passed	The description mostly follows the template with summary, changes, testing, and checklist, with only minor sections left sparse or empty.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch 07-28-fixes_skipped_framework_tests

_{Comment @coderabbitai help to get the list of available commands.}

sammaji · 2026-07-28T09:19:36Z

replaces deprecated huggingface model from core tests #5615
fixes skipped framework tests #5611 👈 (View in Graphite)
fixes skipped cases in semanticcache tests #5605
dev

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

coderabbitai

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

framework/modelcatalog/datasheet/overrides_test.go (1)
78-80: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Ensure every newly enabled fixture has explicit request-type coverage.

Overrides with empty RequestTypes do not match a request mode, so these tests can pass without exercising the behavior named in each test. (raw.githubusercontent.com)

framework/modelcatalog/datasheet/overrides_test.go#L78-L80: define the intended generic/request-specific contract explicitly.

framework/modelcatalog/datasheet/overrides_test.go#L214-L216: add schemas.ChatCompletionRequest so exact model matching is tested.

framework/modelcatalog/datasheet/overrides_test.go#L265-L267: add schemas.ChatCompletionRequest so wildcard non-matching is tested.
🤖 Prompt for 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.

In `@framework/modelcatalog/datasheet/overrides_test.go` around lines 78 - 80,
Update TestGetPricing_RequestTypeSpecificOverrideBeatsGeneric to explicitly
define both generic and request-specific override behavior through RequestTypes.
In framework/modelcatalog/datasheet/overrides_test.go at lines 78-80, establish
the intended generic/request-specific contract; at lines 214-216, add
schemas.ChatCompletionRequest to the exact-model fixture; and at lines 265-267,
add schemas.ChatCompletionRequest to the wildcard non-matching fixture so each
test exercises request-type filtering.

🤖 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 `@framework/modelcatalog/datasheet/overrides_test.go`:
- Line 304: Update the override test around SetOverrides to call DeleteOverride
with "openai-override-0" instead of clearing the full set. Keep a second
override configured, then verify only the targeted override is removed while the
remaining override persists.

---

Outside diff comments:
In `@framework/modelcatalog/datasheet/overrides_test.go`:
- Around line 78-80: Update
TestGetPricing_RequestTypeSpecificOverrideBeatsGeneric to explicitly define both
generic and request-specific override behavior through RequestTypes. In
framework/modelcatalog/datasheet/overrides_test.go at lines 78-80, establish the
intended generic/request-specific contract; at lines 214-216, add
schemas.ChatCompletionRequest to the exact-model fixture; and at lines 265-267,
add schemas.ChatCompletionRequest to the wildcard non-matching fixture so each
test exercises request-type filtering.

🪄 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: CHILL

Plan: Pro Plus

Run ID: 242f37e3-9116-4736-b1c8-deb175e4fb2b

📥 Commits

Reviewing files that changed from the base of the PR and between 00fb4a4 and 531da41.

📒 Files selected for processing (2)

framework/configstore/tables/encryption_test.go
framework/modelcatalog/datasheet/overrides_test.go

coderabbitai

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 `@framework/modelcatalog/datasheet/overrides_test.go`:
- Around line 111-112: Update the override fixtures in
framework/modelcatalog/datasheet/overrides_test.go:111-112, 238-239, and 268-271
so their request modes match the exercised request types. Configure
openai-generic for Responses mode at 111-112, add ChatCompletionRequest at
238-239, and add ChatCompletionRequest to the wildcard override at 268-271,
preserving each test’s intended precedence or non-match assertion.

🪄 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: CHILL

Plan: Pro Plus

Run ID: cc24d86e-47fa-4cae-94d6-f8629bc83adc

📥 Commits

Reviewing files that changed from the base of the PR and between e267a1f and 50d76e0.

📒 Files selected for processing (2)

framework/configstore/tables/encryption_test.go
framework/modelcatalog/datasheet/overrides_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

framework/configstore/tables/encryption_test.go

akshaydeo · 2026-07-29T07:50:44Z

Merge activity

Jul 29, 7:50 AM UTC: A user started a stack merge that includes this pull request via Graphite.
Jul 29, 7:51 AM UTC: @akshaydeo merged this pull request with Graphite.

The base branch was changed.

## Summary Re-enables previously skipped pricing override tests and updates them to work with `InputCostPerToken` and `OutputCostPerToken` now being pointer types (`*float64`) rather than plain `float64` values. Also removes a stale skipped test for a removed Azure API version column. ## Changes - Removed `TestEncryptedColumns_AzureAPIVersion_FitsAfterWidening` which was already skipped with a note that the underlying column no longer exists. - Re-enabled multiple `t.Skip()`-guarded pricing override tests that were previously deferred. - Updated assertions in those tests to dereference pointer fields (`*pricing.InputCostPerToken`, `*pricing.OutputCostPerToken`) and added `require.NotNil` guards before each dereference. - Added `RequestTypes` fields to override fixtures in several tests to satisfy updated matching logic that requires request type scoping. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./framework/modelcatalog/datasheet/... go test ./framework/configstore/tables/... ``` All previously skipped tests should now run and pass. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations None. ## 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

sammaji mentioned this pull request Jul 28, 2026

fixes skipped cases in semanticcache tests #5605

Merged

18 tasks

sammaji marked this pull request as ready for review July 28, 2026 09:19

Copilot AI review requested due to automatic review settings July 28, 2026 09:19

Copilot AI reviewed Jul 28, 2026

coderabbitai Bot requested review from Pratham-Mishra04 and impoiler July 28, 2026 09:21

coderabbitai Bot requested changes Jul 28, 2026

View reviewed changes

Comment thread framework/modelcatalog/datasheet/overrides_test.go Outdated

fixes skipped cases in semanticcache tests

f19842b

sammaji force-pushed the 07-28-fixes_skipped_framework_tests branch from 531da41 to e267a1f Compare July 28, 2026 11:07

sammaji force-pushed the 07-28-fixes_skipped_cases_in_semanticcache_tests branch from 00fb4a4 to f19842b Compare July 28, 2026 11:07

sammaji mentioned this pull request Jul 28, 2026

replaces deprecated huggingface model from core tests #5615

Merged

18 tasks

sammaji force-pushed the 07-28-fixes_skipped_framework_tests branch from e267a1f to 50d76e0 Compare July 28, 2026 11:12

coderabbitai Bot requested changes Jul 28, 2026

View reviewed changes

Comment thread framework/modelcatalog/datasheet/overrides_test.go

fixes skipped framework tests

61dc665

sammaji force-pushed the 07-28-fixes_skipped_framework_tests branch from 50d76e0 to 61dc665 Compare July 28, 2026 11:34

coderabbitai Bot previously approved these changes Jul 28, 2026

View reviewed changes

akshaydeo previously approved these changes Jul 29, 2026

View reviewed changes

akshaydeo changed the base branch from 07-28-fixes_skipped_cases_in_semanticcache_tests to graphite-base/5611 July 29, 2026 07:51

akshaydeo changed the base branch from graphite-base/5611 to dev July 29, 2026 07:51

akshaydeo merged commit b6ea806 into dev Jul 29, 2026
9 checks passed

akshaydeo deleted the 07-28-fixes_skipped_framework_tests branch July 29, 2026 07:51

Navigation Menu

fixes skipped framework tests - #5611

fixes skipped framework tests#5611
akshaydeo merged 2 commits into
devfrom
07-28-fixes_skipped_framework_tests

sammaji commented Jul 28, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 28, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

sammaji commented Jul 28, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

akshaydeo commented Jul 29, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

sammaji commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Type of change

Affected areas

How to test

Breaking changes

Related issues

Security considerations

Checklist

Uh oh!

coderabbitai Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

sammaji commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

akshaydeo commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sammaji commented Jul 28, 2026 •

edited

Loading

coderabbitai Bot commented Jul 28, 2026 •

edited

Loading

sammaji commented Jul 28, 2026 •

edited

Loading

akshaydeo commented Jul 29, 2026 •

edited

Loading