From c66f085681a241e8c03a7a526995b3ac5c47d969 Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Sun, 6 Sep 2026 16:06:17 +0200 Subject: [PATCH] refactor: deprecate the row rebuild, and write down what goes with it `assemble` reads `GroupSignedEvent` now and rebuilds from `Mantra*` rows only what that table does not hold, which is work signed before it existed. The rebuild is therefore on its way out rather than merely second in line, and this says so where a reader will actually meet it -- at the call site, from the compiler -- instead of only in a paragraph they have to find first. **Nine `@Deprecated` markers, and they are load-bearing as documentation.** The eight `toXEvent()` methods and `ArchiveManager.rebuiltEventsOf`, each carrying the same sentence: this is the fallback for pre-v13 work, read the event off the table instead, and it goes when the last such install does. That raises nine warnings in `commonMain` today, all of them inside the walk itself, so the deprecation is visible in every build without anything failing over it. The level is `WARNING` deliberately -- the code is still called, still correct, and still the only thing standing between an older room and an empty archive. **The checklist is a new section in docs/member-archive.md**, because the interesting part of this removal is not the eight methods, it is everything around them that is easy to take out by association or leave behind by accident. *What goes*: the walk and the version-label recovery inside it, the union in `signedEventsOf`, the eight rebuilds, and `ArchiveRoundTripTest` entire -- all ten cases, which exist to hold the rebuild up and cover nothing else. Its own header still opened with "signed events are not stored as events", which stopped being true two commits ago, so it now says what it is: the gate on a deprecated fallback, deleted with what it guards. *Two already-dead cousins to sweep at the same time*, named because they will look like part of the rebuild to whoever does the removal and are not: `MantraTranslation.toTranslationEvent`, which nothing has ever called, and `MantraTranslationChunkProposal.toTranslationChunkEvent`, on a model that is not even a `@Database` entity. *The tests that seed without recording*: in `ArchiveAssemblyJvmTest` the `apply`-only seeding **is** the rebuild path, and two of its cases are about the union specifically and mean nothing without it. `ArchiveApplyJvmTest` seeds its sender the same way but is testing delivery rather than assembly, so it needs the recording call *added* -- otherwise it quietly starts asserting against an empty archive, which is the same silent-success failure this whole feature is about. **What only looks like it goes, which is the half worth writing down.** The `isArchivable` filter in `signedEventsOf` is not part of the rebuild and becomes the only thing standing. It is there *because* of the record: the walk could only ever produce document kinds, so nothing needed filtering while it was the source, and the table holds every kind the group has signed -- starting with the `GroupKeyStateEvent` every room signs as its first act. Dropping it with the walk turns every room's archive into an `IllegalArgumentException` from `ArchiveEvent.build`. Two cases fail with exactly that if it goes, which is the guard against removing it by association rather than by decision. The verify filter in `assemble` stays too. With the rebuild gone it checks events that were verified before they were recorded, so it cannot fail in practice -- which is the argument for keeping it, not against. "Cannot happen" is the state it exists to preserve. `Mantra*.signature` and `Mantra*.publicKey` are explicitly *not* on the list. They were what made a row rebuildable, and since v13 `groupSignedEventId` says whether the group signed a row and points at the proof -- so they are arguably redundant. But four test files assert on them and `MantraTranslationContributor` builds a contributor list out of one, and it is a twelve-table migration with its own tests to rewrite. It should be decided on its own merits, not ride along. **The precondition cannot be checked, and the section says so plainly.** No query answers "does any install still hold pre-v13 work" -- a device that upgraded is indistinguishable from one that never had any, and the rows that need rebuilding are on other people's devices. What is observable is the `signedEventsOf` log line, which fires only when the rebuild actually contributed something; fleet-wide silence is evidence and not proof. The cost of getting it wrong is named as well, because it is not loud: the member keeps their own rows and reads the room normally, and only loses the ability to *answer* a request with the older half of the group's work -- so a newer member asks, is answered, and receives an archive that is quietly short. No behaviour change. 495 jvm tests and 297 android unit tests pass. Co-Authored-By: Claude Opus 5 --- .../compose/database/model/MantraArtifact.kt | 5 + .../database/model/MantraArtifactVersion.kt | 5 + .../compose/database/model/MantraChapter.kt | 5 + .../compose/database/model/MantraChunk.kt | 5 + .../compose/database/model/MantraDialect.kt | 5 + .../model/MantraTranslationArtifactVersion.kt | 5 + .../model/MantraTranslationChapter.kt | 5 + .../database/model/MantraTranslationChunk.kt | 5 + .../mantra/compose/managers/ArchiveManager.kt | 17 +++- .../nostr/archive/ArchiveRoundTripTest.kt | 30 +++--- docs/member-archive.md | 96 +++++++++++++++++++ 11 files changed, 170 insertions(+), 13 deletions(-) diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraArtifact.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraArtifact.kt index 38842414..77f31015 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraArtifact.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraArtifact.kt @@ -87,6 +87,11 @@ data class MantraArtifact( * wrong here until an archive needed to rebuild an artifact and nothing had * ever called this. */ + @Deprecated( + "Archive fallback for work signed before the GroupSignedEvent table. Read the " + + "event off that table instead; this rebuild goes when the last pre-v13 install " + + "does -- see the removal checklist in docs/member-archive.md." + ) fun toArtifactEvent(versionLabel: String): ArtifactEvent { return ArtifactEvent( id = id, diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraArtifactVersion.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraArtifactVersion.kt index 46c2abe8..3ad97588 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraArtifactVersion.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraArtifactVersion.kt @@ -66,6 +66,11 @@ data class MantraArtifactVersion( override val createdAt: Instant = Clock.System.now(), override val updatedAt: Instant = createdAt ): OptionalNostrEventEntity, TimestampedEntity { + @Deprecated( + "Archive fallback for work signed before the GroupSignedEvent table. Read the " + + "event off that table instead; this rebuild goes when the last pre-v13 install " + + "does -- see the removal checklist in docs/member-archive.md." + ) fun toArtifactVersionEvent(): ArtifactVersionEvent { return ArtifactVersionEvent( id = id, diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraChapter.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraChapter.kt index a12f015e..ca9f5920 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraChapter.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraChapter.kt @@ -71,6 +71,11 @@ data class MantraChapter( override val updatedAt: Instant = createdAt ): OptionalNostrEventEntity, TimestampedEntity { + @Deprecated( + "Archive fallback for work signed before the GroupSignedEvent table. Read the " + + "event off that table instead; this rebuild goes when the last pre-v13 install " + + "does -- see the removal checklist in docs/member-archive.md." + ) fun toChapterEvent(): ChapterEvent { return ChapterEvent( id = id, diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraChunk.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraChunk.kt index ab7439cf..fee9571a 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraChunk.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraChunk.kt @@ -67,6 +67,11 @@ data class MantraChunk( override val updatedAt: Instant = createdAt ): OptionalNostrEventEntity, TimestampedEntity { + @Deprecated( + "Archive fallback for work signed before the GroupSignedEvent table. Read the " + + "event off that table instead; this rebuild goes when the last pre-v13 install " + + "does -- see the removal checklist in docs/member-archive.md." + ) fun toChunkEvent(): ChunkEvent { return ChunkEvent( id = id, diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraDialect.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraDialect.kt index a7fef77a..359b9d3a 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraDialect.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraDialect.kt @@ -56,6 +56,11 @@ data class MantraDialect( override val updatedAt: Instant = createdAt ): OptionalNostrEventEntity, TimestampedEntity { + @Deprecated( + "Archive fallback for work signed before the GroupSignedEvent table. Read the " + + "event off that table instead; this rebuild goes when the last pre-v13 install " + + "does -- see the removal checklist in docs/member-archive.md." + ) fun toDialectEvent(): DialectEvent { return DialectEvent( id = id, diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraTranslationArtifactVersion.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraTranslationArtifactVersion.kt index 9b9be98c..109225a1 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraTranslationArtifactVersion.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraTranslationArtifactVersion.kt @@ -75,6 +75,11 @@ data class MantraTranslationArtifactVersion( override val updatedAt: Instant = createdAt ): OptionalNostrEventEntity, TimestampedEntity { + @Deprecated( + "Archive fallback for work signed before the GroupSignedEvent table. Read the " + + "event off that table instead; this rebuild goes when the last pre-v13 install " + + "does -- see the removal checklist in docs/member-archive.md." + ) fun toTranslationArtifactVersionEvent(): TranslationArtifactVersionEvent { return TranslationArtifactVersionEvent( id = id, diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraTranslationChapter.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraTranslationChapter.kt index 0cd2df48..41f5eee8 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraTranslationChapter.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraTranslationChapter.kt @@ -70,6 +70,11 @@ data class MantraTranslationChapter( override val createdAt: Instant = Clock.System.now(), override val updatedAt: Instant = createdAt ): OptionalNostrEventEntity, TimestampedEntity { + @Deprecated( + "Archive fallback for work signed before the GroupSignedEvent table. Read the " + + "event off that table instead; this rebuild goes when the last pre-v13 install " + + "does -- see the removal checklist in docs/member-archive.md." + ) fun toTranslationChapterEvent(): TranslationChapterEvent { return TranslationChapterEvent( id = id, diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraTranslationChunk.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraTranslationChunk.kt index b74380b5..7ed2b2a8 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraTranslationChunk.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraTranslationChunk.kt @@ -72,6 +72,11 @@ data class MantraTranslationChunk( override val updatedAt: Instant = createdAt ): OptionalNostrEventEntity, TimestampedEntity { + @Deprecated( + "Archive fallback for work signed before the GroupSignedEvent table. Read the " + + "event off that table instead; this rebuild goes when the last pre-v13 install " + + "does -- see the removal checklist in docs/member-archive.md." + ) fun toTranslationChunkEvent(): TranslationChunkEvent { return TranslationChunkEvent( id = id, diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/ArchiveManager.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/ArchiveManager.kt index a3659d44..9203a751 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/ArchiveManager.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/ArchiveManager.kt @@ -42,8 +42,10 @@ import press.mantra.compose.nostr.frost.GroupKeyStateEvent * signed; `ArchiveRoundTripTest` is what says it is, per kind, against a real * quorum, and it found two faults in the artifact's rebuild the first time it * ran -- both of which would have shipped payloads that every receiver drops as - * forgeries without a word. The fallback can go once no install still holds - * pre-v13 work. + * forgeries without a word. Both it and everything that exists only to hold it + * up are marked `@Deprecated`; the fallback can go once no install still holds + * pre-v13 work, and docs/member-archive.md's "Retiring the rebuild" is the list + * of what goes with it. * * **The allowlist does real work on the way out now.** The rebuild could only * ever produce document kinds; the table holds everything the group has ever @@ -662,7 +664,18 @@ object ArchiveManager { * * It returns everything it can rebuild and lets [signedEventsOf] and the * verify filter decide what can travel. + * + * Deprecated rather than merely legacy: it is a whole mechanism kept alive + * for a shrinking set of rows, and it takes eight `toXEvent()` methods and a + * ten-case round-trip suite with it. **docs/member-archive.md, "Retiring the + * rebuild", is the checklist** -- what goes, what only looks like it goes, + * and the one thing that has to be true before any of it can. */ + @Deprecated( + "Archive fallback for work signed before the GroupSignedEvent table. " + + "Goes when the last pre-v13 install does -- see the removal checklist " + + "in docs/member-archive.md." + ) private suspend fun rebuiltEventsOf( database: MantraDatabase, chatRoomId: String, diff --git a/composeApp/src/commonTest/kotlin/press/mantra/compose/nostr/archive/ArchiveRoundTripTest.kt b/composeApp/src/commonTest/kotlin/press/mantra/compose/nostr/archive/ArchiveRoundTripTest.kt index d6f274b3..0ef6cdca 100644 --- a/composeApp/src/commonTest/kotlin/press/mantra/compose/nostr/archive/ArchiveRoundTripTest.kt +++ b/composeApp/src/commonTest/kotlin/press/mantra/compose/nostr/archive/ArchiveRoundTripTest.kt @@ -39,22 +39,30 @@ import press.mantra.compose.nostr.nip30303.TranslationChapterEvent import press.mantra.compose.nostr.nip30303.TranslationChunkEvent /** - * The assumption the whole archive rests on: a row can be turned back into the - * event the group signed. + * The assumption the archive's *fallback* rests on: a row can be turned back + * into the event the group signed. * - * Signed events are not stored as events. `FrostSigningManager.complete` applies - * them and what survives is a `Mantra*` row, so an archive has to rebuild each - * one with `toXEvent()` and hope it comes out byte-identical. If it does not, - * the id changes, the signature no longer covers it, and every receiver drops - * the payload as a forgery -- silently, one kind at a time. + * Signed events used to be stored only as rows. `FrostSigningManager.complete` + * applied one and what survived was a `Mantra*` row, so an archive had to + * rebuild each payload with `toXEvent()` and hope it came out byte-identical -- + * and if it did not, the id changed, the signature no longer covered it, and + * every receiver dropped the payload as a forgery, silently, one kind at a time. * * That is what the round-trip note on each `toXEvent` claims and what nothing - * asserted until now. Every kind in `ArchiveEvent.ARCHIVABLE_KINDS` is checked - * here against a real FROST quorum, because a signature that verifies is the - * only evidence that the rebuild is faithful -- comparing fields would only + * asserted until this ran. Every kind in `ArchiveEvent.ARCHIVABLE_KINDS` is + * checked here against a real FROST quorum, because a signature that verifies is + * the only evidence that the rebuild is faithful -- comparing fields would only * prove the test and the code agree about which fields matter. * - * A kind that fails here cannot be archived at all, whatever the allowlist says. + * ### Deprecated, along with what it guards + * + * `GroupSignedEvent` keeps the events now, and `ArchiveManager.assemble` reads + * them; the rebuild survives only for work signed before that table existed, and + * so does this. Nothing else covers the `toXEvent()` methods, so **this whole + * file goes when they do** -- see "Retiring the rebuild" in + * docs/member-archive.md. Until then a kind that fails here cannot be archived + * *from a row*, whatever the allowlist says, which for a pre-v13 room is the + * same thing as not at all. */ class ArchiveRoundTripTest { private val participants = 3 diff --git a/docs/member-archive.md b/docs/member-archive.md index 5eb50732..0551394b 100644 --- a/docs/member-archive.md +++ b/docs/member-archive.md @@ -717,6 +717,102 @@ content. It buys graceful degradation and costs everything listed under --- +## Retiring the rebuild + +`assemble` reads `GroupSignedEvent` and rebuilds from `Mantra*` rows only what +that table does not hold, which by now is work signed before the table existed. +Everything on the rebuild side is marked `@Deprecated` so the compiler names it +at every call site, and it comes out in one piece rather than a method at a time +-- a half-removed rebuild is a rebuild that covers some kinds and silently drops +others. + +### The one precondition + +**No install still holds work signed before schema v13.** Nothing in the app can +check this, and no query answers it: a device that upgraded is indistinguishable +from one that never had pre-v13 work, and the rows that need rebuilding are on +*other people's* devices. It is a judgement about the installed base, not a +condition to test for. What can be checked, on any given device, is that the +rebuild is contributing nothing: + +``` +Archiving : N event(s) as the group signed them, M rebuilt from rows that predate the record +``` + +`ArchiveManager.signedEventsOf` logs that line only when `M > 0`. Silence across +the fleet is the evidence; it is not proof. + +A member whose device still needs it and does not get it is not broken loudly. +They keep their own rows and read the room normally. What they lose is the +ability to *answer* an archive request with the older half of the group's work, +so a newer member asks, is answered, and receives an archive that is quietly +short. That is the failure mode to weigh -- it looks like success on both ends. + +### What goes + +| what | where | +|---|---| +| `rebuiltEventsOf` | [ArchiveManager.kt](../composeApp/src/commonMain/kotlin/press/mantra/compose/managers/ArchiveManager.kt) -- the tree walk, and the version-label recovery inside it | +| the union in `signedEventsOf` | same file -- it collapses to the `GroupSignedEvent` read plus the `isArchivable` filter, which **stays**: see below | +| `MantraDialect.toDialectEvent` | [MantraDialect.kt](../composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraDialect.kt) | +| `MantraArtifact.toArtifactEvent` | [MantraArtifact.kt](../composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraArtifact.kt) -- and with it the `versionLabel` parameter that exists only because the label is not on the row | +| `MantraArtifactVersion.toArtifactVersionEvent` | [MantraArtifactVersion.kt](../composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraArtifactVersion.kt) | +| `MantraChapter.toChapterEvent` | [MantraChapter.kt](../composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraChapter.kt) | +| `MantraChunk.toChunkEvent` | [MantraChunk.kt](../composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraChunk.kt) | +| `MantraTranslationArtifactVersion.toTranslationArtifactVersionEvent` | [MantraTranslationArtifactVersion.kt](../composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraTranslationArtifactVersion.kt) | +| `MantraTranslationChapter.toTranslationChapterEvent` | [MantraTranslationChapter.kt](../composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraTranslationChapter.kt) | +| `MantraTranslationChunk.toTranslationChunkEvent` | [MantraTranslationChunk.kt](../composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MantraTranslationChunk.kt) | +| `ArchiveRoundTripTest`, all ten cases | [ArchiveRoundTripTest.kt](../composeApp/src/commonTest/kotlin/press/mantra/compose/nostr/archive/ArchiveRoundTripTest.kt) -- it exists to hold the rebuild up and covers nothing else | +| the "Where the events come from" reasoning above | this file | + +**Two already-dead cousins to sweep at the same time**, neither of which is part +of the rebuild and both of which will look like it to whoever does the removal: +`MantraTranslation.toTranslationEvent` (nothing has ever called it -- 30311 is +not archivable and nothing builds one) and +`MantraTranslationChunkProposal.toTranslationChunkEvent` (on a model that is not +even a `@Database` entity). See [dead-code.md](./dead-code.md) for the house +style on writing those down rather than deleting them blind. + +**The tests that seed without recording go too**, or they go on proving a path +that no longer exists. In `ArchiveAssemblyJvmTest` the `apply`-only seeding is +the rebuild path and `recordEverythingApplied` is the real one; the cases named +*work held both ways travels exactly once* and *an artifact the rebuild has to +leave out still archives from the record* are about the union specifically and +have no meaning without it. `ArchiveApplyJvmTest` seeds the sender the same way, +so it needs the recording call added rather than removed -- it is testing +delivery, not assembly, and would otherwise start asserting against an empty +archive. + +### What only looks like it goes + +**The `isArchivable` filter in `signedEventsOf` stays, and becomes the only +thing standing.** It is not part of the rebuild; it is there *because* of the +record. The rebuild could only ever produce document kinds, so nothing needed +filtering while it was the source. The table holds every kind the group has +signed, and every room signs a `GroupKeyStateEvent` as its first act -- so +removing that filter along with the walk turns every room's archive into an +`IllegalArgumentException` from `ArchiveEvent.build`. Two cases in +`ArchiveAssemblyJvmTest` fail with exactly that if it is dropped, which is the +guard against removing it by association. + +**The verify filter in `assemble` stays.** With the rebuild gone it is checking +events that were verified before they were recorded, so it can never fail in +practice -- which is an argument for keeping it, not for dropping it. It is one +signature check standing between a corrupted row and a payload every receiver +reads as a forgery, and "cannot happen" is the state it is meant to preserve. + +**`Mantra*.signature` and `Mantra*.publicKey` are not obviously removable, and +are a separate decision.** They were what made a row rebuildable, but they are +also what `SignedArtifactTest`, `SignedChapterTest`, `SignedGroupKeyStateTest` +and `ArchiveApplyJvmTest.rowFingerprints` assert on, and +`MantraTranslationContributor` builds a contributor list out of one. Since v13, +`groupSignedEventId` says whether the group signed a row and points at the proof, +so the columns are arguably redundant -- but that is a schema migration across +twelve tables with its own tests to rewrite, and it should not ride along with +this. + +--- + ## What this does not do Each of these will be reported as a bug. None of them is.