JS/TS: factory object literal is keyed on the class, so sibling methods collide (#2745 follow-up)

[imagineers-tyler]

Follow-up to #2745 (v0.9.47). The feature is right; only its owner id is scoped one level too wide.

Problem

_js_member_assignment_target's new ("object", …) arm mints the owner as
_make_id(function_owner_nid, object_name), and a few lines up:

function_owner_nid = parent_class_nid if parent_class_nid else func_nid

For a method, function_owner_nid is the enclosing class. But const api = {}
is a local of the method, not a member of the class, so two sibling methods that
pick the same local name collapse onto one node.

class Registry {
  buildRead() {
    const api = {};
    api.get = function () { return 1; };
    return api;
  }
  buildWrite() {
    const api = {};
    api.put = function () { return 2; };
    return api;
  }
}

0.9.48 (extract_js, node/edge dump):

NODE factory_registry_api                 'api'            L3
NODE factory_registry_api_get             '.get()'         L4
NODE factory_registry_api_put             '.put()'         L9

EDGE factory_registry     -contains-> factory_registry_api      L3
EDGE factory_registry_api -method->   factory_registry_api_get  L4
EDGE factory_registry     -contains-> factory_registry_api      L8   <-- duplicate
EDGE factory_registry_api -method->   factory_registry_api_put  L9

Two things go wrong:

1. One node for two objects. add_node is first-wins, so the graph represents
   both factory objects as the single api node owning both .get() and .put(),
   keeping buildRead's line (L3), while buildWrite's distinct local object is
   absent. buildWrite is left with no edge to any object node at all — its only
   edge is Registry -method-> buildWrite.
2. The contains edge is emitted twice. That is the duplication
   contained_owners was added to prevent — but that set is created per function
   node, so it cannot see the sibling method. The 0.9.47 release note states the
   guarantee as "the factory's contains edge is emitted once no matter how many
   methods hang off the object"; that holds for a plain function, not for a method.

Fix

Key the object on func_nid. One line plus the comment explaining why the two ids
differ here.

this.X = fn deliberately keeps function_owner_nid — this really is the
instance, so those members do belong to the class. Only the object-literal arm moves.

Tests

Three added next to the existing #2745 tests.

test	without fix	with fix
..._factory_objects_in_sibling_methods_stay_separate	FAIL	pass
..._factory_contains_edge_not_duplicated_across_methods	FAIL	pass
..._this_assignment_still_belongs_to_the_class (control)	pass	pass

Verified by reverting graphify/extractors/engine.py to upstream/v8 and re-running
(2 failed, 1 passed), then restoring (3 passed).

The control passes both ways on purpose: it is there to catch a fix that moves the
this arm along with the object arm.

Two existing behaviours are already covered by #2745's own tests and stay green —
a plain function's factory object, and the single-contains guarantee within one
function. My repro's second control (same class, objects named reader/writer)
was already correct on 0.9.48 and is unchanged by the patch, which isolates the
trigger to same local name in sibling methods.

Field measurement: zero occurrences

I could not find a real-world hit. Extracting with 0.9.48 and with the patch over
six JS corpora gives identical node and edge counts:

corpus	files	nodes	edges	change
mongoose lib/	264	2105	3872	none
playwright lib/	27	4399	10526	none
eslint lib/	388	3729	5876	none
@hapi/hapi lib/	19	347	534	none
handlebars lib/	34	164	349	none
a local Hapi/Mongoose app src/	52	1080	2893	none

The 12-line repro above, run through the same harness in the same invocation, does
change (7 nodes / 1 duplicate contains → 8 nodes / 0), so the harness does
distinguish the two versions — the zeros are data, not a broken measurement. I also
re-ran mongoose and playwright against virgin cache roots to rule out a cache hit
replaying one version's result for the other; identical.

The reason is scope, not rarity of factories: the feature only looks at direct
children of the function body, and the common shapes in these packages are inline
object literals (const api = { get() {…} }, still #2419) and return { get, put }.

So this is a correctness fix with no measured impact on existing graphs. Prioritise
accordingly — I am reporting it because it is one line and it breaks an invariant the
release note states, not because I can show it biting anyone today.

Full suite

25 tests already fail on a clean upstream/v8 in my environment (test_skillgen,
test_terraform, test_ollama_retry_cap, test_install_references, test_labeling
— none touch the JS extractor). The patch introduces no new failure: comparing the
failing sets, nothing is in the patched run that is not in the baseline run.

One caveat so the counts are not mistaken for an improvement:
test_labeling.py::test_label_communities_batches_when_over_batch_size appeared in
one patched full-suite run and not another. Run in isolation it fails deterministically
both with and without the patch, so it flakes on suite order, not on this change.

[graphify-labs]

Graphify reviewed this change.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).

Formal verification. No changes could be formally verified in this run.

---

Graphify review — findings

Fixes namespacing of local factory objects in _extract_generic (JS/TS): key object-literal owners on the enclosing function (func_nid) instead of function_owner_nid, so same-named locals in sibling methods no longer collapse onto one node or emit duplicate contains edges. this.X = fn assignments still resolve to the class. Adds tests covering separate sibling-method objects, contains-edge dedup, and the this control case.

No blocking issues surfaced.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 940 functions depend on the 566 functions this change touches.

Health — this change adds coupling hotspots:

• new: _extract_generic() — 18 callers, 24 callees
• new: extract_xaml() — 19 callers, 17 callees
• new: extract_js() — 83 callers, 3 callees
• new: extract_objc() — 27 callers, 9 callees
• new: extract_julia() — 16 callers, 7 callees
• new: extract_cpp() — 27 callers, 3 callees
• new: extract_vue() — 10 callers, 6 callees
• new: walk() — 1 callers, 56 callees
• …and 8 more — each is listed as a finding

Verification — 940 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: 880 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

· 16 more finding(s) on lines outside this diff (see the check run).