fix(chapters): number a chapter past the ones already proposed
Phase 3 of docs/chapter-uploads.md, and the second fix to something wrong today. A chapter's index is signed into the event and cannot be recounted on arrival: a device applying one cannot recount from a version other members may have added to in a different order. So the number has to be right when the chapter is proposed, and `getChaptersForArtifactVersion(...).size` is not it -- a proposed chapter is not a row until a quorum signs it, so two proposals a second apart read the same table and are signed at the same index. At one chapter at a time that is a race nobody hits. An upload of twelve files hits it eleven times out of eleven. The information was already published. A session's items carry the unsigned events it is signing, and an unsigned event's fields are settled at proposal time -- which is the whole reason FrostSigningRepository.unsignedEvents exists rather than only signedEvents. So ChapterIndexing.nextIndex reads the table and the room's proposals, and returns one past the highest rather than a count: a count is wrong the moment the sequence has a gap, and a chapter that failed to reach a quorum leaves one. Three things that are easy to get wrong and are tested. Every session counts whatever stage it is at, because filtering would trade a harmless gap for a collision with a stalled session that still completes. Items are filtered by kind, because a chunk carries an index too -- its position in the chapter -- and reading those as chapter indices puts the second chapter of a two-paragraph first chapter at 2. And proposals are scoped by the artifactVersionId tag, because the row query is scoped by version while observeSessionsForChatRoom returns the whole room. This fixes one device, not the group: two members proposing at the same moment still collide, and no amount of local reading will change that. That is tolerable where this was not -- an index orders reading, two chapters at index 4 sort arbitrarily between themselves, and nothing else breaks. 951 tests, no failures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -380,25 +380,36 @@ and the last of these exists precisely for this: "A caller wanting an id … nee
|
||||
it while the first session is still running, and the id is settled at proposal
|
||||
time."
|
||||
|
||||
```kotlin
|
||||
// MantraRepository, or a small ChapterIndexing object beside TranslationScaffold
|
||||
/**
|
||||
* The index the next chapter of [artifactVersionId] takes, counting chapters
|
||||
* the room has proposed but not yet signed.
|
||||
*
|
||||
* Rows alone are wrong by however many proposals are in flight, which at N = 1
|
||||
* is almost never and during an upload is always. A proposed chapter's index is
|
||||
* already fixed -- it is signed into the unsigned event the session will sign --
|
||||
* so it counts.
|
||||
*/
|
||||
suspend fun nextChapterIndex(chatRoomId: String, artifactVersionId: String): Int
|
||||
```
|
||||
**Built as `ChapterIndexing.nextIndex`**, an object beside `TranslationScaffold`
|
||||
rather than a method on `MantraRepository`, because it needs both repositories —
|
||||
the table for what is signed and the signing repository for what is proposed —
|
||||
and a repository method that reaches into another repository is the wrong shape.
|
||||
It takes both, which also makes it testable without a view model.
|
||||
|
||||
Implemented as: rows for the version, plus the unsigned `ChapterEvent`s of every
|
||||
open session in the room whose `artifactVersionId()` matches, de-duplicated by
|
||||
event id, and `maxOf(index) + 1` rather than a count — a count is wrong the
|
||||
moment a gap exists, and after a chapter fails to reach quorum a gap is exactly
|
||||
what there is.
|
||||
session in the room whose `artifactVersionId()` matches, and `maxOf(index) + 1`
|
||||
rather than a count — a count is wrong the moment a gap exists, and after a
|
||||
chapter fails to reach quorum a gap is exactly what there is.
|
||||
|
||||
Three details the build settled that the plan left open:
|
||||
|
||||
- **Every session counts, whatever stage it is at.** Filtering to open ones
|
||||
would trade a harmless gap (a failed session's index is spent, since a
|
||||
re-proposed chapter is a different event) for a possible collision with a
|
||||
stalled session that still completes.
|
||||
- **Items are filtered by kind, which is not optional.** A chapter proposal is a
|
||||
batch, and a chunk carries an `index` tag too — its position in the chapter.
|
||||
Reading those as chapter indices would put the second chapter of a
|
||||
two-paragraph first chapter at 2. There is a test for exactly that.
|
||||
- **Proposals are not scoped by version and rows are.** The row query takes an
|
||||
`artifactVersionId`; `observeSessionsForChatRoom` returns the whole room, so
|
||||
the `artifactVersionId` tag is what separates one version's proposals from
|
||||
another's. Getting it wrong would push every version's numbering up by the
|
||||
whole room's.
|
||||
|
||||
Failing soft on purpose: if the proposals cannot be read, the table's answer is
|
||||
still better than nothing, and refusing to propose a chapter because its number
|
||||
could not be checked would be the wrong trade.
|
||||
|
||||
Then `AddChapterViewModel` calls it instead of `.size`, and the upload allocates
|
||||
`base + i` for its i-th file **once**, before proposing anything, rather than
|
||||
@@ -412,9 +423,11 @@ are tolerable: the index orders reading, two chapters at index 4 sort
|
||||
arbitrarily between themselves and nothing else breaks. Worth a sentence in the
|
||||
code so the next reader does not take the local fix for a global one.
|
||||
|
||||
**Test** (`jvmTest`, beside `ChapterBatchProposalJvmTest`): propose three
|
||||
chapters back to back without completing any session, and assert the three
|
||||
signed indices are 0, 1, 2 — which is the test that fails today.
|
||||
**Tests** (`ChapterIndexingJvmTest`, beside `ChapterBatchProposalJvmTest`):
|
||||
propose three chapters back to back without completing any session, and assert
|
||||
the three indices are 0, 1, 2 — which is the test that fails today, since
|
||||
nothing is signed so the table answers 0 three times. Plus the gap, the two
|
||||
versions, and the chunk-vs-chapter confusion above.
|
||||
|
||||
---
|
||||
|
||||
|
||||
Reference in New Issue
Block a user