Skip to content

Commit 7811d6a

Browse files
brandonkachenbrandonkachen
committed
refactor(sdk): extract helpers from run-state.ts (Commit 2.16)
1 parent f2ee56f commit 7811d6a

File tree

9 files changed

+665
-486
lines changed

9 files changed

+665
-486
lines changed

‎REFACTORING_PLAN.md‎

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ This document outlines a prioritized refactoring plan for the 51 issues identifi
4646
| 2.13 | Fix browser actions + string utils | ✅ Complete | Codebuff |
4747
| 2.14 | Refactor agent-builder.ts | ✅ Complete | Codebuff |
4848
| 2.15 | Refactor promptAiSdkStream | ✅ Complete | Codebuff |
49-
| 2.16 | Simplify run-state.ts | ⬜ Not Started | - |
49+
| 2.16 | Simplify run-state.ts | ✅ Complete | Codebuff |
5050

5151
### Phase 3 Progress
5252
| Commit | Description | Status | Completed By |
@@ -1122,20 +1122,40 @@ Extracted pure utility modules instead of React hooks (as originally planned) be
11221122

11231123
---
11241124

1125-
### Commit 2.16: Simplify `run-state.ts` in SDK
1125+
### Commit 2.16: Simplify `run-state.ts` in SDK ✅ COMPLETE
11261126
**Files:** `sdk/src/run-state.ts`
11271127
**Est. Time:** 3-4 hours
1128-
**Est. LOC Changed:** ~400-500
1128+
**Actual Time:** ~2 hours
1129+
**Est. LOC Changed:** ~400-500
1130+
**Actual LOC Changed:** ~420 lines extracted to 5 new files
11291131

11301132
> **Moved from Phase 3:** File is 737 lines, not a minor cleanup task.
11311133
1132-
| Task | Description |
1133-
|------|-------------|
1134-
| Audit state complexity | Identify unnecessary parts |
1135-
| Extract state machine helpers | `createStateTransition()` |
1136-
| Remove unused state fields | Clean up |
1137-
| Simplify state transitions | Reduce complexity |
1138-
| Update tests | Ensure coverage |
1134+
| Task | Description | Status |
1135+
|------|-------------|--------|
1136+
| Audit state complexity | Identified 5 extraction targets | ✅ |
1137+
| Create `file-tree-builder.ts` | `buildFileTree()`, `computeProjectIndex()` | ✅ |
1138+
| Create `git-operations.ts` | `getGitChanges()`, `childProcessToPromise()` | ✅ |
1139+
| Create `knowledge-files.ts` | Knowledge file discovery and selection utilities | ✅ |
1140+
| Create `project-discovery.ts` | `discoverProjectFiles()` | ✅ |
1141+
| Create `session-state-processors.ts` | `processAgentDefinitions()`, `processCustomToolDefinitions()` | ✅ |
1142+
| Simplify main function | Reduced to orchestration only | ✅ |
1143+
| Update re-exports | Maintain backward compatibility for tests | ✅ |
1144+
1145+
**New Files Created:**
1146+
- `sdk/src/impl/file-tree-builder.ts` (~95 lines) - File tree construction and token scoring
1147+
- `sdk/src/impl/git-operations.ts` (~85 lines) - Git state retrieval
1148+
- `sdk/src/impl/knowledge-files.ts` (~115 lines) - Knowledge file discovery and selection
1149+
- `sdk/src/impl/project-discovery.ts` (~50 lines) - Project file discovery using gitignore
1150+
- `sdk/src/impl/session-state-processors.ts` (~55 lines) - Agent/tool definition processing
1151+
1152+
**Code Reduction:**
1153+
- `run-state.ts`: ~737 → ~315 lines (-57%)
1154+
1155+
**Test Results:**
1156+
- 281 SDK tests pass
1157+
- TypeScript compiles cleanly
1158+
- Backward compatibility maintained via re-exports
11391159

11401160
**Dependencies:** Commit 2.15
11411161
**Risk:** Medium

‎sdk/src/__tests__/knowledge-file-selection.test.ts‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,3 +333,131 @@ describe('selectKnowledgeFilePaths', () => {
333333
expect(result).toContain('lib/CLAUDE.md')
334334
})
335335
})
336+
337+
describe('selectKnowledgeFilePaths - pattern files (*.knowledge.md)', () => {
338+
test('includes ALL *.knowledge.md pattern files from same directory', () => {
339+
const files = [
340+
'src/auth.knowledge.md',
341+
'src/api.knowledge.md',
342+
'src/database.knowledge.md',
343+
]
344+
const result = selectKnowledgeFilePaths(files)
345+
346+
expect(result).toHaveLength(3)
347+
expect(result).toContain('src/auth.knowledge.md')
348+
expect(result).toContain('src/api.knowledge.md')
349+
expect(result).toContain('src/database.knowledge.md')
350+
})
351+
352+
test('includes pattern files even when standard file exists in same directory', () => {
353+
const files = [
354+
'src/knowledge.md',
355+
'src/AGENTS.md',
356+
'src/auth.knowledge.md',
357+
'src/api.knowledge.md',
358+
]
359+
const result = selectKnowledgeFilePaths(files)
360+
361+
// Should have 1 standard file (knowledge.md wins by priority) + 2 pattern files
362+
expect(result).toHaveLength(3)
363+
expect(result).toContain('src/knowledge.md')
364+
expect(result).not.toContain('src/AGENTS.md') // Lower priority, same dir
365+
expect(result).toContain('src/auth.knowledge.md')
366+
expect(result).toContain('src/api.knowledge.md')
367+
})
368+
369+
test('includes pattern files alone in a directory', () => {
370+
const files = [
371+
'src/auth.knowledge.md',
372+
'lib/api.knowledge.md',
373+
]
374+
const result = selectKnowledgeFilePaths(files)
375+
376+
expect(result).toHaveLength(2)
377+
expect(result).toContain('src/auth.knowledge.md')
378+
expect(result).toContain('lib/api.knowledge.md')
379+
})
380+
381+
test('includes pattern files from multiple directories', () => {
382+
const files = [
383+
'src/auth.knowledge.md',
384+
'src/api.knowledge.md',
385+
'lib/database.knowledge.md',
386+
'docs/deployment.knowledge.md',
387+
]
388+
const result = selectKnowledgeFilePaths(files)
389+
390+
expect(result).toHaveLength(4)
391+
expect(result).toContain('src/auth.knowledge.md')
392+
expect(result).toContain('src/api.knowledge.md')
393+
expect(result).toContain('lib/database.knowledge.md')
394+
expect(result).toContain('docs/deployment.knowledge.md')
395+
})
396+
397+
test('handles mixed standard and pattern files across multiple directories', () => {
398+
const files = [
399+
'src/knowledge.md',
400+
'src/auth.knowledge.md',
401+
'lib/AGENTS.md',
402+
'lib/api.knowledge.md',
403+
'docs/CLAUDE.md',
404+
'docs/deployment.knowledge.md',
405+
'docs/testing.knowledge.md',
406+
]
407+
const result = selectKnowledgeFilePaths(files)
408+
409+
// 3 standard files (one per directory) + 4 pattern files
410+
expect(result).toHaveLength(7)
411+
// Standard files (one per directory)
412+
expect(result).toContain('src/knowledge.md')
413+
expect(result).toContain('lib/AGENTS.md')
414+
expect(result).toContain('docs/CLAUDE.md')
415+
// ALL pattern files
416+
expect(result).toContain('src/auth.knowledge.md')
417+
expect(result).toContain('lib/api.knowledge.md')
418+
expect(result).toContain('docs/deployment.knowledge.md')
419+
expect(result).toContain('docs/testing.knowledge.md')
420+
})
421+
422+
test('orders standard files before pattern files in result', () => {
423+
const files = [
424+
'src/auth.knowledge.md',
425+
'src/knowledge.md',
426+
'lib/api.knowledge.md',
427+
]
428+
const result = selectKnowledgeFilePaths(files)
429+
430+
expect(result).toHaveLength(3)
431+
// Standard file should come first
432+
expect(result[0]).toBe('src/knowledge.md')
433+
// Pattern files follow
434+
expect(result.slice(1)).toContain('src/auth.knowledge.md')
435+
expect(result.slice(1)).toContain('lib/api.knowledge.md')
436+
})
437+
438+
test('handles case-insensitive matching for pattern files', () => {
439+
const files = [
440+
'src/AUTH.KNOWLEDGE.MD',
441+
'lib/Api.Knowledge.md',
442+
'docs/database.knowledge.md',
443+
]
444+
const result = selectKnowledgeFilePaths(files)
445+
446+
expect(result).toHaveLength(3)
447+
expect(result).toContain('src/AUTH.KNOWLEDGE.MD')
448+
expect(result).toContain('lib/Api.Knowledge.md')
449+
expect(result).toContain('docs/database.knowledge.md')
450+
})
451+
452+
test('does not match files with knowledge in name but wrong pattern', () => {
453+
const files = [
454+
'src/myknowledge.md', // Should NOT match - no dot separator
455+
'src/knowledge-file.md', // Should NOT match - not exact match
456+
'src/auth.knowledge.md', // Should match
457+
]
458+
const result = selectKnowledgeFilePaths(files)
459+
460+
expect(result).toHaveLength(1)
461+
expect(result).toContain('src/auth.knowledge.md')
462+
})
463+
})

‎sdk/src/__tests__/user-knowledge-files.test.ts‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import { describe, it, expect, mock } from 'bun:test'
22

3-
import { loadUserKnowledgeFiles } from '../run-state'
3+
import { loadUserKnowledgeFile } from '../run-state'
44

55
const MOCK_HOME = '/mock/home'
66

7-
describe('loadUserKnowledgeFiles', () => {
7+
describe('loadUserKnowledgeFile', () => {
88
it('should return empty object when no knowledge files exist', async () => {
99
const mockFs = {
1010
readdir: mock(async () => ['.bashrc', '.gitconfig', '.profile']),
@@ -14,7 +14,7 @@ describe('loadUserKnowledgeFiles', () => {
1414
}
1515
const mockLogger = { debug: mock(() => {}) }
1616

17-
const result = await loadUserKnowledgeFiles({
17+
const result = await loadUserKnowledgeFile({
1818
fs: mockFs as any,
1919
logger: mockLogger as any,
2020
homeDir: MOCK_HOME,
@@ -35,7 +35,7 @@ describe('loadUserKnowledgeFiles', () => {
3535
}
3636
const mockLogger = { debug: mock(() => {}) }
3737

38-
const result = await loadUserKnowledgeFiles({
38+
const result = await loadUserKnowledgeFile({
3939
fs: mockFs as any,
4040
logger: mockLogger as any,
4141
homeDir: MOCK_HOME,
@@ -56,7 +56,7 @@ describe('loadUserKnowledgeFiles', () => {
5656
}
5757
const mockLogger = { debug: mock(() => {}) }
5858

59-
const result = await loadUserKnowledgeFiles({
59+
const result = await loadUserKnowledgeFile({
6060
fs: mockFs as any,
6161
logger: mockLogger as any,
6262
homeDir: MOCK_HOME,
@@ -77,7 +77,7 @@ describe('loadUserKnowledgeFiles', () => {
7777
}
7878
const mockLogger = { debug: mock(() => {}) }
7979

80-
const result = await loadUserKnowledgeFiles({
80+
const result = await loadUserKnowledgeFile({
8181
fs: mockFs as any,
8282
logger: mockLogger as any,
8383
homeDir: MOCK_HOME,
@@ -101,7 +101,7 @@ describe('loadUserKnowledgeFiles', () => {
101101
}
102102
const mockLogger = { debug: mock(() => {}) }
103103

104-
const result = await loadUserKnowledgeFiles({
104+
const result = await loadUserKnowledgeFile({
105105
fs: mockFs as any,
106106
logger: mockLogger as any,
107107
homeDir: MOCK_HOME,
@@ -125,7 +125,7 @@ describe('loadUserKnowledgeFiles', () => {
125125
}
126126
const mockLogger = { debug: mock(() => {}) }
127127

128-
const result = await loadUserKnowledgeFiles({
128+
const result = await loadUserKnowledgeFile({
129129
fs: mockFs as any,
130130
logger: mockLogger as any,
131131
homeDir: MOCK_HOME,
@@ -157,7 +157,7 @@ describe('loadUserKnowledgeFiles', () => {
157157
}
158158
const mockLogger = { debug: mock(() => {}) }
159159

160-
const result = await loadUserKnowledgeFiles({
160+
const result = await loadUserKnowledgeFile({
161161
fs: mockFs as any,
162162
logger: mockLogger as any,
163163
homeDir: MOCK_HOME,
@@ -180,7 +180,7 @@ describe('loadUserKnowledgeFiles', () => {
180180
}
181181
const mockLogger = { debug: mock(() => {}) }
182182

183-
const result = await loadUserKnowledgeFiles({
183+
const result = await loadUserKnowledgeFile({
184184
fs: mockFs as any,
185185
logger: mockLogger as any,
186186
homeDir: MOCK_HOME,
@@ -202,7 +202,7 @@ describe('loadUserKnowledgeFiles', () => {
202202
}
203203
const mockLogger = { debug: mock(() => {}) }
204204

205-
const result = await loadUserKnowledgeFiles({
205+
const result = await loadUserKnowledgeFile({
206206
fs: mockFs as any,
207207
logger: mockLogger as any,
208208
homeDir: MOCK_HOME,
@@ -224,7 +224,7 @@ describe('loadUserKnowledgeFiles', () => {
224224
}
225225
const mockLogger = { debug: mock(() => {}) }
226226

227-
const result = await loadUserKnowledgeFiles({
227+
const result = await loadUserKnowledgeFile({
228228
fs: mockFs as any,
229229
logger: mockLogger as any,
230230
homeDir: MOCK_HOME,
@@ -246,7 +246,7 @@ describe('loadUserKnowledgeFiles', () => {
246246
}
247247
const mockLogger = { debug: mock(() => {}) }
248248

249-
const result = await loadUserKnowledgeFiles({
249+
const result = await loadUserKnowledgeFile({
250250
fs: mockFs as any,
251251
logger: mockLogger as any,
252252
homeDir: MOCK_HOME,
@@ -271,7 +271,7 @@ describe('loadUserKnowledgeFiles', () => {
271271
}
272272
const mockLogger = { debug: mock(() => {}) }
273273

274-
const result = await loadUserKnowledgeFiles({
274+
const result = await loadUserKnowledgeFile({
275275
fs: mockFs as any,
276276
logger: mockLogger as any,
277277
homeDir: MOCK_HOME,
@@ -293,7 +293,7 @@ describe('loadUserKnowledgeFiles', () => {
293293
}
294294
const mockLogger = { debug: mock(() => {}) }
295295

296-
const result = await loadUserKnowledgeFiles({
296+
const result = await loadUserKnowledgeFile({
297297
fs: mockFs as any,
298298
logger: mockLogger as any,
299299
homeDir: MOCK_HOME,
@@ -314,7 +314,7 @@ describe('loadUserKnowledgeFiles', () => {
314314
}
315315
const mockLogger = { debug: mock(() => {}) }
316316

317-
const result = await loadUserKnowledgeFiles({
317+
const result = await loadUserKnowledgeFile({
318318
fs: mockFs as any,
319319
logger: mockLogger as any,
320320
homeDir: MOCK_HOME,
@@ -338,7 +338,7 @@ describe('loadUserKnowledgeFiles', () => {
338338
}
339339
const mockLogger = { debug: mock(() => {}) }
340340

341-
const result = await loadUserKnowledgeFiles({
341+
const result = await loadUserKnowledgeFile({
342342
fs: mockFs as any,
343343
logger: mockLogger as any,
344344
homeDir: MOCK_HOME,

0 commit comments

Comments
 (0)