fix(analysis): method attribution on name collision + parser timeout

[DataDave-Dev]

Fixes two findings from the project audit.

Closes #21 — method-to-class attribution on name collision

shared.ts kept methodClass as Map<name, class> (last write wins), so two classes in the same file with a method of the same name (e.g. __init__, render) attached the node to the wrong class. Now a name owned by more than one class in the file is marked ambiguous and the node stays at module level instead of being mis-attributed.

Scope: nodes are keyed file::name, so same-named methods still collapse to one node. Per-class method nodes + method-call resolution by receiver type are tracked in #25.

Closes #23 — parser timeout

parseFile now calls parser.setTimeoutMicros(5_000_000) before parse() and skips the file if parse() returns null (timeout), so a pathological input can't block the event loop.

Test plan

• pnpm test — 22 pass, incl. new regression test (two classes sharing __init__)
• pnpm typecheck
• pnpm lint

Summary by CodeRabbit

• Bug Fixes

  • Improved handling of ambiguous method ownership when multiple classes share method names.
  • Added timeout protection for file parsing to prevent failures on large files.

• Tests

  • Added test coverage for method attribution validation in classes with duplicate method names.

[coderabbitai]

📝 Walkthrough

Walkthrough

parseFile in shared.ts gains a MAX_PARSE_MICROS timeout via setTimeoutMicros with a null-return guard that emits empty FileFacts on timeout. The methodClass map is widened to Map<string, string | null> so duplicate method names across classes are stored as null (ambiguous) instead of overwriting. A new test verifies this attribution behavior for a shared __init__.

Changes

Parser timeout and ambiguous method attribution

Layer / File(s)	Summary
Timeout guard and ambiguous method ownership in parseFile src/lib/analysis/analyzers/shared.ts	Adds MAX_PARSE_MICROS constant; widens methodClass to Map<string, string | null>; configures setTimeoutMicros before parse() and returns empty FileFacts if parse() returns null; marks duplicate method names across different classes as null instead of overwriting.
Ambiguous __init__ attribution test src/lib/analysis/analyzers/analyzers.test.ts	New Vitest case in the clases y herencia suite asserts that a_only and b_only are parented to their respective class nodes, while the shared __init__ is parented to the module node mod::dup.py.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• DataDave-Dev/weftmap#18: Extends the same analyzers.test.ts file with assertions about method/function parent attribution in TypeScript classes, exercising the same underlying method-class ownership logic that this PR corrects.

Poem

🐇 A rabbit once parsed every file in the wood,
But two classes shared __init__ — which parent was good?
So I set the node null when the name was in doubt,
And capped the parse time lest the loop never time out.
Now each method finds home, ambiguity's gone —
The warren is tidy, and the tests carry on! 🌿

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes both main changes: method attribution collision fix and parser timeout addition, directly matching the changeset.
Description check	✅ Passed	Description covers both fixes with clear issue references, rationale, scope notes, and complete test validation checklist, fully matching the template structure.
Linked Issues check	✅ Passed	Changes address both #21 (ambiguous method names mapped to null instead of wrong class) and #23 (parser timeout with 5 second limit), fulfilling all code-related requirements.
Out of Scope Changes check	✅ Passed	All changes are scoped to the two linked issues: method collision handling in shared.ts, timeout mechanism in parseFile, and a regression test for the collision scenario.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/analyzer-collision-and-timeout

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/analysis/analyzers/shared.ts`:
- Around line 118-126: The ambiguity detection in the methodClass.set call only
handles class-vs-class collisions but misses the case where the same name
appears both at module-level and in a class. Update the condition that currently
checks if methodClass.has(name) and methodClass.get(name) !== cls to also
account for the scenario where one of the definitions is at module-level (cls is
null/falsy) and another is in a class, treating this as an ambiguous case by
setting methodClass to null whenever a name is seen in different class contexts
or between module-level and class-level definitions.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f64a1600-40d5-4887-b868-3ef4dd5bcbb5

📥 Commits

Reviewing files that changed from the base of the PR and between 2290889 and ba6b9f7.

📒 Files selected for processing (2)

• src/lib/analysis/analyzers/analyzers.test.ts
• src/lib/analysis/analyzers/shared.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Class-vs-module same-name defs are still mis-attributed to class parents.

The ambiguity rule currently handles only class-vs-class collisions. If a module-level function and a class method share the same name in one file, the merged node can still be parented to a class (wrong ownership).

💡 Proposed fix

   const defs = new Set<string>();
   const methodClass = new Map<string, string | null>();
+  const moduleLevelDefs = new Set<string>();
   const defQuery = language.query(spec.funcDefQuery);
   for (const { node } of defQuery.captures(root)) {
     const name = resolveName(node);
     if (!name) continue;
     defs.add(name);
     const cls = enclosingClassName(node, classNodeTypes);
-    if (!cls) continue;
+    if (!cls) {
+      moduleLevelDefs.add(name);
+      if (methodClass.has(name)) methodClass.set(name, null);
+      continue;
+    }
+    if (moduleLevelDefs.has(name)) {
+      methodClass.set(name, null);
+      continue;
+    }
     // Same method name in two classes (e.g. __init__) -> ambiguous, mark null.
     methodClass.set(name, methodClass.has(name) && methodClass.get(name) !== cls ? null : cls);
   }

Also applies to: 264-272

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/analysis/analyzers/shared.ts` around lines 118 - 126, The ambiguity
detection in the methodClass.set call only handles class-vs-class collisions but
misses the case where the same name appears both at module-level and in a class.
Update the condition that currently checks if methodClass.has(name) and
methodClass.get(name) !== cls to also account for the scenario where one of the
definitions is at module-level (cls is null/falsy) and another is in a class,
treating this as an ambiguous case by setting methodClass to null whenever a
name is seen in different class contexts or between module-level and class-level
definitions.