fixes failing tests

[akshaydeo]

Summary

Fixes database test isolation issues by preserving the shared migrations table across concurrent test packages, and changes the default streaming behavior for prompts to enable streaming by default when no explicit stream parameter is provided.

Changes

• Database test isolation: Modified test setup functions across configstore, logstore, and performance tests to use DELETE FROM migrations instead of DROP TABLE migrations, preventing interference between concurrent test packages that share the same PostgreSQL instance
• Streaming behavior: Updated prompt handling to default to streaming enabled (true) when no stream parameter is specified in model parameters, rather than defaulting to disabled
• Model access control: Enhanced key-based model filtering to support alias matching through the model catalog, allowing keys that permit specific model versions to also grant access to their base model names in listings

Type of change

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

Affected areas

• Core (Go)
• Transports (HTTP)
• Providers/Integrations
• Plugins
• UI (Next.js)
• Docs

How to test

Validate database test isolation and streaming behavior:

# Test database migrations and isolation
go test ./framework/configstore -v
go test ./framework/logstore -v

# Test streaming behavior changes
go test ./plugins/prompts -v

# Test model access control with aliases
go test ./transports/bifrost-http/handlers -v

# Run full test suite to ensure no regressions
go test ./...

Screenshots/Recordings

N/A

Breaking changes

• Yes
• No

The streaming behavior change is potentially breaking - applications that previously relied on streaming being disabled by default when no stream parameter was provided will now have streaming enabled by default. This affects the prompt plugin's behavior and may impact downstream consumers expecting non-streaming responses.

Related issues

N/A

Security considerations

The database test isolation changes improve test reliability but don't introduce security implications. The streaming default change and model alias matching don't affect authentication or authorization mechanisms.

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]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a29e42ac-6116-4270-83c4-3b15f27f9293

📥 Commits

Reviewing files that changed from the base of the PR and between 047d101 and 33fd1fc.

📒 Files selected for processing (5)

• framework/configstore/migrations_test.go
• framework/logstore/migrations_test.go
• framework/logstore/rdb_postgres_perf_test.go
• plugins/prompts/plugin_test.go
• transports/bifrost-http/handlers/providers.go

---

📝 Walkthrough

Summary by CodeRabbit

• Tests

  • Improved test reliability and isolation for database migration tracking in concurrent testing scenarios
  • Enhanced test validation for stream parameter handling in model operations and configurations
  • Refined test state management for configuration and logging database operations across test runs

• Bug Fixes

  • Expanded model access control to recognize and support model aliases during permission verification

Walkthrough

The PR modifies database test setup/teardown across three files to preserve shared migrations tables while clearing their contents (switching from DROP to CREATE IF NOT EXISTS + DELETE), preventing concurrent test interference. Additionally, model access control in the provider handler gains catalog-aware alias matching for allowlisted models.

Changes

Cohort / File(s)	Summary
Database Test Migration Handling framework/configstore/migrations_test.go, framework/logstore/migrations_test.go, framework/logstore/rdb_postgres_perf_test.go	Updated test setup/cleanup to preserve the shared migrations table by replacing DROP TABLE with CREATE TABLE IF NOT EXISTS followed by DELETE FROM migrations. Ensures table schema persists across concurrent test runs while clearing migration tracking records.
Plugin Stream Parameter Assertions plugins/prompts/plugin_test.go	Updated test case expectations in TestIncludesStreamInModelParams and TestHTTPTransportPreHook_NoStreamParam_NoStreamContext to assert that missing or empty "stream" parameters now return/resolve to true instead of false/nil.
Model Allowlist Access Control transports/bifrost-http/handlers/providers.go	Extended keyAllowsModelForList function to accept a ModelCatalog parameter and added catalog-aware alias matching logic. When a queried model is not directly allowlisted, the function now iterates allowlisted models and grants access if their base model name matches via catalog.GetBaseModelName().

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Migrations table stands so tall,
No more dropped when tests do call,
Catalog maps models with care,
Aliases matched everywhere!
Streams flow true, concurrency fair. ✨

✨ 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 04-08-fixes_failing_tests

---

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

[akshaydeo]

• fixes failing tests #2571 👈 (View in Graphite)
• v1.5.0

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

[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.}

[greptile-apps]

Confidence Score: 5/5

Safe to merge; the only findings are P2 test naming and coverage observations that do not affect production behaviour.

All three code changes (DB test isolation, streaming default, alias matching) are logically correct. The two P2 findings are limited to test names and an unreachable unit-test case — neither affects runtime correctness or data integrity.

plugins/prompts/plugin_test.go — misleading test name and an unreachable unit-test case worth cleaning up before the file grows larger.

Vulnerabilities

No security concerns identified. The alias matching in keyAllowsModelForList only affects model listing visibility, not actual request authorization. The streaming default change and DB test isolation fixes do not affect authentication or authorization mechanisms.

Important Files Changed

Filename	Overview
framework/configstore/migrations_test.go	Uses DELETE FROM migrations instead of DROP TABLE to preserve the shared migrations table for concurrent test packages; logic is sound.
framework/logstore/migrations_test.go	Same DELETE FROM migrations pattern applied consistently; cleanup is scoped to logs table and GIN index only, leaving migrations table intact.
framework/logstore/rdb_postgres_perf_test.go	Test setup and cleanup both use DELETE FROM migrations correctly; no regression risk.
plugins/prompts/plugin_test.go	Test name and function comment for TestHTTPTransportPreHook_NoStreamParam_NoStreamContext directly contradict the actual assertion (stream IS expected to be true); the empty-params test case in TestIncludesStreamInModelParams exercises a code branch that is unreachable through the production path.
transports/bifrost-http/handlers/providers.go	Alias matching in keyAllowsModelForList is directionally correct (specific-version → base) and only affects listing visibility, not auth.

Comments Outside Diff (1)

1. plugins/prompts/plugin_test.go, line 807-827 (link)

   Test name and doc comment contradict the assertion

   Both the function name (_NoStreamContext) and the leading comment ("the stream context key is not set") describe the old behaviour, but the assertion on line 825 expects BifrostContextKeyPromptStreamRequest to equal true. Any future reader relying on the name or comment will get exactly the wrong mental model of what this PR changed.

_{Reviews (1): Last reviewed commit: "fixes failing tests" | Re-trigger Greptile}

[greptile-apps]

Unit-test case is unreachable through the production path

includesStreamInModelParams correctly returns true for an empty map, but setPromptStreamFromVersionForTransport (in main.go) guards with len(version.ModelParams) == 0 { return } before ever calling this function. A version with no params will therefore never set BifrostContextKeyPromptStreamRequest, regardless of what this function returns.

The isolated unit test is harmless, but it gives a false signal about observable behaviour. Consider adding an integration-style test through HTTPTransportPreHook with a version whose ModelParams is truly empty, and document the expected result explicitly (streaming context is not set in that case).