new boom mechanic to try out where participant point gain is randomised - #3
new boom mechanic to try out where participant point gain is randomised#3Jesse-Heath wants to merge 3 commits into
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 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 |
| // Stamp the random-scoring cutover on first use so every date already in the ledger keeps its | ||
| // legacy 3-2-1 scoring and is never retroactively re-assigned random points. | ||
| if (!this.data.random_scoring_from) { | ||
| this.data.random_scoring_from = DateTime.now().setZone(TZ).toISODate()!; |
There was a problem hiding this comment.
Blocker: cutover stamped as today, so deploying any time after noon on a day that already scored wipes and re-rolls it.
That date is >= from, so scoreFor returns [] and the points vanish from weeklyTotals. The 30s sweep then re-resolves it via duePending and assigns new random values that contradict the podium already in the channel.
Probe against a store holding today's legacy 3-2-1 podium + daily_announced:
totals BEFORE: []
duePending: [{date:'2026-08-28',game:'boom'}, {...,game:'hadeda'}]
totals AFTER: U1:4 U2:4 U3:4
Deploy on a Friday afternoon and the crown disagrees with the posted message. Stamp tomorrow's date instead.
| * Date+game pairs that have entries and a closed tally window but no awards yet — work the | ||
| * in-process timers missed (e.g. the bot restarted mid-window). | ||
| */ | ||
| duePending(nowMs = Date.now()): Array<{ date: string; game: Game; channel_id: string }> { |
There was a problem hiding this comment.
Same trigger as the cutover issue: duePending doesn't exclude dates with daily_announced.
alreadySettled is false (awards empty), so closeGame re-applies medal reactions to a day the old build already medalled. Skipping announced dates here fixes both.
| } | ||
| } | ||
|
|
||
| await announceDay(client, date, logger); |
There was a problem hiding this comment.
A failed chat.postMessage permanently loses the day's results and the Friday crown.
resolveGame flushes awards before the post, so once all games are resolved duePending returns nothing forever and nothing re-enters announceDay. A transient rate-limit at 12:05 means markDailyAnnounced never runs and the day is silently skipped.
On master the announce block sat in the message handler and got retried by the next message. Suggest a pending-announce check driven by the sweep.
| const sweepClient = (app as any).client; | ||
| if (sweepClient) { | ||
| const sweep = setInterval(() => { | ||
| closeDueWindows(sweepClient).catch(() => {}); |
There was a problem hiding this comment.
Sweep passes no logger, so logger?.error?. (l.94) and logger?.warn?. (l.111) are no-ops and .catch(() => {}) eats the rest.
The restart-recovery path, the one most likely to hit a missing channel or a Slack failure, fails completely silently. Pass app.logger.
| // Compare on the message's own ts (full sub-second precision) so a slow delivery of a | ||
| // message that was sent inside the window still counts. | ||
| const tsMs = Math.round(Number(tsStr) * 1000); | ||
| const tooLate = closesAt != null && Number.isFinite(tsMs) && tsMs > closesAt; |
There was a problem hiding this comment.
The slow-delivery grace this refines is unreachable.
The timer resolves the game at exactly closesAt, so a message sent 12:04:59 and delivered 12:05:02 hits db.isResolved(...) first and is clowned regardless of its own ts. Either resolve after a short grace period, or accept an entry whose ts <= closesAt even when resolved.
| const neededGames: Game[] = weekday === 3 ? ['boom', 'hadeda', 'wednesday'] : ['boom', 'hadeda']; | ||
| const neededGames = neededGamesForDate(date); | ||
|
|
||
| // Settle any window whose deadline passed while no timer was live (e.g. after a restart). |
There was a problem hiding this comment.
This new await yields before the entryFor duplicate check below.
Two messages from the same user delivered in the same tick both see priorEntry === null, so both incrementCount, both addPlacement, and both get a check mark. counts[date][game] then exceeds the entrant count, contradicting docs/OPERATIONS.md ("counts[date][game] therefore equals the number of entrants").
| * How long a game's tally window stays open, measured from its first valid entry. | ||
| * When it closes, every unique entrant is given a unique random point value in 1..n. | ||
| */ | ||
| export const ENTRY_WINDOW_MS = (() => { |
There was a problem hiding this comment.
Tally window can outlive the noon window: a first :hadeda-boom: at 12:57 opens a window closing 13:02, but anyone posting at 13:00+ is clowned by the !inWindow guard first.
README and docs/OPERATIONS.md both describe the tally window as authoritative. Intended, or should the window be clamped to 12:59:59?
|
Good mechanic, but it needs a rebase and three fixes before I can merge.
|
zkrige
left a comment
zkrige
left a comment
There was a problem hiding this comment.
Requesting changes. The test coverage on this is genuinely thorough (tsc clean, 70/70 pass). Five issues, one behavioural.
src/features/boom/store.ts:462 — the first entrant of a day is exempt from the entry deadline. duePending skips any date where !hasAnyEntry(date), so a lone late entrant is still recorded, and scheduleSettle then computes a 0ms delay and settles on the next tick. A second player one second behind hits isResolved and gets :clown_face:. Identical circumstances, opposite outcomes, decided by macrotask ordering.
src/features/boom/store.ts:426 — resolveGame's deadline guard only covers the empty case (!entrants.length && nowMs < windowSettlesAtMs), so a game with entrants resolves at any nowMs, including mid-window. Every current caller is gated, so it is latent, but if nowMs lands just before windowSettlesAtMs (clock step-back, early libuv timer) the played games resolve and the unplayed ones do not, so announceDay can never fire and settleDay has already deleted the per-date timer.
src/features/boom/index.ts:282 — inWindow changed from inNoonWindow to inEntryWindow, but the !inWindow clown still runs before the !isWorkday branch at 290. A 💥 posted on a Saturday at 12:30 used to get "Boom isn't played today"; it now gets :clown_face: and no explanation. README.md and docs/TESTING.md in this diff still promise the notice.
.env.example — BOOM_ANNOUNCE_GRACE_MS has no callers left in src/ but is still in the example; the new BOOM_ENTRY_WINDOW_MS is documented in README.md:72 and docs/CONFIG.md:16 but never appears there. An operator copying the example gets one dead knob and misses the live one.
docs/OPERATIONS.md:121 — "Legacy podium helpers (getPlacements, placementsCount, PODIUM_WEIGHTS) remain only to score dates before random_scoring_from". PODIUM_WEIGHTS is used, but getPlacements, placementsCount, addPlacement, incrementCount and getCounts have zero callers in src/ (tests only) — pre-cutover scoring goes through the private computePodium. Either delete the methods or correct the doc.
2 participants
No description provided.