From e27accc7c271c9c420df8f304d8fb33f245aba3a Mon Sep 17 00:00:00 2001 From: kiboook Date: Mon, 31 Aug 2026 17:33:47 +0900 Subject: [PATCH 1/2] fix: regenerate invalidated modules in whole-repository-mode --update runs first_module_tree.json is only ever written by the LLM clustering path, so it stays {} for any repo small enough to skip clustering ("whole-repository documentation mode"). generate_module_documentation() computed the processing order exclusively from that file, so once --update/--compare-to invalidated (deleted) an affected sub-module's .md via module_tree.json, the regeneration loop had nothing to iterate and the run always ended in IncompleteGenerationError. Fall back to module_tree for the processing order only when first_module_tree is empty. The normal clustered-repo path is untouched, since first_module_tree is non-empty there and the fallback never triggers. Fixes #99 --- .gitignore | 1 + codewiki/src/be/documentation_generator.py | 27 ++++++- tests/test_processing_order_fallback.py | 85 ++++++++++++++++++++++ 3 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 tests/test_processing_order_fallback.py diff --git a/.gitignore b/.gitignore index 8fd22017..7a0e3d2b 100644 --- a/.gitignore +++ b/.gitignore @@ -42,6 +42,7 @@ tests/* !tests/test_gitignore_filtering.py !tests/test_module_tree_validation.py !tests/test_ruby_analyzer.py +!tests/test_processing_order_fallback.py # Jupyter *.ipynb diff --git a/codewiki/src/be/documentation_generator.py b/codewiki/src/be/documentation_generator.py index 74ce15b7..7084d61d 100644 --- a/codewiki/src/be/documentation_generator.py +++ b/codewiki/src/be/documentation_generator.py @@ -116,6 +116,31 @@ def collect_modules(tree: dict[str, Any], path: list[str]): collect_modules(module_tree, parent_path) return processing_order + def resolve_processing_order( + self, + first_module_tree: dict[str, Any], + module_tree: dict[str, Any], + ) -> list[tuple[list[str], str]]: + """Get the processing order, falling back to ``module_tree`` when + ``first_module_tree`` is empty. + + Whole-repository-mode runs (module clustering skipped because the + repo fits in one context window) never populate + ``first_module_tree.json`` — only the LLM clustering path does. + A prior whole-repo agent run can still insert real sub-modules into + ``module_tree.json`` via ``generate_sub_module_documentation_tool``, + though. Without this fallback, ``--update``/``--compare-to`` + correctly invalidates (deletes) an affected sub-module's ``.md`` + using ``module_tree.json``, but the processing loop below never + re-visits it because its order comes from the still-empty + ``first_module_tree`` — the run then always ends in + ``IncompleteGenerationError``. + """ + processing_order = self.get_processing_order(first_module_tree) + if not processing_order and module_tree: + processing_order = self.get_processing_order(module_tree) + return processing_order + def is_leaf_module(self, module_info: dict[str, Any]) -> bool: """Check if a module is a leaf module (has no children or empty children).""" children = module_info.get("children", {}) @@ -208,7 +233,7 @@ async def generate_module_documentation( first_module_tree = file_manager.load_json(first_module_tree_path) # Get processing order (leaf modules first) - processing_order = self.get_processing_order(first_module_tree) + processing_order = self.resolve_processing_order(first_module_tree, module_tree) # Process modules in dependency order final_module_tree = module_tree diff --git a/tests/test_processing_order_fallback.py b/tests/test_processing_order_fallback.py new file mode 100644 index 00000000..e45984d5 --- /dev/null +++ b/tests/test_processing_order_fallback.py @@ -0,0 +1,85 @@ +"""Tests for DocumentationGenerator.resolve_processing_order. + +Covers the whole-repository-mode incremental-update bug: first_module_tree.json +is only ever written by the LLM clustering path and stays {} for repos small +enough to skip clustering, even after a prior whole-repo agent run inserts +real sub-modules into module_tree.json. Without falling back to module_tree, +--update/--compare-to invalidates (deletes) an affected sub-module's .md but +the processing loop never re-visits it, so the run always ends in +IncompleteGenerationError. +""" + +from __future__ import annotations + +from codewiki.src.be.documentation_generator import DocumentationGenerator + + +def _generator() -> DocumentationGenerator: + # get_processing_order/resolve_processing_order don't touch any instance + # state (config/backend/graph_builder), so constructing without __init__ + # keeps this a pure unit test with no Config/LLM backend wiring needed. + return object.__new__(DocumentationGenerator) + + +def test_falls_back_to_module_tree_when_first_module_tree_empty(): + """Whole-repository-mode case: first_module_tree.json is {}, but + module_tree.json carries sub-modules a prior whole-repo agent run + inserted. The invalidated leaf must still show up in processing order.""" + gen = _generator() + module_tree = { + "module_a": {"components": ["src/a.py::A"], "children": {}}, + "module_b": {"components": ["src/b.py::B"], "children": {}}, + } + + order = gen.resolve_processing_order({}, module_tree) + + assert [name for _, name in order] == ["module_a", "module_b"] + + +def test_uses_first_module_tree_when_non_empty(): + """Normal clustered-repo case is unaffected: first_module_tree.json is + non-empty, so its order is used as before and module_tree is ignored.""" + gen = _generator() + first_module_tree = {"module_a": {"components": ["src/a.py::A"], "children": {}}} + # module_tree has since gained an extra nested module the agent + # inserted while processing — resolve_processing_order must not pick + # that up, since first_module_tree already produced a non-empty order. + module_tree = { + "module_a": { + "components": ["src/a.py::A"], + "children": {"sub": {"components": ["src/a.py::A.helper"], "children": {}}}, + }, + } + + order = gen.resolve_processing_order(first_module_tree, module_tree) + + assert [name for _, name in order] == ["module_a"] + + +def test_both_trees_empty_returns_empty_order(): + """A genuinely fresh whole-repo-mode run (nothing inserted into + module_tree.json yet) must not crash — it just returns no processing + order, and generate_module_documentation's whole-repo branch handles it.""" + gen = _generator() + + order = gen.resolve_processing_order({}, {}) + + assert order == [] + + +def test_nested_children_preserved_in_fallback_order(): + """The fallback still walks nested children leaf-first, matching + get_processing_order's normal topological (leaf-first) contract.""" + gen = _generator() + module_tree = { + "parent": { + "components": [], + "children": { + "child": {"components": ["src/c.py::C"], "children": {}}, + }, + }, + } + + order = gen.resolve_processing_order({}, module_tree) + + assert [name for _, name in order] == ["child", "parent"] From fd060a333baa64548f42aa137525352a166d81d3 Mon Sep 17 00:00:00 2001 From: anhnh2002 Date: Fri, 4 Sep 2026 14:14:49 +0700 Subject: [PATCH 2/2] fix: derive processing order from module_tree so --update revisits agent-inserted sub-modules The fallback added for #99 only kicked in when first_module_tree.json produced an empty order, so it fixed whole-repository mode but left the same failure in clustered mode: first_module_tree.json never learns about the sub-modules agents insert into module_tree.json while documenting a complex module. Invalidating one of those with --update/--compare-to deleted its .md (plus the parent and overview), the loop only revisited the parent, and the run ended in IncompleteDocumentationError. Order from module_tree.json unconditionally instead. It is always a superset of the first tree, and modules whose .md already exists short-circuit in run_module_agent / generate_parent_module_docs, so plain re-runs still cost no LLM calls. This makes resolve_processing_order unnecessary, so it is removed. Tests: replace test_processing_order_fallback.py with test_processing_order_update.py, which adds fake-backend integration tests for the clustered nested case, the whole-repo case from #99, and a no-op re-run. get_processing_order / is_leaf_module become staticmethods since they use no instance state. --- .gitignore | 2 +- codewiki/src/be/documentation_generator.py | 49 +++--- tests/test_processing_order_fallback.py | 85 ---------- tests/test_processing_order_update.py | 176 +++++++++++++++++++++ 4 files changed, 195 insertions(+), 117 deletions(-) delete mode 100644 tests/test_processing_order_fallback.py create mode 100644 tests/test_processing_order_update.py diff --git a/.gitignore b/.gitignore index 7a0e3d2b..d607a89f 100644 --- a/.gitignore +++ b/.gitignore @@ -42,7 +42,7 @@ tests/* !tests/test_gitignore_filtering.py !tests/test_module_tree_validation.py !tests/test_ruby_analyzer.py -!tests/test_processing_order_fallback.py +!tests/test_processing_order_update.py # Jupyter *.ipynb diff --git a/codewiki/src/be/documentation_generator.py b/codewiki/src/be/documentation_generator.py index 7084d61d..b93f3516 100644 --- a/codewiki/src/be/documentation_generator.py +++ b/codewiki/src/be/documentation_generator.py @@ -89,8 +89,9 @@ def create_documentation_metadata( metadata_path = os.path.join(working_dir, "metadata.json") file_manager.save_json(metadata, metadata_path) + @staticmethod def get_processing_order( - self, module_tree: dict[str, Any], parent_path: list[str] | None = None + module_tree: dict[str, Any], parent_path: list[str] | None = None ) -> list[tuple[list[str], str]]: """Get the processing order using topological sort (leaf modules first).""" parent_path = parent_path or [] @@ -116,32 +117,8 @@ def collect_modules(tree: dict[str, Any], path: list[str]): collect_modules(module_tree, parent_path) return processing_order - def resolve_processing_order( - self, - first_module_tree: dict[str, Any], - module_tree: dict[str, Any], - ) -> list[tuple[list[str], str]]: - """Get the processing order, falling back to ``module_tree`` when - ``first_module_tree`` is empty. - - Whole-repository-mode runs (module clustering skipped because the - repo fits in one context window) never populate - ``first_module_tree.json`` — only the LLM clustering path does. - A prior whole-repo agent run can still insert real sub-modules into - ``module_tree.json`` via ``generate_sub_module_documentation_tool``, - though. Without this fallback, ``--update``/``--compare-to`` - correctly invalidates (deletes) an affected sub-module's ``.md`` - using ``module_tree.json``, but the processing loop below never - re-visits it because its order comes from the still-empty - ``first_module_tree`` — the run then always ends in - ``IncompleteGenerationError``. - """ - processing_order = self.get_processing_order(first_module_tree) - if not processing_order and module_tree: - processing_order = self.get_processing_order(module_tree) - return processing_order - - def is_leaf_module(self, module_info: dict[str, Any]) -> bool: + @staticmethod + def is_leaf_module(module_info: dict[str, Any]) -> bool: """Check if a module is a leaf module (has no children or empty children).""" children = module_info.get("children", {}) return not children or (isinstance(children, dict) and len(children) == 0) @@ -228,12 +205,22 @@ async def generate_module_documentation( file_manager.ensure_directory(working_dir) module_tree_path = os.path.join(working_dir, MODULE_TREE_FILENAME) - first_module_tree_path = os.path.join(working_dir, FIRST_MODULE_TREE_FILENAME) module_tree = file_manager.load_json(module_tree_path) - first_module_tree = file_manager.load_json(first_module_tree_path) - # Get processing order (leaf modules first) - processing_order = self.resolve_processing_order(first_module_tree, module_tree) + # Get processing order (leaf modules first) from module_tree.json, not + # first_module_tree.json. The first tree only holds the initial + # clustering result (and stays {} in whole-repository mode), while + # module_tree.json also carries the sub-modules agents inserted via + # generate_sub_module_documentation_tool. --update/--compare-to + # invalidates docs against module_tree.json, so ordering from the + # first tree would skip any invalidated agent-inserted module and the + # run would end in IncompleteDocumentationError. Iterating the full + # tree is safe on resume: every module whose .md already exists + # short-circuits in run_module_agent/generate_parent_module_docs, so + # unaffected modules cost no LLM calls. In whole-repository mode a + # regenerated overview.md is rebuilt from the module docs via + # generate_parent_module_docs([]) rather than by the whole-repo agent. + processing_order = self.get_processing_order(module_tree) # Process modules in dependency order final_module_tree = module_tree diff --git a/tests/test_processing_order_fallback.py b/tests/test_processing_order_fallback.py deleted file mode 100644 index e45984d5..00000000 --- a/tests/test_processing_order_fallback.py +++ /dev/null @@ -1,85 +0,0 @@ -"""Tests for DocumentationGenerator.resolve_processing_order. - -Covers the whole-repository-mode incremental-update bug: first_module_tree.json -is only ever written by the LLM clustering path and stays {} for repos small -enough to skip clustering, even after a prior whole-repo agent run inserts -real sub-modules into module_tree.json. Without falling back to module_tree, ---update/--compare-to invalidates (deletes) an affected sub-module's .md but -the processing loop never re-visits it, so the run always ends in -IncompleteGenerationError. -""" - -from __future__ import annotations - -from codewiki.src.be.documentation_generator import DocumentationGenerator - - -def _generator() -> DocumentationGenerator: - # get_processing_order/resolve_processing_order don't touch any instance - # state (config/backend/graph_builder), so constructing without __init__ - # keeps this a pure unit test with no Config/LLM backend wiring needed. - return object.__new__(DocumentationGenerator) - - -def test_falls_back_to_module_tree_when_first_module_tree_empty(): - """Whole-repository-mode case: first_module_tree.json is {}, but - module_tree.json carries sub-modules a prior whole-repo agent run - inserted. The invalidated leaf must still show up in processing order.""" - gen = _generator() - module_tree = { - "module_a": {"components": ["src/a.py::A"], "children": {}}, - "module_b": {"components": ["src/b.py::B"], "children": {}}, - } - - order = gen.resolve_processing_order({}, module_tree) - - assert [name for _, name in order] == ["module_a", "module_b"] - - -def test_uses_first_module_tree_when_non_empty(): - """Normal clustered-repo case is unaffected: first_module_tree.json is - non-empty, so its order is used as before and module_tree is ignored.""" - gen = _generator() - first_module_tree = {"module_a": {"components": ["src/a.py::A"], "children": {}}} - # module_tree has since gained an extra nested module the agent - # inserted while processing — resolve_processing_order must not pick - # that up, since first_module_tree already produced a non-empty order. - module_tree = { - "module_a": { - "components": ["src/a.py::A"], - "children": {"sub": {"components": ["src/a.py::A.helper"], "children": {}}}, - }, - } - - order = gen.resolve_processing_order(first_module_tree, module_tree) - - assert [name for _, name in order] == ["module_a"] - - -def test_both_trees_empty_returns_empty_order(): - """A genuinely fresh whole-repo-mode run (nothing inserted into - module_tree.json yet) must not crash — it just returns no processing - order, and generate_module_documentation's whole-repo branch handles it.""" - gen = _generator() - - order = gen.resolve_processing_order({}, {}) - - assert order == [] - - -def test_nested_children_preserved_in_fallback_order(): - """The fallback still walks nested children leaf-first, matching - get_processing_order's normal topological (leaf-first) contract.""" - gen = _generator() - module_tree = { - "parent": { - "components": [], - "children": { - "child": {"components": ["src/c.py::C"], "children": {}}, - }, - }, - } - - order = gen.resolve_processing_order({}, module_tree) - - assert [name for _, name in order] == ["child", "parent"] diff --git a/tests/test_processing_order_update.py b/tests/test_processing_order_update.py new file mode 100644 index 00000000..8a3a08ce --- /dev/null +++ b/tests/test_processing_order_update.py @@ -0,0 +1,176 @@ +"""Tests for the module processing order used by generate_module_documentation. + +Regression coverage for incremental updates (--update / --compare-to): +_invalidate_affected_modules deletes affected .md files based on +module_tree.json, so the regeneration loop must derive its processing order +from that same tree. first_module_tree.json only holds the initial clustering +result: it stays {} in whole-repository mode (issue #99) and never learns about +sub-modules that agents insert while documenting a complex module. Ordering +from it left invalidated modules unvisited and the run ended in +IncompleteDocumentationError. +""" + +from __future__ import annotations + +import asyncio +import json +from pathlib import Path +from types import SimpleNamespace + +from codewiki.src.be.documentation_generator import DocumentationGenerator + +# --------------------------------------------------------------------------- # +# get_processing_order unit tests +# --------------------------------------------------------------------------- # + + +def test_flat_tree_preserves_insertion_order(): + tree = { + "module_a": {"components": ["src/a.py::A"], "children": {}}, + "module_b": {"components": ["src/b.py::B"], "children": {}}, + } + + order = DocumentationGenerator.get_processing_order(tree) + + assert order == [(["module_a"], "module_a"), (["module_b"], "module_b")] + + +def test_nested_tree_is_leaf_first(): + tree = { + "parent": { + "components": [], + "children": { + "child": {"components": ["src/c.py::C"], "children": {}}, + }, + }, + } + + order = DocumentationGenerator.get_processing_order(tree) + + assert order == [(["parent", "child"], "child"), (["parent"], "parent")] + + +def test_empty_tree_returns_empty_order(): + assert DocumentationGenerator.get_processing_order({}) == [] + + +# --------------------------------------------------------------------------- # +# generate_module_documentation integration tests (no LLM, fake backend) +# --------------------------------------------------------------------------- # + + +class FakeBackend: + """Stand-in for LLMBackend that writes a doc file per module agent call.""" + + def __init__(self, working_dir: Path): + self.working_dir = working_dir + self.module_agent_calls: list[str] = [] + self.complete_calls = 0 + + async def run_module_agent( + self, module_name, components, core_component_ids, module_path, working_dir + ): + module_tree = json.loads(Path(working_dir, "module_tree.json").read_text()) + # Mirror the real backends: a module whose doc already exists is skipped. + if Path(working_dir, f"{module_name}.md").exists(): + return module_tree + self.module_agent_calls.append(module_name) + Path(working_dir, f"{module_name}.md").write_text(f"# {module_name}\n") + return module_tree + + def complete(self, prompt, model=None): + self.complete_calls += 1 + return "overview" + + +def _generator(docs_dir: Path) -> tuple[DocumentationGenerator, FakeBackend]: + # Bypass __init__: it wires a real LLM backend and dependency analyzer. + gen = object.__new__(DocumentationGenerator) + gen.config = SimpleNamespace(docs_dir=str(docs_dir), repo_path=str(docs_dir)) + gen.backend = FakeBackend(docs_dir) + return gen, gen.backend + + +def _write_json(path: Path, data) -> None: + path.write_text(json.dumps(data)) + + +def test_update_regenerates_agent_inserted_nested_module(tmp_path): + """Clustered-repo case: first_module_tree.json knows only module_a, but the + module_a agent later delegated sub1/sub2 into module_tree.json. A change + under sub1 invalidates sub1.md, module_a.md and overview.md; all three must + come back, and untouched sub2 must not be regenerated.""" + _write_json( + tmp_path / "first_module_tree.json", + {"module_a": {"components": ["src/a.py::A"], "children": {}}}, + ) + _write_json( + tmp_path / "module_tree.json", + { + "module_a": { + "components": ["src/a.py::A"], + "children": { + "sub1": {"components": ["src/a.py::A.one"], "children": {}}, + "sub2": {"components": ["src/a.py::A.two"], "children": {}}, + }, + } + }, + ) + (tmp_path / "sub2.md").write_text("# sub2\n") + gen, backend = _generator(tmp_path) + + asyncio.run(gen.generate_module_documentation(components={}, leaf_nodes=[])) + + assert (tmp_path / "sub1.md").exists() + assert (tmp_path / "module_a.md").exists() + assert (tmp_path / "overview.md").exists() + assert backend.module_agent_calls == ["sub1"] + # module_a (parent) + repository overview + assert backend.complete_calls == 2 + + +def test_update_regenerates_module_in_whole_repo_mode(tmp_path): + """Issue #99: whole-repository mode leaves first_module_tree.json as {}, + while the whole-repo agent inserted module_a/module_b into + module_tree.json. Invalidating module_a must regenerate it and the + overview, and skip module_b whose doc is still present.""" + _write_json(tmp_path / "first_module_tree.json", {}) + _write_json( + tmp_path / "module_tree.json", + { + "module_a": {"components": ["src/a.py::A"], "children": {}}, + "module_b": {"components": ["src/b.py::B"], "children": {}}, + }, + ) + (tmp_path / "module_b.md").write_text("# module_b\n") + gen, backend = _generator(tmp_path) + + asyncio.run(gen.generate_module_documentation(components={}, leaf_nodes=[])) + + assert (tmp_path / "module_a.md").exists() + assert (tmp_path / "overview.md").exists() + assert backend.module_agent_calls == ["module_a"] + assert backend.complete_calls == 1 + + +def test_complete_docs_are_not_regenerated(tmp_path): + """Plain re-run with everything already on disk must make no LLM calls, + even though the full module_tree.json is now walked.""" + _write_json(tmp_path / "first_module_tree.json", {}) + _write_json( + tmp_path / "module_tree.json", + { + "module_a": { + "components": ["src/a.py::A"], + "children": {"sub1": {"components": ["src/a.py::A.one"], "children": {}}}, + } + }, + ) + for name in ("sub1", "module_a", "overview"): + (tmp_path / f"{name}.md").write_text(f"# {name}\n") + gen, backend = _generator(tmp_path) + + asyncio.run(gen.generate_module_documentation(components={}, leaf_nodes=[])) + + assert backend.module_agent_calls == [] + assert backend.complete_calls == 0