Files
software-mansion__argent/packages/tool-server/test/profiler-commit-query-by-index.test.ts
filip131311 8b3a8851a2 fix(profiler): honour top_n in profiler-commit-query by_index (#667)
Fixes #630.

## The bug

`by_index` never received `top_n`: `renderByIndex` had no such parameter
and the dispatch passed only `(commits, commit_index)`. Reproduced
against the released 0.18.1 CLI on a real session — `top_n: 8` returned
**306 rows / 20,245 characters**.

It matters because `react-profiler-analyze` points at this exact mode
for drilling into a slow commit, so it is both the most likely call and
the most likely to blow an agent's context.

## Correction to the issue (and to my own first pass)

The issue says "the other three modes honour `top_n` correctly". That's
not right — **`cascade_tree` ignores it too** (`renderCascadeTree` takes
no `topN`; its dispatch also passes two arguments). Only `by_component`
and `by_time_range` slice.

I've deliberately **not** fixed `cascade_tree` here, and not for the
reason that first looked right. A DFS-prefix budget would in fact be
structurally safe there. The real problems are that its roots come from
a `Set` in insertion order with no subtree-cost aggregation, so any cut
keeps an *arbitrary* subtree rather than the expensive one — making it
useful needs a cost-ranking pass, which is design work, not a slice. And
it dedups by `name:depth`, so it never had the 306-row blow-up that
motivated this issue. Filed separately with the other findings.

## Why `top_n` is honoured only when explicitly passed

Applying the schema's existing default of 20 looks obviously right and
is a trap. These rows are **fibers, not components**, and they collapse
hard. Measured on the real commit:

| top_n | distinct components covered |
|---|---|
| 8 | 3 |
| 20 | **10** |
| 50 | 18 |
| (all 306 fibers) | 70 |

The analyze report that recommends this mode already shows up to **15**
distinct components (`MAX_COMPONENT_ENTRIES`). So a default of 20 would
make the escape hatch return *strictly less than the report it escapes
from* — silently, since the footer would only say "298 hidden", not "you
learned nothing new".

`top_n` is therefore `.optional()`, and the two modes that genuinely
defaulted to 20 keep it explicitly at their call sites (`params.top_n ??
20`), so their behaviour is byte-identical.

Since that collapse is invisible from a row count, the footer states it
outright:

```
_Showing the 8 costliest of 306 fibers — 298 cheaper fibers hidden. Note these are
fibers, not components: the full set covers 70 distinct components and this view
covers 3. Omit top_n for everything._
```

## The trap this had to avoid

The root-cause chain block scans the **full** commit, not the truncated
table. The fiber carrying the root cause is typically *cheap* — it
triggers the cascade rather than doing the work — so it falls outside
any `top_n`. Slicing before that lookup would silently drop the single
most useful line in the output. There's a regression test that pins
exactly this: the only fiber with a `rootCauseParent` is the cheapest,
`top_n=5`, and the chain must still be printed.

## Also fixed

The analyze report suggested `` `profiler-commit-query mode=by_index` to
see all `` **without passing `commit_index`** — an agent following it
verbatim queries a different commit than the one being described. Now
names the commit.

Note I did *not* change it to name a `top_n` value:
`totalComponentCount` counts distinct component names, while `by_index`
emits one row per fiber, so passing it would still truncate — a subtler
falsehood inside the line being corrected. With no default cap, "to see
all" is simply true again.

## Verified live

| call | before | after |
|---|---|---|
| `by_index top_n=8` | 306 rows, 20,245 chars | 8 rows, 898 chars,
heading `Top Fibers`, `**Fibers:** 306 (showing 8)` |
| `by_index` (no top_n) | 306 rows | 306 rows, heading `Full Detail`, no
notice |
| `by_component` | — | unchanged |

## Checks

- 3093 tests pass; 7 new. Note the A/B against the pre-fix source is not
meaningful for this file — `renderByIndex` wasn't exported, so those
failures are import errors, not behavioural. The real before/after is
the live table above, captured against the released 0.18.1 build.
- extract-tools 46/46; SpiderShield average 9.10 (tool `description:`
untouched); prettier, eslint, both typechecks clean; lock untouched.
- Touches `renderByIndex` only — no overlap with PR #663, which owns
`renderByComponent` in the same file.
2026-08-11 17:18:11 +02:00

98 lines
3.7 KiB
TypeScript

import { describe, it, expect } from "vitest";
import { renderByIndex } from "../src/tools/profiler/query/profiler-commit-query";
import type { DevToolsFiberCommit } from "../src/utils/react-profiler/types/input";
function fiber(
componentName: string,
actualDuration: number,
extra: Partial<DevToolsFiberCommit> = {}
): DevToolsFiberCommit {
return {
commitIndex: 1,
timestamp: 2868,
commitDuration: 18.7,
componentName,
actualDuration,
selfDuration: actualDuration / 2,
didRender: true,
changeDescription: null,
...extra,
} as DevToolsFiberCommit;
}
/** 30 fibers spanning 10 distinct component names, descending in cost. */
function commit(): DevToolsFiberCommit[] {
const out: DevToolsFiberCommit[] = [];
for (let i = 0; i < 30; i++) {
out.push(fiber(`Comp${i % 10}`, 30 - i));
}
return out;
}
describe("profiler-commit-query by_index — top_n", () => {
it("returns every fiber when top_n is not given", () => {
// by_index is the drill-down the analyze report points at for a full
// breakdown, so an unrequested cap would make it show less than the report.
const out = renderByIndex(commit(), 1);
expect(out.split("\n").filter((l) => l.startsWith("| `")).length).toBe(30);
expect(out).toContain("Full Detail");
expect(out).not.toContain("cheaper fibers hidden");
});
it("caps the table when top_n is given, keeping the costliest fibers", () => {
const out = renderByIndex(commit(), 1, 8);
const rows = out.split("\n").filter((l) => l.startsWith("| `"));
expect(rows.length).toBe(8);
// Sorted by actualDuration desc, so the retained rows are 30..23.
expect(rows[0]).toContain("| 30.0 |");
expect(rows[7]).toContain("| 23.0 |");
});
it("reports the true fiber total alongside the shown count", () => {
const out = renderByIndex(commit(), 1, 8);
expect(out).toContain("**Fibers:** 30 (showing 8)");
// The heading must stop claiming full detail once it is not full.
expect(out).not.toContain("Full Detail");
});
it("says how many distinct components the truncated view actually covers", () => {
// The trap this guards: 8 fibers can be far fewer than 8 components, so a
// caller cannot infer coverage from the row count.
const out = renderByIndex(commit(), 1, 8);
expect(out).toContain("22 cheaper fibers hidden");
expect(out).toContain("10 distinct components");
expect(out).toContain("Omit top_n for everything");
});
it("keeps the root-cause chain when its fiber is too cheap to make the cut", () => {
// The fiber carrying the root cause is usually cheap — it triggers the
// cascade rather than doing the work — so it falls outside any top_n.
// Scanning the truncated list instead of the full commit would silently
// drop the single most useful line in this output.
const commits = [
...commit(),
fiber("TinyTrigger", 0.01, {
rootCauseParent: "ParentThatChanged",
rootCauseChain: ["Root", "Middle"],
rootCauseReason: "props",
}),
];
const out = renderByIndex(commits, 1, 5);
expect(out).toContain("**Root cause chain:**");
expect(out).toContain("ParentThatChanged");
// ...and the cheap fiber itself is indeed not in the table.
expect(out.split("\n").filter((l) => l.startsWith("| `TinyTrigger`")).length).toBe(0);
});
it("adds no truncation notice when top_n exceeds the fiber count", () => {
const out = renderByIndex(commit(), 1, 500);
expect(out).not.toContain("cheaper fibers hidden");
expect(out).toContain("Full Detail");
expect(out).not.toContain("(showing");
});
it("leaves the not-found path alone", () => {
expect(renderByIndex(commit(), 99, 5)).toBe("_Commit #99 not found in stored data._");
});
});