From 6a0aa2eba5784282990745b0fe824e1816fedff9 Mon Sep 17 00:00:00 2001 From: anusbutt Date: Mon, 27 Jul 2026 14:10:30 +0500 Subject: [PATCH] fix(send): explain why notes were skipped instead of just counting them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `prospector send` reported skips as a bare number. On a real vault that came out as "to send: 0 ... skipped: 47", which reads as a malfunction — while the truth was that 46 of those notes had already been emailed and the ledger was doing exactly its job, and the 47th simply needed a subject line. Every skip already carried a reason; the report just filtered them out, and they were the only outcome the operator got no explanation for. Now grouped by reason, most common first, with the affected slugs named and a plain-language hint for what each reason means: skipped: 46 already in ledger (already emailed — the ledger prevents a repeat) a-z-air-duct, ace-duct-cleaning-inc, air-central, and 43 more 1 draft has no subject (add a **Subject:** line, or re-draft the note) monster-vac Long groups are truncated after six slugs so a 46-note group cannot bury the summary. A run that selects nothing now says so outright, because that is the case most likely to be mistaken for a bug. No behavior change to what is sent or skipped — only to what is reported. --- prospector/cli.py | 39 ++++++++++++++++ tests/unit/test_cli_send.py | 92 +++++++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+) diff --git a/prospector/cli.py b/prospector/cli.py index 24730ee..7a08d07 100644 --- a/prospector/cli.py +++ b/prospector/cli.py @@ -216,6 +216,39 @@ def send( _print_send_report(report) +# A skipped note used to report as a bare count, which is the least actionable +# thing the command could say: "skipped: 47" reads like a malfunction when it +# usually means a guarantee is working. Each skip already carries a reason, so +# group and show them. The hints translate the internal reason into what it +# means for the operator's list. +SKIP_HINTS = { + "already in ledger": "already emailed — the ledger prevents a repeat", + "duplicate inbox in this run": "shares an inbox with another note in this batch", + "not an email-channel note": "note has no email channel (pre-existing note, or address never recovered)", + "missing or invalid email address": "no usable recipient on the note", + "draft has no subject": "add a **Subject:** line, or re-draft the note", + "draft has no body": "the ## Draft section is empty — re-draft the note", +} +MAX_LISTED_SLUGS = 6 + + +def _print_skips(results) -> None: + """Explain every skipped note, grouped by reason (most common first).""" + skips = [r for r in results if r.outcome.value.startswith("skipped")] + if not skips: + return + grouped: dict[str, list[str]] = {} + for result in skips: + grouped.setdefault(result.detail or "unspecified", []).append(result.slug) + typer.echo("\n skipped:") + for reason, slugs in sorted(grouped.items(), key=lambda kv: (-len(kv[1]), kv[0])): + hint = SKIP_HINTS.get(reason) + typer.echo(f" {len(slugs):>3} {reason}" + (f" ({hint})" if hint else "")) + shown = slugs[:MAX_LISTED_SLUGS] + more = len(slugs) - len(shown) + typer.echo(f" {', '.join(shown)}" + (f", and {more} more" if more else "")) + + def _print_send_report(report) -> None: from prospector.models import SendOutcome @@ -237,6 +270,12 @@ def _print_send_report(report) -> None: width = max(len(r.slug) for r in rows) for r in rows: typer.echo(f" {r.slug.ljust(width)} {r.outcome.value:12} {r.detail}") + _print_skips(report.results) + # A run that selects nothing is the case most likely to be read as a bug. + if report.sent == 0 and report.skipped and not report.failed: + typer.echo( + "\n Nothing to send: every approved note was skipped for a reason above." + ) def _print_sourcing_summary(summary) -> None: diff --git a/tests/unit/test_cli_send.py b/tests/unit/test_cli_send.py index 8858568..d9eb2a6 100644 --- a/tests/unit/test_cli_send.py +++ b/tests/unit/test_cli_send.py @@ -281,3 +281,95 @@ def test_from_spoofing_fails_preflight(tmp_path, monkeypatch): assert result.exit_code == 1 assert "spoof" in result.output.lower() assert PASSWORD not in result.output + + +# --- Skip reporting ----------------------------------------------------------- +# A bare "skipped: 47" is the least actionable thing this command can say: it +# reads as a malfunction when it usually means the ledger guarantee is working. + + +def test_skipped_notes_report_their_reason(tmp_path, monkeypatch): + """Every skip is explained, grouped by reason, with the slugs named.""" + from prospector import ledger + from prospector.models import LedgerRecord + + _env(tmp_path, monkeypatch) + vault = tmp_path / "Vault" / "Outreach" + _note(vault, "already-mailed", email="dup@acme.com") + _note(vault, "no-subject", email="fine@acme.com") + (vault / "no-subject.md").write_text( + (vault / "no-subject.md").read_text(encoding="utf-8").replace("**Subject:** Hi\n\n", ""), + encoding="utf-8", + ) + ledger.append( + tmp_path / "ledger.jsonl", + LedgerRecord( + ts="2026-07-18T01:00:00", slug="already-mailed", recipient="dup@acme.com", + company="Already Mailed", message_id="", result="sent", error=None, + from_account="outreach@omniveer.com", + ), + ) + + result = runner.invoke(app, ["send", "--vault", str(vault)]) + + assert result.exit_code == 0 + assert "skipped:" in result.output + # the reason, the count, and the note it applies to + assert "already in ledger" in result.output + assert "already-mailed" in result.output + assert "draft has no subject" in result.output + assert "no-subject" in result.output + # and a plain-language hint for what it means + assert "the ledger prevents a repeat" in result.output + + +def test_all_skipped_run_says_nothing_to_send(tmp_path, monkeypatch): + """The zero-selected case is the one most likely to be read as a bug.""" + _env(tmp_path, monkeypatch) + vault = tmp_path / "Vault" / "Outreach" + _note(vault, "wrong-channel") + (vault / "wrong-channel.md").write_text( + (vault / "wrong-channel.md").read_text(encoding="utf-8").replace( + "channel: email", "channel: messenger" + ), + encoding="utf-8", + ) + + result = runner.invoke(app, ["send", "--vault", str(vault)]) + + assert result.exit_code == 0 + assert "Nothing to send" in result.output + assert "not an email-channel note" in result.output + + +def test_long_skip_group_is_truncated_with_a_count(tmp_path, monkeypatch): + """A 46-note skip group must not bury the summary under 46 lines.""" + _env(tmp_path, monkeypatch) + vault = tmp_path / "Vault" / "Outreach" + for i in range(9): + _note(vault, f"note-{i}") + (vault / f"note-{i}.md").write_text( + (vault / f"note-{i}.md").read_text(encoding="utf-8").replace( + "channel: email", "channel: messenger" + ), + encoding="utf-8", + ) + + result = runner.invoke(app, ["send", "--vault", str(vault)]) + + assert result.exit_code == 0 + assert "and 3 more" in result.output # 9 total, 6 listed + + +def test_clean_run_prints_no_skip_block(tmp_path, monkeypatch): + _env(tmp_path, monkeypatch) + vault = tmp_path / "Vault" / "Outreach" + _note(vault, "good-note") + + result = runner.invoke(app, ["send", "--vault", str(vault)]) + + assert result.exit_code == 0 + # the summary line always carries "skipped: 0"; what must be absent is the + # explanation block that follows it + assert "\n skipped:\n" not in result.output + assert "Nothing to send" not in result.output