From eaf27c8da5c7231fbb6de8bf0e669ab463bf8aae Mon Sep 17 00:00:00 2001 From: jaylfc Date: Wed, 16 Sep 2026 19:19:29 +0000 Subject: [PATCH 1/3] The lock screen stops unlocking from a card, and stops flickering Two bugs Jay reported from the glass, both measured in a real browser at the device's own CSS viewport (540x1200) rather than reasoned about from source. tsk-bgxtxn -- swiping up on an agent island opened the keypad. With the six agents the demo phone shows, #ls-feed reports scrollHeight 394 and clientHeight 394: the feed is content-sized, the islands fit, and it does not overflow. The unlock veto asked only how far the feed could still travel, so it read zero room and stood aside, and every drag beginning on an island unlocked the phone. The veto was not broken -- it was doing what it said. The comment shipped with tsk-36i6ed reasoned that a feed which cannot scroll is not being read, so a drag from it should unlock, and treated that as a corner case for a device with one agent and nothing to show. The measurement says it is the ordinary state of the screen. That deliberate rule is the bug. Overflow therefore returns to the veto, as a disjunct rather than 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 its own case and no other: scrollable, at the top -> room left -> veto (reading is not unlocking) scrollable, at the end -> neither -> unlock (tsk-36i6ed, preserved) cannot scroll at all -> !overflows -> veto (cards are not an unlock pad) Two existing tests asserted the old rule and are reversed here deliberately, saying so and saying why. A new mutation control strips the added arm and requires the non-scrolling case to go red while both scrollable cases stay exactly as they are -- a mutation that broke everything would show only that the suite notices change, not that these scenarios tell a dead feed from a feed read to its end. tsk-5baas4 -- the islands flickered every fifteen seconds. paintActivity() wiped #ls-agents and rebuilt every island on each poll. Every one was a new element, and .ls-island carries a 520ms entrance animation with staggered per-child delays, so the whole list replayed its entrance every tick whether or not a byte of the payload had changed. Measured over one cycle with nothing touched: six of six islands fired animationstart and the first island was no longer the same node. After: zero, and the same node. The list is now reconciled by agent name. An island that persists is updated in place and never re-inserted -- re-inserting a node restarts its animation, so the reconcile checks position before it moves anything. A reconfigured agent, a new portrait or a different framework, is still rebuilt; that comes from configuration rather than from a tick. The tests assert node identity, not rendered values: the old code rendered the right names too, which is exactly why a value assertion would have passed on it. The focus save and restore stays for the cases reconciliation cannot cover, and its ordering test now names the repaint instead of the wipe it used to name. Still open, and not in here: the stats and notification pollers repaint wholesale the same way, which is the flicker Jay saw in the system widget. Docs-Reviewed: no route was added, removed or changed. The edit is entirely inside _LOCK_SCREEN_SCRIPT, the client-side script auth.py serves, plus its tests; the HTTP surface and the agent-coordination contract are untouched. --- .../tsk-bgxtxn-lockscreen-glass-bugs.md | 16 + tests/test_lock_screen_gestures.py | 99 ++++- tests/test_lock_screen_repaint.py | 372 ++++++++++++++++++ tests/test_onscreen_keyboard.py | 24 +- tinyagentos/routes/auth.py | 142 ++++++- 5 files changed, 624 insertions(+), 29 deletions(-) create mode 100644 changelog.d/tsk-bgxtxn-lockscreen-glass-bugs.md create mode 100644 tests/test_lock_screen_repaint.py 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..d87134359 100644 --- a/tests/test_lock_screen_gestures.py +++ b/tests/test_lock_screen_gestures.py @@ -140,7 +140,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 +163,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 +181,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 +203,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 +269,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 + + def test_a_non_scrolling_feed_does_not_kill_the_gesture_elsewhere(self): + """The cost of the reversal above, bounded. - 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. + 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 +348,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 +393,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..845977d6e --- /dev/null +++ b/tests/test_lock_screen_repaint.py @@ -0,0 +1,372 @@ +"""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 tests.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. +_HARNESS = 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: "", + textContent: "", + 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; + } + }; + Object.defineProperty(el, "firstChild", { + get: function () { return this.children.length ? this.children[0] : 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 = {}; + +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 = ""; + agentsEl.children.length = 0; + 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"), + ] + 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) diff --git a/tests/test_onscreen_keyboard.py b/tests/test_onscreen_keyboard.py index 352688857..f6bc1745f 100644 --- a/tests/test_onscreen_keyboard.py +++ b/tests/test_onscreen_keyboard.py @@ -726,7 +726,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 @@ -744,19 +744,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 08a143be8..390ce847c 100644 --- a/tinyagentos/routes/auth.py +++ b/tinyagentos/routes/auth.py @@ -1483,6 +1483,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"; @@ -1490,6 +1533,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 @@ -1592,6 +1636,65 @@ 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. + 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 prev = null; + 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); + } + delete existing[name]; + // Put it where the payload says, WITHOUT touching an element that is + // already in position: re-inserting a node restarts its animation, so a + // blind appendChild of every island in order would flicker exactly as + // badly as the wipe it replaced. + var want = prev ? prev.nextSibling : agentsEl.firstChild; + if (el !== want) agentsEl.insertBefore(el, want); + prev = el; + } + // Whatever the payload no longer lists has genuinely gone away. + for (var gone in existing) { + if (Object.prototype.hasOwnProperty.call(existing, gone)) { + existing[gone].remove(); + } + } + } + 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 @@ -1617,15 +1720,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"; @@ -2198,15 +2298,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; From 026b5cb8b8815cd9d08c45985d51948137b38e50 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Wed, 16 Sep 2026 19:44:11 +0000 Subject: [PATCH 2/3] The stats panel and the notification stacks stop flickering Jay, from the glass, in the same breath as the agent islands: "the system stats widget flickers too". It is the same defect one view over, and worse: the stats poll runs every THREE seconds, not fifteen, and `.ls-stat-card` carries the same 520ms `ls-island-in` entrance the islands do. Every tick wiped `#ls-stats` and rebuilt it, so the card replayed its entrance twenty times a minute. Measured in chromium at the device's own 540x1200 viewport, stats view, nothing touched. Before: two entrance replays in 7.5s, 2992ms apart -- the poll -- and the card a different, already-detached element each time. After: none across three consecutive nine-second runs, the card keeping its element and the readings updating in place. Reconciling also makes `.ls-stat-fill`'s 420ms width transition mean something. A transition needs a FROM value, which only a surviving element has, so every meter had been snapping to its reading rather than travelling to it. Measured after: the same fill element, 69.7% -> 89.2%. The notification stacks had it too, at a fifteen-minute cadence: a wipe, the same entrance animation, and -- because a stack is a `role="button"` with a tabindex -- keyboard focus thrown away with it. Keyed by source now. A stack whose notifications genuinely changed is still rebuilt, because that is new content arriving and the animation is what it is for. Two things the reconcile had to get right rather than merely make still: - A reading that stops being measured loses its bar. A meter left at its last value goes on reporting a measurement nobody is making, and reads exactly like a live one. - The minute labels are collected from what is actually on screen rather than from whatever the paint happened to build. A stack that is deliberately left alone builds nothing, so the old list would have silently stopped retouching its clocks -- and a clock frozen at "2h ago" looks like a working one. The weather and task lists are polled and rebuilt the same way and are deliberately left alone: nothing they rebuild carries an animation or a transition, and neither holds focus, so neither can flicker. That is a measurement of those two, not an assumption about them. ALSO, in the test harness, a defect that made the suite pass for the wrong reason: `_balanced` was quote-aware but not comment-aware, so the apostrophe in `// The App Store's own artwork` inside `island()` opened a string that never closed. `_function("island")` was quietly returning 12kB -- island(), reconcileIslands() AND paintActivity() -- instead of 5kB. Everything passed, because the extra functions were the real ones and `reconcileIslands` was being supplied by accident rather than by name. Now comment-aware, and the reconcile helpers are named in `_source` explicitly. The DOM stand-in's `textContent` was a plain string property, so assigning it did not remove children the way a browser does. A panel that failed to empty would have looked empty to the harness. It now clears its children, which is also what the controls were hand-rolling around. Docs-Reviewed: no route surface changes; this is lock-screen client script and its tests. --- changelog.d/tsk-5baas4-stats-notif-flicker.md | 24 + tests/test_lock_screen_gestures.py | 21 + tests/test_lock_screen_repaint.py | 463 +++++++++++++++++- tinyagentos/routes/auth.py | 260 +++++++--- 4 files changed, 692 insertions(+), 76 deletions(-) create mode 100644 changelog.d/tsk-5baas4-stats-notif-flicker.md 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/tests/test_lock_screen_gestures.py b/tests/test_lock_screen_gestures.py index d87134359..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: diff --git a/tests/test_lock_screen_repaint.py b/tests/test_lock_screen_repaint.py index 845977d6e..0dfcb49bf 100644 --- a/tests/test_lock_screen_repaint.py +++ b/tests/test_lock_screen_repaint.py @@ -51,7 +51,7 @@ def _var(name: str) -> str: #: 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. -_HARNESS = r""" +_DOM = r""" var NODE_SEQ = 0; function makeNode(tag) { @@ -65,7 +65,7 @@ def _var(name: str) -> str: style: { setProperty: function () {} }, classList: { contains: function () { return false; } }, innerHTML: "", - textContent: "", + _text: "", addEventListener: function () {}, focus: function () {}, setAttribute: function (k, v) { this._attrs[k] = String(v); }, @@ -104,11 +104,43 @@ def _var(name: str) -> str: 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; @@ -127,7 +159,11 @@ def _var(name: str) -> str: }; 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__ @@ -166,7 +202,6 @@ def _var(name: str) -> str: _WIPE_AND_REBUILD = r""" function wipeAndRebuild(agents) { agentsEl.textContent = ""; - agentsEl.children.length = 0; for (var i = 0; i < agents.length; i++) { agentsEl.appendChild(island(agents[i])); } @@ -185,6 +220,13 @@ def _source(*, reconciled: bool = True) -> str: _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")) @@ -370,3 +412,418 @@ def test_harness_observes_the_defect(self): 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/tinyagentos/routes/auth.py b/tinyagentos/routes/auth.py index 0c43858c3..deade7592 100644 --- a/tinyagentos/routes/auth.py +++ b/tinyagentos/routes/auth.py @@ -2051,6 +2051,45 @@ def _lock_tail_html() -> str: // 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; @@ -2058,7 +2097,7 @@ def _lock_tail_html() -> str: var key = kids[i].getAttribute("data-agent"); if (key !== null) existing[key] = kids[i]; } - var prev = null; + var want = []; for (var j = 0; j < agents.length; j++) { var agent = agents[j]; var name = agent.name || "agent"; @@ -2075,21 +2114,14 @@ def _lock_tail_html() -> str: } 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]; - // Put it where the payload says, WITHOUT touching an element that is - // already in position: re-inserting a node restarts its animation, so a - // blind appendChild of every island in order would flicker exactly as - // badly as the wipe it replaced. - var want = prev ? prev.nextSibling : agentsEl.firstChild; - if (el !== want) agentsEl.insertBefore(el, want); - prev = el; - } - // Whatever the payload no longer lists has genuinely gone away. - for (var gone in existing) { - if (Object.prototype.hasOwnProperty.call(existing, gone)) { - existing[gone].remove(); - } + want.push(el); } + // Whatever the payload no longer lists has genuinely gone away, and + // placeInOrder drops it. + placeInOrder(agentsEl, want); } function paintActivity(data) { @@ -2396,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 @@ -2458,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(); } @@ -2627,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"); @@ -2683,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(); } From e0782ba0aaec85b07e2c069c569c6a4c8e68e4d3 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Wed, 16 Sep 2026 19:58:13 +0000 Subject: [PATCH 3/3] The repaint tests import the way CI's per-file run needs `check-all-skip` is the only job in CI that runs pytest on a SINGLE FILE, and `pytest tests/.py` puts tests/ itself on sys.path rather than the repo root. There is no tests/__init__.py, so the `tests.` package does not exist under that invocation and the import raised ModuleNotFoundError with rc=2. The shards pass the DIRECTORY, so they resolved it and went green: their green was correct and was not evidence that this file imports. Verified both ways rather than just the one that was failing: the file alone (29 passed) and alongside its sibling (47 passed). This is the only file in tests/ that used the `tests.` prefix, so nothing else moves with it. Docs-Reviewed: test-only import fix; no route or API surface involved. --- tests/test_lock_screen_repaint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_lock_screen_repaint.py b/tests/test_lock_screen_repaint.py index 0dfcb49bf..20594d371 100644 --- a/tests/test_lock_screen_repaint.py +++ b/tests/test_lock_screen_repaint.py @@ -33,7 +33,7 @@ from tinyagentos.routes.auth import _LOCK_SCREEN_SCRIPT as LOCK_SCRIPT -from tests.test_lock_screen_gestures import _balanced, _function +from test_lock_screen_gestures import _balanced, _function def _var(name: str) -> str: