Skip to content

test: frozen replay-compat corpus with expected verdicts (#1417) - #1436

Merged
thymikee merged 8 commits into
mainfrom
devin/1785159001-replay-compat-corpus
Jul 28, 2026
Merged

test: frozen replay-compat corpus with expected verdicts (#1417)#1436
thymikee merged 8 commits into
mainfrom
devin/1785159001-replay-compat-corpus

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #1417 (umbrella #1412 Track A). A frozen .ad corpus of scripts released versions wrote, each paired with the verdict today's parser owes it, gated in unit-core.

test/replay-compat/manifest.ts is the whole contract:

{ id: 'integration/android-06-swipe-gestures@v0.11.2',
  file: 'scripts/integration/android-06-swipe-gestures.v0.11.2.ad',
  recordedBy: 'v0.11.2',                                   // must be in REPLAY_COMPAT_RELEASED_TAGS
  provenance: { kind: 'mined', path: 'test/integration/replays/android/06-swipe-gestures.ad',
                blob: '686bace…' },                        // git object id of the historical blob
  covers: ['context-header', 'retired-gesture', 'wait-landmark'],
  verdict: { kind: 'fails', code: 'INVALID_ARGS', hint: 'swipe accepts 4 arguments…' },
  note: '#1393 retired the trailing swipe durationMs…' }

corpus.test.ts asserts each verdict through production parseReplayInput(script, undefined) — the same composition, in the same action-before-metadata order, that replay/test hit — so a multi-fault script reports the code/hint replay really surfaces.

Freezing is mechanical, not asserted in prose:

  • mined bytes must hash to the pinned git blob id (sha1("blob <len>\0" + bytes)), an id obtainable only from the released content, so "edit the script until the parser agrees" cannot pass as a corpus update; derived bytes (scripts/docs/, forms the release's grammar wrote that no shipped .ad carries) are SHA-256-pinned.
  • provenance-rules.ts fixes the kind by corpus area (integration/examplesmined, docsderived, unknown area throws), closing the loophole where an edited mined script is relabelled derived, re-pinned by digest, and skipped by the history verifier. Negative test included.
  • pnpm check:replay-compat re-derives every mined id via git rev-parse <tag>:<path> and validates every cited tag against git tag --list. It needs full history, so it runs in a dedicated Replay-Compat Provenance job (fetch-depth: 0) and is selected by pnpm check:affected for any test/replay-compat/ change.

Coverage spans the surfaces #1417 named: pre-removal gesture forms (#1393 swipe/fling/rotate/gesture swipe durationMs), ${VAR}/quoting/--record-as projections, wait + landmark forms (#1349), target-v1 annotations, and context-header variants including the fail-fast on a conflicting key.

Pruned per review. This is a parser-compat suite, not a device matrix, so the initial 52-entry dataset is down to 22: an entry must be the sole witness of a shipped syntactic form or of a distinct migration refusal. Platform twins and adjacent-release re-recordings that differ only in coordinates/labels are gone (all iOS-simulator/macOS/linux mirrors of Android flows, the iOS copies of checkout-form/gesture-lab, and duplicate fling durationMs witnesses). note is now a required field naming what each entry witnesses, README.md documents the selection rule, and the test caps the corpus at 30 so regrowth has to be deliberate.

Gate cost: 51 tests, ~30ms in unit-core.

Link to Devin session: https://app.devin.ai/sessions/3340a872ac584fea82e03800f512c215
Requested by: @thymikee

@thymikee thymikee self-assigned this Jul 27, 2026
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.87 MB 1.87 MB 0 B
JS gzip 598.6 kB 598.6 kB 0 B
npm tarball 714.1 kB 714.1 kB +19 B
npm unpacked 2.50 MB 2.50 MB +104 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 27.6 ms 27.2 ms -0.4 ms
CLI --help 57.8 ms 59.0 ms +1.2 ms

Top changed chunks: no changes in the largest emitted chunks.

@thymikee

Copy link
Copy Markdown
Member

Two blocking guardrail gaps remain. (1) The “frozen, released” claim is not mechanically protected: corpus.test.ts only regex-checks recordedBy and repeats it in free-form source/file strings. It does not prove the tag is released or that the checked-in script still matches its historical source, so a future PR could rewrite both the frozen script and metadata while the gate stays green. Pin a released-tag/source-blob identity or digest per entry and verify the current corpus bytes against it. (2) The test reconstructs the parse boundary in the wrong order: it calls readReplayScriptMetadata before parseReplayScriptDetailed, while production parseReplayInput composes them in the reverse order. Multi-fault scripts can therefore assert a different code/hint than replay actually surfaces, and changes to the real composition boundary are unguarded. Exercise parseReplayInput(script, undefined) instead. Exact-head Coverage is also red on the inherited ios-system-ui benchmark gap, so this head is not ready.

@devin-ai-integration
devin-ai-integration Bot force-pushed the devin/1785159001-replay-compat-corpus branch from aca0972 to 9e5a56f Compare July 27, 2026 14:19
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

(1) The "frozen, released" claim is not mechanically protected … (2) the test reconstructs the parse boundary in the wrong order

Both addressed in 9e5a56f; branch is also rebased onto e8b779cb3.

(1) Identity is pinned, not asserted. The free-form source string is gone. Each entry carries a typed provenance:

  • mined (40 entries): the git object id of the historical blob. corpus.test.ts hashes the checked-in bytes (sha1("blob <len>\0" + bytes)) and must reproduce that id — so rewriting a frozen script fails the gate, and the id itself is not obtainable except from the released content.
  • derived (12 scripts/docs/ entries — released grammar surfaces with no shipped .ad blob to point at): SHA-256 of the bytes.

The "was this tag actually cut" half needs git, so it lives in pnpm check:replay-compat (scripts/check-replay-compat-provenance.ts): every cited tag is checked against git tag --list, every mined id re-derived via git rev-parse <recordedBy>:<path>, and the released blob byte-compared against the corpus file. Deliberately outside the unit lane — it requires full history and tags, which the fast lane can't assume. Current run: Verified 40 mined replay-compat scripts against their released blobs across 10 tags (12 derived entries are digest-pinned).

Tamper check: appending one line to a corpus script now fails … is byte-identical to its pinned source (Tests 1 failed | 106 passed).

(2) Parse boundary. The test now calls parseReplayInput(script, undefined) rather than composing the two calls itself. No verdict moved under the real ordering (all 52 re-derived through parseReplayInput), but the boundary is now guarded instead of duplicated.

Gates: format:check, lint, typecheck, check:layering, check:fallow, check:affected:test, check:replay-compat green; corpus lane 107 tests / 44 ms.

On Coverage: the ios-system-ui benchmark gap reproduces unchanged on a clean origin/main worktree, so it is inherited rather than from this head. I can add the benchmark case (or a waiver) here if you'd rather not land with it red — it's outside #1417, so tell me which you prefer.

@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-28 08:06 UTC

@thymikee

Copy link
Copy Markdown
Member

Re-review at head 9e5a56f: the parse-order issue is fixed, but one P1 provenance blocker remains. The historical verifier only checks entries while they remain kind mined. A mined corpus script can be edited, reclassified as derived, and given a matching digest in the same editable manifest; the verifier then skips it and the unit hash check accepts the new bytes. Lock provenance kind mechanically by corpus area (at minimum integration/examples must remain mined), add a negative reclassification test, and run check:replay-compat in a full-history CI job. The current green checks do not invoke that verifier. Once that provenance boundary is enforced, the shipped parseReplayInput path and action-before-metadata ordering look correct.

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

A mined corpus script can be edited, reclassified as derived, and given a matching digest…

Fixed in 2e80e27 (+ docs commit). The reclassification escape is closed at the rule level, not by convention:

test/replay-compat/provenance-rules.ts fixes the provenance kind by corpus area, so the manifest line no longer gets a vote:

const CORPUS_AREA_PROVENANCE_KIND = { integration: 'mined', examples: 'mined', docs: 'derived' };
requiredProvenanceKind('scripts/examples/gesture-lab.v0.16.8.ad'); // 'mined'
findProvenanceKindViolations(entries); // '…must stay "mined" … keep the released blob id instead'
  • Enforced in both places: corpus.test.ts (unit lane) and scripts/check-replay-compat-provenance.ts, so a relabelled entry fails even in the lane that can't reach git history.
  • Negative tests added: a mined entry cloned with { kind: 'derived', sha256 } yields exactly one violation, and an undeclared area (scripts/handwritten/…) throws rather than defaulting to the weaker pin — a new area must declare its kind before entries can live under it.

CI now invokes the verifier. New Replay-Compat Provenance job in ci.yml (fetch-depth: 0, fetch-tags: true) runs pnpm check:replay-compat. It's also a first-class replay-compat check in the affected selector: any change under test/replay-compat/ (script or manifest) selects it, covered by two new model.test.ts cases, and the existing "every catalog CI job maps to a real workflow job" parity gate ties the catalog entry to the workflow.

Green locally: format:check, lint, typecheck, check:layering, check:fallow, check:affected:test (33/33), corpus lane 110 tests, and check:replay-compatVerified 40 mined replay-compat scripts against their released blobs across 10 tags (12 derived entries are digest-pinned).

Coverage's ios-system-ui benchmark gap is still inherited from main; happy to fix it here if you want this head fully green, otherwise it stays outside #1417.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 27, 2026
@thymikee

Copy link
Copy Markdown
Member

Re-review at head c1b4af5: clean readiness verdict. The remaining provenance blocker is fixed: integration/examples corpus areas are mechanically locked to mined provenance, unknown areas fail closed, mined-to-derived reclassification has a negative test, and the full-history Replay-Compat Provenance CI job re-derives released blobs with tags. Corpus assertions still use production parseReplayInput in the correct action-before-metadata order. The PR is mergeable/CLEAN and all checks, including provenance, Coverage, and Integration Tests, pass. Marked ready-for-human.

thymikee and others added 4 commits July 27, 2026 16:18
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
… parseReplayInput

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
… in CI

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration
devin-ai-integration Bot force-pushed the devin/1785159001-replay-compat-corpus branch from c1b4af5 to aba1270 Compare July 27, 2026 16:25
Reviewer feedback on #1436: the mechanism earns its place, the dataset did not.
Drop the 30 corpus entries whose bytes repeat a syntactic form or a migration
refusal another entry already witnesses (platform twins and adjacent-release
re-recordings), leaving 22 deliberate entries; make note required and state per
entry which form or refusal it is the sole witness of.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@thymikee

Copy link
Copy Markdown
Member

Re-review at 09a0557: clean and ready for human review.\n\nThe 22-entry corpus retains every #1417 required parser surface and all retired-gesture migration refusals; removed fixtures are platform/adjacent-release duplicates. Required per-entry notes and the 30-entry cap make future additions deliberate. Provenance protections remain intact (area-kind lock, byte/digest pins, and full-history released-blob verification).\n\nValidated: focused corpus unit suite (51/51), local provenance verification, and the reviewed-head CI suite including Replay-Compat Provenance, Coverage, Integration Tests, Typecheck, Lint/Format, Fallow, and affected-check selector. Merge state: CLEAN.

@thymikee

Copy link
Copy Markdown
Member

Reviewed against #1417 including its 2026-07-27 amendments. Approve — this is the reference shape for corpus work under #1412.

Independently verified rather than trusted: 6 sampled mined entries' git blob ids re-derived from their release tags and matched manifest.ts; all 12 derived SHA-256s recomputed and matched; hint strings traced to the literal producer templates in src/replay/script.ts / src/contracts/gesture-normalization.ts (the no-hand-transcription rule is respected); parseReplayInput confirmed as the real daemon parse path; the Replay-Compat Provenance CI job actually ran and enforces released-tags-only per the amendment. 51 tests, 39ms, no new deps, 26/26 checks green.

Three nits, none blocking:

  • derived entries' provenance.from citation is reviewer-verified, not machine-checked (only the byte digest is). One README line saying so would keep future entries honest.
  • The <= 30 corpus cap is a fine deliberate-regrowth gate but the number is underived — a one-line comment on why 30 would do.
  • REQUIRED_COVERAGE hand-mirrors the ReplayCompatCoverage union; a satisfies-style exhaustiveness trick would let the type force the list.

🤖 Review by Claude Code

… derived-citation note)

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Thanks — all three nits addressed in c21e862:

  • README now says only the bytes of a derived entry are machine-checked: the from citation is reviewer-verified against the cited source at that tag, and forms/hints are copied from the release, never hand-transcribed.
  • The <= 30 cap carries its rationale: headroom over the 22 witnesses today's grammar needs, small enough that a re-mirrored fixture tree (52 entries at first cut) cannot land quietly.
  • REQUIRED_COVERAGE is now derived from Record<ReplayCompatCoverage, true>, so adding a tag to the coverage union fails to typecheck until it is required here.

51 tests, check:replay-compat, format/lint/typecheck still green locally.

@thymikee thymikee removed the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 28, 2026
@thymikee

Copy link
Copy Markdown
Member

P2 — test/replay-compat/README.md currently says derived entries should “copy” both the hint and form from the release. Historical releases can provide the emitted form, but a migration hint for a now-retired form belongs to today’s parser and may not exist at that tag (for example, the released rotate grammar emitted velocity, while the expected removal hint is current). Please split the rule: copy the form from the cited release; copy the expected failure code/hint from current parser output. Otherwise contributors cannot follow the guidance and may pin stale or invented verdict text.

thymikee and others added 2 commits July 28, 2026 06:27
… parser

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Re the P2 on the README rule — split in 9bc7478. The two sources are now separate: the form is copied from the cited release (that tag's bytes), the verdict comes from today's parser, since a refusal for a retired form did not exist at the tag that emitted it (v0.16.8 shipped gesture rotate <deg> <x> <y> <velocity>; the "drop the velocity" hint is current output). "Adding an entry" gets a dedicated step: start from verdict: { kind: 'parses' }, run the suite, and paste the reported code and hint substring verbatim — never transcribed from a release, an issue, or memory.

@thymikee

Copy link
Copy Markdown
Member

Re-reviewed 9bc7478: the corpus guidance now correctly separates the released form from today’s observed parser verdict/code/hint, and the adding-entry workflow is internally consistent. The delta is documentation-only, merge state is CLEAN, and all current checks—including Replay-Compat Provenance—pass. No remaining code-review findings.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 28, 2026
@thymikee
thymikee merged commit d747ef6 into main Jul 28, 2026
26 checks passed
@thymikee
thymikee deleted the devin/1785159001-replay-compat-corpus branch July 28, 2026 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test: frozen replay-compat corpus with expected verdicts

1 participant