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
45 changes: 44 additions & 1 deletion packages/agent/src/adapters/claude/claude-agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ import { resolveTaskId } from "../session-meta";
import {
buildBreakdown,
emptyBaseline,
estimateMcpTokens,
estimateRulesTokens,
estimateSkillsTokens,
estimateSystemPrompt,
} from "./context-breakdown";
import { promptToClaude } from "./conversion/acp-to-sdk";
Expand All @@ -84,6 +87,7 @@ import type { EnrichedReadCache } from "./hooks";
import { createLocalToolsMcpServer } from "./mcp/local-tools";
import {
fetchMcpToolMetadata,
getCachedMcpTools,
getConnectedMcpServerNames,
setMcpToolApprovalStates,
} from "./mcp/tool-metadata";
Expand Down Expand Up @@ -123,6 +127,23 @@ const SESSION_VALIDATION_TIMEOUT_MS = 30_000;
const MAX_TITLE_LENGTH = 256;
const LOCAL_ONLY_COMMANDS = new Set(["/context", "/heapdump", "/extra-usage"]);

/** Read CLAUDE.md from the project root so the context breakdown can size the
* Rules category. Best-effort: silent on a missing file, logs otherwise so
* permission errors aren't lost. */
function readClaudeMdQuietly(cwd: string, logger: Logger): string | undefined {
try {
return fs.readFileSync(path.join(cwd, "CLAUDE.md"), "utf-8");
} catch (err) {
if ((err as NodeJS.ErrnoException)?.code !== "ENOENT") {
logger.warn("Failed to read CLAUDE.md for context breakdown", {
cwd,
error: err instanceof Error ? err.message : String(err),
});
}
return undefined;
}
}

function sanitizeTitle(text: string): string {
const sanitized = text
.replace(/[\r\n]+/g, " ")
Expand Down Expand Up @@ -1241,6 +1262,7 @@ export class ClaudeAcpAgent extends BaseAcpAgent {
contextBreakdownBaseline: {
...emptyBaseline(),
systemPrompt: estimateSystemPrompt(systemPrompt),
rules: estimateRulesTokens(readClaudeMdQuietly(cwd, this.logger)),
},

// Custom properties
Expand Down Expand Up @@ -1568,13 +1590,30 @@ export class ClaudeAcpAgent extends BaseAcpAgent {

private async sendAvailableCommandsUpdate(): Promise<void> {
const commands = await this.session.query.supportedCommands();
const available = getAvailableSlashCommands(commands);
await this.client.sessionUpdate({
sessionId: this.sessionId,
update: {
sessionUpdate: "available_commands_update",
availableCommands: getAvailableSlashCommands(commands),
availableCommands: available,
},
});
this.updateBreakdownCategory("skills", estimateSkillsTokens(available));
}

/** Update one category of the context-breakdown baseline so the next
* `_posthog/usage_update` carries fresher numbers. No-op when the baseline
* hasn't been initialized yet (e.g. in a unit-test session). */
private updateBreakdownCategory(
key: keyof NonNullable<Session["contextBreakdownBaseline"]>,
tokens: number,
): void {
if (!this.session?.contextBreakdownBaseline) return;
if (this.session.contextBreakdownBaseline[key] === tokens) return;
this.session.contextBreakdownBaseline = {
...this.session.contextBreakdownBaseline,
[key]: tokens,
};
}

private async replaySessionHistory(sessionId: string): Promise<void> {
Expand Down Expand Up @@ -1631,6 +1670,10 @@ export class ClaudeAcpAgent extends BaseAcpAgent {
this.sendAvailableCommandsUpdate(),
),
fetchMcpToolMetadata(q, this.logger).then(() => {
this.updateBreakdownCategory(
"mcp",
estimateMcpTokens(getCachedMcpTools()),
);
const serverNames = getConnectedMcpServerNames();
if (serverNames.length > 0) {
this.options?.onMcpServersReady?.(serverNames);
Expand Down
46 changes: 46 additions & 0 deletions packages/agent/src/adapters/claude/context-breakdown.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ import {
buildBreakdown,
emptyBaseline,
estimateJsonTokens,
estimateMcpTokens,
estimateRulesTokens,
estimateSkillsTokens,
estimateSystemPrompt,
estimateTokens,
} from "./context-breakdown";
Expand Down Expand Up @@ -57,6 +60,49 @@ describe("estimateSystemPrompt", () => {
});
});

describe("estimateSkillsTokens", () => {
it("is 0 for an empty command list", () => {
expect(estimateSkillsTokens([])).toBe(0);
});

it("counts the JSON of name/description/hint", () => {
// [{"name":"review","description":"Review a PR","hint":"[pr]"}] ~ 55 chars
const result = estimateSkillsTokens([
{ name: "review", description: "Review a PR", input: { hint: "[pr]" } },
]);
expect(result).toBeGreaterThan(10);
expect(result).toBeLessThan(20);
});
});

describe("estimateMcpTokens", () => {
it("is 0 for no connected tools", () => {
expect(estimateMcpTokens([])).toBe(0);
});

it("scales with tool count", () => {
const one = estimateMcpTokens([{ name: "get_user", description: "x" }]);
const many = estimateMcpTokens(
Array.from({ length: 50 }, (_, i) => ({
name: `tool_${i}`,
description: "x",
})),
);
expect(many).toBeGreaterThan(one * 10);
});
});

describe("estimateRulesTokens", () => {
it("is 0 for missing rules", () => {
expect(estimateRulesTokens(undefined)).toBe(0);
expect(estimateRulesTokens("")).toBe(0);
});

it("counts the rules content", () => {
expect(estimateRulesTokens("a".repeat(350))).toBe(100);
});
});
Comment on lines +95 to +104
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 "is 0 for missing rules" test verifies two distinct inputs (undefined and "") inside a single it block without any labelling of which input failed. A parameterised test expresses this more clearly and aligns with the team's convention of always preferring it.each.

Suggested change
describe("estimateRulesTokens", () => {
it("is 0 for missing rules", () => {
expect(estimateRulesTokens(undefined)).toBe(0);
expect(estimateRulesTokens("")).toBe(0);
});
it("counts the rules content", () => {
expect(estimateRulesTokens("a".repeat(350))).toBe(100);
});
});
describe("estimateRulesTokens", () => {
it.each([undefined, ""])("is 0 for missing rules (%s)", (input) => {
expect(estimateRulesTokens(input)).toBe(0);
});
it("counts the rules content", () => {
expect(estimateRulesTokens("a".repeat(350))).toBe(100);
});
});

Context Used: Do not attempt to comment on incorrect alphabetica... (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/agent/src/adapters/claude/context-breakdown.test.ts
Line: 95-104

Comment:
The "is 0 for missing rules" test verifies two distinct inputs (`undefined` and `""`) inside a single `it` block without any labelling of which input failed. A parameterised test expresses this more clearly and aligns with the team's convention of always preferring `it.each`.

```suggestion
describe("estimateRulesTokens", () => {
  it.each([undefined, ""])("is 0 for missing rules (%s)", (input) => {
    expect(estimateRulesTokens(input)).toBe(0);
  });

  it("counts the rules content", () => {
    expect(estimateRulesTokens("a".repeat(350))).toBe(100);
  });
});
```

**Context Used:** Do not attempt to comment on incorrect alphabetica... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))

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!


describe("buildBreakdown", () => {
it("derives conversation from input - stable sum", () => {
const baseline = {
Expand Down
38 changes: 38 additions & 0 deletions packages/agent/src/adapters/claude/context-breakdown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,44 @@ export function estimateJsonTokens(value: unknown): number {
}
}

interface SlashCommandLike {
name?: string;
description?: string;
input?: { hint?: string } | null;
}

/** Tokens for the slash-command list the SDK injects into the system prompt. */
export function estimateSkillsTokens(commands: SlashCommandLike[]): number {
if (!commands.length) return 0;
return estimateJsonTokens(
commands.map((c) => ({
name: c.name,
description: c.description,
hint: c.input?.hint,
})),
);
}

interface McpToolLike {
name?: string;
description?: string;
}

/** Tokens for the connected MCP tools' name + description. The SDK doesn't
* inject their full input schemas into the prompt by default (it relies on
* tool search), so this is a conservative estimate of what's resident. */
export function estimateMcpTokens(tools: McpToolLike[]): number {
if (!tools.length) return 0;
return estimateJsonTokens(
tools.map((t) => ({ name: t.name, description: t.description })),
);
}

/** Tokens for the rules content appended to the system prompt (CLAUDE.md). */
export function estimateRulesTokens(rules: string | undefined): number {
return estimateTokens(rules);
}

export interface ContextBreakdownBaseline {
systemPrompt: number;
tools: number;
Expand Down
6 changes: 6 additions & 0 deletions packages/agent/src/adapters/claude/mcp/tool-metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,12 @@ export function getConnectedMcpServerNames(): string[] {
return [...names];
}

/** Snapshot of every tool currently in the metadata cache. Used by the
* context-breakdown estimator to size the MCP category. */
export function getCachedMcpTools(): McpToolMetadata[] {
return [...mcpToolMetadataCache.values()];
}

export function getMcpToolApprovalState(
toolName: string,
): McpToolApprovalState | undefined {
Expand Down
Loading