[Frontend] Merge "draft" and "proposal" into one review vocabulary and flow #385

Closed
opened 2026-08-25 20:42:22 +00:00 by claude-bot · 2 comments
Contributor

Impact: MEDIUM

Found in the August 2026 session lifecycle review (#319).

What the user experiences

Approving a lore proposal doesn't approve it. With proposed_for_entry_id set, clicking "Approve" instead calls startLoreDraft and navigates to a completely different review page — the draft review — whose only warning beforehand is a small blue "Update" chip reading "Will be appended to <Title>". That page then promises an append but ships a button reading "Approve and replace article". The same draft page labels every non-merge, non-convert draft "LLM rephrase of" — including a GM-requested "Generate backstory" draft, which reads as a rephrase of something that didn't exist yet. Meanwhile approving a proposal without proposed_for_entry_id runs and the row just vanishes — no toast, no "View article" link, no navigation; the user's only signal that it worked is that a row disappeared. None of the four proposal actions has a busy state, so double-clicking "Approve" fires two requests, and all four report failure via a native alert() — the only place in the product that uses one, inconsistent with the inline-error pattern used everywhere else.

Evidence

  • webapp/frontend/src/pages/WikiProposals.jsx:209-231 — "Approve" branches into startLoreDraft + navigate (with proposed_for_entry_id) or approveLoreEntry (without), two unrelated behaviours behind one button.
  • webapp/frontend/src/pages/WikiProposals.jsx:362-366,371-379 — the only pre-navigation warning is a small "Update" chip and "Will be appended to <Title>".
  • webapp/frontend/src/pages/WikiDraftReview.jsx:484-485,810 — "approve it to replace the article" / button text "Approve and replace article", contradicting the "append" copy on the page that led here.
  • webapp/frontend/src/pages/WikiDraftReview.jsx:471 — every non-merge, non-convert draft is labelled "LLM rephrase of", including expand_backstory and generate_statblock.
  • webapp/frontend/src/pages/WikiArticle.jsx:1196-1197 vs WikiDraftReview.jsx:484-485 — one page promises an append, the other promises a replace, for the same draft.
  • webapp/frontend/src/pages/WikiProposals.jsx:222-227approveLoreEntry on the non-draft path: the card vanishes with no toast, no "View article" link, no navigation.
  • webapp/frontend/src/pages/WikiProposals.jsx:383-385,417-418 — no busy/disabled state on any of the four proposal actions.
  • webapp/frontend/src/pages/WikiProposals.jsx:229,239,248,258 — all four failure paths use alert(err.message), the only native modal in the product.

Why it matters for a hosted product

Two backend objects doing one user job ("review AI-written lore before it lands") forces every GM to learn two review pages, two vocabularies, and self-contradicting copy about whether their edit will append or replace — and the one path with no navigation confirmation leaves the GM guessing whether their approval actually did anything.

Proposed fix

Split the overloaded "Approve" button into a clearly-labelled "Approve" (for the direct-approve path) and "Review merge…" (for the draft path), so the destination is signalled before the click, not after. Add busy/disabled states to all four proposal actions. Replace the four alert() calls with the inline-error pattern used elsewhere. Give the direct-approve path a toast with a "View article" link instead of a silent vanish. Reconcile the append-vs-replace copy across WikiArticle.jsx:1196-1197 and WikiDraftReview.jsx:484-485,810 to say the same thing, and fix the "LLM rephrase of" label to reflect the actual draft type. This is the audit's C3 (detailed in §9.2-9.3) together with P25.

Acceptance criteria

  • The proposal-review UI signals before the click whether "Approve" will directly approve or open a merge review.
  • All four proposal actions (approve, discard, edit-and-approve, etc.) show a busy state and are disabled while in flight.
  • Failure on any proposal action shows an inline error, not a native alert().
  • Direct approval shows a confirmation with a link to the resulting article, not a silent row removal.
  • The append-vs-replace copy is consistent between the article page and the draft-review page for the same draft type.
  • The draft-type label reflects what actually happened (e.g. "Generated backstory" instead of "LLM rephrase of" for a backstory-generation draft).
**Impact: MEDIUM** Found in the August 2026 session lifecycle review (#319). ## What the user experiences Approving a lore proposal doesn't approve it. With `proposed_for_entry_id` set, clicking "Approve" instead calls `startLoreDraft` and navigates to a completely different review page — the draft review — whose only warning beforehand is a small blue "Update" chip reading "Will be appended to \<Title\>". That page then promises an append but ships a button reading "Approve and replace article". The same draft page labels every non-merge, non-convert draft "LLM rephrase of" — including a GM-requested "Generate backstory" draft, which reads as a rephrase of something that didn't exist yet. Meanwhile approving a proposal *without* `proposed_for_entry_id` runs and the row just vanishes — no toast, no "View article" link, no navigation; the user's only signal that it worked is that a row disappeared. None of the four proposal actions has a busy state, so double-clicking "Approve" fires two requests, and all four report failure via a native `alert()` — the only place in the product that uses one, inconsistent with the inline-error pattern used everywhere else. ## Evidence - `webapp/frontend/src/pages/WikiProposals.jsx:209-231` — "Approve" branches into `startLoreDraft` + navigate (with `proposed_for_entry_id`) or `approveLoreEntry` (without), two unrelated behaviours behind one button. - `webapp/frontend/src/pages/WikiProposals.jsx:362-366,371-379` — the only pre-navigation warning is a small "Update" chip and "Will be appended to \<Title\>". - `webapp/frontend/src/pages/WikiDraftReview.jsx:484-485,810` — "approve it to **replace** the article" / button text "Approve and replace article", contradicting the "append" copy on the page that led here. - `webapp/frontend/src/pages/WikiDraftReview.jsx:471` — every non-merge, non-convert draft is labelled "LLM rephrase of", including `expand_backstory` and `generate_statblock`. - `webapp/frontend/src/pages/WikiArticle.jsx:1196-1197` vs `WikiDraftReview.jsx:484-485` — one page promises an append, the other promises a replace, for the same draft. - `webapp/frontend/src/pages/WikiProposals.jsx:222-227` — `approveLoreEntry` on the non-draft path: the card vanishes with no toast, no "View article" link, no navigation. - `webapp/frontend/src/pages/WikiProposals.jsx:383-385,417-418` — no busy/disabled state on any of the four proposal actions. - `webapp/frontend/src/pages/WikiProposals.jsx:229,239,248,258` — all four failure paths use `alert(err.message)`, the only native modal in the product. ## Why it matters for a hosted product Two backend objects doing one user job ("review AI-written lore before it lands") forces every GM to learn two review pages, two vocabularies, and self-contradicting copy about whether their edit will append or replace — and the one path with no navigation confirmation leaves the GM guessing whether their approval actually did anything. ## Proposed fix Split the overloaded "Approve" button into a clearly-labelled "Approve" (for the direct-approve path) and "Review merge…" (for the draft path), so the destination is signalled before the click, not after. Add busy/disabled states to all four proposal actions. Replace the four `alert()` calls with the inline-error pattern used elsewhere. Give the direct-approve path a toast with a "View article" link instead of a silent vanish. Reconcile the append-vs-replace copy across `WikiArticle.jsx:1196-1197` and `WikiDraftReview.jsx:484-485,810` to say the same thing, and fix the "LLM rephrase of" label to reflect the actual draft type. This is the audit's C3 (detailed in §9.2-9.3) together with P25. ## Acceptance criteria - [ ] The proposal-review UI signals before the click whether "Approve" will directly approve or open a merge review. - [ ] All four proposal actions (approve, discard, edit-and-approve, etc.) show a busy state and are disabled while in flight. - [ ] Failure on any proposal action shows an inline error, not a native `alert()`. - [ ] Direct approval shows a confirmation with a link to the resulting article, not a silent row removal. - [ ] The append-vs-replace copy is consistent between the article page and the draft-review page for the same draft type. - [ ] The draft-type label reflects what actually happened (e.g. "Generated backstory" instead of "LLM rephrase of" for a backstory-generation draft).
Author
Contributor

Picking this up as v4.3.0 phase 6 (#514). "Approve" splits into "Approve" and "Review merge…" so the destination is signalled before the click; busy states on all four actions; inline errors replace the product's only alert(); direct approval confirms with a "View article" link; the append-versus-replace copy says the same true thing on both pages; the draft-type label reflects what the draft actually is.

Picking this up as v4.3.0 phase 6 (#514). "Approve" splits into "Approve" and "Review merge…" so the destination is signalled before the click; busy states on all four actions; inline errors replace the product's only `alert()`; direct approval confirms with a "View article" link; the append-versus-replace copy says the same true thing on both pages; the draft-type label reflects what the draft actually is.
Author
Contributor

Done in PR #527 (auto-merging on green); ships with v4.3.0.

The backend was read before the copy was touched, because the queue and the draft page disagreed and both were wrong. approve_draft always writes body = current_body; expand_lore_entry_backstory returns the existing body plus a new section (so "replace" is an append in effect); generate_statblock and convert_stats never touch the body at all; and merge is an LLM merge, never the verbatim append the queue promised.

So: Approve and Review merge… are now separate buttons, and the destination is named before the click instead of being signalled by a small "Update" chip (which stays, explained in the page intro). Draft kinds come from one map keyed on mode, with the source_proposal_id fallback the backend's generator also makes for pre-#130 drafts, so a generated backstory stops calling itself an LLM rephrase and each kind's approve button names its own effect ("Approve and add to article", "Approve stat block", "Approve merged article"). WikiArticle's start-draft modal was reworded to promise what the review page performs.

All four queue actions now disable while one is in flight (a double-click used to fire a duplicate request), report failure on the row that failed, and the last four alert() calls in the product are gone. A direct approve leaves a receipt with a View article link rather than just making the row disappear.

20 new tests. No backend change.

Done in PR #527 (auto-merging on green); ships with v4.3.0. The backend was read before the copy was touched, because the queue and the draft page disagreed and both were wrong. `approve_draft` always writes `body = current_body`; `expand_lore_entry_backstory` returns the existing body plus a new section (so "replace" is an append in effect); `generate_statblock` and `convert_stats` never touch the body at all; and `merge` is an LLM merge, never the verbatim append the queue promised. So: **Approve** and **Review merge…** are now separate buttons, and the destination is named before the click instead of being signalled by a small "Update" chip (which stays, explained in the page intro). Draft kinds come from one map keyed on `mode`, with the `source_proposal_id` fallback the backend's generator also makes for pre-#130 drafts, so a generated backstory stops calling itself an LLM rephrase and each kind's approve button names its own effect ("Approve and add to article", "Approve stat block", "Approve merged article"). `WikiArticle`'s start-draft modal was reworded to promise what the review page performs. All four queue actions now disable while one is in flight (a double-click used to fire a duplicate request), report failure on the row that failed, and the last four `alert()` calls in the product are gone. A direct approve leaves a receipt with a **View article** link rather than just making the row disappear. 20 new tests. No backend change.
Sign in to join this conversation.
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
rbrooks/Quest-Board#385
No description provided.