Don't apply the am/pm shift to an unparsed hour in strptime

[carpentry-agent]

Datetime.strptime handed back a fabricated 11:00 for any format that has %p but no hour specifier.

The hour accumulator hr starts at the sentinel -1, and the am/pm adjustment in the strptime result assembly ran before that sentinel was checked. With ampm = 2 (PM) and hr = -1, (< hr 12) is true, so final-hr became -1 + 12 = 11 — and the (= final-hr -1) check on the very next line no longer matched, so the caller got (Just 11) instead of Nothing. AM was unaffected, because (= hr 12) is false for -1.

Measured on unmodified master before the fix:

input	format	hours
PM 2024-03-15	%p %Y-%m-%d	(Just 11)  ← the bug
AM 2024-03-15	%p %Y-%m-%d	(Nothing)
2024-03-15	%Y-%m-%d	(Nothing)
03 PM 2024-03-15	%I %p %Y-%m-%d	(Just 15)

The fix

One condition: skip the am/pm shift when the hour was never parsed, so hours comes back as Nothing — consistent with minutes, seconds, nanoseconds and the timezone, which all resolve their sentinels the same way in that same let. Returning Nothing rather than an error also keeps %p usable purely as an input consumer.

Tests

Four assertions next to the existing %I %p block: %p-only with both AM and PM (both must yield an unset hour), plus %I %p → 15 and a bare %H → 7 pinned so the fix cannot silently break the cases that already worked.

Teeth proved. With the new tests in place and time.carp reverted to master, exactly 1 of the 4 new assertions fails — the PM one, with Expected value: '-1', actual value: '11' (suite: 285 passed, 1 failed). The other three pass on master by design; they are regression pins, not bug demonstrations. With the fix applied the suite is 286 passed, 0 failed.

carp-fmt --check and angler are clean on both changed files.

Coordination with #22

#22 (claude/strftime-scanner) is still open and rewrites the strftime side; this change is in the strptime result assembly. They are independent:

• git merge-tree --write-tree HEAD 9291a18 → exit 0, tree 26587ed, no conflicts.
• Closest hunks in time.carp do not touch: Rewrite strftime as a single-pass byte scanner #22's last hunk ends at base line 1067, mine starts at 1090. In test/time.carp, Rewrite strftime as a single-pass byte scanner #22 works at ~17, ~1001 and ~1364; mine is at 1152.
• The merged tree was checked to contain both sides — my one-line fix and all four new tests, alongside Rewrite strftime as a single-pass byte scanner #22's byte scanner — so the clean merge is not a silent one.

No changelog entry: this repo has no CHANGELOG.md.

---

Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

[carpentry-reviewer]

Build & Tests

Checked out claude/strptime-ampm-sentinel at 12ffc09 and ran the suite locally on armhf:
carp -x test/time.carp → 286 passed, 0 failed, exit 0 (unpiped). CI agrees on both OSes —
the ubuntu job's raw log shows Passed: 286 Failed: 0, so the green check is the tests, not a
masked step.

Merge-base is b33dddd, which is current origin/master, so no stale-branch drift and nothing
to re-check about ordering against the 0.5.1 release commit.

Teeth re-measured, not taken on trust. I reverted the one added line ((= hr -1) hr) and left
the four new assertions in place: the suite went to 285 / 1, and the single failure was
strptime %p with no hour specifier leaves the hour unset (PM) with
Expected value: '-1', actual value: '11'. Exactly the claim in the PR body, exactly one bug
demonstration and three regression pins.

Findings

I probed 16 am/pm shapes through Datetime.strptime, then re-ran the identical probe against
origin/master's time.carp for a differential. Six rows change, ten are byte-identical:

case	master	branch
PM 2024-03-15 / %p %Y-%m-%d	Just 11	Nothing
-1 PM … / %H %p …	Just 11	Nothing
-1 PM … / %I %p …	Just 11	Nothing
PM 30 … / %p %M %Y-%m-%d	Just 11, mins Just 30	Nothing, mins Just 30
AM PM … / %p %p …	Just 11	Nothing
PM / %p	Just 11	Nothing

Every changed row is a case where the hour was never validly parsed, and every one goes from a
fabricated 11 to Nothing. Nothing else moves: %I %p at 03/12 AM/12 PM, %H alone, %I%p
with no separator, lowercase pm (still a clean ERR expected AM or PM) and %H 13 + PM are all
unchanged.

Two things worth recording, neither a defect in this PR:

• Rows 2 and 3 are the strtol leniency case, and the fix happens to improve them too.
  %H/%I on input -1 parses an hour of -1 (Int.from-string accepts a leading sign), which
  collides with the sentinel. On master that collided and got the am/pm shift, yielding Just 11;
  on this branch it resolves to Nothing. That is the better answer — 11 was invented — and it
  also makes -1 behave the same with and without %p, which was already Nothing on master for
  a bare %H. So the sentinel collision is not new and is not widened here; it just stops being
  papered over with a wrong number.
• %H 00 + PM → Just 12 on both master and this branch. Applying an am/pm shift to a
  24-hour specifier is arguably meaningless, but it is untouched by this change and out of scope.

The placement of the new arm is right: putting (= hr -1) hr first in the cond is what makes the
(= final-hr -1) test on the next line reachable again, and it matches how mi, sc, ns and
the timezone resolve their own sentinels in the same let. ampm has no other reader
(grep gives exactly five sites: the init, the two set!s, and the two cond arms), so there is
no second consumer to keep in step.

No changelog entry needed — this repo has no CHANGELOG.md, which I verified rather than assumed.

Independence from open PR #22 holds up on re-check: git merge-base HEAD origin/master is master's
tip, and the two changes sit in different functions with no overlapping hunks.

Nothing else found. The change is one line, the reasoning behind it is correct, and the test
coverage is honestly described rather than inflated.

Verdict: merge

Correct, minimal, and the four new assertions are described exactly as they behave — one real bug
demonstration and three regression pins, with the 285/1 → 286/0 transition reproduced here.