Skip to content

Commit 7f7c18f

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

File tree

8 files changed

+537
-486
lines changed

8 files changed

+537
-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__/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,

sdk/src/impl/file-tree-builder.ts

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
/**
2+
* File tree building and project indexing utilities.
3+
*/
4+
5+
import { getFileTokenScores } from '@codebuff/code-map/parse'
6+
7+
import type { Logger } from '@codebuff/common/types/contracts/logger'
8+
import type { FileTreeNode } from '@codebuff/common/util/file'
9+
10+
/**
11+
* Builds a hierarchical file tree from a flat list of file paths
12+
*/
13+
export function buildFileTree(filePaths: string[]): FileTreeNode[] {
14+
const tree: Record<string, FileTreeNode> = {}
15+
16+
for (const filePath of filePaths) {
17+
const parts = filePath.split('/')
18+
19+
for (let i = 0; i < parts.length; i++) {
20+
const currentPath = parts.slice(0, i + 1).join('/')
21+
const isFile = i === parts.length - 1
22+
23+
if (!tree[currentPath]) {
24+
tree[currentPath] = {
25+
name: parts[i],
26+
type: isFile ? 'file' : 'directory',
27+
filePath: currentPath,
28+
children: isFile ? undefined : [],
29+
}
30+
}
31+
}
32+
}
33+
34+
const rootNodes: FileTreeNode[] = []
35+
const processed = new Set<string>()
36+
37+
for (const [path, node] of Object.entries(tree)) {
38+
if (processed.has(path)) continue
39+
40+
const parentPath = path.substring(0, path.lastIndexOf('/'))
41+
if (parentPath && tree[parentPath]) {
42+
const parent = tree[parentPath]
43+
if (
44+
parent.children &&
45+
!parent.children.some((child) => child.filePath === path)
46+
) {
47+
parent.children.push(node)
48+
}
49+
} else {
50+
rootNodes.push(node)
51+
}
52+
processed.add(path)
53+
}
54+
55+
function sortNodes(nodes: FileTreeNode[]): void {
56+
nodes.sort((a, b) => {
57+
if (a.type !== b.type) {
58+
return a.type === 'directory' ? -1 : 1
59+
}
60+
return a.name.localeCompare(b.name)
61+
})
62+
63+
for (const node of nodes) {
64+
if (node.children) {
65+
sortNodes(node.children)
66+
}
67+
}
68+
}
69+
70+
sortNodes(rootNodes)
71+
return rootNodes
72+
}
73+
74+
/**
75+
* Computes project file indexes (file tree and token scores)
76+
*/
77+
export async function computeProjectIndex(
78+
cwd: string,
79+
projectFiles: Record<string, string>,
80+
logger?: Logger,
81+
): Promise<{
82+
fileTree: FileTreeNode[]
83+
fileTokenScores: Record<string, Record<string, number>>
84+
tokenCallers: Record<string, Record<string, string[]>>
85+
}> {
86+
const filePaths = Object.keys(projectFiles).sort()
87+
const fileTree = buildFileTree(filePaths)
88+
let fileTokenScores: Record<string, Record<string, number>> = {}
89+
let tokenCallers: Record<string, Record<string, string[]>> = {}
90+
91+
if (filePaths.length > 0) {
92+
try {
93+
const tokenData = await getFileTokenScores(
94+
cwd,
95+
filePaths,
96+
(filePath: string) => projectFiles[filePath] ?? null,
97+
)
98+
fileTokenScores = tokenData.tokenScores
99+
tokenCallers = tokenData.tokenCallers
100+
} catch (error) {
101+
logger?.warn?.({ error }, 'Failed to generate parsed symbol scores')
102+
}
103+
}
104+
105+
return { fileTree, fileTokenScores, tokenCallers }
106+
}

sdk/src/impl/git-operations.ts

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/**
2+
* Git operations for retrieving repository state.
3+
*/
4+
5+
import type { Logger } from '@codebuff/common/types/contracts/logger'
6+
import type { CodebuffSpawn } from '@codebuff/common/types/spawn'
7+
8+
function childProcessToPromise(
9+
proc: ReturnType<CodebuffSpawn>,
10+
): Promise<{ stdout: string; stderr: string }> {
11+
return new Promise((resolve, reject) => {
12+
let stdout = ''
13+
let stderr = ''
14+
15+
proc.stdout?.on('data', (data: Buffer) => {
16+
stdout += data.toString()
17+
})
18+
19+
proc.stderr?.on('data', (data: Buffer) => {
20+
stderr += data.toString()
21+
})
22+
23+
proc.on('close', (code: number | null) => {
24+
if (code === 0) {
25+
resolve({ stdout, stderr })
26+
} else {
27+
reject(new Error(`Command exited with code ${code}`))
28+
}
29+
})
30+
31+
proc.on('error', reject)
32+
})
33+
}
34+
35+
export async function getGitChanges(params: {
36+
cwd: string
37+
spawn: CodebuffSpawn
38+
logger: Logger
39+
}): Promise<{
40+
status: string
41+
diff: string
42+
diffCached: string
43+
lastCommitMessages: string
44+
}> {
45+
const { cwd, spawn, logger } = params
46+
47+
const status = childProcessToPromise(spawn('git', ['status'], { cwd }))
48+
.then(({ stdout }) => stdout)
49+
.catch((error) => {
50+
logger.debug?.({ error }, 'Failed to get git status')
51+
return ''
52+
})
53+
54+
const diff = childProcessToPromise(spawn('git', ['diff'], { cwd }))
55+
.then(({ stdout }) => stdout)
56+
.catch((error) => {
57+
logger.debug?.({ error }, 'Failed to get git diff')
58+
return ''
59+
})
60+
61+
const diffCached = childProcessToPromise(
62+
spawn('git', ['diff', '--cached'], { cwd }),
63+
)
64+
.then(({ stdout }) => stdout)
65+
.catch((error) => {
66+
logger.debug?.({ error }, 'Failed to get git diff --cached')
67+
return ''
68+
})
69+
70+
const lastCommitMessages = childProcessToPromise(
71+
spawn('git', ['log', '-n', '10', '--pretty=format:%s'], { cwd }),
72+
)
73+
.then(({ stdout }) => stdout.trim())
74+
.catch((error) => {
75+
logger.debug?.({ error }, 'Failed to get lastCommitMessages')
76+
return ''
77+
})
78+
79+
return {
80+
status: await status,
81+
diff: await diff,
82+
diffCached: await diffCached,
83+
lastCommitMessages: await lastCommitMessages,
84+
}
85+
}

0 commit comments

Comments
 (0)