feat(dev-tools): operation history

[pavelglac]

No description provided.

[github-actions]

📊 Benchmark Analysis Report

✅ No significant performance changes detected

---

Threshold: 5% change

---

Updated: 2025-11-20T11:55:19.841Z

[vladar]

Amazing stuff. Going to be a game changer for debugging state issues! But before releasing it, we need a couple of tweaks in the core ForestRun code.

[vladar]

This could be a generic utility called CircularBuffer. The idea is that jsutils folder contains lower-level things without ForestRun-specific semantics, i.e. applicable in any JS project.

When changing to CircularBuffer, HistoryEntry should become a generic TItem on a class of course

Also make it iterable for convenient iteration and Array.from() compatibility.

Suggested change

	export class HistoryArray {
	export class HistoryArray implements Iterable<HistoryEntry> {
	[Symbol.iterator]() {
	// implementation
	}

[pavelglac]

Have change it to Circular buffer: https://github.com/microsoft/graphitation/pull/598/files#diff-38bfd45bc1835790dfc54f648d8091638169536f59ddb5e000c0a95a75aa0c15

[vladar]

shouldPushOptimisticHistory should return false when history is disabled, which is not the case now. Today this will always add history entry, even if it is disabled, which is not what we want, right?

Oh I see now, history.push ignores undefined values and createOptimisticHistoryEntry returns undefined when history is disabled. This is somewhat counter-intuitive when reading this code. Let's please change it:

• make history.push() accept HistoryEntry (do not accept undefined)
• make shouldPushOptimisticHistory return false when the flag is off.

[pavelglac]

Change the logic the handling the condition in caller.

[vladar]

As a sanity check I suggest you to run the full test suite with history enabled before merging (manually). It may uncover some hidden quirks.

Ideally, also run with other flags enabled/disabled, especially optimizeFragmentReads.

[vladar]

So context.changes are now captured only when history is enabled:

We should revert this. Changes should be always captured. I think it may even break existing code when flag optimizeFragmentReads is enabled.

Regardless, changes should be always captured (if there were dirty fields).

[pavelglac]

Change the logic and pushing always changes. I have change the structure of those changes and since they are not used anywhere it should be safe.

[vladar]

I don't see in the actual code that the key could be CompositeListChunk. Am I missing something?

[pavelglac]

Changed to

export type ChangedChunksMap = Map<
  CompositeListChunk,
  CompositeListDifferenceEntry
> &
  Map<ObjectChunk, (FillerEntry | ReplacementEntry)[]>;

But it is quite tricky to iterate this intersection type. I did not find better way than typecast it when iteration. So there is also a tuple but it is not directly connected to the map.

export type ChangedChunksTuple =
 | [CompositeListChunk, CompositeListDifferenceEntry]
 | [ObjectChunk, (FillerEntry | ReplacementEntry)[]];

[vladar]

Same, we should always capture actual changes, even if history is disabled (we just don't add changes to history if its disabled). Maybe childChanges is a replacement you meant. But then please double-check that this context.changes is not used anywhere, especially in the context of optimizeFragmentReads flag.

[pavelglac]

Changed, now I am capturing always changes