From 01b0c9b10df7fd00fa5f0fe4c34a6cfa6739c654 Mon Sep 17 00:00:00 2001 From: Sylpheed port agent Date: Sun, 30 Aug 2026 22:55:40 +0000 Subject: [PATCH] port: a leak that was not mine, a second narrow anchor, and a result recovered Three findings, one of them a withdrawal of my own fix. The ObjectDB leak line on every run is engine-side. The leaked objects are the Ogg streams and playbacks of exactly the cues that sounded, which reads as MenuAudio holding references past teardown. It does not: releasing every reference the port owns -- stop each player, null every stream, clear _players, clear cues/beds/voices -- moved the count not at all, 8 before and 8 after, with a debug print confirming _exit_tree runs. The cleanup is REVERTED rather than kept, because code that changes nothing under a comment claiming to fix a leak is worse than none: the next reader sees it handled and stops looking. Filed as a negative result so nobody re-investigates. check_focus_persists gets a SECOND NARROW ANCHOR, repairing a weakness I recorded last iteration and did not act on. It anchored on the heading -- the conclusion -- so when the Decoder corrected the run's item names it sailed past, surviving by luck rather than design. It now also rests on the evidence, the ring at y 384.0 before the round trip and 385.5 after, which is the geometry-free equality the conclusion stands on. The two anchors are checked AGAINST EACH OTHER: if one matches and the other does not it reports ANCHOR SPLIT. The second anchor has its own known negative, perturbing only the evidence line -- without that it would be decorative and the check would still rest on the conclusion alone. And their skippability rule recovers a result I had over-withdrawn. Frames can be skipped, bytes consumed cannot; that is why my withdrawal reaches my test and not their read-offset one. Applied backwards: the OVERRUN IS the evidence nothing was skipped. A player that drops frames finishes on schedule; mine took 146.6 s for 137.44 s of media, so ADV +6.7% and S00A -0.5% are time-to-consume measurements after all. The withdrawal stands for the pacing-audit use; the load-starvation result is recovered. Standing caveat recorded: every timing this port publishes is frame-derived, and the only reason those seconds mean anything is that this player demonstrably does not skip -- an empirical property, not a guarantee, and nothing checks it. Reported: the 'do not hardcode the menu's initial focus' HANDOFF section still reads as live while two later sections have overtaken both its claims. Every asserting check passes; 14 controls fire. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01N7FiFFFwbvG2uxdcEh8HyF --- docs/port/BLOCKED.md | 6 ++ docs/port/DECISIONS.md | 114 +++++++++++++++++++++++++++++++++++++- tools/port/contract-check | 29 +++++++++- 3 files changed, 147 insertions(+), 2 deletions(-) diff --git a/docs/port/BLOCKED.md b/docs/port/BLOCKED.md index 56c8b954..8b1af939 100644 --- a/docs/port/BLOCKED.md +++ b/docs/port/BLOCKED.md @@ -147,6 +147,12 @@ HANDOFF. | ~~P1–P7 — the keyframe record layout~~ | ~~adopt the corrected pose/time pairing~~ | — | ✅ **ADOPTED 2026-08-29 by pinning `formats-pin-2026-08-29c`.** This row was wrong twice: it said the change *"cannot be taken yet"* and that it *"reaches the port only when that branch lands on `main`"*. **It arrives when the tag is pinned**, which is what MISSION §2's tagging rule exists for. ⚠️ And the knob I tested first, `SYLPHEED_KF_TIME_SHIFT`, is a **retired partial fix** that left pose 0 untimed — the real correction is the tagged crate's default, with the old reading behind `SYLPHEED_KF_TIME_LEGACY=1`. **The blast radius was far smaller than this row predicted**: under the correction *every pose is timed* (866 keyframes, 0 untimed), so `pose_at`'s synthetic-exit branch became dead code rather than wrong code and nothing needed re-deriving. Oracle: `publisher_logo` 1.00 %→**0.75 %**, `developer_logos` 0.39 %→**0.33 %**, `extras`' differing region collapsing from 736×525 to **398×295 at the sweep position**. 🔴 Open cost: `sylpheed-cli` builds from the workspace crate, so `verify-screen` compares two decoder eras until the tag reaches `main`. Revert to the path dependency then. | | ~~P7 / naming — the four unnamed builds~~ | ~~which locale and variant is each of entries 0, 1, 12, 15?~~ | — | ✅ **answered 2026-08-29** (`docs/re/ui-title-build-map.md`): all four are the loading screen, two variants — plain (7 elements) and dressed (10) — decoded from their own `pgloading_*` element names. ⚠️ **Not adopted as names yet, for two reasons the RE agent gave and one the port found.** Theirs: the executable names exactly two, and *which* bundle takes which name is 🟡 undecided, so `LOADING`/`LOADING2` must not go in an asset path; and locale is 🟡 — the English member of a pair is the one in the first half of `GP_TITLE.p00`, 8/8 structurally but only 3/3 where a capture can check, and the three pairs that matter are the three no capture can check. Mine: **the message gives the bundles as "0/1 and 10/11", which is the `is_build` ordinal, and `authored/screen_names.json` is keyed by PAK ENTRY** — in entry space 10 and 11 are `palogo_sqex` and `palogo_gamearts`, the splashes. See the refutation section in `DECISIONS.md`. | +## Not blocking, recorded so nobody re-investigates — 2026-08-30, HANDOFF `91ada14` + +| Milestone | Needs | HANDOFF | State | +|---|---|---|---| +| P4–P7 — the `ObjectDB` leak line | **nothing; it is engine-side** | `91ada14` | 🟢 **NEGATIVE RESULT, and the obvious diagnosis is wrong.** Every run prints `N ObjectDB instances were leaked at exit`, and the objects are the Ogg streams and playbacks of exactly the cues that sounded — which reads as `MenuAudio` holding references past teardown. It does not. Releasing **every reference the port owns** (stop each player, null every `stream`, clear `_players`, clear `cues`/`beds`/`voices`) moved the count **not at all: 8 before, 8 after**, with a debug print confirming `_exit_tree` runs. The cleanup was **reverted** rather than kept, because code that changes nothing under a comment claiming to fix a leak is worse than none — the next reader sees it handled and stops looking. ⚠️ Cost of leaving it: it is log noise on every run, and the previous iteration found two real defects by reading that log. | + ## Caveat on my own artifacts, 2026-08-30 — derived from HANDOFF `4ed75e6` | Milestone | Needs | HANDOFF | State | diff --git a/docs/port/DECISIONS.md b/docs/port/DECISIONS.md index 6487517b..86c1f38d 100644 --- a/docs/port/DECISIONS.md +++ b/docs/port/DECISIONS.md @@ -9,7 +9,7 @@ dies, which is what this file is for. -250 sections. Search this before re-deriving anything. +254 sections. Search this before re-deriving anything. * [P0 — the exporter, 2026-08-28](#p0--the-exporter-2026-08-28) * [P1 — Godot draws the screen, 2026-08-28](#p1--godot-draws-the-screen-2026-08-28) @@ -261,6 +261,10 @@ dies, which is what this file is for. * [The boot's wall-clock seconds are a property of this container, not of the port](#the-boots-wall-clock-seconds-are-a-property-of-this-container-not-of-the-port) * [Their negative result, and the trap in choosing the more general instrument](#their-negative-result-and-the-trap-in-choosing-the-more-general-instrument) * [🔴 Correction: my media-versus-wall-clock method cannot audit container pacing](#correction-my-media-versus-wall-clock-method-cannot-audit-container-pacing) +* [The leak was not mine — a negative result, and the "fix" is reverted](#the-leak-was-not-mine--a-negative-result-and-the-fix-is-reverted) +* [A second narrow anchor, where I had already found the weakness and not acted](#a-second-narrow-anchor-where-i-had-already-found-the-weakness-and-not-acted) +* [Reported: a live-reading HANDOFF section that two later ones have overtaken](#reported-a-live-reading-handoff-section-that-two-later-ones-have-overtaken) +* [Their rule applied backwards: my video result is stronger than my withdrawal said](#their-rule-applied-backwards-my-video-result-is-stronger-than-my-withdrawal-said) ## P0 — the exporter, 2026-08-28 @@ -12982,3 +12986,111 @@ moment it failed, and the swap felt like rigour.** So when an `ANCHOR LOST` come the cheaper move is **a second narrow anchor, not one looser one** — written into `contract-check`'s header so the next reader hits it before reaching for a general matcher. + +## The leak was not mine — a negative result, and the "fix" is reverted + +Every run ends with `N ObjectDB instances were leaked at exit`, and the leaked +objects are `AudioStreamOggVorbis` / `OggPacketSequence` / their playbacks — +exactly the cues that had actually sounded. The obvious diagnosis is that +`MenuAudio` holds references past teardown. + +**It does not.** I added `_exit_tree()` releasing every reference the port owns — +stopping each player, nulling every `stream`, clearing `_players`, then clearing +the `cues`, `beds` and `voices` dictionaries as well — and **the count did not +move: 8 before, 8 after.** A debug print confirms `_exit_tree` runs. Removing the +cleanup again: still 8. + +📌 **Reverted rather than kept.** Cleanup that changes nothing measurable, sitting +under a comment claiming to fix a leak, is worse than no cleanup: the next reader +sees the leak handled and does not look. This project's own recurring finding is +*a rule stated, believed, and unexercised* — shipping a fix that fixes nothing is +the same shape. + +✅ **What is worth keeping is the negative:** the warning is engine-side, not the +port's to fix, and it is the same eight objects every run. Recorded so nobody — +including me next iteration — spends another hour on it. **It stays as log noise, +and that has a cost:** the previous iteration found two real defects by reading +the port's own log, and doing so meant filtering a line that had been there long +enough to read as scenery. + +## A second narrow anchor, where I had already found the weakness and not acted + +Last iteration I recorded that `check_focus_persists` survived the Decoder's +correction **by luck**: it anchors on the heading — the conclusion — while the +item names that were wrong sat below it. I wrote that down and left the check as +it was. + +Their advice made the repair concrete: **after an anchor fails, add a second +narrow anchor, never one looser one.** So the check now rests on the *evidence* +as well — *"ring sits at y 384.0 before the round trip and 385.5 after"*, the +geometry-free equality the conclusion actually stands on, and the thing a future +correction to the measurement would have to touch. + +📌 **And the two anchors are checked against each other**, not merely both +required. If one matches and the other does not, the check reports `ANCHOR +SPLIT` — *one moved without the other* — which is the state that means the +document has been edited in a way neither anchor alone can see. + +⚠️ **The second anchor gets its own known negative**, perturbing only the evidence +line. Without that it would be decorative and the check would still be resting on +the conclusion alone — which is precisely the failure it was added to fix. Both +controls fire. + +## Reported: a live-reading HANDOFF section that two later ones have overtaken + +`## 🔴 2026-08-30 — do not hardcode the menu's initial focus; the sources +disagree` still reads as current, and carries no forward marker. Two of its +claims are now false: + +* *"the sources disagree … expect it to change"* — settled since, by direct + measurement of a fresh boot's **first** menu entry, `NEW GAME`, 2/2. +* *"Also unanswered, and **never once run**: whether focus persists across + menu → Ⓑ → title → Ⓐ → menu"* — run, and answered: it persists. + +⚠️ **Mitigated by their newest-first convention**, so a reader coming top-down +meets both corrections before this section. Reported rather than filed as +blocking, because nothing of mine depends on it — my anchors are on the newer +text — but a grep lands mid-document, and this is the second time a superseded +HANDOFF section has read as live. + +## Their rule applied backwards: my video result is stronger than my withdrawal said + +The Decoder's rule, taken from my correction and sharper than it: **ask whether +the quantity you are timing can be skipped.** Frames, video and animation +timelines can. Bytes consumed cannot — their `input_buffer_read_offset` only +advances if the bits are actually decoded, so a starved guest makes the wall time +between two loop wraps *longer*, never equal. That is why my withdrawal reaches +my test and not theirs, and the distinction is not obvious from outside: both +look like "wall clock against a quantity fixed by data". + +📌 **Applying it back here changes what my own numbers are worth.** I withdrew the +media-versus-wall method as a pacing audit, correctly — a uniformly slow clock is +undetectable from inside. But the load result I filed alongside it is on firmer +ground than I gave it credit for: + +**The overrun IS the evidence that nothing was skipped.** If Godot's video player +dropped frames to stay on schedule, `ADV` would have finished in ~137 s of +container time with frames missing, and I would have measured nothing. It took +**146.6 s**. A player that runs long is a player that decoded everything — so +`ADV` +6.7 % and `S00A` −0.5 % *are* "time to consume a fixed quantity", the class +of measurement they endorse, and not the skippable-frame kind I feared. + +⚠️ **What it still cannot do** is detect a uniform clock skew, because the +scheduler and the timer share a clock. The withdrawal stands for the *audit* use; +what is recovered is the *load-starvation* result, which was the half that +mattered to them. + +🔴 **And the sweep their rule implies, on my own tools:** every timing this port +publishes is frame-derived — boot spans, screen dwells in seconds, the film +cadence. Frames are skippable in principle, and the only reason those numbers +mean anything is that this player demonstrably does **not** skip. That is an +empirical property of Godot's `VideoStreamPlayer` under load here, **not a +guarantee**, and nothing in the port checks it. Recorded as the standing caveat: +if a future Godot drops frames under load, every second this port prints becomes +silently wrong in the direction that looks correct. + +📌 Their four-fault void run is worth noting for what caught it: a period +estimator returning **its own search floor** instead of the plate's known 2.53 s. +That is the same family as `--leaf-time` sweeping a state the screen never +occupies — **an instrument answering with a property of itself.** Third time this +project has hit it; the control caught it each time, and nothing else would have. diff --git a/tools/port/contract-check b/tools/port/contract-check index d2944b9a..d3f56212 100755 --- a/tools/port/contract-check +++ b/tools/port/contract-check @@ -277,9 +277,31 @@ def check_focus_persists(h): positive assertion of the negative. Now only the measured half is asserted against the contract; the scope is a guard, below. """ - want = bool(re.search(r"the main menu remembers its cursor; re-entry is not a reset", h)) + heading = bool(re.search(r"the main menu remembers its cursor; re-entry is not a reset", h)) + # 🔴 SECOND NARROW ANCHOR, added 2026-08-30 on the Decoder's advice, and it + # repairs a weakness I had already identified and not acted on. The heading + # anchor is on the CONCLUSION; when they corrected the run's item names -- + # `TUTORIAL → EXTRAS → EXTRAS` was actually `NEW GAME → TUTORIAL → TUTORIAL` + # -- this check sailed past it, because the conclusion was above the part + # that was wrong. It survived by luck, not by design. + # + # So the check now also rests on the EVIDENCE: the ring at y 384.0 before the + # round trip and 385.5 after. That pair is the geometry-free equality the + # conclusion actually stands on, and it is what a future correction to the + # measurement would have to touch. + # + # Two narrow anchors, NOT one loosened one. Their words: after a specific + # instrument fails the general one feels safer, and its failure mode is only + # one you have not met yet. + evidence = bool(re.search(r"ring sits at y 384\.0 before the round trip and 385\.5 after", h)) + want = heading and evidence got = (((jload("authored/flow.json") or {}).get("screens") or {}) .get("main_menu", {}).get("focus_persists")) + if heading != evidence: + print(f" {'menu remembers its cursor':<30} 🔴 ANCHOR SPLIT -- heading" + f" {heading}, evidence {evidence}: one moved without the other") + globals()["FAIL"] = FAIL + 1 + return report("menu remembers its cursor", want or None, got, want and got is True) @@ -367,6 +389,11 @@ CONTROLS = [ (check_splash_dwell, "the splashes are 190 and 145", "the splashes are 191 and 145"), (check_focus_persists, "the main menu remembers its cursor; re-entry is not a reset", "the main menu forgets its cursor; re-entry is a reset"), + # The SECOND anchor gets its own known negative. Perturbing only the evidence + # must trip ANCHOR SPLIT -- otherwise the second anchor is decorative and the + # check is still resting on the conclusion alone. + (check_focus_persists, "ring sits at y 384.0 before the round trip and 385.5 after", + "ring sits at y 384.0 before the round trip and 999.9 after"), # The list sits on the line AFTER "times are", so the perturbation has to # carry the newline the check's `\s*` spans. A control whose own anchor is # written from memory of the prose rather than from the prose is the same