Panel P4.2: live monitor — SSE over pg LISTEN/NOTIFY, snapshot endpoint, tablet monitor, LiveStrip upgrade - #81
Merged
Merged
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…NOTIFY transport Adds internal/broker: the Broker seam (Publish/Subscribe with 1-buffered drop-if-full delivery and idempotent unsubscribe), MemBroker as a pure in-process fanout used by handler tests, and PGBroker which wraps a MemBroker and bridges cross-replica delivery via a dedicated LISTEN pgx.Conn plus a small (MaxConns 2) notify pgxpool, with reconnect-on-error backoff (1s doubling to 30s). Lays the foundation for the P4.2 live monitor SSE endpoint (Task 4).
Add the store-layer aggregations backing the P4.2 live monitor snapshot:
GetMonitorCounts (total/checked-in via one COUNT(*) FILTER query),
GetMonitorZones (per-zone checked-in counts + unattributed via one
DISTINCT ON + UNION ALL statement, so sum(zones)+unattributed==checkedIn
holds by construction), GetMonitorMinuteBuckets (date_trunc('minute', ...)
buckets shared by the rate/peak computation), and GetMonitorStations
(per-station checkin counts). Zero migrations — reads existing tables only.
pgxmock tests assert the exact SQL text; a TEST_DATABASE_URL-gated
integration test proves the DISTINCT ON tie-breaker and the invariant
against real Postgres, since pgxmock can only echo canned rows.
GET /api/events/{event_id}/monitor composes Task 2's four store
aggregations (GetMonitorCounts/Zones/MinuteBuckets/Stations) plus the
existing GetCheckinActions feed into the spec §3.1 snapshot response:
totals (checked_in/total/rate_per_min/peak/est_done_at), zones[],
unattributed, stations[], recent[].
Rate/peak/ETA math lives in a pure computeRates(buckets, now, total,
checkedIn) in monitor_rates.go, unit-tested without HTTP: 5-minute
sliding window for rate_per_min, today's max bucket for peak, and
est_done_at nulled out when rate < 0.1/min or the event is already
fully checked in.
openapi.yaml gains MonitorSnapshot + sub-schemas (required fields,
nullable peak/est_done_at, additionalProperties: false) and the new
path; panel/src/shared/api/schema.d.ts regenerated to match.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds the codebase's first Server-Sent Events endpoint, GET
/api/events/{event_id}/monitor/stream, plus the four broker publish call
sites (check-in, undo, reprint, station heartbeat) that feed it.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- useMonitorSnapshot(eventId) + MONITOR_SNAPSHOT_KEY(eventId) over
GET /api/events/{event_id}/monitor, mirroring READINESS_KEY's
[method, path, init] query-key discipline (no refetchInterval — Task 6's
useMonitorStream keeps it fresh via SSE-driven invalidation).
- createSseParser: pure, incremental event:/data: frame parser split on
"\n\n", buffering across chunk boundaries, ignoring comment lines
(": ping"), defaulting event to "message" for data-only frames.
- getApiBaseUrl (http.ts:13) was already exported — no change needed there.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds useMonitorStream (P4.2 Task 6): fetch-streams GET
/api/events/{eventId}/monitor/stream, feeding decoded chunks into Task 5's
createSseParser. hello -> "live"; update frames invalidate
MONITOR_SNAPSHOT_KEY coalesced to <=1/sec (trailing-edge, one invalidation
per burst); any disconnect -> "reconnecting" with 1s/x2/30s-cap +-25%-jitter
backoff, and a successful reconnect immediately re-invalidates the snapshot
(resync guarantee) ahead of the new connection's own hello. AbortController
resets fully on unmount and on eventId change. No polling fallback, no new
dependencies.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fills MonitorPage's right-column placeholders (Task 7) with the Stations card (dot + name + count, amber "stale Ns" text label alongside the dot per board 7e -- never color alone) and the read-only Last-scans card (verdictClasses.allowed for checkin rows only; neutral muted icons for undo/reprint; derived zone name; mono HH:MM:SS; no action buttons). Adds the header's amber reconnecting badge over stale snapshot data, plus a data-testid on the LIVE ring so its live-branch is finally exercised by a test (carried over from Task 7's review). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… breakdown RunningCard now sources counters/progress from useMonitorSnapshot (kept fresh by useMonitorStream's SSE-driven invalidation) instead of the old useEventStats(poll:true) 15s poller, and drops the dead stats.data?.zone_stats read (always undefined -- that field is the unrelated P2 per-verdict breakdown, never populated without a ?zone= param). Adds an "Open monitor" CTA beside the existing "Open event" link (board 1c/1d precedent) and a compact per-zone mini-line (name + count, unattributed only when > 0, reusing monitorUnattributed). UpcomingCard is untouched.
Task 10 (final): i18n sweep (16 monitor* keys, EN/RU parity, real Russian, no hardcoded strings), full gates (panel typecheck/test/lint/ build, packages/ui tests, generate:api zero drift, backend go test + golangci-lint + go test -race on broker/handler), cross-checks against main (router.tsx = only eventMonitorRoute added, backend diff scoped to broker+monitor+four publish sites+main.go+openapi.yaml, web/ untouched, zero new deps, zero migrations), and a spec walk mapping every section (§3.1-§3.3, §4.1-§4.3, §5, §6) to its implementing task and named test. No fixes were required. Marks all 27 plan checkboxes complete.
github-actions
Bot
added
documentation
Owner
Author
|
@codex review |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a live event monitor with PostgreSQL-backed broker fanout, snapshot aggregation, authenticated SSE updates, publish hooks across check-in flows, and a panel monitor page with station/recent-scan views and LiveStrip integration. ChangesLive monitor feature
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a231b8354
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![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: 18
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (5)
backend/internal/handler/monitor_stream_test.go-117-117 (1)
117-117: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winUse a bounded HTTP client.
http.Gethas no timeout, so a failure before response headers can hang the test suite indefinitely. Use a sharedsrv.Client()with a bounded timeout or response-header timeout.As per coding guidelines, all external calls must have timeouts.
Also applies to: 156-156, 195-195, 228-228, 263-263
🤖 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 `@backend/internal/handler/monitor_stream_test.go` at line 117, Replace the unbounded http.Get calls in the monitor stream tests with a shared srv.Client() configured with a bounded timeout or response-header timeout, including the additional call sites. Preserve the existing request and response assertions while ensuring every external HTTP call cannot hang indefinitely.Source: Coding guidelines
docs/superpowers/specs/2026-07-18-panel-p4.2-live-monitor-design.md-63-76 (1)
63-76: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winSpecify a language for the fenced response example.
Use
jsoncbecause the example contains comments.-``` +```jsonc {🤖 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 `@docs/superpowers/specs/2026-07-18-panel-p4.2-live-monitor-design.md` around lines 63 - 76, Update the fenced response example near the totals object to declare the jsonc language, preserving its existing commented JSON content.Source: Linters/SAST tools
panel/src/features/monitor/RecentFeedCard.tsx-74-100 (1)
74-100: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExpose the action type to screen readers.
The only check-in/undo/reprint cue is an
aria-hiddenicon. Add an i18n-backed visually hidden label or rowaria-label, with coverage for each action.As per coding guidelines, every user-facing string must use a
react-i18nextkey with English and Russian entries.🤖 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 `@panel/src/features/monitor/RecentFeedCard.tsx` around lines 74 - 100, Add an i18n-backed accessible action label to each row rendered by the recent.map callback, covering checkin, undo, and reprint actions in both English and Russian translation resources. Apply the label via a visually hidden element or the row’s aria-label while keeping the decorative Icon aria-hidden, and use the existing react-i18next translation mechanism.Source: Coding guidelines
docs/superpowers/plans/2026-07-18-panel-p4.2-live-monitor.md-48-48 (1)
48-48: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the malformed Markdown code spans.
map[uuid][]chanis parsed as a missing reference link, while the nested backticks aroundfetch(...)terminate the span early.Proposed fix
-- Produces: `NewMemBroker() *MemBroker` — pure in-process fanout (map[uuid][]chan + mutex). +- Produces: `NewMemBroker() *MemBroker` — pure in-process fanout (`map[uuid.UUID][]chan struct{}` + mutex). -- Produces: `useMonitorStream(...)` — on mount: `fetch(`${base}/api/events/${eventId}/monitor/stream`, ...)` +- Produces: `useMonitorStream(...)` — on mount: ``fetch(`${base}/api/events/${eventId}/monitor/stream`, ...)``Also applies to: 133-133
🤖 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 `@docs/superpowers/plans/2026-07-18-panel-p4.2-live-monitor.md` at line 48, Fix the Markdown inline-code formatting in the NewMemBroker description and the corresponding occurrence at the later referenced section. Ensure map[uuid][]chan and fetch(...) each remain inside valid code spans without nested or malformed backticks.Source: Linters/SAST tools
panel/src/features/monitor/TotalsCard.tsx-43-55 (1)
43-55: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winLocalize monitor numbers consistently.
toFixed(1)hardcodes a decimal point, while LiveStrip emits ungrouped JavaScript numbers. Use locale-awareIntl.NumberFormatinstances for rates, totals, zone counts, and unattributed counts.
panel/src/features/monitor/TotalsCard.tsx#L43-L55: format rates with exactly one localized fractional digit.panel/src/features/home/LiveStrip.tsx#L104-L130: format all snapshot counts usingi18n.language.🤖 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 `@panel/src/features/monitor/TotalsCard.tsx` around lines 43 - 55, Localize monitor numeric formatting consistently: in panel/src/features/monitor/TotalsCard.tsx lines 43-55, use an Intl.NumberFormat configured with i18n.language and exactly one fractional digit for rate and peak-rate values, replacing toFixed(1); in panel/src/features/home/LiveStrip.tsx lines 104-130, format totals, zone counts, and unattributed counts with locale-aware Intl.NumberFormat using i18n.language.
🧹 Nitpick comments (2)
backend/internal/broker/broker_test.go (1)
14-289: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRun the new independent tests and table subtests in parallel.
backend/internal/broker/broker_test.go#L14-L289: addt.Parallel()to independent tests.backend/internal/handler/attendee_printed_publish_test.go#L23-L114: parallelize the tests and table subtests.backend/internal/handler/checkin_publish_test.go#L39-L244: parallelize the tests and table subtests.As per coding guidelines, Go tests must use “table-driven patterns and parallel execution.”
🤖 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 `@backend/internal/broker/broker_test.go` around lines 14 - 289, Parallelize each independent test in backend/internal/broker/broker_test.go (lines 14-289), backend/internal/handler/attendee_printed_publish_test.go (lines 23-114), and backend/internal/handler/checkin_publish_test.go (lines 39-244) by adding t.Parallel() at the test start; also add t.Parallel() within every table-test subtest closure while preserving safe per-test state and existing table-driven coverage.Source: Coding guidelines
backend/internal/store/pg_store_monitor_test.go (1)
20-316: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftApply the repository’s table-driven and parallel-test convention throughout the new unit suites.
backend/internal/store/pg_store_monitor_test.go#L20-L316: group populated/empty scenarios by Store method and run safe cases in parallel.backend/internal/handler/checkin_stations_publish_test.go#L20-L106: combine the success, not-found, and nil-broker cases into parallel table-driven subtests.As per coding guidelines, “Write unit tests using table-driven patterns and parallel execution.”
🤖 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 `@backend/internal/store/pg_store_monitor_test.go` around lines 20 - 316, Refactor the tests in backend/internal/store/pg_store_monitor_test.go lines 20-316 into table-driven, parallel subtests grouped by GetMonitorCounts, GetMonitorZones, GetMonitorMinuteBuckets, and GetMonitorStations, preserving each scenario’s expectations and assertions. Refactor backend/internal/handler/checkin_stations_publish_test.go lines 20-106 similarly into one parallel table-driven suite covering success, not-found, and nil-broker cases; each site requires direct changes.Source: Coding guidelines
🤖 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 `@backend/internal/broker/pg_broker.go`:
- Around line 192-199: After the successful re-LISTEN path in the broker
connection flow, enqueue one coalesced resynchronization signal for every
currently active MemBroker subscription before returning the connection. Reuse
the existing subscription registry and notification mechanism, ensuring
connected monitors are prompted to refetch state without emitting duplicate
signals per subscription.
- Around line 60-84: Update NewPGBroker, Publish, and the related PostgreSQL
operations to use short per-attempt context deadlines, bounded retries with
exponential backoff where retrying is safe, and cancellation-aware waits. Wrap
every returned database error with contextual fmt.Errorf messages using %w,
while preserving cleanup and avoiding retries for non-retryable operations.
In `@backend/internal/handler/checkin_stations.go`:
- Around line 126-129: Bound both newly introduced PostgreSQL broker calls: in
backend/internal/handler/checkin_stations.go lines 126-129, use a short-lived
context with a publish deadline for Broker.Publish so heartbeat responses are
not blocked; in backend/main.go lines 53-61, replace the unbounded
context.Background() used for broker initialization with a bounded startup
context. Preserve the existing publish error logging and initialization
behavior.
- Around line 120-130: Throttle heartbeat-triggered publishes in the handler’s
Broker.Publish path by coalescing notifications per event, using a cadence
aligned with the 45-second liveness threshold instead of publishing every
heartbeat. Preserve nil-safe, best-effort publishing and retain immediate
notifications for non-heartbeat event flows.
In `@backend/internal/handler/monitor_rates_test.go`:
- Around line 10-231: Refactor the new test suites into table-driven, parallel
tests: in backend/internal/handler/monitor_rates_test.go lines 10-231,
consolidate the rate, peak, and ETA scenarios and call t.Parallel for
independent cases; in
backend/internal/handler/openapi_contract_monitor_p4_test.go lines 29-225,
table-drive the seeded, empty, and foreign-event scenarios and parallelize them;
in backend/internal/handler/monitor_stream_test.go lines 108-353, inject the
ping interval per test case and parallelize independent stream scenarios.
In `@backend/internal/handler/monitor_rates.go`:
- Around line 44-59: The computeRates function incorrectly derives a sliding
five-minute window from minute-start bucket timestamps, excluding partially
overlapping buckets. Replace this calculation with an exact created_at >=
now.Add(-rateWindow) count query, or explicitly align the contract to minute
buckets; preserve peak-rate handling and update callers as needed. Add a
regression test for a non-minute-aligned now value, such as 12:00:30, verifying
the five-minute rate includes events from the overlapping minute.
In `@backend/internal/handler/monitor_stream.go`:
- Around line 54-62: Update the monitor stream handler to reject requests when
h.Broker is nil, returning HTTP 503 before writing stream headers instead of
serving hello and keep-alive frames; remove the nil-safe subscription fallback
and invert the broker-availability condition. Update the corresponding OpenAPI
responses to document 503 for an unavailable broker.
In `@backend/internal/handler/monitor.go`:
- Around line 78-86: Update the monitor aggregation flow around GetMonitorCounts
and GetMonitorZones to retrieve totals, zone counts, and unattributed counts
through one transaction-backed store operation and database snapshot. Preserve
the existing error response behavior while ensuring all returned aggregates
reflect the same point-in-time data.
- Around line 79-105: Update each error branch in the monitor handler around
GetMonitorZones, GetMonitorMinuteBuckets, GetMonitorStations, and
GetCheckinActions to wrap the underlying err with fmt.Errorf("context: %w", err)
and pass it through the existing common logging/tracing path before returning
the current sanitized JSON response. Preserve the existing HTTP status and
response messages.
In `@backend/internal/store/interface.go`:
- Around line 246-273: Make the monitor snapshot totals and zone attribution
atomic by replacing the separate GetMonitorCounts and GetMonitorZones calls with
one method/query, or by executing both aggregations within a consistent
transaction snapshot. Preserve the existing count and zone-attribution semantics
while ensuring totals.checked_in always equals sum(zones[].CheckedIn) plus
unattributed.
In `@backend/internal/store/pg_store_monitor.go`:
- Around line 16-24: Update GetMonitorCounts and the other monitor methods at
the referenced ranges to wrap every database query, scan, and iteration error
with operation-specific context using fmt.Errorf and %w; preserve existing
return values and control flow while ensuring each failure identifies the
monitor operation that failed.
- Around line 16-24: Add a tracing span at the start of each of
GetMonitorCounts, GetMonitorZones, GetMonitorMinuteBuckets, and
GetMonitorStations using the incoming ctx, record any query error on the span,
and ensure the span ends on every return path. Apply the same instrumentation
consistently to all four monitor query methods without changing their existing
results or error behavior.
- Around line 51-62: The latest_checkin CTE must select the most recent
state-changing action for each attendee, including both checkin and undo,
ordered by created_at and id, and the attributed CTE must join a station only
when that latest action is checkin. Update the associated real-Postgres
regression fixture to cover an undo followed by a no-action check-in path and
verify that no obsolete station is attributed.
In `@backend/openapi.yaml`:
- Around line 2309-2315: Update the stream documentation in the event contract
so every hello event, including reconnects, requires the client to refetch or
invalidate the snapshot endpoint before relying on subsequent updates. Clarify
that hello is both the connection confirmation and a snapshot resynchronization
trigger, while preserving the existing update-event behavior.
In `@docs/superpowers/specs/2026-07-18-panel-p4.2-live-monitor-design.md`:
- Around line 109-112: Update the monitor publication design to cover every
monitor-visible check-in mutation, including ApplyBatchCheckin and legacy/sync
check-in update paths, with publication occurring after transaction commit and
remaining log-don't-fail. Alternatively, route all check-in mutations through
one publishing service, while preserving publication for the existing
CheckInAttendee, UndoCheckin, reprint InsertCheckinAction, and
HeartbeatCheckinStation paths.
In `@panel/src/features/monitor/MonitorPage.tsx`:
- Around line 67-73: Replace the cross-surface translation keys with
surface-owned keys: update MonitorPage’s workspaceLoadError and
workspaceBackHome usages to monitor-prefixed keys, and update LiveStrip’s
monitorUnattributed usage to a home-prefixed key. Add matching English entries
in panel/src/shared/i18n/en.json lines 622-638 and Russian entries in
panel/src/shared/i18n/ru.json lines 624-640, preserving equivalent meanings and
translations at each affected site.
- Around line 136-160: Use cached query data as the rendering guard so
background refetch errors do not replace valid content with error states: update
MonitorPage.tsx lines 67-82 and 136-160 to render existing
eventQuery.data/snapshotQuery.data when available, update LiveStrip.tsx lines
99-134 to keep the running strip visible after snapshot refetch failure, and add
success-then-failed-refetch regression coverage in MonitorPage.test.tsx lines
314-338 and LiveStrip.test.tsx lines 176-195.
In `@panel/src/features/monitor/useMonitorStream.ts`:
- Around line 33-43: Update the monitor stream flow around the raw fetch,
reconnect handling, and response processing to use a thin shared API-layer
wrapper instead of calling fetch directly from the feature. Retry only network
failures and 5xx responses; surface documented 400, 403, and 404 responses as
terminal errors without reconnecting. Add coverage for these terminal 4xx cases
while preserving SSE parsing and cleanup behavior.
---
Minor comments:
In `@backend/internal/handler/monitor_stream_test.go`:
- Line 117: Replace the unbounded http.Get calls in the monitor stream tests
with a shared srv.Client() configured with a bounded timeout or response-header
timeout, including the additional call sites. Preserve the existing request and
response assertions while ensuring every external HTTP call cannot hang
indefinitely.
In `@docs/superpowers/plans/2026-07-18-panel-p4.2-live-monitor.md`:
- Line 48: Fix the Markdown inline-code formatting in the NewMemBroker
description and the corresponding occurrence at the later referenced section.
Ensure map[uuid][]chan and fetch(...) each remain inside valid code spans
without nested or malformed backticks.
In `@docs/superpowers/specs/2026-07-18-panel-p4.2-live-monitor-design.md`:
- Around line 63-76: Update the fenced response example near the totals object
to declare the jsonc language, preserving its existing commented JSON content.
In `@panel/src/features/monitor/RecentFeedCard.tsx`:
- Around line 74-100: Add an i18n-backed accessible action label to each row
rendered by the recent.map callback, covering checkin, undo, and reprint actions
in both English and Russian translation resources. Apply the label via a
visually hidden element or the row’s aria-label while keeping the decorative
Icon aria-hidden, and use the existing react-i18next translation mechanism.
In `@panel/src/features/monitor/TotalsCard.tsx`:
- Around line 43-55: Localize monitor numeric formatting consistently: in
panel/src/features/monitor/TotalsCard.tsx lines 43-55, use an Intl.NumberFormat
configured with i18n.language and exactly one fractional digit for rate and
peak-rate values, replacing toFixed(1); in panel/src/features/home/LiveStrip.tsx
lines 104-130, format totals, zone counts, and unattributed counts with
locale-aware Intl.NumberFormat using i18n.language.
---
Nitpick comments:
In `@backend/internal/broker/broker_test.go`:
- Around line 14-289: Parallelize each independent test in
backend/internal/broker/broker_test.go (lines 14-289),
backend/internal/handler/attendee_printed_publish_test.go (lines 23-114), and
backend/internal/handler/checkin_publish_test.go (lines 39-244) by adding
t.Parallel() at the test start; also add t.Parallel() within every table-test
subtest closure while preserving safe per-test state and existing table-driven
coverage.
In `@backend/internal/store/pg_store_monitor_test.go`:
- Around line 20-316: Refactor the tests in
backend/internal/store/pg_store_monitor_test.go lines 20-316 into table-driven,
parallel subtests grouped by GetMonitorCounts, GetMonitorZones,
GetMonitorMinuteBuckets, and GetMonitorStations, preserving each scenario’s
expectations and assertions. Refactor
backend/internal/handler/checkin_stations_publish_test.go lines 20-106 similarly
into one parallel table-driven suite covering success, not-found, and nil-broker
cases; each site requires direct changes.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 067ef22d-b357-4fd8-8a78-92302885d871
📒 Files selected for processing (46)
backend/internal/broker/broker.gobackend/internal/broker/broker_test.gobackend/internal/broker/mem_broker.gobackend/internal/broker/pg_broker.gobackend/internal/handler/attendee_printed.gobackend/internal/handler/attendee_printed_publish_test.gobackend/internal/handler/checkin.gobackend/internal/handler/checkin_publish_test.gobackend/internal/handler/checkin_stations.gobackend/internal/handler/checkin_stations_publish_test.gobackend/internal/handler/handler.gobackend/internal/handler/monitor.gobackend/internal/handler/monitor_rates.gobackend/internal/handler/monitor_rates_test.gobackend/internal/handler/monitor_stream.gobackend/internal/handler/monitor_stream_test.gobackend/internal/handler/openapi_contract_monitor_p4_test.gobackend/internal/handler/testsupport_test.gobackend/internal/store/interface.gobackend/internal/store/pg_store_monitor.gobackend/internal/store/pg_store_monitor_integration_test.gobackend/internal/store/pg_store_monitor_test.gobackend/main.gobackend/openapi.yamldocs/superpowers/plans/2026-07-18-panel-p4.2-live-monitor.mddocs/superpowers/specs/2026-07-18-panel-p4.2-live-monitor-design.mdpanel/src/app/router.tsxpanel/src/features/home/LiveStrip.test.tsxpanel/src/features/home/LiveStrip.tsxpanel/src/features/monitor/MonitorPage.test.tsxpanel/src/features/monitor/MonitorPage.tsxpanel/src/features/monitor/RecentFeedCard.tsxpanel/src/features/monitor/StationsCard.tsxpanel/src/features/monitor/TotalsCard.tsxpanel/src/features/monitor/ZonesCard.tsxpanel/src/features/monitor/hooks.test.tsxpanel/src/features/monitor/hooks.tspanel/src/features/monitor/liveness.test.tspanel/src/features/monitor/liveness.tspanel/src/features/monitor/parseSse.test.tspanel/src/features/monitor/parseSse.tspanel/src/features/monitor/useMonitorStream.test.tsxpanel/src/features/monitor/useMonitorStream.tspanel/src/shared/api/schema.d.tspanel/src/shared/i18n/en.jsonpanel/src/shared/i18n/ru.json
…-aware zone attribution, exact rate window Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…d publishes, legacy-path publishes, fail-closed stream, heartbeat throttle Wave B (broker/stream/publish layer) of the PR #81 bot-review round: - B1: PGBroker fans out MemBroker.BroadcastAll() to every local subscriber after a successful LISTEN reconnect, so NOTIFYs lost during the connection gap no longer leave monitors stale indefinitely. - B2: publishCheckinEvent is now the one shared, nil-safe publish helper — detaches from the caller's request context (context.WithoutCancel) and bounds the result with a 2s timeout, closing both the client-disconnect and stalled-Postgres failure modes at once; main.go's broker startup connect is now bounded by a 10s timeout too. - B3: the three legacy check-in write paths (PUT /api/attendees/{id}, the mobile batch endpoint, offline sync push) now publish on success, which they never did before. - B4: GetEventMonitorStream fails closed with a 503 when no Broker is configured, instead of silently serving a stream with no updates ever. - B5: heartbeat-sourced publishes are throttled to at most one per 15s per event; check-in/undo/reprint publishes stay unthrottled. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Created attendees change monitor-visible state (total count, possibly checked_in if an offline kiosk created-and-checked-in in one push), so their event IDs should be added to affectedEvents and trigger publishes just like Updated attendees. Sync push with ONLY creates now publishes once per distinct affected event. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…/backoff/resync hardening, retain-stale-data, surface-owned i18n Findings C1-C7 from the CodeRabbit + Codex bot-review round on PR #81 (panel P4.2 live-monitor wave, last of three sequential waves): rebuild the monitor header's LIVE/reconnecting badges (and LiveStrip's LIVE NOW badge) on @idento/ui's StatusPill (extended with an additive dot/pulse indicator); strip a trailing slash off getApiBaseUrl so useMonitorStream's raw fetch never double-slashes; give useMonitorStream a terminal "error" status for non-OK 4xx responses (routed through the app's global auth/tenant-suspension handling via a newly-extracted shared/api/handleApiError.ts), reset its backoff attempt counter only once a hello frame actually arrives, and resync the snapshot on every hello (not just a reconnect's); gate MonitorPage/LiveStrip's error cards on missing data rather than isError so a failed background refetch no longer blanks already-good content; and give the monitor page and LiveStrip their own i18n keys instead of borrowing another surface's. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Owner
Author
|
@codex review |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
backend/internal/handler/monitor_rates.go (1)
55-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the rate divisor from
rateWindowto avoid a silent decoupling.
ratePerMinhardcodes/ 5.0whilerateWindowis independently defined as5 * time.Minute. These two must stay in lockstep — the caller counts check-ins overnow-rateWindow, then this divides by a literal5.0. IfrateWindowis ever retuned, the rate silently becomes wrong with no compile-time signal. Bind the divisor to the same constant.♻️ Proposed fix
- ratePerMin = roundToOneDecimal(float64(recentCount) / 5.0) + ratePerMin = roundToOneDecimal(float64(recentCount) / rateWindow.Minutes())🤖 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 `@backend/internal/handler/monitor_rates.go` around lines 55 - 56, Update computeRates so ratePerMin derives its divisor from the shared rateWindow constant rather than the hardcoded 5.0; convert rateWindow to minutes as needed while preserving the existing rounded per-minute rate behavior.backend/internal/handler/event_publish.go (1)
48-60: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAdd
eventIDto the failure log and consider a tracing span for this external-call boundary.The failure log on line 58 omits
eventID, making it hard to correlate a failed publish with the mutation that triggered it. This is also the one shared call site for every monitor-visible mutation's broker publish, and the coding guidelines call for both JSON-formatted, correlatable logs and spans around external-call boundaries — neither is present here.🩹 Proposed fix for the log line
if err := h.Broker.Publish(pubCtx, eventID); err != nil { - log.Printf("publish checkin event: broker publish failed: %v", err) + log.Printf("publish checkin event: broker publish failed: event_id=%s err=%v", eventID, err) }As per coding guidelines, "Include unique request IDs and trace context in all logs for correlation" and "Annotate slow, critical, or error-prone paths with custom spans."
🤖 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 `@backend/internal/handler/event_publish.go` around lines 48 - 60, Update publishCheckinEvent to include eventID in a structured JSON log when Broker.Publish fails, preserving the existing error context. Add a tracing span around the external Broker.Publish call, using the existing context and tracing conventions, and ensure the span records the publish error.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@backend/internal/handler/event_publish.go`:
- Around line 48-60: Update publishCheckinEvent to include eventID in a
structured JSON log when Broker.Publish fails, preserving the existing error
context. Add a tracing span around the external Broker.Publish call, using the
existing context and tracing conventions, and ensure the span records the
publish error.
In `@backend/internal/handler/monitor_rates.go`:
- Around line 55-56: Update computeRates so ratePerMin derives its divisor from
the shared rateWindow constant rather than the hardcoded 5.0; convert rateWindow
to minutes as needed while preserving the existing rounded per-minute rate
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 177f13b4-a597-4633-8b6a-90248092583d
📒 Files selected for processing (42)
backend/internal/broker/broker_test.gobackend/internal/broker/mem_broker.gobackend/internal/broker/pg_broker.gobackend/internal/handler/attendee_printed.gobackend/internal/handler/attendees.gobackend/internal/handler/checkin.gobackend/internal/handler/checkin_stations.gobackend/internal/handler/checkin_stations_publish_test.gobackend/internal/handler/checkins_batch.gobackend/internal/handler/event_publish.gobackend/internal/handler/event_publish_test.gobackend/internal/handler/handler.gobackend/internal/handler/legacy_publish_test.gobackend/internal/handler/monitor.gobackend/internal/handler/monitor_rates.gobackend/internal/handler/monitor_rates_test.gobackend/internal/handler/monitor_stream.gobackend/internal/handler/monitor_stream_test.gobackend/internal/handler/openapi_contract_monitor_p4_test.gobackend/internal/handler/sync.gobackend/internal/handler/testsupport_test.gobackend/internal/store/interface.gobackend/internal/store/pg_store_monitor.gobackend/internal/store/pg_store_monitor_integration_test.gobackend/internal/store/pg_store_monitor_test.gobackend/main.gobackend/openapi.yamlpackages/ui/src/components/status-pill.test.tsxpackages/ui/src/components/status-pill.tsxpanel/src/app/queryClient.tspanel/src/features/home/LiveStrip.test.tsxpanel/src/features/home/LiveStrip.tsxpanel/src/features/monitor/MonitorPage.test.tsxpanel/src/features/monitor/MonitorPage.tsxpanel/src/features/monitor/useMonitorStream.test.tsxpanel/src/features/monitor/useMonitorStream.tspanel/src/shared/api/handleApiError.tspanel/src/shared/api/http.test.tspanel/src/shared/api/http.tspanel/src/shared/api/schema.d.tspanel/src/shared/i18n/en.jsonpanel/src/shared/i18n/ru.json
🚧 Files skipped from review as they are similar to previous changes (15)
- backend/main.go
- backend/internal/handler/attendee_printed.go
- backend/openapi.yaml
- backend/internal/handler/openapi_contract_monitor_p4_test.go
- panel/src/shared/api/schema.d.ts
- panel/src/features/home/LiveStrip.tsx
- panel/src/shared/i18n/ru.json
- panel/src/shared/i18n/en.json
- backend/internal/handler/monitor.go
- backend/internal/handler/monitor_stream.go
- backend/internal/handler/handler.go
- panel/src/features/monitor/MonitorPage.tsx
- panel/src/features/monitor/MonitorPage.test.tsx
- backend/internal/broker/pg_broker.go
- panel/src/features/home/LiveStrip.test.tsx
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48e580e56a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…cope station count join PR #81 round-2 convergence, Findings 1-2: - BatchCheckin only signals the monitor broker when a kind=checkin item was genuinely created; ApplyBatchCheckin deliberately reports BatchCheckinCreated for zone_entry items too (even pre-existing ones), but zone entries write zone_checkins, which the monitor snapshot never reads, so a zone-entry-only access-control sync no longer spuriously publishes and forces every attached monitor to refetch unchanged data. - GetMonitorStations' checkin_actions join now carries an `AND ca.event_id = cs.event_id` predicate so Postgres can use idx_checkin_actions_event_created instead of scanning/hashing the global table for tenants with many actions in other events. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… StatusPill station liveness PR #81 round-2 convergence, Findings 3-5: - useMonitorStream.ts's raw `fetch` call is extracted into a new shared/api/sseStream.ts (`openSseStream`), which owns URL construction, auth/Accept headers, non-OK -> ApiError normalization, and the reader/decoder/parseSse consume loop -- the hook keeps its full state-machine (statuses, backoff, coalescing, resync-on-hello, terminal-4xx stop) but now has zero direct `fetch` references, per panel/AGENTS.md's data-fetching rule. parseSse.ts (pure, no monitor knowledge) moves to shared/api/ alongside it, since shared/ must not depend on features/. - RecentFeedCard.tsx's undo/reprint/checkin rows now carry a localized, visually-hidden (sr-only) action label alongside the existing aria-hidden icon, so a screen reader can tell the three action types apart instead of hearing identical name/zone/time for all of them. - @idento/ui's StatusPill gains an additive `variant="bare"` (a chrome-less, label-as-aria-label status dot) so StationsCard.tsx's per-station liveness dot composes from the shared primitive instead of hand-rolling its own colored circle; visuals (green fresh / amber stale + the mandatory "stale Ns" text) are unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Owner
Author
|
@codex review |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/ui/src/components/status-pill.tsx`:
- Around line 41-56: Update the StatusPill bare variant and its documented
consumer so status meaning is conveyed with a non-color cue, such as an icon or
visible text, while preserving the required aria-label and status coloring.
Adjust the variant-specific rendering and related documentation/tests around
StatusPill to remove the color-only behavior; keep the existing pill and
indicator="dot" behavior unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8b3e990f-b06b-4765-bf53-accdebb922dc
📒 Files selected for processing (16)
backend/internal/handler/checkins_batch.gobackend/internal/handler/legacy_publish_test.gobackend/internal/store/pg_store_monitor.gobackend/internal/store/pg_store_monitor_test.gopackages/ui/src/components/status-pill.test.tsxpackages/ui/src/components/status-pill.tsxpanel/src/features/monitor/MonitorPage.test.tsxpanel/src/features/monitor/RecentFeedCard.tsxpanel/src/features/monitor/StationsCard.tsxpanel/src/features/monitor/useMonitorStream.tspanel/src/shared/api/parseSse.test.tspanel/src/shared/api/parseSse.tspanel/src/shared/api/sseStream.test.tspanel/src/shared/api/sseStream.tspanel/src/shared/i18n/en.jsonpanel/src/shared/i18n/ru.json
🚧 Files skipped from review as they are similar to previous changes (5)
- backend/internal/handler/checkins_batch.go
- panel/src/shared/i18n/en.json
- panel/src/features/monitor/StationsCard.tsx
- panel/src/features/monitor/RecentFeedCard.tsx
- backend/internal/store/pg_store_monitor_test.go
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d489766fc6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…bution, attendee-CRUD publishes PR #81 round-3 convergence, three backend findings: - Finding 1 (Codex P1): NewPGBroker's LISTEN connection (initial + reconnect) used a raw pgx.Connect(ctx, dbURL), re-parsing dbURL via pgx.ParseConfig -- a different parser than pgxpool.ParseConfig that leaves pool-only options (pool_max_conns, pool_min_conns) in RuntimeParams, which Postgres rejects as unrecognized startup parameters. Now reuses the pool's own sanitized ConnConfig via pgx.ConnectConfig (Copy()'d per connect, matching pgxpool's own pattern). - Finding 2 (Codex): zone attribution reused a stale 'checkin' action across a legacy clear (no undo row) + legacy re-checkin (no new action row), since the latest state-changing action was still the old 'checkin'. Scoped attribution to the attendee's current check-in period via ls.created_at >= a.checked_in_at, exploiting CheckInAttendee's same-tx now() for checked_in_at and the action row's created_at. - Finding 3 (Codex): CreateAttendee, DeleteAttendee, and BulkCreateAttendees never published to the monitor broker, leaving totals stale on a running event with no station heartbeat. Added publishCheckinEvent calls to all three (bulk: exactly once per batch, gated on createdCount > 0). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… station liveness PR #81 round-3 convergence, UI Finding 4 (CodeRabbit + Codex, two facets of the round-2 variant="bare" StatusPill): - Codex facet: the dot's aria-label sat on a generic, non-focusable <span>, which many assistive-tech paths don't reliably announce. bare now renders the label as real sr-only DOM text on a nested span instead, matching the sr-only idiom already used by RecentFeedCard.tsx/WorkspaceRail.tsx. - CodeRabbit facet: a bare colored dot with no visible text violates "never color alone" for sighted colorblind users -- the fresh station row rendered no text at all. StationsCard.tsx now also renders its own visible, muted status word next to a fresh row's dot (new monitorStationOnline i18n key, EN+RU), mirroring the stale row's pre-existing visible "stale Ns" span. Deliberately deviates from board 7e's text-free fresh-row spec -- the codified never-color-alone rule governs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1881059cab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…index, station/zone publish sites Fixes four convergence-round findings on PR #81 (panel P4.2 live monitor + SSE): a DATABASE_URL carrying pool_min_conns would leave PGBroker's 2-conn notify pool perpetually churning to satisfy an unreachable minimum; the monitor's per-attendee latest-action attribution query had no supporting index and forced a full sort on every snapshot re-fetch; RegisterCheckinStation and the three zone CRUD handlers changed monitor-visible state without publishing, leaving dormant re-registrations and zone list changes stale indefinitely. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 76a16c19b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…e PATCH ExternalImport (api_keys.go): Creates attendees via API key but never published, leaving monitors' totals stale. Now publishes once when at least one attendee is created, after the import loop and event field schema update (one per request, not per attendee). UpdateAttendeeInfo (attendees.go): PATCH handler updates attendee fields but never publishes, staling monitors' last-scans feed for people whose names/details changed. Now publishes after successful update, exactly like the existing PUT checkin handler. Both use the existing nil-safe publishCheckinEvent helper (event_publish.go) with no publish on validation/store failures; mirroring the 13+ existing publish sites (CreateAttendee, DeleteAttendee, BulkCreateAttendees, StationCheckin, UndoCheckin, RegisterCheckinStation, zone CRUD, etc.). PR #81 round-5. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Owner
Author
|
@codex review |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/migrations/000022_checkin_actions_attendee_idx.up.sql (1)
24-24: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAvoid blocking table writes during index creation.
Creating an index on a potentially large table like
checkin_actionslocks the table against writes for the duration of the build. For live databases, it is highly recommended to useCONCURRENTLYto allow check-ins and other updates to continue uninterrupted.Since your migration runner (
backend/internal/store/pg_store.go,RunMigrations) executes migrations vias.db.Execwithout implicitly wrapping them in a transaction block,CONCURRENTLYis safe to use here. Additionally, the index definition test inpg_store_monitor_integration_test.gowill still pass untouched, becausepg_get_indexdefdoes not include the wordCONCURRENTLYin its returned definition string.💡 Proposed change
-CREATE INDEX idx_checkin_actions_event_attendee ON checkin_actions(event_id, attendee_id, created_at DESC, id DESC); +CREATE INDEX CONCURRENTLY idx_checkin_actions_event_attendee ON checkin_actions(event_id, attendee_id, created_at DESC, id DESC);🤖 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 `@backend/migrations/000022_checkin_actions_attendee_idx.up.sql` at line 24, Update the CREATE INDEX statement for idx_checkin_actions_event_attendee to use concurrent index creation, preserving its existing columns and sort order. Do not add transaction wrapping, since RunMigrations executes this migration through s.db.Exec without an implicit transaction.Source: Linters/SAST tools
🤖 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.
Nitpick comments:
In `@backend/migrations/000022_checkin_actions_attendee_idx.up.sql`:
- Line 24: Update the CREATE INDEX statement for
idx_checkin_actions_event_attendee to use concurrent index creation, preserving
its existing columns and sort order. Do not add transaction wrapping, since
RunMigrations executes this migration through s.db.Exec without an implicit
transaction.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f9a156fa-f9d8-4ed2-8e62-27b2f6caef96
📒 Files selected for processing (11)
backend/internal/broker/broker_test.gobackend/internal/broker/pg_broker.gobackend/internal/handler/api_keys.gobackend/internal/handler/attendees.gobackend/internal/handler/checkin_stations.gobackend/internal/handler/checkin_stations_publish_test.gobackend/internal/handler/zones.gobackend/internal/handler/zones_publish_test.gobackend/internal/store/pg_store_monitor_integration_test.gobackend/migrations/000022_checkin_actions_attendee_idx.down.sqlbackend/migrations/000022_checkin_actions_attendee_idx.up.sql
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/internal/broker/pg_broker.go
- backend/internal/broker/broker_test.go
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc6cb1496a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+357
to
+359
| if existingAttendee.CheckinStatus != beforeCheckinStatus { | ||
| h.publishCheckinEvent(c.Request().Context(), existingAttendee.EventID) | ||
| } |
There was a problem hiding this comment.
When the legacy PUT /api/attendees/{id} resends checkin_status: true for an already checked-in attendee with a different checked_in_at, lines 326-327 still persist the new timestamp, and the new monitor attribution query only keeps a station zone when the latest action satisfies ls.created_at >= a.checked_in_at. This guard suppresses the publish because the boolean status did not flip, so a station-attributed attendee can move to unattributed in GetMonitorOverview while attached monitors never receive an SSE invalidation until some unrelated event change occurs.
Useful? React with 👍 / 👎.
Comment on lines
+222
to
+223
| for eventID := range affectedEvents { | ||
| h.publishCheckinEvent(c.Request().Context(), eventID) |
There was a problem hiding this comment.
When a sync push touches multiple events and the broker publish path stalls, this serial loop waits for publishCheckinEvent's timeout for every affected event before returning the already-completed sync response. With the current 2s publish timeout, a mobile sync that updated 10 events can spend about 20s here after all DB writes succeeded, so use a shared deadline or publish these event notifications concurrently instead of multiplying the timeout by event count.
Useful? React with 👍 / 👎.
thevladbog
added a commit
that referenced
this pull request
Jul 19, 2026
…ach the monitor's rate/peak/recent (#82) * docs: event-wide checkin_actions design (monitor metrics gap follow-up) Resolves the PR #81 deferred gap: rate/peak/recent aggregate only station-path checkin_actions while mobile batch, legacy attendee PUT (mobile's online check-in path), and SyncPush flip checkin_status without action rows. Decision: option (a) — write action rows from all three paths with station_id NULL and explicit created_at equal to the persisted checked_in_at, symmetric undo rows, no provenance column. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: event-wide checkin_actions implementation plan Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * backend: add InsertCheckinActionAt store method (explicit created_at) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * backend: batch check-ins write event-wide checkin_actions rows atomically Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * backend: legacy attendee PUT writes event-wide checkin/undo feed rows Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * backend: sync push writes event-wide checkin/undo feed rows Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * backend: integration-prove event-wide actions feed; retire stale station-only comments Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * backend: DB-arbitrate legacy PUT/sync check-in transitions before feed inserts PR #82 bot round: gating the feed-row insert on a Go-level before/after CheckinStatus compare was a read-compare-write race — two concurrent requests could both observe the old status, both blind-write via UpdateAttendee, and both insert a duplicate checkin_actions row. A new TransitionAttendeeCheckinStatus guarded UPDATE (WHERE on the CURRENT status, RowsAffected as the verdict — the ApplyBatchCheckin / CheckInAttendee pattern) now claims the transition atomically; feed inserts and monitor publishes are gated on the claim, and UpdateAttendee keeps its legacy full-row overwrite role unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * backend: gofmt checkin_actions_feed_test.go Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: CI Bot <ci@example.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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
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
EventBrokerseam (in-memory fanout + Postgres LISTEN/NOTIFY transport, multi-replica-safe), a thin-ping stream endpoint (GET /api/events/{event_id}/monitor/stream), and nil-safe publish sites on the four monitor-visible check-in mutations (check-in, undo, reprint, heartbeat).GET /api/events/{event_id}/monitor) — totals, scans/min + peak + est-done (purecomputeRates), per-zone breakdown whosesum(zones)+unattributed == checked_ininvariant holds by SQL construction, per-station liveness, recent feed. Zero migrations — all aggregations read existing tables./events/$eventId/monitor: chrome-less top-level route (same proven sibling-registration pattern as P4.1's station), LIVE pill, Totals/Zones/Stations/Recent cards, 45s station-staleness with mandatory text label (never color-alone), verdict-color discipline (undo/reprint are neutral, not verdicts), reconnecting badge over stale data.useMonitorStream: fetch-streaming SSE client (Bearer auth — EventSource can't set headers), coalesced snapshot invalidation ≤1/sec, exponential backoff with jitter, resync refetch on reconnect, abort-safe on unmount/scope change. No polling fallback by design.zone_statsread and 15s stats-poll removed.Test plan
OPENAPI_COVERAGE=1 go test ./... -count=1(483 tests) +golangci-lint+go test -raceon broker/handler — cleanpackages/uiuntouched-green (141)npm run generate:api -w panel— zero schema driftrouter.tsxdiff purely additive (onlyeventMonitorRoute;/registerguards byte-for-byte unchanged);web/diff empty; zero new dependencies; zero files underbackend/migrations/🤖 Generated with Claude Code
Summary by CodeRabbit