Skip to content

Commit a6bfac6

Browse files
committed
fix(webui): the favourites test file has no tsc program checking it
Review feedback on #10: `selectWebuiRailViewTabs` gained a required `starred` parameter, and `rail-buckets.test.ts` still called it with two arguments at four sites. Benign at runtime -- the accesses are `starred?.[id]` -- and invisible to every gate, because `packages/webui/tsconfig.test.json` lists exactly two test files by name and `tsconfig.client.json` covers `src/client/**` but no tests. Nothing in this repo compiles that file, so a required argument could be dropped without a byte going red. The arity errors were the symptom. The same hole was holding thirteen more in this PR's own new test file, of the same shape and mine: `const ACTIVITY = {...}` was left unannotated, so `busyReason: "turn"` widened to `string` and stopped being assignable to the `WebuiActiveTurn["busyReason"]` union. Thirteen call sites were handing a wrongly typed activity map to the filter, and the suite stayed green on a fixture the product could not produce. Annotating the fixture fixes the widening by checking the literal in context. So `rail-buckets.ts:96-102` argues that `starred` is required precisely so a missing argument cannot quietly produce wrong counts -- and at its only consumer that guard was decorative. It is not any more, but only because both files happen to be fixed by hand today; the hole itself is still open. Also corrects a comment that outlived its premise. `shell.css` said "Pinned rows sort to the top, which is a statement about order and not about the row" -- true when the mark was the only pin affordance, and no longer the whole truth now that the row carries one. Worse, the ordering it cites lives only in the project view's `sortWebuiProjectSessionIds`; the running, unread and favourites views sort by activity alone, so inside those a pin buys the mark and nothing else. The comment now says that, which is also why nothing in the rule depends on the sort. Left out on purpose: - **Widening `tsconfig.test.json` to `test/unit/**/*.ts`.** Measured before deciding: 155 errors, of which only 13 were this PR's. The rest are config shape, not defects -- `vite.config.ts` (20, and not this PR's file to touch), `markdown.tsx` and `WorkspacePanels.tsx` (16 each, client files handed a server program's lib and types), plus `third_party/` and `local-runtime/` reached by transitive import. 142 real items after this commit's fixes, spread across 12 other test files, several of them pre-existing debt in files this PR never opened. A blanket include is the wrong shape; the right one is a separate program with client lib/types and a deliberate file list, and that is its own change with its own review. - **Giving child rows a star action.** The trap is real and staged: a child is not in `page.sessions`, so the day `openSessionMenu`'s `isChild` branch grows a star, that favourite becomes unreachable in 收藏 and still counts toward nothing. It is unreachable today because there is no star item on a child menu. Deciding whether the favourites view also walks the tree belongs with the feature, so the precondition is written down in `rail-star.spec.mjs` instead of half-solved here. Validation: webui unit 1335 passed / 9 failed (the same 9 pre-existing Windows ones), `typecheck` and `typecheck:webui` clean, `rail-star.spec.mjs` 18/18, `check:source` 4679 files. The widened-config run above was the check for the type fixes: 155 -> 142, with `rail-star-favorites` and `rail-buckets` at zero.
1 parent aedab1d commit a6bfac6

4 files changed

Lines changed: 35 additions & 8 deletions

File tree

‎packages/webui/src/client/styles/shell.css‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1012,9 +1012,15 @@
10121012
color: var(--text_default_tertiary);
10131013
}
10141014
/* "This row is pinned", drawn by the row itself rather than by the button that
1015-
* toggles it. Pinned rows sort to the top, which is a statement about order
1016-
* and not about the row: it stops being true the moment a fresher session
1017-
* arrives, and it was never visible while reading the list.
1015+
* toggles it, because sort position is not a claim about the row: it stops
1016+
* being true the moment a fresher session arrives, and it was never visible
1017+
* while reading the list.
1018+
*
1019+
* Narrower than it looks: that ordering lives only in the project view's
1020+
* `sortWebuiProjectSessionIds`. The running, unread and favourites views sort
1021+
* by activity alone, so inside those a pin buys the mark and nothing else --
1022+
* which is the point of drawing it, and the reason nothing here depends on
1023+
* the sort.
10181024
*
10191025
* Tertiary grey, inherited from the meta block, on purpose. The two colours
10201026
* already in this cluster each carry a meaning that is not "the user chose

‎packages/webui/test/unit/rail-buckets.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ describe("selectWebuiRailViewTabs", () => {
7272
});
7373

7474
it("counts zero rather than going negative or blank when nothing is running", () => {
75-
const idle = selectWebuiRailViewTabs(SESSIONS, {});
75+
const idle = selectWebuiRailViewTabs(SESSIONS, {}, {});
7676
expect(idle.find((entry) => entry.view === "running")?.count).toBe(0);
7777
expect(idle.find((entry) => entry.view === "unread")?.count).toBe(0);
7878
});
@@ -152,7 +152,7 @@ describe("filterWebuiRailViewSessions", () => {
152152
mvs_gone: { lastActivityAt: 9_999, unread: 7, busy: { turnId: "t9", busyReason: "turn" } },
153153
};
154154
expect(selectWebuiRailViewTabs(SESSIONS, stale, {}).find((e) => e.view === "running")?.count).toBe(2);
155-
expect(selectWebuiRailViewTabs(SESSIONS, stale).find((e) => e.view === "unread")?.count).toBe(1);
155+
expect(selectWebuiRailViewTabs(SESSIONS, stale, {}).find((e) => e.view === "unread")?.count).toBe(1);
156156
expect(ids(filterWebuiRailViewSessions(SESSIONS, stale, "running", {}))).toEqual([
157157
"mvs_c",
158158
"mvs_a",
@@ -295,7 +295,7 @@ describe("the rail view tabs", () => {
295295
// sessions; there are just none of the kind this tab collects, and the
296296
// reader needs to know which kind they emptied.
297297
const labels = new Map(
298-
selectWebuiRailViewTabs(SESSIONS, BUSY).map((entry) => [entry.view, entry.emptyLabel]),
298+
selectWebuiRailViewTabs(SESSIONS, BUSY, {}).map((entry) => [entry.view, entry.emptyLabel]),
299299
);
300300
expect(labels.get("running")).toBe("没有运行中的会话");
301301
expect(labels.get("unread")).toBe("没有未读会话");
@@ -308,7 +308,7 @@ describe("the rail view tabs", () => {
308308
it("keeps no English in a Chinese rail", () => {
309309
// The rail is Chinese throughout. An untranslated string here is how
310310
// "Waiting messages" and "No sessions yet" got shipped in the first place.
311-
for (const entry of selectWebuiRailViewTabs(SESSIONS, BUSY)) {
311+
for (const entry of selectWebuiRailViewTabs(SESSIONS, BUSY, {})) {
312312
expect(entry.label, entry.view).not.toMatch(/[A-Za-z]{2,}/u);
313313
expect(entry.emptyLabel, entry.view).not.toMatch(/[A-Za-z]{2,}/u);
314314
}

‎packages/webui/test/unit/rail-star-favorites.test.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
} from "../../src/client/rail-buckets.js";
1111
import { WebuiIconSessionStar } from "../../src/client/icons.js";
1212
import { WebuiProjectList } from "../../src/client/components/SessionRail.js";
13+
import type { WebuiSessionActivityMap } from "../../src/client/session-activity.js";
1314
import type { WebuiClientProject, WebuiClientSession } from "../../src/client/contracts.js";
1415

1516
/**
@@ -51,7 +52,12 @@ const SESSIONS: readonly WebuiClientSession[] = [
5152
session("mvs_e", 5),
5253
];
5354

54-
const ACTIVITY = {
55+
// Annotated, not inferred. Without it `busyReason: "turn"` widens to `string`
56+
// and stops being assignable to `WebuiSessionActiveTurn["busyReason"]`, which
57+
// is a union -- and nothing in this repo's tsc programs compiles test files, so
58+
// the thirteen call sites below would hand a wrongly typed map to the filter
59+
// and the suite would stay green on a fixture the product could never produce.
60+
const ACTIVITY: WebuiSessionActivityMap = {
5561
mvs_a: { lastActivityAt: 5_000, busy: { turnId: "t1", busyReason: "turn" } },
5662
mvs_b: { lastActivityAt: 9_000, unread: 3 },
5763
mvs_c: { lastActivityAt: 7_000 },

‎test/webui-browser/rail-star.spec.mjs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,21 @@ test("a starred child row shows its mark once the parent is expanded", async ({
357357
await expect(starMark(page, "C")).toBeVisible();
358358
// The parent's own state is untouched by the child's.
359359
await expect(starMark(page, "A")).toHaveCount(0);
360+
361+
// ⚠️ Read this before adding star to a child row's context menu.
362+
//
363+
// `filterWebuiRailViewSessions` filters `page.sessions`, and a child is not
364+
// in `page.sessions` -- it only ever appears in the tree. So the moment
365+
// `openSessionMenu`'s `isChild` branch grows a star item, a starred child
366+
// becomes unreachable in TWO ways at once: it still cannot be starred (no
367+
// menu item today), and if it were, it would carry a mark, sit in localStorage
368+
// and still never appear in 收藏 or count toward its badge.
369+
//
370+
// The fixture is already staged for exactly that: C is deliberately outside
371+
// `sessions`, so the test above can star it and the count test can prove the
372+
// badge reads 0. Whoever wires child starring has to decide whether the
373+
// favourites view also walks the tree, and that decision belongs with the
374+
// feature, not with this spec.
360375
});
361376

362377
test("the context menu offers unstar on a starred row", async ({ page }) => {

0 commit comments

Comments
 (0)