Address PR #377 review feedback

1. Fix inlined-frame iteration order in buildFlamegraphTree. Per blazesym,
   an array entry in callframeSymbols is [outermost, ..., innermost] — the
   real function at the address is at [0] and [i>0] are inlined callees. The
   previous implementation iterated N→0, which inverted the call graph in the
   flamegraph (inner inlined functions appeared as parents of the outer
   function they were inlined into). Now iterates 0→N, matching caller→callee.
   Skip nullish slots so sparse arrays (from out-of-order SymbolTableEntry
   events) no longer produce phantom (unknown) tree levels.

2. Wrap showTaskDumpStack click in try/catch matching the
   showIdleTimeFlamegraph pattern — errors now surface as a toast instead of
   silently failing.

3. Extract renderFlamegraphInSidebar({title, subtitle, samples}) helper.
   Both showTaskDumpStack and showIdleTimeFlamegraph now delegate to it,
   removing ~40 lines of duplicated sidebar setup.

4. Extract drawCrossHatch(x, y, w, h) helper for the diagonal stripe pattern
   used on idle periods that have a task dump.

5. Document taskDumps in the ParsedTrace schema
   (dial9-trace-loading/SKILL.md) so agent skills can find it. Extend the
   schema validator to understand Map<K, [{obj}]> (array-of-objects map
   values).

6. Add unit tests in test_trace_analysis.js covering:
   - Flamegraph inline frame ordering (catches the bug fixed in #1)
   - Sparse inline arrays with undefined slots
   - Unresolved address handling
   - TaskDumpEvent parsing shape, sort order, task-id integrity

Author: rcoh

dial9-viewer/skills/dial9-trace-loading/SKILL.md

@@ -35,6 +35,7 @@ description: Parse and load dial9 Tokio runtime trace files. Covers the ParsedTr
   hasSchedWait: boolean,                 // trace includes kernel scheduling wait data
   hasTaskTracking: boolean,              // trace includes task spawn/terminate events
   taskInstrumented: Map<number, boolean>, // task ID → whether task has tracing instrumentation
+  taskDumps: Map<number, [{timestamp, callchain}]>, // task ID → async stack captures (sorted by timestamp); see dial9-tokio-telemetry `taskdump` feature
 }
 ```
 

dial9-viewer/ui/test_all_skills_snippets.js

@@ -214,6 +214,14 @@ async function main() {
       .replace(/\[\w+\]:/g, '_dynamic_:')
       .replace(/:\s*Map<[^,]+,\s*([^>]+)>/g, (_, valType) => {
         const v = valType.trim();
+        // Array of objects: Map<K, [{a, b}]> → values are arrays whose elements have those keys.
+        // Emit a marker string that the later [{...}] pass won't double-process;
+        // a separate pass below expands it back to a JS array literal.
+        const arrObjMatch = v.match(/^\[\{([^}]+)\}\]$/);
+        if (arrObjMatch) {
+          const keys = arrObjMatch[1].split(',').map(k => k.trim()).filter(Boolean);
+          return ': {"_map_":"__ARR_OBJ__' + keys.join('|') + '__"}';
+        }
         const objMatch = v.match(/\{([^}]+)\}/);
         if (objMatch) {
           const keys = objMatch[1].split(',').map(k => k.trim()).filter(Boolean);
@@ -231,6 +239,11 @@ async function main() {
       .replace(/\[\{([^}]+)\}\]/g, (_, inner) => {
         const keys = inner.split(',').map(k => k.trim()).filter(Boolean);
         return '[{' + keys.map(k => `"${k}":"_any_"`).join(',') + '}]';
+      })
+      // Expand Map<K, [{...}]> placeholder from earlier pass into a real array literal.
+      .replace(/"__ARR_OBJ__([^_"]+)__"/g, (_, keysPipe) => {
+        const keys = keysPipe.split('|');
+        return '[{' + keys.map(k => `"${k}":"_any_"`).join(',') + '}]';
       });
     let docSkeleton;
     try { docSkeleton = (new Function('return {' + schemaJs + '}'))(); }

dial9-viewer/ui/test_trace_analysis.js

@@ -367,6 +367,115 @@ async function main() {
     pass("buildFgData returns null for empty samples");
   }
 
+  // Inlined frames: when callframeSymbols.get(addr) returns an array, per
+  // blazesym the array is ordered [outermost, ..., innermost]. entry[0] is the
+  // real function at the address; entry[i>0] are inlined callees so the call
+  // chain goes entry[0] -> entry[1] -> entry[2]. The flamegraph tree must
+  // descend in that same order (outermost as parent, innermost as leaf).
+  function testFlamegraphInlineOrder() {
+    const callframeSymbols = new Map([
+      ["0x1000", [
+        { symbol: "outer_fn", location: "outer.rs:10" },
+        { symbol: "mid_fn", location: "mid.rs:20" },
+        { symbol: "leaf_fn", location: "leaf.rs:30" },
+      ]],
+    ]);
+    const samples = [{ callchain: ["0x1000"], workerId: 0 }];
+    const tree = buildFlamegraphTree(samples, callframeSymbols);
+    if (tree.children.size !== 1) fail(`root has ${tree.children.size} children, expected 1`);
+    const outer = [...tree.children.values()][0];
+    if (!outer.fullName.includes("outer_fn")) fail(`child of root is "${outer.fullName}", expected "outer_fn"`);
+    if (outer.children.size !== 1) fail(`outer has ${outer.children.size} children, expected 1`);
+    const mid = [...outer.children.values()][0];
+    if (!mid.fullName.includes("mid_fn")) fail(`child of outer is "${mid.fullName}", expected "mid_fn"`);
+    const leaf = [...mid.children.values()][0];
+    if (!leaf.fullName.includes("leaf_fn")) fail(`child of mid is "${leaf.fullName}", expected "leaf_fn"`);
+    if (leaf.self !== 1) fail(`leaf.self = ${leaf.self}, expected 1 (innermost frame is where the sample lands)`);
+    pass("Inlined frames expand outermost→innermost as parent→child in the flamegraph");
+  }
+
+  // The inline-expansion code must not crash when an address maps to an array
+  // with nullish elements (can happen with sparse SymbolTableEntry events or
+  // when a child inline is resolved before its parent frame).
+  function testFlamegraphInlineTolerantOfNullSlots() {
+    // arr[0] present, arr[1] undefined, arr[2] present. The iteration should
+    // skip the undefined slot rather than creating a "(unknown)" level.
+    const sparse = new Array(3);
+    sparse[0] = { symbol: "outer_fn", location: null };
+    sparse[2] = { symbol: "leaf_fn", location: null };
+    const callframeSymbols = new Map([["0x2000", sparse]]);
+    const samples = [{ callchain: ["0x2000"], workerId: 0 }];
+    const tree = buildFlamegraphTree(samples, callframeSymbols);
+    // Expected: (all) -> outer_fn -> leaf_fn (sparse slot skipped)
+    const outer = [...tree.children.values()][0];
+    if (!outer || !outer.fullName.includes("outer_fn")) fail(`expected outer_fn child, got ${outer && outer.fullName}`);
+    if (outer.children.size !== 1) fail(`outer has ${outer.children.size} children, expected 1 (sparse slot should be skipped)`);
+    const leaf = [...outer.children.values()][0];
+    if (!leaf.fullName.includes("leaf_fn")) fail(`expected leaf_fn after outer_fn, got ${leaf.fullName}`);
+    pass("Sparse inline arrays do not produce phantom tree levels");
+  }
+
+  // An address that is not present in callframeSymbols should still produce
+  // a single-level child using the raw address as the key (so unresolved
+  // traces remain visible rather than collapsing).
+  function testFlamegraphUnknownAddress() {
+    const callframeSymbols = new Map(); // empty — address resolves to undefined
+    const samples = [{ callchain: ["0x3000"], workerId: 0 }];
+    const tree = buildFlamegraphTree(samples, callframeSymbols);
+    if (tree.children.size !== 1) fail(`root has ${tree.children.size} children for single unresolved address`);
+    const node = [...tree.children.values()][0];
+    if (node.self !== 1) fail(`unresolved node.self = ${node.self}, expected 1`);
+    pass("Unresolved addresses still produce a single tree level");
+  }
+
+  // ── TaskDumpEvent parsing (verified against the demo trace) ──
+
+  function testTaskDumpsParsed() {
+    if (!trace.taskDumps) fail("trace.taskDumps should be a Map");
+    if (!(trace.taskDumps instanceof Map)) fail("trace.taskDumps should be an instance of Map");
+    pass(`trace.taskDumps is a Map with ${trace.taskDumps.size} task IDs`);
+  }
+
+  function testTaskDumpsSortedByTimestamp() {
+    // Every value in taskDumps is an array sorted by timestamp — the renderer
+    // relies on this for its O(n) sweep across idle gaps.
+    for (const [tid, dumps] of trace.taskDumps) {
+      for (let i = 1; i < dumps.length; i++) {
+        if (dumps[i].timestamp < dumps[i - 1].timestamp) {
+          fail(`taskDumps for task ${tid} not sorted (index ${i})`);
+        }
+      }
+    }
+    pass("All taskDumps arrays are sorted by timestamp");
+  }
+
+  function testTaskDumpsShape() {
+    // Each dump is {timestamp, callchain} where callchain is an array of hex address strings.
+    for (const [tid, dumps] of trace.taskDumps) {
+      for (const d of dumps) {
+        if (typeof d.timestamp !== "number") fail(`dump.timestamp for task ${tid} is ${typeof d.timestamp}`);
+        if (!Array.isArray(d.callchain)) fail(`dump.callchain for task ${tid} is not an array`);
+        for (const addr of d.callchain) {
+          if (typeof addr !== "string" || !addr.startsWith("0x")) {
+            fail(`dump.callchain entry ${addr} not a hex string`);
+          }
+        }
+        break; // sample one per task is enough
+      }
+    }
+    pass("TaskDumps have expected {timestamp, callchain} shape with hex-string addresses");
+  }
+
+  function testTaskDumpsTaskIdsKnown() {
+    // Every task ID that has a dump should be a known spawned task (no orphans).
+    for (const tid of trace.taskDumps.keys()) {
+      if (!trace.taskSpawnTimes.has(tid)) {
+        fail(`task ${tid} has taskDumps but is not in taskSpawnTimes`);
+      }
+    }
+    pass("All taskDump task IDs refer to tasks that appear in taskSpawnTimes");
+  }
+
   // ── buildSpanData ──
 
   function testBuildSpanDataPairing() {
@@ -822,6 +931,15 @@ async function main() {
   testFlattenFlamegraph();
   testBuildFgData();
   testBuildFgDataEmpty();
+  testFlamegraphInlineOrder();
+  testFlamegraphInlineTolerantOfNullSlots();
+  testFlamegraphUnknownAddress();
+
+  console.log("\ntaskDumps:");
+  testTaskDumpsParsed();
+  testTaskDumpsSortedByTimestamp();
+  testTaskDumpsShape();
+  testTaskDumpsTaskIdsKnown();
 
   console.log("\nbuildSpanData:");
   testBuildSpanDataPairing();

dial9-viewer/ui/trace_analysis.js

@@ -431,10 +431,17 @@
       node.count++;
       for (const addr of chain) {
         const entry = callframeSymbols.get(addr);
-        // Expand inlined frames: an array entry means multiple frames at one address
+        // Expand inlined frames. Per blazesym, an array entry is ordered
+        // [outermost, ..., innermost]: entry[0] is the real function at this
+        // address, and entry[i>0] are inlined callees (entry[0] calls entry[1]
+        // calls entry[2], etc.). To walk the call graph caller→callee while
+        // descending the flamegraph tree, iterate 0 → N. Skip nullish slots
+        // that can appear in sparse arrays (rare, but can happen if inline
+        // SymbolTableEntry events arrive before their depth=0 sibling).
         const frames = Array.isArray(entry) ? entry : [entry];
-        for (let fi = frames.length - 1; fi >= 0; fi--) {
+        for (let fi = 0; fi < frames.length; fi++) {
           const resolved = frames[fi];
+          if (fi > 0 && !resolved) continue;
           const key = resolved ? resolved.symbol : addr || "??";
           const formatted = resolved ? formatFrame(resolved) : formatFrame(addr, callframeSymbols);
           if (!node.children.has(key)) {

dial9-viewer/ui/viewer.html

@@ -2634,6 +2634,25 @@ <h3>⌨ Keyboard</h3>
                     return formatHumanDuration(ns);
                 }
 
+                // Diagonal cross-hatch fill inside the rect (x, y, w, h), clipped.
+                // Used to visually flag idle periods that have an associated task dump.
+                function drawCrossHatch(x, y, w, h, { stroke = "rgba(140,120,255,0.35)", step = 8 } = {}) {
+                    ctx.save();
+                    ctx.beginPath();
+                    ctx.rect(x, y, w, h);
+                    ctx.clip();
+                    ctx.strokeStyle = stroke;
+                    ctx.lineWidth = 1;
+                    ctx.setLineDash([]);
+                    for (let hx = x - h; hx < x + w; hx += step) {
+                        ctx.beginPath();
+                        ctx.moveTo(hx, y + h);
+                        ctx.lineTo(hx + h, y);
+                        ctx.stroke();
+                    }
+                    ctx.restore();
+                }
+
                 const bandTop = 50, bandH = 30;
                 ctx.font = "9px monospace";
                 ctx.textAlign = "center";
@@ -2835,21 +2854,7 @@ <h3>⌨ Keyboard</h3>
 
                     // Cross-hatch pattern for idle periods with task dumps
                     if (hasDump) {
-                        ctx.save();
-                        ctx.beginPath();
-                        ctx.rect(x1, bandTop, w, bandH);
-                        ctx.clip();
-                        ctx.strokeStyle = "rgba(140, 120, 255, 0.35)";
-                        ctx.lineWidth = 1;
-                        ctx.setLineDash([]);
-                        const step = 8;
-                        for (let hx = x1 - bandH; hx < x2; hx += step) {
-                            ctx.beginPath();
-                            ctx.moveTo(hx, bandTop + bandH);
-                            ctx.lineTo(hx + bandH, bandTop);
-                            ctx.stroke();
-                        }
-                        ctx.restore();
+                        drawCrossHatch(x1, bandTop, w, bandH);
                     }
 
                     ctx.strokeStyle = hasDump ? "#7c6cff" : "#444";
@@ -4432,7 +4437,12 @@ <h3>⌨ Keyboard</h3>
                 // Check if clicking an idle region with task dumps
                 for (const r of taskDetailHitRegions) {
                     if (mx >= r.x1 && mx <= r.x2 && my >= r.y1 && my <= r.y2 && r.taskDumps) {
-                        showTaskDumpStack(r.taskDumps);
+                        try {
+                            showTaskDumpStack(r.taskDumps);
+                        } catch (err) {
+                            console.error("showTaskDumpStack error:", err);
+                            showToast("task-dump-err", "Error: " + err.message, "error", 5000);
+                        }
                         return;
                     }
                 }
@@ -4493,21 +4503,24 @@ <h3>⌨ Keyboard</h3>
             const fgContainer = document.getElementById("fg-container");
             const fgInstance = FlamegraphRenderer.createFlamegraph(fgContainer);
 
-            function showTaskDumpStack(dumps) {
-                const samples = dumps.map(d => ({ callchain: d.callchain, workerId: 0 }));
+            // Open the stack sidebar showing a flamegraph built from `samples`.
+            // Used by `showTaskDumpStack`, `showIdleTimeFlamegraph`, and any
+            // other caller that just wants "here's a pile of callchains, render
+            // them as a flamegraph in the sidebar".
+            function renderFlamegraphInSidebar({ title, subtitle, samples }) {
                 fgActive = true;
                 schedActive = false;
                 const sidebar = document.getElementById("stack-sidebar");
-                const title = document.getElementById("stack-sidebar-title");
+                const titleEl = document.getElementById("stack-sidebar-title");
                 const body = document.getElementById("stack-sidebar-body");
                 document.getElementById("sidebar-tabs").style.display = "none";
-                title.textContent = `Waiting on — ${dumps.length} capture${dumps.length > 1 ? "s" : ""}`;
+                titleEl.textContent = title;
                 body.innerHTML = "";
                 body.style.display = "flex";
                 body.style.flexDirection = "column";
                 const actions = document.createElement("div");
                 actions.style.cssText = "display:flex;gap:8px;margin-bottom:6px;flex-shrink:0;align-items:center";
-                actions.innerHTML = `<span style="color:#888;font-size:0.85em">${dumps.length} async stack capture${dumps.length > 1 ? "s" : ""}</span>`;
+                actions.innerHTML = `<span style="color:#888;font-size:0.85em">${subtitle}</span>`;
                 body.appendChild(actions);
                 fgContainer.style.flex = "1";
                 fgContainer.style.minHeight = "0";
@@ -4521,6 +4534,16 @@ <h3>⌨ Keyboard</h3>
                 });
             }
 
+            function showTaskDumpStack(dumps) {
+                const samples = dumps.map(d => ({ callchain: d.callchain, workerId: 0 }));
+                const s = dumps.length > 1 ? "s" : "";
+                renderFlamegraphInSidebar({
+                    title: `Waiting on — ${dumps.length} capture${s}`,
+                    subtitle: `${dumps.length} async stack capture${s}`,
+                    samples,
+                });
+            }
+
             function showIdleTimeFlamegraph() {
                 if (!selectedTaskId || !trace.taskDumps) return;
                 const dumps = trace.taskDumps.get(selectedTaskId);
@@ -4574,30 +4597,10 @@ <h3>⌨ Keyboard</h3>
                     }
                 }
 
-                // Show in sidebar using the flamegraph renderer
-                fgActive = true;
-                schedActive = false;
-                const sidebar = document.getElementById("stack-sidebar");
-                const title = document.getElementById("stack-sidebar-title");
-                const body = document.getElementById("stack-sidebar-body");
-                document.getElementById("sidebar-tabs").style.display = "none";
-                title.textContent = `Idle time flamegraph — ${dumps.length} samples`;
-                body.innerHTML = "";
-                body.style.display = "flex";
-                body.style.flexDirection = "column";
-                const actions = document.createElement("div");
-                actions.style.cssText = "display:flex;gap:8px;margin-bottom:6px;flex-shrink:0;align-items:center";
-                actions.innerHTML = `<span style="color:#888;font-size:0.85em">${dumps.length} task dumps, time-weighted</span>`;
-                body.appendChild(actions);
-                fgContainer.style.flex = "1";
-                fgContainer.style.minHeight = "0";
-                body.appendChild(fgContainer);
-                const wasHidden = sidebar.style.display !== "flex";
-                sidebar.style.display = "flex";
-                if (wasHidden && trace) requestAnimationFrame(renderAll);
-                requestAnimationFrame(() => {
-                    fgInstance.setData(expandedSamples, trace.callframeSymbols);
-                    fgInstance.resize();
+                renderFlamegraphInSidebar({
+                    title: `Idle time flamegraph — ${dumps.length} samples`,
+                    subtitle: `${dumps.length} task dumps, time-weighted`,
+                    samples: expandedSamples,
                 });
             }