Skip to content

fix(gottext): write a bare wall clock time as the time it names - #148

Merged
SirLouen merged 1 commit into
mainfrom
fix/147-gottext-wall-clock
Nov 8, 2026
Merged

SirLouen merged 1 commit into
mainfrom
fix/147-gottext-wall-clock

Conversation

@SirLouen

@SirLouen SirLouen commented Nov 8, 2026 •

Copy link
Copy Markdown
Member

Closes #147

What

  • formatTime reads a bare HH:MM, such as 09:30, as the time it names and writes it the same in every time zone.
  • A full timestamp still shows on the reader's clock, and an empty string still writes nothing.

Why

A time stored without a day went to new Date, which cannot read it, so the screen showed Invalid Date. A bare calendar day already keeps the day it names, and a bare time now does the same.

Testing

  • The new tests failed first with Invalid Date: es-ES in Los Angeles (09:30, 21:05, 00:00, 23:59), en-US in Tokyo (09:30 AM, 09:05 PM, 12:00 AM), and a timestamp with and without a zone in Los Angeles.
  • 262 tests at 100 percent coverage, types, lint, knip and the build pass on Node 22 and 26.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed formatting for bare times such as 09:30, which now display consistently across locales and time zones instead of showing an invalid date.
    • Empty input now returns a blank result. Zoned timestamps continue to reflect the reader’s local time, while unzoned timestamps retain their stated time.
  • Documentation
    • Clarified that bare wall-clock times are accepted input.

@greptile-apps

This comment has been minimized.

@coderabbitai

coderabbitai Bot commented Nov 8, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

formatTime now formats bare HH:mm strings as the same wall-clock time across time zones. Tests cover multiple locales, time zones, and timestamp inputs.

Changes: Wall-clock time formatting

Layer / File(s) Summary
Recognize and format wall-clock times
gottext/src/format.ts, gottext/test/format.test.ts, gottext/CHANGELOG.md
formatTime handles empty input and formats bare HH:mm strings using UTC. Tests cover Spanish and English locales, different time zones, and zoned and unzoned timestamps. The changelog records the fix.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: codescene-access

Merge Risk: Minimal · up to 90758

Bare times retain the time they name across the tested locales and time zones. No actionable merge risk remains after normal checks.

Pre-merge checks: Passed 5

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title names the primary change: formatTime now reads a bare wall-clock time as the time it names. The raw summary shows a WALL_CLOCK pattern for bare HH:mm strings, UTC-based formatting for those …
Linked Issues check Passed Issue #147 requires that formatTime show a bare HH:MM time as the time it names, the same in every time zone, with tests on two clocks and in a zone far from UTC.

The diff of `gottext/src/format.…
Out of Scope Changes check Passed The changed files are gottext/src/format.ts, gottext/test/format.test.ts, and gottext/CHANGELOG.md.

- format.ts: the WALL_CLOCK pattern, the UTC branch, and the empty-string guard implement…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Full details: Title check

Explanation

The title names the primary change: formatTime now reads a bare wall-clock time as the time it names. The raw summary shows a WALL_CLOCK pattern for bare HH:mm strings, UTC-based formatting for those strings, a CHANGELOG Fixed entry, and tests for nonmatching time zones. The objectives state the same fix for issue #147. The title follows the conventional commit style, is one sentence, and is specific enough for a teammate to understand the change from history.

Full details: Linked Issues check

Explanation

Issue #147 requires that formatTime show a bare HH:MM time as the time it names, the same in every time zone, with tests on two clocks and in a zone far from UTC.

The diff of gottext/src/format.ts (merge base to head) adds WALL_CLOCK = /^\d{2}:\d{2}$/. For a matching string, formatTime builds new Date(1970-01-01T${at}Z) and formats it with timeZone: 'UTC'. The output therefore matches the stated clock time in any reader zone. Empty input still returns ''. Other inputs keep the previous new Date(at) path with the local clock, so full timestamps still display on the reader clock.

The diff of gottext/test/format.test.ts adds three tests:

  • es-ES under America/Los_Angeles for 09:30, 21:05, 00:00, 23:59.
  • en-US under Asia/Tokyo for 09:30 AM, 09:05 PM, 12:00 AM.
  • A zoned timestamp (2026-09-30T09:05:00Z gives 02:05 in Los Angeles) and an unzoned timestamp (2026-09-30T09:05 gives 09:05).

These tests cover the two clocks and the zone far from UTC that #147 requests. The PR also reports that these tests failed before the fix. I did not run the tests. The exact en-US output strings depend on the ICU space character, so the passing result rests on the PR report.

The linked issue is the only active target. No parent or contextual issue adds coding requirements.

Full details: Out of Scope Changes check

Explanation

The changed files are gottext/src/format.ts, gottext/test/format.test.ts, and gottext/CHANGELOG.md.

  • format.ts: the WALL_CLOCK pattern, the UTC branch, and the empty-string guard implement #147. The doc comment update for at describes the new input. No unrelated logic changed.
  • format.test.ts: the added tests verify the new behavior and the unchanged timestamp behavior. They support the objective.
  • CHANGELOG.md: the Unreleased Fixed entry documents this fix. This is documentation for the change, not an unrelated change.

I could not read the CHANGELOG.md text diff, because the changed blob was unavailable in the checkout. This conclusion for the changelog uses the supplied AI-generated summary, which reports a single added Fixed line. That uncertainty does not change the assessment, because a changelog entry for this fix is in scope.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files. (1 skipped: 1 unsupported.)

Finishing Touches

Generate docstrings

  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: b17d0d0d-069d-4bc2-8c7d-956f59a16627
Commits

Reviewing files that changed from the base of the PR and between 35ccbaf and 90758b0.

Files selected for processing: 3

  • gottext/CHANGELOG.md
  • gottext/src/format.ts
  • gottext/test/format.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

@codescene-access codescene-access Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Gates Passed
4 Quality Gates Passed

See analysis details in CodeScene

Quality Gate Profile: Clean Code Collective
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.

@greptile-apps

greptile-apps Bot commented Nov 8, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] This PR appears safe to merge; no actionable issues were found.

Summary

formatTime now displays bare HH:MM values without shifting them to the reader’s time zone.

  • Bare clock times now keep the hour they name in every time zone.

Reviews (1) · Last reviewed commit: "fix(gottext): write a bare wall clock ti..." · Reviewed by Greptile

@SirLouen
SirLouen merged commit cc7c0a6 into main Nov 8, 2026
50 checks passed
@SirLouen
SirLouen deleted the fix/147-gottext-wall-clock branch November 8, 2026 17:57
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.

formatTime writes Invalid Date for a bare wall clock time

1 participant