diff --git a/.gitignore b/.gitignore index 8f5d45bf..8c117953 100644 --- a/.gitignore +++ b/.gitignore @@ -40,6 +40,7 @@ htmlcov/ .hypothesis/ tests/* !tests/test_gitignore_filtering.py +!tests/test_module_tree_validation.py # Jupyter *.ipynb diff --git a/IDE_DRIVEN_GUIDE.md b/IDE_DRIVEN_GUIDE.md index 308f1472..455a79c0 100644 --- a/IDE_DRIVEN_GUIDE.md +++ b/IDE_DRIVEN_GUIDE.md @@ -71,7 +71,7 @@ The server exposes **8 fine-grained tools** (zero LLM config) plus **2 legacy to | `read_code_components` | Write component source code to workspace `.src` files | Each component → `sources/{sanitized_id}.src`, returns file paths | No | | `write_doc_file` | Create .md documents with auto Mermaid validation | Writes file directly to output dir | No | | `edit_doc_file` | Edit documents: `str_replace` / `insert` / `undo` | Modifies file in place, keeps edit history (capped at 20/file) | No | -| `save_module_tree` | Persist IDE agent's module clustering | Writes `module_tree.json` + `first_module_tree.json` + `processing_order.json` | No | +| `save_module_tree` | Persist IDE agent's module clustering | Writes `module_tree.json` + `first_module_tree.json` + `processing_order.json` + `module_tree_validation.json`; returns `warning` for orphaned IDs and `note` for unassigned leaf candidates (IDs capped at 20) | No | | `get_processing_order` | Compute leaf-first processing order | Writes `processing_order.json` to workspace, returns path | No | | `get_prompt` | Retrieve prompt templates for each pipeline stage | Returns inline (small payload) | No | | `close_session` | Write `metadata.json`, clean up workspace files, free memory | Cleans workspace dir + prunes empty parent dirs | No | @@ -311,6 +311,7 @@ Each `analyze_repo` call creates a session workspace at `{repo_path}/.codewiki/s ├── changes.json # Incremental change info (optional) ├── summary.json # Compact analysis summary ├── processing_order.json # Leaf-first generation order (after save_module_tree) +├── module_tree_validation.json # Full unmatched/leftover ID lists (after save_module_tree) └── sources/ └── {sanitized_id}.src # Individual component source files ``` diff --git a/codewiki/mcp/server.py b/codewiki/mcp/server.py index 5ae390ab..703f06c0 100644 --- a/codewiki/mcp/server.py +++ b/codewiki/mcp/server.py @@ -205,7 +205,12 @@ def _fine_grained_tools() -> list[Tool]: "Save the IDE agent's module clustering result. " "Accepts a JSON module tree and persists it to disk. " "Computes the leaf-first processing order and writes it to a workspace file. " - "Returns the file path for the processing order." + "Returns the file path for the processing order. " + "Validates each component id against the analysis index and reports " + "orphaned (unmatched) ids as a 'warning' and unassigned leaf-node " + "candidates as an informational 'note' in the response; ID lists in " + "the response are capped at 20 and full lists go to " + "module_tree_validation.json." ), inputSchema={ "type": "object", diff --git a/codewiki/mcp/tools/module_tree.py b/codewiki/mcp/tools/module_tree.py index 6d439f45..29d487a7 100644 --- a/codewiki/mcp/tools/module_tree.py +++ b/codewiki/mcp/tools/module_tree.py @@ -18,6 +18,17 @@ logger = logging.getLogger(__name__) +# Cap on ID lists embedded in the MCP response. Full lists live in the +# workspace module_tree_validation.json file so stdio stays small. +_MAX_IDS_IN_RESPONSE = 20 + + +def _cap(ids: List[str]) -> Tuple[List[str], bool]: + """Return (ids capped to _MAX_IDS_IN_RESPONSE, was_truncated).""" + if len(ids) <= _MAX_IDS_IN_RESPONSE: + return ids, False + return ids[:_MAX_IDS_IN_RESPONSE], True + def _get_processing_order(module_tree: Dict[str, Any], parent_path: List[str] | None = None) -> List[Dict[str, Any]]: """Compute leaf-first processing order from a module tree. @@ -56,6 +67,46 @@ def _collect(tree: Dict[str, Any], path: List[str]) -> None: return order +def _collect_component_ids(module_tree: Dict[str, Any]) -> set[str]: + """Return the set of all component ids referenced across the module tree. + + Walks every module (and nested ``children``) and collects the entries of + each module's ``components`` list. + """ + ids: set[str] = set() + + def _walk(tree: Dict[str, Any]) -> None: + for module_info in tree.values(): + ids.update(module_info.get("components", []) or []) + children = module_info.get("children", {}) + if isinstance(children, dict): + _walk(children) + + _walk(module_tree) + return ids + + +def _validate_module_tree( + module_tree: Dict[str, Any], + known_ids: set[str], + candidate_ids: set[str], +) -> Tuple[List[str], List[str]]: + """Check the tree's component ids against the analysis index. + + Returns ``(unmatched_ids, leftover_ids)``: + * ``unmatched_ids``: ids referenced by the tree that do not exist in the + index (typos / drift) -- they will be silently omitted from docs. + * ``leftover_ids``: clustering candidate ids (leaf nodes) that are not + assigned to any module -- a coverage gap for the current clustering. + Non-candidate components (excluded / non-essential) are intentionally + not reported as leftover. + """ + assigned = _collect_component_ids(module_tree) + unmatched = sorted(assigned - known_ids) + leftover = sorted(candidate_ids - assigned) + return unmatched, leftover + + def handle_save_module_tree( arguments: Dict[str, Any], store: SessionStore, @@ -68,6 +119,8 @@ def handle_save_module_tree( module_tree = arguments["module_tree"] output_dir = session.output_dir + known_ids = set(session.components.keys()) + candidate_ids = set(session.leaf_nodes) # Save both immutable snapshot and mutable working copy first_path = os.path.join(output_dir, FIRST_MODULE_TREE_FILENAME) @@ -83,6 +136,55 @@ def handle_save_module_tree( # Cache in session session.module_tree = module_tree + # Validate the tree so orphaned / stale ids surface instead of being + # silently dropped from docs. Unmatched ids are checked against the full + # index; leftovers only against the clustering candidate set (leaf nodes), + # since the cluster prompt deliberately excludes non-essential components. + unmatched_ids, leftover_ids = _validate_module_tree( + module_tree, known_ids, candidate_ids + ) + full_validation = { + "unmatched_ids": unmatched_ids, + "unmatched_count": len(unmatched_ids), + "leftover_component_ids": leftover_ids, + "leftover_component_count": len(leftover_ids), + } + if session.workspace is not None: + session.workspace.write_json("module_tree_validation.json", full_validation) + + unmatched_capped, unmatched_truncated = _cap(unmatched_ids) + leftover_capped, leftover_truncated = _cap(leftover_ids) + validation = { + "unmatched_ids": unmatched_capped, + "unmatched_count": len(unmatched_ids), + "unmatched_truncated": unmatched_truncated, + "leftover_component_ids": leftover_capped, + "leftover_component_count": len(leftover_ids), + "leftover_truncated": leftover_truncated, + } + + warning = "" + if unmatched_ids: + warning = ( + f"{len(unmatched_ids)} component id(s) in the module tree do not " + f"exist in the analysis index and will be omitted from docs: " + f"{unmatched_capped}" + ) + if unmatched_truncated: + warning += " ... (see module_tree_validation.json for the full list)" + logger.warning("save_module_tree for session %s: %s", session_id, warning) + + note = "" + if leftover_ids: + note = ( + f"{len(leftover_ids)} clustering candidate component(s) are not " + f"assigned to any module and will receive no documentation: " + f"{leftover_capped}" + ) + if leftover_truncated: + note += " ... (see module_tree_validation.json for the full list)" + logger.info("save_module_tree for session %s: %s", session_id, note) + # Compute processing order and write to workspace file order = _get_processing_order(module_tree) order_file = None @@ -96,6 +198,7 @@ def handle_save_module_tree( "tree_path": working_path, "first_tree_path": first_path, "processing_order_file": order_file, + "validation": validation, "hint": ( "Read the processing_order.json file for the leaf-first generation order. " "Process leaf modules first (is_leaf=true), then parent modules. " @@ -103,6 +206,10 @@ def handle_save_module_tree( "For each parent module: get_prompt('overview_module') + write_doc_file." ), } + if warning: + result["warning"] = warning + if note: + result["note"] = note return json.dumps(result, indent=2, ensure_ascii=False) diff --git a/codewiki/mcp/workspace.py b/codewiki/mcp/workspace.py index 020cadb3..db784f43 100644 --- a/codewiki/mcp/workspace.py +++ b/codewiki/mcp/workspace.py @@ -14,6 +14,7 @@ changes.json summary.json processing_order.json + module_tree_validation.json sources/ {sanitized_component_id}.src """ diff --git a/tests/test_module_tree_validation.py b/tests/test_module_tree_validation.py new file mode 100644 index 00000000..27bb0b57 --- /dev/null +++ b/tests/test_module_tree_validation.py @@ -0,0 +1,253 @@ +"""Tests for save_module_tree validation of component ids. + +Verifies that a module tree referencing unknown/stale component ids is +surfaced in the response (unmatched_ids) and that clustering-candidate +components (leaf nodes) left out of the tree are reported as an +informational coverage gap (leftover_component_ids), without breaking the +save itself. Leftover ids are computed against the leaf-node candidate set, +never the full index, because the cluster prompt intentionally excludes +non-essential components. +""" + +from __future__ import annotations + +import json + +from codewiki.mcp.session import SessionState, SessionStore +from codewiki.mcp.tools.module_tree import handle_save_module_tree +from codewiki.mcp.workspace import SessionWorkspace +from codewiki.src.be.dependency_analyzer.models.core import Node + + +def _make_node(component_id: str) -> Node: + rel_path, _, name = component_id.partition("::") + return Node( + id=component_id, + name=name or component_id, + component_type="class", + file_path=rel_path, + relative_path=rel_path, + ) + + +def _make_session( + store: SessionStore, + tmp_path, + component_ids: list[str], + leaf_nodes: list[str] | None = None, +) -> SessionState: + components = {cid: _make_node(cid) for cid in component_ids} + if leaf_nodes is None: + leaf_nodes = list(component_ids) + session = store.create( + repo_path=str(tmp_path), + output_dir=str(tmp_path), + components=components, + leaf_nodes=leaf_nodes, + ) + session.workspace = SessionWorkspace(tmp_path, session.session_id) + return session + + +def _save(tree: dict, session: SessionState, store: SessionStore) -> dict: + result = handle_save_module_tree( + {"session_id": session.session_id, "module_tree": tree}, + store, + ) + return json.loads(result) + + +def _read_validation_file(session: SessionState) -> dict: + assert session.workspace is not None + validation_path = session.workspace.root / "module_tree_validation.json" + assert validation_path.exists(), "module_tree_validation.json not written" + return json.loads(validation_path.read_text(encoding="utf-8")) + + +def test_valid_tree_no_gaps(tmp_path): + store = SessionStore() + ids = ["src/a.py::A", "src/b.py::B", "src/c.py::C"] + session = _make_session(store, tmp_path, ids) + tree = { + "core": {"components": ["src/a.py::A"]}, + "utils": {"components": ["src/b.py::B", "src/c.py::C"]}, + } + + result = _save(tree, session, store) + + assert result["status"] == "saved" + assert result["module_count"] == 2 + assert result["validation"]["unmatched_ids"] == [] + assert result["validation"]["unmatched_count"] == 0 + assert result["validation"]["unmatched_truncated"] is False + assert result["validation"]["leftover_component_count"] == 0 + assert result["validation"]["leftover_truncated"] is False + assert "warning" not in result + assert "note" not in result + + validation = _read_validation_file(session) + assert validation["unmatched_ids"] == [] + assert validation["leftover_component_ids"] == [] + + +def test_orphaned_id_reported_but_saved(tmp_path): + store = SessionStore() + ids = ["src/a.py::A", "src/b.py::B"] + session = _make_session(store, tmp_path, ids) + tree = { + "core": {"components": ["src/a.py::A", "src/a.py::Typo"]}, + "utils": {"components": ["src/b.py::B"]}, + } + + result = _save(tree, session, store) + + assert result["status"] == "saved" + assert result["validation"]["unmatched_ids"] == ["src/a.py::Typo"] + assert result["validation"]["unmatched_count"] == 1 + assert result["validation"]["leftover_component_count"] == 0 + assert "src/a.py::Typo" in result["warning"] + assert "note" not in result + + validation = _read_validation_file(session) + assert validation["unmatched_ids"] == ["src/a.py::Typo"] + assert validation["leftover_component_ids"] == [] + + +def test_unassigned_leaf_candidates_flagged_as_note(tmp_path): + store = SessionStore() + ids = ["src/a.py::A", "src/b.py::B"] + session = _make_session(store, tmp_path, ids) + tree = { + "core": {"components": ["src/a.py::A"]}, + } + + result = _save(tree, session, store) + + assert result["status"] == "saved" + assert result["validation"]["unmatched_ids"] == [] + assert result["validation"]["leftover_component_count"] == 1 + assert "src/b.py::B" in result["validation"]["leftover_component_ids"] + assert "warning" not in result + assert "src/b.py::B" in result["note"] + + validation = _read_validation_file(session) + assert validation["leftover_component_ids"] == ["src/b.py::B"] + + +def test_nested_children_validated(tmp_path): + store = SessionStore() + ids = ["src/a.py::A", "src/b.py::B"] + session = _make_session(store, tmp_path, ids) + tree = { + "root": { + "components": ["src/a.py::A"], + "children": { + "child": {"components": ["src/b.py::B", "src/missing.py::X"]}, + }, + }, + } + + result = _save(tree, session, store) + + assert result["status"] == "saved" + assert result["validation"]["unmatched_ids"] == ["src/missing.py::X"] + assert result["validation"]["unmatched_count"] == 1 + assert result["validation"]["leftover_component_count"] == 0 + assert "src/missing.py::X" in result["warning"] + + +def test_non_leaf_component_not_flagged_as_leftover(tmp_path): + store = SessionStore() + ids = ["src/a.py::A", "src/b.py::B"] + session = _make_session(store, tmp_path, ids, leaf_nodes=["src/a.py::A"]) + tree = { + "core": {"components": ["src/a.py::A"]}, + } + + result = _save(tree, session, store) + + assert result["status"] == "saved" + assert result["validation"]["unmatched_ids"] == [] + assert result["validation"]["leftover_component_count"] == 0 + assert "warning" not in result + assert "note" not in result + + +def test_leftover_counts_only_leaf_candidates(tmp_path): + store = SessionStore() + ids = ["src/a.py::A", "src/b.py::B", "src/c.py::C"] + session = _make_session( + store, + tmp_path, + ids, + leaf_nodes=["src/a.py::A", "src/b.py::B"], + ) + tree = { + "core": {"components": ["src/a.py::A"]}, + } + + result = _save(tree, session, store) + + assert result["validation"]["leftover_component_count"] == 1 + assert result["validation"]["leftover_component_ids"] == ["src/b.py::B"] + assert "src/b.py::B" in result["note"] + + +def test_multiple_unmatched_and_leftover(tmp_path): + store = SessionStore() + ids = ["src/a.py::A", "src/b.py::B", "src/c.py::C"] + session = _make_session(store, tmp_path, ids) + tree = { + "core": {"components": ["src/a.py::A", "src/typo.py::T1", "src/typo.py::T2"]}, + } + + result = _save(tree, session, store) + + assert result["status"] == "saved" + assert result["validation"]["unmatched_ids"] == [ + "src/typo.py::T1", + "src/typo.py::T2", + ] + assert result["validation"]["unmatched_count"] == 2 + assert result["validation"]["leftover_component_count"] == 2 + assert result["validation"]["leftover_component_ids"] == [ + "src/b.py::B", + "src/c.py::C", + ] + assert "src/typo.py::T1" in result["warning"] + assert "src/b.py::B" in result["note"] + + +def test_id_lists_capped_in_response(tmp_path): + store = SessionStore() + leaf_ids = [f"src/l{i}.py::L{i}" for i in range(25)] + session = _make_session(store, tmp_path, leaf_ids) + typo_ids = [f"src/x{i}.py::X{i}" for i in range(25)] + tree = { + "mod": {"components": typo_ids}, + } + + result = _save(tree, session, store) + + validation = result["validation"] + assert validation["unmatched_count"] == 25 + assert len(validation["unmatched_ids"]) == 20 + assert validation["unmatched_truncated"] is True + assert validation["leftover_component_count"] == 25 + assert len(validation["leftover_component_ids"]) == 20 + assert validation["leftover_truncated"] is True + assert "module_tree_validation.json" in result["warning"] + assert "module_tree_validation.json" in result["note"] + + full = _read_validation_file(session) + assert len(full["unmatched_ids"]) == 25 + assert len(full["leftover_component_ids"]) == 25 + + +def test_missing_session_errors(tmp_path): + store = SessionStore() + result = json.loads(handle_save_module_tree( + {"session_id": "nope", "module_tree": {}}, + store, + )) + assert "error" in result