Skip to content

test(reporting): integrate currency and metric evidence - #1177

Open
bokelley wants to merge 5 commits into
conductor/reporting-durable-records-1167afrom
conductor/reporting-evidence-currency-integration
Open

bokelley wants to merge 5 commits into
conductor/reporting-durable-records-1167afrom
conductor/reporting-evidence-currency-integration

Conversation

@bokelley

@bokelley bokelley commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

This is the explicit join of two parallel, reviewed Reliable Reporting roots: #1173's per-metric source evidence and #1176's durable records on top of #1175's obligation currency. This branch will be the parent of #1168A.

The integration keeps the six legacy positional InlineFetchResult arguments and makes both currency and cell_availability keyword-only. Configuration/obligation currency stays authoritative. Sparse unavailable monetary cells retain available nonmonetary measurements, explicit zero retains its own evidence, and unavailable spend suppresses its total wherever that withdrawal can reach the sum. Both lines' currency/evidence validators, row snapshotting, error classification, control-total bytes, and sealed replay behavior are preserved.

The new private _reliable_support.py harness supplies manual time, finite failure plans and barriers, bounded worker draining, scripted sync/async sources, deterministic destination/receiver stores, and memory/real-PostgreSQL factories. Its trusted publisher preparation helper supplies typed evidence before the real Core store write, composing the worker with the #1167A record APIs. All domain validation still runs in the existing stores.

The combined scenarios exposed an account collision in memory staging: identical rows under the same execution key let a later account replace the earlier account's ownership. Staging now includes the account in its internal key while retaining the same public object references and bytes. The clock audit also corrected memory Core change appends and PostgreSQL Core/reconciliation change timestamps to honor injected clocks, and added an optional clock to source conformance execution/object-read deadlines. Default clocks retain their existing real-time behavior.

Cross-foundation wedge fixed on this branch

The combined scenarios also exposed a defect that neither reviewed root can see alone. #1173 withdrew a metric's control total whenever any cell declared it missing/delayed/unsupported, including when the declaring constituent staged no rows at all. #1175's validate_monetary_content requires the total for a monetary column every retained row carries. So the join sealed an immutable publication the obligation ledger then refused with MONETARY_TOTAL_MISMATCH on the replay of every retry -- permanently wedging the obligation for the one answer per-metric evidence exists to give: a buy that delivered nothing and whose billing is pending.

The fix distinguishes the two shapes rather than exempting money from the withdrawal:

  • A withdrawal by a constituent that staged rows still removes the total. Those rows carry values the adapter disclaimed, so summing them would contradict its own evidence, and the provider payload stays staged byte-for-byte. feat(reporting): support per-metric source availability evidence #1173's behavior is unchanged here.
  • A withdrawal by a constituent that staged no rows reaches no sum and removes nothing. The checksum over the rows that were measured is retained, and the revision commits.
  • A row-empty batch still withdraws either way, so unavailability is never published as an observed zero.

No control total ever sums a value a cell disclaims, and no sealed publication can be one the ledger only ever refuses.

Monetary columns have no third option: dropping the total is refused by the ledger, and keeping it would falsify the evidence. A withdrawn monetary cell whose own constituent's rows report that metric now raises ValueError before anything is staged or sealed. ReportingDefinitionBinding.to_wire() deliberately keeps the frozen unit declarations off the wire so retained contract hashes do not move, so ReportingContractIdentityV1 carries no units and the source cannot derive them. A new keyword-only InlineReportingSource(monetary_metrics=...) lets a trusted caller declare #1171's custom monetary_metric_units columns; spend is always included and the default is unchanged behavior.

Reachability boundary: this requires coverage.expected == "partial". ReportingProducer._build_slice emits expected="full", and a full slice returns retryable PARTIAL_RESULT before sealing, so the wedge sits on the adopter-built partial-coverage path -- which is the only path on which #1173's withdrawn cells can publish at all. Manifest and control-total bytes for a result with no cell_availability are byte-identical to the #1176 base.

There are 69 new cases across the integration module and harness regressions. They cover colliding EUR/USD identities, sparse per-metric evidence, zero versus unavailable spend, obligation-currency totals, validation before staging/sealing, immutable rows during an awaited stage, post-seal retry with changed adapter currency/evidence, store and fresh-process restart, worker-to-record publication, populated legacy-schema upgrade followed by exact-revision reconciliation, zero-row snapshot/official coexistence, a no-row unavailable monetary cell that still commits its neighbour's subtotal, a withdrawn monetary cell contradicted by its own rows failing before any seal, and a custom frozen monetary metric distinguished on both branches. These cases are not marked integration; real PostgreSQL variants run whenever ADCP_PG_TEST_URL is set. Memory restart explicitly restores a test fixture image.

Reviewed roots and preserved history:

Role Exact SHA
#1176 / first merge parent / PR base 3c405a21f978ed9d3208611bb4a7a8434a056933
#1173 / second merge parent 7fd88c6710c26211484032bdd0e51fcd1077e6ed
#1175 / currency ancestor of #1176 ff584b6fbe3436e7afa4980a14e2e5e4004a06a3
Dedicated semantic integration merge 6c7ffa506612e5fa33a6adc4b097987aea2303c5
Joined evidence/currency integration tests d0308e6e22f3329f95ff8d558dff95601f81c6ff
Cross-foundation wedge fix 037de4ac822ecefb2f95d32c15c297fb4c45d683
Verified PR head 037de4ac822ecefb2f95d32c15c297fb4c45d683

#1176 was approved at that exact remote head before branch creation. The branch was created from that head and joined with:

git switch -c conductor/reporting-evidence-currency-integration 3c405a21f978ed9d3208611bb4a7a8434a056933
git merge --no-ff --no-commit origin/conductor/reporting-metric-evidence-1170

Both reviewed histories are parents of the integration merge. The foundation PRs retain their branches and bases.

Validation ran on static commit 037de4ac822ecefb2f95d32c15c297fb4c45d683, with Python 3.12.14 and real PostgreSQL 16.14. ADCP_PG_TEST_URL pointed at the local PostgreSQL server; each test owns an isolated schema.

export ADCP_PG_TEST_URL=postgresql://vercel-sandbox@127.0.0.1:55432/postgres

.venv/bin/pytest tests/conformance/reporting \
  tests/test_reporting_canonical_json.py tests/test_reporting_consumer_loop.py \
  tests/test_reporting_inline_source.py tests/test_reporting_ledger.py \
  tests/test_reporting_metric_evidence.py tests/test_reporting_reconciliation.py \
  tests/test_reporting_source_contract.py tests/test_reporting_webhook_operation_id.py \
  -q --no-cov
# 1,159 passed; no skips or deselections.

make ci-local
# 9,259 passed, 34 skipped, 9 deselected, 1 xfailed; 85.22% coverage.
# Generated-code validation passed. Full pytest duration: 1,098.22 seconds.

# Included in ci-local; strict=true for source in pyproject.toml:
.venv/bin/mypy src/adcp/
# 1,359 source files, no issues.
.venv/bin/mypy --strict tests/type_checks/
# 30 adopter fixtures, no issues.
.venv/bin/python scripts/check_type_ignore_contract.py
# No adopter type: ignore suppressions.

.venv/bin/ruff check src/
# Passed; changed test files also passed Ruff.
# Black --check passed for all 13 changed Python files, including imported foundation files.

.venv/bin/python -m build --outdir .context/review-1177/dist
uv venv .context/review-1177/wheel-venv --python .venv/bin/python
uv pip install --python .context/review-1177/wheel-venv/bin/python \
  .context/review-1177/dist/adcp-8.0.0b15-py3-none-any.whl 'psycopg[binary,pool]>=3.1.0'
uv venv .context/review-1177/sdist-venv --python .venv/bin/python
uv pip install --python .context/review-1177/sdist-venv/bin/python \
  .context/review-1177/dist/adcp-8.0.0b15.tar.gz 'psycopg[binary,pool]>=3.1.0'
.context/review-1177/wheel-venv/bin/python -I \
  .context/review-1177/check_installed_resources.py /home/vercel-sandbox/adcp-client-python
.context/review-1177/sdist-venv/bin/python -I \
  .context/review-1177/check_installed_resources.py /home/vercel-sandbox/adcp-client-python

Both installed environments ran the workspace's check_installed_resources.py with python -I, verifying imports from their own site-packages, the combined constructor signature, the keyword-only monetary_metrics seam, py.typed, all four bundled SQL files and all 3,147 schema JSON files byte-for-byte against the static checkout. Each installed artifact also bootstrapped the full 12-table PostgreSQL ledger twice in a fresh isolated schema.

Artifact SHA-256 values:

Artifact SHA-256
adcp-8.0.0b15-py3-none-any.whl 7f3f7eea17d90d91a3c38df1929566835de7de730fb7f56e5b7a19d09d10e15e
adcp-8.0.0b15.tar.gz 5eeb944b48201538b4e7df1fa069182b8f14ae48b3742465db320f68da5ce178

Byte drift was measured directly at each root with one legacy no-opt-in input. origin/main and #1173's head 7fd88c67 both produce manifest bytes b1d4ab57...0912f42; #1175 ff584b6f, #1176 3c405a21, d0308e6e and the verified head 037de4ac all produce 38afa6c0...fb286233. #1173 introduces no drift, the "unit":"USD" on the spend total is #1175's intentional currency pin, and the wedge fix adds none.

Residual boundaries for reviewers:

  • The monetary distinction depends on the caller declaring monetary_metrics, because the frozen slice request cannot carry the trusted unit declarations. An undeclared custom money column keeps feat(reporting): support per-metric source availability evidence #1173's non-monetary behavior and can still wedge for that column; a producer-side wiring belongs in a follow-up. Both branches are regression-covered.
  • ReportingProducer has no partial-coverage path yet, so per-metric unavailability is reachable only on adopter-built slices.
  • _control_totals pins unit only on spend, so the manifest unit check is a no-op for a custom monetary total. Changing that moves manifest bytes for those adopters.
  • reporting_ledger_changes.committed_at defaults to now() while reporting_reconciliation_changes.committed_at defaults to clock_timestamp(). Each is self-consistent with its own feed and predates this stack; ordering uses seq.
  • One reviewed feat(reporting): support per-metric source availability evidence #1173 assertion changed with the fix: test_a_covered_zero_row_constituent_keeps_zeros_only_for_its_available_metrics now retains the neighbour's checksum, and a new test_a_withdrawal_only_removes_the_total_its_own_rows_could_corrupt pins the shape feat(reporting): support per-metric source availability evidence #1173 did test.

Open workspace in Conductor

bokelley and others added 4 commits September 16, 2026 00:24
Per-cell evidence withdrew a metric's control total whenever *any* cell
for it was unavailable, and dropped every total whenever a staged row
matched no requested constituent. Both conditions are reachable with no
`cell_availability` at all, so five documented legacy shapes -- a `None`
not-ready answer, `covered_constituent_ids`, `unavailable_constituents`,
a non-default `unavailable_status`, and bare rows with one unmatched row
-- silently changed their sealed bytes, `content_fingerprint`, ledger
`control_totals`, and `revision_content_sha256` for adopters who never
opted in. A full-coverage manifest whose every cell was `present` lost
its totals entirely because one extra row was staged.

A control total is a checksum over the staged rows: `reporting_inspection`
and `_reconcile` verify it by recomputing from the revision's rows, so
unmatched rows belong in it and a derived constituent status says nothing
about it. Withdraw a column only when the adapter explicitly declared a
cell of that metric unavailable -- the case where row values exist but the
adapter has said they are not a measurement.

Also stop the zero-row mixing guard from naming `cell_availability`: a
derived result reaches it too, and an adopter who never set that field
should not be sent looking for it.

Verified all twelve representative legacy shapes now seal byte-identical
to origin/main, and re-ran 10,800 randomized override/fallback
combinations through the execution conformance validator.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@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 integration of two previously reviewed roots (#1173 per-metric evidence, #1176 currency) plus a durable-records test harness.

Checked:

  • Core inline_source.py cell resolution, batch-watermark advance, control-total withdrawal, and error classification.
  • Batch-watermark max() is safe: constituents and metrics both have min_length=1, so no empty-array edge case.
  • Multi-tenant staging isolation, clock injection, and semver signal verified.
  • Currency going keyword-only breaks no published caller since it never shipped positionally — no semver break, so no feat!:/migration-note requirement is triggered.

No Critical/High/Medium findings reported. No gated paths, no high-risk flag, no no-auto-approve team match, no prior decision. None of decision-table rows 1–8 fire, so this falls through to row 9.

A cell declaring a metric `missing`/`delayed`/`unsupported` withdrew that
metric's control total unconditionally, including when the declaring
constituent staged no rows at all. `validate_monetary_content` requires the
total for a monetary column every retained row carries, so the composition of
#1173's per-metric evidence with #1175's obligation currency sealed an
immutable publication the ledger then refused with `MONETARY_TOTAL_MISMATCH`
on the replay of every retry -- permanently wedging the obligation for the
answer per-metric evidence exists to give: a buy that delivered nothing and
whose billing is pending.

Distinguish the two shapes rather than exempting money from the withdrawal:

- A withdrawal by a constituent that staged rows still removes the total.
  Those rows carry values the adapter disclaimed, so summing them would
  contradict its own evidence, and the payload stays staged byte-for-byte.
- A withdrawal by a constituent that staged no rows reaches no sum and removes
  nothing. The checksum over the rows that were measured is retained.

A row-empty batch still withdraws, so unavailability is never published as an
observed zero. No total ever sums a disclaimed value.

Monetary columns have no third option: dropping the total is refused by the
ledger and keeping it would falsify the evidence. A withdrawn monetary cell
whose own constituent's rows report that metric now raises before anything is
staged or sealed. `ReportingDefinitionBinding.to_wire()` deliberately keeps
the frozen unit declarations off the wire, so the frozen slice request cannot
carry them; `InlineReportingSource(monetary_metrics=...)` lets a trusted caller
declare #1171's custom `monetary_metric_units` columns, with `spend` always
included. An undeclared custom money column keeps the non-monetary behavior.

Manifest and control-total bytes for a result with no `cell_availability` are
unchanged, byte-identical to the #1176 base.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@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 the delta, no blocking or medium findings.

Checked:

  • Withdrawal invariant traced through _publish, _validate_cell_rows, and _control_totals: narrowing the control-total withdrawal rule so a no-row constituent's withdrawal no longer drops neighbours' subtotal is sound.
  • New fail-closed guard rejecting a withdrawn monetary cell whose own staged rows report that metric (before staging/sealing) prevents ledger wedge on the monetary/sparsity interaction.
  • New keyword-only monetary_metrics param is additive/non-breaking on the public surface — no semver signal required.
  • Docs (docs/reporting-source-adapters.md, README.md) match behavior.
  • Three new parametrized conformance tests cover the new branches.

No gated paths, high_risk false, review_decision APPROVED, no author-team gate. No critical/high/medium findings — falls through to row 9.

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