refactor(settings): drop Safety and the classifier rollback, collapse the Access tree

Safety held one thing: rolling back the AI classification tag assignments. The
classifier is gone, so the panel had nothing left to be about and the section is
removed, along with the snapshot state and the rollback call that fed it.
Permissions — the other thing that section might have grown into — already have
a page in Access.

"Search and sign-up" carried two controls that live somewhere else: a Public
Registration toggle that Access and joining already owns, and an embedding model
field that belongs with the other models. The duplicate registration toggle is
gone and the section is now just Search.

The account menu no longer offers Administration. It pointed at /settings?s=people
— the same page the Settings entry above it opens — so it was two names for one
door. Settings already shows the site sections to an administrator.

Access: the branch tree opened its top level by default and ran to hundreds of
rows, which buried the image libraries below it. Every branch now starts closed,
opening one closes the one before it at the same depth, and the tree scrolls
inside a bounded box. Picking a different person collapses it again. A "Clear all
access" control removes every grant a person holds, including the everything
role, for starting over.

The standfirst is reworded to lead with what to do rather than with a definition.

Tests updated rather than worked around: the Settings test asserted Safety was a
section, and the Access test reached a child branch that is no longer open on
load. Both now assert the new behaviour, plus two new cases — that branches start
collapsed, and that opening one closes the previous. 346 frontend tests pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU
This commit is contained in:
Daniel 2026-09-11 20:27:55 +02:00
parent 5c4823d9a4
commit 3d379d41a8
7 changed files with 116 additions and 182 deletions

View file

@ -166,12 +166,10 @@ function AccountMenu({ user, onLogout }) {
<Link role="menuitem" to="/" onClick={() => setOpen(false)}>Dashboard</Link>
{/* Settings opens on the account, so a separate Account entry was
two doors to the same room. */}
{/* One door. Settings already shows the site sections to an
administrator, so a second entry pointing into the same page was
two names for one place. */}
<Link role="menuitem" to="/settings" onClick={() => setOpen(false)}>Settings</Link>
{user?.role === 'admin' && (
<Link role="menuitem" to="/settings?s=people" onClick={() => setOpen(false)}>
Administration
</Link>
)}
<button type="button" role="menuitem" className="acct-signout"
onClick={() => { setOpen(false); onLogout() }}>Sign out</button>
</div>

View file

@ -83,3 +83,27 @@
.ac-people ul { max-height: 240px; }
.ac-counts { display: none; }
}
/* Bounded branch list
The category tree runs to hundreds of rows. Left to its own height it
pushed the image libraries and everything else off the bottom of the
page, so it scrolls inside itself and starts fully collapsed. */
.ac-tree-box {
max-height: 380px;
overflow-y: auto;
border: 1px solid var(--border);
border-radius: 10px;
padding: 6px 4px;
}
.ac-tree-box .ac-tree { margin: 0; }
.ac-person-head { display: flex; align-items: flex-start; justify-content: space-between; gap: 16px; }
.ac-reset {
flex: 0 0 auto;
font: inherit; font-size: 0.8rem; font-weight: 600;
color: var(--danger, #dc2626);
background: none; border: 1px solid var(--border); border-radius: 8px;
padding: 6px 12px; cursor: pointer;
}
.ac-reset:hover:not(:disabled) { border-color: var(--danger, #dc2626); }
.ac-reset:disabled { opacity: .55; cursor: default; }

View file

@ -27,8 +27,13 @@ function buildTree(categories) {
* separately grantable: the grant already reaches them, and offering a
* checkbox that changes nothing is how a permissions screen starts lying.
*/
function Branch({ node, byParent, granted, covered, onToggle, busy, depth = 0 }) {
const [open, setOpen] = useState(depth === 0)
function Branch({ node, byParent, granted, covered, onToggle, busy, depth = 0, openPath, onOpen }) {
// Which branch is open is held above rather than per-node: opening one has to
// close the one before it, and a node cannot know about its siblings. Every
// branch starts closed the full tree is hundreds of rows, and scrolling
// past all of it to reach the next top-level subject is the thing that made
// this unusable.
const open = openPath.includes(node.id)
const children = byParent.get(node.id) || []
const isGranted = granted.has(node.id)
const isCovered = !isGranted && covered.has(node.id)
@ -39,7 +44,7 @@ function Branch({ node, byParent, granted, covered, onToggle, busy, depth = 0 })
{children.length > 0 ? (
<button type="button" className="ac-twisty" aria-expanded={open}
aria-label={`${open ? 'Collapse' : 'Expand'} ${node.name}`}
onClick={() => setOpen(v => !v)}>{open ? '▾' : '▸'}</button>
onClick={() => onOpen(node.id, depth, !open)}>{open ? '▾' : '▸'}</button>
) : <span className="ac-twisty" aria-hidden="true" />}
<label className="ac-label">
@ -60,7 +65,8 @@ function Branch({ node, byParent, granted, covered, onToggle, busy, depth = 0 })
<ul>
{children.map(child => (
<Branch key={child.id} node={child} byParent={byParent} granted={granted}
covered={covered} onToggle={onToggle} busy={busy} depth={depth + 1} />
covered={covered} onToggle={onToggle} busy={busy} depth={depth + 1}
openPath={openPath} onOpen={onOpen} />
))}
</ul>
)}
@ -85,6 +91,9 @@ export default function AccessPage() {
const [busy, setBusy] = useState(false)
const [error, setError] = useState('')
const [query, setQuery] = useState('')
// The chain of open branches, outermost first. One per depth, so opening a
// subject closes the previous subject but keeps its own ancestors open.
const [openPath, setOpenPath] = useState([])
const load = useCallback(() => {
Promise.all([api.get('/access/'), api.get('/access/tree')])
@ -138,6 +147,22 @@ export default function AccessPage() {
const setRole = (role) => change(
() => api.put(`/access/${selectedId}/role`, { role }), 'Could not change that')
const openBranch = (id, depth, on) => setOpenPath(path =>
on ? [...path.slice(0, depth), id] : path.slice(0, depth))
// Start this person again from nothing. Everything granted goes, including
// the branches reached by a parent, because removing the parent removes them.
const resetPerson = () => change(async () => {
for (const id of selected?.categories || []) {
await api.delete(`/access/${selectedId}/grants/category/${id}`)
}
for (const id of selected?.libraries || []) {
await api.delete(`/access/${selectedId}/grants/library/${id}`)
}
if (selected?.everything) await api.put(`/access/${selectedId}/role`, { role: 'user' })
setOpenPath([])
}, 'Could not clear that')
if (loading) return <div className="loading"><div className="spinner" /></div>
return (
@ -146,9 +171,9 @@ export default function AccessPage() {
<div>
<h1>Access</h1>
<p>
What each person may edit. A branch covers everything beneath it its
questions and the articles filed under them and image libraries are
granted one at a time.
Pick someone to see what they can edit. Granting a subject also grants
everything filed under it its sub-topics, their questions and their
articles. Image libraries are granted one by one.
</p>
</div>
<Link className="btn btn-secondary btn-sm" to="/settings?s=admin">Back to settings</Link>
@ -165,7 +190,7 @@ export default function AccessPage() {
<li key={row.id}>
<button type="button" className={row.id === selectedId ? 'is-active' : undefined}
aria-current={row.id === selectedId ? 'true' : undefined}
onClick={() => setSelectedId(row.id)}>
onClick={() => { setSelectedId(row.id); setOpenPath([]) }}>
<span className="ac-person-name">{row.name}</span>
<span className="ac-person-meta">
{row.everything ? 'Everything'
@ -187,8 +212,16 @@ export default function AccessPage() {
) : (
<>
<div className="ac-person-head">
<h2>{selected.name}</h2>
<span>{selected.email}</span>
<div>
<h2>{selected.name}</h2>
<span>{selected.email}</span>
</div>
{(selected.everything || selected.categories.length || selected.libraries.length) > 0 &&
selected.role !== 'admin' && selected.id !== me?.id && (
<button type="button" className="ac-reset" disabled={busy} onClick={resetPerson}>
Clear all access
</button>
)}
</div>
{selected.role === 'admin' ? (
@ -226,13 +259,17 @@ export default function AccessPage() {
)}
</h3>
{roots.length === 0 ? <p className="ac-empty">No categories yet.</p> : (
<ul className="ac-tree">
{roots.map(node => (
<Branch key={node.id} node={node} byParent={byParent} granted={granted}
covered={covered} busy={busy}
onToggle={(id, on) => toggleGrant('category', id, on)} />
))}
</ul>
/* Bounded: the whole tree is longer than any screen, and
a list that pushes the page down cannot be scanned. */
<div className="ac-tree-box">
<ul className="ac-tree">
{roots.map(node => (
<Branch key={node.id} node={node} byParent={byParent} granted={granted}
covered={covered} busy={busy} openPath={openPath} onOpen={openBranch}
onToggle={(id, on) => toggleGrant('category', id, on)} />
))}
</ul>
</div>
)}
</section>

View file

@ -13,6 +13,10 @@ const TREE = {
{ id: 1, name: 'Cardiology', parent_id: null, questions: 0, articles: 1 },
{ id: 2, name: 'Kawasaki disease', parent_id: 1, questions: 40, articles: 1 },
{ id: 3, name: 'Neurology', parent_id: null, questions: 12, articles: 0 },
// A second branch with a child of its own, so the accordion has two
// subjects to swap between.
{ id: 4, name: 'Dermatology', parent_id: null, questions: 0, articles: 0 },
{ id: 6, name: 'Eczema', parent_id: 4, questions: 8, articles: 0 },
],
libraries: [{ id: 1, name: 'Rashes', assets: 30 }],
}
@ -59,10 +63,32 @@ describe('access', () => {
'/access/5/grants', { kind: 'category', target_id: 1 }))
})
it('starts every branch collapsed, so the list can be scanned', async () => {
mock(users({ categories: [] }))
mount()
await pick('Ada')
// The tree runs to hundreds of rows. Opened by default it buried the image
// libraries and everything under it.
expect(screen.getByRole('checkbox', { name: 'Grant Cardiology' })).toBeInTheDocument()
expect(screen.queryByRole('checkbox', { name: 'Grant Kawasaki disease' })).toBeNull()
})
it('opening one branch closes the one before it', async () => {
mock(users({ categories: [] }))
mount()
await pick('Ada')
await userEvent.click(screen.getByRole('button', { name: /Expand Cardiology/ }))
expect(screen.getByRole('checkbox', { name: 'Grant Kawasaki disease' })).toBeInTheDocument()
// A second subject takes the first one's place rather than adding to it.
await userEvent.click(screen.getByRole('button', { name: /Expand Dermatology/ }))
expect(screen.queryByRole('checkbox', { name: 'Grant Kawasaki disease' })).toBeNull()
})
it('shows a child as covered by its parent, and will not let it be ticked separately', async () => {
mock(users({ categories: [1], categories_covered: 2 }))
mount()
await pick('Ada')
await userEvent.click(screen.getByRole('button', { name: /Expand Cardiology/ }))
const child = screen.getByRole('checkbox', { name: 'Grant Kawasaki disease' })
expect(child).toBeChecked()
expect(child).toBeDisabled()

View file

@ -61,8 +61,6 @@ export default function AdminPage({ section, embedded = false }) {
const [originalEmbedModel, setOriginalEmbedModel] = useState(null)
const [embedModelChanged, setEmbedModelChanged] = useState(false)
const [regenLoading, setRegenLoading] = useState(false)
const [classificationSnapshots, setClassificationSnapshots] = useState([])
const [rollbackLoading, setRollbackLoading] = useState(null)
useEffect(() => {
if (!user?.role || user.role !== 'admin') { navigate('/'); return }
@ -72,16 +70,14 @@ export default function AdminPage({ section, embedded = false }) {
const loadData = async (showSpinner = true) => {
if (showSpinner) setLoading(true)
try {
const [usersRes, modelsRes, settingsRes, snapshotsRes] = await Promise.all([
const [usersRes, modelsRes, settingsRes] = await Promise.all([
api.get('/admin/users'),
api.get('/admin/models'),
api.get('/admin/settings'),
api.get('/admin/classification-snapshots'),
])
setUsers(usersRes.data)
setModels(modelsRes.data)
setSettings(settingsRes.data)
setClassificationSnapshots(snapshotsRes.data)
setOriginalEmbedModel(settingsRes.data.embedding_model || '')
} catch (err) {
setError(err.response?.data?.detail || 'Failed to load data')
@ -250,28 +246,6 @@ export default function AdminPage({ section, embedded = false }) {
}
}
const rollbackClassificationSnapshot = async (snapshot) => {
const when = snapshot.created_at ? new Date(snapshot.created_at).toLocaleString() : `snapshot #${snapshot.id}`
const ok = await openConfirm(
`Rollback question tag assignments to snapshot #${snapshot.id} from ${when}? This replaces all current AI classification tag assignments.`,
{ title: 'Rollback Classification', confirmLabel: 'Rollback', danger: true },
)
if (!ok) return
setRollbackLoading(snapshot.id)
setError('')
try {
const res = await api.post(`/admin/classification-snapshots/${snapshot.id}/rollback`)
setSuccess(`Restored ${res.data.restored_links} tag assignments from snapshot #${snapshot.id}`)
const snapshotsRes = await api.get('/admin/classification-snapshots')
setClassificationSnapshots(snapshotsRes.data)
} catch (err) {
setError(err.response?.data?.detail || 'Failed to roll back classification snapshot')
} finally {
setRollbackLoading(null)
}
}
const addFromSearch = (modelId) => {
setNewModel(m => ({ ...m, model_id: modelId, name: modelId, task: searchTaskHint }))
setSearchResults([])
@ -335,7 +309,6 @@ export default function AdminPage({ section, embedded = false }) {
const adminTabs = [
{ id: 'models', label: 'AI Models' },
{ id: 'users', label: 'Users' },
{ id: 'safety', label: 'Safety' },
{ id: 'settings', label: 'More' },
]
@ -659,136 +632,11 @@ export default function AdminPage({ section, embedded = false }) {
</>
)}
{tab === 'safety' && (
<div className="card">
<h2>Classification Rollback</h2>
<p style={{ color: 'var(--text-muted)', fontSize: '0.85rem', marginBottom: 16 }}>
Every AI classification run saves the current question tag assignments before it starts. Use this if a classification run adds bad subjects, diseases, or keywords.
</p>
{classificationSnapshots.length === 0 ? (
<div style={{ color: 'var(--text-muted)', fontSize: '0.9rem' }}>
No classification snapshots have been saved yet.
</div>
) : (
<div style={{ display: 'grid', gap: 10 }}>
{classificationSnapshots.map(snapshot => (
<div key={snapshot.id} style={{ display: 'flex', justifyContent: 'space-between', alignItems: 'center', gap: 12, padding: 12, border: '1px solid var(--border)', borderRadius: 8 }}>
<div>
<strong style={{ display: 'block', marginBottom: 4 }}>Snapshot #{snapshot.id}</strong>
<div style={{ fontSize: '0.82rem', color: 'var(--text-muted)' }}>
{snapshot.created_at ? new Date(snapshot.created_at).toLocaleString() : 'Unknown date'} · {snapshot.question_count} tagged questions · {snapshot.link_count} tag assignments
</div>
{snapshot.created_by_email && (
<div style={{ fontSize: '0.78rem', color: 'var(--text-muted)', marginTop: 2 }}>
Created by {snapshot.created_by_name || snapshot.created_by_email}
</div>
)}
</div>
<button
className="btn btn-danger btn-sm"
onClick={() => rollbackClassificationSnapshot(snapshot)}
disabled={rollbackLoading === snapshot.id}
>
{rollbackLoading === snapshot.id ? 'Rolling back...' : 'Rollback'}
</button>
</div>
))}
</div>
)}
</div>
)}
{tab === 'settings' && (
<div className="card">
<h2>More Settings</h2>
<h2>Semantic search</h2>
<div style={{ marginTop: 16 }}>
{/* Public Registration */}
<div style={{ display: 'flex', alignItems: 'center', justifyContent: 'space-between', padding: '12px 0', borderBottom: '1px solid var(--border)' }}>
<div>
<strong style={{ display: 'block', marginBottom: 4 }}>Public Registration</strong>
<span style={{ fontSize: '0.85rem', color: 'var(--text-muted)' }}>
Allow new users to create accounts. Admins can always create users manually.
</span>
</div>
<label style={{ display: 'flex', alignItems: 'center', gap: 8, cursor: 'pointer' }}>
<input
type="checkbox"
checked={settings.registration_enabled}
onChange={async (e) => {
const enabled = e.target.checked
setSettings(s => ({ ...s, registration_enabled: enabled }))
try {
await api.put('/admin/settings', { registration_enabled: enabled })
setSuccess(`Registration ${enabled ? 'enabled' : 'disabled'}`)
} catch (err) {
setError(err.response?.data?.detail || 'Failed to update setting')
setSettings(s => ({ ...s, registration_enabled: !enabled }))
}
}}
style={{ width: 'auto', accentColor: 'var(--primary)' }}
/>
<span style={{ fontSize: '0.9rem', fontWeight: 600 }}>
{settings.registration_enabled ? 'Enabled' : 'Disabled'}
</span>
</label>
</div>
{/* SSO Only */}
{settings.sso_configured && (
<div style={{ display: 'flex', alignItems: 'center', justifyContent: 'space-between', padding: '12px 0', borderBottom: '1px solid var(--border)' }}>
<div>
<strong style={{ display: 'block', marginBottom: 4 }}>SSO-Only Login ({settings.sso_provider_name})</strong>
<span style={{ fontSize: '0.85rem', color: 'var(--text-muted)' }}>
Disable password login users must sign in via SSO. Admins can still use CLI to reset passwords.
</span>
</div>
<label style={{ display: 'flex', alignItems: 'center', gap: 8, cursor: 'pointer' }}>
<input
type="checkbox"
checked={settings.sso_only || false}
onChange={async (e) => {
const enabled = e.target.checked
setSettings(s => ({ ...s, sso_only: enabled }))
try {
await api.put('/admin/settings', { sso_only: enabled })
setSuccess(`SSO-only mode ${enabled ? 'enabled' : 'disabled'}`)
} catch (err) {
setError(err.response?.data?.detail || 'Failed to update setting')
setSettings(s => ({ ...s, sso_only: !enabled }))
}
}}
style={{ width: 'auto', accentColor: 'var(--primary)' }}
/>
<span style={{ fontSize: '0.9rem', fontWeight: 600 }}>
{settings.sso_only ? 'Enabled' : 'Disabled'}
</span>
</label>
</div>
)}
{/* Embedding model change warning */}
{embedModelChanged && (
<div style={{ padding: '12px 16px', background: '#fef3c7', border: '1px solid #f59e0b', borderRadius: 8, marginBottom: 0, marginTop: 8 }}>
<strong style={{ color: '#92400e', display: 'block', marginBottom: 6 }}>
Embedding model changed
</strong>
<span style={{ fontSize: '0.85rem', color: '#78350f' }}>
Existing question embeddings were generated with the previous model and may produce inaccurate semantic search results.
Regenerate them to restore full search quality.
</span>
<div style={{ marginTop: 10 }}>
<button className="btn btn-primary btn-sm" onClick={startRegeneration} disabled={regenLoading}>
{regenLoading ? 'Starting…' : 'Regenerate All Embeddings'}
</button>
<button className="btn btn-secondary btn-sm" style={{ marginLeft: 8 }} onClick={() => setEmbedModelChanged(false)}>
Dismiss
</button>
</div>
</div>
)}
{/* Embedding Model */}
<div style={{ padding: '16px 0' }}>
<strong style={{ display: 'block', marginBottom: 4 }}>Embedding Model</strong>

View file

@ -451,12 +451,9 @@ export default function SettingsPage() {
{ key: 'models', group: 'The site', icon: '🧠', label: 'AI models',
render: () => <AdminSection section="models" title="AI models"
description="Which model answers which kind of request, and what happens when one is unavailable." /> },
{ key: 'safety', group: 'The site', icon: '🛡️', label: 'Safety',
render: () => <AdminSection section="safety" title="Safety"
description="Limits on what the assistant will do, and what it refuses." /> },
{ key: 'search', group: 'The site', icon: '🔎', label: 'Search',
render: () => <AdminSection section="settings" title="Search and sign-up"
description="The embedding model behind semantic search, and how accounts are created." /> },
render: () => <AdminSection section="settings" title="Search"
description="The embedding model behind semantic search." /> },
] : []),
]

View file

@ -73,10 +73,14 @@ describe('settings', () => {
await screen.findByRole('heading', { name: 'Account' })
// The old page offered a grid of links, one of which went to /admin and
// its own row of tabs. These are sections of this page now.
for (const label of ['People', 'AI models', 'Safety', 'Search']) {
for (const label of ['People', 'AI models', 'Search']) {
expect(screen.getByRole('button', { name: new RegExp(label) })).toBeInTheDocument()
}
expect(screen.queryByRole('link', { name: /Admin dashboard/ })).toBeNull()
// Safety held one thing: rolling back AI classification tag assignments.
// The classifier is gone, so the section had nothing left to be about, and
// permissions the other thing it might have held are on Access.
expect(screen.queryByRole('button', { name: /Safety/ })).toBeNull()
})
it('carries the exam objective, which was only reachable from the navbar', async () => {