feat: add optional memory frontmatter tools - #1761
Conversation
ed2eebc to
cfbce85
Compare
|
Thanks, @TyceHerrman, @MischaPanch and I will evaluate whether the spec that is implemented here still fits with what happened within the memory system in the meantime. |
Port and update the frontmatter work from oraios#1119 for the current memory manager architecture. Co-authored-by: Mehdi Ait Kajaoud <102723977+Meh10t@users.noreply.github.com>
cfbce85 to
d2c3b5c
Compare
AmirF194
left a comment
There was a problem hiding this comment.
load_memory/save_memory (memory_manager.py:223-243) run FrontmatterParser.parse() unconditionally on every read, not just when the new opt-in frontmatter tools are active. ReadMemoryTool.apply calls load_memory directly, so this changes what the existing read_memory tool returns for any pre-existing memory whose body happens to open with a line-exact ---, one or more key: value lines, and a closing --- (a plausible shape: a markdown horizontal rule followed by a short status block, e.g. after a heading).
Docker repro against this PR's head (d2c3b5c), no mocks, real MemoryManager:
original = (
"---\n"
"replicas: 3\n"
"image: myapp:latest\n"
"---\n"
"This is the actual body text a user wrote, describing a k8s manifest above.\n"
)
mgr._save_memory_raw("k8s-notes", original, is_tool_context=False)
loaded = mgr.load_memory("k8s-notes")Output:
loaded == 'This is the actual body text a user wrote, describing a k8s manifest above.\n'
loaded != original # True: the replicas/image lines are gone
read_memory silently drops that block for a memory nobody ever opted into frontmatter for. It happens to survive a load-then-save round trip here because render() reconstructs it, but that reconstruction isn't guaranteed to be byte-identical (render trims whitespace and never re-adds quotes, so a quoted value like url: "https://x.com:443" loses its quotes on the next round trip and can then mis-split at the extra colon).
Given the CHANGELOG frames this as "opt-in," would it make more sense to only parse frontmatter when at least one of the new tools is active, or to require an explicit marker so existing memory bodies that start with --- aren't reinterpreted?
|
Thanks for the concrete reproduction. This is addressed in
While addressing this feedback, I also took the opportunity for minimal alignment with Google's Open Knowledge Format v0.2 field names: new blocks require Focused validation is |
|
the failing ci checks on commit Multiple unrelated repositories reported the same action-resolution and runner-assignment failures during the same UTC window |
|
Thanks for the PR. @opcode81 and I still haven't decided whether we want to support this, as it brings serena's memories a bit away from "just standard md working with any memory", and the memory tool proliferation also carries a cost. Just a heads up on why there was no comment from us yet |
Summary
memory_get_frontmatterandmemory_add_frontmattertoolsread_memorybody-only and keep the defaultlist_memoriesresponse unchangedlist_memoriesonly when the get-frontmatter tool is activeCompatibility and format
Managed frontmatter has the following scalar-only shape:
serena_frontmatter_version: 1must be the first field, andtypemust be non-empty. The marker is reserved for Serena and is omitted from metadata tool and listing output. Newly created blocks default totype: "Serena Memory".Unmarked legacy content remains ordinary memory body content, including valid-looking, empty, and malformed leading
---blocks. Parsing is independent of active tools, so identical files read consistently in every tool context. Marked blocks are hidden fromread_memory; their original quotes, whitespace, field order, and newline style are retained during body-only writes. Field updates replace only the selected value or insert one line before the closing delimiter, rendering new values as reversible JSON-style quoted strings.Duplicate keys, malformed marked blocks, missing or empty
type, unsupported Serena frontmatter versions, and writes to the reserved marker fail before the file is modified.Minimal OKF alignment
While addressing the compatibility feedback, this also takes the opportunity for minimal field-level alignment with Google's Open Knowledge Format v0.2: Serena uses the useful field names
typeanddescriptionwhere applicable.This does not claim OKF bundle conformance. Serena does not use
okf_version, does not implement reservedindex.mdorlog.mdbehavior, and keeps structured OKF fields outside this scalar-only feature.Validation
171 passedacross the focused frontmatter, memory-manager, and memory-tool suitespoe lintpoe type-checkpoe doc-buildpython -m compileallon changed Python filesgit diff --checkThe new-head GitHub Actions runs for commit
85086235are final: 10 checks passed, 6 failed, 6 were cancelled, and 2 were skipped. Every non-green executable check failed before repository checkout: GitHub either did not acquire a hosted runner after multiple attempts or returnedService Unavailable/Failed to resolve action download infowhile resolving actions. No repository code or tests failed; notably, the JVM matrix passed on Linux, Windows, and macOS. Runs: Tests, Python Versions, CodeQL Advanced, Codespell, and Docs Build.The local macOS sandbox limitations remain recorded separately in #1817 and #1818.
Original implementation: #1119 by @Meh10t.