fix(CODEWIKI-004): CU-86akbhg0r 17 review findings across 11 files - #39
Conversation
🦩 What this fix changed, finding by finding17 finding(s) fixed in this draft. (Inline placement was rejected by GitHub for this PR.) 🟠 1. generate_sub_module_documentations.py lacks a module-level docstring — 🤖 Prompt for AI agentsfix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer 🟠 2. ID-to-FQDN normalization logic duplicated between generate_sub_module_documentations.py and cluster_modules.py — 🤖 Prompt for AI agentsfix confidence: 🟡 60 medium — react 👍/👎 to teach the reviewer 🟠 3. Tool docstring instructs LLM to return FQDN strings, but code path only handles integer IDs — 🤖 Prompt for AI agentsfix confidence: 🔴 55 low — review closely — react 👍/👎 to teach the reviewer 🟠 4. codewiki/src/be/agent_orchestrator.py has no module-level docstring — 🤖 Prompt for AI agentsfix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer 🟠 5. Redundant local logger re-creation shadows module-level logger in AgentOrchestrator.init — 🤖 Prompt for AI agentsfix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer 🟠 6. codewiki/src/be/agent_tools/deps.py has no module docstring at all — 🤖 Prompt for AI agentsfix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer 🟠 7. CodeWikiDeps.module_tree typed as dict[str, any] instead of typing.Any — 🤖 Prompt for AI agentsfix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer 🟠 8. typescript.py analyzer module lacks a module-level docstring — 🤖 Prompt for AI agentsfix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer 🟠 9. _get_parent_context in typescript.py has an implicit fallthrough returning None for 'statement_block' cases — 🤖 Prompt for AI agentsfix confidence: 🟡 85 medium — react 👍/👎 to teach the reviewer 🟠 10. codewiki/src/be/dependency_analyzer/analyzers/c.py missing module docstring — 🤖 Prompt for AI agentsfix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer 🟠 11. C analyzer component IDs constructed with '.' separator, not '::' FQDN format — 🤖 Prompt for AI agentsfix confidence: 🟡 85 medium — react 👍/👎 to teach the reviewer 🟠 12. codewiki/src/config.py lacks a proper module-level docstring — 🤖 Prompt for AI agentsfix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer 🟠 13. dependency_graphs_builder.py lacks a module-level docstring — 🤖 Prompt for AI agentsfix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer 🟠 14. codewiki/src/utils.py has no module-level docstring — 🤖 Prompt for AI agentsfix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer 🟠 15. codewiki/src/be/dependency_analyzer/models/core.py missing module docstring — 🤖 Prompt for AI agentsfix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer 🟠 16. codewiki/src/be/dependency_analyzer/analysis/cloning.py missing module docstring — 🤖 Prompt for AI agentsfix confidence: 🟢 92 high — react 👍/👎 to teach the reviewer 🟠 17. Missing module-level docstring in security.py — 🤖 Prompt for AI agentsfix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer |
|
Blocking: this raises
normalized_specs, total_normalized, total_failed = normalize_component_ids_by_lookup(
sub_module_specs, deps.components, id_to_fqdn
)but the function in def normalize_component_ids_by_lookup(module_tree: Dict, id_to_fqdn: Dict[int, str]) -> Dict:Two args, returns a
Also in scope before this can land:
The docstring additions in this PR are good and uncontroversial; consider splitting them into their own PR so they can land immediately. |
Bring the branch up to date with main (PRs #48, #49, #52, #53, #54, #55). Conflicts: - deps.py, typescript.py: competing module docstrings added by both this branch and #55; resolved in favour of the wording on main. - config.py: this branch's new module docstring kept, layered on top of #49's widened dataclasses import (fields, asdict). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…or change - generate_sub_module_documentations.py: the branch's normalize_component_ids_by_lookup(specs, components, id_to_fqdn) call was a three-argument call against a two-argument function, unpacked as a 3-tuple from a dict return. #47 has since landed the correct dedupe via normalize_component_id_list, so this resolves in favour of main. - analyzers/c.py: reverted the '.' -> '::' component-id change. Changing the separator for C alone is inconsistent with the other seven analyzers, and the FQDN format question belongs with #34/#45 where it can be made coherent end-to-end. The module docstring is kept. What remains is worth having: - deps.py: dict[str, any] -> dict[str, Any]. 'any' is the builtin function, not a type, so the old annotation was meaningless. - agent_orchestrator.__init__: drops a local logger that shadowed the module-level one (same logging.getLogger(__name__) object, so no behaviour change). - typescript.py _get_parent_context: returns 'unknown' instead of falling off the end as None, honouring its '-> str' annotation. Its one caller stores the value and nothing reads it, so this cannot regress. - The tool description now asks for integer IDs, matching what format_potential_core_components actually puts in the prompt. - Module docstrings throughout, plus from_web_job added to the config.py summary. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Closes 17 review findings across 11 files.
Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.
codewiki/src/be/agent_tools/generate_sub_module_documentations.py:1codewiki/src/be/agent_tools/generate_sub_module_documentations.py:44codewiki/src/be/agent_tools/generate_sub_module_documentations.py:185codewiki/src/be/agent_orchestrator.py:1codewiki/src/be/agent_orchestrator.py:58codewiki/src/be/agent_tools/deps.py:1codewiki/src/be/agent_tools/deps.py:12codewiki/src/be/dependency_analyzer/analyzers/typescript.py:1codewiki/src/be/dependency_analyzer/analyzers/typescript.py:178codewiki/src/be/dependency_analyzer/analyzers/c.py:1codewiki/src/be/dependency_analyzer/analyzers/c.py:44codewiki/src/config.py:1codewiki/src/be/dependency_analyzer/dependency_graphs_builder.py:1codewiki/src/utils.py:1codewiki/src/be/dependency_analyzer/models/core.py:1codewiki/src/be/dependency_analyzer/analysis/cloning.py:1codewiki/src/be/dependency_analyzer/utils/security.py:1What 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:
bef4f5a8-e7f3-478b-8731-2becca2de658Merging 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-86akbhg0r Code review fixes: CodeWiki backend and CLI review findings (5 PRs)