fix(tree): retainHistory now retains history in summaries - #28036
Merged
Noah Encke (noencke) merged 4 commits intoAug 21, 2026
Conversation
github-actions
Bot
added
area: tools
area: dds
Contributor
|
Hi! Thank you for opening this PR. Want me to review it? Based on the diff (213 lines, 6 files), I've queued these reviewers:
How this works
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes an oversight in SharedTreeOptions.retainHistory: enabling the flag now retains trunk history not only in-memory but also in summaries/snapshots, so later-joining clients loading from a summary can recover the retained trunk history.
Changes:
- Update
EditManager.getSummaryData()to widen the main-branch summary lower bound to the trunk base whenretainHistoryis enabled, causing summaries to include the full trunk. - Add new test coverage validating summary round-trip behavior for both
retainHistory: trueand the default behavior, including shared-branch base resolution across a round trip. - Add a changeset documenting the behavior correction and its implications (summary size/load/summarize time growth when enabled).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/dds/tree/src/shared-tree-core/editManager.ts | Adjusts summarization bounds so retainHistory causes the full trunk to be written into summaries. |
| packages/dds/tree/src/shared-tree/sharedTree.ts | Updates retainHistory option documentation to reflect summary persistence behavior and limitations. |
| packages/dds/tree/src/test/shared-tree/sharedTree.spec.ts | Adds end-to-end-ish tests that summarize and reload a client to validate retained-history persistence and shared-branch behavior. |
| packages/dds/tree/src/test/shared-tree-core/edit-manager/editManagerCorrectness.test.ts | Adds correctness-level tests asserting summary content differs with/without retainHistory, and persists across repeated load/summarize generations. |
| packages/dds/tree/src/test/shared-tree-core/edit-manager/editManagerTestUtils.ts | Plumbs retainHistory through test EditManager factory helpers to support new tests. |
| .changeset/retain-history-in-summaries.md | Records the behavior fix as a release note for @fluidframework/tree and fluid-framework. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Noah Encke (noencke)
force-pushed
the
fix-trunk-retention
branch
from
August 21, 2026 15:17
d9f111c to
5e9de4c
Compare
Contributor
Bundle size comparisonBase commit: Notable changesNo bundles changed by ≥ 500 bytes parsed. Per-bundle deltas
|
yann-achard-MS
approved these changes
Aug 21, 2026
Tyler Butler (tylerbutler)
approved these changes
Aug 21, 2026
Sonali Deshpande (sonalideshpandemsft)
pushed a commit
that referenced
this pull request
Aug 27, 2026
Noah Encke (noencke)
added a commit
that referenced
this pull request
Aug 27, 2026
…17 (#28104) ## Description Cherry-picks the SharedTree history and persisted-commit-metadata work onto the `release/client/2.117` branch. This is step 4 of the 2.117 release plan (branch created from `release/client/2.116` + version bump); release notes, changelogs and assert tagging are handled separately by the release engineers. Four commits, applied in merge order (each with `git cherry-pick -x`, so the original SHA is recorded in the commit message): | Commit on `main` | PR | What it provides | |---|---|---| | `22e5b4ee` | [#27932](#27932) | Renames the alpha `TreeBranchAlpha` interface to `UntypedTreeViewAlpha` (old name kept as a deprecated alias, so it is additive). Carries no feature, but #28012 is written against the new name, so it is a prerequisite rather than an optional cleanup. | | `3b665a2a` | [#28036](#28036) | `retainHistory` now retains the trunk in *summaries*, not just in memory. Without it, retained history is discarded at the next summary and never reaches clients that load from it. No persisted format change; the `retainHistory: false` default is untouched. | | `920e9469` | [#28012](#28012) | `UntypedTreeViewAlpha.branchHistory` (`length`, `getHead()` → `TreeBranchCommitMetadata { revision, getParent() }`), plus `rewindTo(revision)` and `revertTo(revision, options?)`. | | `90337165` | [#28064](#28064) | Persisted commit metadata: `customMetadata` on `RunTransactionParamsAlpha` and the revert options, read back as `.custom` / `.customTree`. Stored inline on commits in the `EditManager` summary under `EditManagerFormatVersion.v7` / `MessageFormatVersion.v7`, written only when `minVersionForCollab` is at least `2.117.0`. | A fifth commit adapts the picks to this branch — details below. ### Why a minor rather than a patch #28064 changes a persisted format, and the format gate is a version string baked into SharedTree's source. The config map that selects a format version rejects patch versions, so the gate has to be an `X.Y.0`. It is `FluidClientVersion.v2_117 = "2.117.0"`. ### Forward compatibility with 3.0 All four commits are already on `main`, and `main`'s tip was #28064 when this was prepared, so the two lines could be compared directly. Everything that ships here is identical to 3.0: - The whole persisted-format surface — `src/codec/` and `src/shared-tree-core/` — is byte-identical between this branch and `main`, including the `v2_117` → v7 gate. A mixed 2.117/3.0 session cannot silently disagree about the format. - Every alpha symbol added here (`TreeBranchHistory`, `TreeBranchCommitMetadata`, `branchHistory`, `rewindTo`, `revertTo`, `customMetadata`, `customTree`, `retainHistory`, `UntypedTreeViewAlpha`, `RunTransactionParamsAlpha`, `RevertOptionsAlpha`, `RevertToOptionsAlpha`) is identical on both lines, at the same API tier, with the same deprecation state. - `minVersionForCollab: '2.117.0'` stays valid on 3.0: `OldestSupportedClientVersion` *widens* from `` `${1 | 2}.${bigint}.${bigint}` `` to `` `${1 | 2 | 3}....` ``, so consumers who adopt 2.117 need no change in this area when they later move to 3.0. ## Adapting the picks to this branch `main` is on TypeScript 6 and past the 3.0 bump; this branch is TypeScript 5.4 and pre-3.0. Conflicts were all of one shape — an import list where `main` has accumulated names from commits that are *not* being backported — and were resolved by keeping only what this branch actually uses: - `src/index.ts` — kept `asTreeViewAlpha` (removed on `main` by 3.0 work) alongside the incoming `TreeBranchCommitMetadata` / `TreeBranchHistory`. - `src/shared-tree/treeCheckout.ts` — kept `StableId` and `findAncestor`, which #28012 uses; dropped `tagCodeArtifacts` (used on `main` only by schema-change telemetry, #27996) and `getDeltaChangeProfile` (introduced by #27989). Neither is backported here. - `src/test/shared-tree/schematizeTree.spec.ts` — kept `TreeBranchHistory`; dropped `UntypedTreeViewAlpha`, which was added to that import by the TypeScript 6 upgrade (#28052) rather than by #28012. One genuine TypeScript-version difference, in the fifth commit: - `customCommitMetadata.spec.ts` used `.filter((m) => m !== undefined)` and then read `.tag`, relying on **inferred type predicates**, a TypeScript 5.5 feature. On 5.4 the element type stays `T | undefined` and the compiler reports TS18048. The predicate is now spelled explicitly. This was the only TypeScript error in the entire build. The two conflicted `*.api.md` files are generated artifacts, so rather than hand-resolving them they were regenerated by a full clean build from the repo root. That correctly drops `asAlpha`, `codePointCount` and `utf16LengthForCodePoints` — whose exports come from #28011 and #28004, not backported here — and narrows `OldestSupportedServiceClientVersion` to `` `2.${bigint}.0` ``. ## Validation - Full `pnpm clean && pnpm build` from the repo root: succeeded, and left the working tree clean apart from the intended API report updates. No unexpected API report drift. - `@fluidframework/tree` test suite: **15301 passing, 488 pending, 1 failing**. The one failure is a pre-existing Windows-only issue in `snapshotCompatibilityChecker.spec.ts`, which builds the directory under test with `path.join(...)` but hardcodes forward slashes in the expected error string. It is untouched by these commits and passes on Linux CI. ## Reviewer Guidance The review process is outlined in [the pull request guidelines](../docs/content/Contributing/PR-Guidelines.md#guidelines). - The first four commits are unmodified cherry-picks; review effort is best spent on the fifth (`fix(tree): adapt the backported history work to the 2.117 line`) and on the conflict resolutions described above, since those are the only places this branch diverges from what was reviewed on `main`. - **Assert tagging is deliberately not included here.** The three new asserts are still untagged string literals, so `flub release prepare client` will report them. Running `flub generate assertTags` on a release branch allocates short codes from the branch's own high-water mark, which can disagree with `main` — on `main` today it would reassign `0xd33`, a code that shipped in 2.116 as `"compatibilityMode must be defined"`, to a tree assert. Happy to follow whatever sequencing the release engineers prefer. --------- Co-authored-by: jzaffiro <110866475+jzaffiro@users.noreply.github.com> Co-authored-by: yann-achard-MS <97201204+yann-achard-MS@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3af96e03-702e-49da-adaa-bc99ca75ee27 Copilot-Session: eedf6254-a76e-4efe-af90-1f7b43da8665 Copilot-Session: a037b96b-478f-4a65-9555-d7a970e7855e Copilot-Session: 42b443d7-0621-42a1-b087-f4e4765046af Copilot-Session: 797fba3e-0e9a-48db-86c6-530aa2837434
Sonali Deshpande (sonalideshpandemsft)
pushed a commit
that referenced
this pull request
Aug 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
retainHistoryonSharedTreeOptionswas documented as causing growth in both memory and summaries/snapshots, but it only ever prevented trunk commits from being evicted from memory. Summaries continued to contain just the collaboration window, so retained history was discarded at the next summary and was unavailable to clients that loaded from it.This was an oversight, not an intentional design choice — confirmed against the original PR (#27696) that introduced the option, whose shipped changeset already documented the intended (summary-inclusive) behavior.
Summaries produced by a client with
retainHistoryenabled now contain the full trunk. History accumulated while the flag is enabled survives summarization and is available to clients that join later. History from before the flag was enabled (already evicted in a prior session) cannot be recovered. There is no change to the default (retainHistory: false) behavior, and no change to the persisted format.New tests confirm 3 of them fail without the production fix (proving they exercise it); the full
@fluidframework/treesuite (15,203 tests) passes with no regressions.Reviewer Guidance
The review process is outlined in the pull request guidelines.
This PR received an automated rigorous code-review pass; one test assertion was strengthened as a result (exact equality instead of
<=on retained commit count).