From 3f7a2b9e2c204ac0ae2cc070860db18d5977abfa Mon Sep 17 00:00:00 2001 From: Jay Flowers Date: Sun, 3 May 2026 14:36:20 -0400 Subject: [PATCH] =?UTF-8?q?docs:=20add=20/review-pr=20CI=20causality=20blo?= =?UTF-8?q?g=20post=20=E2=80=94=20separating=20signal=20from=20noise?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Write blog post with BA-001 narrative arc: CI triage tax problem, causality classification table (PR-caused vs pre-existing), fix-it- forward pattern with dirty-tree guard, 15-comment cap for in-line PR comments, and review lifecycle timeline (/review-council + /review-pr) - Create OpenSpec artifacts with spec review and code review passed --- .../learnings/blog-review-pr-causality-1.md | 9 ++ content/blog/review-pr-causality.md | 120 ++++++++++++++++++ .../blog-review-pr-causality/.openspec.yaml | 2 + .../blog-review-pr-causality/design.md | 46 +++++++ .../blog-review-pr-causality/proposal.md | 57 +++++++++ .../specs/blog-post.md | 62 +++++++++ .../changes/blog-review-pr-causality/tasks.md | 31 +++++ 7 files changed, 327 insertions(+) create mode 100644 .uf/dewey/learnings/blog-review-pr-causality-1.md create mode 100644 content/blog/review-pr-causality.md create mode 100644 openspec/changes/blog-review-pr-causality/.openspec.yaml create mode 100644 openspec/changes/blog-review-pr-causality/design.md create mode 100644 openspec/changes/blog-review-pr-causality/proposal.md create mode 100644 openspec/changes/blog-review-pr-causality/specs/blog-post.md create mode 100644 openspec/changes/blog-review-pr-causality/tasks.md diff --git a/.uf/dewey/learnings/blog-review-pr-causality-1.md b/.uf/dewey/learnings/blog-review-pr-causality-1.md new file mode 100644 index 0000000..cf136cc --- /dev/null +++ b/.uf/dewey/learnings/blog-review-pr-causality-1.md @@ -0,0 +1,9 @@ +--- +tag: blog-review-pr-causality +category: reference +created_at: 2026-05-03T18:08:30Z +identity: blog-review-pr-causality-1 +tier: draft +--- + +When writing blog posts about /review-pr for the Unbound Force website, the CI causality classification uses a 2x2 matrix based on base branch status and PR check status, with three classifications: PR-caused (base pass, PR fail), pre-existing (base fail, PR fail), and unknown (no base data, PR fail — treated as PR-caused conservatively). The fix branch naming pattern is fix/pr-NUMBER-sanitized-check-name. The in-line comment cap is exactly 15 comments, with CRITICAL prioritized over HIGH when the cap is exceeded. The dirty-tree guard checks git status --porcelain before creating fix branches. All of these details are verified from the upstream command file at .opencode/command/review-pr.md lines 132-139, 428-496, and 519-523. diff --git a/content/blog/review-pr-causality.md b/content/blog/review-pr-causality.md new file mode 100644 index 0000000..11240b9 --- /dev/null +++ b/content/blog/review-pr-causality.md @@ -0,0 +1,120 @@ +--- +title: "Your CI Failed — But Was It Your Fault? How /review-pr Separates Signal from Noise" +description: "When CI fails on a pull request, the first question is: did my changes cause this? /review-pr classifies each failure as PR-caused or pre-existing, then offers to fix the pre-existing ones for you." +lead: "Not your fault? Not your problem. One command classifies CI failures, separates your regressions from pre-existing noise, and offers fix branches for failures that predate your PR." +slug: "review-pr-causality" +date: 2026-05-03T00:00:00+00:00 +draft: false +weight: 60 +toc: true +categories: ["Engineering"] +tags: ["review", "ci", "causality", "pull-request"] +contributors: ["Unbound Force"] +--- + +## The Problem + +You open a pull request. CI runs. Two checks fail. You spend 30 minutes investigating, only to discover that both failures predate your changes — a flaky integration test and a lint rule that was added to the base branch after you created your feature branch. + +This is the CI triage tax. Every developer pays it, and it scales with the number of CI checks. The more comprehensive your CI pipeline, the more noise you sift through on every PR. Flaky tests, pre-existing lint violations, stale dependency warnings, and intermittent network timeouts create a background hum of failures that have nothing to do with the code you wrote. + +The worst outcome is not wasted time — it is learned helplessness. After enough false alarms, developers start ignoring CI failures entirely. "It's probably pre-existing" becomes the default assumption, and genuine regressions slip through alongside the noise. + +## The Solution: CI Causality Analysis + +`/review-pr` fetches your PR's CI results and answers the first question every developer asks: *did my changes cause this failure?* + +```text +/review-pr # auto-detect PR from current branch +/review-pr 42 # review a specific PR by number +``` + +For each failing check, the command fetches the same check's status on the base branch and classifies the failure: + +| Base Branch | PR Check | Classification | +|-------------|----------|----------------| +| Pass | Fail | **PR-caused** — your changes introduced this failure | +| Fail | Fail | **Pre-existing** — this failure exists independently of your PR | +| No data | Fail | **Unknown** — treated as PR-caused (conservative default) | + +**PR-caused failures** are reported as HIGH or CRITICAL findings with the specific change that likely caused the failure. These are your regressions — you need to fix them. + +**Pre-existing failures** are reported separately and do not block the PR verdict. They are someone else's problem — or more precisely, they are a problem that existed before your PR and will continue to exist after it. + +## The Full Review + +CI causality is the first phase. After classifying failures, `/review-pr` continues with a structured review: + +1. **CI check results** — fetch and classify every check (pass, fail, pending, skipped) +2. **Local tool pre-flight** — run linters, tests, and build commands that CI does not already cover (skipping tools whose CI checks passed) +3. **Scoped diff** — fetch the PR diff, skipping lock files, auto-generated code, and binary files +4. **Spec alignment** — locate the associated specification (Speckit or OpenSpec) and check intent alignment +5. **AI review** — apply judgment to what deterministic tools cannot check: architectural alignment, security patterns, constitution compliance +6. **Structured report** — severity-classified findings (CRITICAL, HIGH, MEDIUM, LOW) with specific file paths and line references + +The key design principle: deterministic tools run first. AI judgment only applies to what tools cannot verify. This keeps the review grounded in facts rather than opinions. + +## Fix It Forward + +For pre-existing failures, `/review-pr` does not just report the problem — it offers to fix it. After the review, if pre-existing failures were found: + +```text +I identified 2 pre-existing CI failure(s) that are NOT caused by this PR: + - yamllint: trailing whitespace in config/database.yml + - test-auth-timeout: intermittent timeout in auth_test.go + +These failures also occur on the base branch (main). + +Would you like me to create a fix branch with a proposed resolution? +``` + +If you agree, `/review-pr` creates a fix branch (`fix/pr-42-yamllint`) from the base branch, commits a minimal fix, and reports the branch for your review. The fix stays local — you push and create a PR when ready. + +**Safety guards**: +- **Dirty-tree guard**: Will not create a fix branch if you have uncommitted changes +- **Collision check**: Will not overwrite an existing fix branch with the same name +- **Scope limit**: Will not attempt non-trivial fixes that span more than 3 files or require understanding business logic — instead, it reports the issue and recommends manual investigation + +## In-line PR Comments + +If the review finds HIGH or CRITICAL issues, `/review-pr` offers to post them as in-line comments on the PR. Comments are always shown for your approval before posting — nothing goes on the PR without your explicit confirmation. + +A cap of 15 in-line comments prevents noisy reviews. If more than 15 findings qualify, CRITICAL findings take priority and the rest are consolidated into a single summary comment. + +## The Complete Review Lifecycle + +`/review-pr` pairs with `/review-council` to form a complete review lifecycle: + +```text +Develop Push Review +─────── ──── ────── +Write code ───► Create PR ───► Reviewers look at PR + │ │ │ + ▼ ▼ ▼ +/review-council /review-pr Merge +(pre-PR, local) (post-PR, GitHub) +5+ Divisor personas 1 agent, CI-informed +``` + +**`/review-council`** runs before you push. It launches 5+ Divisor review personas in parallel against your local codebase — catching issues before they reach the PR. Think of it as your personal review team that runs in 2 minutes. + +**`/review-pr`** runs after the PR is created. It reads CI results, fetches the diff from GitHub, and produces a single structured review informed by what CI already verified. It complements the reviewers who look at the PR on GitHub. + +The two commands are independently useful — you do not need both. But together, they catch issues at two different points: before the code leaves your machine, and after it runs through CI. + +## Get Started + +Install the Unbound Force CLI and review your next PR: + +```bash +brew install unbound-force/tap/unbound-force +/review-pr +``` + +Your CI failures are classified. Your regressions are separated from the noise. Pre-existing failures get fix branches. The signal is clear. + +## See Also + +- [Common Workflows](/docs/getting-started/common-workflows/) -- `/review-council` vs `/review-pr` comparison table +- [Quick Start](/docs/getting-started/quick-start/) -- Install and verify the toolchain +- [Developer Guide](/docs/getting-started/developer/) -- Daily workflow with the `uf` CLI diff --git a/openspec/changes/blog-review-pr-causality/.openspec.yaml b/openspec/changes/blog-review-pr-causality/.openspec.yaml new file mode 100644 index 0000000..977c2b7 --- /dev/null +++ b/openspec/changes/blog-review-pr-causality/.openspec.yaml @@ -0,0 +1,2 @@ +schema: unbound-force +created: 2026-05-03 diff --git a/openspec/changes/blog-review-pr-causality/design.md b/openspec/changes/blog-review-pr-causality/design.md new file mode 100644 index 0000000..6861fbb --- /dev/null +++ b/openspec/changes/blog-review-pr-causality/design.md @@ -0,0 +1,46 @@ +## Context + +The website documents `/review-pr` in `common-workflows.md` (PR #74) with command reference, CI causality table, fix branch offering, and comparison table with `/review-council`. Issue #64 provides a narrative angle, key messages, and source references for a blog post that explains the "why" behind CI causality analysis as a broadly useful pattern. + +## Goals / Non-Goals + +### Goals +- Write a blog post following the BA-001 narrative arc (problem → approach → evidence → CTA) +- Lead with "not your fault? not your problem" per BA-007 — frame from the developer's CI triage frustration +- Include the causality classification table (base pass/fail × PR pass/fail → classification) +- Cover the fix branch offering as the "fix it forward" pattern +- Position `/review-council` + `/review-pr` as a complete review lifecycle +- Include the in-line PR comments feature with 15-comment cap + +### Non-Goals +- Duplicate the full command reference (flags, phases) — link to common-workflows docs +- Deep dive into the AI judgment phase — focus on the CI causality pattern +- Performance analysis of the reliability improvements from the follow-up change +- Tutorial-style step-by-step guide (tracked in issue #66) + +## Decisions + +1. **Slug and filename**: `review-pr-causality` — emphasizes the causality analysis which is the unique value proposition. File at `content/blog/review-pr-causality.md`. + +2. **Universal framing**: Open with a problem every developer faces (CI failures on PRs) before introducing `/review-pr`. The causality analysis pattern is useful regardless of whether the reader uses Unbound Force. + +3. **Causality table as centerpiece**: The 2×2 classification table (base pass/fail × PR pass/fail) is the key insight. Make it a prominent, scannable element. + +4. **Review lifecycle diagram**: Show the two commands as a timeline: develop → `/review-council` → push → create PR → `/review-pr`. This positions them as complementary rather than competing. + +5. **Frontmatter pattern**: Follow existing blog post frontmatter. + +6. **Cross-references**: Link to common-workflows docs where `/review-pr` is fully documented (PR #74, if merged). Link to getting-started pages which always exist. + +## Content Sources + +Authoritative upstream source: +- `/review-pr` command: `unbound-force/unbound-force/.opencode/command/review-pr.md` +- Comparison table: `content/docs/getting-started/common-workflows.md` (if PR #74 merged) + +All technical claims must be verified against the upstream command file at implementation time. + +## Risks / Trade-offs + +- **Risk**: The "fix branch" feature has specific behavioral guards (dirty-tree guard, collision check) that are easy to describe inaccurately. Mitigated by verification task against upstream source. +- **Trade-off**: Universal framing vs. tool-specific depth. Chose universal — the CI causality pattern is the hook, `/review-pr` is the implementation. Readers who don't use Unbound Force still get value from understanding the pattern. diff --git a/openspec/changes/blog-review-pr-causality/proposal.md b/openspec/changes/blog-review-pr-causality/proposal.md new file mode 100644 index 0000000..fae92a2 --- /dev/null +++ b/openspec/changes/blog-review-pr-causality/proposal.md @@ -0,0 +1,57 @@ +## Why + +When CI fails on a pull request, developers face a triage problem: is this failure caused by my changes, or was it already broken? Flaky tests, pre-existing lint violations, and stale dependency warnings create noise that obscures real regressions. Developers waste time investigating failures that predate their PR, or worse, dismiss genuine regressions as "probably pre-existing." + +`/review-pr` solves this by fetching CI results and classifying each failure as PR-caused or pre-existing. For pre-existing failures, it offers to create a fix branch. For PR-caused failures, it runs local tools and AI judgment to produce actionable findings. This CI causality analysis pattern is broadly useful — the blog post frames it as a problem any developer faces, not just Unbound Force users. + +This change addresses [GitHub issue #64](https://github.com/unbound-force/website/issues/64). + +## What Changes + +A new blog post at `content/blog/review-pr-causality.md` with the narrative arc: + +1. **The problem** — CI failures on PRs: flaky tests, pre-existing violations, wasted investigation time +2. **The solution** — `/review-pr` with CI causality analysis: PR-caused vs pre-existing classification +3. **How it works** — the classification table (base pass/fail × PR pass/fail), the review phases +4. **Fix it forward** — fix branch offering for pre-existing failures (dirty-tree guard, collision check) +5. **The complete review lifecycle** — `/review-council` (pre-PR) + `/review-pr` (post-PR) as complementary commands +6. **In-line PR comments** — 15-comment cap, human confirmation gate + +## Capabilities + +### New Capabilities +- `blog/review-pr-causality`: Blog post covering CI causality analysis and the review lifecycle + +### Modified Capabilities +- None + +### Removed Capabilities +- None + +## Impact + +- 1 new blog post (Markdown content file) +- No layout, style, or configuration changes +- Cross-references to common-workflows docs (where `/review-pr` is documented) and getting-started pages + +## Constitution Alignment + +Assessed against the Unbound Force website project constitution (`.specify/memory/constitution.md`). + +### I. Content Accuracy + +**Assessment**: PASS + +All technical claims (causality classification table, fix branch behavior, 15-comment cap, dirty-tree guard) will be verified against the upstream implementation at `unbound-force/unbound-force/.opencode/command/review-pr.md` at implementation time. The companion angle (review-council vs review-pr) is already documented in `common-workflows.md` (PR #74). + +### II. Minimal Footprint + +**Assessment**: PASS + +Single Markdown blog post file. No custom layouts, CSS, JavaScript, or dependencies. + +### III. Visitor Clarity + +**Assessment**: PASS + +Follows the BA-001 narrative arc. Leads with a universal developer problem ("your CI failed — but was it your fault?") rather than tool-specific framing. The causality classification table gives visitors a concrete mental model. The review lifecycle section shows how the two review commands complement each other. diff --git a/openspec/changes/blog-review-pr-causality/specs/blog-post.md b/openspec/changes/blog-review-pr-causality/specs/blog-post.md new file mode 100644 index 0000000..016833b --- /dev/null +++ b/openspec/changes/blog-review-pr-causality/specs/blog-post.md @@ -0,0 +1,62 @@ +## ADDED Requirements + +### Requirement: review-pr-blog-post + +The website MUST have a blog post at `content/blog/review-pr-causality.md` explaining the CI causality analysis pattern and how `/review-pr` separates PR-caused failures from pre-existing ones. + +#### Scenario: user reads review-pr blog post + +- **GIVEN** a user navigates to the review-pr causality blog post +- **WHEN** the page renders +- **THEN** the post MUST follow the BA-001 narrative arc: problem (CI failures obscure real regressions), approach (causality classification), evidence (classification table, fix branch offering), and call to action +- **AND** the post MUST lead with benefit framing per BA-007 (developer's CI triage frustration, not tool announcement) + +### Requirement: review-pr-blog-causality-table + +The blog post MUST include the CI causality classification table. + +#### Scenario: user reads causality classification + +- **GIVEN** a user reads the causality section +- **WHEN** they review the classification logic +- **THEN** the table MUST show the 2×2 matrix: base branch pass/fail × PR check pass/fail → classification (PR-caused, pre-existing, unknown) + +### Requirement: review-pr-blog-fix-branch + +The blog post MUST cover the fix branch offering for pre-existing failures. + +#### Scenario: user reads fix branch section + +- **GIVEN** a user reads the fix branch section +- **WHEN** they understand the "fix it forward" pattern +- **THEN** the section MUST explain that `/review-pr` offers to create a fix branch for pre-existing failures +- **AND** the section MUST mention the dirty-tree guard and collision check + +### Requirement: review-pr-blog-lifecycle + +The blog post MUST position `/review-council` and `/review-pr` as a complete review lifecycle. + +#### Scenario: user understands review lifecycle + +- **GIVEN** a user reads the review lifecycle section +- **WHEN** they evaluate which command to use +- **THEN** the section MUST show the timeline: develop → `/review-council` (pre-PR) → push → create PR → `/review-pr` (post-PR) + +### Requirement: review-pr-blog-frontmatter + +The blog post MUST use valid Hugo/Doks blog frontmatter consistent with existing posts. + +#### Scenario: blog post builds correctly + +- **GIVEN** the blog post file exists with frontmatter +- **WHEN** `npm run build` is executed +- **THEN** the build MUST succeed with no errors +- **AND** the frontmatter MUST include: title, description, lead, slug, date, draft (false), weight, toc, categories, tags, contributors + +## MODIFIED Requirements + +_None._ + +## REMOVED Requirements + +_None._ diff --git a/openspec/changes/blog-review-pr-causality/tasks.md b/openspec/changes/blog-review-pr-causality/tasks.md new file mode 100644 index 0000000..690980a --- /dev/null +++ b/openspec/changes/blog-review-pr-causality/tasks.md @@ -0,0 +1,31 @@ +## 1. Create blog post file with frontmatter + +- [x] 1.1 Create `content/blog/review-pr-causality.md` with Hugo/Doks blog frontmatter (title, description, lead, slug, date, draft: false, weight, toc, categories: ["Engineering"], tags: ["review", "ci", "causality", "pull-request"], contributors) + +## 2. Write the problem and solution sections + +- [x] 2.1 Write "The Problem" section: CI failures on PRs — flaky tests, pre-existing violations, wasted investigation time. Frame universally per BA-007 (every developer faces this) +- [x] 2.2 Read upstream source file (`unbound-force/unbound-force/.opencode/command/review-pr.md`) to verify current features before writing +- [x] 2.3 Write "The Solution" section: `/review-pr` with CI causality analysis — one command that fetches CI results, classifies failures, and produces actionable findings + +## 3. Write causality classification and fix branch sections + +- [x] 3.1 Write CI causality classification section with the 2×2 table: base pass/fail × PR pass/fail → PR-caused, pre-existing, unknown +- [x] 3.2 Write "Fix It Forward" section: fix branch offering for pre-existing failures, dirty-tree guard, collision check, user confirmation +- [x] 3.3 Write in-line PR comments section: 15-comment cap, human confirmation gate before posting + +## 4. Write review lifecycle and supplementary sections + +- [x] 4.1 Write review lifecycle section: `/review-council` (pre-PR) + `/review-pr` (post-PR) as complementary commands, with timeline visualization +- [x] 4.2 Write call to action: install command verified at implementation time + +## 5. Cross-references and verification + +- [x] 5.1 Add cross-reference links to common-workflows docs and getting-started pages — only link to pages that exist at implementation time +- [x] 5.2 Run `npm run build` and confirm no build errors +- [x] 5.3 Verify the blog post appears in the blog listing page +- [x] 5.4 Verify all internal links resolve (no dead links) +- [ ] 5.5 Run `npm run dev` and verify the post renders correctly +- [ ] 5.6 Verify both light and dark mode rendering + +