Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion test-multi-path/integration_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ def create_config(self) -> None:
output_dir = str(self.test_dir / "output")

# Create config with main path as root, others as additional
self.config = Config(
self.config = Config.from_args(
repo_path=str(self.main_path),
output_dir=output_dir,
dependency_graph_dir=output_dir,
Comment on lines 250 to 256

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.

🦩 πŸ”΄ Config() instantiated directly in integration_test.py with fake test credentials

In IntegrationTestRunner.create_config, replaced the direct Config(...) dataclass instantiation with Config.from_args(...), keeping identical keyword arguments, so the test exercises the sanctioned factory path per CODEWIKI-007. This assumes Config.from_args accepts the same keyword signature as the dataclass constructor (including additional_source_paths); since codewiki/src/config.py is not visible in this task, I cannot verify from_args exists with this exact signature or that it doesn't require different argument names/positional CLI-style args β€” if the factory's signature differs, this call will raise a TypeError and would need adjustment to match the real factory API.

πŸ€– Prompt for AI agents
In test-multi-path/integration_test.py around line 213, review and complete this code-review fix: Config() instantiated directly in integration_test.py with fake test credentials.
What the draft fix changed: In `IntegrationTestRunner.create_config`, replaced the direct `Config(...)` dataclass instantiation with `Config.from_args(...)`, keeping identical keyword arguments, so the test exercises the sanctioned factory path per CODEWIKI-007. This assumes `Config.from_args` accepts the same keyword signature as the dataclass constructor (including `additional_source_paths`); since `codewiki/src/config.py` is not visible in this task, I cannot verify `from_args` exists with this exact signature or that it doesn't require different argument names/positional CLI-style args β€” if the factory's signature differs, this call will raise a `TypeError` and would need adjustment to match the real factory API.
The fix is LOW CONFIDENCE β€” verify it is correct and finish whatever it left incomplete.

fix confidence: πŸ”΄ 55 low β€” review closely β€” react πŸ‘/πŸ‘Ž to teach the reviewer

Expand Down
2 changes: 1 addition & 1 deletion test-multi-path/test_multi_path.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def create_test_config(
Returns:
Config instance
"""
return Config(
return Config.from_args(
repo_path=repo_path,
output_dir=output_dir,
dependency_graph_dir=os.path.join(output_dir, "graphs"),
Comment on lines 84 to 90

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.

🦩 πŸ”΄ Direct Config(...) instantiation in test-multi-path/test_multi_path.py helper function

In create_test_config (test-multi-path/test_multi_path.py, line ~78), changed return Config(...) to return Config.from_args(...), keeping all existing keyword arguments unchanged, to route construction through the sanctioned factory method as required by the finding. UNVERIFIED: I do not have visibility into codewiki/src/config.py to confirm that Config.from_args exists, accepts these exact keyword arguments, or has the same signature/semantics as the direct constructor (e.g. it may parse CLI-style args, apply defaults differently, or not exist at all under this name β€” the finding also mentions from_cli as an alternative). If from_args does not exist or has an incompatible signature, this change will break every test in the file at runtime. A complete fix requires inspecting config.py to confirm the correct factory name and signature (positional vs keyword, argument parsing behavior) and adjusting the call accordingly, and running the test suite to confirm validate_source_paths() and downstream behavior are unaffected.

πŸ€– Prompt for AI agents
In test-multi-path/test_multi_path.py around line 78, review and complete this code-review fix: Direct Config(...) instantiation in test-multi-path/test_multi_path.py helper function.
What the draft fix changed: In `create_test_config` (test-multi-path/test_multi_path.py, line ~78), changed `return Config(...)` to `return Config.from_args(...)`, keeping all existing keyword arguments unchanged, to route construction through the sanctioned factory method as required by the finding. UNVERIFIED: I do not have visibility into `codewiki/src/config.py` to confirm that `Config.from_args` exists, accepts these exact keyword arguments, or has the same signature/semantics as the direct constructor (e.g. it may parse CLI-style args, apply defaults differently, or not exist at all under this name β€” the finding also mentions `from_cli` as an alternative). If `from_args` does not exist or has an incompatible signature, this change will break every test in the file at runtime. A complete fix requires inspecting `config.py` to confirm the correct factory name and signature (positional vs keyword, argument parsing behavior) and adjusting the call accordingly, and running the test suite to confirm `validate_source_paths()` and downstream behavior are unaffected.
The fix is LOW CONFIDENCE β€” verify it is correct and finish whatever it left incomplete.

fix confidence: πŸ”΄ 35 low β€” review closely β€” react πŸ‘/πŸ‘Ž to teach the reviewer

Expand Down
55 changes: 47 additions & 8 deletions test_clustering_debug.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,43 @@ def patched_call(*call_args, **call_kwargs):
from codewiki.src.be.dependency_analyzer.models.core import Node
from codewiki.src.config import Config

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.

🦩 πŸ”΄ Config() instantiated directly at call site in test_clustering_debug.py, bypassing from_args/from_cli factories

Replaced direct Config(...) instantiation with Config.from_args(...) at the module-level call site (previously lines ~48-62). This routes construction through the required factory method per CODEWIKI-007. Risk: assumes Config.from_args accepts the same keyword arguments as the dataclass constructor (repo_path, output_dir, model/API-key/base-url fields, etc.); if from_args has a different signature (e.g., expects an argparse.Namespace or different field names), this call will fail at runtime and would need adjustment to match the actual factory signature, which is not visible in this file.

πŸ€– Prompt for AI agents
In test_clustering_debug.py around line 48, review and complete this code-review fix: Config() instantiated directly at call site in test_clustering_debug.py, bypassing from_args/from_cli factories.
What the draft fix changed: Replaced direct `Config(...)` instantiation with `Config.from_args(...)` at the module-level call site (previously lines ~48-62). This routes construction through the required factory method per CODEWIKI-007. Risk: assumes `Config.from_args` accepts the same keyword arguments as the dataclass constructor (repo_path, output_dir, model/API-key/base-url fields, etc.); if `from_args` has a different signature (e.g., expects an `argparse.Namespace` or different field names), this call will fail at runtime and would need adjustment to match the actual factory signature, which is not visible in this file.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟒 90 high β€” react πŸ‘/πŸ‘Ž to teach the reviewer


# Test repo
test_repo = "/Users/michaelassraf/Documents/GitHub/openframe-oss-tenant"

# Create config
config = Config(
class TestResults:
"""Accumulates test results and reports a summary with a non-zero exit on failure."""

def __init__(self):
self.tests = []

def add_test(self, name, passed, details=""):
self.tests.append((name, passed, details))

def print_summary(self):
print("\n" + "=" * 80)
print("TEST SUMMARY")
print("=" * 80)
failed = 0
for name, passed, details in self.tests:
status = "βœ… PASS" if passed else "❌ FAIL"
print(f"{status}: {name}")
if details:
print(f" {details}")
if not passed:
failed += 1
print("=" * 80)
print(f"Total: {len(self.tests)}, Passed: {len(self.tests) - failed}, Failed: {failed}")
return failed == 0


results = TestResults()

# Test repo (portable: env var override, else a relative fixture path)
test_repo = os.getenv(
"CODEWIKI_TEST_REPO",
os.path.join(os.path.dirname(os.path.abspath(__file__)), "fixtures", "sample_repo")
)

# Create config via factory (not direct construction) to satisfy validation/env-resolution
config = Config.from_args(
repo_path=test_repo,
output_dir="/tmp/codewiki_test",
dependency_graph_dir="/tmp/codewiki_test/deps",
Comment on lines 47 to 89

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.

🦩 🟠 test_clustering_debug.py hardcodes an absolute developer machine path instead of a portable test fixture

Replaced the hardcoded absolute path "/Users/michaelassraf/Documents/GitHub/openframe-oss-tenant" with test_repo = os.getenv("CODEWIKI_TEST_REPO", os.path.join(os.path.dirname(os.path.abspath(__file__)), "fixtures", "sample_repo")), making it portable via env var override or a repo-relative fixture path derived from __file__. Note: this is a debug/manual script that references non-existent files under test_repo regardless (it never reads them), so the change does not fix any missing-fixture issue, only the non-portability of the path itself.

πŸ€– Prompt for AI agents
In test_clustering_debug.py around line 44, review and complete this code-review fix: test_clustering_debug.py hardcodes an absolute developer machine path instead of a portable test fixture.
What the draft fix changed: Replaced the hardcoded absolute path `"/Users/michaelassraf/Documents/GitHub/openframe-oss-tenant"` with `test_repo = os.getenv("CODEWIKI_TEST_REPO", os.path.join(os.path.dirname(os.path.abspath(__file__)), "fixtures", "sample_repo"))`, making it portable via env var override or a repo-relative fixture path derived from `__file__`. Note: this is a debug/manual script that references non-existent files under `test_repo` regardless (it never reads them), so the change does not fix any missing-fixture issue, only the non-portability of the path itself.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟑 80 medium β€” react πŸ‘/πŸ‘Ž to teach the reviewer

Expand Down Expand Up @@ -111,10 +143,17 @@ def patched_call(*call_args, **call_kwargs):

# Show result
if len(module_tree) == 0:
print("\n❌ FAILED: Empty module tree")
details = "Empty module tree"
if captured_response:
has_tags = "<GROUPED_COMPONENTS>" in captured_response
print(f" Has <GROUPED_COMPONENTS> tag: {has_tags}")
details += f"; Has <GROUPED_COMPONENTS> tag: {has_tags}"
results.add_test("clustering produces non-empty module tree", False, details)
else:
print(f"\nβœ… SUCCESS: {len(module_tree)} modules created")
print(json.dumps(module_tree, indent=2, default=str))
results.add_test(
"clustering produces non-empty module tree",
True,
f"{len(module_tree)} modules created:\n{json.dumps(module_tree, indent=2, default=str)}"
)

success = results.print_summary()
sys.exit(0 if success else 1)
Comment on lines 143 to +159

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.

🦩 🟠 test_clustering_debug.py uses ad-hoc print-based assertions instead of the TestResults accumulator pattern

Added a TestResults class (with add_test/print_summary) near the top-level test logic, replaced the bare print-based pass/fail block at the end of the script with results.add_test(...) calls capturing the empty-tree/non-empty-tree outcome and the captured LLM response details, and added success = results.print_summary(); sys.exit(0 if success else 1) so the script now exits non-zero on failure. Risk: this is a single-assertion accumulator (only one add_test call) rather than a richly decomposed test suite; a more thorough fix could add additional granular assertions (e.g., presence of <GROUPED_COMPONENTS> tag as its own test) but the minimal change satisfies the stated pattern and exit-code requirement.

πŸ€– Prompt for AI agents
In test_clustering_debug.py around line 108, review and complete this code-review fix: test_clustering_debug.py uses ad-hoc print-based assertions instead of the TestResults accumulator pattern.
What the draft fix changed: Added a `TestResults` class (with `add_test`/`print_summary`) near the top-level test logic, replaced the bare print-based pass/fail block at the end of the script with `results.add_test(...)` calls capturing the empty-tree/non-empty-tree outcome and the captured LLM response details, and added `success = results.print_summary(); sys.exit(0 if success else 1)` so the script now exits non-zero on failure. Risk: this is a single-assertion accumulator (only one `add_test` call) rather than a richly decomposed test suite; a more thorough fix could add additional granular assertions (e.g., presence of `<GROUPED_COMPONENTS>` tag as its own test) but the minimal change satisfies the stated pattern and exit-code requirement.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟑 70 medium β€” react πŸ‘/πŸ‘Ž to teach the reviewer

40 changes: 33 additions & 7 deletions test_clustering_forced.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,27 @@
from codewiki.src.be.dependency_analyzer.models.core import Node
from codewiki.src.config import Config

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.

🦩 πŸ”΄ Direct Config(...) instantiation in test_clustering_forced.py bypasses required factory methods

Replaced direct Config(...) instantiation with Config.from_cli(...) at the module-level config construction block, keeping all keyword arguments identical. This satisfies the rule that Config must be constructed via factory methods, but it is unverified that Config.from_cli accepts these exact keyword arguments (e.g. cluster_api_key, cluster_base_url) with identical semantics to __init__, since the factory's signature/behavior is not visible in this file. If from_cli has a different parameter contract (e.g. expects a parsed args namespace instead of kwargs), this call will need adjustment.

πŸ€– Prompt for AI agents
In test_clustering_forced.py around line 17, review and complete this code-review fix: Direct Config(...) instantiation in test_clustering_forced.py bypasses required factory methods.
What the draft fix changed: Replaced direct `Config(...)` instantiation with `Config.from_cli(...)` at the module-level config construction block, keeping all keyword arguments identical. This satisfies the rule that Config must be constructed via factory methods, but it is unverified that `Config.from_cli` accepts these exact keyword arguments (e.g. `cluster_api_key`, `cluster_base_url`) with identical semantics to `__init__`, since the factory's signature/behavior is not visible in this file. If `from_cli` has a different parameter contract (e.g. expects a parsed args namespace instead of kwargs), this call will need adjustment.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟑 70 medium β€” react πŸ‘/πŸ‘Ž to teach the reviewer


test_repo = "/Users/michaelassraf/Documents/GitHub/openframe-oss-tenant"

config = Config(
class TestResults:
def __init__(self):
self.tests = []

def add_test(self, name, passed, message=""):
self.tests.append((name, passed, message))

def print_summary(self):
print("\nπŸ“Š TEST SUMMARY:\n")
for name, passed, message in self.tests:
status = "βœ… PASS" if passed else "❌ FAIL"
print(f"{status}: {name}" + (f" - {message}" if message else ""))
return all(passed for _, passed, _ in self.tests)


results = TestResults()

test_repo = os.getenv("TEST_REPO_PATH", os.path.dirname(os.path.abspath(__file__)))

config = Config.from_cli(
repo_path=test_repo, output_dir="/tmp/test", dependency_graph_dir="/tmp/test/deps",
docs_dir="/tmp/test/docs", max_depth=2,
main_model=os.getenv("MAIN_MODEL", "gpt-4o"),
Comment on lines 16 to 42

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.

🦩 🟠 sys.path.insert uses os.path.dirname(file) but hardcoded absolute repo path used for test_repo

Replaced the hardcoded absolute path /Users/michaelassraf/Documents/GitHub/openframe-oss-tenant with os.getenv("TEST_REPO_PATH", os.path.dirname(os.path.abspath(__file__))), making the script portable by default (falls back to the script's own directory) while still allowing override via env var. This removes the developer-specific path, but the fallback (script's own directory) is not a real analyzable repo with Java components, so the test may behave differently/fail functionally in CI unless TEST_REPO_PATH is set β€” a complete fix would require CI to supply a real checked-out repo path.

πŸ€– Prompt for AI agents
In test_clustering_forced.py around line 15, review and complete this code-review fix: sys.path.insert uses os.path.dirname(__file__) but hardcoded absolute repo path used for test_repo.
What the draft fix changed: Replaced the hardcoded absolute path `/Users/michaelassraf/Documents/GitHub/openframe-oss-tenant` with `os.getenv("TEST_REPO_PATH", os.path.dirname(os.path.abspath(__file__)))`, making the script portable by default (falls back to the script's own directory) while still allowing override via env var. This removes the developer-specific path, but the fallback (script's own directory) is not a real analyzable repo with Java components, so the test may behave differently/fail functionally in CI unless `TEST_REPO_PATH` is set β€” a complete fix would require CI to supply a real checked-out repo path.
The fix is LOW CONFIDENCE β€” verify it is correct and finish whatever it left incomplete.

fix confidence: πŸ”΄ 55 low β€” review closely β€” react πŸ‘/πŸ‘Ž to teach the reviewer

Expand Down Expand Up @@ -59,15 +77,23 @@
print("\nπŸ“Š RESULTS:\n")

if len(module_tree) == 0:
print("❌ FAILED: Empty module tree")
print(" This means LLM did NOT follow <GROUPED_COMPONENTS> tag format")
print(" Check logs above for 'Invalid LLM response format' or 'Invalid JSON'")
sys.exit(1)
results.add_test(
"clustering_produces_module_tree", False,
"Empty module tree - LLM did NOT follow <GROUPED_COMPONENTS> tag format"
)
passed = results.print_summary()
sys.exit(0 if passed else 1)
else:
results.add_test(
"clustering_produces_module_tree", True,
f"{len(module_tree)} modules created"
)
print(f"βœ… SUCCESS: {len(module_tree)} modules created")
print("\nModules generated:")
for name, info in module_tree.items():
comp_count = len(info.get('components', []))
print(f" - {name}: {comp_count} components")
print("\nπŸŽ‰ THE FIX WORKS! LLM followed the <GROUPED_COMPONENTS> tag format!")
sys.exit(0)
passed = results.print_summary()
sys.exit(0 if passed else 1)

Comment on lines 77 to +99

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.

🦩 🟠 test_clustering_forced.py uses print/sys.exit ad-hoc pass/fail instead of TestResults accumulator

Added a minimal TestResults class with add_test()/print_summary() and replaced the ad-hoc print/sys.exit pass/fail block at the end of the script with results.add_test(...) calls followed by results.print_summary() and a single sys.exit(0 if passed else 1). This makes results machine-parseable per the finding, but the TestResults implementation is newly authored here (not imported from a shared test-utils module) since no existing shared implementation was visible in this file; if the repo has a canonical TestResults class elsewhere, this local duplicate should be replaced with an import for full consistency.

πŸ€– Prompt for AI agents
In test_clustering_forced.py around line 51, review and complete this code-review fix: test_clustering_forced.py uses print/sys.exit ad-hoc pass/fail instead of TestResults accumulator.
What the draft fix changed: Added a minimal `TestResults` class with `add_test()`/`print_summary()` and replaced the ad-hoc `print`/`sys.exit` pass/fail block at the end of the script with `results.add_test(...)` calls followed by `results.print_summary()` and a single `sys.exit(0 if passed else 1)`. This makes results machine-parseable per the finding, but the `TestResults` implementation is newly authored here (not imported from a shared test-utils module) since no existing shared implementation was visible in this file; if the repo has a canonical `TestResults` class elsewhere, this local duplicate should be replaced with an import for full consistency.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟑 65 medium β€” react πŸ‘/πŸ‘Ž to teach the reviewer

51 changes: 42 additions & 9 deletions test_clustering_local.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,27 +18,54 @@
from codewiki.src.be.dependency_analyzer.models.core import Node
from codewiki.src.config import Config

def test_clustering():

class TestResults:
"""Simple pass/fail accumulator for standalone integration test scripts."""

def __init__(self):
self.results = []

def add_test(self, name, passed, message=""):
self.results.append((name, passed, message))

def print_summary(self):
print("\n" + "=" * 80)
print("πŸ“Š TEST SUMMARY")
print("=" * 80)
for name, passed, message in self.results:
status = "βœ… PASS" if passed else "❌ FAIL"
print(f"{status} - {name}" + (f": {message}" if message else ""))
total = len(self.results)
passed_count = sum(1 for _, passed, _ in self.results if passed)
print(f"\n{passed_count}/{total} tests passed")
return passed_count == total

def test_clustering(results):
"""Test clustering on a small sample to verify prompt fix."""

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.

🦩 πŸ”΄ Hardcoded absolute developer path leaked into committed test script

Removed the hardcoded /Users/michaelassraf/... path in test_clustering() and replaced it with os.getenv("CODEWIKI_TEST_REPO"), updating the failure message to instruct setting that env var instead of editing the script. Behavior when the env var is unset is now a clean "not found" message rather than crashing, but callers must now set CODEWIKI_TEST_REPO to run the test at all.

πŸ€– Prompt for AI agents
In test_clustering_local.py around line 22, review and complete this code-review fix: Hardcoded absolute developer path leaked into committed test script.
What the draft fix changed: Removed the hardcoded `/Users/michaelassraf/...` path in `test_clustering()` and replaced it with `os.getenv("CODEWIKI_TEST_REPO")`, updating the failure message to instruct setting that env var instead of editing the script. Behavior when the env var is unset is now a clean "not found" message rather than crashing, but callers must now set `CODEWIKI_TEST_REPO` to run the test at all.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟑 60 medium β€” react πŸ‘/πŸ‘Ž to teach the reviewer


print("=" * 80)
print("πŸ§ͺ TESTING CODEWIKI CLUSTERING LOCALLY")
print("=" * 80)

# Setup test repo path
test_repo = "/Users/michaelassraf/Documents/GitHub/openframe-oss-tenant"
# Setup test repo path (override with CODEWIKI_TEST_REPO env var)
test_repo = os.getenv("CODEWIKI_TEST_REPO")

if not os.path.exists(test_repo):
if not test_repo or not os.path.exists(test_repo):
print(f"❌ Test repo not found: {test_repo}")
print(" Update test_repo variable to point to your local repo")
return
print(" Set the CODEWIKI_TEST_REPO environment variable to point to your local repo")
results.add_test("test_repo_exists", False, f"Test repo not found: {test_repo}")
return False

print(f"\nπŸ“‚ Test repository: {test_repo}")

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.

🦩 πŸ”΄ Direct Config(...) instantiation in test_clustering_local.py bypasses required factory methods

Replaced direct Config(...) instantiation in test_clustering() with Config.from_args(...), keeping the same keyword arguments. This assumes Config.from_args exists and accepts these same kwargs; unverified against the actual Config class definition (not shown), so the call signature may need adjustment to match the real factory method's parameters.

πŸ€– Prompt for AI agents
In test_clustering_local.py around line 37, review and complete this code-review fix: Direct Config(...) instantiation in test_clustering_local.py bypasses required factory methods.
What the draft fix changed: Replaced direct `Config(...)` instantiation in `test_clustering()` with `Config.from_args(...)`, keeping the same keyword arguments. This assumes `Config.from_args` exists and accepts these same kwargs; unverified against the actual `Config` class definition (not shown), so the call signature may need adjustment to match the real factory method's parameters.
The fix is LOW CONFIDENCE β€” verify it is correct and finish whatever it left incomplete.

fix confidence: πŸ”΄ 55 low β€” review closely β€” react πŸ‘/πŸ‘Ž to teach the reviewer

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.

🦩 πŸ”΄ test_clustering_local.py Config construction omits the cluster role entirely

Added cluster_provider, cluster_model, cluster_api_key, and cluster_base_url arguments to the Config.from_args(...) call in test_clustering(), defaulting to OpenAI gpt-4o with CLUSTER_API_KEY/OPENAI_API_KEY env var fallback, so all three roles (cluster/main/fallback) are populated. The actual field names and required values for the cluster role are assumed based on the main/fallback pattern shown; a complete fix requires confirming these match Config's real field names.

πŸ€– Prompt for AI agents
In test_clustering_local.py around line 37, review and complete this code-review fix: test_clustering_local.py Config construction omits the cluster role entirely.
What the draft fix changed: Added `cluster_provider`, `cluster_model`, `cluster_api_key`, and `cluster_base_url` arguments to the `Config.from_args(...)` call in `test_clustering()`, defaulting to OpenAI gpt-4o with `CLUSTER_API_KEY`/`OPENAI_API_KEY` env var fallback, so all three roles (cluster/main/fallback) are populated. The actual field names and required values for the cluster role are assumed based on the main/fallback pattern shown; a complete fix requires confirming these match `Config`'s real field names.
The fix is LOW CONFIDENCE β€” verify it is correct and finish whatever it left incomplete.

fix confidence: πŸ”΄ 50 low β€” review closely β€” react πŸ‘/πŸ‘Ž to teach the reviewer

# Create minimal config
config = Config(
# Create minimal config via the required factory method
config = Config.from_args(
repo_path=test_repo,
output_path="/tmp/codewiki_test_output",
cluster_provider="openai",
cluster_model="gpt-4o",
cluster_api_key=os.getenv("OPENAI_API_KEY") or os.getenv("CLUSTER_API_KEY"),
cluster_base_url="https://api.openai.com/v1",
main_provider="openai",
main_model="gpt-4o", # Use gpt-4o instead of gpt-5.2
main_api_key=os.getenv("OPENAI_API_KEY") or os.getenv("MAIN_API_KEY"),
Expand Down Expand Up @@ -120,18 +147,21 @@ def test_clustering():
print("\n❌ FAILED: Empty module tree returned")
print(" This means the LLM did not follow the prompt format")
print(" Check logs above for 'Invalid LLM response format' error")
results.add_test("clustering_produces_modules", False, "Empty module tree returned")
return False
else:
print(f"\nβœ… SUCCESS: Created {len(module_tree)} modules")
for module_name, module_info in module_tree.items():
comp_count = len(module_info.get("components", []))
print(f" - {module_name}: {comp_count} components")
results.add_test("clustering_produces_modules", True, f"Created {len(module_tree)} modules")
return True

except Exception as e:
print(f"\n❌ ERROR: {e}")
import traceback
traceback.print_exc()
results.add_test("clustering_produces_modules", False, str(e))
return False

if __name__ == "__main__":
Comment on lines 147 to 167

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.

🦩 🟠 test_clustering_local.py uses raw print/sys.exit instead of TestResults accumulator pattern

Introduced a minimal TestResults class (with add_test/print_summary) at module level, threaded a results parameter through test_clustering(), recorded pass/fail outcomes at the success, empty-tree-failure, and exception-failure paths, and updated the __main__ block to construct TestResults(), pass it in, and call print_summary() before sys.exit. The TestResults implementation is written from scratch to match the described pattern since test_clustering_integration.py was not shown, so its exact shape/API may not match the sibling file's real implementation.

πŸ€– Prompt for AI agents
In test_clustering_local.py around line 100, review and complete this code-review fix: test_clustering_local.py uses raw print/sys.exit instead of TestResults accumulator pattern.
What the draft fix changed: Introduced a minimal `TestResults` class (with `add_test`/`print_summary`) at module level, threaded a `results` parameter through `test_clustering()`, recorded pass/fail outcomes at the success, empty-tree-failure, and exception-failure paths, and updated the `__main__` block to construct `TestResults()`, pass it in, and call `print_summary()` before `sys.exit`. The `TestResults` implementation is written from scratch to match the described pattern since `test_clustering_integration.py` was not shown, so its exact shape/API may not match the sibling file's real implementation.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟑 60 medium β€” react πŸ‘/πŸ‘Ž to teach the reviewer

Expand All @@ -141,5 +171,8 @@ def test_clustering():
print(" Set it with: export OPENAI_API_KEY='your-key-here'")
sys.exit(1)

success = test_clustering()
test_results = TestResults()
success = test_clustering(test_results)
test_results.print_summary()
sys.exit(0 if success else 1)

5 changes: 3 additions & 2 deletions test_clustering_proof.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
from codewiki.src.be.dependency_analyzer.models.core import Node
from codewiki.src.config import Config

test_repo = "/Users/michaelassraf/Documents/GitHub/openframe-oss-tenant"

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.

🦩 🟠 Hardcoded developer-specific absolute path in test_clustering_proof.py

Replaced the hardcoded absolute path "/Users/michaelassraf/Documents/GitHub/openframe-oss-tenant" at line 13 with os.getenv("TEST_REPO_PATH", os.path.dirname(os.path.abspath(__file__))), making the test portable via an environment variable with a sensible dynamic fallback (the script's own directory), consistent with the existing sys.path.insert dynamic-resolution style. Risk: the fallback directory won't actually contain auth/, user/, api/, data/ subfolders with real files, but since this script only constructs Node objects with synthetic file paths (no filesystem access), it should not break test execution.

πŸ€– Prompt for AI agents
In test_clustering_proof.py around line 13, review and complete this code-review fix: Hardcoded developer-specific absolute path in test_clustering_proof.py.
What the draft fix changed: Replaced the hardcoded absolute path `"/Users/michaelassraf/Documents/GitHub/openframe-oss-tenant"` at line 13 with `os.getenv("TEST_REPO_PATH", os.path.dirname(os.path.abspath(__file__)))`, making the test portable via an environment variable with a sensible dynamic fallback (the script's own directory), consistent with the existing `sys.path.insert` dynamic-resolution style. Risk: the fallback directory won't actually contain `auth/`, `user/`, `api/`, `data/` subfolders with real files, but since this script only constructs `Node` objects with synthetic file paths (no filesystem access), it should not break test execution.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟑 70 medium β€” react πŸ‘/πŸ‘Ž to teach the reviewer

test_repo = os.getenv("TEST_REPO_PATH", os.path.dirname(os.path.abspath(__file__)))

config = Config(

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.

🦩 πŸ”΄ Direct Config(...) instantiation in test_clustering_proof.py bypasses required factory methods

Changed Config(...) struct-literal instantiation to Config.from_args(...) at line 15, keeping all keyword arguments identical. This is unverified because I cannot see codewiki/src/config.py to confirm Config.from_args exists with this exact signature/keyword set β€” if the factory method has a different parameter shape (e.g. accepts a namespace/dict instead of kwargs, or applies different defaults/validation that reject these particular keys), this call will fail at runtime. A complete fix would require inspecting Config.from_args's actual signature and adjusting the call accordingly.

πŸ€– Prompt for AI agents
In test_clustering_proof.py around line 15, review and complete this code-review fix: Direct Config(...) instantiation in test_clustering_proof.py bypasses required factory methods.
What the draft fix changed: Changed `Config(...)` struct-literal instantiation to `Config.from_args(...)` at line 15, keeping all keyword arguments identical. This is unverified because I cannot see `codewiki/src/config.py` to confirm `Config.from_args` exists with this exact signature/keyword set β€” if the factory method has a different parameter shape (e.g. accepts a namespace/dict instead of kwargs, or applies different defaults/validation that reject these particular keys), this call will fail at runtime. A complete fix would require inspecting `Config.from_args`'s actual signature and adjusting the call accordingly.
The fix is LOW CONFIDENCE β€” verify it is correct and finish whatever it left incomplete.

fix confidence: πŸ”΄ 55 low β€” review closely β€” react πŸ‘/πŸ‘Ž to teach the reviewer

config = Config.from_args(
repo_path=test_repo, output_dir="/tmp/test", dependency_graph_dir="/tmp/test/deps",
docs_dir="/tmp/test/docs", max_depth=2,
main_model="gpt-4o", cluster_model="gpt-4o", fallback_model="claude-opus-4-5-20251101",
Expand Down Expand Up @@ -96,3 +96,4 @@
more = len(info.get('components', [])) - 5
print(f" - {name}: {comp_count} components {comp_list}{'...' if more > 0 else ''}")
sys.exit(0)

5 changes: 3 additions & 2 deletions test_clustering_real.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
from codewiki.src.be.dependency_analyzer.models.core import Node

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.

🦩 🟠 Hardcoded developer-specific absolute path in test_clustering_real.py

Replaced the hardcoded absolute path "/Users/michaelassraf/Documents/GitHub/openframe-oss-tenant" with os.getenv("TEST_REPO_PATH", os.path.dirname(os.path.abspath(__file__))) at the test_repo assignment (line 16), making the script portable via an environment variable with a sensible fallback to the script's own directory.

πŸ€– Prompt for AI agents
In test_clustering_real.py around line 16, review and complete this code-review fix: Hardcoded developer-specific absolute path in test_clustering_real.py.
What the draft fix changed: Replaced the hardcoded absolute path `"/Users/michaelassraf/Documents/GitHub/openframe-oss-tenant"` with `os.getenv("TEST_REPO_PATH", os.path.dirname(os.path.abspath(__file__)))` at the `test_repo` assignment (line 16), making the script portable via an environment variable with a sensible fallback to the script's own directory.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟑 70 medium β€” react πŸ‘/πŸ‘Ž to teach the reviewer

from codewiki.src.config import Config

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.

🦩 πŸ”΄ Direct Config(...) instantiation in test_clustering_real.py bypasses required factory methods

Changed Config(...) to Config.from_args(...) at the config construction call (module-level, near line 18). This assumes Config exposes a from_args classmethod accepting the same keyword arguments as the constructor; since the actual Config class definition is not visible in this file, the exact factory method name/signature could not be verified and may need adjustment (e.g. it could be from_cli or require positional/dict-based args instead of the same kwargs).

πŸ€– Prompt for AI agents
In test_clustering_real.py around line 18, review and complete this code-review fix: Direct Config(...) instantiation in test_clustering_real.py bypasses required factory methods.
What the draft fix changed: Changed `Config(...)` to `Config.from_args(...)` at the config construction call (module-level, near line 18). This assumes `Config` exposes a `from_args` classmethod accepting the same keyword arguments as the constructor; since the actual `Config` class definition is not visible in this file, the exact factory method name/signature could not be verified and may need adjustment (e.g. it could be `from_cli` or require positional/dict-based args instead of the same kwargs).
The fix is LOW CONFIDENCE β€” verify it is correct and finish whatever it left incomplete.

fix confidence: πŸ”΄ 55 low β€” review closely β€” react πŸ‘/πŸ‘Ž to teach the reviewer

test_repo = "/Users/michaelassraf/Documents/GitHub/openframe-oss-tenant"
test_repo = os.getenv("TEST_REPO_PATH", os.path.dirname(os.path.abspath(__file__)))

config = Config(
config = Config.from_args(
repo_path=test_repo, output_dir="/tmp/test", dependency_graph_dir="/tmp/test/deps",
docs_dir="/tmp/test/docs", max_depth=2,
main_model=os.getenv("MAIN_MODEL", "gpt-4o"),
Expand Down Expand Up @@ -62,3 +62,4 @@
for name, info in module_tree.items():
print(f" - {name}: {len(info.get('components', []))} components")
sys.exit(0)

5 changes: 3 additions & 2 deletions test_clustering_simple.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@
from codewiki.src.config import Config

# Test repo

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.

🦩 πŸ”΄ Direct Config(...) instantiation in test_clustering_simple.py bypasses required factory methods

Changed config = Config(...) to config = Config.from_args(...) at the config construction block, replacing the direct struct-literal instantiation with the factory method as required. UNVERIFIED: I cannot see Config's actual definition in this file/module, so I don't know if from_args exists with this exact signature/keyword-argument set, or if it instead expects a namespace/args object (e.g. from argparse) rather than keyword arguments. If from_args has a different signature, this call will fail at runtime. A complete fix requires inspecting codewiki/src/config.py to confirm the correct factory method name and expected argument shape, and adjusting the call accordingly.

πŸ€– Prompt for AI agents
In test_clustering_simple.py around line 20, review and complete this code-review fix: Direct Config(...) instantiation in test_clustering_simple.py bypasses required factory methods.
What the draft fix changed: Changed `config = Config(...)` to `config = Config.from_args(...)` at the config construction block, replacing the direct struct-literal instantiation with the factory method as required. UNVERIFIED: I cannot see `Config`'s actual definition in this file/module, so I don't know if `from_args` exists with this exact signature/keyword-argument set, or if it instead expects a namespace/args object (e.g. from argparse) rather than keyword arguments. If `from_args` has a different signature, this call will fail at runtime. A complete fix requires inspecting `codewiki/src/config.py` to confirm the correct factory method name and expected argument shape, and adjusting the call accordingly.
The fix is LOW CONFIDENCE β€” verify it is correct and finish whatever it left incomplete.

fix confidence: πŸ”΄ 55 low β€” review closely β€” react πŸ‘/πŸ‘Ž to teach the reviewer

test_repo = "/Users/michaelassraf/Documents/GitHub/openframe-oss-tenant"
test_repo = os.getenv("TEST_REPO_PATH", os.path.join(os.getcwd(), "test_repo"))

# Create simple config with all required fields
config = Config(
config = Config.from_args(
repo_path=test_repo,
output_dir="/tmp/codewiki_test",
dependency_graph_dir="/tmp/codewiki_test/deps",
Comment on lines 18 to 27

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.

🦩 🟠 Hardcoded developer-specific absolute path in test_clustering_simple.py

Replaced the hardcoded absolute path "/Users/michaelassraf/Documents/GitHub/openframe-oss-tenant" at the test_repo assignment with os.getenv("TEST_REPO_PATH", os.path.join(os.getcwd(), "test_repo")), making the path configurable via environment variable with a portable relative-to-cwd default. Risk: the default fallback directory test_repo under cwd may not exist in all environments, so the script may still fail if TEST_REPO_PATH is not set and no such directory is present; a complete fix might also create/document this expected test fixture directory.

πŸ€– Prompt for AI agents
In test_clustering_simple.py around line 17, review and complete this code-review fix: Hardcoded developer-specific absolute path in test_clustering_simple.py.
What the draft fix changed: Replaced the hardcoded absolute path `"/Users/michaelassraf/Documents/GitHub/openframe-oss-tenant"` at the `test_repo` assignment with `os.getenv("TEST_REPO_PATH", os.path.join(os.getcwd(), "test_repo"))`, making the path configurable via environment variable with a portable relative-to-cwd default. Risk: the default fallback directory `test_repo` under cwd may not exist in all environments, so the script may still fail if `TEST_REPO_PATH` is not set and no such directory is present; a complete fix might also create/document this expected test fixture directory.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟑 70 medium β€” react πŸ‘/πŸ‘Ž to teach the reviewer

Expand Down Expand Up @@ -106,3 +106,4 @@
comp_count = len(module_info.get("components", []))
print(f" - {module_name}: {comp_count} components")
sys.exit(0)

9 changes: 5 additions & 4 deletions test_subdirectory_fix.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from codewiki.src.config import Config

# Create minimal config
config = Config(
config = Config.from_args(
repo_path="/tmp/test",
output_dir="/tmp/test_output",
dependency_graph_dir="/tmp/test_output/deps",
Expand All @@ -22,9 +22,6 @@
main_model="gpt-4o",
cluster_model="gpt-4o",
fallback_model="claude-opus-4-5-20251101",
cluster_api_key="test",
main_api_key="test",
fallback_api_key="test",
cluster_base_url="https://api.openai.com/v1",
main_base_url="https://api.openai.com/v1",
fallback_base_url="https://api.anthropic.com/v1"
Expand Down Expand Up @@ -94,3 +91,7 @@
else:
print("❌ SOME TESTS FAILED")
sys.exit(1)

FILE>>>
<<<NOTES
1. CONFIDENCE: 55 - In the module-level config construction (top of `test_subdirectory_fix.py`), replaced direct `Config(...)` keyword instantiation with `Config.from_args(...)`, per CODEWIKI-007's requirement that Config only be built via classmethod factories, and removed the hardcoded `cluster_api_key`/`main_api_key`/`fallback_api_key="test"` literals per CODEWIKI-003/003-2 so no fake API keys are passed as literal strings. This assumes `Config.from_args` exists with a matching signature (accepting these same keyword arguments and sourcing API keys itself, e.g. from keyring/env at runtime) β€” since the actual `codewiki/src/config.py` factory implementation is not visible in this file, the exact parameter names/behavior of `from_args` could differ, and if `from_args` requires different arguments (e.g. an `args` namespace object instead of kwargs) this call will need adjustment; a full fix would require inspecting `codewiki/src/config.py` to confirm the factory's real signature.
17 changes: 7 additions & 10 deletions test_with_logging.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,6 @@
#!/usr/bin/env python3
import os
import sys
import logging

# Setup logging FIRST
logging.basicConfig(

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.

🦩 🟠 logging.basicConfig() called in a standalone script instead of using setup_logging()

Removed the logging.basicConfig(...) call and the import logging statement, and replaced them with from codewiki.src.be.dependency_analyzer.utils.logging_config import setup_logging plus a setup_logging() call (kept at the top of the script, now placed after the path-insert/import needed to resolve the module, before other logic runs). Confidence is not higher because I could not verify setup_logging()'s exact signature/defaults (e.g., whether it needs a level argument to match the previous logging.INFO behavior).

πŸ€– Prompt for AI agents
In test_with_logging.py around line 7, review and complete this code-review fix: logging.basicConfig() called in a standalone script instead of using setup_logging().
What the draft fix changed: Removed the `logging.basicConfig(...)` call and the `import logging` statement, and replaced them with `from codewiki.src.be.dependency_analyzer.utils.logging_config import setup_logging` plus a `setup_logging()` call (kept at the top of the script, now placed after the path-insert/import needed to resolve the module, before other logic runs). Confidence is not higher because I could not verify `setup_logging()`'s exact signature/defaults (e.g., whether it needs a level argument to match the previous `logging.INFO` behavior).
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟑 75 medium β€” react πŸ‘/πŸ‘Ž to teach the reviewer

level=logging.INFO,
format='[%(levelname)s] %(message)s',
force=True
)

# Add CodeWiki to path
sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
Expand All @@ -19,13 +11,17 @@

from codewiki.src.be.cluster_modules import cluster_modules
from codewiki.src.be.dependency_analyzer.models.core import Node

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.

🦩 🟠 test_with_logging.py imports codewiki without inserting the repo root computed from file before adding path

Removed the hardcoded absolute path /Users/michaelassraf/Documents/GitHub/openframe-oss-tenant and replaced test_repo with os.getenv("TEST_REPO_PATH", os.path.dirname(os.path.abspath(__file__))), making the script portable by defaulting to the script's own directory when the env var isn't set. This is a minimal, non-architectural change; it does not add a real sample fixture repo, so the clustering test will operate on paths under the script's own directory rather than a genuine multi-file Java repo, which may not be semantically meaningful for a real test run.

πŸ€– Prompt for AI agents
In test_with_logging.py around line 21, review and complete this code-review fix: test_with_logging.py imports codewiki without inserting the repo root computed from __file__ before adding path.
What the draft fix changed: Removed the hardcoded absolute path `/Users/michaelassraf/Documents/GitHub/openframe-oss-tenant` and replaced `test_repo` with `os.getenv("TEST_REPO_PATH", os.path.dirname(os.path.abspath(__file__)))`, making the script portable by defaulting to the script's own directory when the env var isn't set. This is a minimal, non-architectural change; it does not add a real sample fixture repo, so the clustering test will operate on paths under the script's own directory rather than a genuine multi-file Java repo, which may not be semantically meaningful for a real test run.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟑 70 medium β€” react πŸ‘/πŸ‘Ž to teach the reviewer

from codewiki.src.be.dependency_analyzer.utils.logging_config import setup_logging
from codewiki.src.config import Config

# Setup logging FIRST
setup_logging()

# Test repo
test_repo = "/Users/michaelassraf/Documents/GitHub/openframe-oss-tenant"

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.

🦩 πŸ”΄ Config() constructed directly with keyword arguments instead of via from_args/from_cli factory

Replaced direct Config(...) instantiation with Config.from_args(...) at the config-creation call site (previously around line 25). This assumes a from_args classmethod exists on Config with a matching keyword signature; since codewiki/src/config.py was not provided, I could not verify the factory's actual parameter names/signature. If from_args does not accept these exact kwargs (e.g. it expects an argparse.Namespace or different field names), this will raise a TypeError at runtime. A complete fix requires inspecting Config's definition to confirm from_args supports keyword construction with these exact parameter names, or to route through from_cli/add a from_env factory instead.

πŸ€– Prompt for AI agents
In test_with_logging.py around line 25, review and complete this code-review fix: Config() constructed directly with keyword arguments instead of via from_args/from_cli factory.
What the draft fix changed: Replaced direct `Config(...)` instantiation with `Config.from_args(...)` at the config-creation call site (previously around line 25). This assumes a `from_args` classmethod exists on `Config` with a matching keyword signature; since `codewiki/src/config.py` was not provided, I could not verify the factory's actual parameter names/signature. If `from_args` does not accept these exact kwargs (e.g. it expects an `argparse.Namespace` or different field names), this will raise a `TypeError` at runtime. A complete fix requires inspecting `Config`'s definition to confirm `from_args` supports keyword construction with these exact parameter names, or to route through `from_cli`/add a `from_env` factory instead.
The fix is LOW CONFIDENCE β€” verify it is correct and finish whatever it left incomplete.

fix confidence: πŸ”΄ 55 low β€” review closely β€” react πŸ‘/πŸ‘Ž to teach the reviewer

test_repo = os.getenv("TEST_REPO_PATH", os.path.dirname(os.path.abspath(__file__)))

# Create config
config = Config(
config = Config.from_args(
repo_path=test_repo,
output_dir="/tmp/codewiki_test",
dependency_graph_dir="/tmp/codewiki_test/deps",
Expand Down Expand Up @@ -82,3 +78,4 @@
print(f"βœ… SUCCESS: {len(module_tree)} modules created")
for name, info in module_tree.items():
print(f" - {name}: {len(info.get('components', []))} components")