Skip to content

fix(offload): emit render-safe MMD metadata#218

Open
rocke2020 wants to merge 3 commits into
TencentCloud:mainfrom
rocke2020:fix/mmd-render-safe-metadata
Open

fix(offload): emit render-safe MMD metadata#218
rocke2020 wants to merge 3 commits into
TencentCloud:mainfrom
rocke2020:fix/mmd-render-safe-metadata

Conversation

@rocke2020

Copy link
Copy Markdown
Contributor

Summary

  • Change L2 Mermaid generation guidance to emit metadata as a Mermaid comment (%% mmd-meta: {...}) instead of an arbitrary directive (%%{...}%%).
  • Add shared MMD metadata parsing that supports the new render-safe comment format while preserving legacy %%{...}%% parsing.
  • Reuse the shared parser in active/history MMD injection paths.
  • Add regression coverage for both new and legacy metadata formats.

Test plan

  • mmdc renders an MMD file containing %% mmd-meta: {...} metadata
  • npx vitest run src/offload/mmd-meta.test.ts
  • npm test
  • npm run build

Signed-off-by: rocke.dong <qc_dong@126.com>
@rocke2020

Copy link
Copy Markdown
Contributor Author

currently 'mmdc -i 001-fix-unauthorized-access.mmd -o 001.svg' will raise error; after this fix, we can render safe mmd file from td agent memory.

@Maxwell-Code07

Copy link
Copy Markdown
Collaborator

Thanks for the PR! We'll take a look at the MMD metadata format change.

The render-safe `%% mmd-meta:` comment does not contain the literal `%%{`,
so isEmptyShellMmd's stale guard let a freshly L2-generated 3-line MMD
(metadata + flowchart TD + one node) be classified as an empty shell and
deleted on task transition. Add a format-aware hasMmdHeaderMeta() presence
check (covers both comment and legacy directive forms, regardless of which
fields are populated) and reuse it in the guard. Note the render-safe
benefit in the L2 prompt: the .mmd stays a real graph humans can render.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@rocke2020

rocke2020 commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up commit 4dfd2df: fixes one site the format switch missed.

isEmptyShellMmd in before-agent-start.ts guarded on the literal substring %%{. The new render-safe metadata line %% mmd-meta: {...} has a space after %%, so it does not contain %%{. A freshly L2-generated minimal MMD is exactly 3 non-empty lines (metadata + flowchart TD + one node), so it slipped past the guard and lines.length <= 3 classified it as an empty shell → deleteMmd() on task transition.

Fix: added a centralized hasMmdHeaderMeta() presence check (matches both the comment and legacy directive forms, regardless of which fields are populated — keying on taskGoal alone would under-protect empty-taskGoal / timestamp-only metadata) and reused it in the guard, consistent with this PR's parser-centralization. Added regression tests.

Benefit recap (also noted in the L2 prompt): the comment form keeps the .mmd a valid, renderable graph, so a human can open it and read the real flowchart — the old %%{...}%% directive broke rendering.

Per data-safety priority, never let the cleanup heuristic remove a
content-bearing graph. Extract a pure isEmptyShellMmdContent() predicate
that flags an MMD as a deletable shell only when it has no metadata header
(either format) AND no node status/summary/Timestamp marker, within the
tiny skeleton size. Any graph with real content is always preserved;
false positives can only ever keep a file, never unlink valid data.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@rocke2020

rocke2020 commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Heads up on a data-loss bug this change introduced (and how I fixed it).

While testing I noticed the format switch broke the empty-shell cleanup. That guard decides whether an MMD is just a throwaway skeleton, and if so it hard-deletes the file (deleteMmd is a plain unlink). The old check looked for %%{ to mean "this has metadata, it's a real graph, keep it". The new %% mmd-meta: line has a space after %%, so it no longer matched — and a freshly generated 3-line graph (metadata + flowchart TD + one node) would get deleted on the next task transition. Silent, irreversible.

Fixed in two steps:

  • hasMmdHeaderMeta() so we recognize metadata in both the new comment form and the old directive form, no matter which fields are filled in (4dfd2df).
  • then tightened the delete check itself so it only removes a file that's genuinely empty — no metadata and no status:/summary:/Timestamp: on any node (0254f7d).

Net effect: the cleanup can now only err on the safe side. Worst case it keeps a skeleton it could've removed; it can never delete a graph that has real content. deleteMmd only has this one caller, so there's no other path to worry about.

Added tests for both pieces and the build's green.

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.

2 participants