From 9a49413c513d7970d6d1c640eebcd73064ab8c56 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 24 Aug 2026 05:09:59 +0000 Subject: [PATCH 1/2] fix(CODEWIKI-006-2): 2 review findings in cluster_modules.py --- codewiki/src/be/cluster_modules.py | 44 +++++++++++------------------- 1 file changed, 16 insertions(+), 28 deletions(-) diff --git a/codewiki/src/be/cluster_modules.py b/codewiki/src/be/cluster_modules.py index e7b1fa78..016c1f92 100644 --- a/codewiki/src/be/cluster_modules.py +++ b/codewiki/src/be/cluster_modules.py @@ -1,3 +1,16 @@ +""" +Module clustering pipeline for CodeWiki. + +This module groups leaf-level code components (classes, functions, files) +into higher-level "modules" using LLM-driven clustering. It builds an +integer ID <-> FQDN mapping for components so the LLM prompt/response can +operate on compact integer IDs instead of full fully-qualified names, +normalizes the LLM's returned component IDs back to FQDNs, and recursively +clusters sub-modules until each unit fits under the configured token budget. + +Also includes a small backward-compatibility layer for functions that were +used by the older short-ID based clustering approach. +""" from typing import List, Dict, Any, Optional from collections import defaultdict import logging @@ -355,38 +368,13 @@ def cluster_modules( logger.error(f"Invalid module tree format - expected dict, got {type(module_tree)}") return {} - # CRITICAL: Validate all component IDs are integers - max_id = len(id_to_fqdn) - 1 - for module_name, module_info in module_tree.items(): - if "components" not in module_info: - continue - - component_ids = module_info["components"] - invalid_ids = [] - - for comp_id in component_ids: - # Check if ID is an integer - if not isinstance(comp_id, int): - invalid_ids.append(f"{comp_id} (type: {type(comp_id).__name__})") - # Check if ID is in valid range - elif comp_id < 0 or comp_id > max_id: - invalid_ids.append(f"{comp_id} (out of range 0-{max_id})") - - if invalid_ids: - logger.error(f"❌ Module '{module_name}' contains invalid component IDs:") - logger.error(f" Invalid IDs: {invalid_ids}") - logger.error(f" Expected: Integers in range 0-{max_id}") - logger.error(f" LLM ignored instructions and returned non-integer IDs!") - return {} - - logger.info(f"✅ LLM response validation passed: All IDs are integers in valid range") - except Exception as e: logger.error(f"Failed to parse LLM response: {e}. Response: {response[:200]}...") logger.error(f"Traceback: {traceback.format_exc()}") return {} - # Normalize component IDs using simple lookup (replaces 200+ lines of fuzzy matching) + # Normalize component IDs using simple lookup (replaces 200+ lines of fuzzy matching + # and the duplicated inline ID validation that previously lived here) module_tree = normalize_component_ids_by_lookup(module_tree, id_to_fqdn) # check if the module tree is valid @@ -517,4 +505,4 @@ def _find_best_path_match(llm_id: str, candidates: List[str]) -> Optional[str]: DeprecationWarning, stacklevel=2 ) - return None \ No newline at end of file + return None From 430768af21621b635373dfbe2337b0e6ae44e6b6 Mon Sep 17 00:00:00 2001 From: Michael Assraf Date: Mon, 7 Sep 2026 23:00:37 -0400 Subject: [PATCH 2/2] fix: keep clustering's abort-on-unresolvable-id contract Removing the inline validation was right - normalize_component_ids_by_lookup already checks the same conditions - but the two are not equivalent. The inline block returned {} and failed clustering visibly; the normalizer logs a warning and drops the id. A module could therefore end up with an empty component list, get documented anyway, and the run would report success with a thin overview.md. That is a worse failure mode for the doc pipeline than an outright failure. Restore the contract by comparing the component-id count either side of normalization and aborting when any id was dropped. Verified to abort on exactly the same inputs as main's inline validation (out-of-range ids, bare class names) and to pass on the same ones (all-valid, no components key). One deliberate relaxation: a string integer such as "1" now resolves, where main's isinstance(comp_id, int) check rejected it. An LLM returning "1" for 1 is answering correctly in the wrong type. Co-Authored-By: Claude Opus 5 --- codewiki/src/be/cluster_modules.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/codewiki/src/be/cluster_modules.py b/codewiki/src/be/cluster_modules.py index d2d23ede..6866d043 100644 --- a/codewiki/src/be/cluster_modules.py +++ b/codewiki/src/be/cluster_modules.py @@ -409,8 +409,25 @@ def cluster_modules( return {} # Normalize component IDs using simple lookup (replaces 200+ lines of fuzzy matching - # and the duplicated inline ID validation that previously lived here) + # and the duplicated inline ID validation that previously lived here). + # + # normalize_component_ids_by_lookup drops ids it cannot resolve and carries on, + # which is the right behaviour for the sub-module path but NOT here: a module + # left holding an empty component list still gets documented, producing a + # plausible-looking but empty page and a run that reports success. The inline + # validation this replaced aborted instead, and clustering keeps that contract + # by comparing the id count either side of normalization. + ids_before = sum(len(m.get('components', [])) for m in module_tree.values()) module_tree = normalize_component_ids_by_lookup(module_tree, id_to_fqdn) + ids_after = sum(len(m.get('components', [])) for m in module_tree.values()) + + if ids_after < ids_before: + logger.error( + f"\u274c Clustering aborted: {ids_before - ids_after} of {ids_before} component " + f"ID(s) could not be resolved to an FQDN (valid range 0-{len(id_to_fqdn) - 1}).\n" + f" \u2514\u2500 The LLM ignored the integer-ID instruction; see the warnings above." + ) + return {} # check if the module tree is valid if len(module_tree) <= 1: