feat: add NHC tropical cyclone auto-posting#618
Conversation
Routes TCV/TCD/TWD/TCU products from IEMBot botstalk feed and NWWS XMPP to a new TropicalCog. Posts formatted embeds with storm classification, name, summary, and product type to a Discord channel. Uses dev channel for testing, production channel when ready.
post_tropical_product crashed on every real NHC product because parsed["summary"] is None (not missing) for most product types, and dict.get(key, default) doesn't apply the default when the key is present with a None value. No tropical product had ever successfully posted before this fix. Also switches from the hardcoded dev channel to a configurable TROPICAL_CHANNEL_ID (defaults to the existing prod channel), with a state-store override hook for future runtime configuration.
TCP (the actual Public Advisory, containing 'ADVISORY NUMBER X') was mislabeled PROBABILITIES, and TCV (the Watch/Warning summary product) was mislabeled ADVISORY. Confirmed live: Hurricane Fausto Advisory 14 and Tropical Storm Bertha Advisory 12 both posted as 'NHC PROBABILITIES' instead of 'NHC ADVISORY'. Corrected per NHC's product descriptions (nhc.noaa.gov/aboutnhcprod.shtml).
The summary block only stopped at a marker Public Advisories never contain, so it silently captured everything to the end of the product (watches/warnings, full discussion, hazards, next advisory, signature) and posted it all inline in the channel. Now bounded to the next section header (all-caps line + dashed underline), matching NHC's own document structure. Also adds a discussion thread on the channel message carrying the full raw product text, matching the SPC Outlook/MD thread convention — the channel message stays a short summary.
|
Warning Review limit reached
Next review available in: 16 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a Tropical Discord cog that parses and posts NHC products, routes products from IEMBot and NWWS feeds, supports configurable channels and threaded full text, and adds tests and documentation for the new behavior. ChangesNHC Tropical Product Auto-Posting
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant IEMBotCog
participant NWWS
participant TropicalCog
participant DiscordChannel
participant DiscussionThread
IEMBotCog->>TropicalCog: Submit tropical product
NWWS->>TropicalCog: Submit tropical product
TropicalCog->>DiscordChannel: Post summary embed
DiscordChannel->>DiscussionThread: Create discussion thread
TropicalCog->>DiscussionThread: Post full product text
🚥 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: 6
🧹 Nitpick comments (2)
cogs/nwws.py (1)
31-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winProduct log path hardcoded instead of using the configured cache directory.
config.pyalready exposes a configurable cache directory (cache_file_dir/CACHE_DIR) and a parallelnwws_firehose_logconfig entry for the raw firehose dump. This new handler instead hardcodes the literal"cache/nwws_products.log", so if an operator changesCACHE_DIR, this log ends up in a different, unconfigured location (and a missingcache/directory would raise at handler-creation time, which — since it's swallowed by the caller's bareexcept: pass— would fail silently forever).🤖 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 `@cogs/nwws.py` around lines 31 - 49, Update _ensure_product_log to derive the RotatingFileHandler path from the configured cache directory (cache_file_dir/CACHE_DIR) and the configured product-log filename, rather than hardcoding "cache/nwws_products.log". Ensure the configured directory exists before creating the handler so initialization does not silently fail when it is absent.config.py (1)
105-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded channel-ID fallback breaks the established config pattern.
Every other optional channel (
dev_channel_id,sounding_channel_id, etc.) falls back through_optional_int(...)to another configured env key.tropical_channel_idinstead bakes a literal production Discord snowflake directly into source. This is inconsistent with the rest of the file and is awkward for anyone self-hosting/forking this bot — the default silently points at a channel ID they don't own, with no configured chain to fall back to.♻️ Suggested fix
- "tropical_channel_id": int(os.getenv("TROPICAL_CHANNEL_ID") or 981540312688230420), + "tropical_channel_id": _optional_int("TROPICAL_CHANNEL_ID", "SPC_CHANNEL_ID"),🤖 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 `@config.py` at line 105, Replace the literal fallback in the tropical_channel_id configuration with the established _optional_int(...) pattern, using the appropriate configured environment-key fallback consistent with dev_channel_id and sounding_channel_id. Preserve integer parsing while ensuring self-hosted deployments do not silently default to a hardcoded production channel ID.
🤖 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 declaration for the .wiki submodule, specifying its
path and repository URL, and verify that the gitlink’s pinned commit exists in
the configured remote so fresh clones and CI can initialize it successfully.
In `@cogs/nwws.py`:
- Around line 617-628: Update the NWWS tropical dispatch in the handler
containing `post_tropical_product` so it does not pass the raw `pil_prefix` code
as the third argument. Pass no resolved prefix (or otherwise let
`post_tropical_product` invoke `_classify_product(product_id)`), while
preserving the existing tropical PIL detection and NWWS source value.
In `@cogs/tropical.py`:
- Around line 37-46: Reorder STORM_TYPE_ORDER so specific storm types precede
the generic terms they contain: place “MAJOR HURRICANE” before “HURRICANE” and
“SUBTROPICAL DEPRESSION” before “TROPICAL DEPRESSION.” Preserve the existing
classifications and ordering of unrelated storm types.
- Around line 228-233: Update the full-text posting flow around full_text_embed
so parsed["raw_text"] is delivered in chunks that respect Discord’s
4096-character embed description limit, rather than slicing it to the first 4096
characters. Send the chunks to the existing target (thread or channel) in order
and preserve the full product text, including its tail.
- Around line 59-69: Update _extract_storm_name so it returns only the first
token after the matched storm type, rather than the entire remaining line.
Preserve the existing trimming and title-casing behavior, ensuring headlines
such as “HURRICANE FAUSTO ADVISORY NUMBER 14” produce “Fausto” while simple
names like “HURRICANE ANNA” still work.
- Around line 169-193: Update post_tropical_product to use the supplied raw_text
when available instead of unconditionally calling _fetch_nhc_product. Extract
the response-text parsing logic from _fetch_nhc_product into a reusable
_parse_nhc_text helper, parse raw_text directly, and retain the fetch path only
when raw_text is absent.
---
Nitpick comments:
In `@cogs/nwws.py`:
- Around line 31-49: Update _ensure_product_log to derive the
RotatingFileHandler path from the configured cache directory
(cache_file_dir/CACHE_DIR) and the configured product-log filename, rather than
hardcoding "cache/nwws_products.log". Ensure the configured directory exists
before creating the handler so initialization does not silently fail when it is
absent.
In `@config.py`:
- Line 105: Replace the literal fallback in the tropical_channel_id
configuration with the established _optional_int(...) pattern, using the
appropriate configured environment-key fallback consistent with dev_channel_id
and sounding_channel_id. Preserve integer parsing while ensuring self-hosted
deployments do not silently default to a hardcoded production channel ID.
🪄 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 Plus
Run ID: 3ca705dd-4ba6-4a09-930f-815e2fae0c6c
📒 Files selected for processing (12)
.env.example.wikiCHANGELOG.mdCONTRIBUTING.mdPROJECT_STRUCTURE.mdcogs/__init__.pycogs/iembot.pycogs/nwws.pycogs/tropical.pyconfig.pyconfig/products.jsontests/test_tropical.py
| @@ -0,0 +1 @@ | |||
| Subproject commit 869e0f950b9d4f8d6caf7f1ef2edd3e6c0de2431 | |||
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
git config -f .gitmodules --get-regexp '^submodule\..*\.(path|url)$'
git ls-tree HEAD .wiki
git -C .wiki rev-parse --verify 869e0f950b9d4f8d6caf7f1ef2edd3e6c0de2431^{commit}Repository: full-bars/spc-bot
Length of output: 392
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '## root files'
git ls-files .gitmodules .wiki || true
echo
echo '## .gitmodules'
if [ -f .gitmodules ]; then
cat -n .gitmodules
else
echo '(missing)'
fi
echo
echo '## tree entry for .wiki'
git ls-tree HEAD .wiki
echo
echo '## submodule metadata from index'
git submodule status -- .wiki || true
echo
echo '## commit object in repo'
git cat-file -t 869e0f950b9d4f8d6caf7f1ef2edd3e6c0de2431 || trueRepository: full-bars/spc-bot
Length of output: 585
Add a .gitmodules entry for .wiki
A gitlink alone won’t initialize in fresh clones or CI; declare the submodule path/url in .gitmodules and ensure the pinned commit is available from that remote.
🤖 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 declaration for the .wiki submodule,
specifying its path and repository URL, and verify that the gitlink’s pinned
commit exists in the configured remote so fresh clones and CI can initialize it
successfully.
| elif any( | ||
| afos_pil.startswith(x) for x in ("TCV", "TCD", "TWD", "TCU", "TWO", "TCE", "TCP") | ||
| ): | ||
| tropical_cog = self.bot.get_cog("Tropical") | ||
| if tropical_cog: | ||
| nhc_pils = ("TCV", "TCD", "TWD", "TCU", "TWO", "TCE", "TCP") | ||
| pil_prefix = next(p for p in nhc_pils if afos_pil.startswith(p)) | ||
| await tropical_cog.post_tropical_product( | ||
| product_id, raw_text, pil_prefix, source="NWWS" | ||
| ) | ||
| logger.info(f"Triggered NHC {pil_prefix} via XMPP") | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
NWWS path passes the raw AFOS PIL as pil_prefix, bypassing the product-name mapping the PR set out to fix.
post_tropical_product uses pil_prefix or _classify_product(product_id) for the display label — i.e. it expects pil_prefix, when supplied, to already be the resolved human-readable name (that's exactly what route_from_product_id in cogs/tropical.py does: pil_prefix = _classify_product(product_id)). Here pil_prefix is set to the raw 3-letter code (e.g. "TCP") and passed straight through, so product_type becomes the literal string "TCP" instead of "ADVISORY". Every NWWS-sourced tropical post will show the raw PIL in its title/thread name, not the corrected label the CHANGELOG describes.
🐛 Suggested fix — don't pass the raw PIL; let it resolve via `_classify_product`
elif any(
afos_pil.startswith(x) for x in ("TCV", "TCD", "TWD", "TCU", "TWO", "TCE", "TCP")
):
tropical_cog = self.bot.get_cog("Tropical")
if tropical_cog:
nhc_pils = ("TCV", "TCD", "TWD", "TCU", "TWO", "TCE", "TCP")
pil_prefix = next(p for p in nhc_pils if afos_pil.startswith(p))
await tropical_cog.post_tropical_product(
- product_id, raw_text, pil_prefix, source="NWWS"
+ product_id, raw_text, source="NWWS"
)
logger.info(f"Triggered NHC {pil_prefix} via XMPP")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| elif any( | |
| afos_pil.startswith(x) for x in ("TCV", "TCD", "TWD", "TCU", "TWO", "TCE", "TCP") | |
| ): | |
| tropical_cog = self.bot.get_cog("Tropical") | |
| if tropical_cog: | |
| nhc_pils = ("TCV", "TCD", "TWD", "TCU", "TWO", "TCE", "TCP") | |
| pil_prefix = next(p for p in nhc_pils if afos_pil.startswith(p)) | |
| await tropical_cog.post_tropical_product( | |
| product_id, raw_text, pil_prefix, source="NWWS" | |
| ) | |
| logger.info(f"Triggered NHC {pil_prefix} via XMPP") | |
| elif any( | |
| afos_pil.startswith(x) for x in ("TCV", "TCD", "TWD", "TCU", "TWO", "TCE", "TCP") | |
| ): | |
| tropical_cog = self.bot.get_cog("Tropical") | |
| if tropical_cog: | |
| nhc_pils = ("TCV", "TCD", "TWD", "TCU", "TWO", "TCE", "TCP") | |
| pil_prefix = next(p for p in nhc_pils if afos_pil.startswith(p)) | |
| await tropical_cog.post_tropical_product( | |
| product_id, raw_text, source="NWWS" | |
| ) | |
| logger.info(f"Triggered NHC {pil_prefix} via XMPP") |
🤖 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 `@cogs/nwws.py` around lines 617 - 628, Update the NWWS tropical dispatch in
the handler containing `post_tropical_product` so it does not pass the raw
`pil_prefix` code as the third argument. Pass no resolved prefix (or otherwise
let `post_tropical_product` invoke `_classify_product(product_id)`), while
preserving the existing tropical PIL detection and NWWS source value.
| def _extract_storm_name(text: str) -> str: | ||
| lines = text.splitlines() | ||
| for i, line in enumerate(lines): | ||
| upper = line.upper() | ||
| for t in STORM_TYPE_ORDER: | ||
| if t in upper: | ||
| parts = upper.split(t, 1) | ||
| if len(parts) > 1: | ||
| name = parts[1].strip().strip(".").strip() | ||
| return name.title() | ||
| return None |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Storm-name extraction captures the whole remainder of the line, not just the name.
Real NHC headlines look like HURRICANE FAUSTO ADVISORY NUMBER 14 (see the FAUSTO_TCP fixture in tests/test_tropical.py). Splitting on the matched storm-type token and title-casing everything after it yields "Fausto Advisory Number 14" as the extracted "storm name," which then pollutes the embed title and the thread name for essentially every real advisory. The existing test only checks the trivial no-trailing-text case ("HURRICANE ANNA"), so this doesn't get caught.
🐛 Suggested fix — take only the first token after the matched type
if t in upper:
parts = upper.split(t, 1)
if len(parts) > 1:
- name = parts[1].strip().strip(".").strip()
- return name.title()
+ remainder = parts[1].strip().strip(".").strip()
+ if remainder:
+ return remainder.split()[0].title()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _extract_storm_name(text: str) -> str: | |
| lines = text.splitlines() | |
| for i, line in enumerate(lines): | |
| upper = line.upper() | |
| for t in STORM_TYPE_ORDER: | |
| if t in upper: | |
| parts = upper.split(t, 1) | |
| if len(parts) > 1: | |
| name = parts[1].strip().strip(".").strip() | |
| return name.title() | |
| return None | |
| def _extract_storm_name(text: str) -> str: | |
| lines = text.splitlines() | |
| for i, line in enumerate(lines): | |
| upper = line.upper() | |
| for t in STORM_TYPE_ORDER: | |
| if t in upper: | |
| parts = upper.split(t, 1) | |
| if len(parts) > 1: | |
| remainder = parts[1].strip().strip(".").strip() | |
| if remainder: | |
| return remainder.split()[0].title() | |
| return None |
🤖 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 `@cogs/tropical.py` around lines 59 - 69, Update _extract_storm_name so it
returns only the first token after the matched storm type, rather than the
entire remaining line. Preserve the existing trimming and title-casing behavior,
ensuring headlines such as “HURRICANE FAUSTO ADVISORY NUMBER 14” produce
“Fausto” while simple names like “HURRICANE ANNA” still work.
| full_text_embed = discord.Embed( | ||
| title=f"Full Text — {product_type}", | ||
| description=parsed["raw_text"][:4096], | ||
| color=discord.Color.dark_gray(), | ||
| ) | ||
| target = thread or channel |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Full product text silently truncated at 4096 chars, defeating the "full text in thread" goal.
Discord's embed description hard limit is 4096 characters. TWD/TCD discussion products routinely exceed that. Slicing parsed["raw_text"][:4096] avoids a Discord API error but silently drops the tail of the product (including, potentially, the forecaster signature or later sections) with no indication to readers that anything was cut.
♻️ Suggested fix — chunk the full text across multiple embeds/messages
- full_text_embed = discord.Embed(
- title=f"Full Text — {product_type}",
- description=parsed["raw_text"][:4096],
- color=discord.Color.dark_gray(),
- )
- target = thread or channel
- try:
- await target.send(embed=full_text_embed)
- except Exception as e:
- logger.warning(f"Failed to post full text for {product_id}: {e}")
+ target = thread or channel
+ raw = parsed["raw_text"]
+ chunks = [raw[i : i + 4000] for i in range(0, len(raw), 4000)] or [""]
+ for idx, chunk in enumerate(chunks):
+ title = f"Full Text — {product_type}" + (f" ({idx + 1}/{len(chunks)})" if len(chunks) > 1 else "")
+ try:
+ await target.send(embed=discord.Embed(title=title, description=chunk, color=discord.Color.dark_gray()))
+ except Exception as e:
+ logger.warning(f"Failed to post full text chunk {idx} for {product_id}: {e}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| full_text_embed = discord.Embed( | |
| title=f"Full Text — {product_type}", | |
| description=parsed["raw_text"][:4096], | |
| color=discord.Color.dark_gray(), | |
| ) | |
| target = thread or channel | |
| target = thread or channel | |
| raw = parsed["raw_text"] | |
| chunks = [raw[i : i + 4000] for i in range(0, len(raw), 4000)] or [""] | |
| for idx, chunk in enumerate(chunks): | |
| title = f"Full Text — {product_type}" + (f" ({idx + 1}/{len(chunks)})" if len(chunks) > 1 else "") | |
| try: | |
| await target.send(embed=discord.Embed(title=title, description=chunk, color=discord.Color.dark_gray())) | |
| except Exception as e: | |
| logger.warning(f"Failed to post full text chunk {idx} for {product_id}: {e}") |
🤖 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 `@cogs/tropical.py` around lines 228 - 233, Update the full-text posting flow
around full_text_embed so parsed["raw_text"] is delivered in chunks that respect
Discord’s 4096-character embed description limit, rather than slicing it to the
first 4096 characters. Send the chunks to the existing target (thread or
channel) in order and preserve the full product text, including its tail.
1 participant
Adds a new TropicalCog that auto-posts NHC tropical cyclone products (advisories, discussions, outlooks, updates) to Discord.
Sources:
/room/nhc) — primary, polls every 30snwws.pyProduct types: TCV/TCP (advisory), TCD (discussion), TWD (tropical weather discussion), TWO (tropical weather outlook), TCU (update), TCE (position estimate)
Features:
TROPICAL_CHANNEL_IDconfigcache/nwws_products.logfor debugging NHC product flowFiles changed:
cogs/tropical.py,cogs/iembot.py,cogs/nwws.py,cogs/__init__.py,config.py,config/products.jsonSummary by CodeRabbit
warning_channel:tropical.TROPICAL_CHANNEL_IDand the supported tropical product types.