Skip to content

fix: spell every weekday correctly in the Routines schedule preview - #369

Open
Sma1lboy wants to merge 1 commit into
mainfrom
claude/keen-einstein-cmamw9
Open

fix: spell every weekday correctly in the Routines schedule preview#369
Sma1lboy wants to merge 1 commit into
mainfrom
claude/keen-einstein-cmamw9

Conversation

@Sma1lboy

@Sma1lboy Sma1lboy commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Direction

Bugs / correctness — self-found during a review of the recently-shipped Routines / cron-composer slice.

Problem

The Routines composer restates a schedule in plain English in its live preview (describeCron, rendered at automation-composer-dialog.tsx). For a single-day schedule it built the plural by taking the three-letter cron code, lowercasing the tail, and appending days:

`${dow.charAt(0)}${dow.slice(1).toLowerCase()}days ${at}`

That only produces correct English for MON/FRI/SUN. The other four are mangled:

cron preview showed should be
TUE Tuedays Tuesdays
WED Weddays Wednesdays
THU Thudays Thursdays
SAT Satdays Saturdays

Every one of these is reachable in the UI — the weekday cell's DOW_LADDER contains all seven days, so a single ↑/↓ step lands the draft on a mangled value, and the composer renders the preview verbatim. So a majority of weekday schedules showed a misspelled restatement in the one field whose entire job is to restate the schedule correctly.

Fix

Replace the mechanical code + "days" with a full-name lookup table (DOW_PLURAL) in packages/kobe/src/tui/component/cron-segments.ts. Pure, single-function change; no behavior change for the shapes that were already correct.

Verification

  • Added describeCron cases for TUE/WED/THU/FRI/SAT/SUN to packages/kobe/test/tui/cron-segments.test.ts.
  • bun test packages/kobe/test/tui/cron-segments.test.ts — 15 pass / 0 fail.
  • bun run lint — clean. bun run typecheck — clean (all three packages).
  • Changeset added (patch).

Follow-ups (deliberately out of scope)

Two unrelated bugs surfaced in the adjacent work-items slice during the same review and are left for separate PRs, since that slice is a different scope (and is CLI-only / not yet on the rail):

  • work-items-page.tsx: reloadTick > 0 makes refresh: true sticky after the first r, so every later repo-switch / filter-toggle bypasses the daemon's 60s cache instead of feeling instant.
  • work-items.ts classifyGhFailure: a repo with only a non-GitHub remote is misreported as "not authenticated" instead of "no GitHub remote" (checks auth before no-remote and omits gh's none of the git remotes wording — the tested sibling pr-status.ts gets the order and patterns right).

Generated by Claude Code

The composer restated a single-day schedule by appending "days" to the
three-letter cron code, which mangles four of the seven — TUE→"Tuedays",
WED→"Weddays", THU→"Thudays", SAT→"Satdays". Each is one arrow-step away
in the weekday cell, so the live preview showed the wrong plural for a
majority of weekday schedules. Restate from a full-name lookup instead.
@vercel

vercel Bot commented Aug 1, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
kobe-landing Error Error Aug 1, 2026 4:18pm

Request Review

@claude

claude Bot commented Aug 1, 2026

Copy link
Copy Markdown

Claude finished @Sma1lboy's task in 3m 5s —— View job


Verdict: ✅ Clean, correctly-scoped bug fix — no blocking issues, safe to merge

House rules: all pass — commit/PR body carry no AI/Anthropic attribution, changeset is present and correctly scoped to patch, both touched files are far under the 500-line cap (129 and 115 lines), and the new test genuinely regression-pins the bug (hand-verified: the old ${dow.charAt(0)}${dow.slice(1).toLowerCase()}days formula produces "Tuedays"/"Weddays"/"Thudays"/"Satdays" for TUE/WED/THU/SAT — exactly the four cases the new test asserts against — and would fail pre-fix, pass post-fix).

Six specialized review passes (code-reviewer, silent-failure-hunter, pr-test-analyzer, type-design-analyzer, comment-analyzer, code-simplifier) converged on the same read: the diff is a minimal, correct, well-tested fix for exactly the bug described, no architectural or UX concerns, and the DOW_PLURAL lookup-table shape is already the simplest reasonable implementation (reusing DOW_LADDER positionally would trade clarity for coupling, for no gain).

Nits (non-blocking)

  • packages/kobe/src/tui/component/cron-segments.ts:113 — the new weekday branch normalizes via dow.toUpperCase() before the lookup, but the sibling branches just above it (dow === "MON-FRI", dow === "SAT,SUN", line 111-112) are still case-sensitive. A hand-typed 0 9 * * mon now restates as "Mondays…" while 0 9 * * mon-fri still returns null — an inconsistency this PR widens slightly (though it doesn't regress anything: previously all lowercase input was silently rejected). Cheap follow-up: hoist const day = dow.toUpperCase() once and compare day in all four branches. Not worth blocking on — the composer's own ladder always emits uppercase (stepSegment uppercases first), so this only affects hand-typed cron text, which isn't a supported entry point today.
  • packages/kobe/src/tui/component/cron-segments.ts:41DOW_PLURAL: Readonly<Record<string, string>> uses a loose string key, whereas the LADDERS: Record<CronSegment, readonly string[]> a few lines below types its keys to a closed union. A type DowCode = "MON" | "TUE" | ... | "SUN" + Record<DowCode, string> would give a compile-time guarantee against a dropped/typo'd key — exactly the failure class this PR just fixed by hand. Low severity (private 7-entry table, already exhaustively tested, guarded by if (plural) at the call site) — worth a fast follow, not this PR.

@codecov

codecov Bot commented Aug 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.90909% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
packages/kobe/src/tui/component/cron-segments.ts 90.90% 1 Missing ⚠️
Files with missing lines Coverage Δ
packages/kobe/src/tui/component/cron-segments.ts 92.30% <90.90%> (+1.13%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

2 participants