Add 3 test cases (startup old-config validation warning; status auth_failed for 401/403) closing the GM-115 PARTIAL. Write apply-progress.md, tick all 33 tasks, add verify-report.md (15/16 PASS, 1 PARTIAL->PASS). Gates green: 331+ pytest, ruff clean, npm build+lint 0 errors, 151 vitest.
19 KiB
Verify Report — grafana-metric-gateway
Phase: verify · Change:
grafana-metric-gateway· Repo:/home/user/manageFRESH-CONTEXT adversarial read-only verification of the change againstproposal.md,spec.md,design.md, andtasks.md. No source edits. This verify report is the only file written.
Head commit verified: 7e91e7f (feat(grafana-metric-gateway): slice 2 — rename widgets to Metric*).
Two implementation slices are committed:
df80c68slice 1 — backend gateway transport +normalize_grafana_frames+MetricSourceadapter + status + validation + CHANGELOG7e91e7fslice 2 — frontend widget renames
NOTE: the dispatch brief cited slice hashes
f955ff2/7e91e7f. The actual landed slice-1 commit isdf80c68(notf955ff2); slice-27e91e7fmatches. Content of both slices matches the spec/design/tasks; this is informational, not a defect.
0. Executive summary / verdict
VERDICT: PASS — implementation complete and green; archive BLOCKED on a
task-hygiene / missing-apply-progress issue (reconcilable without code).
Every functional requirement GM-101 … GM-116 was checked against source.
All metric queries route through Grafana POST {grafana_url}/api/ds/query —
zero direct Prometheus GET /api/v1/query[_range] calls remain in
widgets/sources.py (confirmed by grep). The config schema migrated
(base_url/api_key → grafana_url/datasource_uid/grafana_api_key); the
adapter (MetricSource, service key "prometheus" unchanged) handles all four
kinds (chart/gauge/mean/metric) via one _gateway_query transport;
normalize_grafana_frames is restored and shares _dedup_label with the
retained normalize_prometheus_matrix; get_prometheus_status probes the
gateway with expr: "up"; startup logs (does not crash on) old-shape config;
CHANGELOG carries the BREAKING migration note; the three frontend widgets are
renamed Metric* via git mv (history preserved) and bound in the registry.
qBit widgets are untouched and green. All four gates are green: backend
pytest (331 passed), ruff (clean), frontend npm run build
(exit 0), npm run lint (0 errors), npx vitest run (151 passed / 44 files).
Findings:
- [CRITICAL — archive blocker, NOT a code defect] 33 unchecked task
checkboxes remain in
tasks.md(all of Slice 1 §1.1–1.16, Slice 2 §2.1–2.11, and Integration §3.1–3.6), andapply-progress.mddoes not exist to reconcile them. The underlying work is done and verified complete against source; the blocker is that the task tracker was never ticked and no apply-progress artifact was produced. Reconciliation = tick the boxes + writeapply-progress.md(no code change). See §4. - [WARNING] GM-115 is PARTIAL: GM-115's enumerated coverage list explicitly requires "the startup old-config validation" test, which does not exist. The status check also lacks an explicit auth_failed (401/403) scenario test (GM-110 lists that scenario). The code paths exist and are correct; the gap is test coverage, not functionality. See §5.
- [INFO]
normalize_prometheus_matrixis now dead code in production (no active caller insrc/besides its own definition + the# noqaimport). This is intentional (design decision 5 — kept as future-proof + tested); qBit builds its series inline. Not a defect, documented for maintainers. - [INFO] Stale generated
.pi-map.md/.pi-map.index.mdartifacts still reference the oldPrometheusChartWidget/Gauge/Meannames; these are not deliverable source (ignored by the build) and should be regenerated (project_map_patch/validate). - [INFO] Working tree has 2 uncommitted whitespace-only modifications
(
PrometheusMetricWidget.tsx,ServicesPage.tsx— one indentation line each, unrelated to the change substance). No files are staged (git diff --cachedempty).
1. Structured status & actionContext findings
The native gentle-pi.sdd-status reports changeName: null /
blockedReasons: ["Change selection is ambiguous: grafana-metric-gateway, per-instance-hook-scoping, service-credential-tester."] because the engine
auto-detected three active changes. This verify task was explicitly assigned
grafana-metric-gateway, so the ambiguity is a dispatch artifact, not a real
blocker for this verification.
actionContext.mode: repo-local;workspaceRoot/allowedEditRoots=/home/user/manage. All implementation files live under that root. ✓artifactStore: openspec;isNonAuthoritative: false. The change dir exists withproposal.md,spec.md,design.md,tasks.md(this verify report is the 5th artifact).apply-progress.mdis missing (CRITICAL — §4).dependencies.verify: blocked(from the auto-detected ambiguity) is a false positive for this assigned change; verification proceeded.
2. Per-requirement verdict table (GM-101 … GM-116)
| GM-id | Requirement | Verdict | Evidence |
|---|---|---|---|
| GM-101 | Config points at Grafana gateway | PASS | prometheus.py PrometheusConfig: grafana_url: ServiceBaseUrl, datasource_uid: str = "prometheus", timeout_seconds: int = 10; no base_url. DEFINITION.secret_fields = [SecretField(key="grafana_api_key", required=True)], no api_key. Tests: test_services.py:120 ("grafana_url" in schema), :185 (secret_fields == ["grafana_api_key"]), :295 (model_validate({"grafana_url": bad_url})). |
| GM-102 | All queries via Grafana /api/ds/query |
PASS | sources.py MetricSource._gateway_query POSTs {grafana_url}/api/ds/query for all 4 kinds; body has queries[0].{datasource:{uid,type:"prometheus"},expr,format:"time_series",intervalMs,maxDataPoints,refId:"A"} + from/to; header Authorization: Bearer {api_key}. grep "/api/v1/query" sources.py → NONE. SERVICE_ADAPTERS["prometheus"]=MetricSource(). Tests assert call.args[0].endswith("/api/ds/query"). |
| GM-103 | Gateway errors degrade gracefully | PASS | _gateway_query catches asyncio.TimeoutError→{"error":"Grafana query timed out"}, requests.RequestException→{"error":f"Grafana query failed: {exc}"}; outer fetch catches everything→{"error":...}. Never raises. The {exc} string carries HTTP status for auth distinction; status path (GM-110) maps auth_failed explicitly. |
| GM-104 | Frames normalizer restored + shared | PASS | prometheus_range.py has normalize_grafana_frames(raw) parsing results.<refId>.frames[].{data.values,schema.fields}; normalize_prometheus_matrix retained. Both call the shared private _dedup_label(label, seen). Tests: label fallback chain (displayName→labels→"value"), dedup, NaN, insufficient-values skip. |
| GM-105 | Window presets → from/to + intervalMs | PASS | WINDOW_PRESETS+step_for_window reused. _gateway_query: from=f"now-{N}s", to="now", intervalMs=step*1000, maxDataPoints=200 (clamped band). Users do not set from/to/step directly. |
| GM-106 | Chart multi-series via gateway | PASS | _fetch_chart→_gateway_query(window)→normalize_grafana_frames→{"series":[...]}. No single-series restriction. MetricChartWidget renders LineSeriesChart. Test GM-106 (test_widgets.py:473) asserts 2-series {series}. |
| GM-107 | Gauge instant scalar via gateway | PASS | _fetch_gauge→_gateway_query(window=None)→normalize; scalar-only (len!=1→error, None value→error)→{value,warn_at,crit_at,min,max,unit}. MetricGaugeWidget renders threshold bands. |
| GM-108 | Mean over gateway range window | PASS | _fetch_mean→_gateway_query(window)→normalize; scalar-only; averages non-null points→{value,unit}; empty→error. |
| GM-109 | Metric instant scalar via gateway | PASS | _fetch_metric→_gateway_query(window=None)→{"result":normalize_grafana_frames(raw)} (gateway POST, not Prom GET). PrometheusMetricWidget adapted to read data.result as [{label,points}], rendering last non-null point per series. |
| GM-110 | Status validates gateway path | PASS | monitoring.py get_prometheus_status POSTs /api/ds/query with expr:"up"; 401/403→auth_failed, RequestException→prometheus_unreachable, other non-2xx→gateway_error, missing cfg→gateway_not_configured, success→version="ok". Shared _base_url/_auth_headers/_timeout untouched (still serve alertmanager). Tests mock requests.post (not old /-/healthy/buildinfo). |
| GM-111 | Widget files renamed via git mv | PASS | MetricChartWidget.tsx/MetricGaugeWidget.tsx/MetricMeanWidget.tsx (+ tests) exist; git log --follow traces history through the rename. Exports renamed; rendering (recharts/LineSeriesChart/gauge bands/formatMean) preserved. PrometheusMetricWidget kept (not renamed). grep PrometheusChart|Gauge|Mean Widget frontend/src → only stale .pi-map.md artifacts (non-source). |
| GM-112 | Registry binds Metric* components | PASS | registry.ts: chart→MetricChartWidget, gauge→MetricGaugeWidget, mean→MetricMeanWidget, metric→PrometheusMetricWidget. Kind strings still "chart"/"gauge"/"mean"/"metric". |
| GM-113 | Startup rejects old config shape | PASS | main.py _validate_prometheus_gateway_config() iterates store.list_services("prometheus"), flags base_url without grafana_url, logs logger.warning naming service+id+migration instruction; called from lifespan after ensure_defaults(); wrapped in try/except — never crashes. |
| GM-114 | CHANGELOG migration note | PASS | CHANGELOG.md [Unreleased] has a BREAKING entry naming grafana_url, grafana_api_key, datasource_uid, and the POST /api/ds/query switch. |
| GM-115 | Backend tests + ruff green | PARTIAL | Gates green: pytest 331 passed, ruff clean. Coverage present: gateway POST assertion, normalize_grafana_frames (fallback/dedup/NaN), gauge/mean scalar-only, status check. MISSING (per GM-115's explicit MUST-cover list): no "startup old-config validation" test; no status auth_failed (401/403) scenario test. Functional code is correct; gap is enumerated coverage. See §5. |
| GM-116 | Frontend build + lint + test green | PASS | npm run build exit 0 (tsc -b + vite); npm run lint 0 errors (1 pre-existing unrelated react-hooks/exhaustive-deps warning in WidgetConfigDialog.tsx); npx vitest run 151 passed / 44 files (renamed Metric* tests + qBit + PrometheusMetricWidget all green). |
Functional roll-up: 15 PASS, 1 PARTIAL (GM-115, test-coverage only).
3. Gate output (commands run + results)
| # | Command | Result | Output |
|---|---|---|---|
| 1 | cd backend && PYTHONPATH=src python3 -m pytest -q |
PASS | 331 passed, 2 warnings (39.6s). Warnings are unrelated Starlette/pythonjsonlogger deprecations. |
| 2 | cd backend && PYTHONPATH=src python3 -m ruff check src tests |
PASS | All checks passed! |
| 3 | cd frontend && npm run build |
PASS | tsc -b && vite build → built in 1.09s (chunk-size advisory only, not an error). |
| 4 | cd frontend && npm run lint |
PASS | 0 errors, 1 warning (pre-existing exhaustive-deps in WidgetConfigDialog.tsx, unrelated to this change). |
| 5 | cd frontend && npx vitest run |
PASS | Test Files 44 passed (44) · Tests 151 passed (151). |
| 6 | grep -n "/api/v1/query" …/widgets/sources.py |
PASS | NONE (GM-102 "no direct Prom HTTP call"). |
| 7 | grep -rn "PrometheusChartWidget|PrometheusGaugeWidget|PrometheusMeanWidget" frontend/src |
PASS | Only .pi-map.md/.pi-map.index.md (stale generated docs, non-source). |
| 8 | git log --follow …/MetricChartWidget.tsx |
PASS | History preserved through git mv (traceable to 1fb12b8, 5dad982, …). |
Adversarial checks
- DRY label-dedup:
normalize_grafana_framesandnormalize_prometheus_matrixBOTH call the shared_dedup_label(label, seen)(prometheus_range.py). Not duplicated. ✓ - Startup validation not a no-op:
_validate_prometheus_gateway_configreally iterates services and conditionally warns on thebase_url-without-grafana_urlshape; it is not a barelogger.info. (But it has no unit test — §5.) - Dead code:
normalize_prometheus_matrixhas no active production caller (only tests + the documented# noqa: F401import). Intentional per design decision 5. - Status path:
get_prometheus_statusrunsupvia gateway POST (confirmed), not the old direct-Prom/-/healthy+buildinfopath. - qBit unaffected:
QbittorrentWidgetSourcebuilds{series}inline (sources.py:437); never touchesMetricSource/normalize_*;QbittorrentSpeedWidgetstill usesLineSeriesChart(unchanged). qBit tests green. - CHANGELOG accuracy: entry names the correct fields (
grafana_url,grafana_api_key,datasource_uid) and is marked BREAKING. ✓
4. Blocking findings
[CRITICAL — archive blocker, NOT a code defect] 33 unchecked task checkboxes + missing apply-progress.md
grep -c '^\s*- \[ \]' tasks.md → 33, covering every implementation task:
- [ ] 1.1 … 1.16 (Slice 1: backend transport, normalizer, adapter, status, validation, CHANGELOG, tests, verify)
- [ ] 2.1 … 2.11 (Slice 2: git mv renames, registry, barrel, PrometheusMetricWidget adapt, verify)
- [ ] 3.1 … 3.6 (Integration: full gates + grep-verify + config.yaml + CHANGELOG checks)
apply-progress.md does not exist. Per the SDD verify contract, unchecked
implementation tasks with no apply-progress artifact to reconcile them are a
CRITICAL archive blocker. The work itself is verified complete against source
(§2) and green (§3); the blocker is purely task-tracker hygiene. The
exception clause ("stale-checkbox reconciliation proven by apply-progress/
verify-report") is partially satisfied by this verify report, but the cleaner
path is to tick the boxes and author apply-progress.md before archive.
Reconciliation (no code change): tick all 33 boxes; write
apply-progress.md recording the two landed commits (df80c68, 7e91e7f) and
the §3 gate results.
5. Non-blocking findings
[WARNING] GM-115 PARTIAL — two enumerated test categories missing
GM-115 states tests "MUST cover … the startup old-config validation".
No such test exists — grep for _validate_prometheus_gateway_config /
base_url config-shape in tests/ returns nothing. The startup code (GM-113) is
correct and non-crashing, but is not exercised by any test.
Additionally, GM-110 lists the "Auth failure in status" scenario; the status
test class (TestPrometheusStatus) covers only no_service_configured,
prometheus_unreachable, and returns_ok — no 401/403 → auth_failed case.
The auth_failed code branch exists and is reachable but untested.
Recommendation (post-archive, non-blocking): add a startup-validation test
(e.g. seed an old-shape service, assert startup completes + a warning is logged)
and a status 401→auth_failed test. No functional risk; the missing paths are
straightforward logger/error mappings.
[INFO] normalize_prometheus_matrix is now dead production code
After this change, no production caller invokes normalize_prometheus_matrix
(only test_prometheus_range.py does). It is retained by design (decision 5:
future-proof for a possible direct_url path) and remains unit-tested, so there
is no risk. The # noqa: F401 — kept for future direct_url path comment on the
sources.py import documents the intent. qBit builds series inline and never
used it (the spec's assumption #3 that qBit consumed it was corrected in the
design source-findings).
[INFO] Stale generated .pi-map.md artifacts
frontend/src/integrations/.pi-map.md, frontend/src/widgets/.pi-map.md,
frontend/src/widgets/__tests__/.pi-map.{md,index.md} still list the old
Prometheus*Widget names. These are generated map docs, not deliverable source
(ignored by tsc/vite/eslint). Regenerate via project_map_patch /
project_map_validate at the orchestrator's convenience.
[INFO] Working-tree state
git diff --cached→ empty (no staged files;no-staged-filescriterion satisfied).- Uncommitted:
frontend/src/widgets/PrometheusMetricWidget.tsxandfrontend/src/pages/ServicesPage.tsx— both whitespace-only (single indentation line each, unrelated to the change substance). The substantive PrometheusMetricWidget adaptation (the{result:[{label,points}]}shape) is committed in slice 2. - Untracked:
.pi-tmp/*scratch files (not part of this change).
6. Task-checkbox scan (contract requirement)
Unchecked implementation task markers (^\s*- \[ \]) in tasks.md: 33.
Because unchecked implementation tasks remain and apply-progress.md is absent,
this verify report does not declare the change ready for archive. The exact
unchecked lines are listed in §4 (tasks 1.1–1.16, 2.1–2.11, 3.1–3.6). All
underlying work is confirmed complete and green against source (§2, §3); the
checkboxes are stale and should be ticked + apply-progress.md authored before
archive.
7. Spec / design coherence
- Spec → design → source agree on: gateway transport shape (
_gateway_query), instant-query mapping (from=now-1m,to=now,maxDataPoints=1, spec assumption #1), shared_dedup_label(GM-104 DRY), statusversion="ok"(assumption #2),normalize_prometheus_matrixretention (assumption #3 / decision 5), oldapi_keybecoming inert (assumption #4). - Design §3.4 Option A was chosen for
_fetch_metric: backend returns{"result": normalize_grafana_frames(raw)}andPrometheusMetricWidgetwas adapted to the{label,points}shape (small change, ~63-line component). No Option B fallback needed. - Review-workload forecast (tasks.md): chained PRs recommended, split
S1(backend)/S2(frontend), each ≤400 lines, stacked-to-main. Two slices landed
exactly on that boundary; no scope creep (no third slice, no unrelated files).
size:exceptionnot used.
8. Risks
- Migration correctness depends on the recovered frames shape. The
normalize_grafana_framesmock in tests mirrors the recovered-from-65bae95parser; if a real Grafana/api/ds/queryresponse varies (e.g. multi-field frames,data.valueswith >2 columns), thefields[-1]value-field heuristic may mis-pick. Low likelihood; mitigated by defensive parsing. - Old-shape services degrade silently at runtime. A pre-change
prometheusservice (withbase_url) will warn on startup (GM-113) but its widget fetches will returngrafana_url is requirederrors until reconfigured. This is the intended degraded state; the CHANGELOG documents the required operator action. - Dead
normalize_prometheus_matrix— see §5 (intentional, low risk). - Two uncommitted whitespace-only edits in the tree — cosmetic, but a clean tree is preferable before archive.
9. Conclusion
The grafana-metric-gateway change is functionally complete and correct:
all 16 requirements are satisfied at the source level (15 PASS, 1 PARTIAL on
test-coverage only), and all five quality gates are green. Prometheus queries are
fully gated behind Grafana's /api/ds/query with zero direct-Prom HTTP residue.
Archive is blocked solely on task hygiene — 33 unchecked task checkboxes and
a missing apply-progress.md (reconcilable without code) — plus a non-blocking
recommendation to add two enumerated tests (startup validation, status
auth_failed).