memory-router: drift.ts consumes the loader's frontmatter parse export - #99
Merged
Conversation
lint/drift.ts was the last consumer carrying its own FRONTMATTER_RE + parseYaml() call (PR #95 closed this duplicate class for consolidate/schema-metrics.ts). loader.ts gains an additive parseFrontmatterYaml() export that reuses the loader's own regex/parse step; drift.ts's scanMemories now sources parse results from it and keeps its own field-requirement checks (drift-specific policy the loader's hot-path validation does not perform). loadMemoriesFromDir and parseMemoryFileWithReason are untouched. Refs: b2b135a8-80a9-4dfb-b68c-dce7f47c567b Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HPmP28EwHconrJps6tvtw2
Fixes 5 reviewer findings on the frontmatter-regex dedup change: - CHANGELOG: correct the "last consumer" claim. lint/drift.ts is the last READ-path consumer of the frontmatter-regex-plus-parseYaml duplicate class; src/tag/applier.ts still carries a byte-identical copy on the write path (follow-up task will fold it in), and src/migrate/transform.ts carries a deliberately different variant. - Drop the dead `body` field from FrontmatterYamlResult and parseFrontmatterYaml's return value; drift.ts's only consumer never reads it. Updated the hand-copied type annotation in drift.ts to match. - Add direct unit tests for parseFrontmatterYaml's three outcomes (ok, no-delimiter, yaml-error with non-empty detail) in loader.test.ts. - Add a regression test pinning the deliberate loader-vs-drift validation divergence: a file with name+type but no description is accepted by parseMemoryFileWithReason and flagged invalid_frontmatter by the drift lint. - Replace em dashes introduced by this change (drift.ts, loader.ts, CHANGELOG) with commas/semicolons/parens; pre-existing em dashes elsewhere are left untouched. Refs: b2b135a8-80a9-4dfb-b68c-dce7f47c567b Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HPmP28EwHconrJps6tvtw2
LanNguyenSi
added a commit
that referenced
this pull request
Aug 16, 2026
… centralized (#107) * memory-router: applier.ts consumes the shared frontmatter parse export applier.ts was the last write-path consumer carrying its own byte-identical FRONTMATTER_RE + parseYaml() copy after lint/drift.ts moved onto loader.ts's parseFrontmatterYaml export (#99). Its planChange now sources delimiter-match-plus-YAML-parse from that shared export. parseFrontmatterYaml gains back an additive `body` field (the raw, unprocessed capture group 2 that #99's fix round dropped as dead code) and a new `error` field on its yaml-error outcome carrying the original caught exception, not just its stringified message. Both close a real fidelity gap: applier.ts's own body normalization differs from the read path's (.trim() vs strip-one-leading-newline), and its malformed-YAML handling used to be an uncaught throw that cli.ts's per-file try/catch reports under "errored"; a naive re-wrap (`new Error(detail)`) would have silently changed the reported `${String(err)}` from "YAMLParseError: ..." to "Error: ...". parseMemoryFileWithReason now delegates to parseFrontmatterYaml instead of carrying its own duplicate parse step. loadMemoriesFromDir stays byte-for-byte unchanged, reference-diffed against origin/master across the existing loader fixtures plus a dedicated edge-case corpus. New tests pin the no-delimiter skip path, the malformed-YAML rethrow fidelity, the body-normalization edge case, and parseFrontmatterYaml's new body/error fields directly. Refs: e5428d12-f157-480f-8b04-749117aa99c7 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HPmP28EwHconrJps6tvtw2 * memory-router: fix CRLF mutation-blind tests, centralize FrontmatterYamlResult Fix round on applier.ts's frontmatter parse dedup, per reviewer findings: - Harden the CRLF applier test and add a CRLF unit test for parseFrontmatterYaml directly: the prior CRLF test asserted only `change.eol`, which is computed independently of the frontmatter regex, so a CRLF-blind FRONTMATTER_RE mutant fell through to the no-delimiter skip path and left the whole suite green. Now assert change.skipped and the merged topics field, plus a rendered-output check. - Pin the exact key set on parseFrontmatterYaml's yaml-error branch. - Move the FrontmatterYamlResult type out of loader.ts's local scope into the shared ambient types file (types.d.ts), alongside the existing MemoryScanEntry precedent, and reference it by name from both loader.ts's function signature and drift.ts's require()d import annotation. drift.ts previously hand-copied a structural type that was already stale (missing body/error) and offered no real protection: a require() call resolves to `any`, so the annotation was never checked against loader.ts's real export, and a field rename could silently degrade drift's error reporting to "parse error: undefined". Add a drift-side test asserting the parse-error detail carries the YAML parser's own message, not the literal string "undefined". - Drop the repeated "see loader.ts's parseFrontmatterYaml comment" cross-reference in applier.ts's import comment block. - Split the CHANGELOG's single long paragraph on this dedup into a top-level bullet plus short sub-bullets for the body and error fields. Refs: e5428d12-f157-480f-8b04-749117aa99c7 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HPmP28EwHconrJps6tvtw2 --------- Co-authored-by: Lan Nguyen Si <contact@lan-nguyen-si.de> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
LanNguyenSi
added
review:tests-pass
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
1 participant
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.
Summary
src/lint/drift.tsno longer carries its own frontmatter regex + parseYaml copy; it consumes the new additive loader exportparseFrontmatterYaml(sharedFRONTMATTER_RE). Drift's field-requirement policy stays local by design: the loader accepts files withoutdescriptionthat drift must keep flagging, and the byte-identical-report AC forbids changing that (divergence now pinned by a regression test).loadMemoriesFromDir,parseMemoryFileWithReason,loadMemoriesFromDirWithRejects,parseMemoryFileare byte-identical to master (verified by function extraction + diff, twice).src/tag/applier.tsstill carries a write-path copy, follow-up to be filed), deadbodyfield dropped from the new export, direct unit tests for the export's three outcomes, em dashes removed from new prose.Verification
parseFrontmatterYamlfails the new direct tests.Refs: b2b135a8-80a9-4dfb-b68c-dce7f47c567b