Skip to content

Duplicate keys in one block keep the last value, although YamlSerializer's code and comment say the first occurrence is preserved #154

Open
Open
Duplicate keys in one block keep the last value, although YamlSerializer's code and comment say the first occurrence is preserved#154
Labels
bugSomething isn't working

Description

@matt-edmondson
matt-edmondson
opened on Sep 27, 2026

What's wrong

Frontmatter/YamlSerializer.cs (~lines 74–82) says:

// Convert dictionary keys to strings and preserve the first occurrence of duplicate keys
foreach (KeyValuePair<object, object> pair in rawData)
{
    ...
    // Only add the key if it doesn't already exist
    if (!result.ContainsKey(key))

By the time this loop runs, YamlDotNet's Deserialize<Dictionary<object, object>> has already collapsed duplicates and kept the last value. The ContainsKey guard never sees a duplicate, so the "first occurrence" logic is dead code.

Repro

Frontmatter.ExtractFrontmatter("---\ntitle: First\ntitle: Second\n---\nbody\n");
// title = "Second"

Why it matters

The actual behaviour (last wins within a block) contradicts both this code's stated intent and the first-block-wins rule CombineFrontmatterObjects applies across blocks. A document that accidentally repeats a key (common after hand merges) gets the opposite value from the one the library says it keeps, and Combine then persists that choice.

Suggested fix

Pick one behaviour and make the code match it:

  • First wins (matches the comment and the cross-block rule): parse with the representation model (YamlStream / YamlMappingNode) and skip keys already seen.
  • Reject duplicates: build the deserializer with .WithDuplicateKeyChecking(), treat the block as unparseable, and fix the comment.

Acceptance criteria

  • The repro yields the documented value (First, or a parse failure if you choose rejection).
  • A test pins the behaviour.

Activity

  1. matt-edmondson
    added
    bugSomething isn't working
    on Sep 27, 2026

  2. matt-edmondson commented on Sep 27, 2026

    @matt-edmondson
    matt-edmondson
    on Sep 27, 2026 – with Claude
    ContributorAuthor

    Triage

    • Category: Bug
    • Priority: Low. Only documents that repeat a key inside one block are affected. The value is deterministic (last wins); it just contradicts the comment and the cross-block first-wins rule. The ContainsKey guard is dead code.
    • Area / suggested assignee: YamlSerializer.TryParseYamlObject (YamlSerializer.cs, ~lines 74–82). Owner: @matt-edmondson
    • Duplicates / in progress: not a duplicate. Open PR Treat a frontmatter block with no YAML content as unreadable instead of throwing #149 edits the same method (a null mapping for empty YAML) but does not change duplicate handling, so rebase on it.
    • Next step: this needs an owner decision first: first-wins (parse with YamlStream and skip keys already seen, which matches CombineFrontmatterObjects) or reject (WithDuplicateKeyChecking()). First-wins keeps within-block and cross-block behaviour consistent and is the smaller behavioural surprise. Then fix the comment and add a pinning test.

    Generated by Claude Code

  3. matt-edmondson commented on Sep 28, 2026

    @matt-edmondson
    matt-edmondson
    on Sep 28, 2026 – with Claude
    ContributorAuthor

    Decision (maintainer, 2026-09-28)

    • First wins. Parse with the YAML representation model (YamlStream / YamlMappingNode) and skip keys already seen. Within-block behaviour then matches the existing comment and CombineFrontmatterObjects' cross-block first-wins rule.
    • Duplicates are not rejected with WithDuplicateKeyChecking().

    Next reader: implement first-wins in YamlSerializer.TryParseYamlObject and add a test pinning that title: First / title: Second gives First. Rebase on open PR #149, which edits the same method.

    Generated by Claude Code

  4. matt-edmondson
    added a parent issue on Sep 30, 2026

  5. matt-edmondson commented on Sep 30, 2026

    @matt-edmondson
    matt-edmondson
    on Sep 30, 2026 – with Claude
    ContributorAuthor

    Covered by #180, which redesigns this bug cluster as a whole. Implement it through that issue rather than individually.

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions