diff --git a/src/Explorer/Explorer.tsx b/src/Explorer/Explorer.tsx index c4df7ceee..946c03c47 100644 --- a/src/Explorer/Explorer.tsx +++ b/src/Explorer/Explorer.tsx @@ -447,12 +447,12 @@ export default class Explorer { startKey, ); scenarioMonitor.completePhase(MetricScenario.DatabaseLoad, ApplicationMetricPhase.CollectionsLoaded); - // Start DatabaseTreeRendered before publishing the revision that triggers the render, - // so the phase is always open by the time ResourceTree acknowledges it. Bumping the - // revision is what tells the tree "this is the data the current load is expected to - // show"; the phase is completed only for a render carrying that revision. + // Start DatabaseTreeRendered before publishing the token that triggers the render, so the + // phase is always open by the time ResourceTree acknowledges it. Publishing the token is + // what tells the tree "this is the data the current load is expected to show"; the phase + // is completed only for a render carrying that token. scenarioMonitor.startPhase(MetricScenario.DatabaseLoad, ApplicationMetricPhase.DatabaseTreeRendered); - useDatabases.setState({ treeReadyRevision: useDatabases.getState().treeReadyRevision + 1 }); + useDatabases.setState({ treeReadyToken: {} }); } catch (error) { TelemetryProcessor.traceFailure( Action.LoadCollections, diff --git a/src/Explorer/useDatabases.ts b/src/Explorer/useDatabases.ts index 5494b0f87..20d621a44 100644 --- a/src/Explorer/useDatabases.ts +++ b/src/Explorer/useDatabases.ts @@ -13,11 +13,12 @@ interface DatabasesState { resourceTokenCollection: ViewModels.CollectionBase; sampleDataResourceTokenCollection: ViewModels.CollectionBase; databasesFetchedSuccessfully: boolean; // Track if last database fetch was successful - // Incremented once a load has produced the data the resource tree is expected to show. + // Republished each time a load has produced the data the resource tree is expected to show. // The DatabaseTreeRendered phase is only completed for a render that carries the current - // revision, so an earlier render (databases fetched, collections still loading) cannot be - // mistaken for the loaded tree. 0 means no load has reached that point yet. - treeReadyRevision: number; + // token, so an earlier render (databases fetched, collections still loading) cannot be + // mistaken for the loaded tree. Compared by reference rather than counted, so there is no + // value to overflow or wrap; undefined means no load has reached that point yet. + treeReadyToken: object | undefined; searchText: string; sortOrder: DatabaseSortOrder; pinnedDatabaseIds: Set; @@ -57,7 +58,7 @@ export const useDatabases: UseStore = create((set, get) => ({ resourceTokenCollection: undefined, sampleDataResourceTokenCollection: undefined, databasesFetchedSuccessfully: false, - treeReadyRevision: 0, + treeReadyToken: undefined, searchText: "", sortOrder: loadSortOrder(), pinnedDatabaseIds: loadPinnedDatabases(), diff --git a/src/Metrics/ScenarioMonitor.phaseOrdering.test.ts b/src/Metrics/ScenarioMonitor.phaseOrdering.test.ts index 712d0afbc..556318823 100644 --- a/src/Metrics/ScenarioMonitor.phaseOrdering.test.ts +++ b/src/Metrics/ScenarioMonitor.phaseOrdering.test.ts @@ -8,7 +8,7 @@ * (ResourceTree.tsx), every completion arrived before the phase existed and was dropped: * missing in 1624 of 1627 DatabaseLoad timeouts over three days. * - * The fix is on the producer side — a ready revision published once the load has produced the + * The fix is on the producer side — a ready token published once the load has produced the * data the tree is expected to show, acknowledged by the render that carries it. These tests * pin the monitor half of that contract: completions for phases that were never opened are * refused rather than backdated, so a render observed too early cannot stand in for the @@ -45,7 +45,7 @@ const loadUpToTreeReady = () => { scenarioMonitor.completePhase(MetricScenario.DatabaseLoad, ApplicationMetricPhase.DatabasesFetched); scenarioMonitor.startPhase(MetricScenario.DatabaseLoad, ApplicationMetricPhase.CollectionsLoaded); scenarioMonitor.completePhase(MetricScenario.DatabaseLoad, ApplicationMetricPhase.CollectionsLoaded); - // Explorer publishes the ready revision immediately after opening the phase. + // Explorer publishes the ready token immediately after opening the phase. scenarioMonitor.startPhase(MetricScenario.DatabaseLoad, ApplicationMetricPhase.DatabaseTreeRendered); }; @@ -62,9 +62,9 @@ describe("DatabaseLoad phase accounting (IcM 865096261)", () => { jest.useRealTimers(); }); - it("completes healthy once the render carrying the ready revision is acknowledged", () => { + it("completes healthy once the render carrying the ready token is acknowledged", () => { loadUpToTreeReady(); - // ResourceTree commits the revision-bearing render and acknowledges it. + // ResourceTree commits the token-bearing render and acknowledges it. scenarioMonitor.completePhase(MetricScenario.DatabaseLoad, ApplicationMetricPhase.DatabaseTreeRendered); scenarioMonitor.completePhase(MetricScenario.DatabaseLoad, CommonMetricPhase.Interactive); diff --git a/src/Metrics/useMetricPhases.ts b/src/Metrics/useMetricPhases.ts index 0f5615ba5..742f9f5c1 100644 --- a/src/Metrics/useMetricPhases.ts +++ b/src/Metrics/useMetricPhases.ts @@ -43,31 +43,34 @@ export function useInteractive(scenario: MetricScenario, enabled = true) { /** * Hook to manage DatabaseLoad scenario phase completions. * - * DatabaseTreeRendered is completed only for a render that carries the ready revision - * published by the current load (Explorer.refreshAndExpandNewDatabases). An earlier render — - * databases fetched but collections still loading — carries a stale revision and is ignored, - * so it cannot be mistaken for the loaded tree. Each revision is acknowledged at most once, - * which also discards stale callbacks from a superseded load. + * DatabaseTreeRendered is completed only for a render that carries the ready token published + * by the current load (Explorer.refreshAndExpandNewDatabases). An earlier render — databases + * fetched but collections still loading — carries the previous token and is ignored, so it + * cannot be mistaken for the loaded tree. Each token is acknowledged at most once, which also + * discards stale callbacks from a superseded load. * - * An account with no databases still publishes a revision and renders an empty tree, so it + * The token is a fresh object compared by reference, so repeated refreshes cannot exhaust or + * wrap it the way an incrementing counter could. + * + * An account with no databases still publishes a token and renders an empty tree, so it * completes normally rather than stalling. */ export function useDatabaseLoadScenario(databaseTreeNodes: unknown[], fetchSucceeded: boolean) { - const treeReadyRevision = useDatabases((state) => state.treeReadyRevision); - const acknowledgedRevision = React.useRef(0); + const treeReadyToken = useDatabases((state) => state.treeReadyToken); + const acknowledgedToken = React.useRef(undefined); - // Track DatabaseTreeRendered phase. Runs after commit, so the tree carrying this revision - // is on screen by the time the phase is completed. + // Track DatabaseTreeRendered phase. Runs after commit, so the tree carrying this token is on + // screen by the time the phase is completed. React.useEffect(() => { - if (!fetchSucceeded || treeReadyRevision === 0) { + if (!fetchSucceeded || !treeReadyToken) { return; } - if (acknowledgedRevision.current >= treeReadyRevision) { + if (acknowledgedToken.current === treeReadyToken) { return; } - acknowledgedRevision.current = treeReadyRevision; + acknowledgedToken.current = treeReadyToken; scenarioMonitor.completePhase(MetricScenario.DatabaseLoad, ApplicationMetricPhase.DatabaseTreeRendered); - }, [databaseTreeNodes, fetchSucceeded, treeReadyRevision]); + }, [databaseTreeNodes, fetchSucceeded, treeReadyToken]); // Track Interactive phase useInteractive(MetricScenario.DatabaseLoad);