Skip to content

fix: run rotation/TDS detection independent of display moment availability - #617

Merged
full-bars merged 4 commits into
mainfrom
fix/radar-rotation-detection-on-missing-moment
Jul 26, 2026
Merged

fix: run rotation/TDS detection independent of display moment availability#617
full-bars merged 4 commits into
mainfrom
fix/radar-rotation-detection-on-missing-moment

Conversation

@full-bars

@full-bars full-bars commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Summary

  • CodeRabbit follow-up from feat: /radar — NEXRAD loops with storm-scale zoom, rotation/TVS detection, and PTDS debris-signature scoring #615: in crates/radar_gif/src/main.rs, when build_moment_cache returned None for a scan (the requested display product, e.g. zdr/kdp/phidp, was unavailable), the continue short-circuited rotation/TVS and TDS detection for that scan too — even though detect_rotation_sites runs off velocity/dual-pol moments that are unrelated to the display product.
  • Detection (detect_rotation_sites, TDS colocation sampling, and the all_detections/latest_rotation/tds_timeline accumulators) now runs unconditionally per volume, before the display cache is attempted. Only the frame image render — which does need the display moment — is skipped on a cache miss.

Test plan

  • cargo build --release -p radar_gif
  • cargo fmt --all -- --check
  • cargo clippy -p radar_gif --all-targets --all-features -- -D warnings
  • cargo test -p radar_gif --no-default-features

Summary by CodeRabbit

  • New Features

    • Rotation-site detections and TDS confidence data are captured even when the requested display product can’t be rendered for a volume.
    • Rotation marker overlays now use precomputed detection results for more consistent output.
    • Repeated tilts are deduplicated when rotation analysis is enabled.
  • Bug Fixes

    • Improved rotation and “latest rotation” tracking across volumes.
    • Rotation sidecar outputs are preserved even when no frames are rendered, avoiding missing detections.
    • Volumes that can’t be rendered are skipped without losing available analysis data.

…ility

Previously, if the requested display product (e.g. zdr/kdp/phidp) was
unavailable on a scan, build_moment_cache's None short-circuited the
entire volume iteration via continue, silently skipping rotation and
TDS detection for that scan too — even though detect_rotation_sites
runs off velocity/dual-pol moments unrelated to the display product.

Detection now runs unconditionally per volume before the display cache
is built; only frame rendering (which does need the display moment) is
skipped on cache miss.
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: bb4bf6d7-3563-4687-87d2-c91cc912166d

📥 Commits

Reviewing files that changed from the base of the PR and between d7efe3b and ffa9eb7.

📒 Files selected for processing (1)
  • crates/radar_gif/src/main.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/radar_gif/src/main.rs

📝 Walkthrough

Walkthrough

Rotation and TDS detection now run before per-volume rendering, allowing data to persist when rendering is skipped. Rotation markers reuse precomputed sites, and rotation sidecar generation occurs before the no-frame exit.

Changes

Rotation detection flow

Layer / File(s) Summary
Pre-render detection and marker reuse
crates/radar_gif/src/main.rs
Per-volume rotation sites and TDS confidence are computed before rendering, detection state is updated, and marker rendering reuses the precomputed sites.
Sidecar generation before frame exit
crates/radar_gif/src/main.rs
Rotation sidecar generation runs before the empty-frame check, while GIF encoding retains its existing no-frame handling.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant VolumeLoop
  participant RotationDetector
  participant TDSGrid
  participant Renderer
  participant RotationSidecar
  VolumeLoop->>RotationDetector: Detect rotation sites
  RotationDetector->>TDSGrid: Sample strongest-site confidence
  TDSGrid-->>VolumeLoop: Return TDS confidence
  VolumeLoop->>VolumeLoop: Record detections and latest rotation
  VolumeLoop->>Renderer: Render using precomputed rotation sites
  VolumeLoop->>RotationSidecar: Write recorded detection data
  RotationSidecar-->>VolumeLoop: Complete before no-frame exit
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: rotation/TDS detection now runs independently of display moment availability.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/radar-rotation-detection-on-missing-moment

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/radar_gif/src/main.rs`:
- Around line 221-224: Move the rotation sidecar serialization in the main
processing flow to occur before the empty-frame early exit, including the path
reached when build_moment_cache returns None for every volume. Preserve the
existing rotation detection results and ensure rotation.json is written even
when no renderable frames are produced.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e18e7fed-468c-4406-a8c4-e8c5f1653fdd

📥 Commits

Reviewing files that changed from the base of the PR and between 131d4d7 and d7efe3b.

📒 Files selected for processing (1)
  • crates/radar_gif/src/main.rs

Comment thread crates/radar_gif/src/main.rs
Moves rotation serialization ahead of the no-frames-rendered exit so
detection results are preserved even when no displayable moment exists.
@full-bars
full-bars merged commit 586e547 into main Jul 26, 2026
10 checks passed
@full-bars
full-bars deleted the fix/radar-rotation-detection-on-missing-moment branch July 26, 2026 03:40
@full-bars full-bars mentioned this pull request Jul 28, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant