From aaf6697c5790ce139df7df92695265505b9b6e26 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Fri, 21 Aug 2026 20:00:49 +0200 Subject: [PATCH] ci(nix): put record back after sources, and record why the experiment failed The ordering experiment did not answer its question and could not have. By the time it ran, record had started succeeding from its old position anyway -- twice in a row, first attempt, 12-17ms -- so there was no contrast left to measure. From position 4 it succeeded, which proves nothing it was not already doing from position 9. Sources came in at 2/5, which looks like the other half of the prediction and is not readable as such: the three failures fell at invocations 7, 8 and 9 with successes either side, which is a window rather than a gradient. So the block goes back where it was. The comment says the experiment was tried and why it is not worth a third attempt, because a bare revert would leave the next person to have the same idea. What the six runs did establish is that the premise was wrong, and that is worth more than the experiment would have been. Enumeration is bimodal: 12-31ms when it answers, no return at all when it does not, nothing in between across every measurement taken. The failures cluster by run, and by window within a run, not by command. The record-versus-sources gap that started this -- 1 in 10 against 13 in 20 -- was that clustering seen through a denominator. I read it as a property of the record path and it was not one. Kept: the run_cli counter and the per-attempt labels, which cost nothing and are what makes the question answerable the next time a run actually fails. That is the condition for reopening it. Co-Authored-By: Claude --- .github/workflows/nix-build.yml | 178 +++++++++++++++----------------- 1 file changed, 85 insertions(+), 93 deletions(-) diff --git a/.github/workflows/nix-build.yml b/.github/workflows/nix-build.yml index 3bb79037e..94c5b2bdd 100644 --- a/.github/workflows/nix-build.yml +++ b/.github/workflows/nix-build.yml @@ -216,98 +216,6 @@ jobs: echo "--- openscreen --help ---" run_cli $SANDBOX $CHROME_FLAGS --help - # The real acceptance test, and now the first one to run. Nothing above - # touches the compositor addon, which is what actually renders output: - # record a couple of seconds, export it, and look at what came out. - # - # ORDERING EXPERIMENT -- this block used to sit after the sources loop, - # and moving it is the whole point of the change that put it here. - # Across four runs, record succeeded 1 time in 10 while sources managed - # 13 in 20, and when record does work it returns in 17ms, exactly like - # sources: same mechanism, different frequency. The one difference the - # two had was position. run_cli spawns a fresh `xvfb-run -a` per - # invocation, so record was always invocations 6-8 of the step, after - # five Xvfb servers had been started and torn down, while sources ran at - # 4-8. If display churn is what drives the failures, record should now - # improve. If it stays at roughly 1 in 10 from this position, position - # was never the cause and something in the record path itself is, which - # is a different search. - # - # The other half of that prediction -- sources getting worse -- is NOT - # readable from its aggregate rate, and an earlier draft of this comment - # claimed it was. The workload ahead of sources is not fixed: the sandbox - # probe costs one invocation or two, and the loop below breaks on first - # success, so it costs between two and six. Sources therefore starts at a - # different position on different runs, and a rate averaged over runs - # confounds the two. Hence the run_cli counter: every attempt now prints - # the invocation it is, so the rates can be stratified by position across - # several runs. One run does not settle this; the counter is what makes - # several of them add up. - # - # Read the next runs as that A/B, not as a verdict on the package. Put it - # back if the rates do not move. - # - # Up to three goes, because screen capture on this host is unreliable in - # its own right. One success is enough for the question being asked here. - echo "--- record then export (first run_cli here is #$((RUN_CLI_N + 1))) ---" - EXPORTED="" - # Tracked apart from EXPORTED so the verdict can name the stage that - # actually failed. Every run so far has died in record without export - # ever executing, while the annotation said "the export path does not - # work" -- an accusation aimed at the one component the run never - # reached, and the compositor addon is precisely what this step exists - # to vouch for. - RECORDED=0 - for i in 1 2 3; do - echo "=== export attempt $i/3 (run_cli #$((RUN_CLI_N + 1))) ===" - rm -f /tmp/demo.openscreen /tmp/demo.mp4 - RC=0 - CLI_TIMEOUT=120 OPENSCREEN_DIAGNOSTIC=1 run_cli $SANDBOX $CHROME_FLAGS record --duration 2 --project /tmp/demo.openscreen >"/tmp/rec.$i.out" 2>&1 || RC=$? - # Outside the failure branch for the same reason as above: a record that - # works is exactly the measurement missing from the comparison, since - # this path has never yet produced one. - grep -a "get-sources\]" "/tmp/rec.$i.out" || true - if [ "$RC" -ne 0 ] || [ ! -f /tmp/demo.openscreen ]; then - echo "record failed (rc=$RC); last lines:" - tail -5 "/tmp/rec.$i.out" || true - continue - fi - RECORDED=1 - echo "recorded. project:" - head -c 200 /tmp/demo.openscreen; echo - - RC=0 - CLI_TIMEOUT=180 OPENSCREEN_DIAGNOSTIC=1 run_cli $SANDBOX $CHROME_FLAGS export /tmp/demo.openscreen -o /tmp/demo.mp4 >"/tmp/exp.$i.out" 2>&1 || RC=$? - if [ "$RC" -ne 0 ] || [ ! -f /tmp/demo.mp4 ]; then - echo "export failed (rc=$RC); last lines:" - tail -15 "/tmp/exp.$i.out" || true - continue - fi - EXPORTED=/tmp/demo.mp4 - break - done - - EXPORT_OK=0 - if [ -z "$EXPORTED" ] && [ "$RECORDED" -eq 0 ]; then - echo "::error::No attempt got past record, so export never ran and the compositor addon is unproven. This is a capture failure on this host, not an export failure." - elif [ -z "$EXPORTED" ]; then - echo "::error::record produced a project but no attempt produced an MP4. The compositor addon is packaged and the export path does not work." - else - SIZE=$(wc -c < "$EXPORTED") - # An MP4 opens with a 4-byte length then 'ftyp'. A zero-length or - # truncated file would otherwise pass a mere existence check. - MAGIC=$(dd if="$EXPORTED" bs=1 skip=4 count=4 2>/dev/null || true) - echo "exported $SIZE bytes, magic at offset 4: $MAGIC" - if [ "$MAGIC" != "ftyp" ]; then - echo "::error::output is not an MP4 (no ftyp box)" - elif [ "$SIZE" -lt 10000 ]; then - echo "::error::MP4 is only $SIZE bytes, too small to hold two seconds of video" - else - echo "Export works: $SIZE bytes of MP4." - EXPORT_OK=1 - fi - fi - # `sources` enumerates displays, windows and microphones, so it # exercises the capture stack rather than proving that a usage string # can be printed. @@ -399,7 +307,7 @@ jobs: fi # Noted, not acted on. Enumeration is flaky here and the export - # assertion -- which now runs above rather than below -- is the more + # assertion below is the more # important question; a flaky attempt costs an annotation rather than # the answer. Verdict at the end, once both have had their say. if [ "$HUNG" -gt 0 ]; then @@ -409,6 +317,90 @@ jobs: echo "::warning::openscreen sources failed on $FAILED of $ATTEMPTS attempts. The capture path is not reliable on this host." fi + # The real acceptance test. Everything above proves the package starts + # and can list a screen; none of it touches the compositor addon, which + # is what actually renders output. Record a couple of seconds, export it, + # and look at what came out. + # + # This block was briefly moved ahead of the sources loop and moved back, + # so that it is not tried a third time. The theory was that position + # explained why record seemed to fail far more often than sources -- + # run_cli spawns a fresh `xvfb-run -a` each time, so record was always + # invocations 6-8, after five Xvfb servers had come and gone. The + # experiment could not answer it: by the time it ran, record had started + # succeeding from its old position anyway, so there was no contrast left + # to measure. From position 4 it succeeded, which proves nothing it was + # not already doing from position 9. + # + # What the runs did establish is that the premise was wrong. Enumeration + # here is bimodal -- 12-31ms when it answers, no return at all when it + # does not, with nothing in between across every measurement so far -- + # and the failures cluster by run and by window within a run rather than + # by command. The apparent record-versus-sources gap was that clustering + # seen through a denominator, not a property of either path. Reopen this + # with the run_cli labels, on a run that actually fails, before assuming + # otherwise. + # + # Up to three goes, because screen capture on this host is unreliable in + # its own right. One success is enough for the question being asked here. + echo "--- record then export (first run_cli here is #$((RUN_CLI_N + 1))) ---" + EXPORTED="" + # Tracked apart from EXPORTED so the verdict can name the stage that + # actually failed. For three runs every attempt died in record without + # export ever executing, while the annotation said "the export path does + # not work" -- an accusation aimed at the one component the run never + # reached, and the compositor addon is precisely what this step exists + # to vouch for. + RECORDED=0 + for i in 1 2 3; do + echo "=== export attempt $i/3 (run_cli #$((RUN_CLI_N + 1))) ===" + rm -f /tmp/demo.openscreen /tmp/demo.mp4 + RC=0 + CLI_TIMEOUT=120 OPENSCREEN_DIAGNOSTIC=1 run_cli $SANDBOX $CHROME_FLAGS record --duration 2 --project /tmp/demo.openscreen >"/tmp/rec.$i.out" 2>&1 || RC=$? + # Outside the failure branch for the same reason as above: a record that + # works is exactly the measurement missing from the comparison, since + # this path has never yet produced one. + grep -a "get-sources\]" "/tmp/rec.$i.out" || true + if [ "$RC" -ne 0 ] || [ ! -f /tmp/demo.openscreen ]; then + echo "record failed (rc=$RC); last lines:" + tail -5 "/tmp/rec.$i.out" || true + continue + fi + RECORDED=1 + echo "recorded. project:" + head -c 200 /tmp/demo.openscreen; echo + + RC=0 + CLI_TIMEOUT=180 OPENSCREEN_DIAGNOSTIC=1 run_cli $SANDBOX $CHROME_FLAGS export /tmp/demo.openscreen -o /tmp/demo.mp4 >"/tmp/exp.$i.out" 2>&1 || RC=$? + if [ "$RC" -ne 0 ] || [ ! -f /tmp/demo.mp4 ]; then + echo "export failed (rc=$RC); last lines:" + tail -15 "/tmp/exp.$i.out" || true + continue + fi + EXPORTED=/tmp/demo.mp4 + break + done + + EXPORT_OK=0 + if [ -z "$EXPORTED" ] && [ "$RECORDED" -eq 0 ]; then + echo "::error::No attempt got past record, so export never ran and the compositor addon is unproven. This is a capture failure on this host, not an export failure." + elif [ -z "$EXPORTED" ]; then + echo "::error::record produced a project but no attempt produced an MP4. The compositor addon is packaged and the export path does not work." + else + SIZE=$(wc -c < "$EXPORTED") + # An MP4 opens with a 4-byte length then 'ftyp'. A zero-length or + # truncated file would otherwise pass a mere existence check. + MAGIC=$(dd if="$EXPORTED" bs=1 skip=4 count=4 2>/dev/null || true) + echo "exported $SIZE bytes, magic at offset 4: $MAGIC" + if [ "$MAGIC" != "ftyp" ]; then + echo "::error::output is not an MP4 (no ftyp box)" + elif [ "$SIZE" -lt 10000 ]; then + echo "::error::MP4 is only $SIZE bytes, too small to hold two seconds of video" + else + echo "Export works: $SIZE bytes of MP4." + EXPORT_OK=1 + fi + fi # One verdict, after both questions have been asked. Enumeration being # flaky must not hide whether export works, which is the whole point of