Token efficiency

This commit is contained in:
Colin McDonnell
2025-12-22 18:16:55 -08:00
parent 0cf88e1752
commit 7fb788a883
6 changed files with 25 additions and 44 deletions
+2 -1
View File
@@ -57,7 +57,7 @@ export const claude = agent({
options: { options: {
...sandboxOptions, ...sandboxOptions,
mcpServers, mcpServers,
model: "claude-opus-4-5",
pathToClaudeCodeExecutable: cliPath, pathToClaudeCodeExecutable: cliPath,
env: createAgentEnv({ ANTHROPIC_API_KEY: apiKey }), env: createAgentEnv({ ANTHROPIC_API_KEY: apiKey }),
}, },
@@ -147,6 +147,7 @@ const messageHandlers: SDKMessageHandlers = {
} }
}, },
result: async (data) => { result: async (data) => {
log.debug(JSON.stringify(data, null, 2));
if (data.subtype === "success") { if (data.subtype === "success") {
await log.summaryTable([ await log.summaryTable([
[ [
+10 -19
View File
@@ -92629,7 +92629,7 @@ ${disableProgressComment ? "" : `
description: "Review code, PRs, or implementations; provide feedback or suggestions; identify issues; or check code quality, style, and correctness", description: "Review code, PRs, or implementations; provide feedback or suggestions; identify issues; or check code quality, style, and correctness",
prompt: `Follow these steps. THINK HARDER. prompt: `Follow these steps. THINK HARDER.
1. **CHECKOUT** - Use ${ghPullfrogMcpName}/checkout_pr with the PR number. This fetches the PR branch and returns the diff. Use this diff for your review - it shows exactly what's in the PR. 1. **CHECKOUT** - Use ${ghPullfrogMcpName}/checkout_pr with the PR number. This returns all PR metadata (title, base, head, fork status) AND a \`diffPath\` pointing to a file containing the formatted diff.
2. **UNDERSTAND CONTEXT** - Read the modified files to understand the changes in context. Don't just look at the diff - understand how the changes affect the overall codebase. 2. **UNDERSTAND CONTEXT** - Read the modified files to understand the changes in context. Don't just look at the diff - understand how the changes affect the overall codebase.
@@ -92639,7 +92639,7 @@ ${disableProgressComment ? "" : `
- What bugs, edge cases, or security issues exist? - What bugs, edge cases, or security issues exist?
- Could this be made more elegant? - Could this be made more elegant?
4. **DRAFT** - For each inline comment, find the line in the diff. Each code line shows: \`OLD | NEW | TYPE | CODE\`. Use the NEW line number (second column). 4. **DRAFT** - For each inline comment, find the line in the diff. Each code line shows: \`| OLD | NEW | TYPE | CODE\`. Use the NEW line number (second column).
5. **SELF-CRITIQUE** - Before submitting, review your draft: 5. **SELF-CRITIQUE** - Before submitting, review your draft:
- DO NOT NITPICK. Do not comment on minor formatting changes, changes to playground/scratch files, lack of docs/docsstrings, or small changes that seem irrelevant. Assume these things are intentional by the PR author. - DO NOT NITPICK. Do not comment on minor formatting changes, changes to playground/scratch files, lack of docs/docsstrings, or small changes that seem irrelevant. Assume these things are intentional by the PR author.
@@ -93340,7 +93340,6 @@ var claude = agent({
options: { options: {
...sandboxOptions, ...sandboxOptions,
mcpServers, mcpServers,
model: "claude-opus-4-5",
pathToClaudeCodeExecutable: cliPath, pathToClaudeCodeExecutable: cliPath,
env: createAgentEnv({ ANTHROPIC_API_KEY: apiKey }) env: createAgentEnv({ ANTHROPIC_API_KEY: apiKey })
} }
@@ -93400,6 +93399,7 @@ var messageHandlers = {
} }
}, },
result: async (data) => { result: async (data) => {
log.debug(JSON.stringify(data, null, 2));
if (data.subtype === "success") { if (data.subtype === "success") {
await log.summaryTable([ await log.summaryTable([
[ [
@@ -98288,13 +98288,9 @@ function stripExistingFooter(body) {
// mcp/shared.ts // mcp/shared.ts
var tool = (toolDef) => toolDef; var tool = (toolDef) => toolDef;
var handleToolSuccess = (data) => { var handleToolSuccess = (data) => {
const text = typeof data === "string" ? data : encode(data);
return { return {
content: [ content: [{ type: "text", text }]
{
type: "text",
text: JSON.stringify(data, null, 2)
}
]
}; };
}; };
var handleToolError = (error41) => { var handleToolError = (error41) => {
@@ -123044,16 +123040,16 @@ function formatFilesWithLineNumbers(files) {
const changeType = line[0] || " "; const changeType = line[0] || " ";
const code = line.slice(1); const code = line.slice(1);
if (changeType === "-") { if (changeType === "-") {
output.push(`${padNum(oldLine)} | | - | ${code}`); output.push(`| ${padNum(oldLine)} | | - | ${code}`);
oldLine++; oldLine++;
} else if (changeType === "+") { } else if (changeType === "+") {
output.push(` | ${padNum(newLine)} | + | ${code}`); output.push(`| | ${padNum(newLine)} | + | ${code}`);
newLine++; newLine++;
} else if (changeType === " " || changeType === "\\") { } else if (changeType === " " || changeType === "\\") {
if (changeType === "\\") { if (changeType === "\\") {
output.push(line); output.push(line);
} else { } else {
output.push(`${padNum(oldLine)} | ${padNum(newLine)} | | ${code}`); output.push(`| ${padNum(oldLine)} | ${padNum(newLine)} | | ${code}`);
oldLine++; oldLine++;
newLine++; newLine++;
} }
@@ -123170,8 +123166,7 @@ ${diffPreview}`);
const diffPath = join8(tempDir, `pr-${pull_number}.diff`); const diffPath = join8(tempDir, `pr-${pull_number}.diff`);
writeFileSync4(diffPath, diffContent); writeFileSync4(diffPath, diffContent);
log.debug(`wrote diff to ${diffPath} (${diffContent.length} bytes)`); log.debug(`wrote diff to ${diffPath} (${diffContent.length} bytes)`);
const MAX_INLINE_DIFF_SIZE = 50 * 1024; const diff = `Diff written to ${diffPath} (${(diffContent.length / 1024).toFixed(1)}KB). Use grep or read_file to explore.`;
const diff = diffContent.length <= MAX_INLINE_DIFF_SIZE ? diffContent : `Diff is ${(diffContent.length / 1024).toFixed(0)}KB - use grep or read from ${diffPath}`;
return { return {
success: true, success: true,
number: pr.data.number, number: pr.data.number,
@@ -124276,11 +124271,7 @@ function GetIssueCommentsTool(ctx) {
id: comment.id, id: comment.id,
body: comment.body, body: comment.body,
user: comment.user?.login, user: comment.user?.login,
created_at: comment.created_at, author_association: comment.author_association
updated_at: comment.updated_at,
html_url: comment.html_url,
author_association: comment.author_association,
reactions: comment.reactions
})), })),
count: comments.length count: comments.length
}; };
+6 -10
View File
@@ -12,7 +12,7 @@ type PullFile = RestEndpointMethodTypes["pulls"]["listFiles"]["response"]["data"
/** /**
* formats PR files with explicit line numbers for each code line. * formats PR files with explicit line numbers for each code line.
* preserves all original diff info (file headers, hunk headers) and adds: * preserves all original diff info (file headers, hunk headers) and adds:
* OLD | NEW | TYPE | code * | OLD | NEW | TYPE | code
*/ */
export function formatFilesWithLineNumbers(files: PullFile[]): string { export function formatFilesWithLineNumbers(files: PullFile[]): string {
const output: string[] = []; const output: string[] = [];
@@ -50,18 +50,18 @@ export function formatFilesWithLineNumbers(files: PullFile[]): string {
if (changeType === "-") { if (changeType === "-") {
// removed line: show old line number, no new line number // removed line: show old line number, no new line number
output.push(`${padNum(oldLine)} | | - | ${code}`); output.push(`| ${padNum(oldLine)} | | - | ${code}`);
oldLine++; oldLine++;
} else if (changeType === "+") { } else if (changeType === "+") {
// added line: no old line number, show new line number // added line: no old line number, show new line number
output.push(` | ${padNum(newLine)} | + | ${code}`); output.push(`| | ${padNum(newLine)} | + | ${code}`);
newLine++; newLine++;
} else if (changeType === " " || changeType === "\\") { } else if (changeType === " " || changeType === "\\") {
// context line or "\ No newline at end of file" // context line or "\ No newline at end of file"
if (changeType === "\\") { if (changeType === "\\") {
output.push(line); // pass through as-is output.push(line); // pass through as-is
} else { } else {
output.push(`${padNum(oldLine)} | ${padNum(newLine)} | | ${code}`); output.push(`| ${padNum(oldLine)} | ${padNum(newLine)} | | ${code}`);
oldLine++; oldLine++;
newLine++; newLine++;
} }
@@ -263,12 +263,8 @@ export function CheckoutPrTool(ctx: ToolContext) {
writeFileSync(diffPath, diffContent); writeFileSync(diffPath, diffContent);
log.debug(`wrote diff to ${diffPath} (${diffContent.length} bytes)`); log.debug(`wrote diff to ${diffPath} (${diffContent.length} bytes)`);
// return diff inline only if small enough // don't inline diff - always point to file to reduce context size
const MAX_INLINE_DIFF_SIZE = 50 * 1024; // 50KB const diff = `Diff written to ${diffPath} (${(diffContent.length / 1024).toFixed(1)}KB). Use grep or read_file to explore.`;
const diff =
diffContent.length <= MAX_INLINE_DIFF_SIZE
? diffContent
: `Diff is ${(diffContent.length / 1024).toFixed(0)}KB - use grep or read from ${diffPath}`;
return { return {
success: true, success: true,
-4
View File
@@ -28,11 +28,7 @@ export function GetIssueCommentsTool(ctx: ToolContext) {
id: comment.id, id: comment.id,
body: comment.body, body: comment.body,
user: comment.user?.login, user: comment.user?.login,
created_at: comment.created_at,
updated_at: comment.updated_at,
html_url: comment.html_url,
author_association: comment.author_association, author_association: comment.author_association,
reactions: comment.reactions,
})), })),
count: comments.length, count: comments.length,
}; };
+5 -8
View File
@@ -1,4 +1,5 @@
import type { StandardSchemaV1 } from "@standard-schema/spec"; import type { StandardSchemaV1 } from "@standard-schema/spec";
import { encode as toonEncode } from "@toon-format/toon";
import type { FastMCP, Tool } from "fastmcp"; import type { FastMCP, Tool } from "fastmcp";
import type { ToolContext } from "../main.ts"; import type { ToolContext } from "../main.ts";
@@ -12,14 +13,10 @@ export interface ToolResult {
isError?: boolean; isError?: boolean;
} }
export const handleToolSuccess = (data: Record<string, any>): ToolResult => { export const handleToolSuccess = (data: Record<string, any> | string): ToolResult => {
const text = typeof data === "string" ? data : toonEncode(data);
return { return {
content: [ content: [{ type: "text", text }],
{
type: "text",
text: JSON.stringify(data, null, 2),
},
],
}; };
}; };
@@ -40,7 +37,7 @@ export const handleToolError = (error: unknown): ToolResult => {
* Helper to wrap a tool execute function with error handling. * Helper to wrap a tool execute function with error handling.
* Captures ctx in closure so tools don't need to handle try/catch. * Captures ctx in closure so tools don't need to handle try/catch.
*/ */
export const execute = <T>(fn: (params: T) => Promise<Record<string, any>>) => { export const execute = <T>(fn: (params: T) => Promise<Record<string, any> | string>) => {
return async (params: T): Promise<ToolResult> => { return async (params: T): Promise<ToolResult> => {
try { try {
const result = await fn(params); const result = await fn(params);
+2 -2
View File
@@ -101,7 +101,7 @@ ${
"Review code, PRs, or implementations; provide feedback or suggestions; identify issues; or check code quality, style, and correctness", "Review code, PRs, or implementations; provide feedback or suggestions; identify issues; or check code quality, style, and correctness",
prompt: `Follow these steps. THINK HARDER. prompt: `Follow these steps. THINK HARDER.
1. **CHECKOUT** - Use ${ghPullfrogMcpName}/checkout_pr with the PR number. This fetches the PR branch and returns the diff. Use this diff for your review - it shows exactly what's in the PR. 1. **CHECKOUT** - Use ${ghPullfrogMcpName}/checkout_pr with the PR number. This returns all PR metadata (title, base, head, fork status) AND a \`diffPath\` pointing to a file containing the formatted diff.
2. **UNDERSTAND CONTEXT** - Read the modified files to understand the changes in context. Don't just look at the diff - understand how the changes affect the overall codebase. 2. **UNDERSTAND CONTEXT** - Read the modified files to understand the changes in context. Don't just look at the diff - understand how the changes affect the overall codebase.
@@ -111,7 +111,7 @@ ${
- What bugs, edge cases, or security issues exist? - What bugs, edge cases, or security issues exist?
- Could this be made more elegant? - Could this be made more elegant?
4. **DRAFT** - For each inline comment, find the line in the diff. Each code line shows: \`OLD | NEW | TYPE | CODE\`. Use the NEW line number (second column). 4. **DRAFT** - For each inline comment, find the line in the diff. Each code line shows: \`| OLD | NEW | TYPE | CODE\`. Use the NEW line number (second column).
5. **SELF-CRITIQUE** - Before submitting, review your draft: 5. **SELF-CRITIQUE** - Before submitting, review your draft:
- DO NOT NITPICK. Do not comment on minor formatting changes, changes to playground/scratch files, lack of docs/docsstrings, or small changes that seem irrelevant. Assume these things are intentional by the PR author. - DO NOT NITPICK. Do not comment on minor formatting changes, changes to playground/scratch files, lack of docs/docsstrings, or small changes that seem irrelevant. Assume these things are intentional by the PR author.