Read a notebook cell's source as text, not as characters

[Anish Mehta (anishmehta24)]

Motivation and Context

nbformat's multiline_string is a string or a list of lines, and both are valid. nbformat.validate accepts either. The converter walked the raw value to find the title:

for line in source_lines:
    if line.startswith("# "):

With a list that iterates lines, which is what was intended. With a string it iterates characters — and no single character starts with "# " — so a notebook written that way comes out with no title at all:

notebook = {
    "nbformat": 4, "nbformat_minor": 5, "metadata": {},
    "cells": [{"cell_type": "markdown", "source": "# My Notebook\nintro text\n", "metadata": {}}],
}
IpynbConverter()._convert(notebook).title    # None

The same notebook written with "source": ["# My Notebook\n", "intro text\n"] gives "My Notebook".

The markdown body is unaffected either way, which is why this is easy to miss: "".join(...) over a string reassembles the string, so only the title is lost.

nbformat.writes normalises to lists, so notebooks saved by Jupyter itself aren't affected. Notebooks assembled as JSON and written directly — generated notebooks, tooling that builds cells programmatically and calls json.dump rather than nbformat.writes — are.

Description

Take the cell's text once, then split it for the heading scan:

source = cell.get("source", [])
source_text = source if isinstance(source, str) else "".join(source)

The body uses source_text directly, so its output is byte-identical to before for list sources.

The title scan now splits source_text on \r\n, \r and \n. Besides fixing the string case, that also picks up a heading in a list entry holding several lines (["intro\n# My Notebook\n"]), which the per-entry check couldn't see — same root cause, so I fixed it in the same place rather than leaving a second shape broken.

No behaviour changes for notebooks that already worked; the existing #hashtag title regression test still passes.

Tests

Five tests in packages/markitdown/tests/test_module_misc.py, next to the existing ipynb ones:

• test_ipynb_string_source_is_read_as_text_not_characters — a string source now yields the title
• test_ipynb_string_and_list_sources_agree — the two spellings of the same notebook produce identical title and markdown
• test_ipynb_heading_below_the_first_line_of_a_source_entry — a heading after the first line of a list entry
• test_ipynb_heading_only_counts_at_the_start_of_a_line — a # after U+2028 is not a heading
• test_ipynb_heading_with_cr_line_endings — \r line endings still give just the heading

The first three fail on main; the last two are guards. test_module_misc.py is 173 passed, 1 skipped, 2 failed; the two failures need optional dependencies (speech, pptx) I don't have installed.

[PRABHU KIRAN VANDRANKI (VANDRANKI)]

Community review, so this does not clear the merge gate.

I read the full diff and the surrounding _convert in _ipynb_converter.py. The bug is real and your explanation is right: for line in source_lines iterates characters when source is a str, so line.startswith("# ") can never match and the title is lost, while the body is fine because "".join(str) returns the same string. Normalising once to source_text and reusing it for the markdown, code and raw branches is the correct shape.

I did not run your tests or the package. I ran the old and new title loops on their own against a few inputs to compare behaviour:

• "# T\nx" (string source): old has no way to find it, new gives T. Fixed.
• ["a\n", "# T\n"] and ["x\n", "# T\n"]: same result before and after.
• ["intro", "# Title\n"] (list entries missing their trailing newline): old finds Title, new returns None, because the join makes intro# Title. This is a non-conforming notebook, and the body already renders it as intro# Title, so I think the new answer is defensible. Just noting it is a behaviour change for list sources, not only the string case.
• ["a\u2028# T\n"] and ["a\x0c# T\n"]: old None, new T. str.splitlines() breaks on U+2028, form feed, \x1c-\x1e, \x85 and lone \r, not only \n, so a heading can now start after one of those characters. Notebooks with pasted text can contain U+2028. source_text.split("\n") keeps the intent (a heading must start a line) without that surprise, and "\r\n" lines are still fine because .strip() removes the trailing \r.

Suggested change:

for line in source_text.split("\n"):

The three new tests cover string source, string vs list agreement, and a multi-line list entry, which are the right cases. I would add one for a \u2028 inside a markdown cell if you switch to split("\n").

[Anish Mehta (anishmehta24)]

PRABHU KIRAN VANDRANKI (@VANDRANKI) thanks, switched to split("\n") and added a test with a U+2028 before the "# ". For list entries without a trailing newline I kept the new behavior, since the body already renders them joined.

[XU (kokokoXUY)]

Review: read a cell's source as text, not as characters

Thanks Anish Mehta (@anishmehta24). Commit 1 diagnoses something real, and I can reproduce it: for a markdown cell whose source is a plain string, the old loop walked the raw value one character at a time, so line.startswith("# ") could never be true and the notebook came out without a title. I built both commits in their own worktrees (d47bdd80, da81da6e), ran the converter directly and through the public API, and compared against the branch base. The fix is correct for everything it targets. I found one line-splitting regression that commit 2 introduced, plus two descriptions that do not match what nbformat actually does.

Head reviewed: da81da6ececab6c09ffcdee6732b4d0f5bd2abdf, PR base b8f79c57ebc0044be41323d89b2a45d3fda8460e, current main tip 4cc9fa17653d695d64fb9eee5b33d4de55ff84e8. mergeable_state is dirty (see "Rebase" below).

Verified: title extraction, base -> head

IpynbConverter()._convert(notebook) on hand-built notebooks; "body" is the markdown output, hashed to show whether the fix changed anything besides the title.

cell source	base b8f79c5	head da81da6
"# My Notebook\nintro text\n" (str)	None	My Notebook (body byte-identical, sha 134787ed8104)
["intro paragraph\n# My Notebook\n"]	None	My Notebook
["# My Notebook\nbody text"] (one entry, two lines)	My Notebook\nbody text	My Notebook
"# My Notebook\r\nintro\r\n" (str)	None	My Notebook
"#hashtag\n# Real Title\n" (str)	None	Real Title
"# Spaced Title \n" (str)	None	Spaced Title
two markdown cells, both string sources	None	Later Heading

End-to-end, writing real .ipynb files and going through MarkItDown().convert() and convert_stream() (extension .ipynb): string_src.ipynb None -> My Notebook (markdown sha 50a15ba2654c unchanged), crlf_string_src.ipynb None -> My Notebook (04d4bdc1a7a9 unchanged), late_heading.ipynb None -> My Notebook (a6817126002c unchanged). Title is the only thing that changes, which is what a title fix should look like.

Unchanged on both sides, so not part of this PR's blast radius: code and raw cells never set a title; metadata.title still wins; \u2028, \u2029, NEL, VT and FF are not treated as line breaks; "#\tTitled\n" sets no title (still a gap, pre-existing); "source": null raises Error converting .ipynb file: can only join an iterable on both (the new isinstance(source, str) guard does not cover it).

One regression: CR-only line endings

source: ["# My Notebook\r", "intro text\r"]        expected title "My Notebook"
base b8f79c5     -> My Notebook
commit 1 d47bdd8 -> My Notebook
head   da81da6   -> My Notebook\rintro text        <-- first "# " wins, remainder lands in the title

Same result through the public converter on cr_only_list_src.ipynb (markdown is unchanged, sha 4cdcdaeb844d in all three). The cause is the new split at _ipynb_converter.py:87: source_text.split("\n") treats a \r-only file as a single line, so the heading test matches the first "# " anywhere and removeprefix keeps everything after it. Commit 1 used splitlines() and did not have this; base tolerated it because each list entry was scanned separately. CR-only files are rare, but this is a strict regression against both the base and the previous commit, so it is worth the one-line change:

for line in re.split(r"\r\n|\r|\n", source_text):

That keeps the narrow "only a newline starts a heading" rule commit 2 was asked for, and restores \r and \r\n. Worth a test with "\r" in a list entry, which is also the shape the current tests do not cover.

\u2028: what commit 2 changed

For "intro\u2028# My Notebook\nbody\n" (string or list), commit 1 returned My Notebook, while base and head both return None. So the splitlines() -> split("\n") change makes head agree with base on this shape; it is not a regression, it is a deliberate narrowing, and the new fourth test pins it. The two definitions of "line" are different, though: str.splitlines() is what nbformat itself uses when it writes a notebook (nbformat.v4.nbjson.split_lines calls source.splitlines(True), which also splits on \u2028, \u2029, \x85, \v, \f and \x1c-\x1e). If the intent is "match nbformat", splitlines() is the faithful choice; if the intent is "only LF ends a line", say that explicitly in the comment, because the current test docstring ("a line separator other than a newline") reads as if the separator set were larger than it is.

Two descriptions that do not match nbformat

1. _ipynb_converter.py:72-74 (and the docstring of the first new test) says "nbformat.reads keeps whichever the file used". With nbformat 5.11.1 that is not true: nbformat.reads() runs rejoin_lines, which does cell.source = "".join(cell.source), so a notebook read through nbformat always has a string source; nbformat.writes() runs split_lines (that splitlines(True) call), so the JSON on disk always has a list; and nbformat.validate accepts both spellings. The practical consequence is in the PR's favour: any caller that reads a notebook with nbformat and passes the object to IpynbConverter._convert was always on the broken string branch, so the bug is not limited to files whose spelling survived a plain json.loads. I would state it that way instead - it makes the fix look more valuable, not less.
2. source: null is still an unhandled crash in the same helper (both sides). source = cell.get("source") or [] would cover it; out of scope here, just noting it so it does not get lost.

Rebase: the test file has moved on main

main split up test_module_misc.py: it is 1,627 lines at the PR base and 685 lines at the current tip, with the existing notebook tests now at lines 437-526 (test_ipynb_heading_title_preserves_leading_hash and friends). This PR appends its four tests after the end of the 1,627-line file, which is why the branch is dirty. The source hunk is still fine - _ipynb_converter.py has not changed on main since the base - so the fix needs only a rebase plus moving the tests next to the other ipynb tests. Note that the open notebook PR #2612 touches the same two files (git merge-tree reports conflicts in both), so whoever merges first will need to reconcile the two source/input readers.

CI and local checks

• On commit 1, tests (run 35926642337) and pre-commit (run 35926642446) both finished action_required - first-time fork contributor, so they never actually executed.
• On the current head da81da6 there are no workflow runs at all; the only check is license/cla = success. CI has not verified this revision.
• I ran the suite myself with the head tree's tests and only the source tree swapped (same test set, same interpreter): base 20 failed, 988 passed, 5 skipped / head 17 failed, 991 passed, 5 skipped, and the difference in failing tests is exactly the three new tests that fail on base. The remaining 17 failures are identical on both sides and are missing optional dependencies on this machine (test_cli_vectors.py, test_docintel_html.py, test_cu_converter.py, test_module_misc.py::test_speech_transcription).
• The four new tests: 3 failed, 1 passed against base, 4 passed against head; inside the real file, -k ipynb goes 3 failed, 7 passed -> 10 passed. The fourth test passes on base by construction (base found no title either), so it is a guard rather than a regression test - fine, just worth knowing when reading the counts.
• black 23.7.0 --check (the version pinned in .pre-commit-config.yaml) is clean on both changed files. There is no ruff config in the repository, so that part of pre-commit run --all-files will pass on formatting alone; whatever pre-commit would say, it has not run yet.

AI disclosure

I am an AI agent reviewing this PR on behalf of XU (@kokokoXUY). Every number above was produced by commands I ran against your branch, not copied from the description, and the worktrees were local and disposable; happy to share the exact commands if any result is useful to you.

[Anish Mehta (anishmehta24)]

XU (@kokokoXUY) thanks, switched to re.split(r"\r\n|\r|\n", ...) with a test for \r endings, fixed the nbformat comment, and merged main with the tests moved next to the other ipynb tests.