feat(store): read noergler spend through a newest-rollup-per-PR view

AGENTS.md

@@ -32,7 +32,7 @@ docker compose up                    # Postgres + migrate + app on :8000
 - **`change_type` on Bitbucket events only.** Don't denormalise onto pipeline / Argo rows; join at read time.
 - **Automation detection is config-last.** Order: configured `automation` authors (matched against login *and* display name, case-insensitive) → acting user's `type == "SERVICE"` from the payload → `*-bot` name shape. Senders also declare themselves (`reviewer_handle` / `actor_handle` + account kind, read-time filter). Only accounts nobody reports get a config entry. `automation` is org-wide, at the config root.
 - **CI events are source-tagged, not source-routed.** Every CI lands in `pipeline_events` via `POST /webhooks/pipeline`, told apart by `source`. No per-CI tables or endpoints. Dedup key `source#pipeline_name#run_id#phase`.
-- **Noergler carries finops + reviewer-precision only.** `event_type` ∈ `pr_completed` | `feedback`; the pre-rollup `completed` is rejected. Never re-emit PR lifecycle — `bitbucket_events` covers open / merged / declined. Dedup keys `pr_completed#<pr_key>#<outcome>`, `feedback#<finding_id>#<verdict>`. `pr_completed` is also the source for PR diff size (Bitbucket webhooks carry none) and for the reviewer's own account.
+- **Noergler carries finops + reviewer-precision only.** `event_type` ∈ `pr_completed` | `feedback`; the pre-rollup `completed` is rejected. Never re-emit PR lifecycle — `bitbucket_events` covers open / merged / declined. Dedup keys `pr_completed#<pr_key>#<outcome>`, `feedback#<finding_id>#<verdict>`. A rollup is cumulative and a declined PR reopened and merged emits again (not a lifecycle replay): read spend via the `noergler_pr_rollups` view (newest row per `pr_key`), never a SUM over `noergler_events`. Known gap: declined (or deleted) again after a reopen emits nothing. `pr_completed` is also the source for PR diff size (Bitbucket webhooks carry none) and for the reviewer's own account.
 - **Senders verify at startup via `GET /auth/ping`** — authenticated, returns the caller's team, so a wrong token fails fast. Never reuse `/health` (unauth liveness) or `/ready` (unauth readiness).
 - **`modified_at` has a Postgres trigger** (`riptide_set_modified_at`), so any `UPDATE`, raw SQL included, bumps it. Keep the trigger when changing migrations.
 - **Database is external.** riptide ships no Postgres.

README.md

@@ -172,15 +172,15 @@ because there's no clock-start to subtract from in the first place.
 
 | Metric | How it's computed |
 |---|---|
-| **PR size** | `lines_added`, `lines_removed`, `files_changed` on `noergler_events` (`event_type = 'pr_completed'`), joined to Bitbucket PRs on `pr_key` = `'<repo_full_name>#<pr_id>'`. The same columns exist on `bitbucket_events` but are always NULL: Bitbucket DC webhooks carry no diff stats, and fetching them would put an outbound REST call in the ingest path. Covers the repos noergler reviews. |
+| **PR size** | `lines_added`, `lines_removed`, `files_changed` on `noergler_pr_rollups` (one row per PR; a reopened PR has two `pr_completed` rows in `noergler_events`), joined to Bitbucket PRs on `pr_key` = `'<repo_full_name>#<pr_id>'`. The same columns exist on `bitbucket_events` but are always NULL: Bitbucket DC webhooks carry no diff stats, and fetching them would put an outbound REST call in the ingest path. Covers the repos noergler reviews. |
 | **Revert rate** | `COUNT(*) WHERE is_revert = true` over total commits — a free, weak Change-Failure-Rate proxy. |
 | **Hotfix rate** | `COUNT(*) WHERE change_type = 'hotfix'` over total deploys per window — operational-pain signal. |
 | **Change mix** | Distribution of `change_type` (feature / bugfix / hotfix / chore / refactor / docs / other) per team per week, over `pr:opened` / `pr:merged` rows only — a push to `master` has no prefix to classify and would swamp the distribution with `other`. |
 | **Tickets per deploy** | `COUNT(DISTINCT unnest(jira_keys))` per deploy — small-batch indicator. Jira keys are extracted at write time from PR title, description, branch name, and commit messages via regex `[A-Z][A-Z0-9]+-\d+`, deduplicated, GIN-indexed. |
 | **Untracked-work rate** | `COUNT(*) WHERE jira_keys = '{}'` over merged PRs — process-compliance signal. |
 | **Per-ticket flow** | `WHERE 'ABC-1234' = ANY(jira_keys)` returns every event for a ticket across Bitbucket / pipeline / Argo (joined via commit_sha). |
 | **Human vs automated split** | `WHERE NOT is_automated` (Renovate / Dependabot / Snyk / Mend / generic-bot detection runs at write time and tags `automation_source`; an account the git host itself marks as a service account — Bitbucket DC's built-in system user, which files the default PR tasks — is tagged `service-account` with no config entry), plus the `non_human_identities` filter for accounts a sender declared. Keep `bot` and `service` apart when reading: a **bot** authors work of its own and its velocity is worth its own view, while a **service** account (a CI user pushing merges) authors nothing and should simply not appear in human activity. Default dashboards exclude both. |
-| **AI reviewer precision** *(noergler)* | `1 - count(noergler_events WHERE event_type='feedback' AND verdict='disagreed') / sum(findings_count) FILTER (WHERE event_type='pr_completed')` per repo × week — findings on both sides, since one PR can collect several disagreements. Higher = the AI review is more useful. Filter on `outcome='merged'` to score precision only on PRs that shipped. |
+| **AI reviewer precision** *(noergler)* | `1 - count(noergler_events WHERE event_type='feedback' AND verdict='disagreed') / sum(noergler_pr_rollups.findings_count)` per repo × week — findings on both sides, since one PR can collect several disagreements. Higher = the AI review is more useful. Filter on `outcome='merged'` to score precision only on PRs that shipped. |
 
 ### FinOps signals
 
@@ -191,9 +191,9 @@ events arrive pre-priced in USD.
 
 | Signal | How it's computed |
 |---|---|
-| **LLM review spend per PR / team** *(noergler)* | `SUM(cost_usd), SUM(prompt_tokens + completion_tokens) GROUP BY team` over `event_type = 'pr_completed'`. Each row is a per-PR rollup (one event per merged / declined / deleted PR). Filter on `outcome='merged'` for "spend that actually shipped"; keep all outcomes for total LLM-review spend including abandoned PRs. Pre-priced — no multiplier needed. |
-| **LLM review cost per KLOC** *(noergler)* | `SUM(cost_usd) / NULLIF(SUM(lines_added + lines_removed), 0) * 1000 GROUP BY team, outcome` over `noergler_events WHERE event_type='pr_completed'`. Diff-size normalised cost — fair comparison across small fixes and large refactors. |
-| **Wasted LLM review** *(noergler)* | `SUM(cost_usd) FROM noergler_events WHERE event_type='pr_completed' AND outcome IN ('declined','deleted')`. Review effort spent on code that never shipped. |
+| **LLM review spend per PR / team** *(noergler)* | `SUM(cost_usd), SUM(prompt_tokens + completion_tokens) GROUP BY team` over the `noergler_pr_rollups` view, the newest cumulative rollup per PR (a declined PR reopened and merged emits twice; summing `noergler_events` double-counts it). Filter on `outcome='merged'` for "spend that actually shipped"; keep all outcomes for total LLM-review spend including abandoned PRs. Pre-priced — no multiplier needed. |
+| **LLM review cost per KLOC** *(noergler)* | `SUM(cost_usd) / NULLIF(SUM(lines_added + lines_removed), 0) * 1000 GROUP BY team, outcome` over `noergler_pr_rollups`. Diff-size normalised cost — fair comparison across small fixes and large refactors. |
+| **Wasted LLM review** *(noergler)* | `SUM(cost_usd) FROM noergler_pr_rollups WHERE outcome IN ('declined','deleted')`. Review effort spent on code that never shipped. |
 | **CI compute time per pipeline / team** | `SUM(pipeline_events.duration_seconds) GROUP BY pipeline_name, team`. The unit metric for CI cost attribution. |
 | **Wasted CI** | `SUM(duration_seconds) WHERE status IN ('FAILURE','Failed')` — failed builds × time. Quantifies the cost of flakes / broken tests. |
 | **Bot-driven pipeline churn** | `pipeline_events` joined to `bitbucket_events` via `commit_sha` filtered on `is_automated = true`. Renovate / Dependabot can drive 40–70% of pipeline runs in many orgs; useful input for batching policies. |

backend/api/openapi.yaml

@@ -154,7 +154,8 @@ paths:
       description: |
         Authenticated with the team's `noergler` key. Unknown fields are a
         `422`. Delivery ids: `pr_completed#<pr_key>#<outcome>` and
-        `feedback#<finding_id>#<verdict>`.
+        `feedback#<finding_id>#<verdict>`. A declined PR that is reopened and
+        merged sends a second, cumulative `pr_completed`.
       security: [{ teamKey: [] }]
       requestBody:
         required: true

backend/internal/parse/owned.go

@@ -262,9 +262,11 @@ func parsePRCompleted(o *object, raw []byte) (*NoerglerDraft, error) {
 		d.MergeCommitSHA = ptr(strings.ToLower(*merge))
 	}
 	d.OccurredAt = closed
-	// One terminal outcome per PR, redelivered on retries. The key includes
-	// the outcome so deleted-after-declined lands two rows; pr_key is
-	// lowercased so a casing-flipped redelivery still dedupes.
+	// One rollup per terminal outcome, redelivered on retries. The key
+	// includes the outcome so a PR reopened after a decline and merged lands
+	// its merged rollup as a second row; noergler_pr_rollups reads the
+	// newest. pr_key is lowercased so a casing-flipped redelivery still
+	// dedupes.
 	d.DeliveryID = NoerglerPRCompleted + "#" + d.PRKey + "#" + outcome
 	return d, nil
 }

backend/internal/store/migrations/0002_noergler_pr_rollups.sql

@@ -0,0 +1,31 @@
+-- noergler_pr_rollups: the newest pr_completed row per pr_key. A rollup is
+-- cumulative over the PR's life, and a declined PR that is reopened and
+-- merged emits again (two rows: declined, then merged). Summed over
+-- noergler_events, the earlier row's spend counted twice. Plain view: the
+-- raw rows stay append-only and nothing needs refreshing.
+--
+-- Newest by occurred_at (the sender's emit time), then by id for a tie, so a
+-- delayed older rollup cannot win by arriving last. A deleted rollup ranks
+-- below any other: a declined PR deleted later (older senders emitted both)
+-- carries 0 lines and 0 files, and would wipe the declined row's diff size.
+-- Per pr_key, not per team: the newest rollup already carries the PR's whole
+-- spend. It also moves a reopened PR's whole spend into its merge week.
+-- Known gap: a PR declined (or deleted) again after a reopen sends no second
+-- rollup, so the spend after the first decline is missing here.
+--
+-- Columns are listed, not *: a column added to noergler_events later is a
+-- deliberate CREATE OR REPLACE here, not a silent omission.
+CREATE INDEX ix_noergler_events_pr_completed_newest
+    ON noergler_events (pr_key, (outcome = 'deleted'), occurred_at DESC, id DESC)
+    WHERE event_type = 'pr_completed';
+
+CREATE VIEW noergler_pr_rollups AS
+SELECT DISTINCT ON (pr_key)
+       id, delivery_id, event_type, pr_key, repo, commit_sha,
+       prompt_tokens, completion_tokens, elapsed_ms, findings_count, cost_usd,
+       occurred_at, created_at, team, payload, outcome, merge_commit_sha,
+       lines_added, lines_removed, files_changed, total_runs, models_used,
+       first_review_at, reviewer_handle, reviewer_account_kind
+  FROM noergler_events
+ WHERE event_type = 'pr_completed'
+ ORDER BY pr_key, (outcome = 'deleted'), occurred_at DESC, id DESC;

backend/internal/store/store_test.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"io"
 	"log/slog"
+	"reflect"
 	"strings"
 	"testing"
 	"time"
@@ -55,8 +56,12 @@ func TestMigrateIsIdempotentAndSchemaCurrent(t *testing.T) {
 		t.Fatal(err)
 	}
 	var n int
-	if err := s.pool.QueryRow(ctx, `SELECT count(*) FROM schema_migrations`).Scan(&n); err != nil || n != 1 {
-		t.Fatalf("schema_migrations rows = %d, %v", n, err)
+	files, err := migrationFS.ReadDir("migrations")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := s.pool.QueryRow(ctx, `SELECT count(*) FROM schema_migrations`).Scan(&n); err != nil || n != len(files) {
+		t.Fatalf("schema_migrations rows = %d, want %d, %v", n, len(files), err)
 	}
 	if _, err := s.pool.Exec(ctx, `DELETE FROM schema_migrations`); err != nil {
 		t.Fatal(err)
@@ -97,13 +102,74 @@ func TestCountersAreBigint(t *testing.T) {
 			t.Errorf("%s is %s", col, typ)
 		}
 	}
-	if n != 12 {
-		t.Errorf("found %d counter columns, want 12", n)
+	// 12 on the tables, 8 passed through noergler_pr_rollups.
+	if n != 20 {
+		t.Errorf("found %d counter columns, want 20", n)
 	}
 }
 
 func ptr[T any](v T) *T { return &v }
 
+// A declined PR reopened and merged emits again, and each rollup is
+// cumulative. Summed over noergler_events, declined-then-merged counted the
+// declined spend twice; the view keeps only the newest row per PR.
+func TestNoerglerPRRollupsKeepTheNewestPerPR(t *testing.T) {
+	s, pool := testStore(t)
+	ctx := context.Background()
+	t0 := time.Date(2026, 4, 28, 10, 0, 0, 0, time.UTC)
+	rollup := func(key, outcome, cost string, at time.Time) {
+		t.Helper()
+		lines := int64(100)
+		if outcome == "deleted" {
+			lines = 0
+		}
+		d := &parse.NoerglerDraft{
+			DeliveryID: "pr_completed#" + key + "#" + outcome, EventType: "pr_completed", PRKey: key,
+			Outcome: ptr(outcome), CostUSD: ptr(cost), LinesAdded: ptr(lines), OccurredAt: at, Payload: []byte(`{}`),
+		}
+		if _, err := s.InsertNoergler(ctx, d, "checkout"); err != nil {
+			t.Fatal(err)
+		}
+	}
+	rollup("a#1", "declined", "1.000000", t0)
+	rollup("a#1", "merged", "1.500000", t0.Add(time.Hour))
+	// c#3's older, costlier rollup arrives last: newest is by emit time,
+	// not arrival order, cost or outcome name.
+	rollup("c#3", "merged", "0.500000", t0.Add(time.Hour))
+	rollup("c#3", "declined", "0.900000", t0)
+	rollup("b#2", "merged", "2.000000", t0)
+	// Same emit time: the later arrival wins.
+	rollup("d#4", "declined", "0.100000", t0)
+	rollup("d#4", "merged", "0.200000", t0)
+	// Deleted after declined carries 0 lines: the declined row keeps its size.
+	rollup("e#5", "declined", "0.300000", t0)
+	rollup("e#5", "deleted", "0.300000", t0.Add(time.Hour))
+
+	got := map[string]string{}
+	rs, err := pool.Query(ctx, `SELECT pr_key, outcome || ' ' || cost_usd::text || ' ' || lines_added
+		FROM noergler_pr_rollups`)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for rs.Next() {
+		var k, v string
+		if err := rs.Scan(&k, &v); err != nil {
+			t.Fatal(err)
+		}
+		got[k] = v
+	}
+	if err := rs.Err(); err != nil {
+		t.Fatal(err)
+	}
+	want := map[string]string{
+		"a#1": "merged 1.500000 100", "b#2": "merged 2.000000 100", "c#3": "merged 0.500000 100",
+		"d#4": "merged 0.200000 100", "e#5": "declined 0.300000 100",
+	}
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("rollups = %v, want %v", got, want)
+	}
+}
+
 func TestInsertsDedupeOnDeliveryID(t *testing.T) {
 	s, pool := testStore(t)
 	ctx := context.Background()

docs/setup-noergler-notification.md

@@ -8,7 +8,7 @@ lifecycle — PR open/merge/close already comes in via Bitbucket):
 
 | Event | When | Carries |
 |---|---|---|
-| `pr_completed` | Once per PR, when it reaches a terminal outcome (merged / declined / deleted) | outcome, final diff size, aggregated token counts, elapsed time, cost, models used (finops) |
+| `pr_completed` | When a PR reaches a terminal outcome (merged / declined / deleted); a declined PR that is reopened and merged emits again | outcome, final diff size, aggregated token counts, elapsed time, cost, models used (finops) |
 | `feedback` | When a reviewer disagrees with or acknowledges a finding | finding id, verdict, actor (reviewer-precision) |
 
 Lead-time, activity, and other PR-lifecycle metrics are **not** emitted by
@@ -74,8 +74,12 @@ noergler verifies reachability and bearer validity at startup via
 }
 ```
 
-One rollup per PR: `(pr_key, outcome)` is the idempotency key, so noergler may
-safely retry. `total_cost_usd` may be omitted when the sender cannot price the
+`(pr_key, outcome)` is the idempotency key, so noergler may safely retry. A
+rollup is cumulative over the PR's life, and a declined PR that is reopened
+and merged emits again, so one PR can have two rows (declined, then merged). Read spend through the `noergler_pr_rollups` view, the newest row per
+PR; summing `noergler_events` counts the earlier row twice. A PR declined again
+(or deleted) after a reopen sends no second rollup, so the spend after the
+first decline is missing. `total_cost_usd` may be omitted when the sender cannot price the
 run (unpriced model, gateway not reporting a cost header) — send no cost rather
 than a `0`, and never drop the whole rollup: outcome, diff size, tokens and runs
 still feed the delivery metrics, and a NULL cost makes the pricing gap visible
@@ -127,8 +131,8 @@ Idempotency key is `(finding_id, verdict)`.
 -- per model, so cost cannot be split across a multi-model PR — this counts
 -- PRs a model took part in, not spend attributable to it.
 SELECT m AS model, COUNT(*) AS prs_involved
-FROM noergler_events, unnest(models_used) AS m
-WHERE event_type = 'pr_completed' AND created_at > now() - interval '7 days'
+FROM noergler_pr_rollups, unnest(models_used) AS m
+WHERE created_at > now() - interval '7 days'
 GROUP BY 1
 ORDER BY prs_involved DESC;
 
@@ -137,24 +141,26 @@ SELECT SUM(cost_usd) AS spend,
        SUM(prompt_tokens + completion_tokens) AS tokens,
        COUNT(*) AS prs,
        COUNT(*) FILTER (WHERE cost_usd IS NULL) AS unpriced_prs
-FROM noergler_events
-WHERE event_type = 'pr_completed' AND created_at > now() - interval '7 days';
+FROM noergler_pr_rollups
+WHERE created_at > now() - interval '7 days';
 
 -- review spend that never shipped, last 7 days
 SELECT outcome, COUNT(*) AS prs, SUM(cost_usd) AS spend
-FROM noergler_events
-WHERE event_type = 'pr_completed' AND created_at > now() - interval '7 days'
+FROM noergler_pr_rollups
+WHERE created_at > now() - interval '7 days'
 GROUP BY 1;
 
 -- reviewer precision: 1 - disagreed findings / reported findings, last 7 days.
 -- Both sides count findings: a PR can collect several disagreements, so a
--- per-PR denominator can drive the estimate below zero.
+-- per-PR denominator can drive the estimate below zero. A reopened PR's
+-- findings count in the window of its newest rollup, so short windows wobble.
 SELECT 1.0 - (
-    COUNT(*) FILTER (WHERE event_type = 'feedback' AND verdict = 'disagreed')::numeric
-    / NULLIF(SUM(findings_count) FILTER (WHERE event_type = 'pr_completed'), 0)
-) AS precision_estimate
-FROM noergler_events
-WHERE created_at > now() - interval '7 days';
+    (SELECT COUNT(*) FROM noergler_events
+      WHERE event_type = 'feedback' AND verdict = 'disagreed'
+        AND created_at > now() - interval '7 days')::numeric
+    / NULLIF((SELECT SUM(findings_count) FROM noergler_pr_rollups
+               WHERE created_at > now() - interval '7 days'), 0)
+) AS precision_estimate;
 ```
 
 ## Troubleshooting