feat(ux): compose-confirm form + Manage rules menu + dashboard onboarding (Phase 1.7b)#44
Merged
Merged
Conversation
…ng (Phase 1.7b Tier 1+2+3) Best-practice UX rework for the 5 deferred audit findings (#2 #3 #5 #7 #10) plus 4 new items (#A empty states, #B delete confirm, #C onboarding, #D token cost). Design + tier rationale: claudedocs/2026-05-14-ux-best-practices-plan.md. ## Tier 1 — compose flow (audit #2 #5 #7) ### #2 Compile preview confirmation form (composeConfirmForm) Compile no longer persists straight to draft. After validation, the moderator sees a confirm form with the rule rendered in plain English (humanizeRule()), the original sentence, and the per-compile token cost (~$0.0006 on gpt-5.4-mini). Tick "Edit instead of save" to re-open compose with the original NL pre-filled. Save persists draft + schedules dry-run via the new persistRuleAndStartDryRun() helper. Defence-in-depth: serialized rule is re-validated before write. New endpoint: /internal/form/compose-confirm-submit New form: composeConfirmForm ### #5 Clarification turn limit (3 rounds) Server-side counter shipped as a disabled paragraph field. After round 3, an oscillating LLM gets refused with an actionable toast ("Try rephrasing more concretely"), instead of opening yet another modal. Each clarify modal description prefixed with "(Round X of 3)" so the moderator sees how many tries remain. ### #7 Editable original rule in clarify modal Removed `disabled: true` on the Original rule field. helpText now says "Re-compile uses this text plus your answer below." ## Tier 2 — Manage rules menu + dashboard hardening (audit #3 #10 #A #B #D) ### #3 + #10 Per-rule control surface (Manage rules) New "vibe-mod: Manage rules" menu (subreddit-level, mod-only). Renders one form-group per rule with a `select` of available actions: - Drafts: Keep / Activate (shadow 24h) / Activate immediately / Delete - Active shadow rules: Keep / Promote shadow → live / Pause / Delete - Active live rules: Keep / Pause / Delete Submit applies all non-destructive actions atomically (one redis.set per bundle) via the new applyManageActions() helper. Dashboard becomes read-only — its old "Activate N drafts" boolean removed. New endpoints: /internal/menu/manage-rules, /internal/form/manage-rules-submit, /internal/form/manage-delete-confirm New forms: manageRulesForm, manageDeleteConfirmForm ### #B Delete confirmation Any `delete` action in the Manage submit forwards to a confirm form listing the rules about to disappear, with an explicit "I understand this is permanent" boolean. The pending action map round-trips through a disabled paragraph carrier so the second-step submit knows what to apply. ### #A Empty states - Manage menu emits a guided toast when there are zero rules at all. - Dashboard description shows "No rules yet — open ⋯ → Compose" when both rules count and recent actions are zero. ### #D Token cost transparency - Dashboard description: "Tokens used (lifetime): N in / N out (~$X on gpt-5.4-mini)" via the new estimateTokenCost() helper. - Compose confirm form shows per-compile cost so the moderator sees the unit economics before saving. ## Tier 3 — Onboarding (audit #C) Dashboard shows a 3-step welcome card on first visit (per-sub Redis flag `${sub}:onboarding:dismissed`). Cancel-label switches to "Don't show intro again" and ticks dismissOnboarding to persist the flag. New key in src/shared/redis-keys.ts. ## Verification - `npm run check` 4/4 gates green - 209 unit + integration tests (1 skipped): - 33 compose tests (8 updated for new 2-step flow + 4 new for #2/#5/#7) - 13 dashboard tests (4 updated for read-only + 4 new for onboarding/empty) - 17 manage tests (NEW file routes-manage.test.ts: menu render, all 5 actions, delete confirm round-trip) - existing trigger / scheduler / settings / undo / evaluator / executor unchanged ## Why a single PR Tier 1+2+3 all touch overlapping files (index.ts, devvit.json, the dashboard form shape). Splitting would force back-and-forth conflict resolution. Logical commits inside this PR (split when applicable in follow-up). Refs: claudedocs/2026-05-14-ux-best-practices-plan.md (Phase 1.7a), claudedocs/2026-05-14-compose-flow-audit.md (audit baseline), docs/demo-scenario.md (downstream dependency)
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThis PR implements Phase 1.7b of vibe-mod, introducing a two-step rule-composition flow with confirmation, clarification turn limiting, dashboard onboarding with token-cost visibility, and a new Manage Rules moderator workflow. The server enforces moderator authorization, persists rule state atomically, and adds defensive configuration reading. All existing tests remain compatible via re-export invariants. ChangesPhase 1.7b Feature Implementation
Sequence DiagramsequenceDiagram
participant User
participant ComposeFlow
participant ConfirmForm
participant Persistence
User->>ComposeFlow: Submit rule text
ComposeFlow->>ComposeFlow: Compile rule (with turn limit check)
alt Needs Clarification
ComposeFlow->>User: Show clarification modal (Round X of 3)
User->>ComposeFlow: Answer clarification
ComposeFlow->>ComposeFlow: Re-compile with answer
end
ComposeFlow->>ConfirmForm: Return confirmation form
ConfirmForm->>User: Show deterministic summary + token cost
User->>ConfirmForm: "Edit" or "Save"
alt Edit Path
ConfirmForm->>ComposeFlow: Reopen compose with original rule
else Save Path
ConfirmForm->>Persistence: Revalidate and persist draft
Persistence->>Persistence: Schedule dry-run
Persistence->>User: Success toast
end
🎯 3 (Moderate) | ⏱️ ~25 minutesPossibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a series of UX best practices and structural improvements for the vibe-mod server. Key changes include splitting the rule composition into a two-step confirmation flow, introducing a turn limit for LLM clarifications to prevent infinite loops, and creating a dedicated 'Manage rules' interface for per-rule actions (activate, pause, promote, delete). The dashboard has been updated to be read-only with a new onboarding card and token cost transparency. Feedback highlights several critical issues: the rule management logic lacks atomicity and is vulnerable to race conditions during concurrent edits, the 50-rule limit is not enforced when moving rules between bundles, and using 'manage' as a model fallback breaks token cost estimations. Additionally, performance could be improved by parallelizing dry-run result fetches.
Comment on lines
+956
to
+973
| for (const r of drafts) { | ||
| try { | ||
| const rawDry = await redis.get(keys.dryrun(subredditName, r.id)); | ||
| if (!rawDry) continue; | ||
| const d = JSON.parse(rawDry) as DryRunResult; | ||
| if (d.status === 'ok') { | ||
| dryRunByRuleId.set( | ||
| r.id, | ||
| `Dry-run: would match ${d.matched.length}/${d.sampledPosts} recent post(s)` + | ||
| (d.matched.length ? ` → ${[...new Set(d.matched.flatMap((m) => m.would))].join(', ')}` : ''), | ||
| ); | ||
| } else { | ||
| dryRunByRuleId.set(r.id, `Dry-run: ${d.note ?? 'unavailable'}`); | ||
| } | ||
| } catch (err) { | ||
| console.warn(`[vibe-mod] manage: redis.get(dryrun/${r.id}) threw:`, describeErr(err)); | ||
| } | ||
| } |
There was a problem hiding this comment.
Fetching dry-run results sequentially in a loop for up to 50 rules can significantly slow down the menu rendering, especially in a serverless environment where Redis latency adds up. Parallelizing these requests would improve performance.
References
- Avoid unnecessary loops or sequential iterations for async operations when they can be performed in parallel.
| // keep | activate-shadow | activate-now | promote | pause | delete | ||
| // Plain reads + 1 write per bundle. All plugin RPC wrapped because | ||
| // reddit/devvit#258 still rears its head occasionally. | ||
| async function applyManageActions(actions: Record<string, string>): Promise<{ persisted: boolean; summary: string }> { |
There was a problem hiding this comment.
The 50-rule limit is not enforced in this function. Actions like 'pause' (moving rules from active to draft) or 'activate' (moving from draft to active) could cause a bundle to exceed the cap if many rules are moved at once. This could lead to performance degradation or memory issues during rule evaluation.
Comment on lines
+2010
to
+2019
| llmModel: draft?.llmModel ?? 'manage', | ||
| llmTokensIn: draft?.llmTokensIn ?? 0, | ||
| llmTokensOut: draft?.llmTokensOut ?? 0, | ||
| rules: draftRules, | ||
| }; | ||
| const activeBundle: RuleBundleType = { | ||
| schemaVersion: '1.0.0', | ||
| bundleVersion: (active?.bundleVersion ?? 0) + 1, | ||
| compiledAt: now, | ||
| llmModel: active?.llmModel ?? 'manage', |
There was a problem hiding this comment.
Using 'manage' as a fallback for llmModel will cause token cost estimations to fail (returning 0) because 'manage' is not a key in the OPENAI_PRICING_USD_PER_TOKEN map. It is better to use the currently configured model name as a default.
| try { | ||
| writes.push(redis.set(keys.rulesDraft(subredditName), JSON.stringify(draftBundle))); | ||
| writes.push(redis.set(keys.rulesActive(subredditName), JSON.stringify(activeBundle))); | ||
| await Promise.all(writes); |
There was a problem hiding this comment.
This update is not atomic, which contradicts the PR description's claim. Promise.all triggers two independent redis.set operations. If one succeeds and the other fails, rules could be duplicated or lost between the draft and active bundles. Furthermore, the read-modify-write pattern used here is vulnerable to race conditions if multiple moderators are managing rules at the same time.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server/index.ts (1)
1802-1802:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win정규식이 의도한 동작을 수행하지 않음
정규식
/[-�]/g는 실제로는 hyphen (-)과 문자�(U+FFFF) 두 개만 매칭합니다. 코드 주석의 의도인 "non-ASCII char (>= 0x80)"를 이스케이프하지 않으므로 OpenAI 요청 본문에서 대부분의 비ASCII 문자가 그대로 전달되어 UTF-8 인코딩 문제가 발생합니다.다음 정규식 중 하나로 수정하세요:
- const asciiSafeBody = rawBody.replace(/[-�]/g, (c) => '\\u' + c.charCodeAt(0).toString(16).padStart(4, '0')); + const asciiSafeBody = rawBody.replace(/[\u0080-\uffff]/g, (c) => '\\u' + c.charCodeAt(0).toString(16).padStart(4, '0'));또는:
- const asciiSafeBody = rawBody.replace(/[-�]/g, (c) => '\\u' + c.charCodeAt(0).toString(16).padStart(4, '0')); + const asciiSafeBody = rawBody.replace(/[^\x00-\x7F]/g, (c) => '\\u' + c.charCodeAt(0).toString(16).padStart(4, '0'));🤖 Prompt for 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. In `@src/server/index.ts` at line 1802, The regex used to produce asciiSafeBody is wrong (/[�-�]/g matches only '-' and U+FFFF); update the pattern used on rawBody in the asciiSafeBody assignment to correctly match non-ASCII characters, e.g. replace /[�-�]/g with /[\u0080-\uFFFF]/g or /[^\x00-\x7F]/g so the replacer (c => '\\u' + c.charCodeAt(0).toString(16).padStart(4,'0')) is applied to all characters >= 0x80.
🧹 Nitpick comments (3)
src/server/index.ts (3)
2146-2194: 💤 Low valuehumanizeRule 출력 길이
함수가 전체 PredicateTree를 재귀적으로 렌더링합니다. 매우 복잡한 규칙(깊게 중첩된 all/any)의 경우 출력이 수천 자가 될 수 있어 확인 폼 description을 읽기 어렵게 만들 수 있습니다. 그러나
checkTreeDepth가 이미 깊이를 제한하고 있으므로 실제로는 합리적인 범위 내에 있어야 합니다.복잡한 규칙에 대해 출력을 약 500자로 제한하고 "... (조건이 더 있음)"을 추가하는 것을 고려해보세요. 그러나 현재 구현은 대부분의 실제 사용 사례에서 작동해야 합니다.
🤖 Prompt for 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. In `@src/server/index.ts` around lines 2146 - 2194, humanizeRule currently renders the entire PredicateTree via predicateLabel which can produce extremely long strings for deeply nested/complex rules; cap the output (e.g. ~500 chars) and append a deterministic truncation marker like "... (more conditions)" so the compose-confirm description stays readable. Modify humanizeRule/predicateLabel to track a maxLength constant and either (a) stop recursing once the cumulative output length would exceed that limit and return a truncated fragment with the marker, or (b) build the full string then slice to maxLength and append the marker—ensure the marker is always added deterministically (same input => same output) and that triggers/actions/ratelimit formatting (variables triggers, actions, rateLimit) keep their existing structure. Use the function names predicateLabel and humanizeRule as the insertion points and preserve indentation/line breaks when truncating.
1933-2044: ⚖️ Poor tradeoffapplyManageActions 원자성 고려사항
함수가 두 개의 개별
redis.set()호출을 사용하여 draft와 active 번들을 업데이트합니다(lines 2025-2026). 하나는 성공하고 다른 하나는 실패하면 불일치 상태가 발생할 수 있습니다. Devvit Redis는 트랜잭션을 지원하지 않는 것으로 보이지만, 실패 시 두 쓰기 모두 재시도하거나 보상 롤백을 고려해야 합니다.현재 구현은 두 쓰기 중 하나라도 실패하면
persisted: false를 반환하고 적절한 요약 메시지를 제공합니다. 사용자는 재시도할 수 있습니다. 이는 reddit/devvit#258 불안정성을 고려할 때 합리적인 트레이드오프입니다.운영 관찰: 부분 쓰기로 인한 불일치가 발생하면 moderator는 Manage rules 메뉴를 다시 열어 현재 상태를 확인하고 의도한 작업을 재적용할 수 있습니다. 미래 개선사항으로는 재시도 로직이나 보상 작업을 추가할 수 있습니다.
🤖 Prompt for 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. In `@src/server/index.ts` around lines 1933 - 2044, The applyManageActions function can leave draft and active bundles inconsistent because it performs two separate redis.set calls (keys.rulesDraft and keys.rulesActive) without atomicity; modify applyManageActions to ensure both writes succeed or are rolled back by first reading and storing the previous values (already read into draft/active), then attempt the two writes with a small retry loop (e.g., 3 attempts with backoff) around redis.set for each key, and if after retries one write succeeds and the other ultimately fails, perform a compensating redis.set to restore the prior value for the successful key and set persisted = false so callers know the operation did not fully persist; keep the summary construction and error logging (processLogger/console.warn) but ensure you reference and update keys.rulesDraft and keys.rulesActive and the persisted flag accordingly.
1906-1910: ⚡ Quick winestimateTokenCost 구현 검토
함수가 알 수 없는 모델에 대해 0을 반환하여 크래시를 방지하는 것은 좋습니다. 그러나 음수 토큰 값에 대한 방어 로직은 없습니다. 입력이 서버 제어 disabled 필드에서 오기 때문에 실제 위험은 낮지만, 방어적 검증을 추가하면 더 견고해집니다.
♻️ 음수 토큰 검증 추가
function estimateTokenCost(model: string, tokensIn: number, tokensOut: number): number { const p = OPENAI_PRICING_USD_PER_TOKEN[model]; if (!p) return 0; + // Defensive: ensure non-negative tokens (should always be true from server-controlled fields) + const safeIn = Math.max(0, tokensIn); + const safeOut = Math.max(0, tokensOut); - return tokensIn * p.in + tokensOut * p.out; + return safeIn * p.in + safeOut * p.out; }🤖 Prompt for 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. In `@src/server/index.ts` around lines 1906 - 1910, The estimateTokenCost function should defensively validate token counts: check tokensIn and tokensOut for negative values before using them (alongside the existing model lookup from OPENAI_PRICING_USD_PER_TOKEN), and if any are negative either clamp them to 0 or return 0 immediately; update the logic in estimateTokenCost to use the validated/clamped values when computing tokensIn * p.in + tokensOut * p.out to prevent negative-cost results.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/server/index.ts`:
- Line 2174: The current return statement uses JSON.stringify(t.value) which can
produce extremely long strings for large objects/arrays and break
compiledSummary; replace this with a truncated formatter that serializes t.value
but limits output length (e.g., 200-500 chars) and appends an ellipsis when
truncated. Implement or call a helper (e.g., formatValue or truncateJson) and
use it in the template return `${indent}${t.fact} ${t.op} ${formatted}` so that
t.value is safe to render in compiledSummary without changing t.fact/t.op
semantics.
---
Outside diff comments:
In `@src/server/index.ts`:
- Line 1802: The regex used to produce asciiSafeBody is wrong (/[�-�]/g matches
only '-' and U+FFFF); update the pattern used on rawBody in the asciiSafeBody
assignment to correctly match non-ASCII characters, e.g. replace /[�-�]/g with
/[\u0080-\uFFFF]/g or /[^\x00-\x7F]/g so the replacer (c => '\\u' +
c.charCodeAt(0).toString(16).padStart(4,'0')) is applied to all characters >=
0x80.
---
Nitpick comments:
In `@src/server/index.ts`:
- Around line 2146-2194: humanizeRule currently renders the entire PredicateTree
via predicateLabel which can produce extremely long strings for deeply
nested/complex rules; cap the output (e.g. ~500 chars) and append a
deterministic truncation marker like "... (more conditions)" so the
compose-confirm description stays readable. Modify humanizeRule/predicateLabel
to track a maxLength constant and either (a) stop recursing once the cumulative
output length would exceed that limit and return a truncated fragment with the
marker, or (b) build the full string then slice to maxLength and append the
marker—ensure the marker is always added deterministically (same input => same
output) and that triggers/actions/ratelimit formatting (variables triggers,
actions, rateLimit) keep their existing structure. Use the function names
predicateLabel and humanizeRule as the insertion points and preserve
indentation/line breaks when truncating.
- Around line 1933-2044: The applyManageActions function can leave draft and
active bundles inconsistent because it performs two separate redis.set calls
(keys.rulesDraft and keys.rulesActive) without atomicity; modify
applyManageActions to ensure both writes succeed or are rolled back by first
reading and storing the previous values (already read into draft/active), then
attempt the two writes with a small retry loop (e.g., 3 attempts with backoff)
around redis.set for each key, and if after retries one write succeeds and the
other ultimately fails, perform a compensating redis.set to restore the prior
value for the successful key and set persisted = false so callers know the
operation did not fully persist; keep the summary construction and error logging
(processLogger/console.warn) but ensure you reference and update keys.rulesDraft
and keys.rulesActive and the persisted flag accordingly.
- Around line 1906-1910: The estimateTokenCost function should defensively
validate token counts: check tokensIn and tokensOut for negative values before
using them (alongside the existing model lookup from
OPENAI_PRICING_USD_PER_TOKEN), and if any are negative either clamp them to 0 or
return 0 immediately; update the logic in estimateTokenCost to use the
validated/clamped values when computing tokensIn * p.in + tokensOut * p.out to
prevent negative-cost results.
🪄 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: CHILL
Plan: Pro
Run ID: 06533742-6c89-41fe-ab91-cbc6db968d3b
📒 Files selected for processing (8)
claudedocs/2026-05-14-module-split-plan.mdclaudedocs/2026-05-14-ux-best-practices-plan.mddevvit.jsonsrc/server/index.tssrc/server/routes-compose.test.tssrc/server/routes-dashboard.test.tssrc/server/routes-manage.test.tssrc/shared/redis-keys.ts
External code-review pointed out 5 valid issues. All addressed in this commit on the same branch — PR #44 picks them up automatically. ## Gemini code-assist ### #1 — Manage menu: parallelize per-rule dry-run reads The previous serial `for` loop multiplied Redis latency by the draft count. With up to 50 drafts and ~10ms per round-trip the difference is ~500ms vs ~10ms wall time. Switched to `Promise.allSettled([...])` so a single failed fetch doesn't stop the others. ### #2 — applyManageActions: enforce the 50-rule cap on apply pause / activate / unpause moves can push a bundle over the 50-rule cap even when no compose-rule call is made. Added an explicit check before the dual-write txn — the moderator gets a "Rule cap exceeded (active=N, draft=N, max=50). Delete a rule first" toast and the bundles stay untouched. New test in routes-manage.test.ts. ### #3 — Use the configured openaiModel as the fallback (not the literal "manage") The previous `'manage'` fallback for `bundle.llmModel` broke `estimateTokenCost()` because 'manage' isn't a key in `OPENAI_PRICING_USD_PER_TOKEN`. The dashboard shows "(~$0 on manage)" until a real compile lands. Now we read the configured `openaiModel` setting through `readOpenaiModel()` (already SELECTION-array-safe per PR #39). ### #4 — Atomic dual-write via WATCH/MULTI/EXEC (the previous PR description's claim was wrong) The previous `Promise.all([redis.set(activeKey, ...), redis.set(draftKey, ...)])` was NOT atomic — if the second `set` failed (e.g. plugin RPC blip), pause could leave a rule absent from active without showing up in draft. Switched to Devvit's `redis.watch(...).multi().set(...).set(...).exec()` pattern (same shape executor.ts already uses for audit writes), and check `exec()`'s null return for the WATCH-aborted case so two moderators managing rules at the same time get an actionable "Another moderator changed the rules at the same time — re-open Manage rules" toast. ## CodeRabbit ### #5 — humanizeRule: cap value-stringification at 100 chars A rule that uses `op: in` against a long allowlist (e.g. 50 banned domains) would otherwise render thousands of characters into the confirm form's `compiledSummary` field, blowing past Devvit's modal description budget and obscuring the actual rule structure. Truncate with `...` after 100 chars in the predicate-tree leaf renderer. ## Test infrastructure test/devvit-testkit.ts: fakeRedis.watch().exec() now returns `[] | null` (matching real Devvit Redis MULTI/EXEC semantics) instead of `void`. Without this, the new WATCH-abort detection in applyManageActions would fire on every test. Updated `FakeTxn.exec` type accordingly. ## Verification - `npm run check` 4/4 gates green - 210 tests pass (1 skipped) — added 1 cap-overflow test in routes-manage.test.ts
This was referenced May 14, 2026
ComBba
pushed a commit
that referenced
this pull request
May 15, 2026
External code-review pointed out 5 valid issues. All addressed in this commit on the same branch — PR #44 picks them up automatically. ## Gemini code-assist ### #1 — Manage menu: parallelize per-rule dry-run reads The previous serial `for` loop multiplied Redis latency by the draft count. With up to 50 drafts and ~10ms per round-trip the difference is ~500ms vs ~10ms wall time. Switched to `Promise.allSettled([...])` so a single failed fetch doesn't stop the others. ### #2 — applyManageActions: enforce the 50-rule cap on apply pause / activate / unpause moves can push a bundle over the 50-rule cap even when no compose-rule call is made. Added an explicit check before the dual-write txn — the moderator gets a "Rule cap exceeded (active=N, draft=N, max=50). Delete a rule first" toast and the bundles stay untouched. New test in routes-manage.test.ts. ### #3 — Use the configured openaiModel as the fallback (not the literal "manage") The previous `'manage'` fallback for `bundle.llmModel` broke `estimateTokenCost()` because 'manage' isn't a key in `OPENAI_PRICING_USD_PER_TOKEN`. The dashboard shows "(~$0 on manage)" until a real compile lands. Now we read the configured `openaiModel` setting through `readOpenaiModel()` (already SELECTION-array-safe per PR #39). ### #4 — Atomic dual-write via WATCH/MULTI/EXEC (the previous PR description's claim was wrong) The previous `Promise.all([redis.set(activeKey, ...), redis.set(draftKey, ...)])` was NOT atomic — if the second `set` failed (e.g. plugin RPC blip), pause could leave a rule absent from active without showing up in draft. Switched to Devvit's `redis.watch(...).multi().set(...).set(...).exec()` pattern (same shape executor.ts already uses for audit writes), and check `exec()`'s null return for the WATCH-aborted case so two moderators managing rules at the same time get an actionable "Another moderator changed the rules at the same time — re-open Manage rules" toast. ## CodeRabbit ### #5 — humanizeRule: cap value-stringification at 100 chars A rule that uses `op: in` against a long allowlist (e.g. 50 banned domains) would otherwise render thousands of characters into the confirm form's `compiledSummary` field, blowing past Devvit's modal description budget and obscuring the actual rule structure. Truncate with `...` after 100 chars in the predicate-tree leaf renderer. ## Test infrastructure test/devvit-testkit.ts: fakeRedis.watch().exec() now returns `[] | null` (matching real Devvit Redis MULTI/EXEC semantics) instead of `void`. Without this, the new WATCH-abort detection in applyManageActions would fire on every test. Updated `FakeTxn.exec` type accordingly. ## Verification - `npm run check` 4/4 gates green - 210 tests pass (1 skipped) — added 1 cap-overflow test in routes-manage.test.ts
ComBba
added a commit
that referenced
this pull request
May 15, 2026
…ices feat(ux): compose-confirm form + Manage rules menu + dashboard onboarding (Phase 1.7b)
ComBba
pushed a commit
that referenced
this pull request
May 15, 2026
Pre-publish production verification — covers the new flows that PRs #43, #44, #45 introduced but never round-tripped through Devvit's actual runtime: 1. Compose form rendering 2. Clarify modal renders the **select** field (was paragraph) 3. Re-compile → composeConfirmForm opens + compiledSummary visible 4. Save click → success toast 5. Dashboard onboarding card + token cost line 6. New "vibe-mod: Manage rules" menu opens with per-rule action select Each step writes a screenshot + JSON record into playwright/.auth/. End-of-run summary: playwright/.auth/verify-phase17b-result.json. Pre-req: `npx devvit upload` (NOT publish) — installs latest code on r/SocialSeeding for playtest. Cookie source is the same as chrome-reddit-v3.py (browser_cookie3 reads the user's local Chrome reddit.com cookies, no creds embedded in the script). Usage: source .venv-chrome-auth/bin/activate python scripts/chrome-reddit-verify-phase17b.py # HEADLESS=0 for visible browser, REDDIT_SUB=other for a different sub Why this is needed: 211 unit/integration tests cover the handler logic, but a production runtime issue (Devvit-specific form-field rendering, redis.watch/multi/exec semantics, new menu/form name registration) can still surface only at devvit upload time. Catching those before `devvit publish --public` saves the Reddit App Directory review round.
ComBba
pushed a commit
that referenced
this pull request
May 15, 2026
… DOM Iteration on the Phase 1.7b/c verify script after running it against the v0.0.44 install on r/SocialSeeding. Three real DOM gaps surfaced — none in server code, all in our test harness. ## Server: 14/14 PASS — production confirmed Surfaced live in this script's last run: - Clarify modal renders the suggestedAnswers `select` (PR #43) - clarificationTurn carrier + "(Round X of N)" (PR #44) - composeConfirmForm with compiledSummary + serializedRule (PR #44) - Save toast: 'Compiled rule "X". → post: modqueue. Dry-run started…' - Dashboard onboarding card + token cost line (PR #44 Tier 2/3) - Manage rules menu with per-rule action_* select fields (PR #44) ## Script changes 1. **dump_form()** — Devvit's modal markup doesn't expose a stable `<h2>` title or `[slot="description"]`, so the previous extraction returned empty strings on every form. Added `dialogText` (full text-content of the dialog) as the source of truth for description-style assertions. 2. **Form detection** — switched from title-text matching to **field-name signals**: - clarify form ⇔ presence of `clarificationTurn` - confirm form ⇔ `compiledSummary` + `serializedRule` - dashboard onboarding ⇔ "Welcome to vibe-mod" inside dialogText - manage form ⇔ at least one `action_*` field 3. **clarify-select-pick** — the previous picker called `locator('select, faceplate-select').click()` which timed out (Devvit renders the select as a Lit element with a different tag). Replaced with a single `page.evaluate` that scopes by the field's data-field-name wrapper, tries native `<select>` first, then any option-like child (faceplate-radio-input, faceplate-listbox-item, role=option). Successfully picks "Under 24 hours" in production. ## Verified behaviour \`playwright/.auth/verify-phase17b-result.json\` shows \`"overall": true\` and 15/15 step PASS.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Best-practice UX rework for the 5 deferred audit findings (#2 #3 #5 #7 #10) plus 4 new items (#A empty states, #B delete confirm, #C onboarding, #D token cost). User explicitly opted-in to Tier 1+2+3 + a separate "Manage rules" menu.
Design + tier rationale:
What changes — 9 audit items
Tier 1 — compose flow
Tier 2 — Manage rules menu + dashboard
Tier 3 — Onboarding
New endpoints
Test plan
Why this PR before module split (Phase 2b)
Module split is a pure refactor; this is a behaviour change. Demo video records this UI. UX changes must land before:
D-day status
2026-05-27 18:00 PT (Devpost firm). publish recommended start 2026-05-18 (D-9, 4d). Compose flow already live-verified in production via Chrome automation (PR #43 / v0.0.41). Next deploy = this PR + Phase 2b + Phase 3-5.
🤖 Generated with Claude Code
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항