Add Mosaic Press playable slice#4
Open
wuhuizuo wants to merge 1 commit into
Open
Conversation
Co-authored-by: multica-agent <github@multica.ai>
Contributor
|
Mosaic Press preview deployed. URL: https://NightProgrammers.github.io/games/previews/pr-4/ If this is the first deployment, enable GitHub Pages once in repository settings and point it at the |
📝 WalkthroughWalkthroughA complete new Mosaic Press game implementation is added, comprising game engine logic, data models, daily level generation, frontend application controller, UI styling, HTML entry point, configuration, comprehensive documentation, GitHub Actions preview deployment workflow, and test suite. ChangesMosaic Press Game Implementation
Sequence DiagramsequenceDiagram
participant User
participant App as app.js
participant Engine as engine.js
participant Data as data.js
participant DOM
User->>App: click level
App->>Data: get level (handcrafted or daily)
Data-->>App: level object
App->>Engine: createRuntime(level)
Engine-->>App: runtime (precomputed maps)
App->>Engine: createInitialState(level)
Engine-->>App: initial state
App->>DOM: render gallery/level view
DOM-->>User: display level UI
User->>App: click fastener
App->>Engine: collectLegalActions(level, state, runtime)
Engine-->>App: legal actions (with scoring)
App->>Engine: evaluateFastener(level, state, fastenerId, runtime)
Engine-->>App: evaluation (path, placement, overflow)
App->>Engine: applyAction(level, state, action, runtime)
Engine-->>App: new state + outcome
App->>DOM: render updated state
DOM-->>User: display result (placed, overflow, or remove)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
mosaic-press/src/daily.js (1)
162-170: ⚡ Quick winFail fast when a daily strip is missing from all stacks.
The
?? template.stacks[0].idfallback hides template authoring mistakes and can turn an invalid template into a different, still-playable board. This should throw so the bad template is caught immediately instead of being silently remapped.Suggested fix
const strips = template.strips.map((strip, index) => ({ + const stackId = template.stacks.find((stack) => stack.stripIds.includes(strip.id))?.id; + if (!stackId) { + throw new Error(`Daily template strip ${strip.id} is not assigned to any stack.`); + } + + return { id: strip.id, name: strip.name, - stackId: template.stacks.find((stack) => stack.stripIds.includes(strip.id))?.id ?? template.stacks[0].id, + stackId, family: strip.family, paletteId: paletteOrder[index % paletteOrder.length], labels: [...strip.labels], fasteners: strip.fasteners.map((entry) => ({ ...entry })) - })); + }; + });🤖 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 `@mosaic-press/src/daily.js` around lines 162 - 170, The current strips mapping uses template.stacks.find(...)? .id ?? template.stacks[0].id which silently falls back to the first stack when no stack includes a given strip.id; change this to fail fast by checking the result of template.stacks.find((stack) => stack.stripIds.includes(strip.id)) and throwing a clear Error (including the offending strip.id and maybe template id/name) if no stack is found, then assign stackId from the foundStack.id; update the mapping in the strips creation (the stackId expression) and any related tests or callers to expect the thrown error for malformed templates.mosaic-press/tests/engine.test.js (2)
147-149: 💤 Low valueTight numeric ranges couple the test to current algorithm internals.
Asserting exact bounds on
stacks.length,tray.cells.length, andsolution.sequence.lengthfor a generated daily level will fail on any future tweak to the generation algorithm — even if the level remains valid and solvable. The two assertions immediately above (lines 144–146) already confirm correctness viavalidateLevel; consider removing or loosening these range checks, or at minimum documenting that they are intentionally tied to the current generator version.🤖 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 `@mosaic-press/tests/engine.test.js` around lines 147 - 149, The three tight numeric assertions on first.stacks.length, first.tray.cells.length, and validation.solution.sequence.length are coupling the test to generator internals; remove or relax them: either delete those assert.ok(range) lines entirely (since validateLevel already verifies correctness via validateLevel/validation) or replace them with looser checks (e.g., assert.minimums only: first.stacks.length >= 1, first.tray.cells.length >= 1, validation.solution.sequence.length >= 1) or convert them to non-failing logs/comments that document these expectations and note they are generator-version-dependent; update or add a brief comment referencing validateLevel so future readers know why strict bounds were removed.
49-49: ⚡ Quick winHardcoded
HANDCRAFTED_LEVELSindices make mechanic-specific tests silently exercise the wrong level.
HANDCRAFTED_LEVELS[7](wax),[4](overflow), and[9](hint) will quietly test a different level — or an incorrect mechanic — if the authored order ever changes.Consider either (a) looking up levels by their stable
idfield, or (b) adding an inline comment that locks the expected level id next to each index access so a reviewer catches any drift immediately.♻️ Example lookup by id
- const level = HANDCRAFTED_LEVELS[7]; + const level = HANDCRAFTED_LEVELS.find(l => l.id === "wax-block-level-id"); + assert.ok(level, "wax-tab level not found in HANDCRAFTED_LEVELS");Also applies to: 59-59, 127-127
🤖 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 `@mosaic-press/tests/engine.test.js` at line 49, Replace fragile numeric indexing of HANDCRAFTED_LEVELS (e.g., HANDCRAFTED_LEVELS[7], [4], [9]) with a stable lookup by the level's id: locate the places assigning to the level variable (e.g., the line that currently sets const level = HANDCRAFTED_LEVELS[7]) and change the assignment to find the level by its id (use Array.prototype.find on HANDCRAFTED_LEVELS with the known stable id string for that mechanic); if you prefer not to change code shape, add an inline comment next to each numeric index (HANDCRAFTED_LEVELS[7] // id: "wax", HANDCRAFTED_LEVELS[4] // id: "overflow", HANDCRAFTED_LEVELS[9] // id: "hint") so reviewers catch any order drift—apply the same change to the other occurrences that reference those numeric indices.
🤖 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 `@mosaic-press/src/app.js`:
- Around line 500-556: The modals rendered when appState.modal ===
"start"/"fail"/(default win) are visual only; update the modal markup and focus
handling so they act as real dialogs: add role="dialog" and aria-modal="true" to
the modal container (e.g., the element with class "modal-card"), include an
aria-labelledby that points to the modal h2, make the modal container focusable
(tabindex="-1") and programmatically move focus into it when rendered (focus the
modal or the first actionable button with data-action), implement a simple focus
trap over elements with data-action to keep keyboard focus inside while the
modal is open, restore focus to the previously focused element when the modal
closes, and ensure Escape closes the modal (same handlers as the Back/Quit
actions).
- Around line 17-27: appState.dailySeed is computed once at load and must be
refreshed at interaction time; remove dailySeed from the static appState
initialization and instead call dailySeedFromDate() at each place where the
daily board is shown or opened (e.g., replace usages of appState.dailySeed in
the gallery pill rendering and in the open-daily handler with direct calls to
dailySeedFromDate()). Update the code paths referenced by the diff (the gallery
pill render and the open-daily/openDaily functions) so they compute seed
dynamically rather than relying on the stored appState.dailySeed.
- Around line 46-48: The saveProgress function currently writes to localStorage
without handling failures; update saveProgress to mirror loadProgress's tolerant
behavior by wrapping the localStorage.setItem(STORAGE_KEY,
JSON.stringify(appState.progress)) call in a try/catch, and on error either
silently ignore or log the error (using the same logging approach as
loadProgress) instead of letting the exception propagate; reference the
saveProgress function, STORAGE_KEY, and appState.progress when applying the
change.
In `@mosaic-press/styles.css`:
- Around line 57-73: Add a visible keyboard focus style for interactive
controls: update the selectors for .button, .hud__button, .hud__back and
.fastener to include :focus-visible (e.g. .button:focus-visible,
.hud__button:focus-visible, .hud__back:focus-visible, .fastener:focus-visible)
and apply a clear visual indicator (such as a contrast ring/outline or subtle
box-shadow and the same hover transform) instead of removing the browser
outline; keep transition timing consistent with the hover rules and avoid using
outline: none so keyboard users can see focus.
In `@mosaic-press/tests/engine.test.js`:
- Around line 15-38: The test "handcrafted levels follow the expected stack,
row, and cell ramp" can silently pass if HANDCRAFTED_LEVELS is shorter than
expected; add a length guard before iterating by asserting
HANDCRAFTED_LEVELS.length === expected.length (or at least >= expected.length)
and fail early with a clear message; then keep the existing
HANDCRAFTED_LEVELS.forEach block to validate per-level counts (referencing
HANDCRAFTED_LEVELS, expected, and the test name).
- Around line 135-137: The test for dailySeedFromDate doesn't actually validate
UTC normalization because the chosen timestamp keeps local and UTC dates the
same; update the test input to a datetime that crosses UTC midnight (for example
use "2026-05-05T02:00:00+08:00") so that UTC date differs from local date and
assert that dailySeedFromDate(new Date("2026-05-05T02:00:00+08:00")) returns
"2026-05-04", ensuring the function's UTC-based normalization is exercised.
---
Nitpick comments:
In `@mosaic-press/src/daily.js`:
- Around line 162-170: The current strips mapping uses
template.stacks.find(...)? .id ?? template.stacks[0].id which silently falls
back to the first stack when no stack includes a given strip.id; change this to
fail fast by checking the result of template.stacks.find((stack) =>
stack.stripIds.includes(strip.id)) and throwing a clear Error (including the
offending strip.id and maybe template id/name) if no stack is found, then assign
stackId from the foundStack.id; update the mapping in the strips creation (the
stackId expression) and any related tests or callers to expect the thrown error
for malformed templates.
In `@mosaic-press/tests/engine.test.js`:
- Around line 147-149: The three tight numeric assertions on
first.stacks.length, first.tray.cells.length, and
validation.solution.sequence.length are coupling the test to generator
internals; remove or relax them: either delete those assert.ok(range) lines
entirely (since validateLevel already verifies correctness via
validateLevel/validation) or replace them with looser checks (e.g.,
assert.minimums only: first.stacks.length >= 1, first.tray.cells.length >= 1,
validation.solution.sequence.length >= 1) or convert them to non-failing
logs/comments that document these expectations and note they are
generator-version-dependent; update or add a brief comment referencing
validateLevel so future readers know why strict bounds were removed.
- Line 49: Replace fragile numeric indexing of HANDCRAFTED_LEVELS (e.g.,
HANDCRAFTED_LEVELS[7], [4], [9]) with a stable lookup by the level's id: locate
the places assigning to the level variable (e.g., the line that currently sets
const level = HANDCRAFTED_LEVELS[7]) and change the assignment to find the level
by its id (use Array.prototype.find on HANDCRAFTED_LEVELS with the known stable
id string for that mechanic); if you prefer not to change code shape, add an
inline comment next to each numeric index (HANDCRAFTED_LEVELS[7] // id: "wax",
HANDCRAFTED_LEVELS[4] // id: "overflow", HANDCRAFTED_LEVELS[9] // id: "hint") so
reviewers catch any order drift—apply the same change to the other occurrences
that reference those numeric indices.
🪄 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: 98565cad-98c2-49ad-b06c-e3db16037421
📒 Files selected for processing (10)
.github/workflows/mosaic-press-preview.ymlmosaic-press/README.mdmosaic-press/index.htmlmosaic-press/package.jsonmosaic-press/src/app.jsmosaic-press/src/daily.jsmosaic-press/src/data.jsmosaic-press/src/engine.jsmosaic-press/styles.cssmosaic-press/tests/engine.test.js
Comment on lines
+17
to
+27
| const appState = { | ||
| screen: "gallery", | ||
| mode: "ftue", | ||
| levelIndex: 0, | ||
| dailySeed: dailySeedFromDate(), | ||
| progress: loadProgress(), | ||
| level: null, | ||
| runtime: null, | ||
| puzzle: null, | ||
| modal: null | ||
| }; |
There was a problem hiding this comment.
Refresh the daily seed at interaction time.
appState.dailySeed is captured once and then reused for both the gallery pill and open-daily. If the tab stays open past 00:00 UTC — for example from May 6, 2026 to May 7, 2026 — the app will still show and open the May 6 board until the page is reloaded.
Suggested fix
+function currentDailySeed() {
+ return dailySeedFromDate();
+}
+
function isDailyCleared(seed = appState.dailySeed) {
return appState.progress.dailySeeds.includes(seed);
}-function isDailyCleared(seed = appState.dailySeed) {
+function isDailyCleared(seed = currentDailySeed()) {
return appState.progress.dailySeeds.includes(seed);
} if (action === "open-daily") {
if (!isDailyUnlocked()) {
return;
}
- loadLevel({ mode: "daily", dailySeed: appState.dailySeed, showStart: true });
+ const dailySeed = currentDailySeed();
+ appState.dailySeed = dailySeed;
+ loadLevel({ mode: "daily", dailySeed, showStart: true });
return;
}Also applies to: 54-56, 576-580
🤖 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 `@mosaic-press/src/app.js` around lines 17 - 27, appState.dailySeed is computed
once at load and must be refreshed at interaction time; remove dailySeed from
the static appState initialization and instead call dailySeedFromDate() at each
place where the daily board is shown or opened (e.g., replace usages of
appState.dailySeed in the gallery pill rendering and in the open-daily handler
with direct calls to dailySeedFromDate()). Update the code paths referenced by
the diff (the gallery pill render and the open-daily/openDaily functions) so
they compute seed dynamically rather than relying on the stored
appState.dailySeed.
Comment on lines
+46
to
+48
| function saveProgress() { | ||
| localStorage.setItem(STORAGE_KEY, JSON.stringify(appState.progress)); | ||
| } |
There was a problem hiding this comment.
Handle localStorage write failures the same way reads are handled.
loadProgress() already tolerates unavailable storage, but saveProgress() will still throw in restricted/private/file-based contexts. That turns a successful clear into a runtime failure on the first save attempt.
Suggested fix
function saveProgress() {
- localStorage.setItem(STORAGE_KEY, JSON.stringify(appState.progress));
+ try {
+ localStorage.setItem(STORAGE_KEY, JSON.stringify(appState.progress));
+ } catch {
+ // Keep the session playable even when persistence is unavailable.
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function saveProgress() { | |
| localStorage.setItem(STORAGE_KEY, JSON.stringify(appState.progress)); | |
| } | |
| function saveProgress() { | |
| try { | |
| localStorage.setItem(STORAGE_KEY, JSON.stringify(appState.progress)); | |
| } catch { | |
| // Keep the session playable even when persistence is unavailable. | |
| } | |
| } |
🤖 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 `@mosaic-press/src/app.js` around lines 46 - 48, The saveProgress function
currently writes to localStorage without handling failures; update saveProgress
to mirror loadProgress's tolerant behavior by wrapping the
localStorage.setItem(STORAGE_KEY, JSON.stringify(appState.progress)) call in a
try/catch, and on error either silently ignore or log the error (using the same
logging approach as loadProgress) instead of letting the exception propagate;
reference the saveProgress function, STORAGE_KEY, and appState.progress when
applying the change.
Comment on lines
+500
to
+556
| if (appState.modal === "start") { | ||
| return ` | ||
| <div class="modal-shell"> | ||
| <div class="modal-card"> | ||
| <p class="eyebrow">${levelLabel()}</p> | ||
| <h2>${currentLevel().title}</h2> | ||
| <p>${currentLevel().beat}</p> | ||
| <div class="modal-card__callouts"> | ||
| ${currentLevel().callouts | ||
| .filter((entry) => entry.trigger === "start") | ||
| .map((entry) => `<p>${entry.text}</p>`) | ||
| .join("")} | ||
| </div> | ||
| <div class="modal-card__actions"> | ||
| <button class="button" data-action="start-level">Start Pressing</button> | ||
| <button class="button button--ghost" data-action="back">Back</button> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| `; | ||
| } | ||
|
|
||
| if (appState.modal === "fail") { | ||
| return ` | ||
| <div class="modal-shell"> | ||
| <div class="modal-card"> | ||
| <p class="eyebrow">${appState.puzzle.failReason === "overflow" ? "Gutter Overflow" : "No Move"}</p> | ||
| <h2>${appState.puzzle.message}</h2> | ||
| <p>${currentLevel().callouts.find((entry) => entry.trigger === `fail:${appState.puzzle.failReason}`)?.text ?? "Undo rewinds the last successful fastener removal."}</p> | ||
| <div class="modal-card__actions"> | ||
| <button class="button" data-action="undo">Undo</button> | ||
| <button class="button button--ghost" data-action="restart">Restart</button> | ||
| <button class="button button--ghost" data-action="hint">Hint</button> | ||
| <button class="button button--ghost" data-action="back">Quit</button> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| `; | ||
| } | ||
|
|
||
| return ` | ||
| <div class="modal-shell"> | ||
| <div class="modal-card"> | ||
| <p class="eyebrow">Card Complete</p> | ||
| <h2>${currentLevel().reward?.title ?? "Commission complete"}</h2> | ||
| <p>${currentLevel().reward?.detail ?? "The finished card lifts into the gallery frame."}</p> | ||
| <div class="modal-card__actions"> | ||
| <button class="button" data-action="next-level">${currentLevel().kind === "daily" ? "Back To Gallery" : "Next Card"}</button> | ||
| <button class="button button--ghost" data-action="replay">Replay</button> | ||
| ${ | ||
| isDailyUnlocked() && currentLevel().kind !== "daily" | ||
| ? `<button class="button button--ghost" data-action="open-daily">Daily Commission</button>` | ||
| : "" | ||
| } | ||
| </div> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Treat the overlays as real dialogs and move focus into them.
The start/fail/win modals are rendered visually, but they are not exposed as dialogs and they do not take focus. Keyboard and screen-reader users can remain on background controls behind the overlay, which makes these flows hard to complete accessibly.
🤖 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 `@mosaic-press/src/app.js` around lines 500 - 556, The modals rendered when
appState.modal === "start"/"fail"/(default win) are visual only; update the
modal markup and focus handling so they act as real dialogs: add role="dialog"
and aria-modal="true" to the modal container (e.g., the element with class
"modal-card"), include an aria-labelledby that points to the modal h2, make the
modal container focusable (tabindex="-1") and programmatically move focus into
it when rendered (focus the modal or the first actionable button with
data-action), implement a simple focus trap over elements with data-action to
keep keyboard focus inside while the modal is open, restore focus to the
previously focused element when the modal closes, and ensure Escape closes the
modal (same handlers as the Back/Quit actions).
Comment on lines
+57
to
+73
| .button, | ||
| .hud__button, | ||
| .hud__back { | ||
| border: 0; | ||
| border-radius: 999px; | ||
| background: var(--ink); | ||
| color: #fdf7f0; | ||
| cursor: pointer; | ||
| transition: transform 120ms ease, opacity 120ms ease, background 120ms ease; | ||
| } | ||
|
|
||
| .button:hover, | ||
| .hud__button:hover, | ||
| .hud__back:hover, | ||
| .fastener:hover { | ||
| transform: translateY(-1px); | ||
| } |
There was a problem hiding this comment.
Add visible :focus-visible states for interactive controls.
These controls only get hover styling right now. Keyboard users can tab through the gallery, HUD, and fasteners without a reliable visual focus indicator, which makes the slice hard to operate without a mouse.
Suggested fix
.button:hover,
.hud__button:hover,
.hud__back:hover,
.fastener:hover {
transform: translateY(-1px);
}
+
+.button:focus-visible,
+.hud__button:focus-visible,
+.hud__back:focus-visible,
+.fastener:focus-visible {
+ outline: 3px solid color-mix(in srgb, var(--ink) 35%, white);
+ outline-offset: 3px;
+}Also applies to: 458-469
🤖 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 `@mosaic-press/styles.css` around lines 57 - 73, Add a visible keyboard focus
style for interactive controls: update the selectors for .button, .hud__button,
.hud__back and .fastener to include :focus-visible (e.g. .button:focus-visible,
.hud__button:focus-visible, .hud__back:focus-visible, .fastener:focus-visible)
and apply a clear visual indicator (such as a contrast ring/outline or subtle
box-shadow and the same hover transform) instead of removing the browser
outline; keep transition timing consistent with the hover rules and avoid using
outline: none so keyboard users can see focus.
Comment on lines
+15
to
+38
| test("handcrafted levels follow the expected stack, row, and cell ramp", () => { | ||
| const expected = [ | ||
| [2, 2, 4], | ||
| [2, 2, 5], | ||
| [3, 3, 6], | ||
| [3, 3, 7], | ||
| [3, 3, 8], | ||
| [4, 3, 9], | ||
| [4, 3, 9], | ||
| [4, 3, 10], | ||
| [4, 4, 10], | ||
| [5, 4, 11], | ||
| [5, 4, 12], | ||
| [5, 4, 12] | ||
| ]; | ||
|
|
||
| HANDCRAFTED_LEVELS.forEach((level, index) => { | ||
| assert.deepEqual( | ||
| [level.stacks.length, level.tray.rows.length, level.tray.cells.length], | ||
| expected[index], | ||
| `Unexpected authored counts for level ${level.number}` | ||
| ); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Add a length guard — test silently passes if HANDCRAFTED_LEVELS is shorter than expected.
forEach terminates when the array is exhausted, so if HANDCRAFTED_LEVELS has fewer than 12 entries, none of the "missing" expected rows are ever asserted. The test reports green while the contract is violated.
🛡️ Proposed fix
+ assert.equal(
+ HANDCRAFTED_LEVELS.length,
+ expected.length,
+ `Expected ${expected.length} handcrafted levels, got ${HANDCRAFTED_LEVELS.length}`
+ );
+
HANDCRAFTED_LEVELS.forEach((level, index) => {🤖 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 `@mosaic-press/tests/engine.test.js` around lines 15 - 38, The test
"handcrafted levels follow the expected stack, row, and cell ramp" can silently
pass if HANDCRAFTED_LEVELS is shorter than expected; add a length guard before
iterating by asserting HANDCRAFTED_LEVELS.length === expected.length (or at
least >= expected.length) and fail early with a clear message; then keep the
existing HANDCRAFTED_LEVELS.forEach block to validate per-level counts
(referencing HANDCRAFTED_LEVELS, expected, and the test name).
Comment on lines
+135
to
+137
| test("daily seed uses UTC formatting and generation is deterministic", () => { | ||
| const seed = dailySeedFromDate(new Date("2026-05-04T16:20:00+08:00")); | ||
| assert.equal(seed, "2026-05-04"); |
There was a problem hiding this comment.
UTC normalization is not actually exercised by the chosen timestamp.
2026-05-04T16:20:00+08:00 converts to 2026-05-04T08:20:00Z — both the local date and the UTC date are May 4, so the assertion seed === "2026-05-04" would also pass if the implementation used local time instead of UTC. The test should use a time that crosses midnight in UTC, e.g., 2026-05-05T02:00:00+08:00 → UTC 2026-05-04T18:00:00Z, which must return "2026-05-04" (not "2026-05-05").
🛡️ Proposed fix
- const seed = dailySeedFromDate(new Date("2026-05-04T16:20:00+08:00"));
- assert.equal(seed, "2026-05-04");
+ // 2026-05-05T02:00+08:00 is 2026-05-04T18:00Z — UTC date must win
+ const seed = dailySeedFromDate(new Date("2026-05-05T02:00:00+08:00"));
+ assert.equal(seed, "2026-05-04");🤖 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 `@mosaic-press/tests/engine.test.js` around lines 135 - 137, The test for
dailySeedFromDate doesn't actually validate UTC normalization because the chosen
timestamp keeps local and UTC dates the same; update the test input to a
datetime that crosses UTC midnight (for example use "2026-05-05T02:00:00+08:00")
so that UTC date differs from local date and assert that dailySeedFromDate(new
Date("2026-05-05T02:00:00+08:00")) returns "2026-05-04", ensuring the function's
UTC-based normalization is exercised.
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
mosaic-press/vertical slice with data-driven level, engine, daily, and UI codeTesting
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests
Chores