Skip to content

fix(selfhost): recognize review_audit in the exporter's no-source-tables guard - #3546

Merged
JSONbored merged 1 commit into
mainfrom
fix/grafana-export-guard-review-audit
Jul 5, 2026
Merged

fix(selfhost): recognize review_audit in the exporter's no-source-tables guard#3546
JSONbored merged 1 commit into
mainfrom
fix/grafana-export-guard-review-audit

Conversation

@JSONbored

@JSONbored JSONbored commented Jul 5, 2026

Copy link
Copy Markdown
Owner

Closes #3554

Summary

  • Small follow-up to fix(selfhost): source an open PR's Grafana verdict from review_audit, not dead advisories #3535, addressing a non-blocking nit flagged on that PR's own review: the exporter's "does this database have any reporting signal at all" fail-safe guard (scripts/export-grafana-reporting-db.sh) still only listed pull_requests/advisories/review_targets/ai_usage_events — stale since fix(selfhost): source an open PR's Grafana verdict from review_audit, not dead advisories #3535 made review_audit a genuine dependency of the still-open-PR verdict computation.
  • Not a live bug: pull_requests already dominates the OR-condition on any real deployment, so the guard was never actually wrong in practice — just an inaccurate list for an operator reading it to understand what the script depends on. The only theoretical gap it left: a database with only review_audit present would have been misclassified as empty and skipped.
  • Fix: add review_audit to both the Postgres and SQLite variants of the guard. No issue filed — this is small enough, self-evident from the PR description, and directly traceable to the reviewer comment it addresses (this repo's linkedIssuePolicy is "preferred", not required).
  • Added a regression test proving the specific new branch: a source DB containing only review_audit is recognized as having real data (the exporter runs a fresh pass) rather than triggering the "preserve the last-good snapshot" fail-safe.

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 — no TypeScript files changed)
  • shellcheck scripts/export-grafana-reporting-db.sh (clean)
  • npx vitest run test/unit/selfhost-grafana-reporting.test.ts — 14/14 passing (13 existing + 1 new)
  • 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 — new test covers the specific new OR-branch (a source DB with only review_audit), asserting a fresh pass runs instead of the last-good snapshot being wrongly preserved.

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.
  • Visible UI changes include a UI Evidence section below. — N/A, not an apps/gittensory-ui change.
  • Public docs/changelogs are updated where needed. — N/A, internal reliability fix, no documented/user-facing behavior change.

…les guard

The "does this database have any reporting signal at all" fail-safe
guard still listed only pull_requests/advisories/review_targets/
ai_usage_events -- stale since #3535 made review_audit a genuine
dependency of the verdict computation. In practice pull_requests
already dominates the check on any real deployment, so this was
cosmetic, not a live bug, but a database with only review_audit
present would have been misclassified as empty. Add it to both the
Postgres and SQLite variants of the guard.

Flagged as a non-blocking nit on #3535's own review.
@superagent-security

Copy link
Copy Markdown
Contributor

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

@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
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-05 16:40:02 UTC

2 files · 1 AI reviewer · 2 blockers · readiness 93/100 · CI pending · blocked

⏸️ Suggested Action - Manual Review

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Review summary
This is a narrow, correct follow-up to #3535: it adds `review_audit` to both the Postgres and SQLite variants of the exporter's no-source-tables guard, closing a real (if narrow) gap where a database containing only `review_audit` would have been misclassified as empty and skipped. The new test in selfhost-grafana-reporting.test.ts genuinely exercises the fixed branch — it seeds a last-good snapshot, points the exporter at a source DB with only `review_audit`, and asserts the fresh pass overwrites the stale snapshot (review_targets count goes from 1 to 0) rather than the pre-fix behavior of preserving it and exiting non-zero. The change is minimal, symmetric across both guard sites, and the reasoning in the description matches the diff.

Nits — 3 non-blocking
  • Only the SQLite guard path is covered by the new test; the Postgres variant (scripts/export-grafana-reporting-db.sh:191) gets the identical `pg_table_exists "review_audit"` addition but has no corresponding regression test, presumably because Postgres isn't exercised in this unit suite — worth a one-line note in the test file if that's intentional.
  • No issue was filed, but the PR description explicitly justifies this as traceable to a specific reviewer comment on fix(selfhost): source an open PR's Grafana verdict from review_audit, not dead advisories #3535 and small enough to stand on its own, consistent with this repo's stated 'preferred, not required' issue-link policy.
  • Consider adding a short comment at the guard site itself (scripts/export-grafana-reporting-db.sh:187-192) listing which downstream computation depends on `review_audit` (the still-open-PR verdict logic a few lines below), so the next contributor extending this list doesn't have to rediscover the dependency.

Concerns raised — review before merging

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
Signal Result Evidence
Code review ❌ 2 blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
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 (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 56 registered-repo PR(s), 46 merged, 521 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 56 PR(s), 521 issue(s).
Gate result ❌ Blocking Repo-configured hard 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: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 56 PR(s), 521 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • No action.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
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

@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.10%. Comparing base (4aa3f64) to head (9073ec6).
⚠️ Report is 4 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3546   +/-   ##
=======================================
  Coverage   93.10%   93.10%           
=======================================
  Files         302      302           
  Lines       31504    31504           
  Branches    11504    11504           
=======================================
  Hits        29333    29333           
  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.

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.

Development

Successfully merging this pull request may close these issues.

Grafana exporter's no-source-tables guard doesn't recognize review_audit

1 participant