Add synthetic radar detection generation endpoint

[jonnyspicer]

Summary

• Implements /api/synthetic-detections endpoint for generating realistic radar detections with configurable noise
• Enables testing and validation of passive radar tracking systems with reproducible synthetic data
• Provides frame-based output format compatible with retina-tracker

Key Features

• Configurable Gaussian noise: Realistic measurement errors for delay (km) and Doppler (Hz)
• Missed detections: Simulates detection probability < 1.0 with Bernoulli trials
• False alarms: Poisson-distributed clutter detections in delay-Doppler space
• Reproducible results: Seedable RNG for repeatable test scenarios
• Extended format: Frame-based arrays with parallel ADS-B metadata for ground truth

Implementation Details

• New SyntheticRNG class with uniform, Gaussian, Poisson, and Bernoulli distributions
• Configuration parsing and validation for all noise parameters
• Comprehensive test suite (476 lines) covering statistical properties
• Utility scripts for capturing and processing historical ADS-B snapshots
• Extensive README documentation with API examples

Technical Changes

• Added seedrandom dependency for reproducible random number generation
• Changed docker-compose from network_mode: host to explicit port mapping 49155:49155
• Added data/ directory to .gitignore for snapshot storage

Test Plan

• Run existing test suite (npm test)
• Verify statistical properties of RNG distributions
• Test configuration validation with invalid parameters
• Validate frame generation with various noise levels
• Manual API testing with live ADS-B data
• Integration testing with retina-tracker

🤖 Generated with Claude Code

[claude]

Pull Request Review: Synthetic Radar Detection Generation

Thank you for this comprehensive PR! This is a well-thought-out feature that adds significant testing and validation capabilities. Here is my detailed review:

---

Strengths

Excellent Test Coverage

• 476 lines of tests covering all major functionality
• Statistical validation of RNG distributions (uniform, Gaussian, Poisson, Bernoulli)
• Edge case handling (empty data, missing fields, invalid configs)
• Reproducibility tests for seeded RNG
• This addresses the README stated need for functional tests

Clean Code Architecture

• Well-organized separation of concerns (synthetic.js is pure logic, server endpoint is thin)
• Comprehensive JSDoc documentation with @brief, @param, @return annotations
• Good use of default configuration pattern
• Clear validation with helpful error messages

Thoughtful Feature Design

• Frame-based output compatible with retina-tracker
• Reproducible results via seedable RNG
• Realistic noise modeling (Gaussian, Poisson distributions)
• Parallel ADS-B metadata for ground truth validation

---

Critical Issues Found

1. Potential Performance Issue in Endpoint

Location: src/server.js:291-405

The /api/synthetic-detections endpoint generates frames sequentially in a loop, making multiple blocking HTTP requests. With default settings (10 seconds duration, 500ms intervals = 20 frames), this makes 20 sequential HTTP requests before responding.

Problems:

• Could take 10+ seconds to respond (poor UX)
• May timeout on slow networks
• Could overwhelm tar1090 servers with rapid requests

Recommendation: Add request timeout handling, consider rate limiting, or redesign to stream results.

2. Bug: Inconsistent norm() Function API

Locations: src/server.js:283, 333 vs process_snapshots.js:61

The code uses both array syntax norm([x, y, z]) and object syntax norm({x, y, z}) for the norm() function. This will cause runtime errors if the function doesn't handle both formats.

Recommendation: Check src/node/geometry.js implementation and standardize on one calling convention.

3. Missing Error Handling

Location: src/server.js:339-341

The code only checks if (doppler !== null) but doesn't handle potential exceptions from calculateDopplerFromVelocity(). If this function throws, the entire endpoint crashes.

Recommendation: Add try-catch around the delay-Doppler computation loop.

4. Docker Compose Breaking Change

Location: docker-compose.yml:5-9

Changed from network_mode: host to explicit port mapping 49155:49155.

Problems:

• network_mode: host allows container to access all host network interfaces
• Port mapping isolates the container
• Could break existing deployments that rely on host networking to access tar1090 on localhost

Recommendation: Document this as a BREAKING CHANGE in PR description, explain why necessary, provide migration guidance.

---

Security Concerns

5. DoS Risk: Unbounded Frame Generation

Location: src/server.js:288-289

A malicious user could request duration=100000&frame_interval=1 creating 100 million frames, exhausting memory and CPU.

Recommendation:
Add a MAX_FRAMES limit (e.g., 1000) and reject unreasonable requests.

6. SSRF Risk: External URL Validation

Location: src/server.js:246, 261-273

The endpoint accepts arbitrary server URLs and makes HTTP requests. While there is protocol validation, there is no protection against private IP ranges (192.168.x.x, 10.x.x.x, 127.0.0.1) or cloud metadata endpoints (169.254.169.254).

Recommendation: Add IP range validation to prevent SSRF attacks (check if this exists in original /api/dd endpoint for consistency).

---

Code Quality Suggestions

7. Code Duplication

Location: src/server.js:357-404

The synthetic frame generation logic in the endpoint duplicates code from generateSyntheticFrame() in synthetic.js.

Recommendation: Refactor to use the existing function consistently.

8. Missing Input Validation

Location: src/node/synthetic.js:72-113

parseSyntheticConfig() uses parseFloat() and parseInt() without checking for NaN results. If query.noise_delay = "abc", it becomes NaN which passes parsing but fails validation later.

Recommendation: Validate during parsing or return errors immediately.

9. Hardcoded Configuration

Location: process_snapshots.js:25-49

The radar configuration is hardcoded (specific to 150 Mississippi / KSCZ-LD), limiting reusability.

Recommendation: Accept configuration via command-line arguments or config file.

---

Documentation & Testing

10. Missing API Response Examples

The README shows URLs but not actual JSON responses. Include example output.

11. Integration Testing Checklist

The PR description shows unchecked items:

• Manual API testing with live ADS-B data
• Integration testing with retina-tracker

Recommendation: Complete these before merging, or create follow-up issues.

12. Add Integration Tests

Current tests are unit tests. Consider adding end-to-end tests of the endpoint with mocked HTTP responses.

---

Summary

Category	Count	Severity
Critical Issues	2	High
Security Concerns	2	Medium
Bugs	2	Medium
Code Quality	3	Low
Documentation	3	Low

Must Fix Before Merge:

1. Address DoS risk (max frame limit)
2. Fix norm() API inconsistency
3. Document Docker networking breaking change
4. Add error handling for Doppler calculation

Should Fix:

5. Add request rate limiting/timeout handling
6. SSRF protection (if not already present)
7. Refactor to reduce code duplication
8. Complete integration testing checklist

---

Final Verdict

This is high-quality work with excellent test coverage and thoughtful design. The statistical noise modeling is sophisticated and well-implemented. However, there are some critical issues around performance, DoS protection, and a breaking Docker change that should be addressed before merging.

Great job on the comprehensive documentation and test suite!

[claude]

Pull Request Review: Synthetic Radar Detection Generation

Summary

This PR adds a /api/synthetic-detections endpoint that generates realistic synthetic radar detections with configurable noise. The implementation is well-structured, thoroughly tested, and includes good documentation. Overall, this is high-quality work with attention to detail.

✅ Strengths

Code Quality

• Excellent test coverage: 476 lines of comprehensive tests covering statistical properties, edge cases, and configuration validation
• Clean architecture: Well-separated concerns with dedicated synthetic.js module
• Good documentation: Detailed README with examples and statistical properties explained
• Statistical rigor: Proper implementations of Box-Muller transform, Poisson distribution, and Bernoulli trials

Security

• Proper input validation: All configuration parameters validated with clear error messages
• Security checks preserved: The new endpoint includes all the SSRF protection from /api/dd (private IP filtering, DNS resolution checks)
• Resource limits: MAX_FRAMES (1000) and MAX_DURATION_SECONDS (300) prevent abuse

🔍 Issues & Recommendations

1. Potential Memory/Performance Issue (High Priority)

Location: src/server.js:367-486

The /api/synthetic-detections endpoint makes synchronous API calls in a loop:

for (let i = 0; i < nFrames; i++) {
  // ...
  json = await getTar1090(apiUrl);  // Blocking call in loop
  // ...
}

Problems:

• With default settings (10s duration, 500ms interval = 20 frames), this makes 20 sequential HTTP requests
• Maximum configuration (300s, 100ms interval = 3000 requests) would exceed the 1000 frame limit but could still make 1000 requests
• This will cause very long response times and high memory usage
• The endpoint holds the HTTP connection open for the entire duration

Recommendation:

• Add request timeout to prevent the endpoint from hanging indefinitely
• Consider documenting expected response times (e.g., "This endpoint may take up to duration seconds to respond")
• For production use, consider implementing this as a background job with a job ID rather than a synchronous endpoint

2. Code Duplication (Medium Priority)

Location: src/server.js:232-489

The new endpoint duplicates ~130 lines of validation logic from /api/dd:

• Server URL validation (lines 261-351)
• SSRF protection checks
• Private IP filtering

Recommendation: Extract this validation into a shared function:

function validateAndCheckServerUrl(server) {
  // Returns {valid: boolean, error: string, serverUrl: URL, isAdsbLol: boolean}
}

3. Incomplete Test Coverage (Low Priority)

Location: Test plan in PR description shows unchecked items

Missing tests:

• Manual API testing with live ADS-B data
• Integration testing with retina-tracker

Recommendation: While unit tests are excellent, consider adding an integration test that validates the output format is compatible with retina-tracker.

4. Minor: norm() Function Signature Inconsistency (Low Priority)

Location: src/server.js:359, 410, 412

The code calls norm() with an array:

const dRxTx = norm([ecefRx.x - ecefTx.x, ecefRx.y - ecefTx.y, ecefRx.z - ecefTx.z]);

But based on existing /api/dd code, norm() likely expects an object with {x, y, z} properties. This should be verified - if the function signature was changed, it may break the existing /api/dd endpoint.

5. Minor: Hardcoded SNR in convertToFrameFormat() (Low Priority)

Location: src/node/synthetic.js:311

snrs.push(15.0);  // Hardcoded default

Recommendation: Make this a named constant or add a comment explaining why 15.0 was chosen.

6. Docker Compose Network Change (Medium Priority)

Location: docker-compose.yml:5

Changed from network_mode: host to explicit port mapping 49155:49155.

Concern: The CLAUDE.md documentation states "Uses host networking for direct network access" and this change contradicts that. This could affect:

• The container's ability to reach tar1090 servers on the local network
• Integration with the blah2 radar system

Recommendation:

• Test that tar1090 connectivity still works with this change
• Update CLAUDE.md to reflect the new network configuration
• Verify blah2 integration is unaffected

7. Missing Error Handling in Utility Script (Low Priority)

Location: process_snapshots.js:157

The script reads files with basic error handling but doesn't validate that snapshots contain valid ADS-B data before processing.

Recommendation: Add validation that snapshots have expected structure before computing delay-Doppler.

🛡️ Security Assessment

✅ No major security concerns. The implementation:

• Validates all inputs properly
• Prevents SSRF attacks with comprehensive private IP filtering
• Includes resource limits to prevent DoS
• Uses a well-vetted library (seedrandom) for RNG

Minor note: The seedrandom library could theoretically be used for cryptographic purposes by a user (which would be inappropriate), but given the use case (test data generation), this is not a concern.

📊 Performance Considerations

Current implementation:

• Default config: ~10-20 seconds response time (20 sequential API calls)
• Max config: Could theoretically take 5+ minutes (limited to 1000 frames)
• Memory usage: Accumulates all frames in memory before returning

Recommendations:

1. Add a note in the README about expected response times
2. Consider streaming responses (NDJSON format) for large frame counts
3. Add an HTTP timeout to prevent indefinitely hanging connections

🧪 Testing

Excellent statistical test coverage:

• RNG distributions validated with 10,000 sample tests
• Tolerance checks appropriate (.toBeCloseTo(expected, precision))
• Edge cases covered (missing data, invalid config, etc.)

Suggestion: Add a smoke test that validates a complete end-to-end workflow with actual tar1090 data (if possible in CI environment).

📝 Documentation

Strengths:

• Comprehensive README additions with examples
• Clear API parameter documentation
• Statistical properties explained

Minor improvements:

• Add expected response time information
• Document the synchronous nature of the endpoint (blocks until all frames generated)
• Consider adding example output in the README

🎯 Recommendations Summary

Must fix before merge:

1. Verify norm() function signature compatibility
2. Test docker-compose network change doesn't break existing functionality

Should fix:

1. Extract shared validation logic to reduce duplication
2. Add request timeout to prevent hanging connections
3. Document expected response times

Nice to have:

1. Add integration test with retina-tracker
2. Consider streaming response for large datasets
3. Add named constant for default SNR value

✨ Conclusion

This is high-quality, production-ready code with excellent test coverage and documentation. The statistical implementations are correct, and the security considerations are well-handled. The main concerns are around performance/scalability for large frame counts and code duplication. With the recommended fixes, this will be a valuable addition to the project.

Recommendation: ✅ Approve with minor changes

Great work! 🚀