From 0f70fa0cc0abc5e2b57994a2db4b0d753c60da28 Mon Sep 17 00:00:00 2001 From: Pluze Zhu Date: Fri, 5 Jun 2026 19:50:57 -0500 Subject: [PATCH] refactor: converge electrochem runner helpers --- +labkit/+ui/+tool/private/addOrInsertAnchor.m | 4 + +labkit/+ui/+tool/private/anchorCurvePoints.m | 4 + .../+ui/+tool/private/createLabeledDropdown.m | 4 + .../+tool/private/createLabeledEditField.m | 4 + .../+ui/+tool/private/createLabeledSpinner.m | 4 + .../+ui/+tool/private/createReadOnlyInfoRow.m | 4 + .../+tool/private/createReadOnlyTextField.m | 4 + .../+ui/+tool/private/defaultScaleBarUnits.m | 3 + .../+ui/+tool/private/drawScaleBarOverlay.m | 4 + .../+ui/+tool/private/normalizeScaleBarUnit.m | 4 + +labkit/+ui/+tool/private/scaleBarPanel.m | 5 + apps/AGENTS.md | 6 + apps/electrochem/csc/+csc/+ui/runApp.m | 313 ------------------ apps/electrochem/eis/+eis/+ui/runApp.m | 209 +----------- docs/apps.md | 5 + docs/architecture.md | 6 + docs/testing.md | 6 + tests/AGENTS.md | 4 + .../project/ProjectDebtGuardrailTest.m | 101 ++++++ .../ProjectDocumentationGuardrailTest.m | 11 - 20 files changed, 176 insertions(+), 529 deletions(-) diff --git a/+labkit/+ui/+tool/private/addOrInsertAnchor.m b/+labkit/+ui/+tool/private/addOrInsertAnchor.m index 24227ad..02ceccb 100644 --- a/+labkit/+ui/+tool/private/addOrInsertAnchor.m +++ b/+labkit/+ui/+tool/private/addOrInsertAnchor.m @@ -1,3 +1,7 @@ +% Private anchor-editor insertion helper. Expected caller: +% labkit.ui.tool.anchorEditor. Inputs are existing and candidate image-pixel +% anchor coordinates plus axes/image/style constraints; output is the updated +% N-by-2 anchor array. No graphics or app state are mutated. function points = addOrInsertAnchor(points, newPoint, ax, imageSize, curveStyle, closed, maxPoints) %ADDORINSERTANCHOR Apply anchor insertion policy for createAnchorCurveEditor. % diff --git a/+labkit/+ui/+tool/private/anchorCurvePoints.m b/+labkit/+ui/+tool/private/anchorCurvePoints.m index 3d459fb..d425cb2 100644 --- a/+labkit/+ui/+tool/private/anchorCurvePoints.m +++ b/+labkit/+ui/+tool/private/anchorCurvePoints.m @@ -1,3 +1,7 @@ +% Private anchor-editor path builder. Expected caller: +% labkit.ui.tool.anchorEditor and insertion helpers. Inputs are N-by-2 +% image-pixel anchors, image size, curve style, and closed/open mode; outputs +% are display curve samples and segment-owner indices. No side effects. function [curve, owners] = anchorCurvePoints(points, imageSize, curveStyle, closed) %ANCHORCURVEPOINTS Build displayed anchor path samples for curve editors. % diff --git a/+labkit/+ui/+tool/private/createLabeledDropdown.m b/+labkit/+ui/+tool/private/createLabeledDropdown.m index 6971754..e62bf7c 100644 --- a/+labkit/+ui/+tool/private/createLabeledDropdown.m +++ b/+labkit/+ui/+tool/private/createLabeledDropdown.m @@ -1,3 +1,7 @@ +% Private scale-bar/tool control helper. Expected caller: labkit.ui.tool +% private panels. Inputs are a parent grid, visible label text, and uidropdown +% name/value arguments; outputs are label and dropdown handles. Side effects +% are limited to creating UI controls under the parent. function [lbl, dd] = createLabeledDropdown(parent, labelText, varargin) %CREATELABELEDDROPDOWN Create a right-aligned label followed by a dropdown. % diff --git a/+labkit/+ui/+tool/private/createLabeledEditField.m b/+labkit/+ui/+tool/private/createLabeledEditField.m index f7aa38d..b040fd5 100644 --- a/+labkit/+ui/+tool/private/createLabeledEditField.m +++ b/+labkit/+ui/+tool/private/createLabeledEditField.m @@ -1,3 +1,7 @@ +% Private scale-bar/tool control helper. Expected caller: labkit.ui.tool +% private panels. Inputs are a parent grid, visible label text, edit style, +% and uieditfield name/value arguments; outputs are label and field handles. +% Side effects are limited to creating UI controls under the parent. function [lbl, field] = createLabeledEditField(parent, labelText, style, varargin) %CREATELABELEDEDITFIELD Create a right-aligned label followed by an edit field. % diff --git a/+labkit/+ui/+tool/private/createLabeledSpinner.m b/+labkit/+ui/+tool/private/createLabeledSpinner.m index 63c3dd2..3289932 100644 --- a/+labkit/+ui/+tool/private/createLabeledSpinner.m +++ b/+labkit/+ui/+tool/private/createLabeledSpinner.m @@ -1,3 +1,7 @@ +% Private scale-bar/tool control helper. Expected caller: labkit.ui.tool +% private panels. Inputs are a parent grid, visible label text, and uispinner +% name/value arguments; outputs are label and spinner handles. Side effects +% are limited to creating UI controls under the parent. function [lbl, spinner] = createLabeledSpinner(parent, labelText, varargin) %CREATELABELEDSPINNER Create a right-aligned label followed by a spinner. % diff --git a/+labkit/+ui/+tool/private/createReadOnlyInfoRow.m b/+labkit/+ui/+tool/private/createReadOnlyInfoRow.m index 2f424eb..2581f97 100644 --- a/+labkit/+ui/+tool/private/createReadOnlyInfoRow.m +++ b/+labkit/+ui/+tool/private/createReadOnlyInfoRow.m @@ -1,3 +1,7 @@ +% Private scale-bar/tool readout helper. Expected caller: labkit.ui.tool +% private panels. Inputs are a parent grid, target row, and visible label; +% outputs are a read-only text field and label handles. Side effects are +% limited to creating and placing UI controls under the parent. function [field, lbl] = createReadOnlyInfoRow(parent, row, labelText) %CREATEREADONLYINFOROW Create a labeled read-only text field row. % diff --git a/+labkit/+ui/+tool/private/createReadOnlyTextField.m b/+labkit/+ui/+tool/private/createReadOnlyTextField.m index dc0a51d..d099b37 100644 --- a/+labkit/+ui/+tool/private/createReadOnlyTextField.m +++ b/+labkit/+ui/+tool/private/createReadOnlyTextField.m @@ -1,3 +1,7 @@ +% Private scale-bar/tool readout helper. Expected caller: labkit.ui.tool +% private panels. Inputs are a parent grid and uieditfield name/value +% arguments; output is a read-only text field handle. Side effects are limited +% to creating one UI control under the parent. function field = createReadOnlyTextField(parent, varargin) %CREATEREADONLYTEXTFIELD Create a read-only single-line text field. % diff --git a/+labkit/+ui/+tool/private/defaultScaleBarUnits.m b/+labkit/+ui/+tool/private/defaultScaleBarUnits.m index ef96597..cd63552 100644 --- a/+labkit/+ui/+tool/private/defaultScaleBarUnits.m +++ b/+labkit/+ui/+tool/private/defaultScaleBarUnits.m @@ -1,3 +1,6 @@ +% Private scale-bar unit defaults. Expected caller: labkit.ui.tool scale-bar +% helpers. No inputs; output is the app-neutral unit label order. No side +% effects or app-specific assumptions. function units = defaultScaleBarUnits() %DEFAULTSCALEBARUNITS Return the app-neutral scale-bar unit order. % diff --git a/+labkit/+ui/+tool/private/drawScaleBarOverlay.m b/+labkit/+ui/+tool/private/drawScaleBarOverlay.m index 0396aeb..db2ac6e 100644 --- a/+labkit/+ui/+tool/private/drawScaleBarOverlay.m +++ b/+labkit/+ui/+tool/private/drawScaleBarOverlay.m @@ -1,3 +1,7 @@ +% Private scale-bar overlay renderer. Expected caller: +% labkit.ui.tool.scaleBar.renderOverlay. Inputs are target axes and a prepared +% scale-bar spec; output is line/text handles or empty. Side effects are +% limited to adding overlay graphics to the target axes. function handles = drawScaleBarOverlay(ax, scaleBar) %DRAWSCALEBAROVERLAY Draw a prepared scale-bar spec onto image axes. % diff --git a/+labkit/+ui/+tool/private/normalizeScaleBarUnit.m b/+labkit/+ui/+tool/private/normalizeScaleBarUnit.m index 1018c88..888d070 100644 --- a/+labkit/+ui/+tool/private/normalizeScaleBarUnit.m +++ b/+labkit/+ui/+tool/private/normalizeScaleBarUnit.m @@ -1,3 +1,7 @@ +% Private scale-bar unit normalization helper. Expected caller: +% labkit.ui.tool scale-bar panel/calibration code. Inputs are a candidate unit, +% allowed unit labels, and fallback unit; output is a valid string scalar. No +% side effects. function unitName = normalizeScaleBarUnit(unitName, units, defaultUnit) %NORMALIZESCALEBARUNIT Normalize a scale-bar unit against allowed UI units. % diff --git a/+labkit/+ui/+tool/private/scaleBarPanel.m b/+labkit/+ui/+tool/private/scaleBarPanel.m index 7b5b558..d863a0a 100644 --- a/+labkit/+ui/+tool/private/scaleBarPanel.m +++ b/+labkit/+ui/+tool/private/scaleBarPanel.m @@ -1,3 +1,8 @@ +% Private reusable scale-bar control section. Expected caller: +% labkit.ui.tool.scaleBar. Inputs are parent grid, target row, and options +% struct for units, positions, colors, defaults, and callbacks; output is a UI +% handle/method struct. Side effects are limited to creating controls in the +% parent and invoking supplied callbacks from user control changes. function ui = scaleBarPanel(parent, row, opts) %CREATESCALEBARPANEL Create reusable scale-bar calibration controls. % diff --git a/apps/AGENTS.md b/apps/AGENTS.md index 2437d5d..8751cfd 100644 --- a/apps/AGENTS.md +++ b/apps/AGENTS.md @@ -34,6 +34,12 @@ Apps are first-class deliverables. Do not treat them as examples for a hidden pl builders, app-local IO normalization, and default state/result structs into focused app-owned packages first, and add tests that call those functions directly. +- When the wearable ECG Print app is migrated, target + `apps/wearable/ecg_print/+ecg_print/...` with the public command still named + `labkit_ECGPrint_app`; do not create a direct `apps/wearable/+ecg_print` + package. +- After an app-owned helper is extracted, remove same-named local copies from + `+ui/runApp.m` so GUI paths call the tested package helper. - Do not add new `*Workflow.m` files or app-owned `+core/dispatch.m` string routers. - When a public app file grows large, prefer moving GUI-free app-owned calculations, export builders, formatting utilities, deterministic image/signal transforms, and focused control construction into `apps///+/...`. diff --git a/apps/electrochem/csc/+csc/+ui/runApp.m b/apps/electrochem/csc/+csc/+ui/runApp.m index 55a5555..48f06ad 100644 --- a/apps/electrochem/csc/+csc/+ui/runApp.m +++ b/apps/electrochem/csc/+csc/+ui/runApp.m @@ -549,316 +549,3 @@ function setDropdownValueIfExists(dd, valueText) dd.Value = dd.Items{1}; end end - -%% App-local analysis -function A = computeCSC(curve, opts) -%COMPUTECSC Compute CV/CT charge comparison and CSC for the CSC app. - - if nargin < 2 - opts = struct(); - end - opts = fillOptions(opts); - - A = struct(); - A.ok = false; - A.message = ''; - A.logMessage = ''; - A.mode = opts.mode; - A.scanRate = opts.scanRate; - A.area_cm2 = parsePositiveScalar(opts.area_cm2); - - if ~(isscalar(A.scanRate) && isfinite(A.scanRate) && A.scanRate > 0) - A.message = 'scan rate missing'; - A.logMessage = 'Compare skipped: scan rate missing.'; - return; - end - - if ~hasExactColumns(curve, {'T', 'Vf', 'Im'}) - A.message = 'Need T, Vf, Im'; - A.logMessage = 'Compare skipped: T/Vf/Im not all present.'; - return; - end - - t = exactColumn(curve, 'T'); - V = exactColumn(curve, 'Vf'); - I = exactColumn(curve, 'Im'); - - good = ~(isnan(t) | isnan(V) | isnan(I)); - t = t(good); - V = V(good); - I = I(good); - - if numel(t) < 2 - A.message = 'Not enough points'; - A.logMessage = 'Compare skipped: not enough valid points.'; - return; - end - - CT = computeCTCharge(t, V, I); - CV = computeCVCharge(t, V, I, A.scanRate); - if ~CT.ok - A.message = CT.message; - A.logMessage = 'Compare skipped: not enough valid points.'; - return; - end - if ~CV.ok - A.message = CV.message; - A.logMessage = 'Compare skipped: scan rate missing.'; - return; - end - - A.t = t; - A.Vf = V; - A.Im = I; - A.IcathDisp = CT.IcathDisp; - A.IanodDisp = CT.IanodDisp; - A.QctCath = CT.QctCath; - A.QctAnod = CT.QctAnod; - A.QctFull = CT.QctFull; - A.QcvCath = CV.QcvCath; - A.QcvAnod = CV.QcvAnod; - A.QcvFull = CV.QcvFull; - A.dtErr = CV.dtErr; - - switch A.mode - case 'Cathodic' - A.Qct = A.QctCath; - A.Qcv = A.QcvCath; - case 'Anodic' - A.Qct = A.QctAnod; - A.Qcv = A.QcvAnod; - otherwise - A.mode = 'Full'; - A.Qct = A.QctFull; - A.Qcv = A.QcvFull; - end - - A.diff_C = A.Qct - A.Qcv; - denom = max(abs(A.Qct), abs(A.Qcv)); - if denom == 0 - A.rel_pct = 0; - else - A.rel_pct = 100 * abs(A.diff_C) / denom; - end - - if isfinite(A.area_cm2) && A.area_cm2 > 0 - A.Qct_mC_cm2 = 1e3 * A.Qct / A.area_cm2; - A.Qcv_mC_cm2 = 1e3 * A.Qcv / A.area_cm2; - A.diff_mC_cm2 = 1e3 * A.diff_C / A.area_cm2; - else - A.Qct_mC_cm2 = NaN; - A.Qcv_mC_cm2 = NaN; - A.diff_mC_cm2 = NaN; - end - - A.ok = true; - A.message = 'OK'; -end - -%% Small app-local utilities -function opts = fillOptions(opts) - if ~isfield(opts, 'mode') - opts.mode = 'Full'; - end - if ~isfield(opts, 'scanRate') - opts.scanRate = NaN; - end - if ~isfield(opts, 'area_cm2') - opts.area_cm2 = NaN; - end -end - -function tf = hasExactColumns(curve, names) - tf = isfield(curve, 'headers'); - if ~tf - return; - end - for k = 1:numel(names) - if ~any(strcmp(curve.headers, names{k})) - tf = false; - return; - end - end -end - -function col = exactColumn(curve, name) - idx = find(strcmp(curve.headers, name), 1); - if isempty(idx) - col = []; - else - col = curve.data(:, idx); - end -end - -function R = computeCTCharge(t, V, I) - R = struct(); - R.ok = false; - R.message = ''; - - if nargin < 3 || numel(t) < 2 || numel(V) < 2 || numel(I) < 2 - R.message = 'Not enough points'; - R = fillEmptyCT(R); - return; - end - - S = integrateCVCTSignSplit(t, V, I, NaN); - R = copyFields(R, S, {'QctCath', 'QctAnod', 'IcathDisp', 'IanodDisp'}); - R.QctFull = R.QctCath + R.QctAnod; - R.ok = true; - R.message = 'OK'; -end - -function R = computeCVCharge(t, V, I, scanRate) - R = struct(); - R.ok = false; - R.message = ''; - - if nargin < 4 || ~(isscalar(scanRate) && isfinite(scanRate) && scanRate > 0) - R.message = 'scan rate missing'; - R = fillEmptyCV(R); - return; - end - if numel(t) < 2 || numel(V) < 2 || numel(I) < 2 - R.message = 'Not enough points'; - R = fillEmptyCV(R); - return; - end - - S = integrateCVCTSignSplit(t, V, I, scanRate); - R = copyFields(R, S, {'QcvCath', 'QcvAnod', 'dtErr', 'IcathDisp', 'IanodDisp'}); - R.QcvFull = R.QcvCath + R.QcvAnod; - R.ok = true; - R.message = 'OK'; -end - -function R = integrateCVCTSignSplit(t, V, I, scanRate) - if nargin < 4 - scanRate = NaN; - end - - t = t(:); - V = V(:); - I = I(:); - - R = struct(); - R.QctCath = 0; - R.QctAnod = 0; - R.QcvCath = 0; - R.QcvAnod = 0; - R.dtErr = NaN; - - R.IcathDisp = I; - R.IanodDisp = I; - R.IcathDisp(I >= 0) = NaN; - R.IanodDisp(I <= 0) = NaN; - - dtErrList = []; - useCV = isscalar(scanRate) && isfinite(scanRate) && scanRate > 0; - - for k = 1:numel(t)-1 - t1 = t(k); t2 = t(k+1); - V1 = V(k); V2 = V(k+1); - I1 = I(k); I2 = I(k+1); - - if any(~isfinite([t1 t2 V1 V2 I1 I2])) - continue; - end - - bp = [0, 1]; - s0 = crossingFraction(I1, I2, 0); - if ~isempty(s0) - bp(end+1) = s0; %#ok - end - bp = unique(sort(bp)); - - for j = 1:numel(bp)-1 - sa = bp(j); - sb = bp(j+1); - - ta = lerp(t1, t2, sa); - tb = lerp(t1, t2, sb); - Va = lerp(V1, V2, sa); - Vb = lerp(V1, V2, sb); - Ia = lerp(I1, I2, sa); - Ib = lerp(I1, I2, sb); - - Imid = 0.5 * (Ia + Ib); - if Imid < 0 - R.QctCath = R.QctCath + abs(trapz([ta tb], [Ia Ib])); - elseif Imid > 0 - R.QctAnod = R.QctAnod + trapz([ta tb], [Ia Ib]); - end - - if useCV - dt_act = tb - ta; - dt_cv = abs(Vb - Va) / scanRate; - dtErrList(end+1) = abs(dt_act - dt_cv); %#ok - - if Imid < 0 - R.QcvCath = R.QcvCath + abs(trapz([0 dt_cv], [Ia Ib])); - elseif Imid > 0 - R.QcvAnod = R.QcvAnod + trapz([0 dt_cv], [Ia Ib]); - end - end - end - end - - if ~isempty(dtErrList) - R.dtErr = max(dtErrList); - end -end - -function R = fillEmptyCT(R) - R.QctCath = 0; - R.QctAnod = 0; - R.QctFull = 0; - R.IcathDisp = []; - R.IanodDisp = []; -end - -function R = fillEmptyCV(R) - R.QcvCath = 0; - R.QcvAnod = 0; - R.QcvFull = 0; - R.dtErr = NaN; - R.IcathDisp = []; - R.IanodDisp = []; -end - -function out = copyFields(out, in, names) - for k = 1:numel(names) - out.(names{k}) = in.(names{k}); - end -end - -function y = lerp(a, b, s) - y = a + s * (b - a); -end - -function s = crossingFraction(y1, y2, y0) - if ~isfinite(y1) || ~isfinite(y2) || y1 == y2 - s = []; - return; - end - s = (y0 - y1) / (y2 - y1); - if ~(s > 0 && s < 1) - s = []; - end -end - -function q = parsePositiveScalar(x) - if isnumeric(x) - q = x; - else - x = strtrim(char(x)); - if isempty(x) - q = NaN; - return; - end - q = str2double(x); - end - - if ~isscalar(q) || ~isfinite(q) || q <= 0 - q = NaN; - end -end diff --git a/apps/electrochem/eis/+eis/+ui/runApp.m b/apps/electrochem/eis/+eis/+ui/runApp.m index 9f3772f..5f67297 100644 --- a/apps/electrochem/eis/+eis/+ui/runApp.m +++ b/apps/electrochem/eis/+eis/+ui/runApp.m @@ -247,8 +247,8 @@ function refreshPlot() if isempty(S.items) title(ax, 'EIS Overlay'); - xlabel(ax, labelForAxis(ddX.Value)); - ylabel(ax, labelForAxis(ddY.Value)); + xlabel(ax, eis.view.labelForAxis(ddX.Value)); + ylabel(ax, eis.view.labelForAxis(ddY.Value)); txtSummary.Value = {'No files loaded.'}; return; end @@ -269,9 +269,9 @@ function refreshPlot() plotOpts.showMarkers = cbMarkers.Value; plotOpts.showLegend = cbLegend.Value; plotOpts.showGrid = cbGrid.Value; - plotOverlay(ax, items, plotOpts); + eis.view.plotOverlay(ax, items, plotOpts); - txtSummary.Value = buildSummary(items); + txtSummary.Value = eis.view.buildSummary(items); end function onExportCSV(~, ~) @@ -298,208 +298,7 @@ function addLog(msg) end end -%% App-local plotting and summary helpers -function txt = labelForAxis(axisName) - txt = axisName; -end - -function summary = buildSummary(items) - summary = cell(0, 1); - summary{end+1} = sprintf('Loaded files: %d', numel(items)); - for i = 1:numel(items) - fmin = min(items(i).Freq, [], 'omitnan'); - fmax = max(items(i).Freq, [], 'omitnan'); - summary{end+1} = sprintf('%s | N=%d | Freq %.4g to %.4g Hz | order: %s', ... - items(i).name, items(i).n, fmin, fmax, ternary(items(i).freqDesc, 'high->low', 'low->high/mixed')); - end -end - -function labels = plotOverlay(ax, items, opts) - if nargin < 3 - opts = struct(); - end - opts = fillPlotOptions(opts); - - cla(ax); - ax.XScale = ternary(opts.logX, 'log', 'linear'); - ax.YScale = ternary(opts.logY, 'log', 'linear'); - axis(ax, 'normal'); - - cmap = lines(numel(items)); - labels = cell(1, numel(items)); - marker = 'none'; - if opts.showMarkers - marker = 'o'; - end - - hold(ax, 'on'); - for k = 1:numel(items) - [x, y] = filteredXY(items(k), opts.xName, opts.yName, opts.logX, opts.logY); - plot(ax, x, y, ... - 'LineWidth', opts.lineWidth, ... - 'Marker', marker, ... - 'MarkerSize', opts.markerSize, ... - 'Color', cmap(k, :)); - labels{k} = items(k).name; - end - hold(ax, 'off'); - - xlabel(ax, labelForAxis(opts.xName)); - ylabel(ax, labelForAxis(opts.yName)); - title(ax, sprintf('%s vs %s (%d file%s)', ... - labelForAxis(opts.yName), labelForAxis(opts.xName), numel(items), pluralS(numel(items)))); - - if opts.showGrid - grid(ax, 'on'); - else - grid(ax, 'off'); - end - - if opts.showLegend - legend(ax, labels, 'Interpreter', 'none', 'Location', 'best'); - else - legend(ax, 'off'); - end - - if isNyquistSelection(opts.xName, opts.yName) - axis(ax, 'equal'); - end -end - -function opts = fillPlotOptions(opts) - if ~isfield(opts, 'xName') - opts.xName = 'Zreal (ohm)'; - end - if ~isfield(opts, 'yName') - opts.yName = '-Zimag (ohm)'; - end - if ~isfield(opts, 'logX') - opts.logX = false; - end - if ~isfield(opts, 'logY') - opts.logY = false; - end - if ~isfield(opts, 'lineWidth') - opts.lineWidth = 1.4; - end - if ~isfield(opts, 'markerSize') - opts.markerSize = 6; - end - if ~isfield(opts, 'showMarkers') - opts.showMarkers = true; - end - if ~isfield(opts, 'showLegend') - opts.showLegend = true; - end - if ~isfield(opts, 'showGrid') - opts.showGrid = true; - end -end - -%% App-local export -function T = buildExportTable(items, xName, yName, useLogX, useLogY) - if nargin < 4 - useLogX = false; - end - if nargin < 5 - useLogY = false; - end - - maxLen = 0; - xCell = cell(1, numel(items)); - yCell = cell(1, numel(items)); - - for i = 1:numel(items) - [x, y] = filteredXY(items(i), xName, yName, useLogX, useLogY); - xCell{i} = x(:); - yCell{i} = y(:); - maxLen = max(maxLen, numel(x)); - end - - T = table((1:maxLen).', 'VariableNames', {'RowIndex'}); - for i = 1:numel(items) - safeName = matlab.lang.makeValidName(items(i).name); - xVar = matlab.lang.makeValidName(sprintf('X_%s_%s', sanitizeAxisName(xName), safeName)); - yVar = matlab.lang.makeValidName(sprintf('Y_%s_%s', sanitizeAxisName(yName), safeName)); - T.(xVar) = padWithNaN(xCell{i}, maxLen); - T.(yVar) = padWithNaN(yCell{i}, maxLen); - end -end - %% Small app-local utilities -function [x, y] = filteredXY(item, xName, yName, useLogX, useLogY) - x = valuesForAxis(item, xName); - y = valuesForAxis(item, yName); - valid = isfinite(x) & isfinite(y); - x = x(valid); - y = y(valid); - if useLogX - validX = x > 0; - x = x(validX); - y = y(validX); - end - if useLogY - validY = y > 0; - x = x(validY); - y = y(validY); - end -end - -function values = valuesForAxis(item, axisName) - switch axisName - case 'Freq (Hz)' - values = item.Freq; - case 'log10(Freq)' - values = log10(item.Freq); - case 'Time (s)' - values = item.Time; - case 'Point #' - values = item.Pt; - case 'Zreal (ohm)' - values = item.Zreal; - case 'Zimag (ohm)' - values = item.Zimag; - case '-Zimag (ohm)' - values = item.negZimag; - case 'Zmod (ohm)' - values = item.Zmod; - case 'Zphz (deg)' - values = item.Zphz; - case 'Idc (A)' - values = item.Idc; - case 'Vdc (V)' - values = item.Vdc; - otherwise - error('Unsupported axis selection: %s', axisName); - end -end - -function padded = padWithNaN(v, n) - padded = NaN(n, 1); - if isempty(v) - return; - end - padded(1:numel(v)) = v(:); -end - -function out = sanitizeAxisName(txt) - out = regexprep(lower(txt), '[^a-z0-9]+', '_'); - out = regexprep(out, '^_+|_+$', ''); -end - -function tf = isNyquistSelection(xName, yName) - tf = strcmp(xName, 'Zreal (ohm)') && ... - (strcmp(yName, '-Zimag (ohm)') || strcmp(yName, 'Zimag (ohm)')); -end - -function txt = pluralS(n) - if n == 1 - txt = ''; - else - txt = 's'; - end -end - function txt = ternary(cond, a, b) if cond txt = a; diff --git a/docs/apps.md b/docs/apps.md index 0b7c9f7..e32e45f 100644 --- a/docs/apps.md +++ b/docs/apps.md @@ -131,6 +131,11 @@ GUI-free responsibilities first, and keep the runner focused on orchestration: state setup, control construction, callback wiring, user alerts, log wording, refresh ordering, and app launch/debug behavior. +For the ECG Print app, use `apps/wearable/ecg_print/+ecg_print/...` as the +future app-owned package target while keeping the public command +`labkit_ECGPrint_app`. Do not add a direct `apps/wearable/+ecg_print` package +or a new `private` runner. + Before deleting a private runner debt item, extract directly testable behavior into app-owned package functions: diff --git a/docs/architecture.md b/docs/architecture.md index 3058742..f5a2c1a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -115,6 +115,12 @@ calculations, display formatting, export builders, app-local IO normalization, and default state/result structs into focused app-owned package functions with direct tests. The remaining runner should be orchestration code. +For the wearable ECG Print migration, the target shape is +`apps/wearable/ecg_print/labkit_ECGPrint_app.m` plus +`apps/wearable/ecg_print/+ecg_print/...`. The public launch command remains +`labkit_ECGPrint_app`; do not create a direct `apps/wearable/+ecg_print` +helper package. + Allowed electrochemistry string-dispatch debt: none. The former app-owned `+core/dispatch.m` routers have been replaced by component-local package functions. New apps and new migrations should not add `private` runners, diff --git a/docs/testing.md b/docs/testing.md index d82b8b3..b5a2995 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -194,6 +194,12 @@ Shared setup, structural GUI assertions, and focused support routines live under Architecture guardrails are split by concern under `tests/integration/project/`: public package surface, reusable package dependency boundaries, app entrypoint boundaries, and app-owned workflow boundaries. These guardrails may require workflow code to remain under the owning app tree, but they should not require GUI-free helpers to stay in the public app entry-point file. App-owned packages are checked by boundary rules rather than exact file-list assertions. +Project debt guardrails also prevent app UI runners from keeping local helper +copies that shadow same-named functions already extracted into the app-owned +`+ops`, `+view`, `+export`, `+io`, or `+state` packages. Tests should prove the +package helper behavior directly, and GUI paths should call those helpers +instead of private runner duplicates. + Compatibility bridge behavior should be isolated in named compatibility tests. For example, DTA legacy bridge fields such as `t`, `Vf`, `Im`, `Freq`, and `Zreal` are covered by `DtaCompatibilityBridgeTest`; ordinary DTA and app diff --git a/tests/AGENTS.md b/tests/AGENTS.md index 9504cc8..d4cc21c 100644 --- a/tests/AGENTS.md +++ b/tests/AGENTS.md @@ -26,6 +26,10 @@ Tests mirror source ownership. Do not create a parallel runner framework unless migration creates an app-owned package for DIC or wearable apps, add unit tests that call non-UI package functions such as `+ops`, `+view`, `+export`, `+io`, or `+state` directly. +- Guardrails should also prevent app `+ui/runApp.m` files from keeping + same-named local helper copies once the behavior exists in the app-owned + package. Ordinary tests should call the package helper; GUI structural tests + only prove wiring/layout. - UI public-surface tests should assert the layered `labkit.ui.app/view/tool/diag` facade and keep low-level controls, row resize, panel internals, and popout implementation private. - GUI smoke/debug tests may assert that every app supports debug launch and visible startup trace, but should not claim full interactive workflow validation. - When one test file grows too broad, add new focused `test_*.m` files instead of appending unrelated coverage. diff --git a/tests/integration/project/ProjectDebtGuardrailTest.m b/tests/integration/project/ProjectDebtGuardrailTest.m index 5e70931..fcd46e6 100644 --- a/tests/integration/project/ProjectDebtGuardrailTest.m +++ b/tests/integration/project/ProjectDebtGuardrailTest.m @@ -94,6 +94,26 @@ function appPrivateRunnerDebtDoesNotGrow(testCase) numel(actualFiles), numel(actualDirs)); end + function wearableMigrationTargetUsesAppSubfolder(testCase) + root = setupLabKitTestPath(); + directPackages = collectDirectPackageDirs( ... + fullfile(root, 'apps', 'wearable'), root); + + testCase.verifyTrue(isempty(directPackages), ... + ['Wearable app-owned helpers should live under ' ... + 'apps/wearable//+, not a direct family ' ... + 'package. Files: ' strjoin(cellstr(directPackages), ', ')]); + + ecgFolder = fullfile(root, 'apps', 'wearable', 'ecg_print'); + if isfolder(ecgFolder) + testCase.verifyTrue(isfile(fullfile(ecgFolder, ... + 'labkit_ECGPrint_app.m')), ... + 'ECG Print migration target should keep the public app entrypoint inside apps/wearable/ecg_print.'); + testCase.verifyTrue(isfolder(fullfile(ecgFolder, '+ecg_print')), ... + 'ECG Print migration target should use apps/wearable/ecg_print/+ecg_print.'); + end + end + function appWorkflowDispatchDebtDoesNotGrow(testCase) root = setupLabKitTestPath(); workflowFiles = collectRelativeFiles(root, ... @@ -112,6 +132,34 @@ function appWorkflowDispatchDebtDoesNotGrow(testCase) numel(workflowFiles), numel(dispatchFiles)); end + function appUiRunnersDoNotShadowExtractedPackageHelpers(testCase) + root = setupLabKitTestPath(); + runners = collectRelativeFiles(root, ... + fullfile(root, 'apps', '**', '+ui', 'runApp.m')); + findings = strings(1, 0); + + for k = 1:numel(runners) + runnerPath = fullfile(root, strrep(runners(k), '/', filesep)); + packageRoot = owningPackageRootForRunner(runnerPath); + if strlength(packageRoot) == 0 + continue; + end + + runnerFunctions = setdiff(functionNamesInFile(runnerPath), "runApp"); + packageFunctions = packageComponentFunctionNames(packageRoot); + overlap = intersect(runnerFunctions, packageFunctions); + if ~isempty(overlap) + findings(end+1) = runners(k) + " -> " + ... + strjoin(overlap, ", "); %#ok + end + end + + testCase.verifyTrue(isempty(findings), ... + ['App UI runners should call extracted app-owned package helpers, ' ... + 'not keep same-named local copies. Findings: ' ... + strjoin(cellstr(findings), ', ')]); + end + function dicWearableMigrationsHaveDirectPackageTests(testCase) root = setupLabKitTestPath(); packageRoots = collectDicWearableAppPackageRoots(root); @@ -217,6 +265,18 @@ function dicWearableMigrationsHaveDirectPackageTests(testCase) files = collectRelativeFiles(root, fullfile(root, 'apps', '**', 'private', '*.m')); end +function dirs = collectDirectPackageDirs(folder, root) + entries = dir(fullfile(folder, '+*')); + dirs = strings(1, 0); + for k = 1:numel(entries) + if entries(k).isdir + dirs(end+1) = string(relativePath(root, ... + fullfile(entries(k).folder, entries(k).name))); %#ok + end + end + dirs = unique(dirs); +end + function files = collectOversizedAppRunners(root, maxLines) files = strings(1, 0); entries = [ ... @@ -234,6 +294,47 @@ function dicWearableMigrationsHaveDirectPackageTests(testCase) files = unique(files); end +function packageRoot = owningPackageRootForRunner(runnerPath) + uiDir = fileparts(runnerPath); + packageRoot = string(fileparts(uiDir)); + [~, packageName] = fileparts(char(packageRoot)); + if ~startsWith(packageName, '+') + packageRoot = ""; + end +end + +function names = packageComponentFunctionNames(packageRoot) + components = ["+ops", "+view", "+export", "+io", "+state"]; + names = strings(1, 0); + for k = 1:numel(components) + componentRoot = fullfile(packageRoot, components(k)); + files = dir(fullfile(componentRoot, '*.m')); + for iFile = 1:numel(files) + [~, name] = fileparts(files(iFile).name); + names(end+1) = string(name); %#ok + end + end + names = unique(names); +end + +function names = functionNamesInFile(filepath) + content = fileread(filepath); + withOutput = regexp(content, ... + '(?m)^\s*function\s+(?:\[[^\]]+\]|\w+)\s*=\s*(\w+)\s*\(', ... + 'tokens'); + withoutOutput = regexp(content, ... + '(?m)^\s*function\s+(\w+)\s*\(', ... + 'tokens'); + names = strings(1, 0); + for k = 1:numel(withOutput) + names(end+1) = string(withOutput{k}{1}); %#ok + end + for k = 1:numel(withoutOutput) + names(end+1) = string(withoutOutput{k}{1}); %#ok + end + names = unique(names); +end + function files = collectRelativeFiles(root, pattern) entries = dir(pattern); files = strings(1, 0); diff --git a/tests/integration/project/ProjectDocumentationGuardrailTest.m b/tests/integration/project/ProjectDocumentationGuardrailTest.m index 6ce854c..7aad54c 100644 --- a/tests/integration/project/ProjectDocumentationGuardrailTest.m +++ b/tests/integration/project/ProjectDocumentationGuardrailTest.m @@ -234,17 +234,6 @@ function appOwnedPackageHelpersDocumentImplementationContracts(testCase) "+labkit/+ui/+app/private/attachColumnResize.m", ... "+labkit/+ui/+app/private/createTabbedWorkbenchShell.m", ... "+labkit/+ui/+app/private/disableAxesInteractivity.m", ... - "+labkit/+ui/+tool/private/addOrInsertAnchor.m", ... - "+labkit/+ui/+tool/private/anchorCurvePoints.m", ... - "+labkit/+ui/+tool/private/createLabeledDropdown.m", ... - "+labkit/+ui/+tool/private/createLabeledEditField.m", ... - "+labkit/+ui/+tool/private/createLabeledSpinner.m", ... - "+labkit/+ui/+tool/private/createReadOnlyInfoRow.m", ... - "+labkit/+ui/+tool/private/createReadOnlyTextField.m", ... - "+labkit/+ui/+tool/private/defaultScaleBarUnits.m", ... - "+labkit/+ui/+tool/private/drawScaleBarOverlay.m", ... - "+labkit/+ui/+tool/private/normalizeScaleBarUnit.m", ... - "+labkit/+ui/+tool/private/scaleBarPanel.m", ... "+labkit/+ui/+view/private/appendLog.m", ... "+labkit/+ui/+view/private/clearAxes.m", ... "+labkit/+ui/+view/private/createLabeledDropdown.m", ...