Story: Unify entity timestamp handling — throw on failure, fix optional<db_timestamp>

Table of Contents

This page documents a story in Sprint 19. It captures the goal, current status, acceptance criteria, and the tasks that compose it.

Goal

Enforce the invariant: any timestamp conversion failure must throw. Silent epoch returns mask data corruption and produce misleading display (e.g. "56 years ago" in history dialogs). The codebase has two sources of silent failure:

  1. optional<db_timestamp> declared for NOT NULL columns — sqlgen may leave the optional unpopulated on parse failure, which the old null-check-and-throw guard only partially addressed. Correct type is plain db_timestamp.
  2. timepoint_to_timestamp(tp, lg) on the write side — returns an empty db_timestamp on failure and logs a warning instead of throwing. Replaced by datetime::to_db_string(tp) which always succeeds (to_iso8601_utc does not fail for valid time_points) and returns a plain std::string.

The conversion functions themselves already throw after commit 0446e9536: timestamp_to_timepoint(const db_timestamp&) routes through from_iso8601_utc and throws std::invalid_argument on any parse failure.

Architecture reference: Time and Timestamps: Architecture and Conventions.

Status

Field Value
State STARTED
Parent sprint Sprint 19
Now Tasks 1 and 2 DONE: bulk in PR #1033, stragglers + timepoint_to_timestamp deletion on mop-up branch.
Waiting on Nothing.
Next Run full test suite (task 4); verify no epoch timestamps in history dialogs — unblocks currency Qt verification.
Last touched 2026-06-04

Acceptance

  • Any timestamp conversion failure throws std::invalid_argument (never silently returns epoch).
  • No entity file declares optional<db_timestamp> for a column that is NOT NULL in the DB.
  • timepoint_to_timestamp is deleted from mapper_helpers.hpp; all write sites use datetime::to_db_string.
  • Both overloads of timestamp_to_timepoint remain and both throw on failure.
  • Full build green; no epoch timestamps appear in any history dialog.
  • (Stretch) Entity timestamp fields migrated to std::chrono::system_clock::time_point once sqlgen's native time_point support is confirmed; mappers become direct assignments.

Tasks

Task State Start End Description
Fix optional<db_timestamp> → db_timestamp for all NOT NULL entity fields DONE 2026-06-03 2026-06-04 ~100 entity files: every valid_from/valid_to and other NOT NULL ts fields.
Replace timepoint_to_timestamp with datetime::to_db_string on write side DONE 2026-06-03 2026-06-04 Call sites migrated in PR #1033; function deleted from mapper_helpers in mop-up.
Investigate direct time_point entity fields (stretch) BACKLOG     Confirm sqlgen handles time_point via custom rfl parser; prototype one entity.
Verify full build and tests green BACKLOG     Full build + test run; confirm no epoch timestamps in history dialogs.

Decisions

  • Entity fields stay as db_timestamp (not std::string). std::string loses type safety, breaks sqlgen WHERE clause comparisons, and provides no semantic benefit. db_timestamp is the correct serialisation shim for sqlgen/rfl.
  • optional<db_timestamp> is wrong for NOT NULL columns. The optional wrapper lets sqlgen silently leave the field unpopulated; the correct type is plain db_timestamp. Nullable columns keep optional<db_timestamp>.
  • timepoint_to_timestamp returns {} (empty db_timestamp) on failure — silent epoch on write. Replaced with datetime::to_db_string(tp) which cannot fail and returns a plain std::string. Entity write fields accept std::string via db_timestamp constructor (db_timestamp(str) calls strptime, which will throw if the string is malformed — guaranteed safe since to_db_string always produces valid output).
  • All conversions must throw. A silently wrong timestamp is worse than a crash.

Reverted approach

Commit 1ad3f36a1 replaced all db_timestamp fields with std::string across ~100 entity files and ~136 repository files. This was reverted (git reset --hard 051cf2525) because:

  • std::string provides no type safety for timestamps.
  • sqlgen WHERE clauses require the column and value to have compatible types; std::string vs db_timestamp in a WHERE clause is a static assertion failure.
  • String comparison for dates is semantically fragile.

Out of scope

  • Changing the temporal SQL schema or trigger behaviour.
  • Changing make_timestamp / MAX_TIMESTAMP in repository WHERE clauses.
  • Non-timestamp fields.

Emacs 29.1 (Org mode 9.6.6)