feat(flags): hide Repeated Word Check behind a feature flag (bundles #320)#337
Conversation
|
Warning Review limit reached
Next review available in: 53 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 ignored due to path filters (1)
📒 Files selected for processing (38)
📝 WalkthroughWalkthroughThis PR adds a Repeated Word Check feature to the drafting editor: types, a three-layer suppression cascade (occurrence/global/Greek Room), a ChangesRepeated Word Check Feature
Feature Flags Infrastructure and Debug Diagnostics
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
Suggested reviewers: 🚥 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: 8
🤖 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/repeated-word-check/checks-ui-integration-suggestion.md`:
- Around line 379-382: The GET /self/settings contract is inconsistent because
the docs mention updatedAt while the existing SelfSettings consumer type only
models settings. Either remove updatedAt from the repeated-word-check table
entry or update the full GET path end-to-end so the route/service/client types
and any SelfSettings-related model include updatedAt consistently.
In `@src/features/bible/components/DraftingUI.tsx`:
- Around line 216-225: The useSuppressions hook is still probing /self/settings
on mount even when the repeated-word check is disabled, so gate that behavior
behind checksEnabled. Update DraftingUI to pass checksEnabled into
useSuppressions, and adjust useSuppressions/probeSelfSettings so the settings
probe and related state initialization are skipped when checks are off. Keep the
existing suppression actions and symbols like useSuppressions,
probeSelfSettings, and settingsProbeResolved intact, but make them no-op until
checksEnabled is true.
In `@src/features/checks/checks.types.ts`:
- Around line 40-46: `RepeatedWordsRequest.project_id` is too permissive and
should be narrowed to `string` to match the strict wire contract. Update the
`RepeatedWordsRequest` interface in `checks.types.ts` so `project_id` no longer
accepts numbers, and make sure any callers such as `useRepeatedWordsCheck` only
pass string IDs. If needed, normalize or stringify the project identifier before
constructing the request so the wire schema stays strict.
In `@src/features/checks/hooks/useRepeatedWordsCheck.test.ts`:
- Around line 87-98: The mockPost helper in useRepeatedWordsCheck.test.ts
captures request bodies via calls.lastBody, but no test asserts on that value.
Either remove the unused lastBody tracking from mockPost, or add a test that
exercises the actual HTTP request path and verifies the payload sent to
http.post(CHECK_URL) matches buildRepeatedWordsRequest, using mockPost and
calls.lastBody to close the gap between the builder tests and the network
behavior.
- Around line 191-206: The negative assertion in useRepeatedWordsCheck.test.ts
is relying on a fixed 50ms sleep, which is flaky and slows the suite. Update the
“does not fire when disabled” test (and the other matching test) to assert the
disabled gate synchronously via useRepeatedWordsCheck’s enabled behavior, or
replace the timeout with fake timers / a microtask flush plus a deterministic
waitFor check on the hook state instead of waiting for a request to maybe
appear.
In `@src/features/checks/hooks/useRepeatedWordsCheck.ts`:
- Around line 91-108: The repeated-words API error handling in
postRepeatedWordsCheck should include response status and any returned body
instead of throwing a generic message. Update the non-ok branch after fetch so
the thrown Error carries res.status and, if available, the response text/json
payload to distinguish the documented 502 from other failures. Keep the change
localized to postRepeatedWordsCheck in useRepeatedWordsCheck so
Logger.logException gets actionable details.
In `@src/features/flags/FeatureFlagsDiagnostics.tsx`:
- Around line 26-33: Remove the redundant type assertion in
FeatureFlagsDiagnostics by relying on the inferred type from
Object.entries(features); since features is already a Features record, the cast
to Array<[string, boolean]> is unnecessary. Update the entries declaration in
FeatureFlagsDiagnostics to use the inferred entries type directly and keep the
rest of the component unchanged.
In `@src/features/flags/useFeatureFlags.ts`:
- Around line 31-46: The fetchFeatureFlags function currently trusts
body.features from the API without validating it, so a malformed 200 response
can bypass the fail-closed behavior. Update fetchFeatureFlags (and the query
result handling around useFeatureFlags) to validate or normalize the response
and merge any returned values over failClosedFeatures() instead of returning the
raw asserted object. Ensure every feature key remains boolean and missing keys
default to the fail-closed value so query.data never yields undefined.
🪄 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: f20b7c2f-d66b-496a-8138-1f9b91c2ab32
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (38)
docs/proposals/repeated-word-check/checks-ui-integration-suggestion.mddocs/proposals/repeated-word-check/checks-ui-integration-summary.mddocs/proposals/repeated-word-check/highlight-in-verse-suggestion.mdpackage.jsonsrc/components/ui/switch.test.tsxsrc/components/ui/switch.tsxsrc/features/bible/components/DraftingHeader.tsxsrc/features/bible/components/DraftingResourceSidebar.tsxsrc/features/bible/components/DraftingUI.test.tsxsrc/features/bible/components/DraftingUI.tsxsrc/features/bible/hooks/useResourceStatePersistence.test.tssrc/features/bible/hooks/useResourceStatePersistence.tssrc/features/checks/checks.types.tssrc/features/checks/components/ChecksPanel.test.tsxsrc/features/checks/components/ChecksPanel.tsxsrc/features/checks/components/FindingRow.test.tsxsrc/features/checks/components/FindingRow.tsxsrc/features/checks/components/LeftPanel.test.tsxsrc/features/checks/components/LeftPanel.tsxsrc/features/checks/hooks/useRepeatedWordsCheck.test.tssrc/features/checks/hooks/useRepeatedWordsCheck.tssrc/features/checks/hooks/useResolvedFindings.test.tssrc/features/checks/hooks/useResolvedFindings.tssrc/features/checks/hooks/useSuppressions.test.tssrc/features/checks/hooks/useSuppressions.tssrc/features/dashboard/user/UserHomePage.test.tsxsrc/features/flags/FeatureFlagsDiagnostics.test.tsxsrc/features/flags/FeatureFlagsDiagnostics.tsxsrc/features/flags/FeatureGate.tsxsrc/features/flags/flags.types.tssrc/features/flags/index.tssrc/features/flags/useFeatureFlags.test.tsxsrc/features/flags/useFeatureFlags.tssrc/features/projects/components/ProjectDetailPage.tsxsrc/features/resources/components/ResourcePanel.tsxsrc/lib/types.tssrc/routeTree.gen.tssrc/routes/_authenticated/debug.tsx
- rename wire field targetLanguageCode -> targetLangCode (match sourceLangCode / new api key) - checks: narrow RepeatedWordsRequest.project_id to string (fluent-ai strict contract) - suppressions: skip /self/settings probe when the check feature is gated off - flags: fail-closed merge of /config/features body so partial/malformed 200s stay hidden - checks: include HTTP status in repeated-words error message - diagnostics: drop redundant entries cast - model /self/settings updatedAt on the client type; reconcile doc - tests: assert request body; replace flaky setTimeout negatives with waitFor
|
Thanks for the review @coderabbitai — all inline items addressed in 961102e:
Note: this PR is paired with fluent-api#213 — the |
|
Noted on the ✏️ Learnings added
✅ Action performedReview finished.
|
- Move settings endpoint to a new self domain (GET/PUT /self/settings, authenticateUser only, no userId in path) per kaseywright - Rename actions to 'Ignore Here' / 'Ignore Everywhere' (joelthe1/Ulf) - Follow revised #278: 'Show Ignored' toggle (default off, not persisted), 'Undo Ignore', Ignore Here/Always/Default Ignore labels - Add confirm dialog on 'Ignore Everywhere' while keeping undo reversibility - Drop the showResolvedChecks editor-state key (only activeLeftTab persists) - Mark S1 (zero-state dot) and S2 (language dropdown) resolved; add S7/S8 - Document highlight-in-verse as a follow-on PR - Add §0 change log
…ier to directory-tree code fences (mermaid fence already carried its specifier)
Document why [Undo] reversibility falls out of the layered cascade design rather than being an added feature, and fold out the now-redundant 'why undo despite cannot be undone' bullet. Framed as documentation for anyone following the reasoning, not a request to override the card's 'cannot be undone' wording (S7 remains a confirm-only sign-off item).
Implements the unblocked portions of the Checks UI per the proposal
(docs/proposals/repeated-word-check, decisions W1-W12, file layout S6.1):
- features/checks/checks.types.ts: wire types verbatim snake_case per
fluent-api D8 (lang_code, snt_id, repeated_word, start_position, surf,
legitimate, severity) plus the full ToolJobResponse envelope; UI-derived
types in camelCase (RuleVerdict, OccurrenceRules, ResolvedFinding, ...).
- features/checks/hooks/useRepeatedWordsCheck.ts: TanStack useQuery keyed
(chapterAssignmentId, saveCounter), POST /ai/tools/greek-room/repeated-words
with credentials:'include', consumes the full envelope and branches on
status. snt_id = '{bookCode} {chapter}:{verse}' using projectItem.bookCode
(USFM code). Follows the features/bible/hooks/useBibleTarget.ts fetch pattern.
- features/checks/hooks/useResolvedFindings.ts (+ tests): pure three-layer
active/inactive cascade (W5/W6, most-specific-non-silent-wins) with NFC-exact
comparison (no case folding) and zero-based per-verse ordinals. 24 tests cover
the precedence matrix, ordinals incl. 'the the the', and NFC equivalence.
- features/checks/components/LeftPanel.tsx (+ tests): Resources | Checks tab
container with content-agnostic slots and a notification dot beside 'Checks'
when active findings > 0.
- resources/components/ResourcePanel.tsx: remove the <h3>Resources</h3> header
(the tab row is now the header).
- bible/hooks/useResourceStatePersistence.ts: persist activeLeftTab.
Held for sign-off (not included): S5 toggle-button dot, S7 confirm-dialog +
[Undo], ChecksPanel/FindingRow, useSuppressions, DraftingPage wiring, and the
fluent-api /self/settings persistence half.
Relates to #277, #278
Track per-deliverable build progress on the implementation branch. The proposal branch / PR #305 deliberately preserves the as-reviewed, all-unchecked state; this checklist advances on jel-checks-ui-impl as files land. Marks the committed foundations done and the held items (DraftingPage wiring, ChecksPanel/FindingRow, useSuppressions, the fluent-api /self/settings backend) as pending.
Implementation progress is now tracked privately in the execution harness (self-notes/), not in a doc under the team repo. The proposal docs stay focused on the design.
- Owns global word-pair ignores via /self/settings (session probe + optimistic PUT + rollback; full-object-replace body matching the Phase 1 contract); occurrence rules handled through an injected editor-state writer so useResourceStatePersistence stays the single writer of the editor-state blob. - Extend useResourceStatePersistence state shapes with optional checkOccurrenceRules. - 16 new tests (probe, pure helpers, occurrence round-trip, global optimistic+purge+rollback); lint 0/0, typecheck + prettier clean. - Drive-by: MultiSelectFilter ||->?? to clear a pre-existing lint warning. Relates to #277, #278
Presentational Checks-tab body for the Repeated Word Check (cards #277/#278): - FindingRow: active rows show [Ignore Here]/[Ignore Everywhere] (the latter gated on globalIgnoresAvailable and opening an S7 confirm dialog, firing only on confirm); inactive rows render dimmed with the ignore-type label (Ignore Here / Ignore Always / Default Ignore) and an [Undo \u25be] split button whose chevron offers the deliberate global stop-ignoring action. - ChecksPanel: per-check accordion (Repeated Words, expanded by default), findings grouped by verse with 'Verse N' headings (robust snt_id parse with raw-id fallback) and separators between groups, active-before-inactive, session-local 'Show Ignored' toggle (default off, not persisted), centered 'No issues found' zero state, and an inline 'Checks failed to refresh' error line that never shows the zero state on error. Components are presentational (no query/hook ownership) so Phase 4 can compose them. 19 new tests; full suite 104 green, lint 0/0, typecheck + prettier clean.
Wire the chapter-wide Repeated Word Check into the drafting/view page: - Host LeftPanel (Resources | Checks) in place of the bare ResourcePanel, feeding ChecksPanel the resolved findings and ignore/undo actions. - Run useRepeatedWordsCheck keyed on a saveCounter that bumps on each successful verse auto-save (W3, #172); gate via enabled = !readOnly && hasContent && settingsProbeResolved (W10, sec 9.1). - Apply the three-layer cascade with useResolvedFindings + useSuppressions; occurrence rules ride the existing debounced editor-state save (one writer, sec 7.1). - Thread the active-finding count to two notification dots: the Checks tab dot and an S5 mirror dot on the closed-panel BookText toggle (#277). - Persist + hydrate activeLeftTab and checkOccurrenceRules in the editor-state blob (W11, sec 6.6). Adds useRepeatedWordsCheck.test.ts (builders + hook gating/refire/error). A full DraftingPage render test is intentionally omitted: the component is integration-heavy (router/store/loader) and the wiring behavior is already unit-covered by the checks hooks/components; the page-level contract is verified by the manual smoke pass. Relates to #277, #278, #172
…heck
Two web→AI request-shape fixes found during the Phase-4 manual smoke of the
Repeated Word Check:
- lang_code: send the ISO 639-3 code (e.g. "eng"), not the language display
name ("English"). greek-room keys its legitimate-duplicate whitelist on the
ISO code, so sending the display name silently disabled all layer-0
(legitimate) suppression — duplicates that should be auto-ignored (e.g.
"truly truly") were flagged. Adds targetLanguageCode to ProjectItem (optional,
with a "<unknown>" fallback) and UserChapterAssignment (required); lang_name
still carries the human name.
- project_id: coerce to String(). fluent-ai declares project_id: str under
Pydantic v2 strict (no int→str coercion), so a numeric value was rejected 422
and surfaced by fluent-api as 502.
Tests: new regression cases (distinct code/name, ISO-code, empty/missing-code
fallback) + factory updates. typecheck + lint clean, 116/116 green.
Relates to #277, #278
BUG #3: ignoreHere on a surfaced legitimate finding was stacking 'suppress' over 'surface', requiring two undos. Now deletes the 'surface' override instead, letting the lower layer re-suppress. BUG #4: stopIgnoringEverywhere chevron on a legitimate-only finding was a no-op (no global rule to delete). Now writes a global 'surface' to override the legitimate verdict for the pair everywhere. Also applied the toggle principle symmetrically to ignoreEverywhere (deletes a 'surface' global instead of stacking 'suppress'). Added 3 regression tests (19 total in useSuppressions). 119/119 green.
- Move ignored findings into a dedicated section below the Show Ignored toggle (previously interleaved above it) - Replace Show Ignored checkbox with a Switch (@radix-ui/react-switch) - Inactive FindingRow: label left / Undo right per mock Visual/markup only; suppression cascade and handlers untouched. Undo split-button (delta #3) intentionally left as-is.
The repeated-words check sends greek-room a `lang_code` keyed on the ISO
639-3 target language code; its 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 the project
progress endpoint and silently omitted targetLanguageCode, so the check
sent "<unknown>" and legitimate duplicates were flagged (the translator
path was unaffected because its assignment response already carried the
code).
- Add the now-server-provided targetLanguageCode to ChapterAssignmentProgress
and pass it into the ProjectItem built in ProjectDetailPage.
- Make ProjectItem.targetLanguageCode and ChapterAssignmentProgress
.targetLanguageCode required (was optional) so the compiler forces every
builder to supply it; the runtime "<unknown>" fallback is retained for
empty values so the check degrades instead of crashing.
- Update the unit test's missing-code case to use an empty string (still
exercises the runtime fallback now that the field is typed required).
Requires the matching fluent-api change that surfaces targetLanguageCode on
GET /projects/{id}/chapter-assignments/progress.
- Remove the one generated test that asserted stale findings survive a failed re-check; the query key includes saveCounter, so a post-save failure starts a new key with no cached data — the asserted retention never happens. - Make the useRepeatedWordsCheck stale-data comments honest (the panel renders from DraftingPage-held state on error, not from TanStack keeping data). - Run prettier over the generated test files (missing EOL newlines + wrapping) so lint/format:check pass. Also documents the intentional defensive optional-chain guard on targetLanguageCode at the API trust boundary.
The occurrence key is "{snt_id}|{repeated_word}|{ordinal}". purgeLocalForPair
previously recovered the pair via split('|')[1], which assumes repeated_word
contains no '|'. The two outer fields (snt_id, ordinal) are provably pipe-free,
so slice the middle field between the first and last '|' instead, making the
purge correct even if a verse-derived pair contains a pipe. Adds regression
tests for pairs containing '|', '"', and '\'.
…ePersistence LeftPanel re-declared the identical 'resources' | 'checks' union already exported from useResourceStatePersistence (the editor-state activeLeftTab field, which DraftingPage imports). Import the type from there and re-export it so LeftPanel's public type surface is unchanged. Type-only; no runtime or persisted-shape change.
Wire the drafting-page left-panel tabs to their shared body container: each tab gets a stable id + aria-controls, and the panel div now has role=tabpanel, an id, and aria-labelledby tracking the active tab. Tidies our own unmerged tab markup so assistive tech announces the controlled panel. Markup/attribute-only; no behavior, state, or wire-contract change. Adds two unit tests covering the aria-controls and aria-labelledby wiring.
buildRepeatedWordsRequest trimmed targetLanguageCode only to test
truthiness but forwarded the untrimmed value, so a padded code (" spa ")
would reach the wire verbatim. greek-room keys its legitimate-duplicate
whitelist on the exact code, so a padded value silently disables
suppression. Compute the trimmed code once and forward that; the
<unknown> fallback for missing/empty codes is unchanged. Add a test for
the padded-code case.
writeGlobal optimistically purges the current chapter's occurrence rules (a persisted write via saveOccurrenceRules) before the /self/settings PUT, but the failure catch only rolled back the global map. Snapshot the occurrence map before the purge and restore it in the catch so a failed global write no longer drops the user's per-occurrence ignores for that pair. Adds a regression test for the rollback.
…/CR-12) CR-10 (DraftingPage): give the resource-toggle icon Button an accessible name via aria-label (Show/Hide Resources, mirroring the tooltip), and make the active-checks notification dot decorative (aria-hidden) so a screen reader announces the button action rather than the dot's label. The dot keeps its data-testid. CR-12 (ChecksPanel): groupByVerse() now sorts emitted groups by parsed verse number (segment after the last ':'; leading digits of a range like '4-5'; malformed -> POSITIVE_INFINITY so it sorts last and the never-blank-heading guarantee holds) instead of relying on first-seen input order. Doc comment updated accordingly. Behavior-neutral: no test changes required (no test queried the dot by label; no test asserted first-seen ordering).
…e rollback Two CodeRabbit findings in useSuppressions.ts (PR #320), both in the global-write path, fixed together: CR-14 (data-loss forward-risk): PUT /self/settings is a full-replace of the whole JSONB blob (the API upsert overwrites `settings` wholesale, no server-side merge), but we were sending only `{ checkIgnoredWordPairs }`, so any sibling settings key would be dropped on every global-ignore write. Carry the full GET `settings` snapshot through ProbeResult, hold it in a ref, and send `{ settings: { ...snapshot, checkIgnoredWordPairs: rules } }` so siblings ride through untouched. SelfSettings gains an index signature for unknown sibling keys. Stays on the camelCase /self/settings boundary. Regression test added: GET returns checkIgnoredWordPairs + a sibling -> PUT body still carries the sibling unchanged. CR-15 (concurrency, successor to CR-7): writeGlobal's catch restored call-time snapshots unconditionally, so a failed PUT could clobber a newer in-flight edit. Add a writeSeqRef sequence guard: each call captures mySeq, and the catch only rolls back when mySeq is still the latest issued write.
…tion order The sequence guard added earlier only protects the client rollback path (a stale failed write can't clobber a newer in-flight edit's optimistic state). It does not prevent two overlapping *successful* full-replace PUTs to /self/settings from committing out of order on the server (last-writer-wins, no server merge), which could persist an older blob even though the UI shows the newer edit and have it read back on the next session. Chain each global write onto the previous one via a putChainRef promise so a PUT does not start until the prior one settles. A leading no-op .catch keeps a failed write from breaking the chain for later writes; each write still reports its own failure in its trailing .catch (the rollback guard there is unchanged). Net ~6 lines, no API change. Adds a regression test: with the first PUT held open, a second global toggle does not issue its PUT until the first completes, and the two bodies arrive in UI-action order.
…-restore only purged occurrence keys CodeRabbit thread 3488525698 (NEW-C, follow-up to the CR-15 serialization). With global PUTs now serialized, a queued write B can capture a call-time snapshot that still holds an earlier write A's optimistic-but-unconfirmed state. On a double-failure (A fails, B fails) the seq-guard lets B roll back, which previously restored that phantom state. B now rolls back to a committedGlobalRef updated only on PUT success, so an unconfirmed change can't survive a rollback. Also restore occurrence rules surgically: capture exactly the keys the optimistic purge removed and merge only those back into the live occurrence map, instead of overwriting the whole map with a stale call-time snapshot — so an unrelated occurrence edit made during the in-flight window is preserved. committedGlobalRef is written only in the success path and read only in the catch, keeping the happy path unchanged. +2 regression tests (186 total).
…en AI is unwired Add a fail-closed feature-flag consumer (src/features/flags/) over the API's GET /config/features endpoint, and use it to hide the Repeated Word Check end-to-end when its flag is off: feat(flags): consume feature flags to hide the Repeated Word Check when AI is unwired Add a fail-closed feature-flag consumer (src/features/flags/) over the API's GET /config/features endpoint, and use it to hide the Repeated Word Check end-to-end when its flag is off: - useFeatureFlags / useFeatureFlag / FeatureGate + types, fail-closed (every flag reads false while loading or on error), retry disabled. - DraftingUI consumes the flag once: gates the useRepeatedWordsCheck query's `enabled` (no known-failing POST when off), forces the active-findings count to 0, and forces the left panel to Resources. - DraftingResourceSidebar / LeftPanel gain showChecksTab (default true); LeftPanel hides the Checks tab + notification dot when off. - Add an unlinked, login-gated, read-only feature-flags diagnostics page at /_authenticated/debug (proposal D8). All gates green (lint/format/typecheck/test/build). - useFeatureFlags / useFeatureFlag / FeatureGate + types, fail-closed (every flag reads false while loading or on error), retry disabled. - DraftingUI consumes the flag once: gates the useRepeatedWordsCheck query's (no known-failing POST when off), forces the active-findings count to 0, and forces the left panel to Resources. - DraftingResourceSidebar / LeftPanel gain showChecksTab (default true); LeftPanel hides the Checks tab + notification dot when off. - Add an unlinked, login-gated, read-only feature-flags diagnostics page at /_authenticated/debug (proposal D8). All gates green (lint/format/typecheck/test/build).
- rename wire field targetLanguageCode -> targetLangCode (match sourceLangCode / new api key) - checks: narrow RepeatedWordsRequest.project_id to string (fluent-ai strict contract) - suppressions: skip /self/settings probe when the check feature is gated off - flags: fail-closed merge of /config/features body so partial/malformed 200s stay hidden - checks: include HTTP status in repeated-words error message - diagnostics: drop redundant entries cast - model /self/settings updatedAt on the client type; reconcile doc - tests: assert request body; replace flaky setTimeout negatives with waitFor
961102e to
0e6dae9
Compare
|
Paired with the fluent-api Q1 auth-gate (eten-tech-foundation/fluent-api#213, commit 0812a39): the web client already sends |
Summary
Adds the fluent-web consumer half of the feature-flags mechanism and uses it to hide the Repeated Word Check end-to-end when the flag is off — so the AI-dependent Checks UI can merge and ship hidden in environments where fluent-ai isn't wired.
This PR bundles two things that must land together:
src/features/flags/module plus the wiring that hides the Checks UI when the flag reads false.Dependencies
GET /config/featuresendpoint this UI reads. That endpoint must be deployed for the flag to have any effect (until then, the UI fails closed — the Checks feature stays hidden).⏳ Created ahead of proposal sign-off
The feature-gating design is proposed in fluent-api #211 (eten-tech-foundation/fluent-api#211) and has not yet been formally authorized. This PR is opened ahead of that sign-off to unblock work. Any feedback on the proposal in #211 that changes the shape or naming of the mechanism (e.g. the
/config/featurescontract, the flag key, the fail-closed behavior) will be folded back into this PR before merge.What's in the flag half
src/features/flags/(new) — a fail-closed feature-flag consumer overGET /config/features:useFeatureFlags()/useFeatureFlag(name)(TanStack Query,staleTime5 min, retry disabled) — every flag readsfalsewhile loading or on error, so a slow/failing API keeps gated UI hidden (the safe state) and self-corrects on the next refetch.<FeatureGate>render-wrapper (default fallbacknull) + types.DraftingUI:enabledguard folds in the flag, so no known-failing request fires when the feature is off;showChecksTabthreaded down throughDraftingResourceSidebar→LeftPanel; leaf components stay flag-agnostic)./debug(login-gated, no role gate, no nav entry) that shows whatGET /config/featuresreports for this environment, with an On/Off pill per flag and a Refresh button. Useful for confirming a flag flip took effect.Verification
EN_FEATURE_REPEATED_WORD_CHECK=falsepublished by fluent-api, the Checks tab disappears and no check request is issued; flipping it on restores the tab.How to toggle (operators)
The flag is env-sourced in fluent-api (
EN_FEATURE_REPEATED_WORD_CHECK, defaulting to on only when fluent-ai is wired). fluent-web reads the published value live — no web rebuild needed; reload the page (5-min stale window) to pick up a change.In the local platform stack:
Set the value in
fluent-api/.env, e.g.EN_FEATURE_REPEATED_WORD_CHECK=false.Re-create the api container so it re-reads
.env— a plainrestartdoes not re-readenv_file:Confirm what the API now publishes:
curl -s localhost:9999/config/features # -> {"features":{"repeatedWordCheck":false}}Reload fluent-web to pick up the change (5-min stale window). You can also open the read-only diagnostics page at http://localhost:5173/debug (sign-in required, no nav link) to see each flag's On/Off state, with a Refresh button.
Summary by CodeRabbit
New Features
Bug Fixes