Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import type { ContextUsage } from "@features/sessions/hooks/useContextUsage";
import { Theme } from "@radix-ui/themes";
import { render, screen } from "@testing-library/react";
import { describe, expect, it } from "vitest";
import { ContextBreakdownPopover } from "./ContextBreakdownPopover";

function usageWith(
breakdown: ContextUsage["breakdown"],
overrides?: Partial<ContextUsage>,
): ContextUsage {
return {
used: 74_000,
size: 200_000,
percentage: 37,
cost: null,
breakdown,
...overrides,
};
}

describe("ContextBreakdownPopover", () => {
it("renders the header with aggregate tokens", () => {
render(
<Theme>
<ContextBreakdownPopover usage={usageWith(null)} />
</Theme>,
);
expect(screen.getByText(/74K \/ 200K tokens/)).toBeInTheDocument();
expect(screen.getByText("37% full")).toBeInTheDocument();
});

it("shows the placeholder copy when breakdown is missing", () => {
render(
<Theme>
<ContextBreakdownPopover usage={usageWith(null)} />
</Theme>,
);
expect(
screen.getByText(/Detailed breakdown available after the first response/),
).toBeInTheDocument();
});
Comment on lines +22 to +41
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 The first two tests render exactly the same component (usageWith(null)) — the render call is duplicated. Merging them into a single null-breakdown test (or using a parameterised table for breakdown = null vs breakdown = {...}) removes the redundancy and aligns with the team's preference for parameterised tests.

Suggested change
it("renders the header with aggregate tokens", () => {
render(
<Theme>
<ContextBreakdownPopover usage={usageWith(null)} />
</Theme>,
);
expect(screen.getByText(/74K \/ 200K tokens/)).toBeInTheDocument();
expect(screen.getByText("37% full")).toBeInTheDocument();
});
it("shows the placeholder copy when breakdown is missing", () => {
render(
<Theme>
<ContextBreakdownPopover usage={usageWith(null)} />
</Theme>,
);
expect(
screen.getByText(/Detailed breakdown available after the first response/),
).toBeInTheDocument();
});
it("renders the header and placeholder copy when breakdown is null", () => {
render(
<Theme>
<ContextBreakdownPopover usage={usageWith(null)} />
</Theme>,
);
expect(screen.getByText(/74K \/ 200K tokens/)).toBeInTheDocument();
expect(screen.getByText("37% full")).toBeInTheDocument();
expect(
screen.getByText(/Detailed breakdown available after the first response/),
).toBeInTheDocument();
});
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/renderer/features/sessions/components/ContextBreakdownPopover.test.tsx
Line: 22-41

Comment:
The first two tests render exactly the same component (`usageWith(null)`) — the `render` call is duplicated. Merging them into a single `null`-breakdown test (or using a parameterised table for `breakdown = null` vs `breakdown = {...}`) removes the redundancy and aligns with the team's preference for parameterised tests.

```suggestion
  it("renders the header and placeholder copy when breakdown is null", () => {
    render(
      <Theme>
        <ContextBreakdownPopover usage={usageWith(null)} />
      </Theme>,
    );
    expect(screen.getByText(/74K \/ 200K tokens/)).toBeInTheDocument();
    expect(screen.getByText("37% full")).toBeInTheDocument();
    expect(
      screen.getByText(/Detailed breakdown available after the first response/),
    ).toBeInTheDocument();
  });
```

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


it("renders one row per non-zero category", () => {
render(
<Theme>
<ContextBreakdownPopover
usage={usageWith({
systemPrompt: 4000,
tools: 0,
rules: 0,
skills: 0,
mcp: 1500,
subagents: 0,
conversation: 68_500,
})}
/>
</Theme>,
);
expect(screen.getByText("System prompt")).toBeInTheDocument();
expect(screen.getByText("MCP")).toBeInTheDocument();
expect(screen.getByText("Conversation")).toBeInTheDocument();
expect(screen.queryByText("Tools")).not.toBeInTheDocument();
expect(screen.queryByText("Rules")).not.toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
import type { ContextUsage } from "@features/sessions/hooks/useContextUsage";
import {
CONTEXT_CATEGORIES,
formatTokensCompact,
getOverallUsageColor,
} from "@features/sessions/utils/contextColors";
import { Flex, Text } from "@radix-ui/themes";

interface ContextBreakdownPopoverProps {
usage: ContextUsage;
}

export function ContextBreakdownPopover({
usage,
}: ContextBreakdownPopoverProps) {
const { used, size, percentage, breakdown } = usage;
const fillColor = getOverallUsageColor(percentage);

return (
<Flex direction="column" gap="3" className="min-w-[280px]">
<Flex align="center" justify="between">
<Text className="font-medium text-(--gray-12) text-[13px]">
Context
</Text>
<Text className="text-(--gray-10) text-[12px] tabular-nums">
~{formatTokensCompact(used)} / {formatTokensCompact(size)} tokens
</Text>
</Flex>

<Text className="font-semibold text-(--gray-12) text-[15px]">
{percentage}% full
</Text>

{breakdown ? (
<SegmentedBar breakdown={breakdown} total={used} fallback={fillColor} />
) : (
<SinglePercentBar percentage={percentage} color={fillColor} />
)}

{breakdown ? (
<Flex direction="column" gap="2">
{CONTEXT_CATEGORIES.filter((c) => breakdown[c.key] > 0).map((cat) => (
<Flex
key={cat.key}
align="center"
justify="between"
className="text-[13px]"
>
<Flex align="center" gap="2">
<span
className="inline-block size-2.5 rounded-sm"
style={{ backgroundColor: cat.color }}
/>
<Text className="text-(--gray-12)">{cat.label}</Text>
</Flex>
<Text className="text-(--gray-11) tabular-nums">
{formatTokensCompact(breakdown[cat.key])}
</Text>
</Flex>
))}
</Flex>
) : (
<Text className="text-(--gray-10) text-[12px]">
Detailed breakdown available after the first response.
</Text>
)}
</Flex>
);
}

function SegmentedBar({
breakdown,
total,
fallback,
}: {
breakdown: NonNullable<ContextUsage["breakdown"]>;
total: number;
fallback: string;
}) {
if (total <= 0) {
return <div className="h-1.5 w-full rounded-full bg-(--gray-4)" />;
}
return (
<div className="flex h-1.5 w-full overflow-hidden rounded-full bg-(--gray-4)">
{CONTEXT_CATEGORIES.map((cat) => {
const value = breakdown[cat.key];
if (value <= 0) return null;
return (
<div
key={cat.key}
style={{
width: `${(value / total) * 100}%`,
backgroundColor: cat.color || fallback,
}}
/>
);
})}
</div>
);
}

function SinglePercentBar({
percentage,
color,
}: {
percentage: number;
color: string;
}) {
return (
<div className="h-1.5 w-full overflow-hidden rounded-full bg-(--gray-4)">
<div
className="h-full rounded-full"
style={{ width: `${percentage}%`, backgroundColor: color }}
/>
</div>
);
}
Original file line number Diff line number Diff line change
@@ -1,24 +1,10 @@
import { Tooltip } from "@components/ui/Tooltip";
import type { ContextUsage } from "@features/sessions/hooks/useContextUsage";
import { Flex, Text } from "@radix-ui/themes";

function formatTokensCompact(tokens: number): string {
if (tokens >= 1_000_000) {
return `${(tokens / 1_000_000).toFixed(1)}M`;
}
return `${Math.round(tokens / 1000)}K`;
}

function formatTokensFull(tokens: number): string {
return tokens.toLocaleString();
}

function getUsageColor(percentage: number): string {
if (percentage >= 90) return "var(--red-9)";
if (percentage >= 75) return "var(--orange-9)";
if (percentage >= 50) return "var(--amber-9)";
return "var(--green-9)";
}
import {
formatTokensCompact,
getOverallUsageColor,
} from "@features/sessions/utils/contextColors";
import { Flex, Popover, Text } from "@radix-ui/themes";
import { ContextBreakdownPopover } from "./ContextBreakdownPopover";

const CIRCLE_SIZE = 20;
const STROKE_WIDTH = 2.5;
Expand All @@ -34,45 +20,54 @@ export function ContextUsageIndicator({ usage }: ContextUsageIndicatorProps) {

const { used, size, percentage } = usage;
const strokeDashoffset = CIRCUMFERENCE - (percentage / 100) * CIRCUMFERENCE;
const color = getUsageColor(percentage);
const color = getOverallUsageColor(percentage);

return (
<Tooltip
content={`${formatTokensFull(used)} / ${formatTokensFull(size)} tokens (${percentage}%)`}
side="top"
>
<Flex align="center" gap="1" className="cursor-default select-none">
<svg
width={CIRCLE_SIZE}
height={CIRCLE_SIZE}
className="-rotate-90 shrink-0"
role="img"
<Popover.Root>
<Popover.Trigger>
<button
type="button"
className="flex cursor-pointer select-none items-center gap-1 bg-transparent"
aria-label={`Context usage: ${percentage}%`}
>
Comment on lines +27 to 32
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Popover.Trigger renders a <button> element by default. Wrapping another <button> inside it produces a <button> inside a <button>, which is invalid HTML — browsers strip the outer element and break the Radix focus/aria wiring, so the popover may not open reliably and screen readers will see a confusing interactive structure. Add asChild so Radix reuses the inner button as the trigger directly.

Suggested change
<Popover.Trigger>
<button
type="button"
className="flex cursor-pointer select-none items-center gap-1 bg-transparent"
aria-label={`Context usage: ${percentage}%`}
>
<Popover.Trigger asChild>
<button
type="button"
className="flex cursor-pointer select-none items-center gap-1 bg-transparent"
aria-label={`Context usage: ${percentage}%`}
>
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/renderer/features/sessions/components/ContextUsageIndicator.tsx
Line: 27-32

Comment:
`Popover.Trigger` renders a `<button>` element by default. Wrapping another `<button>` inside it produces a `<button>` inside a `<button>`, which is invalid HTML — browsers strip the outer element and break the Radix focus/aria wiring, so the popover may not open reliably and screen readers will see a confusing interactive structure. Add `asChild` so Radix reuses the inner button as the trigger directly.

```suggestion
      <Popover.Trigger asChild>
        <button
          type="button"
          className="flex cursor-pointer select-none items-center gap-1 bg-transparent"
          aria-label={`Context usage: ${percentage}%`}
        >
```

How can I resolve this? If you propose a fix, please make it concise.

<circle
cx={CIRCLE_SIZE / 2}
cy={CIRCLE_SIZE / 2}
r={RADIUS}
fill="none"
stroke="var(--gray-5)"
strokeWidth={STROKE_WIDTH}
/>
<circle
cx={CIRCLE_SIZE / 2}
cy={CIRCLE_SIZE / 2}
r={RADIUS}
fill="none"
stroke={color}
strokeWidth={STROKE_WIDTH}
strokeDasharray={CIRCUMFERENCE}
strokeDashoffset={strokeDashoffset}
strokeLinecap="round"
/>
</svg>
<Text className="text-[13px] text-gray-10 tabular-nums">
{formatTokensCompact(used)}/{formatTokensCompact(size)}
</Text>
</Flex>
</Tooltip>
<Flex align="center" gap="1">
<svg
width={CIRCLE_SIZE}
height={CIRCLE_SIZE}
className="-rotate-90 shrink-0"
role="img"
aria-hidden="true"
>
<circle
cx={CIRCLE_SIZE / 2}
cy={CIRCLE_SIZE / 2}
r={RADIUS}
fill="none"
stroke="var(--gray-5)"
strokeWidth={STROKE_WIDTH}
/>
<circle
cx={CIRCLE_SIZE / 2}
cy={CIRCLE_SIZE / 2}
r={RADIUS}
fill="none"
stroke={color}
strokeWidth={STROKE_WIDTH}
strokeDasharray={CIRCUMFERENCE}
strokeDashoffset={strokeDashoffset}
strokeLinecap="round"
/>
</svg>
<Text className="text-[13px] text-gray-10 tabular-nums">
{formatTokensCompact(used)}/{formatTokensCompact(size)} ·{" "}
{percentage}%
</Text>
</Flex>
</button>
</Popover.Trigger>
<Popover.Content size="2" side="top" align="end" sideOffset={6}>
<ContextBreakdownPopover usage={usage} />
</Popover.Content>
</Popover.Root>
);
}
33 changes: 33 additions & 0 deletions apps/code/src/renderer/features/sessions/utils/contextColors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import type { ContextBreakdown } from "@features/sessions/hooks/useContextUsage";

export interface CategoryStyle {
key: keyof ContextBreakdown;
label: string;
color: string;
}

// Ordered like the design spec: System prompt, Tools, Rules, Skills, MCP,
// Subagents, Conversation. Colors reuse Radix scales so they read in both
// light/dark modes.
export const CONTEXT_CATEGORIES: readonly CategoryStyle[] = [
{ key: "systemPrompt", label: "System prompt", color: "var(--gray-9)" },
{ key: "tools", label: "Tools", color: "var(--violet-9)" },
{ key: "rules", label: "Rules", color: "var(--green-9)" },
{ key: "skills", label: "Skills", color: "var(--amber-9)" },
{ key: "mcp", label: "MCP", color: "var(--pink-9)" },
{ key: "subagents", label: "Subagents", color: "var(--blue-9)" },
{ key: "conversation", label: "Conversation", color: "var(--orange-9)" },
] as const;

export function getOverallUsageColor(percentage: number): string {
if (percentage >= 90) return "var(--red-9)";
if (percentage >= 75) return "var(--orange-9)";
if (percentage >= 50) return "var(--amber-9)";
return "var(--green-9)";
}

export function formatTokensCompact(tokens: number): string {
if (tokens >= 1_000_000) return `${(tokens / 1_000_000).toFixed(1)}M`;
if (tokens >= 1000) return `${Math.round(tokens / 1000)}K`;
return tokens.toString();
}
Loading