target: track explicit test cache counts

A user reported that the test grid could show cached tests as
non-cached. That happened because test target history inferred cache
state from timestamps and flattened multiple test attempts down to a
lossy bool.

This change keeps target history test-only, adds explicit local,
remote, and total cached attempt counts to ClickHouse-backed test
history, and updates the TAP grid to derive full versus partial
cache state from those fields. The primary DB path stays on the legacy
bool, and cached remains in TargetStatus as a fallback for older
ClickHouse rows that do not have the new count columns yet.

This requires an additive ClickHouse migration for TestTargetStatuses.
Older rows still render via the legacy bool and timestamp heuristic
until they are rewritten, while new rows carry the full cache-attempt
semantics.

Author: sluongng

enterprise/app/tap/grid.tsx

@@ -79,6 +79,15 @@ interface Stat {
   cached: number;
 }
 
+interface CacheInfo {
+  fullyCached: boolean;
+  partiallyCached: boolean;
+  cachedCount: number;
+  cachedLocallyCount: number;
+  cachedRemotelyCount: number;
+  totalRunCount: number;
+}
+
 const Status = api.v1.Status;
 
 const MIN_OPACITY = 0.1;
@@ -174,6 +183,7 @@ export default class TestGridComponent extends React.Component<Props, State> {
     for (let targetHistory of histories) {
       let stats: Stat = { count: 0, pass: 0, totalDuration: 0, maxDuration: 0, avgDuration: 0, flake: 0, cached: 0 };
       for (let status of targetHistory.targetStatus) {
+        const cacheInfo = getCacheInfo(status);
         stats.count += 1;
         let duration = this.durationToNum(status.timing?.duration || undefined);
         stats.totalDuration += duration;
@@ -183,7 +193,7 @@ export default class TestGridComponent extends React.Component<Props, State> {
         } else if (status.status == Status.FLAKY) {
           stats.flake += 1;
         }
-        if (isCached(status)) {
+        if (cacheInfo.fullyCached) {
           stats.cached += 1;
         }
       }
@@ -525,6 +535,7 @@ export default class TestGridComponent extends React.Component<Props, State> {
                       return (
                         <div className="tap-commit-container">
                           {statuses.map((status) => {
+                            const cacheInfo = getCacheInfo(status);
                             let destinationUrl = `/invocation/${status.invocationId}?${new URLSearchParams({
                               target: targetHistory.target?.label || "",
                               targetStatus: String(status),
@@ -536,9 +547,9 @@ export default class TestGridComponent extends React.Component<Props, State> {
                               title += ` at commit ${commitStatus.commitSha}`;
                             }
 
-                            let cached = isCached(status);
-                            if (cached) {
-                              title += ` (cached)`;
+                            const cacheLabel = describeCacheStatus(cacheInfo);
+                            if (cacheLabel) {
+                              title += ` (${cacheLabel})`;
                             }
 
                             return (
@@ -560,7 +571,7 @@ export default class TestGridComponent extends React.Component<Props, State> {
                                 className={`tap-block ${
                                   this.getColorMode() == "status" ? `status-${status.status}` : "timing"
                                 }
-                                ${cached ? `cached` : ""}
+                                ${cacheInfo.fullyCached ? `cached` : ""}
                                 clickable`}>
                                 {this.statusToIcon(status.status || Status.STATUS_UNSPECIFIED)}
                               </a>
@@ -584,7 +595,68 @@ export default class TestGridComponent extends React.Component<Props, State> {
   }
 }
 
-function isCached(status: target.ITargetStatus) {
+function getCacheInfo(status: target.ITargetStatus): CacheInfo {
+  const cachedLocallyCount = Number(status.cachedLocallyCount || 0);
+  const cachedRemotelyCount = Number(status.cachedRemotelyCount || 0);
+  const cachedCount = Math.max(Number(status.cachedCount || 0), cachedLocallyCount + cachedRemotelyCount);
+  const explicitTotalRunCount = Number(status.totalRunCount || 0);
+
+  if (explicitTotalRunCount > 0 || cachedCount > 0 || cachedLocallyCount > 0 || cachedRemotelyCount > 0) {
+    const totalRunCount = Math.max(explicitTotalRunCount, cachedCount);
+    // When explicit counts are available, use them as the source of truth. The
+    // legacy cached bool is only a fallback for older rows without counts.
+    const fullyCached = explicitTotalRunCount > 0 ? cachedCount === totalRunCount : Boolean(status.cached);
+    return {
+      fullyCached,
+      partiallyCached: !fullyCached && cachedCount > 0,
+      cachedCount,
+      cachedLocallyCount,
+      cachedRemotelyCount,
+      totalRunCount,
+    };
+  }
+
+  // Older ClickHouse rows only have the legacy cached bool, so preserve the
+  // historical fallback instead of treating missing counts as uncached.
+  const fullyCached = Boolean(status.cached) || isHeuristicallyCached(status);
+  return {
+    fullyCached,
+    partiallyCached: false,
+    cachedCount: fullyCached ? 1 : 0,
+    cachedLocallyCount: 0,
+    cachedRemotelyCount: 0,
+    totalRunCount: fullyCached ? 1 : 0,
+  };
+}
+
+function describeCacheStatus(cacheInfo: CacheInfo): string {
+  if (!cacheInfo.fullyCached && !cacheInfo.partiallyCached) {
+    return "";
+  }
+
+  const originParts = [];
+  if (cacheInfo.cachedLocallyCount > 0) {
+    originParts.push(`${cacheInfo.cachedLocallyCount} local`);
+  }
+  if (cacheInfo.cachedRemotelyCount > 0) {
+    originParts.push(`${cacheInfo.cachedRemotelyCount} remote`);
+  }
+  const originSuffix = originParts.length ? ` (${originParts.join(", ")})` : "";
+
+  if (cacheInfo.fullyCached) {
+    if (cacheInfo.cachedLocallyCount > 0 && cacheInfo.cachedLocallyCount === cacheInfo.totalRunCount) {
+      return "cached locally";
+    }
+    if (cacheInfo.cachedRemotelyCount > 0 && cacheInfo.cachedRemotelyCount === cacheInfo.totalRunCount) {
+      return "cached remotely";
+    }
+    return `cached${originSuffix}`;
+  }
+
+  return `${cacheInfo.cachedCount}/${cacheInfo.totalRunCount} cached${originSuffix}`;
+}
+
+function isHeuristicallyCached(status: target.ITargetStatus) {
   return (
     +(status.timing?.startTime?.seconds || 0) > 0 &&
     Math.floor(+(status.invocationCreatedAtUsec || 0) / 1000000) > +(status.timing?.startTime?.seconds || 0)

proto/target.proto

@@ -31,6 +31,9 @@ message TargetMetadata {
   api.v1.TestSize test_size = 5;
 }
 
+// Status for a target within target history. Today GetTargetHistory only
+// stores test targets, so the cache fields below describe test attempts rather
+// than generic build actions.
 message TargetStatus {
   // The invocation identifier itself.
   string invocation_id = 1;
@@ -51,6 +54,30 @@ message TargetStatus {
 
   // When the invocation was created.
   int64 invocation_created_at_usec = 5;
+
+  // Whether all recorded test attempts for this target invocation were served
+  // from cache. For new ClickHouse rows, this legacy field is derived from
+  // total_run_count > 0 && cached_count == total_run_count. Older rows that
+  // predate the explicit cache counts have total_run_count == 0, so clients can
+  // fall back to this bool.
+  bool cached = 6;
+
+  // The number of cached test attempts recorded for this target invocation,
+  // including both local and remote cache hits. A non-zero cached_count does
+  // not imply that all attempts were cached; compare against total_run_count.
+  int32 cached_count = 7;
+
+  // The number of cached test attempts served from the local cache.
+  int32 cached_locally_count = 8;
+
+  // The number of cached test attempts served from the remote cache.
+  int32 cached_remotely_count = 9;
+
+  // The total number of cached and uncached test shard attempts recorded for
+  // this target invocation. This comes from Bazel TestSummary.total_run_count;
+  // compare cached_count against it to determine whether all attempts were
+  // cached.
+  int32 total_run_count = 10;
 }
 
 message TargetHistory {

server/build_event_protocol/target_tracker/target_tracker.go

@@ -49,18 +49,23 @@ const (
 )
 
 type target struct {
-	label          string
-	aspect         string
-	ruleType       string
-	firstStartTime time.Time
-	totalDuration  time.Duration
-	state          targetState
-	id             int64
-	overallStatus  build_event_stream.TestStatus
-	cached         bool
-	targetType     cmpb.TargetType
-	testSize       build_event_stream.TestSize
-	buildSuccess   bool
+	label               string
+	aspect              string
+	ruleType            string
+	firstStartTime      time.Time
+	totalDuration       time.Duration
+	state               targetState
+	id                  int64
+	overallStatus       build_event_stream.TestStatus
+	cached              bool
+	cachedCount         int32
+	cachedLocallyCount  int32
+	cachedRemotelyCount int32
+	totalRunCount       int32
+	observedResultCount int32
+	targetType          cmpb.TargetType
+	testSize            build_event_stream.TestSize
+	buildSuccess        bool
 }
 
 func md5Int64(text string) int64 {
@@ -76,6 +81,23 @@ func newTarget(label string, aspect string) *target {
 	}
 }
 
+func (t *target) effectiveCachedCount() int32 {
+	count := t.cachedCount
+	if observed := t.cachedLocallyCount + t.cachedRemotelyCount; observed > count {
+		count = observed
+	}
+	return count
+}
+
+func (t *target) effectiveTotalRunCount(ctx context.Context) int32 {
+	count := t.totalRunCount
+	if t.observedResultCount > count {
+		log.CtxDebugf(ctx, "Using observed TestResult count for target %q because it exceeds TestSummary total_run_count (%d > %d)", t.label, t.observedResultCount, t.totalRunCount)
+		count = t.observedResultCount
+	}
+	return count
+}
+
 func getTestStatus(aborted *build_event_stream.Aborted) build_event_stream.TestStatus {
 	switch aborted.GetReason() {
 	case build_event_stream.Aborted_USER_INTERRUPTED:
@@ -99,11 +121,22 @@ func (t *target) updateFromEvent(event *build_event_stream.BuildEvent) {
 		}
 		t.state = targetStateCompleted
 	case *build_event_stream.BuildEvent_TestResult:
-		t.cached = p.TestResult.GetCachedLocally() || p.TestResult.GetExecutionInfo().GetCachedRemotely()
+		cachedLocally := p.TestResult.GetCachedLocally()
+		cachedRemotely := p.TestResult.GetExecutionInfo().GetCachedRemotely()
+		t.cached = cachedLocally || cachedRemotely
+		t.observedResultCount++
+		if cachedLocally {
+			t.cachedLocallyCount++
+		}
+		if cachedRemotely {
+			t.cachedRemotelyCount++
+		}
 		t.state = targetStateResult
 	case *build_event_stream.BuildEvent_TestSummary:
 		ts := p.TestSummary
 		t.overallStatus = ts.GetOverallStatus()
+		t.cachedCount = ts.GetTotalNumCached()
+		t.totalRunCount = ts.GetTotalRunCount()
 		t.firstStartTime = timeutil.GetTimeWithFallback(ts.GetFirstStartTime(), ts.GetFirstStartTimeMillis())
 		t.totalDuration = timeutil.GetDurationWithFallback(ts.GetTotalRunDuration(), ts.GetTotalRunDurationMillis())
 		t.state = targetStateSummary
@@ -356,25 +389,32 @@ func (t *TargetTracker) writeTestTargetStatusesToOLAPDB(ctx context.Context, per
 			testStartTimeUsec = target.firstStartTime.UnixMicro()
 		}
 
+		cachedCount := target.effectiveCachedCount()
+		totalRunCount := target.effectiveTotalRunCount(ctx)
+
 		entries = append(entries, &schema.TestTargetStatus{
 			GroupID:                 permissions.GroupID,
 			RepoURL:                 repoURL,
 			CommitSHA:               commitSHA,
 			Label:                   target.label,
 			InvocationStartTimeUsec: invocationStartTime.UnixMicro(),
 
-			RuleType:       target.ruleType,
-			UserID:         permissions.UserID,
-			InvocationUUID: invocationUUID,
-			TargetType:     int32(target.targetType),
-			TestSize:       int32(target.testSize),
-			Status:         int32(target.overallStatus),
-			Cached:         target.cached,
-			StartTimeUsec:  testStartTimeUsec,
-			DurationUsec:   target.totalDuration.Microseconds(),
-			BranchName:     t.buildEventAccumulator.Invocation().GetBranchName(),
-			Role:           t.buildEventAccumulator.Invocation().GetRole(),
-			Command:        t.buildEventAccumulator.Invocation().GetCommand(),
+			RuleType:            target.ruleType,
+			UserID:              permissions.UserID,
+			InvocationUUID:      invocationUUID,
+			TargetType:          int32(target.targetType),
+			TestSize:            int32(target.testSize),
+			Status:              int32(target.overallStatus),
+			Cached:              totalRunCount > 0 && cachedCount == totalRunCount,
+			CachedCount:         cachedCount,
+			CachedLocallyCount:  target.cachedLocallyCount,
+			CachedRemotelyCount: target.cachedRemotelyCount,
+			TotalRunCount:       totalRunCount,
+			StartTimeUsec:       testStartTimeUsec,
+			DurationUsec:        target.totalDuration.Microseconds(),
+			BranchName:          t.buildEventAccumulator.Invocation().GetBranchName(),
+			Role:                t.buildEventAccumulator.Invocation().GetRole(),
+			Command:             t.buildEventAccumulator.Invocation().GetCommand(),
 		})
 	}
 	err := t.env.GetOLAPDBHandle().FlushTestTargetStatuses(ctx, entries)