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 1 commit
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, none of the shared object's do incremental summaries. However, if one decides to do it, it needs to take "fullTree" parameter into consideration.

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": 118}}}, "severity": "ERROR"}
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
13 changes: 3 additions & 10 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,25 +33,17 @@ export class DetachedFieldIndexSummarizer implements Summarizable {

public constructor(private readonly detachedFieldIndex: DetachedFieldIndex) {}

public getAttachSummary(
public summarize(
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);
}

public async load(
services: IChannelStorageService,
parse: SummaryElementParser,
Expand Down
13 changes: 3 additions & 10 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,24 +96,16 @@ export class ForestSummarizer implements Summarizable {
return stringify(encoded);
}

public getAttachSummary(
public summarize(
stringify: SummaryElementStringifier,
fullTree?: boolean,
trackState?: boolean,
telemetryContext?: ITelemetryContext,
incrementalSummaryContext?: IExperimentalIncrementalSummaryContext,
): 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 async load(
services: IChannelStorageService,
parse: SummaryElementParser,
Expand Down
19 changes: 5 additions & 14 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,24 @@ export class SchemaSummarizer implements Summarizable {
});
}

public getAttachSummary(
public summarize(
stringify: SummaryElementStringifier,
fullTree?: boolean,
fullTree: boolean = false,
trackState?: boolean,
telemetryContext?: ITelemetryContext,
incrementalSummaryContext?: IExperimentalIncrementalSummaryContext | undefined,
incrementalSummaryContext?: IExperimentalIncrementalSummaryContext,
): ISummaryTreeWithStats {
const builder = new SummaryTreeBuilder();
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 +77,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
13 changes: 3 additions & 10 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,24 +50,16 @@ export class EditManagerSummarizer<TChangeset> implements Summarizable {
private readonly schemaAndPolicy?: SchemaAndPolicy,
) {}

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

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

private summarizeCore(stringify: SummaryElementStringifier): ISummaryTreeWithStats {
const context: EditManagerEncodingContext =
this.schemaAndPolicy !== undefined
Expand Down
33 changes: 16 additions & 17 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,19 +212,29 @@ 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(
s.summarize(
(contents) => serializer.stringify(contents, this.sharedObject.handle),
undefined,
undefined,
fullTree,
undefined /* trackState */,
telemetryContext,
incrementalSummaryContext,
childIncrementalSummaryContext,
),
);
}
Expand Down Expand Up @@ -434,18 +444,6 @@ 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.
Expand All @@ -455,7 +453,8 @@ export interface Summarizable {
fullTree?: boolean,
trackState?: boolean,
telemetryContext?: ITelemetryContext,
): Promise<ISummaryTreeWithStats>;
incrementalSummaryContext?: IExperimentalIncrementalSummaryContext,
): ISummaryTreeWithStats;

/**
* Allows the component to perform custom loading. The storage service is scoped to this component and therefore
Expand Down
10 changes: 5 additions & 5 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 @@ -203,7 +203,7 @@ describe("End to end chunked encoding", () => {
assert(chunk.isShared());
return JSON.stringify(content);
}
forestSummarizer.getAttachSummary(stringifier);
forestSummarizer.summarize(stringifier);
});

// See note on above test.
Expand Down Expand Up @@ -236,7 +236,7 @@ describe("End to end chunked encoding", () => {
assert(chunk.isShared());
return JSON.stringify(content);
}
forestSummarizer.getAttachSummary(stringifier);
forestSummarizer.summarize(stringifier);
});

describe("identifier field encoding", () => {
Expand All @@ -257,7 +257,7 @@ describe("End to end chunked encoding", () => {
function stringifier(content: unknown) {
return JSON.stringify(content);
}
const { summary } = forestSummarizer.getAttachSummary(stringifier);
const { summary } = forestSummarizer.summarize(stringifier);
const tree = summary.tree.ForestTree;
assert(tree.type === SummaryType.Blob);
const treeContent = JSON.parse(tree.content as string);
Expand Down Expand Up @@ -287,7 +287,7 @@ describe("End to end chunked encoding", () => {
function stringifier(content: unknown) {
return JSON.stringify(content);
}
const { summary } = forestSummarizer.getAttachSummary(stringifier);
const { summary } = forestSummarizer.summarize(stringifier);
const tree = summary.tree.ForestTree;
assert(tree.type === SummaryType.Blob);
const treeContent = JSON.parse(tree.content as string);
Expand All @@ -312,7 +312,7 @@ describe("End to end chunked encoding", () => {
function stringifier(content: unknown) {
return JSON.stringify(content);
}
const { summary } = forestSummarizer.getAttachSummary(stringifier);
const { summary } = forestSummarizer.summarize(stringifier);
const tree = summary.tree.ForestTree;
assert(tree.type === SummaryType.Blob);
const treeContent = JSON.parse(tree.content as string);
Expand Down
14 changes: 2 additions & 12 deletions packages/dds/tree/src/test/shared-tree-core/sharedTreeCore.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,7 @@ describe("SharedTreeCore", () => {

interface MockSummarizableEvents extends IEvent {
(event: "loaded", listener: (blobContents?: string) => void): void;
(event: "summarize" | "summarizeAttached" | "summarizeAsync" | "gcRequested"): void;
(event: "summarize" | "summarizeAttached" | "gcRequested"): void;
}

class MockSummarizable
Expand All @@ -613,7 +613,7 @@ describe("SharedTreeCore", () => {
}
}

public getAttachSummary(
public summarize(
stringify: SummaryElementStringifier,
fullTree?: boolean | undefined,
trackState?: boolean | undefined,
Expand All @@ -623,16 +623,6 @@ describe("SharedTreeCore", () => {
return this.summarizeCore(stringify);
}

public async summarize(
stringify: SummaryElementStringifier,
fullTree?: boolean | undefined,
trackState?: boolean | undefined,
telemetryContext?: ITelemetryContext | undefined,
): Promise<ISummaryTreeWithStats> {
this.emit("summarizeAsync");
return this.summarizeCore(stringify);
}

private summarizeCore(stringify: SummaryElementStringifier): ISummaryTreeWithStats {
this.emit("summarize");
return createSingleBlobSummary(
Expand Down
Loading