refactor: give the translation/chapter join one home
TranslationScaffold owns the rows that join a translation to the chapters it is a translation of. Pure refactor: the same events go out in the same order, `TranslationBatchProposalJvmTest` passes unedited apart from the call it makes, and no screen behaves differently. The move is worth making before anything is built on it. A translation chapter carries no words -- it is `(translation, chapter, position)` and nothing else, and it exists so a translated chunk has somewhere to hang. Both of the things it joins arrive on their own schedule: a chapter is signed into an artifact that already has translations, a translation is started on an artifact that already has chapters. So the same cross product has to be built from either side, and a second copy of it is a second chance to disagree about what a translation covers -- a disagreement that shows up as a chapter nobody can translate rather than as anything that looks like a bug. **Over ids, not rows.** `chaptersOf` takes translation ids and a `SourceChapter`, which is a chapter reduced to which one and where it sits, rather than a `MantraChapter`. Neither end is always a row: a translation being proposed exists only as the unsigned event a session is about to sign, and so does a chapter. `SourceChapter.of` is there for the callers that do hold a row. **Two things it decides rather than leaves to a caller.** Item order is apply order, so the nesting is fixed here -- translations outer, chapters inner, which keeps one translation's chapters contiguous and in reading order. And `createdAt` is taken once rather than read per template, so a scaffolding proposed as one act reads as one rather than as events that happen to share a minute. The index is the source chapter's own, never the position in the list handed in. They agree when the list is a whole version in order and stop agreeing the moment a caller holds a subset, and only one of them is what the group signed. **Tests.** TranslationScaffoldTest covers it as the pure function it is: the nesting, both directions it is built from, one timestamp for the lot, the empty cases, and the index surviving a non-contiguous subset. Checked against a broken implementation -- taking the index from the list position passes every test that uses a whole version in order, and is caught by the subset. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,74 @@
|
||||
package press.mantra.compose.managers
|
||||
|
||||
import com.vitorpamplona.quartz.nip01Core.core.HexKey
|
||||
import com.vitorpamplona.quartz.nip01Core.signers.EventTemplate
|
||||
import press.mantra.compose.database.model.MantraChapter
|
||||
import press.mantra.compose.nostr.nip30303.TranslationChapterEvent
|
||||
|
||||
/**
|
||||
* The rows that join a translation to the chapters it is a translation of.
|
||||
*
|
||||
* A translation chapter carries no words. It is `(translation, chapter,
|
||||
* position)` and nothing else, and it exists so that a translated chunk has
|
||||
* somewhere to hang: a translation without one for a chapter is a translation
|
||||
* that chapter cannot be worked on in.
|
||||
*
|
||||
* Both ends of that join arrive on their own schedule -- a chapter is signed
|
||||
* into an artifact that already has translations, a translation is started on
|
||||
* an artifact that already has chapters -- so the same scaffolding has to be
|
||||
* built from either side. One function for both, because two copies of a cross
|
||||
* product is two chances to disagree about what a translation covers.
|
||||
*/
|
||||
object TranslationScaffold {
|
||||
|
||||
/**
|
||||
* A source chapter, reduced to what a translation chapter needs of it:
|
||||
* which chapter, and where it sits.
|
||||
*
|
||||
* Taken as this rather than as a [MantraChapter] because the chapter is not
|
||||
* always a row yet. When a chapter is signed into an artifact, the
|
||||
* scaffolding goes out in the same breath as the chapter itself, against an
|
||||
* id that exists only as the unsigned event a session is about to sign.
|
||||
*/
|
||||
data class SourceChapter(
|
||||
val id: HexKey,
|
||||
val index: Int,
|
||||
) {
|
||||
companion object {
|
||||
fun of(chapter: MantraChapter) = SourceChapter(id = chapter.id, index = chapter.index)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A translation chapter for every pair of [translationArtifactVersionIds]
|
||||
* and [chapters], in that nesting: a translation's chapters stay together
|
||||
* and in reading order.
|
||||
*
|
||||
* The order is the order a session will sign them in and the order they
|
||||
* will be applied in, so it is fixed here rather than left to a caller.
|
||||
*
|
||||
* [createdAt] is stamped on all of them rather than read per event, so that
|
||||
* a scaffolding proposed as one act reads as one -- the timestamp of the
|
||||
* chapter or the translation that occasioned it, not of whichever moment
|
||||
* each template happened to be built in.
|
||||
*/
|
||||
fun chaptersOf(
|
||||
translationArtifactVersionIds: List<HexKey>,
|
||||
chapters: List<SourceChapter>,
|
||||
createdAt: Long,
|
||||
): List<EventTemplate<TranslationChapterEvent>> =
|
||||
translationArtifactVersionIds.flatMap { translationArtifactVersionId ->
|
||||
chapters.map { chapter ->
|
||||
TranslationChapterEvent.build(
|
||||
translationArtifactVersionId = translationArtifactVersionId,
|
||||
chapterId = chapter.id,
|
||||
// The source chapter's own position, never this list's:
|
||||
// a caller may hold a subset, and counting the list would
|
||||
// renumber the chapters it happens to be holding.
|
||||
index = chapter.index,
|
||||
createdAt = createdAt,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
@@ -10,9 +10,7 @@ import androidx.lifecycle.viewModelScope
|
||||
import androidx.lifecycle.viewmodel.initializer
|
||||
import androidx.lifecycle.viewmodel.viewModelFactory
|
||||
import co.touchlab.kermit.Logger
|
||||
import com.vitorpamplona.quartz.nip01Core.core.Event
|
||||
import com.vitorpamplona.quartz.nip01Core.core.HexKey
|
||||
import com.vitorpamplona.quartz.nip01Core.signers.EventTemplate
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.IO
|
||||
import kotlinx.coroutines.launch
|
||||
@@ -22,8 +20,8 @@ import press.mantra.compose.database.model.MantraChapter
|
||||
import press.mantra.compose.database.model.MantraDialect
|
||||
import press.mantra.compose.database.model.intermdiate.LocalChatRoom
|
||||
import press.mantra.compose.managers.FrostSigningManager
|
||||
import press.mantra.compose.managers.TranslationScaffold
|
||||
import press.mantra.compose.nostr.nip30303.TranslationArtifactVersionEvent
|
||||
import press.mantra.compose.nostr.nip30303.TranslationChapterEvent
|
||||
import press.mantra.compose.repository.ChatRepository
|
||||
import press.mantra.compose.repository.FrostSigningRepository
|
||||
import press.mantra.compose.repository.MantraRepository
|
||||
@@ -140,7 +138,13 @@ class AddTranslationArtifactVersionViewModel(
|
||||
// path -- neither of which this screen knows or should. The
|
||||
// translation comes back built, and the chapters are laid
|
||||
// out against it.
|
||||
dependents = { translation -> translationChaptersOf(translation, chapters) }
|
||||
dependents = { translation ->
|
||||
TranslationScaffold.chaptersOf(
|
||||
translationArtifactVersionIds = listOf(translation.id),
|
||||
chapters = chapters.map(TranslationScaffold.SourceChapter::of),
|
||||
createdAt = translation.createdAt,
|
||||
)
|
||||
}
|
||||
)
|
||||
}.onFailure { error ->
|
||||
logger.e("Failed to propose a translation for signing", error)
|
||||
@@ -163,38 +167,6 @@ class AddTranslationArtifactVersionViewModel(
|
||||
companion object {
|
||||
private const val TAG = "AddTranslationViewModel"
|
||||
|
||||
/**
|
||||
* A translation chapter for each of [chapters], ready to be signed with
|
||||
* the translation they hang off.
|
||||
*
|
||||
* A translation and its chapters go to the group as one batch, so these
|
||||
* are built from the translation *after* it has been authored under the
|
||||
* group's key -- [translation] is the unsigned event the session will
|
||||
* sign, which is where the id each chapter carries comes from. Laying
|
||||
* them out anywhere else would mean naming a translation id before one
|
||||
* exists.
|
||||
*
|
||||
* They take the translation's own timestamp, so the batch reads as one
|
||||
* act rather than as events that happen to share a session.
|
||||
*
|
||||
* Empty when the artifact has no chapters, which is a translation with
|
||||
* nothing yet to translate -- the chapters signed into the artifact
|
||||
* afterwards do not reach a translation proposed before them.
|
||||
*/
|
||||
fun translationChaptersOf(
|
||||
translation: Event,
|
||||
chapters: List<MantraChapter>,
|
||||
): List<EventTemplate<TranslationChapterEvent>> = chapters.map { chapter ->
|
||||
TranslationChapterEvent.build(
|
||||
translationArtifactVersionId = translation.id,
|
||||
chapterId = chapter.id,
|
||||
// The source chapter's own position, not this list's: the two
|
||||
// agree today, and only one of them is what the group signed.
|
||||
index = chapter.index,
|
||||
createdAt = translation.createdAt,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* The most chapters an artifact can be translated with in one session.
|
||||
*
|
||||
|
||||
@@ -0,0 +1,139 @@
|
||||
package press.mantra.compose.managers
|
||||
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertTrue
|
||||
import press.mantra.compose.database.model.MantraChapter
|
||||
import press.mantra.compose.nostr.nip30303.TranslationChapterEvent
|
||||
import press.mantra.compose.nostr.nip30303.tags.ChapterIdTag
|
||||
import press.mantra.compose.nostr.nip30303.tags.IndexTag
|
||||
import press.mantra.compose.nostr.nip30303.tags.TranslationArtifactVersionIdTag
|
||||
|
||||
/**
|
||||
* The join between a translation and the chapters it is of, as a pure function.
|
||||
*
|
||||
* Both ends arrive on their own schedule, so this is built from two
|
||||
* directions -- a translation being proposed, and a chapter being proposed.
|
||||
* What it must not do is disagree with itself about what a translation covers
|
||||
* depending on which direction it was built from.
|
||||
*/
|
||||
class TranslationScaffoldTest {
|
||||
|
||||
private val translationA = "a".repeat(64)
|
||||
private val translationB = "b".repeat(64)
|
||||
|
||||
private fun sourceChapter(index: Int, id: String = "c$index".padEnd(64, 'f')) = MantraChapter(
|
||||
id = id,
|
||||
artifactVersionId = "v".repeat(64),
|
||||
publicKey = "p".repeat(64),
|
||||
name = "Chapter ${index + 1}",
|
||||
originalText = "Paragraph.",
|
||||
index = index,
|
||||
wordCount = 1,
|
||||
characterCount = 10,
|
||||
signature = "",
|
||||
chatRoomId = "r".repeat(64),
|
||||
)
|
||||
|
||||
private fun parse(templates: List<com.vitorpamplona.quartz.nip01Core.signers.EventTemplate<TranslationChapterEvent>>) =
|
||||
templates.map { template ->
|
||||
Triple(
|
||||
template.tags.firstNotNullOfOrNull(TranslationArtifactVersionIdTag::parse)?.ref?.eventId,
|
||||
template.tags.firstNotNullOfOrNull(ChapterIdTag::parse)?.ref?.eventId,
|
||||
template.tags.firstNotNullOfOrNull(IndexTag::parse)?.index,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a translation's chapters stay together and in reading order`() {
|
||||
val chapters = (0..2).map(::sourceChapter)
|
||||
|
||||
val templates = TranslationScaffold.chaptersOf(
|
||||
translationArtifactVersionIds = listOf(translationA, translationB),
|
||||
chapters = chapters.map(TranslationScaffold.SourceChapter::of),
|
||||
createdAt = 1_000,
|
||||
)
|
||||
|
||||
// Item order is the order a session signs and applies in, so it is
|
||||
// fixed here rather than left to a caller. Translations outer, chapters
|
||||
// inner: one translation's chapters are contiguous and in order.
|
||||
assertEquals(
|
||||
listOf(
|
||||
Triple(translationA, chapters[0].id, 0),
|
||||
Triple(translationA, chapters[1].id, 1),
|
||||
Triple(translationA, chapters[2].id, 2),
|
||||
Triple(translationB, chapters[0].id, 0),
|
||||
Triple(translationB, chapters[1].id, 1),
|
||||
Triple(translationB, chapters[2].id, 2),
|
||||
),
|
||||
parse(templates)
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a chapter proposed on its own is scaffolded into every translation`() {
|
||||
// The other direction: one chapter, every existing translation. This is
|
||||
// what a chapter's own scaffolding session carries.
|
||||
val templates = TranslationScaffold.chaptersOf(
|
||||
translationArtifactVersionIds = listOf(translationA, translationB),
|
||||
chapters = listOf(TranslationScaffold.SourceChapter(id = "z".repeat(64), index = 7)),
|
||||
createdAt = 1_000,
|
||||
)
|
||||
|
||||
assertEquals(
|
||||
listOf(
|
||||
Triple(translationA, "z".repeat(64), 7),
|
||||
Triple(translationB, "z".repeat(64), 7),
|
||||
),
|
||||
parse(templates)
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a chapter keeps its own position, not its position in the list`() {
|
||||
val chapters = (0..5).map(::sourceChapter)
|
||||
|
||||
// A caller may hold a subset, and the subset is not contiguous.
|
||||
// Counting the list would renumber chapters 3 and 5 as 1 and 2, which
|
||||
// is a translation that reads in a different order than the work.
|
||||
val templates = TranslationScaffold.chaptersOf(
|
||||
translationArtifactVersionIds = listOf(translationA),
|
||||
chapters = listOf(chapters[3], chapters[5]).map(TranslationScaffold.SourceChapter::of),
|
||||
createdAt = 1_000,
|
||||
)
|
||||
|
||||
assertEquals(listOf(3, 5), parse(templates).map { it.third })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the whole scaffolding carries one timestamp`() {
|
||||
val templates = TranslationScaffold.chaptersOf(
|
||||
translationArtifactVersionIds = listOf(translationA, translationB),
|
||||
chapters = (0..2).map(::sourceChapter).map(TranslationScaffold.SourceChapter::of),
|
||||
createdAt = 4_242,
|
||||
)
|
||||
|
||||
// Read once by the caller rather than per template, so a scaffolding
|
||||
// proposed as one act reads as one.
|
||||
assertEquals(listOf(4_242L), templates.map { it.createdAt }.distinct())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `nothing to scaffold when there is nothing on one side`() {
|
||||
assertTrue(
|
||||
TranslationScaffold.chaptersOf(
|
||||
translationArtifactVersionIds = emptyList(),
|
||||
chapters = listOf(TranslationScaffold.SourceChapter("z".repeat(64), 0)),
|
||||
createdAt = 1_000,
|
||||
).isEmpty()
|
||||
)
|
||||
|
||||
assertTrue(
|
||||
TranslationScaffold.chaptersOf(
|
||||
translationArtifactVersionIds = listOf(translationA),
|
||||
chapters = emptyList(),
|
||||
createdAt = 1_000,
|
||||
).isEmpty()
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -172,7 +172,11 @@ class TranslationBatchProposalJvmTest {
|
||||
userPublicKey = proposer,
|
||||
lead = translationTemplate,
|
||||
dependents = { translation ->
|
||||
AddTranslationArtifactVersionViewModel.translationChaptersOf(translation, chapters)
|
||||
TranslationScaffold.chaptersOf(
|
||||
translationArtifactVersionIds = listOf(translation.id),
|
||||
chapters = chapters.map(TranslationScaffold.SourceChapter::of),
|
||||
createdAt = translation.createdAt,
|
||||
)
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user