Commit ff7a786
feat(enricher): add P4.1 streaming implementation with E2E tests (#57)
* docs: add Phase 4.1 Streaming design document
Design P4.1: Real-time streaming support for LLM enrichment
Key features:
- Option pattern for Provider interface (WithStreamingFunc)
- StreamWriter for thread-safe output with batch prefix
- Concurrent processing with mutex-protected output
- CLI flag --no-stream to disable streaming (enabled by default)
Signed-off-by: spencercjh <spencercjh@gmail.com>
* docs: add P4.1 Streaming implementation plan
8 tasks with TDD approach:
1. Provider interface refactoring (Option pattern)
2. StreamWriter implementation
3. Provider streaming implementation
4. BatchProcessor streaming integration
5. Enricher streaming support
6. CLI integration (--no-stream flag)
7. Integration testing
8. Final verification
Signed-off-by: spencercjh <spencercjh@gmail.com>
* feat(provider): add Option pattern for streaming support
- Add GenerateOptions, Option, WithStreamingFunc, applyOptions
- Update Provider interface to accept ...Option
- Update all provider implementations with new signature (streaming logic in next commit)
Signed-off-by: spencercjh <spencercjh@gmail.com>
* feat(processor): add StreamWriter for thread-safe streaming output
- StreamWriter with mutex-protected writes
- WriteWithPrefix adds batch type prefix
- Tested for concurrent access safety
Signed-off-by: spencercjh <spencercjh@gmail.com>
* feat(provider): add streaming support to all providers
- Replace GenerateFromSinglePrompt with GenerateContent
- Pass StreamingFunc to langchaingo via llms.WithStreamingFunc
- Return content from response.Choices for non-streaming callers
Signed-off-by: spencercjh <spencercjh@gmail.com>
* feat(processor): integrate streaming into BatchProcessor
- Add StreamWriter field and WithStreamWriter option
- Pass streaming callback to provider with batch type prefix
- Update NewBatchProcessor to accept functional options
Task 4 of P4.1 streaming implementation
Signed-off-by: spencercjh <spencercjh@gmail.com>
* feat(enricher): add Stream option to EnrichOptions
- Add Stream (bool) and Writer (io.Writer) to EnrichOptions
- Create StreamWriter when streaming is enabled
- Default: streaming enabled to os.Stdout
Signed-off-by: spencercjh <spencercjh@gmail.com>
* feat(cli): add --no-stream flag to enrich command
- Streaming enabled by default
- Use --no-stream to disable and wait for complete response
Signed-off-by: spencercjh <spencercjh@gmail.com>
* test(enricher): add streaming integration test
- Test that streaming callback is invoked
- Test that StreamWriter output contains batch prefix [api]
Signed-off-by: spencercjh <spencercjh@gmail.com>
* style(enricher): fix whitespace alignment
Signed-off-by: spencercjh <spencercjh@gmail.com>
* fix(batch): update streaming function to ignore context parameter
Signed-off-by: spencercjh <spencercjh@gmail.com>
* fix(processor): add nil check to NewStreamWriter
Prevent runtime panic by validating writer parameter is not nil.
Add test to verify panic behavior with nil writer.
Signed-off-by: spencercjh <spencercjh@gmail.com>
* test(enricher): add test for Stream: false path
Add TestEnricher_WithStreamingDisabled to verify that no streaming
callback is passed to the provider when Stream option is disabled.
Signed-off-by: spencercjh <spencercjh@gmail.com>
* test(processor): add streaming callback verification in batch tests
Add TestBatchProcessor_ProcessBatch_WithStreaming to verify that
streaming callback is properly invoked when StreamWriter is configured.
Signed-off-by: spencercjh <spencercjh@gmail.com>
* feat(processor): add chunk buffering to StreamWriter
- Add buffer accumulation for streaming chunks
- Auto-flush on newline, buffer threshold, or prefix change
- Add WithFlushThreshold option for configurable buffering
- Add Flush method for explicit buffer clearing
- Update enricher to flush StreamWriter after processing
- Add tests for buffering behavior
Signed-off-by: spencercjh <spencercjh@gmail.com>
* feat(processor): add streaming debug logging and metrics
- Add StreamWriterMetrics struct to track streaming statistics
- Add GetMetrics() method for retrieving metrics
- Add WithDebug option for enabling debug logging
- Add WithFlushThreshold option for configurable buffering
- Track total chunks, bytes, flushes, and unique prefixes
- Debug logging shows chunk details and flush events
Signed-off-by: spencercjh <spencercjh@gmail.com>
* style: fix formatting issues from golangci-lint
Signed-off-by: spencercjh <spencercjh@gmail.com>
* docs: clarify make verify usage in CLAUDE.md
Explain that make verify checks for uncommitted changes and should only
be used after committing. Recommend individual commands (make fmt, lint,
test) before committing to avoid false failures from git diff check.
Signed-off-by: spencercjh <spencercjh@gmail.com>
* test(e2e): add conditional enrich E2E tests with streaming verification
- Add skipIfNoConfig helper to skip tests without valid E2E config
- Check for .spec-forge.e2e.local.yaml or .spec-forge.local.yaml
- Verify API key environment variable is set before running
- Test streaming output with prefix verification ([api], [schema], [param])
- Test --no-stream flag disables streaming prefixes
- Test local config file loading mechanism
- Add .spec-forge.e2e.example.yaml as configuration template
- Update .gitignore to exclude E2E local configs
Signed-off-by: spencercjh <spencercjh@gmail.com>
* test(e2e): add conditional enrich E2E tests with streaming verification
- Add loadE2EConfig helper to load config from integration-tests/.spec-forge.e2e.local.yaml
- Skip tests gracefully if no valid config found (file missing or API key env not set)
- Add TestE2E_Enrich_NoStreamFlag to verify --no-stream disables streaming prefixes
- Add TestE2E_Enrich_WithStreaming to test real LLM enrichment with streaming output
- Add TestE2E_Enrich_WithLocalConfig to test local config file loading
- Add .spec-forge.e2e.example.yaml as configuration template
- Update .gitignore to exclude integration-tests/.spec-forge.e2e.local.yaml
- Document E2E enrich test setup in integration-tests/README.md
Signed-off-by: spencercjh <spencercjh@gmail.com>
* test(e2e): fix enrich E2E tests for streaming verification
- Remove streaming prefix check in stdout (StreamWriter writes to os.Stdout directly, not Cobra's buffer)
- Remove TestE2E_Enrich_WithLocalConfig (tests Viper config loading, not enrich logic)
- Verify enrichment by checking the output spec file contains descriptions
- Update README to document only the two streaming-related tests
- All tests pass:
- TestE2E_Enrich_Help: PASS
- TestE2E_Enrich_MissingArgs: PASS
- TestE2E_Enrich_NonExistentFile: PASS
- TestE2E_Enrich_NoStreamFlag: SKIP (requires config)
- TestE2E_Enrich_WithStreaming: PASS (with config) / SKIP (without config)
Signed-off-by: spencercjh <spencercjh@gmail.com>
* fix(provider): satisfy goconst in anthropic and custom providers
Agent-Logs-Url: https://github.com/spencercjh/spec-forge/sessions/be20b0d1-0dc2-45c4-adc4-b2bec21a9a95
Co-authored-by: spencercjh <29922079+spencercjh@users.noreply.github.com>
* fix(review): address PR review feedback for P4.1 streaming
- Change EnrichOptions.Stream to *bool (tri-state) to avoid backwards
compatibility issues with zero value of bool (false). Now nil means
use default (true), explicit false/true overrides.
- Add error return when LLM providers return empty choices to prevent
silent enrichment failures (OpenAI, Anthropic, Ollama, Custom).
- Fix comment in batch.go to reflect lowercase prefix values.
- Remove stale TestE2E_Enrich_WithLocalConfig entry from README table.
- Make E2E test assertion language-agnostic (check for non-empty
descriptions instead of specific Chinese text).
Signed-off-by: spencercjh <spencercjh@gmail.com>
* fix(review): address additional PR review feedback for P4.1 streaming
Review feedback addressed:
- CustomProvider: use p.name instead of hardcoded CustomProviderName in error messages
- StreamWriter: optimize bytes.Contains to bytes.IndexByte for hot path
- E2E tests: handle enrich.enabled=false config, pass --custom-api-key-env for custom provider
- Design doc: fix StreamWriter file path, update Stream to *bool type, correct prefix casing
Fixes review comments #11, #12, #13, #16, #17 on PR #57
Signed-off-by: caijiahao <caijh@inesa.com>
Signed-off-by: spencercjh <spencercjh@gmail.com>
* fix(review): address third round of Copilot review feedback for P4.1
Review feedback addressed:
- factory.go: use AnthropicProviderName and CustomProviderName constants instead of string literals
- design doc: update EnrichOptions summary to reflect actual API (Stream *bool, Writer io.Writer)
- design doc: fix data flow example to use lowercase prefix [api] instead of [API]
- stream_writer.go: validate WithFlushThreshold to use DefaultFlushThreshold for zero/negative values
- e2e_enrich_test.go: pass --timeout flag when configured in E2E config
Fixes review comments #21-26 on PR #57
Signed-off-by: caijiahao <caijh@inesa.com>
Signed-off-by: spencercjh <spencercjh@gmail.com>
* fix(review): address fourth round of Copilot review feedback
Review feedback addressed:
- e2e_enrich_test.go: change Enabled to *bool to distinguish between
"not set" (nil, run tests) and "explicitly false" (skip tests)
- README.md: update skip message path to match actual output
- batch.go: add Flush() call after each LLM call to ensure buffered
streaming output is visible for short responses
Note: #30 (streaming to os.Stdout) is intentional design for real-time
terminal feedback and not changed.
Fixes review comments #27-29 on PR #57
Signed-off-by: caijiahao <caijh@inesa.com>
Signed-off-by: spencercjh <spencercjh@gmail.com>
* fix(review): address fifth round of Copilot review feedback
Review feedback addressed:
- e2e_enrich_test.go: match API key env var logic to CLI behavior
(OPENAI_API_KEY, ANTHROPIC_API_KEY, or LLM_API_KEY for custom)
- e2e_enrich_test.go: use YAML parsing for provider-agnostic assertions
instead of language-specific substring matching
- design doc: update output example to show actual raw JSON streaming
behavior instead of hypothetical human-readable text
Note: #32 (streaming writes to os.Stdout) is intentional design for
real-time terminal feedback - already addressed in #30 response.
Fixes review comments #31, #33, #34 on PR #57
Signed-off-by: caijiahao <caijh@inesa.com>
Signed-off-by: spencercjh <spencercjh@gmail.com>
* fix(enricher,stream_writer): improve error handling and metrics updates
Signed-off-by: spencercjh <spencercjh@gmail.com>
* feat(batch): add streaming support and improve processing logic
Signed-off-by: spencercjh <spencercjh@gmail.com>
* fix(review): address Copilot review feedback - streaming, concurrency, docs
- Remove redundant atomic ops in StreamWriter, use mutex-only sync
- Handle short writes in flushLocked()
- Change final Flush error to warning (streaming is ancillary)
- Streaming mode processes batches sequentially for readable output
- --no-stream enables concurrent processing (--concurrency applies)
- Update flag descriptions, config comments, design doc, CLAUDE.md
Signed-off-by: spencercjh <spencercjh@gmail.com>
---------
Signed-off-by: spencercjh <spencercjh@gmail.com>
Signed-off-by: caijiahao <caijh@inesa.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: spencercjh <29922079+spencercjh@users.noreply.github.com>1 parent c3e3584 commit ff7a786
23 files changed
Lines changed: 2589 additions & 169 deletions
File tree
- cmd
- docs/plans
- integration-tests
- internal/enricher
- processor
- provider
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
43 | 43 | | |
44 | 44 | | |
45 | 45 | | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
46 | 49 | | |
47 | 50 | | |
48 | 51 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
30 | 34 | | |
31 | 35 | | |
32 | 36 | | |
| |||
239 | 243 | | |
240 | 244 | | |
241 | 245 | | |
242 | | - | |
| 246 | + | |
243 | 247 | | |
244 | 248 | | |
245 | 249 | | |
246 | 250 | | |
247 | 251 | | |
248 | 252 | | |
249 | 253 | | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
250 | 264 | | |
251 | 265 | | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
252 | 270 | | |
253 | 271 | | |
254 | 272 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
67 | 67 | | |
68 | 68 | | |
69 | 69 | | |
| 70 | + | |
| 71 | + | |
70 | 72 | | |
71 | 73 | | |
72 | 74 | | |
| |||
153 | 155 | | |
154 | 156 | | |
155 | 157 | | |
156 | | - | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
157 | 163 | | |
158 | 164 | | |
159 | 165 | | |
| |||
282 | 288 | | |
283 | 289 | | |
284 | 290 | | |
285 | | - | |
| 291 | + | |
286 | 292 | | |
287 | 293 | | |
288 | 294 | | |
| 295 | + | |
289 | 296 | | |
290 | 297 | | |
291 | 298 | | |
| |||
300 | 307 | | |
301 | 308 | | |
302 | 309 | | |
| 310 | + | |
303 | 311 | | |
304 | 312 | | |
305 | 313 | | |
| |||
309 | 317 | | |
310 | 318 | | |
311 | 319 | | |
312 | | - | |
| 320 | + | |
313 | 321 | | |
314 | 322 | | |
315 | 323 | | |
| 324 | + | |
316 | 325 | | |
File renamed without changes.
0 commit comments