Keep the peak network speed-test sample instead of the last - #41
Keep the peak network speed-test sample instead of the last#41PeriklisTs wants to merge 1 commit into
Conversation
The speed-test backend streams one throughput sample per second for the whole phase, and updateSpeedTestLine() overwrote the reported value on every line. The figure shown was therefore whichever single sample happened to arrive last before speedTestPhaseDuration fired. With the 5s phase budget that window is dominated by TCP slow-start, so the reported speed can be several times lower than the link. On a wired gigabit connection sustaining ~975 Mbps the widget reported 127 Mbps; the backend's samples over that window were 456, 283, 118, 127. Keep the highest sample of the phase instead. runSpeedTest() already clears both values per run, so the peak does not leak between runs. Add a ramping fixture whose samples rise and then dip, so the final sample is not the peak, and assert the service reports the peak.
6519168 to
9ba07e9
Compare
|
Update: I now have live evidence for this, and I have corrected the PR
Wired gigabit, Omarchy 4.0.1-1, suite payload 0.1.1-beta.11. Two corrections to what the description originally said:
The in-harness gap is unchanged: the QML regression still has not executed here, |
9ba07e9 to
7b97d7e
Compare
Affected plugin
hancore.shibumi.network(service only; no visible surface changed).Problem
omarchy-network-speedtestis a streaming backend: it prints one throughput sample per second for the entire phase and runs until it is killed.updateSpeedTestLine()assigned that value straight intospeedTestDownloadMbps/speedTestUploadMbpson every line, so the reported figure was whichever single sample happened to arrive last beforespeedTestPhaseDurationfired.With the current 5000 ms budget — which also absorbs process spawn, the
InlineSpeedTestRunner.pyfork/supervise setup, and a blocking round trip toapi.fast.comfor endpoint URLs — only about three or four samples arrive, all while the transfer is still in TCP slow-start.On my wired gigabit link the widget reported 127 Mbps. The backend's samples across that same window were:
Independent measurement on the same link: 969 Mbps sustained (1156 MB in 10.0 s), stable at 960–987 Mbps once ramped, 1000 Mbps negotiated at
/sys/class/net/enp2s0/speed. So the displayed figure was low by roughly 8x, and the last-sample rule is the dominant cause.Change
Keep the highest sample of the phase rather than the last.
runSpeedTest()already clears both properties at the start of every run, so the peak cannot leak between runs.This is deliberately the minimal fix: no change to
speedTestPhaseDuration, no change to the backend contract, no change to any visible surface. A longer phase with an explicit warm-up discard and a trimmed mean would be more accurate still, but that changes test duration and UX, so I left it for your call — see the note below.Checks run
nodetranscription ofupdateSpeedTestLine()before and after, over five sample streams plus a cross-run reset case. The ramping stream yields210/90on the old logic and880/310on the new; the existing single-sample fixture yields42.5/17.25on both, so current assertions are unaffected. All pass.runSpeedTest()clears both properties, so peak state resets per run.Checks I could NOT run — please verify on the validation system
OMARCHY_PATH=/usr/share/omarchy ./tests/network-plugin-regression.shand the fulltests/contract-regression.shdo not run on my machine, on this branch or on unmodifiedmain:The default
installed-packageprofile pinscontracts/baselines/omarchy-installed-package-v4.0.0.json, and I am on Omarchy 4.0.1-1.SHIBUMI_OMARCHY_BASELINE_PROFILE=forward-compatfails the same way against its own pinned snapshot. I confirmed this failure is pre-existing and unrelated to this change by reproducing it on a cleanmaincheckout.So the QML regression added here is unverified in-harness. It is written to the existing phase-machine pattern in
tests/network-plugin-smoke.qml, but it has never executed. Please run it before merging.Live UI evidence
Measured after this PR was opened; this section originally said "none".
I have since installed this branch on my primary desktop and run the speed test in the bar:
To be transparent about how that was obtained: CONTRIBUTING says not to install a development suite over a primary desktop without an isolated test profile and a recovery plan. I had the recovery plan (config backed up,
shibumi-suite repairavailable, config and third-party plugins verified intact afterwards) but not a separate isolated profile, so weigh this evidence accordingly.The remaining physical gate is unchanged: a real Wayland run on the validation system.
Out of scope, but worth knowing when validating
omarchy-network-speedtest(Omarchy core, not this repo) sources endpoints from fast.com. On my connection that returns Netflix OCA nodes in Milan, Warsaw, and Frankfurt over IPv6 — no nearby node — and that path averages ~535 Mbps with a 153–820 Mbps spread regardless of how samples are aggregated.I originally predicted this fix would therefore land around 700–800 rather than near the Ookla figure. The measured result above (920) is higher than that whole sampled range, so that sample was not representative and the estimate was too conservative — the peak value tracks the link more closely than I expected. I am leaving the note because the endpoint choice is still Omarchy's, results may vary by routing, and I see
backend-boundary-v1.jsonalready anticipates replacing the host helper with a native route.