fix(desktop): suggestion pills paint the current offer, not the first one seen

The bus's change gate compared offers by `provider:id` alone. Providers
rebuild their suggestion objects on every draft sample, so that key is equal
constantly and the write bailed out — pinning the FIRST object for the life
of the offer.

Two consequences, both user-visible. The pill keeps painting a stale reason
("you mentioned linear" after the user pasted a linear.app link), and it
keeps calling a stale `invoke` closure — work built for a draft that no
longer exists.

Compare the fields the pill actually renders instead. The reference-identity
bail-out survives for the common case (same draft, same match, no re-render),
which is what the gate was there for.
This commit is contained in:
Brooklyn Nicholson
2026-08-13 17:25:07 -05:00
committed by brooklyn!
parent 8c8d55bd07
commit 0280cf09c4
2 changed files with 67 additions and 1 deletions
@@ -80,4 +80,47 @@ describe('composer suggestion bus', () => {
offerSuggestions('s6', 'test', [])
})
it('replaces an offer whose rendered copy changed under the same key', () => {
offerSuggestions('s7', 'test', [{ ...suggestion('linear'), tip: 'because you mentioned “linear”' }])
offerSuggestions('s7', 'test', [{ ...suggestion('linear'), tip: 'because you pasted linear.app' }])
// Same key, new trigger — the strip must paint the new reason, not the
// first one it ever saw.
expect(($composerSuggestionsBySession.get().s7 ?? []).map(s => s.tip)).toEqual(['because you pasted linear.app'])
offerSuggestions('s7', 'test', [])
})
it('re-offering the same key swaps in the fresh invoke closure', async () => {
const calls: string[] = []
const offer = (tag: string) =>
offerSuggestions('s8', 'test', [
{ ...suggestion('linear'), invoke: async () => void calls.push(tag), label: `Add linear ${tag}` }
])
offer('first')
offer('second')
await ($composerSuggestionsBySession.get().s8 ?? [])[0]!.invoke({ cancelled: () => false, sessionId: 's8' })
// A pinned first object means the pill runs work built for a draft the
// user has since changed.
expect(calls).toEqual(['second'])
offerSuggestions('s8', 'test', [])
})
it('keeps the array reference when nothing the pill paints changed', () => {
offerSuggestions('s9', 'test', [suggestion('linear')])
const first = $composerSuggestionsBySession.get().s9
offerSuggestions('s9', 'test', [suggestion('linear')])
expect($composerSuggestionsBySession.get().s9).toBe(first)
offerSuggestions('s9', 'test', [])
})
})
+24 -1
View File
@@ -67,8 +67,31 @@ export const $composerSuggestionsBySession = atom<Record<string, ComposerSuggest
const keyFor = (sessionId: string | null | undefined): string => sessionId ?? ''
// Everything the pill actually paints. Compared field-by-field rather than by
// key alone: a provider rebuilds its suggestion objects on every sample, so
// key equality is true constantly, and treating that as "no change" pins the
// FIRST object forever — the strip then paints a stale tip and, worse, calls a
// stale `invoke` closure. Comparing the rendered copy keeps the cheap bail-out
// for the common case (same draft, same match) while letting a genuinely
// changed offer through.
const RENDERED: readonly (keyof ComposerSuggestion)[] = [
'brand',
'doneLabel',
'doneTip',
'icon',
'label',
'tip',
'workingLabel',
'workingTip'
]
const sameSuggestions = (a: readonly ComposerSuggestion[], b: readonly ComposerSuggestion[]) =>
a.length === b.length && a.every((x, i) => suggestionKey(x) === suggestionKey(b[i]!))
a.length === b.length &&
a.every((x, i) => {
const y = b[i]!
return suggestionKey(x) === suggestionKey(y) && RENDERED.every(field => x[field] === y[field])
})
function write(sessionId: string | null | undefined, suggestions: ComposerSuggestion[]): void {
const key = keyFor(sessionId)