Skip to content
Merged
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
51 changes: 51 additions & 0 deletions src/bash-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,54 @@ export function extractBashCommands(rawCommand: string): string[] {

return commands
}

// Read-shaped shell commands: they inspect state and cannot modify the
// worktree, so (1) the read-edit-ratio detector counts them as reads and
// (2) retry detection does NOT treat them as a verification step between two
// edits of the same file (#941 — both detectors previously disagreed about
// the same Bash call, scoring rg-first workflows as reckless AND reworked).
const READ_ONLY_BASH = new Set([
'rg', 'grep', 'egrep', 'fgrep', 'ag',
'cat', 'head', 'tail', 'less', 'more',
'ls', 'find', 'fd', 'tree',
'wc', 'stat', 'file', 'du', 'df',
'which', 'type', 'pwd', 'printenv', 'env',
'readlink', 'realpath', 'basename', 'dirname',
'jq', 'diff',
])

// git subcommands that only inspect history/state. Deliberately excludes
// anything that can create or mutate under any flag (branch, tag, stash,
// remote), so a mutation is never misread as a read.
const GIT_READ_SUBCOMMANDS = new Set([
'log', 'diff', 'status', 'show', 'blame', 'grep',
'shortlog', 'describe', 'rev-parse', 'ls-files',
])

/// True when EVERY segment of the raw command line (split on &&, ;, |) is a
/// read-only inspection command. A single mutating segment makes the whole
/// call non-read (`cat x && sed -i ...` edits). Unknown commands are
/// non-read: the conservative default both call sites want.
export function isReadShapedBashCommand(rawCommand: string): boolean {
if (!rawCommand || !rawCommand.trim()) return false
const stripped = stripQuotedStrings(stripAnsi(rawCommand))
const segments = stripped.split(/\s*(?:&&|;|\|)\s*/)
let sawCommand = false
for (const segment of segments) {
const trimmed = segment.trim()
if (!trimmed) continue
const tokens = trimmed.split(/\s+/)
let i = 0
while (i < tokens.length && (/^\w+=/.test(tokens[i]!) || COMMAND_PREFIXES.has(basename(tokens[i]!)))) i++
const base = i < tokens.length ? basename(tokens[i]!) : ''
if (!base) continue
sawCommand = true
if (base === 'git') {
const sub = tokens[i + 1]
if (!sub || !GIT_READ_SUBCOMMANDS.has(sub)) return false
continue
}
if (!READ_ONLY_BASH.has(base)) return false
}
return sawCommand
}
9 changes: 8 additions & 1 deletion src/classifier.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { ClassifiedTurn, ParsedTurn, TaskCategory, ToolCall } from './types.js'
import { isReadShapedBashCommand } from './bash-utils.js'

const TEST_PATTERNS = /\b(test|pytest|vitest|jest|mocha|spec|coverage|npm\s+test|npx\s+vitest|npx\s+jest)\b/i
const GIT_PATTERNS = /\bgit\s+(push|pull|commit|merge|rebase|checkout|branch|stash|log|diff|status|add|reset|cherry-pick|tag)\b/i
Expand Down Expand Up @@ -170,7 +171,13 @@ function countRetries(turn: ParsedTurn): number {
steps.forEach((step, i) => {
for (const call of step) {
if (BASH_TOOLS.has(call.tool)) {
lastVerifyStep = i
// A read-shaped shell command (rg/cat/git log) is a lookup, not a
// verification of the previous edit: edit -> grep -> edit is research,
// and counting it as a retry penalized bash-first workflows (#941).
// A command we cannot classify keeps the old behavior.
if (!call.command || !isReadShapedBashCommand(call.command)) {
lastVerifyStep = i
}
}
if (EDIT_TOOLS.has(call.tool)) {
const fileKey = call.file ?? '__no_file__'
Expand Down
12 changes: 12 additions & 0 deletions src/optimize.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import chalk from 'chalk'
import { isReadShapedBashCommand } from './bash-utils.js'
import { readdir, stat } from 'fs/promises'
import { existsSync, statSync } from 'fs'
import { basename, join } from 'path'
Expand Down Expand Up @@ -383,6 +384,10 @@ function compactOptimizeInput(name: string, input: unknown): Record<string, unkn
const filePath = cappedString(raw['file_path'], OPTIMIZE_TEXT_CAP)
return filePath ? { file_path: filePath } : {}
}
if (BASH_TOOL_NAMES.has(name)) {
const command = cappedString(raw['command'], OPTIMIZE_TEXT_CAP)
return command ? { command } : {}
}
if (name === 'Agent' || name === 'Task') {
const subagentType = cappedString(raw['subagent_type'])
return subagentType ? { subagent_type: subagentType } : {}
Expand Down Expand Up @@ -2189,6 +2194,7 @@ export function detectBloatedClaudeMd(projectCwds: Set<string>): WasteFinding |

export const READ_TOOL_NAMES = new Set(['Read', 'Grep', 'Glob', 'FileReadTool', 'GrepTool', 'GlobTool'])
export const EDIT_TOOL_NAMES = new Set(['Edit', 'Write', 'FileEditTool', 'FileWriteTool', 'NotebookEdit'])
export const BASH_TOOL_NAMES = new Set(['Bash', 'BashTool', 'PowerShellTool'])

export function detectLowReadEditRatio(calls: ToolCall[]): WasteFinding | null {
let reads = 0
Expand All @@ -2199,6 +2205,12 @@ export function detectLowReadEditRatio(calls: ToolCall[]): WasteFinding | null {
if (READ_TOOL_NAMES.has(call.name)) {
reads++
if (call.recent) recentReads++
} else if (BASH_TOOL_NAMES.has(call.name) && typeof call.input['command'] === 'string' && isReadShapedBashCommand(call.input['command'])) {
// A session that looks things up with rg/cat/git log IS reading (#941):
// ignoring shell reads scored disciplined rg-first workflows as
// reckless editors (90%+ of real reads were invisible to this ratio).
reads++
if (call.recent) recentReads++
} else if (EDIT_TOOL_NAMES.has(call.name)) {
edits++
if (call.recent) recentEdits++
Expand Down
32 changes: 31 additions & 1 deletion tests/bash-commands.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, it, expect } from 'vitest'
import { extractBashCommands } from '../src/bash-utils.js'
import { extractBashCommands, isReadShapedBashCommand } from '../src/bash-utils.js'
import { BASH_TOOLS } from '../src/classifier.js'

describe('extractBashCommands', () => {
Expand Down Expand Up @@ -117,3 +117,33 @@ describe('BASH_TOOLS', () => {
it('recognizes BashTool', () => { expect(BASH_TOOLS.has('BashTool')).toBe(true) })
it('rejects unknown tools', () => { expect(BASH_TOOLS.has('Read')).toBe(false) })
})

describe('isReadShapedBashCommand (#941)', () => {
it('accepts single read commands and read-only git subcommands', () => {
expect(isReadShapedBashCommand('rg -n "x" src/')).toBe(true)
expect(isReadShapedBashCommand('cat file.ts')).toBe(true)
expect(isReadShapedBashCommand('git log --oneline -5')).toBe(true)
expect(isReadShapedBashCommand('git diff HEAD~1')).toBe(true)
expect(isReadShapedBashCommand('VAR=1 sudo head -c 100 f')).toBe(true)
})

it('accepts pipelines where every segment reads', () => {
expect(isReadShapedBashCommand('grep -r "x" src | head -20')).toBe(true)
expect(isReadShapedBashCommand('git log --oneline && git status')).toBe(true)
})

it('rejects any mutating or unknown segment', () => {
expect(isReadShapedBashCommand('npm test')).toBe(false)
expect(isReadShapedBashCommand('sed -i s/a/b/ x.ts')).toBe(false)
expect(isReadShapedBashCommand('cat x && rm -rf dist')).toBe(false)
expect(isReadShapedBashCommand('git commit -m "x"')).toBe(false)
expect(isReadShapedBashCommand('git branch new-branch')).toBe(false)
expect(isReadShapedBashCommand('git')).toBe(false)
expect(isReadShapedBashCommand('')).toBe(false)
expect(isReadShapedBashCommand(' ')).toBe(false)
})

it('is not fooled by read-command names inside quoted strings', () => {
expect(isReadShapedBashCommand('echo "cat file"')).toBe(false)
})
})
22 changes: 22 additions & 0 deletions tests/classifier.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,28 @@ describe('classifyTurn — feature vs debugging precedence (#196)', () => {
})

describe('classifyTurn — retry detection via toolSequence', () => {
it('does not count a read-shaped shell lookup between edits as a retry (#941)', () => {
// edit -> rg -> edit is research, not a failed verification loop.
const call = makeCall({ tools: ['Edit', 'Bash'] })
call.toolSequence = [[{ tool: 'Edit', file: 'a.ts' }], [{ tool: 'Bash', command: 'rg -n "helper" src/' }], [{ tool: 'Edit', file: 'a.ts' }]]
const turn = makeTurn([call], 'fix the build')
expect(classifyTurn(turn).retries).toBe(0)
})

it('still counts a verification-shaped shell command between edits as a retry (#941)', () => {
const call = makeCall({ tools: ['Edit', 'Bash'] })
call.toolSequence = [[{ tool: 'Edit', file: 'a.ts' }], [{ tool: 'Bash', command: 'npm test' }], [{ tool: 'Edit', file: 'a.ts' }]]
const turn = makeTurn([call], 'fix the build')
expect(classifyTurn(turn).retries).toBe(1)
})

it('keeps counting command-less bash steps as verification (unknown stays conservative)', () => {
const call = makeCall({ tools: ['Edit', 'Bash'] })
call.toolSequence = [[{ tool: 'Edit', file: 'a.ts' }], [{ tool: 'Bash' }], [{ tool: 'Edit', file: 'a.ts' }]]
const turn = makeTurn([call], 'fix the build')
expect(classifyTurn(turn).retries).toBe(1)
})

it('detects retries from multi-call turns (Claude-style)', () => {
const turn = makeTurn([
makeCall({ tools: ['Edit'] }),
Expand Down
24 changes: 24 additions & 0 deletions tests/optimize.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,30 @@ describe('detectDuplicateReads', () => {
})

describe('detectLowReadEditRatio', () => {
it('counts read-shaped bash commands as reads (#941)', () => {
// 10 edits with 40 rg/cat/git-log lookups through the shell: a healthy
// 4:1 workflow that previously read as 0 reads and fired at high impact.
const calls = [
...Array.from({ length: 20 }, () => call('Bash', { command: 'rg -n "pattern" src/' })),
...Array.from({ length: 10 }, () => call('Bash', { command: 'cat src/parser.ts | head -50' })),
...Array.from({ length: 10 }, () => call('Bash', { command: 'git log --oneline -5' })),
...Array.from({ length: 10 }, () => call('Edit', {})),
]
expect(detectLowReadEditRatio(calls)).toBeNull()
})

it('does not count mutating or unclassifiable bash as reads (#941)', () => {
const calls = [
...Array.from({ length: 20 }, () => call('Bash', { command: 'npm test' })),
...Array.from({ length: 10 }, () => call('Bash', { command: 'cat notes.md && sed -i s/a/b/ src/x.ts' })),
...Array.from({ length: 10 }, () => call('Bash', {})),
...Array.from({ length: 10 }, () => call('Edit', {})),
]
const finding = detectLowReadEditRatio(calls)
expect(finding).not.toBeNull()
expect(finding!.explanation).toContain('0 reads')
})

it('returns null below minimum edit count', () => {
const calls = [
call('Edit', {}),
Expand Down
Loading