-
-
Notifications
You must be signed in to change notification settings - Fork 512
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs: suggestions architecture WIP #1515
base: main
Are you sure you want to change the base?
Changes from all commits
0f6b4d0
fad5175
934e4d8
4773355
212090a
f4a6b81
dc2147a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,197 @@ | ||
# Versioning & Suggestions | ||
|
||
Due to the similarity between document versioning, red lining, track changes and suggestions, it is worth seeing an overview of all of their requirements to make sure we consider solutions that resolve multiple of them. | ||
|
||
- A diff between two versions of a document, is essentially a set of suggestions that takes your previous document, and transforms it into the more recent document. | ||
- Red-lining and suggestions are the same, red-lining just allows you to attach metadata to the suggestion, like a comment of why the change was made. | ||
- Track changes, is entering a mode where any edit you make, becomes a suggestion to the document. | ||
|
||
## Versioning Overview | ||
|
||
Versioning has already been implemented for Y.js & y-prosemirror, but to be explicit about the sort of thing we are looking for it is good to explicitly state the feature set versioning should have: | ||
|
||
- A document can be _snapshotted_ at any point in time. | ||
- This is stored in a separate data store, in a Y.js document | ||
- This can be done either by: | ||
- A user manually choosing to save the document at a point in time (potentially with metadata like a version name, and a description of the changes made) | ||
- Automatically when changes are made to the document | ||
- Automatically on intervals (e.g. every 5 minutes or 100 changes) | ||
- These snapshots can be viewed independently at any point in time later | ||
- These snapshots can be _compared_ with each other to see what changes were made between them | ||
- Each change, should show: | ||
- The content that was changed (i.e. additions or deletions or updates) | ||
- Updates to content (i.e. changing an attribute) | ||
- Which user identifiers modified the content | ||
- Ideally, a timestamp of when the content was modified | ||
- Changes should only show content that was present in both documents (i.e. if some content was added and subsequently deleted in some version between the two snapshots, because it is not present in the final document of either snapshot, it should not show) | ||
- These changes can be visualized in a number of ways: as a unified diff, split diff, or even stacked diff. _See [Appendix](#displaying-changes) for more information_ | ||
|
||
**Prior Art:** | ||
|
||
- Google Docs | ||
- [Upwelling](https://www.inkandswitch.com/upwelling/) | ||
- [Patchwork](https://www.inkandswitch.com/patchwork/notebook/) | ||
|
||
### Suggestions Overview | ||
|
||
Suggestions are the ability to make changes to a document and have those changes tracked separately from the document content. This allows modifications to the document in a way that can be audited, and either approved or rejected by another party. | ||
|
||
- A user can turn on a mode, suggestion mode, which makes all of their edits to a document be tracked, but stored separately from the document itself. | ||
- The original document should be unmodified when suggestion mode is turned back off. | ||
- If in suggestion mode, and the user has write access to the document, they should be able to make changes to the document without having to leave suggestion mode. Such as applying a suggestion to the document. | ||
- These stored changes, can be reviewed, and either approved or rejected separately. | ||
- Approving a suggestion should apply the change to the document, Rejecting should drop that suggestion | ||
- A suggestion may become "invalid" if the document that the change has been modified in a way that the change can no longer be applied (e.g. an insertion into a deleted paragraph, or a deletion of an already deleted paragraph) | ||
- A suggestion should store: | ||
- The content of the suggestion based on the type of suggestion | ||
- **insertions:** where to insert the change, the content to insert | ||
- **deletions:** where to delete the content, the content to be deleted | ||
- **updates:** where to update the content, what key to update, what value to set the key to | ||
- A timestamp of when the suggestion was proposed | ||
- Which user proposed the suggestion | ||
- An optional thread identifier, for discussion around why the change was made | ||
|
||
**Prior Art:** | ||
|
||
- Google Docs | ||
- [Manuscripts Track Changes](https://github.com/Atypon-OpenSource/manuscripts-track-changes-plugin), [prosemirror-suggest-changes](https://github.com/handlewithcarecollective/prosemirror-suggest-changes), [prosemirror-suggestion-mode](https://github.com/davefowler/prosemirror-suggestion-mode/) and [wax prosemirror tracked changes](https://gitlab.coko.foundation/wax/wax-prosemirror/-/blob/master/wax-prosemirror-core/src/utilities/track-changes/trackedTransaction.js?ref_type=heads) | ||
|
||
### Similarities | ||
|
||
Versioning and suggestions have quite a bit of overlap between them, they both rely on having a set of changes which store: | ||
|
||
- user: who made the change (the change's attribution) | ||
- timestamp: of when the change was made | ||
- content: what changed & it's location in the document | ||
|
||
Ideally, we could re-use some of the logic and data structures between them for both features, since they overlap in representation and visualization to users. | ||
|
||
## What we need to decide | ||
|
||
### How should Y.js snapshots be compared | ||
|
||
Y.js already has versioning in the form of snapshots of the Y.js document, at a point in time. In Y.js, the snapshots have the nice property of having some form of a history associated to those edits. To figure out what changed, you could calculate a diff between the two histories of the document. Doing a comparison based on the document content alone, would not meet our requirement of being able to display who made the change and when. Luckily, in Y.js, there is additional metadata that we can extract from Y.js which can tell us which `clientID` made the change (`clientID` being an ID a user assumes when making edits to a document). More on how to calculate that diff here: [Diff two Y.js Snapshots](./diff-yjs-snapshots.md) | ||
|
||
This would be the most straightforward way to capture changes across versions of a document. The question then becomes how to display those changes within a prosemirror-based editor, which sort of depends on the approach taken for suggestions. | ||
|
||
### How should suggestions be stored | ||
|
||
Suggestions are different from versioning in that, they are not a history of the document, but a set of pending changes that can be applied to the document. They may be rejected, or may be applied to the document. So, in a sense, they are a patch to the document. But, there are a few different mental models we can use to represent or store these changes to the document: | ||
|
||
- [Seeing a suggestion as a _branch of the document history_, and the suggestion as being the diff between the two branches](./suggestions-as-history.md) | ||
- [Storing document _patches_ and later applying that patch to a document](./suggestions-as-patches.md) | ||
- [Suggestions as _document content_ which stores the changes within the document](./suggestions-as-content.md) | ||
|
||
### AttributionManager | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Kevin will write more about the attribution manager, but to give a summary of how it will work:
|
||
|
||
Currently, Y.js has been using a `PermanentUserData` class to keep track and store the `clientID`s that a specific user has used previously. Kevin has some ideas on an improved system for this, which he can elaborate on in detail, later. This will be crucial to how we can attribute users in prior versions of the document. | ||
|
||
If I were to share my wish-list for features this should have, it would be able to answer questions like: | ||
|
||
- Given a `clientID`, what `userID` made this change? | ||
- At what time was this `clientID` first used? | ||
- This gives approximate times for attributing content to a time, without having to store an additional timestamp per change). | ||
- Most people don't need the specific time, they just want to know relative time "Is this days, months or years old?" | ||
- Auditing should be done at a different level, and not in a user writable data structure like a YDoc | ||
|
||
I'd like to have Kevin give more details on the sort of features/properties we can expect from this. It will be a crucial feature to the implementation, and likely to be worked on first since it will be foundational to how the versioning works. | ||
|
||
## Summary | ||
|
||
This document was purposefully written from the perspective that Y.js is one of many document storage and synchronization mechanisms. And, we would like to keep our editor de-coupled from these specific implementation details as much as possible to encourage code re-use between different implementations. | ||
|
||
## Appendix | ||
|
||
This is a collection of scattered thoughts on the _diff_-erent aspects of diffing (sorry, had to), and their complications. | ||
|
||
### Timestamps | ||
|
||
While a timestamp seems like a simple thing to add to a change, it is actually a surprisingly difficult problem. Is a timestamp: | ||
|
||
- The time the change was made? | ||
- From when they started typing? When they finished typing? | ||
- From when they started editing the document? | ||
- The time the change was accepted by the server? | ||
|
||
At what granularity should we be storing timestamps? Do we need to store them to millisecond precision? | ||
|
||
### Keeping user intent, while diffing | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. was thinking about another edge-case earlier today - writing it down here to share and so I don't forget :)
|
||
|
||
Diffing nested data structures (e.g. HTML) is fundamentally different than diffing by the semantics of a document. This, is probably best described in an example: | ||
|
||
Take the following document: | ||
|
||
```html | ||
<p>Some text</p> | ||
<p>More text</p> | ||
``` | ||
|
||
Let's say that the user deletes the range of text from "text" and collapses the two paragraphs into 1 paragraph, resulting in this document: | ||
|
||
```html | ||
<p>Some More text<p> | ||
``` | ||
|
||
If we examine the changes, this is actually multiple operations: | ||
|
||
- deletion of the word "text" | ||
- insertion of the words "More text" at the position where "text" was | ||
- deletion of 2nd paragraph | ||
|
||
If we were to represent this change using HTML elements it was meant to look like this: | ||
|
||
```html | ||
<p>Some <del>text<p> | ||
<p></del>More text</p> | ||
``` | ||
|
||
Yet, a diff would represent it like this: | ||
|
||
```html | ||
<p>Some <del>text</del><ins>More text</ins></p> | ||
<del><p>More text</p></del> | ||
``` | ||
|
||
They ultimately result in the same in the final document but, the difference is in their semantics and user expectations. There are probably ways to simplify these changes in such a way that it is closer to what the user actually intended with the change (e.g. Prosemirror's [simplifyChanges](https://github.com/ProseMirror/prosemirror-changeset)), but it gets complicated rather quickly and we have to face that it may not always be easy to represent/display. | ||
|
||
Whatever the approach, we need to make sure that the changes are displayed in a way that is easy to understand and aligns with the user's intent. | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make clear: this is a tradeoff of a nested element approach, but we are purposefully not choosing to implement a flat YText. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider other limitations of the diff-based approach.
|
||
> [!NOTE] | ||
We are specifically not trying to represent the content as a flat YText document even though it may make this easier. In the interest of re-using the work on y-prosemirror as is. | ||
|
||
### Displaying changes | ||
|
||
There are actually a few different ways to display changes in a document. Above, I was describing changes in what is called a _unified diff_. where changes are inter-leaved into the document using inline content and markers for where the inserted and deleted content exists. But, that is just one way to display a diff, you can also display diffs using a _split diff_ like so: | ||
|
||
```txt | ||
❯ delta a b | ||
1/a ⟶ 2/b | ||
────────────────────────────────────────────────────── | ||
|
||
───┐ | ||
1: │ | ||
───┘ | ||
│ 1 │<p>Some text</p> │ │ | ||
│ 2 │<p>More text</p> │ │ | ||
│ │ │ 1 │<p>Some More text</p> | ||
``` | ||
|
||
And, there is _stacked diffs_, too! | ||
|
||
```txt | ||
❯ diff a b | ||
1,2c1 | ||
< <p>Some text</p> | ||
< <p>More text</p> | ||
\ No newline at end of file | ||
--- | ||
> <p>Some More text</p> | ||
\ No newline at end of file | ||
``` | ||
|
||
The nice thing about these different ways of displaying diffs, is that they are better at representing certain kinds of changes (especially when they are colorized unlike these examples). For example, for block-level changes, the unified diff seems to capture this change the best, it becomes clear that two lines became one. | ||
|
||
So, there are multiple ways to display diffs, and they can be advantageous in different situations. Ideally, our solution would have flexibility in displaying changes in any of these ways. | ||
|
||
> [!NOTE] | ||
As a matter of practicality, we are also looking into using these different ways of displaying diffs for things like displaying LLM responses, like if an LLM were to rewrite some content, it could display in a split diff view to show the changes the LLM actually made. So, this is not just a theoretical want, but something we would want to build this year. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
|
||
# Diff two Y.js snapshots | ||
|
||
Y.js keeps track of metadata around what changes have been made to a document in order to efficiently synchronize changes. So, to compare a document with it's previous version, you can leverage this metadata by taking a snapshot of the full history of a document, and compare the insertions and deletions that have been made between the two snapshots. Because they share history, you can figure out what has been modified from _this point_ in history compared to _that point_ in history. For clarity, let's call this a `y-diff` for now to make sure it is different than a full content diff. | ||
|
||
nperez0111 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
## Representing these changes in the editor | ||
|
||
I won't go into detail about how to implement the `y-diff` between two documents, given that it is partially implemented in y-prosemirror, but will instead focus on how those changes end up being represented within the editor. | ||
|
||
We have a few different options: | ||
|
||
### Changes as _attributes_ | ||
|
||
Kevin has already implemented a preliminary version of the `y-diff` between two documents. Specifically, by using `ychange` attributes to add metadata onto elements to indicate whether they were modified between the two versions. This approach works for simple documents at the moment, with some limitations around: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel that this is basically what you are looking for. You could compute a y-diff based on the ychange attributes. That might make sense, so you can compute a more optimized diff (e.g. filtering out non-edits like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check whether tracked changes within a document also is limited to this or not |
||
|
||
- Certain features are missing (due to this being preliminary work) like | ||
- XYZ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would be super interested in feedback here. I remember that the approach - albeit buggy in certain scenarios - is solid. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ychange does not:
I agree, that it is not a fundamental problem with the approach, which is why I tried to frame it as missing features |
||
- Controlling the display of the diff: | ||
- y-prosemirror is controlling how the diffs are rendered so doing something like a stacked diff would not be possible | ||
- Trying to simplify changes to a more human understandable version as described in the HTML diffing section would not be possible, since y-prosemirror directly writes the changes it finds to elements | ||
- Inability to edit content while displaying diffs | ||
nperez0111 marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not an inherent limitation |
||
- For something like suggestions to work in the future, there needs to be a way to modify the document & still present the suggestions that have been made | ||
|
||
### Changes as _marks_ | ||
|
||
Another approach, would be to model what is currently being represented as attributes, but instead use marks similar to [`@handlewithcare/prosemirror-suggest-changes`'s schema](https://github.com/handlewithcarecollective/prosemirror-suggest-changes?tab=readme-ov-file#schema) where the content changes are represented as marks. This would allow flexibility in display (because of support for [markviews](https://prosemirror.net/docs/ref/#view.MarkView) which can decouple content from presentation). | ||
|
||
Though, I question whether it _should_ be the responsibility of `y-prosemirror` to be both finding the difference and modifying the content of the document to display those differences. It feels like `y-prosemirror` should be scoped to the integration of yjs & prosemirror, and this is somewhat out of scope for it. I would be okay with `y-prosemirror` providing utilities to find the `y-diff` between documents in the y-prosemirror format, but the responsibility of how that integrates with prosemirror should be the responsibility of the caller of that function, rather than, built into the `y-sync` plugin. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this paragraph is key There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How would this diff be represented? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good question. I think the Delta format is a good starting point, but would need additional metadata. Or, we can go with something more specific to Prosemirror since this would be implemented for We can talk about it 😄 |
||
|
||
### Changes as a _list of diffs_ | ||
|
||
One approach for this would be for y-prosemirror to provide the `y-diff` between two documents as a list of modifications, which can then be interpreted by another program (at the prosemirror level) to choose how to display, or render, those sets of changes. This de-couples the responsibility of showing the changes in the editor, from the responsibility of viewing, accepting or rejecting those changes. | ||
|
||
This would mean that the YDoc content would need to be serialized into a format that can be used by the caller to | ||
|
||
- Display the changes in the editor | ||
- Apply/reject a change to the document | ||
- Display those changes outside of the editor (e.g. in a side panel) | ||
|
||
This would be a more complex approach, but would allow for more flexibility in how the changes are displayed within the editor, and how those diffs can be used in the future. | ||
|
||
## To be determined | ||
|
||
- How do we represent a diff in the YDoc? | ||
- At what level should we represent this diff? Yjs should probably have a low-level API for this to compare two documents. But, what should that look like at the Prosemirror level? There is already the Delta format, but would this have the metadata we need to fully represent the changes? |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
# Storing changes as _content_ within the document | ||
|
||
Another way to represent changes to content is actually to embed markers of where the change was made directly into the content. This is actually how Microsoft Office implements their [Tracked Changes feature](https://www.oxygenxml.com/doc/versions/27.0/ug-editor/topics/author-managing-changes.html), which should give you an idea of the viability of such an approach. By storing the changes directly in the document, you have flexibility on whether to display the changes, and accept or reject them, since all states are present already in the document content. | ||
|
||
## Implementation details | ||
|
||
This would introduce 3 new marks for content in the editor, `insertion`s `deletion`s and `modification`s. When entering suggestion mode: | ||
|
||
- a change which would normally result in a deletion is instead marking that content with the `deletion` mark | ||
- an insertion would both insert the new content, but also wrap it with a `insertion` mark | ||
- any change to attributes would mark the content with a `modification` | ||
There will be additional metadata that could be tracked as attributes to these marks too, such as timestamp, or even a reference to a comment. | ||
|
||
## Pros | ||
|
||
- [There is prior art](https://github.com/handlewithcarecollective/prosemirror-suggest-changes) that we can leverage (and [this one](https://github.com/Atypon-OpenSource/manuscripts-track-changes-plugin) and [this one](https://github.com/davefowler/prosemirror-suggestion-mode/) and [this one](https://gitlab.coko.foundation/wax/wax-prosemirror/-/blob/master/wax-prosemirror-core/src/utilities/track-changes/trackedTransaction.js?ref_type=heads)) | ||
- Importing from something like DOCX, would be a 1-1 mapping as you can just map the DOCX insertions and deletions to our equivalents | ||
|
||
## Cons | ||
|
||
The main tradeoff of this approach is around permissions (e.g. a suggestion-only user), at least without resorting to some server-side inspection of content changes (like comments). Theoretically, to get around this, you could use cryptography to sign the content of these changes to ensure that they have not been tampered with across editors, but a suggestion user would still have enough permissions to write into the document when they should not be able to. | ||
|
||
- Adds complexity to displaying content in the editor, may need to rely on things like decorations to hide content we do not want to display | ||
- Since it is not a separate document, the permissions have to be the same as having write permissions to the document | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The main trade-off of this approach, is around having a suggestions-only user (without having to resort to some server undoing logic, inspecting the changes). Theoretically, to get around this, you could use cryptography to sign the content of these changes to ensure that they have not been tampered with across editors, but a suggestion user would still have enough permissions to write into the document when they should not be able to. Perhaps you can offload this to the server so that it is the only entity able to write these marks into the document. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Flesh out timestamp means and at what granularity (typed first keystroke, or when snapshot was taken, or when user assumed a clientID)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be derived from snapshotted version?