-
Notifications
You must be signed in to change notification settings - Fork 1
fix: stamp analysis-run status from one PostgreSQL clock (v2.12.19) #524
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ccd2d05
fdb8f56
4b9d640
5896c4c
4b1691b
64bc52e
6321860
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| ## 2.12.19 — Analysis-run status same PostgreSQL clock | ||
|
|
||
| - Status events now stamp `occurred_at` and `recorded_at` from one | ||
| PostgreSQL `clock_timestamp()` (ADR 0171). The transition trigger | ||
| still records database time and raises `recorded_at` when a caller | ||
| clock is already ahead, so `analysis_run_status_time_check` no | ||
| longer rejects Start on a 15–20 ms Python-vs-PostgreSQL skew. | ||
| After `make seed`, Open the Demo Corp lineage run and Start still | ||
| recovers the designed A-100 fork. Never invent a theta. |
|
seonghobae marked this conversation as resolved.
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -326,15 +326,21 @@ async def _append_status( | |
| analysis_run_id: str, | ||
| status_ordinal: int, | ||
| status_code: str, | ||
| occurred_at: datetime, | ||
| failure_code: str | None = None, | ||
| ) -> None: | ||
| """Append one legal lifecycle event. Failed rows carry a machine code.""" | ||
| """Append one legal lifecycle event. Failed rows carry a machine code. | ||
|
|
||
| Occurrence and recording share one PostgreSQL ``clock_timestamp()`` | ||
| so ``analysis_run_status_time_check`` cannot see a Python clock that | ||
| is ahead of the trigger write clock (ADR 0171). | ||
| """ | ||
| await conn.execute( | ||
| """ | ||
| insert into analysis_run_status_event | ||
| (analysis_run_id, status_ordinal, status_code, occurred_at, failure_code) | ||
| values ($1, $2, $3, clock_timestamp(), $4) | ||
| (analysis_run_id, status_ordinal, status_code, | ||
| occurred_at, recorded_at, failure_code) | ||
| select $1, $2, $3, write_clock, write_clock, $4 | ||
| from (select clock_timestamp() as write_clock) same_clock | ||
|
seonghobae marked this conversation as resolved.
|
||
| """, | ||
| analysis_run_id, | ||
| status_ordinal, | ||
|
Comment on lines
337
to
346
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📝 Info: Status-event monotonicity preserved by uniform DB clock The transition trigger raises (Refers to this code) Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
@@ -570,7 +576,6 @@ async def enqueue_pending_analysis_run( | |
| analysis_run_id, | ||
| await _next_status_ordinal(conn, analysis_run_id), | ||
| _RUNNING, | ||
| now, | ||
| ) | ||
| await conn.execute( | ||
| """ | ||
|
|
@@ -789,7 +794,6 @@ async def _deliver_lineage_reconstruction( | |
| analysis_run_id, | ||
| await _next_status_ordinal(conn, analysis_run_id), | ||
| _SUCCEEDED, | ||
| finished, | ||
| ) | ||
|
|
||
|
|
||
|
|
@@ -801,7 +805,6 @@ async def _deliver_tepp_measurement( | |
| tepp_client: TeppClient, | ||
| ) -> None: | ||
| """Submit the frozen snapshot through ``tepp_client``. Never persist a theta.""" | ||
| now = datetime.now(timezone.utc) | ||
| request = tepp_run_request( | ||
| idempotency_key=str(locked["idempotency_key"]), | ||
| snapshot_sha256=str(locked["snapshot_sha256"]), | ||
|
|
@@ -817,14 +820,10 @@ async def _deliver_tepp_measurement( | |
| ): | ||
| status_code = _FAILED | ||
| failure_code = "tepp_result_not_persisted" | ||
| finished = datetime.now(timezone.utc) | ||
| if finished < now: | ||
| finished = now | ||
| await _append_status( | ||
| conn, | ||
| analysis_run_id, | ||
| await _next_status_ordinal(conn, analysis_run_id), | ||
| status_code, | ||
| finished, | ||
| failure_code, | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| # ADR 0171 — Analysis-run status events share one PostgreSQL write clock | ||
|
|
||
| **Decision status:** Accepted | ||
| **Date:** 2026-08-24 | ||
|
|
||
| Amends [ADR 0013](0013-normalized-analysis-run-registry.md). Independent of | ||
| leftover-map persist, leftover UI, leftover two-axis distance, TEPP | ||
| arithmetic, and Valkey outbox payload shape. | ||
|
|
||
| ## Context | ||
|
|
||
| ADR 0013 distinguishes lifecycle `occurred_at` from durable | ||
| `recorded_at` and requires `occurred_at <= recorded_at`. The 0018 | ||
| trigger overwrote `recorded_at` with `clock_timestamp()` after a | ||
| `FOR UPDATE`. Callers that bound Python `datetime.now(timezone.utc)` | ||
| as `occurred_at` reproducibly landed 15–20 ms *after* that write | ||
| clock, so `analysis_run_status_time_check` rejected the row | ||
| (`CheckViolationError`). Live | ||
| `test_start_analysis_run_recovers_the_a100_fork` and | ||
| `test_tepp_start_persists_published_accepted_evidence` then could not | ||
| Start a Pending lineage or TEPP run against a real PostgreSQL. | ||
|
|
||
| The product start path later switched `occurred_at` to | ||
| `clock_timestamp()` while still omitting `recorded_at` (DEFAULT plus | ||
| trigger). That still leaves a two-clock window: VALUES vs DEFAULT vs | ||
| trigger each call `clock_timestamp()` separately, and any remaining | ||
| Python-ahead caller (seed, live test helper, or a future bind) fails | ||
| the same check. | ||
|
|
||
| ## Decision | ||
|
|
||
| 1. Application inserts of `analysis_run_status_event` stamp | ||
| `occurred_at` and `recorded_at` from **one** PostgreSQL | ||
| `clock_timestamp()` (a `SELECT ... FROM (SELECT clock_timestamp() | ||
| AS write_clock)` row). Do not bind Python `datetime.now` as | ||
| occurrence. | ||
| 2. `enforce_analysis_run_status_transition` still overwrites | ||
| `recorded_at` with database `clock_timestamp()`. If the supplied | ||
| `occurred_at` is already ahead of that clock, raise `recorded_at` | ||
| to `occurred_at` so the check holds. Do not rewrite occurrence: | ||
| monotonicity and "cannot predate the request" stay on the | ||
| caller-supplied instant. | ||
| 3. Migration 0173 is the single source of the trigger replacement; shipped | ||
| migration 0018 remains immutable. The Compose migration service applies | ||
| 0173 after the initial schema on fresh installs and replays it on existing | ||
| volumes. | ||
|
|
||
| Do not invent a leftover score. Do not invent a theta. | ||
|
|
||
| ## Consequences | ||
|
|
||
| Starting a Pending lineage reconstruction or TEPP measurement no | ||
| longer fails closed on a 15–20 ms Python-vs-PostgreSQL skew. After | ||
| `make seed`, Open the Demo Corp lineage run and Start still recovers | ||
| the designed A-100 fork. A synthetic insert whose `occurred_at` is | ||
| 50 ms ahead of `clock_timestamp()` persists with | ||
| `recorded_at >= occurred_at`. | ||
|
|
||
| ## References | ||
|
|
||
| Allen, J. F. (1983). Maintaining knowledge about temporal intervals. | ||
| *Communications of the ACM, 26*(11), 832–843. | ||
| https://doi.org/10.1145/182.358434 | ||
|
|
||
| Lebo, T., Sahoo, S., McGuinness, D., Belhajjame, K., Cheney, J., | ||
| Corsar, D., Garijo, D., Soiland-Reyes, S., Zednik, S., & Zhao, J. | ||
| (2013). *PROV-O: The PROV ontology* (W3C Recommendation). World Wide | ||
| Web Consortium. https://www.w3.org/TR/prov-o/ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| -- Analysis-run status events share one PostgreSQL write clock (ADR 0171). | ||
| -- | ||
| -- Replaces enforce_analysis_run_status_transition so recorded_at cannot | ||
| -- precede occurred_at when a caller supplies a clock that is slightly | ||
| -- ahead of PostgreSQL (the live CheckViolationError on | ||
| -- analysis_run_status_time_check). Occurrence is not rewritten; the | ||
| -- durable write clock is raised to the supplied occurrence when needed. | ||
| -- Idempotent migration 0173: create or replace. Does not invent a theta. | ||
|
|
||
| create or replace function enforce_analysis_run_status_transition() | ||
| returns trigger | ||
| language plpgsql | ||
| as $$ | ||
| declare | ||
| previous_ordinal integer; | ||
| previous_status_code text; | ||
| previous_occurred_at timestamptz; | ||
| run_requested_at timestamptz; | ||
| write_clock timestamptz; | ||
| begin | ||
| -- The immutable parent row is a per-run serialization lock. It prevents | ||
| -- concurrent writers from both accepting the same next ordinal. | ||
| select requested_at | ||
| into run_requested_at | ||
| from analysis_run | ||
| where analysis_run_id = new.analysis_run_id | ||
| for update; | ||
|
|
||
| if not found then | ||
| raise exception 'analysis_run_not_found'; | ||
| end if; | ||
| if not exists ( | ||
| select 1 from analysis_run_scope | ||
| where analysis_run_id = new.analysis_run_id | ||
| ) then | ||
| raise exception 'analysis_run_scope_required'; | ||
| end if; | ||
| if new.occurred_at < run_requested_at then | ||
| raise exception 'analysis_run_status_before_request'; | ||
| end if; | ||
| -- One database clock for the durable write. If the supplied occurrence | ||
| -- is already ahead of that clock (Python datetime.now skew), raise | ||
| -- recorded_at to occurred_at so analysis_run_status_time_check holds | ||
| -- without rewriting occurrence or breaking monotonicity. | ||
| write_clock := clock_timestamp(); | ||
| new.recorded_at := write_clock; | ||
| if new.recorded_at < new.occurred_at then | ||
| new.recorded_at := new.occurred_at; | ||
| end if; | ||
|
devin-ai-integration[bot] marked this conversation as resolved.
|
||
|
|
||
| select status_ordinal, status_code, occurred_at | ||
| into previous_ordinal, previous_status_code, previous_occurred_at | ||
| from analysis_run_status_event | ||
| where analysis_run_id = new.analysis_run_id | ||
| order by status_ordinal desc | ||
| limit 1; | ||
|
|
||
| if previous_ordinal is null then | ||
| if new.status_ordinal <> 1 | ||
| or new.status_code <> 'analysis_status_pending' then | ||
| raise exception 'analysis_run_first_status_must_be_pending'; | ||
| end if; | ||
| return new; | ||
| end if; | ||
|
|
||
| if new.status_ordinal <> previous_ordinal + 1 then | ||
| raise exception 'analysis_run_status_ordinal_not_contiguous'; | ||
| end if; | ||
| if new.occurred_at < previous_occurred_at then | ||
| raise exception 'analysis_run_status_time_not_monotonic'; | ||
| end if; | ||
|
|
||
| if previous_status_code = 'analysis_status_pending' then | ||
| if new.status_code not in ( | ||
| 'analysis_status_running', | ||
| 'analysis_status_cancelled' | ||
| ) then | ||
| raise exception 'analysis_run_status_transition_invalid'; | ||
| end if; | ||
| elsif previous_status_code = 'analysis_status_running' then | ||
| if new.status_code not in ( | ||
| 'analysis_status_succeeded', | ||
| 'analysis_status_failed', | ||
| 'analysis_status_cancelled' | ||
| ) then | ||
| raise exception 'analysis_run_status_transition_invalid'; | ||
| end if; | ||
| else | ||
| raise exception 'analysis_run_terminal_status_has_no_successor'; | ||
| end if; | ||
|
|
||
| return new; | ||
| end | ||
| $$; | ||
|
|
||
| comment on function enforce_analysis_run_status_transition() is | ||
| 'Serializes status appends and requires immutable scope, request-time ' | ||
| 'ordering, database-recorded time that cannot precede occurrence, ' | ||
| 'legal transitions, and terminal finality.'; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Duplicate 2.12.19 changelog section
A second
## [2.12.19] - 2026-08-24heading is added while an identical one already exists for the Log in fix (CHANGELOG.md). One version now has two separate release sections with the same date. The new note belongs in the existing section's### Fixedlist.Prompt for agents
Was this helpful? React with 👍 or 👎 to provide feedback.