Skip to content

Commit e4376f9

Browse files
committed
evalbuff: add patterns/discover-before-implement.md (carve: cli-init-command)
1 parent 9e532f3 commit e4376f9

File tree

1 file changed

+69
-10
lines changed

1 file changed

+69
-10
lines changed

docs/patterns/discover-before-implement.md

Lines changed: 69 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,21 @@
22

33
Before implementing ANY new feature, search for existing utilities, constants, and templates in the codebase. Duplicating content that already exists is the most common source of architectural drift.
44

5+
## CRITICAL: Check Test Files First for Expected API Contracts
6+
7+
If a test file already exists for the module you're creating (check `__tests__/` directories), **read it before writing a single line of implementation**. Tests reveal:
8+
- The **exact function name** the codebase expects to export
9+
- The **exact function signature** (arguments and return type)
10+
- The **exact behavior** expected (message format, file creation patterns, etc.)
11+
12+
Example: `cli/src/commands/__tests__/init.test.ts` imports `{ handleInitializationFlowLocally }` from `'../init'`. This means the function MUST be named `handleInitializationFlowLocally`, not `handleInitCommand`.
13+
14+
**Decision tree:**
15+
1. Does `__tests__/[filename].test.ts` exist? → Read it FIRST
16+
2. What does it import? → That's your required export name
17+
3. How does it call the function? → That's your required signature
18+
4. What does it assert? → That's your required behavior
19+
520
## Key Locations to Check in This Codebase
621

722
### Project Root Utilities
@@ -31,25 +46,41 @@ import agentDefinitionSource from '../../../common/src/templates/initial-agents-
3146

3247
## CLI Command Pattern
3348

34-
When a command produces system messages (not sending to the AI), the handler returns `{ postUserMessage }` and the command-registry calls `params.sendMessage({ content, agentMode, postUserMessage })`:
49+
Commands that scaffold files AND show messages use the `postUserMessage` + `sendMessage` pattern:
3550

3651
```typescript
37-
// In command-registry.ts:
52+
// In init.ts - the handler returns { postUserMessage }, doesn't call setMessages directly
53+
export function handleInitializationFlowLocally(): { postUserMessage: PostUserMessageFn } {
54+
const messages: string[] = []
55+
56+
// ... do file operations, push to messages array ...
57+
58+
const postUserMessage: PostUserMessageFn = (prev) => [
59+
...prev,
60+
...messages.map((message) => getSystemMessage(message)),
61+
]
62+
return { postUserMessage }
63+
}
64+
65+
// In command-registry.ts - the command calls sendMessage with postUserMessage
3866
defineCommand({
3967
name: 'init',
4068
handler: async (params) => {
4169
const { postUserMessage } = handleInitializationFlowLocally()
70+
const trimmed = params.inputValue.trim()
71+
params.saveToHistory(trimmed)
72+
clearInput(params)
4273
// Handle streaming/queue state check...
4374
params.sendMessage({
4475
content: trimmed,
4576
agentMode: params.agentMode,
46-
postUserMessage, // ← injected into message, NOT setMessages directly
77+
postUserMessage, // ← injected, NOT calling setMessages directly
4778
})
4879
},
4980
})
5081
```
5182

52-
For commands that only show system messages (no AI response), use `params.setMessages`:
83+
For commands that ONLY show system messages (no AI call), use `params.setMessages` directly:
5384
```typescript
5485
params.setMessages((prev) => postUserMessage(prev))
5586
```
@@ -64,20 +95,46 @@ type PostUserMessageFn = (prev: ChatMessage[]) => ChatMessage[]
6495
6596
Use `getSystemMessage(text)` from `cli/src/utils/message-history.ts` to create each message.
6697
98+
## Idempotent Initialization Pattern
99+
100+
When creating files/directories, check each item individually (not the whole directory at once):
101+
102+
```typescript
103+
// DO - check each file/dir independently, allowing partial init
104+
if (existsSync(knowledgePath)) {
105+
messages.push(`📋 \`knowledge.md\` already exists.`)
106+
} else {
107+
writeFileSync(knowledgePath, INITIAL_KNOWLEDGE_FILE)
108+
messages.push(`✅ Created \`knowledge.md\``)
109+
}
110+
111+
// DON'T - bail out early if parent dir exists
112+
if (existsSync(agentsDir)) {
113+
return // WRONG: user may be missing sub-items
114+
}
115+
```
116+
67117
## Checklist Before Writing New Code
68118

69-
1. **Is there a constant for this?** Search `common/src/constants/` first
70-
2. **Is there a utility for this path operation?** Check `cli/src/project-files.ts`
71-
3. **Does a template file already exist?** Check `common/src/templates/`
72-
4. **Should I track analytics?** Most user-facing actions should call `trackEvent()`
73-
5. **What is the naming convention?** Look at 2-3 existing similar handlers (e.g., `handleHelpCommand`, `handleUsageCommand`) before naming your function
119+
1. **Does a test file already exist?** Check `__tests__/[filename].test.ts` — read it FIRST for expected function name and signature
120+
2. **Is there a constant for this?** Search `common/src/constants/` first
121+
3. **Is there a utility for this path operation?** Check `cli/src/project-files.ts`
122+
4. **Does a template file already exist?** Check `common/src/templates/`
123+
5. **Should I track analytics?** Most user-facing actions should call `trackEvent()`
124+
6. **What is the naming convention?** Look at 2-3 existing similar handlers before naming your function
74125

75126
## Anti-patterns
76127

128+
**DON'T** ignore existing test files that reveal expected exports:
129+
```typescript
130+
// BAD - test expects handleInitializationFlowLocally but you created:
131+
export async function handleInitCommand(params: RouterParams): Promise<void>
132+
```
133+
77134
**DON'T** hardcode template content as string literals:
78135
```typescript
79136
// BAD - duplicates content that exists in common/src/templates/
80-
const TYPES_AGENT_DEFINITION = `export interface AgentDefinition { ... }`
137+
const AGENT_DEFINITION_TEMPLATE = `export interface AgentDefinition { ... }`
81138
```
82139

83140
**DO** import from the canonical template location:
@@ -98,3 +155,5 @@ const projectRoot = process.cwd()
98155
import { getProjectRoot } from '../project-files'
99156
const projectRoot = getProjectRoot()
100157
```
158+
159+
**DON'T** add scope-creep files not in the requirements — implement exactly what tests and ground truth specify.

0 commit comments

Comments
 (0)