Skip to content

Remove the deprecated check field from check results #47

Closed
#70
Closed
Remove the deprecated check field from check results#47
#70
Assignees
ottonomy
Labels
agent-readyJudged workable by an unattended agent right now

Description

@ottonomy
ottonomy
opened on Oct 7, 2026

What to build

Delete the deprecated check field from CheckResult. id is the identifier; suite stays.

This removes the result field only. The id a suite gives each check when it defines it — VerificationCheck.id, which the id computation takes as input — is unchanged, and so are the messages that quote it: "Not run: failed" reasons, the error detail for a check that threw, and the summary's "not run" message keep the check's local id.

  • Code that reads check switches to id — among others, the recognition-profile extraction in the verifier, and the summary code's two tests for .applies rows (the one that picks a skip reason for a suite that did not apply, and the one that selects rows to keep, if that still exists). Everything that builds a CheckResult stops writing check, including the parse-failure result.
  • Tests that read .check or build rows with a check key (about 145 lines across some 30 files) switch to id. This is not a find-and-replace: an id carries the phase, and a suite defined in a test with no phase produces ids beginning unknown.. Several Open Badges checks are named schema.obv3.* but run in suites named openbadges…, so their ids keep both names (for example semantic.openbadges.semantic.schema.obv3.result-ref).
  • Every example result row in the README and the API guides shows id and suite, not check. Most README rows have no id today, so each needs one added. The README's registry example reads .check in code and switches to id. The architecture doc's description of a check result's fields drops check.
  • The CHANGELOG's unreleased 2.0.0 section lists check as deprecated; replace that with its removal, and say in the migration notes that readers of check use id.

Acceptance

  • CheckResult has no check field, so pnpm run build and pnpm exec tsc -p tsconfig.dev.json reject any remaining read or object-literal write of it. (A bare search for check also hits unrelated variables and parameters of that name.)
  • git grep -n "\.check\b" -- src test finds nothing. Every match on main today is a read of a check result's field.
  • No example result row in the README or the API guides has a check key, and every one has an id.
  • The recognition profile is still extracted: an existing recognition test passes unchanged in what it asserts.
  • pnpm run build and pnpm test pass (build first: one test imports the package through its own exports, which resolve to the build output).
  • pnpm exec tsc -p tsconfig.dev.json — the only command that type-checks the tests — reports no error that it does not also report on the branch the work starts from. It reports six unrelated errors on main today; fixing them is not part of this ticket.

Effort: verifier-core/2026-09-29-2-0-readiness-review

Activity

  1. kayaelle
    added this to DCC Engineeringon Oct 7, 2026

  2. ottonomy commented on Oct 14, 2026

    @ottonomy
    ottonomy
    on Oct 14, 2026
    ContributorAuthor

    Body before refinement on 2026-10-14 (re-checked against main at 35f3efd, after #57–#62 and #66), kept for the record:

    ## What to build

Delete the deprecated `check` field from `CheckResult`. `id` is the identifier; `suite` stays.

- Code that reads `check` switches to `id` — among others, the recognition-profile extraction in the verifier and the summary code's test for `.applies` rows.
- Tests that read `.check` (about 130 lines across some 28 files) switch to `id`. This is not a find-and-replace: an `id` carries the phase, and a suite defined in a test with no phase produces ids beginning `unknown.`.
- Every example result row in the README and the API guides shows `id` and `suite`, not `check`. Note the removal in the CHANGELOG.

## Acceptance

- [ ] `CheckResult` has no `check` field, and nothing in the source or tests reads one.
- [ ] No example result row in the README or the API guides has a `check` key.
- [ ] `pnpm test` and `pnpm run build` pass, and the test files type-check too (the repo's dev TypeScript config covers them; neither vitest nor the build does).

Effort: verifier-core/2026-09-29-2-0-readiness-review

  3. ottonomy commented on Oct 14, 2026

    @ottonomy
    ottonomy
    on Oct 14, 2026
    ContributorAuthor

    Refined against main at 35f3efd. The claims still held; the test surface grew (#57–#60 added about 15 lines of .check reads). Decisions made while refining:

    Decided: say explicitly that VerificationCheck.id (the definition's id, the
  input to the id computation) is unchanged — only the result field goes.
Decided: "Not run: <check> failed" reasons, the thrown-check error detail and
  the summary's not-run message keep the local check id — changing message
  text was not asked for.
Decided: name the second .applies test in the summary code (the skip-reason
  picker) and the parse-failure result — the original named one of two .applies
  tests and not the parse-failure builder.
Decided: README example rows get an id added, not only check removed — most
  have no id today; the README's registry snippet (added by #60) reads .check
  in code and is not a "result row", so it is named separately.
Decided: replace the CHANGELOG's Deprecated entry with a removal and a migration
  note — the 2.0.0 section is unreleased, so the deprecation never shipped.
Decided: add the architecture doc's description of the check-result fields —
  it lists check and was not named.
Decided: use the compiler plus a `\.check\b` grep as the evidence — every grep
  hit on main today is a check-result read, so "finds nothing" is clean.
Ordering: this needs #46's work underneath it (id required and written by the
  suite runner). On main today the recognition extraction runs before ids are
  populated and 14 test files get rows with no id, so switching to id alone
  would silently find nothing. Run it stacked on #46, or after #46 merges.
  4. ottonomy
    added
    agent-readyJudged workable by an unattended agent right now
    on Oct 14, 2026
  5. ottonomy
    self-assigned this
    on Oct 14, 2026

  6. ottonomy commented on Oct 14, 2026

    @ottonomy
    ottonomy
    on Oct 14, 2026
    ContributorAuthor

    Night shift delivered.
    pull request #70
    branch nightshift/47-remove-the-deprecated-check-field-from-check-res → nightshift/46-make-check-ids-required-and-rename-suitesummary in digitalcredentials/verifier-core
    stacked on #69
    commits 1
    turns / cost 219 / $7.13
    Agent-Run 2ed717c0-eeaa-4367-a5fd-dd0646c4d406

    The pull request carries agent-authored. Review it; do not trust it
    blindly. Departures and decisions the ticket did not specify are in its body.

  7. ottonomy
    closed this as completedin #70on Oct 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

agent-readyJudged workable by an unattended agent right now

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions