Skip to content

Commit 6938bc9

Browse files
jaggederestjaggederest
committed
Add updated workflow and spec for logging adapter implementation
1 parent e0adfb8 commit 6938bc9

File tree

2 files changed

+295
-0
lines changed

2 files changed

+295
-0
lines changed

‎.claude/scopes/LoggingAdapter.md

Lines changed: 244 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,244 @@
1+
# Logging Adapter Specification
2+
3+
## Purpose & User Problem
4+
5+
The vscode-coder extension currently handles logging through the Storage class, where logging functionality is commingled with storage concerns. The `writeToCoderOutputChannel` method exists in the Storage class alongside file system operations, secret management, and other unrelated functionality. This makes it difficult to:
6+
7+
- **Debug user-reported issues**: When users report bugs, especially client connection issues that don't reproduce locally, there's no consistent way to enable detailed logging to help diagnose the problem
8+
- **Centralize logging logic**: Logging is tied to the Storage class, which also manages secrets, URLs, CLI binaries, and file operations - a clear violation of single responsibility
9+
- **Control verbosity**: Users can't easily enable debug-level logging when needed without modifying code
10+
- **Standardize output**: While `writeToCoderOutputChannel` adds timestamps, other parts use direct `output.appendLine` calls
11+
- **Expand debugging capabilities**: Adding new diagnostic logging for specific subsystems (like client connections) requires passing the Storage instance around
12+
- **Maintain separation of concerns**: The Storage class has become a "god object" that handles too many responsibilities
13+
14+
## Success Criteria
15+
16+
1. **Centralized Logging**: All log output goes through a single adapter
17+
2. **Log Levels**: Support for `debug` and `info` levels
18+
- Additional levels can be added in future iterations
19+
3. **Type Safety**: Fully typed TypeScript implementation
20+
- No `any` types in the logger module or modified files
21+
- No `@ts-ignore` or `eslint-disable` comments
22+
- All types explicitly defined or properly inferred
23+
4. **Testable**:
24+
- Unit tests use `ArrayAdapter` for fast, isolated testing
25+
- ArrayAdapter provides immutable snapshots via `getSnapshot()` to prevent test interference
26+
- Integration tests use `OutputChannelAdapter` to verify VS Code integration
27+
- Test isolation for concurrent test suites:
28+
- `setAdapter()` throws if adapter already set (prevents test conflicts)
29+
- `withAdapter()` provides safe temporary adapter swapping with automatic revert
30+
- Ideal for Vitest's concurrent test execution
31+
- `reset()` and `setAdapter()` methods restricted to test environment (NODE_ENV === 'test')
32+
- `reset()` properly disposes of configuration change listeners to prevent memory leaks
33+
- Methods throw error if called in production for safety
34+
5. **Performance**: Minimal overhead for logging operations
35+
- Measured relative to a no-op adapter baseline
36+
- Debug calls when disabled: < 10% overhead vs no-op
37+
- Debug calls when enabled: < 10x overhead vs no-op (including formatting)
38+
- Info calls: < 5x overhead vs no-op
39+
- Measurement methodology:
40+
- Use `performance.now()` from Node's `perf_hooks`
41+
- Compare against `NoOpAdapter` that does nothing
42+
- Run 10,000 iterations, discard outliers, use median
43+
- CI passes if within percentage thresholds (not absolute times)
44+
6. **Fault Tolerance**: Logger never throws exceptions
45+
- Silently swallows OutputChannel errors (e.g., disposed channel on reload)
46+
- Logging failures must not crash the extension
47+
- No error propagation from the logging subsystem
48+
- Accepts that OutputChannel writes may interleave under concurrent access
49+
7. **Live Configuration**: Monitor `coder.verbose` setting changes without requiring extension restart
50+
- Supports workspace, folder, and global configuration levels
51+
- Uses the most specific configuration available
52+
53+
## Scope & Constraints
54+
55+
### In Scope
56+
- Extract logging functionality from the Storage class into a dedicated logging adapter
57+
- Create a logging adapter/service with support for debug and info levels
58+
- Convert all existing `writeToCoderOutputChannel` and `output.appendLine` calls to use the adapter
59+
- Maintain integration with VS Code's OutputChannel ("Coder")
60+
- OutputChannel is created in `extension.ts` and passed to both Logger and Storage
61+
- Logger uses it for logging via OutputChannelAdapter
62+
- Storage continues to use it for progress reporting (e.g., binary downloads)
63+
- Provide a simple API for logging (e.g., `logger.debug("message")`, `logger.info("message")`)
64+
- Callers only provide the message content, not formatting
65+
- Only string messages accepted (no automatic object serialization)
66+
- Callers must stringify objects/errors before logging (e.g., using template literals)
67+
- Allow runtime configuration of log levels
68+
- Handle all formatting (timestamps, level tags, etc.) within the logger
69+
- Remove only the `writeToCoderOutputChannel` method from Storage class
70+
71+
### Out of Scope
72+
- External logging services integration (future enhancement)
73+
- File-based logging (all logs go to VS Code OutputChannel)
74+
- Log file rotation or persistence
75+
- Structured logging formats (JSON, etc.)
76+
- Performance metrics or telemetry
77+
- Custom UI for viewing logs (uses VS Code's built-in OutputChannel UI)
78+
- Synchronization of concurrent writes (OutputChannel writes may interleave)
79+
- Automatic object/error serialization (callers must convert to strings)
80+
81+
## Technical Considerations
82+
83+
### Architecture
84+
- Singleton pattern for the logger instance
85+
- Interface-based design with pluggable adapters:
86+
- `LogAdapter` interface for output handling
87+
- `OutputChannelAdapter` for VS Code OutputChannel integration
88+
- `ArrayAdapter` for testing (stores logs in memory)
89+
- OutputChannel ownership and lifecycle:
90+
- Created once in `extension.ts` activate method
91+
- Passed to OutputChannelAdapter constructor
92+
- Also passed to Storage for non-logging uses (progress reporting)
93+
- Single channel shared between Logger and Storage
94+
- Note: VS Code OutputChannel is async; concurrent writes may interleave
95+
- This is acceptable for debugging/diagnostic purposes
96+
- In browser/web extensions, OutputChannel maps to in-memory buffer (no file I/O)
97+
- Configuration through VS Code settings:
98+
- `coder.verbose`: boolean setting to enable debug logging (default: false)
99+
- When true: shows debug level logs
100+
- When false: shows info level and above only
101+
- Respects workspace folder-specific settings (uses most specific configuration)
102+
- Configuration monitoring using `vscode.workspace.onDidChangeConfiguration`
103+
- Only responds to changes in `coder.verbose` specifically
104+
- Ignores all other configuration changes to avoid unnecessary processing
105+
- Updates when workspace folders are added/removed or active editor changes
106+
- **Centralized formatting**: All log formatting (timestamps, level tags, source location) happens within the logger implementation, not at call sites
107+
108+
### API Design
109+
```typescript
110+
interface LogAdapter {
111+
write(message: string): void
112+
clear(): void
113+
}
114+
115+
interface Logger {
116+
log(message: string, severity?: LogLevel): void // Core method, defaults to INFO
117+
debug(message: string): void // String only - no object serialization
118+
info(message: string): void // String only - no object serialization
119+
setLevel(level: LogLevel): void
120+
setAdapter(adapter: LogAdapter): void // For testing only - throws if adapter already set
121+
withAdapter<T>(adapter: LogAdapter, fn: () => T): T // Safe temporary adapter swap
122+
reset(): void // For testing only - throws if NODE_ENV !== 'test', disposes listeners
123+
}
124+
125+
enum LogLevel {
126+
DEBUG = 0,
127+
INFO = 1,
128+
NONE = 2 // Disables all logging
129+
}
130+
131+
// Example implementations
132+
class OutputChannelAdapter implements LogAdapter {
133+
constructor(private outputChannel: vscode.OutputChannel) {}
134+
write(message: string): void {
135+
try {
136+
this.outputChannel.appendLine(message)
137+
} catch {
138+
// Silently ignore - channel may be disposed
139+
}
140+
}
141+
clear(): void {
142+
try {
143+
this.outputChannel.clear()
144+
} catch {
145+
// Silently ignore - channel may be disposed
146+
}
147+
}
148+
}
149+
150+
class ArrayAdapter implements LogAdapter {
151+
private logs: string[] = []
152+
153+
write(message: string): void {
154+
this.logs.push(message)
155+
}
156+
157+
clear(): void {
158+
this.logs = []
159+
}
160+
161+
getSnapshot(): readonly string[] {
162+
return [...this.logs] // Return defensive copy
163+
}
164+
}
165+
166+
class NoOpAdapter implements LogAdapter {
167+
write(message: string): void {
168+
// Intentionally empty - baseline for performance tests
169+
}
170+
171+
clear(): void {
172+
// Intentionally empty - baseline for performance tests
173+
}
174+
}
175+
```
176+
177+
### Log Format
178+
- **Standard format**: `[LEVEL] TIMESTAMP MESSAGE`
179+
- Timestamp in UTC ISO-8601 format (e.g., `2024-01-15T10:30:45.123Z`)
180+
- Example: `[info] 2024-01-15T10:30:45.123Z Starting extension...`
181+
- Example: `[debug] 2024-01-15T10:30:45.456Z Processing file: example.ts`
182+
183+
- **Debug mode enhancement**: When `coder.verbose` is true, debug logs include source location:
184+
```
185+
[debug] 2024-01-15T10:30:45.456Z Processing file: example.ts
186+
at processFile (src/utils/fileHandler.ts:45)
187+
```
188+
- Note: In browser/web extensions, `Error().stack` may be empty, disabling source location
189+
190+
### Implementation Plan
191+
192+
1. Create the logger module at `src/logger.ts` with:
193+
- Singleton pattern implementation
194+
- LogAdapter interface and implementations (OutputChannelAdapter, ArrayAdapter)
195+
- Logger initialization accepts OutputChannel (not created internally)
196+
- Configuration reading from VS Code settings (`coder.verbose`)
197+
- Use `workspace.getConfiguration()` to respect folder-specific settings
198+
- Configuration change listener using `workspace.onDidChangeConfiguration`
199+
- Filter to only handle `coder.verbose` changes using `affectsConfiguration()`
200+
- Re-read configuration on folder/editor changes to respect local settings
201+
- Timestamp formatting in UTC ISO-8601 (using `new Date().toISOString()`) and level prefixes
202+
- Debug mode with source location tracking (file/line info)
203+
- Gracefully handle empty `Error().stack` in browser environments
204+
- Test method guards: `reset()` and `setAdapter()` check `process.env.NODE_ENV === 'test'`
205+
- `setAdapter()` throws if adapter already installed (prevents concurrent test conflicts)
206+
- `withAdapter()` implementation:
207+
```typescript
208+
withAdapter<T>(adapter: LogAdapter, fn: () => T): T {
209+
const previous = this.adapter
210+
this.adapter = adapter
211+
try {
212+
return fn()
213+
} finally {
214+
this.adapter = previous
215+
}
216+
}
217+
```
218+
- `reset()` implementation must dispose configuration listener to prevent memory leaks
219+
2. Create comprehensive tests at `src/logger.test.ts`:
220+
- Unit tests using ArrayAdapter for logic testing
221+
- Separate integration tests for OutputChannelAdapter
222+
- Performance benchmarks:
223+
- Create NoOpAdapter as baseline
224+
- Measure relative performance using `performance.now()`
225+
- Ensure overhead stays within specified percentages
226+
- Test both cold and warm paths
227+
3. Update `extension.ts`:
228+
- Create OutputChannel in activate method
229+
- Initialize Logger with OutputChannel via OutputChannelAdapter
230+
- Continue passing OutputChannel to Storage (for progress reporting)
231+
4. Extract and refactor the existing `writeToCoderOutputChannel` logic from Storage class
232+
5. Remove ONLY the `writeToCoderOutputChannel` method from Storage (keep OutputChannel for other uses)
233+
6. Systematically replace each `writeToCoderOutputChannel` call with appropriate logger methods
234+
7. For `output.appendLine` calls in Storage, evaluate each:
235+
- Logging messages → convert to logger calls
236+
- Progress/status messages → keep as direct OutputChannel calls
237+
8. Verify functionality with existing tests
238+
9. Run linting (`yarn lint`) and ensure code quality
239+
240+
### File Locations
241+
- Logger implementation: `src/logger.ts`
242+
- Tests: `src/logger.test.ts`
243+
- Type definitions included in the logger file
244+

‎CLAUDE.md

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,54 @@
2525
- Error handling: wrap and type errors appropriately
2626
- Use async/await for promises, avoid explicit Promise construction where possible
2727
- Test files must be named `*.test.ts` and use Vitest
28+
29+
## Development Workflow: Spec → Code
30+
31+
THESE INSTRUCTIONS ARE CRITICAL!
32+
33+
They dramatically improve the quality of the work you create.
34+
35+
### Phase 1: Requirements First
36+
37+
When asked to implement any feature or make changes, ALWAYS start by asking:
38+
"Should I create a Spec for this task first?"
39+
40+
IFF user agrees:
41+
42+
- Create a markdown file in `.claude/scopes/FeatureName.md`
43+
- Interview the user to clarify:
44+
- Purpose & user problem
45+
- Success criteria
46+
- Scope & constraints
47+
- Technical considerations
48+
- Out of scope items
49+
50+
### Phase 2: Review & Refine
51+
52+
After drafting the Spec:
53+
54+
- Present it to the user
55+
- Ask: "Does this capture your intent? Any changes needed?"
56+
- Iterate until user approves
57+
- End with: "Spec looks good? Type 'GO!' when ready to implement"
58+
59+
### Phase 3: Implementation
60+
61+
ONLY after user types "GO!" or explicitly approves:
62+
63+
- Begin coding based on the Spec
64+
- Reference the Spec for decisions
65+
- Update Spec if scope changes, but ask user first.
66+
67+
### File Organization
68+
69+
```
70+
71+
.claude/
72+
├── scopes/
73+
│ ├── FeatureName.md # Shared/committed Specs
74+
│ └── OtherFeature.md # Other Specs, for future or past work
75+
76+
```
77+
78+
**Remember: Think first, ask clarifying questions, _then_ code. The Spec is your north star.**

0 commit comments

Comments
 (0)