feat: Implement new unified configuration and migration tools

[pflooky]

• Introduced unified configuration schema and migration scripts to facilitate transitions from legacy formats.
• Added comprehensive documentation for migration processes and examples for new configurations.
• Enhanced memory profiling tools with new scenarios and scripts for performance analysis.
• Updated various documentation sections to reflect recent changes in configuration and validation capabilities.
• Removed obsolete sample plans to streamline the testing framework.

This update aims to improve user experience during configuration migrations and enhance the overall performance profiling capabilities of the application.

---

Note

Introduces unified YAML config support, richer generators, and streaming performance tracking, plus manual testing and docs updates.

• Adds unified YAML models/parser with comprehensive integration tests; expands docs (CLAUDE.md/README) and bumps Docker tag to 0.19.0
• Implements streaming config (application.conf), end-to-end performance metrics (batch + streaming), and integrates metrics into generation pipeline, sink routing, HTML/CSV exporters
• Extends FieldBuilder/GeneratorBuilder with array helpers (fixed/unique/oneOf/weighted/empty), string/date/time/sequential/conditional APIs; improves regex SQL generation and deterministic randomness; supports weekend exclusion for dates
• Adds manualTest Gradle source set/task for external dependency tests; minor test adjustments and config wiring

^{Written by Cursor Bugbot for commit 9572b87. This will update automatically on new commits. Configure here.}

[claude]

Pull Request Review: Unified Configuration and Migration Tools

This is a major feature PR introducing unified YAML configuration format, streaming enhancements, and performance improvements. Overall, this is high-quality work with excellent test coverage.

Strengths

1. Excellent Architecture and Design

• Clean separation of concerns: External API models are properly separated from internal execution models
• Format detection: UnifiedConfigDetector provides intelligent format detection with clear fallback logic
• Conversion pipeline: UnifiedConfigConverter follows systematic conversion with proper environment variable resolution
• Builder pattern: ConditionalBuilder provides type-safe CASE WHEN expressions with fluent API

2. Strong Test Coverage

• 29 test files added covering unit, integration, and manual tests
• Integration tests: UnifiedYamlIntegrationTest tests end-to-end YAML execution
• Manual test framework: Well-designed ManualTestSuite for external dependencies
• Helper method tests: Comprehensive coverage of array/date/string/sequential helpers

3. Performance and Memory Optimizations

• BoundedResponseBuffer: Excellent LRU-based bounded buffer preventing OOM in streaming scenarios
• Thread-safe with ConcurrentLinkedQueue and AtomicLong counters
• StreamingMetrics: Rich metrics model with throughput analysis, percentiles, and pattern validation

4. Documentation

• CLAUDE.md updated with comprehensive guidance on unified format, manual tests, and memory profiling
• Inline documentation: Models have clear JavaDoc explaining purpose and usage

Issues and Recommendations

1. Security: Password Handling (Medium Priority)

Location: UnifiedConfigConverter.scala:68-69, 79-80, 97, 104

Issue: Passwords are resolved from environment variables but not consistently masked in logs.

Recommendation: Add debug logging guards to prevent password leakage and document that users should use env var syntax for passwords.

2. Error Handling: Silent Failures (Medium Priority)

Location: UnifiedConfigConverter.scala:410-478

Issue: convertFieldValidation has a catch-all that logs a warning but returns NullFieldValidation(false) for unknown validation types.

Recommendation: Throw an exception for unknown validation types to fail fast during configuration parsing.

3. Type Safety: Reflection Usage (Low Priority)

Location: StreamingMetrics.scala:271-275

Issue: Uses reflection to extract batch data to avoid circular dependency

Recommendation: Consider extracting a trait/interface to avoid reflection and add error handling for missing methods.

4. Performance: filterKeys Deprecation (Low Priority)

Location: UnifiedConfigConverter.scala:125

Issue: filterKeys is deprecated in Scala 2.13+

Recommendation: Use .filter with tuple destructuring for future-proofing

Test Coverage Analysis

Coverage Summary:

• Unit tests: Comprehensive coverage of helpers, validators, parsers
• Integration tests: End-to-end YAML parsing and execution
• Manual tests: Framework for external dependencies
• Performance tests: Removed ForeignKeyUtilPerformanceTest (440 lines deleted) - verify this did not lose critical benchmarks

Recommendations:

1. Add negative test cases: Test invalid YAML, malformed connections, conflicting foreign keys
2. Add edge case tests: Empty data sources, missing required fields, circular foreign keys
3. Add performance regression tests: Baseline metrics for streaming throughput

Final Verdict

Recommendation: Approve with minor improvements

This is excellent work with:

• Strong architecture and design
• Comprehensive test coverage
• Critical performance optimizations
• Good documentation

The identified issues are minor and can be addressed in follow-up PRs if needed. The security and error handling recommendations should be prioritized.

Pre-merge Checklist:

• Review password logging in debug mode
• Add validation for unknown validation types
• Verify performance test removal did not lose critical benchmarks
• Add negative test cases for invalid YAML
• Document migration path from legacy format

Great work on this major feature addition!

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

This PR is being reviewed by Cursor Bugbot

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

[claude]

Pull Request Review: Unified Configuration and Performance Enhancements

This is a substantial PR with 155 files changed (+22,047/-3,246 lines) introducing unified YAML configuration, streaming improvements, and enhanced generator APIs. Overall, the implementation is well-structured with good test coverage, but there are several areas requiring attention.

---

🟢 Strengths

1. Architecture and Design

• Clean separation of concerns: External API models (UnifiedConfig) properly separated from internal models
• Well-designed converter pattern: UnifiedConfigConverter provides clean bridge between YAML and internal execution
• Memory optimization: BoundedResponseBuffer and BatchTimestampTracker prevent OOM in large-scale streaming
• Backwards compatibility: Legacy format still supported with migration path

2. Code Quality

• Comprehensive test coverage: Integration tests, unit tests, and new manual test framework
• Good documentation: CLAUDE.md updated with clear guidance on new features
• Type safety: Jackson annotations, case classes with proper defaults, Option types instead of nulls
• Builder pattern: Consistent use of builders (e.g., ConditionalBuilder) for clean API

3. Performance Features

• Memory-efficient streaming: On-the-fly emit time calculation (O(1) vs O(n) memory)
• Bounded buffers: Response buffer with LRU eviction prevents unbounded growth
• Batch aggregation: Timestamp tracking with configurable windows reduces overhead
• Dynamic throttling: Pattern-based rate control with Pekko scheduler

---

🟡 Issues Requiring Attention

1. Security Concerns

Password Handling in Logs (UnifiedConfigConverter.scala:68-80)

• Issue: Passwords may be logged in debug statements. Connection configs with passwords could leak in logs
• Recommendation: Add sanitization for sensitive fields in logging, consider using toString overrides to mask passwords

Environment Variable Substitution (UnifiedConfigConverter.scala:142-147)

• Issue: When env var not found, returns literal placeholder which may confuse users
• Recommendation: Log warning when environment variable is missing, consider failing fast for critical configs

2. Potential Bugs

Timeout Calculation Edge Case (PekkoStreamingSinkWriter.scala:293-298)

• Issue: When rate <= 0, timeout is only 10 seconds regardless of record count
• Impact: Large datasets with invalid rates will timeout prematurely
• Recommendation: Use record count to calculate reasonable timeout even when rate is invalid

Missing Null Check in Conditional Builder (ConditionalBuilder.scala:92-95)

• Issue: If value is null, v.toString will throw NPE
• Recommendation: Add null case handling

3. Performance Considerations

String Concatenation in Hot Path (PekkoStreamingSinkWriter.scala:170)

• Issue: String formatting in every progress log (even with threshold checks)
• Recommendation: Move formatting inside the if block to avoid unnecessary allocations

4. Code Quality Issues

Inconsistent Error Messages

• Example: data-source vs data-source-name
• Recommendation: Standardize on one format (prefer data-source)

Test Organization

• Issue: Some test files are very long (600+ lines)
• Recommendation: Consider splitting UnifiedYamlIntegrationTest into focused test suites

5. Missing Test Coverage

Based on the changes, these areas may need additional tests:

1. Error cases in UnifiedConfigConverter (invalid connection types, missing required fields, malformed foreign key definitions)
2. ConditionalBuilder edge cases (null values, empty condition lists, very long conditional chains)
3. Memory profiling scenarios (verify all 6 scenarios are covered with documented expected memory profiles)

---

🔴 Critical Issues

1. Actor System Lifecycle Management (PekkoStreamingSinkWriter.scala:426-436)

• Issue: Swallows exceptions during actor system termination in finally block
• Impact: Resource leaks may go unnoticed
• Recommendation: Rethrow exceptions after logging (or use NonFatal pattern), add metrics/monitoring for failed terminations

2. Data Validation Gap (UnifiedConfigConverter.scala:194-206)

• Issue: No validation that rate, duration, and pattern are mutually compatible
• Example: User could specify both rate: 100 and pattern: {type: ramp} - unclear which takes precedence
• Recommendation: Add validation in converter to fail fast on conflicting configurations

3. Foreign Key Cardinality Handling (UnifiedConfigConverter.scala:278-280)

• Issue: No validation that min <= max, ratio is valid (0-1), or distribution is recognized
• Recommendation: Add validation with clear error messages

---

📋 Recommendations

High Priority

1. Add security audit for password logging
2. Validate configuration conflicts (rate/duration/pattern)
3. Add null safety to ConditionalBuilder.formatValue
4. Fix timeout calculation for invalid rate scenarios
5. Improve actor system error handling

Medium Priority

6. Add validation tests for UnifiedConfigConverter error cases
7. Standardize error message formatting
8. Document memory buffer sizing rationale
9. Add integration test for shared actor system reuse
10. Performance test for very large conditional chains

Low Priority

11. Split large test files for better organization
12. Add examples for all new builder APIs
13. Consider metrics collection for streaming performance
14. Document migration path more clearly in README

---

🎯 Performance Validation

The memory profiling infrastructure is excellent (misc/memory-profiling/). Ensure:

1. All 6 scenarios pass CI (baseline, bounded-buffer, high-throughput, large-batch, sustained-load, stress-test)
2. Memory usage stays within expected bounds (document these bounds)
3. No OOM failures in stress test scenario

---

✅ Summary

This is a well-architected PR that significantly enhances the project with:

• Modern unified configuration format
• Memory-efficient streaming
• Rich builder APIs
• Comprehensive testing infrastructure

Recommendation: Approve with minor revisions

The critical issues (security, validation, error handling) should be addressed before merge, but the overall design is solid and the code quality is high. The test coverage is impressive, particularly the manual test framework and memory profiling tooling.

Great work on this substantial feature addition! 🚀

---

Reviewed by: Claude Code
Review Date: 2026-01-18

[cursor]

Cursor Bugbot has reviewed your changes and found 4 potential issues.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

[claude]

Code Review: PR #127

Executive Summary

This PR introduces unified YAML configuration, streaming performance metrics, and enhanced data generation. The code quality is strong with good architectural patterns and comprehensive testing. However, there are several critical issues that must be addressed before merging.

Overall Assessment: ⚠️ Needs Changes - Critical bugs in thread safety, SQL injection, and memory efficiency

---

🔴 Critical Issues (Must Fix Before Merge)

1. Thread Safety Race Condition in BatchTimestampTracker

File: BatchTimestampTracker.scala:53-65

The recordTimestamp() method has a race condition. The increment happens AFTER the flush check, which can cause records to be assigned to the wrong window when multiple threads enter the flush path simultaneously.

Fix: Move currentWindowCount.incrementAndGet() before the flush check.

Test Gap: No multi-threaded tests exist for this class.

2. SQL Injection Vulnerability in ConditionalBuilder

File: ConditionalBuilder.scala:92-95

String values are embedded into SQL without escaping single quotes. Attack: when("status").equalTo("'; DROP TABLE users; --")

Fix: Escape single quotes properly: s.replace("'", "''")

3. Memory Leak in StreamingMetrics.fromBatches

File: StreamingMetrics.scala:271-288

The method creates O(records) synthetic timestamps (~80MB for 10M records), defeating the batch aggregation optimization.

Fix: Calculate metrics directly from batch aggregates without expanding timestamps.

---

🟡 High Priority Issues

4. Silent Data Loss in StreamingDataSource:88-89

Rows are silently dropped with only a warning when queue backpressure causes timeout. Fix: Throw exception.

5. Null Safety in KafkaSinkProcessor:166

other.toString can throw NPE. Fix: Add null check.

6. Deprecated API in UnifiedConfigConverter:124

filterKeys is deprecated. Fix: Use filter instead.

---

🟢 Positive Highlights

• ✅ Excellent architecture with bounded buffers and batch aggregation
• ✅ Strong test coverage (576-line integration test, manual tests, performance tests)
• ✅ Good Scala practices (Option types, immutability, pattern matching)
• ✅ Comprehensive documentation

---

📝 Recommendations

Before Merge (Blocking)

1. Fix SQL injection in ConditionalBuilder
2. Fix thread safety in BatchTimestampTracker
3. Fix memory leak in StreamingMetrics.fromBatches
4. Add concurrent test coverage

High Priority

5. Handle queue drops as errors
6. Add null safety to KafkaSinkProcessor
7. Replace deprecated filterKeys

---

Summary

This is a high-quality contribution with thoughtful design. The critical issues are fixable and don't reflect fundamental design flaws. With the fixes above, this will be an excellent addition.

Verdict: ⚠️ Request Changes - Fix critical issues before merge

Great work! 🚀

[cursor]

Cursor Bugbot has reviewed your changes and found 3 potential issues.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}