diff --git a/src/flightdeck/feedback.py b/src/flightdeck/feedback.py index c3ab19d..da57dbf 100644 --- a/src/flightdeck/feedback.py +++ b/src/flightdeck/feedback.py @@ -13,7 +13,7 @@ """ import math -from datetime import datetime +from datetime import UTC, datetime from flightdeck.ledger import Ledger from flightdeck.schemas import Feedback, Outcome @@ -48,6 +48,8 @@ def record_feedback( ``feedback_recorded`` event the reports read: ``{run_id, outcome, human_minutes, by}``. ``human_minutes`` left ``None`` means "not timed" and the metrics fall back to the org's conservative ``default_review_minutes``. + ``at`` stamps both the row and the ledger entry, so an imported or backdated + review reads the same on either side of the evidence trail. """ if outcome not in VALID_OUTCOMES: raise FeedbackError(f"outcome must be one of: {', '.join(VALID_OUTCOMES)}") @@ -70,5 +72,15 @@ def record_feedback( ledger.append( "feedback_recorded", {"run_id": run_id, "outcome": outcome, "human_minutes": human_minutes, "by": entry.by}, + # The event time, not the write time -- the same contract runner.record + # keeps with at=run.finished_at. Without it a backfilled review is sealed + # under the wall clock, and the ledger an auditor reads contradicts the + # store row it is evidence for. Normalized to UTC because the row's own + # default is local-offset while every other ledger writer is UTC, and + # `audit tail` prints the stamp without its offset: mixing conventions in + # one file would show a review as happening before the run it reviews. + # Append order is carried by the entry's seq, so this costs the chain + # nothing. + at=entry.at.astimezone(UTC), ) return entry diff --git a/tests/test_feedback.py b/tests/test_feedback.py index 47ddf39..73ff8fd 100644 --- a/tests/test_feedback.py +++ b/tests/test_feedback.py @@ -5,6 +5,8 @@ API and ledger events" is enforced at the source. """ +from datetime import datetime, timedelta, timezone + import pytest from flightdeck.feedback import FeedbackError, record_feedback @@ -61,3 +63,50 @@ def test_record_feedback_rejects_negative_or_non_finite_minutes(org, store, ledg run = _seed_run(org, store, ledger) with pytest.raises(FeedbackError, match="non-negative number"): record_feedback(store, ledger, run.id, "accepted", human_minutes=bad) + + +def test_backdated_feedback_seals_the_event_time_in_the_ledger(org, store, ledger): + # A review imported or backfilled from another system carries its own time. + # The store row and the ledger entry describe the SAME event, so they must not + # disagree about when it happened -- the ledger is the artifact an auditor + # reads, and runner.record already keeps this contract (at=run.finished_at). + run = _seed_run(org, store, ledger) + when = NOW - timedelta(days=200) + + entry = record_feedback(store, ledger, run.id, "accepted", human_minutes=3, by="ana", at=when) + sealed = [e for e in ledger.entries() if e["event"] == "feedback_recorded"][-1] + + assert entry.at == when + assert sealed["at"] == when.isoformat() # was the wall clock, months adrift + assert store.feedback_map()[run.id].at.isoformat() == sealed["at"] + + +def test_feedback_without_an_explicit_time_still_seals_what_the_row_says(org, store, ledger): + # The default path must stay consistent too: whatever "now" the row got is the + # one the chain seals, not a second clock reading taken a moment later. + run = _seed_run(org, store, ledger) + + entry = record_feedback(store, ledger, run.id, "edited", human_minutes=2, by="ana") + sealed = [e for e in ledger.entries() if e["event"] == "feedback_recorded"][-1] + + # Compare instants, not strings: the row's default carries the local offset, + # the ledger is normalized to UTC, and both name the same moment. + assert datetime.fromisoformat(sealed["at"]) == entry.at + assert ledger.verify().ok # and the chain still walks clean + + +def test_ledger_stays_utc_whatever_offset_the_caller_hands_in(org, store, ledger): + # `audit tail` renders entry["at"][:16] -- the offset is sliced off before the + # reader sees it. So every entry in the file has to share one convention, or a + # review shows up hours before the run it reviews. The runner and the demo + # seeder both write UTC; feedback must not be the one exception. + run = _seed_run(org, store, ledger) + tokyo = timezone(timedelta(hours=9)) + + record_feedback(store, ledger, run.id, "accepted", by="ana", at=NOW.astimezone(tokyo)) + stamps = [e["at"] for e in ledger.entries()] + + assert all(s.endswith("+00:00") for s in stamps), stamps + # ...and the feedback still cannot predate the run it attests to. + events = {e["event"]: e["at"] for e in ledger.entries()} + assert events["feedback_recorded"] >= events["run_completed"]