Merge pull request #355 from beyondessential/repo-stats-timeago

fix(ui): repo-stats Observed timestamp placement + bucket bytes measurement time

Author: passcod

crates/database/src/backups.rs

@@ -1607,8 +1607,7 @@ impl BackupRepoSnapshot {
 
 	/// When the group's repo was last inspected (newest `observed_at` across its
 	/// sources), or `None` if never. This table is written *only* by the
-	/// inspection Job — unlike `backup_repo_stats.observed_at`, which the daily
-	/// s3-metrics writer also bumps — so it's the clean "last inspected" signal.
+	/// inspection Job, so it's the clean "last inspected" signal.
 	pub async fn last_inspected_at_for_group(
 		db: &mut AsyncPgConnection,
 		group_id: Uuid,
@@ -1657,10 +1656,19 @@ pub struct BackupRepoStats {
 	/// content outside the repository, or reflects a different point in
 	/// time).
 	pub bucket_bytes: Option<i64>,
-	/// When these stats were last refreshed. Whichever of the two collectors
-	/// wrote most recently.
+	/// When the repo-derived figures (snapshot/source counts, logical/physical
+	/// bytes) were last refreshed by the inspection job.
 	#[diesel(deserialize_as = jiff_diesel::Timestamp, serialize_as = jiff_diesel::Timestamp)]
 	pub observed_at: Timestamp,
+	/// When `bucket_bytes` was last refreshed by the (daily) S3-metrics
+	/// collector, if ever. Tracked separately from `observed_at` because the
+	/// two figures are collected on independent cadences.
+	#[diesel(
+		deserialize_as = jiff_diesel::NullableTimestamp,
+		serialize_as = jiff_diesel::NullableTimestamp,
+		treat_none_as_default_value = false
+	)]
+	pub bucket_bytes_observed_at: Option<Timestamp>,
 }
 
 impl BackupRepoStats {
@@ -1711,7 +1719,8 @@ impl BackupRepoStats {
 	}
 
 	/// Writer 2: the S3-metrics task. Touches only `bucket_bytes`
-	/// (+ `observed_at`), never the repo-derived fields.
+	/// (+ `bucket_bytes_observed_at`), never the repo-derived fields or their
+	/// `observed_at`.
 	pub async fn upsert_bucket_bytes(
 		db: &mut AsyncPgConnection,
 		group_id: Uuid,
@@ -1723,10 +1732,14 @@ impl BackupRepoStats {
 			.values((
 				dsl::group_id.eq(group_id),
 				dsl::bucket_bytes.eq(bucket_bytes),
+				dsl::bucket_bytes_observed_at.eq(now),
 			))
 			.on_conflict(dsl::group_id)
 			.do_update()
-			.set((dsl::bucket_bytes.eq(bucket_bytes), dsl::observed_at.eq(now)))
+			.set((
+				dsl::bucket_bytes.eq(bucket_bytes),
+				dsl::bucket_bytes_observed_at.eq(now),
+			))
 			.execute(db)
 			.await
 			.map_err(AppError::from)?;

crates/database/src/schema.rs

@@ -82,6 +82,7 @@ diesel::table! {
 		physical_bytes -> Nullable<Int8>,
 		bucket_bytes -> Nullable<Int8>,
 		observed_at -> Timestamptz,
+		bucket_bytes_observed_at -> Nullable<Timestamptz>,
 	}
 }
 

crates/database/tests/backups.rs

@@ -1237,7 +1237,11 @@ async fn repo_stats_split_writers_do_not_clobber() {
 			(Some(10), Some(3), Some(1000), Some(500), Some(777))
 		);
 
-		// Bucket-bytes writer must not clobber the repo fields.
+		let repo_observed_at = s.observed_at;
+		let bucket_observed_at = s.bucket_bytes_observed_at.unwrap();
+
+		// Bucket-bytes writer must not clobber the repo fields, and must bump
+		// only its own timestamp — not the inspection one.
 		BackupRepoStats::upsert_bucket_bytes(&mut conn, group_id, Some(888))
 			.await
 			.unwrap();
@@ -1247,8 +1251,15 @@ async fn repo_stats_split_writers_do_not_clobber() {
 			.unwrap();
 		assert_eq!(s.bucket_bytes, Some(888));
 		assert_eq!(s.snapshot_count, Some(10), "repo fields preserved");
+		assert_eq!(s.observed_at, repo_observed_at, "observed_at untouched");
+		assert!(
+			s.bucket_bytes_observed_at.unwrap() >= bucket_observed_at,
+			"bucket_bytes_observed_at bumped"
+		);
+		let bucket_observed_at = s.bucket_bytes_observed_at.unwrap();
 
-		// And the inspection writer must not clobber bucket_bytes.
+		// And the inspection writer must not clobber bucket_bytes or its
+		// timestamp.
 		BackupRepoStats::upsert_repo_fields(
 			&mut conn,
 			group_id,
@@ -1265,6 +1276,11 @@ async fn repo_stats_split_writers_do_not_clobber() {
 			.unwrap();
 		assert_eq!(s.snapshot_count, Some(11));
 		assert_eq!(s.bucket_bytes, Some(888), "bucket_bytes preserved");
+		assert_eq!(
+			s.bucket_bytes_observed_at,
+			Some(bucket_observed_at),
+			"bucket_bytes_observed_at preserved"
+		);
 	})
 	.await;
 }

crates/jobs/src/backup/s3_metrics.rs

@@ -151,8 +151,8 @@ async fn tick(
 	for c in &ready {
 		// Fire once per day at the group's jittered deadline, catching up on a
 		// later tick if this one drifts past the slot or a slow predecessor group
-		// pushed us late. The anchor is in-memory (best-effort daily metric with
-		// no persisted timestamp): it resets on restart, re-firing at the next
+		// pushed us late. The anchor is in-memory (`bucket_bytes_observed_at` is
+		// only persisted on success): it resets on restart, re-firing at the next
 		// deadline. Recorded on attempt, not success, so a persistently-failing
 		// group doesn't hammer CloudWatch every tick.
 		if !slot_deadline_due(

crates/private-server/tests/backups.rs

@@ -607,8 +607,8 @@ async fn stats_includes_runs_and_pending_requests() {
 			"INSERT INTO devices (id, role) VALUES ('{device_id}', 'server');
 			 INSERT INTO servers (id, host, kind, group_id, device_id) VALUES \
 				('{server_id}', 'https://e.test', 'central', '{group_id}', '{device_id}');
-			 INSERT INTO backup_repo_stats (group_id, snapshot_count, source_count, logical_bytes, physical_bytes) \
-				VALUES ('{group_id}', 12, 3, 1000, 800);
+			 INSERT INTO backup_repo_stats (group_id, snapshot_count, source_count, logical_bytes, physical_bytes, bucket_bytes, bucket_bytes_observed_at) \
+				VALUES ('{group_id}', 12, 3, 1000, 800, 2000, now() - interval '1 day');
 			 INSERT INTO backup_runs (id, device_id, group_id, server_id, type, purpose, outcome, bytes_uploaded) \
 				VALUES ('{run_id}', '{device_id}', '{group_id}', '{server_id}', 'tamanu-postgres', 'backup', 'success', 500);
 			 -- The issuance that started the backup 5 minutes before it reported, so
@@ -645,6 +645,10 @@ async fn stats_includes_runs_and_pending_requests() {
 		resp.assert_status_ok();
 		let body: serde_json::Value = resp.json();
 		assert_eq!(body["stats"]["snapshot_count"], 12);
+		assert_eq!(body["stats"]["bucket_bytes"], 2000);
+		// The bucket figure carries its own measurement timestamp, separate
+		// from the inspection-owned observed_at.
+		assert!(body["stats"]["bucket_bytes_observed_at"].is_string());
 		// The reported backup, plus an inferred in-flight restore from its issuance.
 		// The consumer device's restore issuance is excluded (it's shown in the
 		// restore-checks table instead).

migrations/2026-07-07-001122-0000_add_bucket_bytes_observed_at/down.sql

@@ -0,0 +1 @@
+ALTER TABLE backup_repo_stats DROP COLUMN bucket_bytes_observed_at;

migrations/2026-07-07-001122-0000_add_bucket_bytes_observed_at/up.sql

@@ -0,0 +1,6 @@
+-- Give the S3-metrics collector its own timestamp on backup_repo_stats, so
+-- `observed_at` can belong solely to the repo-inspection writer. Backfill from
+-- `observed_at` where a bucket figure exists: the collector runs daily and
+-- bumped `observed_at` until now, so it's within a day of the real measurement.
+ALTER TABLE backup_repo_stats ADD COLUMN bucket_bytes_observed_at TIMESTAMPTZ;
+UPDATE backup_repo_stats SET bucket_bytes_observed_at = observed_at WHERE bucket_bytes IS NOT NULL;

private-web/e2e/backups.spec.ts

@@ -341,6 +341,18 @@ test.describe("backups ready: stats + backup-now", () => {
 		await expect(page.getByText("42")).toBeVisible();
 		// bucket_bytes NULL → "unknown" shown, per the indicators rule.
 		await expect(page.getByText(/bucket bytes:\s*unknown/i)).toBeVisible();
+		// The "Observed" timestamp belongs to the kopia-reported repo stats, so
+		// it sits under "Physical bytes", above the bucket figure.
+		const statsPanel = page
+			.getByRole("heading", { name: /repository stats/i })
+			.locator("..");
+		const panelText = await statsPanel.innerText();
+		expect(panelText.indexOf("Observed")).toBeGreaterThan(
+			panelText.indexOf("Physical bytes"),
+		);
+		expect(panelText.indexOf("Observed")).toBeLessThan(
+			panelText.indexOf("Bucket bytes"),
+		);
 		await expect(page.getByText(/recent runs/i)).toBeVisible();
 		await expect(page.getByText("success")).toBeVisible();
 		// The run carries a server_id, so the table names which server it's from.
@@ -488,6 +500,7 @@ test.describe("backups ready: stats + backup-now", () => {
 		await seedBackupRepoStats(sql, {
 			groupId: group.id,
 			bucketBytes: 107374182400, // 100 GiB
+			bucketBytesObservedAt: new Date(Date.now() - 86_400_000).toISOString(),
 		});
 
 		await page.goto(`/groups/${group.id}/backups`);
@@ -497,6 +510,12 @@ test.describe("backups ready: stats + backup-now", () => {
 		const tooltip = page.getByRole("tooltip");
 		await expect(tooltip).toContainText("$2.50/month");
 		await expect(tooltip).toContainText("ap-southeast-2");
+		// The bucket figure comes from CloudWatch on its own daily cadence, so
+		// the tooltip carries its measurement time and says it updates less
+		// often than the kopia-reported stats.
+		await expect(tooltip).toContainText("From CloudWatch storage metrics");
+		await expect(tooltip).toContainText("measured");
+		await expect(tooltip).toContainText("about once a day");
 	});
 
 	test("recent run shows a truncated, copyable snapshot id", async ({

private-web/e2e/seed.ts

@@ -568,19 +568,22 @@ export async function seedBackupRepoStats(
 		logicalBytes?: number | null;
 		physicalBytes?: number | null;
 		bucketBytes?: number | null;
+		/** ISO timestamp of the bucket-bytes measurement. */
+		bucketBytesObservedAt?: string | null;
 	},
 ): Promise<void> {
 	await sql.query(
 		`INSERT INTO backup_repo_stats
-		 (group_id, snapshot_count, source_count, logical_bytes, physical_bytes, bucket_bytes)
-		 VALUES ($1, $2, $3, $4, $5, $6)`,
+		 (group_id, snapshot_count, source_count, logical_bytes, physical_bytes, bucket_bytes, bucket_bytes_observed_at)
+		 VALUES ($1, $2, $3, $4, $5, $6, $7)`,
 		[
 			opts.groupId,
 			opts.snapshotCount ?? null,
 			opts.sourceCount ?? null,
 			opts.logicalBytes ?? null,
 			opts.physicalBytes ?? null,
 			opts.bucketBytes ?? null,
+			opts.bucketBytesObservedAt ?? null,
 		],
 	);
 }

private-web/openapi.json

@@ -6641,6 +6641,14 @@
             "format": "int64",
             "description": "Total size of the underlying S3 bucket, in bytes, as reported by cloud\nstorage metrics. Can differ from `physical_bytes` (e.g. it includes\ncontent outside the repository, or reflects a different point in\ntime)."
           },
+          "bucket_bytes_observed_at": {
+            "type": [
+              "string",
+              "null"
+            ],
+            "format": "date-time",
+            "description": "When `bucket_bytes` was last refreshed by the (daily) S3-metrics\ncollector, if ever. Tracked separately from `observed_at` because the\ntwo figures are collected on independent cadences."
+          },
           "group_id": {
             "type": "string",
             "format": "uuid",
@@ -6657,7 +6665,7 @@
           "observed_at": {
             "type": "string",
             "format": "date-time",
-            "description": "When these stats were last refreshed. Whichever of the two collectors\nwrote most recently."
+            "description": "When the repo-derived figures (snapshot/source counts, logical/physical\nbytes) were last refreshed by the inspection job."
           },
           "physical_bytes": {
             "type": [