Ft/pericope view#205
Merged
Merged
Conversation
…-server into ft/pericope-view
|
Warning Review limit reached
Next review available in: 58 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughIntroduces a pericopes domain: new ChangesPericope Sets Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
…es that havnt reviewed yet
…-server into ft/pericope-view
Joel-Joseph-George
marked this pull request as ready for review
June 30, 2026 05:29
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/db/schema.ts`:
- Around line 249-255: The pericope verse mapping in schema.ts is currently only
indexed, which allows duplicate (pericopeSetId, bookId, chapterNumber,
verseNumber) rows and can make getPericopeVersesForChapter() return ambiguous
results. Update the idx_pericope_verses_set_book_chapter_verse definition to
enforce uniqueness, and keep the migration in 0012_add_pericope_sets.sql aligned
so the database schema and seed/import path both reject conflicting verse
mappings.
In `@src/db/seeds/pericope-sets.ts`:
- Around line 98-119: In the pericope seed logic, unknown FCBH book names are
only warned about and the seed continues, which leaves `pericope_verses`
partially populated. Update the seeding flow in the records loop and the
`unmappedBooks` handling to fail fast instead of logging a warning, matching the
behavior used by the other seeders; ensure the seed aborts as soon as any
unmapped book is detected and surfaces the offending book names clearly.
- Around line 173-186: The project backfill for legacy projects is currently
happening inside the dev-only seed flow in seedPericopeSets(), which means it
won’t run during a normal production rollout. Move the logic that looks up
fiaSet and updates projects with a null pericopeSetId into the migration/rollout
path used by the database schema change, and keep seedPericopeSets() focused on
seeding data only. Use the existing projects, pericope_sets, and isNull update
logic as the implementation reference, but wire it to the migration path instead
of src/db/scripts/setup.ts.
- Around line 53-59: The `buildBookIdMap` helper currently lets `Map#set`
overwrite earlier `books.eng_display_name` entries, so add a duplicate check
before populating the map and fail fast if the same display name appears more
than once. Use the existing `buildBookIdMap` flow in
`src/db/seeds/pericope-sets.ts` to detect ambiguous `books` rows and throw a
clear error instead of returning a silently corrupted mapping, matching the
strict behavior already used by `seedBibles()` and `seedBibleTexts()`.
In `@src/domains/pericopes/pericopes.route.ts`:
- Around line 27-39: The pericopes route currently documents only 401 and 500,
but authenticateUser can also return 403 for inactive users. Update the
responses in pericopes.route.ts for the route that uses authenticateUser to
include a 403 response with the same message schema style used elsewhere, so the
OpenAPI contract matches the runtime behavior.
In `@src/domains/pericopes/pericopes.service.ts`:
- Around line 26-32: The get-pericopes flow in pericopes.service.ts is returning
the empty verse-by-verse fallback before validating bookCode, which makes
invalid book codes behave differently depending on pericopeSetId. Reorder the
checks in the pericopes lookup so getBookIdByCode runs before the
no-pericope-set ok([]) fallback, and keep the existing ErrorCode.BOOK_NOT_FOUND
path in the same method to ensure bad input is rejected consistently.
In `@src/domains/projects/projects.service.ts`:
- Around line 48-57: Use an explicit nullish check for pericopeSetId in the
project create/update validation paths, since the current truthy check skips
values like 0 and can let invalid references reach the write path. Update the
conditional in the relevant projects.service.ts logic around the pericope_sets
lookup to check only for null/undefined, and apply the same fix in the other
branch noted in the review. Keep the existing PERICOPE_SET_NOT_FOUND return
behavior when a provided ID does not exist.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4e3febc5-be9c-4ca7-a876-b3595a0faf91
📒 Files selected for processing (17)
data/FCBH-All-Books.jsondata/FIA-All-Books.jsoneslint.config.mjssrc/app.tssrc/db/migrations/0012_add_pericope_sets.sqlsrc/db/migrations/meta/0012_snapshot.jsonsrc/db/migrations/meta/_journal.jsonsrc/db/schema.tssrc/db/scripts/setup.tssrc/db/seeds/pericope-sets.tssrc/domains/pericopes/pericopes.repository.tssrc/domains/pericopes/pericopes.route.tssrc/domains/pericopes/pericopes.service.tssrc/domains/pericopes/pericopes.types.tssrc/domains/projects/projects.query-builder.tssrc/domains/projects/projects.service.tssrc/lib/types.ts
kaseywright
requested changes
Jul 1, 2026
kaseywright
left a comment
kaseywright
left a comment
Contributor
There was a problem hiding this comment.
Pretty solid implementation. I would suggest adding at least basic unit tests for new features like this to avoid drift in the future.
The large seed data files might end up being stored either in a separate repo or CDN later as more of these become necessary.
Contributor
Author
I thought about that. but i felt for now keeping it in repo isn't harming that much. But yeah its bigger in size with limited book collection (2 files : 4.5mb approx) |
…-server into ft/pericope-view
kaseywright
approved these changes
Jul 2, 2026
JEdward7777
pushed a commit
to JEdward7777/fluent-api
that referenced
this pull request
Jul 3, 2026
Phase 1 of the repeated-word-check persistence backend.
- db/schema.ts: add user_settings table (user_id PK, settings jsonb,
timestamps, FK to users ON DELETE CASCADE) with select/insert schemas;
add userSettingsSchema (read, .catch({})) + strict userSettingsWriteSchema
so malformed PUT bodies 400 instead of being silently coerced; extend
editorStateResourcesSchema with optional activeLeftTab + checkOccurrenceRules.
- migration 0013_add_user_settings.sql (+meta). Renumbered from 0012 during
rebase onto jel-feature-flags-proposal to avoid colliding with the pericope
0012 migration that landed on main via eten-tech-foundation#205.
- domains/self/settings quartet: GET/PUT /self/settings, authenticateUser only,
full-replace upsert, strict-validate -> 400, read-tolerant.
- register self domain in app.ts.
- tests: self-settings.route.test.ts (8) + schema.editor-state-extension.test.ts (8).
Relates to eten-tech-foundation#172, #277, #278
JEdward7777
pushed a commit
to JEdward7777/fluent-api
that referenced
this pull request
Jul 3, 2026
Phase 1 of the repeated-word-check persistence backend.
- db/schema.ts: add user_settings table (user_id PK, settings jsonb,
timestamps, FK to users ON DELETE CASCADE) with select/insert schemas;
add userSettingsSchema (read, .catch({})) + strict userSettingsWriteSchema
so malformed PUT bodies 400 instead of being silently coerced; extend
editorStateResourcesSchema with optional activeLeftTab + checkOccurrenceRules.
- migration 0013_add_user_settings.sql (+meta). Renumbered from 0012 during
rebase onto jel-feature-flags-proposal to avoid colliding with the pericope
0012 migration that landed on main via eten-tech-foundation#205.
- domains/self/settings quartet: GET/PUT /self/settings, authenticateUser only,
full-replace upsert, strict-validate -> 400, read-tolerant.
- register self domain in app.ts.
- tests: self-settings.route.test.ts (8) + schema.editor-state-extension.test.ts (8).
Relates to eten-tech-foundation#172, #277, #278
JEdward7777
pushed a commit
to JEdward7777/fluent-api
that referenced
this pull request
Jul 7, 2026
Phase 1 of the repeated-word-check persistence backend.
- db/schema.ts: add user_settings table (user_id PK, settings jsonb,
timestamps, FK to users ON DELETE CASCADE) with select/insert schemas;
add userSettingsSchema (read, .catch({})) + strict userSettingsWriteSchema
so malformed PUT bodies 400 instead of being silently coerced; extend
editorStateResourcesSchema with optional activeLeftTab + checkOccurrenceRules.
- migration 0013_add_user_settings.sql (+meta). Renumbered from 0012 during
rebase onto jel-feature-flags-proposal to avoid colliding with the pericope
0012 migration that landed on main via eten-tech-foundation#205.
- domains/self/settings quartet: GET/PUT /self/settings, authenticateUser only,
full-replace upsert, strict-validate -> 400, read-tolerant.
- register self domain in app.ts.
- tests: self-settings.route.test.ts (8) + schema.editor-state-extension.test.ts (8).
Relates to eten-tech-foundation#172, #277, #278
JEdward7777
added a commit
that referenced
this pull request
Jul 7, 2026
…res (+ Repeated Word Check backend) (#213) * docs(proposals): add feature-flags proposal (config/features endpoint) Add a docs-only proposal for a lightweight, env-sourced feature-flag mechanism to gate AI-dependent front-end features (starting with the Repeated Word Check) so their PRs can merge and ship hidden while fluent-ai is unhosted. - EN_FEATURE_* env vars, declared in the Zod schema, swept into a map - new unauthenticated meta route GET /config/features (sibling of /health) - fluent-web consumes via a useFeatureFlags hook + FeatureGate (fail-closed) - unlinked, login-gated diagnostics page for the flag view - publishing only; no server-side enforcement (D5) Includes -suggestion.md (design, D1-D8, Q1-Q4) and -summary.md. * docs(proposals): address CodeRabbit on feature-flags (stringbool + fence lang) * fix(db): correct 0012 pericope snapshot to match schema The 0012_add_pericope_sets migration snapshot was out of sync with its own schema/SQL: - Removed a phantom `public.recordings` table that was never part of the migration (an accidental leak from experimental mobile DB work; absent from Dev/PROD and from schema.ts). - Fixed `idx_pericope_verses_set_book_chapter_verse` to `isUnique: true` to match the CREATE UNIQUE INDEX in 0012_add_pericope_sets.sql. No SQL or schema changes; snapshot-only correction so drizzle-kit generate diffs cleanly against the actual schema. * feat(self): add /self/settings domain + user_settings table Phase 1 of the repeated-word-check persistence backend. - db/schema.ts: add user_settings table (user_id PK, settings jsonb, timestamps, FK to users ON DELETE CASCADE) with select/insert schemas; add userSettingsSchema (read, .catch({})) + strict userSettingsWriteSchema so malformed PUT bodies 400 instead of being silently coerced; extend editorStateResourcesSchema with optional activeLeftTab + checkOccurrenceRules. - migration 0013_add_user_settings.sql (+meta). Renumbered from 0012 during rebase onto jel-feature-flags-proposal to avoid colliding with the pericope 0012 migration that landed on main via #205. - domains/self/settings quartet: GET/PUT /self/settings, authenticateUser only, full-replace upsert, strict-validate -> 400, read-tolerant. - register self domain in app.ts. - tests: self-settings.route.test.ts (8) + schema.editor-state-extension.test.ts (8). Relates to #172, #277, #278 * Prettifier changes. * feat(checks): surface target ISO 639-3 lang code on chapter assignments (phase-04 BUG #2) The repeated-words check sends the target language's ISO 639-3 code to greek-room as lang_code (greek-room keys its legitimate-duplicate whitelist on the code, not the display name). The user chapter-assignment response previously exposed only the human display name (targetLanguage), so the web client had no code to send. Plumb targetLangCode (targetLang.langCodeIso6393) through findAssignmentsProgress -> ChapterAssignmentProgressInfo -> toResponse as targetLanguageCode on UserChapterAssignmentResponse (schema + types). Add a toResponse regression test. Found during the Phase 4 manual smoke. * feat(db): add second dev translator and English to dev seed Enables exercising the full project assign -> draft flow out-of-the-box: - dev-users seeder now provisions a second translator (translator2, t2@fluent.local), so a project meets the >=2-translator requirement for chapter assignment (distinct Drafter + Peer Checker). Idempotent; honours SEED_TRANSLATOR2_EMAIL/SEED_TRANSLATOR2_PASSWORD overrides. - languages seed now includes English (eng), so a draftable English target language is available without manual DB insertion. Both seeders remain idempotent (existing rows skipped on re-run). * fix(checks): surface targetLanguageCode on project progress endpoint The repeated-words check sends greek-room a `lang_code` keyed on the ISO 639-3 target language code. greek-room's legitimate-duplicate whitelist (e.g. "truly truly" for "eng") only matches when the code is correct. The PM "open chapter" path builds its ProjectItem from GET /projects/{id}/chapter-assignments/progress, but that endpoint never serialized the target language code: the repository fetched targetLangCode yet both service mappers (getChapterAssignmentProgressByProject and assignSelectedChapters) dropped it, and the response schema didn't declare it. As a result the PM-path check sent lang_code "<unknown>" and legitimate duplicates were flagged. Add `targetLanguageCode` to chapterAssignmentProgressResponseSchema and populate it in both mappers (falling back to '' when the FK doesn't resolve, matching the existing sourceLangCode handling). Includes TEMP [RW-DIAG] logging used to pin down the bug; removed in the following commit. * chore(checks): remove temporary [RW-DIAG] logging Removes the diagnostic logging added in the previous commit (inbound repeated-words request log, fluent-ai response/error log, and the toResponse targetLangCode log) now that the root cause is fixed and verified. * refactor(self-settings): address review on /self/settings contracts - normalize stored settings JSONB through the tolerant read schema in toResponse (null-guarded) so a malformed/legacy blob degrades to {} instead of leaking through GET; add two read-path tests - declare 403 FORBIDDEN on both GET and PUT route contracts (reachable via authenticateUser's inactive-account branch); add a 403 test to each - tighten the OpenAPI response schema settings field to userSettingsSchema so the spec advertises the real keys/enums (response provably conforms after the read-path normalization); request schema left generic by design * docs(self-settings): note full-replace is single-key-only; merge before adding a key * feat(config): publish env-sourced feature flags via GET /config/features Implements the fluent-api half of the feature-flags proposal (docs in docs/proposals/feature-flags): an unauthenticated, DB-less meta route that publishes which optional, AI-dependent features are enabled per environment, so fluent-web can ship AI UI hidden while fluent-ai is unhosted. - env.ts: declare EN_FEATURE_REPEATED_WORD_CHECK in the Zod EnvSchema via a local envBool() parser (not z.coerce.boolean(), which would read the string "false" as true and invert the safe-off default). Optional so an unset flag can derive its default. - lib/features.ts: typed FLAGS registry tying each env var to its camelCase wire key + unset default (repeatedWordCheck defaults to whether FLUENT_AI_URL + FLUENT_AI_KEY are wired); buildFeatures() derives the published map; named OpenAPI featuresSchema guarded by a compile-time `satisfies`. - routes/config.route.ts: GET /config/features -> { features: { ... } }, sibling of /health; publishes state, does not enforce it (no gating of the AI request path). - app.ts: register the route. .env.example: document the flag. - Tests: buildFeatures behavior + a three-way drift guard (env schema / FLAGS / OpenAPI schema kept in sync, both directions) + route behavior. Relates to the feature-flags proposal (fluent-api #211). * fix(db): seed pericope sets in docker-entrypoint The container seeds the DB via docker-entrypoint.sh, which inlined its own seed list and stopped after bible-texts, never running the pericope-sets seed that setup.ts includes as its final step. As a result every container-based DB (re)generation left pericope_sets empty, so the Create Project page's Pericope Set dropdown had nothing to select. Add the pericope-sets seed after bible-texts (it depends on books, already seeded earlier). The seed is idempotent (upsert set by name + replace verses) and its data files are baked into the image, matching the other entrypoint seeds. * fix(flags): address CodeRabbit review on #213 - rename wire field targetLanguageCode -> targetLangCode (match sourceLangCode) - env: treat blank EN_FEATURE_* as unset so empty .env.example lines don't fail boot - self-settings: return VALIDATION_ERROR (422) on write-schema violation - docs: update feature-flags snippet from z.stringbool() to shipped envBool() - tests: assert userSettingsWriteSchema rejects malformed writes - refactor: extract shared toChapterAssignmentProgressResponse() mapper * test(doc): add regression test asserting GET /doc renders the OpenAPI spec The OpenAPI document walk over every registered schema currently 500s ('Unknown zod object type') and nothing at the unit/type level catches it, so it only surfaced at runtime. This test renders the real assembled document via GET /doc and asserts a 200, so that class of break fails CI. Currently RED (reproduces the live 500); the fix follows in the next commit. * fix(self-settings): keep the .catch({}) read schema off the OpenAPI surface GET /doc 500'd with 'Unknown zod object type' because the /self/settings response schema embedded userSettingsSchema (= userSettingsObjectSchema.catch({})). The .catch wrapper is a ZodCatch node that @asteasolutions/zod-to-openapi (under @hono/zod-openapi) has no renderer for, so building the OpenAPI document threw and took the whole /doc endpoint down. Export the plain userSettingsObjectSchema and embed THAT in userSettingsResponseSchema. The .catch only affects behaviour on invalid input, so it adds/removes no fields: the documented shape and the response bytes are unchanged. The fail-soft W8 read tolerance stays where it belongs (the service read path, toResponse -> userSettingsSchema.parse), and the strict write schema is untouched. No request/response contract change. Turns the doc.route regression test green. * fix(flags): gate /config/features behind auth (review #211 Q1) Per kaseywright's review on proposal #211 (Q1), GET /config/features is now login-gated via authenticateUser (no role required), matching the pattern used by self-settings and pericopes-list. The endpoint still only publishes flags (does not enforce them, D5 unchanged); only the read now requires a session. Reversible later if we decide it should be public. - config.route.ts: add authenticateUser middleware + UNAUTHORIZED (401) response, reword description (no longer 'unauthenticated like /health'). - config.route.test.ts: rewrite against the mocked-auth pattern — null session => 401; valid session => 200 with the features map. * fix(flags): address CodeRabbit re-review on #213 + harden db bootstrap CodeRabbit re-review (auth-gate follow-up): - self-settings.route: add 422 UNPROCESSABLE_ENTITY to PUT OpenAPI responses so the published contract matches upsertSettings validation behavior (R5) - self-settings.route: GET error branch derives status via getHttpStatus(result.error) instead of hardcoding 500, consistent with PUT (R4) - schema.ts: correct stale userSettingsWriteSchema comment (400 -> 422) (R3) - lib/features: tighten FlagDefinition.env to boolean-valued EN_FEATURE_* keys and drop the boolean cast in buildFeatures (R7) - docs/proposals/feature-flags: fold suggestion + summary to the shipped authenticated contract and Q1-Q4 resolutions (R1/R2) Stack durability (unblocks local smoke testing): - db bootstrap: poll for real query readiness before issuing DDL. The compose healthcheck (pg_isready) reports ready as soon as Postgres accepts connections, but during startup/WAL-recovery it rejects queries with 57P03; the entrypoint's set -e then killed the container (and its dependents) on restart. Retry transient startup/connection errors with bounded backoff (~60s ceiling); real errors still fail fast. --------- Co-authored-by: Joshua Lansford <Joshua@Lansfords.com>
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.
Summary by CodeRabbit
New Features
Bug Fixes