fabi/Sylpheed
1
0
Fork 0
Code Issues 9 Pull Requests Actions Packages Projects Releases Wiki Activity

port: a leak that was not mine, a second narrow anchor, and a result recovered

Browse Source 
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N7FiFFFwbvG2uxdcEh8HyF
This commit is contained in:
Sylpheed port agent
2026-08-30 22:55:40 +00:00
parent 099f3adfc3
commit 01b0c9b10d
3 changed files with 147 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
docs/port/BLOCKED.md
View File

@@ -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 |

114
docs/port/DECISIONS.md
View File

@@ -9,7 +9,7 @@ dies, which is what this file is for.
<!-- INDEX: generated by tools/port/index-decisions -- do not hand-edit -->
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)
<!-- /INDEX -->
## 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.

29
tools/port/contract-check
View File

@@ -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