From 2784461741b50b889fee8c51865378f879bea77a Mon Sep 17 00:00:00 2001
From: Andrew Gallant <andrew@astral.sh>
Date: Fri, 16 Jan 2026 14:24:20 -0500
Subject: [PATCH] [ty] Get rid of high-level completion deduplication

It turns out this wasn't actually doing much and was just papering over
the possibility of duplicates being returned from `SemanticModel`. So
this was doing a fair bit of work for no good reason.

Instead, we push deduplication down into semantic completions directly.
This will run over a (typically) much smaller number of completions.

With that said, this doesn't seem to lead to improvement in ad hoc
benchmarking. However, perf wasn't really the main motivation here: this
change is primarily to prep for switching to a max-heap. This
deduplication was problematic because it was being done *after* sorting,
which meant it dependend on having the entire set of completions in
memory. But with a max-heap, we'll only keep around the top-K
completions, so deduplication has to be done "earlier" (if at all).
---
 crates/ty_ide/src/completion.rs               | 31 +++++++++++--------
 .../ty_python_semantic/src/semantic_model.rs  |  5 +++
 2 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/crates/ty_ide/src/completion.rs b/crates/ty_ide/src/completion.rs
index 95721489d3..dea8a49509 100644
--- a/crates/ty_ide/src/completion.rs
+++ b/crates/ty_ide/src/completion.rs
@@ -122,8 +122,6 @@ impl<'db> Completions<'db> {
     /// sequence of completions.
     fn into_completions(mut self) -> Vec<Completion<'db>> {
         self.items.sort_unstable();
-        self.items
-            .dedup_by(|c1, c2| (&c1.0.name, c1.0.module_name) == (&c2.0.name, c2.0.module_name));
         self.items.truncate(Completions::LIMIT);
         self.items
             .into_iter()
@@ -134,8 +132,6 @@ impl<'db> Completions<'db> {
     // Convert this collection into a list of "import..." fixes
     fn into_imports(mut self) -> Vec<ImportEdit> {
         self.items.sort_unstable();
-        self.items
-            .dedup_by(|c1, c2| (&c1.0.name, c1.0.module_name) == (&c2.0.name, c2.0.module_name));
         self.items
             .into_iter()
             .map(|CompletionRanker(c)| c)
@@ -151,8 +147,6 @@ impl<'db> Completions<'db> {
     // Convert this collection into a list of "qualify..." fixes
     fn into_qualifications(mut self, range: TextRange) -> Vec<ImportEdit> {
         self.items.sort_unstable();
-        self.items
-            .dedup_by(|c1, c2| (&c1.0.name, c1.0.module_name) == (&c2.0.name, c2.0.module_name));
         self.items
             .into_iter()
             .map(|CompletionRanker(c)| c)
@@ -208,11 +202,8 @@ impl<'db> Completions<'db> {
         if self.context.exclude(self.db, &builder) {
             return false;
         }
-        self.items.push(CompletionRanker(builder.build(
-            self.db,
-            &self.context,
-            &self.query,
-        )));
+        let completion = builder.build(self.db, &self.context, &self.query);
+        self.items.push(CompletionRanker(completion));
         true
     }
 }
@@ -1409,11 +1400,11 @@ fn add_function_arg_completions<'db>(
     let Some(sig_help) = signature_help(db, file, cursor.offset) else {
         return;
     };
-    let set_function_args = detect_set_function_args(cursor);
+    let mut set_function_args = detect_set_function_args(cursor);
 
     for sig in &sig_help.signatures {
         for p in &sig.parameters {
-            if p.is_positional_only || set_function_args.contains(&p.name.as_str()) {
+            if p.is_positional_only || !set_function_args.insert(p.name.as_str()) {
                 continue;
             }
             let mut builder = CompletionBuilder::argument(&p.name).ty(p.ty);
@@ -8170,6 +8161,20 @@ def f(x: Intersection[int, Any] | str):
         );
     }
 
+    #[test]
+    fn no_duplicate_keyword_arg() {
+        let builder = completion_test_builder(
+            r#"
+import re
+re.match('', '', fla<CURSOR>
+"#,
+        );
+        assert_snapshot!(
+            builder.skip_auto_import().skip_builtins().build().snapshot(),
+            @"flags=",
+        );
+    }
+
     /// A way to create a simple single-file (named `main.py`) completion test
     /// builder.
     ///
diff --git a/crates/ty_python_semantic/src/semantic_model.rs b/crates/ty_python_semantic/src/semantic_model.rs
index aaeacb2cd6..7ae87d78ad 100644
--- a/crates/ty_python_semantic/src/semantic_model.rs
+++ b/crates/ty_python_semantic/src/semantic_model.rs
@@ -251,6 +251,11 @@ impl<'db> SemanticModel<'db> {
         // Builtins are available in all scopes.
         let builtins = ModuleName::new_static("builtins").expect("valid module name");
         completions.extend(self.module_completions(&builtins));
+
+        // The above can sometimes result in duplicates. Get rid of them.
+        completions.sort_by(|c1, c2| c1.name.cmp(&c2.name));
+        completions.dedup_by(|c1, c2| c1.name == c2.name);
+
         completions
     }