Files
accounted/components/agent/__tests__/markdown-links.test.ts
T
Jakob Wennberg 4702a63cff fix(assistant): announce answers to screen readers, one label map, links that keep the thread (#1224)
* fix(assistant): announce answers to screen readers, one label map, links that keep the thread

PR7 polish, three items from dev_docs/assistant_redesign_plan.md section 7.

The chat had no live region at all. A screen-reader user got no signal that the
assistant had answered: the reply simply appeared, for people who could see it.
Announcement fires on turn boundaries rather than over the streaming text,
because a live region on token deltas re-announces on every delta and makes the
surface unusable; the finished answer is read once, capped, with a pointer to
the message for the rest.

Two intent-label maps had drifted. The panel opened on the bokslut wizard titled
"Fråga Anna" while the same thread in the history list read "Hjälp med bokslut",
and the list's fallback returned the intent id itself, putting "bokslut.step" in
front of the user as the name of their own conversation. One map now, and an
unknown intent can no longer fall through to its id.

Links inside an answer were plain anchors, so following one did a full document
load: the app rebooted and took the conversation with it, which is the opposite
of what docking the panel was for. Internal links route client-side. External
ones open in a new tab with rel="noopener noreferrer", since the href came out
of a model that reads customer documents and target="_blank" without it hands
the opened page a handle back into an authenticated session.

Reduced motion needed nothing: globals.css already collapses every animation
under prefers-reduced-motion, so per-class variants would be redundant.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(assistant): review triage: scope the announcement to its own turn

Six findings, all real.

The announcement searched the whole thread, so a turn that produced no text of
its own (tool-only, or an error) found the PREVIOUS answer and read it out as
though it were new: a screen-reader user would hear a stale answer to a question
that had just been asked. It now receives only the current turn's messages,
bounded by an index captured when streaming starts.

It also read an interrupted answer as a finished one. Stop leaves the partial
text with a visible marker, so announcing it as the answer told a screen-reader
user the opposite of what everyone else could see.

messagesRef was assigned during render. React may replay a render, so the
announcement could read a snapshot the user never saw; the write moved into an
effect declared before the one that reads it.

The 400-character cap applied to the preview only, so the appended continuation
suffix pushed the real announcement past the limit the constant promised. The
cap now covers the whole string, and the test asserts against the constant
rather than a looser number the suffix could sneak past.

INTENT_LABELS was a plain object literal, so intentLabel('toString') resolved
Object.prototype.toString, passed the truthiness check and reached React as a
conversation title. Null-prototype now. intent_id comes from the database.

Markdown link titles were dropped: [text](url "title") carries a title that
react-markdown passes through and the renderer ignored.

Both new guards were mutation-checked: removing either makes its test fail. The
turn-boundary index itself is component wiring, which this node-only unit
project cannot exercise; announceableAnswer is tested against the slice it is
given.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-07-27 12:48:40 +02:00

125 lines
5.7 KiB
TypeScript
Raw Blame History

This file contains invisible Unicode characters
This file contains invisible Unicode characters that are indistinguishable to humans but may be processed differently by a computer. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
import { describe, it, expect } from 'vitest'
import { isInternalHref } from '../markdown-links'
import { ANNOUNCEMENT_LIMIT, announceableAnswer } from '../AgentChat'
import { intentLabel } from '../conversation-display'
describe('isInternalHref', () => {
it('routes app paths client-side', () => {
expect(isInternalHref('/invoices/abc-123')).toBe(true)
expect(isInternalHref('/bookkeeping/year-end?step=2')).toBe(true)
expect(isInternalHref('/kpi#marginal')).toBe(true)
})
it('does not treat a protocol-relative url as internal', () => {
// The href comes out of a model that reads customer documents. "//host"
// starts with a slash but leaves the site, and handing it to the router as
// an app path is the one failure mode worth engineering against.
expect(isInternalHref('//evil.example/login')).toBe(false)
expect(isInternalHref('/\\evil.example')).toBe(false)
})
it('leaves absolute and non-http schemes external', () => {
expect(isInternalHref('https://skatteverket.se')).toBe(false)
expect(isInternalHref('http://example.com')).toBe(false)
expect(isInternalHref('mailto:a@b.se')).toBe(false)
expect(isInternalHref('javascript:alert(1)')).toBe(false)
expect(isInternalHref('')).toBe(false)
})
})
describe('announceableAnswer', () => {
const msg = (role: 'user' | 'assistant', text: string) =>
({ role, text }) as Parameters<typeof announceableAnswer>[0][number]
it('announces the finished answer, not the question', () => {
expect(
announceableAnswer([msg('user', 'Hur gick juli?'), msg('assistant', 'Juli gick 12 % bättre.')]),
).toBe('Juli gick 12 % bättre.')
})
it('reads the LAST assistant turn when the thread has several', () => {
expect(
announceableAnswer([
msg('assistant', 'Första svaret.'),
msg('user', 'Och augusti?'),
msg('assistant', 'Andra svaret.'),
]),
).toBe('Andra svaret.')
})
it('caps the WHOLE announcement, suffix included, and says where the rest is', () => {
// A screen reader reads a live region straight through: a full bokslut
// explanation announced in one uninterruptible burst is worse than not
// announcing at all. Asserted against the constant, and at the real limit
// rather than a looser number the suffix could sneak past.
const out = announceableAnswer([msg('assistant', 'a'.repeat(1000))])
expect(out.length).toBeLessThanOrEqual(ANNOUNCEMENT_LIMIT)
expect(out).toContain('Svaret fortsätter i meddelandet')
})
it('does not read an interrupted answer as a finished one', () => {
// Stop leaves the partial text with a visible marker. Announcing it as the
// answer tells a screen-reader user the opposite of what everyone else sees.
const out = announceableAnswer([
{ role: 'assistant', text: 'Momsen för juli blev 12 4', interrupted: true } as Parameters<
typeof announceableAnswer
>[0][number],
])
expect(out).toContain('avbröts')
expect(out).not.toContain('12 4')
})
it('never re-announces an earlier answer for a turn that produced no text', () => {
// The caller passes only THIS turn's messages. Given the whole thread, a
// tool-only turn would find the previous answer and read it out again as
// though it were new: the user hears a stale answer to a new question.
const thisTurn = [msg('user', 'Boka detta')]
expect(announceableAnswer(thisTurn)).toBe('Assistenten är klar.')
})
it('still says something when the turn produced no text', () => {
// Silence would be indistinguishable from the request never having been
// sent. An assistant bubble that only ever held whitespace counts as none.
expect(announceableAnswer([msg('assistant', ' ')])).toBe('Assistenten är klar.')
expect(announceableAnswer([])).toBe('Assistenten är klar.')
})
})
describe('intentLabel', () => {
it('never shows the raw intent id', () => {
// It used to return the id itself, so an unmapped intent put
// "bokslut.step" in front of the user as the name of their conversation.
expect(intentLabel('some.unmapped.intent')).toBe('Fråga din assistent')
expect(intentLabel('some.unmapped.intent', 'Anna')).toBe('Fråga Anna')
})
it('gives the panel and the history list the SAME name for a thread', () => {
// The two maps had drifted: the panel titled a bokslut thread "Fråga Anna"
// while the history list called it "Hjälp med bokslut".
expect(intentLabel('bokslut.step')).toBe('Hjälp med bokslut')
expect(intentLabel('bokslut.step', 'Anna')).toBe('Hjälp med bokslut')
expect(intentLabel('kpi.explain', 'Anna')).toBe('Förklara nyckeltal')
})
it('personalises general help and falls back without a name', () => {
expect(intentLabel('general.help', 'Anna')).toBe('Fråga Anna')
expect(intentLabel('general.help')).toBe('Fråga din assistent')
expect(intentLabel('general.help', ' ')).toBe('Fråga din assistent')
})
it('does not resolve inherited Object keys as labels', () => {
// A plain object literal inherits from Object.prototype, so a lookup of
// 'toString' returned a FUNCTION, passed the truthiness check and reached
// React as a conversation title. intent_id comes from the database.
expect(intentLabel('toString')).toBe('Fråga din assistent')
expect(intentLabel('constructor', 'Anna')).toBe('Fråga Anna')
expect(intentLabel('__proto__')).toBe('Fråga din assistent')
})
it('keeps momsdeklaration spelled correctly through the soft hyphen', () => {
// The label carries a U+00AD so it can break across the narrow panel.
// Stripping it must leave a real word.
expect(intentLabel('vat.review').replace(/­/g, '')).toBe('Granska momsdeklaration')
})
})