Skip to content

fix(observability): add additive audit_events-backed review-activity panels - #5872

Closed
glorydavid03023 wants to merge 1 commit into
JSONbored:mainfrom
glorydavid03023:fix/observability-review-event-counts
Closed

fix(observability): add additive audit_events-backed review-activity panels#5872
glorydavid03023 wants to merge 1 commit into
JSONbored:mainfrom
glorydavid03023:fix/observability-review-event-counts

Conversation

@glorydavid03023

Copy link
Copy Markdown
Contributor

Closes #3717

Summary

Part 1 (#4134) clarified the wording on the existing snapshot panels. This is part 2 — the true additive event-count panels the issue asks for, which the maintainer explicitly left for a contributor.

The bug, restated

All six stat panels on this dashboard count rows in review_targets, whose status/verdict are mutable current-state fields, overwritten as a PR's disposition changes — not an append-only log. And manual / commented / ignored are transient states for most PRs.

So a PR that was held for manual review and then merged inside the same window is counted only under Merged — never under Manual review. The panel answers "how many PRs are currently sitting in this state as of their last update in-window", not "how many times did a PR enter this state". For a maintainer who actively drains their manual-review queue, it structurally under-reports — which is exactly the reported symptom ("I've definitely had more than 21 manual reviews").

The fix

The append-only log already existed; the dashboard simply never queried it. audit_events already records the transitions:

  • agent.action.hold — emitted when a PR is held for manual review (src/queue/processors.ts)
  • agent.action.${actionClass} — emitted for every executed action (src/settings/agent-execution.ts)

So no new event emission is needed, and no application code changes. This PR adds a Review activity (additive, from audit_events) row with:

Panel Counts
Manual reviews entered agent.action.hold events in the window
Merges executed agent.action.merge events in the window
Closes executed agent.action.close events in the window
Review actions per day stacked per-day breakdown of all four action types

Every panel windows on created_at (when the event happened), never updated_at — so a day's count never changes retroactively when a PR's status later moves on. That is precisely what the review_targets-backed panels cannot show.

The existing panels are untouched, and the new row is appended (y: 34), so no existing panel shifts position. The diff is 52 added lines and zero deleted ones.

Two things I want to be upfront about

  1. audit_events has no repo column. Repo scoping goes through target_key LIKE ${repo:sqlstring} || '#%' (the key is owner/repo#number), rather than the repo = … equality the review_targets panels use.
  2. These counts cannot exclude bot-authored PRs. audit_events records the action the agent took, not the PR's submitter, so the submitter NOT LIKE '%[bot]%' filter the snapshot panels use has no equivalent here. Rather than silently produce a subtly different number, this is stated plainly in the panel descriptions, so a maintainer comparing the additive panel against the snapshot above it knows exactly why they differ.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • 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 a currently open issue this PR resolves — see the closing reference at the top of this body.

Validation

  • git diff --check
  • npm run typecheck
  • npm run selfhost:validate-observability
  • npm run engine-parity:drift-check
  • npm run docs:drift-check
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

New suite test/unit/selfhost-grafana-additive-review-events.test.ts (8 tests) pins the panels to:

  • audit_events, never review_targets;
  • created_at, never updated_at (the whole point — an append-only window);
  • the real emitted event types, with a drift guard: every agent.action.* event_type the panels query must be one AGENT_ACTION_CLASSES can actually produce, so renaming an action class can never silently leave these panels reading zero forever;
  • queryText / rawQueryText kept in sync, as the SQLite datasource requires.

It also asserts the six original review_targets panels are still present and unmodified, and that panel ids stay unique.

The existing dashboard suites (selfhost-grafana-dashboard, selfhost-grafana-no-dollar-underscore-sentinel) pass unchanged — 51 tests green across the three files.

codecov/patch has no coverable lines in this diff: the change is dashboard JSON (not in the coverage include set) plus a test file (codecov-ignored).

If any required check was skipped, explain why:

  • None skipped.

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.
  • API/OpenAPI/MCP behavior is updated and tested where needed.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks.
  • Visible UI changes include a UI Evidence section. Not applicable — see below.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

UI Evidence

Not applicable — this is a Grafana dashboard JSON definition, not an application UI surface. It renders inside a self-hosted Grafana against the operator's own loopover-db, so there is no reviewable app screenshot to capture and no apps/loopover-ui/ code is touched. The panel definitions are asserted directly in the new test suite instead.

Notes

  • Zero application-code changes: the audit events these panels read were already being written. The gap was purely that the dashboard never queried them.

…panels (JSONbored#3717)

Part 1 (JSONbored#4134) clarified the wording on the snapshot panels. This is part 2: the true additive
event-count panels the issue asks for.

The six existing stat panels all count rows in `review_targets`, whose `status`/`verdict` are mutable
current-state fields overwritten as a PR's disposition changes. `manual`/`commented`/`ignored` are
transient for most PRs, so a PR that was held for manual review and then merged inside the same window
is counted ONLY under Merged -- never under Manual review. Those panels answer "how many PRs are
currently sitting in this state", not "how many times did a PR enter it", and so they structurally
under-report for a maintainer who actively drains the queue. That is exactly the reported symptom.

`audit_events` is append-only and already records the transitions: `agent.action.hold` when a PR is
held for manual review (processors.ts), and `agent.action.${actionClass}` for every executed action
(agent-execution.ts). Nothing new needs to be emitted -- the log was already there, and the dashboard
simply never queried it.

Adds a "Review activity (additive, from audit_events)" row with three stats (manual reviews entered,
merges executed, closes executed) and a stacked per-day breakdown, all windowed on `created_at` so a
day's count never changes retroactively. Existing panels are untouched; the new row is appended, so no
panel shifts position.

Scoping to the selected repo goes through `target_key LIKE '<repo>#%'` rather than a `repo` column,
which `audit_events` does not have. That also means these counts cannot exclude bot-authored PRs the
way the review_targets panels do -- stated plainly in the panel descriptions rather than papered over.

Tested: 8 assertions pin the panels to audit_events (never review_targets), to `created_at` (never
`updated_at`), to the real emitted event types -- including a drift guard that every event_type the
panels query is one AGENT_ACTION_CLASSES can actually produce, so renaming an action class cannot
silently leave these panels reading zero forever.
@superagent-security

Copy link
Copy Markdown
Contributor

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

@glorydavid03023

Copy link
Copy Markdown
Contributor Author

CI failure here is infrastructure, not this PR

All six validate-tests shards fail at the "Upload coverage to Codecov (fork PR tokenless)" step, which then fails the required validate aggregate. The tests themselves pass; the upload is what fails:

./codecov upload-coverage --fail-on-error --git-service github --pr 5872 --sha 336dc82 --branch glorydavid03023:fix/observability-review-event-counts
error -- Upload queued for processing failed: {"message":"Token required - not valid tokenless upload"}
##[error]Process completed with exit code 1.

Codecov is now rejecting tokenless uploads. Because that step runs with fail_ci_if_error: true, the rejection becomes a hard CI failure.

This is repo-wide for fork PRs, not specific to this one. At the time of writing, every open fork PR shows the identical signature — #5870, #5871, #5872, #5873, #5877, #5879 — while owner-authored PRs (#5874, #5878, #5881) are green.

Since the gate closes a contributor PR on any failing check, correct and fully-tested fork PRs are being auto-closed for a reason unrelated to their content.

A fork PR can't see repo secrets, so its Codecov upload is inherently best-effort. Setting fail_ci_if_error: false on that step would stop a Codecov-side policy change from failing contributors' builds; routing fork coverage through the workflow_run path already precedented in #2009 would additionally keep codecov/patch reporting (the silent-pass gap #2273 closed).

I'd be glad to open that workflow fix, but a PR here requires a linked open issue and issue creation is restricted for non-maintainers, so I can't file one to link. Happy to pick it up if you open an issue for it.

For this PR specifically: the change is dashboard JSON plus one test file, so it adds no coverable lines — codecov/patch has nothing to measure either way.

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

loopover-orb Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 LoopOver review result - fixes required

Review updated: 2026-07-14 21:33:33 UTC

2 files · 1 AI reviewer · no blockers · readiness 78/100 · CI failing · blocked

🛑 Suggested Action - Fix Blockers

Review summary
This is a pure-JSON Grafana dashboard addition (4 new panels + a row header) that queries the existing append-only audit_events table instead of the mutable review_targets snapshot, with no application code changes — exactly the additive fix the issue describes. It's well-tested: the accompanying test file asserts the new panels query audit_events (not review_targets), window on created_at (not updated_at), and includes a drift guard that cross-checks queried event_type strings against AGENT_ACTION_CLASSES so a future action-class rename can't silently zero these panels. Description explicitly acknowledges the one real limitation (bot-authored PRs can't be filtered out of the audit_events counts) rather than hiding it.

Nits — 4 non-blocking
  • The row/panel gridPos y-coordinates (34, 35, 39) are hand-computed absolute offsets from the prior panel's position — if an earlier panel's height changes later, these will silently overlap rather than fail loudly (grafana/dashboards/maintainer-reviews.json).
  • The drift-guard test only checks that queried event_type strings are in the emittable set, not that every AGENT_ACTION_CLASSES entry is actually queried somewhere — so a new action class added later wouldn't be flagged as missing from the dashboard (test/unit/selfhost-grafana-additive-review-events.test.ts).
  • Consider a short comment/link in the dashboard JSON itself (not just the test) noting why hold is hardcoded alongside AGENT_ACTION_CLASSES-derived events, since a future reader of the JSON alone won't see that context.
  • If Grafana provides a way to reference the row's height via variable rather than a literal y-offset, that would remove the fragile-gridPos concern noted above — otherwise this is fine as-is given Grafana's dashboard model doesn't really support that.

CI checks failing

  • validate
  • validate-tests (2)
  • validate-tests (1)
  • validate-tests (6)
  • validate-tests (5)
  • validate-tests (3)
  • validate-tests (4)
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #3717
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 ❌ 5/25 Preflight is holding this PR: the review lane is unavailable, so it is not ready for automated review.
Contributor workload ✅ 10/10 Author activity: 287 registered-repo PR(s), 173 merged, 21 issue(s).
Contributor context ✅ Confirmed Gittensor contributor glorydavid03023; Gittensor profile; 287 PR(s), 21 issue(s).
Gate result ✅ Passing No configured blocker found.
Improvement ⚠️ ℹ️ Insufficient signal risk: clean · value: insufficient-signal — Nothing measurable for the structural-improvement analyzers on this PR (e.g. no code files changed). LLM value judgment: moderate — It closes a clearly-scoped, maintainer-acknowledged issue (#3717) by wiring up dashboard panels to an already-emitted event log with no new app-code risk, backed by tests that guard against the exact under-reporting bug being fixed.
Linked issue satisfaction

Partially addressed
The PR adds the additive audit_events-backed panels (part 2) with well-explained descriptions and tests, but the diff shown does not touch the three existing 'Manual review'/'Commented'/'Ignored' stat panels' descriptions themselves (part 1 is claimed to have been done in a separate PR #4134), and no written finding confirming/refuting the undercounting hypothesis with real data is included in thi

Review context
  • Author: glorydavid03023
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: Python, JavaScript, TypeScript, Rust, C++, Kotlin, MDX, Ruby
  • Official Gittensor activity: 287 PR(s), 21 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Await review-lane availability.
  • Refresh registry data or choose a registered active repo.
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.
[BETA] Chat with Gittensory

Ask Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @gittensory ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @gittensory chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @gittensory mention with a real question is routed to the closest matching read-only command automatically -- no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb

loopover-orb Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Gittensory is closing this pull request on the maintainer's behalf (CI is failing (validate, validate-tests (2), validate-tests (1), validate-tests (6), validate-tests (5), validate-tests (3), validate-tests (4))). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

@loopover-orb loopover-orb Bot closed this Jul 14, 2026
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.

fix(observability): clarify manual/commented/ignored panel semantics on the Reviews & PRs dashboard

1 participant