Preserve Patrol finding volume beyond the history cap

New-finding telemetry lost older activity once the run history reached its
100-entry cap. Persist a bounded daily finding tally with a separate
upgrade cursor, preserving run counts while backfilling retained findings.

Cover restart, repeated saves, upgrade, read failure and UTC-day retention.
Record the measurement boundary and retire the resolved coverage gap.
This commit is contained in:
rcourtman
2026-09-05 19:48:49 +01:00
parent 57dd3a284a
commit 9316507df0
6 changed files with 171 additions and 78 deletions
@@ -10167,48 +10167,6 @@
}
]
},
{
"id": "patrol-investigation-rate-metric-invalid",
"summary": "The Patrol investigation rate (pulse_intelligence_patrol_investigations_30d over pulse_intelligence_patrol_new_findings_30d), which TELEMETRY_SIGNALS.md instructs every reader to report, is not a rate and must not be trended. Four independent reasons, all verified in pkg/server/telemetry_pulse_intelligence.go: the denominator sums run.NewFindings over history.Runs, which SavePatrolRunHistory caps at MaxPatrolRunHistory (100), so it covers at most the last hundred runs rather than thirty days and silently under-reports for any install patrolling faster than that (355 of 449 Patrol-enabled installs in the 6.4 upgrade cohort on 2026-09-03); the numerator instead scans the current findings store and counts surviving finding records investigated in-window, including findings created before it, which is how the paid cohort read 128.57 percent in the week to 2026-08-25; the populations are disjoint because Finding.ShouldInvestigate returns false at monitor autonomy and effective autonomy is licence-gated, so free installs produced 4384 findings and 1 investigation (0.02 percent) in the week to 2026-09-03 while 67 paid installs produced 242; and only 29 clean installs reported any investigation at all, one of which swung the fleet total by 38. The three-week decline that prompted this (7.2 to 5.0 percent) is composition, not regression: the rate is flat in version-stable installs (3.48, 3.39, 3.78), fleet investigations rose once that single install is excluded (170, 213, 225), finding-detection code is identical between v6.3.2 and v6.4.1, and the patrol interval is unchanged at 360 minutes across 6.1 through 6.4. The reported run growth is likewise the 63c40ebe5e daily tally back-filling, not more patrolling. This also corrects the telemetry inference in patrol-findings-hygiene-attention-noise: findings are not mostly 'seen and ignored', they are raised on installs that are licence-gated out of investigating at all, which leaves that gap's independently evidenced UX work (discussions 1623 and 1699) standing on its own footing. Two defects follow: new_findings_30d saturates on the very history cap that 63c40ebe5e fixed for runs_30d in the same function, needing a per-day findings tally alongside DailyRuns; and the blessed doc tells every reader to compute the invalid ratio. The real signal the ratio was hiding is that paid Patrol installs fell from 76 to 67 while free Patrol grew from 888 to 962.",
"owner": "project-owner",
"status": "triaged",
"recorded_at": "2026-09-03",
"lane_ids": [
"L6"
],
"subsystem_ids": [
"patrol-intelligence"
],
"proposed_resolution": "lane-expansion",
"coverage_impact": 2,
"evidence": [
{
"repo": "pulse",
"path": "internal/ai/findings.go",
"kind": "file"
},
{
"repo": "pulse",
"path": "internal/ai/patrol.go",
"kind": "file"
},
{
"repo": "pulse",
"path": "pkg/server/telemetry_pulse_intelligence.go",
"kind": "file"
},
{
"repo": "pulse",
"path": "pkg/server/telemetry_pulse_intelligence_test.go",
"kind": "file"
},
{
"repo": "pulse-pro",
"path": "docs/TELEMETRY_SIGNALS.md",
"kind": "file"
}
]
},
{
"id": "telemetry-test-binary-production-pings",
"summary": "Go test binaries reported themselves to the production telemetry receiver as live installations. pkg/server tests boot the real server through Run() with the version literal \"test-version\", which internal/updates normalizes to 0.0.0-test-version, and each test runs against its own t.TempDir(), so every run minted a fresh install ID. The startup ping waits two minutes and never fired in a short test, but the service-health failure reporter added on 2026-08-29 sends synchronously from a deferred handler as soon as Run() returns an error, so every test run that exercised a startup failure posted one ping. The receiver recorded 317 single-ping installs between 2026-08-29 and 2026-09-03, 311 from linux/amd64 hosts (dominated by the autonomous maintainer fleet running ad-hoc go test, not GitHub Actions, which ran twice in the final 24h) and 3 from a maintainer workstation. The canonical clean denominator excludes single-ping installs and was unaffected, but raw install counts and the operator-evidence Patrol blocked-cause read counted them as real installations. Resolved by refusing production-endpoint sends from a test binary in internal/telemetry, opting the server tests out of telemetry, and putting the operator-evidence read on the production-ping basis. The emitter's development flag was wrong too and is now fixed: it was set only for git build metadata or a prerelease of exactly dev or dev.*, so 0.0.0-test-version and 0.0.0-dev-pro both arrived with version_is_development clear. The 0.0.0 sentinel now decides the channel, so builds reporting after 2026-09-04 carry version_is_development true. Rows already stored keep the values they were sent with and are not backfilled, so any read whose window reaches earlier must still filter on version_is_published_release.",
@@ -2474,6 +2474,18 @@ as investigated, one bucket per finding, and add no finding, resource,
session, or action identity. The receiver canonicalizes every one of the four
strings to its released vocabulary or `unknown` and clamps the counts like
every other counter.
Patrol run and new-finding volumes use bounded local UTC-day tallies in
`internal/config/persistence.go`, preserving aggregate activity after the
operator-facing run history is trimmed. Finding volume has its own persistence
cursor so installations with an existing run tally backfill retained runs on
upgrade without recounting run volume. No run, finding, resource, provider, or
session identity is added to the tally or outbound ping. The rolling totals
include the whole UTC day containing the cutoff. Activity trimmed before the
finding tally existed cannot be recovered. This changes retention of existing
aggregate fields and does not expand the telemetry schema. Investigation
counts remain one per surviving finding investigated in-window, with one
current outcome bucket per finding. They do not count repeated attempts or
form a conversion denominator for newly detected findings.
The earlier draft schema-v8 `business_estate` field was reverted and must not
remain in the license server's accepted ping struct merely because a private receiver build
and database migration briefly carried it. Existing deployed databases need no
+99 -3
View File
@@ -1,7 +1,9 @@
package config
import (
"encoding/json"
"errors"
"os"
"strings"
"testing"
"time"
@@ -110,7 +112,7 @@ func TestPatrolRunTallyReadErrorWarnsAndKeepsSaving(t *testing.T) {
// Seed history whose tally holds a run that has already fallen off the
// capped run list: only a preserved tally would still count it.
runA := PatrolRunRecord{ID: "a", StartedAt: now.Add(-2 * time.Hour), CompletedAt: now.Add(-2 * time.Hour)}
runA := PatrolRunRecord{ID: "a", NewFindings: 2, StartedAt: now.Add(-2 * time.Hour), CompletedAt: now.Add(-2 * time.Hour)}
if err := p.SavePatrolRunHistory([]PatrolRunRecord{runA}); err != nil {
t.Fatalf("SavePatrolRunHistory seed: %v", err)
}
@@ -119,11 +121,11 @@ func TestPatrolRunTallyReadErrorWarnsAndKeepsSaving(t *testing.T) {
mfs := &mockFSError{FileSystem: defaultFileSystem{}, readError: errors.New("transient read failure")}
p.SetFileSystem(mfs)
runB := PatrolRunRecord{ID: "b", StartedAt: now.Add(-1 * time.Hour), CompletedAt: now.Add(-1 * time.Hour)}
runB := PatrolRunRecord{ID: "b", NewFindings: 3, StartedAt: now.Add(-1 * time.Hour), CompletedAt: now.Add(-1 * time.Hour)}
if err := p.SavePatrolRunHistory([]PatrolRunRecord{runB}); err != nil {
t.Fatalf("SavePatrolRunHistory with failing read must still save: %v", err)
}
if !strings.Contains(logs.String(), "daily run tally restarts") {
if !strings.Contains(logs.String(), "daily run and finding tallies restart") {
t.Fatalf("no tally-restart warning was logged; logs: %s", logs.String())
}
@@ -139,4 +141,98 @@ func TestPatrolRunTallyReadErrorWarnsAndKeepsSaving(t *testing.T) {
if got := loaded.PatrolRunsSince(now.AddDate(0, 0, -30)); got != 1 {
t.Fatalf("PatrolRunsSince = %d, want 1 after the tally restart", got)
}
if got := loaded.PatrolNewFindingsSince(now.AddDate(0, 0, -30)); got != 3 {
t.Fatalf("PatrolNewFindingsSince = %d, want 3 after the tally restart", got)
}
}
// Upgrading a persisted run-only tally must backfill retained findings even
// when every retained run is already behind the run tally's cursor. Once
// saved, that finding volume must survive restart, trimming and repeated saves.
func TestPatrolNewFindingsTallyUpgradeAndRestart(t *testing.T) {
dir := t.TempDir()
p := NewConfigPersistence(dir)
now := time.Now().UTC()
since := now.AddDate(0, 0, -30)
at := now.Add(-time.Hour)
legacy := PatrolRunHistoryData{
Version: 1,
Runs: []PatrolRunRecord{
{ID: "b", CompletedAt: at, NewFindings: 3},
{ID: "a", StartedAt: at.Add(-time.Hour), NewFindings: 2},
},
DailyRuns: map[string]int{PatrolRunTallyDayKey(at): 150},
RunTallyThrough: at,
}
encoded, err := json.Marshal(legacy)
if err != nil {
t.Fatal(err)
}
if err := os.WriteFile(p.aiPatrolRunsFile, encoded, 0600); err != nil {
t.Fatal(err)
}
loaded, err := p.LoadPatrolRunHistory()
if err != nil {
t.Fatal(err)
}
if got := loaded.PatrolNewFindingsSince(since); got != 5 {
t.Fatalf("legacy history fallback = %d, want 5", got)
}
if err := p.SavePatrolRunHistory(loaded.Runs); err != nil {
t.Fatal(err)
}
// A new persistence instance must carry both tallies from disk after the
// original source records are dropped. Zero-finding runs advance too.
p = NewConfigPersistence(dir)
next := []PatrolRunRecord{
{ID: "d", CompletedAt: now, NewFindings: 0},
{ID: "c", CompletedAt: now.Add(-time.Minute), NewFindings: 4},
}
for i := 0; i < 3; i++ {
if err := p.SavePatrolRunHistory(next); err != nil {
t.Fatal(err)
}
}
loaded, err = p.LoadPatrolRunHistory()
if err != nil {
t.Fatal(err)
}
if got := loaded.PatrolRunsSince(since); got != 152 {
t.Fatalf("run tally after upgrade = %d, want 152", got)
}
if got := loaded.PatrolNewFindingsSince(since); got != 9 {
t.Fatalf("finding tally after restart and repeated saves = %d, want 9", got)
}
if !loaded.NewFindingsTallyThrough.Equal(now) {
t.Fatalf("finding cursor = %s, want %s", loaded.NewFindingsTallyThrough, now)
}
}
func TestPatrolNewFindingsTallyWindowAndRetention(t *testing.T) {
now := time.Date(2026, 9, 5, 12, 0, 0, 0, time.UTC)
since := now.AddDate(0, 0, -30)
runs := []PatrolRunRecord{
{CompletedAt: since.Add(-time.Hour), NewFindings: 2},
{StartedAt: since, NewFindings: 3},
{CompletedAt: now.AddDate(0, 0, -32), NewFindings: 100},
{CompletedAt: now, NewFindings: -1},
{NewFindings: 100},
}
data := &PatrolRunHistoryData{Runs: runs}
if got := data.PatrolNewFindingsSince(since); got != 3 {
t.Fatalf("history fallback = %d, want 3", got)
}
advancePatrolRunTally(data, runs, now)
data.Runs = nil
if got := data.PatrolNewFindingsSince(since); got != 5 {
t.Fatalf("UTC-day tally = %d, want 5 including cutoff day", got)
}
if len(data.DailyNewFindings) != 1 {
t.Fatalf("tally contains expired or invalid findings: %v", data.DailyNewFindings)
}
var absent *PatrolRunHistoryData
if got := absent.PatrolNewFindingsSince(since); got != 0 {
t.Fatalf("nil history = %d, want 0", got)
}
}
+50 -6
View File
@@ -2783,6 +2783,11 @@ type PatrolRunHistoryData struct {
// cannot answer "how often did Patrol actually run". This tally is the
// uncapped answer and costs one small integer per retained day.
DailyRuns map[string]int `json:"daily_runs,omitempty"`
// DailyNewFindings retains finding volume after its source runs leave the
// operator-facing history. Its separate cursor backfills retained history
// when upgrading an installation that already has a run tally.
DailyNewFindings map[string]int `json:"daily_new_findings,omitempty"`
NewFindingsTallyThrough time.Time `json:"new_findings_tally_through,omitempty"`
// RunTallyThrough is the newest run observation already folded into
// DailyRuns. History is persisted as a full newest-first list on every
// debounced save, so the tally advances from this high-water mark rather
@@ -2819,18 +2824,31 @@ func advancePatrolRunTally(data *PatrolRunHistoryData, runs []PatrolRunRecord, n
if data.DailyRuns == nil {
data.DailyRuns = make(map[string]int, patrolRunTallyRetentionDays)
}
if data.DailyNewFindings == nil {
data.DailyNewFindings = make(map[string]int, patrolRunTallyRetentionDays)
}
newest := data.RunTallyThrough
newestFindings := data.NewFindingsTallyThrough
for _, run := range runs {
observedAt := patrolRunObservedAt(run)
if observedAt.IsZero() || !observedAt.After(data.RunTallyThrough) {
if observedAt.IsZero() {
continue
}
data.DailyRuns[PatrolRunTallyDayKey(observedAt)]++
if observedAt.After(data.RunTallyThrough) {
data.DailyRuns[PatrolRunTallyDayKey(observedAt)]++
}
if observedAt.After(data.NewFindingsTallyThrough) && run.NewFindings > 0 {
data.DailyNewFindings[PatrolRunTallyDayKey(observedAt)] += run.NewFindings
}
if observedAt.After(newest) {
newest = observedAt
}
if observedAt.After(newestFindings) {
newestFindings = observedAt
}
}
data.RunTallyThrough = newest
data.NewFindingsTallyThrough = newestFindings
cutoff := PatrolRunTallyDayKey(now.UTC().AddDate(0, 0, -patrolRunTallyRetentionDays))
for day := range data.DailyRuns {
@@ -2838,6 +2856,11 @@ func advancePatrolRunTally(data *PatrolRunHistoryData, runs []PatrolRunRecord, n
delete(data.DailyRuns, day)
}
}
for day := range data.DailyNewFindings {
if day < cutoff {
delete(data.DailyNewFindings, day)
}
}
}
// PatrolRunsSince counts patrol runs observed at or after since. It prefers the
@@ -2849,17 +2872,33 @@ func (data *PatrolRunHistoryData) PatrolRunsSince(since time.Time) int {
if data == nil {
return 0
}
return data.patrolActivitySince(since, data.DailyRuns, func(PatrolRunRecord) int { return 1 })
}
// PatrolNewFindingsSince has the same UTC-day window and retained-history
// fallback as PatrolRunsSince. It cannot recover findings from runs that were
// trimmed before the first finding tally was saved.
func (data *PatrolRunHistoryData) PatrolNewFindingsSince(since time.Time) int {
if data == nil {
return 0
}
return data.patrolActivitySince(since, data.DailyNewFindings, func(run PatrolRunRecord) int {
return max(0, run.NewFindings)
})
}
func (data *PatrolRunHistoryData) patrolActivitySince(since time.Time, daily map[string]int, countRun func(PatrolRunRecord) int) int {
fromHistory := 0
for _, run := range data.Runs {
observedAt := patrolRunObservedAt(run)
if observedAt.IsZero() || observedAt.Before(since) {
continue
}
fromHistory++
fromHistory += countRun(run)
}
fromTally := 0
sinceDay := PatrolRunTallyDayKey(since)
for day, count := range data.DailyRuns {
for day, count := range daily {
if day >= sinceDay {
fromTally += count
}
@@ -3697,7 +3736,7 @@ func (c *ConfigPersistence) LoadWorkflowPromptActivityHistory() (*WorkflowPrompt
}
// SavePatrolRunHistory persists patrol run history to disk, carrying the
// uncapped daily run tally forward across the capped run list it replaces.
// daily run and finding tallies forward across the capped run list it replaces.
func (c *ConfigPersistence) SavePatrolRunHistory(runs []PatrolRunRecord) error {
now := time.Now()
data := PatrolRunHistoryData{
@@ -3709,10 +3748,12 @@ func (c *ConfigPersistence) SavePatrolRunHistory(runs []PatrolRunRecord) error {
// a real read failure. The tally is telemetry, so keep the save — but say
// the tally is restarting rather than resetting it silently.
if existing, err := c.LoadPatrolRunHistory(); err != nil {
log.Warn().Err(err).Str("file", c.aiPatrolRunsFile).Msg("Failed to read existing patrol run history; daily run tally restarts from this save")
log.Warn().Err(err).Str("file", c.aiPatrolRunsFile).Msg("Failed to read existing patrol run history; daily run and finding tallies restart from this save")
} else if existing != nil {
data.DailyRuns = existing.DailyRuns
data.RunTallyThrough = existing.RunTallyThrough
data.DailyNewFindings = existing.DailyNewFindings
data.NewFindingsTallyThrough = existing.NewFindingsTallyThrough
}
advancePatrolRunTally(&data, runs, now)
return saveHistoryData(c, c.aiPatrolRunsFile, data, len(runs), "patrol run history", "Patrol run history")
@@ -3740,6 +3781,9 @@ func (c *ConfigPersistence) LoadPatrolRunHistory() (*PatrolRunHistoryData, error
if data.DailyRuns == nil {
data.DailyRuns = make(map[string]int)
}
if data.DailyNewFindings == nil {
data.DailyNewFindings = make(map[string]int)
}
},
func(data *PatrolRunHistoryData) error {
jsonData, err := json.Marshal(data)
+2 -13
View File
@@ -244,23 +244,12 @@ func applyPulseIntelligencePatrolRunSnapshot(snap *telemetry.Snapshot, persisten
if err != nil || history == nil {
return
}
// Run volume comes from the uncapped daily tally. Counting history.Runs
// Run and finding volumes come from daily tallies. Counting history.Runs
// directly saturates at the operator-facing history cap, which on an install
// patrolling on a normal schedule is reached within hours of a thirty-day
// window rather than at its end.
snap.PulseIntelligencePatrolRuns30d += history.PatrolRunsSince(since)
for _, run := range history.Runs {
observedAt := run.CompletedAt
if observedAt.IsZero() {
observedAt = run.StartedAt
}
if observedAt.IsZero() || observedAt.Before(since) {
continue
}
if run.NewFindings > 0 {
snap.PulseIntelligencePatrolNewFindings30d += run.NewFindings
}
}
snap.PulseIntelligencePatrolNewFindings30d += history.PatrolNewFindingsSince(since)
}
func applyPulseIntelligenceFindingSnapshot(snap *telemetry.Snapshot, persistence *config.ConfigPersistence, since time.Time) {
@@ -1123,33 +1123,27 @@ func TestApplyPulseIntelligencePatrolRunSnapshotIgnoresHistoryCap(t *testing.T)
}
}
// PulseIntelligencePatrolNewFindings30d is summed over the retained run list,
// which SavePatrolRunHistory caps at MaxPatrolRunHistory, while
// PulseIntelligencePatrolRuns30d rides the uncapped daily tally that
// TestApplyPulseIntelligencePatrolRunSnapshotIgnoresHistoryCap pins. An
// install patrolling faster than the cap therefore reports every run but only
// the findings from the most recent hundred, so the two counters cover
// different spans and no ratio between findings and any other counter is a
// rate. Pinned so the asymmetry stays a known bound rather than a silent one;
// see coverage gap patrol-investigation-rate-metric-invalid.
func TestApplyPulseIntelligencePatrolRunSnapshotTruncatesNewFindingsAtHistoryCap(t *testing.T) {
// Both activity volumes must survive trimming the operator-facing history.
func TestApplyPulseIntelligencePatrolRunSnapshotPreservesNewFindingsBeyondHistoryCap(t *testing.T) {
persistence := config.NewConfigPersistence(t.TempDir())
now := time.Now().UTC()
since := now.AddDate(0, 0, -30)
const historyCap = 100
total := 0
totalFindings := 0
runs := make([]config.PatrolRunRecord, 0, historyCap)
for at := now.AddDate(0, 0, -2); at.Before(now); at = at.Add(10 * time.Minute) {
runs = append([]config.PatrolRunRecord{{
ID: at.Format(time.RFC3339Nano),
StartedAt: at,
CompletedAt: at,
NewFindings: 1,
NewFindings: total % 4,
}}, runs...)
if len(runs) > historyCap {
runs = runs[:historyCap]
}
totalFindings += total % 4
total++
if err := persistence.SavePatrolRunHistory(runs); err != nil {
t.Fatalf("SavePatrolRunHistory: %v", err)
@@ -1164,9 +1158,9 @@ func TestApplyPulseIntelligencePatrolRunSnapshotTruncatesNewFindingsAtHistoryCap
if snap.PulseIntelligencePatrolRuns30d != total {
t.Fatalf("PulseIntelligencePatrolRuns30d = %d, want %d", snap.PulseIntelligencePatrolRuns30d, total)
}
if snap.PulseIntelligencePatrolNewFindings30d != historyCap {
t.Fatalf("PulseIntelligencePatrolNewFindings30d = %d, want %d (one per retained run, not one per run in the window)",
snap.PulseIntelligencePatrolNewFindings30d, historyCap)
if snap.PulseIntelligencePatrolNewFindings30d != totalFindings {
t.Fatalf("PulseIntelligencePatrolNewFindings30d = %d, want %d across all persisted runs",
snap.PulseIntelligencePatrolNewFindings30d, totalFindings)
}
}