-
Notifications
You must be signed in to change notification settings - Fork 2k
Token Counter Widget #4995
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Token Counter Widget #4995
Conversation
@emerzon is attempting to deploy a commit to the Danswer Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
Implements a token counter widget with complete backend-to-frontend infrastructure for tracking LLM token usage in chat sessions.
- Added
.orig
and.rej
files inbackend/onyx/file_processing/
appear to be merge artifacts that should be removed - The
token_usage_tracker.py
implementation using thread-local storage could have thread safety issues, particularly around the 60-second cleanup mechanism - Document processing library imports (docx, openpyxl, pptx) were removed from
extract_file_text.py
while their functions are still referenced - Duplicated token limit calculation logic exists between
ChatPage.tsx
andChatInputBar.tsx
that should be centralized - New database JSONB column
token_usage
properly tracks structured usage data (prompt_tokens, completion_tokens, total_tokens) for billing accuracy
17 files reviewed, 19 comments
Edit PR Review Bot Settings | Greptile
}; | ||
|
||
// Calculate percentage based on context tokens (context window usage) | ||
const maxTokens = currentModel?.maxTokens || 200000; // Default fallback |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: 200000 should be defined as a named constant at the module level for better maintainability
refined_answer_improvement: bool | None = None | ||
is_agentic: bool | None = None | ||
error: str | None = None | ||
token_usage: dict[str, Any] | None = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: The token_usage field should have a more specific type rather than dict[str, Any]. Consider creating a TokenUsage Pydantic model to enforce the expected structure (prompt_tokens, completion_tokens, total_tokens).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Token usage information from LLM API responses | ||
token_usage: Mapped[dict[str, Any] | None] = mapped_column(postgresql.JSONB(), nullable=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Add field description in the comment to explain what token_usage metrics (prompt_tokens, completion_tokens, total_tokens) can be expected in this field
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably better to not define this. Several types of tokens could show up in the response in the future (audio tokens, image tokens, etc)
backend/alembic/versions/42e26b80_add_token_usage_to_chat_message.py
Outdated
Show resolved
Hide resolved
Revision ID: 42e26b80 | ||
Revises: 58c50ef19f08 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Migration ID should be longer (typical format is 32 chars). Current format differs from other migrations like 58c50ef19f08
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will need review at merge time I believe
…age.py Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
This PR is stale because it has been open 75 days with no activity. Remove stale label or comment or this will be closed in 15 days. |
Description
This change adds a Token counter widget near the Chat Inputbox
Data Flow
Key Features
How Has This Been Tested?
Start a new chat session, after the LLM response, the widget should appear.