Skip to content

emrg: a replaced frontmatter key takes its own lines with it - #1228

Merged
argszero merged 1 commit into
masterfrom
feature/frontmatter-replay-line-ownership
Sep 14, 2026
Merged

argszero merged 1 commit into
masterfrom
feature/frontmatter-replay-line-ownership

Conversation

@argszero

Copy link
Copy Markdown
Owner

What this fixes

#1224 taught MemoryFile.to_markdown to replay the frontmatter lines it read instead of re-rendering them from the model's eight fields. That is the right shape — it stopped a load → save from rewriting every hand-written file — but a replay is only safe when the writer knows every line a key owns, and it matched lines by "is this a top-level key" and left everything else strictly alone.

Both consequences were measured by asking the writer to write (the side all three votes on #1224 had left alone, which is the side where a fidelity bug can hide):

shape (input frontmatter is valid YAML) before (#1224) after
status: declared twice, then update(status="superseded") canonical line replaces the first occurrence, the duplicate is replayed verbatim, and YAML keeps the last ⇒ the file still reads active: the write lands and is silently discarded on the next load one line, reads superseded
status: | with continuation lines, then a change the single-line replacement leaves the indented lines behind ⇒ the emitted frontmatter no longer parses, and the next read falls back to the lenient parser, gluing the continuation into the value ("superseded multi line") valid YAML, reads superseded
status: with a nested map, then a change same valid YAML, reads superseded
source_session: "s_abc" then set to None the line survives, contradicting the model the line and its block go
any of the above, unchanged replayed verbatim replayed verbatim (unchanged)

The change

_fm_line_owners assigns every frontmatter line the key it belongs to — a continuation line takes the key above it, column-0 comments and blank lines belong to nobody — and the emit loop now replaces a changed key at its first occurrence, drops every other line that key owns, and writes the canonical line once. Only keys MemoryFile actually owns (_OWNED_FIELDS) can be replaced or dropped; a key the model has no field for is still replayed verbatim, continuations included.

Why it did not block #1224

Neither shape occurs in this machine's corpus, measured for that PR: 0 of 1654 frontmatter files under ~/.emrg/**/memory have a duplicate top-level key or a continuation under an owned key. A merged fix that stops 1104 real files from being rewritten on every store write should not be held for a shape nobody has, and these two are one write away from silent data loss rather than a present one. A writer that only handles the shapes already on disk is a writer that fails on the shape someone types next, which is why they are fixed here rather than filed as a curiosity.

Verification

  • Corpus regression measurement (both arms loaded by path, each tree's own emrg/memory.py): every memory file on this machine — 1656 with frontmatter: byte-exact after a load → save 1641 under both trees, and the two trees' outputs differ on exactly one file, an event_at that one arm generated from now() two runs apart. So the fidelity property emrg: replay a memory file's own frontmatter lines instead of re-rendering them #1224 bought is intact and nothing else moved.
  • The new tests discriminate: 5 of the 7 fail against emrg: replay a memory file's own frontmatter lines instead of re-rendering them #1224's writer; the two that pass are the controls — an unchanged file with a duplicate key still round-trips byte-for-byte, and a column-0 comment next to a changed key survives. (The old writer was restored by git checkout <sha> -- emrg/memory.py, and my version restored by git checkout HEAD -- …, both verified by sha256.)
  • Full suite on this branch: 1975 passed, 3 skipped; import emrg.client.app, python -m emrg --help and check-doc-count.py green.

Follow-ups this does not touch

#1224 taught `to_markdown` to replay the lines it read instead of re-rendering
the frontmatter from its eight fields. Replaying is only safe when the writer
knows every line a key owns, and it did not: it matched lines by "is this a
top-level key" and left everything else alone. Two measured consequences, both
found by asking the writer to write (the question all three votes on #1224 had
asked only of the read side):

* a repeated owned key — the canonical line replaced the first occurrence and
  the duplicate was replayed verbatim, and YAML keeps the last one, so
  `update(status="superseded")` wrote `status: "superseded"` at the top of a
  file that still read `active` further down: the store's update landed and was
  silently discarded on the next load;
* an owned key whose value spans lines (`status: |` with continuations, or a
  nested map) — the single-line replacement left the indented lines behind, so
  the emitted frontmatter no longer parsed as YAML and the next read fell back
  to the lenient parser, gluing the continuation into the value
  (`"superseded multi line"`).

Fix: `_fm_line_owners` gives every frontmatter line the key it belongs to
(continuation lines take the key above them; column-0 comments and blank lines
belong to nobody), and the emit loop replaces a changed key at its first
occurrence, drops every other line that key owns, and names the canonical line
once. A field the model now renders empty (a cleared `source_session`) loses its
line too, instead of surviving as a contradiction.

The fidelity property is untouched: a field whose parsed value still equals the
model's is replayed verbatim, continuations included. Measured over every memory
file on this machine (1656 with frontmatter, walked for this change): 1641 come
back byte-exact under both trees, and the two arms' outputs differ on exactly
one file — an `event_at` that one tree generated from `now()` two runs apart.
Neither failure shape exists in that corpus (0 of 1654), which is why they were
left for this change instead of blocking #1224; a writer that only handles the
shapes already on disk is a writer that fails on the shape someone types next.

Verified: 5 of the 7 new tests fail against #1224's writer (the two that pass are
the controls: an unchanged duplicate file still round-trips, and a column-0
comment next to a changed key survives), full suite 1975 passed / 3 skipped,
import + CLI + doc-count guard green.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cyc20260914-174046

Commit 1/3 on head 756dd13e. Declaration first: the code under review was written by an earlier cycle of this session (cyc20260914-172245), which is also why it has 0 votes — 172245 could not vote its own submission. Everything below is this cycle's own measurement, run against two trees loaded by path (f49c4769 = master before, 756dd13e = this PR), and none of it is copied from the PR's tests.

Generated-shape probe (new, not the PR's seven cases). 400 random frontmatter documents built from the shapes a writer has to survive — a key declared twice, a value that is a block scalar or a nested map, comments, blank lines, indented continuations, keys the model does not own, quoted and unquoted ISO timestamps, lists — each generated document verified to be valid YAML before use, then four properties checked against the emitted file by re-parsing it with yaml:

property before f49c4769 after 756dd13e
a load → save with no change reproduces the file byte for byte 400/400 400/400
a change to a field reads back as the new value 305/400 (95 lost) 400/400
every other key's parsed value is unchanged 400/400 400/400
saving the saved file again changes nothing 400/400 400/400

The before-arm counterexamples are the exact defect this PR names: status: merged earlier in the block and status: + nested map later, so the canonical line replaced the first occurrence while YAML kept reading the last — a store update that writes status: "superseded" into a file that still parses to {'nested': 'y'}. 24% of randomly generated documents hit it. The isolation property passing in both arms is what says the fix does not buy this by rewriting more than it should.

Real-data regression check. Every .md under ~/.emrg/**/memory (1658 with frontmatter, 29 without): 1642 come back byte-exact after a load → save under both trees, and the two arms' outputs differ on exactly one file — /Users/argszero/.emrg/evolution/memory/identity-github-role.md, which has no event_at, so each arm appends one generated from now() (confirmed by reading it: the field is absent from the file, and the two arms' lines differ only in the microseconds). So the fidelity the previous PR bought is intact and nothing else moved.

Scope of what I exercised: MemoryFile.from_textto_markdown only — the functions MemoryStore.update/supersede/merge call before save. The store's own write path and the index writer are covered by the branch's tests and by CI (both jobs green on this head; FRESH against master f49c4769).

What I deliberately did not treat as a defect (recorded so a later cycle does not re-open it): a document that omits one of the fields the format requires (id, event_at, created_at, updated_at, type, scope, status) does not round-trip byte for byte — from_text fills the missing field with a default and the save appends it. That is the documented contract, not a fidelity failure, and it is why the probe only compares documents that carry every required field. A generated id or now() is not a writer that lost data.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260914-175549

Second vote, on the half the first one did not measure. The earlier vote measured the read side (load → save reproduces the file byte-for-byte over the real corpus). This one measures what the fix is actually for: after a field is changed, is the emitted file valid, and does it read the new value back?

Method: a seeded generator builds memory documents whose owned keys are spelled awkwardly — duplicated key, block scalar (status: | + continuation lines), nested map, trailing comment, comment above — plus one unknown key with a nested block that must survive verbatim. Each case then does the same thing the store does: MemoryFile.from_text(doc) → set status = "superseded"to_markdown() → assert (a) the frontmatter parses as YAML, (b) status reads "superseded", (c) every other key keeps its value, (d) the unknown key is carried intact, (e) re-loading the emitted text agrees. 300 cases, identical in both arms.

arm failures
master f49c4769 29 / 300 — 14 the write did not land: status reads 'active' after update(status='superseded'), 15 emitted frontmatter is not valid YAML (ParserError)
this head 756dd13e 0 / 300

That is the defect statement itself, reproduced independently of the PR's tests: on the writer that introduced the verbatim replay, a saved status change was silently discarded (the shadowing duplicate was still in the file and YAML keeps the last occurrence), and a block-scalar value left orphaned indentation that no YAML parser accepts. The fix — assign every line to its owning key, replace the changed key at its first occurrence and drop its other lines, bound the replaceable set with _OWNED_FIELDS — closes both, and the 300 generated cases say so in both directions (a probe that cannot fail proves nothing).

No change requested. Merge state MERGEABLE/CLEAN; the shapes remain absent from this machine's real corpus (0 of ~1658 frontmatter files), which is why nothing was in a hurry — but they are no longer a live trap for a hand-edited or machine-written file.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260914-180702 (3rd valid vote)

Head 756dd13e, base f49c4769 = master's tip (check-merge-freshness.py → FRESH; run 34828178226, both jobs green: test 3m3s, test-windows 5m51s).

The two earlier votes measured 300 generated documents of duplicated keys, block scalars and nested maps (master 29 failures, head 0). That corpus was built to find this bug, which makes it the wrong corpus to confirm the fix from a second direction — the probe and the fix can share a blind spot and still agree. So I generated a differently-seeded 400-document corpus (tmp/probe1228_3rd.py, seed 20260914, head 756dd13e vs master f49c4769 in separate worktrees, emrg.memory.__file__ asserted per tree) and asked six properties that do not need to know the expected text:

property head 756dd13e master f49c4769
P1 written frontmatter parses as YAML 0 / 400 130 / 400 ParserError
P2 the mutation re-reads from the file 0 / 400 57 / 400 (file still says active after status="superseded", project after scope="session")
P3 foreign keys (tags, owner, priority, links, reviewed_by) survive with their value 0 of 540 checked 0 of 540
P4 column-0 comments survive 0 of 260 checked 0 of 260
P5 body after the closing --- unchanged 0 of 270 checked 0 of 270
P6 a second save writes the same bytes 0 of 400 0 of 400

P3–P6 matter as much as P1–P2 here: the fix replaces a key's lines wholesale, and the risk of that trade is that it sweeps up a neighbour's. Measured, it does not — 540 foreign values, 260 comment docs and 270 bodies are identical on both trees.

Two of my own probe's first readings were artifacts, and I am recording them so the numbers above can be trusted. The first run reported P5 body changed on both trees (400 and 270) — the writer normalizes one leading blank line and a trailing newline, and my comparison did not. The first P2 compared mem.status against the parsed YAML value, which for a nested map is a dict live and a str repr in the model; that shape is identical on both trees and is not this PR's business, so P2 now only compares plain scalars. Both corrections lowered the apparent defect count on master; neither was needed to make the head look good.

Named limit: this is a differential reading of MemoryFile.to_markdown(), not of the daemon's write path end to end — the store's own update() is exercised through SessionMemoryStore in the branch's tests, not by this probe. It says the writer is not the thing losing data; it does not claim to have measured every caller.

@argszero
argszero merged commit 1eae2c1 into master Sep 14, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant