feat(ux): redis-backed pending state + multi-block dashboard + short toast (Phase 2c demo polish)#49
Merged
Merged
Conversation
…+ short toast Three demo-recording UX wins driven by user feedback on the Phase 1.7b recording — the confirm/clarify modals were too long to fit on screen, the dashboard description collapsed to an unreadable wall of text, and the success toast got truncated mid-sentence. ## 1. Compose-confirm carries one short pendingId, not 7 internal carriers Before: the composeConfirmForm shipped 7 disabled `(internal)` fields across the form (rule / allowGuarded / serializedRule / tokensIn / tokensOut / llmModel / usingBYOK). The modal was so tall the moderator had to scroll past a wall of "(internal) ..." labels to see the compiled summary or the Save button. After: compose-rule-submit stashes the entire compile state under `keys.composePending(sub, pendingId)` (10-min TTL, JSON), then the form carries only the 12-char hex pendingId. compose-confirm-submit reads + deletes the entry. Three visible fields total: compiledSummary, editInsteadOfSave, pendingId — fits on one screen. New helper `newPendingId()` (crypto.getRandomValues, 12 hex chars). New Redis key `keys.composePending(sub, id)`. ## 2. Dashboard splits its summary into multiple disabled-paragraph blocks Before: every dashboard line was joined with `\n` into one big `description` string, which Devvit's modal collapses into a single soft-wrapping paragraph. The recording showed everything (welcome card, counts, token cost, dry-run preview, recent actions) as one unreadable run-on sentence. After: each section is a separate `paragraph` field with its own `label`. Devvit renders these as visually distinct blocks with the label as a sub-heading, so the moderator can actually read the sections at a glance. ## 3. Success toast is short Before: `Compiled rule "X". → post: modqueue. Dry-run started — open the subreddit ⋯ menu → "vibe-mod: View rules + log" to see preview.` ~150 chars — Devvit toasts truncated it mid-sentence. After: `Saved "X". Dry-run starts now.` < 60 chars. The compose-confirm form already shows the rule name + cost + humanizeRule output, so the toast just needs to confirm "this happened". ## Verification - `npm run check` 4/4 gates green (211 unit/integration tests) - Updated routes-compose.test.ts: pending-entry round-trip + short-toast assertions - Updated routes-dashboard.test.ts: text-helper that concatenates field labels+values (replaces the old single-`description` string scan) - Verify script (chrome-reddit-verify-phase17b.py): added FULLSCREEN + SLOW_MO env vars + multi-round clarify loop + better select picker. Refs: docs/demo-scenario.md (recording target)
|
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 (4)
개요Compose 규칙 제출 흐름을 Redis 임시 스테이징으로 재구성하여 검증된 데이터를 pendingId로 단기 저장하고, 대시보드 UI를 구조화된 다중 블록 레이아웃으로 변경합니다. 브라우저 검증 스크립트에 전체 화면/슬로우 모션 옵션과 다중 라운드 명확화 루프를 추가합니다. 변경 사항Compose 두 단계 레디스 스테이징 흐름
대시보드 다중 블록 레이아웃
브라우저 테스트 다중 라운드 명확화
예상 코드 리뷰 노력🎯 3 (보통) | ⏱️ ~25분 관련 PR
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 refactors the rule composition and dashboard forms to improve the user experience within Devvit's UI constraints. Key changes include moving internal compilation state to Redis to declutter modals, shortening success toasts to prevent truncation, and splitting the dashboard summary into multiple paragraph blocks for better readability. Additionally, the verification script was updated to handle multi-round LLM clarifications and new execution flags like FULLSCREEN and SLOW_MO. Feedback was provided regarding a misleading 'cancel' label on the dashboard onboarding form that fails to persist the dismissal due to platform limitations.
| description: summary, | ||
| description: 'Read-only summary. Per-rule activation lives in "vibe-mod: Manage rules".', | ||
| acceptLabel: 'Close', | ||
| cancelLabel: firstVisit ? "Don't show intro again" : 'Cancel', |
There was a problem hiding this comment.
The cancelLabel is set to "Don't show intro again" when firstVisit is true. However, in Devvit, clicking the cancel button on a form does not trigger the form's submit handler. This means that clicking this button will close the modal but will NOT persist the dismissal to Redis (the dashboard-action route is not called). Consequently, the onboarding intro will reappear on the next visit, making the button label misleading.
To fix this, you should either remove the misleading label or rely solely on the dismissOnboarding checkbox and the "Close" (accept) button for persistence.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/server/routes-compose.test.ts (1)
438-452: ⚡ Quick win테스트에서도
composePending키 헬퍼를 사용해 키 계약 드리프트를 막으세요.현재 하드코딩 문자열(
testsub:compose:pending:...)은 키 포맷 변경 시 테스트가 실제 계약과 어긋날 수 있습니다.♻️ 제안 수정안
import { fakeRedis, fakeReddit, fakeSettings, fakeScheduler, fakeFetch, fakeListing, openaiResponse, openaiError, } from '../../test/setup'; +import { keys } from '../shared/redis-keys'; @@ -const pendingJson = JSON.parse((await fakeRedis.get(`testsub:compose:pending:${fieldsByName.pendingId}`))!); +const pendingKey = keys.composePending('testsub', fieldsByName.pendingId as string); +const pendingJson = JSON.parse((await fakeRedis.get(pendingKey))!); @@ -expect(await fakeRedis.get(`testsub:compose:pending:${fieldsByName.pendingId}`)).toBeUndefined(); +expect(await fakeRedis.get(pendingKey)).toBeUndefined();🤖 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/routes-compose.test.ts` around lines 438 - 452, Replace the hardcoded Redis key string `testsub:compose:pending:${fieldsByName.pendingId}` with the project's key helper (e.g., composePending(...)) to avoid contract drift; update both reads/expectations that call fakeRedis.get(...) to use composePending with the pending id (fieldsByName.pendingId) and ensure any necessary import of composePending is added at the top of src/server/routes-compose.test.ts so fakeRedis.get(composePending(...)) and the final expect compare against the helper-built key.
🤖 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 `@scripts/chrome-reddit-verify-phase17b.py`:
- Line 43: The SLOW_MO environment parsing uses int(os.environ.get("SLOW_MO",
"0")) which will raise ValueError for non-numeric values; change it to a safe
parse that falls back to 0 (or a defined default) on any parsing error or
invalid input—e.g., read os.environ.get("SLOW_MO"), attempt to int() in a
try/except (or validate with str.isdigit()/regex) and set SLOW_MO to the default
on exception, so the script won’t crash when SLOW_MO contains invalid text.
In `@src/server/routes/compose.ts`:
- Around line 502-511: The daily quota increment currently lives inside
persistRuleAndStartDryRun (used on save), which lets users repeatedly trigger
compile operations without increasing the count; move the quota counting to
immediately after a successful compile/submit operation instead: remove the
increment logic from persistRuleAndStartDryRun and add the quota increment call
in the compile-submit flow (the handler/function that performs the compile and
calls OpenAI) so a successful compile immediately increments
todayCount/todayCounterKey. Ensure you reference and update the symbols
persistRuleAndStartDryRun and the compile-submit handler (the function that
invokes the compiler/OpenAI in this route) and preserve existing error handling
around the compile before incrementing.
- Around line 421-423: The read-and-consume of a pending item is not atomic:
code that calls redis.get(keys.composePending(subredditName, pendingId)) (where
pendingJson gets parsed into ComposePending) can be raced by concurrent requests
and later code that deletes or handles the pending (the block that previously
used pendingJson and later removed keys) may run twice; replace the non-atomic
get + later delete with an atomic GETDEL (or equivalent Redis EVAL/Lua
transaction) when fetching the pending entry so the read-and-consume happens
once—use redis GETDEL (or redis.sendCommand('GETDEL', ...)) or a Lua script to
return and remove keys.composePending(subredditName, pendingId) in one atomic
operation to guarantee single consumption and prevent duplicate
save/count/schedule flows.
- Around line 327-333: After calling redis.set for
keys.composePending(subredditName, pendingId) you must detect whether the set
succeeded and, if redis.expire(keys.composePending(...), 600) fails, immediately
roll back by deleting the key; update the try/catch so that after await
redis.set(...) you treat that as a successful write and wrap the subsequent
await redis.expire(...) in its own failure-handling path that calls await
redis.del(keys.composePending(subredditName, pendingId)) on expire error
(include any logged error), ensuring no TTL-less pending entries remain for
pendingId/subredditName.
---
Nitpick comments:
In `@src/server/routes-compose.test.ts`:
- Around line 438-452: Replace the hardcoded Redis key string
`testsub:compose:pending:${fieldsByName.pendingId}` with the project's key
helper (e.g., composePending(...)) to avoid contract drift; update both
reads/expectations that call fakeRedis.get(...) to use composePending with the
pending id (fieldsByName.pendingId) and ensure any necessary import of
composePending is added at the top of src/server/routes-compose.test.ts so
fakeRedis.get(composePending(...)) and the final expect compare against the
helper-built key.
🪄 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: 0c79add6-5f31-45d4-8cfb-c7ef5e7673d3
📒 Files selected for processing (6)
scripts/chrome-reddit-verify-phase17b.pysrc/server/routes-compose.test.tssrc/server/routes-dashboard.test.tssrc/server/routes/compose.tssrc/server/routes/dashboard.tssrc/shared/redis-keys.ts
5 review issues from Gemini code-assist + CodeRabbit on PR #49. ## Major ### CodeRabbit #4 — pending consumption now atomic (WATCH/MULTI/DEL/EXEC) Two concurrent submits with the same `pendingId` (back-button + re-submit, double-click on Save, multi-tab moderator) could both read the entry before either deleted it, doubling the bundle write + dry-run schedule. Replaced the plain GET-then-DEL with a WATCH/MULTI/DEL/EXEC round-trip. Whichever caller commits first wins; the loser sees `exec() == null` and gets an actionable "Another submission consumed this confirmation" toast. Updated FakeTxn to match (added `del()` method on the txn object — the production Devvit Redis client supports it, the fake didn't). ### CodeRabbit #5 — daily compile counter now bumps at compile time Quota was previously incremented inside `persistRuleAndStartDryRun()`, which only runs on Save. A moderator could repeatedly hit Compile + Cancel and burn through OpenAI tokens without the per-day counter ever moving. Moved the increment to immediately after a successful OpenAI return in compose-rule-submit. The token cost is real either way, so the quota now reflects it either way. ## Medium ### Gemini #1 — dashboard cancelLabel no longer says "Don't show intro again" Devvit's Cancel button does NOT trigger the form submit handler, so clicking the misnamed button could not actually persist `dismissOnboarding` — it just closed the modal. Reverted the label to "Cancel"; the dismissOnboarding boolean toggle + Close (acceptLabel) is the real opt-out path that submits the form values. ## Minor ### CodeRabbit #2 — verify script tolerates non-int SLOW_MO `int(os.environ.get("SLOW_MO", "0"))` crashed if a user passed e.g. `SLOW_MO=fast`. Added `_safe_int` helper that warns + falls back to the default rather than ending the recording session before it starts. ### CodeRabbit #3 — atomic SET + TTL via the `expiration` option Previous shape was `redis.set(key, value)` then `redis.expire(key, ttl)`, which could leak a TTL-less pending key if expire failed after set succeeded. Switched to `redis.set(key, value, { expiration: new Date(...) })` which is the same single round-trip the rollback writer in executor.ts already uses. ## Verification - `npm run check` 4/4 gates green (211 unit/integration tests) - All compose tests still pass with the new pending-id flow - Race condition smoke: a second compose-confirm-submit call against the same pendingId now returns the "another submission consumed" toast instead of double-persisting (previously: double bundle write).
ComBba
pushed a commit
that referenced
this pull request
May 15, 2026
5 review issues from Gemini code-assist + CodeRabbit on PR #49. ## Major ### CodeRabbit #4 — pending consumption now atomic (WATCH/MULTI/DEL/EXEC) Two concurrent submits with the same `pendingId` (back-button + re-submit, double-click on Save, multi-tab moderator) could both read the entry before either deleted it, doubling the bundle write + dry-run schedule. Replaced the plain GET-then-DEL with a WATCH/MULTI/DEL/EXEC round-trip. Whichever caller commits first wins; the loser sees `exec() == null` and gets an actionable "Another submission consumed this confirmation" toast. Updated FakeTxn to match (added `del()` method on the txn object — the production Devvit Redis client supports it, the fake didn't). ### CodeRabbit #5 — daily compile counter now bumps at compile time Quota was previously incremented inside `persistRuleAndStartDryRun()`, which only runs on Save. A moderator could repeatedly hit Compile + Cancel and burn through OpenAI tokens without the per-day counter ever moving. Moved the increment to immediately after a successful OpenAI return in compose-rule-submit. The token cost is real either way, so the quota now reflects it either way. ## Medium ### Gemini #1 — dashboard cancelLabel no longer says "Don't show intro again" Devvit's Cancel button does NOT trigger the form submit handler, so clicking the misnamed button could not actually persist `dismissOnboarding` — it just closed the modal. Reverted the label to "Cancel"; the dismissOnboarding boolean toggle + Close (acceptLabel) is the real opt-out path that submits the form values. ## Minor ### CodeRabbit #2 — verify script tolerates non-int SLOW_MO `int(os.environ.get("SLOW_MO", "0"))` crashed if a user passed e.g. `SLOW_MO=fast`. Added `_safe_int` helper that warns + falls back to the default rather than ending the recording session before it starts. ### CodeRabbit #3 — atomic SET + TTL via the `expiration` option Previous shape was `redis.set(key, value)` then `redis.expire(key, ttl)`, which could leak a TTL-less pending key if expire failed after set succeeded. Switched to `redis.set(key, value, { expiration: new Date(...) })` which is the same single round-trip the rollback writer in executor.ts already uses. ## Verification - `npm run check` 4/4 gates green (211 unit/integration tests) - All compose tests still pass with the new pending-id flow - Race condition smoke: a second compose-confirm-submit call against the same pendingId now returns the "another submission consumed" toast instead of double-persisting (previously: double bundle write).
ComBba
added a commit
that referenced
this pull request
May 15, 2026
…-state feat(ux): redis-backed pending state + multi-block dashboard + short toast (Phase 2c demo polish)
ComBba
pushed a commit
that referenced
this pull request
May 15, 2026
…ock dashboard, short toast) Three test-side updates after PR #49 reshaped the modals: 1. **confirm-form-renders** now checks for `pendingId + editInsteadOfSave + compiledSummary` (was: `serializedRule + compiledSummary` — the former field is gone, the rule lives in Redis under composePending). 2. **dashboard-onboarding / token-cost** now scans every field label + defaultValue on top of dialogText. Phase 2c split the dashboard's single description into per-section paragraph fields, so the keywords ("Welcome to vibe-mod", "Tokens used", "gpt-5") moved into field labels/values. 3. **save-toast-success** now matches the new short toast shape (`Saved "X". Dry-run starts now.`) instead of the old `Compiled rule "X". → trigger: action. Dry-run started — open menu...`. Verified against r/SocialSeeding v0.0.46 — 16/16 step PASS, including a two-round clarify (the script's multi-round loop kicks in when the LLM needs another disambiguation 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
Three demo-recording UX wins driven by user feedback on the Phase 1.7b take.
Changes
`src/server/routes/compose.ts`
`src/server/routes/dashboard.ts`
`src/shared/redis-keys.ts`
Tests
`scripts/chrome-reddit-verify-phase17b.py`