fix(core): preserve malformed frontmatter during sync - #1188
Merged
Conversation
Signed-off-by: phernandez <paul@basicmachines.co>
14 tasks
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.
Why
Fixes #1171. Supersedes #1173.
When sync adds or updates a permalink,
FileService.update_frontmatter_with_result()currentlytreats an unparseable fenced YAML block as ordinary Markdown and prepends a new frontmatter block.
That creates the double-frontmatter state reported in #1171.
#1173 found this writer path, but its proposed repair strips the malformed block and writes only
the requested fields. A permalink-only sync can therefore delete the note's title, description,
tags, timestamps, and other user metadata. This replacement keeps the root-cause fix while
preserving Markdown as the authoritative user-owned representation.
What Changed
or search state from a partial rewrite.
review of fix(core): stop frontmatter merge from stacking a second block on parse failure #1173.
Implementation Details
FileServicenow parses an existing frontmatter block before removing or rewriting any bytes. AParseErrorbecomes a specificFileOperationError, and the existing outer error boundarypreserves that error instead of wrapping it a second time.
Valid frontmatter merges and frontmatter creation are unchanged. The deliberate tradeoff is that
a malformed note must be repaired by its owner before Basic Memory can normalize its permalink;
the system will not guess how to reconstruct invalid YAML or discard fields it cannot understand.
The batch-indexing regression exercises the real storage writer and database path. It proves that
the file is reported as an error, remains unchanged, and produces no entity projection.
Testing
Automated
BASIC_MEMORY_ENV=test LOGFIRE_IGNORE_NO_CONFIG=1 uv run pytest -q tests/services/test_file_service.py tests/indexing/test_batch_indexer.py --no-cov: 38 passed.just fast-check: Ruff fix/check, formatting, andty check src tests test-intpassed.git diff --check origin/main...HEAD: passed.BASIC_MEMORY_ENV=test LOGFIRE_IGNORE_NO_CONFIG=1 just fast-test: testmon selected 4,172 tests because its cache was cold/mismatched; the run was stopped manually after 1,488 passed, 25 skipped, and no failures. This interrupted run is not counted as a passing gate.Manual
title,description, andtags; a permalink-only update deleted all three fields. The regression nowasserts that the same update raises and preserves the original bytes.
Risks / Follow-ups
user repairs it. This is intentional: preserving the canonical Markdown is safer than silently
producing partial metadata.
name/descriptionfrom any parser that reads the first #1171 fixed by making this writer unable to emit a second block. If the reportedsymptom is reproduced through another write path, reopen the issue with that new evidence.