fix(mcp-server): Fix same-object-dup copy factor, sliced-string aggregate, eval doc

Summary:
Three correctness/effectiveness improvements to the memlab MCP server. Source of truth is `www/scripts/webspeed/memory_lab/packages/mcp-server/`; the `fbcode/claude-templates/.../memlab-server/src/` copy is synced via the sync script.

**1. `auto_investigate` "Same-Object Duplication" reported a wildly wrong copy factor (correctness).** The duplication factor was computed as `sampleCount / cardinality` over *any* unique-looking field, so a CONSTANT field (e.g. `gala_project_id` with 1 distinct value) reported "~200 retained copies" when the real duplication was 2×. The over-stated magnitude is misleading and (in one case) propagated into a diff summary before being caught. Rewrote the per-shape analysis into two passes that derive the factor from the MOST-DISTINCT string-valued field — the record's true discriminator — instead of an arbitrary `*_id`-shaped key. Now 2 real copies read as `≈2×`, genuinely-distinct records produce no false positive (the most-distinct field is ~unique), and the alert is phrased as a sample-based estimate with a "confirm with `memlab_property_distribution`" pointer.

**2. `sliced_strings` was blind to the "many medium parents" case (effectiveness).** When `min_parent_size` filtered out every parent (e.g. thousands of ~250 KB JSON records, each sub-MB), the tool returned only counts and no actionable output. Added a `**Total parent bytes pinned by slices:**` aggregate (Σ distinct parent self-sizes) reported regardless of the filter, and a fallback that shows the largest parents (with a note) instead of going silent when nothing meets `min_parent_size`.

**3. `eval` `helpers.retainedSizes()` return type was undocumented (DX).** It returns a `Record<id, bytes>` object, not an array; the docs implied an array, leading to `sizes.reduce is not a function`. Documented the object return shape (iterate with `Object.values(sizes)` / index `sizes[id]`) in the `describe_env` output, the tool description, and the agent-facing `SKILL.md`.

Versions bumped (the 4 independently-versioned spots): OSS `mcp-server/package.json` 2.5.0 → 2.6.0, the hardcoded MCP handshake version literal in `src/index.ts` 2.5.0 → 2.6.0 (synced to the plugin), plugin `memlab-server/package.json` 2.5.0 → 2.6.0, and plugin `.claude-plugin/plugin.json` 1.12.0 → 1.13.0.

___

Differential Revision: D108944208

fbshipit-source-id: 9f86231b0062c4410eaff2737dff108bee56aeab

Author: JacksonGL

packages/mcp-server/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@memlab/mcp-server",
-  "version": "2.5.0",
+  "version": "2.6.0",
   "license": "MIT",
   "description": "MCP server for MemLab heap snapshot analysis — gives AI coding assistants tools to explore JavaScript heap snapshots, find memory leaks, and identify optimization opportunities",
   "author": "Liang Gong <lgong@meta.com>",

packages/mcp-server/src/index.ts

@@ -73,7 +73,7 @@ import {registerEventRegistry} from './tools/event-registry.js';
 
 const server = new McpServer({
   name: 'memlab',
-  version: '2.5.0', // keep in sync with package.json
+  version: '2.6.0', // keep in sync with package.json
 });
 
 // Wrap every tool with a wall-clock guardrail (default 90s, override per-call

packages/mcp-server/src/tools/auto-investigate.ts

@@ -1890,8 +1890,19 @@ export function registerAutoInvestigate(server: McpServer): void {
           for (const s of qualifyingShapes) {
             const sampleNodes = shapeBuckets.get(s.properties.join(',')) ?? [];
             if (sampleNodes.length < 50) continue;
-
-            // Analyze each property's cardinality and value characteristics
+            const sampleCount = sampleNodes.length;
+            const shapeKey = s.properties.join(',');
+
+            // Pass 1: per-property cardinality / value characteristics over the
+            // sample.
+            interface PropStat {
+              name: string;
+              cardinality: number;
+              stringValueCount: number;
+              totalValueSize: number;
+              allUnique: boolean;
+            }
+            const propStats: PropStat[] = [];
             for (const propName of s.properties) {
               // Skip prototype / internal pseudo-properties — they are not data
               // columns, so "pre-filter / intern this column" advice is noise
@@ -1924,60 +1935,78 @@ export function registerAutoInvestigate(server: McpServer): void {
                   }
                 }
               }
+              propStats.push({
+                name: propName,
+                cardinality: values.size,
+                stringValueCount,
+                totalValueSize,
+                allUnique,
+              });
+            }
 
-              const cardinality = values.size;
-              const sampleCount = sampleNodes.length;
-              const uniqueKey = looksUniqueKey(propName);
-              const shapeKey = s.properties.join(',');
-
-              // Same-object duplication vs genuine low cardinality (§1b): a field
-              // that *should* be unique per record but has far fewer distinct
-              // values than sampled instances means the SAME records are retained
-              // N times — a retention/concurrency fix, not interning.
-              if (
-                uniqueKey &&
-                sampleCount >= 100 &&
-                cardinality >= 1 &&
-                cardinality < sampleCount * 0.6 &&
-                !dupShapesSeen.has(shapeKey)
-              ) {
-                const dupFactor = Math.round(
-                  sampleCount / Math.max(1, cardinality),
+            // Same-object duplication (§1b), copy-factor corrected (round 4 §1a):
+            // derive the factor from the MOST-DISTINCT string-valued field — the
+            // record's true discriminator — NOT from an arbitrary unique-looking
+            // field. The old code did `sampleCount / cardinality` on any
+            // `*_id`-shaped key, so a CONSTANT field (e.g. `gala_project_id`
+            // with 1 value) reported "~200 copies" when the real duplication was
+            // 2×. If even the most-distinct field repeats, the whole record is
+            // duplicated; if the most-distinct field is ~unique, there is no
+            // duplication regardless of how many constant columns exist.
+            const stringStats = propStats.filter(
+              p => p.stringValueCount >= sampleCount * 0.5,
+            );
+            let maxCard = 0;
+            let discrim = '';
+            for (const p of stringStats) {
+              if (p.cardinality > maxCard) {
+                maxCard = p.cardinality;
+                discrim = p.name;
+              }
+            }
+            if (
+              stringStats.length > 0 &&
+              maxCard >= 1 &&
+              maxCard < sampleCount * 0.6 &&
+              sampleCount >= 100 &&
+              !dupShapesSeen.has(shapeKey)
+            ) {
+              const copyFactor = Math.round(sampleCount / maxCard);
+              if (copyFactor >= 2) {
+                dupShapesSeen.add(shapeKey);
+                duplicationAlerts.push(
+                  `- \`{${s.properties.slice(0, 4).join(', ')}${s.properties.length > 4 ? ', …' : ''}}\` (${formatNumber(s.count)} total instances): even the most-distinct field \`${discrim}\` has only **${formatNumber(maxCard)} distinct value(s)** across ${formatNumber(sampleCount)} sampled — the same records appear to be retained **≈${copyFactor}×** (sample-based estimate; confirm the exact factor with \`memlab_property_distribution\` on \`${discrim}\`, or a content-hash count). Investigate **retention/concurrency** (e.g. many concurrent requests each holding a full copy of this dataset, or the same listing materialized twice), NOT string interning.`,
                 );
-                if (dupFactor >= 2) {
-                  dupShapesSeen.add(shapeKey);
-                  duplicationAlerts.push(
-                    `- \`${propName}\` (a unique-looking key) has only **${formatNumber(cardinality)} distinct value(s)** across ${formatNumber(sampleCount)} sampled of ${formatNumber(s.count)} total \`{${s.properties.slice(0, 4).join(', ')}${s.properties.length > 4 ? ', …' : ''}}\` instances — looks like **~${dupFactor} retained copies of the same records**, not a low-cardinality column. Investigate **retention/concurrency** (e.g. many concurrent requests each holding a full copy of this dataset), NOT string interning.`,
-                  );
-                }
-              } else if (
-                !uniqueKey &&
-                cardinality <= 20 &&
-                sampleCount >= 100
-              ) {
+              }
+            }
+
+            // Pass 2: per-property low-cardinality + high-cost-unique notes.
+            for (const p of propStats) {
+              const uniqueKey = looksUniqueKey(p.name);
+              if (!uniqueKey && p.cardinality <= 20 && sampleCount >= 100) {
                 // Genuine low-cardinality column. Only suggest string interning
                 // when the column actually holds strings — interning a numeric
                 // (SMI/heap-number) or boolean column is meaningless
                 // (Feedback round 4 §2).
-                const isStringColumn = stringValueCount >= sampleCount * 0.5;
+                const isStringColumn = p.stringValueCount >= sampleCount * 0.5;
                 columnAlerts.push(
                   isStringColumn
-                    ? `- Property \`${propName}\` has only **${cardinality} unique value(s)** across ${formatNumber(s.count)} instances — low-cardinality column suitable for pre-filtering at the data source or string interning`
-                    : `- Property \`${propName}\` has only **${cardinality} unique value(s)** across ${formatNumber(s.count)} instances — low-cardinality column suitable for pre-filtering at the data source`,
+                    ? `- Property \`${p.name}\` has only **${p.cardinality} unique value(s)** across ${formatNumber(s.count)} instances — low-cardinality column suitable for pre-filtering at the data source or string interning`
+                    : `- Property \`${p.name}\` has only **${p.cardinality} unique value(s)** across ${formatNumber(s.count)} instances — low-cardinality column suitable for pre-filtering at the data source`,
                 );
               }
 
-              // High-cost unique: all values unique, string, large average size
+              // High-cost unique: all values unique, string, large average size.
               if (
-                allUnique &&
-                stringValueCount > sampleCount * 0.8 &&
-                cardinality > sampleCount * 0.9
+                p.allUnique &&
+                p.stringValueCount > sampleCount * 0.8 &&
+                p.cardinality > sampleCount * 0.9
               ) {
-                const avgSize = totalValueSize / stringValueCount;
+                const avgSize = p.totalValueSize / p.stringValueCount;
                 if (avgSize > 50 && s.count > 10_000) {
                   const estimatedWaste = avgSize * s.count;
                   columnAlerts.push(
-                    `- Property \`${propName}\` has **all unique string values** (avg ${Math.round(avgSize)}B each × ${formatNumber(s.count)} instances = ~${formatBytes(estimatedWaste)}) — verify this field is needed by consumers`,
+                    `- Property \`${p.name}\` has **all unique string values** (avg ${Math.round(avgSize)}B each × ${formatNumber(s.count)} instances = ~${formatBytes(estimatedWaste)}) — verify this field is needed by consumers`,
                   );
                 }
               }

packages/mcp-server/src/tools/eval.ts

@@ -142,7 +142,7 @@ export function registerEval(server: McpServer): void {
       '**Iterating all nodes:** `snapshot.nodes.forEach(node => { ... })` — NOT for-of.\n' +
       '**Get node by ID:** `snapshot.getNodeById(id)` returns IHeapNode or null.\n' +
       '**String values:** `node.toStringNode()?.stringValue` for string nodes.\n' +
-      '**Caveat — retained_size is unreliable here:** inside eval, `node.retained_size`/`.retainedSize` can read back ~0 for every node on some loads. Node counts, property/edge walks, and string values ARE trustworthy. For authoritative retained sizes call `helpers.retainedSize(id)` / `helpers.retainedSizes([ids])` (they re-resolve the node on the real snapshot), or use the dedicated tools (`memlab_largest_objects`, `memlab_class_histogram`, `memlab_pinch_points`, `memlab_object_shape`).\n\n' +
+      '**Caveat — retained_size is unreliable here:** inside eval, `node.retained_size`/`.retainedSize` can read back ~0 for every node on some loads. Node counts, property/edge walks, and string values ARE trustworthy. For authoritative retained sizes call `helpers.retainedSize(id)` (number) / `helpers.retainedSizes([ids])` (a `Record<id, bytes>` object, NOT an array) — they re-resolve the node on the real snapshot — or use the dedicated tools (`memlab_largest_objects`, `memlab_class_histogram`, `memlab_pinch_points`, `memlab_object_shape`).\n\n' +
       '**Example — inspect Map entries:**\n' +
       '```\nconst map = snapshot.getNodeById(12345);\nconst entries = [];\n' +
       'for (const edge of map.references) {\n' +
@@ -169,7 +169,7 @@ export function registerEval(server: McpServer): void {
             'markdownTable, isNodeWorthInspecting, filterLargestObjects, queryNodes, ' +
             'groupReferrersByEdge(nodeId), groupArrayElementsByProperty(arrayNodeId, propName), ' +
             'isOrphaned(nodeId, ownershipEdgeNames[]), countUniqueTargets(arrayNodeId, propName), ' +
-            'retainedSize(id), retainedSizes(ids[]) }), ' +
+            'retainedSize(id)->number, retainedSizes(ids[])->Record<id,bytes> (an OBJECT keyed by id, NOT an array — index it as sizes[id] or Object.values(sizes)) }), ' +
             'and standard JS built-ins. ' +
             'Node traversal: use node.references (outgoing) and node.referrers (incoming) with for-of. ' +
             'Edge properties: .name_or_index, .type, .toNode, .fromNode.',
@@ -437,14 +437,14 @@ function describeEnv(): string {
     '## In-scope globals',
     '- `snapshot` — IHeapSnapshot: `.nodes.forEach(cb)`, `.edges.forEach(cb)`, `.getNodeById(id)`.',
     '- `utils` — @memlab/core utils (e.g. `aggregateDominatorMetrics`, `isFiberNode`, `isDetachedDOMNode`).',
-    '- `helpers` — `serializeNodeSummary`, `serializeNodeDetail`, `formatBytes`, `formatNumber`, `markdownTable`, `isNodeWorthInspecting`, `filterLargestObjects`, `queryNodes`, `groupReferrersByEdge(nodeId)`, `groupArrayElementsByProperty(arrayNodeId, prop)`, `isOrphaned(nodeId, ownerEdges[])`, `countUniqueTargets(arrayNodeId, prop)`, `retainedSize(id)`, `retainedSizes(ids[])`.',
+    '- `helpers` — `serializeNodeSummary`, `serializeNodeDetail`, `formatBytes`, `formatNumber`, `markdownTable`, `isNodeWorthInspecting`, `filterLargestObjects`, `queryNodes`, `groupReferrersByEdge(nodeId)`, `groupArrayElementsByProperty(arrayNodeId, prop)`, `isOrphaned(nodeId, ownerEdges[])`, `countUniqueTargets(arrayNodeId, prop)`, `retainedSize(id) -> number`, `retainedSizes(ids[]) -> Record<id, bytes>` (an OBJECT keyed by id, NOT an array — use `sizes[id]` or `Object.values(sizes)`, not `.reduce`/`.map` directly).',
     '- Standard JS built-ins (Array, Object, Map, Set, JSON, Math, RegExp, …). No require/process/fs/network.',
     '',
     '## IHeapNode API',
     '`.id`, `.name`, `.type`, `.self_size`, `.retainedSize` (alias `.retained_size`), `.edge_count`, `.is_detached`, `.numOfReferrers` (alias `.referrer_count`), `.isString`, `.toStringNode()?.stringValue`, `.hasPathEdge`, `.pathEdge`, `.dominatorNode`, `.location` (`script_id`/`line`/`column`).',
     '',
     '## Caveat: retained_size',
-    'Inside eval, `.retainedSize`/`.retained_size` can read back ~0 for every node on some loads. Counts, property/edge walks, and string values are reliable. For authoritative retained sizes call `helpers.retainedSize(id)` or `helpers.retainedSizes([ids])` (they re-resolve the node on the real snapshot, so you can rank custom analyses by retained size), or use `memlab_largest_objects`, `memlab_class_histogram`, `memlab_pinch_points`, or `memlab_object_shape`.',
+    'Inside eval, `.retainedSize`/`.retained_size` can read back ~0 for every node on some loads. Counts, property/edge walks, and string values are reliable. For authoritative retained sizes call `helpers.retainedSize(id)` (returns a number) or `helpers.retainedSizes([ids])` (returns a `Record<id, bytes>` OBJECT — not an array; iterate with `Object.values(sizes)` / index with `sizes[id]`) — both re-resolve the node on the real snapshot, so you can rank custom analyses by retained size. Or use `memlab_largest_objects`, `memlab_class_histogram`, `memlab_pinch_points`, or `memlab_object_shape`.',
     '',
     '## IHeapEdge API',
     '`.name_or_index`, `.type` (property/element/context/internal/hidden/shortcut), `.toNode`, `.fromNode`.',

packages/mcp-server/src/tools/sliced-strings.ts

@@ -110,11 +110,31 @@ export function registerSlicedStrings(server: McpServer): void {
           );
         }
 
-        let parents = [...parentMap.values()];
+        const allParents = [...parentMap.values()];
+        const totalPinnedBytes = allParents.reduce(
+          (sum, p) => sum + p.self_size,
+          0,
+        );
+
+        // Don't go silent when `min_parent_size` filters everything out. On the
+        // common "many medium parents" case (each parent sub-MB but thousands of
+        // them, e.g. ~250 KB JSON records) the filter used to hide every row and
+        // the tool reported nothing actionable. Fall back to the largest parents
+        // with a note, and ALWAYS report the aggregate bytes pinned so the "lots
+        // of small parents" case is visible (Feedback round 4 §3a).
+        let parents = allParents;
+        let filteredNote = '';
         if (min_parent_size != null) {
-          parents = parents.filter(p => p.self_size >= min_parent_size);
+          const filtered = allParents.filter(
+            p => p.self_size >= min_parent_size,
+          );
+          if (filtered.length > 0) {
+            parents = filtered;
+          } else {
+            filteredNote = ` (no parent ≥ ${formatBytes(min_parent_size)}; showing the largest instead)`;
+          }
         }
-        parents.sort((a, b) => b.self_size - a.self_size);
+        parents = [...parents].sort((a, b) => b.self_size - a.self_size);
         const topParents = parents.slice(0, limit);
 
         const lines: string[] = [];
@@ -129,6 +149,11 @@ export function registerSlicedStrings(server: McpServer): void {
         lines.push(
           `**Unique parent strings referenced:** ${formatNumber(parentMap.size)}`,
         );
+        if (allParents.length > 0) {
+          lines.push(
+            `**Total parent bytes pinned by slices:** ${formatBytes(totalPinnedBytes)} across ${formatNumber(allParents.length)} parent string(s) — substrings keep these alive even when each individual parent is small.`,
+          );
+        }
         lines.push('');
 
         if (topParents.length > 0) {
@@ -149,7 +174,9 @@ export function registerSlicedStrings(server: McpServer): void {
             formatNumber(p.sliced_ref_count),
             formatBytes(p.sliced_total_size),
           ]);
-          lines.push('### Top parent strings (keeping slices alive)');
+          lines.push(
+            `### Top parent strings (keeping slices alive)${filteredNote}`,
+          );
           lines.push('');
           lines.push(markdownTable(headers, rows, rightCols));