feat(tree): add persisted commit metadata by noencke · Pull Request #28064 · microsoft/FluidFramework

Noah Encke (noencke) · 2026-08-25T00:36:35Z

Description

Applications can now attach arbitrary, JSON-serializable metadata to the commit that a transaction produces, replicate it to collaborating clients, and persist it in the document.

Write it via the new customMetadata field on RunTransactionParamsAlpha:

view.runTransaction(
	() => {
		view.root.insertAtEnd("new item");
	},
	{ customMetadata: { author: "alice", intent: "add-item" } },
);

Read it back while walking the branch's history, via the new custom property on TreeBranchCommitMetadata:

for (
	let commit = view.branchHistory.getHead();
	commit !== undefined;
	commit = commit.getParent()
) {
	const metadata = commit.custom;
}

Because a commit may be produced by nested transactions, each of which may supply metadata, custom is the flattened combination of them all (outermost wins on conflicting keys). The structural view is available as commit.customTree, a CustomMetadataTree mirroring the transaction nesting — the same relationship labels.tree has to a change's label set.

The metadata lives directly on the commit — on GraphCommit.customMetadata in memory and inline on the commits in the EditManager summary. That is what makes its lifetime automatically match the commit's: once the commit is trimmed from the trunk, the metadata goes with it, so there is no separate index to populate, reconcile, or prune.

Both the op format and the summary format gain a v7 (MessageFormatVersion.v7 and EditManagerFormatVersion.v7), written only when minVersionForCollab is 3.0.0 or later (gated by FluidClientVersion.v3_0). That floor is a declaration rather than an enforcement mechanism: a client too old for v7 fails cleanly with an unsupported-version error when it reaches v7 data, so adopting this requires deploying v7-capable readers everywhere before raising the floor. The changeset spells out the rollout sequence.

A revert may now be performed inside a transaction provided it is that transaction's only change, which lets the revert be given its own metadata. Attempting any other change in such a transaction throws an error.

Reviewer Guidance

The review process is outlined on this wiki page.

The `GraphCommit.customMetadata` property is required, not optional

This is the load-bearing decision. Two places rebuild a commit from its parts rather than spreading it (mintCommit and rebaseBranch), and an optional property would let both silently drop the metadata. Declaring it required turns each into a compile error, so the type system enumerates every site that has to make a decision.

Where a rebuilt commit is the same logical commit as its source, the property is propagated. undefined is used only where a genuinely new commit is minted — the inverse commit produced by reverting, rollback commits, the synthetic root/trunk-base commits, and edits from the editor that a transaction has not yet annotated.

The metadata tree mirrors transaction nesting

Each transaction in a nested stack contributes a node to a CustomMetadataTree. The root node is the outermost transaction, and nested transactions add child nodes. Aborted nested transactions remove their node again, so they never contribute. The single commit produced by the stack carries the tree, from which the flattened custom view is derived.

Compatibility

The v7 codecs are only selected when minVersionForCollab >= 3.0.0, so nothing changes for existing clients by default. Evidence that this holds: regenerating the full snapshot corpus added new v3_0 directories and modified no existing snapshot file, meaning the v3/v4/v6 output is byte-for-byte unchanged.

If an application supplies metadata while configured below 3.0.0, the value is kept in memory for the local session but is neither replicated nor persisted. There is a test for this.

The version-specific typebox schemas exclude the customMetadata field for pre-v7 formats, so no pre-v7 encoder can write it. The two formats then differ in how strictly they validate, and deliberately so: the summary schemas were already additionalProperties: false, so a payload falsely claiming to be pre-v7 while carrying the field is rejected there. The op Message schema is left permissive, as it has always been — tightening the op envelope would risk rejecting ops over envelope properties unrelated to this feature, and belongs in its own PR — so such a payload is tolerated there instead. Both behaviors are covered by tests.

Breaking Changes

None. All new API surface is @alpha and additive.

Implements the design in packages/dds/tree/docs/wip/persisted-commit-metadata.md. Applications can attach arbitrary JSON-serializable metadata to the commit a transaction produces, via a new `persistedMetadata` field on `RunTransactionParamsAlpha`. The metadata is replicated to peers, persisted in the summary, and read back through `TreeBranchCommitMetadata.persistedMetadata` while walking the branch history. The metadata lives on the commit itself - on `GraphCommit` in memory and inline on commits in the `EditManager` summary - so it shares the commit's lifetime and needs no separate index to populate, reconcile, or prune. `GraphCommit`'s property is declared required so that every site which rebuilds a commit from its parts is a compile error rather than a silent drop. Both the op and summary formats gain a v7, written only when minVersionForCollab is 3.0.0 or later, so a document only ever contains metadata when every client that can open it understands and preserves it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-08-25T00:39:40Z

Hi! Thank you for opening this PR. Want me to review it?

Based on the diff (25468 lines, 114 files), I've queued these reviewers:

Correctness — logic errors, race conditions, lifecycle issues
Security — vulnerabilities, secret exposure, injection
API Compatibility — breaking changes, release tags, type design
Performance — algorithmic regressions, memory leaks
Testing — coverage gaps, hollow tests

How this works

Adjust the reviewer set by ticking/unticking boxes above. Reviewer toggles alone don't trigger anything.
Tick Start review below to dispatch the review fleet.
After review finishes, tick Start review again to request another run — it auto-resets after each dispatch.
This comment updates as new commits land; your reviewer selections are preserved.
Start review

- Drop persisted metadata for every trimmed commit, including the newest one which survives internally as the trunk base. Previously a client could keep reading metadata that had already left the document, either through the reachable trunk-base sentinel or through a TreeBranchCommitMetadata obtained before trimming. - Snapshot the metadata at the transaction boundary by round-tripping it through JSON. This gives the commit a private copy, so later mutation of the caller's object can no longer change an already-created commit, and it guarantees the value read back locally is exactly what peers and future summaries see rather than diverging on values with no JSON representation. Throws a UsageError for values that cannot be represented as a JSON object. - Validate that persisted metadata is an object rather than accepting any value, by replacing Type.Any() in JsonCompatibleReadOnlyObjectSchema. - Add golden op and summary format tests that lock the serialized 'persistedMetadata' key at v7 and assert its absence before v7, so a rename on both the encode and decode sides can no longer pass unnoticed. - Add tests for trimming through a retained metadata object, value snapshotting and normalization, and runTransactionAsync. - Document why the metadata field is declared on the shared commit schema for all format versions, and warn callers of SharedTreeBranch.apply which cases must propagate metadata. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Noah Encke (noencke) · 2026-08-25T03:07:59Z

Pushed d752c3cd addressing an internal review pass. Three substantive fixes:

1. Trimmed commits kept their metadata. Eviction poisons change/revision/parent on trimmed commits, but the newest trimmed commit deliberately survives as the reachable trunk base — so its metadata stayed readable even though it is never written to the summary. A client could therefore read metadata that a client loading from that summary would never see. The same gap let a TreeBranchCommitMetadata captured before trimming keep reporting metadata indefinitely. Metadata is now cleared for every trimmed commit (cleared rather than poisoned, so a stale wrapper reads undefined instead of throwing, and the value can be collected).

2. The commit aliased the caller's object, and could disagree with what was persisted. The metadata is now snapshotted at the transaction boundary via a JSON round trip. This fixes two things: mutating the object after runTransaction returns can no longer change an already-created commit (which mattered most on a fork, where submission can happen much later), and the value read back locally is now exactly what peers and summaries see instead of diverging on values with no JSON representation (\NaN\ read as \NaN\ locally but arrived as
ull\ remotely). Values that cannot be represented as a JSON object now throw a UsageError.

3. JsonCompatibleReadOnlyObjectSchema was Type.Any(), so the runtime schema accepted a primitive where the format declares an object. It now validates the object root.

Also added golden op/summary format tests that pin the serialized persistedMetadata key at v7 and assert its absence before v7 — the existing round-trip suites would have stayed green through a rename on both the encode and decode sides, silently orphaning metadata in documents already written at v7.

Two review points I deliberately did not act on, both documented in code so they are explicit choices rather than oversights:

The metadata field is declared on the shared commit schema for all format versions, so a summary claiming to be v6 that nonetheless contained the field would be accepted rather than rejected as an additional property. Writing is correctly gated, and making the schema version-dependent would mean threading the version through every schema builder.
SharedTreeBranch.apply takes the metadata as an optional parameter. Every current call site is correct (audited), and requiring it would churn many call sites where a genuinely new commit is being minted.

Suite is now 15304 passing / 1 failing, the failure being the pre-existing snapshotCompatibilityChecker Windows path-separator mismatch that also fails on unmodified \main.

… metadata Addresses PR review feedback. - Rename the feature from "persisted" to "custom" metadata: `customMetadata` on RunTransactionParamsAlpha, the op format, the summary format and GraphCommit, and `custom` on TreeBranchCommitMetadata (matching the existing NodeSchemaMetadata.custom / FieldSchemaMetadata.custom convention). - Merge the metadata of all nested transactions into the single commit they produce rather than using only the outermost. Conflicting properties resolve to the outermost transaction, and a nested transaction that is rolled back contributes nothing. - Align JsonCompatibleReadOnlyObjectSchema with the existing PersistedMetadataFormat used for schema metadata, rather than Type.Any(). - Simplify or remove doc comments per review, and remove a duplicated doc comment left on SharedTreeBranch.apply. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Noah Encke (noencke) · 2026-08-25T06:08:03Z

Pushed b7706ae1 with all review feedback applied. Replied inline to each comment; summarizing the three that weren't purely mechanical:

Nested transaction merge - implemented. All transactions in the stack now contribute to the single commit they produce, outermost winning on conflicts. A nested transaction that is rolled back contributes nothing, which fell out of the existing commit/abort handling and seemed clearly right. Four new tests cover it.

JSON validation precedent - the closest existing case is persisted schema metadata (\PersistedMetadataFormat), which validates at the schema level only via \Type.Record(Type.String(), JsonCompatibleReadOnlySchema). I've aligned \JsonCompatibleReadOnlyObjectSchema\ to that exact shape; it had been \Type.Any(), weaker than the format it describes. I kept the snapshot step on top, since it fixes a real aliasing bug rather than just validating.

NaN - normalizing to
ull\ matches what SharedTree already does with user data (\leafNodeSchema.ts, and \SchemaStatics.number\ documents it as matching JSON's limitations), so this is consistent rather than novel. Docs now appeal to \JsonCompatibleReadOnlyObject\ instead of restating the rules.

One thing worth your call on naming: \persistedMetadata\ is already an established name in this package for exactly this concept - app-supplied \JsonCompatibleReadOnlyObject\ that gets persisted - on \FieldPropsAlpha, \NodeSchemaOptionsAlpha\ and \SimpleNodeSchemaBaseAlpha. So the rename does trade consistency with those options bags for consistency with \metadata.custom. I've made the change as requested since \custom\ clearly fits the read property; flagging it only in case the collision with the schema convention changes your mind for the write side.

Validation: 15306 passing / 1 failing (the pre-existing Windows path-separator failure). API reports regenerated - the diff is exactly the two renamed lines per report, no incidental churn. No snapshot files changed, since none of them carry metadata.

Copilot

Copilot reviewed 86 out of 86 changed files in this pull request and generated 2 comments.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

- Remove three tests fully subsumed by others: two "Reading" cases covered by the commit-association test, and the nested-transaction precedence case covered by the three-level merge test (which already includes a conflicting property). - Add coverage for two untested behaviours: the commit produced by reverting an annotated commit must not inherit its metadata, and an empty metadata object must survive replication rather than being treated as absent. Both were verified to fail when the corresponding behaviour is broken. - Replace per-test SharedTree setup with the existing `getView` helper for the cases that need neither replication nor persistence, and add a small `createConnectedViews` helper for the cases that do. - Move the op rollback test out of the persistence group and the async transaction test out of the nested transaction group. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Drop an assertion implied by the deep-equal above it. - Assert on the encoded peer branch structure directly rather than counting occurrences of the key in the serialized string. - Share codec construction between the encode and round-trip cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Noah Encke (noencke) · 2026-08-25T18:46:09Z

Did a pass over the tests themselves against completeness / convention / minimality / uniqueness. Pushed ff233283 and a77a9a2c.

Removed as redundant (3). Two Reading tests ("readable immediately", "undefined when unannotated") were fully subsumed by the test that checks metadata stays on its own commit across an unannotated neighbour, so that one now covers all three. The nested-transaction "outermost wins" test was subsumed by the three-level merge test, which already includes a property that conflicts at every level.

Added (2), both verified non-vacuous by breaking the behaviour and confirming the test fails:

Reverting an annotated commit must not copy its metadata onto the new inverse commit. This is documented behaviour (undefined only for genuinely new commits) that nothing exercised. Confirmed it fails if the revert path passes the reverted commit's metadata through.
An empty metadata object must survive replication rather than being treated as absent. Confirmed it fails if the encoder uses a truthiness check instead of an undefined check — a realistic way to regress this.

Minimality. Most tests needed neither replication nor persistence but were each standing up a TestTreeProviderLite, a view and an initialize. Those now use the existing getView helper (one line), with a small createConnectedViews helper for the ones that genuinely need peers. Net effect is 26 tests in ~13 fewer lines than the previous 27. In the codec tests I dropped an assertion implied by the deep-equal above it, and replaced counting "customMetadata": occurrences in the serialized string with a direct assertion on the encoded peer branch structure.

Organisation. The async transaction test was sitting under Nested transactions, and op rollback under Persistence; both moved.

Gaps I considered and deliberately left: metadata over a shared branch (vSharedBranches) is exercised only at the codec level, since that format is unreleased and test-only; and the detached-then-attached path is covered transitively, since submitCommit's detached branch passes the in-memory commit straight through and the attach summary uses the same encode path already under test. Happy to add either if you'd rather they were explicit.

Suite: 15305 passing, 1 failing (the pre-existing Windows path-separator failure).

…erts Two related changes to the custom commit metadata API. Metadata is now a tree mirroring the transaction nesting, matching how `LabelTree` relates to a change's labels. `TreeBranchCommitMetadata.custom` remains the flattened view (outermost transaction wins on conflict), and the new `customTree` exposes the structure. The persisted form uses abbreviated keys (`m` for metadata, `c` for children) and omits both when empty, so the common un-nested case costs 6 bytes over a bare object rather than 27. Note that op compression only engages for batches over 600 KiB, so it does not help here. Reverts may now be performed inside a transaction, provided the revert is the transaction's only change. This exists so that a revert commit can be given its own metadata rather than inheriting the reverted commit's, which matters for values like a timestamp. The previous blanket restriction was there because the inverse is computed against, and applied to, the branch the transaction forked from; requiring that the transaction have produced no commits makes those two branches identical, so the inverse is computed against exactly the state it would be outside a transaction. Further changes in the transaction are then rejected. The squashed commit still reports the revert's CommitKind, so it remains redoable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…apped reverts Two blocking issues: - `flattenCustomMetadata` used `Object.assign`, so the legal JSON key "__proto__" became the returned object's prototype rather than one of its properties. Since peers control metadata content, this let remote data introduce inherited properties into application reads. Copying now uses own-property semantics, matching `JSON.parse`. - The sole-revert restriction only covered data edits. `EditLock` exposed the schema editor unwrapped, so `upgradeSchema()` after a revert produced a mixed commit that was still reported as an undo. Schema edits are now guarded too, which also closes the same pre-existing gap for the change-event lock. Also: - The precondition for reverting in a transaction now tests for changes to the document rather than for the transaction branch having advanced. Constraint commits added by `preconditions`, and concurrent remote commits, no longer cause false rejections. - A nested transaction that rolls back a revert no longer leaves the edit restriction or the pending commit kind behind. - Disposal of a revertible reverted inside a transaction is deferred until that transaction commits, so rolling it back no longer consumes the undo. - A transaction-wrapped revert once again inherits the reverted commit's labels, unless the wrapping transaction supplies its own. - A decoded metadata tree in which no transaction supplied metadata normalizes to undefined, preserving the invariant that `customTree` is defined exactly when `custom` is. - Corrected the encoded EditManager types to use `EncodedCommit` rather than the in-memory `Commit`, added op and summary goldens that actually contain metadata, documented the staged rollout and its one-way nature, and removed duplicate summary-load fixtures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Link RunTransactionParamsAlpha in changeset (item 1) - Add customMetadata trap on fully evicted commits; gracefully return undefined from LazyTreeBranchCommitMetadata for evicted commits (item 2) - Remove obvious comment in sharedTreeCore.ts (item 4) - Simplify eviction-policy doc wording (item 7) - Replace UsageError with 'An error is thrown' in public docs (item 8) - State explicit version '3.0.0' in metadata doc (item 9) - Trim size-budget wording in API doc (item 10) - Thread includeCustomMetadata flag through typebox schema builders so pre-v7 schemas reject the field via additionalProperties (item 11) - Add tests proving pre-v7 payloads with customMetadata are rejected - Clarify test comment in messageCodec.spec.ts (item 12) - Update PR description to match current API surface (item 13) - Add additionalProperties: false to Message schema Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a037b96b-478f-4a65-9555-d7a970e7855e

yann-achard-MS · 2026-08-26T00:08:31Z

This bullet point is covering two different things:

Regressing the oldestSupportedClient or minVersionForCollab after having bumped it up to or above 3.0.
Regressing the shipped code after having shipped 3.0 or above.

These are sufficiently different that they merit their own separate bullet point.

Refers to: .changeset/custom-commit-metadata.md:59 in 3869984. [](commit_id = 3869984, deletion_comment = False)

The custom commit metadata work threaded an `includeCustomMetadata` flag through the op and summary schemas so that versions below v7 do not declare a field they never write. That threading is correct and is kept, but it also added `additionalProperties: false` to the op `Message` schema, which was permissive before. Tightening the op envelope is a change to the whole envelope rather than to this field, and it risks rejecting ops from other versions that carry envelope properties this client does not know about. Any such change belongs in its own PR, so the flag stays and the strictness does not. The summary schemas were already strict, so they are unaffected. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Reading `custom`/`customTree` through a `TreeBranchCommitMetadata` whose commit had been evicted caught the eviction trap and reported `undefined`, which is indistinguishable from a commit that was never annotated. The other properties of an evicted commit -- including `parent`, which `getParent()` reads -- let the trap through, so shielding this one was inconsistent. Drop the try/catch so retaining a commit metadata object for too long fails the same way as everything else. The trunk base still reports `undefined`, since its metadata is cleared outright rather than trapped. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Both pieces of state a revert leaves behind were tracked globally rather than per transaction, so a nested or rolled-back transaction could leak them: - `pendingRevertLabelTree` was only cleared when a rollback left an enclosing transaction open, so rolling back the *outermost* transaction containing a revert left the labels set, and the next unrelated transaction inherited the abandoned revert's labels. - `deferredRevertibleDisposals` ignored which transaction deferred a revertible, so a revert rolled back by a nested transaction was still disposed when the enclosing transaction committed, silently consuming the application's undo. Both are now keyed by transaction depth: state is re-homed outwards when a nested transaction commits, and discarded when the transaction that owns it rolls back. This also replaces the `transactionHasApplicationFacingChanges()` inference that previously stood in for "this transaction contained a revert", which Yann correctly pointed out did not actually establish that. Also from review: - Give `messageCodecVSharedBranches` the same `Mutable` + conditional-assignment shape as `messageCodecV1ToV4`, with a note on why the key is omitted rather than assigned `undefined`. - Cover custom metadata on every branch of a shared-branches summary; the child branch path was previously unexercised. - Note the broken-state limitation of metadata validation, soften the changeset to "may throw" (the trunk-base sentinel reads `undefined`), and fix two doubled-apostrophe typos. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 42b443d7-0621-42a1-b087-f4e4765046af

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 42b443d7-0621-42a1-b087-f4e4765046af

github-actions · 2026-08-26T02:35:37Z

🔗 Found some broken links! 💔

Run a link check locally to find them. See Checking for Broken Links for more information.

linkcheck output

$ start-server-and-test "npm run serve -- --host 127.0.0.1 --no-open" http://127.0.0.1:3000 check-links
1: starting server using command "npm run serve -- --host 127.0.0.1 --no-open"
and when url "[ 'http://127.0.0.1:3000' ]" is responding with HTTP status code 200
running tests using command "npm run check-links"


> fluid-framework-website@0.0.0 serve
> docusaurus serve --host 127.0.0.1 --no-open

[SUCCESS] Serving "build" directory at: http://127.0.0.1:3000/

> fluid-framework-website@0.0.0 check-links
> linkcheck http://127.0.0.1:3000 --skip-file skipped-urls.txt

Crawling...

http://127.0.0.1:3000/docs/data-structures/tree/schema-evolution/feature-flag-schema-upgrades
- (72:12) 'isStaged..' => http://127.0.0.1:3000/docs/api/fluid-framework/treeviewalpha-interface#isstagedupgradeenabled-methodsignature (HTTP 200 but missing anchor)


Stats:
  338443 links
    2041 destination URLs
    2297 URLs ignored
       1 warnings
       0 errors

Error: Command failed with exit code 1: npm run check-links
    at makeError (/home/runner/work/FluidFramework/FluidFramework/website/node_modules/.pnpm/execa@5.1.1/node_modules/execa/lib/error.js:60:11)
    at handlePromise (/home/runner/work/FluidFramework/FluidFramework/website/node_modules/.pnpm/execa@5.1.1/node_modules/execa/index.js:118:26)
    at process.processTicksAndRejections (node:internal/process/task_queues:103:5) {
  shortMessage: 'Command failed with exit code 1: npm run check-links',
  command: 'npm run check-links',
  escapedCommand: '"npm run check-links"',
  exitCode: 1,
  signal: undefined,
  signalDescription: undefined,
  stdout: undefined,
  stderr: undefined,
  failed: true,
  timedOut: false,
  isCanceled: false,
  killed: false
}
[ELIFECYCLE] Command failed with exit code 1.

yann-achard-MS · 2026-08-26T02:41:33Z

+	 * @remarks
+	 * Every transaction in the stack contributes a node to the resulting {@link CustomMetadataTree}, so
+	 * nested transactions'' metadata is retained rather than discarded. If the transaction produces no
+	 * commit, the metadata is discarded.


A single apostrophe should do, no?

github-actions · 2026-08-26T03:00:22Z

Bundle size comparison

Base commit: 80c8ac643a2343636fab389b6cb11c9fa07cc868
Head commit: 761edb51f3936a81a86856ffbfe0dd91db956fc4

Notable changes

🔴 fluidFrameworkAllAlpha.js: parsed 799531 → 805405 (+5874), gzip 219697 → 221373 (+1676)
🔴 fluidFramework.js: parsed 409576 → 415394 (+5818), gzip 116115 → 117808 (+1693)
🔴 sharedTree.js: parsed 398955 → 404766 (+5811), gzip 113536 → 115248 (+1712)

Per-bundle deltas

`@fluid-example/bundle-size-tests`

🔴 fluidFrameworkAllAlpha.js: parsed 799531 → 805405 (+5874), gzip 219697 → 221373 (+1676)
azureClient.js: parsed 633562 → 633557 (-5), gzip 169791 → 169872 (+81)
odspClient.js: parsed 604831 → 604940 (+109), gzip 162591 → 162738 (+147)
aqueduct.js: parsed 537466 → 537479 (+13), gzip 144387 → 144433 (+46)
🔴 fluidFramework.js: parsed 409576 → 415394 (+5818), gzip 116115 → 117808 (+1693)
🔴 sharedTree.js: parsed 398955 → 404766 (+5811), gzip 113536 → 115248 (+1712)
containerRuntime.js: parsed 314235 → 314213 (-22), gzip 86216 → 86213 (-3)
sharedString.js: parsed 175205 → 175212 (+7), gzip 49660 → 49667 (+7)
experimentalSharedTree.js: parsed 161812 → 161812 (0), gzip 46711 → 46711 (0)
matrix.js: parsed 159584 → 159591 (+7), gzip 45903 → 45910 (+7)
loader.js: parsed 147289 → 147305 (+16), gzip 40028 → 40038 (+10)
odspDriver.js: parsed 105655 → 105713 (+58), gzip 32926 → 32991 (+65)
directory.js: parsed 65635 → 65642 (+7), gzip 18481 → 18490 (+9)
578.js: parsed 58686 → 58686 (0), gzip 17657 → 17657 (0)
odspPrefetchSnapshot.js: parsed 45884 → 45865 (-19), gzip 15335 → 15351 (+16)
map.js: parsed 45786 → 45793 (+7), gzip 14109 → 14116 (+7)
252.js: parsed 44362 → 44362 (0), gzip 13735 → 13735 (0)
summarizerDelayLoadedModule.js: parsed 31287 → 31287 (0), gzip 7929 → 7929 (0)
socketModule.js: parsed 26992 → 26962 (-30), gzip 8019 → 8053 (+34)
createNewModule.js: parsed 12464 → 12464 (0), gzip 4792 → 4805 (+13)
summaryModule.js: parsed 3888 → 3888 (0), gzip 1874 → 1874 (0)
connectionState.js: parsed 909 → 909 (0), gzip 500 → 500 (0)
sharedTreeAttributes.js: parsed 845 → 852 (+7), gzip 493 → 503 (+10)
debugAssert.js: parsed 429 → 429 (0), gzip 299 → 299 (0)
FluidFramework-HashFallback.js: parsed 419 → 419 (0), gzip 313 → 313 (0)

yann-achard-MS · 2026-08-26T03:05:16Z

+	 * Callers that are instead *reconstructing an existing commit* — for example replaying a remote
+	 * commit, rehydrating a stashed op, or applying the commit a transaction produced — must pass the
+	 * source commit's metadata, or it will be silently dropped.
 	 */


The whole remarks section seems wrong and confusing. I don't see why the customMetadata should be optional here. Just make it required.

yann-achard-MS · 2026-08-26T03:07:39Z

+) as unknown as Type.TSchema;
+
+export function encodeCustomMetadataTree(tree: CustomMetadataTree): EncodedCustomMetadataTree {
+	const encoded: Mutable<EncodedCustomMetadataTree> = {};


Put the codecs (encodeCustomMetadataTree and decodeCustomMetadataTree) in a separate file named "customMetaDataCodec.ts". This is a pattern we follow with persisted formats.

yann-achard-MS · 2026-08-26T03:09:52Z

+ * Each node forbids additional properties. Extending the shape of a node is therefore a breaking format
+ * change and must be introduced under a new message/EditManager format version rather than by adding a
+ * key here: silently ignoring an unknown key would lose metadata when an older client re-summarizes.
+ */


Delete this section. This kind of thing implicit for all of our non-versioned formats.

yann-achard-MS · 2026-08-26T03:14:37Z

+	// A tree in which no transaction supplied metadata carries no information, and would otherwise
+	// break the invariant that `customTree` is defined exactly when the flattened view is.
+	return hasMetadata(decoded) ? decoded : undefined;
+}


This concern about the flattened view doesn't belong here.

Noah Encke (noencke) commented Aug 25, 2026

View reviewed changes

Noah Encke (noencke) marked this pull request as ready for review August 25, 2026 17:55

Noah Encke (noencke) requested review from a team as code owners August 25, 2026 17:55

Copilot AI lite review requested due to automatic review settings August 25, 2026 17:55

Copilot started reviewing on behalf of Noah Encke (noencke) August 25, 2026 17:56 View session

Copilot AI reviewed Aug 25, 2026

View reviewed changes

Comment thread packages/dds/tree/src/shared-tree-core/transaction.ts

Comment thread packages/dds/tree/src/simple-tree/api/transactionTypes.ts