Ganze Seite ausblenden (Tombstone-Revision)

[usekaneo]

Es gibt keine Möglichkeit, eine Seite zu entfernen. Vorgesehen: eine neue Revision mit Tombstone-Tag, ergänzt um das Aufräumen der Platzierung.

Aufgabe: Tombstone-Tag in src/nostr/kinds.ts definieren, Revision veröffentlichen, Seite aus Baum, Suche und Navigation nehmen, aber History erreichbar lassen (Wiederherstellen bleibt möglich). Verwaistes 31818-Placement mitentfernen.

Quelle: docs/05-versioning-history.md „Open", docs/02-data-model-events.md („nichts räumt die Platzierung einer gelöschten Seite auf")

---

_{Task: zuupnwo51jvnarazhfxv85np}

[usekaneo]

CON-11

[usekaneo]

**** commented:

Technical refinement

Status: ready to implement. The key design point is that a tombstone is a normal revision with a tag, not a deletion — so the page disappears from tree, search and navigation while its history stays intact and a restore brings it back.

What exists today

• A page is (group, slug) plus its parent-rev chain; the head is a leaf (src/domain/pages.ts:42-86). Every page with at least one revision is in pages, tree and therefore the sidebar, the overview list and search.
• parseRevision (src/domain/revision.ts) reads only the known tags; anything unknown is ignored.
• Moves live in 31818 (src/domain/placement.ts, src/nostr/publish-placement.ts), addressable per (pubkey, slug). docs/02 already notes: 'nothing cleans up the placement of a deleted page — a 31818 whose slug has no revisions is ignored, so it is inert'.
• HistoryView.restore publishes a normal revision with parent-rev = current head; it does not depend on the head being 'live'.
• Docs/05 currently suggests hiding a page with a tombstone plus 9005 on the predecessors — that contradicts 'history stays reachable' and is superseded by this refinement.

Approach

1. Tag. Define TOMBSTONE: 'tombstone' in TAGS (src/nostr/kinds.ts), value '' (presence is the signal, like the group flags). Parse it in Revision as tombstone: boolean.
2. Publish. Add publishTombstone(signer, input) (or a tombstone: true flag on publishRevision): a new 1818 for the same (h, d) with parent-rev = current head, the tombstone tag, the page title retained, empty content, and a summary like 'page removed'. It appends to the chain — nothing is overwritten, so restore on any earlier revision already brings the page back without a special code path.
3. Page state, not deletion. buildPages keeps tombstoned pages in the returned Page[] but sets removed: true (plus removedAt from the head). Filtering happens at the consumers:
   • buildTree skips removed pages (their children then hang at the top level, which buildTree already does for a missing parent);
   • the SpaceOverview page list filters pages.filter((p) => !p.removed);
   • SearchView / the search input excludes removed pages;
   • /s/:group/:slug renders a 'This page was removed' notice with links to History and Restore the last version, instead of the content;
   • History ignores the removed flag entirely, so the chain and restore stay reachable.
     Keeping the page in pages is essential: without it HistoryView could not find the slug and the restore path would be unreachable.
4. Restore un-tombstones for free. A restored revision has no tombstone tag, so it becomes the new head and removed falls back to false. No additional flag or event.
5. Slug reuse stays refused. The editor's existing 'slug already in use' check must treat a removed page as still owning its slug — creating a new page with that slug would graft a new chain onto the old one. The way through is the removed-page notice (restore or history).
6. Placement cleanup, best effort. The store keeps only the winning placement per slug, so a full audit needs a read of all 31818 for that slug (one per author + possibly stale ones). Realistic behaviour:
   • if the viewer authored the winning placement, send a NIP-09 kind 5 for it (CON-10's helper); admins may send 9005;
   • other authors' placements cannot be removed by anyone but their author or an admin; they stay and are inert because the page is filtered out. Say that, do not pretend the cleanup is complete.
7. Permissions. Requirement 4 means every member may edit any page, so any member may tombstone one. The action needs a confirmation; it must be clear that it is reversible from History.

Subtasks

• TAGS.TOMBSTONE, Revision.tombstone, Page.removed / removedAt
• publishTombstone (or a flag on publishRevision)
• Filter removed pages in tree, overview list, search and sidebar; keep them in pages
• Removed-page notice on the page route with History / Restore actions
• Best-effort placement cleanup via CON-10's deletion helper / 9005
• Editor slug-collision check treats removed pages as occupying the slug
• Tests: buildPages removed flag, buildTree filtering, search exclusion, restore un-tombstone
• Docs: docs/02 (tag, remove the 'nothing cleans up the placement' open item), docs/05 (replace the 9005-predecessors suggestion), NOSTR.md tags table

Acceptance criteria

• Removing a page takes it out of the sidebar, the space overview and search, and its route shows a removed notice instead of content.
• Its full history is still listed and a restore from any earlier revision makes it a normal page again.
• Creating a new page with the removed slug is still refused, with a way to the removed page.
• No predecessor is deleted; the chain never breaks.
• The UI says plainly that the removal is reversible and that a placement by another author may remain (but is ignored).

Risks / open decisions

• Deleting predecessors (as docs/05 suggested) is explicitly rejected here: it would make the history unreachable, which the ticket forbids. The doc must be corrected.
• A tombstone by one member over another's work is allowed by requirement 4 and is reversible, but it is a social action; the confirmation should name the head's author and time so it is not done blindly.
• Placement cleanup across authors needs a broader read than the store currently holds; decide whether the first iteration does it only for the viewer's own placement (recommended) or adds a #d-scoped read.
• Interplay with CON-10: deleting the tombstone revision itself would un-remove the page; the two features must share the deletion-set handling.

Depends on / relates to

CON-10 (deletion helper, shared deletion set), CON-8 (cache must reflect the tombstone), CON-21 (placement cleanup), CON-14 (navigation).

[usekaneo]

**** commented:

Reviewed in worktree .worktrees/con-11 (feat/con-11-hide-whole-page @ 2c278af). Verification run for this review on that worktree: npm run typecheck, npm run lint, npm test, npm run build — all four pass.

Aggregate-review note. CON-11 and CON-21 were reviewed together and both are open. The orphaned-31818 cleanup that CON-11's ticket text lists is implemented in the CON-21 branch (publishPlacementDeletion, the orphan panel in PageTreeMaintenance), so Finding 1 below is a cross-ticket bookkeeping point rather than a missing deliverable: the criterion is met across the pair. The reason this branch is not approved as-is is Finding 2, the stale-closure error path.

Code review — CON-11: "Ganze Seite ausblenden (Tombstone-Revision)"

Verdict: ❌ Changes requested

Summary

CON-11 adds a tombstone tag to kind 1818 and derives a new Page.hidden flag from the head revision, then filters hidden pages out of the sidebar tree, local search and the space overview while keeping them in space.pages so the history and the page's own URL still resolve. The UI exposes hide/restore from the foot of the history view, and a later revision without the tag brings the page back. The domain work is small, well-factored and covered by tests. Two things block a clean approve: the explicit acceptance criterion to clean up the orphaned 31818 placement is deliberately not implemented (the docs were rewritten to argue the placement should stay), and the new hide/restore error path reads a stale closure value, so a failed hide shows no message and a later success shows the previous failure.

Acceptance criteria

• tombstone tag defined in src/nostr/kinds.ts — TAGS.TOMBSTONE at src/nostr/kinds.ts:106; written as ['tombstone', '1'] at src/nostr/publish-page.ts:68; parsed by presence, not value, at src/domain/revision.ts:67.
• Publish a revision — useHidePage signs a normal 1818 on top of the current head carrying the tag (src/ui/hide-page.ts:52-65), with the same content, parentSlug, order and a summary.
• Page out of tree, search and navigation — buildTree skips hidden pages (src/domain/pages.ts:180); searchPages skips them (src/domain/search.ts:45); SpaceOverview lists space.pages.filter((page) => !page.hidden) (src/routes/SpaceOverview.tsx:51). The sidebar is fed by space.tree (src/ui/layout/Sidebar.tsx:159).
• History reachable, restore still possible — the hidden page stays in pages (src/domain/pages.ts:134-143), HistoryView finds it by slug (src/routes/HistoryView.tsx:51), shows the tombstone state and a "Bring the page back" action (:272-289), and any later revision without the tag un-hides it (src/domain/pages.ts:142).
• Remove the orphaned 31818 placement — NOT implemented. See Finding 1.

Findings

Must fix before merge

1. blocker — The "orphaned 31818 placement" acceptance criterion is not delivered; the docs instead argue against it.
src/ui/hide-page.ts:19-24 states the decision outright:

* **The placement (31818) is deliberately left alone.** ... The genuinely orphaned case, a placement whose slug has no revisions at all, is a different problem and belongs to CON-21.

docs/05-versioning-history.md:121-125 repeats it. There is no placement-removal path anywhere: src/nostr/publish-placement.ts:23-53 only publishes, SpaceSnapshot does not even expose placements (src/nostr/space-store.ts:23-33), and docs/02-data-model-events.md:259-260 — the source the ticket cites — still reads:

- **Open:** nothing cleans up the placement of a deleted page. A 31818 whose
slug has no revisions is ignored, so it is inert rather than harmful.

Why it matters: the ticket's Aufgabe lists "Verwaistes 31818-Placement mitentfernen" as part of the job, and the change closes the linked Open item neither in code nor in docs/02. Reinterpreting a criterion away in a new doc paragraph is not the same as delivering it; if the position should be preserved for restores that is a plausible design, but it needs the ticket owner's sign-off, not a silent substitution.
Suggested fix: either (a) implement it — expose the winning (and, for correctness across authors, all) placement event id(s) for a slug from SpaceStore, and on hide publish a NIP-09 kind: 5 request for them, accepting that a later restore falls back to title order; or (b) get the ticket amended to drop the criterion, and update docs/02-data-model-events.md:259-260 so the Open list matches reality.

2. major — HistoryView.toggleHidden reads hide.error from a stale closure, so failures are not shown and successes can show the previous error.
src/routes/HistoryView.tsx:210-211:

const ok = await hide.setHidden(page, !page.hidden)
if (hide.error) setError(hide.error)

hide is the object returned by the render that created this closure. setHidden writes the hook's error via setError (src/ui/hide-page.ts:69-73, 76), which triggers a re-render but does not mutate the object this closure already holds. So on the first failure hide.error is still null and no message reaches the "That did not work" callout (:256-262); on a later attempt that succeeds, the previous failure string is copied into error while the success notice is set too (:212-217), so an error and a success are shown together. The established pattern is the opposite: useMovePage's error is rendered directly in Sidebar.tsx:359.
Suggested fix: render the hook's error directly, e.g. const shown = hide.error ?? error in the callout and clear it with hide.setError(null) in the button handlers; or change setHidden to return { ok, message } and set local state from that.
Coverage note: no test exercises useHidePage or HistoryView's toggle at all (src/ui/hide-page.ts has no referencing test), which is why this slipped through.

Nice to have

3. minor — The hide success notice is rendered under the "Deletion requested — not a guarantee" callout title.
src/routes/HistoryView.tsx:264-270 renders every notice under the fixed title "Deletion requested — not a guarantee". Both the NIP-09 path (:132-135) and the new hide path (:213-216) share that state, so after hiding the user reads "The page is out of the tree, the search and the overview…" under a title that asserts a deletion — exactly the framing docs/05-versioning-history.md:100 and src/ui/hide-page.ts:15-17 are built to avoid.
Suggested fix: carry a title with the notice (a small { title, body } object) or give the two flows separate notice state.

4. minor — SearchView still counts hidden pages in its totals.
src/routes/SearchView.tsx:60 ({hits.length} of {space.pages.length} pages) and :82 (Nothing found in ${space.pages.length} pages.) use the unfiltered list, while searchPages skips hidden pages (src/domain/search.ts:45). The denominator is inflated by exactly the pages that can never be returned.
Suggested fix: filter as SpaceOverview.tsx:51 does, or expose a visible count.

5. nit — A lifted subpage still shows its hidden parent in the breadcrumb.
PageView.tsx:24-36's ancestors() walks space.pages unfiltered, so a child that buildTree lifted to the top level (src/domain/pages.ts:180) still renders a crumb to the hidden page — contradicting "subpages come up to the top level" (docs/05:117-120, docs/06:154-159) and the sidebar. The link still works, so this is presentation only.
Suggested fix: filter hidden pages out of ancestors, or state the intent in a comment.

6. nit — tombstone now names two different concepts.
src/nostr/space-store.ts:339 already used "tombstone" for a NIP-09 deletion request ("the tombstone is re-derived from the relay"), and TAGS.TOMBSTONE now means a revision that hides its page. The pre-existing wording is unchanged by this diff, but the collision is now real.
Suggested fix: rename the NIP-09 comment/usage (e.g. "the deletion") so one word maps to one concept.

Tests

The added tests are meaningful and would fail against a dummy implementation: src/domain/pages.test.ts:369-422 pins head-derived visibility, fork behaviour and subpage lifting; src/domain/revision.test.ts:63-81 pins presence-over-value parsing including ['tombstone', ''] and the negative case; src/domain/search.test.ts:79-88 pins search exclusion over a real buildPages result; and src/nostr/publish-page.tombstone.test.ts round-trips through the real signer and the real parser, which is the claim that matters most.

Gaps:

• No test at all for src/ui/hide-page.ts, hideConfirmation, or childSlugs (Finding 2 is invisible to the suite).
• The search test only asserts []; it does not assert that the same page is found when the tombstone is absent, so a regression that always returned [] would pass this particular case (the other search tests would catch it, but the case itself is weak).
• The deliberate "placement survives a hide" decision — the crux of Finding 1 — is not pinned by any test, in either direction.

Docs

Good coverage: docs/05-versioning-history.md:98-125 (rewritten from "Open"), docs/06-ui-information-architecture.md:154-159, NOSTR.md's tag table and the roadmap entry removed from docs/10-roadmap.md. The prose matches the code for the tree, search, overview, head-decides and subpage rules, and the "it is not a deletion" framing is accurate and consistent.

The exception is docs/02-data-model-events.md: it was not touched, and its placement Open item (:259-260) is the ticket's source. The new docs/05 paragraph and docs/02 therefore describe two different policy stances on the same problem; that inconsistency is Finding 1.

What is good

• Deriving hidden from the head in one place (src/domain/pages.ts:142) means hiding inherits the existing fork rule instead of introducing a second visibility model; the fork test shows the intent and the result is predictable.
• Filtering inside searchPages rather than at the call site (src/domain/search.ts:40-45) is the right seam, and the comment explains why the page list stays unfiltered.
• Keeping the full page in space.pages is what makes "URL and history still work" true and also keeps spaceAccess's empty-space heuristic correct (src/domain/space-access.ts:62).
• Carrying the head content and the current placement fields into the tombstone revision is considerate: restore is lossless and does not move the page (src/ui/hide-page.ts:52-65).
• The copy is honest in both directions — confirmation and both callouts say plainly that nothing is deleted, the link still works and subpages stay — which matches the spirit of docs/09-security-privacy.md.
• Test quality is high where it exists: the publisher/parser round trip is the sort of test that would catch a tag-name drift, and the presence-not-value parsing is explicitly pinned.

[usekaneo]

**** commented:

Review response — CON-11

Worked in .worktrees/con-11 (feat/con-11-hide-whole-page, on top of 2c278af). Everything is left uncommitted and unstaged in the working tree for review.

Verification (from that worktree):

npm run typecheck   # pass (tsc --noEmit, no output)
npm run lint        # pass (eslint ., no findings)
npm test            # 50 files, 489 tests — all pass
npm run build       # pass, built in 28.23s

One caveat, stated rather than hidden: the first npm test run failed one test — src/ui/editor-slash-menu.test.tsx > inserts the table skeleton when the first entry is accepted, Test timed out in 5000ms. It passes alone and passed on the immediately following full run (489/489). That run reported ~5.66s of worker startup per file against ~2.98s on the clean run, so it is the default 5s timeout losing to machine load, not a regression — the file is untouched by this change and nothing in the diff reaches the editor. Worth knowing that it can flake under parallel load.

---

Finding 1 — orphaned 31818 placement not removed (blocker)

Verified, and the substance holds: not delivered on this branch. src/nostr/publish-placement.ts only publishes, SpaceSnapshot (src/nostr/space-store.ts:23-33) exposes no placements, and there is no placement-deletion path anywhere in the branch.

Not changed — reported as an open decision, per the reviewer's own framing. The aggregate-review note says the cleanup is implemented in the CON-21 branch (publishPlacementDeletion, the orphan panel), so this is a choice between two designs — (a) delete the placement on hide and accept that a restore lands the page at title order, or (b) keep the position and let CON-21 own orphan cleanup — and the branch argues (b) explicitly in src/ui/hide-page.ts:19-24 and docs/05-versioning-history.md:121-125. Picking between them is the ticket owner's call, not a fix I should make unilaterally. Decision needed: amend the ticket to drop the criterion (CON-21 covers the orphan case), or ask for deletion-on-hide here.

One claim in the finding does not hold up. The review says docs/02 and docs/05 "describe two different policy stances on the same problem". They describe two different problems. docs/02:259-260 is about a placement whose slug has no revisions at all ("nothing cleans up the placement of a deleted page"); docs/05's new paragraph is about a page that still has its full revision chain and is merely hidden, and it closes by handing exactly the no-revisions case back to docs/02 ("The genuinely orphaned case — a placement whose slug has no revisions at all — is a different problem ([02])"). So the two texts are consistent; what is true is the simpler point, that the ticket's Aufgabe lists the cleanup and this branch does not do it.

I deliberately left docs/02-data-model-events.md:259-260 untouched: whether that Open item can be closed depends on the decision above, and CON-21 is live in a sibling worktree and is the change that would legitimately close it. Editing those two lines from here would conflict for no gain.

Finding 2 — toggleHidden reads hide.error from a stale closure (major)

Verified and fixed. The handler held the hide object from the render that created it; setHidden reports by calling the hook's setError, which produces a new object rather than mutating that one, so hide.error after the await was always the previous attempt's value.

Fixed in src/routes/HistoryView.tsx along the established pattern (the sidebar renders useMovePage's error directly):

• the stale read if (hide.error) setError(hide.error) is gone, replaced by a comment explaining why it must not come back;
• the callout renders const shownError = error ?? hide.error (:85), so the hook's error reaches the screen on the render it is set in;
• a new clearFeedback() (:71-80) clears local error, notice and hide.setError(null), and all four actions (removeRevision, requestDeletion, restore, toggleHidden) start with it — so only one of the two sources can be set at a time.

Regression tests: src/routes/HistoryView.test.tsx (new). Two cases pin both symptoms — a failure showing no message, and a success carrying the previous failure's message. Both were checked against the unfixed code by temporarily reverting the fix: they fail there (test 1: nothing rendered; test 2: received text contained both "The relay does not allow you to write here: blocked: not a member" and the success notice) and pass with it.

Finding 3 — hide notice rendered under a "Deletion requested" title (minor)

Verified and fixed. Notice state is now { title, body } (HistoryView.tsx:35-38); the NIP-09 path keeps "Deletion requested — not a guarantee", the hide path gets "The page is removed — and can come back". Pinned by a third case in HistoryView.test.tsx ("does not announce a hidden page as a deletion"), which also fails against the unfixed code.

Finding 4 — SearchView counts hidden pages in its totals (minor)

Verified and fixed. src/routes/SearchView.tsx:50 derives searchable = space.pages.filter((page) => !page.hidden) and both the "x of y pages" line (:66) and the "Nothing found in y pages" message (:88) use it. The full list still goes into searchPages, which owns the filter — the seam the review rightly praised is untouched.

Finding 5 — a lifted subpage still shows its hidden parent in the breadcrumb (nit)

Verified and fixed — cheap and clearly right, since it is the same tree described two ways on one screen. ancestors() in src/routes/PageView.tsx now breaks on parent.hidden as it already did on a missing parent, matching what buildTree does.

Test: src/routes/PageView.crumbs.test.tsx (new) — the parent shows while visible, and the trail stops at a hidden parent. Verified to fail without the fix (expected 'Engineering / Handbook / Onboarding' to be 'Engineering / Onboarding').

Finding 6 — "tombstone" names two concepts (nit)

Verified and fixed (comments and prose only, no behaviour). "Tombstone" now means only TAGS.TOMBSTONE, the page-hiding revision; the NIP-09 deletion path says "deleted"/"removal": src/nostr/space-store.ts:336-345 (with a note saying why the word is avoided there), src/nostr/space-store.test.ts:387, docs/09-security-privacy.md:30, docs/05-versioning-history.md:93, NOSTR.md:76.

Test gaps from the review

• useHidePage / hideConfirmation / childSlugs untested — addressed. HistoryView.test.tsx exercises the hook through the real UI, and src/ui/hide-page.test.ts (new) pins the confirmation's two load-bearing claims (nothing is deleted; the subpages stay, counted, singular and plural, and silent at zero) over a real buildPages/childSlugs result.
• Search test could not fail — addressed. src/domain/search.test.ts now asserts both directions over the same two revisions: with tombstone: false the page is returned, with tombstone: true it is not. An implementation that always returned [] now fails this case on its own.
• "placement survives a hide" not pinned — deliberately not added. That is the crux of Finding 1; a test in either direction would cement an undecided policy and would have to be deleted if the decision goes the other way. Happy to add it the moment the decision above is made.

---

Files changed (all unstaged): src/routes/HistoryView.tsx, src/routes/SearchView.tsx, src/routes/PageView.tsx, src/nostr/space-store.ts, src/domain/search.test.ts, src/nostr/space-store.test.ts, docs/05-versioning-history.md, docs/09-security-privacy.md, NOSTR.md; new: src/routes/HistoryView.test.tsx, src/routes/PageView.crumbs.test.tsx, src/ui/hide-page.test.ts.

[usekaneo]

**** commented:

Merged into main as eb8f715 (#94).

Tested in the browser by the user before the merge — archiving, the
confirmation dialog, the archive entry point in the sidebar and the shortened
texts.

Gates pass on each of the two commits individually, not just on the tip:
typecheck, lint, test (48 files / 468 tests), build. CI green on the PR.

What landed

• Archiving is a revision, not a state: a 1818 on top of the head carrying
  an archived tag, content and placement travelling along unchanged.
  Restoring is the same publish without the tag, so the history stays complete
  on both sides and an archived page can still be read, compared and blamed.
• The placement (31818) is deliberately left alone, so a restore lands the
  page where it was rather than where its title sorts to.
• Subpages do not go with the parent — they move up to the top level, and the
  confirmation says so, counted.
• /s/:group/archive with a row under the page tree (and a count), the space
  overview linking there, and one button per row to bring a page back.
• window.confirm is gone. ConfirmDialog wraps a native <dialog>; jsdom
  implements none of its behaviour, hence the shim in src/test.

Deliberately not done

• No test for ArchiveView.tsx. Its logic lives elsewhere and is covered
  there: archivedPages in pages.test.ts (filter, order, tie-break) and the
  restore path in HistoryView.test.tsx. What is left is markup. The one gap
  this leaves — nothing checks that the view calls archivedPages at all — is
  noted here rather than papered over with a render test.
• The branch carries no NIP-09 deletion code. CON-10 stays on its own branch.

Follow-ups filed

• CON-50 — a page whose slug normalises to archive, new or search is
  shadowed by the app's own route. Older than this branch; new and search
  have had it since they were added.
• CON-51 — AGENTS.md should require that a branch does not change the same
  thing twice. This branch was rebuilt from its final tree before the merge:
  four commits where twenty files were touched two or three times became two
  commits where every file appears exactly once, same tree.

Still open elsewhere: #87 (the old "hide with a tombstone" PR) is
superseded and #88 (CON-21) is still based on its branch, carrying both the
old CON-11 implementation and the abandoned CON-10 commits. Both need a
decision; neither was touched here.