mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
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
This commit is contained in:
committed by
copyberry
parent
d5e256ceb2
commit
5c6f498b0e
@@ -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<AcceptedLineFingerprint>,
|
||||
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<String>,
|
||||
pub(crate) accepted_added_lines: u64,
|
||||
pub(crate) accepted_deleted_lines: u64,
|
||||
pub(crate) line_fingerprints: Vec<AcceptedLineFingerprint>,
|
||||
}
|
||||
|
||||
pub fn accepted_line_fingerprints_from_unified_diff(
|
||||
unified_diff: &str,
|
||||
) -> AcceptedLineFingerprintSummary {
|
||||
let mut current_path: Option<String> = 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<String> {
|
||||
})
|
||||
}
|
||||
|
||||
fn normalize_diff_path(path: &str) -> Option<String> {
|
||||
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<String> {
|
||||
let normalized = line.split_whitespace().collect::<Vec<_>>().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"),
|
||||
}],
|
||||
}
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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: [],
|
||||
},
|
||||
},
|
||||
));
|
||||
|
||||
@@ -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: [],
|
||||
},
|
||||
},
|
||||
))
|
||||
|
||||
@@ -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<String>,
|
||||
pub(crate) accepted_added_lines: u64,
|
||||
pub(crate) accepted_deleted_lines: u64,
|
||||
pub(crate) line_fingerprints: Vec<AcceptedLineFingerprint>,
|
||||
// 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)]
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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,
|
||||
))
|
||||
|
||||
Reference in New Issue
Block a user