From 1521198cb19d08f3df4fef68d2c2d7ff02d62a89 Mon Sep 17 00:00:00 2001
From: pyr0ball <pyroballpcs@gmail.com>
Date: Tue, 5 May 2026 21:35:13 -0700
Subject: [PATCH] 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()
---
 web/src/components/nodes/GpuRow.vue           | 20 +--------
 web/src/components/nodes/HfNodeModelPanel.vue | 25 +++++++----
 web/src/components/nodes/NodeCard.vue         | 29 +------------
 web/src/components/nodes/OllamaModelPanel.vue | 42 +++++++++++++++----
 web/src/components/nodes/ServiceBadge.vue     |  4 +-
 web/src/types/nodes.ts                        | 27 ++++++++++++
 web/src/views/NodeManagementView.vue          | 42 ++++---------------
 7 files changed, 92 insertions(+), 97 deletions(-)
 create mode 100644 web/src/types/nodes.ts

diff --git a/web/src/components/nodes/GpuRow.vue b/web/src/components/nodes/GpuRow.vue
index 3de9cc5..e73d11a 100644
--- a/web/src/components/nodes/GpuRow.vue
+++ b/web/src/components/nodes/GpuRow.vue
@@ -1,25 +1,7 @@
 <script setup lang="ts">
 import { ref, computed } from 'vue'
 import ServiceBadge from './ServiceBadge.vue'
-
-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
-}
+import type { GpuEntry, ServiceInfo } from '../../types/nodes'
 
 const props = defineProps<{
   gpu: GpuEntry
diff --git a/web/src/components/nodes/HfNodeModelPanel.vue b/web/src/components/nodes/HfNodeModelPanel.vue
index b3dff43..4eb8cbd 100644
--- a/web/src/components/nodes/HfNodeModelPanel.vue
+++ b/web/src/components/nodes/HfNodeModelPanel.vue
@@ -1,5 +1,5 @@
 <script setup lang="ts">
-import { ref, onMounted } from 'vue'
+import { ref, onMounted, onUnmounted } from 'vue'
 
 interface CatalogEntry {
   path: string
@@ -27,15 +27,22 @@ const profile = ref<NodeProfile | null>(null)
 const loading = ref(true)
 const error = ref('')
 
+let fetchAbort: AbortController | null = null
+
 async function fetchProfile() {
+  fetchAbort?.abort()
+  fetchAbort = new AbortController()
   loading.value = true
   error.value = ''
   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.ok) throw new Error(`HTTP ${r.status}`)
     profile.value = await r.json() as NodeProfile
   } catch (e) {
+    if (e instanceof Error && e.name === 'AbortError') return
     error.value = e instanceof Error ? e.message : 'Failed to load profile'
   } finally {
     loading.value = false
@@ -43,6 +50,7 @@ async function fetchProfile() {
 }
 
 onMounted(fetchProfile)
+onUnmounted(() => { fetchAbort?.abort() })
 </script>
 
 <template>
@@ -54,10 +62,12 @@ onMounted(fetchProfile)
       Models downloaded there are automatically registered in node catalogs.
     </p>
 
-    <div v-if="loading" class="panel-loading" aria-live="polite">Loading catalog...</div>
-    <div v-else-if="error" class="panel-error" role="alert">{{ error }}</div>
-    <div v-else-if="!profile" class="panel-empty">No profile loaded for this node.</div>
-    <div v-else class="catalog-body">
+    <div aria-live="polite" aria-atomic="true" class="sr-announce">
+      <span v-if="loading">Loading catalog...</span>
+    </div>
+    <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
         v-for="(svcInfo, svcName) in profile.services"
         :key="String(svcName)"
@@ -117,6 +127,7 @@ onMounted(fetchProfile)
 .catalog-model { font-family: monospace; flex: 1; }
 .catalog-vram { color: var(--text-secondary, #888); white-space: nowrap; }
 .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; }
 </style>
diff --git a/web/src/components/nodes/NodeCard.vue b/web/src/components/nodes/NodeCard.vue
index 5ad0ff3..792c919 100644
--- a/web/src/components/nodes/NodeCard.vue
+++ b/web/src/components/nodes/NodeCard.vue
@@ -3,34 +3,7 @@ import { ref } from 'vue'
 import GpuRow from './GpuRow.vue'
 import OllamaModelPanel from './OllamaModelPanel.vue'
 import HfNodeModelPanel from './HfNodeModelPanel.vue'
-
-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>
-}
+import type { NodeSummary } from '../../types/nodes'
 
 const props = defineProps<{ node: NodeSummary }>()
 const emit = defineEmits<{ updated: [] }>()
diff --git a/web/src/components/nodes/OllamaModelPanel.vue b/web/src/components/nodes/OllamaModelPanel.vue
index 5ee54dd..d30b0e4 100644
--- a/web/src/components/nodes/OllamaModelPanel.vue
+++ b/web/src/components/nodes/OllamaModelPanel.vue
@@ -1,5 +1,5 @@
 <script setup lang="ts">
-import { ref, onMounted } from 'vue'
+import { ref, onMounted, onUnmounted } from 'vue'
 
 const props = defineProps<{ nodeId: string }>()
 
@@ -18,15 +18,26 @@ const pullStatus = ref('')
 const pullPct = ref(0)
 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() {
+  fetchAbort?.abort()
+  fetchAbort = new AbortController()
   loading.value = true
   loadError.value = ''
   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 }
     if (data.error) { loadError.value = data.error; return }
     models.value = data.models ?? []
   } catch (e) {
+    if (e instanceof Error && e.name === 'AbortError') return
     loadError.value = e instanceof Error ? e.message : 'Failed to load models'
   } finally {
     loading.value = false
@@ -41,11 +52,15 @@ async function doPull() {
   pullError.value = ''
   pullPct.value = 0
 
+  const ctrl = new AbortController()
+  abortCtrl.value = ctrl
+
   try {
     const resp = await fetch(`/api/nodes-mgmt/nodes/${props.nodeId}/models/ollama/pull`, {
       method: 'POST',
       headers: { 'Content-Type': 'application/json' },
       body: JSON.stringify({ name }),
+      signal: ctrl.signal,
     })
     if (!resp.ok) throw new Error(`HTTP ${resp.status}`)
     if (!resp.body) throw new Error('No response body')
@@ -68,8 +83,7 @@ async function doPull() {
           }
           if (evt.error) {
             pullError.value = evt.error
-            pulling.value = false
-            return
+            break
           }
           if (evt.status) pullStatus.value = evt.status
           if (evt.total && evt.completed) {
@@ -78,15 +92,20 @@ async function doPull() {
           if (evt.status === 'success') {
             pullStatus.value = 'Done!'
             pullName.value = ''
-            await fetchModels()
+            break
           }
         } catch { /* skip malformed line */ }
       }
     }
+
+    // Refresh model list after the stream closes (success or benign end)
+    await fetchModels()
   } catch (e) {
+    if (e instanceof Error && e.name === 'AbortError') return
     pullError.value = e instanceof Error ? e.message : 'Pull failed'
   } finally {
     pulling.value = false
+    abortCtrl.value = null
   }
 }
 
@@ -109,6 +128,10 @@ function formatSize(bytes: number): string {
 }
 
 onMounted(fetchModels)
+onUnmounted(() => {
+  abortCtrl.value?.abort()
+  fetchAbort?.abort()
+})
 </script>
 
 <template>
@@ -150,9 +173,11 @@ onMounted(fetchModels)
       </span>
     </div>
 
-    <div v-if="loading" class="panel-loading" aria-live="polite">Loading...</div>
-    <div v-else-if="loadError" class="panel-error" role="alert">{{ loadError }}</div>
-    <ul v-else class="model-list" role="list">
+    <div aria-live="polite" aria-atomic="true" class="sr-announce">
+      <span v-if="loading">Loading...</span>
+    </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-for="m in models" :key="m.name" class="model-item">
         <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-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; }
+.sr-announce { min-height: 1.2em; }
 .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-item {
diff --git a/web/src/components/nodes/ServiceBadge.vue b/web/src/components/nodes/ServiceBadge.vue
index 93ca73b..d1f1f44 100644
--- a/web/src/components/nodes/ServiceBadge.vue
+++ b/web/src/components/nodes/ServiceBadge.vue
@@ -17,7 +17,7 @@ const props = defineProps<{
 
 const emit = defineEmits<{ toggle: [] }>()
 
-const STATE_LABELS: Record<string, string> = {
+const STATE_LABELS: Record<ServiceState, string> = {
   running: 'Running',
   stopped: 'Stopped',
   'assigned-only': 'Assigned',
@@ -27,7 +27,7 @@ const STATE_LABELS: Record<string, string> = {
   unknown: 'Unknown',
 }
 
-const STATE_ICONS: Record<string, string> = {
+const STATE_ICONS: Record<ServiceState, string> = {
   running: '▶',
   stopped: '⏹',
   'assigned-only': '📌',
diff --git a/web/src/types/nodes.ts b/web/src/types/nodes.ts
new file mode 100644
index 0000000..bf6e5a6
--- /dev/null
+++ b/web/src/types/nodes.ts
@@ -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>
+}
diff --git a/web/src/views/NodeManagementView.vue b/web/src/views/NodeManagementView.vue
index 99b9569..0c461be 100644
--- a/web/src/views/NodeManagementView.vue
+++ b/web/src/views/NodeManagementView.vue
@@ -1,34 +1,7 @@
 <script setup lang="ts">
 import { ref, onMounted } from 'vue'
 import NodeCard from '../components/nodes/NodeCard.vue'
-
-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>
-}
+import type { NodeSummary } from '../types/nodes'
 
 const nodes = ref<NodeSummary[]>([])
 const loading = ref(true)
@@ -40,7 +13,7 @@ async function fetchNodes() {
   try {
     const r = await fetch('/api/nodes-mgmt/nodes')
     if (!r.ok) throw new Error(`HTTP ${r.status}`)
-    nodes.value = await r.json()
+    nodes.value = (await r.json()) as NodeSummary[]
   } catch (e) {
     error.value = e instanceof Error ? e.message : 'Failed to load nodes'
   } finally {
@@ -58,12 +31,14 @@ onMounted(fetchNodes)
       <button class="btn-secondary" @click="fetchNodes" :disabled="loading">Refresh</button>
     </header>
 
-    <div v-if="loading" class="nodes-status" aria-live="polite">Loading nodes...</div>
-    <div v-else-if="error" class="nodes-status nodes-error" role="alert">{{ error }}</div>
-    <div v-else-if="nodes.length === 0" class="nodes-status">
+    <div aria-live="polite" aria-atomic="true" class="sr-announce">
+      <span v-if="loading">Loading nodes...</span>
+    </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.
     </div>
-    <div v-else class="nodes-grid">
+    <div v-else-if="!loading" class="nodes-grid">
       <NodeCard
         v-for="node in nodes"
         :key="node.node_id"
@@ -90,4 +65,5 @@ onMounted(fetchNodes)
   text-align: center;
 }
 .nodes-error { color: var(--color-error, #fc8181); }
+.sr-announce { min-height: 1.2em; }
 </style>