Skip to content

Add Loom Rescue first playable slice#1

Merged
wuhuizuo merged 3 commits into
mainfrom
agent/gameplay-engineer/bacfd36b
May 3, 2026
Merged

Add Loom Rescue first playable slice#1
wuhuizuo merged 3 commits into
mainfrom
agent/gameplay-engineer/bacfd36b

Conversation

@wuhuizuo
Copy link
Copy Markdown
Member

@wuhuizuo wuhuizuo commented May 3, 2026

Summary

  • add a standalone loom-rescue/ web prototype for the first playable vertical slice
  • implement data-driven board rules for bundles, pins, guides, crossings, 15 FTUE levels, and a seeded daily template
  • add portrait UI flows for start, play, undo, hint, restart, fail, win, postcard reveal, and milestone stamp progress

Validation

  • npm test

Summary by CodeRabbit

  • New Features
    • Launched Loom Rescue with 15 FTUE levels, seeded daily challenges, deterministic daily generator, progress postcards and milestone stamps, undo/hint flows, and full in-level UI.
  • Styles
    • Complete responsive UI theme, board visuals, modals and animations for desktop & mobile.
  • Documentation
    • Comprehensive README with run/test/verify and preview deployment info.
  • Tests
    • Engine test suite validating levels, solver, undo/hint, and daily determinism.
  • Chores
    • Package scripts for test/verify and CI preview/stable deployment workflow.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 3, 2026

📝 Walkthrough

Walkthrough

Adds a complete "Loom Rescue" vertical slice: data libraries and 15 FTUE levels, seeded daily level generator, a puzzle engine with validation/solver/undo/hint, a single-page client app with SVG board rendering and persistence, styles, tests, README, HTML entry, package metadata, and a GH Actions preview/deploy workflow.

Changes

Loom Rescue Game Implementation

Layer / File(s) Summary
Data Shape & Constants
loom-rescue/src/data.js
Board size, fail/milestone constants, postcard sets, frozen bundle/guide/crossing libraries, pin slots, helper constructors (pickBundles, pickGuides, makeCrossing, makePin), and HANDCRAFTED_LEVELS (15 FTUE levels).
Daily Level API
loom-rescue/src/daily.js
Seed formatting (dailySeedFromDate) plus deterministic PRNG/shuffle and generateDailyLevel(seed) producing reproducible daily level objects (bundles, pins, guides, crossings, postcard, metadata).
Core Engine / Runtime
loom-rescue/src/engine.js
Runtime derivation (expand polylines, infer pull directions, lookup maps), state init/snapshots, legality checks (evaluateBundle, evaluatePin), action collection/scoring, applyAction, undo/restart, findHint, validateLevel, and greedy solveLevel. Utility exports (cellKey, cellLabel, inferEdge, expandPolyline, etc.).
App Wiring & UI Rendering
loom-rescue/src/app.js
Top-level appState, progress persistence (localStorage), level loading (FTUE vs daily), event delegation (bundle/pin actions, undo, hint, navigation), render() that builds hero/album/rail/board SVG/modals/metrics and UI helpers.
Entrypoint & Package
loom-rescue/index.html, loom-rescue/package.json
Static HTML mounting #app and loading ./src/app.js; package metadata (type: "module", scripts: test, check:app, verify).
Styling
loom-rescue/styles.css
Theme tokens, page layout, card/button primitives, detailed SVG/thread/crossing/pin/bundle state styles and animations, modals, postcard variants, and responsive rules for 980px/640px breakpoints.
Tests
loom-rescue/tests/engine.test.js
Node tests asserting FTUE level shape counts, validation + solver success for handcrafted levels, behavior for blocked pulls/knots, undo semantics, deadlock detection, hint returns, and deterministic daily generation.
Docs
loom-rescue/README.md
Run/test/verify instructions, "What Is Included", implementation notes, and preview deployment docs including workflow path and URL patterns.
CI Preview Deployment
.github/workflows/loom-rescue-preview.yml
Workflow to verify and deploy PR previews to gh-pages/previews/pr-<number>/, cleanup on PR close, and deploy stable loom-rescue/ on main.

Sequence Diagram

sequenceDiagram
    participant Player
    participant UI as App/UI
    participant Engine
    participant State
    participant Storage as LocalStorage

    Player->>UI: Open app / Select level
    UI->>Engine: createRuntime(level)
    Engine-->>UI: runtime (bundles, pins, guides, crossings)
    UI->>State: createInitialState(level)
    State-->>UI: initial puzzle state
    UI->>Engine: validateLevel(level)
    Engine-->>UI: validation report
    UI->>UI: render board + controls
    Player->>UI: Pull bundle / Lift pin
    UI->>Engine: evaluateBundle/evaluatePin(action)
    Engine-->>UI: legality {ok,reason}
    alt legal
        UI->>Engine: applyAction(action)
        Engine->>State: mutate (removed, knots, fail/completed)
        Engine-->>UI: updated state + message
        UI->>Storage: saveProgress(progress)
        UI->>UI: render()
        UI-->>Player: show result / modal
    else illegal
        UI-->>Player: show blocker reason
    end
    Player->>UI: Hint / Undo / Restart
    UI->>Engine: collectLegalActions/findHint/undoAction/restartLevel
    Engine-->>UI: hint / restored state / fresh state
    UI->>UI: render()
    UI-->>Player: updated view
Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 I hopped in code where threads entwine,

bundles bowed and pins did shine.
Seeds of puzzles, daily and new,
stamps and postcards, bright and true.
Loom Rescue — a rabbit's playful view.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add Loom Rescue first playable slice' directly and clearly summarizes the main objective of the pull request: introducing a new vertical slice prototype for the Loom Rescue game.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/gameplay-engineer/bacfd36b

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
loom-rescue/README.md (1)

26-27: ⚡ Quick win

Clarify “two-knot daily failure tuning” wording for non-authors.

That phrase reads like internal tuning criteria, but the README doesn’t define what “two-knot” refers to (a rule case? a specific board state? a threshold?). A brief parenthetical would make this much easier for contributors/QA to act on.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@loom-rescue/README.md` around lines 26 - 27, Update the ambiguous phrase
"two-knot daily failure tuning" by adding a short parenthetical definition so
non-authors understand it (e.g., indicate whether it refers to a rule case, a
board state, or a numeric threshold); locate the exact string "two-knot daily
failure tuning" in the README and append a concise clarification like "(i.e., a
[rule/threshold/board-state] defined as ...)" or replace the phrase with an
explicit term and brief explanation so QA and contributors can act on it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@loom-rescue/src/app.js`:
- Around line 235-250: Pins are currently rendered as non-focusable SVG <g>
elements (see the template building the "pin-node" group and evaluatePin usage)
and only respond to mouse clicks, blocking keyboard users; fix by making each
pin focusable and accessible: add tabindex="0", role="button", and an
appropriate aria-label/aria-pressed on the <g> (or child) in the pin render
template, and update the global click delegation logic to also listen for
keydown events (Enter/Space) and invoke the same handler when data-action="pin"
and data-id match; apply the same changes to the other pin rendering block (the
similar code region around the second occurrence) so keyboard activation mirrors
mouse clicks.

In `@loom-rescue/src/engine.js`:
- Around line 328-367: In applyAction, avoid mutating terminal states by
short-circuiting bundle handling when state.failed or state.completed is true:
in the applyAction function (before calling evaluateBundle or before
incrementing knots), detect if state.failed || state.completed and immediately
return state (unchanged) or a shallow copy that preserves
history/message/hintAction exactly; do not call evaluateBundle, do not increment
knots, and do not alter completed/failed/history/message. Ensure the check
references applyAction, state.failed, state.completed, evaluateBundle, knots and
prevents the existing branch that increments knots and resets completed from
running for terminal states.

In `@loom-rescue/styles.css`:
- Around line 645-649: The mobile rule groups .hero-card, .play-layout, and
.level-rail but .level-rail is a flex container so its grid-template-columns
setting is ignored; update that CSS block (the rule that currently lists
.hero-card, .play-layout, .level-rail) to keep grid-template-columns for
.hero-card and .play-layout but set .level-rail to use flex-direction: column
(or add a separate selector for .level-rail) so its children stack on narrow
screens.

---

Nitpick comments:
In `@loom-rescue/README.md`:
- Around line 26-27: Update the ambiguous phrase "two-knot daily failure tuning"
by adding a short parenthetical definition so non-authors understand it (e.g.,
indicate whether it refers to a rule case, a board state, or a numeric
threshold); locate the exact string "two-knot daily failure tuning" in the
README and append a concise clarification like "(i.e., a
[rule/threshold/board-state] defined as ...)" or replace the phrase with an
explicit term and brief explanation so QA and contributors can act on it.
🪄 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: 44e0fa25-04b8-4174-95bb-618f76dc1dd5

📥 Commits

Reviewing files that changed from the base of the PR and between cc0b56d and b81e5fa.

📒 Files selected for processing (9)
  • loom-rescue/README.md
  • loom-rescue/index.html
  • loom-rescue/package.json
  • loom-rescue/src/app.js
  • loom-rescue/src/daily.js
  • loom-rescue/src/data.js
  • loom-rescue/src/engine.js
  • loom-rescue/styles.css
  • loom-rescue/tests/engine.test.js

Comment thread loom-rescue/src/app.js
Comment on lines +235 to +250
const pins = appState.runtime.pins
.filter(
(pin) =>
!isPinRemoved(appState.puzzle, pin.id) &&
pin.blocks.some((bundleId) => !isBundleRemoved(appState.puzzle, bundleId))
)
.map((pin) => {
const evaluation = evaluatePin(appState.level, appState.puzzle, pin.id, appState.runtime);
const x = pin.cell.x * CELL_SIZE + CELL_SIZE / 2;
const y = pin.cell.y * CELL_SIZE + CELL_SIZE / 2;
return `
<g class="pin-node ${evaluation.ok ? "is-ready" : "is-blocked"}" data-action="pin" data-id="${pin.id}" transform="translate(${x} ${y})">
<circle r="22"></circle>
<text text-anchor="middle" y="7">PIN</text>
</g>
`;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Pin actions are mouse-only, which blocks keyboard completion.

Pins are rendered as clickable SVG groups and only wired through click delegation. Keyboard users have no equivalent way to trigger required pin lifts.

♿ Proposed fix (keyboard support for interactive SVG nodes)
-        <g class="pin-node ${evaluation.ok ? "is-ready" : "is-blocked"}" data-action="pin" data-id="${pin.id}" transform="translate(${x} ${y})">
+        <g
+          class="pin-node ${evaluation.ok ? "is-ready" : "is-blocked"}"
+          data-action="pin"
+          data-id="${pin.id}"
+          role="button"
+          tabindex="0"
+          aria-label="Lift ${pin.name}"
+          transform="translate(${x} ${y})"
+        >
           <circle r="22"></circle>
           <text text-anchor="middle" y="7">PIN</text>
         </g>
 root.addEventListener("click", (event) => {
   const target = event.target.closest("[data-action]");
   if (!target) {
     return;
   }
@@
   if (action === "next-level") {
     openNextLevel();
   }
 });
+
+root.addEventListener("keydown", (event) => {
+  const target = event.target;
+  if (!(target instanceof Element)) {
+    return;
+  }
+  const actionTarget = target.closest("[data-action]");
+  if (!actionTarget) {
+    return;
+  }
+  if (event.key === "Enter" || event.key === " ") {
+    event.preventDefault();
+    actionTarget.click();
+  }
+});

Also applies to: 547-582

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@loom-rescue/src/app.js` around lines 235 - 250, Pins are currently rendered
as non-focusable SVG <g> elements (see the template building the "pin-node"
group and evaluatePin usage) and only respond to mouse clicks, blocking keyboard
users; fix by making each pin focusable and accessible: add tabindex="0",
role="button", and an appropriate aria-label/aria-pressed on the <g> (or child)
in the pin render template, and update the global click delegation logic to also
listen for keydown events (Enter/Space) and invoke the same handler when
data-action="pin" and data-id match; apply the same changes to the other pin
rendering block (the similar code region around the second occurrence) so
keyboard activation mirrors mouse clicks.

Comment thread loom-rescue/src/engine.js
Comment on lines +328 to +367
export function applyAction(level, state, action, runtime = createRuntime(level)) {
if (action.type === "pin") {
const evaluation = evaluatePin(level, state, action.id, runtime);
if (!evaluation.ok) {
return {
...state,
message: evaluation.reason,
hintAction: null
};
}

const pin = runtime.pinMap.get(action.id);
const nextState = {
...state,
removedPins: [...state.removedPins, action.id],
history: [...state.history, snapshotState(state)],
message: `${pin.name} lifts away.`,
hintAction: null
};
return finalizeProgress(level, nextState);
}

const evaluation = evaluateBundle(level, state, action.id, runtime);
const bundle = runtime.bundleMap.get(action.id);

if (!evaluation.ok) {
const knots = state.knots + 1;
const failed = knots >= level.failLimit;
return {
...state,
knots,
failed,
completed: false,
history: [...state.history, snapshotState(state)],
hintAction: null,
message: failed
? `${evaluation.reason} Knot ${knots}/${level.failLimit}. The weave locks shut.`
: `${evaluation.reason} Knot ${knots}/${level.failLimit}.`
};
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Prevent terminal-state mutation in applyAction.

When state.failed or state.completed is true, invalid bundle actions still increment knots (Line 354) and can reset completed (Line 360). This corrupts terminal state semantics.

🛠️ Proposed fix
 export function applyAction(level, state, action, runtime = createRuntime(level)) {
+  if (state.failed || state.completed) {
+    return {
+      ...state,
+      message: "The level is no longer active.",
+      hintAction: null
+    };
+  }
+
   if (action.type === "pin") {
     const evaluation = evaluatePin(level, state, action.id, runtime);
     if (!evaluation.ok) {
       return {
         ...state,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@loom-rescue/src/engine.js` around lines 328 - 367, In applyAction, avoid
mutating terminal states by short-circuiting bundle handling when state.failed
or state.completed is true: in the applyAction function (before calling
evaluateBundle or before incrementing knots), detect if state.failed ||
state.completed and immediately return state (unchanged) or a shallow copy that
preserves history/message/hintAction exactly; do not call evaluateBundle, do not
increment knots, and do not alter completed/failed/history/message. Ensure the
check references applyAction, state.failed, state.completed, evaluateBundle,
knots and prevents the existing branch that increments knots and resets
completed from running for terminal states.

Comment thread loom-rescue/styles.css
Comment on lines +645 to +649
.hero-card,
.play-layout,
.level-rail {
grid-template-columns: 1fr;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix ineffective mobile rule on .level-rail.

Line 647 applies grid-template-columns to .level-rail, but .level-rail is a flex container, so this rule is ignored. Add a flex-direction change here to actually stack content on narrow screens.

💡 Proposed fix
 `@media` (max-width: 980px) {
   body {
     padding: 14px;
   }

   .hero-card,
-  .play-layout,
-  .level-rail {
+  .play-layout {
     grid-template-columns: 1fr;
   }

-  .level-rail,
+  .level-rail,
   .daily-rail {
     align-items: stretch;
   }

+  .level-rail {
+    flex-direction: column;
+  }
+
   .daily-rail {
     flex-direction: column;
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@loom-rescue/styles.css` around lines 645 - 649, The mobile rule groups
.hero-card, .play-layout, and .level-rail but .level-rail is a flex container so
its grid-template-columns setting is ignored; update that CSS block (the rule
that currently lists .hero-card, .play-layout, .level-rail) to keep
grid-template-columns for .hero-card and .play-layout but set .level-rail to use
flex-direction: column (or add a separate selector for .level-rail) so its
children stack on narrow screens.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 3, 2026

Loom Rescue preview deployed.

URL: https://NightProgrammers.github.io/games/previews/pr-1/
Commit: 79dcff6
Path: previews/pr-1/

If this is the first deployment, enable GitHub Pages once in repository settings and point it at the gh-pages branch.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
loom-rescue/src/engine.js (1)

485-548: ⚡ Quick win

Validate authored coordinates against BOARD bounds.

validateLevel() checks references and path membership, but it never rejects out-of-range bundle points or off-board pin/guide/crossing cells. A typo like x: 6 or y: -1 can still pass these checks and just render outside the board. Since this validator is the safety net for data-driven content, it should fail those levels explicitly.

🧭 Suggested guard
+  const inBounds = ({ x, y }) =>
+    Number.isInteger(x) &&
+    Number.isInteger(y) &&
+    x >= 0 &&
+    x < runtime.board.cols &&
+    y >= 0 &&
+    y < runtime.board.rows;
+
   const bundleIds = new Set();
   for (const bundle of runtime.bundles) {
+    if (!bundle.points.every(inBounds)) {
+      issues.push(`Bundle ${bundle.id} leaves the board.`);
+    }
     if (bundleIds.has(bundle.id)) {
       issues.push(`Duplicate bundle id ${bundle.id}.`);
     }
@@
   const pinIds = new Set();
   for (const pin of runtime.pins) {
+    if (!inBounds(pin.cell)) {
+      issues.push(`Pin ${pin.id} is placed outside the board.`);
+    }
     if (pinIds.has(pin.id)) {
       issues.push(`Duplicate pin id ${pin.id}.`);
     }
@@
   for (const guide of runtime.guides) {
+    if (!guide.cells.every(inBounds)) {
+      issues.push(`Guide ${guide.id} leaves the board.`);
+    }
     for (const bundleId of guide.bundleIds) {
@@
   for (const crossing of runtime.crossings) {
+    if (!inBounds(crossing.cell)) {
+      issues.push(`Crossing ${crossing.id} is placed outside the board.`);
+    }
     if (!runtime.bundleMap.has(crossing.over) || !runtime.bundleMap.has(crossing.under)) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@loom-rescue/src/engine.js` around lines 485 - 548, Add explicit bounds checks
against runtime.board for all authored coordinates: in the bundle loop (where
inferBundleEdge is called) validate every bundle.points coordinate is within 0
<= x < runtime.board.width and 0 <= y < runtime.board.height and push an issue
like `Bundle ${bundle.id} has out-of-bounds point ${cellKey(point)}`; in the pin
loop validate pin.cell similarly before using cellKey(pin.cell) and reject pins
with out-of-range cells; in the guide loop validate each guide.cells entry
before comparing with runtime.pathCellSets and report `Guide ${guide.id} has
out-of-bounds cell ${cellKey(cell)}`; and in the crossing loop validate
crossing.cell before checking overPath/underPath and report out-of-range
crossings — use the existing runtime.board dimensions and reuse cellKey for
clear, consistent messages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/loom-rescue-preview.yml:
- Around line 65-73: The template literal assigned to the body variable (with
marker = "<!-- loom-rescue-preview -->") contains leading indentation that gets
preserved and causes the PR comment to render as a code block; remove the extra
leading spaces inside the template literal so the text lines start at column 0
(or build the string using concatenation/join on an array of unindented lines)
so the comment renders as normal prose and links rather than an indented
Markdown block.

In `@loom-rescue/src/app.js`:
- Around line 53-55: The saveProgress() function currently calls
localStorage.setItem(...) without guarding against exceptions; wrap the
persistence call in a try/catch inside saveProgress() so storage failures are
swallowed (or logged) and do not propagate, ensuring recordWin() can complete
UI/state updates even if setItem throws; reference the saveProgress() function
(and callers like recordWin()) and ensure any caught error is non-fatal (e.g.,
console.warn or appLogger.warn) and does not rethrow.
- Around line 647-650: The UI currently uses appState.validation.ok to display
"This board validates from data with one legal solution path available.", which
incorrectly claims uniqueness; change the conditional text in the info card (the
code controlling appState.validation.ok and the validator-copy paragraph that
uses renderSolutionSequence()) to a neutral phrasing such as "Board passes
validation; a solution path was found." or "Board validated; at least one legal
solution path found." so it no longer asserts there is exactly one legal path.

---

Nitpick comments:
In `@loom-rescue/src/engine.js`:
- Around line 485-548: Add explicit bounds checks against runtime.board for all
authored coordinates: in the bundle loop (where inferBundleEdge is called)
validate every bundle.points coordinate is within 0 <= x < runtime.board.width
and 0 <= y < runtime.board.height and push an issue like `Bundle ${bundle.id}
has out-of-bounds point ${cellKey(point)}`; in the pin loop validate pin.cell
similarly before using cellKey(pin.cell) and reject pins with out-of-range
cells; in the guide loop validate each guide.cells entry before comparing with
runtime.pathCellSets and report `Guide ${guide.id} has out-of-bounds cell
${cellKey(cell)}`; and in the crossing loop validate crossing.cell before
checking overPath/underPath and report out-of-range crossings — use the existing
runtime.board dimensions and reuse cellKey for clear, consistent messages.
🪄 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: 14f22731-19b0-41e8-a0a1-9cbe7949d158

📥 Commits

Reviewing files that changed from the base of the PR and between b81e5fa and 7a43f4f.

📒 Files selected for processing (9)
  • .github/workflows/loom-rescue-preview.yml
  • loom-rescue/README.md
  • loom-rescue/package.json
  • loom-rescue/src/app.js
  • loom-rescue/src/daily.js
  • loom-rescue/src/data.js
  • loom-rescue/src/engine.js
  • loom-rescue/styles.css
  • loom-rescue/tests/engine.test.js
✅ Files skipped from review due to trivial changes (1)
  • loom-rescue/README.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • loom-rescue/package.json
  • loom-rescue/tests/engine.test.js

Comment on lines +65 to +73
const marker = "<!-- loom-rescue-preview -->";
const body = `${marker}
Loom Rescue preview deployed.

URL: ${process.env.PREVIEW_URL}
Commit: ${context.sha.slice(0, 7)}
Path: \`${process.env.PREVIEW_DIR}/\`

If this is the first deployment, enable GitHub Pages once in repository settings and point it at the \`gh-pages\` branch.`;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Build the PR comment body without leading indentation.

Those spaces are preserved inside the template literal, so the bot comment renders as an indented Markdown code block instead of normal prose/links.

📝 Suggested fix
-            const body = `${marker}
-            Loom Rescue preview deployed.
-
-            URL: ${process.env.PREVIEW_URL}
-            Commit: ${context.sha.slice(0, 7)}
-            Path: \`${process.env.PREVIEW_DIR}/\`
-
-            If this is the first deployment, enable GitHub Pages once in repository settings and point it at the \`gh-pages\` branch.`;
+            const body = [
+              marker,
+              "Loom Rescue preview deployed.",
+              "",
+              `URL: ${process.env.PREVIEW_URL}`,
+              `Commit: ${context.sha.slice(0, 7)}`,
+              `Path: \`${process.env.PREVIEW_DIR}/\``,
+              "",
+              "If this is the first deployment, enable GitHub Pages once in repository settings and point it at the `gh-pages` branch."
+            ].join("\n");
📝 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
const marker = "<!-- loom-rescue-preview -->";
const body = `${marker}
Loom Rescue preview deployed.
URL: ${process.env.PREVIEW_URL}
Commit: ${context.sha.slice(0, 7)}
Path: \`${process.env.PREVIEW_DIR}/\`
If this is the first deployment, enable GitHub Pages once in repository settings and point it at the \`gh-pages\` branch.`;
const marker = "<!-- loom-rescue-preview -->";
const body = [
marker,
"Loom Rescue preview deployed.",
"",
`URL: ${process.env.PREVIEW_URL}`,
`Commit: ${context.sha.slice(0, 7)}`,
`Path: \`${process.env.PREVIEW_DIR}/\``,
"",
"If this is the first deployment, enable GitHub Pages once in repository settings and point it at the `gh-pages` branch."
].join("\n");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/loom-rescue-preview.yml around lines 65 - 73, The template
literal assigned to the body variable (with marker = "<!-- loom-rescue-preview
-->") contains leading indentation that gets preserved and causes the PR comment
to render as a code block; remove the extra leading spaces inside the template
literal so the text lines start at column 0 (or build the string using
concatenation/join on an array of unindented lines) so the comment renders as
normal prose and links rather than an indented Markdown block.

Comment thread loom-rescue/src/app.js
Comment on lines +53 to +55
function saveProgress() {
localStorage.setItem(STORAGE_KEY, JSON.stringify(appState.progress));
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don't let persistence failures abort the win flow.

loadProgress() already treats storage as fallible, but saveProgress() does not. localStorage.setItem() can throw in quota-restricted or storage-disabled contexts, and recordWin() calls this on completion, so a successful clear can crash before progress and modal state finish updating.

💾 Suggested fix
 function saveProgress() {
-  localStorage.setItem(STORAGE_KEY, JSON.stringify(appState.progress));
+  try {
+    localStorage.setItem(STORAGE_KEY, JSON.stringify(appState.progress));
+  } catch {
+    // Non-blocking: gameplay should continue even if persistence is unavailable.
+  }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@loom-rescue/src/app.js` around lines 53 - 55, The saveProgress() function
currently calls localStorage.setItem(...) without guarding against exceptions;
wrap the persistence call in a try/catch inside saveProgress() so storage
failures are swallowed (or logged) and do not propagate, ensuring recordWin()
can complete UI/state updates even if setItem throws; reference the
saveProgress() function (and callers like recordWin()) and ensure any caught
error is non-fatal (e.g., console.warn or appLogger.warn) and does not rethrow.

Comment thread loom-rescue/src/app.js
Comment on lines +647 to +650
<section class="info-card">
<p class="info-card__label">Validator Notes</p>
<p class="validator-copy">${appState.validation.ok ? "This board validates from data with one legal solution path available." : "Validator warning."}</p>
<p class="validator-sequence">${renderSolutionSequence()}</p>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Avoid claiming the validator proved uniqueness.

validateLevel() only shows that the board passed checks and that a solve sequence was found. It does not prove there's exactly one legal path, and that wording is already contradicted by Level 13's "Multiple valid openings".

✏️ Suggested copy change
-            <p class="validator-copy">${appState.validation.ok ? "This board validates from data with one legal solution path available." : "Validator warning."}</p>
+            <p class="validator-copy">${appState.validation.ok ? "This board validates from data with at least one legal solution path available." : "Validator warning."}</p>
📝 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
<section class="info-card">
<p class="info-card__label">Validator Notes</p>
<p class="validator-copy">${appState.validation.ok ? "This board validates from data with one legal solution path available." : "Validator warning."}</p>
<p class="validator-sequence">${renderSolutionSequence()}</p>
<section class="info-card">
<p class="info-card__label">Validator Notes</p>
<p class="validator-copy">${appState.validation.ok ? "This board validates from data with at least one legal solution path available." : "Validator warning."}</p>
<p class="validator-sequence">${renderSolutionSequence()}</p>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@loom-rescue/src/app.js` around lines 647 - 650, The UI currently uses
appState.validation.ok to display "This board validates from data with one legal
solution path available.", which incorrectly claims uniqueness; change the
conditional text in the info card (the code controlling appState.validation.ok
and the validator-copy paragraph that uses renderSolutionSequence()) to a
neutral phrasing such as "Board passes validation; a solution path was found."
or "Board validated; at least one legal solution path found." so it no longer
asserts there is exactly one legal path.

@wuhuizuo wuhuizuo merged commit 95c18dc into main May 3, 2026
4 checks passed
@wuhuizuo wuhuizuo deleted the agent/gameplay-engineer/bacfd36b branch May 3, 2026 16:47
github-actions Bot added a commit that referenced this pull request May 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant