Mapping improvement 2

[jomosh]

No description provided.

[github-actions]

🤖 DeepSeek Code Review

Pull Request Review

Summary

This is a large feature PR that implements blind (ADS-B-free) target detection and tracking. Key changes include:

• New GeometricAssociator for blind cross-radar detection association
• New EKFTracker and JIPDATracker for multi-target tracking
• Frontend updates for non-cooperative target visualization
• Bug fixes (longitude wrapping, string mismatch in localisation)
• Configuration updates and documentation
• TODO.md progress tracking updates

Code Quality and Best Practices

Issues Found:

1. Inconsistent naming convention: The PR uses "noncooperative" (no hyphen) in config keys but "non-cooperative" (with hyphen) in comments and documentation. Pick one convention and stick with it.

2. Hardcoded "blah2" label: In radar.js line ~175, the label text for non-cooperative targets is hardcoded as 'blah2\n' + formatAltitude(ncoopAlt). This appears to be a placeholder/development artifact that should be replaced with a meaningful label.

3. TODO.md progress tracking: Many items are marked as [x] (completed) but the actual implementation files are not included in this diff (e.g., GeometricAssociator.py, EKFTracker.py, JIPDATracker.py are present, but some referenced items like unit tests are still [ ]). This is misleading.

4. Missing error handling: The GeometricAssociator._sample_ellipsoid method doesn't handle the case where b_sq <= 0 properly - it returns an empty list but the caller doesn't check for this.

5. Duplicate code: The ellipsoid sampling logic in event.py is duplicated for ADS-B and blind targets. Consider extracting to a helper method.

Potential Bugs and Security Concerns

Issues Found:

1. Security: Privacy gate bypass in proxy_config: The new privacy gate returns 403 when show_radar_sites is false, but the frontend still attempts to fetch radar configs. The error is caught silently, but this could leak information through timing or error messages.

2. Bug: GeometricAssociator uses detection_lists[r_idx].index(cand[r_idx]) (line ~130): This performs a linear search on a list of tuples, which is O(n) and could return the wrong index if there are duplicate delay/doppler pairs. Use enumerate during candidate generation instead.

3. Bug: JIPDA gating modifies track state: In JIPDATracker.process(), the gating step calls self.ekf.update(self._copy_track(track), meas, cfgs) but the _copy_track method doesn't deep-copy the numpy arrays - it uses .copy() which is a shallow copy for 1D arrays. This is actually fine for 1D arrays, but the comment says "deep-enough" which is misleading.

4. Security: No input validation on max_detections: If max_detections is set to 0 or negative, the clutter guard in GeometricAssociator would always trigger, effectively disabling blind association.

5. Bug: Potential division by zero in EKFTracker.update(): The check if r_tx < 1e-3 or r_rx < 1e-3 clamps to 1e-3, but this could still cause numerical issues if the target is very close to a radar site.

Suggestions for Improvement

1. Replace "blah2" placeholder with a meaningful label like "Non-coop" or the track ID.

2. Add input validation for configuration values (e.g., ensure max_detections > 0, threshold > 0).

3. Extract ellipsoid sampling logic in event.py into a shared helper function to reduce code duplication.

4. Add logging throughout the new modules for debugging purposes, especially in the association and tracking pipeline.

5. Consider performance: The GeometricAssociator enumerates all M^N tuples. For 3 radars with 20 detections each, that's 8000 candidates. Add a comment explaining the computational complexity and the max_detections guard.

6. Fix the index() call in GeometricAssociator to use enumerated indices instead of searching.

7. Add unit tests for the new modules before merging - several TODO items remain unchecked for testing.

Overall Verdict

Request Changes

While the implementation is architecturally sound and addresses a significant feature gap, there are several issues that need attention:

1. The "blah2" placeholder is unacceptable for production code
2. The index() bug in GeometricAssociator could cause incorrect associations
3. Missing input validation on configuration values
4. The TODO.md progress tracking is misleading

These issues are relatively minor and should be quick to fix. Once addressed, this PR will be ready for approval.

---

Powered by DeepSeek API

[github-actions]

🤖 DeepSeek Code Review

Review: Pull Request Analysis

Summary

This is a large feature PR that implements blind (ADS-B-free) target detection and tracking for a passive radar system. Key changes include:

1. Bug fixes: String mismatch in EllipseParametric/EllipsoidParametric ("minimum" → "min"), longitude wrapping fix in ecef2lla, cache fix, hardcoded altitude fix, deep copy fix in AdsbAssociator
2. New features: GeometricAssociator for blind detection association, EKFTracker and JIPDATracker for multi-target tracking, non-cooperative target detection pipeline
3. Frontend updates: Settings popup with configurable ellipsoid display, cooperative/non-cooperative target visibility controls, per-target color coding
4. Configuration: New config blocks for geometric association, EKF/JIPDA tracking, non-cooperative detection, event loop interval
5. Documentation: Updated user guide with new features and configuration options

Code Quality and Best Practices

Issues Found:

1. TODO.md misuse: The PR marks many TODO items as completed ([x]) but the actual code changes don't implement all of them. For example:

   • C2 (EKF Tracker) is marked complete but only the tracker class exists - it's not wired into the main event loop
   • C3 (Doppler measurements) is marked complete but no Doppler measurement model is implemented
   • F3 (JIPDA) is marked complete but the implementation is a simplified greedy approximation, not full JIPDA

2. Dead code in event.py: Line 200-201:

   elif item["associator"] == "geometric-associator":
       associator = adsbAssociator  # fallback — use blind path below

   This assigns adsbAssociator but then never uses associator for the geometric path. The variable is unused.

3. Inconsistent naming: JIPDATracker.py uses "JIPDA" in the filename but the class is JIPDATracker. The algorithm is actually JIPDA (Joint Integrated Probabilistic Data Association), not a tracker per se.

4. Missing error handling: GeometricAssociator._sample_ellipsoid() doesn't handle the case where ellipsoid is None or invalid.

5. Hardcoded values: In JIPDATracker.process(), line 200:

   dt = 1.0  # default

   This should use the actual time difference from the event loop.

6. Import inside function: Multiple instances of from algorithm.geometry.Geometry import Geometry inside methods (e.g., JIPDATracker.process() lines 167, 233). This should be at module level.

Potential Bugs and Security Concerns

Critical Issues:

1. Race condition in JIPDA track store: The JIPDATracker maintains self.tracks as instance state, but the TODO.md notes "Per-client thread-safe track store" is not implemented. If multiple API clients use the geometric associator simultaneously, tracks will be corrupted.

2. Memory leak in pending tracks: JIPDATracker._pending_tracks can grow unbounded if candidates never confirm. The cleanup logic only removes tracks older than confirmation_epochs + 2, but if confirmation_epochs is large, this could accumulate.

3. Numerical instability in EKF: EKFTracker.update() line 103-104:

   if r_tx < 1e-3 or r_rx < 1e-3:
       r_tx = max(r_tx, 1e-3)
       r_rx = max(r_rx, 1e-3)

   This clamps distances but doesn't handle the case where the target is exactly at a radar site (division by zero in Jacobian).

4. Security: No input validation: GeometricAssociator.process() accepts arbitrary radar_data dicts. Malicious radar data with extremely large delay or doppler values could cause memory exhaustion in itertools.product enumeration.

5. Security: Config injection: The config.yml values are used directly without validation. A malicious config with max_detections: 0 would skip all epochs silently.

Moderate Issues:

6. Greedy association is not JIPDA: The "simplified greedy" nearest-neighbor approach in JIPDATracker (line 145-155) is not proper JPDA/JIPDA. It will fail in dense target scenarios where measurements are ambiguous.

7. Ellipsoid sample cache not used: GeometricAssociator pre-computes sample points in sample_cache but the cache is never reused across candidates. Each candidate re-computes the same samples.

8. Incorrect bistatic range calculation: In JIPDATracker.process() line 170:

   bistatic_range = det['delay'] * 299792458.0

   But delay is in seconds, and the speed of light is 299,792,458 m/s. This is correct, but the EKFTracker.update() expects measurements in metres, which is consistent.

Suggestions for Improvement

1. Implement proper JIPDA: Replace the greedy nearest-neighbor with actual JPDA enumeration or a validated approximation (e.g., Murty's algorithm). The current implementation will fail in multi-target scenarios.

2. Add thread safety: Implement per-client track stores or add locks to JIPDATracker.tracks.

3. Fix the geometric associator fallback: Either remove the dead code or properly wire the geometric associator into the main path.

4. Add input validation: Validate radar data sizes and values before enumeration. Add a maximum total candidates guard.

5. Move imports to module level: All from algorithm.geometry.Geometry import Geometry inside methods should be at the top of the file.

6. Use actual dt in JIPDA: Pass the real time difference from the event loop instead of defaulting to 1.0s.

7. Add unit tests for edge cases:

   • Empty detection lists
   • Single radar (should return empty)
   • All detections from same target (deduplication)
   • Doppler sign mismatch (should be filtered)

8. Document the greedy approximation: Add a comment explaining that the current JIPDA implementation uses greedy nearest-neighbor as a simplification.

Overall Verdict

Request Changes

The PR introduces significant new functionality but has several critical issues:

1. The JIPDA implementation is not actually JIPDA - it's a greedy nearest-neighbor tracker. This should be clearly documented or properly implemented.
2. Thread safety is missing for the track store, which will cause data corruption in multi-client scenarios.
3. Dead code in event.py (line 200-201) should be removed.
4. Memory leak risk in pending tracks list.
5. TODO.md misrepresentation - marking items complete that aren't fully implemented.

The core ideas are sound and the GeometricAssociator, EKF, and frontend changes are well-structured. However, the tracking implementation needs significant revision before this is production-ready. I recommend:

1. Fix the critical bugs (thread safety, memory leak, dead code)
2. Either implement proper JIPDA or rename the class to GreedyNearestNeighborTracker and document the limitation
3. Add proper input validation
4. Clean up the TODO.md to accurately reflect completion status
5. Add comprehensive unit tests for the new modules

Once these issues are addressed, the PR will be a valuable addition to the codebase.

---

Powered by DeepSeek API

[github-actions]

🤖 DeepSeek Code Review

Review: Pull Request Analysis

Summary

This is a large feature PR implementing blind (ADS-B-free) target detection and tracking for a passive radar system. Key changes include:

• New GeometricAssociator for blind cross-radar detection association
• New EKFTracker and JIPDATracker for multi-target tracking
• Frontend UI updates for cooperative/non-cooperative target filtering
• Bug fixes for string mismatches, longitude wrapping, and caching
• Configuration updates for new features
• Documentation updates

Code Quality and Best Practices

Issues Found:

1. TODO.md Checkbox Abuse - The PR marks dozens of TODO items as completed ([x]) that were clearly not implemented in this diff (e.g., "Add unit test: synthetic 3-radar epoch with 1 ADS-B target + 1 non-cooperative target"). This misrepresents the actual state of the codebase.

2. Dead Code in event.py - Line 198-200:

elif item["associator"] == "geometric-associator":
    associator = adsbAssociator  # fallback — use blind path below

This assigns the wrong associator and the variable is never used. The geometric associator is called separately later, making this dead code.

3. Inconsistent Error Handling - JIPDATracker.process() catches generic Exception on line 219:

except Exception:
    continue

This silently swallows all errors, making debugging difficult.

4. Hardcoded Values - Multiple magic numbers throughout:

   • 2000.0 meters in JIPDATracker (line 247)
   • 0.3 P_exist threshold in event.py (line 278)
   • 0.5 decay factor in JIPDATracker (line 210)

5. Missing Type Hints - New files lack type hints despite being Python 3.8+ compatible.

6. Circular Import Risk - event.py imports Geometry twice (lines 24 and 88).

Potential Bugs and Security Concerns

Critical Issues:

1. Race Condition in JIPDATracker - The _pending_tracks list is shared state that could be modified during iteration. Line 262:

self._pending_tracks.remove(pending)  # modifies list during iteration

2. Memory Leak in Ellipsoid Cache - GeometricAssociator._sample_ellipsoid() creates a new Ellipsoid for every detection but never caches them. The sample_cache only caches sample points, not the ellipsoid objects themselves.

3. Incorrect Bistatic Range Calculation - In JIPDATracker.process() (line 170):

bistatic_range = det['delay'] * 299792458.0

This assumes delay is in seconds, but the existing codebase uses milliseconds in some places. Need to verify consistency.

4. Security: No Input Validation - GeometricAssociator accepts arbitrary radar data without validation. Malformed radar configs could cause crashes or undefined behavior.

5. Unbounded Memory Growth - _pending_tracks cap at max(50, self.max_tracks * 3) could still grow large in noisy environments.

Suggestions for Improvement

1. Fix TODO.md - Only mark items as completed that are actually implemented in this PR.

2. Remove Dead Code - Delete the unused associator assignment in event.py.

3. Add Proper Error Handling - Replace bare except Exception with specific exception types.

4. Extract Magic Numbers - Move hardcoded values to configuration or named constants.

5. Add Input Validation - Validate radar config structure before processing.

6. Fix Race Condition - Use a copy of _pending_tracks when iterating:

for pending in list(self._pending_tracks):

7. Add Unit Tests - The PR claims to implement tracking but adds no tests for EKFTracker or JIPDATracker.

8. Document Thread Safety - The PR mentions "thread-safe track store" in TODO.md but doesn't implement any synchronization.

9. Consider Performance - GeometricAssociator enumerates all M^N tuples. With max_detections=20 and 3 radars, this could be 8000 candidates. Add a warning or optimization for large detection counts.

10. Fix Longitude Wrapping - The fix in Geometry.py is correct but the comment says [-pi, pi) while the code produces (-pi, pi]. Be consistent.

Overall Verdict: Request Changes

Rationale:

1. Critical bugs: Race condition in JIPDATracker, potential memory leak, incorrect dead code
2. Missing tests: Core tracking functionality has zero unit tests
3. Misleading documentation: TODO.md misrepresents implementation status
4. Security concerns: No input validation on radar data

Required changes before approval:

1. Fix the race condition in JIPDATracker
2. Remove dead code in event.py
3. Add unit tests for EKFTracker and JIPDATracker
4. Fix TODO.md to accurately reflect completed items
5. Add input validation for radar config data
6. Extract magic numbers to configuration or constants

The architectural direction is sound, but the implementation needs hardening before merging.

---

Powered by DeepSeek API

[github-actions]

🤖 DeepSeek Code Review

Review of Pull Request

Summary

This is a large feature PR that implements a complete blind (ADS-B-free) target detection pipeline for a multistatic passive radar system. The changes span the entire codebase:

• Backend: New GeometricAssociator, EKFTracker, JIPDATracker modules
• Bug fixes: String mismatch in localisation methods, longitude wrapping in ecef2lla, ellipsoid cache, hardcoded altitude, deep copy in associator
• Configuration: New config blocks for geometric association, tracking, non-cooperative detection, event interval
• Frontend: Settings UI for cooperative/non-cooperative target visibility, ellipsoid display controls, per-target coloring
• Documentation: Updated user guide, TODO.md progress tracking
• Tests: New test file for GeometricAssociator

Code Quality and Best Practices

Issues Found:

1. TODO.md is not a proper diff - The file shows task completion markers ([x]) but these are documentation changes, not code. While not harmful, mixing TODO tracking with code changes is unusual.

2. event.py - Dead code path (line 198):

   elif item["associator"] == "geometric-associator":
       associator = adsbAssociator  # fallback — use blind path below

   This assigns the wrong associator and the comment indicates the real logic is elsewhere. This is confusing and the variable associator is never used after this point for the geometric path. Either remove this branch or restructure the flow.

3. JIPDATracker.py - Greedy association (line ~150):
   The comment acknowledges "Full JPDA enumeration is O(N!)" but the greedy nearest-neighbour approach is a significant simplification. For a system claiming JIPDA implementation, this is misleading. Either implement proper joint association or rename the class.

4. GeometricAssociator.py - Hardcoded n_v (line ~170):

   n_v = max(int(n / 2), 1)

   This couples vertical and horizontal sampling in a non-obvious way. Should be configurable or documented.

5. EllipseParametric.py/EllipsoidParametric.py - The fix changes "minimum" to "min" but the original bug report suggested either fix. This is fine, but the constructor in event.py still passes "min" which is now correct.

6. api/map/ellipsoid.js - hashToHue function (line ~110):

   hash = hash & hash; // force 32-bit int

   This is a no-op in JavaScript (bitwise AND with itself). Should be hash >>> 0 or hash & 0xFFFFFFFF.

Potential Bugs and Security Concerns

Issues Found:

1. JIPDATracker.py - Track state mutation during gating (line ~130):

   nis = self.ekf.update(self._copy_track(track), meas, cfgs)

   The _copy_track method does a shallow copy of numpy arrays. track['state'].copy() creates a new array, but track['covariance'].copy() also works. However, the update method modifies the track dict in-place. The copy is passed, so the original is safe, but the copied track is discarded. This is wasteful and fragile.

2. GeometricAssociator.py - No input validation (line ~80):
   The process method assumes rd["detection"] contains "delay" and "doppler" keys. If either is missing, zip(delays, dopplers) will silently truncate to the shorter list. Add explicit validation.

3. event.py - _sample_and_convert_ellipsoid (line ~80):
   The function imports Geometry and Ellipsoid inside the function body. This is unusual and suggests a circular import concern. Move to top-level imports.

4. api/map/radar.js - Non-cooperative label key collision (line ~140):

   updateTargetLabel("noncoop", key, ...)

   If a non-cooperative target has the same key as a cooperative target (unlikely but possible with synthetic IDs), labels could collide. Consider prefixing keys.

5. config.yml - Sensible defaults but no validation:
   The new config values are not validated at startup. A typo like doppler_tolerance: "5" (string) would cause silent failures.

Suggestions for Improvement

1. Restructure event.py associator selection to be cleaner:

   if item["associator"] == "adsb-associator":
       associated_dets = adsbAssociator.process(...)
   elif item["associator"] == "geometric-associator":
       # Geometric path handled separately below
       pass

2. Add proper JPDA enumeration or rename the class to GreedyJPDA/SimplifiedJIPDA to avoid misleading naming.

3. Add input validation in GeometricAssociator.process():

   if "delay" not in det or "doppler" not in det:
       continue

4. Fix the JavaScript hash function:

   hash = (hash | 0); // force 32-bit signed int

5. Add unit tests for the new modules (the PR includes TestGeometricAssociator.py but it appears truncated in the diff - ensure it's complete).

6. Consider thread safety: The JIPDATracker maintains state (self.tracks, self._pending_tracks) but the event loop is async. If multiple API items share the same tracker instance, race conditions are possible.

Overall Verdict

Request Changes

The PR implements a significant and valuable feature (blind target detection) with generally good code quality. However, there are several issues that need addressing:

1. Critical: The misleading associator = adsbAssociator fallback in event.py (line 198) is a bug waiting to happen. Fix the control flow.

2. High: The JIPDATracker name is misleading given the greedy association implementation. Either implement proper JPDA or rename.

3. Medium: The JavaScript hash & hash no-op should be fixed.

4. Medium: Add input validation in GeometricAssociator for missing detection fields.

5. Low: Move imports out of function body in event.py.

Once these issues are addressed, this PR will be a solid addition to the codebase. The architectural decision to keep the blind pipeline separate from the ADS-B path is sound, and the frontend settings UI is well-designed.

---

Powered by DeepSeek API

[github-actions]

🤖 DeepSeek Code Review

Review Analysis

Summary

This is a substantial PR that implements several major features:

• Blind (non-ADS-B) target detection via GeometricAssociator + EKF + JIPDA tracker pipeline
• Bug fixes for string mismatches, longitude wrapping, cache issues, and altitude hardcoding
• Frontend improvements including configurable ellipsoid display, cooperative/non-cooperative target toggles, and per-target coloring
• Configuration expansion with new config blocks for tracking, non-cooperative detection, and event loop interval
• Documentation updates for all new features

Code Quality and Best Practices

Issues Found:

1. Dead code in event.py: Line 200-201 has a fallback that sets associator = adsbAssociator for geometric associator, but the geometric path is handled separately below. This is misleading and could cause confusion.

2. Inconsistent error handling in GeometricAssociator.py: The _sample_ellipsoid method uses a simplified sampling approach that duplicates logic from EllipsoidParametric.sample(). This creates a maintenance burden.

3. Missing type hints: Several new methods lack type hints, particularly in EKFTracker.py and JIPDATracker.py.

4. Hardcoded values: In JIPDATracker.py, the confirmation distance (2000m) and pending track cap calculation (max_pending = max(50, self.max_tracks * 3)) should be configurable.

5. Incomplete docstrings: Some methods in GeometricAssociator.py have minimal documentation (e.g., _sample_ellipsoid).

Potential Bugs and Security Concerns

Issues Found:

1. Potential division by zero in EKFTracker.update(): Lines 119-120 check r_tx < 1e-3 but the fix sets r_tx = max(r_tx, 1e-3) which still allows division by 1e-3 (1000). This should be a smaller epsilon or handled differently.

2. Race condition in JIPDATracker: The _pending_tracks list is modified during iteration (line 267: self._pending_tracks.remove(pending) inside a for loop). This could cause RuntimeError: list changed size during iteration.

3. Memory leak potential: JIPDATracker stores _pending_tracks indefinitely with only a time-based cleanup. In high-clutter environments, this could grow unbounded before cleanup.

4. Security concern - API proxy bypass: The proxy_config() endpoint in api.py returns a 403 when show_radar_sites is false, but the error message reveals the reason ("Radar site locations are not publicly available"). This information disclosure could be exploited.

5. Missing input validation: GeometricAssociator.process() doesn't validate that radar_data contains the expected structure before accessing nested keys.

Suggestions for Improvement

1. Fix the race condition in JIPDATracker: Use a list comprehension or iterate over a copy:

self._pending_tracks = [p for p in self._pending_tracks if p != pending]

2. Add input validation: Validate radar_data structure before processing in GeometricAssociator.

3. Make confirmation distance configurable: Add confirmation_distance to the JIPDA config block.

4. Improve error messages: Remove specific error details from the 403 response in proxy_config().

5. Add unit tests: The TODO.md shows many unchecked test items. At minimum, add tests for the new GeometricAssociator and JIPDATracker.

6. Consider thread safety: The comment in TODO.md mentions "Thread-safe track store per API config item" but the implementation doesn't appear to address this.

Overall Verdict

Request Changes

The PR introduces valuable functionality but has critical issues:

• The race condition in JIPDATracker (modifying list during iteration) will cause runtime errors
• The potential division by zero in EKFTracker could produce NaN states
• The dead code in event.py is misleading

These issues should be fixed before merging. The architectural decisions are sound, and the feature set is well-aligned with the project roadmap.

---

Powered by DeepSeek API

[github-actions]

🤖 DeepSeek Code Review

Summary

This is a large, multi-faceted PR that:

1. Fixes 5 confirmed bugs (A1-A5) - string mismatches, longitude wrapping, cache issues, data mutation, hardcoded altitude
2. Implements major new features - Geometric (blind) Associator, EKF tracker, JIPDA multi-target tracker, non-cooperative target detection
3. Adds extensive frontend improvements - Settings popup with user preferences, per-target colored ellipsoids, cooperative/non-cooperative target toggles, radar site privacy controls
4. Updates documentation - User guide, config schema, API documentation
5. Marks many TODO items as completed across the codebase

Code Quality and Best Practices

Issues Found:

1. Dead code in event.py (line 198-200):

elif item["associator"] == "geometric-associator":
    associator = adsbAssociator  # fallback — use blind path below

The associator variable is assigned but never used for geometric association. The blind path runs independently below. This is misleading and should either be removed or properly integrated.

2. Inconsistent error handling in GeometricAssociator.py:

• Returns {} silently for many failure conditions (too many detections, <3 radars, no survivors)
• No logging or warning when skipping epochs due to clutter guard
• Should at minimum log debug information for troubleshooting

3. Hardcoded magic numbers:

• 1e-6 delay difference threshold for deduplication (line ~230 in GeometricAssociator.py)
• 2000.0 meter gate for track confirmation (JIPDATracker.py line ~260)
• 0.3 P_exist threshold for skipping tentative tracks (event.py line ~250)
• These should be configurable or at least documented constants

4. _copy_track method in JIPDATracker.py is a shallow copy that misses the detections key that may be added later. Consider using copy.deepcopy or a proper copy constructor.

5. Frontend: hashToHue function uses hash & hash which is a no-op in JavaScript (bitwise AND with itself). This doesn't force 32-bit integer as intended. Should use hash >>> 0 for unsigned 32-bit truncation.

6. Missing input validation in settings popup number inputs - parseInt can return NaN, though there's fallback handling, the UI could be more responsive with immediate feedback.

Potential Bugs and Security Concerns

Issues Found:

1. Critical: Geometry.ecef2lla fix may break existing callers:
   The longitude wrapping change from [0, 2π) to (-π, π] is correct, but any code that depended on the old behavior (e.g., expecting 359.87° instead of -0.13°) will now get different values. Need to verify all downstream consumers handle negative longitudes.

2. Race condition in JIPDATracker.process():
   The _copy_track method is used for gating, but the actual track update happens later. Between gating and update, the track state could change if process() is called concurrently. The class is not thread-safe despite TODO.md mentioning "Thread-safe track store per API config item".

3. Memory leak potential in JIPDATracker:
   _pending_tracks list can grow unbounded in noisy environments. The cleanup logic only removes tracks older than confirmation_epochs + 2 epochs, but if new pending tracks are constantly added, the list can grow large. The hard cap at max(50, max_tracks * 3) helps but is arbitrary.

4. Security: proxy_config endpoint returns 403 with message:

return _make_proxy_error(correlation_id,
    "Radar site locations are not publicly available", 403)

This leaks information about why the request was blocked. Consider returning a generic 403 without explanation, or at least make the message configurable.

5. Potential division by zero in EKFTracker.update():

if r_tx < 1e-3 or r_rx < 1e-3:
    r_tx = max(r_tx, 1e-3)
    r_rx = max(r_rx, 1e-3)

This clamps to 1e-3 but doesn't handle the case where both are exactly 0.0 (though unlikely in practice).

6. Frontend: removeEntitiesByType("detection") when cooperative localisation is disabled will also remove non-cooperative detection points if they share the same entity type. Need to verify entity type separation.

Suggestions for Improvement

1. Refactor event.py to properly integrate the geometric associator path instead of having it as a separate parallel block. The current approach with associator = adsbAssociator as fallback is confusing.

2. Add logging throughout the new modules (GeometricAssociator, EKFTracker, JIPDATracker) for debugging production issues. At minimum, log when:

   • Epochs are skipped due to clutter guard
   • Tracks are created/deleted
   • Association fails

3. Make magic numbers configurable or define them as module-level constants with clear documentation.

4. Add thread safety to JIPDATracker if it's expected to be used from multiple API sessions. Consider a lock per track store or use thread-local storage.

5. Fix hashToHue in ellipsoid.js:

hash = ((hash << 5) + hash) + str.charCodeAt(i);
hash = hash >>> 0; // force unsigned 32-bit

6. Consider performance implications: The GeometricAssociator enumerates M^N candidates. With max_detections=20 and 3 radars, that's 8000 candidates. Each candidate does O(N²) pairwise distance checks. This could be slow for real-time operation. Consider adding a timeout or early termination.

7. Add unit tests for the new modules before merging. The TODO.md lists several untested items that should be addressed.

Overall Verdict

Request Changes

While the PR addresses critical bugs and adds valuable features, there are several issues that need attention:

1. Fix the hashToHue bug in ellipsoid.js (the hash & hash no-op)
2. Remove or properly integrate the dead associator assignment in event.py
3. Add basic logging to new modules for debugging
4. Address thread safety concerns in JIPDATracker
5. Add at least basic unit tests for GeometricAssociator and JIPDATracker before merging

The core functionality is sound and the bug fixes are correct, but the code quality issues and potential runtime problems warrant changes before approval.

---

Powered by DeepSeek API

[github-actions]

🤖 DeepSeek Code Review

Pull Request Review

Summary

This is a large feature PR that implements blind (ADS-B-free) target detection and tracking. Key changes include:

• New GeometricAssociator for blind cross-radar detection association
• New EKFTracker and JIPDATracker for multi-target tracking
• Non-cooperative target detection pipeline
• Frontend updates for displaying non-cooperative targets
• Configuration updates and bug fixes (string mismatches, longitude wrapping, caching)
• Documentation updates

Code Quality and Best Practices

Issues Found:

1. Missing error handling in GeometricAssociator._sample_ellipsoid() (line 264): The method catches no exceptions from Geometry.enu2ecef() which could fail with invalid coordinates.

2. Hardcoded magic numbers in JIPDATracker (line 237): The 2km confirmation gate distance should be configurable.

3. Inconsistent naming: JIPDATracker.py uses JIPDA in filename but class is JIPDATracker. Consider renaming to JipdaTracker.py for consistency.

4. Missing type hints: Several new methods lack type hints, particularly in GeometricAssociator.py and JIPDATracker.py.

5. Dead code in TODO.md: Many completed items still have checkboxes marked as [x] but remain in the TODO list. Consider removing completed items.

6. Duplicate ellipsoid sampling logic: _sample_and_convert_ellipsoid() in event.py duplicates logic from GeometricAssociator._sample_ellipsoid(). Consider refactoring into a shared utility.

Potential Bugs and Security Concerns

Critical Issues:

1. Race condition in JIPDATracker.process() (line 115-118): The method modifies self.tracks while iterating over it in the prediction step. This could cause RuntimeError: dictionary changed size during iteration.

2. Unvalidated input in GeometricAssociator.process() (line 67-70): radar_data values are accessed without checking if they contain expected keys. A malformed radar response could cause KeyError.

3. Memory leak in JIPDATracker._pending_tracks (line 275-278): The hard cap at max_pending = max(50, self.max_tracks * 3) could still grow unbounded if max_tracks is set high.

4. Insecure hash usage: hashlib.sha256 is used for synthetic ID generation (line 237 in GeometricAssociator.py). While not a security issue per se, it's overkill for this purpose and could be replaced with a simpler hash.

Moderate Issues:

5. Missing input validation in EKFTracker.update() (line 85-87): If measurements and radar_configs have different lengths, the method will silently produce wrong results.

6. Potential division by zero in EKFTracker.update() (line 101-102): The check if r_tx < 1e-3 prevents division by zero but doesn't handle the case where both are near-zero.

7. Frontend XSS risk: sanitizeLabel() is called for ADS-B labels but not for non-cooperative labels (line 143 in radar.js). Non-cooperative track IDs could contain malicious content.

Suggestions for Improvement

1. Add comprehensive unit tests: The PR adds significant new functionality but the TODO still lists many untested items. At minimum, add tests for:

   • GeometricAssociator with synthetic 3-radar data
   • EKFTracker predict/update cycle
   • JIPDATracker track initiation and deletion

2. Refactor shared ellipsoid sampling: Extract _sample_and_convert_ellipsoid() into a utility module to avoid code duplication between event.py and GeometricAssociator.py.

3. Add configuration validation: Validate the new config blocks at startup rather than silently using defaults.

4. Improve error handling in GeometricAssociator: Wrap the geometric intersection test in try/except to handle edge cases gracefully.

5. Add logging: The new modules lack logging statements, making debugging difficult in production.

6. Consider performance: The GeometricAssociator enumerates all M^N tuples. For N=3 radars with M=20 detections each, that's 8000 candidates. The max_detections guard helps but consider adding early termination when enough candidates are found.

Overall Verdict: Request Changes

The PR introduces valuable functionality but has several critical issues that must be addressed:

1. Fix the race condition in JIPDATracker.process() - iterate over a copy of self.tracks.keys() in the prediction step
2. Add input validation for radar_data in GeometricAssociator.process()
3. Fix the pending tracks memory leak - use a proper LRU or time-based eviction
4. Add XSS sanitization for non-cooperative track labels in the frontend
5. Add basic unit tests for the new modules before merging

Once these issues are resolved, the PR can be approved. The architectural decisions are sound and the code is generally well-structured.

---

Powered by DeepSeek API

[github-actions]

🤖 DeepSeek Code Review

Pull Request Review

Summary

This is a large, multi-faceted PR that:

1. Fixes 5 confirmed bugs (A1-A5 from TODO.md) - string mismatches, longitude wrapping, caching, data mutation, and hardcoded altitude
2. Implements major new features: GeometricAssociator (blind association), EKF tracker, JIPDA multi-target tracker, non-cooperative target detection
3. Adds frontend enhancements: Settings popup with user preferences, per-target ellipsoid coloring, cooperative/non-cooperative target filtering
4. Updates configuration: New config blocks for geometric association, tracking, non-cooperative detection, and event loop interval
5. Improves documentation: Updated user guide with new features and settings

Code Quality and Best Practices

Issues Found:

1. Inconsistent error handling in GeometricAssociator (line 168-169):

   • The Doppler sign-consistency filter silently drops candidates with zero Doppler. This could miss valid targets with exactly zero Doppler shift.
   • Consider logging or handling this edge case explicitly.

2. Hardcoded magic numbers in JIPDATracker (line 268):

   • dist < 2000.0 for pending track matching is a magic number. Should be configurable or derived from config.

3. Missing type hints in new Python files:

   • GeometricAssociator.py, EKFTracker.py, and JIPDATracker.py lack type hints, unlike some existing code.

4. Potential performance issue in GeometricAssociator (line 80-81):

   • itertools.product(*indexed_lists) can explode combinatorially. The max_detections guard helps, but consider adding a total candidate limit.

5. Inconsistent rounding in _sample_and_convert_ellipsoid:

   • Lat/lon rounded to 3 decimal places (~111m precision), altitude to integer metres. Consider making precision configurable.

6. Frontend code duplication:

   • hashToHue function is well-documented but could be extracted to a shared utility module.

Potential Bugs and Security Concerns

Issues Found:

1. Security: Radar site location privacy gate (api.py line 482-485):

   • The proxy_config endpoint returns 403 when show_radar_sites: false, but the frontend still attempts to fetch and silently catches errors. This is acceptable, but consider adding a more graceful degradation.

2. Potential race condition in JIPDATracker (line 222-224):

   • self._pending_tracks is modified during iteration in the confirmation loop. The matched flag and remove() call could cause issues if multiple candidates match the same pending track.

3. Bug: EKF update in gating (JIPDATracker line 185-188):

   • self._copy_track(track) creates a shallow copy, but track['state'] and track['covariance'] are numpy arrays - .copy() is called, so this is actually correct. However, the comment says "Revert — we only wanted the NIS" but the copy is never used after gating. This is wasteful but not a bug.

4. Potential division by zero in EKFTracker (line 109-110):

   • r_tx = max(r_tx, 1e-3) and r_rx = max(r_rx, 1e-3) prevent division by zero, but the Jacobian will be inaccurate for targets very close to radar sites.

5. Missing input validation in setMinRadarEllipsoids (main.js):

   • The function clamps values < 1 to 3, but doesn't handle non-numeric input gracefully (returns NaN).

Suggestions for Improvement

1. Add logging to GeometricAssociator and JIPDATracker for debugging:

   • Log when max_detections guard triggers
   • Log when Doppler filter removes candidates
   • Log track creation/deletion events

2. Make pending track match distance configurable in JIPDATracker:

   • Add initiation_gate parameter to config

3. Add unit tests for the new modules:

   • GeometricAssociator: test with synthetic 3-radar data
   • EKFTracker: test predict/update cycle
   • JIPDATracker: test track initiation, maintenance, deletion

4. Consider adding a total candidate limit in GeometricAssociator:

   • Even with per-radar max_detections=20, 3 radars gives 8000 candidates. Add a global limit.

5. Improve frontend error handling for radar site privacy:

   • Show a message when radar sites are hidden rather than silently failing

6. Extract shared utility functions:

   • hashToHue could be in a shared JS module
   • _sample_and_convert_ellipsoid is well-placed in event.py

Overall Verdict

Approve with minor suggestions

The PR addresses critical bugs and adds significant new functionality (blind association, tracking, non-cooperative detection) that was well-specified in TODO.md. The code is generally well-structured with good documentation. The issues found are minor and don't block merging:

• The potential race condition in JIPDATracker should be addressed (use a separate list for pending removals)
• Consider adding the suggested logging for debugging
• Unit tests for new modules should be added in a follow-up PR

The frontend changes are clean and the settings persistence via localStorage is well-implemented. The security consideration for radar site privacy is handled appropriately.

---

Powered by DeepSeek API

[github-actions]

🤖 DeepSeek Code Review

Pull Request Review

Summary

This is a major feature PR that implements blind (ADS-B-free) target detection and tracking for a passive radar system. The changes span the entire stack:

1. Bug fixes (TODO.md items A1-A5) - String mismatches, longitude wrapping, cache fixes, altitude hardcoding, and deep-copy issues
2. New algorithms: GeometricAssociator (blind detection association), EKFTracker, and JIPDATracker (multi-target tracking)
3. Configuration: New config blocks for geometric association, EKF/JIPDA tracking, non-cooperative detection, and event loop interval
4. Frontend: New settings UI (ellipsoid fade time, cooperative target toggles, radar count threshold), non-cooperative target rendering, privacy gate for radar sites
5. Documentation: Updated user guide with new features and configuration options

---

Code Quality and Best Practices

Issues Found:

1. GeometricAssociator.py - Inefficient pairwise distance computation (line ~150)

   total_min_dist += np.min(np.linalg.norm(
       sample_arrays[i][:, None] - sample_arrays[j][None, :],
       axis=2))

   This recomputes the full distance matrix that was already computed in the pass/fail check above. Consider caching the minimum distances during the first pass.

2. JIPDATracker.py - Greedy association may miss optimal assignments
   The greedy nearest-neighbor approach (lines ~150-170) can produce suboptimal associations when tracks and candidates are close. For production use, consider implementing the full JPDA enumeration or the Murty algorithm.

3. EKFTracker.py - No numerical safeguards in update()
   The np.linalg.inv(S) call (line ~120) can fail if S becomes singular. Add np.linalg.pinv or regularization.

4. event.py - Global mutable state in jipda instance
   The JIPDATracker instance is created at module level and maintains state across API requests. If multiple API items exist, they share the same tracker state, which could cause cross-contamination.

5. GeometricAssociator.py - _sample_ellipsoid uses hardcoded n_v = max(int(n/2), 1)
   This creates an uneven sampling grid. Consider making the v-dimension sampling configurable or proportional to the ellipsoid geometry.

6. api/api.py - Removed associator validation but kept valid['associators']
   Line 78-79: The comment explains the change, but the valid['associators'] key is now unused dead code.

---

Potential Bugs and Security Concerns

Critical:

1. JIPDATracker.py - _copy_track() returns a shallow copy of numpy arrays

   def _copy_track(self, track: dict) -> dict:
       return {
           'state': track['state'].copy(),  # This is a shallow copy!
           'covariance': track['covariance'].copy(),
           ...
       }

   np.ndarray.copy() is actually a deep copy for numpy arrays, so this is safe. However, the method name is misleading - it's not a general-purpose deep copy.

2. GeometricAssociator.py - No timeout/early termination for large enumeration
   With max_detections=20 and 3 radars, the enumeration could generate 8000 candidates. The itertools.product call is eager and could block the event loop. Consider adding a timeout or using a generator.

3. event.py - _sample_and_convert_ellipsoid imports inside function body

   from algorithm.geometry.Geometry import Geometry
   from data.Ellipsoid import Ellipsoid

   These imports should be at the top of the file, not inside the function. This is a performance issue (re-imported on every call) and a code smell.

Security:

4. api/api.py - Privacy gate returns 403 with correlation ID
   The proxy_config() endpoint returns _make_proxy_error(correlation_id, ...) which likely includes the correlation ID in the response. This leaks internal state. Return a generic 403 without details.

5. GeometricAssociator.py - No input validation on radar_data
   The process() method assumes rd["config"] and rd["detection"] exist. If malformed data is passed, this will raise KeyError. Add explicit validation.

---

Suggestions for Improvement

1. Add unit tests for new algorithms
   The PR adds significant new functionality but no tests. At minimum:

   • GeometricAssociator: Test with synthetic 3-radar geometry
   • EKFTracker: Test predict/update cycle with known trajectory
   • JIPDATracker: Test track initiation, maintenance, and deletion

2. Consider making the event loop interval configurable per-request
   The event.interval config is global. Different API items might want different polling rates.

3. JIPDATracker.py - Add logging for track lifecycle events
   Track creation, confirmation, and deletion should be logged for debugging.

4. GeometricAssociator.py - Add a timeout parameter
   The enumeration could be CPU-intensive. Add a configurable timeout to prevent blocking.

5. Frontend: Add loading indicators for settings changes
   The setMinRadarEllipsoids() function calls event_ellipsoid() which triggers a network request. Users might not realize settings changes trigger immediate data fetches.

6. config.yml - Add validation for new config fields
   The new config blocks have many numeric parameters. Consider adding schema validation or at least range checks.

---

Overall Verdict

Request Changes

The PR implements a substantial and well-designed feature set. However, there are several issues that should be addressed before merging:

1. Fix the import inside _sample_and_convert_ellipsoid - Move to top of file
2. Add input validation in GeometricAssociator.process() - Handle missing keys gracefully
3. Add unit tests for the three new algorithm classes - At minimum, basic smoke tests
4. Remove dead code valid['associators'] in api/api.py - Or add a comment explaining it's reserved for future use
5. Consider adding a timeout to GeometricAssociator enumeration - Or document the performance characteristics

The architectural decisions are sound (separating blind association from localisation, using the same schema as AdsbAssociator), and the frontend changes are well-implemented with localStorage persistence and immediate feedback. Once the above issues are addressed, this PR will be ready to merge.

---

Powered by DeepSeek API

[github-actions]

🤖 DeepSeek Code Review

This is a very large and complex pull request. It's not just a bug fix; it's a major feature addition that implements a complete non-cooperative (blind) target detection pipeline, including a Geometric Associator, EKF, and JIPDA tracker, alongside significant UI/UX improvements and configuration changes.

Here is my review structured as requested.

---

Summary

This PR introduces a massive set of changes, primarily focused on adding a "non-cooperative" target detection capability to the 3lips passive radar system. The core changes include:

1. New Backend Pipeline: Implementation of GeometricAssociator.py, EKFTracker.py, and JIPDATracker.py to detect and track targets that do not broadcast ADS-B.
2. Bug Fixes: Fixes for several critical bugs listed in TODO.md, including string mismatches in localisation methods, longitude wrapping in ecef2lla, a non-functional ellipsoid cache, and a hardcoded altitude.
3. Configuration Overhaul: A new config.yml structure with dedicated sections for the new tracker, associator, and non-cooperative features, plus a configurable event loop interval.
4. Frontend Enhancements: A new settings popup on the map with controls for altitude units, ellipsoid display thresholds, and toggles for cooperative/non-cooperative target visibility. The UI for non-cooperative targets is distinct (yellow markers, "Non-coop" labels).
5. API and Routing Changes: The API has been simplified by removing the associator selection from the URL, as the system now always uses the ADS-B associator for the primary path and the geometric associator as a parallel blind path. A privacy gate for radar site locations has been added.
6. Documentation: USER_GUIDE.md has been significantly updated to document all new features, configuration options, and the non-cooperative target workflow.

Code Quality and Best Practices

Issues Found:

1. GeometricAssociator.py - Inefficient Enumeration: The process method uses itertools.product to generate all M^N candidate tuples. While this is the core algorithm, the comment in the code and the max_detections guard suggest awareness of the combinatorial explosion. The current implementation builds the entire list of candidates in memory (list(itertools.product(...))). For even moderate values (e.g., 3 radars, 10 detections each = 1000 candidates), this is fine. However, for 4 radars and 20 detections (the guard limit), this becomes 160,000 candidates, which is a significant memory and CPU spike per epoch. The code should iterate over the product generator directly without materializing the entire list.

2. GeometricAssociator.py - Redundant Distance Calculation: In the geometric intersection test (Step 4), the code calculates np.min(np.linalg.norm(...)) for scoring after already checking np.any(dists < self.threshold). This is a double computation of the same distance matrix. The minimum distance can be stored during the initial check to avoid this redundancy.

3. JIPDATracker.py - Gating Implementation: The gating logic in JIPDATracker.process() is flawed. It calls self.ekf.update(self._copy_track(track), meas, cfgs) to get the NIS, but this is a full EKF update, not just an innovation calculation. The _copy_track method is a shallow copy, and the update method modifies the state and covariance. While the code comments say "Revert — we only wanted the NIS", the _copy_track function does not perform a deep copy of the numpy arrays. The update method will modify the copied arrays, which is fine, but the approach is conceptually wrong and computationally wasteful. A dedicated compute_nis method in EKFTracker would be cleaner and more efficient.

4. JIPDATracker.py - Greedy Association: The comment states "a greedy nearest-neighbour assignment is acceptably close" for the joint association step. This is a significant simplification of JIPDA. Greedy assignment can lead to suboptimal track management in dense target scenarios, potentially causing track swaps or missed associations. This is a known performance trade-off, but it should be clearly documented as a limitation.

5. event/event.py - Global State: The new tracker instances (ekf, jipda) are created as module-level globals. This makes the system stateful and difficult to test or reset cleanly. If the configuration changes, these objects would need to be re-initialized. A more robust design would encapsulate the event loop and its state within a class.

6. api/map/event/ellipsoid.js - Client-Side Filtering Logic: The logic for filtering ellipsoids based on localiseCooperativeTargets and the nc_ prefix is complex and duplicated in the event_radar.js file. This logic should be centralized, perhaps in a shared utility function or by having the backend send a flag indicating the target type.

Potential Bugs and Security Concerns

Issues Found:

1. api/api.py - Security Regression: The proxy_config endpoint now returns a 403 error when show_radar_sites is false. This is a good privacy feature. However, the event_loop still needs this data. The PR correctly notes that the event loop uses the config internally. The potential bug is that the frontend's main.js now checks config.map.show_radar_sites before fetching radar sites. If the config variable is not yet loaded or is undefined when the map loads, the radar site fetching will be skipped entirely, even if show_radar_sites is true. The code if (config && config.map && config.map.show_radar_sites !== false) is a good guard, but the loading order of config must be guaranteed.

2. GeometricAssociator.py - Doppler Sign Consistency: The Doppler sign-consistency filter (Step 5) requires all non-zero Doppler values to have the same sign. This is a strong assumption that may not hold for all geometries, especially with moving transmitters/receivers or in complex multipath environments. This could lead to valid targets being incorrectly filtered out. The doppler_tolerance config parameter is defined but not used in this filter; it's only used in the config definition. The filter should use the tolerance to allow for near-zero Doppler values or small sign inconsistencies.

3. JIPDATracker.py - Track Initiation Position: The track initiation uses the midpoint of the TX-RX pair as a rough initial position. This is a very poor initial guess for a target that could be hundreds of kilometers away. The EKF might diverge or take a long time to converge from such a bad initial state. A better approach would be to use the output of the GeometricAssociator (which has already done an intersection test) to get a better initial position estimate.

4. JIPDATracker.py - P_exist Update Formula: The P_exist update for missed detections (track['P_exist'] *= (1.0 - self.P_D) * 0.5) uses an arbitrary 0.5 factor. The standard JIPDA formula for a missed detection is P_exist = P_exist * (1 - P_D) / (1 - P_D * P_G). The current formula will cause tracks to decay much faster than intended, potentially deleting valid tracks after a short gap.

5. event/event.py - _sample_and_convert_ellipsoid Function: This function is defined inside event.py but imports Geometry and Ellipsoid inside the function body. This is a code smell and can lead to circular import issues or performance overhead if called frequently. These imports should be at the top of the file.

Suggestions for Improvement

1. Refactor GeometricAssociator.process(): Change candidates = list(itertools.product(...)) to iterate over the generator directly. Cache the minimum distance during the pairwise check to avoid redundant calculations.

2. Add compute_nis method to EKFTracker: Create a separate method that calculates the innovation and NIS without modifying the track's state or covariance. This will make the gating logic in JIPDATracker cleaner and more efficient.

3. Improve Track Initiation: Pass the best-matching sample points from the GeometricAssociator to the JIPDATracker to provide a much better initial position estimate for the EKF.

4. Fix P_exist Update: Replace the ad-hoc 0.5 factor in the missed detection update with the correct JIPDA formula.

5. Centralize Frontend Logic: Move the target type classification (cooperative vs. non-cooperative) and the associated styling logic into a shared JavaScript module to avoid duplication between ellipsoid.js and radar.js.

6. Add Unit Tests: The PR adds a lot of new, complex algorithmic code (GeometricAssociator, EKF, JIPDA) with almost no unit tests. The TODO.md lists many test cases that are still unchecked. These are critical for ensuring the correctness and stability of the new pipeline.

Overall Verdict

Request Changes

This PR introduces a highly valuable and ambitious feature set. However, the presence of several potential bugs (especially in the core tracking algorithms), significant code quality issues (inefficient enumeration, flawed gating logic), and a complete lack of unit tests for the new backend components makes it unsuitable for merging in its current state.

The author must address the following before this can be approved:

1. Fix the P_exist update formula in JIPDATracker.py.
2. Refactor the gating logic in JIPDATracker.py to use a dedicated NIS computation method.
3. Improve the track initiation position estimate.
4. Refactor GeometricAssociator.py to iterate over the product generator.
5. Add unit tests for the new GeometricAssociator, EKFTracker, and JIPDATracker classes. At a minimum, test the core logic (e.g., geometric intersection, EKF predict/update cycle, track initiation/deletion).

---

Powered by DeepSeek API

[github-actions]

🤖 DeepSeek Code Review

Pull Request Review

Summary

This is a large, multi-faceted PR that:

1. Fixes several confirmed bugs (A1-A5) in the TODO list
2. Implements major new features: GeometricAssociator (blind association), EKF tracker, JIPDA multi-target tracker
3. Adds non-cooperative target detection pipeline
4. Updates frontend with new settings UI, per-target coloring, and non-cooperative target display
5. Adds configuration options for all new features
6. Updates documentation

Code Quality and Best Practices

Issues Found:

1. Inconsistent code style in new files: GeometricAssociator.py uses np.linalg.norm with axis=2 but the variable naming and structure differs significantly from existing codebase conventions.

2. Missing type hints: New Python files (GeometricAssociator.py, EKFTracker.py, JIPDATracker.py) lack type hints, while the existing codebase appears to use them inconsistently.

3. Hardcoded magic numbers:

   • JIPDATracker.py line 241: 2000.0 meter gate for confirmation - should be configurable
   • JIPDATracker.py line 282: max_pending = max(50, self.max_tracks * 3) - magic number 50

4. Redundant imports: event/event.py imports numpy as np but doesn't appear to use it directly.

5. Dead code removal: The style_ellipsoid.color and style_point.color were removed from JS files, but the style_ellipsoid and style_point objects still exist with other properties.

6. Inconsistent error handling: GeometricAssociator._sample_ellipsoid() silently returns empty list on b_sq <= 0 - should log a warning.

Potential Bugs and Security Concerns

Issues Found:

1. Security: Radar site location privacy bypass: The proxy_config() endpoint returns 403 when show_radar_sites is false, but the radar site data is still fetched client-side in main.js (line 503+). The check happens server-side but the client-side code still attempts the fetch - the error is just caught silently. This is acceptable but could be improved.

2. Potential division by zero: EKFTracker.update() line 107-108 clamps r_tx and r_rx to 1e-3 but doesn't handle the case where both are zero simultaneously.

3. Race condition in JIPDA: The _pending_tracks list is modified during iteration (line 260: self._pending_tracks.remove(pending) while iterating). This could cause RuntimeError: list changed size during iteration.

4. Memory leak potential: JIPDATracker._pending_tracks can grow unboundedly - the cleanup logic (line 274-278) only removes by age, not by count cap. The hard cap at line 280-282 is after the append, so it could temporarily exceed max_pending.

5. Incorrect ellipsoid key parsing: ellipsoid.js line 68: var targetHex = isNoncooperative ? key.substring(3).split('-')[0] : key.split('-')[0] - this assumes the key format is always hex-radarName or nc_hex-radarName, but the backend generates synthetic IDs like aabbccdd (8 hex chars) for non-cooperative targets.

6. Missing input validation: GeometricAssociator.process() doesn't validate that radar_data contains the expected structure before accessing nested keys.

Suggestions for Improvement

1. Fix the race condition in JIPDATracker: Use a list comprehension or iterate over a copy:

self._pending_tracks = [p for p in self._pending_tracks if p not in to_remove]

2. Add logging: New files should have appropriate logging for debugging, especially in error paths.

3. Make magic numbers configurable: The 2000m confirmation gate and 50 max_pending should be config values.

4. Add unit tests: The PR marks many TODO items as complete but doesn't add corresponding unit tests. At minimum, add tests for:

   • GeometricAssociator with synthetic 3-radar data
   • EKF predict/update cycle
   • JIPDA track initiation and deletion

5. Fix the ellipsoid key parsing: Ensure consistent key format between backend and frontend, or add a more robust parsing method.

6. Consider performance: GeometricAssociator enumerates all M^N tuples. With max_detections=20 and 3 radars, that's 8000 candidates. The O(n²) pairwise distance check could be optimized with spatial indexing for production use.

Overall Verdict

Request Changes

The PR implements significant and valuable features, and the bug fixes are correct. However, there are several issues that need addressing:

1. Critical: Fix the race condition in JIPDATracker._pending_tracks iteration (will crash in production)
2. High: Fix ellipsoid key parsing in frontend - non-cooperative targets may not render correctly
3. Medium: Add unit tests for new functionality before marking TODO items as complete
4. Medium: Make hardcoded magic numbers configurable
5. Low: Add type hints and improve error handling in new files

The architectural decisions are sound (especially keeping the GeometricAssociator output schema identical to AdsbAssociator), and the documentation updates are thorough. Once the critical bugs are fixed and tests are added, this will be a strong addition to the codebase.

---

Powered by DeepSeek API

[github-actions]

🤖 DeepSeek Code Review

Summary

This is a large, well-structured PR that adds non-cooperative (ADS-B-free) target detection capability to the radar processing pipeline. The changes span:

• New files: GeometricAssociator.py, EKFTracker.py, JIPDATracker.py
• Bug fixes: String mismatch in parametric localisation ("minimum" → "min"), longitude wrapping fix, ellipsoid cache fix, deep copy fix, hardcoded altitude fix
• Frontend updates: Settings popup with toggles for cooperative/non-cooperative visibility, per-target ellipsoid coloring, non-cooperative target styling
• Configuration: New config blocks for geometric association, EKF, JIPDA, non-cooperative pipeline
• Documentation: Updated .clinerules, TODO.md, USER_GUIDE.md
• API changes: Removed associator selection from UI (always uses ADSB associator), added privacy gate for radar site locations

Code Quality and Best Practices

Issues Found:

1. Hardcoded magic numbers in GeometricAssociator (line 264): The above-ground check uses mid_alt directly without considering that z_rot is in ENU coordinates relative to the midpoint. This could incorrectly filter valid points at altitude.

2. Missing type hints: Several new methods lack type hints, particularly in GeometricAssociator.py and JIPDATracker.py. The project appears to use them elsewhere.

3. Inconsistent error handling: GeometricAssociator._sample_ellipsoid uses print() for warnings instead of proper logging. The project should use a consistent logging approach.

4. JIPDATracker greedy association (line 200-220): The comment acknowledges this is a simplification, but the greedy nearest-neighbor approach can produce suboptimal assignments in dense target scenarios. The TODO mentions full JPDA enumeration is O(N!) - consider Murty's algorithm or a more scalable approach.

5. Redundant computation in JIPDATracker: The _copy_track method is called just to compute NIS, then the copy is discarded. This is wasteful for large track counts.

6. Missing input validation: GeometricAssociator.process() doesn't validate that radar_list items actually exist in radar_data before iterating.

Potential Bugs and Security Concerns

Issues Found:

1. Potential division by zero in EKFTracker (lines 100-103): The check if r_tx < 1e-3 or r_rx < 1e-3 clamps to 1e-3, but this still produces a very large Jacobian value. If a target is extremely close to a radar site, the EKF update could become numerically unstable.

2. Race condition in JIPDATracker: The self.tracks dictionary is modified in-place during process(). If multiple threads call process() concurrently (e.g., for different API clients), this will cause data corruption. The TODO in TODO.md mentions "Thread-safe track store per API config item" but it's marked as handled by JIPDA - it's not.

3. GeometricAssociator memory usage: For N radars with M detections each, the sample_cache stores M×N arrays of size (nSamples, 3). With max_detections=20 and nSamples=50, this is 20×N×50×3 floats per epoch. For 5 radars, that's ~120KB - acceptable, but the itertools.product enumeration creates M^N tuples which could be enormous (20^5 = 3.2M candidates).

4. Missing rate limiting on proxy_config: The privacy gate returns 403, but there's no rate limiting on repeated requests to probe for radar site existence.

5. Frontend XSS potential: The sanitizeLabel function is used for ADS-B callsigns, but non-cooperative target labels use hardcoded "Non-coop" text - safe. However, the hashToHue function uses str.charCodeAt() which is safe.

Suggestions for Improvement

1. Add a candidate pruning step in GeometricAssociator: Before full enumeration, use a coarse grid or clustering to reduce the M^N search space. The current max_detections guard helps but is a blunt instrument.

2. Implement proper logging: Replace print() statements with Python's logging module throughout the new code.

3. Add unit tests for the new modules: The PR fixes several bugs but doesn't add tests for the new GeometricAssociator, EKFTracker, or JIPDATracker. At minimum, add:

   • GeometricAssociator: 3-radar synthetic geometry test
   • EKFTracker: Single-target tracking through 10 epochs
   • JIPDATracker: Multi-target association test

4. Consider numerical stability improvements in EKFTracker: Use np.linalg.solve instead of np.linalg.inv for the Kalman gain computation (line 120-121).

5. Add a maximum age for pending tracks in JIPDATracker: Unconfirmed pending tracks should be removed after some number of epochs to prevent memory leaks.

6. Document the event.interval config change: The polling interval changed from hardcoded 1.0s to configurable 0.5s default. This doubles HTTP load on radar nodes - should be documented more prominently.

Overall Verdict

Approve with the following caveats that should be addressed before or shortly after merge:

1. Critical: Add thread-safety documentation/guard for JIPDATracker (the race condition is real)
2. High: Add unit tests for the three new modules
3. Medium: Replace print() with proper logging
4. Medium: Add candidate pruning to GeometricAssociator for production deployments with >3 radars

The code is well-structured, follows the existing project patterns, and the architectural decision to keep the ADSB associator as primary while running the geometric associator in parallel is sound. The bug fixes in the existing code are correct and well-documented.

---

Powered by DeepSeek API

[github-actions]

🤖 DeepSeek Code Review

Summary

This is a large feature PR that adds non-cooperative (ADS-B-free) target detection and tracking to a passive radar system. The changes span documentation, configuration, backend processing, and frontend visualization:

• New files: GeometricAssociator.py, EKFTracker.py, JIPDATracker.py
• Modified files: Configuration, API, frontend JS/HTML, documentation, and several bug fixes
• Key features: Blind target association via geometric enumeration, EKF tracking, JIPDA multi-target tracking, frontend toggles for cooperative/non-cooperative target visibility, radar site privacy controls

Code Quality and Best Practices

Issues Found:

1. Incomplete JIPDATracker.py - The file ends abruptly mid-function (process method is incomplete). The _copy_track method is referenced but not defined. This will cause runtime errors.

2. Missing error handling in GeometricAssociator - The _sample_ellipsoid method catches b_sq <= 0 but silently continues with degenerate geometry. Should log a warning and skip the candidate.

3. Hardcoded values - EKFTracker.initiate() uses hardcoded position uncertainty (250000) and velocity uncertainty (2500) instead of configurable parameters.

4. No input validation - GeometricAssociator.process() doesn't validate that radar_list contains valid radar names before processing.

5. Inconsistent naming - JIPDATracker uses _pending_tracks but never implements the pending track logic for confirmation.

6. Missing docstrings - Several methods lack proper docstrings (e.g., JIPDATracker.process()).

Potential Bugs and Security Concerns

Issues Found:

1. Critical: Incomplete JIPDATracker implementation - The process method is cut off mid-function. The track initiation logic, P_exist update for new tracks, and output formatting are missing. This will crash at runtime.

2. Race condition in track store - JIPDATracker.tracks is a shared mutable dictionary. If multiple API clients use the same tracker instance, tracks will be corrupted. The TODO mentions "thread-safe track store per API config item" but this isn't implemented.

3. Security: Proxy config bypass - The proxy_config endpoint returns 403 when show_radar_sites is false, but the frontend still attempts to fetch radar configs. The error handling in main.js catches the error but doesn't prevent the request.

4. Potential division by zero - In EKFTracker.update(), r_tx and r_rx are clamped to 1e-3, but the Jacobian computation d_tx / r_tx could still produce large values if the target is very close to a radar site.

5. Memory leak - GeometricAssociator._sample_ellipsoid creates large numpy arrays on every call. No cleanup mechanism for stale sample caches.

6. Unvalidated configuration - max_detections guard in GeometricAssociator returns empty dict if exceeded, but doesn't log this event. Could mask configuration issues.

Suggestions for Improvement

1. Complete JIPDATracker implementation - Finish the process method with track initiation, P_exist updates for new tracks, and proper output formatting.

2. Add thread safety - Implement per-client track stores or add locks to prevent race conditions.

3. Add logging - Log warnings for degenerate geometry, skipped epochs, and track lifecycle events.

4. Make uncertainties configurable - Move hardcoded values in EKFTracker.initiate() to configuration.

5. Add input validation - Validate radar names exist in radar_data before processing.

6. Add unit tests - The TODO marks several test items as incomplete. At minimum, add tests for:

   • GeometricAssociator with synthetic 3-radar data
   • EKF predict/update cycle
   • JIPDA track initiation and deletion

7. Consider performance - The geometric enumeration in GeometricAssociator is O(M^N). With max_detections=20 and 3 radars, that's 8000 candidates. Consider adding early termination or sampling.

Overall Verdict

Request Changes

The PR introduces valuable functionality but has a critical defect: the JIPDATracker.py file is incomplete and will crash at runtime. Additionally, the thread-safety concern for the track store needs to be addressed before this can be considered production-ready.

Required changes:

1. Complete the JIPDATracker.process() method
2. Add thread-safe track store (or document the limitation)
3. Add basic unit tests for the new components

The documentation, configuration, and frontend changes are well-structured and appropriate. The bug fixes in existing code are correct and valuable.

---

Powered by DeepSeek API

[github-actions]

🤖 DeepSeek Code Review

Review: Pull Request Analysis

Summary

This is a large, well-structured pull request that adds non-cooperative (ADS-B-free) target detection capability to a passive radar system. The changes span multiple layers:

• New modules: GeometricAssociator.py, EKFTracker.py, JIPDATracker.py
• Configuration: New config blocks for geometric association, EKF/JIPDA tracking, and non-cooperative detection
• Bug fixes: Several confirmed defects fixed (string mismatches, longitude wrapping, cache issues, mutation bugs)
• Frontend: New settings UI, non-cooperative target rendering, toggle controls for cooperative/non-cooperative visibility
• Documentation: Updated .clinerules, TODO.md, USER_GUIDE.md
• API changes: Removed associator selection from UI (now always ADS-B), added non-cooperative output schema

Code Quality and Best Practices

Issues Found:

1. GeometricAssociator.py - Hardcoded print statement (line 261):

   print(f"Warning: GeometricAssociator: degenerate ellipsoid ...")

   Should use proper logging instead of print. In a production signal-processing pipeline, this will clutter stdout.

2. JIPDATracker.py - Bare except Exception (line 181):

   except Exception:
       continue

   This silently swallows all exceptions during NIS computation. At minimum, log the exception.

3. GeometricAssociator.py - _sample_ellipsoid returns mixed types:
   Returns list of lists, but sample_cache values are later converted with np.array(). The method should consistently return a numpy array for efficiency.

4. JIPDATracker.py - _copy_track method referenced but not defined (line 178):

   nis = self.ekf.update(self._copy_track(track), meas, cfgs)

   The method _copy_track is called but never implemented. This will raise AttributeError at runtime.

5. GeometricAssociator.py - Inefficient pairwise distance computation (lines 159-163):
   The nested loop recomputes np.linalg.norm for each pair. This is O(N²) with redundant computation. Could be vectorised.

6. config.yml - Inconsistent naming: associate.geometric vs tracker.ekf vs tracker.jipda. Consider namespacing consistently (e.g., tracker.ekf and tracker.jipda is fine, but associate.geometric should perhaps be associator.geometric).

Potential Bugs and Security Concerns

Critical:

1. JIPDATracker.py - Missing _copy_track method (line 178):
   This is a runtime crash bug. The method is called but never defined. The NIS computation will fail on every epoch when tracks exist.

2. GeometricAssociator.py - Unit mismatch in _sample_ellipsoid (line 244):

   bistatic_range_m = bistatic_range_ms * 299792458 / 1000

   The parameter is named bistatic_range_ms but the comment says "millimetres". The conversion factor 299792458 / 1000 suggests the input is in seconds (not milliseconds or millimetres). If delay from detections is in seconds (as implied by AdsbAssociator.py line 108: delay = (1000*radar_detections['delay'][i] + ...)/1000), then delay * 1000 at line 116 converts seconds to milliseconds, but then _sample_ellipsoid treats it as milliseconds and divides by 1000 again. This produces ellipsoids ~1000× too small.

   Wait - re-reading: the caller at line 116 does delay_ms = delay * 1000, so the parameter is indeed in milliseconds. But the comment says "millimetres" which is confusing. The conversion bistatic_range_ms * 299792458 / 1000 converts milliseconds to metres correctly. However, the parameter name bistatic_range_ms is misleading - it should be bistatic_range_ms (milliseconds) not "millimetres". The comment on line 233 says "millimetres" which is wrong.

3. api/api.py - Removed associator validation (lines 107-110):

   # associators_api = request.args.getlist('associator')
   # if not all(item in valid['associators'] for item in associators_api):
   #     return 'Invalid associator'

   The associator parameter is no longer validated. While the frontend always passes adsb-associator, a direct API call could pass an invalid or malicious associator ID. This is a minor regression in input validation.

Moderate:

4. GeometricAssociator.py - No timeout on enumeration (line 88):

   candidates = list(itertools.product(*indexed_lists))

   With max_detections=20 and 3 radars, this generates 8000 candidates. With 5 radars, it's 3.2 million. The max_detections guard helps, but there's no CPU time limit. A malicious radar node returning 20 detections could cause significant processing delay.

5. JIPDATracker.py - Greedy association may miss optimal assignments (lines 189-200):
   The greedy nearest-neighbour approach is documented as a simplification, but it can produce suboptimal assignments when tracks cross. This is a known limitation but should be documented more prominently.

Suggestions for Improvement

1. Implement _copy_track in JIPDATracker.py - This is blocking the PR from working correctly.

2. Add logging throughout - Replace print statements with Python's logging module. Add debug logging for association decisions.

3. Add unit tests - The PR marks many TODO items as completed, but no new tests are added. At minimum:

   • GeometricAssociator: Test with 3-radar synthetic data
   • EKFTracker: Test predict/update cycle with known trajectory
   • JIPDATracker: Test track initiation, maintenance, and deletion

4. Add input validation - Validate noncooperative.match_distance is positive, tracker.jipda.P_D is in [0,1], etc.

5. Consider thread safety - The JIPDATracker.tracks dict is mutated in-place. If multiple API clients share the same tracker instance (unclear from the diff), this could cause race conditions.

6. Document the _copy_track issue - Even after fixing, add a comment explaining why a copy is needed (to avoid mutating the real track during gating).

Overall Verdict

Request Changes

The PR introduces significant valuable functionality (non-cooperative target detection, EKF tracking, JIPDA multi-target tracking) and fixes several confirmed bugs. However, the missing _copy_track method in JIPDATracker.py is a critical runtime bug that will crash the event loop on every epoch when tracks exist. Additionally:

• The unit mismatch confusion in GeometricAssociator._sample_ellipsoid needs clarification (parameter naming/comment)
• The bare except Exception should be replaced with specific exception handling
• Unit tests should be added for the new modules

Once these issues are addressed, this PR will be ready for approval. The architectural decisions (parallel blind path, schema compatibility with existing localisation code, frontend toggle design) are sound and well-documented.

---

Powered by DeepSeek API

[github-actions]

🤖 DeepSeek Code Review

Pull Request Review

Summary

This is a large feature PR implementing non-cooperative (ADS-B-free) target detection and tracking. Key changes include:

• New GeometricAssociator for blind cross-radar detection association
• New EKFTracker and JIPDATracker for multi-target tracking
• Frontend UI for toggling cooperative/non-cooperative target visibility
• Configuration updates for the new pipeline
• Bug fixes for existing code (string mismatches, longitude wrapping, cache issues)
• Documentation updates

Code Quality and Best Practices

Issues Found:

1. Missing error handling in GeometricAssociator._sample_ellipsoid: The method catches b_sq <= 0 but doesn't handle the case where ellipsoid itself might be invalid or midpoint_lla is None.

2. Hardcoded values: The JIPDATracker.process() method uses hardcoded dt = 1.0 for new tracks instead of using the configured event.interval value.

3. Inconsistent naming: JIPDATracker uses self._pending_tracks (underscore prefix) but never actually uses it in the process() method. This appears to be dead code.

4. Missing type hints: Several new methods lack type hints, particularly in GeometricAssociator.py and JIPDATracker.py.

5. Print statements for debugging: GeometricAssociator._sample_ellipsoid uses print() for warnings instead of proper logging.

6. Incomplete docstring: The JIPDATracker.__init__ docstring references confirmation_gate parameter but it's not documented.

Potential Bugs and Security Concerns

Issues Found:

1. Race condition in JIPDATracker: The self.tracks dictionary is modified in-place across multiple method calls without any thread safety. If the event loop processes multiple API clients concurrently, tracks could be corrupted.

2. Memory leak in JIPDATracker: There's no mechanism to clean up self._pending_tracks list. If candidates are added but never confirmed, this list grows unbounded.

3. Infinite loop potential: In GeometricAssociator.process(), if max_detections is set very high and there are many detections, itertools.product could generate an enormous number of candidates, potentially causing memory exhaustion.

4. Security concern - config exposure: The proxy_config endpoint now returns 403 when show_radar_sites is false, but the error message reveals the reason ("Radar site locations are not publicly available"), which could be information leakage.

5. Missing input validation: The GeometricAssociator doesn't validate that radar_data contains valid ECEF coordinates before using them in calculations.

6. Potential division by zero: In EKFTracker.update(), the check if r_tx < 1e-3 or r_rx < 1e-3 clamps to 1e-3 but doesn't handle the case where both are zero simultaneously.

Suggestions for Improvement

1. Add thread safety: Implement a lock or use thread-safe data structures for the track store in JIPDATracker.

2. Add input validation: Validate radar config data before processing, especially ECEF coordinates.

3. Use logging instead of print: Replace print() statements with proper Python logging.

4. Add memory management: Implement a cleanup mechanism for _pending_tracks and add a maximum age limit.

5. Add rate limiting for candidate generation: In GeometricAssociator, add a hard cap on total candidates to prevent memory exhaustion.

6. Improve error messages: The 403 error in proxy_config should be more generic to avoid information leakage.

7. Add unit tests: The PR marks many TODO items as complete but doesn't include corresponding unit tests for the new functionality.

8. Consider using configuration for default values: Replace hardcoded dt = 1.0 in JIPDATracker with the configured event.interval.

Overall Verdict

Request Changes

The PR introduces significant new functionality that appears architecturally sound, but there are several critical issues that need addressing:

1. Thread safety in JIPDATracker is essential for production use
2. Memory management for _pending_tracks needs implementation
3. Input validation is missing for critical geometric calculations
4. Unit tests are needed for the new components (many TODO items marked complete but untested)

The core design is good - the separation of concerns between association, tracking, and localisation is clean. However, the implementation needs hardening before it's production-ready.

---

Powered by DeepSeek API

[github-actions]

🤖 DeepSeek Code Review

Looking at this pull request, I'll provide a thorough review of the changes.

Summary

This is a substantial feature addition implementing non-cooperative (ADS-B-free) target detection and tracking for a passive radar system. The changes include:

1. New files: GeometricAssociator.py, EKFTracker.py, JIPDATracker.py
2. Configuration updates: New config blocks for geometric association, EKF/JIPDA tracking, non-cooperative targets, and event loop interval
3. Frontend enhancements: Settings popup with toggles for cooperative/non-cooperative target visibility, ellipsoid fade time, and minimum radar count
4. Bug fixes: Several confirmed defects marked as fixed (string mismatches, longitude wrapping, cache issues, etc.)
5. Documentation: Updated .clinerules, TODO.md, USER_GUIDE.md
6. API changes: Removed associator selection from UI (always uses ADS-B associator), added non-cooperative output schema
7. Security: Privacy gate for radar site locations (show_radar_sites config)

Code Quality and Best Practices

Issues Found:

1. GeometricAssociator.py - Missing import and potential issue with _sample_ellipsoid:

   • The method uses np.meshgrid with indexing='ij' but then accesses .flat which may not iterate in the expected order. Consider using .ravel() or explicit flattening.

2. GeometricAssociator.py - Hardcoded speed of light:

   • Line 249: bistatic_range_m = bistatic_range_ms * 299792458 / 1000 - The comment says "milliseconds to metres" but the variable name bistatic_range_ms suggests milliseconds. The conversion factor 299792458 / 1000 converts milliseconds to metres (since c = 299,792,458 m/s). This is correct but the variable naming is confusing - it's actually in milliseconds, not metres.

3. JIPDATracker.py - Unused import:

   • from scipy.stats import chi2 is imported but never used in the code.

4. JIPDATracker.py - Incomplete diff:

   • The diff cuts off mid-function at line 334, making it impossible to review the complete implementation.

5. api/api.py - Hardcoded associator:

   • Line 107: The associator parameter is completely removed from the API, but the comment says "The AdsbAssociator is always used as the primary associator." This removes user flexibility.

6. Frontend - ellipsoid.js - Potential performance issue:

   • The hashToHue function is called for every point in every ellipsoid, which could be expensive for large datasets. Consider caching the hue per target ID.

7. config.yml - Missing validation:

   • No schema validation for the new config values (e.g., noncooperative.match_distance could be negative).

Potential Bugs and Security Concerns

Issues Found:

1. GeometricAssociator.py - Potential division by zero:

   • Line 249: bistatic_range_m = bistatic_range_ms * 299792458 / 1000 - If bistatic_range_ms is 0, this produces a valid result, but the ellipsoid becomes degenerate (a = distance/2). The check if b_sq <= 0 handles this, but only prints a warning - it doesn't raise an error or handle the degenerate case gracefully.

2. GeometricAssociator.py - Memory issue with large candidate sets:

   • Line 82: candidates = list(itertools.product(*indexed_lists)) - For N radars with M detections each, this creates M^N tuples. With max_detections=20 and 3 radars, that's 8000 candidates, which is manageable. But with 4 radars and 20 detections each, it's 160,000 candidates, which could be memory-intensive.

3. EKFTracker.py - Numerical stability:

   • Line 107: np.linalg.inv(S) - The innovation covariance matrix S could be singular or near-singular, especially with few measurements or poor observability. Consider using np.linalg.lstsq or adding regularization.

4. JIPDATracker.py - Thread safety:

   • The self.tracks dictionary is modified in-place without any locking mechanism. If the event loop processes multiple API clients concurrently, this could lead to race conditions.

5. Security - Config exposure:

   • The show_radar_sites feature returns a 403 error, which is good. However, the error message "Radar site locations are not publicly available" might reveal too much information. Consider a generic "Not Found" or "Forbidden" response.

6. AdsbAssociator.py - Clamping delay to 0:

   • Line 111: radar_detections['delay'][i] = max(0.0, delay) - While clamping negative delays is physically correct, this could mask bugs in the extrapolation logic. Consider logging when clamping occurs.

Suggestions for Improvement

1. Add unit tests for new modules:

   • The TODO.md marks several items as completed but the corresponding unit tests are not added. Specifically:
     • GeometricAssociator: 3-radar synthetic test, empty detection test
     • EKFTracker: 10-epoch simulated trajectory test
     • JIPDATracker: Track maintenance through gaps, track termination tests

2. Improve error handling in GeometricAssociator:

   • Add a try-except block around the main processing loop to handle unexpected errors gracefully.
   • Consider adding a timeout for the enumeration step to prevent CPU starvation.

3. Add logging throughout new modules:

   • The existing codebase uses print() statements for warnings. Consider using Python's logging module for better control over log levels.

4. Optimize GeometricAssociator._sample_ellipsoid:

   • The method creates a full meshgrid and then iterates over all points. For large n values, this could be slow. Consider vectorizing the above-ground check.

5. Add configuration validation:

   • Create a schema validation function that checks all config values at startup (e.g., noncooperative.match_distance > 0, tracker.jipda.P_D between 0 and 1).

6. Consider adding a rate limiter for the event loop:

   • The new event.interval config allows faster polling (0.5s). This could overwhelm radar nodes. Consider adding a minimum interval or a rate-limiting mechanism.

7. Document the JIPDA algorithm parameters:

   • The gamma parameter (16.27) is hardcoded for 3-DOF chi² at P_G=0.999. This should be documented or made configurable based on the actual measurement dimension.

Overall Verdict

Request Changes

While this is a well-structured and substantial feature addition that addresses several important capabilities (non-cooperative target detection, multi-target tracking, privacy controls), there are several issues that need to be addressed before merging:

1. Critical: The JIPDATracker.py diff is incomplete - the full implementation cannot be reviewed.
2. High: Missing unit tests for the new modules (marked as completed in TODO.md but not present in the diff).
3. Medium: Thread safety concerns with shared track state.
4. Medium: Numerical stability in EKF update (potential singular matrix inversion).
5. Low: Several code quality issues (unused imports, confusing variable names, potential performance bottlenecks).

The changes to fix confirmed bugs (A1-A5) and the security improvements (radar site privacy) are excellent and should be retained. However, the new feature code needs additional hardening, testing, and review before it's production-ready.

---

Powered by DeepSeek API

[github-actions]

🤖 DeepSeek Code Review

Summary

This is a large, multi-faceted pull request that adds non-cooperative (ADS-B-free) target detection and tracking to a passive radar system. The changes span:

1. New algorithms: GeometricAssociator (blind detection association), EKFTracker, and JIPDATracker
2. Configuration: New config blocks for geometric association, EKF, JIPDA, non-cooperative targets, and event loop interval
3. Bug fixes: Several confirmed defects fixed (string mismatches, longitude wrapping, cache issues, in-place mutation, hardcoded altitude)
4. Frontend: New settings UI (cooperative/non-cooperative toggles, ellipsoid fade time, radar count threshold), per-target coloring, non-cooperative target rendering
5. Documentation: Updated .clinerules, TODO.md, USER_GUIDE.md with new pipeline details
6. Security: Privacy gate for radar site locations (show_radar_sites: false returns 403)
7. API changes: Removed associator selection from UI (always uses adsb-associator), updated output schema

---

Code Quality and Best Practices

Issues Found

1. GeometricAssociator.py - Missing docstring for _sample_ellipsoid return type (line 244): The docstring says "return list" but the actual return is list[list[float]] (list of ECEF points). Consider being more explicit.

2. GeometricAssociator.py - Hardcoded print statement (line 268): print(f"Warning: GeometricAssociator: degenerate ellipsoid...") should use Python's logging module instead of print for production code.

3. JIPDATracker.py - radar_names variable defined but unused (line ~130): radar_names = list(radar_data.keys()) is assigned but never referenced.

4. EKFTracker.py - math import unused (line 17): import math is imported but never used in the file.

5. ellipsoid.js - hashToHue function (line 135): The hash = hash & hash; operation is a no-op in JavaScript (bitwise AND with itself). This appears to be a C-style 32-bit truncation that doesn't work in JS. Should use hash = hash >>> 0; for unsigned 32-bit truncation.

6. ellipsoid.js - style_ellipsoid.color removed (line 127): The global style_ellipsoid.color was removed but style_ellipsoid is still referenced in the addPoint call for cooperative ellipsoids (line 113). This is fine since color is now computed per-target, but the removal could break other code that references style_ellipsoid.color.

7. radar.js - style_point.color removed (line 158): Same pattern — style_point.color was removed but style_point is still used for pointSize and type. This is fine but worth noting.

8. config.yml - Inconsistent comment style: Some comments use # with space, others don't. Minor but inconsistent.

---

Potential Bugs and Security Concerns

Issues Found

1. GeometricAssociator.py - Potential integer overflow in hash (line 258): hashlib.sha256(str(delays_tuple).encode()).hexdigest()[:8] — using only 8 hex characters (32 bits) for synthetic IDs creates a collision probability of ~2⁻¹⁶ after ~65K candidates (birthday paradox). For a system processing thousands of candidates per epoch, collisions are possible. Consider using at least 12 hex characters (48 bits).

2. GeometricAssociator.py - _sample_ellipsoid unit conversion (line 262): The comment says "Convert bistatic range from milliseconds to metres" but the variable is named bistatic_range_ms and the conversion is bistatic_range_ms * 299792458 / 1000. If bistatic_range_ms is actually in seconds (as suggested by the comment delay * 1000 on line 218), this would produce ellipsoids ~1000× too large. This is a critical bug — the calling code passes delay * 1000 (seconds → ms), but the function parameter is named bistatic_range_ms. The conversion * 299792458 / 1000 would then convert ms → m, which is correct. However, the comment says "milliseconds" while the actual value is in milliseconds. The code is correct but the naming is misleading — bistatic_range_ms should be renamed to bistatic_range_mm (milliseconds → the variable stores milliseconds, not metres).

3. JIPDATracker.py - Missing thread safety (line 60): self.tracks is a plain dict with no locking. If the event loop processes multiple API clients concurrently (as suggested by "Per-client thread-safe track store" in TODO.md), this will cause race conditions.

4. api.py - Removed associator validation (line 110-112): The associator validation was removed entirely. While the comment says "associator is always adsb-associator", this means any invalid associator value in the URL is silently ignored. Consider at least logging a warning.

5. api.py - proxy_config returns 403 for hidden sites (line 485): This is good for privacy, but the error message "Radar site locations are not publicly available" leaks that the system has radar sites. Consider a generic "Not found" or "Forbidden" message.

6. config.yml - event.interval: 0.5 (line 70): Reducing the event loop interval from 1.0s to 0.5s doubles the HTTP load on radar nodes. This should be documented as a performance consideration.

---

Suggestions for Improvement

1. Fix the hashToHue function in ellipsoid.js to properly truncate to 32-bit:

   hash = (hash * 33 + c) | 0; // force 32-bit integer

   Or use hash = Math.imul(hash, 33) + c; for proper 32-bit multiplication.

2. Add logging instead of print in GeometricAssociator.py:

   import logging
   logger = logging.getLogger(__name__)
   # ...
   logger.warning(f"Degenerate ellipsoid (b_sq={b_sq:.1f}) for radar {ellipsoid.name}")

3. Increase synthetic ID length in GeometricAssociator.py from 8 to 12 hex characters to reduce collision probability.

4. Rename bistatic_range_ms to bistatic_range_mm (or bistatic_range_ms_val) in _sample_ellipsoid to clarify units.

5. Add thread safety to JIPDATracker.py using threading.Lock if concurrent access is expected.

6. Remove unused imports: math from EKFTracker.py, radar_names from JIPDATracker.py.

7. Consider adding a test for the hashToHue function to ensure stable coloring across polling cycles.

8. Document the event.interval performance impact in the config file comments.

---

Overall Verdict

Request Changes

The PR is well-structured and adds significant functionality, but there are two issues that need addressing before merge:

1. Critical: The hashToHue function in ellipsoid.js has a no-op bitwise operation (hash = hash & hash;) that doesn't actually truncate to 32-bit. This means the hash can grow unbounded, potentially producing inconsistent hues across polling cycles for the same target. Fix to hash = (hash * 33 + c) | 0; or use Math.imul.

2. Important: The bistatic_range_ms parameter name in _sample_ellipsoid is misleading — it stores milliseconds but the name suggests metres. While the conversion logic is correct, this naming confusion could lead to future bugs. Rename to clarify units.

3. Moderate: The synthetic ID collision probability with only 8 hex characters is a real concern for systems processing many candidates. Increase to 12 characters.

4. Minor: Remove unused imports and variables, replace print with logging.

Once these issues are addressed, this PR should be approved. The architectural decisions (parallel blind path, same output schema, client-side filtering) are sound and well-documented.

---

Powered by DeepSeek API

[github-actions]

🤖 DeepSeek Code Review

Summary

This is a large, multi-file PR that adds a complete non-cooperative (ADS-B-free) target detection pipeline to a passive radar system. Key changes include:

• New files: GeometricAssociator.py, EKFTracker.py, JIPDATracker.py
• Config additions: noncooperative, tracker, event.interval, map.show_radar_sites, 3lips.save_retention_hours
• Bug fixes: "minimum" → "min" string mismatch, ecef2lla longitude wrapping, ellipsoid cache, ADS-B associator mutation, hardcoded 100m altitude
• Frontend updates: Settings popup with toggles for cooperative/non-cooperative visibility, per-target ellipsoid coloring, non-cooperative styling
• Documentation: Updated .clinerules, TODO.md, USER_GUIDE.md
• API changes: Removed associator selection from UI (always uses adsb-associator), added detections_noncooperative to output schema

---

Code Quality and Best Practices

Issues Found

1. GeometricAssociator.py - Missing docstring for _sample_ellipsoid parameter bistatic_range_ms (line 238)

   • The docstring says "bistatic range in millimetres" but the parameter name says ms (milliseconds). The code converts delay * 1000 (seconds → milliseconds) before calling. This is confusing and inconsistent.

2. GeometricAssociator.py - Hardcoded print() for degenerate ellipsoids (line 262)

   • Uses print() instead of Python's logging module. In a production signal-processing pipeline, this should use structured logging.

3. JIPDATracker.py - Incomplete gating logic (line 330, diff truncated)

   • The gating loop appears incomplete in the diff. The for j loop body ends abruptly with if not meas: continue and then # Only gat — the rest is missing from the diff.

4. api/api.py - Removed associator validation but kept valid['associators'] initialization (line 80-81)

   • The valid['associators'] list is still populated but never used for validation. Dead code.

5. api/map/event/ellipsoid.js - hashToHue uses hash & hash for 32-bit truncation (line 143)

   • This is a JavaScript idiom, but hash = hash & hash is a no-op in modern JS (bitwise ops already truncate to 32-bit). The comment says "force 32-bit int" but this doesn't actually do that — hash is already a 64-bit float. Use hash = hash | 0 or hash >>>= 0 for proper truncation.

6. api/map/event/ellipsoid.js - removeEntitiesOlderThanAndFade called twice in same condition (lines 93-96)

   • Both the if (ellipsoidCount < threshold) and else branches call the same fade/remove functions. The logic is duplicated.

---

Potential Bugs and Security Concerns

Issues Found

1. GeometricAssociator.py - _sample_ellipsoid unit conversion bug (line 253-254)

   • The method receives bistatic_range_ms which is delay * 1000 (seconds → milliseconds). But the conversion to metres uses bistatic_range_ms * 299792458 / 1000. This is wrong: if bistatic_range_ms is in milliseconds, the conversion should be bistatic_range_ms * 299792458 / 1000 which gives metres. However, the comment says "Convert bistatic range from milliseconds to metres" — but the input is actually delay in seconds × 1000, not milliseconds. The caller passes delay * 1000 where delay is in seconds. So bistatic_range_ms is actually in milliseconds (seconds × 1000). The conversion ms * c / 1000 is correct for ms → m. However, the .clinerules file (line 484) says this was a bug fixed in June 2026 — but the code still has the same pattern. This needs verification.

2. GeometricAssociator.py - No validation of radar_data config structure (lines 155-165)

   • The code accesses config['location']['tx']['latitude'] etc. without checking if these keys exist. If a radar config is malformed, this will throw KeyError.

3. api/api.py - Removed associator validation but associator parameter still accepted (line 107)

   • The associator query parameter is no longer validated, but it's still accepted from the URL. An attacker could pass ?associator=malicious and it would be silently ignored. While not a security issue per se, it's inconsistent.

4. api/map/main.js - config variable used before declaration (line 510)

   • if (config && config.map && config.map.show_radar_sites !== false) — the config variable is not defined in this scope. It appears to be a global, but there's no guarantee it's initialized before map.on('load', ...) fires.

5. config/config.yml - event.interval: 0.5 changes default polling rate (line 73)

   • The previous hardcoded value was 1.0 seconds. Reducing to 0.5 doubles HTTP load on radar nodes. This should be documented as a breaking change for deployments with many radars.

---

Suggestions for Improvement

1. Add input validation in GeometricAssociator.process() for radar config structure before accessing nested keys.

2. Use logging instead of print() in GeometricAssociator._sample_ellipsoid().

3. Fix hashToHue in ellipsoid.js to use hash >>>= 0 for proper 32-bit truncation.

4. Remove dead code in api/api.py — either remove valid['associators'] or keep it for future use with a comment.

5. Add unit tests for the new files. The TODO.md marks several test items as incomplete (e.g., "Add unit test: synthetic 3-radar epoch...", "Unit test: 10-epoch simulated trajectory..."). These should be added before merging.

6. Document the event.interval change more prominently — halving the polling interval doubles the load on radar nodes and the event loop.

7. Consider thread safety in JIPDATracker — the TODO.md mentions "Thread-safe track store per API config item (JIPDA handles this)" but the current implementation uses instance variables (self.tracks, self._pending_tracks) without any locking. If multiple API clients share the same tracker instance, this is a race condition.

---

Overall Verdict

Request Changes

The PR introduces significant new functionality (non-cooperative target pipeline) and fixes several critical bugs. However, there are issues that need addressing:

1. The GeometricAssociator._sample_ellipsoid unit conversion needs verification — the .clinerules claims this was a fixed bug, but the code pattern looks identical to the bug description.

2. The JIPDATracker gating logic is incomplete in the diff — the for j loop body is truncated.

3. Missing input validation in GeometricAssociator for radar config structure.

4. No thread safety in JIPDATracker despite TODO claiming it's handled.

5. Missing unit tests for the new pipeline components.

6. The hashToHue function has a non-functional 32-bit truncation.

These issues should be resolved before merging. The architectural decisions are sound, the documentation is thorough, and the bug fixes are correct.

---

Powered by DeepSeek API

[github-actions]

🤖 DeepSeek Code Review

I'll review this pull request diff. This is a large PR that adds non-cooperative target detection, EKF/JIPDA tracking, and various improvements to the 3lips passive radar system.

Summary

This PR implements a major feature addition: non-cooperative (ADS-B-free) target detection alongside the existing ADS-B-based pipeline. Key changes include:

1. GeometricAssociator - Blind cross-radar detection association via geometric enumeration
2. EKFTracker - Extended Kalman Filter for single-target tracking
3. JIPDATracker - Joint Integrated Probabilistic Data Association multi-target tracker
4. Frontend updates - Non-cooperative target visualization (yellow markers), settings popup with toggles for cooperative/non-cooperative visibility
5. Bug fixes - Fixed "minimum" → "min" string mismatch, longitude wrapping, ellipsoid cache, ADS-B data mutation, hardcoded altitude
6. Configuration - New config blocks for geometric association, EKF, JIPDA, non-cooperative pipeline
7. Documentation - Updated user guide, .clinerules, TODO.md

Code Quality and Best Practices

Issues Found:

1. GeometricAssociator.py - Inefficient pairwise distance computation (line ~170-180)

   • The nested loops compute np.linalg.norm twice per pair (once for pass/fail, once for scoring)
   • Could compute once and reuse

2. GeometricAssociator.py - Missing type hints

   • The class uses dict/list without generic type parameters (e.g., dict[str, list])
   • While not required, this would improve readability for such a complex class

3. JIPDATracker.py - Large method complexity

   • process() method is ~200 lines with deep nesting
   • Consider extracting gating, association probability computation, and track management into separate methods

4. Frontend - Global state management

   • window.showCooperativeTargets, window.localiseCooperativeTargets, etc. are polluting global namespace
   • Consider using a module pattern or configuration object

5. Config file - Inconsistent naming

   • associate.geometric vs tracker.ekf vs noncooperative - some use dots, some use underscores
   • Consider consistent naming convention

6. GeometricAssociator.py - Hardcoded print statement (line ~250)

   • print(f"Warning: GeometricAssociator: degenerate ellipsoid...") should use logging instead

Potential Bugs and Security Concerns

Issues Found:

1. GeometricAssociator.py - Potential division by zero (line ~240)

   b_sq = a * a - (ellipsoid.distance / 2) ** 2

   If ellipsoid.distance is 0, this is fine, but if a is very small, b_sq could be negative (handled) or zero (fine). However, math.sqrt(b_sq) on line 243 could receive a very small negative due to floating point. The check b_sq <= 0 should use a small epsilon.

2. JIPDATracker.py - Missing import (line ~30)

   • from algorithm.geometry.Geometry import Geometry is imported but Geometry.lla2ecef is called on line ~180
   • This works but is fragile - the import should be explicit

3. Security: API proxy config endpoint (api.py line ~478)

   • The proxy_config endpoint now returns 403 when show_radar_sites is false
   • This is good for privacy, but the error message "Radar site locations are not publicly available" could leak information about the configuration
   • Consider a generic "Not available" message

4. GeometricAssociator.py - No input validation on radar_data

   • The process() method assumes radar_data[radar_name] has config with location.tx.latitude, etc.
   • If any radar has malformed config, this will throw KeyError/AttributeError

5. JIPDATracker.py - Track ID collision risk

   • self.next_track_id is a simple incrementing counter
   • If the tracker is re-initialized or multiple instances exist, IDs could collide

6. Frontend - localStorage key collision

   • Keys like 3lips_minRadarEllipsoids could collide with other applications using similar prefixes
   • Consider using a more unique prefix or namespace

Suggestions for Improvement

1. Add logging instead of print statements throughout the new code
2. Extract JIPDATracker.process() into smaller methods (e.g., _gate_candidates(), _compute_association_probabilities(), _update_tracks())
3. Add unit tests for the new components (GeometricAssociator, EKFTracker, JIPDATracker) - the TODO.md shows many items still unchecked
4. Consider adding a configuration validation step at startup to catch malformed config early
5. Add a small epsilon to the b_sq <= 0 check in GeometricAssociator
6. Consider using a proper logging framework instead of print statements
7. The frontend settings popup could benefit from a more structured approach (e.g., a settings object rather than individual global variables)

Overall Verdict

Approve with minor suggestions

This is a well-structured, substantial feature addition that follows the existing codebase patterns. The code is generally clean, well-documented, and addresses several known bugs. The main concerns are:

• The JIPDATracker.process() method is too large and should be refactored
• Some edge cases in GeometricAssociator (floating point, division by zero) need attention
• Missing unit tests for the new components (though these are tracked in TODO.md)

The PR correctly implements the architecture described in the project documentation and maintains backward compatibility with the existing ADS-B pipeline. The frontend changes are well-thought-out with proper localStorage persistence and immediate visual feedback.

Recommended actions before merging:

1. Add epsilon to b_sq <= 0 check in GeometricAssociator
2. Replace print statements with logging
3. Consider extracting JIPDATracker.process() into smaller methods
4. Add at least basic unit tests for the new components (even if minimal)

---

Powered by DeepSeek API