refactor(views): introduce operation layer with pure and user-intent ops (Phase 2)

Backend (muxplex/views.py):
- Added five pure data operations that mutate settings dict in place:
  add_membership, remove_membership, remove_from_all_views, hide, unhide.
  Each operation is isolated and affects only its named field.
- Added 16 new tests under 'Pure data operations (Phase 2)' in test_views.py.
  Isolation verified: each operation only modifies its target field.

Frontend (muxplex/frontend/app.js):
- Added five pure ops (_opAddMembership, _opRemoveMembership,
  _opRemoveFromAllViews, _opHide, _opUnhide) and _cloneOpState helper.
- Added four user-intent operations with intentional asymmetry:
  * hideSessionOp = hide + removeFromAllViews (federation-safe;
    matches current UX; see design doc for rationale)
  * unhideSessionOp = unhide (orthogonal; does NOT change view membership)
  * addSessionToViewOp = unhide + addMembership (auto-unhide on add;
    expressed as explicit composition)
  * removeSessionFromViewOp = removeMembership (orthogonal; does NOT hide)
- Refactored 7 call sites to use user-intent ops: _doHideSession,
  _doUnhideSession, _doRemoveFromView, flyout submenu toggle, 'New View'
  flow, mobile picker toggle, Manage View checkbox.
- All new operations exported in module.exports.

Frontend tests (muxplex/frontend/tests/test_app.mjs):
- Added 30 new tests under 'Phase 2 operation layer'.
- Cover pure-op isolation, idempotency, no-op behavior, user-intent op
  composition, and guarantee that _serverSettings is never mutated.

Verification:
- muxplex/tests/: 1239 passed (1223 prior + 16 new pure-op tests)
- test_app.mjs: 372 passed, 10 pre-existing failures (Phase 1 had 21;
  Phase 2 reduces to 10; none Phase-2-related; all test unrelated details
  of createNewSession, renderFilterBar, loadGridViewMode)

Intentional inline-PATCH constructions (NOT regressions):
- Creating empty views (~1263, 1347, 1595)
- Moving views up/down in Settings tab (~2520, 2577)
- _saveViewsAndRerender (full views array persistence)
- createNewSession auto-add-to-active-view (session-creation side effect)
These are view-management operations, not session-in-view management.

Builds on Phase 0+1 (commit 0f9623d).
No behavior change — purely structural refactor.

Generated with Amplifier

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
This commit is contained in:
Brian Krabach
2026-05-17 10:10:05 -07:00
parent 0f9623d2c2
commit 03bf24e8d4
4 changed files with 703 additions and 124 deletions
+196 -124
View File
@@ -694,6 +694,139 @@ function getVisibleSessions(sessions) {
return filterVisible(sessions, _serverSettings, _activeView);
}
// =============================================================================
// Operation layer (Phase 2)
//
// Two layers:
// 1. Pure data ops: _opAddMembership/_opRemoveMembership/_opHide/_opUnhide —
// narrow, composable, no side effects beyond their name. Operate on a
// local settings object (typically a clone of _serverSettings) so callers
// can compose multiple operations before PATCHing.
// 2. User-intent ops: hideSessionOp/unhideSessionOp/addSessionToViewOp/
// removeSessionFromViewOp — express what the *user* meant. Some compose
// multiple pure ops:
// - hideSessionOp = hide + removeFromAllViews (asymmetric: v1
// federation-safe; matches current UX)
// - addSessionToViewOp = unhide + addMembership (auto-unhide on add,
// explicit composition)
// - unhideSessionOp = unhide (orthogonal — does NOT touch membership)
// - removeSessionFromViewOp = removeMembership (orthogonal — does NOT
// hide)
//
// The asymmetry between hideSessionOp (which removes from views) and
// addSessionToViewOp (which unhides) is intentional. See
// docs/plans/2026-05-17-hidden-state-redesign-design.md.
// =============================================================================
// --- Pure data ops ---
// Add `key` to view's session list if absent. No-op if view doesn't exist.
function _opAddMembership(state, viewName, key) {
var views = state.views || [];
for (var i = 0; i < views.length; i++) {
if (views[i].name === viewName) {
var sessions = views[i].sessions || [];
if (sessions.indexOf(key) === -1) {
sessions.push(key);
views[i].sessions = sessions;
}
break;
}
}
}
// Remove `key` from view's session list. No-op if view or key absent.
function _opRemoveMembership(state, viewName, key) {
var views = state.views || [];
for (var i = 0; i < views.length; i++) {
if (views[i].name === viewName) {
var sessions = views[i].sessions || [];
var pos = sessions.indexOf(key);
if (pos !== -1) {
sessions.splice(pos, 1);
views[i].sessions = sessions;
}
break;
}
}
}
// Remove `key` from every view's session list.
function _opRemoveFromAllViews(state, key) {
var views = state.views || [];
for (var i = 0; i < views.length; i++) {
var sessions = views[i].sessions || [];
var pos = sessions.indexOf(key);
if (pos !== -1) {
sessions.splice(pos, 1);
views[i].sessions = sessions;
}
}
}
// Append `key` to hidden_sessions if absent.
function _opHide(state, key) {
if (state.hidden_sessions.indexOf(key) === -1) {
state.hidden_sessions.push(key);
}
}
// Remove `key` from hidden_sessions. No-op if absent.
function _opUnhide(state, key) {
var pos = state.hidden_sessions.indexOf(key);
if (pos !== -1) {
state.hidden_sessions.splice(pos, 1);
}
}
// Helper: deep-clone the bits we mutate (avoid touching the cached
// _serverSettings before the PATCH confirms).
function _cloneOpState(settings) {
return JSON.parse(JSON.stringify({
hidden_sessions: (settings && settings.hidden_sessions) || [],
views: (settings && settings.views) || []
}));
}
// --- User-intent ops ---
// Each returns the patch body for /api/settings.
// hideSessionOp: hide(k) + removeFromAllViews(k).
// Asymmetric — removes from all views. This is the v1 federation-safe
// behaviour; matching the current UX. Returns { hidden_sessions, views }.
function hideSessionOp(settings, key) {
var state = _cloneOpState(settings);
_opHide(state, key);
_opRemoveFromAllViews(state, key);
return { hidden_sessions: state.hidden_sessions, views: state.views };
}
// unhideSessionOp: unhide(k) only.
// Orthogonal — does NOT touch view membership. Returns { hidden_sessions }.
function unhideSessionOp(settings, key) {
var state = _cloneOpState(settings);
_opUnhide(state, key);
return { hidden_sessions: state.hidden_sessions };
}
// addSessionToViewOp: unhide(k) + addMembership(k, viewName).
// Auto-unhide on add is explicit composition here, not an invariant
// enforced elsewhere. Returns { hidden_sessions, views }.
function addSessionToViewOp(settings, viewName, key) {
var state = _cloneOpState(settings);
_opUnhide(state, key);
_opAddMembership(state, viewName, key);
return { hidden_sessions: state.hidden_sessions, views: state.views };
}
// removeSessionFromViewOp: removeMembership(k, viewName) only.
// Orthogonal — does NOT hide the session. Returns { views }.
function removeSessionFromViewOp(settings, viewName, key) {
var state = _cloneOpState(settings);
_opRemoveMembership(state, viewName, key);
return { views: state.views };
}
/**
* Resolve the active view name against the known views list.
*
@@ -1924,33 +2057,22 @@ function _openMobileViewPicker(sessionKey, sessionName, unhideFirst) {
if (!viewBtn) return;
var idx = parseInt(viewBtn.dataset.viewIndex, 10);
var updatedViews = JSON.parse(JSON.stringify((_serverSettings && _serverSettings.views) || []));
var view = updatedViews[idx];
var views = (_serverSettings && _serverSettings.views) || [];
var view = views[idx];
if (!view) return;
var sessions = view.sessions || [];
var pos = sessions.indexOf(sessionKey);
var isAlreadyInView = sessions.indexOf(sessionKey) !== -1;
var patch;
var nowIn;
if (pos !== -1) {
sessions.splice(pos, 1);
if (isAlreadyInView) {
patch = removeSessionFromViewOp(_serverSettings, view.name, sessionKey);
nowIn = false;
} else {
sessions.push(sessionKey);
patch = addSessionToViewOp(_serverSettings, view.name, sessionKey);
nowIn = true;
}
view.sessions = sessions;
var patch = { views: updatedViews };
if (unhideFirst && nowIn) {
var hidden = (_serverSettings && _serverSettings.hidden_sessions) || [];
var hiddenIdx = hidden.indexOf(sessionKey);
if (hiddenIdx !== -1) {
var updatedHidden = hidden.slice();
updatedHidden.splice(hiddenIdx, 1);
patch.hidden_sessions = updatedHidden;
unhideFirst = false; // only unhide on first successful add
}
}
// Update checkmark immediately for responsiveness
var checkEl = viewBtn.querySelector('span');
@@ -1959,10 +2081,10 @@ function _openMobileViewPicker(sessionKey, sessionName, unhideFirst) {
api('PATCH', '/api/settings', patch)
.then(function() {
if (_serverSettings) {
_serverSettings.views = updatedViews;
_serverSettings.views = patch.views;
if (patch.hidden_sessions) _serverSettings.hidden_sessions = patch.hidden_sessions;
}
if (patch.hidden_sessions) renderGrid(_currentSessions || []);
if (nowIn && patch.hidden_sessions) renderGrid(_currentSessions || []);
})
.catch(function(err) {
showToast('Couldn\u2019t save \u2014 try again');
@@ -2086,7 +2208,6 @@ function _openFlyoutSubmenu(triggerItem, unhideFirst) {
var newViewAction = e.target.closest('[data-action="new-view-in-flyout"]');
if (newViewAction) {
var capturedKey = sessionKey;
var capturedUnhide = unhideFirst;
closeFlyoutMenu();
var newName = prompt('View name:');
if (!newName || !newName.trim()) return;
@@ -2100,23 +2221,21 @@ function _openFlyoutSubmenu(triggerItem, unhideFirst) {
showToast('View \'' + newName + '\' already exists');
return;
}
// New-view creation: addSessionToViewOp doesn't model view creation, but
// we use it on a temp settings (with the new view already appended) so
// that the hidden_sessions update is expressed via the op layer.
var newView = { name: newName, sessions: [capturedKey] };
var newViews = existViews.concat([newView]);
var flyoutPatch = { views: newViews };
if (capturedUnhide) {
var hiddenList = (_serverSettings && _serverSettings.hidden_sessions) || [];
var hi = hiddenList.indexOf(capturedKey);
if (hi !== -1) {
var updHidden = hiddenList.slice();
updHidden.splice(hi, 1);
flyoutPatch.hidden_sessions = updHidden;
}
}
var tempSettings = {
hidden_sessions: (_serverSettings && _serverSettings.hidden_sessions) || [],
views: newViews
};
var flyoutPatch = addSessionToViewOp(tempSettings, newName, capturedKey);
api('PATCH', '/api/settings', flyoutPatch)
.then(function() {
if (_serverSettings) {
_serverSettings.views = newViews;
if (flyoutPatch.hidden_sessions) _serverSettings.hidden_sessions = flyoutPatch.hidden_sessions;
_serverSettings.views = flyoutPatch.views;
_serverSettings.hidden_sessions = flyoutPatch.hidden_sessions;
}
switchView(newName);
})
@@ -2130,35 +2249,24 @@ function _openFlyoutSubmenu(triggerItem, unhideFirst) {
if (!btn) return;
var idx = parseInt(btn.dataset.viewIndex, 10);
var updatedViews = JSON.parse(JSON.stringify((_serverSettings && _serverSettings.views) || []));
var view = updatedViews[idx];
var views = (_serverSettings && _serverSettings.views) || [];
var view = views[idx];
if (!view) return;
var sessions = view.sessions || [];
var pos = sessions.indexOf(sessionKey);
if (pos !== -1) {
sessions.splice(pos, 1);
var isAlreadyInView = sessions.indexOf(sessionKey) !== -1;
var patch;
if (isAlreadyInView) {
patch = removeSessionFromViewOp(_serverSettings, view.name, sessionKey);
} else {
sessions.push(sessionKey);
}
view.sessions = sessions;
var patch = { views: updatedViews };
if (unhideFirst) {
var hidden = (_serverSettings && _serverSettings.hidden_sessions) || [];
var hiddenIdx = hidden.indexOf(sessionKey);
if (hiddenIdx !== -1) {
var updatedHidden = hidden.slice();
updatedHidden.splice(hiddenIdx, 1);
patch.hidden_sessions = updatedHidden;
}
patch = addSessionToViewOp(_serverSettings, view.name, sessionKey);
}
api('PATCH', '/api/settings', patch)
.then(function() {
if (_serverSettings) {
_serverSettings.views = updatedViews;
_serverSettings.views = patch.views;
if (patch.hidden_sessions) _serverSettings.hidden_sessions = patch.hidden_sessions;
}
// Update checkmarks in submenu
@@ -2167,12 +2275,13 @@ function _openFlyoutSubmenu(triggerItem, unhideFirst) {
for (var ci = 0; ci < checkItems.length; ci++) {
var vi = parseInt(checkItems[ci].dataset.viewIndex, 10);
var checkEl = checkItems[ci].querySelector('.flyout-submenu__check');
if (checkEl && updatedViews[vi]) {
checkEl.textContent = (updatedViews[vi].sessions || []).indexOf(sessionKey) !== -1 ? '\u2713' : '';
var updViews = (_serverSettings && _serverSettings.views) || [];
if (checkEl && updViews[vi]) {
checkEl.textContent = (updViews[vi].sessions || []).indexOf(sessionKey) !== -1 ? '\u2713' : '';
}
}
}
if (unhideFirst) {
if (!isAlreadyInView && patch.hidden_sessions) {
renderGrid(_currentSessions || []);
}
})
@@ -2191,30 +2300,15 @@ function _doHideSession() {
var sessionKey = _flyoutSessionKey;
if (!sessionKey) return;
var hidden = (_serverSettings && _serverSettings.hidden_sessions) || [];
var views = (_serverSettings && _serverSettings.views) || [];
// Add to hidden_sessions
var updatedHidden = hidden.slice();
if (updatedHidden.indexOf(sessionKey) === -1) {
updatedHidden.push(sessionKey);
}
// Remove from all views (mutual exclusion)
var updatedViews = JSON.parse(JSON.stringify(views));
for (var i = 0; i < updatedViews.length; i++) {
var sessions = updatedViews[i].sessions || [];
var idx = sessions.indexOf(sessionKey);
if (idx !== -1) sessions.splice(idx, 1);
}
var patch = hideSessionOp(_serverSettings, sessionKey);
closeFlyoutMenu();
api('PATCH', '/api/settings', { hidden_sessions: updatedHidden, views: updatedViews })
api('PATCH', '/api/settings', patch)
.then(function() {
if (_serverSettings) {
_serverSettings.hidden_sessions = updatedHidden;
_serverSettings.views = updatedViews;
_serverSettings.hidden_sessions = patch.hidden_sessions;
_serverSettings.views = patch.views;
}
renderGrid(_currentSessions || []);
renderViewDropdown();
@@ -2233,18 +2327,17 @@ function _doUnhideSession() {
var sessionKey = _flyoutSessionKey;
if (!sessionKey) return;
// Early-exit if session is not hidden (no PATCH needed).
var hidden = (_serverSettings && _serverSettings.hidden_sessions) || [];
var idx = hidden.indexOf(sessionKey);
if (idx === -1) { closeFlyoutMenu(); return; }
if (hidden.indexOf(sessionKey) === -1) { closeFlyoutMenu(); return; }
var updatedHidden = hidden.slice();
updatedHidden.splice(idx, 1);
var patch = unhideSessionOp(_serverSettings, sessionKey);
closeFlyoutMenu();
api('PATCH', '/api/settings', { hidden_sessions: updatedHidden })
api('PATCH', '/api/settings', patch)
.then(function() {
if (_serverSettings) _serverSettings.hidden_sessions = updatedHidden;
if (_serverSettings) _serverSettings.hidden_sessions = patch.hidden_sessions;
renderGrid(_currentSessions || []);
renderViewDropdown();
})
@@ -2262,24 +2355,13 @@ function _doRemoveFromView() {
var sessionKey = _flyoutSessionKey;
if (!sessionKey || _activeView === 'all' || _activeView === 'hidden') return;
var views = (_serverSettings && _serverSettings.views) || [];
var updatedViews = JSON.parse(JSON.stringify(views));
// Find the active view and remove the session
for (var i = 0; i < updatedViews.length; i++) {
if (updatedViews[i].name === _activeView) {
var sessions = updatedViews[i].sessions || [];
var idx = sessions.indexOf(sessionKey);
if (idx !== -1) sessions.splice(idx, 1);
break;
}
}
var patch = removeSessionFromViewOp(_serverSettings, _activeView, sessionKey);
closeFlyoutMenu();
api('PATCH', '/api/settings', { views: updatedViews })
api('PATCH', '/api/settings', patch)
.then(function() {
if (_serverSettings) _serverSettings.views = updatedViews;
if (_serverSettings) _serverSettings.views = patch.views;
renderGrid(_currentSessions || []);
})
.catch(function(err) {
@@ -2606,40 +2688,18 @@ function renderManageViewList() {
if (!cb) return;
var sessionKey = cb.dataset.sessionKey;
var isChecked = cb.checked;
var isHiddenSession = cb.dataset.isHidden === '1';
var views = (_serverSettings && _serverSettings.views) || [];
var updatedViews = JSON.parse(JSON.stringify(views));
for (var vi = 0; vi < updatedViews.length; vi++) {
if (updatedViews[vi].name === _activeView) {
var vs = updatedViews[vi].sessions || [];
if (isChecked) {
if (vs.indexOf(sessionKey) === -1) vs.push(sessionKey);
} else {
var pos = vs.indexOf(sessionKey);
if (pos !== -1) vs.splice(pos, 1);
}
updatedViews[vi].sessions = vs;
break;
}
}
var patch = { views: updatedViews };
if (isHiddenSession && isChecked) {
var hiddenList = (_serverSettings && _serverSettings.hidden_sessions) || [];
var hi = hiddenList.indexOf(sessionKey);
if (hi !== -1) {
var updatedHidden = hiddenList.slice();
updatedHidden.splice(hi, 1);
patch.hidden_sessions = updatedHidden;
}
var patch;
if (isChecked) {
patch = addSessionToViewOp(_serverSettings, _activeView, sessionKey);
} else {
patch = removeSessionFromViewOp(_serverSettings, _activeView, sessionKey);
}
api('PATCH', '/api/settings', patch)
.then(function() {
if (_serverSettings) {
_serverSettings.views = updatedViews;
_serverSettings.views = patch.views;
if (patch.hidden_sessions) _serverSettings.hidden_sessions = patch.hidden_sessions;
}
// Update summary count in-place — do NOT re-render the full list (avoids layout thrash)
@@ -4528,6 +4588,18 @@ if (typeof module !== 'undefined' && module.exports) {
isHidden,
filterVisible,
visibleCount,
// Operation layer (Phase 2) — pure data ops
_opAddMembership,
_opRemoveMembership,
_opRemoveFromAllViews,
_opHide,
_opUnhide,
_cloneOpState,
// Operation layer (Phase 2) — user-intent ops
hideSessionOp,
unhideSessionOp,
addSessionToViewOp,
removeSessionFromViewOp,
// Federation tiles
buildStatusTileHTML,
// Constants
+283
View File
@@ -5066,3 +5066,286 @@ test('visibleCount always equals filterVisible().length for the same inputs', ()
);
}
});
// =============================================================================
// Phase 2 operation layer
// =============================================================================
// Helper: build a settings-like object with both views and hidden_sessions.
function _settingsWithState() {
return {
hidden_sessions: ['dev1:a', 'dev1:b'],
views: [
{ name: 'Work', sessions: ['dev1:a', 'dev1:c'] },
{ name: 'Personal', sessions: ['dev1:b', 'dev1:d'] },
],
};
}
// --- Exports check ---
test('app.js exports all Phase 2 operation layer functions', () => {
const fns = [
'_opAddMembership', '_opRemoveMembership', '_opRemoveFromAllViews',
'_opHide', '_opUnhide', '_cloneOpState',
'hideSessionOp', 'unhideSessionOp', 'addSessionToViewOp', 'removeSessionFromViewOp',
];
for (const fn of fns) {
assert.ok(fn in app, `app.js should export "${fn}"`);
assert.strictEqual(typeof app[fn], 'function', `"${fn}" should be a function`);
}
});
// --- _cloneOpState ---
test('_cloneOpState returns a deep clone — mutations do not affect the original', () => {
const settings = _settingsWithState();
const clone = app._cloneOpState(settings);
clone.hidden_sessions.push('dev1:new');
clone.views[0].sessions.push('dev1:extra');
// Original untouched
assert.strictEqual(settings.hidden_sessions.indexOf('dev1:new'), -1);
assert.strictEqual(settings.views[0].sessions.indexOf('dev1:extra'), -1);
});
// --- Pure op isolation: each op affects ONLY its named field ---
test('_opAddMembership does not touch hidden_sessions', () => {
const settings = _settingsWithState();
const state = app._cloneOpState(settings);
const hiddenBefore = JSON.stringify(state.hidden_sessions);
app._opAddMembership(state, 'Work', 'dev1:new');
assert.strictEqual(JSON.stringify(state.hidden_sessions), hiddenBefore);
});
test('_opRemoveMembership does not touch hidden_sessions', () => {
const settings = _settingsWithState();
const state = app._cloneOpState(settings);
const hiddenBefore = JSON.stringify(state.hidden_sessions);
app._opRemoveMembership(state, 'Work', 'dev1:a');
assert.strictEqual(JSON.stringify(state.hidden_sessions), hiddenBefore);
});
test('_opRemoveFromAllViews does not touch hidden_sessions', () => {
const settings = _settingsWithState();
const state = app._cloneOpState(settings);
const hiddenBefore = JSON.stringify(state.hidden_sessions);
app._opRemoveFromAllViews(state, 'dev1:a');
assert.strictEqual(JSON.stringify(state.hidden_sessions), hiddenBefore);
});
test('_opHide does not touch views', () => {
const settings = _settingsWithState();
const state = app._cloneOpState(settings);
const viewsBefore = JSON.stringify(state.views);
app._opHide(state, 'dev1:new');
assert.strictEqual(JSON.stringify(state.views), viewsBefore);
});
test('_opUnhide does not touch views', () => {
const settings = _settingsWithState();
const state = app._cloneOpState(settings);
const viewsBefore = JSON.stringify(state.views);
app._opUnhide(state, 'dev1:a');
assert.strictEqual(JSON.stringify(state.views), viewsBefore);
});
// --- _opAddMembership ---
test('_opAddMembership adds key when absent', () => {
const state = app._cloneOpState(_settingsWithState());
app._opAddMembership(state, 'Work', 'dev1:new');
assert.ok(state.views[0].sessions.includes('dev1:new'));
});
test('_opAddMembership is idempotent — no duplicate', () => {
const state = app._cloneOpState(_settingsWithState());
app._opAddMembership(state, 'Work', 'dev1:a'); // already present
assert.strictEqual(state.views[0].sessions.filter(k => k === 'dev1:a').length, 1);
});
test('_opAddMembership is no-op for unknown view', () => {
const settings = _settingsWithState();
const state = app._cloneOpState(settings);
const viewsBefore = JSON.stringify(state.views);
app._opAddMembership(state, 'DoesNotExist', 'dev1:new');
assert.strictEqual(JSON.stringify(state.views), viewsBefore);
});
// --- _opRemoveMembership ---
test('_opRemoveMembership removes key when present', () => {
const state = app._cloneOpState(_settingsWithState());
app._opRemoveMembership(state, 'Work', 'dev1:a');
assert.ok(!state.views[0].sessions.includes('dev1:a'));
});
test('_opRemoveMembership is no-op when key absent', () => {
const settings = _settingsWithState();
const state = app._cloneOpState(settings);
const workBefore = JSON.stringify(state.views[0].sessions);
app._opRemoveMembership(state, 'Work', 'dev1:nothere');
assert.strictEqual(JSON.stringify(state.views[0].sessions), workBefore);
});
test('_opRemoveMembership is no-op for unknown view', () => {
const settings = _settingsWithState();
const state = app._cloneOpState(settings);
const viewsBefore = JSON.stringify(state.views);
app._opRemoveMembership(state, 'DoesNotExist', 'dev1:a');
assert.strictEqual(JSON.stringify(state.views), viewsBefore);
});
// --- _opRemoveFromAllViews ---
test('_opRemoveFromAllViews removes key from every view', () => {
const state = {
hidden_sessions: ['dev1:x'],
views: [
{ name: 'Work', sessions: ['dev1:x', 'dev1:y'] },
{ name: 'Personal', sessions: ['dev1:x', 'dev1:z'] },
],
};
app._opRemoveFromAllViews(state, 'dev1:x');
assert.ok(!state.views[0].sessions.includes('dev1:x'));
assert.ok(!state.views[1].sessions.includes('dev1:x'));
// Other keys untouched
assert.ok(state.views[0].sessions.includes('dev1:y'));
assert.ok(state.views[1].sessions.includes('dev1:z'));
});
// --- _opHide ---
test('_opHide adds key to hidden_sessions when absent', () => {
const state = { hidden_sessions: ['dev1:b'], views: [] };
app._opHide(state, 'dev1:new');
assert.ok(state.hidden_sessions.includes('dev1:new'));
});
test('_opHide is idempotent — no duplicate', () => {
const state = { hidden_sessions: ['dev1:a'], views: [] };
app._opHide(state, 'dev1:a');
assert.strictEqual(state.hidden_sessions.filter(k => k === 'dev1:a').length, 1);
});
// --- _opUnhide ---
test('_opUnhide removes key from hidden_sessions', () => {
const state = { hidden_sessions: ['dev1:a', 'dev1:b'], views: [] };
app._opUnhide(state, 'dev1:a');
assert.ok(!state.hidden_sessions.includes('dev1:a'));
assert.ok(state.hidden_sessions.includes('dev1:b'));
});
test('_opUnhide is no-op when key absent', () => {
const state = { hidden_sessions: ['dev1:b'], views: [] };
app._opUnhide(state, 'dev1:nothere');
assert.deepStrictEqual(state.hidden_sessions, ['dev1:b']);
});
// --- hideSessionOp (user-intent, asymmetric) ---
test('hideSessionOp puts key in hidden_sessions AND removes from all views', () => {
const settings = _settingsWithState();
// dev1:a is in Work and in hidden_sessions already; dev1:c is in Work, not hidden
// Let's use a clean state where dev1:c is visible (in Work, not hidden)
const s = {
hidden_sessions: [],
views: [
{ name: 'Work', sessions: ['dev1:c', 'dev1:d'] },
{ name: 'Personal', sessions: ['dev1:c', 'dev1:e'] },
],
};
const patch = app.hideSessionOp(s, 'dev1:c');
assert.ok(patch.hidden_sessions.includes('dev1:c'), 'hidden_sessions must contain dev1:c');
assert.ok(!patch.views[0].sessions.includes('dev1:c'), 'Work must not contain dev1:c');
assert.ok(!patch.views[1].sessions.includes('dev1:c'), 'Personal must not contain dev1:c');
// Other keys unaffected
assert.ok(patch.views[0].sessions.includes('dev1:d'));
assert.ok(patch.views[1].sessions.includes('dev1:e'));
});
test('hideSessionOp returns both hidden_sessions and views in patch', () => {
const patch = app.hideSessionOp(_settingsWithState(), 'dev1:x');
assert.ok('hidden_sessions' in patch, 'patch must have hidden_sessions');
assert.ok('views' in patch, 'patch must have views');
});
// --- unhideSessionOp (user-intent, orthogonal) ---
test('unhideSessionOp removes key from hidden_sessions', () => {
const settings = _settingsWithState(); // dev1:a is in hidden_sessions
const patch = app.unhideSessionOp(settings, 'dev1:a');
assert.ok(!patch.hidden_sessions.includes('dev1:a'));
});
test('unhideSessionOp returns ONLY hidden_sessions — views key is absent', () => {
const patch = app.unhideSessionOp(_settingsWithState(), 'dev1:a');
assert.ok('hidden_sessions' in patch, 'patch must have hidden_sessions');
assert.ok(!('views' in patch), 'patch must NOT have views');
});
// --- addSessionToViewOp (user-intent, auto-unhide) ---
test('addSessionToViewOp adds key to named view AND removes from hidden_sessions', () => {
// dev1:b is hidden AND not in Work view
const settings = {
hidden_sessions: ['dev1:b'],
views: [
{ name: 'Work', sessions: ['dev1:a'] },
],
};
const patch = app.addSessionToViewOp(settings, 'Work', 'dev1:b');
assert.ok(patch.views[0].sessions.includes('dev1:b'), 'Work must now contain dev1:b');
assert.ok(!patch.hidden_sessions.includes('dev1:b'), 'hidden_sessions must not contain dev1:b');
});
test('addSessionToViewOp returns both hidden_sessions and views', () => {
const patch = app.addSessionToViewOp(_settingsWithState(), 'Work', 'dev1:z');
assert.ok('hidden_sessions' in patch, 'patch must have hidden_sessions');
assert.ok('views' in patch, 'patch must have views');
});
// --- removeSessionFromViewOp (user-intent, orthogonal) ---
test('removeSessionFromViewOp removes key from named view', () => {
const settings = _settingsWithState(); // dev1:a is in Work
const patch = app.removeSessionFromViewOp(settings, 'Work', 'dev1:a');
assert.ok(!patch.views[0].sessions.includes('dev1:a'));
});
test('removeSessionFromViewOp returns ONLY views — hidden_sessions is absent', () => {
const patch = app.removeSessionFromViewOp(_settingsWithState(), 'Work', 'dev1:a');
assert.ok('views' in patch, 'patch must have views');
assert.ok(!('hidden_sessions' in patch), 'patch must NOT have hidden_sessions');
});
// --- No mutation of _serverSettings ---
test('hideSessionOp does not mutate its settings argument', () => {
const settings = _settingsWithState();
const before = JSON.stringify(settings);
app.hideSessionOp(settings, 'dev1:c');
assert.strictEqual(JSON.stringify(settings), before, 'settings must not be mutated');
});
test('unhideSessionOp does not mutate its settings argument', () => {
const settings = _settingsWithState();
const before = JSON.stringify(settings);
app.unhideSessionOp(settings, 'dev1:a');
assert.strictEqual(JSON.stringify(settings), before, 'settings must not be mutated');
});
test('addSessionToViewOp does not mutate its settings argument', () => {
const settings = _settingsWithState();
const before = JSON.stringify(settings);
app.addSessionToViewOp(settings, 'Work', 'dev1:b');
assert.strictEqual(JSON.stringify(settings), before, 'settings must not be mutated');
});
test('removeSessionFromViewOp does not mutate its settings argument', () => {
const settings = _settingsWithState();
const before = JSON.stringify(settings);
app.removeSessionFromViewOp(settings, 'Work', 'dev1:a');
assert.strictEqual(JSON.stringify(settings), before, 'settings must not be mutated');
});
+165
View File
@@ -3,10 +3,15 @@ Tests for muxplex/views.py — views invariant enforcement and v2 visibility hel
"""
from muxplex.views import (
add_membership,
enforce_mutual_exclusion,
filter_visible,
hide,
is_hidden,
normalize_session_keys,
remove_from_all_views,
remove_membership,
unhide,
validate_view_name,
visible_count,
)
@@ -404,3 +409,163 @@ def test_normalize_handles_cross_device_name_collisions_safely():
# implementation uses setdefault, so first-seen wins. Document the behavior
# in this test so the choice is visible.
assert result["hidden_sessions"][0] in {"dev1:a", "dev2:a"}
# ---------------------------------------------------------------------------
# Pure data operations (Phase 2)
# See docs/plans/2026-05-17-hidden-state-redesign-design.md
# ---------------------------------------------------------------------------
def _settings_with_state() -> dict:
"""Return a settings dict with both views and hidden_sessions populated."""
return {
"hidden_sessions": ["dev1:a", "dev1:b"],
"views": [
{"name": "Work", "sessions": ["dev1:a", "dev1:c"]},
{"name": "Personal", "sessions": ["dev1:b", "dev1:d"]},
],
}
# --- add_membership ---
def test_add_membership_adds_key_when_absent():
settings = _settings_with_state()
result = add_membership(settings, "Work", "dev1:new")
assert "dev1:new" in result["views"][0]["sessions"]
def test_add_membership_is_idempotent():
settings = _settings_with_state()
add_membership(settings, "Work", "dev1:a") # already present
assert settings["views"][0]["sessions"].count("dev1:a") == 1
def test_add_membership_is_noop_for_unknown_view():
settings = _settings_with_state()
original_work = settings["views"][0]["sessions"][:]
original_personal = settings["views"][1]["sessions"][:]
result = add_membership(settings, "DoesNotExist", "dev1:new")
assert result["views"][0]["sessions"] == original_work
assert result["views"][1]["sessions"] == original_personal
def test_add_membership_does_not_touch_hidden_sessions():
"""Pure op: add_membership must not touch hidden_sessions."""
settings = _settings_with_state()
original_hidden = settings["hidden_sessions"][:]
add_membership(settings, "Work", "dev1:new")
assert settings["hidden_sessions"] == original_hidden
# --- remove_membership ---
def test_remove_membership_removes_the_key():
settings = _settings_with_state()
result = remove_membership(settings, "Work", "dev1:a")
assert "dev1:a" not in result["views"][0]["sessions"]
def test_remove_membership_is_noop_when_key_absent():
settings = _settings_with_state()
before = settings["views"][0]["sessions"][:]
remove_membership(settings, "Work", "dev1:nothere")
assert settings["views"][0]["sessions"] == before
def test_remove_membership_is_noop_for_unknown_view():
settings = _settings_with_state()
original_work = settings["views"][0]["sessions"][:]
original_personal = settings["views"][1]["sessions"][:]
result = remove_membership(settings, "DoesNotExist", "dev1:a")
assert result["views"][0]["sessions"] == original_work
assert result["views"][1]["sessions"] == original_personal
def test_remove_membership_does_not_touch_hidden_sessions():
"""Pure op: remove_membership must not touch hidden_sessions."""
settings = _settings_with_state()
original_hidden = settings["hidden_sessions"][:]
remove_membership(settings, "Work", "dev1:a")
assert settings["hidden_sessions"] == original_hidden
# --- remove_from_all_views ---
def test_remove_from_all_views_clears_key_from_all_views():
settings = {
"hidden_sessions": ["dev1:x"],
"views": [
{"name": "Work", "sessions": ["dev1:x", "dev1:y"]},
{"name": "Personal", "sessions": ["dev1:x", "dev1:z"]},
],
}
result = remove_from_all_views(settings, "dev1:x")
assert "dev1:x" not in result["views"][0]["sessions"]
assert "dev1:x" not in result["views"][1]["sessions"]
assert "dev1:y" in result["views"][0]["sessions"]
assert "dev1:z" in result["views"][1]["sessions"]
def test_remove_from_all_views_does_not_touch_hidden_sessions():
"""Pure op: remove_from_all_views must not touch hidden_sessions."""
settings = _settings_with_state()
original_hidden = settings["hidden_sessions"][:]
remove_from_all_views(settings, "dev1:a")
assert settings["hidden_sessions"] == original_hidden
# --- hide ---
def test_hide_adds_to_hidden_sessions_when_absent():
settings = {"hidden_sessions": ["dev1:b"], "views": []}
result = hide(settings, "dev1:new")
assert "dev1:new" in result["hidden_sessions"]
def test_hide_is_idempotent():
settings = {"hidden_sessions": ["dev1:a"], "views": []}
hide(settings, "dev1:a") # already present
assert settings["hidden_sessions"].count("dev1:a") == 1
def test_hide_does_not_touch_views():
"""Pure op: hide must not touch views."""
settings = _settings_with_state()
original_work = settings["views"][0]["sessions"][:]
original_personal = settings["views"][1]["sessions"][:]
hide(settings, "dev1:new")
assert settings["views"][0]["sessions"] == original_work
assert settings["views"][1]["sessions"] == original_personal
# --- unhide ---
def test_unhide_removes_from_hidden_sessions():
settings = {"hidden_sessions": ["dev1:a", "dev1:b"], "views": []}
result = unhide(settings, "dev1:a")
assert "dev1:a" not in result["hidden_sessions"]
assert "dev1:b" in result["hidden_sessions"]
def test_unhide_is_noop_when_absent():
settings = {"hidden_sessions": ["dev1:b"], "views": []}
before = settings["hidden_sessions"][:]
unhide(settings, "dev1:nothere")
assert settings["hidden_sessions"] == before
def test_unhide_does_not_touch_views():
"""Pure op: unhide must not touch views."""
settings = _settings_with_state()
original_work = settings["views"][0]["sessions"][:]
original_personal = settings["views"][1]["sessions"][:]
unhide(settings, "dev1:a")
assert settings["views"][0]["sessions"] == original_work
assert settings["views"][1]["sessions"] == original_personal
+59
View File
@@ -203,6 +203,65 @@ def enforce_mutual_exclusion(settings: dict) -> dict:
return settings
# ---------------------------------------------------------------------------
# Pure data ops (Phase 2)
#
# Pure data ops. Composable. No tangling of concerns. User-intent ops live on
# the frontend (where the PATCH boundary is) and call these to build the final
# state.
#
# Each mutates the settings dict in place and returns it. No side effects
# beyond the named operation.
# ---------------------------------------------------------------------------
def add_membership(settings: dict, view_name: str, key: str) -> dict:
"""Add `key` to view's session list if absent. No-op if view doesn't exist."""
for view in settings.get("views") or []:
if view.get("name") == view_name:
sessions = view.setdefault("sessions", [])
if key not in sessions:
sessions.append(key)
break
return settings
def remove_membership(settings: dict, view_name: str, key: str) -> dict:
"""Remove `key` from view's session list. No-op if view or key absent."""
for view in settings.get("views") or []:
if view.get("name") == view_name:
sessions = view.get("sessions") or []
if key in sessions:
sessions.remove(key)
break
return settings
def remove_from_all_views(settings: dict, key: str) -> dict:
"""Remove `key` from every view's session list."""
for view in settings.get("views") or []:
sessions = view.get("sessions") or []
if key in sessions:
sessions.remove(key)
return settings
def hide(settings: dict, key: str) -> dict:
"""Append `key` to hidden_sessions if absent."""
hidden = settings.setdefault("hidden_sessions", [])
if key not in hidden:
hidden.append(key)
return settings
def unhide(settings: dict, key: str) -> dict:
"""Remove `key` from hidden_sessions. No-op if absent."""
hidden = settings.get("hidden_sessions") or []
if key in hidden:
hidden.remove(key)
return settings
def validate_view_name(name: str, existing_views: list[dict]) -> str | None:
"""Validate a view name. Returns an error message string, or None if valid.