Skip to content

(tree) Unify summarize and getAttachSummary methods on summarizables #24761

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Jun 12, 2025
Merged

(tree) Unify summarize and getAttachSummary methods on summarizables #24761

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0764b04
(tree) Unfiy summarize and getAttachSummary methods on summarizables
agarwal-navin Jun 3, 2025
54ec0f4
PR comments
agarwal-navin Jun 6, 2025
dde573d
Fix tests
agarwal-navin Jun 6, 2025
1e4359b
typo
agarwal-navin Jun 9, 2025
8ebc3fd
Doc comments
agarwal-navin Jun 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/loud-dolls-appear.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@fluidframework/shared-object-base": minor
"__section": legacy
---
Added an optional boolean parameter `fullTree` to `SharedObject`'s `summarizeCore` method

Check failure on line 5 in .changeset/loud-dolls-appear.md

View workflow job for this annotation

GitHub Actions / vale 

[vale] reported by reviewdog 🐶
[Vale.Spelling] Did you really mean 'boolean'?

Raw Output:
{"message": "[Vale.Spelling] Did you really mean 'boolean'?", "location": {"path": ".changeset/loud-dolls-appear.md", "range": {"start": {"line": 5, "column": 19}}}, "severity": "ERROR"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our most recent guidance for changeset titles is to not use inlineCode syntax (to prevent the generated changelogs from appearing too noisy). This still needs to be documented in a more accessible place, but here are the guidelines we're using when reviewing: https://eng.ms/docs/experiences-devices/opg/office-shared/fluid-framework/fluid-framework-internal/fluid-framework/docs/on-call/release/release-prep

Suggested change
Added an optional boolean parameter `fullTree` to `SharedObject`'s `summarizeCore` method
Added an optional boolean parameter "fullTree" to SharedObject's summarizeCore method


This parameter tells the shared object that it should generate a full tree summary, i.e., it must not summarize incrementally.

Check failure on line 7 in .changeset/loud-dolls-appear.md

View workflow job for this annotation

GitHub Actions / vale 

[vale] reported by reviewdog 🐶
[Microsoft.Foreign] Use 'that is' instead of 'i.e.,'.

Raw Output:
{"message": "[Microsoft.Foreign] Use 'that is' instead of 'i.e.,'.", "location": {"path": ".changeset/loud-dolls-appear.md", "range": {"start": {"line": 7, "column": 85}}}, "severity": "ERROR"}
Currently no known SharedObjects's do incremental summaries; however any that do exist or are made in the future must take this "fullTree" parameter into consideration to function correctly.

Check warning on line 8 in .changeset/loud-dolls-appear.md

View workflow job for this annotation

GitHub Actions / vale 

[vale] reported by reviewdog 🐶
[Microsoft.Semicolon] Try to simplify this sentence.

Raw Output:
{"message": "[Microsoft.Semicolon] Try to simplify this sentence.", "location": {"path": ".changeset/loud-dolls-appear.md", "range": {"start": {"line": 8, "column": 60}}}, "severity": "INFO"}

Check failure on line 8 in .changeset/loud-dolls-appear.md

View workflow job for this annotation

GitHub Actions / vale 

[vale] reported by reviewdog 🐶
[Vale.Spelling] Did you really mean 'fullTree'?

Raw Output:
{"message": "[Vale.Spelling] Did you really mean 'fullTree'?", "location": {"path": ".changeset/loud-dolls-appear.md", "range": {"start": {"line": 8, "column": 130}}}, "severity": "ERROR"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit (we do like inlineCode syntax in the changeset body when referring to APIs)

Suggested change
Currently no known SharedObjects's do incremental summaries; however any that do exist or are made in the future must take this "fullTree" parameter into consideration to function correctly.
Currently no known `SharedObjects`'s do incremental summaries; however, any that do exist or are made in the future must take this "fullTree" parameter into consideration to function correctly.

2 changes: 1 addition & 1 deletion packages/dds/shared-object-base/api-report/shared-object-base.legacy.alpha.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export abstract class SharedObject<TEvent extends ISharedObjectEvents = ISharedO
// (undocumented)
protected get serializer(): IFluidSerializer;
summarize(fullTree?: boolean, trackState?: boolean, telemetryContext?: ITelemetryContext, incrementalSummaryContext?: IExperimentalIncrementalSummaryContext): Promise<ISummaryTreeWithStats>;
protected abstract summarizeCore(serializer: IFluidSerializer, telemetryContext?: ITelemetryContext, incrementalSummaryContext?: IExperimentalIncrementalSummaryContext): ISummaryTreeWithStats;
protected abstract summarizeCore(serializer: IFluidSerializer, telemetryContext?: ITelemetryContext, incrementalSummaryContext?: IExperimentalIncrementalSummaryContext, fullTree?: boolean): ISummaryTreeWithStats;
}

// @alpha @legacy
Expand Down
9 changes: 8 additions & 1 deletion packages/dds/shared-object-base/src/sharedObject.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -842,7 +842,12 @@ export abstract class SharedObject<
trackState: boolean = false,
telemetryContext?: ITelemetryContext,
): ISummaryTreeWithStats {
const result = this.summarizeCore(this.serializer, telemetryContext);
const result = this.summarizeCore(
this.serializer,
telemetryContext,
undefined /* incrementalSummaryContext */,
fullTree,
);
this.incrementTelemetryMetric(
blobCountPropertyName,
result.stats.blobNodeCount,
Expand All @@ -869,6 +874,7 @@ export abstract class SharedObject<
this.serializer,
telemetryContext,
incrementalSummaryContext,
fullTree,
);
this.incrementTelemetryMetric(
blobCountPropertyName,
Expand Down Expand Up @@ -937,6 +943,7 @@ export abstract class SharedObject<
serializer: IFluidSerializer,
telemetryContext?: ITelemetryContext,
incrementalSummaryContext?: IExperimentalIncrementalSummaryContext,
fullTree?: boolean,
): ISummaryTreeWithStats;

private incrementTelemetryMetric(
Expand Down
9 changes: 8 additions & 1 deletion packages/dds/shared-object-base/src/sharedObjectKernel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ export interface SharedKernel {
serializer: IFluidSerializer,
telemetryContext: ITelemetryContext | undefined,
incrementalSummaryContext: IExperimentalIncrementalSummaryContext | undefined,
fullTree?: boolean,
): ISummaryTreeWithStats;

/**
Expand Down Expand Up @@ -146,8 +147,14 @@ class SharedObjectFromKernel<
serializer: IFluidSerializer,
telemetryContext?: ITelemetryContext,
incrementalSummaryContext?: IExperimentalIncrementalSummaryContext,
fullTree?: boolean,
): ISummaryTreeWithStats {
return this.#kernel.summarizeCore(serializer, telemetryContext, incrementalSummaryContext);
return this.#kernel.summarizeCore(
serializer,
telemetryContext,
incrementalSummaryContext,
fullTree,
);
}

protected override initializeLocalCore(): void {
Expand Down
25 changes: 9 additions & 16 deletions packages/dds/tree/src/feature-libraries/detachedFieldIndexSummarizer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import { bufferToString } from "@fluid-internal/client-utils";
import type { IChannelStorageService } from "@fluidframework/datastore-definitions/internal";
import type {
IExperimentalIncrementalSummaryContext,
ISummaryTreeWithStats,
ITelemetryContext,
} from "@fluidframework/runtime-definitions/internal";
Expand All @@ -32,23 +33,15 @@ export class DetachedFieldIndexSummarizer implements Summarizable {

public constructor(private readonly detachedFieldIndex: DetachedFieldIndex) {}

public getAttachSummary(
stringify: SummaryElementStringifier,
fullTree?: boolean,
trackState?: boolean,
telemetryContext?: ITelemetryContext,
): ISummaryTreeWithStats {
public summarize(props: {
stringify: SummaryElementStringifier;
fullTree?: boolean;
trackState?: boolean;
telemetryContext?: ITelemetryContext;
incrementalSummaryContext?: IExperimentalIncrementalSummaryContext;
}): ISummaryTreeWithStats {
const data = this.detachedFieldIndex.encode();
return createSingleBlobSummary(detachedFieldIndexBlobKey, stringify(data));
}

public async summarize(
stringify: SummaryElementStringifier,
fullTree?: boolean,
trackState?: boolean,
telemetryContext?: ITelemetryContext,
): Promise<ISummaryTreeWithStats> {
return this.getAttachSummary(stringify, fullTree, trackState, telemetryContext);
return createSingleBlobSummary(detachedFieldIndexBlobKey, props.stringify(data));
}

public async load(
Expand Down
25 changes: 9 additions & 16 deletions packages/dds/tree/src/feature-libraries/forest-summary/forestSummarizer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { assert } from "@fluidframework/core-utils/internal";
import type { IChannelStorageService } from "@fluidframework/datastore-definitions/internal";
import type { IIdCompressor } from "@fluidframework/id-compressor";
import type {
IExperimentalIncrementalSummaryContext,
ISummaryTreeWithStats,
ITelemetryContext,
} from "@fluidframework/runtime-definitions/internal";
Expand Down Expand Up @@ -95,22 +96,14 @@ export class ForestSummarizer implements Summarizable {
return stringify(encoded);
}

public getAttachSummary(
stringify: SummaryElementStringifier,
fullTree?: boolean,
trackState?: boolean,
telemetryContext?: ITelemetryContext,
): ISummaryTreeWithStats {
return createSingleBlobSummary(treeBlobKey, this.getTreeString(stringify));
}

public async summarize(
stringify: SummaryElementStringifier,
fullTree?: boolean,
trackState?: boolean,
telemetryContext?: ITelemetryContext,
): Promise<ISummaryTreeWithStats> {
return createSingleBlobSummary(treeBlobKey, this.getTreeString(stringify));
public summarize(props: {
stringify: SummaryElementStringifier;
fullTree?: boolean;
trackState?: boolean;
telemetryContext?: ITelemetryContext;
incrementalSummaryContext?: IExperimentalIncrementalSummaryContext;
}): ISummaryTreeWithStats {
return createSingleBlobSummary(treeBlobKey, this.getTreeString(props.stringify));
}

public async load(
Expand Down
29 changes: 11 additions & 18 deletions packages/dds/tree/src/feature-libraries/schema-index/schemaSummarizer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,23 +51,26 @@ export class SchemaSummarizer implements Summarizable {
});
}

public getAttachSummary(
stringify: SummaryElementStringifier,
fullTree?: boolean,
trackState?: boolean,
telemetryContext?: ITelemetryContext,
incrementalSummaryContext?: IExperimentalIncrementalSummaryContext | undefined,
): ISummaryTreeWithStats {
public summarize(props: {
stringify: SummaryElementStringifier;
fullTree: boolean;
trackState?: boolean;
telemetryContext?: ITelemetryContext;
incrementalSummaryContext?: IExperimentalIncrementalSummaryContext;
}): ISummaryTreeWithStats {
const incrementalSummaryContext = props.incrementalSummaryContext;
const builder = new SummaryTreeBuilder();
const fullTree = props.fullTree ?? false;
if (
!fullTree &&
incrementalSummaryContext !== undefined &&
this.schemaIndexLastChangedSeq !== undefined &&
incrementalSummaryContext.latestSummarySequenceNumber >= this.schemaIndexLastChangedSeq
) {
builder.addHandle(
schemaStringKey,
SummaryType.Blob,
`${incrementalSummaryContext.summaryPath}/indexes/${this.key}/${schemaStringKey}`,
`${incrementalSummaryContext.summaryPath}/${schemaStringKey}`,
);
} else {
const dataString = JSON.stringify(this.codec.encode(this.schema));
Expand All @@ -76,16 +79,6 @@ export class SchemaSummarizer implements Summarizable {
return builder.getSummaryTree();
}

public async summarize(
stringify: SummaryElementStringifier,
fullTree?: boolean,
trackState?: boolean,
telemetryContext?: ITelemetryContext,
incrementalSummaryContext?: IExperimentalIncrementalSummaryContext | undefined,
): Promise<ISummaryTreeWithStats> {
throw new Error("Method not implemented.");
}

public async load(
services: IChannelStorageService,
parse: SummaryElementParser,
Expand Down
25 changes: 9 additions & 16 deletions packages/dds/tree/src/shared-tree-core/editManagerSummarizer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { assert } from "@fluidframework/core-utils/internal";
import type { IChannelStorageService } from "@fluidframework/datastore-definitions/internal";
import type { IIdCompressor } from "@fluidframework/id-compressor";
import type {
IExperimentalIncrementalSummaryContext,
ISummaryTreeWithStats,
ITelemetryContext,
} from "@fluidframework/runtime-definitions/internal";
Expand Down Expand Up @@ -49,22 +50,14 @@ export class EditManagerSummarizer<TChangeset> implements Summarizable {
private readonly schemaAndPolicy?: SchemaAndPolicy,
) {}

public getAttachSummary(
stringify: SummaryElementStringifier,
fullTree?: boolean,
trackState?: boolean,
telemetryContext?: ITelemetryContext,
): ISummaryTreeWithStats {
return this.summarizeCore(stringify);
}

public async summarize(
stringify: SummaryElementStringifier,
fullTree?: boolean,
trackState?: boolean,
telemetryContext?: ITelemetryContext,
): Promise<ISummaryTreeWithStats> {
return this.summarizeCore(stringify);
public summarize(props: {
stringify: SummaryElementStringifier;
fullTree?: boolean;
trackState?: boolean;
telemetryContext?: ITelemetryContext;
incrementalSummaryContext?: IExperimentalIncrementalSummaryContext;
}): ISummaryTreeWithStats {
return this.summarizeCore(props.stringify);
}

private summarizeCore(stringify: SummaryElementStringifier): ISummaryTreeWithStats {
Expand Down
57 changes: 33 additions & 24 deletions packages/dds/tree/src/shared-tree-core/sharedTreeCore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,20 +212,30 @@ export class SharedTreeCore<TEditor extends ChangeFamilyEditor, TChange>
serializer: IFluidSerializer,
telemetryContext?: ITelemetryContext,
incrementalSummaryContext?: IExperimentalIncrementalSummaryContext,
fullTree?: boolean,
): ISummaryTreeWithStats {
const builder = new SummaryTreeBuilder();
const summarizableBuilder = new SummaryTreeBuilder();
// Merge the summaries of all summarizables together under a single ISummaryTree
for (const s of this.summarizables) {
// Add the summarizable's path in the summary tree to the incremental summary context's
// summary path, so that the summarizable can use it to generate incremental summaries.
const childIncrementalSummaryContext =
incrementalSummaryContext === undefined
? undefined
: {
...incrementalSummaryContext,
summaryPath: `${incrementalSummaryContext.summaryPath}/${summarizablesTreeKey}/${s.key}`,
};
summarizableBuilder.addWithStats(
s.key,
s.getAttachSummary(
(contents) => serializer.stringify(contents, this.sharedObject.handle),
undefined,
undefined,
s.summarize({
stringify: (contents: unknown) =>
serializer.stringify(contents, this.sharedObject.handle),
fullTree,
telemetryContext,
incrementalSummaryContext,
),
incrementalSummaryContext: childIncrementalSummaryContext,
}),
);
}

Expand Down Expand Up @@ -434,28 +444,27 @@ export interface Summarizable {
*/
readonly key: string;

/**
* {@inheritDoc @fluidframework/datastore-definitions#(IChannel:interface).getAttachSummary}
* @param stringify - Serializes the contents of the component (including {@link (IFluidHandle:interface)}s) for storage.
*/
getAttachSummary(
stringify: SummaryElementStringifier,
fullTree?: boolean,
trackState?: boolean,
telemetryContext?: ITelemetryContext,
incrementalSummaryContext?: IExperimentalIncrementalSummaryContext,
): ISummaryTreeWithStats;

/**
* {@inheritDoc @fluidframework/datastore-definitions#(IChannel:interface).summarize}
* @param stringify - Serializes the contents of the component (including {@link (IFluidHandle:interface)}s) for storage.
* @param fullTree - flag indicating whether the attempt should generate a full
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: can we call out the default value explicitly here? Also, I'm pushing for us to use full sentence casing consistently when possible.

Suggested change
* @param fullTree - flag indicating whether the attempt should generate a full
* @param fullTree - A flag indicating whether the attempt should generate a full

* summary tree without any handles for unchanged subtrees. It should only be set to true when generating
* a summary from the entire container.
* @param trackState - An optimization for tracking state of objects across summaries. If the state
* of an object did not change since last successful summary, an
* {@link @fluidframework/protocol-definitions#ISummaryHandle} can be used
* instead of re-summarizing it. If this is `false`, the expectation is that you should never
* send an `ISummaryHandle`, since you are not expected to track state.
* @param telemetryContext - See {@link @fluidframework/runtime-definitions#ITelemetryContext}.
* @param incrementalSummaryContext - See {@link @fluidframework/runtime-definitions#IExperimentalIncrementalSummaryContext}.
*/
summarize(
stringify: SummaryElementStringifier,
fullTree?: boolean,
trackState?: boolean,
telemetryContext?: ITelemetryContext,
): Promise<ISummaryTreeWithStats>;
summarize(props: {
stringify: SummaryElementStringifier;
fullTree?: boolean;
trackState?: boolean;
telemetryContext?: ITelemetryContext;
incrementalSummaryContext?: IExperimentalIncrementalSummaryContext;
}): ISummaryTreeWithStats;

/**
* Allows the component to perform custom loading. The storage service is scoped to this component and therefore
Expand Down
23 changes: 7 additions & 16 deletions packages/dds/tree/src/test/feature-libraries/chunked-forest/chunkEncodingEndToEnd.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ describe("End to end chunked encoding", () => {
);

// This function is declared in the test to have access to the original uniform chunk for comparison.
function stringifier(content: unknown) {
function stringify(content: unknown) {
const insertedChunk = decode((content as Format).fields as EncodedFieldBatch, {
idCompressor,
originatorId: idCompressor.localSessionId,
Expand All @@ -203,7 +203,7 @@ describe("End to end chunked encoding", () => {
assert(chunk.isShared());
return JSON.stringify(content);
}
forestSummarizer.getAttachSummary(stringifier);
forestSummarizer.summarize({ stringify });
});

// See note on above test.
Expand All @@ -227,7 +227,7 @@ describe("End to end chunked encoding", () => {
);

// This function is declared in the test to have access to the original uniform chunk for comparison.
function stringifier(content: unknown) {
function stringify(content: unknown) {
const insertedChunk = decode((content as Format).fields as EncodedFieldBatch, {
idCompressor,
originatorId: idCompressor.localSessionId,
Expand All @@ -236,7 +236,7 @@ describe("End to end chunked encoding", () => {
assert(chunk.isShared());
return JSON.stringify(content);
}
forestSummarizer.getAttachSummary(stringifier);
forestSummarizer.summarize({ stringify });
});

describe("identifier field encoding", () => {
Expand All @@ -254,10 +254,7 @@ describe("End to end chunked encoding", () => {
testIdCompressor,
);

function stringifier(content: unknown) {
return JSON.stringify(content);
}
const { summary } = forestSummarizer.getAttachSummary(stringifier);
const { summary } = forestSummarizer.summarize({ stringify: JSON.stringify });
const tree = summary.tree.ForestTree;
assert(tree.type === SummaryType.Blob);
const treeContent = JSON.parse(tree.content as string);
Expand All @@ -284,10 +281,7 @@ describe("End to end chunked encoding", () => {
testIdCompressor,
);

function stringifier(content: unknown) {
return JSON.stringify(content);
}
const { summary } = forestSummarizer.getAttachSummary(stringifier);
const { summary } = forestSummarizer.summarize({ stringify: JSON.stringify });
const tree = summary.tree.ForestTree;
assert(tree.type === SummaryType.Blob);
const treeContent = JSON.parse(tree.content as string);
Expand All @@ -309,10 +303,7 @@ describe("End to end chunked encoding", () => {
testIdCompressor,
);

function stringifier(content: unknown) {
return JSON.stringify(content);
}
const { summary } = forestSummarizer.getAttachSummary(stringifier);
const { summary } = forestSummarizer.summarize({ stringify: JSON.stringify });
const tree = summary.tree.ForestTree;
assert(tree.type === SummaryType.Blob);
const treeContent = JSON.parse(tree.content as string);
Expand Down
Loading
Loading