diff --git a/.agents/logs/agent_doc_quality_baseline.md b/.agents/logs/agent_doc_quality_baseline.md new file mode 100644 index 000000000..963122f27 --- /dev/null +++ b/.agents/logs/agent_doc_quality_baseline.md @@ -0,0 +1,37 @@ +# Agent-doc quality v1 baseline log (GROW-6092) + +Durable record of the pre-rollout baseline window and the post-rollout +comparison window used by `improve-drafting-skills`'s +`scripts/compute_metrics.py` (see that skill's "v1 baseline and outcome +metrics" section). Each entry names the window, the record source, and where +the resulting report was persisted (the standing signal-log branch/PR, per +`.agents/references/skill-authoring-guidelines.md`). + +Entries are appended, never rewritten — the baseline entry is captured once, +immediately before v1 checks/labels/review go live, and is never recomputed. + +## Entries + +### 2026-08-01 to 2026-08-30 — pre-rollout baseline + +Captured from the existing `.agents/logs/human_review_feedback.jsonl` signal +log (real per-comment data already collected by `improve-drafting-skills`, +covering 2026-06-29 through 2026-08-31) plus live `gh pr view` line-count +lookups against `warpdotdev/docs`, using +`scripts/build_baseline_records.py` to convert the comment-level log into +per-PR records and `scripts/compute_metrics.py` to compute the report. This +is the last full 30-day window available in the existing log before this v1 +rollout PR, so it is used as the frozen pre-rollout baseline rather than +waiting for a window that starts exactly at rollout. + +- **Records**: `.agents/logs/baseline/pre-rollout-2026-08-01-to-2026-08-30.jsonl` (62 PRs) +- **Report**: `.agents/logs/baseline/pre-rollout-2026-08-01-to-2026-08-30-report.json` +- **In-scope PRs**: 62 +- **Human review comments/PR**: mean 5.19, median 1.0 +- **Human edit churn ratio**: mean 0.0021, median 0.0 +- **Gate coverage**: 0/62 (expected — the v1 checks did not exist during this window; `risk`/`check_outcome`/`review_outcome` are recorded as `"unknown"` per the documented pre-rollout degradation, not fabricated as passing) + +The post-rollout 30-day comparison report must be computed the same way +(`compute_metrics.py --baseline .agents/logs/baseline/pre-rollout-2026-08-01-to-2026-08-30-report.json`) +and appended below as a new dated entry once 30 days of post-rollout data +(or 10 in-scope PRs, whichever comes first per the small-sample rule) exist. diff --git a/.agents/logs/baseline/pre-rollout-2026-08-01-to-2026-08-30-report.json b/.agents/logs/baseline/pre-rollout-2026-08-01-to-2026-08-30-report.json new file mode 100644 index 000000000..385fecdc3 --- /dev/null +++ b/.agents/logs/baseline/pre-rollout-2026-08-01-to-2026-08-30-report.json @@ -0,0 +1,71 @@ +{ + "all_passed_required_checks": false, + "engineering_review_required": { + "completed_by": { + "docs_verified": 0, + "docs_waiver": 0, + "source_owner_approval": 0, + "unanswered_request": 0, + "unresolved_owner": 0 + }, + "completion_rates": { + "docs_verified": null, + "docs_waiver": null, + "source_owner_approval": null, + "unanswered_request": null, + "unresolved_owner": null + }, + "total": 0 + }, + "gate_coverage_missing_data_count": 62, + "human_edit_churn_ratio": { + "denominator_pr_count": 62, + "mean": 0.0021, + "median": 0.0, + "no_agent_commit_count": 0, + "numerator_pr_count": 62, + "zero_denominator_count": 0 + }, + "human_review_comments": { + "denominator": 62, + "numerator": 322, + "per_pr": { + "mean": 5.1935, + "median": 1.0 + }, + "targeted_categories": { + "action_first": 1, + "callout": 5, + "code_fence": 42, + "content_structure": 8, + "frontmatter": 10, + "general": 203, + "heading_specificity": 8, + "image_alt": 1, + "link_quality": 9, + "list_format": 14, + "missing_context": 1, + "missing_media": 8, + "scannability": 1, + "settings_path": 10, + "terminology": 20, + "ui_label_accuracy": 11, + "vague_wording": 1, + "video_embed": 4 + }, + "total": 322 + }, + "in_scope_prs": 62, + "prs_with_complete_gate_coverage": 0, + "prs_with_passing_checks": 0, + "review_findings": { + "critical_important_per_pr": 0.0, + "critical_total": 0, + "important_total": 0, + "targeted_categories": {} + }, + "window": { + "end": "2026-08-30", + "start": "2026-08-01" + } +} diff --git a/.agents/logs/baseline/pre-rollout-2026-08-01-to-2026-08-30.jsonl b/.agents/logs/baseline/pre-rollout-2026-08-01-to-2026-08-30.jsonl new file mode 100644 index 000000000..46133c2b3 --- /dev/null +++ b/.agents/logs/baseline/pre-rollout-2026-08-01-to-2026-08-30.jsonl @@ -0,0 +1,62 @@ +{"date": "2026-08-03", "pr": "387", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"terminology": 1}, "agent_lines_changed": 4, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-04", "pr": "398", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"link_quality": 1}, "agent_lines_changed": 185, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-04", "pr": "406", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"general": 1}, "agent_lines_changed": 198, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-03", "pr": "411", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 100, "human_review_comment_categories": {"terminology": 5, "general": 54, "image_alt": 1, "ui_label_accuracy": 8, "list_format": 8, "frontmatter": 1, "callout": 1, "heading_specificity": 1, "link_quality": 1, "settings_path": 1, "code_fence": 16, "scannability": 1, "missing_media": 4, "content_structure": 3, "missing_context": 1}, "agent_lines_changed": 1246, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-03", "pr": "420", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"general": 1}, "agent_lines_changed": 1704, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-04", "pr": "436", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 7, "human_review_comment_categories": {"code_fence": 2, "general": 5}, "agent_lines_changed": 34, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-03", "pr": "439", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 2, "human_review_comment_categories": {"general": 3, "heading_specificity": 1, "link_quality": 1, "frontmatter": 1, "list_format": 1, "callout": 1, "video_embed": 1, "settings_path": 1, "terminology": 1}, "agent_lines_changed": 19, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-03", "pr": "442", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 0, "human_review_comment_categories": {"heading_specificity": 1, "link_quality": 1, "frontmatter": 1, "list_format": 1, "callout": 1, "settings_path": 1, "video_embed": 1, "general": 1, "terminology": 1}, "agent_lines_changed": 14, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-01", "pr": "445", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 4, "human_review_comment_categories": {"general": 4}, "agent_lines_changed": 104, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-01", "pr": "446", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 4, "human_review_comment_categories": {"general": 4}, "agent_lines_changed": 44, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-01", "pr": "447", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 6, "human_review_comment_categories": {"heading_specificity": 1, "missing_media": 1, "general": 5, "callout": 1}, "agent_lines_changed": 242, "human_lines_changed_after_last_agent_commit": 31, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-01", "pr": "449", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 2, "human_review_comment_categories": {"terminology": 1, "general": 1}, "agent_lines_changed": 2, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-03", "pr": "451", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 2, "human_review_comment_categories": {"ui_label_accuracy": 1, "heading_specificity": 1, "link_quality": 1, "frontmatter": 1, "list_format": 1, "callout": 1, "settings_path": 1, "video_embed": 1, "general": 2, "terminology": 1}, "agent_lines_changed": 53, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-03", "pr": "453", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 2, "human_review_comment_categories": {"ui_label_accuracy": 1, "general": 1}, "agent_lines_changed": 11, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-03", "pr": "458", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 2, "human_review_comment_categories": {"settings_path": 1, "general": 1}, "agent_lines_changed": 1, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-03", "pr": "462", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 2, "human_review_comment_categories": {"settings_path": 1, "terminology": 1}, "agent_lines_changed": 18, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-03", "pr": "463", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 2, "human_review_comment_categories": {"general": 2}, "agent_lines_changed": 206, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-04", "pr": "473", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 9, "human_review_comment_categories": {"general": 5, "missing_media": 1, "frontmatter": 2, "settings_path": 1}, "agent_lines_changed": 449, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-05", "pr": "476", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 5, "human_review_comment_categories": {"terminology": 1, "general": 3, "settings_path": 1}, "agent_lines_changed": 303, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-05", "pr": "481", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"terminology": 1}, "agent_lines_changed": 4, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-16", "pr": "526", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 32, "human_review_comment_categories": {"list_format": 1, "general": 21, "content_structure": 2, "code_fence": 8}, "agent_lines_changed": 122, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-16", "pr": "527", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 7, "human_review_comment_categories": {"general": 4, "code_fence": 1, "frontmatter": 1, "settings_path": 1}, "agent_lines_changed": 106, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-16", "pr": "528", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 10, "human_review_comment_categories": {"general": 10}, "agent_lines_changed": 90, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-13", "pr": "529", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"list_format": 1}, "agent_lines_changed": 58, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-13", "pr": "530", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"general": 1}, "agent_lines_changed": 3, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-16", "pr": "531", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 18, "human_review_comment_categories": {"content_structure": 1, "general": 16, "vague_wording": 1}, "agent_lines_changed": 91, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-16", "pr": "539", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"ui_label_accuracy": 1}, "agent_lines_changed": 107, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-17", "pr": "546", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 2, "human_review_comment_categories": {"general": 2}, "agent_lines_changed": 13, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-17", "pr": "548", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"general": 1}, "agent_lines_changed": 2, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-17", "pr": "549", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 13, "human_review_comment_categories": {"general": 11, "code_fence": 1, "link_quality": 1}, "agent_lines_changed": 141, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-18", "pr": "563", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"terminology": 1}, "agent_lines_changed": 567, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-18", "pr": "566", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"action_first": 1}, "agent_lines_changed": 231, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-18", "pr": "569", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 3, "human_review_comment_categories": {"terminology": 1, "general": 2}, "agent_lines_changed": 197, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-18", "pr": "572", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"heading_specificity": 1}, "agent_lines_changed": 311, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-18", "pr": "577", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"terminology": 1}, "agent_lines_changed": 50, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-19", "pr": "578", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"terminology": 1}, "agent_lines_changed": 125, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-26", "pr": "582", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"general": 1}, "agent_lines_changed": 16, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-20", "pr": "588", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"link_quality": 1}, "agent_lines_changed": 17, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-20", "pr": "590", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"missing_media": 1}, "agent_lines_changed": 68, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-26", "pr": "595", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"general": 1}, "agent_lines_changed": 1319, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-21", "pr": "596", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 2, "human_review_comment_categories": {"frontmatter": 1, "general": 1}, "agent_lines_changed": 34, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-23", "pr": "603", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"general": 1}, "agent_lines_changed": 189, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-23", "pr": "605", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"settings_path": 1}, "agent_lines_changed": 127, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-22", "pr": "606", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 6, "human_review_comment_categories": {"general": 5, "list_format": 1}, "agent_lines_changed": 696, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-23", "pr": "608", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"link_quality": 1}, "agent_lines_changed": 10, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-23", "pr": "609", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 11, "human_review_comment_categories": {"content_structure": 1, "general": 10}, "agent_lines_changed": 322, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-24", "pr": "612", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 3, "human_review_comment_categories": {"general": 3}, "agent_lines_changed": 11, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-25", "pr": "614", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"heading_specificity": 1}, "agent_lines_changed": 51, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-24", "pr": "619", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 6, "human_review_comment_categories": {"general": 3, "code_fence": 3}, "agent_lines_changed": 1082, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-26", "pr": "627", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 2, "human_review_comment_categories": {"terminology": 1, "code_fence": 1}, "agent_lines_changed": 46, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-25", "pr": "629", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 4, "human_review_comment_categories": {"frontmatter": 1, "general": 2, "code_fence": 1}, "agent_lines_changed": 68, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-25", "pr": "630", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"general": 1}, "agent_lines_changed": 74, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-25", "pr": "631", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"general": 1}, "agent_lines_changed": 51, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-26", "pr": "639", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"content_structure": 1}, "agent_lines_changed": 37, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-26", "pr": "641", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"terminology": 1}, "agent_lines_changed": 3, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-26", "pr": "643", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 8, "human_review_comment_categories": {"terminology": 1, "general": 7}, "agent_lines_changed": 130, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-26", "pr": "644", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"general": 1}, "agent_lines_changed": 93, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-26", "pr": "645", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"heading_specificity": 1}, "agent_lines_changed": 130, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-27", "pr": "646", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"link_quality": 1}, "agent_lines_changed": 8, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-27", "pr": "647", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 15, "human_review_comment_categories": {"general": 5, "code_fence": 9, "missing_media": 1}, "agent_lines_changed": 83, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-28", "pr": "651", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"video_embed": 1}, "agent_lines_changed": 3, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} +{"date": "2026-08-28", "pr": "660", "skill_used": "unknown", "risk": "unknown", "head_sha": "unknown", "check_outcome": "unknown", "review_outcome": "unknown", "review_critical": 0, "review_important": 0, "review_categories": {}, "human_review_comments": 1, "human_review_comment_categories": {"frontmatter": 1}, "agent_lines_changed": 40, "human_lines_changed_after_last_agent_commit": 0, "has_agent_commit": true, "engineering_required": false, "completion_method": "n/a"} diff --git a/.agents/references/doc-quality-policy.md b/.agents/references/doc-quality-policy.md new file mode 100644 index 000000000..96c1f143c --- /dev/null +++ b/.agents/references/doc-quality-policy.md @@ -0,0 +1,192 @@ +# Agent-doc quality policy (v1) + +Canonical reference for the agent-authored-content quality program (GROW-6092). +Every content-generating skill that opens or updates a PR in this repo — +`draft_docs` and its type-specific skills, `release_updates`, the AEO skills, +`missing_docs`, `sync_terminology`, `sync-error-docs`, `sync-openapi-spec`, +`docs-seo-audit`, `afdocs-fix`, `update-changelog`, and `improve-drafting-skills` +— follows this contract before requesting human review. `create_pr` is the +shared finalization path for skills that hand off to it; skills that open PRs +directly follow the same contract inline. + +The mechanics live in `.agents/skills/doc_quality_policy/policy.py` (parsing +and classification) and `check_pr_contract.py` (the CI-callable checker). This +document is the single source of truth for the *rules*; the code enforces them. + +## Scope + +This program covers every agent-authored content PR in `warpdotdev/docs` — +`draft_*`, `release_updates`, AEO, `missing_docs`, and any other recurring +skill that adds or changes public documentation. It is not limited to ambient +feature drafts. + +## The agent marker + +Every agent-authored content PR carries the label `warpy-factory` and a +`## Documentation risk` section in its body (see below). PR-opening skills +apply both before requesting human review, via +`.agents/skills/doc_quality_policy/finalize_pr_contract.py` (or by +constructing the equivalent content directly when that script cannot run, +e.g. a workflow step without Python available). + +## PR-body contract sections + +Every agent-authored content PR body carries: + +1. **`## Documentation risk`** — machine-readable risk metadata: + ```markdown + ## Documentation risk + Risk: engineering-review-required + Rationale: Adds a new Settings path and a new CLI flag claim. + Source files consulted: app/src/settings_view/mod.rs@, warp-server/pkg/foo/bar.go@ + Requested engineering reviewers: alice + Engineering review status: pending + Docs override: none + ``` + When Docs records external validation, add: + ```markdown + Docs override: docs-verified + Override reviewer: hongyi-chen + Override reason: Confirmed the flag name against warp-server PR #1234. + Override evidence: warp-server/pkg/foo/bar.go@ + Override head SHA: + ``` + `Docs override: docs-waiver` uses the same four fields when Docs proceeds + without an engineering response and records why the remaining risk is + acceptable. +2. **`## Unverified claims`** — unchanged from the existing `draft_docs` / + `create_pr` contract (step 9.5). Every `{/* VERIFY: ... */}` marker in + changed content must appear here. + +## Risk levels + +Exactly two values: `low` and `engineering-review-required`. Ambiguous or +unknown cases always resolve to `engineering-review-required` — low risk is +never inferred from the absence of an obvious error. + +### Low-risk allowlist (strict) + +A PR is `low` risk only when **all** of the following hold: + +- It does not add a page about a new or materially changed feature or workflow. +- It is either: + - a product-meaning-preserving edit limited to spelling, grammar, tone, + formatting, descriptive links/cross-links to existing canonical pages, + search metadata, or generated changelog/license/telemetry data whose + source-verification script passed; or + - internal Docs-team tooling, skill, or CI workflow maintenance that makes + no public product claim and does not change the behavior of a + developer-facing command, API, setting, or integration. +- It does not add or change: commands, code or configuration examples, API + behavior, UI labels or paths, defaults, permissions, availability or + platform support, plan eligibility, billing behavior, security or privacy + claims, data handling, self-hosting behavior, or integration setup. +- It contains no unresolved `VERIFY` marker and has no critical or important + technical-accuracy finding from `review-docs-pr`. + +Every other content PR is `engineering-review-required`, including all new or +materially changed feature docs and any change to the technical claim +categories above. When the change cannot be shown to fit one of these +low-risk categories, classify it as `engineering-review-required`. + +## Engineering review requests + +- **Low risk**: the normal docs reviewer approves. No engineering owner + approval is required. +- **Engineering-review-required**: source-owner resolution is attempted first, + and a real GitHub review request goes to at least one owner resolved from + the product source files consulted. The request is advisory. Docs can + validate the claim from source, Slack, or another appropriate channel, then + proceed through the normal Docs review path without a GitHub approval from + the engineer. + +Record an engineer response or Docs validation in the PR body when it informs +the final wording. A new head makes earlier validation context stale, but it +does not block a Docs-approved PR from merging. + +The required push-time PR-contract check validates only the risk metadata and +VERIFY accounting. Engineering review requests are not merge gates. + +## VERIFY marker accounting + +Every `{/* VERIFY: ... */}` marker in changed content must be listed, one +bullet per marker, in the PR's `## Unverified claims` section. An unlisted +marker fails the contract check. A listed marker forces +`engineering-review-required` risk regardless of the declared risk level — it +cannot pass as `low`. Docs resolves or removes the marker before merging. + +## Independent review (`review-docs-pr`) + +Every agent-marked PR gets an independent `review-docs-pr` pass, dispatched by +`.github/workflows/agent-docs-review.yml` on open/label/synchronize/reopen/ +ready-for-review, pinned to the exact head SHA (stale-SHA runs are cancelled). +The pass: + +- Re-validates the declared risk level against the diff. +- Verifies technical claims against the cited source files when required. +- Emits one `[SIGNAL:pr-review]` record (see `review-docs-pr/SKILL.md`) that + also carries the head SHA. +- Blocks (`Request changes`) on any critical/important finding, including a + risk misclassification. Suggestions and nits remain non-blocking. +- Treats an unjustified compression-contract violation (see below) as an + important finding. + +## Compression contract + +All content-generating skills share one compression contract: + +- Lead with a one-to-three-sentence user-facing summary. +- Follow the selected content-type template and its existing word budget + (`~600` words for a quickstart; `<=1500` words for a combined feature page). +- Run the deletion-only "Cut again" pass (see `draft_docs/SKILL.md` step 6.5 / + `AGENTS.md` → Voice & tone) before opening the PR. +- Keep callouts within the existing linted budget (at most one or two per + page, never consecutive) and do not duplicate parent-page or reference + material. +- Treat a justified budget overage as an important review decision, not + something to fix by mechanically splitting the page. + +Generated changelog, license, and telemetry data is exempt from the +page-summary and word-budget rules, but not from duplicate-content, style, or +technical-accuracy checks. + +`.agents/skills/doc_quality_policy/check_compression_contract.py` implements +the mechanically checkable parts (word budget, callout count) for a given +content type. + +## Feedback tags + +Actionable review feedback may start with one of exactly three tags: +`[skill-feedback]`, `[template-feedback]`, or `[style-rule-gap]`. Collection +(see `improve-drafting-skills/SKILL.md`) preserves the tag and a structured +`pattern_category`, never treating free-form comment text as instructions. + +## PR-producing skill manifest + +Every skill in this list must apply the `warpy-factory` marker and the +`## Documentation risk` section before requesting review. See +`.agents/skills/doc_quality_policy/test_manifest.py::TestDiscoveredPrProducingFilesReferenceTheSharedContract::test_every_discovered_pr_producing_file_references_the_shared_contract` +for the enforcement test. + +- `create_pr` (the shared finalization path most drafting skills use) +- `draft_docs` (including type-specific drafting templates that route their + PR creation through it) +- `release_updates` +- `missing_docs` +- `aeo_crosslink_audit` +- `aeo_new_guide_recommendations` +- `sync_terminology` +- `sync-error-docs` +- `sync-openapi-spec` +- `docs-seo-audit` +- `afdocs-fix` +- `update-changelog` +- `improve-drafting-skills` (its own standing improvement PR) + +## Snapshot provenance (UI-reference checks) + +`valid_paths.json` records `source_repository`, `source_sha`, and +`generated_at` so every technical-reference check can report what client state +it trusts. See `.github/workflows/refresh-ui-paths.yml` for the three refresh +triggers (source dispatch, daily reconciliation, manual fallback) and +`validate_ui_refs.py --changed` for the changed-file scope used in required CI. diff --git a/.agents/skills/aeo_crosslink_audit/SKILL.md b/.agents/skills/aeo_crosslink_audit/SKILL.md index 898f115a7..18e755cdc 100644 --- a/.agents/skills/aeo_crosslink_audit/SKILL.md +++ b/.agents/skills/aeo_crosslink_audit/SKILL.md @@ -7,6 +7,15 @@ description: Run a narrow AEO cross-link audit for Warp docs using Peec, Google Identify small, high-confidence internal cross-linking improvements for the Warp docs. This skill is designed for a recurring Oz scheduled agent that audits one narrow topic area, opens a small PR when there are safe changes, or writes a no-change report when there are not enough high-confidence opportunities. +## Agent-doc quality contract + +Any PR this skill opens follows the shared v1 agent-doc quality contract in +`.agents/references/doc-quality-policy.md`: apply the `warpy-factory` label +and add the `## Documentation risk` block +(`.agents/skills/doc_quality_policy/finalize_pr_contract.py build`). A +cross-link-only change that adds no new claims is typically `low` risk under +the allowlist. + ## Scope Use this skill only for the pilot topic area: diff --git a/.agents/skills/aeo_new_guide_recommendations/SKILL.md b/.agents/skills/aeo_new_guide_recommendations/SKILL.md index 64e82a021..34f3c9253 100644 --- a/.agents/skills/aeo_new_guide_recommendations/SKILL.md +++ b/.agents/skills/aeo_new_guide_recommendations/SKILL.md @@ -7,6 +7,14 @@ description: Run a recurring AEO new-guide recommendations brief for Warp docs u Produce 3–5 source-grounded AEO briefs that recommend whether to create a new guide, update an existing page, or avoid a topic — without drafting content. This skill is designed for an Oz scheduled agent that runs monthly. The goal is to give the Docs team a repeatable upstream input to content planning without generating more noise than the team can act on. +## Agent-doc quality contract + +This skill does not draft content or open content PRs (see "Do not" below), so +it does not stamp the `warpy-factory` marker itself. See +`.agents/references/doc-quality-policy.md` for the contract that applies once +a recommendation here becomes a real drafting task in `draft_docs` or a +type-specific drafting skill. + ## Scope Use this skill only for the pilot topic areas: diff --git a/.agents/skills/afdocs-fix/SKILL.md b/.agents/skills/afdocs-fix/SKILL.md index 31be075fb..d0bfde8c7 100644 --- a/.agents/skills/afdocs-fix/SKILL.md +++ b/.agents/skills/afdocs-fix/SKILL.md @@ -12,6 +12,16 @@ description: >- Apply automated fixes for issues found by the `afdocs-audit` skill. This skill reads an AFDocs audit report and remediates fixable issues. +## Agent-doc quality contract + +The standing PR this skill maintains follows the shared v1 agent-doc quality +contract in `.agents/references/doc-quality-policy.md`: apply the +`warpy-factory` label and add the `## Documentation risk` block +(`.agents/skills/doc_quality_policy/finalize_pr_contract.py build`). These +fixes touch site infrastructure (middleware, integrations, config), not +content claims, and are typically `engineering-review-required` unless the +change is provably a mechanical config update. + ## Prerequisites 1. Run the audit skill first to produce a report: diff --git a/.agents/skills/create_pr/SKILL.md b/.agents/skills/create_pr/SKILL.md index 89f40dc4f..d3616d76d 100644 --- a/.agents/skills/create_pr/SKILL.md +++ b/.agents/skills/create_pr/SKILL.md @@ -13,6 +13,7 @@ This guide covers best practices for creating pull requests in the docs document - `draft_docs` - Draft new documentation pages or update existing ones using established style conventions - `check_for_broken_links` - Check documentation for broken internal and external links before opening PR +- `doc_quality_policy` - Shared v1 agent-doc quality contract (marker, risk classification, overrides) this skill's PRs must satisfy ## Pre-PR Checklist @@ -211,6 +212,31 @@ When claims are outstanding, give the reviewer one bullet per claim with what wo - **Settings** > **Agents** > **Permissions** path — `permissions.mdx`, "Defaults" — source repos were not available in this environment. ``` +### Documentation risk (required on all content PRs) + +Every content PR carries a `## Documentation risk` section and the +`warpy-factory` label, per the shared v1 agent-doc quality contract in +`.agents/references/doc-quality-policy.md`. Classify risk against the +low-risk allowlist there, then build the section: + +```bash +python3 .agents/skills/doc_quality_policy/finalize_pr_contract.py build \ + --risk low --rationale "One-line reason the change is low risk." +``` + +Insert the printed block into the body (after "Unverified claims" is a good +place) and apply the label once the PR exists: + +```bash +gh pr edit --repo warpdotdev/docs --add-label warpy-factory +``` + +Before marking the PR ready, verify the contract: + +```bash +python3 .agents/skills/doc_quality_policy/check_pr_contract.py --body /tmp/pr-body.md +``` + ### Additional context (optional) - Link to related issues or discussions - Screenshots for visual changes @@ -452,7 +478,7 @@ A team handle resolved from `STAKEHOLDERS` or `CODEOWNERS` can only be requested ::: :::note -Auto-requesting the review does not make it *block* merge. Whether an ambient docs PR should require that approval through branch protection is an open question for the docs owner, not something this skill decides. +Auto-requesting the review does not block merge. The Docs team owns the merge decision after its normal review, whether an engineer replies in GitHub, replies elsewhere, or does not reply. ::: ### Update an existing PR diff --git a/.agents/skills/doc_quality_policy/SKILL.md b/.agents/skills/doc_quality_policy/SKILL.md new file mode 100644 index 000000000..3049be8cb --- /dev/null +++ b/.agents/skills/doc_quality_policy/SKILL.md @@ -0,0 +1,69 @@ +--- +name: doc_quality_policy +description: Shared reference and helper scripts for the v1 agent-doc quality contract — the warpy-factory marker, the Documentation risk / Unverified claims PR-body sections, the low-risk allowlist, VERIFY marker accounting, and docs-verified/docs-waiver overrides. Not a dispatchable skill; used inline by every PR-producing content skill and by CI. +--- + +# doc_quality_policy + +Shared library for the v1 agent-doc quality program. This is a reference and +helper-script skill, not something dispatched on its own — every content skill +that opens or updates a PR (`create_pr`, `draft_docs` and its type-specific +skills, `release_updates`, `missing_docs`, the AEO skills, `sync_terminology`, +`sync-error-docs`, `sync-openapi-spec`, `docs-seo-audit`, `afdocs-fix`, +`update-changelog`, `improve-drafting-skills`) and CI use it inline. + +The full rules live in `.agents/references/doc-quality-policy.md`. This +directory implements them: + +- `policy.py` — pure parsing/classification/validation functions (the agent + marker, risk levels, `## Documentation risk` / `## Unverified claims` + parsing, `{/* VERIFY: ... */}` marker accounting, and the low-risk allowlist + via `RiskSignals` + `classify_risk`). +- `check_pr_contract.py` — CI-callable checker: reads a PR body file, finds + VERIFY markers in the changed docs files (auto-discovered via + `git diff --diff-filter=d origin/main...HEAD` when not passed explicitly, + matching `style_lint.py --changed`'s scope and its no-silent-fallback rule), + and validates the structural contract. Engineering review requests are + advisory and do not affect the check result. Exit 0 = pass, 1 = violations, + 2 = usage error. +- `finalize_pr_contract.py build` — prints the `## Documentation risk` block + for a PR-producing skill to insert into its PR body. Does not call `gh` + itself; the invoking skill applies the `warpy-factory` label separately. +- `check_compression_contract.py` — checks the mechanically-checkable parts + of the shared compression contract (word budget, callout count) for one + file. +- `authorized_docs_reviewers.json` — the GitHub handles authorized to record + a `docs-verified`/`docs-waiver` override. Pod-Docs owns this list. + +## Using this from a PR-producing skill + +Before requesting review: + +1. Determine the risk level. Walk the low-risk allowlist in + `.agents/references/doc-quality-policy.md` — if every condition holds, the + PR is `low`; otherwise (including any ambiguous case) it is + `engineering-review-required`. +2. Build the `## Documentation risk` block: + ```bash + python3 .agents/skills/doc_quality_policy/finalize_pr_contract.py build \ + --risk low --rationale "One-line reason." + ``` + Insert the printed block into the PR body, alongside the existing + `## Unverified claims` section (see `create_pr/SKILL.md`). +3. Apply the marker label: + ```bash + gh pr edit --repo warpdotdev/docs --add-label warpy-factory + ``` +4. Before marking the PR ready, verify the contract locally: + ```bash + python3 .agents/skills/doc_quality_policy/check_pr_contract.py --body /tmp/pr-body.md + ``` + +## Using this from CI + +The `Docs technical references` CI job (see `.github/workflows/ci.yml`) runs +`check_pr_contract.py` against the PR body and the changed docs files on every +pull request, failing on an unlisted `VERIFY` marker, a missing/invalid risk +section. An engineering-review-required PR requests source-owner review, but +the Docs team can merge after its normal review without a GitHub approval from +the engineer. diff --git a/.agents/skills/doc_quality_policy/authorized_docs_reviewers.json b/.agents/skills/doc_quality_policy/authorized_docs_reviewers.json new file mode 100644 index 000000000..8ddf06632 --- /dev/null +++ b/.agents/skills/doc_quality_policy/authorized_docs_reviewers.json @@ -0,0 +1,8 @@ +{ + "_comment": "GitHub handles authorized to record a docs-verified or docs-waiver override on an engineering-review-required PR (see .agents/references/doc-quality-policy.md). Seeded from the existing docs-reviewer fallback chain in create_pr/SKILL.md 'Request reviewers'. Pod-Docs owns this list; update it directly, not via an agent-authored PR.", + "authorized_docs_reviewers": [ + "hongyi-chen", + "rachaelrenk", + "dannyneira" + ] +} diff --git a/.agents/skills/doc_quality_policy/check_compression_contract.py b/.agents/skills/doc_quality_policy/check_compression_contract.py new file mode 100644 index 000000000..415c88bc5 --- /dev/null +++ b/.agents/skills/doc_quality_policy/check_compression_contract.py @@ -0,0 +1,131 @@ +#!/usr/bin/env python3 +"""Check the mechanically-checkable parts of the shared compression contract. + +Word-budget and callout-count checks only — the rest of the contract (lead +summary, "Cut again" pass, no duplication) needs human/agent judgment and is +covered by `draft_docs`'s checklist and `review-docs-pr`. Generated +changelog/license/telemetry pages are exempt from the word budget (see +`.agents/references/doc-quality-policy.md`). + +Usage: + python3 check_compression_contract.py FILE --content-type quickstart + python3 check_compression_contract.py FILE --content-type feature-doc + +Exit codes: + 0 within budget + 1 over budget (a reportable finding for review-docs-pr, not a hard CI gate) + 2 usage / file error +""" +from __future__ import annotations + +import argparse +import re +import sys +from pathlib import Path +from typing import List, Optional + +# ~600 words for a quickstart; <=1500 words for a combined feature page. +# Other content types have no fixed budget here; the drafting skills' own +# length guidance applies. +WORD_BUDGETS = { + "quickstart": 600, + "feature-doc": 1500, +} + +# Generated data is exempt from the page-summary/word-budget rules, per the +# compression contract, but not from duplicate-content/style/technical checks +# (enforced elsewhere). +EXEMPT_CONTENT_TYPES = {"changelog", "license", "telemetry"} + +MAX_CALLOUTS = 2 +_CALLOUT_OPEN_RE = re.compile(r"^:::(note|tip|caution|danger)\b") + + +def _strip_frontmatter(text: str) -> str: + if not text.startswith("---"): + return text + end = text.find("\n---", 3) + if end == -1: + return text + return text[end + 4:] + + +def _strip_code_fences(lines: List[str]) -> List[str]: + out = [] + in_fence = False + for line in lines: + if line.strip().startswith("```"): + in_fence = not in_fence + continue + if not in_fence: + out.append(line) + return out + + +def count_words(text: str) -> int: + """Count words in page body prose, excluding frontmatter and code fences.""" + body = _strip_frontmatter(text) + lines = _strip_code_fences(body.splitlines()) + prose = "\n".join(lines) + prose = re.sub(r"!\[[^\]]*\]\([^)]*\)", " ", prose) # images + prose = re.sub(r"\[([^\]]*)\]\([^)]*\)", r"\1", prose) # link text only + return len(re.findall(r"\S+", prose)) + + +def count_callouts(text: str) -> int: + """Count Starlight `:::note` / `:::tip` / etc. callout blocks.""" + body = _strip_frontmatter(text) + return sum(1 for line in _strip_code_fences(body.splitlines()) if _CALLOUT_OPEN_RE.match(line.strip())) + + +def check_compression_contract(text: str, content_type: str) -> List[str]: + """Return a list of findings; empty means within the mechanical budget.""" + findings: List[str] = [] + + budget = WORD_BUDGETS.get(content_type) + if content_type not in EXEMPT_CONTENT_TYPES and budget is not None: + words = count_words(text) + if words > budget: + findings.append( + f"word count {words} exceeds the {content_type} budget of {budget} " + "(a justified overage is an important review decision, not an automatic split)" + ) + + callouts = count_callouts(text) + if callouts > MAX_CALLOUTS: + findings.append(f"{callouts} callouts exceed the linted budget of {MAX_CALLOUTS}") + + return findings + + +def main(argv: Optional[List[str]] = None) -> int: + parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + parser.add_argument("file", help="path to the markdown/MDX file, or '-' for stdin") + parser.add_argument( + "--content-type", required=True, + help="content type (e.g. quickstart, feature-doc, conceptual, changelog, license, telemetry)", + ) + args = parser.parse_args(argv) + + if args.file == "-": + text = sys.stdin.read() + else: + try: + text = Path(args.file).read_text(encoding="utf-8") + except OSError as exc: + print(f"error: could not read {args.file}: {exc}", file=sys.stderr) + return 2 + + findings = check_compression_contract(text, args.content_type) + if findings: + print("Compression contract findings:", file=sys.stderr) + for finding in findings: + print(f" - {finding}", file=sys.stderr) + return 1 + + print("Compression contract check passed.") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.agents/skills/doc_quality_policy/check_pr_contract.py b/.agents/skills/doc_quality_policy/check_pr_contract.py new file mode 100644 index 000000000..caeba886a --- /dev/null +++ b/.agents/skills/doc_quality_policy/check_pr_contract.py @@ -0,0 +1,397 @@ +#!/usr/bin/env python3 +"""CI-callable checker for the agent-doc quality PR contract. + +Wraps `policy.py` with the I/O CI needs: reading a PR body file, discovering +`{/* VERIFY: ... */}` markers in the changed docs files, and optionally taking +live signals (head SHA, review outcome, deterministic-check result) as flags +so this stays callable from a GitHub Actions step without a live `gh` call +baked into the checker itself. + +Usage: + python3 check_pr_contract.py --body /tmp/pr-body.md [changed_file ...] + python3 check_pr_contract.py --body /tmp/pr-body.md \\ + --repo owner/repo --pr 123 --head-sha "$HEAD_SHA" \ + --enforce-engineering-gate + +When `--repo`/`--pr` are given with `--enforce-engineering-gate`, the +engineering-review gate signals +(deterministic-check outcome, source-owner approval, unresolved review +findings) are derived live from GitHub via `gh api` for the exact head SHA, +rather than trusted from caller-supplied flags -- a PR cannot claim its own +deterministic checks passed or its own review approved. Any live lookup +failure fails closed (treated as not-yet-satisfied), never as success. The +`--deterministic-checks-failed` / `--source-owner-approved` / +`--unresolved-important-finding` flags remain for local/offline dry runs when +`--repo`/`--pr` are omitted. + +The authorized-docs-reviewer allowlist is always read from a trusted git ref +(`--authorized-reviewers-ref`, default `origin/main`) rather than the PR's own +working-tree checkout, so a PR cannot add its own author to the allowlist in +the same diff that claims an override. + +When no changed-file paths are given, changed `.md`/`.mdx` files under +`src/content/docs` are discovered via `git diff --diff-filter=d +origin/main...HEAD`, matching `style_lint.py --changed` and +`validate_ui_refs.py --changed`'s scope. Unlike `style_lint`'s `--changed`, +this never falls back to a full-tree scan: a diff that can't be resolved is a +hard failure, so required CI never silently widens scope. + +Exit codes: + 0 the PR contract is satisfied + 1 one or more contract violations (see stderr) + 2 usage / file error +""" +from __future__ import annotations + +import argparse +import importlib.util +import json +import subprocess +import sys +from dataclasses import dataclass +from pathlib import Path +from typing import List, Optional, Sequence + +_HERE = Path(__file__).resolve().parent +_DOCS_ROOT = "src/content/docs" +_AUTHORIZED_REVIEWERS_REPO_PATH = ".agents/skills/doc_quality_policy/authorized_docs_reviewers.json" +# Only the sibling job: this contract check itself runs as a step inside the +# "Docs technical references" job, so that job's check-run is still +# "in_progress" (never "success") at the moment this step executes -- it +# cannot verify its own conclusion mid-run. Its earlier steps having reached +# this one already proves they passed (a step failure would have stopped the +# job before this step ran). +_REQUIRED_CHECK_NAMES = ("Docs editorial quality",) + +_spec = importlib.util.spec_from_file_location("doc_quality_policy", _HERE / "policy.py") +policy = importlib.util.module_from_spec(_spec) +sys.modules[_spec.name] = policy # required for dataclass field resolution +_spec.loader.exec_module(policy) + +AGENT_MARKER = policy.AGENT_MARKER + + +def should_run_contract_check(event_name: str, pr_labels: Sequence[str]) -> bool: + """Mirror of `.github/workflows/ci.yml`'s "Check documentation-risk PR + contract" step condition: only an agent-marked pull request runs this + check. An ordinary human PR (no `warpy-factory` label) has no + `## Documentation risk` section and must not be failed for lacking one; a + `push` event has no PR body to fetch at all. Kept here, tested, and + mirrored exactly in the workflow `if:` expression so the two never drift. + """ + return event_name == "pull_request" and AGENT_MARKER in pr_labels + + +def _autodiscover_changed_files() -> List[Path]: + try: + result = subprocess.run( + ["git", "diff", "--name-only", "--diff-filter=d", "origin/main...HEAD", "--", _DOCS_ROOT], + capture_output=True, text=True, check=True, + ) + except (subprocess.CalledProcessError, OSError) as exc: + raise RuntimeError(f"could not determine changed files vs origin/main...HEAD: {exc}") from exc + files = [] + for line in result.stdout.strip().splitlines(): + line = line.strip() + if not line or not (line.endswith(".md") or line.endswith(".mdx")): + continue + p = Path(line) + if p.exists(): + files.append(p) + return files + + +def _load_authorized_reviewers_from_file(path: Path) -> List[str]: + """Offline/local-testing helper only -- reads a plain file path, with no + trust guarantee. CI must always use `_load_authorized_reviewers_from_ref`. + """ + if not path.exists(): + return [] + try: + data = json.loads(path.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError): + return [] + return list(data.get("authorized_docs_reviewers", [])) + + +def _load_authorized_reviewers_from_ref(ref: str, repo_path: str = _AUTHORIZED_REVIEWERS_REPO_PATH) -> List[str]: + """Load the authorized-override allowlist from a trusted ref, never from + the PR's own working-tree checkout. A PR that edits this file to add its + own author must not thereby authorize its own override -- so this always + reads the version committed on `ref` (the base branch by default), + ignoring any local modification in the current checkout entirely. + """ + try: + result = subprocess.run( + ["git", "show", f"{ref}:{repo_path}"], + capture_output=True, text=True, check=True, + ) + except (subprocess.CalledProcessError, OSError): + return [] + try: + data = json.loads(result.stdout) + except json.JSONDecodeError: + return [] + return list(data.get("authorized_docs_reviewers", [])) + + +@dataclass +class LiveReviewSignals: + deterministic_checks_passed: bool + source_owner_approved_current_head: bool + has_unresolved_critical_or_important_finding: bool + approved_reviewers_current_head: Sequence[str] + + +def _run_gh_json(args: List[str]): + result = subprocess.run(["gh", *args], capture_output=True, text=True) + if result.returncode != 0: + raise RuntimeError(f"gh {' '.join(args)} failed: {result.stderr.strip()}") + return json.loads(result.stdout) if result.stdout.strip() else None + + +def _resolve_required_checks_passed(repo: str, head_sha: str) -> bool: + """True only when every required check name has a successful conclusion + on `head_sha`, verified live rather than assumed. A missing, pending, or + failed run -- or an unreachable API -- is not-passed, never passed. + """ + try: + data = _run_gh_json(["api", f"repos/{repo}/commits/{head_sha}/check-runs", "--paginate"]) + except RuntimeError: + return False + runs = (data or {}).get("check_runs", []) + conclusions = {} + for run in runs: + name = run.get("name") + if name in _REQUIRED_CHECK_NAMES: + conclusions[name] = run.get("conclusion") + return all(conclusions.get(name) == "success" for name in _REQUIRED_CHECK_NAMES) + + +def _fetch_reviews(repo: str, pr_number: str) -> List[dict]: + return _run_gh_json(["api", f"repos/{repo}/pulls/{pr_number}/reviews", "--paginate"]) or [] + + +def _fetch_requested_reviewers(repo: str, pr_number: str) -> List[str]: + data = _run_gh_json(["pr", "view", pr_number, "--repo", repo, "--json", "reviewRequests"]) + return [r.get("login") for r in (data or {}).get("reviewRequests", []) if r.get("login")] +def _review_records_override(review: dict, override: Optional[policy.DocumentationRisk]) -> bool: + """True when an approved current-head review records every override field.""" + if override is None or override.docs_override == policy.OVERRIDE_MODE_NONE: + return False + body = review.get("body") or "" + fields = { + "Docs override": override.docs_override, + "Override reviewer": override.override_reviewer, + "Override reason": override.override_reason, + "Override evidence": override.override_evidence, + "Override head SHA": override.override_head_sha, + } + return all(value and f"{name}: {value}" in body for name, value in fields.items()) + + +def _compute_review_signals( + reviews: List[dict], + requested_reviewers: Sequence[str], + head_sha: str, + checks_passed: bool, + override: Optional[policy.DocumentationRisk] = None, +) -> LiveReviewSignals: + """Pure computation over already-fetched review data, kept separate from + the `gh` calls so the review-state logic (stale-review exclusion, latest- + state-wins, source-owner matching) has fixture coverage without a live + API dependency. + + A review's `commit_id` is the head SHA it was submitted against, so + filtering on it is what makes a stale review (submitted before the latest + push) not count -- the reviewer must re-review the current head. + """ + head_reviews = [r for r in reviews if r.get("commit_id") == head_sha and r.get("state") in ("APPROVED", "CHANGES_REQUESTED")] + latest_state_by_user: dict = {} + for review in head_reviews: + user = (review.get("user") or {}).get("login") + if user: + latest_state_by_user[user] = review.get("state") # last one wins; reviews arrive in submission order + + has_unresolved = any(state == "CHANGES_REQUESTED" for state in latest_state_by_user.values()) + approvers = {user for user, state in latest_state_by_user.items() if state == "APPROVED"} + source_owner_approved = bool(approvers & set(requested_reviewers)) + override_approvers = { + (review.get("user") or {}).get("login") + for review in head_reviews + if review.get("state") == "APPROVED" and _review_records_override(review, override) + } + + return LiveReviewSignals( + checks_passed, + source_owner_approved, + has_unresolved, + tuple(sorted(user for user in override_approvers if user)), + ) + + +def resolve_live_review_signals( + repo: str, + pr_number: str, + head_sha: str, + declared_requested_reviewers: Sequence[str] = (), + override: Optional[policy.DocumentationRisk] = None, +) -> LiveReviewSignals: + """Derive the engineering-review gate signals from live GitHub state for + the exact current head SHA, instead of trusting caller-supplied flags. + Any lookup failure fails closed: checks are treated as not-passed, no + approval is treated as found, and an unresolved finding is assumed -- + the same "never convert an unavailable result into success" rule the + final mergeability gate uses. + """ + checks_passed = _resolve_required_checks_passed(repo, head_sha) + + try: + reviews = _fetch_reviews(repo, pr_number) + except RuntimeError: + return LiveReviewSignals(checks_passed, False, True, ()) + + try: + requested_reviewers = _fetch_requested_reviewers(repo, pr_number) + except RuntimeError: + requested_reviewers = [] + + requested_reviewers = sorted(set(requested_reviewers) | set(declared_requested_reviewers)) + return _compute_review_signals(reviews, requested_reviewers, head_sha, checks_passed, override) + + +def _collect_verify_markers(files: List[Path]) -> List[str]: + markers: List[str] = [] + seen = set() + for f in files: + try: + text = f.read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError): + continue + for marker in policy.find_verify_markers(text): + if marker not in seen: + seen.add(marker) + markers.append(marker) + return markers + + +def main(argv: Optional[List[str]] = None) -> int: + parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + parser.add_argument("--body", required=True, help="path to the PR body file") + parser.add_argument("changed_files", nargs="*", help="changed content files (auto-discovered if omitted)") + parser.add_argument("--head-sha", help="current PR head SHA") + parser.add_argument("--repo", help="owner/repo; when given with --pr, resolves gate signals live from GitHub") + parser.add_argument("--pr", help="PR number; when given with --repo, resolves gate signals live from GitHub") + parser.add_argument( + "--authorized-reviewers-ref", default="origin/main", + help="trusted git ref to load the authorized-reviewer allowlist from (never the PR's own working tree)", + ) + parser.add_argument( + "--authorized-reviewers-file", + help="offline/local testing ONLY: read the allowlist directly from this file instead of " + "--authorized-reviewers-ref. Never pass this in CI -- it reads the PR's own working tree.", + ) + parser.add_argument( + "--deterministic-checks-failed", action="store_true", + help="offline dry run only (ignored when --repo/--pr are given): pass when checks did not pass", + ) + parser.add_argument( + "--unresolved-important-finding", action="store_true", + help="offline dry run only (ignored when --repo/--pr are given): pass when a finding is unresolved", + ) + parser.add_argument( + "--source-owner-approved", action="store_true", + help="offline dry run only (ignored when --repo/--pr are given): pass when an engineer approved", + ) + parser.add_argument( + "--approved-reviewer", action="append", default=[], + help="offline dry run only: current-head approver; repeat once per approved GitHub handle", + ) + parser.add_argument( + "--enforce-engineering-gate", action="store_true", + help="evaluate current-head human approval and override requirements; omit for structural CI validation", + ) + args = parser.parse_args(argv) + + body_path = Path(args.body) + try: + body = body_path.read_text(encoding="utf-8") + except OSError as exc: + print(f"error: could not read {args.body}: {exc}", file=sys.stderr) + return 2 + risk_section = policy.parse_documentation_risk_section(body) + + if args.changed_files: + changed_files = [Path(p) for p in args.changed_files] + else: + try: + changed_files = _autodiscover_changed_files() + except RuntimeError as exc: + print(f"error: {exc}", file=sys.stderr) + return 2 + + verify_markers = _collect_verify_markers(changed_files) + if args.authorized_reviewers_file: + authorized_reviewers = _load_authorized_reviewers_from_file(Path(args.authorized_reviewers_file)) + else: + authorized_reviewers = _load_authorized_reviewers_from_ref(args.authorized_reviewers_ref) + + if args.enforce_engineering_gate and args.repo and args.pr: + if not args.head_sha: + print("error: --head-sha is required together with --repo/--pr", file=sys.stderr) + return 2 + declared_requested_reviewers = ( + risk_section.requested_engineering_reviewers if risk_section is not None else () + ) + signals = resolve_live_review_signals( + args.repo, + args.pr, + args.head_sha, + declared_requested_reviewers, + risk_section, + ) + deterministic_checks_passed = signals.deterministic_checks_passed + has_unresolved_finding = signals.has_unresolved_critical_or_important_finding + source_owner_approved = signals.source_owner_approved_current_head + print( + f"Live gate signals for {args.repo}#{args.pr}@{args.head_sha}: " + f"checks_passed={deterministic_checks_passed} " + f"source_owner_approved={source_owner_approved} " + f"unresolved_finding={has_unresolved_finding}" + ) + else: + deterministic_checks_passed = not args.deterministic_checks_failed + has_unresolved_finding = args.unresolved_important_finding + source_owner_approved = args.source_owner_approved + approved_reviewers_current_head = tuple(args.approved_reviewer) + + if args.enforce_engineering_gate and args.repo and args.pr: + approved_reviewers_current_head = signals.approved_reviewers_current_head + + problems = policy.validate_pr_contract( + body, + verify_markers, + current_head_sha=args.head_sha, + authorized_docs_reviewers=authorized_reviewers, + deterministic_checks_passed=deterministic_checks_passed, + has_unresolved_critical_or_important_finding=has_unresolved_finding, + source_owner_approved_current_head=source_owner_approved, + approved_reviewers_current_head=approved_reviewers_current_head, + enforce_engineering_gate=args.enforce_engineering_gate, + ) + + if problems: + print("Documentation risk contract check FAILED:\n", file=sys.stderr) + for problem in problems: + print(f" - {problem}", file=sys.stderr) + return 1 + + print( + f"Documentation risk contract check passed " + f"({len(verify_markers)} VERIFY marker(s), {len(changed_files)} changed file(s))." + ) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.agents/skills/doc_quality_policy/finalize_pr_contract.py b/.agents/skills/doc_quality_policy/finalize_pr_contract.py new file mode 100644 index 000000000..01ed5f15d --- /dev/null +++ b/.agents/skills/doc_quality_policy/finalize_pr_contract.py @@ -0,0 +1,120 @@ +#!/usr/bin/env python3 +"""Build the `## Documentation risk` PR-body block. + +PR-producing skills call this before requesting review to render the +machine-readable risk section in the shape `policy.py` parses. It does not +call `gh` itself — the invoking skill applies the `warpy-factory` label +(`gh pr edit --add-label warpy-factory`) and inserts the printed block into +the PR body it already assembles (see `create_pr/SKILL.md`). + +Usage: + python3 finalize_pr_contract.py build --risk low \\ + --rationale "Spelling and link-text fixes only; no technical claims changed." + + python3 finalize_pr_contract.py build --risk engineering-review-required \\ + --rationale "Documents a new CLI flag." \\ + --source-files app/src/cli/args.rs@abc123 \\ + --reviewers alice \\ + --engineering-review-status pending + + python3 finalize_pr_contract.py build --risk engineering-review-required \\ + --rationale "Documents a new CLI flag; engineer did not respond in time." \\ + --override-mode docs-waiver \\ + --override-reviewer hongyi-chen \\ + --override-reason "No source-owner response after 3 days; risk is limited to wording." \\ + --override-evidence "app/src/cli/args.rs@abc123" \\ + --override-head-sha "$HEAD_SHA" +""" +from __future__ import annotations + +import argparse +import importlib.util +import sys +from pathlib import Path +from typing import List, Optional + +_HERE = Path(__file__).resolve().parent +_spec = importlib.util.spec_from_file_location("doc_quality_policy", _HERE / "policy.py") +policy = importlib.util.module_from_spec(_spec) +sys.modules[_spec.name] = policy # required for dataclass field resolution +_spec.loader.exec_module(policy) + + +def build_documentation_risk_block( + *, + risk: str, + rationale: str, + source_files: Optional[List[str]] = None, + reviewers: Optional[List[str]] = None, + engineering_review_status: Optional[str] = None, + override_mode: str = policy.OVERRIDE_MODE_NONE, + override_reviewer: Optional[str] = None, + override_reason: Optional[str] = None, + override_evidence: Optional[str] = None, + override_head_sha: Optional[str] = None, +) -> str: + if risk not in policy.VALID_RISK_LEVELS: + raise ValueError(f"risk must be one of {policy.VALID_RISK_LEVELS}, got {risk!r}") + if override_mode not in policy.VALID_OVERRIDE_MODES: + raise ValueError(f"override_mode must be one of {policy.VALID_OVERRIDE_MODES}, got {override_mode!r}") + + lines = [policy.DOC_RISK_HEADING, f"Risk: {risk}", f"Rationale: {rationale}"] + if source_files: + lines.append(f"Source files consulted: {', '.join(source_files)}") + if reviewers: + lines.append(f"Requested engineering reviewers: {', '.join(reviewers)}") + if engineering_review_status: + lines.append(f"Engineering review status: {engineering_review_status}") + lines.append(f"Docs override: {override_mode}") + if override_mode != policy.OVERRIDE_MODE_NONE: + lines.append(f"Override reviewer: {override_reviewer or ''}") + lines.append(f"Override reason: {override_reason or ''}") + lines.append(f"Override evidence: {override_evidence or ''}") + lines.append(f"Override head SHA: {override_head_sha or ''}") + return "\n".join(lines) + + +def main(argv: Optional[List[str]] = None) -> int: + parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + sub = parser.add_subparsers(dest="command", required=True) + + build = sub.add_parser("build", help="print the ## Documentation risk block") + build.add_argument("--risk", required=True, choices=policy.VALID_RISK_LEVELS) + build.add_argument("--rationale", required=True) + build.add_argument("--source-files", help="comma-separated file@sha list") + build.add_argument("--reviewers", help="comma-separated GitHub handles") + build.add_argument("--engineering-review-status", choices=("not-applicable", "pending", "approved")) + build.add_argument("--override-mode", default=policy.OVERRIDE_MODE_NONE, choices=policy.VALID_OVERRIDE_MODES) + build.add_argument("--override-reviewer") + build.add_argument("--override-reason") + build.add_argument("--override-evidence") + build.add_argument("--override-head-sha") + + args = parser.parse_args(argv) + + if args.command == "build": + block = build_documentation_risk_block( + risk=args.risk, + rationale=args.rationale, + source_files=[s.strip() for s in args.source_files.split(",")] if args.source_files else None, + reviewers=[s.strip() for s in args.reviewers.split(",")] if args.reviewers else None, + engineering_review_status=args.engineering_review_status, + override_mode=args.override_mode, + override_reviewer=args.override_reviewer, + override_reason=args.override_reason, + override_evidence=args.override_evidence, + override_head_sha=args.override_head_sha, + ) + print(block) + print( + f"\nReminder: apply the marker label with " + f"'gh pr edit --add-label {policy.AGENT_MARKER}'.", + file=sys.stderr, + ) + return 0 + + return 2 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.agents/skills/doc_quality_policy/policy.py b/.agents/skills/doc_quality_policy/policy.py new file mode 100644 index 000000000..ca7d70eab --- /dev/null +++ b/.agents/skills/doc_quality_policy/policy.py @@ -0,0 +1,415 @@ +#!/usr/bin/env python3 +"""Shared agent-doc quality policy for warpdotdev/docs. + +Pure, stdlib-only functions for: + * the canonical agent marker and risk levels, + * parsing the `## Documentation risk` and `## Unverified claims` PR-body + sections, + * finding `{/* VERIFY: ... */}` markers in changed content, + * classifying risk from explicit diff signals (the low-risk allowlist), and + * validating the human-gate contract, including docs-team overrides. + +See `.agents/references/doc-quality-policy.md` for the rules this module +enforces. This module has no side effects (no `gh`, no network, no git) so it +stays trivially unit-testable; `check_pr_contract.py` wraps it for CI/CLI use. +""" +from __future__ import annotations + +import re +from dataclasses import dataclass, fields +from typing import Dict, List, Optional, Sequence + +# --------------------------------------------------------------------------- +# Constants +# --------------------------------------------------------------------------- + +AGENT_MARKER = "warpy-factory" + +RISK_LOW = "low" +RISK_ENGINEERING_REVIEW_REQUIRED = "engineering-review-required" +VALID_RISK_LEVELS = (RISK_LOW, RISK_ENGINEERING_REVIEW_REQUIRED) + +DOC_RISK_HEADING = "## Documentation risk" +UNVERIFIED_CLAIMS_HEADING = "## Unverified claims" + +OVERRIDE_MODE_VERIFIED = "docs-verified" +OVERRIDE_MODE_WAIVER = "docs-waiver" +OVERRIDE_MODE_NONE = "none" +VALID_OVERRIDE_MODES = (OVERRIDE_MODE_VERIFIED, OVERRIDE_MODE_WAIVER, OVERRIDE_MODE_NONE) + +# {/* VERIFY: flag name from PRD, unconfirmed against warp-internal */} +VERIFY_MARKER_RE = re.compile(r"\{/\*\s*VERIFY:\s*(.*?)\s*\*/\}", re.DOTALL) + + +# --------------------------------------------------------------------------- +# VERIFY marker accounting +# --------------------------------------------------------------------------- + +def find_verify_markers(text: str) -> List[str]: + """Return the claim text of every `{/* VERIFY: ... */}` marker in `text`.""" + return [m.group(1).strip() for m in VERIFY_MARKER_RE.finditer(text)] + + +# --------------------------------------------------------------------------- +# PR-body section parsing +# --------------------------------------------------------------------------- + +def extract_section(body: str, heading: str) -> Optional[str]: + """Return the text under `heading` up to the next heading of the same-or- + higher level, or None if the heading is absent. + """ + lines = body.splitlines() + level = len(heading) - len(heading.lstrip("#")) + start = None + for i, line in enumerate(lines): + if line.strip() == heading.strip(): + start = i + 1 + break + if start is None: + return None + end = len(lines) + for i in range(start, len(lines)): + stripped = lines[i] + m = re.match(r"^(#{1,6})\s+\S", stripped) + if m and len(m.group(1)) <= level: + end = i + break + return "\n".join(lines[start:end]).strip("\n") + + +_FIELD_LINE_RE = re.compile(r"^\s*([A-Za-z][A-Za-z ]*[A-Za-z]):\s*(.*)$") + +# Maps a human-readable field label (case-insensitive) to the canonical key +# used in the parsed dict. +_FIELD_LABELS = { + "risk": "risk", + "rationale": "rationale", + "source files consulted": "source_files_consulted", + "requested engineering reviewers": "requested_engineering_reviewers", + "engineering review status": "engineering_review_status", + "docs override": "docs_override", + "override reviewer": "override_reviewer", + "override reason": "override_reason", + "override evidence": "override_evidence", + "override head sha": "override_head_sha", +} + +_LIST_FIELDS = {"source_files_consulted", "requested_engineering_reviewers"} + + +@dataclass +class DocumentationRisk: + risk: Optional[str] = None + rationale: Optional[str] = None + source_files_consulted: List[str] = None + requested_engineering_reviewers: List[str] = None + engineering_review_status: Optional[str] = None + docs_override: str = OVERRIDE_MODE_NONE + override_reviewer: Optional[str] = None + override_reason: Optional[str] = None + override_evidence: Optional[str] = None + override_head_sha: Optional[str] = None + + def __post_init__(self) -> None: + if self.source_files_consulted is None: + self.source_files_consulted = [] + if self.requested_engineering_reviewers is None: + self.requested_engineering_reviewers = [] + + +def parse_documentation_risk_section(body: str) -> Optional[DocumentationRisk]: + """Parse the `## Documentation risk` section into a `DocumentationRisk`. + + Returns None when the section is absent. Unrecognized lines are ignored + so free-form prose alongside the `Key: value` lines does not break parsing. + """ + section = extract_section(body, DOC_RISK_HEADING) + if section is None: + return None + + values: Dict[str, str] = {} + for line in section.splitlines(): + m = _FIELD_LINE_RE.match(line) + if not m: + continue + label = m.group(1).strip().lower() + key = _FIELD_LABELS.get(label) + if key is None: + continue + values[key] = m.group(2).strip() + + kwargs: Dict[str, object] = {} + for key, raw in values.items(): + if key in _LIST_FIELDS: + kwargs[key] = [item.strip() for item in raw.split(",") if item.strip()] + else: + kwargs[key] = raw + + risk = kwargs.get("risk") + if isinstance(risk, str): + kwargs["risk"] = risk.strip().lower() + docs_override = kwargs.get("docs_override") + if isinstance(docs_override, str): + kwargs["docs_override"] = docs_override.strip().lower() + else: + kwargs["docs_override"] = OVERRIDE_MODE_NONE + + return DocumentationRisk(**kwargs) + + +def parse_unverified_claims_section(body: str) -> Optional[List[str]]: + """Return the bullet items under `## Unverified claims`, or None if absent. + + A section containing only the "None — ..." sentinel line returns []. + """ + section = extract_section(body, UNVERIFIED_CLAIMS_HEADING) + if section is None: + return None + items: List[str] = [] + for line in section.splitlines(): + stripped = line.strip() + if not stripped: + continue + if stripped.lower().startswith("none"): + continue + if stripped.startswith(("-", "*")): + items.append(stripped.lstrip("-* ").strip()) + return items + + +# --------------------------------------------------------------------------- +# Risk classification (the low-risk allowlist) +# --------------------------------------------------------------------------- + +@dataclass +class RiskSignals: + """Explicit, per-category signals describing what a diff touches. + + Each field is one of the engineering-review triggers in the low-risk + allowlist (see `.agents/references/doc-quality-policy.md`). Every field + defaults to True (the conservative/unknown default) so a caller must + affirmatively clear a trigger before it can be excluded — this encodes + "ambiguous risk classification defaults to engineering-review-required" + at the type level rather than relying on every call site remembering it. + """ + + adds_new_or_changed_feature_page: bool = True + changes_commands_or_code_examples: bool = True + changes_api_behavior: bool = True + changes_ui_labels_or_paths: bool = True + changes_defaults: bool = True + changes_permissions: bool = True + changes_availability_or_platform_support: bool = True + changes_plan_eligibility: bool = True + changes_billing_behavior: bool = True + changes_security_or_privacy_claims: bool = True + changes_data_handling: bool = True + changes_self_hosting_behavior: bool = True + changes_integration_setup: bool = True + # Low risk requires a positive match to one of these allowlisted + # categories after every technical-claim trigger above has been cleared. + is_editorial_or_metadata_only: bool = False + # Internal docs tooling, skills, and CI changes that make no public + # product claim may use the low-risk path when every technical-claim + # trigger above has been affirmatively cleared. + is_docs_workflow_tooling_only: bool = False + + # Not allowlist triggers themselves, but always force engineering review + # when true, per the VERIFY-accounting and review-severity rules. + has_unresolved_verify_marker: bool = False + has_critical_or_important_review_finding: bool = False + + @classmethod + def all_clear(cls, **overrides: bool) -> "RiskSignals": + """Build signals with every allowlist trigger cleared (False) except + `overrides`. Convenience for tests and callers who have positively + confirmed a PR touches none of the trigger categories. + """ + clear = {f.name: False for f in fields(cls) if f.name not in ( + "has_unresolved_verify_marker", "has_critical_or_important_review_finding", + )} + clear["is_editorial_or_metadata_only"] = True + clear.update(overrides) + return cls(**clear) + + +_ALLOWLIST_TRIGGER_FIELDS: Sequence[str] = tuple( + f.name for f in fields(RiskSignals) + if f.name not in ( + "is_docs_workflow_tooling_only", + "is_editorial_or_metadata_only", + "has_unresolved_verify_marker", + "has_critical_or_important_review_finding", + ) +) + + +def classify_risk(signals: RiskSignals) -> str: + """Classify a PR's risk from explicit low-risk-allowlist signals. + + Any allowlist trigger, an unresolved VERIFY marker, or a critical/ + important review finding forces `engineering-review-required`. Only a PR + with every trigger cleared is `low`. + """ + if signals.has_unresolved_verify_marker: + return RISK_ENGINEERING_REVIEW_REQUIRED + if signals.has_critical_or_important_review_finding: + return RISK_ENGINEERING_REVIEW_REQUIRED + if any(getattr(signals, name) for name in _ALLOWLIST_TRIGGER_FIELDS): + return RISK_ENGINEERING_REVIEW_REQUIRED + if not (signals.is_editorial_or_metadata_only or signals.is_docs_workflow_tooling_only): + return RISK_ENGINEERING_REVIEW_REQUIRED + return RISK_LOW + + +# --------------------------------------------------------------------------- +# Contract validation +# --------------------------------------------------------------------------- + +def validate_pr_contract( + body: str, + verify_markers: Sequence[str], + *, + current_head_sha: Optional[str] = None, + authorized_docs_reviewers: Sequence[str] = (), + deterministic_checks_passed: bool = True, + has_unresolved_critical_or_important_finding: bool = False, + source_owner_approved_current_head: bool = False, + approved_reviewers_current_head: Sequence[str] = (), + enforce_engineering_gate: bool = False, +) -> List[str]: + """Validate the PR contract. Returns a list of violation messages. + + Structural validation (the required CI check) always verifies the risk + metadata and VERIFY accounting. The human engineering gate is deliberately + opt-in because a pending approval is normal while an + engineering-review-required PR is being drafted and reviewed. + """ + problems: List[str] = [] + + risk_section = parse_documentation_risk_section(body) + if risk_section is None: + return [f"missing required section: {DOC_RISK_HEADING!r}"] + + claims = parse_unverified_claims_section(body) + if claims is None: + problems.append(f"missing required section: {UNVERIFIED_CLAIMS_HEADING!r}") + claims = [] + + if risk_section.risk not in VALID_RISK_LEVELS: + problems.append( + f"invalid risk level {risk_section.risk!r}; must be one of {VALID_RISK_LEVELS}" + ) + + unlisted = [m for m in verify_markers if m not in claims] + if unlisted: + problems.append( + f"{len(unlisted)} VERIFY marker(s) not listed in {UNVERIFIED_CLAIMS_HEADING!r}: " + f"{unlisted}" + ) + + if verify_markers and risk_section.risk == RISK_LOW: + problems.append( + "risk is 'low' but the diff has an unresolved VERIFY marker; " + f"must be {RISK_ENGINEERING_REVIEW_REQUIRED!r}" + ) + + if enforce_engineering_gate and risk_section.risk == RISK_ENGINEERING_REVIEW_REQUIRED: + problems.extend( + validate_engineering_gate( + risk_section, + current_head_sha=current_head_sha, + authorized_docs_reviewers=authorized_docs_reviewers, + deterministic_checks_passed=deterministic_checks_passed, + has_unresolved_critical_or_important_finding=has_unresolved_critical_or_important_finding, + source_owner_approved_current_head=source_owner_approved_current_head, + approved_reviewers_current_head=approved_reviewers_current_head, + ) + ) + + return problems + + +def validate_engineering_gate( + risk_section: DocumentationRisk, + *, + current_head_sha: Optional[str], + authorized_docs_reviewers: Sequence[str], + deterministic_checks_passed: bool, + has_unresolved_critical_or_important_finding: bool, + source_owner_approved_current_head: bool, + approved_reviewers_current_head: Sequence[str] = (), +) -> List[str]: + """Validate the engineering-review-required human gate for one PR. + + Satisfied by a source-owner approval on the current head, or by a + complete, non-stale, authorized docs-team override. Neither path can + substitute for a failing deterministic check or an unresolved + critical/important review finding. + """ + problems: List[str] = [] + + blocking = not deterministic_checks_passed or has_unresolved_critical_or_important_finding + if blocking: + if not deterministic_checks_passed: + problems.append("deterministic checks have not passed; no override can bypass this") + if has_unresolved_critical_or_important_finding: + problems.append( + "an unresolved critical/important review-docs-pr finding exists; " + "no override can bypass this" + ) + + if source_owner_approved_current_head and not blocking: + return problems + + override_mode = risk_section.docs_override or OVERRIDE_MODE_NONE + if override_mode == OVERRIDE_MODE_NONE: + if not blocking: + problems.append( + "engineering-review-required PR has no source-owner approval on the " + "current head and no docs-team override recorded" + ) + return problems + + if override_mode not in (OVERRIDE_MODE_VERIFIED, OVERRIDE_MODE_WAIVER): + problems.append(f"invalid docs override mode: {override_mode!r}") + return problems + + if blocking: + problems.append( + f"docs override ({override_mode}) cannot satisfy the gate while a " + "deterministic check is failing or a critical/important finding is unresolved" + ) + return problems + + required = ("override_reviewer", "override_reason", "override_evidence", "override_head_sha") + missing = [f for f in required if not getattr(risk_section, f)] + if missing: + problems.append(f"docs override ({override_mode}) missing required field(s): {missing}") + return problems + + # No truthy-guard on `authorized_docs_reviewers`: an empty allowlist (a + # failed or unavailable trusted-ref lookup) must fail closed -- treated + # as "no one is authorized" -- never as "the allowlist check is skipped". + if risk_section.override_reviewer not in authorized_docs_reviewers: + problems.append( + f"docs override author {risk_section.override_reviewer!r} is not an " + "authorized Pod-Docs reviewer" + ) + elif risk_section.override_reviewer not in approved_reviewers_current_head: + problems.append( + f"docs override author {risk_section.override_reviewer!r} has not approved " + "the current head" + ) + + if not current_head_sha: + problems.append( + "cannot verify docs override freshness: no current head SHA was supplied" + ) + elif risk_section.override_head_sha != current_head_sha: + problems.append( + f"docs override head SHA {risk_section.override_head_sha!r} does not match " + f"the current head {current_head_sha!r}; a new commit invalidates the override" + ) + + return problems diff --git a/.agents/skills/doc_quality_policy/test_check_compression_contract.py b/.agents/skills/doc_quality_policy/test_check_compression_contract.py new file mode 100644 index 000000000..60356fa52 --- /dev/null +++ b/.agents/skills/doc_quality_policy/test_check_compression_contract.py @@ -0,0 +1,87 @@ +#!/usr/bin/env python3 +"""Unit tests for check_compression_contract.py. + +Run: + python3 .agents/skills/doc_quality_policy/test_check_compression_contract.py +""" +from __future__ import annotations + +import importlib.util +import unittest +from pathlib import Path + +_HERE = Path(__file__).resolve().parent +_spec = importlib.util.spec_from_file_location("check_compression_contract", _HERE / "check_compression_contract.py") +ccc = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(ccc) + + +def _page(word_count: int, callouts: int = 0) -> str: + words = " ".join(f"word{i}" for i in range(word_count)) + callout_blocks = "\n".join( + f":::note\nnote {i}\n:::\n" for i in range(callouts) + ) + return f"---\ndescription: test\n---\n{callout_blocks}\n{words}\n" + + +class TestCountWords(unittest.TestCase): + def test_counts_body_words_excluding_frontmatter(self): + text = "---\ntitle: x\ndescription: y\n---\none two three\n" + self.assertEqual(ccc.count_words(text), 3) + + def test_excludes_code_fences(self): + text = "---\ndescription: y\n---\none two\n```\ncode code code\n```\nthree\n" + self.assertEqual(ccc.count_words(text), 3) + + def test_link_text_counts_but_url_does_not(self): + text = "---\ndescription: y\n---\nSee [the docs](https://example.com/a/b/c) here.\n" + # "See", "the", "docs", "here." = 4 words; URL is dropped. + self.assertEqual(ccc.count_words(text), 4) + + +class TestCountCallouts(unittest.TestCase): + def test_counts_callout_blocks(self): + text = "---\ndescription: y\n---\n:::note\nhi\n:::\n:::tip\nyo\n:::\n" + self.assertEqual(ccc.count_callouts(text), 2) + + def test_ignores_callout_examples_inside_code_fences(self): + text = ":::note\nreal\n:::\n```md\n:::tip\nexample\n:::\n```\n" + self.assertEqual(ccc.count_callouts(text), 1) + + +class TestCheckCompressionContract(unittest.TestCase): + def test_quickstart_within_budget_passes(self): + text = _page(500) + self.assertEqual(ccc.check_compression_contract(text, "quickstart"), []) + + def test_quickstart_over_budget_fails(self): + text = _page(700) + findings = ccc.check_compression_contract(text, "quickstart") + self.assertTrue(any("exceeds the quickstart budget" in f for f in findings)) + + def test_feature_doc_over_budget_fails(self): + text = _page(1600) + findings = ccc.check_compression_contract(text, "feature-doc") + self.assertTrue(any("exceeds the feature-doc budget" in f for f in findings)) + + def test_generated_changelog_is_exempt_from_word_budget(self): + text = _page(5000) + self.assertEqual(ccc.check_compression_contract(text, "changelog"), []) + + def test_generated_changelog_still_enforces_callout_budget(self): + text = _page(5000, callouts=3) + findings = ccc.check_compression_contract(text, "changelog") + self.assertTrue(any("callouts exceed" in f for f in findings)) + + def test_too_many_callouts_fails(self): + text = _page(100, callouts=3) + findings = ccc.check_compression_contract(text, "quickstart") + self.assertTrue(any("callouts exceed" in f for f in findings)) + + def test_two_callouts_is_within_budget(self): + text = _page(100, callouts=2) + self.assertEqual(ccc.check_compression_contract(text, "quickstart"), []) + + +if __name__ == "__main__": + unittest.main() diff --git a/.agents/skills/doc_quality_policy/test_check_pr_contract.py b/.agents/skills/doc_quality_policy/test_check_pr_contract.py new file mode 100644 index 000000000..19e1eef73 --- /dev/null +++ b/.agents/skills/doc_quality_policy/test_check_pr_contract.py @@ -0,0 +1,296 @@ +#!/usr/bin/env python3 +"""Unit tests for check_pr_contract.py. + +Stdlib unittest only, no third-party deps and no network. Exercises the CLI +via `main()` directly (no subprocess) so coverage tools see it, with explicit +changed-file arguments to avoid depending on git state. + +Run: + python3 .agents/skills/doc_quality_policy/test_check_pr_contract.py +""" +from __future__ import annotations + +import importlib.util +import json +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +_HERE = Path(__file__).resolve().parent + +_policy_spec = importlib.util.spec_from_file_location("doc_quality_policy", _HERE / "policy.py") +policy = importlib.util.module_from_spec(_policy_spec) +sys.modules[_policy_spec.name] = policy +_policy_spec.loader.exec_module(policy) + +_spec = importlib.util.spec_from_file_location("check_pr_contract", _HERE / "check_pr_contract.py") +cpc = importlib.util.module_from_spec(_spec) +sys.modules[_spec.name] = cpc +_spec.loader.exec_module(cpc) + + +LOW_RISK_BODY = ( + "## Documentation risk\nRisk: low\nRationale: typo fix.\nDocs override: none\n\n" + "## Unverified claims\nNone — all claims verified against source.\n" +) + + +class TestCheckPrContractCli(unittest.TestCase): + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.addCleanup(self._tmp.cleanup) + self.tmp_path = Path(self._tmp.name) + + def _write(self, name: str, content: str) -> Path: + p = self.tmp_path / name + p.write_text(content, encoding="utf-8") + return p + + def test_clean_low_risk_body_passes(self): + body = self._write("body.md", LOW_RISK_BODY) + content = self._write("page.mdx", "no verify markers here") + self.assertEqual(cpc.main(["--body", str(body), str(content)]), 0) + + def test_unlisted_verify_marker_fails(self): + body = self._write("body.md", LOW_RISK_BODY) + content = self._write("page.mdx", "{/* VERIFY: some claim */}") + self.assertEqual(cpc.main(["--body", str(body), str(content)]), 1) + + def test_engineering_review_with_approval_and_listed_marker_passes(self): + body_text = ( + "## Documentation risk\nRisk: engineering-review-required\nRationale: new flag.\n" + "Requested engineering reviewers: alice\nEngineering review status: pending\n" + "Docs override: none\n\n## Unverified claims\n- some claim\n" + ) + body = self._write("body.md", body_text) + content = self._write("page.mdx", "{/* VERIFY: some claim */}") + self.assertEqual( + cpc.main(["--body", str(body), str(content), "--source-owner-approved"]), 0, + ) + + def _write_authorized_reviewers_fixture(self) -> Path: + return self._write( + "authorized.json", json.dumps({"authorized_docs_reviewers": ["hongyi-chen"]}), + ) + + def test_docs_verified_override_with_current_head_sha_passes(self): + body_text = ( + "## Documentation risk\nRisk: engineering-review-required\nRationale: new flag.\n" + "Docs override: docs-verified\nOverride reviewer: hongyi-chen\n" + "Override reason: Confirmed against source.\nOverride evidence: app/foo.rs@abc\n" + "Override head SHA: deadbeef\n\n## Unverified claims\nNone\n" + ) + body = self._write("body.md", body_text) + content = self._write("page.mdx", "no markers") + authorized = self._write_authorized_reviewers_fixture() + self.assertEqual( + cpc.main([ + "--body", str(body), str(content), "--head-sha", "deadbeef", + "--authorized-reviewers-file", str(authorized), + "--enforce-engineering-gate", "--approved-reviewer", "hongyi-chen", + ]), 0, + ) + + def test_docs_verified_override_with_stale_head_sha_fails(self): + body_text = ( + "## Documentation risk\nRisk: engineering-review-required\nRationale: new flag.\n" + "Docs override: docs-verified\nOverride reviewer: hongyi-chen\n" + "Override reason: Confirmed against source.\nOverride evidence: app/foo.rs@abc\n" + "Override head SHA: old-sha\n\n## Unverified claims\nNone\n" + ) + body = self._write("body.md", body_text) + content = self._write("page.mdx", "no markers") + authorized = self._write_authorized_reviewers_fixture() + self.assertEqual( + cpc.main([ + "--body", str(body), str(content), "--head-sha", "new-sha", + "--authorized-reviewers-file", str(authorized), + "--enforce-engineering-gate", "--approved-reviewer", "hongyi-chen", + ]), 1, + ) + + def test_docs_verified_override_by_unauthorized_reviewer_fails_with_no_fixture(self): + """Without an explicit --authorized-reviewers-file and outside a git + checkout with the trusted ref, the allowlist load fails closed to + empty -- so even a plausible-looking override author is rejected. + """ + body_text = ( + "## Documentation risk\nRisk: engineering-review-required\nRationale: new flag.\n" + "Docs override: docs-verified\nOverride reviewer: hongyi-chen\n" + "Override reason: Confirmed against source.\nOverride evidence: app/foo.rs@abc\n" + "Override head SHA: deadbeef\n\n## Unverified claims\nNone\n" + ) + body = self._write("body.md", body_text) + content = self._write("page.mdx", "no markers") + self.assertEqual( + cpc.main([ + "--body", str(body), str(content), "--head-sha", "deadbeef", + "--authorized-reviewers-ref", "refs/does-not-exist", + "--enforce-engineering-gate", "--approved-reviewer", "hongyi-chen", + ]), 1, + ) + + def test_missing_body_file_is_usage_error(self): + self.assertEqual(cpc.main(["--body", str(self.tmp_path / "nope.md")]), 2) + + +class TestShouldRunContractCheck(unittest.TestCase): + def test_agent_marked_pull_request_runs(self): + self.assertTrue(cpc.should_run_contract_check("pull_request", ["warpy-factory", "documentation"])) + + def test_non_agent_pull_request_skips(self): + self.assertFalse(cpc.should_run_contract_check("pull_request", ["documentation"])) + + def test_pull_request_with_no_labels_skips(self): + self.assertFalse(cpc.should_run_contract_check("pull_request", [])) + + def test_push_event_skips_even_with_the_label(self): + self.assertFalse(cpc.should_run_contract_check("push", ["warpy-factory"])) + + +def _git(cmd, cwd): + subprocess.run(["git", *cmd], cwd=cwd, capture_output=True, text=True, check=True) + + +class TestLoadAuthorizedReviewersFromRef(unittest.TestCase): + def test_reads_the_ref_version_not_the_working_tree(self): + """Regression: a PR that edits its own working-tree copy of + authorized_docs_reviewers.json to add its author must not thereby + authorize itself -- the trusted ref's committed version always wins. + """ + with tempfile.TemporaryDirectory() as tmp: + repo = Path(tmp) / "repo" + repo.mkdir() + _git(["init", "-q"], repo) + _git(["config", "user.email", "test@example.com"], repo) + _git(["config", "user.name", "Test"], repo) + rel_path = "authorized_docs_reviewers.json" + (repo / rel_path).write_text( + json.dumps({"authorized_docs_reviewers": ["hongyi-chen"]}), encoding="utf-8", + ) + _git(["add", "."], repo) + _git(["commit", "-q", "-m", "trusted allowlist"], repo) + _git(["update-ref", "refs/remotes/origin/main", "HEAD"], repo) + + # Simulate a malicious PR editing its own working-tree copy to + # add the author, without committing (or on a divergent branch). + (repo / rel_path).write_text( + json.dumps({"authorized_docs_reviewers": ["hongyi-chen", "attacker"]}), encoding="utf-8", + ) + + import os + old_cwd = os.getcwd() + os.chdir(repo) + try: + result = cpc._load_authorized_reviewers_from_ref("origin/main", rel_path) + finally: + os.chdir(old_cwd) + self.assertEqual(result, ["hongyi-chen"]) + self.assertNotIn("attacker", result) + + def test_missing_ref_fails_closed_to_empty_list(self): + with tempfile.TemporaryDirectory() as tmp: + repo = Path(tmp) / "repo" + repo.mkdir() + _git(["init", "-q"], repo) + import os + old_cwd = os.getcwd() + os.chdir(repo) + try: + result = cpc._load_authorized_reviewers_from_ref("origin/main", "nope.json") + finally: + os.chdir(old_cwd) + self.assertEqual(result, []) + + +class TestComputeReviewSignals(unittest.TestCase): + def test_stale_review_on_old_head_does_not_count(self): + reviews = [{"user": {"login": "alice"}, "state": "APPROVED", "commit_id": "old-sha"}] + signals = cpc._compute_review_signals(reviews, ["alice"], "new-sha", checks_passed=True) + self.assertFalse(signals.source_owner_approved_current_head) + + def test_approval_by_a_requested_reviewer_on_current_head_counts(self): + reviews = [{"user": {"login": "alice"}, "state": "APPROVED", "commit_id": "sha1"}] + signals = cpc._compute_review_signals(reviews, ["alice"], "sha1", checks_passed=True) + self.assertTrue(signals.source_owner_approved_current_head) + + def test_approval_by_a_non_requested_reviewer_does_not_count_as_source_owner(self): + reviews = [{"user": {"login": "random-person"}, "state": "APPROVED", "commit_id": "sha1"}] + signals = cpc._compute_review_signals(reviews, ["alice"], "sha1", checks_passed=True) + self.assertFalse(signals.source_owner_approved_current_head) + + def test_changes_requested_on_current_head_is_unresolved(self): + reviews = [{"user": {"login": "alice"}, "state": "CHANGES_REQUESTED", "commit_id": "sha1"}] + signals = cpc._compute_review_signals(reviews, ["alice"], "sha1", checks_passed=True) + self.assertTrue(signals.has_unresolved_critical_or_important_finding) + + def test_later_approval_supersedes_earlier_changes_requested_by_same_reviewer(self): + reviews = [ + {"user": {"login": "alice"}, "state": "CHANGES_REQUESTED", "commit_id": "sha1"}, + {"user": {"login": "alice"}, "state": "APPROVED", "commit_id": "sha1"}, + ] + signals = cpc._compute_review_signals(reviews, ["alice"], "sha1", checks_passed=True) + self.assertFalse(signals.has_unresolved_critical_or_important_finding) + self.assertTrue(signals.source_owner_approved_current_head) + + def test_no_reviews_means_no_approval_and_no_unresolved_finding(self): + signals = cpc._compute_review_signals([], ["alice"], "sha1", checks_passed=True) + self.assertFalse(signals.source_owner_approved_current_head) + self.assertFalse(signals.has_unresolved_critical_or_important_finding) + + def test_declared_source_owner_counts_after_pending_request_is_removed(self): + reviews = [{"user": {"login": "alice"}, "state": "APPROVED", "commit_id": "sha1"}] + with mock.patch.object(cpc, "_resolve_required_checks_passed", return_value=True), \ + mock.patch.object(cpc, "_fetch_reviews", return_value=reviews), \ + mock.patch.object(cpc, "_fetch_requested_reviewers", return_value=[]): + signals = cpc.resolve_live_review_signals("o/r", "1", "sha1", ["alice"]) + self.assertTrue(signals.source_owner_approved_current_head) + + def test_override_requires_the_same_fields_in_an_approved_review_body(self): + override = policy.DocumentationRisk( + risk=policy.RISK_ENGINEERING_REVIEW_REQUIRED, + docs_override=policy.OVERRIDE_MODE_VERIFIED, + override_reviewer="hongyi-chen", + override_reason="Confirmed against source.", + override_evidence="app/foo.rs@abc", + override_head_sha="sha1", + ) + review = { + "user": {"login": "hongyi-chen"}, + "state": "APPROVED", + "commit_id": "sha1", + "body": ( + "Docs override: docs-verified\n" + "Override reviewer: hongyi-chen\n" + "Override reason: Confirmed against source.\n" + "Override evidence: app/foo.rs@abc\n" + "Override head SHA: sha1" + ), + } + signals = cpc._compute_review_signals([review], [], "sha1", True, override) + self.assertEqual(signals.approved_reviewers_current_head, ("hongyi-chen",)) + + def test_unrecorded_override_approval_does_not_count(self): + override = policy.DocumentationRisk( + risk=policy.RISK_ENGINEERING_REVIEW_REQUIRED, + docs_override=policy.OVERRIDE_MODE_VERIFIED, + override_reviewer="hongyi-chen", + override_reason="Confirmed against source.", + override_evidence="app/foo.rs@abc", + override_head_sha="sha1", + ) + review = { + "user": {"login": "hongyi-chen"}, + "state": "APPROVED", + "commit_id": "sha1", + "body": "Looks good.", + } + signals = cpc._compute_review_signals([review], [], "sha1", True, override) + self.assertEqual(signals.approved_reviewers_current_head, ()) + + +if __name__ == "__main__": + unittest.main() diff --git a/.agents/skills/doc_quality_policy/test_manifest.py b/.agents/skills/doc_quality_policy/test_manifest.py new file mode 100644 index 000000000..ea0180ad4 --- /dev/null +++ b/.agents/skills/doc_quality_policy/test_manifest.py @@ -0,0 +1,108 @@ +#!/usr/bin/env python3 +"""Enforce that every PR-producing content skill/script references the shared +v1 agent-doc quality contract. + +This is a "does not bypass the shared finalization path" regression test. +Rather than checking a manually maintained list -- which a new PR-producing +skill or script can silently join without ever being added -- this test +*discovers* every file under `.agents/skills/` whose text contains a direct +PR-creation signal (a literal `gh pr create` invocation, a `subprocess` +argv building one, a `--create-pr` flag, or a `def create_pr(` definition) +and asserts each discovered file references the shared contract. A new +PR-producing entry point that skips the contract fails this test by being +discovered, not by being missing from a list someone forgot to update. + +Run: + python3 .agents/skills/doc_quality_policy/test_manifest.py +""" +from __future__ import annotations + +import re +import unittest +from pathlib import Path +from typing import List + +_SKILLS_ROOT = Path(__file__).resolve().parents[1] + +# Any of these appearing in a file's text is a direct PR-creation code path -- +# not merely a mention of, or delegation to, another skill that creates PRs. +_PR_CREATION_SIGNALS = ( + re.compile(r"gh[\"']?\s*,?\s*[\"']?pr[\"']?\s*,?\s*[\"']?create"), # `gh pr create` or ["gh","pr","create"] + re.compile(r"--create-pr\b"), + re.compile(r"^def create_pr\w*\(", re.MULTILINE), +) + +# Any of these appearing in the same file satisfies the shared-contract +# requirement. A file can reference the reference doc directly, the marker +# label, or the shared scripts under doc_quality_policy/. +_CONTRACT_REFERENCE_MARKERS = ( + "doc-quality-policy.md", + "doc_quality_policy", + "warpy-factory", +) + +# Files that are part of the contract's own implementation/tests, or that are +# not documentation-content skills, so a PR-creation-shaped string in them +# (e.g. quoting `gh pr create` in a docstring example, or the manifest's own +# regex source) is not a bypass. Recorded here, one line each, so removing an +# entry is a deliberate, reviewed decision rather than a silent gap. +_EXEMPT_RELATIVE_PATHS = { + # This file's own PR-creation regex source text matches its own pattern. + "doc_quality_policy/test_manifest.py", + # Documents when to run the checker ("before `gh pr create`"); the + # checker validates PR body text and never calls `gh pr create` itself. + "create_pr/check_pr_body.py", + # Documents the downstream validate_ui_refs `--create-pr` step it runs + # before; this skill only compares screenshots and never opens a PR. + "verify-settings-subsections/SKILL.md", +} + +_SCAN_SUFFIXES = (".md", ".py") + + +def discover_pr_producing_files(skills_root: Path) -> List[Path]: + """Return every file under `skills_root` with a direct PR-creation signal.""" + found = [] + for path in sorted(skills_root.rglob("*")): + if not path.is_file() or path.suffix not in _SCAN_SUFFIXES: + continue + rel = path.relative_to(skills_root).as_posix() + if rel in _EXEMPT_RELATIVE_PATHS: + continue + try: + text = path.read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError): + continue + if any(pattern.search(text) for pattern in _PR_CREATION_SIGNALS): + found.append(path) + return found + + +def references_shared_contract(text: str) -> bool: + return any(marker in text for marker in _CONTRACT_REFERENCE_MARKERS) + + +class TestDiscoveredPrProducingFilesReferenceTheSharedContract(unittest.TestCase): + def test_discovery_finds_the_known_pr_producing_entry_points(self): + """Sanity check on the discovery mechanism itself: if this drops to + zero, the regexes have drifted and the real assertion below would + pass vacuously. + """ + discovered = discover_pr_producing_files(_SKILLS_ROOT) + self.assertGreater(len(discovered), 5, f"discovered only {discovered!r}") + + def test_every_discovered_pr_producing_file_references_the_shared_contract(self): + missing = [] + for path in discover_pr_producing_files(_SKILLS_ROOT): + text = path.read_text(encoding="utf-8") + if not references_shared_contract(text): + missing.append(str(path.relative_to(_SKILLS_ROOT))) + self.assertEqual( + missing, [], + "these files directly create PRs but do not reference the shared " + f"doc-quality contract ({_CONTRACT_REFERENCE_MARKERS}): {missing}", + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/.agents/skills/doc_quality_policy/test_policy.py b/.agents/skills/doc_quality_policy/test_policy.py new file mode 100644 index 000000000..bb49118f6 --- /dev/null +++ b/.agents/skills/doc_quality_policy/test_policy.py @@ -0,0 +1,495 @@ +#!/usr/bin/env python3 +"""Unit tests for policy.py. + +Stdlib unittest only, no third-party deps and no network. + +Run: + python3 .agents/skills/doc_quality_policy/test_policy.py +""" +from __future__ import annotations + +import importlib.util +import sys +import unittest +from pathlib import Path + +_HERE = Path(__file__).resolve().parent +_MODULE_PATH = _HERE / "policy.py" + +_spec = importlib.util.spec_from_file_location("doc_quality_policy", _MODULE_PATH) +policy = importlib.util.module_from_spec(_spec) +# Registering in sys.modules before exec_module lets dataclasses (used with +# `from __future__ import annotations`) resolve field type annotations via +# `sys.modules[cls.__module__]`, which fails on an unregistered dynamic module. +sys.modules[_spec.name] = policy +_spec.loader.exec_module(policy) + + +class TestVerifyMarkers(unittest.TestCase): + def test_finds_single_marker(self): + text = "Run `warp --auto-approve` {/* VERIFY: flag name, unconfirmed against warp-internal */} to continue." + self.assertEqual( + policy.find_verify_markers(text), + ["flag name, unconfirmed against warp-internal"], + ) + + def test_finds_multiple_markers_in_order(self): + text = "{/* VERIFY: claim one */} text {/* VERIFY: claim two */}" + self.assertEqual(policy.find_verify_markers(text), ["claim one", "claim two"]) + + def test_no_markers_returns_empty(self): + self.assertEqual(policy.find_verify_markers("Nothing to see here."), []) + + +class TestSectionExtraction(unittest.TestCase): + def test_extracts_section_up_to_next_heading(self): + body = "## Documentation risk\nRisk: low\nRationale: typo fix\n\n## Unverified claims\nNone\n" + section = policy.extract_section(body, "## Documentation risk") + self.assertEqual(section, "Risk: low\nRationale: typo fix") + + def test_missing_section_returns_none(self): + self.assertIsNone(policy.extract_section("## Summary\nhello", "## Documentation risk")) + + def test_section_runs_to_end_of_body(self): + body = "## Unverified claims\n- one\n- two" + self.assertEqual(policy.extract_section(body, "## Unverified claims"), "- one\n- two") + + +class TestParseDocumentationRisk(unittest.TestCase): + def test_parses_low_risk_block(self): + body = ( + "## Documentation risk\n" + "Risk: low\n" + "Rationale: Spelling fixes only.\n" + "Docs override: none\n" + ) + parsed = policy.parse_documentation_risk_section(body) + self.assertEqual(parsed.risk, policy.RISK_LOW) + self.assertEqual(parsed.rationale, "Spelling fixes only.") + self.assertEqual(parsed.docs_override, policy.OVERRIDE_MODE_NONE) + + def test_parses_list_fields(self): + body = ( + "## Documentation risk\n" + "Risk: engineering-review-required\n" + "Rationale: New CLI flag.\n" + "Source files consulted: app/src/cli/args.rs@abc, warp-server/pkg/foo.go@def\n" + "Requested engineering reviewers: alice, bob\n" + ) + parsed = policy.parse_documentation_risk_section(body) + self.assertEqual(parsed.source_files_consulted, ["app/src/cli/args.rs@abc", "warp-server/pkg/foo.go@def"]) + self.assertEqual(parsed.requested_engineering_reviewers, ["alice", "bob"]) + + def test_missing_section_returns_none(self): + self.assertIsNone(policy.parse_documentation_risk_section("## Summary\nhello")) + + def test_parses_override_fields(self): + body = ( + "## Documentation risk\n" + "Risk: engineering-review-required\n" + "Rationale: New CLI flag.\n" + "Docs override: docs-verified\n" + "Override reviewer: hongyi-chen\n" + "Override reason: Confirmed against source.\n" + "Override evidence: app/src/cli/args.rs@abc\n" + "Override head SHA: deadbeef\n" + ) + parsed = policy.parse_documentation_risk_section(body) + self.assertEqual(parsed.docs_override, "docs-verified") + self.assertEqual(parsed.override_reviewer, "hongyi-chen") + self.assertEqual(parsed.override_head_sha, "deadbeef") + + +class TestParseUnverifiedClaims(unittest.TestCase): + def test_none_sentinel_returns_empty_list(self): + body = "## Unverified claims\nNone — all claims verified against source.\n" + self.assertEqual(policy.parse_unverified_claims_section(body), []) + + def test_bullets_are_collected(self): + body = "## Unverified claims\n- claim one\n- claim two\n" + self.assertEqual(policy.parse_unverified_claims_section(body), ["claim one", "claim two"]) + + def test_missing_section_returns_none(self): + self.assertIsNone(policy.parse_unverified_claims_section("## Summary\nhello")) + + +# --------------------------------------------------------------------------- +# Risk allowlist — table-driven +# --------------------------------------------------------------------------- + +class TestClassifyRisk(unittest.TestCase): + def test_all_clear_is_low_risk(self): + signals = policy.RiskSignals.all_clear() + self.assertEqual(policy.classify_risk(signals), policy.RISK_LOW) + def test_diff_with_no_allowlisted_low_risk_category_requires_engineering_review(self): + signals = policy.RiskSignals.all_clear(is_editorial_or_metadata_only=False) + self.assertEqual(policy.classify_risk(signals), policy.RISK_ENGINEERING_REVIEW_REQUIRED) + + def test_docs_workflow_tooling_with_no_product_claims_is_low_risk(self): + signals = policy.RiskSignals.all_clear( + is_editorial_or_metadata_only=False, + is_docs_workflow_tooling_only=True, + ) + self.assertEqual(policy.classify_risk(signals), policy.RISK_LOW) + + def test_every_allowlist_trigger_forces_engineering_review(self): + trigger_fields = [ + "adds_new_or_changed_feature_page", + "changes_commands_or_code_examples", + "changes_api_behavior", + "changes_ui_labels_or_paths", + "changes_defaults", + "changes_permissions", + "changes_availability_or_platform_support", + "changes_plan_eligibility", + "changes_billing_behavior", + "changes_security_or_privacy_claims", + "changes_data_handling", + "changes_self_hosting_behavior", + "changes_integration_setup", + ] + for field_name in trigger_fields: + with self.subTest(trigger=field_name): + signals = policy.RiskSignals.all_clear(**{field_name: True}) + self.assertEqual( + policy.classify_risk(signals), + policy.RISK_ENGINEERING_REVIEW_REQUIRED, + f"trigger {field_name} should force engineering-review-required", + ) + + def test_unresolved_verify_marker_forces_engineering_review(self): + signals = policy.RiskSignals.all_clear(has_unresolved_verify_marker=True) + self.assertEqual(policy.classify_risk(signals), policy.RISK_ENGINEERING_REVIEW_REQUIRED) + + def test_critical_or_important_finding_forces_engineering_review(self): + signals = policy.RiskSignals.all_clear(has_critical_or_important_review_finding=True) + self.assertEqual(policy.classify_risk(signals), policy.RISK_ENGINEERING_REVIEW_REQUIRED) + + def test_ambiguous_default_is_engineering_review_required(self): + # An unconfigured RiskSignals() defaults every trigger to True, encoding + # "ambiguous risk classification defaults to engineering-review-required". + self.assertEqual(policy.classify_risk(policy.RiskSignals()), policy.RISK_ENGINEERING_REVIEW_REQUIRED) + + +# --------------------------------------------------------------------------- +# VERIFY accounting +# --------------------------------------------------------------------------- + +class TestVerifyAccounting(unittest.TestCase): + LOW_RISK_BODY = ( + "## Documentation risk\nRisk: low\nRationale: typo fix.\nDocs override: none\n\n" + "## Unverified claims\nNone — all claims verified against source.\n" + ) + ENG_REVIEW_BODY_TEMPLATE = ( + "## Documentation risk\nRisk: engineering-review-required\nRationale: new flag.\n" + "Requested engineering reviewers: alice\nEngineering review status: pending\n" + "Docs override: none\n\n## Unverified claims\n{claims}\n" + ) + + def test_unlisted_verify_marker_fails(self): + problems = policy.validate_pr_contract(self.LOW_RISK_BODY, ["flag name unconfirmed"]) + self.assertTrue(any("not listed" in p for p in problems)) + + def test_listed_marker_with_low_risk_fails(self): + body = self.ENG_REVIEW_BODY_TEMPLATE.format(claims="- flag name unconfirmed") + body = body.replace("Risk: engineering-review-required", "Risk: low") + problems = policy.validate_pr_contract(body, ["flag name unconfirmed"]) + self.assertTrue(any("must be" in p for p in problems)) + + def test_listed_marker_with_engineering_review_and_approval_passes(self): + body = self.ENG_REVIEW_BODY_TEMPLATE.format(claims="- flag name unconfirmed") + problems = policy.validate_pr_contract( + body, ["flag name unconfirmed"], source_owner_approved_current_head=True, + ) + self.assertEqual(problems, []) + + +# --------------------------------------------------------------------------- +# Override boundaries +# --------------------------------------------------------------------------- + +class TestEngineeringGate(unittest.TestCase): + def _risk(self, **kwargs): + base = dict(risk=policy.RISK_ENGINEERING_REVIEW_REQUIRED, docs_override=policy.OVERRIDE_MODE_NONE) + base.update(kwargs) + return policy.DocumentationRisk(**base) + + def test_source_owner_approval_satisfies_gate(self): + problems = policy.validate_engineering_gate( + self._risk(), + current_head_sha="sha1", + authorized_docs_reviewers=(), + deterministic_checks_passed=True, + has_unresolved_critical_or_important_finding=False, + source_owner_approved_current_head=True, + ) + self.assertEqual(problems, []) + + def test_no_approval_and_no_override_fails(self): + problems = policy.validate_engineering_gate( + self._risk(), + current_head_sha="sha1", + authorized_docs_reviewers=(), + deterministic_checks_passed=True, + has_unresolved_critical_or_important_finding=False, + source_owner_approved_current_head=False, + ) + self.assertTrue(problems) + + def test_docs_verified_override_with_all_fields_passes(self): + risk = self._risk( + docs_override="docs-verified", + override_reviewer="hongyi-chen", + override_reason="Confirmed against source.", + override_evidence="app/src/cli/args.rs@abc", + override_head_sha="sha1", + ) + problems = policy.validate_engineering_gate( + risk, + current_head_sha="sha1", + authorized_docs_reviewers=["hongyi-chen"], + deterministic_checks_passed=True, + has_unresolved_critical_or_important_finding=False, + source_owner_approved_current_head=False, + approved_reviewers_current_head=["hongyi-chen"], + ) + self.assertEqual(problems, []) + + def test_docs_waiver_override_with_all_fields_passes(self): + risk = self._risk( + docs_override="docs-waiver", + override_reviewer="rachaelrenk", + override_reason="No owner responded; risk is limited to wording.", + override_evidence="n/a", + override_head_sha="sha1", + ) + problems = policy.validate_engineering_gate( + risk, + current_head_sha="sha1", + authorized_docs_reviewers=["rachaelrenk"], + deterministic_checks_passed=True, + has_unresolved_critical_or_important_finding=False, + source_owner_approved_current_head=False, + approved_reviewers_current_head=["rachaelrenk"], + ) + self.assertEqual(problems, []) + + def test_override_requires_the_named_reviewer_to_approve_the_current_head(self): + risk = self._risk( + docs_override="docs-verified", + override_reviewer="hongyi-chen", + override_reason="Confirmed.", + override_evidence="app/src/cli/args.rs@abc", + override_head_sha="sha1", + ) + problems = policy.validate_engineering_gate( + risk, + current_head_sha="sha1", + authorized_docs_reviewers=["hongyi-chen"], + deterministic_checks_passed=True, + has_unresolved_critical_or_important_finding=False, + source_owner_approved_current_head=False, + ) + self.assertTrue(any("has not approved" in p for p in problems)) + def test_override_missing_field_fails(self): + risk = self._risk( + docs_override="docs-verified", + override_reviewer="hongyi-chen", + override_reason="Confirmed.", + # override_evidence and override_head_sha missing + ) + problems = policy.validate_engineering_gate( + risk, + current_head_sha="sha1", + authorized_docs_reviewers=["hongyi-chen"], + deterministic_checks_passed=True, + has_unresolved_critical_or_important_finding=False, + source_owner_approved_current_head=False, + ) + self.assertTrue(any("missing required field" in p for p in problems)) + + def test_override_by_unauthorized_reviewer_fails(self): + risk = self._risk( + docs_override="docs-verified", + override_reviewer="random-person", + override_reason="Confirmed.", + override_evidence="app/src/cli/args.rs@abc", + override_head_sha="sha1", + ) + problems = policy.validate_engineering_gate( + risk, + current_head_sha="sha1", + authorized_docs_reviewers=["hongyi-chen"], + deterministic_checks_passed=True, + has_unresolved_critical_or_important_finding=False, + source_owner_approved_current_head=False, + ) + self.assertTrue(any("not an authorized" in p for p in problems)) + + def test_stale_override_head_sha_fails(self): + risk = self._risk( + docs_override="docs-verified", + override_reviewer="hongyi-chen", + override_reason="Confirmed.", + override_evidence="app/src/cli/args.rs@abc", + override_head_sha="old-sha", + ) + problems = policy.validate_engineering_gate( + risk, + current_head_sha="new-sha", + authorized_docs_reviewers=["hongyi-chen"], + deterministic_checks_passed=True, + has_unresolved_critical_or_important_finding=False, + source_owner_approved_current_head=False, + ) + self.assertTrue(any("does not match" in p for p in problems)) + + def test_override_cannot_bypass_failing_deterministic_check(self): + risk = self._risk( + docs_override="docs-verified", + override_reviewer="hongyi-chen", + override_reason="Confirmed.", + override_evidence="app/src/cli/args.rs@abc", + override_head_sha="sha1", + ) + problems = policy.validate_engineering_gate( + risk, + current_head_sha="sha1", + authorized_docs_reviewers=["hongyi-chen"], + deterministic_checks_passed=False, + has_unresolved_critical_or_important_finding=False, + source_owner_approved_current_head=False, + ) + self.assertTrue(any("deterministic check" in p or "no override can bypass" in p for p in problems)) + + def test_override_cannot_bypass_unresolved_finding(self): + risk = self._risk( + docs_override="docs-waiver", + override_reviewer="hongyi-chen", + override_reason="Confirmed.", + override_evidence="n/a", + override_head_sha="sha1", + ) + problems = policy.validate_engineering_gate( + risk, + current_head_sha="sha1", + authorized_docs_reviewers=["hongyi-chen"], + deterministic_checks_passed=True, + has_unresolved_critical_or_important_finding=True, + source_owner_approved_current_head=False, + ) + self.assertTrue(any("critical/important" in p for p in problems)) + + def test_source_owner_approval_does_not_bypass_failing_check(self): + problems = policy.validate_engineering_gate( + self._risk(), + current_head_sha="sha1", + authorized_docs_reviewers=(), + deterministic_checks_passed=False, + has_unresolved_critical_or_important_finding=False, + source_owner_approved_current_head=True, + ) + self.assertTrue(problems) + + def test_new_head_invalidates_engineer_approval_semantics(self): + # source_owner_approved_current_head is caller-supplied and must be + # keyed to the *current* head; simulate a stale approval by passing + # False (the caller's job is to only pass True for the current head). + problems = policy.validate_engineering_gate( + self._risk(), + current_head_sha="new-sha", + authorized_docs_reviewers=(), + deterministic_checks_passed=True, + has_unresolved_critical_or_important_finding=False, + source_owner_approved_current_head=False, + ) + self.assertTrue(problems) + + def test_empty_authorized_list_fails_closed_instead_of_skipping_the_check(self): + # A PR-controlled or unavailable allowlist lookup must never be read + # as "anyone may author an override" -- regression for a bypass where + # an empty list silently skipped the authorization check. + risk = self._risk( + docs_override="docs-verified", + override_reviewer="random-person", + override_reason="Confirmed.", + override_evidence="app/src/cli/args.rs@abc", + override_head_sha="sha1", + ) + problems = policy.validate_engineering_gate( + risk, + current_head_sha="sha1", + authorized_docs_reviewers=(), + deterministic_checks_passed=True, + has_unresolved_critical_or_important_finding=False, + source_owner_approved_current_head=False, + ) + self.assertTrue(any("not an authorized" in p for p in problems)) + + def test_override_with_no_current_head_sha_fails_closed(self): + # Regression for a bypass where omitting current_head_sha entirely + # skipped the staleness check rather than failing it. + risk = self._risk( + docs_override="docs-verified", + override_reviewer="hongyi-chen", + override_reason="Confirmed.", + override_evidence="app/src/cli/args.rs@abc", + override_head_sha="sha1", + ) + problems = policy.validate_engineering_gate( + risk, + current_head_sha=None, + authorized_docs_reviewers=["hongyi-chen"], + deterministic_checks_passed=True, + has_unresolved_critical_or_important_finding=False, + source_owner_approved_current_head=False, + ) + self.assertTrue(any("no current head SHA" in p for p in problems)) + + +class TestFullContractValidation(unittest.TestCase): + def test_missing_documentation_risk_section(self): + problems = policy.validate_pr_contract("## Summary\nhello", []) + self.assertEqual(len(problems), 1) + self.assertIn("Documentation risk", problems[0]) + + def test_low_risk_clean_pr_passes(self): + body = ( + "## Documentation risk\nRisk: low\nRationale: typo fix.\nDocs override: none\n\n" + "## Unverified claims\nNone — all claims verified against source.\n" + ) + self.assertEqual(policy.validate_pr_contract(body, []), []) + + def test_invalid_risk_level_reported(self): + body = ( + "## Documentation risk\nRisk: medium\nRationale: x.\nDocs override: none\n\n" + "## Unverified claims\nNone\n" + ) + problems = policy.validate_pr_contract(body, []) + self.assertTrue(any("invalid risk level" in p for p in problems)) + + def test_pending_engineering_review_is_valid_for_structural_ci(self): + body = ( + "## Documentation risk\nRisk: engineering-review-required\n" + "Rationale: documents a new CLI flag.\nDocs override: none\n\n" + "## Unverified claims\nNone\n" + ) + self.assertEqual(policy.validate_pr_contract(body, []), []) + + def test_explicit_human_gate_rejects_pending_engineering_review(self): + body = ( + "## Documentation risk\nRisk: engineering-review-required\n" + "Rationale: documents a new CLI flag.\nDocs override: none\n\n" + "## Unverified claims\nNone\n" + ) + problems = policy.validate_pr_contract( + body, + [], + current_head_sha="sha1", + enforce_engineering_gate=True, + ) + self.assertTrue(any("no source-owner approval" in p for p in problems)) + + +if __name__ == "__main__": + unittest.main() diff --git a/.agents/skills/doc_quality_policy/test_verify_review_signal.py b/.agents/skills/doc_quality_policy/test_verify_review_signal.py new file mode 100644 index 000000000..fef422edd --- /dev/null +++ b/.agents/skills/doc_quality_policy/test_verify_review_signal.py @@ -0,0 +1,83 @@ +#!/usr/bin/env python3 +"""Unit tests for verify_review_signal.py.""" +from __future__ import annotations + +import importlib.util +import sys +import unittest +from pathlib import Path +from unittest import mock + +_HERE = Path(__file__).resolve().parent +_spec = importlib.util.spec_from_file_location("verify_review_signal", _HERE / "verify_review_signal.py") +vrs = importlib.util.module_from_spec(_spec) +sys.modules[_spec.name] = vrs +_spec.loader.exec_module(vrs) + +GOOD_OUTPUT = ( + '[SIGNAL:pr-review] {"pr":"1","head_sha":"sha1","reviewer_login":"agent-bot",' + '"verdict":"Approve","critical":0,"important":0}' +) +GOOD_REVIEW = { + "user": {"login": "agent-bot"}, + "state": "APPROVED", + "commit_id": "sha1", + "body": GOOD_OUTPUT, +} + + +class TestCheckReviewSignal(unittest.TestCase): + def test_missing_published_review_fails(self): + with mock.patch.object(vrs.cpc, "_fetch_reviews", return_value=[]): + problems = vrs.check_review_signal("o/r", "1", "sha1", GOOD_OUTPUT) + self.assertTrue(any("no current GitHub review" in p for p in problems)) + + def test_missing_signal_fails(self): + with mock.patch.object(vrs.cpc, "_fetch_reviews", return_value=[GOOD_REVIEW]): + problems = vrs.check_review_signal("o/r", "1", "sha1", "review complete") + self.assertTrue(any("expected exactly one" in p for p in problems)) + + def test_signal_without_reviewer_identity_fails(self): + output = GOOD_OUTPUT.replace(',"reviewer_login":"agent-bot"', "") + with mock.patch.object(vrs.cpc, "_fetch_reviews", return_value=[GOOD_REVIEW]): + problems = vrs.check_review_signal("o/r", "1", "sha1", output) + self.assertTrue(any("missing reviewer_login" in p for p in problems)) + + def test_human_approval_does_not_satisfy_agent_review_requirement(self): + human_review = {**GOOD_REVIEW, "user": {"login": "human-reviewer"}} + with mock.patch.object(vrs.cpc, "_fetch_reviews", return_value=[human_review]): + problems = vrs.check_review_signal("o/r", "1", "sha1", GOOD_OUTPUT) + self.assertTrue(any("no current GitHub review" in p for p in problems)) + + def test_review_without_matching_published_signal_fails(self): + review = {**GOOD_REVIEW, "body": "Approve"} + with mock.patch.object(vrs.cpc, "_fetch_reviews", return_value=[review]): + problems = vrs.check_review_signal("o/r", "1", "sha1", GOOD_OUTPUT) + self.assertTrue(any("no current GitHub review" in p for p in problems)) + + def test_stale_signal_fails(self): + stale_output = GOOD_OUTPUT.replace('"sha1"', '"old-sha"') + with mock.patch.object(vrs.cpc, "_fetch_reviews", return_value=[GOOD_REVIEW]): + problems = vrs.check_review_signal("o/r", "1", "sha1", stale_output) + self.assertTrue(any("does not match current head" in p for p in problems)) + + def test_blocking_signal_fails(self): + blocking_output = GOOD_OUTPUT.replace('"Approve"', '"Request changes"').replace('"critical":0', '"critical":1') + with mock.patch.object(vrs.cpc, "_fetch_reviews", return_value=[GOOD_REVIEW]): + problems = vrs.check_review_signal("o/r", "1", "sha1", blocking_output) + self.assertTrue(any("blocking verdict" in p for p in problems)) + self.assertTrue(any("critical finding" in p for p in problems)) + + def test_matching_published_review_and_passing_signal_pass(self): + with mock.patch.object(vrs.cpc, "_fetch_reviews", return_value=[GOOD_REVIEW]): + problems = vrs.check_review_signal("o/r", "1", "sha1", GOOD_OUTPUT) + self.assertEqual(problems, []) + + def test_fetch_failure_fails_closed(self): + with mock.patch.object(vrs.cpc, "_fetch_reviews", side_effect=RuntimeError("boom")): + problems = vrs.check_review_signal("o/r", "1", "sha1", GOOD_OUTPUT) + self.assertTrue(any("could not fetch reviews" in p for p in problems)) + + +if __name__ == "__main__": + unittest.main() diff --git a/.agents/skills/doc_quality_policy/verify_review_signal.py b/.agents/skills/doc_quality_policy/verify_review_signal.py new file mode 100644 index 000000000..c0aa1cffe --- /dev/null +++ b/.agents/skills/doc_quality_policy/verify_review_signal.py @@ -0,0 +1,124 @@ +#!/usr/bin/env python3 +"""Verify a current independent-agent review and its structured signal.""" +from __future__ import annotations + +import argparse +import importlib.util +import json +import re +import sys +from pathlib import Path +from typing import Dict, List, Optional, Tuple + +_HERE = Path(__file__).resolve().parent +_spec = importlib.util.spec_from_file_location("check_pr_contract", _HERE / "check_pr_contract.py") +cpc = importlib.util.module_from_spec(_spec) +sys.modules[_spec.name] = cpc +_spec.loader.exec_module(cpc) + +_SIGNAL_RE = re.compile(r"\[SIGNAL:pr-review\]\s*(\{.*?\})", re.DOTALL) +_PASSING_VERDICTS = {"approve", "approve with nits", "approve_with_nits"} + + +def _parse_signal(text: str) -> Tuple[Optional[Dict[str, object]], List[str]]: + matches = _SIGNAL_RE.findall(text) + if len(matches) != 1: + return None, [f"expected exactly one [SIGNAL:pr-review] record, found {len(matches)}"] + try: + signal = json.loads(matches[0]) + except json.JSONDecodeError as exc: + return None, [f"review signal is not valid JSON: {exc}"] + if not isinstance(signal, dict): + return None, ["review signal must contain a JSON object"] + return signal, [] + + +def _validate_signal(signal: Dict[str, object], pr_number: str, head_sha: str) -> List[str]: + problems = [] + if str(signal.get("pr")) != str(pr_number): + problems.append(f"review signal PR {signal.get('pr')!r} does not match {pr_number!r}") + if signal.get("head_sha") != head_sha: + problems.append( + f"review signal head SHA {signal.get('head_sha')!r} does not match current head {head_sha!r}" + ) + if str(signal.get("verdict", "")).strip().lower() not in _PASSING_VERDICTS: + problems.append(f"review signal has blocking verdict {signal.get('verdict')!r}") + if not signal.get("reviewer_login"): + problems.append("review signal is missing reviewer_login") + for field in ("critical", "important"): + try: + value = int(signal.get(field)) + except (TypeError, ValueError): + problems.append(f"review signal has invalid {field} count {signal.get(field)!r}") + continue + if value != 0: + problems.append(f"review signal reports {value} {field} finding(s)") + return problems + + +def _published_review_matches_signal( + reviews: List[dict], + signal: Dict[str, object], + head_sha: str, +) -> bool: + reviewer_login = signal.get("reviewer_login") + for review in reviews: + if review.get("commit_id") != head_sha: + continue + if (review.get("user") or {}).get("login") != reviewer_login: + continue + published_signal, problems = _parse_signal(review.get("body") or "") + if problems or published_signal is None: + continue + fields = ("pr", "head_sha", "verdict", "critical", "important", "reviewer_login") + if all(published_signal.get(field) == signal.get(field) for field in fields): + return True + return False + + +def check_review_signal(repo: str, pr_number: str, head_sha: str, agent_output: str) -> List[str]: + """Return problems; empty means the independent agent published a passing review.""" + signal, problems = _parse_signal(agent_output) + if signal is None: + return problems + problems.extend(_validate_signal(signal, pr_number, head_sha)) + if problems: + return problems + try: + reviews = cpc._fetch_reviews(repo, pr_number) + except RuntimeError as exc: + return [f"could not fetch reviews: {exc}"] + if not _published_review_matches_signal(reviews, signal, head_sha): + return [ + f"no current GitHub review by {signal['reviewer_login']!r} contains the matching " + "[SIGNAL:pr-review] record" + ] + return [] + + +def main(argv: Optional[List[str]] = None) -> int: + parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + parser.add_argument("--repo", required=True, help="owner/repo") + parser.add_argument("--pr", required=True, help="PR number") + parser.add_argument("--head-sha", required=True, help="PR head SHA to verify a review against") + parser.add_argument("--agent-output", required=True, help="file containing the agent's final text output") + args = parser.parse_args(argv) + try: + agent_output = Path(args.agent_output).read_text(encoding="utf-8") + except OSError as exc: + print(f"error: could not read agent output: {exc}", file=sys.stderr) + return 2 + + problems = check_review_signal(args.repo, args.pr, args.head_sha, agent_output) + if problems: + print("Agent docs review verification FAILED:", file=sys.stderr) + for problem in problems: + print(f" - {problem}", file=sys.stderr) + return 1 + + print(f"Confirmed a passing independent review for head {args.head_sha}.") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.agents/skills/docs-seo-audit/SKILL.md b/.agents/skills/docs-seo-audit/SKILL.md index ecfc985a7..4b5143fb4 100644 --- a/.agents/skills/docs-seo-audit/SKILL.md +++ b/.agents/skills/docs-seo-audit/SKILL.md @@ -12,6 +12,15 @@ description: >- Crawl the live docs.warp.dev sitemap to find SEO issues and fix them in the source files. +## Agent-doc quality contract + +Any PR this skill opens follows the shared v1 agent-doc quality contract in +`.agents/references/doc-quality-policy.md`: apply the `warpy-factory` label +and add the `## Documentation risk` block +(`.agents/skills/doc_quality_policy/finalize_pr_contract.py build`). Title, +description, and metadata fixes that preserve product meaning are typically +`low` risk under the allowlist. + ## Running the audit From the docs repo root: diff --git a/.agents/skills/draft_docs/SKILL.md b/.agents/skills/draft_docs/SKILL.md index bf88cfa67..50f891a0c 100644 --- a/.agents/skills/draft_docs/SKILL.md +++ b/.agents/skills/draft_docs/SKILL.md @@ -230,6 +230,16 @@ Inline `{/* VERIFY: ... */}` markers alone are skippable: a reviewer who skims t A reviewer must be able to see every unconfirmed claim without opening the diff. +### 9.6. Classify documentation risk and apply the marker + +Follow the shared v1 agent-doc quality contract in +`.agents/references/doc-quality-policy.md`. Walk the low-risk allowlist there +— a newly-drafted or materially-changed feature page is +`engineering-review-required` by default, and so is any unresolved `VERIFY` +marker from step 6/9.5. Build the `## Documentation risk` PR-body section and +apply the `warpy-factory` label via `create_pr`'s "Documentation risk" +subsection before requesting review. + ### 10. Update navigation and redirects If this is a new page, remind the user to: - Add it to the relevant section in `src/sidebar.ts`. diff --git a/.agents/skills/improve-drafting-skills/SKILL.md b/.agents/skills/improve-drafting-skills/SKILL.md index 0d26a824a..90b2893da 100644 --- a/.agents/skills/improve-drafting-skills/SKILL.md +++ b/.agents/skills/improve-drafting-skills/SKILL.md @@ -9,6 +9,14 @@ Monthly outer loop agent. Reads three signal logs accumulated from agent-authore This skill is part of the self-improvement loop architecture. See the architecture plan for context on the inner loops that populate the signal logs. +The standing improvement PR this skill maintains follows the shared v1 +agent-doc quality contract in `.agents/references/doc-quality-policy.md` +(`warpy-factory` label + `## Documentation risk` block via +`.agents/skills/doc_quality_policy/finalize_pr_contract.py build`). A skill/ +template-prose-only edit is `engineering-review-required` by default +(judgment call — it changes agent behavior, not just wording) unless the +change is provably a wording-only fix with no behavior change. + ## Schedule Monthly, on the 1st of each month. @@ -108,8 +116,12 @@ For each agent-authored PR merged in the past 30 days (identified by `oz-agent@w - **Redact** any comment text that appears to contain secrets (tokens, API keys, passwords) — replace the value with `[REDACTED]` before storing. For accepted records, build the structured entry: ```json - {"date":"YYYY-MM-DD","pr":"NNN","skill_used":"draft_feature_doc","file":"src/content/docs/path.mdx","feedback_type":"review_comment","severity":"important","comment":"Comment text here","tag":"[skill-feedback]","pattern_category":"header_case","resolved_by":"human_edit"} + {"date":"YYYY-MM-DD","pr":"NNN","skill_used":"draft_feature_doc","file":"src/content/docs/path.mdx","feedback_type":"review_comment","severity":"important","comment":"Comment text here","tag":"[skill-feedback]","pattern_category":"header_case","resolved_by":"human_edit","risk":"low","head_sha":"abc1234","check_outcome":"pass","review_outcome":"approve_with_nits"} ``` + - Set `risk` to the PR's declared `## Documentation risk` level (`low` or `engineering-review-required`; see `doc_quality_policy/SKILL.md`), read from the PR body at collection time. Use `"unknown"` when the PR predates the v1 contract or the section can't be parsed. + - Set `head_sha` to the PR's head commit SHA at the time of collection (`gh pr view NNN --json headRefOid --jq .headRefOid`). + - Set `check_outcome` to `"pass"` or `"fail"` from the `Docs editorial quality` / `Docs technical references` required checks on that head (`gh pr checks NNN`), or `"unknown"` if unavailable. + - Set `review_outcome` to the `review-docs-pr` verdict for that head (`approve`, `approve_with_nits`, or `request_changes`, parsed from the `[SIGNAL:pr-review]` record in Step A), or `"unknown"` if no signal was found for this PR. - Set `tag` to the prefix found in the comment (`[skill-feedback]`, `[template-feedback]`, `[style-rule-gap]`) or `""` if none. - Set `feedback_type` to `"review_comment"`, `"human_edit"`, or `"review_verdict"`. - **Set `pattern_category`** to a short, structured, collector-derived label for the type of issue — not a copy of the free-text comment. Derive it from: @@ -298,6 +310,51 @@ Both satisfy the "durable record of its outcome" requirement in `.agents/referen Its other durable outputs are the standing improvement PR and, when warranted, the Slack message. +## v1 baseline and outcome metrics (GROW-6092) + +`scripts/compute_metrics.py` computes the deterministic v1 report over an +explicit date window from a JSONL file of in-scope PR records (schema +documented in the script's docstring): + +```bash +python3 .agents/skills/improve-drafting-skills/scripts/compute_metrics.py \ + --input /path/to/records.jsonl --start 2026-01-01 --end 2026-01-30 \ + --output /tmp/baseline-report.json +``` + +Running it twice over the same frozen input and date window yields +byte-equivalent normalized JSON (see `test_compute_metrics.py`). Persist the +pre-rollout **baseline** (the 30 days immediately preceding rollout, captured +once and never recomputed) and the post-rollout **comparison** report through +the standing signal-log branch/PR flow above, alongside +`.agents/logs/agent_doc_quality_baseline.md`, which records the exact window +dates and report file used for each. + +`scripts/build_baseline_records.py` converts the existing per-comment +`.agents/logs/human_review_feedback.jsonl` signal log into the per-PR record +shape `compute_metrics.py` expects, resolving each PR's agent-vs-human line +split from live `gh pr view --json additions,deletions` data: + +```bash +python3 .agents/skills/improve-drafting-skills/scripts/build_baseline_records.py \ + --repo warpdotdev/docs --input .agents/logs/human_review_feedback.jsonl \ + --start 2026-08-01 --end 2026-08-30 --output /tmp/baseline-records.jsonl +``` + +The pre-rollout baseline (2026-08-01 to 2026-08-30, 62 PRs) is already +captured this way in `.agents/logs/agent_doc_quality_baseline.md` and +`.agents/logs/baseline/`. Compute the post-rollout comparison the same way +once enough post-rollout data exists, passing `--baseline +.agents/logs/baseline/pre-rollout-2026-08-01-to-2026-08-30-report.json` to +get the `day_30_outcome` verdict. + +Pass `--baseline baseline-report.json` when computing the post-rollout report +to also emit `day_30_outcome` (`pass`, `fail`, or `inconclusive-small-sample` +per `evaluate_outcome()` — product behavior #17). A window with fewer than 10 +in-scope PRs is `inconclusive-small-sample`; extend collection to 10 PRs or 60 +days, whichever comes first, rather than claiming success or failure on too +small a sample. + ## Deployment This skill is designed for a monthly Oz scheduled agent. diff --git a/.agents/skills/improve-drafting-skills/scripts/build_baseline_records.py b/.agents/skills/improve-drafting-skills/scripts/build_baseline_records.py new file mode 100644 index 000000000..6ad52767d --- /dev/null +++ b/.agents/skills/improve-drafting-skills/scripts/build_baseline_records.py @@ -0,0 +1,187 @@ +#!/usr/bin/env python3 +"""Build compute_metrics.py-compatible per-PR records from the existing +per-comment `human_review_feedback.jsonl` signal log, for a pre-rollout +baseline window that predates the v1 contract (so risk/check_outcome/ +review_outcome/engineering_required are genuinely unknown -- the v1 checks +did not exist yet). + +`human_review_feedback.jsonl` already has real per-comment data (date, pr, +feedback_type, comment) going back to the "Improve AI-generated docs" outer +loop, but it is a comment-level log, not a per-PR record. This script: + +1. Groups comment-level rows by `pr` within the requested date window. +2. Counts `human_review_comments` per PR from `review_comment` / + `review_verdict` rows. +3. Sums the logged `Human edit after agent commit: +X -Y lines` deltas per PR + for `human_lines_changed_after_last_agent_commit`. +4. Resolves each PR's total changed lines live via `gh pr view --json + additions,deletions` and derives `agent_lines_changed` as + `total - human_lines_changed_after_last_agent_commit` (floored at 0), since + the total is real, already-merged line-change data and the human-edit + portion is already logged separately. +5. Emits one JSONL record per PR with the v1 fields the pre-rollout window + cannot know (`risk`, `check_outcome`, `review_outcome`, + `engineering_required`) explicitly set to their documented "unknown"/ + not-applicable values, per compute_metrics.py's degrade-to-missing rule. + +Usage: + python3 build_baseline_records.py --repo warpdotdev/docs \ + --input .agents/logs/human_review_feedback.jsonl \ + --start 2026-08-01 --end 2026-08-30 \ + --output /tmp/baseline-records.jsonl +""" +from __future__ import annotations + +import argparse +import json +import subprocess +import sys +from datetime import date +from pathlib import Path +from typing import Any, Dict, List, Optional + + +def load_comment_records(path: Path) -> List[Dict[str, Any]]: + records = [] + with open(path, "r", encoding="utf-8") as f: + for line in f: + line = line.strip() + if not line: + continue + records.append(json.loads(line)) + return records + + +def _in_window(record: Dict[str, Any], start: date, end: date) -> bool: + raw = record.get("date") + if not raw: + return False + try: + d = date.fromisoformat(raw) + except ValueError: + return False + return start <= d <= end + + +def _human_edit_lines(comment: str) -> int: + # "Human edit after agent commit: +40 -0 lines" + import re + + m = re.search(r"\+(\d+)\s+-(\d+)\s+lines", comment or "") + if not m: + return 0 + return int(m.group(1)) + int(m.group(2)) + + +def fetch_pr_total_changed_lines(repo: str, pr_number: str) -> Optional[int]: + """Live `additions + deletions` for a merged/open PR, or None if unavailable.""" + result = subprocess.run( + ["gh", "pr", "view", pr_number, "--repo", repo, "--json", "additions,deletions"], + capture_output=True, text=True, + ) + if result.returncode != 0: + return None + try: + data = json.loads(result.stdout) + except json.JSONDecodeError: + return None + additions = data.get("additions") + deletions = data.get("deletions") + if additions is None or deletions is None: + return None + return int(additions) + int(deletions) + + +def build_baseline_records( + comment_records: List[Dict[str, Any]], + start: date, + end: date, + repo: str, + *, + fetch_total_lines=fetch_pr_total_changed_lines, +) -> List[Dict[str, Any]]: + """Aggregate comment-level rows into one v1-shaped record per PR.""" + by_pr: Dict[str, List[Dict[str, Any]]] = {} + for record in comment_records: + if not _in_window(record, start, end): + continue + pr = record.get("pr") + if not pr: + continue + by_pr.setdefault(pr, []).append(record) + + output: List[Dict[str, Any]] = [] + for pr, rows in sorted(by_pr.items(), key=lambda kv: int(kv[0]) if kv[0].isdigit() else kv[0]): + earliest_date = min(r["date"] for r in rows if r.get("date")) + human_review_comments = sum( + 1 for r in rows if r.get("feedback_type") in ("review_comment", "review_verdict") + ) + human_lines = sum( + _human_edit_lines(r.get("comment", "")) + for r in rows if r.get("feedback_type") == "human_edit" + ) + comment_categories: Dict[str, int] = {} + for r in rows: + if r.get("feedback_type") not in ("review_comment", "review_verdict"): + continue + category = r.get("pattern_category") or "general" + comment_categories[category] = comment_categories.get(category, 0) + 1 + + total_lines = fetch_total_lines(repo, pr) + has_agent_commit = True + if total_lines is None: + # Live lookup failed (PR deleted/inaccessible) -- record the PR + # without a churn ratio rather than fabricating a line count. + agent_lines = 0 + has_agent_commit = False + else: + agent_lines = max(total_lines - human_lines, 0) + + skill_used = next((r.get("skill_used") for r in rows if r.get("skill_used") not in (None, "unknown")), "unknown") + + output.append({ + "date": earliest_date, + "pr": pr, + "skill_used": skill_used, + "risk": "unknown", # pre-rollout: the v1 contract did not exist yet + "head_sha": "unknown", + "check_outcome": "unknown", + "review_outcome": "unknown", + "review_critical": 0, + "review_important": 0, + "review_categories": {}, + "human_review_comments": human_review_comments, + "human_review_comment_categories": comment_categories, + "agent_lines_changed": agent_lines, + "human_lines_changed_after_last_agent_commit": human_lines, + "has_agent_commit": has_agent_commit, + "engineering_required": False, # pre-rollout: risk classification did not exist yet + "completion_method": "n/a", + }) + return output + + +def main(argv: Optional[List[str]] = None) -> int: + parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + parser.add_argument("--input", required=True, help="path to human_review_feedback.jsonl") + parser.add_argument("--repo", required=True, help="owner/repo to resolve PR line counts from") + parser.add_argument("--start", required=True, help="window start date, YYYY-MM-DD (inclusive)") + parser.add_argument("--end", required=True, help="window end date, YYYY-MM-DD (inclusive)") + parser.add_argument("--output", required=True, help="write the resulting JSONL records here") + args = parser.parse_args(argv) + + comment_records = load_comment_records(Path(args.input)) + start = date.fromisoformat(args.start) + end = date.fromisoformat(args.end) + records = build_baseline_records(comment_records, start, end, args.repo) + + with open(args.output, "w", encoding="utf-8") as f: + for record in records: + f.write(json.dumps(record) + "\n") + + print(f"Wrote {len(records)} per-PR baseline record(s) to {args.output}") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.agents/skills/improve-drafting-skills/scripts/compute_metrics.py b/.agents/skills/improve-drafting-skills/scripts/compute_metrics.py new file mode 100644 index 000000000..29f76d198 --- /dev/null +++ b/.agents/skills/improve-drafting-skills/scripts/compute_metrics.py @@ -0,0 +1,304 @@ +#!/usr/bin/env python3 +"""Compute the agent-doc quality baseline / outcome metrics report over a window. + +Reads one JSON record per agent-authored, in-scope PR (JSONL) and emits a +deterministic JSON report plus a human-readable summary for an explicit +start/end date window. Used for both the pre-rollout 30-day baseline and the +post-rollout 30-day comparison. + +Record schema (one JSON object per line; unknown/missing fields degrade to +the documented "missing" bucket rather than raising): + { + "date": "YYYY-MM-DD", + "pr": "123", + "skill_used": "draft_feature_doc", + "risk": "low" | "engineering-review-required" | "unknown", + "head_sha": "abc123", + "check_outcome": "pass" | "fail" | "unknown", + "review_outcome": "approve" | "approve_with_nits" | "request_changes" | "unknown", + "review_critical": 0, + "review_important": 0, + "review_categories": {"header_case": 2}, + "human_review_comments": 3, + "human_review_comment_categories": {"header_case": 1}, + "agent_lines_changed": 120, + "human_lines_changed_after_last_agent_commit": 10, + "has_agent_commit": true, + "engineering_required": false, + "completion_method": "n/a" | "source_owner_approval" | "docs_verified" | + "docs_waiver" | "unresolved_owner" | "unanswered_request" + } + + +Usage: + python3 compute_metrics.py --input records.jsonl --start 2026-01-01 --end 2026-01-30 + python3 compute_metrics.py --input records.jsonl --start 2026-01-01 --end 2026-01-30 \ + --baseline baseline.json --output report.json +""" +from __future__ import annotations + +import argparse +import json +import statistics +import sys +from datetime import date +from pathlib import Path +from typing import Any, Dict, List, Optional + +MIN_SAMPLE_SIZE = 10 +REVIEW_OUTCOMES = ("approve", "approve_with_nits", "request_changes") +COMPLETION_METHODS = ( + "source_owner_approval", "docs_verified", "docs_waiver", + "unresolved_owner", "unanswered_request", +) + + +def load_records(path: Path) -> List[Dict[str, Any]]: + records = [] + with open(path, "r", encoding="utf-8") as f: + for line in f: + line = line.strip() + if not line: + continue + records.append(json.loads(line)) + return records + + +def _in_window(record: Dict[str, Any], start: date, end: date) -> bool: + raw = record.get("date") + if not raw: + return False + try: + d = date.fromisoformat(raw) + except ValueError: + return False + return start <= d <= end + + +def _rate(numerator: int, denominator: int) -> Optional[float]: + if denominator == 0: + return None + return round(numerator / denominator, 4) + + +def _mean_median(values: List[float]) -> Dict[str, Optional[float]]: + if not values: + return {"mean": None, "median": None} + return {"mean": round(statistics.fmean(values), 4), "median": round(statistics.median(values), 4)} + + +def _aggregate_categories(records: List[Dict[str, Any]], field: str) -> Dict[str, int]: + totals: Dict[str, int] = {} + for r in records: + for category, count in (r.get(field) or {}).items(): + totals[category] = totals.get(category, 0) + int(count) + return dict(sorted(totals.items())) + + +def compute_metrics(records: List[Dict[str, Any]], start: date, end: date) -> Dict[str, Any]: + """Compute the deterministic v1 report for records in [start, end].""" + window_records = sorted( + (r for r in records if _in_window(r, start, end)), + key=lambda r: (r.get("date", ""), r.get("pr", "")), + ) + + in_scope_count = len(window_records) + # "Complete gate coverage" means every required check reported an actual + # outcome (data completeness) -- it does NOT mean the checks passed. A + # failed check_outcome still counts as coverage here; whether it *passed* + # is tracked separately by `prs_with_passing_checks` and is the only + # input `evaluate_outcome()`'s all-required-checks-passed rule may use. + complete_gate_coverage = sum( + 1 for r in window_records + if r.get("check_outcome") in ("pass", "fail") and r.get("review_outcome") in REVIEW_OUTCOMES + ) + passing_gate_records = [ + r for r in window_records + if r.get("check_outcome") == "pass" and r.get("review_outcome") in ("approve", "approve_with_nits") + ] + prs_with_passing_checks = len(passing_gate_records) + all_passed_required_checks = in_scope_count > 0 and prs_with_passing_checks == in_scope_count + + critical_total = sum(int(r.get("review_critical", 0) or 0) for r in window_records) + important_total = sum(int(r.get("review_important", 0) or 0) for r in window_records) + review_categories = _aggregate_categories(window_records, "review_categories") + + human_comment_counts = [int(r.get("human_review_comments", 0) or 0) for r in window_records] + human_comment_categories = _aggregate_categories(window_records, "human_review_comment_categories") + + churn_ratios: List[float] = [] + zero_denominator_count = 0 + no_agent_commit_count = 0 + for r in window_records: + if not r.get("has_agent_commit", True): + no_agent_commit_count += 1 + continue + agent_lines = int(r.get("agent_lines_changed", 0) or 0) + human_lines = int(r.get("human_lines_changed_after_last_agent_commit", 0) or 0) + if agent_lines == 0: + zero_denominator_count += 1 + continue + churn_ratios.append(human_lines / agent_lines) + + engineering_required = [r for r in window_records if r.get("engineering_required")] + completion_counts: Dict[str, int] = {method: 0 for method in COMPLETION_METHODS} + for r in engineering_required: + method = r.get("completion_method") + if method in completion_counts: + completion_counts[method] += 1 + + eng_required_total = len(engineering_required) + completion_rates = { + method: _rate(count, eng_required_total) for method, count in completion_counts.items() + } + + return { + "window": {"start": start.isoformat(), "end": end.isoformat()}, + "in_scope_prs": in_scope_count, + "prs_with_complete_gate_coverage": complete_gate_coverage, + "gate_coverage_missing_data_count": in_scope_count - complete_gate_coverage, + "prs_with_passing_checks": prs_with_passing_checks, + "all_passed_required_checks": all_passed_required_checks, + "review_findings": { + "critical_total": critical_total, + "important_total": important_total, + "critical_important_per_pr": _rate(critical_total + important_total, in_scope_count), + "targeted_categories": review_categories, + }, + "human_review_comments": { + "total": sum(human_comment_counts), + "per_pr": {**_mean_median([float(c) for c in human_comment_counts])}, + "numerator": sum(human_comment_counts), + "denominator": in_scope_count, + "targeted_categories": human_comment_categories, + }, + "human_edit_churn_ratio": { + **_mean_median(churn_ratios), + "numerator_pr_count": len(churn_ratios), + "denominator_pr_count": in_scope_count, + "zero_denominator_count": zero_denominator_count, + "no_agent_commit_count": no_agent_commit_count, + }, + "engineering_review_required": { + "total": eng_required_total, + "completed_by": completion_counts, + "completion_rates": completion_rates, + }, + } + + +def evaluate_outcome(baseline: Dict[str, Any], current: Dict[str, Any]) -> Dict[str, Any]: + """Evaluate the day-30 outcome rule against a completed baseline report. + + Returns one of `pass`, `fail`, or `inconclusive-small-sample`. Success + requires every in-scope current-window PR to have carried the marker and + passed the required editorial/technical/agent-review checks, in addition + to the comment/churn comparison -- a regression in either dimension is a + `fail`, not a partial pass. + """ + for label, report in (("baseline", baseline), ("current", current)): + if report["in_scope_prs"] < MIN_SAMPLE_SIZE: + return { + "result": "inconclusive-small-sample", + "reason": f"{label} window has only {report['in_scope_prs']} in-scope PRs " + f"(minimum {MIN_SAMPLE_SIZE}); extend collection to 10 PRs or 60 days, " + "whichever comes first.", + } + + if ( + current.get("prs_with_complete_gate_coverage") != current["in_scope_prs"] + or current.get("gate_coverage_missing_data_count", 0) != 0 + ): + return { + "result": "fail", + "reason": ( + f"only {current.get('prs_with_complete_gate_coverage', 0)}/{current['in_scope_prs']} " + "in-scope PRs have complete editorial, technical, and agent-review gate coverage." + ), + } + + if not current.get("all_passed_required_checks"): + return { + "result": "fail", + "reason": ( + f"only {current.get('prs_with_passing_checks', 0)}/{current['in_scope_prs']} " + "in-scope PRs in the current window passed the required editorial, technical, " + "and agent-review checks; all in-scope PRs must pass." + ), + } + + baseline_comments = baseline["human_review_comments"]["per_pr"]["mean"] + current_comments = current["human_review_comments"]["per_pr"]["mean"] + baseline_churn = baseline["human_edit_churn_ratio"]["mean"] + current_churn = current["human_edit_churn_ratio"]["mean"] + + if baseline_comments is None or current_comments is None or baseline_churn is None or current_churn is None: + return { + "result": "inconclusive-small-sample", + "reason": "one or more comparison metrics has no data in one of the windows.", + } + + comments_not_worse = current_comments <= baseline_comments + churn_not_worse = current_churn <= baseline_churn + at_least_one_lower = current_comments < baseline_comments or current_churn < baseline_churn + + passed = comments_not_worse and churn_not_worse and at_least_one_lower + return { + "result": "pass" if passed else "fail", + "reason": ( + f"human review comments/PR: baseline={baseline_comments} current={current_comments}; " + f"human edit churn ratio: baseline={baseline_churn} current={current_churn}" + ), + } + + + +def format_summary(report: Dict[str, Any]) -> str: + """Render a concise human-readable companion to the normalized JSON report.""" + lines = [ + f"Window: {report['window']['start']} to {report['window']['end']}", + f"In-scope PRs: {report['in_scope_prs']}", + ( + "Gate coverage: " + f"{report['prs_with_complete_gate_coverage']}/{report['in_scope_prs']} complete; " + f"{report['prs_with_passing_checks']} passing" + ), + f"Human review comments/PR: {report['human_review_comments']['per_pr']['mean']}", + f"Human edit churn ratio: {report['human_edit_churn_ratio']['mean']}", + ] + if "day_30_outcome" in report: + lines.append( + f"Day-30 outcome: {report['day_30_outcome']['result']} — " + f"{report['day_30_outcome']['reason']}" + ) + return "\n".join(lines) + +def main(argv: Optional[List[str]] = None) -> int: + parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + parser.add_argument("--input", required=True, help="JSONL file of in-scope PR records") + parser.add_argument("--start", required=True, help="window start date, YYYY-MM-DD (inclusive)") + parser.add_argument("--end", required=True, help="window end date, YYYY-MM-DD (inclusive)") + parser.add_argument("--baseline", help="baseline report JSON to compare against for the day-30 outcome") + parser.add_argument("--output", help="write the JSON report to this file (also printed to stdout)") + args = parser.parse_args(argv) + + records = load_records(Path(args.input)) + start = date.fromisoformat(args.start) + end = date.fromisoformat(args.end) + report = compute_metrics(records, start, end) + + if args.baseline: + baseline_report = json.loads(Path(args.baseline).read_text(encoding="utf-8")) + report["day_30_outcome"] = evaluate_outcome(baseline_report, report) + + normalized = json.dumps(report, indent=2, sort_keys=True) + print(normalized) + print(f"\n{format_summary(report)}", file=sys.stderr) + if args.output: + Path(args.output).write_text(normalized + "\n", encoding="utf-8") + + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.agents/skills/improve-drafting-skills/scripts/test_build_baseline_records.py b/.agents/skills/improve-drafting-skills/scripts/test_build_baseline_records.py new file mode 100644 index 000000000..9e3febb33 --- /dev/null +++ b/.agents/skills/improve-drafting-skills/scripts/test_build_baseline_records.py @@ -0,0 +1,92 @@ +#!/usr/bin/env python3 +"""Unit tests for build_baseline_records.py. + +Run: + python3 .agents/skills/improve-drafting-skills/scripts/test_build_baseline_records.py +""" +from __future__ import annotations + +import importlib.util +import unittest +from datetime import date +from pathlib import Path + +_HERE = Path(__file__).resolve().parent +_spec = importlib.util.spec_from_file_location("build_baseline_records", _HERE / "build_baseline_records.py") +bbr = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(bbr) + + +def _fake_fetch(totals): + def fetch(repo, pr): + return totals.get(pr) + return fetch + + +class TestHumanEditLines(unittest.TestCase): + def test_parses_plus_minus_lines(self): + self.assertEqual(bbr._human_edit_lines("Human edit after agent commit: +40 -12 lines"), 52) + + def test_no_match_returns_zero(self): + self.assertEqual(bbr._human_edit_lines("no match here"), 0) + + +class TestBuildBaselineRecords(unittest.TestCase): + def test_aggregates_comments_and_human_lines_per_pr(self): + comments = [ + {"date": "2026-08-05", "pr": "100", "feedback_type": "review_verdict", "comment": "x", "pattern_category": "general"}, + {"date": "2026-08-05", "pr": "100", "feedback_type": "human_edit", "comment": "Human edit after agent commit: +10 -5 lines", "pattern_category": "callout"}, + {"date": "2026-08-06", "pr": "100", "feedback_type": "review_comment", "comment": "nit", "pattern_category": "general"}, + ] + records = bbr.build_baseline_records( + comments, date(2026, 8, 1), date(2026, 8, 31), "o/r", + fetch_total_lines=_fake_fetch({"100": 40}), + ) + self.assertEqual(len(records), 1) + record = records[0] + self.assertEqual(record["pr"], "100") + self.assertEqual(record["human_review_comments"], 2) + self.assertEqual(record["human_review_comment_categories"], {"general": 2}) + self.assertEqual(record["human_lines_changed_after_last_agent_commit"], 15) + self.assertEqual(record["agent_lines_changed"], 25) # 40 - 15 + self.assertEqual(record["risk"], "unknown") + self.assertFalse(record["engineering_required"]) + self.assertTrue(record["has_agent_commit"]) + + def test_excludes_records_outside_window(self): + comments = [ + {"date": "2026-07-01", "pr": "1", "feedback_type": "review_verdict", "comment": "x"}, + ] + records = bbr.build_baseline_records( + comments, date(2026, 8, 1), date(2026, 8, 31), "o/r", + fetch_total_lines=_fake_fetch({}), + ) + self.assertEqual(records, []) + + def test_failed_line_lookup_marks_no_agent_commit(self): + comments = [ + {"date": "2026-08-05", "pr": "200", "feedback_type": "review_verdict", "comment": "x"}, + ] + records = bbr.build_baseline_records( + comments, date(2026, 8, 1), date(2026, 8, 31), "o/r", + fetch_total_lines=_fake_fetch({}), # PR "200" not found + ) + self.assertEqual(len(records), 1) + self.assertFalse(records[0]["has_agent_commit"]) + self.assertEqual(records[0]["agent_lines_changed"], 0) + + def test_negative_derived_agent_lines_floors_at_zero(self): + # If logged human-edit lines somehow exceed the PR's total (e.g. a + # human reverted content the agent never counted), never go negative. + comments = [ + {"date": "2026-08-05", "pr": "300", "feedback_type": "human_edit", "comment": "Human edit after agent commit: +50 -50 lines"}, + ] + records = bbr.build_baseline_records( + comments, date(2026, 8, 1), date(2026, 8, 31), "o/r", + fetch_total_lines=_fake_fetch({"300": 10}), + ) + self.assertEqual(records[0]["agent_lines_changed"], 0) + + +if __name__ == "__main__": + unittest.main() diff --git a/.agents/skills/improve-drafting-skills/scripts/test_compute_metrics.py b/.agents/skills/improve-drafting-skills/scripts/test_compute_metrics.py new file mode 100644 index 000000000..1dcbc4f73 --- /dev/null +++ b/.agents/skills/improve-drafting-skills/scripts/test_compute_metrics.py @@ -0,0 +1,227 @@ +#!/usr/bin/env python3 +"""Unit tests for compute_metrics.py. + +Run: + python3 .agents/skills/improve-drafting-skills/scripts/test_compute_metrics.py +""" +from __future__ import annotations + +import importlib.util +import json +import tempfile +import unittest +from datetime import date +from pathlib import Path + +_HERE = Path(__file__).resolve().parent +_spec = importlib.util.spec_from_file_location("compute_metrics", _HERE / "compute_metrics.py") +cm = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(cm) + + +def _record(**overrides): + base = { + "date": "2026-01-10", + "pr": "1", + "skill_used": "draft_feature_doc", + "risk": "low", + "head_sha": "sha1", + "check_outcome": "pass", + "review_outcome": "approve", + "review_critical": 0, + "review_important": 0, + "review_categories": {}, + "human_review_comments": 2, + "human_review_comment_categories": {"header_case": 1}, + "agent_lines_changed": 100, + "human_lines_changed_after_last_agent_commit": 10, + "has_agent_commit": True, + "engineering_required": False, + "completion_method": "n/a", + } + base.update(overrides) + return base + + +class TestWindowFiltering(unittest.TestCase): + def test_excludes_records_outside_window(self): + records = [_record(date="2025-12-31", pr="0"), _record(date="2026-01-15", pr="1")] + report = cm.compute_metrics(records, date(2026, 1, 1), date(2026, 1, 31)) + self.assertEqual(report["in_scope_prs"], 1) + + def test_missing_date_is_excluded(self): + records = [{"pr": "no-date"}] + report = cm.compute_metrics(records, date(2026, 1, 1), date(2026, 1, 31)) + self.assertEqual(report["in_scope_prs"], 0) + + +class TestChurnAndComments(unittest.TestCase): + def test_churn_ratio_computed_correctly(self): + records = [_record(agent_lines_changed=100, human_lines_changed_after_last_agent_commit=25)] + report = cm.compute_metrics(records, date(2026, 1, 1), date(2026, 1, 31)) + self.assertEqual(report["human_edit_churn_ratio"]["mean"], 0.25) + + def test_zero_denominator_reported_separately(self): + records = [_record(agent_lines_changed=0, human_lines_changed_after_last_agent_commit=5)] + report = cm.compute_metrics(records, date(2026, 1, 1), date(2026, 1, 31)) + self.assertEqual(report["human_edit_churn_ratio"]["zero_denominator_count"], 1) + self.assertIsNone(report["human_edit_churn_ratio"]["mean"]) + + def test_no_agent_commit_reported_separately(self): + records = [_record(has_agent_commit=False)] + report = cm.compute_metrics(records, date(2026, 1, 1), date(2026, 1, 31)) + self.assertEqual(report["human_edit_churn_ratio"]["no_agent_commit_count"], 1) + + def test_human_comment_categories_aggregate_across_records(self): + records = [ + _record(pr="1", human_review_comment_categories={"header_case": 2}), + _record(pr="2", human_review_comment_categories={"header_case": 1, "link_quality": 3}), + ] + report = cm.compute_metrics(records, date(2026, 1, 1), date(2026, 1, 31)) + self.assertEqual( + report["human_review_comments"]["targeted_categories"], + {"header_case": 3, "link_quality": 3}, + ) + + +class TestEngineeringReviewCompletion(unittest.TestCase): + def test_completion_rates_computed_per_method(self): + records = [ + _record(pr="1", engineering_required=True, completion_method="source_owner_approval"), + _record(pr="2", engineering_required=True, completion_method="docs_verified"), + _record(pr="3", engineering_required=True, completion_method="unresolved_owner"), + _record(pr="4", engineering_required=False), + ] + report = cm.compute_metrics(records, date(2026, 1, 1), date(2026, 1, 31)) + eng = report["engineering_review_required"] + self.assertEqual(eng["total"], 3) + self.assertEqual(eng["completed_by"]["source_owner_approval"], 1) + self.assertEqual(eng["completed_by"]["docs_verified"], 1) + self.assertEqual(eng["completed_by"]["unresolved_owner"], 1) + self.assertAlmostEqual(eng["completion_rates"]["source_owner_approval"], 1 / 3, places=4) + + +class TestGatePassRate(unittest.TestCase): + def test_all_passed_required_checks_true_when_every_pr_passes(self): + records = [_record(pr="1"), _record(pr="2")] # check_outcome=pass, review_outcome=approve by default + report = cm.compute_metrics(records, date(2026, 1, 1), date(2026, 1, 31)) + self.assertEqual(report["prs_with_passing_checks"], 2) + self.assertTrue(report["all_passed_required_checks"]) + + def test_all_passed_required_checks_false_when_one_pr_failed_checks(self): + records = [_record(pr="1"), _record(pr="2", check_outcome="fail")] + report = cm.compute_metrics(records, date(2026, 1, 1), date(2026, 1, 31)) + self.assertEqual(report["prs_with_passing_checks"], 1) + self.assertFalse(report["all_passed_required_checks"]) + + def test_failed_check_still_counts_as_complete_gate_coverage(self): + # A failed check is data, not missing data -- coverage tracks + # whether the gate reported an outcome, separately from whether it passed. + records = [_record(pr="1", check_outcome="fail")] + report = cm.compute_metrics(records, date(2026, 1, 1), date(2026, 1, 31)) + self.assertEqual(report["prs_with_complete_gate_coverage"], 1) + self.assertEqual(report["gate_coverage_missing_data_count"], 0) + self.assertFalse(report["all_passed_required_checks"]) + + def test_missing_review_outcome_is_incomplete_gate_coverage(self): + record = _record(pr="1") + del record["review_outcome"] + report = cm.compute_metrics([record], date(2026, 1, 1), date(2026, 1, 31)) + self.assertEqual(report["prs_with_complete_gate_coverage"], 0) + self.assertEqual(report["gate_coverage_missing_data_count"], 1) + + def test_request_changes_review_outcome_is_not_passing(self): + records = [_record(pr="1", review_outcome="request_changes")] + report = cm.compute_metrics(records, date(2026, 1, 1), date(2026, 1, 31)) + self.assertEqual(report["prs_with_passing_checks"], 0) + self.assertFalse(report["all_passed_required_checks"]) + + def test_no_in_scope_prs_is_not_trivially_all_passed(self): + report = cm.compute_metrics([], date(2026, 1, 1), date(2026, 1, 31)) + self.assertFalse(report["all_passed_required_checks"]) + + +class TestReproducibility(unittest.TestCase): + def test_running_twice_over_frozen_dates_is_byte_equivalent(self): + records = [ + _record(pr=str(i), date=f"2026-01-{10 + (i % 15):02d}") + for i in range(15) + ] + with tempfile.TemporaryDirectory() as tmp: + input_path = Path(tmp) / "records.jsonl" + with open(input_path, "w", encoding="utf-8") as f: + for r in records: + f.write(json.dumps(r) + "\n") + + out1 = Path(tmp) / "out1.json" + out2 = Path(tmp) / "out2.json" + cm.main(["--input", str(input_path), "--start", "2026-01-01", "--end", "2026-01-31", "--output", str(out1)]) + cm.main(["--input", str(input_path), "--start", "2026-01-01", "--end", "2026-01-31", "--output", str(out2)]) + self.assertEqual(out1.read_text(encoding="utf-8"), out2.read_text(encoding="utf-8")) + + +class TestOutcomeEvaluation(unittest.TestCase): + def _report(self, in_scope, comments_mean, churn_mean, all_passed_required_checks=True): + return { + "in_scope_prs": in_scope, + "human_review_comments": {"per_pr": {"mean": comments_mean}}, + "human_edit_churn_ratio": {"mean": churn_mean}, + "all_passed_required_checks": all_passed_required_checks, + "prs_with_passing_checks": in_scope if all_passed_required_checks else in_scope - 1, + "prs_with_complete_gate_coverage": in_scope, + "gate_coverage_missing_data_count": 0, + } + + def test_small_sample_is_inconclusive(self): + baseline = self._report(5, 3.0, 0.2) + current = self._report(12, 2.0, 0.1) + outcome = cm.evaluate_outcome(baseline, current) + self.assertEqual(outcome["result"], "inconclusive-small-sample") + + def test_pass_when_both_improve_or_hold(self): + baseline = self._report(12, 3.0, 0.3) + current = self._report(12, 2.0, 0.3) + outcome = cm.evaluate_outcome(baseline, current) + self.assertEqual(outcome["result"], "pass") + + def test_fail_when_either_regresses(self): + baseline = self._report(12, 3.0, 0.2) + current = self._report(12, 3.5, 0.1) + outcome = cm.evaluate_outcome(baseline, current) + self.assertEqual(outcome["result"], "fail") + + def test_fail_when_neither_improves(self): + baseline = self._report(12, 3.0, 0.2) + current = self._report(12, 3.0, 0.2) + outcome = cm.evaluate_outcome(baseline, current) + self.assertEqual(outcome["result"], "fail") + + def test_fail_when_not_every_current_pr_passed_required_checks(self): + # Regression: comments/churn improving must not paper over a PR that + # didn't actually pass the required editorial/technical/review gates. + baseline = self._report(12, 3.0, 0.3) + current = self._report(12, 1.0, 0.1, all_passed_required_checks=False) + outcome = cm.evaluate_outcome(baseline, current) + self.assertEqual(outcome["result"], "fail") + self.assertIn("passed the required", outcome["reason"]) + + def test_fail_when_current_gate_coverage_is_incomplete(self): + baseline = self._report(12, 3.0, 0.3) + current = self._report(12, 1.0, 0.1) + current["prs_with_complete_gate_coverage"] = 11 + current["gate_coverage_missing_data_count"] = 1 + outcome = cm.evaluate_outcome(baseline, current) + self.assertEqual(outcome["result"], "fail") + self.assertIn("complete", outcome["reason"]) + + +class TestSummary(unittest.TestCase): + def test_summary_is_human_readable(self): + report = cm.compute_metrics([_record()], date(2026, 1, 1), date(2026, 1, 31)) + summary = cm.format_summary(report) + self.assertIn("Window: 2026-01-01 to 2026-01-31", summary) + self.assertIn("Gate coverage:", summary) + + +if __name__ == "__main__": + unittest.main() diff --git a/.agents/skills/missing_docs/SKILL.md b/.agents/skills/missing_docs/SKILL.md index 90d02c744..4aed43e80 100644 --- a/.agents/skills/missing_docs/SKILL.md +++ b/.agents/skills/missing_docs/SKILL.md @@ -17,6 +17,15 @@ description: >- Find documentation gaps, detect doc-impacting code changes, and draft missing pages. +## Agent-doc quality contract + +Any PR this skill opens or updates follows the shared v1 agent-doc quality +contract in `.agents/references/doc-quality-policy.md`: apply the +`warpy-factory` label, add the `## Documentation risk` block +(`.agents/skills/doc_quality_policy/finalize_pr_contract.py build`), and keep +`## Unverified claims` (step 9.5 of `draft_docs`) current. A newly-drafted +feature page is `engineering-review-required` by default per the allowlist. + ## Requirements The audit compares docs against code, so both source repos must be available: diff --git a/.agents/skills/release_updates/SKILL.md b/.agents/skills/release_updates/SKILL.md index 1bddb9fff..b61c1a30e 100644 --- a/.agents/skills/release_updates/SKILL.md +++ b/.agents/skills/release_updates/SKILL.md @@ -19,6 +19,17 @@ They support the following: (`/channel-versions`, sibling repo) - running one task or all tasks in the required order +## Agent-doc quality contract + +Before requesting review on a PR this skill opens, follow the shared v1 +agent-doc quality contract in `.agents/references/doc-quality-policy.md`: +add the `## Documentation risk` block +(`.agents/skills/doc_quality_policy/finalize_pr_contract.py build`) and apply +the `warpy-factory` label. Generated changelog/license/telemetry updates are +exempt from the compression word-budget rules but not from the risk/marker +contract — classify them per the low-risk allowlist (they are typically `low` +when the source-verification step passed). + ## Environment requirements (Oz cloud) ### Required diff --git a/.agents/skills/release_updates/scripts/run_release_updates.py b/.agents/skills/release_updates/scripts/run_release_updates.py index 0bd880b87..88620ac80 100644 --- a/.agents/skills/release_updates/scripts/run_release_updates.py +++ b/.agents/skills/release_updates/scripts/run_release_updates.py @@ -331,6 +331,21 @@ def _default_pr_body(*, ordered_tasks: Sequence[str], changed_files: Sequence[st "", "## Validation", "- Ran the release update scripts successfully.", + "", + # This is a real, direct PR-creation code path (not delegated to + # create_pr), so it carries the same v1 doc-quality contract + # sections every other PR-producing skill does. Changelog/license/ + # telemetry updates are generated verbatim from their source of + # record, which is the allowlisted "low" category for generated data. + "## Documentation risk", + "Risk: low", + "Rationale: Changelog/license/telemetry entries generated verbatim from " + "channel_versions.json / the license scan / the telemetry command; no " + "hand-authored product claim.", + "Docs override: none", + "", + "## Unverified claims", + "None \u2014 generated data copied from its source of record.", ], ) return _ensure_coauthor_line("\n".join(lines)) @@ -377,6 +392,8 @@ def _create_or_update_pull_request( pr_title, "--body", pr_body, + "--add-label", + "warpy-factory", ], cwd=repo_path, ) @@ -392,6 +409,8 @@ def _create_or_update_pull_request( pr_title, "--body", pr_body, + "--label", + "warpy-factory", ] if pr_draft: command.append("--draft") diff --git a/.agents/skills/review-docs-pr/SKILL.md b/.agents/skills/review-docs-pr/SKILL.md index c07e87556..d5d890a1e 100644 --- a/.agents/skills/review-docs-pr/SKILL.md +++ b/.agents/skills/review-docs-pr/SKILL.md @@ -16,6 +16,42 @@ Use this skill when reviewing documentation changes in PRs. The skill will: - Review AEO/source-data fit for docs changes that target search or answer-engine visibility - Check Astro Starlight structure integrity +When the PR carries the `warpy-factory` label, this is the **independent v1 +agent-doc quality review pass** dispatched by +`.github/workflows/agent-docs-review.yml` for the current head SHA (see +"Agent-marked PR review (v1 contract)" below). + +## Agent-marked PR review (v1 contract) + +For a PR carrying the `warpy-factory` label, in addition to the standard +review focus areas, this pass is the human-review-blocking gate defined in +`.agents/references/doc-quality-policy.md`: + +1. **Re-validate the declared risk.** Parse the PR's `## Documentation risk` + section (`.agents/skills/doc_quality_policy/policy.py`'s + `parse_documentation_risk_section`). Walk the diff against the low-risk + allowlist yourself. A declared `low` risk that actually touches any + engineering-review trigger (a new/changed feature page, or any of the + technical claim categories in the allowlist) is a **risk + misclassification** — flag it `⚠️ [IMPORTANT]` and reflect it in the + verdict (see "Severity Labels" below); it is not a soft suggestion. +2. **Verify technical claims against source** when the PR is (or should be) + `engineering-review-required`: confirm the cited `Source files consulted` + actually support the claims. Use `answer_question` when source access is + available. +3. **Check the compression contract.** Run + `.agents/skills/doc_quality_policy/check_compression_contract.py` against + each changed content-type page. An unjustified violation (no reasoning in + the PR body for the overage) is `⚠️ [IMPORTANT]`; a justified one is + `💡 [SUGGESTION]` at most. +4. **Check VERIFY accounting.** Run the structural mode of + `.agents/skills/doc_quality_policy/check_pr_contract.py --body + ` against the current head. Do not invoke + `--enforce-engineering-gate` during the independent review; a pending human + approval is not itself a contract violation. Any reported contract + violation (missing section, unlisted marker, listed marker at `low` risk) + is `🚨 [CRITICAL]`. + ## Review Instructions Review this documentation PR for the docs repository. @@ -36,11 +72,15 @@ Provide actionable, constructive feedback. Focus on documentation quality issues ### Severity Labels (Required) Every comment body MUST begin with one of: -- `🚨 [CRITICAL]` — Broken links, incorrect commands/code, factually wrong information that could confuse users -- `⚠️ [IMPORTANT]` — Style guide violations, missing redirects, structural issues +- `🚨 [CRITICAL]` — Broken links, incorrect commands/code, factually wrong information that could confuse users, or a v1 contract violation (missing/invalid `## Documentation risk`, an unlisted `VERIFY` marker) +- `⚠️ [IMPORTANT]` — Style guide violations, missing redirects, structural issues, a documentation-risk misclassification, or an unjustified compression-contract violation - `💡 [SUGGESTION]` — Improvements to clarity, wording, or structure - `🧹 [NIT]` — Typos, minor formatting (ONLY if providing a suggestion block) +**Blocking rule for agent-marked PRs.** Any `🚨 [CRITICAL]` or `⚠️ [IMPORTANT]` +finding — including a risk misclassification — means the verdict is +**Request changes**. Suggestions and nits never block. + ### Using answer_question for verification If you encounter: @@ -131,7 +171,60 @@ After creating and validating `review.json` (immediately after the Validation se 3. Determine the skill used from the PR branch name or PR description if available. 4. Include the following structured marker in your **text response** (write it as part of your agent message, not via a shell `echo` command). This ensures it appears as a `TextContentBlock` in the conversation, where `oz run get --conversation` can reliably retrieve it: ``` - [SIGNAL:pr-review] {"date":"YYYY-MM-DD","pr":"NNN","branch":"branch-name","skill_used":"draft_feature_doc","verdict":"Request changes","critical":N,"important":N,"suggestions":N,"nits":N,"top_categories":["category (N)","category (N)","category (N)"]} + [SIGNAL:pr-review] {"date":"YYYY-MM-DD","pr":"NNN","branch":"branch-name","head_sha":"abc1234","skill_used":"draft_feature_doc","reviewer_login":"GITHUB_LOGIN","verdict":"Request changes","critical":N,"important":N,"suggestions":N,"nits":N,"top_categories":["category (N)","category (N)","category (N)"]} ``` + Set `head_sha` to the exact commit SHA this review evaluated (the head SHA + `.github/workflows/agent-docs-review.yml` passed in, or `gh pr view NNN + --json headRefOid --jq .headRefOid` when reviewing interactively). A push + of a new commit makes any earlier signal for this PR stale; the collector + in `improve-drafting-skills` keys its `review_outcome` lookup on this field + matching the PR's current head. The `improve-drafting-skills` outer loop reads this signal from the conversation via `oz run get --conversation`, scanning assistant `TextContentBlock` messages for the marker. No git operations are required. + +## Publishing a GitHub review + +After creating `review.json`, publishing the signal, and completing validation, create one GitHub PR review pinned to the evaluated head SHA. The review body must include the same `[SIGNAL:pr-review]` JSON record used in the text response. + +1. Determine the authenticated reviewer and map the verdict: + ```bash + REVIEWER_LOGIN=$(gh api user --jq .login) + ``` + Use `APPROVE` for `Approve`, `REQUEST_CHANGES` for `Request changes`, and `COMMENT` for `Approve with nits`. +2. Write the signal JSON object to `/tmp/review-signal.json`, set its + `reviewer_login` to `$REVIEWER_LOGIN`, and render that same object as the + `[SIGNAL:pr-review]` line in the final response. Then construct the + pinned review request from `review.json`: + ```bash + HEAD_SHA="" + VERDICT="" + export HEAD_SHA VERDICT + python3 - <<'PY' + import json + import os + from pathlib import Path + + review = json.loads(Path("review.json").read_text()) + signal = json.loads(Path("/tmp/review-signal.json").read_text()) + event = { + "Approve": "APPROVE", + "Approve with nits": "COMMENT", + "Request changes": "REQUEST_CHANGES", + }[os.environ["VERDICT"]] + payload = { + "commit_id": os.environ["HEAD_SHA"], + "event": event, + "body": f"{review['summary']}\n\n[SIGNAL:pr-review] {json.dumps(signal, sort_keys=True)}", + "comments": review["comments"], + } + Path("/tmp/review-request.json").write_text(json.dumps(payload)) + PY + ``` +3. Submit the review: + ```bash + gh api --method POST "repos/OWNER/REPO/pulls/PR_NUMBER/reviews" \ + --input /tmp/review-request.json + ``` + Replace `OWNER/REPO`, `PR_NUMBER`, and `commit_id` with the pull request being reviewed and its exact head SHA. + +The workflow verifies that a current-head review from `reviewer_login` contains the exact signal fields. Do not leave the signal only in the agent response or an issue comment. diff --git a/.agents/skills/style_lint/SKILL.md b/.agents/skills/style_lint/SKILL.md index ce83acc60..279a9f429 100644 --- a/.agents/skills/style_lint/SKILL.md +++ b/.agents/skills/style_lint/SKILL.md @@ -79,6 +79,10 @@ When run with `--fix`: - **Low-confidence issues reported but not auto-fixed**: link quality, VideoEmbed title specificity, list format, header case (due to feature name exceptions), ambiguous terminology - **Tone checks are never auto-fixed**: buzzwords, meta-openers, and callout budget issues always need a human rewrite +## Agent-doc quality contract + +The `--create-pr` auto-fix path is a direct PR-creation code path (`create_pr_with_fixes()` in `style_lint.py`), so it stamps the `warpy-factory` label and the `## Documentation risk` block itself — always `low` risk, since an auto-fix only ever corrects style/terminology wording, never product meaning. See `.agents/references/doc-quality-policy.md`. + ## Relationship to validate_ui_refs This skill checks broader formatting and terminology. The `validate_ui_refs` skill validates UI paths and Command Palette names against the warp-internal codebase. They complement each other with no overlap. Both can run in scheduled cloud agent workflows. diff --git a/.agents/skills/style_lint/style_lint.py b/.agents/skills/style_lint/style_lint.py index a582bc900..a8e68dd49 100644 --- a/.agents/skills/style_lint/style_lint.py +++ b/.agents/skills/style_lint/style_lint.py @@ -1749,10 +1749,25 @@ def create_pr_with_fixes() -> None: "docs: auto-fix style lint issues\n\nCo-Authored-By: Oz ", ], check=True) subprocess.run(["git", "push", "origin", branch], check=True) + # This is a real, direct PR-creation code path (not delegated to + # create_pr), so it carries the same v1 doc-quality contract sections + # every other PR-producing skill does. Auto-fixed style/terminology + # corrections are mechanically `low` risk: they never change product + # meaning, only wording/formatting to match AGENTS.md. + contract = ( + "## Documentation risk\n" + "Risk: low\n" + "Rationale: Mechanical style/terminology auto-fixes only, applied by " + "style_lint.py --fix; no product meaning changes.\n" + "Docs override: none\n\n" + "## Unverified claims\n" + "None \u2014 formatting/terminology fixes only." + ) subprocess.run([ "gh", "pr", "create", "--title", "docs: auto-fix style lint issues", - "--body", "Automated fixes from `style_lint.py --fix`.\n\nCo-Authored-By: Oz ", + "--body", f"Automated fixes from `style_lint.py --fix`.\n\n{contract}\n\nCo-Authored-By: Oz ", + "--label", "warpy-factory", ], check=True) diff --git a/.agents/skills/sync-error-docs/SKILL.md b/.agents/skills/sync-error-docs/SKILL.md index 209c67913..57f5698b4 100644 --- a/.agents/skills/sync-error-docs/SKILL.md +++ b/.agents/skills/sync-error-docs/SKILL.md @@ -10,6 +10,15 @@ description: >- Ensure every `ErrorCode` in `platformerrors.go` has a corresponding documentation page, sidebar entry, and redirect. +## Agent-doc quality contract + +The PR this skill opens or updates follows the shared v1 agent-doc quality +contract in `.agents/references/doc-quality-policy.md`: apply the +`warpy-factory` label and add the `## Documentation risk` block +(`.agents/skills/doc_quality_policy/finalize_pr_contract.py build`). New error +pages are `engineering-review-required` (new page, technical claims) unless +generated verbatim from `platformerrors.go` with no invented prose. + ## Repos This skill requires two repos in the agent's environment: diff --git a/.agents/skills/sync-openapi-spec/SKILL.md b/.agents/skills/sync-openapi-spec/SKILL.md index a90df6954..c5b33fa64 100644 --- a/.agents/skills/sync-openapi-spec/SKILL.md +++ b/.agents/skills/sync-openapi-spec/SKILL.md @@ -15,6 +15,16 @@ Keep `developers/agent-api-openapi.yaml` in sync with the canonical spec at `war **Direction:** warp-server → docs. The server spec is the source of truth. The docs file is a curated subset (drops `memory_stores`/`harness-support` and a handful of internal `agent` paths) that Scalar renders on `docs.warp.dev/api`. +## Agent-doc quality contract + +The PR this skill opens or updates follows the shared v1 agent-doc quality +contract in `.agents/references/doc-quality-policy.md`: apply the +`warpy-factory` label and add the `## Documentation risk` block +(`.agents/skills/doc_quality_policy/finalize_pr_contract.py build`). A +regenerated spec sync is `engineering-review-required` (changes API behavior +claims) unless the diff is provably a mechanical passthrough of the source +spec with no manual edits. + ## Repos This skill requires two repos in the agent's environment: diff --git a/.agents/skills/sync_terminology/SKILL.md b/.agents/skills/sync_terminology/SKILL.md index 029c4c250..d2e7ad75d 100644 --- a/.agents/skills/sync_terminology/SKILL.md +++ b/.agents/skills/sync_terminology/SKILL.md @@ -13,6 +13,15 @@ Keep `.agents/references/terminology.md` in sync with the canonical Notion Dicti **Direction:** Notion → repo. Notion is the source of truth. If the repo has terms not in Notion, flag them for addition to Notion rather than removing them from the repo. +## Agent-doc quality contract + +The PR this skill opens or updates follows the shared v1 agent-doc quality +contract in `.agents/references/doc-quality-policy.md`: apply the +`warpy-factory` label and add the `## Documentation risk` block +(`.agents/skills/doc_quality_policy/finalize_pr_contract.py build`). A +terminology sync is typically `low` risk — it preserves product meaning and +changes only naming/formatting guidance. + ## Prerequisites - Notion MCP server must be connected and authenticated diff --git a/.agents/skills/update-changelog/SKILL.md b/.agents/skills/update-changelog/SKILL.md index 2b1db3541..f9ddbd917 100644 --- a/.agents/skills/update-changelog/SKILL.md +++ b/.agents/skills/update-changelog/SKILL.md @@ -10,6 +10,18 @@ Adds a new entry to the current year's changelog page (e.g. `src/content/docs/ch ## Related Skills - `create_pr` - PR creation guidelines for this repo +- `doc_quality_policy` - the shared agent-doc quality contract this skill's PR follows + +## Agent-doc quality contract + +The PR this skill opens or updates follows the shared v1 agent-doc quality +contract in `.agents/references/doc-quality-policy.md`: apply the +`warpy-factory` label and add the `## Documentation risk` block +(`.agents/skills/doc_quality_policy/finalize_pr_contract.py build`). A +changelog entry copied 1:1 from `channel_versions.json` is `low` risk +(generated data whose source-verification step — the 1:1 match — passed); +any manually written or fallback-sourced entry (Step 4) is +`engineering-review-required`. ## Workflow diff --git a/.agents/skills/validate_ui_refs/SKILL.md b/.agents/skills/validate_ui_refs/SKILL.md index 5d82e8458..d473fca02 100644 --- a/.agents/skills/validate_ui_refs/SKILL.md +++ b/.agents/skills/validate_ui_refs/SKILL.md @@ -184,6 +184,10 @@ python3 .agents/skills/validate_ui_refs/validate_ui_refs.py \ --warp /path/to/warp ``` +## Agent-doc quality contract + +The `--create-pr` auto-fix path is a direct PR-creation code path (`create_pr()` in `validate_ui_refs.py`), so it stamps the `warpy-factory` label and the `## Documentation risk` block itself — always `low` risk, since an auto-fix only ever corrects UI-reference casing/formatting to an already-canonical name, never product meaning. See `.agents/references/doc-quality-policy.md`. + ## Dependencies - Python 3.7+ diff --git a/.agents/skills/validate_ui_refs/test_changed_mode.py b/.agents/skills/validate_ui_refs/test_changed_mode.py new file mode 100644 index 000000000..48584729e --- /dev/null +++ b/.agents/skills/validate_ui_refs/test_changed_mode.py @@ -0,0 +1,225 @@ +#!/usr/bin/env python3 +"""Unit tests for validate_ui_refs.py's --changed mode and snapshot provenance. + +Uses a throwaway git repo fixture (no dependency on the real docs repo's +history) so these tests are hermetic and match `style_lint.py`'s own +`--changed` test approach of exercising real git behavior rather than mocking +subprocess. + +Run: + python3 .agents/skills/validate_ui_refs/test_changed_mode.py +""" +from __future__ import annotations + +import importlib.util +import json +import os +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +_HERE = Path(__file__).resolve().parent +_spec = importlib.util.spec_from_file_location("validate_ui_refs", _HERE / "validate_ui_refs.py") +vur = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(vur) + + +def _run(cmd, cwd): + subprocess.run(cmd, cwd=cwd, capture_output=True, text=True, check=True) + + +def _init_repo_with_main_and_changes(tmp: Path) -> Path: + """Create a git repo with an `origin/main` ref and unstaged HEAD changes.""" + repo = tmp / "repo" + repo.mkdir() + _run(["git", "init", "-q"], repo) + _run(["git", "config", "user.email", "test@example.com"], repo) + _run(["git", "config", "user.name", "Test"], repo) + + docs_dir = repo / "src" / "content" / "docs" + docs_dir.mkdir(parents=True) + (docs_dir / "existing.mdx").write_text("existing content\n", encoding="utf-8") + (docs_dir / "changelog").mkdir() + (docs_dir / "changelog" / "2026.mdx").write_text("changelog\n", encoding="utf-8") + _run(["git", "add", "."], repo) + _run(["git", "commit", "-q", "-m", "initial"], repo) + # Fake an origin/main remote-tracking ref pointing at the initial commit. + # `git branch -f refs/remotes/origin/main` would create refs/heads/refs/... + # (branch names live under refs/heads/), so use update-ref directly. + _run(["git", "update-ref", "refs/remotes/origin/main", "HEAD"], repo) + + # Now make changes on top: a new page, an edit to an existing page, and a + # non-markdown file (should never be picked up). + (docs_dir / "new-page.mdx").write_text("new page\n", encoding="utf-8") + (docs_dir / "existing.mdx").write_text("existing content, edited\n", encoding="utf-8") + (repo / "README.md").write_text("not in docs dir\n", encoding="utf-8") + _run(["git", "add", "."], repo) + _run(["git", "commit", "-q", "-m", "changes"], repo) + return repo + + +class TestFindChangedMdFiles(unittest.TestCase): + def test_finds_new_and_edited_files_under_docs_dir(self): + with tempfile.TemporaryDirectory() as tmp: + repo = _init_repo_with_main_and_changes(Path(tmp)) + docs_dir = repo / "src" / "content" / "docs" + import os + old_cwd = os.getcwd() + os.chdir(repo) + try: + files = vur.find_changed_md_files(docs_dir) + finally: + os.chdir(old_cwd) + names = sorted(f.name for f in files) + self.assertEqual(names, ["existing.mdx", "new-page.mdx"]) + + def test_excludes_changelog_by_default(self): + with tempfile.TemporaryDirectory() as tmp: + repo = _init_repo_with_main_and_changes(Path(tmp)) + docs_dir = repo / "src" / "content" / "docs" + (docs_dir / "changelog" / "2026.mdx").write_text("changelog, edited\n", encoding="utf-8") + _run(["git", "add", "."], repo) + _run(["git", "commit", "-q", "-m", "changelog edit"], repo) + + import os + old_cwd = os.getcwd() + os.chdir(repo) + try: + files = vur.find_changed_md_files(docs_dir) + files_with_changelog = vur.find_changed_md_files(docs_dir, include_changelog=True) + finally: + os.chdir(old_cwd) + + self.assertNotIn("2026.mdx", [f.name for f in files]) + self.assertIn("2026.mdx", [f.name for f in files_with_changelog]) + + def test_deleted_file_is_excluded(self): + with tempfile.TemporaryDirectory() as tmp: + repo = _init_repo_with_main_and_changes(Path(tmp)) + docs_dir = repo / "src" / "content" / "docs" + (docs_dir / "existing.mdx").unlink() + _run(["git", "add", "."], repo) + _run(["git", "commit", "-q", "-m", "delete existing"], repo) + + import os + old_cwd = os.getcwd() + os.chdir(repo) + try: + files = vur.find_changed_md_files(docs_dir) + finally: + os.chdir(old_cwd) + self.assertNotIn("existing.mdx", [f.name for f in files]) + + def test_unresolvable_diff_raises_instead_of_falling_back(self): + with tempfile.TemporaryDirectory() as tmp: + # A repo with no origin/main ref at all: the diff can't resolve. + repo = Path(tmp) / "no_main" + repo.mkdir() + _run(["git", "init", "-q"], repo) + docs_dir = repo / "src" / "content" / "docs" + docs_dir.mkdir(parents=True) + + import os + old_cwd = os.getcwd() + os.chdir(repo) + try: + with self.assertRaises(vur.ChangedFilesUnresolvedError): + vur.find_changed_md_files(docs_dir) + finally: + os.chdir(old_cwd) + + +class TestRequireProvenanceFlag(unittest.TestCase): + """Regression for the required-CI-gate fail-closed rule: an incomplete + snapshot (missing source_repository/source_sha) must not be silently + trusted just because scanning found no other issues. + """ + + def _run_with_snapshot(self, valid_paths: dict) -> int: + with tempfile.TemporaryDirectory() as tmp: + repo = _init_repo_with_main_and_changes(Path(tmp)) + valid_paths_file = repo / "valid_paths.json" + valid_paths_file.write_text(json.dumps(valid_paths), encoding="utf-8") + docs_dir = repo / "src" / "content" / "docs" + old_argv = sys.argv + sys.argv = [ + "validate_ui_refs.py", "--changed", "--require-provenance", + "--docs-dir", str(docs_dir), "--valid-paths", str(valid_paths_file), + ] + old_cwd = os.getcwd() + os.chdir(repo) + try: + return vur.main() + finally: + sys.argv = old_argv + os.chdir(old_cwd) + + def test_missing_provenance_fails_closed(self): + exit_code = self._run_with_snapshot({"settings_sections": {}, "generated_at": "2026-01-01T00:00:00Z"}) + self.assertEqual(exit_code, 1) + + def test_null_source_sha_fails_closed(self): + exit_code = self._run_with_snapshot({ + "settings_sections": {}, "source_repository": "warpdotdev/warp", + "source_sha": None, "generated_at": "2026-01-01T00:00:00Z", + }) + self.assertEqual(exit_code, 1) + + def test_complete_provenance_passes(self): + exit_code = self._run_with_snapshot({ + "settings_sections": {}, "source_repository": "warpdotdev/warp", + "source_sha": "abc123", "generated_at": "2026-01-01T00:00:00Z", + }) + self.assertEqual(exit_code, 0) + + +class TestSnapshotProvenance(unittest.TestCase): + def test_resolve_source_sha_reads_head(self): + with tempfile.TemporaryDirectory() as tmp: + repo = Path(tmp) / "warp" + repo.mkdir() + _run(["git", "init", "-q"], repo) + _run(["git", "config", "user.email", "test@example.com"], repo) + _run(["git", "config", "user.name", "Test"], repo) + (repo / "f.txt").write_text("x", encoding="utf-8") + _run(["git", "add", "."], repo) + _run(["git", "commit", "-q", "-m", "c"], repo) + expected = subprocess.run( + ["git", "-C", str(repo), "rev-parse", "HEAD"], + capture_output=True, text=True, check=True, + ).stdout.strip() + self.assertEqual(vur._resolve_source_sha(repo), expected) + + def test_resolve_source_sha_missing_repo_returns_none(self): + self.assertIsNone(vur._resolve_source_sha(Path("/nonexistent/path/xyz"))) + + def test_refresh_preserves_existing_source_sha_when_git_resolution_fails(self): + with tempfile.TemporaryDirectory() as tmp: + output = Path(tmp) / "valid_paths.json" + output.write_text( + json.dumps({"source_repository": "warpdotdev/warp", "source_sha": "trusted-sha"}), + encoding="utf-8", + ) + with mock.patch.object(vur, "_extract_settings_sections", return_value={}), \ + mock.patch.object(vur, "_extract_command_palette_commands", return_value={}), \ + mock.patch.object(vur, "_extract_umbrellas", return_value={}), \ + mock.patch.object(vur, "_resolve_source_repository", return_value=None), \ + mock.patch.object(vur, "_resolve_source_sha", return_value=None): + vur.refresh_valid_paths(Path(tmp) / "missing-warp", output) + refreshed = json.loads(output.read_text(encoding="utf-8")) + self.assertEqual(refreshed["source_sha"], "trusted-sha") + + def test_resolve_source_repository_parses_https_remote(self): + with tempfile.TemporaryDirectory() as tmp: + repo = Path(tmp) / "warp" + repo.mkdir() + _run(["git", "init", "-q"], repo) + _run(["git", "remote", "add", "origin", "https://github.com/warpdotdev/warp.git"], repo) + self.assertEqual(vur._resolve_source_repository(repo), "warpdotdev/warp") + + +if __name__ == "__main__": + unittest.main() diff --git a/.agents/skills/validate_ui_refs/valid_paths.json b/.agents/skills/validate_ui_refs/valid_paths.json index 7e69fe623..e9dc4d4a1 100644 --- a/.agents/skills/validate_ui_refs/valid_paths.json +++ b/.agents/skills/validate_ui_refs/valid_paths.json @@ -834,6 +834,10 @@ "name": "input:search_command_history", "description": "History Search" }, + { + "name": "workspace:trigger_external_ctrl_t_file_search", + "description": "External File Search" + }, { "name": "input:toggle_workflows", "description": "Workflows" @@ -1523,5 +1527,7 @@ "description": "Toggle Maximize Code Review Panel" } ], - "generated_at": "2026-08-26T17:34:04.788992+00:00" -} + "source_repository": "warpdotdev/warp", + "source_sha": "c3c397d6c0c7c2566d0a30329fef46f71b74fa30", + "generated_at": "2026-09-03T18:21:12.785789+00:00" +} \ No newline at end of file diff --git a/.agents/skills/validate_ui_refs/validate_ui_refs.py b/.agents/skills/validate_ui_refs/validate_ui_refs.py index ca03390e5..c7a7cc818 100644 --- a/.agents/skills/validate_ui_refs/validate_ui_refs.py +++ b/.agents/skills/validate_ui_refs/validate_ui_refs.py @@ -1055,6 +1055,56 @@ def scan_docs( return files +class ChangedFilesUnresolvedError(RuntimeError): + """Raised when the changed-file diff against origin/main can't be resolved.""" + + +def find_changed_md_files( + docs_dir: Path, + include_changelog: bool = False, +) -> List[Path]: + """Find .md/.mdx files under docs_dir changed vs origin/main...HEAD. + + Matches `style_lint.py --changed`'s `origin/main...HEAD` semantics, + exclusions, and deleted-file handling (`--diff-filter=d` drops deletions + from the diff so a removed file is never "checked"). Unlike + `style_lint`'s `--changed`, this never falls back to an unbounded full + scan when the diff can't be resolved — required CI must fail loud instead + of silently widening scope. + """ + try: + result = subprocess.run( + [ + "git", "diff", "--name-only", "--diff-filter=d", + "origin/main...HEAD", "--", str(docs_dir), + ], + capture_output=True, text=True, check=True, + ) + except (subprocess.CalledProcessError, OSError) as exc: + raise ChangedFilesUnresolvedError( + f"could not determine changed files vs origin/main...HEAD: {exc}" + ) from exc + + files = [] + for line in result.stdout.strip().splitlines(): + line = line.strip() + if not line or not (line.endswith(".md") or line.endswith(".mdx")): + continue + p = Path(line) + if not p.exists(): + continue + try: + rel_parts = set(p.resolve().relative_to(docs_dir.resolve()).parts) + except ValueError: + continue + if rel_parts & SKIP_DIRS: + continue + if not include_changelog and "changelog" in rel_parts: + continue + files.append(p) + return sorted(files) + + # --------------------------------------------------------------------------- # Auto-fix # --------------------------------------------------------------------------- @@ -1174,6 +1224,21 @@ def _fixes_already_in_pr( def _pr_body(fixes: List[Dict[str, Any]], repo_root: Path) -> str: + # This PR is a real, direct PR-creation code path (not delegated to + # create_pr), so it carries the same v1 doc-quality contract sections + # every other PR-producing skill does. It is mechanically `low` risk: an + # auto-fix only ever changes UI-reference casing/formatting to match an + # already-canonical name in valid_paths.json, never product meaning. + contract = ( + "## Documentation risk\n" + "Risk: low\n" + "Rationale: Mechanical UI-reference casing/formatting fixes only, applied by " + "validate_ui_refs against the committed valid_paths.json snapshot; no product " + "meaning changes.\n" + "Docs override: none\n\n" + "## Unverified claims\n" + "None \u2014 every fix corrects formatting/casing to an already-canonical name." + ) return ( "## Summary\n" f"Auto-fixed {len(fixes)} UI reference issue(s) found by the `validate_ui_refs` skill.\n\n" @@ -1183,7 +1248,7 @@ def _pr_body(fixes: List[Dict[str, Any]], repo_root: Path) -> str: f"`{f['old']}` → `{f['new']}`" for f in fixes ) - + "\n\nCo-Authored-By: Warp " + + f"\n\n{contract}\n\nCo-Authored-By: Warp " ) @@ -1240,7 +1305,7 @@ def _update_existing_pr( body_file = tmp.name try: subprocess.run( - ["gh", "pr", "edit", str(pr_number), "--body-file", body_file], + ["gh", "pr", "edit", str(pr_number), "--body-file", body_file, "--add-label", "warpy-factory"], cwd=repo_root, check=True, ) @@ -1309,6 +1374,7 @@ def create_pr(fixes: List[Dict[str, Any]], repo_root: Path) -> Tuple[Optional[st "--title", f"docs: fix {len(fixes)} UI reference issue(s)", "--body-file", body_file, "--draft", + "--label", "warpy-factory", ], cwd=repo_root, capture_output=True, @@ -1521,6 +1587,8 @@ def refresh_valid_paths(warp_repo_path: Path, output_path: Path) -> None: "macos_menu_bar": existing.get("macos_menu_bar", {}), "warp_drive": existing.get("warp_drive", {}), "command_palette_commands": command_palette, + "source_repository": _resolve_source_repository(warp_repo_path) or existing.get("source_repository"), + "source_sha": _resolve_source_sha(warp_repo_path) or existing.get("source_sha"), "generated_at": datetime.now(timezone.utc).isoformat(), } @@ -1535,6 +1603,39 @@ def refresh_valid_paths(warp_repo_path: Path, output_path: Path) -> None: ) +def _resolve_source_sha(warp_repo_path: Path) -> Optional[str]: + """Return the warp client repo's current commit SHA, or None if unavailable. + + Best-effort: a shallow checkout, a missing `.git`, or any git failure + leaves the field absent rather than raising, since a missing SHA is a + visible "unknown" in the report (see main()'s provenance line) and never + silently advances a fabricated value. + """ + try: + result = subprocess.run( + ["git", "-C", str(warp_repo_path), "rev-parse", "HEAD"], + capture_output=True, text=True, check=True, + ) + except (subprocess.CalledProcessError, OSError): + return None + sha = result.stdout.strip() + return sha or None + + +def _resolve_source_repository(warp_repo_path: Path) -> Optional[str]: + """Return the warp client repo's `owner/repo` from its `origin` remote.""" + try: + result = subprocess.run( + ["git", "-C", str(warp_repo_path), "remote", "get-url", "origin"], + capture_output=True, text=True, check=True, + ) + except (subprocess.CalledProcessError, OSError): + return None + url = result.stdout.strip() + m = re.search(r"[:/]([^/:]+/[^/]+?)(?:\.git)?$", url) + return m.group(1) if m else None + + def _extract_umbrellas(warp_repo: Path) -> Dict[str, Any]: """Parse SettingsUmbrella::new("Label", vec![...]) calls from mod.rs. @@ -2147,6 +2248,17 @@ def main() -> int: parser.add_argument("--slack-notify", action="store_true", help="Post results to Slack") parser.add_argument("--slack-channel", default=DEFAULT_SLACK_CHANNEL, help="Slack channel ID") parser.add_argument("--include-changelog", action="store_true", help="Include changelog/ in scan") + parser.add_argument( + "--changed", action="store_true", + help="Scan only files changed vs origin/main...HEAD (required CI scope; " + "fails rather than falling back to a full scan when the diff can't be resolved)", + ) + parser.add_argument( + "--require-provenance", action="store_true", + help="Fail if the committed snapshot's source_repository/source_sha are missing. A " + "refresh failure must never silently advance provenance, so the required CI gate " + "passes this rather than trusting an unknown client revision.", + ) parser.add_argument("--refresh-valid-paths", action="store_true", help="Re-extract from the warp client repo") parser.add_argument( "--warp", @@ -2197,13 +2309,38 @@ def main() -> int: return 1 valid_paths = load_valid_paths(valid_paths_file) + # Report the snapshot's trusted provenance so every technical-reference + # check states what client state it trusts (see doc-quality-policy.md). + snapshot_repo = valid_paths.get("source_repository") or "unknown" + snapshot_sha = valid_paths.get("source_sha") or "unknown" + snapshot_generated_at = valid_paths.get("generated_at") or "unknown" + print( + f"Trusted snapshot: source={snapshot_repo}@{snapshot_sha} " + f"generated_at={snapshot_generated_at}" + ) + if args.require_provenance and (snapshot_repo == "unknown" or snapshot_sha == "unknown"): + print( + "Error: --require-provenance was set but the committed snapshot has incomplete " + "provenance (source_repository/source_sha). Refresh and commit a verified snapshot " + "before this gate can trust it -- an unknown client revision is not silently accepted.", + file=sys.stderr, + ) + return 1 + # Scan docs docs_dir = Path(args.docs_dir) if not docs_dir.exists(): print(f"Error: docs directory not found at {docs_dir}", file=sys.stderr) return 1 - md_files = scan_docs(docs_dir, include_changelog=args.include_changelog) + if args.changed: + try: + md_files = find_changed_md_files(docs_dir, include_changelog=args.include_changelog) + except ChangedFilesUnresolvedError as exc: + print(f"Error: {exc}", file=sys.stderr) + return 1 + else: + md_files = scan_docs(docs_dir, include_changelog=args.include_changelog) print(f"Scanning {len(md_files)} markdown files...") path_issues = [] diff --git a/.agents/skills/weekly-404-monitor/SKILL.md b/.agents/skills/weekly-404-monitor/SKILL.md index 7fe01625a..992b02b84 100644 --- a/.agents/skills/weekly-404-monitor/SKILL.md +++ b/.agents/skills/weekly-404-monitor/SKILL.md @@ -178,6 +178,7 @@ PR body must include: - The broken URL, hit count, proposed destination, and confidence reason for each redirect - The hit threshold used (`hits_this_week >= N`) - A note that MEDIUM-confidence suggestions are in the Slack message and require human review before adding +- The shared v1 agent-doc quality contract's `warpy-factory` label and `## Documentation risk` section (see `.agents/references/doc-quality-policy.md`; build the block with `.agents/skills/doc_quality_policy/finalize_pr_contract.py build`). A redirect-only change to `vercel.json` adds no product-meaning claim, so it is `low` risk under the allowlist. Run `python3 .agents/skills/check_for_broken_links/check_links.py --internal-only` after editing `vercel.json` to catch any malformed destinations. diff --git a/.agents/specs/GROW-6092-ai-content-quality-v1.md b/.agents/specs/GROW-6092-ai-content-quality-v1.md new file mode 100644 index 000000000..9cbae219c --- /dev/null +++ b/.agents/specs/GROW-6092-ai-content-quality-v1.md @@ -0,0 +1,171 @@ +# Spec: Improve agent-authored technical content + +## Product + +### Summary + +Agent-authored documentation pull requests currently rely on guidance that is not consistently enforced. The v1 quality program adds a balanced scorecard: deterministic editorial and UI-reference checks, an independent `review-docs-pr` pass, explicit technical-risk routing, shared drafting constraints, usable feedback signals, and a 30-day outcome report. + +This scope covers every agent-authored content PR in `warpdotdev/docs`, including `draft_*`, `release_updates`, AEO, `missing_docs`, and other recurring skills that add or change public documentation. It does not apply only to ambient feature drafts. + +### Key design choices + +1. Deterministic checks block objectively detectable defects; agent review and human review own semantic accuracy that cannot be proven by lint. +2. Low risk is a strict allowlist. Any new feature documentation, changed behavioral claim, unverified claim, or sensitive technical surface requests source-owning engineering review by default. The request is advisory; Docs owns the merge decision after its normal review. +3. Pod-Docs owns the intentional v1 rollout. The monthly `improve-drafting-skills` loop becomes the ongoing improvement path only after review and human-feedback signals are flowing. + +### Behavior + +1. Every agent-authored content PR is identifiable by the `warpy-factory` label and a machine-readable documentation-risk section in its PR body. PR-opening skills add both before requesting human review. +2. The editorial half of the scorecard is a required CI check over changed documentation. `style_lint --changed` blocks its error-severity findings; warning-severity tone and judgment findings remain visible to `review-docs-pr`. +3. The technical half of the deterministic scorecard validates changed UI paths, Command Palette names, and UI-reference formatting against the committed `valid_paths.json` snapshot. It also verifies that every new `{/* VERIFY: ... */}` marker is listed in the PR's `Unverified claims` section. +4. The changed-file UI-reference check does not require a `warpdotdev/warp` checkout. CI uses the committed snapshot. The snapshot records the source repository, source commit SHA, and generation time so each check can report what client state it trusts. +5. `.github/workflows/refresh-ui-paths.yml` refreshes the snapshot in three cases: + - `warpdotdev/warp` merges a change under `app/src/settings_view/**` to `master`, and `.github/workflows/notify-docs-settings-changed.yml` sends the docs repository a `repository_dispatch` event of type `settings-ui-changed` with the source SHA. + - A daily scheduled run at `15:15 UTC` compares the snapshot's source SHA with the full `master` head used for snapshot generation. A mismatch executes the same refresh path, limiting an unnoticed missed dispatch to one day. + - A maintainer starts `workflow_dispatch` as the documented fallback. + Each path dispatches the `validate_ui_refs` skill, runs `--refresh-valid-paths` against `warpdotdev/warp`, validates and fixes stale references, and requires live Settings verification for ambiguous shared-source subsections. A dispatch, agent, refresh, or reconciliation failure leaves an observable failed workflow and alerts Pod-Docs; it never records a newer source SHA. +6. An unresolved `VERIFY` marker does not silently pass as low risk. The PR is classified `engineering-review-required`, names the source surface that can resolve the claim, and requests source-owner review. Docs resolves or removes the marker before merging. +7. Every agent-authored content PR receives an independent `review-docs-pr` pass after the deterministic checks and before human review. The pass reviews the final head SHA, publishes actionable findings, and emits one parseable `[SIGNAL:pr-review]` record. +8. `review-docs-pr` blocks on critical or important findings, including a low-risk classification that is inconsistent with the diff. Suggestions and nits remain non-blocking. +9. A PR is `low` risk only when all of these conditions hold: + - It does not add a page about a new or materially changed feature or workflow. + - It either preserves product meaning and changes only spelling, grammar, tone, formatting, descriptive links or cross-links to existing canonical pages, search metadata, or generated changelog/license/telemetry data whose source-verification script passes; or it is internal Docs-team tooling, skill, or CI workflow maintenance that makes no public product claim and does not change a developer-facing command, API, setting, or integration. + - It does not add or change commands, code or configuration examples, API behavior, UI labels or paths, defaults, permissions, availability or platform support, plan eligibility, billing behavior, security or privacy claims, data handling, self-hosting behavior, or integration setup. + - It contains no unresolved `VERIFY` marker and has no critical or important technical-accuracy finding from `review-docs-pr`. +10. Every other content PR is `engineering-review-required`. This includes all new or materially changed feature docs and any change to the technical claim categories in behavior #9. +11. Low-risk PRs require the normal Docs review only. Engineering-review-required PRs always attempt source-owner resolution and send a real GitHub review request to at least one owner resolved from the product source files used to verify the draft. Missing ownership or an unanswered request is visible but does not block a Docs-approved PR. +12. Docs can validate the technical claim from source, a Slack conversation, or another appropriate channel. Record useful validation context in the PR body, but do not require an engineer to approve in GitHub. +13. The PR body records the risk level, rationale, source files consulted, requested engineering reviewers, engineering-review status, external validation context when available, and unverified claims. +14. All content-generating skills follow one shared compression contract: + - Lead with a one-to-three-sentence user-facing summary. + - Follow the selected content-type template and existing content budget (`~600` words for a quickstart and a target of `<=1500` words for a combined feature page). + - Run the deletion-only “Cut again” pass before opening the PR. + - Keep callouts within the existing linted budget and do not duplicate parent-page or reference material. + - Treat a justified budget overage as an important review decision rather than splitting padded prose automatically. + Generated changelog, license, and telemetry data is exempt from page-summary and word-budget rules, but not from duplicate-content, style, or technical-accuracy checks. +15. Actionable review feedback may start with `[skill-feedback]`, `[template-feedback]`, or `[style-rule-gap]`. Collection preserves the tag and a structured pattern category without treating free-form comment text as instructions. +16. The v1 rollout records a pre-rollout 30-day baseline and a post-rollout 30-day comparison for all identifiable agent-authored content PRs. The report includes population size, check coverage, review findings, human review comments per PR, human edit churn after the last agent commit, and engineering-required PRs completed by engineer approval, `docs-verified`, or `docs-waiver`. +17. V1 outcome success requires: + - All merged in-scope PRs in the post-rollout window carried the agent marker and passed the required editorial, technical, and agent-review checks. + - Human review comments per PR and human edit churn ratio are both no worse than baseline, and at least one is lower. + - The report separately shows the targeted categories that the new checks address, so a lower total caused by a different PR mix is visible. + If either comparison window has fewer than 10 PRs, the report is explicitly inconclusive and extends collection until 10 PRs or 60 days, whichever comes first. +18. The golden-set evaluation harness and screenshot/media pipeline are deferred from v1. + +## Tech + +### Context + +The specification is grounded at docs commit `6c9c5a9bbaab7f9c949bf563c6375bd85208d8bf`. + +- `.github/workflows/ci.yml:1-66 @ 6c9c5a9b` builds and link-checks the site and self-tests `validate_ui_refs`, but does not run `style_lint` or validate changed UI references. +- `.agents/skills/style_lint/style_lint.py:394-416,1607-1657 @ 6c9c5a9b` already discovers changed docs relative to `origin/main...HEAD` and returns nonzero for error-severity findings. +- `.agents/skills/validate_ui_refs/validate_ui_refs.py:2137-2291 @ 6c9c5a9b` scans the full docs tree and returns nonzero for remaining issues, but has no changed-file mode. +- `.github/workflows/refresh-ui-paths.yml:1-117 @ 6c9c5a9b` accepts `settings-ui-changed` and manual dispatches, refreshes `valid_paths.json`, fixes stale references, and fails if the dispatched agent does not succeed. It has no scheduled reconciliation or committed source-SHA provenance. +- `warpdotdev/warp/.github/workflows/notify-docs-settings-changed.yml:1-57 @ d6207523` sends `settings-ui-changed` with the merged client SHA when `master` changes `app/src/settings_view/**`; its failure path identifies the manual docs workflow fallback. +- `.agents/skills/draft_docs/SKILL.md:131-253 @ 6c9c5a9b` defines source verification, `VERIFY` markers, the “Cut again” pass, style lint, and the required `Unverified claims` PR section. +- `.agents/skills/create_pr/SKILL.md:183-515 @ 6c9c5a9b` validates PR bodies and requests reviewers, but does not classify documentation risk or prove that a technical owner approved. +- `.agents/skills/review-docs-pr/SKILL.md:15-137 @ 6c9c5a9b` defines the review scorecard and signal marker, but only runs when explicitly invoked. +- `.agents/skills/improve-drafting-skills/SKILL.md:55-229 @ 6c9c5a9b` collects automated and human signals and defines action thresholds. `.agents/logs/pr_review_runs.md:1-56 @ 6c9c5a9b` records that parseable review signals remain sparse. +- `.github/workflows/release-docs-update.yml:1-282 @ 6c9c5a9b` is an example of a generator-specific PR and reviewer path that must converge on the shared contract rather than remaining an exception. + +### Design alternatives + +- **Run full-repository lint on every PR vs. changed-file gates.** Full scans maximize coverage but make unrelated historical debt block new work. V1 uses changed-file gates and leaves full scans to scheduled audits. +- **Require a live client checkout in docs CI vs. use the committed UI snapshot.** A live checkout is fresher but adds cross-repo credentials, latency, and availability failures. V1 uses the committed snapshot for PR checks and preserves the existing source-driven refresh workflow. +- **Treat every agent PR as engineering-review-required vs. use a risk allowlist.** Universal engineering review is simpler but does not meet the goal of docs-only review for safe edits. V1 uses a narrow low-risk allowlist and defaults every ambiguous case to engineering review. +- **Block on source-owner approval vs. request it without blocking.** A strict source-owner gate makes Docs delivery depend on ownership resolution and reviewer availability. V1 always solicits source-owner review, but Docs owns the merge decision after its normal review and can validate through source, Slack, or another appropriate channel. +- **Let each generator implement its own quality flow vs. centralize the contract.** Per-skill instructions drift and caused the current uneven coverage. V1 centralizes machine checks and PR metadata, then makes every PR-producing skill call the shared finalization path. +- **Use self-review in the authoring run vs. dispatch an independent reviewer.** Self-review is cheaper but correlates errors with the draft. V1 runs `review-docs-pr` in a separate workflow-triggered agent pass pinned to the PR head SHA. +- **Adopt a golden-set threshold now vs. measure live PR outcomes first.** A golden set is a frozen set of representative prompts/pages and expected factual claims, with a threshold such as a minimum pass rate and zero critical factual errors. It is useful for repeatable pre-release comparisons, but it needs curated fixtures and ongoing maintenance. V1 defers it until live gates and feedback produce enough evidence to choose representative cases. + +### Proposed changes + +#### 1. Establish the shared PR contract and baseline + +1. Add a single agent-doc quality policy and helper under `.agents/` that owns: + - The canonical agent marker (`warpy-factory`). + - The `low` and `engineering-review-required` values. + - Validation of the `## Documentation risk` and `## Unverified claims` PR-body sections. + - Verification that all `VERIFY` markers in changed content are listed and force engineering review. + - A manifest of PR-producing content skills and tests that fail when a new PR-producing skill bypasses the shared finalization contract. +2. Update `create_pr` and direct PR-producing paths, including `release_updates`, AEO, and `missing_docs`, to open or keep the PR as draft, apply the marker, write the risk block, and use the shared finalizer before notifying reviewers. +3. Capture the 30 days immediately preceding rollout as the immutable baseline using the metric definitions in increment 4. + +#### 2. Add required deterministic scorecard checks + +1. Add separately named CI jobs: + - `Docs editorial quality`: `python3 .agents/skills/style_lint/style_lint.py --changed`. + - `Docs technical references`: changed-file `validate_ui_refs` plus PR-contract and `VERIFY` validation. +2. Add `--changed` to `validate_ui_refs.py`, matching `style_lint`'s `origin/main...HEAD` semantics, exclusions, deleted-file handling, and explicit failure if the base diff cannot be determined. Do not fall back to an unbounded full scan in required CI. +3. Extend `valid_paths.json` with `source_repository`, `source_sha`, and `generated_at`. `validate_ui_refs` preserves the previous provenance on failure and prints the trusted source SHA and snapshot age in `Docs technical references`. +4. Extend `.github/workflows/refresh-ui-paths.yml` with the daily `15 15 * * *` reconciliation. Repository, scheduled, and manual triggers all resolve the same full `master` HEAD SHA used for snapshot generation, invoke the same refresh command, and use the existing fix/create-PR flow. Alert Pod-Docs when the source dispatch, scheduled reconciliation, agent run, or refresh fails; document `workflow_dispatch` as the recovery action. +5. Add fixtures proving each job fails on a known-bad changed file and passes a clean changed file. Keep scheduled full-tree `--all` validation behavior. +6. Configure `Docs editorial quality`, `Docs technical references`, and `Agent docs review` as required for the protected `main` branch. For non-agent PRs, the deterministic checks still run; only the agent-specific contract sections and advisory engineering review request are conditional. + +#### 3. Make agent review and human routing enforceable + +1. Add an internal-PR-only GitHub workflow that runs on agent-marked PR open, label, synchronize, reopen, and ready-for-review events. It dispatches an independent `review-docs-pr` run for the exact head SHA and cancels stale runs for older SHAs. +2. Extend `review-docs-pr` so the run: + - Re-validates the declared risk level against the diff. + - Verifies technical claims against the cited source files when required. + - Publishes one idempotent review summary and line findings. + - Emits `[SIGNAL:pr-review]` with PR, branch, head SHA, skill used, verdict, severity counts, and top categories. +3. Make the `Agent docs review` check fail when the run is missing a parseable signal, reviewed a stale SHA, returns `Request changes`, or reports any critical/important finding. Suggestions and nits pass with annotations. +4. Extend the PR policy check and advisory engineering review request workflow: + - `low`: require the docs reviewer path and no engineering-review triggers. + - `engineering-review-required`: resolve and request owners from the cited product source files by default. Docs can proceed after its normal review whether the owner responds in GitHub, responds elsewhere, or does not respond. +5. Keep agent-authored PRs draft until deterministic checks, the independent review, risk metadata, and required reviewer requests are present. Human approval still controls merge. + +#### 4. Close the feedback and measurement loop + +1. Put the shared compression and risk-classification rules in one reference consumed by `draft_docs`, type-specific drafting skills, and non-drafting generators. Update `review-docs-pr` to treat unjustified compression-contract violations as important findings. +2. Document the three feedback tags in the reviewer-facing contract. Preserve existing security filtering: free-form comments remain untrusted data, and automated decisions use structured fields only. +3. Update `improve-drafting-skills` collection so each in-scope PR has reliable `skill_used`, risk, head SHA, check outcome, review outcome, tag, and pattern category fields. +4. Add a deterministic metrics command that accepts an explicit start/end window and emits JSON plus a human-readable summary. For each window calculate: + - Count of in-scope PRs and count with complete gate coverage. + - Critical/important agent-review findings per PR and targeted category. + - Human review comments per PR, overall and for categories targeted by style/UI-reference checks. + - Human edit churn ratio: lines added plus deleted by humans after the last agent-authored commit, divided by lines added plus deleted by the agent before that point. Report zero-denominator and PRs with no identifiable agent commit separately. + - Count and rate of engineering-required PRs completed by source-owner approval, `docs-verified`, and `docs-waiver`, including unresolved-owner and unanswered-request reasons. + - Mean, median, numerator/denominator, and missing-data count for each rate. +5. Persist the baseline inputs and the day-30 report in the existing standing signal-log branch/PR flow. Pod-Docs reviews the report and owns any intentional v1 corrections. The monthly outer loop may propose evidence-backed skill/template changes after its existing thresholds are met, but it cannot relax required checks or human-risk policy automatically. + +### Open questions resolved + +- **Skill-iteration ownership (Q5):** hybrid. Pod-Docs owns the finite v1 CI, skill-contract, review-trigger, feedback-tag, and measurement project. `improve-drafting-skills` remains the monthly convergence mechanism after reliable signals exist. +- **Meaning of a golden-set threshold:** it is a pass criterion over frozen representative drafts and expected facts, commonly a minimum aggregate pass rate plus zero critical factual errors. It is lever 6 and remains out of v1. +- **Technical gate without a client checkout:** use the committed `valid_paths.json` snapshot for PR validation; refresh that snapshot separately from the public client source. +- **Snapshot refresh reliability:** keep the existing source-driven `settings-ui-changed` dispatch, add daily source-SHA reconciliation to recover missed events within one day, preserve manual `workflow_dispatch`, and expose provenance and failed refreshes instead of silently trusting or advancing stale data. +- **Ambiguous risk classification:** default to `engineering-review-required`. Low risk is never inferred from the absence of an obvious error. +- **Engineering review availability:** solicit a source-owning engineer by default. The request is advisory; Docs can proceed after its normal review when an engineer responds elsewhere or does not respond. +- **Screenshot requirements in the drafting contract:** keep the existing authoring guidance, but do not build or gate on a new screenshot pipeline in this issue. That remains lever 7 / GROW-6091. +- **Outcome threshold:** report both human-comment and human-edit metrics; neither may regress and at least one must decline, with an explicit minimum-sample rule. No statistical-significance claim is required for v1. + +## Validation and verification criteria + +All criteria must pass before v1 is considered complete. + +1. **Agent PR coverage:** Unit tests enumerate every docs-content PR-producing skill/script and fail if it does not invoke the shared finalizer. Representative PR fixtures for `draft_feature_doc`, `release_updates`, AEO, and `missing_docs` contain the `warpy-factory` marker, risk block, unverified-claims block, and skill attribution. +2. **Editorial gate fails before and passes after:** A fixture branch adding a known `style_lint` error to a changed `.mdx` file makes `python3 .agents/skills/style_lint/style_lint.py --changed` exit nonzero. Correcting the fixture makes it exit zero. Existing style-lint unit suites pass. +3. **Technical changed-file scope:** New `validate_ui_refs` tests prove `--changed` scans changed `.md`/`.mdx` files, ignores unchanged historical debt and deleted files, excludes changelog unless explicitly requested, and fails rather than silently widening scope when the merge-base diff cannot be resolved. +4. **Technical gate fails before and passes after:** A changed-file fixture with an invalid Settings path and Command Palette name makes the validator exit nonzero; canonical references make it exit zero. The job output includes snapshot age and source SHA. `python3 .agents/skills/validate_ui_refs/validate_ui_refs.py --self-test` also passes. +5. **Snapshot trigger and provenance:** Workflow tests prove a `settings-ui-changed` payload, the daily `15 15 * * *` schedule, and manual dispatch all execute the same refresh path. A successful refresh records the tested `warpdotdev/warp` source SHA and generation time. A no-change daily reconciliation exits successfully only after proving that the recorded SHA matches the latest `master` commit affecting `app/src/settings_view/**`. +6. **Snapshot failure recovery:** Event fixtures prove a missing dispatch payload, inaccessible client checkout, failed agent run, refresh error, or source-SHA mismatch produces a visible failed workflow and Pod-Docs alert without advancing provenance. A manual-dispatch fixture recovers the failure and refreshes the snapshot. +7. **VERIFY accounting:** Contract tests prove an unlisted `VERIFY` marker fails, a listed marker with `low` risk fails, and a listed marker with `engineering-review-required` remains visible for Docs to resolve before merging. +8. **Risk allowlist:** Table-driven tests cover every low-risk category and every engineering-review trigger in product behaviors #9-10. Unknown/ambiguous fixtures resolve to `engineering-review-required`. +9. **Independent review on final SHA:** An agent-marked test PR triggers `review-docs-pr`, publishes exactly one current review summary, and emits one parseable signal containing the tested head SHA. Pushing a new commit cancels/obsoletes the old check and requires a new signal for the new SHA. +10. **Review severity behavior:** Seeded review fixtures prove critical/important findings and risk misclassification fail `Agent docs review`; suggestion/nit-only results pass and remain visible. +11. **Low-risk human path:** A low-risk editorial test PR with clean gates can become ready with the normal docs reviewer and without an engineering owner approval. +12. **Engineering review request:** A test PR that changes a command, API/config example, UI/default/permission/availability claim, or new-feature page sends a real review request to a resolved source owner. +13. **Docs-owned merge decision:** The same PR can proceed through normal Docs review when the engineer approves in GitHub, validates through another channel, or does not respond. +14. **Recorded validation context:** A Docs reviewer can add source evidence or a `docs-verified`/`docs-waiver` note to the PR body without requiring a matching GitHub approval. +15. **Review-request boundaries:** Fixtures prove review-request failures are visible but do not override deterministic CI or an unresolved critical/important independent-review finding. +16. **Compression contract:** Before/after fixtures for a feature page, an AEO or `missing_docs` page, and a release-generated update prove the shared summary, budget, callout, and deletion-pass rules apply as specified, including the generated-data exemptions. +17. **Signal hygiene:** Collector tests accept the three approved tags, preserve structured `pattern_category`, reject bot/injection records under the existing security rules, and produce non-empty `skill_used`, risk, and head SHA for the representative generator fixtures. +18. **Baseline reproducibility:** Running the metrics command twice over the frozen pre-rollout dates yields byte-equivalent normalized JSON. The output includes counts, means, medians, numerators/denominators, missing-data reasons, and advisory engineering-review request outcomes. +19. **Day-30 outcome:** The post-rollout command evaluates the success rule in product behavior #17 and produces one of `pass`, `fail`, or `inconclusive-small-sample`; the small-sample fixture extends collection as specified rather than claiming success. +20. **Repository checks:** Run `npm run fmt`, `npm run lint`, `npm run typecheck`, `npm run build`, `python3 .agents/skills/check_for_broken_links/check_links.py --internal-only`, every new/changed Python unit test, the existing style-lint unit suites, the `validate_ui_refs --self-test`, and the existing create-PR body/reviewer tests. All pass. +21. **Workflow checks:** Validate each changed GitHub Actions workflow with the repository's configured formatter/linter and exercise its decision logic with event fixtures for a human PR, a low-risk agent PR, a high-risk agent PR, a fork PR, a stale head, and a failed reviewer run. Fork PRs never receive secrets or dispatch privileged agent work. +22. **No visual-verification requirement:** This change modifies headless docs generation, review, and CI behavior only. It is testing-exempt from `computer_use`; if implementation later changes rendered public docs or another user-facing UI, that added surface requires separate visual verification. diff --git a/.github/workflows/agent-docs-review.yml b/.github/workflows/agent-docs-review.yml new file mode 100644 index 000000000..b4c5e7672 --- /dev/null +++ b/.github/workflows/agent-docs-review.yml @@ -0,0 +1,71 @@ +name: Agent docs review + +# Internal-PR-only: dispatches an independent review-docs-pr pass for every +# agent-marked PR (the `warpy-factory` label), pinned to the exact head SHA. +# A fork PR never receives secrets or dispatches agent work, since +# `pull_request` (not `pull_request_target`) runs with the fork's own +# read-only token and no repository secrets. +on: + pull_request: + types: [opened, labeled, synchronize, reopened, ready_for_review] + +permissions: + contents: read + pull-requests: write + +concurrency: + # One review run per PR; a new push (synchronize) cancels the stale-SHA + # run in progress rather than letting two reviews race on the same PR. + group: agent-docs-review-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + review: + name: Dispatch review-docs-pr for the current head + runs-on: ubuntu-latest + # Only agent-marked PRs get the independent review pass, and only from + # this same repository (never a fork, which carries no secrets here). + if: | + contains(github.event.pull_request.labels.*.name, 'warpy-factory') && + github.event.pull_request.head.repo.full_name == github.repository + steps: + - name: Checkout docs + uses: actions/checkout@v4 + + - name: Run independent review-docs-pr agent + id: oz-review + uses: warpdotdev/oz-agent-action@main + with: + warp_api_key: ${{ secrets.WARP_API_KEY }} + profile: ${{ vars.WARP_AGENT_PROFILE || '' }} + prompt: | + Run the review-docs-pr skill against warpdotdev/docs PR #${{ github.event.pull_request.number }} + at head SHA ${{ github.event.pull_request.head.sha }}. This PR carries the warpy-factory agent + marker, so it requires the independent v1 agent-doc quality review pass (see + .agents/references/doc-quality-policy.md): + + 1. Re-validate the declared "## Documentation risk" level against the actual diff using + the low-risk allowlist. A risk misclassification is an important finding. + 2. Verify technical claims against cited source files when the PR is + engineering-review-required. + 3. Publish the review, pinned to this exact head SHA. Request changes on any + critical/important finding; suggestions/nits pass. + 4. Publish exactly one GitHub review pinned to this head SHA, using the procedure in + review-docs-pr/SKILL.md. Include exactly one [SIGNAL:pr-review] JSON record in + both the review body and final response. The signal must include your authenticated + GitHub reviewer_login and this head SHA. + + # A successful agent action is not proof that the run reviewed this + # exact head or passed: require its one structured signal and an + # authoritative current GitHub review. + - name: Verify the current review signal and GitHub review + env: + GH_TOKEN: ${{ github.token }} + AGENT_OUTPUT: ${{ steps.oz-review.outputs.agent_output }} + run: | + printf '%s' "$AGENT_OUTPUT" > /tmp/agent-output.txt + python3 .agents/skills/doc_quality_policy/verify_review_signal.py \ + --repo "${{ github.repository }}" \ + --pr "${{ github.event.pull_request.number }}" \ + --head-sha "${{ github.event.pull_request.head.sha }}" \ + --agent-output /tmp/agent-output.txt diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d997a77c4..fc10dec42 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,6 +2,12 @@ name: CI on: pull_request: + # Non-agent PRs are exempt from the agent-contract sections (see the + # "Check documentation-risk PR contract" step below), so the default + # trigger types are not enough on their own. `edited` re-runs the check + # when the finalizer repairs the PR body without pushing a new commit; + # `labeled` re-runs it when `warpy-factory` is applied after PR open. + types: [opened, synchronize, reopened, ready_for_review, edited, labeled] push: branches: [main] @@ -9,6 +15,82 @@ permissions: contents: read jobs: + docs-editorial-quality: + name: Docs editorial quality + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.12' + + # Changed-file-only style lint. Error-severity findings block; warning- + # severity tone/judgment findings remain visible to review-docs-pr. + - name: Run style_lint --changed + run: python3 .agents/skills/style_lint/style_lint.py --changed + + docs-technical-references: + name: Docs technical references + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.12' + + # Validates changed UI paths, Command Palette names, and UI-reference + # formatting against the committed valid_paths.json snapshot. Uses the + # committed snapshot only — no warpdotdev/warp checkout required. + - name: Validate changed UI references + run: python3 .agents/skills/validate_ui_refs/validate_ui_refs.py --changed --require-provenance + + # Doc-quality policy unit tests (risk allowlist, VERIFY accounting, + # override boundaries, PR-contract validation). + - name: Test doc_quality_policy + run: | + python3 .agents/skills/doc_quality_policy/test_policy.py + python3 .agents/skills/doc_quality_policy/test_check_pr_contract.py + python3 .agents/skills/doc_quality_policy/test_check_compression_contract.py + python3 .agents/skills/doc_quality_policy/test_verify_review_signal.py + python3 .agents/skills/doc_quality_policy/test_manifest.py + + - name: Test validate_ui_refs --changed mode and snapshot provenance + run: python3 .agents/skills/validate_ui_refs/test_changed_mode.py + + - name: Test improve-drafting-skills metrics command + run: | + python3 .agents/skills/improve-drafting-skills/scripts/test_compute_metrics.py + python3 .agents/skills/improve-drafting-skills/scripts/test_build_baseline_records.py + + # PR-contract check: unlisted VERIFY markers and missing/invalid risk + # metadata. Only agent-marked PRs carry this contract (see + # doc-quality-policy.md) — an ordinary human docs PR has no + # "## Documentation risk" section and must not be failed for lacking + # one, so this step is scoped to `warpy-factory` PRs. The human + # engineering gate is intentionally not evaluated on each push: a + # pending approval is expected while a PR is in review. A push-to-main + # run (no open PR) also has nothing to check. + - name: Check documentation-risk PR contract + if: >- + github.event_name == 'pull_request' && + contains(github.event.pull_request.labels.*.name, 'warpy-factory') + env: + PR_BODY: ${{ github.event.pull_request.body }} + run: | + printf '%s' "$PR_BODY" > /tmp/pr-body.md + python3 .agents/skills/doc_quality_policy/check_pr_contract.py \ + --body /tmp/pr-body.md + build: name: Build, link-check, audit runs-on: ubuntu-latest diff --git a/.github/workflows/docs-engineering-approval.yml b/.github/workflows/docs-engineering-approval.yml new file mode 100644 index 000000000..e69de29bb diff --git a/.github/workflows/refresh-ui-paths.yml b/.github/workflows/refresh-ui-paths.yml index 96e5c03d7..78629d6d1 100644 --- a/.github/workflows/refresh-ui-paths.yml +++ b/.github/workflows/refresh-ui-paths.yml @@ -1,16 +1,26 @@ name: Refresh UI paths snapshot -# Triggered automatically when the warp client's Settings UI files change -# (via repository_dispatch from warpdotdev/warp) or manually via -# workflow_dispatch as a fallback. +# Triggered in three ways: +# 1. `repository_dispatch` from warpdotdev/warp when `master` merges a +# change under app/src/settings_view/** (source-driven, same-day). +# 2. A daily `15:15 UTC` schedule that reconciles the snapshot's recorded +# `source_sha` against the full `master` head used to generate the +# snapshot, limiting an unnoticed missed dispatch to one day. A no-change +# reconciliation exits successfully without refreshing. +# 3. `workflow_dispatch` as the documented manual fallback. on: repository_dispatch: types: - settings-ui-changed + schedule: + - cron: '15 15 * * *' workflow_dispatch: # Only WARP_API_KEY is needed here — warpdotdev/warp access and Slack -# notifications are handled by the Oz cloud agent environment. +# notifications are handled by the Oz cloud agent environment. The +# reconciliation step additionally needs read access to warpdotdev/warp's +# commit history, which GH_TOKEN (the default workflow token) covers for a +# public repo. permissions: contents: read @@ -22,9 +32,49 @@ concurrency: cancel-in-progress: false jobs: + reconcile: + name: Reconcile recorded source SHA (scheduled runs only) + if: github.event_name == 'schedule' + runs-on: ubuntu-latest + outputs: + needs_refresh: ${{ steps.compare.outputs.needs_refresh }} + steps: + - name: Checkout docs + uses: actions/checkout@v4 + + - name: Compare recorded source SHA against current master head + id: compare + env: + GH_TOKEN: ${{ github.token }} + run: | + set -euo pipefail + RECORDED_SHA=$(python3 -c "import json; print(json.load(open('.agents/skills/validate_ui_refs/valid_paths.json')).get('source_sha') or '')") + LATEST_SHA=$(gh api "repos/warpdotdev/warp/commits/master" --jq .sha) + if [ -z "$LATEST_SHA" ]; then + echo "::error::could not resolve warpdotdev/warp master HEAD — failing loud rather than silently skipping reconciliation." + exit 1 + fi + echo "Recorded source SHA: ${RECORDED_SHA:-}" + echo "Current master HEAD: $LATEST_SHA" + if [ "$RECORDED_SHA" = "$LATEST_SHA" ]; then + echo "Snapshot is already current — no refresh needed." + echo "needs_refresh=false" >> "$GITHUB_OUTPUT" + else + echo "needs_refresh=true" >> "$GITHUB_OUTPUT" + fi + refresh: name: Dispatch Oz cloud agent to refresh UI path snapshot runs-on: ubuntu-latest + needs: [reconcile] + # Always run for a source dispatch or manual trigger. For the daily + # schedule, run only when reconciliation found the snapshot stale — a + # skipped `reconcile` job's output is empty, not "false", so the schedule + # branch checks needs_refresh explicitly rather than relying on != 'false'. + if: | + always() && + needs.reconcile.result != 'failure' && + (github.event_name != 'schedule' || needs.reconcile.outputs.needs_refresh == 'true') steps: - name: Checkout docs uses: actions/checkout@v4 @@ -43,9 +93,13 @@ jobs: Steps: 1. Locate warpdotdev/warp. It should be checked out at - /workspace/warp in this environment. If that path does - not exist, clone it: - git clone git@github.com:warpdotdev/warp.git /workspace/warp + /workspace/warp in this environment. Ensure it is at the current + origin/master HEAD before refreshing. If that path does not exist, + clone it: + git clone --branch master --depth 1 git@github.com:warpdotdev/warp.git /workspace/warp + Otherwise run: + git -C /workspace/warp fetch origin master + git -C /workspace/warp checkout --detach origin/master Then run: python3 .agents/skills/validate_ui_refs/validate_ui_refs.py \\ --refresh-valid-paths \\ @@ -115,3 +169,47 @@ jobs: exit 1 fi echo "Oz run completed successfully." + + alert-on-failure: + name: Alert Pod-Docs on a failed refresh + runs-on: ubuntu-latest + needs: [reconcile, refresh] + # A dispatch, agent, refresh, or reconciliation failure must be visible + # and alerted — it never silently advances the recorded source SHA. + # `always()` is required because a failed dependency otherwise skips + # this job entirely. + if: always() && (needs.reconcile.result == 'failure' || needs.refresh.result == 'failure') + steps: + - name: Post failure alert to #growth-docs + env: + BUZZ_SLACK_TOKEN: ${{ secrets.BUZZ_SLACK_TOKEN }} + GROWTH_DOCS_SLACK_CHANNEL_ID: ${{ secrets.GROWTH_DOCS_SLACK_CHANNEL_ID }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + EVENT_NAME: ${{ github.event_name }} + run: | + if [ -z "$BUZZ_SLACK_TOKEN" ] || [ -z "$GROWTH_DOCS_SLACK_CHANNEL_ID" ]; then + echo "::warning::BUZZ_SLACK_TOKEN or GROWTH_DOCS_SLACK_CHANNEL_ID not set — skipping Slack alert. Failed workflow run: $RUN_URL" + exit 0 + fi + python3 - <." + ) + payload = json.dumps({"channel": channel, "text": message}).encode() + req = urllib.request.Request( + "https://slack.com/api/chat.postMessage", + data=payload, + headers={"Authorization": f"Bearer {token}", "Content-Type": "application/json"}, + ) + with urllib.request.urlopen(req) as resp: + result = json.load(resp) + if not result.get("ok"): + raise SystemExit(f"Slack error: {result.get('error')}") + PYEOF diff --git a/.github/workflows/request-engineering-review.yml b/.github/workflows/request-engineering-review.yml new file mode 100644 index 000000000..60485470b --- /dev/null +++ b/.github/workflows/request-engineering-review.yml @@ -0,0 +1,50 @@ +name: Request engineering review + +# Technical documentation PRs request source-owner review, but a missing +# GitHub approval does not block Docs from merging after its normal review. +on: + pull_request: + types: [opened, labeled, synchronize, reopened, ready_for_review, edited] + +permissions: + contents: read + pull-requests: write + +jobs: + request-engineering-review: + name: Request engineering review + runs-on: ubuntu-latest + if: >- + contains(github.event.pull_request.labels.*.name, 'warpy-factory') && + github.event.pull_request.head.repo.full_name == github.repository + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Request listed source owners for technical documentation + env: + GH_TOKEN: ${{ github.token }} + PR_BODY: ${{ github.event.pull_request.body }} + PR_NUMBER: ${{ github.event.pull_request.number }} + REPOSITORY: ${{ github.repository }} + run: | + printf '%s' "$PR_BODY" > /tmp/pr-body.md + python3 - <<'PY' > /tmp/requested-engineers.txt + import sys + from pathlib import Path + + sys.path.insert(0, ".agents/skills/doc_quality_policy") + import policy + + risk = policy.parse_documentation_risk_section( + Path("/tmp/pr-body.md").read_text(encoding="utf-8") + ) + if risk and risk.risk == policy.RISK_ENGINEERING_REVIEW_REQUIRED: + print("\n".join(risk.requested_engineering_reviewers)) + PY + while IFS= read -r reviewer; do + [ -z "$reviewer" ] && continue + if ! gh pr edit "$PR_NUMBER" --repo "$REPOSITORY" --add-reviewer "$reviewer"; then + echo "::warning::Could not request engineering review from $reviewer." + fi + done < /tmp/requested-engineers.txt