build-fix

[akshaydeo]

No description provided.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: eb4b864a-2b9f-4181-8581-22cc9b259571

📥 Commits

Reviewing files that changed from the base of the PR and between 7e2c964 and f325e92.

📒 Files selected for processing (2)

• transports/bifrost-http/handlers/list_models_vk.go
• transports/bifrost-http/handlers/list_models_vk_test.go

---

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes

  • Improved virtual key status validation to correctly handle cases where the status indicator is missing or explicitly disabled, preventing potential errors in model list retrieval.

• Tests

  • Updated test cases to properly validate the enhanced virtual key status handling.

Walkthrough

applyListModelsVirtualKeyProviderFilter now treats a virtual key as inactive when vk.IsActive is nil in addition to when it is explicitly false. Corresponding test fixtures are updated to pass *bool pointer values via new(true) and new(false) instead of bare boolean literals.

Changes

Nil-safe IsActive check and matching test fixtures

Layer / File(s)	Summary
Nil guard on vk.IsActive and updated test fixtures transports/bifrost-http/handlers/list_models_vk.go, transports/bifrost-http/handlers/list_models_vk_test.go	The inactive-check condition in applyListModelsVirtualKeyProviderFilter adds a vk.IsActive == nil guard before dereferencing. Test fixtures for active and inactive virtual key cases are updated from bare bool literals to *bool pointers (new(true) / new(false)) to match the field type.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

• maximhq/bifrost#3187: Modifies the same list_models_vk.go virtual-key filtering flow, treating inactive or invalid resolved VKs as non-scoping.
• maximhq/bifrost#4074: Centers on the same applyListModelsVirtualKeyProviderFilter virtual-key scoping logic, replacing the handler-side helper with a governance pre-hook.

Suggested reviewers

• BearTS
• danpiths
• roroghost17

🐇 A pointer was nil, oh what a fright,
IsActive could crash in the dead of the night.
Now we check if it's missing before we dereference,
No more panics — just calm, safe inference.
Hop hop, the tests pass, all booleans aligned! 🎉

✨ 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 dev

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 @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Confidence Score: 3/5

The handler incorrectly bypasses provider filtering for virtual keys whose IsActive field is nil (the DB-default active state), meaning governance restrictions on model listing are silently dropped for those keys.

The nil guard inverts the documented and code-level semantics of IsActive: the struct comment, the DB default, and the existing IsActiveValue() helper all say nil means active, but the handler treats nil as inactive. Any VK stored without an explicit IsActive value will skip provider config enforcement on GET /v1/models, bypassing the filtering this function is meant to enforce.

transports/bifrost-http/handlers/list_models_vk.go — the IsActive nil guard needs to be replaced with the IsActiveValue() helper.

Important Files Changed

Filename	Overview
transports/bifrost-http/handlers/list_models_vk.go	Migrates IsActive bool field to *bool pointer to match updated schema, but the nil guard inverts the DB-default semantics: nil should be active, but the new condition skips provider filtering for nil IsActive VKs. Should use the existing IsActiveValue() helper instead.
transports/bifrost-http/handlers/list_models_vk_test.go	Tests correctly updated to use pointer literals (new(true)/new(false)) for the IsActive field; the nil IsActive case is not tested, leaving the behavior gap undetected.

Comments Outside Diff (1)

1. transports/bifrost-http/handlers/list_models_vk_test.go, line 27-62 (link)

   Missing nil-IsActive test case

   The test suite covers IsActive: new(true) and IsActive: new(false) but not IsActive: nil. Per the struct definition, nil is the DB-default meaning "active". A test asserting that a VK with IsActive == nil applies provider filtering (same as new(true)) would prevent the regression introduced by the current guard logic from slipping through silently.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

_{Reviews (1): Last reviewed commit: "Merge branch 'main' into dev" | Re-trigger Greptile}

[greptile-apps]

Nil IsActive is semantically active per the field definition (// Nil means true (DB default); false means inactive), but this guard treats it as inactive — provider configs will be silently ignored for any VK whose IsActive field is nil. A helper IsActiveValue() already exists on the struct that handles this correctly; using it eliminates the inversion.

Suggested change

	if vk == nil || vk.IsActive == nil || !*vk.IsActive {
	if vk == nil || !vk.IsActiveValue() {