Skip to content

Conversation

@upsaurav12
Copy link

Summary

fixes #1248
PR fixes doesn't show error_logs under the show_missing_cost.

Changes

  • changed the logic from cost IS NULL OR cost <= 0 to Where("status != ?", "error"). Where("token_usage IS NOT NULL"). Where("cost IS NULL")

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

Describe the steps to validate this change. Include commands and expected outcomes.

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

# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build

If adding new configs or environment variables, document them here.

Screenshots/Recordings

If UI changes, add before/after screenshots or short clips.

Breaking changes

  • Yes
  • No

If yes, describe impact and migration instructions.

Related issues

Link related issues and discussions. Example: Closes #123

Security considerations

Note any security implications (auth, secrets, PII, sandboxing, etc.).

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
Copy link
Contributor

coderabbitai bot commented Jan 6, 2026

Caution

Review failed

An error occurred during the review process. Please try again later.

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved filtering for logs with missing cost information. The filter now excludes error-status logs and applies stricter criteria when identifying missing costs in log entries.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Modified the cost filter logic in applyFilters to correctly exclude error status logs when identifying logs with missing cost information. The new filter requires non-error status, non-null token_usage, and null cost, replacing the previous broader cost-only check.

Changes

Cohort / File(s) Summary
Missing Cost Filter Fix
framework/logstore/rdb.go
Updated applyFilters logic for MissingCostOnly filter: changed from cost IS NULL OR cost <= 0 to status != "error" AND token_usage IS NOT NULL AND cost IS NULL, excluding error logs from missing cost queries.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A filter refined, with care and precision,
Error logs vanish—a bug-fixing decision,
Now missing costs show the truth so clear,
Only the valid requests appear here!

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8e2abee and 3bbd127.

📒 Files selected for processing (1)
  • framework/logstore/rdb.go
🧰 Additional context used
📓 Path-based instructions (1)
**

⚙️ CodeRabbit configuration file

always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)

Files:

  • framework/logstore/rdb.go
🧠 Learnings (2)
📚 Learning: 2025-12-09T17:07:42.007Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.

Applied to files:

  • framework/logstore/rdb.go
📚 Learning: 2025-12-29T11:54:55.836Z
Learnt from: akshaydeo
Repo: maximhq/bifrost PR: 1153
File: framework/configstore/rdb.go:2221-2246
Timestamp: 2025-12-29T11:54:55.836Z
Learning: In Go reviews, do not flag range-over-int patterns like for i := range n as compile-time errors, assuming Go 1.22+ semantics. Only flag actual range-capable values (slices, arrays, maps, channels, strings) and other compile-time issues. This applies to all Go files across the repository.

Applied to files:

  • framework/logstore/rdb.go
🔇 Additional comments (1)
framework/logstore/rdb.go (1)

65-70: Unable to verify the factual claims made in this review comment.

The review asserts that the old logic contained cost IS NULL OR cost <= 0 and claims the changes removed the OR cost <= 0 portion while adding the token_usage IS NOT NULL requirement. However, I cannot confirm these assertions because:

  • The repository contains only a single commit (3bbd127), making git history unavailable for comparison
  • No test files document the prior or current filter behavior
  • Issue [Bug]: Error logs are counted in missing cost logs UI #1248 referenced in the review is not documented in the codebase
  • The commit message states only "fix: don't show error_logs under show_missing_cost," which aligns with the status != "error" condition visible in the current code

To properly assess whether the token_usage IS NOT NULL and cost IS NULL (without <= 0) changes are intentional and undocumented, or standard design choices, verification with the development team or access to prior commit history would be required.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@upsaurav12
Copy link
Author

@akshaydeo, I have raised the PR, I am not sure if it is the correct solution because i am not sure what exactly the show_missing_cost, it will some time to understand the working.

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.

[Bug]: Error logs are counted in missing cost logs UI Files API Support

1 participant