From 38580e0b04150ce21d7863a1352879e5a82d929e Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Sat, 1 Aug 2026 01:36:07 -0500 Subject: [PATCH] fix(python): bare class references produce references edges to classes (#1478) (#1493) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Python's class-as-value idioms (return SomeClass, x = SomeClass, registry dicts, classes passed as arguments) produced no references edges, so callers/impact on a Django/DRF serializer missed the views that consume it. Three gates dropped them: - return_statement was never dispatched by PYTHON_SPEC (kernel mirrored) - the extraction gate (definedHere) collected function/method names only - resolution accepted function/method targets only (matchFunctionRef + the function_ref import fast path) Capture return_statement for Python (single expression; tuple returns not descended), admit same-file CLASS names to the gate, and accept class targets for Python bare identifiers — scoped to Python so the TS/JS KIND FILTER contract is untouched. The docopt false-positive mechanism behind the function-only rule (lowercase locals vs same-named methods) doesn't transfer: methods stay excluded for bare ids, and the same-file/import gate + unique-or-drop rules still apply. Probed on django-rest-framework (~250 files): 559 new references→class edges, 10/10 sampled genuine (serializer_class = AuthTokenSerializer, the ModelSerializer field-mapping registry, aliases, ctor args, isinstance). EXTRACTION_VERSION 24 → 25. Co-authored-by: Claude Fable 5 --- CHANGELOG.md | 1 + __tests__/function-ref.test.ts | 115 ++++++++++++++++++++++++++- codegraph-kernel/src/python.rs | 9 ++- docs/design/function-ref-capture.md | 19 +++-- src/extraction/extraction-version.ts | 2 +- src/extraction/function-ref.ts | 6 ++ src/extraction/tree-sitter.ts | 6 ++ src/resolution/index.ts | 10 ++- src/resolution/name-matcher.ts | 14 +++- 9 files changed, 171 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b36a9dd..b144480 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - The background server's watchdog no longer kills a healthy server that is just waiting on a slow disk: like indexing already does, it now checks whether the database files are still making progress before concluding the process is stuck. Fewer spurious kills also means fewer leftover write-ahead logs. (#1431) - `codegraph status` now shows the write-ahead log's size next to the database size and warns when killed sessions have left it oversized, and every line in the background server's log now carries a timestamp so kills and restarts can be placed in time. (#1431) - On Windows, the Claude Code prompt hook written by `codegraph install` failed with "command not found" when hooks run through Git Bash, which needs the `.cmd` extension to find the launcher. The installer now writes the platform-correct command, and re-running `codegraph install` (or `codegraph upgrade`) repairs an existing install in place. (#1466) +- Python classes used as values — `return SomeSerializer` from a factory method, `handler = SomeClass` aliases, registry dicts and lists, and classes passed as arguments — now produce reference edges in the graph. Previously these idioms were invisible, so on Django and Django REST Framework projects, asking for a serializer's callers or the impact of editing it missed the views that actually use it. Re-index after upgrading to pick up the new edges. (#1478) - When a file changed on disk after its last index sync, `codegraph_node` and `codegraph_explore` could return a different symbol's code under the requested name — current file bytes cut at outdated line positions — while presenting it as verbatim, trustworthy source. This hit hardest on projects queried through `projectPath` (for example, sub-projects of a monorepo), which have no live file watcher to flag pending edits. Both tools now verify each file against the index before showing sliced code: an out-of-date file is either shown whole with its full current source, or its code is withheld with a clear "changed on disk" notice — never served as a wrong slice. A fresh re-index restores normal output automatically. Thanks @inth3shadows for the thorough report and verification passes. (#1474) ## [1.5.0] - 2026-07-21 diff --git a/__tests__/function-ref.test.ts b/__tests__/function-ref.test.ts index 993b686..fe5016c 100644 --- a/__tests__/function-ref.test.ts +++ b/__tests__/function-ref.test.ts @@ -11,7 +11,9 @@ * - decoy: an ambiguous cross-file name (no import, ≥2 definitions) → NO edge * - same-file priority: a same-file definition beats a same-named decoy * - kind filter: a class/variable passed as a value never gets a - * function-ref edge + * function-ref edge — except Python, where class-as-value is a core + * idiom and bare ids ALSO resolve to classes (#1478); methods stay + * excluded for bare ids everywhere * - self: a function passing itself → no self-loop * - drain: all resolvable function_ref rows leave unresolved_refs (no * batched-resolver runaway), and re-index is idempotent @@ -744,6 +746,117 @@ describe('Function-as-value capture (#756)', () => { } }); + it('PYTHON CLASSES: return / alias / registry dict / arg positions produce references edges (#1478)', async () => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-fnref-pycls-')); + fs.writeFileSync( + path.join(tmpDir, 'serializers.py'), + [ + 'class OrgSerializerFull:', + ' pass', + '', + 'class OrgSerializerBrief:', + ' pass', + ].join('\n') + ); + fs.writeFileSync( + path.join(tmpDir, 'views.py'), + [ + 'from serializers import OrgSerializerFull, OrgSerializerBrief', + '', + 'def register(cls):', + ' pass', + '', + 'class OrgViewSet:', + ' def get_serializer_class(self):', + ' if True:', + ' return OrgSerializerFull', + ' return OrgSerializerBrief', + '', + 'SERIALIZER_REGISTRY = {"org": OrgSerializerFull}', + 'register(OrgSerializerBrief)', + ].join('\n') + ); + fs.writeFileSync( + path.join(tmpDir, 'models.py'), + [ + 'class Config:', + ' pass', + '', + 'def make_config_cls():', + ' return Config', + '', + 'ActiveConfig = Config', + ].join('\n') + ); + + const cg = CodeGraph.initSync(tmpDir); + try { + await cg.indexAll(); + + // The DRF wiring: get_serializer_class → the imported serializer class, + // via `return` — the issue's headline gap. The module-level registry + // dict rides the file node. + expect(sourceNames(cg, fnRefEdgesInto(cg, 'OrgSerializerFull'))).toEqual([ + 'get_serializer_class', + 'views.py', + ]); + // Second branch return + a module-level call argument. + expect(sourceNames(cg, fnRefEdgesInto(cg, 'OrgSerializerBrief'))).toEqual([ + 'get_serializer_class', + 'views.py', + ]); + + // Same-file: factory return + module-level alias assignment. + expect(sourceNames(cg, fnRefEdgesInto(cg, 'Config'))).toEqual([ + 'make_config_cls', + 'models.py', + ]); + + // callers() must now surface the view as a consumer of the serializer. + const serializer = cg + .getNodesByName('OrgSerializerFull') + .find((n) => n.kind === 'class')!; + const callers = cg.getCallers(serializer.id); + expect(callers.some((c) => c.node.name === 'get_serializer_class')).toBe(true); + } finally { + cg.destroy(); + tmpDir = undefined; + } + }); + + it('PYTHON KIND FILTER: bare ids still never resolve to methods; unknown names stay silent', async () => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-fnref-pyneg-')); + fs.writeFileSync( + path.join(tmpDir, 'svc.py'), + [ + 'class Svc:', + ' def refresh(self):', + ' pass', + '', + 'def wire(cb):', + ' pass', + '', + 'def setup(refresh):', + // A local/parameter sharing a same-file METHOD name: the gate lets it + // through (methods are in definedHere) but resolution must refuse — + // a bare id can never be a method value in Python. + ' wire(refresh)', + // A name with no matching class/function anywhere: no edge, silently. + ' return unknown_thing', + ].join('\n') + ); + + const cg = CodeGraph.initSync(tmpDir); + try { + await cg.indexAll(); + expect(fnRefEdgesInto(cg, 'refresh')).toHaveLength(0); + expect(fnRefEdgesInto(cg, 'unknown_thing')).toHaveLength(0); + } finally { + cg.destroy(); + tmpDir = undefined; + } + }); + it('DRAIN: resolvable function_ref rows leave unresolved_refs; re-index is stable', async () => { tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-fnref-drain-')); fs.writeFileSync( diff --git a/codegraph-kernel/src/python.rs b/codegraph-kernel/src/python.rs index 0bffcfd..93cb10a 100644 --- a/codegraph-kernel/src/python.rs +++ b/codegraph-kernel/src/python.rs @@ -250,7 +250,9 @@ impl<'t> Walker<'t> { target_id_str: NONE_STR, }); - if kind == "function" || kind == "method" { + // Classes join the fn-ref gate for Python (#1478): class-as-value is + // a first-class idiom (mirrors flushFnRefCandidates' python branch). + if kind == "function" || kind == "method" || kind == "class" { self.defined_fn_names.insert(name.to_string()); } // captureValueRefScope @@ -711,6 +713,11 @@ impl<'t> Walker<'t> { "keyword_argument" => ("value", "value"), "pair" => ("value", "value"), "list" => ("list", ""), + // `return SomeClass` / `return handler` (#1478) — a single + // returned expression is a direct named child ('list' shape); + // tuple returns sit under expression_list and are not descended + // (mirrors PYTHON_SPEC). + "return_statement" => ("list", ""), _ => return, }; if self.stack.is_empty() { diff --git a/docs/design/function-ref-capture.md b/docs/design/function-ref-capture.md index 7c8ef73..b58b720 100644 --- a/docs/design/function-ref-capture.md +++ b/docs/design/function-ref-capture.md @@ -45,7 +45,7 @@ custom `visitNode` hooks like Scala's val/var handler) get a candidates-only | C / ObjC | `argument_list` | `assignment_expression.right` | `initializer_pair.value` | `initializer_list`, `init_declarator.value` | `&fn` (`pointer_expression`), `@selector(...)` (ObjC) | | C++ | **`&` forms only** in args/rhs/varinit | (same — explicit `&` only) | bare ids at FILE scope only | bare ids at FILE scope only | `&fn`, `&Cls::method` (resolved scoped to the class) | | TS / JS (tsx/jsx) | `arguments` | `assignment_expression.right` | `pair.value` | `array`, `variable_declarator.value` | `this.method` (`member_expression`, class-scoped — see rule 3) | -| Python | `argument_list`, `keyword_argument.value` | `assignment.right` | `pair.value` | `list` | `self.method` (`attribute`) | +| Python | `argument_list`, `keyword_argument.value`, `return_statement` (#1478 — single expression only; tuple returns not descended) | `assignment.right` | `pair.value` | `list` | `self.method` (`attribute`) | | Go | `argument_list` | `assignment_statement` / `short_var_declaration` (`expression_list`) | `keyed_element` | `literal_value`, `var_spec.value` | — | | Rust | `arguments` | `assignment_expression.right` | `field_initializer.value` | `array_expression`, `static_item` / `let_declaration.value` | — | | Java | `argument_list` | `assignment_expression.right` | — | `variable_declarator.value` | `method_reference` (`Cls::m`, `this::m`) — the only form | @@ -76,11 +76,18 @@ custom `visitNode` hooks like Scala's val/var handler) get a candidates-only `arena_ind_prev = arena_ind` (redis/jemalloc) each matched a unique same-named function somewhere and produced wrong edges when `rhs`/`varinit` were ungated. -3. **TS/JS/Python: bare ids resolve to `function` kind only.** A bare - identifier can never be a method value in these languages (methods need a - receiver — `this.m` / `self.m`), so allowing method targets soaked up - locals passed as arguments (`new Set(selectedPointsIndices)`; - docopt.py's `name`/`match` params — excalidraw/fmt A/B findings). +3. **TS/JS/Python: bare ids resolve to `function` kind only — plus `class` + for Python (#1478).** A bare identifier can never be a method value in + these languages (methods need a receiver — `this.m` / `self.m`), so + allowing method targets soaked up locals passed as arguments + (`new Set(selectedPointsIndices)`; docopt.py's `name`/`match` params — + excalidraw/fmt A/B findings). Python bare ids ALSO accept CLASS targets: + class-as-value is a core Python idiom (`return SomeSerializer`, registry + dicts, `admin.site.register(Model, Admin)`) with no type-annotation + recovery path, the gate additionally admits same-file CLASS names for + Python, and the docopt false-positive mechanism (lowercase locals vs + same-named methods) doesn't transfer to exact-name class matches. TS/JS + keep the class exclusion (the KIND FILTER contract). TS/JS `this.X` values are captured as `this.`-PREFIXED candidates and resolved CLASS-SCOPED (`resolveThisMemberFnRef` in `src/resolution/index.ts`): the target must be a function/method whose diff --git a/src/extraction/extraction-version.ts b/src/extraction/extraction-version.ts index 618a1b1..07ccbb9 100644 --- a/src/extraction/extraction-version.ts +++ b/src/extraction/extraction-version.ts @@ -21,4 +21,4 @@ * turns the re-index hint into noise — keep it honest (see CLAUDE.md, "Honesty * in the product is load-bearing"). */ -export const EXTRACTION_VERSION = 24; +export const EXTRACTION_VERSION = 25; diff --git a/src/extraction/function-ref.ts b/src/extraction/function-ref.ts index 1bae970..a7359b7 100644 --- a/src/extraction/function-ref.ts +++ b/src/extraction/function-ref.ts @@ -194,6 +194,12 @@ const PYTHON_SPEC: FnRefSpec = { ['keyword_argument', { mode: 'value', field: 'value' }], // Thread(target=worker) ['pair', { mode: 'value', field: 'value' }], ['list', { mode: 'list' }], + // `return SomeClass` / `return handler` — factory returns are how DRF + // wires views to serializers (get_serializer_class) and how Python + // factories hand back callables (#1478). A single returned expression is + // a direct named child, so 'list' covers it; tuple returns (`return A, B`) + // sit under an expression_list child and are deliberately not descended. + ['return_statement', { mode: 'list' }], ]), special: new Set(['attribute']), }; diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index 05b13a9..9e53e62 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -649,6 +649,12 @@ export class TreeSitterExtractor { const definedHere = new Set(); for (const n of this.nodes) { if (n.kind === 'function' || n.kind === 'method') definedHere.add(n.name); + // Python only (#1478): class-as-value is a first-class idiom (DRF + // get_serializer_class, Meta.model, registry dicts), so same-file CLASS + // names pass the gate too. Other languages keep the function/method + // gate — TS/JS recover class references through type annotations, and + // resolution's kind filter would drop their class candidates anyway. + else if (this.language === 'python' && n.kind === 'class') definedHere.add(n.name); } // Import-binding names only (all binding emitters push kind 'imports'). diff --git a/src/resolution/index.ts b/src/resolution/index.ts index 598e5e4..ef3a5fc 100644 --- a/src/resolution/index.ts +++ b/src/resolution/index.ts @@ -915,7 +915,15 @@ export class ReferenceResolver { const viaImport = this.gateLanguage(resolveViaImport(ref, this.context), ref); if (viaImport) { const target = this.queries.getNodeById(viaImport.targetNodeId); - if (target && (target.kind === 'function' || target.kind === 'method')) { + if ( + target && + (target.kind === 'function' || + target.kind === 'method' || + // Python (#1478): an imported class used as a value (`return + // OrgSerializerFull`) resolves through its import like any + // callback — mirrors matchFunctionRef's bareClassOk. + (ref.language === 'python' && target.kind === 'class')) + ) { return viaImport; } } diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 2a1fe0d..967f0b1 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -232,6 +232,16 @@ export function matchFunctionRef( ref.language === 'cpp' || ref.language === 'python' || ref.language === 'php'; + // Python additionally accepts CLASS targets for bare identifiers (#1478): + // class-as-value is a core Python idiom (`return SomeSerializer`, + // `Meta.model = Org`, registry dicts, `admin.site.register(Model, Admin)`) + // and, unlike TS, Python has no type-annotation recovery path. The + // false-positive mechanism behind the function-only rule was lowercase + // locals colliding with same-named METHODS (docopt.py) — a candidate must + // be an exact-name CLASS node here, and the extraction gate (same-file + // class ∪ imports) plus unique-or-drop still apply. Methods stay excluded. + const bareClassOk = ref.language === 'python'; + // Qualified member-pointer (`&Widget::on_click` → "Widget::on_click"): // resolve the member ON THAT SCOPE — exempt from bareFnOnly (the `&Cls::m` // shape is an explicit member reference). Unique-or-drop like everything else. @@ -264,7 +274,9 @@ export function matchFunctionRef( .getNodesByName(ref.referenceName) .filter( (n) => - (n.kind === 'function' || (!bareFnOnly && n.kind === 'method')) && + (n.kind === 'function' || + (!bareFnOnly && n.kind === 'method') || + (bareClassOk && n.kind === 'class')) && sameLanguageFamily(n.language, ref.language) && n.id !== ref.fromNodeId // a function registering itself is not a dependency edge );