Repository navigation
Never tie scores: reachable 5,000 rubric, score_taken, board-first serving - #29
Open
arilivigni wants to merge 1 commit into
Open
arilivigni wants to merge 1 commit into
arilivigni wants to merge 1 commit into
Conversation
…rving - Rubric: finer categories plus a letter-value category; maxima sum to 5,000 and a test shows one valid name scoring exactly 5,000. - Service: refuse a different drink whose score an active drink holds (409 score_taken) inside the admission under the write lock; resends stay idempotent and takedowns free the score. - Booth: with a leaderboard, score -> admit -> serve. Refusals keep the attendee naming; no answer keeps the attempt in the ledger and resends the same token, so a drink is served and admitted once. Wipes refuse while an admission is unsettled. Standalone booths enforce unique scores locally. - Surfaces, docs, and tests updated. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved publication attempts can still settle after rejection or restart, allowing duplicate drinks for the same attendee and potentially overwriting their submission.
3 open issues
What changed in this PR
This PR makes 5,000 points attainable and prevents new active drinks from sharing a score. Configured booths now wait for leaderboard admission before serving, with persisted attempts for retry recovery.
Changes:
- Rebalance the rubric and add single-point letter values.
- Enforce score uniqueness and introduce board-first serving.
- Update attendee feedback, operational guidance, and regression coverage.
| File | Description |
|---|---|
| tests/name-score.test.mjs | Verify reachable 5,000 and single-point scoring. |
| tests/leaderboard-sync.test.mjs | Cover admission failures and retry recovery. |
| tests/leaderboard-service.test.mjs | Test score conflicts, races, and revised publication flow. |
| tests/booth-menu.test.mjs | Cover standalone score uniqueness and preparation. |
| tests/booth-engine.test.mjs | Give multi-drink fixtures distinct scores. |
| tests/board-first.test.mjs | Add end-to-end admission and recovery coverage. |
| README.md | Summarize scoring and admission guarantees. |
| leaderboard-service/store.mjs | Reject occupied scores during atomic admission. |
| leaderboard-service/app.mjs | Return 409 score_taken for score conflicts. |
| docs/leaderboard-service.md | Document admission behavior and coordinated deployment. |
| docs/integration-contract.md | Define score uniqueness and board-first contracts. |
| docs/architecture.md | Explain persisted admissions and regression requirements. |
| booth/RUNBOOK.md | Update outage handling and staff procedures. |
| booth/CHEAT-SHEET.md | Add quick guidance for outages and score conflicts. |
| .github/steps/1-name-a-drink.md | Explain score tweaks and the revised breakdown. |
| .github/skills/order-up/SKILL.md | Update attendee guidance for admission outcomes. |
| .github/extensions/commit-and-sip/services/name-score.mjs | Reweight categories and add letter values. |
| .github/extensions/commit-and-sip/services/leaderboard.mjs | Classify score conflicts as terminal publication refusals. |
| .github/extensions/commit-and-sip/services/booth-menu.mjs | Separate preparation from insertion; optionally enforce unique scores. |
| .github/extensions/commit-and-sip/renderer/booth.js | Show checking status during submission. |
| .github/extensions/commit-and-sip/renderer/booth.html | Add an accessible checking notice. |
| .github/extensions/commit-and-sip/canvas.mjs | Describe board-first submission outcomes. |
| .github/extensions/commit-and-sip/booth-panel.mjs | Remove post-serving background publication. |
| .github/extensions/commit-and-sip/booth-engine.mjs | Persist and settle admissions before serving; guard event wipes. |
Review effort: Balanced
| // A standalone booth is its own board, so it refuses a score another | ||
| // drink on its menu already holds. A booth with an event leaderboard | ||
| // never reaches here: submitName asks the board instead. | ||
| const entry = addDrink(this.houseMenu(data), { |
Comment on lines
+999
to
+1001
| if (refusedByBoard(outcome.error, firstSend)) { | ||
| const refusal = admissionRefusal(outcome.error.code, admission.entry); | ||
| delete run.admission; |
| this.publicationSweep ??= (async () => { | ||
| try { | ||
| if (!this.leaderboardClient) return 0; | ||
| await this.resolveAdmissions(); |
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.
Why
Scores tied: every category paid in steps of 50, and the real ceiling was 4,600 even though
MAX_SCOREis 5,000. With this change, no two active drinks share a score, and 5,000 is reachable.What changes
Rubric (
services/name-score.mjs). Still a pure function of the name.Leaderboard service (
store.mjs,app.mjs)score_taken.admitunder the existing write lock, so concurrent requests cannot both land.Booth (
booth-engine.mjs)leaderboardApi, the flow is score → admit → serve. The attempt and its publication token are written to the ledger (run.admission) before sending, and the drink is served only after the board admits it.score_taken,duplicate_drink,handle_taken,unavailable_drink): the attendee stays on the naming screen.score_takenasks them to tweak the name.board_rebuilding, a mismatched receipt, or any other 4xx on a resend): nothing is served and the booth returns503 leaderboard_unreachable("try again in a moment"). The next try, the retry sweep, or a wipe resends the same token, so the drink is served and admitted exactly once.409 admission_in_progressstops a double tap or a second process.archiveAndWiperefuses withadmissions_unsettleduntil every admission is settled.leaderboardApi) refuse a score already on the local menu with the samescore_taken.Docs: integration contract, leaderboard service, architecture, RUNBOOK, CHEAT-SHEET, README, step 1, and SKILL.md.
Tests
npm test: 353/353 pass, andnpm run checkis clean. Newtests/board-first.test.mjsruns against the real service and covers:score_taken, then a tweak that lands;board_rebuilding;The service, sync, engine, menu and name-score suites are updated as well.
Reviewer check:
renderer/booth.html/booth.jsgain a hidden#name-checkingnotice. Perdocs/architecture.md, drive the booth screen in a real canvas before the event.The service re-scores every submission, so a scoring mismatch gets
422 score_mismatch. Old booth builds also serve a drink before publishing it. The service and every booth must therefore run the exact same merge commit.leaderboardApiunset, during the change.npm run leaderboard:packageplus the zip deploy). Verify the build by diffing/home/data/SitePackages/<packagename.txt>from Kudu against the merge commit.npm testthere.Do not merge until reviewed. Auto-merge is not enabled.