Skip to content

Commit fb4ea4a

Browse files
ZhaoChaoqunclaude
andcommitted
fix: address code review feedback (PR #20)
- Fix #1: Update doc comments to reflect precise ms (not bucketed) design decision; remove unused fineLatencyBucket() - Fix #2: Always initialize TelemetryDeck SDK regardless of enabled state, eliminating uninitialized-signal risk on toggle - Fix #3: Extract trackPostProcessingCompleted() helper to eliminate copy-paste analytics params between pipeline branches - Fix #4: Extract hasCloudRewriteAPIKey computed property for clarity Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 2c4d593 commit fb4ea4a

2 files changed

Lines changed: 67 additions & 55 deletions

File tree

Sources/AnalyticsService.swift

Lines changed: 24 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -7,24 +7,28 @@ private let analyticsLogger = Logger(subsystem: "com.typeless.app", category: "A
77
/// Lightweight wrapper around TelemetryDeck for privacy-first analytics.
88
///
99
/// Design principles:
10-
/// - All numeric values are bucketed (e.g. "500-1000ms" not "743ms") to prevent re-identification
11-
/// - Text content is NEVER sent
10+
/// - Latency values use precise milliseconds for maximum diagnostic value
11+
/// - Text content is NEVER sent; text lengths are bucketed (e.g. "11-50" not "37")
1212
/// - Users can opt out via Settings toggle (UserDefaults "analyticsEnabled", default true)
1313
/// - DEBUG builds are automatically tagged as test signals by TelemetryDeck
1414
enum AnalyticsService {
1515

1616
private static let appID = "6EB06114-0222-4BE1-9DDC-BB283B640436"
1717

1818
/// Initialize TelemetryDeck. Call once from AppDelegate.applicationDidFinishLaunching.
19+
///
20+
/// Always initializes the SDK regardless of enabled state, so that toggling
21+
/// analytics on later doesn't require re-initialization. The `track()` method
22+
/// checks `isEnabled` before sending any signals.
1923
static func initialize() {
20-
guard isEnabled else {
21-
analyticsLogger.info("Analytics disabled by user preference")
22-
return
23-
}
24-
2524
let config = TelemetryDeck.Config(appID: appID)
2625
TelemetryDeck.initialize(config: config)
27-
analyticsLogger.info("Analytics initialized (TelemetryDeck)")
26+
27+
if isEnabled {
28+
analyticsLogger.info("Analytics initialized (TelemetryDeck)")
29+
} else {
30+
analyticsLogger.info("Analytics initialized but disabled by user preference")
31+
}
2832
}
2933

3034
/// Whether analytics is enabled (opt-out model: default true).
@@ -37,15 +41,11 @@ enum AnalyticsService {
3741
return true
3842
}
3943

40-
/// Update enabled state and reinitialize if needed.
44+
/// Update enabled state. TelemetryDeck is already initialized;
45+
/// this only controls whether `track()` sends signals.
4146
static func setEnabled(_ enabled: Bool) {
4247
UserDefaults.standard.set(enabled, forKey: "analyticsEnabled")
43-
if enabled {
44-
let config = TelemetryDeck.Config(appID: appID)
45-
TelemetryDeck.initialize(config: config)
46-
analyticsLogger.info("Analytics re-enabled")
47-
}
48-
// TelemetryDeck doesn't have a teardown; disabled state is checked in track()
48+
analyticsLogger.info("Analytics \(enabled ? "enabled" : "disabled")")
4949
}
5050

5151
/// Track an analytics event with optional parameters.
@@ -56,7 +56,7 @@ enum AnalyticsService {
5656
var params = parameters
5757
params["appVersion"] = Bundle.main.infoDictionary?["CFBundleShortVersionString"] as? String ?? "unknown"
5858
params["selectedEngine"] = UserDefaults.standard.string(forKey: "selectedASRModel") ?? "streamingParaformer"
59-
params["cloudRewriteEnabled"] = "\(!((UserDefaults.standard.string(forKey: "cloudRewriteAPIKey") ?? "").isEmpty) || GeneratedSecrets.cloudRewriteAPIKey != nil)"
59+
params["cloudRewriteEnabled"] = "\(hasCloudRewriteAPIKey)"
6060

6161
TelemetryDeck.signal(event, parameters: params)
6262
}
@@ -89,19 +89,6 @@ enum AnalyticsService {
8989
}
9090
}
9191

92-
/// Fine-grained latency bucket for fast pipeline stages (TermNorm, ITN, etc.).
93-
static func fineLatencyBucket(ms: Int) -> String {
94-
switch ms {
95-
case ..<10: return "0-10ms"
96-
case 10..<50: return "10-50ms"
97-
case 50..<100: return "50-100ms"
98-
case 100..<200: return "100-200ms"
99-
case 200..<500: return "200-500ms"
100-
case 500..<1000: return "500-1000ms"
101-
default: return "1000ms+"
102-
}
103-
}
104-
10592
/// Map recording duration (seconds) to a privacy-safe bucket.
10693
static func durationBucket(seconds: Double) -> String {
10794
switch seconds {
@@ -123,4 +110,12 @@ enum AnalyticsService {
123110
default: return "200+"
124111
}
125112
}
113+
114+
// MARK: - Private Helpers
115+
116+
/// Whether a Cloud Rewrite API key is configured (user-provided or built-in).
117+
private static var hasCloudRewriteAPIKey: Bool {
118+
let userKey = UserDefaults.standard.string(forKey: "cloudRewriteAPIKey") ?? ""
119+
return !userKey.isEmpty || GeneratedSecrets.cloudRewriteAPIKey != nil
120+
}
126121
}

Sources/PostProcessingPipeline.swift

Lines changed: 43 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -93,19 +93,14 @@ final class PostProcessingPipeline {
9393
let rewrittenText = await self.cloudRewriteService.rewriteOrPassthrough(processedText)
9494
let cloudRewriteMs = AnalyticsService.elapsedMs(since: cloudRewriteStart)
9595
self.processingQueue.async {
96-
let totalMs = AnalyticsService.elapsedMs(since: pipelineStart)
97-
AnalyticsService.track("PostProcessing.Completed", parameters: [
98-
"itnApplied": "\(itnApplied)",
99-
"cscApplied": "false",
100-
"punctuationApplied": "false",
101-
"totalLatencyMs": "\(totalMs)",
102-
"termNormLatencyMs": "\(termNormMs)",
103-
"itnLatencyMs": "\(itnMs)",
104-
"cscLatencyMs": "0",
105-
"punctLatencyMs": "0",
106-
"cloudRewriteLatencyMs": "\(cloudRewriteMs)",
107-
"termNormChanged": "\(termNormChanged)",
108-
])
96+
Self.trackPostProcessingCompleted(
97+
pipelineStart: pipelineStart,
98+
termNormMs: termNormMs, termNormChanged: termNormChanged,
99+
itnMs: itnMs, itnApplied: itnApplied,
100+
cscMs: 0, cscApplied: false,
101+
punctMs: 0, punctuationApplied: false,
102+
cloudRewriteMs: cloudRewriteMs
103+
)
109104
logger.info("最终结果: \(rewrittenText, privacy: .public)")
110105
completion(rewrittenText)
111106
}
@@ -139,22 +134,44 @@ final class PostProcessingPipeline {
139134
let rewrittenText = await self.cloudRewriteService.rewriteOrPassthrough(punctuatedText)
140135
let cloudRewriteMs = AnalyticsService.elapsedMs(since: cloudRewriteStart)
141136
self.processingQueue.async {
142-
let totalMs = AnalyticsService.elapsedMs(since: pipelineStart)
143-
AnalyticsService.track("PostProcessing.Completed", parameters: [
144-
"itnApplied": "\(itnApplied)",
145-
"cscApplied": "\(cscApplied)",
146-
"punctuationApplied": "\(punctuationApplied)",
147-
"totalLatencyMs": "\(totalMs)",
148-
"termNormLatencyMs": "\(termNormMs)",
149-
"itnLatencyMs": "\(itnMs)",
150-
"cscLatencyMs": "\(cscMs)",
151-
"punctLatencyMs": "\(punctMs)",
152-
"cloudRewriteLatencyMs": "\(cloudRewriteMs)",
153-
"termNormChanged": "\(termNormChanged)",
154-
])
137+
Self.trackPostProcessingCompleted(
138+
pipelineStart: pipelineStart,
139+
termNormMs: termNormMs, termNormChanged: termNormChanged,
140+
itnMs: itnMs, itnApplied: itnApplied,
141+
cscMs: cscMs, cscApplied: cscApplied,
142+
punctMs: punctMs, punctuationApplied: punctuationApplied,
143+
cloudRewriteMs: cloudRewriteMs
144+
)
155145
completion(rewrittenText)
156146
}
157147
}
158148
}
159149
}
150+
151+
// MARK: - Analytics Helper
152+
153+
/// Build and send the PostProcessing.Completed event with per-stage latency.
154+
/// Single source of truth — called from both pipeline branches.
155+
private static func trackPostProcessingCompleted(
156+
pipelineStart: ContinuousClock.Instant,
157+
termNormMs: Int, termNormChanged: Bool,
158+
itnMs: Int, itnApplied: Bool,
159+
cscMs: Int, cscApplied: Bool,
160+
punctMs: Int, punctuationApplied: Bool,
161+
cloudRewriteMs: Int
162+
) {
163+
let totalMs = AnalyticsService.elapsedMs(since: pipelineStart)
164+
AnalyticsService.track("PostProcessing.Completed", parameters: [
165+
"itnApplied": "\(itnApplied)",
166+
"cscApplied": "\(cscApplied)",
167+
"punctuationApplied": "\(punctuationApplied)",
168+
"totalLatencyMs": "\(totalMs)",
169+
"termNormLatencyMs": "\(termNormMs)",
170+
"itnLatencyMs": "\(itnMs)",
171+
"cscLatencyMs": "\(cscMs)",
172+
"punctLatencyMs": "\(punctMs)",
173+
"cloudRewriteLatencyMs": "\(cloudRewriteMs)",
174+
"termNormChanged": "\(termNormChanged)",
175+
])
176+
}
160177
}

0 commit comments

Comments
 (0)