Repository navigation
Render cited Prometheus range queries as chart cards with change markers - #1684
Merged
Merged
Conversation
PR Summary by QodoRender Prometheus findings as charts with change markers
AI Description
Diagram
High-Level Assessment
Files changed (20)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
Author
This was referenced Sep 8, 2026
Contributor
Author
|
Review round (Cursor Bugbot, head 9ea7553):
|
Findings gains a metrics kind: query_prometheus results become a chart card (range), a value table (instant) or a truncation limitation. Scope is decided from the producer's selector inventory only: target when every selector carries the target namespace and names the target's pods or workload, producer-related for namespace-only selectors, broader for anything else. Uncited broader charts are withheld and counted. AreaChart accepts annotations and an explicit domain, handles negative minima, and stacks marker labels; markers come only from same-turn change observations on the subject or its producer-established relatives. The RunManager probes Prometheus before read-only investigation turns and the prompt tells the agent it is connected, so it queries instead of guessing.
A Pod target is named only by its exact pod name, an anchored namespace regex counts as the target namespace, and credentials in a configured Prometheus URL never reach the prompt. The post-apply verification turn no longer probes for metrics. Series names are derived once from the whole result so a chart subset and its sparse table stay distinguishable, and a series with only null samples is reported as returning no finite values instead of a single sample.
An aggregation keeps the unit its metrics state, a rate turns bytes into bytes per second, and a division or disagreeing metrics leave the axis unitless. Unitless axes scale with k, M and G instead of a runaway k suffix. Prettier pass on the files this branch touched.
count, count_over_time, absent, changes, resets and timestamp return a number that is not the metric's quantity, so a count of byte series must not render as bytes.
The grouping clause sits between the aggregation name and its parenthesis, so the counting-function check must accept that form.
Opt-in layout='auto' switches to a compact layout below the label-hiding width: two Y ticks, first and last X ticks, larger text and a taller plot, so a narrow Findings pane no longer shows a strip with unreadable ticks. Existing callers keep the full layout. A count axis ends on a whole 1/2/5 step so its ticks are integers; an all-zero count series shows 0 and 1.
Compact text is larger, so a label like 177.7 MiB overran the fixed margin and was clipped by the SVG.
Helm history rows and older stores omit apiVersion, so a Helm upgrade of the workload never drew a marker. Without an apiVersion a change now matches on kind, namespace and name against the subject and its producer-established relatives; with one, the exact group match stands.
A query over one metric is titled with that metric and carries Prometheus in the subtitle; the generic title stays only when the selectors name several metrics or are unknown. The title is Radar's reading of the query, never the agent's.
The metrics nudge names the Prometheus address so the agent knows where the backend is, and that prompt is model-visible and leaves the machine. The redaction removed userinfo only. A Prometheus behind an auth proxy is commonly configured with the credential in the query string instead — `?token=…` — so the address was reduced to scheme, host and path, and the fragment goes with it for the same reason. The test covered `user:password@host` alone, which is what hid the rest. Found by Codex in the merge sign-off review.
nadaverell
force-pushed
the
feat/findings-chart-card
branch
from
September 9, 2026 18:45
01fbd78 to
bd0d5c1
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit bd0d5c1. Configure here.
The pane heads two lines with these counts — "results about other resources" and "broader metric results" — and a withheld chart was incrementing both, so one withheld result was announced twice and the two numbers stopped summing to what was actually held back. The counters are now disjoint: a metrics group goes to the metrics count and everything else to the broader one, which is what each line already claims. Introduced resolving this branch's conflict with #1681, where the two counters first met. Found by Cursor Bugbot.
Contributor
Author
|
Merge sign-off review on the rebased branch (Claude + Codex). One fix, one deferral, no other blockers. Fixed — Withheld metrics counted twice ( Deferred, and the reason is the merge order — Codex's only blocker is that pod-name prefixes do not establish ownership: Rebase check. The conflict region needed hand resolution over #1681: 12 hunks were additions on both sides, 7 were real overlaps. Two would have gone wrong silently — #1681 refactored the focused-kinds list into Verification: tsc, 989 web tests, 3,603 k8s-ui tests, both Go modules, |
This was referenced Sep 9, 2026
nadaverell
added a commit
that referenced
this pull request
Sep 9, 2026
## Summary `diagnose` returns a `metrics` field for workload targets — CPU, memory and restarts over the exact pods the bundle resolved (#1679, on `main`). Findings did not render it. This PR projects each category as a chart card in the shape the Prometheus chart cards already use (#1684), so the chart, the agent and the ledger hold the same numbers over the same window and the same pods, and the changes Radar recorded for that workload in the same turn are marked on it. ## What changed **`addDiagnoseMetrics`** (`investigationEvidence.ts`) — one `metrics` observation per category with `origin: "diagnose"`, `mode: "range"`, the producer's own `query` and `unit`, a category label (`CPU usage`, `Memory working set`, `Restarts`), the diagnosed resource as `subject`, and `pods` / `partial` from the field. - **Relevance follows the bundle** (`relevanceForResource`), never an unconditional target — a neighbour's vitals stay `broader` with the neighbour as subject. - **Identity is `metrics:diagnose:<subject>:<category>`**, keyed by the diagnosed resource rather than its display scope, so a re-diagnose revises the same three cards instead of stacking new ones. - **Summary line** reads `N pods · 60m window · 1m2s step`; `first 50 of 55 pods … · partial pod set` when the producer capped the set; `No samples in the window` for an empty series. - **Failure is distinguished from absence.** `metrics.error` becomes a "Workload metrics" limitation while the categories that did answer still chart. An *absent* field emits nothing at all — Prometheus not configured, or the read not permitted, is not a collection failure and must not be reported as one. - **Validation enforces the documented contract** before anything renders: a known category (own-property test, so inherited keys like `__proto__` cannot pass), a non-empty unit, non-negative integer counts, and a window whose end is after its start. Anything else is an invalid-payload limitation, never a card. **`metricsChangeMarkers`** (`investigationMetrics.ts`) — no markers on a `broader` chart. The turn's non-broader relatives and change observations all belong to the target, so drawing them on a neighbour's chart would attribute the target's changes to it. That path is reachable only now that a diagnose chart can carry a neighbour as its subject. **`partitionInvestigationEvidence`** (`InvestigationEvidencePane.tsx`) — inside "More evidence about this workload", cards that carry facts sort ahead of receipt-kind cards ("No classified workload issues", "No matching warning events"), which keep their order among themselves. A healthy workload no longer opens on four empty tiles before its vitals. Placement: these observations sit in "More evidence about this workload" rather than the main collection. ## Testing - `make tsc` ✓ · `web` 87 files / 1037 tests ✓ · `git diff --check` clean. No Go delta. - Adapter tests: three series → three cards with subject, unit, label, window and pod count; a re-diagnose merging into the same cards; a producer error becoming a limitation while the surviving series still chart; an unreachable Prometheus as a limitation with no card; an absent field emitting nothing; a partial pod set noted in the summary; a neighbour's diagnose landing as `broader` with the neighbour as subject; six malformed envelopes plus an out-of-contract category and a missing unit rejected. - Pane tests: a diagnose-origin chart in the workload collection with the same-turn Deployment change marked and the resource link present; the neighbour guard on the marker function; the limitation row for a producer error; and the receipt ordering — vitals first, receipts last and contiguous. - Visual test (light, dark, narrow) against a saved investigation with the `metrics` field present: three cards titled per category showing `1 pod · 60m window · 1m2s step`, axis label and unit, three recorded changes marked with "3 changes recorded in this window are marked on the chart", the earlier diagnose folded under "Previous observations · 1", and the "Open current Deployment" link. ## Scope and tradeoffs - Restarts render with unit `count`, which the shared formatter treats as unitless (values like `1.00`). A `count` case in `formatMetricValue` is a `k8s-ui` follow-up. - Marker labels for several changes on the same resource within minutes overlap in the chart — that is the marker rendering shared with #1684. - Recorded changes carrying no `apiVersion` (synthesized Helm rows, older stores) never match a chart subject and so are not marked. That rule is #1684's; it is restated here because a chart of a workload's own vitals is where its absence is most visible. ## Note on history This is #1690 re-based onto `main`. It was approved and marked merged, but its base was the integration branch `feat/findings-next-layer`, so its content never reached `main`. The commits are the same three; the Prettier-only commit was dropped and the formatting applied directly. Rebase conflicts were confined to `InvestigationEvidencePane` and its test — the counters #1684 made disjoint (`hiddenBroader` / `hiddenMetrics`) are preserved alongside the new receipt-ordering sort. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_0125QDX24TNuod7FKLwTDhvW <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes evidence projection and chart annotation rules for diagnose output; validation is strict but malformed or partial metrics could affect what users see in investigations. > > **Overview** > Findings now surfaces the **`metrics` payload from `diagnose`** as workload chart cards (CPU, memory, restarts), aligned with existing Prometheus-style metrics evidence instead of ignoring that field. > > **`addDiagnoseMetrics`** turns each valid category into a range chart with `origin: "diagnose"`, the producer query/unit, the diagnosed resource as **subject**, and relevance that follows the bundle (neighbour diagnoses stay **broader**). Re-diagnoses merge on stable identity; **`metrics.error`** becomes a "Workload metrics" limitation while partial series still render; a missing field stays silent; malformed payloads are rejected with limitations, not cards. > > **`metricsChangeMarkers`** skips **broader** charts so same-turn target changes are not drawn on a neighbour’s vitals. **`partitionInvestigationEvidence`** sorts the workload section so fact cards (including vitals) appear before contiguous **receipt** tiles. > > Coverage is mostly new adapter and pane tests for projection, merging, errors, ordering, change markers, and neighbour guardrails. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 3bf9c12. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
This was referenced Sep 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When the investigating agent runs a Prometheus range query, Findings now renders it as a chart card with the query underneath, and draws the changes Radar recorded for that workload in the same window as vertical markers on the chart. The prompt tells the agent when Prometheus is actually connected, so it queries instead of guessing. Track B of the Findings next layer, stacked on #1678 (Track 0), which provides
internal/prometheus.Availabilityand theselectorsinventory onquery_prometheusresults.A chart only claims to be about the investigation target when every selector in the query is scoped to it; anything broader shows only when the assessment cites it. Markers say "a change was recorded here", never "this caused that".
What changed
Findings (
web/)metricsevidence kind and aquery_prometheusadapter. Range results become a chart, instant results a compact value table, producer-truncated results a coverage limitation carrying the cardinality summary (no chart).selectorsinventory, never from the expression text.targetonly when every selector carries the target namespace (namespace="shop"or the anchorednamespace=~"shop") and a pod or workload matcher naming the target:pod="<name>-…",deployment|statefulset|daemonset|workload="<name>"matching the target kind, or the exact regex formspod=~"<name>-.*"/<label>=~"<name>"with the name's dot escaped (an unescapedapi.v2or anapi-?.*also selects other workloads, so those stay unclaimed). A Pod target is named only by its own exact name, since its<name>-prefix selects other pods. Namespace-only selectors areproducer-related; a bare selector, a sibling workload, an alternation, empty selectors orselectorsUnknownarebroader.metricsjoins the focused kinds inpartitionInvestigationEvidence, so a cited broader chart enters the main list (one observation perquery_prometheusresult, so today's legacy citation selects it). Uncited target-scoped charts sit in "More evidence about this workload"; uncited broader charts are withheld and counted: "N broader metric results are not shown; they appear here when the assessment cites them."investigationMetrics.ts): only same-turnchangesobservations with relevance target or producer-related, only changes whose resource is the chart's subject or one of the relatives producers named in that turn (consumed ConfigMaps/Secrets, the owner, and the pods/ReplicaSets the diagnosis rows identified). AbsoluteIssueRecentChange.timestamponly, inside the chart window, deduplicated on apiVersion+kind+namespace+name+timestamp. A change without an apiVersion (Helm history rows, older stores) matches on kind, namespace and name against the subject and its relatives; one with an apiVersion must match the group exactly. Label names the changed resource; hover tooltip says "Change recorded". No crash markers, no relativechangeContext.when._bytesstays bytes through aggregations, arate()of bytes is bytes/s,_secondsstays seconds; a division, disagreeing metrics or a bare number leave the axis unitless, and unitless axes scale with k/M/G.by (container)readscontainer=api, notseries-0), derived once from the whole result so names stay distinct across chart and table, with a legend under multi-series charts. A series with a single finite sample cannot draw a line, so it is listed with its time and value under the chart; a series whose samples are all gaps (Prometheus serializes NaN and infinities as null) is named as returning no finite values.@skyhook-io/k8s-uiAreaChart(additive)annotations?: ChartAnnotation[]anddomain?: {start, end}. Vertical dashed line plus a label pill per annotation; labels stack into rows to avoid collisions, are hidden past three rows or when the rendered chart is narrower than 420px, and the text moves into the hover tooltip in both cases.countunit gets an integer axis on a 1/2/5 step (0 and 1 for an all-zero series, 0/5/10/15/20 for a maximum of 18). Existing callers are unchanged: with no annotations, no domain and non-negative data the geometry is identical.layout="auto"(opt-in; Findings uses it) switches to a compact layout below the 420px label-hiding width: two Y ticks, first and last X ticks, larger text and a taller plot, so a narrow pane shows a readable chart instead of a strip.layout="compact"forces it for tests.ChartAnnotationexported fromcharts/types.tsand the charts index;formatMetricValuegains asecondsunit and a sign for negative values;seriesDisplayLabelsis the one place the chart tooltip andSeriesLegendderive series names from, and both accept an optionalseriesLabelsoverride for callers that chart a subset of a result.Prompt nudge (Go)
RunManager.MetricsAvailabilityhook, wired ininternal/servertoprometheus.Availability. Probed under a 2 s timeout only for investigation turns (initial and follow-up); explanation, apply and the scripted post-apply verification turn never probe, so verification stays on checking the confirmed fix.Request.Metricscarries the result; prompt selection moved intoturnPrompt, which appends the sentence after selection so follow-ups get it too. Onlyconnectedproduces text: "Prometheus is connected at ; for resource, restart, throttling or latency questions run onequery_prometheusrange query over the failure window and cite it." Other states say nothing. Credentials embedded in a configured URL are stripped before the address reaches the model-visible prompt.Testing
make tsc: passcd web && npm test: 927 passed, 2 failed insrc/api/client.authRedirect.test.ts(5 s timeouts). The same file fails the same way onmainat b54b311 in a clean checkout; unrelated to this diff.cd packages/k8s-ui && npm test: pass (3560 tests, 1 skipped)go test ./...: passmake build: passgit diff --check: cleanAreaChart.test.tsx(annotations in/out of domain, row stacking and hidden labels, truncation, explicit domain wider than samples, negative values and zero line, null gaps, sparse samples, empty, compact layout, count axes) andaxis.test.ts, Go (nudge text per turn kind and connection state, credential stripping; RunManager probes only investigation turns with a deadline, not explanation, apply or verification).kind-radar-kyverno-demo): verified the chart card in the main list with both markers labelled, the hover tooltip reading "Change recorded · ConfigMap demo-api-config" next to the sample value, the withheld-metrics note, light and dark themes, a 720px viewport, a forced 380px-wide card showing marker lines without labels, and a forced 390px card in the docked pane rendering the compact layout (two Y ticks, edge X ticks, 225px-tall plot). No console errors.Scope and tradeoffs
pod=~"<name>-.*"prefix is accepted as naming a workload target per the plan; a sibling whose name starts with the same prefix would match that regex in Prometheus too, so this mirrors what the query actually selects.Note
Medium Risk
Touches investigation evidence partitioning, model-visible prompts, and new chart rendering paths; scope rules and credential stripping reduce mis-attribution and secret leakage risk.
Overview
Investigation Findings now surfaces
query_prometheusresults as first-class metrics evidence: range queries render as chart cards (with instant queries as value tables and oversized results as limitations), scoped using the producerselectorsinventory so only fully target-scoped charts show uncited; broader queries stay hidden until cited, with a separate withheld-metrics count.Charts can show same-turn change markers (subject and producer-named relatives inside the query window) via extended
AreaChartsupport (annotations,domain, compactlayout="auto", count axes, negative values). The Go RunManager probes Prometheus on read-only investigation turns only and appends a prompt nudge when connected, with URL credential stripping before text reaches the model.Reviewed by Cursor Bugbot for commit 54842ba. Bugbot is set up for automated code reviews on this repo. Configure here.