fix(CODEWIKI-007): CU-86akhf8u6 3 review findings across 3 files - #76
flamingo[bot] wants to merge 3 commits into
Conversation
| @@ -83,7 +83,7 @@ def print_summary(self): | |||
| ) | |||
|
|
|||
| # Create config via factory (not direct construction) to satisfy validation/env-resolution | |||
There was a problem hiding this comment.
🦩 🔴 Direct Config(...) instantiation bypasses required classmethod factories
Changed the Config(...) direct instantiation at the config-creation block to Config.from_args(...), passing the same keyword arguments, so the comment claiming factory usage matches the actual call. This assumes Config.from_args exists in codewiki/src/config.py and accepts the same keyword arguments as the constructor (repo_path, output_dir, dependency_graph_dir, docs_dir, max_depth, main_model, cluster_model, fallback_model, and the api_key/base_url fields). I could not view codewiki/src/config.py to confirm the factory's exact signature or that it forwards these kwargs unchanged to validation/env-resolution logic — if from_args has a different signature (e.g. it parses argparse.Namespace rather than kwargs, or omits/renames some fields), this call will raise a TypeError at runtime. A complete fix requires inspecting config.py to confirm from_args's actual signature and adjusting the call accordingly, or using from_cli instead if that is the kwarg-based factory.
🤖 Prompt for AI agents
In test_clustering_debug.py around line 85, review and complete this code-review fix: Direct Config(...) instantiation bypasses required classmethod factories.
What the draft fix changed: Changed the `Config(...)` direct instantiation at the config-creation block to `Config.from_args(...)`, passing the same keyword arguments, so the comment claiming factory usage matches the actual call. This assumes `Config.from_args` exists in `codewiki/src/config.py` and accepts the same keyword arguments as the constructor (repo_path, output_dir, dependency_graph_dir, docs_dir, max_depth, main_model, cluster_model, fallback_model, and the api_key/base_url fields). I could not view `codewiki/src/config.py` to confirm the factory's exact signature or that it forwards these kwargs unchanged to validation/env-resolution logic — if `from_args` has a different signature (e.g. it parses `argparse.Namespace` rather than kwargs, or omits/renames some fields), this call will raise a `TypeError` at runtime. A complete fix requires inspecting `config.py` to confirm `from_args`'s actual signature and adjusting the call accordingly, or using `from_cli` instead if that is the kwarg-based factory.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.
fix confidence: 🔴 40 low — review closely — react 👍/👎 to teach the reviewer
| print(f"\n📂 Test repository: {test_repo}") | ||
|
|
||
| # Create minimal config via the required factory method | ||
| config = Config( |
There was a problem hiding this comment.
🦩 🔴 Direct Config(...) instantiation bypasses required from_cli/from_args factory
In test_clustering (test_clustering_local.py), replaced the direct Config(...) keyword-argument instantiation with Config.from_args(...), passing the same keyword arguments through the mandated factory method per CODEWIKI-007. This assumes Config.from_args accepts the same keyword arguments as the direct constructor (repo_path, output_dir, dependency_graph_dir, docs_dir, max_depth, cluster_model, cluster_api_key, cluster_base_url, main_model, main_api_key, main_base_url, fallback_model, fallback_api_key, fallback_base_url); since codewiki/src/config.py was not provided, the exact signature of from_args could not be verified, and a complete fix would require confirming that signature matches or adjusting keyword names accordingly.
🤖 Prompt for AI agents
In test_clustering_local.py around line 62, review and complete this code-review fix: Direct Config(...) instantiation bypasses required from_cli/from_args factory.
What the draft fix changed: In `test_clustering` (test_clustering_local.py), replaced the direct `Config(...)` keyword-argument instantiation with `Config.from_args(...)`, passing the same keyword arguments through the mandated factory method per CODEWIKI-007. This assumes `Config.from_args` accepts the same keyword arguments as the direct constructor (repo_path, output_dir, dependency_graph_dir, docs_dir, max_depth, cluster_model, cluster_api_key, cluster_base_url, main_model, main_api_key, main_base_url, fallback_model, fallback_api_key, fallback_base_url); since `codewiki/src/config.py` was not provided, the exact signature of `from_args` could not be verified, and a complete fix would require confirming that signature matches or adjusting keyword names accordingly.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟡 65 medium — react 👍/👎 to teach the reviewer
| from codewiki.src.config import Config | ||
|
|
||
| # Create minimal config | ||
| config = Config( |
There was a problem hiding this comment.
🦩 🔴 Config instantiated directly with positional/keyword args, bypassing from_args/from_cli factories
In test_subdirectory_fix.py, replaced the direct Config(...) positional/keyword instantiation with Config.from_args(...), keeping the same keyword arguments, so construction goes through the mandated factory. This assumes Config.from_args exists and accepts these same keyword argument names (visible in the original call but the factory's actual signature was not shown to me); if from_args uses a different parameter set or does additional required env resolution incompatible with passing explicit keys, the call will need adjustment — a complete fix would require inspecting codewiki/src/config.py to confirm from_args's exact signature.
🤖 Prompt for AI agents
In test_subdirectory_fix.py around line 16, review and complete this code-review fix: Config instantiated directly with positional/keyword args, bypassing from_args/from_cli factories.
What the draft fix changed: In `test_subdirectory_fix.py`, replaced the direct `Config(...)` positional/keyword instantiation with `Config.from_args(...)`, keeping the same keyword arguments, so construction goes through the mandated factory. This assumes `Config.from_args` exists and accepts these same keyword argument names (visible in the original call but the factory's actual signature was not shown to me); if `from_args` uses a different parameter set or does additional required env resolution incompatible with passing explicit keys, the call will need adjustment — a complete fix would require inspecting `codewiki/src/config.py` to confirm `from_args`'s exact signature.
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
Closes 3 review findings across 3 files.
Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.
test_clustering_debug.py:85test_clustering_local.py:62test_subdirectory_fix.py:16What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.
Run: https://product-hub.flamingo.so/admin/code-review
Run id:
4b0306d9-ca7f-413c-857e-fc323d1e9f21Merging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.
ClickUp task: CU-86akhf8u6 CodeWiki review findings sweep (9 PRs)