You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "liuxiaocs7 (via GitHub)" <gi...@apache.org> on 2026/08/29 19:45:57 UTC
[PR] fix(desktop): name the session in the usage activity task column [maka]
liuxiaocs7 opened a new pull request, #4219:
URL: https://github.com/apache/maka/pull/4219
## Problem
On the Usage Statistics page (使用统计) → Activity records (活动记录), the **任务 (Task)** column shows `未命名会话 · <short id>` ("Untitled session") for **every** row, even when the session has a real title in the WorkHub.
This is a regression. #3833 fixed exactly this column by carrying `SessionHeader.name` through the old `packages/storage/src/usage-stats-store.ts` pipeline. #3697 then moved usage onto the Runtime Host projection and #4060 deleted that store. The new pipeline (`usage-pricing-coordinator.ts` → protocol `LlmUsageLogProjection`/`ToolUsageLogProjection` → `runtime-host-usage-ipc-main.ts`) only carries `sessionId`, never the session name. So `UsageRequestLog.sessionName` stayed declared-but-never-populated, and the UI's `usageSessionDisplayLabel` always hit the untitled fallback.
## Fix
Recover the title in the desktop main process — a `sessionId → name` join, the same thing the deleted store used to do, relocated to the surviving pipeline:
- `loadSessionNames()` reads the Host session catalog via the existing `client.listSessions()` and builds a `sessionId → name` map (non-empty titles only). A catalog read failure degrades to an empty map (rows fall back to the untitled label) instead of failing the whole page — mirroring the tolerance `loadUsageStats` already applies to summary/row disagreement.
- `loadUsageStats` fetches the map in parallel with the existing usage/pricing reads; `projectLlmLog`/`projectToolLog` attach it as `sessionName`.
Deliberately **not** touched: the Host wire protocol / `usage-pricing-coordinator.ts`, and the renderer — `usageSessionDisplayLabel` already reads `row.sessionName` and does the untitled fallback (from #3833).
## Why the main-process layer
Raw usage rows (`UsageLogRow`) are keyed only by `sessionId`; the title lives in the session catalog. The main process already holds `client.listSessions()` (id + name), and `onOpenSession(row.sessionId)` already opens the matching session — so `row.sessionId` shares the catalog id namespace and the join is safe. Projected ids stay verbatim for normal ids (`projectIdentity` only hashes values over 1024 bytes or with control chars), so the join key survives the wire.
## Verification
- `tsc -p tsconfig.main.json` (desktop main) type-checks clean.
- `node --test` on `runtime-host-usage-ipc-main.test.js` — 7/7 pass, including two new cases:
- names each row by joining its session against the catalog (model + tool rows; a session absent from the catalog stays nameless);
- the page still renders (nameless rows) when the catalog read throws.
- `biome check` clean on the changed files.
- The e2e usage fixture (`apps/desktop/src/main/e2e-fixture/scenarios-usage.ts`) already seeds named sessions with usage keyed by the same `session.id`, so the Task column now resolves those real names end-to-end.
Closes #4218
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): name the session in the usage activity task column [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4219:
URL: https://github.com/apache/maka/pull/4219#discussion_r3889725779
##########
packages/runtime-host/src/server/usage-pricing-coordinator.ts:
##########
@@ -75,18 +75,49 @@ export class HostUsagePricingCoordinator {
readonly #requestDrain: () => void;
readonly #activation: RuntimePolicyActivationGate;
readonly #onCommittedPricingMutation: () => void;
+ // Resolves a session's human-readable title for the Task column. Reads the
+ // durable session header directly (unfiltered, in-process), so it covers
+ // reserved-role, coordination, and legacy sessions the catalog omits.
+ readonly #readSessionTitle?: (sessionId: string) => Promise<string | undefined>;
#poisonDrainRequested = false;
constructor(
stores: InteractiveUsageStoresWriter,
requestDrain: () => void,
activation: RuntimePolicyActivationGate,
onCommittedPricingMutation: () => void = () => {},
+ readSessionTitle?: (sessionId: string) => Promise<string | undefined>,
) {
this.#stores = authenticateInteractiveUsageStoresWriter(stores);
this.#requestDrain = requestDrain;
this.#activation = activation;
this.#onCommittedPricingMutation = onCommittedPricingMutation;
+ this.#readSessionTitle = readSessionTitle;
+ }
+
+ // Resolve titles for exactly the sessions on this page. A session we cannot
+ // read (deleted, or a shape this reader rejects) is simply left untitled — one
+ // unreadable session never blanks the rest.
+ async #resolveSessionTitles(
+ rows: ReadonlyArray<{ readonly sessionId?: string }>,
+ ): Promise<ReadonlyMap<string, string>> {
+ const titles = new Map<string, string>();
+ const read = this.#readSessionTitle;
+ if (!read) return titles;
+ const ids = [
+ ...new Set(rows.map((row) => row.sessionId).filter((id): id is string => id !== undefined)),
+ ];
+ await Promise.all(
+ ids.map(async (id) => {
+ try {
+ const title = (await read(id))?.trim();
+ if (title) titles.set(id, title);
+ } catch {
Review Comment:
[P2] Suggestion (category ② — reasonable storage failure/recovery): could we catch only the explicit Session-not-found case here? `readHeaderSnapshot()` can also fail because the metadata store is closing, SQLite I/O failed, or a durable header is malformed. Swallowing every error makes `usage.query` return success with an untitled label, so the outer `#mapReadFailure()` never emits `host_draining`/`persistence_failed` and the Desktop loses its normal reconnect path. I suggest tolerating `isSessionNotFoundError(error)` while rethrowing lifecycle/persistence/invalid-document failures, with one missing-Session case and one propagated storage-failure regression. Please push back if all title reads are deliberately specified as non-authoritative best effort, including store lifecycle failures.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): name the session in the usage activity task column [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4219:
URL: https://github.com/apache/maka/pull/4219#discussion_r3891109386
##########
packages/runtime-host/src/__tests__/usage-pricing-two-client-uds.test.ts:
##########
@@ -295,6 +299,105 @@ test('pricing root identity failure requests drain while expected failures do no
});
});
+test('usage logs carry the Host-resolved session title and tolerate unreadable sessions', async () => {
+ await withUsageAuthority('session-title', async ({ stores }) => {
+ await Promise.all([
+ stores.telemetry.recordLlmCall({
+ ...usageRecord('llm-named', 10, 'openai', 'gpt-a'),
+ sessionId: 'session-named',
+ turnId: 'turn-1',
+ }),
+ stores.telemetry.recordLlmCall({
+ ...usageRecord('llm-blank', 11, 'openai', 'gpt-a'),
+ sessionId: 'session-blank',
+ turnId: 'turn-2',
+ }),
+ stores.telemetry.recordLlmCall({
+ ...usageRecord('llm-missing', 12, 'openai', 'gpt-a'),
+ sessionId: 'session-missing',
+ turnId: 'turn-3',
+ }),
+ stores.telemetry.recordToolInvocation({
+ ...toolRecord('tool-a', 13),
+ sessionId: 'session-named',
+ }),
+ ]);
+ const titles = new Map([
+ // Leading/trailing whitespace must be trimmed; a blank title is not a title.
+ ['session-named', ' 重构使用统计页请求日志的任务列 '],
+ ['session-blank', ' '],
+ ]);
+ const coordinator = new HostUsagePricingCoordinator(
+ stores,
+ () => {},
+ new RuntimePolicyActivationGate(),
+ () => {},
+ async (sessionId) => {
+ // A genuinely missing session is tolerated: its row stays untitled.
+ if (sessionId === 'session-missing') throw new SessionNotFoundError('session-missing');
+ return titles.get(sessionId);
+ },
+ );
+
+ const llm = await coordinator.handlers['usage.query'](
+ { kind: 'logs', source: 'llm', query: { range: 'all' }, offset: 0, limit: 100 },
+ CONNECTION_CONTEXT,
+ );
+ assert.ok(llm.ok);
+ assert.equal(llm.result.kind, 'logs');
+ if (llm.result.kind !== 'logs' || llm.result.source !== 'llm')
+ throw new Error('expected llm logs');
+ const llmById = new Map(llm.result.rows.map((row) => [row.id, row]));
+ assert.equal(llmById.get('llm-named')?.sessionTitle, '重构使用统计页请求日志的任务列');
+ // Whitespace-only title is dropped; the row stays untitled and the UI falls back.
+ assert.equal(llmById.get('llm-blank')?.sessionTitle, undefined);
+ // A genuinely missing session is tolerated — one missing session never blanks the rest.
+ assert.equal(llmById.get('llm-missing')?.sessionTitle, undefined);
+ assert.equal(llmById.get('llm-named')?.sessionId, 'session-named');
+
+ const tool = await coordinator.handlers['usage.query'](
+ { kind: 'logs', source: 'tool', query: { range: 'all' }, offset: 0, limit: 100 },
+ CONNECTION_CONTEXT,
+ );
+ assert.ok(tool.ok);
+ if (tool.result.kind !== 'logs' || tool.result.source !== 'tool')
+ throw new Error('expected tool logs');
+ assert.equal(
+ tool.result.rows.find((row) => row.id === 'tool-a')?.sessionTitle,
+ '重构使用统计页请求日志的任务列',
+ );
+ });
+});
+
+test('a store failure while resolving a session title propagates as a reconnectable read failure', async () => {
+ await withUsageAuthority('session-title-failure', async ({ stores }) => {
+ await stores.telemetry.recordLlmCall({
+ ...usageRecord('llm-draining', 20, 'openai', 'gpt-a'),
+ sessionId: 'session-live',
+ turnId: 'turn-1',
+ });
+ const coordinator = new HostUsagePricingCoordinator(
+ stores,
+ () => {},
+ new RuntimePolicyActivationGate(),
+ () => {},
+ // Not a missing session — the metadata store is draining/closed. This must
+ // not be swallowed into an untitled row; it has to reach the Desktop's
+ // normal reconnect path via #mapReadFailure.
+ async () => {
+ throw new InteractiveUsageStoresClosedError();
Review Comment:
Thanks for tightening the catch to the real not-found case. [P3] (category ② — storage failure/recovery) This regression injects `InteractiveUsageStoresClosedError` into the session-title callback, but the production callback is `SessionStore.readHeaderSnapshot`; it cannot emit a usage-store lifecycle error. The test therefore proves the coordinator classifier with a fabricated cross-store error, not that an actual session-metadata failure reaches the reconnectable path. Could we replace this case, rather than add another test, with the real execution-store/SessionStore failure at the composition boundary, or narrow the test name and claim to what it actually covers? Please push back if there is an adapter that translates production title-read failures to this exact type.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): name the session in the usage activity task column [maka]
Posted by "liuxiaocs7 (via GitHub)" <gi...@apache.org>.
liuxiaocs7 commented on code in PR #4219:
URL: https://github.com/apache/maka/pull/4219#discussion_r3890560823
##########
packages/runtime-host/src/server/usage-pricing-coordinator.ts:
##########
@@ -75,18 +75,49 @@ export class HostUsagePricingCoordinator {
readonly #requestDrain: () => void;
readonly #activation: RuntimePolicyActivationGate;
readonly #onCommittedPricingMutation: () => void;
+ // Resolves a session's human-readable title for the Task column. Reads the
+ // durable session header directly (unfiltered, in-process), so it covers
+ // reserved-role, coordination, and legacy sessions the catalog omits.
+ readonly #readSessionTitle?: (sessionId: string) => Promise<string | undefined>;
#poisonDrainRequested = false;
constructor(
stores: InteractiveUsageStoresWriter,
requestDrain: () => void,
activation: RuntimePolicyActivationGate,
onCommittedPricingMutation: () => void = () => {},
+ readSessionTitle?: (sessionId: string) => Promise<string | undefined>,
) {
this.#stores = authenticateInteractiveUsageStoresWriter(stores);
this.#requestDrain = requestDrain;
this.#activation = activation;
this.#onCommittedPricingMutation = onCommittedPricingMutation;
+ this.#readSessionTitle = readSessionTitle;
+ }
+
+ // Resolve titles for exactly the sessions on this page. A session we cannot
+ // read (deleted, or a shape this reader rejects) is simply left untitled — one
+ // unreadable session never blanks the rest.
+ async #resolveSessionTitles(
+ rows: ReadonlyArray<{ readonly sessionId?: string }>,
+ ): Promise<ReadonlyMap<string, string>> {
+ const titles = new Map<string, string>();
+ const read = this.#readSessionTitle;
+ if (!read) return titles;
+ const ids = [
+ ...new Set(rows.map((row) => row.sessionId).filter((id): id is string => id !== undefined)),
+ ];
+ await Promise.all(
+ ids.map(async (id) => {
+ try {
+ const title = (await read(id))?.trim();
+ if (title) titles.set(id, title);
+ } catch {
Review Comment:
Applied — thanks, this is the right call. The title read is best-effort **only for a genuinely missing session**, not for store failures.
`#resolveSessionTitles` now tolerates just `isSessionNotFoundError(error)` and rethrows everything else, so a draining/closed store, SQLite I/O, or a malformed durable header propagates through `#queryUsage` → `#mapReadFailure()` and surfaces as `host_draining` / `persistence_failed` — the Desktop keeps its normal reconnect path instead of a spurious untitled row.
Covered both ways in `usage-pricing-two-client-uds`:
- the existing missing-session row now throws a real `SessionNotFoundError` and stays tolerated (untitled), and
- a new case whose reader throws `InteractiveUsageStoresClosedError` asserts `usage.query` fails with `host_draining` (dropping the `isSessionNotFoundError` guard fails it).
Pushed as `d77975feb` (also rebased onto latest main).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): name the session in the usage activity task column [maka]
Posted by "liuxiaocs7 (via GitHub)" <gi...@apache.org>.
liuxiaocs7 commented on code in PR #4219:
URL: https://github.com/apache/maka/pull/4219#discussion_r3891199468
##########
packages/runtime-host/src/__tests__/usage-pricing-two-client-uds.test.ts:
##########
@@ -295,6 +299,105 @@ test('pricing root identity failure requests drain while expected failures do no
});
});
+test('usage logs carry the Host-resolved session title and tolerate unreadable sessions', async () => {
+ await withUsageAuthority('session-title', async ({ stores }) => {
+ await Promise.all([
+ stores.telemetry.recordLlmCall({
+ ...usageRecord('llm-named', 10, 'openai', 'gpt-a'),
+ sessionId: 'session-named',
+ turnId: 'turn-1',
+ }),
+ stores.telemetry.recordLlmCall({
+ ...usageRecord('llm-blank', 11, 'openai', 'gpt-a'),
+ sessionId: 'session-blank',
+ turnId: 'turn-2',
+ }),
+ stores.telemetry.recordLlmCall({
+ ...usageRecord('llm-missing', 12, 'openai', 'gpt-a'),
+ sessionId: 'session-missing',
+ turnId: 'turn-3',
+ }),
+ stores.telemetry.recordToolInvocation({
+ ...toolRecord('tool-a', 13),
+ sessionId: 'session-named',
+ }),
+ ]);
+ const titles = new Map([
+ // Leading/trailing whitespace must be trimmed; a blank title is not a title.
+ ['session-named', ' 重构使用统计页请求日志的任务列 '],
+ ['session-blank', ' '],
+ ]);
+ const coordinator = new HostUsagePricingCoordinator(
+ stores,
+ () => {},
+ new RuntimePolicyActivationGate(),
+ () => {},
+ async (sessionId) => {
+ // A genuinely missing session is tolerated: its row stays untitled.
+ if (sessionId === 'session-missing') throw new SessionNotFoundError('session-missing');
+ return titles.get(sessionId);
+ },
+ );
+
+ const llm = await coordinator.handlers['usage.query'](
+ { kind: 'logs', source: 'llm', query: { range: 'all' }, offset: 0, limit: 100 },
+ CONNECTION_CONTEXT,
+ );
+ assert.ok(llm.ok);
+ assert.equal(llm.result.kind, 'logs');
+ if (llm.result.kind !== 'logs' || llm.result.source !== 'llm')
+ throw new Error('expected llm logs');
+ const llmById = new Map(llm.result.rows.map((row) => [row.id, row]));
+ assert.equal(llmById.get('llm-named')?.sessionTitle, '重构使用统计页请求日志的任务列');
+ // Whitespace-only title is dropped; the row stays untitled and the UI falls back.
+ assert.equal(llmById.get('llm-blank')?.sessionTitle, undefined);
+ // A genuinely missing session is tolerated — one missing session never blanks the rest.
+ assert.equal(llmById.get('llm-missing')?.sessionTitle, undefined);
+ assert.equal(llmById.get('llm-named')?.sessionId, 'session-named');
+
+ const tool = await coordinator.handlers['usage.query'](
+ { kind: 'logs', source: 'tool', query: { range: 'all' }, offset: 0, limit: 100 },
+ CONNECTION_CONTEXT,
+ );
+ assert.ok(tool.ok);
+ if (tool.result.kind !== 'logs' || tool.result.source !== 'tool')
+ throw new Error('expected tool logs');
+ assert.equal(
+ tool.result.rows.find((row) => row.id === 'tool-a')?.sessionTitle,
+ '重构使用统计页请求日志的任务列',
+ );
+ });
+});
+
+test('a store failure while resolving a session title propagates as a reconnectable read failure', async () => {
+ await withUsageAuthority('session-title-failure', async ({ stores }) => {
+ await stores.telemetry.recordLlmCall({
+ ...usageRecord('llm-draining', 20, 'openai', 'gpt-a'),
+ sessionId: 'session-live',
+ turnId: 'turn-1',
+ });
+ const coordinator = new HostUsagePricingCoordinator(
+ stores,
+ () => {},
+ new RuntimePolicyActivationGate(),
+ () => {},
+ // Not a missing session — the metadata store is draining/closed. This must
+ // not be swallowed into an untitled row; it has to reach the Desktop's
+ // normal reconnect path via #mapReadFailure.
+ async () => {
+ throw new InteractiveUsageStoresClosedError();
Review Comment:
Good catch — you're right, and there's no such adapter. `SessionStore.readHeaderSnapshot`'s only usage-relevant failures are `SessionNotFoundError` (tolerated) or, for a closed metadata store, a plain `Error('SQLite session metadata store is closed')`. That's `unknown` to `classifyInteractiveUsageStoresFailure`, so `#mapReadFailure` rethrows it — it never becomes `host_draining`. My `InteractiveUsageStoresClosedError` case was asserting a cross-store mapping the production reader can't reach.
Replaced (not added) that case in `e415a1aff`: it now injects the exact generic error a closed metadata store throws and asserts `usage.query` **propagates/rejects** rather than returning a false-success untitled page, with the name + comment narrowed to that claim (no `host_draining`).
On the reconnect path: a genuinely draining host is already caught by the *primary* usage read (`telemetry.logs`/canonical), which runs before title resolution and emits `host_draining` itself. The title read only reaches this branch when the session store fails on its own — there the fix's job is just to surface it, not mask it, which the narrowed test proves. Missing-session tolerance is unchanged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): name the session in the usage activity task column [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han merged PR #4219:
URL: https://github.com/apache/maka/pull/4219
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org