Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 48 additions & 16 deletions src/frontend/src/components/portal/PortalSidebar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,20 @@
<span class="font-semibold">Workspace</span>
<!-- ent#364: an ASK is a distinct fact from an unread reply — one is waiting
on you to decide, the other on you to read — so it gets its own badge
rather than being summed into that one. Amber, before the unread count,
because a blocked agent outranks unread chatter. -->
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. -->
<span
v-if="askCount"
class="ml-auto shrink-0 min-w-[1.25rem] px-1.5 h-5 rounded-full bg-amber-500 text-white text-[11px] font-semibold flex items-center justify-center"
class="ml-auto shrink-0 min-w-[1.25rem] px-1.5 h-5 rounded-full bg-status-urgent-500 text-white text-[11px] font-semibold flex items-center justify-center"
data-testid="sidebar-ask-count"
:title="`${askCount} ${askCount === 1 ? 'agent is' : 'agents are'} waiting on your answer`"
:title="askBadgeTitle(askCount)"
>{{ askCount > 99 ? '99+' : askCount }}</span>
<span
v-if="totalWaiting"
Expand Down Expand Up @@ -135,6 +142,17 @@
<span class="shrink-0 min-w-[4.5rem] flex justify-end">
<BaseBadge v-if="chipFor(a)" :variant="chipFor(a).variant">{{ chipFor(a).label }}</BaseBadge>
</span>
<!-- #2424: the ask badge ent#364's comment above already promised.
It got an aggregate in the brand header and nothing per row, so
the header advertised a count with no way to reach the agent it
meant. Same token as that header badge; deliberately a DIFFERENT
colour from the unread pill beside it, because they are
different obligations. -->
<span
v-if="askCountFor(a.name)"
class="shrink-0 min-w-[1.25rem] px-1.5 h-5 rounded-full bg-status-urgent-500 text-white text-[11px] font-semibold flex items-center justify-center"
data-testid="agent-ask-count"
>{{ askCountFor(a.name) > 99 ? '99+' : askCountFor(a.name) }}</span>
<span
v-if="waitingFor(a.name)"
class="shrink-0 min-w-[1.25rem] px-1.5 h-5 rounded-full bg-action-primary-600 text-white text-[11px] font-semibold flex items-center justify-center"
Expand Down Expand Up @@ -230,6 +248,8 @@ import BaseBadge from '@/components/base/BaseBadge.vue'
import { useClientPortalStore } from '@/stores/clientPortal'
import {
groupThreadsByDate, partitionStarred, unreadByAgent, totalUnread, availabilityChip,
asksByAgent, askBadgeTitle, agentRowTitle as buildAgentRowTitle,
visibleAgentRows, AGENT_COLLAPSE_LIMIT,
signOutLabelFor,
} from './portalUtils'

Expand Down Expand Up @@ -268,6 +288,10 @@ const totalWaiting = computed(() => 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.
Expand Down Expand Up @@ -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
Expand Down
74 changes: 74 additions & 0 deletions src/frontend/src/components/portal/portalUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
180 changes: 180 additions & 0 deletions src/frontend/tests/unit/portalAskDiscoverability.spec.js
Original file line number Diff line number Diff line change
@@ -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 <name>" 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/)
})
})
12 changes: 11 additions & 1 deletion src/frontend/tests/unit/portalAvailabilityChip.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -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)
})
})
Loading
Loading