Skip to content

fix(CODEWIKI-005-2): CU-86akn96pk 2 review findings in csharp.py #110

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 1 commit into
base: main
Choose a base branch
from
+2 −1
Draft

fix(CODEWIKI-005-2): CU-86akn96pk 2 review findings in csharp.py #110

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion codewiki/src/be/dependency_analyzer/analyzers/csharp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def _get_relative_path(self) -> str:

def _get_component_id(self, name: str) -> str:

@flamingo flamingo Bot Sep 28, 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.

🦩 🔴 csharp analyzer generates component IDs without :: separator when repo_path is empty

In TreeSitterCSharpAnalyzer._get_component_id (line ~54), removed the falsy-module_path fallback so the method now always returns f"{module_path}::{name}", guaranteeing the :: separator is present in the FQDN as required by CODEWIKI-005-2/CODEWIKI-008, matching the suggested fix exactly.

🤖 Prompt for AI agents 
In codewiki/src/be/dependency_analyzer/analyzers/csharp.py around line 54, review and complete this code-review fix: csharp analyzer generates component IDs without `::` separator when repo_path is empty.
What the draft fix changed: In `TreeSitterCSharpAnalyzer._get_component_id` (line ~54), removed the falsy-`module_path` fallback so the method now always returns `f"{module_path}::{name}"`, guaranteeing the `::` separator is present in the FQDN as required by CODEWIKI-005-2/CODEWIKI-008, matching the suggested fix exactly.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer

module_path = self._get_module_path()
return f"{module_path}::{name}" if module_path else name
return f"{module_path}::{name}"

def _analyze(self):
language_capsule = tree_sitter_c_sharp.language()
Expand Down Expand Up @@ -304,3 +304,4 @@ def analyze_csharp_file(file_path: str, content: str, repo_path: str = None) ->
return analyzer.nodes, analyzer.call_relationships



Comment on lines 304 to +307

@flamingo flamingo Bot Sep 28, 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.

🦩 🟠 codewiki/src/be/dependency_analyzer/analyzers/csharp#analyze_csharp_file duplicates an existing definition

The analyze_csharp_file module-level function (line ~302) was left unchanged. Extracting a shared implementation would require touching c.py, cpp.py, php.py and creating/aligning a common module across multiple analyzer files, which is out of scope for a single-file fix and risks behavioral drift between per-language analyzers (each has language-specific node handling). No shared module exists yet to import safely, and inventing one here would violate the "every import must exist" rule if not mirrored consistently in the other files, which are not in scope for this change. A complete fix requires a follow-up cross-file refactor introducing a shared analyze_file/dispatch helper module, updating all duplicate call sites simultaneously.

🤖 Prompt for AI agents 
In codewiki/src/be/dependency_analyzer/analyzers/csharp.py around line 302, review and complete this code-review fix: codewiki/src/be/dependency_analyzer/analyzers/csharp#analyze_csharp_file duplicates an existing definition.
What the draft fix changed: The `analyze_csharp_file` module-level function (line ~302) was left unchanged. Extracting a shared implementation would require touching `c.py`, `cpp.py`, `php.py` and creating/aligning a common module across multiple analyzer files, which is out of scope for a single-file fix and risks behavioral drift between per-language analyzers (each has language-specific node handling). No shared module exists yet to import safely, and inventing one here would violate the "every import must exist" rule if not mirrored consistently in the other files, which are not in scope for this change. A complete fix requires a follow-up cross-file refactor introducing a shared `analyze_file`/dispatch helper module, updating all duplicate call sites simultaneously.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 30 low — review closely — react 👍/👎 to teach the reviewer