Parser fixes: keep distinct same-titled pages, use current page metadata, survive pyOneNote terminator crash#1
Merged
Conversation
…ata, survive pyOneNote terminator crash Three fixes in the .one page-building path, validated against real notebooks and a small synthetic fixture (tests/test_data/ExporterEval, dummy content): 1. Distinct pages that share a title are no longer dropped. `_build_pages` built one page per page-node GUID (correct) and then collapsed pages by lowercased title, silently discarding legitimately-distinct pages that share a name (e.g. repeated subpages, many "Notes"). Removed the title collapse; identity is the page-node GUID only. 2. Use the current page-metadata revision, not a stale one. A page's object space holds multiple PageMetaData revisions; the GUID->meta lookup overwrote with whichever was processed last — a stale copy with an outdated PageLevel and/or title (from before a page was renamed or promoted/demoted to a subpage). Keep the first (current) metadata. 3. Survive the pyOneNote FileNodeListFragment terminator crash. A terminator node (file_node_id 0/255, baseType 2) is dereferenced before the fragment loop's terminator-break runs, so heavily-revised sections raise AttributeError. Added a guard in _patch_pyonenote(). This is a workaround for an upstream pyOneNote bug (a fix is proposed separately to DissectMalware/pyOneNote); it can be removed once that is released. Adds tests/test_eval_notebook.py (parser-level asserts) + the fixture.
There was a problem hiding this comment.
Pull request overview
This PR improves the .one parsing/page-building path in OneStoreParser to preserve distinct pages that share titles, prefer the “current” page metadata revision when multiple metadata objects exist, and work around a pyOneNote crash on certain FileNodeListFragment terminator nodes. It also adds fixture-based tests to validate the updated behavior against a committed synthetic notebook.
Changes:
- Removed title-based page deduplication so distinct same-titled pages are retained.
- Adjusted GUID→metadata selection to keep the first-seen metadata entry per GUID (to avoid stale title/level overwrites).
- Added a pyOneNote
FileNode.__init__guard to survive the terminator-nodeAttributeError, plus new tests using theExporterEvalfixture.
Reviewed changes
Copilot reviewed 2 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
onenote_export/parser/one_store.py |
Fixes page construction to avoid title collapse, prefers first metadata revision per GUID, and monkey-patches pyOneNote to handle the terminator crash. |
tests/test_eval_notebook.py |
Adds regression tests using the ExporterEval fixture for duplicate titles and hierarchy/order parsing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+144
to
+158
| def _patched_filenode_init(self, file, document): | ||
| try: | ||
| _original_filenode_init(self, file, document) | ||
| except AttributeError: | ||
| # Only swallow the terminator case (id 0/255); anything else | ||
| # is a genuine parse error and must propagate. | ||
| header = getattr(self, "file_node_header", None) | ||
| fnid = getattr(header, "file_node_id", None) | ||
| if fnid in (0, 255): | ||
| if not hasattr(self, "data"): | ||
| self.data = None | ||
| if not hasattr(self, "children"): | ||
| self.children = [] | ||
| else: | ||
| raise |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three fixes in the
.onepage-building path (onenote_export/parser/one_store.py), validated against real notebooks and a small synthetic fixture (tests/test_data/ExporterEval, dummy content).1. Distinct pages that share a title are no longer dropped
_build_pages()builds one page per page-node GUID (correct), then applied a second dedup keyed on lowercased title — collapsing legitimately-distinct pages that share a name (e.g. repeated subpages, many "Notes" / "Talking Notes"). This silently discarded real content. Removed the title collapse; page identity is the page-node GUID only.2. Use the current page-metadata revision, not a stale one
A page's object space holds multiple
PageMetaDatarevisions. The GUID→meta lookup overwrote with whichever was processed last — a stale copy with an outdatedPageLeveland/or title (from before a page was renamed or promoted/demoted to a subpage). Now keeps the first (current) metadata per GUID. Fixes subpages reported as top-level and stale/renamed titles.3. Survive the pyOneNote FileNodeListFragment terminator crash
Large / heavily-revised sections raised
AttributeErrorfrom pyOneNote'sFileNode.__init__on a terminator node, aborting the parse. Added a guard in_patch_pyonenote(). This is a workaround for an upstream pyOneNote bug — a fix is proposed at DissectMalware/pyOneNote#28 — and can be removed once that releases.Tests
Adds
tests/test_eval_notebook.pyplus a small synthetic fixture notebook (ExporterEval, dummy content) asserting (1) distinct same-titled pages are retained and (2) subpage display order + levels 1–3.Note: this PR is the general decoder fixes only. Obsidian-specific output changes I made (numeric ordering, folder-notes, etc.) live in a separate downstream project and are intentionally not included here. Thanks for onenote-exporter!