Skip to content

Commit 1a11272

Browse files
committed
Improve conversation arc snapshots & instrumentation
Make the conversation-arc event snapshot frozen and read-only (return ReadonlyArray) to preserve referential stability for useSyncExternalStore consumers. Cache an EMPTY_FROZEN_SNAPSHOT and freeze snapshots on refresh to prevent accidental mutation. Update types and the useConversationArc result to expose a frozen history. Integrate useSyncExternalStore in useChartSuggestions to track the arc "enabled" flag, add cross-instance deduping by peeking the store's most recent suggestion-shown event, and ensure signatures reset when recording is disabled so re-enables re-emit the current ranking. In useChartInterrogation, truncate long queries/answers before recording (configurable constants) and clamp latency to >= 0 to avoid negative deltas from clock skews. Add related comments and documentation for event fields. Add tests covering dedup across mount cycles and re-emission after disable→enable, and remove an unused import from the docs page. Overall intent: make the arc buffer stable and safe to share across hooks, avoid accidental mutations and buffer bloat, and reduce duplicate or spurious recorded events.
1 parent d8e5c89 commit 1a11272

6 files changed

Lines changed: 155 additions & 26 deletions

File tree

docs/src/pages/features/ConversationArcPage.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import {
77
disableConversationArc,
88
enableConversationArc,
99
executivePersona,
10-
getConversationArcStore,
1110
recordAudienceChange,
1211
useChartSuggestions,
1312
useConversationArc,

src/components/ai/conversationArc.ts

Lines changed: 41 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,12 @@ export interface InterrogationAskedEvent extends ConversationArcEventBase {
109109
type: "interrogation-asked"
110110
/** Chart the question was directed at, if known. */
111111
component?: string
112-
/** Free-form question text, truncated to a reasonable length by the caller. */
112+
/**
113+
* Question text. The `useChartInterrogation` instrumentation
114+
* truncates to ~500 chars before recording so the ring buffer
115+
* stays bounded; callers stamping their own events should do the
116+
* same.
117+
*/
113118
query: string
114119
/** Optional payload size hint (e.g. summary token count) for diagnostics. */
115120
contextSize?: number
@@ -119,11 +124,21 @@ export interface InterrogationAnsweredEvent extends ConversationArcEventBase {
119124
type: "interrogation-answered"
120125
/** Chart the answer was directed at, if known. */
121126
component?: string
122-
/** Free-form answer text, truncated by the caller. */
127+
/**
128+
* Answer text. The `useChartInterrogation` instrumentation
129+
* truncates to ~2000 chars before recording so multi-kilobyte LLM
130+
* responses don't bloat the ring buffer. Callers stamping their
131+
* own events should follow the same convention.
132+
*/
123133
answer?: string
124134
/** Number of annotations the response attached, if known. */
125135
annotationCount?: number
126-
/** Round-trip latency in ms from ask to answer, when the caller knows it. */
136+
/**
137+
* Round-trip latency in ms from ask to answer, clamped to ≥ 0.
138+
* The instrumentation measures via `performance.now()` when
139+
* available; the `Date.now()` fallback can produce negative
140+
* deltas under clock changes, hence the clamp.
141+
*/
127142
latencyMs?: number
128143
/** Set when the response was an error rather than a successful answer. */
129144
error?: boolean
@@ -174,8 +189,13 @@ export interface ConversationArcStore {
174189
record(input: ConversationArcEventInput): ConversationArcEvent | null
175190
/** Returns the current buffer (newest last) and clears it. */
176191
flush(): ConversationArcEvent[]
177-
/** Returns a snapshot of the current buffer without clearing. */
178-
getEvents(): ConversationArcEvent[]
192+
/**
193+
* Returns a frozen, referentially-stable snapshot of the current
194+
* buffer. Stable across consecutive calls until the next mutation,
195+
* so it can drive `useSyncExternalStore`. Read-only — callers that
196+
* need a mutable copy should `.slice()` the result.
197+
*/
198+
getEvents(): ReadonlyArray<ConversationArcEvent>
179199
/**
180200
* Subscribe to new events. Returns an unsubscribe function.
181201
*
@@ -209,16 +229,24 @@ const listeners = new Set<ConversationArcListener>()
209229

210230
// `useSyncExternalStore` requires `getSnapshot()` to return a stable
211231
// reference until something actually changes. The in-memory buffer is
212-
// mutated in place on `record()`, so we cache an immutable snapshot
213-
// next to the buffer and invalidate it on every push / clear / reset.
214-
// Returning a `slice()` from the public `getEvents()` API still gives
215-
// callers an array they can mutate without disturbing the buffer.
216-
let cachedSnapshot: ConversationArcEvent[] = []
232+
// mutated in place on `record()`, so we cache a frozen snapshot next
233+
// to the buffer and invalidate it on every push / clear / reset.
234+
//
235+
// The snapshot is FROZEN (Object.freeze) because it's shared across
236+
// every consumer: mutating it would corrupt subsequent snapshots and
237+
// break `useSyncExternalStore`'s referential-stability contract.
238+
// Callers that need a mutable copy can `.slice()` the result.
239+
const EMPTY_FROZEN_SNAPSHOT: ReadonlyArray<ConversationArcEvent> = Object.freeze(
240+
[]
241+
) as ReadonlyArray<ConversationArcEvent>
242+
let cachedSnapshot: ReadonlyArray<ConversationArcEvent> = EMPTY_FROZEN_SNAPSHOT
217243
let snapshotDirty = false
218244

219-
function refreshSnapshotIfNeeded(): ConversationArcEvent[] {
245+
function refreshSnapshotIfNeeded(): ReadonlyArray<ConversationArcEvent> {
220246
if (!snapshotDirty) return cachedSnapshot
221-
cachedSnapshot = store ? store.buffer.slice() : []
247+
cachedSnapshot = store
248+
? (Object.freeze(store.buffer.slice()) as ReadonlyArray<ConversationArcEvent>)
249+
: EMPTY_FROZEN_SNAPSHOT
222250
snapshotDirty = false
223251
return cachedSnapshot
224252
}
@@ -445,7 +473,7 @@ const facade: ConversationArcStore = {
445473
},
446474
reset() {
447475
listeners.clear()
448-
cachedSnapshot = []
476+
cachedSnapshot = EMPTY_FROZEN_SNAPSHOT
449477
snapshotDirty = false
450478
if (!store) {
451479
// Still fire the change notification so a hook subscribed

src/components/ai/useChartSuggestions.test.tsx

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,4 +79,40 @@ describe("useChartSuggestions — conversation-arc instrumentation", () => {
7979
expect((events[0] as { intent?: string }).intent).toBe("trend")
8080
expect((events[1] as { intent?: string }).intent).toBe("compare-categories")
8181
})
82+
83+
it("deduplicates against the store's most recent event across mount cycles", () => {
84+
// Simulates the StrictMode mount → unmount → remount pattern: a
85+
// per-instance ref resets across the cycle, so cross-instance
86+
// dedup is what catches the duplicate.
87+
enableConversationArc()
88+
const seen: Array<{ type: string }> = []
89+
getConversationArcStore().subscribe((e) => seen.push(e))
90+
91+
const { unmount } = renderHook(() =>
92+
useChartSuggestions(TREND_DATA, { intent: "trend" })
93+
)
94+
unmount()
95+
renderHook(() => useChartSuggestions(TREND_DATA, { intent: "trend" }))
96+
97+
const events = seen.filter((e) => e.type === "suggestion-shown")
98+
expect(events).toHaveLength(1)
99+
})
100+
101+
it("re-emits after disable → enable when suggestions are unchanged", () => {
102+
// A consumer that enables the arc store mid-session should still
103+
// see the current ranking. The disable path drops the signature
104+
// so the first re-enable emits.
105+
const seen: Array<{ type: string }> = []
106+
getConversationArcStore().subscribe((e) => seen.push(e))
107+
108+
const { rerender } = renderHook(() =>
109+
useChartSuggestions(TREND_DATA, { intent: "trend" })
110+
)
111+
// Hook ran while store was disabled — nothing recorded.
112+
expect(seen.filter((e) => e.type === "suggestion-shown")).toHaveLength(0)
113+
114+
enableConversationArc()
115+
rerender()
116+
expect(seen.filter((e) => e.type === "suggestion-shown")).toHaveLength(1)
117+
})
82118
})

src/components/ai/useChartSuggestions.ts

Lines changed: 56 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,22 @@
11
"use client"
2-
import { useEffect, useMemo, useRef } from "react"
2+
import { useEffect, useMemo, useRef, useSyncExternalStore } from "react"
33
import type { Datum } from "../charts/shared/datumTypes"
44
import { profileData, type ProfileDataOptions } from "./profileData"
55
import { suggestCharts, type SuggestChartsOptions } from "./suggestCharts"
66
import type { ChartDataProfile, Suggestion } from "./chartCapabilityTypes"
7-
import { getConversationArcStore } from "./conversationArc"
7+
import {
8+
getConversationArcStore,
9+
subscribeToConversationArcChange,
10+
} from "./conversationArc"
11+
12+
// Snapshot fn for the change subscription — `useSyncExternalStore`
13+
// compares with `Object.is`, so same-value mutations (every record()
14+
// while enabled, etc.) don't re-render the consumer. Only actual
15+
// enable/disable flips do. Lives outside the hook so the function
16+
// reference stays stable.
17+
const subscribeArc = (onChange: () => void) => subscribeToConversationArcChange(onChange)
18+
const getArcEnabled = () => getConversationArcStore().enabled
19+
const getArcEnabledOnServer = () => false
820

921
export interface UseChartSuggestionsOptions extends SuggestChartsOptions, ProfileDataOptions {}
1022

@@ -60,27 +72,64 @@ export function useChartSuggestions(
6072
// until a consumer calls `enableConversationArc()` — at which point
6173
// every recomputation of `useChartSuggestions` lands in the buffer.
6274
//
63-
// Dedup by the (component-list, intent, audience-target) signature
64-
// so React's strict-mode double-invocation doesn't double-stamp,
65-
// and a stable suggestions list across renders doesn't either.
75+
// Dedup happens in two layers:
76+
// 1. A per-instance signature ref catches stable-suggestions
77+
// re-renders within one mounted hook.
78+
// 2. A peek at the store's most recent `suggestion-shown` event
79+
// catches React StrictMode's mount → unmount → remount cycle
80+
// (where the per-instance ref resets) plus cross-instance
81+
// duplicates from a parent re-mounting children.
82+
// The signature also resets when recording is disabled, so a
83+
// mid-session enable correctly emits the current suggestions —
84+
// tracked via `useSyncExternalStore` so the effect re-runs when
85+
// the enabled flag flips.
86+
const arcEnabled = useSyncExternalStore(subscribeArc, getArcEnabled, getArcEnabledOnServer)
6687
const lastSignatureRef = useRef<string | null>(null)
6788
useEffect(() => {
6889
if (suggestions.length === 0) {
6990
lastSignatureRef.current = null
7091
return
7192
}
93+
if (!arcEnabled) {
94+
// Drop the signature so the next enable cycle re-emits the
95+
// current ranking. Otherwise a consumer that enables after
96+
// the suggester has already run sees no `suggestion-shown` at
97+
// all.
98+
lastSignatureRef.current = null
99+
return
100+
}
101+
const store = getConversationArcStore()
102+
72103
const audienceTarget = audience?.name ?? (audience ? "custom" : undefined)
73104
const signature = `${intent ?? ""}|${audienceTarget ?? ""}|${suggestions.map((s) => s.component).join(",")}`
74105
if (signature === lastSignatureRef.current) return
106+
107+
// Cross-instance dedup: if the most recent buffered
108+
// suggestion-shown event matches this signature, skip. Catches
109+
// StrictMode remounts and parent re-mounts that would otherwise
110+
// double-stamp the same ranking.
111+
const recent = store.getEvents()
112+
for (let i = recent.length - 1; i >= 0; i--) {
113+
const e = recent[i]
114+
if (e.type !== "suggestion-shown") continue
115+
const recentIntent = Array.isArray(e.intent) ? e.intent.join(",") : (e.intent ?? "")
116+
const recentSignature = `${recentIntent}|${e.audience ?? ""}|${e.components.join(",")}`
117+
if (recentSignature === signature) {
118+
lastSignatureRef.current = signature
119+
return
120+
}
121+
break // only check the most recent one
122+
}
123+
75124
lastSignatureRef.current = signature
76-
getConversationArcStore().record({
125+
store.record({
77126
type: "suggestion-shown",
78127
intent,
79128
components: suggestions.map((s) => s.component),
80129
topScore: suggestions[0]?.score,
81130
audience: audienceTarget,
82131
})
83-
}, [suggestions, intent, audience])
132+
}, [suggestions, intent, audience, arcEnabled])
84133

85134
return { suggestions, profile }
86135
}

src/components/ai/useConversationArc.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,8 @@ export interface UseConversationArcOptions extends EnableConversationArcOptions
141141
*/
142142
export interface UseConversationArcResult {
143143
/** Live event buffer, refreshed on every recorded event. */
144-
history: ConversationArcEvent[]
144+
/** Frozen snapshot of the live event buffer. Read-only; `.slice()` for a mutable copy. */
145+
history: ReadonlyArray<ConversationArcEvent>
145146
/** Reduced summary of the current buffer. Recomputed on each event. */
146147
summary: ConversationArcSummary
147148
/** `true` while the store is actively recording. */

src/components/store/useChartInterrogation.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,20 @@
11
"use client"
22
import { useCallback, useMemo, useRef, useState } from "react"
33
import { getConversationArcStore } from "../ai/conversationArc"
4+
5+
// Truncate long strings before they land in the arc ring buffer.
6+
// LLM responses can be kilobytes; multiply by 1000 events of capacity
7+
// and the buffer grows fast. Clamping the recorded payload keeps the
8+
// buffer reasonable while still leaving enough context for replay
9+
// fixtures and analytics readouts. Slightly under the visible cap so
10+
// the ellipsis doesn't replace meaningful tail content.
11+
const MAX_ARC_QUERY_LENGTH = 500
12+
const MAX_ARC_ANSWER_LENGTH = 2000
13+
function truncateForArc(text: string | undefined, max: number): string | undefined {
14+
if (text == null) return text
15+
if (text.length <= max) return text
16+
return text.slice(0, max - 1) + "…"
17+
}
418
import type { Datum } from "../charts/shared/datumTypes"
519
import { summarizeData, type DataSummary } from "../data/DataSummarizer"
620
import { profileData } from "../ai/profileData"
@@ -224,7 +238,7 @@ export function useChartInterrogation(
224238
getConversationArcStore().record({
225239
type: "interrogation-asked",
226240
component: componentNameRef.current,
227-
query: trimmed,
241+
query: truncateForArc(trimmed, MAX_ARC_QUERY_LENGTH)!,
228242
})
229243
try {
230244
const result = await onQueryRef.current(trimmed, {
@@ -242,9 +256,11 @@ export function useChartInterrogation(
242256
getConversationArcStore().record({
243257
type: "interrogation-answered",
244258
component: componentNameRef.current,
245-
answer: result.answer,
259+
answer: truncateForArc(result.answer, MAX_ARC_ANSWER_LENGTH),
246260
annotationCount: result.annotations?.length,
247-
latencyMs: Math.round(answeredAt - askedAt),
261+
// Clamp to ≥0 in case the Date.now() fallback was used and
262+
// the system clock moved backwards between ask and answer.
263+
latencyMs: Math.max(0, Math.round(answeredAt - askedAt)),
248264
})
249265
} catch (err) {
250266
const e = err instanceof Error ? err : new Error(String(err))
@@ -258,7 +274,7 @@ export function useChartInterrogation(
258274
type: "interrogation-answered",
259275
component: componentNameRef.current,
260276
error: true,
261-
latencyMs: Math.round(answeredAt - askedAt),
277+
latencyMs: Math.max(0, Math.round(answeredAt - askedAt)),
262278
})
263279
} finally {
264280
setLoading(false)

0 commit comments

Comments
 (0)