feat(mcp): improve metric discovery, add quiet-saturation eval scenario - #2861
Conversation
🦋 Changeset detectedLatest commit: fdeeac5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🟡 Tier 3 — StandardIntroduces new logic, modifies core functionality, or touches areas with non-trivial risk. Why this tier:
Additional context: 8 file(s) in private internal-tooling packages, excluded from the line count Review process: Full human review — logic, architecture, edge cases. Stats
|
Greptile SummaryThe PR makes metric names directly discoverable from MCP source listings and adds widening lookback sampling shared with source description. It also adds the quiet-saturation evaluation scenario and informational adoption-check reporting. Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| packages/api/src/mcp/tools/sources/listSources.ts | Adds bounded, concurrent metric-name preview collection and direct-query guidance to source discovery. |
| packages/api/src/mcp/tools/sources/metricNames.ts | Extracts shared metric-name sampling, widening lookbacks, and optional unit and description enrichment. |
| packages/api/src/mcp/tools/sources/describeSource.ts | Reuses the shared widening-lookback sampler for metric source descriptions. |
| packages/hdx-eval/src/scenarios/quiet-saturation/generate.ts | Adds deterministic telemetry for diagnosing gradual connection-pool saturation. |
| packages/hdx-eval/src/grading/programmatic.ts | Evaluates informational adoption checks while excluding them from the weighted adoption score. |
| packages/hdx-eval/src/reports/aggregate.ts | Carries informational adoption metadata into aggregate reports. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[clickstack_list_sources] --> B[Metric source tables]
B --> C[Sample metric names]
C --> D{Recent names found?}
D -->|No| E[Widen lookback]
E --> C
D -->|Yes| F[metricNamesPreview]
F --> G[Direct table or timeseries query]
Reviews (9): Last reviewed commit: "Merge branch 'main' into karl/improve-me..." | Re-trigger Greptile
Deep ReviewScope: MCP metric discovery ( ✅ No critical issues found. The concurrency/abort/timeout design in 🟡 P2 — recommended
🔵 P3 nitpicks (2)
Reviewers (2): previous-comments, orchestrator diff analysis (correctness, reliability, performance, testing, security/injection lenses). Testing gaps: timeout-omission path and dedup cache in |
E2E Test Results✅ All tests passed • 315 passed • 1 skipped • 1145s
Tests ran across 4 shards in parallel. |
3b1a262 to
8a7ad67
Compare
pulpdrew
left a comment
There was a problem hiding this comment.
I see my comments in packages/api/src/mcp/tools/sources/metricNames.ts are on code that was just relocated. It'd be nice to fix them but we can treat them as non-blocking.
8a7ad67 to
b3395ca
Compare
b3395ca to
2ced399
Compare
9588258 to
8e0bb8e
Compare
Why
Eval transcripts showed agents solving investigations without ever touching metrics — not because metrics weren't useful, but because they were the only signal behind a discovery wall. After
list_sources, logs and traces are queryable immediately (key columns are in the catalog), while a metric source showed only opaque table names: querying it cost 1–3 extra calls (describe_source/list_metrics) just to learn what exists. Under a turn budget, agents rationally skipped it — even when explicitly nudged (we tested this: prompt- and output-level hints naming the exact metrics were ignored in 7/7 deliveries when metrics weren't on the efficient path).What changed
clickstack_list_sources: metric sources now includemetricNamesPreview— up to 10 recently-reported metric names per kind, sampled from the team's own tables — plus a usage note that metrics are queried directly viaclickstack_table/clickstack_timeserieswithmetricType + metricName, no describe hop needed. Best-effort under a 3s wall-clock budget with a concurrency pool and per-table dedup; omitted silently on timeout.metricNames.ts(new): the metric-name sampler extracted fromdescribeSource.tsinto a shared module, with a widening lookback (24h → 30d, first non-empty window wins) so sparse or batch-emitted metrics still produce a sample.describe_sourceuses the same sampler, so its per-kind sample gets the lookback too.Measured wins
Benchmarked with the hdx-eval framework on
quiet-saturation(connection-pool leak where metric history is the efficient diagnostic path), branch vsmain, identical seed/anchor/prompts, claude-fable-5 + claude-opus-4-6, 3 runs/cell, two independent batches.First tool call whose args name a target metric (the load-bearing pool gauges), every run, both batches:
Zero overlap across all 24 runs (branch ≤7, main ≥8; p ≈ 0.001 by permutation): the preview removes the discovery hops entirely (
list_sources→ query), where main insertslist_metrics/describe_sourcedetours or trace-grinds first.Headline metrics from the second batch (which ran exactly this minimal build):
distinguishes_true_onset)