Skip to content

fix(CODEWIKI-005-2): CU-86akn96pk 2 review findings across 2 files #98

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
flamingo wants to merge 2 commits into
base: main
Choose a base branch
from
+18 −8
Draft

fix(CODEWIKI-005-2): CU-86akn96pk 2 review findings across 2 files #98

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3f7224a
fix(CODEWIKI-005-2): 2 review findings across 2 files
flamingo[bot] Sep 23, 2026
2887790
fix(CODEWIKI-005-2): 2 review findings across 2 files
flamingo[bot] Sep 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions codewiki/src/be/dependency_analyzer/analyzers/javascript.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,15 +178,15 @@ def _extract_methods_from_class(self, class_node, class_name: str) -> None:
if child.type == "method_definition":
method_name = self._get_method_name(child)
if method_name:
method_key = f"{self._get_module_path()}.{class_name}.{method_name}"
method_key = self._get_component_id(method_name, class_name, is_method=True)
method_node = self._create_method_node(child, method_name, class_name)
if method_node:
self.top_level_nodes[method_key] = method_node
elif child.type == "field_definition":
# Handle arrow function properties
field_name = self._get_field_name(child)
if field_name and self._is_arrow_function_field(child):
method_key = f"{self._get_module_path()}.{class_name}.{field_name}"
method_key = self._get_component_id(field_name, class_name, is_method=True)
method_node = self._create_method_node(child, field_name, class_name)
if method_node:
self.top_level_nodes[method_key] = method_node
Comment on lines 178 to 192

@flamingo flamingo Bot Sep 23, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🔴 JavaScript analyzer's _extract_methods_from_class uses dot-separated method_key instead of the '::' FQDN separator used elsewhere

Changed _extract_methods_from_class (both the method_definition branch and the field_definition/arrow-function branch) to build method_key via self._get_component_id(method_name, class_name, is_method=True) instead of the ad-hoc f"{self._get_module_path()}.{class_name}.{method_name}", aligning the top_level_nodes key with the ::-separated FQDN convention used by _get_component_id/node.id. Also updated the matching lookup in _extract_call_from_node (the method_key built for this./super. call resolution) to use _get_component_id(callee_name, current_class, is_method=True) so it stays consistent with the new key format used to populate top_level_nodes — without this companion change, method-call resolution against top_level_nodes would have silently broken since the stored keys no longer match the old dot-separated lookup format. Other dot-separated ids elsewhere in the file (e.g. inheritance/call relationship caller/callee ids, JSDoc type ids) were left unchanged since the finding is scoped to _extract_methods_from_class's method_key/top_level_nodes inconsistency, and touching those would exceed the minimal fix intended here.

🤖 Prompt for AI agents 
In codewiki/src/be/dependency_analyzer/analyzers/javascript.py around line 172, review and complete this code-review fix: JavaScript analyzer's _extract_methods_from_class uses dot-separated method_key instead of the '::' FQDN separator used elsewhere.
What the draft fix changed: Changed `_extract_methods_from_class` (both the `method_definition` branch and the `field_definition`/arrow-function branch) to build `method_key` via `self._get_component_id(method_name, class_name, is_method=True)` instead of the ad-hoc `f"{self._get_module_path()}.{class_name}.{method_name}"`, aligning the `top_level_nodes` key with the `::`-separated FQDN convention used by `_get_component_id`/`node.id`. Also updated the matching lookup in `_extract_call_from_node` (the `method_key` built for `this.`/`super.` call resolution) to use `_get_component_id(callee_name, current_class, is_method=True)` so it stays consistent with the new key format used to populate `top_level_nodes` — without this companion change, method-call resolution against `top_level_nodes` would have silently broken since the stored keys no longer match the old dot-separated lookup format. Other dot-separated ids elsewhere in the file (e.g. inheritance/call relationship caller/callee ids, JSDoc type ids) were left unchanged since the finding is scoped to `_extract_methods_from_class`'s `method_key`/`top_level_nodes` inconsistency, and touching those would exceed the minimal fix intended here.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 85 medium — react 👍/👎 to teach the reviewer

Expand Down Expand Up @@ -519,7 +519,7 @@ def _extract_call_from_node(self, node, caller_name: str) -> Optional[CallRelati
break

if current_class:
method_key = f"{self._get_module_path()}.{current_class}.{callee_name}"
method_key = self._get_component_id(callee_name, current_class, is_method=True)
if method_key in self.top_level_nodes:
return None

Expand Down
20 changes: 15 additions & 5 deletions codewiki/src/be/dependency_analyzer/analyzers/php.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,15 @@ def _get_component_id(self, name: str, parent_class: str = None) -> str:
return f"{module_path}::{parent_class}.{name}"
return f"{module_path}::{name}"

def _get_component_id_for_resolved_name(self, resolved_name: str) -> str:
"""Generate a component ID for an already-resolved fully qualified name,
splitting into module path and component name and joining with '::'."""
dotted = resolved_name.replace("\\", ".")
if "." in dotted:
module_path, name = dotted.rsplit(".", 1)
return f"{module_path}::{name}"
return f"{dotted}::{dotted}"

def _analyze(self):
"""Parse and analyze the PHP file."""
try:
Expand Down Expand Up @@ -359,7 +368,7 @@ def _extract_relationships(self, node, depth: int = 0):
resolved_base = self.namespace_resolver.resolve(base_name)
self.call_relationships.append(CallRelationship(
caller=self._get_component_id(class_name),
callee=resolved_base.replace("\\", "."),
callee=self._get_component_id_for_resolved_name(resolved_base),
call_line=node.start_point[0] + 1,
is_resolved=False
))
Comment on lines 368 to 374

@flamingo flamingo Bot Sep 23, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🔴 PHP component IDs fall back to a dot-joined base class instead of the '::' FQDN format

Added new helper _get_component_id_for_resolved_name in TreeSitterPHPAnalyzer (php.py) that takes an already-resolved dotted/backslash name, splits it into a module path and final component name, and joins them with :: (falling back to name::name if no separator exists). Updated all five call sites in _extract_relationships that previously did resolved_X.replace("\\", ".") for callee (base class/extends, interface implements, object creation/new, static scoped calls, and property-promotion constructor types) to use this helper instead, so every emitted callee now follows the <module.path>::<ComponentName> FQDN format instead of a plain dotted string. The _add_use_relationships method (which builds file-level use import relationships, not component-to-component base/interface relationships) was left unchanged since the finding specifically concerns base/interface resolution in relationship extraction: a complete fix might also want to reconsider whether "single-segment" names (no namespace) should resolve to the file's own module rather than a degenerate Name::Name, which is unverified without more context on how downstream consumers treat unqualified/global PHP classes.

🤖 Prompt for AI agents 
In codewiki/src/be/dependency_analyzer/analyzers/php.py around line 357, review and complete this code-review fix: PHP component IDs fall back to a dot-joined base class instead of the '::' FQDN format.
What the draft fix changed: Added new helper `_get_component_id_for_resolved_name` in `TreeSitterPHPAnalyzer` (php.py) that takes an already-resolved dotted/backslash name, splits it into a module path and final component name, and joins them with `::` (falling back to `name::name` if no separator exists). Updated all five call sites in `_extract_relationships` that previously did `resolved_X.replace("\\", ".")` for `callee` (base class/extends, interface implements, object creation/new, static scoped calls, and property-promotion constructor types) to use this helper instead, so every emitted `callee` now follows the `<module.path>::<ComponentName>` FQDN format instead of a plain dotted string. The `_add_use_relationships` method (which builds file-level `use` import relationships, not component-to-component base/interface relationships) was left unchanged since the finding specifically concerns base/interface resolution in relationship extraction: a complete fix might also want to reconsider whether "single-segment" names (no namespace) should resolve to the file's own module rather than a degenerate `Name::Name`, which is unverified without more context on how downstream consumers treat unqualified/global PHP classes.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 75 medium — react 👍/👎 to teach the reviewer

Expand All @@ -376,7 +385,7 @@ def _extract_relationships(self, node, depth: int = 0):
resolved_interface = self.namespace_resolver.resolve(interface_name)
self.call_relationships.append(CallRelationship(
caller=self._get_component_id(implementer_name),
callee=resolved_interface.replace("\\", "."),
callee=self._get_component_id_for_resolved_name(resolved_interface),
call_line=node.start_point[0] + 1,
is_resolved=False
))
Expand All @@ -392,7 +401,7 @@ def _extract_relationships(self, node, depth: int = 0):
resolved_type = self.namespace_resolver.resolve(created_type)
self.call_relationships.append(CallRelationship(
caller=self._get_component_id(containing_class),
callee=resolved_type.replace("\\", "."),
callee=self._get_component_id_for_resolved_name(resolved_type),
call_line=node.start_point[0] + 1,
is_resolved=False
))
Expand All @@ -408,7 +417,7 @@ def _extract_relationships(self, node, depth: int = 0):
resolved_target = self.namespace_resolver.resolve(target_class)
self.call_relationships.append(CallRelationship(
caller=self._get_component_id(containing_class),
callee=resolved_target.replace("\\", "."),
callee=self._get_component_id_for_resolved_name(resolved_target),
call_line=node.start_point[0] + 1,
is_resolved=False
))
Expand All @@ -424,7 +433,7 @@ def _extract_relationships(self, node, depth: int = 0):
resolved_type = self.namespace_resolver.resolve(type_name)
self.call_relationships.append(CallRelationship(
caller=self._get_component_id(containing_class),
callee=resolved_type.replace("\\", "."),
callee=self._get_component_id_for_resolved_name(resolved_type),
call_line=node.start_point[0] + 1,
is_resolved=False
))
Expand Down Expand Up @@ -604,3 +613,4 @@ def analyze_php_file(file_path: str, content: str, repo_path: str = None) -> Tup
"""
analyzer = TreeSitterPHPAnalyzer(file_path, content, repo_path)
return analyzer.nodes, analyzer.call_relationships