Skip to content

Stop the digest painting kept text as deleted when a heading is deleted or inserted #450

Description

@HMarzban

Summary

A section owns every block up to the next heading. When a heading is deleted, or a new heading is inserted inside a section, body blocks change owner. The digest diffs each section on its own. So the same text paints red and struck under one heading, and green under another. The text was never deleted, but the digest reports a deletion.

Land after #448. It adds status to the payload, the removed-heading paint and the "Edited" label this fix relies on. It also edits runsAround and wholeSection in diffSections.ts, so find each line below by its symbol.

Where

  • apps/hocuspocus.server/src/modules/document-changes/domain/segmentSections.ts:26-57: a section owns the nodes up to the next heading of any level.
  • apps/hocuspocus.server/src/modules/document-changes/domain/pairSections.ts:40-50: sections pair by toc-id. Lines 79-99 emit each unpaired baseline section as removed, right after the last paired head section. movedTocIds (110) is the precedent for a pass over all pairs.
  • apps/hocuspocus.server/src/modules/document-changes/domain/diffSections.ts: wholeSection (123-140) paints a removed body red and an added body green. quantify (148-159) diffs a paired section alone, so blocks it gained or lost read as added or removed. diffSections (193-197) takes moved as an optional argument.
  • apps/hocuspocus.server/src/modules/document-changes/domain/computeDocumentChanges.ts:176-183: runs pairSections, then diffSections, one pair at a time.
  • apps/hocuspocus.server/src/modules/document-versions/domain/matchBlocks.ts: matchBlocks (trim, LCS, zip). diffBlocks.ts:35-39 builds a block key from canonicalizeBlock.
  • packages/email-templates/src/digestWalk.ts:11-21: passageRuns paints excerpt and removed when runs is empty. So stripping runs alone would repaint the moved body.
  • apps/hocuspocus.server/src/lib/email/digestContentChanges.ts:93: a new section with empty heading text is dropped today. Show red, green or a label on every changed heading in the digest email #448 keeps it as "Untitled heading".

What happens

All cases are read from code, not measured.

  • A heading line is deleted and its body stays. The body moves into the previous section P. P is modified, and its passage shows that body green. The deleted section is removed, and its passage shows the same body red and struck. The reader thinks the text was deleted and rewritten.
  • A heading is turned into a paragraph. The same as above. The old heading words also show green under P.
  • A new heading is inserted inside section A. The tail of A moves under it. A shows the tail red and struck. The new section shows the same tail green.
  • The new heading has no text yet. Today the new row is dropped at line 93. The reader sees only red: text struck out that still exists.

A heading level change does not move blocks, because a section ends at the next heading of any level. A cut-and-paste move pairs as unchanged, because the canonical hash drops toc-id. Neither case belongs here.

Expected

Fix

This adds one function and no new field. Status, magnitude, the section counts and changed do not change.

  • Add one function, reparentedBlocks(pairs): ReadonlySet<Record<string, unknown>>, in pairSections.ts beside movedTocIds. It returns the re-parented body nodes themselves, by object identity. Call it in computeDocumentChanges.ts next to movedTocIds. Pass the set to diffSections as a fourth optional argument, as moved is passed. Add no field to Section or SectionPair.
  • The rule. Split pairs into runs. A run starts at each pair that has both sides, and holds the added and removed pairs after it. A leading run may have no paired section.
    • Skip a run with no added or removed pair.
    • For every other run, list the body nodes (section.nodes, no heading) of its baseline sides in order. Do the same for its head sides.
    • Make one matchBlocks call over the two lists. Key each node as diffBlocks.ts does: { hash: JSON.stringify(canonicalizeBlock(node)), nodeType: node.type }.
    • A re-parented block is an unchanged match whose baseline node and head node belong to different pairs. Put both node objects in the set.
  • This one rule covers a merge, a split, a heading turned into a paragraph, and two deleted headings in a row. It also covers a deleted first heading whose body becomes the preamble. A coarse result keeps only prefix and suffix matches, so the pass strips less and never strips wrongly.
  • In diffSections. Status and magnitude still come from the original pair. Only a section that holds a re-parented block takes a new path.
    • Filter section.nodes by the set first, then build sectionDoc from { ...section, nodes: filtered }. The set holds the decoded JSON objects, and nodeFromJSON loses that identity.
    • In quantify, keep today's ChangeSet over the full sections for magnitude. Build a second ChangeSet over the two filtered docs, heading kept. Take runs, excerpt and removedExcerpt from it.
    • In wholeSection, keep magnitude over sectionNodes(section). Build runs and the excerpt from the filtered section. That is the body text Show red, green or a label on every changed heading in the digest email #448 builds with leafWord, minus the re-parented blocks.
  • Do not filter runs afterwards. Runs carry no block identity, so the ChangeSet positions would not line up.
  • Update apps/hocuspocus.server/API.md in two places: the runs row of the section field table (line 802), and the removed row of §Section status (line 820). State that runs, excerpt and removedExcerpt may leave out a block that only changed section because a heading was deleted or inserted. State that status and magnitude still count it.
  • Do not change the pairSections function, zippable or matchBlocks. The new function sits in the same file.

Out of scope

Acceptance criteria

  • Deleting a heading line and keeping its body: the body text is in no removed run and no removedExcerpt. It is in no added run under the previous section.
  • Deleting two headings in a row: the same holds for both bodies.
  • Turning a heading into a paragraph: the same holds for the body. The old heading words may still show green under the previous section, because that paragraph is a new block.
  • Inserting a heading inside a section: the text that moved under it is in no removed run and no removedExcerpt. It is in no added run under the new heading.
  • The removed section is still in the output with status removed.
  • A re-parented block that was also edited still shows removed in one section and added in the other.
  • In each case above, status, magnitude, changed and the summary counts equal what the same input gives without the new function.
  • A section whose only change was gaining or losing re-parented blocks has status: 'modified' and no runs, so the mail shows the Show red, green or a label on every changed heading in the digest email #448 "Edited" label.

Verify

  • apps/hocuspocus.server/src/modules/document-changes/__tests__/unit/diffSections.test.ts: make the changesOf helper (lines 10-14) call reparentedBlocks and pass its set to diffSections, as computeDocumentChanges.ts does. Add a merge (heading line deleted, body kept), two headings deleted in a row, a heading turned into a paragraph, and a split. Each must fail on today's main. Assert the same status and magnitude that today's main returns.
  • Add no case to computeDocumentChanges.test.ts. Its Prisma stub returns no snapshot bytes, so it cannot carry a document pair.
  • Run bun test src/modules/document-changes in apps/hocuspocus.server.

Open decisions

  1. Should the match stay inside one run (a paired section and the added and removed sections after it)? Recommended: yes. A section owns the blocks after its heading. So a deleted or inserted heading moves blocks only between the pairs of one run, never to the next paired section. A wider search risks false matches on repeated text.

No activity

Activity on this issue will appear here.

Activity

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