feat(drafting): store the text of section headings - #269
Merged
henrique221 merged 1 commit intoAug 18, 2026
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
7 tasks
Follow-up to #264, and the last storage gap behind fluent-web#397. A paragraph record is a marker plus an offset into the *verse's* text, so it cannot hold a section heading: a heading is a block of its own, with its own words, belonging to no verse. Exported today it renders as a bare "\s1" and the translator's words are gone. markers.headings carries them, in order, as blocks emitted before the verse: { "headings": [{ "marker": "s1", "text": "The Creation" }] } Markers are restricted to the heading subset of USFM_PARAGRAPH_MARKERS, so body text can never be stored as a heading and lost from the row, and the text is guarded against backslashes and line breaks exactly as the book fields are, since it is written straight into the USFM stream. paragraphs became optional, because a verse may carry only a heading, and the object now has to hold one or the other — null already means "no structure". No migration: the column is jsonb, so the shape lives in Zod. Confirmed with drizzle-kit generate ("No schema changes, nothing to migrate"). Claude-Session: https://claude.ai/code/session_01R6Xec2CgkML6mYyHmqiGK5
henrique221
force-pushed
the
feat/section-heading-text
branch
from
August 13, 2026 18:28
14b4fc0 to
d9fd279
Compare
kaseywright
approved these changes
Aug 14, 2026
henrique221
merged commit Aug 18, 2026
f63d228
into
feat/263-verse-markers-book-fields
1 check passed
henrique221
added a commit
that referenced
this pull request
Aug 21, 2026
…lds (#264) * feat(drafting): store verse paragraph markers and book-level USFM fields Closes #263. Unblocks fluent-web#314 (paragraph authoring in the RTE) and fluent-web#398 (Book Details panel). All additive: legacy rows and existing exports behave exactly as before. - translated_verses.markers (nullable jsonb): paragraph starts per verse. Offset 0 means the verse opens a paragraph; a mid-text offset splits the verse across paragraphs. Zod-validated: marker pattern (no USFM injection), strictly increasing offsets, offsets bounded by the content on insert. Upsert writes it, responses return it. - project_unit_bible_books.running_header / book_title: authored \h and \mt1 per unit and book, validated against marker injection, empty clears to null. - USFM export honors both: stored paragraph markers replace the hardcoded single \p per chapter (which remains the fallback for rows without markers), and \h/\mt prefer the authored fields with the display-name fallback. - GET /project-units/{id}/book-details and PATCH .../book-details/{bookId}, gated by authenticateUser + requirePermission + requireProjectUnitAccess. 21 new tests: the export generator (golden legacy output, opening markers, mid-verse splits, out-of-range offsets, book-field fallbacks) and the two validation schemas. Migration 0019 verified against a scratch Postgres. Claude-Session: https://claude.ai/code/session_01R6Xec2CgkML6mYyHmqiGK5 * fix(drafting): allowlist the paragraph markers a verse may open with The stored marker is emitted verbatim as `\<marker>` by the USFM export, so the previous `^[a-z][a-z0-9]{0,9}$` pattern let a client store `v` or `c` and put a bare `\v` / `\c` — a structural marker with no number — ahead of the real one. Replace the pattern with `z.enum` over the USFM 3.x body-text paragraph set, which also documents the accepted values in the OpenAPI spec for the editor. Introduction markers stay out: they precede \c 1 and cannot open inside a verse. Refs: #264 * fix(book-details): reject control and line-separator characters in book fields The guard covered `\`, LF and CR, but NUL, ESC, U+2028 and U+2029 passed straight into the `\h` / `\mt` line of the export, where a consumer treating the separators as line breaks sees a different file than the validator did. Reject the Cc, Zl and Zp ranges, spelled as literal ranges rather than `\p{...}` so the pattern published in the OpenAPI document carries the same meaning without the unicode flag. Also corrects the endpoint descriptions: the export emits `\mt`, not `\mt1`. Refs: #264 * test(usfm): assert a non-default marker on the untranslated-verse export The case stored `p`, which is exactly what the chapter falls back to, so it passed whether or not the exporter read `markers` at all. Use `q1` and assert no `\p` is emitted, so the test fails if the stored marker is ignored. Refs: #264 * style: prettier over the markers select and drizzle-generated migration meta Formatting only, and the cause of the red validate job: the markers select was inserted with off-by-two indentation, and drizzle-kit writes its meta JSON in its own style, which the repo's format check does not accept. Both meta files parse identically before and after. Claude-Session: https://claude.ai/code/session_01R6Xec2CgkML6mYyHmqiGK5 * refactor(book-details): own the project-unit auth check Review feedback: book-details imported requireProjectUnitAccess from usfm's middleware, coupling book metadata authoring to export generation because the two checks happen to look alike. The domain now carries its own. usfm and translated-verses each already do the same, so this follows the existing convention rather than inventing one; consolidating the three into shared infrastructure stays a separate, deliberate decision. A boundary test pins it: book-details imports nothing from a sibling feature domain, and its route uses its own middleware. Both fail against the code as reviewed. Claude-Session: https://claude.ai/code/session_01R6Xec2CgkML6mYyHmqiGK5 * feat(drafting): store the text of section headings (#269) Follow-up to #264, and the last storage gap behind fluent-web#397. A paragraph record is a marker plus an offset into the *verse's* text, so it cannot hold a section heading: a heading is a block of its own, with its own words, belonging to no verse. Exported today it renders as a bare "\s1" and the translator's words are gone. markers.headings carries them, in order, as blocks emitted before the verse: { "headings": [{ "marker": "s1", "text": "The Creation" }] } Markers are restricted to the heading subset of USFM_PARAGRAPH_MARKERS, so body text can never be stored as a heading and lost from the row, and the text is guarded against backslashes and line breaks exactly as the book fields are, since it is written straight into the USFM stream. paragraphs became optional, because a verse may carry only a heading, and the object now has to hold one or the other — null already means "no structure". No migration: the column is jsonb, so the shape lives in Zod. Confirmed with drizzle-kit generate ("No schema changes, nothing to migrate"). Claude-Session: https://claude.ai/code/session_01R6Xec2CgkML6mYyHmqiGK5 * style: prettier over the heading schema and its test Pre-merge validation caught the test file. The schema was unformatted too, and fixing only the reported file would have left the check red: prettier expands the USFM_HEADING_MARKERS array to one entry per line. Reflow only, no content change. Claude-Session: https://claude.ai/code/session_01R6Xec2CgkML6mYyHmqiGK5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #264, and the last storage gap behind fluent-web#397 (chapter view).
Targets
feat/263-verse-markers-book-fields, notmain, so it does not disturb the review in flight on #264. It retargets tomainwhen that merges, and the diff stays the same.The gap
A paragraph record is
{ marker, offset }— a marker plus an offset into the verse's text. A section heading is a block of its own, carrying its own words, belonging to no verse, so there is nowhere to put them. Today the export renders a heading as a bare marker and the translator's words are gone:fluent-web#408 can therefore apply a heading as a block type but cannot let anyone author its text, which is called out in that PR.
The shape
{ "headings": [ { "marker": "s1", "text": "The Creation" } ], "paragraphs": [ { "marker": "p", "offset": 0 } ] }Headings are blocks emitted before the verse, in order, and before its paragraph marker — a heading is not a paragraph, so the verse that follows still needs one:
Three constraints worth a look:
USFM_PARAGRAPH_MARKERS(\s1–\s4,\ms,\mr,\sr,\d, …). Allowing\phere would let a caller store the verse's own prose as a heading and lose it from the row.paragraphsbecame optional, because a verse may carry only a heading — and the object must now hold one or the other, sincenullalready means "no structure".No migration
The column is
jsonb, so the shape lives in Zod rather than in DDL. Confirmed rather than assumed:drizzle-kit generatereports "No schema changes, nothing to migrate", and the migration folder is untouched. That also means this could have landed before or after #264 at the same cost — I said earlier it was cheaper to fold into #264 while it was open, and that was wrong.Verification
296 tests pass. Twelve are new: eight over the schema (accepting one and several headings, rejecting body-text markers, rejecting injection attempts and empty text, and old rows still parsing) and four over the export (a heading before its verse, several in order, the chapter's default paragraph still emitted after a heading, and a mid-chapter heading landing before its own verse rather than at the chapter top). Lint and typecheck clean.