feat(clp-s::timestamp_parser): Add support for preserving leap seconds in date-time timestamps. by gibber9809 · Pull Request #1673 · y-scope/clp

gibber9809 · 2025-11-26T16:46:57Z

Description

This is the final (for now) PR for implementing the new clp_s::timestamp_parser utility. This PR follows up on #1646, #1626, #1599, #1564, and #1385.

This PR implements support for preserving leap-seconds in date-time timestamps. This is accomplished by:

Modifying the \S format specifier to only accept seconds in the range 00-59
Adding the \J format specifier which accepts the second value 60, always marshalls the seconds value as 60, and parses the seconds value as 59 (since we store timestamps in epoch nanosecond time, which doesn't count leap-seconds).
Adding the \s CAT sequence which transforms into \S or \J depending on the seconds value.
Modifying all of the default format specifiers to use \s instead of \S.

Note that while this can make timestamps during a leap second and prior to a leap second unordered in epoch time (which is what we store), it is possible to accurately transform each timestamp into UTC time, in which the timestamps are properly ordered.

This solution is explained in more detail in the timestamp parser design doc.

Checklist

The PR satisfies the contribution guidelines.
This is a breaking change and that has been indicated in the PR title, OR this isn't a
breaking change.
Necessary docs have been updated, OR no docs need to be updated.

Validation performed

Added tests validating that timestamps directly before, during, and after a leap second are transformed into epoch time as expected, and are round-tripped correctly.

Summary by CodeRabbit

New Features
- Added leap-second support across parsing and marshalling, enabling 60-second timestamps and a dedicated leap-second specifier.
- Introduced a generic zero-padded second specifier to improve pattern recognition and dynamic substitution between regular and leap seconds.
Bug Fixes
- Adjusted standard second validation to 00–59 and harmonized second-handling behavior.
Tests
- Expanded tests to cover leap-second and end-of-year timestamp scenarios.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-11-26T16:47:08Z

Walkthrough

Adds leap-second support: introduces J (leap second = 60), a generic s specifier (00–60) that resolves to S or J, tightens S to 00–59, and updates marshalling, pattern creation, parsing, pattern-replacement recording, and tests to handle leap-second cases.

Changes

Cohort / File(s)	Change Summary
Core implementation `components/core/src/clp_s/timestamp_parser/TimestampParser.cpp`	Introduced `cLeapSecond = 60` and lowered `cMaxParsedSecond` to 59. Marshaling emits `J` for leap seconds; parsing accepts `J` (60) and generic `s` (00–60), validates and normalizes leap-second values, records replacements mapping `s`→`S` or `J`, and reconstructs final patterns accordingly. Default date-time pattern set updated for `s` substitutions and leap-second awareness.
Header documentation `components/core/src/clp_s/timestamp_parser/TimestampParser.hpp`	Updated spec docs: `\S` is now zero-padded non-leap second (00–59), added `\J` for leap second (60), and documented `\s` as a generic zero-padded second (00–60) that resolves to `\S` or `\J`. No API signature changes.
Tests `components/core/src/clp_s/timestamp_parser/test/test_TimestampParser.cpp`	Adjusted `S` valid range to end at 59; added `J` (60) cases and `\s` transformation tests (00–59 → `\S`, 60 → `\J`); expanded timestamp parsing tests to include leap-second and adjacent end-of-year timestamps and updated expected transformed patterns.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Inspect parsing logic that accepts s and records replacements transforming patterns to S or J.
Verify marshaling emits J/60 only for leap-second semantics and that non-leap behavior remains unchanged.
Review default pattern updates for backward compatibility and correct literal placement.
Run and validate expanded tests covering leap-second boundaries (e.g., 2016-12-31T23:59:60,... and adjacent timestamps).

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check	✅ Passed	The title accurately and specifically describes the main change: adding support for preserving leap seconds in date-time timestamps, which is the core focus of all modifications across the three files.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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

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

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 55f2c9c and 291dae3.

📒 Files selected for processing (3)

components/core/src/clp_s/timestamp_parser/TimestampParser.cpp (7 hunks)
components/core/src/clp_s/timestamp_parser/TimestampParser.hpp (2 hunks)
components/core/src/clp_s/timestamp_parser/test/test_TimestampParser.cpp (3 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}

⚙️ CodeRabbit configuration file

Prefer false == <expression> rather than !<expression>.

Files:

components/core/src/clp_s/timestamp_parser/TimestampParser.hpp
components/core/src/clp_s/timestamp_parser/test/test_TimestampParser.cpp
components/core/src/clp_s/timestamp_parser/TimestampParser.cpp

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)

GitHub Check: ubuntu-jammy-static-linked-bins
GitHub Check: ubuntu-jammy-dynamic-linked-bins
GitHub Check: ubuntu-jammy-lint
GitHub Check: centos-stream-9-static-linked-bins
GitHub Check: centos-stream-9-dynamic-linked-bins
GitHub Check: musllinux_1_2-x86_64-static-linked-bins
GitHub Check: package-image
GitHub Check: musllinux_1_2-x86_64-dynamic-linked-bins
GitHub Check: manylinux_2_28-x86_64-static-linked-bins
GitHub Check: manylinux_2_28-x86_64-dynamic-linked-bins
GitHub Check: conventional-commits
GitHub Check: lint-check (macos-15)
GitHub Check: lint-check (ubuntu-24.04)
GitHub Check: build-macos (macos-15, false)
GitHub Check: build-macos (macos-15, true)

🔇 Additional comments (9)

components/core/src/clp_s/timestamp_parser/TimestampParser.hpp (1)

109-110: LGTM! Documentation updates are clear and consistent.

The updated specifier documentation correctly reflects the new leap-second handling:

\S now limited to 00-59

\J for leap second value 60

\s as a generic CAT sequence that resolves to either \S or \J

Also applies to: 128-128

components/core/src/clp_s/timestamp_parser/test/test_TimestampParser.cpp (3)

255-259: LGTM! Test coverage for seconds specifiers is comprehensive.

The test correctly validates:

\S accepts only 00-59

\J accepts only 60 (leap second)

412-420: LGTM! Generic second transformation tests cover all edge cases.

Good coverage of boundary conditions: 00, 01, 58, 59 map to \S, and 60 maps to \J.

513-516: LGTM! Leap second parsing tests verify correct epoch time mapping.

The tests appropriately verify that:

23:59:59 and 23:59:60 map to the same epoch nanoseconds (as leap seconds are stored as second 59)

00:00:00 on the next day has the expected epoch time

components/core/src/clp_s/timestamp_parser/TimestampParser.cpp (5)

44-45: LGTM! Constant definitions are clear and well-named.

Good separation between the maximum regular second (59) and the leap second value (60).

116-137: LGTM! Default patterns correctly updated to use generic second specifier.

Using \s instead of \S enables automatic leap-second detection across all default date-time patterns.

632-635: LGTM! Leap second marshalling outputs the correct value.

The 'J' specifier correctly outputs 60 for the leap second.

850-852: LGTM! New specifiers correctly registered in pattern creation.

Both 'J' (leap second) and 's' (generic second) are properly marked as requiring date-type representation.

Also applies to: 904-906

1219-1238: LGTM! Leap second parsing logic is correct.

The 'J' specifier correctly:

Validates that the parsed value is exactly 60

Maps leap second to cMaxParsedSecond (59) for storage

…ap-seconds

LinZhihao-723

Overall lgtm. Some nit comments.

LinZhihao-723 · 2025-11-30T00:41:10Z

 constexpr int cMinParsedSecond{0};
-constexpr int cMaxParsedSecond{60};
+constexpr int cMaxParsedSecond{59};
+constexpr int cParsedLeapSecond{60};


How about cLeapSecond?

Co-authored-by: Lin Zhihao <59785146+LinZhihao-723@users.noreply.github.com>

…ap-seconds

LinZhihao-723

For the PR title, how about:

feat(clp-s::timestamp_parser): Add support for preserving leap seconds in date-time timestamps.

Only recourses seem to refer to "leap second" not "leap-second"

…s in date-time timestamps. (y-scope#1673) Co-authored-by: Lin Zhihao <59785146+LinZhihao-723@users.noreply.github.com>

Add support for leap seconds; add tests for leap second support.

291dae3

gibber9809 requested a review from a team as a code owner November 26, 2025 16:46

gibber9809 requested a review from LinZhihao-723 November 26, 2025 16:48

coderabbitai Bot reviewed Nov 26, 2025

View reviewed changes

Comment thread components/core/src/clp_s/timestamp_parser/TimestampParser.cpp

gibber9809 added 2 commits November 27, 2025 17:53

Address rabbit comment.

ba30107

Merge remote-tracking branch 'upstream/main' into timestamp-parser-le…

a72b4bd

…ap-seconds

LinZhihao-723 requested changes Nov 30, 2025

View reviewed changes

gibber9809 and others added 4 commits December 1, 2025 11:25

Apply suggestions from code review

19fa42b

Co-authored-by: Lin Zhihao <59785146+LinZhihao-723@users.noreply.github.com>

Address remaining review comment; make terminology consistent.

2d7277f

Merge remote-tracking branch 'upstream/main' into timestamp-parser-le…

7a2589e

…ap-seconds

Merge branch 'main' into timestamp-parser-leap-seconds

f81991f

gibber9809 requested a review from LinZhihao-723 December 1, 2025 17:34

LinZhihao-723 approved these changes Dec 1, 2025

View reviewed changes

gibber9809 changed the title ~~feat(clp-s::timestamp_parser): Add support for preserving leap-seconds in date-time timestamps.~~ feat(clp-s::timestamp_parser): Add support for preserving leap seconds in date-time timestamps. Dec 1, 2025

gibber9809 merged commit 0b43e8f into y-scope:main Dec 1, 2025
25 checks passed

This was referenced Dec 1, 2025

feat(clp-s::timestamp_parser): Add support for specifying locale-dependent weekday and month names. #1708

Merged

feat(clp-s::timestamp_parser): Make control characters and lone double quotes illegal in timestamp patterns. #1723

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(clp-s::timestamp_parser): Add support for preserving leap seconds in date-time timestamps.#1673

feat(clp-s::timestamp_parser): Add support for preserving leap seconds in date-time timestamps.#1673
gibber9809 merged 7 commits into
y-scope:mainfrom
gibber9809:timestamp-parser-leap-seconds

gibber9809 commented Nov 26, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Nov 26, 2025 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

LinZhihao-723 left a comment

Uh oh!

LinZhihao-723 Nov 30, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LinZhihao-723 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

gibber9809 commented Nov 26, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Validation performed

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

LinZhihao-723 left a comment

Choose a reason for hiding this comment

Uh oh!

LinZhihao-723 Nov 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LinZhihao-723 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gibber9809 commented Nov 26, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Nov 26, 2025 •

edited

Loading