Skip to content

fix: match model filter on canonical_model_name and restore routing info for cost recalc - #5029

Merged
Pratham-Mishra04 merged 1 commit into
devfrom
07-08-feat_consider_canonical_model_name_as_fallback_into_filters_and_recalculation
Jul 9, 2026
Merged

fix: match model filter on canonical_model_name and restore routing info for cost recalc#5029
Pratham-Mishra04 merged 1 commit into
devfrom
07-08-feat_consider_canonical_model_name_as_fallback_into_filters_and_recalculation

Conversation

@impoiler

@impoiler impoiler commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Model filtering in the log store only matched against the wire model column, so filtering by a canonical model name (e.g. gpt-4o-mini) would miss requests routed through an alias whose wire model differed. Similarly, cost recalculation for stored logs dropped the canonical model name, causing pricing lookups to fail and return null costs for those entries.

Changes

  • Updated applyFilters in rdb.go to match against both model and canonical_model_name when filtering by model, so alias-routed requests surface correctly under their canonical name.
  • In calculateCostForLog, the RoutingInfo field is now populated with provider and model from the log entry. When a canonical_model_name is present, a ResolvedKeyAlias is reconstructed so that cost recalculation has the same routing context that live logging used, preventing null costs.

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

  1. Create a key alias that maps to a model with a different wire model name (e.g. an alias for gpt-4o-mini that routes to a provider-specific wire model).
  2. Send a request through that alias and confirm the log entry is stored with both model and canonical_model_name populated.
  3. Filter logs by the canonical model name and verify the aliased request appears in results.
  4. Trigger a cost recalculation for that log entry and confirm a non-null cost is returned.
go test ./framework/logstore/...
go test ./plugins/logging/...

Screenshots/Recordings

N/A

Breaking changes

  • Yes
  • No

Related issues

N/A

Security considerations

No security implications. Changes are scoped to log filtering and cost recalculation logic.

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 Jul 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 43 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c23d21ed-1fbe-4103-803b-e60fac25775c

📥 Commits

Reviewing files that changed from the base of the PR and between ca6e660 and 838e2d5.

📒 Files selected for processing (3)
  • framework/logstore/migrations.go
  • framework/logstore/rdb.go
  • plugins/logging/operations.go
📝 Walkthrough

Walkthrough

Log storage now indexes canonical model names, log filtering matches either wire model or canonical model name, and cost recalculation now rebuilds routing info from stored canonical model data.

Changes

Canonical Model Name Handling

Layer / File(s) Summary
Log storage index for canonical model
framework/logstore/migrations.go
performanceIndexes adds idx_logs_canonical_model_name on logs(canonical_model_name) with a non-null predicate.
Log filter includes canonical model name
framework/logstore/rdb.go
applyFilters extends the Models predicate to match either model or canonical_model_name.
Cost calculation reconstructs routing info
plugins/logging/operations.go
calculateCostForLog populates RoutingInfo with provider and requested model, and reconstructs ResolvedKeyAlias from stored CanonicalModelName.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: akshaydeo, danpiths, roroghost17

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title is concise and accurately captures the main changes to model filtering and cost recalculation.
Description check ✅ Passed The description follows the template and includes summary, changes, type, affected areas, testing, breaking changes, security, and checklist sections.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 07-08-feat_consider_canonical_model_name_as_fallback_into_filters_and_recalculation

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

@impoiler impoiler self-assigned this Jul 8, 2026
@impoiler
impoiler force-pushed the 07-08-feat_consider_canonical_model_name_as_fallback_into_filters_and_recalculation branch from e466e95 to a2be949 Compare July 8, 2026 10:02
@impoiler
impoiler force-pushed the 07-06-feat_logs_cost_recalc_ui branch from d7cd79a to 4077e3a Compare July 8, 2026 10:02
@impoiler impoiler changed the title feat: consider canonical_model_name as fallback into filters and recalculation fix: match model filter on canonical_model_name and restore routing info for cost recalc Jul 8, 2026
@impoiler
impoiler marked this pull request as ready for review July 8, 2026 14:10
@greptile-apps

greptile-apps Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

Safe to merge — both bug fixes are logically correct, the new index is non-blocking (CONCURRENTLY + partial), and the RoutingInfo reconstruction preserves the same candidate order resolvePricing used at live-log time.

The OR predicate in applyFilters is correctly grouped and backed by an appropriate partial index. The ResolvedKeyAlias reconstruction in calculateCostForLog accurately mirrors the live-logging path for entries that have a canonical model name. No data correctness, concurrency, or security issues were found in the changed code.

plugins/logging/operations.go — the CanonicalModelName reconstruction path has no dedicated test, so a future change to resolvePricing candidate ordering could silently regress the fix.

Important Files Changed

Filename Overview
framework/logstore/rdb.go Adds OR predicate (model IN ? OR canonical_model_name IN ?) so model filters match alias-routed requests by canonical name; parens preserve correct GORM AND grouping.
framework/logstore/migrations.go Appends idx_logs_canonical_model_name as a concurrent partial index (WHERE canonical_model_name IS NOT NULL), enabling BitmapOr plans alongside idx_logs_model without blocking writes.
plugins/logging/operations.go Populates RoutingInfo and conditionally reconstructs ResolvedKeyAlias (with ModelID = wire model and ModelName = canonical name) for cost recalculation; fixes null costs for canonical-name alias entries but no new tests cover this path.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[SearchLogs / applyFilters] -->|filters.Models set| B{"model IN filters.Models?"}
    B -->|yes| C[Row matches]
    B -->|no| D{"canonical_model_name IN filters.Models?"}
    D -->|yes| C
    D -->|no| E[Row excluded]

    F[calculateCostForLog] --> G[Build RoutingInfo\nProvider + Model]
    G --> H{"CanonicalModelName != nil?"}
    H -->|yes| I[Reconstruct ResolvedKeyAlias\nModelID=wire_model\nModelName=canonical]
    H -->|no| J[ResolvedKeyAlias stays nil]
    I --> K[resolvePricing candidates:\n1. canonical name\n2. wire model\n3. alias key]
    J --> L[resolvePricing candidates:\n1. alias key only]
    K --> M[Cost resolved]
    L --> N[May return nil cost\nif alias key not in catalog]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[SearchLogs / applyFilters] -->|filters.Models set| B{"model IN filters.Models?"}
    B -->|yes| C[Row matches]
    B -->|no| D{"canonical_model_name IN filters.Models?"}
    D -->|yes| C
    D -->|no| E[Row excluded]

    F[calculateCostForLog] --> G[Build RoutingInfo\nProvider + Model]
    G --> H{"CanonicalModelName != nil?"}
    H -->|yes| I[Reconstruct ResolvedKeyAlias\nModelID=wire_model\nModelName=canonical]
    H -->|no| J[ResolvedKeyAlias stays nil]
    I --> K[resolvePricing candidates:\n1. canonical name\n2. wire model\n3. alias key]
    J --> L[resolvePricing candidates:\n1. alias key only]
    K --> M[Cost resolved]
    L --> N[May return nil cost\nif alias key not in catalog]
Loading

Reviews (12): Last reviewed commit: "feat: consider canonical_model_name as f..." | Re-trigger Greptile

Comment thread framework/logstore/rdb.go
@impoiler
impoiler force-pushed the 07-06-feat_logs_cost_recalc_ui branch from 4077e3a to 8429b4f Compare July 9, 2026 01:38
@impoiler
impoiler force-pushed the 07-08-feat_consider_canonical_model_name_as_fallback_into_filters_and_recalculation branch from a2be949 to d19f0cc Compare July 9, 2026 01:38
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 9, 2026
Comment thread plugins/logging/operations.go
@impoiler
impoiler force-pushed the 07-08-feat_consider_canonical_model_name_as_fallback_into_filters_and_recalculation branch from d19f0cc to d8627f7 Compare July 9, 2026 03:03
@impoiler
impoiler force-pushed the 07-06-feat_logs_cost_recalc_ui branch from 8429b4f to 7d2975f Compare July 9, 2026 03:06
@impoiler
impoiler force-pushed the 07-08-feat_consider_canonical_model_name_as_fallback_into_filters_and_recalculation branch from d8627f7 to 964cf81 Compare July 9, 2026 03:06
@impoiler
impoiler force-pushed the 07-06-feat_logs_cost_recalc_ui branch from 7d2975f to 5babf22 Compare July 9, 2026 03:58
@impoiler
impoiler force-pushed the 07-08-feat_consider_canonical_model_name_as_fallback_into_filters_and_recalculation branch 2 times, most recently from becea36 to 0475258 Compare July 9, 2026 04:07
@impoiler
impoiler force-pushed the 07-06-feat_logs_cost_recalc_ui branch from 5babf22 to a9c404a Compare July 9, 2026 04:07
@impoiler
impoiler force-pushed the 07-08-feat_consider_canonical_model_name_as_fallback_into_filters_and_recalculation branch from 0475258 to fab2b0f Compare July 9, 2026 04:23
@impoiler
impoiler force-pushed the 07-06-feat_logs_cost_recalc_ui branch from a9c404a to 53af499 Compare July 9, 2026 04:23
@impoiler
impoiler force-pushed the 07-08-feat_consider_canonical_model_name_as_fallback_into_filters_and_recalculation branch from fab2b0f to a09364b Compare July 9, 2026 04:50
@impoiler
impoiler force-pushed the 07-06-feat_logs_cost_recalc_ui branch 2 times, most recently from 6247490 to 98e1996 Compare July 9, 2026 10:58
@impoiler
impoiler force-pushed the 07-08-feat_consider_canonical_model_name_as_fallback_into_filters_and_recalculation branch from a09364b to 62560d6 Compare July 9, 2026 10:58
@impoiler
impoiler force-pushed the 07-06-feat_logs_cost_recalc_ui branch from 98e1996 to 3a04068 Compare July 9, 2026 11:36
@impoiler
impoiler force-pushed the 07-08-feat_consider_canonical_model_name_as_fallback_into_filters_and_recalculation branch 2 times, most recently from 9491a4b to ca6e660 Compare July 9, 2026 12:09
@impoiler
impoiler force-pushed the 07-06-feat_logs_cost_recalc_ui branch from 3a04068 to 439b154 Compare July 9, 2026 12:09

Pratham-Mishra04 commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Merge activity

  • Jul 9, 12:16 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jul 9, 12:31 PM UTC: Graphite rebased this pull request as part of a merge.
  • Jul 9, 12:32 PM UTC: @Pratham-Mishra04 merged this pull request with Graphite.

@Pratham-Mishra04
Pratham-Mishra04 changed the base branch from 07-06-feat_logs_cost_recalc_ui to graphite-base/5029 July 9, 2026 12:27
@Pratham-Mishra04
Pratham-Mishra04 changed the base branch from graphite-base/5029 to dev July 9, 2026 12:30
@Pratham-Mishra04
Pratham-Mishra04 dismissed coderabbitai[bot]’s stale review July 9, 2026 12:30

The base branch was changed.

@Pratham-Mishra04
Pratham-Mishra04 force-pushed the 07-08-feat_consider_canonical_model_name_as_fallback_into_filters_and_recalculation branch from ca6e660 to 838e2d5 Compare July 9, 2026 12:30
@Pratham-Mishra04
Pratham-Mishra04 merged commit 1f662f8 into dev Jul 9, 2026
11 of 13 checks passed
@Pratham-Mishra04
Pratham-Mishra04 deleted the 07-08-feat_consider_canonical_model_name_as_fallback_into_filters_and_recalculation branch July 9, 2026 12:32
akshaydeo pushed a commit that referenced this pull request Jul 14, 2026
…nfo for cost recalc (#5029)

## Summary

Model filtering in the log store only matched against the wire model column, so filtering by a canonical model name (e.g. `gpt-4o-mini`) would miss requests routed through an alias whose wire model differed. Similarly, cost recalculation for stored logs dropped the canonical model name, causing pricing lookups to fail and return null costs for those entries.

## Changes

- Updated `applyFilters` in `rdb.go` to match against both `model` and `canonical_model_name` when filtering by model, so alias-routed requests surface correctly under their canonical name.
- In `calculateCostForLog`, the `RoutingInfo` field is now populated with provider and model from the log entry. When a `canonical_model_name` is present, a `ResolvedKeyAlias` is reconstructed so that cost recalculation has the same routing context that live logging used, preventing null costs.

## Type of change

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

## Affected areas

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

## How to test

1. Create a key alias that maps to a model with a different wire model name (e.g. an alias for `gpt-4o-mini` that routes to a provider-specific wire model).
2. Send a request through that alias and confirm the log entry is stored with both `model` and `canonical_model_name` populated.
3. Filter logs by the canonical model name and verify the aliased request appears in results.
4. Trigger a cost recalculation for that log entry and confirm a non-null cost is returned.

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

## Screenshots/Recordings

N/A

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

N/A

## Security considerations

No security implications. Changes are scoped to log filtering and cost recalculation logic.

## 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
akhsaul pushed a commit to akhsaul/bifrost that referenced this pull request Aug 27, 2026
…nfo for cost recalc (maximhq#5029)

## Summary

Model filtering in the log store only matched against the wire model column, so filtering by a canonical model name (e.g. `gpt-4o-mini`) would miss requests routed through an alias whose wire model differed. Similarly, cost recalculation for stored logs dropped the canonical model name, causing pricing lookups to fail and return null costs for those entries.

## Changes

- Updated `applyFilters` in `rdb.go` to match against both `model` and `canonical_model_name` when filtering by model, so alias-routed requests surface correctly under their canonical name.
- In `calculateCostForLog`, the `RoutingInfo` field is now populated with provider and model from the log entry. When a `canonical_model_name` is present, a `ResolvedKeyAlias` is reconstructed so that cost recalculation has the same routing context that live logging used, preventing null costs.

## Type of change

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

## Affected areas

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

## How to test

1. Create a key alias that maps to a model with a different wire model name (e.g. an alias for `gpt-4o-mini` that routes to a provider-specific wire model).
2. Send a request through that alias and confirm the log entry is stored with both `model` and `canonical_model_name` populated.
3. Filter logs by the canonical model name and verify the aliased request appears in results.
4. Trigger a cost recalculation for that log entry and confirm a non-null cost is returned.

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

## Screenshots/Recordings

N/A

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

N/A

## Security considerations

No security implications. Changes are scoped to log filtering and cost recalculation logic.

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