Skip to content

buffa-types: add jiff feature (Timestamp/Duration conversions)#264

Merged
iainmcgin merged 5 commits into
anthropics:mainfrom
macalinao:jiff
Jul 8, 2026
Merged

buffa-types: add jiff feature (Timestamp/Duration conversions)#264
iainmcgin merged 5 commits into
anthropics:mainfrom
macalinao:jiff

Conversation

@macalinao

Copy link
Copy Markdown
Contributor

Summary

Adds a jiff Cargo feature to buffa-types providing conversions between the protobuf well-known types and jiff, mirroring the existing chrono feature:

  • Timestampjiff::Timestamp — reuses the shared TimestampError (InvalidNanos + Overflow). jiff::Timestamp reports sub-second nanos sign-consistently (negative pre-epoch), so the From impl re-normalizes to proto's always-non-negative nanos convention by borrowing a second.
  • Durationjiff::SignedDurationSignedDuration is the faithful analog of proto Duration (sign-consistent seconds + sub-second nanos; jiff::Span is deliberately not a target since its calendar units have no fixed length). Introduces a narrow DurationJiffError with only InvalidNanos: a validated proto Duration can never overflow SignedDuration.

Details

  • no_std-compatible: jiff is pulled with default-features = false + alloc; buffa-types' std feature layers jiff/std on top via jiff?/std.
  • Wired into buffa-test so the conversions are exercised by cargo test --workspace in CI (parity with chrono).
  • jiff 0.2 declares MSRV 1.70, within buffa's 1.75 pin (so msrv-check stays green).
  • Thorough unit + doc tests: pre-epoch borrow, exact-second pre-epoch, nanos boundaries, both jiff::Timestamp extremes, both SignedDuration extremes, i32::MIN nanos, bidirectional sign-mismatch rejection.

Checks

  • cargo test -p buffa-types --features jiff
  • cargo clippy -p buffa-types --features jiff ✅ (clean)
  • cargo check -p buffa-types --no-default-features --features jiff ✅ (no_std)
  • cargo fmt --check
  • Changelog fragment added under .changes/unreleased/.

Reviewed pre-submission by the repo's rust-code-reviewer (no Critical/High/Medium correctness findings) and rust-api-ergonomics-reviewer (the flagged missing "proto JSON spec range" doc warning on the infallible From impls has been added for chrono parity).

Add a `jiff` Cargo feature to buffa-types providing conversions between
the protobuf well-known types and the jiff crate, mirroring the existing
`chrono` feature:

- Timestamp <-> jiff::Timestamp (reuses TimestampError; jiff's
  sign-consistent subsec nanos are re-normalized to proto's non-negative
  nanos convention by borrowing a second pre-epoch).
- Duration <-> jiff::SignedDuration (new DurationJiffError with only
  InvalidNanos: SignedDuration's range is wide enough that a validated
  proto Duration never overflows it).

no_std-compatible (jiff pulled with default-features=false + alloc); the
std feature layers jiff/std on top. Wired into buffa-test for CI coverage.
jiff 0.2 MSRV is 1.70, within buffa's 1.75 pin.
@github-actions

Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

- Drop inline comments that restate the doc/code; keep the load-bearing
  underflow- and overflow-safety rationale.
- Add a no_std jiff check to the check-nostd task (parity with chrono).
@macalinao macalinao marked this pull request as ready for review June 29, 2026 08:46
iainmcgin added 2 commits July 7, 2026 19:00
…ration note

- timestamp_ext.rs's TimestampError and NANOS_MAX docs enumerated exactly
  the std and chrono consumers; the jiff conversion is now a third.
- Add a regression test for the subtlest borrow case: one nanosecond
  short of jiff's MIN second round-trips through proto { seconds: MIN,
  nanos: 1 }, which jiff::Timestamp::new accepts (only negative nanos at
  MIN are out of range).
- Changelog: note for chrono users that the jiff Duration conversion has
  no Overflow mode, and what to do about it.
- Keep [workspace.dependencies] alphabetical.

No-Verification-Needed: doc/test-only fixups, no product behavior change

@iainmcgin iainmcgin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[claude code] Thanks @macalinao — this is an exemplary feature addition. Review verified the load-bearing claims against the jiff 0.2.29 source directly, and everything holds:

  • jiff::Timestamp::new / SignedDuration::new panic-freedom: the From borrow branch is pure integer arithmetic, and the TryFrom guards keep every admitted input out of jiff's error/balance paths — including the subtle ones: (MIN, positive nanos) is accepted by jiff (only negative nanos at MIN are out of range), and (i64::MIN, -999_999_999) takes SignedDuration's no-carry path.
  • Policy parity: the sign-mismatch rejection matches duration_chrono.rs exactly and agrees with the JSON/text codec's validate_duration; the timestamp side correctly relies on normalization instead, because proto's non-negative-nanos convention makes that a genuinely different case.
  • The SignedDuration-not-Span choice and the absent Overflow mode are both right and both well-argued in the docs.
  • Feature wiring (dep:jiff, weak jiff?/std, alloc-only pulling zero transitive crates in no_std config), docs.rs doc(cfg) coverage, MSRV comment (jiff declares 1.70), Taskfile no_std check, buffa-test wiring, changelog fragment: all verified.

Since the remaining items were trivial, I've applied them directly to your branch (maintainerCanModify), pushed as 4194601 + 74964af:

  1. Merged main (the branch was 13 commits behind; clean merge, full workspace clippy + tests pass on the result).
  2. timestamp_ext.rs: the TimestampError / NANOS_MAX docs enumerated exactly the std + chrono consumers — your PR makes jiff a third, so the "both targets" wording had become false.
  3. Added a regression test for the one edge the suite didn't pin: a near-MIN jiff instant with negative subsec nanos, whose borrow produces proto { seconds: MIN, nanos: 1 } — the case where the round-trip depends on jiff accepting positive nanos at MIN.
  4. Changelog fragment: added a migration sentence for chrono users about the (correctly) absent Overflow mode, so the behavioral difference is discoverable before production.
  5. Moved the jiff workspace-dependency entry to its alphabetical slot.

Approving. Two deliberate design points a future reader should know were considered and accepted: the JSON-spec-range failure surfacing at serialization time rather than conversion time (inherent to the infallible From, same as chrono), and InvalidNanos covering the sign-mismatch case (matches the chrono convention; the codec's validate_duration is more granular, but changing that here would diverge from DurationChronoError).

@iainmcgin iainmcgin added this pull request to the merge queue Jul 8, 2026
Merged via the queue into anthropics:main with commit 699192f Jul 8, 2026
9 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 8, 2026
@iainmcgin

Copy link
Copy Markdown
Collaborator

[claude code] Merged as 699192f — thank you @macalinao! The jiff conversions are a great addition, and the level of care in the edge-case analysis (sign normalization, the Span-vs-SignedDuration rationale, the no_std wiring) made this one of the smoothest external reviews we've had.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants