Fix zeroed "caught before merge" metric and empty severity trend chart

[mk7luke]

Summary

Fixes two independent bugs on the Impact / Overview pages that appeared on real instances but were hidden in demo mode (demo uses hardcoded fixtures).

Bug 1 — "critical & major caught before merge" and "merged PRs" stuck at 0

prs.merged_at was 永 NULL on any live instance. The webhook pull_request.closed handler only aborted in-flight reviews and never recorded the merge, while the live review path re-upserts merged_at = NULL on every review. The Impact/Overview "caught before merge" & "merged PRs" metrics (and cross-PR memory) filter on merged_at IS NOT NULL, so they stayed 0 unless the manual backfill CLI was run.

Fix: new markPRClosed() in src/storage/dao.ts, called from the closed handler in src/webhook/dispatch.ts, persists merged/merged_at/closed_at from the payload (UPDATE-only — reviewed PRs already have a row).

Bug 2 — "Findings by severity over time" chart empty on the Impact page

A CSS layout bug, not data. StackedSeverityBar renders percentage-height segments, which only resolve against a definite ancestor height. .chart-bar used flex: 1 1 0% + min-height (not definite). It worked on Overview only because a grid row stretched the card; standalone on Impact every column collapsed to 0 — even though the legend totals were correct.

Fix: .chart-bar now uses flex: 1 1 auto; height: 200px (web/src/styles/base.css) — a definite fallback height in any layout while still growing to fill a stretched card.

Post-deploy note

The webhook fix records merges going forward. Existing merged PRs still have merged_at = NULL, so run npm run backfill (optionally -- --repo owner/name) after deploying to populate historical data.

Verification

• Backend tsc clean, web tsc clean
• smoke:dao and smoke:webhooks pass
• Temp-DB script driving the real getImpact(): caught-before-merge flips 0 → 2, merged PRs 0 → 1, unmerged close keeps merged_at NULL
• Bug 2 confirmed rendering live in demo mode (Impact chart shows all day columns; Overview unchanged)

🤖 Generated with Claude Code

---

Summary

Fixed live PR merge persistence so Impact and Overview metrics no longer stayed at zero after reviewed PRs were merged. Also corrected the severity trend chart layout by giving chart bars a definite height so stacked columns render outside the stretched Overview grid.

Changes

File	Changes
src/storage/dao.ts	Added a new update-only DAO helper to persist PR close and merge state, including merged_at and closed_at, from closed webhook events.
src/webhook/dispatch.ts	Updated pull_request.closed webhook handling to record merge or close metadata before aborting in-flight reviews, and improved logging around merged versus closed PRs.
web/src/styles/base.css	Adjusted chart bar flex and height styles so percentage-based stacked severity columns render with a definite height in standalone layouts.

[diffsentry]

✅ DiffSentry has completed the review — Looks good!

[diffsentry]

📝 Walkthrough

Walkthrough

Fixed live PR merge persistence so Impact and Overview metrics no longer stayed at zero after reviewed PRs were merged. Also corrected the severity trend chart layout by giving chart bars a definite height so stacked columns render outside the stretched Overview grid.

Changes

Cohort / File(s)	Summary
Merge State Persistence src/storage/dao.ts, src/webhook/dispatch.ts	Added an explicit live path for persisting PR close and merge metadata when a pull request closes. This fixed downstream metrics and cross-PR logic that depended on prs.merged_at being populated on reviewed PRs.
Severity Chart Layout web/src/styles/base.css	Reworked the chart bar container sizing so stacked percentage-height segments resolve correctly in both grid-stretched and standalone card layouts. This fixed the empty Impact severity trend chart without changing the underlying data flow.

Sequence Diagram(s)

sequenceDiagram
    participant GitHub
    participant Webhook as Webhook Dispatcher
    participant DAO as DAO
    participant Reviewer
    GitHub->>Webhook: pull_request.closed event
    Webhook->>DAO: markPRClosed(owner repo number merged mergedAt closedAt)
    DAO-->>Webhook: update prs row
    Webhook->>Reviewer: handlePRClose(owner repo number)
    Reviewer-->>Webhook: abort in-flight review
    Webhook-->>GitHub: 200 ok

Loading

sequenceDiagram
    participant UI as Impact Page
    participant CSS as Chart Layout
    participant Bar as StackedSeverityBar
    UI->>Bar: render severity columns with percentage segments
    Bar->>CSS: resolve parent bar height
    CSS-->>Bar: definite 200px or stretched card height
    Bar-->>UI: visible stacked columns

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~20 minutes

Suggested Labels

bug

Risk Assessment

Score: 10/100 — 🟢 Low

Factor	Weight	Detail
No new tests	+10	Production code added/changed without accompanying tests

Test Coverage Signal

🔴 53 line(s) of production code added with no test changes.

Source files changed	Test files changed	Source lines +	Test lines +
2	0	53	0

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
PR Title	⚠️ Warning	The title has no trailing period, no Conventional Commits prefix, and is 65 characters long, but it does not use an imperative verb. 'Fix' would be imperative; 'Fixes' is descriptive/present tense.	Address before merging or downgrade to non-blocking.
PR Description	⚠️ Warning	The description satisfies the requirement to explain both WHAT changed and WHY, with clear summaries for the backend merge-tracking fix and the frontend chart CSS fix. However, it does not include any link or reference to a related issue/PR, so it does not fully meet the stated requirements unless no related issue/PR exists.	Address before merging or downgrade to non-blocking.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Schema bump	✅ Passed	src/storage/db.ts was not changed in this PR. Changes are limited to DAO/webhook logic in src/storage/dao.ts and src/webhook/dispatch.ts, plus CSS in web/src/styles/base.css, so the schema-version check passes.
Provider parity	✅ Passed	No changes in this PR touch src/ai/anthropic.ts or any AI provider request/response contract. The diff is limited to webhook/DAO merge-state persistence and a CSS chart height fix, so there is no corresponding update needed in openai.ts or openai-compatible.ts.
Pattern test coverage	✅ Passed	No changes in src/safety-scanner.ts or src/pattern-checks.ts in this PR, so the new-rule scenario coverage check is not applicable.

_{✏️ Tip: You can configure your own custom pre-merge checks in your .diffsentry.yaml.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

📝 Generate docstrings (beta)

• Push docstring commit to this branch

🧹 Simplify (beta)

• Push simplification commit to this branch

🪄 Autofix unresolved comments (beta)

• Push autofix commit to this branch

---

_{Comment @diffsentry help to get the list of available commands and usage tips.}

[diffsentry]

Actionable comments posted: 0

The PR cleanly addresses both reported production-only regressions with focused, low-risk changes. The webhook/DAO fix correctly persists merge state on pull_request.closed, and the CSS change gives the stacked bars a definite height so the Impact chart can render outside the stretched grid layout. I did not find any blocking issues in the diff.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: .diffsentry.yaml

Review profile: ASSERTIVE

Run ID: 6eb17dee-de61-4bcd-b6cf-ebfde36d0cf7

📥 Commits

Reviewing files at c72bc58 (base SHA unavailable).

📒 Files selected for processing (3)

• src/storage/dao.ts
• src/webhook/dispatch.ts
• web/src/styles/base.css

[diffsentry]

📌 Status — last updated c72bc58

🟢 Approved

Risk score	10/100 (Low) ▁
Unresolved threads	0
Failing checks	0
Pending checks	1
Files reviewed	3
Updated	2026-07-05 00:43Z

_{Live-updated by DiffSentry on every push. Use @diffsentry ship for a verdict, @diffsentry timeline for full history.}