feat(account): close the page with what authGD last pushed - #42
Merged
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 9 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThe account view now calculates per-service push telemetry from sync runs and cron schedules. The account page displays last and next push times for standings, map, and Discord, updates the contacts heading to “Standings,” and adds closing artwork. Tests cover schedule parsing, status selection, and account states. ChangesAccount Push Telemetry
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
guarzo
force-pushed
the
worktree-account-closing-beat
branch
2 times, most recently
from
August 3, 2026 22:30
0a0622e to
4e7a7f3
Compare
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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 `@e2e/account.spec.ts`:
- Around line 146-156: Make the relative-time check in the account page test
deterministic by freezing the test clock or asserting an acceptable bounded
range around the 12-minute fixture age. Update the expectation associated with
the “Last pushed” heading and pushed locator while preserving validation of the
rendered relative time.
- Around line 154-160: Update the assertions around the pushed telemetry rows to
locate each row by its <dt> label, then verify the expected <dd> values for
standings, map, and Discord individually. Replace the broad pushed-container
text checks and first-match cadence assertion with row-scoped checks that
validate each surface’s configured next occurrence, using the existing pushed
locator as the test scope.
In `@src/app/account/page.tsx`:
- Around line 307-315: Replace the native img element in the closing section of
the account page with next/image, preserving its source, dimensions, alt text,
and loading behavior while adding an appropriate sizes value for responsive
image selection. Retain the native element only if this deployment cannot use
the Next.js image optimizer.
In `@src/app/globals.css`:
- Around line 1425-1438: Update the push status-column rule for .push > time and
.push > .st to use a fixed flex basis of 8ch rather than only min-width,
ensuring all status values reserve the same width and the “next HH:mm” column
aligns consistently across rows.
In `@src/core/schedules.ts`:
- Around line 98-104: Update the cron parsing loop around part.split("/") to
capture all slash-separated segments and reject any part containing more than
one slash before parsing spec and stepRaw. Preserve the existing validation for
non-numeric and zero step values, while ensuring inputs such as "*/5/2" throw an
unsupported-cron-step error.
In `@src/services/account-view.ts`:
- Around line 59-90: Cache the account-independent database results used by
getPushStatus with a short revalidation window and no account-specific key,
while keeping now outside the cached computation so nextCheckAt is calculated
against the current request time. Preserve the existing per-job query semantics
and returned PushStatus shape.
- Around line 82-85: Update getPushStatus around the nextOccurrence call to
defensively resolve JOB_CRON[jobType]. Catch missing or unsupported cadence
errors and set PushStatus.nextCheckAt to null, while preserving the calculated
occurrence for valid expressions so account rendering continues without
throwing.
In `@tests/account-view.test.ts`:
- Around line 140-148: Strengthen the test “takes the newest run per job,
keeping the three independent” by adding a later-inserted, higher-id run whose
finishedAt is earlier than the existing contacts run, so id ordering and
finishedAt ordering disagree. Update the expected contacts lastPushedAt
assertion to confirm getPushStatus selects that higher-id row while leaving the
other job and never-ran assertions unchanged.
In `@tests/schedules.test.ts`:
- Around line 79-141: Add a test within the nextOccurrence suite that supplies a
cron expression restricting both day-of-month and day-of-week, with an instant
covering a date matching only one restriction before the date matching both.
Assert the returned occurrence satisfies both fields, thereby pinning the
documented intersection semantics rather than standard cron union behavior.
🪄 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: 665195fb-32c2-4bd8-bbb9-a7b32737d89d
📒 Files selected for processing (8)
e2e/account.spec.tspublic/brand/lander-moon.webpsrc/app/account/page.tsxsrc/app/globals.csssrc/core/schedules.tssrc/services/account-view.tstests/account-view.test.tstests/schedules.test.ts
The account page ended on a control and a disclaimer, leaving the member without the one thing they came to check: whether the system has actually acted for them, and when it will look again. Adds a "Last pushed" telemetry block — standings, map, Discord — each with a relative last-push time and the next scheduled check in UTC, and closes the page on Faoble's lander cresting the lunar horizon. Sourcing note: the timestamps come from sync_run, not from per-character state. contact_sync_state is keyed by the FLYGD character being written, so a blue or green member has no rows and would read "never run" forever; wanderer_acl_observation is a delete-and-replace snapshot where "not on the ACL" is indistinguishable from "not yet run"; audit_log records role *changes*, and "last changed" is not "last checked". The job's own completion is the only answer that is true for every tier. - SCHEDULES lifted into src/config.ts so the worker's cadence and the cadence shown to members cannot silently diverge - src/core/schedule.ts: a small pure next-occurrence helper over the cron grammar we actually use, throwing loudly on anything outside it - crew manifest column renamed Contacts -> Standings, matching the language the rest of the page uses
Adds e2e coverage for the three states the section can be in: a surface that has pushed, one that is scheduled but has never run, and a Discord row with nothing linked at all. Plus the zero-character case, where the section is absent rather than showing three empty rows. Also centres the closing artwork with a margin. The global reset makes images block-level, so the text-align on the wrapper was silently doing nothing and the lander sat flush left.
- reject a second cron step ("*/5/2") instead of dropping the tail
- degrade an unknown or unparseable JOB_CRON entry to a null next-check
rather than throwing and taking the account page down
- reserve the state column in rem, not ch: ch resolves per element
font-size and the two branches render at different sizes, so the
"next" column did not actually line up
- scope the e2e push assertions per row and assert the alignment
- pin newest-by-id (not max(finished_at)) and the dom+dow intersection
guarzo
force-pushed
the
worktree-account-closing-beat
branch
from
August 3, 2026 22:57
e9d9308 to
ae09bf4
Compare
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.
The account page ended on a control and a disclaimer. It now ends on what authGD
is actually doing on the member's behalf, and then on the artwork.
Reviewer focus: the data source changed from the brief
The plan was to aggregate per-character state. All three candidate sources are
wrong in the same direction — they report "never" for members who are being
synced correctly:
contact_sync_stateis keyed by the FLYGD character whose contact list iswritten. A blue or green member is the content of that push, never a target,
so their rows are structurally absent.
wanderer_acl_observationis delete-and-replace. A character legitimately offthe ACL has no row, indistinguishable from a job that never ran.
audit_logrecords role changes. "Last changed" is not "last checked".So each row reads
sync_runfor its job, which is true for every memberregardless of tier.
partialcounts as pushed;faileddoes not, so a brokenjob visibly stops advancing rather than reporting freshness it did not deliver.
Rationale is in the
PUSH_JOBSdoc comment.No N+1: three indexed lookups issued concurrently, ordered by serial
idtomatch the
(job_type, id desc)index, following the precedent inservices/health.ts.Other decisions
src/config.ts, and this PRoriginally did. Sync: promote the answer, collapse the evidence #47 landed
src/core/schedules.tsfor the same reason (theadmin sync page renders cadence from what the worker registers), so on rebase
that became the single source and the
src/config.tscopy was dropped ratherthan shipped alongside it. The dependency-free next-occurrence helper (12 unit
tests) now lives in that same module, under
formatCadence: that says howoften a job runs, this says when it runs next.
src/worker/queues.tsisunchanged from
main.next 22:05) in UTC, the timezone EVE schedules arealready read in.
has not run".
rows would read as a broken system rather than an empty one.
lander-moon.webp, 1120×711 cut from the master at itsalpha bounding box and drawn at 560px — a derivative cut for the size it is
drawn at, per PRODUCT.md principle 5, not a scaled-down master or a decorative
crop.
Known inconsistency, not fixed here
The top notice ("First sync has not run yet") and the crew-manifest STANDINGS
cells both key off per-character
contactSyncResult— the same structurally-nullfield described above — so they can read "not yet run" while LAST PUSHED reports
a recent push. Same defect, different surfaces, out of scope for this PR.
Verification
npx tsc --noEmit— cleannpx prettier --check .— all files matchnpx eslint .— 0 errors, 5 warnings (all pre-existing-styleno-img-element)npx vitest run— 367 passed (48 files)npx playwright test— 21 passed