Skip to content

Commit b998e19

Browse files
aapelivclaude
andcommitted
Trim verbose comments in native diagnostics code
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1175aac commit b998e19

6 files changed

Lines changed: 15 additions & 49 deletions

File tree

app/backend/src/couchers/servicers/bugs.py

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -196,12 +196,9 @@ def ReportDiagnostics(
196196
def CheckNativeStatus(
197197
self, request: bugs_pb2.CheckNativeStatusReq, context: CouchersContext, session: Session
198198
) -> bugs_pb2.CheckNativeStatusRes:
199-
# Stub for now: just log the client's diagnostics blob alongside the session user so we can
200-
# see real pings in prod. A later pass will persist these and decide whether to force-update.
199+
# Stub: log the ping for now. TODO: persist it and decide whether to force-update.
201200
logger.info("CheckNativeStatus: user_id=%s debug=%s", context._user_id, request.debug_json)
202201

203-
# TODO: decide based on the reported app version / build whether this client is below a
204-
# minimum supported version and should be force-updated.
205202
return bugs_pb2.CheckNativeStatusRes(
206203
update_required=False,
207204
update_available=False,

app/mobile/features/diagnostics/installId.ts

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,7 @@ import AsyncStorage from "@react-native-async-storage/async-storage";
22
import * as Application from "expo-application";
33
import { Platform } from "react-native";
44

5-
// A UUID we generate and persist on first launch. It's the primary key for a device install in the
6-
// diagnostics pipeline. It survives app restarts and OTA updates, but resets on reinstall (the
7-
// platform identifiers below cover cross-install correlation where the OS allows it).
5+
// UUID generated and persisted on first launch; survives restarts/OTA, resets on reinstall.
86
const INSTALL_ID_KEY = "diagnostics.installId";
97

108
function uuidv4(): string {
@@ -25,11 +23,7 @@ export async function getInstallId(): Promise<string> {
2523
return id;
2624
}
2725

28-
// Best-effort platform identifiers reported alongside install_id for cross-install correlation.
29-
// - iOS: identifierForVendor persists across reinstall only while another app from our Apple Team
30-
// is installed; otherwise it regenerates.
31-
// - Android: ANDROID_ID (SSAID) is stable across reinstalls of apps signed with the same key and
32-
// resets only on factory reset.
26+
// Best-effort platform identifiers for cross-install correlation (iOS IDFV, Android SSAID).
3327
export async function getPlatformDeviceIds(): Promise<{
3428
idfv?: string;
3529
androidId?: string;
@@ -43,7 +37,7 @@ export async function getPlatformDeviceIds(): Promise<{
4337
return { androidId: Application.getAndroidId() ?? undefined };
4438
}
4539
} catch {
46-
// Identifiers are best-effort; never block a diagnostics ping on them.
40+
// best-effort; never block a ping
4741
}
4842
return {};
4943
}

app/mobile/features/diagnostics/pushTokenStore.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
import AsyncStorage from "@react-native-async-storage/async-storage";
22

3-
// The last Expo push token we successfully registered with the backend. Persisted so the
4-
// diagnostics ping can report it (letting the backend correlate push reachability with delivery
5-
// receipts) without re-fetching a token on every app open.
3+
// The last Expo push token registered with the backend, persisted so diagnostics can report it.
64
const PUSH_TOKEN_KEY = "diagnostics.pushToken";
75

86
export async function setStoredPushToken(token: string): Promise<void> {

app/mobile/features/diagnostics/useNativeDiagnostics.ts

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,26 +16,16 @@ import {
1616
import { getStoredPushToken } from "@/features/diagnostics/pushTokenStore";
1717
import { checkNativeStatus } from "@/service/checkNativeStatus";
1818

19-
// We treat each foreground activation as a potential "app open", but throttle pings so a user
20-
// quickly toggling in and out of the app doesn't flood the pipeline. time_since_last_open is the
21-
// gap since the previous ping, which for a normally-used app is the day-to-day open cadence.
2219
const LAST_OPEN_KEY = "diagnostics.lastOpenAt";
23-
const PING_THROTTLE_MS = 30 * 60 * 1000; // 30 minutes
20+
const PING_THROTTLE_MS = 30 * 60 * 1000;
2421

25-
/**
26-
* Periodically reports a diagnostics snapshot (app version, OTA bundle, push state, etc.) to the
27-
* backend's CheckNativeStatus endpoint. Fires on cold start and on each background→foreground
28-
* transition, throttled to at most once per PING_THROTTLE_MS.
29-
*
30-
* Everything here is best-effort and fire-and-forget: a failed or skipped ping must never affect
31-
* app startup or the user.
32-
*/
22+
// Reports a diagnostics snapshot to CheckNativeStatus on cold start and each foreground
23+
// transition, throttled to at most once per PING_THROTTLE_MS. Best-effort and fire-and-forget.
3324
export function useNativeDiagnostics(): void {
3425
const { authenticated } = useAuthContext();
3526
const { i18n } = useTranslation();
3627

37-
// Read auth/locale via refs so the AppState listener always sees current values without
38-
// re-subscribing on every change.
28+
// Refs so the AppState listener sees current values without re-subscribing.
3929
const authenticatedRef = useRef(authenticated);
4030
authenticatedRef.current = authenticated;
4131
const localeRef = useRef(i18n.language);
@@ -92,9 +82,7 @@ export function useNativeDiagnostics(): void {
9282

9383
await AsyncStorage.setItem(LAST_OPEN_KEY, String(now));
9484

95-
// TODO: surface a force-update / soft-update UI when the backend asks the client to update.
96-
// For now the server always returns permissive defaults; once the decision logic exists
97-
// we'll wire result.updateRequired / updateAvailable into a modal here.
85+
// TODO: wire result.updateRequired / updateAvailable into a force-update UI.
9886
if (result.updateRequired || result.updateAvailable) {
9987
console.log("Native status update prompt:", result);
10088
}
@@ -103,10 +91,8 @@ export function useNativeDiagnostics(): void {
10391
}
10492
}
10593

106-
// Cold start.
10794
maybeReport();
10895

109-
// Subsequent background→foreground transitions.
11096
const subscription = AppState.addEventListener(
11197
"change",
11298
(state: AppStateStatus) => {

app/mobile/service/checkNativeStatus.ts

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,7 @@ export interface NativeStatusResult {
88
updateMessage: string;
99
}
1010

11-
/**
12-
* Sends a best-effort diagnostics snapshot to the backend and returns its update decision.
13-
*
14-
* The snapshot is an arbitrary object (app version, OTA bundle, push state, etc.) that we just
15-
* JSON-encode into a single field; the backend only logs it for now, so new fields can be added
16-
* here without a proto change.
17-
*/
11+
// JSON-encodes a diagnostics snapshot into debug_json and returns the backend's update decision.
1812
export async function checkNativeStatus(
1913
debugInfo: Record<string, unknown>,
2014
): Promise<NativeStatusResult> {

app/proto/bugs.proto

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,8 @@ service Bugs {
5252
rpc ReportDiagnostics(ReportDiagnosticsReq) returns (google.protobuf.Empty) {}
5353

5454
rpc CheckNativeStatus(CheckNativeStatusReq) returns (CheckNativeStatusRes) {
55-
// Diagnostics ping from the native mobile apps. The client sends a best-effort JSON blob of
56-
// diagnostics (app version, OTA bundle, push state, etc.); for now the backend just logs it.
57-
// The response carries status back to the client, e.g. whether it must force the user to update.
55+
// Diagnostics ping from the native mobile apps. The client sends a JSON blob (just logged for
56+
// now); the response tells the client whether it must force the user to update.
5857
}
5958

6059
rpc EvaluateFeatureFlag(EvaluateFeatureFlagReq) returns (EvaluateFeatureFlagRes) {
@@ -127,10 +126,8 @@ message ReportDiagnosticsReq {
127126
}
128127

129128
message CheckNativeStatusReq {
130-
// Best-effort diagnostics snapshot, JSON-encoded by the client (app/build version, OTA bundle,
131-
// platform/device, push state, install id, etc.). Free-form on purpose: the backend just logs it
132-
// for now, so the client can add fields without a proto change. The session's user is read
133-
// server-side, so this need not (and should not) contain auth tokens or PII.
129+
// Free-form diagnostics snapshot, JSON-encoded by the client. The session's user is read
130+
// server-side, so this should not contain auth tokens or PII.
134131
string debug_json = 1;
135132
}
136133

0 commit comments

Comments
 (0)