docs(proposals): feature flags to ship AI-dependent UI hidden (config/features)#211
docs(proposals): feature flags to ship AI-dependent UI hidden (config/features)#211JEdward7777 wants to merge 2 commits into
Conversation
|
Warning Review limit reached
Next review available in: 57 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 (2)
📝 WalkthroughWalkthroughThis PR adds two new proposal documents describing a feature-flag mechanism where fluent-api derives flags from ChangesFeature Flags Proposal
Estimated code review effort: 1 (Trivial) | ~5 minutes Sequence Diagram(s)sequenceDiagram
participant EnvVars as EN_FEATURE_* env vars
participant FluentApi as fluent-api
participant FluentWeb as fluent-web
participant FeatureGate as FeatureGate/useFeatureFlags
EnvVars->>FluentApi: Declared via Zod schema
FluentApi->>FluentApi: GET /config/features builds features map
FluentWeb->>FluentApi: Request /config/features
FluentApi-->>FluentWeb: features: {name: boolean}
FluentWeb->>FeatureGate: Pass feature map
FeatureGate-->>FluentWeb: Show/hide AI-dependent UI
Related labels: skip_review Suggested reviewers: N/A (documentation-only proposal, low review effort) Poem: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/proposals/feature-flags/feature-flags-suggestion.md`:
- Around line 139-147: The fenced endpoint example in the feature-flags proposal
is unlabeled, which triggers MD040 and hurts readability. Update the markdown
code fence around the GET /config/features example in the proposal doc to
include an appropriate language tag such as json or http, keeping the example
content unchanged. Use the existing example block in the feature-flags
suggestion document as the target.
- Around line 93-105: The feature-flag schema in EnvSchema uses
z.coerce.boolean(), which can turn the string "false" into true. Replace it with
an explicit boolean string parser for EN_FEATURE_REPEATED_WORD_CHECK so only
intended true/false env values are accepted, and keep the existing optional
behavior so the derived default still applies when the variable is unset.
🪄 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: 330ab80d-9b1b-4ff5-a58e-7135cd06a35d
📒 Files selected for processing (2)
docs/proposals/feature-flags/feature-flags-suggestion.mddocs/proposals/feature-flags/feature-flags-summary.md
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 eten-tech-foundation#211).
1f26c44 to
a020a55
Compare
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.
a020a55 to
ccacce9
Compare
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 eten-tech-foundation#211).
…ation#211 Q1) Per kaseywright's review on proposal eten-tech-foundation#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.
…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>
|
Closing this docs-only proposal PR. The implementation PR #213 has been reviewed, approved, and merged (merge commit |
Proposal (docs only)
A design for environment-driven feature flags so front-end features that depend on fluent-ai can merge and ride to production hidden, then be turned on (per environment) wherever fluent-ai is actually hosted — no code redeploy. This unblocks the queued AI-dependent PRs that currently can't merge because doing so would surface UI whose backend isn't deployed.
No code changes here — just
docs/proposals/feature-flags/.The shape
EN_FEATURE_*prefix, each declared explicitly in the ZodEnvSchema(the schema stays the authoritative flag catalog, and this survives Zod's unknown-key stripping). The repeated-word-check flag defaults off unlessFLUENT_AI_URL+FLUENT_AI_KEYare wired.GET /config/features(sibling of/health) returning a named, extensible map:{ "features": { "repeatedWordCheck": true } }.useFeatureFlagshook + aFeatureGatecomponent that fails closed / hidden while loading or on error, plus an unlinked, login-gated diagnostics page that renders the flag map.Open questions for review (also in the doc)
/config/featuresacceptable, or should it sit behind auth?/config/featuresvs folding "what's enabled" into/health?EN_FEATURE_*naming + declaring each flag in the schema — comfortable with the mild DRY cost for the self-documenting/validated catalog?Docs
docs/proposals/feature-flags/feature-flags-suggestion.md— full design, decisions D1–D8, open questions, rollout.docs/proposals/feature-flags/feature-flags-summary.md— reviewer orientation.Relates to the Repeated Word Check work (fluent-api #172), whose front-end is the first consumer:
Summary by CodeRabbit