feat(clp-s::timestamp_parser): Add support for parsing RFC 2822 / 822 timestamp strings (fixes #2027).

[Nathan903]

Description

Adds support for parsing RFC 2822 / RFC 822 timestamp strings to the CLP-S timestamp parser.

• Added a new \o{name,offset} specifier to handle named timezones.
• Updated \Z to automatically recognize standard named timezones (like EST, PDT, etc).
• Hardcoded obsolete timezones (UT and GMT) as literals to map to a +0000 offset.
• Added the RFC 2822 patterns to the default fallback patterns list.

Also added tests and caught & fixed a bug: the timestamp parser throws an IncompatibleTimestampPattern error when using \Z to parse a string ending in "UTC" without any trailing offset (e.g., parsing "01 Jan 2000 00:00:00 UTC" against pattern \d \B{Jan,Feb,Mar,Apr,May,Jun,Jul,Aug,Sep,Oct,Nov,Dec} \Y \H:\M:\S \Z). This is fixed by adding optional_timezone_offset_in_minutes = 0; to the cUtc handling block.

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 unit tests to test_TimestampParser.cpp verifying the \o specifier, \Z fallback logic, and full RFC 2822 formatting strings.

cmake --build build/core --target unitTest && ./build/core/unitTest "[clp-s][timestamp-parser]"

Summary by CodeRabbit

• New Features
  • Added support for named time zones with explicit offsets in timestamp patterns and formatting via the o specifier (\o{name,offset}).
• Bug Fixes
  • Improved validation and parsing of timezone segments, including handling for common abbreviations (EST, UT, GMT) and correct generic timezone expansion.
• Tests
  • Expanded coverage for invalid timestamp templates and additional CAT-sequence transformations, plus new timestamp parsing cases for named time zones.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

This PR adds named timezone support via \o{name,offset}, updates generic \Z resolution to map named abbreviations, and extends parser documentation and tests for the new timezone forms.

Changes

Named Timezone Specifier

Layer / File(s)	Summary
Named timezone table and constants components/core/src/clp_s/timestamp_parser/TimestampParser.cpp	Adds NamedTimezone and cNamedTimezones with timezone abbreviations mapped to fixed offsets, plus UT and GMT aliases.
'o' specifier marshalling, pattern creation, and parsing components/core/src/clp_s/timestamp_parser/TimestampParser.cpp	Adds case 'o' handling to emit, create, and parse named timezone plus offset segments, and rejects duplicate timezone segments in case 'z'.
Generic 'Z' matching against named zones components/core/src/clp_s/timestamp_parser/TimestampParser.cpp	Updates Z parsing to recognise named abbreviations and rewrite them as \o{name,offset}.
Documentation and test coverage for named timezones components/core/src/clp_s/timestamp_parser/TimestampParser.hpp, components/core/src/clp_s/timestamp_parser/test/test_TimestampParser.cpp	Documents \o{...,...} and \Z resolution, and expands timezone transformation and parsing test cases.

Estimated code review effort: 3 (Moderate) | ~25 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: adding RFC 2822/822 timestamp parsing support in clp-s::timestamp_parser.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

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

1175-1225: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

z and o specifiers silently overwrite each other's shared state.

Both the z case (Line 1175-1199) and the new o case (Line 1200-1225) write to the single optional_timezone_size_and_offset field via .emplace(...). format_specifiers only rejects a repeated occurrence of the same character, so a pattern combining both, e.g. \z{+0500} \o{EST,-0500}, is accepted by create, but the field ends up holding whichever specifier was parsed last. marshal_date_time_timestamp's z case (Line 844-853) and o case (Line 854-867) both read from this same field, so whichever specifier was overwritten will emit/parse the wrong-length substring — silently corrupting the marshalled or parsed timestamp instead of failing.

Add a check that rejects a pattern if optional_timezone_size_and_offset already has a value when either z or o is encountered.

🐛 Suggested fix (apply to both `z` and `o` cases)

             case 'o': {  // Named time-zone with specific offset.
+                if (optional_timezone_size_and_offset.has_value()) {
+                    return ErrorCode{ErrorCodeEnum::InvalidTimestampPattern};
+                }
                 auto const bracket_pattern{YSTDLIB_ERROR_HANDLING_TRYX(

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@components/core/src/clp_s/timestamp_parser/TimestampParser.cpp` around lines
1175 - 1225, The shared timezone state in TimestampParser::create is being
overwritten when both the z and o specifiers appear in one pattern, because each
case unconditionally emplaces into optional_timezone_size_and_offset. Update the
z and o branches to reject a second timezone specifier by checking whether
optional_timezone_size_and_offset already contains a value and returning
InvalidTimestampPattern if so, so marshal_date_time_timestamp does not later
read a corrupted size/offset pair.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@components/core/src/clp_s/timestamp_parser/TimestampParser.cpp`:
- Around line 1200-1225: The 'o' branch in TimestampParser::parse_pattern
accepts an empty named-timezone name because name_str is taken from the
bracketed pattern without validation. Add a check in the same handler, before
storing into optional_timezone_size_and_offset, to reject patterns where the
extracted name is empty (for example when comma_idx is immediately after the
opening brace) and return InvalidTimestampPattern. This should ensure
parse_timestamp’s 'o' matching logic cannot end up with an empty expected_name
and a no-op starts_with("") match.
- Around line 81-96: Store the timezone offset as a precomputed minute value in
NamedTimezone instead of keeping offset_str and re-parsing it in the Z handler.
Update the cNamedTimezones table and the matching logic in TimestampParser so
the named-zone branch uses the stored offset_minutes directly, avoiding repeated
extract_timezone_offset_in_minutes calls and the silent invalid-offset path.

---

Outside diff comments:
In `@components/core/src/clp_s/timestamp_parser/TimestampParser.cpp`:
- Around line 1175-1225: The shared timezone state in TimestampParser::create is
being overwritten when both the z and o specifiers appear in one pattern,
because each case unconditionally emplaces into
optional_timezone_size_and_offset. Update the z and o branches to reject a
second timezone specifier by checking whether optional_timezone_size_and_offset
already contains a value and returning InvalidTimestampPattern if so, so
marshal_date_time_timestamp does not later read a corrupted size/offset pair.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7dcdbb70-608f-44d1-8720-ead5e158f0a9

📥 Commits

Reviewing files that changed from the base of the PR and between f54f68d and 819e9e4.

📒 Files selected for processing (3)

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

[gibber9809]

Left an initial review for the whole PR.