Skip to content

Require commit on endpoints that state a record count - #201

Merged
chaksaray merged 1 commit into
aveproject:mainfrom
astrogilda:crosswalk-require-commit
Aug 25, 2026
Merged

Require commit on endpoints that state a record count#201
chaksaray merged 1 commit into
aveproject:mainfrom
astrogilda:crosswalk-require-commit

Conversation

@astrogilda

Copy link
Copy Markdown
Contributor

Step 3 of the sequence on #126. The commit field becomes required on endpoints that state a record_count, so a stated number can be re-derived instead of trusted. An endpoint stating no count has nothing to re-derive, so it is untouched.

Nine crosswalk files, eighteen endpoints, nine stating a count, and all nine already carry a commit. The scoped rule fails nothing on a clean checkout. Requiring commit outright still fails four sides that state no count: the ave-to-ast10 target and the cfgaudit, clawscan and skillspector sources. Re-measured on main at cd1010e, after #199 landed, rather than carried over from the estimate on #126.

The exclusion is load-bearing

The rule skips a side that declares itself unpinnable, and that clause is why this is not a one-line entry in the endpoint's required list.

The endpoint description defines three pinning states. One is a side that declares itself unpinnable and carries a content digest instead. Such a side can still state a count. Requiring commit of every count-stating endpoint would then demand a field the unpinnable rule directly above forbids. No document satisfies both, so that state stops being expressible, and nothing here would report it, because no endpoint declares itself unpinnable today.

The validator already agrees. Its warning reads does not declare pin_status unpinnable, so it was written with this scoping and only the schema was missing it.

Tests

Two staged tests flip with the promotion, as written.

A third needed restructuring rather than a new expected value. test_forbidding_commit_on_an_unpinnable_side_does_not_read_as_promoting_it proves the escalation switch ignores a required list underneath a not, and it read the shipped schema to prove it. That worked only while nothing else promoted the field. Against this branch it would pass for the wrong reason, and it would keep passing if the skip were deleted. It now removes the promotion first.

One test is new: an unpinnable side that states a count must validate.

Checks on this branch

Nine of nine crosswalks valid with zero warnings. Eighty records valid, with the six pre-existing attribution warnings unchanged. Eighty fixture pairs present. The suite is 342 passing, up from 339.

Both new assertions are mutation-checked. Deleting the exclusion fails only the unpinnable-with-count test. Reverting the promotion fails only the two promotion tests.

Promotes commit from optional to required, scoped to endpoints that state a
record_count, so a stated number can be re-derived rather than taken on
trust. An endpoint that states no count has nothing to re-derive, so it is
left alone.

Measured on main at cd1010e rather than assumed, and re-measured after the
owasp_asi audit landed: nine crosswalk files, eighteen endpoints, nine of
them stating a count, and all nine already carry a commit. The scoped rule
therefore fails nothing on a clean checkout. Requiring commit outright would
still fail four sides that state no count at all.

The exclusion for a declared-unpinnable side is load-bearing rather than
defensive. The endpoint description defines three pinning states, one of
which is a side that declares itself unpinnable and carries a content digest
instead, and such a side may still state a count. Requiring commit of every
count-stating endpoint without that exclusion would demand a field the
unpinnable rule immediately above forbids: no document satisfies both, and
one of the three states would quietly stop being expressible. A test now
covers that combination directly.

Two staged tests flip with the promotion, as they were written to. A third
needed restructuring rather than a new expected value: it proves the
escalation switch ignores a required list underneath a not, and it read the
shipped schema to do so, which only distinguished anything while nothing
else promoted commit. It now removes the promotion first, so it still fails
if the skip is deleted.

Validators on this branch: nine of nine crosswalks with zero warnings, eighty
records valid with the six pre-existing attribution warnings unchanged, and
eighty fixture pairs present. Suite is 342 passing, up from 339. Both new
assertions were mutation-checked: deleting the exclusion fails only the
unpinnable-with-count test, and reverting the promotion fails only the two
promotion tests.
@chaksaray

Copy link
Copy Markdown
Contributor

Traced the full diff, and everything holds up. The allOf/not interaction is correctly reasoned: without that exclusion, a declared-unpinnable endpoint stating a count would be simultaneously required and forbidden to carry commit, an unsatisfiable state that would silently delete one of the three pinning states the endpoint's own description defines. Good that this was caught in the design rather than found later as a bug.

Worth naming specifically: commit_is_required needed zero changes, because the new promotion lives in a required list that isn't nested under not, so the existing recursive walk from #171 picks it up on its own. That's real confirmation the original design was built more generally than it needed to be for the case it was first tested against, not a lucky coincidence.

The test restructuring is the sharpest part of this PR. Stripping the allOf out before asserting on test_forbidding_commit_on_an_unpinnable_side_does_not_read_as_promoting_it is exactly right, leaving it unchanged would have made it pass for a reason unrelated to what it's supposed to prove, and it would have kept passing even if the not-skip logic itself broke later. That's a real, non-obvious test-design catch.

Re-measuring fresh against cd1010e rather than the #126 estimate, same discipline this whole sequence has held to throughout.

Worth saying plainly: this closes the sequence that started with the crosswalk offer on #94, ran through the confidence_baseline design, the schema itself, the staleness audit, the pin-status mechanism, and the backfill. Genuinely one of the better-executed pieces of collaborative design work this project has had.

Merging.

@chaksaray
chaksaray merged commit 126a0ab into aveproject:main Aug 25, 2026
6 checks passed
@chaksaray chaksaray linked an issue Aug 27, 2026 that may be closed by this pull request
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.

Make commit required in crosswalk schema (after backfill lands)

2 participants