Skip to content

Keep civil-date math in one utils::time module: days_from_civil is written twice and vex::time claims to be the only copy #1867

Open

Description

[agent] Filed by the scheduled architecture audit (audit-core). Register: register.

Kind: refactor. Register ID: A26.

Problem

Calendar arithmetic (Howard Hinnant's civil-date algorithms) lives in three modules at 5ca5623:

  • api/date.rs L48-L56: days_from_civil(year: i64, month: u32, day: u32), the parser half (parse_timestamp_secs, used by api::ranking and api::retry).
  • formats/cargo/floor.rs L25-L33: a second days_from_civil(year: i64, month: i64, day: i64), spelled with div_euclid instead of the if y >= 0 { y } else { y - 399 } / 400 form. It is used to date nightly-YYYY-MM-DD toolchains.
  • vex/time.rs L30-L36: the inverse civil_from_days, inside unix_to_ymdhms. Its doc claims "this is the single civil-date implementation in the crate". telemetry also formats timestamps through it.

Today the two days_from_civil copies compute the same values; api/date.rs's round-trip test pins only its own copy. A time module under vex/ that telemetry and api import, plus a date parser under api/, puts a general utility in two feature modules.

Proposed change

  • Add crates/socket-patch-core/src/utils/time.rs holding days_from_civil, unix_to_ymdhms, now_rfc3339 and parse_timestamp_secs, with the tests of api/date.rs and vex/time.rs.
  • Delete formats/cargo/floor.rs's copy and call utils::time::days_from_civil (convert its i64 month and day at the call).
  • Delete api/date.rs and vex/time.rs, or reduce them to pub use lines if the CLI or tests import them by path (tests/e2e_hosted_production.rs imports api::date::parse_timestamp_secs).
  • Drop the "single civil-date implementation" claim.

Size

4–5 files. The diff is mostly moves; about −25 net lines.

Acceptance criteria

  • Behavior is unchanged. days_from_civil_inverts_unix_to_ymdhms, parsed_order_is_chronological_in_both_formats, unix_to_ymdhms_matches_brute_force and floor.rs's dated-channel tests pass unchanged.
  • One days_from_civil remains in the crate (grep -rn "fn days_from_civil" crates returns one hit).
  • While moving the tests, drop the fake assert assert!(secs > prev_2822 || secs == 0) in parsed_order_is_chronological_in_both_formats (date.rs L523). It checks the test's own loop counter, not the parser.

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent:triagedarch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)priority:p3refactorStructural change: duplicated code or logic, missing abstraction, layering, dead code

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions