diff --git a/changelog.d/tsk-5baas4-stats-notif-flicker.md b/changelog.d/tsk-5baas4-stats-notif-flicker.md new file mode 100644 index 000000000..a46a81b86 --- /dev/null +++ b/changelog.d/tsk-5baas4-stats-notif-flicker.md @@ -0,0 +1,24 @@ +### Fixed + +- Phone lock screen: the system stats widget no longer flickers. The stats poll + runs every three seconds and rebuilt the whole panel each time, and + `.ls-stat-card` carries the same 520ms entrance animation the agent islands + do -- so the card replayed its entrance twenty times a minute. Measured in + chromium at the device's 540x1200 viewport before the fix: two entrance + replays in 7.5 seconds, the card a different and already-detached element + each time; after: none. The panel is now reconciled part by part and the + readings are written in place. + +- Phone lock screen: the meter bars now animate to their new value instead of + snapping to it. `.ls-stat-fill` has a 420ms width transition, which needs an + element that survives the repaint to have a width to travel FROM; every bar + was a brand new element, so every bar jumped. + +- Phone lock screen: a reading that stops being measured now loses its bar + rather than leaving the last one on screen. A meter frozen at its final value + is indistinguishable from a live one. + +- Phone lock screen: the notification stacks are reconciled by source, so a + stack whose notifications have not changed keeps its element, its entrance + animation and its keyboard focus. A stack whose contents genuinely changed is + still rebuilt, because that is new content arriving. diff --git a/changelog.d/tsk-bgxtxn-lockscreen-glass-bugs.md b/changelog.d/tsk-bgxtxn-lockscreen-glass-bugs.md new file mode 100644 index 000000000..957b044da --- /dev/null +++ b/changelog.d/tsk-bgxtxn-lockscreen-glass-bugs.md @@ -0,0 +1,16 @@ +### Fixed + +- Phone lock screen: swiping up from an agent island no longer opens the PIN + keypad. Measured at the device's real viewport with a full screen of agents, + `#ls-feed` does not overflow -- it is content-sized and the islands fit -- so + the unlock veto, which asked only how far the feed could still travel, was + never arming. A feed that cannot scroll at all is now treated as the cards + they are rather than as a feed already read to its end. Swiping up at the end + of a feed that CAN scroll still unlocks, unchanged. + +- Phone lock screen: the agent islands no longer flicker every fifteen seconds. + The activity poll wiped the list and rebuilt every island, so each one was a + new element replaying its 520ms entrance animation on every tick, whether or + not anything had changed. The list is now reconciled by agent name and updated + in place: an island that persists keeps its element, its animation and its + keyboard focus. diff --git a/tests/test_lock_screen_gestures.py b/tests/test_lock_screen_gestures.py index 7952c40b0..fa9e5be08 100644 --- a/tests/test_lock_screen_gestures.py +++ b/tests/test_lock_screen_gestures.py @@ -34,17 +34,38 @@ def _balanced(src: str, start: int, opener: str, closer: str) -> str: Quote-aware, because a brace inside a string literal would otherwise end the span early and hand the caller a slice that happens to parse. + + COMMENT-aware for the same reason, and it is not hypothetical: the comment + ``// The App Store's own artwork`` inside ``island()`` opened an apostrophe + that never closed, so every brace after it was read as string content and + ``_function("island")`` quietly returned 12kB -- the whole of island(), + reconcileIslands() AND paintActivity(). It still parsed, and every test + still passed, because the extra functions were the real ones. A harness + that hands back three times what it was asked for is not measuring what it + claims to; the next divergence would not be so harmless. """ i = src.index(opener, start) depth, quote = 0, "" while i < len(src): ch = src[i] + nxt = src[i + 1] if i + 1 < len(src) else "" if quote: if ch == "\\": i += 2 continue if ch == quote: quote = "" + elif ch == "/" and nxt == "/": + i = src.find("\n", i) + if i == -1: + break + continue + elif ch == "/" and nxt == "*": + end = src.find("*/", i + 2) + if end == -1: + break + i = end + 2 + continue elif ch in "\"'": quote = ch elif ch == opener: @@ -140,7 +161,9 @@ def _unlock_wiring() -> str: """ -def _gesture_source(*, with_fix: bool = True, ignore_scroll: bool = False) -> str: +def _gesture_source( + *, with_fix: bool = True, ignore_scroll: bool = False, ignore_dead_feed: bool = False +) -> str: """The real source of the gesture machinery, optionally de-fixed. `with_fix=False` rebuilds the wiring as it stood before tsk-6bjsvg -- the @@ -161,6 +184,16 @@ def _gesture_source(*, with_fix: bool = True, ignore_scroll: bool = False) -> st ) room = mutated wiring = _unlock_wiring() + if ignore_dead_feed: + # The plausible wrong answer for Jay's glass bug: keep asking how much + # room is left and drop the question of whether the feed can scroll at + # all. That is the code as it stood when he reported it. + mutated = wiring.replace(" || !feedOverflows()", "") + assert mutated != wiring, ( + "the mutation changed nothing -- the veto no longer asks " + "!feedOverflows() the way this mutation assumes, so it proves nothing" + ) + wiring = mutated if not with_fix: # Drop the 5th argument (the veto) and nothing else. guard_end = wiring.rindex("}, function (ev) {") @@ -169,7 +202,13 @@ def _gesture_source(*, with_fix: bool = True, ignore_scroll: bool = False) -> st return "\n".join([_function("feedOverflows"), room, _function("swipe"), wiring]) -def _drive(scenario: dict, *, with_fix: bool = True, ignore_scroll: bool = False) -> int: +def _drive( + scenario: dict, + *, + with_fix: bool = True, + ignore_scroll: bool = False, + ignore_dead_feed: bool = False, +) -> int: """Run one gesture and return how many times unlock was triggered.""" node = shutil.which("node") if node is None: # pragma: no cover - depends on the runner image @@ -185,7 +224,11 @@ def _drive(scenario: dict, *, with_fix: bool = True, ignore_scroll: bool = False ) script = _HARNESS.replace( "__GESTURE_SOURCE__", - _gesture_source(with_fix=with_fix, ignore_scroll=ignore_scroll), + _gesture_source( + with_fix=with_fix, + ignore_scroll=ignore_scroll, + ignore_dead_feed=ignore_dead_feed, + ), ) done = subprocess.run( [node, "-e", script], @@ -247,15 +290,33 @@ def test_drag_anywhere_else_still_unlocks(self): """The positive case, without which a veto could pass by never unlocking.""" assert _drive(_scenario(startOn="body", endOn="body")) == 1 - def test_a_feed_with_nothing_to_scroll_still_unlocks(self): - """A feed that cannot move is not being read. + def test_a_feed_with_nothing_to_scroll_does_not_unlock_from_a_card(self): + """REVERSED DELIBERATELY. This test used to assert the opposite. + + The old rule said a feed that cannot move is not being read, so a drag + starting on it should unlock. That was reasoned from a device with one + agent and no notifications. MEASURED on the real device instead -- + 540x1200, sway scale 2.0, the six agents the demo phone actually shows + -- `#ls-feed` reports scrollHeight 394 and clientHeight 394. The feed is + content-sized: it does not overflow, and it never did. So this was not a + corner case for a brand-new phone, it was the ORDINARY state of the + screen, and every drag that began on an agent island opened the keypad. + That is the bug Jay reported from the glass. + + The unlock gesture is not lost: the feed is 394px of a 1200px screen and + the rest of the glass still unlocks, which is what + `test_a_non_scrolling_feed_does_not_kill_the_gesture_elsewhere` holds to. + """ + assert _drive(_scenario(feedScrollHeight=300)) == 0 - On a device with one agent and no notifications the feed still covers - the middle of the glass. Vetoing there would trade Jay's bug for a dead - unlock gesture over most of the screen, on the first screen a new user - sees. The cut-edge fade already measures overflow for the same reason. + def test_a_non_scrolling_feed_does_not_kill_the_gesture_elsewhere(self): + """The cost of the reversal above, bounded. + + Vetoing a non-scrolling feed is only acceptable while the unlock swipe + still works everywhere else. Without this, the fix for Jay's bug could + be "nothing unlocks any more" and the suite would not notice. """ - assert _drive(_scenario(feedScrollHeight=300)) == 1 + assert _drive(_scenario(feedScrollHeight=300, startOn="body", endOn="body")) == 1 def test_the_veto_does_not_reach_past_the_resting_screen(self): """With a sheet open the unlock swipe was already disarmed; keep it so.""" @@ -308,9 +369,24 @@ def test_a_fractional_pixel_short_of_the_end_counts_as_the_end(self): """ assert _drive(_scenario(feedScrollTop=599.6)) == 1 - def test_a_feed_with_nothing_to_scroll_is_already_at_its_end(self): - """#3102's narrowing, restated in the new terms and still true.""" - assert _drive(_scenario(feedScrollHeight=300, feedScrollTop=0)) == 1 + def test_a_feed_with_nothing_to_scroll_is_not_treated_as_at_its_end(self): + """The distinction the room measurement ALONE cannot draw. + + A feed scrolled to its end and a feed that never scrolled both report + zero room, and they are opposite situations. At the end of a long feed + the drag that got there is finished and an upward swipe means unlock + (tsk-36i6ed). On a feed that cannot scroll, an upward drag on a card is + not the end of anything. Telling them apart needs a second reading -- + whether the feed overflows at all -- which is why `feedOverflows()` is + back in the veto. + + It is back as a DISJUNCT, not the conjunct removed with tsk-36i6ed. That + one could not change the answer, because the browser clamps scrollTop to + 0 on a feed that cannot scroll; this one decides this case by itself and + decides no other. `test_the_suite_fails_the_mutation_that_ignores_a_ + dead_feed` is what holds it to that. + """ + assert _drive(_scenario(feedScrollHeight=300, feedScrollTop=0)) == 0 def test_the_end_of_the_feed_is_judged_from_where_the_touch_LANDED(self): """The latch, measured rather than asserted on structure. @@ -338,6 +414,24 @@ def test_the_suite_fails_the_mutation_that_ignores_scroll_position(self): # that discriminates, not simply break everything. assert _drive(_scenario(feedScrollTop=0), ignore_scroll=True) == 0 + def test_the_suite_fails_the_mutation_that_ignores_a_dead_feed(self): + """THE MUTATION CONTROL for Jay's glass bug. + + Drop `|| !feedOverflows()` and the veto is back to asking only how much + room is left -- the code exactly as it was when Jay reported that + swiping an agent island opened the keypad. The non-scrolling case must + go RED under it. + + The two assertions after it are the point. A mutation that breaks + everything proves nothing: it would show only that the suite notices + change, not that these scenarios separate "cannot scroll" from "scrolled + to the end". Both of those stay exactly as they are under the mutation, + so the one case that moves is the one that discriminates. + """ + assert _drive(_scenario(feedScrollHeight=300), ignore_dead_feed=True) == 1 + assert _drive(_scenario(feedScrollTop=600), ignore_dead_feed=True) == 1 + assert _drive(_scenario(feedScrollTop=0), ignore_dead_feed=True) == 0 + class TestGestureLatching: """Properties of `swipe()` itself that the scenarios above rely on.""" diff --git a/tests/test_lock_screen_repaint.py b/tests/test_lock_screen_repaint.py new file mode 100644 index 000000000..20594d371 --- /dev/null +++ b/tests/test_lock_screen_repaint.py @@ -0,0 +1,829 @@ +"""The lock screen's 15s repaint, EXECUTED rather than grepped for. + +Jay, from the glass: "the agent islands flicker occasionally". They did, every +fifteen seconds. `paintActivity()` wiped `#ls-agents` with `textContent = ""` +and appended a freshly built island for every agent, so every island was a NEW +DOM node -- and `.ls-island` carries `ls-island-in`, a 520ms entrance animation +with staggered per-child delays. A new node replays it. Six agents, six +entrances, every poll, whether or not one byte of the payload had changed. + +Measured in a real browser before the fix (chromium at 540x1200, the device's +own CSS viewport, six agents, nothing touched, one poll cycle): six of six +islands fired `animationstart` and the first island was no longer the same node. +After: zero `animationstart`, same node. + +**These tests assert on NODE IDENTITY, not on rendered values.** "The names are +still right" passes on the broken code too -- it was always rebuilding the list +correctly, that was the whole problem. Identity is the only thing that +distinguishes a repaint that flickers from one that does not. + +Like `test_lock_screen_gestures.py`, the real source runs under node against a +DOM stand-in, and `test_harness_observes_the_defect` puts the wipe back and +requires identity to break. Without that control a stub that quietly does +nothing would report every property below as satisfied. +""" +from __future__ import annotations + +import json +import os +import shutil +import subprocess + +import pytest + +from tinyagentos.routes.auth import _LOCK_SCREEN_SCRIPT as LOCK_SCRIPT + +from test_lock_screen_gestures import _balanced, _function + + +def _var(name: str) -> str: + """The source of a `var = ...;` declaration.""" + head = f"var {name} = " + assert head in LOCK_SCRIPT, f"{head!r} is gone from the lock screen script" + i = LOCK_SCRIPT.index(head) + return LOCK_SCRIPT[i : LOCK_SCRIPT.index(";", i) + 1] + + +#: A DOM only as real as island building and reconciliation need. +#: +#: It is deliberately a real tree -- children arrays, insertBefore, remove -- +#: because the properties under test are about WHICH NODES SURVIVE and in what +#: order. A stub that returned plausible values without maintaining a tree could +#: not tell a reconcile from a rebuild, which is the one thing this file exists +#: to tell apart. +_DOM = r""" +var NODE_SEQ = 0; + +function makeNode(tag) { + var el = { + tagName: tag, + __id: ++NODE_SEQ, // identity, which is what the assertions read + className: "", + children: [], + parent: null, + _attrs: {}, + style: { setProperty: function () {} }, + classList: { contains: function () { return false; } }, + innerHTML: "", + _text: "", + addEventListener: function () {}, + focus: function () {}, + setAttribute: function (k, v) { this._attrs[k] = String(v); }, + getAttribute: function (k) { + return Object.prototype.hasOwnProperty.call(this._attrs, k) ? this._attrs[k] : null; + }, + hasAttribute: function (k) { + return Object.prototype.hasOwnProperty.call(this._attrs, k); + }, + removeAttribute: function (k) { delete this._attrs[k]; }, + appendChild: function (c) { + if (c.parent) c.parent.removeChild(c); + c.parent = this; this.children.push(c); return c; + }, + removeChild: function (c) { + var i = this.children.indexOf(c); + if (i !== -1) this.children.splice(i, 1); + c.parent = null; return c; + }, + remove: function () { if (this.parent) this.parent.removeChild(this); }, + insertBefore: function (c, ref) { + if (c.parent) c.parent.removeChild(c); + c.parent = this; + var i = ref ? this.children.indexOf(ref) : -1; + if (i === -1) this.children.push(c); else this.children.splice(i, 0, c); + return c; + }, + // Enough of a selector engine for `.ls-status`, which is what applyAgent + // reaches for. Depth-first, class names only. + querySelector: function (sel) { + var want = sel.replace(/^\./, ""); + for (var i = 0; i < this.children.length; i++) { + var c = this.children[i]; + if (String(c.className).split(/\s+/).indexOf(want) !== -1) return c; + var deep = c.querySelector(sel); + if (deep) return deep; + } + return null; + }, + // `.cls` and `.cls[attr]` -- enough for the notification clock sweep, + // which is the only querySelectorAll the repaint code performs. + querySelectorAll: function (sel) { + var m = /^\.([A-Za-z0-9_-]+)(?:\[([A-Za-z0-9_-]+)\])?$/.exec(sel); + if (!m) throw new Error("harness cannot match selector: " + sel); + var out = []; + (function walk(node) { + for (var i = 0; i < node.children.length; i++) { + var c = node.children[i]; + var hasClass = String(c.className).split(/\s+/).indexOf(m[1]) !== -1; + if (hasClass && (!m[2] || c.hasAttribute(m[2]))) out.push(c); + walk(c); + } + })(this); + return out; + } + }; + // Assigning textContent REMOVES EVERY CHILD -- that is the whole mechanism + // of the wipe these tests exist to detect. Modelled as a plain string + // property it did not, so `notifsEl.textContent = ""` cleared nothing and a + // panel that failed to empty would have looked empty to the harness. + Object.defineProperty(el, "textContent", { + get: function () { return this._text; }, + set: function (v) { + while (this.children.length) this.removeChild(this.children[0]); + this._text = String(v); + } + }); + Object.defineProperty(el, "firstChild", { + get: function () { return this.children.length ? this.children[0] : null; } + }); + Object.defineProperty(el, "lastChild", { + get: function () { + return this.children.length ? this.children[this.children.length - 1] : null; + } + }); + Object.defineProperty(el, "nextSibling", { + get: function () { + if (!this.parent) return null; + var i = this.parent.children.indexOf(this); + return (i === -1 || i + 1 >= this.parent.children.length) + ? null : this.parent.children[i + 1]; + } + }); + return el; +} + +var document = { + createElement: makeNode, + createElementNS: function (_ns, tag) { return makeNode(tag); }, + activeElement: null +}; +var CSS = null; +var window = {}; +""" + +#: The island driver: the shared DOM, the island machinery, and one snapshot +#: of `#ls-agents` per payload. +_HARNESS = _DOM + r""" +var agentsEl = makeNode("div"); + +__SOURCE__ + +// How many ISLANDS were constructed -- not how many DOM nodes, which is a much +// larger and far less interesting number (an island is a dozen elements). The +// real `island()` is wrapped rather than edited, so what runs is still the +// shipped function. +var ISLANDS_BUILT = 0; +var __realIsland = island; +island = function (agent) { ISLANDS_BUILT += 1; return __realIsland(agent); }; + +// Each tick is one payload. After every one, record what the list looks like +// AND the identity of every node in it, so the test can compare across ticks. +var SCN = JSON.parse(process.env.LS_REPAINT); +var snapshots = []; +for (var t = 0; t < SCN.ticks.length; t++) { + __PAINT__(SCN.ticks[t]); + snapshots.push(agentsEl.children.map(function (el) { + return { + id: el.__id, + agent: el.getAttribute("data-agent"), + state: el.getAttribute("data-state"), + attention: el.getAttribute("data-attention"), + label: el.getAttribute("aria-label"), + status: (el.querySelector(".ls-status") || {}).textContent, + record: el.__agent ? el.__agent.name : null + }; + })); +} +process.stdout.write(JSON.stringify({ snapshots: snapshots, built: ISLANDS_BUILT })); +""" + +#: The repaint as it stood before the fix: wipe the container, rebuild each +#: island, append in order. Used only by the control. +_WIPE_AND_REBUILD = r""" +function wipeAndRebuild(agents) { + agentsEl.textContent = ""; + for (var i = 0; i < agents.length; i++) { + agentsEl.appendChild(island(agents[i])); + } +} +""" + + +def _source(*, reconciled: bool = True) -> str: + """The real island machinery, with either repaint strategy wired in.""" + parts = [ + _var("FRAMEWORKS"), + _var("RESTING"), + _function("hueFor"), + _function("initials"), + _function("islandIdentity"), + _function("applyAgent"), + _function("setAttrIfChanged"), + _function("island"), + # The shared reconcile helpers, named here rather than arriving by + # accident: until `_balanced` learned about comments, `_function` + # over-captured and swallowed these along with island(). The suite + # passed for the wrong reason, which is the reason it is spelled out. + _function("placeInOrder"), + _function("partOf"), + _function("setText"), + ] + if reconciled: + parts.append(_function("reconcileIslands")) + else: + parts.append(_WIPE_AND_REBUILD) + return "\n".join(parts) + + +def _paint(ticks: list, *, reconciled: bool = True) -> dict: + """Run a sequence of payloads through the repaint and report each tick.""" + node = shutil.which("node") + if node is None: # pragma: no cover - depends on the runner image + # FAIL, do not skip. A skipped test here reads as green to the shard + # summary while proving nothing at all about the repaint. + pytest.fail( + "node is required to execute the lock screen repaint source, and was " + "not found on PATH. These tests cannot be skipped: skipping them " + "would report green while proving nothing about the flicker." + ) + script = _HARNESS.replace("__SOURCE__", _source(reconciled=reconciled)).replace( + "__PAINT__", "reconcileIslands" if reconciled else "wipeAndRebuild" + ) + done = subprocess.run( + [node, "-e", script], + env={**os.environ, "LS_REPAINT": json.dumps({"ticks": ticks})}, + capture_output=True, + text=True, + timeout=30, + ) + assert done.returncode == 0, f"node failed:\n{done.stderr}" + return json.loads(done.stdout) + + +def _agent(name: str, **over) -> dict: + base = {"name": name, "status": "running", "framework": "", "avatar": ""} + base.update(over) + return base + + +SIX = [ + _agent("taOS Agent", system=True), + _agent("Scout", status="idle"), + _agent("Ledger"), + _agent("Relay"), + _agent("Courier", status="idle"), + _agent("Nightwatch"), +] + + +def _ids(snapshot) -> list: + return [row["id"] for row in snapshot] + + +def _names(snapshot) -> list: + return [row["agent"] for row in snapshot] + + +class TestIslandsSurviveTheRepaint: + """Bug 2: the islands must not be rebuilt by a poll that changed nothing.""" + + def test_an_unchanged_payload_keeps_every_island_node(self): + """THE RED CASE, and Jay's report verbatim. + + Six agents, polled twice, nothing different between the polls. Every + island must be the same DOM node afterwards. On the old code all six + were new nodes and all six replayed their 520ms entrance -- which is + what the flicker was. + """ + out = _paint([SIX, SIX]) + assert _ids(out["snapshots"][0]) == _ids(out["snapshots"][1]) + + def test_an_unchanged_payload_builds_no_new_islands_at_all(self): + """Identity could be preserved by luck if nodes were reused from a pool. + + Counting construction says it outright: the second tick must not build + anything. This is also what makes the assertion above cheap to trust -- + six islands built in total, not twelve. + """ + out = _paint([SIX, SIX]) + assert out["built"] == len(SIX) + + def test_a_changed_status_is_written_into_the_SAME_island(self): + """The fix must update in place, not swap the element for a new one. + + This is the assertion that would fail on a "fix" that compared payloads + and skipped the repaint entirely: skipping keeps identity and drops the + update, so identity and freshness are asserted together, in one tick. + """ + second = [dict(a) for a in SIX] + second[1] = _agent("Scout", status="thinking") + out = _paint([SIX, second]) + before, after = out["snapshots"] + assert _ids(before) == _ids(after), "the island was replaced, not updated" + assert after[1]["status"] == "thinking" + assert after[1]["state"] == "busy", "a working agent must read as busy" + assert "thinking" in after[1]["label"], "the accessible name went stale" + + def test_the_record_the_handlers_read_is_refreshed_in_place(self): + """`el.__agent` is what a press opens, and it is not visible. + + The island's handlers read the whole record off the element rather than + looking it up by name. Reconciling without refreshing it would leave the + screen correct and the SHEET stale -- opening an agent's conversation on + fifteen-second-old state, which nothing on the glass would reveal. + """ + second = [dict(a) for a in SIX] + second[2] = _agent("Ledger", status="waiting") + out = _paint([SIX, second]) + assert out["snapshots"][1][2]["record"] == "Ledger" + assert out["snapshots"][1][2]["status"] == "waiting" + + def test_attention_is_cleared_when_it_goes_away(self): + """Attributes that are ADDED must also be REMOVED. + + A reconcile writes over what it finds, so a flag set on one tick and + absent from the next survives unless it is explicitly cleared. The wipe + got this right for free -- it threw the element away -- so it is exactly + the class of regression that switching to reconciliation introduces. + """ + first = [dict(a) for a in SIX] + first[3] = _agent("Relay", attention=True, + decision={"question": "deploy to prod?"}) + out = _paint([first, SIX]) + assert out["snapshots"][0][3]["attention"] == "1" + assert out["snapshots"][1][3]["attention"] is None + assert "needs a decision" not in out["snapshots"][1][3]["label"] + + def test_a_new_agent_is_added_without_disturbing_the_others(self): + out = _paint([SIX, SIX + [_agent("Sentry")]]) + before, after = out["snapshots"] + assert _names(after)[-1] == "Sentry" + assert _ids(after)[: len(SIX)] == _ids(before), "the existing islands were rebuilt" + assert out["built"] == len(SIX) + 1 + + def test_a_departed_agent_is_removed_without_disturbing_the_others(self): + fewer = [a for a in SIX if a["name"] != "Ledger"] + out = _paint([SIX, fewer]) + before, after = out["snapshots"] + assert "Ledger" not in _names(after) + assert _ids(after) == [i for i, r in zip(_ids(before), before) + if r["agent"] != "Ledger"] + + def test_a_reordered_payload_moves_islands_without_rebuilding_them(self): + """Order follows the payload, and a move is not a rebuild. + + `insertBefore` on a node already in position would re-insert it and + restart its animation, so the reconcile has to check before it moves. + Reordering is the case where that check is load-bearing. + """ + flipped = list(reversed(SIX)) + out = _paint([SIX, flipped]) + before, after = out["snapshots"] + assert _names(after) == [a["name"] for a in flipped] + assert sorted(_ids(after)) == sorted(_ids(before)), "islands were rebuilt to reorder" + assert out["built"] == len(SIX) + + def test_a_reconfigured_agent_IS_rebuilt(self): + """The deliberate exception, so it is a decision and not an oversight. + + An avatar or a framework badge is built once and never mutated, so when + one of those changes the element really is wrong and rebuilding it is + the honest answer. It comes from configuration, not from a tick, so it + does not flicker in practice. + """ + second = [dict(a) for a in SIX] + second[4] = _agent("Courier", status="idle", avatar="/static/courier.png") + out = _paint([SIX, second]) + before, after = out["snapshots"] + assert _ids(after)[4] != _ids(before)[4], "the reconfigured island was not rebuilt" + assert _ids(after)[:4] == _ids(before)[:4], "its neighbours were rebuilt too" + + def test_harness_observes_the_defect(self): + """THE CONTROL. Put the wipe back and identity must break. + + Every assertion above is about nodes surviving. If this stand-in ever + stopped maintaining a real tree, "the nodes survived" and "nothing + happened at all" would read identically and the whole file would pass + while measuring nothing. Under the old wipe-and-rebuild, the same + unchanged payload must produce six entirely new islands. + """ + out = _paint([SIX, SIX], reconciled=False) + before, after = out["snapshots"] + assert _names(after) == _names(before), "the old code did render the right names" + assert not set(_ids(after)) & set(_ids(before)), "no island should have survived" + assert out["built"] == 2 * len(SIX) + + +# --------------------------------------------------------------------------- +# BUG 2b: the same disease in the pollers Jay reported next. +# +# "the system stats widget flickers too" -- said in the same breath as the +# islands, and it is the same defect one view over. `paintStats()` wiped +# `#ls-stats` and rebuilt the card, and `.ls-stat-card` carries the very same +# 520ms `ls-island-in` entrance. The difference is the cadence: the stats poll +# runs every THREE seconds, not fifteen. +# +# Measured in chromium at 540x1200 before the fix, stats view, nothing touched: +# two `ls-island-in` replays in 7.5s (2992ms apart -- the poll), and the card +# was a different, DETACHED node each time. `.ls-stat-fill` was new each time +# too, so its `transition: width 420ms` never ran and the meters snapped. +# --------------------------------------------------------------------------- + +_STATS_HARNESS = _DOM + r""" +var statsEl = makeNode("div"); +function syncFeedFade() {} + +__SOURCE__ + +// Restores the wipe, and nothing else. partOf() can then find nothing, so +// every part is rebuilt -- which is precisely the old behaviour. +var __shippedPaintStats = paintStats; +function wipeAndPaintStats(d) { + statsEl.textContent = ""; + __shippedPaintStats(d); +} + +function snap(el) { + return { + id: el.__id, + part: el.getAttribute("data-part"), + cls: el.className, + text: el.textContent, + width: el.style && el.style.width ? el.style.width : null, + kids: el.children.map(snap) + }; +} + +var SCN = JSON.parse(process.env.LS_STATS); +var snapshots = []; +for (var t = 0; t < SCN.ticks.length; t++) { + __PAINT__(SCN.ticks[t]); + snapshots.push(statsEl.children.map(snap)); +} +process.stdout.write(JSON.stringify({ snapshots: snapshots })); +""" + + +def _stats_source() -> str: + return "\n".join([ + _function("placeInOrder"), + _function("partOf"), + _function("setText"), + _function("statRow"), + _function("statNote"), + _function("gib"), + _function("paintStats"), + ]) + + +def _paint_stats(ticks: list, *, reconciled: bool = True) -> dict: + node = shutil.which("node") + if node is None: # pragma: no cover - depends on the runner image + pytest.fail( + "node is required to execute the lock screen stats repaint, and was " + "not found on PATH. Skipping would report green while proving " + "nothing about the flicker Jay reported." + ) + script = _STATS_HARNESS.replace("__SOURCE__", _stats_source()).replace( + "__PAINT__", "paintStats" if reconciled else "wipeAndPaintStats" + ) + done = subprocess.run( + [node, "-e", script], + env={**os.environ, "LS_STATS": json.dumps({"ticks": ticks})}, + capture_output=True, + text=True, + timeout=30, + ) + assert done.returncode == 0, f"node failed:\n{done.stderr}" + return json.loads(done.stdout) + + +def _reading(cpu=41.0, mem_pct=63.0, dsps=("adsp", "cdsp"), gpu=True) -> dict: + d = { + "cpu_percent": cpu, + "cpu_cores": 8, + "memory": {"used_kb": 5033164, "total_kb": 7902168, "percent": mem_pct}, + "models": ["qwen2.5:3b"], + "dsps": [{"name": n, "state": "running"} for n in dsps], + } + if gpu: + d["gpu"] = {"freq_hz": 305000000, "max_freq_hz": 812000000, + "active_percent": 12.5} + return d + + +def _find(rows, part): + for row in rows: + if row["part"] == part: + return row + return None + + +def _card(snapshot): + return _find(snapshot, "card") + + +class TestStatsSurviveTheRepaint: + """Node identity across a poll -- not "the numbers are right". + + The broken code rendered the right numbers too. It rendered them into a + brand new card every three seconds, which is the whole complaint. + """ + + def test_an_unchanged_reading_keeps_the_stats_card(self): + before, after = _paint_stats([_reading(), _reading()])["snapshots"] + assert _card(after)["id"] == _card(before)["id"], ( + "the stats card was rebuilt, so it replays its 520ms entrance" + ) + + def test_a_CHANGED_reading_still_keeps_the_stats_card(self): + """The poll exists to deliver new numbers; new numbers are the norm. + + A fix that only held still for an identical payload would flicker on + every real device, where the CPU percentage moves every single tick. + """ + before, after = _paint_stats([_reading(cpu=41.0), _reading(cpu=78.0)])["snapshots"] + assert _card(after)["id"] == _card(before)["id"] + cpu = _find(_card(after)["kids"], "cpu") + assert cpu["kids"][0]["kids"][1]["text"] == "78%", "the new reading must land" + + def test_every_stat_row_keeps_its_node_across_a_changed_reading(self): + before, after = _paint_stats([_reading(cpu=41.0), _reading(cpu=78.0)])["snapshots"] + for part in ("cpu", "memory", "gpu"): + assert _find(_card(after)["kids"], part)["id"] == \ + _find(_card(before)["kids"], part)["id"], f"{part} row was rebuilt" + + def test_the_meter_keeps_its_node_so_its_width_can_animate(self): + """`.ls-stat-fill` has `transition: width 420ms`. + + A transition needs a FROM value, which only a surviving node has. A + fresh node starts at its final width and the bar snaps there -- which + is what every meter did before this fix. + """ + before, after = _paint_stats([_reading(cpu=10.0), _reading(cpu=90.0)])["snapshots"] + fill_before = _find(_card(before)["kids"], "cpu")["kids"][1]["kids"][0] + fill_after = _find(_card(after)["kids"], "cpu")["kids"][1]["kids"][0] + assert fill_after["id"] == fill_before["id"], "the meter was rebuilt" + assert fill_before["width"] == "10%" and fill_after["width"] == "90%" + + def test_a_reading_that_stops_being_measured_loses_its_bar(self): + """A meter left at its last value goes on reporting a dead measurement. + + This is the same trap as a check that passes because it measured + nothing: an idle 0% and an unmeasured 0% must not look alike, and a + STALE 63% must not look like a live one. + """ + ticks = [_reading(mem_pct=63.0), _reading()] + ticks[1]["memory"] = None + before, after = _paint_stats(ticks)["snapshots"] + assert len(_find(_card(before)["kids"], "memory")["kids"]) == 2, "bar expected" + mem = _find(_card(after)["kids"], "memory") + assert len(mem["kids"]) == 1, "the track must go when the number does" + assert mem["kids"][0]["kids"][1]["text"] == "--" + + def test_chips_are_keyed_so_a_steady_accelerator_is_not_rebuilt(self): + before, after = _paint_stats([_reading(), _reading()])["snapshots"] + chips_before = _find(before, "chips") + chips_after = _find(after, "chips") + assert chips_after["id"] == chips_before["id"] + assert [c["id"] for c in chips_after["kids"]] == \ + [c["id"] for c in chips_before["kids"]] + assert [c["part"] for c in chips_after["kids"]] == ["adsp", "cdsp"] + + def test_accelerators_that_go_away_take_their_caption_with_them(self): + before, after = _paint_stats( + [_reading(dsps=("adsp", "cdsp")), _reading(dsps=())])["snapshots"] + assert _find(before, "chips") is not None + assert _find(after, "chips") is None, "chips must not outlive their DSPs" + assert _find(after, "accel-note") is None, "nor may their caption" + assert _find(after, "models") is not None, "the rest of the panel stays" + + def test_the_gpu_note_appears_and_disappears_without_rebuilding_the_card(self): + ticks = [_reading(), _reading(gpu=False)] + before, after = _paint_stats(ticks)["snapshots"] + assert _find(_card(before)["kids"], "gpu-note") is not None + assert _find(_card(after)["kids"], "gpu-note") is None + assert _card(after)["id"] == _card(before)["id"] + assert _find(_card(after)["kids"], "gpu")["kids"][0]["kids"][1]["text"] == "--" + + def test_the_shipped_source_no_longer_wipes_the_panel(self): + """The control below is only a mutation while this is true. + + If the wipe ever comes back to `paintStats`, `wipeAndPaintStats` stops + changing anything and the control would pass by doing nothing at all. + """ + shipped = _function("paintStats") + assert 'statsEl.textContent = ""' not in shipped, ( + "the wholesale wipe is back in paintStats, which makes the control " + "below a no-op: it would 'prove' the defect is observable while " + "testing identical code" + ) + + def test_harness_observes_the_defect(self): + """Restore the wipe and every property above must break. + + Without this the suite could not tell a reconcile from a stub that + quietly did nothing. + """ + out = _paint_stats([_reading(), _reading()], reconciled=False) + before, after = out["snapshots"] + # It still renders correctly -- that was never the problem. + assert [p["part"] for p in after] == [p["part"] for p in before] + assert _card(after)["id"] != _card(before)["id"], ( + "the wipe must produce a new card, or it is not the old behaviour" + ) + assert not {k["id"] for k in _card(after)["kids"]} & \ + {k["id"] for k in _card(before)["kids"]}, "no row should have survived" + + +# --------------------------------------------------------------------------- +# BUG 2b, second poller: the notification stacks. +# +# `.ls-notif-group` carries the same 520ms `ls-island-in` entrance as an +# island and a stats card, and `paintNotifications()` wiped `#ls-notifs` and +# rebuilt every stack. This poll runs every fifteen MINUTES, so it is the +# least often seen of the three -- but a stack is also a `role="button"` with +# a tabindex, so the wipe threw away keyboard focus as well as animating. +# +# A stack whose notifications genuinely changed SHOULD animate: that is new +# content arriving. One that did not change must not move at all. +# --------------------------------------------------------------------------- + +_NOTIF_HARNESS = _DOM + r""" +var notifsEl = makeNode("div"); +var screenEl = makeNode("div"); +screenEl.setAttribute("data-sheet", "none"); +function syncFeedFade() {} + +__SOURCE__ + +var __shippedPaint = paintNotifications; +function wipeAndPaintNotifications(d) { + notifsEl.textContent = ""; + __shippedPaint(d); +} + +function snap(el) { + return { + id: el.__id, + source: el.getAttribute("data-source"), + identity: el.getAttribute("data-identity"), + open: el.getAttribute("data-open"), + kids: el.children.map(function (c) { return c.__id; }) + }; +} + +var SCN = JSON.parse(process.env.LS_NOTIFS); +var snapshots = []; +var clocks = []; +for (var t = 0; t < SCN.ticks.length; t++) { + __PAINT__(SCN.ticks[t]); + snapshots.push(notifsEl.children.map(snap)); + clocks.push(notifClocks.map(function (c) { return { id: c.el.__id, at: c.at }; })); +} +process.stdout.write(JSON.stringify({ + snapshots: snapshots, clocks: clocks, hidden: !!notifsEl.hidden +})); +""" + + +def _notif_source() -> str: + return "\n".join([ + _var("NOTIF_GLYPHS"), + _var("notifOpen"), + _var("notifClocks"), + _function("placeInOrder"), + _function("partOf"), + _function("setText"), + _function("whenText"), + _function("notifCard"), + _function("notifGroup"), + _function("notifIdentity"), + _function("paintNotifications"), + ]) + + +def _paint_notifs(ticks: list, *, reconciled: bool = True) -> dict: + node = shutil.which("node") + if node is None: # pragma: no cover - depends on the runner image + pytest.fail( + "node is required to execute the notification repaint, and was not " + "found on PATH. Skipping would report green while proving nothing." + ) + script = _NOTIF_HARNESS.replace("__SOURCE__", _notif_source()).replace( + "__PAINT__", "paintNotifications" if reconciled else "wipeAndPaintNotifications" + ) + done = subprocess.run( + [node, "-e", script], + env={**os.environ, "LS_NOTIFS": json.dumps({"ticks": ticks})}, + capture_output=True, + text=True, + timeout=30, + ) + assert done.returncode == 0, f"node failed:\n{done.stderr}" + return json.loads(done.stdout) + + +def _group(source, app, *items): + return { + "source": source, + "app": app, + "mono": app[:2], + "glyph": "", + "items": [ + {"at": 1789580000 - (i * 600), "title": t, "text": "body " + t} + for i, t in enumerate(items) + ], + } + + +def _stacks(**over): + base = [ + _group("mail", "Mail", "Invoice 4021", "Re: shipping"), + _group("sms", "Messages", "On my way"), + ] + base[0].update(over.get("mail", {})) + return {"groups": base} + + +def _by_source(snapshot, source): + for row in snapshot: + if row["source"] == source: + return row + return None + + +class TestNotificationStacksSurviveTheRepaint: + + def test_an_unchanged_payload_keeps_every_stack_node(self): + before, after = _paint_notifs([_stacks(), _stacks()])["snapshots"] + assert [r["id"] for r in after] == [r["id"] for r in before], ( + "a stack was rebuilt, so it replays its 520ms entrance and drops focus" + ) + + def test_an_unchanged_payload_keeps_the_CARDS_inside_a_stack(self): + before, after = _paint_notifs([_stacks(), _stacks()])["snapshots"] + assert _by_source(after, "mail")["kids"] == _by_source(before, "mail")["kids"] + + def test_a_stack_whose_notifications_CHANGED_is_rebuilt(self): + """New content is exactly what the entrance animation is for. + + The property is not "never rebuild"; it is "rebuild only what changed". + """ + first = _stacks() + second = _stacks(mail={"items": _group( + "mail", "Mail", "Invoice 4021", "Re: shipping", "New thing")["items"]}) + before, after = _paint_notifs([first, second])["snapshots"] + assert _by_source(after, "mail")["id"] != _by_source(before, "mail")["id"] + # ...and its untouched neighbour must NOT be dragged along with it. + assert _by_source(after, "sms")["id"] == _by_source(before, "sms")["id"] + + def test_a_departed_stack_is_removed_without_disturbing_the_others(self): + second = {"groups": [_group("sms", "Messages", "On my way")]} + before, after = _paint_notifs([_stacks(), second])["snapshots"] + assert _by_source(after, "mail") is None + assert _by_source(after, "sms")["id"] == _by_source(before, "sms")["id"] + + def test_a_reordered_payload_moves_stacks_without_rebuilding_them(self): + first = _stacks() + second = {"groups": [first["groups"][1], first["groups"][0]]} + before, after = _paint_notifs([first, second])["snapshots"] + assert [r["source"] for r in after] == ["sms", "mail"] + assert {r["id"] for r in after} == {r["id"] for r in before} + + def test_the_minute_labels_are_collected_from_what_is_on_screen(self): + """The clock list must cover stacks the paint deliberately left alone. + + Before the fix it was whatever `notifCard` happened to push while + building. Once a stack is NOT rebuilt nothing is pushed for it, so a + list built that way would silently stop retouching its minutes -- and + a clock frozen at "2h ago" reads exactly like a working one. + """ + out = _paint_notifs([_stacks(), _stacks()]) + first_tick, second_tick = out["clocks"] + assert len(second_tick) == 3, "three notifications, three clocks" + assert [c["id"] for c in second_tick] == [c["id"] for c in first_tick] + assert all(isinstance(c["at"], int) and c["at"] > 0 for c in second_tick) + + def test_an_empty_payload_clears_the_stack_and_its_clocks(self): + out = _paint_notifs([_stacks(), {"groups": []}]) + assert out["snapshots"][1] == [] + assert out["clocks"][1] == [] + assert out["hidden"] is True + + def test_the_shipped_source_reconciles_rather_than_wipes(self): + """Keeps the control below honest: if reconciliation is removed, this + says so in one line instead of leaving the control silently comparing + identical code against itself.""" + shipped = _function("paintNotifications") + assert "placeInOrder(notifsEl, want)" in shipped + + def test_harness_observes_the_defect(self): + out = _paint_notifs([_stacks(), _stacks()], reconciled=False) + before, after = out["snapshots"] + assert [r["source"] for r in after] == [r["source"] for r in before], ( + "the old code did render the right stacks" + ) + assert not {r["id"] for r in after} & {r["id"] for r in before}, ( + "the wipe must rebuild every stack, or it is not the old behaviour" + ) diff --git a/tests/test_onscreen_keyboard.py b/tests/test_onscreen_keyboard.py index ade79cc5c..84f8aee8b 100644 --- a/tests/test_onscreen_keyboard.py +++ b/tests/test_onscreen_keyboard.py @@ -738,7 +738,7 @@ def test_the_fade_is_measured_not_assumed(self, login_console): class TestTheIslandRepaintKeepsKeyboardFocus: - """The islands poll every 15 seconds and paintActivity rebuilds the list. + """The islands poll every 15 seconds and paintActivity repaints the list. Measured on the handset over CDP before this was fixed: focus an island, wait 17s, and document.activeElement had fallen back to the lock screen @@ -756,19 +756,27 @@ def _paint_activity(self): end = LOCK_SCRIPT.index("function pollActivity(", start) return LOCK_SCRIPT[start:end] - def test_focus_is_captured_before_the_wipe_and_restored_after_the_rebuild(self): + def test_focus_is_captured_before_the_repaint_and_restored_after_it(self): """Ordering is the whole assertion. - Reading activeElement AFTER `agentsEl.textContent = ""` reads the body, - because the wipe is what moved focus there -- so a capture in the wrong - place records nothing and restores nothing while looking correct. + This used to be expressed against `agentsEl.textContent = ""`, because + the wipe was what moved focus to the body. The wipe is gone: the list is + now reconciled in place (see `test_lock_screen_repaint.py`), so a + persisting island is the same DOM node afterwards and NEVER loses focus + in the first place -- a stronger guarantee than restoring it. + + The capture and restore stay, and stay in this order, for the case + reconciliation cannot cover: an island whose avatar or framework changed + is genuinely rebuilt, and an agent that goes away takes its element with + it. Reading activeElement after the repaint would read whatever those + cases left behind. """ body = self._paint_activity() capture = body.index("document.activeElement") - wipe = body.index('agentsEl.textContent = ""') + repaint = body.index("reconcileIslands(agents)") restore = body.index(".focus()") - assert capture < wipe, "focus must be read BEFORE the list is wiped" - assert wipe < restore, "focus must be restored AFTER the list is rebuilt" + assert capture < repaint, "focus must be read BEFORE the list is repainted" + assert repaint < restore, "focus must be restored AFTER the list is repainted" def test_the_island_is_found_again_by_a_stable_key_not_by_position(self): """Restoring by index moves focus to a DIFFERENT agent whenever the list diff --git a/tinyagentos/routes/auth.py b/tinyagentos/routes/auth.py index e744d3d0b..deade7592 100644 --- a/tinyagentos/routes/auth.py +++ b/tinyagentos/routes/auth.py @@ -1880,6 +1880,49 @@ def _lock_tail_html() -> str: var FRAMEWORKS = { hermes: 1, openclaw: 1, deepseek: 1, omp: 1 }; var RESTING = ["", "stopped", "idle", "exited", "error"]; + // An island's IMMUTABLE half, as a string. Everything here comes from + // configuration rather than from runtime state, so a change is a + // reconfiguration and not a tick: rare enough to rebuild the element for, + // and not worth the code to mutate an avatar or a framework badge in place. + // The agent's NAME is not in here because the name is the key itself. + // + // JSON.stringify rather than a joined string: an avatar is a URL and a + // framework name is user-supplied, so any separator character I picked + // could appear inside a value and make two different agents compare equal. + function islandIdentity(agent) { + return JSON.stringify([ + agent.avatar || "", + agent.framework_icon || "", + String(agent.framework || "").toLowerCase(), + agent.system ? "1" : "", + ]); + } + + // An island's MUTABLE half: everything a 15s poll can legitimately change. + // Written in place, and only where the value actually differs, so that a + // tick which changes nothing touches nothing. + function applyAgent(el, agent) { + var name = agent.name || "agent"; + var status = agent.status || "idle"; + var busy = RESTING.indexOf(status.trim().toLowerCase()) === -1; + // The handlers read the whole record off the element rather than + // re-looking it up by name, so this has to be refreshed even when every + // visible field is unchanged. + el.__agent = agent; + setAttrIfChanged(el, "data-state", busy ? "busy" : "idle"); + if (agent.attention) setAttrIfChanged(el, "data-attention", "1"); + else if (el.hasAttribute("data-attention")) el.removeAttribute("data-attention"); + setAttrIfChanged(el, "aria-label", (agent.attention && agent.decision) + ? name + " needs a decision: " + (agent.decision.question || "") + : name + ", " + status + ". Open conversation."); + var s = el.querySelector(".ls-status"); + if (s && s.textContent !== status) s.textContent = status; + } + + function setAttrIfChanged(el, attr, value) { + if (el.getAttribute(attr) !== value) el.setAttribute(attr, value); + } + function island(agent) { var name = agent.name || "agent"; var status = agent.status || "idle"; @@ -1887,6 +1930,7 @@ def _lock_tail_html() -> str: var el = document.createElement("div"); el.className = "ls-island"; + el.setAttribute("data-identity", islandIdentity(agent)); // Stable identity across repaints. The 15s poll rebuilds this list, and // without a key there is no way to put keyboard focus back on the island // the user was actually on -- an index would silently move the focus to a @@ -1989,6 +2033,97 @@ def _lock_tail_html() -> str: return el; } + // Bring the island list to match `agents` by CHANGING it, never by + // rebuilding it. + // + // The old code did `agentsEl.textContent = ""` and appended six fresh + // elements every 15 seconds. Every one of them was a new node, so every one + // of them replayed `ls-island-in` -- a 520ms entrance animation with + // staggered per-child delays. On the glass that is the whole list blinking + // every fifteen seconds, which is what Jay reported, and it happened + // whether or not a single byte of the payload had changed. + // + // Measured before the fix, over one poll with nothing touched: six of six + // islands fired `animationstart`, and the first island was no longer the + // same DOM node. Both of those are what the test asserts, because "the + // names are still right" would have passed on the broken code too. + // + // Keyed by agent name: that is already the identity this list uses for + // focus restoration, and an index would move an agent's island under the + // user's finger the moment the list reordered. + // Put exactly `els` inside `parent`, in this order, and remove whatever + // else is in there -- WITHOUT touching an element that is already in + // position. Re-inserting a node restarts its CSS animation, so a blind + // appendChild of every child in order flickers exactly as badly as the + // wipe it replaces. Every keyed list on this screen goes through here. + function placeInOrder(parent, els) { + var prev = null; + for (var i = 0; i < els.length; i++) { + var want = prev ? prev.nextSibling : parent.firstChild; + if (els[i] !== want) parent.insertBefore(els[i], want); + prev = els[i]; + } + // Anything past the last wanted element is no longer in the payload. + while (prev ? prev.nextSibling : parent.firstChild) { + parent.removeChild(prev ? prev.nextSibling : parent.firstChild); + } + } + + // The child of `parent` carrying this key, created once if it is not + // there yet. A new element comes back DETACHED -- placeInOrder puts it + // where the payload says it goes. + function partOf(parent, key, cls, tag) { + var kids = parent.children; + for (var i = 0; i < kids.length; i++) { + if (kids[i].getAttribute("data-part") === key) return kids[i]; + } + var el = document.createElement(tag || "div"); + el.className = cls; + el.setAttribute("data-part", key); + return el; + } + + // Only write text that actually changed: assigning textContent replaces + // the text node even when the string is identical. + function setText(el, text) { + if (el.textContent !== text) el.textContent = text; + return el; + } + + function reconcileIslands(agents) { + var existing = {}; + var kids = agentsEl.children; + for (var i = 0; i < kids.length; i++) { + var key = kids[i].getAttribute("data-agent"); + if (key !== null) existing[key] = kids[i]; + } + var want = []; + for (var j = 0; j < agents.length; j++) { + var agent = agents[j]; + var name = agent.name || "agent"; + var el = Object.prototype.hasOwnProperty.call(existing, name) + ? existing[name] : null; + // A reconfigured agent -- new portrait, different framework -- is the + // one case where the element itself is wrong rather than merely stale. + if (el && el.getAttribute("data-identity") !== islandIdentity(agent)) { + el.remove(); + el = null; + } + if (el) { + applyAgent(el, agent); + } else { + el = island(agent); + } + // Claimed: a second agent sharing this name gets its own island + // rather than the two of them fighting over one element. + delete existing[name]; + want.push(el); + } + // Whatever the payload no longer lists has genuinely gone away, and + // placeInOrder drops it. + placeInOrder(agentsEl, want); + } + function paintActivity(data) { // A repaint while a sheet is open would destroy the very island the sheet // was opened from -- dropping its record, restarting every entrance @@ -2020,15 +2155,12 @@ def _lock_tail_html() -> str: } } - agentsEl.textContent = ""; tasksEl.textContent = ""; var agents = data.agents || []; var tasks = data.tasks || []; if (!agents.length && !tasks.length) { card.hidden = true; return; } - for (var i = 0; i < agents.length; i++) { - agentsEl.appendChild(island(agents[i])); - } + reconcileIslands(agents); for (var j = 0; j < tasks.length; j++) { var row = document.createElement("div"); row.className = "ls-task"; @@ -2296,60 +2428,87 @@ def _lock_tail_html() -> str: .catch(function () { /* a lock screen does not show network errors */ }); } - function statRow(label, value, pct) { - var row = document.createElement("div"); - row.className = "ls-stat"; - var top = document.createElement("div"); - top.className = "ls-stat-top"; - var l = document.createElement("span"); - l.className = "ls-stat-label"; - l.textContent = label; - var v = document.createElement("span"); - v.className = "ls-stat-value"; - v.textContent = value; - top.appendChild(l); top.appendChild(v); - row.appendChild(top); + // A stat row is built once and thereafter only its numbers change. + // + // The old code built a fresh one every poll, inside a freshly built + // `.ls-stat-card`, after `statsEl.textContent = ""`. That is the islands' + // disease one view over, except the stats poll runs every THREE seconds, + // not fifteen, and `.ls-stat-card` carries the same 520ms `ls-island-in` + // entrance. Measured in chromium at 540x1200 before the fix: two entrance + // replays in 7.5s, and the card was a different, detached node each time. + // That is the "the system stats widget flickers too" Jay reported in the + // same breath as the islands. + // + // Keeping the row also makes `.ls-stat-fill`'s `transition: width 420ms` + // mean something. A brand-new node has no previous width to travel from, + // so every meter SNAPPED to its reading; kept in place, they glide. + function statRow(parent, key, label, value, pct) { + var row = partOf(parent, key, "ls-stat"); + var top = row.firstChild; + if (!top) { + top = document.createElement("div"); + top.className = "ls-stat-top"; + var l = document.createElement("span"); + l.className = "ls-stat-label"; + var v = document.createElement("span"); + v.className = "ls-stat-value"; + top.appendChild(l); top.appendChild(v); + row.appendChild(top); + } + setText(top.firstChild, label); + setText(top.lastChild, value); // A bar ONLY when there is a real percentage behind it. A meter drawn at // zero because nothing was measured looks exactly like a meter drawn at - // zero because the thing is idle. + // zero because the thing is idle -- and a meter LEFT at its last reading + // once the readings stop is worse still, because it goes on reporting a + // measurement nobody is making. So the track goes when the number does. + var track = row.lastChild === top ? null : row.lastChild; if (typeof pct === "number") { - var track = document.createElement("div"); - track.className = "ls-stat-track"; - var fill = document.createElement("div"); - fill.className = "ls-stat-fill"; - fill.style.width = Math.max(0, Math.min(100, pct)) + "%"; - track.appendChild(fill); - row.appendChild(track); + if (!track) { + track = document.createElement("div"); + track.className = "ls-stat-track"; + var fill = document.createElement("div"); + fill.className = "ls-stat-fill"; + track.appendChild(fill); + row.appendChild(track); + } + track.firstChild.style.width = Math.max(0, Math.min(100, pct)) + "%"; + } else if (track) { + row.removeChild(track); } return row; } + function statNote(parent, key, text) { + return setText(partOf(parent, key, "ls-stat-note"), text); + } + function gib(kb) { return (kb / 1048576).toFixed(1) + " GB"; } function paintStats(d) { if (!statsEl) return; - statsEl.textContent = ""; // Same as the placeholders: `hidden` meant "empty", and it no longer is. statsEl.hidden = false; - var card = document.createElement("div"); - card.className = "ls-stat-card"; + var parts = []; + var card = partOf(statsEl, "card", "ls-stat-card"); + var rows = []; var cores = d.cpu_cores ? " · " + d.cpu_cores + " cores" : ""; - card.appendChild(statRow( + rows.push(statRow(card, "cpu", "CPU" + cores, typeof d.cpu_percent === "number" ? d.cpu_percent.toFixed(0) + "%" : "--", typeof d.cpu_percent === "number" ? d.cpu_percent : null )); if (d.memory) { - card.appendChild(statRow( + rows.push(statRow(card, "memory", "Memory", gib(d.memory.used_kb) + " / " + gib(d.memory.total_kb), d.memory.percent )); } else { - card.appendChild(statRow("Memory", "--")); + rows.push(statRow(card, "memory", "Memory", "--")); } // GPU: LABELLED AS FREQUENCY, because that is what it is. The bar is the @@ -2358,52 +2517,49 @@ def _lock_tail_html() -> str: if (d.gpu && d.gpu.freq_hz) { var mhz = Math.round(d.gpu.freq_hz / 1000000); var maxMhz = d.gpu.max_freq_hz ? Math.round(d.gpu.max_freq_hz / 1000000) : 0; - card.appendChild(statRow( + rows.push(statRow(card, "gpu", "GPU clock", maxMhz ? mhz + " / " + maxMhz + " MHz" : mhz + " MHz", maxMhz ? (mhz * 100 / maxMhz) : null )); if (typeof d.gpu.active_percent === "number") { - var note = document.createElement("div"); - note.className = "ls-stat-note"; - note.textContent = "Above idle clock " + d.gpu.active_percent.toFixed(0) - + "% of uptime. The GPU reports no utilisation counter."; - card.appendChild(note); + rows.push(statNote(card, "gpu-note", + "Above idle clock " + d.gpu.active_percent.toFixed(0) + + "% of uptime. The GPU reports no utilisation counter.")); } } else { - card.appendChild(statRow("GPU clock", "--")); + rows.push(statRow(card, "gpu", "GPU clock", "--")); } - statsEl.appendChild(card); + placeInOrder(card, rows); + parts.push(card); // The remote processors, as state chips. This is where the NPU lives, and // running/offline is genuinely all it reports. if (d.dsps && d.dsps.length) { - var chips = document.createElement("div"); - chips.className = "ls-chips"; + var chips = partOf(statsEl, "chips", "ls-chips"); + var want = []; for (var i = 0; i < d.dsps.length; i++) { - var c = document.createElement("span"); - c.className = "ls-chip"; - var up = String(d.dsps[i].state || "") === "running"; - c.setAttribute("data-on", up ? "1" : "0"); - c.textContent = (d.dsps[i].name || "dsp") + " · " + (d.dsps[i].state || "unknown"); - chips.appendChild(c); + var name = d.dsps[i].name || "dsp"; + var state = d.dsps[i].state || "unknown"; + var c = partOf(chips, name, "ls-chip", "span"); + c.setAttribute("data-on", String(d.dsps[i].state || "") === "running" ? "1" : "0"); + want.push(setText(c, name + " · " + state)); } - statsEl.appendChild(chips); - var why = document.createElement("div"); - why.className = "ls-stat-note"; - why.textContent = "Accelerators report running or offline only — no usage counter exists for them."; - statsEl.appendChild(why); + placeInOrder(chips, want); + parts.push(chips); + parts.push(statNote(statsEl, "accel-note", + "Accelerators report running or offline only — no usage counter exists for them.")); } + // With no DSPs the chips and their caption are simply absent from + // `parts`, and placeInOrder takes them out. // "Nobody asked" and "none loaded" are different answers. - var models = document.createElement("div"); - models.className = "ls-stat-note"; - models.textContent = d.models + parts.push(statNote(statsEl, "models", d.models ? (d.models.length ? d.models.join(", ") : "No models loaded.") - : "Loaded models are not reported by this device."; - statsEl.appendChild(models); + : "Loaded models are not reported by this device.")); + placeInOrder(statsEl, parts); syncFeedFade(); } @@ -2527,7 +2683,9 @@ def _lock_tail_html() -> str: var when = document.createElement("span"); when.className = "ls-notif-when"; when.textContent = whenText(item.at); - notifClocks.push({ el: when, at: item.at }); + // The paint collects these from the DOM afterwards, so a stack it left + // untouched still gets its minutes retouched. + when.setAttribute("data-at", item.at); meta.appendChild(when); var title = document.createElement("div"); @@ -2583,19 +2741,75 @@ def _lock_tail_html() -> str: return el; } + // What a stack is CURRENTLY showing. Two payloads with the same signature + // are the same notifications, so the stack on screen is already right and + // must not be touched. + function notifIdentity(group) { + var parts = [group.app || "", group.glyph || "", group.mono || ""]; + for (var i = 0; i < group.items.length; i++) { + var it = group.items[i]; + parts.push(String(it.at) + "" + (it.title || "") + + "" + (it.text || "")); + } + return parts.join(""); + } + function paintNotifications(data) { if (!notifsEl) return; // Same rule as the islands: never rebuild under an open sheet. var sheetNow = screenEl ? screenEl.getAttribute("data-sheet") : "none"; if (sheetNow && sheetNow !== "none") return; var groups = (data && data.groups) || []; - notifsEl.textContent = ""; + if (!groups.length) { + notifsEl.textContent = ""; + notifClocks = []; + notifsEl.hidden = true; + return; + } + + // Keyed by source, exactly like the islands, and for the same reason: + // the old code wiped the whole stack and rebuilt it, and + // `.ls-notif-group` carries the same 520ms `ls-island-in` entrance, so + // every stack replayed its entrance on every poll. It also threw away + // keyboard focus, and a stack IS a button. + var existing = {}; + var kids = notifsEl.children; + for (var i = 0; i < kids.length; i++) { + var key = kids[i].getAttribute("data-source"); + if (key !== null) existing[key] = kids[i]; + } + var want = []; + for (var j = 0; j < groups.length; j++) { + var group = groups[j]; + if (!group.items || !group.items.length) continue; + var source = String(group.source); + var el = Object.prototype.hasOwnProperty.call(existing, source) + ? existing[source] : null; + var identity = notifIdentity(group); + // A stack whose notifications genuinely CHANGED is new content, and + // new content is exactly what the entrance animation is for. A stack + // that did not change keeps its node, so it does not animate. + if (el && el.getAttribute("data-identity") !== identity) el = null; + if (!el) { + el = notifGroup(group); + el.setAttribute("data-source", source); + el.setAttribute("data-identity", identity); + } + delete existing[source]; + want.push(el); + } + placeInOrder(notifsEl, want); + + // The minute labels are retouched in place on their own timer, so the + // list of them is rebuilt from what is ACTUALLY on screen -- a stack + // that was left alone still has its own `when` nodes, and they are not + // the ones notifCard just pushed. notifClocks = []; - if (!groups.length) { notifsEl.hidden = true; return; } - for (var i = 0; i < groups.length; i++) { - if (!groups[i].items || !groups[i].items.length) continue; - notifsEl.appendChild(notifGroup(groups[i])); + var whens = notifsEl.querySelectorAll(".ls-notif-when[data-at]"); + for (var k = 0; k < whens.length; k++) { + notifClocks.push({ el: whens[k], at: Number(whens[k].getAttribute("data-at")) }); } + notifsEl.hidden = !notifsEl.firstChild; syncFeedFade(); } @@ -2855,15 +3069,41 @@ def _lock_tail_html() -> str: // because scrollTop, clientHeight and scrollHeight are all fractional under // a non-integer device pixel ratio and never sum exactly. // - // A feed that cannot scroll at all has no room either, so it still unlocks - // across the whole screen -- the device with one agent and no - // notifications, which is the first screen a new user ever sees. + // A feed that CANNOT SCROLL AT ALL is the third case, and it was wrong. + // Room alone cannot tell it apart from a feed scrolled to its end: both + // report zero. They are opposite situations, though. At the end of a long + // feed the drag that got you there is finished and an upward swipe means + // unlock. On a feed that never scrolled, an upward drag on a card is not + // the end of anything -- it is the user pushing at the cards -- and + // throwing them into the keypad is the same complaint as tsk-36i6ed from + // the other side. + // + // MEASURED at the device's real viewport (540x1200, sway scale 2.0) with a + // full lock screen of SIX agents: #ls-feed scrollHeight 394 == clientHeight + // 394, so it does not overflow and never did. The feed is content-sized; + // the islands fit. So this branch is not a corner case on a new user's + // phone -- it is the ordinary state of the demo device, and it meant every + // drag that started on an island opened the keypad. + // + // Overflow therefore comes back into the veto, but as a DISJUNCT rather + // than the conjunct that was removed with tsk-36i6ed. That conjunct could + // not change the answer -- the browser clamps scrollTop to 0 on a feed that + // cannot scroll, so it was an unfailable arm. Here each arm decides a case + // by itself and all three are reachable: + // + // scrollable, at the top -> room > 4 -> veto (reading != unlock) + // scrollable, at the end -> neither -> unlock (tsk-36i6ed) + // cannot scroll at all -> !overflows -> veto (cards are not a + // hidden unlock pad) + // + // The gesture is not lost: the feed is 394px of a 1200px screen, so the + // other two thirds of the glass still unlock on a single upward swipe. swipe(document.body, openPasscode, null, function () { return !screenEl || screenEl.getAttribute("data-sheet") === "none"; }, function (ev) { var t = ev.target; if (!t || !t.closest || !t.closest(".ls-feed")) return false; - return feedScrollRoom() > 4; + return feedScrollRoom() > 4 || !feedOverflows(); }); // Dismiss: only by dragging the sheet's own header. var chatHead = chatSheet ? chatSheet.querySelector(".ls-sheet-head") : null;