Skip to content

Conversation

@ultmaster
Copy link
Contributor

Summary

  • show token counts per chat message and total at bottom
  • style token counters in content.tsx
  • update CSS styling for token count display

Testing

  • npm run build-webview
  • npm run build-cli
  • npm run lint
  • npm test
  • python -m pytest python/tests
  • npm run generate-component-spec

https://chatgpt.com/codex/tasks/task_e_686e406d2a44832e859df55c19a499f9

@ultmaster ultmaster requested a review from Copilot July 11, 2025 05:32
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds token count indicators for individual chat messages and a total token count at the bottom of the webview, along with related CSS updates for styling.

  • Introduce a tokens prop in ChatMessages and render per-message token counts in the header.
  • Update Content to pass tokens.perMessage to ChatMessages and display total tokens.
  • Enhance CSS with a new font-size variable and styles for .token-count and .token-total.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
packages/poml-vscode/panel/content.tsx Added tokens prop to chat components and rendered counts.
media/style.css Defined --poml-font-size-09 and added styles for token displays.
Comments suppressed due to low confidence (2)

packages/poml-vscode/panel/content.tsx:192

  • [nitpick] No tests appear to cover the new per-message token count rendering. Consider adding unit tests to verify ChatMessages correctly displays counts when tokens is provided.
                <span className="token-count">{tokens[idx]} tokens</span>

media/style.css:306

  • The CSS variable --poml-padding is not defined elsewhere; consider using an existing padding variable (e.g., --poml-padding-02) or define --poml-padding.
  margin-left: var(--poml-padding);

media/style.css Outdated
margin: 0;
}

.chat-message-header .content .name .token-count {
Copy link

Copilot AI Jul 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The selector includes .content which doesn't exist in the markup. It should be .chat-message-header .name .token-count to correctly target the token-count span.

Copilot uses AI. Check for mistakes.
<div className="hidden" id="copy-content" data-value={toCopy} />
{result}
{tokens && (
<div className="token-total">Total tokens: {tokens.total}</div>
Copy link

Copilot AI Jul 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] For better screen reader support, consider adding an aria-label or visually hidden text to this total token element so it’s announced clearly.

Suggested change
<div className="token-total">Total tokens: {tokens.total}</div>
<div className="token-total" aria-label={`Total tokens: ${tokens.total}`}>Total tokens: {tokens.total}</div>

Copilot uses AI. Check for mistakes.
@ultmaster ultmaster merged commit 9bfe934 into token-counter Jul 11, 2025
4 checks passed
@ultmaster ultmaster deleted the codex/update-token-counts-display-and-styles branch August 27, 2025 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants