fix(model): replace inline fragment data on tool re-application

openspec/changes/archive/2026-07-17-replace-inline-fragment-data-on-reformat/.openspec.yaml

@@ -0,0 +1,2 @@
+schema: spec-driven
+created: 2026-07-17

openspec/changes/archive/2026-07-17-replace-inline-fragment-data-on-reformat/design.md

@@ -0,0 +1,43 @@
+## Context
+
+`FormattingInlineNode.format()` (`packages/model/src/entities/inline-fragments/FormattingInlineNode/index.ts:130`) short-circuits with `return []` whenever `tool === this.tool`, so re-applying a data-carrying inline tool never updates the fragment's `data`. `isEqual()` (line 193) and `mergeWith()` (line 213) ignore `data` as well, so `ParentNode.normalize()` can merge adjacent same-tool fragments that differ only in data. Every `format` op reaches the tree as plain `(tool: InlineToolName, start, end, data)` via `EditorJSModel.format → EditorDocument → BlockNode → ParentInlineNode → FormattingInlineNode`, and these ops are replayed for remote users and undo/redo — so the comparison must be deterministic and must not depend on non-serializable tool objects being threaded through the op.
+
+The `inline-link` tool (`packages/tools/inline-link/src/index.ts:133`) currently works around the model gap by sending `FormattingAction.None` on re-apply, which `SelectionManager` (`packages/core/src/components/SelectionManager.ts:193`) silently ignores (no `None` case) — so the new `href` never reaches the model.
+
+## Goals / Non-Goals
+
+**Goals:**
+- Re-applying the same tool with **different** data replaces the affected fragment's data; with **equal** data it stays a no-op (preserves the current optimization for dataless tools like bold).
+- Correct behavior when the re-applied range only partially covers an existing fragment (split, replace the middle segment).
+- Data equality defaults to a deep structural comparison.
+- `isEqual()` / `mergeWith()` become data-aware so normalization never merges distinct-data fragments.
+- `inline-link` re-apply sends `FormattingAction.Format` and updates the URL.
+
+**Non-Goals:**
+- A full `intersectType` engine (`Extend` / `LeaveBoth`) — only the replace-on-same-tool path is in scope.
+- Changing the collaborative op shape or threading functions through it.
+- Building the DI registry that lets a tool register a custom comparator — deferred (see Decision 3).
+
+## Decisions
+
+### Decision 1: Deep-equality comparison is the model's default
+
+Add a pure helper (e.g. `isSameInlineData(a?: InlineToolData, b?: InlineToolData): boolean`) performing deep structural equality, treating both-`undefined` as equal. `FormattingInlineNode.format()` uses it: when `tool === this.tool`, return `[]` (no-op) if data is equal, otherwise perform a data replacement. `isEqual()` and `mergeWith()` use the same helper so only truly-equal fragments merge.
+
+### Decision 2: Replace via split, reusing the existing `unformat` pattern
+
+For a same-tool re-apply with different data:
+- If `[start, end]` fully covers the node (`0..length`): replace the node's `data` in place.
+- If partial: split at `start` and `end` (mirroring `unformat`'s `split` logic) so the middle segment is isolated, and give that middle segment the new `data`.
+
+`data` is currently `readonly`; the implementation will introduce a controlled way to set it (internal setter or reconstruct-and-swap) rather than exposing it publicly.
+
+### Decision 3: Tool-overridable comparator — seam now, wiring deferred
+
+Because the op carries only a tool **name**, a custom comparator must be resolved by name from a model-side registry populated at tool-registration time, not passed through the op. That registry spans `sdk → core → model` and is larger than this change warrants.
+
+**This change:** add an optional `isSameData?(a, b): boolean` to the SDK `InlineTool` interface as a documented extension point, and centralize the model's comparison behind the single `isSameInlineData` helper so the override has an obvious future home. **Deferred:** the DI registry and core wiring that actually resolve a tool's `isSameData` — no built-in tool needs a custom comparator yet (link's `{ href }` is handled correctly by deep equality).
+
+### Decision 4: Remove the link workaround
+
+In `inline-link`, drop the `isActive ? FormattingAction.None : FormattingAction.Format` branch and always send `FormattingAction.Format` with `data: { href: linkInput.value }`. The stale `@todo Replace link…` comment is removed since the model now handles it.

openspec/changes/archive/2026-07-17-replace-inline-fragment-data-on-reformat/proposal.md

@@ -0,0 +1,25 @@
+## Why
+
+When a data-carrying inline tool (like **link**) is re-applied over a range that already has that same tool, the new data is silently dropped — editing a link's URL does nothing and the old `href` sticks. The root cause is in the model: `FormattingInlineNode.format()` returns early whenever the tool matches an already-applied tool, never comparing or replacing `data`. Because the model can't do this, the `inline-link` tool works around it by sending a no-op `FormattingAction.None` on re-apply, so the new value never even reaches the model.
+
+## What Changes
+
+- `FormattingInlineNode.format()` stops no-op'ing on same-tool re-apply. When the same tool is re-applied with **different data**, the fragment's data is replaced; when the data is unchanged, it stays a no-op (preserving the current optimization for tools like bold that carry no data).
+- Data equality defaults to a deep comparison, but an inline tool MAY provide its own comparator so it can decide what "same data" means for its fragments.
+- Node normalization becomes data-aware: `isEqual()` / `mergeWith()` account for `data`, so adjacent same-tool fragments with different data are not incorrectly treated as equal or merged together (which would silently resurrect stale data).
+- The `inline-link` tool stops sending the `FormattingAction.None` workaround and sends `FormattingAction.Format` with the new data on re-apply, now that the model handles replacement.
+
+## Capabilities
+
+### New Capabilities
+<!-- None; this refines existing behavior. -->
+
+### Modified Capabilities
+- `model`: the "Inline text tree" requirement gains data-aware re-formatting — re-applying the same data-carrying tool over a fragment replaces its data (deep-equality default, tool-overridable comparator) instead of being a no-op, and normalization no longer merges same-tool fragments whose data differs.
+- `tools`: the "Inline link tool" requirement changes so that confirming a URL on an already-linked selection re-applies the link with the new `href` via `FormattingAction.Format`, rather than the current `FormattingAction.None` no-op.
+
+## Impact
+
+- `packages/model/src/entities/inline-fragments/FormattingInlineNode/index.ts`: make `format()`, `isEqual()`, and `mergeWith()` data-aware.
+- `packages/model-types` / `packages/sdk` (`InlineTool`): expose the optional per-tool data comparator so tools can override deep equality.
+- `packages/tools/inline-link/src/index.ts`: remove the `FormattingAction.None` re-apply workaround; send `FormattingAction.Format` with the new data.

openspec/changes/archive/2026-07-17-replace-inline-fragment-data-on-reformat/specs/model/spec.md

@@ -0,0 +1,36 @@
+## MODIFIED Requirements
+
+### Requirement: Inline text tree
+The system SHALL provide `TextNode` (extending `ParentInlineNode`) as the inline-fragment tree root for a text data node, supporting text insert/remove, format/unformat over ranges, fragment listing, and index/range validation with auto-merging/normalization of children. When a data-carrying inline tool is re-applied over a range already formatted with that same tool, the system SHALL replace the affected fragment's data with the newly supplied data instead of ignoring it. Two fragments of the same tool SHALL be considered equal (for normalization/merging) only when their data is also equal; data equality SHALL default to a deep structural comparison, which an inline tool MAY override with its own comparator.
+
+#### Scenario: Inserting text into an empty tree
+- **GIVEN** a `TextNode` with no children
+- **WHEN** text is inserted
+- **THEN** a child inline text node is auto-created and a `TextAddedEvent` is emitted
+
+#### Scenario: Removing text with an out-of-range index
+- **GIVEN** a `TextNode` with a bounded length
+- **WHEN** `removeText` is called with indices outside that range
+- **THEN** it throws a range/index validation error
+
+#### Scenario: Overlapping formatting from the same or different tools
+- **GIVEN** a text range that already has formatting applied
+- **WHEN** `format` is applied again over an overlapping range (same tool or a different tool)
+- **THEN** the resulting fragments nest or merge correctly without duplicating formatting
+
+#### Scenario: Re-applying the same tool with different data
+- **GIVEN** a fragment already formatted with a data-carrying tool (e.g. a `link` with `{ href: "a" }`)
+- **WHEN** `format` is applied over that range with the same tool but different data (e.g. `{ href: "b" }`)
+- **THEN** the fragment's data is replaced with the new data (`{ href: "b" }`) rather than the call being a no-op
+
+#### Scenario: Re-applying the same tool with identical data
+- **GIVEN** a fragment already formatted with a data-carrying tool
+- **WHEN** `format` is applied over that range with the same tool and data that is equal under the effective comparator
+- **THEN** the tree is left unchanged (no redundant fragments or mutations)
+
+#### Scenario: Normalization keeps distinct-data fragments separate
+- **GIVEN** two adjacent fragments of the same tool whose data differs
+- **WHEN** the tree is normalized
+- **THEN** the fragments are NOT merged, so each retains its own data
+
+Implemented in `src/entities/inline-fragments/ParentInlineNode/index.ts`, `src/entities/inline-fragments/FormattingInlineNode/index.ts`, `src/entities/inline-fragments/TextNode/index.ts`, validated by `src/entities/inline-fragments/TextNode/*.spec.ts`, `src/entities/inline-fragments/FormattingInlineNode/*.spec.ts`, and `src/entities/inline-fragments/specs/InlineTree.integration.spec.ts`.

openspec/changes/archive/2026-07-17-replace-inline-fragment-data-on-reformat/specs/tools/spec.md

@@ -0,0 +1,31 @@
+## MODIFIED Requirements
+
+### Requirement: Inline link tool
+The system SHALL provide `LinkInlineTool`, implementing `InlineTool` with `name: 'link'` and `intersectType: IntersectType.Replace` (a new link replaces any overlapping link rather than merging), rendering an `<a>` wrapper and exposing a popover for entering/editing the URL. Confirming a URL SHALL always re-apply the link via `FormattingAction.Format`, so editing an existing link updates its `href` to the new value.
+
+#### Scenario: Editing an existing link
+- **GIVEN** the current selection is inside an existing link fragment
+- **WHEN** the toolbar renders `getToolbarConfig`
+- **THEN** the icon is `IconUnlink`, the popover pre-opens with the existing `href` value, and activating the icon calls `api.selection.applyInlineTool` with `FormattingAction.Unformat` to remove the link
+
+#### Scenario: Creating a new link
+- **GIVEN** the current selection is not inside an existing link
+- **WHEN** the toolbar renders `getToolbarConfig`
+- **THEN** the popover opens with an empty input, autofocused via a microtask
+
+#### Scenario: Confirming a link URL
+- **GIVEN** the popover's URL input has focus
+- **WHEN** the user presses Enter
+- **THEN** `api.selection.applyInlineTool` is called with `data: { href: <input value> }` and `FormattingAction.Format`, whether the tool was already active or not
+
+#### Scenario: Updating the URL of an existing link
+- **GIVEN** the current selection is inside an existing link with `href: "a"`
+- **WHEN** the user enters `"b"` in the popover input and presses Enter
+- **THEN** the link fragment's `href` is replaced with `"b"`
+
+#### Scenario: Wrapper href assignment
+- **GIVEN** `createWrapper(data)` is called
+- **WHEN** `data.href` is a string
+- **THEN** the produced `<a>` element's `href` attribute is set to that value; otherwise a bare `<a>` is produced
+
+Implemented in `packages/tools/inline-link/src/index.ts`.

openspec/changes/archive/2026-07-17-replace-inline-fragment-data-on-reformat/tasks.md

@@ -0,0 +1,20 @@
+## 1. Model: data-aware comparison
+
+- [x] 1.1 Add a pure `isSameInlineData(a?, b?)` deep-equality helper in the model's inline-fragments utils
+- [x] 1.2 Make `FormattingInlineNode.isEqual()` and `mergeWith()` use `isSameInlineData` so distinct-data fragments don't merge
+- [x] 1.3 Rewrite `FormattingInlineNode.format()`: same tool + equal data → no-op; same tool + different data → replace via split (reuse the `unformat` split pattern), full-cover → replace `data` in place
+- [x] 1.4 Introduce a controlled way to set the fragment's `data` (private `#data` field + public getter), keeping it non-public
+- [x] 1.5 Make `ParentInlineNode.getFragments()` normalization data-aware so the read/serialization path keeps distinct-data fragments separate
+
+## 2. SDK: comparator seam
+
+- [x] 2.1 Add optional `isSameData?(a, b): boolean` to the `InlineTool` interface with a doc comment marking it the deferred override point
+
+## 3. Tool: remove the link workaround
+
+- [x] 3.1 In `inline-link`, always send `FormattingAction.Format` on confirm; drop the `FormattingAction.None` branch and the stale `@todo`
+
+## 4. Verify
+
+- [x] 4.1 Add/extend tests for the spec scenarios: re-apply different data → replaced; re-apply equal data → no-op; partial-range replace splits correctly; normalization keeps distinct-data fragments separate (integration spec) plus a focused `isSameInlineData` unit spec
+- [x] 4.2 Run the model + tools test suites and typecheck; confirm green

openspec/changes/wire-inline-tool-data-comparator/.openspec.yaml

@@ -0,0 +1,2 @@
+schema: spec-driven
+created: 2026-07-17

openspec/changes/wire-inline-tool-data-comparator/proposal.md

@@ -0,0 +1,53 @@
+## Why
+
+The change `replace-inline-fragment-data-on-reformat` (archived `2026-07-17`) made the model
+data-aware when re-applying an inline tool, using a **deep-equality** default
+(`isSameInlineData`, `packages/model/src/utils/isSameInlineData.ts`). As part of that work it
+added an optional `isSameData?(a, b)` to the SDK `InlineTool` interface
+(`packages/sdk/src/entities/InlineTool.ts`) as a **documented seam only** — the model does not
+yet consult it. See that change's `design.md` Decision 3 for the rationale (the wiring spans
+sdk → core → model and no built-in tool needed custom equality yet).
+
+This is the deferred follow-up: actually let a tool override the model's default data equality.
+Do it when a tool genuinely needs equality that differs from deep structural comparison
+(e.g. a mention/comment tool that should treat two fragments as equal ignoring a derived or
+display-only field, or normalize a URL before comparing).
+
+## What Changes
+
+- Resolve a tool's `isSameData` comparator inside the model by **tool name**, defaulting to
+  `isSameInlineData` when a tool does not provide one.
+- The comparator must be registered/looked up at tool-registration time, **not** threaded
+  through the `format` op — the op is replayed for remote users / undo-redo and carries only a
+  tool name and plain data, so it must stay serializable and deterministic.
+- Consume the resolved comparator wherever the model currently calls `isSameInlineData`:
+  `FormattingInlineNode.format()` (no-op vs replace), `FormattingInlineNode.isEqual()` /
+  `mergeWith()`, and `ParentInlineNode.getFragments()` normalization.
+- Remove the `@todo`s left at the seam once wired (`InlineTool.isSameData`,
+  `isSameInlineData`).
+
+## Capabilities
+
+### New Capabilities
+<!-- None; refines existing behavior. -->
+
+### Modified Capabilities
+- `model`: the "Inline text tree" requirement's data-equality default (currently deep
+  structural comparison) becomes overridable per inline tool via a comparator resolved by tool
+  name.
+- `sdk`: the inline-tool contract's `isSameData` moves from a documented-but-unused seam to a
+  hook the model actually invokes.
+
+## Impact
+
+- `packages/model` — a comparator registry/resolver keyed by `InlineToolName`; the four call
+  sites listed above consult it instead of calling `isSameInlineData` directly.
+- `packages/core` — register each tool's `isSameData` into the model at tool-registration time
+  (near where tools/IoC are wired).
+- `packages/sdk` — `InlineTool.isSameData` doc comment updated (no longer "not yet consulted").
+
+## Notes
+
+This is a **stub** created to track deferred work — proposal only. Specs, design, and tasks
+are intentionally not written yet; generate them (e.g. `/opsx:continue
+wire-inline-tool-data-comparator` or `/opsx:ff`) when the work is picked up.