Skip to content

feat(reporting): persist scheduled provisional observations - #1221

Open
bokelley wants to merge 7 commits into
mainfrom
feat/reporting-provisional-observations
Open

bokelley wants to merge 7 commits into
mainfrom
feat/reporting-provisional-observations

Conversation

@bokelley

@bokelley bokelley commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

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.

Comment thread src/adcp/reporting/ledger/provisional.py Fixed
Comment thread src/adcp/reporting/ledger/provisional.py Fixed
Comment thread src/adcp/reporting/ledger/provisional.py Fixed
aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Sep 25, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-store ProvisionalObservationStore contract 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 despite review_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.

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Sep 26, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Sep 26, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 matching served_at::text types.

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.

aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Sep 26, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Sep 26, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant