diff --git a/docs/proposals/repeated-word-check/checks-ui-integration-suggestion.md b/docs/proposals/repeated-word-check/checks-ui-integration-suggestion.md index 43756f3a..dac70a6d 100644 --- a/docs/proposals/repeated-word-check/checks-ui-integration-suggestion.md +++ b/docs/proposals/repeated-word-check/checks-ui-integration-suggestion.md @@ -24,14 +24,14 @@ This document's own decisions are numbered **W1–W12** (W = web) to avoid colli Revisions made in response to PR [#305](https://github.com/eten-tech-foundation/fluent-web/pull/305) review (reviewers: kaseywright, chadw-eten; product input via joelthe1/Ulf). The cards themselves were edited mid-review; this revision tracks the **current** card text and mocks (re-pulled 2026-06-16). -| Change | Driver | Where | -| --- | --- | --- | -| Settings endpoint moves from `GET/PUT /users/settings` to a new **`self` domain**: `GET/PUT /self/settings`, guarded by `authenticateUser` alone. | kaseywright (CHANGES_REQUESTED): clean semantic separation, no redundant `{id}` param, no over-exposure risk, simpler middleware (no `requireSelf`/`requireUserAccess`/`UserPolicy`). | W7, §7.2, §8 | -| Button labels become **"Ignore Here" / "Ignore Everywhere"**. | joelthe1/Ulf (card #278 comment); chadw updated the #278 mocks to match. | W6, §5, §6.5 | -| **"Ignore Everywhere" now shows a confirmation dialog**, while remaining reversible via `[Undo ▾]`. | card #278 ("Ignore Always should have a standard confirm dialog"). Kept the three-layer cascade & undo (no explicit directive against it; ambiguity of "undo" with surface/suppress entries at multiple layers is better resolved by the cascade — see §6.4/§6.5); added the dialog. | W6, §5.3, §6.5 | -| **"Show Ignored" toggle**: now default **OFF** and **not persisted** (resets per session). Ignored items show the ignore-type label (e.g. _"Ignore Here"_, _"Ignore Everywhere"_, _"Default Ignore"_) + an **"Undo Ignore"** control. | card #278 was revised to specify exactly this; we drop our earlier default-ON/persisted proposal and the `showResolvedChecks` editor-state key. | W5, W6, W11, §5.3, §6.4, §6.6, §7.1 | -| §5.2 mock/text inconsistencies **S1 (zero-state dot)** and **S2 (language dropdown)** are **resolved**: chadw fixed the zero-state mock and removed the language dropdown from the #278 Checks mocks. | chadw-eten ("Fixed." / "good idea… I have adjusted the mocks."). | §5.2, §12 | -| New requirement adopted: **highlight repeated words in the verse text** (underline/color). | joelthe1/Ulf (card #278 comment). We will implement it; because it touches the verse-rendering pane (outside the left panel this proposal builds) and warrants its own design pass, it ships as a **separate, follow-on PR** so the tab/panel work can land independently. §5.3 item 4 records the intent; the visual/interaction details are specced just before that PR. | §5.3, §11 | +| Change | Driver | Where | +| ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------- | +| Settings endpoint moves from `GET/PUT /users/settings` to a new **`self` domain**: `GET/PUT /self/settings`, guarded by `authenticateUser` alone. | kaseywright (CHANGES_REQUESTED): clean semantic separation, no redundant `{id}` param, no over-exposure risk, simpler middleware (no `requireSelf`/`requireUserAccess`/`UserPolicy`). | W7, §7.2, §8 | +| Button labels become **"Ignore Here" / "Ignore Everywhere"**. | joelthe1/Ulf (card #278 comment); chadw updated the #278 mocks to match. | W6, §5, §6.5 | +| **"Ignore Everywhere" now shows a confirmation dialog**, while remaining reversible via `[Undo ▾]`. | card #278 ("Ignore Always should have a standard confirm dialog"). Kept the three-layer cascade & undo (no explicit directive against it; ambiguity of "undo" with surface/suppress entries at multiple layers is better resolved by the cascade — see §6.4/§6.5); added the dialog. | W6, §5.3, §6.5 | +| **"Show Ignored" toggle**: now default **OFF** and **not persisted** (resets per session). Ignored items show the ignore-type label (e.g. _"Ignore Here"_, _"Ignore Everywhere"_, _"Default Ignore"_) + an **"Undo Ignore"** control. | card #278 was revised to specify exactly this; we drop our earlier default-ON/persisted proposal and the `showResolvedChecks` editor-state key. | W5, W6, W11, §5.3, §6.4, §6.6, §7.1 | +| §5.2 mock/text inconsistencies **S1 (zero-state dot)** and **S2 (language dropdown)** are **resolved**: chadw fixed the zero-state mock and removed the language dropdown from the #278 Checks mocks. | chadw-eten ("Fixed." / "good idea… I have adjusted the mocks."). | §5.2, §12 | +| New requirement adopted: **highlight repeated words in the verse text** (underline/color). | joelthe1/Ulf (card #278 comment). We will implement it; because it touches the verse-rendering pane (outside the left panel this proposal builds) and warrants its own design pass, it ships as a **separate, follow-on PR** so the tab/panel work can land independently. §5.3 item 4 records the intent; the visual/interaction details are specced just before that PR. | §5.3, §11 | --- @@ -54,12 +54,12 @@ The ignore actions require server-side persistence that does not exist yet. #172 ### 1.2 Repos touched -| Repo | Touched | What changes | -| --- | --- | --- | -| **fluent-web** | yes (bulk) | Left-panel tab container, Checks panel feature, check-trigger hook, suppression cascade, editor-state keys. | -| **fluent-api** | yes (small) | New `user_settings` table + a new `self` domain exposing `GET/PUT /self/settings`; Zod-schema extension of the editor-state `resources` blob (no migration). | -| **fluent-ai** | no | The check itself is unchanged. | -| **fluent-platform** | no | No new services or env vars. | +| Repo | Touched | What changes | +| ------------------- | ----------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| **fluent-web** | yes (bulk) | Left-panel tab container, Checks panel feature, check-trigger hook, suppression cascade, editor-state keys. | +| **fluent-api** | yes (small) | New `user_settings` table + a new `self` domain exposing `GET/PUT /self/settings`; Zod-schema extension of the editor-state `resources` blob (no migration). | +| **fluent-ai** | no | The check itself is unchanged. | +| **fluent-platform** | no | No new services or env vars. | --- @@ -92,20 +92,20 @@ The ignore actions require server-side persistence that does not exist yet. #172 Restated conclusions; supporting analysis in the cited sections. -| # | Decision | Short rationale | -| --- | --- | --- | -| **W1** | Suppression persistence ships as an extension of #172's scope; no new product card. | #172's wording covers backend dependencies of the UI work; D1's deferral was about caching tool results, not user preferences. See §1.1. | -| **W2** | Hybrid storage: "Ignore This Time" lives in the existing per-chapter editor-state JSONB (schema-key addition, no migration); "Ignore Always" lives in a new `user_settings` table (one row per user, single Zod-typed JSONB column). Findings are filtered **client-side**. | Each rule is stored at exactly the scope it governs. Client-side filtering keeps the AI proxy a pure pass-through (D8/D9). `user_settings` deliberately establishes Fluent's user-global preference store: future settings become schema extensions, not migrations. See §7. | -| **W3** | Chapter-wide check on every successful verse save: a TanStack `useQuery` keyed on `(chapterAssignmentId, saveCounter)`, where `saveCounter` increments in the verse-save mutation's `onSuccess`. The request sends **all currently drafted verses** of the chapter. No extra coalescing debounce. | The dot needs chapter-wide awareness; per-verse merging is more code for no user-visible gain. The existing 2 s per-verse save debounce already rate-limits; the check is <1 s. KISS — optimize if heavier checks arrive. See §6.2. | -| **W4** | `snt_id` = `"{bookCode} {chapter}:{verse}"` (USFM book code, e.g. `JDG 4:3`), matching the convention already used by the repo smoke tests. Occurrence identity for suppressions = `(snt_id, repeated_word, ordinal)` where ordinal is the index of the finding among same-`repeated_word` findings in the verse, ordered by `start_position`. | Ordinals survive unrelated edits (positions don't); adding/removing an earlier same-pair occurrence conservatively re-flags, the safe failure direction. We count Greek Room's findings, never tokenize text ourselves, so Greek Room's equivalence policy is inherited consistently. See §6.3. | -| **W5** | Every finding resolves to **active/inactive** via a three-layer cascade — Greek Room verdict (`legitimate`), user-global word-pair rule, occurrence rule — where the **most specific non-silent verdict wins**. Rules are tri-state maps (`absent / 'suppress' / 'surface'`). The dot counts active findings only. Inactive findings are hidden by default and revealed by the **"Show Ignored" toggle** (default OFF, not persisted, per revised #278); when shown they render greyed with the ignore-type label. | One mechanism explains machine-legitimate and user-ignored alike ("pre-ignored by Greek Room"). Specificity, not temporal order, keeps resolution deterministic. Greyed-not-removed (under the toggle) keeps undo discoverable. See §6.4. | -| **W6** | Active findings show `[Ignore Here] [Ignore Everywhere]` (per revised card). `[Ignore Everywhere]` opens a **standard confirm dialog** before writing the global rule. Inactive findings show one `[Undo ▾]` split button: default click acts at the occurrence layer (delete own rule, or write an occurrence `surface` override); the chevron menu offers explicit global actions with consequence-naming labels. Any global write first purges the user's occurrence rules for that pair **in the current chapter's editor state only**. | Default click never silently edits global state. The card asks for a confirm on the global action because it reads as irreversible; we add the dialog **and** keep the `[Undo ▾]` reversibility — the two are complementary (confirm guards the click; undo guards the mistake). Purge-local prevents the just-clicked panel from appearing to ignore the action; other chapters' specific pronouncements deliberately stand. See §6.5. | -| **W7** | Settings endpoint lives in a **new `self` domain**: **`GET /self/settings` + `PUT /self/settings`**, session-implicit user (no user in URL), guarded by `authenticateUser` alone, full-replace upsert of one Zod-typed JSONB blob. File quartet `domains/self/settings/self-settings.{route,service,repository,types}.ts`. | Per kaseywright: a `self` domain gives clean semantic separation, avoids the redundant `{userId}` param (and the `requireSelf`/`UserPolicy` middleware that `/users/{id}/…` routes carry), and prevents accidental over-exposure (no `{id}` to misuse). Last-writer-wins on concurrent tabs is inherited from editor-state and accepted. See §8. | -| **W8** | Graceful degradation by feature detection: if `GET /self/settings` 404s, the session records `globalIgnoresAvailable = false` and the `[Ignore Everywhere]` button plus global menu entries are **not rendered** (capability hidden, never a dead control). Unknown/absent JSONB keys parse as empty on both sides. Either repo's PR can land first. | The UI must not assume the backend half exists. An invisible capability is honest; a dead button is a bug report. See §9. | -| **W9** | Check-call failure surfaces as a single inline `text-sm text-red-500` line at the top of the Checks panel ("Checks failed to refresh"); the panel keeps rendering the last successful findings (TanStack keeps `query.data` on refetch failure) and the dot reflects that last-known state. Failures log via the existing `Logger`. No toast/banner/popup. | Matches the drafting page's own inline-status precedent ("Auto-save failed"). Failure mode degrades to "results are one save behind." See §9.2. | -| **W10** | The check runs in drafting mode only — not in the read-only `/view` route — and is skipped (`enabled: false`) when no verse has content and while the settings feature-detection probe (§9.1) is unresolved. | Card scope is the drafting view; empty chapters have nothing to check; the probe is one fast `GET`, and waiting for it means the first findings render is already cascade-correct. Review-stage checks are future work. See §6.2. | -| **W11** | Left-panel architecture follows the mocks: a text-tab header row ("Resources \| Checks", blue underline active state, blue dot after "Checks"), dot visible from either tab; Checks content = per-check accordion sections with verse-grouped snippets, the two buttons, and a "Show Ignored" toggle at the bottom; zero state inside the accordion. Only `activeLeftTab` is persisted in editor state (the "Show Ignored" state resets per session, per revised #278). Proposed deviation for sign-off: when the whole panel is closed, the dot is mirrored on the panel-toggle button. | Mock-faithful where the mocks speak; the toggle-button dot preserves #277's intent (translator is notified) when the panel is hidden. See §5. | -| **W12** | One proposal document (this file) covering both repos; two implementation PRs (fluent-web, fluent-api), cross-referencing each other and the cards. | Splitting the proposal doubles reviewer overhead for a design whose halves only make sense together. See §9.4. | +| # | Decision | Short rationale | +| ------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **W1** | Suppression persistence ships as an extension of #172's scope; no new product card. | #172's wording covers backend dependencies of the UI work; D1's deferral was about caching tool results, not user preferences. See §1.1. | +| **W2** | Hybrid storage: "Ignore This Time" lives in the existing per-chapter editor-state JSONB (schema-key addition, no migration); "Ignore Always" lives in a new `user_settings` table (one row per user, single Zod-typed JSONB column). Findings are filtered **client-side**. | Each rule is stored at exactly the scope it governs. Client-side filtering keeps the AI proxy a pure pass-through (D8/D9). `user_settings` deliberately establishes Fluent's user-global preference store: future settings become schema extensions, not migrations. See §7. | +| **W3** | Chapter-wide check on every successful verse save: a TanStack `useQuery` keyed on `(chapterAssignmentId, saveCounter)`, where `saveCounter` increments in the verse-save mutation's `onSuccess`. The request sends **all currently drafted verses** of the chapter. No extra coalescing debounce. | The dot needs chapter-wide awareness; per-verse merging is more code for no user-visible gain. The existing 2 s per-verse save debounce already rate-limits; the check is <1 s. KISS — optimize if heavier checks arrive. See §6.2. | +| **W4** | `snt_id` = `"{bookCode} {chapter}:{verse}"` (USFM book code, e.g. `JDG 4:3`), matching the convention already used by the repo smoke tests. Occurrence identity for suppressions = `(snt_id, repeated_word, ordinal)` where ordinal is the index of the finding among same-`repeated_word` findings in the verse, ordered by `start_position`. | Ordinals survive unrelated edits (positions don't); adding/removing an earlier same-pair occurrence conservatively re-flags, the safe failure direction. We count Greek Room's findings, never tokenize text ourselves, so Greek Room's equivalence policy is inherited consistently. See §6.3. | +| **W5** | Every finding resolves to **active/inactive** via a three-layer cascade — Greek Room verdict (`legitimate`), user-global word-pair rule, occurrence rule — where the **most specific non-silent verdict wins**. Rules are tri-state maps (`absent / 'suppress' / 'surface'`). The dot counts active findings only. Inactive findings are hidden by default and revealed by the **"Show Ignored" toggle** (default OFF, not persisted, per revised #278); when shown they render greyed with the ignore-type label. | One mechanism explains machine-legitimate and user-ignored alike ("pre-ignored by Greek Room"). Specificity, not temporal order, keeps resolution deterministic. Greyed-not-removed (under the toggle) keeps undo discoverable. See §6.4. | +| **W6** | Active findings show `[Ignore Here] [Ignore Everywhere]` (per revised card). `[Ignore Everywhere]` opens a **standard confirm dialog** before writing the global rule. Inactive findings show one `[Undo ▾]` split button: default click acts at the occurrence layer (delete own rule, or write an occurrence `surface` override); the chevron menu offers explicit global actions with consequence-naming labels. Any global write first purges the user's occurrence rules for that pair **in the current chapter's editor state only**. | Default click never silently edits global state. The card asks for a confirm on the global action because it reads as irreversible; we add the dialog **and** keep the `[Undo ▾]` reversibility — the two are complementary (confirm guards the click; undo guards the mistake). Purge-local prevents the just-clicked panel from appearing to ignore the action; other chapters' specific pronouncements deliberately stand. See §6.5. | +| **W7** | Settings endpoint lives in a **new `self` domain**: **`GET /self/settings` + `PUT /self/settings`**, session-implicit user (no user in URL), guarded by `authenticateUser` alone, full-replace upsert of one Zod-typed JSONB blob. File quartet `domains/self/settings/self-settings.{route,service,repository,types}.ts`. | Per kaseywright: a `self` domain gives clean semantic separation, avoids the redundant `{userId}` param (and the `requireSelf`/`UserPolicy` middleware that `/users/{id}/…` routes carry), and prevents accidental over-exposure (no `{id}` to misuse). Last-writer-wins on concurrent tabs is inherited from editor-state and accepted. See §8. | +| **W8** | Graceful degradation by feature detection: if `GET /self/settings` 404s, the session records `globalIgnoresAvailable = false` and the `[Ignore Everywhere]` button plus global menu entries are **not rendered** (capability hidden, never a dead control). Unknown/absent JSONB keys parse as empty on both sides. Either repo's PR can land first. | The UI must not assume the backend half exists. An invisible capability is honest; a dead button is a bug report. See §9. | +| **W9** | Check-call failure surfaces as a single inline `text-sm text-red-500` line at the top of the Checks panel ("Checks failed to refresh"); the panel keeps rendering the last successful findings (TanStack keeps `query.data` on refetch failure) and the dot reflects that last-known state. Failures log via the existing `Logger`. No toast/banner/popup. | Matches the drafting page's own inline-status precedent ("Auto-save failed"). Failure mode degrades to "results are one save behind." See §9.2. | +| **W10** | The check runs in drafting mode only — not in the read-only `/view` route — and is skipped (`enabled: false`) when no verse has content and while the settings feature-detection probe (§9.1) is unresolved. | Card scope is the drafting view; empty chapters have nothing to check; the probe is one fast `GET`, and waiting for it means the first findings render is already cascade-correct. Review-stage checks are future work. See §6.2. | +| **W11** | Left-panel architecture follows the mocks: a text-tab header row ("Resources \| Checks", blue underline active state, blue dot after "Checks"), dot visible from either tab; Checks content = per-check accordion sections with verse-grouped snippets, the two buttons, and a "Show Ignored" toggle at the bottom; zero state inside the accordion. Only `activeLeftTab` is persisted in editor state (the "Show Ignored" state resets per session, per revised #278). Proposed deviation for sign-off: when the whole panel is closed, the dot is mirrored on the panel-toggle button. | Mock-faithful where the mocks speak; the toggle-button dot preserves #277's intent (translator is notified) when the panel is hidden. See §5. | +| **W12** | One proposal document (this file) covering both repos; two implementation PRs (fluent-web, fluent-api), cross-referencing each other and the cards. | Splitting the proposal doubles reviewer overhead for a design whose halves only make sense together. See §9.4. | --- @@ -141,7 +141,7 @@ The response envelope is consumed whole (D9): the hook inspects `status` and `re The cards' mockups (drafting page, Judges 4, Gujarati IRV project) define the visual target. The #278 mocks were updated during the first review round (§0); the description below tracks the **current** mocks: -- **Tab header (#277 mock):** the left panel's current `Resources` heading is replaced by a text-tab row — **"Resources | Checks"** — active tab in blue with a blue underline, inactive tab plain. The **notification dot is a solid blue filled circle immediately right of the "Checks" label**, and the mock shows it while the *Resources* tab is active: the dot is visible from either tab whenever the panel is open. +- **Tab header (#277 mock):** the left panel's current `Resources` heading is replaced by a text-tab row — **"Resources | Checks"** — active tab in blue with a blue underline, inactive tab plain. The **notification dot is a solid blue filled circle immediately right of the "Checks" label**, and the mock shows it while the _Resources_ tab is active: the dot is visible from either tab whenever the panel is open. - **Checks content (#278 mocks):** below the tab header, a card containing a **collapsible "Repeated Words" accordion section** (expanded by default). The accordion-per-check structure anticipates future sibling checks (the board already holds draft cards for Greek Room Wildebeest and Spell checks) without UI rework. Inside the section, findings are grouped by verse: - bold **"Verse N"** heading per verse that has findings (verses without findings get no group); - one row per occurrence: a one-line context snippet showing the repeated pair in context; @@ -221,14 +221,14 @@ useQuery({ - **Request body** is the full `RepeatedWordsRequest` (D8 shape): `lang_code`/`lang_name` from the project's target language, `project_id`/`project_name` from `projectItem`, and `verses[]` covering **all currently drafted verses** of the chapter (content from the drafting state, not a refetch — what the translator sees is what gets checked). - **`snt_id` = `"{bookCode} {chapter}:{verse}"`** with the USFM book code (e.g. `JDG 4:3`), the convention the repo smoke tests already use. Implementation note: verify the field carrying the USFM code on the drafting page's `projectItem` (vs. display name) and thread it into the builder. - **Where it runs (W10):** drafting route only; `enabled` is false in the read-only `/view` route (reviewers see no Checks activity in v1), when no verse has content, and while the settings feature-detection probe (§9.1) is unresolved — the probe is a single fast `GET`, and waiting for it means the first findings render is already cascade-correct (the user's global rules are known, present or absent). -- **Emptied chapters resolve naturally:** any save of emptied text triggers a fresh check whose empty findings clear the panel and dot. (When *every* verse is emptied the query disables instead; the panel and dot then treat findings as empty rather than rendering stale data.) +- **Emptied chapters resolve naturally:** any save of emptied text triggers a fresh check whose empty findings clear the panel and dot. (When _every_ verse is emptied the query disables instead; the panel and dot then treat findings as empty rather than rendering stale data.) ### 6.3 Occurrence identity (W4) A suppression must survive verse edits without our re-parsing text. The key is **`(snt_id, repeated_word, ordinal)`**: - `ordinal` = index of this finding among findings in the same verse with the same `repeated_word`, ordered by `start_position` ("x of n"). Computed from **Greek Room's findings only** — we never tokenize verse text ourselves, so Greek Room's case/diacritic equivalence policy is inherited and consistent between runs. -- Ordinals survive unrelated edits (`start_position` does not). Adding or removing an *earlier* same-pair occurrence shifts later ordinals and conservatively re-flags — the safe failure direction. +- Ordinals survive unrelated edits (`start_position` does not). Adding or removing an _earlier_ same-pair occurrence shifts later ordinals and conservatively re-flags — the safe failure direction. - String comparison between a stored rule's `repeated_word` and a fresh finding's: **NFC-normalize both, compare exactly, no case folding of our own.** Unicode case folding is locale-sensitive (the Turkish dotless-ı problem) and Fluent targets minority languages; NFC handles composed/decomposed accent variation. Note that Greek Room already delivers `repeated_word` lowercased ("word word" form, per the fluent-ai schema) — original casing lives only in `surf`, which we display but never compare — so case equivalence is wholly Greek Room's policy, inherited rather than re-implemented. - Documented caveat: a triple repetition ("the the the") yields two overlapping findings (ordinals 1 and 2). Mechanically fine; slightly odd UX; accepted for v1. @@ -236,11 +236,11 @@ A suppression must survive verse edits without our re-parsing text. The key is * Every finding resolves through three layers; the **most specific non-silent verdict wins** (specificity, not temporal order — deterministic and reproducible from stored state): -| Layer | Scope | Verdicts | -| --- | --- | --- | -| 0 — Greek Room | this finding | `active` (suspicious) / `inactive` (`legitimate: true`) — always present | -| 1 — User global | word pair, all the user's projects | _silent_ / `suppress` / `surface` | -| 2 — Occurrence | `(snt_id, repeated_word, ordinal)`, this chapter assignment | _silent_ / `suppress` / `surface` | +| Layer | Scope | Verdicts | +| --------------- | ----------------------------------------------------------- | ------------------------------------------------------------------------ | +| 0 — Greek Room | this finding | `active` (suspicious) / `inactive` (`legitimate: true`) — always present | +| 1 — User global | word pair, all the user's projects | _silent_ / `suppress` / `surface` | +| 2 — Occurrence | `(snt_id, repeated_word, ordinal)`, this chapter assignment | _silent_ / `suppress` / `surface` | - Stored rules are **tri-state maps** (`key → 'suppress' | 'surface'`, absent = silent), not bare suppression sets — `surface` is what makes per-occurrence undo of a global rule (or of a Greek Room `legitimate` verdict) possible. - A machine-legitimate finding is conceptually "pre-ignored by Greek Room": same inactive state as user-ignored, different actor, same undo affordance. @@ -253,17 +253,18 @@ Every finding resolves through three layers; the **most specific non-silent verd - **Confirm dialog on `[Ignore Everywhere]`** (revised #278): the card asks for it because the action reads as global and irreversible. We honor the request **and** keep the action reversible via `[Undo ▾]` below — the dialog guards the deliberate click; the undo guards a mistake that slips past it. The two are not redundant: confirm is pre-action friction, undo is post-action recourse. Why reversibility falls out of the design rather than being an extra feature we chose to bolt on — the implication chain (see also §6.4, which this depends on): - 1. Greek Room reports **both** polarities: repetitions it flags (suspicious) and repetitions it judges `legitimate` (a *negative* / "don't flag" verdict). + 1. Greek Room reports **both** polarities: repetitions it flags (suspicious) and repetitions it judges `legitimate` (a _negative_ / "don't flag" verdict). 2. Once we decide to **show** the legitimate ones (§5.3 item 2 — so the translator can see what the checker waved through), it's natural to let the translator disagree and mark one **illegitimate** (re-surface it). 3. Expressing "re-surface this" requires a **positive** record (`surface`), not only the negative (`suppress`) records an ignore-list would hold. So the stores must be tri-state, not suppression-only (§6.4). - 4. Given positive *and* negative records at more than one scope, a **layered** model is the natural fit: a local (occurrence) record can override a global (word-pair) one, and the override works in **both polarities** (locally surface something suppressed globally, or locally suppress something surfaced). - 5. With that layering in place, a single bare "undo" is **ambiguous** — at which layer, in which polarity? Resolving that ambiguity (the `[Undo ▾]` split button: default = act at the occurrence layer; chevron = act globally) is what the design *has* to do anyway — and the same mechanism incidentally gives the translator the "oops, I hit the wrong button" recourse for `[Ignore Everywhere]`. Reversibility is therefore inherited, not added. + 4. Given positive _and_ negative records at more than one scope, a **layered** model is the natural fit: a local (occurrence) record can override a global (word-pair) one, and the override works in **both polarities** (locally surface something suppressed globally, or locally suppress something surfaced). + 5. With that layering in place, a single bare "undo" is **ambiguous** — at which layer, in which polarity? Resolving that ambiguity (the `[Undo ▾]` split button: default = act at the occurrence layer; chevron = act globally) is what the design _has_ to do anyway — and the same mechanism incidentally gives the translator the "oops, I hit the wrong button" recourse for `[Ignore Everywhere]`. Reversibility is therefore inherited, not added. We are not requesting that this override the card's "cannot be undone" framing; it is documented here for anyone who wants to follow the reasoning, and the implemented PR is reviewed again before merge. + - **Inactive finding:** one `[Undo ▾]` split button (shadcn `dropdown-menu`, already in the codebase — no hidden gestures): - - **Default click** acts at the occurrence layer: if inactive via its *own* occurrence rule → delete that entry; if inactive via a global rule or Greek Room's verdict → write an occurrence-level `surface` override (the global rule / machine verdict survives; only this occurrence resurfaces). + - **Default click** acts at the occurrence layer: if inactive via its _own_ occurrence rule → delete that entry; if inactive via a global rule or Greek Room's verdict → write an occurrence-level `surface` override (the global rule / machine verdict survives; only this occurrence resurfaces). - **Chevron menu** offers the deliberate global actions with consequence-naming labels, e.g. _"Stop ignoring 'the the' everywhere"_ (deletes the global entry). -- **Global writes purge local rules — current chapter only.** Executing any global (everywhere) suppress or surface first deletes the user's occurrence-level rules for that word pair in the *current chapter assignment's* editor state, then writes the global rule. This is purely UI coherence: without it, the just-clicked panel would appear to ignore the action (occurrence beats global in the cascade). Occurrence rules in **other** chapters/projects deliberately stand — they were specific pronouncements, and we don't revert decisions from afar. Client-side operation; no special endpoint semantics. +- **Global writes purge local rules — current chapter only.** Executing any global (everywhere) suppress or surface first deletes the user's occurrence-level rules for that word pair in the _current chapter assignment's_ editor state, then writes the global rule. This is purely UI coherence: without it, the just-clicked panel would appear to ignore the action (occurrence beats global in the cascade). Occurrence rules in **other** chapters/projects deliberately stand — they were specific pronouncements, and we don't revert decisions from afar. Client-side operation; no special endpoint semantics. - **Direct edits resolve silently** (card #278): if the translator fixes the text, the next auto-save's fresh findings simply no longer contain the occurrence — no action needed, panel and dot update. ### 6.6 Left-panel container and persisted UI state (W11) @@ -273,7 +274,7 @@ Every finding resolves through three layers; the **most specific non-silent verd - **`LeftPanel`** owns the tab header row ("Resources | Checks", blue underline on the active tab, blue dot right of "Checks" when active findings exist) and renders either the existing `ResourcePanel` or the new `ChecksPanel` below it. `ResourcePanel` loses only its `

Resources

` heading (the tab row replaces it); its content, language dropdown, and accordion are untouched. - The dot's state comes from the cascade-resolved active count (§6.4), which `DraftingPage` computes once and threads to both `LeftPanel` (tab dot) and the header toggle button (mirror dot, §5.3 item 3). The check query runs at `DraftingPage` level — **not** inside `ChecksPanel` — so the dot stays live while the Resources tab (or a closed panel) is showing. - **Persisted UI state.** One new key rides the same per-chapter editor-state blob the page already saves (debounced 500 ms, §7.1): `activeLeftTab: 'resources' | 'checks'` (reopen where you left off). The **"Show Ignored"** toggle is deliberately **not** persisted — per revised #278 it resets to off each session — so it lives in component state only. The persisted key is cosmetic; failure to persist degrades to defaults. -- The panel's existing resize/drag behavior and 20–40 % width constraints are inherited unchanged — `LeftPanel` lives *inside* the resizable container. +- The panel's existing resize/drag behavior and 20–40 % width constraints are inherited unchanged — `LeftPanel` lives _inside_ the resizable container. --- @@ -299,7 +300,7 @@ export const editorStateResourcesSchema = z .nullable(); ``` -"Ignore This Time" is scoped to *this user's view of this chapter assignment* — precisely this table's grain. The fluent-api PR extends the schema with **optional** keys (old rows parse unchanged; **no SQL migration** — the column is already JSONB): +"Ignore This Time" is scoped to _this user's view of this chapter assignment_ — precisely this table's grain. The fluent-api PR extends the schema with **optional** keys (old rows parse unchanged; **no SQL migration** — the column is already JSONB): ```ts export const editorStateResourcesSchema = z @@ -312,9 +313,7 @@ export const editorStateResourcesSchema = z tabStatus: z.boolean(), // --- NEW, all optional (backward/forward compatible) --- activeLeftTab: z.enum(['resources', 'checks']).optional(), - checkOccurrenceRules: z - .record(z.string(), z.enum(['suppress', 'surface'])) - .optional(), + checkOccurrenceRules: z.record(z.string(), z.enum(['suppress', 'surface'])).optional(), }) .nullable(); ``` @@ -331,9 +330,7 @@ export const editorStateResourcesSchema = z // fluent-api/src/db/schema.ts — NEW export const userSettingsSchema = z .object({ - checkIgnoredWordPairs: z - .record(z.string(), z.enum(['suppress', 'surface'])) - .optional(), + checkIgnoredWordPairs: z.record(z.string(), z.enum(['suppress', 'surface'])).optional(), }) .catch({}); // unknown/old shapes parse as empty, never throw (W8) @@ -359,7 +356,7 @@ export const user_settings = pgTable('user_settings', { The alternative — teaching the AI proxy to subtract suppressed findings server-side — was rejected: 1. It breaks the proxy's reviewed role as a **verbatim pass-through** (D8: contract mirrored exactly; D9: envelope passthrough). Filtering injects Fluent domain state into an AI-contract endpoint. -2. The cascade needs *all* findings anyway to render inactive items with reason labels (§5.3 item 1) — a server that pre-filters would have to return them annotated regardless, which is the client cascade with extra steps. +2. The cascade needs _all_ findings anyway to render inactive items with reason labels (§5.3 item 1) — a server that pre-filters would have to return them annotated regardless, which is the client cascade with extra steps. 3. Suppression maps are small (a translator's ignore list, not a corpus) and already at the client's fingertips via blobs it loads for other reasons. The proxy therefore remains untouched by this proposal. @@ -370,7 +367,7 @@ The proxy therefore remains untouched by this proposal. The settings endpoint lives in a **new `self` domain**, requested by kaseywright in review. `self` is the authenticated caller acting on their own resources — the route never names a user. This is cleaner than hanging settings off `/users/{userId}/…`: -- **Clean semantic separation** — `self/*` is unambiguously "me," leaving the `users/*` domain for (future) admin-style tooling that operates on *other* users by id. +- **Clean semantic separation** — `self/*` is unambiguously "me," leaving the `users/*` domain for (future) admin-style tooling that operates on _other_ users by id. - **No redundant `{userId}` param** — the user comes from the session, not the URL (the same source `user-chapter-assignment-editor-state.route.ts` already uses). - **No accidental over-exposure** — with no `{id}` in the path there is nothing to mis-scope; contrast `/users/{userId}/projects`, which must add `requireSelf()` to stop one user reading another's data. - **Simpler middleware** — just `authenticateUser`. No `requireSelf`, no `requireUserAccess`, no `UserPolicy`. @@ -379,10 +376,10 @@ Otherwise it keeps the editor-state idiom: **full-replace upsert** on PUT, one Z ### 8.1 Routes -| Method | Path | Auth | Body | Response | -| --- | --- | --- | --- | --- | -| `GET` | `/self/settings` | `authenticateUser` only | — | `200` `{ settings: UserSettings \| null, updatedAt }` — `settings: null` when the user has no row yet (mirrors editor-state's no-state-yet response; **not** a 404) | -| `PUT` | `/self/settings` | `authenticateUser` only | `{ settings: UserSettings }` | `200` saved state; `400` on schema violation | +| Method | Path | Auth | Body | Response | +| ------ | ---------------- | ----------------------- | ---------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `GET` | `/self/settings` | `authenticateUser` only | — | `200` `{ settings: UserSettings \| null, updatedAt }` — `settings: null` when the user has no row yet (mirrors editor-state's no-state-yet response; **not** a 404) | +| `PUT` | `/self/settings` | `authenticateUser` only | `{ settings: UserSettings }` | `200` saved state (`{ settings, updatedAt }`); `422` on schema violation (a well-formed request whose body fails validation) | - **No permission middleware beyond `authenticateUser`.** Every caller owns exactly their own settings row, and the handler scopes all reads/writes to `currentUser.id` from context. (Editor-state additionally guards with `CONTENT_UPDATE` + chapter-assignment participation because it is tied to a chapter resource; `self/settings` has no resource to scope, so session identity alone is the right guard — and the `self` prefix makes that explicit.) - **PUT is full-replace.** The client GETs, merges in memory, PUTs the whole blob — the editor-state write pattern. Concurrent tabs are last-writer-wins, identical to editor-state's accepted behavior; for an ignore-list this worst case is "one ignore from another tab is lost until re-clicked." @@ -398,7 +395,7 @@ fluent-api/src/domains/self/settings/ └── self-settings.types.ts # response schema; re-exports UserSettings from db schema ``` -Plus: the `user_settings` table + `userSettingsSchema` in `src/db/schema.ts` (§7.2), one numbered migration in `src/db/migrations/`, and route registration in `src/app.ts`. (The table is still named `user_settings` — it stores per-user rows; only the *route* surface is `self`. The repository upserts on the `userId` PK using the session user's id.) +Plus: the `user_settings` table + `userSettingsSchema` in `src/db/schema.ts` (§7.2), one numbered migration in `src/db/migrations/`, and route registration in `src/app.ts`. (The table is still named `user_settings` — it stores per-user rows; only the _route_ surface is `self`. The repository upserts on the `userId` PK using the session user's id.) ### 8.3 What fluent-api explicitly does **not** do @@ -426,14 +423,14 @@ When the repeated-words query errors (network, 502 `AI_SERVICE_UNAVAILABLE` / `A - A single inline line renders at the top of the Checks panel: `Checks failed to refresh` in `text-sm text-red-500` — the drafting page's own status idiom (it already shows "Auto-save failed" inline in the header). **No toast, banner, or popup**; none exist in this codebase and a transient check hiccup doesn't warrant introducing one. - TanStack Query retains the last successful `data` on refetch failure, so the panel **keeps rendering the last-known findings** below the error line, and the dot reflects that last-known state. The failure mode is "results are one save behind," not "results vanish." - The error is logged via the existing `Logger` for diagnosis; the next successful auto-save naturally retries (new `saveCounter` key). -- A failure on the *initial* load (no previous data) renders the error line over an empty section — not the "No issues found" zero state, which must never appear on error. +- A failure on the _initial_ load (no previous data) renders the error line over an empty section — not the "No issues found" zero state, which must never appear on error. ### 9.3 Suppression-write failure Ignore actions apply **optimistically** (the finding greys immediately) and roll back on write failure: - Occurrence rules ride the debounced editor-state save; a failed save leaves the in-memory rule intact and retries with the next editor-state write (the page already tolerates editor-state save failures this way). -- Global rules PUT immediately; on failure the optimistic update is reverted (the finding returns to active) and the action can be re-clicked. No queued retry for v1 — the user sees the flag come back, which *is* the failure notification. +- Global rules PUT immediately; on failure the optimistic update is reverted (the finding returns to active) and the action can be re-clicked. No queued retry for v1 — the user sees the flag come back, which _is_ the failure notification. ### 9.4 Rollout / landing order (W12) @@ -451,21 +448,21 @@ Ignore actions apply **optimistically** (the finding greys immediately) and roll The repo now ships test infrastructure under `src/test/` (`msw/server.ts`, `render.tsx`, `setup.ts`); all UI tests use it — MSW intercepts at the network boundary, so the hooks under test exercise their real fetch paths. -| Area | Representative cases | -| --- | --- | -| `useResolvedFindings` (pure) | Cascade precedence table: machine-legitimate vs. global vs. occurrence, `suppress`/`surface`/silent at each layer, most-specific-wins; ordinal assignment incl. "the the the" overlap; NFC composed/decomposed key equivalence. | -| `useRepeatedWordsCheck` | Fires on `saveCounter` change; request body shape (snake_case verbatim, `snt_id` format, all drafted verses); `enabled` gating (readOnly, empty chapter); previous data retained on 502 (MSW error handler). | -| `useSuppressions` | Feature-detect: 404 ⇒ `globalIgnoresAvailable=false`; 200-with-null ⇒ available; occurrence rule round-trip through editor-state blob; global write purges current-chapter occurrence rules for the pair; optimistic rollback on PUT failure. | -| `ChecksPanel` / `FindingRow` | Verse grouping + separators; zero state (and *not* on error); inline error line; active `[Ignore Here]`/`[Ignore Everywhere]` vs. greyed-with-label + `[Undo ▾]`; "Show Ignored" toggle defaults off and is not persisted; `[Ignore Everywhere]` opens confirm dialog and only writes on confirm; `[Ignore Everywhere]` absent when capability unavailable. | -| `LeftPanel` / dot | Dot iff cascade-active > 0, visible from Resources tab; tab switch persistence key written; toggle-button mirror dot when panel closed. | +| Area | Representative cases | +| ---------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `useResolvedFindings` (pure) | Cascade precedence table: machine-legitimate vs. global vs. occurrence, `suppress`/`surface`/silent at each layer, most-specific-wins; ordinal assignment incl. "the the the" overlap; NFC composed/decomposed key equivalence. | +| `useRepeatedWordsCheck` | Fires on `saveCounter` change; request body shape (snake_case verbatim, `snt_id` format, all drafted verses); `enabled` gating (readOnly, empty chapter); previous data retained on 502 (MSW error handler). | +| `useSuppressions` | Feature-detect: 404 ⇒ `globalIgnoresAvailable=false`; 200-with-null ⇒ available; occurrence rule round-trip through editor-state blob; global write purges current-chapter occurrence rules for the pair; optimistic rollback on PUT failure. | +| `ChecksPanel` / `FindingRow` | Verse grouping + separators; zero state (and _not_ on error); inline error line; active `[Ignore Here]`/`[Ignore Everywhere]` vs. greyed-with-label + `[Undo ▾]`; "Show Ignored" toggle defaults off and is not persisted; `[Ignore Everywhere]` opens confirm dialog and only writes on confirm; `[Ignore Everywhere]` absent when capability unavailable. | +| `LeftPanel` / dot | Dot iff cascade-active > 0, visible from Resources tab; tab switch persistence key written; toggle-button mirror dot when panel closed. | ### 10.2 fluent-api (Vitest, route-level — same style as the ai-tools and editor-state tests) -| Area | Representative cases | -| --- | --- | -| `GET /self/settings` | 401 unauthenticated; `settings: null` for new user (200, not 404); returns saved blob; user isolation (only own row, scoped to session user). | -| `PUT /self/settings` | Upsert create + replace; 400 on schema violation; full-replace semantics (omitted keys gone); `.catch({})` tolerance on read of unknown-shaped stored blobs. | -| Editor-state schema extension | Existing editor-state tests still pass unchanged (proves backward compatibility); round-trip of `checkOccurrenceRules` / `activeLeftTab`. | +| Area | Representative cases | +| ----------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `GET /self/settings` | 401 unauthenticated; `settings: null` for new user (200, not 404); returns saved blob; user isolation (only own row, scoped to session user). | +| `PUT /self/settings` | Upsert create + replace; 400 on schema violation; full-replace semantics (omitted keys gone); `.catch({})` tolerance on read of unknown-shaped stored blobs. | +| Editor-state schema extension | Existing editor-state tests still pass unchanged (proves backward compatibility); round-trip of `checkOccurrenceRules` / `activeLeftTab`. | Manual verification continues to use the stack via `fluent-platform` compose plus the existing repeated-words smoke script for the proxy half. @@ -486,16 +483,16 @@ Manual verification continues to use the stack via `fluent-platform` compose plu Items needing explicit confirmation in review (referenced from §5.2/§5.3); none block reading the rest of the design. Status reflects the first review round (§0): -| # | Item | Proposed resolution | Status | -| --- | --- | --- | --- | -| S1 | Zero-state mock showed the notification dot; #277 text says no flags ⇒ no dot | Follow the text: no dot at zero active flags | ✅ Resolved — chadw fixed the mock | -| S2 | #278 mocks showed a language dropdown above checks content | Omit it on the Checks tab (no function for checks) | ✅ Resolved — chadw removed it from the mocks | -| S3 | Card originally said ignored items are "removed from the panel" | Reveal them dimmed under a "Show Ignored" toggle (default off, not persisted) with "Undo Ignore"; dot unaffected | ✅ Largely card-aligned — revised #278 added the toggle; confirm the labels match | -| S4 | Cards did not enumerate Greek-Room-`legitimate` findings | Show as ignored items labeled **"Default Ignore"** (the revised #278 mock's term), per-occurrence surfaceable | ☑ Confirm "Default Ignore" = machine-legitimate | -| S5 | Dot is invisible when the whole left panel is closed | Mirror the dot on the panel-toggle button while closed | ☐ Open | -| S6 | Suppression persistence has no card of its own | Ship as extension of #172's scope (W1) — backend dependency of the UI cards | ☐ Open | -| S7 | Revised #278 confirm dialog says "Ignore Everywhere" "cannot be undone" | Add the confirm dialog **and** keep `[Undo ▾]` reversibility (§6.5); the two are complementary | ☐ Confirm we may retain undo | -| S8 | Highlight repeated words in the verse text (joelthe1/Ulf) | We will implement it as a **follow-on PR** layered on this one (verse-pane scope; §5.3 item 4) | ☐ Confirm follow-on-PR sequencing | +| # | Item | Proposed resolution | Status | +| --- | ----------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------- | +| S1 | Zero-state mock showed the notification dot; #277 text says no flags ⇒ no dot | Follow the text: no dot at zero active flags | ✅ Resolved — chadw fixed the mock | +| S2 | #278 mocks showed a language dropdown above checks content | Omit it on the Checks tab (no function for checks) | ✅ Resolved — chadw removed it from the mocks | +| S3 | Card originally said ignored items are "removed from the panel" | Reveal them dimmed under a "Show Ignored" toggle (default off, not persisted) with "Undo Ignore"; dot unaffected | ✅ Largely card-aligned — revised #278 added the toggle; confirm the labels match | +| S4 | Cards did not enumerate Greek-Room-`legitimate` findings | Show as ignored items labeled **"Default Ignore"** (the revised #278 mock's term), per-occurrence surfaceable | ☑ Confirm "Default Ignore" = machine-legitimate | +| S5 | Dot is invisible when the whole left panel is closed | Mirror the dot on the panel-toggle button while closed | ☐ Open | +| S6 | Suppression persistence has no card of its own | Ship as extension of #172's scope (W1) — backend dependency of the UI cards | ☐ Open | +| S7 | Revised #278 confirm dialog says "Ignore Everywhere" "cannot be undone" | Add the confirm dialog **and** keep `[Undo ▾]` reversibility (§6.5); the two are complementary | ☐ Confirm we may retain undo | +| S8 | Highlight repeated words in the verse text (joelthe1/Ulf) | We will implement it as a **follow-on PR** layered on this one (verse-pane scope; §5.3 item 4) | ☐ Confirm follow-on-PR sequencing | Engineering-side confirmations sought from reviewers (same spirit as the D-series confirmations in the fluent-api proposal): @@ -505,4 +502,4 @@ Engineering-side confirmations sought from reviewers (same spirit as the D-serie --- -*Prepared against fluent-web `main` (post `2e936d1`), fluent-api `main` (post `7c7f63d`), and the approved fluent-api proposal (PR #173). Revised 2026-06-16 after the first review round on PR #305. Author: JEdward7777.* +_Prepared against fluent-web `main` (post `2e936d1`), fluent-api `main` (post `7c7f63d`), and the approved fluent-api proposal (PR #173). Revised 2026-06-16 after the first review round on PR #305. Author: JEdward7777._ diff --git a/docs/proposals/repeated-word-check/checks-ui-integration-summary.md b/docs/proposals/repeated-word-check/checks-ui-integration-summary.md index 12af7694..e51a1fe1 100644 --- a/docs/proposals/repeated-word-check/checks-ui-integration-summary.md +++ b/docs/proposals/repeated-word-check/checks-ui-integration-summary.md @@ -18,7 +18,7 @@ Give translators the user-facing half of the Repeated Word Check: a **Checks tab 4. **Chapter-wide check on every successful verse auto-save** (W3/W4): a TanStack `useQuery` keyed on `(chapterAssignmentId, saveCounter)` sending all drafted verses; `snt_id` = `"{bookCode} {chapter}:{verse}"` (USFM, the smoke-test convention); no extra debounce beyond the existing 2 s save debounce. Drafting mode only (W10). -5. **Three-layer active/inactive cascade, most-specific-non-silent-wins** (W5/W6): Greek Room's `legitimate` verdict → user-global word-pair rule → occurrence rule `(snt_id, repeated_word, ordinal)`; rules are tri-state maps (`absent/'suppress'/'surface'`), which is what makes per-occurrence **undo** of a global rule or machine verdict possible. The dot counts active findings only; ignored items are hidden by default and revealed dimmed (with "Default Ignore"/"Ignore Here"/"Ignore Always" reason labels and "Undo Ignore") under a **"Show Ignored" toggle** (default off, not persisted), matching the revised #278. +5. **Three-layer active/inactive cascade, most-specific-non-silent-wins** (W5/W6): Greek Room's `legitimate` verdict → user-global word-pair rule → occurrence rule `(snt_id, repeated_word, ordinal)`; rules are tri-state maps (`absent/'suppress'/'surface'`), which is what makes per-occurrence **undo** of a global rule or machine verdict possible. The dot counts active findings only; ignored items are hidden by default and revealed dimmed (with "Default Ignore"/"Ignore Here"/"Ignore Everywhere" reason labels and "Undo Ignore") under a **"Show Ignored" toggle** (default off, not persisted), matching the revised #278. 6. **Graceful degradation, either-PR-lands-first** (W8/W9): the UI feature-detects `GET /self/settings` (404 ⇒ "Ignore Everywhere" simply not rendered — capability hidden, never a dead control); check failures surface as one inline red line in the panel while the last successful findings stay rendered (TanStack default). No toasts/banners — none exist in this codebase. **"Ignore Everywhere" shows a confirm dialog** (per revised #278) while still being reversible via undo. diff --git a/docs/proposals/repeated-word-check/highlight-in-verse-suggestion.md b/docs/proposals/repeated-word-check/highlight-in-verse-suggestion.md new file mode 100644 index 00000000..560e4513 --- /dev/null +++ b/docs/proposals/repeated-word-check/highlight-in-verse-suggestion.md @@ -0,0 +1,304 @@ +# Highlight Repeated Words in Verse Text — Design Options & Recommendation + +**Status:** Proposal / decision-requested. Docs-only. No implementation yet. +**Type:** Follow-on to the Repeated Word Check UI (cards +[fluent-web#277](https://github.com/eten-tech-foundation/fluent-web/issues/277) / +[fluent-web#278](https://github.com/eten-tech-foundation/fluent-web/issues/278)), which +shipped the **Checks tab + panel** +([PR #320](https://github.com/eten-tech-foundation/fluent-web/pull/320), +[fluent-api #203](https://github.com/eten-tech-foundation/fluent-api/pull/203)). +**Relates to #277, #278.** The tab/panel design this layers on is +[`checks-ui-integration-suggestion.md`](checks-ui-integration-suggestion.md) (decisions +**W1–W12**). + +> **Why this is a proposal PR and not just an implementation PR.** The highlight-in-verse work +> was agreed to in the #305 review, and it _looked_ like a small presentational add-on. On +> close inspection of the actual drafting editor it is **not** small: it forces a real decision +> about how editable verse text is rendered. That decision is bigger than one feature and will +> outlive it, so the team should weigh in **before** implementation rather than have an +> architecture picked unilaterally on an experimental branch. + +--- + +## 0. The original request (what Joel/Ulf asked for) + +During the Repeated Word Check proposal review (PR #305), **joelthe1 (Joel) and Ulf** asked +that, in addition to _listing_ repeated-word findings in the Checks panel, the repeated words +be **visually marked in the verse text itself** — an underline or color on the offending words +where they appear, so the translator sees the problem in place, not only in a side list. + +It was agreed to, and **deliberately carved out as a separate follow-on PR** so the tab/panel +(#277/#278) could land first without being blocked on verse-rendering changes. This document is +that follow-on, brought forward for a design decision. + +The concrete visual target discussed on the product side of this thread: **the only visible +change should be that the flagged (active) repeated words turn red** — no new chrome, no layout +shift, the editing experience otherwise identical. + +--- + +## 1. TL;DR / recommendation + +- Coloring words _inside the box the translator is typing into_ is the hard part. Today that + box is a **plain `