fix(mcp): drop the health verdict from clickstack_service_health

.changeset/service-health-no-verdict.md

@@ -0,0 +1,13 @@
+---
+'@hyperdx/api': patch
+---
+
+fix(mcp): drop the health verdict from clickstack_service_health
+
+The tool no longer labels services critical, degraded or healthy, and no longer
+tells agents a service needs "no action". Each service keeps its threshold
+flags, score and a factual summary, so agents reason from the numbers instead of
+a verdict that service-wide totals can get wrong. `summary.statusCounts` is
+replaced by `summary.servicesFlagged`, and the span attributes
+`mcp.service_health.critical_count` and `mcp.service_health.degraded_count` are
+replaced by `mcp.service_health.flagged_count`.

packages/api/src/mcp/__tests__/serviceHealthTool.int.test.ts

@@ -32,7 +32,6 @@ type Span = {
 
 type ServiceReport = {
   service: string;
-  status: string;
   flags: string[];
   current: {
     requests: number;
@@ -218,23 +217,21 @@ describe('MCP Service Health Tool', () => {
       ]);
     });
 
-    it('ranks regressions first and leaves steady services healthy', async () => {
+    it('ranks regressions first and leaves steady services unflagged', async () => {
       const { summary, services } = parse(await call());
-      expect(services.map(s => [s.service, s.status])).toEqual([
-        [svc('erroring'), 'critical'],
-        [svc('slow'), 'degraded'],
-        [svc('steady'), 'healthy'],
+      expect(services.map(s => s.service)).toEqual([
+        svc('erroring'),
+        svc('slow'),
+        svc('steady'),
       ]);
+      expect(services[0]).not.toHaveProperty('status');
+      expect(services[2].flags).toEqual([]);
       expect(services[0].flags).toEqual(
         expect.arrayContaining(['error_rate_up', 'high_error_rate']),
       );
       expect(services[1].flags).toContain('latency_up');
       expect(services[1].current.p95Ms).toBeCloseTo(600, -1);
-      expect(summary.statusCounts).toEqual({
-        critical: 1,
-        degraded: 1,
-        healthy: 1,
-      });
+      expect(summary.servicesFlagged).toBe(2);
     });
 
     it('counts server spans only by default', async () => {

packages/api/src/mcp/tools/query/__tests__/serviceHealthRank.test.ts

@@ -2,6 +2,7 @@ import { errorGroupKey } from '@/mcp/tools/query/serviceHealth/errorGroups';
 import type { NewOperation } from '@/mcp/tools/query/serviceHealth/operations';
 import {
   assessService,
+  crossedThreshold,
   HEALTH_THRESHOLDS,
   type WindowStats,
 } from '@/mcp/tools/query/serviceHealth/rank';
@@ -28,39 +29,39 @@ const stats = (s: Partial<WindowStats> = {}): WindowStats => ({
 });
 
 describe('assessService', () => {
-  it('is healthy when nothing moved', () => {
+  it('flags nothing when nothing moved', () => {
     const a = assessService(stats(), stats(), 50);
-    expect(a.status).toBe('healthy');
+    expect(crossedThreshold(a.flags)).toBe(false);
     expect(a.flags).toEqual([]);
   });
 
   it('ignores a doubled error rate below the absolute floor', () => {
     // 0.5% -> 1.0%: 2x, but only +0.5pp
     const a = assessService(stats({ errors: 10 }), stats({ errors: 5 }), 50);
     expect(a.flags).not.toContain('error_rate_up');
-    expect(a.status).toBe('healthy');
+    expect(crossedThreshold(a.flags)).toBe(false);
   });
 
   it('flags error_rate_up at exactly the ratio and delta thresholds', () => {
     // 1% -> 2%: exactly 2x and +1pp
     const a = assessService(stats({ errors: 20 }), stats({ errors: 10 }), 50);
     expect(a.flags).toContain('error_rate_up');
-    expect(a.status).toBe('degraded');
+    expect(crossedThreshold(a.flags)).toBe(true);
     expect(a.deltas.errorRateDeltaPp).toBe(1);
   });
 
-  it('is critical when errors rose past the high error rate', () => {
+  it('flags both error rules when errors rose past the high error rate', () => {
     const a = assessService(stats({ errors: 80 }), stats({ errors: 5 }), 50);
     expect(a.flags).toEqual(
       expect.arrayContaining(['error_rate_up', 'high_error_rate']),
     );
-    expect(a.status).toBe('critical');
+    expect(crossedThreshold(a.flags)).toBe(true);
   });
 
-  it('marks a chronically high error rate degraded, not critical', () => {
+  it('flags a chronically high error rate without error_rate_up', () => {
     const a = assessService(stats({ errors: 80 }), stats({ errors: 80 }), 50);
     expect(a.flags).toEqual(['high_error_rate']);
-    expect(a.status).toBe('degraded');
+    expect(crossedThreshold(a.flags)).toBe(true);
   });
 
   it('needs both the ratio and the ms delta for latency_up', () => {
@@ -79,27 +80,27 @@ describe('assessService', () => {
   it('flags traffic drops and spikes', () => {
     const drop = assessService(stats({ requests: 500 }), stats(), 50);
     expect(drop.flags).toContain('traffic_drop');
-    expect(drop.status).toBe('degraded');
+    expect(crossedThreshold(drop.flags)).toBe(true);
 
     const spike = assessService(stats({ requests: 3000 }), stats(), 50);
     expect(spike.flags).toContain('traffic_spike');
-    expect(spike.status).toBe('healthy');
+    expect(crossedThreshold(spike.flags)).toBe(false);
   });
 
   it('flags new and missing services', () => {
     const fresh = assessService(stats(), null, 50);
     expect(fresh.flags).toEqual(['new_service']);
-    expect(fresh.status).toBe('healthy');
+    expect(crossedThreshold(fresh.flags)).toBe(false);
 
     const gone = assessService(null, stats(), 50);
     expect(gone.flags).toEqual(['missing_service']);
-    expect(gone.status).toBe('degraded');
+    expect(crossedThreshold(gone.flags)).toBe(true);
   });
 
   it('treats a handful of prior requests as no baseline', () => {
     const a = assessService(stats(), stats({ requests: 1, errors: 1 }), 50);
     expect(a.flags).toEqual(['new_service']);
-    expect(a.status).toBe('healthy');
+    expect(crossedThreshold(a.flags)).toBe(false);
   });
 
   it('never flags services under minRequests in both windows', () => {
@@ -110,7 +111,7 @@ describe('assessService', () => {
     );
     expect(a.flags).toEqual(['low_volume']);
     expect(a.score).toBe(0);
-    expect(a.status).toBe('healthy');
+    expect(crossedThreshold(a.flags)).toBe(false);
   });
 
   it('applies absolute rules only when comparisons are off', () => {
@@ -126,7 +127,7 @@ describe('assessService', () => {
 });
 
 describe('rankServices', () => {
-  it('sorts by status, then by score with volume as a multiplier', () => {
+  it('sorts flagged services first, then by score with volume as a multiplier', () => {
     const cur = new Map([
       ['steady', stats()],
       ['small-regression', stats({ requests: 100, errors: 10 })],
@@ -280,6 +281,45 @@ describe('buildReport', () => {
     expect(report.services[0].flags).toContain('new_errors');
   });
 
+  it('keeps a flagged service ahead of an unflagged one with new errors', () => {
+    const report = buildReport({
+      ranked: rankServices(
+        new Map([
+          ['slow', stats({ p95Ms: 600 })],
+          ['quiet', stats()],
+        ]),
+        new Map([
+          ['slow', stats()],
+          ['quiet', stats()],
+        ]),
+        50,
+      ),
+      current: { start: new Date(0), end: new Date(600_000) },
+      comparison: { start: new Date(-600_000), end: new Date(0) },
+      comparisonMode: 'previous_period',
+      spanKind: 'server',
+      topN: 1,
+      errorGroupBy: ['SpanName', 'StatusMessage'],
+      currentErrors: new Map([
+        [
+          'quiet',
+          [
+            {
+              service: 'quiet',
+              group: { SpanName: 'POST /pay', StatusMessage: 'unavailable' },
+              count: 8,
+            },
+          ],
+        ],
+      ]),
+      comparisonErrorCounts: new Map(),
+      newOperations: new Map(),
+      spanNameKey: 'SpanName',
+    });
+    expect(report.services.map(s => s.service)).toEqual(['slow']);
+    expect(report.summary.servicesFlagged).toBe(1);
+  });
+
   const reportFor = (
     cur: WindowStats,
     cmp: WindowStats,
@@ -326,6 +366,49 @@ describe('buildReport', () => {
     expect(svc.topErrors[0].status).toBe('new_operation');
   });
 
+  it('reports facts, not a health verdict', () => {
+    const steady = reportFor(stats(), stats());
+    expect(steady).not.toHaveProperty('status');
+    expect(steady.summary).toMatch(
+      /^No metric crossed a threshold vs previous_period: error rate 0\.50% vs 0\.50%, p95 200ms vs 200ms, requests 1000 vs 1000\./,
+    );
+    const regressed = reportFor(stats({ errors: 80 }), stats());
+    expect(regressed.summary).toMatch(
+      /^Crossed thresholds: error rate 8\.00% vs 0\.50%\./,
+    );
+    for (const s of [steady.summary, regressed.summary]) {
+      expect(s).not.toMatch(/healthy|degraded|critical|no action needed/i);
+    }
+  });
+
+  it('counts flagged services instead of statuses', () => {
+    const report = buildReport({
+      ranked: rankServices(
+        new Map([
+          ['a', stats({ errors: 80 })],
+          ['b', stats()],
+        ]),
+        new Map([
+          ['a', stats()],
+          ['b', stats()],
+        ]),
+        50,
+      ),
+      current: { start: new Date(0), end: new Date(600_000) },
+      comparison: { start: new Date(-600_000), end: new Date(0) },
+      comparisonMode: 'previous_period',
+      spanKind: 'server',
+      topN: 5,
+      errorGroupBy: ['SpanName'],
+      currentErrors: new Map(),
+      comparisonErrorCounts: new Map(),
+      newOperations: new Map(),
+      spanNameKey: 'SpanName',
+    });
+    expect(report.summary.servicesFlagged).toBe(1);
+    expect(report.summary).not.toHaveProperty('statusCounts');
+  });
+
   it('does not call errors new when the service has no baseline', () => {
     const svc = reportFor(stats(), stats({ requests: 1, errors: 0 }));
     expect(svc.flags).toEqual(['new_service']);
@@ -363,6 +446,35 @@ describe('errorTiming', () => {
     expect(errorTiming(group('17:20', '17:21'), window).pattern).toBe('burst');
   });
 
+  it('states absolute values when comparisons are off', () => {
+    expect(
+      serviceSummary({
+        current: stats(),
+        comparison: undefined,
+        assessment: assessService(stats(), undefined, 50),
+        changedErrors: [],
+        newOperations: [],
+        window,
+        comparisonMode: 'none',
+      }),
+    ).toBe(
+      'No absolute threshold crossed (no comparison window): error rate 0.50%, p95 200ms.',
+    );
+  });
+
+  it('prints n/a, not "n/ams", for a missing p95', () => {
+    const summary = serviceSummary({
+      current: stats(),
+      comparison: stats({ p95Ms: null }),
+      assessment: assessService(stats(), stats({ p95Ms: null }), 50),
+      changedErrors: [],
+      newOperations: [],
+      window,
+      comparisonMode: 'yesterday',
+    });
+    expect(summary).toMatch(/p95 200ms vs n\/a,/);
+  });
+
   it('only calls bursts transient in the summary', () => {
     const summary = (g: ReturnType<typeof group>) =>
       serviceSummary({

packages/api/src/mcp/tools/query/serviceHealth/index.ts

@@ -37,9 +37,9 @@ const TIMEOUT_SUFFIX =
 const DESCRIPTION =
   'One-call health report for every service on a trace source: throughput, ' +
   'error rate, p50/p95/p99 latency and top error groups, compared with the ' +
-  'same window yesterday and ranked worst first. Each service gets a status ' +
-  '(critical / degraded / healthy), flags, and a one-line summary of which ' +
-  'changes crossed a threshold. New or growing error groups come first, ' +
+  'same window yesterday and ranked by how far each moved the wrong way. ' +
+  'Each service gets flags for changes that crossed a fixed threshold and ' +
+  'a one-line factual summary. New or growing error groups come first, ' +
   'with first/last-seen times; operations with no prior traffic are listed ' +
   'as likely rollouts.\n\n' +
   'USE THIS FIRST for status checks, "is X healthy", post-deploy checks, and ' +
@@ -53,8 +53,10 @@ const DESCRIPTION =
   'use clickstack_event_deltas and ' +
   'clickstack_trace_top_time_consuming_operations.\n\n' +
   'CALIBRATION: flags fire only past fixed thresholds (see `usage` in the ' +
-  'response). Small swings on a healthy service are normal variance. An ' +
-  'all-healthy report is a valid answer.\n\n' +
+  'response); smaller swings are usually normal variance, and no flags is ' +
+  'a valid result for a routine check. Thresholds apply to service-wide ' +
+  'totals, so an unflagged service can still hide a regression in one ' +
+  'endpoint, pod or segment. Weigh the numbers against the question.\n\n' +
   PREFER_BUILDER_OVER_SQL_NUDGE;
 
 export function registerServiceHealth({
@@ -172,10 +174,7 @@ export function registerServiceHealth({
           'mcp.service_health.window_ms':
             current.end.getTime() - current.start.getTime(),
           'mcp.service_health.services_assessed': ranked.length,
-          'mcp.service_health.critical_count':
-            report.summary.statusCounts.critical,
-          'mcp.service_health.degraded_count':
-            report.summary.statusCounts.degraded,
+          'mcp.service_health.flagged_count': report.summary.servicesFlagged,
         });
 
         const { data, isTrimmed, notes: trimNotes } = trimToolResponse(report);