fix(core): stop frontmatter merge from stacking a second block on parse failure - #1173
fix(core): stop frontmatter merge from stacking a second block on parse failure#1173phernandez wants to merge 1 commit into
Conversation
…se failure
update_frontmatter_with_result only stripped the existing frontmatter
block on the success path of parsing it as YAML. When an existing block
had fences but failed YAML parsing (e.g. an unescaped special character
from an older basic-memory version), the except branch reset the parsed
fields to {} but left the original block in the content, so the merge
prepended a brand-new block ahead of the untouched original — silently
shadowing name/description/etc. behind the first block any frontmatter
parser reads (#1171).
Now the existing block is always stripped once fences are detected,
regardless of whether it parses, so a malformed block is replaced in
place instead of duplicated.
Co-authored-by: Paul Hernandez <60959+phernandez@users.noreply.github.com>
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dea6cbd2c1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # existing one — that silently shadows every field in it (issue #1171). | ||
| # Outcome: always strip the detected block from the body so the new | ||
| # block replaces it in place instead of stacking on top of it. | ||
| content = file_utils.remove_frontmatter(content) |
There was a problem hiding this comment.
When a fenced block contains any YAML parse error, current_fm is reset to {} and this line removes the entire original block; the replacement therefore contains only updates. Callers such as batch permalink normalization and note moves commonly update only permalink, so merely syncing or moving such a note silently erases its title, tags, timestamps, description, and other user metadata. Fail the rewrite or preserve/repair the malformed block rather than writing a partial replacement.
AGENTS.md reference: AGENTS.md:L132-L133
Useful? React with 👍 / 👎.
1 participant
Fixes #1171.
Summary
FileService.update_frontmatter_with_resultonly stripped the existing frontmatter block when YAML parsing of it succeeded. If the block had valid fences but invalid YAML inside (e.g. an unescaped special character in an older-style note), the original block was left in place and a new merge-only block was prepended ahead of it, silently shadowingname/description/etc. from any first-block frontmatter parser.Test plan
uv run pytest tests/services/test_file_service.py tests/indexing/test_batch_indexer.py— all passruff check/ruff format --checkon changed files — cleanty checkon changed files — cleanGenerated with Claude Code