Skip to content

Make CONTRACT-001 content-digest honour the shared-drift allowlist its sibling honours - #103

Draft
yakimoto wants to merge 1 commit into
mainfrom
fix/content-digest-honours-allowlist
Draft

yakimoto wants to merge 1 commit into
mainfrom
fix/content-digest-honours-allowlist

Conversation

@yakimoto

@yakimoto yakimoto commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

What this fixes

CONTRACT-001 has two sub-checks. They run over the same operation set, and they disagreed about which divergences are accepted.

operation-parity reads compare()'s findings, which are post-allowlist. content-digest re-derived its own list from the raw shared keys and subtracted nothing:

const sharedKeys = [...repoOps.keys()].filter((k) => liveOps.has(k)).sort();
for (const key of sharedKeys) { /* … digest every one … */ }

That is not a stricter check. It is an incoherent one, and the incoherence is load-bearing: operation-parity deliberately filters shared-drift out of its own verdict, with a comment saying that condition is "already reported, more precisely, by content-digest below." So the allowlist's entire shared-drift direction was suppressing a finding that nothing then consumed — nine documented, owner-attributed, predicate-guarded exemptions were dead letters, and the criterion they were written for failed on them anyway. Either the allowlist means something to this criterion or the shared-drift direction should not exist; it cannot half-mean something.

Confirming the asymmetry before changing anything

Measured against origin/main and a snapshot of the published contract fetched 2026-09-11:

  • The allowlist holds 11 entries: 9 shared-drift + 2 unpublished-repo. Only the 9 are candidates for subtraction here. (The 11 in operation-parity's evidence line is the total, not the shared-drift count.)
  • All 9 still satisfy their lapsing predicate, and all 9 differ under the digest — so all 9 were being both exempted and failed simultaneously.

What changed

The digest now excludes exactly the operations compare() still exempts — nothing more.

Granularity is per-operation: an exempted operation contributes neither its own object nor its reachable $ref content. That is deliberate and it is measured, not assumed. Six of the nine also diverge in referenced component schemas, precisely because of the divergence their entry documents (a gateway-native or draft-placeholder operation reaches different components than the hand-written one), so a refs-only carve-out would exempt the stated cause and still fail on the consequence that flows from it.

The residual risk is stated plainly in the file header rather than hidden: while an exemption stands, a change to a component only that operation reaches is not seen. That is intrinsic to granting the exemption, not introduced here, and it is bounded by machinery that already exists — every entry carries a lapsing predicate, a lapsed entry drops straight back into findings and therefore back into this digest, and compare() separately surfaces entries matching nothing.

The exempted count is now reported in the evidence line on pass and fail both, so a growing exemption set is visible in the receipt itself.

opts.allowlistPath is an in-process test seam only, deliberately not env-readable — the CLI passes no opts, so a real run can only ever read the committed allowlist at its fixed path.

Proof — local runs only

CI is unmeasured org-wide right now (an account-level billing lock removed the required checks); nothing below comes from CI, and this PR is opened as a draft so auto-merge cannot arm.

measurement before after
new tests in contract-001-check.test.mjs 0 / 6 pass 6 / 6 pass
npm run test:ga 35 / 35 41 / 41
node --test ".github/scripts/**/*.test.mjs" 93 / 93 93 / 93
differing shared ops, origin/main spec 56 of 255 47 (9 excluded)
differing shared ops, PR #101's spec 20 of 255 11 (9 excluded)

The pass-after test fails on origin/main for the right reason — content-digest.ok is false !== true, not a missing option.

content-digest still FAILS after this change, and that is the correct outcome. This PR removes a defect in the criterion; it does not clear the criterion. Eleven genuine content differences remain.

Both directions are pinned

Subtracting an exemption from a digest is exactly the shape of change that can quietly become a way to switch a criterion off, so showing that an allowlisted difference stops failing is not enough. The new tests also hold that:

  • a non-allowlisted difference still fails, while a sibling exemption stands;
  • a non-allowlisted $ref-only difference still fails (the reachable-ref fold survives the subtraction);
  • an entry whose lapsing predicate no longer matches is not subtracted;
  • an entry in any other direction is not subtracted — the subtraction is keyed on shared-drift, not on path+method.

An exemption may only ever remove an operation from the comparison. It may never turn a real mismatch into a pass.

The eleven real differences — reported, not allowlisted

No allowlist entry was added. The allowlist is unchanged at 11. These are itemised in CHANGELOG.md for separate per-operation judgement:

  • Six are the 3.0-vs-3.1 nullable divergence (POST /compose, POST /compose/run, GET /compose/proposals/{proposalId}, plus prose-only diffs on POST /render, POST /captions, POST /transcribe). CHANGELOG.md already records this and states it "does not affect the … shared-drift finding" — that is true of the operation-level comparator only. content-digest resolves $ref content and does see it. That sentence needs correcting; this PR corrects it.
  • Three reach Error / ErrorBody schemas the published contract does not define at all (GET and POST /videos/{videoId}/chapters, POST /videos/{videoId}/chapters/detect). A missing error envelope is a real spec-vs-service gap.
  • Two are repo-side validation constraints absent live — SearchIndexRequest/SearchIndexDoc.id.pattern, and BraidPublishRequest.sources.minItems: 2 with its anyOf required-one-of. These are the interesting ones: the repo promises a constraint the gateway may not enforce.

None is an allowlist candidate on current evidence.

Relationship to #101

Independent, no file overlap in the fix itself. #101 edits openapi.yaml + contract-drift.json; this edits scripts/ga/*. Both add to CHANGELOG.md under [Unreleased] — expect a trivial textual conflict there and nothing more. #101's own body named this defect and left it for a reviewed change; this is that change.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Summary by Sourcery

Align CONTRACT-001 content-digest with operation-parity by applying only active shared-drift exemptions while preserving detection of all other content mismatches.

Bug Fixes:

  • Make CONTRACT-001's content-digest check honor active shared-drift allowlist exemptions consistently with operation-parity.
  • Ensure expired or differently directed allowlist entries do not suppress digest mismatches.

Enhancements:

  • Expose compared, shared, and excluded operation counts in content-digest evidence.
  • Add an in-process allowlist path for hermetic testing without changing the CLI's committed allowlist behavior.

Documentation:

  • Document the allowlist behavior, residual risk, and the remaining genuine content differences in the changelog.

Tests:

  • Add coverage confirming allowlisted operations are excluded while non-allowlisted operation and reachable-reference differences still fail.

Review in cubic

operation-parity and content-digest are siblings over the same operation set,
but they disagreed about which divergences are accepted. operation-parity reads
compare()'s post-allowlist findings; content-digest re-derived its own list from
the raw shared keys and subtracted nothing.

That was not a stricter check, it was an incoherent one. operation-parity
deliberately filters shared-drift out of its own verdict ("already reported,
more precisely, by content-digest below"), so the allowlist's entire
shared-drift direction suppressed a finding that nothing then consumed: nine
documented, predicate-guarded exemptions were dead letters, and the criterion
they were written for failed on them anyway.

The digest now excludes exactly the operations compare() still exempts, at
operation granularity - object and reachable $ref content. Six of the nine
diverge in referenced schemas precisely because of the divergence their entry
documents, so a refs-only carve-out would exempt the stated cause and still fail
on its direct consequence. The residual risk is stated in the file header.

An exemption can only remove an operation from the comparison; it can never turn
a real mismatch into a pass. Six new tests pin both directions: a lapsed
predicate is not subtracted, a non-shared-drift direction is not subtracted, and
a non-allowlisted difference (including a $ref-only one) still fails while a
sibling exemption stands. The exempted count is reported in the evidence line on
pass and on fail, so a growing exemption set stays visible.

opts.allowlistPath is an in-process test seam only, deliberately not env-readable
- the CLI passes no opts, so a real run can only read the committed allowlist.

No allowlist entry was added and no gate was weakened. The allowlist is unchanged
at 11 entries and content-digest still fails on eleven genuine content
differences, itemised in CHANGELOG.md for separate per-operation judgement.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Comment @coderabbitai help to get the list of available commands.

@sourcery-ai

sourcery-ai Bot commented Sep 11, 2026

Copy link
Copy Markdown

Reviewer's Guide

This PR fixes CONTRACT-001’s inconsistent sibling checks by making content-digest consume the same active shared-drift exemptions as operation-parity, excluding exempt operations and their reachable references while retaining all other drift detection. It adds focused hermetic tests, makes skipped-work counts visible in evidence, preserves the committed allowlist as the production source, and documents the residual risks and remaining genuine failures.

Sequence diagram for CONTRACT-001 sibling checks sharing exemptions

sequenceDiagram
    participant Run as CONTRACT001
    participant Compare
    participant Allowlist
    participant Digest

    Run->>Allowlist: read committed allowlist
    Run->>Compare: compare(repoDoc, liveDoc, allowlist)
    Allowlist-->>Compare: active shared-drift exemptions
    Compare-->>Run: post-allowlist findings and allowlisted entries
    Run->>Digest: select shared operations
    Digest->>Digest: exclude active shared-drift operations
    Digest->>Digest: normalize operations and reachable refs
    Digest-->>Run: repo/live digest comparison
    Run-->>Run: report compared and exempted counts
Loading

State diagram for a shared-drift allowlist entry

stateDiagram-v2
    [*] --> Candidate
    Candidate --> Active: expect or expectAbsent matches
    Candidate --> Lapsed: predicate no longer matches
    Active --> ExcludedFromDigest: direction is shared-drift
    Active --> Compared: another direction
    Lapsed --> Compared
    ExcludedFromDigest --> [*]
    Compared --> [*]
Loading

Flow diagram for CONTRACT-001 content-digest allowlist handling

flowchart LR
    A["repo and live OpenAPI documents"] --> B["indexOperations"]
    B --> C["compare with active allowlist"]
    C --> D["shared-drift exemptions"]
    C --> E["shared operation keys"]
    E --> F["exclude exempt operations"]
    D --> F
    F --> G["normalize operations and reachable refs"]
    G --> H["compute repo and live SHA-256 digests"]
    H --> I["content-digest result"]
Loading

File-Level Changes

Change Details Files
Align content-digest with operation-parity by applying active shared-drift allowlist exemptions at operation granularity.
  • Derive exempt operation keys from compare()'s post-allowlist results, filtering exclusively on shared-drift.
  • Exclude each exempt operation and its reachable $ref content from both digest walks.
  • Preserve lapsed, differently directed, and non-allowlisted differences in the digest.
  • Expose shared, compared, and excluded operation counts in result evidence and details.
  • Document the residual visibility tradeoff and allowlist semantics.
scripts/ga/contract-001-check.mjs
Add hermetic coverage proving exemptions are narrowly applied without weakening genuine drift detection.
  • Add an injectable allowlist path for tests while keeping the CLI bound to the committed allowlist.
  • Cover before/after exemption behavior, sibling non-exempt drift, $ref-only drift, lapsed predicates, and incorrect directions.
  • Assert digest status and shared/digested/exempted counts.
scripts/ga/contract-001-check.test.mjs
Document the corrected CONTRACT-001 behavior and remaining content differences.
  • Record the allowlist alignment fix and six-test coverage.
  • Explain why eleven genuine digest differences remain and are not allowlisted.
  • Correct the prior statement about nullable drift not affecting the criterion.
CHANGELOG.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@cubic-dev-ai cubic-dev-ai 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.

All reported issues were addressed across 3 files

Architecture diagram
sequenceDiagram
    participant CLI as CLI (scripts/ga/main)
    participant Check as contract-001-check.mjs
    participant Compare as compare()
    participant Allowlist as published-drift-allowlist.json
    participant Index as indexOperations()
    participant Normalize as normalizePair()
    participant Digest as content-digest logic

    Note over CLI,Digest: CONTRACT-001 content-digest flow

    CLI->>Check: run() (no opts - production)
    Check->>Check: Load repo spec + live doc
    
    Check->>Index: indexOperations(repoDoc)
    Index-->>Check: repoOps
    Check->>Index: indexOperations(liveDoc)
    Index-->>Check: liveOps
    
    Check->>Check: Compute sharedKeys (intersection)
    
    Check->>Compare: compare(repoOps, liveOps, allowlist)
    Compare->>Allowlist: Load and evaluate exemptions
    Allowlist-->>Compare: allowlist entries
    
    alt Entry lapsing predicate matches
        Compare->>Compare: Include in result.allowlisted
    else Entry lapsed or no match
        Compare->>Compare: Include in findings (not allowlisted)
    end
    
    Compare-->>Check: result { allowlisted, findings }
    
    Check->>Check: Filter allowlisted entries for direction == 'shared-drift'
    Check->>Check: Build exemptKeys set (method + path)
    Check->>Check: digestKeys = sharedKeys - exemptKeys
    Check->>Check: Calculate exemptedCount
    
    loop For each key in digestKeys
        Check->>Repo: Get repo operation
        Check->>Live: Get live operation
        Check->>Normalize: normalizePair(repoOp, liveOp, path, method)
        Normalize-->>Check: { repo, live }
        Check->>Check: Walk operation + reachable $refs
        Check->>Check: Compute repoDigest + liveDigest
    end
    
    Check->>Check: Build evidence with counts
    Check->>Check: Evaluate content-digest result
    
    alt localDigest === liveDigest
        Check-->>CLI: ok: true + detail with counts
    else digests differ
        Check-->>CLI: ok: false + detail with counts
    end
Loading

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread CHANGELOG.md
Comment thread scripts/ga/contract-001-check.mjs
Comment thread CHANGELOG.md
@wave-bugbot

wave-bugbot Bot commented Sep 26, 2026

Copy link
Copy Markdown

🟢 WAVE BugBot — clear

No confident findings on the changed lines.

Local review · $0 inference · wave-dispatch

yakimoto added a commit that referenced this pull request Oct 1, 2026
wave-gateway omits every operation of an operator-paused product from
GET /openapi.json (src/openapi-paused.ts) and declares it at the document
root as x-wave-paused-products. Build 06e4efce1050 (live 2026-10-01)
declares /v1/render, /v1/renders, /v1/enhance and /v1/video-gen. The
compare never read that, so CONTRACT-001 operation-parity FAILED on main
with four unexplained unpublished-repo findings (POST /enhance,
POST /render, GET /render/{jobId}, GET /render/{jobId}/events): red for
a reason no change to this repo can fix.

published-drift-paused.mjs (new leaf, pure): a repo operation that is
ABSENT from the published document under a path prefix the document
itself declares paused is listed as pausedNotPublished and counted in the
headline, never dropped silently. A paused operation that is published is
still compared; an absence with no declaration is still a finding; a
prefix never covers a sibling (/v1/render does not cover /v1/renders).

Live: operation-parity PASS (256 declared, 249 live, 248 shared, 9
allowlisted, 0 draft, 6 paused). content-digest still fails, as on main
(#103).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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