Skip to content

fix(CODEWIKI-005-2): CU-86akbhhru 7 review findings across 4 files #45

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

Merged
michaelassraf merged 6 commits into from
Sep 8, 2026
+50 −20
Merged

fix(CODEWIKI-005-2): CU-86akbhhru 7 review findings across 4 files #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
35b09b2
fix(CODEWIKI-005-2): 7 review findings across 4 files
flamingo[bot] Sep 7, 2026
13a760f
fix(CODEWIKI-005-2): 7 review findings across 4 files
flamingo[bot] Sep 7, 2026
e4f0168
fix(CODEWIKI-005-2): 7 review findings across 4 files
flamingo[bot] Sep 7, 2026
69f8a5f
fix(CODEWIKI-005-2): 7 review findings across 4 files
flamingo[bot] Sep 7, 2026
beeb6d6
Merge branch 'main' into pr45
michaelassraf Sep 8, 2026
9eb2f2c
fix: align the verification script with the one-separator FQDN format
michaelassraf Sep 8, 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: 6 additions & 0 deletions FQDN_NORMALIZATION_FIX.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@
4. Partial path matching for complex Java packages

Replace the normalization loop in cluster_modules.py:212-233 with this code.

NOTE: This is a scratch/reference module, not imported anywhere in the
codebase. It documents a proposed patch for cluster_modules.py and should be
merged into that file (or moved to docs/ or a PR description) rather than
kept as standalone code at the repository root.
"""

from typing import Dict, List
Comment on lines 13 to 23

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

🦩 🟠 Standalone proposed-fix module FQDN_NORMALIZATION_FIX.py sits at repo root, unused and undocumented as a scratch file

Added a clarifying note to the module docstring at the top of FQDN_NORMALIZATION_FIX.py stating this is a scratch/reference module not imported anywhere and that it should be merged into cluster_modules.py or moved to docs/PR description. This documents the issue in-file but does not remove or relocate the file itself, since deleting it or merging it into cluster_modules.py is out of scope for a single-file fix and risks losing the proposed logic without visibility into the target file's current state — a complete fix requires editing codewiki/src/be/cluster_modules.py (not shown) and then deleting this file, which cannot be safely done here.

🤖 Prompt for AI agents 
In FQDN_NORMALIZATION_FIX.py around line 1, review and complete this code-review fix: Standalone proposed-fix module FQDN_NORMALIZATION_FIX.py sits at repo root, unused and undocumented as a scratch file.
What the draft fix changed: Added a clarifying note to the module docstring at the top of `FQDN_NORMALIZATION_FIX.py` stating this is a scratch/reference module not imported anywhere and that it should be merged into `cluster_modules.py` or moved to docs/PR description. This documents the issue in-file but does not remove or relocate the file itself, since deleting it or merging it into `cluster_modules.py` is out of scope for a single-file fix and risks losing the proposed logic without visibility into the target file's current state — a complete fix requires editing `codewiki/src/be/cluster_modules.py` (not shown) and then deleting this file, which cannot be safely done here.
Verify the change is correct and complete; do not refactor unrelated code.

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

Expand Down Expand Up @@ -135,6 +140,7 @@ def normalize_component_ids_enhanced(
# This handles cases where LLM includes partial path
# Example: "deps.openframe-oss-lib.src.main.java.Class"
# should match "openframe-oss-lib.different.path.java.Class"
suffix_matches: List[str] = []
if '.' in comp_id:
# Try matching last 2-4 segments
segments = comp_id.split('.')
Comment on lines 140 to 146

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

🦩 🔴 suffix_matches referenced outside its enclosing for-loop scope, possible NameError/UnboundLocalError or stale-value bug

In normalize_component_ids_enhanced, initialized suffix_matches: List[str] = [] immediately before the if '.' in comp_id: block (Strategy 5), so the variable is always defined before the subsequent if suffix_matches and len(suffix_matches) == 1: check, eliminating the possible NameError/UnboundLocalError and stale-value read when segments has fewer than 2 elements or the for-loop's last iteration produced no matches.

🤖 Prompt for AI agents 
In FQDN_NORMALIZATION_FIX.py around line 152, review and complete this code-review fix: suffix_matches referenced outside its enclosing for-loop scope, possible NameError/UnboundLocalError or stale-value bug.
What the draft fix changed: In `normalize_component_ids_enhanced`, initialized `suffix_matches: List[str] = []` immediately before the `if '.' in comp_id:` block (Strategy 5), so the variable is always defined before the subsequent `if suffix_matches and len(suffix_matches) == 1:` check, eliminating the possible NameError/UnboundLocalError and stale-value read when `segments` has fewer than 2 elements or the for-loop's last iteration produced no matches.
Verify the change is correct and complete; do not refactor unrelated code.

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

Expand Down
55 changes: 35 additions & 20 deletions codewiki/src/be/cluster_modules.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,18 @@ def extract_module_hint(fqdn: str) -> str:
"openframe-oss-lib.openframe-api-service-core..." → "api-service"
"main-repo.src/services/auth.py::AuthService" → "auth"
"""
if '::' not in fqdn:
logger.warning(
f"FQDN '{fqdn}' does not conform to the required 'module.path::ClassName' "
f"format (missing '::' separator); rejecting dot-only FQDN for module hint extraction"
)
return "unknown"

# Only operate on the module/path portion before the '::' separator
module_part = fqdn.split('::')[0]

# Strategy 1: Look for service-like patterns (openframe-api-service → api-service)
parts = fqdn.split('.')
parts = module_part.split('.')
for part in parts:
if '-service' in part or '-api' in part:
# Extract meaningful part (e.g., "openframe-api-service" → "api-service")
Expand All @@ -43,13 +53,10 @@ def extract_module_hint(fqdn: str) -> str:
return '-'.join(segments[-2:])

# Strategy 2: Extract from file path (src/services/auth.py → auth)
if '::' in fqdn:
file_path = fqdn.split('::')[0]
# Get last meaningful directory or file name
path_parts = file_path.replace('\\', '/').split('/')
for part in reversed(path_parts):
if part and part not in ['src', 'main', 'java', 'com']:
return part.replace('.py', '').replace('.java', '').replace('.ts', '')
path_parts = module_part.replace('\\', '/').split('/')
for part in reversed(path_parts):
if part and part not in ['src', 'main', 'java', 'com']:
return part.replace('.py', '').replace('.java', '').replace('.ts', '')

# Fallback: Use first segment
return parts[0] if parts else "unknown"
Expand All @@ -63,26 +70,34 @@ def extract_package_hint(fqdn: str) -> str:
"...src.main.java.com.openframe.api.controller.Class" → "controller"
"main-repo.src/models/device.py::DeviceModel" → "models"
"""
if '::' not in fqdn:
logger.warning(
f"FQDN '{fqdn}' does not conform to the required 'module.path::ClassName' "
f"format (missing '::' separator); rejecting dot-only FQDN for package hint extraction"
)
return "core"

# Only operate on the module/path portion before the '::' separator
module_part = fqdn.split('::')[0]

# Strategy 1: Look for common package patterns
common_packages = ['controller', 'service', 'repository', 'model', 'dto',
'config', 'util', 'helper', 'handler', 'processor']

fqdn_lower = fqdn.lower()
module_part_lower = module_part.lower()
for pkg in common_packages:
if pkg in fqdn_lower:
if pkg in module_part_lower:
return pkg

# Strategy 2: Extract from file path structure
if '::' in fqdn:
file_path = fqdn.split('::')[0]
path_parts = file_path.replace('\\', '/').split('/')
# Look for meaningful directory names
for part in reversed(path_parts[:-1]): # Skip filename
if part and part not in ['src', 'main', 'java', 'com', 'org']:
return part

# Fallback: Extract from path
parts = fqdn.split('.')
path_parts = module_part.replace('\\', '/').split('/')
# Look for meaningful directory names
for part in reversed(path_parts[:-1]): # Skip filename
if part and part not in ['src', 'main', 'java', 'com', 'org']:
return part

# Fallback: Extract from dotted module path
parts = module_part.split('.')
if len(parts) >= 2:
return parts[-2]

Expand Down
9 changes: 9 additions & 0 deletions codewiki/src/be/dependency_analyzer/analyzers/python.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
"""Python AST-based dependency analyzer.

This module implements an AST-based analyzer for Python source files, extracting
classes, functions, and their call relationships for use in the dependency
analysis pipeline. Component IDs are generated as module::ClassName style
fully-qualified names to remain consistent with the dependency graph and
clustering system's expected FQDN format.
"""

import ast

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

🦩 🟠 Missing module-level docstring in python.py analyzer

Added a module-level triple-quoted docstring at the top of the file (before imports) describing the analyzer's responsibility, matching the style of the sibling php.py analyzer.

🤖 Prompt for AI agents 
In codewiki/src/be/dependency_analyzer/analyzers/python.py around line 1, review and complete this code-review fix: Missing module-level docstring in python.py analyzer.
What the draft fix changed: Added a module-level triple-quoted docstring at the top of the file (before imports) describing the analyzer's responsibility, matching the style of the sibling php.py analyzer.
Verify the change is correct and complete; do not refactor unrelated code.

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

import logging
import warnings
Expand Down