From 759bb20911946ee0418bccc4b2209c3400a15e29 Mon Sep 17 00:00:00 2001 From: Andrew Yim Date: Sat, 27 Jun 2026 10:33:09 +0900 Subject: [PATCH] fix(notion-writer): fence-aware image/translate, NtnUploadError boundary, dead-code removal FIX 1: _content_has_local_images and extract_local_images now skip lines inside ``` fences so a local-image ref inside a code block is not treated as real. FIX 2: md_translate.translate() tracks fence state; callout/columns/mention transforms are suppressed for lines inside ``` ... ``` blocks (pass verbatim). FIX 3: main() catches NtnUploadError and prints a clean error + e.stderr before sys.exit(1); body moved to _main_body(parser, args). FIX 4: ntn_files.upload() raises NtnUploadError on empty stdout instead of IndexError, giving a clean message combined with FIX 3. FIX 5: Removed unused write_to_page() (both page paths are inlined). FIX 6: create_page_markdown() omits the 'markdown' key when value is falsy, avoiding empty-markdown sends on DB rows created without --file/--stdin. FIX 7: CLAUDE.md documents --allow-deleting-content scope (markdown engine only). TDD: Added test_fence_passthrough_no_transform (md_translate, suite now 8) and test_local_image_inside_fence_ignored (engine_routing, suite now 6). Co-Authored-By: Claude Opus 4.8 (1M context) --- custom-skills/32-notion-writer/code/CLAUDE.md | 2 +- .../code/scripts/_notion_compat.py | 12 ++--- .../code/scripts/md_translate.py | 14 ++++++ .../code/scripts/notion_writer.py | 44 ++++++++++++------- .../code/scripts/ntn_files.py | 4 ++ .../code/scripts/test_engine_routing.py | 13 ++++++ .../code/scripts/test_md_translate.py | 23 ++++++++++ 7 files changed, 90 insertions(+), 22 deletions(-) diff --git a/custom-skills/32-notion-writer/code/CLAUDE.md b/custom-skills/32-notion-writer/code/CLAUDE.md index af5951d..636c3d3 100644 --- a/custom-skills/32-notion-writer/code/CLAUDE.md +++ b/custom-skills/32-notion-writer/code/CLAUDE.md @@ -223,7 +223,7 @@ Two write engines, selected with `--engine {blocks,markdown}`. Default is `block **`--notion-version`**: Override the API version for the markdown engine (default: `2026-03-11`). -**`--allow-deleting-content`**: Required when `--replace` needs to delete child pages or databases under the target page; the Notion API refuses such deletions unless this flag is present. +**`--allow-deleting-content`**: Required when `--replace` needs to delete child pages or databases under the target page; the Notion API refuses such deletions unless this flag is present. Applies only to the markdown engine's `--replace` path (`--engine markdown --replace`); has no effect with `--engine blocks`. ```bash # Markdown engine, create a row from a doc with callouts/columns diff --git a/custom-skills/32-notion-writer/code/scripts/_notion_compat.py b/custom-skills/32-notion-writer/code/scripts/_notion_compat.py index 281fda8..ff45961 100644 --- a/custom-skills/32-notion-writer/code/scripts/_notion_compat.py +++ b/custom-skills/32-notion-writer/code/scripts/_notion_compat.py @@ -262,11 +262,13 @@ def find_existing_page( def create_page_markdown(client, parent, properties, markdown): - """POST /v1/pages with the enhanced-markdown body param.""" - return client.request( - path="pages", method="POST", - body={"parent": parent, "properties": properties, "markdown": markdown}, - ) + """POST /v1/pages with the enhanced-markdown body param. + `markdown` is only included in the body when non-empty to avoid sending + a blank markdown field when no --file/--stdin was supplied.""" + body: Dict[str, Any] = {"parent": parent, "properties": properties} + if markdown: + body["markdown"] = markdown + return client.request(path="pages", method="POST", body=body) def append_markdown(client, page_id, markdown): diff --git a/custom-skills/32-notion-writer/code/scripts/md_translate.py b/custom-skills/32-notion-writer/code/scripts/md_translate.py index c280b7c..2f4ca06 100644 --- a/custom-skills/32-notion-writer/code/scripts/md_translate.py +++ b/custom-skills/32-notion-writer/code/scripts/md_translate.py @@ -43,9 +43,23 @@ def translate(content: str) -> str: lines = content.split("\n") out: List[str] = [] i = 0 + in_fence = False while i < len(lines): line = lines[i] + # Fence tracking: pass through unchanged, toggle state + if line.strip().startswith("```"): + in_fence = not in_fence + out.append(line) + i += 1 + continue + + # Inside fence: pass through verbatim, no transformation + if in_fence: + out.append(line) + i += 1 + continue + # Callout: > [!TYPE] then contiguous > body lines if line.lstrip().startswith(">"): body_first = line.lstrip()[1:].strip() diff --git a/custom-skills/32-notion-writer/code/scripts/notion_writer.py b/custom-skills/32-notion-writer/code/scripts/notion_writer.py index f0a1dbd..0a62a66 100644 --- a/custom-skills/32-notion-writer/code/scripts/notion_writer.py +++ b/custom-skills/32-notion-writer/code/scripts/notion_writer.py @@ -18,6 +18,7 @@ from notion_client.errors import APIResponseError import _notion_compat as compat import ntn_files +from ntn_files import NtnUploadError import md_translate MARKDOWN_NOTION_VERSION = "2026-03-11" @@ -533,7 +534,13 @@ def create_image_block(alt: str, target: str) -> Dict[str, Any]: def _content_has_local_images(content: str) -> bool: + in_fence = False for line in content.split('\n'): + if line.strip().startswith('```'): + in_fence = not in_fence + continue # fence delimiter lines are never image lines + if in_fence: + continue m = IMAGE_RE.match(line) if m and not m.group(2).startswith(('http://', 'https://')): return True @@ -542,9 +549,18 @@ def _content_has_local_images(content: str) -> bool: def extract_local_images(content: str): """Remove standalone LOCAL image lines; keep remote ones inline. - Returns (content_without_local_images, [(alt, target), ...]).""" + Returns (content_without_local_images, [(alt, target), ...]). + Lines inside fenced code blocks are passed through unchanged.""" kept, imgs = [], [] + in_fence = False for line in content.split('\n'): + if line.strip().startswith('```'): + in_fence = not in_fence + kept.append(line) + continue + if in_fence: + kept.append(line) + continue m = IMAGE_RE.match(line) if m and not m.group(2).startswith(('http://', 'https://')): imgs.append((m.group(1), m.group(2))) @@ -792,21 +808,6 @@ def update_page_properties(notion: Client, page_id: str, properties: Dict) -> bo return False -def write_to_page(notion: Client, page_id: str, markdown_content: str, mode: str = 'append') -> bool: - """Write markdown content to a Notion page.""" - blocks = markdown_to_notion_blocks(markdown_content) - - if not blocks: - print("No content to write") - return False - - if mode == 'replace': - if not clear_page_content(notion, page_id): - return False - - return append_to_page(notion, page_id, blocks) - - def main(): parser = argparse.ArgumentParser( description='Push markdown content to Notion pages or databases', @@ -853,6 +854,17 @@ Examples: args = parser.parse_args() + try: + _main_body(parser, args) + except NtnUploadError as e: + print(f"Error: {e}") + if e.stderr: + print(e.stderr) + sys.exit(1) + + +def _main_body(parser, args): + """Body of main() after argparse; separated so NtnUploadError can be caught cleanly.""" if not NOTION_TOKEN: print("Error: NOTION_API_KEY not set in environment.") print("Preferred: fetch from 1Password at runtime —") diff --git a/custom-skills/32-notion-writer/code/scripts/ntn_files.py b/custom-skills/32-notion-writer/code/scripts/ntn_files.py index 38b1914..992111b 100644 --- a/custom-skills/32-notion-writer/code/scripts/ntn_files.py +++ b/custom-skills/32-notion-writer/code/scripts/ntn_files.py @@ -80,6 +80,10 @@ def upload(path: Path) -> str: f"ntn upload failed for {path}", path=str(path), stderr=result.stderr.strip(), ) + if not result.stdout.strip(): + raise NtnUploadError( + "ntn returned no output", path=str(path), stderr=result.stderr.strip() + ) first_line = result.stdout.strip().splitlines()[0] upload_id = first_line.split("\t")[0].strip() return upload_id diff --git a/custom-skills/32-notion-writer/code/scripts/test_engine_routing.py b/custom-skills/32-notion-writer/code/scripts/test_engine_routing.py index 11c6ac8..7278cfb 100644 --- a/custom-skills/32-notion-writer/code/scripts/test_engine_routing.py +++ b/custom-skills/32-notion-writer/code/scripts/test_engine_routing.py @@ -64,6 +64,18 @@ def test_content_has_local_images(): assert notion_writer._content_has_local_images("no images") is False +def test_local_image_inside_fence_ignored(): + """A local image reference inside a fenced code block must NOT be detected + as a real image by _content_has_local_images or extracted by extract_local_images.""" + fenced = "```markdown\n![x](./y.png)\n```" + assert notion_writer._content_has_local_images(fenced) is False, \ + "_content_has_local_images should return False for image inside fence" + without, imgs = notion_writer.extract_local_images(fenced) + assert len(imgs) == 0, "extract_local_images should not extract image inside fence" + # The fenced lines (including the image line) should be preserved verbatim + assert "![x](./y.png)" in without, "fence content must be preserved in output" + + def run_all(): tests = [ test_create_page_markdown, @@ -71,6 +83,7 @@ def run_all(): test_replace_markdown, test_extract_local_images_splits, test_content_has_local_images, + test_local_image_inside_fence_ignored, ] for t in tests: print(f"\n{t.__name__}") diff --git a/custom-skills/32-notion-writer/code/scripts/test_md_translate.py b/custom-skills/32-notion-writer/code/scripts/test_md_translate.py index e55c8a7..245a699 100644 --- a/custom-skills/32-notion-writer/code/scripts/test_md_translate.py +++ b/custom-skills/32-notion-writer/code/scripts/test_md_translate.py @@ -56,6 +56,28 @@ def test_mention_invalid_plain(): assert "mention-page" not in out +def test_fence_passthrough_no_transform(): + """Lines inside fenced code blocks must pass through verbatim — no callout, + columns, or mention transformation applied.""" + raw_id = "abcdef0123456789abcdef0123456789" + src = ( + "```\n" + "> [!NOTE]\n" + "::: columns\n" + f"@[Page]({raw_id})\n" + "```" + ) + out = md_translate.translate(src) + # No transformation should have occurred + assert "" not in out, "columns must not be emitted inside fence" + assert " [!NOTE]" in out + assert "::: columns" in out + assert f"@[Page]({raw_id})" in out + + def run_all(): tests = [ test_passthrough_basics, @@ -65,6 +87,7 @@ def run_all(): test_toggle_children_indented, test_mention_url, test_mention_invalid_plain, + test_fence_passthrough_no_transform, ] for t in tests: print(f"\n{t.__name__}")