Skip to content

[codex] Make signal coverage scalar optional#5102

Merged
bokelley merged 1 commit into
mainfrom
issue-5089
May 28, 2026
Merged

[codex] Make signal coverage scalar optional#5102
bokelley merged 1 commit into
mainfrom
issue-5089

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Summary

Makes deprecated coverage_percentage optional in get_signals and wholesale signal feed payloads while keeping coverage_forecast authoritative for signal-level coverage. Updates signal docs, compliance storyboards, and schema tests to cover aggregate present buckets, low/high coverage bounds, and wholesale payloads without the legacy scalar.

Validation

Ran npm run test:schemas, npm run test:composed, npm run test:json-schema, npm run build:schemas, npm run build:compliance, precommit checks, pre-push storyboard matrices, and docs broken-link validation. npm run test:snippets is still known-red for unrelated auth/live-fixture drift; tracked separately in #5100.

Closes #5089.

@bokelley bokelley marked this pull request as ready for review May 28, 2026 07:32

@aao-release-bot aao-release-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Surgical relaxation of an already-deprecated field; coverage_forecast is the authoritative discovery surface and the scalar stays fallback-only.

Things I checked

  • static/schemas/source/signals/get-signals-response.json and static/schemas/source/core/wholesale-feed-event.json: only the required arrays drop coverage_percentage. type: number, minimum: 0, maximum: 100, and deprecated: true are preserved verbatim. Descriptions updated to name coverage_forecast authoritative.
  • presence: "present" without signal_value is schema-legal per static/schemas/source/core/forecast-point-dimensions.json — the conditional only constrains the type of signal_value when present, doesn't promote it to required. New test at tests/schema-validation.test.cjs:662-679 pins this shape.
  • completeExclusiveCoverageRatesPartition rewrite at tests/schema-validation.test.cjs:787-796 iterates low/mid/high, skips a level if every point omits it, fails if some points have it and others don't. Right invariant.
  • Changeset exists at .changeset/signal-coverage-legacy-scalar.md with minor. Defensible — .changeset/4796-review-followups.md is the precedent (dropping a required array entry on a deprecated field shipped as minor). Field has been advertised deprecated: true since #5088 landed in the same cycle, so strict consumers had a window.
  • Compliance YAML: signal-marketplace/index.yaml drops the field_present: signals[0].coverage_percentage assertion. signal-owned/index.yaml never had an equivalent assertion (its validations at L134-153 only cover signal_agent_segment_id, pricing_options, context, signal_id.source) — symmetric with marketplace by construction. universal/schema-validation-signals.yaml narrative correctly trimmed to schema-required fields.
  • No doc surface still calls coverage_percentage required.

Follow-ups (non-blocking — file as issues)

  • Strict-validator adopter-action row. Codegen consumers (Pydantic, quicktype, OpenAPI) will see the field flip required→optional and need fixture refreshes. Worth a row in docs/reference/release-notes.mdx matching the precedent set by #4796.
  • Storyboard assertions trail the narrative. signal-marketplace/index.yaml:118 and signal-owned/index.yaml:118 expected: blocks now describe coverage_forecast as the disclosure surface but neither validates signals[0].coverage_forecast presence. The prose moved ahead of the assertions.
  • min_coverage_percentage request filter. docs/signals/tasks/get_signals.mdx:711 still documents it as operating on the legacy scalar. With the scalar now optional, the filter's denominator semantics get muddier — a sentence on filter behavior when the scalar is absent would help.

Minor nits (non-blocking)

  1. Vacuous-pass on empty partition. tests/schema-validation.test.cjs:787-796 returns true if every point omits all of low/mid/high. Pre-existed in the mid-only version, inherited here. Tracking anyLevelPresent and returning false if no level had numeric data on any point closes the gap.
  2. Idiomatic clone. tests/composed-schema-validation.test.cjs:732 builds signalListingCoreWithoutLegacyCoverage via spread+delete; const { coverage_percentage: _legacy, ...rest } = signalListingCore is one line. Pure style.

Approving on the strength of the surgical required-array trim plus the new low/high partition coverage. Safe to merge.

@bokelley bokelley merged commit 1584e44 into main May 28, 2026
32 checks passed
@bokelley bokelley deleted the issue-5089 branch May 28, 2026 07:38
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.

Follow up signal coverage forecast schema cleanup

1 participant