Skip to content

test(cross-version-wire): compare published fields per frame - #17301

Merged
nwparker merged 1 commit into
stablyai:mainfrom
nwparker:nwparker/test-cross-version-frame-fields
Aug 30, 2026
Merged

nwparker merged 1 commit into
stablyai:mainfrom
nwparker:nwparker/test-cross-version-frame-fields

Conversation

@nwparker

@nwparker nwparker commented Aug 30, 2026 •

Copy link
Copy Markdown
Contributor

ELI5

The cross-version test checked which fields Orca publishes by combining all three snapshot-start frames into one set. That let one healthy frame hide a field missing from another. The test now compares initial to initial, reveal to reveal, and reconnect to reconnect.

Root cause

publishedFieldNames() unioned keys across every SnapshotStart occurrence. If the reveal frame stopped publishing seq, the initial and reconnect frames still contributed seq to the union, so the additive-only compatibility oracle stayed green.

This was an oracle defect, not a wire defect. The correction belongs in the cross-version test helper and adds no production behavior.

What changed

  • Read sorted field names from one published frame rather than a sequence-wide union.
  • Compare corresponding initial, reveal, and reconnect occurrences.
  • Reject unequal occurrence counts before comparison, avoiding silent zip truncation.
  • Keep occurrence labels in the terminal-domain assertion so failures name the exact journey step.
  • Add a real-suite mutation that clones the current reference, proves reveal currently has seq, removes only that field, and verifies the oracle fails with reveal and seq in the diagnostic.
  • Document why repeated frames must never be unioned.

Follow-up to #17178.

Red / green proof

Before the helper change, the reveal-only mutation failed its proving test:

AssertionError: expected [] to deeply equal [ 'seq' ]

The aggregate oracle saw no removal because sibling frames still published seq.

After the change, the same shape is detected at the reveal occurrence. The committed regression runs through the exact expectSnapshotStartFieldsRemainPublished → comparePublishedFieldOccurrences path used by the real baseline/current gate.

Exact-head verification

Published head: 5abd60553f397343869922a1babb1bdb425ef5ed

  • Focused helper + real terminal suite: 2 files / 17 tests passed.
  • pnpm tc: passed.
  • pnpm run check:code-quality:changed: zero code, type-aware, and React Doctor findings across 3 changed code files.
  • pnpm run check:reliability-gates: 99 gates valid.
  • pnpm exec oxfmt --check on all four changed files: passed.
  • git diff --check: passed.

Readiness and code elegance

Independent audits found zero proven P0/P1/P2 findings; counsel was unnecessary because there were no candidates.

  • Exactly four files change: three test/helper files and the existing remote-wire reference.
  • No production, protocol, RPC/IPC, SSH, relay, mobile, persistence, dependency, packaging, provider, platform, or workspace behavior changes.
  • The mutation reuses the already-loaded current reference; it adds no journey, checkout, process, network request, polling, or runtime cost beyond in-memory object cloning.
  • No release field list is hard-coded. Literal seq exists only as a mutation of the current build's observed contract.
  • Equal-count enforcement, direct helper coverage, real-oracle mutation coverage, and occurrence-specific diagnostics remain cohesive; no parallel comparator was added.
  • The regression is in cross-version-terminal-wire.unit.test.ts, which the dedicated cross-version CI job actually executes.

Native-host and Electron verification are N/A because this PR changes only test-oracle and documentation code.

Visual proof

N/A — no rendered Orca UI surface changes.

AI disclosure

AI-assisted implementation, mutation construction, and review were used. All claims above are tied to the exact committed diff and test output.

Checklist

  • Root oracle defect fixed rather than baseline symptoms allowlisted
  • No production or wire behavior changed
  • Red/green mutation proof recorded
  • Exact real CI oracle carries the regression
  • Typecheck, focused tests, reliability manifest, formatting, whitespace, and changed-code quality pass
  • Readiness and code-elegance reviews have zero findings
  • Fresh GitHub CI run 33282334469 on 5abd60553f3 completed cleanly (18 successful, 9 skipped, 0 failed or pending).

@greptile-apps

greptile-apps Bot commented Aug 30, 2026

Copy link
Copy Markdown

Greptile Summary

The PR corrects the cross-version terminal-wire compatibility oracle so repeated SnapshotStart frames are compared by corresponding journey occurrence rather than by a union of their fields.

  • Adds equal-count validation and per-occurrence field comparison.
  • Updates the terminal-wire suite to label initial, reveal, and reconnect failures precisely.
  • Adds focused helper coverage and a real-suite reveal-only field-removal regression.
  • Documents why repeated published frames must not be unioned.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete correctness, security, or independently actionable quality issues identified.

The changed helper’s callers are fully migrated, journey ordering and occurrence counts are validated before comparison, and the regression mutation exercises the production oracle without modifying shared reference data.

Important Files Changed

Filename Overview
tests/e2e/cross-version-wire/published-field-shape.ts Replaces sequence-wide key unioning with single-frame extraction and count-checked positional comparison.
tests/e2e/cross-version-wire/cross-version-terminal-wire.unit.test.ts Applies occurrence-specific comparisons to validated journey records and proves reveal-only field removal is detected.
tests/e2e/cross-version-wire/published-field-shape.unit.test.ts Covers sorted extraction, occurrence-specific removals, unequal counts, and existing skew behavior.
docs/reference/remote-wire-compatibility.md Documents occurrence-by-occurrence comparison and the masking defect caused by unioning repeated frame fields.

Reviews (1): Last reviewed commit: "test(cross-version-wire): compare publis..." | Re-trigger Greptile

@coderabbitai

coderabbitai Bot commented Aug 30, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 79fa70f0-5eea-47d1-987f-76661cf7cc62

📥 Commits

Reviewing files that changed from the base of the PR and between de851d5 and 5abd605.

📒 Files selected for processing (4)
  • docs/reference/remote-wire-compatibility.md
  • tests/e2e/cross-version-wire/cross-version-terminal-wire.unit.test.ts
  • tests/e2e/cross-version-wire/published-field-shape.ts
  • tests/e2e/cross-version-wire/published-field-shape.unit.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The wire field helpers now read keys from individual frames and compare older and newer frames by position. They reject mismatched occurrence counts. Terminal wire tests apply these checks to initial, reveal, and reconnect SnapshotStart frames. New regression coverage detects removal of seq from only the reveal frame. Documentation now describes occurrence-based comparison and warns against unioning keys across frames.

Merge Risk: ⚪ Minimal · up to 5abd6

This PR makes repeated wire-frame compatibility checks compare each corresponding frame instead of combining them, improving detection of test regressions without changing production behavior. No actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 3 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the main change: comparing published fields per cross-version frame.
Description check ✅ Passed The description is detailed and relevant. It explains the root cause, changes, rationale, regression proof, testing, visual impact, AI use, and scope. It also references linked issue #17178 and covers…
Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 3 files. (1 skipped: 1 unsupported.)

Full details: Description check

Explanation

The description is detailed and relevant. It explains the root cause, changes, rationale, regression proof, testing, visual impact, AI use, and scope. It also references linked issue #17178 and covers the required checklist topics, although some template headings differ.

  • Fix all pre-merge checks with AI

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@nwparker
nwparker merged commit 0942976 into stablyai:main Aug 30, 2026
27 checks passed
enrick-dev pushed a commit to enrick-dev/orca that referenced this pull request Oct 15, 2026
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