From b95f0544198928997cfbd41b2b705680748d93d4 Mon Sep 17 00:00:00 2001 From: Aldominguez12 <191896285+Aldominguez12@users.noreply.github.com> Date: Sun, 12 Jul 2026 07:23:08 +0200 Subject: [PATCH] fix(images): write note-relative image links in sources pages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Short-doc source pages live at wiki/sources/.md but embedded their images with wiki-root-relative links (sources/images//file.png). Markdown renderers resolve links relative to the containing file — Obsidian, GitHub, VS Code — so every image resolved to the non-existent wiki/sources/sources/images/... and rendered broken. - New md_image_ref() helper emits note-relative images//... links, used by the three .md-visible writers (convert_pdf_with_images, extract_base64_images, copy_relative_images). - Long-doc JSON page metadata keeps wiki-root-relative paths: it is internal, consumed by get_wiki_page_content/read_wiki_image against the wiki root. Now documented explicitly in the docstrings. - read_wiki_image() retries note-relative paths under sources/, so agents can pass image paths verbatim as seen in either surface. - Query/skill-factory prompts and the openkb skill's wiki-schema.md updated to describe both path forms. Existing KBs keep their old-style links (the read_wiki_image fallback does not cover them in renderers); a migration helper for already ingested sources pages is left as a follow-up. Co-Authored-By: Claude Fable 5 --- openkb/agent/query.py | 12 +++-- openkb/agent/tools.py | 13 ++++- openkb/images.py | 37 ++++++++++---- openkb/skill/creator.py | 6 ++- skills/openkb/references/wiki-schema.md | 7 ++- tests/test_agent_tools.py | 50 ++++++++++++++++++ tests/test_images.py | 67 ++++++++++++++++++++++--- 7 files changed, 166 insertions(+), 26 deletions(-) diff --git a/openkb/agent/query.py b/openkb/agent/query.py index 885ec235..f4627494 100644 --- a/openkb/agent/query.py +++ b/openkb/agent/query.py @@ -37,8 +37,11 @@ - PageIndex documents (doc_type: pageindex): use get_page_content(doc_name, pages) with tight page ranges. The summary shows document tree structure with page ranges to help you target. Never fetch the whole document. -6. Source content may reference images (e.g. ![image](sources/images/doc/file.png)). - Use the get_image tool to view them when needed. +6. Source content may reference images. Short-doc .md pages link them + note-relative (e.g. ![image](images/doc/file.png), resolved from + wiki/sources/); long-doc JSON page metadata lists them wiki-root-relative + (e.g. sources/images/doc/file.png). Pass either form as seen to the + get_image tool — it accepts both. 7. Synthesize a clear, concise, well-cited answer grounded in wiki content. Answer based only on wiki content. Be concise. @@ -81,7 +84,10 @@ def get_image(image_path: str) -> ToolOutputImage | ToolOutputText: you'd need to see to answer accurately. Args: - image_path: Image path relative to wiki root (e.g. 'sources/images/doc/p1_img1.png'). + image_path: Image path as it appears in the content — either + wiki-root-relative ('sources/images/doc/p1_img1.png') or + note-relative as used in sources/ .md pages + ('images/doc/p1_img1.png'). """ result = read_wiki_image(image_path, wiki_root) if result["type"] == "image": diff --git a/openkb/agent/tools.py b/openkb/agent/tools.py index 6ab4366c..2df93b0e 100644 --- a/openkb/agent/tools.py +++ b/openkb/agent/tools.py @@ -147,7 +147,10 @@ def read_wiki_image(path: str, wiki_root: str) -> dict: """Read an image file from the wiki and return as base64 data URL. Args: - path: Image path relative to *wiki_root* (e.g. ``"sources/images/doc/p1_img1.png"``). + path: Image path relative to *wiki_root* + (e.g. ``"sources/images/doc/p1_img1.png"``), or note-relative + as embedded in sources/ .md pages + (``"images/doc/p1_img1.png"`` — retried under ``sources/``). wiki_root: Absolute path to the wiki root directory. Returns: @@ -161,7 +164,13 @@ def read_wiki_image(path: str, wiki_root: str) -> dict: if not full_path.is_relative_to(root): return {"type": "text", "text": "Access denied: path escapes wiki root."} if not full_path.exists(): - return {"type": "text", "text": f"Image not found: {path}"} + # Source .md pages embed images note-relative ("images//", + # resolved from wiki/sources/). Callers pass those verbatim — retry + # under sources/ before failing. + alt_path = (root / "sources" / path).resolve() + if not (alt_path.is_relative_to(root) and alt_path.exists()): + return {"type": "text", "text": f"Image not found: {path}"} + full_path = alt_path mime = _MIME_TYPES.get(full_path.suffix.lower(), "image/png") b64 = base64.b64encode(full_path.read_bytes()).decode() diff --git a/openkb/images.py b/openkb/images.py index 0e71fde0..002ab2d5 100644 --- a/openkb/images.py +++ b/openkb/images.py @@ -23,6 +23,19 @@ _MIN_IMAGE_DIM = 32 +def md_image_ref(alt: str, doc_name: str, filename: str) -> str: + """Markdown image reference as written into ``wiki/sources/.md``. + + Note-relative (``images/{doc}/{file}``): source pages live directly in + ``wiki/sources/``, so this resolves in every renderer that resolves links + relative to the containing file (Obsidian, GitHub, VS Code). Internal + metadata (the per-page JSON of long docs) keeps wiki-root-relative + ``sources/images/...`` paths instead — those are consumed by tools that + resolve against the wiki root, not rendered from a note. + """ + return f"![{alt}](images/{doc_name}/{filename})" + + def extract_pdf_images(pdf_path: Path, doc_name: str, images_dir: Path) -> dict[int, list[str]]: """Extract images from a PDF using pymupdf's dict-mode block iteration. @@ -31,7 +44,9 @@ def extract_pdf_images(pdf_path: Path, doc_name: str, images_dir: Path) -> dict[ as PNG. This captures both embedded bitmaps *and* vector-rendered figures that ``get_images()`` would miss. - Returns a mapping of page_number (1-based) → list of relative image paths. + Returns a mapping of page_number (1-based) → list of image paths. Paths + are wiki-root-relative (``sources/images/...``) — internal metadata for + tools that resolve against the wiki root, not note-rendered markdown. """ images_dir.mkdir(parents=True, exist_ok=True) page_images: dict[int, list[str]] = {} @@ -77,7 +92,10 @@ def convert_pdf_to_pages(pdf_path: Path, doc_name: str, images_dir: Path) -> lis """Convert a PDF to per-page dicts with text content and images. Each dict has ``{"page": int, "content": str, "images": [{"path": str}]}``. - Images are saved to *images_dir* and referenced with wiki-root-relative paths. + Images are saved to *images_dir* and referenced with wiki-root-relative + ``sources/images/...`` paths — these pages land in ``sources/.json`` + (never rendered from a note), and both ``get_wiki_page_content`` and + ``read_wiki_image`` resolve them against the wiki root. """ images_dir.mkdir(parents=True, exist_ok=True) pages: list[dict] = [] @@ -134,8 +152,9 @@ def convert_pdf_with_images(pdf_path: Path, doc_name: str, images_dir: Path) -> """Convert a PDF to markdown with inline images using pymupdf dict-mode. Iterates blocks in reading order per page. Text blocks become text, - image blocks are saved to disk and replaced with ``![image](path)`` - inline — preserving the original position in the document. + image blocks are saved to disk and replaced with a note-relative + ``![image](images/{doc_name}/...)`` link inline — preserving the + original position in the document. Returns the full markdown string. """ @@ -173,7 +192,7 @@ def convert_pdf_with_images(pdf_path: Path, doc_name: str, images_dir: Path) -> filename = f"p{page_num}_img{img_counter}.png" (images_dir / filename).write_bytes(pix.tobytes("png")) pix = None - parts.append(f"\n![image](sources/images/{doc_name}/{filename})\n") + parts.append(f"\n{md_image_ref('image', doc_name, filename)}\n") except Exception: logger.warning("Failed to save image block on page %d", page_num) return "\n".join(parts) @@ -184,7 +203,7 @@ def extract_base64_images(markdown: str, doc_name: str, images_dir: Path) -> str For each ``![alt](data:image/ext;base64,DATA)`` match: - Decode base64 bytes → save to ``images_dir/img_NNN.ext`` - - Replace the link with ``![alt](sources/images/{doc_name}/img_NNN.ext)`` + - Replace the link with ``![alt](images/{doc_name}/img_NNN.ext)`` - On decode failure: log a warning and leave the original text unchanged. """ counter = 0 @@ -208,7 +227,7 @@ def extract_base64_images(markdown: str, doc_name: str, images_dir: Path) -> str images_dir.mkdir(parents=True, exist_ok=True) dest.write_bytes(image_bytes) - new_ref = f"![{alt}](sources/images/{doc_name}/{filename})" + new_ref = md_image_ref(alt, doc_name, filename) result = result.replace(match.group(0), new_ref, 1) return result @@ -220,7 +239,7 @@ def copy_relative_images(markdown: str, source_dir: Path, doc_name: str, images_ For each ``![alt](relative/path)`` match (skipping http/https and data URIs): - Resolve path relative to ``source_dir`` - Copy to ``images_dir/{filename}`` - - Replace link with ``![alt](sources/images/{doc_name}/{filename})`` + - Replace link with ``![alt](images/{doc_name}/{filename})`` - Missing source file: log a warning and leave the original text unchanged. """ result = markdown @@ -254,7 +273,7 @@ def copy_relative_images(markdown: str, source_dir: Path, doc_name: str, images_ images_dir.mkdir(parents=True, exist_ok=True) shutil.copy2(src, images_dir / filename) - new_ref = f"![{alt}](sources/images/{doc_name}/{filename})" + new_ref = md_image_ref(alt, doc_name, filename) result = result.replace(match.group(0), new_ref, 1) return result diff --git a/openkb/skill/creator.py b/openkb/skill/creator.py index a7748701..9e71bfec 100644 --- a/openkb/skill/creator.py +++ b/openkb/skill/creator.py @@ -105,8 +105,10 @@ def get_image(image_path: str) -> ToolOutputImage | ToolOutputText: need to see in order to distil it correctly into the skill. Args: - image_path: Path relative to wiki/ - (e.g. ``"sources/images/doc/p1_img1.png"``). + image_path: Image path as it appears in the content — either + wiki-root-relative (``"sources/images/doc/p1_img1.png"``) + or note-relative as used in sources/ .md pages + (``"images/doc/p1_img1.png"``). """ result = _read_image_impl(image_path, wiki_root) if result["type"] == "image": diff --git a/skills/openkb/references/wiki-schema.md b/skills/openkb/references/wiki-schema.md index ca95026f..76b346fe 100644 --- a/skills/openkb/references/wiki-schema.md +++ b/skills/openkb/references/wiki-schema.md @@ -99,8 +99,11 @@ thing, read the matching entity page first. ## `wiki/sources/.md` (short docs) -The markitdown-converted full text. Image refs appear as -`![](sources/images//p1_img1.png)`. +The markitdown-converted full text. Image refs are note-relative — +`![](images//p1_img1.png)` — and resolve from `wiki/sources/` +(the files live in `wiki/sources/images//`). The per-page JSON of +long docs instead lists images wiki-root-relative +(`sources/images//...`). ## `wiki/sources/.json` (long PDFs) diff --git a/tests/test_agent_tools.py b/tests/test_agent_tools.py index 857b10d1..d1a9d162 100644 --- a/tests/test_agent_tools.py +++ b/tests/test_agent_tools.py @@ -7,9 +7,59 @@ list_wiki_files, parse_pages, read_wiki_file, + read_wiki_image, write_wiki_file, ) +FAKE_PNG = b"\x89PNG\r\n\x1a\n" + b"\x00" * 8 + + +# --------------------------------------------------------------------------- +# read_wiki_image +# --------------------------------------------------------------------------- + + +class TestReadWikiImage: + def _make_image(self, tmp_path): + images_dir = tmp_path / "sources" / "images" / "doc" + images_dir.mkdir(parents=True) + (images_dir / "p1_img1.png").write_bytes(FAKE_PNG) + + def test_reads_wiki_root_relative_path(self, tmp_path): + self._make_image(tmp_path) + + result = read_wiki_image("sources/images/doc/p1_img1.png", str(tmp_path)) + + assert result["type"] == "image" + assert result["image_url"].startswith("data:image/png;base64,") + + def test_reads_note_relative_path_from_sources(self, tmp_path): + # Source .md pages embed images as "images//" (relative to + # wiki/sources/); the tool must resolve those verbatim too. + self._make_image(tmp_path) + + result = read_wiki_image("images/doc/p1_img1.png", str(tmp_path)) + + assert result["type"] == "image" + assert result["image_url"].startswith("data:image/png;base64,") + + def test_missing_image_reports_not_found(self, tmp_path): + self._make_image(tmp_path) + + result = read_wiki_image("images/doc/nope.png", str(tmp_path)) + + assert result["type"] == "text" + assert "not found" in result["text"].lower() + + def test_path_escape_denied(self, tmp_path): + self._make_image(tmp_path) + + result = read_wiki_image("../outside.png", str(tmp_path)) + + assert result["type"] == "text" + assert "Access denied" in result["text"] + + # --------------------------------------------------------------------------- # list_wiki_files # --------------------------------------------------------------------------- diff --git a/tests/test_images.py b/tests/test_images.py index 872cc2fa..cb0e2189 100644 --- a/tests/test_images.py +++ b/tests/test_images.py @@ -41,7 +41,7 @@ def test_single_base64_image_extracted(self, tmp_path): # Result should reference a saved file, not the raw base64 assert "data:image/png;base64," not in result - assert "![alt text](sources/images/doc/img_001.png)" == result + assert "![alt text](images/doc/img_001.png)" == result # File should exist on disk saved = images_dir / "img_001.png" @@ -56,8 +56,8 @@ def test_multiple_base64_images_numbered_sequentially(self, tmp_path): md = f"![fig1](data:image/png;base64,{b64_png})\n![fig2](data:image/jpeg;base64,{b64_jpg})" result = extract_base64_images(md, "doc", images_dir) - assert "![fig1](sources/images/doc/img_001.png)" in result - assert "![fig2](sources/images/doc/img_002.jpeg)" in result + assert "![fig1](images/doc/img_001.png)" in result + assert "![fig2](images/doc/img_002.jpeg)" in result assert (images_dir / "img_001.png").exists() assert (images_dir / "img_002.jpeg").exists() @@ -85,7 +85,7 @@ def test_mixed_valid_invalid_base64(self, tmp_path, caplog): with caplog.at_level(logging.WARNING, logger="openkb.images"): result = extract_base64_images(md, "doc", images_dir) - assert "![good](sources/images/doc/img_001.png)" in result + assert "![good](images/doc/img_001.png)" in result assert f"data:image/png;base64,{bad}" in result @@ -107,7 +107,7 @@ def test_existing_relative_image_copied_and_rewritten(self, tmp_path): md = "![diagram](diagram.png)" result = copy_relative_images(md, source_dir, "doc", images_dir) - assert "![diagram](sources/images/doc/diagram.png)" == result + assert "![diagram](images/doc/diagram.png)" == result assert (images_dir / "diagram.png").read_bytes() == FAKE_PNG def test_missing_relative_image_leaves_original(self, tmp_path, caplog): @@ -157,8 +157,8 @@ def test_multiple_relative_images_all_copied(self, tmp_path): md = "![a](a.png)\n![b](b.jpg)" result = copy_relative_images(md, source_dir, "doc", images_dir) - assert "![a](sources/images/doc/a.png)" in result - assert "![b](sources/images/doc/b.jpg)" in result + assert "![a](images/doc/a.png)" in result + assert "![b](images/doc/b.jpg)" in result assert (images_dir / "a.png").exists() assert (images_dir / "b.jpg").exists() @@ -195,4 +195,55 @@ def test_same_image_referenced_twice_is_copied_once(self, tmp_path): result = copy_relative_images(md, source_dir, "doc", images_dir) assert [p.name for p in images_dir.iterdir()] == ["logo.png"] - assert result.count("sources/images/doc/logo.png") == 2 + assert result.count("images/doc/logo.png") == 2 + + +# --------------------------------------------------------------------------- +# Note-relative resolution (Obsidian / GitHub compatibility) +# --------------------------------------------------------------------------- + + +class TestNoteRelativeResolution: + """Generated ``![...](...)`` links must resolve relative to the note. + + Source pages live at ``wiki/sources/.md`` and their images at + ``wiki/sources/images//`` — renderers that resolve links relative + to the containing file (Obsidian, GitHub, VS Code) must find the image. + A wiki-root-relative link (``sources/images/...``) would resolve to the + non-existent ``wiki/sources/sources/images/...`` from those notes. + """ + + @staticmethod + def _link_paths(markdown: str) -> list[str]: + import re + + return re.findall(r"!\[[^\]]*\]\(([^)]+)\)", markdown) + + def test_base64_image_ref_resolves_from_sources_note(self, tmp_path): + wiki = tmp_path / "wiki" + note = wiki / "sources" / "doc.md" + images_dir = wiki / "sources" / "images" / "doc" + images_dir.mkdir(parents=True) + + md = f"![alt](data:image/png;base64,{_make_b64(FAKE_PNG)})" + result = extract_base64_images(md, "doc", images_dir) + note.write_text(result, encoding="utf-8") + + for rel in self._link_paths(result): + assert (note.parent / rel).exists(), rel + + def test_copied_image_ref_resolves_from_sources_note(self, tmp_path): + source_dir = tmp_path / "clip" + source_dir.mkdir() + (source_dir / "figure.png").write_bytes(FAKE_PNG) + + wiki = tmp_path / "wiki" + note = wiki / "sources" / "doc.md" + images_dir = wiki / "sources" / "images" / "doc" + images_dir.mkdir(parents=True) + + result = copy_relative_images("![f](figure.png)", source_dir, "doc", images_dir) + note.write_text(result, encoding="utf-8") + + for rel in self._link_paths(result): + assert (note.parent / rel).exists(), rel