fix: code quality fixes from review (SSE abort, aria-live, shared types, type safety)

- Add AbortController to SSE pull stream in OllamaModelPanel; abort on unmount
- Fix SSE loop: break on success/error events, call fetchModels() after the loop
- Add AbortController to fetchModels() and fetchProfile() one-shot fetches
- Add onUnmounted cleanup to both panel components
- Extract GpuEntry, ServiceInfo, NodeSummary to web/src/types/nodes.ts
- Remove duplicate interface definitions from NodeCard, GpuRow, NodeManagementView
- Fix aria-live regions: persistent container with v-if on inner span (avoids
  screen reader announcement miss on initial mount)
- Tighten STATE_LABELS/STATE_ICONS to Record<ServiceState, string> for exhaustiveness
- Add explicit (await r.json()) as NodeSummary[] cast in fetchNodes()
This commit is contained in:
pyr0ball 2026-05-05 21:35:13 -07:00
parent 8dda040480
commit 1521198cb1
7 changed files with 92 additions and 97 deletions

View file

@ -1,25 +1,7 @@
<script setup lang="ts"> <script setup lang="ts">
import { ref, computed } from 'vue' import { ref, computed } from 'vue'
import ServiceBadge from './ServiceBadge.vue' import ServiceBadge from './ServiceBadge.vue'
import type { GpuEntry, ServiceInfo } from '../../types/nodes'
interface GpuEntry {
gpu_id: number
card: string
vram_total_mb: number
vram_used_mb: number
vram_free_mb: number
temp_c: number | null
utilization_pct: number | null
compute_cap: number | null
services_assigned: string[]
services_running: string[]
}
interface ServiceInfo {
min_compute_cap: number
max_mb: number
catalog_size: number
}
const props = defineProps<{ const props = defineProps<{
gpu: GpuEntry gpu: GpuEntry

View file

@ -1,5 +1,5 @@
<script setup lang="ts"> <script setup lang="ts">
import { ref, onMounted } from 'vue' import { ref, onMounted, onUnmounted } from 'vue'
interface CatalogEntry { interface CatalogEntry {
path: string path: string
@ -27,15 +27,22 @@ const profile = ref<NodeProfile | null>(null)
const loading = ref(true) const loading = ref(true)
const error = ref('') const error = ref('')
let fetchAbort: AbortController | null = null
async function fetchProfile() { async function fetchProfile() {
fetchAbort?.abort()
fetchAbort = new AbortController()
loading.value = true loading.value = true
error.value = '' error.value = ''
try { try {
const r = await fetch(`/api/nodes-mgmt/nodes/${props.nodeId}/profile`) const r = await fetch(`/api/nodes-mgmt/nodes/${props.nodeId}/profile`, {
signal: fetchAbort.signal,
})
if (r.status === 404) { profile.value = null; return } if (r.status === 404) { profile.value = null; return }
if (!r.ok) throw new Error(`HTTP ${r.status}`) if (!r.ok) throw new Error(`HTTP ${r.status}`)
profile.value = await r.json() as NodeProfile profile.value = await r.json() as NodeProfile
} catch (e) { } catch (e) {
if (e instanceof Error && e.name === 'AbortError') return
error.value = e instanceof Error ? e.message : 'Failed to load profile' error.value = e instanceof Error ? e.message : 'Failed to load profile'
} finally { } finally {
loading.value = false loading.value = false
@ -43,6 +50,7 @@ async function fetchProfile() {
} }
onMounted(fetchProfile) onMounted(fetchProfile)
onUnmounted(() => { fetchAbort?.abort() })
</script> </script>
<template> <template>
@ -54,10 +62,12 @@ onMounted(fetchProfile)
Models downloaded there are automatically registered in node catalogs. Models downloaded there are automatically registered in node catalogs.
</p> </p>
<div v-if="loading" class="panel-loading" aria-live="polite">Loading catalog...</div> <div aria-live="polite" aria-atomic="true" class="sr-announce">
<div v-else-if="error" class="panel-error" role="alert">{{ error }}</div> <span v-if="loading">Loading catalog...</span>
<div v-else-if="!profile" class="panel-empty">No profile loaded for this node.</div> </div>
<div v-else class="catalog-body"> <div v-if="error" class="panel-error" role="alert">{{ error }}</div>
<div v-else-if="!loading && !profile" class="panel-empty">No profile loaded for this node.</div>
<div v-else-if="!loading && profile" class="catalog-body">
<div <div
v-for="(svcInfo, svcName) in profile.services" v-for="(svcInfo, svcName) in profile.services"
:key="String(svcName)" :key="String(svcName)"
@ -117,6 +127,7 @@ onMounted(fetchProfile)
.catalog-model { font-family: monospace; flex: 1; } .catalog-model { font-family: monospace; flex: 1; }
.catalog-vram { color: var(--text-secondary, #888); white-space: nowrap; } .catalog-vram { color: var(--text-secondary, #888); white-space: nowrap; }
.catalog-desc { color: var(--text-secondary, #888); font-size: 0.75rem; flex: 2; } .catalog-desc { color: var(--text-secondary, #888); font-size: 0.75rem; flex: 2; }
.catalog-empty, .panel-empty, .panel-loading { color: var(--text-secondary, #888); font-size: 0.875rem; } .catalog-empty, .panel-empty { color: var(--text-secondary, #888); font-size: 0.875rem; }
.sr-announce { min-height: 1.2em; }
.panel-error { color: var(--color-error, #fc8181); font-size: 0.8rem; } .panel-error { color: var(--color-error, #fc8181); font-size: 0.8rem; }
</style> </style>

View file

@ -3,34 +3,7 @@ import { ref } from 'vue'
import GpuRow from './GpuRow.vue' import GpuRow from './GpuRow.vue'
import OllamaModelPanel from './OllamaModelPanel.vue' import OllamaModelPanel from './OllamaModelPanel.vue'
import HfNodeModelPanel from './HfNodeModelPanel.vue' import HfNodeModelPanel from './HfNodeModelPanel.vue'
import type { NodeSummary } from '../../types/nodes'
interface GpuEntry {
gpu_id: number
card: string
vram_total_mb: number
vram_used_mb: number
vram_free_mb: number
temp_c: number | null
utilization_pct: number | null
compute_cap: number | null
services_assigned: string[]
services_running: string[]
}
interface ServiceInfo {
min_compute_cap: number
max_mb: number
catalog_size: number
}
interface NodeSummary {
node_id: string
online: boolean
agent_url: string
gpus: GpuEntry[]
profile_loaded: boolean
services_catalog: Record<string, ServiceInfo>
}
const props = defineProps<{ node: NodeSummary }>() const props = defineProps<{ node: NodeSummary }>()
const emit = defineEmits<{ updated: [] }>() const emit = defineEmits<{ updated: [] }>()

View file

@ -1,5 +1,5 @@
<script setup lang="ts"> <script setup lang="ts">
import { ref, onMounted } from 'vue' import { ref, onMounted, onUnmounted } from 'vue'
const props = defineProps<{ nodeId: string }>() const props = defineProps<{ nodeId: string }>()
@ -18,15 +18,26 @@ const pullStatus = ref('')
const pullPct = ref(0) const pullPct = ref(0)
const pullError = ref('') const pullError = ref('')
// AbortController for the SSE pull stream
const abortCtrl = ref<AbortController | null>(null)
// AbortController for the one-shot fetchModels request
let fetchAbort: AbortController | null = null
async function fetchModels() { async function fetchModels() {
fetchAbort?.abort()
fetchAbort = new AbortController()
loading.value = true loading.value = true
loadError.value = '' loadError.value = ''
try { try {
const r = await fetch(`/api/nodes-mgmt/nodes/${props.nodeId}/models/ollama`) const r = await fetch(`/api/nodes-mgmt/nodes/${props.nodeId}/models/ollama`, {
signal: fetchAbort.signal,
})
const data = await r.json() as { models?: OllamaModel[]; error?: string } const data = await r.json() as { models?: OllamaModel[]; error?: string }
if (data.error) { loadError.value = data.error; return } if (data.error) { loadError.value = data.error; return }
models.value = data.models ?? [] models.value = data.models ?? []
} catch (e) { } catch (e) {
if (e instanceof Error && e.name === 'AbortError') return
loadError.value = e instanceof Error ? e.message : 'Failed to load models' loadError.value = e instanceof Error ? e.message : 'Failed to load models'
} finally { } finally {
loading.value = false loading.value = false
@ -41,11 +52,15 @@ async function doPull() {
pullError.value = '' pullError.value = ''
pullPct.value = 0 pullPct.value = 0
const ctrl = new AbortController()
abortCtrl.value = ctrl
try { try {
const resp = await fetch(`/api/nodes-mgmt/nodes/${props.nodeId}/models/ollama/pull`, { const resp = await fetch(`/api/nodes-mgmt/nodes/${props.nodeId}/models/ollama/pull`, {
method: 'POST', method: 'POST',
headers: { 'Content-Type': 'application/json' }, headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ name }), body: JSON.stringify({ name }),
signal: ctrl.signal,
}) })
if (!resp.ok) throw new Error(`HTTP ${resp.status}`) if (!resp.ok) throw new Error(`HTTP ${resp.status}`)
if (!resp.body) throw new Error('No response body') if (!resp.body) throw new Error('No response body')
@ -68,8 +83,7 @@ async function doPull() {
} }
if (evt.error) { if (evt.error) {
pullError.value = evt.error pullError.value = evt.error
pulling.value = false break
return
} }
if (evt.status) pullStatus.value = evt.status if (evt.status) pullStatus.value = evt.status
if (evt.total && evt.completed) { if (evt.total && evt.completed) {
@ -78,15 +92,20 @@ async function doPull() {
if (evt.status === 'success') { if (evt.status === 'success') {
pullStatus.value = 'Done!' pullStatus.value = 'Done!'
pullName.value = '' pullName.value = ''
await fetchModels() break
} }
} catch { /* skip malformed line */ } } catch { /* skip malformed line */ }
} }
} }
// Refresh model list after the stream closes (success or benign end)
await fetchModels()
} catch (e) { } catch (e) {
if (e instanceof Error && e.name === 'AbortError') return
pullError.value = e instanceof Error ? e.message : 'Pull failed' pullError.value = e instanceof Error ? e.message : 'Pull failed'
} finally { } finally {
pulling.value = false pulling.value = false
abortCtrl.value = null
} }
} }
@ -109,6 +128,10 @@ function formatSize(bytes: number): string {
} }
onMounted(fetchModels) onMounted(fetchModels)
onUnmounted(() => {
abortCtrl.value?.abort()
fetchAbort?.abort()
})
</script> </script>
<template> <template>
@ -150,9 +173,11 @@ onMounted(fetchModels)
</span> </span>
</div> </div>
<div v-if="loading" class="panel-loading" aria-live="polite">Loading...</div> <div aria-live="polite" aria-atomic="true" class="sr-announce">
<div v-else-if="loadError" class="panel-error" role="alert">{{ loadError }}</div> <span v-if="loading">Loading...</span>
<ul v-else class="model-list" role="list"> </div>
<div v-if="loadError" class="panel-error" role="alert">{{ loadError }}</div>
<ul v-if="!loading && !loadError" class="model-list" role="list">
<li v-if="!models.length" class="model-empty">No Ollama models installed on this node.</li> <li v-if="!models.length" class="model-empty">No Ollama models installed on this node.</li>
<li v-for="m in models" :key="m.name" class="model-item"> <li v-for="m in models" :key="m.name" class="model-item">
<span class="model-name">{{ m.name }}</span> <span class="model-name">{{ m.name }}</span>
@ -198,6 +223,7 @@ onMounted(fetchModels)
.progress-fill { height: 100%; background: var(--color-primary, #4080ff); transition: width 0.2s; } .progress-fill { height: 100%; background: var(--color-primary, #4080ff); transition: width 0.2s; }
.progress-label { font-size: 0.75rem; color: var(--text-secondary, #888); } .progress-label { font-size: 0.75rem; color: var(--text-secondary, #888); }
.pull-error, .panel-error { color: var(--color-error, #fc8181); font-size: 0.8rem; margin-bottom: 0.5rem; } .pull-error, .panel-error { color: var(--color-error, #fc8181); font-size: 0.8rem; margin-bottom: 0.5rem; }
.sr-announce { min-height: 1.2em; }
.panel-loading { color: var(--text-secondary, #888); font-size: 0.875rem; } .panel-loading { color: var(--text-secondary, #888); font-size: 0.875rem; }
.model-list { list-style: none; margin: 0; padding: 0; display: flex; flex-direction: column; gap: 0.3rem; } .model-list { list-style: none; margin: 0; padding: 0; display: flex; flex-direction: column; gap: 0.3rem; }
.model-item { .model-item {

View file

@ -17,7 +17,7 @@ const props = defineProps<{
const emit = defineEmits<{ toggle: [] }>() const emit = defineEmits<{ toggle: [] }>()
const STATE_LABELS: Record<string, string> = { const STATE_LABELS: Record<ServiceState, string> = {
running: 'Running', running: 'Running',
stopped: 'Stopped', stopped: 'Stopped',
'assigned-only': 'Assigned', 'assigned-only': 'Assigned',
@ -27,7 +27,7 @@ const STATE_LABELS: Record<string, string> = {
unknown: 'Unknown', unknown: 'Unknown',
} }
const STATE_ICONS: Record<string, string> = { const STATE_ICONS: Record<ServiceState, string> = {
running: '▶', running: '▶',
stopped: '⏹', stopped: '⏹',
'assigned-only': '📌', 'assigned-only': '📌',

27
web/src/types/nodes.ts Normal file
View file

@ -0,0 +1,27 @@
export interface GpuEntry {
gpu_id: number
card: string
vram_total_mb: number
vram_used_mb: number
vram_free_mb: number
temp_c: number | null
utilization_pct: number | null
compute_cap: number | null
services_assigned: string[]
services_running: string[]
}
export interface ServiceInfo {
min_compute_cap: number
max_mb: number
catalog_size: number
}
export interface NodeSummary {
node_id: string
online: boolean
agent_url: string
gpus: GpuEntry[]
profile_loaded: boolean
services_catalog: Record<string, ServiceInfo>
}

View file

@ -1,34 +1,7 @@
<script setup lang="ts"> <script setup lang="ts">
import { ref, onMounted } from 'vue' import { ref, onMounted } from 'vue'
import NodeCard from '../components/nodes/NodeCard.vue' import NodeCard from '../components/nodes/NodeCard.vue'
import type { NodeSummary } from '../types/nodes'
interface GpuEntry {
gpu_id: number
card: string
vram_total_mb: number
vram_used_mb: number
vram_free_mb: number
temp_c: number | null
utilization_pct: number | null
compute_cap: number | null
services_assigned: string[]
services_running: string[]
}
interface ServiceInfo {
min_compute_cap: number
max_mb: number
catalog_size: number
}
interface NodeSummary {
node_id: string
online: boolean
agent_url: string
gpus: GpuEntry[]
profile_loaded: boolean
services_catalog: Record<string, ServiceInfo>
}
const nodes = ref<NodeSummary[]>([]) const nodes = ref<NodeSummary[]>([])
const loading = ref(true) const loading = ref(true)
@ -40,7 +13,7 @@ async function fetchNodes() {
try { try {
const r = await fetch('/api/nodes-mgmt/nodes') const r = await fetch('/api/nodes-mgmt/nodes')
if (!r.ok) throw new Error(`HTTP ${r.status}`) if (!r.ok) throw new Error(`HTTP ${r.status}`)
nodes.value = await r.json() nodes.value = (await r.json()) as NodeSummary[]
} catch (e) { } catch (e) {
error.value = e instanceof Error ? e.message : 'Failed to load nodes' error.value = e instanceof Error ? e.message : 'Failed to load nodes'
} finally { } finally {
@ -58,12 +31,14 @@ onMounted(fetchNodes)
<button class="btn-secondary" @click="fetchNodes" :disabled="loading">Refresh</button> <button class="btn-secondary" @click="fetchNodes" :disabled="loading">Refresh</button>
</header> </header>
<div v-if="loading" class="nodes-status" aria-live="polite">Loading nodes...</div> <div aria-live="polite" aria-atomic="true" class="sr-announce">
<div v-else-if="error" class="nodes-status nodes-error" role="alert">{{ error }}</div> <span v-if="loading">Loading nodes...</span>
<div v-else-if="nodes.length === 0" class="nodes-status"> </div>
<div v-if="error" class="nodes-status nodes-error" role="alert">{{ error }}</div>
<div v-else-if="!loading && nodes.length === 0" class="nodes-status">
No nodes found. Check <code>coordinator_url</code> in config. No nodes found. Check <code>coordinator_url</code> in config.
</div> </div>
<div v-else class="nodes-grid"> <div v-else-if="!loading" class="nodes-grid">
<NodeCard <NodeCard
v-for="node in nodes" v-for="node in nodes"
:key="node.node_id" :key="node.node_id"
@ -90,4 +65,5 @@ onMounted(fetchNodes)
text-align: center; text-align: center;
} }
.nodes-error { color: var(--color-error, #fc8181); } .nodes-error { color: var(--color-error, #fc8181); }
.sr-announce { min-height: 1.2em; }
</style> </style>