From 18b38c991dac9ec400af1553ee3287f88c8e62f3 Mon Sep 17 00:00:00 2001 From: Sean Whalen <44679+seanthegeek@users.noreply.github.com> Date: Sun, 12 Jul 2026 09:33:57 -0400 Subject: [PATCH] Add verify skill and document model roles in CLAUDE.md (#817) * Raise test coverage: utils, elastic, and opensearch to 100% Coverage of the shipped library rises from 88% to 90%, with parsedmarc/utils.py 86% -> 100% and elastic.py / opensearch.py 99% -> 100%. All new tests assert on observable behaviour and mock only at SDK boundaries (dnspython Resolver.resolve, requests.get, subprocess.check_call, elasticsearch_dsl/opensearchpy Document.save). New tests cover: query_dns transient-error retries, the load_ip_db download/cache/bundled fallback chain, the IPinfo API token probe and per-request MMDB fallbacks, _normalize_ip_record schema handling, reverse-DNS-map invalid-CSV fallback, caller-provided reverse DNS maps, Outlook MSG conversion (missing msgconvert and success paths), parse_email Cc/Bcc/attachment-hash branches, aggregate-XML edge cases (bytes input, repeated policy_published, unknown RFC 9990 override types, missing org_name, attribute-only ), extract_report on non-seekable streams, and the _AggregateReportDoc.save() override that derives passed_dmarc. Bugs found by the new tests, fixed in the same PR per the testing standards: - parse_email() crashed with KeyError: 'Headers' on messages whose From header is present but unparseable (e.g. a bare "From:" line): the fallback read parsed_email["Headers"], but the parsed headers are stored under lowercase "headers" (assigned a few lines up in the same function), so the key never exists. At the CLI surface this made any failure report whose embedded sample had an empty From: header fail to parse ("Missing value: 'Headers'"). - configure_ipinfo_api(probe=True) logged "IPinfo API configured" when the probe could not reach the API, contradicting its own docstring ("other errors are logged and the token is still accepted"): _ipinfo_api_lookup() returns None on network errors instead of raising, so the probe's exception handler was unreachable. The probe now checks the lookup result and warns on failure; 401/403 still raises InvalidIPinfoAPIKey. Dead code deleted rather than padded with tests: - _SMTPTLSReportDoc.add_policy() in elastic.py and opensearch.py (the save paths construct _SMTPTLSPolicyDoc directly). - The no-op "for failure_index in failure_indexes: pass" loop in both migrate_indexes() implementations (parameter still accepted). - The importlib.resources ImportError fallback in utils.py, which re-imported the same module and is unreachable on Python >= 3.10. - The "Invalid report content" guard in extract_report(): every input branch assigns file_object or raises first (confirmed by pyright narrowing with the guard removed). Also widens parse_aggregate_report_xml's annotation to str | bytes to match its existing runtime behaviour (bytes are decoded with errors ignored). Co-Authored-By: Claude Fable 5 * Use assertGreater for the reverse-DNS-map fallback size check Addresses the github-code-quality bot finding on PR #816: assertTrue with a comparison inside can't show the operands on failure, while assertGreater reports both values and the failed relation. No change to test behavior. Co-Authored-By: Claude Fable 5 * Add verification claude skill for parsedmarc CLI usage and sample inputs * Document model roles for feature work in CLAUDE.md Codifies the plan-with-Fable / implement-with-Sonnet / review-with-Fable split (Opus as fallback) for feature work and PR reviews. Co-Authored-By: Claude Sonnet 5 --------- Co-authored-by: Claude Fable 5 --- .claude/skills/verify/SKILL.md | 46 ++++++++++++++++++++++++++++++++++ CLAUDE.md | 10 ++++++++ 2 files changed, 56 insertions(+) create mode 100644 .claude/skills/verify/SKILL.md diff --git a/.claude/skills/verify/SKILL.md b/.claude/skills/verify/SKILL.md new file mode 100644 index 0000000..7d399f2 --- /dev/null +++ b/.claude/skills/verify/SKILL.md @@ -0,0 +1,46 @@ +--- +name: verify +description: Launch and drive parsedmarc's CLI to verify parser/output changes end-to-end against the bundled sample reports. +--- + +# Verifying parsedmarc changes + +The runtime surface is the `parsedmarc` CLI; the library's stream APIs are +drivable via `python -c` through the public `parsedmarc` package. + +## Launch + +```bash +# No config file → results print as JSON to stdout. --offline skips DNS/downloads. +GITHUB_ACTIONS=true .venv/bin/python -m parsedmarc.cli --offline +``` + +- Do **not** use `-c ci.ini` locally: it points at `http://localhost:9200` + Elasticsearch (a CI service container) and retries for ~75s before failing. +- `GITHUB_ACTIONS=true` skips live DNS lookups. +- `--debug` surfaces per-file parse warnings/errors (invalid reports are + otherwise dropped silently from the JSON). + +## Good sample inputs (all under `samples/`) + +- `aggregate/rfc9990-sample.xml` — RFC 9990 aggregate report +- `aggregate/*.xml.zip`, `aggregate/*.xml.gz` — archive extraction paths +- `failure/dmarc_ruf_report_linkedin.eml` — failure (RUF) report with an + embedded rfc822 sample (exercises `utils.parse_email`) +- `aggregate/invalid_xml.xml` — recovered via lxml, parses with `errors` set + +## Stream API (stdin is genuinely non-seekable when piped) + +```bash +cat samples/aggregate/*.xml.gz | .venv/bin/python -c \ + "import sys, parsedmarc; print(parsedmarc.extract_report(sys.stdin.buffer)[:80])" +# Text-mode stdin must raise ParserError ("binary (rb) mode"): +cat samples/extract_report/nice-input.xml | .venv/bin/python -c \ + "import sys, parsedmarc; parsedmarc.extract_report(sys.stdin)" +``` + +## Gotchas + +- Parse failures are WARNING-level log lines, not stderr errors — grep the + `--debug` output; the JSON just omits the report. +- To compare against unfixed code: `git stash push -- `, run, `git stash pop`. diff --git a/CLAUDE.md b/CLAUDE.md index 078c29c..749642c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -2,4 +2,14 @@ This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. +## Model roles for feature work + +Any new feature or modification to an existing feature must follow this model split: + +1. **Plan with Fable** (fall back to Opus only if Fable is unavailable). Enter plan mode, design the implementation, and present the plan to the user for approval or modification. Do not start implementing until the user approves the plan. +2. **Implement with Sonnet.** Once the plan is approved, carry out the implementation using Sonnet (e.g. by delegating the implementation steps to Sonnet subagents via the Agent tool with `model: "sonnet"`). +3. **Review with Fable** (fall back to Opus only if Fable is unavailable). After implementation, all work must be reviewed by Fable before it is considered done. + +**PR reviews** must also use Fable, with Opus as the fallback if Fable is unavailable. + @AGENTS.md