merge storage and disk health into one card with two sections

Volume capacity and physical drive health were two adjacent cards, and the operator reading a full disk is usually the operator reading a dying disk in the same minute. They are now one Storage card with two sections. Visual grouping only - the two tables share no key, storage being per volume and health per physical device, and one drive can back several volumes or none the operator sees. Nothing is reconciled across the sections; this is not the mapped-drive merge where two sources described the same drive and the worse status won. The tabs render type was the only sections-in-one-card mechanism and emitted label/value fields with no table path, so it now takes table sections too. That is the renderer every plugin uses, so both behaviour changes in it are mutation-tested. Two things only surfaced on contact: the empty-section filter had to grow a table branch, because a table section declares columns up front and dropping it when it has no rows takes its tab with it and reads as no disks rather than nothing reported; and full-width span was keyed off the declared render type, but a tabs panel holding a six-column table needs the same span and that is only knowable from fetched data. The diskhealth route stays. It no longer backs a panel, but it is a stable read for callers outside the renderer and removing it for a UI-only change would break them.

docs/notes/endpoint-telemetry-and-ui-phases.md

@@ -242,24 +242,25 @@ Depends on Phase 2 data existing.
   until the collection question is settled.
 - Boot type placement to be decided when the data is visible.
 - **Storage + Disk health into one card with two collapsible sections** -
-  AGREED, deferred to its own phase. VISUAL GROUPING ONLY. These two tables
-  cannot be merged into shared rows: Storage is per volume, Disk health is per
-  physical device, and one drive can back several volumes or none the operator
-  sees. This is emphatically NOT the Phase 1 mapped-drive treatment, where two
-  sources described the same drive and the worse status won. Nothing here gets
-  reconciled.
-  Cost, measured rather than guessed: the `tabs` render type is the only
-  existing sections-in-one-card mechanism and it emits label/value fields only
-  (PluginAssetPanels.vue), with no table path. Both cards here are `table`
-  renders with different columns AND different keys, so nothing existing
-  carries them. Preferred approach is extending `tabs` to support table
-  sections (~40 lines in the shared renderer plus a helper), then collapsing
-  the two routes into one returning `sections`. A new `sections` render type is
-  the alternative but is a larger addition to that fixed set.
-  Deferred deliberately: this modifies the renderer EVERY plugin uses, so it
-  needs its own regression tests and must not be bundled with the Phase 2
-  telemetry commits - otherwise a UI regression and a telemetry regression
-  become one commit to bisect.
+  DONE. VISUAL GROUPING ONLY. These two tables are not merged into shared rows:
+  Storage is per volume, Disk health is per physical device, and one drive can
+  back several volumes or none the operator sees. This is emphatically NOT the
+  Phase 1 mapped-drive treatment, where two sources described the same drive and
+  the worse status won. Nothing here is reconciled.
+  Built as predicted: `tabs` was extended to support table sections (columns/
+  rows/empty per section) in the shared renderer, and the two routes collapsed
+  into one `/storage` returning `sections`. `/diskhealth` is kept as a route -
+  it is a stable read for non-panel callers, and removing it for a UI-only
+  change would break them.
+  Two things only found by building it:
+  - The empty-section filter had to grow a table branch. A table section
+    declares its columns up front and so must survive having no rows; dropping
+    it takes its TAB with it, and a missing tab reads as "this machine has no
+    disks" rather than "nothing was reported".
+  - Full-width span was keyed off `render === 'table'`. A `tabs` panel holding a
+    six-column table needs the same span, and whether it does is only knowable
+    from FETCHED DATA, not the declaration - hence `spansFullWidth()`.
+  Both are mutation-tested in `tabSectionsTable.spec.js`.
   Checked and cleared: the "inline label/value pairs, not nested cards" density
   rule in Phase 5 does NOT block this. That rule is scoped to the PC Details
   HEADER block, where the constraint is adding zero height so the tab strip is

frontend/src/components/PluginAssetPanels.vue

@@ -3,7 +3,7 @@
        a panel as JSON (title, endpoint, render mode, field map); this renders it
        on any asset detail page, so a plugin adds detail-page UI with no Vue. -->
   <template v-for="panel in groupedPanels" :key="panel.id">
-    <div v-if="panelVisible(panel)" class="section-card" :class="{ 'pap-span': panel.render === 'table' }">
+    <div v-if="panelVisible(panel)" class="section-card" :class="{ 'pap-span': spansFullWidth(panel) }">
       <!-- collapsible panels get a clickable header with a caret and, when the
            panel declares a status rollup, a green/yellow/red dot for the worst
            row (a stopped service or a warning/critical event). --><h3
@@ -120,7 +120,39 @@
             @click="setTab(panel, si)"
           >{{ section.label }}</button>
         </div>
-        <div class="pap-kv">
+        <!-- A section carrying columns renders as a table; otherwise it is the
+             original label/value grid. Both shapes coexist in one panel. -->
+        <div
+          v-if="((tabSections(panel)[activeTab(panel)] || {}).columns || []).length"
+          class="pap-table-wrap"
+        >
+          <table class="pap-table">
+            <thead>
+              <tr>
+                <th
+                  v-for="col in (tabSections(panel)[activeTab(panel)] || {}).columns"
+                  :key="col.key"
+                >{{ col.label }}</th>
+              </tr>
+            </thead>
+            <tbody>
+              <tr
+                v-for="(row, ri) in (tabSections(panel)[activeTab(panel)] || {}).rows || []"
+                :key="ri"
+              >
+                <td
+                  v-for="col in (tabSections(panel)[activeTab(panel)] || {}).columns"
+                  :key="col.key"
+                >{{ cell(row, col) }}</td>
+              </tr>
+            </tbody>
+          </table>
+          <div
+            v-if="!((tabSections(panel)[activeTab(panel)] || {}).rows || []).length"
+            class="pap-empty"
+          >{{ (tabSections(panel)[activeTab(panel)] || {}).empty || 'Nothing reported.' }}</div>
+        </div>
+        <div v-else class="pap-kv">
           <div
             v-for="(field, fi) in (tabSections(panel)[activeTab(panel)] || {}).fields || []"
             :key="fi"
@@ -258,7 +290,7 @@ import { colorStyle } from '@/utils/colorStyle'
 import {
   toApiPath, rows, panelVisible, mapTitle, mapBadge, mapMeta, mapBody, manageLink,
   keyvalueFields, tableColumns, cell, badges, tabSections, rollupStatus,
-  rowAction, facets, facetedRows, formatValue,
+  rowAction, facets, facetedRows, formatValue, spansFullWidth,
 } from './pluginAssetPanels'
 
 // Operator report screenshots are served by a JWT-gated route, so a bare

frontend/src/components/pluginAssetPanels.js

@@ -218,17 +218,38 @@ export function rollupStatus(panel) {
 // tabs: [{ label, fields: [{label, value, mono}] }]. One section is shown at a
 // time, so a panel with 30 values stays the height of its largest section
 // instead of dragging one multicol column far past the other.
+//
+// A section may instead carry { columns, rows, empty } and render as a table.
+// This exists so two tables that belong side by side for the reader but share
+// NO key can sit in one card - Storage is per volume, Disk health is per
+// physical device. The sections are independent tables by design: nothing here
+// reconciles rows across them, unlike the mapped-drive merge where two sources
+// described the same drive.
 export function tabSections(panel) {
   const data = panel._data
   const sections = (data && Array.isArray(data.sections)) ? data.sections : []
   return sections
-    .map((s) => ({
-      label: s.label,
-      fields: (s.fields || []).map((f) => ({
-        label: f.label, value: formatValue(f.value, f), mono: !!f.mono,
-      })),
-    }))
-    .filter((s) => s.fields.length > 0)
+    .map((s) => {
+      if (Array.isArray(s.columns) && s.columns.length) {
+        return {
+          label: s.label,
+          columns: s.columns.map((c) => ({ key: c.key, label: c.label || humanize(c.key) })),
+          rows: Array.isArray(s.rows) ? s.rows : [],
+          empty: s.empty || '',
+          fields: [],
+        }
+      }
+      return {
+        label: s.label,
+        fields: (s.fields || []).map((f) => ({
+          label: f.label, value: formatValue(f.value, f), mono: !!f.mono,
+        })),
+      }
+    })
+    // A table section declares its columns up front, so it survives having no
+    // rows and shows its own empty message. Dropping it would take the tab with
+    // it and read as "this machine has no disks" rather than "nothing reported".
+    .filter((s) => s.fields.length > 0 || (s.columns && s.columns.length > 0))
 }
 
 // Inline row action for a list panel. A monitored services/processes row may
@@ -255,6 +276,16 @@ export function rowAction(panel, item) {
   }
 }
 
+// A table needs the full card width. `table` panels get it via panel.render,
+// but a `tabs` panel only needs it when one of its sections is a table, and
+// that is not knowable from the declaration - it depends on fetched data.
+export function spansFullWidth(panel) {
+  if (!panel) return false
+  if (panel.render === 'table') return true
+  if (panel.render !== 'tabs') return false
+  return tabSections(panel).some((s) => (s.columns || []).length > 0)
+}
+
 // A panel declaring `empty` is asking to stay visible with nothing in it and
 // say so - "No installed software reported." is an answer, where an absent
 // card is a question. `list` honoured that and `table` did not, for no reason

frontend/src/components/tabSectionsTable.spec.js

@@ -0,0 +1,99 @@
+import { describe, it, expect } from 'vitest'
+import { tabSections, panelVisible, spansFullWidth } from './pluginAssetPanels'
+
+// The tabs render type carried label/value sections only. It now also carries
+// table sections, so that two tables which belong together for the reader but
+// share no key can sit in one card. This is the shared renderer every plugin
+// uses, so the label/value path is pinned here alongside the new one.
+
+function tabsPanel(sections) {
+  return { id: 'p', render: 'tabs', _data: { sections } }
+}
+
+const FIELD_SECTION = { label: 'Fields', fields: [{ label: 'A', value: '1' }] }
+const TABLE_SECTION = {
+  label: 'Volumes',
+  columns: [{ key: 'drive', label: 'Drive' }],
+  rows: [{ drive: 'C:' }],
+  empty: 'No disks reported.',
+}
+
+describe('tabSections with table sections', () => {
+  it('keeps rendering label/value sections as before', () => {
+    const sections = tabSections(tabsPanel([FIELD_SECTION]))
+    expect(sections).toHaveLength(1)
+    expect(sections[0].fields).toEqual([{ label: 'A', value: '1', mono: false }])
+    expect(sections[0].columns).toBeUndefined()
+  })
+
+  it('carries columns and rows through for a table section', () => {
+    const sections = tabSections(tabsPanel([TABLE_SECTION]))
+    expect(sections[0].columns).toEqual([{ key: 'drive', label: 'Drive' }])
+    expect(sections[0].rows).toEqual([{ drive: 'C:' }])
+  })
+
+  it('lets one panel mix a table section and a field section', () => {
+    const sections = tabSections(tabsPanel([TABLE_SECTION, FIELD_SECTION]))
+    expect(sections.map((s) => s.label)).toEqual(['Volumes', 'Fields'])
+  })
+
+  // The reason the empty-section filter needed a table branch at all. A card
+  // whose second section is dropped loses that tab, and "no disks" is a very
+  // different claim from "the collector reported nothing".
+  it('keeps an empty table section so its tab and message survive', () => {
+    const sections = tabSections(tabsPanel([{ ...TABLE_SECTION, rows: [] }]))
+    expect(sections).toHaveLength(1)
+    expect(sections[0].empty).toBe('No disks reported.')
+  })
+
+  it('still drops a label/value section with no fields', () => {
+    const sections = tabSections(tabsPanel([{ label: 'Empty', fields: [] }]))
+    expect(sections).toHaveLength(0)
+  })
+
+  it('derives a column label from the key when none is given', () => {
+    const sections = tabSections(tabsPanel([
+      { label: 'V', columns: [{ key: 'drive' }], rows: [] },
+    ]))
+    expect(sections[0].columns[0].label).toBe('Drive')
+  })
+
+  it('treats a missing rows array as no rows rather than throwing', () => {
+    const sections = tabSections(tabsPanel([{ label: 'V', columns: [{ key: 'drive' }] }]))
+    expect(sections[0].rows).toEqual([])
+  })
+})
+
+describe('panelVisible for a tabs panel of tables', () => {
+  it('shows a panel whose only section is an empty table', () => {
+    expect(panelVisible(tabsPanel([{ ...TABLE_SECTION, rows: [] }]))).toBe(true)
+  })
+
+  it('still hides a tabs panel with no sections at all', () => {
+    expect(panelVisible(tabsPanel([]))).toBe(false)
+  })
+})
+
+describe('spansFullWidth', () => {
+  // A six-column table squeezed into one half of a two-column grid is the
+  // failure this prevents.
+  it('spans a tabs panel that contains a table section', () => {
+    expect(spansFullWidth(tabsPanel([TABLE_SECTION]))).toBe(true)
+  })
+
+  it('does not span a tabs panel of label/value sections', () => {
+    expect(spansFullWidth(tabsPanel([FIELD_SECTION]))).toBe(false)
+  })
+
+  it('still spans a plain table panel', () => {
+    expect(spansFullWidth({ render: 'table' })).toBe(true)
+  })
+
+  it('does not span a keyvalue panel', () => {
+    expect(spansFullWidth({ render: 'keyvalue' })).toBe(false)
+  })
+
+  it('tolerates a null panel', () => {
+    expect(spansFullWidth(null)).toBe(false)
+  })
+})

plugins/endpoints/api/routes.py

@@ -636,19 +636,15 @@ def panel_osinfo(asset_id: int):
     return success_response({'fields': fields})
 
 
-@endpoints_bp.route('/asset/<int:asset_id>/storage', methods=['GET'])
-@require_permission('endpoints.view')
-def panel_storage(asset_id: int):
-    """Table panel: fixed disks with capacity and free space.
+def _storage_rows(disks):
+    """Turn the agent's volume list into display rows.
 
     freepercent is computed by the agent from 1.7.0; for a snapshot written
     by an older agent it is derived here from the byte counts so the column
     is not empty for the one field an operator actually scans.
     """
-    state = EndpointState.query.filter_by(assetid=asset_id).first()
-    disks = _state_json(state, 'disksjson') if state else None
     if not isinstance(disks, list):
-        return success_response([])
+        return []
 
     rows = []
     for disk in disks:
@@ -668,7 +664,62 @@ def panel_storage(asset_id: int):
             'free': _format_bytes(disk.get('freebytes')),
             'freepercent': '{}%'.format(freepercent) if freepercent is not None else None,
         })
-    return success_response(rows)
+    return rows
+
+
+_STORAGE_COLUMNS = [
+    {'key': 'drive', 'label': 'Drive'},
+    {'key': 'label', 'label': 'Label'},
+    {'key': 'filesystem', 'label': 'Filesystem'},
+    {'key': 'size', 'label': 'Size'},
+    {'key': 'free', 'label': 'Free'},
+    {'key': 'freepercent', 'label': 'Free %'},
+]
+
+_DISKHEALTH_COLUMNS = [
+    {'key': 'model', 'label': 'Drive'},
+    {'key': 'mediatype', 'label': 'Media'},
+    {'key': 'bustype', 'label': 'Bus'},
+    {'key': 'healthstatus', 'label': 'Health'},
+    {'key': 'verdict', 'label': 'Verdict'},
+    {'key': 'source', 'label': 'Source'},
+]
+
+
+@endpoints_bp.route('/asset/<int:asset_id>/storage', methods=['GET'])
+@require_permission('endpoints.view')
+def panel_storage(asset_id: int):
+    """Tabs panel: volume capacity and physical drive health as two sections.
+
+    One card because an operator looking at a full disk and an operator looking
+    at a dying disk are usually the same person in the same minute. TWO
+    SECTIONS because the two do not share a key: storage is per volume (C:, D:)
+    and health is per physical device, and one drive can back several volumes
+    or none the operator sees. Nothing is reconciled across the sections -
+    this is not the mapped-drive merge, where two sources described the same
+    drive and the worse status won.
+
+    Both sections are emitted even when empty so the section keeps its own
+    empty message; a dropped section would take its tab with it and read as
+    "no disks" rather than "nothing reported".
+    """
+    state = EndpointState.query.filter_by(assetid=asset_id).first()
+    disks = _state_json(state, 'disksjson') if state else None
+    drives = _state_json(state, 'diskhealthjson') if state else None
+    return success_response({'sections': [
+        {
+            'label': 'Volumes',
+            'columns': _STORAGE_COLUMNS,
+            'rows': _storage_rows(disks),
+            'empty': 'No disks reported.',
+        },
+        {
+            'label': 'Disk health',
+            'columns': _DISKHEALTH_COLUMNS,
+            'rows': _diskhealth_rows(drives),
+            'empty': 'No disk health reported.',
+        },
+    ]})
 
 
 def _diskhealth_rows(drives):
@@ -717,11 +768,11 @@ def _diskhealth_rows(drives):
 @endpoints_bp.route('/asset/<int:asset_id>/diskhealth', methods=['GET'])
 @require_permission('endpoints.view')
 def panel_diskhealth(asset_id: int):
-    """Table panel: physical drive health.
+    """Physical drive health rows.
 
-    Separate from the storage panel because the two do not share a key. Storage
-    is per volume (C:, D:), this is per physical device, and one physical drive
-    can back several volumes or none the operator can see.
+    No longer backs a panel of its own - the Storage card renders it as a
+    section. Kept as a route because it is a stable read for callers outside
+    the panel renderer and removing it would break them for a UI-only change.
 
     Rows are ordered so anything predicting failure sorts first: a failing
     drive must not be below the fold on a machine with several disks.

plugins/endpoints/plugin.py

@@ -558,48 +558,22 @@ class EndpointsPlugin(BasePlugin):
                 'group': 'monitors',
             },
             {
+                # Volumes and physical drive health in one card, as two
+                # sections. VISUAL GROUPING ONLY: the two tables share no key
+                # (storage is per volume, health is per physical device) and
+                # nothing is reconciled across them. They are one card because
+                # the operator reading a full disk and the one reading a dying
+                # disk are usually the same person in the same minute.
                 'id': 'endpoints-storage',
                 'title': 'Storage',
                 'assettypes': ['computer'],
                 'endpoint': '/api/endpoints/asset/{assetid}/storage',
-                'render': 'table',
+                'render': 'tabs',
                 'collapsible': True,
-                'columns': [
-                    {'key': 'drive', 'label': 'Drive'},
-                    {'key': 'label', 'label': 'Label'},
-                    {'key': 'filesystem', 'label': 'Filesystem'},
-                    {'key': 'size', 'label': 'Size'},
-                    {'key': 'free', 'label': 'Free'},
-                    {'key': 'freepercent', 'label': 'Free %'},
-                ],
                 'empty': 'No disks reported.',
                 'position': 17,
                 'group': 'monitors',
             },
-            {
-                # Sits directly under Storage because an operator looking at a
-                # full disk and an operator looking at a dying disk are usually
-                # the same person in the same minute. Kept a separate panel
-                # because storage is per volume and this is per physical
-                # device; the two do not share a key.
-                'id': 'endpoints-diskhealth',
-                'title': 'Disk health',
-                'assettypes': ['computer'],
-                'endpoint': '/api/endpoints/asset/{assetid}/diskhealth',
-                'render': 'table',
-                'collapsible': True,
-                'columns': [
-                    {'key': 'model', 'label': 'Drive'},
-                    {'key': 'mediatype', 'label': 'Media'},
-                    {'key': 'bustype', 'label': 'Bus'},
-                    {'key': 'healthstatus', 'label': 'Health'},
-                    {'key': 'verdict', 'label': 'Verdict'},
-                    {'key': 'source', 'label': 'Source'},
-                ],
-                'empty': 'No disk health reported.',
-                'position': 18,
-                'group': 'monitors',
-            },
             {
                 # The listening set is the box's exposed network surface. The
                 # full connection table is deliberately not here: it is an

tests/test_plugins/test_endpoints_panel_groups.py

@@ -61,5 +61,10 @@ def test_system_monitors_keeps_panels_of_its_own():
 
 
 def test_disk_health_stayed_with_the_system_health_panels():
-    """Disk health is the box's own health, not something running on it."""
-    assert _panels()['endpoints-diskhealth']['group'] == 'monitors'
+    """Disk health is the box's own health, not something running on it.
+
+    It is now a section of the Storage card rather than a panel of its own,
+    so the group assertion follows it there.
+    """
+    assert 'endpoints-diskhealth' not in _panels()
+    assert _panels()['endpoints-storage']['group'] == 'monitors'

tests/test_plugins/test_endpoints_plugin.py

@@ -69,12 +69,12 @@ def test_endpoints_asset_panels_are_computer_scoped():
     """Every asset panel targets computer assets only, with an endpoint."""
     plugin = EndpointsPlugin()
     panels = plugin.get_asset_panels()
-    assert len(panels) == 13
+    assert len(panels) == 12
     for panel in panels:
         assert panel['assettypes'] == ['computer']
         assert panel['endpoint'].startswith('/api/endpoints/')
         assert '{assetid}' in panel['endpoint']
-        assert panel['render'] in ('keyvalue', 'list', 'table', 'operatorreports')
+        assert panel['render'] in ('keyvalue', 'list', 'table', 'tabs', 'operatorreports')
 
 
 def test_services_and_processes_are_split_into_two_filtered_cards():

tests/test_plugins/test_endpoints_storage_sections.py

@@ -0,0 +1,51 @@
+"""The Storage panel serves volumes and disk health as two sections.
+
+Visual grouping only. The two tables share no key - storage is per volume and
+health is per physical device - so nothing is reconciled across them. These
+tests pin that the sections stay independent and that neither disappears when
+empty, because a dropped section takes its tab with it and reads as "no disks"
+rather than "nothing was reported".
+"""
+import pytest
+
+from plugins.endpoints.api.routes import _storage_rows, _STORAGE_COLUMNS, _DISKHEALTH_COLUMNS
+
+
+def test_freepercent_is_derived_when_an_older_agent_omits_it():
+    rows = _storage_rows([{'drive': 'C:', 'sizebytes': 1000, 'freebytes': 250}])
+    assert rows[0]['freepercent'] == '25.0%'
+
+
+def test_an_agent_supplied_freepercent_is_preferred_over_recomputing():
+    rows = _storage_rows([
+        {'drive': 'C:', 'sizebytes': 1000, 'freebytes': 250, 'freepercent': 24.9},
+    ])
+    assert rows[0]['freepercent'] == '24.9%'
+
+
+def test_a_zero_size_volume_does_not_divide_by_zero():
+    rows = _storage_rows([{'drive': 'C:', 'sizebytes': 0, 'freebytes': 0}])
+    assert rows[0]['freepercent'] is None
+
+
+def test_free_percent_is_none_rather_than_zero_when_free_is_unknown():
+    """Zero free space and unknown free space must not look identical."""
+    rows = _storage_rows([{'drive': 'C:', 'sizebytes': 1000}])
+    assert rows[0]['freepercent'] is None
+
+
+def test_non_dict_entries_are_skipped():
+    rows = _storage_rows([{'drive': 'C:', 'sizebytes': 1000, 'freebytes': 500}, 'junk', None])
+    assert len(rows) == 1
+
+
+@pytest.mark.parametrize('value', [None, {}, 'x'])
+def test_a_missing_snapshot_yields_no_rows(value):
+    assert _storage_rows(value) == []
+
+
+def test_the_two_section_column_sets_share_no_key():
+    """The point of two sections rather than one merged table."""
+    storage = {column['key'] for column in _STORAGE_COLUMNS}
+    health = {column['key'] for column in _DISKHEALTH_COLUMNS}
+    assert not storage & health