MILAB-6002: fix PlDataTable column visibility reset on re-run with changed filters

.changeset/milab-6002-table-visibility-deviations.md

@@ -0,0 +1,26 @@
+---
+'@platforma-sdk/model': minor
+'@platforma-sdk/ui-vue': patch
+---
+
+PlDataTable: persist column visibility as user deviations from block defaults
+
+Column visibility reset when a block re-ran with a changed filter/ranking
+configuration (MILAB-6002). The saved grid state stored the absolute
+hidden-column set, which once present overrode the block's default visibility. A
+column whose default flipped between runs — e.g. a filter/ranking column
+reverting to `optional` when its filter is removed — stayed visible instead of
+reverting to hidden.
+
+Column visibility is now the user's explicit show/hide deviations from the
+block's `pl7.app/table/visibility` default, so the current default always applies
+to untouched columns. Persisted state migrates v7 -> v8: a one-time reset of
+custom column show/hide, after which defaults apply correctly.
+
+Adds `resolveColumnHidden` to the public model API — the shared default-vs-override
+precedence used by the visible table handle and the grid — so this is a `minor` bump.
+
+Also routes the deprecated `createPlDataTableV2` path through the shared
+`computeHiddenColumns`, so the ~29 blocks still on it reconcile the same deviations (and
+honour `shownColIds`) as the grid instead of misreading the lists as an absolute hidden
+set — which over-included columns once visibility was customised.

sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV2.test.ts

@@ -0,0 +1,76 @@
+import {
+  Annotation,
+  type PColumnSpec,
+  type PObjectId,
+  type PTableColumnId,
+} from "@milaboratories/pl-model-common";
+import { describe, expect, test } from "vitest";
+import { computeHiddenColumnsV2 } from "./createPlDataTableV2";
+
+function col(id: string, visibility?: "optional" | "hidden"): { id: PObjectId; spec: PColumnSpec } {
+  return {
+    id: id as PObjectId,
+    spec: {
+      kind: "PColumn",
+      name: id,
+      valueType: "Int",
+      axesSpec: [],
+      annotations: visibility ? { [Annotation.Table.Visibility]: visibility } : {},
+    } as PColumnSpec,
+  };
+}
+
+const colRef = (id: string): PTableColumnId =>
+  ({ type: "column", id: id as PObjectId }) as PTableColumnId;
+
+const hiddenIds = (s: Set<PObjectId>): string[] => [...s].sort();
+
+describe("computeHiddenColumnsV2 (deviation-aware)", () => {
+  test("with no overrides, hides optional columns and shows the rest", () => {
+    const cols = [col("visible"), col("opt", "optional")];
+    expect(hiddenIds(computeHiddenColumnsV2(cols, null, null))).toEqual(["opt"]);
+  });
+
+  test("a user-hidden column (block default visible) becomes hidden", () => {
+    const cols = [col("a"), col("b")];
+    expect(hiddenIds(computeHiddenColumnsV2(cols, [colRef("a")], null))).toEqual(["a"]);
+  });
+
+  test("a user-shown column (block default optional) becomes visible", () => {
+    const cols = [col("a", "optional"), col("b", "optional")];
+    expect(hiddenIds(computeHiddenColumnsV2(cols, null, [colRef("a")]))).toEqual(["b"]);
+  });
+
+  // Regression for the old absolute-set reader: a non-null hide list was treated as
+  // "hide exactly these", so an empty list (user only showed a column) unhid EVERY
+  // optional column. Deviations must keep untouched optional columns hidden.
+  test("empty hiddenColIds with a show-override keeps other optional columns hidden", () => {
+    const cols = [col("opt1", "optional"), col("opt2", "optional")];
+    expect(hiddenIds(computeHiddenColumnsV2(cols, [], [colRef("opt1")]))).toEqual(["opt2"]);
+  });
+
+  // MILAB-6002: an untouched column follows its CURRENT default across re-runs,
+  // rather than being pinned by a stale saved set.
+  test("a column whose default flips to optional is re-hidden when untouched", () => {
+    expect(hiddenIds(computeHiddenColumnsV2([col("flip")], null, null))).toEqual([]);
+    expect(hiddenIds(computeHiddenColumnsV2([col("flip", "optional")], null, null))).toEqual([
+      "flip",
+    ]);
+  });
+
+  // Forced-hidden (`visibility: "hidden"`) columns are dropped from the visible table,
+  // matching V3 and the grid (delegates to computeHiddenColumns). Guards against
+  // regressing V2 back to leaving them in.
+  test("a forced-hidden column is hidden", () => {
+    expect(hiddenIds(computeHiddenColumnsV2([col("forced", "hidden")], null, null))).toEqual([
+      "forced",
+    ]);
+  });
+
+  // Axis references in the override lists are ignored (only column overrides apply).
+  test("axis-type override entries are ignored", () => {
+    const cols = [col("a")];
+    const axisRef = { type: "axis", id: { name: "x" } } as unknown as PTableColumnId;
+    expect(hiddenIds(computeHiddenColumnsV2(cols, [axisRef], null))).toEqual([]);
+  });
+});

sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV2.ts

@@ -1,6 +1,7 @@
 import type {
   AxisId,
   PColumn,
+  PColumnSpec,
   PObjectId,
   PTableColumnId,
   PTableColumnIdAxis,
@@ -32,7 +33,8 @@ import { getMatchingLabelColumns } from "../labels";
 import { collectFilterSpecColumns } from "../../../filters/traverse";
 import { isEmpty } from "es-toolkit/compat";
 import { createPTableDefV2 } from "./createPTableDefV2";
-import { isColumnOptional } from "./utils";
+import { computeHiddenColumns } from "./createPlDataTableV3";
+import type { Nil } from "@milaboratories/helpers";
 
 /**
  * @deprecated This function is deprecated and will be removed in future. Please migrate to createPlDataTable with v3 options for improved column discovery and display configuration. See createPlDataTableOptionsV3 for details on the new options format and migration guidance.
@@ -135,21 +137,15 @@ export function createPlDataTableV2<A, U>(
   const pframeHandle = ctx.createPFrame(fullColumns);
   if (!fullHandle || !pframeHandle) return undefined;
 
-  const hiddenColumns = new Set<PObjectId>(
-    ((): PObjectId[] => {
-      // Inner join works as a filter - all columns must be present
-      if (coreJoinType === "inner") return [];
-
-      const hiddenColIds = tableStateNormalized.pTableParams.hiddenColIds;
-      if (hiddenColIds !== null) {
-        return hiddenColIds
-          .filter((s): s is PTableColumnIdColumn => s.type === "column")
-          .map((s) => s.id);
-      }
-
-      return columns.filter((c) => isColumnOptional(c.spec)).map((c) => c.id);
-    })(),
-  );
+  const hiddenColumns =
+    // Inner join works as a filter - all columns must be present.
+    coreJoinType === "inner"
+      ? new Set<PObjectId>()
+      : computeHiddenColumnsV2(
+          columns,
+          tableStateNormalized.pTableParams.hiddenColIds,
+          tableStateNormalized.pTableParams.shownColIds,
+        );
 
   // Preserve linker columns
   columns.filter((c) => isLinkerColumn(c.spec)).forEach((c) => hiddenColumns.delete(c.id));
@@ -209,6 +205,26 @@ export function createPlDataTableV2<A, U>(
   } satisfies PlDataTableModel;
 }
 
+/**
+ * V2's base hide decision: which columns to drop from the visible table, reconciling each
+ * column's block default with the user's show/hide deviations. Delegates to the shared
+ * {@link computeHiddenColumns} so the deprecated V2 path uses the exact same precedence as
+ * the V3 model and the grid's `makeColDef` — one implementation, no drift, and forced-hidden
+ * (`visibility: "hidden"`) columns are dropped here just as they are in V3.
+ *
+ * Sort/filter preservation is skipped (both args `null`); the caller then force-keeps
+ * sorted, filtered, linker, and core columns visible on top of this base set.
+ *
+ * Exported for unit testing.
+ */
+export function computeHiddenColumnsV2(
+  columns: { readonly id: PObjectId; readonly spec: PColumnSpec }[],
+  hiddenSpecs: Nil | PTableColumnId[],
+  shownSpecs: Nil | PTableColumnId[],
+): Set<PObjectId> {
+  return computeHiddenColumns(columns, null, null, hiddenSpecs, shownSpecs);
+}
+
 function getAllLabelColumns(
   resultPool: AxisLabelProvider & ColumnProvider,
 ): PColumn<PColumnDataUniversal>[] | undefined {

sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV3.test.ts

@@ -0,0 +1,82 @@
+import {
+  Annotation,
+  type PColumnSpec,
+  type PObjectId,
+  type PTableColumnId,
+  type PTableSorting,
+} from "@milaboratories/pl-model-common";
+import { describe, expect, test } from "vitest";
+import { computeHiddenColumns } from "./createPlDataTableV3";
+
+function col(id: string, visibility?: "optional" | "hidden"): { id: PObjectId; spec: PColumnSpec } {
+  return {
+    id: id as PObjectId,
+    spec: {
+      kind: "PColumn",
+      name: id,
+      valueType: "Int",
+      axesSpec: [],
+      annotations: visibility ? { [Annotation.Table.Visibility]: visibility } : {},
+    } as PColumnSpec,
+  };
+}
+
+const colRef = (id: string): PTableColumnId =>
+  ({ type: "column", id: id as PObjectId }) as PTableColumnId;
+
+const hiddenIds = (s: Set<PObjectId>): string[] => [...s].sort();
+
+describe("computeHiddenColumns", () => {
+  test("with no saved overrides, hides forced-hidden and optional columns and shows the rest", () => {
+    const cols = [col("visible"), col("opt", "optional"), col("hid", "hidden")];
+    expect(hiddenIds(computeHiddenColumns(cols, null, null, null, null))).toEqual(["hid", "opt"]);
+  });
+
+  test("a user-hidden column (block default visible) becomes hidden", () => {
+    const cols = [col("a"), col("b")];
+    expect(hiddenIds(computeHiddenColumns(cols, null, null, [colRef("a")], null))).toEqual(["a"]);
+  });
+
+  test("a user-shown column (block default optional) becomes visible", () => {
+    const cols = [col("a", "optional"), col("b", "optional")];
+    expect(hiddenIds(computeHiddenColumns(cols, null, null, null, [colRef("a")]))).toEqual(["b"]);
+  });
+
+  test("show overrides win over hide overrides", () => {
+    const cols = [col("a")];
+    expect(hiddenIds(computeHiddenColumns(cols, null, null, [colRef("a")], [colRef("a")]))).toEqual(
+      [],
+    );
+  });
+
+  test("forced-hidden columns stay hidden even when the user showed them", () => {
+    const cols = [col("a", "hidden")];
+    expect(hiddenIds(computeHiddenColumns(cols, null, null, null, [colRef("a")]))).toEqual(["a"]);
+  });
+
+  // MILAB-6002 regression: an untouched column follows its CURRENT default across
+  // re-runs, rather than being pinned by a stale saved hidden set.
+  test("a column whose default flips to optional is re-hidden when untouched", () => {
+    expect(hiddenIds(computeHiddenColumns([col("flip")], null, null, null, null))).toEqual([]);
+    expect(
+      hiddenIds(computeHiddenColumns([col("flip", "optional")], null, null, null, null)),
+    ).toEqual(["flip"]);
+  });
+
+  test("sorted columns are force-kept visible even when optional", () => {
+    const cols = [col("a", "optional")];
+    const sorting = [{ column: colRef("a") }] as unknown as PTableSorting[];
+    expect(hiddenIds(computeHiddenColumns(cols, sorting, null, null, null))).toEqual([]);
+  });
+
+  // Preservation (collectPreservedColumnIds) wins over an explicit user hide: a column the
+  // user hid but is now sorted/filtered is force-kept visible so the active sort/filter has
+  // its data. Pins the precedence between resolveColumnHidden and preservation — the grid's
+  // makeColDef does NOT preserve, so the column is in the model's visible table yet hidden in
+  // the grid (intended "sort by hidden column"; see the model/UI divergence note).
+  test("a sorted column the user explicitly hid is still force-kept visible", () => {
+    const cols = [col("a")]; // block default visible
+    const sorting = [{ column: colRef("a") }] as unknown as PTableSorting[];
+    expect(hiddenIds(computeHiddenColumns(cols, sorting, null, [colRef("a")], null))).toEqual([]);
+  });
+});

sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV3.ts

@@ -28,6 +28,7 @@ import {
   evaluateRules,
   isColumnHidden,
   isColumnOptional,
+  resolveColumnHidden,
   withHidenAxesAnnotations,
   withLabelAnnotations,
   withTableVisualAnnotations,
@@ -186,11 +187,13 @@ export function createPlDataTableV3<A, U>(
   ]);
 
   const hiddenSpecs = state.pTableParams.hiddenColIds;
+  const shownSpecs = state.pTableParams.shownColIds;
   const hiddenColumnIds = computeHiddenColumns(
     [...annotated.direct, ...annotated.linked].map((v) => v.column),
     sorting,
     filters,
     hiddenSpecs,
+    shownSpecs,
   );
 
   const visible = buildVisibleColumns(annotated, hiddenColumnIds);
@@ -446,21 +449,42 @@ function buildSecondaryGroups(
   ];
 }
 
-/** Determine which columns should be hidden based on state or optional-column defaults. */
-function computeHiddenColumns(
+/** Determine which columns should be hidden, reconciling block defaults with the
+ * user's explicit show/hide overrides. Sorted/filtered columns are force-kept visible.
+ *
+ * Exported for unit testing. */
+export function computeHiddenColumns(
   columns: { readonly id: PObjectId; readonly spec: PColumnSpec }[],
   sorting: Nil | PTableSorting[],
   filters: Nil | PlDataTableFilters,
   hiddenSpecs: Nil | PTableColumnId[],
+  shownSpecs: Nil | PTableColumnId[],
 ): Set<PObjectId> {
-  const alwaysHidden = columns.filter((c) => isColumnHidden(c.spec)).map((c) => c.id);
-  const optionalHidden = !isNil(hiddenSpecs)
-    ? hiddenSpecs.filter((s): s is PTableColumnIdColumn => s.type === "column").map((s) => s.id)
-    : columns.filter((c) => isColumnOptional(c.spec)).map((c) => c.id);
-  const initial = [...alwaysHidden, ...optionalHidden];
+  const userHidden = new Set(
+    (hiddenSpecs ?? [])
+      .filter((s): s is PTableColumnIdColumn => s.type === "column")
+      .map((s) => s.id),
+  );
+  const userShown = new Set(
+    (shownSpecs ?? [])
+      .filter((s): s is PTableColumnIdColumn => s.type === "column")
+      .map((s) => s.id),
+  );
+  // Reconcile each column's block default with the user's explicit overrides via the
+  // shared resolveColumnHidden, so the model and UI (makeColDef) can never diverge.
+  const hidden = columns
+    .filter((c) =>
+      resolveColumnHidden({
+        forcedHidden: isColumnHidden(c.spec),
+        optional: isColumnOptional(c.spec),
+        userShown: userShown.has(c.id),
+        userHidden: userHidden.has(c.id),
+      }),
+    )
+    .map((c) => c.id);
   const preserved = collectPreservedColumnIds(sorting, filters);
 
-  return new Set(initial.filter((id) => !preserved.has(id)));
+  return new Set(hidden.filter((id) => !preserved.has(id)));
 }
 
 /** Collect IDs of columns that must remain visible (sorted, filtered). */

sdk/model/src/components/PlDataTable/createPlDataTable/utils.test.ts

@@ -1,7 +1,13 @@
 import { Annotation, type PColumnSpec, type PObjectId } from "@milaboratories/pl-model-common";
 import { SpecDriver } from "@milaboratories/pf-spec-driver";
 import { describe, expect, test } from "vitest";
-import { deriveAllLabels, evaluateRules, type LabelableColumn, type RuleColumn } from "./utils";
+import {
+  deriveAllLabels,
+  evaluateRules,
+  resolveColumnHidden,
+  type LabelableColumn,
+  type RuleColumn,
+} from "./utils";
 import type { ColumnOrderRule, ColumnVisibilityRule } from "./createPlDataTableV3";
 
 // ---------------------------------------------------------------------------
@@ -225,3 +231,39 @@ describe("evaluateRules", () => {
     expect(result.get("d" as PObjectId)?.visibility).toBe("hidden");
   });
 });
+
+// ---------------------------------------------------------------------------
+// resolveColumnHidden — shared precedence for model (computeHiddenColumns) and
+// UI (makeColDef). The single place the reconciliation rule lives.
+// ---------------------------------------------------------------------------
+
+describe("resolveColumnHidden", () => {
+  const base = { forcedHidden: false, optional: false, userShown: false, userHidden: false };
+
+  test("forced-hidden columns stay hidden, even when the user showed them", () => {
+    expect(resolveColumnHidden({ ...base, forcedHidden: true })).toBe(true);
+    expect(resolveColumnHidden({ ...base, forcedHidden: true, userShown: true })).toBe(true);
+  });
+
+  test("an explicit user show wins over a hide override and over the optional default", () => {
+    expect(resolveColumnHidden({ ...base, userShown: true })).toBe(false);
+    expect(resolveColumnHidden({ ...base, userShown: true, userHidden: true })).toBe(false);
+    expect(resolveColumnHidden({ ...base, userShown: true, optional: true })).toBe(false);
+  });
+
+  test("an explicit user hide hides a column the default would have shown", () => {
+    expect(resolveColumnHidden({ ...base, userHidden: true })).toBe(true);
+  });
+
+  test("untouched columns follow their current optional default", () => {
+    expect(resolveColumnHidden({ ...base, optional: true })).toBe(true);
+    expect(resolveColumnHidden({ ...base, optional: false })).toBe(false);
+  });
+
+  // MILAB-6002 regression: a column whose default flips between runs and that the
+  // user never touched must follow the CURRENT default, not stale saved state.
+  test("a flipped default with no user override follows the new default", () => {
+    expect(resolveColumnHidden({ ...base, optional: false })).toBe(false); // run 1: shown
+    expect(resolveColumnHidden({ ...base, optional: true })).toBe(true); // run 2: re-hidden
+  });
+});