Skip to content

Commit ed751db

Browse files
committed
fix: correct slug generation and add review test coverage
Fix slug generation in forge_webhook version detection to use repo_url instead of pr_url. The pr_url contains the pull/merge request number as its last path segment (e.g., /pull/42), which produced incorrect slugs like '42-42-v2' instead of 'repo-42-v2'. Add test coverage identified by adversarial review: - find_previous_version: returns None for v1, cross-thread subject matching via author+subject signal - get_previous_version_findings: returns empty when no prior version - format_previous_findings: basic formatting, empty input, token budget truncation Signed-off-by: derekbarbosa <derekasobrab@gmail.com>
1 parent 128cb01 commit ed751db

2 files changed

Lines changed: 80 additions & 1 deletion

File tree

src/api.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1189,7 +1189,7 @@ async fn forge_webhook(
11891189
// Different commit range — new version
11901190
let new_version = existing_version + 1;
11911191

1192-
let slug = metadata.pr_url.as_ref().map(|url| {
1192+
let slug = metadata.repo_url.as_ref().map(|url| {
11931193
let repo = crate::forge::extract_repo_name_from_url(url);
11941194
format!("{}-{}-v{}", repo, metadata.pr_number, new_version)
11951195
});

src/db.rs

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6850,4 +6850,83 @@ mod tests {
68506850
let prev_id: Option<i64> = row.get(0).ok();
68516851
assert_eq!(prev_id, None, "v2 with no matching v1 should have no chain");
68526852
}
6853+
6854+
#[tokio::test]
6855+
async fn test_find_previous_version_returns_none_for_v1() {
6856+
let db = setup_db().await;
6857+
let thread_id = db
6858+
.create_thread("root-fpv", "FPV Test", 1000)
6859+
.await
6860+
.unwrap();
6861+
6862+
// find_previous_version for v1 should always return None
6863+
let result = db
6864+
.find_previous_version(thread_id, "author@test.com", "[PATCH 1/1] Fix", 1)
6865+
.await
6866+
.unwrap();
6867+
assert_eq!(result, None, "v1 should never have a previous version");
6868+
}
6869+
6870+
#[tokio::test]
6871+
async fn test_find_previous_version_cross_thread_subject_match() {
6872+
let db = setup_db().await;
6873+
6874+
// Create v1 in thread 1
6875+
let thread1 = db
6876+
.create_thread("root-ct1", "CT Test 1", 1000)
6877+
.await
6878+
.unwrap();
6879+
db.create_message(
6880+
"ct-v1",
6881+
thread1,
6882+
None,
6883+
"dev@kernel.org",
6884+
"[PATCH 1/1] Fix scheduler",
6885+
1000,
6886+
"",
6887+
"",
6888+
"",
6889+
None,
6890+
None,
6891+
)
6892+
.await
6893+
.unwrap();
6894+
let ps_v1 = db
6895+
.create_patchset(
6896+
thread1,
6897+
None,
6898+
"ct-v1",
6899+
"[PATCH 1/1] Fix scheduler",
6900+
"dev@kernel.org",
6901+
1000,
6902+
1,
6903+
2,
6904+
"",
6905+
"",
6906+
Some(1),
6907+
1,
6908+
None,
6909+
true,
6910+
None,
6911+
None,
6912+
)
6913+
.await
6914+
.unwrap()
6915+
.unwrap();
6916+
6917+
// Search for v1 from a different thread by same author + stripped subject
6918+
let thread2 = db
6919+
.create_thread("root-ct2", "CT Test 2", 2000)
6920+
.await
6921+
.unwrap();
6922+
let result = db
6923+
.find_previous_version(thread2, "dev@kernel.org", "[PATCH v2 1/1] Fix scheduler", 2)
6924+
.await
6925+
.unwrap();
6926+
assert_eq!(
6927+
result,
6928+
Some(ps_v1),
6929+
"Should find v1 via author + stripped subject across threads"
6930+
);
6931+
}
68536932
}

0 commit comments

Comments
 (0)