feat: /radar — NEXRAD loops with storm-scale zoom, rotation/TVS detection, and PTDS debris-signature scoring#615
Merged
Merged
Conversation
Creates a new rust crate (radar_gif) that fetches NEXRAD Level II volumes from the public AWS S3 archive, decodes and renders them using BowEcho's nexrad_io and render2d crates, and outputs animated GIFs. The Python cog (cogs/radar.py) exposes a /radar slash command with site, product, and frames parameters.
The radar renderer was hardcoded to a fixed 460km range mapped onto an 800x800 canvas, making mesocyclone couplets and tornadic debris signatures impossible to resolve. Switched to render2d's viewport rendering API with a configurable --range-km flag and a Discord zoom choice (Storm-Scale/Regional/Wide/Full Range), and applied smooth_moment_grid() to fix the streaky/gapped look at closer zoom. Also fixes a compass rose typo (W was showing as F) and adds a backing panel behind the timestamp/site label so it stays legible over storm echoes. State outlines, city labels, and range rings now scale correctly with the selected zoom level.
…oring for /radar Adds BowEcho's operational rotation-detection algorithm as an always-on overlay on /radar loops, with per-frame markers and a loop-wide peak Vrot/ΔV summary (fixes a bug where only the last frame's detections were ever reported, hiding real rotation that had already weakened by the end of a loop). Also adds PTDS, a new /radar product combining reflectivity, CC, and ZDR into a single debris-signature confidence score, sampled at each scan's strongest couplet and reported scan-by-scan. PTDS is explicitly marked experimental in the Discord output itself. Fixes along the way: GIF frames were not chronologically sorted; TDWR/SAILS volumes rescan the same low elevation multiple times, which was starving the rotation detector of the vertical tilt diversity it needs; rotation markers and geography were misaligned from the actual radar pixel data by about 1%; marker and corner labels had no backing panel and were unreadable over bright echoes; the compass rose mislabeled west as "F".
Rotation markers now get full ring+label treatment only for the top 6 sites by Vrot (matching the embed's cap); weaker circulations beyond that draw as small unlabeled dots instead of piling up overlapping rings and labels at storm-scale zoom. Also adds autocomplete on the site parameter, matching against both the 4-letter code and city name from the existing site tables.
No behavior changes — reformatting plus is_none_or/lifetime-elision/ too-many-arguments fixes so the crate passes CI's fmt/clippy gates.
Dead code — superseded by the Rust imageproc-based state outline rendering in radar_gif, never used at runtime, and the Cartopy pre-render approach it implements was abandoned for being both too slow on the ARM server and low quality.
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a Rust-based NEXRAD radar GIF renderer with PTDS/TDS and rotation overlays, integrates it into ChangesRadar GIF rendering
Project metadata and governance
Estimated code review effort: 5 (Critical) | ~90+ minutes Sequence Diagram(s)sequenceDiagram
participant DiscordUser
participant RadarCommand
participant radar_gif
participant RadarData
participant Discord
DiscordUser->>RadarCommand: submit /radar request
RadarCommand->>radar_gif: run site, product, frames, zoom, rotation
radar_gif->>RadarData: fetch realtime and archived volumes
RadarData-->>radar_gif: return radar volumes
radar_gif-->>RadarCommand: write GIF and rotation sidecar
RadarCommand->>Discord: upload embed and generated GIF
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
crates/radar_gif/src/main.rs (1)
1207-1217: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
encode_gifpanics via.expect()instead of the file's usual graceful-exit pattern.Every other fallible step in
main(decode/download/detect failures) is handled viaeprintln!+std::process::exit(1), butencode_gifuses four.expect()calls that will panic (exit code 101, stack unwind message on stderr) on file-creation or encoding failure. Functionally this still surfaces as a non-zero exit to the calling Discord cog (context snippet 2 checksproc.returncode != 0), so it's not a correctness bug, just an inconsistency in error-handling style.🤖 Prompt for 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. In `@crates/radar_gif/src/main.rs` around lines 1207 - 1217, Update encode_gif to follow main’s graceful error-handling pattern instead of panicking through its four expect calls: report file-creation, repeat-configuration, and frame-encoding failures with eprintln! and terminate via std::process::exit(1), while preserving the existing GIF encoding flow.
🤖 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 @.wiki:
- Line 1: Add a .gitmodules configuration entry mapping the existing .wiki
gitlink to its repository URL, using the appropriate submodule path and matching
the repository’s expected wiki remote so git submodule update --init can
initialize it.
In `@CHANGELOG.md`:
- Line 11: Update the NEXRAD Radar Loops product list in the changelog to
include spectrum width alongside the existing supported products, using the
project’s established naming style.
In `@cogs/radar/__init__.py`:
- Around line 565-569: Update the subprocess cleanup around proc.communicate()
to handle both asyncio.TimeoutError and task cancellation: kill the renderer,
then await proc.communicate() or proc.wait() to reap it before re-raising the
original timeout or cancellation. Preserve the existing timeout error message
and ensure cleanup itself does not mask the triggering exception.
- Around line 438-445: Update the render flow around _run_radar_cli so each
request writes to a unique temporary GIF path based on an interaction ID or
UUID, rather than directly to the shared RADAR_GIF_CACHE path. After rendering
completes, atomically publish the temporary GIF and corresponding .rotation.json
metadata to the shared cache, preserving cache reuse while preventing concurrent
requests from interleaving writes.
- Line 184: Update periodic_cleanup to include RADAR_GIF_CACHE alongside
OUTPUT_DIR when removing expired files, ensuring both rendered GIFs and rotation
sidecars are cleaned up while preserving the existing cleanup behavior.
In `@crates/radar_gif/src/main.rs`:
- Around line 87-132: Deduplicate collected radar volumes by volume_time before
final playback ordering. In the volume assembly flow around volumes and the
existing sort_by_key call, sort by v.volume_time first, then remove adjacent
duplicate timestamps with dedup_by_key, preserving the current oldest-to-newest
ordering.
- Around line 169-190: Decouple the per-volume detection flow from display
rendering: in the loop around build_moment_cache and render_moment_rgba_into,
treat unavailable display moments or invalid image buffers as visual-frame skips
only, then continue into the existing rotation/PTDS block for every volume.
Ensure detection still uses the independent rotation velocity and
lowest_dual_pol_cut data and updates all_detections and tds_timeline even when
no radar_img is produced.
- Around line 76-81: Update the site coordinate lookup in the main
radar-processing flow to emit an eprintln! warning when site_coords lacks the
requested site before using the existing default coordinates. Preserve the
current fallback coordinates and all downstream geo_to_pixel calculations for
recognized and unrecognized sites.
---
Nitpick comments:
In `@crates/radar_gif/src/main.rs`:
- Around line 1207-1217: Update encode_gif to follow main’s graceful
error-handling pattern instead of panicking through its four expect calls:
report file-creation, repeat-configuration, and frame-encoding failures with
eprintln! and terminate via std::process::exit(1), while preserving the existing
GIF encoding flow.
🪄 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: 8de84e26-bbc6-453d-a441-0562ad7a9911
⛔ Files ignored due to path filters (3)
Cargo.lockis excluded by!**/*.lockcrates/radar_gif/assets/fonts/DejaVuSans.ttfis excluded by!**/*.ttfcrates/radar_gif/assets/geo/nexrad_sites.csvis excluded by!**/*.csv
📒 Files selected for processing (12)
.gitignore.wikiCHANGELOG.mdCODE_OF_CONDUCT.mdCREDITS.mdCargo.tomlREADME.mdcogs/radar/__init__.pycrates/radar_gif/Cargo.tomlcrates/radar_gif/assets/geo/us_states_110m.jsoncrates/radar_gif/assets/geo/us_states_50m.jsoncrates/radar_gif/src/main.rs
| @@ -0,0 +1 @@ | |||
| Subproject commit 869e0f950b9d4f8d6caf7f1ef2edd3e6c0de2431 | |||
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-tree HEAD .wiki
git -C .wiki rev-parse HEAD
git -C .wiki cat-file -e 869e0f950b9d4f8d6caf7f1ef2edd3e6c0de2431^{commit}
git -C .wiki show --stat --oneline 869e0f950b9d4f8d6caf7f1ef2edd3e6c0de2431Repository: full-bars/spc-bot
Length of output: 433
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- .gitmodules ---\n'
if [ -f .gitmodules ]; then
cat -n .gitmodules
else
echo 'missing'
fi
printf '\n--- submodule status ---\n'
git submodule status --recursive || true
printf '\n--- .wiki git metadata ---\n'
git -C .wiki rev-parse --git-dir
git -C .wiki config --get remote.origin.url || true
git -C .wiki config --get remote.origin.fetch || true
git -C .wiki status --short --branch || trueRepository: full-bars/spc-bot
Length of output: 246
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- .gitmodules ---'
if [ -f .gitmodules ]; then
cat -n .gitmodules
else
echo 'missing'
fi
echo
echo '--- submodule status ---'
git submodule status --recursive || true
echo
echo '--- .wiki git metadata ---'
git -C .wiki rev-parse --git-dir
git -C .wiki config --get remote.origin.url || true
git -C .wiki config --get remote.origin.fetch || true
git -C .wiki status --short --branch || trueRepository: full-bars/spc-bot
Length of output: 430
Add the .gitmodules entry for .wiki
The tree contains a gitlink at .wiki, but there’s no .gitmodules mapping for it, so git submodule update --init can’t initialize the wiki checkout.
🤖 Prompt for 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.
In @.wiki at line 1, Add a .gitmodules configuration entry mapping the existing
.wiki gitlink to its repository URL, using the appropriate submodule path and
matching the repository’s expected wiki remote so git submodule update --init
can initialize it.
Comment on lines
+169
to
+190
| for volume in &volumes { | ||
| let cache = match build_moment_cache(volume, is_ptds, &moment, &color_tables) { | ||
| Some(c) => c, | ||
| None => { | ||
| eprintln!(" Skipping volume: product unavailable"); | ||
| continue; | ||
| } | ||
| }; | ||
| let mut pixels = vec![0u8; viewport_rgba_buffer_len(viewport_options)]; | ||
| let radar_img = match cache.render_moment_rgba_into(volume, viewport_options, &mut pixels) { | ||
| Ok((rw, rh)) => match RgbaImage::from_raw(rw, rh, pixels) { | ||
| Some(img) => img, | ||
| None => { | ||
| eprintln!(" Skipping volume: buffer size mismatch"); | ||
| continue; | ||
| } | ||
| }, | ||
| Err(e) => { | ||
| eprintln!(" Skipping volume: {e}"); | ||
| continue; | ||
| } | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Rotation/PTDS detection is skipped entirely when the display product's moment is unavailable for a scan.
build_moment_cache failing (e.g. requested zdr/kdp/phidp missing on a cut) causes continue at Line 174/183/188, which bypasses the whole rest of the loop body — including the if args.rotation { ... } block (346-424) that performs mesocyclone/TVS detection and PTDS/TDS sampling. Since rotation velocity data and the TDS dual-pol cut (lowest_dual_pol_cut) are independent of the user's selected display moment, a single unavailable display-moment scan silently drops that scan from all_detections/tds_timeline, degrading the accuracy of the loop-wide peak Vrot/ΔV summary and PTDS timeline described as a core feature of this PR — with no warning to the user beyond a generic "Skipping volume" line.
Consider decoupling: run rotation/TDS detection unconditionally per volume (independent of build_moment_cache), and only skip the visual frame when the display moment is unavailable.
🐛 Sketch of decoupling rotation/TDS from moment-cache availability
for volume in &volumes {
+ if args.rotation {
+ // rotation/TDS detection independent of the selected display moment
+ let deduped = dedup_repeated_tilts(volume);
+ let sites = detect_rotation_sites(&deduped);
+ // ... TDS sampling + all_detections/tds_timeline bookkeeping ...
+ }
let cache = match build_moment_cache(volume, is_ptds, &moment, &color_tables) {
Some(c) => c,
None => {
eprintln!(" Skipping volume: product unavailable");
continue;
}
};
- ...
- if args.rotation {
- // (moved above)
- }Also applies to: 346-424
🤖 Prompt for 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.
In `@crates/radar_gif/src/main.rs` around lines 169 - 190, Decouple the per-volume
detection flow from display rendering: in the loop around build_moment_cache and
render_moment_rgba_into, treat unavailable display moments or invalid image
buffers as visual-frame skips only, then continue into the existing
rotation/PTDS block for every volume. Ensure detection still uses the
independent rotation velocity and lowest_dual_pol_cut data and updates
all_detections and tds_timeline even when no radar_img is produced.
…ing, dedup) - periodic_cleanup now also prunes RADAR_GIF_CACHE, which was never cleaned and grew unbounded (only /download's OUTPUT_DIR was covered). - Output paths are now unique per interaction. They were never actually reused as a cache (every call regenerates), so the shared filename was pure collision risk between concurrent requests, not a cache hit. - The renderer subprocess is now awaited after kill() on timeout/ cancellation instead of just signaled, avoiding leaked zombie processes on repeated timeouts. - Realtime and archive-fill volumes are deduplicated by timestamp before sorting, since the archive's most recent object can overlap the already-fetched realtime volume and produce a visually "stuck" duplicate frame. - An unrecognized site now logs a warning instead of silently substituting a default lat/lon (Python-side validation already blocks this in practice, but the CLI had no diagnostic of its own). - CHANGELOG's product list was missing spectrum width.
Broken pointer left over from an unfinished attempt to embed the wiki as a submodule -- no .gitmodules entry existed to resolve it, so it was just dead weight in the tree. Doesn't touch the actual GitHub wiki. Also gitignored .wiki/ so a local wiki checkout doesn't reintroduce this.
sounderpy's acars_data module imports bs4 directly but never declares it, relying on it being pulled in transitively. A CI image rebuild stopped bringing it in, breaking every sounding-related test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/radarcommand renders animated NEXRAD Level II loops (reflectivity, velocity, ZDR,CC, KDP, PHIDP, spectrum width) via BowEcho's Rust crates, with a
zoomoption(Storm-Scale ~75km through Full Range ~460km) using proper viewport rendering instead of
a fixed full-range disk — makes couplets and debris signatures actually resolvable.
loop-wide peak Vrot/ΔV summary (not just the last frame), and site autocomplete matching
on both ICAO code and city name.
reflectivity/CC/ZDR into a 0–100% debris-signature score, sampled at each scan's
strongest couplet and reported scan-by-scan. Explicitly marked experimental in the
Discord output itself (footer + inline disclaimer) — not validated against a case archive.
volumes' repeated low-tilt rescans were starving the rotation detector of the vertical
diversity it needs; markers/geography were ~1% misaligned from the actual radar pixels;
several legibility bugs (unlabeled compass direction, no backing panel behind labels).
Test plan
cargo fmt --all -- --checkandcargo clippy --all-targets --all-features -- -D warningsclean across the workspaceruff check/ruff format --checkclean oncogs/radar/test_radar_views.py,test_radar_cleanup.py,test_radar_downloads.py)KLWX, KRLX), including real-time testing against an actual tornado near KPBZ/TPIT
Python cog paths (autocomplete, PTDS formatting, rotation embed) — flagging honestly,
not blocking
Summary by CodeRabbit
/radar, including site autocomplete, selectable products/moments, zoom-based range control, and configurable frame counts.beautifulsoup4to prevent import errors during ACARS-related functionality.