Conversation
Three-track design covering prod infra bootstrap, build & harden, and cutover & go-live. Key decisions: anonymous SMS deferred (RA 10173 §16 blocker), monitoring alerts activate at cutover only, staging-parity gates all Track 2 artifacts, restore drill in staging before prod touch. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Scraper + manual toggle deploy to prod in Track 1; official PAGASA webhook API is not a pilot-launch gate. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n\nEstablishes full design system context for the Bantayog Alert citizen PWA.\nThe Calm Sentinel north star, Authority Navy + Alert Sienna palette, WCAG AAA targets.\n\nCo-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…g hints, elevate copy\n\n- Fix .field-group--urgent: remove side-stripe (border-left violation) â�� full 2px border\n- Add .tl-hint and .memory-hint utility classes to globals.css\n- Step1: subtitle Tagalog hint, improved camera placeholder text, incident label hint\n- Step2: subtitle rewrite with Tagalog, location picker prompt more welcoming\n- Step3: consent banner Tagalog inline, checkbox text explicit + Tagalog, submit CTA bilingual\n\nCo-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ail sheets\n\n- FilterBar: replace cycling pills with simultaneous chip groups (All/High/Medium/Low + 24h/7d/30d)\n- FilterBar.test: rewrite tests for chip-based API with aria-pressed assertions\n- PeekSheet: add optional severity dot, split label into primary/secondary lines, styled Details button\n- DetailSheet: public mode full redesign â�� severity badge, location row, Report similar CTA\n- index.tsx: pass severity to SelectedPin for color-coded peek, wire onReportSimilar via useNavigate\n\nCo-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- FeedTab: sticky header + chip filters (severity/window), FeedCard, SkeletonCard, empty/error states, taps navigate to /incidents/:id - IncidentDetailPage: full public detail view at /incidents/:id with severity badge, location/status rows, report-similar CTA - useIncident: getDoc-based hook with isPublicIncidentData guard - routes: /feed wired to FeedTab, /incidents/:id added (hideBottomNav) - App.routes.test: mock FeedTab + IncidentDetailPage, fix tab assertion Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- ProfileTab: sticky header, report cards (icon/severity dot/status badge/ref), loading skeletons, empty state with submit CTA - Report cards tap to /reports/:publicRef (TrackingScreen) - Privacy section retains DeleteAccountFlow with goodbye nav - routes: import ProfileTab, remove inline stub + unused DeleteAccountFlow Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- useAlerts: wraps subscribeAlerts from shared-firebase, handles Firebase not-configured fallback gracefully - AlertsTab: sticky header, severity-sorted cards (critical→info), loading skeletons, empty state with Tagalog subtitle - Alert cards: tinted bg for critical/high, severity badge pill, title/body/time layout — no side-stripe design violations - routes: /alerts wired to AlertsTab, StubTab removed Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ard, Tagalog hints - .reveal-sheet: max-height 90svh + overflow-y:auto for small screens - @Keyframes revealSlideUp: spring-eased entrance (0.28s cubic-bezier) - success state: Tagalog subline "Narinig namin kayo..." - failed state: Tagalog subline "Ligtas pa rin ang inyong ulat..." Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (16)
📝 WalkthroughWalkthroughAdds a Phase 9 design system (DESIGN.json / DESIGN.md / PRODUCT.md), new UI screens and components (AlertsTab, FeedTab, ProfileTab, IncidentDetailPage, MapTab refinements), Firebase-backed hooks (useAlerts, useIncident, subscribeAlerts onError), routing for /incidents/:id, bilingual copy updates, tests, and documentation/spec additions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Router
participant FeedTab
participant usePublicIncidents as Hook
participant Firestore
participant IncidentDetailPage
User->>Router: Navigate to /feed
Router->>FeedTab: render
FeedTab->>Hook: request incidents (filters)
Hook->>Firestore: subscribe/query
Firestore-->>Hook: stream results
Hook-->>FeedTab: incidents data
FeedTab-->>User: render incident cards
User->>FeedTab: click incident card
FeedTab->>Router: navigate /incidents/{id}
Router->>IncidentDetailPage: render (with id)
IncidentDetailPage->>Firestore: getDoc via useIncident
Firestore-->>IncidentDetailPage: document
IncidentDetailPage-->>User: render details
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 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. Review rate limit: 0/1 reviews remaining, refill in 32 minutes and 24 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/citizen-pwa/src/styles/globals.css (1)
395-408:⚠️ Potential issue | 🟠 MajorAdd reduced-motion fallback for the new sheet animation.
The new slide-up motion currently has no
prefers-reduced-motionbranch, which conflicts with the documented accessibility requirement.Suggested fix
.reveal-sheet { ... animation: reveal-slide-up 0.28s cubic-bezier(0.34, 1.56, 0.64, 1) forwards; } + +@media (prefers-reduced-motion: reduce) { + .reveal-sheet { + animation: none; + } +}Also applies to: 508-515
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/citizen-pwa/src/styles/globals.css` around lines 395 - 408, The .reveal-sheet animation lacks a prefers-reduced-motion fallback; update the CSS to respect the user preference by adding a `@media` (prefers-reduced-motion: reduce) rule that targets .reveal-sheet and disables or replaces the animation (for example remove animation and use a non-animated state such as setting transform/opacity to the final values), and ensure the keyframes referenced (revealSlideUp) are not applied under that media query; apply the same change for the other instance of .reveal-sheet/revealSlideUp in the file so both occurrences are covered.apps/citizen-pwa/src/components/SubmitReportForm/Step1Evidence.tsx (1)
132-141:⚠️ Potential issue | 🟡 MinorButton text/ARIA overstate behavior.
At Line 140 and Line 138, the control says it will “continue,” but the click handler only clears
photoFile. This can mislead users and screen-reader flow expectations.Suggested fix
- aria-label="Skip photo and continue without one" + aria-label="Skip photo for now" > - Skip photo — continue without + Skip photo for now </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/citizen-pwa/src/components/SubmitReportForm/Step1Evidence.tsx` around lines 132 - 141, The button text/ARIA claim it will "continue" but the onClick only calls setPhotoFile(null), which is misleading; either (A) update the onClick to also call the step-advancing handler (e.g., call the component's next-step function such as handleNext or onNext immediately after setPhotoFile(null)) so the control actually proceeds, or (B) change the visible text and aria-label to reflect only removing the photo (e.g., "Remove photo — stay on this step" and aria-label "Remove photo and remain on this step") so behavior and label match; locate the button using setPhotoFile and photoFile to apply the fix.apps/citizen-pwa/src/App.routes.test.tsx (1)
59-66:⚠️ Potential issue | 🟡 MinorAdd a direct route test for
/incidents/:id.Line 59 starts shell-tab navigation coverage, but the new incident-detail route is still unverified in this suite. That leaves
handle.hideBottomNavbehavior and route rendering unguarded.✅ Suggested test addition
describe('App routes', () => { @@ it('navigates between shell tabs', async () => { await renderAppAt('/') fireEvent.click(screen.getByRole('button', { name: /feed/i })) await waitFor(() => { expect(screen.getByText(/Feed tab/)).toBeInTheDocument() }) }) + + it('shows incident detail without shell chrome at /incidents/:id', async () => { + await renderAppAt('/incidents/test-id') + expect(screen.getByText('Incident detail')).toBeInTheDocument() + expect(screen.queryByRole('banner')).not.toBeInTheDocument() + expect(screen.queryByRole('navigation', { name: /main navigation/i })).not.toBeInTheDocument() + }) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/citizen-pwa/src/App.routes.test.tsx` around lines 59 - 66, Add a new test in App.routes.test.tsx that directly exercises the incident-detail route: use renderAppAt('/incidents/1') (same helper used in the suite), then waitFor and assert the incident detail UI is rendered via screen.getByText or getByRole for the expected content, and verify bottom navigation is hidden by either asserting the nav element is not in the document (queryByRole/queryByTestId) or by spying on the hide handler with jest.spyOn(handle, 'hideBottomNav') and asserting it was called; use the existing helpers (renderAppAt, screen, waitFor) and the handle.hideBottomNav symbol to locate and verify the behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/citizen-pwa/src/components/SubmitReportForm/Step2WhoWhere.tsx`:
- Line 184: Update the misleading CTA string in the Step2WhoWhere React
component: replace the span text "Pick my municipality on the map" with a
clearer label such as "Pick my municipality manually" (or similar) so it
accurately reflects that the UI uses municipality/barangay selectors rather than
a map; locate the span inside the Step2WhoWhere component and update the copy
accordingly.
In `@apps/citizen-pwa/src/hooks/useAlerts.ts`:
- Around line 6-26: Add an explicit error state to useAlerts and stop treating
backend/permission failures as an empty alerts array: change useAlerts to return
{ alerts: AlertDoc[]; loading: boolean; error: Error | null }, initialize error
to null, and setError when the subscription reports a failure; update
subscribeAlerts so it signals errors (e.g., call the callback with null or
provide an error callback) instead of mapping Firestore errors to callback([]),
and in the useEffect handle that failure signal by setError(err),
setLoading(false), and leaving alerts unchanged so the UI can show a real error
banner instead of “No active alerts.”
In `@apps/citizen-pwa/src/hooks/useIncident.ts`:
- Around line 6-25: The isPublicIncidentData type guard currently allows any
string for reportType, severity, and status; tighten it by checking those fields
against the actual PublicIncident enum/value sets (e.g. the specific union or
enum types used by PublicIncident such as PublicIncidentReportType,
PublicIncidentSeverity, PublicIncidentStatus) instead of typeof === 'string'.
Update isPublicIncidentData to import or reference the canonical allowed values
and validate that data.reportType, data.severity, and data.status are members of
those sets (while keeping the existing numeric checks for submittedAt and
lat/lng and the object checks for publicLocation).
- Around line 37-77: The effect launches an async getDoc() that can complete
after the id/firebaseConfigured changes and overwrite state; wrap the async work
with a local cancellation flag (e.g., let cancelled = false) created inside
useEffect, return a cleanup that sets cancelled = true, and before every
setIncident/setLoading/setError call inside the .then and .catch handlers (and
early-return branches) check if cancelled and skip setting state if true; update
the useEffect closure around getDoc/doc/db/isPublicIncidentData so you only
mutate state when cancelled is false.
In `@apps/citizen-pwa/src/styles/globals.css`:
- Line 407: The keyframe name revealSlideUp violates the kebab-case pattern;
rename the `@keyframes` declaration from revealSlideUp to reveal-slide-up and
update every reference (e.g., animation: revealSlideUp 0.28s ... and any
animation-name usages) to use reveal-slide-up; also apply the same rename for
the other occurrence block around lines 508-515 so all animations and keyframe
names match kebab-case.
In `@DESIGN.json`:
- Line 418: The DESIGN.json entries for "Text Input Field" and "Bottom Tab
Navigation" incorrectly use "refersTo": "button-primary"; update those component
mappings so each refers to a semantically correct token (e.g., change the "Text
Input Field" entry's refersTo to a text/input token like "input-default" or
"text-field" and change the "Bottom Tab Navigation" entry's refersTo to a
navigation/tab token like "tab-bar" or "bottom-tab"); locate the objects titled
"Text Input Field" and "Bottom Tab Navigation" and replace their refersTo values
accordingly so automated consumers receive correct semantic tokens.
In `@DESIGN.md`:
- Around line 292-293: The current wording calls out the legacy
`.field-group--urgent` left-stripe pattern even though the design now uses a
full-border treatment; remove the specific reference and rephrase the rule as a
general prohibition: state that teams must not use `border-left` or
`border-right` greater than 1px as a colored stripe on cards, list items,
alerts, or callouts and instead use a full border, background tint, or leading
icon; keep the separate prohibition against gradient text (`background-clip:
text`) for status colors.
- Around line 209-215: The elevation section contains conflicting rules: the
"Hair Lift" entry permits a card shadow (box-shadow: 0 1px 3px rgba(0,0,0,0.10))
while the "Flat-By-Default Rule" forbids shadows on static content like cards;
reconcile this by choosing one consistent policy and updating the entries
accordingly — either remove or reclassify "Hair Lift" so cards are flat by
default, or amend "Flat-By-Default Rule" to explicitly allow the "Hair Lift" for
cards; update the text for "Hair Lift", "Sheet Rise", "Focus Ring", and "Frosted
Nav" to reflect the chosen single rule and ensure the prohibition/allowance
language is unambiguous.
In `@docs/superpowers/specs/2026-04-29-phase9-design.md`:
- Around line 20-24: Several fenced code blocks (for example the block
containing "Track 1 — Prod Infra Bootstrap (~2 days) / Track 2 — Build & Harden
/ Track 3 — Cutover & Go-Live") are missing language identifiers and trigger
MD040; update each opening triple-backtick to include an appropriate language
tag (e.g., ```text, ```bash, or ```ts) for that schedule/list block and the
other similar fenced blocks in the document so the markdown linter stops
flagging them.
In `@PRODUCT.md`:
- Around line 3-6: Remove the accidental stub by deleting the "## Register"
heading and the lone "product" line from PRODUCT.md; ensure no other content
relies on that placeholder and adjust surrounding headings so the document
structure remains coherent (look for the literal strings "## Register" and
"product" to locate the lines to remove).
---
Outside diff comments:
In `@apps/citizen-pwa/src/App.routes.test.tsx`:
- Around line 59-66: Add a new test in App.routes.test.tsx that directly
exercises the incident-detail route: use renderAppAt('/incidents/1') (same
helper used in the suite), then waitFor and assert the incident detail UI is
rendered via screen.getByText or getByRole for the expected content, and verify
bottom navigation is hidden by either asserting the nav element is not in the
document (queryByRole/queryByTestId) or by spying on the hide handler with
jest.spyOn(handle, 'hideBottomNav') and asserting it was called; use the
existing helpers (renderAppAt, screen, waitFor) and the handle.hideBottomNav
symbol to locate and verify the behavior.
In `@apps/citizen-pwa/src/components/SubmitReportForm/Step1Evidence.tsx`:
- Around line 132-141: The button text/ARIA claim it will "continue" but the
onClick only calls setPhotoFile(null), which is misleading; either (A) update
the onClick to also call the step-advancing handler (e.g., call the component's
next-step function such as handleNext or onNext immediately after
setPhotoFile(null)) so the control actually proceeds, or (B) change the visible
text and aria-label to reflect only removing the photo (e.g., "Remove photo —
stay on this step" and aria-label "Remove photo and remain on this step") so
behavior and label match; locate the button using setPhotoFile and photoFile to
apply the fix.
In `@apps/citizen-pwa/src/styles/globals.css`:
- Around line 395-408: The .reveal-sheet animation lacks a
prefers-reduced-motion fallback; update the CSS to respect the user preference
by adding a `@media` (prefers-reduced-motion: reduce) rule that targets
.reveal-sheet and disables or replaces the animation (for example remove
animation and use a non-animated state such as setting transform/opacity to the
final values), and ensure the keyframes referenced (revealSlideUp) are not
applied under that media query; apply the same change for the other instance of
.reveal-sheet/revealSlideUp in the file so both occurrences are covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f64ce6e7-8372-4ce5-bd40-68b757acbff9
📒 Files selected for processing (24)
DESIGN.jsonDESIGN.mdPRODUCT.mdapps/citizen-pwa/src/App.routes.test.tsxapps/citizen-pwa/src/components/AlertsTab.tsxapps/citizen-pwa/src/components/FeedTab.tsxapps/citizen-pwa/src/components/IncidentDetailPage.tsxapps/citizen-pwa/src/components/MapTab/DetailSheet.tsxapps/citizen-pwa/src/components/MapTab/FilterBar.test.tsxapps/citizen-pwa/src/components/MapTab/FilterBar.tsxapps/citizen-pwa/src/components/MapTab/PeekSheet.tsxapps/citizen-pwa/src/components/MapTab/index.tsxapps/citizen-pwa/src/components/ProfileTab.tsxapps/citizen-pwa/src/components/RevealSheet.tsxapps/citizen-pwa/src/components/SubmitReportForm/Step1Evidence.tsxapps/citizen-pwa/src/components/SubmitReportForm/Step2WhoWhere.tsxapps/citizen-pwa/src/components/SubmitReportForm/Step3Review.tsxapps/citizen-pwa/src/hooks/useAlerts.tsapps/citizen-pwa/src/hooks/useIncident.tsapps/citizen-pwa/src/routes.tsxapps/citizen-pwa/src/styles/globals.cssdocs/learnings.mddocs/progress.mddocs/superpowers/specs/2026-04-29-phase9-design.md
Address PR #85 review comment: stop silently mapping Firestore errors to an empty array. Consumers can now pass an onError handler to receive the actual error.
Address PR #85 review comment: return { alerts, loading, error } so backend/permission failures surface as real errors instead of empty arrays.
…tion Address PR #85 review comments: - Validate reportType, severity, status against canonical value sets. - Add cancelled flag in useEffect to prevent state updates after unmount.
Address PR #85 review comment: 'Pick my municipality manually' reflects selector-based UI instead of map-based language.
Address PR #85 review comment: verify incident detail renders and bottom nav is hidden at the dynamic route.
Address PR #85 review comment: Text Input Field and Bottom Tab Navigation now point to semantically correct tokens (text-field, bottom-tab).
…ence Address PR #85 review comments: - Clarify Hair Lift as the single exception to Flat-By-Default. - Remove outdated .field-group--urgent left-stripe callout.
Address PR #85 review comment: delete stray '## Register' heading.
Address PR #85 review comment (MD040): annotate fence blocks with 'text' where no language was specified.
…remony layer\n\nTwo-sub-spec design covering DESIGN.md compliance fixes (wordmark, emojis->Lucide,\nchip styles, badge sizing) and role-spec-v2 gaps (secret code, LookupScreen redesign,\nauth-aware ProfileTab, /register OTP flow, /settings page, offline banner).\n\nIncludes a motion and ceremony design language section (anticipation -> reveal ->\nafterglow) mapping the three emotional stages to specific app moments: typewriter\nreference code reveal, haptic confirmation, OTP digit micro-ceremony, resolved\nafterglow card, and session upgrade prompt.\n\nCo-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…yer\n\nDesign spec for the Responder Capacitor app full redesign covering:\n- The Night Watch creative north star â�� dark operational surface\n- Design token extensions: shell-bg, ops-teal accent, dark text palette\n- 4-tab shell (Dispatches / Map / Messages / Profile)\n- 5 ceremony moments: dispatch acceptance, race loss, on-scene,\n resolution card, SOS 3-second hold\n- Ceremony hooks architecture with useHaptic + useReducedMotion primitives\n- 12h re-auth non-dismissible modal ceremony\n- Full reduced-motion fallback table + accessibility spec\n- Sub-spec decomposition: Sub-spec 1 shell+ceremony, Sub-spec 2 spec gaps\n\nCo-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nce + role-spec-v2 Completes Sub-spec 1 (design compliance) and Sub-spec 2 (role-spec-v2 gaps): Sub-spec 1: - utils/incident-meta.tsx â�� shared Lucide icon/label maps for 12 incident types - Wordmark VIGILANT â�� BANTAYOG ALERT in CitizenShell - FeedTab: no-border chips, #f2f4f6 inactive, badge 0.75rem, Lucide icons - ProfileTab: same consolidation, auth-aware pseudonymous banner - AlertsTab: Lucide severity icons (Siren/Bell), issuedBy attribution - RevealSheet: typewriter (60ms/char, 400ms settle), haptic(200), copy prompt Sub-spec 2: - useReducedMotion hook â�� matchMedia listener - Toggle component â�� accessible switch with keyboard + reduced-motion - useToast + Toast â�� 3s auto-dismiss with slide-up animation - useOfflineQueueCount â�� polls draftStore every 5s - Offline banner in CitizenShell â�� useOnlineStatus + useOfflineQueueCount - LookupScreen redesign â�� navy header, dual-code inputs, requestLookup callable - RegisterPage â�� 3-step OTP (phoneâ��OTPâ��name) with RecaptchaVerifier - SettingsPage â�� push/offline toggles, storage estimate, data export, sign-out - /register and /settings routes - requestDataExport callable wrapper (placeholder) All 203 tests pass, lint + typecheck clean. Refs #85
Contributor
There was a problem hiding this comment.
Actionable comments posted: 23
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/citizen-pwa/src/components/RevealSheet.tsx (1)
169-173:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeyboard close behavior is inconsistent with click close behavior.
Line 168 allows backdrop click to close in all states, but Line 172 blocks Enter/Space closing unless
state === 'success'. Keyboard users should get the same behavior.Suggested fix
- if (e.key === 'Enter' || e.key === ' ') { + if (e.key === 'Enter' || e.key === ' ') { e.preventDefault() - if (state === 'success') onClose?.() + onClose?.() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/citizen-pwa/src/components/RevealSheet.tsx` around lines 169 - 173, In RevealSheet's onKeyDown handler, the Enter/Space key handling currently only calls onClose when state === 'success', causing inconsistent keyboard vs backdrop click behavior; update the handler in the RevealSheet component to call onClose for Enter or Space without the state === 'success' guard (or mirror the same condition used for backdrop click) so keyboard users can close the sheet the same way as pointer users.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/citizen-pwa/src/components/AlertsTab.tsx`:
- Around line 175-251: The component drops the error from useAlerts and thus
shows the empty "No active alerts" state on fetch/permission failures; update
AlertsTab to destructure error from useAlerts (const { alerts, loading, error }
= useAlerts()), check error before rendering the empty-state branch, and render
an error UI (e.g., a visible message with brief description, optionally a retry
control) when error is present; also log or send the error (console.error or a
logger) so failures in useAlerts are surfaced instead of falling through to
sorted.map/.length logic.
In `@apps/citizen-pwa/src/components/CitizenShell.tsx`:
- Line 63: The main element in CitizenShell is adding extra top padding when
showBanner is true (paddingTop: showBanner ? 100 : 64), resulting in double
offset; update the style for the main element to stop adding the extra 36px when
the banner is visible — e.g., set paddingTop to the base value (64) and remove
the conditional addition, or compute paddingTop by subtracting the banner's
height when showBanner is true so the banner's own layout space isn't doubled;
locate the main JSX in CitizenShell.tsx and adjust the inline style using the
showBanner boolean accordingly.
In `@apps/citizen-pwa/src/components/FeedTab.test.tsx`:
- Around line 30-37: The test 'renders filter chips without border' can
false-pass if no aria-pressed chips exist; update the test in FeedTab.test.tsx
(inside the it block using renderFeedTab and chips =
screen.getAllByRole('button')) to first filter chips for those with
chip.getAttribute('aria-pressed') !== null and assert that the filtered array
length is > 0 (or use expect(...).toBeGreaterThan(0)) before iterating to check
chip.style.border contains 'none' so the border assertion only runs when at
least one filter chip is present.
In `@apps/citizen-pwa/src/components/LookupScreen.tsx`:
- Around line 114-131: The secret input currently renders as plain text; update
the input in LookupScreen (the element bound to value={secret} and onChange
calling setSecret) to use type="password" and disable accidental exposure by
adding attributes like autoComplete="new-password" (or "off"),
autoCorrect="off", autoCapitalize="off", and spellCheck={false}; keep the
existing handlers and styles intact so only the input attributes change.
In `@apps/citizen-pwa/src/components/RevealSheet.test.tsx`:
- Around line 30-36: The test enables fake timers in the beforeEach block via
vi.useFakeTimers() but never restores real timers; add an afterEach cleanup that
calls vi.useRealTimers() to restore real timers for test isolation and also
consider reverting the navigator.vibrate override (undo the
Object.defineProperty(navigator, 'vibrate') change) in the same afterEach so
RevealSheet.test.tsx leaves global state unchanged between tests.
In `@apps/citizen-pwa/src/components/RevealSheet.tsx`:
- Around line 69-90: The useEffect in the RevealSheet component leaks the
charInterval because the cleanup only clears settleTimeout; declare charInterval
in the outer scope of the effect (e.g., let charInterval = null), assign the
interval to it inside the timeout, and ensure the cleanup function clears both
settleTimeout and charInterval (if non-null) so
setDisplayedChars/setTypewriterComplete cannot run after unmount; keep the
existing clearInterval inside the interval callback but still clear charInterval
on unmount to be safe.
- Around line 92-100: The handleCopySecret function currently calls
navigator.clipboard.writeText without handling rejections, so add a try/catch
around the async write operation in handleCopySecret to catch errors (e.g.,
permission denied or unavailable API); on success call setCopied(true) and start
the timeout to reset via setCopied(false), and on failure ensure
setCopied(false) is set (or left false) and log or surface the error (e.g.,
console.error or a user-facing toast) so the user isn't misled; keep the timeout
cleanup logic (clearTimeout if needed) and reference handleCopySecret,
navigator.clipboard.writeText, setCopied, and the timeout variable when making
changes.
In `@apps/citizen-pwa/src/components/Toast.test.tsx`:
- Around line 11-13: The tests enable fake timers in beforeEach via
vi.useFakeTimers() but never advance or use timers, which can leak state; either
remove the vi.useFakeTimers() call from the beforeEach block or add a teardown
to restore real timers by adding an afterEach(() => vi.useRealTimers()) so the
suite returns to normal timer behavior (refer to the beforeEach and
vi.useFakeTimers() invocation and, if added, the afterEach/vi.useRealTimers()
call).
In `@apps/citizen-pwa/src/hooks/useAlerts.ts`:
- Around line 21-28: The snapshot success handler in useAlerts sets alerts and
loading but doesn’t clear any previous error, so after a prior subscription
error consumers may still see error state; update the success callback (the
function that calls setAlerts and setLoading) to also clear error by calling
setError(null) (or the appropriate empty value) so successful docs reset error
state; ensure the error callback still sets setError(err) and setLoading(false).
In `@apps/citizen-pwa/src/hooks/useIncident.ts`:
- Around line 6-18: The VALID_REPORT_TYPES array is missing the supported
incident type "power_outage", causing valid incidents to be treated as invalid;
update the VALID_REPORT_TYPES constant in useIncident (the readonly string[]
named VALID_REPORT_TYPES) to include 'power_outage' alongside the other entries
so it matches the set defined in incident-meta.tsx and accepts power outage
reports.
- Around line 40-61: The type guard isPublicIncidentData currently ignores the
type of an optional verifiedAt field, so you must validate it before narrowing
to Omit<PublicIncident, 'id'>; update the guard (isPublicIncidentData) to check
that if data.verifiedAt !== undefined then typeof data.verifiedAt === 'number'
and Number.isFinite(data.verifiedAt) (or allow null only if PublicIncident
permits), ensuring the optional verifiedAt is correctly typed before returning
true; adjust the return condition in isPublicIncidentData accordingly so
downstream code receives properly validated PublicIncident-shaped data.
In `@apps/citizen-pwa/src/hooks/useOfflineQueueCount.ts`:
- Around line 10-21: The polling callback (update) can reject and setCount may
run after unmount; wrap the async work in update (the call to draftStore.list
and subsequent logic) in a try/catch and log/suppress errors to avoid unhandled
rejections, and add an isMounted flag (or AbortController) scoped to the hook to
guard calls to setCount so it only runs when mounted; also ensure the interval
created with POLL_INTERVAL_MS (the interval variable) is cleared on cleanup to
prevent continued execution after unmount.
In `@apps/citizen-pwa/src/hooks/useToast.ts`:
- Around line 13-21: The timeout set in toast (timerRef) can fire after unmount
causing stale state updates; add a useEffect with a cleanup that clears
timerRef.current on unmount and also ensure the timeout callback resets
timerRef.current = null after hiding the toast to avoid double-clears; update
the hook around the toast function (reference: timerRef, toast, setState) to
include useEffect(() => () => { if (timerRef.current)
clearTimeout(timerRef.current); timerRef.current = null; }) and set
timerRef.current = null inside the setTimeout callback after calling setState.
In `@apps/citizen-pwa/src/pages/RegisterPage.tsx`:
- Around line 31-37: Guard accesses to auth() before dereferencing currentUser:
in the handlers (e.g., handleSendOtp and the other handler around lines 66–71)
call auth() once inside a try/catch or assign it to a local variable (e.g.,
const firebaseAuth = auth()) and verify firebaseAuth is defined before reading
firebaseAuth.currentUser; if auth() throws or returns falsy, short-circuit to
the existing toast('Not signed in','error') / return path and avoid accessing
.currentUser to prevent runtime exceptions. Ensure both handler functions use
the same guarded pattern.
In `@apps/citizen-pwa/src/pages/SettingsPage.tsx`:
- Around line 41-45: The catch block around
localStorage.setItem('bantayog_offline_mode', String(v)) is empty and swallows
errors; update the try/catch to capture the error (e) and report it (e.g.,
console.error or your app telemetry/captureException) including context such as
the key 'bantayog_offline_mode' and the value variable v so failures are
observable and diagnosable; keep existing behavior (don’t rethrow) if storage is
optional but always log the caught error in SettingsPage (where
localStorage.setItem is called).
- Around line 48-51: handleSignOut currently awaits signOut(auth()) without
handling rejection; wrap the signOut call in a try-catch inside the
handleSignOut function, call signOut(auth()) in the try block and only call
navigate('/', { replace: true }) on success, and in the catch block use the
toast from useToast() to show a user-facing error message (and optionally log
the error) so failures are surfaced to users.
- Around line 26-37: Guard against browsers that lack StorageManager by checking
navigator and navigator.storage and that navigator.storage.estimate is a
function before calling it in the useEffect; if unavailable, immediately call
setStorageInfo('Storage info unavailable'). Also wrap the estimate call in a
try/catch to handle any synchronous throws and still call setStorageInfo on
error. Update the block referencing useEffect, navigator.storage.estimate(), and
setStorageInfo to perform the feature-detection and safe fallback.
In `@DESIGN.json`:
- Around line 331-342: The DESIGN.json contains a contradictory shadow policy:
the "hair-lift" token (used for cards) conflicts with the Flat-By-Default rule
that forbids shadows on static cards; reconcile by choosing one source of
truth—either remove or repurpose "hair-lift" or relax the Flat-By-Default rule
to allow a specific exception. Update the "hair-lift" token's "purpose" to
explicitly state it is deprecated/conditional (e.g., "only for interactive/hover
states") or remove it entirely, and adjust the Flat-By-Default policy text to
reference that exception; ensure any duplicated definitions (the other
occurrence referenced in the comment) are changed consistently so the artifact
contains a single unambiguous rule about card shadows.
- Line 413: The status badge is using the old micro size; update the
.ds-card-status rule to use the redesign label size by changing its font-size
from 0.625rem to 0.75rem and ensure any variant classes like
.ds-card-status--dispatched continue to inherit that size; verify
spacing/line-height still looks correct after the change and adjust padding if
needed so the label-size badge matches the redesign tokens.
In `@docs/superpowers/plans/2026-04-30-citizen-pwa-redesign.md`:
- Around line 348-351: The fenced TypeScript code block containing the route
entries ({ path: '/register', element: <RegisterPage /> }, { path: '/settings',
element: <SettingsPage /> }) needs a blank line before the opening ```ts fence
and a blank line after the closing ``` fence to satisfy markdownlint MD031;
update the “Add routes” section so there is an empty line immediately above the
```ts and an empty line immediately below the closing ``` to fix the spacing.
In `@docs/superpowers/plans/2026-04-30-responder-app-redesign.md`:
- Around line 821-840: The onPointerDown handler in useSosHold can start
overlapping intervals and never clears timers on unmount; add a re-entry guard
at the top of onPointerDown (e.g., if (intervalRef.current) return or check
phase/holding flag) and clear any existing interval before creating a new one
(clearInterval(intervalRef.current)); also ensure the hook returns a cleanup
effect that clears intervalRef.current and sets cancelledRef.current = true on
unmount to prevent leaked timers and duplicate activations; update references to
intervalRef, cancelledRef, startRef, onPointerDown and the hook's useEffect
cleanup accordingly.
- Line 13: Heading level jumps from top-level heading to "### Task 1: Set Up
Test Infrastructure", triggering MD001; change the heading "Task 1: Set Up Test
Infrastructure" from "###" to "##" (i.e., make it a second-level heading) so the
document flows from the existing top-level "#" to "##" and satisfies markdown
lint.
In `@docs/superpowers/specs/2026-04-30-responder-app-redesign-design.md`:
- Around line 111-118: The fenced code block that starts with the lines "[status
badge] [`#NNNN` mono]" is missing a language identifier and
triggers MD040; update the opening fence from ``` to ```text (e.g., change the
code block containing "[status badge] ... [action buttons]" to start with
```text) and keep the closing ``` unchanged so the block is correctly labeled as
plain text.
---
Outside diff comments:
In `@apps/citizen-pwa/src/components/RevealSheet.tsx`:
- Around line 169-173: In RevealSheet's onKeyDown handler, the Enter/Space key
handling currently only calls onClose when state === 'success', causing
inconsistent keyboard vs backdrop click behavior; update the handler in the
RevealSheet component to call onClose for Enter or Space without the state ===
'success' guard (or mirror the same condition used for backdrop click) so
keyboard users can close the sheet the same way as pointer users.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d996ae2f-2f52-48ae-ad17-1455bd4f9226
📒 Files selected for processing (48)
.gitignoreDESIGN.jsonDESIGN.mdPRODUCT.mdapps/citizen-pwa/src/App.routes.test.tsxapps/citizen-pwa/src/components/AlertsTab.test.tsxapps/citizen-pwa/src/components/AlertsTab.tsxapps/citizen-pwa/src/components/CitizenShell.test.tsxapps/citizen-pwa/src/components/CitizenShell.tsxapps/citizen-pwa/src/components/FeedTab.test.tsxapps/citizen-pwa/src/components/FeedTab.tsxapps/citizen-pwa/src/components/LookupScreen.test.tsxapps/citizen-pwa/src/components/LookupScreen.tsxapps/citizen-pwa/src/components/ProfileTab.test.tsxapps/citizen-pwa/src/components/ProfileTab.tsxapps/citizen-pwa/src/components/RevealSheet.test.tsxapps/citizen-pwa/src/components/RevealSheet.tsxapps/citizen-pwa/src/components/SubmitReportForm/Step1Evidence.tsxapps/citizen-pwa/src/components/SubmitReportForm/Step2WhoWhere.tsxapps/citizen-pwa/src/components/Toast.test.tsxapps/citizen-pwa/src/components/Toast.tsxapps/citizen-pwa/src/components/Toggle.test.tsxapps/citizen-pwa/src/components/Toggle.tsxapps/citizen-pwa/src/hooks/useAlerts.tsapps/citizen-pwa/src/hooks/useIncident.tsapps/citizen-pwa/src/hooks/useOfflineQueueCount.test.tsapps/citizen-pwa/src/hooks/useOfflineQueueCount.tsapps/citizen-pwa/src/hooks/useReducedMotion.test.tsapps/citizen-pwa/src/hooks/useReducedMotion.tsapps/citizen-pwa/src/hooks/useToast.tsapps/citizen-pwa/src/pages/RegisterPage.test.tsxapps/citizen-pwa/src/pages/RegisterPage.tsxapps/citizen-pwa/src/pages/SettingsPage.test.tsxapps/citizen-pwa/src/pages/SettingsPage.tsxapps/citizen-pwa/src/routes.tsxapps/citizen-pwa/src/services/callables.test.tsapps/citizen-pwa/src/services/callables.tsapps/citizen-pwa/src/styles/globals.cssapps/citizen-pwa/src/utils/incident-meta.test.tsxapps/citizen-pwa/src/utils/incident-meta.tsxdocs/learnings.mddocs/progress.mddocs/superpowers/plans/2026-04-30-citizen-pwa-redesign.mddocs/superpowers/plans/2026-04-30-responder-app-redesign.mddocs/superpowers/specs/2026-04-29-phase9-design.mddocs/superpowers/specs/2026-04-30-citizen-pwa-redesign-design.mddocs/superpowers/specs/2026-04-30-responder-app-redesign-design.mdpackages/shared-firebase/src/firestore.ts
Comment on lines
+348
to
+351
| ```ts | ||
| { path: '/register', element: <RegisterPage /> }, | ||
| { path: '/settings', element: <SettingsPage /> }, | ||
| ``` |
Contributor
There was a problem hiding this comment.
Fix fenced block spacing to satisfy markdownlint (MD031).
Add blank lines before and after the fenced code block under “Add routes”.
Suggested fix
- Add routes:
+
```ts
{ path: '/register', element: <RegisterPage /> },
{ path: '/settings', element: <SettingsPage /> },</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 348-348: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @docs/superpowers/plans/2026-04-30-citizen-pwa-redesign.md around lines 348 -
351, The fenced TypeScript code block containing the route entries ({ path:
'/register', element: }, { path: '/settings', element:
}) needs a blank line before the opening ts fence and a blank line after the closing fence to satisfy markdownlint MD031; update the
“Add routes” section so there is an empty line immediately above the ts and an empty line immediately below the closing to fix the spacing.
</details>
<!-- fingerprinting:phantom:poseidon:ocelot:5493cb93-2508-4a15-96fa-61e0b2f07ff8 -->
<!-- d98c2f50 -->
<!-- This is an auto-generated comment by CodeRabbit -->
Comment on lines
+821
to
+840
| const onPointerDown = useCallback(() => { | ||
| cancelledRef.current = false | ||
| startRef.current = Date.now() | ||
| setPhase('holding') | ||
| setProgress(0) | ||
|
|
||
| intervalRef.current = setInterval(() => { | ||
| if (cancelledRef.current) return | ||
| const elapsed = Date.now() - startRef.current | ||
| const p = Math.min(elapsed / HOLD_MS, 1) | ||
| setProgress(p) | ||
|
|
||
| if (p >= 1) { | ||
| clearInterval(intervalRef.current) | ||
| setPhase('activated') | ||
| fire([30, 50, 30, 50, 50]) | ||
| void onActivate() | ||
| } | ||
| }, TICK_MS) | ||
| }, [onActivate, fire]) |
Contributor
There was a problem hiding this comment.
Add re-entry guard and timer cleanup in useSosHold guidance.
Repeated onPointerDown can start overlapping intervals, and the snippet lacks unmount cleanup. That can cause duplicate activations and leaked timers.
Proposed snippet adjustment
export function useSosHold(onActivate: () => void | Promise<void>) {
@@
const onPointerDown = useCallback(() => {
+ if (phase === 'holding') return
+ if (intervalRef.current) clearInterval(intervalRef.current)
@@
intervalRef.current = setInterval(() => {
@@
if (p >= 1) {
- clearInterval(intervalRef.current)
+ if (intervalRef.current) clearInterval(intervalRef.current)
+ intervalRef.current = undefined
setPhase('activated')
fire([30, 50, 30, 50, 50])
void onActivate()
}
}, TICK_MS)
- }, [onActivate, fire])
+ }, [onActivate, fire, phase])
@@
const onPointerUp = useCallback(() => {
@@
cancelledRef.current = true
- clearInterval(intervalRef.current)
+ if (intervalRef.current) clearInterval(intervalRef.current)
+ intervalRef.current = undefined
setPhase('idle')
setProgress(0)
}
}, [])
+
+ useEffect(() => {
+ return () => {
+ if (intervalRef.current) clearInterval(intervalRef.current)
+ }
+ }, [])Also applies to: 842-850
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/superpowers/plans/2026-04-30-responder-app-redesign.md` around lines 821
- 840, The onPointerDown handler in useSosHold can start overlapping intervals
and never clears timers on unmount; add a re-entry guard at the top of
onPointerDown (e.g., if (intervalRef.current) return or check phase/holding
flag) and clear any existing interval before creating a new one
(clearInterval(intervalRef.current)); also ensure the hook returns a cleanup
effect that clears intervalRef.current and sets cancelledRef.current = true on
unmount to prevent leaked timers and duplicate activations; update references to
intervalRef, cancelledRef, startRef, onPointerDown and the hook's useEffect
cleanup accordingly.
Functional fixes: - RevealSheet: keyboard close consistent with click, charInterval leak fixed, clipboard error handling added - useIncident: added power_outage type, verifiedAt validation, cancellation - useOfflineQueueCount: isMounted guard + try/catch - useToast: useEffect cleanup for timer leak - RegisterPage: guard auth() with try/catch - SettingsPage: log localStorage errors, catch signOut, guard navigator.storage - AlertsTab: display error state from useAlerts - CitizenShell: remove double padding when banner shown - LookupScreen: secret input type=password with autocomplete off - Test files: afterEach cleanup for fake timers, filter chip guard Refs #85
- DESIGN.json: update .ds-card-status font-size 0.625rem → 0.75rem - citizen-pwa-redesign.md: add blank lines around fenced code block (MD031) - responder-app-redesign.md: fix heading level jump # → ## (MD001) - responder-app-redesign-design.md: add text language tag to code block (MD040) Refs #85
This was referenced May 1, 2026
Merged
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
Design rules honoured
Test plan
Summary by CodeRabbit
New Features
Improvements
Documentation