amorfo77/parsedmarc
1
0
Fork 0
mirror of https://github.com/domainaware/parsedmarc.git synced 2026-09-26 15:40:31 +00:00
Code Issues Projects Releases Wiki Activity

AGENTS.md: reviews must cover prose and hunk context, not just function

Browse Source 
Codifies the lessons from the PR #834 Copilot review: whole-file
canonical dashboard exports put pre-existing titles/markdown in the
diff, so they get a text-level pass; proofread the full hunk around
prose edits, not only changed lines; and mid-incident glue code gets
the same review bar (and helper-reuse check) as planned code.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Sean Whalen
2026-07-21 19:15:55 -04:00
co-authored by Claude Fable 5
parent 32d80e4289
commit 32094a1460
1 changed files with 8 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
AGENTS.md
+8
View File
@@ -174,6 +174,14 @@ If a config file is listed in `.gitignore`, treat its contents as secret. Do not
Before rewriting a tracked list/data file from freshly-generated content (anything under `parsedmarc/resources/maps/`, CSVs, `.txt` lists), check the existing file first — `git show HEAD:<path> | wc -l`, `git log -1 -- <path>`, `git diff --stat`. Files like `known_unknown_base_reverse_dns.txt` and `base_reverse_dns_map.csv` accumulate manually-curated entries across many sessions, and a "fresh" regeneration that drops the row count is almost certainly destroying prior work. If the new content is meant to *add* rather than *replace*, use a merge/append pattern. Treat any unexpected row-count drop in the pending diff as a red flag.
## Review passes cover prose, not just function
A review that only verifies functional/numeric correctness (queries return the right values, files import cleanly, types check) will sail past exactly the defects a text-first reviewer catches. On PR #834, four such misses survived a thorough functional review: two long-standing typos inside the OSD ndjson ("SMPT TLS", "filed DMARC"), a typo on an *unchanged* line adjacent to a docs edit, and hand-written bootstrap glue that duplicated the script's existing `wait_for()` helper. Rules drawn from that:
- **Whole-file canonical exports put every line in the diff — review them as text, too.** Re-exporting `dashboards/opensearch/opensearch_dashboards.ndjson` or a Grafana JSON from a running instance rewrites the entire file, so pre-existing user-facing strings (saved-object titles, markdown panels, column labels) are formally part of the change. A semantic before/after comparison ("attributes identical") proves no unintended changes but deliberately looks through pre-existing content problems; add one text-level pass over titles and markdown before committing.
- **Proofread the whole hunk around prose edits, not just the `+`/`-` lines.** Typos one line away from an edit are in the reviewer's context window and fair game; they should be in yours.
- **Code written mid-incident gets the same review bar as planned code.** Before writing new shell/infra glue while firefighting, check the file for an existing helper that already does it (e.g. `wait_for()` in `dashboard-dev-bootstrap.sh`), and give your own inline code the same scrutiny you'd give a subagent's.
## Releases
A release isn't done until built artifacts are attached to the GitHub release page. Full sequence: