fix(observability): stop MCP paying for the browser's comparison grid

internal/agent/tools_integration_test.go

@@ -25,7 +25,7 @@ func (registryQueries) Topology(context.Context, observability.Scope, int) (obse
 	return observability.Result[observability.Topology]{}, nil
 }
 
-func (registryQueries) Performance(context.Context, observability.Scope, string, int) (observability.Result[observability.Performance], error) {
+func (registryQueries) Performance(context.Context, observability.Scope, observability.PerformanceOptions) (observability.Result[observability.Performance], error) {
 	return observability.Result[observability.Performance]{}, nil
 }
 

internal/api/observability.go

@@ -15,7 +15,7 @@ import (
 type ObservabilityQueries interface {
 	Overview(context.Context, observability.Scope, int) (observability.Result[observability.Overview], error)
 	Topology(context.Context, observability.Scope, int) (observability.Result[observability.Topology], error)
-	Performance(context.Context, observability.Scope, string, int) (observability.Result[observability.Performance], error)
+	Performance(context.Context, observability.Scope, observability.PerformanceOptions) (observability.Result[observability.Performance], error)
 	Trace(context.Context, observability.Scope, string, string, int) (observability.Result[observability.TraceDetail], error)
 	Logs(context.Context, observability.Scope, string, string, string, int) (observability.Result[observability.Logs], error)
 }
@@ -72,7 +72,9 @@ func (h *ObservabilityHandler) performance(c *echo.Context) error {
 	if err != nil {
 		return err
 	}
-	result, err := h.queries.Performance(c.Request().Context(), scope, c.QueryParam("service"), limit)
+	result, err := h.queries.Performance(c.Request().Context(), scope, observability.PerformanceOptions{
+		Service: c.QueryParam("service"), Limit: limit, Heatmap: true,
+	})
 	if err != nil {
 		return mapQueryError(err)
 	}

internal/api/observability_test.go

@@ -35,7 +35,7 @@ func (f *fakeQueries) Topology(_ context.Context, scope observability.Scope, _ i
 	}, nil
 }
 
-func (f *fakeQueries) Performance(_ context.Context, _ observability.Scope, _ string, _ int) (observability.Result[observability.Performance], error) {
+func (f *fakeQueries) Performance(_ context.Context, _ observability.Scope, _ observability.PerformanceOptions) (observability.Result[observability.Performance], error) {
 	return observability.Result[observability.Performance]{Schema: observability.PerformanceSchema}, nil
 }
 
@@ -151,7 +151,7 @@ func (p deadlineProbe) Topology(ctx context.Context, scope observability.Scope,
 	return observability.Result[observability.Topology]{}, nil
 }
 
-func (p deadlineProbe) Performance(ctx context.Context, scope observability.Scope, service string, limit int) (observability.Result[observability.Performance], error) {
+func (p deadlineProbe) Performance(ctx context.Context, scope observability.Scope, opts observability.PerformanceOptions) (observability.Result[observability.Performance], error) {
 	p.note(ctx, "/api/observability/performance")
 	return observability.Result[observability.Performance]{}, nil
 }

internal/cmd/lintdocs/doc_placement_test.go

@@ -0,0 +1,124 @@
+package lintdocs
+
+import (
+	"go/ast"
+	"go/parser"
+	"go/token"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// Inserting a declaration between a doc comment and the thing it documents is
+// invisible to gofmt, go vet and the compiler, and it silently reassigns the
+// comment: the new declaration inherits a block describing something else, and
+// the original is left undocumented. It happened three times in this tree, twice
+// in one afternoon, every time from a scripted edit that matched on the
+// declaration line and inserted above it.
+//
+// Requiring every doc comment to start with its own declaration's name would
+// flag hundreds that simply do not follow that convention, and a guard that is
+// mostly false positives gets muted. This checks the exact signature of the bug
+// instead: a doc comment whose first word names the declaration immediately
+// BELOW the one it is attached to. That is what such an insertion produces, and
+// nobody writes it on purpose.
+func TestDocCommentsDocumentWhatTheySitOn(t *testing.T) {
+	root := "../../.."
+	fset := token.NewFileSet()
+
+	// Every declaration in source order, documented or not. Tracking only the
+	// documented ones would compare against the next *documented* declaration
+	// and skip straight past the undocumented victim -- which is precisely the
+	// declaration this is looking for.
+	type decl struct {
+		file string
+		line int
+		name string
+		doc  string
+	}
+	var decls []decl
+
+	err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		if info.IsDir() {
+			switch info.Name() {
+			case ".git", "node_modules", "vendor", "dist", "site", "experiments":
+				return filepath.SkipDir
+			}
+			return nil
+		}
+		if !strings.HasSuffix(path, ".go") {
+			return nil
+		}
+		parsed, err := parser.ParseFile(fset, path, nil, parser.ParseComments)
+		if err != nil {
+			return nil // not this guard's job to report unparseable files
+		}
+		add := func(name string, doc *ast.CommentGroup, pos token.Pos) {
+			text := ""
+			if doc != nil {
+				text = doc.Text()
+			}
+			decls = append(decls, decl{file: path, line: fset.Position(pos).Line, name: name, doc: text})
+		}
+		for _, d := range parsed.Decls {
+			switch d := d.(type) {
+			case *ast.FuncDecl:
+				add(d.Name.Name, d.Doc, d.Pos())
+			case *ast.GenDecl:
+				// A grouped block is one declaration: its doc describes the
+				// block, which conventionally means its first name.
+				for si, spec := range d.Specs {
+					var name string
+					switch spec := spec.(type) {
+					case *ast.TypeSpec:
+						name = spec.Name.Name
+					case *ast.ValueSpec:
+						if len(spec.Names) > 0 {
+							name = spec.Names[0].Name
+						}
+					}
+					if name == "" {
+						continue
+					}
+					if si == 0 {
+						add(name, d.Doc, d.Pos())
+					} else {
+						add(name, nil, spec.Pos())
+					}
+				}
+			}
+		}
+		return nil
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(decls) < 500 {
+		t.Fatalf("only %d declarations found; this guard is not looking at the right tree", len(decls))
+	}
+
+	documented, flagged := 0, 0
+	for i, d := range decls {
+		if d.doc == "" {
+			continue
+		}
+		documented++
+		if i+1 >= len(decls) || decls[i+1].file != d.file {
+			continue
+		}
+		first, _, _ := strings.Cut(strings.TrimSpace(d.doc), " ")
+		first = strings.TrimRight(first, ".,:")
+		if first == d.name || first != decls[i+1].name {
+			continue
+		}
+		flagged++
+		t.Errorf("%s:%d: the doc comment on %s describes %s, which is declared immediately below it\n"+
+			"a declaration was inserted between that comment and what it documents",
+			d.file, d.line, d.name, first)
+	}
+	t.Logf("checked %d declarations, %d documented, %d flagged", len(decls), documented, flagged)
+}

internal/ingest/server.go

@@ -373,16 +373,18 @@ func toJSON(v interface{}) string {
 	return string(b)
 }
 
-// attrsJSON flattens an OTLP attribute list into a flat JSON object keyed by the
-// literal (dotted) attribute name, e.g. {"http.method":"GET","http.status_code":200}.
-// This is the shape the attr() macro and json_extract_string(col, '$."key"') paths
-// expect. Marshaling the raw []*KeyValue (as the old toJSON path did) produced an
-// array of {Key,Value} structs that no JSON-path query could read. Returns nil for
-// an empty list so the column stays NULL.
+// attrBufPool holds the buffers attrsJSON's fast path writes into.
 var attrBufPool = sync.Pool{New: func() any { return new(bytes.Buffer) }}
 
-// attrsJSON flattens an OTLP attribute list into a flat JSON object. The fast
-// path writes the object directly into a pooled buffer, skipping the
+// attrsJSON flattens an OTLP attribute list into a flat JSON object keyed by
+// the literal (dotted) attribute name, e.g.
+// {"http.method":"GET","http.status_code":200}. That is the shape the attr()
+// macro and json_extract_string(col, '$."key"') paths expect; marshaling the
+// raw []*KeyValue, as the old toJSON path did, produced an array of
+// {Key,Value} structs that no JSON-path query could read. Returns "" for an
+// empty list so the column stays NULL.
+//
+// The fast path writes the object directly into a pooled buffer, skipping the
 // map[string]any + interface boxing + reflection that json.Marshal needs — that
 // path dominated ingest allocations under load (profiled: ~7GB / 14% of
 // alloc_space at 175k rows/s). Attributes whose values are nested (array/kvlist)

internal/intelligence/detector.go

@@ -148,37 +148,61 @@ func (d *Detector) detectAnomalies(ctx context.Context, start, end time.Time) []
 	return anomalies
 }
 
-// detectErrorRateAnomalies detects error rate spikes
-func (d *Detector) detectErrorRateAnomalies(ctx context.Context, start, end time.Time) []Anomaly {
-	startNano := start.UnixNano()
-	endNano := end.UnixNano()
-	namespace := d.duck.DefaultNamespace()
-	scope := detectorScopeClause(namespace)
+// minErrorRateStddev floors the error-rate z-score denominator at one
+// percentage point. See errorRateAnomalySQL.
+const minErrorRateStddev = 0.01
 
-	// Compare current error rate to baseline (previous period)
-	sql := fmt.Sprintf(`
+// errorRateAnomalySQL compares the error rate in [startNano, endNano) against
+// the window of equal length immediately before it.
+//
+// The z-score divides a difference of rates, so its denominator has to be the
+// scatter of the RATE from bucket to bucket. It used to be
+// STDDEV(CASE WHEN status = error THEN 1.0 ELSE 0.0 END) over raw spans, which
+// is the scatter of individual span outcomes -- sqrt(p(1-p)), about 0.34 at a
+// 14% error rate. Those are different quantities, and the mismatch punished
+// exactly the services worth watching: the noisier the service, the larger the
+// denominator, so at the 13.8% baseline the live demo ran, clearing the 2.0
+// threshold needed the rate to jump 69 percentage points. A tripling to 41%
+// scored 0.80.
+//
+// The denominator is floored because a healthy service's rate is flat at zero,
+// giving it a bucket-to-bucket stddev of exactly zero -- so the service that
+// just started failing was the one that could not alert (z=0.00 going from no
+// errors to 42%). One percentage point is the least noise worth assuming: it
+// keeps a single stray error in a few thousand spans below the threshold while
+// letting a real break through.
+func errorRateAnomalySQL(startNano, endNano int64, scope string) string {
+	return fmt.Sprintf(`
 		WITH current_period AS (
 			SELECT
-				service as service_name,
-				COUNT(*) FILTER (WHERE status IN ('STATUS_CODE_ERROR', 'ERROR')) AS error_count,
-				COUNT(*) AS total_count,
-				(COUNT(*) FILTER (WHERE status IN ('STATUS_CODE_ERROR', 'ERROR'))::DOUBLE / COUNT(*)::DOUBLE) AS error_rate
-			FROM spans
-			WHERE start_unix_nano >= %d AND start_unix_nano < %d
-			%s
-			GROUP BY service
+				service_name,
+				AVG(bucket_rate) AS error_rate
+			FROM (
+				SELECT
+					service as service_name,
+					(COUNT(*) FILTER (WHERE status IN ('STATUS_CODE_ERROR', 'ERROR'))::DOUBLE / COUNT(*)::DOUBLE) AS bucket_rate
+				FROM spans
+				WHERE start_unix_nano >= %d AND start_unix_nano < %d
+				%s
+				GROUP BY service, time_bucket(INTERVAL '5 minutes', start_time)
+			) buckets
+			GROUP BY service_name
 		),
 		baseline_period AS (
 			SELECT
-				service as service_name,
-				COUNT(*) FILTER (WHERE status IN ('STATUS_CODE_ERROR', 'ERROR')) AS error_count,
-				COUNT(*) AS total_count,
-				(COUNT(*) FILTER (WHERE status IN ('STATUS_CODE_ERROR', 'ERROR'))::DOUBLE / COUNT(*)::DOUBLE) AS error_rate,
-				STDDEV(CASE WHEN status IN ('STATUS_CODE_ERROR', 'ERROR') THEN 1.0 ELSE 0.0 END) AS error_stddev
-			FROM spans
-			WHERE start_unix_nano >= %d AND start_unix_nano < %d
-			%s
-			GROUP BY service
+				service_name,
+				AVG(bucket_rate) AS error_rate,
+				GREATEST(COALESCE(STDDEV(bucket_rate), 0.0), %f) AS error_stddev
+			FROM (
+				SELECT
+					service as service_name,
+					(COUNT(*) FILTER (WHERE status IN ('STATUS_CODE_ERROR', 'ERROR'))::DOUBLE / COUNT(*)::DOUBLE) AS bucket_rate
+				FROM spans
+				WHERE start_unix_nano >= %d AND start_unix_nano < %d
+				%s
+				GROUP BY service, time_bucket(INTERVAL '5 minutes', start_time)
+			) buckets
+			GROUP BY service_name
 		)
 		SELECT
 			c.service_name,
@@ -191,7 +215,18 @@ func (d *Detector) detectErrorRateAnomalies(ctx context.Context, start, end time
 		FROM current_period c
 		LEFT JOIN baseline_period b ON c.service_name = b.service_name
 		WHERE c.error_rate > 0
-	`, startNano, endNano, scope, startNano-endNano+startNano, startNano, scope)
+	`, startNano, endNano, scope, minErrorRateStddev, startNano-endNano+startNano, startNano, scope)
+}
+
+// detectErrorRateAnomalies detects error rate spikes
+func (d *Detector) detectErrorRateAnomalies(ctx context.Context, start, end time.Time) []Anomaly {
+	startNano := start.UnixNano()
+	endNano := end.UnixNano()
+	namespace := d.duck.DefaultNamespace()
+	scope := detectorScopeClause(namespace)
+
+	// Compare current error rate to baseline (previous period)
+	sql := errorRateAnomalySQL(startNano, endNano, scope)
 
 	resp := d.duck.ExecuteSQL(ctx, query.SQLRequest{Query: sql})
 	if resp.Error != "" {