feat(contract): qa-contract v5 tags redesign + publish testnet contract 9tshSfq5#4031
Conversation
|
Warning Review limit reached
Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR introduces a v5 "tags" vocabulary for QA contract test cases, replacing Group/MultiWallet categories with cross-cutting tags. It adds tag validation and testId remapping in codes.mjs, wires tags through schema/parse/seed scripts, adds a migrate-runs.mjs script to migrate v4 test runs to v5, updates the registered testnet contract metadata, hardens register.mjs error logging, and revises TEST_PLAN.md accordingly. ChangesQA contract v5 tags and migration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Script as migrate-runs.mjs
participant V4Contract as v4 Contract
participant V5Contract as v5 Contract
Script->>V4Contract: query testCase docs (legacy category codes)
V4Contract-->>Script: legacy testId set
Script->>Script: remapTestId(oldId)
Script->>V4Contract: query testRun docs (owner, app, testId)
V4Contract-->>Script: v4 testRun documents
Script->>V5Contract: query existing testRun (result, buildRef)
V5Contract-->>Script: idempotency check result
Script->>V5Contract: create testRun (remapped fields)
V5Contract-->>Script: created or nonce error (retry)
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
✅ Review complete (commit 9f160c5) |
…n migrator v5 starts a fresh QA-results contract that dissolves the MultiWallet and Group *categories* into cross-cutting **tags** (a test's domain is its category; its modality is a tag). DPP rejects typed arrays, so tags are stored as a comma-separated string. - schema: add `testCase.tags`. - codes.mjs: categories minus MultiWallet/Group + canonical TAGS vocab + v4→v5 TESTID_REMAP (MW-*/GRP-* folded into their domain sequence). - seed.mjs / parse-test-plan.mjs: parse + validate the Tags column. - migrate-runs.mjs: re-point historical testRun.testId, reconnecting on nonce drift. - register.mjs: verbose error logging. - contract-id.testnet.json: canonical v5 contract 9tshSfq5 (owner 2YBjQKEP), which the QA dashboard reads. This lifts the tooling that registered the live testnet QA contract off a single local worktree and into review. No consensus/protocol code. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…_PLAN Reworks the QA test plan onto the v5 tags taxonomy so it matches the qa-contract tooling, while preserving current v4.1-dev content: - Add a **Tags** column + tag index; dissolve the MultiWallet (§4.13) and Group (§4.11) sections — their rows fold into their home categories (MW-* → ID-14/15, TOK-17, DPNS-08, DOC-15, SH-14/15/16, SYS-07/08; GRP-* → TOK-18/19/20, GRP-03 merged into TOK-15) tagged `multiwallet` / `group`. - Keep v4.1-dev's newer rows: DashPay DP-01..10 (Add Contact / Ignore / QR / contactInfo / backfill), ADDR-02/04 production sheets, and the fixed ADDR-09 no-double-credit regression guard. The cross-wallet DashPay loop lands at DP-11 (DashPay grew past the branch's DP-07). - Add ADDR-07 (BLAST platform-address sync) and ADDR-08 (clear & resync). - Recompute the §5 tier/layer counts and refresh the §1/§6 selectors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
4f74e73 to
9f160c5
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/swift-sdk/SwiftExampleApp/TEST_PLAN.md`:
- Line 27: The current wording conflates Category with the §4 section, but the
index maps rows like SH-11, CORE-21, and ID-06/ID-08/ID-11 to categories that
are intentionally separate from where they appear. Reword the TEST_PLAN.md
guidance around the category/tags explanation so Category remains a distinct
selection axis, and make sure the examples still point readers to use Tags plus
other columns for cross-cutting filtering.
- Around line 318-323: The tier summary matrix in TEST_PLAN is out of date after
the catalog reshuffle. Recompute and update the counts in the table so they
match the current catalog totals, especially the entries for Essential, Common,
Thorough, Uncommon, and Manual. Use the existing summary section in TEST_PLAN to
locate the stale matrix and replace it with the corrected totals from the
current row set.
In `@qa-contract/src/migrate-runs.mjs`:
- Around line 117-162: The nonce retry path in migrate-runs.mjs can duplicate a
testRun because the idempotency check is only done once before the create loop.
In the retry logic inside the `for (let attempt...)` block, after a
nonce-related error and `reconnect()`, re-query for the current `(newId,
run.result, run.buildRef)` or refresh the `present` set before creating a new
`Document`; if it already exists, skip the retry and count it as
migrated/skipped. Use the existing `queryAll`, `present`, and `reconnect` flow
to keep the `testRun` migration idempotent.
- Around line 34-41: queryAll() currently only returns the first page because it
always limits results to 100 and never advances pagination. Update queryAll to
keep calling sdk.documents.query with startAfter/startAt based on the last
document’s $id until a page returns fewer than 100 items, and keep collecting
toJSON() results across pages. Also make the callers that rely on paging,
including the testCase scan and idempotency lookup, pass an explicit stable
orderBy so queryAll can paginate deterministically.
In `@qa-contract/src/parse-test-plan.mjs`:
- Around line 93-102: Require a full 7-cell row before destructuring in
parse-test-plan.mjs: the current cells.length check in the row parsing logic
allows 6-cell rows through, which misassigns the notes column. Update the guard
in the test-plan parsing flow around the cells split/destructure so only rows
with all 7 expected columns are unpacked, preserving tagsCell and rest for valid
TEST_PLAN.md entries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 67933da8-49f5-4f84-9a7c-eb7890883ede
📒 Files selected for processing (8)
packages/swift-sdk/SwiftExampleApp/TEST_PLAN.mdqa-contract/contract-id.testnet.jsonqa-contract/schema/qa-contract.documents.jsonqa-contract/src/codes.mjsqa-contract/src/migrate-runs.mjsqa-contract/src/parse-test-plan.mjsqa-contract/src/register.mjsqa-contract/src/seed.mjs
…& docs - migrate-runs.mjs: `queryAll` now pages through every match via `startAfter` on the last doc's $id (was capped at the first 100), so per-test run history and the idempotency lookup can't silently truncate. - migrate-runs.mjs: after a nonce reconnect, re-verify the run didn't already land before retrying the create — a lost-ack false negative could otherwise duplicate a testRun. - parse-test-plan.mjs: require the full 7-cell row before destructuring so a short row can't shift Notes into the Tags cell. - TEST_PLAN.md (§1/§6): keep `Category` a distinct selection axis — a few rows are cross-cutting and belong to a category other than their §4 section (ID-06/08/11 → Address, SH-11 → Identity, CORE-21 → Shielded), resolved via the §6 index; tags cover cross-cutting modalities. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
….1-dev merge 675402b added the KotlinExampleApp (code 1) entry to the QA-contract codes; the v4.1-dev merge (92b9a93) brought in the v5 codes.mjs rewrite (#4031) and its resolution took the single-app version, silently dropping the entry — every Android submit-run/seed then fails with "Unknown app 'KotlinExampleApp'". The on-chain app doc (code 1) and all Android testCase/testRun rows were never affected; only the local lookup regressed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Issue being fixed or feature implemented
Preserves and opens for review the QA-contract v5 "tags" tooling — the branch that registered the current live testnet QA-results contract
9tshSfq5TU3gAEJCVa6KWxmb7cS7kkzRSmSSTmteKGcU(owner2YBjQKEP…) that the QA dashboard (https://dashpay.github.io/qa-dashboard-site/) reads.Until now this tooling lived only in a local, unpushed worktree — a durability risk (two prior QA contracts were lost exactly this way, when their tooling/keys lived somewhere ephemeral).
v5 dissolves the
MultiWalletandGroupcategories into cross-cutting tags (DPP rejects typed arrays, so tags are stored as a comma-separated string), remapsMW-*/GRP-*testIds into their domain categories, and adds a run migrator.What was done?
qa-contract/: v5 schema (testCase.tags),codes.mjs(categories minus MultiWallet/Group +TAGSvocab + v4→v5TESTID_REMAP),seed.mjs/parse-test-plan.mjstags support, newmigrate-runs.mjs(with nonce-drift reconnect),register.mjsverbose error logging.contract-id.testnet.jsonat the canonical9tshSfq5….packages/swift-sdk/SwiftExampleApp/TEST_PLAN.md: rebuilt onto the currentv4.1-devplan — adds the Tags column + tag index and dissolves the MultiWallet (§4.13) / Group (§4.11) sections into their home categories, while keeping v4.1-dev's newer content: DashPayDP-01..10(Add Contact / Ignore / QR / contactInfo / backfill), theADDR-02/ADDR-04production sheets, and the fixedADDR-09no-double-credit regression guard. The cross-wallet DashPay loop lands atDP-11(v4.1-dev grew DashPay past the branch's oldDP-07), socodes.mjsremapsMW-03 → DP-11.Branch hygiene (done)
This branch was long-diverged (branched 2026-06-21); it has now been rebased onto current
v4.1-dev:clear-all-banscommits — the diff is nowqa-contract/+TEST_PLAN.mdonly.TEST_PLAN.mdtags restructure against the current plan (3 structural conflicts, resolved in favour of v4.1-dev's newer rows).How Has This Been Tested?
The v5 contract is live on testnet and serving the QA dashboard: 122 testCases + migrated testRuns were seeded, and new runs record successfully via
submit-run.mjs. The reconciledTEST_PLAN.mdparses cleanly throughparse-test-plan.mjs(132 catalog rows, no duplicate testIds) and every tag validates against thecodes.mjsvocabulary.Breaking Changes
None — QA tooling + a testnet data contract only; no consensus/protocol code.
Checklist:
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation