From 5c6f498b0e35058ddf0d3ba0959a0967c5b6495b Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Fri, 14 Aug 2026 01:29:24 +0000 Subject: [PATCH] Stop generating accepted-line fingerprints (#38473) ## What changed - Replace accepted-line fingerprint generation with diff-based added and deleted line counts. - Keep `line_fingerprints` in the analytics payload as a statically empty field for schema compatibility. - Remove the fingerprint types and public parsing API that are no longer used. ## Testing - Update diff parsing tests to cover accepted-line counts without generating fingerprints. GitOrigin-RevId: 4247c76c7b56b1a323f70f960823937352b84f7b --- codex-rs/analytics/src/accepted_lines.rs | 113 ++++-------------- .../analytics/src/analytics_client_tests.rs | 2 +- codex-rs/analytics/src/client_tests.rs | 2 +- codex-rs/analytics/src/events.rs | 5 +- codex-rs/analytics/src/facts.rs | 6 - codex-rs/analytics/src/lib.rs | 2 - codex-rs/analytics/src/reducer.rs | 5 +- 7 files changed, 29 insertions(+), 106 deletions(-) diff --git a/codex-rs/analytics/src/accepted_lines.rs b/codex-rs/analytics/src/accepted_lines.rs index fd94bb2827..f858df8199 100644 --- a/codex-rs/analytics/src/accepted_lines.rs +++ b/codex-rs/analytics/src/accepted_lines.rs @@ -1,17 +1,15 @@ use crate::events::CodexAcceptedLineFingerprintsEventParams; use crate::events::CodexAcceptedLineFingerprintsEventRequest; use crate::events::TrackEventRequest; -use crate::facts::AcceptedLineFingerprint; use codex_git_utils::canonicalize_git_remote_url; use codex_git_utils::get_git_remote_urls_assume_git_repo; use sha1::Digest; use std::path::Path; #[derive(Clone, Debug, PartialEq, Eq)] -pub struct AcceptedLineFingerprintSummary { - pub accepted_added_lines: u64, - pub accepted_deleted_lines: u64, - pub line_fingerprints: Vec, +pub(crate) struct AcceptedLineCounts { + pub(crate) accepted_added_lines: u64, + pub(crate) accepted_deleted_lines: u64, } pub(crate) struct AcceptedLineFingerprintEventInput { @@ -24,21 +22,15 @@ pub(crate) struct AcceptedLineFingerprintEventInput { pub(crate) repo_hash: Option, pub(crate) accepted_added_lines: u64, pub(crate) accepted_deleted_lines: u64, - pub(crate) line_fingerprints: Vec, } -pub fn accepted_line_fingerprints_from_unified_diff( - unified_diff: &str, -) -> AcceptedLineFingerprintSummary { - let mut current_path: Option = None; +pub(crate) fn accepted_line_counts_from_unified_diff(unified_diff: &str) -> AcceptedLineCounts { let mut in_hunk = false; let mut accepted_added_lines = 0; let mut accepted_deleted_lines = 0; - let mut line_fingerprints = Vec::new(); for line in unified_diff.lines() { if line.starts_with("diff --git ") { - current_path = None; in_hunk = false; continue; } @@ -48,25 +40,12 @@ pub fn accepted_line_fingerprints_from_unified_diff( continue; } - if !in_hunk && let Some(path) = line.strip_prefix("+++ ") { - current_path = normalize_diff_path(path); + if !in_hunk && (line.starts_with("+++ ") || line.starts_with("--- ")) { continue; } - if !in_hunk && line.starts_with("--- ") { - continue; - } - - if let Some(added_line) = line.strip_prefix('+') { + if line.starts_with('+') { accepted_added_lines += 1; - if let Some(path) = current_path.as_deref() - && let Some(normalized_line) = normalize_effective_line(added_line) - { - line_fingerprints.push(AcceptedLineFingerprint { - path_hash: fingerprint_hash("path", path), - line_hash: fingerprint_hash("line", &normalized_line), - }); - } continue; } @@ -75,10 +54,9 @@ pub fn accepted_line_fingerprints_from_unified_diff( } } - AcceptedLineFingerprintSummary { + AcceptedLineCounts { accepted_added_lines, accepted_deleted_lines, - line_fingerprints, } } @@ -104,7 +82,6 @@ pub(crate) fn accepted_line_fingerprint_event_requests( repo_hash, accepted_added_lines, accepted_deleted_lines, - line_fingerprints: _line_fingerprints, } = input; vec![TrackEventRequest::AcceptedLineFingerprints(Box::new( @@ -120,9 +97,7 @@ pub(crate) fn accepted_line_fingerprint_event_requests( repo_hash, accepted_added_lines, accepted_deleted_lines, - // Keep computing local fingerprints for parsing tests and future attribution, - // but do not upload path/line hashes in the analytics event payload. - line_fingerprints: Vec::new(), + line_fingerprints: [], }, }, ))] @@ -140,40 +115,12 @@ pub async fn accepted_line_repo_hash_for_cwd(cwd: &Path) -> Option { }) } -fn normalize_diff_path(path: &str) -> Option { - let path = path.trim(); - if path == "/dev/null" { - return None; - } - - Some( - path.strip_prefix("b/") - .or_else(|| path.strip_prefix("a/")) - .unwrap_or(path) - .to_string(), - ) -} - -fn normalize_effective_line(line: &str) -> Option { - let normalized = line.split_whitespace().collect::>().join(" "); - if normalized.len() <= 3 { - return None; - } - if !normalized - .chars() - .any(|ch| ch.is_alphanumeric() || ch == '_') - { - return None; - } - Some(normalized) -} - #[cfg(test)] mod tests { use super::*; #[test] - fn parses_counts_and_effective_added_fingerprints() { + fn parses_accepted_line_counts() { let diff = "\ diff --git a/src/lib.rs b/src/lib.rs index 1111111..2222222 @@ -187,24 +134,12 @@ index 1111111..2222222 context "; - let summary = accepted_line_fingerprints_from_unified_diff(diff); - assert_eq!( - summary, - AcceptedLineFingerprintSummary { + accepted_line_counts_from_unified_diff(diff), + AcceptedLineCounts { accepted_added_lines: 3, accepted_deleted_lines: 1, - line_fingerprints: vec![ - AcceptedLineFingerprint { - path_hash: fingerprint_hash("path", "src/lib.rs"), - line_hash: fingerprint_hash("line", "fn useful() {"), - }, - AcceptedLineFingerprint { - path_hash: fingerprint_hash("path", "src/lib.rs"), - line_hash: fingerprint_hash("line", "return user.id;"), - }, - ], - } + }, ); } @@ -220,11 +155,13 @@ index 0000000..1111111 +print('hello') "; - let summary = accepted_line_fingerprints_from_unified_diff(diff); - - assert_eq!(summary.accepted_added_lines, 1); - assert_eq!(summary.accepted_deleted_lines, 0); - assert_eq!(summary.line_fingerprints.len(), 1); + assert_eq!( + accepted_line_counts_from_unified_diff(diff), + AcceptedLineCounts { + accepted_added_lines: 1, + accepted_deleted_lines: 0, + }, + ); } #[test] @@ -239,18 +176,12 @@ index 1111111..2222222 +++ new value "; - let summary = accepted_line_fingerprints_from_unified_diff(diff); - assert_eq!( - summary, - AcceptedLineFingerprintSummary { + accepted_line_counts_from_unified_diff(diff), + AcceptedLineCounts { accepted_added_lines: 1, accepted_deleted_lines: 1, - line_fingerprints: vec![AcceptedLineFingerprint { - path_hash: fingerprint_hash("path", "src/lib.rs"), - line_hash: fingerprint_hash("line", "++ new value"), - }], - } + }, ); } } diff --git a/codex-rs/analytics/src/analytics_client_tests.rs b/codex-rs/analytics/src/analytics_client_tests.rs index b594ad5509..3c6a782e2b 100644 --- a/codex-rs/analytics/src/analytics_client_tests.rs +++ b/codex-rs/analytics/src/analytics_client_tests.rs @@ -1178,7 +1178,7 @@ fn accepted_line_fingerprints_event_serializes_expected_shape() { repo_hash: Some("repo-hash-1".to_string()), accepted_added_lines: 42, accepted_deleted_lines: 40, - line_fingerprints: Vec::new(), + line_fingerprints: [], }, }, )); diff --git a/codex-rs/analytics/src/client_tests.rs b/codex-rs/analytics/src/client_tests.rs index c3609045e9..a4b122a2b3 100644 --- a/codex-rs/analytics/src/client_tests.rs +++ b/codex-rs/analytics/src/client_tests.rs @@ -130,7 +130,7 @@ fn sample_accepted_line_fingerprint_event(thread_id: &str) -> TrackEventRequest repo_hash: None, accepted_added_lines: 1, accepted_deleted_lines: 0, - line_fingerprints: Vec::new(), + line_fingerprints: [], }, }, )) diff --git a/codex-rs/analytics/src/events.rs b/codex-rs/analytics/src/events.rs index 34c5e35997..7080aa20b7 100644 --- a/codex-rs/analytics/src/events.rs +++ b/codex-rs/analytics/src/events.rs @@ -1,6 +1,5 @@ use std::time::Instant; -use crate::facts::AcceptedLineFingerprint; use crate::facts::AppInvocation; use crate::facts::ArtifactOperation; use crate::facts::ArtifactOperationLifecycle; @@ -179,7 +178,9 @@ pub(crate) struct CodexAcceptedLineFingerprintsEventParams { pub(crate) repo_hash: Option, pub(crate) accepted_added_lines: u64, pub(crate) accepted_deleted_lines: u64, - pub(crate) line_fingerprints: Vec, + // Analytics ingestion and warehouse schemas require this field on the wire. + // Keep it statically empty; line fingerprints are no longer generated. + pub(crate) line_fingerprints: [(); 0], } #[derive(Serialize)] diff --git a/codex-rs/analytics/src/facts.rs b/codex-rs/analytics/src/facts.rs index 74c1fe67e8..111442e5b2 100644 --- a/codex-rs/analytics/src/facts.rs +++ b/codex-rs/analytics/src/facts.rs @@ -32,12 +32,6 @@ use serde::Serialize; use std::collections::BTreeMap; use std::path::PathBuf; -#[derive(Clone, Debug, PartialEq, Eq, Serialize)] -pub struct AcceptedLineFingerprint { - pub path_hash: String, - pub line_hash: String, -} - #[derive(Clone)] pub struct TrackEventsContext { pub model_slug: String, diff --git a/codex-rs/analytics/src/lib.rs b/codex-rs/analytics/src/lib.rs index af64fdc1b0..7e7142abd3 100644 --- a/codex-rs/analytics/src/lib.rs +++ b/codex-rs/analytics/src/lib.rs @@ -9,7 +9,6 @@ mod reducer; use std::time::SystemTime; use std::time::UNIX_EPOCH; -pub use accepted_lines::accepted_line_fingerprints_from_unified_diff; pub use accepted_lines::fingerprint_hash; pub use client::AnalyticsEventsClient; pub use events::AppServerRpcTransport; @@ -23,7 +22,6 @@ pub use events::GuardianReviewSessionKind; pub use events::GuardianReviewTerminalStatus; pub use events::GuardianReviewTrackContext; pub use events::GuardianReviewedAction; -pub use facts::AcceptedLineFingerprint; pub use facts::AnalyticsJsonRpcError; pub use facts::AppInvocation; pub use facts::ArtifactOperation; diff --git a/codex-rs/analytics/src/reducer.rs b/codex-rs/analytics/src/reducer.rs index 301ed4e5eb..c7be5aab88 100644 --- a/codex-rs/analytics/src/reducer.rs +++ b/codex-rs/analytics/src/reducer.rs @@ -1,6 +1,6 @@ use crate::accepted_lines::AcceptedLineFingerprintEventInput; +use crate::accepted_lines::accepted_line_counts_from_unified_diff; use crate::accepted_lines::accepted_line_fingerprint_event_requests; -use crate::accepted_lines::accepted_line_fingerprints_from_unified_diff; use crate::accepted_lines::accepted_line_repo_hash_for_cwd; use crate::events::AppServerRpcTransport; use crate::events::CodexAppMentionedEventRequest; @@ -3166,7 +3166,7 @@ fn accepted_line_event_input( turn_state: &TurnState, ) -> Option<(AcceptedLineFingerprintEventInput, PathBuf)> { let latest_diff = turn_state.latest_diff.as_deref()?; - let summary = accepted_line_fingerprints_from_unified_diff(latest_diff); + let summary = accepted_line_counts_from_unified_diff(latest_diff); if summary.accepted_added_lines == 0 && summary.accepted_deleted_lines == 0 { return None; } @@ -3185,7 +3185,6 @@ fn accepted_line_event_input( repo_hash: None, accepted_added_lines: summary.accepted_added_lines, accepted_deleted_lines: summary.accepted_deleted_lines, - line_fingerprints: summary.line_fingerprints, }, resolved_config.permission_profile_cwd, ))