fix(js): support nested class extraction and mixin factory resolution - #3380
fix(js): support nested class extraction and mixin factory resolution#3380hopstreax wants to merge 1 commit into
Conversation
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 5 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Extends JS/TS nested-function scanning to also detect classes returned or declared inside factory functions — concise-arrow-returned classes ((Base) => class extends Base {}), return class {} expressions, nested class_declarations, and class-valued const/let bindings — emitting a contains-edged node for each and walking its members via walk_fn. Anonymous returned classes get a synthetic <fn>@class label, disambiguated with an @L<line> suffix when the id already exists, and are registered in both callable_def_nids and the new callable_class_nids set so mixin factory calls resolve instead of dangling (#3349).
Worth a look
- Factory application extraction reads into rg_name but validates arg_name —
graphify/extractors/resolution.py· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Class declarator suppresses scanning sibling declarators —
graphify/extractors/engine.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- class branch returns early for non-recursive walk_fn callers —
graphify/extractors/engine.py:2118· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- seen_ids never updated after generating a class node, so @l suffix dedup only handles one collision —
graphify/extractors/engine.py:2130· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Class-expression extraction skips heritage/base processing —
graphify/extractors/engine.py:2543· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2104 functions depend on the 445 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 557 callers, 43 callees - new:
_rebuild_code()— 115 callers, 51 callees - new:
_extract_generic()— 18 callers, 26 callees - new:
extract_js()— 93 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
extract_objc()— 27 callers, 9 callees - new:
_resolve_js_module_path()— 27 callers, 6 callees - …and 38 more — each is listed as a finding
Verification — 2104 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 1368 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify \_extract\_generic.
The verifier did not have enough to check \_extract\_generic, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_js\_extra\_walk.
The verifier did not have enough to check \_js\_extra\_walk, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `walk_fn` is annotated `Any` — outside the synthesizable primitive/collection set
Verification did not run: Verification did not run for \_scan\_js\_nested\_function\_declarations.
The verification could not execute (an environment/toolchain issue, not a statement about the code).
Guarantee: No guarantee, the check itself did not complete.
Note: Detail: exec: NameError
Could not verify: Could not verify \_apply\_symbol\_resolution\_facts.
The verifier did not have enough to check \_apply\_symbol\_resolution\_facts, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_collect\_js\_symbol\_resolution\_facts.
The verifier did not have enough to check \_collect\_js\_symbol\_resolution\_facts, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `facts` is annotated `_SymbolResolutionFacts` — outside the synthesizable primitive/collection set
· 2 grounded finding(s) anchored inline below; 44 more finding(s) on lines outside this diff (see the check run).
| return base_name if base_name else None | ||
|
|
||
|
|
||
| def _js_extract_factory_fact( |
There was a problem hiding this comment.
_js_extract_factory_fact()
fans out to 8 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| ) | ||
|
|
||
|
|
||
| def _collect_js_mixin_facts( |
There was a problem hiding this comment.
_collect_js_mixin_facts()
fans out to 6 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
a09685d to
f66a629
Compare
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 5 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Extends JS/TS nested-declaration scanning so factory patterns are graphed: named/nested class declarations, return class ... (including through parentheses/as/satisfies), const X = class ..., and concise-arrow-returned classes now each emit a node, a contains edge under the enclosing scope, and register as callable/class defs, with their class bodies walked via the passed-in walk_fn. Anonymous returned classes fall back to a <fn>@class label, disambiguated to <fn>@class@L<line> when the id already exists. Threads walk_fn, callable_class_nids, seen_ids, and enclosing_fn_name through the recursion so calls made inside these nested classes resolve instead of dangling.
Worth a look
- Typo crashes factory application extraction for calls with arguments —
graphify/extractors/resolution.py· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Mixed JS declarations skip existing nested function discovery —
graphify/extractors/engine.py:2267· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Wrapped function initializers still bypass callable handling —
graphify/extractors/engine.py:2513· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Factory application extraction references unassigned arg_name —
graphify/extractors/resolution.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- factory application resolution path uses .resolve() while factory facts stored with .resolve() but symbol_nodes/owned keyed by unresolved paths —
graphify/extractors/resolution.py:1213· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2106 functions depend on the 451 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 557 callers, 43 callees - new:
_rebuild_code()— 115 callers, 51 callees - new:
_extract_generic()— 18 callers, 26 callees - new:
extract_js()— 93 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
extract_objc()— 27 callers, 9 callees - new:
_resolve_js_module_path()— 27 callers, 6 callees - …and 38 more — each is listed as a finding
Verification — 2106 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 1373 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify \_extract\_generic.
The verifier did not have enough to check \_extract\_generic, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_js\_extra\_walk.
The verifier did not have enough to check \_js\_extra\_walk, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `walk_fn` is annotated `Any` — outside the synthesizable primitive/collection set
Verification did not run: Verification did not run for \_scan\_js\_nested\_function\_declarations.
The verification could not execute (an environment/toolchain issue, not a statement about the code).
Guarantee: No guarantee, the check itself did not complete.
Note: Detail: exec: NameError
Could not verify: Could not verify \_apply\_symbol\_resolution\_facts.
The verifier did not have enough to check \_apply\_symbol\_resolution\_facts, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_collect\_js\_symbol\_resolution\_facts.
The verifier did not have enough to check \_collect\_js\_symbol\_resolution\_facts, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `facts` is annotated `_SymbolResolutionFacts` — outside the synthesizable primitive/collection set
· 2 grounded finding(s) anchored inline below; 44 more finding(s) on lines outside this diff (see the check run).
| return base_name if base_name else None | ||
|
|
||
|
|
||
| def _js_extract_factory_fact( |
There was a problem hiding this comment.
_js_extract_factory_fact()
fans out to 8 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| ) | ||
|
|
||
|
|
||
| def _collect_js_mixin_facts( |
There was a problem hiding this comment.
_collect_js_mixin_facts()
fans out to 6 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
1 participant
Summary
Fixes #3349 by extending JavaScript/TypeScript extraction and symbol resolution to support classes defined inside functions and class expressions used by mixin/factory patterns.
Changes
mixes_inandinheritsedges only when the relationship can be resolved statically.Tests
Added focused coverage for:
Validation:
git diff --checkpassesCommit
a09685d—feat(js): resolve mixin factory inheritance