From 8a1c7c7996063ee0519906fffd8eaacbb1c98f56 Mon Sep 17 00:00:00 2001 From: Josh Hudson <313875020+hudsonwa@users.noreply.github.com> Date: Tue, 11 Aug 2026 16:43:24 +0800 Subject: [PATCH] fix(tree_renderer): embed PageIndex page images in the summary (#166) Long-doc (PageIndex) images are extracted to wiki/sources/images// and referenced in the per-page JSON, but render_summary_md never read them, so they were invisible in the rendered summary a human actually opens. Pass the per-page list through from _write_long_doc_artifacts, build a page -> image-path map, and embed each node's page-range figures inline with paths relative to the summary's own directory (../sources/images/...). Already-emitted paths are tracked so a figure spanning several sibling nodes is shown only once. --- openkb/indexer.py | 5 +- openkb/tree_renderer.py | 95 +++++++++++++++++++++++++++++++++---- tests/test_tree_renderer.py | 63 ++++++++++++++++++++++++ 3 files changed, 152 insertions(+), 11 deletions(-) diff --git a/openkb/indexer.py b/openkb/indexer.py index 6a4ae1ee..07e76c57 100644 --- a/openkb/indexer.py +++ b/openkb/indexer.py @@ -148,7 +148,10 @@ def _write_long_doc_artifacts( summaries_dir.mkdir(parents=True, exist_ok=True) summary_path = summaries_dir / f"{doc_name}.md" summary_path.write_text( - render_summary_md(tree, doc_name, doc_id, description=description), encoding="utf-8" + render_summary_md( + tree, doc_name, doc_id, description=description, pages=pages + ), + encoding="utf-8", ) return summary_path diff --git a/openkb/tree_renderer.py b/openkb/tree_renderer.py index 2424ba10..f22c7a4a 100644 --- a/openkb/tree_renderer.py +++ b/openkb/tree_renderer.py @@ -15,8 +15,62 @@ def _yaml_frontmatter(source_name: str, doc_id: str, description: str = "") -> s return "---\n" + "\n".join(lines) + "\n---\n" -def _render_nodes_summary(nodes: list[dict], depth: int) -> str: - """Recursively render nodes for the *summary* view (summaries only).""" +def _image_per_page(pages: list[dict] | None) -> dict[int, list[str]]: + """Map 1-based page number -> list of wiki-root-relative image paths. + + ``pages`` is the per-page list written to ``wiki/sources/.json`` (each + item has a 1-based ``page`` and an ``images`` list of ``{"path": ...}`` + dicts whose paths are wiki-root-relative like + ``sources/images//p1_img1.png``). Returns a dict keyed by page number + for O(1) lookup while rendering nodes. + """ + if not pages: + return {} + per_page: dict[int, list[str]] = {} + for item in pages: + page = item.get("page") + if not isinstance(page, int) or page < 1: + continue + paths = [ + img["path"] + for img in item.get("images", []) + if isinstance(img, dict) and isinstance(img.get("path"), str) + ] + if paths: + per_page.setdefault(page, []).extend(paths) + return per_page + + +def _summary_relative_path(wiki_root_path: str) -> str: + """Rewrite a wiki-root-relative image path for a page under ``wiki/summaries/``. + + Image paths in the per-page JSON are wiki-root-relative + (``sources/images//file.png``). The summary lives one directory deeper + (``wiki/summaries/.md``), so the path that resolves for Obsidian / + GitHub is ``../`` + the wiki-root-relative path. + """ + return f"../{wiki_root_path}" if wiki_root_path else "" + + +def _render_nodes_summary( + nodes: list[dict], + depth: int, + per_page_images: dict[int, list[str]] | None = None, + emitted: set[str] | None = None, +) -> str: + """Recursively render nodes for the *summary* view (summaries only). + + When ``per_page_images`` is provided, each node's page range embeds the + images extracted from those pages (as ``![...](../sources/images/...)`` + links), skirting the PageIndex private-cache refs that are stripped from + node text. ``emitted`` tracks already-rendered paths so a figure spanning + pages covered by several sibling nodes is only shown once. + """ + if per_page_images is None: + per_page_images = {} + if emitted is None: + emitted = set() + lines: list[str] = [] heading_prefix = "#" * min(depth, 6) for node in nodes: @@ -27,22 +81,43 @@ def _render_nodes_summary(nodes: list[dict], depth: int) -> str: children = node.get("nodes", []) lines.append(f"{heading_prefix} {title} (pages {start}–{end})\n") + + # Embed figures for the node's page range. Node indices are 0-based + # page indices; the per-page image map is keyed by 1-based page number. + node_images: list[str] = [] + if isinstance(start, int) and isinstance(end, int): + lo, hi = start + 1, end + 1 + for page_num in range(lo, hi + 1): + for path in per_page_images.get(page_num, []): + if path not in emitted: + emitted.add(path) + node_images.append(_summary_relative_path(path)) + for img_path in node_images: + lines.append(f"![image]({img_path})\n") + if summary: lines.append(f"Summary: {summary}\n") if children: - lines.append(_render_nodes_summary(children, depth + 1)) + lines.append(_render_nodes_summary(children, depth + 1, per_page_images, emitted)) return "\n".join(lines) -def render_summary_md(tree: dict, source_name: str, doc_id: str, description: str = "") -> str: +def render_summary_md( + tree: dict, + source_name: str, + doc_id: str, + description: str = "", + pages: list[dict] | None = None, +) -> str: """Render the summary Markdown page for a PageIndex tree. - Renders each node as a heading with page range and its summary text. - Includes a YAML frontmatter block with ``type: "Summary"`` and an - optional ``description`` field. + Renders each node as a heading with page range and its summary text, and + embeds the page images (when ``pages`` is supplied). Includes a YAML + frontmatter block with ``type: "Summary"`` and an optional ``description`` + field. """ - frontmatter = _yaml_frontmatter(source_name, doc_id, description) + frontmatter_block = _yaml_frontmatter(source_name, doc_id, description) structure = tree.get("structure", []) - body = _render_nodes_summary(structure, depth=1) - return frontmatter + "\n" + body + body = _render_nodes_summary(structure, depth=1, per_page_images=_image_per_page(pages)) + return frontmatter_block + "\n" + body diff --git a/tests/test_tree_renderer.py b/tests/test_tree_renderer.py index 3786cfe4..87cb018a 100644 --- a/tests/test_tree_renderer.py +++ b/tests/test_tree_renderer.py @@ -62,3 +62,66 @@ def test_summary_full_text_quoted_yaml_safe(): fm = yaml.safe_load(md.split("---")[1]) assert fm["full_text"] == "sources/weird: name.json" assert fm["type"] == "Summary" + + +# --------------------------------------------------------------------------- +# render_summary_md with pages (issue #166: long-doc images never surface) +# --------------------------------------------------------------------------- + + +def test_images_from_page_range_are_embedded(): + # Node covers pages 1-2 (1-based); the per-page JSON lists an image on + # each. Both must appear in the summary with paths relative to + # wiki/summaries/ (../sources/images/...). + tree = {"structure": [{"title": "Intro", "start_index": 0, "end_index": 1, "summary": "s"}]} + pages = [ + { + "page": 1, + "content": "a", + "images": [{"path": "sources/images/doc/p1_img1.png"}], + }, + { + "page": 2, + "content": "b", + "images": [{"path": "sources/images/doc/p2_img1.png"}], + }, + ] + md = render_summary_md(tree, "doc", "doc-1", pages=pages) + assert "![image](../sources/images/doc/p1_img1.png)" in md + assert "![image](../sources/images/doc/p2_img1.png)" in md + + +def test_no_images_rendered_without_pages(): + # Regression: without the pages argument the summary is unchanged. + tree = {"structure": [{"title": "Intro", "start_index": 0, "end_index": 1, "summary": "s"}]} + md = render_summary_md(tree, "doc", "doc-1") + assert "![image]" not in md + + +def test_figure_spanning_sibling_nodes_is_not_duplicated(): + # A parent and child both cover page 1 (with an image): the figure must be + # embedded once (in the first node that reaches it), not repeated. + tree = { + "structure": [ + { + "title": "Parent", + "start_index": 0, + "end_index": 2, + "summary": "p", + "nodes": [ + {"title": "Child", "start_index": 0, "end_index": 1, "summary": "c"}, + ], + } + ] + } + pages = [{"page": 1, "content": "a", "images": [{"path": "sources/images/doc/p1_img1.png"}]}] + md = render_summary_md(tree, "doc", "doc-1", pages=pages) + assert md.count("![image](../sources/images/doc/p1_img1.png)") == 1 + + +def test_image_outside_node_range_is_not_embedded(): + tree = {"structure": [{"title": "Intro", "start_index": 0, "end_index": 0, "summary": "s"}]} + pages = [{"page": 5, "content": "a", "images": [{"path": "sources/images/doc/p5.png"}]}] + md = render_summary_md(tree, "doc", "doc-1", pages=pages) + assert "![image]" not in md +