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) <noreply@anthropic.com>
This commit is contained in:
@@ -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`).
|
**`--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
|
```bash
|
||||||
# Markdown engine, create a row from a doc with callouts/columns
|
# Markdown engine, create a row from a doc with callouts/columns
|
||||||
|
|||||||
@@ -262,11 +262,13 @@ def find_existing_page(
|
|||||||
|
|
||||||
|
|
||||||
def create_page_markdown(client, parent, properties, markdown):
|
def create_page_markdown(client, parent, properties, markdown):
|
||||||
"""POST /v1/pages with the enhanced-markdown body param."""
|
"""POST /v1/pages with the enhanced-markdown body param.
|
||||||
return client.request(
|
`markdown` is only included in the body when non-empty to avoid sending
|
||||||
path="pages", method="POST",
|
a blank markdown field when no --file/--stdin was supplied."""
|
||||||
body={"parent": parent, "properties": properties, "markdown": markdown},
|
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):
|
def append_markdown(client, page_id, markdown):
|
||||||
|
|||||||
@@ -43,9 +43,23 @@ def translate(content: str) -> str:
|
|||||||
lines = content.split("\n")
|
lines = content.split("\n")
|
||||||
out: List[str] = []
|
out: List[str] = []
|
||||||
i = 0
|
i = 0
|
||||||
|
in_fence = False
|
||||||
while i < len(lines):
|
while i < len(lines):
|
||||||
line = lines[i]
|
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
|
# Callout: > [!TYPE] then contiguous > body lines
|
||||||
if line.lstrip().startswith(">"):
|
if line.lstrip().startswith(">"):
|
||||||
body_first = line.lstrip()[1:].strip()
|
body_first = line.lstrip()[1:].strip()
|
||||||
|
|||||||
@@ -18,6 +18,7 @@ from notion_client.errors import APIResponseError
|
|||||||
|
|
||||||
import _notion_compat as compat
|
import _notion_compat as compat
|
||||||
import ntn_files
|
import ntn_files
|
||||||
|
from ntn_files import NtnUploadError
|
||||||
import md_translate
|
import md_translate
|
||||||
|
|
||||||
MARKDOWN_NOTION_VERSION = "2026-03-11"
|
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:
|
def _content_has_local_images(content: str) -> bool:
|
||||||
|
in_fence = False
|
||||||
for line in content.split('\n'):
|
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)
|
m = IMAGE_RE.match(line)
|
||||||
if m and not m.group(2).startswith(('http://', 'https://')):
|
if m and not m.group(2).startswith(('http://', 'https://')):
|
||||||
return True
|
return True
|
||||||
@@ -542,9 +549,18 @@ def _content_has_local_images(content: str) -> bool:
|
|||||||
|
|
||||||
def extract_local_images(content: str):
|
def extract_local_images(content: str):
|
||||||
"""Remove standalone LOCAL image lines; keep remote ones inline.
|
"""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 = [], []
|
kept, imgs = [], []
|
||||||
|
in_fence = False
|
||||||
for line in content.split('\n'):
|
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)
|
m = IMAGE_RE.match(line)
|
||||||
if m and not m.group(2).startswith(('http://', 'https://')):
|
if m and not m.group(2).startswith(('http://', 'https://')):
|
||||||
imgs.append((m.group(1), m.group(2)))
|
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
|
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():
|
def main():
|
||||||
parser = argparse.ArgumentParser(
|
parser = argparse.ArgumentParser(
|
||||||
description='Push markdown content to Notion pages or databases',
|
description='Push markdown content to Notion pages or databases',
|
||||||
@@ -853,6 +854,17 @@ Examples:
|
|||||||
|
|
||||||
args = parser.parse_args()
|
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:
|
if not NOTION_TOKEN:
|
||||||
print("Error: NOTION_API_KEY not set in environment.")
|
print("Error: NOTION_API_KEY not set in environment.")
|
||||||
print("Preferred: fetch from 1Password at runtime —")
|
print("Preferred: fetch from 1Password at runtime —")
|
||||||
|
|||||||
@@ -80,6 +80,10 @@ def upload(path: Path) -> str:
|
|||||||
f"ntn upload failed for {path}", path=str(path),
|
f"ntn upload failed for {path}", path=str(path),
|
||||||
stderr=result.stderr.strip(),
|
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]
|
first_line = result.stdout.strip().splitlines()[0]
|
||||||
upload_id = first_line.split("\t")[0].strip()
|
upload_id = first_line.split("\t")[0].strip()
|
||||||
return upload_id
|
return upload_id
|
||||||
|
|||||||
@@ -64,6 +64,18 @@ def test_content_has_local_images():
|
|||||||
assert notion_writer._content_has_local_images("no images") is False
|
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\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 "" in without, "fence content must be preserved in output"
|
||||||
|
|
||||||
|
|
||||||
def run_all():
|
def run_all():
|
||||||
tests = [
|
tests = [
|
||||||
test_create_page_markdown,
|
test_create_page_markdown,
|
||||||
@@ -71,6 +83,7 @@ def run_all():
|
|||||||
test_replace_markdown,
|
test_replace_markdown,
|
||||||
test_extract_local_images_splits,
|
test_extract_local_images_splits,
|
||||||
test_content_has_local_images,
|
test_content_has_local_images,
|
||||||
|
test_local_image_inside_fence_ignored,
|
||||||
]
|
]
|
||||||
for t in tests:
|
for t in tests:
|
||||||
print(f"\n{t.__name__}")
|
print(f"\n{t.__name__}")
|
||||||
|
|||||||
@@ -56,6 +56,28 @@ def test_mention_invalid_plain():
|
|||||||
assert "mention-page" not in out
|
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 "<callout" not in out, "callout must not be emitted inside fence"
|
||||||
|
assert "<columns>" not in out, "columns must not be emitted inside fence"
|
||||||
|
assert "<mention-page" not in out, "mention must not be emitted inside fence"
|
||||||
|
# Original lines must be present verbatim
|
||||||
|
assert "> [!NOTE]" in out
|
||||||
|
assert "::: columns" in out
|
||||||
|
assert f"@[Page]({raw_id})" in out
|
||||||
|
|
||||||
|
|
||||||
def run_all():
|
def run_all():
|
||||||
tests = [
|
tests = [
|
||||||
test_passthrough_basics,
|
test_passthrough_basics,
|
||||||
@@ -65,6 +87,7 @@ def run_all():
|
|||||||
test_toggle_children_indented,
|
test_toggle_children_indented,
|
||||||
test_mention_url,
|
test_mention_url,
|
||||||
test_mention_invalid_plain,
|
test_mention_invalid_plain,
|
||||||
|
test_fence_passthrough_no_transform,
|
||||||
]
|
]
|
||||||
for t in tests:
|
for t in tests:
|
||||||
print(f"\n{t.__name__}")
|
print(f"\n{t.__name__}")
|
||||||
|
|||||||
Reference in New Issue
Block a user