Skip to content

Commit e45a179

Browse files
committed
refactor(sdk): extract helpers from promptAiSdkStream (Commit 2.15)
📊 ~1,450 implementation lines, ~970 test lines Extracts helpers from run-state.ts and llm.ts into focused modules: - claude-oauth-errors.ts: OAuth error detection and handling - file-tree-builder.ts: Project file tree construction - git-operations.ts: Git status and diff operations - knowledge-files.ts: Knowledge file loading - project-discovery.ts: Project root and file discovery - session-state-processors.ts: Session state building - stream-cost-tracker.ts: Streaming cost calculation - tool-call-repair.ts: Malformed tool call recovery Includes comprehensive unit tests (150+ tests) for each extracted module.
1 parent 577bb07 commit e45a179

16 files changed

+1698
-727
lines changed
Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
import { describe, expect, it } from 'bun:test'
2+
3+
import {
4+
isClaudeOAuthRateLimitError,
5+
isClaudeOAuthAuthError,
6+
} from '../impl/claude-oauth-errors'
7+
8+
/**
9+
* These tests focus on DOMAIN LOGIC - the specific status codes and string patterns
10+
* we use to detect Claude OAuth errors. Low-value tests that just verify JavaScript
11+
* built-in behavior (typeof, null checks) have been removed.
12+
*/
13+
14+
describe('isClaudeOAuthRateLimitError', () => {
15+
describe('status code 429 detection', () => {
16+
it('should detect statusCode 429', () => {
17+
expect(isClaudeOAuthRateLimitError({ statusCode: 429 })).toBe(true)
18+
})
19+
20+
it('should detect status 429 (AI SDK format)', () => {
21+
expect(isClaudeOAuthRateLimitError({ status: 429 })).toBe(true)
22+
})
23+
24+
it('should NOT detect status 500 as rate limit', () => {
25+
expect(isClaudeOAuthRateLimitError({ statusCode: 500 })).toBe(false)
26+
})
27+
})
28+
29+
describe('message pattern detection', () => {
30+
it('should detect "rate_limit" (underscore)', () => {
31+
expect(isClaudeOAuthRateLimitError({ message: 'rate_limit exceeded' })).toBe(true)
32+
})
33+
34+
it('should detect "rate limit" (space)', () => {
35+
expect(isClaudeOAuthRateLimitError({ message: 'Rate limit exceeded' })).toBe(true)
36+
})
37+
38+
it('should detect "overloaded"', () => {
39+
expect(isClaudeOAuthRateLimitError({ message: 'API is overloaded' })).toBe(true)
40+
})
41+
42+
it('should be case-insensitive (calls toLowerCase)', () => {
43+
expect(isClaudeOAuthRateLimitError({ message: 'RATE_LIMIT' })).toBe(true)
44+
expect(isClaudeOAuthRateLimitError({ message: 'OVERLOADED' })).toBe(true)
45+
})
46+
})
47+
48+
describe('responseBody pattern detection', () => {
49+
it('should detect rate_limit in responseBody', () => {
50+
expect(isClaudeOAuthRateLimitError({ responseBody: '{"error": "rate_limit"}' })).toBe(true)
51+
})
52+
53+
it('should detect overloaded in responseBody', () => {
54+
expect(isClaudeOAuthRateLimitError({ responseBody: 'server is overloaded' })).toBe(true)
55+
})
56+
})
57+
58+
it('should work with real Error objects', () => {
59+
const error = new Error('Rate limit exceeded')
60+
;(error as any).statusCode = 429
61+
expect(isClaudeOAuthRateLimitError(error)).toBe(true)
62+
})
63+
})
64+
65+
describe('isClaudeOAuthAuthError', () => {
66+
describe('status code 401/403 detection', () => {
67+
it('should detect statusCode 401', () => {
68+
expect(isClaudeOAuthAuthError({ statusCode: 401 })).toBe(true)
69+
})
70+
71+
it('should detect statusCode 403', () => {
72+
expect(isClaudeOAuthAuthError({ statusCode: 403 })).toBe(true)
73+
})
74+
75+
it('should NOT detect status 429 as auth error', () => {
76+
expect(isClaudeOAuthAuthError({ statusCode: 429 })).toBe(false)
77+
})
78+
})
79+
80+
describe('message pattern detection', () => {
81+
it('should detect "unauthorized"', () => {
82+
expect(isClaudeOAuthAuthError({ message: 'Request unauthorized' })).toBe(true)
83+
})
84+
85+
it('should detect "invalid_token"', () => {
86+
expect(isClaudeOAuthAuthError({ message: 'invalid_token: expired' })).toBe(true)
87+
})
88+
89+
it('should detect "authentication"', () => {
90+
expect(isClaudeOAuthAuthError({ message: 'Authentication failed' })).toBe(true)
91+
})
92+
93+
it('should detect "expired"', () => {
94+
expect(isClaudeOAuthAuthError({ message: 'Token expired' })).toBe(true)
95+
})
96+
97+
it('should be case-insensitive', () => {
98+
expect(isClaudeOAuthAuthError({ message: 'UNAUTHORIZED' })).toBe(true)
99+
})
100+
})
101+
102+
describe('responseBody pattern detection', () => {
103+
it('should detect auth patterns in responseBody', () => {
104+
expect(isClaudeOAuthAuthError({ responseBody: '{"error": "unauthorized"}' })).toBe(true)
105+
expect(isClaudeOAuthAuthError({ responseBody: 'invalid_token' })).toBe(true)
106+
expect(isClaudeOAuthAuthError({ responseBody: 'token has expired' })).toBe(true)
107+
})
108+
})
109+
})
110+
111+
describe('error type mutual exclusivity', () => {
112+
it('rate limit errors should NOT be auth errors', () => {
113+
const rateLimitError = { statusCode: 429, message: 'rate_limit' }
114+
expect(isClaudeOAuthRateLimitError(rateLimitError)).toBe(true)
115+
expect(isClaudeOAuthAuthError(rateLimitError)).toBe(false)
116+
})
117+
118+
it('auth errors should NOT be rate limit errors', () => {
119+
const authError = { statusCode: 401, message: 'unauthorized' }
120+
expect(isClaudeOAuthAuthError(authError)).toBe(true)
121+
expect(isClaudeOAuthRateLimitError(authError)).toBe(false)
122+
})
123+
124+
it('server errors (500) should be neither', () => {
125+
const serverError = { statusCode: 500, message: 'internal server error' }
126+
expect(isClaudeOAuthRateLimitError(serverError)).toBe(false)
127+
expect(isClaudeOAuthAuthError(serverError)).toBe(false)
128+
})
129+
})
130+
131+
/**
132+
* Mutation tests - verify our tests would catch real bugs.
133+
* These document the specific patterns our implementation relies on.
134+
*/
135+
describe('mutation detection (documents implementation requirements)', () => {
136+
it('REQUIRES status 429 for rate limit (not 428)', () => {
137+
// If implementation changed 429 to 428, this test would catch it
138+
expect(isClaudeOAuthRateLimitError({ statusCode: 428 })).toBe(false)
139+
expect(isClaudeOAuthRateLimitError({ statusCode: 429 })).toBe(true)
140+
})
141+
142+
it('REQUIRES "overloaded" pattern for rate limit detection', () => {
143+
// If implementation removed "overloaded" check, this test would catch it
144+
expect(isClaudeOAuthRateLimitError({ message: 'overloaded' })).toBe(true)
145+
})
146+
147+
it('REQUIRES both 401 AND 403 for auth errors', () => {
148+
// If implementation only checked 401, this test would catch it
149+
expect(isClaudeOAuthAuthError({ statusCode: 401 })).toBe(true)
150+
expect(isClaudeOAuthAuthError({ statusCode: 403 })).toBe(true)
151+
})
152+
153+
it('REQUIRES "expired" pattern for auth error detection', () => {
154+
// If implementation removed "expired" check, this test would catch it
155+
expect(isClaudeOAuthAuthError({ message: 'expired' })).toBe(true)
156+
})
157+
})
Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
import { describe, expect, it, mock } from 'bun:test'
2+
3+
import { buildFileTree, computeProjectIndex } from '../impl/file-tree-builder'
4+
5+
/**
6+
* These tests focus on DOMAIN LOGIC - the tree building algorithm,
7+
* sorting rules (directories before files, alphabetical), and hierarchy.
8+
* Low-value tests that just verify JavaScript built-in behavior have been removed.
9+
*/
10+
11+
describe('buildFileTree', () => {
12+
describe('tree structure building', () => {
13+
it('should create nested directory structure from path', () => {
14+
const result = buildFileTree(['src/components/Button.tsx'])
15+
16+
expect(result[0].name).toBe('src')
17+
expect(result[0].type).toBe('directory')
18+
expect(result[0].children![0].name).toBe('components')
19+
expect(result[0].children![0].children![0].name).toBe('Button.tsx')
20+
expect(result[0].children![0].children![0].type).toBe('file')
21+
})
22+
23+
it('should group multiple files in same directory', () => {
24+
const result = buildFileTree(['src/a.ts', 'src/b.ts', 'src/c.ts'])
25+
26+
expect(result).toHaveLength(1) // single src directory
27+
expect(result[0].children).toHaveLength(3) // three files inside
28+
})
29+
30+
it('should create separate root directories', () => {
31+
const result = buildFileTree(['src/file.ts', 'lib/file.ts', 'tests/file.ts'])
32+
33+
expect(result).toHaveLength(3)
34+
expect(result.map(n => n.name)).toContain('src')
35+
expect(result.map(n => n.name)).toContain('lib')
36+
expect(result.map(n => n.name)).toContain('tests')
37+
})
38+
39+
it('should handle mixed root files and directories', () => {
40+
const result = buildFileTree(['root.ts', 'src/nested.ts'])
41+
42+
const rootFile = result.find(n => n.name === 'root.ts')
43+
const srcDir = result.find(n => n.name === 'src')
44+
45+
expect(rootFile?.type).toBe('file')
46+
expect(srcDir?.type).toBe('directory')
47+
})
48+
})
49+
50+
describe('sorting: directories before files, then alphabetical', () => {
51+
it('should sort directories BEFORE files at same level', () => {
52+
const result = buildFileTree(['file.ts', 'src/file.ts', 'another.ts'])
53+
54+
// Directory should come first
55+
expect(result[0].name).toBe('src')
56+
expect(result[0].type).toBe('directory')
57+
// Then files
58+
expect(result[1].type).toBe('file')
59+
expect(result[2].type).toBe('file')
60+
})
61+
62+
it('should sort alphabetically within same type', () => {
63+
const result = buildFileTree(['z.ts', 'a.ts', 'm.ts'])
64+
65+
expect(result[0].name).toBe('a.ts')
66+
expect(result[1].name).toBe('m.ts')
67+
expect(result[2].name).toBe('z.ts')
68+
})
69+
70+
it('should sort children within directories', () => {
71+
const result = buildFileTree(['src/z.ts', 'src/a.ts', 'src/utils/helper.ts'])
72+
73+
// utils directory should come before files
74+
expect(result[0].children![0].name).toBe('utils')
75+
expect(result[0].children![0].type).toBe('directory')
76+
// then files alphabetically
77+
expect(result[0].children![1].name).toBe('a.ts')
78+
expect(result[0].children![2].name).toBe('z.ts')
79+
})
80+
})
81+
82+
describe('filePath property', () => {
83+
it('should set correct filePath for nested items', () => {
84+
const result = buildFileTree(['src/components/Button.tsx'])
85+
86+
expect(result[0].filePath).toBe('src')
87+
expect(result[0].children![0].filePath).toBe('src/components')
88+
expect(result[0].children![0].children![0].filePath).toBe('src/components/Button.tsx')
89+
})
90+
})
91+
92+
describe('complex project structure', () => {
93+
it('should handle typical project layout', () => {
94+
const files = [
95+
'package.json',
96+
'tsconfig.json',
97+
'src/index.ts',
98+
'src/components/Button.tsx',
99+
'src/utils/helpers.ts',
100+
'tests/unit/button.test.ts',
101+
]
102+
const result = buildFileTree(files)
103+
104+
// Directories first (src, tests), then files (package.json, tsconfig.json)
105+
expect(result[0].type).toBe('directory')
106+
expect(result[1].type).toBe('directory')
107+
expect(result.map(n => n.name)).toContain('src')
108+
expect(result.map(n => n.name)).toContain('tests')
109+
expect(result.map(n => n.name)).toContain('package.json')
110+
})
111+
})
112+
})
113+
114+
describe('computeProjectIndex', () => {
115+
it('should return file tree for project files', async () => {
116+
const projectFiles = {
117+
'src/index.ts': 'export const hello = "world"',
118+
'src/utils.ts': 'export const add = (a, b) => a + b',
119+
}
120+
121+
const result = await computeProjectIndex('/mock/cwd', projectFiles)
122+
123+
expect(result.fileTree).toHaveLength(1)
124+
expect(result.fileTree[0].name).toBe('src')
125+
expect(result.fileTree[0].children).toHaveLength(2)
126+
})
127+
128+
it('should sort file paths before building tree', async () => {
129+
const projectFiles = {
130+
'z.ts': 'const z = 1',
131+
'a.ts': 'const a = 1',
132+
'm.ts': 'const m = 1',
133+
}
134+
135+
const result = await computeProjectIndex('/mock/cwd', projectFiles)
136+
137+
expect(result.fileTree[0].name).toBe('a.ts')
138+
expect(result.fileTree[1].name).toBe('m.ts')
139+
expect(result.fileTree[2].name).toBe('z.ts')
140+
})
141+
142+
it('should return correct structure shape', async () => {
143+
const result = await computeProjectIndex('/mock/cwd', { 'file.ts': 'export const x = 1' })
144+
145+
expect(result).toHaveProperty('fileTree')
146+
expect(result).toHaveProperty('fileTokenScores')
147+
expect(result).toHaveProperty('tokenCallers')
148+
expect(Array.isArray(result.fileTree)).toBe(true)
149+
})
150+
})
151+
152+
/**
153+
* Mutation tests - verify our tests would catch real bugs
154+
*/
155+
describe('mutation detection', () => {
156+
it('REQUIRES directories to sort before files', () => {
157+
// If sorting was removed/broken, this would fail
158+
const result = buildFileTree(['z-file.ts', 'a-dir/file.ts'])
159+
expect(result[0].name).toBe('a-dir') // directory first, even though z < a alphabetically for files
160+
expect(result[0].type).toBe('directory')
161+
})
162+
163+
it('REQUIRES alphabetical sorting within type', () => {
164+
// If localeCompare was removed, this would fail
165+
const result = buildFileTree(['z.ts', 'a.ts'])
166+
expect(result[0].name).toBe('a.ts')
167+
})
168+
169+
it('REQUIRES recursive sorting of children', () => {
170+
// If sortNodes wasn't called recursively, this would fail
171+
const result = buildFileTree(['parent/z.ts', 'parent/a.ts'])
172+
expect(result[0].children![0].name).toBe('a.ts')
173+
})
174+
})

0 commit comments

Comments
 (0)