fix(engine): include async, type annotations, and class methods in Python public surface hashes (#76)

packages/engine/src/python-analysis.ts

@@ -249,26 +249,50 @@ def alias_public_name(alias):
         return alias.asname
     return alias.name.split(".")[0]
 
+def format_arg(arg, default=None):
+    res = arg.arg
+    if getattr(arg, "annotation", None) is not None:
+        res += ":" + unparse(arg.annotation)
+    if default is not None:
+        res += "=" + unparse(default)
+    return res
+
 def function_signature(args):
     positional_defaults = [None] * (len(args.posonlyargs) + len(args.args) - len(args.defaults)) + list(args.defaults)
     pieces = []
     for arg, default in zip(args.posonlyargs, positional_defaults[:len(args.posonlyargs)]):
-        pieces.append(arg.arg + (("=" + unparse(default)) if default is not None else ""))
+        pieces.append(format_arg(arg, default))
     if args.posonlyargs:
         pieces.append("/")
     offset = len(args.posonlyargs)
     for arg, default in zip(args.args, positional_defaults[offset:]):
-        pieces.append(arg.arg + (("=" + unparse(default)) if default is not None else ""))
+        pieces.append(format_arg(arg, default))
     if args.vararg is not None:
-        pieces.append("*" + args.vararg.arg)
+        pieces.append("*" + format_arg(args.vararg))
     elif args.kwonlyargs:
         pieces.append("*")
     for arg, default in zip(args.kwonlyargs, args.kw_defaults):
-        pieces.append(arg.arg + (("=" + unparse(default)) if default is not None else ""))
+        pieces.append(format_arg(arg, default))
     if args.kwarg is not None:
-        pieces.append("**" + args.kwarg.arg)
+        pieces.append("**" + format_arg(args.kwarg))
     return ",".join(pieces)
 
+def function_surface_part(node):
+    prefix = "py:async_function:" if isinstance(node, ast.AsyncFunctionDef) else "py:function:"
+    ret = (":" + unparse(node.returns)) if getattr(node, "returns", None) is not None else ""
+    return prefix + node.name + "(" + function_signature(node.args) + ")" + ret
+
+def class_surface_parts(node):
+    bases = ",".join([unparse(base) for base in node.bases])
+    parts = ["py:class:" + node.name + "(" + bases + ")"]
+    for item in node.body:
+        if isinstance(item, (ast.FunctionDef, ast.AsyncFunctionDef)):
+            if (bool(item.name) and not item.name.startswith("_")) or item.name == "__init__":
+                method_prefix = "async_method:" if isinstance(item, ast.AsyncFunctionDef) else "method:"
+                ret = (":" + unparse(item.returns)) if getattr(item, "returns", None) is not None else ""
+                parts.append("py:class_member:" + node.name + "." + method_prefix + item.name + "(" + function_signature(item.args) + ")" + ret)
+    return parts
+
 imports = []
 import_keys = set()
 warnings = []
@@ -277,9 +301,11 @@ top_level_public = set()
 surface_parts = []
 
 def surface_part_name(part):
-    for prefix in ("py:function:", "py:class:"):
+    for prefix in ("py:async_function:", "py:function:", "py:class:"):
         if part.startswith(prefix):
             return part[len(prefix):].split("(", 1)[0]
+    if part.startswith("py:class_member:"):
+        return part[len("py:class_member:"):].split(".", 1)[0]
     if part.startswith("py:variable:"):
         return part[len("py:variable:"):].split(":", 1)[0]
     if part.startswith("py:import:"):
@@ -346,13 +372,11 @@ for node in tree.body:
     elif isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)):
         if is_public(node.name):
             top_level_public.add(node.name)
-            surface_parts.append("py:function:" + node.name + "(" + function_signature(node.args) + ")")
+            surface_parts.append(function_surface_part(node))
     elif isinstance(node, ast.ClassDef):
         if is_public(node.name):
             top_level_public.add(node.name)
-            bases = ",".join([unparse(base) for base in node.bases])
-            top_level_public.add(node.name)
-            surface_parts.append("py:class:" + node.name + "(" + bases + ")")
+            surface_parts.extend(class_surface_parts(node))
     elif isinstance(node, ast.Assign):
         names = []
         for target in node.targets:

tests/module-resolution.test.mjs

@@ -3004,8 +3004,8 @@ test("module resolution extracts Python public symbols and surface hashes", () =
     );
     assert.deepEqual([...extractPublicSymbols(inferredPath)].sort(), ["Box", "VERSION", "fetch", "public_helper", "run"]);
     const expectedParts = [
+      "py:async_function:fetch(value,limit=1)",
       "py:class:Box(Base)",
-      "py:function:fetch(value,limit=1)",
       "py:function:run(value)",
       "py:import:public_helper",
       "py:variable:VERSION:str",

tests/review-regressions.test.mjs

@@ -561,3 +561,71 @@ test("bug #61 empty Changed-Cells is checked against actual ownership", (testCon
   git(rootDir, ["add", "."]); git(rootDir, ["commit", "-qm", message("none")]);
   assert.equal(checkCommitEvidence({ rootDir, manifest, commit: "HEAD" }).ok, true);
 });
+
+test("bug #76 Python public surface hashes differentiate async, type annotations, and class methods", () => {
+  const rootDir = fs.mkdtempSync(path.join(os.tmpdir(), "cellfence-bug-76-"));
+  try {
+    const filePath = path.join(rootDir, "api.py");
+    // Pair 1: sync vs async
+    writeFile(filePath, ["def api(value):", "    return value"]);
+    const syncHash = publicSurfaceHash(filePath);
+    const syncParts = inspectPythonSource(filePath).surfaceParts;
+    assert.deepEqual(syncParts, ["py:function:api(value)"]);
+
+    writeFile(filePath, ["async def api(value):", "    return value"]);
+    const asyncHash = publicSurfaceHash(filePath);
+    const asyncParts = inspectPythonSource(filePath).surfaceParts;
+    assert.deepEqual(asyncParts, ["py:async_function:api(value)"]);
+    assert.notEqual(asyncHash, syncHash);
+
+    // Pair 2: type annotations
+    writeFile(filePath, ["def api(value: str) -> str:", "    return value"]);
+    const strHash = publicSurfaceHash(filePath);
+    const strParts = inspectPythonSource(filePath).surfaceParts;
+    assert.deepEqual(strParts, ["py:function:api(value:str):str"]);
+
+    writeFile(filePath, ["def api(value: int) -> int:", "    return value"]);
+    const intHash = publicSurfaceHash(filePath);
+    const intParts = inspectPythonSource(filePath).surfaceParts;
+    assert.deepEqual(intParts, ["py:function:api(value:int):int"]);
+    assert.notEqual(intHash, strHash);
+
+    // Pair 3: class methods
+    writeFile(filePath, [
+      "class Api:",
+      "    def fetch(self, value):",
+      "        return value",
+    ]);
+    const classHash1 = publicSurfaceHash(filePath);
+    const classParts1 = inspectPythonSource(filePath).surfaceParts;
+    assert.deepEqual(classParts1, [
+      "py:class:Api()",
+      "py:class_member:Api.method:fetch(self,value)",
+    ]);
+
+    writeFile(filePath, [
+      "class Api:",
+      "    def fetch(self, value, required):",
+      "        return required",
+    ]);
+    const classHash2 = publicSurfaceHash(filePath);
+    const classParts2 = inspectPythonSource(filePath).surfaceParts;
+    assert.deepEqual(classParts2, [
+      "py:class:Api()",
+      "py:class_member:Api.method:fetch(self,value,required)",
+    ]);
+    assert.notEqual(classHash2, classHash1);
+
+    // Changes confined to function body should not change the hash
+    writeFile(filePath, [
+      "class Api:",
+      "    def fetch(self, value, required):",
+      "        # Internal comment or changed body logic",
+      "        temp = value + required",
+      "        return temp",
+    ]);
+    assert.equal(publicSurfaceHash(filePath), classHash2);
+  } finally {
+    fs.rmSync(rootDir, { recursive: true, force: true });
+  }
+});