fix(reporting)!: allow conditional metrics under full coverage

[bokelley]

A full-coverage report currently fails when a display buy has no applicable completed_views metric, and source conformance rejects offering metrics declared partial. Admit both exact and partial metrics and let available cells coexist with inapplicable cells in a present constituent. Runtime and conformance preserve each cell's reason, watermark, semantic contract, and control-total behavior.

Only an offering metric declared support: partial with a reason may emit unsupported. Inline sources reject contradictory exact-support evidence before staging; producer admission and public conformance apply the same rule to custom executors. A completed custom-executor result with partial or no coverage is rejected when the frozen request requires full coverage. Pending billing uses MetricEvidence.delayed("billing_pending") and retains partial coverage.

A conditional metric may be unsupported in every constituent when other applicable measurements are complete. A constituent with zero applicable metrics must instead remain unsupported, and a batch containing only such constituents has coverage none. It cannot claim full or partial coverage, or an observed zero. A full request returns PARTIAL_RESULT without publication; a partial request may retain the diagnostic none result. Missing/delayed/stale cells still prevent full coverage. Available/unavailable mixtures with no rows retain the existing representation restriction.

Adapter migration: replace MetricEvidence.unavailable("billing_pending") with MetricEvidence.delayed("billing_pending") for an applicable measurement that has not arrived. Declare genuinely conditional metrics with support: partial and an offering reason before emitting unsupported cells. Custom executors must return PARTIAL_RESULT for incomplete full requests; all-unsupported diagnostics use constituent status unsupported and batch coverage none. These changes apply to adapter inputs and new executions; do not rewrite retained sealed evidence. See the source adapter guide for the complete rules.

Closes #1239.

Validation:

• make lint typecheck-all validate-generated and normal commit hooks pass.
• Focused source, service, evidence, and currency tests: 319 passed, including memory and real PostgreSQL 16.
• Producer lifecycle and memory/PostgreSQL ledger tests: 130 passed.
• Public service and conformance vectors cover mixed exact/partial offerings, a conditional metric unsupported everywhere, every metric unsupported in every constituent, exact-support contradictions in inline/custom executors, and forged full/partial coverage. New conditional-metric regressions fail against main.
• Full make test through the repository test harness: 12,167 passed, 2,408 skipped, 9 deselected, 1 xfailed, 81.67% coverage. PostgreSQL vectors are covered separately by the focused real-database run above and CI.
• ed134 independently cleared 07a53d1fb in a GitHub review comment; no blocking findings remain. All 53 checks passed on this head. aao-secretariat confirmed the implementation is sound. Brian chose a breaking release marker for the adapter validation change.

BREAKING CHANGE: An exact-support metric that emits unsupported evidence now fails instead of publishing partial. Use MetricEvidence.delayed for pending measurements, or declare the metric support: partial with a reason.

[bokelley]

REQUEST_CHANGES — independent review at e5ab6816a0c0fe59d742d48b94fd9638ec75d9b6

(GitHub refuses a formal verdict on a PR authored by the same account, so this is filed as a review comment. Reviewer session is independent of the author session; verdict applies to this exact head.)

The core change is sound and the new tests are real: reverting only src/adcp/reporting/ to main's 3e2325c7e and re-running test_reporting_conditional_metrics.py gives 5 failed, 2 passed — the regressions reproduce. The anti-forging guard, the unsupported-only roll-up, and the full-coverage denominator all behave as described, on memory and on real PostgreSQL 16 alike. Two P2s below.

P2-1 — unsupported cells are admitted under full coverage regardless of the offering's declared support

source.py:242 adds "unsupported" to the present roll-up without reference to MetricOfferingV1.support. The PR is titled and documented as conditional metrics, but the admission is unconditional: a metric the offering declares support: "exact" (the field default) can be withdrawn at the cell level and the slice still validates as full.

This repo's own fixture is that case, and this PR changed its expectation rather than the fixture. test_reporting_evidence_currency_integration.py:93 is MetricEvidence.unavailable("billing_pending") on spend, under an offering that declares spend: support="exact", reason=None. Lines 164 and 310 were flipped partial → full / ["present","partial"] → ["present","present"].

I ran the same fixture both ways, memory and PostgreSQL, identical results:

spend as 'unavailable': coverage=full     constituents=['present']  spend cell='unsupported'
spend as 'delayed':     coverage=partial  constituents=['partial']  spend cell='delayed'

So the flip to full is carried entirely by the evidence label — and by the rule this same commit adds at docs/reporting-source-adapters.md:46, billing_pending is the wrong label: "Use missing or delayed for an applicable measurement that has not arrived." Billing that has not closed is an applicable measurement that has not arrived. As it stands the canonical currency-conformance fixture, which is what adapter authors copy, demonstrates the pattern the new docs forbid, and the result is that full coverage is now asserted for a slice whose spend is genuinely pending.

Blocking ask (narrow): relabel that cell MetricEvidence.delayed("billing_pending") and restore lines 164/310 to partial, so the example agrees with the documentation.

Design question for the PR description, your call: should an unsupported cell be rejected outright for a metric the offering declares support: "exact"? _validate_request_against_capabilities already reads metric.support in this diff, so the signal is at hand, and partial + reason is precisely how an offering declares a metric may not apply. Without that gate, full no longer implies a source kept the exact-measurement promise it published.

P2-2 — the "requested metric unsupported in every constituent" behavior changed and is now untested

test_full_request_with_mixed_cells_fails_without_staging covered exactly this — metric_unsupported_everywhere(...) under coverage.expected == "full", asserting failure. It was replaced by a missing/delayed variant, and nothing asserts the new outcome. test_bulk_helpers_cover_only_the_requested_metric (test_inline_cell_availability.py:742) still builds that shape but only checks cell statuses, never coverage.status.

I confirmed the new outcome directly: with a requested metric unsupported in every constituent, every constituent is present, coverage.status == "full", and that metric has no control total anywhere. A consumer that asked for the metric under expected="full" is told the slice is fully covered while the metric has no evidence in the batch at all.

The new conditional-metrics test never exercises it — completed_views is only ever unsupported for display-buy, never for both constituents. If full is the intended answer, please assert it explicitly; the deleted test is the only thing that previously pinned it.

Nits (non-blocking)

• inline_source.py:1145: if cell_statuses == {"unsupported"} is subsumed by the following elif len(cell_statuses) == 1, which already yields "unsupported". The branch is a comment anchor, not logic — fine to keep, but worth confirming you didn't mean it to catch the no-overrides path too.
• The new guard fires only for status in _AVAILABLE_STATUSES, so the partial roll-up still launders the same zero-applicable-metrics condition: I can build a manifest whose every constituent is partial with all-unsupported cells and it validates with coverage.status == "partial", not none. Pre-existing (that row of _CONSTITUENT_ALLOWS_METRIC is unchanged), but the doc line this PR adds — "A result containing only such constituents has coverage none" — now claims the stronger property. inline_source won't emit it; a third-party adapter publishing against the model can.

Verified correct

• {explicit_zero, unsupported} → present is necessary, not a shortcut: _CONSTITUENT_ALLOWS_METRIC["explicit_zero"] admits only explicit_zero.
• All-unsupported cannot reach full — _status_matches_the_evidence counts only _AVAILABLE_STATUSES toward covered.
• The anti-forging guard rejects cleanly: metric status 'unsupported' contradicts constituent status 'explicit_zero'.
• Docs match the code.

What I ran (own worktree at this head, own venv, own PostgreSQL 16)

make lint	pass
make typecheck-all	pass (1472 + 44 files, no issues)
test_reporting_conditional_metrics.py + test_inline_cell_availability.py (memory + PG)	97 passed
test_reporting_evidence_currency_integration.py (memory + PG)	58 passed
test_reporting_source_contract.py, test_reporting_metric_evidence.py, test_reporting_inline_source.py, test_reliable_reporting_service.py	143 passed
same conditional-metrics file against main's src/adcp/reporting/	5 failed, 2 passed — regressions confirmed

Full tests/conformance/reporting/ on PG is still running; I'll follow up if it turns anything up.

[bokelley]

Both P2s are addressed in 76cfc1f0f:

• P2-1: billing-pending fixtures and the typed adopter example now use MetricEvidence.delayed, with partial expectations restored. Only declared partial support with a reason permits unsupported; inline staging, custom-executor producer admission, and conformance enforce it. Memory/PG tests cover both valid conditional evidence and exact-support rejection.
• P2-2: explicit full-request vectors cover a conditional metric unsupported in both constituents, and separately every metric unsupported in every constituent. The latter fails full publication, retains none only for partial requests, and cannot be forged into full or partial coverage through public conformance. Documentation covers the zero-denominator case.

Validation: 315 focused tests plus 130 producer/ledger tests pass, including real PostgreSQL 16. Lint, source/adopter typing, generated checks, and normal hooks pass. Fresh full make test and CI are running; the PR remains draft. Please re-review the correction on this head.

[bokelley]

Follow-up at 07a53d1fb: producer admission now also rejects a completed custom-executor none or partial manifest when the frozen request requires full coverage, matching conformance. The zero-applicability custom-executor regression failed on both memory and PostgreSQL before the fix; all 449 focused/producer/ledger tests now pass on the updated source. Lint/type/generated checks and normal hooks pass. Fresh full make test and CI are running on this head; ed134 re-review is requested here.

[bokelley]

APPROVE — re-review at 07a53d1fb266ef77f47ad33ca60dd5d0c00d70e5

(Review comment rather than a formal approval: GitHub refuses a verdict on a PR under the same account. Verdict applies to this exact head.)

Both P2s from my review of e5ab6816a are resolved, and the third issue found during the correction is resolved with it. I re-ran my own probes against each head rather than relying on the new tests.

P2-1 — resolved, and at the right layer

_validate_metric_applicability (source.py:1060) admits an unsupported cell only when the offering declares that metric support="partial" with a reason. My probe that previously slipped through — a metric declared exact, withdrawn with reason billing_pending — is now rejected with unsupported cells require partial metric support with a reason.

Enforcing it in three places is the right call, and the split is correct: the model validator can't see the offering, so SourceBatchManifestV1 still accepts such a manifest in isolation while inline_source, producer, and conformance each gate it against declared capabilities. The custom-executor parametrization on test_exact_support_cannot_withdraw_a_cell_as_unsupported is what makes that layering trustworthy — it proves the producer gate fires for an adapter that never goes through the inline source.

The sparse_fetch fixture is relabelled delayed/partial, so the canonical currency example now agrees with the documentation instead of contradicting it.

P2-2 — resolved

unsupported-everywhere is now an explicit case on test_full_core_publication_and_conformance_accept_conditional_metrics: completed_views unsupported for both constituents, both constituents present, coverage.status == "full", completed_views absent from both the control totals and the retained rows. That is exactly the assertion the deleted test used to pin. Naming the four cases (measured-everywhere, measured-zero, conditional-inventory, unsupported-everywhere) reads better than the old boolean too.

Third issue — independently reproduced on 76cfc1f0f, fixed on this head

I reproduced your custom-executor finding before the fix landed, with a DowngradingExecutor that rewrites coverage.expected to "partial" and answers the frozen full request. On 76cfc1f0f, memory and PostgreSQL: configuration_errors: {}, revisions_committed: 1, one revision persisted against a full-coverage obligation. Same probe on 07a53d1fb, both backends:

LedgerConflictError: a full-coverage request cannot complete with partial or missing coverage
revisions persisted: 0

The fix reads request.coverage.expected from the producer's own frozen request rather than anything the executor saw, which is what closes the substitution, and it sits ahead of _read_rows so nothing is read for a result that will be refused. test_custom_executor_cannot_complete_a_full_request_with_diagnostic_coverage covers both none and partial on both backends and also asserts conformance rejects the same sealed execution — good, since these are now two independent gates on the same rule and they should not be able to drift apart silently.

Earlier nits

• The partial roll-up laundering I flagged as pre-existing is closed as a side effect: the guard is now status != "unsupported" and constituent_id not in applicable_constituents, so any non-unsupported roll-up needs at least one applicable cell. My probe that built an all-unsupported partial constituent is rejected with a constituent without applicable metrics must be unsupported. The doc line "a result containing only such constituents has coverage none" is now true of the model, not just of the inline source.
• The redundant cell_statuses == {"unsupported"} branch remains. Still harmless, still fine as a comment anchor.

What I ran on this exact head

make lint	pass
make typecheck-all	pass
conditional-metrics + inline-cell-availability + currency-integration + feed-contract (memory + real PG 16)	231 passed
source-contract + metric-evidence + inline-source + reliable-service	148 passed
my own coverage-gate probe, memory + PG	red on 76cfc1f0f, green on 07a53d1fb

No blocking findings remain. Worth confirming CI's PG lanes are green on this exact head before it leaves draft, since three of the four gates added here only execute under a real database.

[bokelley]

Local validation is complete on 07a53d1fb: make test passed with 12,167 passed, 2,408 skipped, 9 deselected, 1 xfailed, 81.67% coverage (repository test harness, exit 0). The combined focused/producer/ledger run passed 449 tests, including real PostgreSQL 16. Lint, source/adopter type checks, generated-code checks, and normal hooks pass. ed134's independent re-review has no blocking findings. CI currently has 31 passing checks and 18 pending; keeping draft until those finish.

[aao-secretariat]

Ladon verdict: Comment (human reviewer recommended)

Comment — potential unmarked breaking change on the adapter surface; human should confirm semver signal.

The reviewer found the core roll-up, coverage floor, and applicability enforcement across inline sealing, conformance, producer commit, and the full-coverage guard to be sound and fail-closed, backed by extensive tests. No critical or high findings were formally posted, so the decision table's row 1 does not fire, and there are no gated paths, high-risk deletions/modifications, or team gates in play.

However, the reviewer surfaced one inline concern worth a human's eye: _validate_metric_applicability (src/adcp/reporting/conformance.py) changes adapter-surface behavior — unsupported cells on non-partial metrics now hard-fail where they previously published as partial — yet the change ships under a non-breaking fix(reporting): prefix. Per this repo's mandatory semver-signal rule, a change to a response model's shape / adapter behavior that stops existing consumers from getting prior output can warrant fix!: / BREAKING CHANGE: plus a migration note. release-please would otherwise cut a patch/minor and the break would ship without a major.

Because the reviewer classified this as a judgment call rather than a firm high finding (no findings recorded in the findings list), I am not blocking. But this is not cleanly auto-approvable: a maintainer should decide whether a breaking marker and migration note are warranted before merge. Surfacing as a comment so a human weighs the semver signal.

Medium findings

• src/adcp/reporting/conformance.py — _validate_metric_applicability changes adapter behavior (unsupported non-partial cells now hard-fail) under a non-breaking fix(reporting): prefix; confirm whether fix!:/BREAKING CHANGE: + migration note is required.

[bokelley]

Brian's release-policy decision (2026-09-29): use the breaking marker for #1247. The PR title is now fix(reporting)!: allow conditional metrics under full coverage, and the PR body carries this exact squash footer:

BREAKING CHANGE: An exact-support metric that emits unsupported evidence now fails instead of publishing partial. Use MetricEvidence.delayed for pending measurements, or declare the metric support: partial with a reason.

The tested source head is unchanged. The same breaking-marker treatment applies to #1241/#1248, whose title and PR body already include it. This resolves the semver classification raised by the aao Medium review thread.

[aao-secretariat]

Ladon verdict: Approve

Approve — clean re-review at head 07a53d1 with no findings.

No blocking or medium findings. Verified the conditional-metric roll-up (unsupported-only → unsupported, available+unsupported mix → present), the applicability floor enforced fail-closed at staging/producer-admission/conformance, the full-coverage guards in producer and conformance, and the correct breaking semver signal (fix(reporting)!: + BREAKING CHANGE footer with migration note). CI gates, import layering, and credential rules all intact.

Decision-table walk: no critical/high (row 1 n/a); gated_paths false (row 2 n/a); high_risk false, no deletions (rows 3-5 n/a); prior decision was comment, not escalate, and this run has no new findings anyway (row 6 n/a); no no-auto-approve team match (row 7 n/a); zero medium findings (row 8 n/a) → row 9 approve.

Note: review_decision is REVIEW_REQUIRED, but gated_paths is false so row 2 does not apply and the diff falls through to approve.