Skip to content

Add shared parity cases with independent expected-result validation - #2104

Merged
ryanbr merged 4 commits into
ryanbr:mainfrom
bhelm:feature/parity-guard-pr2
Sep 21, 2026
Merged

ryanbr merged 4 commits into
ryanbr:mainfrom
bhelm:feature/parity-guard-pr2

Conversation

@bhelm

@bhelm bhelm commented Sep 11, 2026

Copy link
Copy Markdown

Add shared parity cases with independent expected-result validation

PR1 (#1534) protects the static Swift/Kotlin inventory and parity-debt baseline;
it does not execute both platforms. PR2 adds the portable behavioral-test
contract: cases with shared inputs and literal expected results, deterministic
expansion, an authoritative shard/operation registry, exact split/remerge, and
input-bound result artifacts.

A platform can validate its own artifact against the shared expectations without
building the other platform or waiting for its output. A later comparison takes
both result files and checks their agreement without rebuilding either platform.
Both sides are also checked against the expected results, so two implementations
returning the same wrong answer cannot pass merely by agreeing. Expected values
are checked-in literals, not results generated from the Python reference runner.

Every case hash binds its inputs, identity, function, shard, expected result and
explicit suite version. Output metadata must match those cases and the full Git
source revision explicitly required by the caller. A stale or mixed revision,
changed case, malformed output or missing result fails closed. Revision metadata
is a consistency contract, not cryptographic proof of which code ran; later CI
must establish trustworthy build/artifact provenance. Source revisions are kept
out of persistent case definitions so cases remain reusable across code changes.

Relationship to native tests and the remaining series

The goal is to maintain platform-independent behavioral cases once and execute
them on each platform. UI, permissions, OS integration and platform-internal
checks remain native. PR2 preserves all existing native tests and contains only
a small Python clamp demonstration plus mutant controls on both sides. Python
agreement is not evidence of Swift/Kotlin product parity.

PR3 adds native runners and migrates the first small real algorithm's existing
Swift/Kotlin cases into the shared format, preserving their expected assertions.
Redundant native cases should only be removed after their former checks are
proven to run through both adapters. Further migrations follow incrementally.
PR4 adds product-source CI enforcement. PR2 extends the governance tool self-tests
and case-spec triggers without introducing coupled native builds or enforcement.

See Tools/PARITY_HARNESS.md for exact metadata fields, standalone and artifact-only
CLI examples, canonical JSON rules and exit codes.

Validation

All 138 governance tests pass, including 35 portable-harness tests. Broad Tools
self-test discovery passes all 247 tests (the same 138 governance tests plus
109 adjacent tests). Coverage includes standalone CLI success without a peer
artifact, agreeing-but-wrong failures, exact oracle types, stale/mixed revisions,
version/hash binding, required expectations, and both mutant controls.
Independent subprocess checks and read-only review passed. git diff --check
passes; the draft OKF record has no conformance or security errors.

@ryanbr

ryanbr commented Sep 14, 2026 •

Copy link
Copy Markdown
Owner

Reviewed at eabfad9. The harness design holds up where it matters. One thing in the diff does not belong to it.

Verified rather than taken on trust

The anti-circularity property is real, which is the whole load-bearing claim. core_reference.json carries hand-written literals ({"id": "above", "expected": 10, "args": {...}}), not values emitted by the Python runner, so a platform is checked against the expectation and not against whatever the reference happened to produce. That is what stops two implementations agreeing on the same wrong answer, and it only works if the literals stay hand-written; worth guarding in review on every later spec, since generating them would quietly remove the property while every test still passed.

The discovery floor moved with the suite. 103 to 138, with Tools/parity_case_specs/** added to both path filters and the acceptance test's baseline list updated to match. Raising that floor is the part people forget, and the error text already says the right thing: discovery is broken, not the suite.

138 tests pass. Worth saying how I got there: they failed once locally, on test_checked_in_inventory_and_baseline_match_current_sources, and that was my working tree rather than this PR. The inventory scanner walks the filesystem, so a tree carrying anything from another branch produces a false failure on exactly that test. In a pristine clone of this head it is 138 and OK, which agrees with CI.

okf/ does not belong in this PR

Three files under a new top-level okf/ directory, carrying okf_version: "0.2" frontmatter, a "Directory Update Log", and notes recording "local PR2 preparation status" and that "The concept remains draft". That is your own project-knowledge tooling, not NOOP documentation, and it arrives as a new top-level directory in a repository it has no other presence in.

Whether this repo carries such a bundle is something I would want to decide deliberately, not inherit from a harness PR. Please drop those three files; the harness stands entirely without them.

One thing that is a judgement call, not a defect

The only operation this currently exercises is Portable.clamp/3, so the 1,400 lines are contract and scaffolding whose value arrives with PR3's native runners and first real migration. You say so plainly, including that Python agreement is not evidence of Swift/Kotlin parity, and that honesty is why this reads as a staged plan rather than as overreach. It does mean I would be accepting the harness now on the strength of the series continuing, which I am willing to do knowingly; I just want it said out loud rather than arriving as an unnoticed consequence of merging scaffolding.

Drop okf/ and I am happy with the rest.

The okf/ notes are local project-knowledge tooling, not NOOP
documentation; the parity harness does not depend on them.
@bhelm

bhelm commented Sep 14, 2026

Copy link
Copy Markdown
Author

Dropped okf/ in a follow-up commit; the PR now only touches the harness, its tests, the case spec and the workflow. Parity-governance suite passes locally (138 tests).

@ryanbr

ryanbr commented Sep 21, 2026

Copy link
Copy Markdown
Owner

Re-reviewed at 0ea15d7. My previous review had exactly one ask and it has been met.

okf/ is gone. The diff is now 7 files, all under Tools/ plus the one workflow, with nothing outside and nothing new arriving alongside the removal. Thank you for taking that cleanly rather than arguing it.

The discovery floor was re-derived a second time. I recorded it moving 103 to 138 last time; it now reads 122 to 159, so you re-derived it after main moved underneath the branch instead of leaving the stale number in place. That is the part that usually rots, and it has now survived two rebases.

I measured it rather than trusting the arithmetic. Merged onto current main (a6c1c01) in a pristine worktree, CI's exact command gives Ran 159 tests ... OK, 35 of them from test_parity_harness. The pristine part matters here specifically: the inventory scanner walks the filesystem, so a tree carrying anything from another branch produces a false failure on test_checked_in_inventory_and_baseline_match_current_sources alone. That misled me on the first review and I did not want to report it as a regression twice.

One observation, inherited rather than introduced by you. 159 is exactly main's current 124 plus your 35, so there is no margin. The gate is -lt, so it only trips when tests go missing, which is the intent. But a future change that legitimately removes one governance test will produce "discovery is broken, not the suite" on main, landing on whoever pushes next rather than on the change responsible. The old 122 floor had the identical property, so this is a known cost of the design and not a reason to hold the PR. Worth writing down somewhere, because this repository has been bitten twice today by a different gate with the same shape: the parity-governance path filter excludes product source, so ordinary feature merges invalidate the derived authority and nothing reports it until the next scheduled run fails on a bystander.

Taking it without a rebase. It is 101 commits behind, but I have verified the actual merge result twice, against f8ada93 and again against a6c1c01, and the merge is clean at 7 files both times. CI validated your head against its base; I validated the merged tree against current main.

Reaffirming what I said last time, since merging makes it real: the only operation exercised is Portable.clamp/3, so this is contract and scaffolding whose value arrives with PR3's native runners and the first real migration. You say that plainly, including that Python agreement is not evidence of Swift/Kotlin product parity, and that honesty is why this reads as a staged plan rather than overreach. Accepting it now means accepting it on the strength of the series continuing. I am doing that knowingly, and saying so out loud rather than letting it arrive as an unnoticed consequence of merging scaffolding.

The anti-circularity property remains the load-bearing claim and it still holds: core_reference.json carries hand-written literals, not values emitted by the Python runner, so a platform is checked against the expectation rather than against whatever the reference produced. Please keep that invariant in every later spec. Generating those literals would remove the property silently while every test still passed.

@ryanbr
ryanbr merged commit 9590d72 into ryanbr:main Sep 21, 2026
5 checks passed
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.

2 participants