diff --git a/src/frontend/src/components/portal/PortalSidebar.vue b/src/frontend/src/components/portal/PortalSidebar.vue index 2207e9138..f7d814391 100644 --- a/src/frontend/src/components/portal/PortalSidebar.vue +++ b/src/frontend/src/components/portal/PortalSidebar.vue @@ -8,13 +8,20 @@ Workspace + rather than being summed into that one. Before the unread count, + because a blocked agent outranks unread chatter. + + #2424: `status-urgent` (not amber) — the token the operator NavBar's + pending-operator-queue badge already uses for "waiting on you", so the + two surfaces agree. Amber maps to `state-autonomous`, an operating + mode, which is a different claim. The wording comes from + `askBadgeTitle` because the inline literal said "agents" while + `askCount` counted asks. --> {{ askCount > 99 ? '99+' : askCount }} {{ chipFor(a).label }} + + {{ askCountFor(a.name) > 99 ? '99+' : askCountFor(a.name) }} totalUnread(props.threads)) // would be a second path to the same fact, free to disagree with it. const asksStore = useClientPortalStore() const askCount = computed(() => asksStore.askCount) +// #2424: the ask twin of `waiting`. Kept a separate map on purpose — see the +// brand-badge comment in the template. +const asksPerAgent = computed(() => asksByAgent(asksStore.openAsks)) +const askCountFor = (name) => asksPerAgent.value[name] || 0 // A row key has to include the kind: thread ids and room ids are independent // spaces, so two chats of different kinds could collide on a bare id. @@ -300,25 +324,33 @@ const agentLabel = (a) => (a.display_label || '').trim() || a.name // deleted or lost the agent, and neither is actionable for a client. const chipFor = (a) => availabilityChip(a, { detailed: props.isPlatformSession }) +// The state must be reachable without relying on colour — which is why #2424 +// is an accessibility fix too: a blocked agent's title was the bare +// "Open ws-sage" while two asks waited on it. function agentRowTitle(a) { - const n = waitingFor(a.name) - const label = agentLabel(a) - const who = label === a.name ? label : `${label} (${a.name})` - const base = n - ? `${who} — ${n} unread ${n === 1 ? 'reply' : 'replies'}` - : `Open ${who}` - // The state must be reachable without relying on colour. const chip = chipFor(a) - return chip ? `${base} — ${chip.title}` : base + return buildAgentRowTitle({ + label: agentLabel(a), + name: a.name, + unread: waitingFor(a.name), + askCount: askCountFor(a.name), + chipTitle: chip ? chip.title : '', + }) } // #2159: a long fleet made the agents block the whole sidebar, pushing chats // below the fold. Top 5, expandable in place. -const AGENT_COLLAPSE_LIMIT = 5 +// +// #2424: an agent with an open ask is never collapsed out. The slice was plain +// roster order, so the one row a person needed could be the hidden one — the +// reported case had it 11th of 12 while the header advertised its two asks. The +// rule is in portalUtils (`AGENT_COLLAPSE_LIMIT` now lives there too, one +// definition for the slice and the toggle). const agentsExpanded = ref(false) -const shownAgents = computed(() => ( - agentsExpanded.value ? props.roster : props.roster.slice(0, AGENT_COLLAPSE_LIMIT) -)) +const shownAgents = computed(() => visibleAgentRows(props.roster, { + expanded: agentsExpanded.value, + askCounts: asksPerAgent.value, +})) // ent#186: history + search rows show the conversation's agent avatar instead of // a bare color dot. The URL is resolved from the roster already loaded at sign-in diff --git a/src/frontend/src/components/portal/portalUtils.js b/src/frontend/src/components/portal/portalUtils.js index 7abf7efae..d9629c238 100644 --- a/src/frontend/src/components/portal/portalUtils.js +++ b/src/frontend/src/components/portal/portalUtils.js @@ -65,6 +65,80 @@ export function unreadByAgent(threads) { return out } +// ent#364 / #2424: asks per agent — the ask twin of `unreadByAgent`. +// +// Deliberately a SEPARATE map, never summed into the unread count. The two are +// different facts about different obligations: an ask is waiting on you to +// DECIDE, an unread reply on you to READ. PortalSidebar has said so since +// ent#364; what it lacked was this half. +export function asksByAgent(asks) { + const out = {} + for (const a of Array.isArray(asks) ? asks : []) { + const name = a?.agent_name + if (!name) continue + out[name] = (out[name] || 0) + 1 + } + return out +} + +// The aggregate badge's accessible name. +// +// #2424: it counted ASKS and said "agents" — two asks raised by one agent read +// as "2 agents are waiting on your answer". The number was right and the noun +// was wrong, and the two only diverge when a single agent raises more than one +// ask, which is why nobody caught it. +// +// Resolved toward asks rather than agents, because the row badges added +// alongside this now answer "which agent" — so the header's job is "how many +// decisions", and that is a count of asks. +export function askBadgeTitle(count) { + const n = Number(count) || 0 + if (n <= 0) return '' + return `${n} ${n === 1 ? 'ask is' : 'asks are'} waiting on your answer` +} + +// The agent row's accessible name. +// +// #2424: this composed unread replies and the availability chip and never +// mentioned asks, so a blocked agent's title was the bare "Open ws-sage" — the +// pending decision was unreachable for a screen-reader user as well as +// invisible. Asks lead: a decision outranks unread chatter. +export function agentRowTitle({ label, name, unread = 0, askCount = 0, chipTitle = '' } = {}) { + const who = label && label !== name ? `${label} (${name})` : (label || name || '') + const asks = Number(askCount) || 0 + const reads = Number(unread) || 0 + + const parts = [] + if (asks > 0) parts.push(`${asks} ${asks === 1 ? 'ask' : 'asks'} waiting on you`) + if (reads > 0) parts.push(`${reads} unread ${reads === 1 ? 'reply' : 'replies'}`) + + const base = parts.length ? `${who} — ${parts.join(', ')}` : `Open ${who}` + return chipTitle ? `${base} — ${chipTitle}` : base +} + +// #2159 capped the roster at five so a long fleet could not push chats below +// the fold. #2424: the cap is a plain roster-order slice, so on any fleet larger +// than five the agent WAITING ON YOU is as likely as not to be behind the +// toggle — observed with an agent 11th of 12 while the header advertised its +// two asks. +// +// Ask-bearing agents are appended, not floated to the top: re-sorting on a +// transient count moves rows under the cursor between refreshes, which is the +// same reason the roster is not re-sorted by availability. So the first N stay +// exactly where they were and the visible list simply grows. +export const AGENT_COLLAPSE_LIMIT = 5 + +export function visibleAgentRows(roster, { expanded = false, askCounts = {}, limit = AGENT_COLLAPSE_LIMIT } = {}) { + const list = Array.isArray(roster) ? roster : [] + if (expanded) return list + + const head = list.slice(0, limit) + const shown = new Set(head.map((a) => a?.name)) + const counts = askCounts || {} + const waiting = list.filter((a) => a?.name && !shown.has(a.name) && (Number(counts[a.name]) || 0) > 0) + return waiting.length ? [...head, ...waiting] : head +} + export function totalUnread(threads) { return (Array.isArray(threads) ? threads : []) .reduce((sum, t) => sum + (Number(t?.unread) || 0), 0) diff --git a/src/frontend/tests/unit/portalAskDiscoverability.spec.js b/src/frontend/tests/unit/portalAskDiscoverability.spec.js new file mode 100644 index 000000000..20d0ec274 --- /dev/null +++ b/src/frontend/tests/unit/portalAskDiscoverability.spec.js @@ -0,0 +1,180 @@ +/** + * #2424 — the Workspace said "2 asks are waiting" and gave you no way to find them. + * + * Three failures compounded, and they are only worth fixing together: the header + * badge reported the wrong unit, the agent row carried no ask indicator at all, + * and the agent could be collapsed out of the roster entirely. A person saw a + * count, had no row to click, and the agent was not on screen. + * + * Everything decidable lives in `portalUtils` because vitest runs + * `environment: 'node'` with no component-mount harness — a rule that lived + * inside the SFC would be one no test could reach, which is how all three of + * these shipped. + */ +import { describe, it, expect } from 'vitest' +import fs from 'fs' +import path from 'path' +import { + asksByAgent, + askBadgeTitle, + agentRowTitle, + visibleAgentRows, + AGENT_COLLAPSE_LIMIT, +} from '../../src/components/portal/portalUtils.js' + +const SIDEBAR = path.resolve(__dirname, '../../src/components/portal/PortalSidebar.vue') +const sidebarSource = () => fs.readFileSync(SIDEBAR, 'utf8') + +// --------------------------------------------------------------------------- +// 1. The unit the badge reports +// --------------------------------------------------------------------------- +describe('#2424 part 1 — the badge counts asks, so it must say asks', () => { + it('says "asks" for many, never "agents"', () => { + const title = askBadgeTitle(2) + expect(title).toContain('2') + expect(title).toMatch(/asks/i) + // The reported bug verbatim: two asks on ONE agent rendered as + // "2 agents are waiting on your answer". + expect(title).not.toMatch(/agents/i) + }) + + it('is singular for one', () => { + expect(askBadgeTitle(1)).toMatch(/\b1 ask\b/i) + expect(askBadgeTitle(1)).not.toMatch(/asks/i) + }) + + it('says nothing at zero — an empty title is worse than no attribute', () => { + expect(askBadgeTitle(0)).toBe('') + }) + + it('the SFC no longer builds that sentence inline', () => { + // The old bug was a template literal in the template. If it comes back, the + // pure function above stops being the single source of the wording. + expect(sidebarSource()).not.toMatch(/agents are.*waiting on your answer/) + }) +}) + +// --------------------------------------------------------------------------- +// 2. Per-agent counts, and the row that shows them +// --------------------------------------------------------------------------- +describe('#2424 part 2 — asks are attributable to an agent', () => { + it('groups by agent', () => { + expect(asksByAgent([ + { agent_name: 'ws-sage' }, + { agent_name: 'ws-sage' }, + { agent_name: 'scout' }, + ])).toEqual({ 'ws-sage': 2, scout: 1 }) + }) + + it('survives junk without throwing — the sidebar must not blank on a bad row', () => { + expect(asksByAgent(null)).toEqual({}) + expect(asksByAgent(undefined)).toEqual({}) + expect(asksByAgent([null, {}, { agent_name: '' }, { agent_name: 'a' }])).toEqual({ a: 1 }) + }) + + it('the row title names the pending decision', () => { + const t = agentRowTitle({ label: 'ws-sage', name: 'ws-sage', askCount: 2 }) + expect(t).toMatch(/2 asks/i) + // The observed title was the bare "Open ws-sage" while two asks waited. + expect(t).not.toBe('Open ws-sage') + }) + + it('is singular for one ask', () => { + expect(agentRowTitle({ label: 'a', name: 'a', askCount: 1 })).toMatch(/\b1 ask\b/i) + }) + + it('keeps unread replies as a SEPARATE fact, never summed', () => { + const t = agentRowTitle({ label: 'a', name: 'a', askCount: 2, unread: 3 }) + expect(t).toMatch(/2 asks/i) + expect(t).toMatch(/3 unread/i) + // "5" would mean the two counts were added — the exact conflation + // PortalSidebar.vue's own comment forbids. + expect(t).not.toMatch(/\b5\b/) + }) + + it('still renders the display label with its slug, and the availability chip', () => { + const t = agentRowTitle({ + label: 'Sage', name: 'ws-sage', askCount: 0, unread: 0, chipTitle: 'This agent is stopped', + }) + expect(t).toContain('Sage') + expect(t).toContain('ws-sage') + expect(t).toContain('This agent is stopped') + }) + + it('falls back to "Open " when nothing is pending', () => { + expect(agentRowTitle({ label: 'a', name: 'a' })).toBe('Open a') + }) + + it('the row renders an ask badge, tokenised and distinct from the unread pill', () => { + const src = sidebarSource() + expect(src).toMatch(/askCountFor\(/) + // status-urgent is the platform's "waiting on you" token — the same one the + // operator NavBar's pending-operator-queue badge uses. Not amber: that maps + // to `state-autonomous`, which is an operating mode, not a pending decision. + expect(src).toContain('bg-status-urgent-500') + // Raw palette classes are ratcheted to zero for new code (design contract). + expect(src).not.toContain('bg-amber-500') + }) +}) + +// --------------------------------------------------------------------------- +// 3. A blocked agent is never collapsed out of view +// --------------------------------------------------------------------------- +describe('#2424 part 3 — the agent you need is on screen', () => { + const roster = Array.from({ length: 12 }, (_, i) => ({ name: `a${String(i).padStart(2, '0')}` })) + + it('collapsed, shows the first N in roster order when nothing is pending', () => { + const out = visibleAgentRows(roster, { expanded: false, askCounts: {} }) + expect(out).toHaveLength(AGENT_COLLAPSE_LIMIT) + expect(out.map((a) => a.name)).toEqual(['a00', 'a01', 'a02', 'a03', 'a04']) + }) + + it('lifts an ask-bearing agent that would otherwise be hidden', () => { + // The reported case: ws-sage was 11th of 12, so on a fresh load the one row + // the person needed was behind the "show more" toggle. + const out = visibleAgentRows(roster, { expanded: false, askCounts: { a10: 2 } }) + expect(out.map((a) => a.name)).toContain('a10') + }) + + it('keeps roster order rather than floating the ask to the top', () => { + // Re-sorting on a transient count makes rows move under the cursor between + // refreshes — the layout-stability rule the availability chip already obeys. + const out = visibleAgentRows(roster, { expanded: false, askCounts: { a10: 2 } }) + const names = out.map((a) => a.name) + expect(names.indexOf('a10')).toBe(names.length - 1) + expect(names.slice(0, 5)).toEqual(['a00', 'a01', 'a02', 'a03', 'a04']) + }) + + it('does not duplicate an ask-bearing agent already inside the slice', () => { + const out = visibleAgentRows(roster, { expanded: false, askCounts: { a02: 1 } }) + expect(out.filter((a) => a.name === 'a02')).toHaveLength(1) + expect(out).toHaveLength(AGENT_COLLAPSE_LIMIT) + }) + + it('expanded, shows everything', () => { + expect(visibleAgentRows(roster, { expanded: true, askCounts: { a10: 2 } })).toHaveLength(12) + }) + + it('short rosters are untouched', () => { + const three = roster.slice(0, 3) + expect(visibleAgentRows(three, { expanded: false, askCounts: {} })).toHaveLength(3) + }) + + it('tolerates a missing roster', () => { + expect(visibleAgentRows(null, { expanded: false, askCounts: {} })).toEqual([]) + expect(visibleAgentRows(roster, {})).toHaveLength(AGENT_COLLAPSE_LIMIT) + }) + + it('the SFC drives its rows through the shared rule', () => { + const src = sidebarSource() + expect(src).toMatch(/visibleAgentRows\(/) + // The raw slice was the bug; it must not survive alongside the fix. + expect(src).not.toMatch(/roster\.slice\(0,\s*AGENT_COLLAPSE_LIMIT\)/) + }) + + it('the "show more" affordance still keys off the full roster', () => { + // With ask-lifting, the visible count can exceed the limit — so a toggle + // gated on `shown.length` would vanish exactly when an ask is pending. + expect(sidebarSource()).toMatch(/roster\.length > AGENT_COLLAPSE_LIMIT/) + }) +}) diff --git a/src/frontend/tests/unit/portalAvailabilityChip.spec.js b/src/frontend/tests/unit/portalAvailabilityChip.spec.js index c1c8dbf87..8ef901467 100644 --- a/src/frontend/tests/unit/portalAvailabilityChip.spec.js +++ b/src/frontend/tests/unit/portalAvailabilityChip.spec.js @@ -17,6 +17,7 @@ * helper, and that nothing became disabled. */ import { describe, it, expect } from 'vitest' +import { agentRowTitle } from '../../src/components/portal/portalUtils.js' import { readFileSync } from 'fs' import { fileURLToPath } from 'url' @@ -164,6 +165,15 @@ describe('#2196 the surfaces consume the shared rule', () => { }) it('the row title carries the state, so it is reachable without colour', () => { - expect(SIDEBAR).toMatch(/chip \? `\$\{base\} — \$\{chip\.title\}` : base/) + // #2424 moved the composition into portalUtils::agentRowTitle, so this + // asserts the property instead of the old inline ternary. Stronger: it now + // catches a chip title that is dropped as well as one that is reworded. + const withChip = agentRowTitle({ + label: 'a', name: 'a', chipTitle: 'This agent is stopped — ask admin to start it.', + }) + expect(withChip).toContain('This agent is stopped') + expect(agentRowTitle({ label: 'a', name: 'a' })).not.toMatch(/—/) + // #2424 additionally requires a pending ask to be reachable the same way. + expect(agentRowTitle({ label: 'a', name: 'a', askCount: 2 })).toMatch(/2 asks/i) }) }) diff --git a/src/frontend/tests/unit/portalRosterRow.spec.js b/src/frontend/tests/unit/portalRosterRow.spec.js index 73463fc12..4e08943b5 100644 --- a/src/frontend/tests/unit/portalRosterRow.spec.js +++ b/src/frontend/tests/unit/portalRosterRow.spec.js @@ -13,6 +13,7 @@ * agent would render as blank. */ import { describe, it, expect } from 'vitest' +import { visibleAgentRows, AGENT_COLLAPSE_LIMIT } from '../../src/components/portal/portalUtils.js' import { readFileSync } from 'fs' import { fileURLToPath } from 'url' @@ -69,8 +70,16 @@ describe('#2159 the row no longer shows the description', () => { describe('#2159 the roster is bounded and expandable', () => { it('shows a fixed number by default rather than the whole fleet', () => { - expect(source).toMatch(/const AGENT_COLLAPSE_LIMIT = \d+/) - expect(source).toMatch(/agentsExpanded\.value \? props\.roster : props\.roster\.slice\(0, AGENT_COLLAPSE_LIMIT\)/) + // #2424 moved the rule into portalUtils, so this asserts the PROPERTY + // rather than the old inline slice expression. Same guarantee, and now it + // fails on a broken bound instead of only on a reworded one. + const roster = Array.from({ length: 12 }, (_, i) => ({ name: `a${i}` })) + const collapsed = visibleAgentRows(roster, { expanded: false, askCounts: {} }) + expect(collapsed).toHaveLength(AGENT_COLLAPSE_LIMIT) + expect(AGENT_COLLAPSE_LIMIT).toBeLessThan(roster.length) + expect(visibleAgentRows(roster, { expanded: true, askCounts: {} })).toHaveLength(12) + // ...and the component still routes its rows through it. + expect(source).toMatch(/visibleAgentRows\(props\.roster/) }) it('uses ONE persistent toggle, not two v-if-alternated buttons', () => {