mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-21 18:13:26 +00:00
xarmian
a2195997f7
* fix(web): an entry missing from a refreshed first page is not necessarily deleted (BUG-2773) The SSE refresh re-fetches the FIRST page — the newest N entries — and treated anything previously on it and now absent as deleted. Once enough newer entries exist, a perfectly alive entry rolls off that window, and it disappeared from the reader's view; for anyone who had pressed Load More it vanished from the MIDDLE of a timeline whose neighbours on both sides were still shown. A full reload brought it back, which is the tell that this was display state and not data. Per the lead's ruling (option 1): deletion is inferred only for a position the fresh page still COVERS — at or newer than its oldest entry, compared in the same (created_at, id) space the server's cursor uses. Anything older is out of window and left alone. An empty fresh page covers nothing and so deletes nothing: it means every row in that window was unrenderable, not that the history was erased. Every test leg pairs a roll-off with a real deletion, because a fix that simply stopped removing anything passes the roll-off half alone. The refresh helper asserts the refresh actually FIRED — the first version of these tests waited 400ms against a 500ms debounce, and one leg passed vacuously on a refresh that never happened. Mutation matrix, each detected by its own leg: the old rule (3 legs); string comparison instead of instants at the boundary (the sub-second leg); an empty fresh page covering everything; the id tie-break dropped. Refs: BUG-2773 Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X * fix(web): a final page covers everything, and coverage replaces the first-page gate (codex round 1) Two findings, both real. has_more was ignored. A refresh whose page is FINAL returned the whole history, so an entry the client holds and that page does not contain has nothing to have rolled off into — it is gone, deleted or no longer renderable. Coverage now extends to everything on a final page, which is also what makes an empty FINAL page clear the view while an empty page with more behind it still deletes nothing. Both directions have a leg; asserting either alone would let "empty always clears" or "empty never clears" pass. firstPageIds is retired rather than repaired. It tracked the last first-page fetch so older-page entries would not be judged by a first-page comparison — which coverage now does directly and better: an older-page entry sits below the floor and is left alone, while an entry INSIDE coverage that the page does not contain is gone regardless of which page delivered it. Keeping both would have leaked: an entry preserved as a roll-off dropped out of the tracked set, so a later window expanding back over it could never remove it again. Mutation matrix, six mutations, each detected: no final-page rule (2 legs); the old missing-means-deleted rule (3); never deleting (5); string comparison at the boundary; the id tie-break dropped; an empty non-final page treated as covering. Refs: BUG-2773 Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X * fix(web): fence overlapping SSE refreshes, and state what inference cannot do (codex round 2) Only the newest dispatched refresh may write. The item/workspace check catches a switch but not two refreshes of the SAME item in flight at once — the retry path fires 2s after a failure while a newly debounced one is already running — and an older response landing last re-adds an entry the newer one removed or removes one it added. A monotonic seq, a plain let rather than $state because it is read and written inside the refresh (CONVE-1688). Its test holds both in flight and resolves them out of order; it fails with the fence removed. Two other round-2 findings are DOCUMENTED, not fixed, because they are limits of inferring deletions from a first-page comparison rather than defects of this change, and the lead ruled the event-based alternative out of scope for this unit: - an entry deleted below a non-final page's floor is never inspected again and stays until a reload. The old rule removed it — by removing every rolled-off entry with it, which is the bug being fixed. Strictly better, not complete. - whether a row renders depends on the window it was fetched in, since the cross-source drops need both rows in one fetch, so an entry that rendered on an older page can be absent-and-covered here. That matches what a fresh load shows, which is the ceiling for any first-page comparison. The fourth finding — the refresh not adopting has_more/next_cursor — is declined a second time, on the same grounds and now with its concrete harm checked: after paging to the end the reader already HOLDS everything behind page one, so a fresh has_more=true strands nothing. Refs: BUG-2773 Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X * test+docs: a leg that fails against the OLD rule, and three claims corrected (codex round 3) The two final-page legs passed against main: the old gate removed any entry missing from a refreshed first page unconditionally, so they discriminated against an intermediate version of this fix and not against the behaviour it replaces. Added the leg that does — an entry loaded via LOAD MORE, which the old gate preserved unconditionally because it had never been on a first page, and which a final refreshed page must now remove. Verified by restoring the pre-fix rule: that leg fails, along with three others. Two comments were false and are corrected rather than softened: - Sub-second timestamps are NOT reachable from this server. The store writes RFC3339 seconds and the handler truncates the structured kinds' hand-written ones to match (handlers_timeline.go's stamp()). The instant-vs-string comparison is a guard on what the ordering MEANS, not a live scenario, and both the code and the test that pins it now say so. The claim came from the BUG-2765 unit and was wrong there too. - Comment-linked activities are excluded by the store's SQL whether or not the comment is in the window, so they are not an example of window-dependent rendering. The version-coincidence suppression is; the note names that one now. Refs: BUG-2773 Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X * fix(web): a stale Load More page cannot resurrect a deleted entry (codex round 4) A paging request can be in flight while a refresh removes an entry as deleted. Its page is older than the refresh and still carries that row, so appending it verbatim put the entry back on screen — visibly undoing a deletion the reader had already seen happen. The refresh now records what it removed, and Load More filters its page through those tombstones. Chosen over discarding the whole stale page (the reader's click would do nothing) and over a generation fence (same). The set is cleared whenever loadTimeline resets the view, so it is bounded by one mount's deletions. Its test holds the paging response, deletes the entry via a refresh in between, then lands the stale page and asserts both halves: the deleted row does not come back AND the rest of the page still does — without the second, discarding everything would pass. Fails with the tombstone check removed. Refs: BUG-2773 Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X * fix(web): discard a stale Load More page instead of tombstoning what it may resurrect (codex round 5) Round 4's tombstone set answered the right question with the wrong machinery, and round 5 found the bill: it grew for the life of a mount with no prune, its clear point raced older paging continuations (a same-item mutation calls loadTimeline, clears the set, and a stale page can then resurrect a deleted entry anyway), and it would suppress a same-id structured entry legitimately rewritten. Each of those is fixable; together they are a sign the mechanism was too clever for the race it guards. Replaced with the fence already used for overlapping refreshes: loadMore captures the refresh sequence before its await and discards the response if a refresh applied in between. One integer, no growth, no lifetime, nothing to clear at a switch. The page is dropped WHOLE rather than filtered — it was assembled before the deletion and nothing in it reflects the current view — and the cursor is untouched, so Load More is still offered and the next click fetches the same page against the current state. The test now asserts all three halves: the deleted entry does not come back, the rest of the stale page does not land either, and the button is still there. Without the third that would read as a silent drop rather than a deliberate one. Fails with the fence removed. Refs: BUG-2773 Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X * fix(web): one view generation for reloads as well as refreshes (codex round 6) The fence only counted SSE refreshes, so the path that most obviously replaces the view did not advance it: a local comment delete calls loadTimeline, page 1 comes back without the entry, and a Load More page in flight from before that re-adds it. Identity and sequence both pass, because neither noticed. One counter now, incremented by every reload and every refresh, captured by every continuation that writes entries — including loadTimeline itself, which could otherwise overwrite a newer view with an older page-1 response. "The view was replaced" is the same fact whichever path replaced it. Tested through the harness-reachable form: a Load More page held across a switch AWAY and BACK. By the time it resolves the identity check passes again — same item — and only the generation can tell that the view it was fetched against has been replaced twice. Fails with the reload's increment removed. The local-delete path itself is not directly driven: it needs the comment controls, which need mutationsEnabled plus canEditItem plus the confirm flow. It goes through the same single increment as the switch case, but that is an argument, not a test, and this note is here so nobody reads the coverage as wider than it is. Refs: BUG-2773 Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X * fix(web): count view replacements that LANDED, and fence the cleanup paths (codex round 7) Two regressions my own fence introduced. The generation advanced at DISPATCH, so a refresh that then FAILED still invalidated an in-flight reload: the reload's good response was discarded and the view stayed empty until something else redrew it. It now advances where the write lands — a request that never writes is not a replacement. Its test holds the initial load, fails a refresh across it, and asserts the load still renders; it fails when counting goes back to dispatches. The catch and finally only checked identity, so a stale request's cleanup could clear the spinner a newer one owns or restore an old error over a current load. Both are gated on ownership now. Ownership needed a flag rather than a bare reqGen === viewGen: the writer advances the generation ITSELF, so after writing it reads as stale by its own test — the first version of this blocked its own `loading = false` and left an empty page under a permanent spinner. Caught by two existing tests failing, not by review, which is the instrument working. (This message is a re-write: the first one was passed through a double-quoted shell string and the backticked span was executed and blanked — CONVE-13, the convention that exists because of exactly this. Caught by re-reading the artifact rather than the success line, which is the other half of it.) Refs: BUG-2773 Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X * fix(web): ticket plus high-water mark, so newest wins and a failure costs nothing (codex round 8) One counter could not express both halves, and each single-counter version failed a different way. Counting DISPATCHES let a refresh that then failed invalidate an in-flight reload (round 7). Counting APPLIES let an OLDER response landing first claim the view and lock the newer one out behind it (round 8) — the mirror of the case the out-of-order test already covered, and the reason that test alone was not enough. Every view-replacing request now takes a unique increasing ticket at dispatch and may write only if it beats the high-water mark of what has actually written, which it then owns. Newest wins among concurrent responses, and a request that never lands costs nothing. Second round-8 finding, same root: an overlapped load declines to clear the spinner once a newer write has landed, so the writer has to. Without it an empty refresh result sat under a permanent spinner and the list never rendered. Mutation checks, each with its anchor count asserted after two mutations silently failed to apply and left a GREEN run that proved nothing: the refresh not clearing the spinner fails the new spinner leg; the round-7 applied-counter semantics fail the new newest-wins leg; the loadMore fence removed fails the switch-away-and-back and stale-page legs. Refs: BUG-2773 Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X * fix(web): a reload clears loadingMore too (codex round 9) loadMore's cleanup is identity-guarded, so a page resolving while the reader is on another item never cleared the flag. Coming back found Load More permanently disabled — a dead control, which reads as "there is nothing more" rather than as a bug. Pre-existing (the identity guard is TASK-2112's), and one line to close now that loadTimeline is already the single place paging state resets. The existing switch test released its stale page AFTER returning, which is the ordering that never exercised this; the new leg resolves it while away and asserts the button comes back enabled. Fails with the reset removed. Refs: BUG-2773 Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X * fix(web): the coverage boundary is the server's cursor, not the oldest row returned (codex round 10) How far back a page LOOKED and how far back it RETURNED rows are different positions, and BUG-2765 made the first one available: next_before is where the next page starts, so everything at or newer than it was examined. When one source exhausts its over-fetch window while another returns an older rendered row, the cursor sits NEWER than the page's oldest entry — and judging by the returned floor then treats a live entry from the exhausted source, one this page never reached, as deleted. Using the cursor makes the rule say what it means. The returned floor stays as the fallback for a server predating that field; it is the slightly-too-eager version, and still narrower than the rule this unit replaces. Two comments corrected rather than left: "the oldest entry the fresh page reached" was the oldest RETURNED, and "has_more=false means the server returned everything it has" ignored that rows are fetched and dropped as unrenderable — which is precisely why absence from a final page still means gone. The new leg builds the shape only a server can produce (cursor newer than the oldest returned row) and asserts both sides of the boundary: the entry below the cursor is kept, the one above it is removed. Fails with the boundary put back to the returned floor. Refs: BUG-2765, BUG-2773 Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X * docs: three of my own claims corrected, including one a commit message overstated (codex round 12) - The refresh's comment still said only the newest DISPATCHED refresh may write. Round 8 replaced that with ticket-versus-high-water, which lets an older response write first and be replaced by the newer one — deliberately, and with its own test. The comment described the scheme two rounds ago. - The final-page test still said has_more=false means the server "returned everything it has". It reached the end of the rows; some were dropped as unrenderable on the way, which is why absence from a final page still means gone. My round-10 commit claimed this wording was corrected — it was, in the component, and not here. The claim was true of half the sites and written as though it covered both. - The file header said every leg pairs a roll-off with a real deletion. That was true of the first leg and stopped being true as the legs accumulated; and the empty-final-page leg described "both refreshes" when it makes one. Both now say what the tests do. Refs: BUG-2773 Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X * fix(web): a superseded refresh does not retry (codex round 13) The retry exists so a transient failure does not leave the panel quietly stale (BUG-2508). Once a newer refresh has written the view there is nothing stale to repair: the retry is traffic for a question already answered, and its answer would arrive older than what is on screen. Guarded on the same high-water mark every other write path uses. Its test fails an older refresh after a newer one landed and asserts the request COUNT does not move across the retry backoff — the entries look identical either way, so the count is the only thing that distinguishes the two behaviours. Fails with the guard removed. Refs: BUG-2773 Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X