Skip to content

fix(observability): every Grafana query-variable dropdown was silently empty in production#5349

Merged
JSONbored merged 1 commit into
mainfrom
claude/fix-grafana-variable-query-text
Jul 12, 2026
Merged

fix(observability): every Grafana query-variable dropdown was silently empty in production#5349
JSONbored merged 1 commit into
mainfrom
claude/fix-grafana-variable-query-text

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

User-reported live bug: the AI-usage dashboard's Provider/Feature/Model filters showed red error
icons and couldn't actually filter anything, and several panels (tokens-by-feature, cost-share-by-
feature) showed no data despite the dashboard's own summary panels clearly showing real, non-zero
totals.

Root cause, confirmed empirically against the live frser-sqlite-datasource plugin via
POST /api/ds/query: every "query"-type template variable across every dashboard
($repo, $scope, $provider/$feature/$model, added across #5234/#5267/#5302) had its
query object carry only rawQueryText — a display/round-trip field the plugin's variable-query
path never actually executes — never queryText, the field its query builder actually requires.
Every one of these dropdowns silently resolved to zero options in production; the identical raw SQL
run directly via the sqlite3 CLI, or via the API with queryText added, returns real data
immediately. Panel targets were never affected — they already set both fields at the top level —
which is exactly why every table/timeseries panel kept rendering real data while every variable
dropdown was broken, cascading into "No data"/zeroed panels wherever a panel's own WHERE clause
referenced one of the broken variables.

Since these are file-provisioned dashboards (editable: false), the fix couldn't be hotfixed via the
dashboard-save API — I updated the files directly on edge-nl-01 and restarted Grafana to confirm the
fix live before opening this PR (see test plan).

Adds a cross-dashboard regression test scanning every file in grafana/dashboards/ for this exact
shape, so a future dashboard can't reintroduce it by copying the variable pattern without the fix.

Test plan

  • npm run typecheck — clean
  • npm run test:ci — full local gate green
  • New selfhost-grafana-variable-query-text.test.ts: scans every dashboard file for this exact
    missing-queryText shape; verified it fails when the bug is reintroduced (temporarily reverted
    one file, confirmed the test catches it) and passes with the fix
  • Live-verified on edge-nl-01 via POST /api/ds/query against the running
    frser-sqlite-datasource: the $provider variable's query returned zero rows before the fix,
    real provider names after; the previously-empty "cost share by feature" panel query also
    confirmed to return real, non-zero data with the fix in place

…y empty in production

Every "query"-type template variable across every dashboard ($repo, $scope,
$provider/$feature/$model) resolved to zero options in the live Grafana
instance -- confirmed empirically against a running frser-sqlite-datasource
via POST /api/ds/query -- because the variable's `query` object only carried
`rawQueryText` (a display/round-trip field the plugin's variable-query path
never actually executes), never `queryText` (the field the plugin's query
builder requires). Panel targets were never affected -- they already set
both fields at the top level -- which is exactly why every table/timeseries
panel rendered real data while every $variable dropdown showed a red error
icon in the UI, cascading into "No data"/zeroed panels wherever a panel's
own WHERE clause referenced one of the broken variables.

Adds a cross-dashboard regression test scanning every file in
grafana/dashboards/ for this exact shape, so a future dashboard can't
reintroduce it by copying the variable pattern without the fix.
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.36%. Comparing base (b779f28) to head (593e698).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5349      +/-   ##
==========================================
- Coverage   94.40%   94.36%   -0.04%     
==========================================
  Files         550      550              
  Lines       44119    44119              
  Branches    14632    14632              
==========================================
- Hits        41652    41635      -17     
- Misses       1792     1809      +17     
  Partials      675      675              
Flag Coverage Δ
shard-1 43.75% <ø> (-0.26%) ⬇️
shard-2 34.13% <ø> (-0.52%) ⬇️
shard-3 32.16% <ø> (+0.55%) ⬆️
shard-4 31.21% <ø> (-0.21%) ⬇️
shard-5 33.04% <ø> (-0.12%) ⬇️
shard-6 43.63% <ø> (+0.27%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

gittensory-orb Bot commented Jul 12, 2026

Copy link
Copy Markdown

Warning

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

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-12 14:45:52 UTC

4 files · 1 AI reviewer · 2 blockers · readiness 93/100 · CI green · unstable

⏸️ 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
The diff adds `queryText` (mirroring the existing `rawQueryText`) to the query-type template variables in three provisioned dashboards, matching the documented root cause that the frser-sqlite-datasource plugin's variable-query path reads `queryText`, not `rawQueryText`. The fix is applied consistently across all four affected variables ($repo/$scope-equivalents, $provider, $feature, $model) and is backed by a new cross-dashboard regression test that scans every dashboard file for the missing-queryText shape, guarding against recurrence. The existing test file shown in context still only asserts on `rawQueryText`, so it will keep passing without validating the new `queryText` field, but that's a pre-existing test, not something this diff needs to fix.

Blockers

Nits — 5 non-blocking
  • The new regression test in test/unit/selfhost-grafana-variable-query-text.test.ts only checks variables whose datasource type is exactly 'frser-sqlite-datasource'; if a dashboard's per-variable datasource block is ever omitted (inheriting the panel/dashboard-level datasource) the check would silently skip that variable — worth a comment noting this is a known scanning limitation.
  • The pre-existing selfhost-grafana-ai-usage-dashboard.test.ts (test/unit/selfhost-grafana-ai-usage-dashboard.test.ts) still only asserts on `query?.rawQueryText`; consider updating it to also assert `queryText` so a future accidental removal of `queryText` alone wouldn't slip past that suite.
  • github-prs.json's added queryText is a large inlined SQL string duplicated byte-for-byte from rawQueryText — acceptable given the plugin's requirement, but a helper/constant to keep the two in sync would reduce future copy-paste drift risk.
  • Consider having the new test also verify queryText against a golden/expected string per known variable name (like the existing ai-usage test does for rawQueryText) rather than only structural equality with rawQueryText, to catch cases where both fields could be updated together but incorrectly.
  • Add a one-line PR-description or code comment cross-reference between the two test files so future readers understand why there are two overlapping Grafana dashboard test suites.

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: 44 registered-repo PR(s), 36 merged, 476 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 44 PR(s), 476 issue(s).
Gate result ❌ Blocking Repo-configured hard 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).
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 44 PR(s), 476 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.
  • 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.
[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 Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@gittensory-orb gittensory-orb Bot added the manual-review Gittensor contributor context label Jul 12, 2026
@JSONbored JSONbored merged commit 17f988e into main Jul 12, 2026
19 checks passed
@JSONbored JSONbored deleted the claude/fix-grafana-variable-query-text branch July 12, 2026 14:48
JSONbored added a commit that referenced this pull request Jul 12, 2026
…eryText

The model template variable's queryText/rawQueryText fallback (target?.rawQueryText ?? target?.queryText)
meant an unescaped queryText would never be caught once both fields existed. Assert both explicitly, and
extend the TemplateVar.query type to declare queryText (missing since #5349 added the field to the JSON).
JSONbored added a commit that referenced this pull request Jul 12, 2026
* fix(grafana): escape AI usage SQL variables

* test(grafana): assert sqlstring escaping on queryText, not just rawQueryText

The model template variable's queryText/rawQueryText fallback (target?.rawQueryText ?? target?.queryText)
meant an unescaped queryText would never be caught once both fields existed. Assert both explicitly, and
extend the TemplateVar.query type to declare queryText (missing since #5349 added the field to the JSON).
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. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant