Skip to content

fix(selfhost): give verdict the same state/merged_at precedence status has#3512

Merged
loopover-orb[bot] merged 1 commit into
mainfrom
fix/grafana-verdict-export-precedence
Jul 5, 2026
Merged

fix(selfhost): give verdict the same state/merged_at precedence status has#3512
loopover-orb[bot] merged 1 commit into
mainfrom
fix/grafana-verdict-export-precedence

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Closes #3511

Summary

  • Bug: the Grafana "Reviews & PRs (maintainer)" dashboard's "Manual review" stat matched "PRs tracked" exactly (472/472) — every PR, including merged/closed ones, showed verdict=manual.
  • Root cause: scripts/export-grafana-reporting-db.sh builds the reporting-database verdict column purely from advisories.conclusion, unlike the status column right above it (which checks p.state/p.merged_at FIRST). Verified against live production Postgres: advisories.conclusion is stuck at only neutral/action_required for every row (a separate, pre-existing gap — persistAdvisory in src/db/repositories.ts has zero callers anywhere in src/, confirmed by grep, so nothing ever writes a success/failure/skipped row for a live gate evaluation). Since both surviving values map to 'manual', verdict was mathematically forced to 'manual' regardless of the PR's real outcome.
  • Fix: mirror status's own precedence into verdict's CASE (both the Postgres-source and SQLite-source blocks): check p.state/p.merged_at first, and only fall back to a.conclusion for a PR that's still open. Verified against live production data for the exact 5 PRs visible in the reported dashboard screenshot (gittensory#3503/fix(signals): classify PHP protobuf stubs as generated #3505/feat(review): add review.tone public-safe voice brief knob (#2044) #3506, metagraphed#3646, awesome-claude#4409) — all now resolve to merge/close instead of manual.
  • Still-open PRs are intentionally unaffected — they still resolve verdict from the same (separately broken) advisories.conclusion column as before. Wiring up persistAdvisory into the live gate-evaluation path is a bigger, separate application-code change; not bundled into this reporting-script fix.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked an issue, or this is small enough that the summary explains why an issue is not needed.

Validation

  • git diff --check
  • npm run typecheck (clean)
  • shellcheck scripts/export-grafana-reporting-db.sh (clean, no CI step requires this but ran it as extra diligence on a shell-script change)
  • npx vitest run test/unit/selfhost-grafana-reporting.test.ts — 11/11 passing (10 existing + 1 new regression test)
  • Verified the exact fixed SQL directly against live production Postgres (read-only) for the 5 known-wrong PRs from the reported dashboard screenshot — all resolve correctly now.
  • npm run test:workers / npm run build:mcp / npm run test:mcp-pack / npm run ui:openapi:check / npm run ui:build — not run individually; no worker/MCP/OpenAPI/UI surface touched.
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries — added a regression test reproducing the exact production bug shape (a merged PR and a closed-not-merged PR, each with advisories.conclusion stuck at neutral/action_required), asserting the correct verdict now surfaces, plus a control case confirming a genuinely still-open PR's behavior is unchanged.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. — N/A; no auth/session/CORS surface touched.
  • API/OpenAPI/MCP behavior is updated and tested where needed. — N/A, no request/response contract change.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. — N/A, no UI change (this is an internal maintainer-only Grafana dashboard, not the product UI).
  • Visible UI changes include a UI Evidence section below. — N/A, not a apps/gittensory-ui change.
  • Public docs/changelogs are updated where needed. — N/A, internal reporting-pipeline fix, no documented/user-facing behavior change.

Notes

Root-caused via a 3-way parallel investigation (dashboard JSON, live production DB, application source code) after the user reported the dashboard symptom directly. All three independently converged on the same file/root cause, then independently cross-verified against live Postgres before implementing.

…s has (#3511)

The Grafana reporting exporter's verdict CASE switched purely on
advisories.conclusion, unlike the status CASE right above it (which
checks the PR's own terminal state first). Live advisories.conclusion
is stuck at neutral/action_required for every PR (a separate,
pre-existing gap -- persistAdvisory has zero callers), so verdict was
mathematically forced to 'manual' for every merged/closed PR on the
maintainer dashboard.

Mirror status's precedence into verdict in both the Postgres-source
and SQLite-source blocks: check state/merged_at first, fall back to
advisories.conclusion only for a PR that's still open.

Closes #3511
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.09%. Comparing base (f5574ba) to head (250046b).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3512   +/-   ##
=======================================
  Coverage   93.09%   93.09%           
=======================================
  Files         301      301           
  Lines       31450    31450           
  Branches    11483    11483           
=======================================
  Hits        29279    29279           
  Misses       1517     1517           
  Partials      654      654           
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 5, 2026
@loopover-orb

loopover-orb Bot commented Jul 5, 2026

Copy link
Copy Markdown

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-05 15:14:08 UTC

2 files · 1 AI reviewer · no blockers · readiness 100/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
The change correctly gives reporting verdicts the same terminal PR state precedence already used by status, so closed merged PRs no longer remain classified as manual because of stale advisory conclusions. The SQLite-source path has a direct regression test for merged, closed-unmerged, and still-open rows, and the issue link is present. I do not see a reachable correctness break in the changed script.

Nits — 4 non-blocking
  • nit: test/unit/selfhost-grafana-reporting.test.ts:263 covers the SQLite-source exporter path, but the matching Postgres-source CASE in scripts/export-grafana-reporting-db.sh is only protected by a fake psql fixture that returns prebuilt CSV rather than evaluating the changed SQL.
  • nit: scripts/export-grafana-reporting-db.sh:224 puts a lot of time-sensitive production diagnosis into an operational script comment; consider shortening it to the stable invariant that terminal PR state wins over advisory conclusion.
  • Add a Postgres fixture assertion that fails unless the COPY query text contains the terminal-state verdict precedence, or extend fakePsql to return closed rows with neutral/action_required conclusions for the current_pull_requests export.
  • Consider extracting the duplicated verdict/status CASE text into a shell variable or generated SQL fragment if this script keeps accumulating mirrored Postgres/SQLite logic.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #3511
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 56 registered-repo PR(s), 46 merged, 544 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 56 PR(s), 544 issue(s).
Gate result ✅ Passing No configured blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 56 PR(s), 544 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • No action.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gittensory approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 86aa97c into main Jul 5, 2026
11 checks passed
@loopover-orb
loopover-orb Bot deleted the fix/grafana-verdict-export-precedence branch July 5, 2026 15:17
JSONbored added a commit that referenced this pull request Jul 5, 2026
… not dead advisories (#3535)

The exporter's still-open-PR verdict was joined against advisories,
which only ever holds a PRE-gate rules-severity summary (always
neutral/action_required in practice) -- never the gate's own actual
merge/close/hold decision. That decision already has a live, correct,
continuously-populated home: review_audit's gittensory-native
gate_decision rows (written unconditionally on self-host by
recordNativeGateDecision). Point the exporter there instead so a
still-open PR's verdict reflects the gate's real, current state
instead of an eternal placeholder.

Verified against live production Postgres: a still-open PR now shows
its actual hold/merge/close decision instead of always 'manual', while
merged/closed PRs (the majority, from PR #3512) are unaffected.

Closes #3511
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Grafana maintainer dashboard shows verdict=manual for every PR (export CASE ignores state/merged_at)

1 participant