Slots refactor

[theahmadzai]

No description provided.

[doron2402]

Code Review — PR #1 (time-slots)

This PR was opened 2026-02-20T07:12:07Z ("Slots refactor"). It sits on the same auxiliary time-slots repo as PR #2 ("Debug"), and both have been open without an obvious update since.

Recommendation: Same as PR #2 — confirm whether this is still relevant. If yes, rebase + add a clear description (what's the old shape, what's the new shape, why the refactor). If no, close.

A "refactor" PR open across multiple months is risky on its own — even if the diff was clean when opened, every day adds drift against main, and merging an old refactor on top of new feature commits routinely hides regressions in the merge resolution.

— Automated review by Claude

[doron2402]

Code Review — PR #1

21-file refactor (+712 / –25). Open since Feb 2026.

Security: No concerns visible in the configuration / scaffolding changes (.nvmrc, biome.json, .vscode). The substantive logic is in src/slots/*.ts files (generate-slots.ts, pipeline.ts, resolve-shifts.ts, transforms) — recommend a targeted security review there for any user-controlled date/time inputs that flow into time-zone math (timezone strings can be a vector for prototype-pollution-via-string-keys patterns).

Clean Code:

• Adoption of Biome formatter + linter with consistent indent/line-width is good.
• File structure (slots/transforms/*, pipeline.ts, resolve-shifts.ts) reads as a clean pipeline-of-transforms pattern — verb-named files, single-responsibility per file. ✅
• .claude/worktrees/busy-cray submodule was added — confirm this is intentional and not committed by accident.
• .nvmrc pin to v22.21.1 is good (reproducible builds).

Test Coverage: ⚠️ I don't see a tests/ or *.spec.ts directory in the file list. A slot-generation library is exactly the kind of code where unit tests are critical (date math, DST, timezones, weekday filters, prep-time padding all have edge cases). Strongly recommend adding tests for each transform (filter-by-weekday, filter-past, prep-time, restrict-dates) and the full pipeline before merging.

Performance: The transform pipeline pattern is naturally O(n) per step. As long as generate-dates/generate-slots doesn't materialize huge ranges (e.g. months × minutes × stores), it should be fine. Worth a benchmark on a realistic worst-case window.

Bugs: Cannot evaluate the actual logic without reading src/slots/*.ts. Specific things to verify:

• DST transitions (do generated slots double-count or skip the spring-forward / fall-back hour?)
• Timezone-aware "today" calculations (using merchant timezone, not server's)
• filter-past: edge at exactly now — included or excluded?

Assessment: Minor suggestions — approve with comments ⚠️ — Add unit tests for each transform + DST-aware integration tests, and double-check the .claude/worktrees/busy-cray submodule was meant to land here.

Automated review by Claude — review based on visible config changes; the slots/* TS files weren't fully loaded in the file viewer.