review: address ds-review-bot findings on session-stats
- stateVersion starts at 1: the package is new, no persisted rows predate it - pendingCalls pairs by own key so a provider callId naming a prototype property cannot fold toolMs to NaN on an unmatched crash-recovery result - StatsLine folds the window fallback only when no sessionStats value is served, and gates the token group on actual token activity instead of steps, so failed-only sessions drop the zero-token group - correct the crash-step counting semantics in the README and Agent Note: recovery closes interrupted steps with a synthetic step/end on reload - reword the window-scoped alternative as a plain rejected option and name isTokenDelta's home beside the StreamChunk type
This commit is contained in:
@@ -6,7 +6,7 @@ import { isTokenDelta } from '@deepseek-ai/dsh-llm/message'
|
||||
import type { SessionEvent } from '@deepseek-ai/dsh-session/types'
|
||||
import type { AssistantTiming } from './conversation.ts'
|
||||
|
||||
// The first-token predicate lives with the StreamChunk vocabulary in dsh-llm;
|
||||
// The first-token predicate lives beside the StreamChunk type in dsh-llm;
|
||||
// re-exported here so Chat Definitions keep their client-runtime import.
|
||||
export { isTokenDelta } from '@deepseek-ai/dsh-llm/message'
|
||||
|
||||
|
||||
@@ -162,12 +162,13 @@ export interface StatsLineProps {
|
||||
|
||||
export const StatsLine = memo(function StatsLine({ useSession, useProjection, t }: StatsLineProps) {
|
||||
const settledNodes = useSession(s => s.chat.legacy.nodes)
|
||||
const windowStats = useMemo(() => deriveStats(settledNodes), [settledNodes])
|
||||
const usage = useProjection('tokenUsage')
|
||||
// Every figure rides the durable sessionStats projection, so paging and
|
||||
// compaction cannot change any of them; an assembly without the unit falls
|
||||
// back to the window-scoped fold wholesale (same field names).
|
||||
const stats = useProjection('sessionStats') ?? windowStats
|
||||
// back to the window-scoped fold wholesale (same field names), paid only
|
||||
// while no projection value is served.
|
||||
const projected = useProjection('sessionStats')
|
||||
const stats = useMemo(() => projected ?? deriveStats(settledNodes), [projected, settledNodes])
|
||||
// Pipe-separated groups (figma stats strip); a group with no data drops out whole.
|
||||
const groups: string[] = []
|
||||
if (stats.steps > 0) {
|
||||
@@ -190,9 +191,11 @@ export const StatsLine = memo(function StatsLine({ useSession, useProjection, t
|
||||
// Context occupancy deliberately lives on the composer's ContextMeter ring,
|
||||
// not here — one home per fact.
|
||||
// Billing rides the durable projection, so these survive paging and
|
||||
// compaction. Suppress the empty projection on a brand-new session.
|
||||
// compaction. Gated on actual token activity: a session whose steps all
|
||||
// settled without billing (e.g. every request failed) shows its counts
|
||||
// without a zero-token group.
|
||||
if (usage !== undefined
|
||||
&& (stats.steps > 0 || billedInputTokens(usage) > 0 || usage.outputTokens > 0)) {
|
||||
&& (billedInputTokens(usage) > 0 || usage.outputTokens > 0)) {
|
||||
const cacheHit = cacheHitPercent(usage)
|
||||
if (cacheHit !== null) groups.push(t('stats.cacheHit', { percent: cacheHit }))
|
||||
groups.push(t('stats.tokens', {
|
||||
|
||||
@@ -314,6 +314,17 @@ describe('StatsLine', () => {
|
||||
expect(view.container.textContent).toBe('')
|
||||
})
|
||||
|
||||
it('hides the zero-token group when steps closed without any billed activity', () => {
|
||||
// A session whose only turn failed before billing (e.g. an auth error):
|
||||
// the counts group renders alone, not an uninformative zero-token group.
|
||||
const { source } = makeSource()
|
||||
const view = render(<StatsLine {...props(source, {
|
||||
tokenUsage: { uncachedInputTokens: 0, outputTokens: 0, cacheReadTokens: 0, cacheWriteTokens: 0 },
|
||||
sessionStats: sessionStats({ turns: 1, steps: 1 }),
|
||||
})} />)
|
||||
expect(view.container.textContent).toBe('1 turns · 1 steps')
|
||||
})
|
||||
|
||||
it('keeps the counts group over an empty visible window when the projection carries totals', () => {
|
||||
// Extends the durable-groups guarantee: full-session counts survive a
|
||||
// window that compaction (or paging) left without assistant nodes.
|
||||
|
||||
@@ -2,5 +2,5 @@
|
||||
# side as of the last confirmed-consistent state. Both languages carry equal authority;
|
||||
# after editing either side, bring the other along and re-record with:
|
||||
# pnpm run verify-translation-pairing --write packages/session/session-stats/README.md
|
||||
README.md: a42d5b1b6efe8ff522a43431f1ccb68b71607876
|
||||
README.zh.md: 5929280a8a619fb37a6f6468fb839962702a68c3
|
||||
README.md: 81b0de17e335b67936afd6fb11f15411beee3b76
|
||||
README.zh.md: 606628ea09bc34203a5374e49db62c1646293846
|
||||
|
||||
@@ -33,7 +33,7 @@ None; the plugin never assembles or sends provider requests.
|
||||
|
||||
## Known Limitations and Deferred Work
|
||||
|
||||
- **Steps count work attempted, not visible output** — a step that failed before producing any visible content still closed with `step/end` and counts; a step truncated by a crash between `step/start` and `step/end` does not.
|
||||
- **Steps count work attempted, not visible output** — a step that failed before producing any visible content still closed with `step/end` and counts; a step interrupted by a crash counts after the session reloads, when crash recovery appends its synthetic `step/end` (`interruptedTurnClosers` in dsh-session).
|
||||
- **A cancelled step is counted but untimed** — no assistant message assembles, so its partial stream time enters no wall-time figure, matching the window fold's untimed interrupted node; a max-tokens usage-host message conversely contributes model time the surface does not show.
|
||||
- **Counts are log-scoped, not surface-scoped** — steps whose messages were later compacted away stay counted; the figures describe the whole session, not the current model-visible surface.
|
||||
- **Mounted only in the web-app bundle** — other assemblies serve no `sessionStats` key, and their consumers fall back to window-scoped counting (the web stats strip's fallback path).
|
||||
|
||||
@@ -33,7 +33,7 @@
|
||||
|
||||
## 已知局限与延后工作
|
||||
|
||||
- **步数统计的是已发生的工作,而非可见输出**——在产生任何可见内容前就失败的步仍以 `step/end` 关闭并计入;进程崩溃恰好截断在 `step/start` 与 `step/end` 之间的步不计。
|
||||
- **步数统计的是已发生的工作,而非可见输出**——在产生任何可见内容前就失败的步仍以 `step/end` 关闭并计入;被崩溃打断的步在会话重新加载后计入,届时崩溃恢复为其补写合成的 `step/end`(dsh-session 的 `interruptedTurnClosers`)。
|
||||
- **被取消的步计数但不计时**——没有组装出 assistant 消息,其部分流式时间不进入任何墙钟数字,与窗口折叠的无计时 interrupted 节点一致;反之 max-tokens 的 usage 宿主消息贡献 surface 上看不到的模型时间。
|
||||
- **计数是日志口径,不是 surface 口径**——消息后来被压缩掉的步仍然计入;数字描述整个会话,而非当前模型可见 surface。
|
||||
- **仅挂载于 web-app bundle**——其他装配不提供 `sessionStats` 键,其消费者回退到窗口口径计数(Web 统计条的回退路径)。
|
||||
|
||||
@@ -140,8 +140,12 @@ export const sessionStatsProjectionDefinition: ProjectionDefinition<'sessionStat
|
||||
case 'tool/call':
|
||||
return { ...state, pendingCalls: { ...state.pendingCalls, [event.data.callId]: event.time } }
|
||||
case 'tool/result': {
|
||||
// Own-key check: callId is provider-minted (model/tool JSON boundary),
|
||||
// so a prototype property name ('constructor', 'toString') on a result
|
||||
// with no recorded call must read as unmatched, not as an inherited
|
||||
// function that would poison toolMs with NaN.
|
||||
const callId = event.data.message.source.callId
|
||||
const dispatched = state.pendingCalls[callId]
|
||||
const dispatched = Object.hasOwn(state.pendingCalls, callId) ? state.pendingCalls[callId] : undefined
|
||||
if (dispatched === undefined) return state
|
||||
const pendingCalls = Object.fromEntries(
|
||||
Object.entries(state.pendingCalls).filter(([id]) => id !== callId),
|
||||
@@ -175,5 +179,5 @@ export const sessionStatsProjectionDefinition: ProjectionDefinition<'sessionStat
|
||||
decodeMs: state.decodeMs,
|
||||
decodeTokens: state.decodeTokens,
|
||||
}),
|
||||
stateVersion: 2,
|
||||
stateVersion: 1,
|
||||
}
|
||||
|
||||
@@ -237,6 +237,27 @@ describe('sessionStats wall-time fold (controlled timestamps)', () => {
|
||||
expect(pruned).toEqual(totals({ turns: 1, steps: 1 }))
|
||||
})
|
||||
|
||||
it('pairs only own pendingCalls keys: a prototype-name callId without a recorded call stays unmatched', () => {
|
||||
const result = (callId: string): unknown =>
|
||||
({ turn: 1, step: 1, message: { source: { kind: 'tool', callId } } })
|
||||
// Crash recovery (TOOL_NOT_STARTED) emits results with no preceding
|
||||
// tool/call; a provider-minted callId colliding with an Object prototype
|
||||
// property must read as absent, not as an inherited function that would
|
||||
// fold toolMs to NaN and fail the value schema.
|
||||
expect(fold([
|
||||
at(1_000, 'step/start', { turn: 1, step: 1 }),
|
||||
at(1_500, 'tool/result', result('toString')),
|
||||
at(2_000, 'step/end', { turn: 1, step: 1 }),
|
||||
])).toEqual(totals({ turns: 1, steps: 1 }))
|
||||
// The same name pairs normally once its call is recorded.
|
||||
expect(fold([
|
||||
at(1_000, 'step/start', { turn: 1, step: 1 }),
|
||||
at(1_100, 'tool/call', { turn: 1, step: 1, callId: 'constructor', name: 'read', arguments: '{}' }),
|
||||
at(1_600, 'tool/result', result('constructor')),
|
||||
at(2_000, 'step/end', { turn: 1, step: 1 }),
|
||||
])).toEqual(totals({ turns: 1, steps: 1, toolMs: 500 }))
|
||||
})
|
||||
|
||||
it('skips decode for an invalid usage report and ignores a duplicate assembled message', () => {
|
||||
const events = [
|
||||
at(1_000, 'step/start', { turn: 1, step: 1 }),
|
||||
|
||||
Reference in New Issue
Block a user