Skip to content

fix: exclude empty model entries from model rankings matview queries - #3758

Merged
akshaydeo merged 1 commit into
devfrom
05-26-fix_model_ranking_data_query
May 26, 2026
Merged

fix: exclude empty model entries from model rankings matview queries#3758
akshaydeo merged 1 commit into
devfrom
05-26-fix_model_ranking_data_query

Conversation

@impoiler

@impoiler impoiler commented May 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Filters out entries with empty model values from model rankings queries to prevent blank/unknown models from appearing in rankings results.

Changes

  • Added WHERE model != '' filter to both the current period and previous period queries in getModelRankingsFromMatView, ensuring that rows with no model identifier are excluded from aggregation and ranking calculations.

Type of change

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

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

Verify that model rankings no longer include entries with an empty model name. Query the mv_logs_hourly materialized view directly to confirm rows with model = '' exist, then check that the rankings endpoint/response omits them.

go test ./framework/logstore/...

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

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e6dacb4d-ef51-4d15-a377-6295945967d3

📥 Commits

Reviewing files that changed from the base of the PR and between 8d24199 and 1b44092.

📒 Files selected for processing (1)
  • framework/logstore/matviews.go

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved model ranking accuracy by excluding blank or empty model entries from ranking and trend calculations. This prevents empty model groups from appearing in leaderboards and from skewing comparisons between current and previous periods, yielding more reliable historical trends and cleaner results.

Walkthrough

The getModelRankingsFromMatView function now excludes empty model values from both current-period and previous-period mv_logs_hourly queries by adding model IS NOT NULL AND model != '' predicates.

Model filtering in ranking queries

Layer / File(s) Summary
Filter empty models from ranking matview queries
framework/logstore/matviews.go
Both the current-period and previous-period mv_logs_hourly matview queries add model IS NOT NULL AND model != '' predicates to exclude empty-model aggregates from model/provider rankings and trend calculations.

🎯 2 (Simple) | ⏱️ ~5 minutes

🐰 A filter here, a filter there,
Empty models vanish in the air!
Rankings now pristine and clean,
Only valid models on the screen! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: excluding empty model entries from model rankings matview queries.
Description check ✅ Passed The description covers all required sections with sufficient detail: summary explains the problem, changes detail the specific fix, type of change is marked, affected areas are identified, testing instructions are provided, and breaking changes are clarified.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 05-26-fix_model_ranking_data_query

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.

@impoiler impoiler changed the title fix: model ranking data query fix: exclude empty model entries from model rankings matview queries May 26, 2026
@impoiler impoiler self-assigned this May 26, 2026
@impoiler
impoiler changed the base branch from 05-26-chore_moved_team_creation_from_popup_to_sheet to graphite-base/3758 May 26, 2026 11:01
@impoiler
impoiler force-pushed the graphite-base/3758 branch from 200db11 to 90487f6 Compare May 26, 2026 11:01
@impoiler
impoiler force-pushed the 05-26-fix_model_ranking_data_query branch from f21e70a to 5785779 Compare May 26, 2026 11:01
@impoiler
impoiler changed the base branch from graphite-base/3758 to dev May 26, 2026 11:01
@impoiler
impoiler marked this pull request as ready for review May 26, 2026 11:02
@greptile-apps

greptile-apps Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

Safe to merge — the two-line addition is a straightforward filter with no side-effects on unrelated query paths.

The change adds a well-formed NULL-safe filter (IS NOT NULL AND != '') to both subqueries, matching the established pattern elsewhere in the codebase. The scope is narrow and the logic is easy to verify.

No files require special attention.

Important Files Changed

Filename Overview
framework/logstore/matviews.go Adds model IS NOT NULL AND model != '' filter to both current-period and previous-period queries in getModelRankingsFromMatView, correctly excluding blank and NULL model entries from rankings.

Reviews (3): Last reviewed commit: "fix: model ranking data query" | Re-trigger Greptile

coderabbitai[bot]
coderabbitai Bot previously approved these changes May 26, 2026
akshaydeo
akshaydeo previously approved these changes May 26, 2026
@impoiler
impoiler force-pushed the 05-26-fix_model_ranking_data_query branch from 5785779 to 8d24199 Compare May 26, 2026 15:58
Comment thread framework/logstore/matviews.go Outdated
Comment thread framework/logstore/matviews.go Outdated
@impoiler
impoiler dismissed stale reviews from akshaydeo and coderabbitai[bot] via 1b44092 May 26, 2026 16:27
@impoiler
impoiler force-pushed the 05-26-fix_model_ranking_data_query branch from 8d24199 to 1b44092 Compare May 26, 2026 16:27

akshaydeo commented May 26, 2026

Copy link
Copy Markdown
Contributor

Merge activity

  • May 26, 5:44 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • May 26, 5:45 PM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo
akshaydeo merged commit cc9c6dd into dev May 26, 2026
14 checks passed
@akshaydeo
akshaydeo deleted the 05-26-fix_model_ranking_data_query branch May 26, 2026 17:45
akshaydeo pushed a commit that referenced this pull request May 26, 2026
…3758)

## Summary

Filters out entries with empty `model` values from model rankings queries to prevent blank/unknown models from appearing in rankings results.

## Changes

- Added `WHERE model != ''` filter to both the current period and previous period queries in `getModelRankingsFromMatView`, ensuring that rows with no model identifier are excluded from aggregation and ranking calculations.

## 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

Verify that model rankings no longer include entries with an empty model name. Query the `mv_logs_hourly` materialized view directly to confirm rows with `model = ''` exist, then check that the rankings endpoint/response omits them.

```sh
go test ./framework/logstore/...
```

## Breaking changes

- [ ] Yes
- [x] 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
@akshaydeo akshaydeo mentioned this pull request May 26, 2026
akshaydeo added a commit that referenced this pull request May 26, 2026
## ✨ Features

- **Azure v1 API Migration** — Migrated Azure provider to the v1 API:
removed the `api-version` query parameter and the
`/openai/deployments/{model}/...` URL pattern in favor of
`/openai/v1/{operation}`; the `api_version` field has been dropped from
`AzureKeyConfig` (#3661, #3756)
- **EnvVar Support for OTEL & Prometheus Configs** — `CollectorURL`,
`MetricsEndpoint`, headers, push gateway URL, and basic auth credentials
can now be sourced from environment variables (e.g.,
`env.OTEL_COLLECTOR_URL`); added a new `ConfigMarshallerPlugin`
interface that lets plugins control storage/redaction round-trips
(#3651)
- **OTel Extra Header Forwarding** — `x-bf-eh-*` extra headers forwarded
to upstream providers are now also emitted on the request span under
`gen_ai.request.extra_header.*` for end-to-end tracing (#3730)
- **OTel Semantic Conventions** — Aligned OTel attribute keys with the
OpenTelemetry GenAI spec (canonical `gen_ai.*` and new `bifrost.*`
attributes); legacy attributes are retained in parallel to avoid
breaking existing dashboards (#3732)
- **VK Quota with Provider Configs** — `GetVirtualKeyQuotaByValue` and
the `getVirtualKeyQuota` HTTP response now include `provider_configs`
with their budgets and rate limits (#3721)
- **MCP Temp Token Non-Auth Toggle** — Added
`mcp_enable_temp_token_auth` client config flag to gate short-lived MCP
token minting for non-authenticated users (#3720)
- **Responses Stream in JSON Parser** — `jsonparser` plugin now handles
OpenAI Responses API streaming (`ResponsesStreamRequest`) in addition to
chat completions (#3749)
- **Session API Rework** — Logout now calls both the password-based
session logout and OAuth logout endpoints and resets all RTK Query cache
state (#3698)

## 🐞 Fixed

- **Streaming Latency for Observability** — Deferred root span
termination to the trace completer callback for streaming requests so
request latency is no longer inflated by header-flush time (#3762)
- **Stream Cancellation Race** — Set `BifrostContextKeyConnectionClosed`
before closing the stream and short-circuit `idleTimeoutReader.Read`
when the connection is already closed to avoid panics and hangs on
cancellation (#3733)
- **Bedrock Cache Points** — Strip cache points from Bedrock requests
for models that do not support prompt caching (e.g., GLM, Llama) to
avoid Converse API errors (#3754)
- **Bedrock Empty Text Blocks** — Skip empty/nil text blocks during
Bedrock response conversion to avoid invalid messages (#3747)
- **Bedrock Reasoning + Tools** — Preserve reasoning content blocks on
assistant turns that also contain tool calls in the Bedrock chat
converter (#3690)
- **Bedrock Search Content & Video** — Restored search content and video
parts that were being dropped from Bedrock-native passthrough requests
(#3729)
- **Structured Output Stop Reason** — Fixed an incorrect `tool_calls`
finish reason when structured output is combined with extended-thinking
tools (#3685)
- **Gemini Tool Schema Passthrough** — Forward full tool parameter
schemas via `parametersJsonSchema` instead of the lossy `parameters`
form; corrected tool response role to `user`; resolved structured output
+ tools conflict (#3761)
- **Anthropic Stop Reason & Tool Versions** — Normalized stop reason
mapping (`end_turn` to `stop`, `tool_use` to `tool_calls`, `max_tokens`
to `length`) and upgraded `text_editor_20250124`/`str_replace_editor` to
`text_editor_20250728` for computer-use tools (#3761)
- **Azure Endpoint Redaction** — Fixed a panic when
`AzureKeyConfig.Endpoint` is a literal value rather than an env
reference (#3761)
- **Auth Middleware Path Match** — Match temp-token auth middleware
whitelist against the request path only, not the full URI with query
parameters (#3737)
- **Governance Blocked Models UI** — Restored the missing Blocked Models
create/edit UI in the VK provider config sheet (#3750)
- **Logging Plugin Cleanup Drain** — Fixed a shutdown race where
`batchWriter` could drop in-flight log entries; `Cleanup` now drains
both the recovered batch and remaining queue within a 30-second budget
(#3717)
- **Model Rankings Empty Entries** — Excluded entries with empty `model`
values from model rankings matview queries so blank rows no longer
surface in the UI (#3758)
- **User Filter Duplicates** — Recreated `mv_filter_users` matview to
require non-empty `user_name`, eliminating duplicate filter dropdown
entries (#3764)
- **User Filter Display Name** — Use `user_name` instead of `user_id` as
the display label for users in logging filters (#3691)
- **Large Numeric ID Precision** — Preserve large numeric IDs in URL
search params by skipping JSON parsing for plain strings (#3692)

## 🔧 Refactors & Chores

- **Error Propagation for GetAvailable\* APIs** — `GetAvailable*`
methods on `LoggerPlugin`/`LogManager` now return wrapped errors instead
of silently logging and returning empty slices (#3759)
- **Governance Blocklist Matching** — Use `slices.Contains` for VK
blocked-model matching for clearer code with identical semantics (#3727)
- **Exported `ResolvePeriod`** — Renamed `resolvePeriod` to
`ResolvePeriod` so external packages can reuse the period parsing
(#3763)

## 📚 Docs

- **OTEL Env Var Documentation** — Documented `env.VAR_NAME` support for
`collector_url`, `metrics_endpoint`, and headers in OTEL/Prometheus
plugin docs
- **OTEL OSS Features & Examples** — Added OTEL documentation to the OSS
features list with usage examples (#3731)
- **Anthropic Auth Recommendation** — Recommend `ANTHROPIC_AUTH_TOKEN`
over `ANTHROPIC_CUSTOM_HEADERS` for Claude Code authentication (#3686)
@akshaydeo akshaydeo mentioned this pull request May 27, 2026
18 tasks
akshaydeo added a commit that referenced this pull request May 27, 2026
## Summary

This PR releases Bifrost OSS `v1.5.5` and Enterprise `v1.4.4`, bumping all module pins from `v1.5.12`/`v1.3.12` to `v1.5.13`/`v1.3.13` across core, framework, and all plugins. It also hardens the Docker manifest shell scripts, expands CI egress allowlists, and updates documentation to reflect the new SCIM-based user provisioning feature.

## Changes

- **Module version bumps**: All `go.mod`/`go.sum` files updated from `core v1.5.12` → `v1.5.13`, `framework v1.3.12` → `v1.3.13`, and all plugin versions incremented accordingly (`compat`, `governance`, `jsonparser`, `logging`, `maxim`, `mocker`, `otel`, `prompts`, `semanticcache`, `telemetry`).
- **Docker manifest scripts**: Added `#!/usr/bin/env bash` shebang and `set -euo pipefail` to `create-docker-manifest.sh` and `create-docker-manifest-ubi9.sh`; quoted all variable expansions and switched `jq -r` to `jq -er` to fail on null digests.
- **CI egress allowlist**: Added `production.cloudfront.docker.com:443` to Docker-related job allowlists, and added `_https._tcp.dl.google.com:443` and `motd.ubuntu.com:443` to the Ubuntu package job allowlist.
- **Changelog files**: Cleared per-module `changelog.md` files (content moved into the new versioned docs). Added `docs/changelogs/v1.5.5.mdx` and `docs/changelogs/ent-v1.4.4.mdx` with full release notes, and registered both in `docs/docs.json`.
- **Documentation**: Replaced the SSO Integration link with a User Provisioning (SCIM) link in both `README.md` and `transports/README.md`.
- **Enterprise v1.4.4 highlights** (documented): Kafka and Google Cloud Pub/Sub observability sinks, chunked streaming with a 100 MB inter-node message ceiling, BigQuery custom labels via env vars using the new `ConfigMarshallerPlugin` interface, temporary access token expiry extensions, and a multi-node cluster integration harness.
- **OSS v1.5.5 highlights** (documented): Azure v1 API migration, env-var support for OTel/Prometheus configs, OTel extra-header forwarding and semantic-convention alignment, virtual key quota including provider configs, Responses API streaming in `jsonparser`, and a batch of Bedrock, Gemini, Anthropic, Azure, and logging plugin fixes.

## Type of change

- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [x] Documentation
- [x] Chore/CI

## Affected areas

- [x] Core (Go)
- [x] Transports (HTTP)
- [x] Providers/Integrations
- [x] Plugins
- [ ] UI (React)
- [x] Docs

## How to test

```sh
# Core/Transports
go version
go test ./...

# Verify Docker manifest scripts exit on error
bash -n .github/workflows/scripts/create-docker-manifest.sh
bash -n .github/workflows/scripts/create-docker-manifest-ubi9.sh
```

Validate that the new changelog pages (`changelogs/v1.5.5` and `changelogs/ent-v1.4.4`) render correctly in the docs site.

## Screenshots/Recordings

N/A

## Breaking changes

- [x] Yes
- [ ] No

The Azure provider no longer accepts `api_version` in `AzureKeyConfig` and has migrated to the `/openai/v1/{operation}` URL pattern. See the [v1.4.0 Migration Guide](https://docs.getbifrost.ai/enterprise/migration-guides/v1.4.0) for full details.

## Related issues

#3661, #3756, #3651, #3730, #3732, #3754, #3747, #3690, #3729, #3685, #3733, #3761, #3735, #3721, #3720, #3749, #3698, #3762, #3750, #3727, #3717, #3759, #3758, #3764, #3691, #3692, #3737, #3763

## Security considerations

- The `ConfigMarshallerPlugin` interface redacts secrets (OTel collector URLs, Prometheus push gateway credentials, BigQuery labels) at config storage time and rehydrates them at load time, preventing plaintext secret persistence.
- Docker manifest scripts now use `set -euo pipefail`, preventing silent failures that could result in malformed or missing image manifests being pushed.

## Checklist

- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [x] I verified the CI pipeline passes locally if applicable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants