Conversation
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — snapshot-settling rework for the provisional observations ledger.
Checked:
- No Critical/High/Medium findings from the reviewer.
- Control flow verified: next_due anchoring, explicit-close precedence, request canonicalization, transactional rollback, account isolation, bounded source I/O, additive idempotent schema.
- Atomic revision/checkpoint/observation publication across in-memory and Postgres stores; 72h SDK fallback window and frozen per-observation policy.
- Semver signal:
feat:is defensible since the settling surface is an unreleased RC (3.2.0-rc.6) and the new custom-storeProvisionalObservationStorecontract is additive. Retired-obligation activation gap is self-declared out of scope.
Gate checks:
high_risk: false.gated_paths: false — row 2 does not apply despitereview_decision: REVIEW_REQUIRED.- No no-auto-approve team match.
- No prior decision (first review).
Decision table: rows 1–8 do not fire (zero findings). Falls through to row 9 → approve.
9b2aeef to
8388b48
Compare
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean pass on PR #1221 (scheduled provisional observations).
No blocking findings. The reviewer verified the full change surface: durable acquisition/observation persistence (pg.py, in-memory store), producer scheduling rework, new provisional.py records, SQL/schema manifest, fail-closed schema validation, and the extensive test matrix. Confirmed atomic revision+checkpoint+observation rollback, account-composite FK multi-tenant isolation, __getattr__-proof capability detection, single-winner concurrency, source timeout handling, and the correct feat: semver signal for the public surface.
No Critical/High/Medium findings. The only limitations (restart-safe retry backoff, durable due-time selection) are pre-existing and explicitly documented as pending.
Gate checks: high_risk false; gated_paths false; no no-auto-approve team match. Prior decision was also approve on an earlier head with no findings. No decision-table rows 1–8 fired → row 9 approve.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — subsequent pass on PR #1221, net clean.
The delta since the prior clean approve (9b2aeef → d0ec796) is a single inert commit swapping three ProvisionalObservationStore Protocol stub bodies from ... to pass in provisional.py. For a runtime_checkable Protocol whose method bodies never execute, this is functionally identical — no behavioral change, no wire-shape or public-API impact.
No critical/high/medium findings this run and none carried over from the prior approve. No author team gate (no no-auto-approve match), gated_paths is false, and high_risk is false. None of decision-table rows 1–8 fire, so this falls through to row 9 = approve.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean delta pass, consistent with the prior approve.
The delta since the prior review (head d0ec796 → c6de2ce) is test-only across two conformance files:
tests/conformance/reporting/test_reporting_core_lifecycle.py— strengthens assertions to track the new SDK default-restatement-window (P3D fallback) behavior, verifying the automatic superseding revision and the extended supersede chain.tests/conformance/reporting/test_reporting_production_late_accounts.py— replaces a one-shot snapshot assert with a fail-closed 30s polling helper (next_due_first_turn) with matchingserved_at::texttypes.
No tests disabled, no assertions relaxed to hide a bug, no CI gate weakened; neither delta file exceeds 200 net lines. No findings.
Decision-table walk: no critical/high/medium findings (rows 1, 4–6, 8 don't fire); gated_paths is false (row 2 doesn't fire despite REVIEW_REQUIRED); high_risk is false (rows 3, 5 don't fire); no no-auto-approve team match (row 7 doesn't fire). Falls through to row 9 → approve.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — delta pass on a prior clean approval.
The only change since the prior clean review (head c6de2ce) is .github/workflows/ci.yml, converting the pg-conformance job to an include-only matrix with per-lane timeouts (core=30m, process=15m). Matrix syntax, the ${{ matrix.lane }} job name, and the pg-conformance-required-gate fan-in all resolve correctly. No CI gate was disarmed, no test disabled, no code/wire-shape/public-surface changes. No findings surfaced this run and the prior run had none.
Gate checks: no critical/high/medium findings (rows 1, 4, 5, 8 clear). gated_paths is false (row 2 clear). high_risk is false (rows 3, 5 clear). No no-auto-approve team match (row 7 clear). Prior decision was approve, so row 6 (sticky escalation) does not apply. Falls through to row 9 → approve.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean delta pass, no findings.
This subsequent review follows a prior clean approve (head f3c094c). The new commits are limited to operational timeout bumps: .github/workflows/ci.yml (job 40→50 min, step 35→45 min) and tests/conformance/reporting/_production_packaging.py (inner bound 2400s / 40 min when Postgres driver present). The timeout nesting is correct — inner 40 min < step 45 min < job 50 min — so a hung installed-Postgres contract test surfaces output before a bare kill rather than an opaque CI timeout.
No source, wire-shape, public-surface, or spec-drift changes. No CI gate disabled, no ruff rule dropped, no blanket mypy silencing. No blocking or medium findings in this run.
Decision-table path: no critical/high findings (row 1 n/a); gated_paths=false (row 2 n/a); high_risk=false, no deletions/modifications triggers (rows 3-5 n/a); prior decision was approve, not escalate (row 6 n/a); no no-auto-approve team match (row 7 n/a); zero medium findings (row 8 n/a) → falls through to row 9: approve.
Note: review_decision is REVIEW_REQUIRED, but that only forces escalate when gated_paths is true (row 2), which it is not here. The deprecated protected-branch rule does not apply.
Scheduled snapshot reporting previously stopped after one successful read when an offering omitted
restatement_window. An unchanged successful re-read also produced no new revision. This PR adds a 72-hour fallback window, reserves each acquisition before source work, and records every successful observation as a distinct immutable revision. The revision, rows, observation, checkpoint, change, and notification effects commit atomically in memory and PostgreSQL. Retrying the same acquisition preserves its original winner and timing.The PostgreSQL extension adds private acquisition and observation tables with catalog readiness checks. Existing revision-row storage and explicit official-close behavior remain compatible. Custom publisher decorators must explicitly implement the observation and checkpoint methods and prepare observation revisions before delegating the atomic commit. Older producer writers should be stopped before enabling the new schedule because they do not write observation metadata.
Validation on the current branch:
make lint,make typecheck-all, and 61 focused memory/PostgreSQL tests passed. Broader historical migration and full-suite gates are running; CI is the merge gate.