Skip to content

fix: rebuild token_usage from denormalized columns in hybrid log list - #4764

Merged
akshaydeo merged 4 commits into
mainfrom
fix/rebuild-token-usage-hybrid-log-list
Jun 29, 2026
Merged

fix: rebuild token_usage from denormalized columns in hybrid log list#4764
akshaydeo merged 4 commits into
mainfrom
fix/rebuild-token-usage-hybrid-log-list

Conversation

@impoiler

Copy link
Copy Markdown
Contributor

Cherry-picked from dev: 0c13539

…dle special characters in URLs (#4714)

## Summary

Array-type query parameters in the logs page (models, providers, etc.) were not using the safe URI-encoding parser, meaning values containing characters like `://` (e.g. model names such as `gpt://host/model`) could be misinterpreted as path or query delimiters by TanStack Router. This introduces `parseAsSafeArrayOf` and applies it consistently across all array filters.

## Changes

- Added `parseAsSafeArrayOf` to `queryParamsParser.ts` by composing `parseAsArrayOf` with the existing `parseAsSafeString` parser, ensuring full URI-encoding for comma-separated filter values.
- Replaced all usages of `parseAsArrayOf(parseAsString)` in the logs page with `parseAsSafeArrayOf` so that array filters (models, providers, aliases, status, etc.) benefit from the same encoding guarantees as string filters.
- Added unit tests for both `parseAsSafeString` and `parseAsSafeArrayOf` to verify round-trip correctness with model names containing `://`.

## Type of change

- [x] Bug fix

## Affected areas

- [x] UI (React)

## How to test

```sh
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```

To manually verify, navigate to the logs page and apply a filter using a model name containing `://` (e.g. `gpt://host/model`). Confirm the URL encodes correctly and the filter persists on page reload without routing errors.

## Screenshots/Recordings

N/A

## Breaking changes

- [x] No

## Related issues

#4603

## Security considerations

No security implications. This change only affects URL query parameter encoding in the UI.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
@mintlify

mintlify Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
Bifrost 🟡 Building Jun 29, 2026, 7:00 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@coderabbitai

coderabbitai Bot commented Jun 29, 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 0bbb27f3-a06b-47ac-8240-247eeb725874

📥 Commits

Reviewing files that changed from the base of the PR and between 94df2c9 and 18b1af3.

📒 Files selected for processing (4)
  • framework/changelog.md
  • framework/logstore/hybrid_test.go
  • framework/logstore/tables.go
  • framework/logstore/tables_test.go
✅ Files skipped from review due to trivial changes (1)
  • framework/changelog.md
🚧 Files skipped from review as they are similar to previous changes (3)
  • framework/logstore/tables_test.go
  • framework/logstore/tables.go
  • framework/logstore/hybrid_test.go

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Token usage is now shown more reliably in log previews and search results, even when full details are stored separately.
    • Log details continue to display the full token usage information when opening an individual record.
    • Improved handling for cases where token usage data is missing or malformed, preventing incorrect summaries.

Walkthrough

Log.DeserializeFields now rebuilds TokenUsageParsed from denormalized token columns when the serialized payload is absent. Unit tests and a hybrid integration test cover reconstruction, precedence, malformed input, zero values, list previews, and object-storage hydration.

Changes

Token usage reconstruction in hybrid log store

Layer / File(s) Summary
DeserializeFields reconstruction logic and unit tests
framework/logstore/tables.go, framework/logstore/tables_test.go
Adds conditional reconstruction of TokenUsageParsed from denormalized token columns and cached read tokens; unit tests cover reconstruction, serialized-value precedence, malformed JSON, and zero-value skip.
Hybrid integration test and changelog
framework/logstore/hybrid_test.go, framework/changelog.md
TestHybrid_TokenUsageSummaryForListPreview verifies SearchLogs rebuilds token usage from DB columns and FindByID hydrates from object storage; changelog entry added.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • maximhq/bifrost#4722: Contains the same Log.DeserializeFields token-usage reconstruction change, matching unit tests, and the TestHybrid_TokenUsageSummaryForListPreview integration test.

Suggested reviewers

  • danpiths
  • Pratham-Mishra04
  • akshaydeo

Poem

🐇 The token counts went out to play,
But came back home the shorter way.
From columns neat, they hop right in,
With list and detail checks to win.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description only notes a cherry-pick and omits the required template sections, testing, affected areas, and issue context. Fill in the template sections: Summary, Changes, Type of change, Affected areas, How to test, Breaking changes, Related issues, Security, and Checklist.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change to rebuild token_usage in the hybrid log list.
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.
✨ 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 fix/rebuild-token-usage-hybrid-log-list

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.

@greptile-apps

greptile-apps Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
framework/logstore/tables.go Rebuilds parsed token usage from denormalized DB columns for hybrid list reads.
framework/logstore/hybrid_test.go Adds coverage for token usage in hybrid list previews without object-store hydration.
framework/logstore/tables_test.go Adds unit coverage for token usage reconstruction, precedence, malformed JSON, and all-zero rows.
framework/changelog.md Adds the changelog entry for the framework token usage fix.

Reviews (4): Last reviewed commit: "fix: rebuild token_usage from denormaliz..." | Re-trigger Greptile

Comment thread framework/logstore/tables.go
Comment thread framework/logstore/tables.go
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 29, 2026
@impoiler
impoiler changed the base branch from main to graphite-base/4764 June 29, 2026 07:05
@impoiler
impoiler force-pushed the fix/rebuild-token-usage-hybrid-log-list branch from 276a075 to 94df2c9 Compare June 29, 2026 07:05
@impoiler
impoiler changed the base branch from graphite-base/4764 to feat/truncated-label-tooltip-overflow June 29, 2026 07:06
## Summary

Adds a reusable `TruncatedLabel` component that displays truncated text with an automatic tooltip when the content overflows its container.

## Changes

- Introduces `TruncatedLabel`, a `<span>`-based component that detects when its text content is truncated via CSS overflow and conditionally renders a `Tooltip` to show the full content
- Truncation detection is performed by comparing `scrollWidth` to `clientWidth`, and re-evaluated on window resize or when `children` changes
- The tooltip content defaults to the `children` value if it is a string, but accepts an explicit `tooltip` prop for custom content
- The tooltip is only rendered when the text is actually truncated, avoiding unnecessary DOM overhead when content fits

## Type of change

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

## Affected areas

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

## How to test

```sh
cd ui
pnpm i || npm i
pnpm build || npm run build
```

Render a `TruncatedLabel` inside a constrained-width container with a long string. Verify that hovering over the truncated text shows a tooltip with the full content, and that no tooltip appears when the text is not truncated.

## Screenshots/Recordings

N/A

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

N/A

## Security considerations

No security implications. The component renders user-provided React nodes, which is consistent with existing UI patterns.

## 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
Comment thread framework/logstore/tables.go Outdated
Comment thread framework/logstore/tables.go Outdated
…ation logic (#4716)

## Summary

Extracts the truncated label logic (truncate text with a tooltip on overflow) into a shared `TruncatedLabel` component and replaces all inline implementations with it.

## Changes

- Added a reusable `TruncatedLabel` component in `ui/components/ui/truncatedLabel` that handles text truncation and conditionally renders a tooltip when the content overflows
- Removed the local `TruncatedName` component from the providers page, which duplicated this logic using `useRef`, `useState`, and a resize event listener
- Replaced inline `<span className="truncate ...">` elements in the logs and MCP filter sidebars with `TruncatedLabel`

## Type of change

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

## Affected areas

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

## How to test

```sh
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```

Verify that truncated labels in the providers list and filter sidebars still show a tooltip on hover when the text overflows, and no tooltip when it does not.

## Screenshots/Recordings

Verify the providers sidebar and log/MCP filter sidebars visually behave the same as before — truncated text shows a tooltip, non-truncated text does not.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

#4604

## 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
@impoiler
impoiler changed the base branch from feat/truncated-label-tooltip-overflow to graphite-base/4764 June 29, 2026 07:08
@impoiler
impoiler force-pushed the fix/rebuild-token-usage-hybrid-log-list branch from 94df2c9 to cfaf786 Compare June 29, 2026 07:09
@impoiler
impoiler force-pushed the graphite-base/4764 branch from 76ee099 to 11d6f95 Compare June 29, 2026 07:09
@impoiler
impoiler changed the base branch from graphite-base/4764 to refactor/extract-truncated-label-replace-inline-truncation June 29, 2026 07:09
Comment thread framework/logstore/tables.go Outdated
Comment thread framework/logstore/tables.go
…#4722)

* fix: rebuild token_usage from denormalized columns in hybrid log list

* refactor: inline hybrid token usage reconstruction

* fix: preserve malformed serialized token usage state

---------

Co-authored-by: gexiangdong <xiangdong.ge@pandasofcaribbean.com>
@roroghost17
roroghost17 force-pushed the fix/rebuild-token-usage-hybrid-log-list branch from cfaf786 to 18b1af3 Compare June 29, 2026 09:09

akshaydeo commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Merge activity

  • Jun 29, 9:56 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jun 29, 10:00 AM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo
akshaydeo changed the base branch from refactor/extract-truncated-label-replace-inline-truncation to graphite-base/4764 June 29, 2026 09:59
@akshaydeo
akshaydeo changed the base branch from graphite-base/4764 to main June 29, 2026 09:59
@akshaydeo
akshaydeo dismissed coderabbitai[bot]’s stale review June 29, 2026 09:59

The base branch was changed.

@akshaydeo
akshaydeo merged commit d18b5f3 into main Jun 29, 2026
9 of 10 checks passed
@akshaydeo
akshaydeo deleted the fix/rebuild-token-usage-hybrid-log-list branch June 29, 2026 10:00
R-droid101 pushed a commit to R-droid101/bifrost that referenced this pull request Jul 1, 2026
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.

4 participants