Skip to content

fix(schema): restore constraints canonical-forecast-point dropped - #6868

Open
bokelley wants to merge 1 commit into
mainfrom
fix-canonical-forecast-point-parity
Open

fix(schema): restore constraints canonical-forecast-point dropped#6868
bokelley wants to merge 1 commit into
mainfrom
fix-canonical-forecast-point-parity

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Summary

Implements item 7 of #6628. The canonical twin of forecast-point.json silently dropped three normative constraint groups its source carries:

  • the maximum: 1 bounds on viewable_rate range values,
  • the maximum: 1 bounds on metrics.coverage_rate range values,
  • the anyOf requiring standard whenever any viewability value is present.

Net effect before this fix: a canonical payload could carry viewable_rate: {mid: 1.3} under no stated viewability standard — a shape the source schema rejects — because nothing compares canonical pairs against their source twins (the enum-drift linter only covers inline enums).

This restores the constraints in the canonical file's compact style and adds tests/canonical-forecast-point-parity.test.cjs, which asserts the twins' viewability property sets match (with the brand-ref→brand-key swap pinned as the one intended difference), the anyOf is identical, the rate bounds agree, and — behaviorally, via Ajv — the canonical twin now rejects out-of-bounds rates and standard-less viewability values while accepting well-formed rows.

Constraint tightening on the unreleased canonical 3.2 surface; minor changeset. Item 8 of #6628 (the reporting-webhook frequency subset) remains WG-queued — deliberately not bundled here.

🤖 Generated with Claude Code

The canonical twin lost three normative constraint groups its source
forecast-point carries: the maximum-1 bounds on viewable_rate and
metrics.coverage_rate ranges, and the anyOf requiring standard whenever
any viewability value is present — so a canonical payload could carry a
viewable_rate of 1.3 under no stated standard that the source shape
rejects. Restore them in the compact style, and add a parity contract
test comparing the twins' viewability property sets, anyOf, and rate
bounds so this pair can no longer drift silently (tracked as item 7 in
the reporting-coherence findings).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@aao-secretariat aao-secretariat Bot added the ladon/needs-human-review Ladon has escalated this PR for human review. label Aug 25, 2026

@aao-secretariat aao-secretariat 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.

Ladon verdict: Escalate to human review

Escalate — gated path requires human/CODEOWNERS approval.

This PR modifies static/schemas/source/core/canonical-forecast-point.json, which matches the repo's gated-paths glob static/schemas/source/** (gated_paths: true). The required review status is REVIEW_REQUIRED, not APPROVED, so the hard approval gate (decision row 2) is not satisfied and the PR cannot be auto-approved.

The reviewer found no blocking issues: the change restores three normative constraint groups (viewable_rate/coverage_rate maximum: 1 bounds and the standard-required anyOf) to align the canonical twin with forecast-point.json's source, in the compact allOf style. The restored anyOf is byte-for-byte structurally identical, rate bounds match, a new Ajv-backed parity test pins all three groups plus behavioral accept/reject, and the change is correctly carried by a minor changeset with no schema↔docs drift. This is constraint tightening on the unreleased canonical 3.2 surface — a Normative (non-breaking) change, so the breaking-class hard gate does not independently apply.

The diff is clean, but the schema source file is under a deterministic hard approval gate that only lifts when review_decision is APPROVED. Human/CODEOWNERS review is required before merge.

Escalation reasons

  • Modifies static/schemas/source/core/canonical-forecast-point.json (gated path, static/schemas/source/**); review_decision is REVIEW_REQUIRED, not APPROVED.

Why human review

  • Modifies gated path static/schemas/source/core/canonical-forecast-point.json (static/schemas/source/**) while review_decision is REVIEW_REQUIRED, not APPROVED — human/CODEOWNERS approval required (decision row 2).
  • This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/canonical-forecast-point.json (modified) matches static/schemas/source/**) and the current GitHub review decision is 'REVIEW_REQUIRED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.

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

Labels

ladon/needs-human-review Ladon has escalated this PR for human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant