Correct year, ordinal, and meridiem date-mask formatting - #6964
Conversation
🦋 Changeset detectedLatest commit: 927c9c1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Important
The regression test is accurate, but the implementation fixes promised by the PR title and commit subject have not been applied to packages/effect/src/unstable/cli/Prompt.ts yet. Merging this branch as-is will fail CI.
Reviewed changes — this run reviewed the single added test in packages/effect/test/unstable/cli/Prompt.test.ts that reproduces the three date-mask rendering bugs identified in the audit.
- Add
Prompt.dateregression test — exercisesYYtwo-digit year,Doordinal for day 11, andAmeridiem at noon, expecting24 11th PM.
⚠️ Implementation fixes are missing
The test fails against current HEAD because Prompt.ts still contains all three bugs the PR title claims to correct:
Year.toString()usesyear.substring(-2), which returns the full four-digit year. Useyear.slice(-2)instead.ordinalIndicator()omits the11/12/13exception and renders day 11 as"11st". Return"th"for days 11–13.Meridiem.toString()checksgetHours() > 12, so noon (hour 12) renders asAM. Use>= 12instead.
ℹ️ Nitpicks
- The repo convention is to use
Clock/TestClockinstead ofnew Datein tests. Consider constructing the fixture through the EffectClockso the test follows the existing style guide.
@v0 or keep the SHA fresh with Dependabot | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using Kimi K2 (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — this run reviewed the second commit 927c9c1, which applies the implementation fixes promised by the PR title on top of the previously-reviewed regression test.
- Applied the three
Prompt.tsfixes — switchedYear.toString()fromsubstring(-2)toslice(-2), added the11–13ordinal exception inordinalIndicator(), and changed the meridiem check from> 12to>= 12. - Updated the regression test fixture — replaced the direct
new Dateconstruction withDateTime.makeUnsafefollowed byDateTime.toDateUtc, aligning with the repo's preference forDateTime/Clockabstractions in tests. - Added the patch changeset —
.changeset/tidy-dates-smile.mdrecords theeffectpatch.
The focused Prompt.date test passes locally.
@v0 or keep the SHA fresh with Dependabot | View workflow run | Using Kimi K2 (free via Pullfrog for OSS) | 𝕏
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|

Summary
YYdate-mask tokens as two-digit yearsthfor the 11th, 12th, and 13thPrompt.test.tsfileeffectValidation
pnpm test --run packages/effect/test/unstable/cli/Prompt.test.tspnpm lint-fixpnpm checkAudit provenance
c9b56ab507f224426ee8388dc450da447ec4715funstable-ai-cli-prompt-date-mask-renderingCloses EFF-410