Make action-summary concatenation sound by chunking bundles

app/client/components/ActionLog.ts

@@ -13,9 +13,9 @@ import { basicButton } from "app/client/ui2018/buttons";
 import { labeledSquareCheckbox } from "app/client/ui2018/checkbox";
 import { theme } from "app/client/ui2018/cssVars";
 import { ActionGroup } from "app/common/ActionGroup";
-import { concatenateSummaryPair } from "app/common/ActionSummarizer";
+import { concatenateSummaries } from "app/common/ActionSummarizer";
 import {
-  ActionSummary, asTabularDiffs, createEmptyActionSummary, defunctTableName, getAffectedTables,
+  ActionSummary, asTabularDiffs, defunctTableName, getAffectedTables,
   LabelDelta,
 } from "app/common/ActionSummary";
 import { CellDelta, TabularDiff, TabularDiffs } from "app/common/TabularDiff";
@@ -110,8 +110,13 @@ export class ActionLog extends dispose.Disposable implements IDomComponent {
    */
   public async getChangesSince(actionNum: number): Promise<ActionSummary> {
     await this._loadActionSummaries();
-    return takeWhile(this.displayStack.all(), item => item.actionNum > actionNum)
-      .reduce((summary, item) => concatenateSummaryPair(item.actionSummary, summary), createEmptyActionSummary());
+    // displayStack is newest-first; reverse to oldest-first so we combine the
+    // changes in the order they happened. concatenateSummaries drops the changes
+    // that cancel out, leaving what display should show.
+    const summaries = takeWhile(this.displayStack.all(), item => item.actionNum > actionNum)
+      .map(item => item.actionSummary)
+      .reverse();
+    return concatenateSummaries(summaries);
   }
 
   /**

app/client/models/DataTableModelWithDiff.ts

@@ -250,6 +250,9 @@ export class TableDataWithDiff {
   private _leftRemovals: Set<number>;
   private _rightRemovals: Set<number>;
   private _updates: Set<number>;
+  // Rows added locally: real positive ids in the core table, kept in the "changed rows only"
+  // view alongside updates and removals. (Remote adds use synthetic negative ids; see rowId < 0.)
+  private _localAdds: Set<number>;
   // Stores pre-deletion column values for each removed row, keyed by rowId.
   // Used to distinguish undo (values match) from ID recycling (values differ).
   private _removedRowValues = new Map<number, Record<string, CellDelta[0]>>();
@@ -268,6 +271,7 @@ export class TableDataWithDiff {
       ...leftTableDelta.updateRows.filter(r => !this._rightRemovals.has(r)),
       ...rightTableDelta.updateRows.filter(r => !this._leftRemovals.has(r)),
     ]);
+    this._localAdds = new Set(leftTableDelta.addRows);
   }
 
   public getColIds(): string[] {
@@ -309,6 +313,7 @@ export class TableDataWithDiff {
     if (this._options?.showAllRows) { return undefined; }
     return (rowId: number | "new") => {
       return rowId === "new" || this._updates.has(rowId) || rowId < 0 ||
+        this._localAdds.has(rowId) ||
         this._leftRemovals.has(rowId) || this._rightRemovals.has(rowId);
     };
   }

app/common/ActionBundle.ts

@@ -63,6 +63,12 @@ export interface SandboxActionBundle {
   direct: EnvContent<boolean>[];
   calc: EnvContent<DocAction>[];
   undo: EnvContent<DocAction>[];   // Inverse actions for all 'stored' actions.
+  // Parallel to 'undo': the index into 'stored' of the action that produced each undo action, as
+  // the engine knows it directly, or null for a front calc-flush restore that the summarizer
+  // attributes for itself. Lets the summarizer chunk a bundle without re-inferring the grouping
+  // (see app/common/ActionLayout.ts chunkByOwners). Enveloped like the other parallel arrays,
+  // though the index is global to the bundle; flattened into LocalActionBundle alongside 'undo'.
+  undoOwner?: EnvContent<number | null>[];
   retValues: any[];                     // Contains retValue for each of userActions.
   rowCount: RowCounts;
   // Mapping of keys (hashes of request args) to all unique requests made in a round of calculation
@@ -91,4 +97,11 @@ export interface LocalActionBundle extends ActionBundle {
   // Applying 'undo' is governed by EDIT rather than READ permissions, so we always apply all undo
   // actions. (It is the result of applying 'undo' that may be addressed to different recipients).
   undo: DocAction[];
+
+  // Parallel to 'undo': the stored-action index each undo corresponds to (or null), as reported
+  // by the engine. Absent on bundles produced before this was recorded, and on bundles assembled
+  // without the data engine; the summarizer then falls back to inferring the grouping. Persisted
+  // with the bundle in the action history, and folded into the action hash when present (see
+  // computeActionHash) so it is covered by the checksum.
+  undoOwner?: (number | null)[];
 }