From 69e901a785f3083ad0e7f01101e2b4340c1b08f5 Mon Sep 17 00:00:00 2001 From: Rachael Rose Renk <91027132+rachaelrenk@users.noreply.github.com> Date: Mon, 29 Jun 2026 13:01:48 -0600 Subject: [PATCH 1/4] Reframe weekly 404 monitor alert to lead with volume trend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The weekly-404-monitor Slack report led with "Uncovered broken URLs: N (+M new this week)" — a raw distinct-URL count. In the current low-sample window (instrumentation is only weeks old) most broken URLs are one-off bot/crawler/old-bookmark hits, so that count churns heavily week-over-week and overstates the problem even when total 404 volume falls. - run_404_report.py: add REPORT_MIN_HITS (default 5) splitting uncovered URLs into significant gaps (>= threshold) vs long-tail noise; add trend_pct, report_min_hits, significant_uncovered_count, significant_new_gaps_count, top_significant_uncovered, long_tail_count to the JSON summary. Legacy counts kept for continuity; CSV unchanged. - SKILL.md: reframe Slack format to lead with the 404 volume trend, list only significant gaps, and roll the long tail into a single line. Clarify reporting threshold vs the Phase 2 automation threshold. Co-Authored-By: Oz --- .agents/skills/weekly-404-monitor/SKILL.md | 26 ++++++++++------ .../weekly-404-monitor/run_404_report.py | 30 ++++++++++++++++++- 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/.agents/skills/weekly-404-monitor/SKILL.md b/.agents/skills/weekly-404-monitor/SKILL.md index c4d5a113..27cddc01 100644 --- a/.agents/skills/weekly-404-monitor/SKILL.md +++ b/.agents/skills/weekly-404-monitor/SKILL.md @@ -5,7 +5,7 @@ description: Weekly recurring agent that surfaces broken docs.warp.dev URLs by q # Weekly 404 monitor -Runs every Monday at 9am PT. Identifies new broken URL patterns on docs.warp.dev, surfaces the top uncovered paths, and posts a concise Slack summary so the docs team can prioritize redirect additions. +Runs every Monday at 9am PT. Leads with the overall 404 volume trend, surfaces the uncovered paths that get enough traffic to be worth a redirect, and posts a concise Slack summary so the docs team can prioritize redirect additions without being distracted by long-tail bot/old-link noise. ## Prerequisites @@ -51,6 +51,10 @@ Compare this week's uncovered gaps against last week's uncovered gaps (from step **New gaps** = uncovered this week AND not seen as uncovered last week. **Resolved** = uncovered last week AND now either covered (has redirect) or no longer generating 404s. +**Significant vs long-tail.** Split uncovered URLs by the reporting threshold (`REPORT_MIN_HITS`, default 5): +- **Significant gaps** = uncovered URLs with `hits_this_week >= REPORT_MIN_HITS`. These are worth a redirect and belong in the headline. +- **Long-tail noise** = uncovered URLs below the threshold. Because the monitor is only weeks old (low sample), most broken URLs are hit once by bots, crawlers, or stale bookmarks, so the raw uncovered and "new gap" counts churn heavily week-over-week and overstate the problem. Roll these up into a single count — never list them individually or put them in the headline. + ### 5. Post Slack summary Post a Slack message using the Block Kit format defined in the "Slack message format" section below. @@ -76,23 +80,26 @@ Use Slack Block Kit. The message should be scannable in under 30 seconds. ``` 📊 *docs.warp.dev 404 Report* — week of {YYYY-MM-DD} -*Total 404s this week:* {N} ({+N / -N vs last week}) -*Uncovered broken URLs:* {M} ({+N new this week}) +*404 volume:* {total_404s_this_week} this week — {▼|▲} {abs(trend_delta)} ({trend_pct}%) vs {total_404s_last_week} last week +{one-line read, e.g. "Down — redirect coverage is holding." or "Up — check the gaps below."} -*Top 10 uncovered URLs (by hits):* +*Gaps worth fixing (≥{report_min_hits} hits):* {significant_uncovered_count} ({significant_new_gaps_count} new) {hit_count} `/path` {🆕 if new this week} ... -*{K} resolved since last week* (redirect added or traffic stopped) +_+{long_tail_count} other uncovered URLs under {report_min_hits} hits each (mostly bots/old links) — see CSV._ +*{resolved_count} resolved since last week* (redirect added or traffic stopped) -→ Add missing redirects: `vercel.json` › `redirects` array (PR against `main`) +→ Add redirects for the gaps above: `vercel.json` › `redirects` array (PR against `main`) → Full breakdown: {oz_run_url} ``` Rules: -- Cap the list at 10 entries. If there are more, note "and N more — see full CSV in the run." +- **Lead with volume trend, not distinct-URL counts.** The first line is always the total-404 trend (`trend_delta` / `trend_pct`) — the metric that reflects real user impact. Use ▼ when `trend_delta` is negative (fewer 404s — good) and ▲ when positive. +- **Only list significant gaps.** List `top_significant_uncovered` (URLs with `hits_this_week >= report_min_hits`), capped at 10. If there are more, note "and N more — see full CSV in the run." If `significant_uncovered_count` is 0, write "None this week — remaining 404s are all low-hit long-tail traffic." and omit the list. +- **Roll up the long tail.** Never list sub-threshold URLs individually; collapse them into the single `long_tail_count` line so noise doesn't dominate the report. - Mark new gaps with 🆕. -- If total 404s this week is less than 50, add a brief positive note: "404 volume is low — good signal that redirect coverage is working." +- If `total_404s_this_week` is less than 50, add a brief positive note: "404 volume is low — good signal that redirect coverage is working." - Never include raw user data (e.g. query strings with user IDs, tokens) in the Slack message. Strip query params from broken_url before displaying. ## Phase 2: Redirect drafter @@ -101,7 +108,7 @@ After the Slack summary is posted and the CSV artifact is written, continue with ### Threshold and confidence scoring -Only process gaps where `hits_this_week >= 10`. This threshold reduces noise; review and adjust after the first four weeks of data. +Only process gaps where `hits_this_week >= 10`. This is the **automation** threshold for opening redirect PRs — deliberately higher than the **reporting** threshold (`REPORT_MIN_HITS`, default 5) used for the Phase 1 Slack summary. This threshold reduces noise; review and adjust after the first four weeks of data. For each qualifying uncovered URL, attempt to find a redirect target using these heuristics in order: @@ -165,6 +172,7 @@ Before posting to Slack, verify: - The `broken_url` field was present in the event properties for at least some rows. If it is consistently null, the `docs_404` tracking implementation has a bug — report it in the Slack message and tag the docs team. - The vercel.json redirect list was loaded successfully and contains more than 500 entries (sanity check that the file is not truncated). - The CSV artifact was written before posting to Slack. +- The Slack summary leads with the volume trend and lists only significant gaps (`hits_this_week >= report_min_hits`); long-tail URLs are rolled up into the `long_tail_count` line, never listed individually. ## No-data report diff --git a/.agents/skills/weekly-404-monitor/run_404_report.py b/.agents/skills/weekly-404-monitor/run_404_report.py index 6f70cebe..79bdbfbe 100644 --- a/.agents/skills/weekly-404-monitor/run_404_report.py +++ b/.agents/skills/weekly-404-monitor/run_404_report.py @@ -153,6 +153,11 @@ def normalise_url(url: str) -> str: def main(): vercel_path = Path(os.environ.get("VERCEL_JSON_PATH", "vercel.json")) report_dir = Path(os.environ.get("REPORT_DIR", "data/404-reports")) + # Minimum weekly hits for an uncovered URL to count as a "gap worth fixing" + # in the Slack headline. Anything below this is long-tail noise (one-off + # bot/crawler/old-bookmark traffic) and is rolled up into a single count so + # it doesn't dominate the report. Tunable via env; see SKILL.md. + report_min_hits = int(os.environ.get("REPORT_MIN_HITS", "5")) today = date.today() print(f"Running weekly 404 report for week ending {today}", file=sys.stderr) @@ -231,15 +236,38 @@ def main(): uncovered = [r for r in report_rows if not r["is_covered_by_redirect"]] new_gaps = [r for r in uncovered if r["is_new_gap"]] + # Split uncovered URLs into "signal" (enough hits to be worth a redirect) + # and long-tail "noise" (below the reporting threshold). In a low-sample + # dataset most broken URLs are hit once by bots/old links, so the raw + # uncovered and new-gap counts churn heavily week-over-week and overstate + # the problem. The headline leads with volume trend + significant gaps; + # the long tail is reported only as a single rolled-up count. + significant = [r for r in uncovered if r["hits_this_week"] >= report_min_hits] + significant_new_gaps = [r for r in significant if r["is_new_gap"]] + long_tail_count = len(uncovered) - len(significant) + + trend_pct = ( + round((total_current - total_prior) / total_prior * 100, 1) + if total_prior else None + ) + summary = { "report_date": today.isoformat(), + # --- Headline: overall 404 volume trend (the metric that matters) --- "total_404s_this_week": total_current, "total_404s_last_week": total_prior, "trend_delta": total_current - total_prior, + "trend_pct": trend_pct, + # --- Signal: uncovered URLs with enough hits to be worth a redirect --- + "report_min_hits": report_min_hits, + "significant_uncovered_count": len(significant), + "significant_new_gaps_count": len(significant_new_gaps), + "top_significant_uncovered": significant[:10], + # --- Context only: raw/long-tail counts (do NOT headline these) --- "uncovered_count": len(uncovered), "new_gaps_count": len(new_gaps), + "long_tail_count": long_tail_count, "resolved_count": resolved_count, - "top_10_uncovered": uncovered[:10], "csv_path": str(csv_path), "has_data": len(current_week) > 0, } From 46661110b9e3fa5ee69dbd62170a6f0efb8b8a6f Mon Sep 17 00:00:00 2001 From: Rachael Rose Renk <91027132+rachaelrenk@users.noreply.github.com> Date: Mon, 29 Jun 2026 14:11:25 -0600 Subject: [PATCH 2/4] Aggregate 404 broken URLs by normalised path The SQL groups broken URLs by their raw value (only ?/# stripped), so trailing-slash, case, and scheme/host variants (/foo, /foo/, /Foo, https://host/foo) came back as separate rows that were never re-merged after normalise_url. The same page was counted multiple times in the uncovered / new-gap counts and could appear twice in the top list, and its hits were split across variants (sometimes dropping a real gap below the reporting threshold). Add aggregate_by_norm() to collapse rows by normalise_url (summing hits) for both current and prior week. report_rows, prior_gaps, and current_urls now derive from the aggregated maps, and report_rows is re-sorted by hits. SKILL.md steps 1/3/6 document the per-page aggregation. Co-Authored-By: Oz --- .agents/skills/weekly-404-monitor/SKILL.md | 7 +-- .../weekly-404-monitor/run_404_report.py | 52 +++++++++++-------- 2 files changed, 35 insertions(+), 24 deletions(-) diff --git a/.agents/skills/weekly-404-monitor/SKILL.md b/.agents/skills/weekly-404-monitor/SKILL.md index 27cddc01..018ad040 100644 --- a/.agents/skills/weekly-404-monitor/SKILL.md +++ b/.agents/skills/weekly-404-monitor/SKILL.md @@ -27,7 +27,8 @@ The script: - Queries `warp-data-357114.prod.stg_website_events` via the Metabase API - Extracts `broken_url` from `event_properties` for all `event_name = 'docs_404'` events in the past 7 days - Groups by `broken_url`, sorted by hit count descending -- Returns a ranked list of broken URLs and their hit counts for the current week +- Aggregates those rows by normalised path (lowercase, no trailing slash, no scheme/host) so trailing-slash, case, and host variants of the same page are counted once with their hits summed +- Returns a ranked list of broken pages and their hit counts for the current week - Computes the same for the prior week (days 8–14) for trend comparison - Total weekly 404 count (current + prior) for the trend line @@ -39,7 +40,7 @@ Extract all `source` values from the `redirects` array. Normalise: lowercase, st ### 3. Find uncovered URLs -For each broken URL in the current week's data: +Broken URLs are first aggregated by their normalised form, so `/foo`, `/foo/`, and `/Foo` collapse into one page with summed hits (this prevents the same page being reported as several separate gaps). For each aggregated page: - Normalise (lowercase, strip trailing slash, strip query params and fragments) - Check if it exists as a `source` in `vercel.json` redirects - If not covered, it is a **gap** @@ -63,7 +64,7 @@ If `SLACK_BOT_TOKEN` is unavailable, write the full Slack message body to the ru ### 6. Write CSV artifact -Write `404-report-YYYY-MM-DD.csv` to `data/404-reports/` in the docs repo working directory. Format: +Write `404-report-YYYY-MM-DD.csv` to `data/404-reports/` in the docs repo working directory. Each row is one normalised page (hits summed across trailing-slash/case/host variants). Format: ``` broken_url,hits_this_week,hits_last_week,is_covered_by_redirect,is_new_gap diff --git a/.agents/skills/weekly-404-monitor/run_404_report.py b/.agents/skills/weekly-404-monitor/run_404_report.py index 79bdbfbe..ee4a471b 100644 --- a/.agents/skills/weekly-404-monitor/run_404_report.py +++ b/.agents/skills/weekly-404-monitor/run_404_report.py @@ -150,6 +150,24 @@ def normalise_url(url: str) -> str: return url or "/" +def aggregate_by_norm(rows: list[dict]) -> dict[str, int]: + """Collapse raw broken_url rows into one entry per normalised path. + + The SQL groups by the raw broken_url, so trailing-slash, case, and + scheme/host variants (e.g. /foo, /foo/, /Foo) arrive as separate rows. + Merging them by normalise_url means each logical page is counted once and + its hits are summed — otherwise hits split across variants can fall below + the reporting threshold and the same page can appear multiple times. + """ + agg: dict[str, int] = {} + for row in rows: + norm = normalise_url(row.get("broken_url") or "") + if not norm: + continue + agg[norm] = agg.get(norm, 0) + int(row.get("hits") or 0) + return agg + + def main(): vercel_path = Path(os.environ.get("VERCEL_JSON_PATH", "vercel.json")) report_dir = Path(os.environ.get("REPORT_DIR", "data/404-reports")) @@ -165,10 +183,15 @@ def main(): # 1. Query current and prior week print("Querying current week (past 7 days)...", file=sys.stderr) current_week = query_404_events(1, 8) - print(f" {len(current_week)} unique broken URLs found", file=sys.stderr) + # Collapse trailing-slash / case / host variants so each logical page is + # counted once and its hits are summed (see aggregate_by_norm). + current_agg = aggregate_by_norm(current_week) + print(f" {len(current_week)} raw rows -> {len(current_agg)} unique pages", + file=sys.stderr) print("Querying prior week (days 8-14)...", file=sys.stderr) prior_week = query_404_events(8, 15) + prior_agg = aggregate_by_norm(prior_week) total_current = total_404_count(1, 8) total_prior = total_404_count(8, 15) @@ -178,38 +201,25 @@ def main(): redirect_sources = load_redirect_sources(vercel_path) # 3. Build prior-week gap set for delta calculation - prior_gaps: set[str] = set() - for row in prior_week: - norm = normalise_url(row["broken_url"]) - if norm and norm not in redirect_sources: - prior_gaps.add(norm) + prior_gaps = {u for u in prior_agg if u not in redirect_sources} - # 4. Build current-week report + # 4. Build current-week report (one row per normalised page) report_rows = [] - for row in current_week: - raw_url = row.get("broken_url") or "" - norm = normalise_url(raw_url) - if not norm: - continue - hits_current = int(row.get("hits") or 0) - hits_prior = next( - (int(r["hits"]) for r in prior_week - if normalise_url(r.get("broken_url") or "") == norm), - 0 - ) + for norm, hits_current in current_agg.items(): is_covered = norm in redirect_sources is_new_gap = (not is_covered) and (norm not in prior_gaps) - report_rows.append({ "broken_url": norm, "hits_this_week": hits_current, - "hits_last_week": hits_prior, + "hits_last_week": prior_agg.get(norm, 0), "is_covered_by_redirect": is_covered, "is_new_gap": is_new_gap, }) + # Summing variants can change ordering, so re-sort by hits descending. + report_rows.sort(key=lambda r: r["hits_this_week"], reverse=True) # 5. Compute resolved (was a gap last week, is no longer generating hits) - current_urls = {normalise_url(r["broken_url"]) for r in current_week} + current_urls = set(current_agg) newly_covered = { g for g in prior_gaps if g in redirect_sources # redirect was added From 6d3c02a59f3201f0bee9a823a7acf37882c408b9 Mon Sep 17 00:00:00 2001 From: Rachael Rose Renk <91027132+rachaelrenk@users.noreply.github.com> Date: Mon, 29 Jun 2026 15:12:56 -0600 Subject: [PATCH 3/4] Address Oz review nits: validate REPORT_MIN_HITS + safe trend display - parse_min_hits(): treat REPORT_MIN_HITS as a positive integer; fall back to 5 (with a stderr warning) for non-numeric, zero, or negative values instead of crashing or reintroducing long-tail noise. - format_trend(): pre-format the headline volume line and return a "no prior-week baseline yet" string when last week had 0 events, so the Slack template ({trend_summary}) never renders a null percentage. - SKILL.md: headline uses {trend_summary}; document the no-baseline fallback and the positive-integer requirement for REPORT_MIN_HITS. Co-Authored-By: Oz --- .agents/skills/weekly-404-monitor/SKILL.md | 6 +-- .../weekly-404-monitor/run_404_report.py | 49 ++++++++++++++++--- 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/.agents/skills/weekly-404-monitor/SKILL.md b/.agents/skills/weekly-404-monitor/SKILL.md index 018ad040..50cf8b97 100644 --- a/.agents/skills/weekly-404-monitor/SKILL.md +++ b/.agents/skills/weekly-404-monitor/SKILL.md @@ -52,7 +52,7 @@ Compare this week's uncovered gaps against last week's uncovered gaps (from step **New gaps** = uncovered this week AND not seen as uncovered last week. **Resolved** = uncovered last week AND now either covered (has redirect) or no longer generating 404s. -**Significant vs long-tail.** Split uncovered URLs by the reporting threshold (`REPORT_MIN_HITS`, default 5): +**Significant vs long-tail.** Split uncovered URLs by the reporting threshold (`REPORT_MIN_HITS`, default 5; must be a positive integer — invalid or non-positive values fall back to 5): - **Significant gaps** = uncovered URLs with `hits_this_week >= REPORT_MIN_HITS`. These are worth a redirect and belong in the headline. - **Long-tail noise** = uncovered URLs below the threshold. Because the monitor is only weeks old (low sample), most broken URLs are hit once by bots, crawlers, or stale bookmarks, so the raw uncovered and "new gap" counts churn heavily week-over-week and overstate the problem. Roll these up into a single count — never list them individually or put them in the headline. @@ -81,7 +81,7 @@ Use Slack Block Kit. The message should be scannable in under 30 seconds. ``` 📊 *docs.warp.dev 404 Report* — week of {YYYY-MM-DD} -*404 volume:* {total_404s_this_week} this week — {▼|▲} {abs(trend_delta)} ({trend_pct}%) vs {total_404s_last_week} last week +*404 volume:* {trend_summary} {one-line read, e.g. "Down — redirect coverage is holding." or "Up — check the gaps below."} *Gaps worth fixing (≥{report_min_hits} hits):* {significant_uncovered_count} ({significant_new_gaps_count} new) @@ -96,7 +96,7 @@ _+{long_tail_count} other uncovered URLs under {report_min_hits} hits each (most ``` Rules: -- **Lead with volume trend, not distinct-URL counts.** The first line is always the total-404 trend (`trend_delta` / `trend_pct`) — the metric that reflects real user impact. Use ▼ when `trend_delta` is negative (fewer 404s — good) and ▲ when positive. +- **Lead with volume trend, not distinct-URL counts.** The first line is always `trend_summary` — the pre-formatted total-404 trend, which reflects real user impact. It already includes the direction arrow (▼ fewer 404s, ▲ more, → no change) and falls back to a "no prior-week baseline yet" message when last week had no data, so the percentage is never rendered as null. - **Only list significant gaps.** List `top_significant_uncovered` (URLs with `hits_this_week >= report_min_hits`), capped at 10. If there are more, note "and N more — see full CSV in the run." If `significant_uncovered_count` is 0, write "None this week — remaining 404s are all low-hit long-tail traffic." and omit the list. - **Roll up the long tail.** Never list sub-threshold URLs individually; collapse them into the single `long_tail_count` line so noise doesn't dominate the report. - Mark new gaps with 🆕. diff --git a/.agents/skills/weekly-404-monitor/run_404_report.py b/.agents/skills/weekly-404-monitor/run_404_report.py index ee4a471b..163bad4f 100644 --- a/.agents/skills/weekly-404-monitor/run_404_report.py +++ b/.agents/skills/weekly-404-monitor/run_404_report.py @@ -27,6 +27,7 @@ import urllib.request from datetime import date, timedelta from pathlib import Path +from typing import Optional BASE = "https://warp.metabaseapp.com/api" @@ -168,6 +169,44 @@ def aggregate_by_norm(rows: list[dict]) -> dict[str, int]: return agg +def parse_min_hits(raw: str, default: int = 5) -> int: + """Parse the REPORT_MIN_HITS env value as a positive integer. + + Falls back to `default` (with a warning) for non-numeric, zero, or negative + values, which would otherwise crash or reintroduce long-tail noise. + """ + try: + value = int(raw) + except (TypeError, ValueError): + value = None + if value is None or value < 1: + print(f"WARNING: REPORT_MIN_HITS={raw!r} is not a positive integer; " + f"falling back to {default}.", file=sys.stderr) + return default + return value + + +def format_trend(total_current: int, total_prior: int) -> tuple[int, Optional[float], str]: + """Return (trend_delta, trend_pct, trend_summary) for the headline. + + trend_pct is None when there is no prior-week baseline (total_prior == 0), + and trend_summary is pre-formatted so the Slack template never has to render + a null percentage. + """ + trend_delta = total_current - total_prior + trend_pct: Optional[float] = None + if total_prior: + trend_pct = round(trend_delta / total_prior * 100, 1) + arrow = "▼" if trend_delta < 0 else ("▲" if trend_delta > 0 else "→") + trend_summary = ( + f"{total_current} this week — {arrow} {abs(trend_delta)} " + f"({trend_pct}%) vs {total_prior} last week" + ) + else: + trend_summary = f"{total_current} this week (no prior-week baseline yet)" + return trend_delta, trend_pct, trend_summary + + def main(): vercel_path = Path(os.environ.get("VERCEL_JSON_PATH", "vercel.json")) report_dir = Path(os.environ.get("REPORT_DIR", "data/404-reports")) @@ -175,7 +214,7 @@ def main(): # in the Slack headline. Anything below this is long-tail noise (one-off # bot/crawler/old-bookmark traffic) and is rolled up into a single count so # it doesn't dominate the report. Tunable via env; see SKILL.md. - report_min_hits = int(os.environ.get("REPORT_MIN_HITS", "5")) + report_min_hits = parse_min_hits(os.environ.get("REPORT_MIN_HITS", "5")) today = date.today() print(f"Running weekly 404 report for week ending {today}", file=sys.stderr) @@ -256,18 +295,16 @@ def main(): significant_new_gaps = [r for r in significant if r["is_new_gap"]] long_tail_count = len(uncovered) - len(significant) - trend_pct = ( - round((total_current - total_prior) / total_prior * 100, 1) - if total_prior else None - ) + trend_delta, trend_pct, trend_summary = format_trend(total_current, total_prior) summary = { "report_date": today.isoformat(), # --- Headline: overall 404 volume trend (the metric that matters) --- "total_404s_this_week": total_current, "total_404s_last_week": total_prior, - "trend_delta": total_current - total_prior, + "trend_delta": trend_delta, "trend_pct": trend_pct, + "trend_summary": trend_summary, # --- Signal: uncovered URLs with enough hits to be worth a redirect --- "report_min_hits": report_min_hits, "significant_uncovered_count": len(significant), From 611abe840bf2c28ea5e7a6a9a3e826fc05e36caa Mon Sep 17 00:00:00 2001 From: Rachael Rose Renk <91027132+rachaelrenk@users.noreply.github.com> Date: Tue, 30 Jun 2026 11:24:17 -0600 Subject: [PATCH 4/4] Aggregate 404 URLs by normalised path in SQL before the top-500 cap Addresses the remaining Oz review nit on PR #268: LIMIT 500 was applied to raw broken_url variants before aggregate_by_norm summed them, so a page whose hits split across trailing-slash/case/host variants could fall outside the top 500 and be undercounted against the reporting threshold. Normalisation + aggregation now happen in SQL so the cap applies to normalised pages; the Python aggregate_by_norm pass remains as an idempotent safety net. Co-Authored-By: Oz --- .agents/skills/weekly-404-monitor/SKILL.md | 6 +- .../weekly-404-monitor/run_404_report.py | 75 +++++++++++++------ 2 files changed, 54 insertions(+), 27 deletions(-) diff --git a/.agents/skills/weekly-404-monitor/SKILL.md b/.agents/skills/weekly-404-monitor/SKILL.md index 50cf8b97..da44e1de 100644 --- a/.agents/skills/weekly-404-monitor/SKILL.md +++ b/.agents/skills/weekly-404-monitor/SKILL.md @@ -26,9 +26,9 @@ Run `python3 .agents/skills/weekly-404-monitor/run_404_report.py` in the docs re The script: - Queries `warp-data-357114.prod.stg_website_events` via the Metabase API - Extracts `broken_url` from `event_properties` for all `event_name = 'docs_404'` events in the past 7 days -- Groups by `broken_url`, sorted by hit count descending -- Aggregates those rows by normalised path (lowercase, no trailing slash, no scheme/host) so trailing-slash, case, and host variants of the same page are counted once with their hits summed -- Returns a ranked list of broken pages and their hit counts for the current week +- Normalises and groups by path **in SQL** (lowercase, no trailing slash, no scheme/host, no query/fragment) so trailing-slash, case, and host variants of the same page are summed into one row *before* the top-500 cap is applied — a page whose hits are split across variants can't be dropped by the cap and undercounted +- Returns a ranked list of broken pages and their hit counts for the current week, sorted by hit count descending +- Re-applies the same normalisation in Python as an idempotent safety net - Computes the same for the prior week (days 8–14) for trend comparison - Total weekly 404 count (current + prior) for the trend line diff --git a/.agents/skills/weekly-404-monitor/run_404_report.py b/.agents/skills/weekly-404-monitor/run_404_report.py index 163bad4f..d93de56e 100644 --- a/.agents/skills/weekly-404-monitor/run_404_report.py +++ b/.agents/skills/weekly-404-monitor/run_404_report.py @@ -74,23 +74,50 @@ def query_404_events(days_start: int, days_end: int) -> list[dict]: Return broken_url counts for the window [days_start, days_end) days ago. days_start=1, days_end=8 → past 7 days (current week) days_start=8, days_end=15 → 8-14 days ago (prior week) + + Normalisation and aggregation happen in SQL so trailing-slash, case, and + scheme/host variants of the same page are summed into a single row BEFORE + the LIMIT is applied. If we grouped by the raw broken_url and capped first, + a page whose hits are split across variants could fall outside the top 500 + and be dropped before its hits were summed, undercounting it against the + reporting threshold. The normalisation below mirrors normalise_url(). """ sql = f""" -SELECT - REGEXP_REPLACE( - SPLIT(JSON_VALUE(event_properties, '$.broken_url'), '?')[OFFSET(0)], - r'#.*$', '' - ) AS broken_url, - COUNT(*) AS hits -FROM `warp-data-357114.prod.stg_website_events` -WHERE event_type = 'track' - AND event_name = 'docs_404' - AND JSON_VALUE(event_properties, '$.broken_url') IS NOT NULL - AND event_date >= DATE_SUB(CURRENT_DATE(), INTERVAL {days_end - 1} DAY) - AND event_date < DATE_SUB(CURRENT_DATE(), INTERVAL {days_start - 1} DAY) -GROUP BY 1 -HAVING broken_url IS NOT NULL AND broken_url != '' -ORDER BY 2 DESC +WITH normalised AS ( + SELECT + COALESCE( + NULLIF( + REGEXP_REPLACE( + LOWER( + REGEXP_REPLACE( + SPLIT( + REGEXP_REPLACE( + JSON_VALUE(event_properties, '$.broken_url'), + r'^https?://[^/]+', '' + ), + '?' + )[OFFSET(0)], + r'#.*$', '' + ) + ), + r'/+$', '' + ), + '' + ), + '/' + ) AS broken_url + FROM `warp-data-357114.prod.stg_website_events` + WHERE event_type = 'track' + AND event_name = 'docs_404' + AND JSON_VALUE(event_properties, '$.broken_url') IS NOT NULL + AND JSON_VALUE(event_properties, '$.broken_url') != '' + AND event_date >= DATE_SUB(CURRENT_DATE(), INTERVAL {days_end - 1} DAY) + AND event_date < DATE_SUB(CURRENT_DATE(), INTERVAL {days_start - 1} DAY) +) +SELECT broken_url, COUNT(*) AS hits +FROM normalised +GROUP BY broken_url +ORDER BY hits DESC LIMIT 500 """ return run_query(sql) @@ -152,13 +179,13 @@ def normalise_url(url: str) -> str: def aggregate_by_norm(rows: list[dict]) -> dict[str, int]: - """Collapse raw broken_url rows into one entry per normalised path. + """Collapse broken_url rows into one entry per normalised path. - The SQL groups by the raw broken_url, so trailing-slash, case, and - scheme/host variants (e.g. /foo, /foo/, /Foo) arrive as separate rows. - Merging them by normalise_url means each logical page is counted once and - its hits are summed — otherwise hits split across variants can fall below - the reporting threshold and the same page can appear multiple times. + query_404_events already normalises and aggregates in SQL (so the LIMIT is + applied to normalised pages, not raw variants). This step re-applies + normalise_url() in Python as an idempotent safety net: it reconciles any + rows that SQL normalisation and normalise_url() would treat differently, + guaranteeing each logical page is counted once with its hits summed. """ agg: dict[str, int] = {} for row in rows: @@ -222,10 +249,10 @@ def main(): # 1. Query current and prior week print("Querying current week (past 7 days)...", file=sys.stderr) current_week = query_404_events(1, 8) - # Collapse trailing-slash / case / host variants so each logical page is - # counted once and its hits are summed (see aggregate_by_norm). + # query_404_events already normalises + aggregates in SQL; this is an + # idempotent safety net that reconciles any normalisation drift. current_agg = aggregate_by_norm(current_week) - print(f" {len(current_week)} raw rows -> {len(current_agg)} unique pages", + print(f" {len(current_week)} pages (SQL) -> {len(current_agg)} pages (normalised)", file=sys.stderr) print("Querying prior week (days 8-14)...", file=sys.stderr)